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]

Message-ID: <20110718000011.GA13563@ponder.secretlab.ca>
Date:	Sun, 17 Jul 2011 18:00:11 -0600
From:	Grant Likely <grant.likely@...retlab.ca>
To:	Jean Delvare <khali@...ux-fr.org>
Cc:	Mark Brown <broonie@...nsource.wolfsonmicro.com>,
	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

On Sun, Jul 17, 2011 at 05:53:44PM +0200, Jean Delvare wrote:
> 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.

Hahaha.  Mark *did* do it that way and I suggested splitting up into
the i2c and spi directories to keep bus specific infrastructure
together.  Very well, if you feel strongly about it then I withdraw my
comment.

g.

> 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/