[PATCH 1/6] spi/mpc8xxx: refactor the common code for SPI/eSPI controller
grant.likely at secretlab.ca
Mon Jul 26 17:45:26 EST 2010
On Mon, Jul 26, 2010 at 1:07 AM, Zang Roy-R61911 <r61911 at freescale.com> wrote:
>> -----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
>> 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
>> 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.
Then the config item for the shared code should be renamed to reflect this.
More information about the Linuxppc-dev