| Message ID | 1440757911-9120-3-git-send-email-lee.jones@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Fri, Aug 28, 2015 at 5:31 AM, Lee Jones <lee.jones@linaro.org> wrote: > Signed-off-by: Ludovic Barre <ludovic.barre@st.com> > Signed-off-by: Lee Jones <lee.jones@linaro.org> > --- > .../devicetree/bindings/remoteproc/st-rproc.txt | 35 ++++++++++++++++++++++ > 1 file changed, 35 insertions(+) > create mode 100644 Documentation/devicetree/bindings/remoteproc/st-rproc.txt > > diff --git a/Documentation/devicetree/bindings/remoteproc/st-rproc.txt b/Documentation/devicetree/bindings/remoteproc/st-rproc.txt > new file mode 100644 > index 0000000..fbd7d78 > --- /dev/null > +++ b/Documentation/devicetree/bindings/remoteproc/st-rproc.txt > @@ -0,0 +1,35 @@ > +STMicroelectronics Remote Processor "remote processor" is what STM datasheets call them? This seems like Linux naming is leaking in here. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Lee, On Fri, 28 Aug 2015, Lee Jones wrote: > Signed-off-by: Ludovic Barre <ludovic.barre@st.com> > Signed-off-by: Lee Jones <lee.jones@linaro.org> > --- > .../devicetree/bindings/remoteproc/st-rproc.txt | 35 ++++++++++++++++++++++ > 1 file changed, 35 insertions(+) > create mode 100644 Documentation/devicetree/bindings/remoteproc/st-rproc.txt The patch documening the DT bindings should be ordered before the patch which adds the DT node to aid reviewing. > > diff --git a/Documentation/devicetree/bindings/remoteproc/st-rproc.txt b/Documentation/devicetree/bindings/remoteproc/st-rproc.txt > new file mode 100644 > index 0000000..fbd7d78 > --- /dev/null > +++ b/Documentation/devicetree/bindings/remoteproc/st-rproc.txt > @@ -0,0 +1,35 @@ > +STMicroelectronics Remote Processor > +----------------------------------- > + > +This binding provides support for adjunct processors found on ST SoCs. > + > +The remote processors can be controlled from the bootloader or the primary OS. > +If the bootloader starts a remote processor processor the primary OS must detect > +its state and act accordingly. > + > +Required properties: > +- compatible Should be one of: > + "st,st231-rproc" > + "st,st40-rproc" st40-proc isn't used anywhere. The stih407 doesn't have a ST40 copro, and looking in the vendor tree remoteproc support isn't present for stih415/6 which are the the only upstream SoC's to have a ST40 co-pro. So I think st40-rproc support can be removed. > +- reg Size and length of reserved co-processor memory > +- resets Reset lines (See: ../reset/reset.txt) > +- reset-names Must be "sw_reset" and "pwr_reset" pwr_reset isn't used by any of the st231 co-processors. It seems to be related to ST40 support which I don't think is required upstream. Removing it would make the driver a fair bit smaller. > +- clocks Clock for co-processor (See: ../clock/clock-bindings.txt) > +- clock-names Must be "rproc_clk" I can't see any co-pro which uses more than one clock, so clock-names looks superflous. > +- clock-frequency Clock frequency to set co-processor at if the bootloader > + hasn't already done so > +- st,syscfg-boot The register that holds the boot vector for the co-processor I would prefer to see this binding match how most other sti drivers reference syscfg registers which is: - st,syscfg = <&syscfg_core 0xf4>; Description: phandle of sysconfig bank plus integer array containing register offsets. It also means it is easily extendable if more than one syscfg register is required in the future to boot a co-pro. regards, Peter. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On Tue, 01 Sep 2015, Peter Griffin wrote: > Hi Lee, > > On Fri, 28 Aug 2015, Lee Jones wrote: > > > Signed-off-by: Ludovic Barre <ludovic.barre@st.com> > > Signed-off-by: Lee Jones <lee.jones@linaro.org> > > --- > > .../devicetree/bindings/remoteproc/st-rproc.txt | 35 ++++++++++++++++++++++ > > 1 file changed, 35 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/remoteproc/st-rproc.txt > > The patch documening the DT bindings should be ordered before the patch which adds > the DT node to aid reviewing. > > > > diff --git a/Documentation/devicetree/bindings/remoteproc/st-rproc.txt b/Documentation/devicetree/bindings/remoteproc/st-rproc.txt > > new file mode 100644 > > index 0000000..fbd7d78 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/remoteproc/st-rproc.txt > > @@ -0,0 +1,35 @@ > > +STMicroelectronics Remote Processor > > +----------------------------------- > > + > > +This binding provides support for adjunct processors found on ST SoCs. > > + > > +The remote processors can be controlled from the bootloader or the primary OS. > > +If the bootloader starts a remote processor processor the primary OS must detect > > +its state and act accordingly. > > + > > +Required properties: > > +- compatible Should be one of: > > + "st,st231-rproc" > > + "st,st40-rproc" > > st40-proc isn't used anywhere. The stih407 doesn't have a ST40 copro, and > looking in the vendor tree remoteproc support isn't present for stih415/6 which > are the the only upstream SoC's to have a ST40 co-pro. > > So I think st40-rproc support can be removed. > > > +- reg Size and length of reserved co-processor memory > > +- resets Reset lines (See: ../reset/reset.txt) > > +- reset-names Must be "sw_reset" and "pwr_reset" > > pwr_reset isn't used by any of the st231 co-processors. It seems to > be related to ST40 support which I don't think is required upstream. > Removing it would make the driver a fair bit smaller. > > > +- clocks Clock for co-processor (See: ../clock/clock-bindings.txt) > > +- clock-names Must be "rproc_clk" > > I can't see any co-pro which uses more than one clock, so clock-names looks > superflous. > > > +- clock-frequency Clock frequency to set co-processor at if the bootloader > > + hasn't already done so > > +- st,syscfg-boot The register that holds the boot vector for the co-processor > > I would prefer to see this binding match how most other sti drivers reference syscfg > registers which is: - > > st,syscfg = <&syscfg_core 0xf4>; > > Description: phandle of sysconfig bank plus integer array containing register offsets. > > It also means it is easily extendable if more than one syscfg register is > required in the future to boot a co-pro. Ack. Good points, will fix.
On Mon, 31 Aug 2015, Rob Herring wrote: > On Fri, Aug 28, 2015 at 5:31 AM, Lee Jones <lee.jones@linaro.org> wrote: > > Signed-off-by: Ludovic Barre <ludovic.barre@st.com> > > Signed-off-by: Lee Jones <lee.jones@linaro.org> > > --- > > .../devicetree/bindings/remoteproc/st-rproc.txt | 35 ++++++++++++++++++++++ > > 1 file changed, 35 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/remoteproc/st-rproc.txt > > > > diff --git a/Documentation/devicetree/bindings/remoteproc/st-rproc.txt b/Documentation/devicetree/bindings/remoteproc/st-rproc.txt > > new file mode 100644 > > index 0000000..fbd7d78 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/remoteproc/st-rproc.txt > > @@ -0,0 +1,35 @@ > > +STMicroelectronics Remote Processor > > "remote processor" is what STM datasheets call them? This seems like > Linux naming is leaking in here. That's what this directory is called and the other binding within it.
On Tue, Sep 1, 2015 at 5:41 AM, Lee Jones <lee.jones@linaro.org> wrote: > On Mon, 31 Aug 2015, Rob Herring wrote: > >> On Fri, Aug 28, 2015 at 5:31 AM, Lee Jones <lee.jones@linaro.org> wrote: >> > Signed-off-by: Ludovic Barre <ludovic.barre@st.com> >> > Signed-off-by: Lee Jones <lee.jones@linaro.org> >> > --- >> > .../devicetree/bindings/remoteproc/st-rproc.txt | 35 ++++++++++++++++++++++ >> > 1 file changed, 35 insertions(+) >> > create mode 100644 Documentation/devicetree/bindings/remoteproc/st-rproc.txt >> > >> > diff --git a/Documentation/devicetree/bindings/remoteproc/st-rproc.txt b/Documentation/devicetree/bindings/remoteproc/st-rproc.txt >> > new file mode 100644 >> > index 0000000..fbd7d78 >> > --- /dev/null >> > +++ b/Documentation/devicetree/bindings/remoteproc/st-rproc.txt >> > @@ -0,0 +1,35 @@ >> > +STMicroelectronics Remote Processor >> >> "remote processor" is what STM datasheets call them? This seems like >> Linux naming is leaking in here. > > That's what this directory is called and the other binding within it. That doesn't make it right or answer my question. Certainly our directory structure too closely mirrors the kernel. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Keeping interested parties in the loop. Peter and I had a discussion with ST. Here is the result. On Tue, 01 Sep 2015, Peter Griffin wrote: > On Fri, 28 Aug 2015, Lee Jones wrote: > > > Signed-off-by: Ludovic Barre <ludovic.barre@st.com> > > Signed-off-by: Lee Jones <lee.jones@linaro.org> > > --- > > .../devicetree/bindings/remoteproc/st-rproc.txt | 35 ++++++++++++++++++++++ > > 1 file changed, 35 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/remoteproc/st-rproc.txt > > The patch documening the DT bindings should be ordered before the patch which adds > the DT node to aid reviewing. I've always thought this was a silly rule, but I will re-order nonetheless. > > diff --git a/Documentation/devicetree/bindings/remoteproc/st-rproc.txt b/Documentation/devicetree/bindings/remoteproc/st-rproc.txt > > new file mode 100644 > > index 0000000..fbd7d78 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/remoteproc/st-rproc.txt > > @@ -0,0 +1,35 @@ > > +STMicroelectronics Remote Processor > > +----------------------------------- > > + > > +This binding provides support for adjunct processors found on ST SoCs. > > + > > +The remote processors can be controlled from the bootloader or the primary OS. > > +If the bootloader starts a remote processor processor the primary OS must detect > > +its state and act accordingly. > > + > > +Required properties: > > +- compatible Should be one of: > > + "st,st231-rproc" > > + "st,st40-rproc" > > st40-proc isn't used anywhere. The stih407 doesn't have a ST40 copro, and > looking in the vendor tree remoteproc support isn't present for stih415/6 which > are the the only upstream SoC's to have a ST40 co-pro. > > So I think st40-rproc support can be removed. Agreed that the current state of platforms present in Mainline do not warrant support for more than one type of co-processor; however, platforms exist which are either not going upstream, or aren't upstreamable *yet*, which will require support for a) different types of co-processors and/or b) the capability to configure which reset lines, if any, are present. I'm keen to keep this option for two reasons. Firstly, it allows ST to use the driver in Mainline for existing platforms and secondly, this driver will not have to be heavily modified when new platforms are added. > > +- reg Size and length of reserved co-processor memory > > +- resets Reset lines (See: ../reset/reset.txt) > > +- reset-names Must be "sw_reset" and "pwr_reset" > > pwr_reset isn't used by any of the st231 co-processors. It seems to > be related to ST40 support which I don't think is required upstream. > Removing it would make the driver a fair bit smaller. > > > +- clocks Clock for co-processor (See: ../clock/clock-bindings.txt) > > +- clock-names Must be "rproc_clk" > > I can't see any co-pro which uses more than one clock, so clock-names looks > superflous. Sounds reasonable. I will remove it. > > +- clock-frequency Clock frequency to set co-processor at if the bootloader > > + hasn't already done so > > +- st,syscfg-boot The register that holds the boot vector for the co-processor > > I would prefer to see this binding match how most other sti drivers reference syscfg > registers which is: - > > st,syscfg = <&syscfg_core 0xf4>; Agreed. Will change.
diff --git a/Documentation/devicetree/bindings/remoteproc/st-rproc.txt b/Documentation/devicetree/bindings/remoteproc/st-rproc.txt new file mode 100644 index 0000000..fbd7d78 --- /dev/null +++ b/Documentation/devicetree/bindings/remoteproc/st-rproc.txt @@ -0,0 +1,35 @@ +STMicroelectronics Remote Processor +----------------------------------- + +This binding provides support for adjunct processors found on ST SoCs. + +The remote processors can be controlled from the bootloader or the primary OS. +If the bootloader starts a remote processor processor the primary OS must detect +its state and act accordingly. + +Required properties: +- compatible Should be one of: + "st,st231-rproc" + "st,st40-rproc" +- reg Size and length of reserved co-processor memory +- resets Reset lines (See: ../reset/reset.txt) +- reset-names Must be "sw_reset" and "pwr_reset" +- clocks Clock for co-processor (See: ../clock/clock-bindings.txt) +- clock-names Must be "rproc_clk" +- clock-frequency Clock frequency to set co-processor at if the bootloader + hasn't already done so +- st,syscfg-boot The register that holds the boot vector for the co-processor + +Example: + + st231-audio@bae00000 { + compatible = "st,st231-rproc"; + reg = <0xbae00000 0x01000000>; + reg-names = "rproc_mem"; + resets = <&softreset STIH407_ST231_AUD_SOFTRESET>; + reset-names = "sw_reset"; + clocks = <&clk_s_c0_flexgen CLK_ST231_AUD_0>; + clock-names = "rproc_clk"; + clock-frequency = <600000000>; + st,syscfg-boot = <&syscfg_core 0x228>; + };