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

[Grant Likely <grant.likely@secretlab.ca>]

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