Message ID | 20220622091145.1207923-1-kory.maincent@bootlin.com |
---|---|
State | Accepted |
Commit | 7886c45d422ca92f86a88580442bd526435bec25 |
Delegated to: | Tom Rini |
Headers | show |
Series | [v2] mtd: rawnand: Add support to dedicated function to set timings | expand |
Hi Köry, kory.maincent@bootlin.com wrote on Wed, 22 Jun 2022 11:11:45 +0200: > From: Kory Maincent <kory.maincent@bootlin.com> > > With the current code if the board has an ONFI compliant NAND without > support to the get and set features, U-boot returns an ENOTSUP error when > trying to tune the timings which prevents the probe of the device. > Indeed onfi_set_features() return ENOTSUP error if set/get features is not > supported. In the case of timings we should not return ENOTSUP because we > can use the default timings. The NAND is already capable of listening at > its highest supported rate, so we assume in this case that it is fine to > skip the operation. > > Fix it by adding an intermediate nand_onfi_set_timings() function which > does not error out if set/get feature is not supported. > > Signed-off-by: Kory Maincent <kory.maincent@bootlin.com> Seems legitimate. Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com> > --- > > Change since v1: > - Update commit message > > drivers/mtd/nand/raw/nand_base.c | 30 +++++++++++++++++++----------- > 1 file changed, 19 insertions(+), 11 deletions(-) > > diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c > index 6f81257cf1..e8ece0a4a0 100644 > --- a/drivers/mtd/nand/raw/nand_base.c > +++ b/drivers/mtd/nand/raw/nand_base.c > @@ -974,6 +974,22 @@ static int nand_reset_data_interface(struct nand_chip *chip, int chipnr) > return ret; > } > > +static int nand_onfi_set_timings(struct mtd_info *mtd, struct nand_chip *chip) > +{ > + if (!chip->onfi_version || > + !(le16_to_cpu(chip->onfi_params.opt_cmd) > + & ONFI_OPT_CMD_SET_GET_FEATURES)) > + return 0; > + > + u8 tmode_param[ONFI_SUBFEATURE_PARAM_LEN] = { > + chip->onfi_timing_mode_default, > + }; > + > + return chip->onfi_set_features(mtd, chip, > + ONFI_FEATURE_ADDR_TIMING_MODE, > + tmode_param); > +} > + > /** > * nand_setup_data_interface - Setup the best data interface and timings > * @chip: The NAND chip > @@ -999,17 +1015,9 @@ static int nand_setup_data_interface(struct nand_chip *chip, int chipnr) > * Ensure the timing mode has been changed on the chip side > * before changing timings on the controller side. > */ > - if (chip->onfi_version) { > - u8 tmode_param[ONFI_SUBFEATURE_PARAM_LEN] = { > - chip->onfi_timing_mode_default, > - }; > - > - ret = chip->onfi_set_features(mtd, chip, > - ONFI_FEATURE_ADDR_TIMING_MODE, > - tmode_param); > - if (ret) > - goto err; > - } > + ret = nand_onfi_set_timings(mtd, chip); > + if (ret) > + goto err; > > ret = chip->setup_data_interface(mtd, chipnr, chip->data_interface); > err: Thanks, Miquèl
On Wed, Jun 22, 2022 at 11:11:45AM +0200, kory.maincent@bootlin.com wrote: > From: Kory Maincent <kory.maincent@bootlin.com> > > With the current code if the board has an ONFI compliant NAND without > support to the get and set features, U-boot returns an ENOTSUP error when > trying to tune the timings which prevents the probe of the device. > Indeed onfi_set_features() return ENOTSUP error if set/get features is not > supported. In the case of timings we should not return ENOTSUP because we > can use the default timings. The NAND is already capable of listening at > its highest supported rate, so we assume in this case that it is fine to > skip the operation. > > Fix it by adding an intermediate nand_onfi_set_timings() function which > does not error out if set/get feature is not supported. > > Signed-off-by: Kory Maincent <kory.maincent@bootlin.com> > Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com> Applied to u-boot/next, thanks!
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c index 6f81257cf1..e8ece0a4a0 100644 --- a/drivers/mtd/nand/raw/nand_base.c +++ b/drivers/mtd/nand/raw/nand_base.c @@ -974,6 +974,22 @@ static int nand_reset_data_interface(struct nand_chip *chip, int chipnr) return ret; } +static int nand_onfi_set_timings(struct mtd_info *mtd, struct nand_chip *chip) +{ + if (!chip->onfi_version || + !(le16_to_cpu(chip->onfi_params.opt_cmd) + & ONFI_OPT_CMD_SET_GET_FEATURES)) + return 0; + + u8 tmode_param[ONFI_SUBFEATURE_PARAM_LEN] = { + chip->onfi_timing_mode_default, + }; + + return chip->onfi_set_features(mtd, chip, + ONFI_FEATURE_ADDR_TIMING_MODE, + tmode_param); +} + /** * nand_setup_data_interface - Setup the best data interface and timings * @chip: The NAND chip @@ -999,17 +1015,9 @@ static int nand_setup_data_interface(struct nand_chip *chip, int chipnr) * Ensure the timing mode has been changed on the chip side * before changing timings on the controller side. */ - if (chip->onfi_version) { - u8 tmode_param[ONFI_SUBFEATURE_PARAM_LEN] = { - chip->onfi_timing_mode_default, - }; - - ret = chip->onfi_set_features(mtd, chip, - ONFI_FEATURE_ADDR_TIMING_MODE, - tmode_param); - if (ret) - goto err; - } + ret = nand_onfi_set_timings(mtd, chip); + if (ret) + goto err; ret = chip->setup_data_interface(mtd, chipnr, chip->data_interface); err: