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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
