Re: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI controller

2018-11-16 Thread Schrempf Frieder
On 16.11.18 10:46, Yogesh Narayan Gaur wrote:
> Hi Frieder,
> 
>> -Original Message-
>> From: Schrempf Frieder [mailto:frieder.schre...@kontron.de]
>> Sent: Friday, November 16, 2018 3:12 PM
>> To: Yogesh Narayan Gaur 
>> Cc: Boris Brezillon ; 
>> linux-...@lists.infradead.org;
>> linux-...@vger.kernel.org; Marek Vasut ; Mark
>> Brown ; Han Xu ;
>> dw...@infradead.org; computersforpe...@gmail.com; rich...@nod.at;
>> miquel.ray...@bootlin.com; David Wolfe ; Fabio
>> Estevam ; Prabhakar Kushwaha
>> ; shawn...@kernel.org; linux-
>> ker...@vger.kernel.org
>> Subject: Re: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI
>> controller
>>
>> Hi Yogesh,
>>
>> On 16.11.18 06:41, Yogesh Narayan Gaur wrote:
>>> Hi Frieder,
>>>
>>>> -Original Message-
>>>> From: Schrempf Frieder [mailto:frieder.schre...@kontron.de]
>>>> Sent: Thursday, November 15, 2018 7:32 PM
>>>> To: Yogesh Narayan Gaur 
>>>> Cc: Boris Brezillon ;
>>>> linux-...@lists.infradead.org; linux-...@vger.kernel.org; Marek Vasut
>>>> ; Mark Brown ; Han Xu
>>>> ; dw...@infradead.org;
>> computersforpe...@gmail.com;
>>>> rich...@nod.at; miquel.ray...@bootlin.com; David Wolfe
>>>> ; Fabio Estevam ;
>>>> Prabhakar Kushwaha ;
>> shawn...@kernel.org;
>>>> linux- ker...@vger.kernel.org
>>>> Subject: Re: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP
>>>> QuadSPI controller
>>>>
>>>> Hi Yogesh,
>>>>
>>>> On 15.11.18 14:12, Boris Brezillon wrote:
>>>>> On Thu, 15 Nov 2018 11:43:05 +
>>>>> Schrempf Frieder  wrote:
>>>>>
>>>>>> On 15.11.18 07:22, Yogesh Narayan Gaur wrote:
>>>>>>> Hi Frieder,
>>>>>>>
>>>>>>> With below patch on top of your v5, Read/Write/Erase on CS1 is
>>>>>>> working
>>>> fine for me.
>>>>>>
>>>>>> Ok, are you sure, that AHB read is working too with this patch?
>>>>>> You are removing the memmap_phy offset from SFAR and the SFXXAD
>>>>>> register values.
>>>>>>
>>>>>> I can understand that selection of the CS and IP commands will work
>>>>>> like this, but I can't understand how AHB read should work without
>>>>>> the base address of the mapped memory.
>>>>>>
>>>>>> I'm afraid I still don't fully understand the background of these
>>>>>> things,
>>>>>
>>>>> Same here. Yogesh, can you give us more detail on why you decided to
>>>>> drop the memmap_phy offset?
>>>>
>>>> Your changes do not work on my setup (i.MX6UL). It looks like your
>>>> hardware is different.
>>>>
>>>> I found this patch for LS2080A: [1]. This would explain why you need
>>>> to remove the offset to make it work.
>>>>
>>>> To verify this, could you please test your setup with the current
>>>> spi-nor driver (fsl_quadspi.c). If our assumptions are right, it
>>>> should only work on CS0 and CS1 with [1] applied.
>>>>
>>>
>>> Yes, I need to remove the offset to make it work and this is required for 
>>> the
>> NXP Layerscape-2.x SoCs like LS208x/Ls108x etc.
>>>
>>> I have modified the patch and have introduced entry in quirks for ls2080a. 
>>> With
>> this Read/Write/Erase are working for me for both CS.
>>
>> Ok, what I was asking for is a test with the original, unmodified SPI NOR 
>> driver in
>> mtd/spi-nor/fsl-quadspi.c. We need this to confirm that the problem is really
>> what we think, or to find out if we missed something.
>>
>> Can you please do a quick test? If it confirms our assumptions, I will send 
>> a new
>> version with the quirk and hopefully we can then move on.
>>
> 
> Yes, problem exist with original un-modified upstread SPI-NOR driver also.
> Actually, internally we are maintaining driver with some local change and one 
> of the change is related to same i.e. making having map_addr as 0 for 
> layerscape chips.
> 
> I have tested, that by removing that CS1 access shows error.

Ok, thank you for clarifying this!

> 
> Please integrate these changes in your next version.

Ok.

Thanks,
Frieder

Re: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI controller

2018-11-16 Thread Schrempf Frieder
On 16.11.18 10:46, Yogesh Narayan Gaur wrote:
> Hi Frieder,
> 
>> -Original Message-
>> From: Schrempf Frieder [mailto:frieder.schre...@kontron.de]
>> Sent: Friday, November 16, 2018 3:12 PM
>> To: Yogesh Narayan Gaur 
>> Cc: Boris Brezillon ; 
>> linux-...@lists.infradead.org;
>> linux-...@vger.kernel.org; Marek Vasut ; Mark
>> Brown ; Han Xu ;
>> dw...@infradead.org; computersforpe...@gmail.com; rich...@nod.at;
>> miquel.ray...@bootlin.com; David Wolfe ; Fabio
>> Estevam ; Prabhakar Kushwaha
>> ; shawn...@kernel.org; linux-
>> ker...@vger.kernel.org
>> Subject: Re: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI
>> controller
>>
>> Hi Yogesh,
>>
>> On 16.11.18 06:41, Yogesh Narayan Gaur wrote:
>>> Hi Frieder,
>>>
>>>> -Original Message-
>>>> From: Schrempf Frieder [mailto:frieder.schre...@kontron.de]
>>>> Sent: Thursday, November 15, 2018 7:32 PM
>>>> To: Yogesh Narayan Gaur 
>>>> Cc: Boris Brezillon ;
>>>> linux-...@lists.infradead.org; linux-...@vger.kernel.org; Marek Vasut
>>>> ; Mark Brown ; Han Xu
>>>> ; dw...@infradead.org;
>> computersforpe...@gmail.com;
>>>> rich...@nod.at; miquel.ray...@bootlin.com; David Wolfe
>>>> ; Fabio Estevam ;
>>>> Prabhakar Kushwaha ;
>> shawn...@kernel.org;
>>>> linux- ker...@vger.kernel.org
>>>> Subject: Re: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP
>>>> QuadSPI controller
>>>>
>>>> Hi Yogesh,
>>>>
>>>> On 15.11.18 14:12, Boris Brezillon wrote:
>>>>> On Thu, 15 Nov 2018 11:43:05 +
>>>>> Schrempf Frieder  wrote:
>>>>>
>>>>>> On 15.11.18 07:22, Yogesh Narayan Gaur wrote:
>>>>>>> Hi Frieder,
>>>>>>>
>>>>>>> With below patch on top of your v5, Read/Write/Erase on CS1 is
>>>>>>> working
>>>> fine for me.
>>>>>>
>>>>>> Ok, are you sure, that AHB read is working too with this patch?
>>>>>> You are removing the memmap_phy offset from SFAR and the SFXXAD
>>>>>> register values.
>>>>>>
>>>>>> I can understand that selection of the CS and IP commands will work
>>>>>> like this, but I can't understand how AHB read should work without
>>>>>> the base address of the mapped memory.
>>>>>>
>>>>>> I'm afraid I still don't fully understand the background of these
>>>>>> things,
>>>>>
>>>>> Same here. Yogesh, can you give us more detail on why you decided to
>>>>> drop the memmap_phy offset?
>>>>
>>>> Your changes do not work on my setup (i.MX6UL). It looks like your
>>>> hardware is different.
>>>>
>>>> I found this patch for LS2080A: [1]. This would explain why you need
>>>> to remove the offset to make it work.
>>>>
>>>> To verify this, could you please test your setup with the current
>>>> spi-nor driver (fsl_quadspi.c). If our assumptions are right, it
>>>> should only work on CS0 and CS1 with [1] applied.
>>>>
>>>
>>> Yes, I need to remove the offset to make it work and this is required for 
>>> the
>> NXP Layerscape-2.x SoCs like LS208x/Ls108x etc.
>>>
>>> I have modified the patch and have introduced entry in quirks for ls2080a. 
>>> With
>> this Read/Write/Erase are working for me for both CS.
>>
>> Ok, what I was asking for is a test with the original, unmodified SPI NOR 
>> driver in
>> mtd/spi-nor/fsl-quadspi.c. We need this to confirm that the problem is really
>> what we think, or to find out if we missed something.
>>
>> Can you please do a quick test? If it confirms our assumptions, I will send 
>> a new
>> version with the quirk and hopefully we can then move on.
>>
> 
> Yes, problem exist with original un-modified upstread SPI-NOR driver also.
> Actually, internally we are maintaining driver with some local change and one 
> of the change is related to same i.e. making having map_addr as 0 for 
> layerscape chips.
> 
> I have tested, that by removing that CS1 access shows error.

Ok, thank you for clarifying this!

> 
> Please integrate these changes in your next version.

Ok.

Thanks,
Frieder

RE: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI controller

2018-11-16 Thread Yogesh Narayan Gaur
Hi Frieder,

