| Message ID | MN2PR08MB5951E35FED92DD502F57B590B8140@MN2PR08MB5951.namprd08.prod.outlook.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Miquel Raynal |
| Headers | show |
| Series | Introduce generic ONFI support | expand |
Hi Shivamurthy, On Mon, 3 Jun 2019 12:43:28 +0000 "Shivamurthy Shastri (sshivamurthy)" <sshivamurthy@micron.com> wrote: > Create onfi_helper object. This is base to turn ONFI code to generic. > > Signed-off-by: Shivamurthy Shastri <sshivamurthy@micron.com> > --- > include/linux/mtd/nand.h | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h > index 3cdf06cae8b6..645dde4c5797 100644 > --- a/include/linux/mtd/nand.h > +++ b/include/linux/mtd/nand.h > @@ -11,6 +11,7 @@ > #define __LINUX_MTD_NAND_H > > #include <linux/mtd/mtd.h> > +#include <linux/mtd/onfi.h> > > /** > * struct nand_memory_organization - Memory organization structure > @@ -157,6 +158,24 @@ struct nand_ops { > bool (*isbad)(struct nand_device *nand, const struct nand_pos *pos); > }; > > +/** > + * struct onfi_helper - ONFI helper functions that should be implemented by > + * specialized layers (raw NAND, SPI NAND, etc.) > + * @page: Page number for ONFI parameter table > + * @check_revision: Check ONFI revision number > + * @parameter_page_read: Function to read parameter pages > + * @init_intf_data: Initialize interface specific data or fixups > + */ > +struct onfi_helper { > + u8 page; > + int (*check_revision)(struct nand_device *base, > + struct nand_onfi_params *p, int *onfi_version); > + int (*parameter_page_read)(struct nand_device *base, u8 page, > + void *buf, unsigned int len); > + int (*init_intf_data)(struct nand_device *base, > + struct nand_onfi_params *p); > +}; > + > /** > * struct nand_device - NAND device > * @mtd: MTD instance attached to the NAND device > @@ -165,6 +184,7 @@ struct nand_ops { > * @rowconv: position to row address converter > * @bbt: bad block table info > * @ops: NAND operations attached to the NAND device > + * @helper: Helper functions to detect and initialize ONFI NAND > * > * Generic NAND object. Specialized NAND layers (raw NAND, SPI NAND, OneNAND) > * should declare their own NAND object embedding a nand_device struct (that's > @@ -183,6 +203,7 @@ struct nand_device { > struct nand_row_converter rowconv; > struct nand_bbt bbt; > const struct nand_ops *ops; > + struct onfi_helper helper; Sorry, but I don't think that's the right solution. When I said we should have ONFI code shared I was thinking about the code that parses the ONFI struct/data to extract nand_memory_organization bits or other generic info, not something that would abstract how to retrieve the ONFI param page. Clearly, the generic NAND layer is not supposed to handle such protocol/low-level details. Regards, Boris
Hi Boris, > > Create onfi_helper object. This is base to turn ONFI code to generic. > > > > Signed-off-by: Shivamurthy Shastri <sshivamurthy@micron.com> > > --- > > include/linux/mtd/nand.h | 21 +++++++++++++++++++++ > > 1 file changed, 21 insertions(+) > > > > diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h > > index 3cdf06cae8b6..645dde4c5797 100644 > > --- a/include/linux/mtd/nand.h > > +++ b/include/linux/mtd/nand.h > > @@ -11,6 +11,7 @@ > > #define __LINUX_MTD_NAND_H > > > > #include <linux/mtd/mtd.h> > > +#include <linux/mtd/onfi.h> > > > > /** > > * struct nand_memory_organization - Memory organization structure > > @@ -157,6 +158,24 @@ struct nand_ops { > > bool (*isbad)(struct nand_device *nand, const struct nand_pos > *pos); > > }; > > > > +/** > > + * struct onfi_helper - ONFI helper functions that should be implemented > by > > + * specialized layers (raw NAND, SPI NAND, etc.) > > + * @page: Page number for ONFI parameter table > > + * @check_revision: Check ONFI revision number > > + * @parameter_page_read: Function to read parameter pages > > + * @init_intf_data: Initialize interface specific data or fixups > > + */ > > +struct onfi_helper { > > + u8 page; > > + int (*check_revision)(struct nand_device *base, > > + struct nand_onfi_params *p, int *onfi_version); > > + int (*parameter_page_read)(struct nand_device *base, u8 page, > > + void *buf, unsigned int len); > > + int (*init_intf_data)(struct nand_device *base, > > + struct nand_onfi_params *p); > > +}; > > + > > /** > > * struct nand_device - NAND device > > * @mtd: MTD instance attached to the NAND device > > @@ -165,6 +184,7 @@ struct nand_ops { > > * @rowconv: position to row address converter > > * @bbt: bad block table info > > * @ops: NAND operations attached to the NAND device > > + * @helper: Helper functions to detect and initialize ONFI NAND > > * > > * Generic NAND object. Specialized NAND layers (raw NAND, SPI NAND, > OneNAND) > > * should declare their own NAND object embedding a nand_device struct > (that's > > @@ -183,6 +203,7 @@ struct nand_device { > > struct nand_row_converter rowconv; > > struct nand_bbt bbt; > > const struct nand_ops *ops; > > + struct onfi_helper helper; > > Sorry, but I don't think that's the right solution. When I said we > should have ONFI code shared I was thinking about the code that parses > the ONFI struct/data to extract nand_memory_organization bits or other > generic info, not something that would abstract how to retrieve the > ONFI param page. Clearly, the generic NAND layer is not supposed to > handle such protocol/low-level details. > In that case, I am thinking to design as follows, which splits into generic independent code. Let me know, if you have any concerns or inputs. I will parsing code from nand_onfi_detect function and move it to mtd/nand/onfi.c. Also, I will move functions like sanitize_string, nand_bit_wise_majority, onfi_crc16, and any other generic info to mtd/nand/onfi.c. Thanks, Shiva
On Tue, 4 Jun 2019 12:02:28 +0000 "Shivamurthy Shastri (sshivamurthy)" <sshivamurthy@micron.com> wrote: > Hi Boris, > > > > Create onfi_helper object. This is base to turn ONFI code to generic. > > > > > > Signed-off-by: Shivamurthy Shastri <sshivamurthy@micron.com> > > > --- > > > include/linux/mtd/nand.h | 21 +++++++++++++++++++++ > > > 1 file changed, 21 insertions(+) > > > > > > diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h > > > index 3cdf06cae8b6..645dde4c5797 100644 > > > --- a/include/linux/mtd/nand.h > > > +++ b/include/linux/mtd/nand.h > > > @@ -11,6 +11,7 @@ > > > #define __LINUX_MTD_NAND_H > > > > > > #include <linux/mtd/mtd.h> > > > +#include <linux/mtd/onfi.h> > > > > > > /** > > > * struct nand_memory_organization - Memory organization structure > > > @@ -157,6 +158,24 @@ struct nand_ops { > > > bool (*isbad)(struct nand_device *nand, const struct nand_pos > > *pos); > > > }; > > > > > > +/** > > > + * struct onfi_helper - ONFI helper functions that should be implemented > > by > > > + * specialized layers (raw NAND, SPI NAND, etc.) > > > + * @page: Page number for ONFI parameter table > > > + * @check_revision: Check ONFI revision number > > > + * @parameter_page_read: Function to read parameter pages > > > + * @init_intf_data: Initialize interface specific data or fixups > > > + */ > > > +struct onfi_helper { > > > + u8 page; > > > + int (*check_revision)(struct nand_device *base, > > > + struct nand_onfi_params *p, int *onfi_version); > > > + int (*parameter_page_read)(struct nand_device *base, u8 page, > > > + void *buf, unsigned int len); > > > + int (*init_intf_data)(struct nand_device *base, > > > + struct nand_onfi_params *p); > > > +}; > > > + > > > /** > > > * struct nand_device - NAND device > > > * @mtd: MTD instance attached to the NAND device > > > @@ -165,6 +184,7 @@ struct nand_ops { > > > * @rowconv: position to row address converter > > > * @bbt: bad block table info > > > * @ops: NAND operations attached to the NAND device > > > + * @helper: Helper functions to detect and initialize ONFI NAND > > > * > > > * Generic NAND object. Specialized NAND layers (raw NAND, SPI NAND, > > OneNAND) > > > * should declare their own NAND object embedding a nand_device struct > > (that's > > > @@ -183,6 +203,7 @@ struct nand_device { > > > struct nand_row_converter rowconv; > > > struct nand_bbt bbt; > > > const struct nand_ops *ops; > > > + struct onfi_helper helper; > > > > Sorry, but I don't think that's the right solution. When I said we > > should have ONFI code shared I was thinking about the code that parses > > the ONFI struct/data to extract nand_memory_organization bits or other > > generic info, not something that would abstract how to retrieve the > > ONFI param page. Clearly, the generic NAND layer is not supposed to > > handle such protocol/low-level details. > > > > In that case, I am thinking to design as follows, which splits into generic independent code. > Let me know, if you have any concerns or inputs. > > I will parsing code from nand_onfi_detect function and move it to mtd/nand/onfi.c. > Also, I will move functions like sanitize_string, nand_bit_wise_majority, onfi_crc16, and > any other generic info to mtd/nand/onfi.c. Sounds good.
diff --git a/include/linux/mtd/nand.h b/include/linux/mtd/nand.h index 3cdf06cae8b6..645dde4c5797 100644 --- a/include/linux/mtd/nand.h +++ b/include/linux/mtd/nand.h @@ -11,6 +11,7 @@ #define __LINUX_MTD_NAND_H #include <linux/mtd/mtd.h> +#include <linux/mtd/onfi.h> /** * struct nand_memory_organization - Memory organization structure @@ -157,6 +158,24 @@ struct nand_ops { bool (*isbad)(struct nand_device *nand, const struct nand_pos *pos); }; +/** + * struct onfi_helper - ONFI helper functions that should be implemented by + * specialized layers (raw NAND, SPI NAND, etc.) + * @page: Page number for ONFI parameter table + * @check_revision: Check ONFI revision number + * @parameter_page_read: Function to read parameter pages + * @init_intf_data: Initialize interface specific data or fixups + */ +struct onfi_helper { + u8 page; + int (*check_revision)(struct nand_device *base, + struct nand_onfi_params *p, int *onfi_version); + int (*parameter_page_read)(struct nand_device *base, u8 page, + void *buf, unsigned int len); + int (*init_intf_data)(struct nand_device *base, + struct nand_onfi_params *p); +}; + /** * struct nand_device - NAND device * @mtd: MTD instance attached to the NAND device @@ -165,6 +184,7 @@ struct nand_ops { * @rowconv: position to row address converter * @bbt: bad block table info * @ops: NAND operations attached to the NAND device + * @helper: Helper functions to detect and initialize ONFI NAND * * Generic NAND object. Specialized NAND layers (raw NAND, SPI NAND, OneNAND) * should declare their own NAND object embedding a nand_device struct (that's @@ -183,6 +203,7 @@ struct nand_device { struct nand_row_converter rowconv; struct nand_bbt bbt; const struct nand_ops *ops; + struct onfi_helper helper; }; /**
Create onfi_helper object. This is base to turn ONFI code to generic. Signed-off-by: Shivamurthy Shastri <sshivamurthy@micron.com> --- include/linux/mtd/nand.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)