Re: [PATCH] mtd: rawnand: denali_dt: Add support for configuring SPARE_AREA_SKIP_BYTES

[Marek Vasut <marex@xxxxxxx>]

On 12/9/19 6:38 AM, Masahiro Yamada wrote:
[...]

>> diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c
>> index 3102ddbd8abdb..b6c463d021677 100644
>> --- a/drivers/mtd/nand/raw/denali.c
>> +++ b/drivers/mtd/nand/raw/denali.c
>> @@ -1302,14 +1302,21 @@ int denali_init(struct denali_controller *denali)
>>
>>         /*
>>          * Set how many bytes should be skipped before writing data in OOB.
>> +        * If a non-zero value has already been configured, update it in HW.
>>          * If a non-zero value has already been set (by firmware or something),
>>          * just use it. Otherwise, set the driver's default.
>>          */
>> -       denali->oob_skip_bytes = ioread32(denali->reg + SPARE_AREA_SKIP_BYTES);
>> -       if (!denali->oob_skip_bytes) {
>> -               denali->oob_skip_bytes = DENALI_DEFAULT_OOB_SKIP_BYTES;
>> +       if (denali->oob_skip_bytes) {
>>                 iowrite32(denali->oob_skip_bytes,
>>                           denali->reg + SPARE_AREA_SKIP_BYTES);
>> +       } else {
>> +               denali->oob_skip_bytes =
>> +                       ioread32(denali->reg + SPARE_AREA_SKIP_BYTES);
>> +               if (!denali->oob_skip_bytes) {
>> +                       denali->oob_skip_bytes = DENALI_DEFAULT_OOB_SKIP_BYTES;
>> +                       iowrite32(denali->oob_skip_bytes,
>> +                                 denali->reg + SPARE_AREA_SKIP_BYTES);
> 
> This fallback is ugly, and should be removed, I think.
> It is only reachable by PCI platform (Intel MRST), where
> DENALI_DEFAULT_OOB_SKIP_BYTES is probably useless.

This fallback retains the original behavior on all platforms. It might
not be to your liking, but it does not break other platforms while
fixing SoCFPGA. We don't know what other platforms might be depending on
this behavior, do we ?

>> @@ -209,6 +213,8 @@ static int denali_dt_probe(struct platform_device *pdev)
>>         denali->clk_rate = clk_get_rate(dt->clk);
>>         denali->clk_x_rate = clk_get_rate(dt->clk_x);
>>
>> +       denali->oob_skip_bytes = data->oob_skip_bytes;
>> +
> 
> Please move this to the relevant hunk.
> Preferably, based on this:
> http://patchwork.ozlabs.org/patch/1205912/

I don't understand what you want me to change ? Shall I rebase this on
top of your patch from today and repost ?

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