Re: [PATCH 1/3] mfd/omap-usb-tll: Delete two error messages for a failed memory allocation in usbtll_omap_probe()

From: Ladislav Michl
Date: Mon Jan 15 2018 - 08:41:10 EST


Marcus,

On Mon, Jan 15, 2018 at 02:15:11PM +0100, SF Markus Elfring wrote:
> Omit extra messages for a memory allocation failure in this function.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx>
> ---
> drivers/mfd/omap-usb-tll.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/mfd/omap-usb-tll.c b/drivers/mfd/omap-usb-tll.c
> index 44a5d66314c6..7945efa0152e 100644
> --- a/drivers/mfd/omap-usb-tll.c
> +++ b/drivers/mfd/omap-usb-tll.c
> @@ -222,10 +222,8 @@ static int usbtll_omap_probe(struct platform_device *pdev)
> dev_dbg(dev, "starting TI HSUSB TLL Controller\n");
>
> tll = devm_kzalloc(dev, sizeof(struct usbtll_omap), GFP_KERNEL);
> - if (!tll) {
> - dev_err(dev, "Memory allocation failed\n");
> + if (!tll)
> return -ENOMEM;
> - }
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> tll->base = devm_ioremap_resource(dev, res);
> @@ -258,7 +256,6 @@ static int usbtll_omap_probe(struct platform_device *pdev)
> GFP_KERNEL);
> if (!tll->ch_clk) {
> ret = -ENOMEM;
> - dev_err(dev, "Couldn't allocate memory for channel clocks\n");

I'd either leave this one, just to know which allocation failed or better use
something like this (it is pseudo patch only, just to show idea):

diff --git a/drivers/mfd/omap-usb-tll.c b/drivers/mfd/omap-usb-tll.c
index 44a5d66314c6..d217211d6b8f 100644
--- a/drivers/mfd/omap-usb-tll.c
+++ b/drivers/mfd/omap-usb-tll.c
@@ -108,9 +108,9 @@
(x) != OMAP_EHCI_PORT_MODE_PHY)

struct usbtll_omap {
- int nch; /* num. of channels */
- struct clk **ch_clk;
void __iomem *base;
+ int nch; /* num. of channels */
+ struct clk ch_clk[0];
};

/*-------------------------------------------------------------------------*/
@@ -221,18 +221,11 @@ static int usbtll_omap_probe(struct platform_device *pdev)

dev_dbg(dev, "starting TI HSUSB TLL Controller\n");

- tll = devm_kzalloc(dev, sizeof(struct usbtll_omap), GFP_KERNEL);
- if (!tll) {
- dev_err(dev, "Memory allocation failed\n");
- return -ENOMEM;
- }
-
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- tll->base = devm_ioremap_resource(dev, res);
- if (IS_ERR(tll->base))
- return PTR_ERR(tll->base);
+ base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(base))
+ return PTR_ERR(base);

- platform_set_drvdata(pdev, tll);
pm_runtime_enable(dev);
pm_runtime_get_sync(dev);

@@ -240,27 +233,27 @@ static int usbtll_omap_probe(struct platform_device *pdev)
switch (ver) {
case OMAP_USBTLL_REV1:
case OMAP_USBTLL_REV4:
- tll->nch = OMAP_TLL_CHANNEL_COUNT;
+ num = OMAP_TLL_CHANNEL_COUNT;
break;
case OMAP_USBTLL_REV2:
case OMAP_USBTLL_REV3:
- tll->nch = OMAP_REV2_TLL_CHANNEL_COUNT;
+ num = OMAP_REV2_TLL_CHANNEL_COUNT;
break;
default:
- tll->nch = OMAP_TLL_CHANNEL_COUNT;
+ num = OMAP_TLL_CHANNEL_COUNT;
dev_dbg(dev,
"USB TLL Rev : 0x%x not recognized, assuming %d channels\n",
- ver, tll->nch);
+ ver, num);
break;
}

- tll->ch_clk = devm_kzalloc(dev, sizeof(struct clk *) * tll->nch,
- GFP_KERNEL);
- if (!tll->ch_clk) {
- ret = -ENOMEM;
- dev_err(dev, "Couldn't allocate memory for channel clocks\n");
- goto err_clk_alloc;
- }
+ tll = devm_kzalloc(dev, sizeof(struct usbtll_omap) + (num * sizeof(...)), GFP_KERNEL);
+ if (!tll)
+ return -ENOMEM;
+
+ tll->nch = num;
+ tll->base = base;
+ platform_set_drvdata(pdev, tll);

for (i = 0; i < tll->nch; i++) {
char clkname[] = "usb_tll_hs_usb_chx_clk";

> goto err_clk_alloc;
> }

What do you think? I'll prepare proper patch in case there's an agreement
on above approach.

Best regards,
ladis