| Message ID | 20220721195502.1525214-1-bhupesh.sharma@linaro.org (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | dt-bindings: leds: Describe optional 'reg' property used for Qualcomm LPG nodes | expand |
On Thu, 21 Jul 2022 at 22:55, Bhupesh Sharma <bhupesh.sharma@linaro.org> wrote: > > As Bjorn noted in [1], it is useful to describe the optional > 'reg' property for Qualcomm LPG nodes as it is used in > some Qualcomm dts files. I don't think this is correct. LPG block maps to several regions, so using just one of them in reg doesn't look correct. > This fixes the following 'make dtbs_check' error reported for > pm8350c & sc8280xp pwm nodes: > > arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dtb: > pwm@e800: 'reg' does not match any of the regexes: > '^led@[0-9a-f]$', 'pinctrl-[0-9]+' I'd prefer to follow the existing schema and to drop the region from those files. > > [1]. https://lore.kernel.org/linux-arm-msm/Ytg3tIaL5h5b9ewH@builder.lan/ > > Cc: Bjorn Andersson <bjorn.andersson@linaro.org> > Cc: robh@kernel.org > Cc: pavel@ucw.cz > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > --- > Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml > index fe336fa16518..f394ab7a757b 100644 > --- a/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml > +++ b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml > @@ -27,6 +27,9 @@ properties: > - qcom,pmi8994-lpg > - qcom,pmi8998-lpg > > + reg: > + maxItems: 1 > + > "#pwm-cells": > const: 2 > > -- > 2.35.3 >
On Thu 21 Jul 13:19 PDT 2022, Dmitry Baryshkov wrote: > On Thu, 21 Jul 2022 at 22:55, Bhupesh Sharma <bhupesh.sharma@linaro.org> wrote: > > > > As Bjorn noted in [1], it is useful to describe the optional > > 'reg' property for Qualcomm LPG nodes as it is used in > > some Qualcomm dts files. > > I don't think this is correct. LPG block maps to several regions, so > using just one of them in reg doesn't look correct. > I agree, but I also like the uniformity of having unit addresses for the devices on the spmi buses. > > This fixes the following 'make dtbs_check' error reported for > > pm8350c & sc8280xp pwm nodes: > > > > arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dtb: > > pwm@e800: 'reg' does not match any of the regexes: > > '^led@[0-9a-f]$', 'pinctrl-[0-9]+' > > I'd prefer to follow the existing schema and to drop the region from > those files. > I'm fine either way, but we have more of these nodes, so I would like to hear from the DT maintainers on the direction to take. All nodes on the spmi bus has an (at least one) address, so it would be accurate to state this in the node. It does however not seem like devicetree@, nor Krzysztof is Cc'ed on this patch, so I've added them... Regards, Bjorn > > > > [1]. https://lore.kernel.org/linux-arm-msm/Ytg3tIaL5h5b9ewH@builder.lan/ > > > > Cc: Bjorn Andersson <bjorn.andersson@linaro.org> > > Cc: robh@kernel.org > > Cc: pavel@ucw.cz > > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> > > --- > > Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml > > index fe336fa16518..f394ab7a757b 100644 > > --- a/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml > > +++ b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml > > @@ -27,6 +27,9 @@ properties: > > - qcom,pmi8994-lpg > > - qcom,pmi8998-lpg > > > > + reg: > > + maxItems: 1 > > + > > "#pwm-cells": > > const: 2 > > > > -- > > 2.35.3 > > > > > -- > With best wishes > Dmitry
On 08/08/2022 21:44, Bjorn Andersson wrote: > On Thu 21 Jul 13:19 PDT 2022, Dmitry Baryshkov wrote: > >> On Thu, 21 Jul 2022 at 22:55, Bhupesh Sharma <bhupesh.sharma@linaro.org> wrote: >>> >>> As Bjorn noted in [1], it is useful to describe the optional >>> 'reg' property for Qualcomm LPG nodes as it is used in >>> some Qualcomm dts files. >> >> I don't think this is correct. LPG block maps to several regions, so >> using just one of them in reg doesn't look correct. >> > > I agree, but I also like the uniformity of having unit addresses for the > devices on the spmi buses. regulators also do not have reg, so I guess consistency is already gone. I vote here to reflect the real hardware/device which means: 1. IIUC, the design of entire SPMI bindings and its implementation is around parent device sitting on SPMI bus and children using its regmap/io space. 2. The children are not really re-usable for different cases/devices (e.g. standalone WLED or LPG, outside of PMIC). 3. This means entire design is tightly coupled and LPG (or wled, regulators) bindings describe the piece of PMIC, thus I find appropriate skipping "reg". 4. If we want to keep the "reg", then it should rather reflect reality, so if Dmitry said - multiple items for separate IO address ranges. > >>> This fixes the following 'make dtbs_check' error reported for >>> pm8350c & sc8280xp pwm nodes: >>> >>> arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dtb: >>> pwm@e800: 'reg' does not match any of the regexes: >>> '^led@[0-9a-f]$', 'pinctrl-[0-9]+' >> >> I'd prefer to follow the existing schema and to drop the region from >> those files. >> > > I'm fine either way, but we have more of these nodes, so I would like to > hear from the DT maintainers on the direction to take. All nodes on the > spmi bus has an (at least one) address, so it would be accurate to state > this in the node. > > It does however not seem like devicetree@, nor Krzysztof is Cc'ed on > this patch, so I've added them... > Anyway this patch has to be resent to properly reach DT patchwork. Bhupesh, Please use scripts/get_maintainer.pl to Cc relevant folks and mailing lists. While resending, add appropriate device prefix to subject, so: dt-bindings: leds: qcom-lpg: Best regards, Krzysztof
On 8/9/22 12:21 PM, Krzysztof Kozlowski wrote: > On 08/08/2022 21:44, Bjorn Andersson wrote: >> On Thu 21 Jul 13:19 PDT 2022, Dmitry Baryshkov wrote: >> >>> On Thu, 21 Jul 2022 at 22:55, Bhupesh Sharma <bhupesh.sharma@linaro.org> wrote: >>>> >>>> As Bjorn noted in [1], it is useful to describe the optional >>>> 'reg' property for Qualcomm LPG nodes as it is used in >>>> some Qualcomm dts files. >>> >>> I don't think this is correct. LPG block maps to several regions, so >>> using just one of them in reg doesn't look correct. >>> >> >> I agree, but I also like the uniformity of having unit addresses for the >> devices on the spmi buses. > > regulators also do not have reg, so I guess consistency is already gone. > > I vote here to reflect the real hardware/device which means: > 1. IIUC, the design of entire SPMI bindings and its implementation is > around parent device sitting on SPMI bus and children using its > regmap/io space. > 2. The children are not really re-usable for different cases/devices > (e.g. standalone WLED or LPG, outside of PMIC). > 3. This means entire design is tightly coupled and LPG (or wled, > regulators) bindings describe the piece of PMIC, thus I find appropriate > skipping "reg". > 4. If we want to keep the "reg", then it should rather reflect reality, > so if Dmitry said - multiple items for separate IO address ranges. Ok, so I think the majority opinion is to skip 'reg' from the devicetree-binding. Lets stick to that. >> >>>> This fixes the following 'make dtbs_check' error reported for >>>> pm8350c & sc8280xp pwm nodes: >>>> >>>> arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dtb: >>>> pwm@e800: 'reg' does not match any of the regexes: >>>> '^led@[0-9a-f]$', 'pinctrl-[0-9]+' >>> >>> I'd prefer to follow the existing schema and to drop the region from >>> those files. >>> >> >> I'm fine either way, but we have more of these nodes, so I would like to >> hear from the DT maintainers on the direction to take. All nodes on the >> spmi bus has an (at least one) address, so it would be accurate to state >> this in the node. >> >> It does however not seem like devicetree@, nor Krzysztof is Cc'ed on >> this patch, so I've added them... >> > > Anyway this patch has to be resent to properly reach DT patchwork. > > Bhupesh, > > Please use scripts/get_maintainer.pl to Cc relevant folks and mailing > lists. While resending, add appropriate device prefix to subject, so: > dt-bindings: leds: qcom-lpg: Sure, will send v2 accordingly. Thanks.
diff --git a/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml index fe336fa16518..f394ab7a757b 100644 --- a/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml +++ b/Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml @@ -27,6 +27,9 @@ properties: - qcom,pmi8994-lpg - qcom,pmi8998-lpg + reg: + maxItems: 1 + "#pwm-cells": const: 2
As Bjorn noted in [1], it is useful to describe the optional 'reg' property for Qualcomm LPG nodes as it is used in some Qualcomm dts files. This fixes the following 'make dtbs_check' error reported for pm8350c & sc8280xp pwm nodes: arch/arm64/boot/dts/qcom/sc7280-herobrine-herobrine-r1.dtb: pwm@e800: 'reg' does not match any of the regexes: '^led@[0-9a-f]$', 'pinctrl-[0-9]+' [1]. https://lore.kernel.org/linux-arm-msm/Ytg3tIaL5h5b9ewH@builder.lan/ Cc: Bjorn Andersson <bjorn.andersson@linaro.org> Cc: robh@kernel.org Cc: pavel@ucw.cz Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org> --- Documentation/devicetree/bindings/leds/leds-qcom-lpg.yaml | 3 +++ 1 file changed, 3 insertions(+)