Re: [PATCH v2] PCI: dra7xx: Fix reset behaviour

[Luca Ceresoli]

Hi,

On 25/06/21 02:09, Linus Walleij wrote:
> On Fri, Jun 25, 2021 at 1:34 AM Pali Rohár <pali@xxxxxxxxxx> wrote:
> 
>>> gpiod_set_value(gpiod, 1) == assert the line
>>> gpiod_set_value(gpiod, 0) == de-assert the line
>>
>> Problem is that some pci controller drivers (e.g. pci-j721e.c or
>> pcie-rockchip-host.c) expects that gpiod_set_value_cansleep(gpiod, 1)
>> de-asserts the line and it is already used in this way.
>>
>> Which is opposite of the behavior which you wrote above.
> 
> I sketched a way out of the problem using a quirk in
> gpiolib in another response. We have a few of these
> cases where we have to code our way out of mistakes,
> such things happen.
> 
> The problem is common, and due to the fact that device tree
> authors ignores the flag GPIO_ACTIVE_HIGH (which has
> been around since the early days of device tree on PowerPC)
> instead they opt to do the inversion in code. Which violates the
> contract that the DT should describe the hardware.
> 
> The ambition of the DT specifications/schemas are to be operating
> system independent, and this kind of stuff creates a situation
> where other operating systems can't use the specification without
> also going and looking at how Linux has implemented stuff.
> Which is against the ambition of the device tree work.
> 
>> I would suggest to define enum/macro with word ASSERT and DEASSERT in
>> its name instead of just true/false boolean or 0/1 int.
>>
>> In case of this PERST# misunderstanding, having assert/deassert in name
>> should really help.
> 
> Hm that looks useful, Bart &co what do you think?
> 
> #define GPIOD_ASSERTED 1
> #define GPIOD_DEASSERTED 0
> 
> in consumer.h, would that be helpful for users?

Looks like a good idea to me. It would help making people aware that
gpiod_set_value() & co do _not_ take a physical line value. It's done
that way since ages, it's documented, yet many developers are still
unaware of that...

-- 
Luca