Hi Ladislav,
On 15/01/18 15:41, Ladislav Michl wrote:
> 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];
How about putting a comment here that says ch_clk needs to be the last member of this structure?
> };
>
> /*-------------------------------------------------------------------------*/
> @@ -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;
need to declare num. Maybe better call it nch instead?
> 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);
num * sizeof(tll->ch_clk[0]) ?
> + 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.
I think it is a good approach.
>
> Best regards,
> ladis
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
cheers,
-roger
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
