Re: [PATCH 1/3] platform: add common resource requesting and mapping helper

[Wolfram Sang <w.sang@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 1595 bytes --]

Barry,

> > You don't need to do the error checking for 'res'. You can simply do
> >
> > res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > base = devm_request_and_ioremap(&dev->dev, res);
> 
> i do know  devm_request_and_ioremap() does res checking. but that is
> implicit, confused and not a smart way actually.

I agree about the implicit thing (keep in mind the function is new). But
calling a function "not smart" because it checks its arguments? I do
like the NULL check of kfree() for example.

> actually, no people by now really use the implicit checking. that
> shows people don't really think that is a good programming way.

I'd think most people just copy&paste and don't have an opinion either
way, so the quantity doesn't show much.

> > devm_request_and_ioremap() will check res. Given that, I don't think
> > we can save a lot with another wrapper.
> 
> i think we can save some.
> The story begins from Grant's feedback in:
> http://www.spinics.net/lists/arm-kernel/msg157644.html

I am not sure using 'platform_devm_request_and_ioremap' and later using
plain 'devm_*' functions (without platform_-prefix) is less confusing.
The alternative would be to check which helper functions also use
'struct resource' and if they do checks on that. If all do that, you
would have the simple rule, that you only need to check yourself if you
access it yourself.

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]