lkml.org 
[lkml]   [2020]   [Jun]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [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

\
 
 \ /
  Last update: 2020-06-13 22:24    [W:0.118 / U:1.144 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site