| Message ID | 20240517-gpmi_nand-v1-3-73bb8d2cd441@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | mtd: nand: gpmi-nand: add imx8qxp gpmi nand support | expand |
Hi Frank, Frank.Li@nxp.com wrote on Fri, 17 May 2024 14:09:50 -0400: > From: Han Xu <han.xu@nxp.com> > > Add "fsl,imx8qxp-gpmi-nand" compatible string. iMX8QXP gpmi nand is similar > with iMX7D. But it using 4 clock: "gpmi_io", "gpmi_apb", "gpmi_bch" and to? is clocks > "gpmi_bch_apb". > > Signed-off-by: Han Xu <han.xu@nxp.com> > Signed-off-by: Frank Li <Frank.Li@nxp.com> > --- > drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c | 20 +++++++++++++++++--- > drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.h | 4 ++++ > 2 files changed, 21 insertions(+), 3 deletions(-) > > diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c > index e71ad2fcec232..f90c5207bacb6 100644 > --- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c > +++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c > @@ -983,7 +983,8 @@ static int gpmi_setup_interface(struct nand_chip *chip, int chipnr, > return PTR_ERR(sdr); > > /* Only MX28/MX6 GPMI controller can reach EDO timings */ > - if (sdr->tRC_min <= 25000 && !GPMI_IS_MX28(this) && !GPMI_IS_MX6(this)) > + if (sdr->tRC_min <= 25000 && !GPMI_IS_MX28(this) && > + !(GPMI_IS_MX6(this) || GPMI_IS_MX8(this))) Feels completely redundant, no? If it's not an imx6 nor an imx28, it already returns -ENOTSUPP. > return -ENOTSUPP; Fine otherwise. Thanks, Miquèl
On Fri, May 17, 2024 at 08:39:04PM +0200, Miquel Raynal wrote: > Hi Frank, > > Frank.Li@nxp.com wrote on Fri, 17 May 2024 14:09:50 -0400: > > > From: Han Xu <han.xu@nxp.com> > > > > Add "fsl,imx8qxp-gpmi-nand" compatible string. iMX8QXP gpmi nand is similar > > with iMX7D. But it using 4 clock: "gpmi_io", "gpmi_apb", "gpmi_bch" and > > to? is clocks > > > "gpmi_bch_apb". > > > > Signed-off-by: Han Xu <han.xu@nxp.com> > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > --- > > drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c | 20 +++++++++++++++++--- > > drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.h | 4 ++++ > > 2 files changed, 21 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c > > index e71ad2fcec232..f90c5207bacb6 100644 > > --- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c > > +++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c > > @@ -983,7 +983,8 @@ static int gpmi_setup_interface(struct nand_chip *chip, int chipnr, > > return PTR_ERR(sdr); > > > > /* Only MX28/MX6 GPMI controller can reach EDO timings */ > > - if (sdr->tRC_min <= 25000 && !GPMI_IS_MX28(this) && !GPMI_IS_MX6(this)) > > + if (sdr->tRC_min <= 25000 && !GPMI_IS_MX28(this) && > > + !(GPMI_IS_MX6(this) || GPMI_IS_MX8(this))) > > Feels completely redundant, no? If it's not an imx6 nor an imx28, it > already returns -ENOTSUPP. if this is mx8 sdr->tRC_min <= 25000: true !GPMI_IS_MX28(this): true !GPMI_IS_MX6(this): true so whole statement is true after change sdr->tRC_min <= 25000: true !GPMI_IS_MX28(this): true !(GPMI_IS_MX6(this) || GPMI_IS_MX8(this)): false (GPMI_IS_MX6(this) || GPMI_IS_MX8(this)): true GPMI_IS_MX6(this): false GPMI_IS_MX8(this): true so whole statement is false. Maybe use sdr->tRC_min <= 25000 && !GPMI_IS_MX28(this) && !(GPMI_IS_MX6(this) && !(GPMI_IS_MX8(this)) looks better. Or use a drvflag. if (sdr->tRC_min <= 25000 && (!this->drvflag->support_tRC_min)) Frank > > > return -ENOTSUPP; > > > Fine otherwise. > > Thanks, > Miquèl
diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c index e71ad2fcec232..f90c5207bacb6 100644 --- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c +++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c @@ -983,7 +983,8 @@ static int gpmi_setup_interface(struct nand_chip *chip, int chipnr, return PTR_ERR(sdr); /* Only MX28/MX6 GPMI controller can reach EDO timings */ - if (sdr->tRC_min <= 25000 && !GPMI_IS_MX28(this) && !GPMI_IS_MX6(this)) + if (sdr->tRC_min <= 25000 && !GPMI_IS_MX28(this) && + !(GPMI_IS_MX6(this) || GPMI_IS_MX8(this))) return -ENOTSUPP; /* Stop here if this call was just a check */ @@ -1178,6 +1179,18 @@ static const struct gpmi_devdata gpmi_devdata_imx7d = { .clks_count = ARRAY_SIZE(gpmi_clks_for_mx7d), }; +static const char *gpmi_clks_for_mx8qxp[GPMI_CLK_MAX] = { + "gpmi_io", "gpmi_apb", "gpmi_bch", "gpmi_bch_apb", +}; + +static const struct gpmi_devdata gpmi_devdata_imx8qxp = { + .type = IS_MX8QXP, + .bch_max_ecc_strength = 62, + .max_chain_delay = 12000, + .clks = gpmi_clks_for_mx8qxp, + .clks_count = ARRAY_SIZE(gpmi_clks_for_mx8qxp), +}; + static int acquire_register_block(struct gpmi_nand_data *this, const char *res_name) { @@ -2281,7 +2294,7 @@ static int gpmi_init_last(struct gpmi_nand_data *this) * (1) the chip is imx6, and * (2) the size of the ECC parity is byte aligned. */ - if (GPMI_IS_MX6(this) && + if ((GPMI_IS_MX6(this) || GPMI_IS_MX8(this)) && ((bch_geo->gf_len * bch_geo->ecc_strength) % 8) == 0) { ecc->read_subpage = gpmi_ecc_read_subpage; chip->options |= NAND_SUBPAGE_READ; @@ -2692,7 +2705,7 @@ static int gpmi_nand_init(struct gpmi_nand_data *this) this->base.ops = &gpmi_nand_controller_ops; chip->controller = &this->base; - ret = nand_scan(chip, GPMI_IS_MX6(this) ? 2 : 1); + ret = nand_scan(chip, (GPMI_IS_MX6(this) || GPMI_IS_MX8(this)) ? 2 : 1); if (ret) goto err_out; @@ -2721,6 +2734,7 @@ static const struct of_device_id gpmi_nand_id_table[] = { { .compatible = "fsl,imx6q-gpmi-nand", .data = &gpmi_devdata_imx6q, }, { .compatible = "fsl,imx6sx-gpmi-nand", .data = &gpmi_devdata_imx6sx, }, { .compatible = "fsl,imx7d-gpmi-nand", .data = &gpmi_devdata_imx7d,}, + { .compatible = "fsl,imx8qxp-gpmi-nand", .data = &gpmi_devdata_imx8qxp, }, {} }; MODULE_DEVICE_TABLE(of, gpmi_nand_id_table); diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.h b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.h index c3ff56ac62a7e..502f01a8338c3 100644 --- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.h +++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.h @@ -78,6 +78,7 @@ enum gpmi_type { IS_MX6Q, IS_MX6SX, IS_MX7D, + IS_MX8QXP, }; struct gpmi_devdata { @@ -172,8 +173,11 @@ struct gpmi_nand_data { #define GPMI_IS_MX6Q(x) ((x)->devdata->type == IS_MX6Q) #define GPMI_IS_MX6SX(x) ((x)->devdata->type == IS_MX6SX) #define GPMI_IS_MX7D(x) ((x)->devdata->type == IS_MX7D) +#define GPMI_IS_MX8QXP(x) ((x)->devdata->type == IS_MX8QXP) #define GPMI_IS_MX6(x) (GPMI_IS_MX6Q(x) || GPMI_IS_MX6SX(x) || \ GPMI_IS_MX7D(x)) + +#define GPMI_IS_MX8(x) (GPMI_IS_MX8QXP(x)) #define GPMI_IS_MXS(x) (GPMI_IS_MX23(x) || GPMI_IS_MX28(x)) #endif