* [PATCH] Add Master Sample Data Mode for SPI
@ 2017-02-10 22:02 ` Vinicius Maciel
0 siblings, 0 replies; 8+ messages in thread
From: Vinicius Maciel @ 2017-02-10 22:02 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,
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.
Signed-off-by: Vinicius Maciel <viniciusfre-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] 8+ messages in thread
* [PATCH] Add Master Sample Data Mode for SPI
@ 2017-02-10 22:02 ` Vinicius Maciel
0 siblings, 0 replies; 8+ messages in thread
From: Vinicius Maciel @ 2017-02-10 22:02 UTC (permalink / raw)
To: linux-arm-kernel
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.
Signed-off-by: Vinicius Maciel <viniciusfre@gmail.com>
---
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] 8+ messages in thread
* Re: [PATCH] Add Master Sample Data Mode for SPI
2017-02-10 22:02 ` Vinicius Maciel
@ 2017-02-11 2:53 ` Siarhei Siamashka
-1 siblings, 0 replies; 8+ messages in thread
From: Siarhei Siamashka @ 2017-02-11 2:53 UTC (permalink / raw)
To: Vinicius Maciel
Cc: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
broonie-DgEjT+Ai2ygdnm+yROfE0A,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8, wens-jdAy2FN1RRM,
linux-spi-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
On Fri, 10 Feb 2017 19:02:47 -0300
Vinicius Maciel <viniciusfre-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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.
>
> Signed-off-by: Vinicius Maciel <viniciusfre-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;
>
Thanks! That's a good catch. This particular bit is actually set in the
reset default register value, according to the Allwinner A20 manual.
But on Allwinner A10 and Allwinner A13 it is documented as unused and
can't be changed (it remains zero even if we try to modify it).
So looks like only A20 is affected, because the kernel currently sets a
non-standard mode, deviating from both Allwinner's default and normal
SPI behaviour.
You still need to update the summary line to add all the necessary
sunxi and spi specific prefixes (see similar commits). Also a similar
fix most likely needs to be applied to the spi-sun6i.c file too (due
to the copy-paste curse and code duplication), but I'm not sure if it
needs to be a part of this patch or a separate one.
Reviewed-by: Siarhei Siamashka <siarhei.siamashka-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
--
Best regards,
Siarhei Siamashka
^ permalink raw reply [flat|nested] 8+ messages in thread
* [linux-sunxi] [PATCH] Add Master Sample Data Mode for SPI
@ 2017-02-11 2:53 ` Siarhei Siamashka
0 siblings, 0 replies; 8+ messages in thread
From: Siarhei Siamashka @ 2017-02-11 2:53 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 10 Feb 2017 19:02:47 -0300
Vinicius Maciel <viniciusfre@gmail.com> 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.
>
> Signed-off-by: Vinicius Maciel <viniciusfre@gmail.com>
> ---
> 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;
>
Thanks! That's a good catch. This particular bit is actually set in the
reset default register value, according to the Allwinner A20 manual.
But on Allwinner A10 and Allwinner A13 it is documented as unused and
can't be changed (it remains zero even if we try to modify it).
So looks like only A20 is affected, because the kernel currently sets a
non-standard mode, deviating from both Allwinner's default and normal
SPI behaviour.
You still need to update the summary line to add all the necessary
sunxi and spi specific prefixes (see similar commits). Also a similar
fix most likely needs to be applied to the spi-sun6i.c file too (due
to the copy-paste curse and code duplication), but I'm not sure if it
needs to be a part of this patch or a separate one.
Reviewed-by: Siarhei Siamashka <siarhei.siamashka@gmail.com>
--
Best regards,
Siarhei Siamashka
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Add Master Sample Data Mode for SPI
2017-02-11 2:53 ` [linux-sunxi] " Siarhei Siamashka
(?)
@ 2017-02-11 3:05 ` Vinicius Maciel
[not found] ` <CAEz5UgEGO+3CZ6DGhUgXqHpvXc7PBpsujER7LrVi2sfY7Vmvsw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
-1 siblings, 1 reply; 8+ messages in thread
From: Vinicius Maciel @ 2017-02-11 3:05 UTC (permalink / raw)
To: Siarhei Siamashka
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
wens-jdAy2FN1RRM, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
linux-spi-u79uwXL29TY76Z2rM5mHXA,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
broonie-DgEjT+Ai2ygdnm+yROfE0A
[-- Attachment #1: Type: text/plain, Size: 2978 bytes --]
Hi Siarhei,
I was really aiming only A20 in this patch.
Em 10 de fev de 2017 23:53, "Siarhei Siamashka" <siarhei.siamashka-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
escreveu:
> On Fri, 10 Feb 2017 19:02:47 -0300
> Vinicius Maciel <viniciusfre-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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.
> >
> > Signed-off-by: Vinicius Maciel <viniciusfre-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;
> >
>
> Thanks! That's a good catch. This particular bit is actually set in the
> reset default register value, according to the Allwinner A20 manual.
> But on Allwinner A10 and Allwinner A13 it is documented as unused and
> can't be changed (it remains zero even if we try to modify it).
>
> So looks like only A20 is affected, because the kernel currently sets a
> non-standard mode, deviating from both Allwinner's default and normal
> SPI behaviour.
>
> You still need to update the summary line to add all the necessary
> sunxi and spi specific prefixes (see similar commits). Also a similar
> fix most likely needs to be applied to the spi-sun6i.c file too (due
> to the copy-paste curse and code duplication), but I'm not sure if it
> needs to be a part of this patch or a separate one.
>
> Reviewed-by: Siarhei Siamashka <siarhei.siamashka-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> --
> Best regards,
> Siarhei Siamashka
>
--
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 #2: Type: text/html, Size: 4026 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Add Master Sample Data Mode for SPI
2017-02-11 3:05 ` Vinicius Maciel
@ 2017-02-11 3:33 ` Siarhei Siamashka
0 siblings, 0 replies; 8+ messages in thread
From: Siarhei Siamashka @ 2017-02-11 3:33 UTC (permalink / raw)
To: Vinicius Maciel
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
wens-jdAy2FN1RRM, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
linux-spi-u79uwXL29TY76Z2rM5mHXA,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
broonie-DgEjT+Ai2ygdnm+yROfE0A
On Sat, 11 Feb 2017 00:05:42 -0300
Vinicius Maciel <viniciusfre-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Em 10 de fev de 2017 23:53, "Siarhei Siamashka" <siarhei.siamashka-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> escreveu:
>
> > On Fri, 10 Feb 2017 19:02:47 -0300
> > Vinicius Maciel <viniciusfre-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 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.
> > >
> > > Signed-off-by: Vinicius Maciel <viniciusfre-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;
> > >
> >
> > Thanks! That's a good catch. This particular bit is actually set in the
> > reset default register value, according to the Allwinner A20 manual.
> > But on Allwinner A10 and Allwinner A13 it is documented as unused and
> > can't be changed (it remains zero even if we try to modify it).
> >
> > So looks like only A20 is affected, because the kernel currently sets a
> > non-standard mode, deviating from both Allwinner's default and normal
> > SPI behaviour.
> >
> > You still need to update the summary line to add all the necessary
> > sunxi and spi specific prefixes (see similar commits). Also a similar
> > fix most likely needs to be applied to the spi-sun6i.c file too (due
> > to the copy-paste curse and code duplication), but I'm not sure if it
> > needs to be a part of this patch or a separate one.
> >
> > Reviewed-by: Siarhei Siamashka <siarhei.siamashka-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Hi Siarhei,
>
> I was really aiming only A20 in this patch.
Well, regardless of whether you like it or not, A10 and A13 are also
currently using the same code path. So they are potentially affected.
Yes, we may try to introduce a separate compatible property to describe
this particular new A20-specific feature, which did not exist in the
older A10/A13 SoCs and then set this bit only for A20 in the SPI
driver. But since A20 is backwards compatible with A10/A13 (as long
as we don't override the default values of the reserved bits in hardware
registers) and enabling this particular feature is apparently undesired
(it breaks your max11043 ADC use case), it does not seem to be necessary
to me right now. Maxime and others may have a different opinion though.
As for the spi-sun6i.c file, somebody else might encounter the same
problem on A23/A33/H3/H5/A83T/A64 later and waste time debugging
something that had been already resolved by you now. Also having the
spi-sun4i.c and spi-sun6i.c files diverge even more than they are now
is not nice.
--
Best regards,
Siarhei Siamashka
^ permalink raw reply [flat|nested] 8+ messages in thread
* [linux-sunxi] [PATCH] Add Master Sample Data Mode for SPI
@ 2017-02-11 3:33 ` Siarhei Siamashka
0 siblings, 0 replies; 8+ messages in thread
From: Siarhei Siamashka @ 2017-02-11 3:33 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, 11 Feb 2017 00:05:42 -0300
Vinicius Maciel <viniciusfre@gmail.com> wrote:
> Em 10 de fev de 2017 23:53, "Siarhei Siamashka" <siarhei.siamashka@gmail.com>
> escreveu:
>
> > On Fri, 10 Feb 2017 19:02:47 -0300
> > Vinicius Maciel <viniciusfre@gmail.com> 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.
> > >
> > > Signed-off-by: Vinicius Maciel <viniciusfre@gmail.com>
> > > ---
> > > 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;
> > >
> >
> > Thanks! That's a good catch. This particular bit is actually set in the
> > reset default register value, according to the Allwinner A20 manual.
> > But on Allwinner A10 and Allwinner A13 it is documented as unused and
> > can't be changed (it remains zero even if we try to modify it).
> >
> > So looks like only A20 is affected, because the kernel currently sets a
> > non-standard mode, deviating from both Allwinner's default and normal
> > SPI behaviour.
> >
> > You still need to update the summary line to add all the necessary
> > sunxi and spi specific prefixes (see similar commits). Also a similar
> > fix most likely needs to be applied to the spi-sun6i.c file too (due
> > to the copy-paste curse and code duplication), but I'm not sure if it
> > needs to be a part of this patch or a separate one.
> >
> > Reviewed-by: Siarhei Siamashka <siarhei.siamashka@gmail.com>
> Hi Siarhei,
>
> I was really aiming only A20 in this patch.
Well, regardless of whether you like it or not, A10 and A13 are also
currently using the same code path. So they are potentially affected.
Yes, we may try to introduce a separate compatible property to describe
this particular new A20-specific feature, which did not exist in the
older A10/A13 SoCs and then set this bit only for A20 in the SPI
driver. But since A20 is backwards compatible with A10/A13 (as long
as we don't override the default values of the reserved bits in hardware
registers) and enabling this particular feature is apparently undesired
(it breaks your max11043 ADC use case), it does not seem to be necessary
to me right now. Maxime and others may have a different opinion though.
As for the spi-sun6i.c file, somebody else might encounter the same
problem on A23/A33/H3/H5/A83T/A64 later and waste time debugging
something that had been already resolved by you now. Also having the
spi-sun4i.c and spi-sun6i.c files diverge even more than they are now
is not nice.
--
Best regards,
Siarhei Siamashka
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Add Master Sample Data Mode for SPI
2017-02-11 3:33 ` [linux-sunxi] " Siarhei Siamashka
(?)
@ 2017-02-11 4:15 ` Vinicius Maciel
-1 siblings, 0 replies; 8+ messages in thread
From: Vinicius Maciel @ 2017-02-11 4:15 UTC (permalink / raw)
To: linux-sunxi
Cc: viniciusfre-Re5JQEeQqe8AvxtiuMwx3w,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
wens-jdAy2FN1RRM, linux-spi-u79uwXL29TY76Z2rM5mHXA,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
broonie-DgEjT+Ai2ygdnm+yROfE0A
[-- Attachment #1.1: Type: text/plain, Size: 4959 bytes --]
Hi Siarhei,
I'm aware this driver is used by A13 and A10. Indeed, I did test this patch
in A13 and the bit is set to 1, despite the fact that apparently is not be
used. But I did not test SPI communication on A13 with max11043.
Em sábado, 11 de fevereiro de 2017 00:33:14 UTC-3, Siarhei Siamashka
escreveu:
>
> On Sat, 11 Feb 2017 00:05:42 -0300
> Vinicius Maciel <vinic...-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org <javascript:>> wrote:
>
> > Em 10 de fev de 2017 23:53, "Siarhei Siamashka" <siarhei....-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
> <javascript:>>
> > escreveu:
> >
> > > On Fri, 10 Feb 2017 19:02:47 -0300
> > > Vinicius Maciel <vinic...-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org <javascript:>> 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.
> > > >
> > > > Signed-off-by: Vinicius Maciel <vinic...-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org <javascript:>>
> > > > ---
> > > > 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;
> > > >
> > >
> > > Thanks! That's a good catch. This particular bit is actually set in
> the
> > > reset default register value, according to the Allwinner A20 manual.
> > > But on Allwinner A10 and Allwinner A13 it is documented as unused and
> > > can't be changed (it remains zero even if we try to modify it).
> > >
> > > So looks like only A20 is affected, because the kernel currently sets
> a
> > > non-standard mode, deviating from both Allwinner's default and normal
> > > SPI behaviour.
> > >
> > > You still need to update the summary line to add all the necessary
> > > sunxi and spi specific prefixes (see similar commits). Also a similar
> > > fix most likely needs to be applied to the spi-sun6i.c file too (due
> > > to the copy-paste curse and code duplication), but I'm not sure if it
> > > needs to be a part of this patch or a separate one.
> > >
> > > Reviewed-by: Siarhei Siamashka <siarhei....-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org <javascript:>>
>
> > Hi Siarhei,
> >
> > I was really aiming only A20 in this patch.
>
> Well, regardless of whether you like it or not, A10 and A13 are also
> currently using the same code path. So they are potentially affected.
>
> Yes, we may try to introduce a separate compatible property to describe
> this particular new A20-specific feature, which did not exist in the
> older A10/A13 SoCs and then set this bit only for A20 in the SPI
> driver. But since A20 is backwards compatible with A10/A13 (as long
> as we don't override the default values of the reserved bits in hardware
> registers) and enabling this particular feature is apparently undesired
> (it breaks your max11043 ADC use case), it does not seem to be necessary
> to me right now. Maxime and others may have a different opinion though.
>
> As for the spi-sun6i.c file, somebody else might encounter the same
> problem on A23/A33/H3/H5/A83T/A64 later and waste time debugging
> something that had been already resolved by you now. Also having the
> spi-sun4i.c and spi-sun6i.c files diverge even more than they are now
> is not nice.
>
> --
> Best regards,
> Siarhei Siamashka
>
--
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: 6975 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-02-11 4:15 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-10 22:02 [PATCH] Add Master Sample Data Mode for SPI Vinicius Maciel
2017-02-10 22:02 ` Vinicius Maciel
[not found] ` <20170210220247.8478-1-viniciusfre-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-02-11 2:53 ` Siarhei Siamashka
2017-02-11 2:53 ` [linux-sunxi] " Siarhei Siamashka
2017-02-11 3:05 ` Vinicius Maciel
[not found] ` <CAEz5UgEGO+3CZ6DGhUgXqHpvXc7PBpsujER7LrVi2sfY7Vmvsw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-02-11 3:33 ` Siarhei Siamashka
2017-02-11 3:33 ` [linux-sunxi] " Siarhei Siamashka
2017-02-11 4:15 ` Vinicius Maciel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.