Hi Geert-san, > -----Original Message----- > From: devicetree-owner@xxxxxxxxxxxxxxx [mailto:devicetree-owner@xxxxxxxxxxxxxxx] On Behalf Of Geert Uytterhoeven > Sent: Monday, December 01, 2014 5:27 PM > > Hi Shimoda-san, Laurent, > > On Sun, Nov 30, 2014 at 9:23 PM, Laurent Pinchart > <laurent.pinchart@xxxxxxxxxxxxxxxx> 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??? After I asked HW team about this restriction, this controller can set the DTDL/SYNCDL in transfer mode. (In other words, the current manual has error about this specification.) So, I would like to send patches for this configurations. < snip > > >> 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. Thank you very much for your comment. I understood we can use specify the delay values in DT. > 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)? I will use a percentage of the clock cycle. > 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. Since MSIOF can set the configurations in transmit actually, I will set the values for transmit mode. > 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? I checked the files in Documentation/devicetree/spi/, and then I found 2 files about such configurations: - fsl,csbef and fsl,csaft in fsl-spi.txt: - I already mentions in this RFC patch. - samsung,spi-deedback-delay in spi-samsung.txt: - But it is only affect to miso line?? I don't know about the detail of this parameter. So, almost all drivers doesn't have such configuration at the moment, I think. Best regards, Yoshihiro Shimoda > > 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@xxxxxxxxxxxxxx > > 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@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html ��.n��������+%������w��{.n����z�{��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
