[PATCH v2 0/5] s3c64xx: consider the case of a disconnected CS line and some code rework

[PATCH v2 0/5] s3c64xx: consider the case of a disconnected CS line and some code rework

[Andi Shyti]

Hi,

the main goal of the patchset is to support SPI cnnected device
without CS line link.

The first two patches make the s3c64xx driver to consider the
case of a disconnected CS line. This is done by adding a property
in the DTS ("no-cs-readback") which informs the device driver the
absence of a chip selection link.

The last three patches are just some code re-work and
beautification.

Changelog: V1 -> V2

 - the first version of the patchset was removing an 'if'
   statement on "!spi->chip_select" which was causing the SPI
   core to fail. In this case the drivers would have been able to
   set a chip_select = 0 in absence of a CS link. After a short
   discussion with Mark, this has been replaced, as described
   above, by a property in the DTS.

 - one more patch has been added which assigns to some variable
   the proper type

 - some typos fixed in the commit messages

Thanks,
Andi

Andi Shyti (5):
  spi: s3c64xx: group the CS signalling writes in a single function
  spi: s3c64xx: consider the case when the CS line is not connected
  spi: s3c64xx: do not configure the device twice
  spi: s3c64xx: simplify if statement in prepare_transfer function
  spi: s3c64xx: use unsigned type for fifo handling variables

 .../devicetree/bindings/spi/spi-samsung.txt        |   3 +
 drivers/spi/spi-s3c64xx.c                          | 114 +++++++++++----------
 include/linux/platform_data/spi-s3c64xx.h          |   1 +
 3 files changed, 65 insertions(+), 53 deletions(-)

-- 
2.8.1

[PATCH v2 0/5] s3c64xx: consider the case of a disconnected CS line and some code rework

[Andi Shyti]

Hi,

the main goal of the patchset is to support SPI cnnected device
without CS line link.

The first two patches make the s3c64xx driver to consider the
case of a disconnected CS line. This is done by adding a property
in the DTS ("no-cs-readback") which informs the device driver the
absence of a chip selection link.

The last three patches are just some code re-work and
beautification.

Changelog: V1 -> V2

 - the first version of the patchset was removing an 'if'
   statement on "!spi->chip_select" which was causing the SPI
   core to fail. In this case the drivers would have been able to
   set a chip_select = 0 in absence of a CS link. After a short
   discussion with Mark, this has been replaced, as described
   above, by a property in the DTS.

 - one more patch has been added which assigns to some variable
   the proper type

 - some typos fixed in the commit messages

Thanks,
Andi

Andi Shyti (5):
  spi: s3c64xx: group the CS signalling writes in a single function
  spi: s3c64xx: consider the case when the CS line is not connected
  spi: s3c64xx: do not configure the device twice
  spi: s3c64xx: simplify if statement in prepare_transfer function
  spi: s3c64xx: use unsigned type for fifo handling variables

 .../devicetree/bindings/spi/spi-samsung.txt        |   3 +
 drivers/spi/spi-s3c64xx.c                          | 114 +++++++++++----------
 include/linux/platform_data/spi-s3c64xx.h          |   1 +
 3 files changed, 65 insertions(+), 53 deletions(-)

-- 
2.8.1

[PATCH v2 1/5] spi: s3c64xx: group the CS signalling writes in a single function

[Andi Shyti]

To enable/disable the CS line, the driver performs a writel in
the S3C64XX_SPI_SLAVE_SEL registers. Group the register's
configuration in a single function.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 drivers/spi/spi-s3c64xx.c | 36 ++++++++++++++++++++++++++----------
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 5a76a50..972367d 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -310,6 +310,28 @@ static void prepare_dma(struct s3c64xx_spi_dma_data *dma,
 	dma_async_issue_pending(dma->ch);
 }
 
