Re: [PATCH v2] mtd: spinand: Do not re-enable continuous reads vetoed during variant selection

[Miquel Raynal]

Hi Frieder,

> Fixes: 6eb7c193e751 ("mtd: spinand: Use secondary ops for continuous reads")
> Assisted-by: LLM
> Signed-off-by: Frieder Schrempf <frieder.schrempf@xxxxxxxxxx>
> ---
> I was using an LLM to check sync patches for U-Boot and the LLM reported
> a bug in the upstream kernel code. To me it looks like this is indeed a
> bug and the fix also looks correct to me, but I would like to mention, that
> this is purely LLM-generated and theoretical. I didn't verify the bug and
> the fix on actual hardware.
>
> @Miquel: Do you have a hardware setup that would be affected by this?

I do not.

> Do you think there is a better fix?

I agree with your bug analysis and with the proposal, just a few nits below.

> --- a/include/linux/mtd/spinand.h
> +++ b/include/linux/mtd/spinand.h
> @@ -772,6 +772,9 @@ struct spinand_mem_ops {
>   *		suitable to use or not in general with this chip/configuration.
>   *		A per-transfer check must of course be done to ensure it is
>   *		actually relevant to enable this feature.
> + * @has_cont_read_variants: The chip needs dedicated continuous read from
> + *			    cache operations, which must then be available for
> + *			    the bus interface in use

"The chip needs dedicated continuous read from cache variants"

>   * @set_cont_read: Enable/disable the continuous read feature
>   * @priv: manufacturer private data
>   * @fact_otp: SPI NAND factory OTP info.
> @@ -810,6 +813,7 @@ struct spinand_device {
>  	int (*configure_chip)(struct spinand_device *spinand,
>  			      enum spinand_bus_interface iface);
>  	bool cont_read_possible;
> +	bool has_cont_read_variants;

Can we name it needs_cont_read_op?

>  	int (*set_cont_read)(struct spinand_device *spinand,
>  			     bool enable);
>  	int (*set_randomizer)(struct spinand_device *spinand,
>

The rest is good to me.

Thanks,
Miquèl