linux-kernel - Re: [PATCH 4/4] regulator: Convert tps65023 to use regmap API

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:	Sun, 17 Jul 2011 17:53:44 +0200
From:	Jean Delvare <khali@...ux-fr.org>
To:	Mark Brown <broonie@...nsource.wolfsonmicro.com>
Cc:	Greg KH <greg@...ah.com>, Grant Likely <grant@...retlab.ca>,
	Ben Dooks <ben-linux@...ff.org>,
	Dimitris Papastamos <dp@...nsource.wolfsonmicro.com>,
	Liam Girdwood <lrg@...com>,
	Samuel Oritz <sameo@...ux.intel.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/4] regulator: Convert tps65023 to use regmap API

Hi Mark,

On Fri, 15 Jul 2011 22:17:51 +0900, Mark Brown wrote:
> On Fri, Jul 15, 2011 at 02:58:48PM +0200, Jean Delvare wrote:
> > On Fri, 15 Jul 2011 21:16:50 +0900, Mark Brown wrote:
> 
> > > Does that sound reasonable to you?
> 
> > Yes, no objection.
> 
> BTW, if this does sound reasonable are you OK with adding your ack for
> the I2C bus interface patch or are there any updates you want me to do?

I did not review the patch carefully, so I can't ask for updates. As
none of "my" drivers will use it, I don't really feel qualified (nor
interested, honestly) to review it.

I don't quite get why you put the i2c bindings into
drivers/i2c/i2c-regmap.c. This means that Ben and I end up being the
maintainers of that file, while it's your thing. And this module is a
user of i2c, not a provider, so it doesn't really belong there anyway.
Same goes with spi. And what's the rationale for putting the regmap core
under drivers/base?

What's wrong with the more direct approach:
drivers/regmap/regmap-core.c
drivers/regmap/regmap-i2c.c
drivers/regmap/regmap-spi.c
?

At least you would have everything in one place and under your control.
With your current plan, every update is likely to spawn
cross-subsystems, which always results in delays and conflicts.

Now if you have a good reason for the current design, that's OK with
me, I can live with that. Simply it seems more complex than needed.

Also, your Kconfig setup is such that all bindings will be selected as
soon as any driver needs one. And the selection (module vs. built-in)
will be aligned on the core setting (e.g. CONFIG_I2C) rather than the
drivers which use it. I'd rather have e.g. REGULATOR_TPS65023 select
REGMAP_I2C, and in turn have REGMAP_I2C select REGMAP. This should
address the issues I pointed out.

-- 
Jean Delvare
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/