| Message ID | 1452867667-2447-2-git-send-email-kernel@martin.sperl.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
kernel@martin.sperl.org writes: > From: Martin Sperl <kernel@martin.sperl.org> > > Testing with different clock divider values has shown > that (at least for the PCM clock) the clock divider > has to be at least 2, otherwise the clock will not > output a signal. For a MASH clock (PWM, PCM, SLIMBUS, but not the others), the minimum integer component of the divider is: mash 0: 1 mash 1: 2 mash 2: 3 mash 3: 5
Eric Anholt <eric@anholt.net> writes: > kernel@martin.sperl.org writes: > >> From: Martin Sperl <kernel@martin.sperl.org> >> >> Testing with different clock divider values has shown >> that (at least for the PCM clock) the clock divider >> has to be at least 2, otherwise the clock will not >> output a signal. > > For a MASH clock (PWM, PCM, SLIMBUS, but not the others), the minimum > integer component of the divider is: > > mash 0: 1 > mash 1: 2 > mash 2: 3 > mash 3: 5 More specific MASH list: GP0 GP1 (*not* gp2) PCM PWM SLIM
On 02.02.2016 00:15, Eric Anholt wrote: > kernel@martin.sperl.org writes: > >> From: Martin Sperl <kernel@martin.sperl.org> >> >> Testing with different clock divider values has shown >> that (at least for the PCM clock) the clock divider >> has to be at least 2, otherwise the clock will not >> output a signal. > > For a MASH clock (PWM, PCM, SLIMBUS, but not the others), the minimum > integer component of the divider is: > > mash 0: 1 > mash 1: 2 > mash 2: 3 > mash 3: 5 > I know that that is what the datasheet says - see also the errata: http://elinux.org/BCM2835_datasheet_errata#p105_table. Experimentation has show that a divider between 1 and 1 + 4095 / 4096 does not provide any output PCM clock - only 2 and above does work for mash = 0, 1, 2 or 3. Example: Requesting 12288000Hz (=192kHz at 64bit) with the 19.2Mhz oscillator results in a divider of: 1 + 2304 / 4096 and this does not give a clock output. See the report by hiassoft here: https://github.com/raspberrypi/linux/issues/1231#issuecomment-171003182 (note that it also applies to mash = 0, but there may be no comment on this fact in this thread) Note that there is patch 7 that implements the above "mash" limits for a divider and downgrades to a lower mash level if the divider does not qualify. Martin
On 02.02.2016 02:52, Eric Anholt wrote: > Eric Anholt <eric@anholt.net> writes: > >> kernel@martin.sperl.org writes: >> >>> From: Martin Sperl <kernel@martin.sperl.org> >>> >>> Testing with different clock divider values has shown >>> that (at least for the PCM clock) the clock divider >>> has to be at least 2, otherwise the clock will not >>> output a signal. >> >> For a MASH clock (PWM, PCM, SLIMBUS, but not the others), the minimum >> integer component of the divider is: >> >> mash 0: 1 >> mash 1: 2 >> mash 2: 3 >> mash 3: 5 > > More specific MASH list: > > GP0 > GP1 > (*not* gp2) > PCM > PWM > SLIM > I got the list from the broadcom provided headers for the VC4. and where CM_*_DIV range does not start at 12 we have a fractional divider (also requires if CM_*_FRAC or CM_*_MASH is set) And if CM_*_MASH is set we got a mash enabled clock: CM_GNRICCTL_MASH CM_GP0CTL_MASH CM_GP1CTL_MASH CM_PCMCTL_MASH CM_PWMCTL_MASH CM_SLIMCTL_MASH And this is what can get implemented by configuring mash in the DT - otherwise only frac (a.k.a. MASH = 1) is used for any clock that has .frac_bits > 0.
Martin Sperl <kernel@martin.sperl.org> writes: > On 02.02.2016 00:15, Eric Anholt wrote: >> kernel@martin.sperl.org writes: >> >>> From: Martin Sperl <kernel@martin.sperl.org> >>> >>> Testing with different clock divider values has shown >>> that (at least for the PCM clock) the clock divider >>> has to be at least 2, otherwise the clock will not >>> output a signal. >> >> For a MASH clock (PWM, PCM, SLIMBUS, but not the others), the minimum >> integer component of the divider is: >> >> mash 0: 1 >> mash 1: 2 >> mash 2: 3 >> mash 3: 5 >> > > I know that that is what the datasheet says - see also the errata: > http://elinux.org/BCM2835_datasheet_errata#p105_table. > > Experimentation has show that a divider between 1 and 1 + 4095 / 4096 > does not provide any output PCM clock - only 2 and above does work for > mash = 0, 1, 2 or 3. > > Example: Requesting 12288000Hz (=192kHz at 64bit) with the 19.2Mhz > oscillator results in a divider of: 1 + 2304 / 4096 and this does not > give a clock output. > > See the report by hiassoft here: > https://github.com/raspberrypi/linux/issues/1231#issuecomment-171003182 > (note that it also applies to mash = 0, but there may be no comment on > this fact in this thread) > > Note that there is patch 7 that implements the above "mash" limits > for a divider and downgrades to a lower mash level if the divider > does not qualify. I do see "2, 2, 3, 5" being used as the minimum dividers used for mash clocks in the firmware's clock setup. So, as long as you limit this to the MASH clocks, I think you're right. What I'd expect of this series as a respin: 1) Fix setup of existing MASH clocks - Mark the few existing MASH clocks as being MASH. - Apply MASH integer divider limits to MASH clocks. - Set the MASH field on MASH clocks. 2) Add PCM clock If you want to add some other clocks that are definitely the same kinds of clock generators and where we have known parents for them, that's fine with me.
diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c index 015e687..10e97b7 100644 --- a/drivers/clk/bcm/clk-bcm2835.c +++ b/drivers/clk/bcm/clk-bcm2835.c @@ -1178,7 +1178,11 @@ static u32 bcm2835_clock_choose_div(struct clk_hw *hw, div &= ~unused_frac_mask; /* Clamp to the limits. */ - div = max(div, unused_frac_mask + 1); + + /* divider must be >= 2 */ + div = max_t(u32, div, (2 << CM_DIV_FRAC_BITS)); + + /* clamp to max divider allowed */ div = min_t(u32, div, GENMASK(data->int_bits + CM_DIV_FRAC_BITS - 1, CM_DIV_FRAC_BITS - data->frac_bits));