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

[bhupesh.sharma@linaro.org]

Hi Bjorn,

On 7/20/22 3:49 AM, Bjorn Andersson <bjorn.andersson@linaro.org> 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 <bjorn.andersson@linaro.org>
> > Signed-off-by: Bhupesh Sharma <bhupesh.sharma@linaro.org>
> > ---
> >   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
> >
>