> -Original Message-
> From: Schrempf Frieder [mailto:frieder.schre...@kontron.de]
> Sent: Friday, November 16, 2018 3:12 PM
> To: Yogesh Narayan Gaur 
> Cc: Boris Brezillon ; 
> linux-...@lists.infradead.org;
> linux-...@vger.kernel.org; Marek Vasut ; Mark
> Brown ; Han Xu ;
> dw...@infradead.org; computersforpe...@gmail.com; rich...@nod.at;
> miquel.ray...@bootlin.com; David Wolfe ; Fabio
> Estevam ; Prabhakar Kushwaha
> ; shawn...@kernel.org; linux-
> ker...@vger.kernel.org
> Subject: Re: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI
> controller
> 
> Hi Yogesh,
> 
> On 16.11.18 06:41, Yogesh Narayan Gaur wrote:
> > Hi Frieder,
> >
> >> -Original Message-
> >> From: Schrempf Frieder [mailto:frieder.schre...@kontron.de]
> >> Sent: Thursday, November 15, 2018 7:32 PM
> >> To: Yogesh Narayan Gaur 
> >> Cc: Boris Brezillon ;
> >> linux-...@lists.infradead.org; linux-...@vger.kernel.org; Marek Vasut
> >> ; Mark Brown ; Han Xu
> >> ; dw...@infradead.org;
> computersforpe...@gmail.com;
> >> rich...@nod.at; miquel.ray...@bootlin.com; David Wolfe
> >> ; Fabio Estevam ;
> >> Prabhakar Kushwaha ;
> shawn...@kernel.org;
> >> linux- ker...@vger.kernel.org
> >> Subject: Re: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP
> >> QuadSPI controller
> >>
> >> Hi Yogesh,
> >>
> >> On 15.11.18 14:12, Boris Brezillon wrote:
> >>> On Thu, 15 Nov 2018 11:43:05 +
> >>> Schrempf Frieder  wrote:
> >>>
> >>>> On 15.11.18 07:22, Yogesh Narayan Gaur wrote:
> >>>>> Hi Frieder,
> >>>>>
> >>>>> With below patch on top of your v5, Read/Write/Erase on CS1 is
> >>>>> working
> >> fine for me.
> >>>>
> >>>> Ok, are you sure, that AHB read is working too with this patch?
> >>>> You are removing the memmap_phy offset from SFAR and the SFXXAD
> >>>> register values.
> >>>>
> >>>> I can understand that selection of the CS and IP commands will work
> >>>> like this, but I can't understand how AHB read should work without
> >>>> the base address of the mapped memory.
> >>>>
> >>>> I'm afraid I still don't fully understand the background of these
> >>>> things,
> >>>
> >>> Same here. Yogesh, can you give us more detail on why you decided to
> >>> drop the memmap_phy offset?
> >>
> >> Your changes do not work on my setup (i.MX6UL). It looks like your
> >> hardware is different.
> >>
> >> I found this patch for LS2080A: [1]. This would explain why you need
> >> to remove the offset to make it work.
> >>
> >> To verify this, could you please test your setup with the current
> >> spi-nor driver (fsl_quadspi.c). If our assumptions are right, it
> >> should only work on CS0 and CS1 with [1] applied.
> >>
> >
> > Yes, I need to remove the offset to make it work and this is required for 
> > the
> NXP Layerscape-2.x SoCs like LS208x/Ls108x etc.
> >
> > I have modified the patch and have introduced entry in quirks for ls2080a. 
> > With
> this Read/Write/Erase are working for me for both CS.
> 
> Ok, what I was asking for is a test with the original, unmodified SPI NOR 
> driver in
> mtd/spi-nor/fsl-quadspi.c. We need this to confirm that the problem is really
> what we think, or to find out if we missed something.
> 
> Can you please do a quick test? If it confirms our assumptions, I will send a 
> new
> version with the quirk and hopefully we can then move on.
> 

Yes, problem exist with original un-modified upstread SPI-NOR driver also.
Actually, internally we are maintaining driver with some local change and one 
of the change is related to same i.e. making having map_addr as 0 for 
layerscape chips.

I have tested, that by removing that CS1 access shows error.

Please integrate these changes in your next version.

--
Regards
Yogesh Gaur

> Thanks,
> Frieder
> 
> >
> > diff --git a/drivers/spi/spi-fsl-qspi.c b/drivers/spi/spi-fsl-qspi.c
> > index ce45e8e..5d26f73 100644
> > --- a/drivers/spi/spi-fsl-qspi.c
> > +++ b/drivers/spi/spi-fsl-qspi.c
> > @@ -175,6 +175,9 @@
> >   /* TKT245618, the controller cannot wake up from wait mode */
> >   #define QUADSPI_QUIRK_TKT245618BIT(3)
> >
> > +/* QSPI_AMBA_BASE is internally added by SOC design for LS-2.x architecture
> */
> > +

RE: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI controller

2018-11-16 Thread Yogesh Narayan Gaur
Hi Frieder,

> -Original Message-
> From: Schrempf Frieder [mailto:frieder.schre...@kontron.de]
> Sent: Friday, November 16, 2018 3:12 PM
> To: Yogesh Narayan Gaur 
> Cc: Boris Brezillon ; 
> linux-...@lists.infradead.org;
> linux-...@vger.kernel.org; Marek Vasut ; Mark
> Brown ; Han Xu ;
> dw...@infradead.org; computersforpe...@gmail.com; rich...@nod.at;
> miquel.ray...@bootlin.com; David Wolfe ; Fabio
> Estevam ; Prabhakar Kushwaha
> ; shawn...@kernel.org; linux-
> ker...@vger.kernel.org
> Subject: Re: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI
> controller
> 
> Hi Yogesh,
> 
> On 16.11.18 06:41, Yogesh Narayan Gaur wrote:
> > Hi Frieder,
> >
> >> -Original Message-
> >> From: Schrempf Frieder [mailto:frieder.schre...@kontron.de]
> >> Sent: Thursday, November 15, 2018 7:32 PM
> >> To: Yogesh Narayan Gaur 
> >> Cc: Boris Brezillon ;
> >> linux-...@lists.infradead.org; linux-...@vger.kernel.org; Marek Vasut
> >> ; Mark Brown ; Han Xu
> >> ; dw...@infradead.org;
> computersforpe...@gmail.com;
> >> rich...@nod.at; miquel.ray...@bootlin.com; David Wolfe
> >> ; Fabio Estevam ;
> >> Prabhakar Kushwaha ;
> shawn...@kernel.org;
> >> linux- ker...@vger.kernel.org
> >> Subject: Re: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP
> >> QuadSPI controller
> >>
> >> Hi Yogesh,
> >>
> >> On 15.11.18 14:12, Boris Brezillon wrote:
> >>> On Thu, 15 Nov 2018 11:43:05 +
> >>> Schrempf Frieder  wrote:
> >>>
> >>>> On 15.11.18 07:22, Yogesh Narayan Gaur wrote:
> >>>>> Hi Frieder,
> >>>>>
> >>>>> With below patch on top of your v5, Read/Write/Erase on CS1 is
> >>>>> working
> >> fine for me.
> >>>>
> >>>> Ok, are you sure, that AHB read is working too with this patch?
> >>>> You are removing the memmap_phy offset from SFAR and the SFXXAD
> >>>> register values.
> >>>>
> >>>> I can understand that selection of the CS and IP commands will work
> >>>> like this, but I can't understand how AHB read should work without
> >>>> the base address of the mapped memory.
> >>>>
> >>>> I'm afraid I still don't fully understand the background of these
> >>>> things,
> >>>
> >>> Same here. Yogesh, can you give us more detail on why you decided to
> >>> drop the memmap_phy offset?
> >>
> >> Your changes do not work on my setup (i.MX6UL). It looks like your
> >> hardware is different.
> >>
> >> I found this patch for LS2080A: [1]. This would explain why you need
> >> to remove the offset to make it work.
> >>
> >> To verify this, could you please test your setup with the current
> >> spi-nor driver (fsl_quadspi.c). If our assumptions are right, it
> >> should only work on CS0 and CS1 with [1] applied.
> >>
> >
> > Yes, I need to remove the offset to make it work and this is required for 
> > the
> NXP Layerscape-2.x SoCs like LS208x/Ls108x etc.
> >
> > I have modified the patch and have introduced entry in quirks for ls2080a. 
> > With
> this Read/Write/Erase are working for me for both CS.
> 
> Ok, what I was asking for is a test with the original, unmodified SPI NOR 
> driver in
> mtd/spi-nor/fsl-quadspi.c. We need this to confirm that the problem is really
> what we think, or to find out if we missed something.
> 
> Can you please do a quick test? If it confirms our assumptions, I will send a 
> new
> version with the quirk and hopefully we can then move on.
> 

Yes, problem exist with original un-modified upstread SPI-NOR driver also.
Actually, internally we are maintaining driver with some local change and one 
of the change is related to same i.e. making having map_addr as 0 for 
layerscape chips.

I have tested, that by removing that CS1 access shows error.

Please integrate these changes in your next version.

--
Regards
Yogesh Gaur

> Thanks,
> Frieder
> 
> >
> > diff --git a/drivers/spi/spi-fsl-qspi.c b/drivers/spi/spi-fsl-qspi.c
> > index ce45e8e..5d26f73 100644
> > --- a/drivers/spi/spi-fsl-qspi.c
> > +++ b/drivers/spi/spi-fsl-qspi.c
> > @@ -175,6 +175,9 @@
> >   /* TKT245618, the controller cannot wake up from wait mode */
> >   #define QUADSPI_QUIRK_TKT245618BIT(3)
> >
> > +/* QSPI_AMBA_BASE is internally added by SOC design for LS-2.x architecture
> */
> > +

Re: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI controller

2018-11-16 Thread Schrempf Frieder
Hi Yogesh,

On 16.11.18 06:41, Yogesh Narayan Gaur wrote:
> Hi Frieder,
> 
>> -Original Message-
>> From: Schrempf Frieder [mailto:frieder.schre...@kontron.de]
>> Sent: Thursday, November 15, 2018 7:32 PM
>> To: Yogesh Narayan Gaur 
>> Cc: Boris Brezillon ; 
>> linux-...@lists.infradead.org;
>> linux-...@vger.kernel.org; Marek Vasut ; Mark
>> Brown ; Han Xu ;
>> dw...@infradead.org; computersforpe...@gmail.com; rich...@nod.at;
>> miquel.ray...@bootlin.com; David Wolfe ; Fabio
>> Estevam ; Prabhakar Kushwaha
>> ; shawn...@kernel.org; linux-
>> ker...@vger.kernel.org
>> Subject: Re: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI
>> controller
>>
>> Hi Yogesh,
>>
>> On 15.11.18 14:12, Boris Brezillon wrote:
>>> On Thu, 15 Nov 2018 11:43:05 +
>>> Schrempf Frieder  wrote:
>>>
>>>> On 15.11.18 07:22, Yogesh Narayan Gaur wrote:
>>>>> Hi Frieder,
>>>>>
>>>>> With below patch on top of your v5, Read/Write/Erase on CS1 is working
>> fine for me.
>>>>
>>>> Ok, are you sure, that AHB read is working too with this patch?
>>>> You are removing the memmap_phy offset from SFAR and the SFXXAD
>>>> register values.
>>>>
>>>> I can understand that selection of the CS and IP commands will work
>>>> like this, but I can't understand how AHB read should work without
>>>> the base address of the mapped memory.
>>>>
>>>> I'm afraid I still don't fully understand the background of these
>>>> things,
>>>
>>> Same here. Yogesh, can you give us more detail on why you decided to
>>> drop the memmap_phy offset?
>>
>> Your changes do not work on my setup (i.MX6UL). It looks like your hardware 
>> is
>> different.
>>
>> I found this patch for LS2080A: [1]. This would explain why you need to 
>> remove
>> the offset to make it work.
>>
>> To verify this, could you please test your setup with the current spi-nor 
>> driver
>> (fsl_quadspi.c). If our assumptions are right, it should only work on CS0 
>> and CS1
>> with [1] applied.
>>
> 
> Yes, I need to remove the offset to make it work and this is required for the 
> NXP Layerscape-2.x SoCs like LS208x/Ls108x etc.
> 
> I have modified the patch and have introduced entry in quirks for ls2080a. 
> With this Read/Write/Erase are working for me for both CS.

