diff mbox series

[RFC,1/2] spi: Add multiple CS support for a single SPI device

Message ID 20220606112607.20800-2-amit.kumar-mahapatra@xilinx.com (mailing list archive)
State New, archived
Headers show
Series spi: Add support for stacked/parallel memories | expand

Commit Message

Amit Kumar Mahapatra June 6, 2022, 11:26 a.m. UTC
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_slect[n].

When flashes are connected in stacked mode SPI-NOR will enable the required
chip select by setting the appropriate bit in spi->cs_index_mask.

In parallel connection both the flashes need to be asserted simultaneously,
this can be achieved by enabling 0th(CS0) & 1st(CS1) bit in spi->cs_index_mask.

Signed-off-by: Amit Kumar Mahapatra <amit.kumar-mahapatra@xilinx.com>
---
 drivers/spi/spi-zynqmp-gqspi.c | 30 ++++++++++++++++++++++++++----
 drivers/spi/spi.c              | 10 +++++++---
 include/linux/spi/spi.h        | 10 +++++++++-
 3 files changed, 42 insertions(+), 8 deletions(-)

Comments

Mark Brown June 9, 2022, 11:54 a.m. UTC | #1
On Mon, Jun 06, 2022 at 04:56:06PM +0530, Amit Kumar Mahapatra wrote:

> ---
>  drivers/spi/spi-zynqmp-gqspi.c | 30 ++++++++++++++++++++++++++----
>  drivers/spi/spi.c              | 10 +++++++---
>  include/linux/spi/spi.h        | 10 +++++++++-
>  3 files changed, 42 insertions(+), 8 deletions(-)

Please split the core and driver support into separate patches, they are
separate things.

> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -2082,6 +2082,8 @@ static int of_spi_parse_dt(struct spi_controller *ctlr, struct spi_device *spi,
>  {
>  	u32 value;
>  	int rc;
> +	u32 cs[SPI_CS_CNT_MAX];
> +	u8 idx;
>  
>  	/* Mode (clock phase/polarity/etc.) */
>  	if (of_property_read_bool(nc, "spi-cpha"))

This is changing the DT binding but doesn't have any updates to the
binding document.  The binding code also doesn't validate that we don't
have too many chip selects.

> +	/* Bit mask of the chipselect(s) that the driver
> +	 * need to use form the chipselect array.
> +	 */
> +	u8			cs_index_mask : 2;

Why make this a bitfield?  

I'm also not seeing anything here that checks that the driver supports
multiple chip selects - it seems like something that's going to cause
issues and we should probably have something to handle that situation.
Mahapatra, Amit Kumar June 23, 2022, 11:39 a.m. UTC | #2
Hello Mark,

> -----Original Message-----
> From: linux-arm-kernel <linux-arm-kernel-bounces@lists.infradead.org> On
> Behalf Of Mark Brown
> Sent: Thursday, June 9, 2022 5:24 PM
> To: Amit Kumar Mahapatra <amit.kumar-mahapatra@xilinx.com>
> Cc: p.yadav@ti.com; miquel.raynal@bootlin.com; richard@nod.at;
> vigneshr@ti.com; git@xilinx.com; michal.simek@xilinx.com; linux-
> spi@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; michael@walle.cc; linux-mtd@lists.infradead.org
> Subject: Re: [RFC PATCH 1/2] spi: Add multiple CS support for a single SPI
> device
> 
> On Mon, Jun 06, 2022 at 04:56:06PM +0530, Amit Kumar Mahapatra wrote:
> 
> > ---
> >  drivers/spi/spi-zynqmp-gqspi.c | 30 ++++++++++++++++++++++++++----
> >  drivers/spi/spi.c              | 10 +++++++---
> >  include/linux/spi/spi.h        | 10 +++++++++-
> >  3 files changed, 42 insertions(+), 8 deletions(-)
> 
> Please split the core and driver support into separate patches, they are
> separate things.

Ok, I will split the patches.
> 
> > --- a/drivers/spi/spi.c
> > +++ b/drivers/spi/spi.c
> > @@ -2082,6 +2082,8 @@ static int of_spi_parse_dt(struct spi_controller
> > *ctlr, struct spi_device *spi,  {
> >  	u32 value;
> >  	int rc;
> > +	u32 cs[SPI_CS_CNT_MAX];
> > +	u8 idx;
> >
> >  	/* Mode (clock phase/polarity/etc.) */
> >  	if (of_property_read_bool(nc, "spi-cpha"))
> 
> This is changing the DT binding but doesn't have any updates to the binding
> document.  The binding code also doesn't validate that we don't have too
> many chip selects.

The following updates are done in the binding documents for adding multiple
CS support:
In jedec,spi-nor.yaml file " maxItems " of the "reg" DT property has been 
updated to accommodate two CS per SPI device.  
https://github.com/torvalds/linux/blob/de5c208d533a46a074eb46ea17f672cc005a7269/Documentation/devicetree/bindings/mtd/jedec%2Cspi-nor.yaml#L49

An example of a flash node with two CS has been added in spi-controller.yaml
https://github.com/torvalds/linux/blob/de5c208d533a46a074eb46ea17f672cc005a7269/Documentation/devicetree/bindings/spi/spi-controller.yaml#L141
> 
> > +	/* Bit mask of the chipselect(s) that the driver
> > +	 * need to use form the chipselect array.
> > +	 */
> > +	u8			cs_index_mask : 2;
> 
> Why make this a bitfield?

https://github.com/torvalds/linux/blob/de5c208d533a46a074eb46ea17f672cc005a7269/Documentation/devicetree/bindings/mtd/jedec%2Cspi-nor.yaml#L49

As per the DT bindings we are supporting max 2 chip selects per SPI device
that is the reason I had taken it as an bitfield of 2. But now I think that in 
future when the number of chip selects per device would increase i.e., 
more than 2, then we have to again increase the bitfield allocation for 
accommodating the increase in the number of chip selects per SPI device, 
So I think it's better to drop the bitfield for now and use cs_index_mask 
as an u8
> 
> I'm also not seeing anything here that checks that the driver supports
> multiple chip selects - it seems like something that's going to cause issues
> and we should probably have something to handle that situation.

In my approach the chip select member (chip_select) of the spi_device structure 
is changed to an array (chip_select[2]). This array is used to store the CS values 
coming from the "reg" DT property. 
In case of multiple chip selects  spi->chip_slect[0] will hold CS0 value & 
spi->chip_select[1] wil hold CS1 value.
In case of single chip select the spi->chip_select[0] will hold the chip select value.

As per the current implementation all the drivers fetch their chip select value form
spi->chip_select, but now the driver code needs to be modified to fetch the value
from spi->chip_select[0] instead and by this approach we do not need to check if the 
driver supports single or multiple CS.

Hope I addressed all your concerns and please let us know what you think.

Regards,
Amit
Mark Brown June 23, 2022, 12:06 p.m. UTC | #3
On Thu, Jun 23, 2022 at 11:39:19AM +0000, Mahapatra, Amit Kumar wrote:

> > >  	/* Mode (clock phase/polarity/etc.) */
> > >  	if (of_property_read_bool(nc, "spi-cpha"))

> > This is changing the DT binding but doesn't have any updates to the binding
> > document.  The binding code also doesn't validate that we don't have too
> > many chip selects.

> The following updates are done in the binding documents for adding multiple
> CS support:
> In jedec,spi-nor.yaml file " maxItems " of the "reg" DT property has been 
> updated to accommodate two CS per SPI device.  

This is a change to a binding for a specific driver, this is changing
the SPI core.

> > I'm also not seeing anything here that checks that the driver supports
> > multiple chip selects - it seems like something that's going to cause issues
> > and we should probably have something to handle that situation.

> In my approach the chip select member (chip_select) of the spi_device structure 
> is changed to an array (chip_select[2]). This array is used to store the CS values 
> coming from the "reg" DT property. 
> In case of multiple chip selects  spi->chip_slect[0] will hold CS0 value & 
> spi->chip_select[1] wil hold CS1 value.
> In case of single chip select the spi->chip_select[0] will hold the chip select value.

That doesn't address the issue, the issue is checking that the driver
can support multiple chip selects.
Michal Simek July 11, 2022, 12:47 p.m. UTC | #4
Hi Mark,

On 6/9/22 13:54, Mark Brown wrote:
> On Mon, Jun 06, 2022 at 04:56:06PM +0530, Amit Kumar Mahapatra wrote:
> 
>> ---
>>   drivers/spi/spi-zynqmp-gqspi.c | 30 ++++++++++++++++++++++++++----
>>   drivers/spi/spi.c              | 10 +++++++---
>>   include/linux/spi/spi.h        | 10 +++++++++-
>>   3 files changed, 42 insertions(+), 8 deletions(-)
> 
> Please split the core and driver support into separate patches, they are
> separate things.
> 
>> --- a/drivers/spi/spi.c
>> +++ b/drivers/spi/spi.c
>> @@ -2082,6 +2082,8 @@ static int of_spi_parse_dt(struct spi_controller *ctlr, struct spi_device *spi,
>>   {
>>   	u32 value;
>>   	int rc;
>> +	u32 cs[SPI_CS_CNT_MAX];
>> +	u8 idx;
>>   
>>   	/* Mode (clock phase/polarity/etc.) */
>>   	if (of_property_read_bool(nc, "spi-cpha"))
> 
> This is changing the DT binding but doesn't have any updates to the
> binding document.  The binding code also doesn't validate that we don't
> have too many chip selects.

I would like to better understand your request here in connection to change in 
the binding code for validation.
What exactly do you want to validate?
That child reg property is not bigger than num-cs in controller node?

Adding also Krzysztof because I was talking to him over IRC about this.

Thanks,
Michal
Mark Brown July 11, 2022, 2:52 p.m. UTC | #5
On Mon, Jul 11, 2022 at 02:47:54PM +0200, Michal Simek wrote:
> On 6/9/22 13:54, Mark Brown wrote:
> > On Mon, Jun 06, 2022 at 04:56:06PM +0530, Amit Kumar Mahapatra wrote:

> > > +	u32 cs[SPI_CS_CNT_MAX];
> > > +	u8 idx;
> > >   	/* Mode (clock phase/polarity/etc.) */
> > >   	if (of_property_read_bool(nc, "spi-cpha"))

> > This is changing the DT binding but doesn't have any updates to the
> > binding document.  The binding code also doesn't validate that we don't
> > have too many chip selects.

> I would like to better understand your request here in connection to change
> in the binding code for validation.
> What exactly do you want to validate?
> That child reg property is not bigger than num-cs in controller node?

If you are adding support for multiple chip selects in the driver then
there must be some mechanism for expressing that in the bindings which I
would expect to see appear as a change to the binding document.
Mahapatra, Amit Kumar July 15, 2022, 3:35 p.m. UTC | #6
Hello Mark,

> -----Original Message-----
> From: Mark Brown <broonie@kernel.org>
> Sent: Thursday, June 23, 2022 5:37 PM
> To: Mahapatra, Amit Kumar <amit.kumar-mahapatra@amd.com>
> Cc: Amit Kumar Mahapatra <amit.kumar-mahapatra@xilinx.com>;
> p.yadav@ti.com; miquel.raynal@bootlin.com; richard@nod.at;
> vigneshr@ti.com; git@xilinx.com; michal.simek@xilinx.com; linux-
> spi@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; michael@walle.cc; linux-mtd@lists.infradead.org;
> git (AMD-Xilinx) <git@amd.com>
> Subject: Re: [RFC PATCH 1/2] spi: Add multiple CS support for a single SPI
> device
> 
> On Thu, Jun 23, 2022 at 11:39:19AM +0000, Mahapatra, Amit Kumar wrote:
> 
> > > >  	/* Mode (clock phase/polarity/etc.) */
> > > >  	if (of_property_read_bool(nc, "spi-cpha"))
> 
> > > This is changing the DT binding but doesn't have any updates to the
> > > binding document.  The binding code also doesn't validate that we
> > > don't have too many chip selects.
> 
> > The following updates are done in the binding documents for adding
> > multiple CS support:
> > In jedec,spi-nor.yaml file " maxItems " of the "reg" DT property has
> > been updated to accommodate two CS per SPI device.
> 
> This is a change to a binding for a specific driver, this is changing the SPI
> core.
> 
> > > I'm also not seeing anything here that checks that the driver
> > > supports multiple chip selects - it seems like something that's
> > > going to cause issues and we should probably have something to handle
> that situation.
> 
> > In my approach the chip select member (chip_select) of the spi_device
> > structure is changed to an array (chip_select[2]). This array is used
> > to store the CS values coming from the "reg" DT property.
> > In case of multiple chip selects  spi->chip_slect[0] will hold CS0
> > value &
> > spi->chip_select[1] wil hold CS1 value.
> > In case of single chip select the spi->chip_select[0] will hold the chip select
> value.
> 
> That doesn't address the issue, the issue is checking that the driver can
> support multiple chip selects.

To address this issue, in spi core we will check the number of items 
in the "reg" property of the flash node(which is nothing but the 
number of chip selects) against the "num-cs" property of the spi 
controller(which is total number of chip selects supported by the 
controller). If the number of items mentioned in the "reg" property 
is greater than "num-cs" value then we error out.

For eg.,

rc = of_property_read_variable_u32_array(nc, "reg", &cs[0], 1, 
						SPI_CS_CNT_MAX);
if(rc > ctlr->num_chipselect) {
	dev_err(&ctlr->dev, "%pOF has invalid 'reg' property (%d)\n", 
							nc, rc);
	return -EINVAL;
}

Please let us know if you have any other approach in mind.

Regards,
Amit
Mahapatra, Amit Kumar July 15, 2022, 3:36 p.m. UTC | #7
Hello Mark,

> -----Original Message-----
> From: Mark Brown <broonie@kernel.org>
> Sent: Monday, July 11, 2022 8:23 PM
> To: Michal Simek <michal.simek@xilinx.com>
> Cc: Amit Kumar Mahapatra <amit.kumar-mahapatra@xilinx.com>; Krzysztof
> Kozlowski <krzysztof.kozlowski+dt@linaro.org>; p.yadav@ti.com;
> miquel.raynal@bootlin.com; richard@nod.at; vigneshr@ti.com;
> git@xilinx.com; linux-spi@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; michael@walle.cc;
> linux-mtd@lists.infradead.org
> Subject: Re: [RFC PATCH 1/2] spi: Add multiple CS support for a single SPI
> device
> 
> On Mon, Jul 11, 2022 at 02:47:54PM +0200, Michal Simek wrote:
> > On 6/9/22 13:54, Mark Brown wrote:
> > > On Mon, Jun 06, 2022 at 04:56:06PM +0530, Amit Kumar Mahapatra
> wrote:
> 
> > > > +	u32 cs[SPI_CS_CNT_MAX];
> > > > +	u8 idx;
> > > >   	/* Mode (clock phase/polarity/etc.) */
> > > >   	if (of_property_read_bool(nc, "spi-cpha"))
> 
> > > This is changing the DT binding but doesn't have any updates to the
> > > binding document.  The binding code also doesn't validate that we
> > > don't have too many chip selects.
> 
> > I would like to better understand your request here in connection to
> > change in the binding code for validation.
> > What exactly do you want to validate?
> > That child reg property is not bigger than num-cs in controller node?
> 
> If you are adding support for multiple chip selects in the driver then there
> must be some mechanism for expressing that in the bindings which I would
> expect to see appear as a change to the binding document.

Thank you for the clarification.
As per my understanding we would not be needing a new DT 
property for expressing multi chip select support in the driver.
We can use "num-cs" property of the spi controller for 
broadcasting multi chip select support of the driver.
If "num-cs" value is greater than 1 then the driver can support 
multiple chip selects.

Please let us know if you have any other approach in mind.

Regards,
Amit
Mark Brown July 15, 2022, 3:54 p.m. UTC | #8
On Fri, Jul 15, 2022 at 03:35:49PM +0000, Mahapatra, Amit Kumar wrote:

> > That doesn't address the issue, the issue is checking that the driver can
> > support multiple chip selects.

> To address this issue, in spi core we will check the number of items 
> in the "reg" property of the flash node(which is nothing but the 
> number of chip selects) against the "num-cs" property of the spi 
> controller(which is total number of chip selects supported by the 
> controller). If the number of items mentioned in the "reg" property 
> is greater than "num-cs" value then we error out.

> For eg.,

> rc = of_property_read_variable_u32_array(nc, "reg", &cs[0], 1, 
> 						SPI_CS_CNT_MAX);
> if(rc > ctlr->num_chipselect) {
> 	dev_err(&ctlr->dev, "%pOF has invalid 'reg' property (%d)\n", 
> 							nc, rc);
> 	return -EINVAL;
> }

This would check that the controller has at least the number of chip
selects specified but it would not check that the controller is actually
capable of using more than one chip select at once.  We should be
validating both that the chip selects are available and that the
controller can do something useful with them (and probably have an
implementation in the core for doing so via GPIO).
Mark Brown July 15, 2022, 4:03 p.m. UTC | #9
On Fri, Jul 15, 2022 at 03:36:23PM +0000, Mahapatra, Amit Kumar wrote:

> Thank you for the clarification.
> As per my understanding we would not be needing a new DT 
> property for expressing multi chip select support in the driver.
> We can use "num-cs" property of the spi controller for 
> broadcasting multi chip select support of the driver.
> If "num-cs" value is greater than 1 then the driver can support 
> multiple chip selects.

That's true in some sense but not in the sense of being actually able to
use more than one chip select for a single device.  If the controller
hardware manages the chip selects it may not be physically possible for
it to control more than one chip select.

While it's not an issue for the bindings even hardware that can
physically do it is going to need a driver update (in the core for GPIO
stuff) to do so.
Mahapatra, Amit Kumar July 19, 2022, 1:21 p.m. UTC | #10
Hello Mark,

> -----Original Message-----
> From: Mark Brown <broonie@kernel.org>
> Sent: Friday, July 15, 2022 9:24 PM
> To: Mahapatra, Amit Kumar <amit.kumar-mahapatra@amd.com>
> Cc: Amit Kumar Mahapatra <amit.kumar-mahapatra@xilinx.com>;
> p.yadav@ti.com; miquel.raynal@bootlin.com; richard@nod.at;
> vigneshr@ti.com; git@xilinx.com; michal.simek@xilinx.com; linux-
> spi@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; michael@walle.cc; linux-mtd@lists.infradead.org;
> git (AMD-Xilinx) <git@amd.com>
> Subject: Re: [RFC PATCH 1/2] spi: Add multiple CS support for a single SPI
> device
> 
> On Fri, Jul 15, 2022 at 03:35:49PM +0000, Mahapatra, Amit Kumar wrote:
> 
> > > That doesn't address the issue, the issue is checking that the
> > > driver can support multiple chip selects.
> 
> > To address this issue, in spi core we will check the number of items
> > in the "reg" property of the flash node(which is nothing but the
> > number of chip selects) against the "num-cs" property of the spi
> > controller(which is total number of chip selects supported by the
> > controller). If the number of items mentioned in the "reg" property is
> > greater than "num-cs" value then we error out.
> 
> > For eg.,
> 
> > rc = of_property_read_variable_u32_array(nc, "reg", &cs[0], 1,
> > 						SPI_CS_CNT_MAX);
> > if(rc > ctlr->num_chipselect) {
> > 	dev_err(&ctlr->dev, "%pOF has invalid 'reg' property (%d)\n",
> > 							nc, rc);
> > 	return -EINVAL;
> > }
> 
> This would check that the controller has at least the number of chip selects
> specified but it would not check that the controller is actually capable of
> using more than one chip select at once. We should be validating both that

I agree, so for checking the controller multiple chip select capability(using 
more than one chip select at once) we can define a new spi controller DT 
property like "multi-cs-cap"(please suggest a better name). 
The controller that can support multiple chip selects should have this property 
in the spi controller DT node. The spi core will check ctlr->multi-cs-cap to 
operate multiple chip select in parallel.

> the chip selects are available and that the controller can do something useful
> with them (and probably have an implementation in the core for doing so via
> GPIO).

Here are you referring to the usecase in which a controller implementing multi CS
support using GPIO?  


Regards,
Amit
Mark Brown July 19, 2022, 5:53 p.m. UTC | #11
On Tue, Jul 19, 2022 at 01:21:41PM +0000, Mahapatra, Amit Kumar wrote:

> I agree, so for checking the controller multiple chip select capability(using 
> more than one chip select at once) we can define a new spi controller DT 
> property like "multi-cs-cap"(please suggest a better name). 
> The controller that can support multiple chip selects should have this property 
> in the spi controller DT node. The spi core will check ctlr->multi-cs-cap to 
> operate multiple chip select in parallel.

I'm not sure this needs to be a DT property, it's more just something we
infer from the compatible.  The name seems fine, as does the flag in the
controller data.

> > the chip selects are available and that the controller can do something useful
> > with them (and probably have an implementation in the core for doing so via
> > GPIO).

> Here are you referring to the usecase in which a controller implementing multi CS
> support using GPIO?  

Yes, we probably ought to.
Mahapatra, Amit Kumar July 27, 2022, 1:02 p.m. UTC | #12
Hello Mark,

> -----Original Message-----
> From: Mark Brown <broonie@kernel.org>
> Sent: Tuesday, July 19, 2022 11:23 PM
> To: Mahapatra, Amit Kumar <amit.kumar-mahapatra@amd.com>
> Cc: Amit Kumar Mahapatra <amit.kumar-mahapatra@xilinx.com>;
> p.yadav@ti.com; miquel.raynal@bootlin.com; richard@nod.at;
> vigneshr@ti.com; git@xilinx.com; michal.simek@xilinx.com; linux-
> spi@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; michael@walle.cc; linux-mtd@lists.infradead.org;
> git (AMD-Xilinx) <git@amd.com>
> Subject: Re: [RFC PATCH 1/2] spi: Add multiple CS support for a single SPI
> device
> 
> On Tue, Jul 19, 2022 at 01:21:41PM +0000, Mahapatra, Amit Kumar wrote:
> 
> > I agree, so for checking the controller multiple chip select
> > capability(using more than one chip select at once) we can define a
> > new spi controller DT property like "multi-cs-cap"(please suggest a better
> name).
> > The controller that can support multiple chip selects should have this
> > property in the spi controller DT node. The spi core will check
> > ctlr->multi-cs-cap to operate multiple chip select in parallel.
> 
> I'm not sure this needs to be a DT property, it's more just something we infer
> from the compatible.  The name seems fine, as does the flag in the controller
> data.

I agree that we can infer this from the compatible and set the flag in the controller data.

> 
> > > the chip selects are available and that the controller can do
> > > something useful with them (and probably have an implementation in
> > > the core for doing so via GPIO).
> 
> > Here are you referring to the usecase in which a controller
> > implementing multi CS support using GPIO?
> 
> Yes, we probably ought to.

In my next version I will add the implementation in the spi core for multi CS support using GPIO, but I will not be able test it as I don't have the necessary hardware setup .

Regards,
Amit
diff mbox series

Patch

diff --git a/drivers/spi/spi-zynqmp-gqspi.c b/drivers/spi/spi-zynqmp-gqspi.c
index c760aac070e5..2535a8bca4da 100644
--- a/drivers/spi/spi-zynqmp-gqspi.c
+++ b/drivers/spi/spi-zynqmp-gqspi.c
@@ -136,6 +136,11 @@ 
 
 #define GQSPI_MAX_NUM_CS	2	/* Maximum number of chip selects */
 
+#define GQSPI_SELECT_LOWER_CS	BIT(0)
+#define GQSPI_SELECT_UPPER_CS	BIT(1)
+#define GQSPI_SELECT_BOTH_CS	(GQSPI_SELECT_LOWER_CS | \
+				 GQSPI_SELECT_UPPER_CS)
+				 
 #define SPI_AUTOSUSPEND_TIMEOUT		3000
 enum mode_type {GQSPI_MODE_IO, GQSPI_MODE_DMA};
 
@@ -361,16 +366,33 @@  static void zynqmp_qspi_chipselect(struct spi_device *qspi, bool is_high)
 	struct zynqmp_qspi *xqspi = spi_master_get_devdata(qspi->master);
 	ulong timeout;
 	u32 genfifoentry = 0, statusreg;
+	u8 cs_index_mask = qspi->cs_index_mask;
 
 	genfifoentry |= GQSPI_GENFIFO_MODE_SPI;
 
 	if (!is_high) {
-		if (!qspi->chip_select) {
-			xqspi->genfifobus = GQSPI_GENFIFO_BUS_LOWER;
-			xqspi->genfifocs = GQSPI_GENFIFO_CS_LOWER;
-		} else {
+		/*
+		 * GQSPI controller only supports two chip selects,
+		 * CS0 and CS1
+		 */
+		if (cs_index_mask & GQSPI_SELECT_BOTH_CS) {
+
+			xqspi->genfifobus = GQSPI_GENFIFO_BUS_LOWER |
+				GQSPI_GENFIFO_BUS_UPPER;
+			xqspi->genfifocs = GQSPI_GENFIFO_CS_LOWER |
+				GQSPI_GENFIFO_CS_UPPER;
+
+		} else if ((cs_index_mask & GQSPI_SELECT_UPPER_CS) &&
+			   (qspi->chip_select[GQSPI_SELECT_UPPER_CS - 1])) {
+
 			xqspi->genfifobus = GQSPI_GENFIFO_BUS_UPPER;
 			xqspi->genfifocs = GQSPI_GENFIFO_CS_UPPER;
+
+		} else if ((cs_index_mask & GQSPI_SELECT_LOWER_CS) &&
+			   (!qspi->chip_select[GQSPI_SELECT_LOWER_CS - 1])) {
+
+			xqspi->genfifobus = GQSPI_GENFIFO_BUS_LOWER;
+			xqspi->genfifocs = GQSPI_GENFIFO_CS_LOWER;
 		}
 		genfifoentry |= xqspi->genfifobus;
 		genfifoentry |= xqspi->genfifocs;
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 2e6d6bbeb784..d3077874e6e8 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -2082,6 +2082,8 @@  static int of_spi_parse_dt(struct spi_controller *ctlr, struct spi_device *spi,
 {
 	u32 value;
 	int rc;
+	u32 cs[SPI_CS_CNT_MAX];
+	u8 idx;
 
 	/* Mode (clock phase/polarity/etc.) */
 	if (of_property_read_bool(nc, "spi-cpha"))
@@ -2154,13 +2156,15 @@  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) {
 		dev_err(&ctlr->dev, "%pOF has no valid 'reg' property (%d)\n",
 			nc, rc);
 		return rc;
 	}
-	spi->chip_select = value;
+	for(idx = 0; idx < rc; idx++)
+		spi->chip_select[idx] = cs[idx];
 
 	/* Device speed */
 	if (!of_property_read_u32(nc, "spi-max-frequency", &value))
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 5f8c063ddff4..e930d987f3c2 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -18,6 +18,9 @@ 
 #include <uapi/linux/spi/spi.h>
 #include <linux/acpi.h>
 
+/* Max no. of CS supported per spi device */
+#define SPI_CS_CNT_MAX	2
+
 struct dma_chan;
 struct software_node;
 struct ptp_system_timestamp;
@@ -148,6 +151,7 @@  extern int spi_delay_exec(struct spi_delay *_delay, struct spi_transfer *xfer);
  *	deasserted. If @cs_change_delay is used from @spi_transfer, then the
  *	two delays will be added up.
  * @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.
@@ -163,7 +167,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 */
@@ -194,6 +198,10 @@  struct spi_device {
 	/* the statistics */
 	struct spi_statistics	statistics;
 
+	/* Bit mask of the chipselect(s) that the driver
+	 * need to use form the chipselect array.
+	 */
+	u8			cs_index_mask : 2;
 	/*
 	 * likely need more hooks for more protocol options affecting how
 	 * the controller talks to each chip, like: