| Message ID | 1548138816-1149-2-git-send-email-alokc@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add interconnect support for GENI QUPs | expand |
+Mark Brown On 2019-01-22 12:03, Alok Chauhan wrote: > Add documentation for the interconnect and interconnect-names bindings > for the GENI QUP as detailed by bindings/interconnect/interconnect.txt. > > Signed-off-by: Alok Chauhan <alokc@codeaurora.org> > --- > Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt | 10 > ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git > a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt > b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt > index dab7ca9..44d7e02 100644 > --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt > @@ -17,6 +17,12 @@ Required properties if child node exists: > - #address-cells: Must be <1> for Serial Engine Address > - #size-cells: Must be <1> for Serial Engine Address Size > - ranges: Must be present > +- interconnects: phandle to a interconnect provider. Please refer > + ../interconnect/interconnect.txt for details. > + Must be 2 paths corresponding to 2 AXI ports. > +- interconnect-names: Port names to differentiate between the > + 2 interconnect paths defined with interconnect > + specifier. > > Properties for children: > > @@ -67,6 +73,10 @@ Example: > #size-cells = <1>; > ranges; > > + interconnects = <&qnoc 11 &qnoc 512>, > + <&qnoc 0 &qnoc 543>; > + interconnect-names = "qup-memory", "qup-config"; > + > i2c0: i2c@a94000 { > compatible = "qcom,geni-i2c"; > reg = <0xa94000 0x4000>;
On Mon 21 Jan 22:33 PST 2019, Alok Chauhan wrote: > Add documentation for the interconnect and interconnect-names bindings > for the GENI QUP as detailed by bindings/interconnect/interconnect.txt. > > Signed-off-by: Alok Chauhan <alokc@codeaurora.org> > --- > Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt > index dab7ca9..44d7e02 100644 > --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt > @@ -17,6 +17,12 @@ Required properties if child node exists: > - #address-cells: Must be <1> for Serial Engine Address > - #size-cells: Must be <1> for Serial Engine Address Size > - ranges: Must be present > +- interconnects: phandle to a interconnect provider. Please refer > + ../interconnect/interconnect.txt for details. > + Must be 2 paths corresponding to 2 AXI ports. > +- interconnect-names: Port names to differentiate between the s/Port names/Path names/ > + 2 interconnect paths defined with interconnect > + specifier. These two names are significant in that they must match what the driver expects, hence you must actually specify them here. And as the scope of these strings are local to the QUP node you can omit "qup" from them, so make them "memory" and "config" (or perhaps iface, to match the clock naming?). Regards, Bjorn > > Properties for children: > > @@ -67,6 +73,10 @@ Example: > #size-cells = <1>; > ranges; > > + interconnects = <&qnoc 11 &qnoc 512>, > + <&qnoc 0 &qnoc 543>; I presu > + interconnect-names = "qup-memory", "qup-config"; > + > i2c0: i2c@a94000 { > compatible = "qcom,geni-i2c"; > reg = <0xa94000 0x4000>; > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project >
Hi Alok, Thanks for the patches! On 1/22/19 08:33, Alok Chauhan wrote: > Add documentation for the interconnect and interconnect-names bindings s/interconnect /interconnects / > for the GENI QUP as detailed by bindings/interconnect/interconnect.txt. > > Signed-off-by: Alok Chauhan <alokc@codeaurora.org> > --- > Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt > index dab7ca9..44d7e02 100644 > --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt > @@ -17,6 +17,12 @@ Required properties if child node exists: > - #address-cells: Must be <1> for Serial Engine Address > - #size-cells: Must be <1> for Serial Engine Address Size > - ranges: Must be present > +- interconnects: phandle to a interconnect provider. Please refer s/a interconnect/an interconnect/ > + ../interconnect/interconnect.txt for details. > + Must be 2 paths corresponding to 2 AXI ports. > +- interconnect-names: Port names to differentiate between the > + 2 interconnect paths defined with interconnect > + specifier. > > Properties for children: > > @@ -67,6 +73,10 @@ Example: > #size-cells = <1>; > ranges; > > + interconnects = <&qnoc 11 &qnoc 512>, > + <&qnoc 0 &qnoc 543>; Please take a snippet from your patch 6/6 and put it here instead of the hard-coded integers above. > + interconnect-names = "qup-memory", "qup-config"; > + > i2c0: i2c@a94000 { > compatible = "qcom,geni-i2c"; > reg = <0xa94000 0x4000>; > When you post a new version, please change the subject of the patch series to PATCH v2, PATCH v3 etc, in order to be able to distinguish between different versions. Thanks, Georgi
Hi, On 1/23/19 19:07, Bjorn Andersson wrote: > On Mon 21 Jan 22:33 PST 2019, Alok Chauhan wrote: > >> Add documentation for the interconnect and interconnect-names bindings >> for the GENI QUP as detailed by bindings/interconnect/interconnect.txt. >> >> Signed-off-by: Alok Chauhan <alokc@codeaurora.org> >> --- >> Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt >> index dab7ca9..44d7e02 100644 >> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt >> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt >> @@ -17,6 +17,12 @@ Required properties if child node exists: >> - #address-cells: Must be <1> for Serial Engine Address >> - #size-cells: Must be <1> for Serial Engine Address Size >> - ranges: Must be present >> +- interconnects: phandle to a interconnect provider. Please refer >> + ../interconnect/interconnect.txt for details. >> + Must be 2 paths corresponding to 2 AXI ports. >> +- interconnect-names: Port names to differentiate between the > > s/Port names/Path names/ > >> + 2 interconnect paths defined with interconnect >> + specifier. > > These two names are significant in that they must match what the driver > expects, hence you must actually specify them here. > > And as the scope of these strings are local to the QUP node you can omit > "qup" from them, so make them "memory" and "config" (or perhaps iface, > to match the clock naming?). Actually there was a discussion in the past where we decided include both the src and dst endpoint names in this property so that there is some symmetry with the "interconnects" property. It would be nice to be consistent across different drivers at least for now. If we want to denote the master and slave ports here, my two cents would be for "qup-mem" and "cpu-qup" or something similar? Thanks, Georgi
On Mon, Jan 21, 2019 at 10:34 PM Alok Chauhan <alokc@codeaurora.org> wrote: > > Add documentation for the interconnect and interconnect-names bindings > for the GENI QUP as detailed by bindings/interconnect/interconnect.txt. > > Signed-off-by: Alok Chauhan <alokc@codeaurora.org> > --- > Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt > index dab7ca9..44d7e02 100644 > --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt > @@ -17,6 +17,12 @@ Required properties if child node exists: > - #address-cells: Must be <1> for Serial Engine Address > - #size-cells: Must be <1> for Serial Engine Address Size > - ranges: Must be present > +- interconnects: phandle to a interconnect provider. Please refer > + ../interconnect/interconnect.txt for details. This path to the interconnect documentation is not correct.
On 2019-01-23 17:10, Evan Green wrote: > On Mon, Jan 21, 2019 at 10:34 PM Alok Chauhan <alokc@codeaurora.org> > wrote: >> >> Add documentation for the interconnect and interconnect-names bindings >> for the GENI QUP as detailed by >> bindings/interconnect/interconnect.txt. >> >> Signed-off-by: Alok Chauhan <alokc@codeaurora.org> >> --- >> Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt | 10 >> ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git >> a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt >> b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt >> index dab7ca9..44d7e02 100644 >> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt >> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt >> @@ -17,6 +17,12 @@ Required properties if child node exists: >> - #address-cells: Must be <1> for Serial Engine Address >> - #size-cells: Must be <1> for Serial Engine Address >> Size >> - ranges: Must be present >> +- interconnects: phandle to a interconnect provider. Please >> refer >> + ../interconnect/interconnect.txt for details. > > This path to the interconnect documentation is not correct. sorry, i will correct this in next patch.
On 2019-01-23 10:35, Georgi Djakov wrote: > Hi Alok, > > Thanks for the patches! > > On 1/22/19 08:33, Alok Chauhan wrote: >> Add documentation for the interconnect and interconnect-names bindings > > s/interconnect /interconnects / > >> for the GENI QUP as detailed by >> bindings/interconnect/interconnect.txt. >> >> Signed-off-by: Alok Chauhan <alokc@codeaurora.org> >> --- >> Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt | 10 >> ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git >> a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt >> b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt >> index dab7ca9..44d7e02 100644 >> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt >> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt >> @@ -17,6 +17,12 @@ Required properties if child node exists: >> - #address-cells: Must be <1> for Serial Engine Address >> - #size-cells: Must be <1> for Serial Engine Address Size >> - ranges: Must be present >> +- interconnects: phandle to a interconnect provider. Please refer > > s/a interconnect/an interconnect/ sure, will do. > >> + ../interconnect/interconnect.txt for details. >> + Must be 2 paths corresponding to 2 AXI ports. >> +- interconnect-names: Port names to differentiate between the >> + 2 interconnect paths defined with interconnect >> + specifier. >> >> Properties for children: >> >> @@ -67,6 +73,10 @@ Example: >> #size-cells = <1>; >> ranges; >> >> + interconnects = <&qnoc 11 &qnoc 512>, >> + <&qnoc 0 &qnoc 543>; > > Please take a snippet from your patch 6/6 and put it here instead of > the > hard-coded integers above. sure > >> + interconnect-names = "qup-memory", "qup-config"; >> + >> i2c0: i2c@a94000 { >> compatible = "qcom,geni-i2c"; >> reg = <0xa94000 0x4000>; >> > > When you post a new version, please change the subject of the patch > series to PATCH v2, PATCH v3 etc, in order to be able to distinguish > between different versions. sure, will do this. > > Thanks, > Georgi
On Wed, Jan 23, 2019 at 08:41:20PM +0200, Georgi Djakov wrote: > Hi, > > On 1/23/19 19:07, Bjorn Andersson wrote: > > On Mon 21 Jan 22:33 PST 2019, Alok Chauhan wrote: > > > >> Add documentation for the interconnect and interconnect-names bindings > >> for the GENI QUP as detailed by bindings/interconnect/interconnect.txt. > >> > >> Signed-off-by: Alok Chauhan <alokc@codeaurora.org> > >> --- > >> Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt | 10 ++++++++++ > >> 1 file changed, 10 insertions(+) > >> > >> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt > >> index dab7ca9..44d7e02 100644 > >> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt > >> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt > >> @@ -17,6 +17,12 @@ Required properties if child node exists: > >> - #address-cells: Must be <1> for Serial Engine Address > >> - #size-cells: Must be <1> for Serial Engine Address Size > >> - ranges: Must be present > >> +- interconnects: phandle to a interconnect provider. Please refer > >> + ../interconnect/interconnect.txt for details. > >> + Must be 2 paths corresponding to 2 AXI ports. > >> +- interconnect-names: Port names to differentiate between the > > > > s/Port names/Path names/ > > > >> + 2 interconnect paths defined with interconnect > >> + specifier. > > > > These two names are significant in that they must match what the driver > > expects, hence you must actually specify them here. > > > > And as the scope of these strings are local to the QUP node you can omit > > "qup" from them, so make them "memory" and "config" (or perhaps iface, > > to match the clock naming?). > > Actually there was a discussion in the past where we decided include > both the src and dst endpoint names in this property so that there is > some symmetry with the "interconnects" property. It would be nice to be > consistent across different drivers at least for now. > If we want to denote the master and slave ports here, my two cents would > be for "qup-mem" and "cpu-qup" or something similar? Well, there's a proposal from Maxime to add 'dma-memory' or something. You all need to sort this out. I assume config or cpu-qup is for register access? Why is this needed? That should get described thru the DT tree. The interconnect stuff was supposed to be for the non-cpu centric view (i.e. DMA masters). Maybe it's fine, but that's not my initial reaction. Rob
On 2/23/19 02:26, Rob Herring wrote: > On Wed, Jan 23, 2019 at 08:41:20PM +0200, Georgi Djakov wrote: >> Hi, >> >> On 1/23/19 19:07, Bjorn Andersson wrote: >>> On Mon 21 Jan 22:33 PST 2019, Alok Chauhan wrote: >>> >>>> Add documentation for the interconnect and interconnect-names bindings >>>> for the GENI QUP as detailed by bindings/interconnect/interconnect.txt. >>>> >>>> Signed-off-by: Alok Chauhan <alokc@codeaurora.org> >>>> --- >>>> Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt | 10 ++++++++++ >>>> 1 file changed, 10 insertions(+) >>>> >>>> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt >>>> index dab7ca9..44d7e02 100644 >>>> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt >>>> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt >>>> @@ -17,6 +17,12 @@ Required properties if child node exists: >>>> - #address-cells: Must be <1> for Serial Engine Address >>>> - #size-cells: Must be <1> for Serial Engine Address Size >>>> - ranges: Must be present >>>> +- interconnects: phandle to a interconnect provider. Please refer >>>> + ../interconnect/interconnect.txt for details. >>>> + Must be 2 paths corresponding to 2 AXI ports. >>>> +- interconnect-names: Port names to differentiate between the >>> >>> s/Port names/Path names/ >>> >>>> + 2 interconnect paths defined with interconnect >>>> + specifier. >>> >>> These two names are significant in that they must match what the driver >>> expects, hence you must actually specify them here. >>> >>> And as the scope of these strings are local to the QUP node you can omit >>> "qup" from them, so make them "memory" and "config" (or perhaps iface, >>> to match the clock naming?). >> >> Actually there was a discussion in the past where we decided include >> both the src and dst endpoint names in this property so that there is >> some symmetry with the "interconnects" property. It would be nice to be >> consistent across different drivers at least for now. >> If we want to denote the master and slave ports here, my two cents would >> be for "qup-mem" and "cpu-qup" or something similar? > > Well, there's a proposal from Maxime to add 'dma-memory' or something. > You all need to sort this out. Agree. I haven't commented on the latest MBUS patches yet. Meanwhile i am trying to get a better understanding of the hardware. In general if there are no better suggestions, i am fine with adding a specific name to handle this kind of dma transfers. > I assume config or cpu-qup is for register access? Why is this needed? > That should get described thru the DT tree. The interconnect stuff was > supposed to be for the non-cpu centric view (i.e. DMA masters). Maybe > it's fine, but that's not my initial reaction. Yes, cpu-qup should be for register access. Alok, please correct me if i am wrong, but I believe this is a separate bus used only for configuration access to some HW blocks. This bus might be disabled by default and we may need to request some bandwidth in order to enable it. It's described as a separate path in DT and we are trying to give it a meaningful name. Thanks, Georgi
diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt index dab7ca9..44d7e02 100644 --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt @@ -17,6 +17,12 @@ Required properties if child node exists: - #address-cells: Must be <1> for Serial Engine Address - #size-cells: Must be <1> for Serial Engine Address Size - ranges: Must be present +- interconnects: phandle to a interconnect provider. Please refer + ../interconnect/interconnect.txt for details. + Must be 2 paths corresponding to 2 AXI ports. +- interconnect-names: Port names to differentiate between the + 2 interconnect paths defined with interconnect + specifier. Properties for children: @@ -67,6 +73,10 @@ Example: #size-cells = <1>; ranges; + interconnects = <&qnoc 11 &qnoc 512>, + <&qnoc 0 &qnoc 543>; + interconnect-names = "qup-memory", "qup-config"; + i2c0: i2c@a94000 { compatible = "qcom,geni-i2c"; reg = <0xa94000 0x4000>;
Add documentation for the interconnect and interconnect-names bindings for the GENI QUP as detailed by bindings/interconnect/interconnect.txt. Signed-off-by: Alok Chauhan <alokc@codeaurora.org> --- Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt | 10 ++++++++++ 1 file changed, 10 insertions(+)