Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1331201 > unrolled thread

Re: [PATCH v2] gpio: Add driver for TI TPIC2810

Started byLinus Walleij <linus.walleij@linaro.org>
First post2016-02-10 15:30 +0100
Last post2016-02-14 14:20 +0100
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH v2] gpio: Add driver for TI TPIC2810 Linus Walleij <linus.walleij@linaro.org> - 2016-02-10 15:30 +0100
    Re: [PATCH v2] gpio: Add driver for TI TPIC2810 Andy Shevchenko <andy.shevchenko@gmail.com> - 2016-02-10 15:30 +0100
      Re: [PATCH v2] gpio: Add driver for TI TPIC2810 Linus Walleij <linus.walleij@linaro.org> - 2016-02-14 14:20 +0100

#1331201 — Re: [PATCH v2] gpio: Add driver for TI TPIC2810

FromLinus Walleij <linus.walleij@linaro.org>
Date2016-02-10 15:30 +0100
SubjectRe: [PATCH v2] gpio: Add driver for TI TPIC2810
Message-ID<r0DWi-3jE-3@gated-at.bofh.it>
On Sun, Jan 31, 2016 at 11:52 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:

> It reminds me how 12 channel PWM chip is used on Intel Galileo Gen 2.
> Half pins are PWM, the other half is GPIO used for discrete based pin
> muxing and control. Nevertheless I think it's a userspace issue for
> now, otherwise we have to provide some 'semi-virtual' way of
> presenting pins as GPIO lines.

That sounds like an MFD spawning a GPIO and a PWM cell.
That it is called "a PWM chip" is no big deal, it should be
modeled according to what it is, not what it claims to be.

(Which makes me wanna merge this present patch as a GPIO
driver since it claims to be a LED driver but is a GPO.)

See the ST Multipurpose Expander for an example
drivers/mfd/stmpe.c
drivers/gpio/gpio-stmpe.c
drivers/input/keyboard/stmpe-keypad.c

Yours,
Linus Walleij

[toc] | [next] | [standalone]


#1331206

FromAndy Shevchenko <andy.shevchenko@gmail.com>
Date2016-02-10 15:30 +0100
Message-ID<r0DWi-3jE-17@gated-at.bofh.it>
In reply to#1331201
On Wed, Feb 10, 2016 at 4:21 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Sun, Jan 31, 2016 at 11:52 PM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>
>> It reminds me how 12 channel PWM chip is used on Intel Galileo Gen 2.
>> Half pins are PWM, the other half is GPIO used for discrete based pin
>> muxing and control. Nevertheless I think it's a userspace issue for
>> now, otherwise we have to provide some 'semi-virtual' way of
>> presenting pins as GPIO lines.
>
> That sounds like an MFD spawning a GPIO and a PWM cell.
> That it is called "a PWM chip" is no big deal, it should be
> modeled according to what it is, not what it claims to be.

Although I agree with model I barely imagine how in this case drivers
should access PWM chip registers in non-race way (take into account
that PWM itself is connected to i2c bus).

> (Which makes me wanna merge this present patch as a GPIO
> driver since it claims to be a LED driver but is a GPO.)
>
> See the ST Multipurpose Expander for an example
> drivers/mfd/stmpe.c
> drivers/gpio/gpio-stmpe.c
> drivers/input/keyboard/stmpe-keypad.c

I will look to the example later, thanks.

-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]


#1333500

FromLinus Walleij <linus.walleij@linaro.org>
Date2016-02-14 14:20 +0100
Message-ID<r24KJ-35d-1@gated-at.bofh.it>
In reply to#1331206
On Wed, Feb 10, 2016 at 3:29 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Wed, Feb 10, 2016 at 4:21 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
>> On Sun, Jan 31, 2016 at 11:52 PM, Andy Shevchenko
>> <andy.shevchenko@gmail.com> wrote:
>>
>>> It reminds me how 12 channel PWM chip is used on Intel Galileo Gen 2.
>>> Half pins are PWM, the other half is GPIO used for discrete based pin
>>> muxing and control. Nevertheless I think it's a userspace issue for
>>> now, otherwise we have to provide some 'semi-virtual' way of
>>> presenting pins as GPIO lines.
>>
>> That sounds like an MFD spawning a GPIO and a PWM cell.
>> That it is called "a PWM chip" is no big deal, it should be
>> modeled according to what it is, not what it claims to be.
>
> Although I agree with model I barely imagine how in this case drivers
> should access PWM chip registers in non-race way (take into account
> that PWM itself is connected to i2c bus).

There is a pattern for that. You add a set of accessor functions that
performs the I2C traffic in the MFD layer.

The accessor functions take a mutex. Since this is all slowpath,
waiting/preempting in a mutex is perfectly fine for all subdrivers.

Look at this:

/**
 * stmpe_reg_write() - write a single STMPE register
 * @stmpe:      Device to write to
 * @reg:        Register to write
 * @val:        Value to write
 */
int stmpe_reg_write(struct stmpe *stmpe, u8 reg, u8 val)
{
        int ret;

        mutex_lock(&stmpe->lock);
        ret = __stmpe_reg_write(stmpe, reg, val);
        mutex_unlock(&stmpe->lock);

        return ret;
}
EXPORT_SYMBOL_GPL(stmpe_reg_write);

Yours,
Linus Walleij

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web