[1/2] dt/bindings: Add bindings for Broadcom STB DRAM Sensors

Submitted by Markus Mayer on April 18, 2017, 8:17 p.m.

Details

Message ID 20170418201702.57019-2-code@mmayer.net
State Changes Requested
Headers show

Commit Message

Markus Mayer April 18, 2017, 8:17 p.m.
From: Markus Mayer <mmayer@broadcom.com>

Provide bindings for the Broadcom STB DDR PHY Front End (DPFE).

Signed-off-by: Markus Mayer <mmayer@broadcom.com>
---
 .../devicetree/bindings/hwmon/brcmstb-dpfe.txt     | 68 ++++++++++++++++++++++
 1 file changed, 68 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/brcmstb-dpfe.txt

Comments

Markus Mayer April 25, 2017, 7:29 p.m.
Hi Rob,

On 18 April 2017 at 13:17, Markus Mayer <code@mmayer.net> wrote:
> From: Markus Mayer <mmayer@broadcom.com>
>
> Provide bindings for the Broadcom STB DDR PHY Front End (DPFE).

Would you be able to have a look at this binding? The driver won't be
upstreamed as hwmon driver (as per Guenter's comments). I am currently
converting the driver to a "soc" driver instead, but the proposed
binding remains unchanged.

If you have comments or suggestions, I would like to incorporate them
with the new series I will be sending out.

Thanks,
-Markus

> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
> ---
>  .../devicetree/bindings/hwmon/brcmstb-dpfe.txt     | 68 ++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/brcmstb-dpfe.txt
>
> diff --git a/Documentation/devicetree/bindings/hwmon/brcmstb-dpfe.txt b/Documentation/devicetree/bindings/hwmon/brcmstb-dpfe.txt
> new file mode 100644
> index 0000000..3519197
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/brcmstb-dpfe.txt
> @@ -0,0 +1,68 @@
> +DDR PHY Front End (DPFE) for Broadcom STB
> +=========================================
> +
> +DPFE and the DPFE firmware provide an interface for the host CPU to
> +communicate with the DCPU, which resides inside the DDR PHY.
> +
> +There are three memory regions for interacting with the DCPU.
> +
> +The DCPU Register Space
> +-----------------------
> +
> +Required properties:
> +  - compatible: must be one of brcm,bcm7271-dpfe-cpu, brcm,dpfe-cpu-v12.0.0.0
> +    or brcm,dpfe-cpu
> +  - reg: must reference the start address and length of the DCPU register
> +    space
> +
> +Optional properties:
> +  - cell-index: the index of the DPFE instance; will default to 0 if not set
> +
> +Example:
> +       dpfe_cpu0: dpfe-cpu@f1132000 {
> +               compatible = "brcm,bcm7271-dpfe-cpu",
> +                       "brcm,dpfe-cpu-v12.0.0.0",
> +                       "brcm,dpfe-cpu";
> +               reg = <0xf1132000 0x180>;
> +               cell-index = <0>;
> +       };
> +
> +The DCPU Data Memory Space
> +--------------------------
> +
> +Required properties:
> +  - compatible: must be one of brcm,bcm7271-dpfe-dmem, brcm,dpfe-dmem-v12.0.0.0
> +    or brcm,dpfe-dmem
> +  - reg: must reference the start address and length of the DCPU DMEM space
> +
> +Optional properties:
> +  - cell-index: the index of the DPFE instance; will default to 0 if not set
> +
> +Example:
> +       dpfe_dmem0: dpfe-dmem@f1134000 {
> +               compatible = "brcm,bcm7271-dpfe-dmem",
> +                       "brcm,dpfe-dmem-v12.0.0.0",
> +                       "brcm,dpfe-dmem";
> +               reg = <0xf1134000 0x1000>;
> +               cell-index = <0>;
> +       };
> +
> +The DCPU Instruction Memory Space
> +---------------------------------
> +
> +Required properties:
> +  - compatible: must be one of brcm,bcm7271-dpfe-imem, brcm,dpfe-imem-v12.0.0.0
> +    or brcm,dpfe-imem
> +  - reg: must reference the start address and length of the DCPU IMEM space
> +
> +Optional properties:
> +  - cell-index: the index of the DPFE instance; will default to 0 if not set
> +
> +Example:
> +       dpfe_imem0: dpfe-imem@f1138000 {
> +               compatible = "brcm,bcm7271-dpfe-imem",
> +                       "brcm,dpfe-imem-v12.0.0.0",
> +                       "brcm,dpfe-imem";
> +               reg = <0xf1138000 0x4000>;
> +               cell-index = <0>;
> +       };
> --
> 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
Markus Mayer April 27, 2017, 6:28 p.m.
On 25 April 2017 at 12:29, Markus Mayer <markus.mayer@broadcom.com> wrote:
> Hi Rob,
>
> On 18 April 2017 at 13:17, Markus Mayer <code@mmayer.net> wrote:
>> From: Markus Mayer <mmayer@broadcom.com>
>>
>> Provide bindings for the Broadcom STB DDR PHY Front End (DPFE).
>
> Would you be able to have a look at this binding? The driver won't be
> upstreamed as hwmon driver (as per Guenter's comments). I am currently
> converting the driver to a "soc" driver instead, but the proposed
> binding remains unchanged.
>
> If you have comments or suggestions, I would like to incorporate them
> with the new series I will be sending out.

To explain a bit more what we are looking for: we had a internal
discussions how to structure this binding and are looking for some
guidance.

Should we create three different nodes for the three different memory
areas (dpfe-cpu@..., dpfe-dmem@..., dpfe-imem@...), each with a single
"reg" property (which is the proposal below) or should this be one
single property with 3 "reg" cells, i.e. something like this:

dpfe-cpu@f1132000 {
    ...
    reg = <0xf1132000 0x180     /* register space */
           0xf1134000 0x1000    /* data memory */
           0xf1138000 0x4000>;  /* instruction memory */
    ...
};

Regards,
-Markus

>> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
>> ---
>>  .../devicetree/bindings/hwmon/brcmstb-dpfe.txt     | 68 ++++++++++++++++++++++
>>  1 file changed, 68 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/hwmon/brcmstb-dpfe.txt
>>
>> diff --git a/Documentation/devicetree/bindings/hwmon/brcmstb-dpfe.txt b/Documentation/devicetree/bindings/hwmon/brcmstb-dpfe.txt
>> new file mode 100644
>> index 0000000..3519197
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/hwmon/brcmstb-dpfe.txt
>> @@ -0,0 +1,68 @@
>> +DDR PHY Front End (DPFE) for Broadcom STB
>> +=========================================
>> +
>> +DPFE and the DPFE firmware provide an interface for the host CPU to
>> +communicate with the DCPU, which resides inside the DDR PHY.
>> +
>> +There are three memory regions for interacting with the DCPU.
>> +
>> +The DCPU Register Space
>> +-----------------------
>> +
>> +Required properties:
>> +  - compatible: must be one of brcm,bcm7271-dpfe-cpu, brcm,dpfe-cpu-v12.0.0.0
>> +    or brcm,dpfe-cpu
>> +  - reg: must reference the start address and length of the DCPU register
>> +    space
>> +
>> +Optional properties:
>> +  - cell-index: the index of the DPFE instance; will default to 0 if not set
>> +
>> +Example:
>> +       dpfe_cpu0: dpfe-cpu@f1132000 {
>> +               compatible = "brcm,bcm7271-dpfe-cpu",
>> +                       "brcm,dpfe-cpu-v12.0.0.0",
>> +                       "brcm,dpfe-cpu";
>> +               reg = <0xf1132000 0x180>;
>> +               cell-index = <0>;
>> +       };
>> +
>> +The DCPU Data Memory Space
>> +--------------------------
>> +
>> +Required properties:
>> +  - compatible: must be one of brcm,bcm7271-dpfe-dmem, brcm,dpfe-dmem-v12.0.0.0
>> +    or brcm,dpfe-dmem
>> +  - reg: must reference the start address and length of the DCPU DMEM space
>> +
>> +Optional properties:
>> +  - cell-index: the index of the DPFE instance; will default to 0 if not set
>> +
>> +Example:
>> +       dpfe_dmem0: dpfe-dmem@f1134000 {
>> +               compatible = "brcm,bcm7271-dpfe-dmem",
>> +                       "brcm,dpfe-dmem-v12.0.0.0",
>> +                       "brcm,dpfe-dmem";
>> +               reg = <0xf1134000 0x1000>;
>> +               cell-index = <0>;
>> +       };
>> +
>> +The DCPU Instruction Memory Space
>> +---------------------------------
>> +
>> +Required properties:
>> +  - compatible: must be one of brcm,bcm7271-dpfe-imem, brcm,dpfe-imem-v12.0.0.0
>> +    or brcm,dpfe-imem
>> +  - reg: must reference the start address and length of the DCPU IMEM space
>> +
>> +Optional properties:
>> +  - cell-index: the index of the DPFE instance; will default to 0 if not set
>> +
>> +Example:
>> +       dpfe_imem0: dpfe-imem@f1138000 {
>> +               compatible = "brcm,bcm7271-dpfe-imem",
>> +                       "brcm,dpfe-imem-v12.0.0.0",
>> +                       "brcm,dpfe-imem";
>> +               reg = <0xf1138000 0x4000>;
>> +               cell-index = <0>;
>> +       };
>> --
>> 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
Rob Herring April 27, 2017, 9:57 p.m.
On Thu, Apr 27, 2017 at 11:28:37AM -0700, Markus Mayer wrote:
> On 25 April 2017 at 12:29, Markus Mayer <markus.mayer@broadcom.com> wrote:
> > Hi Rob,
> >
> > On 18 April 2017 at 13:17, Markus Mayer <code@mmayer.net> wrote:
> >> From: Markus Mayer <mmayer@broadcom.com>
> >>
> >> Provide bindings for the Broadcom STB DDR PHY Front End (DPFE).
> >
> > Would you be able to have a look at this binding? The driver won't be
> > upstreamed as hwmon driver (as per Guenter's comments). I am currently
> > converting the driver to a "soc" driver instead, but the proposed
> > binding remains unchanged.
> >
> > If you have comments or suggestions, I would like to incorporate them
> > with the new series I will be sending out.
> 
> To explain a bit more what we are looking for: we had a internal
> discussions how to structure this binding and are looking for some
> guidance.
> 
> Should we create three different nodes for the three different memory
> areas (dpfe-cpu@..., dpfe-dmem@..., dpfe-imem@...), each with a single
> "reg" property (which is the proposal below) or should this be one
> single property with 3 "reg" cells, i.e. something like this:

Either way could be okay. It is conceptually 1 thing or 3?

> 
> dpfe-cpu@f1132000 {
>     ...
>     reg = <0xf1132000 0x180     /* register space */
>            0xf1134000 0x1000    /* data memory */
>            0xf1138000 0x4000>;  /* instruction memory */
>     ...
> };
> 
> Regards,
> -Markus
> 
> >> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
> >> ---
> >>  .../devicetree/bindings/hwmon/brcmstb-dpfe.txt     | 68 ++++++++++++++++++++++
> >>  1 file changed, 68 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/hwmon/brcmstb-dpfe.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/hwmon/brcmstb-dpfe.txt b/Documentation/devicetree/bindings/hwmon/brcmstb-dpfe.txt
> >> new file mode 100644
> >> index 0000000..3519197
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/hwmon/brcmstb-dpfe.txt
> >> @@ -0,0 +1,68 @@
> >> +DDR PHY Front End (DPFE) for Broadcom STB
> >> +=========================================
> >> +
> >> +DPFE and the DPFE firmware provide an interface for the host CPU to
> >> +communicate with the DCPU, which resides inside the DDR PHY.
> >> +
> >> +There are three memory regions for interacting with the DCPU.
> >> +
> >> +The DCPU Register Space
> >> +-----------------------
> >> +
> >> +Required properties:
> >> +  - compatible: must be one of brcm,bcm7271-dpfe-cpu, brcm,dpfe-cpu-v12.0.0.0
> >> +    or brcm,dpfe-cpu

3 compatibles is a bit excessive. You can always use 
brcm,bcm7271-dpfe-cpu as a fallback for other chips. I wouldn't expect a 
DDR phy to be around a long time without changes given process and DDR 
technology changes.

> >> +  - reg: must reference the start address and length of the DCPU register
> >> +    space
> >> +
> >> +Optional properties:
> >> +  - cell-index: the index of the DPFE instance; will default to 0 if not set

Don't use cell-index. It's not a valid property for FDT (only real 
OpenFirmware).

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
Florian Fainelli April 27, 2017, 10 p.m.
On 04/27/2017 02:57 PM, Rob Herring wrote:
>>>> +  - reg: must reference the start address and length of the DCPU register
>>>> +    space
>>>> +
>>>> +Optional properties:
>>>> +  - cell-index: the index of the DPFE instance; will default to 0 if not set
> 
> Don't use cell-index. It's not a valid property for FDT (only real 
> OpenFirmware).

My bad, I was advising Markus to use this property since it was largely
used throughout Documentation/devicetree/bindings/. What would be a more
appropriate way to have the same information? Aliases?
Markus Mayer May 3, 2017, 10:29 p.m.
Hi Rob,

On 27 April 2017 at 15:00, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 04/27/2017 02:57 PM, Rob Herring wrote:
>>>>> +Optional properties:
>>>>> +  - cell-index: the index of the DPFE instance; will default to 0 if not set
>>
>> Don't use cell-index. It's not a valid property for FDT (only real
>> OpenFirmware).
>
> My bad, I was advising Markus to use this property since it was largely
> used throughout Documentation/devicetree/bindings/. What would be a more
> appropriate way to have the same information? Aliases?

Hopefully this will be the last time we need to pester you about this.
What should we be using instead of cell-index to identify multiple
instances of a device?

To give you an idea of what the code looks like right now:

ret = of_property_read_u32(dev->of_node, "cell-index", &index);
if (ret)
    index = 0;
[...]
dev_set_name(dpfe_dev, "dpfe%u", index);
ret = device_register(dpfe_dev);

Enumerating the devices like this is what we are after.

Thanks,
-Markus
--
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

Patch hide | download patch | download mbox

diff --git a/Documentation/devicetree/bindings/hwmon/brcmstb-dpfe.txt b/Documentation/devicetree/bindings/hwmon/brcmstb-dpfe.txt
new file mode 100644
index 0000000..3519197
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/brcmstb-dpfe.txt
@@ -0,0 +1,68 @@ 
+DDR PHY Front End (DPFE) for Broadcom STB
+=========================================
+
+DPFE and the DPFE firmware provide an interface for the host CPU to
+communicate with the DCPU, which resides inside the DDR PHY.
+
+There are three memory regions for interacting with the DCPU.
+
+The DCPU Register Space
+-----------------------
+
+Required properties:
+  - compatible: must be one of brcm,bcm7271-dpfe-cpu, brcm,dpfe-cpu-v12.0.0.0
+    or brcm,dpfe-cpu
+  - reg: must reference the start address and length of the DCPU register
+    space
+
+Optional properties:
+  - cell-index: the index of the DPFE instance; will default to 0 if not set
+
+Example:
+	dpfe_cpu0: dpfe-cpu@f1132000 {
+		compatible = "brcm,bcm7271-dpfe-cpu",
+			"brcm,dpfe-cpu-v12.0.0.0",
+			"brcm,dpfe-cpu";
+		reg = <0xf1132000 0x180>;
+		cell-index = <0>;
+	};
+
+The DCPU Data Memory Space
+--------------------------
+
+Required properties:
+  - compatible: must be one of brcm,bcm7271-dpfe-dmem, brcm,dpfe-dmem-v12.0.0.0
+    or brcm,dpfe-dmem
+  - reg: must reference the start address and length of the DCPU DMEM space
+
+Optional properties:
+  - cell-index: the index of the DPFE instance; will default to 0 if not set
+
+Example:
+	dpfe_dmem0: dpfe-dmem@f1134000 {
+		compatible = "brcm,bcm7271-dpfe-dmem",
+			"brcm,dpfe-dmem-v12.0.0.0",
+			"brcm,dpfe-dmem";
+		reg = <0xf1134000 0x1000>;
+		cell-index = <0>;
+	};
+
+The DCPU Instruction Memory Space
+---------------------------------
+
+Required properties:
+  - compatible: must be one of brcm,bcm7271-dpfe-imem, brcm,dpfe-imem-v12.0.0.0
+    or brcm,dpfe-imem
+  - reg: must reference the start address and length of the DCPU IMEM space
+
+Optional properties:
+  - cell-index: the index of the DPFE instance; will default to 0 if not set
+
+Example:
+	dpfe_imem0: dpfe-imem@f1138000 {
+		compatible = "brcm,bcm7271-dpfe-imem",
+			"brcm,dpfe-imem-v12.0.0.0",
+			"brcm,dpfe-imem";
+		reg = <0xf1138000 0x4000>;
+		cell-index = <0>;
+	};