[PATCH 0/2] HWMON driver for Broadcom STB DPFE

[PATCH 0/2] HWMON driver for Broadcom STB DPFE

[Markus Mayer]

From: Markus Mayer <mmayer@broadcom.com>

This series introduces a driver to interact with the Broadcom STB DDR
PHY Front End (DPFE), specifically to communicate with the DCPU that is
part of the DDR PHY and which is running its own firmware.

The DCPU provides information such as DRAM refresh rate, which can be
used as indirect indicator for the DRAM temperature (the higher the
refresh rate, the hotter the RAM).

Markus Mayer (2):
  dt/bindings: Add bindings for Broadcom STB DRAM Sensors
  hwmon: (brcmstb) Add driver for Broadcom STB DPFE

 .../devicetree/bindings/hwmon/brcmstb-dpfe.txt     |  68 ++
 MAINTAINERS                                        |   8 +
 drivers/hwmon/Kconfig                              |  13 +
 drivers/hwmon/Makefile                             |   1 +
 drivers/hwmon/brcmstb-dpfe.c                       | 689 +++++++++++++++++++++
 5 files changed, 779 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/brcmstb-dpfe.txt
 create mode 100644 drivers/hwmon/brcmstb-dpfe.c

-- 
2.7.4

[PATCH 0/2] HWMON driver for Broadcom STB DPFE

[Markus Mayer]

From: Markus Mayer <mmayer@broadcom.com>

This series introduces a driver to interact with the Broadcom STB DDR
PHY Front End (DPFE), specifically to communicate with the DCPU that is
part of the DDR PHY and which is running its own firmware.

The DCPU provides information such as DRAM refresh rate, which can be
used as indirect indicator for the DRAM temperature (the higher the
refresh rate, the hotter the RAM).

Markus Mayer (2):
  dt/bindings: Add bindings for Broadcom STB DRAM Sensors
  hwmon: (brcmstb) Add driver for Broadcom STB DPFE

 .../devicetree/bindings/hwmon/brcmstb-dpfe.txt     |  68 ++
 MAINTAINERS                                        |   8 +
 drivers/hwmon/Kconfig                              |  13 +
 drivers/hwmon/Makefile                             |   1 +
 drivers/hwmon/brcmstb-dpfe.c                       | 689 +++++++++++++++++++++
 5 files changed, 779 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/brcmstb-dpfe.txt
 create mode 100644 drivers/hwmon/brcmstb-dpfe.c

-- 
2.7.4

[PATCH 0/2] HWMON driver for Broadcom STB DPFE

[Markus Mayer]

From: Markus Mayer <mmayer-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

This series introduces a driver to interact with the Broadcom STB DDR
PHY Front End (DPFE), specifically to communicate with the DCPU that is
part of the DDR PHY and which is running its own firmware.

The DCPU provides information such as DRAM refresh rate, which can be
used as indirect indicator for the DRAM temperature (the higher the
refresh rate, the hotter the RAM).

Markus Mayer (2):
  dt/bindings: Add bindings for Broadcom STB DRAM Sensors
  hwmon: (brcmstb) Add driver for Broadcom STB DPFE

 .../devicetree/bindings/hwmon/brcmstb-dpfe.txt     |  68 ++
 MAINTAINERS                                        |   8 +
 drivers/hwmon/Kconfig                              |  13 +
 drivers/hwmon/Makefile                             |   1 +
 drivers/hwmon/brcmstb-dpfe.c                       | 689 +++++++++++++++++++++
 5 files changed, 779 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/brcmstb-dpfe.txt
 create mode 100644 drivers/hwmon/brcmstb-dpfe.c

-- 
2.7.4

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

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

[Markus Mayer]

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

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

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

[Markus Mayer]

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

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 at 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 at 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 at 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

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

[Markus Mayer]

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
>

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

[Markus Mayer]

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 at 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 at 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 at 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
>

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

[Markus Mayer]

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
>>

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

[Markus Mayer]

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 at ..., dpfe-dmem at ..., dpfe-imem at ...), 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 at 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 at 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 at 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 at 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
>>

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

[Markus Mayer]

On 25 April 2017 at 12:29, Markus Mayer <markus.mayer-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
> Hi Rob,
>
> On 18 April 2017 at 13:17, Markus Mayer <code-7CzEARzsJhSsTnJN9+BGXg@public.gmane.org> wrote:
>> From: Markus Mayer <mmayer-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>>
>> 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-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>> ---
>>  .../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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Rob Herring]

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

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

[Rob Herring]

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 at ..., dpfe-dmem at ..., dpfe-imem at ...), 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 at 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

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

[Rob Herring]

On Thu, Apr 27, 2017 at 11:28:37AM -0700, Markus Mayer wrote:
> On 25 April 2017 at 12:29, Markus Mayer <markus.mayer-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
> > Hi Rob,
> >
> > On 18 April 2017 at 13:17, Markus Mayer <code-7CzEARzsJhSsTnJN9+BGXg@public.gmane.org> wrote:
> >> From: Markus Mayer <mmayer-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> >>
> >> 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-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> >> ---
> >>  .../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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Florian Fainelli]

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?
-- 
Florian

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

[Florian Fainelli]

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?
-- 
Florian

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

[Markus Mayer]

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

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

[Markus Mayer]

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

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

[Markus Mayer]

Hi Rob,

On 27 April 2017 at 15:00, Florian Fainelli <f.fainelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] hwmon: (brcmstb) Add driver for Broadcom STB DPFE

[Markus Mayer]

From: Markus Mayer <mmayer@broadcom.com>

This driver allows access to DRAM properties, such as the refresh rate,
via the Broadcom STB DDR PHY Front End (DPFE). The refresh rate can be
used as indirect indicator of the DRAM temperature.

The driver also allows setting of the sampling interval.

Signed-off-by: Markus Mayer <mmayer@broadcom.com>
---
 MAINTAINERS                  |   8 +
 drivers/hwmon/Kconfig        |  13 +
 drivers/hwmon/Makefile       |   1 +
 drivers/hwmon/brcmstb-dpfe.c | 689 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 711 insertions(+)
 create mode 100644 drivers/hwmon/brcmstb-dpfe.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 676c139..d2d0495 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2825,6 +2825,14 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/cpufreq/brcm,stb-avs-cpu-freq.txt
 F:	drivers/cpufreq/brcmstb*
 
+BROADCOM STB DPFE HWMON DRIVER
+M:	Markus Mayer <mmayer@broadcom.com>
+M:	bcm-kernel-feedback-list@broadcom.com
+L:	linux-hwmon@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/hwmon/brcmstb*
+F:	drivers/hwmon/brcmstb*
+
 BROADCOM SPECIFIC AMBA DRIVER (BCMA)
 M:	Rafał Miłecki <zajec5@gmail.com>
 L:	linux-wireless@vger.kernel.org
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 0649d53f3..54bb55c 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -355,6 +355,19 @@ config SENSORS_ATXP1
 	  This driver can also be built as a module.  If so, the module
 	  will be called atxp1.
 
+config SENSORS_BRCMSTB_DPFE
+	tristate "BRCMSTB DPFE driver"
+	depends on ARCH_BRCMSTB
+	default y
+	help
+	  If you say yes here you get support for the Broadcom set-top box
+	  DDR PHY Front End (DPFE) interface.
+
+	  The driver allows one to query the DRAM refresh rate, which in turn,
+	  provides an indirect means for deriving the DRAM temperature.
+
+	  If in doubt, say N.
+
 config SENSORS_DS620
 	tristate "Dallas Semiconductor DS620"
 	depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 5509edf..cec13ec 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_SENSORS_ATK0110)	+= asus_atk0110.o
 # Native drivers
 # asb100, then w83781d go first, as they can override other drivers' addresses.
 obj-$(CONFIG_SENSORS_ASB100)	+= asb100.o
+obj-$(CONFIG_SENSORS_BRCMSTB_DPFE) += brcmstb-dpfe.o
 obj-$(CONFIG_SENSORS_W83627HF)	+= w83627hf.o
 obj-$(CONFIG_SENSORS_W83792D)	+= w83792d.o
 obj-$(CONFIG_SENSORS_W83793)	+= w83793.o
