diff mbox series

[1/2] dt-bindings: arm: bcm: unify version notation of Northstar devices

Message ID 20230520112601.11821-1-zajec5@gmail.com (mailing list archive)
State New, archived
Headers show
Series [1/2] dt-bindings: arm: bcm: unify version notation of Northstar devices | expand

Commit Message

Rafał Miłecki May 20, 2023, 11:26 a.m. UTC 
From: Rafał Miłecki <rafal@milecki.pl>

Always use a minus/hyphen char to separate model from version. This
unifies binding's "compatible" strings.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 .../devicetree/bindings/arm/bcm/brcm,bcm4708.yaml    | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Linus Walleij May 20, 2023, 6:30 p.m. UTC | #1 
On Sat, May 20, 2023 at 1:26 PM Rafał Miłecki <zajec5@gmail.com> wrote:

> From: Rafał Miłecki <rafal@milecki.pl>
>
> Always use a minus/hyphen char to separate model from version. This
> unifies binding's "compatible" strings.
>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>

Makes sense,
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
Conor Dooley May 22, 2023, 5:08 p.m. UTC | #2 
On Sat, May 20, 2023 at 01:26:00PM +0200, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> Always use a minus/hyphen char to separate model from version. This
> unifies binding's "compatible" strings.

Am I just being paranoid in thinking that software may have relied on
the former naming scheme?
On the other hand, my OCD really likes the change.

> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>  .../devicetree/bindings/arm/bcm/brcm,bcm4708.yaml    | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/bcm/brcm,bcm4708.yaml b/Documentation/devicetree/bindings/arm/bcm/brcm,bcm4708.yaml
> index 454b0e93245d..cc34025fdc78 100644
> --- a/Documentation/devicetree/bindings/arm/bcm/brcm,bcm4708.yaml
> +++ b/Documentation/devicetree/bindings/arm/bcm/brcm,bcm4708.yaml
> @@ -28,10 +28,10 @@ properties:
>                - buffalo,wzr-1750dhp
>                - linksys,ea6300-v1
>                - linksys,ea6500-v2
> -              - luxul,xap-1510v1
> +              - luxul,xap-1510-v1
>                - luxul,xwc-1000
> -              - netgear,r6250v1
> -              - netgear,r6300v2
> +              - netgear,r6250-v1
> +              - netgear,r6300-v2
>                - smartrg,sr400ac
>                - brcm,bcm94708
>            - const: brcm,bcm4708
> @@ -42,8 +42,8 @@ properties:
>                - asus,rt-n18u
>                - buffalo,wzr-600dhp2
>                - buffalo,wzr-900dhp
> -              - luxul,xap-1410v1
> -              - luxul,xwr-1200v1
> +              - luxul,xap-1410-v1
> +              - luxul,xwr-1200-v1
>                - tplink,archer-c5-v2
>            - const: brcm,bcm47081
>            - const: brcm,bcm4708
> @@ -72,7 +72,7 @@ properties:
>                - luxul,xap-1610-v1
>                - luxul,xbr-4500-v1
>                - luxul,xwc-2000-v1
> -              - luxul,xwr-3100v1
> +              - luxul,xwr-3100-v1
>                - luxul,xwr-3150-v1
>                - netgear,r8500
>                - phicomm,k3
> -- 
> 2.35.3
>
Rafał Miłecki May 22, 2023, 8:29 p.m. UTC | #3 
On 22.05.2023 19:08, Conor Dooley wrote:
> On Sat, May 20, 2023 at 01:26:00PM +0200, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> Always use a minus/hyphen char to separate model from version. This
>> unifies binding's "compatible" strings.
> 
> Am I just being paranoid in thinking that software may have relied on
> the former naming scheme?
> On the other hand, my OCD really likes the change.

That's a very reasonable concern.


TLDR: The risk of any breakage is extremely low due to Northstar CFE
bootloader & projects with Northstar support.


There are very few Northstar devices with bootloader other than CFE.
All devices affected by this PATCH use CFE actually.
CFE on Northstar has no support for DTS (DTB).
DTB files are always appended to kernel on all affected devices.

So problem of some DTB stored in bootloader getting out of sync with
kernel / user-space is non-existent in this case.

We still should consider a risk of some out-of-tree driver or just
user-space checking for those compatible strings. I'm not aware of any
project other than OpenWrt providing system images for those devices.
There is some basic support in buildroot but it's quite dead. Even in
OpenWrt case the only possibly affected device is Netgear R6300 V2.
OpenWrt doesn't provide images for any of affected Luxul devices.

So there isn't any known project this change can actually break. If
there is one (very unlikely) it can still update its user-space or
out of kernel driver while updating DTB.

So while this change may be not the best approach (in general terms)
in this case it's very unlikely to break anything.


>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> ---
>>   .../devicetree/bindings/arm/bcm/brcm,bcm4708.yaml    | 12 ++++++------
>>   1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/bcm/brcm,bcm4708.yaml b/Documentation/devicetree/bindings/arm/bcm/brcm,bcm4708.yaml
>> index 454b0e93245d..cc34025fdc78 100644
>> --- a/Documentation/devicetree/bindings/arm/bcm/brcm,bcm4708.yaml
>> +++ b/Documentation/devicetree/bindings/arm/bcm/brcm,bcm4708.yaml
>> @@ -28,10 +28,10 @@ properties:
>>                 - buffalo,wzr-1750dhp
>>                 - linksys,ea6300-v1
>>                 - linksys,ea6500-v2
>> -              - luxul,xap-1510v1
>> +              - luxul,xap-1510-v1
>>                 - luxul,xwc-1000
>> -              - netgear,r6250v1
>> -              - netgear,r6300v2
>> +              - netgear,r6250-v1
>> +              - netgear,r6300-v2
>>                 - smartrg,sr400ac
>>                 - brcm,bcm94708
>>             - const: brcm,bcm4708
>> @@ -42,8 +42,8 @@ properties:
>>                 - asus,rt-n18u
>>                 - buffalo,wzr-600dhp2
>>                 - buffalo,wzr-900dhp
>> -              - luxul,xap-1410v1
>> -              - luxul,xwr-1200v1
>> +              - luxul,xap-1410-v1
>> +              - luxul,xwr-1200-v1
>>                 - tplink,archer-c5-v2
>>             - const: brcm,bcm47081
>>             - const: brcm,bcm4708
>> @@ -72,7 +72,7 @@ properties:
>>                 - luxul,xap-1610-v1
>>                 - luxul,xbr-4500-v1
>>                 - luxul,xwc-2000-v1
>> -              - luxul,xwr-3100v1
>> +              - luxul,xwr-3100-v1
>>                 - luxul,xwr-3150-v1
>>                 - netgear,r8500
>>                 - phicomm,k3
>> -- 
>> 2.35.3
>>
Conor Dooley May 23, 2023, 4:07 p.m. UTC | #4 
On Mon, May 22, 2023 at 10:29:38PM +0200, Rafał Miłecki wrote:
> On 22.05.2023 19:08, Conor Dooley wrote:
> > On Sat, May 20, 2023 at 01:26:00PM +0200, Rafał Miłecki wrote:
> > > From: Rafał Miłecki <rafal@milecki.pl>
> > > 
> > > Always use a minus/hyphen char to separate model from version. This
> > > unifies binding's "compatible" strings.
> > 
> > Am I just being paranoid in thinking that software may have relied on
> > the former naming scheme?
> > On the other hand, my OCD really likes the change.
> 
> That's a very reasonable concern.
> 
> 
> TLDR: The risk of any breakage is extremely low due to Northstar CFE
> bootloader & projects with Northstar support.
> 
> 
> There are very few Northstar devices with bootloader other than CFE.
> All devices affected by this PATCH use CFE actually.
> CFE on Northstar has no support for DTS (DTB).
> DTB files are always appended to kernel on all affected devices.
> 
> So problem of some DTB stored in bootloader getting out of sync with
> kernel / user-space is non-existent in this case.
> 
> We still should consider a risk of some out-of-tree driver or just
> user-space checking for those compatible strings. I'm not aware of any
> project other than OpenWrt providing system images for those devices.
> There is some basic support in buildroot but it's quite dead. Even in
> OpenWrt case the only possibly affected device is Netgear R6300 V2.
> OpenWrt doesn't provide images for any of affected Luxul devices.
> 
> So there isn't any known project this change can actually break. If
> there is one (very unlikely) it can still update its user-space or
> out of kernel driver while updating DTB.
> 
> So while this change may be not the best approach (in general terms)
> in this case it's very unlikely to break anything.

