Re: [PATCH] usb: usb3503: Convert to use GPIO descriptors

[Marek Szyprowski <m.szyprowski@samsung.com>]

Hi Linus,

On 06.12.2019 10:56, Linus Walleij wrote:
> On Fri, Dec 6, 2019 at 10:14 AM Marek Szyprowski
> <m.szyprowski@samsung.com> wrote:
>> On 06.12.2019 08:55, Marek Szyprowski wrote:
>>> NAK.
>>>
>>> Sorry, but this patch breaks USB3503 HUB operation on Arndale5250
>>> board. A brief scan through the code reveals that the whole control
>>> logic for the 'intn' gpio is lost.
>> Well, I've checked further and 'intn' logic is there. The issue with
>> Arndale5250 board is something different. Changing the gpio active
>> values in arch/arm/boot/dts/exynos5250-arndale.dts from GPIO_ACTIVE_LOW
>> to GPIO_ACTIVE_HIGH fixed operation of usb3503 HUB. I really wonder why
>> it worked fine with non-descriptor code and the ACTIVE_LOW DT flags...
>>
>> I'm not sure how to handle this. Old code works also fine with DT flags
>> changed to GPIO_ACTIVE_HIGH, which seems to be a proper value for those
>> gpio lines.
> We should of course fix up the device trees so the polarity in them
> is correct.

Okay. I've checked the driver and dts:

According to the USB3503 datasheet, reset-gpios should be ACTIVE_LOW 
probably for the all boards. The driver itself should be then fixed to 
set reset line to the opposite values: HIGH (ASSERTED) during probe and 
suspend, and LOW (DE-ASSERTED) during normal operation.

With the above assumptions, the following DTS should be fixed:

arch/arm/boot/dts/exynos4412-odroid-common.dtsi: invert RESET gpio 
polarity (to ACTIVE_LOW)

arch/arm/boot/dts/exynos5250-arndale.dts: invert CONNECT gpio polarity 
(to ACTIVE_HIGH)

arch/arm/boot/dts/exynos5410-odroidxu.dts: invert RESET gpio polarity 
(to ACTIVE_LOW)

arch/arm/boot/dts/qcom-mdm9615-wp8548-mangoh-green.dts: invert RESET 
gpio polarity (to ACTIVE_LOW), not sure about INTN gpio

arch/arm/boot/dts/sun8i-a83t-cubietruck-plus.dts: invert RESET gpio 
polarity (to ACTIVE_LOW)

I've tested such changes with your patch on Odroid X2, U3, XU and 
Arndale boards - USB3503 worked fine.

I can prepare patchset with the above changes (dts and the driver logic).

> If the compatibility with elder device trees is mandatory I will make
> a quirk into the gpiolib-of.c that enforce active high on this specific
> GPIO line. This is pretty straight-forward, I can just use the compatible
> of the board and usb3503 in combination to enforce it.

Frankly, I don't care about compatibility with old dtbs. It is already 
broken by other changes in the bindings.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland