On Wed, 11 May 2011 22:57:04 +0200
Mark Brown <broonie@xxxxxxxxxxxxxxxxxxxxxxxxxxx> wrote:
> On Wed, May 11, 2011 at 10:53:37PM +0200, Antonio Ospite wrote:
>
> > OK, avoiding some duplication will be good, I agree.
>
> > I am resending a v4 with the equivalent code:
> >
> > if (host->vcc) {
> > int ret;
> >
> > if (power_mode == MMC_POWER_OFF)
> > vdd = 0;
> >
>
> This isn't really what I meant - what I meant was that all this logic
> looks like it's generic to multiple drivers. We either set a regulator
> or call a pdata callback to set power, both of which are completely
> external to the controller.
>
So you mean something like the following:
mmc_regulator_set_power(...)
{
if (host->vcc) {
...
}
if (host->pdata && host->pdata->setpower)
host->pdata->setpower(mmc_dev(host->mmc), vdd);
}
I like the idea, the only concern I have is that the mmc host drivers
can call 'vcc' and 'pdata' and 'setpower' with arbitrary names, what is
the kernel practice in cases like this where we want to use some common
code accessing driver-specific structures? I can think to three
alternatives right now:
1. Consider only code currently in mainline (names are consistently
vcc, pdata and setpower) and use a _macro_ based on that, assuming
than future patches are going to copy the var names anyways.
2. Add a proper function with all the needed parameters to abstract
the actual var names, this would be something like:
mmc_regulator_set_power(mmc, power_mode, vdd, vcc, pdata)
and yet checking for pdata->setpower can be tricky as 'setpower'
is an arbitrary name.
3. Move stuff from driver structures to subsystem structures, which I
don't think is needed in this case.
4. (The hidden lazy one) make the code equal across all the drivers
so new drivers copying it will be consistent, but still leave it
duplicate.
Regards,
Antonio
--
Antonio Ospite
http://ao2.it
PGP public key ID: 0x4553B001
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
Attachment:
pgpEEdek19ww1.pgp
Description: PGP signature
