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:0.025 / U:91.932 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site