| Message ID | 1529572997-5995-4-git-send-email-vipul.kumar@xilinx.com |
|---|---|
| State | Superseded |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
| Series | spi:xilinx_spi: Modify xilinx spi driver | expand |
On Thu, Jun 21, 2018 at 2:53 PM, Vipul Kumar <vipul.kumar@xilinx.com> wrote: > This patch is for the startup block issue in the spi controller. > SPI clock is passing through STARTUP block to FLASH. STARTUP block > don't provide clock as soon as QSPI provides command. So, first > command fails. Does this a controller issue? > > This patch added support to read JEDEC id in xilinx_spi_xfer (). > > Signed-off-by: Vipul Kumar <vipul.kumar@xilinx.com> > --- > Changes in v3: > - No change > --- > drivers/spi/xilinx_spi.c | 41 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 41 insertions(+) > > diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c > index 4026540..61301c2 100644 > --- a/drivers/spi/xilinx_spi.c > +++ b/drivers/spi/xilinx_spi.c > @@ -204,6 +204,40 @@ static u32 xilinx_spi_read_rxfifo(struct udevice *bus, u8 *rxp, u32 rxbytes) > return i; > } > > +static void xilinx_startup_block(struct udevice *dev, unsigned int bytes, > + const void *dout, void *din) > +{ > + struct udevice *bus = dev_get_parent(dev); > + struct xilinx_spi_priv *priv = dev_get_priv(bus); > + struct xilinx_spi_regs *regs = priv->regs; > + struct dm_spi_slave_platdata *slave_plat = dev_get_parent_platdata(dev); > + const unsigned char *txp = dout; > + unsigned char *rxp = din; > + static int startup; > + u32 reg, count; > + u32 txbytes = bytes; > + u32 rxbytes = bytes; > + > + /* > + * This loop runs two times. First time to send the command. > + * Second time to transfer data. After transferring data, > + * it sets txp to the initial value for the normal operation. > + */ > + for ( ; startup < 2; startup++) { > + count = xilinx_spi_fill_txfifo(bus, txp, txbytes); > + reg = readl(®s->spicr) & ~SPICR_MASTER_INHIBIT; > + writel(reg, ®s->spicr); > + count = xilinx_spi_read_rxfifo(bus, rxp, rxbytes); > + txp = din; > + > + if (startup) { > + spi_cs_deactivate(dev); > + spi_cs_activate(dev, slave_plat->cs); > + txp = dout; > + } > + } > +} > + > static int xilinx_spi_xfer(struct udevice *dev, unsigned int bitlen, > const void *dout, void *din, unsigned long flags) > { > @@ -236,6 +270,13 @@ static int xilinx_spi_xfer(struct udevice *dev, unsigned int bitlen, > if (flags & SPI_XFER_BEGIN) > spi_cs_activate(dev, slave_plat->cs); > > + /* > + * This is the work around for the startup block issue in > + * the spi controller. SPI clock is passing through STARTUP > + * block to FLASH. STARTUP block don't provide clock as soon > + * as QSPI provides command. So first command fails. > + */ > + xilinx_startup_block(dev, bytes, dout, din); But not every spi_xfer from flash side is to read JEDEC ID? during probe of sf, the relevant spi_xfer is to read JEDEC ID
Hi Jagan, > -----Original Message----- > From: Jagan Teki [mailto:jagannadh.teki@gmail.com] > Sent: Monday, June 25, 2018 3:47 PM > To: Vipul Kumar <vipulk@xilinx.com> > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Michal Simek > <michals@xilinx.com> > Subject: Re: [U-Boot] [UBOOT PATCH v3 3/3] spi: xilinx_spi: Added support to > read JEDEC-id twice at the boot time > > On Thu, Jun 21, 2018 at 2:53 PM, Vipul Kumar <vipul.kumar@xilinx.com> > wrote: > > This patch is for the startup block issue in the spi controller. > > SPI clock is passing through STARTUP block to FLASH. STARTUP block > > don't provide clock as soon as QSPI provides command. So, first > > command fails. > > Does this a controller issue? Yes, this is a controller issue. > > > > > This patch added support to read JEDEC id in xilinx_spi_xfer (). > > > > Signed-off-by: Vipul Kumar <vipul.kumar@xilinx.com> > > --- > > Changes in v3: > > - No change > > --- > > drivers/spi/xilinx_spi.c | 41 > > +++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 41 insertions(+) > > > > diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c index > > 4026540..61301c2 100644 > > --- a/drivers/spi/xilinx_spi.c > > +++ b/drivers/spi/xilinx_spi.c > > @@ -204,6 +204,40 @@ static u32 xilinx_spi_read_rxfifo(struct udevice > *bus, u8 *rxp, u32 rxbytes) > > return i; > > } > > > > +static void xilinx_startup_block(struct udevice *dev, unsigned int bytes, > > + const void *dout, void *din) { > > + struct udevice *bus = dev_get_parent(dev); > > + struct xilinx_spi_priv *priv = dev_get_priv(bus); > > + struct xilinx_spi_regs *regs = priv->regs; > > + struct dm_spi_slave_platdata *slave_plat = > dev_get_parent_platdata(dev); > > + const unsigned char *txp = dout; > > + unsigned char *rxp = din; > > + static int startup; > > + u32 reg, count; > > + u32 txbytes = bytes; > > + u32 rxbytes = bytes; > > + > > + /* > > + * This loop runs two times. First time to send the command. > > + * Second time to transfer data. After transferring data, > > + * it sets txp to the initial value for the normal operation. > > + */ > > + for ( ; startup < 2; startup++) { > > + count = xilinx_spi_fill_txfifo(bus, txp, txbytes); > > + reg = readl(®s->spicr) & ~SPICR_MASTER_INHIBIT; > > + writel(reg, ®s->spicr); > > + count = xilinx_spi_read_rxfifo(bus, rxp, rxbytes); > > + txp = din; > > + > > + if (startup) { > > + spi_cs_deactivate(dev); > > + spi_cs_activate(dev, slave_plat->cs); > > + txp = dout; > > + } > > + } > > +} > > + > > static int xilinx_spi_xfer(struct udevice *dev, unsigned int bitlen, > > const void *dout, void *din, unsigned long > > flags) { @@ -236,6 +270,13 @@ static int xilinx_spi_xfer(struct > > udevice *dev, unsigned int bitlen, > > if (flags & SPI_XFER_BEGIN) > > spi_cs_activate(dev, slave_plat->cs); > > > > + /* > > + * This is the work around for the startup block issue in > > + * the spi controller. SPI clock is passing through STARTUP > > + * block to FLASH. STARTUP block don't provide clock as soon > > + * as QSPI provides command. So first command fails. > > + */ > > + xilinx_startup_block(dev, bytes, dout, din); > > But not every spi_xfer from flash side is to read JEDEC ID? during probe of sf, > the relevant spi_xfer is to read JEDEC ID Inside xilinx_startup_block() function, we are using static variable, so that code will execute only first time. Regards, Vipul
On Mon, Jun 25, 2018 at 4:06 PM, Vipul Kumar <vipulk@xilinx.com> wrote: > Hi Jagan, > >> -----Original Message----- >> From: Jagan Teki [mailto:jagannadh.teki@gmail.com] >> Sent: Monday, June 25, 2018 3:47 PM >> To: Vipul Kumar <vipulk@xilinx.com> >> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Michal Simek >> <michals@xilinx.com> >> Subject: Re: [U-Boot] [UBOOT PATCH v3 3/3] spi: xilinx_spi: Added support to >> read JEDEC-id twice at the boot time >> >> On Thu, Jun 21, 2018 at 2:53 PM, Vipul Kumar <vipul.kumar@xilinx.com> >> wrote: >> > This patch is for the startup block issue in the spi controller. >> > SPI clock is passing through STARTUP block to FLASH. STARTUP block >> > don't provide clock as soon as QSPI provides command. So, first >> > command fails. >> >> Does this a controller issue? > > Yes, this is a controller issue. > >> >> > >> > This patch added support to read JEDEC id in xilinx_spi_xfer (). >> > >> > Signed-off-by: Vipul Kumar <vipul.kumar@xilinx.com> >> > --- >> > Changes in v3: >> > - No change >> > --- >> > drivers/spi/xilinx_spi.c | 41 >> > +++++++++++++++++++++++++++++++++++++++++ >> > 1 file changed, 41 insertions(+) >> > >> > diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c index >> > 4026540..61301c2 100644 >> > --- a/drivers/spi/xilinx_spi.c >> > +++ b/drivers/spi/xilinx_spi.c >> > @@ -204,6 +204,40 @@ static u32 xilinx_spi_read_rxfifo(struct udevice >> *bus, u8 *rxp, u32 rxbytes) >> > return i; >> > } >> > >> > +static void xilinx_startup_block(struct udevice *dev, unsigned int bytes, >> > + const void *dout, void *din) { >> > + struct udevice *bus = dev_get_parent(dev); >> > + struct xilinx_spi_priv *priv = dev_get_priv(bus); >> > + struct xilinx_spi_regs *regs = priv->regs; >> > + struct dm_spi_slave_platdata *slave_plat = >> dev_get_parent_platdata(dev); >> > + const unsigned char *txp = dout; >> > + unsigned char *rxp = din; >> > + static int startup; >> > + u32 reg, count; >> > + u32 txbytes = bytes; >> > + u32 rxbytes = bytes; >> > + >> > + /* >> > + * This loop runs two times. First time to send the command. >> > + * Second time to transfer data. After transferring data, >> > + * it sets txp to the initial value for the normal operation. >> > + */ >> > + for ( ; startup < 2; startup++) { >> > + count = xilinx_spi_fill_txfifo(bus, txp, txbytes); >> > + reg = readl(®s->spicr) & ~SPICR_MASTER_INHIBIT; >> > + writel(reg, ®s->spicr); >> > + count = xilinx_spi_read_rxfifo(bus, rxp, rxbytes); >> > + txp = din; >> > + >> > + if (startup) { >> > + spi_cs_deactivate(dev); >> > + spi_cs_activate(dev, slave_plat->cs); >> > + txp = dout; >> > + } >> > + } >> > +} >> > + >> > static int xilinx_spi_xfer(struct udevice *dev, unsigned int bitlen, >> > const void *dout, void *din, unsigned long >> > flags) { @@ -236,6 +270,13 @@ static int xilinx_spi_xfer(struct >> > udevice *dev, unsigned int bitlen, >> > if (flags & SPI_XFER_BEGIN) >> > spi_cs_activate(dev, slave_plat->cs); >> > >> > + /* >> > + * This is the work around for the startup block issue in >> > + * the spi controller. SPI clock is passing through STARTUP >> > + * block to FLASH. STARTUP block don't provide clock as soon >> > + * as QSPI provides command. So first command fails. >> > + */ >> > + xilinx_startup_block(dev, bytes, dout, din); >> >> But not every spi_xfer from flash side is to read JEDEC ID? during probe of sf, >> the relevant spi_xfer is to read JEDEC ID > > Inside xilinx_startup_block() function, we are using static variable, so that code will execute only first time. Sorry, IMHO mainline won't accept this kind of hack (particularly avoiding function execution through static variables) by for the sake of bug fix. Having a proper condition to check and fix the errata look fine to me, any idea how Linux handling this?
Hi Jagan, On 27.6.2018 08:43, Jagan Teki wrote: > On Mon, Jun 25, 2018 at 4:06 PM, Vipul Kumar <vipulk@xilinx.com> wrote: >> Hi Jagan, >> >>> -----Original Message----- >>> From: Jagan Teki [mailto:jagannadh.teki@gmail.com] >>> Sent: Monday, June 25, 2018 3:47 PM >>> To: Vipul Kumar <vipulk@xilinx.com> >>> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Michal Simek >>> <michals@xilinx.com> >>> Subject: Re: [U-Boot] [UBOOT PATCH v3 3/3] spi: xilinx_spi: Added support to >>> read JEDEC-id twice at the boot time >>> >>> On Thu, Jun 21, 2018 at 2:53 PM, Vipul Kumar <vipul.kumar@xilinx.com> >>> wrote: >>>> This patch is for the startup block issue in the spi controller. >>>> SPI clock is passing through STARTUP block to FLASH. STARTUP block >>>> don't provide clock as soon as QSPI provides command. So, first >>>> command fails. >>> >>> Does this a controller issue? >> >> Yes, this is a controller issue. >> >>> >>>> >>>> This patch added support to read JEDEC id in xilinx_spi_xfer (). >>>> >>>> Signed-off-by: Vipul Kumar <vipul.kumar@xilinx.com> >>>> --- >>>> Changes in v3: >>>> - No change >>>> --- >>>> drivers/spi/xilinx_spi.c | 41 >>>> +++++++++++++++++++++++++++++++++++++++++ >>>> 1 file changed, 41 insertions(+) >>>> >>>> diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c index >>>> 4026540..61301c2 100644 >>>> --- a/drivers/spi/xilinx_spi.c >>>> +++ b/drivers/spi/xilinx_spi.c >>>> @@ -204,6 +204,40 @@ static u32 xilinx_spi_read_rxfifo(struct udevice >>> *bus, u8 *rxp, u32 rxbytes) >>>> return i; >>>> } >>>> >>>> +static void xilinx_startup_block(struct udevice *dev, unsigned int bytes, >>>> + const void *dout, void *din) { >>>> + struct udevice *bus = dev_get_parent(dev); >>>> + struct xilinx_spi_priv *priv = dev_get_priv(bus); >>>> + struct xilinx_spi_regs *regs = priv->regs; >>>> + struct dm_spi_slave_platdata *slave_plat = >>> dev_get_parent_platdata(dev); >>>> + const unsigned char *txp = dout; >>>> + unsigned char *rxp = din; >>>> + static int startup; >>>> + u32 reg, count; >>>> + u32 txbytes = bytes; >>>> + u32 rxbytes = bytes; >>>> + >>>> + /* >>>> + * This loop runs two times. First time to send the command. >>>> + * Second time to transfer data. After transferring data, >>>> + * it sets txp to the initial value for the normal operation. >>>> + */ >>>> + for ( ; startup < 2; startup++) { >>>> + count = xilinx_spi_fill_txfifo(bus, txp, txbytes); >>>> + reg = readl(®s->spicr) & ~SPICR_MASTER_INHIBIT; >>>> + writel(reg, ®s->spicr); >>>> + count = xilinx_spi_read_rxfifo(bus, rxp, rxbytes); >>>> + txp = din; >>>> + >>>> + if (startup) { >>>> + spi_cs_deactivate(dev); >>>> + spi_cs_activate(dev, slave_plat->cs); >>>> + txp = dout; >>>> + } >>>> + } >>>> +} >>>> + >>>> static int xilinx_spi_xfer(struct udevice *dev, unsigned int bitlen, >>>> const void *dout, void *din, unsigned long >>>> flags) { @@ -236,6 +270,13 @@ static int xilinx_spi_xfer(struct >>>> udevice *dev, unsigned int bitlen, >>>> if (flags & SPI_XFER_BEGIN) >>>> spi_cs_activate(dev, slave_plat->cs); >>>> >>>> + /* >>>> + * This is the work around for the startup block issue in >>>> + * the spi controller. SPI clock is passing through STARTUP >>>> + * block to FLASH. STARTUP block don't provide clock as soon >>>> + * as QSPI provides command. So first command fails. >>>> + */ >>>> + xilinx_startup_block(dev, bytes, dout, din); >>> >>> But not every spi_xfer from flash side is to read JEDEC ID? during probe of sf, >>> the relevant spi_xfer is to read JEDEC ID >> >> Inside xilinx_startup_block() function, we are using static variable, so that code will execute only first time. > > Sorry, IMHO mainline won't accept this kind of hack (particularly > avoiding function execution through static variables) by for the sake > of bug fix. Having a proper condition to check and fix the errata look > fine to me, any idea how Linux handling this? ok - static startup variable should be move to private data structure to be private for every instance. Also please put a link to errata for a record. Jagan: It will be much easier if you can simply write snippet which you are suggesting. Thanks, Michal
On Wed, Jun 27, 2018 at 12:59 PM, Michal Simek <michal.simek@xilinx.com> wrote: > Hi Jagan, > > On 27.6.2018 08:43, Jagan Teki wrote: >> On Mon, Jun 25, 2018 at 4:06 PM, Vipul Kumar <vipulk@xilinx.com> wrote: >>> Hi Jagan, >>> >>>> -----Original Message----- >>>> From: Jagan Teki [mailto:jagannadh.teki@gmail.com] >>>> Sent: Monday, June 25, 2018 3:47 PM >>>> To: Vipul Kumar <vipulk@xilinx.com> >>>> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Michal Simek >>>> <michals@xilinx.com> >>>> Subject: Re: [U-Boot] [UBOOT PATCH v3 3/3] spi: xilinx_spi: Added support to >>>> read JEDEC-id twice at the boot time >>>> >>>> On Thu, Jun 21, 2018 at 2:53 PM, Vipul Kumar <vipul.kumar@xilinx.com> >>>> wrote: >>>>> This patch is for the startup block issue in the spi controller. >>>>> SPI clock is passing through STARTUP block to FLASH. STARTUP block >>>>> don't provide clock as soon as QSPI provides command. So, first >>>>> command fails. >>>> >>>> Does this a controller issue? >>> >>> Yes, this is a controller issue. >>> >>>> >>>>> >>>>> This patch added support to read JEDEC id in xilinx_spi_xfer (). >>>>> >>>>> Signed-off-by: Vipul Kumar <vipul.kumar@xilinx.com> >>>>> --- >>>>> Changes in v3: >>>>> - No change >>>>> --- >>>>> drivers/spi/xilinx_spi.c | 41 >>>>> +++++++++++++++++++++++++++++++++++++++++ >>>>> 1 file changed, 41 insertions(+) >>>>> >>>>> diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c index >>>>> 4026540..61301c2 100644 >>>>> --- a/drivers/spi/xilinx_spi.c >>>>> +++ b/drivers/spi/xilinx_spi.c >>>>> @@ -204,6 +204,40 @@ static u32 xilinx_spi_read_rxfifo(struct udevice >>>> *bus, u8 *rxp, u32 rxbytes) >>>>> return i; >>>>> } >>>>> >>>>> +static void xilinx_startup_block(struct udevice *dev, unsigned int bytes, >>>>> + const void *dout, void *din) { >>>>> + struct udevice *bus = dev_get_parent(dev); >>>>> + struct xilinx_spi_priv *priv = dev_get_priv(bus); >>>>> + struct xilinx_spi_regs *regs = priv->regs; >>>>> + struct dm_spi_slave_platdata *slave_plat = >>>> dev_get_parent_platdata(dev); >>>>> + const unsigned char *txp = dout; >>>>> + unsigned char *rxp = din; >>>>> + static int startup; >>>>> + u32 reg, count; >>>>> + u32 txbytes = bytes; >>>>> + u32 rxbytes = bytes; >>>>> + >>>>> + /* >>>>> + * This loop runs two times. First time to send the command. >>>>> + * Second time to transfer data. After transferring data, >>>>> + * it sets txp to the initial value for the normal operation. >>>>> + */ >>>>> + for ( ; startup < 2; startup++) { >>>>> + count = xilinx_spi_fill_txfifo(bus, txp, txbytes); >>>>> + reg = readl(®s->spicr) & ~SPICR_MASTER_INHIBIT; >>>>> + writel(reg, ®s->spicr); >>>>> + count = xilinx_spi_read_rxfifo(bus, rxp, rxbytes); >>>>> + txp = din; >>>>> + >>>>> + if (startup) { >>>>> + spi_cs_deactivate(dev); >>>>> + spi_cs_activate(dev, slave_plat->cs); >>>>> + txp = dout; >>>>> + } >>>>> + } >>>>> +} >>>>> + >>>>> static int xilinx_spi_xfer(struct udevice *dev, unsigned int bitlen, >>>>> const void *dout, void *din, unsigned long >>>>> flags) { @@ -236,6 +270,13 @@ static int xilinx_spi_xfer(struct >>>>> udevice *dev, unsigned int bitlen, >>>>> if (flags & SPI_XFER_BEGIN) >>>>> spi_cs_activate(dev, slave_plat->cs); >>>>> >>>>> + /* >>>>> + * This is the work around for the startup block issue in >>>>> + * the spi controller. SPI clock is passing through STARTUP >>>>> + * block to FLASH. STARTUP block don't provide clock as soon >>>>> + * as QSPI provides command. So first command fails. >>>>> + */ >>>>> + xilinx_startup_block(dev, bytes, dout, din); >>>> >>>> But not every spi_xfer from flash side is to read JEDEC ID? during probe of sf, >>>> the relevant spi_xfer is to read JEDEC ID >>> >>> Inside xilinx_startup_block() function, we are using static variable, so that code will execute only first time. >> >> Sorry, IMHO mainline won't accept this kind of hack (particularly >> avoiding function execution through static variables) by for the sake >> of bug fix. Having a proper condition to check and fix the errata look >> fine to me, any idea how Linux handling this? > > ok - static startup variable should be move to private data structure to > be private for every instance. > > Also please put a link to errata for a record. > > Jagan: It will be much easier if you can simply write snippet which you > are suggesting. thought of, I'm still thinking how we can do this. I didn't see the Linux code yet, any idea how Linux does? Jagan.
On 27.6.2018 09:35, Jagan Teki wrote: > On Wed, Jun 27, 2018 at 12:59 PM, Michal Simek <michal.simek@xilinx.com> wrote: >> Hi Jagan, >> >> On 27.6.2018 08:43, Jagan Teki wrote: >>> On Mon, Jun 25, 2018 at 4:06 PM, Vipul Kumar <vipulk@xilinx.com> wrote: >>>> Hi Jagan, >>>> >>>>> -----Original Message----- >>>>> From: Jagan Teki [mailto:jagannadh.teki@gmail.com] >>>>> Sent: Monday, June 25, 2018 3:47 PM >>>>> To: Vipul Kumar <vipulk@xilinx.com> >>>>> Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Michal Simek >>>>> <michals@xilinx.com> >>>>> Subject: Re: [U-Boot] [UBOOT PATCH v3 3/3] spi: xilinx_spi: Added support to >>>>> read JEDEC-id twice at the boot time >>>>> >>>>> On Thu, Jun 21, 2018 at 2:53 PM, Vipul Kumar <vipul.kumar@xilinx.com> >>>>> wrote: >>>>>> This patch is for the startup block issue in the spi controller. >>>>>> SPI clock is passing through STARTUP block to FLASH. STARTUP block >>>>>> don't provide clock as soon as QSPI provides command. So, first >>>>>> command fails. >>>>> >>>>> Does this a controller issue? >>>> >>>> Yes, this is a controller issue. >>>> >>>>> >>>>>> >>>>>> This patch added support to read JEDEC id in xilinx_spi_xfer (). >>>>>> >>>>>> Signed-off-by: Vipul Kumar <vipul.kumar@xilinx.com> >>>>>> --- >>>>>> Changes in v3: >>>>>> - No change >>>>>> --- >>>>>> drivers/spi/xilinx_spi.c | 41 >>>>>> +++++++++++++++++++++++++++++++++++++++++ >>>>>> 1 file changed, 41 insertions(+) >>>>>> >>>>>> diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c index >>>>>> 4026540..61301c2 100644 >>>>>> --- a/drivers/spi/xilinx_spi.c >>>>>> +++ b/drivers/spi/xilinx_spi.c >>>>>> @@ -204,6 +204,40 @@ static u32 xilinx_spi_read_rxfifo(struct udevice >>>>> *bus, u8 *rxp, u32 rxbytes) >>>>>> return i; >>>>>> } >>>>>> >>>>>> +static void xilinx_startup_block(struct udevice *dev, unsigned int bytes, >>>>>> + const void *dout, void *din) { >>>>>> + struct udevice *bus = dev_get_parent(dev); >>>>>> + struct xilinx_spi_priv *priv = dev_get_priv(bus); >>>>>> + struct xilinx_spi_regs *regs = priv->regs; >>>>>> + struct dm_spi_slave_platdata *slave_plat = >>>>> dev_get_parent_platdata(dev); >>>>>> + const unsigned char *txp = dout; >>>>>> + unsigned char *rxp = din; >>>>>> + static int startup; >>>>>> + u32 reg, count; >>>>>> + u32 txbytes = bytes; >>>>>> + u32 rxbytes = bytes; >>>>>> + >>>>>> + /* >>>>>> + * This loop runs two times. First time to send the command. >>>>>> + * Second time to transfer data. After transferring data, >>>>>> + * it sets txp to the initial value for the normal operation. >>>>>> + */ >>>>>> + for ( ; startup < 2; startup++) { >>>>>> + count = xilinx_spi_fill_txfifo(bus, txp, txbytes); >>>>>> + reg = readl(®s->spicr) & ~SPICR_MASTER_INHIBIT; >>>>>> + writel(reg, ®s->spicr); >>>>>> + count = xilinx_spi_read_rxfifo(bus, rxp, rxbytes); >>>>>> + txp = din; >>>>>> + >>>>>> + if (startup) { >>>>>> + spi_cs_deactivate(dev); >>>>>> + spi_cs_activate(dev, slave_plat->cs); >>>>>> + txp = dout; >>>>>> + } >>>>>> + } >>>>>> +} >>>>>> + >>>>>> static int xilinx_spi_xfer(struct udevice *dev, unsigned int bitlen, >>>>>> const void *dout, void *din, unsigned long >>>>>> flags) { @@ -236,6 +270,13 @@ static int xilinx_spi_xfer(struct >>>>>> udevice *dev, unsigned int bitlen, >>>>>> if (flags & SPI_XFER_BEGIN) >>>>>> spi_cs_activate(dev, slave_plat->cs); >>>>>> >>>>>> + /* >>>>>> + * This is the work around for the startup block issue in >>>>>> + * the spi controller. SPI clock is passing through STARTUP >>>>>> + * block to FLASH. STARTUP block don't provide clock as soon >>>>>> + * as QSPI provides command. So first command fails. >>>>>> + */ >>>>>> + xilinx_startup_block(dev, bytes, dout, din); >>>>> >>>>> But not every spi_xfer from flash side is to read JEDEC ID? during probe of sf, >>>>> the relevant spi_xfer is to read JEDEC ID >>>> >>>> Inside xilinx_startup_block() function, we are using static variable, so that code will execute only first time. >>> >>> Sorry, IMHO mainline won't accept this kind of hack (particularly >>> avoiding function execution through static variables) by for the sake >>> of bug fix. Having a proper condition to check and fix the errata look >>> fine to me, any idea how Linux handling this? >> >> ok - static startup variable should be move to private data structure to >> be private for every instance. >> >> Also please put a link to errata for a record. >> >> Jagan: It will be much easier if you can simply write snippet which you >> are suggesting. > > thought of, I'm still thinking how we can do this. I didn't see the > Linux code yet, any idea how Linux does? The issue is present in Linux but it is not fixed there yet. It means Linux can't be taken as pattern. It is quite clear that this should be handled in the driver code and it should be enabled by default for all IPs till this is fixed in hw IP. Thanks, Michal
diff --git a/drivers/spi/xilinx_spi.c b/drivers/spi/xilinx_spi.c index 4026540..61301c2 100644 --- a/drivers/spi/xilinx_spi.c +++ b/drivers/spi/xilinx_spi.c @@ -204,6 +204,40 @@ static u32 xilinx_spi_read_rxfifo(struct udevice *bus, u8 *rxp, u32 rxbytes) return i; } +static void xilinx_startup_block(struct udevice *dev, unsigned int bytes, + const void *dout, void *din) +{ + struct udevice *bus = dev_get_parent(dev); + struct xilinx_spi_priv *priv = dev_get_priv(bus); + struct xilinx_spi_regs *regs = priv->regs; + struct dm_spi_slave_platdata *slave_plat = dev_get_parent_platdata(dev); + const unsigned char *txp = dout; + unsigned char *rxp = din; + static int startup; + u32 reg, count; + u32 txbytes = bytes; + u32 rxbytes = bytes; + + /* + * This loop runs two times. First time to send the command. + * Second time to transfer data. After transferring data, + * it sets txp to the initial value for the normal operation. + */ + for ( ; startup < 2; startup++) { + count = xilinx_spi_fill_txfifo(bus, txp, txbytes); + reg = readl(®s->spicr) & ~SPICR_MASTER_INHIBIT; + writel(reg, ®s->spicr); + count = xilinx_spi_read_rxfifo(bus, rxp, rxbytes); + txp = din; + + if (startup) { + spi_cs_deactivate(dev); + spi_cs_activate(dev, slave_plat->cs); + txp = dout; + } + } +} + static int xilinx_spi_xfer(struct udevice *dev, unsigned int bitlen, const void *dout, void *din, unsigned long flags) { @@ -236,6 +270,13 @@ static int xilinx_spi_xfer(struct udevice *dev, unsigned int bitlen, if (flags & SPI_XFER_BEGIN) spi_cs_activate(dev, slave_plat->cs); + /* + * This is the work around for the startup block issue in + * the spi controller. SPI clock is passing through STARTUP + * block to FLASH. STARTUP block don't provide clock as soon + * as QSPI provides command. So first command fails. + */ + xilinx_startup_block(dev, bytes, dout, din); while (txbytes && rxbytes) { count = xilinx_spi_fill_txfifo(bus, txp, txbytes);
This patch is for the startup block issue in the spi controller. SPI clock is passing through STARTUP block to FLASH. STARTUP block don't provide clock as soon as QSPI provides command. So, first command fails. This patch added support to read JEDEC id in xilinx_spi_xfer (). Signed-off-by: Vipul Kumar <vipul.kumar@xilinx.com> --- Changes in v3: - No change --- drivers/spi/xilinx_spi.c | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) -- 2.7.4 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.