* [PATCH 1/2] usb: bdc: Fix an error handling path in 'bdc_probe()' when no suitable DMA config is available @ 2021-08-18 19:32 Christophe JAILLET 2021-08-18 19:32 ` [PATCH 2/2] usb: bdc: Fix a resource leak in the error handling path of 'bdc_probe()' Christophe JAILLET 2021-08-20 20:01 ` [PATCH 1/2] usb: bdc: Fix an error handling path in 'bdc_probe()' when no suitable DMA config is available Florian Fainelli 0 siblings, 2 replies; 5+ messages in thread From: Christophe JAILLET @ 2021-08-18 19:32 UTC (permalink / raw) To: alcooperx, balbi, gregkh, f.fainelli Cc: linux-usb, bcm-kernel-feedback-list, linux-kernel, kernel-janitors, Christophe JAILLET If no suitable DMA configuration is available, a previous 'bdc_phy_init()' call must be undone by a corresponding 'bdc_phy_exit()' call. Branch to the existing error handling path instead of returning directly. Fixes: cc29d4f67757 ("usb: bdc: Add support for USB phy") Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> --- checkpatch.pl warns that: WARNING: ENOTSUPP is not a SUSV4 error code, prefer EOPNOTSUPP #29: FILE: drivers/usb/gadget/udc/bdc/bdc_core.c:563: + ret = -ENOTSUPP; I've never seen this warning before and don't want to make a blind fix for that. Let me know if I should fix it or not. --- drivers/usb/gadget/udc/bdc/bdc_core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/gadget/udc/bdc/bdc_core.c b/drivers/usb/gadget/udc/bdc/bdc_core.c index 0bef6b3f049b..251db57e51fa 100644 --- a/drivers/usb/gadget/udc/bdc/bdc_core.c +++ b/drivers/usb/gadget/udc/bdc/bdc_core.c @@ -560,7 +560,8 @@ static int bdc_probe(struct platform_device *pdev) if (ret) { dev_err(dev, "No suitable DMA config available, abort\n"); - return -ENOTSUPP; + ret = -ENOTSUPP; + goto phycleanup; } dev_dbg(dev, "Using 32-bit address\n"); } -- 2.30.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] usb: bdc: Fix a resource leak in the error handling path of 'bdc_probe()' 2021-08-18 19:32 [PATCH 1/2] usb: bdc: Fix an error handling path in 'bdc_probe()' when no suitable DMA config is available Christophe JAILLET @ 2021-08-18 19:32 ` Christophe JAILLET 2021-08-20 20:03 ` Florian Fainelli 2021-08-20 20:01 ` [PATCH 1/2] usb: bdc: Fix an error handling path in 'bdc_probe()' when no suitable DMA config is available Florian Fainelli 1 sibling, 1 reply; 5+ messages in thread From: Christophe JAILLET @ 2021-08-18 19:32 UTC (permalink / raw) To: alcooperx, balbi, gregkh, f.fainelli Cc: linux-usb, bcm-kernel-feedback-list, linux-kernel, kernel-janitors, Christophe JAILLET If an error occurs after a successful 'clk_prepare_enable()' call, it must be undone by a corresponding 'clk_disable_unprepare()' call. This call is already present in the remove function. Add this call in the error handling path and reorder the code so that the 'clk_prepare_enable()' call happens later in the function. The goal is to have as much managed resources functions as possible before the 'clk_prepare_enable()' call in order to keep the error handling path simple. While at it, remove the now unneeded 'clk' variable. Fixes: c87dca047849 ("usb: bdc: Add clock enable for new chips with a separate BDC clock") Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> --- Review with care. I don't like shuffling code like that because of possible side effect. Moving the code related to this clk looks fine to me, but who knows... --- drivers/usb/gadget/udc/bdc/bdc_core.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/drivers/usb/gadget/udc/bdc/bdc_core.c b/drivers/usb/gadget/udc/bdc/bdc_core.c index 251db57e51fa..fa1a3908ec3b 100644 --- a/drivers/usb/gadget/udc/bdc/bdc_core.c +++ b/drivers/usb/gadget/udc/bdc/bdc_core.c @@ -488,27 +488,14 @@ static int bdc_probe(struct platform_device *pdev) int irq; u32 temp; struct device *dev = &pdev->dev; - struct clk *clk; int phy_num; dev_dbg(dev, "%s()\n", __func__); - clk = devm_clk_get_optional(dev, "sw_usbd"); - if (IS_ERR(clk)) - return PTR_ERR(clk); - - ret = clk_prepare_enable(clk); - if (ret) { - dev_err(dev, "could not enable clock\n"); - return ret; - } - bdc = devm_kzalloc(dev, sizeof(*bdc), GFP_KERNEL); if (!bdc) return -ENOMEM; - bdc->clk = clk; - bdc->regs = devm_platform_ioremap_resource(pdev, 0); if (IS_ERR(bdc->regs)) return PTR_ERR(bdc->regs); @@ -545,10 +532,20 @@ static int bdc_probe(struct platform_device *pdev) } } + bdc->clk = devm_clk_get_optional(dev, "sw_usbd"); + if (IS_ERR(bdc->clk)) + return PTR_ERR(bdc->clk); + + ret = clk_prepare_enable(bdc->clk); + if (ret) { + dev_err(dev, "could not enable clock\n"); + return ret; + } + ret = bdc_phy_init(bdc); if (ret) { dev_err(bdc->dev, "BDC phy init failure:%d\n", ret); - return ret; + goto disable_clk; } temp = bdc_readl(bdc->regs, BDC_BDCCAP1); @@ -581,6 +578,8 @@ static int bdc_probe(struct platform_device *pdev) bdc_hw_exit(bdc); phycleanup: bdc_phy_exit(bdc); +disable_clk: + clk_disable_unprepare(bdc->clk); return ret; } -- 2.30.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] usb: bdc: Fix a resource leak in the error handling path of 'bdc_probe()' 2021-08-18 19:32 ` [PATCH 2/2] usb: bdc: Fix a resource leak in the error handling path of 'bdc_probe()' Christophe JAILLET @ 2021-08-20 20:03 ` Florian Fainelli 2021-08-31 13:42 ` Alan Cooper 0 siblings, 1 reply; 5+ messages in thread From: Florian Fainelli @ 2021-08-20 20:03 UTC (permalink / raw) To: Christophe JAILLET, alcooperx, balbi, gregkh Cc: linux-usb, bcm-kernel-feedback-list, linux-kernel, kernel-janitors On 8/18/2021 9:32 PM, Christophe JAILLET wrote: > If an error occurs after a successful 'clk_prepare_enable()' call, it must > be undone by a corresponding 'clk_disable_unprepare()' call. > This call is already present in the remove function. > > Add this call in the error handling path and reorder the code so that the > 'clk_prepare_enable()' call happens later in the function. > The goal is to have as much managed resources functions as possible > before the 'clk_prepare_enable()' call in order to keep the error handling > path simple. > > While at it, remove the now unneeded 'clk' variable. > > Fixes: c87dca047849 ("usb: bdc: Add clock enable for new chips with a separate BDC clock") > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> Acked-by: Florian Fainelli <f.fainelli@gmail.com> > --- > Review with care. > I don't like shuffling code like that because of possible side effect. > Moving the code related to this clk looks fine to me, but who knows... There are no register accesses until bdc_phy_init() gets called, so this looks fine to me. Al knows this code better than I do though, so it would be better to wait for his Acked-by tag. -- Florian ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] usb: bdc: Fix a resource leak in the error handling path of 'bdc_probe()' 2021-08-20 20:03 ` Florian Fainelli @ 2021-08-31 13:42 ` Alan Cooper 0 siblings, 0 replies; 5+ messages in thread From: Alan Cooper @ 2021-08-31 13:42 UTC (permalink / raw) To: Florian Fainelli Cc: Christophe JAILLET, Felipe Balbi, Greg Kroah-Hartman, USB list, BCM Kernel Feedback, : Linux Kernel Mailing List, kernel-janitors On Fri, Aug 20, 2021 at 4:03 PM Florian Fainelli <f.fainelli@gmail.com> wrote: > > > > On 8/18/2021 9:32 PM, Christophe JAILLET wrote: > > If an error occurs after a successful 'clk_prepare_enable()' call, it must > > be undone by a corresponding 'clk_disable_unprepare()' call. > > This call is already present in the remove function. > > > > Add this call in the error handling path and reorder the code so that the > > 'clk_prepare_enable()' call happens later in the function. > > The goal is to have as much managed resources functions as possible > > before the 'clk_prepare_enable()' call in order to keep the error handling > > path simple. > > > > While at it, remove the now unneeded 'clk' variable. > > > > Fixes: c87dca047849 ("usb: bdc: Add clock enable for new chips with a separate BDC clock") > > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > > Acked-by: Florian Fainelli <f.fainelli@gmail.com> > > > --- > > Review with care. > > I don't like shuffling code like that because of possible side effect. > > Moving the code related to this clk looks fine to me, but who knows... > > There are no register accesses until bdc_phy_init() gets called, so this > looks fine to me. Al knows this code better than I do though, so it > would be better to wait for his Acked-by tag. This is safe. Acked-by: Al Cooper <alcooperx@gmail.com> > -- > Florian ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] usb: bdc: Fix an error handling path in 'bdc_probe()' when no suitable DMA config is available 2021-08-18 19:32 [PATCH 1/2] usb: bdc: Fix an error handling path in 'bdc_probe()' when no suitable DMA config is available Christophe JAILLET 2021-08-18 19:32 ` [PATCH 2/2] usb: bdc: Fix a resource leak in the error handling path of 'bdc_probe()' Christophe JAILLET @ 2021-08-20 20:01 ` Florian Fainelli 1 sibling, 0 replies; 5+ messages in thread From: Florian Fainelli @ 2021-08-20 20:01 UTC (permalink / raw) To: Christophe JAILLET, alcooperx, balbi, gregkh Cc: linux-usb, bcm-kernel-feedback-list, linux-kernel, kernel-janitors On 8/18/2021 9:32 PM, Christophe JAILLET wrote: > If no suitable DMA configuration is available, a previous 'bdc_phy_init()' > call must be undone by a corresponding 'bdc_phy_exit()' call. > > Branch to the existing error handling path instead of returning > directly. > > Fixes: cc29d4f67757 ("usb: bdc: Add support for USB phy") > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> Acked-by: Florian Fainelli <f.fainelli@gmail.com> -- Florian ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-08-31 13:42 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-08-18 19:32 [PATCH 1/2] usb: bdc: Fix an error handling path in 'bdc_probe()' when no suitable DMA config is available Christophe JAILLET 2021-08-18 19:32 ` [PATCH 2/2] usb: bdc: Fix a resource leak in the error handling path of 'bdc_probe()' Christophe JAILLET 2021-08-20 20:03 ` Florian Fainelli 2021-08-31 13:42 ` Alan Cooper 2021-08-20 20:01 ` [PATCH 1/2] usb: bdc: Fix an error handling path in 'bdc_probe()' when no suitable DMA config is available Florian Fainelli
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).