Am 31.05.2017 um 10:28 schrieb Phil Elwell:
> Hi Stefan,
>
> On 30/05/2017 19:41, Stefan Wahren wrote:
>> Hi Phil,
>>
>>> Phil Elwell <[email protected]> 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 <[email protected]>
>>> ---
>>> 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.
In case you want to save an iteration, you could add short comment
instead of a dummy entry.
>
>>> +
>>> #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
