diff mbox series

[v3,3/3] dt-bindings: eeprom: at24: Add Microchip 24AA025E48/24AA025E64

Message ID 20240628080146.49545-4-andrei.simion@microchip.com (mailing list archive)
State New, archived
Headers show
Series Read MAC address through NVMEM for sama7g5ek | expand

Commit Message

Andrei Simion June 28, 2024, 8:01 a.m. UTC
Add compatible for Microchip 24AA025E48/24AA025E64 EEPROMs.

Reviewed-by: Connor Dooley <conor.dooley@microchip.com>
Signed-off-by: Andrei Simion <andrei.simion@microchip.com>
---
v2 -> v3:
- commit subject changed to reference Microchip 24AA025E48/24AA025E64
- drop the pattern: mac02e4$ and mac02e6$ and a-z from regex
- add these two devices down at the bottom
- added Reviewed-by

v1 -> v2:
- change pattern into "^atmel,(24(c|cs|mac)[a-z0-9]+|spd)$" to keep simpler
---
 Documentation/devicetree/bindings/eeprom/at24.yaml | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Conor Dooley June 28, 2024, 9:09 a.m. UTC | #1
Hey,

On Fri, Jun 28, 2024 at 11:01:46AM +0300, Andrei Simion wrote:
> Add compatible for Microchip 24AA025E48/24AA025E64 EEPROMs.
> 
> Reviewed-by: Connor Dooley <conor.dooley@microchip.com>
                 ^^             ^
There's no way that I provided a tag with my name spelt incorrectly
given I use a macro to insert them. Please copy-paste tags or use b4
to pick them up, rather than type them out yourself.

> Signed-off-by: Andrei Simion <andrei.simion@microchip.com>
> ---
> v2 -> v3:
> - commit subject changed to reference Microchip 24AA025E48/24AA025E64
> - drop the pattern: mac02e4$ and mac02e6$ and a-z from regex
> - add these two devices down at the bottom
> - added Reviewed-by
> 
> v1 -> v2:
> - change pattern into "^atmel,(24(c|cs|mac)[a-z0-9]+|spd)$" to keep simpler
> ---
>  Documentation/devicetree/bindings/eeprom/at24.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/eeprom/at24.yaml b/Documentation/devicetree/bindings/eeprom/at24.yaml
> index 3c36cd0510de..699c2bbc16f5 100644
> --- a/Documentation/devicetree/bindings/eeprom/at24.yaml
> +++ b/Documentation/devicetree/bindings/eeprom/at24.yaml
> @@ -132,6 +132,10 @@ properties:
>                - renesas,r1ex24128
>                - samsung,s524ad0xd1
>            - const: atmel,24c128
> +      - items:
> +          - const: microchip,24aa025e48
> +      - items:
> +          - const: microchip,24aa025e64

I don't think this patch works, the schema has a select in it that only
matches ^atmel,(24(c|cs|mac)[0-9]+|spd)$. You either need to have these
fall back to an existing compatible (iff actually compatible) or else do
something like:
diff --git a/Documentation/devicetree/bindings/eeprom/at24.yaml b/Documentation/devicetree/bindings/eeprom/at24.yaml
index 699c2bbc16f5..4d46b8c5439d 100644
--- a/Documentation/devicetree/bindings/eeprom/at24.yaml
+++ b/Documentation/devicetree/bindings/eeprom/at24.yaml
@@ -18,7 +18,9 @@ select:
   properties:
     compatible:
       contains:
-        pattern: "^atmel,(24(c|cs|mac)[0-9]+|spd)$"
+        anyOf:
+          - pattern: "^atmel,(24(c|cs|mac)[0-9]+|spd)$"
+          - enum: ["microchip,24aa025e48", "microchip,24aa025e64"]

Thanks,
Conor.

