| Message ID | 1516021559-19327-1-git-send-email-jassisinghbrar@gmail.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | spi: support for Socionext Synquacer platform | expand |
On Mon, Jan 15, 2018 at 7:05 AM, <jassisinghbrar@gmail.com> wrote: > From: Jassi Brar <jaswinder.singh@linaro.org> > > This patch adds documentation for Device-Tree bindings for the > Socionext Synquacer spi driver. > > Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org> > --- > .../devicetree/bindings/spi/spi-synquacer.txt | 24 ++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > create mode 100644 Documentation/devicetree/bindings/spi/spi-synquacer.txt > > diff --git a/Documentation/devicetree/bindings/spi/spi-synquacer.txt b/Documentation/devicetree/bindings/spi/spi-synquacer.txt > new file mode 100644 > index 0000000..d013cfd > --- /dev/null > +++ b/Documentation/devicetree/bindings/spi/spi-synquacer.txt > @@ -0,0 +1,24 @@ > +* Socionext Synquacer HS-SPI bindings > + > +Required Properties: > +- compatible: should be "socionext,synquacer-spi" > +- reg: physical base address of the controller and length of memory mapped > + region. > +- clocks: Must contain an entry for rate source clock(s). > +- clock-names: Shall be "iHCLK" or "iPCLK". iHCLK is preferred over iPCLK Huh? The clock binding should reflect all clocks connected to a block, not a selection of which one you want to use. 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
On Mon, Jan 15, 2018 at 8:45 PM, Rob Herring <robh+dt@kernel.org> wrote: > On Mon, Jan 15, 2018 at 7:05 AM, <jassisinghbrar@gmail.com> wrote: >> From: Jassi Brar <jaswinder.singh@linaro.org> >> >> This patch adds documentation for Device-Tree bindings for the >> Socionext Synquacer spi driver. >> >> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org> >> --- >> .../devicetree/bindings/spi/spi-synquacer.txt | 24 ++++++++++++++++++++++ >> 1 file changed, 24 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/spi/spi-synquacer.txt >> >> diff --git a/Documentation/devicetree/bindings/spi/spi-synquacer.txt b/Documentation/devicetree/bindings/spi/spi-synquacer.txt >> new file mode 100644 >> index 0000000..d013cfd >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/spi/spi-synquacer.txt >> @@ -0,0 +1,24 @@ >> +* Socionext Synquacer HS-SPI bindings >> + >> +Required Properties: >> +- compatible: should be "socionext,synquacer-spi" >> +- reg: physical base address of the controller and length of memory mapped >> + region. >> +- clocks: Must contain an entry for rate source clock(s). >> +- clock-names: Shall be "iHCLK" or "iPCLK". iHCLK is preferred over iPCLK > > Huh? The clock binding should reflect all clocks connected to a block, > not a selection of which one you want to use. > Both the clocks are internal to the block and derived from the same source. Instead of defining a new "use-ipclk" property, the driver uses the clock-names to choose the appropriate divider. I am open to any better option. Thanks -- 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
On Mon, Jan 15, 2018 at 9:53 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote: > On Mon, Jan 15, 2018 at 8:45 PM, Rob Herring <robh+dt@kernel.org> wrote: >> On Mon, Jan 15, 2018 at 7:05 AM, <jassisinghbrar@gmail.com> wrote: >>> From: Jassi Brar <jaswinder.singh@linaro.org> >>> >>> This patch adds documentation for Device-Tree bindings for the >>> Socionext Synquacer spi driver. >>> >>> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org> >>> --- >>> .../devicetree/bindings/spi/spi-synquacer.txt | 24 ++++++++++++++++++++++ >>> 1 file changed, 24 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/spi/spi-synquacer.txt >>> >>> diff --git a/Documentation/devicetree/bindings/spi/spi-synquacer.txt b/Documentation/devicetree/bindings/spi/spi-synquacer.txt >>> new file mode 100644 >>> index 0000000..d013cfd >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/spi/spi-synquacer.txt >>> @@ -0,0 +1,24 @@ >>> +* Socionext Synquacer HS-SPI bindings >>> + >>> +Required Properties: >>> +- compatible: should be "socionext,synquacer-spi" >>> +- reg: physical base address of the controller and length of memory mapped >>> + region. >>> +- clocks: Must contain an entry for rate source clock(s). >>> +- clock-names: Shall be "iHCLK" or "iPCLK". iHCLK is preferred over iPCLK >> >> Huh? The clock binding should reflect all clocks connected to a block, >> not a selection of which one you want to use. >> > Both the clocks are internal to the block and derived from the same source. > Instead of defining a new "use-ipclk" property, the driver uses the > clock-names to choose the appropriate divider. > I am open to any better option. If one is preferred, then why not always use it? Or how does one decide which clock to use? 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
On Mon, Jan 15, 2018 at 10:47 PM, Rob Herring <robh+dt@kernel.org> wrote: > On Mon, Jan 15, 2018 at 9:53 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote: >> On Mon, Jan 15, 2018 at 8:45 PM, Rob Herring <robh+dt@kernel.org> wrote: >>> On Mon, Jan 15, 2018 at 7:05 AM, <jassisinghbrar@gmail.com> wrote: >>>> From: Jassi Brar <jaswinder.singh@linaro.org> >>>> >>>> This patch adds documentation for Device-Tree bindings for the >>>> Socionext Synquacer spi driver. >>>> >>>> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org> >>>> --- >>>> .../devicetree/bindings/spi/spi-synquacer.txt | 24 ++++++++++++++++++++++ >>>> 1 file changed, 24 insertions(+) >>>> create mode 100644 Documentation/devicetree/bindings/spi/spi-synquacer.txt >>>> >>>> diff --git a/Documentation/devicetree/bindings/spi/spi-synquacer.txt b/Documentation/devicetree/bindings/spi/spi-synquacer.txt >>>> new file mode 100644 >>>> index 0000000..d013cfd >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/spi/spi-synquacer.txt >>>> @@ -0,0 +1,24 @@ >>>> +* Socionext Synquacer HS-SPI bindings >>>> + >>>> +Required Properties: >>>> +- compatible: should be "socionext,synquacer-spi" >>>> +- reg: physical base address of the controller and length of memory mapped >>>> + region. >>>> +- clocks: Must contain an entry for rate source clock(s). >>>> +- clock-names: Shall be "iHCLK" or "iPCLK". iHCLK is preferred over iPCLK >>> >>> Huh? The clock binding should reflect all clocks connected to a block, >>> not a selection of which one you want to use. >>> >> Both the clocks are internal to the block and derived from the same source. >> Instead of defining a new "use-ipclk" property, the driver uses the >> clock-names to choose the appropriate divider. >> I am open to any better option. > > If one is preferred, then why not always use it? Or how does one > decide which clock to use? > A slight correction, there is a mux inside the block which selects clock from two input ports (iPCLK and iHCLK) and send that to the divider. Depending upon the spi slave speed requirements the platform may choose to connect either PCLK or HCLK (or maybe both but switching is said to be not feasible without block reset and DT can't suggest switch in runtime so we ask DT to provide only the source clock). Now the idea dawns that DT provide both clocks and the driver select from the mux looking at speed requirements of the slave at runtime. However, then, there will be a wide range of speeds that both could provide. Not to forget the block drives 4 slaves and optimising speed for one could potentially break other slaves. Practically the h/w designer would have already thought of speed requirements of the slaves and made sure that port is populated .... the driver could simple count upon that good design. Thanks -- 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
On Mon, Jan 15, 2018 at 10:47 PM, Rob Herring <robh+dt@kernel.org> wrote: > On Mon, Jan 15, 2018 at 9:53 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote: >> On Mon, Jan 15, 2018 at 8:45 PM, Rob Herring <robh+dt@kernel.org> wrote: >>> On Mon, Jan 15, 2018 at 7:05 AM, <jassisinghbrar@gmail.com> wrote: >>>> From: Jassi Brar <jaswinder.singh@linaro.org> >>>> >>>> This patch adds documentation for Device-Tree bindings for the >>>> Socionext Synquacer spi driver. >>>> >>>> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org> >>>> --- >>>> .../devicetree/bindings/spi/spi-synquacer.txt | 24 ++++++++++++++++++++++ >>>> 1 file changed, 24 insertions(+) >>>> create mode 100644 Documentation/devicetree/bindings/spi/spi-synquacer.txt >>>> >>>> diff --git a/Documentation/devicetree/bindings/spi/spi-synquacer.txt b/Documentation/devicetree/bindings/spi/spi-synquacer.txt >>>> new file mode 100644 >>>> index 0000000..d013cfd >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/spi/spi-synquacer.txt >>>> @@ -0,0 +1,24 @@ >>>> +* Socionext Synquacer HS-SPI bindings >>>> + >>>> +Required Properties: >>>> +- compatible: should be "socionext,synquacer-spi" >>>> +- reg: physical base address of the controller and length of memory mapped >>>> + region. >>>> +- clocks: Must contain an entry for rate source clock(s). >>>> +- clock-names: Shall be "iHCLK" or "iPCLK". iHCLK is preferred over iPCLK >>> >>> Huh? The clock binding should reflect all clocks connected to a block, >>> not a selection of which one you want to use. >>> >> Both the clocks are internal to the block and derived from the same source. >> Instead of defining a new "use-ipclk" property, the driver uses the >> clock-names to choose the appropriate divider. >> I am open to any better option. > > If one is preferred, then why not always use it? Or how does one > decide which clock to use? > So I got my native speaker friend to interpret the Japanese datasheet. The iHCLK clocks the whole block and can also be the source for internal dividers. An external, optional, source iPCLK is also provided for producing lower (Fpclk < Fhclk) rates from the dividers. That is, iHCLK is always needed, while iPCLK can be provided if that is to be fed to the dividers. So the dt bindings should define one required clock (iHCLK) and another optional clock (iPCLK). The driver would use iPCLK if provided, otherwise the default iHCLK. Will update DT and driver for next submission. Thanks. -- 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
On Tue, Jan 16, 2018 at 08:02:06PM +0000, Trent Piepho wrote: > On Tue, 2018-01-16 at 08:59 +0530, Jassi Brar wrote: > > Now the idea dawns that DT provide both clocks and the driver select > > from the mux looking at speed requirements of the slave at runtime. > add this feature at some point. It might be simpler to just prefer > HCLK if both are there. The idea being that the DT should describe the > hardware rather than tell the driver what to do. Or only try one if the other fails to give something. > Speed can be changed on a xfer by xfer basis on a single message to a > single slave. Trying to switch clocks during a message, if it requires > resetting the whole block, sounds rather difficult. In practice that's so vanishingly rare it's probably not worth worrying about.
diff --git a/Documentation/devicetree/bindings/spi/spi-synquacer.txt b/Documentation/devicetree/bindings/spi/spi-synquacer.txt new file mode 100644 index 0000000..d013cfd --- /dev/null +++ b/Documentation/devicetree/bindings/spi/spi-synquacer.txt @@ -0,0 +1,24 @@ +* Socionext Synquacer HS-SPI bindings + +Required Properties: +- compatible: should be "socionext,synquacer-spi" +- reg: physical base address of the controller and length of memory mapped + region. +- clocks: Must contain an entry for rate source clock(s). +- clock-names: Shall be "iHCLK" or "iPCLK". iHCLK is preferred over iPCLK + +Optional Properties: +- num-cs: total number of chipselects +- socionext,use-rtm: boolean, if required to use "retimed clock" for RX +- socionext,set-aces: boolean, if same active clock edges field to be set. + +Example: + + spi0: spi@ff110000 { + compatible = "socionext,synquacer-spi"; + reg = <0xff110000 0x1000>; + clocks = <&clk_fip006_spi>; + clock-names = "iHCLK"; + socionext,use-rtm; + socionext,set-aces; + };