RE: [LINUX RFC v2 1/4] spi: add support of two chip selects & data stripe

2015-09-04 Thread Ranjit Abhimanyu Waghmode
Hi Mark,

> -Original Message-
> From: Mark Brown [mailto:broo...@kernel.org]
> Sent: Thursday, September 03, 2015 5:43 PM
> To: Ranjit Abhimanyu Waghmode
> Cc: dw...@infradead.org; computersforpe...@gmail.com; Michal Simek;
> Soren Brinkmann; zaj...@gmail.com; b...@decadent.org.uk; ma...@denx.de;
> b32...@freescale.com; knut.wohl...@de.bosch.com; juh...@openwrt.org;
> bean...@micron.com; linux-...@lists.infradead.org; linux-
> ker...@vger.kernel.org; linux-...@vger.kernel.org; linux-arm-
> ker...@lists.infradead.org; Harini Katakam; Punnaiah Choudary Kalluri
> Subject: Re: [LINUX RFC v2 1/4] spi: add support of two chip selects & data
> stripe
> 
> On Wed, Aug 26, 2015 at 11:56:04AM +0530, Ranjit Waghmode wrote:
> 
> > To support dual parallel mode operation of ZynqMP GQSPI controller
> > following API's are added inside the core:
> 
> As covered in SubmittingPatches please try to make each patch a single change
> rather than having multiple separate changes in one commit.
I will split the patch.
> 
> > +   /* Controller may support more than one chip.
> > +* This flag will enable that feature.
> > +*/
> > +#define SPI_MASTER_BOTH_CS BIT(8)  /* enable both
> chips */
> 
> This isn't saying that the controller supports more than one chip, it's 
> saying that
> the controller supports asserting more than one chip select at once which 
> isn't
> the same thing.  I'm also not entirely sure that this makes sense as a 
> separate
> feature to the data striping one - I'm struggling to think of a way to use 
> this
> sensibly separately to that.
If the SPI controller is having more than one chip select and the data lines 
are distributed equally.
And also there is requirement to activate all the chip selects in one go.

Now we can consider following use cases:

Suppose we need to send the same data to multiple slaves of same kind:
Here the application need not to do individual slave access for writing, 
instead it can send data to all the devices in one go.

Let's take another case where application is trying to send data in such a way 
that first nibble of the byte will got to the one slave and the second nibble 
of the byte will go to the other slave:
Here data in slave devices can be organized by taking advantage of above 
topology along with the support in hardware.

Regards,
Ranjit
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [LINUX RFC v2 1/4] spi: add support of two chip selects & data stripe

2015-09-04 Thread Ranjit Abhimanyu Waghmode
Hi Mark,

> -Original Message-
> From: Mark Brown [mailto:broo...@kernel.org]
> Sent: Thursday, September 03, 2015 5:43 PM
> To: Ranjit Abhimanyu Waghmode
> Cc: dw...@infradead.org; computersforpe...@gmail.com; Michal Simek;
> Soren Brinkmann; zaj...@gmail.com; b...@decadent.org.uk; ma...@denx.de;
> b32...@freescale.com; knut.wohl...@de.bosch.com; juh...@openwrt.org;
> bean...@micron.com; linux-...@lists.infradead.org; linux-
> ker...@vger.kernel.org; linux-...@vger.kernel.org; linux-arm-
> ker...@lists.infradead.org; Harini Katakam; Punnaiah Choudary Kalluri
> Subject: Re: [LINUX RFC v2 1/4] spi: add support of two chip selects & data
> stripe
> 
> On Wed, Aug 26, 2015 at 11:56:04AM +0530, Ranjit Waghmode wrote:
> 
> > To support dual parallel mode operation of ZynqMP GQSPI controller
> > following API's are added inside the core:
> 
> As covered in SubmittingPatches please try to make each patch a single change
> rather than having multiple separate changes in one commit.
I will split the patch.
> 
> > +   /* Controller may support more than one chip.
> > +* This flag will enable that feature.
> > +*/
> > +#define SPI_MASTER_BOTH_CS BIT(8)  /* enable both
> chips */
> 
> This isn't saying that the controller supports more than one chip, it's 
> saying that
> the controller supports asserting more than one chip select at once which 
> isn't
> the same thing.  I'm also not entirely sure that this makes sense as a 
> separate
> feature to the data striping one - I'm struggling to think of a way to use 
> this
> sensibly separately to that.
If the SPI controller is having more than one chip select and the data lines 
are distributed equally.
And also there is requirement to activate all the chip selects in one go.

Now we can consider following use cases:

Suppose we need to send the same data to multiple slaves of same kind:
Here the application need not to do individual slave access for writing, 
instead it can send data to all the devices in one go.

Let's take another case where application is trying to send data in such a way 
that first nibble of the byte will got to the one slave and the second nibble 
of the byte will go to the other slave:
Here data in slave devices can be organized by taking advantage of above 
topology along with the support in hardware.

Regards,
Ranjit
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [LINUX RFC v2 0/4] spi: add dual parallel mode support in Zynq MPSoC GQSPI controller

2015-09-03 Thread Ranjit Abhimanyu Waghmode
Hi,

> -Original Message-
> From: Marek Vasut [mailto:ma...@denx.de]
> Sent: Thursday, September 03, 2015 12:26 AM
> To: Ranjit Abhimanyu Waghmode
> Cc: dw...@infradead.org; computersforpe...@gmail.com;
> broo...@kernel.org; Michal Simek; Soren Brinkmann; zaj...@gmail.com;
> b...@decadent.org.uk; b32...@freescale.com; knut.wohl...@de.bosch.com;
> juh...@openwrt.org; bean...@micron.com; linux-...@lists.infradead.org;
> linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; linux-arm-
> ker...@lists.infradead.org; Harini Katakam; Punnaiah Choudary Kalluri
> Subject: Re: [LINUX RFC v2 0/4] spi: add dual parallel mode support in Zynq
> MPSoC GQSPI controller
> 
> On Wednesday, September 02, 2015 at 07:12:14 PM, Ranjit Abhimanyu
> Waghmode
> wrote:
> > Hi Marek,
> >
> > > -Original Message-
> > > From: Marek Vasut [mailto:ma...@denx.de]
> > > Sent: Wednesday, August 26, 2015 12:26 PM
> > > To: Ranjit Abhimanyu Waghmode
> > > Cc: dw...@infradead.org; computersforpe...@gmail.com;
> > > broo...@kernel.org; Michal Simek; Soren Brinkmann; zaj...@gmail.com;
> > > b...@decadent.org.uk; b32...@freescale.com;
> > > knut.wohl...@de.bosch.com; juh...@openwrt.org;
> bean...@micron.com;
> > > linux-...@lists.infradead.org; linux-kernel@vger.kernel.org;
> > > linux-...@vger.kernel.org; linux-arm- ker...@lists.infradead.org;
> > > Harini Katakam; Punnaiah Choudary Kalluri
> > > Subject: Re: [LINUX RFC v2 0/4] spi: add dual parallel mode support
> > > in Zynq MPSoC GQSPI controller
> > >
> > > On Wednesday, August 26, 2015 at 08:26:03 AM, Ranjit Waghmode wrote:
> > > > This series adds dual parallel mode support for Zynq Ultrascale+
> > > > MPSoC GQSPI controller driver.
> > > >
> > > > What is dual parallel mode?
> > > > ---
> > > > ZynqMP GQSPI controller supports Dual Parallel mode with following
> > > > functionalities: 1) Supporting two SPI flash memories operating in
> > > > parallel. 8 I/O lines. 2) Chip selects and clock are shared to
> > > > both the flash devices
> > > > 3) This mode is targeted for faster read/write speed and also
> > > > doubles the size 4) Commands/data can be transmitted/received from
> > > > both the devices(mirror), or only upper or only lower flash memory
> devices.
> > > >
> > > > 5) Data arrangement:
> > > >With stripe enabled,
> > > >Even bytes i.e. 0, 2, 4,... are transmitted on Lower Data Bus
> > > >Odd bytes i.e. 1, 3, 5,.. are transmitted on Upper Data Bus.
> > >
> > > This might be a dumb question, but why don't you just treat this as
> > > an SPI NOR flash with 8-bit bus ?
> >
> > In case of dual parallel configuration of this controller there are
> > different modes like single, dual and quad mode. Whatever you are
> > suggesting would fit only in the case of Quad mode operation as both
> > buses would have 4 lines each. In case of single mode of parallel
> > configuration, there would be two buses; but the line on each bus
> > would one. So altogether there will be two lines. And in case of dual
> > mode of parallel configuration each bus will be having two lines. So
> > altogether 4 lines will be there. So keeping 8 lines would not support
> > above two modes of parallel configuration correctly.
> >
> > Logically it is a single flash with 8 IO lines but physically it's a
> > two flash devices and each has 4 IO lines. So, in this case, read and
> > write addresses should be always even and minimum data that can be
> > accessed is 2 bytes.
> 
> Oh, I see what the issue is now. It has to do with configuring the flash into
> correct bus-width mode, right ?
Yes.
> 
> Best regards,
> Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [LINUX RFC v2 0/4] spi: add dual parallel mode support in Zynq MPSoC GQSPI controller

2015-09-03 Thread Ranjit Abhimanyu Waghmode
Hi,