Ok, what I was asking for is a test with the original, unmodified SPI 
NOR driver in mtd/spi-nor/fsl-quadspi.c. We need this to confirm that 
the problem is really what we think, or to find out if we missed something.

Can you please do a quick test? If it confirms our assumptions, I will 
send a new version with the quirk and hopefully we can then move on.

Thanks,
Frieder

> 
> diff --git a/drivers/spi/spi-fsl-qspi.c b/drivers/spi/spi-fsl-qspi.c
> index ce45e8e..5d26f73 100644
> --- a/drivers/spi/spi-fsl-qspi.c
> +++ b/drivers/spi/spi-fsl-qspi.c
> @@ -175,6 +175,9 @@
>   /* TKT245618, the controller cannot wake up from wait mode */
>   #define QUADSPI_QUIRK_TKT245618BIT(3)
> 
> +/* QSPI_AMBA_BASE is internally added by SOC design for LS-2.x architecture 
> */
> +#define QUADSPI_AMBA_BASE_INTERNAL BIT(4)
> +
>   struct fsl_qspi_devtype_data {
>  unsigned int rxfifo;
>  unsigned int txfifo;
> @@ -227,7 +230,7 @@ static const struct fsl_qspi_devtype_data ls2080a_data = {
>  .rxfifo = SZ_128,
>  .txfifo = SZ_64,
>  .ahb_buf_size = SZ_1K,
> -   .quirks = QUADSPI_QUIRK_TKT253890,
> +   .quirks = QUADSPI_QUIRK_TKT253890 | QUADSPI_AMBA_BASE_INTERNAL,
>  .little_endian = true,
>   };
> 
> @@ -235,6 +238,7 @@ struct fsl_qspi {
>  void __iomem *iobase;
>  void __iomem *ahb_addr;
>  u32 memmap_phy;
> +   u32 amba_base_addr;
>  struct clk *clk, *clk_en;
>  struct device *dev;
>  struct completion c;
> @@ -264,6 +268,11 @@ static inline int needs_wakeup_wait_mode(struct fsl_qspi 
> *q)
>  return q->devtype_data->quirks & QUADSPI_QUIRK_TKT245618;
>   }
> 
> +static inline int has_added_amba_base_internal(struct fsl_qspi *q)
> +{
> +   return q->devtype_data->quirks & QUADSPI_AMBA_BASE_INTERNAL;
> +}
> +
>   /*
>* An IC bug makes it necessary to rearrange the 32-bit data.
>* Later chips, such as IMX6SLX, have fixed this bug.
> @@ -489,29 +498,11 @@ static void fsl_qspi_invalidate(struct fsl_qspi *q)
>   static void fsl_qspi_select_mem(struct fsl

Re: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI controller

2018-11-16 Thread Schrempf Frieder
Hi Yogesh,

On 16.11.18 06:41, Yogesh Narayan Gaur wrote:
> Hi Frieder,
> 
>> -Original Message-
>> From: Schrempf Frieder [mailto:frieder.schre...@kontron.de]
>> Sent: Thursday, November 15, 2018 7:32 PM
>> To: Yogesh Narayan Gaur 
>> Cc: Boris Brezillon ; 
>> linux-...@lists.infradead.org;
>> linux-...@vger.kernel.org; Marek Vasut ; Mark
>> Brown ; Han Xu ;
>> dw...@infradead.org; computersforpe...@gmail.com; rich...@nod.at;
>> miquel.ray...@bootlin.com; David Wolfe ; Fabio
>> Estevam ; Prabhakar Kushwaha
>> ; shawn...@kernel.org; linux-
>> ker...@vger.kernel.org
>> Subject: Re: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI
>> controller
>>
>> Hi Yogesh,
>>
>> On 15.11.18 14:12, Boris Brezillon wrote:
>>> On Thu, 15 Nov 2018 11:43:05 +
>>> Schrempf Frieder  wrote:
>>>
>>>> On 15.11.18 07:22, Yogesh Narayan Gaur wrote:
>>>>> Hi Frieder,
>>>>>
>>>>> With below patch on top of your v5, Read/Write/Erase on CS1 is working
>> fine for me.
>>>>
>>>> Ok, are you sure, that AHB read is working too with this patch?
>>>> You are removing the memmap_phy offset from SFAR and the SFXXAD
>>>> register values.
>>>>
>>>> I can understand that selection of the CS and IP commands will work
>>>> like this, but I can't understand how AHB read should work without
>>>> the base address of the mapped memory.
>>>>
>>>> I'm afraid I still don't fully understand the background of these
>>>> things,
>>>
>>> Same here. Yogesh, can you give us more detail on why you decided to
>>> drop the memmap_phy offset?
>>
>> Your changes do not work on my setup (i.MX6UL). It looks like your hardware 
>> is
>> different.
>>
>> I found this patch for LS2080A: [1]. This would explain why you need to 
>> remove
>> the offset to make it work.
>>
>> To verify this, could you please test your setup with the current spi-nor 
>> driver
>> (fsl_quadspi.c). If our assumptions are right, it should only work on CS0 
>> and CS1
>> with [1] applied.
>>
> 
> Yes, I need to remove the offset to make it work and this is required for the 
> NXP Layerscape-2.x SoCs like LS208x/Ls108x etc.
> 
> I have modified the patch and have introduced entry in quirks for ls2080a. 
> With this Read/Write/Erase are working for me for both CS.

Ok, what I was asking for is a test with the original, unmodified SPI 
NOR driver in mtd/spi-nor/fsl-quadspi.c. We need this to confirm that 
the problem is really what we think, or to find out if we missed something.

Can you please do a quick test? If it confirms our assumptions, I will 
send a new version with the quirk and hopefully we can then move on.

Thanks,
Frieder

> 
> diff --git a/drivers/spi/spi-fsl-qspi.c b/drivers/spi/spi-fsl-qspi.c
> index ce45e8e..5d26f73 100644
> --- a/drivers/spi/spi-fsl-qspi.c
> +++ b/drivers/spi/spi-fsl-qspi.c
> @@ -175,6 +175,9 @@
>   /* TKT245618, the controller cannot wake up from wait mode */
>   #define QUADSPI_QUIRK_TKT245618BIT(3)
> 
> +/* QSPI_AMBA_BASE is internally added by SOC design for LS-2.x architecture 
> */
> +#define QUADSPI_AMBA_BASE_INTERNAL BIT(4)
> +
>   struct fsl_qspi_devtype_data {
>  unsigned int rxfifo;
>  unsigned int txfifo;
> @@ -227,7 +230,7 @@ static const struct fsl_qspi_devtype_data ls2080a_data = {
>  .rxfifo = SZ_128,
>  .txfifo = SZ_64,
>  .ahb_buf_size = SZ_1K,
> -   .quirks = QUADSPI_QUIRK_TKT253890,
> +   .quirks = QUADSPI_QUIRK_TKT253890 | QUADSPI_AMBA_BASE_INTERNAL,
>  .little_endian = true,
>   };
> 
> @@ -235,6 +238,7 @@ struct fsl_qspi {
>  void __iomem *iobase;
>  void __iomem *ahb_addr;
>  u32 memmap_phy;
> +   u32 amba_base_addr;
>  struct clk *clk, *clk_en;
>  struct device *dev;
>  struct completion c;
> @@ -264,6 +268,11 @@ static inline int needs_wakeup_wait_mode(struct fsl_qspi 
> *q)
>  return q->devtype_data->quirks & QUADSPI_QUIRK_TKT245618;
>   }
> 
> +static inline int has_added_amba_base_internal(struct fsl_qspi *q)
> +{
> +   return q->devtype_data->quirks & QUADSPI_AMBA_BASE_INTERNAL;
> +}
> +
>   /*
>* An IC bug makes it necessary to rearrange the 32-bit data.
>* Later chips, such as IMX6SLX, have fixed this bug.
> @@ -489,29 +498,11 @@ static void fsl_qspi_invalidate(struct fsl_qspi *q)
>   static void fsl_qspi_select_mem(struct fsl

RE: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI controller

2018-11-15 Thread Yogesh Narayan Gaur
Hi Frieder,

> -Original Message-
> From: Schrempf Frieder [mailto:frieder.schre...@kontron.de]
> Sent: Thursday, November 15, 2018 7:32 PM
> To: Yogesh Narayan Gaur 
> Cc: Boris Brezillon ; 
> linux-...@lists.infradead.org;
> linux-...@vger.kernel.org; Marek Vasut ; Mark
> Brown ; Han Xu ;
> dw...@infradead.org; computersforpe...@gmail.com; rich...@nod.at;
> miquel.ray...@bootlin.com; David Wolfe ; Fabio
> Estevam ; Prabhakar Kushwaha
> ; shawn...@kernel.org; linux-
> ker...@vger.kernel.org
> Subject: Re: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI
> controller
> 
> Hi Yogesh,
> 
> On 15.11.18 14:12, Boris Brezillon wrote:
> > On Thu, 15 Nov 2018 11:43:05 +
> > Schrempf Frieder  wrote:
> >
> >> On 15.11.18 07:22, Yogesh Narayan Gaur wrote:
> >>> Hi Frieder,
> >>>
> >>> With below patch on top of your v5, Read/Write/Erase on CS1 is working
> fine for me.
> >>
> >> Ok, are you sure, that AHB read is working too with this patch?
> >> You are removing the memmap_phy offset from SFAR and the SFXXAD
> >> register values.
> >>
> >> I can understand that selection of the CS and IP commands will work
> >> like this, but I can't understand how AHB read should work without
> >> the base address of the mapped memory.
> >>
> >> I'm afraid I still don't fully understand the background of these
> >> things,
> >
> > Same here. Yogesh, can you give us more detail on why you decided to
> > drop the memmap_phy offset?
> 
> Your changes do not work on my setup (i.MX6UL). It looks like your hardware is
> different.
> 
> I found this patch for LS2080A: [1]. This would explain why you need to remove
> the offset to make it work.
> 
> To verify this, could you please test your setup with the current spi-nor 
> driver
> (fsl_quadspi.c). If our assumptions are right, it should only work on CS0 and 
> CS1
> with [1] applied.
> 

Yes, I need to remove the offset to make it work and this is required for the 
NXP Layerscape-2.x SoCs like LS208x/Ls108x etc.

I have modified the patch and have introduced entry in quirks for ls2080a. With 
this Read/Write/Erase are working for me for both CS.

diff --git a/drivers/spi/spi-fsl-qspi.c b/drivers/spi/spi-fsl-qspi.c
index ce45e8e..5d26f73 100644
--- a/drivers/spi/spi-fsl-qspi.c
+++ b/drivers/spi/spi-fsl-qspi.c
@@ -175,6 +175,9 @@
 /* TKT245618, the controller cannot wake up from wait mode */
 #define QUADSPI_QUIRK_TKT245618BIT(3)

+/* QSPI_AMBA_BASE is internally added by SOC design for LS-2.x architecture */
+#define QUADSPI_AMBA_BASE_INTERNAL BIT(4)
+
 struct fsl_qspi_devtype_data {
unsigned int rxfifo;
unsigned int txfifo;
@@ -227,7 +230,7 @@ static const struct fsl_qspi_devtype_data ls2080a_data = {
.rxfifo = SZ_128,
.txfifo = SZ_64,
.ahb_buf_size = SZ_1K,
-   .quirks = QUADSPI_QUIRK_TKT253890,
+   .quirks = QUADSPI_QUIRK_TKT253890 | QUADSPI_AMBA_BASE_INTERNAL,
.little_endian = true,
 };

@@ -235,6 +238,7 @@ struct fsl_qspi {
void __iomem *iobase;
void __iomem *ahb_addr;
u32 memmap_phy;
+   u32 amba_base_addr;
struct clk *clk, *clk_en;
struct device *dev;
struct completion c;
@@ -264,6 +268,11 @@ static inline int needs_wakeup_wait_mode(struct fsl_qspi 
*q)
return q->devtype_data->quirks & QUADSPI_QUIRK_TKT245618;
 }

+static inline int has_added_amba_base_internal(struct fsl_qspi *q)
+{
+   return q->devtype_data->quirks & QUADSPI_AMBA_BASE_INTERNAL;
+}
+
 /*
  * An IC bug makes it necessary to rearrange the 32-bit data.
  * Later chips, such as IMX6SLX, have fixed this bug.
@@ -489,29 +498,11 @@ static void fsl_qspi_invalidate(struct fsl_qspi *q)
 static void fsl_qspi_select_mem(struct fsl_qspi *q, struct spi_device *spi)
 {
unsigned long rate = spi->max_speed_hz;
-   int ret, i;
-   u32 map_addr;
+   int ret;

if (q->selected == spi->chip_select)
return;

-   /*
-* In HW there can be a maximum of four chips on two buses with
-* two chip selects on each bus. We use four chip selects in SW
-* to differentiate between the four chips.
-* We use the SFA1AD, SFA2AD, SFB1AD, SFB2AD registers to select
-* the chip we want to access.
-*/
-   for (i = 0; i < 4; i++) {
-   if (i < spi->chip_select)
-   map_addr = q->memmap_phy;
-   else
-   map_addr = q->memmap_phy +
-  2 * q->devtype_data->ahb_buf_size;
-
-   qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD + (i * 4));
-   }
-
  

RE: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI controller

2018-11-15 Thread Yogesh Narayan Gaur
Hi Frieder,

> -Original Message-
> From: Schrempf Frieder [mailto:frieder.schre...@kontron.de]
> Sent: Thursday, November 15, 2018 7:32 PM
> To: Yogesh Narayan Gaur 
> Cc: Boris Brezillon ; 
> linux-...@lists.infradead.org;
> linux-...@vger.kernel.org; Marek Vasut ; Mark
> Brown ; Han Xu ;
> dw...@infradead.org; computersforpe...@gmail.com; rich...@nod.at;
> miquel.ray...@bootlin.com; David Wolfe ; Fabio
> Estevam ; Prabhakar Kushwaha
> ; shawn...@kernel.org; linux-
> ker...@vger.kernel.org
> Subject: Re: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI
> controller
> 
> Hi Yogesh,
> 
> On 15.11.18 14:12, Boris Brezillon wrote:
> > On Thu, 15 Nov 2018 11:43:05 +
> > Schrempf Frieder  wrote:
> >
> >> On 15.11.18 07:22, Yogesh Narayan Gaur wrote:
> >>> Hi Frieder,
> >>>
> >>> With below patch on top of your v5, Read/Write/Erase on CS1 is working
> fine for me.
> >>
> >> Ok, are you sure, that AHB read is working too with this patch?
> >> You are removing the memmap_phy offset from SFAR and the SFXXAD
> >> register values.
> >>
> >> I can understand that selection of the CS and IP commands will work
> >> like this, but I can't understand how AHB read should work without
> >> the base address of the mapped memory.
> >>
> >> I'm afraid I still don't fully understand the background of these
> >> things,
> >
> > Same here. Yogesh, can you give us more detail on why you decided to
> > drop the memmap_phy offset?
> 
> Your changes do not work on my setup (i.MX6UL). It looks like your hardware is
> different.
> 
> I found this patch for LS2080A: [1]. This would explain why you need to remove
> the offset to make it work.
> 
> To verify this, could you please test your setup with the current spi-nor 
> driver
> (fsl_quadspi.c). If our assumptions are right, it should only work on CS0 and 
> CS1
> with [1] applied.
> 

Yes, I need to remove the offset to make it work and this is required for the 
NXP Layerscape-2.x SoCs like LS208x/Ls108x etc.

I have modified the patch and have introduced entry in quirks for ls2080a. With 
this Read/Write/Erase are working for me for both CS.

diff --git a/drivers/spi/spi-fsl-qspi.c b/drivers/spi/spi-fsl-qspi.c
index ce45e8e..5d26f73 100644
--- a/drivers/spi/spi-fsl-qspi.c
+++ b/drivers/spi/spi-fsl-qspi.c
@@ -175,6 +175,9 @@
 /* TKT245618, the controller cannot wake up from wait mode */
 #define QUADSPI_QUIRK_TKT245618BIT(3)

+/* QSPI_AMBA_BASE is internally added by SOC design for LS-2.x architecture */
+#define QUADSPI_AMBA_BASE_INTERNAL BIT(4)
+
 struct fsl_qspi_devtype_data {
unsigned int rxfifo;
unsigned int txfifo;
@@ -227,7 +230,7 @@ static const struct fsl_qspi_devtype_data ls2080a_data = {
.rxfifo = SZ_128,
.txfifo = SZ_64,
.ahb_buf_size = SZ_1K,
-   .quirks = QUADSPI_QUIRK_TKT253890,
+   .quirks = QUADSPI_QUIRK_TKT253890 | QUADSPI_AMBA_BASE_INTERNAL,
.little_endian = true,
 };

@@ -235,6 +238,7 @@ struct fsl_qspi {
void __iomem *iobase;
void __iomem *ahb_addr;
u32 memmap_phy;
+   u32 amba_base_addr;
struct clk *clk, *clk_en;
struct device *dev;
struct completion c;
@@ -264,6 +268,11 @@ static inline int needs_wakeup_wait_mode(struct fsl_qspi 
*q)
return q->devtype_data->quirks & QUADSPI_QUIRK_TKT245618;
 }

+static inline int has_added_amba_base_internal(struct fsl_qspi *q)
+{
+   return q->devtype_data->quirks & QUADSPI_AMBA_BASE_INTERNAL;
+}
+
 /*
  * An IC bug makes it necessary to rearrange the 32-bit data.
  * Later chips, such as IMX6SLX, have fixed this bug.
@@ -489,29 +498,11 @@ static void fsl_qspi_invalidate(struct fsl_qspi *q)
 static void fsl_qspi_select_mem(struct fsl_qspi *q, struct spi_device *spi)
 {
unsigned long rate = spi->max_speed_hz;
-   int ret, i;
-   u32 map_addr;
+   int ret;

if (q->selected == spi->chip_select)
return;

-   /*
-* In HW there can be a maximum of four chips on two buses with
-* two chip selects on each bus. We use four chip selects in SW
-* to differentiate between the four chips.
-* We use the SFA1AD, SFA2AD, SFB1AD, SFB2AD registers to select
-* the chip we want to access.
-*/
-   for (i = 0; i < 4; i++) {
-   if (i < spi->chip_select)
-   map_addr = q->memmap_phy;
-   else
-   map_addr = q->memmap_phy +
-  2 * q->devtype_data->ahb_buf_size;
-
-   qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD + (i * 4));
-   }
-
  

Re: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI controller

2018-11-15 Thread Schrempf Frieder
Hi Yogesh,

On 15.11.18 14:12, Boris Brezillon wrote:
> On Thu, 15 Nov 2018 11:43:05 +
> Schrempf Frieder  wrote:
> 
>> On 15.11.18 07:22, Yogesh Narayan Gaur wrote:
>>> Hi Frieder,
>>>
>>> With below patch on top of your v5, Read/Write/Erase on CS1 is working fine 
>>> for me.
>>
>> Ok, are you sure, that AHB read is working too with this patch?
>> You are removing the memmap_phy offset from SFAR and the SFXXAD register
>> values.
>>
>> I can understand that selection of the CS and IP commands will work like
>> this, but I can't understand how AHB read should work without the base
>> address of the mapped memory.
>>
>> I'm afraid I still don't fully understand the background of these
>> things,
> 
> Same here. Yogesh, can you give us more detail on why you decided to
> drop the memmap_phy offset?

Your changes do not work on my setup (i.MX6UL). It looks like your 
hardware is different.

I found this patch for LS2080A: [1]. This would explain why you need to 
remove the offset to make it work.

To verify this, could you please test your setup with the current 
spi-nor driver (fsl_quadspi.c). If our assumptions are right, it should 
only work on CS0 and CS1 with [1] applied.

Thanks,
Frieder

[1]: http://patchwork.ozlabs.org/patch/660364/

Re: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI controller

2018-11-15 Thread Schrempf Frieder
Hi Yogesh,

On 15.11.18 14:12, Boris Brezillon wrote:
> On Thu, 15 Nov 2018 11:43:05 +
> Schrempf Frieder  wrote:
> 
>> On 15.11.18 07:22, Yogesh Narayan Gaur wrote:
>>> Hi Frieder,
>>>
>>> With below patch on top of your v5, Read/Write/Erase on CS1 is working fine 
>>> for me.
>>
>> Ok, are you sure, that AHB read is working too with this patch?
>> You are removing the memmap_phy offset from SFAR and the SFXXAD register
>> values.
>>
>> I can understand that selection of the CS and IP commands will work like
>> this, but I can't understand how AHB read should work without the base
>> address of the mapped memory.
>>
>> I'm afraid I still don't fully understand the background of these
>> things,
> 
> Same here. Yogesh, can you give us more detail on why you decided to
> drop the memmap_phy offset?

Your changes do not work on my setup (i.MX6UL). It looks like your 
hardware is different.

I found this patch for LS2080A: [1]. This would explain why you need to 
remove the offset to make it work.

To verify this, could you please test your setup with the current 
spi-nor driver (fsl_quadspi.c). If our assumptions are right, it should 
only work on CS0 and CS1 with [1] applied.

Thanks,
Frieder

[1]: http://patchwork.ozlabs.org/patch/660364/

Re: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI controller

2018-11-15 Thread Boris Brezillon
On Thu, 15 Nov 2018 11:43:05 +
Schrempf Frieder  wrote:

> On 15.11.18 07:22, Yogesh Narayan Gaur wrote:
> > Hi Frieder,
> > 
> > With below patch on top of your v5, Read/Write/Erase on CS1 is working fine 
> > for me.  
> 
> Ok, are you sure, that AHB read is working too with this patch?
> You are removing the memmap_phy offset from SFAR and the SFXXAD register 
> values.
> 
> I can understand that selection of the CS and IP commands will work like 
> this, but I can't understand how AHB read should work without the base 
> address of the mapped memory.
> 
> I'm afraid I still don't fully understand the background of these 
> things,

Same here. Yogesh, can you give us more detail on why you decided to
drop the memmap_phy offset?

> but still thank you for testing.
> 


Re: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI controller

2018-11-15 Thread Boris Brezillon
On Thu, 15 Nov 2018 11:43:05 +
Schrempf Frieder  wrote:

> On 15.11.18 07:22, Yogesh Narayan Gaur wrote:
> > Hi Frieder,
> > 
> > With below patch on top of your v5, Read/Write/Erase on CS1 is working fine 
> > for me.  
> 
> Ok, are you sure, that AHB read is working too with this patch?
> You are removing the memmap_phy offset from SFAR and the SFXXAD register 
> values.
> 
> I can understand that selection of the CS and IP commands will work like 
> this, but I can't understand how AHB read should work without the base 
> address of the mapped memory.
> 
> I'm afraid I still don't fully understand the background of these 
> things,

Same here. Yogesh, can you give us more detail on why you decided to
drop the memmap_phy offset?

> but still thank you for testing.
> 


Re: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI controller

2018-11-15 Thread Schrempf Frieder
On 15.11.18 07:22, Yogesh Narayan Gaur wrote:
> Hi Frieder,
> 
> With below patch on top of your v5, Read/Write/Erase on CS1 is working fine 
> for me.

Ok, are you sure, that AHB read is working too with this patch?
You are removing the memmap_phy offset from SFAR and the SFXXAD register 
values.

I can understand that selection of the CS and IP commands will work like 
this, but I can't understand how AHB read should work without the base 
address of the mapped memory.

I'm afraid I still don't fully understand the background of these 
things, but still thank you for testing.

> 
> I have tested with JFFS2 mounting and booting also for both CS0 and CS1.
> 
> diff --git a/drivers/spi/spi-fsl-qspi.c b/drivers/spi/spi-fsl-qspi.c
> index ce45e8e..4467983 100644
> --- a/drivers/spi/spi-fsl-qspi.c
> +++ b/drivers/spi/spi-fsl-qspi.c
> @@ -490,28 +490,10 @@ static void fsl_qspi_select_mem(struct fsl_qspi *q, 
> struct spi_device *spi)
>   {
>  unsigned long rate = spi->max_speed_hz;
>  int ret, i;
> -   u32 map_addr;
> 
>  if (q->selected == spi->chip_select)
>  return;
> 
> -   /*
> -* In HW there can be a maximum of four chips on two buses with
> -* two chip selects on each bus. We use four chip selects in SW
> -* to differentiate between the four chips.
> -* We use the SFA1AD, SFA2AD, SFB1AD, SFB2AD registers to select
> -* the chip we want to access.
> -*/
> -   for (i = 0; i < 4; i++) {
> -   if (i < spi->chip_select)
> -   map_addr = q->memmap_phy;
> -   else
> -   map_addr = q->memmap_phy +
> -  2 * q->devtype_data->ahb_buf_size;
> -
> -   qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD + (i * 
> 4));
> -   }
> -
>  if (needs_4x_clock(q))
>  rate *= 4;
> 
> @@ -534,7 +516,9 @@ static void fsl_qspi_select_mem(struct fsl_qspi *q, 
> struct spi_device *spi)
> 
>   static void fsl_qspi_read_ahb(struct fsl_qspi *q, const struct spi_mem_op 
> *op)
>   {
> -   memcpy_fromio(op->data.buf.in, q->ahb_addr, op->data.nbytes);
> +   memcpy_fromio(op->data.buf.in,
> + q->ahb_addr + q->selected * 
> q->devtype_data->ahb_buf_size,
> + op->data.nbytes);
>   }
> 
>   static void fsl_qspi_fill_txfifo(struct fsl_qspi *q,
> @@ -634,7 +618,9 @@ static int fsl_qspi_exec_op(struct spi_mem *mem, const 
> struct spi_mem_op *op)
> 
>  fsl_qspi_select_mem(q, mem->spi);
> 
> -   qspi_writel(q, q->memmap_phy, base + QUADSPI_SFAR);
> +   qspi_writel(q,
> +   q->selected * q->devtype_data->ahb_buf_size,
> +   base + QUADSPI_SFAR);
> 
>  qspi_writel(q, qspi_readl(q, base + QUADSPI_MCR) |
>  QUADSPI_MCR_CLR_RXF_MASK | QUADSPI_MCR_CLR_TXF_MASK,
> @@ -733,6 +719,19 @@ static int fsl_qspi_default_setup(struct fsl_qspi *q)
>  QUADSPI_BUF3CR_ADATSZ(q->devtype_data->ahb_buf_size / 8),
>  base + QUADSPI_BUF3CR);
> 
> +   /*
> +* In HW there can be a maximum of four chips on two buses with
> +* two chip selects on each bus. We use four chip selects in SW
> +* to differentiate between the four chips.
> +* We use the SFA1AD, SFA2AD, SFB1AD, SFB2AD registers to select
> +* the chip we want to access.
> +*/
> +
> +   qspi_writel(q, q->devtype_data->ahb_buf_size, base + QUADSPI_SFA1AD);
> +   qspi_writel(q, q->devtype_data->ahb_buf_size * 2 , base + 
> QUADSPI_SFA2AD);
> +   qspi_writel(q, q->devtype_data->ahb_buf_size * 3 , base + 
> QUADSPI_SFB1AD);
> +   qspi_writel(q, q->devtype_data->ahb_buf_size * 4 , base + 
> QUADSPI_SFB2AD);
> +
>  q->selected = -1;
> 
> --
> Regards
> Yogesh Gaur
> 
> [..]
> 

Re: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI controller

2018-11-15 Thread Schrempf Frieder
On 15.11.18 07:22, Yogesh Narayan Gaur wrote:
> Hi Frieder,
> 
> With below patch on top of your v5, Read/Write/Erase on CS1 is working fine 
> for me.

Ok, are you sure, that AHB read is working too with this patch?
You are removing the memmap_phy offset from SFAR and the SFXXAD register 
values.

I can understand that selection of the CS and IP commands will work like 
this, but I can't understand how AHB read should work without the base 
address of the mapped memory.

I'm afraid I still don't fully understand the background of these 
things, but still thank you for testing.

> 
> I have tested with JFFS2 mounting and booting also for both CS0 and CS1.
> 
> diff --git a/drivers/spi/spi-fsl-qspi.c b/drivers/spi/spi-fsl-qspi.c
> index ce45e8e..4467983 100644
> --- a/drivers/spi/spi-fsl-qspi.c
> +++ b/drivers/spi/spi-fsl-qspi.c
> @@ -490,28 +490,10 @@ static void fsl_qspi_select_mem(struct fsl_qspi *q, 
> struct spi_device *spi)
>   {
>  unsigned long rate = spi->max_speed_hz;
>  int ret, i;
> -   u32 map_addr;
> 
>  if (q->selected == spi->chip_select)
>  return;
> 
> -   /*
> -* In HW there can be a maximum of four chips on two buses with
> -* two chip selects on each bus. We use four chip selects in SW
> -* to differentiate between the four chips.
> -* We use the SFA1AD, SFA2AD, SFB1AD, SFB2AD registers to select
> -* the chip we want to access.
> -*/
> -   for (i = 0; i < 4; i++) {
> -   if (i < spi->chip_select)
> -   map_addr = q->memmap_phy;
> -   else
> -   map_addr = q->memmap_phy +
> -  2 * q->devtype_data->ahb_buf_size;
> -
> -   qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD + (i * 
> 4));
> -   }
> -
>  if (needs_4x_clock(q))
>  rate *= 4;
> 
> @@ -534,7 +516,9 @@ static void fsl_qspi_select_mem(struct fsl_qspi *q, 
> struct spi_device *spi)
> 
>   static void fsl_qspi_read_ahb(struct fsl_qspi *q, const struct spi_mem_op 
> *op)
>   {
> -   memcpy_fromio(op->data.buf.in, q->ahb_addr, op->data.nbytes);
> +   memcpy_fromio(op->data.buf.in,
> + q->ahb_addr + q->selected * 
> q->devtype_data->ahb_buf_size,
> + op->data.nbytes);
>   }
> 
>   static void fsl_qspi_fill_txfifo(struct fsl_qspi *q,
> @@ -634,7 +618,9 @@ static int fsl_qspi_exec_op(struct spi_mem *mem, const 
> struct spi_mem_op *op)
> 
>  fsl_qspi_select_mem(q, mem->spi);
> 
> -   qspi_writel(q, q->memmap_phy, base + QUADSPI_SFAR);
> +   qspi_writel(q,
> +   q->selected * q->devtype_data->ahb_buf_size,
> +   base + QUADSPI_SFAR);
> 
>  qspi_writel(q, qspi_readl(q, base + QUADSPI_MCR) |
>  QUADSPI_MCR_CLR_RXF_MASK | QUADSPI_MCR_CLR_TXF_MASK,
> @@ -733,6 +719,19 @@ static int fsl_qspi_default_setup(struct fsl_qspi *q)
>  QUADSPI_BUF3CR_ADATSZ(q->devtype_data->ahb_buf_size / 8),
>  base + QUADSPI_BUF3CR);
> 
> +   /*
> +* In HW there can be a maximum of four chips on two buses with
> +* two chip selects on each bus. We use four chip selects in SW
> +* to differentiate between the four chips.
> +* We use the SFA1AD, SFA2AD, SFB1AD, SFB2AD registers to select
> +* the chip we want to access.
> +*/
> +
> +   qspi_writel(q, q->devtype_data->ahb_buf_size, base + QUADSPI_SFA1AD);
> +   qspi_writel(q, q->devtype_data->ahb_buf_size * 2 , base + 
> QUADSPI_SFA2AD);
> +   qspi_writel(q, q->devtype_data->ahb_buf_size * 3 , base + 
> QUADSPI_SFB1AD);
> +   qspi_writel(q, q->devtype_data->ahb_buf_size * 4 , base + 
> QUADSPI_SFB2AD);
> +
>  q->selected = -1;
> 
> --
> Regards
> Yogesh Gaur
> 
> [..]
> 

RE: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI controller

2018-11-14 Thread Yogesh Narayan Gaur
Hi Frieder,

With below patch on top of your v5, Read/Write/Erase on CS1 is working fine for 
me.

I have tested with JFFS2 mounting and booting also for both CS0 and CS1.

diff --git a/drivers/spi/spi-fsl-qspi.c b/drivers/spi/spi-fsl-qspi.c
index ce45e8e..4467983 100644
--- a/drivers/spi/spi-fsl-qspi.c
+++ b/drivers/spi/spi-fsl-qspi.c
@@ -490,28 +490,10 @@ static void fsl_qspi_select_mem(struct fsl_qspi *q, 
struct spi_device *spi)
 {
unsigned long rate = spi->max_speed_hz;
int ret, i;
-   u32 map_addr;

if (q->selected == spi->chip_select)
return;

-   /*
-* In HW there can be a maximum of four chips on two buses with
-* two chip selects on each bus. We use four chip selects in SW
-* to differentiate between the four chips.
-* We use the SFA1AD, SFA2AD, SFB1AD, SFB2AD registers to select
-* the chip we want to access.
-*/
-   for (i = 0; i < 4; i++) {
-   if (i < spi->chip_select)
-   map_addr = q->memmap_phy;
-   else
-   map_addr = q->memmap_phy +
-  2 * q->devtype_data->ahb_buf_size;
-
-   qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD + (i * 4));
-   }
-
if (needs_4x_clock(q))
rate *= 4;

@@ -534,7 +516,9 @@ static void fsl_qspi_select_mem(struct fsl_qspi *q, struct 
spi_device *spi)

 static void fsl_qspi_read_ahb(struct fsl_qspi *q, const struct spi_mem_op *op)
 {
-   memcpy_fromio(op->data.buf.in, q->ahb_addr, op->data.nbytes);
+   memcpy_fromio(op->data.buf.in,
+ q->ahb_addr + q->selected * q->devtype_data->ahb_buf_size,
+ op->data.nbytes);
 }

 static void fsl_qspi_fill_txfifo(struct fsl_qspi *q,
@@ -634,7 +618,9 @@ static int fsl_qspi_exec_op(struct spi_mem *mem, const 
struct spi_mem_op *op)

fsl_qspi_select_mem(q, mem->spi);

-   qspi_writel(q, q->memmap_phy, base + QUADSPI_SFAR);
+   qspi_writel(q,
+   q->selected * q->devtype_data->ahb_buf_size,
+   base + QUADSPI_SFAR);

qspi_writel(q, qspi_readl(q, base + QUADSPI_MCR) |
QUADSPI_MCR_CLR_RXF_MASK | QUADSPI_MCR_CLR_TXF_MASK,
@@ -733,6 +719,19 @@ static int fsl_qspi_default_setup(struct fsl_qspi *q)
QUADSPI_BUF3CR_ADATSZ(q->devtype_data->ahb_buf_size / 8),
base + QUADSPI_BUF3CR);

+   /*
+* In HW there can be a maximum of four chips on two buses with
+* two chip selects on each bus. We use four chip selects in SW
+* to differentiate between the four chips.
+* We use the SFA1AD, SFA2AD, SFB1AD, SFB2AD registers to select
+* the chip we want to access.
+*/
+
+   qspi_writel(q, q->devtype_data->ahb_buf_size, base + QUADSPI_SFA1AD);
+   qspi_writel(q, q->devtype_data->ahb_buf_size * 2 , base + 
QUADSPI_SFA2AD);
+   qspi_writel(q, q->devtype_data->ahb_buf_size * 3 , base + 
QUADSPI_SFB1AD);
+   qspi_writel(q, q->devtype_data->ahb_buf_size * 4 , base + 
QUADSPI_SFB2AD);
+
q->selected = -1;

--
Regards
Yogesh Gaur

[..]


RE: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI controller

2018-11-14 Thread Yogesh Narayan Gaur
Hi Frieder,

With below patch on top of your v5, Read/Write/Erase on CS1 is working fine for 
me.

I have tested with JFFS2 mounting and booting also for both CS0 and CS1.

diff --git a/drivers/spi/spi-fsl-qspi.c b/drivers/spi/spi-fsl-qspi.c
index ce45e8e..4467983 100644
--- a/drivers/spi/spi-fsl-qspi.c
+++ b/drivers/spi/spi-fsl-qspi.c
@@ -490,28 +490,10 @@ static void fsl_qspi_select_mem(struct fsl_qspi *q, 
struct spi_device *spi)
 {
unsigned long rate = spi->max_speed_hz;
int ret, i;
-   u32 map_addr;

if (q->selected == spi->chip_select)
return;

-   /*
-* In HW there can be a maximum of four chips on two buses with
-* two chip selects on each bus. We use four chip selects in SW
-* to differentiate between the four chips.
-* We use the SFA1AD, SFA2AD, SFB1AD, SFB2AD registers to select
-* the chip we want to access.
-*/
-   for (i = 0; i < 4; i++) {
-   if (i < spi->chip_select)
-   map_addr = q->memmap_phy;
-   else
-   map_addr = q->memmap_phy +
-  2 * q->devtype_data->ahb_buf_size;
-
-   qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD + (i * 4));
-   }
-
if (needs_4x_clock(q))
rate *= 4;

@@ -534,7 +516,9 @@ static void fsl_qspi_select_mem(struct fsl_qspi *q, struct 
spi_device *spi)

 static void fsl_qspi_read_ahb(struct fsl_qspi *q, const struct spi_mem_op *op)
 {
-   memcpy_fromio(op->data.buf.in, q->ahb_addr, op->data.nbytes);
+   memcpy_fromio(op->data.buf.in,
+ q->ahb_addr + q->selected * q->devtype_data->ahb_buf_size,
+ op->data.nbytes);
 }

 static void fsl_qspi_fill_txfifo(struct fsl_qspi *q,
@@ -634,7 +618,9 @@ static int fsl_qspi_exec_op(struct spi_mem *mem, const 
struct spi_mem_op *op)

fsl_qspi_select_mem(q, mem->spi);

-   qspi_writel(q, q->memmap_phy, base + QUADSPI_SFAR);
+   qspi_writel(q,
+   q->selected * q->devtype_data->ahb_buf_size,
+   base + QUADSPI_SFAR);

qspi_writel(q, qspi_readl(q, base + QUADSPI_MCR) |
QUADSPI_MCR_CLR_RXF_MASK | QUADSPI_MCR_CLR_TXF_MASK,
@@ -733,6 +719,19 @@ static int fsl_qspi_default_setup(struct fsl_qspi *q)
QUADSPI_BUF3CR_ADATSZ(q->devtype_data->ahb_buf_size / 8),
base + QUADSPI_BUF3CR);

+   /*
+* In HW there can be a maximum of four chips on two buses with
+* two chip selects on each bus. We use four chip selects in SW
+* to differentiate between the four chips.
+* We use the SFA1AD, SFA2AD, SFB1AD, SFB2AD registers to select
+* the chip we want to access.
+*/
+
+   qspi_writel(q, q->devtype_data->ahb_buf_size, base + QUADSPI_SFA1AD);
+   qspi_writel(q, q->devtype_data->ahb_buf_size * 2 , base + 
QUADSPI_SFA2AD);
+   qspi_writel(q, q->devtype_data->ahb_buf_size * 3 , base + 
QUADSPI_SFB1AD);
+   qspi_writel(q, q->devtype_data->ahb_buf_size * 4 , base + 
QUADSPI_SFB2AD);
+
q->selected = -1;

--
Regards
Yogesh Gaur

[..]


Re: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI controller

2018-11-14 Thread Schrempf Frieder


On 14.11.18 11:43, Yogesh Narayan Gaur wrote:
> Hi Frieder,
> 
> [..]
>>>
>>> Ok, I will have a look at what could make the chip selection fail in
>>> case of AHB read.
>>
>> Could you try with this change applied:
>>
>> @@ -503,7 +503,7 @@ static void fsl_qspi_select_mem(struct fsl_qspi *q, 
>> struct
>> spi_device *spi)
>>   map_addr = q->memmap_phy;
>>   else
>>   map_addr = q->memmap_phy +
>> -  2 * q->devtype_data->ahb_buf_size;
>> +  q->devtype_data->ahb_buf_size;
>>
>>   qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD +
>> (i * 4));
>>   }
>>
> 
> I have tried above change and also have done few more changes but still AHB 
> read for CS1 is falling.

Maybe CS1 is not selected because we reuse the same memory offset as for 
CS0. Can you try with a fixed mapping for all four CS lines, like this: 
https://paste.ee/p/mYwjP?

Thanks,
Frieder

Re: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI controller

2018-11-14 Thread Schrempf Frieder


On 14.11.18 11:43, Yogesh Narayan Gaur wrote:
> Hi Frieder,
> 
> [..]
>>>
>>> Ok, I will have a look at what could make the chip selection fail in
>>> case of AHB read.
>>
>> Could you try with this change applied:
>>
>> @@ -503,7 +503,7 @@ static void fsl_qspi_select_mem(struct fsl_qspi *q, 
>> struct
>> spi_device *spi)
>>   map_addr = q->memmap_phy;
>>   else
>>   map_addr = q->memmap_phy +
>> -  2 * q->devtype_data->ahb_buf_size;
>> +  q->devtype_data->ahb_buf_size;
>>
>>   qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD +
>> (i * 4));
>>   }
>>
> 
> I have tried above change and also have done few more changes but still AHB 
> read for CS1 is falling.

Maybe CS1 is not selected because we reuse the same memory offset as for 
CS0. Can you try with a fixed mapping for all four CS lines, like this: 
https://paste.ee/p/mYwjP?

Thanks,
Frieder

Re: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI controller

2018-11-14 Thread Boris Brezillon
On Wed, 14 Nov 2018 10:43:00 +
Yogesh Narayan Gaur  wrote:

> Hi Frieder,
> 
> [..]
> > >
> > > Ok, I will have a look at what could make the chip selection fail in
> > > case of AHB read.  
> > 
> > Could you try with this change applied:
> > 
> > @@ -503,7 +503,7 @@ static void fsl_qspi_select_mem(struct fsl_qspi *q, 
> > struct
> > spi_device *spi)
> >  map_addr = q->memmap_phy;
> >  else
> >  map_addr = q->memmap_phy +
> > -  2 * q->devtype_data->ahb_buf_size;
> > +  q->devtype_data->ahb_buf_size;
> > 
> >  qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD +
> > (i * 4));
> >  }
> >   
> 
> I have tried above change and also have done few more changes but still AHB 
> read for CS1 is falling.

