[U-Boot] [RFC] [PATCH 0/4] Add support for Freescale's 85xx and P1/P2xxx eSPI controller

Reinhard Meyer
2010-09-28T11:02:06Z

Dear Can Aydin,
> The reason this is an RFC is that unfortunately the hardware on these
> chips does not permit indefinite SPI transactions on a given chip
> select. A chip select is asserted only when a 'transaction length' has
> been passed to the controller. Once the number of characters specified
> in the transaction length have been transmitted, the controller decides
> that the 'frame' has ended and de-asserts the chip select after a
> defined delay. It is not possible to initiate a second transfer without
> re-initialising the command register, and hence clearing and
> re-asserting a chip select signal.
>
> This patch set addresses the issue by defining a read/write function in
> the spi_flash_internal API. Subsequent patches add the freescale eSPI
> driver and add support for it in the spansion driver and the P1/P2 board
> configuration header.
>
> I'm pretty sure that there are better ways of doing this, especially if
> a driver model with support for driver quirks was implemented for
> instance. Until then however, I assume having some sort of ability to
> use the SPI controller on these boards would be better than not being
> able to do anything at all. If anyone has a better solution please feel
> free to comment.
Can the Chip Select Pins be used as GPIO? It might be simpler to do that than
adding a speciality to common code.

> mode change 100644 => 100755 drivers/mtd/spi/spansion.c
> mode change 100644 => 100755 drivers/mtd/spi/spi_flash.c
> mode change 100644 => 100755 drivers/mtd/spi/spi_flash_internal.h
> create mode 100755 drivers/spi/fsl_espi.c
> create mode 100755 include/fsl_espi.h
Having edited from W****** using Samba?
Make sure the file modes are and stay 644!

Without looking at the functionality I saw several coding style issues in
the patch series:
+ if ( cmd_len&& cmd )
+ memcpy(buffer, cmd, cmd_len);
should be like "if (cmd_len && cmd)"

Similar space issues seem to happen all over your patches,
examples:

+ for ( i = 0; i< size; i++)

+ return bus == 0&& cs< ESPI_MAX_CS_NUM;

+#define ESPI_COM_CS(x) ((x)<< 30)

You need to fix that globally...

Best Regards,

Reinhard

PS: you might want to wait and see if that patch in common code to
fix a special hardware issue is welcome@all ;)