Messages in this thread | | | Subject | Re: [PATCH 1/3] ASoC: simple-card-utils: Fix device module clock | From | Sameer Pujar <> | Date | Sun, 14 Feb 2021 23:26:36 +0530 |
| |
Hi Morimoto-san,
On 2/12/2021 5:14 AM, Kuninori Morimoto wrote: >> diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c >> index bc0b62e..0754d70 100644 >> --- a/sound/soc/generic/simple-card-utils.c >> +++ b/sound/soc/generic/simple-card-utils.c >> @@ -173,16 +173,15 @@ int asoc_simple_parse_clk(struct device *dev, >> * or device's module clock. >> */ >> clk = devm_get_clk_from_child(dev, node, NULL); >> - if (!IS_ERR(clk)) { >> - simple_dai->sysclk = clk_get_rate(clk); >> + if (IS_ERR(clk)) >> + clk = devm_get_clk_from_child(dev, dlc->of_node, NULL); >> >> + if (!IS_ERR(clk)) { >> simple_dai->clk = clk; >> - } else if (!of_property_read_u32(node, "system-clock-frequency", &val)) { >> + simple_dai->sysclk = clk_get_rate(clk); >> + } else if (!of_property_read_u32(node, "system-clock-frequency", >> + &val)) { >> simple_dai->sysclk = val; >> - } else { >> - clk = devm_get_clk_from_child(dev, dlc->of_node, NULL); >> - if (!IS_ERR(clk)) >> - simple_dai->sysclk = clk_get_rate(clk); >> } > The comment is indicating that that the clock parsing order, > but this patch exchanges it. > This comment also should be updated, I think. > > /* > * Parse dai->sysclk come from "clocks = <&xxx>" > * (if system has common clock) > * or "system-clock-frequency = <xxx>" > * or device's module clock. > */
Yes, this can be rephrased now.
> asoc_simple_set_clk_rate() will be called if it has simple_dai->clk. > CPU or Codec component clock rate will be exchanged by this patch, I think. > I'm not sure the effect of this patch to existing boards.
If CPU or Codec node does not specifiy "mclk-fs" factor, asoc_simple_set_clk_rate() won't be called. So I don't think there would be any effect w.r.t clock rate. With this patch clocks would get enabled/disabled.
> > And also, this patch has too many unneeded exchange, > thus it was difficult to read for me. > I think it can be simply like this ? > It is understandable what it want to do.
I think the patch does exactly the same thing as what you are suggesting below. Am I missing anything?
> > diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c > index 8c423afb9d2e..d441890de4dc 100644 > --- a/sound/soc/generic/simple-card-utils.c > +++ b/sound/soc/generic/simple-card-utils.c > @@ -168,16 +168,14 @@ int asoc_simple_parse_clk(struct device *dev, > * or device's module clock. > */ > clk = devm_get_clk_from_child(dev, node, NULL); > + if (IS_ERR(clk)) > + clk = devm_get_clk_from_child(dev, dlc->of_node, NULL); > + > if (!IS_ERR(clk)) { > simple_dai->sysclk = clk_get_rate(clk); > - > simple_dai->clk = clk; > } else if (!of_property_read_u32(node, "system-clock-frequency", &val)) { > simple_dai->sysclk = val; > - } else { > - clk = devm_get_clk_from_child(dev, dlc->of_node, NULL); > - if (!IS_ERR(clk)) > - simple_dai->sysclk = clk_get_rate(clk); > } > > if (of_property_read_bool(node, "system-clock-direction-out"))
| |