[PATCH 5/8] xhci: mediatek: Add support for MTK xHCI host controller

[Marek Vasut]

On 3/21/20 5:42 PM, Simon Glass wrote:
> Hi Marek,

Hi,

> On Sat, 21 Mar 2020 at 09:09, Marek Vasut <marex at denx.de> wrote:
>>
>> On 3/21/20 3:12 PM, Simon Glass wrote:
>>> Hi Marek,
>>>
>>> On Wed, 11 Mar 2020 at 01:11, Marek Vasut <marex at denx.de> wrote:
>>>>
>>>> On 3/11/20 7:50 AM, Chunfeng Yun wrote:
>>>> [...]
>>>>> + * @u3_ctrl_p[x]: ip usb3 port x control register, only low 4bytes are used
>>>>> + * @u2_ctrl_p[x]: ip usb2 port x control register, only low 4bytes are used
>>>>> + * @u2_phy_pll: usb2 phy pll control register
>>>>> + */
>>>>> +struct mtk_ippc_regs {
>>>>> +     __le32 ip_pw_ctr0;
>>>>> +     __le32 ip_pw_ctr1;
>>>>> +     __le32 ip_pw_ctr2;
>>>>
>>>> Please define the registers with #define macros , this struct-based
>>>> approach doesn't scale.
>>>>
>>>
>>> What does this mean? I much prefer the struct approach, unless the
>>> registers move around in future generations.
>>
>> This means I want to see #define MTK_XHCI_<register name> 0xf00
>>
>> The struct based approach doesn't scale and on e.g. altera is causing us
>> a massive amount of problems now. It looked like a good idea back then,
>> but now it's a massive burden, so I don't want that to proliferate. And
>> here I would expect the registers to move around, just like everywhere else.
> 
> That sounds like something that is easily avoided. For example,
> Designware doesn't seem to move things around.
> 
> Perhaps MediaTek has a policy of not doing this either?

Such policies change as the hardware evolves. I got burned by this
struct-based approach more often then it was beneficial, if it ever
really was beneficial, hence I don't want to see it used.