* [PATCH v2] ARM: spi/sun7i: Add Master Sample Data Mode for SPI @ 2017-02-11 3:54 Vinicius Maciel [not found] ` <20170211035447.3819-1-viniciusfre-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 0 siblings, 1 reply; 7+ messages in thread From: Vinicius Maciel @ 2017-02-11 3:54 UTC (permalink / raw) To: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw Cc: broonie-DgEjT+Ai2ygdnm+yROfE0A, maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8, wens-jdAy2FN1RRM, linux-spi-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, siarhei.siamashka-Re5JQEeQqe8AvxtiuMwx3w, Vinicius Maciel In order to work appropriately, the max11043 ADC chip and probably others, needs SPI master samples the data at the correct edge. From max11043 datasheet: "The data at DIN is latched on the rising edge of SCLK". Same to DOUT. This patch add Master Sample Data Mode bit in normal sample mode. It will affect only A20. Signed-off-by: Vinicius Maciel <viniciusfre-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Reviewed-by: Siarhei Siamashka <siarhei.siamashka-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> --- drivers/spi/spi-sun4i.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c index c5cd635c28f3..6325be2ce8d9 100644 --- a/drivers/spi/spi-sun4i.c +++ b/drivers/spi/spi-sun4i.c @@ -44,6 +44,7 @@ #define SUN4I_CTL_CS_MANUAL BIT(16) #define SUN4I_CTL_CS_LEVEL BIT(17) #define SUN4I_CTL_TP BIT(18) +#define SUN4I_CTL_SDM BIT(20) #define SUN4I_INT_CTL_REG 0x0c #define SUN4I_INT_CTL_RF_F34 BIT(4) @@ -407,7 +408,8 @@ static int sun4i_spi_runtime_resume(struct device *dev) } sun4i_spi_write(sspi, SUN4I_CTL_REG, - SUN4I_CTL_ENABLE | SUN4I_CTL_MASTER | SUN4I_CTL_TP); + SUN4I_CTL_ENABLE | SUN4I_CTL_MASTER | SUN4I_CTL_TP | + SUN4I_CTL_SDM); return 0; -- 2.11.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
[parent not found: <20170211035447.3819-1-viniciusfre-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH v2] ARM: spi/sun7i: Add Master Sample Data Mode for SPI [not found] ` <20170211035447.3819-1-viniciusfre-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2017-02-13 7:12 ` Maxime Ripard 2017-02-13 12:52 ` Mark Brown 1 sibling, 0 replies; 7+ messages in thread From: Maxime Ripard @ 2017-02-13 7:12 UTC (permalink / raw) To: Vinicius Maciel Cc: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, broonie-DgEjT+Ai2ygdnm+yROfE0A, wens-jdAy2FN1RRM, linux-spi-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, siarhei.siamashka-Re5JQEeQqe8AvxtiuMwx3w [-- Attachment #1: Type: text/plain, Size: 777 bytes --] On Sat, Feb 11, 2017 at 12:54:47AM -0300, Vinicius Maciel wrote: > In order to work appropriately, the max11043 ADC chip and probably > others, needs SPI master samples the data at the correct edge. From > max11043 datasheet: "The data at DIN is latched on the rising edge > of SCLK". Same to DOUT. > > This patch add Master Sample Data Mode bit in normal sample mode. > It will affect only A20. > > Signed-off-by: Vinicius Maciel <viniciusfre-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > Reviewed-by: Siarhei Siamashka <siarhei.siamashka-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Acked-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] ARM: spi/sun7i: Add Master Sample Data Mode for SPI [not found] ` <20170211035447.3819-1-viniciusfre-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2017-02-13 7:12 ` Maxime Ripard @ 2017-02-13 12:52 ` Mark Brown [not found] ` <20170213125239.f5vsxrfwz5t6gqyp-GFdadSzt00ze9xe1eoZjHA@public.gmane.org> 1 sibling, 1 reply; 7+ messages in thread From: Mark Brown @ 2017-02-13 12:52 UTC (permalink / raw) To: Vinicius Maciel Cc: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8, wens-jdAy2FN1RRM, linux-spi-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, siarhei.siamashka-Re5JQEeQqe8AvxtiuMwx3w [-- Attachment #1: Type: text/plain, Size: 616 bytes --] On Sat, Feb 11, 2017 at 12:54:47AM -0300, Vinicius Maciel wrote: > In order to work appropriately, the max11043 ADC chip and probably > others, needs SPI master samples the data at the correct edge. From > max11043 datasheet: "The data at DIN is latched on the rising edge > of SCLK". Same to DOUT. > > This patch add Master Sample Data Mode bit in normal sample mode. > It will affect only A20. This should be controlled by the SPI mode settings, different chips have different requirements. If the controller supports multiple modes then it should expose that and let the drivers and system integrations choose. ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <20170213125239.f5vsxrfwz5t6gqyp-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>]
* Re: [PATCH v2] ARM: spi/sun7i: Add Master Sample Data Mode for SPI [not found] ` <20170213125239.f5vsxrfwz5t6gqyp-GFdadSzt00ze9xe1eoZjHA@public.gmane.org> @ 2017-02-26 14:10 ` Vinicius Maciel [not found] ` <186c0f31-01d0-4a1b-889b-a390a57bd1d9-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org> 0 siblings, 1 reply; 7+ messages in thread From: Vinicius Maciel @ 2017-02-26 14:10 UTC (permalink / raw) To: linux-sunxi Cc: viniciusfre-Re5JQEeQqe8AvxtiuMwx3w, maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8, wens-jdAy2FN1RRM, linux-spi-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, siarhei.siamashka-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A [-- Attachment #1.1: Type: text/plain, Size: 911 bytes --] Em segunda-feira, 13 de fevereiro de 2017 09:52:53 UTC-3, Mark Brown escreveu: > > This should be controlled by the SPI mode settings, different chips have > different requirements. If the controller supports multiple modes then > it should expose that and let the drivers and system integrations > choose. > Do you mean SPI_CPHA/SPI_CPOL, right? Well, I don't know how this mode can to fit in existing modes. Since this bit is to control when the data will be sampled, in the same clock edge or at the edge that is half cycle delayed. That has nothing to do with SPI_CPHA/SPI_CPOL. -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit https://groups.google.com/d/optout. [-- Attachment #1.2: Type: text/html, Size: 2123 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <186c0f31-01d0-4a1b-889b-a390a57bd1d9-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org>]
* Re: [PATCH v2] ARM: spi/sun7i: Add Master Sample Data Mode for SPI [not found] ` <186c0f31-01d0-4a1b-889b-a390a57bd1d9-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org> @ 2017-03-02 20:44 ` Siarhei Siamashka 2017-03-02 22:00 ` Vinicius Maciel 0 siblings, 1 reply; 7+ messages in thread From: Siarhei Siamashka @ 2017-03-02 20:44 UTC (permalink / raw) To: Vinicius Maciel Cc: linux-sunxi, maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8, wens-jdAy2FN1RRM, linux-spi-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, broonie-DgEjT+Ai2ygdnm+yROfE0A On Sun, 26 Feb 2017 06:10:50 -0800 (PST) Vinicius Maciel <viniciusfre-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote: > Em segunda-feira, 13 de fevereiro de 2017 09:52:53 UTC-3, Mark Brown > escreveu: > > > > This should be controlled by the SPI mode settings, different chips have > > different requirements. If the controller supports multiple modes then > > it should expose that and let the drivers and system integrations > > choose. > > > Do you mean SPI_CPHA/SPI_CPOL, right? Well, I don't know how this mode can > to fit in existing modes. Since this bit is to control when the data will > be sampled, in the same clock edge or at the edge that is half cycle > delayed. That has nothing to do with SPI_CPHA/SPI_CPOL. I'm not sure if it is realistic to make any progress with this issue any time soon, but I would suggest you to re-send the patch with an updated commit message, which could better explain what's going on. I still think that your bugfix is good, that's why you got my Reviewed-by tag. And we had an older discussion about it (Mark was not participating there, so probably he missed it): https://patchwork.kernel.org/patch/9567547/ Here is a short summary. Older Allwinner A10/A13 SoCs used to have reserved RAZ/WI (Read-As-Zero, Writes Ignored) bits 20-31 in the SPI_CTL register. Newer Allwinner A20 SoC introduced some sort of a wacky half-cycle delay mode and used the previously reserved bit 20 from the SPI_CTL to enable/disable it. Now the tricky part is that the meaning of this new bit is inverted for some unknown reason: the wacky mode is activated when the bit is set to zero, while the standard behaviour (same as A10/A13) is preserved when this bit is set to 1. The reset default of this bit is 1 on A20 (normal SPI behaviour). The problem of the current SPI driver is that it clears bits 20-31 of SPI_CTL, which results in normal SPI behaviour on A10/A13, and a wacky half-cycle delayed mode on A20. Your bugfix just makes A20 behave in the same way as A10/A13. We have exactly zero users of the wacky half-cycle delayed mode. Moreover, if it gets enabled accidentally (due to a bug in the SPI_CTL register initialization in the SPI driver as explained above), then it breaks your max11043 ADC chip. Also even if we assume that there is some hypothetical equipment, which actually needs this wacky mode to function preperly, then this equipment is not compatible with older A10/A13 SoCs because the older SoCs did not implement this mode at all. You could rephrase the above two paragraphs and use them as the commit message for the v3 patch. And maybe also add an extra comment to the SPI driver code. If Mark has any comments on this, then I would be very happy to hear them. So far his suggestion looks like he prefers to see a support for this wacky mode implemented in the SPI driver (a *new feature*, which has no actual users and can't be realistically tested). While your patch just prevents accidental and undesired activation of the wacky mode (an *important bugfix* for a real problem encountered in the wild). -- Best regards, Siarhei Siamashka ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] ARM: spi/sun7i: Add Master Sample Data Mode for SPI 2017-03-02 20:44 ` Siarhei Siamashka @ 2017-03-02 22:00 ` Vinicius Maciel 0 siblings, 0 replies; 7+ messages in thread From: Vinicius Maciel @ 2017-03-02 22:00 UTC (permalink / raw) To: linux-sunxi Cc: viniciusfre-Re5JQEeQqe8AvxtiuMwx3w, maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8, wens-jdAy2FN1RRM, linux-spi-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, broonie-DgEjT+Ai2ygdnm+yROfE0A [-- Attachment #1.1: Type: text/plain, Size: 1603 bytes --] Em quinta-feira, 2 de março de 2017 17:44:16 UTC-3, Siarhei Siamashka escreveu: > Here is a short summary. Older Allwinner A10/A13 SoCs used to have > reserved RAZ/WI (Read-As-Zero, Writes Ignored) bits 20-31 in the > SPI_CTL register. Newer Allwinner A20 SoC introduced some sort of > a wacky half-cycle delay mode and used the previously reserved bit > 20 from the SPI_CTL to enable/disable it. Now the tricky part is that > the meaning of this new bit is inverted for some unknown reason: the > wacky mode is activated when the bit is set to zero, while the standard > behaviour (same as A10/A13) is preserved when this bit is set to 1. The > reset default of this bit is 1 on A20 (normal SPI behaviour). The > problem of the current SPI driver is that it clears bits 20-31 of > SPI_CTL, which results in normal SPI behaviour on A10/A13, and a wacky > half-cycle delayed mode on A20. Your bugfix just makes A20 behave in > the same way as A10/A13. These are the exact words I was looking for, thanks Siarhei. And I don't really know which chip could to use this wacky half-cycle delay mode. Anyway, maybe a new dts property could be a solution to let user to enable or not normal mode and half-cycle mode? Best regards, Vinicius -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org For more options, visit https://groups.google.com/d/optout. [-- Attachment #1.2: Type: text/html, Size: 2026 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] ARM: spi/sun7i: Add Master Sample Data Mode for SPI
@ 2017-02-11 6:58 Icenowy Zheng
0 siblings, 0 replies; 7+ messages in thread
From: Icenowy Zheng @ 2017-02-11 6:58 UTC (permalink / raw)
To: Vinicius Maciel
Cc: broonie-DgEjT+Ai2ygdnm+yROfE0A,
siarhei.siamashka-Re5JQEeQqe8AvxtiuMwx3w,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
wens-jdAy2FN1RRM, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
linux-spi-u79uwXL29TY76Z2rM5mHXA,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8
ARM: is not needed.
2017年2月11日 11:54于 Vinicius Maciel <viniciusfre-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>写道:
>
> In order to work appropriately, the max11043 ADC chip and probably
> others, needs SPI master samples the data at the correct edge. From
> max11043 datasheet: "The data at DIN is latched on the rising edge
> of SCLK". Same to DOUT.
>
> This patch add Master Sample Data Mode bit in normal sample mode.
> It will affect only A20.
>
> Signed-off-by: Vinicius Maciel <viniciusfre-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Reviewed-by: Siarhei Siamashka <siarhei.siamashka-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> drivers/spi/spi-sun4i.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c
> index c5cd635c28f3..6325be2ce8d9 100644
> --- a/drivers/spi/spi-sun4i.c
> +++ b/drivers/spi/spi-sun4i.c
> @@ -44,6 +44,7 @@
> #define SUN4I_CTL_CS_MANUAL BIT(16)
> #define SUN4I_CTL_CS_LEVEL BIT(17)
> #define SUN4I_CTL_TP BIT(18)
> +#define SUN4I_CTL_SDM BIT(20)
>
> #define SUN4I_INT_CTL_REG 0x0c
> #define SUN4I_INT_CTL_RF_F34 BIT(4)
> @@ -407,7 +408,8 @@ static int sun4i_spi_runtime_resume(struct device *dev)
> }
>
> sun4i_spi_write(sspi, SUN4I_CTL_REG,
> - SUN4I_CTL_ENABLE | SUN4I_CTL_MASTER | SUN4I_CTL_TP);
> + SUN4I_CTL_ENABLE | SUN4I_CTL_MASTER | SUN4I_CTL_TP |
> + SUN4I_CTL_SDM);
>
> return 0;
>
> --
> 2.11.0
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
> For more options, visit https://groups.google.com/d/optout.
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-03-02 22:00 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-02-11 3:54 [PATCH v2] ARM: spi/sun7i: Add Master Sample Data Mode for SPI Vinicius Maciel [not found] ` <20170211035447.3819-1-viniciusfre-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2017-02-13 7:12 ` Maxime Ripard 2017-02-13 12:52 ` Mark Brown [not found] ` <20170213125239.f5vsxrfwz5t6gqyp-GFdadSzt00ze9xe1eoZjHA@public.gmane.org> 2017-02-26 14:10 ` Vinicius Maciel [not found] ` <186c0f31-01d0-4a1b-889b-a390a57bd1d9-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org> 2017-03-02 20:44 ` Siarhei Siamashka 2017-03-02 22:00 ` Vinicius Maciel 2017-02-11 6:58 Icenowy Zheng
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).