Re: [U-Boot] [PATCH v3 1/8] spi: add support of SPI flash commands
Hi Jagan,
Thank you for your comments.
I will rework this patch set according to your advice. Thank you!
Best Regards,
Wenyou Yang
> -Original Message-
> From: Jagan Teki [mailto:jagannadh.t...@gmail.com]
> Sent: 2017年8月30日 21:50
> To: Wenyou Yang - A41535
> Cc: U-Boot Mailing List ; Marek Vasut ;
> Cyrille Pitchen ; Jagan Teki
> Subject: Re: [U-Boot] [PATCH v3 1/8] spi: add support of SPI flash commands
>
> On Tue, Jul 25, 2017 at 12:30 PM, Wenyou Yang
> wrote:
> > From: Cyrille Pitchen
> >
> > This patch introduces 'struct spi_flash_command' and functions
> > spi_is_flash_command_supported() / spi_exec_flash_command().
> >
>
> Answer for why this shouldn't be part of SPI area.
>
> [snip]
>
> drivers/spi and include/spi.h is Generic SPI area code - this can deal all
> connected slave devices like EEPROM, SPI-NOR etc drivers/mtd/spi and
> include/spi_flash.h is SPI-Flash or SPI-NOR code - this can handle only SPI
> flashes.
>
> >
> > +bool dm_spi_is_flash_command_supported(struct udevice *dev,
> > + const struct spi_flash_command
> > +*cmd) {
> > + struct udevice *bus = dev->parent;
> > + struct dm_spi_ops *ops = spi_get_ops(bus);
> > +
> > + if (ops->is_flash_command_supported)
> > + return ops->is_flash_command_supported(dev, cmd);
> > +
> > + return false;
> > +}
> > +
> > +int dm_spi_exec_flash_command(struct udevice *dev,
> > + const struct spi_flash_command *cmd) {
> > + struct udevice *bus = dev->parent;
> > + struct dm_spi_ops *ops = spi_get_ops(bus);
> > +
> > + if (ops->exec_flash_command)
> > + return ops->exec_flash_command(dev, cmd);
> > +
> > + return -EINVAL;
> > +}
> > +
> > int spi_claim_bus(struct spi_slave *slave) {
> > return dm_spi_claim_bus(slave->dev); @@ -107,6 +131,18 @@ int
> > spi_xfer(struct spi_slave *slave, unsigned int bitlen,
> > return dm_spi_xfer(slave->dev, bitlen, dout, din, flags); }
> >
> > +bool spi_is_flash_command_supported(struct spi_slave *slave,
> > + const struct spi_flash_command
> > +*cmd) {
> > + return dm_spi_is_flash_command_supported(slave->dev, cmd); }
> > +
> > +int spi_exec_flash_command(struct spi_slave *slave,
> > + const struct spi_flash_command *cmd) {
> > + return dm_spi_exec_flash_command(slave->dev, cmd); }
>
> Handling spi-flash stuff in spi core is not proper way of dealing, and I saw
> these
> functions are not-at-all needed for generic controller drivers in drivers/spi
> except
> the Atmel QSPI driver (in v3,8/8).
>
> > +
> > #if !CONFIG_IS_ENABLED(OF_PLATDATA)
> > static int spi_child_post_bind(struct udevice *dev) { @@ -144,6
> > +180,10 @@ static int spi_post_probe(struct udevice *bus)
> > ops->set_mode += gd->reloc_off;
> > if (ops->cs_info)
> > ops->cs_info += gd->reloc_off;
> > + if (ops->is_flash_command_supported)
> > + ops->is_flash_command_supported += gd->reloc_off;
> > + if (ops->exec_flash_command)
> > + ops->exec_flash_command += gd->reloc_off;
> > #endif
> >
> > return 0;
> > diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index
> > 7d81fbd7f8..e47acdc9e4 100644
> > --- a/drivers/spi/spi.c
> > +++ b/drivers/spi/spi.c
> > @@ -5,6 +5,7 @@
> > */
> >
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -58,3 +59,15 @@ struct spi_slave *spi_base_setup_slave_fdt(const void
> *blob, int busnum,
> > return spi_setup_slave(busnum, cs, max_hz, mode); } #endif
> > +
> > +__weak bool spi_is_flash_command_supported(struct spi_slave *slave,
> > + const struct
> > +spi_flash_command *cmd) {
> > + return false;
> > +}
> > +
> > +__weak int spi_exec_flash_command(struct spi_slave *slave,
> > + const struct spi_flash_command *cmd)
> > +{
> > + return -EINVAL;
> > +}
> > diff --git a/include/spi.h b/include/spi.h index
> > 8c4b882c54..a090266b52 100644
> > --- a/include/spi.h
> > +++ b/include/spi.h
> > @@ -10,6 +10,8 @@
> > #ifndef _SPI_H_
> > #define _SPI_H_
> >
> > +#include
Re: [U-Boot] [PATCH v3 1/8] spi: add support of SPI flash commands
On Tue, Jul 25, 2017 at 12:30 PM, Wenyou Yang wrote:
> From: Cyrille Pitchen
>
> This patch introduces 'struct spi_flash_command' and functions
> spi_is_flash_command_supported() / spi_exec_flash_command().
>
Answer for why this shouldn't be part of SPI area.
[snip]
drivers/spi and include/spi.h is Generic SPI area code - this can deal
all connected slave devices like EEPROM, SPI-NOR etc
drivers/mtd/spi and include/spi_flash.h is SPI-Flash or SPI-NOR code -
this can handle only SPI flashes.
>
> +bool dm_spi_is_flash_command_supported(struct udevice *dev,
> + const struct spi_flash_command *cmd)
> +{
> + struct udevice *bus = dev->parent;
> + struct dm_spi_ops *ops = spi_get_ops(bus);
> +
> + if (ops->is_flash_command_supported)
> + return ops->is_flash_command_supported(dev, cmd);
> +
> + return false;
> +}
> +
> +int dm_spi_exec_flash_command(struct udevice *dev,
> + const struct spi_flash_command *cmd)
> +{
> + struct udevice *bus = dev->parent;
> + struct dm_spi_ops *ops = spi_get_ops(bus);
> +
> + if (ops->exec_flash_command)
> + return ops->exec_flash_command(dev, cmd);
> +
> + return -EINVAL;
> +}
> +
> int spi_claim_bus(struct spi_slave *slave)
> {
> return dm_spi_claim_bus(slave->dev);
> @@ -107,6 +131,18 @@ int spi_xfer(struct spi_slave *slave, unsigned int
> bitlen,
> return dm_spi_xfer(slave->dev, bitlen, dout, din, flags);
> }
>
> +bool spi_is_flash_command_supported(struct spi_slave *slave,
> + const struct spi_flash_command *cmd)
> +{
> + return dm_spi_is_flash_command_supported(slave->dev, cmd);
> +}
> +
> +int spi_exec_flash_command(struct spi_slave *slave,
> + const struct spi_flash_command *cmd)
> +{
> + return dm_spi_exec_flash_command(slave->dev, cmd);
> +}
Handling spi-flash stuff in spi core is not proper way of dealing, and
I saw these functions are not-at-all needed for generic controller
drivers in drivers/spi except the Atmel QSPI driver (in v3,8/8).
> +
> #if !CONFIG_IS_ENABLED(OF_PLATDATA)
> static int spi_child_post_bind(struct udevice *dev)
> {
> @@ -144,6 +180,10 @@ static int spi_post_probe(struct udevice *bus)
> ops->set_mode += gd->reloc_off;
> if (ops->cs_info)
> ops->cs_info += gd->reloc_off;
> + if (ops->is_flash_command_supported)
> + ops->is_flash_command_supported += gd->reloc_off;
> + if (ops->exec_flash_command)
> + ops->exec_flash_command += gd->reloc_off;
> #endif
>
> return 0;
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 7d81fbd7f8..e47acdc9e4 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -5,6 +5,7 @@
> */
>
> #include
> +#include
> #include
> #include
> #include
> @@ -58,3 +59,15 @@ struct spi_slave *spi_base_setup_slave_fdt(const void
> *blob, int busnum,
> return spi_setup_slave(busnum, cs, max_hz, mode);
> }
> #endif
> +
> +__weak bool spi_is_flash_command_supported(struct spi_slave *slave,
> + const struct spi_flash_command
> *cmd)
> +{
> + return false;
> +}
> +
> +__weak int spi_exec_flash_command(struct spi_slave *slave,
> + const struct spi_flash_command *cmd)
> +{
> + return -EINVAL;
> +}
> diff --git a/include/spi.h b/include/spi.h
> index 8c4b882c54..a090266b52 100644
> --- a/include/spi.h
> +++ b/include/spi.h
> @@ -10,6 +10,8 @@
> #ifndef _SPI_H_
> #define _SPI_H_
>
> +#include /* memset() */
> +
> /* SPI mode flags */
> #define SPI_CPHA BIT(0) /* clock phase */
> #define SPI_CPOL BIT(1) /* clock polarity */
> @@ -64,6 +66,116 @@ struct dm_spi_slave_platdata {
> #endif /* CONFIG_DM_SPI */
>
> /**
> + * enum spi_flash_protocol - SPI flash command protocol
> + */
> +#define SPI_FPROTO_INST_SHIFT 16
> +#define SPI_FPROTO_INST_MASK GENMASK(23, 16)
> +#define SPI_FPROTO_INST(nbits) \
> + unsigned long)(nbits)) << SPI_FPROTO_INST_SHIFT) & \
> +SPI_FPROTO_INST_MASK)
> +
> +#define SPI_FPROTO_ADDR_SHIFT 8
> +#define SPI_FPROTO_ADDR_MASK GENMASK(15, 8)
> +#define SPI_FPROTO_ADDR(nbits) \
> + unsigned long)(nbits)) << SPI_FPROTO_ADDR_SHIFT) & \
> +SPI_FPROTO_ADDR_MASK)
> +
> +#define SPI_FPROTO_DATA_SHIFT 0
> +#define SPI_FPROTO_DATA_MASK GENMASK(7, 0)
> +#define SPI_FPROTO_DATA(nbits) \
> + unsigned long)(nbits)) << SPI_FPROTO_DATA_SHIFT) & \
> +SPI_FPROTO_DATA_MASK)
> +
> +#define SPI_FPROTO(inst_nbits, addr_nbits, data_nbits) \
> + (SPI_FPROTO_INST(inst_nbits) | \
> +SPI_FPROTO_ADDR(addr_nbits) | \
> +SPI_FPROTO_DATA(data_n
[U-Boot] [PATCH v3 1/8] spi: add support of SPI flash commands
From: Cyrille Pitchen
This patch introduces 'struct spi_flash_command' and functions
spi_is_flash_command_supported() / spi_exec_flash_command().
The 'struct spi_flash_command' describes all the relevant parameters to
execute any SPI flash command:
- the instruction op code
- the number of bytes used to send the address: 0, 3 or 4 bytes
- the number of mode and wait-state clock cycles, also called dummy cycles
- the number and values of data bytes to be sent or received
- the SPI x-y-z protocol [1]
- the flash command type [2]
[1] SPI x-y-z protocol:
- x is the number of I/O lines used to send the instruction op code.
- y is the number of I/O lines used during address, mode and wait-state
clock cycles.
- z is the number of I/O lines used to send or received data.
[2] Flash command type:
The flash command type is provided to differenciate "memory"
read/write/erase operations from "flash internal register" read/write
operations. Indeed some SPI controller drivers handle those command type
in different ways.
However SPI controller drivers should not check the value of the
instruction op code to guess the actual kind of flash command to perform.
Many instruction op codes are SPI flash manufacturer specific and only
drivers/mtd/spi/spi_flash.c should have the knowledge of all of them.
Besides, more and more QSPI controllers, like those of TI and Candence,
have special way to support (Fast) Read operations using some
"memory like" area mapped into the system bus. Hence, if those drivers
choose to override the default implementation of
spi_is_flash_command_supported() so that their own functions return true
only for a "memory read" flash command type, then spi_exec_flash_command()
might be used to implement the read from the "memory like" area mapped
into the system bus.
It means that spi_exec_flash_command() could be used to supersede the
actual flash->memory_map mechanism; spi_is_flash_command_supported() /
spi_exec_flash_command() being more generic and covering more use cases.
For instance, the Atmel QSPI hardware controller uses its "memory like"
area mapped ino the system to perform not only (Fast) Read operations but
actually all other types of flash commands. Hence the regular SPI API
based on the spi_xfer() function is not suited to support the Atmel QSPI
controller.
Signed-off-by: Cyrille Pitchen
Signed-off-by: Wenyou Yang
---
Changes in v3: None
Changes in v2: None
drivers/spi/spi-uclass.c | 40 +++
drivers/spi/spi.c| 13
include/spi.h| 168 +++
3 files changed, 221 insertions(+)
diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c
index e06a603ab1..5de4510363 100644
--- a/drivers/spi/spi-uclass.c
+++ b/drivers/spi/spi-uclass.c
@@ -91,6 +91,30 @@ int dm_spi_xfer(struct udevice *dev, unsigned int bitlen,
return spi_get_ops(bus)->xfer(dev, bitlen, dout, din, flags);
}
+bool dm_spi_is_flash_command_supported(struct udevice *dev,
+ const struct spi_flash_command *cmd)
+{
+ struct udevice *bus = dev->parent;
+ struct dm_spi_ops *ops = spi_get_ops(bus);
+
+ if (ops->is_flash_command_supported)
+ return ops->is_flash_command_supported(dev, cmd);
+
+ return false;
+}
+
+int dm_spi_exec_flash_command(struct udevice *dev,
+ const struct spi_flash_command *cmd)
+{
+ struct udevice *bus = dev->parent;
+ struct dm_spi_ops *ops = spi_get_ops(bus);
+
+ if (ops->exec_flash_command)
+ return ops->exec_flash_command(dev, cmd);
+
+ return -EINVAL;
+}
+
int spi_claim_bus(struct spi_slave *slave)
{
return dm_spi_claim_bus(slave->dev);
@@ -107,6 +131,18 @@ int spi_xfer(struct spi_slave *slave, unsigned int bitlen,
return dm_spi_xfer(slave->dev, bitlen, dout, din, flags);
}
+bool spi_is_flash_command_supported(struct spi_slave *slave,
+ const struct spi_flash_command *cmd)
+{
+ return dm_spi_is_flash_command_supported(slave->dev, cmd);
+}
+
+int spi_exec_flash_command(struct spi_slave *slave,
+ const struct spi_flash_command *cmd)
+{
+ return dm_spi_exec_flash_command(slave->dev, cmd);
+}
+
#if !CONFIG_IS_ENABLED(OF_PLATDATA)
static int spi_child_post_bind(struct udevice *dev)
{
@@ -144,6 +180,10 @@ static int spi_post_probe(struct udevice *bus)
ops->set_mode += gd->reloc_off;
if (ops->cs_info)
ops->cs_info += gd->reloc_off;
+ if (ops->is_flash_command_supported)
+ ops->is_flash_command_supported += gd->reloc_off;
+ if (ops->exec_flash_command)
+ ops->exec_flash_command += gd->reloc_off;
#endif
return 0;
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 7d81fbd7f8..e47acdc9e4 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -5,6 +5,7 @@
*/
#include
+#include
#inclu