Have plugged a scope on the CS1 line, to make sure it's properly
asserted when the memory is accessed?

> 
> I guess we need to implement dynamic memory mapping [1] for AHB Read as was 
> being done in previous driver implementation.
> Would try this and update you.

Sorry but I don't see why it would solve the problem we have here, but
if it does, I'd like to have a clear explanation ;-).


Re: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI controller

2018-11-14 Thread Boris Brezillon
On Wed, 14 Nov 2018 10:43:00 +
Yogesh Narayan Gaur  wrote:

> Hi Frieder,
> 
> [..]
> > >
> > > Ok, I will have a look at what could make the chip selection fail in
> > > case of AHB read.  
> > 
> > Could you try with this change applied:
> > 
> > @@ -503,7 +503,7 @@ static void fsl_qspi_select_mem(struct fsl_qspi *q, 
> > struct
> > spi_device *spi)
> >  map_addr = q->memmap_phy;
> >  else
> >  map_addr = q->memmap_phy +
> > -  2 * q->devtype_data->ahb_buf_size;
> > +  q->devtype_data->ahb_buf_size;
> > 
> >  qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD +
> > (i * 4));
> >  }
> >   
> 
> I have tried above change and also have done few more changes but still AHB 
> read for CS1 is falling.

Have plugged a scope on the CS1 line, to make sure it's properly
asserted when the memory is accessed?