>        - pattern: '^atmel,24c(32|64)d-wl$' # Actual vendor is st
>  
>    label:
> -- 
> 2.34.1
>
Andrei Simion July 1, 2024, 2:37 p.m. UTC | #2
On 28.06.2024 12:09, Conor Dooley wrote:
> Hey,
> 
> On Fri, Jun 28, 2024 at 11:01:46AM +0300, Andrei Simion wrote:
>> Add compatible for Microchip 24AA025E48/24AA025E64 EEPROMs.
>>
>> Reviewed-by: Connor Dooley <conor.dooley@microchip.com>
>                  ^^             ^
> There's no way that I provided a tag with my name spelt incorrectly
> given I use a macro to insert them. Please copy-paste tags or use b4
> to pick them up, rather than type them out yourself.
> 
>> Signed-off-by: Andrei Simion <andrei.simion@microchip.com>
>> ---
>> v2 -> v3:
>> - commit subject changed to reference Microchip 24AA025E48/24AA025E64
>> - drop the pattern: mac02e4$ and mac02e6$ and a-z from regex
>> - add these two devices down at the bottom
>> - added Reviewed-by
>>
>> v1 -> v2:
>> - change pattern into "^atmel,(24(c|cs|mac)[a-z0-9]+|spd)$" to keep simpler
>> ---
>>  Documentation/devicetree/bindings/eeprom/at24.yaml | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/eeprom/at24.yaml b/Documentation/devicetree/bindings/eeprom/at24.yaml
>> index 3c36cd0510de..699c2bbc16f5 100644
>> --- a/Documentation/devicetree/bindings/eeprom/at24.yaml
>> +++ b/Documentation/devicetree/bindings/eeprom/at24.yaml
>> @@ -132,6 +132,10 @@ properties:
>>                - renesas,r1ex24128
>>                - samsung,s524ad0xd1
>>            - const: atmel,24c128
>> +      - items:
>> +          - const: microchip,24aa025e48
>> +      - items:
>> +          - const: microchip,24aa025e64
> 
> I don't think this patch works, the schema has a select in it that only
> matches ^atmel,(24(c|cs|mac)[0-9]+|spd)$. You either need to have these
> fall back to an existing compatible (iff actually compatible) or else do
> something like:
> diff --git a/Documentation/devicetree/bindings/eeprom/at24.yaml b/Documentation/devicetree/bindings/eeprom/at24.yaml
> index 699c2bbc16f5..4d46b8c5439d 100644
> --- a/Documentation/devicetree/bindings/eeprom/at24.yaml
> +++ b/Documentation/devicetree/bindings/eeprom/at24.yaml
> @@ -18,7 +18,9 @@ select:
>    properties:
>      compatible:
>        contains:
> -        pattern: "^atmel,(24(c|cs|mac)[0-9]+|spd)$"
> +        anyOf:
> +          - pattern: "^atmel,(24(c|cs|mac)[0-9]+|spd)$"
> +          - enum: ["microchip,24aa025e48", "microchip,24aa025e64"]
> 
> Thanks,
> Conor.
> 

If I use your solution (and drop both items: const) -> it is selected the compatible but does not match anything.

What do you think about :

diff --git a/Documentation/devicetree/bindings/eeprom/at24.yaml b/Documentation/devicetree/bindings/eeprom/at24.yaml
index 699c2bbc16f5..bfaf7eac658a 100644
--- a/Documentation/devicetree/bindings/eeprom/at24.yaml
+++ b/Documentation/devicetree/bindings/eeprom/at24.yaml
@@ -18,7 +18,7 @@ select:
   properties:
     compatible:
       contains:
-        pattern: "^atmel,(24(c|cs|mac)[0-9]+|spd)$"
+        pattern: "^(atmel|microchip),(24(aa|c|cs|mac)[0-9]+|spd)$"
   required:
     - compatible
 
@@ -37,9 +37,13 @@ properties:
       - allOf:
           - minItems: 1
             items:
-              - pattern: "^(atmel|catalyst|microchip|nxp|ramtron|renesas|rohm|st),(24(c|cs|lc|mac)[0-9]+|spd)$"
+              - pattern: "^(atmel|catalyst|microchip|nxp|ramtron|renesas|rohm|st),(24(aa|c|cs|lc|mac)[0-9]+|spd)$"
               - pattern: "^atmel,(24(c|cs|mac)[0-9]+|spd)$"
           - oneOf:
+              - items:
+                  pattern: aa025e48$
+              - items:
+                  pattern: aa025e64$
               - items:
                   pattern: c00$
               - items:

Best Regards,
Andrei 


