| Message ID | 20240325131624.26023-6-brgl@bgdev.pl |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | power: sequencing: implement the subsystem and add first users | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
Bartosz Golaszewski <brgl@bgdev.pl> writes: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > Add device-tree bindings for the ATH12K module found in the WCN7850 > package. > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > --- > .../bindings/net/wireless/qcom,ath12k.yaml | 100 ++++++++++++++++++ > 1 file changed, 100 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml > > diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml > new file mode 100644 > index 000000000000..c0aad4815953 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml > @@ -0,0 +1,100 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +# Copyright (c) 2024 Linaro Limited > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/net/wireless/qcom,ath12k.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Qualcomm Technologies ath12k wireless devices (PCIe) > + > +maintainers: > + - Bartosz Golaszewski <bartosz.golaszewski@linaro.org> IMHO it would be better to have just driver maintainers listed here. > + - Jeff Johnson <quic_jjohnson@quicinc.com> > + - Kalle Valo <kvalo@kernel.org> > + > +description: > + Qualcomm Technologies IEEE 802.11ax PCIe devices. > + > +properties: > + compatible: > + enum: > + - pci17cb,1107 # WCN7850 > + > + reg: > + maxItems: 1 > + > + vddaon-supply: > + description: VDD_AON supply regulator handle > + > + vddwlcx-supply: > + description: VDD_WLCX supply regulator handle > + > + vddwlmx-supply: > + description: VDD_WLMX supply regulator handle > + > + vddrfacmn-supply: > + description: VDD_RFA_CMN supply regulator handle > + > + vddrfa0p8-supply: > + description: VDD_RFA_0P8 supply regulator handle > + > + vddrfa1p2-supply: > + description: VDD_RFA_1P2 supply regulator handle > + > + vddrfa1p8-supply: > + description: VDD_RFA_1P8 supply regulator handle > + > + vddpcie0p9-supply: > + description: VDD_PCIE_0P9 supply regulator handle > + > + vddpcie1p8-supply: > + description: VDD_PCIE_1P8 supply regulator handle > + > +required: > + - compatible > + - reg > + - vddaon-supply > + - vddwlcx-supply > + - vddwlmx-supply > + - vddrfacmn-supply > + - vddrfa0p8-supply > + - vddrfa1p2-supply > + - vddrfa1p8-supply > + - vddpcie0p9-supply > + - vddpcie1p8-supply Same comment here as in patch 4. There are also ath12k PCI devices which don't need DT at all. I don't know if that should be reflected in the bindings doc but I want to point out this.
On 25/03/2024 14:16, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > Add device-tree bindings for the ATH12K module found in the WCN7850 > package. > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > --- > .../bindings/net/wireless/qcom,ath12k.yaml | 100 ++++++++++++++++++ > 1 file changed, 100 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
On Mon, Mar 25, 2024 at 3:01 PM Kalle Valo <kvalo@kernel.org> wrote: > > Bartosz Golaszewski <brgl@bgdev.pl> writes: > > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > + > > +maintainers: > > + - Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > IMHO it would be better to have just driver maintainers listed here. > Why? What's wrong with having the author of the bindings in the Cc list? > > +required: > > + - compatible > > + - reg > > + - vddaon-supply > > + - vddwlcx-supply > > + - vddwlmx-supply > > + - vddrfacmn-supply > > + - vddrfa0p8-supply > > + - vddrfa1p2-supply > > + - vddrfa1p8-supply > > + - vddpcie0p9-supply > > + - vddpcie1p8-supply > > Same comment here as in patch 4. There are also ath12k PCI devices which > don't need DT at all. I don't know if that should be reflected in the > bindings doc but I want to point out this. > But DT bindings don't apply to devices that don't have DT nodes. This isn't an issue at all. Bart
Bartosz Golaszewski <brgl@bgdev.pl> writes: > On Mon, Mar 25, 2024 at 3:01 PM Kalle Valo <kvalo@kernel.org> wrote: >> >> Bartosz Golaszewski <brgl@bgdev.pl> writes: >> >> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >> > >> > + >> > +maintainers: >> > + - Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >> >> IMHO it would be better to have just driver maintainers listed here. >> > > Why? What's wrong with having the author of the bindings in the Cc list? If you want follow the ath12k development and review patches then you can join the ath12k list. I'm not fond of having too many maintainers, it's not really helping anything and just extra work to periodically cleanup the silent maintainers. I would ask the opposite question: why add you as the maintainer? There's not even a single ath12k patch from you, nor I haven't seen you doing any patch review or otherwise helping others related to ath12k. Don't get me wrong, I value the work you do with this important powerseq feature and hopefully we get it into the tree soon. But I don't see adding you as a maintainer at this point.
On 05/04/2024 11:33, Kalle Valo wrote: > Bartosz Golaszewski <brgl@bgdev.pl> writes: > >> On Mon, Mar 25, 2024 at 3:01 PM Kalle Valo <kvalo@kernel.org> wrote: >>> >>> Bartosz Golaszewski <brgl@bgdev.pl> writes: >>> >>>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >>>> >>>> + >>>> +maintainers: >>>> + - Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >>> >>> IMHO it would be better to have just driver maintainers listed here. >>> >> >> Why? What's wrong with having the author of the bindings in the Cc list? > > If you want follow the ath12k development and review patches then you > can join the ath12k list. I'm not fond of having too many maintainers, > it's not really helping anything and just extra work to periodically > cleanup the silent maintainers. > > I would ask the opposite question: why add you as the maintainer? > There's not even a single ath12k patch from you, nor I haven't seen you > doing any patch review or otherwise helping others related to ath12k. > Don't get me wrong, I value the work you do with this important powerseq > feature and hopefully we get it into the tree soon. But I don't see > adding you as a maintainer at this point. This is not a maintainer of driver. This is maintainer of bindings, so someone who has hardware, datasheets, knowledge and/or interest in keeping the bindings accurate. All your arguments above suggest you talk about the driver. This is not the point here. Best regards, Krzysztof
On 05/04/2024 11:37, Krzysztof Kozlowski wrote: > On 05/04/2024 11:33, Kalle Valo wrote: >> Bartosz Golaszewski <brgl@bgdev.pl> writes: >> >>> On Mon, Mar 25, 2024 at 3:01 PM Kalle Valo <kvalo@kernel.org> wrote: >>>> >>>> Bartosz Golaszewski <brgl@bgdev.pl> writes: >>>> >>>>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >>>>> >>>>> + >>>>> +maintainers: >>>>> + - Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >>>> >>>> IMHO it would be better to have just driver maintainers listed here. >>>> >>> >>> Why? What's wrong with having the author of the bindings in the Cc list? >> >> If you want follow the ath12k development and review patches then you >> can join the ath12k list. I'm not fond of having too many maintainers, >> it's not really helping anything and just extra work to periodically >> cleanup the silent maintainers. >> >> I would ask the opposite question: why add you as the maintainer? >> There's not even a single ath12k patch from you, nor I haven't seen you >> doing any patch review or otherwise helping others related to ath12k. >> Don't get me wrong, I value the work you do with this important powerseq >> feature and hopefully we get it into the tree soon. But I don't see >> adding you as a maintainer at this point. > > This is not a maintainer of driver. This is maintainer of bindings, so > someone who has hardware, datasheets, knowledge and/or interest in > keeping the bindings accurate. > > All your arguments above suggest you talk about the driver. This is not > the point here. > And to clarify: I do not have opinion whether Bartosz is a suitable person here and whether driver maintainers should be instead or not. I only want to clarify the purpose of the binding maintainer. Best regards, Krzysztof
On Fri, Apr 5, 2024 at 11:34 AM Kalle Valo <kvalo@kernel.org> wrote: > > Bartosz Golaszewski <brgl@bgdev.pl> writes: > > > On Mon, Mar 25, 2024 at 3:01 PM Kalle Valo <kvalo@kernel.org> wrote: > >> > >> Bartosz Golaszewski <brgl@bgdev.pl> writes: > >> > >> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > >> > > >> > + > >> > +maintainers: > >> > + - Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > >> > >> IMHO it would be better to have just driver maintainers listed here. > >> > > > > Why? What's wrong with having the author of the bindings in the Cc list? > > If you want follow the ath12k development and review patches then you > can join the ath12k list. I'm not fond of having too many maintainers, > it's not really helping anything and just extra work to periodically > cleanup the silent maintainers. > > I would ask the opposite question: why add you as the maintainer? > There's not even a single ath12k patch from you, nor I haven't seen you > doing any patch review or otherwise helping others related to ath12k. > Don't get me wrong, I value the work you do with this important powerseq > feature and hopefully we get it into the tree soon. But I don't see > adding you as a maintainer at this point. > In addition to what Krzysztof already said about you seamingly confusing the maintenance of the driver vs maintenance of the device-tree bindings (IOW: structured hardware description) and in response to your question: I don't see any functional change to any dt-bindings neither from you nor from Jeff. Are you convinced you can maintain and properly review any changes? If so, I don't really care, I can drop myself and have less work. Bartosz > -- > https://patchwork.kernel.org/project/linux-wireless/list/ > > https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On 4/5/2024 2:52 AM, Bartosz Golaszewski wrote: > In addition to what Krzysztof already said about you seamingly > confusing the maintenance of the driver vs maintenance of the > device-tree bindings (IOW: structured hardware description) and in > response to your question: I don't see any functional change to any > dt-bindings neither from you nor from Jeff. Are you convinced you can > maintain and properly review any changes? Speaking just for myself, I know Bartosz is far more capable in this regard than I am, so I'd expect him to be listed as a maintainer before me. /jeff
diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml new file mode 100644 index 000000000000..c0aad4815953 --- /dev/null +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath12k.yaml @@ -0,0 +1,100 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +# Copyright (c) 2024 Linaro Limited +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/net/wireless/qcom,ath12k.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Qualcomm Technologies ath12k wireless devices (PCIe) + +maintainers: + - Bartosz Golaszewski <bartosz.golaszewski@linaro.org> + - Jeff Johnson <quic_jjohnson@quicinc.com> + - Kalle Valo <kvalo@kernel.org> + +description: + Qualcomm Technologies IEEE 802.11ax PCIe devices. + +properties: + compatible: + enum: + - pci17cb,1107 # WCN7850 + + reg: + maxItems: 1 + + vddaon-supply: + description: VDD_AON supply regulator handle + + vddwlcx-supply: + description: VDD_WLCX supply regulator handle + + vddwlmx-supply: + description: VDD_WLMX supply regulator handle + + vddrfacmn-supply: + description: VDD_RFA_CMN supply regulator handle + + vddrfa0p8-supply: + description: VDD_RFA_0P8 supply regulator handle + + vddrfa1p2-supply: + description: VDD_RFA_1P2 supply regulator handle + + vddrfa1p8-supply: + description: VDD_RFA_1P8 supply regulator handle + + vddpcie0p9-supply: + description: VDD_PCIE_0P9 supply regulator handle + + vddpcie1p8-supply: + description: VDD_PCIE_1P8 supply regulator handle + +required: + - compatible + - reg + - vddaon-supply + - vddwlcx-supply + - vddwlmx-supply + - vddrfacmn-supply + - vddrfa0p8-supply + - vddrfa1p2-supply + - vddrfa1p8-supply + - vddpcie0p9-supply + - vddpcie1p8-supply + +additionalProperties: false + +examples: + - | + #include <dt-bindings/clock/qcom,rpmh.h> + #include <dt-bindings/gpio/gpio.h> + pcie { + #address-cells = <3>; + #size-cells = <2>; + + pcie@0 { + device_type = "pci"; + reg = <0x0 0x0 0x0 0x0 0x0>; + #address-cells = <3>; + #size-cells = <2>; + ranges; + + bus-range = <0x01 0xff>; + + wifi@0 { + compatible = "pci17cb,1107"; + reg = <0x10000 0x0 0x0 0x0 0x0>; + + vddaon-supply = <&vreg_pmu_aon_0p59>; + vddwlcx-supply = <&vreg_pmu_wlcx_0p8>; + vddwlmx-supply = <&vreg_pmu_wlmx_0p85>; + vddrfacmn-supply = <&vreg_pmu_rfa_cmn>; + vddrfa0p8-supply = <&vreg_pmu_rfa_0p8>; + vddrfa1p2-supply = <&vreg_pmu_rfa_1p2>; + vddrfa1p8-supply = <&vreg_pmu_rfa_1p8>; + vddpcie0p9-supply = <&vreg_pmu_pcie_0p9>; + vddpcie1p8-supply = <&vreg_pmu_pcie_1p8>; + }; + }; + };