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

Reinhard Meyer u-boot at emk-elektronik.de
Tue Sep 28 13:02:06 CEST 2010


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 at all ;)


More information about the U-Boot mailing list