diff mbox series

[3/3] dt-bindings: eeprom: at24: Add at24,mac02e4 and at24,mac02e6

Message ID 20240619072231.6876-4-andrei.simion@microchip.com
State Superseded
Headers show
Series [1/3] eeprom: at24: avoid adjusting offset for 24AA025E{48, 64} | expand

Commit Message

Andrei Simion June 19, 2024, 7:22 a.m. UTC
Update regex check and add pattern to match both EEPROMs.

Signed-off-by: Andrei Simion <andrei.simion@microchip.com>
---
 Documentation/devicetree/bindings/eeprom/at24.yaml | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

Andrei Simion June 20, 2024, 10:45 a.m. UTC | #1
On 19.06.2024 20:53, Conor Dooley wrote:
>> Update regex check and add pattern to match both EEPROMs.
>>
>> Signed-off-by: Andrei Simion <andrei.simion@microchip.com>
>> ---
>>  Documentation/devicetree/bindings/eeprom/at24.yaml | 10 +++++++---
>>  1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/eeprom/at24.yaml b/Documentation/devicetree/bindings/eeprom/at24.yaml
>> index 3c36cd0510de..46daa662f6e7 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,(24(c|cs|mac)[0-9]+[a-z0-9]*|spd)$"

> Could we relax the pattern instead to make this bloat less? Would it be
> problematic to just allow "^atmel,(24(c|cs|mac)[a-z0-9]+|spd)$"?

I) "^atmel,(24(c|cs|mac)[a-z0-9]+|spd)$" :
The first pattern does not specify where the digits must occur within the alphanumeric sequence that follows 24c, 24cs, or 24mac. It allows the sequence to be all letters, all digits, or any mix thereof.

II) "^atmel,(24(c|cs|mac)[0-9]+[a-z0-9]*|spd)$" :
The second pattern specifically requires that at least one digit appears immediately after 24c, 24cs, or 24mac, and only after this digit can letters appear.

As hypothetical example :
atmel,24cabc would match the first pattern but not the second because there are no digits immediately following 24c.
atmel,24c123 would match both patterns because there are digits immediately following 24c, and the first pattern doesn't care about the position of the digits within the alphanumeric sequence.

In case of at24,mac02e4 and at24,mac02e6 match both patterns.

Let me know your thoughts.

I agree to change the pattern as you suggest.

BR,
Andrei
Conor Dooley June 20, 2024, 11:10 a.m. UTC | #2
On Thu, Jun 20, 2024 at 10:45:58AM +0000, Andrei.Simion@microchip.com wrote:
> On 19.06.2024 20:53, Conor Dooley wrote:
> >> Update regex check and add pattern to match both EEPROMs.
> >>
> >> Signed-off-by: Andrei Simion <andrei.simion@microchip.com>
> >> ---
> >>  Documentation/devicetree/bindings/eeprom/at24.yaml | 10 +++++++---
> >>  1 file changed, 7 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/eeprom/at24.yaml b/Documentation/devicetree/bindings/eeprom/at24.yaml
> >> index 3c36cd0510de..46daa662f6e7 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,(24(c|cs|mac)[0-9]+[a-z0-9]*|spd)$"
> 
> > Could we relax the pattern instead to make this bloat less? Would it be
> > problematic to just allow "^atmel,(24(c|cs|mac)[a-z0-9]+|spd)$"?
> 
> I) "^atmel,(24(c|cs|mac)[a-z0-9]+|spd)$" :
> The first pattern does not specify where the digits must occur within
> the alphanumeric sequence that follows 24c, 24cs, or 24mac. It allows
> the sequence to be all letters, all digits, or any mix thereof.
> 
> II) "^atmel,(24(c|cs|mac)[0-9]+[a-z0-9]*|spd)$" :
> The second pattern specifically requires that at least one digit appears
> immediately after 24c, 24cs, or 24mac, and only after this digit can
> letters appear.

> As hypothetical example :
> atmel,24cabc would match the first pattern but not the second because
> there are no digits immediately following 24c.
> atmel,24c123 would match both patterns because there are digits
> immediately following 24c, and the first pattern doesn't care about
> the position of the digits within the alphanumeric sequence.
> 
> In case of at24,mac02e4 and at24,mac02e6 match both patterns.
> 
> Let me know your thoughts.

Basically my reasoning here is that both patterns are very permissive
(although one clearly more than the other) and do not stop people from
creating compatibles that do not correspond to a real device, so I felt
that the more complex regex didn't really provide enough benefit
compared to keeping the regex simpler.

> I agree to change the pattern as you suggest.

:+1:
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/eeprom/at24.yaml b/Documentation/devicetree/bindings/eeprom/at24.yaml
index 3c36cd0510de..46daa662f6e7 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,(24(c|cs|mac)[0-9]+[a-z0-9]*|spd)$"
   required:
     - compatible
 
@@ -37,8 +37,8 @@  properties:
       - allOf:
           - minItems: 1
             items:
-              - pattern: "^(atmel|catalyst|microchip|nxp|ramtron|renesas|rohm|st),(24(c|cs|lc|mac)[0-9]+|spd)$"
-              - pattern: "^atmel,(24(c|cs|mac)[0-9]+|spd)$"
+              - pattern: "^(atmel|catalyst|microchip|nxp|ramtron|renesas|rohm|st),(24(c|cs|lc|mac)[0-9]+[a-z0-9]*|spd)$"
+              - pattern: "^atmel,(24(c|cs|mac)[0-9]+[a-z0-9]*|spd)$"
           - oneOf:
               - items:
                   pattern: c00$
@@ -54,6 +54,10 @@  properties:
                   pattern: mac402$
               - items:
                   pattern: mac602$
+              - items:
+                  pattern: mac02e4$
+              - items:
+                  pattern: mac02e6$
               - items:
                   pattern: c04$
               - items: