Re: [PATCH] ARM: Kirkwood: Add SPI_CHPA and SPI_CPOL support to spi-orion

[Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>]

On Thu, Dec 06, 2012 at 02:25:21PM +0000, Grant Likely wrote:
> On Wed, 21 Nov 2012 12:23:35 -0700, Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> wrote:
> > Support these transfer modes from the SPI layer by setting
> > the appropriate register bits before doing the transfer.
> > 
> > This was tested on the Marvell kirkwood SOC that uses this driver.
> 
> Woo! a note about how it was tested. I can't (and often don't) see
> enough of these.  :-)

Thanks for looking at this Grant,

But I think that Jason's comment (see
https://patchwork.kernel.org/patch/1782061/) is valid.

This will likely switch all current users from using 'whatever the
firmware left behind' to 'whatever the kernel default is' - which will
surely break something here and there??

I was thinking of a transitory patch that did:

reg = readl(spi_reg(orion_spi, ORION_SPI_IF_CONFIG_REG));
nreg = reg & ~ORION_SPI_MODE_MASK;
if (spi->mode & SPI_CPOL)
	nreg |= ORION_SPI_MODE_CPOL;
if (spi->mode & SPI_CPHA)
	nreg |= ORION_SPI_MODE_CPHA;
#ifdef TRANSITION
 if (nreg != reg)
        dev_error(..,"Not allowing a SPI mode change away from firmware
 defaults. Board support needs to be updated!");
#else
 writel(nreg, spi_reg(orion_spi, ORION_SPI_IF_CONFIG_REG));
#endif

At least that way there is a warning and the DTSs can be fixed up.

I haven't had a moment to look into that though..

But I'm not sure that is in line with kernel philosophy WRT to DT..

What do you think?

Regards,
Jason

------------------------------------------------------------------------------
LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
Remotely access PCs and mobile devices and provide instant support
Improve your efficiency, and focus on delivering more value-add services
Discover what IT Professionals Know. Rescue delivers
http://p.sf.net/sfu/logmein_12329d2d