[PATCH 1/1] [INPUT/KEYPAD] gpio keypad: Replace current blackfin specific pfbutton driver with kernel generic gpio key driver - Kernel

This is a discussion on [PATCH 1/1] [INPUT/KEYPAD] gpio keypad: Replace current blackfin specific pfbutton driver with kernel generic gpio key driver - Kernel ; From: Michael Hennerich It's an actual deficiency in the hardware that we can't address, so it needs to be worked around in software. Signed-off-by: Michael Hennerich Signed-off-by: Bryan Wu --- drivers/input/keyboard/gpio_keys.c | 15 ++++++++++++++- 1 files changed, 14 insertions(+), 1 ...

+ Reply to Thread
Results 1 to 10 of 10

Thread: [PATCH 1/1] [INPUT/KEYPAD] gpio keypad: Replace current blackfin specific pfbutton driver with kernel generic gpio key driver

  1. [PATCH 1/1] [INPUT/KEYPAD] gpio keypad: Replace current blackfin specific pfbutton driver with kernel generic gpio key driver

    From: Michael Hennerich

    It's an actual deficiency in the hardware that we can't address,
    so it needs to be worked around in software.

    Signed-off-by: Michael Hennerich
    Signed-off-by: Bryan Wu
    ---
    drivers/input/keyboard/gpio_keys.c | 15 ++++++++++++++-
    1 files changed, 14 insertions(+), 1 deletions(-)

    diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
    index bbd00c3..d856eb9 100644
    --- a/drivers/input/keyboard/gpio_keys.c
    +++ b/drivers/input/keyboard/gpio_keys.c
    @@ -26,6 +26,18 @@

    #include

    +#if defined(CONFIG_BLACKFIN) && !defined(BF548_FAMILY)
    +
    +/*
    + * On some Blackfin CPUs reading edge triggered
    + * GPIOs doesn't return the current value
    + */
    +
    +#define GPIOKEYS_EDGE_SENSE(x) set_gpio_edge(gpio, x)
    +#else
    +#define GPIOKEYS_EDGE_SENSE(x) do {} while (0)
    +#endif
    +
    static irqreturn_t gpio_keys_isr(int irq, void *dev_id)
    {
    int i;
    @@ -39,8 +51,9 @@ static irqreturn_t gpio_keys_isr(int irq, void *dev_id)

    if (irq == gpio_to_irq(gpio)) {
    unsigned int type = button->type ?: EV_KEY;
    + GPIOKEYS_EDGE_SENSE(0);
    int state = (gpio_get_value(gpio) ? 1 : 0) ^ button->active_low;
    -
    + GPIOKEYS_EDGE_SENSE(1);
    input_event(input, type, button->code, !!state);
    input_sync(input);
    return IRQ_HANDLED;
    --
    1.5.5

    --
    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/

  2. Re: [PATCH 1/1] [INPUT/KEYPAD] gpio keypad: Replace current blackfin specific pfbutton driver with kernel generic gpio key driver

    Hi,

    On Mon, May 12, 2008 at 12:17 PM, Bryan Wu wrote:
    > From: Michael Hennerich
    >
    > It's an actual deficiency in the hardware that we can't address,
    > so it needs to be worked around in software.
    >
    > Signed-off-by: Michael Hennerich
    > Signed-off-by: Bryan Wu
    > ---
    > drivers/input/keyboard/gpio_keys.c | 15 ++++++++++++++-
    > 1 files changed, 14 insertions(+), 1 deletions(-)
    >
    > diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
    > index bbd00c3..d856eb9 100644
    > --- a/drivers/input/keyboard/gpio_keys.c
    > +++ b/drivers/input/keyboard/gpio_keys.c
    > @@ -26,6 +26,18 @@
    >
    > #include
    >
    > +#if defined(CONFIG_BLACKFIN) && !defined(BF548_FAMILY)
    > +
    > +/*
    > + * On some Blackfin CPUs reading edge triggered
    > + * GPIOs doesn't return the current value
    > + */


    If this is a generic problem, shouldn't this be addressed inside gpio_get_value?

    > +
    > +#define GPIOKEYS_EDGE_SENSE(x) set_gpio_edge(gpio, x)
    > +#else
    > +#define GPIOKEYS_EDGE_SENSE(x) do {} while (0)
    > +#endif
    > +
    > static irqreturn_t gpio_keys_isr(int irq, void *dev_id)
    > {
    > int i;
    > @@ -39,8 +51,9 @@ static irqreturn_t gpio_keys_isr(int irq, void *dev_id)
    >
    > if (irq == gpio_to_irq(gpio)) {
    > unsigned int type = button->type ?: EV_KEY;
    > + GPIOKEYS_EDGE_SENSE(0);
    > int state = (gpio_get_value(gpio) ? 1 : 0) ^ button->active_low;
    > -
    > + GPIOKEYS_EDGE_SENSE(1);
    > input_event(input, type, button->code, !!state);
    > input_sync(input);
    > return IRQ_HANDLED;
    > --
    > 1.5.5
    >
    >


    regards
    Philipp
    --
    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/

  3. Re: [PATCH 1/1] [INPUT/KEYPAD] gpio keypad: Replace current blackfin specific pfbutton driver with kernel generic gpio key driver

    On Mon, May 12, 2008 at 7:27 AM, pHilipp Zabel wrote:
    > On Mon, May 12, 2008 at 12:17 PM, Bryan Wu wrote:
    >> From: Michael Hennerich
    >>
    >> It's an actual deficiency in the hardware that we can't address,
    >> so it needs to be worked around in software.
    >>
    >> Signed-off-by: Michael Hennerich
    >> Signed-off-by: Bryan Wu
    >> ---
    >> drivers/input/keyboard/gpio_keys.c | 15 ++++++++++++++-
    >> 1 files changed, 14 insertions(+), 1 deletions(-)
    >>
    >> diff --git a/drivers/input/keyboard/gpio_keys.c

    b/drivers/input/keyboard/gpio_keys.c
    >> index bbd00c3..d856eb9 100644
    >> --- a/drivers/input/keyboard/gpio_keys.c
    >> +++ b/drivers/input/keyboard/gpio_keys.c
    >> @@ -26,6 +26,18 @@
    >>
    >> #include
    >>
    >> +#if defined(CONFIG_BLACKFIN) && !defined(BF548_FAMILY)
    >> +
    >> +/*
    >> + * On some Blackfin CPUs reading edge triggered
    >> + * GPIOs doesn't return the current value
    >> + */

    >
    > If this is a generic problem, shouldn't this be addressed inside gpio_get_value?


    it's an issue only when the GPIO is an interrupt source and the
    trigger condition is set to both rising and falling. but i guess your
    point is that in gpio_get_value(), we can check to see if these
    conditions are met and if so, temporarily fiddle things there ?

    Michael: that sounds reasonable, what do you think ?
    -mike
    --
    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/

  4. Re: [PATCH 1/1] [INPUT/KEYPAD] gpio keypad: Replace current blackfin specific pfbutton driver with kernel generic gpio key driver

    On Mon, May 12, 2008 at 7:42 PM, Mike Frysinger wrote:
    > On Mon, May 12, 2008 at 7:27 AM, pHilipp Zabel wrote:
    > > On Mon, May 12, 2008 at 12:17 PM, Bryan Wu wrote:
    > >> From: Michael Hennerich
    > >>
    > >> It's an actual deficiency in the hardware that we can't address,
    > >> so it needs to be worked around in software.
    > >>
    > >> Signed-off-by: Michael Hennerich
    > >> Signed-off-by: Bryan Wu
    > >> ---
    > >> drivers/input/keyboard/gpio_keys.c | 15 ++++++++++++++-
    > >> 1 files changed, 14 insertions(+), 1 deletions(-)
    > >>
    > >> diff --git a/drivers/input/keyboard/gpio_keys.c

    > b/drivers/input/keyboard/gpio_keys.c
    > >> index bbd00c3..d856eb9 100644
    > >> --- a/drivers/input/keyboard/gpio_keys.c
    > >> +++ b/drivers/input/keyboard/gpio_keys.c
    > >> @@ -26,6 +26,18 @@
    > >>
    > >> #include
    > >>
    > >> +#if defined(CONFIG_BLACKFIN) && !defined(BF548_FAMILY)
    > >> +
    > >> +/*
    > >> + * On some Blackfin CPUs reading edge triggered
    > >> + * GPIOs doesn't return the current value
    > >> + */

    > >
    > > If this is a generic problem, shouldn't this be addressed inside gpio_get_value?

    >
    > it's an issue only when the GPIO is an interrupt source and the
    > trigger condition is set to both rising and falling. but i guess your
    > point is that in gpio_get_value(), we can check to see if these
    > conditions are met and if so, temporarily fiddle things there ?
    >
    > Michael: that sounds reasonable, what do you think ?
    > -mike
    >


    IMO, maybe it is feasible for fix this in generic GPIO layer, or will
    break other things?

    -Bryan
    --
    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/

  5. Re: [PATCH 1/1] [INPUT/KEYPAD] gpio keypad: Replace current blackfin specific pfbutton driver with kernel generic gpio key driver

    On Mon, May 12, 2008 at 8:11 AM, Bryan Wu wrote:
    > On Mon, May 12, 2008 at 7:42 PM, Mike Frysinger wrote:
    >> On Mon, May 12, 2008 at 7:27 AM, pHilipp Zabel

    wrote:
    >> > On Mon, May 12, 2008 at 12:17 PM, Bryan Wu wrote:
    >> >> From: Michael Hennerich
    >> >>
    >> >> It's an actual deficiency in the hardware that we can't address,
    >> >> so it needs to be worked around in software.
    >> >>
    >> >> Signed-off-by: Michael Hennerich
    >> >> Signed-off-by: Bryan Wu
    >> >> ---
    >> >> drivers/input/keyboard/gpio_keys.c | 15 ++++++++++++++-
    >> >> 1 files changed, 14 insertions(+), 1 deletions(-)
    >> >>
    >> >> diff --git a/drivers/input/keyboard/gpio_keys.c

    >> b/drivers/input/keyboard/gpio_keys.c
    >> >> index bbd00c3..d856eb9 100644
    >> >> --- a/drivers/input/keyboard/gpio_keys.c
    >> >> +++ b/drivers/input/keyboard/gpio_keys.c
    >> >> @@ -26,6 +26,18 @@
    >> >>
    >> >> #include
    >> >>
    >> >> +#if defined(CONFIG_BLACKFIN) && !defined(BF548_FAMILY)
    >> >> +
    >> >> +/*
    >> >> + * On some Blackfin CPUs reading edge triggered
    >> >> + * GPIOs doesn't return the current value
    >> >> + */
    >> >
    >> > If this is a generic problem, shouldn't this be addressed inside

    gpio_get_value?
    >>
    >> it's an issue only when the GPIO is an interrupt source and the
    >> trigger condition is set to both rising and falling. but i guess your
    >> point is that in gpio_get_value(), we can check to see if these
    >> conditions are met and if so, temporarily fiddle things there ?
    >>
    >> Michael: that sounds reasonable, what do you think ?

    >
    > IMO, maybe it is feasible for fix this in generic GPIO layer, or will
    > break other things?


    where are you referring to ? i think what's being proposed is to move
    these checks to arch/blackfin/kernel/bfin_gpio.c ... any call to
    gpio_get_value() under these circumstances will need the edge sense
    toggle ...
    -mike
    --
    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/

  6. Re: [PATCH 1/1] [INPUT/KEYPAD] gpio keypad: Replace current blackfin specific pfbutton driver with kernel generic gpio key driver

    On Mon, May 12, 2008 at 8:47 PM, Mike Frysinger wrote:
    >
    > On Mon, May 12, 2008 at 8:11 AM, Bryan Wu wrote:
    > > On Mon, May 12, 2008 at 7:42 PM, Mike Frysinger wrote:
    > >> On Mon, May 12, 2008 at 7:27 AM, pHilipp Zabel

    > wrote:
    > >> > On Mon, May 12, 2008 at 12:17 PM, Bryan Wu wrote:
    > >> >> From: Michael Hennerich
    > >> >>
    > >> >> It's an actual deficiency in the hardware that we can't address,
    > >> >> so it needs to be worked around in software.
    > >> >>
    > >> >> Signed-off-by: Michael Hennerich
    > >> >> Signed-off-by: Bryan Wu
    > >> >> ---
    > >> >> drivers/input/keyboard/gpio_keys.c | 15 ++++++++++++++-
    > >> >> 1 files changed, 14 insertions(+), 1 deletions(-)
    > >> >>
    > >> >> diff --git a/drivers/input/keyboard/gpio_keys.c
    > >> b/drivers/input/keyboard/gpio_keys.c
    > >> >> index bbd00c3..d856eb9 100644
    > >> >> --- a/drivers/input/keyboard/gpio_keys.c
    > >> >> +++ b/drivers/input/keyboard/gpio_keys.c
    > >> >> @@ -26,6 +26,18 @@
    > >> >>
    > >> >> #include
    > >> >>
    > >> >> +#if defined(CONFIG_BLACKFIN) && !defined(BF548_FAMILY)
    > >> >> +
    > >> >> +/*
    > >> >> + * On some Blackfin CPUs reading edge triggered
    > >> >> + * GPIOs doesn't return the current value
    > >> >> + */
    > >> >
    > >> > If this is a generic problem, shouldn't this be addressed inside

    > gpio_get_value?
    > >>
    > >> it's an issue only when the GPIO is an interrupt source and the
    > >> trigger condition is set to both rising and falling. but i guess your
    > >> point is that in gpio_get_value(), we can check to see if these
    > >> conditions are met and if so, temporarily fiddle things there ?
    > >>
    > >> Michael: that sounds reasonable, what do you think ?

    > >

    >
    > > IMO, maybe it is feasible for fix this in generic GPIO layer, or will
    > > break other things?

    >
    > where are you referring to ? i think what's being proposed is to move
    > these checks to arch/blackfin/kernel/bfin_gpio.c ... any call to
    > gpio_get_value() under these circumstances will need the edge sense
    > toggle ...
    > -mike
    >


    Right. I mean the generic GPIO layer in Blackfin port, not the common code.

    And on the other hand, maybe there are also other hardware which has similar
    issue as Blackfin. So the common GPIO layer can take care of this, how
    do you think, David?

    -Bryan
    --
    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/

  7. Re: [PATCH 1/1] [INPUT/KEYPAD] gpio keypad: Replace current blackfin specific pfbutton driver with kernel generic gpio key driver

    On Monday 12 May 2008, Bryan Wu wrote:
    > Right. I mean the generic GPIO layer in Blackfin port, not the common code.
    >
    > And on the other hand, maybe there are also other hardware which has similar
    > issue as Blackfin. So the common GPIO layer can take care of this, how
    > do you think, David?


    No; it's a Blackfin-specific design flaw, one that I've not seen
    in any other chip's GPIO support. So it should stay in the Blackfin
    support code. (Possibly even specialized for the specific chips
    which have that flaw/erratum.)

    - Dave
    --
    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/

  8. Re: [PATCH 1/1] [INPUT/KEYPAD] gpio keypad: Replace current blackfin specific pfbutton driver with kernel generic gpio key driver

    On Monday 12 May 2008, Bryan Wu wrote:
    > ************************unsigned int type = button->type ?: EV_KEY;
    > +***********************GPIOKEYS_EDGE_SENSE(0);
    > ************************int state = (gpio_get_value(gpio) ? 1 : 0) ^ button->active_low;
    > -
    > +***********************GPIOKEYS_EDGE_SENSE(1);


    By the way ... don't intersperse declarations like "int state"
    with code like what those EDGE_SENSE macros wraps...

    --
    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/

  9. Re: [PATCH 1/1] [INPUT/KEYPAD] gpio keypad: Replace current blackfin specific pfbutton driver with kernel generic gpio key driver

    On Mon, May 12, 2008 at 12:54 PM, David Brownell wrote:
    > On Monday 12 May 2008, Bryan Wu wrote:
    > > Right. I mean the generic GPIO layer in Blackfin port, not the common code.
    > >
    > > And on the other hand, maybe there are also other hardware which has similar
    > > issue as Blackfin. So the common GPIO layer can take care of this, how
    > > do you think, David?

    >
    > No; it's a Blackfin-specific design flaw, one that I've not seen
    > in any other chip's GPIO support. So it should stay in the Blackfin
    > support code. (Possibly even specialized for the specific chips
    > which have that flaw/erratum.)


    afaik, it was a design flaw, not an erratum ... newer parts have the
    situation rectified. but Michael would be able to comment more
    authoritatively on the topic. we already have the logic to only make
    the swap for parts that need it.
    -mike
    --
    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/

  10. RE: [PATCH 1/1] [INPUT/KEYPAD] gpio keypad: Replace current blackfin specific pfbutton driver with kernel generic gpio key driver



    >-----Original Message-----
    >From: Mike Frysinger [mailto:vapier.adi@gmail.com]
    >Sent: Montag, 12. Mai 2008 19:00
    >To: David Brownell
    >Cc: Bryan Wu; pHilipp Zabel; dbrownell@users.sourceforge.net;
    >dmitry.torokhov@gmail.com; linux-kernel@vger.kernel.org; Michael

    Hennerich
    >Subject: Re: [PATCH 1/1] [INPUT/KEYPAD] gpio keypad: Replace current
    >blackfin specific pfbutton driver with kernel generic gpio key driver
    >
    >On Mon, May 12, 2008 at 12:54 PM, David Brownell wrote:
    >> On Monday 12 May 2008, Bryan Wu wrote:
    >> > Right. I mean the generic GPIO layer in Blackfin port, not the

    common
    >code.
    >> >
    >> > And on the other hand, maybe there are also other hardware which

    has
    >similar
    >> > issue as Blackfin. So the common GPIO layer can take care of this,

    how
    >> > do you think, David?

    >>
    >> No; it's a Blackfin-specific design flaw, one that I've not seen
    >> in any other chip's GPIO support. So it should stay in the Blackfin
    >> support code. (Possibly even specialized for the specific chips
    >> which have that flaw/erratum.)

    >
    >afaik, it was a design flaw, not an erratum ... newer parts have the
    >situation rectified. but Michael would be able to comment more
    >authoritatively on the topic. we already have the logic to only make
    >the swap for parts that need it.
    >-mike


    I agree with all. This hack patch shouldn't be discussed here. I must
    have overseen this while we discussed what to send upstream.

    Please disregard.

    -Michael

    --
    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/

+ Reply to Thread