Quoting Marek Szyprowski (2018-02-20 01:36:03) > Hi Robin, > > On 2018-02-19 17:29, Robin Murphy wrote: > > > > Seeing how every subsequent patch ends up with the roughly this same > > stanza: > > > > x = devm_clk_bulk_alloc(dev, num, names); > > if (IS_ERR(x) > > return PTR_ERR(x); > > ret = devm_clk_bulk_get(dev, x, num); > > > > I wonder if it might be better to simply implement: > > > > int devm_clk_bulk_alloc_get(dev, &x, num, names) > > > > that does the whole lot in one go, and let drivers that want to do > > more fiddly things continue to open-code the allocation. > > > > But perhaps that's an abstraction too far... I'm not all that familiar > > with the lie of the land here. > > Hmmm. This patchset clearly shows, that it would be much simpler if we > get rid of clk_bulk_data structure at all and let clk_bulk_* functions > to operate on struct clk *array[]. Typically the list of clock names > is already in some kind of array (taken from driver data or statically > embedded into driver), so there is little benefit from duplicating it > in clk_bulk_data. Sadly, I missed clk_bulk_* api discussion and maybe > there are other benefits from this approach. > > If not, I suggest simplifying clk_bulk_* api by dropping clk_bulk_data > structure and switching to clock ptr array: > > int clk_bulk_get(struct device *dev, int num_clock, struct clk *clocks[], > const char *clk_names[]); > int clk_bulk_prepare(int num_clks, struct clk *clks[]); > int clk_bulk_enable(int num_clks, struct clk *clks[]); > ... > If you have an array of pointers to names of clks then we can put the struct clk pointers adjacent to them at the same time. I suppose the problem there is that some drivers want to mark that array of pointers to names as const. And then we can't update the clk pointers next to them. This is the same design that regulators has done so that's why it's written like this for clks. Honestly, we're talking about a handful of pointers here so I'm not sure it really matters much. Just duplicate the pointer and be done if you want to mark the array of names as const or have your const 'setup' structure point to the bulk_data array that you define statically non-const somewhere globally. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
 |