lkml.org 
[lkml]   [2017]   [Sep]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH net-next RFC 3/9] net: dsa: mv88e6xxx: add support for GPIO configuration
    On Thu, Sep 28, 2017 at 10:45:03AM -0700, Florian Fainelli wrote:
    > On 09/28/2017 08:25 AM, Brandon Streiff wrote:
    > > The Scratch/Misc register is a windowed interface that provides access
    > > to the GPIO configuration. Provide a new method for configuration of
    > > GPIO functions.
    > >
    > > Signed-off-by: Brandon Streiff <brandon.streiff@ni.com>
    > > ---
    >
    > > +/* Offset 0x1A: Scratch and Misc. Register */
    > > +static int mv88e6xxx_g2_scratch_reg_read(struct mv88e6xxx_chip *chip,
    > > + int reg, u8 *data)
    > > +{
    > > + int err;
    > > + u16 value;
    > > +
    > > + err = mv88e6xxx_g2_write(chip, MV88E6XXX_G2_SCRATCH_MISC_MISC,
    > > + reg << 8);
    > > + if (err)
    > > + return err;
    > > +
    > > + err = mv88e6xxx_g2_read(chip, MV88E6XXX_G2_SCRATCH_MISC_MISC, &value);
    > > + if (err)
    > > + return err;
    > > +
    > > + *data = (value & MV88E6XXX_G2_SCRATCH_MISC_DATA_MASK);
    > > +
    > > + return 0;
    > > +}
    >
    > With the write and read acquiring and then releasing the lock
    > immediately, is no there room for this sequence to be interrupted in the
    > middle and end-up returning inconsistent reads?

    Hi Florian

    The general pattern in this code is that the lock chip->reg_lock is
    taken at a higher level. That protects against other threads. The
    driver tends to do that at the highest levels, at the entry points
    into the driver. I've not yet checked this code follows the pattern
    yet. However, we have a check in the low level to ensure the lock has
    been taken. So it seems likely the lock is held.

    > Would there be any value in implementing a proper gpiochip structure
    > here such that other pieces of SW can see this GPIO controller as a
    > provider and you can reference it from e.g: Device Tree using GPIO
    > descriptors?

    That would be my preference as well, or maybe a pinctrl driver.

    Andrew

    \
     
     \ /
      Last update: 2017-09-28 20:01    [W:4.044 / U:0.180 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site