> 
> I guess we need to implement dynamic memory mapping [1] for AHB Read as was 
> being done in previous driver implementation.
> Would try this and update you.

Sorry but I don't see why it would solve the problem we have here, but
if it does, I'd like to have a clear explanation ;-).


RE: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI controller

2018-11-14 Thread Yogesh Narayan Gaur
Hi Frieder,

[..]
> >
> > Ok, I will have a look at what could make the chip selection fail in
> > case of AHB read.
> 
> Could you try with this change applied:
> 
> @@ -503,7 +503,7 @@ static void fsl_qspi_select_mem(struct fsl_qspi *q, struct
> spi_device *spi)
>  map_addr = q->memmap_phy;
>  else
>  map_addr = q->memmap_phy +
> -  2 * q->devtype_data->ahb_buf_size;
> +  q->devtype_data->ahb_buf_size;
> 
>  qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD +
> (i * 4));
>  }
> 

I have tried above change and also have done few more changes but still AHB 
read for CS1 is falling.

I guess we need to implement dynamic memory mapping [1] for AHB Read as was 
being done in previous driver implementation.
Would try this and update you.

[1] https://patchwork.ozlabs.org/patch/503655/

--
Regards
Yogesh Gaur
[..]


RE: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI controller

2018-11-14 Thread Yogesh Narayan Gaur
Hi Frieder,

