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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 31 Jan 2012 22:15:54 +0100
From:	Wolfram Sang <w.sang@...gutronix.de>
To:	Grant Likely <grant.likely@...retlab.ca>
Cc:	Barry Song <21cnbao@...il.com>, Barry Song <Barry.Song@....com>,
	Linus Walleij <linus.walleij@...aro.org>,
	Greg Kroah-Hartman <gregkh@...e.de>,
	linux-kernel@...r.kernel.org, workgroup.linux@....com,
	Erik Gilling <konkers@...gle.com>,
	linux-mtd@...ts.infradead.org, Barry Song <Baohua.Song@....com>,
	David Woodhouse <dwmw2@...radead.org>,
	Atsushi Nemoto <anemo@....ocn.ne.jp>
Subject: Re: [PATCH 1/3] platform: add common resource requesting and mapping
 helper

> > 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.
> 
> The reason I suggested the wrapper is that then the driver code doesn't need
> to fart around with the res pointer at all.  It reduces boilerplate in platform
> drivers which I think is a good thing.

I do understand your motivation and fully agree with what you are aiming for
(that's exactly why I implemented devm_request_and_ioremap()).

This patch is a micro-optimization, though, and won't cut it IMHO. I still have
issues with only one platform_devm_* and all the rest being devm_* (without
platform_). Things might look better, if we'd for example also have
platform_devm_request_irq() or something similar. That might be an approach
where we can play around with and see what is left to do. Or, if other
approaches might be more elegant.

To discuss that, try things, etc, I'd simply like to have a bit more time. If
we are accepting the first iteration right away, and people let run their
coccinelle-scripts based on that, it might get annoying to change that a second
time, I'd think.

Regards,

   Wolfram

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

Download attachment "signature.asc" of type "application/pgp-signature" (199 bytes)