2022-07-19 20:52:31

by Bhupesh Sharma

[permalink] [raw]
Subject: [PATCH 1/3] arm64: dts: qcom: Fix 'dtbs_check' errors for pm8350c & sc8280xp pwm nodes

make dtbs_check currently reports the following errors
with 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]+'

Fix the same.

Cc: Bjorn Andersson <[email protected]>
Signed-off-by: Bhupesh Sharma <[email protected]>
---
arch/arm64/boot/dts/qcom/pm8350c.dtsi | 1 -
arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi | 1 -
2 files changed, 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/pm8350c.dtsi b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
index e0bbb67717fe..33f939132571 100644
--- a/arch/arm64/boot/dts/qcom/pm8350c.dtsi
+++ b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
@@ -32,7 +32,6 @@ pm8350c_gpios: gpio@8800 {

pm8350c_pwm: pwm@e800 {
compatible = "qcom,pm8350c-pwm";
- reg = <0xe800>;
#pwm-cells = <2>;
status = "disabled";
};
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
index ae90b97aecb8..69f5bc8127b2 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
@@ -62,7 +62,6 @@ pmc8280c_gpios: gpio@8800 {

pmc8280c_lpg: lpg@e800 {
compatible = "qcom,pm8350c-pwm";
- reg = <0xe800>;

#address-cells = <1>;
#size-cells = <0>;
--
2.35.3


2022-07-19 21:06:42

by Bhupesh Sharma

[permalink] [raw]
Subject: [PATCH 3/3] arm64: dts: qcom: qrb5165-rb5: Fix 'dtbs_check' error for led nodes

make dtbs_check currently reports the following errors
with qrb5165-rb5 led nodes:

arch/arm64/boot/dts/qcom/qrb5165-rb5.dtb:
leds: 'bt', 'user4', 'wlan' do not match any of the regexes:
'(^led-[0-9a-f]$|led)', 'pinctrl-[0-9]+'

Fix the same.

Also while at it, fix a blank line issue in the led
node.

Cc: Bjorn Andersson <[email protected]>
Cc: Dmitry Baryshkov <[email protected]>
Signed-off-by: Bhupesh Sharma <[email protected]>
---
arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
index a7278928efc1..bf8077a1cf9a 100644
--- a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
+++ b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
@@ -58,7 +58,7 @@ hdmi_con: endpoint {
leds {
compatible = "gpio-leds";

- user4 {
+ led-user4 {
label = "green:user4";
function = LED_FUNCTION_INDICATOR;
color = <LED_COLOR_ID_GREEN>;
@@ -67,7 +67,7 @@ user4 {
default-state = "off";
};

- wlan {
+ led-wlan {
label = "yellow:wlan";
function = LED_FUNCTION_WLAN;
color = <LED_COLOR_ID_YELLOW>;
@@ -76,7 +76,7 @@ wlan {
default-state = "off";
};

- bt {
+ led-bt {
label = "blue:bt";
function = LED_FUNCTION_BLUETOOTH;
color = <LED_COLOR_ID_BLUE>;
@@ -84,7 +84,6 @@ bt {
linux,default-trigger = "bluetooth-power";
default-state = "off";
};
-
};

lt9611_1v2: lt9611-vdd12-regulator {
--
2.35.3

2022-07-19 21:17:44

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH 3/3] arm64: dts: qcom: qrb5165-rb5: Fix 'dtbs_check' error for led nodes

On 19/07/2022 23:50, Bhupesh Sharma wrote:
> make dtbs_check currently reports the following errors
> with qrb5165-rb5 led nodes:
>
> arch/arm64/boot/dts/qcom/qrb5165-rb5.dtb:
> leds: 'bt', 'user4', 'wlan' do not match any of the regexes:
> '(^led-[0-9a-f]$|led)', 'pinctrl-[0-9]+'
>
> Fix the same.
>
> Also while at it, fix a blank line issue in the led
> node.
>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Dmitry Baryshkov <[email protected]>
> Signed-off-by: Bhupesh Sharma <[email protected]>

Reviewed-by: Dmitry Baryshkov <[email protected]>

--
With best wishes
Dmitry

2022-07-19 21:19:07

by Bhupesh Sharma

[permalink] [raw]
Subject: [PATCH 2/3] arm64: dts: qcom: qrb5165-rb5: Fix 'dtbs_check' error for lpg nodes

make dtbs_check currently reports the following warnings
with qrb5165-rb5 lpg nodes:

arch/arm64/boot/dts/qcom/qrb5165-rb5.dts:
Warning (reg_format): /soc@0/spmi@c440000/pmic@5/lpg/led@1:reg:
property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1)
arch/arm64/boot/dts/qcom/qrb5165-rb5.dts:
Warning (avoid_default_addr_size): /soc@0/spmi@c440000/pmic@5/lpg/led@1:
Relying on default #address-cells value

Fix the same.

Cc: Bjorn Andersson <[email protected]>
Cc: Dmitry Baryshkov <[email protected]>
Signed-off-by: Bhupesh Sharma <[email protected]>
---
arch/arm64/boot/dts/qcom/qrb5165-rb5.dts | 3 +++
1 file changed, 3 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
index b3740375dd4c..a7278928efc1 100644
--- a/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
+++ b/arch/arm64/boot/dts/qcom/qrb5165-rb5.dts
@@ -816,6 +816,9 @@ lt9611_rst_pin: lt9611-rst-state {
&pm8150l_lpg {
status = "okay";

+ #address-cells = <1>;
+ #size-cells = <0>;
+
led@1 {
reg = <1>;
color = <LED_COLOR_ID_GREEN>;
--
2.35.3

2022-07-19 21:20:28

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH 2/3] arm64: dts: qcom: qrb5165-rb5: Fix 'dtbs_check' error for lpg nodes

On 19/07/2022 23:50, Bhupesh Sharma wrote:
> make dtbs_check currently reports the following warnings
> with qrb5165-rb5 lpg nodes:
>
> arch/arm64/boot/dts/qcom/qrb5165-rb5.dts:
> Warning (reg_format): /soc@0/spmi@c440000/pmic@5/lpg/led@1:reg:
> property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1)
> arch/arm64/boot/dts/qcom/qrb5165-rb5.dts:
> Warning (avoid_default_addr_size): /soc@0/spmi@c440000/pmic@5/lpg/led@1:
> Relying on default #address-cells value
>
> Fix the same.
>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Dmitry Baryshkov <[email protected]>
> Signed-off-by: Bhupesh Sharma <[email protected]>

Reviewed-by: Dmitry Baryshkov <[email protected]>


--
With best wishes
Dmitry

2022-07-19 22:40:51

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH 1/3] arm64: dts: qcom: Fix 'dtbs_check' errors for pm8350c & sc8280xp pwm nodes

On Tue 19 Jul 15:50 CDT 2022, Bhupesh Sharma wrote:

> make dtbs_check currently reports the following errors
> with 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]+'
>
> Fix the same.
>
> Cc: Bjorn Andersson <[email protected]>
> Signed-off-by: Bhupesh Sharma <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/pm8350c.dtsi | 1 -
> arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi | 1 -
> 2 files changed, 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/pm8350c.dtsi b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
> index e0bbb67717fe..33f939132571 100644
> --- a/arch/arm64/boot/dts/qcom/pm8350c.dtsi
> +++ b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
> @@ -32,7 +32,6 @@ pm8350c_gpios: gpio@8800 {
>
> pm8350c_pwm: pwm@e800 {

You need to also drop the unit address.

That said, looking at the dts it's quite nice to have the address of
these nodes. So perhaps we should fix up the binding and populate reg
instead?

Regards,
Bjorn

> compatible = "qcom,pm8350c-pwm";
> - reg = <0xe800>;
> #pwm-cells = <2>;
> status = "disabled";
> };
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
> index ae90b97aecb8..69f5bc8127b2 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
> @@ -62,7 +62,6 @@ pmc8280c_gpios: gpio@8800 {
>
> pmc8280c_lpg: lpg@e800 {
> compatible = "qcom,pm8350c-pwm";
> - reg = <0xe800>;
>
> #address-cells = <1>;
> #size-cells = <0>;
> --
> 2.35.3
>

2022-07-20 08:13:11

by Bhupesh Sharma

[permalink] [raw]
Subject: Re: [PATCH 1/3] arm64: dts: qcom: Fix 'dtbs_check' errors for pm8350c & sc8280xp pwm nodes

Hi Bjorn,

On 7/20/22 3:49 AM, Bjorn Andersson <[email protected]> wrote:
> On Tue 19 Jul 15:50 CDT 2022, Bhupesh Sharma wrote:
>
> > make dtbs_check currently reports the following errors
> > with 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]+'
> >
> > Fix the same.
> >
> > Cc: Bjorn Andersson <[email protected]>
> > Signed-off-by: Bhupesh Sharma <[email protected]>
> > ---
> > arch/arm64/boot/dts/qcom/pm8350c.dtsi | 1 -
> > arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi | 1 -
> > 2 files changed, 2 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/pm8350c.dtsi b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
> > index e0bbb67717fe..33f939132571 100644
> > --- a/arch/arm64/boot/dts/qcom/pm8350c.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
> > @@ -32,7 +32,6 @@ pm8350c_gpios: gpio@8800 {
> >
> > pm8350c_pwm: pwm@e800 {
>
> You need to also drop the unit address.
>
> That said, looking at the dts it's quite nice to have the address of
> these nodes. So perhaps we should fix up the binding and populate reg
> instead?

Well, leaving the unit address in the node label was done to
preserve the address information which might be needed later
(for better readibility).

However, fixing up the binding and populating reg property would
make more sense if the driver actually needs it. Looking at the
qcom led driver in its current form, it doesn't seem to require the
same. Please correct me if I am wrong (as I just had a quick look
at the same).

However, if we still want to have the unit addresses and the reg
property for better readibility in the dts, may be we can mark reg
as an optional property in the binding and leave it up to the
driver to use it (with a future update) optionally.

Please let me know your views.

Regards,
Bhupesh


> > compatible = "qcom,pm8350c-pwm";
> > - reg = <0xe800>;
> > #pwm-cells = <2>;
> > status = "disabled";
> > };
> > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
> > index ae90b97aecb8..69f5bc8127b2 100644
> > --- a/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
> > @@ -62,7 +62,6 @@ pmc8280c_gpios: gpio@8800 {
> >
> > pmc8280c_lpg: lpg@e800 {
> > compatible = "qcom,pm8350c-pwm";
> > - reg = <0xe800>;
> >
> > #address-cells = <1>;
> > #size-cells = <0>;
> > --
> > 2.35.3
> >
>

2022-07-20 15:25:27

by Bjorn Andersson

[permalink] [raw]
Subject: Re: (subset) [PATCH 1/3] arm64: dts: qcom: Fix 'dtbs_check' errors for pm8350c & sc8280xp pwm nodes

On Wed, 20 Jul 2022 02:20:56 +0530, Bhupesh Sharma wrote:
> make dtbs_check currently reports the following errors
> with 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]+'
>
> [...]

Applied, thanks!

[2/3] arm64: dts: qcom: qrb5165-rb5: Fix 'dtbs_check' error for lpg nodes
commit: 1282fa32d71633bce5330a592db6e53cf73d2c28
[3/3] arm64: dts: qcom: qrb5165-rb5: Fix 'dtbs_check' error for led nodes
commit: 360d9526761270f2497893946bb48de468a229cc

Best regards,
--
Bjorn Andersson <[email protected]>

2022-07-20 18:01:16

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH 1/3] arm64: dts: qcom: Fix 'dtbs_check' errors for pm8350c & sc8280xp pwm nodes

On Wed 20 Jul 02:31 CDT 2022, [email protected] wrote:

> Hi Bjorn,
>
> On 7/20/22 3:49 AM, Bjorn Andersson <[email protected]> wrote:
> > On Tue 19 Jul 15:50 CDT 2022, Bhupesh Sharma wrote:
> >
> > > make dtbs_check currently reports the following errors
> > > with 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]+'
> > >
> > > Fix the same.
> > >
> > > Cc: Bjorn Andersson <[email protected]>
> > > Signed-off-by: Bhupesh Sharma <[email protected]>
> > > ---
> > > arch/arm64/boot/dts/qcom/pm8350c.dtsi | 1 -
> > > arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi | 1 -
> > > 2 files changed, 2 deletions(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/pm8350c.dtsi b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
> > > index e0bbb67717fe..33f939132571 100644
> > > --- a/arch/arm64/boot/dts/qcom/pm8350c.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/pm8350c.dtsi
> > > @@ -32,7 +32,6 @@ pm8350c_gpios: gpio@8800 {
> > > > pm8350c_pwm: pwm@e800 {
> >
> > You need to also drop the unit address.
> >
> > That said, looking at the dts it's quite nice to have the address of
> > these nodes. So perhaps we should fix up the binding and populate reg
> > instead?
>
> Well, leaving the unit address in the node label was done to
> preserve the address information which might be needed later
> (for better readibility).
>

Right, I like this part.

> However, fixing up the binding and populating reg property would
> make more sense if the driver actually needs it. Looking at the
> qcom led driver in its current form, it doesn't seem to require the
> same. Please correct me if I am wrong (as I just had a quick look
> at the same).
>

That is correct, the current Linux implementation does not make use of
this information.

> However, if we still want to have the unit addresses and the reg
> property for better readibility in the dts, may be we can mark reg
> as an optional property in the binding and leave it up to the
> driver to use it (with a future update) optionally.
>

Requiring the property only means that the dts is required to have it
specified, not that the driver actually needs to read and use it.

So I'm in favor of amending the DT binding to have the reg specified -
and leaving the Linux driver as is.

Thanks,
Bjorn

> Please let me know your views.
>
> Regards,
> Bhupesh
>
>
> > > compatible = "qcom,pm8350c-pwm";
> > > - reg = <0xe800>;
> > > #pwm-cells = <2>;
> > > status = "disabled";
> > > };
> > > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
> > > index ae90b97aecb8..69f5bc8127b2 100644
> > > --- a/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi
> > > @@ -62,7 +62,6 @@ pmc8280c_gpios: gpio@8800 {
> > > > pmc8280c_lpg: lpg@e800 {
> > > compatible = "qcom,pm8350c-pwm";
> > > - reg = <0xe800>;
> > > > #address-cells = <1>;
> > > #size-cells = <0>;
> > > -- > 2.35.3
> > >
> >