Re: [PATCH v1 4/9] pinctrl: tegra-xusb: Add USB PHY support

[Stephen Warren]

On 06/27/2014 09:00 AM, Felipe Balbi wrote:
> On Wed, Jun 25, 2014 at 04:30:48PM -0700, Andrew Bresticker wrote:
>>>> +static int usb3_phy_power_on(struct phy *phy)
>>>> +{
>>>> +     struct tegra_xusb_padctl *padctl = phy_get_drvdata(phy);
>>>> +     int port = usb3_phy_to_port(phy);
>>>> +     int lane = padctl->usb3_ports[port].lane;
>>>> +     u32 value, offset;
>>>> +
>>>> +     value = padctl_readl(padctl, XUSB_PADCTL_IOPHY_USB3_PADX_CTL2(port));
>>>> +     value &= ~((XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_RX_WANDER_MASK <<
>>>> +                 XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_RX_WANDER_SHIFT) |
>>>> +                (XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_RX_EQ_MASK <<
>>>> +                 XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_RX_EQ_SHIFT) |
>>>> +                (XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_CDR_CNTL_MASK <<
>>>> +                 XUSB_PADCTL_IOPHY_USB3_PAD_CTL2_CDR_CNTL_SHIFT));
>>>
>>> Hmm. So there is a lot of "PHY" stuff here after all.
>>>
>>> However, the PHYs implemented here appear to implement very low-level
>>> I/O pad code, whereas the PHYs we have for our USB 2.0 controller are
>>> somewhat higher-level; they're more USB-oriented than just IO pad
>>> oriented. Do you know which level of abstraction a Linux PHY object is
>>> supposed to be? I could never get an answer when I asked before.
>>
>> The only other PHY driver I've worked with (Exynos USB2/3 PHYs) also
>> mainly only did low-level pad control stuff, but looking at a couple
>> of other USB PHYs (MSM, MV), there appear to be others that have
>> higher-level USB stuff in the PHY driver.  Perhaps Kishon or Felipe
>> could offer us some guidance?
> 
> well, if you're adding a new driver, I'd rather see folks moving over to
> the generic phy framework (drivers/phy) because we're trying really hard
> to get rid of drivers/usb/phy/. And I think, in your case, it's actually
> ok to use pinctrl because you actually are muxing pads to the USB3 PHY,
> you just do it lazyly.

What I'm looking for is a good definition of exactly what a PHY is
supposed to be in Linux.

Is it purely something that turns some IO pads/drivers off/on, and
nothing more?

Or, does the PHY concept encompass protocol-specific concepts such as
USB VBUS enable, USB VBUS detection, USB OTG switching,
UTMI-vs-ULPI-vs-HSIC selection... all of which are irrelevant of the PHY
(or at least IO pads) are used for SATA or PCIe instead.

This obviously affects which code goes in the PHY driver, and which in
the EHCI/XHCI controller driver.

Attachment: signature.asc
Description: OpenPGP digital signature