mbox series

[V6,00/15] Add support for stacked/parallel memories

Message ID 20230310173217.3429788-1-amit.kumar-mahapatra@amd.com (mailing list archive)
Headers show
Series Add support for stacked/parallel memories | expand

Message

Mahapatra, Amit Kumar March 10, 2023, 5:32 p.m. UTC
This patch is in the continuation to the discussions which happened on
'commit f89504300e94 ("spi: Stacked/parallel memories bindings")' for
adding dt-binding support for stacked/parallel memories.

This patch series updated the spi-nor, spi core and the spi drivers
to add stacked and parallel memories support.

The first patch
https://lore.kernel.org/all/20230119185342.2093323-1-amit.kumar-mahapatra@amd.com/
of the previous series got applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
But the rest of the patches in the series did not get applied due to merge
conflict, so send the remaining patches in the series after rebasing it
on top of for-next branch.
---
BRANCH: for-next

Changes in v6:
- Rebased on top of latest v6.3-rc1 and fixed merge conflicts in
  spi-mpc512x-psc.c, sfdp.c, spansion.c files and removed spi-omap-100k.c.
- Updated spi_dev_check( ) to reject new devices if any one of the
  chipselect is used by another device.

Changes in v5:
- Rebased the patches on top of v6.3-rc1 and fixed the merge conflicts.
- Fixed compilation warnings in spi-sh-msiof.c with shmobile_defconfig  

Changes in v4:
- Fixed build error in spi-pl022.c file - reported by Mark.
- Fixed build error in spi-sn-f-ospi.c file.
- Added Reviewed-by: Serge Semin <fancer.lancer@gmail.com> tag.
- Added two more patches to replace spi->chip_select with API calls in
  mpc832x_rdb.c & cs35l41_hda_spi.c files.

Changes in v3:
- Rebased the patches on top of
  https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
- Added a patch to convert spi_nor_otp_region_len(nor) &
  spi_nor_otp_n_regions(nor) macros into inline functions
- Added Reviewed-by & Acked-by tags

Changes in v2:
- Rebased the patches on top of v6.2-rc1
- Created separate patch to add get & set APIs for spi->chip_select &
  spi->cs_gpiod, and replaced all spi->chip_select and spi->cs_gpiod
  references with the API calls.
- Created separate patch to add get & set APIs for nor->params.
---

Amit Kumar Mahapatra (15):
  spi: Replace all spi->chip_select and spi->cs_gpiod references with
    function call
  net: Replace all spi->chip_select and spi->cs_gpiod references with
    function call
  iio: imu: Replace all spi->chip_select and spi->cs_gpiod references
    with function call
  mtd: devices: Replace all spi->chip_select and spi->cs_gpiod
    references with function call
  staging: Replace all spi->chip_select and spi->cs_gpiod references
    with function call
  platform/x86: serial-multi-instantiate: Replace all spi->chip_select
    and spi->cs_gpiod references with function call
  powerpc/83xx/mpc832x_rdb: Replace all spi->chip_select references with
    function call
  ALSA: hda: cs35l41: Replace all spi->chip_select references with
    function call
  spi: Add stacked and parallel memories support in SPI core
  mtd: spi-nor: Convert macros with inline functions
  mtd: spi-nor: Add APIs to set/get nor->params
  mtd: spi-nor: Add stacked memories support in spi-nor
  spi: spi-zynqmp-gqspi: Add stacked memories support in GQSPI driver
  mtd: spi-nor: Add parallel memories support in spi-nor
  spi: spi-zynqmp-gqspi: Add parallel memories support in GQSPI driver

 arch/powerpc/platforms/83xx/mpc832x_rdb.c     |   2 +-
 drivers/iio/imu/adis16400.c                   |   2 +-
 drivers/mtd/devices/mtd_dataflash.c           |   2 +-
 drivers/mtd/spi-nor/atmel.c                   |  17 +-
 drivers/mtd/spi-nor/core.c                    | 665 +++++++++++++++---
 drivers/mtd/spi-nor/core.h                    |   8 +
 drivers/mtd/spi-nor/debugfs.c                 |   4 +-
 drivers/mtd/spi-nor/gigadevice.c              |   4 +-
 drivers/mtd/spi-nor/issi.c                    |  11 +-
 drivers/mtd/spi-nor/macronix.c                |   6 +-
 drivers/mtd/spi-nor/micron-st.c               |  39 +-
 drivers/mtd/spi-nor/otp.c                     |  48 +-
 drivers/mtd/spi-nor/sfdp.c                    |  29 +-
 drivers/mtd/spi-nor/spansion.c                |  50 +-
 drivers/mtd/spi-nor/sst.c                     |   7 +-
 drivers/mtd/spi-nor/swp.c                     |  22 +-
 drivers/mtd/spi-nor/winbond.c                 |  10 +-
 drivers/mtd/spi-nor/xilinx.c                  |  18 +-
 drivers/net/ethernet/adi/adin1110.c           |   2 +-
 drivers/net/ethernet/asix/ax88796c_main.c     |   2 +-
 drivers/net/ethernet/davicom/dm9051.c         |   2 +-
 drivers/net/ethernet/qualcomm/qca_debug.c     |   2 +-
 drivers/net/ieee802154/ca8210.c               |   2 +-
 drivers/net/wan/slic_ds26522.c                |   2 +-
 .../net/wireless/marvell/libertas/if_spi.c    |   2 +-
 drivers/net/wireless/silabs/wfx/bus_spi.c     |   2 +-
 drivers/net/wireless/st/cw1200/cw1200_spi.c   |   2 +-
 .../platform/x86/serial-multi-instantiate.c   |   3 +-
 drivers/spi/spi-altera-core.c                 |   2 +-
 drivers/spi/spi-amd.c                         |   4 +-
 drivers/spi/spi-ar934x.c                      |   2 +-
 drivers/spi/spi-armada-3700.c                 |   4 +-
 drivers/spi/spi-aspeed-smc.c                  |  13 +-
 drivers/spi/spi-at91-usart.c                  |   2 +-
 drivers/spi/spi-ath79.c                       |   4 +-
 drivers/spi/spi-atmel.c                       |  26 +-
 drivers/spi/spi-au1550.c                      |   4 +-
 drivers/spi/spi-axi-spi-engine.c              |   2 +-
 drivers/spi/spi-bcm-qspi.c                    |  10 +-
 drivers/spi/spi-bcm2835.c                     |  19 +-
 drivers/spi/spi-bcm2835aux.c                  |   4 +-
 drivers/spi/spi-bcm63xx-hsspi.c               |  30 +-
 drivers/spi/spi-bcm63xx.c                     |   2 +-
 drivers/spi/spi-bcmbca-hsspi.c                |  30 +-
 drivers/spi/spi-cadence-quadspi.c             |   5 +-
 drivers/spi/spi-cadence-xspi.c                |   4 +-
 drivers/spi/spi-cadence.c                     |   4 +-
 drivers/spi/spi-cavium.c                      |   8 +-
 drivers/spi/spi-coldfire-qspi.c               |   8 +-
 drivers/spi/spi-davinci.c                     |  18 +-
 drivers/spi/spi-dln2.c                        |   6 +-
 drivers/spi/spi-dw-core.c                     |   2 +-
 drivers/spi/spi-dw-mmio.c                     |   4 +-
 drivers/spi/spi-falcon.c                      |   2 +-
 drivers/spi/spi-fsi.c                         |   2 +-
 drivers/spi/spi-fsl-dspi.c                    |  16 +-
 drivers/spi/spi-fsl-espi.c                    |   6 +-
 drivers/spi/spi-fsl-lpspi.c                   |   2 +-
 drivers/spi/spi-fsl-qspi.c                    |   6 +-
 drivers/spi/spi-fsl-spi.c                     |   2 +-
 drivers/spi/spi-geni-qcom.c                   |   6 +-
 drivers/spi/spi-gpio.c                        |   4 +-
 drivers/spi/spi-gxp.c                         |   4 +-
 drivers/spi/spi-hisi-sfc-v3xx.c               |   2 +-
 drivers/spi/spi-img-spfi.c                    |  14 +-
 drivers/spi/spi-imx.c                         |  30 +-
 drivers/spi/spi-ingenic.c                     |   4 +-
 drivers/spi/spi-intel.c                       |   2 +-
 drivers/spi/spi-jcore.c                       |   4 +-
 drivers/spi/spi-lantiq-ssc.c                  |   6 +-
 drivers/spi/spi-mem.c                         |   4 +-
 drivers/spi/spi-meson-spicc.c                 |   2 +-
 drivers/spi/spi-microchip-core.c              |   6 +-
 drivers/spi/spi-mpc512x-psc.c                 |   8 +-
 drivers/spi/spi-mpc52xx.c                     |   2 +-
 drivers/spi/spi-mt65xx.c                      |   6 +-
 drivers/spi/spi-mt7621.c                      |   2 +-
 drivers/spi/spi-mux.c                         |   8 +-
 drivers/spi/spi-mxic.c                        |  10 +-
 drivers/spi/spi-mxs.c                         |   2 +-
 drivers/spi/spi-npcm-fiu.c                    |  20 +-
 drivers/spi/spi-nxp-fspi.c                    |  10 +-
 drivers/spi/spi-omap-uwire.c                  |   8 +-
 drivers/spi/spi-omap2-mcspi.c                 |  24 +-
 drivers/spi/spi-orion.c                       |   4 +-
 drivers/spi/spi-pci1xxxx.c                    |   4 +-
 drivers/spi/spi-pic32-sqi.c                   |   2 +-
 drivers/spi/spi-pic32.c                       |   4 +-
 drivers/spi/spi-pl022.c                       |   4 +-
 drivers/spi/spi-pxa2xx.c                      |   6 +-
 drivers/spi/spi-qcom-qspi.c                   |   2 +-
 drivers/spi/spi-rb4xx.c                       |   2 +-
 drivers/spi/spi-rockchip-sfc.c                |   2 +-
 drivers/spi/spi-rockchip.c                    |  26 +-
 drivers/spi/spi-rspi.c                        |  10 +-
 drivers/spi/spi-s3c64xx.c                     |   2 +-
 drivers/spi/spi-sc18is602.c                   |   4 +-
 drivers/spi/spi-sh-msiof.c                    |   6 +-
 drivers/spi/spi-sh-sci.c                      |   2 +-
 drivers/spi/spi-sifive.c                      |   6 +-
 drivers/spi/spi-sn-f-ospi.c                   |   2 +-
 drivers/spi/spi-st-ssc4.c                     |   2 +-
 drivers/spi/spi-stm32-qspi.c                  |  12 +-
 drivers/spi/spi-sun4i.c                       |   2 +-
 drivers/spi/spi-sun6i.c                       |   2 +-
 drivers/spi/spi-synquacer.c                   |   6 +-
 drivers/spi/spi-tegra114.c                    |  28 +-
 drivers/spi/spi-tegra20-sflash.c              |   2 +-
 drivers/spi/spi-tegra20-slink.c               |   6 +-
 drivers/spi/spi-tegra210-quad.c               |   8 +-
 drivers/spi/spi-ti-qspi.c                     |  16 +-
 drivers/spi/spi-topcliff-pch.c                |   4 +-
 drivers/spi/spi-wpcm-fiu.c                    |  12 +-
 drivers/spi/spi-xcomm.c                       |   2 +-
 drivers/spi/spi-xilinx.c                      |   6 +-
 drivers/spi/spi-xlp.c                         |   4 +-
 drivers/spi/spi-zynq-qspi.c                   |   2 +-
 drivers/spi/spi-zynqmp-gqspi.c                |  58 +-
 drivers/spi/spi.c                             | 225 ++++--
 drivers/spi/spidev.c                          |   6 +-
 drivers/staging/fbtft/fbtft-core.c            |   2 +-
 drivers/staging/greybus/spilib.c              |   2 +-
 include/linux/mtd/spi-nor.h                   |  18 +-
 include/linux/spi/spi.h                       |  34 +-
 include/trace/events/spi.h                    |  10 +-
 sound/pci/hda/cs35l41_hda_spi.c               |   2 +-
 126 files changed, 1350 insertions(+), 615 deletions(-)

Comments

Mark Brown March 11, 2023, 9:16 p.m. UTC | #1
On Fri, 10 Mar 2023 23:02:02 +0530, Amit Kumar Mahapatra wrote:
> This patch is in the continuation to the discussions which happened on
> 'commit f89504300e94 ("spi: Stacked/parallel memories bindings")' for
> adding dt-binding support for stacked/parallel memories.
> 
> This patch series updated the spi-nor, spi core and the spi drivers
> to add stacked and parallel memories support.
> 
> [...]

Applied to

   broonie/spi.git for-next

Thanks!

[01/15] spi: Replace all spi->chip_select and spi->cs_gpiod references with function call
        commit: 9e264f3f85a56cc109cc2d6010a48aa89d5c1ff1
[02/15] net: Replace all spi->chip_select and spi->cs_gpiod references with function call
        commit: 25fd0550d9b9c92288a17fb7d605cdcdb4a65a64
[03/15] iio: imu: Replace all spi->chip_select and spi->cs_gpiod references with function call
        commit: 0183f81fce154ae1d4df2bb28d22ad6612317148
[04/15] mtd: devices: Replace all spi->chip_select and spi->cs_gpiod references with function call
        commit: 0817bcef53e4e3df23c023eddaa2b35b7288400e
[05/15] staging: Replace all spi->chip_select and spi->cs_gpiod references with function call
        commit: caa9d3475b1c5566f0272273c147cc9b72f2be28
[06/15] platform/x86: serial-multi-instantiate: Replace all spi->chip_select and spi->cs_gpiod references with function call
        commit: e20451f44ca33ec40422e9868775e117ef2da935
[07/15] powerpc/83xx/mpc832x_rdb: Replace all spi->chip_select references with function call
        commit: 3aba06a9fee04f6fefa9df71d3ee27dd4c464ad5
[08/15] ALSA: hda: cs35l41: Replace all spi->chip_select references with function call
        commit: 06b5e53c8b2b016e06a53ab6f01006ca7bbfa5df

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark
Jonas Gorski March 12, 2023, 3:59 p.m. UTC | #2
Hi,

On Fri, 10 Mar 2023 at 18:37, Amit Kumar Mahapatra
<amit.kumar-mahapatra@amd.com> wrote:
>
> For supporting multiple CS the SPI device need to be aware of all the CS
> values. So, the "chip_select" member in the spi_device structure is now an
> array that holds all the CS values.
>
> spi_device structure now has a "cs_index_mask" member. This acts as an
> index to the chip_select array. If nth bit of spi->cs_index_mask is set
> then the driver would assert spi->chip_select[n].
>
> In parallel mode all the chip selects are asserted/de-asserted
> simultaneously and each byte of data is stored in both devices, the even
> bits in one, the odd bits in the other. The split is automatically handled
> by the GQSPI controller. The GQSPI controller supports a maximum of two
> flashes connected in parallel mode. A "multi-cs-cap" flag is added in the
> spi controntroller data, through ctlr->multi-cs-cap the spi core will make
> sure that the controller is capable of handling multiple chip selects at
> once.
>
> For supporting multiple CS via GPIO the cs_gpiod member of the spi_device
> structure is now an array that holds the gpio descriptor for each
> chipselect.
>
> Multi CS support using GPIO is not tested due to unavailability of
> necessary hardware setup.

Can you pinmux your SPI controller's (cs) pins as GPIO? If so, you
should be able use that for testing.

>
> Signed-off-by: Amit Kumar Mahapatra <amit.kumar-mahapatra@amd.com>
> ---
>  drivers/spi/spi.c       | 225 +++++++++++++++++++++++++++-------------
>  include/linux/spi/spi.h |  34 ++++--
>  2 files changed, 182 insertions(+), 77 deletions(-)
>
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index c725b4bab7af..742bd688381c 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -612,10 +612,17 @@ static int spi_dev_check(struct device *dev, void *data)
>  {
>         struct spi_device *spi = to_spi_device(dev);
>         struct spi_device *new_spi = data;
> -
> -       if (spi->controller == new_spi->controller &&
> -           spi_get_chipselect(spi, 0) == spi_get_chipselect(new_spi, 0))
> -               return -EBUSY;
> +       int idx, nw_idx;
> +
> +       if (spi->controller == new_spi->controller) {
> +               for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) {
> +                       for (nw_idx = 0; nw_idx < SPI_CS_CNT_MAX; nw_idx++) {
> +                               if (spi_get_chipselect(spi, idx) ==
> +                                   spi_get_chipselect(new_spi, nw_idx))
> +                                       return -EBUSY;
> +                       }
> +               }

AFAICT unused chip selects are initialized to 0, so all single chip
select devices would have it as their second one. This will then cause
this check to reject every single chip select device after the first
one. So you first need to make sure to only compare valid chip
selects.

So the loop condition should be something along idx <
spi_get_num_chipselect(), not idx < SPI_CS_CNT_MAX.

> +       }
>         return 0;
>  }
>
> @@ -629,7 +636,7 @@ static int __spi_add_device(struct spi_device *spi)
>  {
>         struct spi_controller *ctlr = spi->controller;
>         struct device *dev = ctlr->dev.parent;
> -       int status;
> +       int status, idx;
>
>         /*
>          * We need to make sure there's no other device with this
> @@ -638,8 +645,7 @@ static int __spi_add_device(struct spi_device *spi)
>          */
>         status = bus_for_each_dev(&spi_bus_type, NULL, spi, spi_dev_check);
>         if (status) {
> -               dev_err(dev, "chipselect %d already in use\n",
> -                               spi_get_chipselect(spi, 0));
> +               dev_err(dev, "chipselect %d already in use\n", spi_get_chipselect(spi, 0));

The message might be misleading for multi cs devices where the first
one is free, but the second one is already in use.

So maybe move this error message into spi_dev_check(), where you have
that information available. You then even have the chance to state
what is using the CS then, but that might be something for a different
patch.


>                 return status;
>         }
>
> @@ -649,8 +655,10 @@ static int __spi_add_device(struct spi_device *spi)
>                 return -ENODEV;
>         }
>
> -       if (ctlr->cs_gpiods)
> -               spi_set_csgpiod(spi, 0, ctlr->cs_gpiods[spi_get_chipselect(spi, 0)]);
> +       if (ctlr->cs_gpiods) {
> +               for (idx = 0; idx < SPI_CS_CNT_MAX; idx++)
> +                       spi_set_csgpiod(spi, idx, ctlr->cs_gpiods[spi_get_chipselect(spi, idx)]);
> +       }
>
>         /*
>          * Drivers may modify this initial i/o setup, but will
> @@ -690,13 +698,15 @@ int spi_add_device(struct spi_device *spi)
>  {
>         struct spi_controller *ctlr = spi->controller;
>         struct device *dev = ctlr->dev.parent;
> -       int status;
> +       int status, idx;
>
> -       /* Chipselects are numbered 0..max; validate. */
> -       if (spi_get_chipselect(spi, 0) >= ctlr->num_chipselect) {
> -               dev_err(dev, "cs%d >= max %d\n", spi_get_chipselect(spi, 0),
> -                       ctlr->num_chipselect);
> -               return -EINVAL;
> +       for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) {
> +               /* Chipselects are numbered 0..max; validate. */
> +               if (spi_get_chipselect(spi, idx) >= ctlr->num_chipselect) {
> +                       dev_err(dev, "cs%d >= max %d\n", spi_get_chipselect(spi, idx),
> +                               ctlr->num_chipselect);
> +                       return -EINVAL;
> +               }
>         }
>
>         /* Set the bus ID string */
> @@ -713,12 +723,15 @@ static int spi_add_device_locked(struct spi_device *spi)
>  {
>         struct spi_controller *ctlr = spi->controller;
>         struct device *dev = ctlr->dev.parent;
> +       int idx;
>
> -       /* Chipselects are numbered 0..max; validate. */
> -       if (spi_get_chipselect(spi, 0) >= ctlr->num_chipselect) {
> -               dev_err(dev, "cs%d >= max %d\n", spi_get_chipselect(spi, 0),
> -                       ctlr->num_chipselect);
> -               return -EINVAL;
> +       for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) {
> +               /* Chipselects are numbered 0..max; validate. */
> +               if (spi_get_chipselect(spi, idx) >= ctlr->num_chipselect) {
> +                       dev_err(dev, "cs%d >= max %d\n", spi_get_chipselect(spi, idx),
> +                               ctlr->num_chipselect);
> +                       return -EINVAL;
> +               }
>         }
>
>         /* Set the bus ID string */
> @@ -966,58 +979,119 @@ static void spi_res_release(struct spi_controller *ctlr, struct spi_message *mes
>  static void spi_set_cs(struct spi_device *spi, bool enable, bool force)
>  {
>         bool activate = enable;
> +       u32 cs_num = __ffs(spi->cs_index_mask);
> +       int idx;
>
>         /*
> -        * Avoid calling into the driver (or doing delays) if the chip select
> -        * isn't actually changing from the last time this was called.
> +        * In parallel mode all the chip selects are asserted/de-asserted
> +        * at once
>          */
> -       if (!force && ((enable && spi->controller->last_cs == spi_get_chipselect(spi, 0)) ||
> -                      (!enable && spi->controller->last_cs != spi_get_chipselect(spi, 0))) &&
> -           (spi->controller->last_cs_mode_high == (spi->mode & SPI_CS_HIGH)))
> -               return;
> -
> -       trace_spi_set_cs(spi, activate);
> -
> -       spi->controller->last_cs = enable ? spi_get_chipselect(spi, 0) : -1;
> -       spi->controller->last_cs_mode_high = spi->mode & SPI_CS_HIGH;
> -
> -       if ((spi_get_csgpiod(spi, 0) || !spi->controller->set_cs_timing) && !activate)
> -               spi_delay_exec(&spi->cs_hold, NULL);
> -
> -       if (spi->mode & SPI_CS_HIGH)
> -               enable = !enable;
> +       if ((spi->cs_index_mask & SPI_PARALLEL_CS_MASK) == SPI_PARALLEL_CS_MASK) {
> +               spi->controller->last_cs_mode_high = spi->mode & SPI_CS_HIGH;
> +
> +               if ((spi_get_csgpiod(spi, 0) || !spi->controller->set_cs_timing) && !activate)
> +                       spi_delay_exec(&spi->cs_hold, NULL);
> +
> +               if (spi->mode & SPI_CS_HIGH)
> +                       enable = !enable;
> +
> +               if (spi_get_csgpiod(spi, 0) && spi_get_csgpiod(spi, 1)) {
> +                       if (!(spi->mode & SPI_NO_CS)) {
> +                               /*
> +                                * Historically ACPI has no means of the GPIO polarity and
> +                                * thus the SPISerialBus() resource defines it on the per-chip
> +                                * basis. In order to avoid a chain of negations, the GPIO
> +                                * polarity is considered being Active High. Even for the cases
> +                                * when _DSD() is involved (in the updated versions of ACPI)
> +                                * the GPIO CS polarity must be defined Active High to avoid
> +                                * ambiguity. That's why we use enable, that takes SPI_CS_HIGH
> +                                * into account.
> +                                */
> +                               if (has_acpi_companion(&spi->dev)) {
> +                                       for (idx = 0; idx < SPI_CS_CNT_MAX; idx++)
> +                                               gpiod_set_value_cansleep(spi_get_csgpiod(spi, idx),
> +                                                                        !enable);
> +                               } else {
> +                                       for (idx = 0; idx < SPI_CS_CNT_MAX; idx++)
> +                                               /* Polarity handled by GPIO library */
> +                                               gpiod_set_value_cansleep(spi_get_csgpiod(spi, idx),
> +                                                                        activate);
> +                               }
> +                       }
> +                       /* Some SPI masters need both GPIO CS & slave_select */
> +                       if ((spi->controller->flags & SPI_MASTER_GPIO_SS) &&
> +                           spi->controller->set_cs)
> +                               spi->controller->set_cs(spi, !enable);

> +                       else if (spi->controller->set_cs)
> +                               spi->controller->set_cs(spi, !enable);

this else if belongs to the following brace as the else of the if
(spi_get_csgpiod(spi, 0) && spi_get_csgpiod(spi, 1). Currently it
would make the first check redundant, as the second case would always
be true if the first one is.

Actually shouldn't you iterate over the cs's here in case one is using
set_cs() and the other one is gpiod? You can only get here if both are
backed by gpiods. And you would only set the first cs, but not the
second one. The ->set_cs() callback doesn't allow specifying which of
the (multiple) cs's should be set though.

> +               }
>
> -       if (spi_get_csgpiod(spi, 0)) {
> -               if (!(spi->mode & SPI_NO_CS)) {
> -                       /*
> -                        * Historically ACPI has no means of the GPIO polarity and
> -                        * thus the SPISerialBus() resource defines it on the per-chip
> -                        * basis. In order to avoid a chain of negations, the GPIO
> -                        * polarity is considered being Active High. Even for the cases
> -                        * when _DSD() is involved (in the updated versions of ACPI)
> -                        * the GPIO CS polarity must be defined Active High to avoid
> -                        * ambiguity. That's why we use enable, that takes SPI_CS_HIGH
> -                        * into account.
> -                        */
> -                       if (has_acpi_companion(&spi->dev))
> -                               gpiod_set_value_cansleep(spi_get_csgpiod(spi, 0), !enable);
> -                       else
> -                               /* Polarity handled by GPIO library */
> -                               gpiod_set_value_cansleep(spi_get_csgpiod(spi, 0), activate);
> +               for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) {
> +                       if (spi_get_csgpiod(spi, idx) || !spi->controller->set_cs_timing) {
> +                               if (activate)
> +                                       spi_delay_exec(&spi->cs_setup, NULL);
> +                               else
> +                                       spi_delay_exec(&spi->cs_inactive, NULL);
> +                       }

Won't you delay twice if both CS's are backed by gpiod (and the
controller does not implement set_cs_timing)? You should probably
break after the first or so.

I wonder if it would makes sense to have a helper function to set cs
state to all cs's indicated by cs_index_mask so you can share most of
the logic between the single and multi cs paths.

Currently it seems both paths have a lot of code (and comment)
duplication, with the difference being one path is touching one cs and
the other two (or all).

>                 }
> -               /* Some SPI masters need both GPIO CS & slave_select */
> -               if ((spi->controller->flags & SPI_MASTER_GPIO_SS) &&
> -                   spi->controller->set_cs)
> +       } else {
> +               /*
> +                * Avoid calling into the driver (or doing delays) if the chip select
> +                * isn't actually changing from the last time this was called.
> +                */
> +               if (!force && ((enable && spi->controller->last_cs ==
> +                               spi_get_chipselect(spi, cs_num)) ||
> +                               (!enable && spi->controller->last_cs !=
> +                                spi_get_chipselect(spi, cs_num))) &&
> +                   (spi->controller->last_cs_mode_high ==
> +                    (spi->mode & SPI_CS_HIGH)))
> +                       return;
> +
> +               trace_spi_set_cs(spi, activate);
> +
> +               spi->controller->last_cs = enable ? spi_get_chipselect(spi, cs_num) : -1;
> +               spi->controller->last_cs_mode_high = spi->mode & SPI_CS_HIGH;
> +
> +               if ((spi_get_csgpiod(spi, cs_num) || !spi->controller->set_cs_timing) && !activate)
> +                       spi_delay_exec(&spi->cs_hold, NULL);
> +
> +               if (spi->mode & SPI_CS_HIGH)
> +                       enable = !enable;
> +
> +               if (spi_get_csgpiod(spi, cs_num)) {
> +                       if (!(spi->mode & SPI_NO_CS)) {
> +                               /*
> +                                * Historically ACPI has no means of the GPIO polarity and
> +                                * thus the SPISerialBus() resource defines it on the per-chip
> +                                * basis. In order to avoid a chain of negations, the GPIO
> +                                * polarity is considered being Active High. Even for the cases
> +                                * when _DSD() is involved (in the updated versions of ACPI)
> +                                * the GPIO CS polarity must be defined Active High to avoid
> +                                * ambiguity. That's why we use enable, that takes SPI_CS_HIGH
> +                                * into account.
> +                                */
> +                               if (has_acpi_companion(&spi->dev))
> +                                       gpiod_set_value_cansleep(spi_get_csgpiod(spi, cs_num),
> +                                                                !enable);
> +                               else
> +                                       /* Polarity handled by GPIO library */
> +                                       gpiod_set_value_cansleep(spi_get_csgpiod(spi, cs_num),
> +                                                                activate);
> +                       }
> +                       /* Some SPI masters need both GPIO CS & slave_select */
> +                       if ((spi->controller->flags & SPI_MASTER_GPIO_SS) &&
> +                           spi->controller->set_cs)
> +                               spi->controller->set_cs(spi, !enable);
> +               } else if (spi->controller->set_cs) {
>                         spi->controller->set_cs(spi, !enable);
> -       } else if (spi->controller->set_cs) {
> -               spi->controller->set_cs(spi, !enable);
> -       }
> +               }
>
> -       if (spi_get_csgpiod(spi, 0) || !spi->controller->set_cs_timing) {
> -               if (activate)
> -                       spi_delay_exec(&spi->cs_setup, NULL);
> -               else
> -                       spi_delay_exec(&spi->cs_inactive, NULL);
> +               if (spi_get_csgpiod(spi, cs_num) || !spi->controller->set_cs_timing) {
> +                       if (activate)
> +                               spi_delay_exec(&spi->cs_setup, NULL);
> +                       else
> +                               spi_delay_exec(&spi->cs_inactive, NULL);
> +               }
>         }
>  }
>
> @@ -2246,8 +2320,8 @@ static void of_spi_parse_dt_cs_delay(struct device_node *nc,
>  static int of_spi_parse_dt(struct spi_controller *ctlr, struct spi_device *spi,
>                            struct device_node *nc)
>  {
> -       u32 value;
> -       int rc;
> +       u32 value, cs[SPI_CS_CNT_MAX] = {0};
> +       int rc, idx;
>
>         /* Mode (clock phase/polarity/etc.) */
>         if (of_property_read_bool(nc, "spi-cpha"))
> @@ -2320,13 +2394,21 @@ static int of_spi_parse_dt(struct spi_controller *ctlr, struct spi_device *spi,
>         }
>
>         /* Device address */
> -       rc = of_property_read_u32(nc, "reg", &value);
> -       if (rc) {
> +       rc = of_property_read_variable_u32_array(nc, "reg", &cs[0], 1,
> +                                                SPI_CS_CNT_MAX);
> +       if (rc < 0 || rc > ctlr->num_chipselect) {
>                 dev_err(&ctlr->dev, "%pOF has no valid 'reg' property (%d)\n",
>                         nc, rc);
>                 return rc;
> +       } else if ((of_property_read_bool(nc, "parallel-memories")) &&
> +                  (!ctlr->multi_cs_cap)) {
> +               dev_err(&ctlr->dev, "SPI controller doesn't support multi CS\n");
> +               return -EINVAL;
>         }
> -       spi_set_chipselect(spi, 0, value);
> +       for (idx = 0; idx < rc; idx++)
> +               spi_set_chipselect(spi, idx, cs[idx]);
> +       /* By default set the spi->cs_index_mask as 1 */
> +       spi->cs_index_mask = 0x01;
>
>         /* Device speed */
>         if (!of_property_read_u32(nc, "spi-max-frequency", &value))
> @@ -3846,6 +3928,7 @@ static int __spi_validate(struct spi_device *spi, struct spi_message *message)
>         struct spi_controller *ctlr = spi->controller;
>         struct spi_transfer *xfer;
>         int w_size;
> +       u32 cs_num = __ffs(spi->cs_index_mask);
>
>         if (list_empty(&message->transfers))
>                 return -EINVAL;
> @@ -3858,7 +3941,7 @@ static int __spi_validate(struct spi_device *spi, struct spi_message *message)
>          * cs_change is set for each transfer.
>          */
>         if ((spi->mode & SPI_CS_WORD) && (!(ctlr->mode_bits & SPI_CS_WORD) ||
> -                                         spi_get_csgpiod(spi, 0))) {
> +                                         spi_get_csgpiod(spi, cs_num))) {

Wouldn't you need to check for any of the cs_index_mask enabled CS's,
and not just the first one? AFAICT you would currently fail to catch a
SPI_CS_WORD transfer with both cs enabled where the first one is a
SPI_CS_WORD capable native CS and the second one a gpiod.

>                 size_t maxsize;
>                 int ret;
>
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index bdb35a91b4bf..452682aa1a39 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -19,6 +19,11 @@
>  #include <linux/acpi.h>
>  #include <linux/u64_stats_sync.h>
>
> +/* Max no. of CS supported per spi device */
> +#define SPI_CS_CNT_MAX 2
> +
> +/* chip select mask */
> +#define SPI_PARALLEL_CS_MASK   (BIT(0) | BIT(1))
>  struct dma_chan;
>  struct software_node;
>  struct ptp_system_timestamp;
> @@ -166,6 +171,7 @@ extern void spi_transfer_cs_change_delay_exec(struct spi_message *msg,
>   *     deasserted. If @cs_change_delay is used from @spi_transfer, then the
>   *     two delays will be added up.
>   * @pcpu_statistics: statistics for the spi_device
> + * @cs_index_mask: Bit mask of the active chipselect(s) in the chipselect array
>   *
>   * A @spi_device is used to interchange data between an SPI slave
>   * (usually a discrete chip) and CPU memory.
> @@ -181,7 +187,7 @@ struct spi_device {
>         struct spi_controller   *controller;
>         struct spi_controller   *master;        /* Compatibility layer */
>         u32                     max_speed_hz;
> -       u8                      chip_select;
> +       u8                      chip_select[SPI_CS_CNT_MAX];
>         u8                      bits_per_word;
>         bool                    rt;
>  #define SPI_NO_TX      BIT(31)         /* No transmit wire */
> @@ -202,7 +208,7 @@ struct spi_device {
>         void                    *controller_data;
>         char                    modalias[SPI_NAME_SIZE];
>         const char              *driver_override;
> -       struct gpio_desc        *cs_gpiod;      /* Chip select gpio desc */
> +       struct gpio_desc        *cs_gpiod[SPI_CS_CNT_MAX];      /* Chip select gpio desc */
>         struct spi_delay        word_delay; /* Inter-word delay */
>         /* CS delays */
>         struct spi_delay        cs_setup;
> @@ -212,6 +218,13 @@ struct spi_device {
>         /* The statistics */
>         struct spi_statistics __percpu  *pcpu_statistics;
>
> +       /* Bit mask of the chipselect(s) that the driver need to use from
> +        * the chipselect array.When the controller is capable to handle
> +        * multiple chip selects & memories are connected in parallel
> +        * then more than one bit need to be set in cs_index_mask.
> +        */
> +       u32                     cs_index_mask : 2;

SPI_CS_CNT_MAX?

> +
>         /*
>          * likely need more hooks for more protocol options affecting how
>          * the controller talks to each chip, like:
> @@ -268,22 +281,22 @@ static inline void *spi_get_drvdata(struct spi_device *spi)
>
>  static inline u8 spi_get_chipselect(struct spi_device *spi, u8 idx)
>  {
> -       return spi->chip_select;
> +       return spi->chip_select[idx];
>  }
>
>  static inline void spi_set_chipselect(struct spi_device *spi, u8 idx, u8 chipselect)
>  {
> -       spi->chip_select = chipselect;
> +       spi->chip_select[idx] = chipselect;
>  }
>
>  static inline struct gpio_desc *spi_get_csgpiod(struct spi_device *spi, u8 idx)
>  {
> -       return spi->cs_gpiod;
> +       return spi->cs_gpiod[idx];
>  }
>
>  static inline void spi_set_csgpiod(struct spi_device *spi, u8 idx, struct gpio_desc *csgpiod)
>  {
> -       spi->cs_gpiod = csgpiod;
> +       spi->cs_gpiod[idx] = csgpiod;
>  }
>
>  /**
> @@ -388,6 +401,8 @@ extern struct spi_device *spi_new_ancillary_device(struct spi_device *spi, u8 ch
>   * @bus_lock_spinlock: spinlock for SPI bus locking
>   * @bus_lock_mutex: mutex for exclusion of multiple callers
>   * @bus_lock_flag: indicates that the SPI bus is locked for exclusive use
> + * @multi_cs_cap: indicates that the SPI Controller can assert/de-assert
> + *     more than one chip select at once.
>   * @setup: updates the device mode and clocking records used by a
>   *     device's SPI controller; protocol code may call this.  This
>   *     must fail if an unrecognized or unsupported mode is requested.
> @@ -585,6 +600,13 @@ struct spi_controller {
>         /* Flag indicating that the SPI bus is locked for exclusive use */
>         bool                    bus_lock_flag;
>
> +       /*
> +        * Flag indicating that the spi-controller has multi chip select
> +        * capability and can assert/de-assert more than one chip select
> +        * at once.
> +        */
> +       bool                    multi_cs_cap;

I admit I haven't followed the first iterations, but Is there a reason
this isn't a SPI_XXX flag in spi.h? There seem to be quite a few free
bits left.

I would think multi_cs can be emulated (somewhat) via gpiod for the
second CS as long as the controller supports set_cs() (and
SPI_NO_CS?).

> +
>         /* Setup mode and clock, etc (spi driver may call many times).
>          *
>          * IMPORTANT:  this may be called when transfers to another
> --
> 2.25.1
>

Regards
Jonas
Stefan Binding March 13, 2023, 5:15 p.m. UTC | #3
Hi,

I tested this patch on an existing laptop which uses SPI with a GPIO
Chipselect,
(HP EliteBook 860 G9), and I get the error:

[    2.671655] pxa2xx-spi pxa2xx-spi.2: chipselect 1 already in use
[    2.671711] spi_master spi0: error -EBUSY: failed to add SPI device
CSC3551:00 from ACPI
[    2.690903] Serial bus multi instantiate pseudo device driver:
probe of CSC3551:00 failed with error -16

Please don't merge this until we have fully investigated.

Thanks,
Stefan Binding

> -----Original Message-----
> From: Amit Kumar Mahapatra <amit.kumar-mahapatra@amd.com>
> Sent: 10 March 2023 17:32
> To: broonie@kernel.org; miquel.raynal@bootlin.com; richard@nod.at;
> vigneshr@ti.com; jic23@kernel.org; tudor.ambarus@microchip.com;
> pratyush@kernel.org; Sanju.Mehta@amd.com; chin-
> ting_kuo@aspeedtech.com; clg@kaod.org; kdasu.kdev@gmail.com;
> f.fainelli@gmail.com; rjui@broadcom.com; sbranden@broadcom.com;
> eajames@linux.ibm.com; olteanv@gmail.com; han.xu@nxp.com;
> john.garry@huawei.com; shawnguo@kernel.org;
> s.hauer@pengutronix.de; narmstrong@baylibre.com;
> khilman@baylibre.com; matthias.bgg@gmail.com;
> haibo.chen@nxp.com; linus.walleij@linaro.org; daniel@zonque.org;
> haojian.zhuang@gmail.com; robert.jarzmik@free.fr;
> agross@kernel.org; bjorn.andersson@linaro.org; heiko@sntech.de;
> krzysztof.kozlowski@linaro.org; andi@etezian.org;
> mcoquelin.stm32@gmail.com; alexandre.torgue@foss.st.com;
> wens@csie.org; jernej.skrabec@gmail.com; samuel@sholland.org;
> masahisa.kojima@linaro.org; jaswinder.singh@linaro.org;
> rostedt@goodmis.org; mingo@redhat.com;
> l.stelmach@samsung.com; davem@davemloft.net;
> edumazet@google.com; kuba@kernel.org; pabeni@redhat.com;
> alex.aring@gmail.com; stefan@datenfreihafen.org; kvalo@kernel.org;
> james.schulman@cirrus.com; david.rhodes@cirrus.com;
> tanureal@opensource.cirrus.com; rf@opensource.cirrus.com;
> perex@perex.cz; tiwai@suse.com; npiggin@gmail.com;
> christophe.leroy@csgroup.eu; mpe@ellerman.id.au;
> oss@buserror.net; windhl@126.com; yangyingliang@huawei.com;
> william.zhang@broadcom.com; kursad.oney@broadcom.com;
> jonas.gorski@gmail.com; anand.gore@broadcom.com;
> rafal@milecki.pl
> Cc: git@amd.com; linux-spi@vger.kernel.org; linux-
> kernel@vger.kernel.org; joel@jms.id.au; andrew@aj.id.au;
> radu_nicolae.pirea@upb.ro; nicolas.ferre@microchip.com;
> alexandre.belloni@bootlin.com; claudiu.beznea@microchip.com; bcm-
> kernel-feedback-list@broadcom.com; fancer.lancer@gmail.com;
> kernel@pengutronix.de; festevam@gmail.com; linux-imx@nxp.com;
> jbrunet@baylibre.com; martin.blumenstingl@googlemail.com;
> avifishman70@gmail.com; tmaimon77@gmail.com;
> tali.perry1@gmail.com; venture@google.com; yuenn@google.com;
> benjaminfair@google.com; yogeshgaur.83@gmail.com;
> konrad.dybcio@somainline.org; alim.akhtar@samsung.com;
> ldewangan@nvidia.com; thierry.reding@gmail.com;
> jonathanh@nvidia.com; michal.simek@amd.com; linux-
> aspeed@lists.ozlabs.org; openbmc@lists.ozlabs.org; linux-arm-
> kernel@lists.infradead.org; linux-rpi-kernel@lists.infradead.org;
linux-
> amlogic@lists.infradead.org; linux-mediatek@lists.infradead.org;
linux-
> arm-msm@vger.kernel.org; linux-rockchip@lists.infradead.org; linux-
> samsung-soc@vger.kernel.org; linux-stm32@st-md-
> mailman.stormreply.com; linux-sunxi@lists.linux.dev; linux-
> tegra@vger.kernel.org; netdev@vger.kernel.org; linux-
> wpan@vger.kernel.org; libertas-dev@lists.infradead.org; linux-
> wireless@vger.kernel.org; linux-mtd@lists.infradead.org;
> lars@metafoo.de; Michael.Hennerich@analog.com; linux-
> iio@vger.kernel.org; michael@walle.cc; palmer@dabbelt.com; linux-
> riscv@lists.infradead.org; alsa-devel@alsa-project.org;
> patches@opensource.cirrus.com; linuxppc-dev@lists.ozlabs.org;
> amitrkcian2002@gmail.com; amit.kumar-mahapatra@amd.com
> Subject: [PATCH V6 09/15] spi: Add stacked and parallel memories
> support in SPI core
> 
> For supporting multiple CS the SPI device need to be aware of all
the
> CS
> values. So, the "chip_select" member in the spi_device structure is
> now an
> array that holds all the CS values.
> 
> spi_device structure now has a "cs_index_mask" member. This acts as
> an
> index to the chip_select array. If nth bit of spi->cs_index_mask is
set
> then the driver would assert spi->chip_select[n].
> 
> In parallel mode all the chip selects are asserted/de-asserted
> simultaneously and each byte of data is stored in both devices, the
> even
> bits in one, the odd bits in the other. The split is automatically
handled
> by the GQSPI controller. The GQSPI controller supports a maximum of
> two
> flashes connected in parallel mode. A "multi-cs-cap" flag is added
in
> the
> spi controntroller data, through ctlr->multi-cs-cap the spi core
will
> make
> sure that the controller is capable of handling multiple chip
selects at
> once.
> 
> For supporting multiple CS via GPIO the cs_gpiod member of the
> spi_device
> structure is now an array that holds the gpio descriptor for each
> chipselect.
> 
> Multi CS support using GPIO is not tested due to unavailability of
> necessary hardware setup.
> 
> Signed-off-by: Amit Kumar Mahapatra <amit.kumar-
> mahapatra@amd.com>
> ---
>  drivers/spi/spi.c       | 225
+++++++++++++++++++++++++++------------
> -
>  include/linux/spi/spi.h |  34 ++++--
>  2 files changed, 182 insertions(+), 77 deletions(-)
> 
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index c725b4bab7af..742bd688381c 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -612,10 +612,17 @@ static int spi_dev_check(struct device *dev,
> void *data)
>  {
>  	struct spi_device *spi = to_spi_device(dev);
>  	struct spi_device *new_spi = data;
> -
> -	if (spi->controller == new_spi->controller &&
> -	    spi_get_chipselect(spi, 0) == spi_get_chipselect(new_spi,
0))
> -		return -EBUSY;
> +	int idx, nw_idx;
> +
> +	if (spi->controller == new_spi->controller) {
> +		for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) {
> +			for (nw_idx = 0; nw_idx < SPI_CS_CNT_MAX;
> nw_idx++) {
> +				if (spi_get_chipselect(spi, idx) ==
> +				    spi_get_chipselect(new_spi,
> nw_idx))
> +					return -EBUSY;
> +			}
> +		}
> +	}
>  	return 0;
>  }
> 
> @@ -629,7 +636,7 @@ static int __spi_add_device(struct spi_device
> *spi)
>  {
>  	struct spi_controller *ctlr = spi->controller;
>  	struct device *dev = ctlr->dev.parent;
> -	int status;
> +	int status, idx;
> 
>  	/*
>  	 * We need to make sure there's no other device with this
> @@ -638,8 +645,7 @@ static int __spi_add_device(struct spi_device
> *spi)
>  	 */
>  	status = bus_for_each_dev(&spi_bus_type, NULL, spi,
> spi_dev_check);
>  	if (status) {
> -		dev_err(dev, "chipselect %d already in use\n",
> -				spi_get_chipselect(spi, 0));
> +		dev_err(dev, "chipselect %d already in use\n",
> spi_get_chipselect(spi, 0));
>  		return status;
>  	}
> 
> @@ -649,8 +655,10 @@ static int __spi_add_device(struct spi_device
> *spi)
>  		return -ENODEV;
>  	}
> 
> -	if (ctlr->cs_gpiods)
> -		spi_set_csgpiod(spi, 0, ctlr-
> >cs_gpiods[spi_get_chipselect(spi, 0)]);
> +	if (ctlr->cs_gpiods) {
> +		for (idx = 0; idx < SPI_CS_CNT_MAX; idx++)
> +			spi_set_csgpiod(spi, idx, ctlr-
> >cs_gpiods[spi_get_chipselect(spi, idx)]);
> +	}
> 
>  	/*
>  	 * Drivers may modify this initial i/o setup, but will
> @@ -690,13 +698,15 @@ int spi_add_device(struct spi_device *spi)
>  {
>  	struct spi_controller *ctlr = spi->controller;
>  	struct device *dev = ctlr->dev.parent;
> -	int status;
> +	int status, idx;
> 
> -	/* Chipselects are numbered 0..max; validate. */
> -	if (spi_get_chipselect(spi, 0) >= ctlr->num_chipselect) {
> -		dev_err(dev, "cs%d >= max %d\n",
> spi_get_chipselect(spi, 0),
> -			ctlr->num_chipselect);
> -		return -EINVAL;
> +	for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) {
> +		/* Chipselects are numbered 0..max; validate. */
> +		if (spi_get_chipselect(spi, idx) >= ctlr-
> >num_chipselect) {
> +			dev_err(dev, "cs%d >= max %d\n",
> spi_get_chipselect(spi, idx),
> +				ctlr->num_chipselect);
> +			return -EINVAL;
> +		}
>  	}
> 
>  	/* Set the bus ID string */
> @@ -713,12 +723,15 @@ static int spi_add_device_locked(struct
> spi_device *spi)
>  {
>  	struct spi_controller *ctlr = spi->controller;
>  	struct device *dev = ctlr->dev.parent;
> +	int idx;
> 
> -	/* Chipselects are numbered 0..max; validate. */
> -	if (spi_get_chipselect(spi, 0) >= ctlr->num_chipselect) {
> -		dev_err(dev, "cs%d >= max %d\n",
> spi_get_chipselect(spi, 0),
> -			ctlr->num_chipselect);
> -		return -EINVAL;
> +	for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) {
> +		/* Chipselects are numbered 0..max; validate. */
> +		if (spi_get_chipselect(spi, idx) >= ctlr-
> >num_chipselect) {
> +			dev_err(dev, "cs%d >= max %d\n",
> spi_get_chipselect(spi, idx),
> +				ctlr->num_chipselect);
> +			return -EINVAL;
> +		}
>  	}
> 
>  	/* Set the bus ID string */
> @@ -966,58 +979,119 @@ static void spi_res_release(struct
> spi_controller *ctlr, struct spi_message *mes
>  static void spi_set_cs(struct spi_device *spi, bool enable, bool
force)
>  {
>  	bool activate = enable;
> +	u32 cs_num = __ffs(spi->cs_index_mask);
> +	int idx;
> 
>  	/*
> -	 * Avoid calling into the driver (or doing delays) if the chip
> select
> -	 * isn't actually changing from the last time this was called.
> +	 * In parallel mode all the chip selects are
asserted/de-asserted
> +	 * at once
>  	 */
> -	if (!force && ((enable && spi->controller->last_cs ==
> spi_get_chipselect(spi, 0)) ||
> -		       (!enable && spi->controller->last_cs !=
> spi_get_chipselect(spi, 0))) &&
> -	    (spi->controller->last_cs_mode_high == (spi->mode &
> SPI_CS_HIGH)))
> -		return;
> -
> -	trace_spi_set_cs(spi, activate);
> -
> -	spi->controller->last_cs = enable ? spi_get_chipselect(spi, 0)
: -
> 1;
> -	spi->controller->last_cs_mode_high = spi->mode &
> SPI_CS_HIGH;
> -
> -	if ((spi_get_csgpiod(spi, 0) ||
!spi->controller->set_cs_timing)
> && !activate)
> -		spi_delay_exec(&spi->cs_hold, NULL);
> -
> -	if (spi->mode & SPI_CS_HIGH)
> -		enable = !enable;
> +	if ((spi->cs_index_mask & SPI_PARALLEL_CS_MASK) ==
> SPI_PARALLEL_CS_MASK) {
> +		spi->controller->last_cs_mode_high = spi->mode &
> SPI_CS_HIGH;
> +
> +		if ((spi_get_csgpiod(spi, 0) || !spi->controller-
> >set_cs_timing) && !activate)
> +			spi_delay_exec(&spi->cs_hold, NULL);
> +
> +		if (spi->mode & SPI_CS_HIGH)
> +			enable = !enable;
> +
> +		if (spi_get_csgpiod(spi, 0) && spi_get_csgpiod(spi,
1)) {
> +			if (!(spi->mode & SPI_NO_CS)) {
> +				/*
> +				 * Historically ACPI has no means of
the
> GPIO polarity and
> +				 * thus the SPISerialBus() resource
> defines it on the per-chip
> +				 * basis. In order to avoid a chain of
> negations, the GPIO
> +				 * polarity is considered being Active
> High. Even for the cases
> +				 * when _DSD() is involved (in the
> updated versions of ACPI)
> +				 * the GPIO CS polarity must be
> defined Active High to avoid
> +				 * ambiguity. That's why we use
> enable, that takes SPI_CS_HIGH
> +				 * into account.
> +				 */
> +				if (has_acpi_companion(&spi->dev)) {
> +					for (idx = 0; idx <
> SPI_CS_CNT_MAX; idx++)
> +
> 	gpiod_set_value_cansleep(spi_get_csgpiod(spi, idx),
> +
> !enable);
> +				} else {
> +					for (idx = 0; idx <
> SPI_CS_CNT_MAX; idx++)
> +						/* Polarity handled by
> GPIO library */
> +
> 	gpiod_set_value_cansleep(spi_get_csgpiod(spi, idx),
> +
> activate);
> +				}
> +			}
> +			/* Some SPI masters need both GPIO CS &
> slave_select */
> +			if ((spi->controller->flags &
> SPI_MASTER_GPIO_SS) &&
> +			    spi->controller->set_cs)
> +				spi->controller->set_cs(spi, !enable);
> +			else if (spi->controller->set_cs)
> +				spi->controller->set_cs(spi, !enable);
> +		}
> 
> -	if (spi_get_csgpiod(spi, 0)) {
> -		if (!(spi->mode & SPI_NO_CS)) {
> -			/*
> -			 * Historically ACPI has no means of the GPIO
> polarity and
> -			 * thus the SPISerialBus() resource defines it
on
> the per-chip
> -			 * basis. In order to avoid a chain of
negations,
> the GPIO
> -			 * polarity is considered being Active High.
Even
> for the cases
> -			 * when _DSD() is involved (in the updated
> versions of ACPI)
> -			 * the GPIO CS polarity must be defined Active
> High to avoid
> -			 * ambiguity. That's why we use enable, that
> takes SPI_CS_HIGH
> -			 * into account.
> -			 */
> -			if (has_acpi_companion(&spi->dev))
> -
> 	gpiod_set_value_cansleep(spi_get_csgpiod(spi, 0), !enable);
> -			else
> -				/* Polarity handled by GPIO library */
> -
> 	gpiod_set_value_cansleep(spi_get_csgpiod(spi, 0), activate);
> +		for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) {
> +			if (spi_get_csgpiod(spi, idx) ||
!spi->controller-
> >set_cs_timing) {
> +				if (activate)
> +					spi_delay_exec(&spi-
> >cs_setup, NULL);
> +				else
> +					spi_delay_exec(&spi-
> >cs_inactive, NULL);
> +			}
>  		}
> -		/* Some SPI masters need both GPIO CS &
> slave_select */
> -		if ((spi->controller->flags & SPI_MASTER_GPIO_SS) &&
> -		    spi->controller->set_cs)
> +	} else {
> +		/*
> +		 * Avoid calling into the driver (or doing delays) if
the
> chip select
> +		 * isn't actually changing from the last time this was
> called.
> +		 */
> +		if (!force && ((enable && spi->controller->last_cs ==
> +				spi_get_chipselect(spi, cs_num)) ||
> +				(!enable && spi->controller->last_cs
!=
> +				 spi_get_chipselect(spi, cs_num))) &&
> +		    (spi->controller->last_cs_mode_high ==
> +		     (spi->mode & SPI_CS_HIGH)))
> +			return;
> +
> +		trace_spi_set_cs(spi, activate);
> +
> +		spi->controller->last_cs = enable ?
> spi_get_chipselect(spi, cs_num) : -1;
> +		spi->controller->last_cs_mode_high = spi->mode &
> SPI_CS_HIGH;
> +
> +		if ((spi_get_csgpiod(spi, cs_num) || !spi->controller-
> >set_cs_timing) && !activate)
> +			spi_delay_exec(&spi->cs_hold, NULL);
> +
> +		if (spi->mode & SPI_CS_HIGH)
> +			enable = !enable;
> +
> +		if (spi_get_csgpiod(spi, cs_num)) {
> +			if (!(spi->mode & SPI_NO_CS)) {
> +				/*
> +				 * Historically ACPI has no means of
the
> GPIO polarity and
> +				 * thus the SPISerialBus() resource
> defines it on the per-chip
> +				 * basis. In order to avoid a chain of
> negations, the GPIO
> +				 * polarity is considered being Active
> High. Even for the cases
> +				 * when _DSD() is involved (in the
> updated versions of ACPI)
> +				 * the GPIO CS polarity must be
> defined Active High to avoid
> +				 * ambiguity. That's why we use
> enable, that takes SPI_CS_HIGH
> +				 * into account.
> +				 */
> +				if (has_acpi_companion(&spi->dev))
> +
> 	gpiod_set_value_cansleep(spi_get_csgpiod(spi, cs_num),
> +
> !enable);
> +				else
> +					/* Polarity handled by GPIO
> library */
> +
> 	gpiod_set_value_cansleep(spi_get_csgpiod(spi, cs_num),
> +
> activate);
> +			}
> +			/* Some SPI masters need both GPIO CS &
> slave_select */
> +			if ((spi->controller->flags &
> SPI_MASTER_GPIO_SS) &&
> +			    spi->controller->set_cs)
> +				spi->controller->set_cs(spi, !enable);
> +		} else if (spi->controller->set_cs) {
>  			spi->controller->set_cs(spi, !enable);
> -	} else if (spi->controller->set_cs) {
> -		spi->controller->set_cs(spi, !enable);
> -	}
> +		}
> 
> -	if (spi_get_csgpiod(spi, 0) ||
!spi->controller->set_cs_timing) {
> -		if (activate)
> -			spi_delay_exec(&spi->cs_setup, NULL);
> -		else
> -			spi_delay_exec(&spi->cs_inactive, NULL);
> +		if (spi_get_csgpiod(spi, cs_num) || !spi->controller-
> >set_cs_timing) {
> +			if (activate)
> +				spi_delay_exec(&spi->cs_setup,
> NULL);
> +			else
> +				spi_delay_exec(&spi->cs_inactive,
> NULL);
> +		}
>  	}
>  }
> 
> @@ -2246,8 +2320,8 @@ static void of_spi_parse_dt_cs_delay(struct
> device_node *nc,
>  static int of_spi_parse_dt(struct spi_controller *ctlr, struct
spi_device
> *spi,
>  			   struct device_node *nc)
>  {
> -	u32 value;
> -	int rc;
> +	u32 value, cs[SPI_CS_CNT_MAX] = {0};
> +	int rc, idx;
> 
>  	/* Mode (clock phase/polarity/etc.) */
>  	if (of_property_read_bool(nc, "spi-cpha"))
> @@ -2320,13 +2394,21 @@ static int of_spi_parse_dt(struct
> spi_controller *ctlr, struct spi_device *spi,
>  	}
> 
>  	/* Device address */
> -	rc = of_property_read_u32(nc, "reg", &value);
> -	if (rc) {
> +	rc = of_property_read_variable_u32_array(nc, "reg", &cs[0], 1,
> +						 SPI_CS_CNT_MAX);
> +	if (rc < 0 || rc > ctlr->num_chipselect) {
>  		dev_err(&ctlr->dev, "%pOF has no valid 'reg' property
> (%d)\n",
>  			nc, rc);
>  		return rc;
> +	} else if ((of_property_read_bool(nc, "parallel-memories"))
> &&
> +		   (!ctlr->multi_cs_cap)) {
> +		dev_err(&ctlr->dev, "SPI controller doesn't support
> multi CS\n");
> +		return -EINVAL;
>  	}
> -	spi_set_chipselect(spi, 0, value);
> +	for (idx = 0; idx < rc; idx++)
> +		spi_set_chipselect(spi, idx, cs[idx]);
> +	/* By default set the spi->cs_index_mask as 1 */
> +	spi->cs_index_mask = 0x01;
> 
>  	/* Device speed */
>  	if (!of_property_read_u32(nc, "spi-max-frequency", &value))
> @@ -3846,6 +3928,7 @@ static int __spi_validate(struct spi_device
> *spi, struct spi_message *message)
>  	struct spi_controller *ctlr = spi->controller;
>  	struct spi_transfer *xfer;
>  	int w_size;
> +	u32 cs_num = __ffs(spi->cs_index_mask);
> 
>  	if (list_empty(&message->transfers))
>  		return -EINVAL;
> @@ -3858,7 +3941,7 @@ static int __spi_validate(struct spi_device
> *spi, struct spi_message *message)
>  	 * cs_change is set for each transfer.
>  	 */
>  	if ((spi->mode & SPI_CS_WORD) && (!(ctlr->mode_bits &
> SPI_CS_WORD) ||
> -					  spi_get_csgpiod(spi, 0))) {
> +					  spi_get_csgpiod(spi,
> cs_num))) {
>  		size_t maxsize;
>  		int ret;
> 
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index bdb35a91b4bf..452682aa1a39 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -19,6 +19,11 @@
>  #include <linux/acpi.h>
>  #include <linux/u64_stats_sync.h>
> 
> +/* Max no. of CS supported per spi device */
> +#define SPI_CS_CNT_MAX 2
> +
> +/* chip select mask */
> +#define SPI_PARALLEL_CS_MASK	(BIT(0) | BIT(1))
>  struct dma_chan;
>  struct software_node;
>  struct ptp_system_timestamp;
> @@ -166,6 +171,7 @@ extern void
> spi_transfer_cs_change_delay_exec(struct spi_message *msg,
>   *	deasserted. If @cs_change_delay is used from @spi_transfer,
> then the
>   *	two delays will be added up.
>   * @pcpu_statistics: statistics for the spi_device
> + * @cs_index_mask: Bit mask of the active chipselect(s) in the
> chipselect array
>   *
>   * A @spi_device is used to interchange data between an SPI slave
>   * (usually a discrete chip) and CPU memory.
> @@ -181,7 +187,7 @@ struct spi_device {
>  	struct spi_controller	*controller;
>  	struct spi_controller	*master;	/* Compatibility layer
> */
>  	u32			max_speed_hz;
> -	u8			chip_select;
> +	u8			chip_select[SPI_CS_CNT_MAX];
>  	u8			bits_per_word;
>  	bool			rt;
>  #define SPI_NO_TX	BIT(31)		/* No transmit wire */
> @@ -202,7 +208,7 @@ struct spi_device {
>  	void			*controller_data;
>  	char			modalias[SPI_NAME_SIZE];
>  	const char		*driver_override;
> -	struct gpio_desc	*cs_gpiod;	/* Chip select gpio
desc
> */
> +	struct gpio_desc	*cs_gpiod[SPI_CS_CNT_MAX];	/*
Chip
> select gpio desc */
>  	struct spi_delay	word_delay; /* Inter-word delay */
>  	/* CS delays */
>  	struct spi_delay	cs_setup;
> @@ -212,6 +218,13 @@ struct spi_device {
>  	/* The statistics */
>  	struct spi_statistics __percpu	*pcpu_statistics;
> 
> +	/* Bit mask of the chipselect(s) that the driver need to use
> from
> +	 * the chipselect array.When the controller is capable to
handle
> +	 * multiple chip selects & memories are connected in parallel
> +	 * then more than one bit need to be set in cs_index_mask.
> +	 */
> +	u32			cs_index_mask : 2;
> +
>  	/*
>  	 * likely need more hooks for more protocol options affecting
> how
>  	 * the controller talks to each chip, like:
> @@ -268,22 +281,22 @@ static inline void *spi_get_drvdata(struct
> spi_device *spi)
> 
>  static inline u8 spi_get_chipselect(struct spi_device *spi, u8 idx)
>  {
> -	return spi->chip_select;
> +	return spi->chip_select[idx];
>  }
> 
>  static inline void spi_set_chipselect(struct spi_device *spi, u8
idx, u8
> chipselect)
>  {
> -	spi->chip_select = chipselect;
> +	spi->chip_select[idx] = chipselect;
>  }
> 
>  static inline struct gpio_desc *spi_get_csgpiod(struct spi_device
*spi,
> u8 idx)
>  {
> -	return spi->cs_gpiod;
> +	return spi->cs_gpiod[idx];
>  }
> 
>  static inline void spi_set_csgpiod(struct spi_device *spi, u8 idx,
struct
> gpio_desc *csgpiod)
>  {
> -	spi->cs_gpiod = csgpiod;
> +	spi->cs_gpiod[idx] = csgpiod;
>  }
> 
>  /**
> @@ -388,6 +401,8 @@ extern struct spi_device
> *spi_new_ancillary_device(struct spi_device *spi, u8 ch
>   * @bus_lock_spinlock: spinlock for SPI bus locking
>   * @bus_lock_mutex: mutex for exclusion of multiple callers
>   * @bus_lock_flag: indicates that the SPI bus is locked for
exclusive
> use
> + * @multi_cs_cap: indicates that the SPI Controller can assert/de-
> assert
> + *	more than one chip select at once.
>   * @setup: updates the device mode and clocking records used by a
>   *	device's SPI controller; protocol code may call this.  This
>   *	must fail if an unrecognized or unsupported mode is
> requested.
> @@ -585,6 +600,13 @@ struct spi_controller {
>  	/* Flag indicating that the SPI bus is locked for exclusive
use */
>  	bool			bus_lock_flag;
> 
> +	/*
> +	 * Flag indicating that the spi-controller has multi chip
select
> +	 * capability and can assert/de-assert more than one chip
> select
> +	 * at once.
> +	 */
> +	bool			multi_cs_cap;
> +
>  	/* Setup mode and clock, etc (spi driver may call many times).
>  	 *
>  	 * IMPORTANT:  this may be called when transfers to another
> --
> 2.25.1