| Message ID | 20170418201702.57019-2-code@mmayer.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 >
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 >>
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
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?
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
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>; + };