+static void s3c64xx_spi_set_cs(struct spi_device *spi, bool enable)
+{
+	struct s3c64xx_spi_driver_data *sdd =
+					spi_master_get_devdata(spi->master);
+
+	if (enable) {
+		if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO)) {
+			writel(0, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
+		} else {
+			u32 ssel = readl(sdd->regs + S3C64XX_SPI_SLAVE_SEL);
+
+			ssel |= (S3C64XX_SPI_SLAVE_AUTO |
+						S3C64XX_SPI_SLAVE_NSC_CNT_2);
+			writel(ssel, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
+		}
+	} else {
+		if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO))
+		writel(S3C64XX_SPI_SLAVE_SIG_INACT,
+		sdd->regs + S3C64XX_SPI_SLAVE_SEL);
+	}
+}
+
 static int s3c64xx_spi_prepare_transfer(struct spi_master *spi)
 {
 	struct s3c64xx_spi_driver_data *sdd = spi_master_get_devdata(spi);
@@ -706,12 +728,7 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
 	enable_datapath(sdd, spi, xfer, use_dma);
 
 	/* Start the signals */
-	if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO))
-		writel(0, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
-	else
-		writel(readl(sdd->regs + S3C64XX_SPI_SLAVE_SEL)
-			| S3C64XX_SPI_SLAVE_AUTO | S3C64XX_SPI_SLAVE_NSC_CNT_2,
-			sdd->regs + S3C64XX_SPI_SLAVE_SEL);
+	s3c64xx_spi_set_cs(spi, true);
 
 	spin_unlock_irqrestore(&sdd->lock, flags);
 
@@ -861,16 +878,15 @@ static int s3c64xx_spi_setup(struct spi_device *spi)
 
 	pm_runtime_mark_last_busy(&sdd->pdev->dev);
 	pm_runtime_put_autosuspend(&sdd->pdev->dev);
-	if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO))
-		writel(S3C64XX_SPI_SLAVE_SIG_INACT, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
+	s3c64xx_spi_set_cs(spi, false);
+
 	return 0;
 
 setup_exit:
 	pm_runtime_mark_last_busy(&sdd->pdev->dev);
 	pm_runtime_put_autosuspend(&sdd->pdev->dev);
 	/* setup() returns with device de-selected */
-	if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO))
-		writel(S3C64XX_SPI_SLAVE_SIG_INACT, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
+	s3c64xx_spi_set_cs(spi, false);
 
 	if (gpio_is_valid(spi->cs_gpio))
 		gpio_free(spi->cs_gpio);
-- 
2.8.1

[PATCH v2 1/5] spi: s3c64xx: group the CS signalling writes in a single function

[Andi Shyti]

To enable/disable the CS line, the driver performs a writel in
the S3C64XX_SPI_SLAVE_SEL registers. Group the register's
configuration in a single function.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 drivers/spi/spi-s3c64xx.c | 36 ++++++++++++++++++++++++++----------
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 5a76a50..972367d 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -310,6 +310,28 @@ static void prepare_dma(struct s3c64xx_spi_dma_data *dma,
 	dma_async_issue_pending(dma->ch);
 }
 
+static void s3c64xx_spi_set_cs(struct spi_device *spi, bool enable)
+{
+	struct s3c64xx_spi_driver_data *sdd =
+					spi_master_get_devdata(spi->master);
+
+	if (enable) {
+		if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO)) {
+			writel(0, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
+		} else {
+			u32 ssel = readl(sdd->regs + S3C64XX_SPI_SLAVE_SEL);
+
+			ssel |= (S3C64XX_SPI_SLAVE_AUTO |
+						S3C64XX_SPI_SLAVE_NSC_CNT_2);
+			writel(ssel, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
+		}
+	} else {
+		if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO))
+		writel(S3C64XX_SPI_SLAVE_SIG_INACT,
+		sdd->regs + S3C64XX_SPI_SLAVE_SEL);
+	}
+}
+
 static int s3c64xx_spi_prepare_transfer(struct spi_master *spi)
 {
 	struct s3c64xx_spi_driver_data *sdd = spi_master_get_devdata(spi);
@@ -706,12 +728,7 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
 	enable_datapath(sdd, spi, xfer, use_dma);
 
 	/* Start the signals */
-	if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO))
-		writel(0, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
-	else
-		writel(readl(sdd->regs + S3C64XX_SPI_SLAVE_SEL)
-			| S3C64XX_SPI_SLAVE_AUTO | S3C64XX_SPI_SLAVE_NSC_CNT_2,
-			sdd->regs + S3C64XX_SPI_SLAVE_SEL);
+	s3c64xx_spi_set_cs(spi, true);
 
 	spin_unlock_irqrestore(&sdd->lock, flags);
 
@@ -861,16 +878,15 @@ static int s3c64xx_spi_setup(struct spi_device *spi)
 
 	pm_runtime_mark_last_busy(&sdd->pdev->dev);
 	pm_runtime_put_autosuspend(&sdd->pdev->dev);
-	if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO))
-		writel(S3C64XX_SPI_SLAVE_SIG_INACT, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
+	s3c64xx_spi_set_cs(spi, false);
+
 	return 0;
 
 setup_exit:
 	pm_runtime_mark_last_busy(&sdd->pdev->dev);
 	pm_runtime_put_autosuspend(&sdd->pdev->dev);
 	/* setup() returns with device de-selected */
-	if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO))
-		writel(S3C64XX_SPI_SLAVE_SIG_INACT, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
+	s3c64xx_spi_set_cs(spi, false);
 
 	if (gpio_is_valid(spi->cs_gpio))
 		gpio_free(spi->cs_gpio);
-- 
2.8.1

[PATCH v2 1/5] spi: s3c64xx: group the CS signalling writes in a single function

[Andi Shyti]

To enable/disable the CS line, the driver performs a writel in
the S3C64XX_SPI_SLAVE_SEL registers. Group the register's
configuration in a single function.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 drivers/spi/spi-s3c64xx.c | 36 ++++++++++++++++++++++++++----------
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 5a76a50..972367d 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -310,6 +310,28 @@ static void prepare_dma(struct s3c64xx_spi_dma_data *dma,
 	dma_async_issue_pending(dma->ch);
 }
 
+static void s3c64xx_spi_set_cs(struct spi_device *spi, bool enable)
+{
+	struct s3c64xx_spi_driver_data *sdd =
+					spi_master_get_devdata(spi->master);
+
+	if (enable) {
+		if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO)) {
+			writel(0, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
+		} else {
+			u32 ssel = readl(sdd->regs + S3C64XX_SPI_SLAVE_SEL);
+
+			ssel |= (S3C64XX_SPI_SLAVE_AUTO |
+						S3C64XX_SPI_SLAVE_NSC_CNT_2);
+			writel(ssel, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
+		}
+	} else {
+		if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO))
+		writel(S3C64XX_SPI_SLAVE_SIG_INACT,
+		sdd->regs + S3C64XX_SPI_SLAVE_SEL);
+	}
+}
+
 static int s3c64xx_spi_prepare_transfer(struct spi_master *spi)
 {
 	struct s3c64xx_spi_driver_data *sdd = spi_master_get_devdata(spi);
@@ -706,12 +728,7 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
 	enable_datapath(sdd, spi, xfer, use_dma);
 
 	/* Start the signals */
-	if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO))
-		writel(0, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
-	else
-		writel(readl(sdd->regs + S3C64XX_SPI_SLAVE_SEL)
-			| S3C64XX_SPI_SLAVE_AUTO | S3C64XX_SPI_SLAVE_NSC_CNT_2,
-			sdd->regs + S3C64XX_SPI_SLAVE_SEL);
+	s3c64xx_spi_set_cs(spi, true);
 
 	spin_unlock_irqrestore(&sdd->lock, flags);
 
@@ -861,16 +878,15 @@ static int s3c64xx_spi_setup(struct spi_device *spi)
 
 	pm_runtime_mark_last_busy(&sdd->pdev->dev);
 	pm_runtime_put_autosuspend(&sdd->pdev->dev);
-	if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO))
-		writel(S3C64XX_SPI_SLAVE_SIG_INACT, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
+	s3c64xx_spi_set_cs(spi, false);
+
 	return 0;
 
 setup_exit:
 	pm_runtime_mark_last_busy(&sdd->pdev->dev);
 	pm_runtime_put_autosuspend(&sdd->pdev->dev);
 	/* setup() returns with device de-selected */
-	if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO))
-		writel(S3C64XX_SPI_SLAVE_SIG_INACT, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
+	s3c64xx_spi_set_cs(spi, false);
 
 	if (gpio_is_valid(spi->cs_gpio))
 		gpio_free(spi->cs_gpio);
-- 
2.8.1

[PATCH v2 2/5] spi: s3c64xx: consider the case when the CS line is not connected

[Andi Shyti]

When the CS line is not connected, it is not needed to enable or
disable the chip selection functionality from the s3c64xx
devices in order to perform a transfer.
Set the CS controller logically always enabled already during
initialization (by writing '0' in the S3C64XX_SPI_SLAVE_SEL
register) and never disable it.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 Documentation/devicetree/bindings/spi/spi-samsung.txt | 3 +++
 drivers/spi/spi-s3c64xx.c                             | 9 ++++++++-
 include/linux/platform_data/spi-s3c64xx.h             | 1 +
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/spi/spi-samsung.txt b/Documentation/devicetree/bindings/spi/spi-samsung.txt
index 6dbdeb3..930bc73 100644
--- a/Documentation/devicetree/bindings/spi/spi-samsung.txt
+++ b/Documentation/devicetree/bindings/spi/spi-samsung.txt
@@ -40,6 +40,9 @@ Optional Board Specific Properties:
 
 - cs-gpios: should specify GPIOs used for chipselects (see spi-bus.txt)
 
+- no-cs-readback: the CS line is disconnected, therefore the device should not
+  operate based on CS signalling.
+
 SPI Controller specific data in SPI slave nodes:
 
 - The spi slave nodes should provide the following information which is required
diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 972367d..14269b0 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -315,6 +315,9 @@ static void s3c64xx_spi_set_cs(struct spi_device *spi, bool enable)
 	struct s3c64xx_spi_driver_data *sdd =
 					spi_master_get_devdata(spi->master);
 
+	if (sdd->cntrlr_info->no_cs)
+		return;
+
 	if (enable) {
 		if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO)) {
 			writel(0, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
@@ -960,7 +963,9 @@ static void s3c64xx_spi_hwinit(struct s3c64xx_spi_driver_data *sdd, int channel)
 
 	sdd->cur_speed = 0;
 
-	if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO))
+	if (sci->no_cs)
+		writel(0, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
+	else if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO))
 		writel(S3C64XX_SPI_SLAVE_SIG_INACT, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
 
 	/* Disable Interrupts - we use Polling if not DMA mode */
@@ -1015,6 +1020,8 @@ static struct s3c64xx_spi_info *s3c64xx_spi_parse_dt(struct device *dev)
 		sci->num_cs = temp;
 	}
 