>>        - pattern: '^atmel,24c(32|64)d-wl$' # Actual vendor is st
>>  
>>    label:
>> -- 
>> 2.34.1
>>
Conor Dooley July 1, 2024, 2:47 p.m. UTC | #3
On Mon, Jul 01, 2024 at 02:37:57PM +0000, Andrei.Simion@microchip.com wrote:
> On 28.06.2024 12:09, Conor Dooley wrote:
> > Hey,
> > 
> > On Fri, Jun 28, 2024 at 11:01:46AM +0300, Andrei Simion wrote:
> >> Add compatible for Microchip 24AA025E48/24AA025E64 EEPROMs.
> >>
> >> Reviewed-by: Connor Dooley <conor.dooley@microchip.com>
> >                  ^^             ^
> > There's no way that I provided a tag with my name spelt incorrectly
> > given I use a macro to insert them. Please copy-paste tags or use b4
> > to pick them up, rather than type them out yourself.
> > 
> >> Signed-off-by: Andrei Simion <andrei.simion@microchip.com>
> >> ---
> >> v2 -> v3:
> >> - commit subject changed to reference Microchip 24AA025E48/24AA025E64
> >> - drop the pattern: mac02e4$ and mac02e6$ and a-z from regex
> >> - add these two devices down at the bottom
> >> - added Reviewed-by
> >>
> >> v1 -> v2:
> >> - change pattern into "^atmel,(24(c|cs|mac)[a-z0-9]+|spd)$" to keep simpler
> >> ---
> >>  Documentation/devicetree/bindings/eeprom/at24.yaml | 4 ++++
> >>  1 file changed, 4 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/eeprom/at24.yaml b/Documentation/devicetree/bindings/eeprom/at24.yaml
> >> index 3c36cd0510de..699c2bbc16f5 100644
> >> --- a/Documentation/devicetree/bindings/eeprom/at24.yaml
> >> +++ b/Documentation/devicetree/bindings/eeprom/at24.yaml
> >> @@ -132,6 +132,10 @@ properties:
> >>                - renesas,r1ex24128
> >>                - samsung,s524ad0xd1
> >>            - const: atmel,24c128
> >> +      - items:
> >> +          - const: microchip,24aa025e48
> >> +      - items:
> >> +          - const: microchip,24aa025e64
> > 
> > I don't think this patch works, the schema has a select in it that only
> > matches ^atmel,(24(c|cs|mac)[0-9]+|spd)$. You either need to have these
> > fall back to an existing compatible (iff actually compatible) or else do
> > something like:
> > diff --git a/Documentation/devicetree/bindings/eeprom/at24.yaml b/Documentation/devicetree/bindings/eeprom/at24.yaml
> > index 699c2bbc16f5..4d46b8c5439d 100644
> > --- a/Documentation/devicetree/bindings/eeprom/at24.yaml
> > +++ b/Documentation/devicetree/bindings/eeprom/at24.yaml
> > @@ -18,7 +18,9 @@ select:
> >    properties:
> >      compatible:
> >        contains:
> > -        pattern: "^atmel,(24(c|cs|mac)[0-9]+|spd)$"
> > +        anyOf:
> > +          - pattern: "^atmel,(24(c|cs|mac)[0-9]+|spd)$"
> > +          - enum: ["microchip,24aa025e48", "microchip,24aa025e64"]
> 
> If I use your solution (and drop both items: const) -> it is selected the compatible but does not match anything.

My intention was that you added my diff to your existing patch.

