[U-Boot] [PATCH] spi: tegra: fix hand in set_mode()

[Tom Warren]

Jagan,

Are you planning on taking this in via the SPI repo, or can I take it in to Tegra?

Tom

> -----Original Message-----
> From: Stephen Warren [mailto:swarren at wwwdotorg.org]
> Sent: Saturday, August 20, 2016 7:17 PM
> To: Simon Glass <sjg at chromium.org>
> Cc: U-Boot Mailing List <u-boot at lists.denx.de>; Jagan Teki
> <jteki at openedev.com>; Tom Warren <TWarren at nvidia.com>; Stephen
> Warren <swarren at nvidia.com>; Mirza Krak <mirza.krak at hostmobility.com>
> Subject: Re: [PATCH] spi: tegra: fix hand in set_mode()
> 
> On 08/20/2016 05:52 PM, Simon Glass wrote:
> > Hi Stephen,
> >
> > On 18 August 2016 at 10:53, Stephen Warren <swarren at wwwdotorg.org>
> wrote:
> >> From: Stephen Warren <swarren at nvidia.com>
> >>
> >> In tegra20_slink.c, the set_mode() function may be executed before
> >> the SPI bus is claimed the first time, and hence the clocks to the
> >> SPI controller may not be running. If so, any register read/write at
> >> this time will hang the CPU. Fix this by ensuring the clock is
> >> running as soon as the driver is probed. This is observed on the Tegra30
> Beaver board.
> >>
> >> Apply the same clock initialization fix to all other Tegra SPI
> >> drivers so that if set_mode() is ever implemented there, the same bug will
> not appear.
> >> Note that tegra114_spi.c already operates in this fashion.
> >>
> >> The clock manipulation code is copied from claim_bus() to probe()
> >> rather than moved. This ensures that any calls to set_speed() take
> >> effect; the clock can't be set once during probe and left unchanged.
> >>
> >> Fixes: 5cb1b7b395c0 ("spi: tegra20: Add support for mode selection")
> >> Cc: Mirza Krak <mirza.krak at hostmobility.com>
> >> Signed-off-by: Stephen Warren <swarren at nvidia.com>
> >> ---
> >>  drivers/spi/tegra20_sflash.c | 4 ++++  drivers/spi/tegra20_slink.c
> >> | 4 ++++  drivers/spi/tegra210_qspi.c  | 3 +++
> >>  3 files changed, 11 insertions(+)
> >
> > Reviewed-by: Simon Glass <sjg at chromium.org>
> >
> > You might consider storing the mode and not acting on it until later.
> > But this is fine.
> 
> I did that in previous patch (I suppose you could call it V1, but it was so different
> in implementation that I didn't), but Jagan asserted that
> claim_bus() isn't the correct place to process the mode value, and that's the
> place any deferred value would need to be applied.
--
nvpublic