+	sci->no_cs = of_property_read_bool(dev->of_node, "broken-cs");
+
 	return sci;
 }
 #else
diff --git a/include/linux/platform_data/spi-s3c64xx.h b/include/linux/platform_data/spi-s3c64xx.h
index fb5625b..5c1e21c 100644
--- a/include/linux/platform_data/spi-s3c64xx.h
+++ b/include/linux/platform_data/spi-s3c64xx.h
@@ -38,6 +38,7 @@ struct s3c64xx_spi_csinfo {
 struct s3c64xx_spi_info {
 	int src_clk_nr;
 	int num_cs;
+	bool no_cs;
 	int (*cfg_gpio)(void);
 	dma_filter_fn filter;
 	void *dma_tx;
-- 
2.8.1

[PATCH v2 2/5] spi: s3c64xx: consider the case when the CS line is not connected

[Andi Shyti]

When the CS line is not connected, it is not needed to enable or
disable the chip selection functionality from the s3c64xx
devices in order to perform a transfer.
Set the CS controller logically always enabled already during
initialization (by writing '0' in the S3C64XX_SPI_SLAVE_SEL
register) and never disable it.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 Documentation/devicetree/bindings/spi/spi-samsung.txt | 3 +++
 drivers/spi/spi-s3c64xx.c                             | 9 ++++++++-
 include/linux/platform_data/spi-s3c64xx.h             | 1 +
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/spi/spi-samsung.txt b/Documentation/devicetree/bindings/spi/spi-samsung.txt
index 6dbdeb3..930bc73 100644
--- a/Documentation/devicetree/bindings/spi/spi-samsung.txt
+++ b/Documentation/devicetree/bindings/spi/spi-samsung.txt
@@ -40,6 +40,9 @@ Optional Board Specific Properties:
 
 - cs-gpios: should specify GPIOs used for chipselects (see spi-bus.txt)
 
+- no-cs-readback: the CS line is disconnected, therefore the device should not
+  operate based on CS signalling.
+
 SPI Controller specific data in SPI slave nodes:
 
 - The spi slave nodes should provide the following information which is required
diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 972367d..14269b0 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -315,6 +315,9 @@ static void s3c64xx_spi_set_cs(struct spi_device *spi, bool enable)
 	struct s3c64xx_spi_driver_data *sdd =
 					spi_master_get_devdata(spi->master);
 
+	if (sdd->cntrlr_info->no_cs)
+		return;
+
 	if (enable) {
 		if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO)) {
 			writel(0, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
@@ -960,7 +963,9 @@ static void s3c64xx_spi_hwinit(struct s3c64xx_spi_driver_data *sdd, int channel)
 
 	sdd->cur_speed = 0;
 
-	if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO))
+	if (sci->no_cs)
+		writel(0, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
+	else if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO))
 		writel(S3C64XX_SPI_SLAVE_SIG_INACT, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
 
 	/* Disable Interrupts - we use Polling if not DMA mode */
@@ -1015,6 +1020,8 @@ static struct s3c64xx_spi_info *s3c64xx_spi_parse_dt(struct device *dev)
 		sci->num_cs = temp;
 	}
 
+	sci->no_cs = of_property_read_bool(dev->of_node, "broken-cs");
+
 	return sci;
 }
 #else