> -Original Message-
> From: Marek Vasut [mailto:ma...@denx.de]
> Sent: Thursday, September 03, 2015 12:26 AM
> To: Ranjit Abhimanyu Waghmode
> Cc: dw...@infradead.org; computersforpe...@gmail.com;
> broo...@kernel.org; Michal Simek; Soren Brinkmann; zaj...@gmail.com;
> b...@decadent.org.uk; b32...@freescale.com; knut.wohl...@de.bosch.com;
> juh...@openwrt.org; bean...@micron.com; linux-...@lists.infradead.org;
> linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; linux-arm-
> ker...@lists.infradead.org; Harini Katakam; Punnaiah Choudary Kalluri
> Subject: Re: [LINUX RFC v2 0/4] spi: add dual parallel mode support in Zynq
> MPSoC GQSPI controller
> 
> On Wednesday, September 02, 2015 at 07:12:14 PM, Ranjit Abhimanyu
> Waghmode
> wrote:
> > Hi Marek,
> >
> > > -Original Message-
> > > From: Marek Vasut [mailto:ma...@denx.de]
> > > Sent: Wednesday, August 26, 2015 12:26 PM
> > > To: Ranjit Abhimanyu Waghmode
> > > Cc: dw...@infradead.org; computersforpe...@gmail.com;
> > > broo...@kernel.org; Michal Simek; Soren Brinkmann; zaj...@gmail.com;
> > > b...@decadent.org.uk; b32...@freescale.com;
> > > knut.wohl...@de.bosch.com; juh...@openwrt.org;
> bean...@micron.com;
> > > linux-...@lists.infradead.org; linux-kernel@vger.kernel.org;
> > > linux-...@vger.kernel.org; linux-arm- ker...@lists.infradead.org;
> > > Harini Katakam; Punnaiah Choudary Kalluri
> > > Subject: Re: [LINUX RFC v2 0/4] spi: add dual parallel mode support
> > > in Zynq MPSoC GQSPI controller
> > >
> > > On Wednesday, August 26, 2015 at 08:26:03 AM, Ranjit Waghmode wrote:
> > > > This series adds dual parallel mode support for Zynq Ultrascale+
> > > > MPSoC GQSPI controller driver.
> > > >
> > > > What is dual parallel mode?
> > > > ---
> > > > ZynqMP GQSPI controller supports Dual Parallel mode with following
> > > > functionalities: 1) Supporting two SPI flash memories operating in
> > > > parallel. 8 I/O lines. 2) Chip selects and clock are shared to
> > > > both the flash devices
> > > > 3) This mode is targeted for faster read/write speed and also
> > > > doubles the size 4) Commands/data can be transmitted/received from
> > > > both the devices(mirror), or only upper or only lower flash memory
> devices.
> > > >
> > > > 5) Data arrangement:
> > > >With stripe enabled,
> > > >Even bytes i.e. 0, 2, 4,... are transmitted on Lower Data Bus
> > > >Odd bytes i.e. 1, 3, 5,.. are transmitted on Upper Data Bus.
> > >
> > > This might be a dumb question, but why don't you just treat this as
> > > an SPI NOR flash with 8-bit bus ?
> >
> > In case of dual parallel configuration of this controller there are
> > different modes like single, dual and quad mode. Whatever you are
> > suggesting would fit only in the case of Quad mode operation as both
> > buses would have 4 lines each. In case of single mode of parallel
> > configuration, there would be two buses; but the line on each bus
> > would one. So altogether there will be two lines. And in case of dual
> > mode of parallel configuration each bus will be having two lines. So
> > altogether 4 lines will be there. So keeping 8 lines would not support
> > above two modes of parallel configuration correctly.
> >
> > Logically it is a single flash with 8 IO lines but physically it's a
> > two flash devices and each has 4 IO lines. So, in this case, read and
> > write addresses should be always even and minimum data that can be
> > accessed is 2 bytes.
> 
> Oh, I see what the issue is now. It has to do with configuring the flash into
> correct bus-width mode, right ?
Yes.
> 
> Best regards,
> Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [LINUX RFC v2 0/4] spi: add dual parallel mode support in Zynq MPSoC GQSPI controller

2015-09-02 Thread Ranjit Abhimanyu Waghmode
Hi Marek,

> -Original Message-
> From: Marek Vasut [mailto:ma...@denx.de]
> Sent: Wednesday, August 26, 2015 12:26 PM
> To: Ranjit Abhimanyu Waghmode
> Cc: dw...@infradead.org; computersforpe...@gmail.com;
> broo...@kernel.org; Michal Simek; Soren Brinkmann; zaj...@gmail.com;
> b...@decadent.org.uk; b32...@freescale.com; knut.wohl...@de.bosch.com;
> juh...@openwrt.org; bean...@micron.com; linux-...@lists.infradead.org;
> linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; linux-arm-
> ker...@lists.infradead.org; Harini Katakam; Punnaiah Choudary Kalluri
> Subject: Re: [LINUX RFC v2 0/4] spi: add dual parallel mode support in Zynq
> MPSoC GQSPI controller
> 
> On Wednesday, August 26, 2015 at 08:26:03 AM, Ranjit Waghmode wrote:
> > This series adds dual parallel mode support for Zynq Ultrascale+ MPSoC
> > GQSPI controller driver.
> >
> > What is dual parallel mode?
> > ---
> > ZynqMP GQSPI controller supports Dual Parallel mode with following
> > functionalities: 1) Supporting two SPI flash memories operating in
> > parallel. 8 I/O lines. 2) Chip selects and clock are shared to both
> > the flash devices
> > 3) This mode is targeted for faster read/write speed and also doubles
> > the size 4) Commands/data can be transmitted/received from both the
> > devices(mirror), or only upper or only lower flash memory devices.
> > 5) Data arrangement:
> >With stripe enabled,
> >Even bytes i.e. 0, 2, 4,... are transmitted on Lower Data Bus
> >Odd bytes i.e. 1, 3, 5,.. are transmitted on Upper Data Bus.
> 
> This might be a dumb question, but why don't you just treat this as an SPI NOR
> flash with 8-bit bus ?

In case of dual parallel configuration of this controller there are different 
modes like single, dual and quad mode.
Whatever you are suggesting would fit only in the case of Quad mode operation 
as both buses would have 4 lines each.
In case of single mode of parallel configuration, there would be two buses; but 
the line on each bus would one. So altogether there will be two lines.
And in case of dual mode of parallel configuration each bus will be having two 
lines. So altogether 4 lines will be there.
So keeping 8 lines would not support above two modes of parallel configuration 
correctly.

Logically it is a single flash with 8 IO lines but physically it's a two flash 
devices and each has 4 IO lines. 
So, in this case, read and write addresses should be always even and minimum 
data that can be accessed is 2 bytes.

Regards,
Ranjit

> 
> Best regards,
> Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [LINUX RFC v2 0/4] spi: add dual parallel mode support in Zynq MPSoC GQSPI controller

2015-09-02 Thread Ranjit Abhimanyu Waghmode
Hi Marek,

> -Original Message-
> From: Marek Vasut [mailto:ma...@denx.de]
> Sent: Wednesday, August 26, 2015 12:26 PM
> To: Ranjit Abhimanyu Waghmode
> Cc: dw...@infradead.org; computersforpe...@gmail.com;
> broo...@kernel.org; Michal Simek; Soren Brinkmann; zaj...@gmail.com;
> b...@decadent.org.uk; b32...@freescale.com; knut.wohl...@de.bosch.com;
> juh...@openwrt.org; bean...@micron.com; linux-...@lists.infradead.org;
> linux-kernel@vger.kernel.org; linux-...@vger.kernel.org; linux-arm-
> ker...@lists.infradead.org; Harini Katakam; Punnaiah Choudary Kalluri
> Subject: Re: [LINUX RFC v2 0/4] spi: add dual parallel mode support in Zynq
> MPSoC GQSPI controller
> 
> On Wednesday, August 26, 2015 at 08:26:03 AM, Ranjit Waghmode wrote:
> > This series adds dual parallel mode support for Zynq Ultrascale+ MPSoC
> > GQSPI controller driver.
> >
> > What is dual parallel mode?
> > ---
> > ZynqMP GQSPI controller supports Dual Parallel mode with following
> > functionalities: 1) Supporting two SPI flash memories operating in
> > parallel. 8 I/O lines. 2) Chip selects and clock are shared to both
> > the flash devices
> > 3) This mode is targeted for faster read/write speed and also doubles
> > the size 4) Commands/data can be transmitted/received from both the
> > devices(mirror), or only upper or only lower flash memory devices.
> > 5) Data arrangement:
> >With stripe enabled,
> >Even bytes i.e. 0, 2, 4,... are transmitted on Lower Data Bus
> >Odd bytes i.e. 1, 3, 5,.. are transmitted on Upper Data Bus.
> 
> This might be a dumb question, but why don't you just treat this as an SPI NOR
> flash with 8-bit bus ?

In case of dual parallel configuration of this controller there are different 
modes like single, dual and quad mode.
Whatever you are suggesting would fit only in the case of Quad mode operation 
as both buses would have 4 lines each.
In case of single mode of parallel configuration, there would be two buses; but 
the line on each bus would one. So altogether there will be two lines.
And in case of dual mode of parallel configuration each bus will be having two 
lines. So altogether 4 lines will be there.
So keeping 8 lines would not support above two modes of parallel configuration 
correctly.

Logically it is a single flash with 8 IO lines but physically it's a two flash 
devices and each has 4 IO lines. 
So, in this case, read and write addresses should be always even and minimum 
data that can be accessed is 2 bytes.

Regards,
Ranjit

> 
> Best regards,
> Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [LINUX RFC 1/2] mtd: spi-nor: add dual parallel mode support