Okay. I think this sounds reasonable to do then. Thanks for the
explanation :)
Acked-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.
Florian Fainelli May 25, 2023, 5:40 p.m. UTC | #5 
From: Florian Fainelli <f.fainelli@gmail.com>

On Sat, 20 May 2023 13:26:00 +0200, Rafał Miłecki <zajec5@gmail.com> wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> Always use a minus/hyphen char to separate model from version. This
> unifies binding's "compatible" strings.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---

Applied to https://github.com/Broadcom/stblinux/commits/devicetree/next, thanks!
--
Florian
Krzysztof Kozlowski June 1, 2023, 7:07 a.m. UTC | #6 
On 22/05/2023 22:29, Rafał Miłecki wrote:
> On 22.05.2023 19:08, Conor Dooley wrote:
>> On Sat, May 20, 2023 at 01:26:00PM +0200, Rafał Miłecki wrote:
>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>
>>> Always use a minus/hyphen char to separate model from version. This
>>> unifies binding's "compatible" strings.
>>
>> Am I just being paranoid in thinking that software may have relied on
>> the former naming scheme?
>> On the other hand, my OCD really likes the change.
> 
> That's a very reasonable concern.
> 
> 
> TLDR: The risk of any breakage is extremely low due to Northstar CFE
> bootloader & projects with Northstar support.
> 
> 
> There are very few Northstar devices with bootloader other than CFE.
> All devices affected by this PATCH use CFE actually.
> CFE on Northstar has no support for DTS (DTB).
> DTB files are always appended to kernel on all affected devices.
> 
> So problem of some DTB stored in bootloader getting out of sync with
> kernel / user-space is non-existent in this case.
> 
> We still should consider a risk of some out-of-tree driver or just
> user-space checking for those compatible strings. I'm not aware of any
> project other than OpenWrt providing system images for those devices.
> There is some basic support in buildroot but it's quite dead. Even in
> OpenWrt case the only possibly affected device is Netgear R6300 V2.
> OpenWrt doesn't provide images for any of affected Luxul devices.
> 
> So there isn't any known project this change can actually break. If
> there is one (very unlikely) it can still update its user-space or
> out of kernel driver while updating DTB.
> 
> So while this change may be not the best approach (in general terms)
> in this case it's very unlikely to break anything.

You should explain this - reason for ABI break - in commit msg.

Or just keep old compatibles as deprecated.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/arm/bcm/brcm,bcm4708.yaml b/Documentation/devicetree/bindings/arm/bcm/brcm,bcm4708.yaml
index 454b0e93245d..cc34025fdc78 100644
--- a/Documentation/devicetree/bindings/arm/bcm/brcm,bcm4708.yaml
+++ b/Documentation/devicetree/bindings/arm/bcm/brcm,bcm4708.yaml
@@ -28,10 +28,10 @@  properties:
               - buffalo,wzr-1750dhp
               - linksys,ea6300-v1
               - linksys,ea6500-v2
-              - luxul,xap-1510v1
+              - luxul,xap-1510-v1
               - luxul,xwc-1000
-              - netgear,r6250v1
-              - netgear,r6300v2
+              - netgear,r6250-v1
+              - netgear,r6300-v2
               - smartrg,sr400ac
               - brcm,bcm94708
           - const: brcm,bcm4708
@@ -42,8 +42,8 @@  properties:
               - asus,rt-n18u
               - buffalo,wzr-600dhp2
               - buffalo,wzr-900dhp
-              - luxul,xap-1410v1
-              - luxul,xwr-1200v1
+              - luxul,xap-1410-v1
+              - luxul,xwr-1200-v1
               - tplink,archer-c5-v2
           - const: brcm,bcm47081
           - const: brcm,bcm4708
@@ -72,7 +72,7 @@  properties:
               - luxul,xap-1610-v1
               - luxul,xbr-4500-v1
               - luxul,xwc-2000-v1
-              - luxul,xwr-3100v1
+              - luxul,xwr-3100-v1
               - luxul,xwr-3150-v1
               - netgear,r8500
               - phicomm,k3