SPI PXA2xx main driver is a compound of a core library and
a platform driver code. Decoupling that allows to eventually
get rid of an additional layer of devices hierarchy in PCI driver.
There are also precursor cleanups.
In v2:
- made better handling of pxa_ssp_free() calls
Andy Shevchenko (11):
spi: pxa2xx: Wrap pxa_ssp_request() to be device managed resource
spi: pxa2xx: Reorganize the SSP type retrieval
spi: pxa2xx: Remove no more needed driver data
spi: pxa2xx: Remove hard coded number of chip select pins
spi: pxa2xx: Utilise temporary variable for struct device
spi: pxa2xx: Print DMA burst size only when DMA is enabled
spi: pxa2xx: Remove duplicate check
spi: pxa2xx: Remove superflous check for Intel Atom SoCs
spi: pxa2xx: Extract pxa2xx_spi_platform_*() callbacks
spi: pxa2xx: Move platform driver to a separate file
spi: pxa2xx: Convert PCI driver to use spi-pxa2xx code directly
drivers/spi/Makefile | 3 +-
drivers/spi/spi-pxa2xx-pci.c | 39 ++---
drivers/spi/spi-pxa2xx-platform.c | 214 +++++++++++++++++++++++++
drivers/spi/spi-pxa2xx.c | 253 +++++-------------------------
drivers/spi/spi-pxa2xx.h | 6 +
5 files changed, 270 insertions(+), 245 deletions(-)
create mode 100644 drivers/spi/spi-pxa2xx-platform.c
--
2.43.0.rc1.1336.g36b5255a03ac
The spi-pxa2xx.c is bloated with a platform driver code while
pretending to provide a core functionality. Make it real core
library by splitting out the platform driver to a separate file.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/spi/Makefile | 3 +-
drivers/spi/spi-pxa2xx-platform.c | 214 +++++++++++++++++++++++++++++
drivers/spi/spi-pxa2xx.c | 215 ++----------------------------
drivers/spi/spi-pxa2xx.h | 6 +
4 files changed, 230 insertions(+), 208 deletions(-)
create mode 100644 drivers/spi/spi-pxa2xx-platform.c
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index e694254dec04..bcfb6efd88e0 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -107,7 +107,8 @@ obj-$(CONFIG_SPI_PIC32) += spi-pic32.o
obj-$(CONFIG_SPI_PIC32_SQI) += spi-pic32-sqi.o
obj-$(CONFIG_SPI_PL022) += spi-pl022.o
obj-$(CONFIG_SPI_PPC4xx) += spi-ppc4xx.o
-spi-pxa2xx-platform-objs := spi-pxa2xx.o spi-pxa2xx-dma.o
+obj-$(CONFIG_SPI_PXA2XX) += spi-pxa2xx-core.o
+spi-pxa2xx-core-y := spi-pxa2xx.o spi-pxa2xx-dma.o
obj-$(CONFIG_SPI_PXA2XX) += spi-pxa2xx-platform.o
obj-$(CONFIG_SPI_PXA2XX_PCI) += spi-pxa2xx-pci.o
obj-$(CONFIG_SPI_QCOM_GENI) += spi-geni-qcom.o
diff --git a/drivers/spi/spi-pxa2xx-platform.c b/drivers/spi/spi-pxa2xx-platform.c
new file mode 100644
index 000000000000..98a8ceb7db6f
--- /dev/null
+++ b/drivers/spi/spi-pxa2xx-platform.c
@@ -0,0 +1,214 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <linux/acpi.h>
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/mod_devicetable.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/types.h>
+
+#include "spi-pxa2xx.h"
+
+static bool pxa2xx_spi_idma_filter(struct dma_chan *chan, void *param)
+{
+ return param == chan->device->dev;
+}
+
+static int
+pxa2xx_spi_init_ssp(struct platform_device *pdev, struct ssp_device *ssp, enum pxa_ssp_type type)
+{
+ struct device *dev = &pdev->dev;
+ struct resource *res;
+ int status;
+ u64 uid;
+
+ ssp->mmio_base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
+ if (IS_ERR(ssp->mmio_base))
+ return PTR_ERR(ssp->mmio_base);
+
+ ssp->phys_base = res->start;
+
+ ssp->clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(ssp->clk))
+ return PTR_ERR(ssp->clk);
+
+ ssp->irq = platform_get_irq(pdev, 0);
+ if (ssp->irq < 0)
+ return ssp->irq;
+
+ ssp->type = type;
+ ssp->dev = dev;
+
+ status = acpi_dev_uid_to_integer(ACPI_COMPANION(dev), &uid);
+ if (status)
+ ssp->port_id = -1;
+ else
+ ssp->port_id = uid;
+
+ return 0;
+}
+
+static void pxa2xx_spi_ssp_release(void *ssp)
+{
+ pxa_ssp_free(ssp);
+}
+
+static struct ssp_device *pxa2xx_spi_ssp_request(struct platform_device *pdev)
+{
+ struct ssp_device *ssp;
+ int status;
+
+ ssp = pxa_ssp_request(pdev->id, pdev->name);
+ if (!ssp)
+ return ssp;
+
+ status = devm_add_action_or_reset(&pdev->dev, pxa2xx_spi_ssp_release, ssp);
+ if (status)
+ return ERR_PTR(status);
+
+ return ssp;
+}
+
+static struct pxa2xx_spi_controller *
+pxa2xx_spi_init_pdata(struct platform_device *pdev)
+{
+ struct pxa2xx_spi_controller *pdata;
+ struct device *dev = &pdev->dev;
+ struct device *parent = dev->parent;
+ const void *match = device_get_match_data(dev);
+ enum pxa_ssp_type type = SSP_UNDEFINED;
+ struct ssp_device *ssp;
+ bool is_lpss_priv;
+ u32 num_cs = 1;
+ int status;
+
+ ssp = pxa2xx_spi_ssp_request(pdev);
+ if (IS_ERR(ssp))
+ return ERR_CAST(ssp);
+ if (ssp) {
+ type = ssp->type;
+ } else if (match) {
+ type = (enum pxa_ssp_type)(uintptr_t)match;
+ } else {
+ u32 value;
+
+ status = device_property_read_u32(dev, "intel,spi-pxa2xx-type", &value);
+ if (status)
+ return ERR_PTR(status);
+
+ type = (enum pxa_ssp_type)value;
+ }
+
+ /* Validate the SSP type correctness */
+ if (!(type > SSP_UNDEFINED && type < SSP_MAX))
+ return ERR_PTR(-EINVAL);
+
+ pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return ERR_PTR(-ENOMEM);
+
+ /* Platforms with iDMA 64-bit */
+ is_lpss_priv = platform_get_resource_byname(pdev, IORESOURCE_MEM, "lpss_priv");
+ if (is_lpss_priv) {
+ pdata->tx_param = parent;
+ pdata->rx_param = parent;
+ pdata->dma_filter = pxa2xx_spi_idma_filter;
+ }
+
+ /* Read number of chip select pins, if provided */
+ device_property_read_u32(dev, "num-cs", &num_cs);
+
+ pdata->num_chipselect = num_cs;
+ pdata->is_target = device_property_read_bool(dev, "spi-slave");
+ pdata->enable_dma = true;
+ pdata->dma_burst_size = 1;
+
+ /* If SSP has been already enumerated, use it */
+ if (ssp)
+ return pdata;
+
+ status = pxa2xx_spi_init_ssp(pdev, &pdata->ssp, type);
+ if (status)
+ return ERR_PTR(status);
+
+ return pdata;
+}
+
+static int pxa2xx_spi_platform_probe(struct platform_device *pdev)
+{
+ struct pxa2xx_spi_controller *platform_info;
+ struct device *dev = &pdev->dev;
+ struct ssp_device *ssp;
+
+ platform_info = dev_get_platdata(dev);
+ if (!platform_info) {
+ platform_info = pxa2xx_spi_init_pdata(pdev);
+ if (IS_ERR(platform_info))
+ return dev_err_probe(dev, PTR_ERR(platform_info), "missing platform data\n");
+
+ dev->platform_data = platform_info;
+ }
+
+ ssp = pxa2xx_spi_ssp_request(pdev);
+ if (IS_ERR(ssp))
+ return PTR_ERR(ssp);
+ if (!ssp)
+ ssp = &platform_info->ssp;
+
+ return pxa2xx_spi_probe(dev, ssp);
+}
+
+static void pxa2xx_spi_platform_remove(struct platform_device *pdev)
+{
+ pxa2xx_spi_remove(&pdev->dev);
+}
+
+static const struct acpi_device_id pxa2xx_spi_acpi_match[] = {
+ { "80860F0E" },
+ { "8086228E" },
+ { "INT33C0" },
+ { "INT33C1" },
+ { "INT3430" },
+ { "INT3431" },
+ {}
+};
+MODULE_DEVICE_TABLE(acpi, pxa2xx_spi_acpi_match);
+
+static const struct of_device_id pxa2xx_spi_of_match[] = {
+ { .compatible = "marvell,mmp2-ssp", .data = (void *)MMP2_SSP },
+ {}
+};
+MODULE_DEVICE_TABLE(of, pxa2xx_spi_of_match);
+
+static struct platform_driver driver = {
+ .driver = {
+ .name = "pxa2xx-spi",
+ .pm = pm_ptr(&pxa2xx_spi_pm_ops),
+ .acpi_match_table = pxa2xx_spi_acpi_match,
+ .of_match_table = pxa2xx_spi_of_match,
+ },
+ .probe = pxa2xx_spi_platform_probe,
+ .remove_new = pxa2xx_spi_platform_remove,
+};
+
+static int __init pxa2xx_spi_init(void)
+{
+ return platform_driver_register(&driver);
+}
+subsys_initcall(pxa2xx_spi_init);
+
+static void __exit pxa2xx_spi_exit(void)
+{
+ platform_driver_unregister(&driver);
+}
+module_exit(pxa2xx_spi_exit);
+
+MODULE_AUTHOR("Stephen Street");
+MODULE_DESCRIPTION("PXA2xx SSP SPI Controller platform driver");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS(SPI_PXA2xx);
+MODULE_ALIAS("platform:pxa2xx-spi");
+MODULE_SOFTDEP("pre: dw_dmac");
diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index 74f242e652df..1fb30201459f 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -4,7 +4,6 @@
* Copyright (C) 2013, 2021 Intel Corporation
*/
-#include <linux/acpi.h>
#include <linux/atomic.h>
#include <linux/bitops.h>
#include <linux/bug.h>
@@ -14,15 +13,12 @@
#include <linux/dmaengine.h>
#include <linux/err.h>
#include <linux/gpio/consumer.h>
-#include <linux/init.h>
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/ioport.h>
#include <linux/math64.h>
#include <linux/minmax.h>
-#include <linux/mod_devicetable.h>
#include <linux/module.h>
-#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
#include <linux/property.h>
#include <linux/slab.h>
@@ -32,11 +28,6 @@
#include "spi-pxa2xx.h"
-MODULE_AUTHOR("Stephen Street");
-MODULE_DESCRIPTION("PXA2xx SSP SPI Controller");
-MODULE_LICENSE("GPL");
-MODULE_ALIAS("platform:pxa2xx-spi");
-
#define TIMOUT_DFLT 1000
/*
@@ -1263,131 +1254,6 @@ static void cleanup(struct spi_device *spi)
kfree(chip);
}
-static bool pxa2xx_spi_idma_filter(struct dma_chan *chan, void *param)
-{
- return param == chan->device->dev;
-}
-
-static int
-pxa2xx_spi_init_ssp(struct platform_device *pdev, struct ssp_device *ssp, enum pxa_ssp_type type)
-{
- struct device *dev = &pdev->dev;
- struct resource *res;
- int status;
- u64 uid;
-
- ssp->mmio_base = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
- if (IS_ERR(ssp->mmio_base))
- return PTR_ERR(ssp->mmio_base);
-
- ssp->phys_base = res->start;
-
- ssp->clk = devm_clk_get(dev, NULL);
- if (IS_ERR(ssp->clk))
- return PTR_ERR(ssp->clk);
-
- ssp->irq = platform_get_irq(pdev, 0);
- if (ssp->irq < 0)
- return ssp->irq;
-
- ssp->type = type;
- ssp->dev = dev;
-
- status = acpi_dev_uid_to_integer(ACPI_COMPANION(dev), &uid);
- if (status)
- ssp->port_id = -1;
- else
- ssp->port_id = uid;
-
- return 0;
-}
-
-static void pxa2xx_spi_ssp_release(void *ssp)
-{
- pxa_ssp_free(ssp);
-}
-
-static struct ssp_device *pxa2xx_spi_ssp_request(struct platform_device *pdev)
-{
- struct ssp_device *ssp;
- int status;
-
- ssp = pxa_ssp_request(pdev->id, pdev->name);
- if (!ssp)
- return ssp;
-
- status = devm_add_action_or_reset(&pdev->dev, pxa2xx_spi_ssp_release, ssp);
- if (status)
- return ERR_PTR(status);
-
- return ssp;
-}
-
-static struct pxa2xx_spi_controller *
-pxa2xx_spi_init_pdata(struct platform_device *pdev)
-{
- struct pxa2xx_spi_controller *pdata;
- struct device *dev = &pdev->dev;
- struct device *parent = dev->parent;
- const void *match = device_get_match_data(dev);
- enum pxa_ssp_type type = SSP_UNDEFINED;
- struct ssp_device *ssp;
- bool is_lpss_priv;
- u32 num_cs = 1;
- int status;
-
- ssp = pxa2xx_spi_ssp_request(pdev);
- if (IS_ERR(ssp))
- return ERR_CAST(ssp);
- if (ssp) {
- type = ssp->type;
- } else if (match) {
- type = (enum pxa_ssp_type)(uintptr_t)match;
- } else {
- u32 value;
-
- status = device_property_read_u32(dev, "intel,spi-pxa2xx-type", &value);
- if (status)
- return ERR_PTR(status);
-
- type = (enum pxa_ssp_type)value;
- }
-
- /* Validate the SSP type correctness */
- if (!(type > SSP_UNDEFINED && type < SSP_MAX))
- return ERR_PTR(-EINVAL);
-
- pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
- if (!pdata)
- return ERR_PTR(-ENOMEM);
-
- /* Platforms with iDMA 64-bit */
- is_lpss_priv = platform_get_resource_byname(pdev, IORESOURCE_MEM, "lpss_priv");
- if (is_lpss_priv) {
- pdata->tx_param = parent;
- pdata->rx_param = parent;
- pdata->dma_filter = pxa2xx_spi_idma_filter;
- }
-
- /* Read number of chip select pins, if provided */
- device_property_read_u32(dev, "num-cs", &num_cs);
-
- pdata->num_chipselect = num_cs;
- pdata->is_target = device_property_read_bool(dev, "spi-slave");
- pdata->enable_dma = true;
- pdata->dma_burst_size = 1;
-
- /* If SSP has been already enumerated, use it */
- if (ssp)
- return pdata;
-
- status = pxa2xx_spi_init_ssp(pdev, &pdata->ssp, type);
- if (status)
- return ERR_PTR(status);
-
- return pdata;
-}
-
static int pxa2xx_spi_fw_translate_cs(struct spi_controller *controller,
unsigned int cs)
{
@@ -1413,7 +1279,7 @@ static size_t pxa2xx_spi_max_dma_transfer_size(struct spi_device *spi)
return MAX_DMA_LEN;
}
-static int pxa2xx_spi_probe(struct device *dev, struct ssp_device *ssp)
+int pxa2xx_spi_probe(struct device *dev, struct ssp_device *ssp)
{
struct pxa2xx_spi_controller *platform_info;
struct spi_controller *controller;
@@ -1613,8 +1479,9 @@ static int pxa2xx_spi_probe(struct device *dev, struct ssp_device *ssp)
return status;
}
+EXPORT_SYMBOL_NS_GPL(pxa2xx_spi_probe, SPI_PXA2xx);
-static void pxa2xx_spi_remove(struct device *dev)
+void pxa2xx_spi_remove(struct device *dev)
{
struct driver_data *drv_data = dev_get_drvdata(dev);
struct ssp_device *ssp = drv_data->ssp;
@@ -1637,6 +1504,7 @@ static void pxa2xx_spi_remove(struct device *dev)
/* Release IRQ */
free_irq(ssp->irq, drv_data);
}
+EXPORT_SYMBOL_NS_GPL(pxa2xx_spi_remove, SPI_PXA2xx);
static int pxa2xx_spi_suspend(struct device *dev)
{
@@ -1688,78 +1556,11 @@ static int pxa2xx_spi_runtime_resume(struct device *dev)
return clk_prepare_enable(drv_data->ssp->clk);
}
-static const struct dev_pm_ops pxa2xx_spi_pm_ops = {
+EXPORT_NS_GPL_DEV_PM_OPS(pxa2xx_spi_pm_ops, SPI_PXA2xx) = {
SYSTEM_SLEEP_PM_OPS(pxa2xx_spi_suspend, pxa2xx_spi_resume)
RUNTIME_PM_OPS(pxa2xx_spi_runtime_suspend, pxa2xx_spi_runtime_resume, NULL)
};
-static int pxa2xx_spi_platform_probe(struct platform_device *pdev)
-{
- struct pxa2xx_spi_controller *platform_info;
- struct device *dev = &pdev->dev;
- struct ssp_device *ssp;
-
- platform_info = dev_get_platdata(dev);
- if (!platform_info) {
- platform_info = pxa2xx_spi_init_pdata(pdev);
- if (IS_ERR(platform_info))
- return dev_err_probe(dev, PTR_ERR(platform_info), "missing platform data\n");
-
- dev->platform_data = platform_info;
- }
-
- ssp = pxa2xx_spi_ssp_request(pdev);
- if (IS_ERR(ssp))
- return PTR_ERR(ssp);
- if (!ssp)
- ssp = &platform_info->ssp;
-
- return pxa2xx_spi_probe(dev, ssp);
-}
-
-static void pxa2xx_spi_platform_remove(struct platform_device *pdev)
-{
- pxa2xx_spi_remove(&pdev->dev);
-}
-
-static const struct acpi_device_id pxa2xx_spi_acpi_match[] = {
- { "80860F0E" },
- { "8086228E" },
- { "INT33C0" },
- { "INT33C1" },
- { "INT3430" },
- { "INT3431" },
- {}
-};
-MODULE_DEVICE_TABLE(acpi, pxa2xx_spi_acpi_match);
-
-static const struct of_device_id pxa2xx_spi_of_match[] = {
- { .compatible = "marvell,mmp2-ssp", .data = (void *)MMP2_SSP },
- {}
-};
-MODULE_DEVICE_TABLE(of, pxa2xx_spi_of_match);
-
-static struct platform_driver driver = {
- .driver = {
- .name = "pxa2xx-spi",
- .pm = pm_ptr(&pxa2xx_spi_pm_ops),
- .acpi_match_table = pxa2xx_spi_acpi_match,
- .of_match_table = pxa2xx_spi_of_match,
- },
- .probe = pxa2xx_spi_platform_probe,
- .remove_new = pxa2xx_spi_platform_remove,
-};
-
-static int __init pxa2xx_spi_init(void)
-{
- return platform_driver_register(&driver);
-}
-subsys_initcall(pxa2xx_spi_init);
-
-static void __exit pxa2xx_spi_exit(void)
-{
- platform_driver_unregister(&driver);
-}
-module_exit(pxa2xx_spi_exit);
-
-MODULE_SOFTDEP("pre: dw_dmac");
+MODULE_AUTHOR("Stephen Street");
+MODULE_DESCRIPTION("PXA2xx SSP SPI Controller core driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/spi/spi-pxa2xx.h b/drivers/spi/spi-pxa2xx.h
index 93e1e471e1c6..a470d3d634d3 100644
--- a/drivers/spi/spi-pxa2xx.h
+++ b/drivers/spi/spi-pxa2xx.h
@@ -14,6 +14,7 @@
#include <linux/pxa2xx_ssp.h>
+struct device;
struct gpio_desc;
/*
@@ -131,4 +132,9 @@ extern void pxa2xx_spi_dma_stop(struct driver_data *drv_data);
extern int pxa2xx_spi_dma_setup(struct driver_data *drv_data);
extern void pxa2xx_spi_dma_release(struct driver_data *drv_data);
+int pxa2xx_spi_probe(struct device *dev, struct ssp_device *ssp);
+void pxa2xx_spi_remove(struct device *dev);
+
+extern const struct dev_pm_ops pxa2xx_spi_pm_ops;
+
#endif /* SPI_PXA2XX_H */
--
2.43.0.rc1.1336.g36b5255a03ac
Remove hard coded number of chip select pins for Intel Braswell.
This comes via property.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/spi/spi-pxa2xx.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index b62a613378e0..53815aab41aa 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -99,7 +99,6 @@ struct lpss_config {
/* Chip select control */
unsigned cs_sel_shift;
unsigned cs_sel_mask;
- unsigned cs_num;
/* Quirks */
unsigned cs_clk_stays_gated : 1;
};
@@ -137,7 +136,6 @@ static const struct lpss_config lpss_platforms[] = {
.tx_threshold_hi = 224,
.cs_sel_shift = 2,
.cs_sel_mask = 1 << 2,
- .cs_num = 2,
},
{ /* LPSS_SPT_SSP */
.offset = 0x200,
@@ -1594,8 +1592,6 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
tmp &= LPSS_CAPS_CS_EN_MASK;
tmp >>= LPSS_CAPS_CS_EN_SHIFT;
platform_info->num_chipselect = ffz(tmp);
- } else if (config->cs_num) {
- platform_info->num_chipselect = config->cs_num;
}
}
controller->num_chipselect = platform_info->num_chipselect;
--
2.43.0.rc1.1336.g36b5255a03ac
We have a temporary variable to keep a pointer to struct device.
Utilise it where it makes sense.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/spi/spi-pxa2xx.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index 53815aab41aa..19ee7739f4bd 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -1606,13 +1606,13 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
}
}
- pm_runtime_set_autosuspend_delay(&pdev->dev, 50);
- pm_runtime_use_autosuspend(&pdev->dev);
- pm_runtime_set_active(&pdev->dev);
- pm_runtime_enable(&pdev->dev);
+ pm_runtime_set_autosuspend_delay(dev, 50);
+ pm_runtime_use_autosuspend(dev);
+ pm_runtime_set_active(dev);
+ pm_runtime_enable(dev);
/* Register with the SPI framework */
- platform_set_drvdata(pdev, drv_data);
+ dev_set_drvdata(dev, drv_data);
status = spi_register_controller(controller);
if (status) {
dev_err_probe(dev, status, "problem registering SPI controller\n");
@@ -1622,7 +1622,7 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
return status;
out_error_pm_runtime_enabled:
- pm_runtime_disable(&pdev->dev);
+ pm_runtime_disable(dev);
out_error_clock_enabled:
clk_disable_unprepare(ssp->clk);
@@ -1636,10 +1636,11 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
static void pxa2xx_spi_remove(struct platform_device *pdev)
{
- struct driver_data *drv_data = platform_get_drvdata(pdev);
+ struct device *dev = &pdev->dev;
+ struct driver_data *drv_data = dev_get_drvdata(dev);
struct ssp_device *ssp = drv_data->ssp;
- pm_runtime_get_sync(&pdev->dev);
+ pm_runtime_get_sync(dev);
spi_unregister_controller(drv_data->controller);
@@ -1651,8 +1652,8 @@ static void pxa2xx_spi_remove(struct platform_device *pdev)
if (drv_data->controller_info->enable_dma)
pxa2xx_spi_dma_release(drv_data);
- pm_runtime_put_noidle(&pdev->dev);
- pm_runtime_disable(&pdev->dev);
+ pm_runtime_put_noidle(dev);
+ pm_runtime_disable(dev);
/* Release IRQ */
free_irq(ssp->irq, drv_data);
--
2.43.0.rc1.1336.g36b5255a03ac
In the error path or remove path the reference counter in PXA SSP driver
may be dropped before the other resources, that were allocated after
bumbing the reference counter. This breaks reversed order of freeing and
might have an undesired side effects. Prevent this from happening by
wrapping pxa_ssp_request() to be device managed resource.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/spi/spi-pxa2xx.c | 50 +++++++++++++++++++++++++---------------
1 file changed, 32 insertions(+), 18 deletions(-)
diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index efe76d0c21bb..820a3702447a 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -1304,6 +1304,27 @@ pxa2xx_spi_init_ssp(struct platform_device *pdev, struct ssp_device *ssp, enum p
return 0;
}
+static void pxa2xx_spi_ssp_release(void *ssp)
+{
+ pxa_ssp_free(ssp);
+}
+
+static struct ssp_device *pxa2xx_spi_ssp_request(struct platform_device *pdev)
+{
+ struct ssp_device *ssp;
+ int status;
+
+ ssp = pxa_ssp_request(pdev->id, pdev->name);
+ if (!ssp)
+ return ssp;
+
+ status = devm_add_action_or_reset(&pdev->dev, pxa2xx_spi_ssp_release, ssp);
+ if (status)
+ return ERR_PTR(status);
+
+ return ssp;
+}
+
static struct pxa2xx_spi_controller *
pxa2xx_spi_init_pdata(struct platform_device *pdev)
{
@@ -1331,11 +1352,11 @@ pxa2xx_spi_init_pdata(struct platform_device *pdev)
type = (enum pxa_ssp_type)value;
} else {
- ssp = pxa_ssp_request(pdev->id, pdev->name);
- if (ssp) {
+ ssp = pxa2xx_spi_ssp_request(pdev);
+ if (IS_ERR(ssp))
+ return ERR_CAST(ssp);
+ if (ssp)
type = ssp->type;
- pxa_ssp_free(ssp);
- }
}
/* Validate the SSP type correctness */
@@ -1420,7 +1441,9 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
}
dev_dbg(dev, "DMA burst size set to %u\n", platform_info->dma_burst_size);
- ssp = pxa_ssp_request(pdev->id, pdev->name);
+ ssp = pxa2xx_spi_ssp_request(pdev);
+ if (IS_ERR(ssp))
+ return PTR_ERR(ssp);
if (!ssp)
ssp = &platform_info->ssp;
@@ -1431,11 +1454,9 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
controller = devm_spi_alloc_target(dev, sizeof(*drv_data));
else
controller = devm_spi_alloc_host(dev, sizeof(*drv_data));
+ if (!controller)
+ return dev_err_probe(dev, -ENOMEM, "cannot alloc spi_controller\n");
- if (!controller) {
- status = dev_err_probe(dev, -ENOMEM, "cannot alloc spi_controller\n");
- goto out_error_controller_alloc;
- }
drv_data = spi_controller_get_devdata(controller);
drv_data->controller = controller;
drv_data->controller_info = platform_info;
@@ -1486,10 +1507,8 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
status = request_irq(ssp->irq, ssp_int, IRQF_SHARED, dev_name(dev),
drv_data);
- if (status < 0) {
- dev_err_probe(dev, status, "cannot get IRQ %d\n", ssp->irq);
- goto out_error_controller_alloc;
- }
+ if (status < 0)
+ return dev_err_probe(dev, status, "cannot get IRQ %d\n", ssp->irq);
/* Setup DMA if requested */
if (platform_info->enable_dma) {
@@ -1619,8 +1638,6 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
pxa2xx_spi_dma_release(drv_data);
free_irq(ssp->irq, drv_data);
-out_error_controller_alloc:
- pxa_ssp_free(ssp);
return status;
}
@@ -1646,9 +1663,6 @@ static void pxa2xx_spi_remove(struct platform_device *pdev)
/* Release IRQ */
free_irq(ssp->irq, drv_data);
-
- /* Release SSP */
- pxa_ssp_free(ssp);
}
static int pxa2xx_spi_suspend(struct device *dev)
--
2.43.0.rc1.1336.g36b5255a03ac
The mmio_base can't be NULL at this point. It's either checked
in both pxa_ssp_probe() and pxa2xx_spi_init_ssp() or correctly
provided by PCI core. Hence, remove duplicate check which is
a dead code.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/spi/spi-pxa2xx.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c
index 30a829b74a22..9724d9455837 100644
--- a/drivers/spi/spi-pxa2xx.c
+++ b/drivers/spi/spi-pxa2xx.c
@@ -1441,9 +1441,6 @@ static int pxa2xx_spi_probe(struct platform_device *pdev)
if (!ssp)
ssp = &platform_info->ssp;
- if (!ssp->mmio_base)
- return dev_err_probe(dev, -ENODEV, "failed to get SSP\n");
-
if (platform_info->is_target)
controller = devm_spi_alloc_target(dev, sizeof(*drv_data));
else
--
2.43.0.rc1.1336.g36b5255a03ac
On Thu, 30 May 2024 18:09:56 +0300, Andy Shevchenko wrote:
> SPI PXA2xx main driver is a compound of a core library and
> a platform driver code. Decoupling that allows to eventually
> get rid of an additional layer of devices hierarchy in PCI driver.
> There are also precursor cleanups.
>
> In v2:
> - made better handling of pxa_ssp_free() calls
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
Thanks!
[01/11] spi: pxa2xx: Wrap pxa_ssp_request() to be device managed resource
commit: a2fca8f2e242b3cdfed2a15084e733348ef68509
[02/11] spi: pxa2xx: Reorganize the SSP type retrieval
commit: 8aa5062e26054b8c081d5bba930baac4faadd1b0
[03/11] spi: pxa2xx: Remove no more needed driver data
commit: 7b0f2c1050643c4793e6eae0246a8de3b22c950a
[04/11] spi: pxa2xx: Remove hard coded number of chip select pins
commit: c1b93986dfb2a31b0528fe929d574843801089f5
[05/11] spi: pxa2xx: Utilise temporary variable for struct device
commit: c65174fdb2f7fe83ee515966c08de9a990e722f9
[06/11] spi: pxa2xx: Print DMA burst size only when DMA is enabled
commit: 9b328f5f5c921ec83e1765075b82e6cc05e576b9
[07/11] spi: pxa2xx: Remove duplicate check
commit: 560fb06df2fd250004a1cac079717dbe7f863ff2
[08/11] spi: pxa2xx: Remove superflous check for Intel Atom SoCs
commit: 75bfdccaecf96189318b29100b880c416d89ed46
[09/11] spi: pxa2xx: Extract pxa2xx_spi_platform_*() callbacks
commit: 20ade9b9771c80eb58eb42ccd0a48ba24bdc3c4f
[10/11] spi: pxa2xx: Move platform driver to a separate file
commit: 3d8f037fbcab53e03ab2ef18a66f202be3653d50
[11/11] spi: pxa2xx: Convert PCI driver to use spi-pxa2xx code directly
commit: cc160697a576150975280a4b5394fe9c70700503
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