On Tue, Jul 25, 2017 at 12:30 PM, Wenyou Yang <wenyou.y...@microchip.com> wrote:
> From: Cyrille Pitchen <cyrille.pitc...@atmel.com>
>
> 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 <common.h>
> +#include <errno.h>
> #include <fdtdec.h>
> #include <malloc.h>
> #include <spi.h>
> @@ -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 <linux/string.h> /* 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_nbits))
> +
> +enum spi_flash_protocol {
> + SPI_FPROTO_1_1_1 = SPI_FPROTO(1, 1, 1),
> + SPI_FPROTO_1_1_2 = SPI_FPROTO(1, 1, 2),
> + SPI_FPROTO_1_1_4 = SPI_FPROTO(1, 1, 4),
> + SPI_FPROTO_1_2_2 = SPI_FPROTO(1, 2, 2),
> + SPI_FPROTO_1_4_4 = SPI_FPROTO(1, 4, 4),
> + SPI_FPROTO_2_2_2 = SPI_FPROTO(2, 2, 2),
> + SPI_FPROTO_4_4_4 = SPI_FPROTO(4, 4, 4),
> +};
these protocol also includes IO's like dual and quad and IO's which
are specific for SPI-NOR flash and not Generic SPI protocols.
> +
> +static inline
> +u8 spi_flash_protocol_get_inst_nbits(enum spi_flash_protocol proto)
> +{
> + return ((unsigned long)(proto & SPI_FPROTO_INST_MASK)) >>
> + SPI_FPROTO_INST_SHIFT;
> +}
> +
> +static inline
> +u8 spi_flash_protocol_get_addr_nbits(enum spi_flash_protocol proto)
> +{
> + return ((unsigned long)(proto & SPI_FPROTO_ADDR_MASK)) >>
> + SPI_FPROTO_ADDR_SHIFT;
> +}
> +
> +static inline
> +u8 spi_flash_protocol_get_data_nbits(enum spi_flash_protocol proto)
> +{
> + return ((unsigned long)(proto & SPI_FPROTO_DATA_MASK)) >>
> + SPI_FPROTO_DATA_SHIFT;
> +}
> +
> +/**
> + * struct spi_flash_command - SPI flash command structure
> + *
> + * @instr: Opcode sent to the SPI slave during instr clock
> cycles.
> + * @mode: Value sent to the SPI slave during mode clock cycles.
> + * @num_mode_cycles: Number of mode clock cycles.
> + * @num_wait_states: Number of wait-state clock cycles.
> + * @addr_len: Number of bytes sent during address clock cycles:
> + * should be 0, 3, or 4.
> + * @addr: Value sent to the SPI slave during address clock
> cycles.
> + * @data_len: Number of bytes to be sent during data clock cycles.
> + * @tx_data: Data sent to the SPI slave during data clock cycles.
> + * @rx_data: Data read from the SPI slave during data clock cycles.
> + */
> +struct spi_flash_command {
> + enum spi_flash_protocol proto;
> + u8 flags;
> +#define SPI_FCMD_TYPE GENMASK(2, 0)
> +#define SPI_FCMD_READ (0x0U << 0)
> +#define SPI_FCMD_WRITE (0x1U << 0)
> +#define SPI_FCMD_ERASE (0x2U << 0)
> +#define SPI_FCMD_READ_REG (0x3U << 0)
> +#define SPI_FCMD_WRITE_REG (0x4U << 0)
> +
> + u8 inst;
> + u8 mode;
> + u8 num_mode_cycles;
> + u8 num_wait_states;
> + u8 addr_len;
> + u32 addr;
> + size_t data_len;
> + const void *tx_data;
> + void *rx_data;
> +};
I saw from the other patches, this structure is initializing for each
I/O interface between SPI to driver/mtd like spi_transfer, if so it
shouldn't have flash attributes. Better to move flash contents to
spi_flash structure and do the respective operations.
thanks!
--
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