diff --git a/include/linux/platform_data/spi-s3c64xx.h b/include/linux/platform_data/spi-s3c64xx.h
index fb5625b..5c1e21c 100644
--- a/include/linux/platform_data/spi-s3c64xx.h
+++ b/include/linux/platform_data/spi-s3c64xx.h
@@ -38,6 +38,7 @@ struct s3c64xx_spi_csinfo {
 struct s3c64xx_spi_info {
 	int src_clk_nr;
 	int num_cs;
+	bool no_cs;
 	int (*cfg_gpio)(void);
 	dma_filter_fn filter;
 	void *dma_tx;
-- 
2.8.1

[PATCH v2 2/5] spi: s3c64xx: consider the case when the CS line is not connected

[Andi Shyti]

When the CS line is not connected, it is not needed to enable or
disable the chip selection functionality from the s3c64xx
devices in order to perform a transfer.
Set the CS controller logically always enabled already during
initialization (by writing '0' in the S3C64XX_SPI_SLAVE_SEL
register) and never disable it.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 Documentation/devicetree/bindings/spi/spi-samsung.txt | 3 +++
 drivers/spi/spi-s3c64xx.c                             | 9 ++++++++-
 include/linux/platform_data/spi-s3c64xx.h             | 1 +
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/spi/spi-samsung.txt b/Documentation/devicetree/bindings/spi/spi-samsung.txt
index 6dbdeb3..930bc73 100644
--- a/Documentation/devicetree/bindings/spi/spi-samsung.txt
+++ b/Documentation/devicetree/bindings/spi/spi-samsung.txt
@@ -40,6 +40,9 @@ Optional Board Specific Properties:
 
 - cs-gpios: should specify GPIOs used for chipselects (see spi-bus.txt)
 
+- no-cs-readback: the CS line is disconnected, therefore the device should not
+  operate based on CS signalling.
+
 SPI Controller specific data in SPI slave nodes:
 
 - The spi slave nodes should provide the following information which is required
diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 972367d..14269b0 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -315,6 +315,9 @@ static void s3c64xx_spi_set_cs(struct spi_device *spi, bool enable)
 	struct s3c64xx_spi_driver_data *sdd =
 					spi_master_get_devdata(spi->master);
 
+	if (sdd->cntrlr_info->no_cs)
+		return;
+
 	if (enable) {
 		if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO)) {
 			writel(0, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
@@ -960,7 +963,9 @@ static void s3c64xx_spi_hwinit(struct s3c64xx_spi_driver_data *sdd, int channel)
 
 	sdd->cur_speed = 0;
 
-	if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO))
+	if (sci->no_cs)
+		writel(0, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
+	else if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO))
 		writel(S3C64XX_SPI_SLAVE_SIG_INACT, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
 
 	/* Disable Interrupts - we use Polling if not DMA mode */
@@ -1015,6 +1020,8 @@ static struct s3c64xx_spi_info *s3c64xx_spi_parse_dt(struct device *dev)
 		sci->num_cs = temp;
 	}
 
+	sci->no_cs = of_property_read_bool(dev->of_node, "broken-cs");
+
 	return sci;
 }
 #else
diff --git a/include/linux/platform_data/spi-s3c64xx.h b/include/linux/platform_data/spi-s3c64xx.h
index fb5625b..5c1e21c 100644
--- a/include/linux/platform_data/spi-s3c64xx.h
+++ b/include/linux/platform_data/spi-s3c64xx.h
@@ -38,6 +38,7 @@ struct s3c64xx_spi_csinfo {
 struct s3c64xx_spi_info {
 	int src_clk_nr;
 	int num_cs;
+	bool no_cs;
 	int (*cfg_gpio)(void);
 	dma_filter_fn filter;
 	void *dma_tx;
-- 
2.8.1

Applied "spi: s3c64xx: consider the case when the CS line is not connected" to the spi tree

[Mark Brown]

The patch

   spi: s3c64xx: consider the case when the CS line is not connected

has been applied to the spi tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From a92e7c3d82a1313ab1954e5cdfd8f04efdb4ca78 Mon Sep 17 00:00:00 2001
From: Andi Shyti <andi.shyti@samsung.com>
Date: Tue, 28 Jun 2016 11:41:12 +0900
Subject: [PATCH] spi: s3c64xx: consider the case when the CS line is not
 connected

When the CS line is not connected, it is not needed to enable or
disable the chip selection functionality from the s3c64xx
devices in order to perform a transfer.
Set the CS controller logically always enabled already during
initialization (by writing '0' in the S3C64XX_SPI_SLAVE_SEL
register) and never disable it.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 Documentation/devicetree/bindings/spi/spi-samsung.txt | 3 +++
 drivers/spi/spi-s3c64xx.c                             | 9 ++++++++-
 include/linux/platform_data/spi-s3c64xx.h             | 1 +
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/spi/spi-samsung.txt b/Documentation/devicetree/bindings/spi/spi-samsung.txt
index 6dbdeb3c361a..930bc7349271 100644
--- a/Documentation/devicetree/bindings/spi/spi-samsung.txt
+++ b/Documentation/devicetree/bindings/spi/spi-samsung.txt
@@ -40,6 +40,9 @@ Optional Board Specific Properties:
 
 - cs-gpios: should specify GPIOs used for chipselects (see spi-bus.txt)
 
+- no-cs-readback: the CS line is disconnected, therefore the device should not
+  operate based on CS signalling.
+
 SPI Controller specific data in SPI slave nodes:
 
 - The spi slave nodes should provide the following information which is required
diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 972367d5c7f2..14269b07804c 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -315,6 +315,9 @@ static void s3c64xx_spi_set_cs(struct spi_device *spi, bool enable)
 	struct s3c64xx_spi_driver_data *sdd =
 					spi_master_get_devdata(spi->master);
 
+	if (sdd->cntrlr_info->no_cs)
+		return;
+
 	if (enable) {
 		if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO)) {
 			writel(0, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
@@ -960,7 +963,9 @@ static void s3c64xx_spi_hwinit(struct s3c64xx_spi_driver_data *sdd, int channel)
 
 	sdd->cur_speed = 0;
 
-	if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO))
+	if (sci->no_cs)
+		writel(0, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
+	else if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO))
 		writel(S3C64XX_SPI_SLAVE_SIG_INACT, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
 
 	/* Disable Interrupts - we use Polling if not DMA mode */
@@ -1015,6 +1020,8 @@ static struct s3c64xx_spi_info *s3c64xx_spi_parse_dt(struct device *dev)
 		sci->num_cs = temp;
 	}
 
+	sci->no_cs = of_property_read_bool(dev->of_node, "broken-cs");
+
 	return sci;
 }
 #else
