Messages in this thread | | | Date | Sun, 5 Mar 2006 00:12:54 +0000 | From | Pavel Machek <> | Subject | Re: [rfc] Collie battery status sensing code |
| |
Hi!
> > This is collie battery sensing code. It differs from sharpsl code a > > bit -- because it is dependend on ucb1x00, not on platform bus. > > > > I guess I should reorganize #include's and remove #if 0-ed > > code. Anything else > > Basically looks good. Could probably use a > s/printk/dev_dbg(sharpsl_pm.dev, /. I've made a few other comments > below.
I'll just remove most printks before next merge attempt, I guess.
> Just for my interest, can you summarise the status of PM and charging on > collie with this code?
Well, with heavy ifdefs into sharpsl_pm, it can correctly sense AC/battery. Voltmeters seem to return *some* values. Temperature is probably okay, battery level is *extremely* noisy, and outside range where sharp code expects it, but seems to correlate with actual battery levels.
Now... something does charge my battery, but it is definitely not my code. I don't dare enabling charging yet.
(More detailed reply will come later today, when I'm close to collie).
> > +#include "../drivers/mfd/ucb1x00.h" > > +#include <asm/mach/sharpsl_param.h> > > + > > +#ifdef CONFIG_MCP_UCB1200_TS > > +#error Battery interferes with touchscreen > > +#endif > > Is this a case of bad locking or something more serious?
Original sharp code does some magic between TS and battery reading. It seems AD0 is used by both, or something similary ugly. I'll have to decipher sharp code and figure out how to do it properly.
> > +static int __init collie_pm_add(struct ucb1x00_dev *pdev) > > +{ > > + sharpsl_pm.dev = NULL; > > + sharpsl_pm.machinfo = &collie_pm_machinfo; > > + ucb = pdev->ucb; > > + return sharpsl_pm_init(); > > +} > > I don't understand how this is supposed to work at all. For a start, > sharpsl_pm.c says "static int __devinit sharpsl_pm_init(void)" so that > function isn't available. I've just noticed your further patches > although I still don't like this. > > The correct approach is to register a platform device called > "sharpsl-pm" in collie_pm_add() which the driver will then see and > attach to. I'd also not register the platform device if ucb is NULL for > whatever reason.
I thought about it, and considered it quite ugly. Result would be all data on the platform bus with half-empty device on ucb1x00 "bus". It would bring me some problems with registering order: if platform device is registered too soon, ucb will be NULL and it will crash and burn. OTOH I already have static *ucb, so it is doable, and I can do it if you prefer it that way... -- Thanks, Sharp! - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |