Re: [PATCH v2] mtd: spinand: Add support for AllianceMemory AS5F34G04SND

Mario Kicherer
2023-01-25T12:13:32Z

Hello Miquel,

On 2023-01-23 11:33, Miquel Raynal wrote:
>> +static int as5f34g04snd_ooblayout_ecc(struct mtd_info *mtd, int
>> section,
>> + struct mtd_oob_region *region)
>> +{
>> + if (section > 3)
>> + return -ERANGE;
>> +
>> + region->offset = 0x48;
>> + region->length = 0x38;
>
> Why would you have three sections if you have static offsets and
> length? That does not look correct.

Hm, I do not remember. I guess this is a leftover I forgot to remove.

>> + /*
>> + * It is unclear how many bytes are used for the bad block marker.
>> We
>> + * reserve one byte here.
>
> So far we reserved two bytes in Linux to mimic the raw NAND BBM.

Ok, changed.

>> +static int am_ecc_get_status(struct spinand_device *spinand, u8
>> status)
>> +{
>> + switch (status & AM_STATUS_ECC_BITMASK) {
>> + case AM_STATUS_ECC_NONE_DETECTED:
>> + return 0;
>> +
>> + case AM_STATUS_ECC_1_CORRECTED:
>> + return 1;
>> +
>> + case AM_STATUS_ECC_MAX_CORRECTED:
>> + return 8;
>> +
>> + case AM_STATUS_ECC_1_DETECTED:
>
> What does this mean "1 detected"?

According to the manual, the chip can report that a bit error was
detected
but could not be corrected.

>> +static const struct spinand_manufacturer_ops
>> alliancememory_spinand_manuf_ops = {
>> + .init = alliancememory_spinand_init,
>
> I don't think .init is mandatory, so if you do nothing in it, you
> can just drop it.

Done.

Thanks for the review!
Mario