diff --git a/include/linux/platform_data/spi-s3c64xx.h b/include/linux/platform_data/spi-s3c64xx.h
index fb5625bcca9a..5c1e21c87270 100644
--- a/include/linux/platform_data/spi-s3c64xx.h
+++ b/include/linux/platform_data/spi-s3c64xx.h
@@ -38,6 +38,7 @@ struct s3c64xx_spi_csinfo {
 struct s3c64xx_spi_info {
 	int src_clk_nr;
 	int num_cs;
+	bool no_cs;
 	int (*cfg_gpio)(void);
 	dma_filter_fn filter;
 	void *dma_tx;
-- 
2.8.1

Applied "spi: s3c64xx: consider the case when the CS line is not connected" to the spi tree

[Mark Brown]

The patch

   spi: s3c64xx: consider the case when the CS line is not connected

has been applied to the spi tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From a92e7c3d82a1313ab1954e5cdfd8f04efdb4ca78 Mon Sep 17 00:00:00 2001
From: Andi Shyti <andi.shyti@samsung.com>
Date: Tue, 28 Jun 2016 11:41:12 +0900
Subject: [PATCH] spi: s3c64xx: consider the case when the CS line is not
 connected

When the CS line is not connected, it is not needed to enable or
disable the chip selection functionality from the s3c64xx
devices in order to perform a transfer.
Set the CS controller logically always enabled already during
initialization (by writing '0' in the S3C64XX_SPI_SLAVE_SEL
register) and never disable it.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 Documentation/devicetree/bindings/spi/spi-samsung.txt | 3 +++
 drivers/spi/spi-s3c64xx.c                             | 9 ++++++++-
 include/linux/platform_data/spi-s3c64xx.h             | 1 +
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/spi/spi-samsung.txt b/Documentation/devicetree/bindings/spi/spi-samsung.txt
index 6dbdeb3c361a..930bc7349271 100644
--- a/Documentation/devicetree/bindings/spi/spi-samsung.txt
+++ b/Documentation/devicetree/bindings/spi/spi-samsung.txt
@@ -40,6 +40,9 @@ Optional Board Specific Properties:
 
 - cs-gpios: should specify GPIOs used for chipselects (see spi-bus.txt)
 
+- no-cs-readback: the CS line is disconnected, therefore the device should not
+  operate based on CS signalling.
+
 SPI Controller specific data in SPI slave nodes:
 
 - The spi slave nodes should provide the following information which is required
diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 972367d5c7f2..14269b07804c 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -315,6 +315,9 @@ static void s3c64xx_spi_set_cs(struct spi_device *spi, bool enable)
 	struct s3c64xx_spi_driver_data *sdd =
 					spi_master_get_devdata(spi->master);
 
+	if (sdd->cntrlr_info->no_cs)
+		return;
+
 	if (enable) {
 		if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO)) {
 			writel(0, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
@@ -960,7 +963,9 @@ static void s3c64xx_spi_hwinit(struct s3c64xx_spi_driver_data *sdd, int channel)
 
 	sdd->cur_speed = 0;
 
-	if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO))
+	if (sci->no_cs)
+		writel(0, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
+	else if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO))
 		writel(S3C64XX_SPI_SLAVE_SIG_INACT, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
 
 	/* Disable Interrupts - we use Polling if not DMA mode */
@@ -1015,6 +1020,8 @@ static struct s3c64xx_spi_info *s3c64xx_spi_parse_dt(struct device *dev)
 		sci->num_cs = temp;
 	}
 
+	sci->no_cs = of_property_read_bool(dev->of_node, "broken-cs");
+
 	return sci;
 }
 #else
