Hi Mark Rutland
Thank you for your feedback
+- frame-master : bool property. add this if subnode was frame master +- bitclock-master : bool property. add this if subnode was bitclock master
s/was/is/
Oops, I will fix it on v5
+- bitclock-inversion : bool property. add this if subnode has clock inversion +- frame-inversion : bool property. add this if subnode has frame inversion +- clocks / system-clock-frequency : specify subnode's clock if needed.
it can be specified via "clocks" if system has clock node,or "system-clock-frequency" if system doesn't have it.What does "if system doesn't have it" mean? If it doesn't have a clock, how does said non-existent clock have a frequency?
It means "if system doesn't support common clock". I will fix it
+static int +asoc_simple_card_sub_parse_of(struct device_node *np,
struct asoc_simple_dai *dai,struct device_node **node)+{
- struct clk *clk;
- int ret;
- /*
* get node via "sound-dai = <&phandle port>"* it will be used as xxx_of_node on soc_bind_dai_link()*/- *node = of_parse_phandle(np, "sound-dai", 0);
- if (!*node)
return -ENODEV;- /* get dai->name */
- ret = snd_soc_of_get_dai_name(np, &dai->name);
- if (ret < 0)
goto parse_error;- /*
* bitclock-inversion, frame-inversion* bitclock-master, frame-master* and specific "format" if it has*/This comment looks confusing to me. I'm not sure it's all that helpful.
- dai->fmt = snd_soc_of_parse_daifmt(np, NULL);
As a general note, I'm surprised there isn't a helper that does all of the above, from of_parse_phandle to here.
snd_soc_of_parse_daifmt() is the helper fucntion, and above comment is explaining it. Or, am I misunderstanding your review ?
- /*
* dai->sysclk come from* "clolks = <&xxx>" or "system-clock-frequency = <xxx>"s/clolks/clocks/
Grr, thank you
- clk = of_clk_get(np, 0);
- if (IS_ERR(clk))
of_property_read_u32(np,"system-clock-frequency",&dai->sysclk);What it this isn't present?
If sysclk doesn't have common clock support
- /* simple-card assumes platform == cpu */
- *of_platform = *of_cpu;
Why? What does this imply?
This is the one of reason why this driver is "simple" card
Best regards --- Kuninori Morimoto