[..]
> >
> > Ok, I will have a look at what could make the chip selection fail in
> > case of AHB read.
> 
> Could you try with this change applied:
> 
> @@ -503,7 +503,7 @@ static void fsl_qspi_select_mem(struct fsl_qspi *q, struct
> spi_device *spi)
>  map_addr = q->memmap_phy;
>  else
>  map_addr = q->memmap_phy +
> -  2 * q->devtype_data->ahb_buf_size;
> +  q->devtype_data->ahb_buf_size;
> 
>  qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD +
> (i * 4));
>  }
> 

I have tried above change and also have done few more changes but still AHB 
read for CS1 is falling.

I guess we need to implement dynamic memory mapping [1] for AHB Read as was 
being done in previous driver implementation.
Would try this and update you.

[1] https://patchwork.ozlabs.org/patch/503655/

--
Regards
Yogesh Gaur
[..]


Re: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI controller

2018-11-14 Thread Schrempf Frieder
Hi Yogesh,

On 14.11.18 09:50, Frieder Schrempf wrote:
> Hi Yogesh,
> 
> On 14.11.18 09:39, Yogesh Narayan Gaur wrote:
>> Hi Frieder,
>>
>> I have tried v5 version of the patch and have observed that Read is 
>> failing for CS1.
> 
> Thanks a lot for doing the test. I really appreciate it.
> 
>> In my target 2 flash devices are connected on same bus i.e. A1 -> CS0 
>> and A2 -> CS1.
>>
>> On initial debugging, I figured that Read is failing for the AHB mode 
>> i.e. if I attempt to read data size less than rxfifo read is working 
>> fine without any issue.
>>
>> For data size more than rxfifo Read out data is content of same 
>> requested address of CS0.
>> mtd_debug read /dev/mtd1 0xf0 0x70 read --> Data is correct
>> mtd_debug read /dev/mtd1 0xf0 0x100 read --> Data is 
>> in-correct and data content are of the address 0xf0 of CS0 
>> connected flash device.
> 
> Ok, I will have a look at what could make the chip selection fail in 
> case of AHB read.

