On Wed, Dec 04, 2024 at 02:34:56PM +0200, Roger Quadros wrote: > > > On 04/12/2024 12:05, Siddharth Vadapalli wrote:
[...] > > Why not do what this patch does? What is the issue with the current > > patch (apart from the checks, which I could retain as well if others > > wish so)? The caller clearly mentions the expected role, so why not use > > that? > > Because the patch is more complicated than it needs to be, now that we know: > 1) generic_host_probe() will only be called when dr_mode is "host", > 2) generic_peripheral_probe() will only be called when dr_mode is "peripheral" > or "otg". > > The problem case is generic_peripheral_probe() being called with when > dr_mode is "otg". We can also be sure that generic_periperhal_probe() expects dr_mode to be "peripheral" only and not "otg". So why not fix it at the root, which is what this patch does? Independent of the issue, is the following a valid use-case? generic_peripheral_probe() with plat->dr_mode = "otg" and hence generic_probe() using "otg" rather than "peripheral" as is the case now. Is this resulting in something functional for other users of the USB Designware Controller? If the user requests "peripheral" mode of operation, will "otg" meet the user's requirement? If yes, I have no further objections. Irrespective of this, if you believe that the fix should be in the core as you indicated below, please feel free to post the patch. > > Another alternative could be to override dr_mode to "peripheral" if > it was "otg" in dwc3_generic_probe before calling dwc3_init() but > I don't prefer that as we are loosing platform information here > and it will have to be reverted when dual-role support is added in core.c. > > So better to deal with this in core.c and add a note why we are doing it. > [...] Regards, Siddharth.
