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

Simon Glass sjg at chromium.org
Tue Mar 31 15:24:59 CEST 2020


Hi Marek,

On Mon, 30 Mar 2020 at 18:39, Marek Vasut <marex at 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 <marex at 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 <marex at 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

Separately, I sent a patch a while back which updated checkpatch for
U-Boot. I got some pushback, 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


More information about the U-Boot mailing list