2015-08-04 Thread Ranjit Abhimanyu Waghmode
Hi Mark,

> -Original Message-
> From: Mark Brown [mailto:broo...@kernel.org]
> Sent: Monday, August 03, 2015 9:38 PM
> To: Ranjit Abhimanyu Waghmode
> Cc: dw...@infradead.org; computersforpe...@gmail.com; Michal Simek;
> Soren Brinkmann; zaj...@gmail.com; b...@decadent.org.uk; ma...@denx.de;
> b32...@freescale.com; knut.wohl...@de.bosch.com; juh...@openwrt.org;
> bean...@micron.com; linux-...@lists.infradead.org; linux-
> ker...@vger.kernel.org; linux-...@vger.kernel.org; linux-arm-
> ker...@lists.infradead.org; Harini Katakam; Punnaiah Choudary Kalluri; Ranjit
> Abhimanyu Waghmode; ran27...@gmail.com
> Subject: Re: [LINUX RFC 1/2] mtd: spi-nor: add dual parallel mode support
> 
> On Mon, Aug 03, 2015 at 02:35:06PM +0530, Ranjit Waghmode wrote:
> 
> >  drivers/mtd/devices/m25p80.c  |  1 +
> >  drivers/mtd/spi-nor/spi-nor.c | 92 ++--
> ---
> >  include/linux/mtd/spi-nor.h   |  3 ++
> >  include/linux/spi/spi.h   |  2 +
> >  4 files changed, 79 insertions(+), 19 deletions(-)
> 
> You need to at least split this into two patches, one adding a new SPI 
> interface
> and another using it in MTD.  Probably the MTD core and driver changes need
> splitting too.  Please see SubmittingPatches for discussion of splitting 
> things.
> 

I will split and resend the same.

> > diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index
> > d673072..8dec349 100644
> > --- a/include/linux/spi/spi.h
> > +++ b/include/linux/spi/spi.h
> > @@ -355,6 +355,8 @@ struct spi_master {
> >  #define SPI_MASTER_NO_TX   BIT(2)  /* can't do buffer write */
> >  #define SPI_MASTER_MUST_RX  BIT(3) /* requires rx */
> >  #define SPI_MASTER_MUST_TX  BIT(4) /* requires tx */
> > +#define SPI_MASTER_DATA_STRIPE BIT(7)  /* support
> data stripe */
> > +#define SPI_MASTER_BOTH_CS BIT(8)  /* enable both
> chips */
> 
> This is really not adequate description for a new API, I can't tell what "data
> stripe" is supposed to mean at all and I've got at best a vague idea what 
> "both
> chips" really means.  This means other developers won't be able to tell how to
> use or implement these flags either, and it means I can't really review this. 
>  You
> need to provide more information here, both in the code and in the commit
> message.
> 

I'm sorry about that. I have added description in cover letter, but will add 
more information about the same here too.

> I'd also expect some handling in the core for these, for example error 
> handling if
> they can't be supported.

Will update and send you the updated version.

Thanks,
Ranjit
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [LINUX RFC 1/2] mtd: spi-nor: add dual parallel mode support

2015-08-04 Thread Ranjit Abhimanyu Waghmode
Hi Mark,

 -Original Message-
 From: Mark Brown [mailto:broo...@kernel.org]
 Sent: Monday, August 03, 2015 9:38 PM
 To: Ranjit Abhimanyu Waghmode
 Cc: dw...@infradead.org; computersforpe...@gmail.com; Michal Simek;
 Soren Brinkmann; zaj...@gmail.com; b...@decadent.org.uk; ma...@denx.de;
 b32...@freescale.com; knut.wohl...@de.bosch.com; juh...@openwrt.org;
 bean...@micron.com; linux-...@lists.infradead.org; linux-
 ker...@vger.kernel.org; linux-...@vger.kernel.org; linux-arm-
 ker...@lists.infradead.org; Harini Katakam; Punnaiah Choudary Kalluri; Ranjit
 Abhimanyu Waghmode; ran27...@gmail.com
 Subject: Re: [LINUX RFC 1/2] mtd: spi-nor: add dual parallel mode support
 
 On Mon, Aug 03, 2015 at 02:35:06PM +0530, Ranjit Waghmode wrote:
 
   drivers/mtd/devices/m25p80.c  |  1 +
   drivers/mtd/spi-nor/spi-nor.c | 92 ++--
 ---
   include/linux/mtd/spi-nor.h   |  3 ++
   include/linux/spi/spi.h   |  2 +
   4 files changed, 79 insertions(+), 19 deletions(-)
 
 You need to at least split this into two patches, one adding a new SPI 
 interface
 and another using it in MTD.  Probably the MTD core and driver changes need
 splitting too.  Please see SubmittingPatches for discussion of splitting 
 things.
 

I will split and resend the same.

  diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index
  d673072..8dec349 100644
  --- a/include/linux/spi/spi.h
  +++ b/include/linux/spi/spi.h
  @@ -355,6 +355,8 @@ struct spi_master {
   #define SPI_MASTER_NO_TX   BIT(2)  /* can't do buffer write */
   #define SPI_MASTER_MUST_RX  BIT(3) /* requires rx */
   #define SPI_MASTER_MUST_TX  BIT(4) /* requires tx */
  +#define SPI_MASTER_DATA_STRIPE BIT(7)  /* support
 data stripe */
  +#define SPI_MASTER_BOTH_CS BIT(8)  /* enable both
 chips */
 
 This is really not adequate description for a new API, I can't tell what data
 stripe is supposed to mean at all and I've got at best a vague idea what 
 both
 chips really means.  This means other developers won't be able to tell how to
 use or implement these flags either, and it means I can't really review this. 
  You
 need to provide more information here, both in the code and in the commit
 message.
 

I'm sorry about that. I have added description in cover letter, but will add 
more information about the same here too.

 I'd also expect some handling in the core for these, for example error 
 handling if
 they can't be supported.

Will update and send you the updated version.

Thanks,
Ranjit
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [RFC PATCH 0/2] spi: add dual parallel & stacked mode support in Zynq MPSoC GQSPI controller

2015-07-27 Thread Ranjit Abhimanyu Waghmode
Hi Mark,

> -Original Message-
> From: Mark Brown [mailto:broo...@kernel.org]
> Sent: Friday, July 24, 2015 4:22 PM
> To: Ranjit Abhimanyu Waghmode
> Cc: Michal Simek; Soren Brinkmann; zaj...@gmail.com; ma...@denx.de;
> shijie.hu...@intel.com; juh...@openwrt.org; b...@decadent.org.uk; linux-
> m...@lists.infradead.org; linux-...@vger.kernel.org; linux-arm-
> ker...@lists.infradead.org; linux-kernel@vger.kernel.org; Harini Katakam;
> Punnaiah Choudary Kalluri; ran27...@gmail.com; dw...@infradead.org;
> computersforpe...@gmail.com
> Subject: Re: [RFC PATCH 0/2] spi: add dual parallel & stacked mode support in
> Zynq MPSoC GQSPI controller
> 
> On Fri, Jul 24, 2015 at 10:42:35AM +, Ranjit Abhimanyu Waghmode wrote:
> 
> As I think you've been asked before please fix your mail client to word wrap
> within paragraphs so your mails are more legible.
> 

Sorry about this, I did some changes but it's kind of broken. Will fix this.

> > To support the dual parallel mode in this controller, following minor
> > things can be added to the driver.
> 
> > 1) Controller needs to know in which mode it is working, then it's
> > obvious to set the appropriate flag for the same
> > 2) There are more than one chip selects, so need to set the same
> >
> > So kindly suggest your view on the above request.
> 
> I'm not entirely sure what you're asking here from the point of view of SPI, 
> sorry
> - what exactly are you requesting?  If you want to add support for new SPI bus
> modes please go ahead and do that, you need to clearly document what any
> new modes you're adding are so that other people can understand them.

Ok, my description was too short to get it completely.

For adding dual parallel mode support to current driver:
Are following points enough? Or do you want to suggest something better on top 
of it?

Driver:
1) Controller needs to know in which mode it is working.
2) As there are more than one chip selects, may need to add code for handling 
that as well.

MTD:
1) Adding TWO_FLASH support
2) Adding DATA_STRIPE support
3) For reading array size needs to be doubled.
4) Need to access even addresses. Basically address/2.

Please suggest your view on above points.

Regards,
Ranjit
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [RFC PATCH 0/2] spi: add dual parallel stacked mode support in Zynq MPSoC GQSPI controller

2015-07-27 Thread Ranjit Abhimanyu Waghmode
Hi Mark,

 -Original Message-
 From: Mark Brown [mailto:broo...@kernel.org]
 Sent: Friday, July 24, 2015 4:22 PM
 To: Ranjit Abhimanyu Waghmode
 Cc: Michal Simek; Soren Brinkmann; zaj...@gmail.com; ma...@denx.de;
 shijie.hu...@intel.com; juh...@openwrt.org; b...@decadent.org.uk; linux-
 m...@lists.infradead.org; linux-...@vger.kernel.org; linux-arm-
 ker...@lists.infradead.org; linux-kernel@vger.kernel.org; Harini Katakam;
 Punnaiah Choudary Kalluri; ran27...@gmail.com; dw...@infradead.org;
 computersforpe...@gmail.com
 Subject: Re: [RFC PATCH 0/2] spi: add dual parallel  stacked mode support in
 Zynq MPSoC GQSPI controller
 
 On Fri, Jul 24, 2015 at 10:42:35AM +, Ranjit Abhimanyu Waghmode wrote:
 
 As I think you've been asked before please fix your mail client to word wrap
 within paragraphs so your mails are more legible.
 

