lkml.org 
[lkml]   [2006]   [Mar]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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.

Ok, comments below.

> > +#include <asm/arch/collie.h>
> > +#include "../mach-pxa/sharpsl.h"
>
> Do you need anything in the above header? If so, it should probably be
> in asm/hardware/sharpsl_pm.h

Thanks, fixed.

> > +#ifdef I_AM_SURE
> > +#define CF_BUF_CTRL_BASE 0xF0800000
> > +#define SCOOP_REG(adr) (*(volatile unsigned short*)(CF_BUF_CTRL_BASE+(adr)))
> > +#define SCOOP_REG_GPWR SCOOP_REG(SCOOP_GPWR)
> > +
> > + if (on) {
> > + SCOOP_REG_GPWR |= COLLIE_SCP_CHARGE_ON;
> > + } else {
> > + SCOOP_REG_GPWR &= ~COLLIE_SCP_CHARGE_ON;
>
> Ick. Use arch/arm/common/scoop.c to do this. Something like:

Thanks, fixed.

> > +extern struct sharpsl_pm_status sharpsl_pm;
> > +
> > +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.
>
> By setting sharpsl_pm.dev = NULL you're also going to miss out on
> suspend/resume calls and risk breaking things like
> dev_dbg(sharpsl_pm.dev, xxx).

Ok, I'll try to switch it to two-device config.
Pavel
--
28:class DeDRMS
-
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/

\
 
 \ /
  Last update: 2006-03-10 09:58    [W:1.604 / U:0.340 seconds]
©2003-2014 Jasper Spaans. hosted at Digital Ocean