lkml.org 
[lkml]   [2007]   [Dec]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [patch/rfc 2/4] pcf857x I2C GPIO expander driver
    Date
    On Thursday 06 December 2007, Jean Delvare wrote:

    > Also, I don't quite see what
    > is supposed to make compatibility with the legacy drivers easier, nor
    > how, not why it matters in the first place.

    There's a clear either/or disjunction. No fuzzy/confusing middle ground.


    > > +static int pcf857x_get8(struct gpio_chip *chip, unsigned offset)
    > > +{
    > > + struct pcf857x *gpio = container_of(chip, struct pcf857x, chip);
    > > + s32 value;
    > > +
    > > + value = i2c_smbus_read_byte(gpio->client);
    > > + return (value < 0) ? 0 : (value & (1 << offset));
    >
    > This is no longer a boolean value, is that OK? I guess that it doesn't
    > matter but maybe it should be documented (what GPIO drivers are allowed
    > to return in these callback functions.)

    Already documented -- as zero/nonzero, the original boolean model for C.
    Anything else would be at least tristate, not boolean. :)


    > > + /* Let platform code set up the GPIOs and their users.
    > > + * Now is the first time anyone can use them.
    > > + */
    > > + if (pdata->setup) {
    > > + status = pdata->setup(client,
    > > + gpio->chip.base, gpio->chip.ngpio,
    > > + pdata->context);
    > > + if (status < 0)
    > > + dev_err(&client->dev, "%s --> %d\n",
    > > + "setup", status);
    >
    > Shouldn't this be degraded to dev_warn? The probe still succeeds. Or
    > keep dev_err but make the probe fail (in which case you'll probably
    > want to swap this block of code with the dev_info above.)

    Good point.


    > The rest looks fine to me.

    Thanks for the comments. I'll send this in with the next batch
    of gpiolib patches.

    - Dave



    \
     
     \ /
      Last update: 2007-12-07 07:21    [W:0.023 / U:60.608 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site