Message ID | 20200528231612.8958-20-miquel.raynal@bootlin.com |
---|---|
State | Changes Requested |
Delegated to: | Miquel Raynal |
Headers | show |
Series | Allow vendor drivers to propose their own timings | expand |
On Fri, 29 May 2020 01:16:01 +0200 Miquel Raynal <miquel.raynal@bootlin.com> wrote: > All NAND chips will always use the same interface configuration for > reset and at startup: SDR mode 0. Instead of copying around the > interface configuration, let's just define a helper to retrieve the > reset interface configuration object. > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > --- > drivers/mtd/nand/raw/nand_base.c | 10 +++------- > drivers/mtd/nand/raw/nand_timings.c | 6 ++++++ > include/linux/mtd/rawnand.h | 3 +++ > 3 files changed, 12 insertions(+), 7 deletions(-) > > diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c > index 07a7d918b462..4a72ec221262 100644 > --- a/drivers/mtd/nand/raw/nand_base.c > +++ b/drivers/mtd/nand/raw/nand_base.c > @@ -928,9 +928,7 @@ static int nand_reset_interface(struct nand_chip *chip, int chipnr) > * timings to timing mode 0. > */ > > - onfi_fill_interface_config(chip, &chip->interface_config, > - NAND_SDR_IFACE, 0); > - ret = ops->setup_interface(chip, chipnr, &chip->interface_config); > + ret = ops->setup_interface(chip, chipnr, nand_get_reset_interface()); > if (ret) > pr_err("Failed to configure data interface to SDR timing mode 0\n"); > > @@ -2483,7 +2481,6 @@ EXPORT_SYMBOL_GPL(nand_subop_get_data_len); > */ > int nand_reset(struct nand_chip *chip, int chipnr) > { > - struct nand_interface_config saved_intf_config = chip->interface_config; > int ret; > > ret = nand_reset_interface(chip, chipnr); > @@ -2508,11 +2505,10 @@ int nand_reset(struct nand_chip *chip, int chipnr) > * nand_setup_interface() uses ->set/get_features() which would > * fail anyway as the parameter page is not available yet. > */ > - if (!memcmp(&chip->interface_config, &saved_intf_config, > - sizeof(saved_intf_config))) > + if (!memcmp(&chip->interface_config, nand_get_reset_interface(), > + sizeof(chip->interface_config))) > return 0; > > - chip->interface_config = saved_intf_config; Are you sure you can do that now? Looks like the current interface config is not preserved, and you end up with mode 0 after a reset. > ret = nand_setup_interface(chip, chipnr); > if (ret) > return ret; > diff --git a/drivers/mtd/nand/raw/nand_timings.c b/drivers/mtd/nand/raw/nand_timings.c > index 6c12cd52323d..0089196b9e36 100644 > --- a/drivers/mtd/nand/raw/nand_timings.c > +++ b/drivers/mtd/nand/raw/nand_timings.c > @@ -292,6 +292,12 @@ static const struct nand_interface_config onfi_sdr_timings[] = { > }, > }; > > +/* All NAND chips share the same reset data interface: SDR mode 0 */ > +const struct nand_interface_config *nand_get_reset_interface(void) > +{ > + return &onfi_sdr_timings[0]; > +} > + > /** > * onfi_find_closest_sdr_mode - Derive the closest ONFI SDR timing mode given a > * set of timings > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h > index c2fcac38bcc2..b37eb5ee11c8 100644 > --- a/include/linux/mtd/rawnand.h > +++ b/include/linux/mtd/rawnand.h > @@ -1202,6 +1202,9 @@ static inline struct device_node *nand_get_flash_node(struct nand_chip *chip) > return mtd_get_of_node(nand_to_mtd(chip)); > } > > +/* Default/reset data interface */ > +const struct nand_interface_config *nand_get_reset_interface(void); > + > /* > * A helper for defining older NAND chips where the second ID byte fully > * defined the chip, including the geometry (chip size, eraseblock size, page
Boris Brezillon <boris.brezillon@collabora.com> wrote on Fri, 29 May 2020 09:32:05 +0200: > On Fri, 29 May 2020 01:16:01 +0200 > Miquel Raynal <miquel.raynal@bootlin.com> wrote: > > > All NAND chips will always use the same interface configuration for > > reset and at startup: SDR mode 0. Instead of copying around the > > interface configuration, let's just define a helper to retrieve the > > reset interface configuration object. > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > --- > > drivers/mtd/nand/raw/nand_base.c | 10 +++------- > > drivers/mtd/nand/raw/nand_timings.c | 6 ++++++ > > include/linux/mtd/rawnand.h | 3 +++ > > 3 files changed, 12 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c > > index 07a7d918b462..4a72ec221262 100644 > > --- a/drivers/mtd/nand/raw/nand_base.c > > +++ b/drivers/mtd/nand/raw/nand_base.c > > @@ -928,9 +928,7 @@ static int nand_reset_interface(struct nand_chip *chip, int chipnr) > > * timings to timing mode 0. > > */ > > > > - onfi_fill_interface_config(chip, &chip->interface_config, > > - NAND_SDR_IFACE, 0); > > - ret = ops->setup_interface(chip, chipnr, &chip->interface_config); > > + ret = ops->setup_interface(chip, chipnr, nand_get_reset_interface()); > > if (ret) > > pr_err("Failed to configure data interface to SDR timing mode 0\n"); > > > > @@ -2483,7 +2481,6 @@ EXPORT_SYMBOL_GPL(nand_subop_get_data_len); > > */ > > int nand_reset(struct nand_chip *chip, int chipnr) > > { > > - struct nand_interface_config saved_intf_config = chip->interface_config; > > int ret; > > > > ret = nand_reset_interface(chip, chipnr); > > @@ -2508,11 +2505,10 @@ int nand_reset(struct nand_chip *chip, int chipnr) > > * nand_setup_interface() uses ->set/get_features() which would > > * fail anyway as the parameter page is not available yet. > > */ > > - if (!memcmp(&chip->interface_config, &saved_intf_config, > > - sizeof(saved_intf_config))) > > + if (!memcmp(&chip->interface_config, nand_get_reset_interface(), > > + sizeof(chip->interface_config))) > > return 0; > > > > - chip->interface_config = saved_intf_config; > > Are you sure you can do that now? Looks like the current interface > config is not preserved, and you end up with mode 0 after a reset. You're probably right, I'll move that to the patch handling interface_config pointers. > > > ret = nand_setup_interface(chip, chipnr); > > if (ret) > > return ret; > > diff --git a/drivers/mtd/nand/raw/nand_timings.c b/drivers/mtd/nand/raw/nand_timings.c > > index 6c12cd52323d..0089196b9e36 100644 > > --- a/drivers/mtd/nand/raw/nand_timings.c > > +++ b/drivers/mtd/nand/raw/nand_timings.c > > @@ -292,6 +292,12 @@ static const struct nand_interface_config onfi_sdr_timings[] = { > > }, > > }; > > > > +/* All NAND chips share the same reset data interface: SDR mode 0 */ > > +const struct nand_interface_config *nand_get_reset_interface(void) > > +{ > > + return &onfi_sdr_timings[0]; > > +} > > + > > /** > > * onfi_find_closest_sdr_mode - Derive the closest ONFI SDR timing mode given a > > * set of timings > > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h > > index c2fcac38bcc2..b37eb5ee11c8 100644 > > --- a/include/linux/mtd/rawnand.h > > +++ b/include/linux/mtd/rawnand.h > > @@ -1202,6 +1202,9 @@ static inline struct device_node *nand_get_flash_node(struct nand_chip *chip) > > return mtd_get_of_node(nand_to_mtd(chip)); > > } > > > > +/* Default/reset data interface */ > > +const struct nand_interface_config *nand_get_reset_interface(void); > > + > > /* > > * A helper for defining older NAND chips where the second ID byte fully > > * defined the chip, including the geometry (chip size, eraseblock size, page >
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c index 07a7d918b462..4a72ec221262 100644 --- a/drivers/mtd/nand/raw/nand_base.c +++ b/drivers/mtd/nand/raw/nand_base.c @@ -928,9 +928,7 @@ static int nand_reset_interface(struct nand_chip *chip, int chipnr) * timings to timing mode 0. */ - onfi_fill_interface_config(chip, &chip->interface_config, - NAND_SDR_IFACE, 0); - ret = ops->setup_interface(chip, chipnr, &chip->interface_config); + ret = ops->setup_interface(chip, chipnr, nand_get_reset_interface()); if (ret) pr_err("Failed to configure data interface to SDR timing mode 0\n"); @@ -2483,7 +2481,6 @@ EXPORT_SYMBOL_GPL(nand_subop_get_data_len); */ int nand_reset(struct nand_chip *chip, int chipnr) { - struct nand_interface_config saved_intf_config = chip->interface_config; int ret; ret = nand_reset_interface(chip, chipnr); @@ -2508,11 +2505,10 @@ int nand_reset(struct nand_chip *chip, int chipnr) * nand_setup_interface() uses ->set/get_features() which would * fail anyway as the parameter page is not available yet. */ - if (!memcmp(&chip->interface_config, &saved_intf_config, - sizeof(saved_intf_config))) + if (!memcmp(&chip->interface_config, nand_get_reset_interface(), + sizeof(chip->interface_config))) return 0; - chip->interface_config = saved_intf_config; ret = nand_setup_interface(chip, chipnr); if (ret) return ret; diff --git a/drivers/mtd/nand/raw/nand_timings.c b/drivers/mtd/nand/raw/nand_timings.c index 6c12cd52323d..0089196b9e36 100644 --- a/drivers/mtd/nand/raw/nand_timings.c +++ b/drivers/mtd/nand/raw/nand_timings.c @@ -292,6 +292,12 @@ static const struct nand_interface_config onfi_sdr_timings[] = { }, }; +/* All NAND chips share the same reset data interface: SDR mode 0 */ +const struct nand_interface_config *nand_get_reset_interface(void) +{ + return &onfi_sdr_timings[0]; +} + /** * onfi_find_closest_sdr_mode - Derive the closest ONFI SDR timing mode given a * set of timings diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h index c2fcac38bcc2..b37eb5ee11c8 100644 --- a/include/linux/mtd/rawnand.h +++ b/include/linux/mtd/rawnand.h @@ -1202,6 +1202,9 @@ static inline struct device_node *nand_get_flash_node(struct nand_chip *chip) return mtd_get_of_node(nand_to_mtd(chip)); } +/* Default/reset data interface */ +const struct nand_interface_config *nand_get_reset_interface(void); + /* * A helper for defining older NAND chips where the second ID byte fully * defined the chip, including the geometry (chip size, eraseblock size, page
All NAND chips will always use the same interface configuration for reset and at startup: SDR mode 0. Instead of copying around the interface configuration, let's just define a helper to retrieve the reset interface configuration object. Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> --- drivers/mtd/nand/raw/nand_base.c | 10 +++------- drivers/mtd/nand/raw/nand_timings.c | 6 ++++++ include/linux/mtd/rawnand.h | 3 +++ 3 files changed, 12 insertions(+), 7 deletions(-)