| Message ID | 20240621121340.114486-4-andrei.simion@microchip.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Read MAC address through NVMEM for sama7g5ek | expand |
On Fri, Jun 21, 2024 at 03:13:40PM +0300, Andrei Simion wrote: > Update regex check and add pattern to match both EEPROMs. > > Signed-off-by: Andrei Simion <andrei.simion@microchip.com> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
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
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? > Rob Best Regards, Andrei Simion
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 --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:
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(-)