Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1630928 > unrolled thread
| Started by | Markus Mayer <markus.mayer@broadcom.com> |
|---|---|
| First post | 2017-04-25 21:30 +0200 |
| Last post | 2017-05-04 00:30 +0200 |
| Articles | 5 — 3 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH 1/2] dt/bindings: Add bindings for Broadcom STB DRAM Sensors Markus Mayer <markus.mayer@broadcom.com> - 2017-04-25 21:30 +0200
Re: [PATCH 1/2] dt/bindings: Add bindings for Broadcom STB DRAM Sensors Markus Mayer <markus.mayer@broadcom.com> - 2017-04-27 20:30 +0200
Re: [PATCH 1/2] dt/bindings: Add bindings for Broadcom STB DRAM Sensors Rob Herring <robh@kernel.org> - 2017-04-28 00:00 +0200
Re: [PATCH 1/2] dt/bindings: Add bindings for Broadcom STB DRAM Sensors Florian Fainelli <f.fainelli@gmail.com> - 2017-04-28 00:10 +0200
Re: [PATCH 1/2] dt/bindings: Add bindings for Broadcom STB DRAM Sensors Markus Mayer <markus.mayer@broadcom.com> - 2017-05-04 00:30 +0200
| From | Markus Mayer <markus.mayer@broadcom.com> |
|---|---|
| Date | 2017-04-25 21:30 +0200 |
| Subject | Re: [PATCH 1/2] dt/bindings: Add bindings for Broadcom STB DRAM Sensors |
| Message-ID | <tAejU-o2-13@gated-at.bofh.it> |
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
>
[toc] | [next] | [standalone]
| From | Markus Mayer <markus.mayer@broadcom.com> |
|---|---|
| Date | 2017-04-27 20:30 +0200 |
| Message-ID | <tAWkX-4nM-25@gated-at.bofh.it> |
| In reply to | #1630928 |
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
>>
[toc] | [prev] | [next] | [standalone]
| From | Rob Herring <robh@kernel.org> |
|---|---|
| Date | 2017-04-28 00:00 +0200 |
| Subject | Re: [PATCH 1/2] dt/bindings: Add bindings for Broadcom STB DRAM Sensors |
| Message-ID | <tAZCa-6zp-25@gated-at.bofh.it> |
| In reply to | #1632315 |
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
[toc] | [prev] | [next] | [standalone]
| From | Florian Fainelli <f.fainelli@gmail.com> |
|---|---|
| Date | 2017-04-28 00:10 +0200 |
| Subject | Re: [PATCH 1/2] dt/bindings: Add bindings for Broadcom STB DRAM Sensors |
| Message-ID | <tAZLQ-6S4-13@gated-at.bofh.it> |
| In reply to | #1632397 |
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
[toc] | [prev] | [next] | [standalone]
| From | Markus Mayer <markus.mayer@broadcom.com> |
|---|---|
| Date | 2017-05-04 00:30 +0200 |
| Message-ID | <tDaWu-34r-7@gated-at.bofh.it> |
| In reply to | #1632403 |
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
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web