spi: fsl-(e)spi: simplify cleanup code

PATCH In review Heiner Kallweit 2 participants 8 messages Aug 23, 2015 – Aug 26, 2015 6 messages not in the archive SPI

lore

Share & tools

lore

Heiner Kallweit

author

2015-08-23 12:21 UTC #0 lore raw

PATCH 2/3 spi: fsl-(e)spi: simplify cleanup code

Now that most cleanup is done automatically the remove functions can be significantly simplified.

Signed-off-by Heiner Kallweit <[email protected]>

---
 drivers/spi/spi-fsl-espi.c |  7 +------
 drivers/spi/spi-fsl-lib.c  | 15 ---------------
 drivers/spi/spi-fsl-lib.h  |  3 ---
 drivers/spi/spi-fsl-spi.c  | 18 +++++++-----------
 4 files changed, 8 insertions(+), 35 deletions(-)

drivers/spi/spi-fsl-espi.c +1 -6

index bfd6259..fdd85be 100644
@@ -604,10 +604,6 @@ static irqreturn_t fsl_espi_irq(s32 irq, void *context_data)
 	return ret;
 }
 
-static void fsl_espi_remove(struct mpc8xxx_spi *mspi)
-{
-}
-
 static int fsl_espi_suspend(struct spi_master *master)
 {
 	struct mpc8xxx_spi *mpc8xxx_spi;

@@ -670,7 +666,6 @@ static struct spi_master * fsl_espi_probe(struct device *dev,
 	master->unprepare_transfer_hardware = fsl_espi_suspend;
 
 	mpc8xxx_spi = spi_master_get_devdata(master);
-	mpc8xxx_spi->spi_remove = fsl_espi_remove;
 
 	mpc8xxx_spi->reg_base = devm_ioremap(dev, mem->start,
 					     resource_size(mem));

@@ -805,7 +800,7 @@ err:
 
 static int of_fsl_espi_remove(struct platform_device *dev)
 {
-	return mpc8xxx_spi_remove(&dev->dev);
+	return 0;
 }
 
 #ifdef CONFIG_PM_SLEEP

drivers/spi/spi-fsl-lib.c +0 ~~-15~~

index e50fd06..1e43412 100644
@@ -114,21 +114,6 @@ void mpc8xxx_spi_probe(struct device *dev, struct resource *mem,
 }
 EXPORT_SYMBOL_GPL(mpc8xxx_spi_probe);
 
-int mpc8xxx_spi_remove(struct device *dev)
-{
-	struct mpc8xxx_spi *mpc8xxx_spi;
-	struct spi_master *master;
-
-	master = dev_get_drvdata(dev);
-	mpc8xxx_spi = spi_master_get_devdata(master);
-
-	if (mpc8xxx_spi->spi_remove)
-		mpc8xxx_spi->spi_remove(mpc8xxx_spi);
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(mpc8xxx_spi_remove);
-
 int of_mpc8xxx_spi_probe(struct platform_device *ofdev)
 {
 	struct device *dev = &ofdev->dev;

drivers/spi/spi-fsl-lib.h +0 -3

index 1326a39..84f5dcb 100644
@@ -54,9 +54,6 @@ struct mpc8xxx_spi {
 	void (*get_rx) (u32 rx_data, struct mpc8xxx_spi *);
 	u32(*get_tx) (struct mpc8xxx_spi *);
 
-	/* hooks for different controller driver */
-	void (*spi_remove) (struct mpc8xxx_spi *mspi);
-
 	unsigned int count;
 	unsigned int irq;

drivers/spi/spi-fsl-spi.c +7 ~~-11~~

index fbbab60..b487dab 100644
@@ -559,11 +559,6 @@ static irqreturn_t fsl_spi_irq(s32 irq, void *context_data)
 	return ret;
 }
 
-static void fsl_spi_remove(struct mpc8xxx_spi *mspi)
-{
-	fsl_spi_cpm_free(mspi);
-}
-
 static void fsl_spi_grlib_cs_control(struct spi_device *spi, bool on)
 {
 	struct mpc8xxx_spi *mpc8xxx_spi = spi_master_get_devdata(spi->master);

@@ -630,7 +625,6 @@ static struct spi_master * fsl_spi_probe(struct device *dev,
 	master->transfer_one_message = fsl_spi_do_one_msg;
 
 	mpc8xxx_spi = spi_master_get_devdata(master);
-	mpc8xxx_spi->spi_remove = fsl_spi_remove;
 	mpc8xxx_spi->max_bits_per_word = 32;
 	mpc8xxx_spi->type = fsl_spi_get_type(dev);

@@ -862,11 +856,8 @@ static int of_fsl_spi_remove(struct platform_device *ofdev)
 {
 	struct spi_master *master = platform_get_drvdata(ofdev);
 	struct mpc8xxx_spi *mpc8xxx_spi = spi_master_get_devdata(master);
-	int ret;
 
-	ret = mpc8xxx_spi_remove(&ofdev->dev);
-	if (ret)
-		return ret;
+	fsl_spi_cpm_free(mpc8xxx_spi);
 	if (mpc8xxx_spi->type == TYPE_FSL)
 		of_fsl_spi_free_chipselects(&ofdev->dev);
 	return 0;

@@ -912,7 +903,12 @@ static int plat_mpc8xxx_spi_probe(struct platform_device *pdev)
 
 static int plat_mpc8xxx_spi_remove(struct platform_device *pdev)
 {
-	return mpc8xxx_spi_remove(&pdev->dev);
+	struct spi_master *master = platform_get_drvdata(pdev);
+	struct mpc8xxx_spi *mpc8xxx_spi = spi_master_get_devdata(master);
+
+	fsl_spi_cpm_free(mpc8xxx_spi);
+
+	return 0;
 }
 
 MODULE_ALIAS("platform:mpc8xxx_spi");

Signature · 6 lines

2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Mark Brown

2015-08-25 17:45 UTC #2 lore raw

In reply to a message not in the archive

drivers/spi/spi-fsl-espi.c err: · line 804

Quoted 5 lines · Heiner Kallweit

static int of_fsl_espi_remove(struct platform_device *dev) {
drivers/spi/spi-fsl-espi.c +1 -1
-	return mpc8xxx_spi_remove(&dev->dev);
+	return 0;
 }

You should be able to remove empty functions like this one.

Heiner Kallweit

author

2015-08-25 20:19 UTC #4 lore raw

In reply to a message not in the archive

Am 25.08.2015 um 19:45 schrieb Mark Brown:

Quoted 9 lines · Heiner Kallweit

drivers/spi/spi-fsl-espi.c err: · line 804
Quoted 6 lines
static int of_fsl_espi_remove(struct platform_device *dev) {
drivers/spi/spi-fsl-espi.c +1 -1
-	return mpc8xxx_spi_remove(&dev->dev);
+	return 0;
 }
You should be able to remove empty functions like this one.

In patch 3 a call to pm_runtime_disable is added to this function. Therefore I don't remove it in patch 2. Fine with you?

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Mark Brown

2015-08-26 10:58 UTC #6 lore raw

In reply to a message not in the archive

Quoted 6 lines · Heiner Kallweit

Am 25.08.2015 um 19:45 schrieb Mark Brown:
Quoted 1 line
You should be able to remove empty functions like this one.
In patch 3 a call to pm_runtime_disable is added to this function. Therefore I don't remove it in patch 2. Fine with you?

No, just re-add it when you add users.

Heiner Kallweit

author

2015-08-26 17:56 UTC #8 lore raw

In reply to a message not in the archive

Am 26.08.2015 um 12:58 schrieb Mark Brown:

Quoted 10 lines · Heiner Kallweit

Quoted 6 lines
Am 25.08.2015 um 19:45 schrieb Mark Brown:
Quoted 1 line
You should be able to remove empty functions like this one.
In patch 3 a call to pm_runtime_disable is added to this function. Therefore I don't remove it in patch 2. Fine with you?
No, just re-add it when you add users.

OK, I will consider this when sending v2 of the patch series.

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Heiner Kallweit

author

2015-08-26 18:14 UTC #9 lore raw

In reply to a message not in the archive

Am 25.08.2015 um 19:45 schrieb Mark Brown:

Quoted 10 lines · Heiner Kallweit

drivers/spi/spi-fsl-espi.c err: · line 804
Quoted 6 lines
static int of_fsl_espi_remove(struct platform_device *dev) {
drivers/spi/spi-fsl-espi.c +1 -1
-	return mpc8xxx_spi_remove(&dev->dev);
+	return 0;
 }
You should be able to remove empty functions like this one.

When checking I just recognized that this (now temporarily empty) function is the remove callback of the platform driver. AFAIK providing a remove callback is mandatory.
Is it therefore ok to have a temporarily empty function here?

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Mark Brown

2015-08-26 18:45 UTC #11 lore raw

In reply to a message not in the archive

Quoted 11 lines · Heiner Kallweit

Am 25.08.2015 um 19:45 schrieb Mark Brown:
Quoted 9 lines · Heiner Kallweit
drivers/spi/spi-fsl-espi.c err: · line 804
Quoted 6 lines
static int of_fsl_espi_remove(struct platform_device *dev) {
drivers/spi/spi-fsl-espi.c +1 -1
-	return mpc8xxx_spi_remove(&dev->dev);
+	return 0;
 }
You should be able to remove empty functions like this one.

Quoted 4 lines

When checking I just recognized that this (now temporarily empty) function is the remove callback of the platform driver. AFAIK providing a remove callback is mandatory.
Is it therefore ok to have a temporarily empty function here?

No, remove callbacks are completely optional.

Heiner Kallweit

author

2015-08-26 18:54 UTC #13 lore raw

In reply to a message not in the archive

Am 26.08.2015 um 20:45 schrieb Mark Brown:

Quoted 12 lines · Heiner Kallweit

Quoted 10 lines
Am 25.08.2015 um 19:45 schrieb Mark Brown:
Quoted 9 lines · Heiner Kallweit
Review
Quoted 6 lines
static int of_fsl_espi_remove(struct platform_device *dev) {
(quoted diff) +1 -1
-	return mpc8xxx_spi_remove(&dev->dev);
+	return 0;
 }
You should be able to remove empty functions like this one.

Quoted 7 lines

Quoted 4 lines
When checking I just recognized that this (now temporarily empty) function is the remove callback of the platform driver. AFAIK providing a remove callback is mandatory.
Is it therefore ok to have a temporarily empty function here?
No, remove callbacks are completely optional.

I see, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html