On 3/21/20 5:42 PM, Simon Glass wrote: > Hi Marek, Hi,
> On Sat, 21 Mar 2020 at 09:09, Marek Vasut <ma...@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 <ma...@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.