| Message ID | 20240905180928.382090-5-rosenp@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | wifi: ath9k: remove platform_data support | expand |
On 05/09/2024 20:09, Rosen Penev wrote: > These platform_data options are now available for OF. This explains nothing. Describe the hardware, not driver. > > Signed-off-by: Rosen Penev <rosenp@gmail.com> <form letter> Please use scripts/get_maintainers.pl to get a list of necessary people and lists to CC. It might happen, that command when run on an older kernel, gives you outdated entries. Therefore please be sure you base your patches on recent Linux kernel. Tools like b4 or scripts/get_maintainer.pl provide you proper list of people, so fix your workflow. Tools might also fail if you work on some ancient tree (don't, instead use mainline) or work on fork of kernel (don't, instead use mainline). Just use b4 and everything should be fine, although remember about `b4 prep --auto-to-cc` if you added new patches to the patchset. You missed at least devicetree list (maybe more), so this won't be tested by automated tooling. Performing review on untested code might be a waste of time. Please kindly resend and include all necessary To/Cc entries. </form letter> > --- > .../devicetree/bindings/net/wireless/qca,ath9k.yaml | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml > index 0e5412cff2bc..5c293d558a94 100644 > --- a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml > +++ b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml > @@ -44,6 +44,16 @@ properties: > > ieee80211-freq-limit: true > > + qca,led-active-high: > + $ref: /schemas/types.yaml#/definitions/flag > + description: > + Indicates that the LED pin is active high instead of low Where is DTS user of it? Best regards, Krzysztof
On 05/09/2024 20:09, Rosen Penev wrote: > These platform_data options are now available for OF. > > Signed-off-by: Rosen Penev <rosenp@gmail.com> > --- > .../devicetree/bindings/net/wireless/qca,ath9k.yaml | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml > index 0e5412cff2bc..5c293d558a94 100644 > --- a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml > +++ b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml > @@ -44,6 +44,16 @@ properties: > > ieee80211-freq-limit: true > > + qca,led-active-high: > + $ref: /schemas/types.yaml#/definitions/flag > + description: > + Indicates that the LED pin is active high instead of low > + > + qca,led-pin: So this all looks like re-inventing standard GPIOs, at least for devices using GPIOLIB. Your commit msg needs proper rationale. Best regards, Krzysztof
On Thu, Sep 5, 2024 at 2:29 PM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > On 05/09/2024 20:09, Rosen Penev wrote: > > These platform_data options are now available for OF. > > This explains nothing. Describe the hardware, not driver. > > > > > Signed-off-by: Rosen Penev <rosenp@gmail.com> > > <form letter> > Please use scripts/get_maintainers.pl to get a list of necessary people > and lists to CC. It might happen, that command when run on an older > kernel, gives you outdated entries. Therefore please be sure you base > your patches on recent Linux kernel. > > Tools like b4 or scripts/get_maintainer.pl provide you proper list of > people, so fix your workflow. Tools might also fail if you work on some > ancient tree (don't, instead use mainline) or work on fork of kernel > (don't, instead use mainline). Just use b4 and everything should be > fine, although remember about `b4 prep --auto-to-cc` if you added new > patches to the patchset. > > You missed at least devicetree list (maybe more), so this won't be > tested by automated tooling. Performing review on untested code might be > a waste of time. > > Please kindly resend and include all necessary To/Cc entries. Will do. > </form letter> > > > --- > > .../devicetree/bindings/net/wireless/qca,ath9k.yaml | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml > > index 0e5412cff2bc..5c293d558a94 100644 > > --- a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml > > +++ b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml > > @@ -44,6 +44,16 @@ properties: > > > > ieee80211-freq-limit: true > > > > + qca,led-active-high: > > + $ref: /schemas/types.yaml#/definitions/flag > > + description: > > + Indicates that the LED pin is active high instead of low > > Where is DTS user of it? None at this time. There are only downstream users. This maps to the led_active_high in ath9k_platform_device , which gets removed in a subsequent commit. drivers/net/wireless/mediatek/mt76/mac80211.c uses led-sources as the name for the same purpose. Maybe rename? > > Best regards, > Krzysztof >
On Thu, Sep 5, 2024 at 2:33 PM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > On 05/09/2024 20:09, Rosen Penev wrote: > > These platform_data options are now available for OF. > > > > Signed-off-by: Rosen Penev <rosenp@gmail.com> > > --- > > .../devicetree/bindings/net/wireless/qca,ath9k.yaml | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml > > index 0e5412cff2bc..5c293d558a94 100644 > > --- a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml > > +++ b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml > > @@ -44,6 +44,16 @@ properties: > > > > ieee80211-freq-limit: true > > > > + qca,led-active-high: > > + $ref: /schemas/types.yaml#/definitions/flag > > + description: > > + Indicates that the LED pin is active high instead of low > > + > > + qca,led-pin: > > So this all looks like re-inventing standard GPIOs, at least for devices > using GPIOLIB. Your commit msg needs proper rationale. git grep led_classdev_register drivers/net/wireless/ | wc -l 16 Both old and new drivers register their own GPIO devices to control LEDs. > > Best regards, > Krzysztof >
On 06/09/2024 00:35, Rosen Penev wrote: > On Thu, Sep 5, 2024 at 2:29 PM Krzysztof Kozlowski <krzk@kernel.org> wrote: >> >> On 05/09/2024 20:09, Rosen Penev wrote: >>> These platform_data options are now available for OF. >> >> This explains nothing. Describe the hardware, not driver. >> >>> >>> Signed-off-by: Rosen Penev <rosenp@gmail.com> >> >> <form letter> >> Please use scripts/get_maintainers.pl to get a list of necessary people >> and lists to CC. It might happen, that command when run on an older >> kernel, gives you outdated entries. Therefore please be sure you base >> your patches on recent Linux kernel. >> >> Tools like b4 or scripts/get_maintainer.pl provide you proper list of >> people, so fix your workflow. Tools might also fail if you work on some >> ancient tree (don't, instead use mainline) or work on fork of kernel >> (don't, instead use mainline). Just use b4 and everything should be >> fine, although remember about `b4 prep --auto-to-cc` if you added new >> patches to the patchset. >> >> You missed at least devicetree list (maybe more), so this won't be >> tested by automated tooling. Performing review on untested code might be >> a waste of time. >> >> Please kindly resend and include all necessary To/Cc entries. > Will do. >> </form letter> >> >>> --- >>> .../devicetree/bindings/net/wireless/qca,ath9k.yaml | 12 ++++++++++++ >>> 1 file changed, 12 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml >>> index 0e5412cff2bc..5c293d558a94 100644 >>> --- a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml >>> +++ b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml >>> @@ -44,6 +44,16 @@ properties: >>> >>> ieee80211-freq-limit: true >>> >>> + qca,led-active-high: >>> + $ref: /schemas/types.yaml#/definitions/flag >>> + description: >>> + Indicates that the LED pin is active high instead of low >> >> Where is DTS user of it? > None at this time. There are only downstream users. We do not add stuff for downstream. > > This maps to the led_active_high in ath9k_platform_device , which gets > removed in a subsequent commit. > > drivers/net/wireless/mediatek/mt76/mac80211.c uses led-sources as the > name for the same purpose. Maybe rename? Again, you add undocumented, unused compatibles along with unused properties. We don't care about downstream kernels. You need upstream users. Best regards, Krzysztof
On Fri, Sep 6, 2024 at 12:18 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > On 06/09/2024 00:35, Rosen Penev wrote: > > On Thu, Sep 5, 2024 at 2:29 PM Krzysztof Kozlowski <krzk@kernel.org> wrote: > >> > >> On 05/09/2024 20:09, Rosen Penev wrote: > >>> These platform_data options are now available for OF. > >> > >> This explains nothing. Describe the hardware, not driver. > >> > >>> > >>> Signed-off-by: Rosen Penev <rosenp@gmail.com> > >> > >> <form letter> > >> Please use scripts/get_maintainers.pl to get a list of necessary people > >> and lists to CC. It might happen, that command when run on an older > >> kernel, gives you outdated entries. Therefore please be sure you base > >> your patches on recent Linux kernel. > >> > >> Tools like b4 or scripts/get_maintainer.pl provide you proper list of > >> people, so fix your workflow. Tools might also fail if you work on some > >> ancient tree (don't, instead use mainline) or work on fork of kernel > >> (don't, instead use mainline). Just use b4 and everything should be > >> fine, although remember about `b4 prep --auto-to-cc` if you added new > >> patches to the patchset. > >> > >> You missed at least devicetree list (maybe more), so this won't be > >> tested by automated tooling. Performing review on untested code might be > >> a waste of time. > >> > >> Please kindly resend and include all necessary To/Cc entries. > > Will do. > >> </form letter> > >> > >>> --- > >>> .../devicetree/bindings/net/wireless/qca,ath9k.yaml | 12 ++++++++++++ > >>> 1 file changed, 12 insertions(+) > >>> > >>> diff --git a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml > >>> index 0e5412cff2bc..5c293d558a94 100644 > >>> --- a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml > >>> +++ b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml > >>> @@ -44,6 +44,16 @@ properties: > >>> > >>> ieee80211-freq-limit: true > >>> > >>> + qca,led-active-high: > >>> + $ref: /schemas/types.yaml#/definitions/flag > >>> + description: > >>> + Indicates that the LED pin is active high instead of low > >> > >> Where is DTS user of it? > > None at this time. There are only downstream users. > > We do not add stuff for downstream. > > > > > This maps to the led_active_high in ath9k_platform_device , which gets > > removed in a subsequent commit. > > > > drivers/net/wireless/mediatek/mt76/mac80211.c uses led-sources as the > > name for the same purpose. Maybe rename? > > Again, you add undocumented, unused compatibles along with unused > properties. We don't care about downstream kernels. You need upstream users. OK. Will remove in v2. > > Best regards, > Krzysztof >
diff --git a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml index 0e5412cff2bc..5c293d558a94 100644 --- a/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml +++ b/Documentation/devicetree/bindings/net/wireless/qca,ath9k.yaml @@ -44,6 +44,16 @@ properties: ieee80211-freq-limit: true + qca,led-active-high: + $ref: /schemas/types.yaml#/definitions/flag + description: + Indicates that the LED pin is active high instead of low + + qca,led-pin: + $ref: /schemas/types.yaml#/definitions/uint8 + description: + Sets the GPIO number for the LED pin instead of the default + qca,no-eeprom: $ref: /schemas/types.yaml#/definitions/flag description: @@ -75,6 +85,8 @@ examples: reg = <0 0 0 0 0>; interrupts = <3>; qca,no-eeprom; + qca,led-active-high; + qca,led-pin = /bits/ 8 <11>; }; }; - |
These platform_data options are now available for OF. Signed-off-by: Rosen Penev <rosenp@gmail.com> --- .../devicetree/bindings/net/wireless/qca,ath9k.yaml | 12 ++++++++++++ 1 file changed, 12 insertions(+)