| Message ID | 1516021559-19327-1-git-send-email-jassisinghbrar@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 linux-spi" 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 linux-spi" 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 linux-spi" 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 linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
T24gVHVlLCAyMDE4LTAxLTE2IGF0IDA4OjU5ICswNTMwLCBKYXNzaSBCcmFyIHdyb3RlOg0KPiBP biBNb24sIEphbiAxNSwgMjAxOCBhdCAxMDo0NyBQTSwgUm9iIEhlcnJpbmcgPHJvYmgrZHRAa2Vy bmVsLm9yZz4gd3JvdGU6DQo+ID4gT24gTW9uLCBKYW4gMTUsIDIwMTggYXQgOTo1MyBBTSwgSmFz c2kgQnJhciA8amFzc2lzaW5naGJyYXJAZ21haWwuY29tPiB3cm90ZToNCj4gPiA+IE9uIE1vbiwg SmFuIDE1LCAyMDE4IGF0IDg6NDUgUE0sIFJvYiBIZXJyaW5nIDxyb2JoK2R0QGtlcm5lbC5vcmc+ IHdyb3RlOg0KPiA+ID4gPiBPbiBNb24sIEphbiAxNSwgMjAxOCBhdCA3OjA1IEFNLCAgPGphc3Np c2luZ2hicmFyQGdtYWlsLmNvbT4gd3JvdGU6DQo+ID4gPiA+ID4gDQo+ID4gPiA+ID4gKy0gY2xv Y2tzOiBNdXN0IGNvbnRhaW4gYW4gZW50cnkgZm9yIHJhdGUgc291cmNlIGNsb2NrKHMpLg0KPiA+ ID4gPiA+ICstIGNsb2NrLW5hbWVzOiBTaGFsbCBiZSAiaUhDTEsiIG9yICJpUENMSyIuIGlIQ0xL IGlzIHByZWZlcnJlZCBvdmVyIGlQQ0xLDQo+ID4gPiA+IA0KPiA+ID4gPiBIdWg/IFRoZSBjbG9j ayBiaW5kaW5nIHNob3VsZCByZWZsZWN0IGFsbCBjbG9ja3MgY29ubmVjdGVkIHRvIGEgYmxvY2ss DQo+ID4gPiA+IG5vdCBhIHNlbGVjdGlvbiBvZiB3aGljaCBvbmUgeW91IHdhbnQgdG8gdXNlLg0K PiA+ID4gPiANCj4gPiA+IA0KPiA+ID4gQm90aCB0aGUgY2xvY2tzIGFyZSBpbnRlcm5hbCB0byB0 aGUgYmxvY2sgYW5kIGRlcml2ZWQgZnJvbSB0aGUgc2FtZSBzb3VyY2UuDQo+ID4gPiBJbnN0ZWFk IG9mIGRlZmluaW5nIGEgbmV3ICJ1c2UtaXBjbGsiIHByb3BlcnR5LCB0aGUgZHJpdmVyIHVzZXMg dGhlDQo+ID4gPiBjbG9jay1uYW1lcyB0byBjaG9vc2UgdGhlIGFwcHJvcHJpYXRlIGRpdmlkZXIu DQo+ID4gPiBJIGFtIG9wZW4gdG8gYW55IGJldHRlciBvcHRpb24uDQo+ID4gDQo+ID4gSWYgb25l IGlzIHByZWZlcnJlZCwgdGhlbiB3aHkgbm90IGFsd2F5cyB1c2UgaXQ/IE9yIGhvdyBkb2VzIG9u ZQ0KPiA+IGRlY2lkZSB3aGljaCBjbG9jayB0byB1c2U/DQo+ID4gDQo+IA0KPiBBIHNsaWdodCBj b3JyZWN0aW9uLCB0aGVyZSBpcyBhIG11eCBpbnNpZGUgdGhlIGJsb2NrIHdoaWNoIHNlbGVjdHMN Cj4gY2xvY2sgZnJvbSB0d28gaW5wdXQgcG9ydHMgKGlQQ0xLIGFuZCBpSENMSykgYW5kIHNlbmQg dGhhdCB0byB0aGUNCj4gZGl2aWRlci4gRGVwZW5kaW5nIHVwb24gdGhlIHNwaSBzbGF2ZSBzcGVl ZCByZXF1aXJlbWVudHMgdGhlIHBsYXRmb3JtDQoNCkl0IHNlZW1zIGxpa2UgdGhlIERUIGlzIGEg cmVhc29uYWJsZSBkZXNjcmlwdGlvbiBvZiBob3cgdGhlIGhhcmR3YXJlIGlzDQpkZXNpZ25lZC4g IEl0J3MgcG9zc2libGUgdG8gY2hvb3NlIGRpZmZlcmVudCBjbG9ja3Mgd2l0aCBkaWZmZXJlbnQN CnJhdGVzIHRvIGJlIHdpcmVkIHRvIFBDTEsgYW5kL29yIEhDTEssIHZpYSBzb21lIGludGVybmFs IGNsb2NrIHRyZWUgaW4NCnRoZSBTb0M/ICBUaGF0J3Mgd2hhdCB0aGUgRFQgd291bGQgaW5kaWNh dGUuDQoNCllvdSBjb3VsZCBtYXliZSBhbGxvdyBvbmUgb3IgYm90aCB0byBiZSBjb25uZWN0ZWQs IGFuZCBpZiBib3RoIGFyZQ0KY29ubmVjdGVkLCBoYXZlIHRoZSBkcml2ZXIgcHJlZmVyIG9uZSBi YXNlZCBvbiBzb21lIGRyaXZlciBsb2dpYy4gDQpXaGljaCB3b3VsZCBhbGxvdyBvbmUgdG8gdGhl bi4uLg0KDQo+IE5vdyB0aGUgaWRlYSBkYXducyB0aGF0IERUIHByb3ZpZGUgYm90aCBjbG9ja3Mg YW5kIHRoZSBkcml2ZXIgc2VsZWN0DQo+IGZyb20gdGhlIG11eCBsb29raW5nIGF0IHNwZWVkIHJl cXVpcmVtZW50cyBvZiB0aGUgc2xhdmUgYXQgcnVudGltZS4NCg0KYWRkIHRoaXMgZmVhdHVyZSBh dCBzb21lIHBvaW50LiAgSXQgbWlnaHQgYmUgc2ltcGxlciB0byBqdXN0IHByZWZlcg0KSENMSyBp ZiBib3RoIGFyZSB0aGVyZS4gIFRoZSBpZGVhIGJlaW5nIHRoYXQgdGhlIERUIHNob3VsZCBkZXNj cmliZSB0aGUNCmhhcmR3YXJlIHJhdGhlciB0aGFuIHRlbGwgdGhlIGRyaXZlciB3aGF0IHRvIGRv Lg0KDQpTcGVlZCBjYW4gYmUgY2hhbmdlZCBvbiBhIHhmZXIgYnkgeGZlciBiYXNpcyBvbiBhIHNp bmdsZSBtZXNzYWdlIHRvIGENCnNpbmdsZSBzbGF2ZS4gIFRyeWluZyB0byBzd2l0Y2ggY2xvY2tz IGR1cmluZyBhIG1lc3NhZ2UsIGlmIGl0IHJlcXVpcmVzDQpyZXNldHRpbmcgdGhlIHdob2xlIGJs b2NrLCBzb3VuZHMgcmF0aGVyIGRpZmZpY3VsdC4NCg0K -- To unsubscribe from this list: send the line "unsubscribe linux-spi" 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 linux-spi" 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; + };