[PATCH 1/6] spi/mpc8xxx: refactor the common code for SPI/eSPI controller

[Zang Roy-R61911]

> -----Original Message-----
> From: Grant Likely [mailto:glikely at secretlab.ca] On Behalf Of 
> Grant Likely
> Sent: Monday, July 26, 2010 8:14 AM
> To: Hu Mingkai-B21284; "@angua.secretlab.ca"@angua.secretlab.ca
> Cc: linuxppc-dev at ozlabs.org; galak at kernel.crashing.org; Zang 
> Roy-R61911
> Subject: Re: [PATCH 1/6] spi/mpc8xxx: refactor the common 
> code for SPI/eSPI controller
> 
> On Tue, Jul 20, 2010 at 10:08:20AM +0800, Mingkai Hu wrote:
> > Refactor the common code to file spi_mpc8xxx.c used by SPI/eSPI
> > controller driver, move the SPI controller driver to a new file
> > fsl_spi.c, and leave the QE/CPM SPI controller code in this file.
> > 
> > Because the register map of the SPI controller and eSPI controller
> > is so different, also leave the code operated the register to the
> > driver code, not the common code.
> > 
> > Signed-off-by: Mingkai Hu <Mingkai.hu at freescale.com>
> > ---
> >  drivers/spi/Kconfig       |   13 +-
> >  drivers/spi/Makefile      |    1 +
> >  drivers/spi/fsl_spi.c     | 1118 
> ++++++++++++++++++++++++++++++++++++++++++
> >  drivers/spi/spi_mpc8xxx.c | 1198 
> ++-------------------------------------------
> >  drivers/spi/spi_mpc8xxx.h |  135 +++++
> 
> Please name files spi-*.[ch].  I'm going to start enforcing 
> this naming convention in the drivers/spi directory.
> 
> >  5 files changed, 1299 insertions(+), 1166 deletions(-)
> >  create mode 100644 drivers/spi/fsl_spi.c
> >  create mode 100644 drivers/spi/spi_mpc8xxx.h
> > 
> > diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> > index 91c2f4f..cd564e2 100644
> > --- a/drivers/spi/Kconfig
> > +++ b/drivers/spi/Kconfig
> > @@ -183,11 +183,18 @@ config SPI_MPC512x_PSC
> >  	  Controller in SPI master mode.
> >  
> >  config SPI_MPC8xxx
> > -	tristate "Freescale MPC8xxx SPI controller"
> > +	bool
> 
> This should be tristate so it can be loaded as a module.
> 
> >  	depends on FSL_SOC
> >  	help
> > -	  This enables using the Freescale MPC8xxx SPI 
> controllers in master
> > -	  mode.
> > +	  This enables using the Freescale MPC8xxx SPI/eSPI controllers
> > +	  driver library.
> 
> Drop the help text entirely.  It isn't needed on 
> non-user-visible options.
> 
> > +
> > +config SPI_FSL_SPI
> > +	tristate "Freescale SPI controller"
> 
> "Freescale SPI controller is rather generic and doesn't give any clues
> as to which devices actually have this controller.  At the very least
> the help text should state what parts contain this controller.
> 
> On that note, the naming convention seems a little loose.  
> Since both the eSPI and SPI are using SPI_MPC8xxx, then 
> really the names should be:
> 
> config SPI_MPC8xxx_SPI
> and
> config SPI_MPC8xxx_ESPI

SPI_FSL_SPI and SPI_FSL_ESDP are better at this point.
83xx platforms and some of 85xx platforms use SPI controller. 
>From mpc8536, 85xx use ESP controller, BUT currently all P10xx, P20xx,
P40xx use ESPI controller. 
8xxx will not cover pxxx serial processor.
Thanks.
Roy