> On 10.01.2016, at 19:56, Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote: > > Hi Martin, > > On Sun, Jan 10, 2016 at 7:01 PM, Martin Sperl <kernel@xxxxxxxxxxxxxxxx> wrote: >>> On 10.01.2016, at 14:02, Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote: >>> I wrote: >>> | Hence it can better be replaced (it seems to be unused in dts files, but you >>> | can keep the definition to be 100% sure) by an ARRAY_SIZE() in the C driver. >>> | This requires changing the driver to e.g. initialize clks[] in >>> |bcm2835_clk_probe() based on a table instead of explicit code. >>> >>> If you fill in clks[] from a static table, you can take ARRAY_SIZE of >>> the static table. >> >> You mean something like the below? >> (note: copy/paste from console issues - spaces instead of tabs) > > More or less. > >> }; >> >> static inline void cprman_write(struct bcm2835_cprman *cprman, u32 reg, u32 val >> @@ -496,6 +496,14 @@ static const struct bcm2835_pll_data bcm2835_pllh_data = { >> .max_fb_rate = BCM2835_MAX_FB_RATE, >> }; >> >> +static const struct bcm2835_pll_data *bcm2835_plls[] = { >> + [BCM2835_PLLA] = &bcm2835_plla_data, >> + [BCM2835_PLLB] = &bcm2835_pllb_data, >> + [BCM2835_PLLC] = &bcm2835_pllc_data, >> + [BCM2835_PLLD] = &bcm2835_plld_data, >> + [BCM2835_PLLH] = &bcm2835_pllh_data, >> +}; > > This is a sparse array? Yes - for all. > >> struct bcm2835_pll { >> struct clk_hw hw; >> struct bcm2835_cprman *cprman; >> @@ -1560,8 +1600,13 @@ static int bcm2835_clk_probe(struct platform_device *pdev) >> struct clk **clks; >> struct bcm2835_cprman *cprman; >> struct resource *res; >> + const int clks_cnt = max(ARRAY_SIZE(bcm2835_plls), >> + max(ARRAY_SIZE(bcm2835_pll_divs), >> + ARRAY_SIZE(bcm2835_clks))) + 1; >> + size_t alloc = sizeof(*cprman) + clks_cnt * sizeof(*clks); >> + size_t i; > > If you combine all 3 arrays in a single non-sparse array, you could get rid > of the dynamic allocation using the maximum of the 3 sizes, and can just > use a single ARRAY_SIZE(). The problem is that we need different initialization methods for pll, pll-dividers and derived clocks, so we can not easily make them a common setting unless we would use function and void pointers to work arround this, which would make it less readable (and more code again just for the - repeated - assignment). As far as I can tell from my testing the patched version works and the kernel boots correctly. I will respin the patchset tomorrow - it might take slightly longer, as I found that the fractional clock divider is not working unless the MASH functionality is activated in the registers. This results in “frequency shifts” for audio, which we want to avoid - this means we need another patch to enable this MASH feature. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html