[PATCH] usb: dwc3: am62: Use UCLASS_NOP for glue driver
Siddharth Vadapalli
s-vadapalli at ti.com
Thu Feb 27 11:25:01 CET 2025
On Thu, Feb 27, 2025 at 11:19:37AM +0100, Mattijs Korpershoek wrote:
> Hi Siddharth,
>
> Thank you for the the review.
>
> On jeu., févr. 27, 2025 at 13:44, Siddharth Vadapalli <s-vadapalli at ti.com> wrote:
>
> > On Wed, Feb 26, 2025 at 03:47:11PM +0100, Mattijs Korpershoek wrote:
> >
> > Hello Mattijs,
> >
> >> The dwc3 driver allows SoC-specific configuration via the dwc3_glue_ops
> >> structure.
> >> These glue drivers usually just implement the .glue_configure function.
> >
> > nitpick: Is there a reason behind an abrupt newline above? The sentence:
> > "These glue drivers..." could continue on the same line as the previous
> > sentence. Also, "These glue drivers..." is referring to glue drivers
> > which haven't been mentioned yet and it is the first time that they are
> > being referred to. It is not clear to me if there was more content in
> > between the first sentence and the second which accidentally got
> > removed.
>
> I'll rephrase as:
>
> """
> The dwc3 driver allows SoC-specific configuration via the dwc3_glue_ops
> structure. Glue drivers, which implement dwc3_glue_ops, usually just
> define the .glue_configure() function.
> """
>
> Does that looks good ?
Yes.
>
> >
> >>
> >> For such glue drivers, we don't need to be a SIMPLE_BUS, since we
> >> don't interact with any child devices.
> >
> > This sounds confusing. What does "we" refer to? The dwc3-am62.c driver
> > which is being modified in this patch is *the* glue driver itself.
> > "For such glue drivers" and "we don't need to be a SIMPLE_BUS" seem to
> > be referring to two different entities when they are probably the same.
> >
> > "For such glue drivers" => dwc3-am62.c
> > "we don't need to be a SIMPLE_BUS" => Again dwc3-am62.c ?
>
> I'll rephrase as:
>
> """
> dwc3-am62 is such glue driver which does not need to be a SIMPLE_BUS
> since it does not interact with any child device.
> """
>
> Is that okay ?
Yes.
>
> >
> > Did you mean something like the following?
> > "Glue drivers don't need to be a SIMPLE_BUS since they don't interact
> > with any child devices"
>
> I can't write that down, because that's not true. Some glue drivers (like
> dwc3-meson-g12a) need to be SIMPLE_BUS because they rely on
> .child_pre_probe() and .child_post_remove().
Understood. Thank you for clarifying and for rephrasing the commit
message above. Please feel free to add my Reviewed-by tag when posting
the v2 patch.
Regards,
Siddharth.
More information about the U-Boot
mailing list