Re: [PATCH] mfd: add bq2415x charger driver

[Felipe Contreras]

Hi,

On Tue, Dec 6, 2011 at 1:05 AM, Pali RohÃr <pali.rohar@xxxxxxxxx> wrote:
> I started writing other implementaion of bq2415x charger driver, which should
> support also setting usb host mode. Code is still unfinished, but now is
> devided into 2 parts: one power_supply driver and one driver which cover all
> bq registers. See:
>
> http://atrey.karlin.mff.cuni.cz/~pali/bq2415x/
>
> Felipe Contreras, I think that my implementation is better - it will export
> all bq registers (which is needed for hostmode boost) and will also register
> regulator interface.

I took a look at your driver, and there's definitely good stuff in it.
However, I think there's a lot of unnecessary stuff, like the
miscdevice stuff (which was frowned upon for bq27x00), and a lot of
user-space interface. Moreover, it doesn't seem to do anything on its
own (it needs interaction from user-space).

IMO the first step should be to have a minimal driver that just works,
even if it doesn't achieve the absolute best charging performance.
More features could be added later on.

Also, I'm not familiar with the regulator interface, but it seems to
be meant for real regulators, which have consumers, and based on those
consumers's needs the real voltage changes. This battery charger on
the other hand doesn't have anything like that. There will be no
consumer, and some stuff like the weak battery voltage is not even
related to a voltage supply, but rather a threshold that can be
configured to change some behavior of the charger, but there's no
point in changing it dynamically (or maybe at all).

I guess the important one is the charge voltage, which is linked to a
real voltage, but what consumers would it have? I don't think there's
any.

Finally, I don't think user-space interaction should be needed at all.
The driver should start charging immediately when there power supply
available, and stop when there isn't any. Maybe at some point a
user-space interface will be useful later on (I don't see why), but I
don't think it should be *necessary*.

I'm not familiar with any of this stuff, so don't take my opinions too
seriously :)

Cheers.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/