Re: BGPIOF_READ_OUTPUT_REG_SET problem

[Michael Trimarchi <michael@amarulasolutions.com>]

Hi Vladimir

On Wed, Dec 16, 2015 at 9:01 PM, Vladimir Zapolskiy
<vladimir_zapolskiy@mentor.com> wrote:
> Hi Michael,
>
> On 16.12.2015 21:38, Michael Trimarchi wrote:
>> Hi
>>
>> On Dec 16, 2015 8:18 PM, "Vladimir Zapolskiy" <vladimir_zapolskiy@mentor.com
>> <mailto:vladimir_zapolskiy@mentor.com>> wrote:
>>>
>>> Hi Michael,
>>>
>>> On 16.12.2015 20:17, Michael Trimarchi wrote:
>>> > Hi
>>> >
>>> > On Wed, Dec 16, 2015 at 04:25:11PM +0100, Michael Trimarchi wrote:
>>> >> Hi
>>> >>
>>> >> On Wed, Dec 16, 2015 at 04:50:07PM +0200, Vladimir Zapolskiy wrote:
>>> >>> Hi Michael,
>>> >>>
>>> >>> On 16.12.2015 16:05, Michael Trimarchi wrote:
>>> >>>> Hi
>>> >>>>
>>> >>>> I have get a problem using gpio and imx6q freescale architecture.
>>> >>>
>>> >>> what is the problem you experience? What is the version of the kernel
>>> >>> (commit hash id) you run?
>>> >>>
>>> >>
>>> >> I don't want the mask back when I read the gpio. I want to know if it's
>>> >> 1 or 0
>>>
>>> now I got what you want, but actually I didn't get completely what is your
>>> problem. Please be more descriptive.
>>>
>>> >
>>> > This is my starting commit on my local tree:
>>> >
>>> > dd5ae6818ccd4ab0aa6f7d84e88f60a48af0fb52
>>>
>>> Good, so it is on Torvalds' branch and you have a potentially problematic
>>> commit e20538b82 applied.
>>>
>>> By the way you may try to git-bisect the problem, I may be wrong in my
>>> assumptions regarding that commit.
>>>
>>
>> gc.get pointer should be assigned to function with same semantic. So on my
>> tree semantic of function is different.
>
> Putting aside returned negative values, the problem is gone, if you change
> the semantics in your tree and assume that gc.get() returns values: low - 0
> and high - !0 (not necessary 1)? Because if you apply this logic, you get
> +18 correctly working gpiochip drivers in your tree.
>
>> I understand your point but we have two problems in mainline. One
>> 0x10000000 and other get function.
>
> Yes, the problem with the Bjorn's commit still exists. As well as another
> kind of problem, if the commit is not applied.
>

It can be fine if >get() return 0 or 1 to the corresponding gpio and error
in case of failure (like in gpio expander for example). My patch make
the implementation
of bgpio_get_set equal to bgpio_get. I don't find any sense to let the
bgpio_get_set to return
the mask because ofcourse 31 gpio can not work. Revert does not solve
the problem because
we need to have int return value in general and not boolean

Michael

> With best wishes,
> Vladimir
>
>>> >>>> I don't really understand how should work your change and I have done
>> a quick
>>> >>>> fix for me. Can you explain what was the idea?
>>> >>>
>>> >>> I believe here you are talking about changes b19e7f51a5 and
>>> >>> 442b2494b17d1 , please explain the problem.
>>> >>>
>>> >>> With best wishes,
>>> >>> Vladimir
>>> >>>
>>> >>>> Michael
>>> >>>>
>>> >>>> diff --git a/drivers/gpio/gpio-generic.c b/drivers/gpio/gpio-generic.c
>>> >>>> index a3f0753..f9c6ff7 100644
>>> >>>> --- a/drivers/gpio/gpio-generic.c
>>> >>>> +++ b/drivers/gpio/gpio-generic.c
>>> >>>> @@ -139,11 +139,14 @@ static int bgpio_get_set(struct gpio_chip *gc,
>> unsigned int gpio)
>>> >>>>  {
>>> >>>>    struct bgpio_chip *bgc = to_bgpio_chip(gc);
>>> >>>>    unsigned long pinmask = bgc->pin2mask(bgc, gpio);
>>> >>>> +  unsigned long reg;
>>> >>>>
>>> >>>>    if (bgc->dir & pinmask)
>>> >>>> -          return bgc->read_reg(bgc->reg_set) & pinmask;
>>> >>>> +          reg = bgc->read_reg(bgc->reg_set) & pinmask;
>>> >>>>    else
>>> >>>> -          return bgc->read_reg(bgc->reg_dat) & pinmask;
>>> >>>> +          reg = bgc->read_reg(bgc->reg_dat) & pinmask;
>>> >>>> +
>>> >>>> +  return !!(reg &  bgc->pin2mask(bgc, gpio));
>>> >>>>  }
>>> >>>>
>>> >>>>  static int bgpio_get(struct gpio_chip *gc, unsigned int gpio)
>>> >>>>
>>> >>
>>> >> --
>>> >> | Michael Nazzareno Trimarchi                     Amarula Solutions BV |
>>> >> | COO  -  Founder                                      Cruquiuskade 47 |
>>> >> | +31(0)851119172                                 Amsterdam 1018 AM NL |
>>> >> |                  [`as] http://www.amarulasolutions.com               |
>>> >
>>>
>>
>



-- 
| Michael Nazzareno Trimarchi                     Amarula Solutions BV |
| COO  -  Founder                                      Cruquiuskade 47 |
| +31(0)851119172                                 Amsterdam 1018 AM NL |
|                  [`as] http://www.amarulasolutions.com               |