diff --git a/drivers/hwmon/brcmstb-dpfe.c b/drivers/hwmon/brcmstb-dpfe.c
new file mode 100644
index 0000000..01abea8
--- /dev/null
+++ b/drivers/hwmon/brcmstb-dpfe.c
@@ -0,0 +1,689 @@
+/*
+ * Temperature sensor driver for Broadcom set top box SoCs
+ *
+ * Copyright (c) 2017 Broadcom
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/delay.h>
+#include <linux/firmware.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+
+#define DRVNAME			"brcmstb-dpfe"
+#define FIRMWARE_NAME		"dpfe.bin"
+#define DT_COMPAT_DMEM		"brcm,dpfe-dmem"
+#define DT_COMPAT_IMEM		"brcm,dpfe-imem"
+
+/* DCPU register offsets */
+#define REG_DCPU_RESET		0x0
+#define REG_TO_DCPU_MBOX	0x10
+#define REG_TO_HOST_MBOX	0x14
+
+/* Message RAM */
+#define DCPU_MSG_RAM(x)		(0x100 + (x) * sizeof(u32))
+
+/* DRAM Info Offsets & Masks */
+#define DRAM_INFO_INTERVAL	0x0
+#define DRAM_INFO_MR4		0x4
+#define DRAM_INFO_ERROR		0x8
+#define DRAM_INFO_MASK		0xff
+
+/* DRAM MR4 Offsets & Masks */
+#define DRAM_MR4_REFRESH	0x0	/* Refresh rate */
+#define DRAM_MR4_SR_ABORT	0x3	/* Self Refresh Abort */
+#define DRAM_MR4_PPRE		0x4	/* Post-package repair entry/exit */
+#define DRAM_MR4_TH_OFFS	0x5	/* Thermal Offset; vendor specific */
+#define DRAM_MR4_TUF		0x7	/* Temperature Update Flag */
+
+#define DRAM_MR4_REFRESH_MASK	0x7
+#define DRAM_MR4_SR_ABORT_MASK	0x1
+#define DRAM_MR4_PPRE_MASK	0x1
+#define DRAM_MR4_TH_OFFS_MASK	0x3
+#define DRAM_MR4_TUF_MASK	0x1
+
+/* DRAM Vendor Offsets & Masks */
+#define DRAM_VENDOR_MR5		0x0
+#define DRAM_VENDOR_MR6		0x4
+#define DRAM_VENDOR_MR7		0x8
+#define DRAM_VENDOR_MR8		0xc
+#define DRAM_VENDOR_ERROR	0x10
+#define DRAM_VENDOR_MASK	0xff
+
+/* Reset register bits & masks */
+#define DCPU_RESET_SHIFT	0x0
+#define DCPU_RESET_MASK		0x1
+#define DCPU_CLK_DISABLE_SHIFT	0x2
+
+/* DCPU return codes */
+#define DCPU_RET_SUCCESS	0x00000001
+#define DCPU_RET_ERR_HEADER	0x80000001
+#define DCPU_RET_ERR_INVAL	0x80000002
+#define DCPU_RET_ERR_CHKSUM	0x80000004
+#define DCPU_RET_ERR_OTHER	0x80000008
+
+/* Firmware magic */
+#define DPFE_BE_MAGIC		0xfe1010fe
+#define DPFE_LE_MAGIC		0xfe0101fe
+
+/* Error codes */
+#define ERR_INVALID_MAGIC	-1
+#define ERR_INVALID_SIZE	-2
+#define ERR_INVALID_CHKSUM	-3
+
+/* Message types */
+#define DPFE_MSG_TYPE_COMMAND	1
+#define DPFE_MSG_TYPE_RESPONSE	2
+
+#define DELAY_LOOP_MAX		200000
+
+enum dpfe_msg_fields {
+	MSG_HEADER,
+	MSG_COMMAND,
+	MSG_ARG_COUNT,
+	MSG_ARG0,
+	MSG_CHKSUM,
+	MSG_FIELD_MAX /* Last entry */
+};
+
+enum dpfe_commands {
+	DPFE_CMD_GET_INFO,
+	DPFE_CMD_GET_REFRESH,
+	DPFE_CMD_GET_VENDOR,
+	DPFE_CMD_MAX /* Last entry */
+};
+
+struct dpfe_msg {
+	u32 header;
+	u32 command;
+	u32 arg_count;
+	u32 arg0;
+	u32 chksum; /* This is the sum of all other entries. */
+};
+
+/*
+ * Format of the binary firmware file:
+ *
+ *   entry
+ *      0    header
+ *              value:  0xfe0101fe  <== little endian
+ *                      0xfe1010fe  <== big endian
+ *      1    sequence:
+ *              [31:16] total segments on this build
+ *              [15:0]  this segment sequence.
+ *      2    FW version
+ *      3    IMEM byte size
+ *      4    DMEM byte size
+ *           IMEM
+ *           DMEM
+ *      last checksum ==> sum of everything
+ */
+struct dpfe_firmware_header {
+	u32 magic;
+	u32 sequence;
+	u32 version;
+	u32 imem_size;
+	u32 dmem_size;
+};
+
+/* Things we only need during initialization. */
+struct init_data {
+	void __iomem *dmem;
+	void __iomem *imem;
+	unsigned int dmem_len;
+	unsigned int imem_len;
+	unsigned int chksum;
+	bool is_big_endian;
+};
+
+/* Things we need for as long as we are active. */
+struct private_data {
+	void __iomem *regs;
+	void __iomem *dmem;
+	struct mutex lock;
+};
+
+/* List of supported firmware commands */
+const u32 dpfe_commands[DPFE_CMD_MAX][MSG_FIELD_MAX] = {
+	[DPFE_CMD_GET_INFO] = {
+		[MSG_HEADER] = DPFE_MSG_TYPE_COMMAND,
+		[MSG_COMMAND] = 1,
+		[MSG_ARG_COUNT] = 1,
+		[MSG_ARG0] = 1,
+		[MSG_CHKSUM] = 4,
+	},
+	[DPFE_CMD_GET_REFRESH] = {
+		[MSG_HEADER] = DPFE_MSG_TYPE_COMMAND,
+		[MSG_COMMAND] = 2,
+		[MSG_ARG_COUNT] = 1,
+		[MSG_ARG0] = 1,
+		[MSG_CHKSUM] = 5,
+	},
+	[DPFE_CMD_GET_VENDOR] = {
+		[MSG_HEADER] = DPFE_MSG_TYPE_COMMAND,
+		[MSG_COMMAND] = 2,
+		[MSG_ARG_COUNT] = 1,
+		[MSG_ARG0] = 2,
+		[MSG_CHKSUM] = 6,
+	},
+};
+
+static u32 dpfe_readl(const void __iomem *addr)
+{
+	return le32_to_cpu(readl_relaxed(addr));
+}
+
+static void dpfe_writel(u32 value, void __iomem *addr)
+{
+	writel_relaxed(cpu_to_le32(value), addr);
+}
+
+static void __iomem *__map_region(const char *name)
+{
+	struct device_node *np;
+	void __iomem *ptr;
+
+	np = of_find_compatible_node(NULL, NULL, name);
+	if (!np)
+		return NULL;
+
+	ptr = of_iomap(np, 0);
+	of_node_put(np);
+
+	return ptr;
+}
+
+static void __disable_dcpu(void __iomem *regs)
+{
+	u32 val;
+
+	/* Check if DCPU is running */
+	val = dpfe_readl(regs + REG_DCPU_RESET);
+	if (!(val & DCPU_RESET_MASK)) {
+		/* Put DCPU in reset */
+		val |= (1 << DCPU_RESET_SHIFT);
+		dpfe_writel(val, regs + REG_DCPU_RESET);
+	}
+}
+
+static void __enable_dcpu(void __iomem *regs)
+{
+	u32 val;
+
+	/* Clear mailbox registers. */
+	dpfe_writel(0, regs + REG_TO_DCPU_MBOX);
+	dpfe_writel(0, regs + REG_TO_HOST_MBOX);
+
+	/* Disable DCPU clock gating */
+	val = dpfe_readl(regs + REG_DCPU_RESET);
+	val &= ~(1 << DCPU_CLK_DISABLE_SHIFT);
+	dpfe_writel(val, regs + REG_DCPU_RESET);
+
+	/* Take DCPU out of reset */
+	val = dpfe_readl(regs + REG_DCPU_RESET);
+	val &= ~(1 << DCPU_RESET_SHIFT);
+	dpfe_writel(val, regs + REG_DCPU_RESET);
+}
+
+static unsigned int get_msg_chksum(const u32 msg[])
+{
+	unsigned int sum = 0;
+	unsigned int i;
+
+	/* Don't include the last field in the checksum. */
+	for (i = 0; i < MSG_FIELD_MAX - 1; i++)
+		sum += msg[i];
+
+	return sum;
+}
+
+static int __send_command(struct private_data *priv, unsigned int cmd,
+			  u32 result[])
+{
+	const u32 *msg = dpfe_commands[cmd];
+	void __iomem *regs = priv->regs;
+	unsigned int i, chksum;
+	int ret = 0;
+	u32 resp;
+
+	if (cmd >= DPFE_CMD_MAX)
+		return -1;
+
+	mutex_lock(&priv->lock);
+
+	/* Write command and arguments to message area */
+	for (i = 0; i < MSG_FIELD_MAX; i++)
+		dpfe_writel(msg[i], regs + DCPU_MSG_RAM(i));
+
+	/* Tell DCPU there is a command waiting */
+	dpfe_writel(1, regs + REG_TO_DCPU_MBOX);
+
+	/* Wait for DCPU to process the command */
+	for (i = 0; i < DELAY_LOOP_MAX; i++) {
+		/* Read response code */
+		resp = dpfe_readl(regs + REG_TO_HOST_MBOX);
+		if (resp > 0)
+			break;
+		udelay(5);
+	}
+	if (i == DELAY_LOOP_MAX)
+		ret = -ETIMEDOUT;
+
+	/* Read response data */
+	for (i = 0; i < MSG_FIELD_MAX; i++)
+		result[i] = dpfe_readl(regs + DCPU_MSG_RAM(i));
+
+	/* Tell DCPU we are done */
+	dpfe_writel(0, regs + REG_TO_HOST_MBOX);
+
+	mutex_unlock(&priv->lock);
+
+	if (!ret) {
+		/* Verify response */
+		chksum = get_msg_chksum(result);
+		if (chksum != result[MSG_CHKSUM])
+			ret = -1;
+	}
+
+	if (!ret) {
+		switch (resp) {
+		case DCPU_RET_SUCCESS:
+			break;
+		case DCPU_RET_ERR_HEADER:
+		case DCPU_RET_ERR_INVAL:
+		case DCPU_RET_ERR_CHKSUM:
+		case DCPU_RET_ERR_OTHER:
+			ret = -1;
+			break;
+		}
+	}
+
+	return ret;
+}
+
+/* Ensure that the firmware file loaded meets all the requirements. */
+static int __verify_firmware(struct init_data *init,
+			     const struct firmware *fw)
+{
+	const struct dpfe_firmware_header *header = (void *)fw->data;
+	unsigned int dmem_size, imem_size, total_size;
+	bool is_big_endian = false;
+	const u32 *chksum;
+
+	if (header->magic == DPFE_BE_MAGIC)
+		is_big_endian = true;
+	else if (header->magic != DPFE_LE_MAGIC)
+		return ERR_INVALID_MAGIC;
+
+	if (is_big_endian) {
+		dmem_size = be32_to_cpu(header->dmem_size);
+		imem_size = be32_to_cpu(header->imem_size);
+	} else {
+		dmem_size = header->dmem_size;
+		imem_size = header->imem_size;
+	}
+
+	/* Data and instruction sections are 32 bit words. */
+	if ((dmem_size % sizeof(u32)) != 0 || (imem_size % sizeof(u32)) != 0)
+		return ERR_INVALID_SIZE;
+
+	/*
+	 * The header + the data section + the instruction section + the
+	 * checksum must be equal to the total firmware size.
+	 */
+	total_size = dmem_size + imem_size + sizeof(*header) + sizeof(*chksum);
+	if (total_size != fw->size)
+		return ERR_INVALID_SIZE;
+
+	/* The checksum comes at the very end. */
+	chksum = (void *)fw->data + sizeof(*header) + dmem_size + imem_size;
+
+	init->is_big_endian = is_big_endian;
+	init->dmem_len = dmem_size;
+	init->imem_len = imem_size;
+	init->chksum = (is_big_endian) ? be32_to_cpu(*chksum) : *chksum;
+
+	return 0;
+}
+
+/* Verify checksum by reading back the firmware from co-processor RAM. */
+static int __verify_fw_checksum(struct init_data *init,
+			     const struct dpfe_firmware_header *header,
+			     u32 checksum)
+{
+	u32 magic, sequence, version, sum;
+	u32 __iomem *dmem = init->dmem;
+	u32 __iomem *imem = init->imem;
+	unsigned int i;
+
+	if (init->is_big_endian) {
+		magic = be32_to_cpu(header->magic);
+		sequence = be32_to_cpu(header->sequence);
+		version = be32_to_cpu(header->version);
+	} else {
+		magic = header->magic;
+		sequence = header->sequence;
+		version = header->version;
+	}
+
+	sum = magic + sequence + version + init->dmem_len + init->imem_len;
+
+	for (i = 0; i < init->dmem_len / sizeof(u32); i++)
+		sum += dpfe_readl(dmem + i);
+
+	for (i = 0; i < init->imem_len / sizeof(u32); i++)
+		sum += dpfe_readl(imem + i);
+
+	return (sum == checksum) ? 0 : -1;
+}
+
+static int __write_firmware(u32 __iomem *mem, const u32 *fw,
+			    unsigned int size, bool is_big_endian)
+{
+	unsigned int i;
+
+	/* Convert size to 32-bit words. */
+	size /= sizeof(u32);
+
+	/* It is recommended to clear the firmware area first. */
+	for (i = 0; i < size; i++)
+		dpfe_writel(0, mem + i);
+
+	/* Now copy it. */
+	if (is_big_endian) {
+		for (i = 0; i < size; i++)
+			dpfe_writel(be32_to_cpu(fw[i]), mem + i);
+	} else {
+		for (i = 0; i < size; i++)
+			dpfe_writel(fw[i], mem + i);
+	}
+
+	return 0;
+}
+
+static int brcmstb_hwmon_download_firwmare(struct platform_device *pdev,
+					   struct init_data *init)
+{
+	const struct dpfe_firmware_header *header;
+	unsigned int dmem_size, imem_size;
+	struct device *dev = &pdev->dev;
+	bool is_big_endian = false;
+	struct private_data *priv;
+	const struct firmware *fw;
+	const u32 *dmem, *imem;
+	const void *fw_blob;
+	int ret;
+
+	ret = request_firmware(&fw, FIRMWARE_NAME, dev);
+	/* request_firmware() prints its own error messages. */
+	if (ret)
+		return ret;
+
+	priv = platform_get_drvdata(pdev);
+
+	ret = __verify_firmware(init, fw);
+	if (ret)
+		return -EFAULT;
+
+	__disable_dcpu(priv->regs);
+
+	is_big_endian = init->is_big_endian;
+	dmem_size = init->dmem_len;
+	imem_size = init->imem_len;
+
+	/* At the beginning of the firmware blob is a header. */
+	header = (struct dpfe_firmware_header *)fw->data;
+	/* Void pointer to the beginning of the actual firmware. */
+	fw_blob = fw->data + sizeof(*header);
+	/* IMEM comes right after the header. */
+	imem = fw_blob;
+	/* DMEM follows after IMEM. */
+	dmem = fw_blob + imem_size;
+
+	ret = __write_firmware(init->dmem, dmem, dmem_size, is_big_endian);
+	if (ret)
+		return ret;
+	ret = __write_firmware(init->imem, imem, imem_size, is_big_endian);
+	if (ret)
+		return ret;
+
+	ret = __verify_fw_checksum(init, header, init->chksum);
+	if (ret)
+		return ret;
+
+	__enable_dcpu(priv->regs);
+
+	return 0;
+}
+
+static ssize_t generic_show(unsigned int command, u32 response[],
+			    struct device *dev, char *buf)
+{
+	struct private_data *priv;
+	int ret;
+
+	priv = dev_get_drvdata(dev);
+
+	ret = __send_command(priv, command, response);
+	if (ret)
+		return sprintf(buf, "error %d\n", ret);
+
+	return 0;
+}
+
+static ssize_t show_info(struct device *dev, struct device_attribute *devattr,
+			 char *buf)
+{
+	u32 response[MSG_FIELD_MAX];
+	unsigned int info;
+	int ret;
+
+	ret = generic_show(DPFE_CMD_GET_INFO, response, dev, buf);
+	if (ret)
+		return ret;
+
+	info = response[MSG_ARG0];
+
+	return sprintf(buf, "%u.%u.%u.%u\n",
+		       (info >> 24) & 0xff,
+		       (info >> 16) & 0xff,
+		       (info >> 8) & 0xff,
+		       info & 0xff);
+}
+
+static ssize_t show_refresh(struct device *dev,
+			    struct device_attribute *devattr, char *buf)
+{
+	u32 response[MSG_FIELD_MAX];
+	void __iomem *info;
+	struct private_data *priv;
+	unsigned int offset;
+	u8 refresh, sr_abort, ppre, thermal_offs, tuf;
+	u32 mr4;
+	int ret;
+
+	ret = generic_show(DPFE_CMD_GET_REFRESH, response, dev, buf);
+	if (ret)
+		return ret;
+
+	priv = dev_get_drvdata(dev);
+	offset = response[MSG_ARG0];
+	info = priv->dmem + offset;
+
+	mr4 = dpfe_readl(info + DRAM_INFO_MR4) & DRAM_INFO_MASK;
+
+	refresh = (mr4 >> DRAM_MR4_REFRESH) & DRAM_MR4_REFRESH_MASK;
+	sr_abort = (mr4 >> DRAM_MR4_SR_ABORT) & DRAM_MR4_SR_ABORT_MASK;
+	ppre = (mr4 >> DRAM_MR4_PPRE) & DRAM_MR4_PPRE_MASK;
+	thermal_offs = (mr4 >> DRAM_MR4_TH_OFFS) & DRAM_MR4_TH_OFFS_MASK;
+	tuf = (mr4 >> DRAM_MR4_TUF) & DRAM_MR4_TUF_MASK;
+
+	return sprintf(buf, "%#x %#x %#x %#x %#x %#x %#x\n",
+		       dpfe_readl(info + DRAM_INFO_INTERVAL),
+		       refresh, sr_abort, ppre, thermal_offs, tuf,
+		       dpfe_readl(info + DRAM_INFO_ERROR));
+}
+
+static ssize_t store_refresh(struct device *dev, struct device_attribute *attr,
+			  const char *buf, size_t count)
+{
+	u32 response[MSG_FIELD_MAX];
+	struct private_data *priv;
+	void __iomem *info;
+	unsigned int offset;
+	unsigned long val;
+	int ret;
+
+	if (kstrtoul(buf, 0, &val) < 0)
+		return -EINVAL;
+
+	priv = dev_get_drvdata(dev);
+
+	ret = __send_command(priv, DPFE_CMD_GET_REFRESH, response);
+	if (ret)
+		return ret;
+
+	offset = response[MSG_ARG0];
+	info = priv->dmem + offset + DRAM_MR4_REFRESH;
+	dpfe_writel(val, info);
+
+	return count;
+}
+
+
+static ssize_t show_vendor(struct device *dev, struct device_attribute *devattr,
+			 char *buf)
+{
+	u32 response[MSG_FIELD_MAX];
+	struct private_data *priv;
+	void __iomem *info;
+	unsigned int offset;
+	int ret;
+
+	ret = generic_show(DPFE_CMD_GET_VENDOR, response, dev, buf);
+	if (ret)
+		return ret;
+
+	offset = response[MSG_ARG0];
+	priv = dev_get_drvdata(dev);
+	info = priv->dmem + offset;
+
+	return sprintf(buf, "%#x %#x %#x %#x %#x\n",
+		       dpfe_readl(info + DRAM_VENDOR_MR5) & DRAM_VENDOR_MASK,
+		       dpfe_readl(info + DRAM_VENDOR_MR6) & DRAM_VENDOR_MASK,
+		       dpfe_readl(info + DRAM_VENDOR_MR7) & DRAM_VENDOR_MASK,
+		       dpfe_readl(info + DRAM_VENDOR_MR8) & DRAM_VENDOR_MASK,
+		       dpfe_readl(info + DRAM_VENDOR_ERROR));
+}
+
+static SENSOR_DEVICE_ATTR(dpfe_info, 0444, show_info, NULL, 1000);
+static SENSOR_DEVICE_ATTR(dpfe_refresh, 0644, show_refresh, store_refresh,
+			  1000);
+static SENSOR_DEVICE_ATTR(dpfe_vendor, 0444, show_vendor, NULL, 1000);
+static struct attribute *dpfe_attrs[] = {
+	&sensor_dev_attr_dpfe_info.dev_attr.attr,
+	&sensor_dev_attr_dpfe_refresh.dev_attr.attr,
+	&sensor_dev_attr_dpfe_vendor.dev_attr.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(dpfe);
+
+static int brcmstb_hwmon_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device *hwmon_dev = NULL;
+	struct private_data *priv;
+	struct init_data init;
+	struct resource *res;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	mutex_init(&priv->lock);
+	platform_set_drvdata(pdev, priv);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	priv->regs = devm_ioremap_resource(dev, res);
+	if (IS_ERR(priv->regs)) {
+		dev_err(dev, "couldn't map DT entry brcm,dpfe-cpu\n");
+		return -ENODEV;
+	}
+
+	init.dmem = __map_region(DT_COMPAT_DMEM);
+	if (!init.dmem) {
+		dev_err(dev, "Couldn't map %s\n", DT_COMPAT_DMEM);
+		return -ENOENT;
+	}
+	init.imem = __map_region(DT_COMPAT_IMEM);
+	if (init.imem) {
+		ret = brcmstb_hwmon_download_firwmare(pdev, &init);
+	} else {
+		ret = -ENOENT;
+		dev_err(dev, "Couldn't map %s\n", DT_COMPAT_IMEM);
+	}
+
+	/* We don't need IMEM after initialization. */
+	iounmap(init.imem);
+
+	if (!ret) {
+		hwmon_dev = devm_hwmon_device_register_with_groups(dev,
+			"brcmstb_dpfe", priv, dpfe_groups);
+		if (IS_ERR(hwmon_dev))
+			ret = PTR_ERR(hwmon_dev);
+	}
+
+	if (ret) {
+		iounmap(init.dmem);
+		dev_err(dev, "failed to initialize -- error %d\n", ret);
+	} else {
+		priv->dmem = init.dmem;
+		dev_info(dev, "registered.\n");
+	}
+
+	return ret;
+}
+
+static int brcmstb_hwmon_remove(struct platform_device *pdev)
+{
+	struct private_data *priv;
+
+	priv = platform_get_drvdata(pdev);
+	iounmap(priv->dmem);
+
+	return 0;
+}
+
+static const struct of_device_id brcmstb_hwmon_of_match[] = {
+	{ .compatible = "brcm,dpfe-cpu", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, brcmstb_hwmon_of_match);
+
+static struct platform_driver brcmstb_hwmon_driver = {
+	.driver	= {
+		.name = DRVNAME,
+		.of_match_table = brcmstb_hwmon_of_match,
+	},
+	.probe = brcmstb_hwmon_probe,
+	.remove = brcmstb_hwmon_remove,
+};
+
+module_platform_driver(brcmstb_hwmon_driver);
+
+MODULE_AUTHOR("Markus Mayer <mmayer@broadcom.com>");
+MODULE_DESCRIPTION("BRCMSTB Hardware Monitoring");
+MODULE_LICENSE("GPL");
-- 
2.7.4

[PATCH 2/2] hwmon: (brcmstb) Add driver for Broadcom STB DPFE

[Markus Mayer]

From: Markus Mayer <mmayer@broadcom.com>

This driver allows access to DRAM properties, such as the refresh rate,
via the Broadcom STB DDR PHY Front End (DPFE). The refresh rate can be
used as indirect indicator of the DRAM temperature.

The driver also allows setting of the sampling interval.

Signed-off-by: Markus Mayer <mmayer@broadcom.com>
---
 MAINTAINERS                  |   8 +
 drivers/hwmon/Kconfig        |  13 +
 drivers/hwmon/Makefile       |   1 +
 drivers/hwmon/brcmstb-dpfe.c | 689 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 711 insertions(+)
 create mode 100644 drivers/hwmon/brcmstb-dpfe.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 676c139..d2d0495 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2825,6 +2825,14 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/cpufreq/brcm,stb-avs-cpu-freq.txt
 F:	drivers/cpufreq/brcmstb*
 
+BROADCOM STB DPFE HWMON DRIVER
+M:	Markus Mayer <mmayer@broadcom.com>
+M:	bcm-kernel-feedback-list at broadcom.com
+L:	linux-hwmon at vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/hwmon/brcmstb*
+F:	drivers/hwmon/brcmstb*
+
 BROADCOM SPECIFIC AMBA DRIVER (BCMA)
 M:	Rafa? Mi?ecki <zajec5@gmail.com>
 L:	linux-wireless at vger.kernel.org
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 0649d53f3..54bb55c 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -355,6 +355,19 @@ config SENSORS_ATXP1
 	  This driver can also be built as a module.  If so, the module
 	  will be called atxp1.
 
+config SENSORS_BRCMSTB_DPFE
+	tristate "BRCMSTB DPFE driver"
+	depends on ARCH_BRCMSTB
+	default y
+	help
+	  If you say yes here you get support for the Broadcom set-top box
+	  DDR PHY Front End (DPFE) interface.
+
+	  The driver allows one to query the DRAM refresh rate, which in turn,
+	  provides an indirect means for deriving the DRAM temperature.
+
+	  If in doubt, say N.
+
 config SENSORS_DS620
 	tristate "Dallas Semiconductor DS620"
 	depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 5509edf..cec13ec 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_SENSORS_ATK0110)	+= asus_atk0110.o
 # Native drivers
 # asb100, then w83781d go first, as they can override other drivers' addresses.
 obj-$(CONFIG_SENSORS_ASB100)	+= asb100.o
+obj-$(CONFIG_SENSORS_BRCMSTB_DPFE) += brcmstb-dpfe.o
 obj-$(CONFIG_SENSORS_W83627HF)	+= w83627hf.o
 obj-$(CONFIG_SENSORS_W83792D)	+= w83792d.o
 obj-$(CONFIG_SENSORS_W83793)	+= w83793.o
diff --git a/drivers/hwmon/brcmstb-dpfe.c b/drivers/hwmon/brcmstb-dpfe.c
new file mode 100644
index 0000000..01abea8
--- /dev/null
+++ b/drivers/hwmon/brcmstb-dpfe.c
@@ -0,0 +1,689 @@
+/*
+ * Temperature sensor driver for Broadcom set top box SoCs
+ *
+ * Copyright (c) 2017 Broadcom
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/delay.h>
+#include <linux/firmware.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+
+#define DRVNAME			"brcmstb-dpfe"
+#define FIRMWARE_NAME		"dpfe.bin"
+#define DT_COMPAT_DMEM		"brcm,dpfe-dmem"
+#define DT_COMPAT_IMEM		"brcm,dpfe-imem"
+
+/* DCPU register offsets */
+#define REG_DCPU_RESET		0x0
+#define REG_TO_DCPU_MBOX	0x10
+#define REG_TO_HOST_MBOX	0x14
+
+/* Message RAM */
+#define DCPU_MSG_RAM(x)		(0x100 + (x) * sizeof(u32))
+
+/* DRAM Info Offsets & Masks */
+#define DRAM_INFO_INTERVAL	0x0
+#define DRAM_INFO_MR4		0x4
+#define DRAM_INFO_ERROR		0x8
+#define DRAM_INFO_MASK		0xff
+
+/* DRAM MR4 Offsets & Masks */
+#define DRAM_MR4_REFRESH	0x0	/* Refresh rate */
+#define DRAM_MR4_SR_ABORT	0x3	/* Self Refresh Abort */
+#define DRAM_MR4_PPRE		0x4	/* Post-package repair entry/exit */
+#define DRAM_MR4_TH_OFFS	0x5	/* Thermal Offset; vendor specific */
+#define DRAM_MR4_TUF		0x7	/* Temperature Update Flag */
+
+#define DRAM_MR4_REFRESH_MASK	0x7
+#define DRAM_MR4_SR_ABORT_MASK	0x1
+#define DRAM_MR4_PPRE_MASK	0x1
+#define DRAM_MR4_TH_OFFS_MASK	0x3
+#define DRAM_MR4_TUF_MASK	0x1
+
+/* DRAM Vendor Offsets & Masks */
+#define DRAM_VENDOR_MR5		0x0
+#define DRAM_VENDOR_MR6		0x4
+#define DRAM_VENDOR_MR7		0x8
+#define DRAM_VENDOR_MR8		0xc
+#define DRAM_VENDOR_ERROR	0x10
+#define DRAM_VENDOR_MASK	0xff
+
+/* Reset register bits & masks */
+#define DCPU_RESET_SHIFT	0x0
+#define DCPU_RESET_MASK		0x1
+#define DCPU_CLK_DISABLE_SHIFT	0x2
+
+/* DCPU return codes */
+#define DCPU_RET_SUCCESS	0x00000001
+#define DCPU_RET_ERR_HEADER	0x80000001
+#define DCPU_RET_ERR_INVAL	0x80000002
+#define DCPU_RET_ERR_CHKSUM	0x80000004
+#define DCPU_RET_ERR_OTHER	0x80000008
+
+/* Firmware magic */
+#define DPFE_BE_MAGIC		0xfe1010fe
+#define DPFE_LE_MAGIC		0xfe0101fe
+
+/* Error codes */
+#define ERR_INVALID_MAGIC	-1
+#define ERR_INVALID_SIZE	-2
+#define ERR_INVALID_CHKSUM	-3
+
+/* Message types */
+#define DPFE_MSG_TYPE_COMMAND	1
+#define DPFE_MSG_TYPE_RESPONSE	2
+
+#define DELAY_LOOP_MAX		200000
+
+enum dpfe_msg_fields {
+	MSG_HEADER,
+	MSG_COMMAND,
+	MSG_ARG_COUNT,
+	MSG_ARG0,
+	MSG_CHKSUM,
+	MSG_FIELD_MAX /* Last entry */
+};
+
+enum dpfe_commands {
+	DPFE_CMD_GET_INFO,
+	DPFE_CMD_GET_REFRESH,
+	DPFE_CMD_GET_VENDOR,
+	DPFE_CMD_MAX /* Last entry */
+};
+
+struct dpfe_msg {
+	u32 header;
+	u32 command;
+	u32 arg_count;
+	u32 arg0;
+	u32 chksum; /* This is the sum of all other entries. */
+};
+
+/*
+ * Format of the binary firmware file:
+ *
+ *   entry
+ *      0    header
+ *              value:  0xfe0101fe  <== little endian
+ *                      0xfe1010fe  <== big endian
+ *      1    sequence:
+ *              [31:16] total segments on this build
+ *              [15:0]  this segment sequence.
+ *      2    FW version
+ *      3    IMEM byte size
+ *      4    DMEM byte size
+ *           IMEM
+ *           DMEM
+ *      last checksum ==> sum of everything
+ */
+struct dpfe_firmware_header {
+	u32 magic;
+	u32 sequence;
+	u32 version;
+	u32 imem_size;
+	u32 dmem_size;
+};
+
+/* Things we only need during initialization. */
+struct init_data {
+	void __iomem *dmem;
+	void __iomem *imem;
+	unsigned int dmem_len;
+	unsigned int imem_len;
+	unsigned int chksum;
+	bool is_big_endian;
+};
+
+/* Things we need for as long as we are active. */
+struct private_data {
+	void __iomem *regs;
+	void __iomem *dmem;
+	struct mutex lock;
+};
+
+/* List of supported firmware commands */
+const u32 dpfe_commands[DPFE_CMD_MAX][MSG_FIELD_MAX] = {
+	[DPFE_CMD_GET_INFO] = {
+		[MSG_HEADER] = DPFE_MSG_TYPE_COMMAND,
+		[MSG_COMMAND] = 1,
+		[MSG_ARG_COUNT] = 1,
+		[MSG_ARG0] = 1,
+		[MSG_CHKSUM] = 4,
+	},
+	[DPFE_CMD_GET_REFRESH] = {
+		[MSG_HEADER] = DPFE_MSG_TYPE_COMMAND,
+		[MSG_COMMAND] = 2,
+		[MSG_ARG_COUNT] = 1,
+		[MSG_ARG0] = 1,
+		[MSG_CHKSUM] = 5,
+	},
+	[DPFE_CMD_GET_VENDOR] = {
+		[MSG_HEADER] = DPFE_MSG_TYPE_COMMAND,
+		[MSG_COMMAND] = 2,
+		[MSG_ARG_COUNT] = 1,
+		[MSG_ARG0] = 2,
+		[MSG_CHKSUM] = 6,
+	},
+};
+
+static u32 dpfe_readl(const void __iomem *addr)
+{
+	return le32_to_cpu(readl_relaxed(addr));
+}
+
+static void dpfe_writel(u32 value, void __iomem *addr)
+{
+	writel_relaxed(cpu_to_le32(value), addr);
+}
+
+static void __iomem *__map_region(const char *name)
+{
+	struct device_node *np;
+	void __iomem *ptr;
+
+	np = of_find_compatible_node(NULL, NULL, name);
+	if (!np)
+		return NULL;
+
+	ptr = of_iomap(np, 0);
+	of_node_put(np);
+
+	return ptr;
+}
+
+static void __disable_dcpu(void __iomem *regs)
+{
+	u32 val;
+
+	/* Check if DCPU is running */
+	val = dpfe_readl(regs + REG_DCPU_RESET);
+	if (!(val & DCPU_RESET_MASK)) {
+		/* Put DCPU in reset */
+		val |= (1 << DCPU_RESET_SHIFT);
+		dpfe_writel(val, regs + REG_DCPU_RESET);
+	}
+}
+
+static void __enable_dcpu(void __iomem *regs)
+{
+	u32 val;
+
+	/* Clear mailbox registers. */
+	dpfe_writel(0, regs + REG_TO_DCPU_MBOX);
+	dpfe_writel(0, regs + REG_TO_HOST_MBOX);
+
+	/* Disable DCPU clock gating */
+	val = dpfe_readl(regs + REG_DCPU_RESET);
+	val &= ~(1 << DCPU_CLK_DISABLE_SHIFT);
+	dpfe_writel(val, regs + REG_DCPU_RESET);
+
+	/* Take DCPU out of reset */
+	val = dpfe_readl(regs + REG_DCPU_RESET);
+	val &= ~(1 << DCPU_RESET_SHIFT);
+	dpfe_writel(val, regs + REG_DCPU_RESET);
+}
+
+static unsigned int get_msg_chksum(const u32 msg[])
+{
+	unsigned int sum = 0;
+	unsigned int i;
+
+	/* Don't include the last field in the checksum. */
+	for (i = 0; i < MSG_FIELD_MAX - 1; i++)
+		sum += msg[i];
+
+	return sum;
+}
+
+static int __send_command(struct private_data *priv, unsigned int cmd,
+			  u32 result[])
+{
+	const u32 *msg = dpfe_commands[cmd];
+	void __iomem *regs = priv->regs;
+	unsigned int i, chksum;
+	int ret = 0;
+	u32 resp;
+
+	if (cmd >= DPFE_CMD_MAX)
+		return -1;
+
+	mutex_lock(&priv->lock);
+
+	/* Write command and arguments to message area */
+	for (i = 0; i < MSG_FIELD_MAX; i++)
+		dpfe_writel(msg[i], regs + DCPU_MSG_RAM(i));
+
+	/* Tell DCPU there is a command waiting */
+	dpfe_writel(1, regs + REG_TO_DCPU_MBOX);
+
+	/* Wait for DCPU to process the command */
+	for (i = 0; i < DELAY_LOOP_MAX; i++) {
+		/* Read response code */
+		resp = dpfe_readl(regs + REG_TO_HOST_MBOX);
+		if (resp > 0)
+			break;
+		udelay(5);
+	}
+	if (i == DELAY_LOOP_MAX)
+		ret = -ETIMEDOUT;
+
+	/* Read response data */
+	for (i = 0; i < MSG_FIELD_MAX; i++)
+		result[i] = dpfe_readl(regs + DCPU_MSG_RAM(i));
+
+	/* Tell DCPU we are done */
+	dpfe_writel(0, regs + REG_TO_HOST_MBOX);
+
+	mutex_unlock(&priv->lock);
+
+	if (!ret) {
+		/* Verify response */
+		chksum = get_msg_chksum(result);
+		if (chksum != result[MSG_CHKSUM])
+			ret = -1;
+	}
+
+	if (!ret) {
+		switch (resp) {
+		case DCPU_RET_SUCCESS:
+			break;
+		case DCPU_RET_ERR_HEADER:
+		case DCPU_RET_ERR_INVAL:
+		case DCPU_RET_ERR_CHKSUM:
+		case DCPU_RET_ERR_OTHER:
+			ret = -1;
+			break;
+		}
+	}
+
+	return ret;
+}
+
+/* Ensure that the firmware file loaded meets all the requirements. */
+static int __verify_firmware(struct init_data *init,
+			     const struct firmware *fw)
+{
+	const struct dpfe_firmware_header *header = (void *)fw->data;
+	unsigned int dmem_size, imem_size, total_size;
+	bool is_big_endian = false;
+	const u32 *chksum;
+
+	if (header->magic == DPFE_BE_MAGIC)
+		is_big_endian = true;
+	else if (header->magic != DPFE_LE_MAGIC)
+		return ERR_INVALID_MAGIC;
+
+	if (is_big_endian) {
+		dmem_size = be32_to_cpu(header->dmem_size);
+		imem_size = be32_to_cpu(header->imem_size);
+	} else {
+		dmem_size = header->dmem_size;
+		imem_size = header->imem_size;
+	}
+
+	/* Data and instruction sections are 32 bit words. */
+	if ((dmem_size % sizeof(u32)) != 0 || (imem_size % sizeof(u32)) != 0)
+		return ERR_INVALID_SIZE;
+
+	/*
+	 * The header + the data section + the instruction section + the
+	 * checksum must be equal to the total firmware size.
+	 */
+	total_size = dmem_size + imem_size + sizeof(*header) + sizeof(*chksum);
+	if (total_size != fw->size)
+		return ERR_INVALID_SIZE;
+
+	/* The checksum comes at the very end. */
+	chksum = (void *)fw->data + sizeof(*header) + dmem_size + imem_size;
+
+	init->is_big_endian = is_big_endian;
+	init->dmem_len = dmem_size;
+	init->imem_len = imem_size;
+	init->chksum = (is_big_endian) ? be32_to_cpu(*chksum) : *chksum;
+
+	return 0;
+}
+
+/* Verify checksum by reading back the firmware from co-processor RAM. */
+static int __verify_fw_checksum(struct init_data *init,
+			     const struct dpfe_firmware_header *header,
+			     u32 checksum)
+{
+	u32 magic, sequence, version, sum;
+	u32 __iomem *dmem = init->dmem;
+	u32 __iomem *imem = init->imem;
+	unsigned int i;
+
+	if (init->is_big_endian) {
+		magic = be32_to_cpu(header->magic);
+		sequence = be32_to_cpu(header->sequence);
+		version = be32_to_cpu(header->version);
+	} else {
+		magic = header->magic;
+		sequence = header->sequence;
+		version = header->version;
+	}
+
+	sum = magic + sequence + version + init->dmem_len + init->imem_len;
+
+	for (i = 0; i < init->dmem_len / sizeof(u32); i++)
+		sum += dpfe_readl(dmem + i);
+
+	for (i = 0; i < init->imem_len / sizeof(u32); i++)
+		sum += dpfe_readl(imem + i);
+
+	return (sum == checksum) ? 0 : -1;
+}
+
+static int __write_firmware(u32 __iomem *mem, const u32 *fw,
+			    unsigned int size, bool is_big_endian)
+{
+	unsigned int i;
+
+	/* Convert size to 32-bit words. */
+	size /= sizeof(u32);
+
+	/* It is recommended to clear the firmware area first. */
+	for (i = 0; i < size; i++)
+		dpfe_writel(0, mem + i);
+
+	/* Now copy it. */
+	if (is_big_endian) {
+		for (i = 0; i < size; i++)
+			dpfe_writel(be32_to_cpu(fw[i]), mem + i);
+	} else {
+		for (i = 0; i < size; i++)
+			dpfe_writel(fw[i], mem + i);
+	}
+
+	return 0;
+}
+
+static int brcmstb_hwmon_download_firwmare(struct platform_device *pdev,
+					   struct init_data *init)
+{
+	const struct dpfe_firmware_header *header;
+	unsigned int dmem_size, imem_size;
+	struct device *dev = &pdev->dev;
+	bool is_big_endian = false;
+	struct private_data *priv;
+	const struct firmware *fw;
+	const u32 *dmem, *imem;
+	const void *fw_blob;
+	int ret;
+
+	ret = request_firmware(&fw, FIRMWARE_NAME, dev);
+	/* request_firmware() prints its own error messages. */
+	if (ret)
+		return ret;
+
+	priv = platform_get_drvdata(pdev);
+
+	ret = __verify_firmware(init, fw);
+	if (ret)
+		return -EFAULT;
+
+	__disable_dcpu(priv->regs);
+
+	is_big_endian = init->is_big_endian;
+	dmem_size = init->dmem_len;
+	imem_size = init->imem_len;
+
+	/* At the beginning of the firmware blob is a header. */
+	header = (struct dpfe_firmware_header *)fw->data;
+	/* Void pointer to the beginning of the actual firmware. */
+	fw_blob = fw->data + sizeof(*header);
+	/* IMEM comes right after the header. */
+	imem = fw_blob;
+	/* DMEM follows after IMEM. */
+	dmem = fw_blob + imem_size;
+
+	ret = __write_firmware(init->dmem, dmem, dmem_size, is_big_endian);
+	if (ret)
+		return ret;
+	ret = __write_firmware(init->imem, imem, imem_size, is_big_endian);
+	if (ret)
+		return ret;
+
+	ret = __verify_fw_checksum(init, header, init->chksum);
+	if (ret)
+		return ret;
+
+	__enable_dcpu(priv->regs);
+
+	return 0;
+}
+
+static ssize_t generic_show(unsigned int command, u32 response[],
+			    struct device *dev, char *buf)
+{
+	struct private_data *priv;
+	int ret;
+
+	priv = dev_get_drvdata(dev);
+
+	ret = __send_command(priv, command, response);
+	if (ret)
+		return sprintf(buf, "error %d\n", ret);
+
+	return 0;
+}
+
+static ssize_t show_info(struct device *dev, struct device_attribute *devattr,
+			 char *buf)
+{
+	u32 response[MSG_FIELD_MAX];
+	unsigned int info;
+	int ret;
+
+	ret = generic_show(DPFE_CMD_GET_INFO, response, dev, buf);
+	if (ret)
+		return ret;
+
+	info = response[MSG_ARG0];
+
+	return sprintf(buf, "%u.%u.%u.%u\n",
+		       (info >> 24) & 0xff,
+		       (info >> 16) & 0xff,
+		       (info >> 8) & 0xff,
+		       info & 0xff);
+}
+
+static ssize_t show_refresh(struct device *dev,
+			    struct device_attribute *devattr, char *buf)
+{
+	u32 response[MSG_FIELD_MAX];
+	void __iomem *info;
+	struct private_data *priv;
+	unsigned int offset;
+	u8 refresh, sr_abort, ppre, thermal_offs, tuf;
+	u32 mr4;
+	int ret;
+
+	ret = generic_show(DPFE_CMD_GET_REFRESH, response, dev, buf);
+	if (ret)
+		return ret;
+
+	priv = dev_get_drvdata(dev);
+	offset = response[MSG_ARG0];
+	info = priv->dmem + offset;
+
+	mr4 = dpfe_readl(info + DRAM_INFO_MR4) & DRAM_INFO_MASK;
+
+	refresh = (mr4 >> DRAM_MR4_REFRESH) & DRAM_MR4_REFRESH_MASK;
+	sr_abort = (mr4 >> DRAM_MR4_SR_ABORT) & DRAM_MR4_SR_ABORT_MASK;
+	ppre = (mr4 >> DRAM_MR4_PPRE) & DRAM_MR4_PPRE_MASK;
+	thermal_offs = (mr4 >> DRAM_MR4_TH_OFFS) & DRAM_MR4_TH_OFFS_MASK;
+	tuf = (mr4 >> DRAM_MR4_TUF) & DRAM_MR4_TUF_MASK;
+
+	return sprintf(buf, "%#x %#x %#x %#x %#x %#x %#x\n",
+		       dpfe_readl(info + DRAM_INFO_INTERVAL),
+		       refresh, sr_abort, ppre, thermal_offs, tuf,
+		       dpfe_readl(info + DRAM_INFO_ERROR));
+}
+
+static ssize_t store_refresh(struct device *dev, struct device_attribute *attr,
+			  const char *buf, size_t count)
+{
+	u32 response[MSG_FIELD_MAX];
+	struct private_data *priv;
+	void __iomem *info;
+	unsigned int offset;
+	unsigned long val;
+	int ret;
+
+	if (kstrtoul(buf, 0, &val) < 0)
+		return -EINVAL;
+
+	priv = dev_get_drvdata(dev);
+
+	ret = __send_command(priv, DPFE_CMD_GET_REFRESH, response);
+	if (ret)
+		return ret;
+
+	offset = response[MSG_ARG0];
+	info = priv->dmem + offset + DRAM_MR4_REFRESH;
+	dpfe_writel(val, info);
+
+	return count;
+}
+
+
+static ssize_t show_vendor(struct device *dev, struct device_attribute *devattr,
+			 char *buf)
+{
+	u32 response[MSG_FIELD_MAX];
+	struct private_data *priv;
+	void __iomem *info;
+	unsigned int offset;
+	int ret;
+
+	ret = generic_show(DPFE_CMD_GET_VENDOR, response, dev, buf);
+	if (ret)
+		return ret;
+
+	offset = response[MSG_ARG0];
+	priv = dev_get_drvdata(dev);
+	info = priv->dmem + offset;
+
+	return sprintf(buf, "%#x %#x %#x %#x %#x\n",
+		       dpfe_readl(info + DRAM_VENDOR_MR5) & DRAM_VENDOR_MASK,
+		       dpfe_readl(info + DRAM_VENDOR_MR6) & DRAM_VENDOR_MASK,
+		       dpfe_readl(info + DRAM_VENDOR_MR7) & DRAM_VENDOR_MASK,
+		       dpfe_readl(info + DRAM_VENDOR_MR8) & DRAM_VENDOR_MASK,
+		       dpfe_readl(info + DRAM_VENDOR_ERROR));
+}
+
+static SENSOR_DEVICE_ATTR(dpfe_info, 0444, show_info, NULL, 1000);
+static SENSOR_DEVICE_ATTR(dpfe_refresh, 0644, show_refresh, store_refresh,
+			  1000);
+static SENSOR_DEVICE_ATTR(dpfe_vendor, 0444, show_vendor, NULL, 1000);
+static struct attribute *dpfe_attrs[] = {
+	&sensor_dev_attr_dpfe_info.dev_attr.attr,
+	&sensor_dev_attr_dpfe_refresh.dev_attr.attr,
+	&sensor_dev_attr_dpfe_vendor.dev_attr.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(dpfe);
+
+static int brcmstb_hwmon_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device *hwmon_dev = NULL;
+	struct private_data *priv;
+	struct init_data init;
+	struct resource *res;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	mutex_init(&priv->lock);
+	platform_set_drvdata(pdev, priv);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	priv->regs = devm_ioremap_resource(dev, res);
+	if (IS_ERR(priv->regs)) {
+		dev_err(dev, "couldn't map DT entry brcm,dpfe-cpu\n");
+		return -ENODEV;
+	}
+
+	init.dmem = __map_region(DT_COMPAT_DMEM);
+	if (!init.dmem) {
+		dev_err(dev, "Couldn't map %s\n", DT_COMPAT_DMEM);
+		return -ENOENT;
+	}
+	init.imem = __map_region(DT_COMPAT_IMEM);
+	if (init.imem) {
+		ret = brcmstb_hwmon_download_firwmare(pdev, &init);
+	} else {
+		ret = -ENOENT;
+		dev_err(dev, "Couldn't map %s\n", DT_COMPAT_IMEM);
+	}
+
+	/* We don't need IMEM after initialization. */
+	iounmap(init.imem);
+
+	if (!ret) {
+		hwmon_dev = devm_hwmon_device_register_with_groups(dev,
+			"brcmstb_dpfe", priv, dpfe_groups);
+		if (IS_ERR(hwmon_dev))
+			ret = PTR_ERR(hwmon_dev);
+	}
+
+	if (ret) {
+		iounmap(init.dmem);
+		dev_err(dev, "failed to initialize -- error %d\n", ret);
+	} else {
+		priv->dmem = init.dmem;
+		dev_info(dev, "registered.\n");
+	}
+
+	return ret;
+}
+
+static int brcmstb_hwmon_remove(struct platform_device *pdev)
+{
+	struct private_data *priv;
+
+	priv = platform_get_drvdata(pdev);
+	iounmap(priv->dmem);
+
+	return 0;
+}
+
+static const struct of_device_id brcmstb_hwmon_of_match[] = {
+	{ .compatible = "brcm,dpfe-cpu", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, brcmstb_hwmon_of_match);
+
+static struct platform_driver brcmstb_hwmon_driver = {
+	.driver	= {
+		.name = DRVNAME,
+		.of_match_table = brcmstb_hwmon_of_match,
+	},
+	.probe = brcmstb_hwmon_probe,
+	.remove = brcmstb_hwmon_remove,
+};
+
+module_platform_driver(brcmstb_hwmon_driver);
+
+MODULE_AUTHOR("Markus Mayer <mmayer@broadcom.com>");
+MODULE_DESCRIPTION("BRCMSTB Hardware Monitoring");
+MODULE_LICENSE("GPL");
-- 
2.7.4

Re: [PATCH 2/2] hwmon: (brcmstb) Add driver for Broadcom STB DPFE

[Guenter Roeck]

Hi Markus,

On Tue, Apr 18, 2017 at 01:17:02PM -0700, Markus Mayer wrote:
> From: Markus Mayer <mmayer@broadcom.com>
> 
> This driver allows access to DRAM properties, such as the refresh rate,
> via the Broadcom STB DDR PHY Front End (DPFE). The refresh rate can be
> used as indirect indicator of the DRAM temperature.
> 
> The driver also allows setting of the sampling interval.
> 
> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
> ---
[ ... ]

> +
> +static SENSOR_DEVICE_ATTR(dpfe_info, 0444, show_info, NULL, 1000);
> +static SENSOR_DEVICE_ATTR(dpfe_refresh, 0644, show_refresh, store_refresh,
> +			  1000);
> +static SENSOR_DEVICE_ATTR(dpfe_vendor, 0444, show_vendor, NULL, 1000);
> +static struct attribute *dpfe_attrs[] = {
> +	&sensor_dev_attr_dpfe_info.dev_attr.attr,
> +	&sensor_dev_attr_dpfe_refresh.dev_attr.attr,
> +	&sensor_dev_attr_dpfe_vendor.dev_attr.attr,
> +	NULL
> +};
> +ATTRIBUTE_GROUPS(dpfe);
> +

There is not a single standard hwmon attribute. I don't know how
to classify this driver, and where it should reside, but it is not
a hwmon driver. 

Thanks,
Guenter

[PATCH 2/2] hwmon: (brcmstb) Add driver for Broadcom STB DPFE

[Guenter Roeck]

Hi Markus,

On Tue, Apr 18, 2017 at 01:17:02PM -0700, Markus Mayer wrote:
> From: Markus Mayer <mmayer@broadcom.com>
> 
> This driver allows access to DRAM properties, such as the refresh rate,
> via the Broadcom STB DDR PHY Front End (DPFE). The refresh rate can be
> used as indirect indicator of the DRAM temperature.
> 
> The driver also allows setting of the sampling interval.
> 
> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
> ---
[ ... ]

> +
> +static SENSOR_DEVICE_ATTR(dpfe_info, 0444, show_info, NULL, 1000);
> +static SENSOR_DEVICE_ATTR(dpfe_refresh, 0644, show_refresh, store_refresh,
> +			  1000);
> +static SENSOR_DEVICE_ATTR(dpfe_vendor, 0444, show_vendor, NULL, 1000);
> +static struct attribute *dpfe_attrs[] = {
> +	&sensor_dev_attr_dpfe_info.dev_attr.attr,
> +	&sensor_dev_attr_dpfe_refresh.dev_attr.attr,
> +	&sensor_dev_attr_dpfe_vendor.dev_attr.attr,
> +	NULL
> +};
> +ATTRIBUTE_GROUPS(dpfe);
> +

There is not a single standard hwmon attribute. I don't know how
to classify this driver, and where it should reside, but it is not
a hwmon driver. 

Thanks,
Guenter

Re: [PATCH 2/2] hwmon: (brcmstb) Add driver for Broadcom STB DPFE

[Florian Fainelli]

Hey Guenter,

On 04/18/2017 01:58 PM, Guenter Roeck wrote:
> Hi Markus,
> 
> On Tue, Apr 18, 2017 at 01:17:02PM -0700, Markus Mayer wrote:
>> From: Markus Mayer <mmayer@broadcom.com>
>>
>> This driver allows access to DRAM properties, such as the refresh rate,
>> via the Broadcom STB DDR PHY Front End (DPFE). The refresh rate can be
>> used as indirect indicator of the DRAM temperature.
>>
>> The driver also allows setting of the sampling interval.
>>
>> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
>> ---
> [ ... ]
> 
>> +
>> +static SENSOR_DEVICE_ATTR(dpfe_info, 0444, show_info, NULL, 1000);
>> +static SENSOR_DEVICE_ATTR(dpfe_refresh, 0644, show_refresh, store_refresh,
>> +			  1000);
>> +static SENSOR_DEVICE_ATTR(dpfe_vendor, 0444, show_vendor, NULL, 1000);
>> +static struct attribute *dpfe_attrs[] = {
>> +	&sensor_dev_attr_dpfe_info.dev_attr.attr,
>> +	&sensor_dev_attr_dpfe_refresh.dev_attr.attr,
>> +	&sensor_dev_attr_dpfe_vendor.dev_attr.attr,
>> +	NULL
>> +};
>> +ATTRIBUTE_GROUPS(dpfe);
>> +
> 
> There is not a single standard hwmon attribute. I don't know how
> to classify this driver, and where it should reside, but it is not
> a hwmon driver. 

This is a driver that talks to an embedded CPU running firmware which is
capable of giving various informations about the DRAM chip being
populated, including a temperature trend (hotter or cooler). We thought
initially we would be able to expose the actual temperature, but this in
turn required a lot more knowledge about the DRAM chip that we wish we
knew about. That is sort of where and why this driver was proposed for
hwmon.

Which subsystem do you think would be best for this driver drivers/misc/
or drivers/soc/bcm/brcmstb/ maybe?
-- 
Florian

[PATCH 2/2] hwmon: (brcmstb) Add driver for Broadcom STB DPFE

[Florian Fainelli]

Hey Guenter,

On 04/18/2017 01:58 PM, Guenter Roeck wrote:
> Hi Markus,
> 
> On Tue, Apr 18, 2017 at 01:17:02PM -0700, Markus Mayer wrote:
>> From: Markus Mayer <mmayer@broadcom.com>
>>
>> This driver allows access to DRAM properties, such as the refresh rate,
>> via the Broadcom STB DDR PHY Front End (DPFE). The refresh rate can be
>> used as indirect indicator of the DRAM temperature.
>>
>> The driver also allows setting of the sampling interval.
>>
>> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
>> ---
> [ ... ]
> 
>> +
>> +static SENSOR_DEVICE_ATTR(dpfe_info, 0444, show_info, NULL, 1000);
>> +static SENSOR_DEVICE_ATTR(dpfe_refresh, 0644, show_refresh, store_refresh,
>> +			  1000);
>> +static SENSOR_DEVICE_ATTR(dpfe_vendor, 0444, show_vendor, NULL, 1000);
>> +static struct attribute *dpfe_attrs[] = {
>> +	&sensor_dev_attr_dpfe_info.dev_attr.attr,
>> +	&sensor_dev_attr_dpfe_refresh.dev_attr.attr,
>> +	&sensor_dev_attr_dpfe_vendor.dev_attr.attr,
>> +	NULL
>> +};
>> +ATTRIBUTE_GROUPS(dpfe);
>> +
> 
> There is not a single standard hwmon attribute. I don't know how
> to classify this driver, and where it should reside, but it is not
> a hwmon driver. 

This is a driver that talks to an embedded CPU running firmware which is
capable of giving various informations about the DRAM chip being
populated, including a temperature trend (hotter or cooler). We thought
initially we would be able to expose the actual temperature, but this in
turn required a lot more knowledge about the DRAM chip that we wish we
knew about. That is sort of where and why this driver was proposed for
hwmon.

Which subsystem do you think would be best for this driver drivers/misc/
or drivers/soc/bcm/brcmstb/ maybe?
-- 
Florian

Re: [PATCH 2/2] hwmon: (brcmstb) Add driver for Broadcom STB DPFE

[Guenter Roeck]

Hi Florian,

On Tue, Apr 18, 2017 at 03:29:55PM -0700, Florian Fainelli wrote:
> Hey Guenter,
> 
> On 04/18/2017 01:58 PM, Guenter Roeck wrote:
> > Hi Markus,
> > 
> > On Tue, Apr 18, 2017 at 01:17:02PM -0700, Markus Mayer wrote:
> >> From: Markus Mayer <mmayer@broadcom.com>
> >>
> >> This driver allows access to DRAM properties, such as the refresh rate,
> >> via the Broadcom STB DDR PHY Front End (DPFE). The refresh rate can be
> >> used as indirect indicator of the DRAM temperature.
> >>
> >> The driver also allows setting of the sampling interval.
> >>
> >> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
> >> ---
> > [ ... ]
> > 
> >> +
> >> +static SENSOR_DEVICE_ATTR(dpfe_info, 0444, show_info, NULL, 1000);
> >> +static SENSOR_DEVICE_ATTR(dpfe_refresh, 0644, show_refresh, store_refresh,
> >> +			  1000);
> >> +static SENSOR_DEVICE_ATTR(dpfe_vendor, 0444, show_vendor, NULL, 1000);
> >> +static struct attribute *dpfe_attrs[] = {
> >> +	&sensor_dev_attr_dpfe_info.dev_attr.attr,
> >> +	&sensor_dev_attr_dpfe_refresh.dev_attr.attr,
> >> +	&sensor_dev_attr_dpfe_vendor.dev_attr.attr,
> >> +	NULL
> >> +};
> >> +ATTRIBUTE_GROUPS(dpfe);
> >> +
> > 
> > There is not a single standard hwmon attribute. I don't know how
> > to classify this driver, and where it should reside, but it is not
> > a hwmon driver. 
> 
> This is a driver that talks to an embedded CPU running firmware which is
> capable of giving various informations about the DRAM chip being
> populated, including a temperature trend (hotter or cooler). We thought
> initially we would be able to expose the actual temperature, but this in
> turn required a lot more knowledge about the DRAM chip that we wish we
> knew about. That is sort of where and why this driver was proposed for
> hwmon.
> 
> Which subsystem do you think would be best for this driver drivers/misc/
> or drivers/soc/bcm/brcmstb/ maybe?

Both should work. I would probably try misc first and let Greg tell me
which way to go ;-).

Thanks,
Guenter

[PATCH 2/2] hwmon: (brcmstb) Add driver for Broadcom STB DPFE

[Guenter Roeck]

Hi Florian,

On Tue, Apr 18, 2017 at 03:29:55PM -0700, Florian Fainelli wrote:
> Hey Guenter,
> 
> On 04/18/2017 01:58 PM, Guenter Roeck wrote:
> > Hi Markus,
> > 
> > On Tue, Apr 18, 2017 at 01:17:02PM -0700, Markus Mayer wrote:
> >> From: Markus Mayer <mmayer@broadcom.com>
> >>
> >> This driver allows access to DRAM properties, such as the refresh rate,
> >> via the Broadcom STB DDR PHY Front End (DPFE). The refresh rate can be
> >> used as indirect indicator of the DRAM temperature.
> >>
> >> The driver also allows setting of the sampling interval.
> >>
> >> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
> >> ---
> > [ ... ]
> > 
> >> +
> >> +static SENSOR_DEVICE_ATTR(dpfe_info, 0444, show_info, NULL, 1000);
> >> +static SENSOR_DEVICE_ATTR(dpfe_refresh, 0644, show_refresh, store_refresh,
> >> +			  1000);
> >> +static SENSOR_DEVICE_ATTR(dpfe_vendor, 0444, show_vendor, NULL, 1000);
> >> +static struct attribute *dpfe_attrs[] = {
> >> +	&sensor_dev_attr_dpfe_info.dev_attr.attr,
> >> +	&sensor_dev_attr_dpfe_refresh.dev_attr.attr,
> >> +	&sensor_dev_attr_dpfe_vendor.dev_attr.attr,
> >> +	NULL
> >> +};
> >> +ATTRIBUTE_GROUPS(dpfe);
> >> +
> > 
> > There is not a single standard hwmon attribute. I don't know how
> > to classify this driver, and where it should reside, but it is not
> > a hwmon driver. 
> 
> This is a driver that talks to an embedded CPU running firmware which is
> capable of giving various informations about the DRAM chip being
> populated, including a temperature trend (hotter or cooler). We thought
> initially we would be able to expose the actual temperature, but this in
> turn required a lot more knowledge about the DRAM chip that we wish we
> knew about. That is sort of where and why this driver was proposed for
> hwmon.
> 
> Which subsystem do you think would be best for this driver drivers/misc/
> or drivers/soc/bcm/brcmstb/ maybe?