Sorry about this, I did some changes but it's kind of broken. Will fix this.

  To support the dual parallel mode in this controller, following minor
  things can be added to the driver.
 
  1) Controller needs to know in which mode it is working, then it's
  obvious to set the appropriate flag for the same
  2) There are more than one chip selects, so need to set the same
 
  So kindly suggest your view on the above request.
 
 I'm not entirely sure what you're asking here from the point of view of SPI, 
 sorry
 - what exactly are you requesting?  If you want to add support for new SPI bus
 modes please go ahead and do that, you need to clearly document what any
 new modes you're adding are so that other people can understand them.

Ok, my description was too short to get it completely.

For adding dual parallel mode support to current driver:
Are following points enough? Or do you want to suggest something better on top 
of it?

Driver:
1) Controller needs to know in which mode it is working.
2) As there are more than one chip selects, may need to add code for handling 
that as well.

MTD:
1) Adding TWO_FLASH support
2) Adding DATA_STRIPE support
3) For reading array size needs to be doubled.
4) Need to access even addresses. Basically address/2.

Please suggest your view on above points.

Regards,
Ranjit
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [RFC PATCH 0/2] spi: add dual parallel & stacked mode support in Zynq MPSoC GQSPI controller

2015-07-24 Thread Ranjit Abhimanyu Waghmode
Hi Mark,

> > > For an example take two flashes connected in stacked mode.
> > > For user it doesn't matter whether how many flashes are really connected.
> > > There will be situation like, single partition is spread across two
> > > flashes
> > (partition staring at the end of one flash and continued to the second
> > flash). But it has to be shown contiguous to user.
> > > In this scenario, I am not clear how MTD layer will handle the case.
> > > It would be great if you could just put some light on it.
> >
> > That's something for the MTD layer or possibly even a layer above it
> > to worry about - this situation is the same as we have with disks
> > where we have md which combines other devices, if something similar is
> > needed for flash we should use a similar pattern.

To support the dual parallel mode in this controller, following minor things 
can be added to the driver.
1) Controller needs to know in which mode it is working, then it's obvious to 
set the appropriate flag for the same
2) There are more than one chip selects, so need to set the same

And for supporting the same dual parallel mode, MTD layer may need to update 
for:
1) Adding TWO_FLASH support
2) Adding DATA_STRIPE support
3) For reading array size needs to be doubled.
4) Need to access even addresses. Basically address/2.

So kindly suggest your view on the above request.

Regards,
Ranjit

> 
> Kindly help in understanding, how can we represent the stacked mode and
> parallel mode changes in MTD layer?
> 
> Thanks & Regards,
> Ranjit
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [RFC PATCH 0/2] spi: add dual parallel stacked mode support in Zynq MPSoC GQSPI controller

2015-07-24 Thread Ranjit Abhimanyu Waghmode
Hi Mark,

   For an example take two flashes connected in stacked mode.
   For user it doesn't matter whether how many flashes are really connected.
   There will be situation like, single partition is spread across two
   flashes
  (partition staring at the end of one flash and continued to the second
  flash). But it has to be shown contiguous to user.
   In this scenario, I am not clear how MTD layer will handle the case.
   It would be great if you could just put some light on it.
 
  That's something for the MTD layer or possibly even a layer above it
  to worry about - this situation is the same as we have with disks
  where we have md which combines other devices, if something similar is
  needed for flash we should use a similar pattern.

To support the dual parallel mode in this controller, following minor things 
can be added to the driver.
1) Controller needs to know in which mode it is working, then it's obvious to 
set the appropriate flag for the same
2) There are more than one chip selects, so need to set the same

And for supporting the same dual parallel mode, MTD layer may need to update 
for:
1) Adding TWO_FLASH support
2) Adding DATA_STRIPE support
3) For reading array size needs to be doubled.
4) Need to access even addresses. Basically address/2.

So kindly suggest your view on the above request.

Regards,
Ranjit

 
 Kindly help in understanding, how can we represent the stacked mode and
 parallel mode changes in MTD layer?
 
 Thanks  Regards,
 Ranjit
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [RFC PATCH 0/2] spi: add dual parallel & stacked mode support in Zynq MPSoC GQSPI controller

2015-07-17 Thread Ranjit Abhimanyu Waghmode
Hi,

> -Original Message-
> From: Mark Brown [mailto:broo...@kernel.org]
> Sent: Thursday, July 16, 2015 2:28 PM
> To: Ranjit Abhimanyu Waghmode
> Cc: Michal Simek; Soren Brinkmann; dw...@infradead.org;
> computersforpe...@gmail.com; zaj...@gmail.com; ma...@denx.de;
> shijie.hu...@intel.com; juh...@openwrt.org; b...@decadent.org.uk; linux-
> m...@lists.infradead.org; linux-...@vger.kernel.org; linux-arm-
> ker...@lists.infradead.org; linux-kernel@vger.kernel.org; Harini Katakam;
> Punnaiah Choudary Kalluri; ran27...@gmail.com
> Subject: Re: [RFC PATCH 0/2] spi: add dual parallel & stacked mode support in
> Zynq MPSoC GQSPI controller
>
> On Thu, Jul 16, 2015 at 07:27:34AM +, Ranjit Abhimanyu Waghmode wrote:
>
> > For an example take two flashes connected in stacked mode.
> > For user it doesn't matter whether how many flashes are really connected.
> > There will be situation like, single partition is spread across two flashes
> (partition staring at the end of one flash and continued to the second 
> flash). But
> it has to be shown contiguous to user.
> > In this scenario, I am not clear how MTD layer will handle the case.
> > It would be great if you could just put some light on it.
>
> That's something for the MTD layer or possibly even a layer above it to worry
> about - this situation is the same as we have with disks where we have md 
> which
> combines other devices, if something similar is needed for flash we should 
> use a
> similar pattern.

Kindly help in understanding, how can we represent the stacked mode and 
parallel mode changes in MTD layer?

Thanks & Regards,
Ranjit


This email and any attachments are intended for the sole use of the named 
recipient(s) and contain(s) confidential information that may be proprietary, 
privileged or copyrighted under applicable law. If you are not the intended 
recipient, do not read, copy, or forward this email message or any attachments. 
Delete this email message and any attachments immediately.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [RFC PATCH 0/2] spi: add dual parallel stacked mode support in Zynq MPSoC GQSPI controller

2015-07-17 Thread Ranjit Abhimanyu Waghmode
Hi,

 -Original Message-
 From: Mark Brown [mailto:broo...@kernel.org]
 Sent: Thursday, July 16, 2015 2:28 PM
 To: Ranjit Abhimanyu Waghmode
 Cc: Michal Simek; Soren Brinkmann; dw...@infradead.org;
 computersforpe...@gmail.com; zaj...@gmail.com; ma...@denx.de;
 shijie.hu...@intel.com; juh...@openwrt.org; b...@decadent.org.uk; linux-
 m...@lists.infradead.org; linux-...@vger.kernel.org; linux-arm-
 ker...@lists.infradead.org; linux-kernel@vger.kernel.org; Harini Katakam;
 Punnaiah Choudary Kalluri; ran27...@gmail.com
 Subject: Re: [RFC PATCH 0/2] spi: add dual parallel  stacked mode support in
 Zynq MPSoC GQSPI controller

 On Thu, Jul 16, 2015 at 07:27:34AM +, Ranjit Abhimanyu Waghmode wrote:

  For an example take two flashes connected in stacked mode.
  For user it doesn't matter whether how many flashes are really connected.
  There will be situation like, single partition is spread across two flashes
 (partition staring at the end of one flash and continued to the second 
 flash). But
 it has to be shown contiguous to user.
  In this scenario, I am not clear how MTD layer will handle the case.
  It would be great if you could just put some light on it.

 That's something for the MTD layer or possibly even a layer above it to worry
 about - this situation is the same as we have with disks where we have md 
 which
 combines other devices, if something similar is needed for flash we should 
 use a
 similar pattern.

Kindly help in understanding, how can we represent the stacked mode and 
parallel mode changes in MTD layer?

Thanks  Regards,
Ranjit


This email and any attachments are intended for the sole use of the named 
recipient(s) and contain(s) confidential information that may be proprietary, 
privileged or copyrighted under applicable law. If you are not the intended 
recipient, do not read, copy, or forward this email message or any attachments. 
Delete this email message and any attachments immediately.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [RFC PATCH 0/2] spi: add dual parallel & stacked mode support in Zynq MPSoC GQSPI controller

2015-07-16 Thread Ranjit Abhimanyu Waghmode
Hi Mark,

> > > > What is stacked mode?
> > > > -
> > > > ZynqMP GQSPI controller supports stacked mode with following
> > > functionalities:
> > > > 1) The Generic Quad-SPI controller also supports two SPI flash memories
> > > >in a shared bus arrangement to reduce IO pin count.
> > > > 2) Separate chip select lines
> > > > 3) Shared I/O lines
> > > > 4) This mode is targeted for increasing the flash memory and no
> performance
> > > >improvement when compared with single.
> 
> > > This is just a normal SPI controller from a SPI point of view.
> 
> > How can we really represent the stacked mode in current configuration?
> 
> In the same way as any other controller with two chip selects...  there are 
> quite
> a few other drivers that provide examples of this, you should look for one 
> that
> has hardware control similar to yours.

Thanks Mark for your suggestion. But I have minor doubts.

