diff mbox series

[v1,1/3] dt-bindings: nvmem: add description for UniPhier eFuse

Message ID 1504221620-358-2-git-send-email-hayashibara.keiji@socionext.com
State New
Headers show
Series add UniPhier efuse support | expand

Commit Message

Keiji Hayashibara Aug. 31, 2017, 11:20 p.m. UTC
Add uniphier-efuse dt-bindings documentation.

Signed-off-by: Keiji Hayashibara <hayashibara.keiji@socionext.com>

---
 .../devicetree/bindings/nvmem/uniphier-efuse.txt   | 45 ++++++++++++++++++++++
 1 file changed, 45 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/nvmem/uniphier-efuse.txt

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Masahiro Yamada Sept. 4, 2017, 12:55 p.m. UTC | #1
2017-09-01 8:20 GMT+09:00 Keiji Hayashibara <hayashibara.keiji@socionext.com>:
> Add uniphier-efuse dt-bindings documentation.

>

> Signed-off-by: Keiji Hayashibara <hayashibara.keiji@socionext.com>

> ---

>  .../devicetree/bindings/nvmem/uniphier-efuse.txt   | 45 ++++++++++++++++++++++

>  1 file changed, 45 insertions(+)

>  create mode 100644 Documentation/devicetree/bindings/nvmem/uniphier-efuse.txt

>

> diff --git a/Documentation/devicetree/bindings/nvmem/uniphier-efuse.txt b/Documentation/devicetree/bindings/nvmem/uniphier-efuse.txt

> new file mode 100644

> index 0000000..09024a2

> --- /dev/null

> +++ b/Documentation/devicetree/bindings/nvmem/uniphier-efuse.txt

> @@ -0,0 +1,45 @@

> += UniPhier eFuse device tree bindings =

> +

> +This UniPhier eFuse must be under soc-glue.

> +

> +Required properties:

> +- compatible: should be "socionext,uniphier-efuse"

> +- reg: should contain the register base and length

> +

> += Data cells =

> +Are child nodes of efuse, bindings of which as described in

> +bindings/nvmem/nvmem.txt

> +

> +Example:

> +

