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

[Mario Kicherer <dev@kicherer.org>]

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

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