For an example take two flashes connected in stacked mode.
For user it doesn't matter whether how many flashes are really connected. 
There will be situation like, single partition is spread across two flashes 
(partition staring at the end of one flash and continued to the second flash). 
But it has to be shown contiguous to user.
In this scenario, I am not clear how MTD layer will handle the case.
It would be great if you could just put some light on it.

Regards,
Ranjit
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [RFC PATCH 0/2] spi: add dual parallel stacked mode support in Zynq MPSoC GQSPI controller

2015-07-16 Thread Ranjit Abhimanyu Waghmode
Hi Mark,

What is stacked mode?
-
ZynqMP GQSPI controller supports stacked mode with following
   functionalities:
1) The Generic Quad-SPI controller also supports two SPI flash memories
   in a shared bus arrangement to reduce IO pin count.
2) Separate chip select lines
3) Shared I/O lines
4) This mode is targeted for increasing the flash memory and no
 performance
   improvement when compared with single.
 
   This is just a normal SPI controller from a SPI point of view.
 
  How can we really represent the stacked mode in current configuration?
 
 In the same way as any other controller with two chip selects...  there are 
 quite
 a few other drivers that provide examples of this, you should look for one 
 that
 has hardware control similar to yours.

Thanks Mark for your suggestion. But I have minor doubts.

For an example take two flashes connected in stacked mode.
For user it doesn't matter whether how many flashes are really connected. 
There will be situation like, single partition is spread across two flashes 
(partition staring at the end of one flash and continued to the second flash). 
But it has to be shown contiguous to user.
In this scenario, I am not clear how MTD layer will handle the case.
It would be great if you could just put some light on it.

Regards,
Ranjit
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [RFC PATCH 0/2] spi: add dual parallel & stacked mode support in Zynq MPSoC GQSPI controller

2015-07-15 Thread Ranjit Abhimanyu Waghmode
Hi Mark,

> > What is dual parallel mode?
> > ---
> > ZynqMP GQSPI controller supports Dual Parallel mode with following
> functionalities:
> > 1) Supporting two SPI flash memories operating in parallel. 8 I/O lines.
> > 2) Chip selects and clock are shared to both the flash devices
> > 3) This mode is targeted for faster read/write speed and also doubles the 
> > size
> > 4) Commands/data can be transmitted/received from both the devices(mirror),
> >or only upper or only lower flash memory devices.
> > 5) Data arrangement:
> >With stripe enabled,
> >Even bytes i.e. 0, 2, 4,... are transmitted on Lower Data Bus
> >Odd bytes i.e. 1, 3, 5,.. are transmitted on Upper Data Bus.
> 
> For the SPI code this just seems like SPI with an 8 bit data width.
> 
> > What is stacked mode?
> > -
> > ZynqMP GQSPI controller supports stacked mode with following
> functionalities:
> > 1) The Generic Quad-SPI controller also supports two SPI flash memories
> >in a shared bus arrangement to reduce IO pin count.
> > 2) Separate chip select lines
> > 3) Shared I/O lines
> > 4) This mode is targeted for increasing the flash memory and no performance
> >improvement when compared with single.
> 
> This is just a normal SPI controller from a SPI point of view.

How can we really represent the stacked mode in current configuration?

Thanks & Regards,
Ranjit
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [RFC PATCH 0/2] spi: add dual parallel stacked mode support in Zynq MPSoC GQSPI controller

2015-07-15 Thread Ranjit Abhimanyu Waghmode
Hi Mark,

  What is dual parallel mode?
  ---
  ZynqMP GQSPI controller supports Dual Parallel mode with following
 functionalities:
  1) Supporting two SPI flash memories operating in parallel. 8 I/O lines.
  2) Chip selects and clock are shared to both the flash devices
  3) This mode is targeted for faster read/write speed and also doubles the 
  size
  4) Commands/data can be transmitted/received from both the devices(mirror),
 or only upper or only lower flash memory devices.
  5) Data arrangement:
 With stripe enabled,
 Even bytes i.e. 0, 2, 4,... are transmitted on Lower Data Bus
 Odd bytes i.e. 1, 3, 5,.. are transmitted on Upper Data Bus.
 
 For the SPI code this just seems like SPI with an 8 bit data width.
 
  What is stacked mode?
  -
  ZynqMP GQSPI controller supports stacked mode with following
 functionalities:
  1) The Generic Quad-SPI controller also supports two SPI flash memories
 in a shared bus arrangement to reduce IO pin count.
  2) Separate chip select lines
  3) Shared I/O lines
  4) This mode is targeted for increasing the flash memory and no performance
 improvement when compared with single.
 
 This is just a normal SPI controller from a SPI point of view.

How can we really represent the stacked mode in current configuration?

Thanks  Regards,
Ranjit
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH] spi: SPI_ZYNQMP_GQSPI should depend on HAS_DMA

2015-06-29 Thread Ranjit Abhimanyu Waghmode
Acked-by: Ranjit Waghmode 

> -Original Message-
> From: Geert Uytterhoeven [mailto:ge...@linux-m68k.org]
> Sent: Friday, June 26, 2015 5:37 PM
> To: Mark Brown; Ranjit Abhimanyu Waghmode
> Cc: linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; Geert
> Uytterhoeven
> Subject: [PATCH] spi: SPI_ZYNQMP_GQSPI should depend on HAS_DMA
>
> If NO_DMA=y:
>
> ERROR: "dma_unmap_single" [drivers/spi/spi-zynqmp-gqspi.ko] undefined!
> ERROR: "dma_mapping_error" [drivers/spi/spi-zynqmp-gqspi.ko] undefined!
> ERROR: "dma_map_single" [drivers/spi/spi-zynqmp-gqspi.ko] undefined!
>
> Add a dependency on HAS_DMA to fix this.
>
> Signed-off-by: Geert Uytterhoeven 
> ---
>  drivers/spi/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index
> 1c0b261b66fcfdcb..b787b515bd73322b 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -618,7 +618,7 @@ config SPI_XTENSA_XTFPGA
>
>  config SPI_ZYNQMP_GQSPI
>   tristate "Xilinx ZynqMP GQSPI controller"
> - depends on SPI_MASTER
> + depends on SPI_MASTER && HAS_DMA
>   help
> Enables Xilinx GQSPI controller driver for Zynq UltraScale+ MPSoC.
>
> --
> 1.9.1



This email and any attachments are intended for the sole use of the named 
recipient(s) and contain(s) confidential information that may be proprietary, 
privileged or copyrighted under applicable law. If you are not the intended 
recipient, do not read, copy, or forward this email message or any attachments. 
Delete this email message and any attachments immediately.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH] spi: SPI_ZYNQMP_GQSPI should depend on HAS_DMA

2015-06-29 Thread Ranjit Abhimanyu Waghmode
Acked-by: Ranjit Waghmode ranj...@xilinx.com

 -Original Message-
 From: Geert Uytterhoeven [mailto:ge...@linux-m68k.org]
 Sent: Friday, June 26, 2015 5:37 PM
 To: Mark Brown; Ranjit Abhimanyu Waghmode
 Cc: linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; Geert
 Uytterhoeven
 Subject: [PATCH] spi: SPI_ZYNQMP_GQSPI should depend on HAS_DMA

 If NO_DMA=y:

 ERROR: dma_unmap_single [drivers/spi/spi-zynqmp-gqspi.ko] undefined!
 ERROR: dma_mapping_error [drivers/spi/spi-zynqmp-gqspi.ko] undefined!
 ERROR: dma_map_single [drivers/spi/spi-zynqmp-gqspi.ko] undefined!

 Add a dependency on HAS_DMA to fix this.

 Signed-off-by: Geert Uytterhoeven ge...@linux-m68k.org
 ---
  drivers/spi/Kconfig | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index
 1c0b261b66fcfdcb..b787b515bd73322b 100644
 --- a/drivers/spi/Kconfig
 +++ b/drivers/spi/Kconfig
 @@ -618,7 +618,7 @@ config SPI_XTENSA_XTFPGA

  config SPI_ZYNQMP_GQSPI
   tristate Xilinx ZynqMP GQSPI controller
 - depends on SPI_MASTER
 + depends on SPI_MASTER  HAS_DMA
   help
 Enables Xilinx GQSPI controller driver for Zynq UltraScale+ MPSoC.

 --
 1.9.1



This email and any attachments are intended for the sole use of the named 
recipient(s) and contain(s) confidential information that may be proprietary, 
privileged or copyrighted under applicable law. If you are not the intended 
recipient, do not read, copy, or forward this email message or any attachments. 
Delete this email message and any attachments immediately.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [LINUX RFC V2 1/2] devicetree: Add DT bindings documentation for Zynq Ultrascale+ MPSoC GQSPI controller

2015-06-08 Thread Ranjit Abhimanyu Waghmode
Hi Soren,

