Messages in this thread |  | | From | Doug Anderson <> | Date | Sat, 13 Jun 2020 13:23:09 -0700 | Subject | Re: [RFC v2 3/3] arm64: dts: qcom: sc7180: Add qfprom-efuse |
| |
Hi,
On Thu, Jun 11, 2020 at 2:49 AM Ravi Kumar Bokka <rbokka@codeaurora.org> wrote: > > This patch adds device tree node for qfprom-efuse controller. > > Signed-off-by: Ravi Kumar Bokka <rbokka@codeaurora.org> > --- > arch/arm64/boot/dts/qcom/sc7180-idp.dts | 4 ++++ > arch/arm64/boot/dts/qcom/sc7180.dtsi | 10 ++++++++-- > 2 files changed, 12 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts b/arch/arm64/boot/dts/qcom/sc7180-idp.dts > index 4e9149d..2a9224e 100644 > --- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts > +++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts > @@ -287,6 +287,10 @@ > }; > }; > > +&qfprom { > + vcc-supply = <&vreg_l11a_1p8>; > +}; > + > &qspi { > status = "okay"; > pinctrl-names = "default"; > diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi > index 31b9217..20f3480 100644 > --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi > @@ -498,9 +498,15 @@ > #power-domain-cells = <1>; > }; > > - qfprom@784000 { > + qfprom: qfprom@780000 {
The dt schema checker claims that your node should actually be called:
eeprom|efuse|nvram
So you probably want the above to actually be:
qfprom: efuse@780000
The label to the side doesn't matter and so it can stay qfprom--just the node name itself is what the checker cares about.
> compatible = "qcom,qfprom";
As per my response in the bindings, this should be:
"qcom,sc7180-qfprom", "qcom,qfprom"
...even if the driver only ever makes use of "qcom,qfprom" this future-proofs us a bit.
> - reg = <0 0x00784000 0 0x8ff>; > + reg = <0 0x00780000 0 0x7a0>, > + <0 0x00782000 0 0x100>, > + <0 0x00784000 0 0x8ff>; > + reg-names = "raw", "conf", "corrected";
As per my response in the bindings, reg-names is discouraged so you should remove and make it so that the driver doesn't need.
> +
It's hard to tell in email, but checkpatch yells above the above line:
ERROR: trailing whitespace #53: FILE: arch/arm64/boot/dts/qcom/sc7180.dtsi:512: +^I^I^I$
> + clocks = <&gcc GCC_SEC_CTRL_CLK_SRC>; > + clock-names = "secclk";
As per the binding spec, clock name shouldn't end in "clk".
For your edification, I provided a patch to fix all my review feedback at:
https://crrev.com/c/2244933
Feel free to squash it into your next version.
-Doug
|  |