[PATCH 1/2] spi/s3c64xx: Convert to devm_request_and_ioremap()

[PATCH 1/2] spi/s3c64xx: Convert to devm_request_and_ioremap()

[Mark Brown]

Saves some error handling and a small amount of code.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/spi/spi-s3c64xx.c |   25 +------------------------
 1 file changed, 1 insertion(+), 24 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index f4e2341..b7aeb5d 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -1028,19 +1028,7 @@ static int __init s3c64xx_spi_probe(struct platform_device *pdev)
 	/* the spi->mode bits understood by this driver: */
 	master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
 
-	if (request_mem_region(mem_res->start,
-			resource_size(mem_res), pdev->name) == NULL) {
-		dev_err(&pdev->dev, "Req mem region failed\n");
-		ret = -ENXIO;
-		goto err0;
-	}
-
-	sdd->regs = ioremap(mem_res->start, resource_size(mem_res));
-	if (sdd->regs == NULL) {
-		dev_err(&pdev->dev, "Unable to remap IO\n");
-		ret = -ENXIO;
-		goto err1;
-	}
+	sdd->regs = devm_request_and_ioremap(&pdev->dev, mem_res);
 
 	if (sci->cfg_gpio == NULL || sci->cfg_gpio(pdev)) {
 		dev_err(&pdev->dev, "Unable to config gpio\n");
@@ -1124,10 +1112,6 @@ err4:
 	clk_put(sdd->clk);
 err3:
 err2:
-	iounmap((void *) sdd->regs);
-err1:
-	release_mem_region(mem_res->start, resource_size(mem_res));
-err0:
 	platform_set_drvdata(pdev, NULL);
 	spi_master_put(master);
 
@@ -1138,7 +1122,6 @@ static int s3c64xx_spi_remove(struct platform_device *pdev)
 {
 	struct spi_master *master = spi_master_get(platform_get_drvdata(pdev));
 	struct s3c64xx_spi_driver_data *sdd = spi_master_get_devdata(master);
-	struct resource	*mem_res;
 
 	pm_runtime_disable(&pdev->dev);
 
@@ -1154,12 +1137,6 @@ static int s3c64xx_spi_remove(struct platform_device *pdev)
 	clk_disable(sdd->clk);
 	clk_put(sdd->clk);
 
-	iounmap((void *) sdd->regs);
-
-	mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (mem_res != NULL)
-		release_mem_region(mem_res->start, resource_size(mem_res));
-
 	platform_set_drvdata(pdev, NULL);
 	spi_master_put(master);
 
-- 
1.7.10

[PATCH 2/2] spi/s3c64xx: Expand S3C64XX_SPI_{DE,}ACT macros at call sites

[Mark Brown]

They have very few users and they're both just doing a single register
write so the advantage of having the macro is a bit limited. An inline
function might make sense but it's as easy to just do the writes directly.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/spi/spi-s3c64xx.c |   12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index b7aeb5d..3514ef9 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -74,11 +74,6 @@
 #define S3C64XX_SPI_SLAVE_AUTO			(1<<1)
 #define S3C64XX_SPI_SLAVE_SIG_INACT		(1<<0)
 
-#define S3C64XX_SPI_ACT(c) writel(0, (c)->regs + S3C64XX_SPI_SLAVE_SEL)
-
-#define S3C64XX_SPI_DEACT(c) writel(S3C64XX_SPI_SLAVE_SIG_INACT, \
-					(c)->regs + S3C64XX_SPI_SLAVE_SEL)
-
 #define S3C64XX_SPI_INT_TRAILING_EN		(1<<6)
 #define S3C64XX_SPI_INT_RX_OVERRUN_EN		(1<<5)
 #define S3C64XX_SPI_INT_RX_UNDERRUN_EN		(1<<4)
@@ -712,14 +707,15 @@ static int s3c64xx_spi_transfer_one_message(struct spi_master *master,
 		enable_cs(sdd, spi);
 
 		/* Start the signals */
-		S3C64XX_SPI_ACT(sdd);
+		writel(0, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
 
 		spin_unlock_irqrestore(&sdd->lock, flags);
 
 		status = wait_for_xfer(sdd, xfer, use_dma);
 
 		/* Quiese the signals */
-		S3C64XX_SPI_DEACT(sdd);
+		writel(S3C64XX_SPI_SLAVE_SIG_INACT,
+		       sdd->regs + S3C64XX_SPI_SLAVE_SEL);
 
 		if (status) {
 			dev_err(&spi->dev, "I/O Error: "
@@ -923,7 +919,7 @@ static void s3c64xx_spi_hwinit(struct s3c64xx_spi_driver_data *sdd, int channel)
 
 	sdd->cur_speed = 0;
 
-	S3C64XX_SPI_DEACT(sdd);
+	writel(S3C64XX_SPI_SLAVE_SIG_INACT, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
 
 	/* Disable Interrupts - we use Polling if not DMA mode */
 	writel(0, regs + S3C64XX_SPI_INT_EN);
-- 
1.7.10

Re: [PATCH 1/2] spi/s3c64xx: Convert to devm_request_and_ioremap()

[Linus Walleij]

On Thu, Jun 28, 2012 at 3:23 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:

> Saves some error handling and a small amount of code.
>
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Elegant, monsieur.
Acked-by: Linus Walleij <linus.walleij@linaro.org>

I'm starting to wonder if it would not be possible to mass-convert these
using coccinelle.

Yours,
Linus Walleij

Re: [PATCH 2/2] spi/s3c64xx: Expand S3C64XX_SPI_{DE,}ACT macros at call sites

[Linus Walleij]

On Thu, Jun 28, 2012 at 3:23 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:

> They have very few users and they're both just doing a single register
> write so the advantage of having the macro is a bit limited. An inline
> function might make sense but it's as easy to just do the writes directly.
>
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Much clearer.
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 1/2] spi/s3c64xx: Convert to devm_request_and_ioremap()

[Mark Brown]

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

On Thu, Jun 28, 2012 at 08:51:44PM +0200, Linus Walleij wrote:

> I'm starting to wonder if it would not be possible to mass-convert these
> using coccinelle.

Probably, yes.  I'm not actually going through these particularly,
really what I'm doing is looking to find drivers I run on my systems
that might be able to use regmap-mmio and fixing things I find as I
look.

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

RE: [PATCH 2/2] spi/s3c64xx: Expand S3C64XX_SPI_{DE,}ACT macros at call sites

[Kukjin Kim]

Linus Walleij wrote:
> 
> On Thu, Jun 28, 2012 at 3:23 PM, Mark Brown
> <broonie@opensource.wolfsonmicro.com> wrote:
> 
> > They have very few users and they're both just doing a single register
> > write so the advantage of having the macro is a bit limited. An inline
> > function might make sense but it's as easy to just do the writes
> directly.
> >
> > Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> 
> Much clearer.
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> 
Yeah, I agree, looks good to me.

But I think, nobody takes this series...?

So may I take then send this series via samsung tree? Because for samsung
spi dt, the file has been touched in samsung tree so it helps to avoid bad
conflicts.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

Re: [PATCH 2/2] spi/s3c64xx: Expand S3C64XX_SPI_{DE,}ACT macros at call sites

[Mark Brown]

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

On Tue, Jul 17, 2012 at 08:52:20AM +0900, Kukjin Kim wrote:

> So may I take then send this series via samsung tree? Because for samsung
> spi dt, the file has been touched in samsung tree so it helps to avoid bad
> conflicts.

May as well, yes.

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

[PATCH 2/2] spi/s3c64xx: Expand S3C64XX_SPI_{DE,}ACT macros at call sites

[Mark Brown]

They have very few users and they're both just doing a single register
write so the advantage of having the macro is a bit limited. An inline
function might make sense but it's as easy to just do the writes directly.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/spi/spi-s3c64xx.c |   12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index b7aeb5d..3514ef9 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -74,11 +74,6 @@
 #define S3C64XX_SPI_SLAVE_AUTO			(1<<1)
 #define S3C64XX_SPI_SLAVE_SIG_INACT		(1<<0)
 
-#define S3C64XX_SPI_ACT(c) writel(0, (c)->regs + S3C64XX_SPI_SLAVE_SEL)
-
-#define S3C64XX_SPI_DEACT(c) writel(S3C64XX_SPI_SLAVE_SIG_INACT, \
-					(c)->regs + S3C64XX_SPI_SLAVE_SEL)
-
 #define S3C64XX_SPI_INT_TRAILING_EN		(1<<6)
 #define S3C64XX_SPI_INT_RX_OVERRUN_EN		(1<<5)
 #define S3C64XX_SPI_INT_RX_UNDERRUN_EN		(1<<4)
@@ -712,14 +707,15 @@ static int s3c64xx_spi_transfer_one_message(struct spi_master *master,
 		enable_cs(sdd, spi);
 
 		/* Start the signals */
-		S3C64XX_SPI_ACT(sdd);
+		writel(0, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
 
 		spin_unlock_irqrestore(&sdd->lock, flags);
 
 		status = wait_for_xfer(sdd, xfer, use_dma);
 
 		/* Quiese the signals */
-		S3C64XX_SPI_DEACT(sdd);
+		writel(S3C64XX_SPI_SLAVE_SIG_INACT,
+		       sdd->regs + S3C64XX_SPI_SLAVE_SEL);
 
 		if (status) {
 			dev_err(&spi->dev, "I/O Error: "
@@ -923,7 +919,7 @@ static void s3c64xx_spi_hwinit(struct s3c64xx_spi_driver_data *sdd, int channel)
 
 	sdd->cur_speed = 0;
 
-	S3C64XX_SPI_DEACT(sdd);
+	writel(S3C64XX_SPI_SLAVE_SIG_INACT, sdd->regs + S3C64XX_SPI_SLAVE_SEL);
 
 	/* Disable Interrupts - we use Polling if not DMA mode */
 	writel(0, regs + S3C64XX_SPI_INT_EN);
-- 
1.7.10