> +       soc-glue@5f900000 {

> +               compatible = "socionext,uniphier-ld20-soc-glue-debug",

> +                            "simple-mfd";

> +               #address-cells = <1>;

> +               #size-cells = <1>;

> +               ranges = <0x0 0x5f900000 0x2000>;



IMHO, I think an empty "ranges;" will clarify the code,
but it is up to your taste.


> +

> +               efuse {

> +                       compatible = "socionext,uniphier-efuse",

> +                                    "syscon";



You are adding a dedicated driver for "socionext,uniphier-efuse".

Then, "syscon" as well?




> +                       reg = <0x100 0xf00>;



Not so many efuse registers exist on the SoC.

reg = <0x100 0x200>; will be enough.


Or if you want to be strict to the hw spec, you can write as follows:

        soc-glue@5f900000 {
               compatible = "socionext,uniphier-ld20-soc-glue-debug";
                            "simple-mfd";
               #address-cells = <1>;
               #size-cells = <1>;
               ranges = <0x0 0x5f900000 0x2000>;

               efuse@100 {
                           compatible = "socionext,uniphier-efuse";
                           reg = <0x100 0x28>;
               };

               efuse@200 {
                           compatible = "socionext,uniphier-efuse";
                           reg = <0x200 0x68>;
               };
       };




> +                       #address-cells = <1>;

> +                       #size-cells = <1>;

> +

> +                       /* Data cells */

> +                       usb_mon: usb_mon {

> +                               reg = <0x154 0xc>;

> +                       };



This <0x154 0xc> represents 0x5f900254 in CPU address view.
(0x5f900000 + 0x100 + 0x154)

So many ranges conversion, and how error-prone..





> +       };

> +

> += Data consumers =

> +Are device nodes which consume nvmem data cells.

> +

> +Example:

> +

> +       usb {

> +               ...

> +               nvmem-cells = <&usb_mon>;

> +               nvmem-cell-names = "usb_mon";

> +       }

> --

> 2.7.4

>

> --

> To unsubscribe from this list: send the line "unsubscribe devicetree" in

> the body of a message to majordomo@vger.kernel.org

> More majordomo info at  http://vger.kernel.org/majordomo-info.html




-- 
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Keiji Hayashibara Sept. 5, 2017, 7:04 a.m. UTC | #2
Hello Yamada-san,

Thank you for your comment.

> From: Masahiro Yamada [mailto:yamada.masahiro@socionext.com]

> Sent: Monday, September 4, 2017 9:56 PM

> 

> 2017-09-01 8:20 GMT+09:00 Keiji Hayashibara

> <hayashibara.keiji@socionext.com>:

> > Add uniphier-efuse dt-bindings documentation.

> >

> > Signed-off-by: Keiji Hayashibara <hayashibara.keiji@socionext.com>

> > ---

> >  .../devicetree/bindings/nvmem/uniphier-efuse.txt   | 45

> ++++++++++++++++++++++

> >  1 file changed, 45 insertions(+)

> >  create mode 100644

> > Documentation/devicetree/bindings/nvmem/uniphier-efuse.txt

> >

> > diff --git

> > a/Documentation/devicetree/bindings/nvmem/uniphier-efuse.txt

> > b/Documentation/devicetree/bindings/nvmem/uniphier-efuse.txt

> > new file mode 100644

> > index 0000000..09024a2

> > --- /dev/null

> > +++ b/Documentation/devicetree/bindings/nvmem/uniphier-efuse.txt

> > @@ -0,0 +1,45 @@

> > += UniPhier eFuse device tree bindings =

> > +

> > +This UniPhier eFuse must be under soc-glue.

> > +

> > +Required properties:

> > +- compatible: should be "socionext,uniphier-efuse"

> > +- reg: should contain the register base and length

> > +

> > += Data cells =

> > +Are child nodes of efuse, bindings of which as described in

> > +bindings/nvmem/nvmem.txt

> > +

> > +Example:

> > +

> > +       soc-glue@5f900000 {

> > +               compatible = "socionext,uniphier-ld20-soc-glue-debug",

> > +                            "simple-mfd";

> > +               #address-cells = <1>;

> > +               #size-cells = <1>;

> > +               ranges = <0x0 0x5f900000 0x2000>;

> 

> 

> IMHO, I think an empty "ranges;" will clarify the code, but it is up to

> your taste.

> 

> 

> > +

> > +               efuse {

> > +                       compatible = "socionext,uniphier-efuse",

> > +                                    "syscon";

> 

> 

> You are adding a dedicated driver for "socionext,uniphier-efuse".

> 

> Then, "syscon" as well?

> 


Since I was using the syscon interface to implement the driver,
I specified "syscon". It's interface is syscon_node_to_regmap().

I will rethink this in v2.

> 

> 

> > +                       reg = <0x100 0xf00>;

> 

> 

> Not so many efuse registers exist on the SoC.

> 

> reg = <0x100 0x200>; will be enough.

> 

> 

> Or if you want to be strict to the hw spec, you can write as follows:

> 

>         soc-glue@5f900000 {

>                compatible = "socionext,uniphier-ld20-soc-glue-debug";

>                             "simple-mfd";

>                #address-cells = <1>;

>                #size-cells = <1>;

>                ranges = <0x0 0x5f900000 0x2000>;

> 

>                efuse@100 {

>                            compatible = "socionext,uniphier-efuse";

>                            reg = <0x100 0x28>;

>                };

> 

>                efuse@200 {

>                            compatible = "socionext,uniphier-efuse";

>                            reg = <0x200 0x68>;

>                };

>        };

> 

> 

> 

> 

> > +                       #address-cells = <1>;

> > +                       #size-cells = <1>;

> > +

> > +                       /* Data cells */

> > +                       usb_mon: usb_mon {

> > +                               reg = <0x154 0xc>;

> > +                       };

> 

> 

> This <0x154 0xc> represents 0x5f900254 in CPU address view.

> (0x5f900000 + 0x100 + 0x154)

> 

> So many ranges conversion, and how error-prone..

> 


Yes, indeed...
I will modify as below.

        soc-glue@5f900000 {
                compatible = "socionext,uniphier-ld20-soc-glue-debug",
                             "simple-mfd";
                #address-cells = <1>;
                #size-cells = <1>;
                ranges;

                efuse@5f900100 {
                        compatible = "socionext,uniphier-efuse";
                        reg = <0x5f900100 0x28>;
                };

                efuse@5f900200 {
                        compatible = "socionext,uniphier-efuse";
                        reg = <0x5f900200 0x68>;
                };
        };


> 

> 

> 

> > +       };

> > +

> > += Data consumers =

> > +Are device nodes which consume nvmem data cells.

> > +

> > +Example:

> > +

> > +       usb {

> > +               ...

> > +               nvmem-cells = <&usb_mon>;

> > +               nvmem-cell-names = "usb_mon";

> > +       }

> > --

> > 2.7.4

> >

> > --

> > To unsubscribe from this list: send the line "unsubscribe devicetree"

> > in the body of a message to majordomo@vger.kernel.org More majordomo

> > info at  http://vger.kernel.org/majordomo-info.html

> 

> 

> 

> --

> Best Regards

> Masahiro Yamada


Best Regards,
Keiji Hayashibara


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring Sept. 12, 2017, 4:15 p.m. UTC | #3
On Tue, Sep 05, 2017 at 04:04:31PM +0900, Keiji Hayashibara wrote:
> Hello Yamada-san,

> 

> Thank you for your comment.

> 

> > From: Masahiro Yamada [mailto:yamada.masahiro@socionext.com]

> > Sent: Monday, September 4, 2017 9:56 PM

> > 

> > 2017-09-01 8:20 GMT+09:00 Keiji Hayashibara

> > <hayashibara.keiji@socionext.com>:

> > > Add uniphier-efuse dt-bindings documentation.

> > >

> > > Signed-off-by: Keiji Hayashibara <hayashibara.keiji@socionext.com>

> > > ---

> > >  .../devicetree/bindings/nvmem/uniphier-efuse.txt   | 45

> > ++++++++++++++++++++++

> > >  1 file changed, 45 insertions(+)

> > >  create mode 100644

> > > Documentation/devicetree/bindings/nvmem/uniphier-efuse.txt


> > > +Example:

> > > +

> > > +       soc-glue@5f900000 {

> > > +               compatible = "socionext,uniphier-ld20-soc-glue-debug",

> > > +                            "simple-mfd";

> > > +               #address-cells = <1>;

> > > +               #size-cells = <1>;

> > > +               ranges = <0x0 0x5f900000 0x2000>;

> > 

> > 

> > IMHO, I think an empty "ranges;" will clarify the code, but it is up to

> > your taste.

> > 

> > 

> > > +

> > > +               efuse {

> > > +                       compatible = "socionext,uniphier-efuse",

> > > +                                    "syscon";

> > 

> > 

> > You are adding a dedicated driver for "socionext,uniphier-efuse".

> > 

> > Then, "syscon" as well?

> > 

> 

> Since I was using the syscon interface to implement the driver,

> I specified "syscon". It's interface is syscon_node_to_regmap().

> 

> I will rethink this in v2.

> 

> > 

> > 

> > > +                       reg = <0x100 0xf00>;

> > 

> > 

> > Not so many efuse registers exist on the SoC.

> > 

> > reg = <0x100 0x200>; will be enough.

> > 

> > 

> > Or if you want to be strict to the hw spec, you can write as follows:

> > 

> >         soc-glue@5f900000 {

> >                compatible = "socionext,uniphier-ld20-soc-glue-debug";

> >                             "simple-mfd";

> >                #address-cells = <1>;

> >                #size-cells = <1>;

> >                ranges = <0x0 0x5f900000 0x2000>;

> > 

> >                efuse@100 {

> >                            compatible = "socionext,uniphier-efuse";

> >                            reg = <0x100 0x28>;

> >                };

> > 

> >                efuse@200 {

> >                            compatible = "socionext,uniphier-efuse";

> >                            reg = <0x200 0x68>;

> >                };

> >        };

> > 

> > 

> > 

> > 

> > > +                       #address-cells = <1>;

> > > +                       #size-cells = <1>;

> > > +

> > > +                       /* Data cells */

> > > +                       usb_mon: usb_mon {

> > > +                               reg = <0x154 0xc>;

> > > +                       };

> > 

> > 

> > This <0x154 0xc> represents 0x5f900254 in CPU address view.

> > (0x5f900000 + 0x100 + 0x154)

> > 

> > So many ranges conversion, and how error-prone..

> > 

> 

> Yes, indeed...

> I will modify as below.


Please don't. A non-empty ranges is preferred. It limits the scope and 
chance for errors (smaller range allows fewer possible values and 
limits the chances of having address ranges duplicated in multiple 
nodes). But yes, it does add the requirement of doing addition and/or 
OR operations. I can't review whether the address ends up being correct 
either way, but having non-empty ranges helps enforce the other things.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/nvmem/uniphier-efuse.txt b/Documentation/devicetree/bindings/nvmem/uniphier-efuse.txt
new file mode 100644
index 0000000..09024a2
--- /dev/null
+++ b/Documentation/devicetree/bindings/nvmem/uniphier-efuse.txt
@@ -0,0 +1,45 @@ 
+= UniPhier eFuse device tree bindings =
+
+This UniPhier eFuse must be under soc-glue.
+
+Required properties:
+- compatible: should be "socionext,uniphier-efuse"
+- reg: should contain the register base and length
+
+= Data cells =
+Are child nodes of efuse, bindings of which as described in
+bindings/nvmem/nvmem.txt
+
+Example:
+
+	soc-glue@5f900000 {
+		compatible = "socionext,uniphier-ld20-soc-glue-debug",
+			     "simple-mfd";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges = <0x0 0x5f900000 0x2000>;
+
+		efuse {
+			compatible = "socionext,uniphier-efuse",
+				     "syscon";
+			reg = <0x100 0xf00>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			/* Data cells */
+			usb_mon: usb_mon {
+				reg = <0x154 0xc>;
+			};
+		};
+	};
+
+= Data consumers =
+Are device nodes which consume nvmem data cells.
+
+Example:
+
+	usb {
+		...
+		nvmem-cells = <&usb_mon>;
+		nvmem-cell-names = "usb_mon";
+	}