RE: [LINUX PATCH v12] mtd: rawnand: pl353: Add basic driver for arm pl353 smc nand interface

2019-01-17 Thread Naga Sureshkumar Relli
Hi Romain,

> -Original Message-
> From: Romain Perier [mailto:romain.per...@gmail.com]
> Sent: Monday, January 7, 2019 3:56 PM
> To: Naga Sureshkumar Relli 
> Cc: Miquel Raynal ; Boris Brezillon
> ; linux-...@lists.infradead.org; 
> peterpand...@micron.com;
> linux-kernel@vger.kernel.org
> Subject: Re: [LINUX PATCH v12] mtd: rawnand: pl353: Add basic driver for arm 
> pl353
> smc nand interface
> 
> Hi,
> 
> Le mer. 2 janv. 2019 à 10:23, Naga Sureshkumar Relli  a 
> écrit :
> >
> > Hi,
> >
> > > -Original Message-
> > > From: Miquel Raynal [mailto:miquel.ray...@bootlin.com]
> > > Sent: Wednesday, January 2, 2019 2:04 PM
> > > To: Romain Perier 
> > > Cc: Naga Sureshkumar Relli ; Boris Brezillon
> > > ; linux-...@lists.infradead.org;
> > > peterpand...@micron.com; linux-kernel@vger.kernel.org
> > > Subject: Re: [LINUX PATCH v12] mtd: rawnand: pl353: Add basic driver
> > > for arm pl353 smc nand interface
> > >
> > > Hi Romain,
> > >
> > > Switching Boris address.
> > >
> > > Romain Perier  wrote on Fri, 21 Dec 2018
> > > 10:17:50 +0100:
> > >
> > > > Hello,
> > > >
> > > > I have rebased this patch onto 4.19.11. I use it on a
> > > > Zynq7000-based board with a NAND chip Micron MT29F4G08ABADAH4, since ~2
> weeks now.
> > > > The only problem I have to report is that when I boot with an
> > > > unchanged driver on my board, I get the following logs:
> > > >
> > > > [1.988797] nand: device found, Manufacturer ID: 0x2c, Chip ID: 0xdc
> > > > [1.995184] nand: Micron MT29F4G08ABADAH4
> > > > [1.999187] nand: 512 MiB, SLC, erase size: 128 KiB, page size: 
> > > > 2048, OOB size: 64
> > > > [2.402661] nand: timeout while waiting for chip to become ready
> > > > [2.408665] nand: timing mode 5 not acknowledged by the NAND chip
> > > > [2.416251] Bad block table not found for chip 0
> > > > [2.422278] Bad block table not found for chip 0
> > > > [2.426903] Scanning device for bad blocks
> > > > [2.431024] Bad eraseblock 0 at 0x
> > > > [2.435509] Bad eraseblock 1 at 0x0002
> > > > [2.439978] Bad eraseblock 2 at 0x0004
> > > > [2.65] Bad eraseblock 3 at 0x0006
> > > > [2.448936] Bad eraseblock 4 at 0x0008
> > > > [2.453423] Bad eraseblock 5 at 0x000a
> > > > [2.457893] Bad eraseblock 6 at 0x000c
> > > > [2.462354] Bad eraseblock 7 at 0x000e
> > > > [2.466841] Bad eraseblock 8 at 0x0010
> > > > [2.471304] Bad eraseblock 9 at 0x0012
> > > > [2.475793] Bad eraseblock 10 at 0x0014
> > > > [2.480349] Bad eraseblock 11 at 0x0016
> > > >
> > > > [...]
> > > >
> > > >
> > > > After investigation, it seems that during the nand_scan phase, the
> > > > NAND subsystem tests different timing modes on the NAND chip (mode
> > > > 0 seems to be apply during reset, and then it tries to detect the
> > > > best mode supported by the NAND chip). Only the mode 0 works here,
> > > > trying the use the mode 5 resuls in an error (as you can see in
> > > > the log) and a bad BBT detection. Both modes are supported by the
> > > > NAND chip. In order to fix this, I had to put the nfc timing into
> > > > the device node of the nfc, inside the DT
> > > (that's not a real fix, ihmo).
> > >
> > > Thanks for testing! Indeed, the ->setup_data_interface() callback should 
> > > be fixed.
> > Ok, let me check.
> > Meanwhile, can you share the timings that you put inside the DT?
> 
> Sure, I have simply added an array in the DT:
> 
> pl353,nand-controller-timings=<4 4 2 2 1 1 2>;
But the below values are not matching with sdr mode 5 timings.
The value at index 3, represents tWP_min and as per SDR mode5 timings this 
value is 1 (expressed in clock cycles).
And also value at index 1 represents tWC_min and as per SDR mode5 timings this 
value is 2.
I will also cross check the setup_data_interface() hook.

Miqual, this version of driver is rebased on top of 4.19, I will send next 
version on top of 4.20

Thanks,
Naga Sureshkumar Relli
> 
> Then, I pass this array directly to pl353_smc_set_cycles().  (I got these 
> value from the hdf
> originally, then I porte

Re: [LINUX PATCH v12] mtd: rawnand: pl353: Add basic driver for arm pl353 smc nand interface

2019-01-07 Thread Romain Perier
Hi,

Le mer. 2 janv. 2019 à 10:23, Naga Sureshkumar Relli
 a écrit :
>
> Hi,
>
> > -Original Message-
> > From: Miquel Raynal [mailto:miquel.ray...@bootlin.com]
> > Sent: Wednesday, January 2, 2019 2:04 PM
> > To: Romain Perier 
> > Cc: Naga Sureshkumar Relli ; Boris Brezillon
> > ; linux-...@lists.infradead.org; 
> > peterpand...@micron.com;
> > linux-kernel@vger.kernel.org
> > Subject: Re: [LINUX PATCH v12] mtd: rawnand: pl353: Add basic driver for 
> > arm pl353
> > smc nand interface
> >
> > Hi Romain,
> >
> > Switching Boris address.
> >
> > Romain Perier  wrote on Fri, 21 Dec 2018
> > 10:17:50 +0100:
> >
> > > Hello,
> > >
> > > I have rebased this patch onto 4.19.11. I use it on a Zynq7000-based
> > > board with a NAND chip Micron MT29F4G08ABADAH4, since ~2 weeks now.
> > > The only problem I have to report is that when I boot with an
> > > unchanged driver on my board, I get the following logs:
> > >
> > > [1.988797] nand: device found, Manufacturer ID: 0x2c, Chip ID: 0xdc
> > > [1.995184] nand: Micron MT29F4G08ABADAH4
> > > [1.999187] nand: 512 MiB, SLC, erase size: 128 KiB, page size: 2048, 
> > > OOB size: 64
> > > [2.402661] nand: timeout while waiting for chip to become ready
> > > [2.408665] nand: timing mode 5 not acknowledged by the NAND chip
> > > [2.416251] Bad block table not found for chip 0
> > > [2.422278] Bad block table not found for chip 0
> > > [2.426903] Scanning device for bad blocks
> > > [2.431024] Bad eraseblock 0 at 0x
> > > [2.435509] Bad eraseblock 1 at 0x0002
> > > [2.439978] Bad eraseblock 2 at 0x0004
> > > [2.65] Bad eraseblock 3 at 0x0006
> > > [2.448936] Bad eraseblock 4 at 0x0008
> > > [2.453423] Bad eraseblock 5 at 0x000a
> > > [2.457893] Bad eraseblock 6 at 0x000c
> > > [2.462354] Bad eraseblock 7 at 0x000e
> > > [2.466841] Bad eraseblock 8 at 0x0010
> > > [2.471304] Bad eraseblock 9 at 0x0012
> > > [2.475793] Bad eraseblock 10 at 0x0014
> > > [2.480349] Bad eraseblock 11 at 0x0016
> > >
> > > [...]
> > >
> > >
> > > After investigation, it seems that during the nand_scan phase, the
> > > NAND subsystem tests different timing modes on the NAND chip (mode 0
> > > seems to be apply during reset, and then it tries to detect the best
> > > mode supported by the NAND chip). Only the mode 0 works here, trying
> > > the use the mode 5 resuls in an error (as you can see in the log) and
> > > a bad BBT detection. Both modes are supported by the NAND chip. In
> > > order to fix this, I had to put the nfc timing into the device node of 
> > > the nfc, inside the DT
> > (that's not a real fix, ihmo).
> >
> > Thanks for testing! Indeed, the ->setup_data_interface() callback should be 
> > fixed.
> Ok, let me check.
> Meanwhile, can you share the timings that you put inside the DT?

Sure, I have simply added an array in the DT:

pl353,nand-controller-timings=<4 4 2 2 1 1 2>;

Then, I pass this array directly to pl353_smc_set_cycles().  (I got
these value from the hdf originally, then
I ported the DT to a mainline format, written by hand).

Hope this helps,
Regards,
Romain

> >
> > > Except this, everything is working as expected. Everything is stable
> > > with correct performances.
> > >
> > > If I can provide more informations, feel free to ask.
> >
> > [...]
> >
> > > > +static int pl353_setup_data_interface(struct mtd_info *mtd, int csline,
> > > > +const struct nand_data_interface 
> > > > *conf) {
> > > > + struct nand_chip *chip = mtd_to_nand(mtd);
> > > > + struct pl353_nand_controller *xnfc =
> > > > + container_of(chip, struct pl353_nand_controller, chip);
> > > > + const struct nand_sdr_timings *sdr;
> > > > + u32 timings[7], mckperiodps;
> > > > +
> > > > + if (csline == NAND_DATA_IFACE_CHECK_ONLY)
> > > > + return 0;
> > > > +
> > > > + sdr = nand_get_sdr_timings(conf);
> > > > + if (IS_ERR(sdr))
> > > > + return PTR_ERR(sdr);
> > > > +
> > > > + /*
> > > > +  * SDR timings are given in pico-seconds while NFC timings must be
> &g

RE: [LINUX PATCH v12] mtd: rawnand: pl353: Add basic driver for arm pl353 smc nand interface

2019-01-02 Thread Naga Sureshkumar Relli
Hi,

> -Original Message-
> From: Miquel Raynal [mailto:miquel.ray...@bootlin.com]
> Sent: Wednesday, January 2, 2019 2:04 PM
> To: Romain Perier 
> Cc: Naga Sureshkumar Relli ; Boris Brezillon
> ; linux-...@lists.infradead.org; 
> peterpand...@micron.com;
> linux-kernel@vger.kernel.org
> Subject: Re: [LINUX PATCH v12] mtd: rawnand: pl353: Add basic driver for arm 
> pl353
> smc nand interface
> 
> Hi Romain,
> 
> Switching Boris address.
> 
> Romain Perier  wrote on Fri, 21 Dec 2018
> 10:17:50 +0100:
> 
> > Hello,
> >
> > I have rebased this patch onto 4.19.11. I use it on a Zynq7000-based
> > board with a NAND chip Micron MT29F4G08ABADAH4, since ~2 weeks now.
> > The only problem I have to report is that when I boot with an
> > unchanged driver on my board, I get the following logs:
> >
> > [1.988797] nand: device found, Manufacturer ID: 0x2c, Chip ID: 0xdc
> > [1.995184] nand: Micron MT29F4G08ABADAH4
> > [1.999187] nand: 512 MiB, SLC, erase size: 128 KiB, page size: 2048, 
> > OOB size: 64
> > [2.402661] nand: timeout while waiting for chip to become ready
> > [2.408665] nand: timing mode 5 not acknowledged by the NAND chip
> > [2.416251] Bad block table not found for chip 0
> > [2.422278] Bad block table not found for chip 0
> > [2.426903] Scanning device for bad blocks
> > [2.431024] Bad eraseblock 0 at 0x
> > [2.435509] Bad eraseblock 1 at 0x0002
> > [2.439978] Bad eraseblock 2 at 0x0004
> > [2.65] Bad eraseblock 3 at 0x0006
> > [2.448936] Bad eraseblock 4 at 0x0008
> > [2.453423] Bad eraseblock 5 at 0x000a
> > [2.457893] Bad eraseblock 6 at 0x000c
> > [2.462354] Bad eraseblock 7 at 0x000e
> > [2.466841] Bad eraseblock 8 at 0x0010
> > [2.471304] Bad eraseblock 9 at 0x0012
> > [2.475793] Bad eraseblock 10 at 0x0014
> > [2.480349] Bad eraseblock 11 at 0x0016
> >
> > [...]
> >
> >
> > After investigation, it seems that during the nand_scan phase, the
> > NAND subsystem tests different timing modes on the NAND chip (mode 0
> > seems to be apply during reset, and then it tries to detect the best
> > mode supported by the NAND chip). Only the mode 0 works here, trying
> > the use the mode 5 resuls in an error (as you can see in the log) and
> > a bad BBT detection. Both modes are supported by the NAND chip. In
> > order to fix this, I had to put the nfc timing into the device node of the 
> > nfc, inside the DT
> (that's not a real fix, ihmo).
> 
> Thanks for testing! Indeed, the ->setup_data_interface() callback should be 
> fixed.
Ok, let me check.
Meanwhile, can you share the timings that you put inside the DT?
> 
> > Except this, everything is working as expected. Everything is stable
> > with correct performances.
> >
> > If I can provide more informations, feel free to ask.
> 
> [...]
> 
> > > +static int pl353_setup_data_interface(struct mtd_info *mtd, int csline,
> > > +const struct nand_data_interface *conf) {
> > > + struct nand_chip *chip = mtd_to_nand(mtd);
> > > + struct pl353_nand_controller *xnfc =
> > > + container_of(chip, struct pl353_nand_controller, chip);
> > > + const struct nand_sdr_timings *sdr;
> > > + u32 timings[7], mckperiodps;
> > > +
> > > + if (csline == NAND_DATA_IFACE_CHECK_ONLY)
> > > + return 0;
> > > +
> > > + sdr = nand_get_sdr_timings(conf);
> > > + if (IS_ERR(sdr))
> > > + return PTR_ERR(sdr);
> > > +
> > > + /*
> > > +  * SDR timings are given in pico-seconds while NFC timings must be
> > > +  * expressed in NAND controller clock cycles.
> > > +  */
> > > + mckperiodps = NSEC_PER_SEC / clk_get_rate(xnfc->mclk);
> > > + mckperiodps *= 1000;
> > > + if (sdr->tRC_min <= 2)
> > > + /*
> > > +  * PL353 SMC needs one extra read cycle in SDR Mode 5
> > > +  * This is not written anywhere in the datasheet but
> > > +  * the results observed during testing.
> > > +  */
> > > + timings[0] = DIV_ROUND_UP(sdr->tRC_min, mckperiodps) + 1;
> > > + else
> > > + timings[0] = DIV_ROUND_UP(sdr->tRC_min, mckperiodps);
> > > +
> > > + timings[1] = DIV_ROUND_UP(sdr->tWC_min, mckperiodps);
> > > + /*
> > > +  * For all SDR

Re: [LINUX PATCH v12] mtd: rawnand: pl353: Add basic driver for arm pl353 smc nand interface

2019-01-02 Thread Miquel Raynal
Hi Romain,

Switching Boris address.

Romain Perier  wrote on Fri, 21 Dec 2018
10:17:50 +0100:

> Hello,
> 
> I have rebased this patch onto 4.19.11. I use it on a Zynq7000-based
> board with a NAND chip Micron MT29F4G08ABADAH4, since ~2 weeks now.
> The only problem I have to report is that when I boot with an unchanged
> driver on my board, I get the following logs:
> 
> [1.988797] nand: device found, Manufacturer ID: 0x2c, Chip ID: 0xdc
> [1.995184] nand: Micron MT29F4G08ABADAH4
> [1.999187] nand: 512 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB 
> size: 64
> [2.402661] nand: timeout while waiting for chip to become ready
> [2.408665] nand: timing mode 5 not acknowledged by the NAND chip
> [2.416251] Bad block table not found for chip 0
> [2.422278] Bad block table not found for chip 0
> [2.426903] Scanning device for bad blocks
> [2.431024] Bad eraseblock 0 at 0x
> [2.435509] Bad eraseblock 1 at 0x0002
> [2.439978] Bad eraseblock 2 at 0x0004
> [2.65] Bad eraseblock 3 at 0x0006
> [2.448936] Bad eraseblock 4 at 0x0008
> [2.453423] Bad eraseblock 5 at 0x000a
> [2.457893] Bad eraseblock 6 at 0x000c
> [2.462354] Bad eraseblock 7 at 0x000e
> [2.466841] Bad eraseblock 8 at 0x0010
> [2.471304] Bad eraseblock 9 at 0x0012
> [2.475793] Bad eraseblock 10 at 0x0014
> [2.480349] Bad eraseblock 11 at 0x0016
> 
> [...]
> 
> 
> After investigation, it seems that during the nand_scan phase, the NAND
> subsystem tests different timing modes on the NAND chip (mode 0 seems to be
> apply during reset, and then it tries to detect the best mode supported by the
> NAND chip). Only the mode 0 works here, trying the use the mode 5 resuls in an
> error (as you can see in the log) and a bad BBT detection. Both modes are
> supported by the NAND chip. In order to fix this, I had to put the nfc timing
> into the device node of the nfc, inside the DT (that's not a real fix, ihmo).

Thanks for testing! Indeed, the ->setup_data_interface() callback should be 
fixed.

> Except this, everything is working as expected. Everything is stable with 
> correct
> performances.
> 
> If I can provide more informations, feel free to ask.

[...]

> > +static int pl353_setup_data_interface(struct mtd_info *mtd, int csline,
> > +  const struct nand_data_interface *conf)
> > +{
> > +   struct nand_chip *chip = mtd_to_nand(mtd);
> > +   struct pl353_nand_controller *xnfc =
> > +   container_of(chip, struct pl353_nand_controller, chip);
> > +   const struct nand_sdr_timings *sdr;
> > +   u32 timings[7], mckperiodps;
> > +
> > +   if (csline == NAND_DATA_IFACE_CHECK_ONLY)
> > +   return 0;
> > +
> > +   sdr = nand_get_sdr_timings(conf);
> > +   if (IS_ERR(sdr))
> > +   return PTR_ERR(sdr);
> > +
> > +   /*
> > +* SDR timings are given in pico-seconds while NFC timings must be
> > +* expressed in NAND controller clock cycles.
> > +*/
> > +   mckperiodps = NSEC_PER_SEC / clk_get_rate(xnfc->mclk);
> > +   mckperiodps *= 1000;
> > +   if (sdr->tRC_min <= 2)
> > +   /*
> > +* PL353 SMC needs one extra read cycle in SDR Mode 5
> > +* This is not written anywhere in the datasheet but
> > +* the results observed during testing.
> > +*/
> > +   timings[0] = DIV_ROUND_UP(sdr->tRC_min, mckperiodps) + 1;
> > +   else
> > +   timings[0] = DIV_ROUND_UP(sdr->tRC_min, mckperiodps);
> > +
> > +   timings[1] = DIV_ROUND_UP(sdr->tWC_min, mckperiodps);
> > +   /*
> > +* For all SDR modes, PL353 SMC needs tREA max value as 1,
> > +* Results observed during testing.
> > +*/
> > +   timings[2] = PL353_TREA_MAX_VALUE;
> > +   timings[3] = DIV_ROUND_UP(sdr->tWP_min, mckperiodps);
> > +   timings[4] = DIV_ROUND_UP(sdr->tCLR_min, mckperiodps);
> > +   timings[5] = DIV_ROUND_UP(sdr->tAR_min, mckperiodps);
> > +   timings[6] = DIV_ROUND_UP(sdr->tRR_min, mckperiodps);
> > +   pl353_smc_set_cycles(timings);
> > +
> > +   return 0;
> > +}  
> 
> If I hack this function in order to limit the timings only to mode 0,
> everything works. Otherwise it hangs when it tries to apply mode 5.
> 

Maybe Naga is not using a chip compatible with mode 5 and did not ran
into this issue?


Thanks,
Miquèl


Re: [LINUX PATCH v12] mtd: rawnand: pl353: Add basic driver for arm pl353 smc nand interface

2018-12-21 Thread Romain Perier
Hello,

I have rebased this patch onto 4.19.11. I use it on a Zynq7000-based
board with a NAND chip Micron MT29F4G08ABADAH4, since ~2 weeks now.
The only problem I have to report is that when I boot with an unchanged
driver on my board, I get the following logs:

[1.988797] nand: device found, Manufacturer ID: 0x2c, Chip ID: 0xdc
[1.995184] nand: Micron MT29F4G08ABADAH4
[1.999187] nand: 512 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB 
size: 64
[2.402661] nand: timeout while waiting for chip to become ready
[2.408665] nand: timing mode 5 not acknowledged by the NAND chip
[2.416251] Bad block table not found for chip 0
[2.422278] Bad block table not found for chip 0
[2.426903] Scanning device for bad blocks
[2.431024] Bad eraseblock 0 at 0x
[2.435509] Bad eraseblock 1 at 0x0002
[2.439978] Bad eraseblock 2 at 0x0004
[2.65] Bad eraseblock 3 at 0x0006
[2.448936] Bad eraseblock 4 at 0x0008
[2.453423] Bad eraseblock 5 at 0x000a
[2.457893] Bad eraseblock 6 at 0x000c
[2.462354] Bad eraseblock 7 at 0x000e
[2.466841] Bad eraseblock 8 at 0x0010
[2.471304] Bad eraseblock 9 at 0x0012
[2.475793] Bad eraseblock 10 at 0x0014
[2.480349] Bad eraseblock 11 at 0x0016

[...]


After investigation, it seems that during the nand_scan phase, the NAND
subsystem tests different timing modes on the NAND chip (mode 0 seems to be
apply during reset, and then it tries to detect the best mode supported by the
NAND chip). Only the mode 0 works here, trying the use the mode 5 resuls in an
error (as you can see in the log) and a bad BBT detection. Both modes are
supported by the NAND chip. In order to fix this, I had to put the nfc timing
into the device node of the nfc, inside the DT (that's not a real fix, ihmo).
Except this, everything is working as expected. Everything is stable with 
correct
performances.

If I can provide more informations, feel free to ask.


On Tue, Aug 07, 2018 at 11:10:14AM +0530, Naga Sureshkumar Relli wrote:
> Add driver for arm pl353 static memory controller nand interface with
> HW ECC support. This controller is used in Xilinx Zynq SoC for
> interfacing the NAND flash memory.
> 
> Signed-off-by: Naga Sureshkumar Relli 
Tested-by: Romain Perier 

> ---
> Changes in v12:
>  - Rebased the driver on top of v4.19 nand tree
>  - Removed nand_scan_ident() and nand_scan_tail(), and added 
> nand_controller_ops
>with ->attach_chip() and used nand_scan() instead.
>  - Renamed pl353_nand_info structure to pl353_nand_controller
>  - Renamed nand_base and nandaddr in pl353_nand_controller to 'regs' and 
> 'buf_addr'
>  - Added new API pl353_wait_for_ecc_done() to wait for ecc done and call it 
> from
>pl353_nand_write_page_hwecc() and pl353_nand_read_page_hwecc()
>  - Defined new macro for max ECC blocks
>  - Added return value check for ecc.calculate()
>  - Renamed pl353_nand_cmd_function() to pl353_nand_exec_op_cmd()
>  - Added x16 bus-width support
>  - The dependent driver pl353-smc is already reviewed and hence dropped the
>smc driver
> Changes in v11:
>  - Removed Documentation patch and added the required info in driver as
>per Boris comments.
>  - Removed unwanted variables from pl353_nand_info as per Miquel comments
>  - Removed IO_ADDR_R/W.
>  - Replaced onhot() with hweight32()
>  - Defined macros for static values in function pl353_nand_correct_data()
>  - Removed all unnecessary delays
>  - Used nand_wait_ready() where ever is required
>  - Modifed the pl353_setup_data_interface() logic as per Miquel comments.
>  - Taken array instead of 7 values in pl353_setup_data_interface() and pass
>it to smc driver.
>  - Added check to collect the return value of mtd_device_register().
> Changes in 10:
>  - Typos correction like nand to NAND and soc to SOC etc..
>  - Defined macros for the values in pl353_nand_calculate_hwecc()
>  - Modifed ecc_status from int to char in pl353_nand_calculate_hwecc()
>  - Changed the return type form int to bool to the function
>onehot()
>  - Removed udelay(1000) in pl353_cmd_function, as it is not required
>  - Dropped ecc->hwctl = NULL in pl353_ecc_init()
>  - Added an error message in pl353_ecc_init(), when there is no matching
>oobsize
>  - Changed the variable from xnand to xnfc
>  - Added logic to get mtd->name from DT, if it is specified in DT
> Changes in v9:
>  - Addressed the below comments given by Miquel
>  - instead of using pl353_nand_write32, use directly writel_relaxed
>  - Fixed check patch warnings
>  - Renamed write_buf/read_buf to write_data_op/read_data_op
>  - use BIT macro instead of 1 << nr
>  - Use NAND_ROW_ADDR_3 flag
>  - Use nand_wait_ready()
>  - Removed swecc functions
>  - Use address cycles as per size, instead of reading it from Parameter page
>  - Instead of writing too many patterns, use optional property
> Changes in v8:
>  - Added exec

Re: [LINUX PATCH v12] mtd: rawnand: pl353: Add basic driver for arm pl353 smc nand interface

2018-11-05 Thread Miquel Raynal
Hi Helmut,

Helmut Grohne  wrote on Mon, 5 Nov 2018
09:40:43 +0100:

> On Tue, Aug 07, 2018 at 11:10:14AM +0530, Naga Sureshkumar Relli wrote:
> > Add driver for arm pl353 static memory controller nand interface with
> > HW ECC support. This controller is used in Xilinx Zynq SoC for
> > interfacing the NAND flash memory.
> > 
> > Signed-off-by: Naga Sureshkumar Relli 
> > ---
> > Changes in v12:  
> 
> I attempted to apply this v12 together with the v11 of pl353-smc (which seems
> to be the latest version) onto a vanilla v4.19 kernel tree. Application was
> smooth and a build was without errors. During boot, my dmesg was spammed with
> lots of repetitions of this:
> 
> [5.816275] [ cut here ]
> [5.820981] WARNING: CPU: 0 PID: 1 at 
> .../linux/drivers/mtd/nand/raw/nand_base.c:2773 
> nand_subop_get_data_len+0x60/0xa4
> [5.836868] Modules linked in:
> [5.836912] CPU: 0 PID: 1 Comm: swapper/0 Tainted: GW 
> 4.19.0-dirty #3
> [5.836929] Hardware name: Xilinx Zynq Platform
> [5.836943] Backtrace: 
> [5.836981] [] (dump_backtrace) from [] 
> (show_stack+0x20/0x24)
> [5.837004]  r7:c062e618 r6: r5:600f0013 r4:c062e618
> [5.837027] [] (show_stack) from [] 
> (dump_stack+0xac/0xd8)
> [5.837055] [] (dump_stack) from [] 
> (__warn+0x118/0x130)
> [5.837079]  r10: r9:c0270be8 r8:0ad5 r7:c0550358 r6:0009 
> r5:
> [5.837097]  r4: r3:de4c6000
> [5.837122] [] (__warn) from [] 
> (warn_slowpath_null+0x50/0x58)
> [5.837146]  r9:de4c78c5 r8:da4d1010 r7:de4c78f8 r6:c0270be8 r5:0ad5 
> r4:c0550358
> [5.837173] [] (warn_slowpath_null) from [] 
> (nand_subop_get_data_len+0x60/0xa4)
> [5.837192]  r6:003c r5:0003 r4:de4c7870
> [5.837217] [] (nand_subop_get_data_len) from [] 
> (pl353_nand_exec_op_cmd+0x60/0x314)
> [5.837238]  r7:de4c78f8 r6:003c r5:de4c7870 r4:0003
> [5.837262] [] (pl353_nand_exec_op_cmd) from [] 
> (nand_op_parser_exec_op+0x484/0x4e8)
> [5.837285]  r10:0002 r9:0014 r8:de4c790c r7:0004 r6:c0634260 
> r5:c052cc44
> [5.837301]  r4:c0620bcc
> [5.837325] [] (nand_op_parser_exec_op) from [] 
> (pl353_nfc_exec_op+0x24/0x2c)
> [5.837348]  r10:de4c7a18 r9:0080 r8:03463678 r7:89705f41 r6:36b4a597 
> r5:de4c78d0
> [5.837364]  r4:da4d1010
> [5.837387] [] (pl353_nfc_exec_op) from [] 
> (nand_erase_op+0x150/0x1f0)
> [5.837411] [] (nand_erase_op) from [] 
> (single_erase+0x28/0x2c)
> [5.837434]  r9:e08c1000 r8:0006 r7:0040 r6:0001 r5:0001ff80 
> r4:da4d1010
> [5.837459] [] (single_erase) from [] 
> (nand_erase_nand+0x1f8/0x3f8)
> [5.837484] [] (nand_erase_nand) from [] 
> (write_bbt+0x30c/0x748)
> [5.837508]  r10:0006 r9:e08c1000 r8:0006 r7:0002 r6:da4d1010 
> r5:
> [5.837524]  r4:0ffc
> [5.837549] [] (write_bbt) from [] 
> (nand_create_bbt+0x30c/0x6d0)
> [5.837572]  r10:c061fd2c r9:c061fce8 r8: r7:c061fd2c r6: 
> r5:0003
> [5.837587]  r4:
> [5.837614] [] (nand_create_bbt) from [] 
> (nand_scan_with_ids+0x1248/0x1e74)
> [5.837636]  r10:c0620524 r9:0004 r8:0001 r7:0004 r6:da4d1378 
> r5:0001
> [5.837652]  r4:da4d1010
> [5.837676] [] (nand_scan_with_ids) from [] 
> (pl353_nand_probe+0x144/0x208)
> [5.837699]  r10: r9:c0620ae0 r8:c0633ba0 r7: r6:da4d8e10 
> r5:da4d8e00
> [5.837715]  r4:da4d1010
> [5.837738] [] (pl353_nand_probe) from [] 
> (platform_drv_probe+0x44/0x7c)
> [5.837757]  r6:c0e0205c r5:c0620ae0 r4:da4d8e10
> [5.837783] [] (platform_drv_probe) from [] 
> (really_probe+0x27c/0x408)
> [5.837801]  r5:da4d8e10 r4:c0e02058
> [5.837827] [] (really_probe) from [] 
> (driver_probe_device+0x138/0x198)
> [5.837850]  r10: r9: r8:c0e02014 r7:0001 r6:de4c7c70 
> r5:c0620ae0
> [5.837865]  r4:da4d8e10
> [5.837891] [] (driver_probe_device) from [] 
> (__device_attach_driver+0xc8/0x144)
> [5.837913]  r9: r8:c0e02014 r6:de4c7c70 r5:da4d8e10 r4:c0620ae0
> [5.837938] [] (__device_attach_driver) from [] 
> (bus_for_each_drv+0x70/0xa4)
> [5.837958]  r7:0001 r6:c0251f0c r5:de4c7c70 r4:
> [5.837982] [] (bus_for_each_drv) from [] 
> (__device_attach+0xb0/0x11c)
> [5.838001]  r6:c061e8d0 r5:da4d8e44 r4:da4d8e10
> [5.838027] [] (__device_attach) from [] 
> (device_initial_probe+0x1c/0x20)
> [5.838047]  r7:da4d8e10 r6:c061e8d0 r5:da4d8e10 r4:da4d8e18
> [5.838073] [] (device_initial_probe) from [] 
> (bus_probe_device+0x98/0xa0)
> [5.838097] [] (bus_probe_device) from [] 
> (device_add+0x380/0x5dc)
> [5.838117]  r7:da4d8e10 r6:de595e00 r5: r4:da4d8e18
> [5.838142] [] (device_add) from [] 
> (of_device_add+0x44/0x4c)
> [5.838164]  r10:c0563da4 r9:dfbf0a70 r8:dfbf0ac0 r7: r6:de595e00 
> r5:
> [5.838180]  r4:da4d8e00
> [5.838205] [] (of

Re: [LINUX PATCH v12] mtd: rawnand: pl353: Add basic driver for arm pl353 smc nand interface

2018-11-05 Thread Helmut Grohne
On Tue, Aug 07, 2018 at 11:10:14AM +0530, Naga Sureshkumar Relli wrote:
> Add driver for arm pl353 static memory controller nand interface with
> HW ECC support. This controller is used in Xilinx Zynq SoC for
> interfacing the NAND flash memory.
> 
> Signed-off-by: Naga Sureshkumar Relli 
> ---
> Changes in v12:

I attempted to apply this v12 together with the v11 of pl353-smc (which seems
to be the latest version) onto a vanilla v4.19 kernel tree. Application was
smooth and a build was without errors. During boot, my dmesg was spammed with
lots of repetitions of this:

[5.816275] [ cut here ]
[5.820981] WARNING: CPU: 0 PID: 1 at 
.../linux/drivers/mtd/nand/raw/nand_base.c:2773 
nand_subop_get_data_len+0x60/0xa4
[5.836868] Modules linked in:
[5.836912] CPU: 0 PID: 1 Comm: swapper/0 Tainted: GW 
4.19.0-dirty #3
[5.836929] Hardware name: Xilinx Zynq Platform
[5.836943] Backtrace: 
[5.836981] [] (dump_backtrace) from [] 
(show_stack+0x20/0x24)
[5.837004]  r7:c062e618 r6: r5:600f0013 r4:c062e618
[5.837027] [] (show_stack) from [] 
(dump_stack+0xac/0xd8)
[5.837055] [] (dump_stack) from [] (__warn+0x118/0x130)
[5.837079]  r10: r9:c0270be8 r8:0ad5 r7:c0550358 r6:0009 
r5:
[5.837097]  r4: r3:de4c6000
[5.837122] [] (__warn) from [] 
(warn_slowpath_null+0x50/0x58)
[5.837146]  r9:de4c78c5 r8:da4d1010 r7:de4c78f8 r6:c0270be8 r5:0ad5 
r4:c0550358
[5.837173] [] (warn_slowpath_null) from [] 
(nand_subop_get_data_len+0x60/0xa4)
[5.837192]  r6:003c r5:0003 r4:de4c7870
[5.837217] [] (nand_subop_get_data_len) from [] 
(pl353_nand_exec_op_cmd+0x60/0x314)
[5.837238]  r7:de4c78f8 r6:003c r5:de4c7870 r4:0003
[5.837262] [] (pl353_nand_exec_op_cmd) from [] 
(nand_op_parser_exec_op+0x484/0x4e8)
[5.837285]  r10:0002 r9:0014 r8:de4c790c r7:0004 r6:c0634260 
r5:c052cc44
[5.837301]  r4:c0620bcc
[5.837325] [] (nand_op_parser_exec_op) from [] 
(pl353_nfc_exec_op+0x24/0x2c)
[5.837348]  r10:de4c7a18 r9:0080 r8:03463678 r7:89705f41 r6:36b4a597 
r5:de4c78d0
[5.837364]  r4:da4d1010
[5.837387] [] (pl353_nfc_exec_op) from [] 
(nand_erase_op+0x150/0x1f0)
[5.837411] [] (nand_erase_op) from [] 
(single_erase+0x28/0x2c)
[5.837434]  r9:e08c1000 r8:0006 r7:0040 r6:0001 r5:0001ff80 
r4:da4d1010
[5.837459] [] (single_erase) from [] 
(nand_erase_nand+0x1f8/0x3f8)
[5.837484] [] (nand_erase_nand) from [] 
(write_bbt+0x30c/0x748)
[5.837508]  r10:0006 r9:e08c1000 r8:0006 r7:0002 r6:da4d1010 
r5:
[5.837524]  r4:0ffc
[5.837549] [] (write_bbt) from [] 
(nand_create_bbt+0x30c/0x6d0)
[5.837572]  r10:c061fd2c r9:c061fce8 r8: r7:c061fd2c r6: 
r5:0003
[5.837587]  r4:
[5.837614] [] (nand_create_bbt) from [] 
(nand_scan_with_ids+0x1248/0x1e74)
[5.837636]  r10:c0620524 r9:0004 r8:0001 r7:0004 r6:da4d1378 
r5:0001
[5.837652]  r4:da4d1010
[5.837676] [] (nand_scan_with_ids) from [] 
(pl353_nand_probe+0x144/0x208)
[5.837699]  r10: r9:c0620ae0 r8:c0633ba0 r7: r6:da4d8e10 
r5:da4d8e00
[5.837715]  r4:da4d1010
[5.837738] [] (pl353_nand_probe) from [] 
(platform_drv_probe+0x44/0x7c)
[5.837757]  r6:c0e0205c r5:c0620ae0 r4:da4d8e10
[5.837783] [] (platform_drv_probe) from [] 
(really_probe+0x27c/0x408)
[5.837801]  r5:da4d8e10 r4:c0e02058
[5.837827] [] (really_probe) from [] 
(driver_probe_device+0x138/0x198)
[5.837850]  r10: r9: r8:c0e02014 r7:0001 r6:de4c7c70 
r5:c0620ae0
[5.837865]  r4:da4d8e10
[5.837891] [] (driver_probe_device) from [] 
(__device_attach_driver+0xc8/0x144)
[5.837913]  r9: r8:c0e02014 r6:de4c7c70 r5:da4d8e10 r4:c0620ae0
[5.837938] [] (__device_attach_driver) from [] 
(bus_for_each_drv+0x70/0xa4)
[5.837958]  r7:0001 r6:c0251f0c r5:de4c7c70 r4:
[5.837982] [] (bus_for_each_drv) from [] 
(__device_attach+0xb0/0x11c)
[5.838001]  r6:c061e8d0 r5:da4d8e44 r4:da4d8e10
[5.838027] [] (__device_attach) from [] 
(device_initial_probe+0x1c/0x20)
[5.838047]  r7:da4d8e10 r6:c061e8d0 r5:da4d8e10 r4:da4d8e18
[5.838073] [] (device_initial_probe) from [] 
(bus_probe_device+0x98/0xa0)
[5.838097] [] (bus_probe_device) from [] 
(device_add+0x380/0x5dc)
[5.838117]  r7:da4d8e10 r6:de595e00 r5: r4:da4d8e18
[5.838142] [] (device_add) from [] 
(of_device_add+0x44/0x4c)
[5.838164]  r10:c0563da4 r9:dfbf0a70 r8:dfbf0ac0 r7: r6:de595e00 
r5:
[5.838180]  r4:da4d8e00
[5.838205] [] (of_device_add) from [] 
(of_platform_device_create_pdata+0xa0/0xd0)
[5.838229] [] (of_platform_device_create_pdata) from [] 
(of_platform_device_create+0x20/0x24)
[5.838252]  r9:dfbf0a70 r8:dfbf0780 r7:c0470470 r6:ddfe9f50 r5: 
r4:de595e00
[5.838280] [] (of_platform_device_create) fr

[LINUX PATCH v12] mtd: rawnand: pl353: Add basic driver for arm pl353 smc nand interface

2018-08-06 Thread Naga Sureshkumar Relli
Add driver for arm pl353 static memory controller nand interface with
HW ECC support. This controller is used in Xilinx Zynq SoC for
interfacing the NAND flash memory.

Signed-off-by: Naga Sureshkumar Relli 
---
Changes in v12:
 - Rebased the driver on top of v4.19 nand tree
 - Removed nand_scan_ident() and nand_scan_tail(), and added nand_controller_ops
   with ->attach_chip() and used nand_scan() instead.
 - Renamed pl353_nand_info structure to pl353_nand_controller
 - Renamed nand_base and nandaddr in pl353_nand_controller to 'regs' and 
'buf_addr'
 - Added new API pl353_wait_for_ecc_done() to wait for ecc done and call it from
   pl353_nand_write_page_hwecc() and pl353_nand_read_page_hwecc()
 - Defined new macro for max ECC blocks
 - Added return value check for ecc.calculate()
 - Renamed pl353_nand_cmd_function() to pl353_nand_exec_op_cmd()
 - Added x16 bus-width support
 - The dependent driver pl353-smc is already reviewed and hence dropped the
   smc driver
Changes in v11:
 - Removed Documentation patch and added the required info in driver as
   per Boris comments.
 - Removed unwanted variables from pl353_nand_info as per Miquel comments
 - Removed IO_ADDR_R/W.
 - Replaced onhot() with hweight32()
 - Defined macros for static values in function pl353_nand_correct_data()
 - Removed all unnecessary delays
 - Used nand_wait_ready() where ever is required
 - Modifed the pl353_setup_data_interface() logic as per Miquel comments.
 - Taken array instead of 7 values in pl353_setup_data_interface() and pass
   it to smc driver.
 - Added check to collect the return value of mtd_device_register().
Changes in 10:
 - Typos correction like nand to NAND and soc to SOC etc..
 - Defined macros for the values in pl353_nand_calculate_hwecc()
 - Modifed ecc_status from int to char in pl353_nand_calculate_hwecc()
 - Changed the return type form int to bool to the function
   onehot()
 - Removed udelay(1000) in pl353_cmd_function, as it is not required
 - Dropped ecc->hwctl = NULL in pl353_ecc_init()
 - Added an error message in pl353_ecc_init(), when there is no matching
   oobsize
 - Changed the variable from xnand to xnfc
 - Added logic to get mtd->name from DT, if it is specified in DT
Changes in v9:
 - Addressed the below comments given by Miquel
 - instead of using pl353_nand_write32, use directly writel_relaxed
 - Fixed check patch warnings
 - Renamed write_buf/read_buf to write_data_op/read_data_op
 - use BIT macro instead of 1 << nr
 - Use NAND_ROW_ADDR_3 flag
 - Use nand_wait_ready()
 - Removed swecc functions
 - Use address cycles as per size, instead of reading it from Parameter page
 - Instead of writing too many patterns, use optional property
Changes in v8:
 - Added exec_op() implementation
 - Fixed the below v7 review comments
 - removed mtd_info from pl353_nand_info struct
 - Corrected ecc layout offsets
 - Added on-die ecc support
Changes in v7:
 - Currently not implemented the memclk rate adjustments. I will
   look into this later and once the basic driver is accepted.
 - Fixed GPL licence ident
Changes in v6:
 - Fixed the checkpatch.pl reported warnings
 - Using the address cycles information from the onfi param page
   earlier it is hardcoded to 5 in driver
Changes in v5:
 - Configure the nand timing parameters as per the onfi spec Changes in v4:
 - Updated the driver to sync with pl353_smc driver APIs
Changes in v3:
 - implemented the proper error codes
 - further breakdown this patch to multiple sets
 - added the controller and driver details to Documentation section
 - updated the licenece to GPLv2
 - reorganized the pl353_nand_ecc_init function
Changes in v2:
 - use "depends on" rather than "select" option in kconfig
 - remove unused variable parts
---
 drivers/mtd/nand/raw/Kconfig  |8 +
 drivers/mtd/nand/raw/Makefile |1 +
 drivers/mtd/nand/raw/pl353_nand.c | 1398 +
 3 files changed, 1407 insertions(+)
 create mode 100644 drivers/mtd/nand/raw/pl353_nand.c

diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
index b6738ec..e87591e 100644
--- a/drivers/mtd/nand/raw/Kconfig
+++ b/drivers/mtd/nand/raw/Kconfig
@@ -560,4 +560,12 @@ config MTD_NAND_TEGRA
  is supported. Extra OOB bytes when using HW ECC are currently
  not supported.
 
+config MTD_NAND_PL353
+   tristate "ARM Pl353 NAND flash driver"
+   depends on MTD_NAND && ARM
+   depends on PL353_SMC
+   help
+ Enables support for PrimeCell Static Memory Controller PL353.
+
+
 endif # MTD_NAND
diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile
index d5a5f98..dc5c332 100644
--- a/drivers/mtd/nand/raw/Makefile
+++ b/drivers/mtd/nand/raw/Makefile
@@ -57,6 +57,7 @@ obj-$(CONFIG_MTD_NAND_BRCMNAND)   += brcmnand/
 obj-$(CONFIG_MTD_NAND_QCOM)+= qcom_nandc.o
 obj-$(CONFIG_MTD_NAND_MTK) += mtk_ecc.o mtk_nand.o
 obj-$(CONFIG_MTD_NAND_TEGRA)   += tegra_nand.o
+obj-$(CONFIG_MTD_NA