On Tue, Jan 14, 2014 at 05:12:58PM +0100, Jean-Francois Moine wrote: > Mark Brown <broo...@kernel.org> wrote:
> > Please send this as a patch series to aid review, one patch doing four > > different changes is much harder to review. > As there are other bugs to fix, I may put back the 'of_device_is_available', > but there are not 3 different changes: I just explain the visible > effects of the patch. The patch itself is, as the subject says, > 'simplify code', that is, 'have a simpler code with no change in the > logic'. There's several different simplifications going on here, or at least it sounded that way. For example creating a private data struct doesn't seem obviously related to deleting unused fields from the platform data. The larger a change is the more benefit there is from a series of mechanical individual updates rather than several at once. > > > ret = asoc_simple_card_sub_parse_of(np, > > > - &info->cpu_dai, > > > - of_cpu); > > > + &priv->cpu_dai, > > > + (struct device_node **) > > > + &dai_link->cpu_of_node, > > > + &dai_link->cpu_dai_name); > > What's this cast here for? That code doesn't look at all safe. > dai_link->cpu_of_node is 'const struct device_node *' and both > of_clk_get() and of_node_put() want 'struct device_node *'. So, there > must be a cast somewhere. > Do you prefer I put these ones when calling the 'of_xx' functions? No, I think this stuff needs to actually be type safe with no dodgy casts. I'm not sure why we're doing an of_node_put(), for the of_clk_get() it's not immediately obvious why it's not taking a const clk. Or perhaps the pointer shouldn't be stored as const.
signature.asc
Description: Digital signature