* [PATCH -next 1/6] spi: oc-tiny: Switch to use devm_spi_alloc_master()
2022-09-20 13:48 [PATCH -next 0/6] spi: Switch to use devm_spi_alloc_master() in some drivers Yang Yingliang
@ 2022-09-20 13:48 ` Yang Yingliang
2022-09-23 4:55 ` Lukas Wunner
2022-09-20 13:48 ` [PATCH -next 2/6] spi: ath79: " Yang Yingliang
` (6 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Yang Yingliang @ 2022-09-20 13:48 UTC (permalink / raw)
To: linux-spi; +Cc: broonie, yangyingliang
Switch to use devm_spi_alloc_master() to simpify error path.
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
drivers/spi/spi-oc-tiny.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/spi/spi-oc-tiny.c b/drivers/spi/spi-oc-tiny.c
index 38c14c4e4e21..1ab807741d26 100644
--- a/drivers/spi/spi-oc-tiny.c
+++ b/drivers/spi/spi-oc-tiny.c
@@ -215,7 +215,7 @@ static int tiny_spi_probe(struct platform_device *pdev)
struct spi_master *master;
int err = -ENODEV;
- master = spi_alloc_master(&pdev->dev, sizeof(struct tiny_spi));
+ master = devm_spi_alloc_master(&pdev->dev, sizeof(struct tiny_spi));
if (!master)
return err;
@@ -235,10 +235,8 @@ static int tiny_spi_probe(struct platform_device *pdev)
/* find and map our resources */
hw->base = devm_platform_ioremap_resource(pdev, 0);
- if (IS_ERR(hw->base)) {
- err = PTR_ERR(hw->base);
- goto exit;
- }
+ if (IS_ERR(hw->base))
+ return PTR_ERR(hw->base);
/* irq is optional */
hw->irq = platform_get_irq(pdev, 0);
if (hw->irq >= 0) {
@@ -246,7 +244,7 @@ static int tiny_spi_probe(struct platform_device *pdev)
err = devm_request_irq(&pdev->dev, hw->irq, tiny_spi_irq, 0,
pdev->name, hw);
if (err)
- goto exit;
+ return err;
}
/* find platform data */
if (platp) {
@@ -255,20 +253,16 @@ static int tiny_spi_probe(struct platform_device *pdev)
} else {
err = tiny_spi_of_probe(pdev);
if (err)
- goto exit;
+ return err;
}
/* register our spi controller */
err = spi_bitbang_start(&hw->bitbang);
if (err)
- goto exit;
+ return err;
dev_info(&pdev->dev, "base %p, irq %d\n", hw->base, hw->irq);
return 0;
-
-exit:
- spi_master_put(master);
- return err;
}
static int tiny_spi_remove(struct platform_device *pdev)
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH -next 2/6] spi: ath79: Switch to use devm_spi_alloc_master()
2022-09-20 13:48 [PATCH -next 0/6] spi: Switch to use devm_spi_alloc_master() in some drivers Yang Yingliang
2022-09-20 13:48 ` [PATCH -next 1/6] spi: oc-tiny: Switch to use devm_spi_alloc_master() Yang Yingliang
@ 2022-09-20 13:48 ` Yang Yingliang
2022-09-23 5:00 ` Lukas Wunner
2022-09-20 13:48 ` [PATCH -next 3/6] spi: omap-uwire: " Yang Yingliang
` (5 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Yang Yingliang @ 2022-09-20 13:48 UTC (permalink / raw)
To: linux-spi; +Cc: broonie, yangyingliang
Switch to use devm_spi_alloc_master() to simpify error path.
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
drivers/spi/spi-ath79.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/spi/spi-ath79.c b/drivers/spi/spi-ath79.c
index 607e7a49fb89..4cc5c4e8504e 100644
--- a/drivers/spi/spi-ath79.c
+++ b/drivers/spi/spi-ath79.c
@@ -173,7 +173,7 @@ static int ath79_spi_probe(struct platform_device *pdev)
unsigned long rate;
int ret;
- master = spi_alloc_master(&pdev->dev, sizeof(*sp));
+ master = devm_spi_alloc_master(&pdev->dev, sizeof(*sp));
if (master == NULL) {
dev_err(&pdev->dev, "failed to allocate spi master\n");
return -ENOMEM;
@@ -195,20 +195,16 @@ static int ath79_spi_probe(struct platform_device *pdev)
sp->bitbang.flags = SPI_CS_HIGH;
sp->base = devm_platform_ioremap_resource(pdev, 0);
- if (IS_ERR(sp->base)) {
- ret = PTR_ERR(sp->base);
- goto err_put_master;
- }
+ if (IS_ERR(sp->base))
+ return PTR_ERR(sp->base);
sp->clk = devm_clk_get(&pdev->dev, "ahb");
- if (IS_ERR(sp->clk)) {
- ret = PTR_ERR(sp->clk);
- goto err_put_master;
- }
+ if (IS_ERR(sp->clk))
+ return PTR_ERR(sp->clk);
ret = clk_prepare_enable(sp->clk);
if (ret)
- goto err_put_master;
+ return ret;
rate = DIV_ROUND_UP(clk_get_rate(sp->clk), MHZ);
if (!rate) {
@@ -231,8 +227,6 @@ static int ath79_spi_probe(struct platform_device *pdev)
ath79_spi_disable(sp);
err_clk_disable:
clk_disable_unprepare(sp->clk);
-err_put_master:
- spi_master_put(sp->bitbang.master);
return ret;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH -next 3/6] spi: omap-uwire: Switch to use devm_spi_alloc_master()
2022-09-20 13:48 [PATCH -next 0/6] spi: Switch to use devm_spi_alloc_master() in some drivers Yang Yingliang
2022-09-20 13:48 ` [PATCH -next 1/6] spi: oc-tiny: Switch to use devm_spi_alloc_master() Yang Yingliang
2022-09-20 13:48 ` [PATCH -next 2/6] spi: ath79: " Yang Yingliang
@ 2022-09-20 13:48 ` Yang Yingliang
2022-09-23 5:06 ` Lukas Wunner
2022-09-20 13:48 ` [PATCH -next 4/6] spi: ppc4xx: " Yang Yingliang
` (4 subsequent siblings)
7 siblings, 1 reply; 19+ messages in thread
From: Yang Yingliang @ 2022-09-20 13:48 UTC (permalink / raw)
To: linux-spi; +Cc: broonie, yangyingliang
Switch to use devm_spi_alloc_master() to simpify error path.
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
drivers/spi/spi-omap-uwire.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/spi-omap-uwire.c b/drivers/spi/spi-omap-uwire.c
index 29198e6815b2..f1e66f758d45 100644
--- a/drivers/spi/spi-omap-uwire.c
+++ b/drivers/spi/spi-omap-uwire.c
@@ -448,7 +448,6 @@ static void uwire_off(struct uwire_spi *uwire)
{
uwire_write_reg(UWIRE_SR3, 0);
clk_disable_unprepare(uwire->ck);
- spi_master_put(uwire->bitbang.master);
}
static int uwire_probe(struct platform_device *pdev)
@@ -457,7 +456,7 @@ static int uwire_probe(struct platform_device *pdev)
struct uwire_spi *uwire;
int status;
- master = spi_alloc_master(&pdev->dev, sizeof(*uwire));
+ master = devm_spi_alloc_master(&pdev->dev, sizeof(*uwire));
if (!master)
return -ENODEV;
@@ -466,7 +465,6 @@ static int uwire_probe(struct platform_device *pdev)
uwire_base = devm_ioremap(&pdev->dev, UWIRE_BASE_PHYS, UWIRE_IO_SIZE);
if (!uwire_base) {
dev_dbg(&pdev->dev, "can't ioremap UWIRE\n");
- spi_master_put(master);
return -ENOMEM;
}
@@ -476,7 +474,6 @@ static int uwire_probe(struct platform_device *pdev)
if (IS_ERR(uwire->ck)) {
status = PTR_ERR(uwire->ck);
dev_dbg(&pdev->dev, "no functional clock?\n");
- spi_master_put(master);
return status;
}
clk_prepare_enable(uwire->ck);
@@ -518,6 +515,7 @@ static int uwire_remove(struct platform_device *pdev)
spi_bitbang_stop(&uwire->bitbang);
uwire_off(uwire);
+ spi_master_put(uwire->bitbang.master);
return 0;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH -next 3/6] spi: omap-uwire: Switch to use devm_spi_alloc_master()
2022-09-20 13:48 ` [PATCH -next 3/6] spi: omap-uwire: " Yang Yingliang
@ 2022-09-23 5:06 ` Lukas Wunner
0 siblings, 0 replies; 19+ messages in thread
From: Lukas Wunner @ 2022-09-23 5:06 UTC (permalink / raw)
To: Yang Yingliang; +Cc: linux-spi, broonie
On Tue, Sep 20, 2022 at 09:48:16PM +0800, Yang Yingliang wrote:
> @@ -476,7 +474,6 @@ static int uwire_probe(struct platform_device *pdev)
> if (IS_ERR(uwire->ck)) {
> status = PTR_ERR(uwire->ck);
> dev_dbg(&pdev->dev, "no functional clock?\n");
> - spi_master_put(master);
> return status;
> }
> clk_prepare_enable(uwire->ck);
Change this hunk to return PTR_ERR(uwire->ck) directly without
assigning to status first.
> @@ -518,6 +515,7 @@ static int uwire_remove(struct platform_device *pdev)
>
> spi_bitbang_stop(&uwire->bitbang);
> uwire_off(uwire);
> + spi_master_put(uwire->bitbang.master);
> return 0;
> }
>
No, drop this hunk, it results in a use-after-free.
Thanks,
Lukas
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH -next 4/6] spi: ppc4xx: Switch to use devm_spi_alloc_master()
2022-09-20 13:48 [PATCH -next 0/6] spi: Switch to use devm_spi_alloc_master() in some drivers Yang Yingliang
` (2 preceding siblings ...)
2022-09-20 13:48 ` [PATCH -next 3/6] spi: omap-uwire: " Yang Yingliang
@ 2022-09-20 13:48 ` Yang Yingliang
2022-09-20 13:48 ` [PATCH -next 5/6] spi: sh-sci: " Yang Yingliang
` (3 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Yang Yingliang @ 2022-09-20 13:48 UTC (permalink / raw)
To: linux-spi; +Cc: broonie, yangyingliang
Switch to use devm_spi_alloc_master() to simpify error path.
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
drivers/spi/spi-ppc4xx.c | 17 ++++++-----------
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/spi/spi-ppc4xx.c b/drivers/spi/spi-ppc4xx.c
index d65f047b6c82..b49db0acdbdb 100644
--- a/drivers/spi/spi-ppc4xx.c
+++ b/drivers/spi/spi-ppc4xx.c
@@ -349,7 +349,7 @@ static int spi_ppc4xx_of_probe(struct platform_device *op)
int ret;
const unsigned int *clk;
- master = spi_alloc_master(dev, sizeof(*hw));
+ master = devm_spi_alloc_master(dev, sizeof(*hw));
if (master == NULL)
return -ENOMEM;
master->dev.of_node = np;
@@ -384,16 +384,14 @@ static int spi_ppc4xx_of_probe(struct platform_device *op)
opbnp = of_find_compatible_node(NULL, NULL, "ibm,opb");
if (opbnp == NULL) {
dev_err(dev, "OPB: cannot find node\n");
- ret = -ENODEV;
- goto free_master;
+ return -ENODEV;
}
/* Get the clock (Hz) for the OPB */
clk = of_get_property(opbnp, "clock-frequency", NULL);
if (clk == NULL) {
dev_err(dev, "OPB: no clock-frequency property set\n");
of_node_put(opbnp);
- ret = -ENODEV;
- goto free_master;
+ return -ENODEV;
}
hw->opb_freq = *clk;
hw->opb_freq >>= 2;
@@ -402,7 +400,7 @@ static int spi_ppc4xx_of_probe(struct platform_device *op)
ret = of_address_to_resource(np, 0, &resource);
if (ret) {
dev_err(dev, "error while parsing device node resource\n");
- goto free_master;
+ return ret;
}
hw->mapbase = resource.start;
hw->mapsize = resource_size(&resource);
@@ -410,8 +408,7 @@ static int spi_ppc4xx_of_probe(struct platform_device *op)
/* Sanity check */
if (hw->mapsize < sizeof(struct spi_ppc4xx_regs)) {
dev_err(dev, "too small to map registers\n");
- ret = -EINVAL;
- goto free_master;
+ return -EINVAL;
}
/* Request IRQ */
@@ -420,7 +417,7 @@ static int spi_ppc4xx_of_probe(struct platform_device *op)
0, "spi_ppc4xx_of", (void *)hw);
if (ret) {
dev_err(dev, "unable to allocate interrupt\n");
- goto free_master;
+ return ret;
}
if (!request_mem_region(hw->mapbase, hw->mapsize, DRIVER_NAME)) {
@@ -457,8 +454,6 @@ static int spi_ppc4xx_of_probe(struct platform_device *op)
release_mem_region(hw->mapbase, hw->mapsize);
request_mem_error:
free_irq(hw->irqnum, hw);
-free_master:
- spi_master_put(master);
dev_err(dev, "initialization failed\n");
return ret;
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH -next 5/6] spi: sh-sci: Switch to use devm_spi_alloc_master()
2022-09-20 13:48 [PATCH -next 0/6] spi: Switch to use devm_spi_alloc_master() in some drivers Yang Yingliang
` (3 preceding siblings ...)
2022-09-20 13:48 ` [PATCH -next 4/6] spi: ppc4xx: " Yang Yingliang
@ 2022-09-20 13:48 ` Yang Yingliang
2022-09-20 13:48 ` [PATCH -next 6/6] spi: altera: " Yang Yingliang
` (2 subsequent siblings)
7 siblings, 0 replies; 19+ messages in thread
From: Yang Yingliang @ 2022-09-20 13:48 UTC (permalink / raw)
To: linux-spi; +Cc: broonie, yangyingliang
Switch to use devm_spi_alloc_master() to simpify error path.
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
drivers/spi/spi-sh-sci.c | 34 +++++++++++++---------------------
1 file changed, 13 insertions(+), 21 deletions(-)
diff --git a/drivers/spi/spi-sh-sci.c b/drivers/spi/spi-sh-sci.c
index 8f30531e1418..2cd5cd23e37c 100644
--- a/drivers/spi/spi-sh-sci.c
+++ b/drivers/spi/spi-sh-sci.c
@@ -118,11 +118,10 @@ static int sh_sci_spi_probe(struct platform_device *dev)
struct sh_sci_spi *sp;
int ret;
- master = spi_alloc_master(&dev->dev, sizeof(struct sh_sci_spi));
+ master = devm_spi_alloc_master(&dev->dev, sizeof(struct sh_sci_spi));
if (master == NULL) {
dev_err(&dev->dev, "failed to allocate spi master\n");
- ret = -ENOMEM;
- goto err0;
+ return -ENOMEM;
}
sp = spi_master_get_devdata(master);
@@ -131,8 +130,7 @@ static int sh_sci_spi_probe(struct platform_device *dev)
sp->info = dev_get_platdata(&dev->dev);
if (!sp->info) {
dev_err(&dev->dev, "platform data is missing\n");
- ret = -ENOENT;
- goto err1;
+ return -ENOENT;
}
/* setup spi bitbang adaptor */
@@ -147,28 +145,22 @@ static int sh_sci_spi_probe(struct platform_device *dev)
sp->bitbang.txrx_word[SPI_MODE_3] = sh_sci_spi_txrx_mode3;
r = platform_get_resource(dev, IORESOURCE_MEM, 0);
- if (r == NULL) {
- ret = -ENOENT;
- goto err1;
- }
+ if (r == NULL)
+ return -ENOENT;
sp->membase = ioremap(r->start, resource_size(r));
- if (!sp->membase) {
- ret = -ENXIO;
- goto err1;
- }
+ if (!sp->membase)
+ return -ENXIO;
sp->val = ioread8(SCSPTR(sp));
setbits(sp, PIN_INIT, 1);
ret = spi_bitbang_start(&sp->bitbang);
- if (!ret)
- return 0;
+ if (ret) {
+ setbits(sp, PIN_INIT, 0);
+ iounmap(sp->membase);
+ return ret;
+ }
- setbits(sp, PIN_INIT, 0);
- iounmap(sp->membase);
- err1:
- spi_master_put(sp->bitbang.master);
- err0:
- return ret;
+ return 0;
}
static int sh_sci_spi_remove(struct platform_device *dev)
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH -next 6/6] spi: altera: Switch to use devm_spi_alloc_master()
2022-09-20 13:48 [PATCH -next 0/6] spi: Switch to use devm_spi_alloc_master() in some drivers Yang Yingliang
` (4 preceding siblings ...)
2022-09-20 13:48 ` [PATCH -next 5/6] spi: sh-sci: " Yang Yingliang
@ 2022-09-20 13:48 ` Yang Yingliang
2022-09-20 18:33 ` [PATCH -next 0/6] spi: Switch to use devm_spi_alloc_master() in some drivers Mark Brown
2022-09-23 5:33 ` Lukas Wunner
7 siblings, 0 replies; 19+ messages in thread
From: Yang Yingliang @ 2022-09-20 13:48 UTC (permalink / raw)
To: linux-spi; +Cc: broonie, yangyingliang
Switch to use devm_spi_alloc_master() to simpify error path.
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
drivers/spi/spi-altera-platform.c | 23 ++++++++---------------
1 file changed, 8 insertions(+), 15 deletions(-)
diff --git a/drivers/spi/spi-altera-platform.c b/drivers/spi/spi-altera-platform.c
index 65147aae82a1..376df7b6f543 100644
--- a/drivers/spi/spi-altera-platform.c
+++ b/drivers/spi/spi-altera-platform.c
@@ -43,7 +43,7 @@ static int altera_spi_probe(struct platform_device *pdev)
int err = -ENODEV;
u16 i;
- master = spi_alloc_master(&pdev->dev, sizeof(struct altera_spi));
+ master = devm_spi_alloc_master(&pdev->dev, sizeof(struct altera_spi));
if (!master)
return err;
@@ -55,8 +55,7 @@ static int altera_spi_probe(struct platform_device *pdev)
dev_err(&pdev->dev,
"Invalid number of chipselect: %u\n",
pdata->num_chipselect);
- err = -EINVAL;
- goto exit;
+ return -EINVAL;
}
master->num_chipselect = pdata->num_chipselect;
@@ -83,7 +82,7 @@ static int altera_spi_probe(struct platform_device *pdev)
hw->regmap = dev_get_regmap(pdev->dev.parent, NULL);
if (!hw->regmap) {
dev_err(&pdev->dev, "get regmap failed\n");
- goto exit;
+ return err;
}
regoff = platform_get_resource(pdev, IORESOURCE_REG, 0);
@@ -93,17 +92,14 @@ static int altera_spi_probe(struct platform_device *pdev)
void __iomem *res;
res = devm_platform_ioremap_resource(pdev, 0);
- if (IS_ERR(res)) {
- err = PTR_ERR(res);
- goto exit;
- }
+ if (IS_ERR(res))
+ return PTR_ERR(res);
hw->regmap = devm_regmap_init_mmio(&pdev->dev, res,
&spi_altera_config);
if (IS_ERR(hw->regmap)) {
dev_err(&pdev->dev, "regmap mmio init failed\n");
- err = PTR_ERR(hw->regmap);
- goto exit;
+ return PTR_ERR(hw->regmap);
}
}
@@ -115,12 +111,12 @@ static int altera_spi_probe(struct platform_device *pdev)
err = devm_request_irq(&pdev->dev, hw->irq, altera_spi_irq, 0,
pdev->name, master);
if (err)
- goto exit;
+ return err;
}
err = devm_spi_register_master(&pdev->dev, master);
if (err)
- goto exit;
+ return err;
if (pdata) {
for (i = 0; i < pdata->num_devices; i++) {
@@ -134,9 +130,6 @@ static int altera_spi_probe(struct platform_device *pdev)
dev_info(&pdev->dev, "regoff %u, irq %d\n", hw->regoff, hw->irq);
return 0;
-exit:
- spi_master_put(master);
- return err;
}
#ifdef CONFIG_OF
--
2.25.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH -next 0/6] spi: Switch to use devm_spi_alloc_master() in some drivers
2022-09-20 13:48 [PATCH -next 0/6] spi: Switch to use devm_spi_alloc_master() in some drivers Yang Yingliang
` (5 preceding siblings ...)
2022-09-20 13:48 ` [PATCH -next 6/6] spi: altera: " Yang Yingliang
@ 2022-09-20 18:33 ` Mark Brown
2022-09-21 2:02 ` Yang Yingliang
2022-09-23 5:33 ` Lukas Wunner
7 siblings, 1 reply; 19+ messages in thread
From: Mark Brown @ 2022-09-20 18:33 UTC (permalink / raw)
To: Yang Yingliang; +Cc: linux-spi
[-- Attachment #1: Type: text/plain, Size: 435 bytes --]
On Tue, Sep 20, 2022 at 09:48:13PM +0800, Yang Yingliang wrote:
> This patchset is trying to replace spi_alloc_master() with
> devm_spi_alloc_master() in some spi drivers. With this helper,
> spi_master_put() is called in devres_release_all() whenever
> the device is unbound, so the spi_master_put() in error path
> can be removed.
If we're switching please update to the modern naming and use
"controller" rather than the old name.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH -next 0/6] spi: Switch to use devm_spi_alloc_master() in some drivers
2022-09-20 18:33 ` [PATCH -next 0/6] spi: Switch to use devm_spi_alloc_master() in some drivers Mark Brown
@ 2022-09-21 2:02 ` Yang Yingliang
2022-09-21 12:37 ` Mark Brown
0 siblings, 1 reply; 19+ messages in thread
From: Yang Yingliang @ 2022-09-21 2:02 UTC (permalink / raw)
To: Mark Brown; +Cc: linux-spi
Hi Mark,
On 2022/9/21 2:33, Mark Brown wrote:
> On Tue, Sep 20, 2022 at 09:48:13PM +0800, Yang Yingliang wrote:
>> This patchset is trying to replace spi_alloc_master() with
>> devm_spi_alloc_master() in some spi drivers. With this helper,
>> spi_master_put() is called in devres_release_all() whenever
>> the device is unbound, so the spi_master_put() in error path
>> can be removed.
> If we're switching please update to the modern naming and use
> "controller" rather than the old name.
Do you mean to use spi_controller instead of spi_master? Something like
this:
'struct spi_controller * ctlr = devm_spi_alloc_master();'
Dose spi_master_get_devdata() need be changed to
spi_controller_get_devdata() ?
Thanks,
Yang
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH -next 0/6] spi: Switch to use devm_spi_alloc_master() in some drivers
2022-09-21 2:02 ` Yang Yingliang
@ 2022-09-21 12:37 ` Mark Brown
2022-09-21 13:19 ` Yang Yingliang
2022-09-23 4:42 ` Lukas Wunner
0 siblings, 2 replies; 19+ messages in thread
From: Mark Brown @ 2022-09-21 12:37 UTC (permalink / raw)
To: Yang Yingliang; +Cc: linux-spi
[-- Attachment #1: Type: text/plain, Size: 519 bytes --]
On Wed, Sep 21, 2022 at 10:02:25AM +0800, Yang Yingliang wrote:
> On 2022/9/21 2:33, Mark Brown wrote:
> > If we're switching please update to the modern naming and use
> > "controller" rather than the old name.
> Do you mean to use spi_controller instead of spi_master? Something like
> this:
> 'struct spi_controller * ctlr = devm_spi_alloc_master();'
Or just use devm_spi_alloc_controller() directly.
> Dose spi_master_get_devdata() need be changed to
> spi_controller_get_devdata() ?
Ideally.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH -next 0/6] spi: Switch to use devm_spi_alloc_master() in some drivers
2022-09-21 12:37 ` Mark Brown
@ 2022-09-21 13:19 ` Yang Yingliang
2022-09-23 5:39 ` Lukas Wunner
2022-09-23 4:42 ` Lukas Wunner
1 sibling, 1 reply; 19+ messages in thread
From: Yang Yingliang @ 2022-09-21 13:19 UTC (permalink / raw)
To: Mark Brown; +Cc: linux-spi
On 2022/9/21 20:37, Mark Brown wrote:
> On Wed, Sep 21, 2022 at 10:02:25AM +0800, Yang Yingliang wrote:
>> On 2022/9/21 2:33, Mark Brown wrote:
>>> If we're switching please update to the modern naming and use
>>> "controller" rather than the old name.
>> Do you mean to use spi_controller instead of spi_master? Something like
>> this:
>> 'struct spi_controller * ctlr = devm_spi_alloc_master();'
> Or just use devm_spi_alloc_controller() directly.
Does __devm_spi_alloc_controller() need be changed to
devm_spi_alloc_controller(), then use
it, or just use __devm_spi_alloc_controller() directly.
Thanks,
Yang
>
>> Dose spi_master_get_devdata() need be changed to
>> spi_controller_get_devdata() ?
> Ideally.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH -next 0/6] spi: Switch to use devm_spi_alloc_master() in some drivers
2022-09-21 13:19 ` Yang Yingliang
@ 2022-09-23 5:39 ` Lukas Wunner
0 siblings, 0 replies; 19+ messages in thread
From: Lukas Wunner @ 2022-09-23 5:39 UTC (permalink / raw)
To: Yang Yingliang; +Cc: Mark Brown, linux-spi
On Wed, Sep 21, 2022 at 09:19:35PM +0800, Yang Yingliang wrote:
> On 2022/9/21 20:37, Mark Brown wrote:
> > On Wed, Sep 21, 2022 at 10:02:25AM +0800, Yang Yingliang wrote:
> > > On 2022/9/21 2:33, Mark Brown wrote:
> > > > If we're switching please update to the modern naming and use
> > > > "controller" rather than the old name.
> > > Do you mean to use spi_controller instead of spi_master? Something like
> > > this:
> > > 'struct spi_controller * ctlr = devm_spi_alloc_master();'
> >
> > Or just use devm_spi_alloc_controller() directly.
>
> Does __devm_spi_alloc_controller() need be changed to
> devm_spi_alloc_controller(), then use
> it, or just use __devm_spi_alloc_controller() directly.
Modern drivers only differentiate between master and slave on
allocation. They use struct spi_controller instead of spi_master
and generally only call the spi_controller_*() functions (again,
except on allocation). I think Mark wanted you to convert the
drivers so that they use struct spi_controller everywhere,
but that can be done in separate patches.
Thanks,
Lukas
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH -next 0/6] spi: Switch to use devm_spi_alloc_master() in some drivers
2022-09-21 12:37 ` Mark Brown
2022-09-21 13:19 ` Yang Yingliang
@ 2022-09-23 4:42 ` Lukas Wunner
2022-09-23 10:12 ` Mark Brown
1 sibling, 1 reply; 19+ messages in thread
From: Lukas Wunner @ 2022-09-23 4:42 UTC (permalink / raw)
To: Mark Brown; +Cc: Yang Yingliang, linux-spi
On Wed, Sep 21, 2022 at 01:37:49PM +0100, Mark Brown wrote:
> On Wed, Sep 21, 2022 at 10:02:25AM +0800, Yang Yingliang wrote:
> > On 2022/9/21 2:33, Mark Brown wrote:
> > > If we're switching please update to the modern naming and use
> > > "controller" rather than the old name.
>
> > Do you mean to use spi_controller instead of spi_master? Something like
> > this:
> > 'struct spi_controller * ctlr = devm_spi_alloc_master();'
>
> Or just use devm_spi_alloc_controller() directly.
There's no such thing. The driver needs to explicitly allocate a
master or slave and that will result in the slave bit being set
correctly in struct spi_controller.
Yang's v2 series now calls __devm_spi_alloc_controller()
but drivers should never call that directly.
Thanks,
Lukas
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH -next 0/6] spi: Switch to use devm_spi_alloc_master() in some drivers
2022-09-23 4:42 ` Lukas Wunner
@ 2022-09-23 10:12 ` Mark Brown
2022-09-23 14:48 ` Yang Yingliang
0 siblings, 1 reply; 19+ messages in thread
From: Mark Brown @ 2022-09-23 10:12 UTC (permalink / raw)
To: Lukas Wunner; +Cc: Yang Yingliang, linux-spi
[-- Attachment #1: Type: text/plain, Size: 582 bytes --]
On Fri, Sep 23, 2022 at 06:42:58AM +0200, Lukas Wunner wrote:
> On Wed, Sep 21, 2022 at 01:37:49PM +0100, Mark Brown wrote:
> > Or just use devm_spi_alloc_controller() directly.
> There's no such thing. The driver needs to explicitly allocate a
> master or slave and that will result in the slave bit being set
> correctly in struct spi_controller.
> Yang's v2 series now calls __devm_spi_alloc_controller()
> but drivers should never call that directly.
Right, we should probably make the actual function to wrap that though -
I'd misremembered that that hadn't been created.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH -next 0/6] spi: Switch to use devm_spi_alloc_master() in some drivers
2022-09-23 10:12 ` Mark Brown
@ 2022-09-23 14:48 ` Yang Yingliang
0 siblings, 0 replies; 19+ messages in thread
From: Yang Yingliang @ 2022-09-23 14:48 UTC (permalink / raw)
To: Mark Brown, Lukas Wunner; +Cc: linux-spi, yangyingliang
On 2022/9/23 18:12, Mark Brown wrote:
> On Fri, Sep 23, 2022 at 06:42:58AM +0200, Lukas Wunner wrote:
>> On Wed, Sep 21, 2022 at 01:37:49PM +0100, Mark Brown wrote:
>>> Or just use devm_spi_alloc_controller() directly.
>> There's no such thing. The driver needs to explicitly allocate a
>> master or slave and that will result in the slave bit being set
>> correctly in struct spi_controller.
>> Yang's v2 series now calls __devm_spi_alloc_controller()
>> but drivers should never call that directly.
> Right, we should probably make the actual function to wrap that though -
> I'd misremembered that that hadn't been created.
How about introduce devm_spi_alloc_controller() like this:
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index f089ee1ead58..67e510c8d15e 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -763,6 +763,8 @@ struct spi_controller
*__devm_spi_alloc_controller(struct device *dev,
unsigned int size,
bool slave);
+#define devm_spi_alloc_controller devm_spi_alloc_master
+
Thanks,
Yang
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH -next 0/6] spi: Switch to use devm_spi_alloc_master() in some drivers
2022-09-20 13:48 [PATCH -next 0/6] spi: Switch to use devm_spi_alloc_master() in some drivers Yang Yingliang
` (6 preceding siblings ...)
2022-09-20 18:33 ` [PATCH -next 0/6] spi: Switch to use devm_spi_alloc_master() in some drivers Mark Brown
@ 2022-09-23 5:33 ` Lukas Wunner
7 siblings, 0 replies; 19+ messages in thread
From: Lukas Wunner @ 2022-09-23 5:33 UTC (permalink / raw)
To: Yang Yingliang; +Cc: linux-spi, broonie
On Tue, Sep 20, 2022 at 09:48:13PM +0800, Yang Yingliang wrote:
> This patchset is trying to replace spi_alloc_master() with
> devm_spi_alloc_master() in some spi drivers. With this helper,
> spi_master_put() is called in devres_release_all() whenever
> the device is unbound, so the spi_master_put() in error path
> can be removed.
>
> Yang Yingliang (6):
> spi: oc-tiny: Switch to use devm_spi_alloc_master()
> spi: ath79: Switch to use devm_spi_alloc_master()
> spi: omap-uwire: Switch to use devm_spi_alloc_master()
> spi: ppc4xx: Switch to use devm_spi_alloc_master()
> spi: sh-sci: Switch to use devm_spi_alloc_master()
> spi: altera: Switch to use devm_spi_alloc_master()
I'm withdrawing my objections to patches 1, 2 and 3:
I failed to appreciate that these drivers use spi_bitbang_start(),
which takes an extra reference on the controller. Sorry for the noise.
Whole series is
Reviewed-by: Lukas Wunner <lukas@wunner.de>
This pertains to v1 of the series, not v2 (which incorrectly uses
__devm_spi_alloc_controller()).
Thanks,
Lukas
^ permalink raw reply [flat|nested] 19+ messages in thread