Could you try with this change applied:

@@ -503,7 +503,7 @@ static void fsl_qspi_select_mem(struct fsl_qspi *q, 
struct spi_device *spi)
 map_addr = q->memmap_phy;
 else
 map_addr = q->memmap_phy +
-  2 * q->devtype_data->ahb_buf_size;
+  q->devtype_data->ahb_buf_size;

 qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD + 
(i * 4));
 }

> 
>> On the setup where you have done testing, did AHB mode read is being 
>> verified for CS1?
> 
> No, I currently have only hardware with CS0 connected. So I didn't test 
> with CS1.
> 
>>
>> I am doing further debugging of this issue.
> 
> Thanks,
> Frieder

Re: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI controller

2018-11-14 Thread Schrempf Frieder
Hi Yogesh,

On 14.11.18 09:50, Frieder Schrempf wrote:
> Hi Yogesh,
> 
> On 14.11.18 09:39, Yogesh Narayan Gaur wrote:
>> Hi Frieder,
>>
>> I have tried v5 version of the patch and have observed that Read is 
>> failing for CS1.
> 
> Thanks a lot for doing the test. I really appreciate it.
> 
>> In my target 2 flash devices are connected on same bus i.e. A1 -> CS0 
>> and A2 -> CS1.
>>
>> On initial debugging, I figured that Read is failing for the AHB mode 
>> i.e. if I attempt to read data size less than rxfifo read is working 
>> fine without any issue.
>>
>> For data size more than rxfifo Read out data is content of same 
>> requested address of CS0.
>> mtd_debug read /dev/mtd1 0xf0 0x70 read --> Data is correct
>> mtd_debug read /dev/mtd1 0xf0 0x100 read --> Data is 
>> in-correct and data content are of the address 0xf0 of CS0 
>> connected flash device.
> 
> Ok, I will have a look at what could make the chip selection fail in 
> case of AHB read.

Could you try with this change applied:

@@ -503,7 +503,7 @@ static void fsl_qspi_select_mem(struct fsl_qspi *q, 
struct spi_device *spi)
 map_addr = q->memmap_phy;
 else
 map_addr = q->memmap_phy +
-  2 * q->devtype_data->ahb_buf_size;
+  q->devtype_data->ahb_buf_size;

 qspi_writel(q, map_addr, q->iobase + QUADSPI_SFA1AD + 
(i * 4));
 }

> 
>> On the setup where you have done testing, did AHB mode read is being 
>> verified for CS1?
> 
> No, I currently have only hardware with CS0 connected. So I didn't test 
> with CS1.
> 
>>
>> I am doing further debugging of this issue.
> 
> Thanks,
> Frieder

Re: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI controller

2018-11-14 Thread Schrempf Frieder
Hi Yogesh,

On 14.11.18 09:39, Yogesh Narayan Gaur wrote:
> Hi Frieder,
> 
> I have tried v5 version of the patch and have observed that Read is failing 
> for CS1.

Thanks a lot for doing the test. I really appreciate it.

> In my target 2 flash devices are connected on same bus i.e. A1 -> CS0 and A2 
> -> CS1.
> 
> On initial debugging, I figured that Read is failing for the AHB mode i.e. if 
> I attempt to read data size less than rxfifo read is working fine without any 
> issue.
> 
> For data size more than rxfifo Read out data is content of same requested 
> address of CS0.
>   mtd_debug read /dev/mtd1 0xf0 0x70 read --> Data is correct
>   mtd_debug read /dev/mtd1 0xf0 0x100 read --> Data is in-correct and 
> data content are of the address 0xf0 of CS0 connected flash device.

Ok, I will have a look at what could make the chip selection fail in 
case of AHB read.

> On the setup where you have done testing, did AHB mode read is being verified 
> for CS1?

No, I currently have only hardware with CS0 connected. So I didn't test 
with CS1.

> 
> I am doing further debugging of this issue.

Thanks,
Frieder

Re: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI controller

2018-11-14 Thread Schrempf Frieder
Hi Yogesh,

On 14.11.18 09:39, Yogesh Narayan Gaur wrote:
> Hi Frieder,
> 
> I have tried v5 version of the patch and have observed that Read is failing 
> for CS1.

Thanks a lot for doing the test. I really appreciate it.

> In my target 2 flash devices are connected on same bus i.e. A1 -> CS0 and A2 
> -> CS1.
> 
> On initial debugging, I figured that Read is failing for the AHB mode i.e. if 
> I attempt to read data size less than rxfifo read is working fine without any 
> issue.
> 
> For data size more than rxfifo Read out data is content of same requested 
> address of CS0.
>   mtd_debug read /dev/mtd1 0xf0 0x70 read --> Data is correct
>   mtd_debug read /dev/mtd1 0xf0 0x100 read --> Data is in-correct and 
> data content are of the address 0xf0 of CS0 connected flash device.

Ok, I will have a look at what could make the chip selection fail in 
case of AHB read.

> On the setup where you have done testing, did AHB mode read is being verified 
> for CS1?

No, I currently have only hardware with CS0 connected. So I didn't test 
with CS1.

> 
> I am doing further debugging of this issue.

Thanks,
Frieder

RE: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI controller

2018-11-14 Thread Yogesh Narayan Gaur
Hi Frieder,

I have tried v5 version of the patch and have observed that Read is failing for 
CS1.

In my target 2 flash devices are connected on same bus i.e. A1 -> CS0 and A2 -> 
CS1.

On initial debugging, I figured that Read is failing for the AHB mode i.e. if I 
attempt to read data size less than rxfifo read is working fine without any 
issue.

For data size more than rxfifo Read out data is content of same requested 
address of CS0.
mtd_debug read /dev/mtd1 0xf0 0x70 read --> Data is correct
mtd_debug read /dev/mtd1 0xf0 0x100 read --> Data is in-correct and 
data content are of the address 0xf0 of CS0 connected flash device.

On the setup where you have done testing, did AHB mode read is being verified 
for CS1?

I am doing further debugging of this issue.

--
Regards
Yogesh Gaur

