diff mbox series

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

Message ID 20240621121340.114486-4-andrei.simion@microchip.com
State New
Headers show
Series Read MAC address through NVMEM for sama7g5ek | expand

Commit Message

Andrei Simion June 21, 2024, 12:13 p.m. UTC
Update regex check and add pattern to match both EEPROMs.

Signed-off-by: Andrei Simion <andrei.simion@microchip.com>
---
v1 -> v2:
- change patter into "^atmel,(24(c|cs|mac)[a-z0-9]+|spd)$" to keep simpler
---
 Documentation/devicetree/bindings/eeprom/at24.yaml | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

Rob Herring June 24, 2024, 7:49 p.m. UTC | #1
On Fri, Jun 21, 2024 at 03:13:40PM +0300, Andrei Simion wrote:
> Update regex check and add pattern to match both EEPROMs.

The subject is wrong as 'at24' is not the vendor.

> 
> Signed-off-by: Andrei Simion <andrei.simion@microchip.com>
> ---
> v1 -> v2:
> - change patter into "^atmel,(24(c|cs|mac)[a-z0-9]+|spd)$" to keep simpler
> ---
>  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..f914ca37ceea 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)[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)[a-z0-9]+|spd)$"
> +              - pattern: "^atmel,(24(c|cs|mac)[a-z0-9]+|spd)$"

Are these devices available from multiple vendors? If not, I think I'd 
add specific compatible strings with the right vendor rather than adding 
to this pattern. It's rather loosely defined because that's what was in 
use already.

Rob
Conor Dooley June 25, 2024, 4:15 p.m. UTC | #2
On Tue, Jun 25, 2024 at 07:33:18AM +0000, Andrei.Simion@microchip.com wrote:
> On 24.06.2024 22:49, Rob Herring wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> > 
> > On Fri, Jun 21, 2024 at 03:13:40PM +0300, Andrei Simion wrote:
> >> Update regex check and add pattern to match both EEPROMs.
> > 
> > The subject is wrong as 'at24' is not the vendor.
> >
> 
> My mistake. It needs to be atmel,24mac02e4 and atmel,24mac02e6.
> 
> >>
> >> Signed-off-by: Andrei Simion <andrei.simion@microchip.com>
> >> ---
> >> v1 -> v2:
> >> - change patter into "^atmel,(24(c|cs|mac)[a-z0-9]+|spd)$" to keep simpler
> >> ---
> >>  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..f914ca37ceea 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)[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)[a-z0-9]+|spd)$"
> >> +              - pattern: "^atmel,(24(c|cs|mac)[a-z0-9]+|spd)$"
> > 
> > Are these devices available from multiple vendors? If not, I think I'd
> > add specific compatible strings with the right vendor rather than adding
> > to this pattern. It's rather loosely defined because that's what was in
> > use already.
> >
> 
> So, would you like me to keep how it was before:  "^atmel,(24(c|cs|mac)[0-9]+|spd)$" and  "^(atmel|catalyst|microchip|nxp|ramtron|renesas|rohm|st),(24(c|cs|lc|mac)[0-9]+|spd)$"
> and to add only: 
> - items:
>  pattern: mac02e4$
> - items:
>  pattern: mac02e6$
> ?
> 
> Or would you like me to add to "the special cases that don't conform to the above pattern. Each requires a standard at24 model as fallback."  area?

I think the suggestion is to explicitly add these two devices down at the
bottom instead of adding to the regex. The first hunk I think in this
patch needs to remain a regex.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/eeprom/at24.yaml b/Documentation/devicetree/bindings/eeprom/at24.yaml
index 3c36cd0510de..f914ca37ceea 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)[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)[a-z0-9]+|spd)$"
+              - pattern: "^atmel,(24(c|cs|mac)[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: