[U-Boot] [PATCH v3 03/30] sunxi: Fix USB PHY index for H3/H5/A64

[Jagan Teki]

On Mon, Jan 29, 2018 at 10:04 PM, Marek Vasut <marex at denx.de> wrote:
> On 01/28/2018 07:40 PM, Marek Vasut wrote:
>> On 01/28/2018 07:29 PM, Jagan Teki wrote:
>>> On Sun, Jan 28, 2018 at 11:55 PM, Marek Vasut <marex at denx.de> wrote:
>>>> On 01/28/2018 07:20 PM, Jagan Teki wrote:
>>>>> On Sun, Jan 28, 2018 at 10:01 PM, Marek Vasut <marex at denx.de> wrote:
>>>>>> On 01/28/2018 05:19 PM, Jagan Teki wrote:
>>>>>>> From: Chen-Yu Tsai <wens at csie.org>
>>>>>>>
>>>>>>> On the new chips such as H3, H5, and A64, the USB OTG controller is
>>>>>>> paired with a set of proper EHCI/OHCI USB hosts. To enable these hosts,
>>>>>>> the USB PHY index count has to be reworked to start from this pair.
>>>>>>>
>>>>>>> This patch reworks the USB clock gate and reset indices, and how the
>>>>>>> USB host is mapped to a USB phy, for the newer chips.
>>>>>>
>>>>>> The ifdeffery is awful. The driver is DT capable, do why don't you
>>>>>> detect the block type / soc type from DT and handle this dynamically
>>>>>> instead of adding ifdefs ?
>>>>>
>>>>> Though this driver is DT capable phy, reset, clock and other still
>>>>> need to have have it. till now we are relying on ifdef's to move
>>>>> feature to work first.
>>>
>>>>
>>>> This statement makes no sense, just use the DT compatible to discern the
>>>> block type and get rid of statements like this:
>>>>
>>>> +#if defined(CONFIG_MACH_SUNXI_H3_H5) || defined(CONFIG_MACH_SUN50I)
>>>> +       /* Newer chips have a EHCI/OHCI host pair for OTG host mode */
>>>> +       priv->phy_index = ((uintptr_t)hccr - SUNXI_USB0_BASE) / BASE_DIST;
>>>> +#else
>>>>         priv->phy_index = ((uintptr_t)hccr - SUNXI_USB1_BASE) / BASE_DIST;
>>>> +#endif
>>>
>>> if you read it previous comment clearly for "phy, reset, clock" writes
>>> it may not possible and true for above ifdef.
>>
>> I only see if (platform is foo or bar) set phy_index to something else
>> set it to something else, therefore I think the comment about 'phy,
>> reset, clock' is irrelevant.
>>
>> You can very well add ie.
>> { .compatible = "allwinner,sun5i-a13-ohci", .data = TYPE_FOO },
>>
>> And then do
>>
>> if (dev_get_driver_data(...) == TYPE_FOO)
>>   priv->phy_index = bar;
>> else
>>   priv->.......;
>>
>> If you think this is not possible, please do explain why in detail.
>
> Can I expect a new patchset using DT properly ?

Yes Marek, I'm working on infact I'm trying to get these SOC's specs
out from musb and writing phy driver for that matter.