diff --git a/include/linux/platform_data/spi-s3c64xx.h b/include/linux/platform_data/spi-s3c64xx.h
index fb5625bcca9a..5c1e21c87270 100644
--- a/include/linux/platform_data/spi-s3c64xx.h
+++ b/include/linux/platform_data/spi-s3c64xx.h
@@ -38,6 +38,7 @@ struct s3c64xx_spi_csinfo {
 struct s3c64xx_spi_info {
 	int src_clk_nr;
 	int num_cs;
+	bool no_cs;
 	int (*cfg_gpio)(void);
 	dma_filter_fn filter;
 	void *dma_tx;
-- 
2.8.1

Applied "spi: s3c64xx: consider the case when the CS line is not connected" to the spi tree

[Mark Brown]

The patch

   spi: s3c64xx: consider the case when the CS line is not connected

has been applied to the spi tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From a92e7c3d82a1313ab1954e5cdfd8f04efdb4ca78 Mon Sep 17 00:00:00 2001
From: Andi Shyti <andi.shyti@samsung.com>
Date: Tue, 28 Jun 2016 11:41:12 +0900
Subject: [PATCH] spi: s3c64xx: consider the case when the CS line is not
 connected

When the CS line is not connected, it is not needed to enable or
disable the chip selection functionality from the s3c64xx
devices in order to perform a transfer.
Set the CS controller logically always enabled already during
initialization (by writing '0' in the S3C64XX_SPI_SLAVE_SEL
register) and never disable it.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 Documentation/devicetree/bindings/spi/spi-samsung.txt | 3 +++
 drivers/spi/spi-s3c64xx.c                             | 9 ++++++++-
 include/linux/platform_data/spi-s3c64xx.h             | 1 +
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/spi/spi-samsung.txt b/Documentation/devicetree/bindings/spi/spi-samsung.txt
index 6dbdeb3c361a..930bc7349271 100644
--- a/Documentation/devicetree/bindings/spi/spi-samsung.txt
+++ b/Documentation/devicetree/bindings/spi/spi-samsung.txt
@@ -40,6 +40,9 @@ Optional Board Specific Properties:
 
 - cs-gpios: should specify GPIOs used for chipselects (see spi-bus.txt)
 
+- no-cs-readback: the CS line is disconnected, therefore the device should not
+  operate based on CS signalling.
+
 SPI Controller specific data in SPI slave nodes:
 
 - The spi slave nodes should provide the following information which is required
diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 972367d5c7f2..14269b07804c 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -315,6 +315,9 @@ static void s3c64xx_spi_set_cs(struct spi_device *spi, bool enable)
 	struct s3c64xx_spi_driver_data *sdd =
 					spi_master_get_devdata(spi->master);
 
+	if (sdd->cntrlr_info->no_cs)
+		return;
+
 	if (enable) {
 		if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO)) {
 			writel(0, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
@@ -960,7 +963,9 @@ static void s3c64xx_spi_hwinit(struct s3c64xx_spi_driver_data *sdd, int channel)
 
 	sdd->cur_speed = 0;
 
-	if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO))
+	if (sci->no_cs)
+		writel(0, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
+	else if (!(sdd->port_conf->quirks & S3C64XX_SPI_QUIRK_CS_AUTO))
 		writel(S3C64XX_SPI_SLAVE_SIG_INACT, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
 
 	/* Disable Interrupts - we use Polling if not DMA mode */
@@ -1015,6 +1020,8 @@ static struct s3c64xx_spi_info *s3c64xx_spi_parse_dt(struct device *dev)
 		sci->num_cs = temp;
 	}
 
+	sci->no_cs = of_property_read_bool(dev->of_node, "broken-cs");
+
 	return sci;
 }
 #else
diff --git a/include/linux/platform_data/spi-s3c64xx.h b/include/linux/platform_data/spi-s3c64xx.h
index fb5625bcca9a..5c1e21c87270 100644
--- a/include/linux/platform_data/spi-s3c64xx.h
+++ b/include/linux/platform_data/spi-s3c64xx.h
@@ -38,6 +38,7 @@ struct s3c64xx_spi_csinfo {
 struct s3c64xx_spi_info {
 	int src_clk_nr;
 	int num_cs;
+	bool no_cs;
 	int (*cfg_gpio)(void);
 	dma_filter_fn filter;
 	void *dma_tx;
-- 
2.8.1

[PATCH v2 3/5] spi: s3c64xx: do not configure the device twice

[Andi Shyti]

At the start of the transfer, the spi_config function is called
twice, the first time when the 3c64xx_spi_prepare_message is
called and the second time with the s3c64xx_spi_transfer_one,
both called from the spi framework.

Remove the first call at the prepare message because in that
point we don't have the imformation about "bit per word" and
frequency.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 drivers/spi/spi-s3c64xx.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 14269b0..c65a9e6 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -676,16 +676,6 @@ static int s3c64xx_spi_prepare_message(struct spi_master *master,
 	struct spi_device *spi = msg->spi;
 	struct s3c64xx_spi_csinfo *cs = spi->controller_data;
 
-	/* If Master's(controller) state differs from that needed by Slave */
-	if (sdd->cur_speed != spi->max_speed_hz
-			|| sdd->cur_mode != spi->mode
-			|| sdd->cur_bpw != spi->bits_per_word) {
-		sdd->cur_bpw = spi->bits_per_word;
-		sdd->cur_speed = spi->max_speed_hz;
-		sdd->cur_mode = spi->mode;
-		s3c64xx_spi_config(sdd);
-	}
-
 	/* Configure feedback delay */
 	writel(cs->fb_delay & 0x3, sdd->regs + S3C64XX_SPI_FB_CLK);
 
@@ -712,6 +702,7 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
 	if (bpw != sdd->cur_bpw || speed != sdd->cur_speed) {
 		sdd->cur_bpw = bpw;
 		sdd->cur_speed = speed;
+		sdd->cur_mode = spi->mode;
 		s3c64xx_spi_config(sdd);
 	}
 
-- 
2.8.1

[PATCH v2 3/5] spi: s3c64xx: do not configure the device twice

[Andi Shyti]

At the start of the transfer, the spi_config function is called
twice, the first time when the 3c64xx_spi_prepare_message is
called and the second time with the s3c64xx_spi_transfer_one,
both called from the spi framework.

Remove the first call at the prepare message because in that
point we don't have the imformation about "bit per word" and
frequency.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 drivers/spi/spi-s3c64xx.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 14269b0..c65a9e6 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -676,16 +676,6 @@ static int s3c64xx_spi_prepare_message(struct spi_master *master,
 	struct spi_device *spi = msg->spi;
 	struct s3c64xx_spi_csinfo *cs = spi->controller_data;
 
-	/* If Master's(controller) state differs from that needed by Slave */
-	if (sdd->cur_speed != spi->max_speed_hz
-			|| sdd->cur_mode != spi->mode
-			|| sdd->cur_bpw != spi->bits_per_word) {
-		sdd->cur_bpw = spi->bits_per_word;
-		sdd->cur_speed = spi->max_speed_hz;
-		sdd->cur_mode = spi->mode;
-		s3c64xx_spi_config(sdd);
-	}
-
 	/* Configure feedback delay */
 	writel(cs->fb_delay & 0x3, sdd->regs + S3C64XX_SPI_FB_CLK);
 
@@ -712,6 +702,7 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
 	if (bpw != sdd->cur_bpw || speed != sdd->cur_speed) {
 		sdd->cur_bpw = bpw;
 		sdd->cur_speed = speed;
+		sdd->cur_mode = spi->mode;
 		s3c64xx_spi_config(sdd);
 	}
 
-- 
2.8.1

[PATCH v2 4/5] spi: s3c64xx: simplify if statement in prepare_transfer function

[Andi Shyti]

The whole function is inside an 'if' statement
("!is_polling(sdd)").

Check the opposite of that statement at the beginning and exit,
this way we can have one level less of indentation.

Remove the goto paths as they are redundant.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 drivers/spi/spi-s3c64xx.c | 50 +++++++++++++++++++++--------------------------
 1 file changed, 22 insertions(+), 28 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index c65a9e6..6d8486f 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -341,38 +341,32 @@ static int s3c64xx_spi_prepare_transfer(struct spi_master *spi)
 	dma_filter_fn filter = sdd->cntrlr_info->filter;
 	struct device *dev = &sdd->pdev->dev;
 	dma_cap_mask_t mask;
-	int ret;
 
-	if (!is_polling(sdd)) {
-		dma_cap_zero(mask);
-		dma_cap_set(DMA_SLAVE, mask);
-
-		/* Acquire DMA channels */
-		sdd->rx_dma.ch = dma_request_slave_channel_compat(mask, filter,
-				   sdd->cntrlr_info->dma_rx, dev, "rx");
-		if (!sdd->rx_dma.ch) {
-			dev_err(dev, "Failed to get RX DMA channel\n");
-			ret = -EBUSY;
-			goto out;
-		}
-		spi->dma_rx = sdd->rx_dma.ch;
-
-		sdd->tx_dma.ch = dma_request_slave_channel_compat(mask, filter,
-				   sdd->cntrlr_info->dma_tx, dev, "tx");
-		if (!sdd->tx_dma.ch) {
-			dev_err(dev, "Failed to get TX DMA channel\n");
-			ret = -EBUSY;
-			goto out_rx;
-		}
-		spi->dma_tx = sdd->tx_dma.ch;
+	if (is_polling(sdd))
+		return 0;
+
+	dma_cap_zero(mask);
+	dma_cap_set(DMA_SLAVE, mask);
+
+	/* Acquire DMA channels */
+	sdd->rx_dma.ch = dma_request_slave_channel_compat(mask, filter,
+			   sdd->cntrlr_info->dma_rx, dev, "rx");
+	if (!sdd->rx_dma.ch) {
+		dev_err(dev, "Failed to get RX DMA channel\n");
+		return -EBUSY;
 	}
+	spi->dma_rx = sdd->rx_dma.ch;
 
-	return 0;
+	sdd->tx_dma.ch = dma_request_slave_channel_compat(mask, filter,
+			   sdd->cntrlr_info->dma_tx, dev, "tx");
+	if (!sdd->tx_dma.ch) {
+		dev_err(dev, "Failed to get TX DMA channel\n");
+		dma_release_channel(sdd->rx_dma.ch);
+		return -EBUSY;
+	}
+	spi->dma_tx = sdd->tx_dma.ch;
 
-out_rx:
-	dma_release_channel(sdd->rx_dma.ch);
-out:
-	return ret;
+	return 0;
 }
 
 static int s3c64xx_spi_unprepare_transfer(struct spi_master *spi)
-- 
2.8.1

[PATCH v2 4/5] spi: s3c64xx: simplify if statement in prepare_transfer function

[Andi Shyti]

The whole function is inside an 'if' statement
("!is_polling(sdd)").

Check the opposite of that statement at the beginning and exit,
this way we can have one level less of indentation.

Remove the goto paths as they are redundant.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 drivers/spi/spi-s3c64xx.c | 50 +++++++++++++++++++++--------------------------
 1 file changed, 22 insertions(+), 28 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index c65a9e6..6d8486f 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -341,38 +341,32 @@ static int s3c64xx_spi_prepare_transfer(struct spi_master *spi)
 	dma_filter_fn filter = sdd->cntrlr_info->filter;
 	struct device *dev = &sdd->pdev->dev;
 	dma_cap_mask_t mask;
-	int ret;
 
-	if (!is_polling(sdd)) {
-		dma_cap_zero(mask);
-		dma_cap_set(DMA_SLAVE, mask);
-
-		/* Acquire DMA channels */
-		sdd->rx_dma.ch = dma_request_slave_channel_compat(mask, filter,
-				   sdd->cntrlr_info->dma_rx, dev, "rx");
-		if (!sdd->rx_dma.ch) {
-			dev_err(dev, "Failed to get RX DMA channel\n");
-			ret = -EBUSY;
-			goto out;
-		}
-		spi->dma_rx = sdd->rx_dma.ch;
-
-		sdd->tx_dma.ch = dma_request_slave_channel_compat(mask, filter,
-				   sdd->cntrlr_info->dma_tx, dev, "tx");
-		if (!sdd->tx_dma.ch) {
-			dev_err(dev, "Failed to get TX DMA channel\n");
-			ret = -EBUSY;
-			goto out_rx;
-		}
-		spi->dma_tx = sdd->tx_dma.ch;
+	if (is_polling(sdd))
+		return 0;
+
+	dma_cap_zero(mask);
+	dma_cap_set(DMA_SLAVE, mask);
+
+	/* Acquire DMA channels */
+	sdd->rx_dma.ch = dma_request_slave_channel_compat(mask, filter,
+			   sdd->cntrlr_info->dma_rx, dev, "rx");
+	if (!sdd->rx_dma.ch) {
+		dev_err(dev, "Failed to get RX DMA channel\n");
+		return -EBUSY;
 	}
+	spi->dma_rx = sdd->rx_dma.ch;
 
-	return 0;
+	sdd->tx_dma.ch = dma_request_slave_channel_compat(mask, filter,
+			   sdd->cntrlr_info->dma_tx, dev, "tx");
+	if (!sdd->tx_dma.ch) {
+		dev_err(dev, "Failed to get TX DMA channel\n");
+		dma_release_channel(sdd->rx_dma.ch);
+		return -EBUSY;
+	}
+	spi->dma_tx = sdd->tx_dma.ch;
 
-out_rx:
-	dma_release_channel(sdd->rx_dma.ch);
-out:
-	return ret;
+	return 0;
 }
 
 static int s3c64xx_spi_unprepare_transfer(struct spi_master *spi)
-- 
2.8.1

[PATCH v2 5/5] spi: s3c64xx: use unsigned type for fifo handling variables

[Andi Shyti]

The 'quirks' variable cannot ever be negative, therefore use u8
instead of int. The 8 bit size is given from the fact that
currently the quirks variable has very few statuses.

The rx_lvl_offset and tx_st_done store shift values, so that u8
is a proper size.

fifo_lvl_mask stores a series of masks, to be in we will keep the
32 bit size.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
 drivers/spi/spi-s3c64xx.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 6d8486f..6c9503a 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -150,10 +150,10 @@ struct s3c64xx_spi_dma_data {
  * which is provided as driver data to the driver.
  */
 struct s3c64xx_spi_port_config {
-	int	fifo_lvl_mask[MAX_SPI_PORTS];
-	int	rx_lvl_offset;
-	int	tx_st_done;
-	int	quirks;
+	u32	fifo_lvl_mask[MAX_SPI_PORTS];
+	u8	rx_lvl_offset;
+	u8	tx_st_done;
+	u8	quirks;
 	bool	high_speed;
 	bool	clk_from_cmu;
 };
-- 
2.8.1

[PATCH v2 5/5] spi: s3c64xx: use unsigned type for fifo handling variables

[Andi Shyti]

The 'quirks' variable cannot ever be negative, therefore use u8
instead of int. The 8 bit size is given from the fact that
currently the quirks variable has very few statuses.

The rx_lvl_offset and tx_st_done store shift values, so that u8
is a proper size.

fifo_lvl_mask stores a series of masks, to be in we will keep the
32 bit size.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
 drivers/spi/spi-s3c64xx.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 6d8486f..6c9503a 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -150,10 +150,10 @@ struct s3c64xx_spi_dma_data {
  * which is provided as driver data to the driver.
  */
 struct s3c64xx_spi_port_config {
-	int	fifo_lvl_mask[MAX_SPI_PORTS];
-	int	rx_lvl_offset;
-	int	tx_st_done;
-	int	quirks;
+	u32	fifo_lvl_mask[MAX_SPI_PORTS];
+	u8	rx_lvl_offset;
+	u8	tx_st_done;
+	u8	quirks;
 	bool	high_speed;
 	bool	clk_from_cmu;
 };
-- 
2.8.1

[PATCH v2 5/5] spi: s3c64xx: use unsigned type for fifo handling variables

[Andi Shyti]

The 'quirks' variable cannot ever be negative, therefore use u8
instead of int. The 8 bit size is given from the fact that
currently the quirks variable has very few statuses.

The rx_lvl_offset and tx_st_done store shift values, so that u8
is a proper size.

fifo_lvl_mask stores a series of masks, to be in we will keep the
32 bit size.

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
---
 drivers/spi/spi-s3c64xx.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 6d8486f..6c9503a 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -150,10 +150,10 @@ struct s3c64xx_spi_dma_data {
  * which is provided as driver data to the driver.
  */
 struct s3c64xx_spi_port_config {
-	int	fifo_lvl_mask[MAX_SPI_PORTS];
-	int	rx_lvl_offset;
-	int	tx_st_done;
-	int	quirks;
+	u32	fifo_lvl_mask[MAX_SPI_PORTS];
+	u8	rx_lvl_offset;
+	u8	tx_st_done;
+	u8	quirks;
 	bool	high_speed;
 	bool	clk_from_cmu;
 };
-- 
2.8.1

Re: [PATCH v2 5/5] spi: s3c64xx: use unsigned type for fifo handling variables

[Mark Brown]

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

On Tue, Jun 28, 2016 at 11:41:15AM +0900, Andi Shyti wrote:
> The 'quirks' variable cannot ever be negative, therefore use u8
> instead of int. The 8 bit size is given from the fact that
> currently the quirks variable has very few statuses.
> 
> The rx_lvl_offset and tx_st_done store shift values, so that u8
> is a proper size.
> 
> fifo_lvl_mask stores a series of masks, to be in we will keep the
> 32 bit size.

What's the advantage of these changes?  This feels like microptimisation
of something that shouldn't be a big performance issue and it's not
always the case that the compiler does something more sensible with
smaller or unsigned types.  I'm not saying don't do it, I'm just saying
that it's not clear to me what the win is.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[PATCH v2 5/5] spi: s3c64xx: use unsigned type for fifo handling variables

[Mark Brown]

On Tue, Jun 28, 2016 at 11:41:15AM +0900, Andi Shyti wrote:
> The 'quirks' variable cannot ever be negative, therefore use u8
> instead of int. The 8 bit size is given from the fact that
> currently the quirks variable has very few statuses.
> 
> The rx_lvl_offset and tx_st_done store shift values, so that u8
> is a proper size.
> 
> fifo_lvl_mask stores a series of masks, to be in we will keep the
> 32 bit size.

What's the advantage of these changes?  This feels like microptimisation
of something that shouldn't be a big performance issue and it's not
always the case that the compiler does something more sensible with
smaller or unsigned types.  I'm not saying don't do it, I'm just saying
that it's not clear to me what the win is.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160705/f7e2e514/attachment.sig>

Re: [PATCH v2 5/5] spi: s3c64xx: use unsigned type for fifo handling variables

[Mark Brown]

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

On Tue, Jun 28, 2016 at 11:41:15AM +0900, Andi Shyti wrote:
> The 'quirks' variable cannot ever be negative, therefore use u8
> instead of int. The 8 bit size is given from the fact that
> currently the quirks variable has very few statuses.
> 
> The rx_lvl_offset and tx_st_done store shift values, so that u8
> is a proper size.
> 
> fifo_lvl_mask stores a series of masks, to be in we will keep the
> 32 bit size.

What's the advantage of these changes?  This feels like microptimisation
of something that shouldn't be a big performance issue and it's not
always the case that the compiler does something more sensible with
smaller or unsigned types.  I'm not saying don't do it, I'm just saying
that it's not clear to me what the win is.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH v2 5/5] spi: s3c64xx: use unsigned type for fifo handling variables

[Andi Shyti]

Hi Mark,

> > The 'quirks' variable cannot ever be negative, therefore use u8
> > instead of int. The 8 bit size is given from the fact that
> > currently the quirks variable has very few statuses.
> > 
> > The rx_lvl_offset and tx_st_done store shift values, so that u8
> > is a proper size.
> > 
> > fifo_lvl_mask stores a series of masks, to be in we will keep the
> > 32 bit size.
> 
> What's the advantage of these changes?  This feels like microptimisation
> of something that shouldn't be a big performance issue and it's not
> always the case that the compiler does something more sensible with
> smaller or unsigned types.  I'm not saying don't do it, I'm just saying
> that it's not clear to me what the win is.

not much indeed. Just something that was a little bothering me
while I was reading through.

Please, feel free to drop it.

Thanks,
Andi

[PATCH v2 5/5] spi: s3c64xx: use unsigned type for fifo handling variables

[Andi Shyti]

Hi Mark,

> > The 'quirks' variable cannot ever be negative, therefore use u8
> > instead of int. The 8 bit size is given from the fact that
> > currently the quirks variable has very few statuses.
> > 
> > The rx_lvl_offset and tx_st_done store shift values, so that u8
> > is a proper size.
> > 
> > fifo_lvl_mask stores a series of masks, to be in we will keep the
> > 32 bit size.
> 
> What's the advantage of these changes?  This feels like microptimisation
> of something that shouldn't be a big performance issue and it's not
> always the case that the compiler does something more sensible with
> smaller or unsigned types.  I'm not saying don't do it, I'm just saying
> that it's not clear to me what the win is.

not much indeed. Just something that was a little bothering me
while I was reading through.

Please, feel free to drop it.

Thanks,
Andi

Re: [PATCH v2 5/5] spi: s3c64xx: use unsigned type for fifo handling variables

[Andi Shyti]

Hi Mark,

> > The 'quirks' variable cannot ever be negative, therefore use u8
> > instead of int. The 8 bit size is given from the fact that
> > currently the quirks variable has very few statuses.
> > 
> > The rx_lvl_offset and tx_st_done store shift values, so that u8
> > is a proper size.
> > 
> > fifo_lvl_mask stores a series of masks, to be in we will keep the
> > 32 bit size.
> 
> What's the advantage of these changes?  This feels like microptimisation
> of something that shouldn't be a big performance issue and it's not
> always the case that the compiler does something more sensible with
> smaller or unsigned types.  I'm not saying don't do it, I'm just saying
> that it's not clear to me what the win is.

not much indeed. Just something that was a little bothering me
while I was reading through.

Please, feel free to drop it.

Thanks,
Andi
--
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