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

[Marek Vasut]

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.

-- 
Best regards,
Marek Vasut