Both should work. I would probably try misc first and let Greg tell me
which way to go ;-).

Thanks,
Guenter

Re: [PATCH 2/2] hwmon: (brcmstb) Add driver for Broadcom STB DPFE

[Guenter Roeck]

Hi Florian,

On Tue, Apr 18, 2017 at 03:29:55PM -0700, Florian Fainelli wrote:
> Hey Guenter,
> 
> On 04/18/2017 01:58 PM, Guenter Roeck wrote:
> > Hi Markus,
> > 
> > On Tue, Apr 18, 2017 at 01:17:02PM -0700, Markus Mayer wrote:
> >> From: Markus Mayer <mmayer-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> >>
> >> This driver allows access to DRAM properties, such as the refresh rate,
> >> via the Broadcom STB DDR PHY Front End (DPFE). The refresh rate can be
> >> used as indirect indicator of the DRAM temperature.
> >>
> >> The driver also allows setting of the sampling interval.
> >>
> >> Signed-off-by: Markus Mayer <mmayer-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> >> ---
> > [ ... ]
> > 
> >> +
> >> +static SENSOR_DEVICE_ATTR(dpfe_info, 0444, show_info, NULL, 1000);
> >> +static SENSOR_DEVICE_ATTR(dpfe_refresh, 0644, show_refresh, store_refresh,
> >> +			  1000);
> >> +static SENSOR_DEVICE_ATTR(dpfe_vendor, 0444, show_vendor, NULL, 1000);
> >> +static struct attribute *dpfe_attrs[] = {
> >> +	&sensor_dev_attr_dpfe_info.dev_attr.attr,
> >> +	&sensor_dev_attr_dpfe_refresh.dev_attr.attr,
> >> +	&sensor_dev_attr_dpfe_vendor.dev_attr.attr,
> >> +	NULL
> >> +};
> >> +ATTRIBUTE_GROUPS(dpfe);
> >> +
> > 
> > There is not a single standard hwmon attribute. I don't know how
> > to classify this driver, and where it should reside, but it is not
> > a hwmon driver. 
> 
> This is a driver that talks to an embedded CPU running firmware which is
> capable of giving various informations about the DRAM chip being
> populated, including a temperature trend (hotter or cooler). We thought
> initially we would be able to expose the actual temperature, but this in
> turn required a lot more knowledge about the DRAM chip that we wish we
> knew about. That is sort of where and why this driver was proposed for
> hwmon.
> 
> Which subsystem do you think would be best for this driver drivers/misc/
> or drivers/soc/bcm/brcmstb/ maybe?

Both should work. I would probably try misc first and let Greg tell me
which way to go ;-).

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

Re: [PATCH 2/2] hwmon: (brcmstb) Add driver for Broadcom STB DPFE

[Markus Mayer]

On 18 April 2017 at 15:47, Guenter Roeck <linux@roeck-us.net> wrote:
> Hi Florian,
>
> On Tue, Apr 18, 2017 at 03:29:55PM -0700, Florian Fainelli wrote:
>> Hey Guenter,
>>
>> On 04/18/2017 01:58 PM, Guenter Roeck wrote:
>> > Hi Markus,
>> >
>> > On Tue, Apr 18, 2017 at 01:17:02PM -0700, Markus Mayer wrote:
>> >> From: Markus Mayer <mmayer@broadcom.com>
>> >>
>> >> This driver allows access to DRAM properties, such as the refresh rate,
>> >> via the Broadcom STB DDR PHY Front End (DPFE). The refresh rate can be
>> >> used as indirect indicator of the DRAM temperature.
>> >>
>> >> The driver also allows setting of the sampling interval.
>> >>
>> >> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
>> >> ---
>> > [ ... ]
>> >
>> >> +
>> >> +static SENSOR_DEVICE_ATTR(dpfe_info, 0444, show_info, NULL, 1000);
>> >> +static SENSOR_DEVICE_ATTR(dpfe_refresh, 0644, show_refresh, store_refresh,
>> >> +                    1000);
>> >> +static SENSOR_DEVICE_ATTR(dpfe_vendor, 0444, show_vendor, NULL, 1000);
>> >> +static struct attribute *dpfe_attrs[] = {
>> >> +  &sensor_dev_attr_dpfe_info.dev_attr.attr,
>> >> +  &sensor_dev_attr_dpfe_refresh.dev_attr.attr,
>> >> +  &sensor_dev_attr_dpfe_vendor.dev_attr.attr,
>> >> +  NULL
>> >> +};
>> >> +ATTRIBUTE_GROUPS(dpfe);
>> >> +
>> >
>> > There is not a single standard hwmon attribute. I don't know how
>> > to classify this driver, and where it should reside, but it is not
>> > a hwmon driver.
>>
>> This is a driver that talks to an embedded CPU running firmware which is
>> capable of giving various informations about the DRAM chip being
>> populated, including a temperature trend (hotter or cooler). We thought
>> initially we would be able to expose the actual temperature, but this in
>> turn required a lot more knowledge about the DRAM chip that we wish we
>> knew about. That is sort of where and why this driver was proposed for
>> hwmon.
>>
>> Which subsystem do you think would be best for this driver drivers/misc/
>> or drivers/soc/bcm/brcmstb/ maybe?
>
> Both should work. I would probably try misc first and let Greg tell me
> which way to go ;-).

