Re: [PATCH] dt-bindings: mtd: spi-nor: deprecate Everspin MRAM devices

[Tudor Ambarus]

On 6/5/24 18:40, Conor Dooley wrote:
> On Tue, Jun 04, 2024 at 07:42:16PM +0200, Michael Walle wrote:
>> On Tue Jun 4, 2024 at 7:01 PM CEST, Conor Dooley wrote:
>>> On Tue, Jun 04, 2024 at 09:42:31AM +0200, Michael Walle wrote:
>>>> These devices are more like an AT25 compatible EEPROM instead of
>>>> flashes. Like an EEPROM the user doesn't need to explicitly erase the
>>>> memory, nor are there sectors or pages. Thus, instead of the SPI-NOR
>>>> (flash) driver, one should instead use the at25 EEPROM driver.
>>>>
>>>> Signed-off-by: Michael Walle <mwalle@xxxxxxxxxx>
>>>> Cc: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
>>>> Cc: Thorsten Scherer <t.scherer@xxxxxxxxxxxx>
>>>> Cc: Marek Vasut <marex@xxxxxxx>
>>>> Cc: Imre Kaloz <kaloz@xxxxxxxxxxx>
>>>> Cc: Andrew Lunn <andrew@xxxxxxx>
>>>> Cc: Flavio Suligoi <f.suligoi@xxxxxxx>
>>>> ---
>>>> The referenced binding only supports the true AT25 compatible EEPROMs
>>>> where you have to specify additional properties like size and page size
>>>> or cypress FRAM devices where all the properties are discovered by the
>>>> driver. I don't have the actual hardware, therefore I can't work on a
>>>> proper driver and binding. But I really want to deprecate the use of
>>>> these EEPROM like devices in SPI-NOR. So as a first step, mark the
>>>> devices in the DT bindings as deprecated.
>>>>
>>>> There are three in-tree users of this. I hope I've CCed all the relevant
>>>> people. With the switch to the at25 driver also comes a user-space
>>>> facing change: there is no more MTD device. Instead there is an "eeprom"
>>>> file in /sys now, just like for every other EEPROM.
>>>>
>>>> Marek already expressed, that the sps1 dts can likely be removed
>>>> altogether. I'd like to hear from the other board DTS maintainers if
>>>> they seem some problems moving to the EEPROM interface - or maybe that
>>>> device isn't used at all anyway. So in the end, we can hopefully move
>>>> all the users over to the at25 driver.
>>>> ---
>>>>  Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml | 9 ++++++++-
>>>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
>>>> index 6e3afb42926e..2dccb6b049ea 100644
>>>> --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
>>>> +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
>>>> @@ -21,7 +21,6 @@ properties:
>>>>                (m25p(40|80|16|32|64|128)|\
>>>>                n25q(32b|064|128a11|128a13|256a|512a|164k)))|\
>>>>                atmel,at25df(321a|641|081a)|\
>>>> -              everspin,mr25h(10|40|128|256)|\
>>>>                (mxicy|macronix),mx25l(4005a|1606e|6405d|8005|12805d|25635e)|\
>>>>                (mxicy|macronix),mx25u(4033|4035)|\
>>>>                (spansion,)?s25fl(128s|256s1|512s|008k|064k|164k)|\
>>>> @@ -42,6 +41,14 @@ properties:
>>>>                - spansion,s25fs512s
>>>>            - const: jedec,spi-nor
>>>>        - const: jedec,spi-nor
>>>> +
>>>> +      # Deprecated bindings
>>>> +      - items:
>>>> +          - pattern: "^everspin,mr25h(10|40|128|256)$"
>>>> +          - const: jedec,spi-nor
>>>> +        description:
>>>> +          Deprecated binding, use Documentation/devicetree/bindings/eeprom/at25.yaml.
>>>> +        deprecated: true
>>>
>>> The idea here seems okay, but directing people to use the at25 binding,
>>> without actually documenting the replacement compatibles etc is far from
>>> ideal. I think even a wording change that points out that that these
>>> devices need to be documented in that file would be an improvement, the
>>> current wording makes it seem like the works been done.
>>> Until there's a replacement driver, I don't think you could really
>>> expect anyone to move to a new binding anyway.
>>
>> Fair enough. The driver is already there and it basically works -
>> Flavio is already using it. It is just, that at the moment you have
>> to use the (deprecated) "atmel,at25" compatible and you'll have to
>> specify pagesize etc. That is really hacky, because F/MRAM devices
>> doesn't have a pagesize.
>>
>> Anyway, I was already working on the at25 binding but then I've
>> noticed that the current FRAM binding is really hardcoded to cypress
>> devices and as mentioned in the commit message, I don't have any

Takahiro from cc may help with the cypress FRAM testing.

>> hardware to actually write the proper driver support. Maybe we
>> should settle on the binding first, i.e.
>>
>>  compatible = "everspin,mr25", "atmel,at25";
>>  size = <N>;
>>
>> vs
>>
>>  compatible = "everspin,mr25h256"; # no size needed
> 
> I dunno, I am usually biased to having the more specific compatible
> and not needing the extra properties.

I agree with the more specific compatible idea, but we shall aim that
the specific compatible to be generic: "spi-fram" and maybe "spi-mram".
Can it be done?

> 
>>
>> For reference, the already supported cypress fram has the following:
>>
>>  compatible = "cypress,fm25", "atmel,at25";
>>  # no size needed, because the driver will figure it out by reading
>>  # the ID
>>
>> Besides that, I would really get some feedback from the three
>> in-tree users on migrating to the EEPROM driver and thus away from
>> MTD.
>>
>> -michael
>>