| Message ID | 20240216203215.40870-4-brgl@bgdev.pl (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | power: sequencing: implement the subsystem and add first users | expand |
On Fri, Feb 16, 2024 at 09:32:00PM +0100, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > The QCA6390 package contains discreet modules for WLAN and Bluetooth. They > are powered by the Power Management Unit (PMU) that takes inputs from the > host and provides LDO outputs. This document describes this module. Please submit patches using subject lines reflecting the style for the subsystem, this makes it easier for people to identify relevant patches. Look at what existing commits in the area you're changing are doing and make sure your subject lines visually resemble what they're doing. There's no need to resubmit to fix this alone.
On Sat, Feb 17, 2024 at 4:48 PM Mark Brown <broonie@kernel.org> wrote: > > On Fri, Feb 16, 2024 at 09:32:00PM +0100, Bartosz Golaszewski wrote: > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > The QCA6390 package contains discreet modules for WLAN and Bluetooth. They > > are powered by the Power Management Unit (PMU) that takes inputs from the > > host and provides LDO outputs. This document describes this module. > > Please submit patches using subject lines reflecting the style for the > subsystem, this makes it easier for people to identify relevant patches. > Look at what existing commits in the area you're changing are doing and > make sure your subject lines visually resemble what they're doing. > There's no need to resubmit to fix this alone. Mark, This is quite vague, could you elaborate? I have no idea what is wrong with this patch. Bartosz
On Sat, Feb 17, 2024 at 07:32:16PM +0100, Bartosz Golaszewski wrote: > On Sat, Feb 17, 2024 at 4:48 PM Mark Brown <broonie@kernel.org> wrote: > > Please submit patches using subject lines reflecting the style for the > > subsystem, this makes it easier for people to identify relevant patches. > > Look at what existing commits in the area you're changing are doing and > > make sure your subject lines visually resemble what they're doing. > > There's no need to resubmit to fix this alone. > This is quite vague, could you elaborate? I have no idea what is wrong > with this patch. The subject line does not look like the subject line for a regulator patch rendering it almost invisible in my inbox. As you will see if you follow the above suggestion and look at other commits to the same area you should see that subject lines should start regulator:.
On 17/02/2024 19:32, Bartosz Golaszewski wrote: > On Sat, Feb 17, 2024 at 4:48 PM Mark Brown <broonie@kernel.org> wrote: >> >> On Fri, Feb 16, 2024 at 09:32:00PM +0100, Bartosz Golaszewski wrote: >>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >>> >>> The QCA6390 package contains discreet modules for WLAN and Bluetooth. They >>> are powered by the Power Management Unit (PMU) that takes inputs from the >>> host and provides LDO outputs. This document describes this module. >> >> Please submit patches using subject lines reflecting the style for the >> subsystem, this makes it easier for people to identify relevant patches. >> Look at what existing commits in the area you're changing are doing and >> make sure your subject lines visually resemble what they're doing. >> There's no need to resubmit to fix this alone. > > Mark, > > This is quite vague, could you elaborate? I have no idea what is wrong > with this patch. Use subject prefixes matching the subsystem. You can get them for example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory your patch is touching. Best regards, Krzysztof
On 16/02/2024 21:32, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > The QCA6390 package contains discreet modules for WLAN and Bluetooth. They > are powered by the Power Management Unit (PMU) that takes inputs from the > host and provides LDO outputs. This document describes this module. > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > --- > .../bindings/regulator/qcom,qca6390-pmu.yaml | 166 ++++++++++++++++++ > 1 file changed, 166 insertions(+) > create mode 100644 Documentation/devicetree/bindings/regulator/qcom,qca6390-pmu.yaml > This matches our internal discussions and how we see the QCA6390 power handling based on datasheet. Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
On Mon, Feb 19, 2024 at 8:32 AM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 17/02/2024 19:32, Bartosz Golaszewski wrote: > > On Sat, Feb 17, 2024 at 4:48 PM Mark Brown <broonie@kernel.org> wrote: > >> > >> On Fri, Feb 16, 2024 at 09:32:00PM +0100, Bartosz Golaszewski wrote: > >>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > >>> > >>> The QCA6390 package contains discreet modules for WLAN and Bluetooth. They > >>> are powered by the Power Management Unit (PMU) that takes inputs from the > >>> host and provides LDO outputs. This document describes this module. > >> > >> Please submit patches using subject lines reflecting the style for the > >> subsystem, this makes it easier for people to identify relevant patches. > >> Look at what existing commits in the area you're changing are doing and > >> make sure your subject lines visually resemble what they're doing. > >> There's no need to resubmit to fix this alone. > > > > Mark, > > > > This is quite vague, could you elaborate? I have no idea what is wrong > > with this patch. > > Use subject prefixes matching the subsystem. You can get them for > example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory > your patch is touching. > > Best regards, > Krzysztof > Yes, I always do. And for Documentation/devicetree/bindings/regulator/ the subjects are split 50:50 between "dt-bindings: regulator: ..." and "regulator: dt-bindings: ...". For Documentation/devicetree/bindings/ it's overwhelmingly "dt-bindings: <subsystem>: ...". It's the first time someone wants me to send a DT bindings patch without "dt-bindings" coming first in the subject. I mean: I can do it alright but it's not stated anywhere explicitly. Bartosz
On 19/02/2024 13:53, Bartosz Golaszewski wrote: > On Mon, Feb 19, 2024 at 8:32 AM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> >> On 17/02/2024 19:32, Bartosz Golaszewski wrote: >>> On Sat, Feb 17, 2024 at 4:48 PM Mark Brown <broonie@kernel.org> wrote: >>>> >>>> On Fri, Feb 16, 2024 at 09:32:00PM +0100, Bartosz Golaszewski wrote: >>>>> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> >>>>> >>>>> The QCA6390 package contains discreet modules for WLAN and Bluetooth. They >>>>> are powered by the Power Management Unit (PMU) that takes inputs from the >>>>> host and provides LDO outputs. This document describes this module. >>>> >>>> Please submit patches using subject lines reflecting the style for the >>>> subsystem, this makes it easier for people to identify relevant patches. >>>> Look at what existing commits in the area you're changing are doing and >>>> make sure your subject lines visually resemble what they're doing. >>>> There's no need to resubmit to fix this alone. >>> >>> Mark, >>> >>> This is quite vague, could you elaborate? I have no idea what is wrong >>> with this patch. >> >> Use subject prefixes matching the subsystem. You can get them for >> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory >> your patch is touching. >> >> Best regards, >> Krzysztof >> > > Yes, I always do. And for Documentation/devicetree/bindings/regulator/ > the subjects are split 50:50 between "dt-bindings: regulator: ..." and No, there are only ~54 "dt + regulator" ones and around 400 starting with "regulator" (Mark removes first prefix if it is not regulator:). So 50 to 400 is not 50:50. > "regulator: dt-bindings: ...". For Documentation/devicetree/bindings/ > it's overwhelmingly "dt-bindings: <subsystem>: ...". It's the first > time someone wants me to send a DT bindings patch without > "dt-bindings" coming first in the subject. > > I mean: I can do it alright but it's not stated anywhere explicitly. Well, practice was kind of known and obvious, but it is also stated: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/submitting-patches.rst?h=next-20240219#n18 Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/regulator/qcom,qca6390-pmu.yaml b/Documentation/devicetree/bindings/regulator/qcom,qca6390-pmu.yaml new file mode 100644 index 000000000000..b71fa782fc63 --- /dev/null +++ b/Documentation/devicetree/bindings/regulator/qcom,qca6390-pmu.yaml @@ -0,0 +1,166 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/regulator/qcom,qca6390-pmu.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Qualcomm Technologies, Inc. QCA6390 PMU Regulators + +maintainers: + - Bartosz Golaszewski <bartosz.golaszewski@linaro.org> + +description: + The QCA6390 package contains discreet modules for WLAN and Bluetooth. They + are powered by the Power Management Unit (PMU) that takes inputs from the + host and provides LDO outputs. This document describes this module. + +properties: + compatible: + const: qcom,qca6390-pmu + + vddaon-supply: + description: VDD_AON supply regulator handle + + vddpmu-supply: + description: VDD_PMU supply regulator handle + + vddrfa0p95-supply: + description: VDD_RFA_0P95 supply regulator handle + + vddrfa1p3-supply: + description: VDD_RFA_1P3 supply regulator handle + + vddrfa1p9-supply: + description: VDD_RFA_1P9 supply regulator handle + + vddpcie1p3-supply: + description: VDD_PCIE_1P3 supply regulator handle<S-Del> + + vddpcie1p9-supply: + description: VDD_PCIE_1P9 supply regulator handle + + vddio-supply: + description: VDD_IO supply regulator handle + + wlan-enable-gpios: + maxItems: 1 + description: + GPIO line enabling the ATH11K WLAN module supplied by the PMU + + bt-enable-gpios: + maxItems: 1 + description: + GPIO line enabling the ATH11K Bluetooth module supplied by the PMU + + regulators: + type: object + description: + LDO outputs of the PMU + + patternProperties: + "^ldo[0-9]$": + $ref: regulator.yaml# + type: object + unevaluatedProperties: false + + additionalProperties: false + +required: + - compatible + - vddaon-supply + - vddpmu-supply + - vddrfa0p95-supply + - vddrfa1p3-supply + - vddrfa1p9-supply + - vddpcie1p3-supply + - vddpcie1p9-supply + - vddio-supply + - wlan-enable-gpios + - bt-enable-gpios + - regulators + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + pmu { + compatible = "qcom,qca6390-pmu"; + + pinctrl-names = "default"; + pinctrl-0 = <&bt_en_state>, <&wlan_en_state>; + + vddaon-supply = <&vreg_s6a_0p95>; + vddpmu-supply = <&vreg_s2f_0p95>; + vddrfa0p95-supply = <&vreg_s2f_0p95>; + vddrfa1p3-supply = <&vreg_s8c_1p3>; + vddrfa1p9-supply = <&vreg_s5a_1p9>; + vddpcie1p3-supply = <&vreg_s8c_1p3>; + vddpcie1p9-supply = <&vreg_s5a_1p9>; + vddio-supply = <&vreg_s4a_1p8>; + + wlan-enable-gpios = <&tlmm 20 GPIO_ACTIVE_HIGH>; + bt-enable-gpios = <&tlmm 21 GPIO_ACTIVE_HIGH>; + + regulators { + vreg_pmu_rfa_cmn: ldo0 { + regulator-name = "vreg_pmu_rfa_cmn"; + regulator-min-microvolt = <760000>; + regulator-max-microvolt = <840000>; + }; + + vreg_pmu_aon_0p59: ldo1 { + regulator-name = "vreg_pmu_aon_0p59"; + regulator-min-microvolt = <540000>; + regulator-max-microvolt = <840000>; + }; + + vreg_pmu_wlcx_0p8: ldo2 { + regulator-name = "vreg_pmu_wlcx_0p8"; + regulator-min-microvolt = <760000>; + regulator-max-microvolt = <840000>; + }; + + vreg_pmu_wlmx_0p85: ldo3 { + regulator-name = "vreg_pmu_wlmx_0p85"; + regulator-min-microvolt = <810000>; + regulator-max-microvolt = <890000>; + }; + + vreg_pmu_btcmx_0p85: ldo4 { + regulator-name = "vreg_pmu_btcmx_0p85"; + regulator-min-microvolt = <810000>; + regulator-max-microvolt = <890000>; + }; + + vreg_pmu_rfa_0p8: ldo5 { + regulator-name = "vreg_pmu_rfa_0p8"; + regulator-min-microvolt = <760000>; + regulator-max-microvolt = <840000>; + }; + + vreg_pmu_rfa_1p2: ldo6 { + regulator-name = "vreg_pmu_rfa_1p2"; + regulator-min-microvolt = <1187000>; + regulator-max-microvolt = <1313000>; + }; + + vreg_pmu_rfa_1p7: ldo7 { + regulator-name = "vreg_pmu_rfa_1p7"; + regulator-min-microvolt = <1710000>; + regulator-max-microvolt = <1890000>; + }; + + vreg_pmu_pcie_0p9: ldo8 { + regulator-name = "vreg_pmu_pcie_0p9"; + regulator-min-microvolt = <870000>; + regulator-max-microvolt = <970000>; + }; + + vreg_pmu_pcie_1p8: ldo9 { + regulator-name = "vreg_pmu_pcie_1p8"; + regulator-min-microvolt = <1710000>; + regulator-max-microvolt = <1890000>; + }; + }; + };