Thanks for the tip. As Florian said, it was not the idea to submit a
completely non-standard hwmon driver. I was sure I would be able to
report at least some standard hwmon data also.

Regards,
-Markus

[PATCH 2/2] hwmon: (brcmstb) Add driver for Broadcom STB DPFE

[Markus Mayer]

On 18 April 2017 at 15:47, Guenter Roeck <linux@roeck-us.net> wrote:
> Hi Florian,
>
> On Tue, Apr 18, 2017 at 03:29:55PM -0700, Florian Fainelli wrote:
>> Hey Guenter,
>>
>> On 04/18/2017 01:58 PM, Guenter Roeck wrote:
>> > Hi Markus,
>> >
>> > On Tue, Apr 18, 2017 at 01:17:02PM -0700, Markus Mayer wrote:
>> >> From: Markus Mayer <mmayer@broadcom.com>
>> >>
>> >> This driver allows access to DRAM properties, such as the refresh rate,
>> >> via the Broadcom STB DDR PHY Front End (DPFE). The refresh rate can be
>> >> used as indirect indicator of the DRAM temperature.
>> >>
>> >> The driver also allows setting of the sampling interval.
>> >>
>> >> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
>> >> ---
>> > [ ... ]
>> >
>> >> +
>> >> +static SENSOR_DEVICE_ATTR(dpfe_info, 0444, show_info, NULL, 1000);
>> >> +static SENSOR_DEVICE_ATTR(dpfe_refresh, 0644, show_refresh, store_refresh,
>> >> +                    1000);
>> >> +static SENSOR_DEVICE_ATTR(dpfe_vendor, 0444, show_vendor, NULL, 1000);
>> >> +static struct attribute *dpfe_attrs[] = {
>> >> +  &sensor_dev_attr_dpfe_info.dev_attr.attr,
>> >> +  &sensor_dev_attr_dpfe_refresh.dev_attr.attr,
>> >> +  &sensor_dev_attr_dpfe_vendor.dev_attr.attr,
>> >> +  NULL
>> >> +};
>> >> +ATTRIBUTE_GROUPS(dpfe);
>> >> +
>> >
>> > There is not a single standard hwmon attribute. I don't know how
>> > to classify this driver, and where it should reside, but it is not
>> > a hwmon driver.
>>
>> This is a driver that talks to an embedded CPU running firmware which is
>> capable of giving various informations about the DRAM chip being
>> populated, including a temperature trend (hotter or cooler). We thought
>> initially we would be able to expose the actual temperature, but this in
>> turn required a lot more knowledge about the DRAM chip that we wish we
>> knew about. That is sort of where and why this driver was proposed for
>> hwmon.
>>
>> Which subsystem do you think would be best for this driver drivers/misc/
>> or drivers/soc/bcm/brcmstb/ maybe?
>
> Both should work. I would probably try misc first and let Greg tell me
> which way to go ;-).

