Messages in this thread | | | Subject | Re: [PATCH 1/2] clk: bcm2835: Limit PCM clock to OSC and PLLD_PER | From | Phil Elwell <> | Date | Wed, 31 May 2017 09:28:31 +0100 |
| |
Hi Stefan,
On 30/05/2017 19:41, Stefan Wahren wrote: > Hi Phil, > >> Phil Elwell <phil@raspberrypi.org> hat am 30. Mai 2017 um 18:28 geschrieben: >> >> >> Restrict clock sources for the PCM peripheral to the oscillator and >> PLLD_PER because other source may have varying rates or be switched off. > >> Prevent other sources from being selected by replacing their names in >> the list of potential parents with dummy entries (entry index is >> significant). > > i like to have this as a comment above the definition of bcm2835_pcm_per_parents.
Sure - good idea.
>> >> Signed-off-by: Phil Elwell <phil@raspberrypi.org> >> --- >> drivers/clk/bcm/clk-bcm2835.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c >> index 0258538..facc346 100644 >> --- a/drivers/clk/bcm/clk-bcm2835.c >> +++ b/drivers/clk/bcm/clk-bcm2835.c >> @@ -1511,6 +1511,16 @@ struct bcm2835_clk_desc { >> "pllh_aux", >> }; >> >> +static const char *const bcm2835_pcm_per_parents[] = { > > As mentioned above, there should be a comment like all the others.
Yes, will do.
>> + "-", >> + "xosc", >> + "-", >> + "-", >> + "-", >> + "-", >> + "plld_per", >> +}; > > Is there a dummy entry for "pllh_aux" missing?
Yes and no - adding it will cause an extra iteration around the loop, but perhaps it's less confusing. I'll add one.
>> + >> #define REGISTER_PER_CLK(...) REGISTER_CLK( \ >> .num_mux_parents = ARRAY_SIZE(bcm2835_clock_per_parents), \ >> .parents = bcm2835_clock_per_parents, \ >> @@ -2000,6 +2010,7 @@ struct bcm2835_clk_desc { >> .int_bits = 12, >> .frac_bits = 12, >> .is_mash_clock = true, >> + .parents = bcm2835_pcm_per_parents, > > This looks a little bit hacky to me. Not sure, but can we do something like this? > > #define REGISTER_PCM_CLK(...) REGISTER_CLK( \ > .num_mux_parents = ARRAY_SIZE(bcm2835_pcm_per_parents), \ > .parents = bcm2835_pcm_per_parents, \ > __VA_ARGS__)
Of course - no problem.
Thanks for the feedback - it will be incorporated into V2.
Phil
| |