[PATCH v1] mtd: rawnand: meson: fix bitmask for length in command word

[Miquel Raynal]

Hi Arseniy,

avkrasnov at sberdevices.ru wrote on Tue, 28 Mar 2023 18:56:19 +0300:

> Hello!
> 
> @Miquel Raynal, what is the status of this patch?

Please, it's been 6 days, there is also a maintainer (Liang) in
between, I'm fine with the patch but it was too late to take it as
part of my previous fixes PR. As said earlier today on the mailing list
to Christophe I will make another fixes PR next week (I'll wait for the
current one to be part of the next -rc tag).

By the way any reason not to have Cc'ed stable?

> 
> Thanks, Arseniy
> 
> On 23.03.2023 10:57, Arseniy Krasnov wrote:
> > 
> > 
> > On 22.03.2023 23:10, Martin Blumenstingl wrote:  
> >> Hello Arseniy,
> >>
> >> thank you for submitting this fix!  
> > Thanks!  
> >>
> >> On Wed, Mar 22, 2023 at 7:45 PM Arseniy Krasnov
> >> <avkrasnov at sberdevices.ru> wrote:  
> >>>
> >>> Valid mask is 0x3FFF, without this patch the following problems were
> >>> found:
> >>>
> >>> 1) [    0.938914] Could not find a valid ONFI parameter page, trying
> >>>                   bit-wise majority to recover it
> >>>    [    0.947384] ONFI parameter recovery failed, aborting
> >>>
> >>> 2) Read with disabled ECC mode was broken.
> >>>
> >>> Fixes: 8fae856c5350 ("mtd: rawnand: meson: add support for Amlogic NAND flash controller")
> >>> Signed-off-by: Arseniy Krasnov <AVKrasnov at sberdevices.ru>  
> >> This matches what I can see in the old vendor driver, so:  
> > Moreover it was clear that mask of 0x3f is too small for length of data in
> > bytes, for example for 2048 + OOB size.  
> >> Acked-by: Martin Blumenstingl <martin.blumenstingl at googlemail.com>
> >>
> >> [...]  
> >>> -               cmd = (len & GENMASK(5, 0)) | scrambler | DMA_DIR(dir);
> >>> +               cmd = (len & GENMASK(13, 0)) | scrambler | DMA_DIR(dir);  
> >> My understanding of the vendor driver is that this "len" is only used
> >> for "raw" access (my own words: any access that doesn't use the HW ECC
> >> engine).  
> > Exactly, 'len' is only for raw access.  
> >> As a future improvement (no need to update re-send this patch) it
> >> would be great to have a #define with a meaningful name for
> >> "GENMASK(13, 0)" (maybe something like NFC_CMD_RAW_LENGTH) as it's
> >> used in multiple places now  
> > Ack
> > 
> > Thanks, Arseniy  
> >>
> >>
> >> Best regards,
> >> Martin  


Thanks,
Miquèl