> >  .../devicetree/bindings/spi/spi-zynqmp-qspi.txt| 26
> ++
> >  1 file changed, 26 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.txt
> >
> > diff --git a/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.txt
> > b/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.txt
> > new file mode 100644
> > index 000..cec6330
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.txt
> > @@ -0,0 +1,26 @@
> > +Xilinx Zynq UltraScale+ MPSoC GQSPI controller Device Tree Bindings
> > +---
> > +
> > +Required properties:
> > +- compatible   : Should be "xlnx,zynqmp-qspi-1.0".
> > +- reg  : Physical base address and size of GQSPI 
> > registers map.
> > +- interrupts   : Property with a value describing the interrupt
> > + number.
> > +- interrupt-parent : Must be core interrupt controller.
> > +- clock-names  : List of input clock names - "ref_clk", "pclk"
> > + (See clock bindings for details).
> > +- clocks   : Clock phandles (see clock bindings for details).
> > +
> > +Optional properties:
> > +- num-cs   : Number of chip selects used.
> > +
> > +Example:
> > +   qspi: spi@ff0f {
> > +   compatible = "xlnx,zynqmp-qspi-1.0";
> > +   clock-names = "ref_clk", "pclk";
> > +   clocks = <_clk _clk>;
> > +   interrupts = <0 15 4>;
> > +   interrupt-parent = <>;
> > +   num-cs = <1>;
> > +   reg = <0x0 0xff0f 0x1000 0x0 0xc000 0x800>;
>
> Please make this
>   reg = <0x0 0xff0f 0x1000>, <0x0 0xc000 0x800>;
>

Sorry for this miss. Will update in next version.

Regards,
Ranjit Waghmode,
ranj...@xilinx.com


This email and any attachments are intended for the sole use of the named 
recipient(s) and contain(s) confidential information that may be proprietary, 
privileged or copyrighted under applicable law. If you are not the intended 
recipient, do not read, copy, or forward this email message or any attachments. 
Delete this email message and any attachments immediately.



RE: [LINUX RFC V2 2/2] spi: Add support for Zynq Ultrascale+ MPSoC GQSPI controller

2015-06-08 Thread Ranjit Abhimanyu Waghmode
T_GEN_FIFO_MASK);
> > +
> > +   if (xqspi->txbuf != NULL)
> > +   /* Enable interrupts for TX */
> > +   zynqmp_gqspi_write(xqspi, GQSPI_IER_OFST,
> > +  GQSPI_IER_TXEMPTY_MASK |
> > +   GQSPI_IER_GENFIFOEMPTY_MASK |
> > +   GQSPI_IER_TXNOT_FULL_MASK);
> > +
> > +   if (xqspi->rxbuf != NULL) {
> > +   /* Enable interrupts for RX */
> > +   if (xqspi->mode == GQSPI_MODE_DMA) {
> > +   /* Enable DMA interrupts */
> > +   zynqmp_gqspi_write(xqspi,
> > +   GQSPI_QSPIDMA_DST_I_EN_OFST,
> > +   GQSPI_QSPIDMA_DST_I_EN_DONE_MASK);
> > +   } else {
> > +   zynqmp_gqspi_write(xqspi, GQSPI_IER_OFST,
> > +   GQSPI_IER_GENFIFOEMPTY_MASK |
> > +   GQSPI_IER_RXNEMPTY_MASK |
> > +   GQSPI_IER_RXEMPTY_MASK);
> > +   }
> > +   }
> > +
> > +   return transfer->len;
> > +}
> > +
> > +/**
> > + * zynqmp_qspi_suspend:Suspend method for the QSPI driver
> > + * @_dev:  Address of the platform_device structure
> > + *
> > + * This function stops the QSPI driver queue and disables the QSPI
> > +controller
> > + *
> > + * Return: Always 0
> > + */
> > +static int __maybe_unused zynqmp_qspi_suspend(struct device *dev) {
> > +   struct platform_device *pdev = container_of(dev,
> > +   struct platform_device,
> > +   dev);
> > +   struct spi_master *master = platform_get_drvdata(pdev);
> > +
> > +   spi_master_suspend(master);
> > +
> > +   zynqmp_unprepare_transfer_hardware(master);
> > +
> > +   return 0;
> > +}
> > +
> > +/**
> > + * zynqmp_qspi_resume: Resume method for the QSPI driver
> > + * @dev:   Address of the platform_device structure
> > + *
> > + * The function starts the QSPI driver queue and initializes the QSPI
> > + * controller
> > + *
> > + * Return: 0 on success and error value on error
> > + */
> > +static int __maybe_unused zynqmp_qspi_resume(struct device *dev) {
> > +   struct platform_device *pdev = container_of(dev,
> > +   struct platform_device,
> > +   dev);
> > +   struct spi_master *master = platform_get_drvdata(pdev);
> > +   struct zynqmp_qspi *xqspi = spi_master_get_devdata(master);
> > +   int ret = 0;
>
> is the initialisation required
I will update this.
>
> > +
> > +   ret = clk_enable(xqspi->pclk);
> > +   if (ret) {
> > +   dev_err(dev, "Cannot enable APB clock.\n");
> > +   return ret;
> > +   }
> > +
> > +   ret = clk_enable(xqspi->refclk);
> > +   if (ret) {
> > +   dev_err(dev, "Cannot enable device clock.\n");
> > +   clk_disable(xqspi->pclk);
> > +   return ret;
> > +   }
> > +
> > +   spi_master_resume(master);
> > +
> > +   return 0;
> > +}
> > +
> > +static SIMPLE_DEV_PM_OPS(zynqmp_qspi_dev_pm_ops,
> zynqmp_qspi_suspend,
> > +zynqmp_qspi_resume);
> > +
> > +/**
> > + * zynqmp_qspi_probe:  Probe method for the QSPI driver
> > + * @pdev:  Pointer to the platform_device structure
> > + *
> > + * This function initializes the driver data structures and the hardware.
> > + *
> > + * Return: 0 on success and error value on failure
> > + */
> > +static int zynqmp_qspi_probe(struct platform_device *pdev) {
> > +   int ret = 0;
> > +   struct spi_master *master;
> > +   struct zynqmp_qspi *xqspi;
> > +   struct resource *res;
> > +   struct device *dev = >dev;
> > +
> > +   master = spi_alloc_master(>dev, sizeof(*xqspi));
> > +   if (!master)
> > +   return -ENOMEM;
> > +
> > +   xqspi = spi_master_get_devdata(master);
> > +   master->dev.of_node = pdev->dev.of_node;
> > +   platform_set_drvdata(pdev, master);
> > +
> > +   res = platform_get_resource(pdev, I

RE: [LINUX RFC V2 2/2] spi: Add support for Zynq Ultrascale+ MPSoC GQSPI controller

2015-06-08 Thread Ranjit Abhimanyu Waghmode
);
  +   ret = PTR_ERR(xqspi-refclk);
  +   goto remove_master;
  +   }
  +
  +   ret = clk_prepare_enable(xqspi-pclk);
  +   if (ret) {
  +   dev_err(dev, Unable to enable APB clock.\n);
  +   goto remove_master;
  +   }
  +
  +   ret = clk_prepare_enable(xqspi-refclk);
  +   if (ret) {
  +   dev_err(dev, Unable to enable device clock.\n);
  +   goto clk_dis_pclk;
  +   }
  +
  +   /* QSPI controller initializations */
  +   zynqmp_qspi_init_hw(xqspi);
  +
  +   xqspi-irq = platform_get_irq(pdev, 0);
  +   if (xqspi-irq = 0) {
  +   ret = -ENXIO;
  +   dev_err(dev, irq resource not found\n);
  +   goto remove_master;
  +   }
  +   ret = devm_request_irq(pdev-dev, xqspi-irq, zynqmp_qspi_irq,
  +  0, pdev-name, master);
  +   if (ret != 0) {
  +   ret = -ENXIO;
  +   dev_err(dev, request_irq failed\n);
  +   goto remove_master;
  +   }

 Shouldnt the clk disable be called here.
Will update this too.


  +   if (master-dev.parent == NULL)
  +   master-dev.parent = master-dev;
  +
  +   ret = spi_register_master(master);
  +   if (ret)
  +   goto clk_dis_all;
  +
  +   return 0;
  +
  +clk_dis_all:
  +   clk_disable_unprepare(xqspi-refclk);
  +clk_dis_pclk:
  +   clk_disable_unprepare(xqspi-pclk);
  +remove_master:
  +   spi_master_put(master);
  +
  +   return ret;
  +}
  +
  +/**
  + * zynqmp_qspi_remove: Remove method for the QSPI driver
  + * @pdev:  Pointer to the platform_device structure
  + *
  + * This function is called if a device is physically removed from the
  +system or
  + * if the driver module is being unloaded. It frees all resources
  +allocated to
  + * the device.
  + *
  + * Return: 0 Always
  + */
  +static int zynqmp_qspi_remove(struct platform_device *pdev) {
  +   struct spi_master *master = platform_get_drvdata(pdev);
  +   struct zynqmp_qspi *xqspi = spi_master_get_devdata(master);
  +
  +   zynqmp_gqspi_write(xqspi, GQSPI_EN_OFST, 0x0);
  +   clk_disable_unprepare(xqspi-refclk);
  +   clk_disable_unprepare(xqspi-pclk);
  +
  +   spi_unregister_master(master);
  +
  +   return 0;
  +}
  +
  +static const struct of_device_id zynqmp_qspi_of_match[] = {
  +   { .compatible = xlnx,zynqmp-qspi-1.0, },
  +   { /* End of table */ }
  +};
  +
  +MODULE_DEVICE_TABLE(of, zynqmp_qspi_of_match);
  +
  +static struct platform_driver zynqmp_qspi_driver = {
  +   .probe = zynqmp_qspi_probe,
  +   .remove = zynqmp_qspi_remove,
  +   .driver = {
  +   .name = zynqmp-qspi,
  +   .of_match_table = zynqmp_qspi_of_match,
  +   .pm = zynqmp_qspi_dev_pm_ops,
  +   },
  +};
  +
  +module_platform_driver(zynqmp_qspi_driver);
  +
  +MODULE_AUTHOR(Xilinx, Inc.);
  +MODULE_DESCRIPTION(Xilinx Zynqmp QSPI driver);
  +MODULE_LICENSE(GPL);
  --
  2.1.2
 

