Re: [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB

Kishon Vijay Abraham I
2016-09-19T04:59:27Z

Hi,

On Monday 19 September 2016 01:26 AM, Martin Blumenstingl wrote:
> Hi Kishon,
>
> On Fri, Sep 16, 2016 at 10:19 AM, Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org> wrote:
>> Hi,
>>
>> On Friday 09 September 2016 09:44 PM, Martin Blumenstingl wrote:
>>> On Fri, Sep 9, 2016 at 5:33 PM, Kevin Hilman <khilman-rdvid1DuHRBWk0Htik3J/w@public.gmane.org> wrote:
>>>> However, the problem with all of the solutions proposed (runtime PM ones
>>>> included) is that we're forcing a board-specific design issue (2 devices
>>>> sharing a reset line) into a driver that should not have any
>>>> board-specific assumptions in it.
>>>>
>>>> For example, if this driver is used on another platform where different
>>>> PHYs have different reset lines, then one of them (the unlucky one who
>>>> is not probed first) will never get reset. So any form of per-device
>>>> ref-counting is not a portable solution.
>>> maybe we should also consider Ben's solution: he played with the USB
>>> PHY on his Meson8b board. His approach was to have only one USB PHY
>>> driver instance which exposes two PHYs.
>>> The downside of this: the driver would have to know the offset of the
>>> PHYs (0x0 for the first PHY, 0x20 for the second), but we could handle
>>> the reset using runtime PM without any hacks.
>>
>> I think the offset information can come from the devicetree too. The phy can be
>> modeled something like below.
>>
>> usb-phys@c0000000 {
>> compatible = "amlogic,meson-gxbb-usb2-phy";
>> reg = <0x0 0xc0000000 0x0 0x40>;
>> #address-cells = <2>;
>> #size-cells = <2>;
>> ranges = <0x0 0x0 0x0 0xc0000000 0x0 0x40>;
>> resets = <&reset 34>;
>>
>> usb0_phy: usb_phy@0 {
>> #phy-cells = <0>;
>> reg = <0x0 0x0 0x0 0x20>;
>> clocks = <&clkc CLKID_USB &clkc CLKID_USB0>;
>> clock-names = "usb_general", "usb";
>> status = "disabled";
>> };
>>
>> usb1_phy: usb_phy@20 {
>> #phy-cells = <0>;
>> reg = <0x0 0x20 0x0 0x20>;
>> clocks = <&clkc CLKID_USB &clkc CLKID_USB1>;
>> clock-names = "usb_general", "usb";
>> status = "disabled";
>> };
>> };
>>
>> This way the driver will be probed only once (the reset can be done during
>> probe). The phy driver should scan the dt node and for every sub-node it
>> invokes phy_create?
> I'll recap what we have discussed so far (so you don't have to re-read
> the whole thread):
> The reference driver treats both USB PHYs as separate devices (the
> datasheet has no information about the PHYs though). The only
> "special" thing is the shared reset line -> together with Philipp
> Zabel (the reset framework maintainer) we decided to make
> reset_control_reset work for shared reset lines.
>
> That means we can keep the two PHYs as separate devices inside the
> .dts, while keeping everything else separate (just like the reference
> driver)
> Is this fine for you and Arnd?

yeah.. I'm fine with that.

Thanks
Kishon
>
>
> Regards,
> Martin
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in