Re: [PATCH 1/2] mtd: spinand: Introduce a way to avoid raw access

[Takahiro Kuwano <tkuw584924@gmail.com>]

Hi,

On 11/12/2024 3:54 AM, Miquel Raynal wrote:
> Hi,
> 
> On 31/10/2024 at 11:21:54 +09, tkuw584924@gmail.com wrote:
> 
>> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
>>
>> SkyHigh spinand device has ECC enable bit in configuration register but
>> it must be always enabled. If ECC is disabled, read and write ops
>> results in undetermined state. For such devices, a way to avoid raw
>> access is needed.
>>
>> Introduce SPINAND_NO_RAW_ACCESS flag to advertise the device does not
>> support raw access. Read and write page in raw mode for the device
>> returns error.
>>
>> Checking and marking BBM need to be performed with ECC enabled to read
>> and write the BBM correctly.
> 
> I see your point but I'm a bit puzzled by how it's being done.
> 
> First, you disregard completely the isbad() and markbad()
> situations. Please have look into that because these functions are
> broken with your devices.
> 
Yes... now I understand how they are broken.

> Second, what about adding this detail to the ondie ECC engine? You could
> simply return an error from there, so basically a single (or maybe two)
> changes overall.
> 
Thank you for the suggestion.
How about change like below in prepare_finish_io_req()?

static int spinand_ondie_ecc_prepare_io_req(struct nand_device *nand,
					    struct nand_page_io_req *req)
{
	struct spinand_device *spinand = nand_to_spinand(nand);
	bool enable = (req->mode != MTD_OPS_RAW);

+	/*
+	 * For the devices that prohibit raw access (on-die ECC must be always
+	 * enabled), return error in case of raw data access. Accessing to OOB
+	 * needs to be allowed with on-die ECC enabled to support BBM checking
+	 * and marking.
+	 */
+	if (!enable && spinand->flags & SPINAND_NO_RAW_ACCESS) {
+		if (req->datalen)
+			return -ENOTSUPP;
+
+		enable = true;
+	}
...

> Thanks,
> Miquèl

Thanks!
Takahiro

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/