| Message ID | 5477EFD7.50307@renesas.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Shimoda-san, Thank you for the patch. On Friday 28 November 2014 12:45:27 Yoshihiro Shimoda wrote: > The MSIOF controller has DTDL and SYNCDL in SITMDR1 and SIRMDR1 > registers. So, this patch adds new properties like the following > commit: > d0fb47a5237d8b9576113568bacfd27892308b62 > (spi: fsl-espi: Configure FSL eSPI CSBEF and CSAFT) > > The DTDL is the chip select (SYNC) setup time. > b'000: No bit delay > b'001: 1-clock-cycle delay > b'010: 2-clock-cycle delay > b'101: 0.5-clock-cycle delay > b'110: 1.5-clock-cycle delay > > The SYNCDL is the chip select (SYNC) hold time. > b'000: No bit delay > b'001: 1-clock-cycle delay > b'010: 2-clock-cycle delay > b'011: 3-clock-cycle delay > b'101: 0.5-clock-cycle delay > b'110: 1.5-clock-cycle delay > > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > --- > > I would like to add new properties for sh-msiof driver to adjust > the SYNC siginal timing using DTDL and SYNCDL. In the current driver, > these parameters are hardcoded to 0. And then, I checked other spi > drivers, and I found the following commit: > d0fb47a5237d8b9576113568bacfd27892308b62 > (spi: fsl-espi: Configure FSL eSPI CSBEF and CSAFT) > > If this patch is reasonable, I will modify the sh-msiof driver. > Or, should we add a new function for this timing adjusting in the > spi framework? > > Documentation/devicetree/bindings/spi/sh-msiof.txt | 8 ++++++++ > drivers/spi/spi-sh-msiof.c | 2 ++ > 2 files changed, 10 insertions(+) > > diff --git a/Documentation/devicetree/bindings/spi/sh-msiof.txt > b/Documentation/devicetree/bindings/spi/sh-msiof.txt index d11c372..5fe8ffd > 100644 > --- a/Documentation/devicetree/bindings/spi/sh-msiof.txt > +++ b/Documentation/devicetree/bindings/spi/sh-msiof.txt > @@ -30,6 +30,14 @@ Optional properties: > specifiers, one for transmission, and one for > reception. > - dma-names : Must contain a list of two DMA names, "tx" and > "rx". > +- renesas,tdmr-dtdl : delay sync signal (setup) in transmit mode > + (default is 0, we can set it to 0, 1, 2, 5, or 6) > +- renesas,tdmr-syncdl : delay sync signal (hold) in transmit mode > + (default is 0, we can set it to 0, 1, 2, 3, 5, or 6) > +- renesas,rdmr-dtdl : delay sync signal (setup) in receive mode > + (default is 0, we can set it to 0, 1, 2, 5, or 6) > +- renesas,rdmr-syncdl : delay sync signal (hold) in receive mode > + (default is 0, we can set it to 0, 1, 2, 3, 5, or 6) Was it really the intent of this patch to add DT properties without providing an implementation in the sh-msiof driver ? > Optional properties, deprecated for soctype-specific bindings: > - renesas,tx-fifo-size : Overrides the default tx fifo size given in words > diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c > index 3f36540..09e0c38 100644 > --- a/drivers/spi/spi-sh-msiof.c > +++ b/drivers/spi/spi-sh-msiof.c > @@ -296,11 +296,13 @@ static void sh_msiof_spi_set_pin_regs(struct > sh_msiof_spi_priv *p, tmp = MDR1_SYNCMD_SPI | 1 << MDR1_FLD_SHIFT | > MDR1_XXSTP; > tmp |= !cs_high << MDR1_SYNCAC_SHIFT; > tmp |= lsb_first << MDR1_BITLSB_SHIFT; > +printk("%s: TMDR1 = %x\n", __func__, tmp | MDR1_TRMD | TMDR1_PCON); > sh_msiof_write(p, TMDR1, tmp | MDR1_TRMD | TMDR1_PCON); > if (p->chipdata->master_flags & SPI_MASTER_MUST_TX) { > /* These bits are reserved if RX needs TX */ > tmp &= ~0x0000ffff; > } > +printk("%s: RMDR1 = %x\n", __func__, tmp); I suppose these printk are debugging leftovers. > sh_msiof_write(p, RMDR1, tmp); > > tmp = 0;
Hi Shimoda-san, Laurent, On Sun, Nov 30, 2014 at 9:23 PM, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > On Friday 28 November 2014 12:45:27 Yoshihiro Shimoda wrote: >> The MSIOF controller has DTDL and SYNCDL in SITMDR1 and SIRMDR1 >> registers. So, this patch adds new properties like the following >> commit: >> d0fb47a5237d8b9576113568bacfd27892308b62 >> (spi: fsl-espi: Configure FSL eSPI CSBEF and CSAFT) >> >> The DTDL is the chip select (SYNC) setup time. >> b'000: No bit delay >> b'001: 1-clock-cycle delay >> b'010: 2-clock-cycle delay >> b'101: 0.5-clock-cycle delay >> b'110: 1.5-clock-cycle delay >> >> The SYNCDL is the chip select (SYNC) hold time. >> b'000: No bit delay >> b'001: 1-clock-cycle delay >> b'010: 2-clock-cycle delay >> b'011: 3-clock-cycle delay >> b'101: 0.5-clock-cycle delay >> b'110: 1.5-clock-cycle delay You forgot to quote the last line from the DTDL and SYNCDL paragraphs: | In case of SPI mode, only 000 is allowed to set to this field. And the spi-sh-msiof driver is using the MSIOF in SPI mode??? >> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> >> --- >> >> I would like to add new properties for sh-msiof driver to adjust >> the SYNC siginal timing using DTDL and SYNCDL. In the current driver, >> these parameters are hardcoded to 0. And then, I checked other spi >> drivers, and I found the following commit: >> d0fb47a5237d8b9576113568bacfd27892308b62 >> (spi: fsl-espi: Configure FSL eSPI CSBEF and CSAFT) >> >> If this patch is reasonable, I will modify the sh-msiof driver. >> Or, should we add a new function for this timing adjusting in the >> spi framework? >> >> Documentation/devicetree/bindings/spi/sh-msiof.txt | 8 ++++++++ >> drivers/spi/spi-sh-msiof.c | 2 ++ >> 2 files changed, 10 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/spi/sh-msiof.txt >> b/Documentation/devicetree/bindings/spi/sh-msiof.txt index d11c372..5fe8ffd >> 100644 >> --- a/Documentation/devicetree/bindings/spi/sh-msiof.txt >> +++ b/Documentation/devicetree/bindings/spi/sh-msiof.txt >> @@ -30,6 +30,14 @@ Optional properties: >> specifiers, one for transmission, and one for >> reception. >> - dma-names : Must contain a list of two DMA names, "tx" and >> "rx". >> +- renesas,tdmr-dtdl : delay sync signal (setup) in transmit mode >> + (default is 0, we can set it to 0, 1, 2, 5, or 6) >> +- renesas,tdmr-syncdl : delay sync signal (hold) in transmit mode >> + (default is 0, we can set it to 0, 1, 2, 3, 5, or 6) >> +- renesas,rdmr-dtdl : delay sync signal (setup) in receive mode >> + (default is 0, we can set it to 0, 1, 2, 5, or 6) >> +- renesas,rdmr-syncdl : delay sync signal (hold) in receive mode >> + (default is 0, we can set it to 0, 1, 2, 3, 5, or 6) In general, it's frowned upon the specify actual register values in DT. So I think it's better to specify the delay values instead of the bitfield values, and drop the "tdmr" and "rdmr" from the names. As DT supports integers only, that should become a fixed point value. E.g. delay in deci-clocks (0, 5, 10, 15, 20)? Or as a percentage of the clock cycle (0, 50, 100, 150, 200)? Do you really need two sets of values, for both transmit and receive? I.e. do you need different values for transmit and receive for your use case? SPI does both transmit and receive using the same clock and chip select. I know some MSIOF implementations can handle both indepedently. And to answer your last question: if several drivers need this, it makes sense to make this general, and add support in the SPI core. Mark? > Was it really the intent of this patch to add DT properties without providing > an implementation in the sh-msiof driver ? Given the sentence "If this patch is reasonable, I will modify the sh-msiof driver." in the introduction, I think so. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- 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
Hi Geert-san, Laurent-san, Thank you very much for your comments! > On Sun, Nov 30, 2014 at 9:23 PM, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > On Friday 28 November 2014 12:45:27 Yoshihiro Shimoda wrote: > >> The MSIOF controller has DTDL and SYNCDL in SITMDR1 and SIRMDR1 > >> registers. So, this patch adds new properties like the following > >> commit: > >> d0fb47a5237d8b9576113568bacfd27892308b62 > >> (spi: fsl-espi: Configure FSL eSPI CSBEF and CSAFT) > >> > >> The DTDL is the chip select (SYNC) setup time. > >> b'000: No bit delay > >> b'001: 1-clock-cycle delay > >> b'010: 2-clock-cycle delay > >> b'101: 0.5-clock-cycle delay > >> b'110: 1.5-clock-cycle delay > >> > >> The SYNCDL is the chip select (SYNC) hold time. > >> b'000: No bit delay > >> b'001: 1-clock-cycle delay > >> b'010: 2-clock-cycle delay > >> b'011: 3-clock-cycle delay > >> b'101: 0.5-clock-cycle delay > >> b'110: 1.5-clock-cycle delay > > You forgot to quote the last line from the DTDL and SYNCDL paragraphs: > > | In case of SPI mode, only 000 is allowed to set to this field. Thank you for the point! I asked HW team about the paragraphs. And then, the paragraphs are correct. > And the spi-sh-msiof driver is using the MSIOF in SPI mode??? Yes, the driver is using the MSIOF in SPI mode. So, we have no chance to set the DTDL and SYNCDL to 0 in SPI mode. I should have looked into the datasheet more before I sent this RFC. < snip > > > Was it really the intent of this patch to add DT properties without > > providing an implementation in the sh-msiof driver ? > > Given the sentence "If this patch is reasonable, I will modify the sh-msiof driver." in the introduction, I think so. Yes, I meant that. Best regards, Yoshihiro Shimoda > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say > "programmer" or something like that. > -- Linus Torvalds > -- > 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
SGkgR2VlcnQtc2FuLA0KDQo+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+IEZyb206IGRl dmljZXRyZWUtb3duZXJAdmdlci5rZXJuZWwub3JnIFttYWlsdG86ZGV2aWNldHJlZS1vd25lckB2 Z2VyLmtlcm5lbC5vcmddIE9uIEJlaGFsZiBPZiBHZWVydCBVeXR0ZXJob2V2ZW4NCj4gU2VudDog TW9uZGF5LCBEZWNlbWJlciAwMSwgMjAxNCA1OjI3IFBNDQo+IA0KPiBIaSBTaGltb2RhLXNhbiwg TGF1cmVudCwNCj4gDQo+IE9uIFN1biwgTm92IDMwLCAyMDE0IGF0IDk6MjMgUE0sIExhdXJlbnQg UGluY2hhcnQNCj4gPGxhdXJlbnQucGluY2hhcnRAaWRlYXNvbmJvYXJkLmNvbT4gd3JvdGU6DQo+ ID4gT24gRnJpZGF5IDI4IE5vdmVtYmVyIDIwMTQgMTI6NDU6MjcgWW9zaGloaXJvIFNoaW1vZGEg d3JvdGU6DQo+ID4+IFRoZSBNU0lPRiBjb250cm9sbGVyIGhhcyBEVERMIGFuZCBTWU5DREwgaW4g U0lUTURSMSBhbmQgU0lSTURSMQ0KPiA+PiByZWdpc3RlcnMuIFNvLCB0aGlzIHBhdGNoIGFkZHMg bmV3IHByb3BlcnRpZXMgbGlrZSB0aGUgZm9sbG93aW5nDQo+ID4+IGNvbW1pdDoNCj4gPj4gICBk MGZiNDdhNTIzN2Q4Yjk1NzYxMTM1NjhiYWNmZDI3ODkyMzA4YjYyDQo+ID4+ICAgKHNwaTogZnNs LWVzcGk6IENvbmZpZ3VyZSBGU0wgZVNQSSBDU0JFRiBhbmQgQ1NBRlQpDQo+ID4+DQo+ID4+IFRo ZSBEVERMIGlzIHRoZSBjaGlwIHNlbGVjdCAoU1lOQykgc2V0dXAgdGltZS4NCj4gPj4gIGInMDAw OiBObyBiaXQgZGVsYXkNCj4gPj4gIGInMDAxOiAxLWNsb2NrLWN5Y2xlIGRlbGF5DQo+ID4+ICBi JzAxMDogMi1jbG9jay1jeWNsZSBkZWxheQ0KPiA+PiAgYicxMDE6IDAuNS1jbG9jay1jeWNsZSBk ZWxheQ0KPiA+PiAgYicxMTA6IDEuNS1jbG9jay1jeWNsZSBkZWxheQ0KPiA+Pg0KPiA+PiBUaGUg U1lOQ0RMIGlzIHRoZSBjaGlwIHNlbGVjdCAoU1lOQykgaG9sZCB0aW1lLg0KPiA+PiAgYicwMDA6 IE5vIGJpdCBkZWxheQ0KPiA+PiAgYicwMDE6IDEtY2xvY2stY3ljbGUgZGVsYXkNCj4gPj4gIGIn MDEwOiAyLWNsb2NrLWN5Y2xlIGRlbGF5DQo+ID4+ICBiJzAxMTogMy1jbG9jay1jeWNsZSBkZWxh eQ0KPiA+PiAgYicxMDE6IDAuNS1jbG9jay1jeWNsZSBkZWxheQ0KPiA+PiAgYicxMTA6IDEuNS1j bG9jay1jeWNsZSBkZWxheQ0KPiANCj4gWW91IGZvcmdvdCB0byBxdW90ZSB0aGUgbGFzdCBsaW5l IGZyb20gdGhlIERUREwgYW5kIFNZTkNETCBwYXJhZ3JhcGhzOg0KPiANCj4gfCBJbiBjYXNlIG9m IFNQSSBtb2RlLCBvbmx5IDAwMCBpcyBhbGxvd2VkIHRvIHNldCB0byB0aGlzIGZpZWxkLg0KPiAN Cj4gQW5kIHRoZSBzcGktc2gtbXNpb2YgZHJpdmVyIGlzIHVzaW5nIHRoZSBNU0lPRiBpbiBTUEkg bW9kZT8/Pw0KDQpBZnRlciBJIGFza2VkIEhXIHRlYW0gYWJvdXQgdGhpcyByZXN0cmljdGlvbiwg dGhpcyBjb250cm9sbGVyIGNhbiBzZXQNCnRoZSBEVERML1NZTkNETCBpbiB0cmFuc2ZlciBtb2Rl Lg0KKEluIG90aGVyIHdvcmRzLCB0aGUgY3VycmVudCBtYW51YWwgaGFzIGVycm9yIGFib3V0IHRo aXMgc3BlY2lmaWNhdGlvbi4pDQoNClNvLCBJIHdvdWxkIGxpa2UgdG8gc2VuZCBwYXRjaGVzIGZv ciB0aGlzIGNvbmZpZ3VyYXRpb25zLg0KDQo8IHNuaXAgPg0KPiA+PiBkaWZmIC0tZ2l0IGEvRG9j dW1lbnRhdGlvbi9kZXZpY2V0cmVlL2JpbmRpbmdzL3NwaS9zaC1tc2lvZi50eHQNCj4gPj4gYi9E b2N1bWVudGF0aW9uL2RldmljZXRyZWUvYmluZGluZ3Mvc3BpL3NoLW1zaW9mLnR4dCBpbmRleCBk MTFjMzcyLi41ZmU4ZmZkDQo+ID4+IDEwMDY0NA0KPiA+PiAtLS0gYS9Eb2N1bWVudGF0aW9uL2Rl dmljZXRyZWUvYmluZGluZ3Mvc3BpL3NoLW1zaW9mLnR4dA0KPiA+PiArKysgYi9Eb2N1bWVudGF0 aW9uL2RldmljZXRyZWUvYmluZGluZ3Mvc3BpL3NoLW1zaW9mLnR4dA0KPiA+PiBAQCAtMzAsNiAr MzAsMTQgQEAgT3B0aW9uYWwgcHJvcGVydGllczoNCj4gPj4gICAgICAgICAgICAgICAgICAgICAg ICBzcGVjaWZpZXJzLCBvbmUgZm9yIHRyYW5zbWlzc2lvbiwgYW5kIG9uZSBmb3INCj4gPj4gICAg ICAgICAgICAgICAgICAgICAgICByZWNlcHRpb24uDQo+ID4+ICAtIGRtYS1uYW1lcyAgICAgICAg ICAgIDogTXVzdCBjb250YWluIGEgbGlzdCBvZiB0d28gRE1BIG5hbWVzLCAidHgiIGFuZA0KPiA+ PiAicngiLg0KPiA+PiArLSByZW5lc2FzLHRkbXItZHRkbCAgICA6IGRlbGF5IHN5bmMgc2lnbmFs IChzZXR1cCkgaW4gdHJhbnNtaXQgbW9kZQ0KPiA+PiArICAgICAgICAgICAgICAgICAgICAgIChk ZWZhdWx0IGlzIDAsIHdlIGNhbiBzZXQgaXQgdG8gMCwgMSwgMiwgNSwgb3IgNikNCj4gPj4gKy0g cmVuZXNhcyx0ZG1yLXN5bmNkbCAgOiBkZWxheSBzeW5jIHNpZ25hbCAoaG9sZCkgaW4gdHJhbnNt aXQgbW9kZQ0KPiA+PiArICAgICAgICAgICAgICAgICAgICAgIChkZWZhdWx0IGlzIDAsIHdlIGNh biBzZXQgaXQgdG8gMCwgMSwgMiwgMywgNSwgb3IgNikNCj4gPj4gKy0gcmVuZXNhcyxyZG1yLWR0 ZGwgICAgOiBkZWxheSBzeW5jIHNpZ25hbCAoc2V0dXApIGluIHJlY2VpdmUgbW9kZQ0KPiA+PiAr ICAgICAgICAgICAgICAgICAgICAgIChkZWZhdWx0IGlzIDAsIHdlIGNhbiBzZXQgaXQgdG8gMCwg MSwgMiwgNSwgb3IgNikNCj4gPj4gKy0gcmVuZXNhcyxyZG1yLXN5bmNkbCAgOiBkZWxheSBzeW5j IHNpZ25hbCAoaG9sZCkgaW4gcmVjZWl2ZSBtb2RlDQo+ID4+ICsgICAgICAgICAgICAgICAgICAg ICAgKGRlZmF1bHQgaXMgMCwgd2UgY2FuIHNldCBpdCB0byAwLCAxLCAyLCAzLCA1LCBvciA2KQ0K PiANCj4gSW4gZ2VuZXJhbCwgaXQncyBmcm93bmVkIHVwb24gdGhlIHNwZWNpZnkgYWN0dWFsIHJl Z2lzdGVyIHZhbHVlcyBpbiBEVC4NCj4gU28gSSB0aGluayBpdCdzIGJldHRlciB0byBzcGVjaWZ5 IHRoZSBkZWxheSB2YWx1ZXMgaW5zdGVhZCBvZiB0aGUNCj4gYml0ZmllbGQgdmFsdWVzLA0KPiBh bmQgZHJvcCB0aGUgInRkbXIiIGFuZCAicmRtciIgZnJvbSB0aGUgbmFtZXMuDQoNClRoYW5rIHlv dSB2ZXJ5IG11Y2ggZm9yIHlvdXIgY29tbWVudC4NCkkgdW5kZXJzdG9vZCB3ZSBjYW4gdXNlIHNw ZWNpZnkgdGhlIGRlbGF5IHZhbHVlcyBpbiBEVC4NCg0KPiBBcyBEVCBzdXBwb3J0cyBpbnRlZ2Vy cyBvbmx5LCB0aGF0IHNob3VsZCBiZWNvbWUgYSBmaXhlZCBwb2ludCB2YWx1ZS4NCj4gRS5nLiBk ZWxheSBpbiBkZWNpLWNsb2NrcyAoMCwgNSwgMTAsIDE1LCAyMCk/DQo+IE9yIGFzIGEgcGVyY2Vu dGFnZSBvZiB0aGUgY2xvY2sgY3ljbGUgKDAsIDUwLCAxMDAsIDE1MCwgMjAwKT8NCg0KSSB3aWxs IHVzZSBhIHBlcmNlbnRhZ2Ugb2YgdGhlIGNsb2NrIGN5Y2xlLg0KDQo+IERvIHlvdSByZWFsbHkg bmVlZCB0d28gc2V0cyBvZiB2YWx1ZXMsIGZvciBib3RoIHRyYW5zbWl0IGFuZCByZWNlaXZlPw0K PiBJLmUuIGRvIHlvdSBuZWVkIGRpZmZlcmVudCB2YWx1ZXMgZm9yIHRyYW5zbWl0IGFuZCByZWNl aXZlIGZvciB5b3VyIHVzZSBjYXNlPw0KPiBTUEkgZG9lcyBib3RoIHRyYW5zbWl0IGFuZCByZWNl aXZlIHVzaW5nIHRoZSBzYW1lIGNsb2NrIGFuZCBjaGlwIHNlbGVjdC4NCj4gSSBrbm93IHNvbWUg TVNJT0YgaW1wbGVtZW50YXRpb25zIGNhbiBoYW5kbGUgYm90aCBpbmRlcGVkZW50bHkuDQoNClNp bmNlIE1TSU9GIGNhbiBzZXQgdGhlIGNvbmZpZ3VyYXRpb25zIGluIHRyYW5zbWl0IGFjdHVhbGx5 LA0KSSB3aWxsIHNldCB0aGUgdmFsdWVzIGZvciB0cmFuc21pdCBtb2RlLg0KDQo+IEFuZCB0byBh bnN3ZXIgeW91ciBsYXN0IHF1ZXN0aW9uOiBpZiBzZXZlcmFsIGRyaXZlcnMgbmVlZCB0aGlzLCBp dCBtYWtlcyBzZW5zZQ0KPiB0byBtYWtlIHRoaXMgZ2VuZXJhbCwgYW5kIGFkZCBzdXBwb3J0IGlu IHRoZSBTUEkgY29yZS4gTWFyaz8NCg0KSSBjaGVja2VkIHRoZSBmaWxlcyBpbiBEb2N1bWVudGF0 aW9uL2RldmljZXRyZWUvc3BpLywgYW5kIHRoZW4NCkkgZm91bmQgMiBmaWxlcyBhYm91dCBzdWNo IGNvbmZpZ3VyYXRpb25zOg0KIC0gZnNsLGNzYmVmIGFuZCBmc2wsY3NhZnQgaW4gZnNsLXNwaS50 eHQ6DQogIC0gSSBhbHJlYWR5IG1lbnRpb25zIGluIHRoaXMgUkZDIHBhdGNoLg0KDQogLSBzYW1z dW5nLHNwaS1kZWVkYmFjay1kZWxheSBpbiBzcGktc2Ftc3VuZy50eHQ6DQogIC0gQnV0IGl0IGlz IG9ubHkgYWZmZWN0IHRvIG1pc28gbGluZT8/DQogICAgSSBkb24ndCBrbm93IGFib3V0IHRoZSBk ZXRhaWwgb2YgdGhpcyBwYXJhbWV0ZXIuDQoNClNvLCBhbG1vc3QgYWxsIGRyaXZlcnMgZG9lc24n dCBoYXZlIHN1Y2ggY29uZmlndXJhdGlvbiBhdCB0aGUgbW9tZW50LCBJIHRoaW5rLg0KDQpCZXN0 IHJlZ2FyZHMsDQpZb3NoaWhpcm8gU2hpbW9kYQ0KDQo+ID4gV2FzIGl0IHJlYWxseSB0aGUgaW50 ZW50IG9mIHRoaXMgcGF0Y2ggdG8gYWRkIERUIHByb3BlcnRpZXMgd2l0aG91dCBwcm92aWRpbmcN Cj4gPiBhbiBpbXBsZW1lbnRhdGlvbiBpbiB0aGUgc2gtbXNpb2YgZHJpdmVyID8NCj4gDQo+IEdp dmVuIHRoZSBzZW50ZW5jZSAiSWYgdGhpcyBwYXRjaCBpcyByZWFzb25hYmxlLCBJIHdpbGwgbW9k aWZ5IHRoZSBzaC1tc2lvZg0KPiBkcml2ZXIuIiBpbiB0aGUgaW50cm9kdWN0aW9uLCBJIHRoaW5r IHNvLg0KPiANCj4gR3J7b2V0amUsZWV0aW5nfXMsDQo+IA0KPiAgICAgICAgICAgICAgICAgICAg ICAgICBHZWVydA0KPiANCj4gLS0NCj4gR2VlcnQgVXl0dGVyaG9ldmVuIC0tIFRoZXJlJ3MgbG90 cyBvZiBMaW51eCBiZXlvbmQgaWEzMiAtLSBnZWVydEBsaW51eC1tNjhrLm9yZw0KPiANCj4gSW4g cGVyc29uYWwgY29udmVyc2F0aW9ucyB3aXRoIHRlY2huaWNhbCBwZW9wbGUsIEkgY2FsbCBteXNl bGYgYSBoYWNrZXIuIEJ1dA0KPiB3aGVuIEknbSB0YWxraW5nIHRvIGpvdXJuYWxpc3RzIEkganVz dCBzYXkgInByb2dyYW1tZXIiIG9yIHNvbWV0aGluZyBsaWtlIHRoYXQuDQo+ICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgICAgLS0gTGludXMgVG9ydmFsZHMNCj4gLS0NCj4gVG8gdW5zdWJz Y3JpYmUgZnJvbSB0aGlzIGxpc3Q6IHNlbmQgdGhlIGxpbmUgInVuc3Vic2NyaWJlIGRldmljZXRy ZWUiIGluDQo+IHRoZSBib2R5IG9mIGEgbWVzc2FnZSB0byBtYWpvcmRvbW9Admdlci5rZXJuZWwu b3JnDQo+IE1vcmUgbWFqb3Jkb21vIGluZm8gYXQgIGh0dHA6Ly92Z2VyLmtlcm5lbC5vcmcvbWFq b3Jkb21vLWluZm8uaHRtbA0K -- 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
diff --git a/Documentation/devicetree/bindings/spi/sh-msiof.txt b/Documentation/devicetree/bindings/spi/sh-msiof.txt index d11c372..5fe8ffd 100644 --- a/Documentation/devicetree/bindings/spi/sh-msiof.txt +++ b/Documentation/devicetree/bindings/spi/sh-msiof.txt @@ -30,6 +30,14 @@ Optional properties: specifiers, one for transmission, and one for reception. - dma-names : Must contain a list of two DMA names, "tx" and "rx". +- renesas,tdmr-dtdl : delay sync signal (setup) in transmit mode + (default is 0, we can set it to 0, 1, 2, 5, or 6) +- renesas,tdmr-syncdl : delay sync signal (hold) in transmit mode + (default is 0, we can set it to 0, 1, 2, 3, 5, or 6) +- renesas,rdmr-dtdl : delay sync signal (setup) in receive mode + (default is 0, we can set it to 0, 1, 2, 5, or 6) +- renesas,rdmr-syncdl : delay sync signal (hold) in receive mode + (default is 0, we can set it to 0, 1, 2, 3, 5, or 6) Optional properties, deprecated for soctype-specific bindings: - renesas,tx-fifo-size : Overrides the default tx fifo size given in words diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c index 3f36540..09e0c38 100644 --- a/drivers/spi/spi-sh-msiof.c +++ b/drivers/spi/spi-sh-msiof.c @@ -296,11 +296,13 @@ static void sh_msiof_spi_set_pin_regs(struct sh_msiof_spi_priv *p, tmp = MDR1_SYNCMD_SPI | 1 << MDR1_FLD_SHIFT | MDR1_XXSTP; tmp |= !cs_high << MDR1_SYNCAC_SHIFT; tmp |= lsb_first << MDR1_BITLSB_SHIFT; +printk("%s: TMDR1 = %x\n", __func__, tmp | MDR1_TRMD | TMDR1_PCON); sh_msiof_write(p, TMDR1, tmp | MDR1_TRMD | TMDR1_PCON); if (p->chipdata->master_flags & SPI_MASTER_MUST_TX) { /* These bits are reserved if RX needs TX */ tmp &= ~0x0000ffff; } +printk("%s: RMDR1 = %x\n", __func__, tmp); sh_msiof_write(p, RMDR1, tmp); tmp = 0;
The MSIOF controller has DTDL and SYNCDL in SITMDR1 and SIRMDR1 registers. So, this patch adds new properties like the following commit: d0fb47a5237d8b9576113568bacfd27892308b62 (spi: fsl-espi: Configure FSL eSPI CSBEF and CSAFT) The DTDL is the chip select (SYNC) setup time. b'000: No bit delay b'001: 1-clock-cycle delay b'010: 2-clock-cycle delay b'101: 0.5-clock-cycle delay b'110: 1.5-clock-cycle delay The SYNCDL is the chip select (SYNC) hold time. b'000: No bit delay b'001: 1-clock-cycle delay b'010: 2-clock-cycle delay b'011: 3-clock-cycle delay b'101: 0.5-clock-cycle delay b'110: 1.5-clock-cycle delay Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> --- I would like to add new properties for sh-msiof driver to adjust the SYNC siginal timing using DTDL and SYNCDL. In the current driver, these parameters are hardcoded to 0. And then, I checked other spi drivers, and I found the following commit: d0fb47a5237d8b9576113568bacfd27892308b62 (spi: fsl-espi: Configure FSL eSPI CSBEF and CSAFT) If this patch is reasonable, I will modify the sh-msiof driver. Or, should we add a new function for this timing adjusting in the spi framework? Documentation/devicetree/bindings/spi/sh-msiof.txt | 8 ++++++++ drivers/spi/spi-sh-msiof.c | 2 ++ 2 files changed, 10 insertions(+)