On 11/20/2012 02:05 PM, AnilKumar, Chimata wrote:
[...]
>>> static struct platform_device_id c_can_id_table[] = {
>>> [BOSCH_C_CAN_PLATFORM] = {
>>> .name = KBUILD_MODNAME,
>>> @@ -99,7 +119,7 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
>>> const struct of_device_id *match;
>>> const struct platform_device_id *id;
>>> struct pinctrl *pinctrl;
>>> - struct resource *mem;
>>> + struct resource *mem, *res;
>>> int irq;
>>> struct clk *clk;
>>>
>>> @@ -178,6 +198,18 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
>>> priv->can.ctrlmode_supported |= CAN_CTRLMODE_3_SAMPLES;
>>> priv->read_reg = c_can_plat_read_reg_aligned_to_16bit;
>>> priv->write_reg = c_can_plat_write_reg_aligned_to_16bit;
>>> +
>>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>>> + priv->raminit_ctrlreg =
>>> + devm_request_and_ioremap(&pdev->dev, res);
>>
>> What happens if there isn't a second IORESOURCE_MEM region? devm_request
>> will print an error in this case and any other error, too [1]. Do we
>> need streamlining here?
>>
>> [1] http://lxr.free-electrons.com/source/drivers/base/platform.c#L59
>
> I did not get what the point is.
>
> In most of the cases above two API's returns NULL. Even res is NULL
> nothing to worry, first condition in devm_request_and_ioremap() is
> NULL pointer check of res. If "res" is NULL then devm_xx will return
> NULL which result into printing of below warning.
>
>>
>>> + if (!priv->raminit_ctrlreg)
>>> + dev_warn(&pdev->dev, "failed to obtain control memory\n");
>
> I will change this warning to info
>
> if (!priv->raminit_ctrlreg)
> dev_info(&pdev->dev, "control memory is not used for raminit\n");
That's more descriptive, good.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
Attachment:
signature.asc
Description: OpenPGP digital signature