Thanks,
Conor.
Andrei Simion July 1, 2024, 2:56 p.m. UTC | #4
On 01.07.2024 17:47, Conor Dooley wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> 
> ForwardedMessage.eml
> 
> Subject:
> Re: [PATCH v3 3/3] dt-bindings: eeprom: at24: Add Microchip 24AA025E48/24AA025E64
> From:
> Conor Dooley <conor@kernel.org>
> Date:
> 01.07.2024, 17:47
> 
> To:
> Andrei.Simion@microchip.com
> CC:
> Conor.Dooley@microchip.com, brgl@bgdev.pl, robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org, Nicolas.Ferre@microchip.com, alexandre.belloni@bootlin.com, claudiu.beznea@tuxon.dev, arnd@arndb.de, gregkh@linuxfoundation.org, linux-i2c@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org
> 
> 
> 
> On Mon, Jul 01, 2024 at 02:37:57PM +0000, Andrei.Simion@microchip.com wrote:
>> On 28.06.2024 12:09, Conor Dooley wrote:
>>> Hey,
>>>
>>> On Fri, Jun 28, 2024 at 11:01:46AM +0300, Andrei Simion wrote:
>>>> Add compatible for Microchip 24AA025E48/24AA025E64 EEPROMs.
>>>>
>>>> Reviewed-by: Connor Dooley <conor.dooley@microchip.com>
>>>                  ^^             ^
>>> There's no way that I provided a tag with my name spelt incorrectly
>>> given I use a macro to insert them. Please copy-paste tags or use b4
>>> to pick them up, rather than type them out yourself.
>>>
>>>> Signed-off-by: Andrei Simion <andrei.simion@microchip.com>
>>>> ---
>>>> v2 -> v3:
>>>> - commit subject changed to reference Microchip 24AA025E48/24AA025E64
>>>> - drop the pattern: mac02e4$ and mac02e6$ and a-z from regex
>>>> - add these two devices down at the bottom
>>>> - added Reviewed-by
>>>>
>>>> v1 -> v2:
>>>> - change pattern into "^atmel,(24(c|cs|mac)[a-z0-9]+|spd)$" to keep simpler
>>>> ---
>>>>  Documentation/devicetree/bindings/eeprom/at24.yaml | 4 ++++
>>>>  1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/eeprom/at24.yaml b/Documentation/devicetree/bindings/eeprom/at24.yaml
>>>> index 3c36cd0510de..699c2bbc16f5 100644
>>>> --- a/Documentation/devicetree/bindings/eeprom/at24.yaml
>>>> +++ b/Documentation/devicetree/bindings/eeprom/at24.yaml
>>>> @@ -132,6 +132,10 @@ properties:
>>>>                - renesas,r1ex24128
>>>>                - samsung,s524ad0xd1
>>>>            - const: atmel,24c128
>>>> +      - items:
>>>> +          - const: microchip,24aa025e48
>>>> +      - items:
>>>> +          - const: microchip,24aa025e64
>>> I don't think this patch works, the schema has a select in it that only
>>> matches ^atmel,(24(c|cs|mac)[0-9]+|spd)$. You either need to have these
>>> fall back to an existing compatible (iff actually compatible) or else do
>>> something like:
>>> diff --git a/Documentation/devicetree/bindings/eeprom/at24.yaml b/Documentation/devicetree/bindings/eeprom/at24.yaml
>>> index 699c2bbc16f5..4d46b8c5439d 100644
>>> --- a/Documentation/devicetree/bindings/eeprom/at24.yaml
>>> +++ b/Documentation/devicetree/bindings/eeprom/at24.yaml
>>> @@ -18,7 +18,9 @@ select:
>>>    properties:
>>>      compatible:
>>>        contains:
>>> -        pattern: "^atmel,(24(c|cs|mac)[0-9]+|spd)$"
>>> +        anyOf:
>>> +          - pattern: "^atmel,(24(c|cs|mac)[0-9]+|spd)$"
>>> +          - enum: ["microchip,24aa025e48", "microchip,24aa025e64"]
>> If I use your solution (and drop both items: const) -> it is selected the compatible but does not match anything.
> My intention was that you added my diff to your existing patch.
>

I also tested this scenario, it is OK. 
Thank you for your clarifications.

Andrei

> Thanks,
> Conor.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/eeprom/at24.yaml b/Documentation/devicetree/bindings/eeprom/at24.yaml
index 3c36cd0510de..699c2bbc16f5 100644
--- a/Documentation/devicetree/bindings/eeprom/at24.yaml
+++ b/Documentation/devicetree/bindings/eeprom/at24.yaml
@@ -132,6 +132,10 @@  properties:
               - renesas,r1ex24128
               - samsung,s524ad0xd1
           - const: atmel,24c128
+      - items:
+          - const: microchip,24aa025e48
+      - items:
+          - const: microchip,24aa025e64
       - pattern: '^atmel,24c(32|64)d-wl$' # Actual vendor is st
 
   label: