serial: clk: bcm2835: Strange effects when using aux-uart in console

[Stefan Wahren]

Hi Martin,

> Martin Sperl <kernel at martin.sperl.org> hat am 16. Februar 2016 um 17:31
> geschrieben:
>
>
>
> > On 13.02.2016, at 21:45, Stefan Wahren <stefan.wahren at i2se.com> wrote:
> >
> >
> > According to the datasheet busy bit shouldn't be set while changing the
> > clock.
> > So this isn't good. I hope this could be fixed, too.
> >
>
> I had hoped that the patch by Eric:
> "clk: bcm2835: Fix setting of PLL divider clock rates"
> would fix the issue, but that was unfortunately not the case.
>
> Some more investigation using the debugfs patch I had posted,
> which exposes the registers showed that:
> plld_per_a2w had a value of 4 before starting getty
> and a value of 0 after starting getty.
>
> That made me investigate why this value was changed and I found
> the culprit in the form of:
> @@ -1167,7 +1171,9 @@ static void bcm2835_pll_divider_off(struct clk_hw *hw)
> cprman_write(cprman, data->cm_reg,
> (cprman_read(cprman, data->cm_reg) &
> ~data->load_mask) | data->hold_mask);
> - cprman_write(cprman, data->a2w_reg, A2W_PLL_CHANNEL_DISABLE);
> + cprman_write(cprman, data->a2w_reg,
> + cprman_read(cprman, data->a2w_reg) |
> + A2W_PLL_CHANNEL_DISABLE);
> }

good job. This change make sense because the divider value shouldn't be zero.

But maybe we should protect the RMW by a lock like in bcm2835_clock_off?

>
> after that (with plld itself marked as “prepared” during probe) everything is
> working fine.
>
> bcm2835_pll_off shows a similar pattern, but patching that
> the same way:
> @@ -955,8 +955,12 @@ static void bcm2835_pll_off(struct clk_hw *hw)
> struct bcm2835_cprman *cprman = pll->cprman;
> const struct bcm2835_pll_data *data = pll->data;
>
> - cprman_write(cprman, data->cm_ctrl_reg, CM_PLL_ANARST);
> - cprman_write(cprman, data->a2w_ctrl_reg, A2W_PLL_CTRL_PWRDN);
> + cprman_write(cprman, data->cm_ctrl_reg,
> + cprman_read(cprman, data->cm_ctrl_reg) |
> + CM_PLL_ANARST);
> + cprman_write(cprman, data->a2w_ctrl_reg,
> + cprman_read(cprman, data->a2w_ctrl_reg) |
> + A2W_PLL_CTRL_PWRDN);
> }
>
> does not resolve the issue - the system crashes - but without he pll
> lock message…

Without the knowledge about the registers it's hard to say what exactly causes
the crash.

Regards
Stefan