On 3/21/20 7:41 PM, Simon Glass wrote:
Hi Marek,
Hi,
On Sat, 21 Mar 2020 at 11:00, Marek Vasut marex@denx.de wrote:
On 3/21/20 5:42 PM, Simon Glass wrote:
Hi Marek,
Hi,
On Sat, 21 Mar 2020 at 09:09, Marek Vasut marex@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@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.
Some benefits:
- using C struct instead of 'assembler-like' addition is less error-prone
In my experience, this is exactly the opposite. If I have a long structure describing some register set and I have to count lines to figure out which register it is at offset e.g. 0x124 , then I am very likely to make a mistake. However, if I have a macro which looks like #define REG_FOO 0x124 , I see it right away.
And I am not even talking about modifying such structures, where one has to over and over recount and verify that nothing got padded the wrong way. Worse yet, that the structure might just be embedded in some other super-structure, in which case the superstructure gets padded differently if the sub-structure changes in size.
Let alone the problematic fact that you might have multiple versions of the same IP in the system, with just slightly different register layouts, at which point the structures become ridden with unions {} and it becomes a total hell.
So no, this argument I am not buying, sorry.
Also, how is C preprocessor macro assembler like ?
- you can pass the regs struct around between functions in the driver,
particularly helpful in large drivers
You can pass base addresses around too.
- lower-case letters are reasier to read
That's a matter of taste.
I find it much easier to identify register offsets (and other macros) among functions if the offsets are written in uppercase while functions in lowercase.
- you can specify the data size and endianness in the struct using C types
Yes, you can, which is not a benefit but another problem, esp. if that structure gets used on systems where CPU and bus endianness can differ. (take a look e.g. at the macro monstrosity that is NS16550)
If the hardware changes enough, then you need a new driver anyway, or perhaps separate C implementations of the low-level register access if the high-level flow is similar.
Not really, there is a lot of hardware where registers just move around, but existing driver can very well handle all such instances.
In general you can't always determine this sort of thing at compile time since the version of the hardware may not be apparent until run-time. You end up with variables holding the offset of each register.
You can use some/any sort of register abstraction to do the remapping. This is completely independent of how you represent the register offsets.
Sometimes is it better to have an umbrella driver with a common core.
So I would push back against a move in this direction in general. It depends on the circumstances, and anyway, converting from struct to offsets if desirable later is not that hard.
NAK, I am opposed to the struct based access, sorry.
We had massive problems moving toward it in multiple areas (socfpga was hit very bad and I regret it to this day), only to move back from it these days because it is borderline impossible to accommodate newly emerging hardware and we suffered problems outlined above often.
If you want one more argument, then Linux is not using this anywhere and it has much larger driver base. One might expect Linux to be somewhat ahead of U-Boot in adopting such techniques.