| Message ID | 20191219094559.4339-1-rasmus.villemoes@prevas.dk |
|---|---|
| State | Accepted |
| Commit | fddf876a8f56ff53d1354387ebf7fd3997189349 |
| Delegated to: | Mario Six |
| Headers | show |
| Series | mpc83xx_clk: always treat MPC83XX_CLK_PCI as invalid | expand |
On Thu, Dec 19, 2019 at 10:46 AM Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote: > > The current mpc83xx_clk driver is broken for any board for which > mpc83xx_has_pci() is true, i.e. anything not MPC8308: > > When is_clk_valid() reports that MPC83XX_CLK_PCI is valid, > init_all_clks() proceeds to call init_single_clk(), but that doesn't > know about either MPC83XX_CLK_PCI or has any handling of the > TYPE_SCCR_ONOFF mode correctly returned by retrieve_mode(). Hence > init_single_clk() ends up returning -EINVAL, and the whole board hangs > in serial_init(). > > The quickest fix is to simply pretend that clock is invalid for > all, since nobody can have been relying on it. Adding proper support > seems to be a bit more involved than just handling TYPE_SCCR_ONOFF: > > - The power-on-reset value of SCCR[PCICM] is 0, so > mpc83xx_clk_enable() would probably need to be tought to enable the > clock. > > - The frequency of PCI_SYNC_OUT is either SYS_CLK_IN or SYS_CLK_IN/2 > depending on the CFG_CLKIN_DIV configuration input, but that can't > be read from software, so to properly fill out > ->speed[MPC83XX_CLK_PCI] I think one would need guidance from > Kconfig or dtb. > > Partially fixes: 07d538d281 clk: Add MPC83xx clock driver > > Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk> > --- > drivers/clk/mpc83xx_clk.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/clk/mpc83xx_clk.c b/drivers/clk/mpc83xx_clk.c > index 32d2db9eda..1b2b216596 100644 > --- a/drivers/clk/mpc83xx_clk.c > +++ b/drivers/clk/mpc83xx_clk.c > @@ -65,7 +65,10 @@ static inline bool is_clk_valid(struct udevice *clk, int id) > case MPC83XX_CLK_DMAC: > return (type == SOC_MPC8308) || (type == SOC_MPC8309); > case MPC83XX_CLK_PCI: > - return mpc83xx_has_pci(type); > + /* > + * FIXME: implement proper support for this. > + */ > + return 0 && mpc83xx_has_pci(type); > case MPC83XX_CLK_CSB: > return true; > case MPC83XX_CLK_I2C2: > -- > 2.23.0 > That's OK as a workaround, but should probably be properly fixed (ideally with the addition of a proper DM PCI driver that works with the clock driver). But for now: Reviewed-by: Mario Six <mario.six@gdsys.cc> Applied to mpc83xx/next. Best regards, Mario
diff --git a/drivers/clk/mpc83xx_clk.c b/drivers/clk/mpc83xx_clk.c index 32d2db9eda..1b2b216596 100644 --- a/drivers/clk/mpc83xx_clk.c +++ b/drivers/clk/mpc83xx_clk.c @@ -65,7 +65,10 @@ static inline bool is_clk_valid(struct udevice *clk, int id) case MPC83XX_CLK_DMAC: return (type == SOC_MPC8308) || (type == SOC_MPC8309); case MPC83XX_CLK_PCI: - return mpc83xx_has_pci(type); + /* + * FIXME: implement proper support for this. + */ + return 0 && mpc83xx_has_pci(type); case MPC83XX_CLK_CSB: return true; case MPC83XX_CLK_I2C2:
The current mpc83xx_clk driver is broken for any board for which mpc83xx_has_pci() is true, i.e. anything not MPC8308: When is_clk_valid() reports that MPC83XX_CLK_PCI is valid, init_all_clks() proceeds to call init_single_clk(), but that doesn't know about either MPC83XX_CLK_PCI or has any handling of the TYPE_SCCR_ONOFF mode correctly returned by retrieve_mode(). Hence init_single_clk() ends up returning -EINVAL, and the whole board hangs in serial_init(). The quickest fix is to simply pretend that clock is invalid for all, since nobody can have been relying on it. Adding proper support seems to be a bit more involved than just handling TYPE_SCCR_ONOFF: - The power-on-reset value of SCCR[PCICM] is 0, so mpc83xx_clk_enable() would probably need to be tought to enable the clock. - The frequency of PCI_SYNC_OUT is either SYS_CLK_IN or SYS_CLK_IN/2 depending on the CFG_CLKIN_DIV configuration input, but that can't be read from software, so to properly fill out ->speed[MPC83XX_CLK_PCI] I think one would need guidance from Kconfig or dtb. Partially fixes: 07d538d281 clk: Add MPC83xx clock driver Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk> --- drivers/clk/mpc83xx_clk.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)