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

[Marek Vasut]

On 01/29/2018 05:56 PM, Jagan Teki wrote:
> 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.

Good!

-- 
Best regards,
Marek Vasut