On Thu, Oct 28, 2010 at 02:17:41PM -0700, Rhyland Klein wrote:
[...]
> > Hm... I guess it can be, but on the other hand most platform bus chargers
> > type
> > devices can be, because the pda_power driver is keep pretty generic with
> > custom init,
> > status and exit callbacks.
> >
> > - Lars
>
> I didn't see any more discussion on this. Is the plan to integrate
> the gpio-charger driver as is or to instead try to integrate support
> for this into pda_power?
Sorry for the delayed response, and thanks for the pings! ;-)
The main thing I'm afraid of is duplication. I.e. someday you
will want debouncing (include/linux/pda_power.h's wait_for_status,
wait_for_charger parameters) support, regulators support etc.
And your gpio driver will look very similar to pda_power.
So I'd vote for adding the GPIO functionality to pda_power, and
refactoring it if needed.
Though, if there are strong objections against this idea, I
can merge the GPIO driver, and let's see how things will evolve.
Thanks!
--
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2
> From: Anton Vorontsov [mailto:[email protected]]
> Sent: Thursday, November 04, 2010 10:48 AM
> To: Rhyland Klein
> Cc: Lars-Peter Clausen; [email protected]; Andrew Chew;
> [email protected]; [email protected]
> Subject: Re: [PATCH v2] POWER: Add gpio charger driver
>
> On Thu, Oct 28, 2010 at 02:17:41PM -0700, Rhyland Klein wrote:
> [...]
> > > Hm... I guess it can be, but on the other hand most platform bus
> chargers
> > > type
> > > devices can be, because the pda_power driver is keep pretty generic
> with
> > > custom init,
> > > status and exit callbacks.
> > >
> > > - Lars
> >
> > I didn't see any more discussion on this. Is the plan to integrate
> > the gpio-charger driver as is or to instead try to integrate support
> > for this into pda_power?
>
> Sorry for the delayed response, and thanks for the pings! ;-)
>
> The main thing I'm afraid of is duplication. I.e. someday you
> will want debouncing (include/linux/pda_power.h's wait_for_status,
> wait_for_charger parameters) support, regulators support etc.
>
> And your gpio driver will look very similar to pda_power.
>
> So I'd vote for adding the GPIO functionality to pda_power, and
> refactoring it if needed.
>
> Though, if there are strong objections against this idea, I
> can merge the GPIO driver, and let's see how things will evolve.
>
> Thanks!
>
> --
> Anton Vorontsov
> email: [email protected]
> irc://irc.freenode.net/bd2
My guess, is that since no one has responded, no one is working on integrating the functionality into pda-power?
-rhyland
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
Rhyland Klein wrote:
>> From: Anton Vorontsov [mailto:[email protected]]
>> Sent: Thursday, November 04, 2010 10:48 AM
>> To: Rhyland Klein
>> Cc: Lars-Peter Clausen; [email protected]; Andrew Chew;
>> [email protected]; [email protected]
>> Subject: Re: [PATCH v2] POWER: Add gpio charger driver
>>
>> On Thu, Oct 28, 2010 at 02:17:41PM -0700, Rhyland Klein wrote:
>> [...]
>>>> Hm... I guess it can be, but on the other hand most platform bus
>> chargers
>>>> type
>>>> devices can be, because the pda_power driver is keep pretty generic
>> with
>>>> custom init,
>>>> status and exit callbacks.
>>>>
>>>> - Lars
>>> I didn't see any more discussion on this. Is the plan to integrate
>>> the gpio-charger driver as is or to instead try to integrate support
>>> for this into pda_power?
>> Sorry for the delayed response, and thanks for the pings! ;-)
>>
>> The main thing I'm afraid of is duplication. I.e. someday you
>> will want debouncing (include/linux/pda_power.h's wait_for_status,
>> wait_for_charger parameters) support, regulators support etc.
>>
>> And your gpio driver will look very similar to pda_power.
>>
>> So I'd vote for adding the GPIO functionality to pda_power, and
>> refactoring it if needed.
>>
>> Though, if there are strong objections against this idea, I
>> can merge the GPIO driver, and let's see how things will evolve.
>>
>> Thanks!
>>
>> --
>> Anton Vorontsov
>> email: [email protected]
>> irc://irc.freenode.net/bd2
>
> My guess, is that since no one has responded, no one is working on integrating the functionality into pda-power?
>
> -rhyland
hi
Well I'm still not convinced that both drivers should be merged, yet I'm not totally
opposed to it. In my opinion we are in some kind of dilemma here.
I can see Antons point regarding to introducing code duplication, but on the other
hand you'll always have code duplication amongst similar drivers. This will
especially stand out if the setup functions take up a relatively large part of the
whole code size.
One way to avoid this is by putting everything into one driver which handles all
different cases. But the next time yet another sort of similar driver comes along
another if-else-branch is added to the pda-power driver. And slowly over time things
will get messy.
Another option to solve the problem is to add another level of indirection. For
example in form of a simple_charger driver which will take callbacks for add, remove
and status. The gpio-charger and pda-power could then instantiate such a driver and
provide their callbacks. But by adding more and more levels of indirection things
will slowly get messy as well.
One solution that might could work is to provide library functions which aim at
providing aid for implementing (simple) charger drivers.
- Lars
> From: Lars-Peter Clausen [mailto:[email protected]]
> Sent: Wednesday, November 10, 2010 5:34 PM
> To: Rhyland Klein
> Cc: Anton Vorontsov; [email protected]; Andrew Chew;
> [email protected]; [email protected]
> Subject: Re: [PATCH v2] POWER: Add gpio charger driver
>
> Rhyland Klein wrote:
> >> From: Anton Vorontsov [mailto:[email protected]]
> >> Sent: Thursday, November 04, 2010 10:48 AM
> >> To: Rhyland Klein
> >> Cc: Lars-Peter Clausen; [email protected]; Andrew
> Chew;
> >> [email protected]; [email protected]
> >> Subject: Re: [PATCH v2] POWER: Add gpio charger driver
> >>
> >> On Thu, Oct 28, 2010 at 02:17:41PM -0700, Rhyland Klein wrote:
> >> [...]
> >>>> Hm... I guess it can be, but on the other hand most platform bus
> >> chargers
> >>>> type
> >>>> devices can be, because the pda_power driver is keep pretty generic
> >> with
> >>>> custom init,
> >>>> status and exit callbacks.
> >>>>
> >>>> - Lars
> >>> I didn't see any more discussion on this. Is the plan to integrate
> >>> the gpio-charger driver as is or to instead try to integrate support
> >>> for this into pda_power?
> >> Sorry for the delayed response, and thanks for the pings! ;-)
> >>
> >> The main thing I'm afraid of is duplication. I.e. someday you
> >> will want debouncing (include/linux/pda_power.h's wait_for_status,
> >> wait_for_charger parameters) support, regulators support etc.
> >>
> >> And your gpio driver will look very similar to pda_power.
> >>
> >> So I'd vote for adding the GPIO functionality to pda_power, and
> >> refactoring it if needed.
> >>
> >> Though, if there are strong objections against this idea, I
> >> can merge the GPIO driver, and let's see how things will evolve.
> >>
> >> Thanks!
> >>
> >> --
> >> Anton Vorontsov
> >> email: [email protected]
> >> irc://irc.freenode.net/bd2
> >
> > My guess, is that since no one has responded, no one is working on
> integrating the functionality into pda-power?
> >
> > -rhyland
>
> hi
>
> Well I'm still not convinced that both drivers should be merged, yet I'm
> not totally
> opposed to it. In my opinion we are in some kind of dilemma here.
> I can see Antons point regarding to introducing code duplication, but on
> the other
> hand you'll always have code duplication amongst similar drivers. This will
> especially stand out if the setup functions take up a relatively large part
> of the
> whole code size.
> One way to avoid this is by putting everything into one driver which
> handles all
> different cases. But the next time yet another sort of similar driver comes
> along
> another if-else-branch is added to the pda-power driver. And slowly over
> time things
> will get messy.
> Another option to solve the problem is to add another level of indirection.
> For
> example in form of a simple_charger driver which will take callbacks for
> add, remove
> and status. The gpio-charger and pda-power could then instantiate such a
> driver and
> provide their callbacks. But by adding more and more levels of indirection
> things
> will slowly get messy as well.
> One solution that might could work is to provide library functions which
> aim at
> providing aid for implementing (simple) charger drivers.
>
> - Lars
Well, this is how I look at it. For now, we can integrate the generic gpio-charger driver. Then there are 2 concerns. If someone wants a gpio-charger with more functionality that duplicates what pda-power does, then maybe the correct thing to do is to make a new driver instead of trying to extend gpio-charger. Also, if we put in the gpio-charger now, we can still work to integrate that functionality into pda-power, and then deprecate gpio-charger.
-rhyland
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
> Rhyland Klein wrote:
> >> From: Anton Vorontsov [mailto:[email protected]]
> >> Sent: Thursday, November 04, 2010 10:48 AM
> >> To: Rhyland Klein
> >> Cc: Lars-Peter Clausen; [email protected]; Andrew
> Chew;
> >> [email protected]; [email protected]
> >> Subject: Re: [PATCH v2] POWER: Add gpio charger driver
> >>
> >> On Thu, Oct 28, 2010 at 02:17:41PM -0700, Rhyland Klein wrote:
> >> [...]
> >>>> Hm... I guess it can be, but on the other hand most platform bus
> >> chargers
> >>>> type
> >>>> devices can be, because the pda_power driver is keep pretty generic
> >> with
> >>>> custom init,
> >>>> status and exit callbacks.
> >>>>
> >>>> - Lars
> >>> I didn't see any more discussion on this. Is the plan to integrate
> >>> the gpio-charger driver as is or to instead try to integrate support
> >>> for this into pda_power?
> >> Sorry for the delayed response, and thanks for the pings! ;-)
> >>
> >> The main thing I'm afraid of is duplication. I.e. someday you
> >> will want debouncing (include/linux/pda_power.h's wait_for_status,
> >> wait_for_charger parameters) support, regulators support etc.
> >>
> >> And your gpio driver will look very similar to pda_power.
> >>
> >> So I'd vote for adding the GPIO functionality to pda_power, and
> >> refactoring it if needed.
> >>
> >> Though, if there are strong objections against this idea, I
> >> can merge the GPIO driver, and let's see how things will evolve.
> >>
> >> Thanks!
> >>
> >> --
> >> Anton Vorontsov
> >> email: [email protected]
> >> irc://irc.freenode.net/bd2
> >
> > My guess, is that since no one has responded, no one is working on
> integrating the functionality into pda-power?
> >
> > -rhyland
>
> hi
>
> Well I'm still not convinced that both drivers should be merged, yet I'm
> not totally
> opposed to it. In my opinion we are in some kind of dilemma here.
> I can see Antons point regarding to introducing code duplication, but on
> the other
> hand you'll always have code duplication amongst similar drivers. This will
> especially stand out if the setup functions take up a relatively large part
> of the
> whole code size.
> One way to avoid this is by putting everything into one driver which
> handles all
> different cases. But the next time yet another sort of similar driver comes
> along
> another if-else-branch is added to the pda-power driver. And slowly over
> time things
> will get messy.
> Another option to solve the problem is to add another level of indirection.
> For
> example in form of a simple_charger driver which will take callbacks for
> add, remove
> and status. The gpio-charger and pda-power could then instantiate such a
> driver and
> provide their callbacks. But by adding more and more levels of indirection
> things
> will slowly get messy as well.
> One solution that might could work is to provide library functions which
> aim at
> providing aid for implementing (simple) charger drivers.
>
> - Lars
Anton, for the time being, can we integrate this driver? That way the functionality is there and can be used now?
-rhyland
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?