On Monday 23 February 2009, Mark Brown wrote:
> On Mon, Feb 23, 2009 at 12:45:44PM -0800, David Brownell wrote:
>
> > > another with a TWL4030 driver using that API
> > > and a third patch making the core do something with that data.
>
> > Best IMO to switch the last two around. Effectively
> > there'd be one patch "add new features to regulator
> > core", followed by the first of a set of "implement
> > those new features in the driver for regulator X".
>
> > And in fact that's what I've done with the two patches
> > I'll be sending in a moment.
>
> The reason I'm suggesting splitting things up the way I am is that it
> separates out the TWL4030 driver (which looks very mergable to me right
> now) from the behaviour changes. Ordering things that way makes it
> clear what the dependency is. Another way of splitting it out would be
> to remove the new API from the TWL4030, make that the first patch, then
> have further patches adding the new API and the TWL4030 code to use it.
>
> I don't see any reason why the TWL4030 regulator support needs to be
> blocked on adding the new API and it makes review easier to keep them
> separate.
I think we're talking past each other. I agree the twl4030 driver
is very mergeable right now; that's why it was submitted. You could
do so, and then apply the two patches on top ... very clear what the
dependency is, and as I understand it the result would be more or less
to your liking.
My comment was more along the lines of "avoid adding unused hooks,
just merge the create-hook and use-hook patches". Having "create"
separate from "use" is often troublesome.
> > Then what would you call constraints by/from the regulator?
>
> They're subsumed within the constraints supplied by the machine driver
> at the minute.
That is, they are not named. :)
> > I suggest updating your terminology. "machine constraints"
> > would be much more clear for what's there now: they relate
> > to the machine. Other constraints (regulator, consumer)
> > relate to the other components ... the ones for which they
> > are an adjective.
>
> Yeah, I kind of agree. To avoid confusion from changing the names I'd
> be tempted to go for something like "regulator driver constraints" but
> it's not desparately nice.
Hence my suggestion: {regulator,machine,consumer} constraints,
going from bottom up. They aren't driver constraints; they
are hardware constraints: regulators can't supply arbitrary
voltages.
> To be honest I'm not
> 100% clear why this new feature is essential to supporting the TWL4030 -
> I can see that it could be useful but I'm not clear on what makes it
> essential for this driver.
I never said it was "essential". However it does simplify
the core driver code a lot by moving a lot of error checks
to the init code; the checks need to live someplace. You're
the one asking for them to be packaged as a new framework
feature.
- Dave
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
