Re: [PATCH 4.19.y-cip 00/23] Renesas RZ/G2E USB Type-C Backport

[Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>]

Hi Pavel!

First of all, thank you for the review. 

> -----Original Message-----
> From: Pavel Machek <pavel@denx.de>
> Sent: 19 February 2020 07:42
> To: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
> Cc: cip-dev@lists.cip-project.org
> Subject: Re: [cip-dev] [PATCH 4.19.y-cip 00/23] Renesas RZ/G2E USB Type-C
> Backport
> 
> Hi!
> 
> > This patchset revolves around the following patch set from upstream:
> > https://patchwork.kernel.org/cover/10969899/
> >
> > Unfortunately, the driver is using a slightly different version of the
> > USB API. In upstream, the connection between the fwnode of the USB
> > controller device tree node, that is the connector device tree node,
> > and the USB peripheral device tree node is nicely done via some new
> > function calls that are basically some extensions to the graph
> > traverse and graph discovery methods for the device tree parser. This is
> needed for role switch feature.
> >
> > I tried to create the minimum set for the USB API extension and it
> > should not affect at all the current behavior. All the modifications
> > are additions and some rewritings of some of the current functions.
> >
> > I also backported some fixes where I have found them.
> >
> > Tested on Si-Linux EK874 (with RZ/G2E SoC) that we have in house.
> 
> Ok, so this does not look bad, but it affects other people's boards, too. I went
> through it and will have some questions/comments.
> 
> Is someone else in -cip project using usb-otg? Are we reasonably sure this
> won't cause problems elsewhere?

There are not many USB Type-C connectors, not even in upstream. For the common
area of the code (drivers/usb/roles/class.c) I see only two drivers that call
usb_role_switch_register(), Renesas's and x86 Intel xHCI driver.

Does linux-cip support x86 platforms?

I am actively trying to find what could go wrong with this implementation. Also, there
might be another way of doing this by attaching the graph connection to the port
controller device tree node and create a device connection between the two devices
(the role switch device and the port controller device). This might get rid of all the extra
additional patches in the general area of the code (no reference to fwnode), but might
introduce modifications into the port controller driver and into the role switch driver of
Renesas.

I will attempt to answer to all your comments, maybe this will look better in a v2 patchset.

Best regards,
Marian