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

[<Andrei.Simion@microchip.com>]

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
>>