Thanks for your review.

Regards,
Ranjit Abhimanyu Waghmode,
ranjit.waghm...@xilinx.com


This email and any attachments are intended for the sole use of the named 
recipient(s) and contain(s) confidential information that may be proprietary, 
privileged or copyrighted under applicable law. If you are not the intended 
recipient, do not read, copy, or forward this email message or any attachments. 
Delete this email message and any attachments immediately.



RE: [LINUX RFC V2 1/2] devicetree: Add DT bindings documentation for Zynq Ultrascale+ MPSoC GQSPI controller

2015-06-08 Thread Ranjit Abhimanyu Waghmode
Hi Soren,

   .../devicetree/bindings/spi/spi-zynqmp-qspi.txt| 26
 ++
   1 file changed, 26 insertions(+)
   create mode 100644
  Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.txt
 
  diff --git a/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.txt
  b/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.txt
  new file mode 100644
  index 000..cec6330
  --- /dev/null
  +++ b/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.txt
  @@ -0,0 +1,26 @@
  +Xilinx Zynq UltraScale+ MPSoC GQSPI controller Device Tree Bindings
  +---
  +
  +Required properties:
  +- compatible   : Should be xlnx,zynqmp-qspi-1.0.
  +- reg  : Physical base address and size of GQSPI 
  registers map.
  +- interrupts   : Property with a value describing the interrupt
  + number.
  +- interrupt-parent : Must be core interrupt controller.
  +- clock-names  : List of input clock names - ref_clk, pclk
  + (See clock bindings for details).
  +- clocks   : Clock phandles (see clock bindings for details).
  +
  +Optional properties:
  +- num-cs   : Number of chip selects used.
  +
  +Example:
  +   qspi: spi@ff0f {
  +   compatible = xlnx,zynqmp-qspi-1.0;
  +   clock-names = ref_clk, pclk;
  +   clocks = misc_clk misc_clk;
  +   interrupts = 0 15 4;
  +   interrupt-parent = gic;
  +   num-cs = 1;
  +   reg = 0x0 0xff0f 0x1000 0x0 0xc000 0x800;

 Please make this
   reg = 0x0 0xff0f 0x1000, 0x0 0xc000 0x800;


Sorry for this miss. Will update in next version.

Regards,
Ranjit Waghmode,
ranj...@xilinx.com


This email and any attachments are intended for the sole use of the named 
recipient(s) and contain(s) confidential information that may be proprietary, 
privileged or copyrighted under applicable law. If you are not the intended 
recipient, do not read, copy, or forward this email message or any attachments. 
Delete this email message and any attachments immediately.



RE: [RFC PATCH 2/2] spi: Add support for Zynq Ultrascale+ MPSoC GQSPI controller

2015-05-28 Thread Ranjit Abhimanyu Waghmode
Hi Soren,

Sorry for being late for this reply as this thread is moved a step ahead.

On Wed, 2015-05-20 at 12:57PM +0530, Ranjit Waghmode wrote:
> This patch adds support for GQSPI controller driver used by Zynq 
> Ultrascale+ MPSoC
> 
> Signed-off-by: Ranjit Waghmode 
> ---
[...]
> +/**
> + * zynqmp_gqspi_read:For GQSPI controller read operation
> + * @xqspi:   Pointer to the zynqmp_qspi structure
> + * @offset:  Offset from where to read
> + */
> +static u32 zynqmp_gqspi_read(struct zynqmp_qspi *xqspi, u32 offset) {
> + return readl_relaxed(xqspi->regs + offset); }
> +
> +/**
> + * zynqmp_gqspi_write:   For GQSPI controller write operation
> + * @xqspi:   Pointer to the zynqmp_qspi structure
> + * @offset:  Offset where to write
> + * @val: Value to be written
> + */
> +static inline void zynqmp_gqspi_write(struct zynqmp_qspi *xqspi, u32 offset,
> +   u32 val)
> +{
> + writel_relaxed(val, (xqspi->regs + offset)); }

why are you wrapping (readl|writel)_relaxed?

[...]
> +/**
> + * zynqmp_qspi_copy_read_data:   Copy data to RX buffer
> + * @xqspi:   Pointer to the zynqmp_qspi structure
> + * @data:The 32 bit variable where data is stored
> + * @size:Number of bytes to be copied from data to RX buffer
> + */
> +static void zynqmp_qspi_copy_read_data(struct zynqmp_qspi *xqspi,
> +u32 data, u8 size)
> +{
> + memcpy(xqspi->rxbuf, ((u8 *) ), size);

Is this cast really needed? IIRC, memcpy takes (void *). The outer parentheses 
for that argument are not needed.
[Ranjit] : taken care

> + xqspi->rxbuf += size;
> + xqspi->bytes_to_receive -= size;
> +}
> +
> +/**
> + * zynqmp_prepare_transfer_hardware: Prepares hardware for transfer.
> + * @master:  Pointer to the spi_master structure which provides
> + *   information about the controller.
> + *
> + * This function enables SPI master controller.
> + *
> + * Return:   Always 0
> + */
> +static int zynqmp_prepare_transfer_hardware(struct spi_master 
> +*master) {
> + struct zynqmp_qspi *xqspi = spi_master_get_devdata(master);
> +
> + clk_enable(xqspi->refclk);
> + clk_enable(xqspi->pclk);

Did you consider using runtime_pm? Then this would just bit
pm_runtime_get_sync(...)

> + zynqmp_gqspi_write(xqspi, GQSPI_EN_OFST, GQSPI_EN_MASK);
> + return 0;
> +}
> +
> +/**
> + * zynqmp_unprepare_transfer_hardware:   Relaxes hardware after transfer
> + * @master:  Pointer to the spi_master structure which provides
> + *   information about the controller.
> + *
> + * This function disables the SPI master controller.
> + *
> + * Return:   Always 0
> + */
> +static int zynqmp_unprepare_transfer_hardware(struct spi_master 
> +*master) {
> + struct zynqmp_qspi *xqspi = spi_master_get_devdata(master);
> +
> + zynqmp_gqspi_write(xqspi, GQSPI_EN_OFST, 0x0);
> + clk_disable(xqspi->refclk);
> + clk_disable(xqspi->pclk);

and this would become pm_runtime_put()

[...]
> +/**
> + * zynqmp_qspi_filltxfifo:   Fills the TX FIFO as long as there is room in
> + *   the FIFO or the bytes required to be
> + *   transmitted.
> + * @xqspi:   Pointer to the zynqmp_qspi structure
> + * @size:Number of bytes to be copied from TX buffer to TX FIFO
> + */
> +static void zynqmp_qspi_filltxfifo(struct zynqmp_qspi *xqspi, int 
> +size) {
> + u32 count = 0;
> +
> + while ((xqspi->bytes_to_transfer > 0) && (count < size)) {
> + writel(*((u32 *) xqspi->txbuf), xqspi->regs + GQSPI_TXD_OFST);

Here the writel wrapper is not used. Is there a reason, then it would probably  
deserve a comment.

[...]
> +/**
> + * zynqmp_process_dma_irq:   Handler for DMA done interrupt of QSPI
> + *   controller
> + * @xqspi:   zynqmp_qspi instance pointer
> + *
> + * This function handles DMA interrupt only.
> + */
> +static void zynqmp_process_dma_irq(struct zynqmp_qspi *xqspi) {
> + u32 config_reg, genfifoentry;
> +
> + dma_unmap_single(xqspi->dev, xqspi->dma_addr,
> + xqspi->dma_rx_bytes, DMA_FROM_DEVICE);
> + xqspi->rxbuf += xqspi->dma_rx_bytes;
> + xqspi->bytes_to_receive -= xqspi->dma_rx_bytes;
> + xqspi->dma_rx_bytes = 0;
> +
> + /* Disabling the DMA interrupts */
> + writel(GQSPI_QSPIDMA_DST_I_EN_DONE_MASK,
> + xqspi->regs + GQSPI_QSPIDMA_DST_I_DIS_OFST);
> +
> + if (xqspi->bytes_to_receive > 0) {
> + /* Switch to IO mode,for remaining bytes to receive */
> + config_reg = readl(xqspi->regs + GQSPI_CONFIG_OFST);
> + config_reg &= ~GQSPI_CFG_MODE_EN_MASK;
> + writel(config_reg, xqspi->regs + GQSPI_CONFIG_OFST);
> +
> + /* Initiate the transfer of remaining bytes */
> + genfifoentry = xqspi->genfifoentry;
> + genfifoentry |= xqspi->bytes_to_receive;
> + writel(genfifoentry,
> +   

RE: [RFC PATCH 2/2] spi: Add support for Zynq Ultrascale+ MPSoC GQSPI controller

2015-05-28 Thread Ranjit Abhimanyu Waghmode
Hi Soren,

Sorry for being late for this reply as this thread is moved a step ahead.

On Wed, 2015-05-20 at 12:57PM +0530, Ranjit Waghmode wrote:
 This patch adds support for GQSPI controller driver used by Zynq 
 Ultrascale+ MPSoC
 
 Signed-off-by: Ranjit Waghmode ranjit.waghm...@xilinx.com
 ---
[...]
 +/**
 + * zynqmp_gqspi_read:For GQSPI controller read operation
 + * @xqspi:   Pointer to the zynqmp_qspi structure
 + * @offset:  Offset from where to read
 + */
 +static u32 zynqmp_gqspi_read(struct zynqmp_qspi *xqspi, u32 offset) {
 + return readl_relaxed(xqspi-regs + offset); }
 +
 +/**
 + * zynqmp_gqspi_write:   For GQSPI controller write operation
 + * @xqspi:   Pointer to the zynqmp_qspi structure
 + * @offset:  Offset where to write
 + * @val: Value to be written
 + */
 +static inline void zynqmp_gqspi_write(struct zynqmp_qspi *xqspi, u32 offset,
 +   u32 val)
 +{
 + writel_relaxed(val, (xqspi-regs + offset)); }

why are you wrapping (readl|writel)_relaxed?

[...]
 +/**
 + * zynqmp_qspi_copy_read_data:   Copy data to RX buffer
 + * @xqspi:   Pointer to the zynqmp_qspi structure
 + * @data:The 32 bit variable where data is stored
 + * @size:Number of bytes to be copied from data to RX buffer
 + */
 +static void zynqmp_qspi_copy_read_data(struct zynqmp_qspi *xqspi,
 +u32 data, u8 size)
 +{
 + memcpy(xqspi-rxbuf, ((u8 *) data), size);

Is this cast really needed? IIRC, memcpy takes (void *). The outer parentheses 
for that argument are not needed.
[Ranjit] : taken care

 + xqspi-rxbuf += size;
 + xqspi-bytes_to_receive -= size;
 +}
 +
 +/**
 + * zynqmp_prepare_transfer_hardware: Prepares hardware for transfer.
 + * @master:  Pointer to the spi_master structure which provides
 + *   information about the controller.
 + *
 + * This function enables SPI master controller.
 + *
 + * Return:   Always 0
 + */
 +static int zynqmp_prepare_transfer_hardware(struct spi_master 
 +*master) {
 + struct zynqmp_qspi *xqspi = spi_master_get_devdata(master);
 +
 + clk_enable(xqspi-refclk);
 + clk_enable(xqspi-pclk);

Did you consider using runtime_pm? Then this would just bit
pm_runtime_get_sync(...)

 + zynqmp_gqspi_write(xqspi, GQSPI_EN_OFST, GQSPI_EN_MASK);
 + return 0;
 +}
 +
 +/**
 + * zynqmp_unprepare_transfer_hardware:   Relaxes hardware after transfer
 + * @master:  Pointer to the spi_master structure which provides
 + *   information about the controller.
 + *
 + * This function disables the SPI master controller.
 + *
 + * Return:   Always 0
 + */
 +static int zynqmp_unprepare_transfer_hardware(struct spi_master 
 +*master) {
 + struct zynqmp_qspi *xqspi = spi_master_get_devdata(master);
 +
 + zynqmp_gqspi_write(xqspi, GQSPI_EN_OFST, 0x0);
 + clk_disable(xqspi-refclk);
 + clk_disable(xqspi-pclk);

and this would become pm_runtime_put()

[...]
 +/**
 + * zynqmp_qspi_filltxfifo:   Fills the TX FIFO as long as there is room in
 + *   the FIFO or the bytes required to be
 + *   transmitted.
 + * @xqspi:   Pointer to the zynqmp_qspi structure
 + * @size:Number of bytes to be copied from TX buffer to TX FIFO
 + */
 +static void zynqmp_qspi_filltxfifo(struct zynqmp_qspi *xqspi, int 
 +size) {
 + u32 count = 0;
 +
 + while ((xqspi-bytes_to_transfer  0)  (count  size)) {
 + writel(*((u32 *) xqspi-txbuf), xqspi-regs + GQSPI_TXD_OFST);

Here the writel wrapper is not used. Is there a reason, then it would probably  
deserve a comment.

[...]
 +/**
 + * zynqmp_process_dma_irq:   Handler for DMA done interrupt of QSPI
 + *   controller
 + * @xqspi:   zynqmp_qspi instance pointer
 + *
 + * This function handles DMA interrupt only.
 + */
 +static void zynqmp_process_dma_irq(struct zynqmp_qspi *xqspi) {
 + u32 config_reg, genfifoentry;
 +
 + dma_unmap_single(xqspi-dev, xqspi-dma_addr,
 + xqspi-dma_rx_bytes, DMA_FROM_DEVICE);
 + xqspi-rxbuf += xqspi-dma_rx_bytes;
 + xqspi-bytes_to_receive -= xqspi-dma_rx_bytes;
 + xqspi-dma_rx_bytes = 0;
 +
 + /* Disabling the DMA interrupts */
 + writel(GQSPI_QSPIDMA_DST_I_EN_DONE_MASK,
 + xqspi-regs + GQSPI_QSPIDMA_DST_I_DIS_OFST);
 +
 + if (xqspi-bytes_to_receive  0) {
 + /* Switch to IO mode,for remaining bytes to receive */
 + config_reg = readl(xqspi-regs + GQSPI_CONFIG_OFST);
 + config_reg = ~GQSPI_CFG_MODE_EN_MASK;
 + writel(config_reg, xqspi-regs + GQSPI_CONFIG_OFST);
 +
 + /* Initiate the transfer of remaining bytes */
 + genfifoentry = xqspi-genfifoentry;
 + genfifoentry |= xqspi-bytes_to_receive;
 + writel(genfifoentry,
 + xqspi-regs + GQSPI_GEN_FIFO_OFST);
 +
 + /* Dummy generic FIFO entry */
 +  

RE: [RFC PATCH 1/2] devicetree: Add devicetree bindings documentation for ZynqMP GQSPI

2015-05-21 Thread Ranjit Abhimanyu Waghmode
Hi Soren,

On Wed, 2015-05-20 at 12:57PM +0530, Ranjit Waghmode wrote:
> Add bindings documentation for GQSPI controller driver used by Zynq 
> Ultrascale+ MPSoC
> 
> Signed-off-by: Ranjit Waghmode 
> ---
>  .../devicetree/bindings/spi/spi-zynqmp-qspi.txt| 26 
> ++
>  1 file changed, 26 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.txt
> 
> diff --git a/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.txt 
> b/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.txt
> new file mode 100644
> index 000..cec6330
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.txt
> @@ -0,0 +1,26 @@
> +Xilinx Zynq UltraScale+ MPSoC GQSPI controller Device Tree Bindings
> +---
> +
> +Required properties:
> +- compatible : Should be "xlnx,zynqmp-qspi-1.0".
> +- reg: Physical base address and size of GQSPI 
> registers map.
> +- interrupts : Property with a value describing the interrupt
> +   number.
> +- interrupt-parent   : Must be core interrupt controller.
> +- clock-names: List of input clock names - "ref_clk", "pclk"
> +   (See clock bindings for details).
> +- clocks : Clock phandles (see clock bindings for details).
> +
> +Optional properties:
> +- num-cs : Number of chip selects used.
> +
> +Example:
> + qspi: spi@ff0f {
> + compatible = "xlnx,zynqmp-qspi-1.0";
> + clock-names = "ref_clk", "pclk";
> + clocks = <_clk _clk>;
> + interrupts = <0 15 4>;
> + interrupt-parent = <>;
> + num-cs = <1>;
> + reg = <0x0 0xff0f 0x1000 0x0 0xc000 0x800>;

I find things a lot easier to read when there is something separating the 
groups. Could this become:
reg = <0x0 0xff0f 0x1000>, <0x0 0xc000 0x800>; ?

[Ranjit]: I will try your suggestion and send you in the next version.

Regards,
Ranjit


RE: [RFC PATCH 1/2] devicetree: Add devicetree bindings documentation for ZynqMP GQSPI

2015-05-21 Thread Ranjit Abhimanyu Waghmode
Hi Soren,

On Wed, 2015-05-20 at 12:57PM +0530, Ranjit Waghmode wrote:
 Add bindings documentation for GQSPI controller driver used by Zynq 
 Ultrascale+ MPSoC
 
 Signed-off-by: Ranjit Waghmode ranjit.waghm...@xilinx.com
 ---
  .../devicetree/bindings/spi/spi-zynqmp-qspi.txt| 26 
 ++
  1 file changed, 26 insertions(+)
  create mode 100644 
 Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.txt
 
 diff --git a/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.txt 
 b/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.txt
 new file mode 100644
 index 000..cec6330
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/spi/spi-zynqmp-qspi.txt
 @@ -0,0 +1,26 @@
 +Xilinx Zynq UltraScale+ MPSoC GQSPI controller Device Tree Bindings
 +---
 +
 +Required properties:
 +- compatible : Should be xlnx,zynqmp-qspi-1.0.
 +- reg: Physical base address and size of GQSPI 
 registers map.
 +- interrupts : Property with a value describing the interrupt
 +   number.
 +- interrupt-parent   : Must be core interrupt controller.
 +- clock-names: List of input clock names - ref_clk, pclk
 +   (See clock bindings for details).
 +- clocks : Clock phandles (see clock bindings for details).
 +
 +Optional properties:
 +- num-cs : Number of chip selects used.
 +
 +Example:
 + qspi: spi@ff0f {
 + compatible = xlnx,zynqmp-qspi-1.0;
 + clock-names = ref_clk, pclk;
 + clocks = misc_clk misc_clk;
 + interrupts = 0 15 4;
 + interrupt-parent = gic;
 + num-cs = 1;
 + reg = 0x0 0xff0f 0x1000 0x0 0xc000 0x800;

I find things a lot easier to read when there is something separating the 
groups. Could this become:
reg = 0x0 0xff0f 0x1000, 0x0 0xc000 0x800; ?

[Ranjit]: I will try your suggestion and send you in the next version.

Regards,
Ranjit