On 3/31/20 3:24 PM, Simon Glass wrote: > Hi Marek, > > On Mon, 30 Mar 2020 at 18:39, Marek Vasut <ma...@denx.de> wrote: >> >> On 3/31/20 1:31 AM, Simon Glass wrote: >>> Hi Marek, >> >> Hi, >> >>> On Sun, 22 Mar 2020 at 09:34, Marek Vasut <ma...@denx.de> wrote: >>>> >>>> On 3/22/20 4:17 PM, Simon Glass wrote: >>>>> Hi Marek, >>>> >>>> Hi, >>>> >>>>> On Sat, 21 Mar 2020 at 20:15, Marek Vasut <ma...@denx.de> wrote: >>>>>> >>>>>> On 3/22/20 3:08 AM, Simon Glass wrote: >>>>>>> Hi Marek, >>>>>> >>>>>> Hi, >>>>>> >>>>>>> I think at this point we've covered all the ground and mentioned the >>>>>>> pros and cons of each method, so I'll leave the discussion where it >>>>>>> is. >>>>>> >>>>>> Great, so let's remove the struct-based access from the driver and use >>>>>> regular #define REGISTER 0xoffset. >>>>> >>>>> I think any individual decision depends on the pros and cons we >>>>> outlined in our discussion. I don't have any information to suggest >>>>> that the Mediatek XHCI driver has any of the variations you talked >>>>> about in your worst-case scenarios, so I can't comment on that. I am >>>>> more concerned about this as a general rule as I feel that the >>>>> struct-based approach is generally best for U-Boot, except for the >>>>> cases you highlighted: >>>>> >>>>> - where the registers appear at different offsets in different >>>>> hardware revisions served by the same driver >>>>> - where the driver only uses a small subset of the registers and it is >>>>> not worth defining a struct to cover them all, with associated empty >>>>> regions, etc. >>>>> >>>>> Anything else? >>>> >>>> It's also very difficult to easily figure out the address of a register >>>> that's buried somewhere down in a long structure, possibly with embedded >>>> sub-structures. >>> >>> OK I have updated the coding style page with all of this. >> >> Which page ? > > https://www.denx.de/wiki/U-Boot/CodingStyle
" U-Boot typically uses a C structure to map out the registers in an I/O region, rather than offsets. The reasons for this are: " is misleading and suggests that using structures is the best practice. This should be reworded to make it clear both options are equally valid. > Separately, I sent a patch a while back which updated checkpatch for > U-Boot. I got some pushback Link ? I am very much opposed to this struct-based approach, so there is my pushback too. I think we should NOT do it. >, but I think that was wrong and we should > do it. For example I am saying many of the same things in code reviews > and many of them could be caught by the script. Examples include using > if() instead of #if where possible. > > Regards, > Simon > -- Best regards, Marek Vasut
