Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Date:   Mon, 15 Mar 2021 11:01:53 +0100
From:   Michael Walle <michael@walle.cc>
To:     Tudor.Ambarus@microchip.com
Cc:     vigneshr@ti.com, linux-mtd@lists.infradead.org,
        linux-kernel@vger.kernel.org, miquel.raynal@bootlin.com,
        richard@nod.at
Subject: Re: [PATCH v4 1/4] mtd: spi-nor: add OTP support
In-Reply-To: <f0ffd9b3-94d1-05ca-7e90-5014bf7a3db4@microchip.com>
References: <20210306000535.9890-1-michael@walle.cc>
 <20210306000535.9890-2-michael@walle.cc>
 <7dd4bfb0-bb38-20c8-68e1-ece836c847fa@microchip.com>
 <b83dbbcdee6bdeee0d494ba79fd792e4@walle.cc>
 <8b35d8f6-6ff8-9bf3-02bd-434cf46acccb@microchip.com>
 <f0ffd9b3-94d1-05ca-7e90-5014bf7a3db4@microchip.com>
User-Agent: Roundcube Webmail/1.4.11
Message-ID: <bbbc6230b7d60a2421520ce7d163be56@walle.cc>
Precedence: bulk

Am 2021-03-15 10:44, schrieb Tudor.Ambarus@microchip.com:
> On 3/15/21 11:39 AM, Tudor Ambarus - M18064 wrote:
>> On 3/15/21 11:23 AM, Michael Walle wrote:
>> 
>> cut
>> 
>>>>> diff --git a/drivers/mtd/spi-nor/otp.c b/drivers/mtd/spi-nor/otp.c
>>>>> new file mode 100644
>>>>> index 000000000000..4e301fd5156b
>>>>> --- /dev/null
>>>>> +++ b/drivers/mtd/spi-nor/otp.c
>>>>> @@ -0,0 +1,218 @@
>>>>> +// SPDX-License-Identifier: GPL-2.0
>>>>> +/*
>>>>> + * OTP support for SPI NOR flashes
>>>>> + *
>>>>> + * Copyright (C) 2021 Michael Walle <michael@walle.cc>> + */
>>>>> +
>>>>> +#include <linux/log2.h>
>>>>> +#include <linux/mtd/mtd.h>
>>>>> +#include <linux/mtd/spi-nor.h>
>>>>> +
>>>>> +#include "core.h"
>>>>> +
>>>>> +#define spi_nor_otp_ops(nor) ((nor)->params->otp.ops)
>>>>> +#define spi_nor_otp_region_len(nor) ((nor)->params->otp.org->len)
>>>>> +#define spi_nor_otp_n_regions(nor)
>>>>> ((nor)->params->otp.org->n_regions)
>>>> 
>>>> I don't like these wrappers because they gratuiously hide what's 
>>>> really
>>>> there.
>>>> I find the code more easier to read without these wrappers, because 
>>>> I
>>>> don't
>>>> have to memorize what these wrappers are doing, and I better see how
>>>> the code
>>>> is organized.
>>> 
>>> TBH I find it easier on the eye and I've never seen so much 
>>> dereferences
>>> as in mtd/spi-nor.
> 
> the dereferences will still be there, but will be just hidden by these 
> wrappers,
> don't they? Why would one prefer the wrappers?

That's why I wrote "easier on the eye", yes that is subjective. But
there are also technical aspects, for example, the helpers all
operate on "struct nor*", so you don't have to use the
dereference chain or build up local variables. For example, see
spi_nor_otp_read_write(), it doesn't have to know
anything about the "struct spi_nor_otp_organization".

And of course you could change the actual implementation
without touching the code everywhere,.

-michael