> -Original Message-
> From: Schrempf Frieder [mailto:frieder.schre...@kontron.de]
> Sent: Tuesday, November 13, 2018 7:18 PM
> To: linux-...@lists.infradead.org; boris.brezil...@bootlin.com; linux-
> s...@vger.kernel.org; Marek Vasut ; Mark Brown
> ; Han Xu 
> Cc: dw...@infradead.org; computersforpe...@gmail.com; rich...@nod.at;
> miquel.ray...@bootlin.com; David Wolfe ; Fabio
> Estevam ; Prabhakar Kushwaha
> ; Yogesh Narayan Gaur
> ; shawn...@kernel.org; Schrempf Frieder
> ; linux-kernel@vger.kernel.org
> Subject: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI
> controller
> 
> This driver is derived from the SPI NOR driver at mtd/spi-nor/fsl-quadspi.c. 
> It
> uses the new SPI memory interface of the SPI framework to issue flash memory
> operations to up to four connected flash chips (2 buses with 2 CS each).
> 
> The controller does not support generic SPI messages.
> 
> This patch also disables the build of the "old" driver and reuses its Kconfig 
> option
> CONFIG_SPI_FSL_QUADSPI to replace it.
> 
> Signed-off-by: Frieder Schrempf 
> ---
>  drivers/mtd/spi-nor/Kconfig  |   9 -
>  drivers/mtd/spi-nor/Makefile |   1 -
>  drivers/spi/Kconfig  |  11 +
>  drivers/spi/Makefile |   1 +
>  drivers/spi/spi-fsl-qspi.c   | 946 ++
>  5 files changed, 958 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig index
> 6cc9c92..d1ca307 100644
> --- a/drivers/mtd/spi-nor/Kconfig
> +++ b/drivers/mtd/spi-nor/Kconfig
> @@ -59,15 +59,6 @@ config SPI_CADENCE_QUADSPI
> device with a Cadence QSPI controller and want to access the
> Flash as an MTD device.
> 
> -config SPI_FSL_QUADSPI
> - tristate "Freescale Quad SPI controller"
> - depends on ARCH_MXC || SOC_LS1021A || ARCH_LAYERSCAPE ||
> COMPILE_TEST
> - depends on HAS_IOMEM
> - help
> -   This enables support for the Quad SPI controller in master mode.
> -   This controller does not support generic SPI. It only supports
> -   SPI NOR.
> -
>  config SPI_HISI_SFC
>   tristate "Hisilicon SPI-NOR Flash Controller(SFC)"
>   depends on ARCH_HISI || COMPILE_TEST
> diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile index
> f4c61d2..3f160c2e3 100644
> --- a/drivers/mtd/spi-nor/Makefile
> +++ b/drivers/mtd/spi-nor/Makefile
> @@ -3,7 +3,6 @@ obj-$(CONFIG_MTD_SPI_NOR) += spi-nor.o
>  obj-$(CONFIG_SPI_ASPEED_SMC) += aspeed-smc.o
>  obj-$(CONFIG_SPI_ATMEL_QUADSPI)  += atmel-quadspi.o
>  obj-$(CONFIG_SPI_CADENCE_QUADSPI)+= cadence-quadspi.o
> -obj-$(CONFIG_SPI_FSL_QUADSPI)+= fsl-quadspi.o
>  obj-$(CONFIG_SPI_HISI_SFC)   += hisi-sfc.o
>  obj-$(CONFIG_MTD_MT81xx_NOR)+= mtk-quadspi.o
>  obj-$(CONFIG_SPI_NXP_SPIFI)  += nxp-spifi.o
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index 7d3a5c9..8c84186
> 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -259,6 +259,17 @@ config SPI_FSL_LPSPI
>   help
> This enables Freescale i.MX LPSPI controllers in master mode.
> 
> +config SPI_FSL_QUADSPI
> + tristate "Freescale QSPI controller"
> + depends on ARCH_MXC || SOC_LS1021A || ARCH_LAYERSCAPE ||
> COMPILE_TEST
> + depends on HAS_IOMEM
> + help
> +   This enables support for the Quad SPI controller in master mode.
> +   Up to four flash chips can be connected on two buses with two
> +   chipselects each.
> +   This controller does not support generic SPI messages. It only
> +   supports the high-level SPI memory interface.
> +
>  config SPI_GPIO
>   tristate "GPIO-based bitbanging SPI Master"
>   depends on GPIOLIB || COMPILE_TEST
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index 
> 3575205..5377e61
> 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -44,6 +44,7 @@ obj-$(CONFIG_SPI_FSL_DSPI)  += spi-fsl-
> dspi.o
>  obj-$(CONFIG_SPI_FSL_LIB)+= spi-fsl-lib.o
>  obj-$(CONFIG_SPI_FSL_ESPI)   += spi-fsl-espi.o
>  obj-$(CONFIG_SPI_FSL_LPSPI)  += spi-fsl-lpspi.o
> +obj-$(CONFIG_SPI_FSL_QUADSPI)+= 

RE: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI controller

2018-11-14 Thread Yogesh Narayan Gaur
Hi Frieder,

I have tried v5 version of the patch and have observed that Read is failing for 
CS1.

In my target 2 flash devices are connected on same bus i.e. A1 -> CS0 and A2 -> 
CS1.

On initial debugging, I figured that Read is failing for the AHB mode i.e. if I 
attempt to read data size less than rxfifo read is working fine without any 
issue.

For data size more than rxfifo Read out data is content of same requested 
address of CS0.
mtd_debug read /dev/mtd1 0xf0 0x70 read --> Data is correct
mtd_debug read /dev/mtd1 0xf0 0x100 read --> Data is in-correct and 
data content are of the address 0xf0 of CS0 connected flash device.

On the setup where you have done testing, did AHB mode read is being verified 
for CS1?

I am doing further debugging of this issue.

--
Regards
Yogesh Gaur

> -Original Message-
> From: Schrempf Frieder [mailto:frieder.schre...@kontron.de]
> Sent: Tuesday, November 13, 2018 7:18 PM
> To: linux-...@lists.infradead.org; boris.brezil...@bootlin.com; linux-
> s...@vger.kernel.org; Marek Vasut ; Mark Brown
> ; Han Xu 
> Cc: dw...@infradead.org; computersforpe...@gmail.com; rich...@nod.at;
> miquel.ray...@bootlin.com; David Wolfe ; Fabio
> Estevam ; Prabhakar Kushwaha
> ; Yogesh Narayan Gaur
> ; shawn...@kernel.org; Schrempf Frieder
> ; linux-kernel@vger.kernel.org
> Subject: [PATCH v5 3/9] spi: Add a driver for the Freescale/NXP QuadSPI
> controller
> 
> This driver is derived from the SPI NOR driver at mtd/spi-nor/fsl-quadspi.c. 
> It
> uses the new SPI memory interface of the SPI framework to issue flash memory
> operations to up to four connected flash chips (2 buses with 2 CS each).
> 
> The controller does not support generic SPI messages.
> 
> This patch also disables the build of the "old" driver and reuses its Kconfig 
> option
> CONFIG_SPI_FSL_QUADSPI to replace it.
> 
> Signed-off-by: Frieder Schrempf 
> ---
>  drivers/mtd/spi-nor/Kconfig  |   9 -
>  drivers/mtd/spi-nor/Makefile |   1 -
>  drivers/spi/Kconfig  |  11 +
>  drivers/spi/Makefile |   1 +
>  drivers/spi/spi-fsl-qspi.c   | 946 ++
>  5 files changed, 958 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig index
> 6cc9c92..d1ca307 100644
> --- a/drivers/mtd/spi-nor/Kconfig
> +++ b/drivers/mtd/spi-nor/Kconfig
> @@ -59,15 +59,6 @@ config SPI_CADENCE_QUADSPI
> device with a Cadence QSPI controller and want to access the
> Flash as an MTD device.
> 
> -config SPI_FSL_QUADSPI
> - tristate "Freescale Quad SPI controller"
> - depends on ARCH_MXC || SOC_LS1021A || ARCH_LAYERSCAPE ||
> COMPILE_TEST
> - depends on HAS_IOMEM
> - help
> -   This enables support for the Quad SPI controller in master mode.
> -   This controller does not support generic SPI. It only supports
> -   SPI NOR.
> -
>  config SPI_HISI_SFC
>   tristate "Hisilicon SPI-NOR Flash Controller(SFC)"
>   depends on ARCH_HISI || COMPILE_TEST
> diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile index
> f4c61d2..3f160c2e3 100644
> --- a/drivers/mtd/spi-nor/Makefile
> +++ b/drivers/mtd/spi-nor/Makefile
> @@ -3,7 +3,6 @@ obj-$(CONFIG_MTD_SPI_NOR) += spi-nor.o
>  obj-$(CONFIG_SPI_ASPEED_SMC) += aspeed-smc.o
>  obj-$(CONFIG_SPI_ATMEL_QUADSPI)  += atmel-quadspi.o
>  obj-$(CONFIG_SPI_CADENCE_QUADSPI)+= cadence-quadspi.o
> -obj-$(CONFIG_SPI_FSL_QUADSPI)+= fsl-quadspi.o
>  obj-$(CONFIG_SPI_HISI_SFC)   += hisi-sfc.o
>  obj-$(CONFIG_MTD_MT81xx_NOR)+= mtk-quadspi.o
>  obj-$(CONFIG_SPI_NXP_SPIFI)  += nxp-spifi.o
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index 7d3a5c9..8c84186
> 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -259,6 +259,17 @@ config SPI_FSL_LPSPI
>   help
> This enables Freescale i.MX LPSPI controllers in master mode.
> 
> +config SPI_FSL_QUADSPI
> + tristate "Freescale QSPI controller"
> + depends on ARCH_MXC || SOC_LS1021A || ARCH_LAYERSCAPE ||
> COMPILE_TEST
> + depends on HAS_IOMEM
> + help
> +   This enables support for the Quad SPI controller in master mode.
> +   Up to four flash chips can be connected on two buses with two
> +   chipselects each.
> +   This controller does not support generic SPI messages. It only
> +   supports the high-level SPI memory interface.
> +
>  config SPI_GPIO
>   tristate "GPIO-based bitbanging SPI Master"
>   depends on GPIOLIB || COMPILE_TEST
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index 
> 3575205..5377e61
> 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -44,6 +44,7 @@ obj-$(CONFIG_SPI_FSL_DSPI)  += spi-fsl-
> dspi.o
>  obj-$(CONFIG_SPI_FSL_LIB)+= spi-fsl-lib.o
>  obj-$(CONFIG_SPI_FSL_ESPI)   += spi-fsl-espi.o
>  obj-$(CONFIG_SPI_FSL_LPSPI)  += spi-fsl-lpspi.o
> +obj-$(CONFIG_SPI_FSL_QUADSPI)+=