Re: [PATCH v3 06/10] dt-bindings: phy: mvebu-utmi: add UTMI PHY bindings

From: Miquel Raynal <miquel.raynal@bootlin.com>
To: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Mark Rutland <mark.rutland@arm.com>, Andrew Lunn <andrew@lunn.ch>,
	Jason Cooper <jason@lakedaemon.net>,
	Mathias Nyman <mathias.nyman@intel.com>,
	devicetree@vger.kernel.org,
	Antoine Tenart <antoine.tenart@bootlin.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Gregory Clement <gregory.clement@bootlin.com>,
	linux-usb@vger.kernel.org, Kishon Vijay Abraham I <kishon@ti.com>,
	Nadav Haklai <nadavh@marvell.com>,
	Rob Herring <robh+dt@kernel.org>,
	Alan Stern <stern@rowland.harvard.edu>,
	Maxime Chevallier <maxime.chevallier@bootlin.com>,
	linux-arm-kernel@lists.infradead.org,
	Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Subject: Re: [PATCH v3 06/10] dt-bindings: phy: mvebu-utmi: add UTMI PHY bindings
Date: Tue, 22 Jan 2019 20:16:35 +0100	[thread overview]
Message-ID: <20190122201635.4a6c28d0@xps13> (raw)
In-Reply-To: <20190122183833.40077d7a@windsurf>

Hi Thomas,

Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote on Tue, 22 Jan
2019 18:38:33 +0100:

> Hello,
> 
> On Mon, 21 Jan 2019 12:23:32 +0100, Miquel Raynal wrote:
> 
> > +- compatible: Should be one of:
> > +	      * "marvell,a3700-utmi-host-phy" for the PHY connected to
> > +	        the USB2 host-only controller.
> > +	      * "marvell,a3700-utmi-otg-phy" for the PHY connected to
> > +	        the USB3 and USB2 OTG capable controller.  
> 
> Do we really need different compatible strings for those ? I assume the
> IP block is exactly the same for the two PHYs, right ? If so, we should
> use the same compatible string.

For what I understand, the PHYs are different. At least this is how
they are described in the specification. I can list at least two
differences visible in the register set:
* one has OTG registers, the other does not.
* one has charger detection capabilities (and registers), the other
  does not.

> 
> Didn't we discuss that we can lookup from the PHY whether the USB
> controller it is connected to is in host mode or otg mode ?

Yes, but having two different PHYs meant having two different
compatibles, hence there was no need in the driver to use this
lookup approach. Anyway this is an implementation detail and it is
always possible to go back. 

> 
> > +Example:
> > +
> > +	usb2_utmi_host_phy: phy@5f000 {
> > +		compatible = "marvell,armada-3700-utmi-host-phy";
> > +		reg = <0x5f000 0x800>;
> > +		marvell,usb-misc-reg = <&usb2_syscon>;
> > +		#phy-cells = <0>;
> > +	};
> > +
> > +	usb2_syscon: system-controller@5f800 {
> > +		compatible = "marvell,usb2-host-miscellaneous", "syscon";
> > +		reg = <0x5f800 0x800>;
> > +	};  
> 
> Those registers are contiguous to the register range of the PHY itself.
> What was the criteria used to decide that we need two separate DT nodes
> for these ?

Because this area contains a bunch of registers, most of them are
there to manage the PHY, but others are related to the USB controller
(ie. a software reset). I know the current USB controller driver does
not access this area but what if one day we decide to do it?

Thanks,
Miquèl

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel