Re: [PATCH 1/4] dt-bindings: usb: musb: Add support for MediaTek musb controller

On Thu, Jan 3, 2019 at 9:00 PM Min Guo [off-list ref] wrote:

On Thu, 2019-01-03 at 16:14 -0600, Rob Herring wrote:

quoted

On Thu, Dec 27, 2018 at 03:34:23PM +0800, min.guo@mediatek.com wrote:

quoted

From: Min Guo <redacted>

This adds support for MediaTek musb controller in
host, peripheral and otg mode

[...]

quoted

quoted

+ - interrupts      : interrupt used by musb controller
+ - interrupt-names : must be "mc"

-names is pointless when there is only one.

The MUSB core driver has two interrupts, one is for MAC, another for DMA,
but on MTK platform, there is only a MAC interrupt, here following the binding
of MUSB core driver.

You should probably be listing the same interrupt number twice if 2
interrupts are combined.

quoted

quoted

+Optional properties:
+ - extcon : external connector for VBUS and IDPIN changes detection,
+   needed when supports dual-role mode.

Don't use extcon for new bindings. The usb-connector binding should be
used instead.

This is used to detect the changes of the IDPIN and VBUS, the change
events are provided by other drivers, such as extcon-usb-gpio.c, and
then switch MUSB controller to host or device mode, but the
usb-connector can't detect these changes.

To repeat, do not use extcon binding for new bindings. It is poorly
designed as it reflects extcon driver needs, not a description of the
hardware. If you have ID on GPIO, then that belongs in a usb-connector
node because that GPIO goes to the connector. For Vbus, you should
have a vbus-supply in the connector and use a gpio-regulator if it is
GPIO controlled.

quoted

quoted

+ - vbus-supply : reference to the VBUS regulator, needed when supports
+   dual-role mode.

The controller is powered from Vbus? Probably not. This belongs in the
connector or maybe the phy (if the phy is powered from Vbus).

The Vbus is used to provide 5V voltage to the connected device when the
controller works as host mode.

I know what Vbus is. Unless Vbus is providing power to the host
controller, putting the Vbus supply in the controller node is not a
accurate representation of the hardware.

Rob