[PATCH] mtd: spi-nor: core: Discard HW capabilities if no enable function

[liao jaime]

Hi Michael

>
> Hi,
>
> > Discard corresponding HW capabilities to prevent carrying the
> > wrong protocol if no QUAD/Octal DTR enable function hooked.
> >
> > Signed-off-by: JaimeLiao <jaimeliao at mxic.com.tw>
> > ---
> >  drivers/mtd/spi-nor/core.c  | 13 +++++++------
> >  include/linux/mtd/spi-nor.h |  3 +++
> >  2 files changed, 10 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> > index 1c443fe568cf..a3988e8768b3 100644
> > --- a/drivers/mtd/spi-nor/core.c
> > +++ b/drivers/mtd/spi-nor/core.c
> > @@ -2621,6 +2621,13 @@ static int spi_nor_default_setup(struct spi_nor
> > *nor,
> >        */
> >       shared_mask = hwcaps->mask & params->hwcaps.mask;
> >
> > +     /* Mask out Octal DTR if no enable function */
> > +     if (!params->set_octal_dtr)
> > +             shared_mask &= ~SNOR_HWCAPS_X_X_X_DTR;
>
> please also create a new SNOR_HWCAPS_8_8_8_DTR. While this is
> the same for now, I suspect the SNOR_HWCAPS_X_X_X_DTR is might also
> carry the 2_2_2_DTR and 4_4_4_DTR flags, just like SNOR_HWCAPS_X_X_X.
> In any case, it's easier to read and more consistent with the code
> below if you use SNOR_HWCAPS_8_8_8_DTR.
Got it.
I will prepare the next version.

>
> > +
> > +     if (!params->quad_enable)
> > +             shared_mask &= ~SNOR_HWCAPS_4_4_4;
> > +
> >       if (nor->spimem) {
> >               /*
> >                * When called from spi_nor_probe(), all caps are set and we
> > @@ -3093,9 +3100,6 @@ static int spi_nor_set_octal_dtr(struct spi_nor
> > *nor, bool enable)
> >  {
> >       int ret;
> >
> > -     if (!nor->params->set_octal_dtr)
> > -             return 0;
> > -
>
> I'd keep these checks, just in case.
OK

>
> >       if (!(nor->read_proto == SNOR_PROTO_8_8_8_DTR &&
> >             nor->write_proto == SNOR_PROTO_8_8_8_DTR))
> >               return 0;
> > @@ -3123,9 +3127,6 @@ static int spi_nor_set_octal_dtr(struct spi_nor
> > *nor, bool enable)
> >   */
> >  static int spi_nor_quad_enable(struct spi_nor *nor)
> >  {
> > -     if (!nor->params->quad_enable)
> > -             return 0;
> > -
>
> same here.
OK

>
> Otherwise looks good.
>
> -michael
>
> >       if (!(spi_nor_get_protocol_width(nor->read_proto) == 4 ||
> >             spi_nor_get_protocol_width(nor->write_proto) == 4))
> >               return 0;
> > diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> > index cdcfe0fd2e7d..c421018dadf7 100644
> > --- a/include/linux/mtd/spi-nor.h
> > +++ b/include/linux/mtd/spi-nor.h
> > @@ -266,6 +266,9 @@ struct spi_nor_hwcaps {
> >  #define SNOR_HWCAPS_PP_8_8_8         BIT(22)
> >  #define SNOR_HWCAPS_PP_8_8_8_DTR     BIT(23)
> >
> > +#define SNOR_HWCAPS_4_4_4    (SNOR_HWCAPS_READ_4_4_4 |       \
> > +                              SNOR_HWCAPS_PP_4_4_4)
> > +
> >  #define SNOR_HWCAPS_X_X_X    (SNOR_HWCAPS_READ_2_2_2 |       \
> >                                SNOR_HWCAPS_READ_4_4_4 |       \
> >                                SNOR_HWCAPS_READ_8_8_8 |       \

Thanks
Jaime