Re: [PATCH v2 1/7] usb: dwc3: get "usb_phy" only if the platformindicates the presence of PHY's

[Vivek Gautam]

On Tue, Nov 5, 2013 at 11:41 PM, Vivek Gautam <gautamvivek1987@xxxxxxxxx> wrote:
> Dear Kishon, Roger
>
>
> On Wed, Oct 16, 2013 at 6:40 PM, Kishon Vijay Abraham I <kishon@xxxxxx> wrote:
>> Hi roger,
>>
>> On Wednesday 16 October 2013 06:33 PM, Roger Quadros wrote:
>>> Hi Kishon,
>
> Apologies for missing this thread for so long.
>
>>>
>>> On 10/15/2013 10:54 PM, Kishon Vijay Abraham I wrote:
>>>> There can be systems which does not have a external usb_phy, so get
>>>> usb_phy only if dt data indicates the presence of PHY in the case of dt boot or
>>>> if platform_data indicates the presence of PHY. Also remove checking if
>>>> return value is -ENXIO since it's now changed to always enable usb_phy layer.
>>>>
>>>> Signed-off-by: Kishon Vijay Abraham I <kishon@xxxxxx>
>>>> ---
>>>> In usb_get_phy_by_phandle, index 0 always refers to usb2 phy and index 1 always
>>>> refers to usb3 phy. Since we've lived so long with this, this patch will make
>>>> an assumption that if only one entry is populated in *usb-phy* property, it will
>>>> be usb2 phy and the next entry will be usb3 phy.
>>>>
>>>>  drivers/usb/dwc3/Kconfig         |    1 +
>>>>  drivers/usb/dwc3/core.c          |   72 ++++++++++++++++++++------------------
>>>>  drivers/usb/dwc3/platform_data.h |    2 ++
>>>>  3 files changed, 41 insertions(+), 34 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
>>>> index 70fc430..8e385b4 100644
>>>> --- a/drivers/usb/dwc3/Kconfig
>>>> +++ b/drivers/usb/dwc3/Kconfig
>>>> @@ -1,6 +1,7 @@
>>>>  config USB_DWC3
>>>>      tristate "DesignWare USB3 DRD Core Support"
>>>>      depends on (USB || USB_GADGET) && HAS_DMA
>>>> +    select USB_PHY
>>>>      select USB_XHCI_PLATFORM if USB_SUPPORT && USB_XHCI_HCD
>>>>      help
>>>>        Say Y or M here if your system has a Dual Role SuperSpeed
>>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>>> index 474162e..cb91d70 100644
>>>> --- a/drivers/usb/dwc3/core.c
>>>> +++ b/drivers/usb/dwc3/core.c
>>>> @@ -354,6 +354,7 @@ static int dwc3_probe(struct platform_device *pdev)
>>>>      struct device_node      *node = dev->of_node;
>>>>      struct resource         *res;
>>>>      struct dwc3             *dwc;
>>>> +    int                     count;
>>>>
>>>>      int                     ret = -ENOMEM;
>>>>
>>>> @@ -387,16 +388,49 @@ static int dwc3_probe(struct platform_device *pdev)
>>>>      if (node) {
>>>>              dwc->maximum_speed = of_usb_get_maximum_speed(node);
>>>>
>>>> -            dwc->usb2_phy = devm_usb_get_phy_by_phandle(dev, "usb-phy", 0);
>>>> -            dwc->usb3_phy = devm_usb_get_phy_by_phandle(dev, "usb-phy", 1);
>>>> +            count = of_count_phandle_with_args(node, "usb-phy", NULL);
>>>> +            switch (count) {
>>>> +            case 2:
>>>> +                    dwc->usb3_phy = devm_usb_get_phy_by_phandle(dev,
>>>> +                            "usb-phy", 1);
>>>> +                    if (IS_ERR(dwc->usb3_phy)) {
>>>> +                            dev_err(dev, "usb3 phy not found\n");
>>>> +                            return PTR_ERR(dwc->usb3_phy);
>>>> +                    }
>>>> +            case 1:
>>>> +                    dwc->usb2_phy = devm_usb_get_phy_by_phandle(dev,
>>>> +                            "usb-phy", 0);
>>>> +                    if (IS_ERR(dwc->usb2_phy)) {
>>>> +                            dev_err(dev, "usb2 phy not found\n");
>>>> +                            return PTR_ERR(dwc->usb2_phy);
>>>> +                    }
>>>> +                    break;
>>>
>>> In the Exynos case, there is only 1 phy and it is the USB3 phy. This code
>>> will wrongly treat it as usb2_phy.
>
> Thank you Roger for your concern regarding Exynos case.
> It's true that, Exynos5 series of SoCs have got only one IP block for
> DWC3'c PHY.
> This block is actually a combo of USB2 phy (UTMI+) as well as USB3 phy (PIPE3).
> And the same is served by a single USB phy driver. This is also
> clarified in the thread : https://lkml.org/lkml/2013/11/5/160
>
>>
>> That was the case even before this patch no? Unfortunately the old USB PHY
>> library doesn't have APIs to get PHYs in a better way. If we try modifying the
>> USB PHY library, it'll be kind of duplicating what is already there in the
>> Generic PHY library. I'd rather prefer Exynos guys to use the new framework.

we have tried moving Samsung's USB phy controller driver for DWC3, to
generic phy framework
so that things look clearer on Samsung's side too. The necessary
patches are availble at:
https://lkml.org/lkml/2013/10/31/79

>>
>> Thanks
>> Kishon
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Best Regards
> Vivek Gautam
> Samsung R&D Institute, Bangalore
> India



-- 
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/