Re: [PATCH 0/6] ipmi: Convert to platform remove callback returning void
Hello Corey, On Sat, May 25, 2024 at 09:39:36AM -0500, Corey Minyard wrote: > On Sat, May 25, 2024 at 12:10:38PM +0200, Uwe Kleine-König wrote: > > These changes are in next since a while but didn't land in Linus tree > > for v6.10-rc1. I intend to send a PR to Greg early next week changing > > platform_driver::remove to match remove_new. If these commits don't make > > it in in time, I'll be so bold and just include the commits from your > > for-next branch in my PR. > > I sent them to Linus right after 6.9 dropped, let me resend... That worked, they landed now in Linus' tree. Thanks, that makes it a bit less ugly for me. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
Re: [PATCH 0/6] ipmi: Convert to platform remove callback returning void
Hello Corey,
On Thu, Apr 11, 2024 at 10:11:21AM -0500, Corey Minyard wrote:
> On Thu, Apr 11, 2024 at 09:15:03AM +0200, Uwe Kleine-König wrote:
> > Hello,
> >
> > On Tue, Mar 05, 2024 at 05:26:57PM +0100, Uwe Kleine-König wrote:
> > > this series converts all drivers below drivers/char/ipmi to struct
> > > platform_driver::remove_new(). See commit 5c5a7680e67b ("platform:
> > > Provide a
> > > remove callback that returns no value") for an extended explanation and
> > > the
> > > eventual goal.
> > >
> > > All conversations are trivial, because their .remove() callbacks
> > > returned zero unconditionally.
> > >
> > > There are no interdependencies between these patches, so they could be
> > > picked up individually. But I'd hope that they get picked up all
> > > together by Corey.
>
> Yeah, I was kind of waiting for more reviews, but this is pretty
> straightforward. I've pulled this into my tree.
These changes are in next since a while but didn't land in Linus tree
for v6.10-rc1. I intend to send a PR to Greg early next week changing
platform_driver::remove to match remove_new. If these commits don't make
it in in time, I'll be so bold and just include the commits from your
for-next branch in my PR.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |
signature.asc
Description: PGP signature
Re: [PATCH 0/6] ipmi: Convert to platform remove callback returning void
Hello,
On Tue, Mar 05, 2024 at 05:26:57PM +0100, Uwe Kleine-König wrote:
> this series converts all drivers below drivers/char/ipmi to struct
> platform_driver::remove_new(). See commit 5c5a7680e67b ("platform: Provide a
> remove callback that returns no value") for an extended explanation and the
> eventual goal.
>
> All conversations are trivial, because their .remove() callbacks
> returned zero unconditionally.
>
> There are no interdependencies between these patches, so they could be
> picked up individually. But I'd hope that they get picked up all
> together by Corey.
Apart from a (positive) review reply I didn't get any feedback to this
series. My quest to change the prototype of struct
platform_driver::remove depends on these patches, so it would be great
if they made it in during the next merge window.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |
signature.asc
Description: PGP signature
[PATCH] powerpc/powernv: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/char/powernv-op-panel.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/char/powernv-op-panel.c b/drivers/char/powernv-op-panel.c
index 3c99696b145e..f2cff1a6fed5 100644
--- a/drivers/char/powernv-op-panel.c
+++ b/drivers/char/powernv-op-panel.c
@@ -195,12 +195,11 @@ static int oppanel_probe(struct platform_device *pdev)
return rc;
}
-static int oppanel_remove(struct platform_device *pdev)
+static void oppanel_remove(struct platform_device *pdev)
{
misc_deregister(_dev);
kfree(oppanel_lines);
kfree(oppanel_data);
- return 0;
}
static const struct of_device_id oppanel_match[] = {
@@ -214,7 +213,7 @@ static struct platform_driver oppanel_driver = {
.of_match_table = oppanel_match,
},
.probe = oppanel_probe,
- .remove = oppanel_remove,
+ .remove_new = oppanel_remove,
};
module_platform_driver(oppanel_driver);
base-commit: 6ebf211bb11dfc004a2ff73a9de5386fa309c430
--
2.43.0
Re: [PATCH] MAINTAINERS: Drop Li Yang as their email address stopped working
On Tue, Apr 09, 2024 at 02:42:04PM -0700, Jakub Kicinski wrote: > On Fri, 5 Apr 2024 09:20:41 +0200 Uwe Kleine-König wrote: > > When sending a patch to (among others) Li Yang the nxp MTA replied that > > the address doesn't exist and so the mail couldn't be delivered. The > > error code was 550, so at least technically that's not a temporal issue. > > > > Signed-off-by: Uwe Kleine-König > > FWIW it's eaac25d026a1 in net, thanks! Greg also picked it up, it's fbdd90334a6205e8a99d0bc2dfc738ee438f00bc in https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-linus . Both are included in next-20240410. I guess that's not a big problem. (And please prevent that the patch is dropped from both trees as it's already included in the other :-) Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
[PATCH net-next] net: wan: fsl_qmc_hdlc: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
Hello,
the drivers below of drivers/net were already converted to struct
platform_driver::remove_new during the v6.9-rc1 development cycle. This
driver was added for v6.9-rc1 and so missed during my conversion.
There are still more drivers to be converted, so there is from my side
no need to get this into v6.9, but the next merge window would be nice.
Best regards
Uwe
drivers/net/wan/fsl_qmc_hdlc.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wan/fsl_qmc_hdlc.c b/drivers/net/wan/fsl_qmc_hdlc.c
index f69b1f579a0c..c5e7ca793c43 100644
--- a/drivers/net/wan/fsl_qmc_hdlc.c
+++ b/drivers/net/wan/fsl_qmc_hdlc.c
@@ -765,15 +765,13 @@ static int qmc_hdlc_probe(struct platform_device *pdev)
return ret;
}
-static int qmc_hdlc_remove(struct platform_device *pdev)
+static void qmc_hdlc_remove(struct platform_device *pdev)
{
struct qmc_hdlc *qmc_hdlc = platform_get_drvdata(pdev);
unregister_hdlc_device(qmc_hdlc->netdev);
free_netdev(qmc_hdlc->netdev);
qmc_hdlc_framer_exit(qmc_hdlc);
-
- return 0;
}
static const struct of_device_id qmc_hdlc_id_table[] = {
@@ -788,7 +786,7 @@ static struct platform_driver qmc_hdlc_driver = {
.of_match_table = qmc_hdlc_id_table,
},
.probe = qmc_hdlc_probe,
- .remove = qmc_hdlc_remove,
+ .remove_new = qmc_hdlc_remove,
};
module_platform_driver(qmc_hdlc_driver);
base-commit: 11cb68ad52ac78c81e33b806b531f097e68edfa2
--
2.43.0
Zhang Wei also unreachable working
On Fri, Apr 05, 2024 at 09:20:41AM +0200, Uwe Kleine-König wrote: > -M: Li Yang > M: Zhang Wei This address of Zhang Wei doesn't seem to work either. zh-kernel.org doesn't have an MX in DNS, the host behind its A entry doesn't react to pings and connection attempts to tcp port 25 time out. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
[PATCH] MAINTAINERS: Drop Li Yang as their email address stopped working
When sending a patch to (among others) Li Yang the nxp MTA replied that the address doesn't exist and so the mail couldn't be delivered. The error code was 550, so at least technically that's not a temporal issue. Signed-off-by: Uwe Kleine-König --- Hello, I added the affected maintainers and lists to Cc:, maybe someone there knows if this issue is only temporal? @Greg: Given that I noticed the non-existing address when sending an usb patch, I suggest you care for application of this patch (iff it should be applied now). If Li Yang disappeared indeed, I'd prefer to drop the contact from MAINTAINERS early to not give wrong expectations to contributors. Best regards Uwe MAINTAINERS | 11 +++ 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 7c121493f43d..be19aad15045 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2191,7 +2191,6 @@ N:mxs ARM/FREESCALE LAYERSCAPE ARM ARCHITECTURE M: Shawn Guo -M: Li Yang L: linux-arm-ker...@lists.infradead.org (moderated for non-subscribers) S: Maintained T: git git://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git @@ -8523,7 +8522,6 @@ S:Maintained F: drivers/video/fbdev/fsl-diu-fb.* FREESCALE DMA DRIVER -M: Li Yang M: Zhang Wei L: linuxppc-dev@lists.ozlabs.org S: Maintained @@ -8688,10 +8686,9 @@ F: drivers/soc/fsl/qe/tsa.h F: include/dt-bindings/soc/cpm1-fsl,tsa.h FREESCALE QUICC ENGINE UCC ETHERNET DRIVER -M: Li Yang L: net...@vger.kernel.org L: linuxppc-dev@lists.ozlabs.org -S: Maintained +S: Orphan F: drivers/net/ethernet/freescale/ucc_geth* FREESCALE QUICC ENGINE UCC HDLC DRIVER @@ -8708,10 +8705,9 @@ S: Maintained F: drivers/tty/serial/ucc_uart.c FREESCALE SOC DRIVERS -M: Li Yang L: linuxppc-dev@lists.ozlabs.org L: linux-arm-ker...@lists.infradead.org (moderated for non-subscribers) -S: Maintained +S: Orphan F: Documentation/devicetree/bindings/misc/fsl,dpaa2-console.yaml F: Documentation/devicetree/bindings/soc/fsl/ F: drivers/soc/fsl/ @@ -8745,10 +8741,9 @@ F: Documentation/devicetree/bindings/sound/fsl,qmc-audio.yaml F: sound/soc/fsl/fsl_qmc_audio.c FREESCALE USB PERIPHERAL DRIVERS -M: Li Yang L: linux-...@vger.kernel.org L: linuxppc-dev@lists.ozlabs.org -S: Maintained +S: Orphan F: drivers/usb/gadget/udc/fsl* FREESCALE USB PHY DRIVER base-commit: c85af715cac0a951eea97393378e84bb49384734 -- 2.43.0
[PATCH] usb: gadget: fsl: Initialize udc before using it
fsl_ep_queue() is only called by usb_ep_queue() (as ep->ops->queue()).
So _ep isn't NULL.
As ep->ops->queue = fsl_ep_queue, the ep was initialized by
struct_ep_setup() and so ep->udc isn't NULL either.
Drop the check for _ep being NULL and assign udc earlier to prevent
following an uninitialized pointer in the two dev_vdbg()s in lines 878
and 882. This fixes a compiler warning when using clang and
CONFIG_USB_GADGET_VERBOSE=y.
Reported-by: kernel test robot
Closes:
https://lore.kernel.org/oe-kbuild-all/202404050227.ttvccpbu-...@intel.com/
Fixes: 6025f20f16c2 ("usb: gadget: fsl-udc: Replace custom log wrappers by
dev_{err,warn,dbg,vdbg}")
Signed-off-by: Uwe Kleine-König
---
Hello,
6025f20f16c2 was included in v6.9-rc1 (and no earlier release), so
assuming this patch will be merged before v6.9 there is no need for a
stable tag.
Best regards
Uwe
drivers/usb/gadget/udc/fsl_udc_core.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c
b/drivers/usb/gadget/udc/fsl_udc_core.c
index e82d03224f94..3432ebfae978 100644
--- a/drivers/usb/gadget/udc/fsl_udc_core.c
+++ b/drivers/usb/gadget/udc/fsl_udc_core.c
@@ -868,7 +868,7 @@ fsl_ep_queue(struct usb_ep *_ep, struct usb_request *_req,
gfp_t gfp_flags)
{
struct fsl_ep *ep = container_of(_ep, struct fsl_ep, ep);
struct fsl_req *req = container_of(_req, struct fsl_req, req);
- struct fsl_udc *udc;
+ struct fsl_udc *udc = ep->udc;
unsigned long flags;
int ret;
@@ -878,7 +878,7 @@ fsl_ep_queue(struct usb_ep *_ep, struct usb_request *_req,
gfp_t gfp_flags)
dev_vdbg(>gadget.dev, "%s, bad params\n", __func__);
return -EINVAL;
}
- if (unlikely(!_ep || !ep->ep.desc)) {
+ if (unlikely(!ep->ep.desc)) {
dev_vdbg(>gadget.dev, "%s, bad ep\n", __func__);
return -EINVAL;
}
@@ -887,7 +887,6 @@ fsl_ep_queue(struct usb_ep *_ep, struct usb_request *_req,
gfp_t gfp_flags)
return -EMSGSIZE;
}
- udc = ep->udc;
if (!udc->driver || udc->gadget.speed == USB_SPEED_UNKNOWN)
return -ESHUTDOWN;
base-commit: c85af715cac0a951eea97393378e84bb49384734
--
2.43.0
[PATCH] serial: pmac_zilog: Drop usage of platform_driver_probe()
There are considerations to drop platform_driver_probe() as a concept
that isn't relevant any more today. It comes with an added complexity
that makes many users hold it wrong. (E.g. this driver should have
marked the driver struct with __refdata to prevent the below mentioned
false positive section mismatch warning.)
This fixes a W=1 build warning:
WARNING: modpost: drivers/tty/serial/pmac_zilog: section mismatch in
reference: pmz_driver+0x8 (section: .data) -> pmz_detach (section: .exit.text)
Signed-off-by: Uwe Kleine-König
---
drivers/tty/serial/pmac_zilog.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/pmac_zilog.c b/drivers/tty/serial/pmac_zilog.c
index 05d97e89511e..e44621218248 100644
--- a/drivers/tty/serial/pmac_zilog.c
+++ b/drivers/tty/serial/pmac_zilog.c
@@ -1695,7 +1695,7 @@ static void pmz_dispose_port(struct uart_pmac_port *uap)
memset(uap, 0, sizeof(struct uart_pmac_port));
}
-static int __init pmz_attach(struct platform_device *pdev)
+static int pmz_attach(struct platform_device *pdev)
{
struct uart_pmac_port *uap;
int i;
@@ -1714,7 +1714,7 @@ static int __init pmz_attach(struct platform_device *pdev)
return uart_add_one_port(_uart_reg, >port);
}
-static void __exit pmz_detach(struct platform_device *pdev)
+static void pmz_detach(struct platform_device *pdev)
{
struct uart_pmac_port *uap = platform_get_drvdata(pdev);
@@ -1789,7 +1789,8 @@ static struct macio_driver pmz_driver = {
#else
static struct platform_driver pmz_driver = {
- .remove_new = __exit_p(pmz_detach),
+ .probe = pmz_attach,
+ .remove_new = pmz_detach,
.driver = {
.name = "scc",
},
@@ -1837,7 +1838,7 @@ static int __init init_pmz(void)
#ifdef CONFIG_PPC_PMAC
return macio_register_driver(_driver);
#else
- return platform_driver_probe(_driver, pmz_attach);
+ return platform_driver_register(_driver);
#endif
}
base-commit: a6bd6c997f5a0e2667d4d82fef8c970108f2
--
2.43.0
[PATCH 0/6] ipmi: Convert to platform remove callback returning void
Hello,
this series converts all drivers below drivers/char/ipmi to struct
platform_driver::remove_new(). See commit 5c5a7680e67b ("platform: Provide a
remove callback that returns no value") for an extended explanation and the
eventual goal.
All conversations are trivial, because their .remove() callbacks
returned zero unconditionally.
There are no interdependencies between these patches, so they could be
picked up individually. But I'd hope that they get picked up all
together by Corey.
Best regards
Uwe
Uwe Kleine-König (6):
ipmi: bt-bmc: Convert to platform remove callback returning void
ipmi: ipmi_powernv: Convert to platform remove callback returning void
ipmi: ipmi_si_platform: Convert to platform remove callback returning void
ipmi: ipmi_ssif: Convert to platform remove callback returning void
ipmi: kcs_bmc_aspeed: Convert to platform remove callback returning void
ipmi: kcs_bmc_npcm7xx: Convert to platform remove callback returning void
drivers/char/ipmi/bt-bmc.c | 5 ++---
drivers/char/ipmi/ipmi_powernv.c | 6 ++
drivers/char/ipmi/ipmi_si_platform.c | 6 ++
drivers/char/ipmi/ipmi_ssif.c| 5 ++---
drivers/char/ipmi/kcs_bmc_aspeed.c | 6 ++
drivers/char/ipmi/kcs_bmc_npcm7xx.c | 6 ++
6 files changed, 12 insertions(+), 22 deletions(-)
base-commit: 11afac187274a6177a7ac82997f8691c0f469e41
--
2.43.0
[PATCH 2/6] ipmi: ipmi_powernv: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/char/ipmi/ipmi_powernv.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/char/ipmi/ipmi_powernv.c b/drivers/char/ipmi/ipmi_powernv.c
index da22a8cbe68e..c59a86eb58c7 100644
--- a/drivers/char/ipmi/ipmi_powernv.c
+++ b/drivers/char/ipmi/ipmi_powernv.c
@@ -281,15 +281,13 @@ static int ipmi_powernv_probe(struct platform_device
*pdev)
return rc;
}
-static int ipmi_powernv_remove(struct platform_device *pdev)
+static void ipmi_powernv_remove(struct platform_device *pdev)
{
struct ipmi_smi_powernv *smi = dev_get_drvdata(>dev);
ipmi_unregister_smi(smi->intf);
free_irq(smi->irq, smi);
irq_dispose_mapping(smi->irq);
-
- return 0;
}
static const struct of_device_id ipmi_powernv_match[] = {
@@ -304,7 +302,7 @@ static struct platform_driver powernv_ipmi_driver = {
.of_match_table = ipmi_powernv_match,
},
.probe = ipmi_powernv_probe,
- .remove = ipmi_powernv_remove,
+ .remove_new = ipmi_powernv_remove,
};
--
2.43.0
Re: [PATCH 00/11] misc: Convert to platform remove callback returning void
Hello Arnd, hello Greg,
On Wed, Feb 21, 2024 at 02:52:29PM +0100, Arnd Bergmann wrote:
> On Wed, Feb 21, 2024, at 10:53, Uwe Kleine-König wrote:
> > Hello,
> >
> > this series converts all drivers below drivers/misc to struct
> > platform_driver::remove_new(). See commit 5c5a7680e67b ("platform:
> > Provide a remove callback that returns no value") for an extended
> > explanation and the eventual goal.
> >
> > All conversations are trivial, because their .remove() callbacks
> > returned zero unconditionally.
> >
> > There are no interdependencies between these patches, so they could be
> > picked up individually. But I'd hope that Greg or Arnd picks them up all
> > together.
>
> These all look good to me, whole series
>
> Acked-by: Arnd Bergmann
Thanks.
You (= Arnd and Greg) are the listed maintainers for drivers/misc/. How
is this series supposed to be merged? Would a pull request help?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |
signature.asc
Description: PGP signature
[PATCH v2] usb: gadget: fsl-udc: Replace custom log wrappers by dev_{err,warn,dbg,vdbg}
The custom log wrappers ERR, WARNING, DBG and VDBG don't add anything
useful that cannot easily be done with dev_err() and friends. Replace
the custom stuff by the well known functions from printk.h.
Also drop some dead code in a #if 0 block.
Signed-off-by: Uwe Kleine-König
---
Changes since (implicit) v1:
- Use dev_* instead of pr_*
- Adapt commit log accordingly.
drivers/usb/gadget/udc/fsl_udc_core.c | 115 ++
drivers/usb/gadget/udc/fsl_usb2_udc.h | 47 ---
2 files changed, 62 insertions(+), 100 deletions(-)
diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c
b/drivers/usb/gadget/udc/fsl_udc_core.c
index e8042c158f6d..840d024fb75b 100644
--- a/drivers/usb/gadget/udc/fsl_udc_core.c
+++ b/drivers/usb/gadget/udc/fsl_udc_core.c
@@ -13,7 +13,7 @@
* code from Dave Liu and Shlomi Gridish.
*/
-#undef VERBOSE
+#define pr_fmt(x) "udc: " x
#include
#include
@@ -183,9 +183,9 @@ __acquires(ep->udc->lock)
usb_gadget_unmap_request(>udc->gadget, >req, ep_is_in(ep));
if (status && (status != -ESHUTDOWN))
- VDBG("complete %s req %p stat %d len %u/%u",
- ep->ep.name, >req, status,
- req->req.actual, req->req.length);
+ dev_vdbg(>gadget.dev, "complete %s req %p stat %d len
%u/%u\n",
+ep->ep.name, >req, status,
+req->req.actual, req->req.length);
ep->stopped = 1;
@@ -285,7 +285,7 @@ static int dr_controller_setup(struct fsl_udc *udc)
timeout = jiffies + FSL_UDC_RESET_TIMEOUT;
while (fsl_readl(_regs->usbcmd) & USB_CMD_CTRL_RESET) {
if (time_after(jiffies, timeout)) {
- ERR("udc reset timeout!\n");
+ dev_err(>gadget.dev, "udc reset timeout!\n");
return -ETIMEDOUT;
}
cpu_relax();
@@ -308,9 +308,10 @@ static int dr_controller_setup(struct fsl_udc *udc)
tmp &= USB_EP_LIST_ADDRESS_MASK;
fsl_writel(tmp, _regs->endpointlistaddr);
- VDBG("vir[qh_base] is %p phy[qh_base] is 0x%8x reg is 0x%8x",
- udc->ep_qh, (int)tmp,
- fsl_readl(_regs->endpointlistaddr));
+ dev_vdbg(>gadget.dev,
+"vir[qh_base] is %p phy[qh_base] is 0x%8x reg is 0x%8x\n",
+udc->ep_qh, (int)tmp,
+fsl_readl(_regs->endpointlistaddr));
max_no_of_ep = (0x001F & fsl_readl(_regs->dccparams));
for (ep_num = 1; ep_num < max_no_of_ep; ep_num++) {
@@ -498,7 +499,7 @@ static void struct_ep_qh_setup(struct fsl_udc *udc,
unsigned char ep_num,
tmp = max_pkt_len << EP_QUEUE_HEAD_MAX_PKT_LEN_POS;
break;
default:
- VDBG("error ep type is %d", ep_type);
+ dev_vdbg(>gadget.dev, "error ep type is %d\n", ep_type);
return;
}
if (zlt)
@@ -611,10 +612,10 @@ static int fsl_ep_enable(struct usb_ep *_ep,
spin_unlock_irqrestore(>lock, flags);
retval = 0;
- VDBG("enabled %s (ep%d%s) maxpacket %d",ep->ep.name,
- ep->ep.desc->bEndpointAddress & 0x0f,
- (desc->bEndpointAddress & USB_DIR_IN)
- ? "in" : "out", max);
+ dev_vdbg(>gadget.dev, "enabled %s (ep%d%s) maxpacket %d\n",
+ep->ep.name, ep->ep.desc->bEndpointAddress & 0x0f,
+(desc->bEndpointAddress & USB_DIR_IN) ? "in" : "out",
+max);
en_done:
return retval;
}
@@ -633,7 +634,10 @@ static int fsl_ep_disable(struct usb_ep *_ep)
ep = container_of(_ep, struct fsl_ep, ep);
if (!_ep || !ep->ep.desc) {
- VDBG("%s not enabled", _ep ? ep->ep.name : NULL);
+ /*
+* dev_vdbg(>gadget.dev, "%s not enabled\n",
+* _ep ? ep->ep.name : NULL);
+*/
return -EINVAL;
}
@@ -659,7 +663,7 @@ static int fsl_ep_disable(struct usb_ep *_ep)
ep->stopped = 1;
spin_unlock_irqrestore(>lock, flags);
- VDBG("disabled %s OK", _ep->name);
+ dev_vdbg(>gadget.dev, "disabled %s OK\n", _ep->name);
return 0;
}
@@ -719,8 +723,8 @@ static void fsl_queue_td(struct fsl_ep *ep, struct fsl_req
*req)
{
u32 temp, bitmask, tmp_stat;
- /* VDBG("QH addr Register 0x%8x", dr_regs->endpointlistaddr);
- VDBG("ep_qh[%d] addr is 0x%8x", i, (u32)&(ep->udc->ep_qh[i])); */
+ /* dev_vdbg(>
[PATCH] usb: gadget: fsl-udc: Replace custom log wrappers by pr_{err,warn,debug}
The custom log wrappers ERR, WARNING and DBG don't add anything useful
that cannot easily be done with pr_err() and friends. Replace the custom
stuff by the well known functions from printk.h.
Signed-off-by: Uwe Kleine-König
---
Hello,
I didn't replace VDBG by pr_vdebug because the latter doesn't exist.
Alternatively all these macros could be replaced by
dev_{err,warn,dbg,vdbg}, which however would be a bigger change that
affects the output more.
Thoughts?
Best regards
Uwe
drivers/usb/gadget/udc/fsl_udc_core.c | 36 ++-
drivers/usb/gadget/udc/fsl_usb2_udc.h | 19 +++---
2 files changed, 23 insertions(+), 32 deletions(-)
diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c
b/drivers/usb/gadget/udc/fsl_udc_core.c
index e8042c158f6d..ebe611d4a7f9 100644
--- a/drivers/usb/gadget/udc/fsl_udc_core.c
+++ b/drivers/usb/gadget/udc/fsl_udc_core.c
@@ -15,6 +15,8 @@
#undef VERBOSE
+#define pr_fmt(x) "udc: " x
+
#include
#include
#include
@@ -285,7 +287,7 @@ static int dr_controller_setup(struct fsl_udc *udc)
timeout = jiffies + FSL_UDC_RESET_TIMEOUT;
while (fsl_readl(_regs->usbcmd) & USB_CMD_CTRL_RESET) {
if (time_after(jiffies, timeout)) {
- ERR("udc reset timeout!\n");
+ pr_err("udc reset timeout!\n");
return -ETIMEDOUT;
}
cpu_relax();
@@ -1105,7 +1107,7 @@ static void fsl_ep_fifo_flush(struct usb_ep *_ep)
/* Wait until flush complete */
while (fsl_readl(_regs->endptflush)) {
if (time_after(jiffies, timeout)) {
- ERR("ep flush timeout\n");
+ pr_err("ep flush timeout\n");
return;
}
cpu_relax();
@@ -1543,7 +1545,7 @@ static void ep0_req_complete(struct fsl_udc *udc, struct
fsl_ep *ep0,
udc->ep0_state = WAIT_FOR_SETUP;
break;
case WAIT_FOR_SETUP:
- ERR("Unexpected ep0 packets\n");
+ pr_err("Unexpected ep0 packets\n");
break;
default:
ep0stall(udc);
@@ -1612,7 +1614,7 @@ static int process_ep_req(struct fsl_udc *udc, int pipe,
errors = hc32_to_cpu(curr_td->size_ioc_sts);
if (errors & DTD_ERROR_MASK) {
if (errors & DTD_STATUS_HALTED) {
- ERR("dTD error %08x QH=%d\n", errors, pipe);
+ pr_err("dTD error %08x QH=%d\n", errors, pipe);
/* Clear the errors and Halt condition */
tmp = hc32_to_cpu(curr_qh->size_ioc_int_sts);
tmp &= ~errors;
@@ -1631,7 +1633,7 @@ static int process_ep_req(struct fsl_udc *udc, int pipe,
status = -EILSEQ;
break;
} else
- ERR("Unknown error has occurred (0x%x)!\n",
+ pr_err("Unknown error has occurred (0x%x)!\n",
errors);
} else if (hc32_to_cpu(curr_td->size_ioc_sts)
@@ -1691,7 +1693,7 @@ static void dtd_complete_irq(struct fsl_udc *udc)
/* If the ep is configured */
if (!curr_ep->ep.name) {
- WARNING("Invalid EP?");
+ pr_warn("Invalid EP?\n");
continue;
}
@@ -1820,7 +1822,7 @@ static void reset_irq(struct fsl_udc *udc)
while (fsl_readl(_regs->endpointprime)) {
/* Wait until all endptprime bits cleared */
if (time_after(jiffies, timeout)) {
- ERR("Timeout for reset\n");
+ pr_err("Timeout for reset\n");
break;
}
cpu_relax();
@@ -1958,7 +1960,7 @@ static int fsl_udc_start(struct usb_gadget *g,
udc_controller->transceiver->otg,
_controller->gadget);
if (retval < 0) {
- ERR("can't bind to transceiver\n");
+ pr_err("can't bind to transceiver\n");
udc_controller->driver = NULL;
return retval;
}
@@ -2243,7 +2245,7 @@ static int struct_udc_setup(struct fsl_udc *udc,
udc->eps = kcalloc(udc->max_ep, sizeof(struct fsl_ep), GFP_KERNEL);
if (!udc->eps) {
-
Re: [PATCH] powerpc: Add allmodconfig for all 32-bit sub-arches
On Thu, Feb 29, 2024 at 10:41:08PM +1100, Michael Ellerman wrote: > 32-bit powerpc kernels can be built for one of 5 sub-arches, see > Kconfig.cputype: > > PPC_BOOK3S_32: "512x/52xx/6xx/7xx/74xx/82xx/83xx/86xx" > PPC_85xx: "Freescale 85xx" > PPC_8xx: "Freescale 8xx" > 40x: "AMCC 40x" > 44x: "AMCC 44x, 46x or 47x" > > By default none of these are built for a plain allmodconfig build, > because it selects PPC64 which builds a 64-bit kernel. > > There is already a ppc32_allmodconfig, which enables PPC_BOOK3S_32. > > Add similar targets for the other 32-bit sub-arches to increase build > coverage: > ppc40x_allmodconfig > ppc44x_allmodconfig > ppc8xx_allmodconfig > ppc85xx_allmodconfig > > Signed-off-by: Michael Ellerman \o/ Acked-by: Uwe Kleine-König Thanks Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
Re: Increasing build coverage for drivers/spi/spi-ppc4xx.c
On Tue, Feb 27, 2024 at 01:52:07PM +, Christophe Leroy wrote:
>
>
> Le 27/02/2024 à 11:58, Uwe Kleine-König a écrit :
> > Hello Christophe,
> >
> > On Tue, Feb 27, 2024 at 10:25:15AM +, Christophe Leroy wrote:
> >> Le 27/02/2024 à 09:46, Uwe Kleine-König a écrit :
> >>> recently the spi-ppc4xx.c driver suffered from build errors and warnings
> >>> that were undetected for longer than I expected. I think it would be
> >>> very beneficial if this driver was enabled in (at least) a powerpc
> >>> allmodconfig build.
> >>>
> >>> The challenge to do so is that spi-ppc4xx.c uses dcri_clrset() which is
> >>> only defined for 4xx (as these select PPC_DCR_NATIVE).
> >>>
> >>> I wonder if dcri_clrset() could be defined for the PPC_DCR_MMIO case,
> >>> too. I tried and failed. The best I came up without extensive doc
> >>> reading is:
> >>>
> >>> diff --git a/arch/powerpc/include/asm/dcr-native.h
> >>> b/arch/powerpc/include/asm/dcr-native.h
> >>> index a92059964579..159ab7abfe46 100644
> >>> --- a/arch/powerpc/include/asm/dcr-native.h
> >>> +++ b/arch/powerpc/include/asm/dcr-native.h
> >>> @@ -115,15 +115,11 @@ static inline void __dcri_clrset(int base_addr, int
> >>> base_data, int reg,
> >>> unsigned int val;
> >>>
> >>> spin_lock_irqsave(_ind_lock, flags);
> >>> - if (cpu_has_feature(CPU_FTR_INDEXED_DCR)) {
> >>> - mtdcrx(base_addr, reg);
> >>> - val = (mfdcrx(base_data) & ~clr) | set;
> >>> - mtdcrx(base_data, val);
> >>> - } else {
> >>> - __mtdcr(base_addr, reg);
> >>> - val = (__mfdcr(base_data) & ~clr) | set;
> >>> - __mtdcr(base_data, val);
> >>> - }
> >>> +
> >>> + mtdcr(base_addr, reg);
> >>> + val = (mfdcr(base_data) & ~clr) | set;
> >>> + mtdcr(base_data, val);
> >>> +
> >>> spin_unlock_irqrestore(_ind_lock, flags);
> >>>}
> >>>
> >>> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> >>> index bc7021da2fe9..9a0a5e8c70c8 100644
> >>> --- a/drivers/spi/Kconfig
> >>> +++ b/drivers/spi/Kconfig
> >>> @@ -810,7 +810,8 @@ config SPI_PL022
> >>>
> >>>config SPI_PPC4xx
> >>> tristate "PPC4xx SPI Controller"
> >>> - depends on PPC32 && 4xx
> >>> + depends on 4xx || COMPILE_TEST
> >>> + depends on PPC32 || PPC64
> >>> select SPI_BITBANG
> >>> help
> >>> This selects a driver for the PPC4xx SPI Controller.
> >>>
> >>> While this is a step in the right direction (I think) it's not enough to
> >>> make the driver build (but maybe make it easier to define
> >>> dcri_clrset()?)
> >>>
> >>> Could someone with more powerpc knowledge jump in and help (for the
> >>> benefit of better compile coverage of the spi driver and so less
> >>> breakage)? (If you do so based on my changes above, you don't need to
> >>> credit me for my effort, claim it as your's. I'm happy enough if the
> >>> situation improves.)
> >>
> >> What about this ?
> >>
> >> diff --git a/arch/powerpc/include/asm/dcr-mmio.h
> >> b/arch/powerpc/include/asm/dcr-mmio.h
> >> index fc6d93ef4a13..38b515afbffc 100644
> >> --- a/arch/powerpc/include/asm/dcr-mmio.h
> >> +++ b/arch/powerpc/include/asm/dcr-mmio.h
> >> @@ -38,6 +38,11 @@ static inline void dcr_write_mmio(dcr_host_mmio_t host,
> >>out_be32(host.token + ((host.base + dcr_n) * host.stride),
> >> value);
> >>}
> >>
> >> +static inline void __dcri_clrset(int base_addr, int base_data, int reg,
> >> + unsigned clr, unsigned set)
> >> +{
> >> +}
> >> +
> >
> > The downside of that one is that if we find a matching device where
> > dcr-mmio is used, the driver claims to work but silently fails. Is this
> > good enough?
>
> I don't know the details of DCR, but it looks like this spi-ppc4xx
> driver is really specific to 4xx, which is PPC32.
>
> Do you really need/want it to be built with allmodconfig ?
>
> Maybe it would be easier to have it work with ppc32_allmodconfig ?
>
> Or even easier with ppc44x_defconfig ?
The reason I'd like to see it in allmodconfig is that testing
allmodconfig on several archs is the check I do for my patch series.
Also I assume I'm not the only one relying on allmodconfig for this
purpose. So in my eyes every driver that is built there is a net win.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |
signature.asc
Description: PGP signature
Re: Increasing build coverage for drivers/spi/spi-ppc4xx.c
Hello Christophe,
On Tue, Feb 27, 2024 at 10:25:15AM +, Christophe Leroy wrote:
> Le 27/02/2024 à 09:46, Uwe Kleine-König a écrit :
> > recently the spi-ppc4xx.c driver suffered from build errors and warnings
> > that were undetected for longer than I expected. I think it would be
> > very beneficial if this driver was enabled in (at least) a powerpc
> > allmodconfig build.
> >
> > The challenge to do so is that spi-ppc4xx.c uses dcri_clrset() which is
> > only defined for 4xx (as these select PPC_DCR_NATIVE).
> >
> > I wonder if dcri_clrset() could be defined for the PPC_DCR_MMIO case,
> > too. I tried and failed. The best I came up without extensive doc
> > reading is:
> >
> > diff --git a/arch/powerpc/include/asm/dcr-native.h
> > b/arch/powerpc/include/asm/dcr-native.h
> > index a92059964579..159ab7abfe46 100644
> > --- a/arch/powerpc/include/asm/dcr-native.h
> > +++ b/arch/powerpc/include/asm/dcr-native.h
> > @@ -115,15 +115,11 @@ static inline void __dcri_clrset(int base_addr, int
> > base_data, int reg,
> > unsigned int val;
> >
> > spin_lock_irqsave(_ind_lock, flags);
> > - if (cpu_has_feature(CPU_FTR_INDEXED_DCR)) {
> > - mtdcrx(base_addr, reg);
> > - val = (mfdcrx(base_data) & ~clr) | set;
> > - mtdcrx(base_data, val);
> > - } else {
> > - __mtdcr(base_addr, reg);
> > - val = (__mfdcr(base_data) & ~clr) | set;
> > - __mtdcr(base_data, val);
> > - }
> > +
> > + mtdcr(base_addr, reg);
> > + val = (mfdcr(base_data) & ~clr) | set;
> > + mtdcr(base_data, val);
> > +
> > spin_unlock_irqrestore(_ind_lock, flags);
> > }
> >
> > diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> > index bc7021da2fe9..9a0a5e8c70c8 100644
> > --- a/drivers/spi/Kconfig
> > +++ b/drivers/spi/Kconfig
> > @@ -810,7 +810,8 @@ config SPI_PL022
> >
> > config SPI_PPC4xx
> > tristate "PPC4xx SPI Controller"
> > - depends on PPC32 && 4xx
> > + depends on 4xx || COMPILE_TEST
> > + depends on PPC32 || PPC64
> > select SPI_BITBANG
> > help
> > This selects a driver for the PPC4xx SPI Controller.
> >
> > While this is a step in the right direction (I think) it's not enough to
> > make the driver build (but maybe make it easier to define
> > dcri_clrset()?)
> >
> > Could someone with more powerpc knowledge jump in and help (for the
> > benefit of better compile coverage of the spi driver and so less
> > breakage)? (If you do so based on my changes above, you don't need to
> > credit me for my effort, claim it as your's. I'm happy enough if the
> > situation improves.)
>
> What about this ?
>
> diff --git a/arch/powerpc/include/asm/dcr-mmio.h
> b/arch/powerpc/include/asm/dcr-mmio.h
> index fc6d93ef4a13..38b515afbffc 100644
> --- a/arch/powerpc/include/asm/dcr-mmio.h
> +++ b/arch/powerpc/include/asm/dcr-mmio.h
> @@ -38,6 +38,11 @@ static inline void dcr_write_mmio(dcr_host_mmio_t host,
> out_be32(host.token + ((host.base + dcr_n) * host.stride), value);
> }
>
> +static inline void __dcri_clrset(int base_addr, int base_data, int reg,
> + unsigned clr, unsigned set)
> +{
> +}
> +
The downside of that one is that if we find a matching device where
dcr-mmio is used, the driver claims to work but silently fails. Is this
good enough?
> #endif /* __KERNEL__ */
> #endif /* _ASM_POWERPC_DCR_MMIO_H */
>
> diff --git a/arch/powerpc/include/asm/dcr-native.h
> b/arch/powerpc/include/asm/dcr-native.h
> index a92059964579..2f6221bf5406 100644
> --- a/arch/powerpc/include/asm/dcr-native.h
> +++ b/arch/powerpc/include/asm/dcr-native.h
> @@ -135,10 +135,6 @@ static inline void __dcri_clrset(int base_addr, int
> base_data, int reg,
>DCRN_ ## base ## _CONFIG_DATA, \
>reg, data)
>
> -#define dcri_clrset(base, reg, clr, set) __dcri_clrset(DCRN_ ## base ##
> _CONFIG_ADDR, \
> - DCRN_ ## base ##
> _CONFIG_DATA,\
> - reg, clr, set)
> -
> #endif /* __ASSEMBLY__ */
> #endif /* __KERNEL__ */
> #endif /* _ASM_POWERPC_DCR_NATIVE_H */
> diff --git a/arch/powerpc/include/asm/dcr.h b/arch/powerpc/include/asm/dcr.h
> index 64030e3a1f30..15c123ae38a1 100644
> --- a/arch/powerpc/include/asm/dcr.h
> +++ b/arch/powerpc/include/asm/dcr.h
> @@ -18,6 +1
Re: Increasing build coverage for drivers/spi/spi-ppc4xx.c
Hello, On Tue, Feb 27, 2024 at 08:54:03AM +, Tudor Ambarus wrote: > On 2/27/24 08:46, Uwe Kleine-König wrote: > > recently the spi-ppc4xx.c driver suffered from build errors and warnings > > that were undetected for longer than I expected. I think it would be > > long enough so that we remove the driver altogether? I know at least one user who noticed the driver being broken because he needs it and not because a build bot stumbled. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
Increasing build coverage for drivers/spi/spi-ppc4xx.c
Hello,
recently the spi-ppc4xx.c driver suffered from build errors and warnings
that were undetected for longer than I expected. I think it would be
very beneficial if this driver was enabled in (at least) a powerpc
allmodconfig build.
The challenge to do so is that spi-ppc4xx.c uses dcri_clrset() which is
only defined for 4xx (as these select PPC_DCR_NATIVE).
I wonder if dcri_clrset() could be defined for the PPC_DCR_MMIO case,
too. I tried and failed. The best I came up without extensive doc
reading is:
diff --git a/arch/powerpc/include/asm/dcr-native.h
b/arch/powerpc/include/asm/dcr-native.h
index a92059964579..159ab7abfe46 100644
--- a/arch/powerpc/include/asm/dcr-native.h
+++ b/arch/powerpc/include/asm/dcr-native.h
@@ -115,15 +115,11 @@ static inline void __dcri_clrset(int base_addr, int
base_data, int reg,
unsigned int val;
spin_lock_irqsave(_ind_lock, flags);
- if (cpu_has_feature(CPU_FTR_INDEXED_DCR)) {
- mtdcrx(base_addr, reg);
- val = (mfdcrx(base_data) & ~clr) | set;
- mtdcrx(base_data, val);
- } else {
- __mtdcr(base_addr, reg);
- val = (__mfdcr(base_data) & ~clr) | set;
- __mtdcr(base_data, val);
- }
+
+ mtdcr(base_addr, reg);
+ val = (mfdcr(base_data) & ~clr) | set;
+ mtdcr(base_data, val);
+
spin_unlock_irqrestore(_ind_lock, flags);
}
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index bc7021da2fe9..9a0a5e8c70c8 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -810,7 +810,8 @@ config SPI_PL022
config SPI_PPC4xx
tristate "PPC4xx SPI Controller"
- depends on PPC32 && 4xx
+ depends on 4xx || COMPILE_TEST
+ depends on PPC32 || PPC64
select SPI_BITBANG
help
This selects a driver for the PPC4xx SPI Controller.
While this is a step in the right direction (I think) it's not enough to
make the driver build (but maybe make it easier to define
dcri_clrset()?)
Could someone with more powerpc knowledge jump in and help (for the
benefit of better compile coverage of the spi driver and so less
breakage)? (If you do so based on my changes above, you don't need to
credit me for my effort, claim it as your's. I'm happy enough if the
situation improves.)
Thanks
Uwe
--
Pengutronix e.K. | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |
signature.asc
Description: PGP signature
[PATCH 0/2] usb: gadget: fsl: Two improvements
Hello, here come two patches that I created while debugging an issue around USB on MPC8313. Best regards Uwe Uwe Kleine-König (2): usb: gadget: fsl: Add of device table to enable module autoloading usb: gadget: fsl: Increase size of name buffer for endpoints drivers/usb/gadget/udc/fsl_udc_core.c | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) base-commit: 33e1d31873f87d119e5120b88cd350efa68ef276 -- 2.43.0
[PATCH 1/2] usb: gadget: fsl: Add of device table to enable module autoloading
With this table added, the fsl_usb2_udc module is automatically loaded
by udev in the presence of a matching device.
Signed-off-by: Uwe Kleine-König
---
drivers/usb/gadget/udc/fsl_udc_core.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c
b/drivers/usb/gadget/udc/fsl_udc_core.c
index e8042c158f6d..65346a322a77 100644
--- a/drivers/usb/gadget/udc/fsl_udc_core.c
+++ b/drivers/usb/gadget/udc/fsl_udc_core.c
@@ -2666,6 +2666,15 @@ static const struct platform_device_id fsl_udc_devtype[]
= {
}
};
MODULE_DEVICE_TABLE(platform, fsl_udc_devtype);
+
+static const struct of_device_id fsl_udc_dt_ids[] = {
+ { .compatible = "fsl-usb2-dr" },
+ { .compatible = "fsl-usb2-mph" },
+ { .compatible = "fsl,mpc5121-usb2-dr" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, fsl_udc_dt_ids);
+
static struct platform_driver udc_driver = {
.probe = fsl_udc_probe,
.remove_new = fsl_udc_remove,
@@ -2675,6 +2684,7 @@ static struct platform_driver udc_driver = {
.resume = fsl_udc_resume,
.driver = {
.name = driver_name,
+ .of_match_table = fsl_udc_dt_ids,
/* udc suspend/resume called from OTG driver */
.suspend = fsl_udc_otg_suspend,
.resume = fsl_udc_otg_resume,
--
2.43.0
[PATCH 2/2] usb: gadget: fsl: Increase size of name buffer for endpoints
This fixes a W=1 warning about sprintf writing up to 16 bytes into a
buffer of size 14. There is no practical relevance because there are not
more than 32 endpoints.
Signed-off-by: Uwe Kleine-König
---
Hello,
I hesitated to add a Fixes: trailer because the problem is never hit in
practise. Anyhow, if you think it's a good idea to add one, the commit
introducing the too small buffer is:
b504882da539 ("USB: add Freescale high-speed USB SOC device controller
driver")
Best regards
Uwe
drivers/usb/gadget/udc/fsl_udc_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c
b/drivers/usb/gadget/udc/fsl_udc_core.c
index 65346a322a77..b7e7ae5e051b 100644
--- a/drivers/usb/gadget/udc/fsl_udc_core.c
+++ b/drivers/usb/gadget/udc/fsl_udc_core.c
@@ -2486,7 +2486,7 @@ static int fsl_udc_probe(struct platform_device *pdev)
/* setup the udc->eps[] for non-control endpoints and link
* to gadget.ep_list */
for (i = 1; i < (int)(udc_controller->max_ep / 2); i++) {
- char name[14];
+ char name[16];
sprintf(name, "ep%dout", i);
struct_ep_setup(udc_controller, i * 2, name, 1);
--
2.43.0
[PATCH 3/6] powerpc: opal-prd: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
arch/powerpc/platforms/powernv/opal-prd.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/opal-prd.c
b/arch/powerpc/platforms/powernv/opal-prd.c
index b66b06efcef1..24f04f20d3e8 100644
--- a/arch/powerpc/platforms/powernv/opal-prd.c
+++ b/arch/powerpc/platforms/powernv/opal-prd.c
@@ -425,12 +425,11 @@ static int opal_prd_probe(struct platform_device *pdev)
return 0;
}
-static int opal_prd_remove(struct platform_device *pdev)
+static void opal_prd_remove(struct platform_device *pdev)
{
misc_deregister(_prd_dev);
opal_message_notifier_unregister(OPAL_MSG_PRD, _prd_event_nb);
opal_message_notifier_unregister(OPAL_MSG_PRD2, _prd_event_nb2);
- return 0;
}
static const struct of_device_id opal_prd_match[] = {
@@ -444,7 +443,7 @@ static struct platform_driver opal_prd_driver = {
.of_match_table = opal_prd_match,
},
.probe = opal_prd_probe,
- .remove = opal_prd_remove,
+ .remove_new = opal_prd_remove,
};
module_platform_driver(opal_prd_driver);
--
2.43.0
[PATCH 1/6] powerpc: sgy_cts1000: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
arch/powerpc/platforms/85xx/sgy_cts1000.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/platforms/85xx/sgy_cts1000.c
b/arch/powerpc/platforms/85xx/sgy_cts1000.c
index 751395cbf022..34ce21f42623 100644
--- a/arch/powerpc/platforms/85xx/sgy_cts1000.c
+++ b/arch/powerpc/platforms/85xx/sgy_cts1000.c
@@ -114,7 +114,7 @@ static int gpio_halt_probe(struct platform_device *pdev)
return ret;
}
-static int gpio_halt_remove(struct platform_device *pdev)
+static void gpio_halt_remove(struct platform_device *pdev)
{
free_irq(halt_irq, pdev);
cancel_work_sync(_halt_wq);
@@ -124,8 +124,6 @@ static int gpio_halt_remove(struct platform_device *pdev)
gpiod_put(halt_gpio);
halt_gpio = NULL;
-
- return 0;
}
static const struct of_device_id gpio_halt_match[] = {
@@ -145,7 +143,7 @@ static struct platform_driver gpio_halt_driver = {
.of_match_table = gpio_halt_match,
},
.probe = gpio_halt_probe,
- .remove = gpio_halt_remove,
+ .remove_new = gpio_halt_remove,
};
module_platform_driver(gpio_halt_driver);
--
2.43.0
[PATCH 0/6] powerpc: Convert to platform remove callback returning void
Hello,
this series converts all platform drivers below drivers/powerpc to
struct platform_driver::remove_new(). See commit 5c5a7680e67b
("platform: Provide a remove callback that returns no value") for an
extended explanation and the eventual goal.
All conversations are trivial, because their .remove() callbacks
returned zero unconditionally.
There are no interdependencies between these patches, so they could be
picked up individually. But I'd hope that Michael picks them up all
together.
Best regards
Uwe
Uwe Kleine-König (6):
powerpc: sgy_cts1000: Convert to platform remove callback returning void
powerpc: gpio_mdio: Convert to platform remove callback returning void
powerpc: opal-prd: Convert to platform remove callback returning void
powerpc: papr_scm: Convert to platform remove callback returning void
powerpc: fsl_msi: Convert to platform remove callback returning void
powerpc: pmi: Convert to platform remove callback returning void
arch/powerpc/platforms/85xx/sgy_cts1000.c | 6 ++
arch/powerpc/platforms/pasemi/gpio_mdio.c | 6 ++
arch/powerpc/platforms/powernv/opal-prd.c | 5 ++---
arch/powerpc/platforms/pseries/papr_scm.c | 6 ++
arch/powerpc/sysdev/fsl_msi.c | 6 ++
arch/powerpc/sysdev/pmi.c | 6 ++
6 files changed, 12 insertions(+), 23 deletions(-)
base-commit: 4893c639cc3659cefaa675bf1e59f4e7571afb5c
--
2.43.0
[PATCH 6/6] powerpc: pmi: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
arch/powerpc/sysdev/pmi.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/sysdev/pmi.c b/arch/powerpc/sysdev/pmi.c
index fcf8d1516210..737f97fd67d7 100644
--- a/arch/powerpc/sysdev/pmi.c
+++ b/arch/powerpc/sysdev/pmi.c
@@ -173,7 +173,7 @@ static int pmi_of_probe(struct platform_device *dev)
return rc;
}
-static int pmi_of_remove(struct platform_device *dev)
+static void pmi_of_remove(struct platform_device *dev)
{
struct pmi_handler *handler, *tmp;
@@ -189,13 +189,11 @@ static int pmi_of_remove(struct platform_device *dev)
kfree(data);
data = NULL;
-
- return 0;
}
static struct platform_driver pmi_of_platform_driver = {
.probe = pmi_of_probe,
- .remove = pmi_of_remove,
+ .remove_new = pmi_of_remove,
.driver = {
.name = "pmi",
.of_match_table = pmi_match,
--
2.43.0
[PATCH 2/6] powerpc: gpio_mdio: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
arch/powerpc/platforms/pasemi/gpio_mdio.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/platforms/pasemi/gpio_mdio.c
b/arch/powerpc/platforms/pasemi/gpio_mdio.c
index fd130fe7a65a..4e983af32949 100644
--- a/arch/powerpc/platforms/pasemi/gpio_mdio.c
+++ b/arch/powerpc/platforms/pasemi/gpio_mdio.c
@@ -260,7 +260,7 @@ static int gpio_mdio_probe(struct platform_device *ofdev)
}
-static int gpio_mdio_remove(struct platform_device *dev)
+static void gpio_mdio_remove(struct platform_device *dev)
{
struct mii_bus *bus = dev_get_drvdata(>dev);
@@ -271,8 +271,6 @@ static int gpio_mdio_remove(struct platform_device *dev)
kfree(bus->priv);
bus->priv = NULL;
mdiobus_free(bus);
-
- return 0;
}
static const struct of_device_id gpio_mdio_match[] =
@@ -287,7 +285,7 @@ MODULE_DEVICE_TABLE(of, gpio_mdio_match);
static struct platform_driver gpio_mdio_driver =
{
.probe = gpio_mdio_probe,
- .remove = gpio_mdio_remove,
+ .remove_new = gpio_mdio_remove,
.driver = {
.name = "gpio-mdio-bitbang",
.of_match_table = gpio_mdio_match,
--
2.43.0
[PATCH 4/6] powerpc: papr_scm: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
arch/powerpc/platforms/pseries/papr_scm.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c
b/arch/powerpc/platforms/pseries/papr_scm.c
index 1a53e048ceb7..c233f9db039b 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -1521,7 +1521,7 @@ err: kfree(p);
return rc;
}
-static int papr_scm_remove(struct platform_device *pdev)
+static void papr_scm_remove(struct platform_device *pdev)
{
struct papr_scm_priv *p = platform_get_drvdata(pdev);
@@ -1538,8 +1538,6 @@ static int papr_scm_remove(struct platform_device *pdev)
pdev->archdata.priv = NULL;
kfree(p->bus_desc.provider_name);
kfree(p);
-
- return 0;
}
static const struct of_device_id papr_scm_match[] = {
@@ -1550,7 +1548,7 @@ static const struct of_device_id papr_scm_match[] = {
static struct platform_driver papr_scm_driver = {
.probe = papr_scm_probe,
- .remove = papr_scm_remove,
+ .remove_new = papr_scm_remove,
.driver = {
.name = "papr_scm",
.of_match_table = papr_scm_match,
--
2.43.0
[PATCH 5/6] powerpc: fsl_msi: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
arch/powerpc/sysdev/fsl_msi.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/sysdev/fsl_msi.c b/arch/powerpc/sysdev/fsl_msi.c
index 558ec68d768e..8e6c84df4ca1 100644
--- a/arch/powerpc/sysdev/fsl_msi.c
+++ b/arch/powerpc/sysdev/fsl_msi.c
@@ -320,7 +320,7 @@ static irqreturn_t fsl_msi_cascade(int irq, void *data)
return ret;
}
-static int fsl_of_msi_remove(struct platform_device *ofdev)
+static void fsl_of_msi_remove(struct platform_device *ofdev)
{
struct fsl_msi *msi = platform_get_drvdata(ofdev);
int virq, i;
@@ -343,8 +343,6 @@ static int fsl_of_msi_remove(struct platform_device *ofdev)
if ((msi->feature & FSL_PIC_IP_MASK) != FSL_PIC_IP_VMPIC)
iounmap(msi->msi_regs);
kfree(msi);
-
- return 0;
}
static struct lock_class_key fsl_msi_irq_class;
@@ -603,7 +601,7 @@ static struct platform_driver fsl_of_msi_driver = {
.of_match_table = fsl_of_msi_ids,
},
.probe = fsl_of_msi_probe,
- .remove = fsl_of_msi_remove,
+ .remove_new = fsl_of_msi_remove,
};
static __init int fsl_of_msi_init(void)
--
2.43.0
[PATCH 02/11] cxl: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/misc/cxl/of.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/misc/cxl/of.c b/drivers/misc/cxl/of.c
index 25ce725035e7..bcc005dff1c0 100644
--- a/drivers/misc/cxl/of.c
+++ b/drivers/misc/cxl/of.c
@@ -431,7 +431,7 @@ int cxl_of_read_adapter_properties(struct cxl *adapter,
struct device_node *np)
return 0;
}
-static int cxl_of_remove(struct platform_device *pdev)
+static void cxl_of_remove(struct platform_device *pdev)
{
struct cxl *adapter;
int afu;
@@ -441,7 +441,6 @@ static int cxl_of_remove(struct platform_device *pdev)
cxl_guest_remove_afu(adapter->afu[afu]);
cxl_guest_remove_adapter(adapter);
- return 0;
}
static void cxl_of_shutdown(struct platform_device *pdev)
@@ -501,6 +500,6 @@ struct platform_driver cxl_of_driver = {
.owner = THIS_MODULE
},
.probe = cxl_of_probe,
- .remove = cxl_of_remove,
+ .remove_new = cxl_of_remove,
.shutdown = cxl_of_shutdown,
};
--
2.43.0
[PATCH 00/11] misc: Convert to platform remove callback returning void
Hello,
this series converts all drivers below drivers/misc to struct
platform_driver::remove_new(). See commit 5c5a7680e67b ("platform:
Provide a remove callback that returns no value") for an extended
explanation and the eventual goal.
All conversations are trivial, because their .remove() callbacks
returned zero unconditionally.
There are no interdependencies between these patches, so they could be
picked up individually. But I'd hope that Greg or Arnd picks them up all
together.
Best regards
Uwe
Uwe Kleine-König (11):
misc: atmel-ssc: Convert to platform remove callback returning void
cxl: Convert to platform remove callback returning void
misc: fastrpc: Convert to platform remove callback returning void
misc: hisi_hikey_usb: Convert to platform remove callback returning
void
mei: vsc: Convert to platform remove callback returning void
misc: open-dice: Convert to platform remove callback returning void
misc: sram: Convert to platform remove callback returning void
misc: ti-st: st_kim: Convert to platform remove callback returning
void
misc: vcpu_stall_detector: Convert to platform remove callback
returning void
misc: xilinx_sdfec: Convert to platform remove callback returning void
misc: xilinx_tmr_inject: Convert to platform remove callback returning
void
drivers/misc/atmel-ssc.c | 6 ++
drivers/misc/cxl/of.c | 5 ++---
drivers/misc/fastrpc.c | 6 ++
drivers/misc/hisi_hikey_usb.c | 6 ++
drivers/misc/mei/platform-vsc.c| 6 ++
drivers/misc/open-dice.c | 5 ++---
drivers/misc/sram.c| 6 ++
drivers/misc/ti-st/st_kim.c| 5 ++---
drivers/misc/vcpu_stall_detector.c | 6 ++
drivers/misc/xilinx_sdfec.c| 5 ++---
drivers/misc/xilinx_tmr_inject.c | 5 ++---
11 files changed, 22 insertions(+), 39 deletions(-)
base-commit: 4893c639cc3659cefaa675bf1e59f4e7571afb5c
--
2.43.0
[PATCH 0/3] tty: Convert to platform remove callback returning void
Hello,
this series converts all remaining (three) drivers below drivers/tty to
struct platform_driver::remove_new(). See commit 5c5a7680e67b
("platform: Provide a remove callback that returns no value") for an
extended explanation and the eventual goal.
Two conversations are trivial, because their .remove() callbacks
returned zero unconditionally. The pmac_zilog serial driver had an error
path in its remove callback that however was never taken and so could be
dropped.
There are no interdependencies between these patches, so they could be
picked up individually. However I'd expect them to go in all together
via Greg's tree.
Uwe Kleine-König (3):
tty: amiserial: Convert to platform remove callback returning void
tty: goldfish: Convert to platform remove callback returning void
serial: pmac_zilog: Convert to platform remove callback returning void
drivers/tty/amiserial.c | 6 ++
drivers/tty/goldfish.c | 5 ++---
drivers/tty/serial/pmac_zilog.c | 9 ++---
3 files changed, 6 insertions(+), 14 deletions(-)
base-commit: d37e1e4c52bc60578969f391fb81f947c3e83118
--
2.43.0
[PATCH 3/3] serial: pmac_zilog: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Note that pmz_detach() is only called once pmz_attach() was successfully
called. In that case platform_set_drvdata() was called and so
platform_get_drvdata() won't return NULL. This allows to drop the
respective check and so get rid of the only error path in pmz_detach().
After that the driver can be trivially converted from always returning
zero in the remove callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/tty/serial/pmac_zilog.c | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/tty/serial/pmac_zilog.c b/drivers/tty/serial/pmac_zilog.c
index c8bf08c19c64..05b5b3b456ac 100644
--- a/drivers/tty/serial/pmac_zilog.c
+++ b/drivers/tty/serial/pmac_zilog.c
@@ -1717,18 +1717,13 @@ static int __init pmz_attach(struct platform_device
*pdev)
return uart_add_one_port(_uart_reg, >port);
}
-static int __exit pmz_detach(struct platform_device *pdev)
+static void __exit pmz_detach(struct platform_device *pdev)
{
struct uart_pmac_port *uap = platform_get_drvdata(pdev);
- if (!uap)
- return -ENODEV;
-
uart_remove_one_port(_uart_reg, >port);
uap->port.dev = NULL;
-
- return 0;
}
#endif /* !CONFIG_PPC_PMAC */
@@ -1797,7 +1792,7 @@ static struct macio_driver pmz_driver = {
#else
static struct platform_driver pmz_driver = {
- .remove = __exit_p(pmz_detach),
+ .remove_new = __exit_p(pmz_detach),
.driver = {
.name = "scc",
},
--
2.43.0
Re: [PATCH 0/7] macintosh: Convert to platform remove callback returning void
Hello,
On Wed, Jan 10, 2024 at 04:42:47PM +0100, Uwe Kleine-König wrote:
> Hello,
>
> this series converts all drivers below drivers/macintosh to use
> .remove_new(). See commit 5c5a7680e67b ("platform: Provide a remove
> callback that returns no value") for an extended explanation and the
> eventual goal. The TL;DR; is to make it harder for driver authors to
> leak resources without noticing.
>
> This is merge window material. All patches are pairwise independent of
> each other so they can be applied individually. There isn't a maintainer
> for drivers/macintosh, I'm still sending this as a series in the hope
> Michael feels repsonsible and applies it completely.
this didn't happen yet. Michael, is this still on your radar? Or is
there someone more suiteable to take these patches?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |
signature.asc
Description: PGP signature
[PATCH 3/7] macintosh: windfarm_pm121: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/macintosh/windfarm_pm121.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/macintosh/windfarm_pm121.c
b/drivers/macintosh/windfarm_pm121.c
index 82500417ebee..cd45fbc4fe1c 100644
--- a/drivers/macintosh/windfarm_pm121.c
+++ b/drivers/macintosh/windfarm_pm121.c
@@ -992,15 +992,14 @@ static int pm121_probe(struct platform_device *ddev)
return 0;
}
-static int pm121_remove(struct platform_device *ddev)
+static void pm121_remove(struct platform_device *ddev)
{
wf_unregister_client(_events);
- return 0;
}
static struct platform_driver pm121_driver = {
.probe = pm121_probe,
- .remove = pm121_remove,
+ .remove_new = pm121_remove,
.driver = {
.name = "windfarm",
.bus = _bus_type,
--
2.43.0
[PATCH 1/7] macintosh: therm_windtunnel: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/macintosh/therm_windtunnel.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/macintosh/therm_windtunnel.c
b/drivers/macintosh/therm_windtunnel.c
index 3c1b29476ce2..37cdc6931f6d 100644
--- a/drivers/macintosh/therm_windtunnel.c
+++ b/drivers/macintosh/therm_windtunnel.c
@@ -481,11 +481,9 @@ static int therm_of_probe(struct platform_device *dev)
return -ENODEV;
}
-static int
-therm_of_remove( struct platform_device *dev )
+static void therm_of_remove(struct platform_device *dev)
{
i2c_del_driver( _driver );
- return 0;
}
static const struct of_device_id therm_of_match[] = {{
@@ -501,7 +499,7 @@ static struct platform_driver therm_of_driver = {
.of_match_table = therm_of_match,
},
.probe = therm_of_probe,
- .remove = therm_of_remove,
+ .remove_new = therm_of_remove,
};
struct apple_thermal_info {
--
2.43.0
[PATCH 6/7] macintosh: windfarm_pm91: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/macintosh/windfarm_pm91.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/macintosh/windfarm_pm91.c
b/drivers/macintosh/windfarm_pm91.c
index 120a9cfba0c5..fba02a375435 100644
--- a/drivers/macintosh/windfarm_pm91.c
+++ b/drivers/macintosh/windfarm_pm91.c
@@ -647,7 +647,7 @@ static int wf_smu_probe(struct platform_device *ddev)
return 0;
}
-static int wf_smu_remove(struct platform_device *ddev)
+static void wf_smu_remove(struct platform_device *ddev)
{
wf_unregister_client(_smu_events);
@@ -691,13 +691,11 @@ static int wf_smu_remove(struct platform_device *ddev)
kfree(wf_smu_slots_fans);
kfree(wf_smu_drive_fans);
kfree(wf_smu_cpu_fans);
-
- return 0;
}
static struct platform_driver wf_smu_driver = {
-.probe = wf_smu_probe,
-.remove = wf_smu_remove,
+ .probe = wf_smu_probe,
+ .remove_new = wf_smu_remove,
.driver = {
.name = "windfarm",
},
--
2.43.0
[PATCH 2/7] macintosh: windfarm_pm112: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/macintosh/windfarm_pm112.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/macintosh/windfarm_pm112.c
b/drivers/macintosh/windfarm_pm112.c
index d1dec314ae30..876b4d8cbe37 100644
--- a/drivers/macintosh/windfarm_pm112.c
+++ b/drivers/macintosh/windfarm_pm112.c
@@ -662,16 +662,14 @@ static int wf_pm112_probe(struct platform_device *dev)
return 0;
}
-static int wf_pm112_remove(struct platform_device *dev)
+static void wf_pm112_remove(struct platform_device *dev)
{
wf_unregister_client(_events);
- /* should release all sensors and controls */
- return 0;
}
static struct platform_driver wf_pm112_driver = {
.probe = wf_pm112_probe,
- .remove = wf_pm112_remove,
+ .remove_new = wf_pm112_remove,
.driver = {
.name = "windfarm",
},
--
2.43.0
[PATCH 0/7] macintosh: Convert to platform remove callback returning void
Hello,
this series converts all drivers below drivers/macintosh to use
.remove_new(). See commit 5c5a7680e67b ("platform: Provide a remove
callback that returns no value") for an extended explanation and the
eventual goal. The TL;DR; is to make it harder for driver authors to
leak resources without noticing.
This is merge window material. All patches are pairwise independent of
each other so they can be applied individually. There isn't a maintainer
for drivers/macintosh, I'm still sending this as a series in the hope
Michael feels repsonsible and applies it completely.
Best regards
Uwe
Uwe Kleine-König (7):
macintosh: therm_windtunnel: Convert to platform remove callback returning
void
macintosh: windfarm_pm112: Convert to platform remove callback returning void
macintosh: windfarm_pm121: Convert to platform remove callback returning void
macintosh: windfarm_pm72: Convert to platform remove callback returning void
macintosh: windfarm_pm81: Convert to platform remove callback returning void
macintosh: windfarm_pm91: Convert to platform remove callback returning void
macintosh: windfarm_rm31: Convert to platform remove callback returning void
drivers/macintosh/therm_windtunnel.c | 6 ++
drivers/macintosh/windfarm_pm112.c | 6 ++
drivers/macintosh/windfarm_pm121.c | 5 ++---
drivers/macintosh/windfarm_pm72.c| 7 ++-
drivers/macintosh/windfarm_pm81.c| 8 +++-
drivers/macintosh/windfarm_pm91.c| 8 +++-
drivers/macintosh/windfarm_rm31.c| 7 ++-
7 files changed, 16 insertions(+), 31 deletions(-)
base-commit: 8cb47d7cd090a690c1785385b2f3d407d4a53ad0
--
2.43.0
[PATCH 4/7] macintosh: windfarm_pm72: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/macintosh/windfarm_pm72.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/macintosh/windfarm_pm72.c
b/drivers/macintosh/windfarm_pm72.c
index e21f973551cc..14fa1e9ac3e0 100644
--- a/drivers/macintosh/windfarm_pm72.c
+++ b/drivers/macintosh/windfarm_pm72.c
@@ -775,17 +775,14 @@ static int wf_pm72_probe(struct platform_device *dev)
return 0;
}
-static int wf_pm72_remove(struct platform_device *dev)
+static void wf_pm72_remove(struct platform_device *dev)
{
wf_unregister_client(_events);
-
- /* should release all sensors and controls */
- return 0;
}
static struct platform_driver wf_pm72_driver = {
.probe = wf_pm72_probe,
- .remove = wf_pm72_remove,
+ .remove_new = wf_pm72_remove,
.driver = {
.name = "windfarm",
},
--
2.43.0
[PATCH 7/7] macintosh: windfarm_rm31: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/macintosh/windfarm_rm31.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/macintosh/windfarm_rm31.c
b/drivers/macintosh/windfarm_rm31.c
index e9eb7fdde48c..dc8f2c7ef103 100644
--- a/drivers/macintosh/windfarm_rm31.c
+++ b/drivers/macintosh/windfarm_rm31.c
@@ -668,17 +668,14 @@ static int wf_rm31_probe(struct platform_device *dev)
return 0;
}
-static int wf_rm31_remove(struct platform_device *dev)
+static void wf_rm31_remove(struct platform_device *dev)
{
wf_unregister_client(_events);
-
- /* should release all sensors and controls */
- return 0;
}
static struct platform_driver wf_rm31_driver = {
.probe = wf_rm31_probe,
- .remove = wf_rm31_remove,
+ .remove_new = wf_rm31_remove,
.driver = {
.name = "windfarm",
},
--
2.43.0
[PATCH 5/7] macintosh: windfarm_pm81: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/macintosh/windfarm_pm81.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/macintosh/windfarm_pm81.c
b/drivers/macintosh/windfarm_pm81.c
index 257fb2c695c5..404d2454e33d 100644
--- a/drivers/macintosh/windfarm_pm81.c
+++ b/drivers/macintosh/windfarm_pm81.c
@@ -724,7 +724,7 @@ static int wf_smu_probe(struct platform_device *ddev)
return 0;
}
-static int wf_smu_remove(struct platform_device *ddev)
+static void wf_smu_remove(struct platform_device *ddev)
{
wf_unregister_client(_smu_events);
@@ -761,13 +761,11 @@ static int wf_smu_remove(struct platform_device *ddev)
/* Destroy control loops state structures */
kfree(wf_smu_sys_fans);
kfree(wf_smu_cpu_fans);
-
- return 0;
}
static struct platform_driver wf_smu_driver = {
-.probe = wf_smu_probe,
-.remove = wf_smu_remove,
+ .probe = wf_smu_probe,
+ .remove_new = wf_smu_remove,
.driver = {
.name = "windfarm",
},
--
2.43.0
Re: [PATCH net-next v2 0/9] net*: Convert to platform remove callback returning void
Hello Miquel, On Tue, Dec 05, 2023 at 07:51:10AM +0100, Miquel Raynal wrote: > u.kleine-koe...@pengutronix.de wrote on Mon, 4 Dec 2023 19:30:40 +0100: > > (implicit) v1 of this series can be found at > > https://lore.kernel.org/netdev/20231117095922.876489-1-u.kleine-koe...@pengutronix.de. > > Changes since then: > > > > - Dropped patch #1 as Alex objected. Patch #1 (was #2 before) now > >converts ipa to remove_new() and introduces an error message in the > >error path that failed before. > > > > - Rebased to today's next > > > > - Add the tags received in the previous round. > > > > Uwe Kleine-König (9): > > net: ipa: Convert to platform remove callback returning void > > net: fjes: Convert to platform remove callback returning void > > net: pcs: rzn1-miic: Convert to platform remove callback returning > > void > > net: sfp: Convert to platform remove callback returning void > > net: wan/fsl_ucc_hdlc: Convert to platform remove callback returning > > void > > net: wan/ixp4xx_hss: Convert to platform remove callback returning > > void > > net: wwan: qcom_bam_dmux: Convert to platform remove callback > > returning void > > ieee802154: fakelb: Convert to platform remove callback returning void > > ieee802154: hwsim: Convert to platform remove callback returning void > > FYI, I plan on taking patches 8 and 9 through wpan-next. I forgot to mention explicitly that there are no interdependencies in this series. So each maintainer picking up up their patches is fine. Thanks Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
[PATCH net-next v2 5/9] net: wan/fsl_ucc_hdlc: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Link:
https://lore.kernel.org/r/20231117095922.876489-7-u.kleine-koe...@pengutronix.de
Signed-off-by: Uwe Kleine-König
---
drivers/net/wan/fsl_ucc_hdlc.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index fd50bb313b92..605e70f7baac 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -1259,7 +1259,7 @@ static int ucc_hdlc_probe(struct platform_device *pdev)
return ret;
}
-static int ucc_hdlc_remove(struct platform_device *pdev)
+static void ucc_hdlc_remove(struct platform_device *pdev)
{
struct ucc_hdlc_private *priv = dev_get_drvdata(>dev);
@@ -1277,8 +1277,6 @@ static int ucc_hdlc_remove(struct platform_device *pdev)
kfree(priv);
dev_info(>dev, "UCC based hdlc module removed\n");
-
- return 0;
}
static const struct of_device_id fsl_ucc_hdlc_of_match[] = {
@@ -1292,7 +1290,7 @@ MODULE_DEVICE_TABLE(of, fsl_ucc_hdlc_of_match);
static struct platform_driver ucc_hdlc_driver = {
.probe = ucc_hdlc_probe,
- .remove = ucc_hdlc_remove,
+ .remove_new = ucc_hdlc_remove,
.driver = {
.name = DRV_NAME,
.pm = HDLC_PM_OPS,
--
2.42.0
[PATCH net-next v2 0/9] net*: Convert to platform remove callback returning void
Hello, (implicit) v1 of this series can be found at https://lore.kernel.org/netdev/20231117095922.876489-1-u.kleine-koe...@pengutronix.de. Changes since then: - Dropped patch #1 as Alex objected. Patch #1 (was #2 before) now converts ipa to remove_new() and introduces an error message in the error path that failed before. - Rebased to today's next - Add the tags received in the previous round. Uwe Kleine-König (9): net: ipa: Convert to platform remove callback returning void net: fjes: Convert to platform remove callback returning void net: pcs: rzn1-miic: Convert to platform remove callback returning void net: sfp: Convert to platform remove callback returning void net: wan/fsl_ucc_hdlc: Convert to platform remove callback returning void net: wan/ixp4xx_hss: Convert to platform remove callback returning void net: wwan: qcom_bam_dmux: Convert to platform remove callback returning void ieee802154: fakelb: Convert to platform remove callback returning void ieee802154: hwsim: Convert to platform remove callback returning void drivers/net/fjes/fjes_main.c | 6 ++--- drivers/net/ieee802154/fakelb.c | 5 ++-- drivers/net/ieee802154/mac802154_hwsim.c | 6 ++--- drivers/net/ipa/ipa_main.c | 29 +++- drivers/net/pcs/pcs-rzn1-miic.c | 6 ++--- drivers/net/phy/sfp.c| 6 ++--- drivers/net/wan/fsl_ucc_hdlc.c | 6 ++--- drivers/net/wan/ixp4xx_hss.c | 5 ++-- drivers/net/wwan/qcom_bam_dmux.c | 6 ++--- 9 files changed, 29 insertions(+), 46 deletions(-) base-commit: 629a3b49f3f957e975253c54846090b8d5ed2e9b -- 2.42.0
Re: linux-next: manual merge of the tty tree with the powerpc tree
Hello,
On Mon, Nov 27, 2023 at 10:00:58PM +1100, Michael Ellerman wrote:
> Stephen Rothwell writes:
> > Hi all,
> >
> > Today's linux-next merge of the tty tree got a conflict in:
> >
> > drivers/tty/hvc/hvc_console.h
> >
> > between commit:
> >
> > c9e38dc90e1c ("tty: hvc: Make hvc_remove() return no value")
> >
> > from the powerpc tree and commit:
> >
> > 7f30c19caf94 ("tty: hvc: Make hvc_remove() return no value")
> >
> > from the tty tree.
> >
> > These are slightly different versions of the same patch.
>
> I'll drop it from my tree.
FTR: Regarding the slightly difference: The variant in the tty tree is
the better one. So nothing to do for Greg (or me) here.
Thanks
Uwe
--
Pengutronix e.K. | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |
signature.asc
Description: PGP signature
[PATCH 0/5] usb: gadget: udc: Convert to platform remove callback returning void
Hello,
this patch set converts the platform drivers below drivers/usb/gadget to
use .remove_new. These drivers all have an error path if the driver is
still in use. Returning there early leaks resources, but fixing this
isn't trivial, so I just added an error message. The patches don't make
a difference to the drivers apart from the improved error message.
See commit 5c5a7680e67b ("platform: Provide a remove callback that
returns no value") for an extended explanation and the eventual goal of
.remove_new().
Best regards
Uwe
Uwe Kleine-König (5):
usb: gadget: at91_udc: Convert to platform remove callback returning void
usb: gadget: fsl_udc: Convert to platform remove callback returning void
usb: gadget: gr_udc: Convert to platform remove callback returning void
usb: gadget: lpc32xx_udc: Convert to platform remove callback returning void
usb: gadget: pxa25x_udc: Convert to platform remove callback returning void
drivers/usb/gadget/udc/at91_udc.c | 13 +++--
drivers/usb/gadget/udc/fsl_udc_core.c | 13 +++--
drivers/usb/gadget/udc/gr_udc.c | 13 +++--
drivers/usb/gadget/udc/lpc32xx_udc.c | 13 +++--
drivers/usb/gadget/udc/pxa25x_udc.c | 12 +++-
5 files changed, 35 insertions(+), 29 deletions(-)
base-commit: 5a82d69d48c82e89aef44483d2a129f869f3506a
--
2.42.0
[PATCH 2/5] usb: gadget: fsl_udc: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
In the error path emit an error message replacing the (less useful)
message by the core. Apart from the improved error message there is no
change in behaviour.
Signed-off-by: Uwe Kleine-König
---
drivers/usb/gadget/udc/fsl_udc_core.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c
b/drivers/usb/gadget/udc/fsl_udc_core.c
index 2693a10eb0c7..535b6e79a198 100644
--- a/drivers/usb/gadget/udc/fsl_udc_core.c
+++ b/drivers/usb/gadget/udc/fsl_udc_core.c
@@ -2532,15 +2532,18 @@ static int fsl_udc_probe(struct platform_device *pdev)
/* Driver removal function
* Free resources and finish pending transactions
*/
-static int fsl_udc_remove(struct platform_device *pdev)
+static void fsl_udc_remove(struct platform_device *pdev)
{
struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
struct fsl_usb2_platform_data *pdata = dev_get_platdata(>dev);
DECLARE_COMPLETION_ONSTACK(done);
- if (!udc_controller)
- return -ENODEV;
+ if (!udc_controller) {
+ dev_err(>dev,
+ "Driver still in use but removing anyhow\n");
+ return;
+ }
udc_controller->done =
usb_del_gadget_udc(_controller->gadget);
@@ -2568,8 +2571,6 @@ static int fsl_udc_remove(struct platform_device *pdev)
*/
if (pdata->exit)
pdata->exit(pdev);
-
- return 0;
}
/*-
@@ -2667,7 +2668,7 @@ static const struct platform_device_id fsl_udc_devtype[]
= {
MODULE_DEVICE_TABLE(platform, fsl_udc_devtype);
static struct platform_driver udc_driver = {
.probe = fsl_udc_probe,
- .remove = fsl_udc_remove,
+ .remove_new = fsl_udc_remove,
.id_table = fsl_udc_devtype,
/* these suspend and resume are not usb suspend and resume */
.suspend= fsl_udc_suspend,
--
2.42.0
[PATCH net-next 00/10] net*: Convert to platform remove callback returning void
Hello,
this series converts the platform drivers below drivers/net that are not
covered in the two other series converting drivers/net/ethernet and
drivers/net/wireless. I put them all in a single series even though they
are not maintained together. I thought that to be better than sending
them out individually, I hope you agree.
See commit 5c5a7680e67b ("platform: Provide a remove callback that
returns no value") for an extended explanation and the eventual goal.
The TL;DR; is to make it harder for driver authors to leak resources
without noticing.
The first patch is a fix, but I don't think it's worth to add that to
stable, it was broken since v5.7-rc1 and nobody seems to have hit the
problem.
Best regards
Uwe
Uwe Kleine-König (10):
net: ipa: Don't error out in .remove()
net: ipa: Convert to platform remove callback returning void
net: fjes: Convert to platform remove callback returning void
net: pcs: rzn1-miic: Convert to platform remove callback returning
void
net: sfp: Convert to platform remove callback returning void
net: wan/fsl_ucc_hdlc: Convert to platform remove callback returning
void
net: wan/ixp4xx_hss: Convert to platform remove callback returning
void
net: wwan: qcom_bam_dmux: Convert to platform remove callback
returning void
ieee802154: fakelb: Convert to platform remove callback returning void
ieee802154: hwsim: Convert to platform remove callback returning void
drivers/net/fjes/fjes_main.c | 6 ++
drivers/net/ieee802154/fakelb.c | 5 ++---
drivers/net/ieee802154/mac802154_hwsim.c | 6 ++
drivers/net/ipa/ipa_main.c | 20 +---
drivers/net/pcs/pcs-rzn1-miic.c | 6 ++
drivers/net/phy/sfp.c| 6 ++
drivers/net/wan/fsl_ucc_hdlc.c | 6 ++
drivers/net/wan/ixp4xx_hss.c | 5 ++---
drivers/net/wwan/qcom_bam_dmux.c | 6 ++
9 files changed, 21 insertions(+), 45 deletions(-)
base-commit: eff99d8edbed7918317331ebd1e365d8e955d65e
--
2.42.0
[PATCH net-next 06/10] net: wan/fsl_ucc_hdlc: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/net/wan/fsl_ucc_hdlc.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index fd50bb313b92..605e70f7baac 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -1259,7 +1259,7 @@ static int ucc_hdlc_probe(struct platform_device *pdev)
return ret;
}
-static int ucc_hdlc_remove(struct platform_device *pdev)
+static void ucc_hdlc_remove(struct platform_device *pdev)
{
struct ucc_hdlc_private *priv = dev_get_drvdata(>dev);
@@ -1277,8 +1277,6 @@ static int ucc_hdlc_remove(struct platform_device *pdev)
kfree(priv);
dev_info(>dev, "UCC based hdlc module removed\n");
-
- return 0;
}
static const struct of_device_id fsl_ucc_hdlc_of_match[] = {
@@ -1292,7 +1290,7 @@ MODULE_DEVICE_TABLE(of, fsl_ucc_hdlc_of_match);
static struct platform_driver ucc_hdlc_driver = {
.probe = ucc_hdlc_probe,
- .remove = ucc_hdlc_remove,
+ .remove_new = ucc_hdlc_remove,
.driver = {
.name = DRV_NAME,
.pm = HDLC_PM_OPS,
--
2.42.0
Re: [PATCH 1/2] tty: hvc: Make hvc_remove() return no value
On Mon, Nov 13, 2023 at 07:45:27PM +1000, Nicholas Piggin wrote:
> On Mon Nov 6, 2023 at 7:44 AM AEST, Uwe Kleine-König wrote:
> > The function hvc_remove() returns zero unconditionally. Make it return
> > void instead to make it obvious that the caller doesn't need to do any
> > error handling. Accordingly drop the error handling from
> > hvc_opal_remove().
> >
> > Signed-off-by: Uwe Kleine-König
>
> IIUC these are functionally no change, just tidying and removing
> dead code?
In case this isn't only a rethorical question: There is indeed no
change in behaviour. hvc_remove() returned always zero, so
rc = hvc_remove(hp);
if (rc == 0) {
... some code not changing rc ...
}
... some more code not changing rc ...
return rc
can be simplified to
hvc_remove(hp);
... some code not changing rc ...
... some more code not changing rc ...
return 0;
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |
signature.asc
Description: PGP signature
[PATCH 00/52] serial: Convert to platform remove callback returning void
Hello,
this series starts with two fixes. The first one fixes a resource leak
and use after free. The second only improves error reporting. I added a
Fixes: marker to these. I let you decide if you want to drop them (or
the second only) or add a Cc: stable (to both or only the first one).
After that all drivers below drivers/tty/serial are converted to struct
platform_driver::remove_new. See commit 5c5a7680e67b ("platform: Provide
a remove callback that returns no value") for an extended explanation
and the eventual goal. The TL;DR; is to prevent bugs like the two fixed
here.
After these two fixes all conversations are trivial, because all
.remove() callbacks returned zero unconditionally.
The conversion patches are merge window material. The two fixes might go
in also before v6.7, but given the fixed problems are already old
(v6.1-rc6 + v3.10-rc1) there is probably no urge.
Best regards
Uwe
Uwe Kleine-König (52):
serial: 8250: omap: Don't skip resource freeing if
pm_runtime_resume_and_get() failed
serial: sccnxp: Improve error message if regulator_disable() fails
serial: 8250: Convert to platform remove callback returning void
serial: altera_jtaguart: Convert to platform remove callback returning
void
serial: altera: Convert to platform remove callback returning void
serial: amba-pl011: Convert to platform remove callback returning void
serial: ar933x: Convert to platform remove callback returning void
serial: atmel: Convert to platform remove callback returning void
serial: bcm63xx: Convert to platform remove callback returning void
serial: clps711x: Convert to platform remove callback returning void
serial: cpm: Convert to platform remove callback returning void
serial: digicolor: Convert to platform remove callback returning void
serial: esp32_acm: Convert to platform remove callback returning void
serial: esp32: Convert to platform remove callback returning void
serial: fsl_linflexuart: Convert to platform remove callback returning
void
serial: fsl_lpuart: Convert to platform remove callback returning void
serial: imx: Convert to platform remove callback returning void
serial: lantiq: Convert to platform remove callback returning void
serial: liteuart: Convert to platform remove callback returning void
serial: lpc32xx_hs: Convert to platform remove callback returning void
serial: ma35d1: Convert to platform remove callback returning void
serial: mcf: Convert to platform remove callback returning void
serial: meson: Convert to platform remove callback returning void
serial: milbeaut_usio: Convert to platform remove callback returning
void
serial: mpc52xx: Convert to platform remove callback returning void
serial: msm: Convert to platform remove callback returning void
serial: mxs-auart: Convert to platform remove callback returning void
serial: omap: Convert to platform remove callback returning void
serial: owl: Convert to platform remove callback returning void
serial: pic32: Convert to platform remove callback returning void
serial: qcom_geni: Convert to platform remove callback returning void
serial: rda: Convert to platform remove callback returning void
serial: sa1100: Convert to platform remove callback returning void
serial: samsung: Convert to platform remove callback returning void
serial: sccnxp: Convert to platform remove callback returning void
serial: tegra: Convert to platform remove callback returning void
serial: txx9: Convert to platform remove callback returning void
serial: sh-sci: Convert to platform remove callback returning void
serial: sifive: Convert to platform remove callback returning void
serial: sprd: Convert to platform remove callback returning void
serial: st-asc: Convert to platform remove callback returning void
serial: stm32: Convert to platform remove callback returning void
serial: sunhv: Convert to platform remove callback returning void
serial: sunplus: Convert to platform remove callback returning void
serial: sunsab: Convert to platform remove callback returning void
serial: sunsu: Convert to platform remove callback returning void
serial: sunzilog: Convert to platform remove callback returning void
serial: tegra-tcu: Convert to platform remove callback returning void
serial: timbuart: Convert to platform remove callback returning void
serial: uartlite: Convert to platform remove callback returning void
serial: ucc: Convert to platform remove callback returning void
serial: xilinx_uartps: Convert to platform remove callback returning
void
drivers/tty/serial/8250/8250_aspeed_vuart.c | 6 ++
drivers/tty/serial/8250/8250_bcm2835aux.c | 6 ++
drivers/tty/serial/8250/8250_bcm7271.c | 5 ++---
drivers/tty/serial/8250/8250_core.c | 5 ++---
drivers/tty/serial/8250/8250_dw.c | 6 ++
drivers/tty/serial/8250/8250_em.c | 5 ++---
drivers/tty/serial/8250/8250_fsl.c
Re: [PATCH 04/22] [RESEND] time: make sysfs_get_uname() function visible in header
Hello Arnd, On Wed, Nov 08, 2023 at 01:58:25PM +0100, Arnd Bergmann wrote: > From: Arnd Bergmann > > This function is defined globally in clocksource.c and used conditionally > in clockevent.c, which the declaration hidden when clockevent support s/which/with/ ? > is disabled. This causes a harmless warning in the definition: > > kernel/time/clocksource.c:1324:9: warning: no previous prototype for > 'sysfs_get_uname' [-Wmissing-prototypes] > 1324 | ssize_t sysfs_get_uname(const char *buf, char *dst, size_t cnt) > > Move the declaration out of the #ifdef so it is always visible. > > Signed-off-by: Arnd Bergmann Other than that: Reviewed-by: Uwe Kleine-König -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
[PATCH 51/52] serial: ucc: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/tty/serial/ucc_uart.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/ucc_uart.c b/drivers/tty/serial/ucc_uart.c
index ed7a6bb5596a..f9ad942c9aaf 100644
--- a/drivers/tty/serial/ucc_uart.c
+++ b/drivers/tty/serial/ucc_uart.c
@@ -1459,7 +1459,7 @@ static int ucc_uart_probe(struct platform_device *ofdev)
return ret;
}
-static int ucc_uart_remove(struct platform_device *ofdev)
+static void ucc_uart_remove(struct platform_device *ofdev)
{
struct uart_qe_port *qe_port = platform_get_drvdata(ofdev);
@@ -1470,8 +1470,6 @@ static int ucc_uart_remove(struct platform_device *ofdev)
of_node_put(qe_port->np);
kfree(qe_port);
-
- return 0;
}
static const struct of_device_id ucc_uart_match[] = {
@@ -1492,7 +1490,7 @@ static struct platform_driver ucc_uart_of_driver = {
.of_match_table= ucc_uart_match,
},
.probe = ucc_uart_probe,
- .remove = ucc_uart_remove,
+ .remove_new = ucc_uart_remove,
};
static int __init ucc_uart_init(void)
--
2.42.0
[PATCH 1/2] tty: hvc: Make hvc_remove() return no value
The function hvc_remove() returns zero unconditionally. Make it return
void instead to make it obvious that the caller doesn't need to do any
error handling. Accordingly drop the error handling from
hvc_opal_remove().
Signed-off-by: Uwe Kleine-König
---
drivers/tty/hvc/hvc_console.c | 3 +--
drivers/tty/hvc/hvc_console.h | 2 +-
drivers/tty/hvc/hvc_opal.c| 15 +++
3 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
index 959fae54ca39..57f5c37125e6 100644
--- a/drivers/tty/hvc/hvc_console.c
+++ b/drivers/tty/hvc/hvc_console.c
@@ -976,7 +976,7 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data,
}
EXPORT_SYMBOL_GPL(hvc_alloc);
-int hvc_remove(struct hvc_struct *hp)
+void hvc_remove(struct hvc_struct *hp)
{
unsigned long flags;
struct tty_struct *tty;
@@ -1010,7 +1010,6 @@ int hvc_remove(struct hvc_struct *hp)
tty_vhangup(tty);
tty_kref_put(tty);
}
- return 0;
}
EXPORT_SYMBOL_GPL(hvc_remove);
diff --git a/drivers/tty/hvc/hvc_console.h b/drivers/tty/hvc/hvc_console.h
index 9668f821db01..78f7543511f1 100644
--- a/drivers/tty/hvc/hvc_console.h
+++ b/drivers/tty/hvc/hvc_console.h
@@ -77,7 +77,7 @@ extern int hvc_instantiate(uint32_t vtermno, int index,
extern struct hvc_struct * hvc_alloc(uint32_t vtermno, int data,
const struct hv_ops *ops, int outbuf_size);
/* remove a vterm from hvc tty operation (module_exit or hotplug remove) */
-extern int hvc_remove(struct hvc_struct *hp);
+extern void hvc_remove(struct hvc_struct *hp);
/* data available */
int hvc_poll(struct hvc_struct *hp);
diff --git a/drivers/tty/hvc/hvc_opal.c b/drivers/tty/hvc/hvc_opal.c
index 992e199e0ea8..8995b253cf90 100644
--- a/drivers/tty/hvc/hvc_opal.c
+++ b/drivers/tty/hvc/hvc_opal.c
@@ -235,16 +235,15 @@ static int hvc_opal_probe(struct platform_device *dev)
static int hvc_opal_remove(struct platform_device *dev)
{
struct hvc_struct *hp = dev_get_drvdata(>dev);
- int rc, termno;
+ int termno;
termno = hp->vtermno;
- rc = hvc_remove(hp);
- if (rc == 0) {
- if (hvc_opal_privs[termno] != _opal_boot_priv)
- kfree(hvc_opal_privs[termno]);
- hvc_opal_privs[termno] = NULL;
- }
- return rc;
+ hvc_remove(hp);
+ if (hvc_opal_privs[termno] != _opal_boot_priv)
+ kfree(hvc_opal_privs[termno]);
+ hvc_opal_privs[termno] = NULL;
+
+ return 0;
}
static struct platform_driver hvc_opal_driver = {
--
2.42.0
[PATCH 2/2] tty: hvc: hvc_opal: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/tty/hvc/hvc_opal.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/hvc/hvc_opal.c b/drivers/tty/hvc/hvc_opal.c
index 8995b253cf90..2cdf66e395cc 100644
--- a/drivers/tty/hvc/hvc_opal.c
+++ b/drivers/tty/hvc/hvc_opal.c
@@ -232,7 +232,7 @@ static int hvc_opal_probe(struct platform_device *dev)
return 0;
}
-static int hvc_opal_remove(struct platform_device *dev)
+static void hvc_opal_remove(struct platform_device *dev)
{
struct hvc_struct *hp = dev_get_drvdata(>dev);
int termno;
@@ -242,13 +242,11 @@ static int hvc_opal_remove(struct platform_device *dev)
if (hvc_opal_privs[termno] != _opal_boot_priv)
kfree(hvc_opal_privs[termno]);
hvc_opal_privs[termno] = NULL;
-
- return 0;
}
static struct platform_driver hvc_opal_driver = {
.probe = hvc_opal_probe,
- .remove = hvc_opal_remove,
+ .remove_new = hvc_opal_remove,
.driver = {
.name = hvc_opal_name,
.of_match_table = hvc_opal_match,
--
2.42.0
[PATCH 0/2] tty: hvc: Convert to platform remove callback returning void
Hello,
The hvc_opal driver had an error path in its remove function resulting
in returning a non-zero value. This is a bad thing because the core
doesn't do error handling and effectively ignores the return value.
In this case it's not as bad as it sounds though, as this error path is
never taken.
After the first patch changes hvc_remove() to return void the remove
function obviously always returns zero. Then it can be trivially
converted to .remove_new(). See commit 5c5a7680e67b ("platform: Provide
a remove callback that returns no value") for an extended explanation
and the eventual goal of this conversion.
Uwe Kleine-König (2):
tty: hvc: Make hvc_remove() return no value
tty: hvc: hvc_opal: Convert to platform remove callback returning void
drivers/tty/hvc/hvc_console.c | 3 +--
drivers/tty/hvc/hvc_console.h | 2 +-
drivers/tty/hvc/hvc_opal.c| 17 +++--
3 files changed, 9 insertions(+), 13 deletions(-)
base-commit: e27090b1413ff236ca1aec26d6b022149115de2c
--
2.42.0
[PATCH 2/6] usb: gadget: fsl-udc: Convert to use module_platform_driver()
module_platform_driver_probe() has the advantage that the .probe() and
.remove() calls can live in .init.text and .exit.text respectively and
so some memory is saved. The downside is that dynamic bind and unbind
are impossible. As the driver doesn't benefit from the advantages (both
.probe and .remove are defined in plain .text), stop suffering from the
downsides and use module_platform_driver() instead of
module_platform_driver_probe().
Signed-off-by: Uwe Kleine-König
---
drivers/usb/gadget/udc/fsl_udc_core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c
b/drivers/usb/gadget/udc/fsl_udc_core.c
index ee5705d336e3..2693a10eb0c7 100644
--- a/drivers/usb/gadget/udc/fsl_udc_core.c
+++ b/drivers/usb/gadget/udc/fsl_udc_core.c
@@ -2666,6 +2666,7 @@ static const struct platform_device_id fsl_udc_devtype[]
= {
};
MODULE_DEVICE_TABLE(platform, fsl_udc_devtype);
static struct platform_driver udc_driver = {
+ .probe = fsl_udc_probe,
.remove = fsl_udc_remove,
.id_table = fsl_udc_devtype,
/* these suspend and resume are not usb suspend and resume */
@@ -2679,7 +2680,7 @@ static struct platform_driver udc_driver = {
},
};
-module_platform_driver_probe(udc_driver, fsl_udc_probe);
+module_platform_driver(udc_driver);
MODULE_DESCRIPTION(DRIVER_DESC);
MODULE_AUTHOR(DRIVER_AUTHOR);
--
2.42.0
[PATCH 0/6] usb: gadget: Convert to use module_platform_driver()
Hello, module_platform_driver_probe() is an alternative to module_platform_driver(). Comparing the two the former has the advantage that the probe and remove callbacks can live in .init.text and .exit.text respectively. The latter has the advantage that it's a bit easier to use correctly and you can bind/unbind via sysfs and/or hotplug. There are considerations about deprecating module_platform_driver_probe()[1] as very few drivers use it, still less make actually use of the advantages and saving a few bytes isn't as important any more as it was (say) 10 years ago. Given that the drivers below drivers/usb/gadget making use of module_platform_driver_probe() doesn't benefit from the advantages at all (probe and remove are all defined in .text), convert these drivers to module_platform_driver(). Best regards Uwe [1] https://lore.kernel.org/linux-kbuild/20231017132045.afswdgcv4axjf...@pengutronix.de Uwe Kleine-König (6): usb: gadget: at91-udc: Convert to use module_platform_driver() usb: gadget: fsl-udc: Convert to use module_platform_driver() usb: gadget: fusb300-udc: Convert to use module_platform_driver() usb: gadget: lpc32xx-udc: Convert to use module_platform_driver() usb: gadget: m66592-udc: Convert to use module_platform_driver() usb: gadget: r8a66597-udc: Convert to use module_platform_driver() drivers/usb/gadget/udc/at91_udc.c | 3 ++- drivers/usb/gadget/udc/fsl_udc_core.c | 3 ++- drivers/usb/gadget/udc/fusb300_udc.c | 7 --- drivers/usb/gadget/udc/lpc32xx_udc.c | 3 ++- drivers/usb/gadget/udc/m66592-udc.c | 3 ++- drivers/usb/gadget/udc/r8a66597-udc.c | 3 ++- 6 files changed, 14 insertions(+), 8 deletions(-) base-commit: 4d5ab2376ec576af173e5eac3887ed0b51bd8566 -- 2.42.0
[GIT PULL] Convert drivers/soc to struct platform_driver::remove_new()
Hello Arnd,
the following changes since commit 0bb80ecc33a8fb5a682236443c1e740d5c917d1d:
Linux 6.6-rc1 (2023-09-10 16:28:41 -0700)
are available in the Git repository at:
https://git.pengutronix.de/git/ukl/linux
tags/platform-remove-void-soc-for-6.7-rc
for you to fetch changes up to e77e6e3e909d33361c58af848a96e1f7f71ba7e4:
soc/pxa: ssp: Convert to platform remove callback returning void (2023-10-14
23:27:34 +0200)
Convert drivers/soc to struct platform_driver::remove_new()
This PR contains the patches I sent in the series available at
https://lore.kernel.org/all/20230925095532.1984344-1-u.kleine-koe...@pengutronix.de
that were not yet picked up in next as of next-20231013.
It converts all drivers below drivers/soc to let their remove callback
return void. See commit 5c5a7680e67b ("platform: Provide a remove
callback that returns no value") for the rationale.
On Thu, Sep 28, 2023 at 08:14:49AM +0200, Uwe Kleine-König wrote:
> On Wed, Sep 27, 2023 at 04:01:58PM -0700, Bjorn Andersson wrote:
> > On Wed, Sep 27, 2023 at 10:43:16AM +0200, Arnd Bergmann wrote:
> > > On Wed, Sep 27, 2023, at 04:25, Joel Stanley wrote:
> > > > On Mon, 25 Sept 2023 at 09:55, Uwe Kleine-König
> > > > wrote:
> > > >>
> > > >> this series converts all platform drivers below drivers/soc to use
> > > >> .remove_new(). The motivation is to get rid of an integer return code
> > > >> that is (mostly) ignored by the platform driver core and error prone on
> > > >> the driver side.
> > > >>
> > > >> See commit 5c5a7680e67b ("platform: Provide a remove callback that
> > > >> returns no value") for an extended explanation and the eventual goal.
> > > >>
> > > >> As there is no single maintainer team for drivers/soc, I suggest the
> > > >> individual maintainers to pick up "their" patches.
> > > >
> > > > I'd be happy if Arnd merged the lot at once. Arnd, what do you think?
> > > >
> > > > If that will be too messy then I understand. I have queued the aspeed
> > > > ones locally and will push that out if we decide that's the best way
> > > > to go.
> > >
> > > The main downside of merging it all at once through the soc tree
> > > is that there may be patches that conflict with other work going on
> > > in individual drivers.
> > >
> > > What I'd suggest doing here is:
> > >
> > > - have platform maintainers pick up patches for their drivers
> > > if that is their preference for any reason
> >
> > I'd prefer this for the qcom drivers at least, please let me know if you
> > would like me to proceed.
>
> I can send a pull request as Arnd suggested. So iff you want the qcom
> drivers not be a part of that PR, just make sure they appear in next
> during the next week. :-)
>
> > > - get a pull request from Uwe for the soc tree for anything that has
> > > not been picked up in one or two weeks from now
Here comes the promised PR. The qcom patches are among the set of
patches dropped here as they are already in next.
To state the obvious: This is merge window material and the idea is that
it's pulled into armsoc and then included in the armsoc v6.7-rc1 PR to
Linus Torvalds. I hope it's not too late for that already.
Best regards and thanks
Uwe
Uwe Kleine-König (12):
soc/fsl: dpaa2-console: Convert to platform remove callback returning void
soc/fsl: cpm: qmc: Convert to platform remove callback returning void
soc/fsl: cpm: tsa: Convert to platform remove callback returning void
soc/fujitsu: a64fx-diag: Convert to platform remove callback returning
void
soc/hisilicon: kunpeng_hccs: Convert to platform remove callback
returning void
soc/ixp4xx: ixp4xx-npe: Convert to platform remove callback returning void
soc/ixp4xx: ixp4xx-qmgr: Convert to platform remove callback returning
void
soc/litex: litex_soc_ctrl: Convert to platform remove callback returning
void
soc/loongson: loongson2_guts: Convert to platform remove callback
returning void
soc/mediatek: mtk-devapc: Convert to platform remove callback returning
void
soc/mediatek: mtk-mmsys: Convert to platform remove callback returning
void
soc/pxa: ssp: Convert to platform remove callback returning void
drivers/soc/fsl/dpaa2-console.c | 6 ++
drivers/soc/fsl/qe/qmc.c | 6 ++
drivers/soc/fsl/qe/tsa.c | 5 ++---
drivers/soc/fujitsu/a64fx-diag.c | 6 ++
drivers/soc/hisilicon/kunpeng
[PATCH 00/20] mtd: Convert to platform remove callback returning void
Hello,
this series converts all platform drivers below drivers/mtd to use the
.remove_new() callback. Compared to the traditional .remove() callback
.remove_new() returns no value. This is a good thing because the driver
core doesn't (and cannot) cope for errors during remove. The only effect
of a non-zero return value in .remove() is that the driver core emits a
warning. The device is removed anyhow and an early return from .remove()
usually yields resource leaks and/or use-after-free bugs.
All drivers touched here returned zero unconditionally in their remove
callback, so they could all be converted trivially to .remove_new().
See commit 5c5a7680e67b ("platform: Provide a remove callback that
returns no value") for an extended explanation and the eventual goal.
As an added bonus the series starts with a minor fix and a
simplification of the txx9ndfmc driver.
The only interdependencies in this series is between the three txx9ndfmc
patches. As there are still quite a few drivers to convert, I'm happy
about every patch that makes it in. So even if there is a merge conflict
with one patch until you apply (or a different concern that doesn't
apply to all patches), please apply the remainder of this series anyhow.
I'll come back to the part that you (maybe) skipped at a later point.
Best regards
Uwe
Uwe Kleine-König (20):
mtd: rawnand: txx9ndfmc: Mark driver struct with __refdata to prevent
section mismatch warning
mtd: rawnand: txx9ndfmc: Drop if block with always false condition
mtd: bcm47xxsflash: Convert to platform remove callback returning void
mtd: docg3: Convert to platform remove callback returning void
mtd: phram: Convert to platform remove callback returning void
mtd: powernv_flash: Convert to platform remove callback returning void
mtd: spear_smi: Convert to platform remove callback returning void
mtd: st_spi_fsm: Convert to platform remove callback returning void
mtd: hyperbus: hbmc-am654: Convert to platform remove callback
returning void
mtd: hyperbus: rpc-if: Convert to platform remove callback returning
void
mtd: lpddr2_nvm: Convert to platform remove callback returning void
mtd: maps: lantiq-flash: Convert to platform remove callback returning
void
mtd: maps: physmap-core: Convert to platform remove callback returning
void
mtd: maps: plat-ram: Convert to platform remove callback returning
void
mtd: maps: pxa2xx-flash: Convert to platform remove callback returning
void
mtd: maps: sa1100-flash: Convert to platform remove callback returning
void
mtd: maps: sun_uflash: Convert to platform remove callback returning
void
mtd: rawnand: txx9ndfmc: Convert to platform remove callback returning
void
mtd: spi-nor: hisi-sfc: Convert to platform remove callback returning
void
mtd: spi-nor: nxp-spifi: Convert to platform remove callback returning
void
drivers/mtd/devices/bcm47xxsflash.c | 6 ++
drivers/mtd/devices/docg3.c | 5 ++---
drivers/mtd/devices/phram.c | 6 ++
drivers/mtd/devices/powernv_flash.c | 6 ++
drivers/mtd/devices/spear_smi.c | 6 ++
drivers/mtd/devices/st_spi_fsm.c| 6 ++
drivers/mtd/hyperbus/hbmc-am654.c | 6 ++
drivers/mtd/hyperbus/rpc-if.c | 6 ++
drivers/mtd/lpddr/lpddr2_nvm.c | 6 ++
drivers/mtd/maps/lantiq-flash.c | 6 ++
drivers/mtd/maps/physmap-core.c | 5 ++---
drivers/mtd/maps/plat-ram.c | 8 +++-
drivers/mtd/maps/pxa2xx-flash.c | 5 ++---
drivers/mtd/maps/sa1100-flash.c | 6 ++
drivers/mtd/maps/sun_uflash.c | 6 ++
drivers/mtd/nand/raw/txx9ndfmc.c| 15 +--
drivers/mtd/spi-nor/controllers/hisi-sfc.c | 5 ++---
drivers/mtd/spi-nor/controllers/nxp-spifi.c | 6 ++
18 files changed, 44 insertions(+), 71 deletions(-)
base-commit: 0bb80ecc33a8fb5a682236443c1e740d5c917d1d
--
2.40.1
[PATCH 06/20] mtd: powernv_flash: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/mtd/devices/powernv_flash.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/devices/powernv_flash.c
b/drivers/mtd/devices/powernv_flash.c
index 36e060386e59..66044f4f5bad 100644
--- a/drivers/mtd/devices/powernv_flash.c
+++ b/drivers/mtd/devices/powernv_flash.c
@@ -265,14 +265,12 @@ static int powernv_flash_probe(struct platform_device
*pdev)
*
* Returns 0
*/
-static int powernv_flash_release(struct platform_device *pdev)
+static void powernv_flash_release(struct platform_device *pdev)
{
struct powernv_flash *data = dev_get_drvdata(&(pdev->dev));
/* All resources should be freed automatically */
WARN_ON(mtd_device_unregister(>mtd));
-
- return 0;
}
static const struct of_device_id powernv_flash_match[] = {
@@ -285,7 +283,7 @@ static struct platform_driver powernv_flash_driver = {
.name = "powernv_flash",
.of_match_table = powernv_flash_match,
},
- .remove = powernv_flash_release,
+ .remove_new = powernv_flash_release,
.probe = powernv_flash_probe,
};
--
2.40.1
Re: [PATCH 00/40] soc: Convert to platform remove callback returning void
On Wed, Sep 27, 2023 at 04:01:58PM -0700, Bjorn Andersson wrote:
> On Wed, Sep 27, 2023 at 10:43:16AM +0200, Arnd Bergmann wrote:
> > On Wed, Sep 27, 2023, at 04:25, Joel Stanley wrote:
> > > On Mon, 25 Sept 2023 at 09:55, Uwe Kleine-König
> > > wrote:
> > >>
> > >> this series converts all platform drivers below drivers/soc to use
> > >> .remove_new(). The motivation is to get rid of an integer return code
> > >> that is (mostly) ignored by the platform driver core and error prone on
> > >> the driver side.
> > >>
> > >> See commit 5c5a7680e67b ("platform: Provide a remove callback that
> > >> returns no value") for an extended explanation and the eventual goal.
> > >>
> > >> As there is no single maintainer team for drivers/soc, I suggest the
> > >> individual maintainers to pick up "their" patches.
> > >
> > > I'd be happy if Arnd merged the lot at once. Arnd, what do you think?
> > >
> > > If that will be too messy then I understand. I have queued the aspeed
> > > ones locally and will push that out if we decide that's the best way
> > > to go.
> >
> > The main downside of merging it all at once through the soc tree
> > is that there may be patches that conflict with other work going on
> > in individual drivers.
> >
> > What I'd suggest doing here is:
> >
> > - have platform maintainers pick up patches for their drivers
> > if that is their preference for any reason
>
> I'd prefer this for the qcom drivers at least, please let me know if you
> would like me to proceed.
I can send a pull request as Arnd suggested. So iff you want the qcom
drivers not be a part of that PR, just make sure they appear in next
during the next week. :-)
(I'm not sure if "this" in your last quoted sentence is "platform
maintainers pick up" or "merging it all at once through the soc tree". I
think you mean the former. Still if you don't want me to pick up the
qcom patches and won't manage to make them appear in next via your tree,
I ask you to tell me explicitly to skip these in my PR.)
Best regards and thanks,
Uwe
--
Pengutronix e.K. | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |
signature.asc
Description: PGP signature
[PATCH 00/40] soc: Convert to platform remove callback returning void
Hello,
this series converts all platform drivers below drivers/soc to use
.remove_new(). The motivation is to get rid of an integer return code
that is (mostly) ignored by the platform driver core and error prone on
the driver side.
See commit 5c5a7680e67b ("platform: Provide a remove callback that
returns no value") for an extended explanation and the eventual goal.
As there is no single maintainer team for drivers/soc, I suggest the
individual maintainers to pick up "their" patches. There are no
interdependencies between the patches, so that should work fine. As
there are still quite a few drivers to convert in other areas than
drivers/soc, I'm happy about every patch that makes it in and there is
no need for further coordination. So even if there is a merge conflict
with one patch until you apply or a subject prefix is suboptimal, please
don't let you stop by negative feedback for other patches (unless it
applies to "your" patches, too, of course).
Best regards and thanks for considering,
Uwe
Uwe Kleine-König (40):
soc/aspeed: aspeed-lpc-ctrl: Convert to platform remove callback
returning void
soc/aspeed: aspeed-lpc-snoop: Convert to platform remove callback
returning void
soc/aspeed: aspeed-p2a-ctrl: Convert to platform remove callback
returning void
soc/aspeed: aspeed-uart-routing: Convert to platform remove callback
returning void
soc/fsl: dpaa2-console: Convert to platform remove callback returning
void
soc/fsl: cpm: qmc: Convert to platform remove callback returning void
soc/fsl: cpm: tsa: Convert to platform remove callback returning void
soc/fujitsu: a64fx-diag: Convert to platform remove callback returning
void
soc/hisilicon: kunpeng_hccs: Convert to platform remove callback
returning void
soc/ixp4xx: ixp4xx-npe: Convert to platform remove callback returning
void
soc/ixp4xx: ixp4xx-qmgr: Convert to platform remove callback returning
void
soc/litex: litex_soc_ctrl: Convert to platform remove callback
returning void
soc/loongson: loongson2_guts: Convert to platform remove callback
returning void
soc/mediatek: mtk-devapc: Convert to platform remove callback
returning void
soc/mediatek: mtk-mmsys: Convert to platform remove callback returning
void
soc/microchip: mpfs-sys-controller: Convert to platform remove
callback returning void
soc/pxa: ssp: Convert to platform remove callback returning void
soc/qcom: icc-bwmon: Convert to platform remove callback returning
void
soc/qcom: llcc-qcom: Convert to platform remove callback returning
void
soc/qcom: ocmem: Convert to platform remove callback returning void
soc/qcom: pmic_glink: Convert to platform remove callback returning
void
soc/qcom: qcom_aoss: Convert to platform remove callback returning
void
soc/qcom: qcom_gsbi: Convert to platform remove callback returning
void
soc/qcom: qcom_stats: Convert to platform remove callback returning
void
soc/qcom: rmtfs_mem: Convert to platform remove callback returning
void
soc/qcom: smem: Convert to platform remove callback returning void
soc/qcom: smp2p: Convert to platform remove callback returning void
soc/qcom: smsm: Convert to platform remove callback returning void
soc/qcom: socinfo: Convert to platform remove callback returning void
soc/rockchip: io-domain: Convert to platform remove callback returning
void
soc/samsung: exynos-chipid: Convert to platform remove callback
returning void
soc/tegra: cbb: tegra194-cbb: Convert to platform remove callback
returning void
soc/ti: k3-ringacc: Convert to platform remove callback returning void
soc/ti: knav_dma: Convert to platform remove callback returning void
soc/ti: knav_qmss_queue: Convert to platform remove callback returning
void
soc/ti: pm33xx: Convert to platform remove callback returning void
soc/ti: pruss: Convert to platform remove callback returning void
soc/ti: smartreflex: Convert to platform remove callback returning
void
soc/ti: wkup_m3_ipc: Convert to platform remove callback returning
void
soc/xilinx: zynqmp_power: Convert to platform remove callback
returning void
drivers/soc/aspeed/aspeed-lpc-ctrl.c| 6 ++
drivers/soc/aspeed/aspeed-lpc-snoop.c | 6 ++
drivers/soc/aspeed/aspeed-p2a-ctrl.c| 6 ++
drivers/soc/aspeed/aspeed-uart-routing.c| 6 ++
drivers/soc/fsl/dpaa2-console.c | 6 ++
drivers/soc/fsl/qe/qmc.c| 6 ++
drivers/soc/fsl/qe/tsa.c| 5 ++---
drivers/soc/fujitsu/a64fx-diag.c| 6 ++
drivers/soc/hisilicon/kunpeng_hccs.c| 6 ++
drivers/soc/ixp4xx/ixp4xx-npe.c | 6 ++
drivers/soc/ixp4xx/ixp4xx-qmgr.c| 5 ++---
drivers/soc/litex/litex_soc_ctrl.c | 5 ++---
drivers/soc/loongson/loongson2_guts.c | 6 ++
drivers/soc/mediatek/mtk-devapc.c |
[PATCH 06/40] soc/fsl: cpm: qmc: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new() which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/soc/fsl/qe/qmc.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/soc/fsl/qe/qmc.c b/drivers/soc/fsl/qe/qmc.c
index b3c292c9a14e..92ec76c03965 100644
--- a/drivers/soc/fsl/qe/qmc.c
+++ b/drivers/soc/fsl/qe/qmc.c
@@ -1415,7 +1415,7 @@ static int qmc_probe(struct platform_device *pdev)
return ret;
}
-static int qmc_remove(struct platform_device *pdev)
+static void qmc_remove(struct platform_device *pdev)
{
struct qmc *qmc = platform_get_drvdata(pdev);
@@ -1427,8 +1427,6 @@ static int qmc_remove(struct platform_device *pdev)
/* Disconnect the serial from TSA */
tsa_serial_disconnect(qmc->tsa_serial);
-
- return 0;
}
static const struct of_device_id qmc_id_table[] = {
@@ -1443,7 +1441,7 @@ static struct platform_driver qmc_driver = {
.of_match_table = of_match_ptr(qmc_id_table),
},
.probe = qmc_probe,
- .remove = qmc_remove,
+ .remove_new = qmc_remove,
};
module_platform_driver(qmc_driver);
--
2.40.1
[PATCH 05/40] soc/fsl: dpaa2-console: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new() which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/soc/fsl/dpaa2-console.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/soc/fsl/dpaa2-console.c b/drivers/soc/fsl/dpaa2-console.c
index 1dca693b6b38..6dbc77db7718 100644
--- a/drivers/soc/fsl/dpaa2-console.c
+++ b/drivers/soc/fsl/dpaa2-console.c
@@ -300,12 +300,10 @@ static int dpaa2_console_probe(struct platform_device
*pdev)
return error;
}
-static int dpaa2_console_remove(struct platform_device *pdev)
+static void dpaa2_console_remove(struct platform_device *pdev)
{
misc_deregister(_mc_console_dev);
misc_deregister(_aiop_console_dev);
-
- return 0;
}
static const struct of_device_id dpaa2_console_match_table[] = {
@@ -322,7 +320,7 @@ static struct platform_driver dpaa2_console_driver = {
.of_match_table = dpaa2_console_match_table,
},
.probe = dpaa2_console_probe,
- .remove = dpaa2_console_remove,
+ .remove_new = dpaa2_console_remove,
};
module_platform_driver(dpaa2_console_driver);
--
2.40.1
[PATCH 07/40] soc/fsl: cpm: tsa: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new() which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/soc/fsl/qe/tsa.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/soc/fsl/qe/tsa.c b/drivers/soc/fsl/qe/tsa.c
index 3646153117b3..3f9981335590 100644
--- a/drivers/soc/fsl/qe/tsa.c
+++ b/drivers/soc/fsl/qe/tsa.c
@@ -706,7 +706,7 @@ static int tsa_probe(struct platform_device *pdev)
return 0;
}
-static int tsa_remove(struct platform_device *pdev)
+static void tsa_remove(struct platform_device *pdev)
{
struct tsa *tsa = platform_get_drvdata(pdev);
int i;
@@ -729,7 +729,6 @@ static int tsa_remove(struct platform_device *pdev)
clk_put(tsa->tdm[i].l1rclk_clk);
}
}
- return 0;
}
static const struct of_device_id tsa_id_table[] = {
@@ -744,7 +743,7 @@ static struct platform_driver tsa_driver = {
.of_match_table = of_match_ptr(tsa_id_table),
},
.probe = tsa_probe,
- .remove = tsa_remove,
+ .remove_new = tsa_remove,
};
module_platform_driver(tsa_driver);
--
2.40.1
[PATCH 00/59] dma: Convert to platform remove callback returning void
Hello,
this series convert nearly all platform drivers below drivers/dma to use
.remove_new(). The motivation is to get rid of an integer return code
that is (mostly) ignored by the platform driver core and error prone on
the driver side.
See commit 5c5a7680e67b ("platform: Provide a remove callback that
returns no value") for an extended explanation and the eventual goal.
There are 4 drivers I didn't convert in this series:
drivers/dma/milbeaut-hdmac.c
drivers/dma/milbeaut-xdmac.c
drivers/dma/uniphier-mdmac.c
drivers/dma/uniphier-xdmac.c
These all might return early in .remove() if dmaengine_terminate_sync()
fails. I only looked deeper into the first one, and this shows exactly
the error that is easy to make with .remove() returning an int: When
returning early from .remove(), some cleanup (here:
dma_async_device_unregister()) is skipped. So the dma device stays
known, but the device is still unregistered and the devm allocated stuff
(here e.g. *mdev) is freed. So it can probably easily happen, that
something tries to use the dma device and this will likely result in an
oops.
I don't know enough about the dma framework to address this properly,
maybe someone else knows what to do?
There are no interdependencies between the patches. As there are still
quite a few drivers to convert, I'm happy about every patch that makes
it in. So even if there is a merge conflict with one patch until you
apply or I picked a wrong subject prefix, please apply the remainder of
this series anyhow.
Best regards
Uwe
Uwe Kleine-König (59):
dma: altera-msgdma: Convert to platform remove callback returning void
dma: apple-admac: Convert to platform remove callback returning void
dma: at_hdmac: Convert to platform remove callback returning void
dma: at_xdmac: Convert to platform remove callback returning void
dma: bcm-sba-raid: Convert to platform remove callback returning void
dma: bcm2835-dma: Convert to platform remove callback returning void
dma: bestcomm: bestcomm: Convert to platform remove callback returning
void
dma: dma-axi-dmac: Convert to platform remove callback returning void
dma: dma-jz4780: Convert to platform remove callback returning void
dma: dw-axi-dmac: dw-axi-dmac-platform: Convert to platform remove
callback returning void
dma: dw: platform: Convert to platform remove callback returning void
dma: fsl-edma-main: Convert to platform remove callback returning void
dma: fsl-qdma: Convert to platform remove callback returning void
dma: fsl_raid: Convert to platform remove callback returning void
dma: fsldma: Convert to platform remove callback returning void
dma: idma64: Convert to platform remove callback returning void
dma: img-mdc-dma: Convert to platform remove callback returning void
dma: imx-dma: Convert to platform remove callback returning void
dma: imx-sdma: Convert to platform remove callback returning void
dma: k3dma: Convert to platform remove callback returning void
dma: mcf-edma-main: Convert to platform remove callback returning void
dma: mediatek: mtk-cqdma: Convert to platform remove callback
returning void
dma: mediatek: mtk-hsdma: Convert to platform remove callback
returning void
dma: mediatek: mtk-uart-apdma: Convert to platform remove callback
returning void
dma: mmp_pdma: Convert to platform remove callback returning void
dma: mmp_tdma: Convert to platform remove callback returning void
dma: moxart-dma: Convert to platform remove callback returning void
dma: mpc512x_dma: Convert to platform remove callback returning void
dma: mv_xor_v2: Convert to platform remove callback returning void
dma: nbpfaxi: Convert to platform remove callback returning void
dma: owl-dma: Convert to platform remove callback returning void
dma: ppc4xx: adma: Convert to platform remove callback returning void
dma: pxa_dma: Convert to platform remove callback returning void
dma: qcom: bam_dma: Convert to platform remove callback returning void
dma: qcom: hidma: Convert to platform remove callback returning void
dma: qcom: qcom_adm: Convert to platform remove callback returning
void
dma: sa11x0-dma: Convert to platform remove callback returning void
dma: sf-pdma: sf-pdma: Convert to platform remove callback returning
void
dma: sh: rcar-dmac: Convert to platform remove callback returning void
dma: sh: rz-dmac: Convert to platform remove callback returning void
dma: sh: shdmac: Convert to platform remove callback returning void
dma: sh: usb-dmac: Convert to platform remove callback returning void
dma: sprd-dma: Convert to platform remove callback returning void
dma: st_fdma: Convert to platform remove callback returning void
dma: sun4i-dma: Convert to platform remove callback returning void
dma: sun6i-dma: Convert to platform remove callback returning void
dma: tegra186-gpc-dma: Convert to platform remove callback returning
void
dma: tegra
[PATCH 15/59] dma: fsldma: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new() which already returns void. Eventually after all drivers
are converted, .remove_new() is renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/dma/fsldma.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index ddcf736d283d..18a6c4bf6275 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -1306,7 +1306,7 @@ static int fsldma_of_probe(struct platform_device *op)
return err;
}
-static int fsldma_of_remove(struct platform_device *op)
+static void fsldma_of_remove(struct platform_device *op)
{
struct fsldma_device *fdev;
unsigned int i;
@@ -1324,8 +1324,6 @@ static int fsldma_of_remove(struct platform_device *op)
iounmap(fdev->regs);
kfree(fdev);
-
- return 0;
}
#ifdef CONFIG_PM
@@ -1406,7 +1404,7 @@ static struct platform_driver fsldma_of_driver = {
#endif
},
.probe = fsldma_of_probe,
- .remove = fsldma_of_remove,
+ .remove_new = fsldma_of_remove,
};
/**/
--
2.40.1
[PATCH] powerpc/ep8248e: Mark driver as non removable
Instead of resorting to BUG() ensure that the driver isn't unbound by
suppressing its bind and unbind sysfs attributes. As the driver is
built-in there is no way to remove a device once bound.
As a nice side effect this allows to drop the remove function.
Signed-off-by: Uwe Kleine-König
---
arch/powerpc/platforms/82xx/ep8248e.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/arch/powerpc/platforms/82xx/ep8248e.c
b/arch/powerpc/platforms/82xx/ep8248e.c
index 8f1856ba692e..3409cf04b630 100644
--- a/arch/powerpc/platforms/82xx/ep8248e.c
+++ b/arch/powerpc/platforms/82xx/ep8248e.c
@@ -140,12 +140,6 @@ static int ep8248e_mdio_probe(struct platform_device
*ofdev)
return ret;
}
-static int ep8248e_mdio_remove(struct platform_device *ofdev)
-{
- BUG();
- return 0;
-}
-
static const struct of_device_id ep8248e_mdio_match[] = {
{
.compatible = "fsl,ep8248e-mdio-bitbang",
@@ -157,9 +151,9 @@ static struct platform_driver ep8248e_mdio_driver = {
.driver = {
.name = "ep8248e-mdio-bitbang",
.of_match_table = ep8248e_mdio_match,
+ .suppress_bind_attrs = true,
},
.probe = ep8248e_mdio_probe,
- .remove = ep8248e_mdio_remove,
};
struct cpm_pin {
base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5
--
2.39.2
Re: [PATCH v2] tty: Explicitly include correct DT includes
Hello Rob, On Mon, Jul 24, 2023 at 02:54:38PM -0600, Rob Herring wrote: > The DT of_device.h and of_platform.h date back to the separate > of_platform_bus_type before it as merged into the regular platform bus. > As part of that merge prepping Arm DT support 13 years ago, they > "temporarily" include each other. They also include platform_device.h > and of.h. As a result, there's a pretty much random mix of those include > files used throughout the tree. In order to detangle these headers and > replace the implicit includes with struct declarations, users need to > explicitly include the correct includes. > > Signed-off-by: Rob Herring Acked-by: Uwe Kleine-König # for imx Thanks for your efforts! Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
[PATCH v2] powerpc/512x: lpbfifo: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new() which already returns void. Eventually after all drivers
are converted, .remove_new() is renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
Changes since (implicit) v1:
- provide an actually compilable patch :-\
Best regards
Uwe
arch/powerpc/platforms/512x/mpc512x_lpbfifo.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/platforms/512x/mpc512x_lpbfifo.c
b/arch/powerpc/platforms/512x/mpc512x_lpbfifo.c
index 1bfb29574caa..c1e981649bd9 100644
--- a/arch/powerpc/platforms/512x/mpc512x_lpbfifo.c
+++ b/arch/powerpc/platforms/512x/mpc512x_lpbfifo.c
@@ -477,7 +477,7 @@ static int mpc512x_lpbfifo_probe(struct platform_device
*pdev)
return ret;
}
-static int mpc512x_lpbfifo_remove(struct platform_device *pdev)
+static void mpc512x_lpbfifo_remove(struct platform_device *pdev)
{
unsigned long flags;
struct dma_device *dma_dev = lpbfifo.chan->device;
@@ -494,8 +494,6 @@ static int mpc512x_lpbfifo_remove(struct platform_device
*pdev)
free_irq(lpbfifo.irq, >dev);
irq_dispose_mapping(lpbfifo.irq);
dma_release_channel(lpbfifo.chan);
-
- return 0;
}
static const struct of_device_id mpc512x_lpbfifo_match[] = {
@@ -506,7 +504,7 @@ MODULE_DEVICE_TABLE(of, mpc512x_lpbfifo_match);
static struct platform_driver mpc512x_lpbfifo_driver = {
.probe = mpc512x_lpbfifo_probe,
- .remove = mpc512x_lpbfifo_remove,
+ .remove_new = mpc512x_lpbfifo_remove,
.driver = {
.name = DRV_NAME,
.of_match_table = mpc512x_lpbfifo_match,
base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5
--
2.39.2
[PATCH] powerpc/512x: lpbfifo: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.
To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new() which already returns void. Eventually after all drivers
are converted, .remove_new() is renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
arch/powerpc/platforms/512x/mpc512x_lpbfifo.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/platforms/512x/mpc512x_lpbfifo.c
b/arch/powerpc/platforms/512x/mpc512x_lpbfifo.c
index 1bfb29574caa..dbe722f7b855 100644
--- a/arch/powerpc/platforms/512x/mpc512x_lpbfifo.c
+++ b/arch/powerpc/platforms/512x/mpc512x_lpbfifo.c
@@ -477,7 +477,7 @@ static int mpc512x_lpbfifo_probe(struct platform_device
*pdev)
return ret;
}
-static int mpc512x_lpbfifo_remove(struct platform_device *pdev)
+static void pc512x_lpbfifo_remove(struct platform_device *pdev)
{
unsigned long flags;
struct dma_device *dma_dev = lpbfifo.chan->device;
@@ -494,8 +494,6 @@ static int mpc512x_lpbfifo_remove(struct platform_device
*pdev)
free_irq(lpbfifo.irq, >dev);
irq_dispose_mapping(lpbfifo.irq);
dma_release_channel(lpbfifo.chan);
-
- return 0;
}
static const struct of_device_id mpc512x_lpbfifo_match[] = {
@@ -506,7 +504,7 @@ MODULE_DEVICE_TABLE(of, mpc512x_lpbfifo_match);
static struct platform_driver mpc512x_lpbfifo_driver = {
.probe = mpc512x_lpbfifo_probe,
- .remove = mpc512x_lpbfifo_remove,
+ .remove_new = mpc512x_lpbfifo_remove,
.driver = {
.name = DRV_NAME,
.of_match_table = mpc512x_lpbfifo_match,
base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5
--
2.39.2
[PATCH net-next v3 0/8] net: freescale: Convert to platform remove callback returning void
Hello, v2 of this series was sent in June[1], code changes since then only affect patch #1 where the dev_err invocation was adapted to emit the error code of dpaa_fq_free(). Thanks for feedback by Maciej Fijalkowski and Russell King. Other than that I added Reviewed-by tags for Simon Horman and Wei Fang and rebased to v6.5-rc1. There is only one dependency in this series: Patch #2 depends on patch #1. Best regards Uwe [1] https://lore.kernel.org/netdev/20230606162829.166226-1-u.kleine-koe...@pengutronix.de Uwe Kleine-König (8): net: dpaa: Improve error reporting net: dpaa: Convert to platform remove callback returning void net: fec: Convert to platform remove callback returning void net: fman: Convert to platform remove callback returning void net: fs_enet: Convert to platform remove callback returning void net: fsl_pq_mdio: Convert to platform remove callback returning void net: gianfar: Convert to platform remove callback returning void net: ucc_geth: Convert to platform remove callback returning void drivers/net/ethernet/freescale/dpaa/dpaa_eth.c| 9 + drivers/net/ethernet/freescale/fec_main.c | 5 ++--- drivers/net/ethernet/freescale/fec_mpc52xx.c | 6 ++ drivers/net/ethernet/freescale/fec_mpc52xx_phy.c | 6 ++ drivers/net/ethernet/freescale/fman/mac.c | 5 ++--- drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c | 5 ++--- drivers/net/ethernet/freescale/fs_enet/mii-bitbang.c | 6 ++ drivers/net/ethernet/freescale/fs_enet/mii-fec.c | 6 ++ drivers/net/ethernet/freescale/fsl_pq_mdio.c | 6 ++ drivers/net/ethernet/freescale/gianfar.c | 6 ++ drivers/net/ethernet/freescale/ucc_geth.c | 6 ++ 11 files changed, 25 insertions(+), 41 deletions(-) base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5 -- 2.39.2
[PATCH net-next v3 8/8] net: ucc_geth: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Reviewed-by: Michal Kubiak
Reviewed-by: Simon Horman
Signed-off-by: Uwe Kleine-König
---
drivers/net/ethernet/freescale/ucc_geth.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/freescale/ucc_geth.c
b/drivers/net/ethernet/freescale/ucc_geth.c
index 7a4cb4f07c32..2b3a15f24e7c 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -3753,7 +3753,7 @@ static int ucc_geth_probe(struct platform_device* ofdev)
return err;
}
-static int ucc_geth_remove(struct platform_device* ofdev)
+static void ucc_geth_remove(struct platform_device* ofdev)
{
struct net_device *dev = platform_get_drvdata(ofdev);
struct ucc_geth_private *ugeth = netdev_priv(dev);
@@ -3767,8 +3767,6 @@ static int ucc_geth_remove(struct platform_device* ofdev)
of_node_put(ugeth->ug_info->phy_node);
kfree(ugeth->ug_info);
free_netdev(dev);
-
- return 0;
}
static const struct of_device_id ucc_geth_match[] = {
@@ -3787,7 +3785,7 @@ static struct platform_driver ucc_geth_driver = {
.of_match_table = ucc_geth_match,
},
.probe = ucc_geth_probe,
- .remove = ucc_geth_remove,
+ .remove_new = ucc_geth_remove,
.suspend= ucc_geth_suspend,
.resume = ucc_geth_resume,
};
--
2.39.2
[PATCH net-next v3 5/8] net: fs_enet: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Reviewed-by: Michal Kubiak
Reviewed-by: Simon Horman
Signed-off-by: Uwe Kleine-König
---
drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c | 5 ++---
drivers/net/ethernet/freescale/fs_enet/mii-bitbang.c | 6 ++
drivers/net/ethernet/freescale/fs_enet/mii-fec.c | 6 ++
3 files changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
index 8844a9a04fcf..f9f5b28cc72e 100644
--- a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
+++ b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
@@ -1051,7 +1051,7 @@ static int fs_enet_probe(struct platform_device *ofdev)
return ret;
}
-static int fs_enet_remove(struct platform_device *ofdev)
+static void fs_enet_remove(struct platform_device *ofdev)
{
struct net_device *ndev = platform_get_drvdata(ofdev);
struct fs_enet_private *fep = netdev_priv(ndev);
@@ -1066,7 +1066,6 @@ static int fs_enet_remove(struct platform_device *ofdev)
if (of_phy_is_fixed_link(ofdev->dev.of_node))
of_phy_deregister_fixed_link(ofdev->dev.of_node);
free_netdev(ndev);
- return 0;
}
static const struct of_device_id fs_enet_match[] = {
@@ -1113,7 +1112,7 @@ static struct platform_driver fs_enet_driver = {
.of_match_table = fs_enet_match,
},
.probe = fs_enet_probe,
- .remove = fs_enet_remove,
+ .remove_new = fs_enet_remove,
};
#ifdef CONFIG_NET_POLL_CONTROLLER
diff --git a/drivers/net/ethernet/freescale/fs_enet/mii-bitbang.c
b/drivers/net/ethernet/freescale/fs_enet/mii-bitbang.c
index 21de56345503..91a69fc2f7c2 100644
--- a/drivers/net/ethernet/freescale/fs_enet/mii-bitbang.c
+++ b/drivers/net/ethernet/freescale/fs_enet/mii-bitbang.c
@@ -192,7 +192,7 @@ static int fs_enet_mdio_probe(struct platform_device *ofdev)
return ret;
}
-static int fs_enet_mdio_remove(struct platform_device *ofdev)
+static void fs_enet_mdio_remove(struct platform_device *ofdev)
{
struct mii_bus *bus = platform_get_drvdata(ofdev);
struct bb_info *bitbang = bus->priv;
@@ -201,8 +201,6 @@ static int fs_enet_mdio_remove(struct platform_device
*ofdev)
free_mdio_bitbang(bus);
iounmap(bitbang->dir);
kfree(bitbang);
-
- return 0;
}
static const struct of_device_id fs_enet_mdio_bb_match[] = {
@@ -219,7 +217,7 @@ static struct platform_driver fs_enet_bb_mdio_driver = {
.of_match_table = fs_enet_mdio_bb_match,
},
.probe = fs_enet_mdio_probe,
- .remove = fs_enet_mdio_remove,
+ .remove_new = fs_enet_mdio_remove,
};
module_platform_driver(fs_enet_bb_mdio_driver);
diff --git a/drivers/net/ethernet/freescale/fs_enet/mii-fec.c
b/drivers/net/ethernet/freescale/fs_enet/mii-fec.c
index 59a8f0bd0f5c..1910df250c33 100644
--- a/drivers/net/ethernet/freescale/fs_enet/mii-fec.c
+++ b/drivers/net/ethernet/freescale/fs_enet/mii-fec.c
@@ -187,7 +187,7 @@ static int fs_enet_mdio_probe(struct platform_device *ofdev)
return ret;
}
-static int fs_enet_mdio_remove(struct platform_device *ofdev)
+static void fs_enet_mdio_remove(struct platform_device *ofdev)
{
struct mii_bus *bus = platform_get_drvdata(ofdev);
struct fec_info *fec = bus->priv;
@@ -196,8 +196,6 @@ static int fs_enet_mdio_remove(struct platform_device
*ofdev)
iounmap(fec->fecp);
kfree(fec);
mdiobus_free(bus);
-
- return 0;
}
static const struct of_device_id fs_enet_mdio_fec_match[] = {
@@ -220,7 +218,7 @@ static struct platform_driver fs_enet_fec_mdio_driver = {
.of_match_table = fs_enet_mdio_fec_match,
},
.probe = fs_enet_mdio_probe,
- .remove = fs_enet_mdio_remove,
+ .remove_new = fs_enet_mdio_remove,
};
module_platform_driver(fs_enet_fec_mdio_driver);
--
2.39.2
Re: [PATCH] powerpc: Switch i2c drivers back to use .probe()
Hello,
On Thu, May 25, 2023 at 10:56:22PM +0200, Uwe Kleine-König wrote:
> After commit b8a1a4cd5a98 ("i2c: Provide a temporary .probe_new()
> call-back type"), all drivers being converted to .probe_new() and then
> 03c835f498b5 ("i2c: Switch .probe() to not take an id parameter")
> convert back to (the new) .probe() to be able to eventually drop
> .probe_new() from struct i2c_driver.
>
> Signed-off-by: Uwe Kleine-König
Is there a chance to get this patch in before v6.5-rc1? I intend to
complete the conversion (i.e. drop .probe_new() from struct i2c_driver)
after v6.5-rc1. If this patch doesn't make it in, I'd include it in a
pull request for Wolfram's i2c tree early after v6.5-rc1. So if you
don't take it any more for the upcoming merge window, an ack would be
great.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |
signature.asc
Description: PGP signature
Re: [PATCH] macintosh: Switch i2c drivers back to use .probe()
Hello,
On Tue, May 23, 2023 at 09:50:53PM +0200, Uwe Kleine-König wrote:
> After commit b8a1a4cd5a98 ("i2c: Provide a temporary .probe_new()
> call-back type"), all drivers being converted to .probe_new() and then
> 03c835f498b5 ("i2c: Switch .probe() to not take an id parameter") convert
> back to (the new) .probe() to be able to eventually drop .probe_new() from
> struct i2c_driver.
>
> Signed-off-by: Uwe Kleine-König
> ---
> Hello,
>
> this patch was generated using coccinelle, but I aligned the result to
> the per-file indention.
>
> I chose to convert all drivers below drivers/macintosh in a single
> patch, but if you prefer I can split by driver.
>
> v6.4-rc1 was taken as a base, as there are no commits in next touching
> drivers/macintosh I don't expect problems when applying this patch. If
> conflicts arise until this is applied, feel free to just drop the files
> with conflicts from this patch. I'll care about the fallout later then.
>
> Also note there is no coordination necessary with the i2c tree. Dropping
> .probe_new() will happen only when all (or most) drivers are converted,
> which will happen after v6.5-rc1 for sure.
Can someone still pick up this patch for v6.5-rc1? I intend to send a
pull request to Wolfram's i2c-tree that drops .probe_new() from struct
i2c_driver on top of v6.5-rc1 early after the merge window closes. So
getting this in before would be great. Otherwise I'm happily adding
received acks to this patch for my PR :-)
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |
signature.asc
Description: PGP signature
[PATCH v4 1/2] powerpc/legacy_serial: Handle SERIAL_8250_FSL=n build failures
With SERIAL_8250=y and SERIAL_8250_FSL_CONSOLE=n the both
IS_ENABLED(CONFIG_SERIAL_8250) and IS_REACHABLE(CONFIG_SERIAL_8250)
evaluate to true and so fsl8250_handle_irq() is used. However this
function is only available if CONFIG_SERIAL_8250_CONSOLE=y (and thus
SERIAL_8250_FSL=y).
To prepare SERIAL_8250_FSL becoming tristate and being enabled in more
cases, check for IS_REACHABLE(CONFIG_SERIAL_8250_FSL) before making use
of fsl8250_handle_irq(). This check is correct with and without the
change to make SERIAL_8250_FSL modular.
Reported-by: Randy Dunlap
Fixes: 66eff0ef528b ("powerpc/legacy_serial: Warn about 8250 devices operated
without active FSL workarounds")
Signed-off-by: Uwe Kleine-König
---
arch/powerpc/kernel/legacy_serial.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/legacy_serial.c
b/arch/powerpc/kernel/legacy_serial.c
index fdbd85aafeb1..6ee65741dbd5 100644
--- a/arch/powerpc/kernel/legacy_serial.c
+++ b/arch/powerpc/kernel/legacy_serial.c
@@ -510,7 +510,7 @@ static void __init fixup_port_irq(int index,
if (IS_ENABLED(CONFIG_SERIAL_8250) &&
of_device_is_compatible(np, "fsl,ns16550")) {
- if (IS_REACHABLE(CONFIG_SERIAL_8250)) {
+ if (IS_REACHABLE(CONFIG_SERIAL_8250_FSL)) {
port->handle_irq = fsl8250_handle_irq;
port->has_sysrq =
IS_ENABLED(CONFIG_SERIAL_8250_CONSOLE);
} else {
--
2.39.2
[PATCH v4 0/2] serial: 8250: Apply FSL workarounds also without SERIAL_8250_CONSOLE
Hello,
this is the fourth iteration of trying to make the FSL workaround code
active even without 8250 console support.
The first patch is a fix for commit 66eff0ef528b (powerpc/legacy_serial:
Warn about 8250 devices operated without active FSL workarounds) that
currently is in tty-next. This patch originates from my v3 that was only
partially applied. (That is a lame excuse though. While the applying the
full series would not have shown this problem, bisection would still
have a problem.)
The second patch makes SERIAL_8250_FSL tristate and thus allows this to
be enabled also with SERIAL_8250=m. This is also the relevant change
since v3, where 8250_fsl.o was linked into 8250-base.ko.
This series is build tested on amd64 and powerpc with all 27 possible
configurations for
SERIAL_8250={y,m,n}
SERIAL_8250_FSL={y,m,n}
SERIAL_OF_PLATFORM={y,m,n}
using:
choices=(y m n)
for i in $(seq 0 26); do
perl -p -e "s/SERIAL_8250=y/SERIAL_8250=${choices[$(((i / 9) %
3))]}/; s/SERIAL_8250_FSL=y/SERIAL_8250_FSL=${choices[$(((i / 3) % 3))]}/;
s/SERIAL_OF_PLATFORM=y/SERIAL_OF_PLATFORM=${choices[$((i % 3))]}/;" .config-pre
> .config &&
make -j 12 ||
break;
done
with .config-pre having COMPILE_TEST=y so this time there shouldn't be a
build regression. (Not all 27 variants are possible, so some valid
configurations are tested twice or more, but that's still good enough.)
The patches have no strong dependency on each other, so they could go in
via different trees. But given that 66eff0ef528b is in tty-next, taking
both via tty sounds most sensible.
Best regards
Uwe
Uwe Kleine-König (2):
powerpc/legacy_serial: Handle SERIAL_8250_FSL=n build failures
serial: 8250: Apply FSL workarounds also without SERIAL_8250_CONSOLE
arch/powerpc/kernel/legacy_serial.c | 2 +-
drivers/tty/serial/8250/8250_fsl.c | 3 +++
drivers/tty/serial/8250/8250_of.c | 2 +-
drivers/tty/serial/8250/Kconfig | 6 +++---
4 files changed, 8 insertions(+), 5 deletions(-)
base-commit: 66eff0ef528b6d6e9a45b68f6cd969dcbe7b800a
--
2.39.2
Re: [PATCH] powerpc/legacy_serial: check CONFIG_SERIAL_8250_CONSOLE
Hello Randy,
On Thu, Jun 08, 2023 at 05:33:28PM -0700, Randy Dunlap wrote:
> When SERIAL_8250_CONSOLE is not set but PPC_UDBG_16550=y,
> the legacy_serial code references fsl8250_handle_irq, which is
> only built when SERIAL_8250_CONSOLE is set.
>
> Be consistent in referencing the used CONFIG_SERIAL_8250*
> symbols so that the build errors do not happen.
>
> Prevents these build errors:
>
> powerpc-linux-ld: arch/powerpc/kernel/legacy_serial.o: in function
> `serial_dev_init':
> legacy_serial.c:(.init.text+0x2aa): undefined reference to
> `fsl8250_handle_irq'
> powerpc-linux-ld: legacy_serial.c:(.init.text+0x2b2): undefined reference to
> `fsl8250_handle_irq'
>
> Fixes: 66eff0ef528b ("powerpc/legacy_serial: Warn about 8250 devices operated
> without active FSL workarounds")
> Signed-off-by: Randy Dunlap
> Cc: Uwe Kleine-König
> Cc: Greg Kroah-Hartman
> Cc: linux-ser...@vger.kernel.org
> Cc: Michael Ellerman
> Cc: Nicholas Piggin
> Cc: Christophe Leroy
> Cc: linuxppc-dev@lists.ozlabs.org
> ---
> arch/powerpc/kernel/legacy_serial.c |4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff -- a/arch/powerpc/kernel/legacy_serial.c
> b/arch/powerpc/kernel/legacy_serial.c
> --- a/arch/powerpc/kernel/legacy_serial.c
> +++ b/arch/powerpc/kernel/legacy_serial.c
> @@ -508,9 +508,9 @@ static void __init fixup_port_irq(int in
>
> port->irq = virq;
>
> - if (IS_ENABLED(CONFIG_SERIAL_8250) &&
> + if (IS_ENABLED(CONFIG_SERIAL_8250_CONSOLE) &&
> of_device_is_compatible(np, "fsl,ns16550")) {
> - if (IS_REACHABLE(CONFIG_SERIAL_8250)) {
> + if (IS_REACHABLE(CONFIG_SERIAL_8250_CONSOLE)) {
> port->handle_irq = fsl8250_handle_irq;
> port->has_sysrq =
> IS_ENABLED(CONFIG_SERIAL_8250_CONSOLE);
> } else {
Argh, indeed there is a problem. Your patch however defeats the idea of
66eff0ef528b[1] which is still valid. And with your patch the else
branch that starts at the end of the above hunk is never taken.
With the feedback I got on
https://lore.kernel.org/linux-serial/20230605130857.85543-3-u.kleine-koe...@pengutronix.de
the probable outcome is that CONFIG_SERIAL_8250_FSL becomes tristate and
so the fix that is more future proof and keeps the warning, looks as
follows:
diff --git a/arch/powerpc/kernel/legacy_serial.c
b/arch/powerpc/kernel/legacy_serial.c
index fdbd85aafeb1..6ee65741dbd5 100644
--- a/arch/powerpc/kernel/legacy_serial.c
+++ b/arch/powerpc/kernel/legacy_serial.c
@@ -510,7 +510,7 @@ static void __init fixup_port_irq(int index,
if (IS_ENABLED(CONFIG_SERIAL_8250) &&
of_device_is_compatible(np, "fsl,ns16550")) {
- if (IS_REACHABLE(CONFIG_SERIAL_8250)) {
+ if (IS_REACHABLE(CONFIG_SERIAL_8250_FSL)) {
port->handle_irq = fsl8250_handle_irq;
port->has_sysrq =
IS_ENABLED(CONFIG_SERIAL_8250_CONSOLE);
} else {
This should to the right thing now (while CONFIG_SERIAL_8250_FSL is
still bool and only on if CONFIG_SERIAL_8250 is =y) and also once
CONFIG_SERIAL_8250_FSL can be =m (which would make fsl8250_handle_irq
not available for powerpc platform code).
But given that I screwed this up several times now, I will think about
this some more and do some more tests before submitting that as a proper
patch.
Best regards
Uwe
[1] Warn if the 8250 device is used but the required FSL workarounds are
not.
--
Pengutronix e.K. | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |
signature.asc
Description: PGP signature
[PATCH net-next v2 0/8] net: freescale: Convert to platform remove callback returning void
Hello, (implicit) v1 of this series was sent in March and can be found at https://lore.kernel.org/netdev/20230313103653.2753139-1-u.kleine-koe...@pengutronix.de Changes since then: - Dropped "net: fec: Don't return early on error in .remove()", this is replaced by "net: fec: Better handle pm_runtime_get() failing in .remove()" which is already in v6.4-rc3 as f816b9829b19394d318e01953aa3b2721bca040d. - Simplified dev_err(>dev, ... to dev_err(dev, ... in patch #1; pointed out by Madalin Bucur. - Added some review tags received for v1. Thanks to Madalin Bucur, Andrew Lunn, Michal Kubiak and Jakub Kicinski for their replies to v1. Best regards Uwe Uwe Kleine-König (8): net: dpaa: Improve error reporting net: dpaa: Convert to platform remove callback returning void net: fec: Convert to platform remove callback returning void net: fman: Convert to platform remove callback returning void net: fs_enet: Convert to platform remove callback returning void net: fsl_pq_mdio: Convert to platform remove callback returning void net: gianfar: Convert to platform remove callback returning void net: ucc_geth: Convert to platform remove callback returning void drivers/net/ethernet/freescale/dpaa/dpaa_eth.c| 8 drivers/net/ethernet/freescale/fec_main.c | 5 ++--- drivers/net/ethernet/freescale/fec_mpc52xx.c | 6 ++ drivers/net/ethernet/freescale/fec_mpc52xx_phy.c | 6 ++ drivers/net/ethernet/freescale/fman/mac.c | 5 ++--- drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c | 5 ++--- drivers/net/ethernet/freescale/fs_enet/mii-bitbang.c | 6 ++ drivers/net/ethernet/freescale/fs_enet/mii-fec.c | 6 ++ drivers/net/ethernet/freescale/fsl_pq_mdio.c | 6 ++ drivers/net/ethernet/freescale/gianfar.c | 6 ++ drivers/net/ethernet/freescale/ucc_geth.c | 6 ++ 11 files changed, 24 insertions(+), 41 deletions(-) base-commit: d4031ec844bc52fe7f2f844e9c476727fd6b8240 -- 2.39.2
[PATCH net-next v2 5/8] net: fs_enet: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Reviewed-by: Michal Kubiak
Signed-off-by: Uwe Kleine-König
---
drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c | 5 ++---
drivers/net/ethernet/freescale/fs_enet/mii-bitbang.c | 6 ++
drivers/net/ethernet/freescale/fs_enet/mii-fec.c | 6 ++
3 files changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
index 8844a9a04fcf..f9f5b28cc72e 100644
--- a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
+++ b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
@@ -1051,7 +1051,7 @@ static int fs_enet_probe(struct platform_device *ofdev)
return ret;
}
-static int fs_enet_remove(struct platform_device *ofdev)
+static void fs_enet_remove(struct platform_device *ofdev)
{
struct net_device *ndev = platform_get_drvdata(ofdev);
struct fs_enet_private *fep = netdev_priv(ndev);
@@ -1066,7 +1066,6 @@ static int fs_enet_remove(struct platform_device *ofdev)
if (of_phy_is_fixed_link(ofdev->dev.of_node))
of_phy_deregister_fixed_link(ofdev->dev.of_node);
free_netdev(ndev);
- return 0;
}
static const struct of_device_id fs_enet_match[] = {
@@ -1113,7 +1112,7 @@ static struct platform_driver fs_enet_driver = {
.of_match_table = fs_enet_match,
},
.probe = fs_enet_probe,
- .remove = fs_enet_remove,
+ .remove_new = fs_enet_remove,
};
#ifdef CONFIG_NET_POLL_CONTROLLER
diff --git a/drivers/net/ethernet/freescale/fs_enet/mii-bitbang.c
b/drivers/net/ethernet/freescale/fs_enet/mii-bitbang.c
index 21de56345503..91a69fc2f7c2 100644
--- a/drivers/net/ethernet/freescale/fs_enet/mii-bitbang.c
+++ b/drivers/net/ethernet/freescale/fs_enet/mii-bitbang.c
@@ -192,7 +192,7 @@ static int fs_enet_mdio_probe(struct platform_device *ofdev)
return ret;
}
-static int fs_enet_mdio_remove(struct platform_device *ofdev)
+static void fs_enet_mdio_remove(struct platform_device *ofdev)
{
struct mii_bus *bus = platform_get_drvdata(ofdev);
struct bb_info *bitbang = bus->priv;
@@ -201,8 +201,6 @@ static int fs_enet_mdio_remove(struct platform_device
*ofdev)
free_mdio_bitbang(bus);
iounmap(bitbang->dir);
kfree(bitbang);
-
- return 0;
}
static const struct of_device_id fs_enet_mdio_bb_match[] = {
@@ -219,7 +217,7 @@ static struct platform_driver fs_enet_bb_mdio_driver = {
.of_match_table = fs_enet_mdio_bb_match,
},
.probe = fs_enet_mdio_probe,
- .remove = fs_enet_mdio_remove,
+ .remove_new = fs_enet_mdio_remove,
};
module_platform_driver(fs_enet_bb_mdio_driver);
diff --git a/drivers/net/ethernet/freescale/fs_enet/mii-fec.c
b/drivers/net/ethernet/freescale/fs_enet/mii-fec.c
index d37d7a19a759..94bd76c6cf9e 100644
--- a/drivers/net/ethernet/freescale/fs_enet/mii-fec.c
+++ b/drivers/net/ethernet/freescale/fs_enet/mii-fec.c
@@ -187,7 +187,7 @@ static int fs_enet_mdio_probe(struct platform_device *ofdev)
return ret;
}
-static int fs_enet_mdio_remove(struct platform_device *ofdev)
+static void fs_enet_mdio_remove(struct platform_device *ofdev)
{
struct mii_bus *bus = platform_get_drvdata(ofdev);
struct fec_info *fec = bus->priv;
@@ -196,8 +196,6 @@ static int fs_enet_mdio_remove(struct platform_device
*ofdev)
iounmap(fec->fecp);
kfree(fec);
mdiobus_free(bus);
-
- return 0;
}
static const struct of_device_id fs_enet_mdio_fec_match[] = {
@@ -220,7 +218,7 @@ static struct platform_driver fs_enet_fec_mdio_driver = {
.of_match_table = fs_enet_mdio_fec_match,
},
.probe = fs_enet_mdio_probe,
- .remove = fs_enet_mdio_remove,
+ .remove_new = fs_enet_mdio_remove,
};
module_platform_driver(fs_enet_fec_mdio_driver);
--
2.39.2
[PATCH net-next v2 8/8] net: ucc_geth: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Reviewed-by: Michal Kubiak
Signed-off-by: Uwe Kleine-König
---
drivers/net/ethernet/freescale/ucc_geth.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/freescale/ucc_geth.c
b/drivers/net/ethernet/freescale/ucc_geth.c
index 7a4cb4f07c32..2b3a15f24e7c 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -3753,7 +3753,7 @@ static int ucc_geth_probe(struct platform_device* ofdev)
return err;
}
-static int ucc_geth_remove(struct platform_device* ofdev)
+static void ucc_geth_remove(struct platform_device* ofdev)
{
struct net_device *dev = platform_get_drvdata(ofdev);
struct ucc_geth_private *ugeth = netdev_priv(dev);
@@ -3767,8 +3767,6 @@ static int ucc_geth_remove(struct platform_device* ofdev)
of_node_put(ugeth->ug_info->phy_node);
kfree(ugeth->ug_info);
free_netdev(dev);
-
- return 0;
}
static const struct of_device_id ucc_geth_match[] = {
@@ -3787,7 +3785,7 @@ static struct platform_driver ucc_geth_driver = {
.of_match_table = ucc_geth_match,
},
.probe = ucc_geth_probe,
- .remove = ucc_geth_remove,
+ .remove_new = ucc_geth_remove,
.suspend= ucc_geth_suspend,
.resume = ucc_geth_resume,
};
--
2.39.2
Re: [PATCH v3 2/2] serial: 8250: Apply FSL workarounds also without SERIAL_8250_CONSOLE
Hello Ilpo,
On Mon, Jun 05, 2023 at 04:44:08PM +0300, Ilpo Järvinen wrote:
> On Mon, 5 Jun 2023, Uwe Kleine-König wrote:
> > On Mon, Jun 05, 2023 at 04:22:55PM +0300, Ilpo Järvinen wrote:
> > > On Mon, 5 Jun 2023, Uwe Kleine-König wrote:
> > >
> > > > The need to handle the FSL variant of 8250 in a special way is also
> > > > present without console support. So soften the dependency for
> > > > SERIAL_8250_FSL accordingly. Note that with the 8250 driver compiled as
> > > > a module, some devices still might not make use of the needed
> > > > workarounds. That affects the ports instantiated in
> > > > arch/powerpc/kernel/legacy_serial.c.
> > > >
> > > > This issue was identified by Dominik Andreas Schorpp.
> > > >
> > > > To cope for CONFIG_SERIAL_8250=m + CONFIG_SERIAL_8250_FSL=y, 8250_fsl.o
> > > > must be put in the same compilation unit as 8250_port.o because the
> > > > latter defines some functions needed in the former and so 8250_fsl.o
> > > > must not be built-in if 8250_port.o is available in a module.
> > > >
> > > > Acked-by: Ilpo Järvinen
> > > > Link:
> > > > https://lore.kernel.org/r/20230531083230.2702181-1-u.kleine-koe...@pengutronix.de
> > > > Signed-off-by: Uwe Kleine-König
> > > > ---
> > > > drivers/tty/serial/8250/Kconfig | 2 +-
> > > > drivers/tty/serial/8250/Makefile | 2 +-
> > > > 2 files changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/tty/serial/8250/Kconfig
> > > > b/drivers/tty/serial/8250/Kconfig
> > > > index 5313aa31930f..10c09b19c871 100644
> > > > --- a/drivers/tty/serial/8250/Kconfig
> > > > +++ b/drivers/tty/serial/8250/Kconfig
> > > > @@ -378,7 +378,7 @@ config SERIAL_8250_BCM2835AUX
> > > >
> > > > config SERIAL_8250_FSL
> > > > bool "Freescale 16550 UART support" if COMPILE_TEST && !(PPC ||
> > > > ARM || ARM64)
> > > > - depends on SERIAL_8250_CONSOLE
> > > > + depends on SERIAL_8250
> > >
> > > Just one additional thought: After the adding the arch side
> > > workaround/hack, SERIAL_8250_FSL could become a tristate?
> >
> > I see no benefit for a module separate from 8250_base.ko. There are
> > dependencies in both directions between 8250_port.o and 8250_fsl.o[1].
> > So in my book a bool SERIAL_8250_FSL that modifies 8250_base.ko (with
> > SERIAL_8250=m) is fine.
> >
> > Best regards
> > Uwe
> >
> > [1] 8250_port.o uses fsl8250_handle_irq() from 8250_fsl.o
>
> Is that after some fix which isn't in tty-next? I see only these:
>
> $ git grep -l fsl8250_handle_irq
> arch/powerpc/kernel/legacy_serial.c
> drivers/tty/serial/8250/8250_fsl.c
> drivers/tty/serial/8250/8250_of.c
> include/linux/serial_8250.h
>
> No users of fsl8250_handle_irq in 8250_port.c.
Ah right, I was too quick:
8250_of.o uses fsl8250_handle_irq() from 8250_fsl.o
8250_fsl.o uses serial8250_modem_status() from 8250_port.o (which is in
8250_base.o)
However linking 8250_fsl.o in 8250_of.o isn't a good solution either as
8250_fsl.o should also be available with CONFIG_SERIAL_OF_PLATFORM
disabled to provide the ACPI driver. And as 8250_of.o already depends on
8250_port.o (e.g. via serial8250_em485_config()) adding 8250_fsl.o
together with 8250_port.o into 8250_base.ko is fine and doesn't add new
dependencies.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |
signature.asc
Description: PGP signature
Re: [PATCH v3 2/2] serial: 8250: Apply FSL workarounds also without SERIAL_8250_CONSOLE
On Mon, Jun 05, 2023 at 04:22:55PM +0300, Ilpo Järvinen wrote: > On Mon, 5 Jun 2023, Uwe Kleine-König wrote: > > > The need to handle the FSL variant of 8250 in a special way is also > > present without console support. So soften the dependency for > > SERIAL_8250_FSL accordingly. Note that with the 8250 driver compiled as > > a module, some devices still might not make use of the needed > > workarounds. That affects the ports instantiated in > > arch/powerpc/kernel/legacy_serial.c. > > > > This issue was identified by Dominik Andreas Schorpp. > > > > To cope for CONFIG_SERIAL_8250=m + CONFIG_SERIAL_8250_FSL=y, 8250_fsl.o > > must be put in the same compilation unit as 8250_port.o because the > > latter defines some functions needed in the former and so 8250_fsl.o > > must not be built-in if 8250_port.o is available in a module. > > > > Acked-by: Ilpo Järvinen > > Link: > > https://lore.kernel.org/r/20230531083230.2702181-1-u.kleine-koe...@pengutronix.de > > Signed-off-by: Uwe Kleine-König > > --- > > drivers/tty/serial/8250/Kconfig | 2 +- > > drivers/tty/serial/8250/Makefile | 2 +- > > 2 files changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/tty/serial/8250/Kconfig > > b/drivers/tty/serial/8250/Kconfig > > index 5313aa31930f..10c09b19c871 100644 > > --- a/drivers/tty/serial/8250/Kconfig > > +++ b/drivers/tty/serial/8250/Kconfig > > @@ -378,7 +378,7 @@ config SERIAL_8250_BCM2835AUX > > > > config SERIAL_8250_FSL > > bool "Freescale 16550 UART support" if COMPILE_TEST && !(PPC || ARM || > > ARM64) > > - depends on SERIAL_8250_CONSOLE > > + depends on SERIAL_8250 > > Just one additional thought: After the adding the arch side > workaround/hack, SERIAL_8250_FSL could become a tristate? I see no benefit for a module separate from 8250_base.ko. There are dependencies in both directions between 8250_port.o and 8250_fsl.o[1]. So in my book a bool SERIAL_8250_FSL that modifies 8250_base.ko (with SERIAL_8250=m) is fine. Best regards Uwe [1] 8250_port.o uses fsl8250_handle_irq() from 8250_fsl.o, and 8250_fsl.o uses serial8250_modem_status from 8250_port.o. -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
[PATCH v3 2/2] serial: 8250: Apply FSL workarounds also without SERIAL_8250_CONSOLE
The need to handle the FSL variant of 8250 in a special way is also present without console support. So soften the dependency for SERIAL_8250_FSL accordingly. Note that with the 8250 driver compiled as a module, some devices still might not make use of the needed workarounds. That affects the ports instantiated in arch/powerpc/kernel/legacy_serial.c. This issue was identified by Dominik Andreas Schorpp. To cope for CONFIG_SERIAL_8250=m + CONFIG_SERIAL_8250_FSL=y, 8250_fsl.o must be put in the same compilation unit as 8250_port.o because the latter defines some functions needed in the former and so 8250_fsl.o must not be built-in if 8250_port.o is available in a module. Acked-by: Ilpo Järvinen Link: https://lore.kernel.org/r/20230531083230.2702181-1-u.kleine-koe...@pengutronix.de Signed-off-by: Uwe Kleine-König --- drivers/tty/serial/8250/Kconfig | 2 +- drivers/tty/serial/8250/Makefile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig index 5313aa31930f..10c09b19c871 100644 --- a/drivers/tty/serial/8250/Kconfig +++ b/drivers/tty/serial/8250/Kconfig @@ -378,7 +378,7 @@ config SERIAL_8250_BCM2835AUX config SERIAL_8250_FSL bool "Freescale 16550 UART support" if COMPILE_TEST && !(PPC || ARM || ARM64) - depends on SERIAL_8250_CONSOLE + depends on SERIAL_8250 default PPC || ARM || ARM64 help Selecting this option enables a workaround for a break-detection diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile index 4fc2fc1f41b6..8824ba5295b6 100644 --- a/drivers/tty/serial/8250/Makefile +++ b/drivers/tty/serial/8250/Makefile @@ -12,6 +12,7 @@ obj-$(CONFIG_SERIAL_8250) += 8250.o 8250_base.o 8250_base-$(CONFIG_SERIAL_8250_DMA)+= 8250_dma.o 8250_base-$(CONFIG_SERIAL_8250_DWLIB) += 8250_dwlib.o 8250_base-$(CONFIG_SERIAL_8250_FINTEK) += 8250_fintek.o +8250_base-$(CONFIG_SERIAL_8250_FSL)+= 8250_fsl.o 8250_base-$(CONFIG_SERIAL_8250_PCILIB) += 8250_pcilib.o obj-$(CONFIG_SERIAL_8250_PARISC) += 8250_parisc.o obj-$(CONFIG_SERIAL_8250_PCI) += 8250_pci.o @@ -28,7 +29,6 @@ obj-$(CONFIG_SERIAL_8250_BOCA)+= 8250_boca.o obj-$(CONFIG_SERIAL_8250_EXAR_ST16C554)+= 8250_exar_st16c554.o obj-$(CONFIG_SERIAL_8250_HUB6) += 8250_hub6.o obj-$(CONFIG_SERIAL_8250_PCI1) += 8250_pci1.o -obj-$(CONFIG_SERIAL_8250_FSL) += 8250_fsl.o obj-$(CONFIG_SERIAL_8250_MEN_MCB) += 8250_men_mcb.o obj-$(CONFIG_SERIAL_8250_DFL) += 8250_dfl.o obj-$(CONFIG_SERIAL_8250_DW) += 8250_dw.o -- 2.39.2
[PATCH v3 0/2] serial: 8250: Apply FSL workarounds also without SERIAL_8250_CONSOLE
Hello, this is v3 of the series that now also copes for arch/powerpc/kernel/legacy_serial.c using fsl8250_handle_irq(). For kernel configurations that already before were correctly using the Freescale workarounds, this is the case with this series applied, too. So in all cases the situation doesn't get worse. The upside is that even with the 8250 driver compiled as a module (or built-in but without console support) the workarounds are now applied for all devices but for the ones instantiated in arch/powerpc/kernel/legacy_serial.c. (And even for these there might not be a problem as they might benefit from enabling the workarounds in drivers/tty/serial/8250/8250_of.c. Not sure though.) Patch #1 is new here. Patch #2 only changed lightly: I restored alphabetic order in drivers/tty/serial/8250/Kconfig. As patch #1 is needed for patch #2 to not introduce a build failure, both patches should be taken together. I suggest to add them to Greg's serial tree, but the changes pending there should not conflict with this series such that taking them both via powerpc works, too. Best regards Uwe Uwe Kleine-König (2): powerpc/legacy_serial: Warn about 8250 devices operated without active FSL workarounds serial: 8250: Apply FSL workarounds also without SERIAL_8250_CONSOLE arch/powerpc/kernel/legacy_serial.c | 14 +- drivers/tty/serial/8250/Kconfig | 2 +- drivers/tty/serial/8250/Makefile| 2 +- 3 files changed, 11 insertions(+), 7 deletions(-) base-commit: ac9a78681b921877518763ba0e89202254349d1b -- 2.39.2
[PATCH v3 1/2] powerpc/legacy_serial: Warn about 8250 devices operated without active FSL workarounds
If the 8250 driver is built as a module (or built-in without console
support) the Freescale specific workaround were silently not activated.
Add a warning in this case.
Currently CONFIG_SERIAL_8250_FSL=y implies that the function
fsl8250_handle_irq() is built-in and can be used. However with the
changes of the next commit CONFIG_SERIAL_8250_FSL might be enabled also
when the 8250 driver is a module and so more care is needed when
fsl8250_handle_irq() is to be used. The code added here is able to
handle the new situation already.
Signed-off-by: Uwe Kleine-König
---
arch/powerpc/kernel/legacy_serial.c | 14 +-
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kernel/legacy_serial.c
b/arch/powerpc/kernel/legacy_serial.c
index c9ad12461d44..fdbd85aafeb1 100644
--- a/arch/powerpc/kernel/legacy_serial.c
+++ b/arch/powerpc/kernel/legacy_serial.c
@@ -508,12 +508,16 @@ static void __init fixup_port_irq(int index,
port->irq = virq;
-#ifdef CONFIG_SERIAL_8250_FSL
- if (of_device_is_compatible(np, "fsl,ns16550")) {
- port->handle_irq = fsl8250_handle_irq;
- port->has_sysrq = IS_ENABLED(CONFIG_SERIAL_8250_CONSOLE);
+ if (IS_ENABLED(CONFIG_SERIAL_8250) &&
+ of_device_is_compatible(np, "fsl,ns16550")) {
+ if (IS_REACHABLE(CONFIG_SERIAL_8250)) {
+ port->handle_irq = fsl8250_handle_irq;
+ port->has_sysrq =
IS_ENABLED(CONFIG_SERIAL_8250_CONSOLE);
+ } else {
+ pr_warn_once("Not activating Freescale specific
workaround for device %pOFP\n",
+np);
+ }
}
-#endif
}
static void __init fixup_port_pio(int index,
--
2.39.2
Re: [tty:tty-testing 19/19] legacy_serial.c:undefined reference to `fsl8250_handle_irq'
Hello,
On Fri, Jun 02, 2023 at 10:27:52AM +0800, kernel test robot wrote:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git
> tty-testing
> head: 3a3d09a9ee0ef5b417d6bdf8486a4da2bef06dc3
> commit: 3a3d09a9ee0ef5b417d6bdf8486a4da2bef06dc3 [19/19] serial: 8250: Apply
> FSL workarounds also without SERIAL_8250_CONSOLE
> config: powerpc-allmodconfig
> (https://download.01.org/0day-ci/archive/20230602/202306021041.qbrzzene-...@intel.com/config)
> compiler: powerpc-linux-gcc (GCC) 12.3.0
> reproduce (this is a W=1 build):
> mkdir -p ~/bin
> wget
> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
> ~/bin/make.cross
> chmod +x ~/bin/make.cross
> #
> https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git/commit/?id=3a3d09a9ee0ef5b417d6bdf8486a4da2bef06dc3
> git remote add tty
> https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git
> git fetch --no-tags tty tty-testing
> git checkout 3a3d09a9ee0ef5b417d6bdf8486a4da2bef06dc3
For the new readers, this is about
https://lore.kernel.org/r/20230531083230.2702181-1-u.kleine-koe...@pengutronix.de
Greg already dropped it from his tty-testing tree. Thanks and sorry for
this second breakage. :-\
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.3.0 ~/bin/make.cross
> W=1 O=build_dir ARCH=powerpc olddefconfig
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.3.0 ~/bin/make.cross
> W=1 O=build_dir ARCH=powerpc SHELL=/bin/bash
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot
> | Closes:
> https://lore.kernel.org/oe-kbuild-all/202306021041.qbrzzene-...@intel.com/
>
> All errors (new ones prefixed by >>):
>
>powerpc-linux-ld: arch/powerpc/kernel/legacy_serial.o: in function
> `serial_dev_init':
> >> legacy_serial.c:(.init.text+0x46a): undefined reference to
> >> `fsl8250_handle_irq'
> >> powerpc-linux-ld: legacy_serial.c:(.init.text+0x472): undefined reference
> >> to `fsl8250_handle_irq'
Urgs, this is ugly. Arch code uses a function from the 8250 driver
introduced in commit 9deaa53ac7fa ("serial: add irq handler for
Freescale 16550 errata.").
So the problematic case is SERIAL_8250=m which results in
fsl8250_handle_irq being in a module.
diff --git a/arch/powerpc/kernel/legacy_serial.c
b/arch/powerpc/kernel/legacy_serial.c
index c9ad12461d44..ad9f15902abb 100644
--- a/arch/powerpc/kernel/legacy_serial.c
+++ b/arch/powerpc/kernel/legacy_serial.c
@@ -510,8 +510,12 @@ static void __init fixup_port_irq(int index,
#ifdef CONFIG_SERIAL_8250_FSL
if (of_device_is_compatible(np, "fsl,ns16550")) {
- port->handle_irq = fsl8250_handle_irq;
- port->has_sysrq = IS_ENABLED(CONFIG_SERIAL_8250_CONSOLE);
+ if (IS_REACHABLE(CONFIG_SERIAL_8250)) {
+ port->handle_irq = fsl8250_handle_irq;
+ port->has_sysrq =
IS_ENABLED(CONFIG_SERIAL_8250_CONSOLE);
+ } else {
+ pr_warn("Not activating fsl workarounds for 8250 port
%d\n", index);
+ }
}
#endif
}
would work. The warning would trigger in cases where before the port
just silently used the default irq handler and so the FSL bug isn't
workarounded[1]. If the warning isn't wanted, it could be simplified to:
#if IS_REACHABLE(CONFIG_SERIAL_8250)
if (of_device_is_compatible(np, "fsl,ns16550")) {
...
}
#endif
But I wonder if in the presence of
if (IS_ENABLED(CONFIG_SERIAL_8250_FSL) &&
(of_device_is_compatible(np, "fsl,ns16550") ||
of_device_is_compatible(np, "fsl,16550-FIFO64"))) {
port->handle_irq = fsl8250_handle_irq;
port->has_sysrq = IS_ENABLED(CONFIG_SERIAL_8250_CONSOLE);
}
in of_platform_serial_setup() the code in
arch/powerpc/kernel/legacy_serial.c can just be dropped instead?!
Best regards
Uwe
[1] Of course this won't happen because the help text of SERIAL_8250
clearly indicates that =m isn't a safe choice in the presence of
"non-standard serial ports". So the issue is purly theoretic.
--
Pengutronix e.K. | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |
signature.asc
Description: PGP signature
Re: [PATCH 00/89] i2c: Convert to platform remove callback returning void
[Dropped Phil Edworthy from recipents as his email address has problems]
Hello,
On Mon, May 08, 2023 at 10:51:37PM +0200, Uwe Kleine-König wrote:
> this series convers the drivers below drivers/i2c to the .remove_new()
> callback of struct platform_driver(). The motivation is to make the
> remove callback less prone for errors and wrong assumptions. See commit
> 5c5a7680e67b ("platform: Provide a remove callback that returns no
> value") for a more detailed rationale.
>
> All but one driver already returned zero unconditionally in their
> .remove() callback, so converting them to .remove_new() is trivial.
> i2c-davinci has two patches in this series, first the error path is
> improved to not return an error code, then it's converted as the others
> drivers are.
>
> The two davinci patches are also the only interdependency in this
> series. I was unsure if I should split the series in two, the busses and
> the mux changes; if convenient these can be applied independent of each
> other.
I wonder how this series will go in. My expectation was that Wolfram
picks up the whole series via his tree?!
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |
signature.asc
Description: PGP signature
Re: [PATCH 0/6] bus: fsl-mc: Make remove function return void
Hello, On Mon, May 08, 2023 at 04:57:00PM -0500, Li Yang wrote: > On Mon, May 8, 2023 at 8:44 AM Uwe Kleine-König > wrote: > > On Thu, Apr 13, 2023 at 08:00:04AM +0200, Uwe Kleine-König wrote: > > > On Wed, Apr 12, 2023 at 09:30:05PM +, Leo Li wrote: > > > > > On Fri, Mar 10, 2023 at 11:41:22PM +0100, Uwe Kleine-König wrote: > > > > > > Hello, > > > > > > > > > > > > many bus remove functions return an integer which is a historic > > > > > > misdesign that makes driver authors assume that there is some kind > > > > > > of > > > > > > error handling in the upper layers. This is wrong however and > > > > > > returning and error code only yields an error message. > > > > > > > > > > > > This series improves the fsl-mc bus by changing the remove callback > > > > > > to > > > > > > return no value instead. As a preparation all drivers are changed to > > > > > > return zero before so that they don't trigger the error message. > > > > > > > > > > Who is supposed to pick up this patch series (or point out a good > > > > > reason for > > > > > not taking it)? > > > > > > > > Previously Greg KH picked up MC bus patches. > > > > > > > > If no one is picking up them this time, I probably can take it through > > > > the fsl soc tree. > > > > > > I guess Greg won't pick up this series as he didn't get a copy of it :-) > > > > > > Browsing through the history of drivers/bus/fsl-mc there is no > > > consistent maintainer to see. So if you can take it, that's very > > > appreciated. > > > > My mail was meant encouraging, maybe it was too subtile? I'll try again: > > > > Yes, please apply, that would be wonderful! > > Sorry for missing your previous email. I will do that. Thanks. Either you didn't apply this patch set yet, or your tree isn't in next. Both variants would be great to be fixed. I have another change pending for drivers/bus/fsl-mc/fsl-mc-bus.c, would be great to see these base patches in next first. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
[PATCH] powerpc: Switch i2c drivers back to use .probe()
After commit b8a1a4cd5a98 ("i2c: Provide a temporary .probe_new()
call-back type"), all drivers being converted to .probe_new() and then
03c835f498b5 ("i2c: Switch .probe() to not take an id parameter")
convert back to (the new) .probe() to be able to eventually drop
.probe_new() from struct i2c_driver.
Signed-off-by: Uwe Kleine-König
---
arch/powerpc/platforms/44x/ppc476.c| 2 +-
arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/44x/ppc476.c
b/arch/powerpc/platforms/44x/ppc476.c
index fbc6edad481f..164cbcd4588e 100644
--- a/arch/powerpc/platforms/44x/ppc476.c
+++ b/arch/powerpc/platforms/44x/ppc476.c
@@ -103,7 +103,7 @@ static struct i2c_driver avr_driver = {
.driver = {
.name = "akebono-avr",
},
- .probe_new = avr_probe,
+ .probe = avr_probe,
.id_table = avr_id,
};
diff --git a/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
b/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
index 77ed61306a73..4d8fa9ed1a67 100644
--- a/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
+++ b/arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c
@@ -211,7 +211,7 @@ static struct i2c_driver mcu_driver = {
.name = "mcu-mpc8349emitx",
.of_match_table = mcu_of_match_table,
},
- .probe_new = mcu_probe,
+ .probe = mcu_probe,
.remove = mcu_remove,
.id_table = mcu_ids,
};
base-commit: ac9a78681b921877518763ba0e89202254349d1b
--
2.39.2
[PATCH] sound: Switch i2c drivers back to use .probe()
After commit b8a1a4cd5a98 ("i2c: Provide a temporary .probe_new()
call-back type"), all drivers being converted to .probe_new() and then
03c835f498b5 ("i2c: Switch .probe() to not take an id parameter") convert
back to (the new) .probe() to be able to eventually drop .probe_new() from
struct i2c_driver.
Signed-off-by: Uwe Kleine-König
---
sound/aoa/codecs/onyx.c | 2 +-
sound/aoa/codecs/tas.c | 2 +-
sound/pci/hda/cs35l41_hda_i2c.c | 2 +-
sound/ppc/keywest.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/sound/aoa/codecs/onyx.c b/sound/aoa/codecs/onyx.c
index 4c75381f5ab8..a8a59d71dcec 100644
--- a/sound/aoa/codecs/onyx.c
+++ b/sound/aoa/codecs/onyx.c
@@ -1048,7 +1048,7 @@ static struct i2c_driver onyx_driver = {
.driver = {
.name = "aoa_codec_onyx",
},
- .probe_new = onyx_i2c_probe,
+ .probe = onyx_i2c_probe,
.remove = onyx_i2c_remove,
.id_table = onyx_i2c_id,
};
diff --git a/sound/aoa/codecs/tas.c b/sound/aoa/codecs/tas.c
index f906e9aaddcf..ab1472390061 100644
--- a/sound/aoa/codecs/tas.c
+++ b/sound/aoa/codecs/tas.c
@@ -936,7 +936,7 @@ static struct i2c_driver tas_driver = {
.driver = {
.name = "aoa_codec_tas",
},
- .probe_new = tas_i2c_probe,
+ .probe = tas_i2c_probe,
.remove = tas_i2c_remove,
.id_table = tas_i2c_id,
};
diff --git a/sound/pci/hda/cs35l41_hda_i2c.c b/sound/pci/hda/cs35l41_hda_i2c.c
index 7826b1a12d7d..b44536fbba17 100644
--- a/sound/pci/hda/cs35l41_hda_i2c.c
+++ b/sound/pci/hda/cs35l41_hda_i2c.c
@@ -58,7 +58,7 @@ static struct i2c_driver cs35l41_i2c_driver = {
.pm = _hda_pm_ops,
},
.id_table = cs35l41_hda_i2c_id,
- .probe_new = cs35l41_hda_i2c_probe,
+ .probe = cs35l41_hda_i2c_probe,
.remove = cs35l41_hda_i2c_remove,
};
module_i2c_driver(cs35l41_i2c_driver);
diff --git a/sound/ppc/keywest.c b/sound/ppc/keywest.c
index 0c4f43963c75..dfc1fc9b701d 100644
--- a/sound/ppc/keywest.c
+++ b/sound/ppc/keywest.c
@@ -90,7 +90,7 @@ static struct i2c_driver keywest_driver = {
.driver = {
.name = "PMac Keywest Audio",
},
- .probe_new = keywest_probe,
+ .probe = keywest_probe,
.remove = keywest_remove,
.id_table = keywest_i2c_id,
};
base-commit: ac9a78681b921877518763ba0e89202254349d1b
--
2.39.2
[PATCH] macintosh: Switch i2c drivers back to use .probe()
After commit b8a1a4cd5a98 ("i2c: Provide a temporary .probe_new()
call-back type"), all drivers being converted to .probe_new() and then
03c835f498b5 ("i2c: Switch .probe() to not take an id parameter") convert
back to (the new) .probe() to be able to eventually drop .probe_new() from
struct i2c_driver.
Signed-off-by: Uwe Kleine-König
---
Hello,
this patch was generated using coccinelle, but I aligned the result to
the per-file indention.
I chose to convert all drivers below drivers/macintosh in a single
patch, but if you prefer I can split by driver.
v6.4-rc1 was taken as a base, as there are no commits in next touching
drivers/macintosh I don't expect problems when applying this patch. If
conflicts arise until this is applied, feel free to just drop the files
with conflicts from this patch. I'll care about the fallout later then.
Also note there is no coordination necessary with the i2c tree. Dropping
.probe_new() will happen only when all (or most) drivers are converted,
which will happen after v6.5-rc1 for sure.
Best regards
Uwe
drivers/macintosh/ams/ams-i2c.c | 2 +-
drivers/macintosh/therm_adt746x.c | 2 +-
drivers/macintosh/therm_windtunnel.c| 2 +-
drivers/macintosh/windfarm_ad7417_sensor.c | 2 +-
drivers/macintosh/windfarm_fcu_controls.c | 2 +-
drivers/macintosh/windfarm_lm75_sensor.c| 2 +-
drivers/macintosh/windfarm_lm87_sensor.c| 2 +-
drivers/macintosh/windfarm_max6690_sensor.c | 2 +-
drivers/macintosh/windfarm_smu_sat.c| 2 +-
9 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/macintosh/ams/ams-i2c.c b/drivers/macintosh/ams/ams-i2c.c
index a4a1035eb412..f9bfe84b1c73 100644
--- a/drivers/macintosh/ams/ams-i2c.c
+++ b/drivers/macintosh/ams/ams-i2c.c
@@ -69,7 +69,7 @@ static struct i2c_driver ams_i2c_driver = {
.driver = {
.name = "ams",
},
- .probe_new = ams_i2c_probe,
+ .probe = ams_i2c_probe,
.remove = ams_i2c_remove,
.id_table = ams_id,
};
diff --git a/drivers/macintosh/therm_adt746x.c
b/drivers/macintosh/therm_adt746x.c
index 384b87d661e1..53ea56b286f9 100644
--- a/drivers/macintosh/therm_adt746x.c
+++ b/drivers/macintosh/therm_adt746x.c
@@ -598,7 +598,7 @@ static struct i2c_driver thermostat_driver = {
.driver = {
.name = "therm_adt746x",
},
- .probe_new = probe_thermostat,
+ .probe = probe_thermostat,
.remove = remove_thermostat,
.id_table = therm_adt746x_id,
};
diff --git a/drivers/macintosh/therm_windtunnel.c
b/drivers/macintosh/therm_windtunnel.c
index 22b15efcc025..18a982454321 100644
--- a/drivers/macintosh/therm_windtunnel.c
+++ b/drivers/macintosh/therm_windtunnel.c
@@ -442,7 +442,7 @@ static struct i2c_driver g4fan_driver = {
.driver = {
.name = "therm_windtunnel",
},
- .probe_new = do_probe,
+ .probe = do_probe,
.remove = do_remove,
.id_table = therm_windtunnel_id,
};
diff --git a/drivers/macintosh/windfarm_ad7417_sensor.c
b/drivers/macintosh/windfarm_ad7417_sensor.c
index 33b4723d235e..49ce37fde930 100644
--- a/drivers/macintosh/windfarm_ad7417_sensor.c
+++ b/drivers/macintosh/windfarm_ad7417_sensor.c
@@ -320,7 +320,7 @@ static struct i2c_driver wf_ad7417_driver = {
.name = "wf_ad7417",
.of_match_table = wf_ad7417_of_id,
},
- .probe_new = wf_ad7417_probe,
+ .probe = wf_ad7417_probe,
.remove = wf_ad7417_remove,
.id_table = wf_ad7417_id,
};
diff --git a/drivers/macintosh/windfarm_fcu_controls.c
b/drivers/macintosh/windfarm_fcu_controls.c
index e027d889d7e8..603ef6c600ba 100644
--- a/drivers/macintosh/windfarm_fcu_controls.c
+++ b/drivers/macintosh/windfarm_fcu_controls.c
@@ -589,7 +589,7 @@ static struct i2c_driver wf_fcu_driver = {
.name = "wf_fcu",
.of_match_table = wf_fcu_of_id,
},
- .probe_new = wf_fcu_probe,
+ .probe = wf_fcu_probe,
.remove = wf_fcu_remove,
.id_table = wf_fcu_id,
};
diff --git a/drivers/macintosh/windfarm_lm75_sensor.c
b/drivers/macintosh/windfarm_lm75_sensor.c
index 9c6febce2376..48dbdb2bda15 100644
--- a/drivers/macintosh/windfarm_lm75_sensor.c
+++ b/drivers/macintosh/windfarm_lm75_sensor.c
@@ -177,7 +177,7 @@ static struct i2c_driver wf_lm75_driver = {
.name = "wf_lm75",
.of_match_table = wf_lm75_of_id,
},
- .probe_new = wf_lm75_probe,
+ .probe = wf_lm75_probe,
.remove = wf_lm75_remove,
.id_table = wf_lm75_id,
};
diff --git a/drivers/macintosh/windfarm_lm87_sensor.c
b/drivers/macintosh/windfarm_lm87_sensor.c
index f37a32c2070c..975361c23a
[PATCH 00/97] usb: Convert to platform remove callback returning void
Hello,
this series convers the drivers below drivers/usb to the .remove_new()
callback of struct platform_driver(). The motivation is to make the
remove callback less prone for errors and wrong assumptions. See commit
5c5a7680e67b ("platform: Provide a remove callback that returns no
value") for a more detailed rationale.
All drivers converted here already returned zero unconditionally in their
.remove() callback, so converting them to .remove_new() is trivial.
Best regards
Uwe
Uwe Kleine-König (97):
usb: c67x00-drv: Convert to platform remove callback returning void
usb: cdns3-imx: Convert to platform remove callback returning void
usb: cdns3-plat: Convert to platform remove callback returning void
usb: cdns3-ti: Convert to platform remove callback returning void
usb: chipidea/ci_hdrc_imx: Convert to platform remove callback
returning void
usb: chipidea/ci_hdrc_msm: Convert to platform remove callback
returning void
usb: chipidea/ci_hdrc_tegra: Convert to platform remove callback
returning void
usb: chipidea/ci_hdrc_usb2: Convert to platform remove callback
returning void
usb: chipidea/core: Convert to platform remove callback returning void
usb: common: usb-conn-gpio: Convert to platform remove callback
returning void
usb: dwc2/platform: Convert to platform remove callback returning void
usb: core: Convert to platform remove callback returning void
usb: dwc3-am62: Convert to platform remove callback returning void
usb: dwc3-exynos: Convert to platform remove callback returning void
usb: dwc3-imx8mp: Convert to platform remove callback returning void
usb: dwc3-keystone: Convert to platform remove callback returning void
usb: dwc3-meson-g12a: Convert to platform remove callback returning
void
usb: dwc3-of-simple: Convert to platform remove callback returning
void
usb: dwc3-omap: Convert to platform remove callback returning void
usb: dwc3-qcom: Convert to platform remove callback returning void
usb: dwc3-st: Convert to platform remove callback returning void
usb: dwc3-xilinx: Convert to platform remove callback returning void
usb: fotg210: Convert to platform remove callback returning void
usb: gadget: hid: Convert to platform remove callback returning void
usb: gadget: aspeed: Convert to platform remove callback returning
void
usb: gadget/atmel_usba_udc: Convert to platform remove callback
returning void
usb: gadget/bcm63xx_udc: Convert to platform remove callback returning
void
usb: bdc: Convert to platform remove callback returning void
usb: gadget/dummy_hcd: Convert to platform remove callback returning
void
usb: gadget/fsl_qe_udc: Convert to platform remove callback returning
void
usb: gadget/fusb300_udc: Convert to platform remove callback returning
void
usb: gadget/m66592-udc: Convert to platform remove callback returning
void
usb: gadget/mv_u3d_core: Convert to platform remove callback returning
void
usb: gadget/mv_udc_core: Convert to platform remove callback returning
void
usb: gadget/net2272: Convert to platform remove callback returning
void
usb: gadget/omap_udc: Convert to platform remove callback returning
void
usb: gadget/pxa27x_udc: Convert to platform remove callback returning
void
usb: gadget/r8a66597-udc: Convert to platform remove callback
returning void
usb: gadget/renesas_usb3: Convert to platform remove callback
returning void
usb: gadget/renesas_usbf: Convert to platform remove callback
returning void
usb: gadget/rzv2m_usb3drd: Convert to platform remove callback
returning void
usb: gadget/snps_udc_plat: Convert to platform remove callback
returning void
usb: gadget/tegra-xudc: Convert to platform remove callback returning
void
usb: gadget/udc-xilinx: Convert to platform remove callback returning
void
usb: ehci-atmel: Convert to platform remove callback returning void
usb: ehci-brcm: Convert to platform remove callback returning void
usb: ehci-exynos: Convert to platform remove callback returning void
usb: ehci-fsl: Convert to platform remove callback returning void
usb: ehci-grlib: Convert to platform remove callback returning void
usb: ehci-mv: Convert to platform remove callback returning void
usb: ehci-npcm7xx: Convert to platform remove callback returning void
usb: ehci-omap: Convert to platform remove callback returning void
usb: ehci-orion: Convert to platform remove callback returning void
usb: ehci-platform: Convert to platform remove callback returning void
usb: ehci-ppc-of: Convert to platform remove callback returning void
usb: ehci-sh: Convert to platform remove callback returning void
usb: ehci-spear: Convert to platform remove callback returning void
usb: ehci-st: Convert to platform remove callback returning void
usb: ehci-xilinx-of: Convert to platform remove callback returning
void
usb: fsl-mph-dr-of: Convert to platform remov
[PATCH 30/97] usb: gadget/fsl_qe_udc: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart from
emitting a warning) and this typically results in resource leaks. To improve
here there is a quest to make the remove callback return void. In the first
step of this quest all drivers are converted to .remove_new() which already
returns void. Eventually after all drivers are converted, .remove_new() is
renamed to .remove().
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/usb/gadget/udc/fsl_qe_udc.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/udc/fsl_qe_udc.c
b/drivers/usb/gadget/udc/fsl_qe_udc.c
index 3b1cc8fa30c8..9c5dc1c1a68e 100644
--- a/drivers/usb/gadget/udc/fsl_qe_udc.c
+++ b/drivers/usb/gadget/udc/fsl_qe_udc.c
@@ -2628,7 +2628,7 @@ static int qe_udc_resume(struct platform_device *dev)
}
#endif
-static int qe_udc_remove(struct platform_device *ofdev)
+static void qe_udc_remove(struct platform_device *ofdev)
{
struct qe_udc *udc = platform_get_drvdata(ofdev);
struct qe_ep *ep;
@@ -2679,8 +2679,6 @@ static int qe_udc_remove(struct platform_device *ofdev)
/* wait for release() of gadget.dev to free udc */
wait_for_completion();
-
- return 0;
}
/*-*/
@@ -2708,7 +2706,7 @@ static struct platform_driver udc_driver = {
.of_match_table = qe_udc_match,
},
.probe = qe_udc_probe,
- .remove = qe_udc_remove,
+ .remove_new = qe_udc_remove,
#ifdef CONFIG_PM
.suspend= qe_udc_suspend,
.resume = qe_udc_resume,
--
2.39.2
Re: [PATCH] powerpc: Drop MPC5200 LocalPlus bus FIFO driver
Hello, On Thu, Apr 13, 2023 at 08:16:42AM +0200, Uwe Kleine-König wrote: > While mpc5200b.dtsi contains a device that this driver can bind to, the > only purpose of a bound device is to be used by the four exported functions > mpc52xx_lpbfifo_submit(), mpc52xx_lpbfifo_abort(), mpc52xx_lpbfifo_poll() > and mpc52xx_lpbfifo_start_xfer(). However there are no callers to this > function and so the driver is effectively superfluous and can be deleted. > Also drop some defines and a struct from that are unused > now together with the declarations of the four mentioned functions. > > Signed-off-by: Uwe Kleine-König > --- > Hello Michael, > > On Thu, Apr 13, 2023 at 10:11:25AM +1000, Michael Ellerman wrote: > > Uwe Kleine-König writes: > > > On Wed, Dec 28, 2022 at 03:51:29PM +0100, Uwe Kleine-König wrote: > > >> The four exported functions mpc52xx_lpbfifo_submit(), > > >> mpc52xx_lpbfifo_abort(), mpc52xx_lpbfifo_poll(), and > > >> mpc52xx_lpbfifo_start_xfer() are not used. So they can be dropped and the > > >> definitions needed to call them can be moved into the driver file. > > >> > > >> Signed-off-by: Uwe Kleine-König > > > > > > I never got feedback about this driver and it has not appeared in next > > > up to now. Did it fell through the cracks? > > > > Yeah. I was hoping someone would explain what's going on with the > > driver. > > > > Presumably there are some out-of-tree drivers that use the routines > > provided by this driver? > > I googled for the function names but the only related hits were > references to this thread :-) > > > I think rather than merging this patch, which keeps the code but makes > > it completely useless, do you mind sending a patch to remove the whole > > driver? Maybe that will get someone's attention. > > fair enough, here it is. What is your thought about this patch? If you (also) think getting it into next soon after a merge window closed, around now would be a good opportunity to do so .. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
[PATCH 00/89] i2c: Convert to platform remove callback returning void
Hello,
this series convers the drivers below drivers/i2c to the .remove_new()
callback of struct platform_driver(). The motivation is to make the
remove callback less prone for errors and wrong assumptions. See commit
5c5a7680e67b ("platform: Provide a remove callback that returns no
value") for a more detailed rationale.
All but one driver already returned zero unconditionally in their
.remove() callback, so converting them to .remove_new() is trivial.
i2c-davinci has two patches in this series, first the error path is
improved to not return an error code, then it's converted as the others
drivers are.
The two davinci patches are also the only interdependency in this
series. I was unsure if I should split the series in two, the busses and
the mux changes; if convenient these can be applied independent of each
other.
Best regards
Uwe
Uwe Kleine-König (89):
i2c: altera: Convert to platform remove callback returning void
i2c: amd-mp2-plat: Convert to platform remove callback returning void
i2c: aspeed: Convert to platform remove callback returning void
i2c: at91-core: Convert to platform remove callback returning void
i2c: au1550: Convert to platform remove callback returning void
i2c: axxia: Convert to platform remove callback returning void
i2c: bcm-iproc: Convert to platform remove callback returning void
i2c: bcm-kona: Convert to platform remove callback returning void
i2c: bcm2835: Convert to platform remove callback returning void
i2c: brcmstb: Convert to platform remove callback returning void
i2c: cadence: Convert to platform remove callback returning void
i2c: cbus-gpio: Convert to platform remove callback returning void
i2c: cht-wc: Convert to platform remove callback returning void
i2c: cpm: Convert to platform remove callback returning void
i2c: cros-ec-tunnel: Convert to platform remove callback returning
void
i2c: davinci: Improve error reporting for problems during .remove()
i2c: davinci: Convert to platform remove callback returning void
i2c: designware-platdrv: Convert to platform remove callback returning
void
i2c: digicolor: Convert to platform remove callback returning void
i2c: dln2: Convert to platform remove callback returning void
i2c: emev2: Convert to platform remove callback returning void
i2c: exynos5: Convert to platform remove callback returning void
i2c: gpio: Convert to platform remove callback returning void
i2c: gxp: Convert to platform remove callback returning void
i2c: highlander: Convert to platform remove callback returning void
i2c: hix5hd2: Convert to platform remove callback returning void
i2c: ibm_iic: Convert to platform remove callback returning void
i2c: img-scb: Convert to platform remove callback returning void
i2c: imx-lpi2c: Convert to platform remove callback returning void
i2c: imx: Convert to platform remove callback returning void
i2c: iop3xx: Convert to platform remove callback returning void
i2c: isch: Convert to platform remove callback returning void
i2c: jz4780: Convert to platform remove callback returning void
i2c: kempld: Convert to platform remove callback returning void
i2c: lpc2k: Convert to platform remove callback returning void
i2c: meson: Convert to platform remove callback returning void
i2c: microchip-corei2c: Convert to platform remove callback returning
void
i2c: mlxbf: Convert to platform remove callback returning void
i2c: mlxcpld: Convert to platform remove callback returning void
i2c: mpc: Convert to platform remove callback returning void
i2c: mt65xx: Convert to platform remove callback returning void
i2c: mt7621: Convert to platform remove callback returning void
i2c: mv64xxx: Convert to platform remove callback returning void
i2c: mxs: Convert to platform remove callback returning void
i2c: npcm7xx: Convert to platform remove callback returning void
i2c: ocores: Convert to platform remove callback returning void
i2c: octeon-platdrv: Convert to platform remove callback returning
void
i2c: omap: Convert to platform remove callback returning void
i2c: opal: Convert to platform remove callback returning void
i2c: pasemi-platform: Convert to platform remove callback returning
void
i2c: pca-platform: Convert to platform remove callback returning void
i2c: pnx: Convert to platform remove callback returning void
i2c: powermac: Convert to platform remove callback returning void
i2c: pxa: Convert to platform remove callback returning void
i2c: qcom-cci: Convert to platform remove callback returning void
i2c: qcom-geni: Convert to platform remove callback returning void
i2c: qup: Convert to platform remove callback returning void
i2c: rcar: Convert to platform remove callback returning void
i2c: riic: Convert to platform remove callback returning void
i2c: rk3x: Convert to platform remove callback returning void
i2c: rzv2m: Convert to platform remove callback returning void
i2
