On 2/25/25 10:11, Krzysztof Kozlowski wrote: > On 24/02/2025 11:48, Ivaylo Ivanov wrote: >> On 2/24/25 10:56, Krzysztof Kozlowski wrote: >>> On Sun, Feb 23, 2025 at 02:22:22PM +0200, Ivaylo Ivanov wrote: >>>> The Exynos2200 SoC has a USB controller PHY, which acts as an >>>> intermediary between a USB controller (typically DWC3) and other PHYs >>>> (UTMI, PIPE3). Add a dt-binding schema for it. >>>> >>>> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@xxxxxxxxx> >>>> --- >>>> .../phy/samsung,exynos2200-usbcon-phy.yaml | 76 +++++++++++++++++++ >>>> 1 file changed, 76 insertions(+) >>>> create mode 100644 Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml >>> You have undocumented dependencies which prevent merging this file. >>> First, dependencies have to be clearly expressed. >> They are, in the cover letter. > Where? I read it twice. Dependencies is the most important thing and > should scream at beginning of the cover letter, so if you bury them > somewhere deep it also would not matter - just like they were missing. > >>> Second, you should >>> rather decouple the code from header dependencies, otherwise this cannot >>> be merged for current release (just use clocks with long names, without IDs). >> Sure > >>>> diff --git a/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml b/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml >>>> new file mode 100644 >>>> index 000000000..7d879ec8b >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/phy/samsung,exynos2200-usbcon-phy.yaml >>>> @@ -0,0 +1,76 @@ >>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >>>> +%YAML 1.2 >>>> +--- >>>> +$id: http://devicetree.org/schemas/phy/samsung,exynos2200-usbcon-phy.yaml# >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>>> + >>>> +title: Exynos2200 USB controller PHY >>>> + >>>> +maintainers: >>>> + - Ivaylo Ivanov <ivo.ivanov.ivanov1@xxxxxxxxx> >>>> + >>>> +description: >>>> + Exynos2200 USB controller PHY is an intermediary between a USB controller >>>> + (typically DWC3) and other PHYs (UTMI, PIPE3). >>> Isn't this the same as usbdrd phy? see: samsung,usb3-drd-phy.yaml >> It's not (I think). There's a few reasons I've decided to make this separate >> from the usb3-drd-phy bindings and exynos5-usbdrd driver: >> >> 1. This PHY does not provide UTMI and PIPE3 on its own. There's no tuning > USBDRD phy does not provide UTMI and PIPE on its own either if you look > at diagram - they call it phy controller. Ughm. What? So in most exynos cases, there's a combination of multiple phys? > >> for them, and all that is needed from it is to disable HWACG, assert/ >> deassert reset and force bvalid/vbusvalid. After that SNPS eUSB2 >> initialization can be done and USB2 works. If the USBCON phy is not set >> up before the eUSB2 one, the device hangs, so there is definitely a >> dependancy between them. For PIPE3 we'd need to control the pipe3 >> attaching/deattaching and then initialize the synopsys USBDP combophy. > Does it mean there is no USB DRD phy controller as before? > > Anyway the problem is you have DWC3 -> PHY -> PHY. Looks one phy too many. So... DWC3 -> USBDRD (USBCON) -> PHYs? ...with usbdrd controller connecting and controlling the USB2 and USB3 phys, as well as dual role mode? Well, where is the DRD part in the exynos5 driver? I guess it does perfectly fit the job of a usbdrd controller then (if it even deals with DRD). But then again, this brings up two questions: 1. Should this driver even be named exynos2200-usbcon and not, for example, exynos2200-usbdrd? 2. Are the exynos5-usbdrd phys really only USBDRD, or do they implement USB speed functionality? What is the UTMI/PIPE3 setup for then? ps: dealing with this without any documentations sucks. Best regards, Ivaylo > > >> 2. With the way it's modelled, we need to parse phandles from eUSB2 and >> USBDP to the controller. Adding that to the usbdrd driver would be... >> weird. It makes more sense to model it as a separate driver, because >> it functions in a different way. > Just to be clear: we don't talk about drivers here. > > > Best regards, > Krzysztof
