[PATCH 1/2] spi: cadence: Make sure that clock polarity changes are applied

[PATCH 1/2] spi: cadence: Make sure that clock polarity changes are applied

[Lars-Peter Clausen]

It seems that the cadence SPI controller does not immediately change the clock
polarity setting when writing the CR register. Instead the change is delayed
until the next transfer starts. This happens after the chip select line has
already been asserted. As a result the first transfer after a clock polarity
change will generate spurious clock transitions which typically results in the
SPI slave not being able to properly understand the message. Toggling the ER
register seems to cause the SPI controller to apply the clock polarity changes,
so implement this as a workaround to fix the issue.

Signed-off-by: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
---
 drivers/spi/spi-cadence.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi-cadence.c b/drivers/spi/spi-cadence.c
index 98b763b..03f4d5e 100644
--- a/drivers/spi/spi-cadence.c
+++ b/drivers/spi/spi-cadence.c
@@ -205,18 +205,30 @@ static void cdns_spi_chipselect(struct spi_device *spi, bool is_high)
 static void cdns_spi_config_clock_mode(struct spi_device *spi)
 {
 	struct cdns_spi *xspi = spi_master_get_devdata(spi->master);
-	u32 ctrl_reg;
+	u32 ctrl_reg, new_ctrl_reg;
 
-	ctrl_reg = cdns_spi_read(xspi, CDNS_SPI_CR_OFFSET);
+	new_ctrl_reg = ctrl_reg = cdns_spi_read(xspi, CDNS_SPI_CR_OFFSET);
 
 	/* Set the SPI clock phase and clock polarity */
-	ctrl_reg &= ~(CDNS_SPI_CR_CPHA_MASK | CDNS_SPI_CR_CPOL_MASK);
+	new_ctrl_reg &= ~(CDNS_SPI_CR_CPHA_MASK | CDNS_SPI_CR_CPOL_MASK);
 	if (spi->mode & SPI_CPHA)
-		ctrl_reg |= CDNS_SPI_CR_CPHA_MASK;
+		new_ctrl_reg |= CDNS_SPI_CR_CPHA_MASK;
 	if (spi->mode & SPI_CPOL)
-		ctrl_reg |= CDNS_SPI_CR_CPOL_MASK;
-
-	cdns_spi_write(xspi, CDNS_SPI_CR_OFFSET, ctrl_reg);
+		new_ctrl_reg |= CDNS_SPI_CR_CPOL_MASK;
+
+	if (new_ctrl_reg != ctrl_reg) {
+		/*
+		 * Just writing the CR register does not seem to apply the clock
+		 * setting changes. This is problematic when changing the clock
+		 * polarity as it will cause the SPI slave to see spurious clock
+		 * transitions. To workaround the issue toggle the ER register.
+		 */
+		cdns_spi_write(xspi, CDNS_SPI_ER_OFFSET,
+				   CDNS_SPI_ER_DISABLE_MASK);
+		cdns_spi_write(xspi, CDNS_SPI_CR_OFFSET, new_ctrl_reg);
+		cdns_spi_write(xspi, CDNS_SPI_ER_OFFSET,
+				   CDNS_SPI_ER_ENABLE_MASK);
+	}
 }
 
 /**
-- 
1.8.0

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] spi: cadence: Configure SPI clock in the prepare_message() callback

[Lars-Peter Clausen]

Currently the cadence SPI driver does the SPI clock configuration (setup CPOL
and CPHA) in the prepare_transfer_hardware() callback. The
prepare_transfer_hardware() callback is only called though when the controller
transitions from a idle state to a non-idle state. Such a transitions happens
when the message queue goes from empty to non-empty. If multiple messages from
different SPI slaves with different clock settings are in the message queue the
clock settings will not be properly updated when switching from one slave device
to another. Instead do the updating of the clock configuration in the
prepare_message() callback which will be called for each individual message.

Signed-off-by: Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
---
 drivers/spi/spi-cadence.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-cadence.c b/drivers/spi/spi-cadence.c
index 03f4d5e..562ff83 100644
--- a/drivers/spi/spi-cadence.c
+++ b/drivers/spi/spi-cadence.c
@@ -382,6 +382,12 @@ static irqreturn_t cdns_spi_irq(int irq, void *dev_id)
 
 	return status;
 }
+static int cdns_prepare_message(struct spi_master *master,
+				struct spi_message *msg)
+{
+	cdns_spi_config_clock_mode(msg->spi);
+	return 0;
+}
 
 /**
  * cdns_transfer_one - Initiates the SPI transfer
@@ -428,8 +434,6 @@ static int cdns_prepare_transfer_hardware(struct spi_master *master)
 {
 	struct cdns_spi *xspi = spi_master_get_devdata(master);
 
-	cdns_spi_config_clock_mode(master->cur_msg->spi);
-
 	cdns_spi_write(xspi, CDNS_SPI_ER_OFFSET,
 		       CDNS_SPI_ER_ENABLE_MASK);
 
@@ -544,6 +548,7 @@ static int cdns_spi_probe(struct platform_device *pdev)
 		xspi->is_decoded_cs = 0;
 
 	master->prepare_transfer_hardware = cdns_prepare_transfer_hardware;
+	master->prepare_message = cdns_prepare_message;
 	master->transfer_one = cdns_transfer_one;
 	master->unprepare_transfer_hardware = cdns_unprepare_transfer_hardware;
 	master->set_cs = cdns_spi_chipselect;
-- 
1.8.0

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] spi: cadence: Configure SPI clock in the prepare_message() callback

[Harini Katakam]

Hi,

On Thu, Jul 10, 2014 at 2:56 PM, Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org> wrote:
> Currently the cadence SPI driver does the SPI clock configuration (setup CPOL
> and CPHA) in the prepare_transfer_hardware() callback. The
> prepare_transfer_hardware() callback is only called though when the controller
> transitions from a idle state to a non-idle state. Such a transitions happens
> when the message queue goes from empty to non-empty. If multiple messages from
> different SPI slaves with different clock settings are in the message queue the
> clock settings will not be properly updated when switching from one slave device
> to another. Instead do the updating of the clock configuration in the
> prepare_message() callback which will be called for each individual message.
>

Yes, the requirement from the controller is that CPOL/CPHA setting changes
will not take effect when SPI is enabled. CPOL/CPHA setting is done in
prepare_hardware
before SPI is enabled. So this works.
According to your patches I understand that you might change CPOL/CPHA for
each message. Is this possible? Is this a requirement?

Regards,
Harini
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] spi: cadence: Configure SPI clock in the prepare_message() callback

[Lars-Peter Clausen]

On 07/10/2014 12:43 PM, Harini Katakam wrote:
> Hi,
>
> On Thu, Jul 10, 2014 at 2:56 PM, Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org> wrote:
>> Currently the cadence SPI driver does the SPI clock configuration (setup CPOL
>> and CPHA) in the prepare_transfer_hardware() callback. The
>> prepare_transfer_hardware() callback is only called though when the controller
>> transitions from a idle state to a non-idle state. Such a transitions happens
>> when the message queue goes from empty to non-empty. If multiple messages from
>> different SPI slaves with different clock settings are in the message queue the
>> clock settings will not be properly updated when switching from one slave device
>> to another. Instead do the updating of the clock configuration in the
>> prepare_message() callback which will be called for each individual message.
>>
>
> Yes, the requirement from the controller is that CPOL/CPHA setting changes
> will not take effect when SPI is enabled. CPOL/CPHA setting is done in
> prepare_hardware
> before SPI is enabled. So this works.
> According to your patches I understand that you might change CPOL/CPHA for
> each message. Is this possible? Is this a requirement?

The message can be from different SPI devices, so yes this is a requirement. 
I'm seeing the issue in one of my setups.

- Lars

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] spi: cadence: Make sure that clock polarity changes are applied

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 329 bytes --]

On Thu, Jul 10, 2014 at 11:26:28AM +0200, Lars-Peter Clausen wrote:
> It seems that the cadence SPI controller does not immediately change the clock
> polarity setting when writing the CR register. Instead the change is delayed
> until the next transfer starts. This happens after the chip select line has

Applied both, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]