Hi Linus, On 06.12.2019 10:56, Linus Walleij wrote: > On Fri, Dec 6, 2019 at 10:14 AM Marek Szyprowski > <m.szyprowski@xxxxxxxxxxx> 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
