* Re: [PATCH 1/3] mfd/omap-usb-tll: Delete two error messages for a failed memory allocation in usbtll_omap_probe()
@ 2018-01-15 13:41 ` Ladislav Michl
0 siblings, 0 replies; 51+ messages in thread
From: Ladislav Michl @ 2018-01-15 13:41 UTC (permalink / raw)
To: SF Markus Elfring
Cc: linux-omap, Lee Jones, Tony Lindgren, LKML, kernel-janitors
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@users.sourceforge.net>
> ---
> 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
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [PATCH 1/3] mfd/omap-usb-tll: Delete two error messages for a failed memory allocation in usbtll
2018-01-15 13:41 ` [PATCH 1/3] mfd/omap-usb-tll: Delete two error messages for a failed memory allocation in usbtll_omap_probe() Ladislav Michl
(?)
@ 2018-01-15 15:34 ` Roger Quadros
-1 siblings, 0 replies; 51+ messages in thread
From: Roger Quadros @ 2018-01-15 15:34 UTC (permalink / raw)
To: Ladislav Michl, SF Markus Elfring
Cc: linux-omap, Lee Jones, Tony Lindgren, LKML, kernel-janitors
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@users.sourceforge.net>
>> ---
>> 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@vger.kernel.org
> 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
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH 1/3] mfd/omap-usb-tll: Delete two error messages for a failed memory allocation in usbtll_omap_probe()
@ 2018-01-15 15:34 ` Roger Quadros
0 siblings, 0 replies; 51+ messages in thread
From: Roger Quadros @ 2018-01-15 15:34 UTC (permalink / raw)
To: Ladislav Michl, SF Markus Elfring
Cc: linux-omap, Lee Jones, Tony Lindgren, LKML, kernel-janitors
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@users.sourceforge.net>
>> ---
>> 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@vger.kernel.org
> 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
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH 1/3] mfd/omap-usb-tll: Delete two error messages for a failed memory allocation in usbtll_omap_probe()
@ 2018-01-15 15:34 ` Roger Quadros
0 siblings, 0 replies; 51+ messages in thread
From: Roger Quadros @ 2018-01-15 15:34 UTC (permalink / raw)
To: Ladislav Michl, SF Markus Elfring
Cc: linux-omap, Lee Jones, Tony Lindgren, LKML, kernel-janitors
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@users.sourceforge.net>
>> ---
>> 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@vger.kernel.org
> 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
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 1/3] mfd/omap-usb-tll: Delete two error messages for a failed memory allocation in usbtll
2018-01-15 13:41 ` [PATCH 1/3] mfd/omap-usb-tll: Delete two error messages for a failed memory allocation in usbtll_omap_probe() Ladislav Michl
@ 2018-01-15 15:38 ` SF Markus Elfring
-1 siblings, 0 replies; 51+ messages in thread
From: SF Markus Elfring @ 2018-01-15 15:38 UTC (permalink / raw)
To: Ladislav Michl, linux-omap
Cc: Lee Jones, Tony Lindgren, LKML, kernel-janitors
>> @@ -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 …
Are you aware on the structure for a Linux allocation failure report?
Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH 1/3] mfd/omap-usb-tll: Delete two error messages for a failed memory allocation in usbtll_omap_probe()
@ 2018-01-15 15:38 ` SF Markus Elfring
0 siblings, 0 replies; 51+ messages in thread
From: SF Markus Elfring @ 2018-01-15 15:38 UTC (permalink / raw)
To: Ladislav Michl, linux-omap
Cc: Lee Jones, Tony Lindgren, LKML, kernel-janitors
>> @@ -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 …
Are you aware on the structure for a Linux allocation failure report?
Regards,
Markus
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH 1/3] mfd/omap-usb-tll: Delete two error messages for a failed memory allocation in usbtll
2018-01-15 15:38 ` [PATCH 1/3] mfd/omap-usb-tll: Delete two error messages for a failed memory allocation in usbtll_omap_probe() SF Markus Elfring
@ 2018-01-15 16:05 ` Ladislav Michl
-1 siblings, 0 replies; 51+ messages in thread
From: Ladislav Michl @ 2018-01-15 16:05 UTC (permalink / raw)
To: SF Markus Elfring
Cc: linux-omap, Lee Jones, Tony Lindgren, LKML, kernel-janitors
Marcus,
On Mon, Jan 15, 2018 at 04:38:43PM +0100, SF Markus Elfring wrote:
> >> @@ -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 …
>
> Are you aware on the structure for a Linux allocation failure report?
Just created one (not OMAP and not this driver, but that does not matter now):
------------[ cut here ]------------
WARNING: CPU: 0 PID: 1 at mm/slab_common.c:1012 kmalloc_slab+0x38/0xdc
Modules linked in:
CPU: 0 PID: 1 Comm: swapper Not tainted 4.15.0-rc7-next-20180115 #25
Hardware name: Atmel AT91SAM9
[<c0107950>] (unwind_backtrace) from [<c010565c>] (show_stack+0x10/0x14)
[<c010565c>] (show_stack) from [<c010f6cc>] (__warn+0xcc/0xe4)
[<c010f6cc>] (__warn) from [<c010f7a4>] (warn_slowpath_null+0x38/0x44)
[<c010f7a4>] (warn_slowpath_null) from [<c018ef90>] (kmalloc_slab+0x38/0xdc)
[<c018ef90>] (kmalloc_slab) from [<c01a5494>] (__kmalloc_track_caller+0xc/0xb0)
[<c01a5494>] (__kmalloc_track_caller) from [<c02fe5a8>] (devm_kmalloc+0x1c/0x58)
[<c02fe5a8>] (devm_kmalloc) from [<c03f96ec>] (max9867_i2c_probe+0x1c/0xe0)
[<c03f96ec>] (max9867_i2c_probe) from [<c038a708>] (i2c_device_probe+0x270/0x298)
[<c038a708>] (i2c_device_probe) from [<c02fb37c>] (driver_probe_device+0x2b4/0x458)
[<c02fb37c>] (driver_probe_device) from [<c02fb59c>] (__driver_attach+0x7c/0xec)
[<c02fb59c>] (__driver_attach) from [<c02f9840>] (bus_for_each_dev+0x58/0x7c)
[<c02f9840>] (bus_for_each_dev) from [<c02fa7cc>] (bus_add_driver+0x1a8/0x220)
[<c02fa7cc>] (bus_add_driver) from [<c02fbe7c>] (driver_register+0xa0/0xe0)
[<c02fbe7c>] (driver_register) from [<c038aa6c>] (i2c_register_driver+0x74/0xa0)
[<c038aa6c>] (i2c_register_driver) from [<c0102570>] (do_one_initcall+0x134/0x15c)
[<c0102570>] (do_one_initcall) from [<c0700da8>] (kernel_init_freeable+0x178/0x1b4)
[<c0700da8>] (kernel_init_freeable) from [<c050122c>] (kernel_init+0x8/0x100)
[<c050122c>] (kernel_init) from [<c01010e0>] (ret_from_fork+0x14/0x34)
Exception stack(0xc381bfb0 to 0xc381bff8)
bfa0: 00000000 00000000 00000000 00000000
bfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
---[ end trace 3c79eadf2363e939 ]---
max9867: probe of 1-0018 failed with error -12
driver was instructed to alloc insane number of bytes using devm_kzalloc in
max9867_i2c_probe.
Now, if probe function calls devm_kzalloc two times and one of them fails,
you cannot easily say which one without looking at assembly listing.
Or did I misunderstand your question?
Best regards,
ladis
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH 1/3] mfd/omap-usb-tll: Delete two error messages for a failed memory allocation in usbtll_omap_probe()
@ 2018-01-15 16:05 ` Ladislav Michl
0 siblings, 0 replies; 51+ messages in thread
From: Ladislav Michl @ 2018-01-15 16:05 UTC (permalink / raw)
To: SF Markus Elfring
Cc: linux-omap, Lee Jones, Tony Lindgren, LKML, kernel-janitors
Marcus,
On Mon, Jan 15, 2018 at 04:38:43PM +0100, SF Markus Elfring wrote:
> >> @@ -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 …
>
> Are you aware on the structure for a Linux allocation failure report?
Just created one (not OMAP and not this driver, but that does not matter now):
------------[ cut here ]------------
WARNING: CPU: 0 PID: 1 at mm/slab_common.c:1012 kmalloc_slab+0x38/0xdc
Modules linked in:
CPU: 0 PID: 1 Comm: swapper Not tainted 4.15.0-rc7-next-20180115 #25
Hardware name: Atmel AT91SAM9
[<c0107950>] (unwind_backtrace) from [<c010565c>] (show_stack+0x10/0x14)
[<c010565c>] (show_stack) from [<c010f6cc>] (__warn+0xcc/0xe4)
[<c010f6cc>] (__warn) from [<c010f7a4>] (warn_slowpath_null+0x38/0x44)
[<c010f7a4>] (warn_slowpath_null) from [<c018ef90>] (kmalloc_slab+0x38/0xdc)
[<c018ef90>] (kmalloc_slab) from [<c01a5494>] (__kmalloc_track_caller+0xc/0xb0)
[<c01a5494>] (__kmalloc_track_caller) from [<c02fe5a8>] (devm_kmalloc+0x1c/0x58)
[<c02fe5a8>] (devm_kmalloc) from [<c03f96ec>] (max9867_i2c_probe+0x1c/0xe0)
[<c03f96ec>] (max9867_i2c_probe) from [<c038a708>] (i2c_device_probe+0x270/0x298)
[<c038a708>] (i2c_device_probe) from [<c02fb37c>] (driver_probe_device+0x2b4/0x458)
[<c02fb37c>] (driver_probe_device) from [<c02fb59c>] (__driver_attach+0x7c/0xec)
[<c02fb59c>] (__driver_attach) from [<c02f9840>] (bus_for_each_dev+0x58/0x7c)
[<c02f9840>] (bus_for_each_dev) from [<c02fa7cc>] (bus_add_driver+0x1a8/0x220)
[<c02fa7cc>] (bus_add_driver) from [<c02fbe7c>] (driver_register+0xa0/0xe0)
[<c02fbe7c>] (driver_register) from [<c038aa6c>] (i2c_register_driver+0x74/0xa0)
[<c038aa6c>] (i2c_register_driver) from [<c0102570>] (do_one_initcall+0x134/0x15c)
[<c0102570>] (do_one_initcall) from [<c0700da8>] (kernel_init_freeable+0x178/0x1b4)
[<c0700da8>] (kernel_init_freeable) from [<c050122c>] (kernel_init+0x8/0x100)
[<c050122c>] (kernel_init) from [<c01010e0>] (ret_from_fork+0x14/0x34)
Exception stack(0xc381bfb0 to 0xc381bff8)
bfa0: 00000000 00000000 00000000 00000000
bfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
bfe0: 00000000 00000000 00000000 00000000 00000013 00000000
---[ end trace 3c79eadf2363e939 ]---
max9867: probe of 1-0018 failed with error -12
driver was instructed to alloc insane number of bytes using devm_kzalloc in
max9867_i2c_probe.
Now, if probe function calls devm_kzalloc two times and one of them fails,
you cannot easily say which one without looking at assembly listing.
Or did I misunderstand your question?
Best regards,
ladis
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [1/3] mfd/omap-usb-tll: Delete two error messages for a failed memory allocation in usbtll_omap_
2018-01-15 16:05 ` [PATCH 1/3] mfd/omap-usb-tll: Delete two error messages for a failed memory allocation in usbtll_omap_probe() Ladislav Michl
@ 2018-01-15 16:21 ` SF Markus Elfring
-1 siblings, 0 replies; 51+ messages in thread
From: SF Markus Elfring @ 2018-01-15 16:21 UTC (permalink / raw)
To: Ladislav Michl, linux-omap
Cc: Lee Jones, Tony Lindgren, LKML, kernel-janitors
>>>> @@ -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 …
>>
>> Are you aware on the structure for a Linux allocation failure report?
>
> Just created one (not OMAP and not this driver, but that does not matter now):
Thanks for your example.
> ---[ end trace 3c79eadf2363e939 ]---
> max9867: probe of 1-0018 failed with error -12
>
> driver was instructed to alloc insane number of bytes using devm_kzalloc in
> max9867_i2c_probe.
> Now, if probe function calls devm_kzalloc two times and one of them fails,
> you cannot easily say which one without looking at assembly listing.
Will this situation change with any other implementation for such backtraces?
> Or did I misunderstand your question?
No. - It seems that we have found a “common wavelength”.
Would it become acceptable to move the mentioned memory allocation into
an additional function implementation so that you could see a difference
from the function call stack dump already?
Regards,
Markus
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [1/3] mfd/omap-usb-tll: Delete two error messages for a failed memory allocation in usbtll_omap_probe()
@ 2018-01-15 16:21 ` SF Markus Elfring
0 siblings, 0 replies; 51+ messages in thread
From: SF Markus Elfring @ 2018-01-15 16:21 UTC (permalink / raw)
To: Ladislav Michl, linux-omap
Cc: Lee Jones, Tony Lindgren, LKML, kernel-janitors
>>>> @@ -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 …
>>
>> Are you aware on the structure for a Linux allocation failure report?
>
> Just created one (not OMAP and not this driver, but that does not matter now):
Thanks for your example.
> ---[ end trace 3c79eadf2363e939 ]---
> max9867: probe of 1-0018 failed with error -12
>
> driver was instructed to alloc insane number of bytes using devm_kzalloc in
> max9867_i2c_probe.
> Now, if probe function calls devm_kzalloc two times and one of them fails,
> you cannot easily say which one without looking at assembly listing.
Will this situation change with any other implementation for such backtraces?
> Or did I misunderstand your question?
No. - It seems that we have found a “common wavelength”.
Would it become acceptable to move the mentioned memory allocation into
an additional function implementation so that you could see a difference
from the function call stack dump already?
Regards,
Markus
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [1/3] mfd/omap-usb-tll: Delete two error messages for a failed memory allocation in usbtll_omap_
2018-01-15 16:21 ` [1/3] mfd/omap-usb-tll: Delete two error messages for a failed memory allocation in usbtll_omap_probe() SF Markus Elfring
@ 2018-01-15 16:35 ` Ladislav Michl
-1 siblings, 0 replies; 51+ messages in thread
From: Ladislav Michl @ 2018-01-15 16:35 UTC (permalink / raw)
To: SF Markus Elfring
Cc: linux-omap, Lee Jones, Tony Lindgren, LKML, kernel-janitors
On Mon, Jan 15, 2018 at 05:21:47PM +0100, SF Markus Elfring wrote:
> >>>> @@ -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 …
> >>
> >> Are you aware on the structure for a Linux allocation failure report?
> >
> > Just created one (not OMAP and not this driver, but that does not matter now):
>
> Thanks for your example.
>
> > ---[ end trace 3c79eadf2363e939 ]---
> > max9867: probe of 1-0018 failed with error -12
> >
> > driver was instructed to alloc insane number of bytes using devm_kzalloc in
> > max9867_i2c_probe.
> > Now, if probe function calls devm_kzalloc two times and one of them fails,
> > you cannot easily say which one without looking at assembly listing.
>
> Will this situation change with any other implementation for such backtraces?
How much that situation changes depends mainly on that very person who is
sending bugreport and his/her ability and willigness to eventually change
said implementation. In the other words your question (presumably) expects
a world of ideal backtraces, which is (so far) rarely the case.
Anyway, if we agree to change the way we allocate driver data here, the issue
this debate is about will no longer exist.
Best reards,
ladis
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [1/3] mfd/omap-usb-tll: Delete two error messages for a failed memory allocation in usbtll_omap_probe()
@ 2018-01-15 16:35 ` Ladislav Michl
0 siblings, 0 replies; 51+ messages in thread
From: Ladislav Michl @ 2018-01-15 16:35 UTC (permalink / raw)
To: SF Markus Elfring
Cc: linux-omap, Lee Jones, Tony Lindgren, LKML, kernel-janitors
On Mon, Jan 15, 2018 at 05:21:47PM +0100, SF Markus Elfring wrote:
> >>>> @@ -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 …
> >>
> >> Are you aware on the structure for a Linux allocation failure report?
> >
> > Just created one (not OMAP and not this driver, but that does not matter now):
>
> Thanks for your example.
>
> > ---[ end trace 3c79eadf2363e939 ]---
> > max9867: probe of 1-0018 failed with error -12
> >
> > driver was instructed to alloc insane number of bytes using devm_kzalloc in
> > max9867_i2c_probe.
> > Now, if probe function calls devm_kzalloc two times and one of them fails,
> > you cannot easily say which one without looking at assembly listing.
>
> Will this situation change with any other implementation for such backtraces?
How much that situation changes depends mainly on that very person who is
sending bugreport and his/her ability and willigness to eventually change
said implementation. In the other words your question (presumably) expects
a world of ideal backtraces, which is (so far) rarely the case.
Anyway, if we agree to change the way we allocate driver data here, the issue
this debate is about will no longer exist.
Best reards,
ladis
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [1/3] mfd/omap-usb-tll: Delete two error messages for a failed memory allocation in usbtll_omap_
2018-01-15 16:35 ` [1/3] mfd/omap-usb-tll: Delete two error messages for a failed memory allocation in usbtll_omap_probe() Ladislav Michl
@ 2018-01-15 17:06 ` SF Markus Elfring
-1 siblings, 0 replies; 51+ messages in thread
From: SF Markus Elfring @ 2018-01-15 17:06 UTC (permalink / raw)
To: Ladislav Michl, linux-omap
Cc: Lee Jones, Tony Lindgren, LKML, kernel-janitors
>>> Now, if probe function calls devm_kzalloc two times and one of them fails,
>>> you cannot easily say which one without looking at assembly listing.
>>
>> Will this situation change with any other implementation for such backtraces?
>
> How much that situation changes depends mainly on that very person who is
> sending bugreport and his/her ability and willigness to eventually change
> said implementation.
Have you got any more influence on the selection?
Which variant was applied for your example?
> In the other words your question (presumably) expects a world of
> ideal backtraces, which is (so far) rarely the case.
I assume that further software evolution will matter.
Does an article like “The ORCs are coming” (by Jonathan Corbet from 2017-07-20)
point information out which is also useful for this issue here?
https://lwn.net/Articles/728339/
> Anyway, if we agree to change the way we allocate driver data here,
> the issue this debate is about will no longer exist.
Does your update suggestion contain still any additional error messages for
memory allocation failures?
Regards,
Markus
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [1/3] mfd/omap-usb-tll: Delete two error messages for a failed memory allocation in usbtll_omap_probe()
@ 2018-01-15 17:06 ` SF Markus Elfring
0 siblings, 0 replies; 51+ messages in thread
From: SF Markus Elfring @ 2018-01-15 17:06 UTC (permalink / raw)
To: Ladislav Michl, linux-omap
Cc: Lee Jones, Tony Lindgren, LKML, kernel-janitors
>>> Now, if probe function calls devm_kzalloc two times and one of them fails,
>>> you cannot easily say which one without looking at assembly listing.
>>
>> Will this situation change with any other implementation for such backtraces?
>
> How much that situation changes depends mainly on that very person who is
> sending bugreport and his/her ability and willigness to eventually change
> said implementation.
Have you got any more influence on the selection?
Which variant was applied for your example?
> In the other words your question (presumably) expects a world of
> ideal backtraces, which is (so far) rarely the case.
I assume that further software evolution will matter.
Does an article like “The ORCs are coming” (by Jonathan Corbet from 2017-07-20)
point information out which is also useful for this issue here?
https://lwn.net/Articles/728339/
> Anyway, if we agree to change the way we allocate driver data here,
> the issue this debate is about will no longer exist.
Does your update suggestion contain still any additional error messages for
memory allocation failures?
Regards,
Markus
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [1/3] mfd/omap-usb-tll: Delete two error messages for a failed memory allocation in usbtll_omap_
2018-01-15 17:06 ` [1/3] mfd/omap-usb-tll: Delete two error messages for a failed memory allocation in usbtll_omap_probe() SF Markus Elfring
@ 2018-01-15 17:41 ` Ladislav Michl
-1 siblings, 0 replies; 51+ messages in thread
From: Ladislav Michl @ 2018-01-15 17:41 UTC (permalink / raw)
To: SF Markus Elfring
Cc: linux-omap, Lee Jones, Tony Lindgren, LKML, kernel-janitors
On Mon, Jan 15, 2018 at 06:06:20PM +0100, SF Markus Elfring wrote:
> >>> Now, if probe function calls devm_kzalloc two times and one of them fails,
> >>> you cannot easily say which one without looking at assembly listing.
> >>
> >> Will this situation change with any other implementation for such backtraces?
> >
> > How much that situation changes depends mainly on that very person who is
> > sending bugreport and his/her ability and willigness to eventually change
> > said implementation.
>
> Have you got any more influence on the selection?
?
> Which variant was applied for your example?
ARM_UNWIND
> > In the other words your question (presumably) expects a world of
> > ideal backtraces, which is (so far) rarely the case.
>
> I assume that further software evolution will matter.
>
> Does an article like “The ORCs are coming” (by Jonathan Corbet from 2017-07-20)
> point information out which is also useful for this issue here?
>
> https://lwn.net/Articles/728339/
I'm aware of this article. Please bear in mind which driver you are modifying.
Not everything is desktop or server machine with almost unlimited resources
and embedded people are rarely using latest stuff with all that bells and
whistles.
That said, you might end having only fragment of log in only one of thousands
machines in field. And saying technician he needs to use another kernel
(upgrade all machines) and wait another several weeks for bug to trigger
is no way.
So until evolution reaches ARM land, my point stands unchanged: Make it
single allocation or leave one of those two messages in place.
In practice it probably does not matter if we know which allocation
failed. We simply run out of memmory.
> > Anyway, if we agree to change the way we allocate driver data here,
> > the issue this debate is about will no longer exist.
>
> Does your update suggestion contain still any additional error messages for
> memory allocation failures?
Of course not as there will be only one memory allocation in the probe
function.
Best regards,
ladis
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [1/3] mfd/omap-usb-tll: Delete two error messages for a failed memory allocation in usbtll_omap_probe()
@ 2018-01-15 17:41 ` Ladislav Michl
0 siblings, 0 replies; 51+ messages in thread
From: Ladislav Michl @ 2018-01-15 17:41 UTC (permalink / raw)
To: SF Markus Elfring
Cc: linux-omap, Lee Jones, Tony Lindgren, LKML, kernel-janitors
On Mon, Jan 15, 2018 at 06:06:20PM +0100, SF Markus Elfring wrote:
> >>> Now, if probe function calls devm_kzalloc two times and one of them fails,
> >>> you cannot easily say which one without looking at assembly listing.
> >>
> >> Will this situation change with any other implementation for such backtraces?
> >
> > How much that situation changes depends mainly on that very person who is
> > sending bugreport and his/her ability and willigness to eventually change
> > said implementation.
>
> Have you got any more influence on the selection?
?
> Which variant was applied for your example?
ARM_UNWIND
> > In the other words your question (presumably) expects a world of
> > ideal backtraces, which is (so far) rarely the case.
>
> I assume that further software evolution will matter.
>
> Does an article like “The ORCs are coming” (by Jonathan Corbet from 2017-07-20)
> point information out which is also useful for this issue here?
>
> https://lwn.net/Articles/728339/
I'm aware of this article. Please bear in mind which driver you are modifying.
Not everything is desktop or server machine with almost unlimited resources
and embedded people are rarely using latest stuff with all that bells and
whistles.
That said, you might end having only fragment of log in only one of thousands
machines in field. And saying technician he needs to use another kernel
(upgrade all machines) and wait another several weeks for bug to trigger
is no way.
So until evolution reaches ARM land, my point stands unchanged: Make it
single allocation or leave one of those two messages in place.
In practice it probably does not matter if we know which allocation
failed. We simply run out of memmory.
> > Anyway, if we agree to change the way we allocate driver data here,
> > the issue this debate is about will no longer exist.
>
> Does your update suggestion contain still any additional error messages for
> memory allocation failures?
Of course not as there will be only one memory allocation in the probe
function.
Best regards,
ladis
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [1/3] mfd/omap-usb-tll: Delete two error messages for a failed memory allocation in usbtll_omap_
2018-01-15 17:41 ` [1/3] mfd/omap-usb-tll: Delete two error messages for a failed memory allocation in usbtll_omap_probe() Ladislav Michl
@ 2018-01-15 18:12 ` SF Markus Elfring
-1 siblings, 0 replies; 51+ messages in thread
From: SF Markus Elfring @ 2018-01-15 18:12 UTC (permalink / raw)
To: Ladislav Michl, linux-omap
Cc: Lee Jones, Tony Lindgren, LKML, kernel-janitors
> That said, you might end having only fragment of log in only one of thousands
> And saying technician he needs to use another kernel (upgrade all machines)
> and wait another several weeks for bug to trigger is no way.
This was not really my intention here.
> So until evolution reaches ARM land, my point stands unchanged:
> Make it single allocation
Did I indicate a similar design direction (for the preferred stack trace
convenience) after your constructive feedback?
> or leave one of those two messages in place.
Are there any more preferences involved?
> In practice it probably does not matter if we know which allocation
> failed. We simply run out of memmory.
Would anybody insist to know for which driver instance an allocation attempt
was performed?
>> Does your update suggestion contain still any additional error messages for
>> memory allocation failures?
>
> Of course not as there will be only one memory allocation in the probe function.
Thanks for this clarification. - So I hope that your solution approach
will be also fine. Will it supersede my proposal?
Regards,
Markus
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [1/3] mfd/omap-usb-tll: Delete two error messages for a failed memory allocation in usbtll_omap_probe()
@ 2018-01-15 18:12 ` SF Markus Elfring
0 siblings, 0 replies; 51+ messages in thread
From: SF Markus Elfring @ 2018-01-15 18:12 UTC (permalink / raw)
To: Ladislav Michl, linux-omap
Cc: Lee Jones, Tony Lindgren, LKML, kernel-janitors
> That said, you might end having only fragment of log in only one of thousands
> And saying technician he needs to use another kernel (upgrade all machines)
> and wait another several weeks for bug to trigger is no way.
This was not really my intention here.
> So until evolution reaches ARM land, my point stands unchanged:
> Make it single allocation
Did I indicate a similar design direction (for the preferred stack trace
convenience) after your constructive feedback?
> or leave one of those two messages in place.
Are there any more preferences involved?
> In practice it probably does not matter if we know which allocation
> failed. We simply run out of memmory.
Would anybody insist to know for which driver instance an allocation attempt
was performed?
>> Does your update suggestion contain still any additional error messages for
>> memory allocation failures?
>
> Of course not as there will be only one memory allocation in the probe function.
Thanks for this clarification. - So I hope that your solution approach
will be also fine. Will it supersede my proposal?
Regards,
Markus
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [1/3] mfd/omap-usb-tll: Delete two error messages for a failed memory allocation in usbtll_omap_
2018-01-15 18:12 ` [1/3] mfd/omap-usb-tll: Delete two error messages for a failed memory allocation in usbtll_omap_probe() SF Markus Elfring
@ 2018-01-15 18:30 ` Ladislav Michl
-1 siblings, 0 replies; 51+ messages in thread
From: Ladislav Michl @ 2018-01-15 18:30 UTC (permalink / raw)
To: SF Markus Elfring
Cc: linux-omap, Lee Jones, Tony Lindgren, Roger Quadros, LKML,
kernel-janitors
On Mon, Jan 15, 2018 at 07:12:37PM +0100, SF Markus Elfring wrote:
> Thanks for this clarification. - So I hope that your solution approach
> will be also fine. Will it supersede my proposal?
Who knows, perhaps it would be the best if you could judge yourself...
8<--------
Subject: [PATCH] mfd/omap-usb-tll: Allocate driver data at once
Allocating memory to store clk array together with driver
data simplifies error unwinding and allows deleting memory
allocation failure message as there is now only single point
where allocation could fail.
Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
---
drivers/mfd/omap-usb-tll.c | 57 +++++++++++++++++++---------------------------
1 file changed, 24 insertions(+), 33 deletions(-)
diff --git a/drivers/mfd/omap-usb-tll.c b/drivers/mfd/omap-usb-tll.c
index 44a5d66314c6..f327fe6d3292 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;
+ void __iomem *base;
+ int nch; /* num. of channels */
+ struct clk *ch_clk[0]; /* must be the last member */
};
/*-------------------------------------------------------------------------*/
@@ -216,53 +216,50 @@ static int usbtll_omap_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct resource *res;
struct usbtll_omap *tll;
- int ret = 0;
- int i, ver;
+ void __iomem *base;
+ int i, nch, ver;
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);
- ver = usbtll_read(tll->base, OMAP_USBTLL_REVISION);
+ ver = usbtll_read(base, OMAP_USBTLL_REVISION);
switch (ver) {
case OMAP_USBTLL_REV1:
case OMAP_USBTLL_REV4:
- tll->nch = OMAP_TLL_CHANNEL_COUNT;
+ nch = OMAP_TLL_CHANNEL_COUNT;
break;
case OMAP_USBTLL_REV2:
case OMAP_USBTLL_REV3:
- tll->nch = OMAP_REV2_TLL_CHANNEL_COUNT;
+ nch = OMAP_REV2_TLL_CHANNEL_COUNT;
break;
default:
- tll->nch = OMAP_TLL_CHANNEL_COUNT;
+ nch = OMAP_TLL_CHANNEL_COUNT;
dev_dbg(dev,
"USB TLL Rev : 0x%x not recognized, assuming %d channels\n",
- ver, tll->nch);
+ ver, nch);
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(*tll) + sizeof(tll->ch_clk[nch]),
+ GFP_KERNEL);
+ if (!tll) {
+ pm_runtime_put_sync(dev);
+ pm_runtime_disable(dev);
+ return -ENOMEM;
}
- for (i = 0; i < tll->nch; i++) {
+ tll->base = base;
+ tll->nch = nch;
+ platform_set_drvdata(pdev, tll);
+
+ for (i = 0; i < nch; i++) {
char clkname[] = "usb_tll_hs_usb_chx_clk";
snprintf(clkname, sizeof(clkname),
@@ -282,12 +279,6 @@ static int usbtll_omap_probe(struct platform_device *pdev)
spin_unlock(&tll_lock);
return 0;
-
-err_clk_alloc:
- pm_runtime_put_sync(dev);
- pm_runtime_disable(dev);
-
- return ret;
}
/**
--
2.15.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: [1/3] mfd/omap-usb-tll: Delete two error messages for a failed memory allocation in usbtll_omap_probe()
@ 2018-01-15 18:30 ` Ladislav Michl
0 siblings, 0 replies; 51+ messages in thread
From: Ladislav Michl @ 2018-01-15 18:30 UTC (permalink / raw)
To: SF Markus Elfring
Cc: linux-omap, Lee Jones, Tony Lindgren, Roger Quadros, LKML,
kernel-janitors
On Mon, Jan 15, 2018 at 07:12:37PM +0100, SF Markus Elfring wrote:
> Thanks for this clarification. - So I hope that your solution approach
> will be also fine. Will it supersede my proposal?
Who knows, perhaps it would be the best if you could judge yourself...
8<--------
Subject: [PATCH] mfd/omap-usb-tll: Allocate driver data at once
Allocating memory to store clk array together with driver
data simplifies error unwinding and allows deleting memory
allocation failure message as there is now only single point
where allocation could fail.
Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
---
drivers/mfd/omap-usb-tll.c | 57 +++++++++++++++++++---------------------------
1 file changed, 24 insertions(+), 33 deletions(-)
diff --git a/drivers/mfd/omap-usb-tll.c b/drivers/mfd/omap-usb-tll.c
index 44a5d66314c6..f327fe6d3292 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;
+ void __iomem *base;
+ int nch; /* num. of channels */
+ struct clk *ch_clk[0]; /* must be the last member */
};
/*-------------------------------------------------------------------------*/
@@ -216,53 +216,50 @@ static int usbtll_omap_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct resource *res;
struct usbtll_omap *tll;
- int ret = 0;
- int i, ver;
+ void __iomem *base;
+ int i, nch, ver;
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);
- ver = usbtll_read(tll->base, OMAP_USBTLL_REVISION);
+ ver = usbtll_read(base, OMAP_USBTLL_REVISION);
switch (ver) {
case OMAP_USBTLL_REV1:
case OMAP_USBTLL_REV4:
- tll->nch = OMAP_TLL_CHANNEL_COUNT;
+ nch = OMAP_TLL_CHANNEL_COUNT;
break;
case OMAP_USBTLL_REV2:
case OMAP_USBTLL_REV3:
- tll->nch = OMAP_REV2_TLL_CHANNEL_COUNT;
+ nch = OMAP_REV2_TLL_CHANNEL_COUNT;
break;
default:
- tll->nch = OMAP_TLL_CHANNEL_COUNT;
+ nch = OMAP_TLL_CHANNEL_COUNT;
dev_dbg(dev,
"USB TLL Rev : 0x%x not recognized, assuming %d channels\n",
- ver, tll->nch);
+ ver, nch);
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(*tll) + sizeof(tll->ch_clk[nch]),
+ GFP_KERNEL);
+ if (!tll) {
+ pm_runtime_put_sync(dev);
+ pm_runtime_disable(dev);
+ return -ENOMEM;
}
- for (i = 0; i < tll->nch; i++) {
+ tll->base = base;
+ tll->nch = nch;
+ platform_set_drvdata(pdev, tll);
+
+ for (i = 0; i < nch; i++) {
char clkname[] = "usb_tll_hs_usb_chx_clk";
snprintf(clkname, sizeof(clkname),
@@ -282,12 +279,6 @@ static int usbtll_omap_probe(struct platform_device *pdev)
spin_unlock(&tll_lock);
return 0;
-
-err_clk_alloc:
- pm_runtime_put_sync(dev);
- pm_runtime_disable(dev);
-
- return ret;
}
/**
--
2.15.1
^ permalink raw reply related [flat|nested] 51+ messages in thread* Re: mfd/omap-usb-tll: Allocate driver data at once in usbtll_omap_probe()
2018-01-15 18:30 ` [1/3] mfd/omap-usb-tll: Delete two error messages for a failed memory allocation in usbtll_omap_probe() Ladislav Michl
@ 2018-01-15 19:04 ` SF Markus Elfring
-1 siblings, 0 replies; 51+ messages in thread
From: SF Markus Elfring @ 2018-01-15 19:04 UTC (permalink / raw)
To: Ladislav Michl, linux-omap
Cc: Lee Jones, Tony Lindgren, Roger Quadros, LKML, kernel-janitors
>> So I hope that your solution approach will be also fine.
>> Will it supersede my proposal?
>
> Who knows, perhaps it would be the best if you could judge yourself...
I am also curious on how other contributors will respond to
your following update suggestion.
> 8<--------
>
> Subject: [PATCH] mfd/omap-usb-tll: Allocate driver data at once
Would it have been clearer to use this information as the subject
for this reply already?
Are you fine with that this change approach was recorded in
a possibly questionable format?
https://patchwork.kernel.org/patch/10165193/
> Allocating memory to store clk array together with driver
> data simplifies error unwinding and allows deleting memory
> allocation failure message as there is now only single point
> where allocation could fail.
* Will it matter to mention the previous software situation a bit
in such a commit description?
* Would you like to add a tag “link”?
* Are you going to “supersede” any more source code adjustments
around questionable error messages?
Regards,
Markus
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: mfd/omap-usb-tll: Allocate driver data at once in usbtll_omap_probe()
@ 2018-01-15 19:04 ` SF Markus Elfring
0 siblings, 0 replies; 51+ messages in thread
From: SF Markus Elfring @ 2018-01-15 19:04 UTC (permalink / raw)
To: Ladislav Michl, linux-omap
Cc: Lee Jones, Tony Lindgren, Roger Quadros, LKML, kernel-janitors
>> So I hope that your solution approach will be also fine.
>> Will it supersede my proposal?
>
> Who knows, perhaps it would be the best if you could judge yourself...
I am also curious on how other contributors will respond to
your following update suggestion.
> 8<--------
>
> Subject: [PATCH] mfd/omap-usb-tll: Allocate driver data at once
Would it have been clearer to use this information as the subject
for this reply already?
Are you fine with that this change approach was recorded in
a possibly questionable format?
https://patchwork.kernel.org/patch/10165193/
> Allocating memory to store clk array together with driver
> data simplifies error unwinding and allows deleting memory
> allocation failure message as there is now only single point
> where allocation could fail.
* Will it matter to mention the previous software situation a bit
in such a commit description?
* Would you like to add a tag “link”?
* Are you going to “supersede” any more source code adjustments
around questionable error messages?
Regards,
Markus
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: mfd/omap-usb-tll: Allocate driver data at once in usbtll_omap_probe()
2018-01-15 19:04 ` SF Markus Elfring
@ 2018-01-15 19:23 ` Ladislav Michl
-1 siblings, 0 replies; 51+ messages in thread
From: Ladislav Michl @ 2018-01-15 19:23 UTC (permalink / raw)
To: SF Markus Elfring
Cc: linux-omap, Lee Jones, Tony Lindgren, Roger Quadros, LKML,
kernel-janitors
On Mon, Jan 15, 2018 at 08:04:03PM +0100, SF Markus Elfring wrote:
> >> So I hope that your solution approach will be also fine.
> >> Will it supersede my proposal?
> >
> > Who knows, perhaps it would be the best if you could judge yourself...
>
> I am also curious on how other contributors will respond to
> your following update suggestion.
>
>
> > 8<--------
> >
> > Subject: [PATCH] mfd/omap-usb-tll: Allocate driver data at once
>
> Would it have been clearer to use this information as the subject
> for this reply already?
>
> Are you fine with that this change approach was recorded in
> a possibly questionable format?
> https://patchwork.kernel.org/patch/10165193/
Sure. It does not seem anyone involved cares about patchwork.
> > Allocating memory to store clk array together with driver
> > data simplifies error unwinding and allows deleting memory
> > allocation failure message as there is now only single point
> > where allocation could fail.
>
> * Will it matter to mention the previous software situation a bit
> in such a commit description?
>
> * Would you like to add a tag “link”?
Are you okay with this one?
https://lkml.org/lkml/2018/1/15/411
> * Are you going to “supersede” any more source code adjustments
> around questionable error messages?
I'm going to handle it usual way:
- wait for feedback and modify accordingly
- collect tags
- resend as v2
Also, patch contains all your changes, so you should be credited
somehow - hence the need for v2 anyway.
What about:
[marcus: simplified error unwinding, error message removal]
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Feel free to propose anything else.
Best regards,
ladis
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: mfd/omap-usb-tll: Allocate driver data at once in usbtll_omap_probe()
@ 2018-01-15 19:23 ` Ladislav Michl
0 siblings, 0 replies; 51+ messages in thread
From: Ladislav Michl @ 2018-01-15 19:23 UTC (permalink / raw)
To: SF Markus Elfring
Cc: linux-omap, Lee Jones, Tony Lindgren, Roger Quadros, LKML,
kernel-janitors
On Mon, Jan 15, 2018 at 08:04:03PM +0100, SF Markus Elfring wrote:
> >> So I hope that your solution approach will be also fine.
> >> Will it supersede my proposal?
> >
> > Who knows, perhaps it would be the best if you could judge yourself...
>
> I am also curious on how other contributors will respond to
> your following update suggestion.
>
>
> > 8<--------
> >
> > Subject: [PATCH] mfd/omap-usb-tll: Allocate driver data at once
>
> Would it have been clearer to use this information as the subject
> for this reply already?
>
> Are you fine with that this change approach was recorded in
> a possibly questionable format?
> https://patchwork.kernel.org/patch/10165193/
Sure. It does not seem anyone involved cares about patchwork.
> > Allocating memory to store clk array together with driver
> > data simplifies error unwinding and allows deleting memory
> > allocation failure message as there is now only single point
> > where allocation could fail.
>
> * Will it matter to mention the previous software situation a bit
> in such a commit description?
>
> * Would you like to add a tag “link”?
Are you okay with this one?
https://lkml.org/lkml/2018/1/15/411
> * Are you going to “supersede” any more source code adjustments
> around questionable error messages?
I'm going to handle it usual way:
- wait for feedback and modify accordingly
- collect tags
- resend as v2
Also, patch contains all your changes, so you should be credited
somehow - hence the need for v2 anyway.
What about:
[marcus: simplified error unwinding, error message removal]
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Feel free to propose anything else.
Best regards,
ladis
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: mfd/omap-usb-tll: Allocate driver data at once in usbtll_omap_probe()
2018-01-15 19:23 ` Ladislav Michl
@ 2018-01-15 19:40 ` SF Markus Elfring
-1 siblings, 0 replies; 51+ messages in thread
From: SF Markus Elfring @ 2018-01-15 19:40 UTC (permalink / raw)
To: Ladislav Michl
Cc: linux-omap, Lee Jones, Tony Lindgren, Roger Quadros, LKML,
kernel-janitors
>> * Would you like to add a tag “link”?
>
> Are you okay with this one?
> https://lkml.org/lkml/2018/1/15/411
Yes.
> - resend as v2
I was unsure about your patch handling from the previous replies.
> Also, patch contains all your changes, so you should be credited
> somehow - hence the need for v2 anyway.
Thanks.
> What about:
> [marcus: simplified error unwinding, error message removal]
^
I would prefer my name written as in the other places.
Will this software development dialogue evolve in further constructive ways?
Regards,
Markus
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: mfd/omap-usb-tll: Allocate driver data at once in usbtll_omap_probe()
@ 2018-01-15 19:40 ` SF Markus Elfring
0 siblings, 0 replies; 51+ messages in thread
From: SF Markus Elfring @ 2018-01-15 19:40 UTC (permalink / raw)
To: Ladislav Michl
Cc: linux-omap, Lee Jones, Tony Lindgren, Roger Quadros, LKML,
kernel-janitors
>> * Would you like to add a tag “link”?
>
> Are you okay with this one?
> https://lkml.org/lkml/2018/1/15/411
Yes.
> - resend as v2
I was unsure about your patch handling from the previous replies.
> Also, patch contains all your changes, so you should be credited
> somehow - hence the need for v2 anyway.
Thanks.
> What about:
> [marcus: simplified error unwinding, error message removal]
^
I would prefer my name written as in the other places.
Will this software development dialogue evolve in further constructive ways?
Regards,
Markus
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: mfd/omap-usb-tll: Allocate driver data at once in usbtll_omap_probe()
2018-01-15 18:30 ` [1/3] mfd/omap-usb-tll: Delete two error messages for a failed memory allocation in usbtll_omap_probe() Ladislav Michl
@ 2018-01-15 19:26 ` SF Markus Elfring
-1 siblings, 0 replies; 51+ messages in thread
From: SF Markus Elfring @ 2018-01-15 19:26 UTC (permalink / raw)
To: Ladislav Michl, linux-omap
Cc: Lee Jones, Tony Lindgren, Roger Quadros, LKML, kernel-janitors
> dev_dbg(dev,
> "USB TLL Rev : 0x%x not recognized, assuming %d channels\n",
> - ver, tll->nch);
> + ver, nch);
> break;
Does this format string need an other indentation when you touch this statement?
Regards,
Markus
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: mfd/omap-usb-tll: Allocate driver data at once in usbtll_omap_probe()
@ 2018-01-15 19:26 ` SF Markus Elfring
0 siblings, 0 replies; 51+ messages in thread
From: SF Markus Elfring @ 2018-01-15 19:26 UTC (permalink / raw)
To: Ladislav Michl, linux-omap
Cc: Lee Jones, Tony Lindgren, Roger Quadros, LKML, kernel-janitors
> dev_dbg(dev,
> "USB TLL Rev : 0x%x not recognized, assuming %d channels\n",
> - ver, tll->nch);
> + ver, nch);
> break;
Does this format string need an other indentation when you touch this statement?
Regards,
Markus
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: mfd/omap-usb-tll: Allocate driver data at once in usbtll_omap_probe()
2018-01-15 19:26 ` SF Markus Elfring
@ 2018-01-15 19:33 ` Ladislav Michl
-1 siblings, 0 replies; 51+ messages in thread
From: Ladislav Michl @ 2018-01-15 19:33 UTC (permalink / raw)
To: SF Markus Elfring
Cc: linux-omap, Lee Jones, Tony Lindgren, Roger Quadros, LKML,
kernel-janitors
On Mon, Jan 15, 2018 at 08:26:00PM +0100, SF Markus Elfring wrote:
> > dev_dbg(dev,
> > "USB TLL Rev : 0x%x not recognized, assuming %d channels\n",
> > - ver, tll->nch);
> > + ver, nch);
> > break;
>
> Does this format string need an other indentation when you touch this statement?
Well, lets that the chance and make it shorter as well:
default:
- tll->nch = OMAP_TLL_CHANNEL_COUNT;
- dev_dbg(dev,
- "USB TLL Rev : 0x%x not recognized, assuming %d channels\n",
- ver, tll->nch);
+ nch = OMAP_TLL_CHANNEL_COUNT;
+ dev_dbg(dev, "rev 0x%x not recognized, assuming %d channels\n",
+ ver, nch);
break;
Other proposals?
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: mfd/omap-usb-tll: Allocate driver data at once in usbtll_omap_probe()
@ 2018-01-15 19:33 ` Ladislav Michl
0 siblings, 0 replies; 51+ messages in thread
From: Ladislav Michl @ 2018-01-15 19:33 UTC (permalink / raw)
To: SF Markus Elfring
Cc: linux-omap, Lee Jones, Tony Lindgren, Roger Quadros, LKML,
kernel-janitors
On Mon, Jan 15, 2018 at 08:26:00PM +0100, SF Markus Elfring wrote:
> > dev_dbg(dev,
> > "USB TLL Rev : 0x%x not recognized, assuming %d channels\n",
> > - ver, tll->nch);
> > + ver, nch);
> > break;
>
> Does this format string need an other indentation when you touch this statement?
Well, lets that the chance and make it shorter as well:
default:
- tll->nch = OMAP_TLL_CHANNEL_COUNT;
- dev_dbg(dev,
- "USB TLL Rev : 0x%x not recognized, assuming %d channels\n",
- ver, tll->nch);
+ nch = OMAP_TLL_CHANNEL_COUNT;
+ dev_dbg(dev, "rev 0x%x not recognized, assuming %d channels\n",
+ ver, nch);
break;
Other proposals?
^ permalink raw reply [flat|nested] 51+ messages in thread