Thanks for the tip. As Florian said, it was not the idea to submit a
completely non-standard hwmon driver. I was sure I would be able to
report at least some standard hwmon data also.

Regards,
-Markus

Re: [PATCH 2/2] hwmon: (brcmstb) Add driver for Broadcom STB DPFE

[Markus Mayer]

On 18 April 2017 at 15:47, Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org> wrote:
> Hi Florian,
>
> On Tue, Apr 18, 2017 at 03:29:55PM -0700, Florian Fainelli wrote:
>> Hey Guenter,
>>
>> On 04/18/2017 01:58 PM, Guenter Roeck wrote:
>> > Hi Markus,
>> >
>> > On Tue, Apr 18, 2017 at 01:17:02PM -0700, Markus Mayer wrote:
>> >> From: Markus Mayer <mmayer-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>> >>
>> >> This driver allows access to DRAM properties, such as the refresh rate,
>> >> via the Broadcom STB DDR PHY Front End (DPFE). The refresh rate can be
>> >> used as indirect indicator of the DRAM temperature.
>> >>
>> >> The driver also allows setting of the sampling interval.
>> >>
>> >> Signed-off-by: Markus Mayer <mmayer-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>> >> ---
>> > [ ... ]
>> >
>> >> +
>> >> +static SENSOR_DEVICE_ATTR(dpfe_info, 0444, show_info, NULL, 1000);
>> >> +static SENSOR_DEVICE_ATTR(dpfe_refresh, 0644, show_refresh, store_refresh,
>> >> +                    1000);
>> >> +static SENSOR_DEVICE_ATTR(dpfe_vendor, 0444, show_vendor, NULL, 1000);
>> >> +static struct attribute *dpfe_attrs[] = {
>> >> +  &sensor_dev_attr_dpfe_info.dev_attr.attr,
>> >> +  &sensor_dev_attr_dpfe_refresh.dev_attr.attr,
>> >> +  &sensor_dev_attr_dpfe_vendor.dev_attr.attr,
>> >> +  NULL
>> >> +};
>> >> +ATTRIBUTE_GROUPS(dpfe);
>> >> +
>> >
>> > There is not a single standard hwmon attribute. I don't know how
>> > to classify this driver, and where it should reside, but it is not
>> > a hwmon driver.
>>
>> This is a driver that talks to an embedded CPU running firmware which is
>> capable of giving various informations about the DRAM chip being
>> populated, including a temperature trend (hotter or cooler). We thought
>> initially we would be able to expose the actual temperature, but this in
>> turn required a lot more knowledge about the DRAM chip that we wish we
>> knew about. That is sort of where and why this driver was proposed for
>> hwmon.
>>
>> Which subsystem do you think would be best for this driver drivers/misc/
>> or drivers/soc/bcm/brcmstb/ maybe?
>
> Both should work. I would probably try misc first and let Greg tell me
> which way to go ;-).

Thanks for the tip. As Florian said, it was not the idea to submit a
completely non-standard hwmon driver. I was sure I would be able to
report at least some standard hwmon data also.

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

Re: [PATCH 2/2] hwmon: (brcmstb) Add driver for Broadcom STB DPFE

[Florian Fainelli]

On 04/18/2017 05:15 PM, Markus Mayer wrote:
> On 18 April 2017 at 15:47, Guenter Roeck <linux@roeck-us.net> wrote:
>> Hi Florian,
>>
>> On Tue, Apr 18, 2017 at 03:29:55PM -0700, Florian Fainelli wrote:
>>> Hey Guenter,
>>>
>>> On 04/18/2017 01:58 PM, Guenter Roeck wrote:
>>>> Hi Markus,
>>>>
>>>> On Tue, Apr 18, 2017 at 01:17:02PM -0700, Markus Mayer wrote:
>>>>> From: Markus Mayer <mmayer@broadcom.com>
>>>>>
>>>>> This driver allows access to DRAM properties, such as the refresh rate,
>>>>> via the Broadcom STB DDR PHY Front End (DPFE). The refresh rate can be
>>>>> used as indirect indicator of the DRAM temperature.
>>>>>
>>>>> The driver also allows setting of the sampling interval.
>>>>>
>>>>> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
>>>>> ---
>>>> [ ... ]
>>>>
>>>>> +
>>>>> +static SENSOR_DEVICE_ATTR(dpfe_info, 0444, show_info, NULL, 1000);
>>>>> +static SENSOR_DEVICE_ATTR(dpfe_refresh, 0644, show_refresh, store_refresh,
>>>>> +                    1000);
>>>>> +static SENSOR_DEVICE_ATTR(dpfe_vendor, 0444, show_vendor, NULL, 1000);
>>>>> +static struct attribute *dpfe_attrs[] = {
>>>>> +  &sensor_dev_attr_dpfe_info.dev_attr.attr,
>>>>> +  &sensor_dev_attr_dpfe_refresh.dev_attr.attr,
>>>>> +  &sensor_dev_attr_dpfe_vendor.dev_attr.attr,
>>>>> +  NULL
>>>>> +};
>>>>> +ATTRIBUTE_GROUPS(dpfe);
>>>>> +
>>>>
>>>> There is not a single standard hwmon attribute. I don't know how
>>>> to classify this driver, and where it should reside, but it is not
>>>> a hwmon driver.
>>>
>>> This is a driver that talks to an embedded CPU running firmware which is
>>> capable of giving various informations about the DRAM chip being
>>> populated, including a temperature trend (hotter or cooler). We thought
>>> initially we would be able to expose the actual temperature, but this in
>>> turn required a lot more knowledge about the DRAM chip that we wish we
>>> knew about. That is sort of where and why this driver was proposed for
>>> hwmon.
>>>
>>> Which subsystem do you think would be best for this driver drivers/misc/
>>> or drivers/soc/bcm/brcmstb/ maybe?
>>
>> Both should work. I would probably try misc first and let Greg tell me
>> which way to go ;-).
> 
> Thanks for the tip. As Florian said, it was not the idea to submit a
> completely non-standard hwmon driver. I was sure I would be able to
> report at least some standard hwmon data also.

Good thing with drivers/soc/bcm/brcmstb/ is that I can take such patches
directly through ARM SoC pull requests, and since this is inherently
Broadcom STB specific, that sounds like the most ideal location IMHO.
-- 
Florian

[PATCH 2/2] hwmon: (brcmstb) Add driver for Broadcom STB DPFE

[Florian Fainelli]

On 04/18/2017 05:15 PM, Markus Mayer wrote:
> On 18 April 2017 at 15:47, Guenter Roeck <linux@roeck-us.net> wrote:
>> Hi Florian,
>>
>> On Tue, Apr 18, 2017 at 03:29:55PM -0700, Florian Fainelli wrote:
>>> Hey Guenter,
>>>
>>> On 04/18/2017 01:58 PM, Guenter Roeck wrote:
>>>> Hi Markus,
>>>>
>>>> On Tue, Apr 18, 2017 at 01:17:02PM -0700, Markus Mayer wrote:
>>>>> From: Markus Mayer <mmayer@broadcom.com>
>>>>>
>>>>> This driver allows access to DRAM properties, such as the refresh rate,
>>>>> via the Broadcom STB DDR PHY Front End (DPFE). The refresh rate can be
>>>>> used as indirect indicator of the DRAM temperature.
>>>>>
>>>>> The driver also allows setting of the sampling interval.
>>>>>
>>>>> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
>>>>> ---
>>>> [ ... ]
>>>>
>>>>> +
>>>>> +static SENSOR_DEVICE_ATTR(dpfe_info, 0444, show_info, NULL, 1000);
>>>>> +static SENSOR_DEVICE_ATTR(dpfe_refresh, 0644, show_refresh, store_refresh,
>>>>> +                    1000);
>>>>> +static SENSOR_DEVICE_ATTR(dpfe_vendor, 0444, show_vendor, NULL, 1000);
>>>>> +static struct attribute *dpfe_attrs[] = {
>>>>> +  &sensor_dev_attr_dpfe_info.dev_attr.attr,
>>>>> +  &sensor_dev_attr_dpfe_refresh.dev_attr.attr,
>>>>> +  &sensor_dev_attr_dpfe_vendor.dev_attr.attr,
>>>>> +  NULL
>>>>> +};
>>>>> +ATTRIBUTE_GROUPS(dpfe);
>>>>> +
>>>>
>>>> There is not a single standard hwmon attribute. I don't know how
>>>> to classify this driver, and where it should reside, but it is not
>>>> a hwmon driver.
>>>
>>> This is a driver that talks to an embedded CPU running firmware which is
>>> capable of giving various informations about the DRAM chip being
>>> populated, including a temperature trend (hotter or cooler). We thought
>>> initially we would be able to expose the actual temperature, but this in
>>> turn required a lot more knowledge about the DRAM chip that we wish we
>>> knew about. That is sort of where and why this driver was proposed for
>>> hwmon.
>>>
>>> Which subsystem do you think would be best for this driver drivers/misc/
>>> or drivers/soc/bcm/brcmstb/ maybe?
>>
>> Both should work. I would probably try misc first and let Greg tell me
>> which way to go ;-).
> 
> Thanks for the tip. As Florian said, it was not the idea to submit a
> completely non-standard hwmon driver. I was sure I would be able to
> report at least some standard hwmon data also.

Good thing with drivers/soc/bcm/brcmstb/ is that I can take such patches
directly through ARM SoC pull requests, and since this is inherently
Broadcom STB specific, that sounds like the most ideal location IMHO.
-- 
Florian