[PATCH] mtd: spi-nor: core: Introduce spi_nor_abort_octal_dtr()

[liao jaime]

Hi Michael


>
> Hi,
>
> >> >> To me it seems, that spi_nor_select_{read,pp,erase}() will select
> >> >> the wrong commands/proto. So it should be fixed there. Probably
> >> >> shared_mask is wrong.
> >> > Features in shared_mask have include 2 parts as below.
> >> > 1. Flash support the feature (by parse SFDP or ID table flags)
> >> > 2. spi host controller support the feature (In
> >> > spi_nor_spimem_adjust_hwcaps)
> >> >
> >> > But I think Octal DTR is a special case because, in addition to the 2
> >> > points
> >> > mentioned above, there are also other conditions that need to be met.
> >>
> >> Correct. And btw we would have the same problem for quad_enable if we
> >> wouldn't
> >> have a default.
> >>
> >> So in spi_nor_default_setup() I'd suggest to add something along
> >>
> >> if (!nor->quad_enable)
> >>    shared_mask &= ~SNOR_HWCAPS_4_4_4;
> > I think this check is unnecessary for quad_enable.
> > Because of "spi_nor_sr2_bit1_quad_enable" would be a default
> > method for enabling quad mode in spi_nor_init_default_params().
>
> Yes it is at the moment. But if a flash happen to have no
> quad_enable we'll have a problem for whatever reason. so for the
> sake of completeness we should mask that bit.
Got it.

>
> >> /* skip octal command mode if we don't have a .octal_enable callback
> >> */
> >> if (!nor->octal_enable)
> >>    shared_mask &= ~SNOR_HWCAPS_8_8_8;
> > Sound great.
> >
> > Could I follow the suggestion for preparing v2 patch?
>
> sure.
>
> > BTW, I think discard is better than abort.
> > What do you think?
>
> Where do you need that word? You shouln'd need any more
> than the code above.
Ok.

>
> -michael

Thanks
Jaime