[U-Boot] [PATCH 1/2] dm: gpio: mxc: fix mxc_gpio_bank_get_value

[Simon Glass]

Hi Peng,

On 4 April 2016 at 23:04, Peng Fan <van.freenix at gmail.com> wrote:
> Hi Stefano,
>
> On Sun, Apr 03, 2016 at 08:09:35PM +0200, Stefano Babic wrote:
>>Hi Peng,
>>
>>On 17/03/2016 02:44, Peng Fan wrote:
>>> On Wed, Mar 16, 2016 at 09:48:24AM -0300, Fabio Estevam wrote:
>>>> Hi Peng,
>>>>
>>>> On Tue, Mar 15, 2016 at 10:27 PM, Peng Fan <van.freenix at gmail.com> wrote:
>>>>
>>>>> If set the SION bit, we need to change the pinmux settings in device tree,
>>>>> however device tree are introduced from linux kernel. I would not like
>>>>> to change the pinmux setting value.
>>>>>
>>>>> So I choose to use data register but not PSR register.
>>>>
>>>> Haven't checked the kernel driver, but just curious: how does the
>>>> kernel handle this?
>>>
>>> bgc->gc.get = bgpio_get;
>>>
>>> The get method:
>>> * @get: returns value for signal "offset"; for output signals this
>>> *      returns either the value actually sensed, or zero
>>>
>>> 138 static int bgpio_get(struct gpio_chip *gc, unsigned int gpio)
>>> 139 {
>>> 140         struct bgpio_chip *bgc = to_bgpio_chip(gc);
>>> 141
>>> 142         return !!(bgc->read_reg(bgc->reg_dat) & bgc->pin2mask(bgc, gpio));
>>> 143 }
>>>
>>> The gpio subsystem use reg_data. The reg_data is GPIO_PSR for gpio-mxc.c:
>>>
>>> 450         err = bgpio_init(&port->bgc, &pdev->dev, 4,
>>> 451                          port->base + GPIO_PSR,
>>> 452                          port->base + GPIO_DR, NULL,
>>> 453                          port->base + GPIO_GDIR, NULL, 0);
>>>
>>> So we know that kernel gpio-mxc use GPIO_PSR for reading data, but
>>> as the comments for get method "for output signals this returns
>>> either the value actually sensed, or zero", if no SION bit set, reading
>>> PSR will return 0 with direction output.
>>>
>>
>>There was already a debate in the past about this point. You can take a
>>look at the commit:
>>
>>commit 5dafa4543c399d329c7b01df1afa98437861cac0
>>Author: Benoît Thébaudeau <benoit.thebaudeau at advansee.com>
>>Date:   Mon Aug 20 10:55:41 2012 +0000
>>
>>    mxc: Make gpio_get_value() use PSR
>>
>>
>>There were good reasons to switch to PSR (mxc-gpio used DR before) - we
>>cannot switch any time back to solve a single issue.
>>
>>I would like to remain compliant with Linux - your change breaks also
>>boards that rely on this behavior.
>
> Thanks for this. I'll look at whether I can have some new way.

Two possible ideas:

1. Cache the output state in the driver so you can return it
2. Add a new get_output_value() method to the uclass (although maybe I
misunderstand the problem here)

>
> Regards,
> Peng.
>>
>>
>>> But in U-Boot, the regulator driver needs to get the value when direction is
>>> configured as output, we can use data register here or configure SION in pinmux.
>>> But I prefer using data register here.
>>>
>>
>>Best regards,
>>Stefano
>>
>>--
>>=====================================================================
>>DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
>>HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>>Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
>>=====================================================================

Regards,
Simon