diff mbox series

[v1,4/5] usb: dwc3-meson-g12a: support OTG switch

Message ID 20230414152423.19842-5-ddrokosov@sberdevices.ru
State New
Headers
Series arm64: meson: support Amlogic A1 USB OTG controller |

Commit Message

Dmitry Rokosov April 14, 2023, 3:24 p.m. UTC 
  From now, the Amlogic A1 USB controller is capable of switching between
host and gadget modes, based on the status of the OTG_ID signal or
by manual usb role changing.

Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
---
 drivers/usb/dwc3/dwc3-meson-g12a.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Martin Blumenstingl April 16, 2023, 8:56 p.m. UTC | #1 
On Fri, Apr 14, 2023 at 5:24 PM Dmitry Rokosov <ddrokosov@sberdevices.ru> wrote:
[...]
>  static const struct dwc3_meson_g12a_drvdata a1_drvdata = {
> -       .otg_switch_supported = false,
> +       .otg_switch_supported = true,
it would be great if you could also follow up with a patch that
removes otg_switch_supported.
A1 was the only variant that needed it and after this patch it's just dead code.


Best regards,
Martin
Dmitry Rokosov April 17, 2023, 11:47 a.m. UTC | #2 
Hello Martin,

Thank you for quick review, appreciate it!
Please find my comments below and in the other replies.

On Sun, Apr 16, 2023 at 10:56:36PM +0200, Martin Blumenstingl wrote:
> On Fri, Apr 14, 2023 at 5:24 PM Dmitry Rokosov <ddrokosov@sberdevices.ru> wrote:
> [...]
> >  static const struct dwc3_meson_g12a_drvdata a1_drvdata = {
> > -       .otg_switch_supported = false,
> > +       .otg_switch_supported = true,
> it would be great if you could also follow up with a patch that
> removes otg_switch_supported.
> A1 was the only variant that needed it and after this patch it's just dead code.

It makes sense. I thought about it before sending the first version, but
I found a counter-argument: future SoCs may use this parameter.
But if you ask, I will remove 'otg_switch_supported' in the next version
Neil Armstrong April 17, 2023, 12:22 p.m. UTC | #3 
On 17/04/2023 13:47, Dmitry Rokosov wrote:
> Hello Martin,
> 
> Thank you for quick review, appreciate it!
> Please find my comments below and in the other replies.
> 
> On Sun, Apr 16, 2023 at 10:56:36PM +0200, Martin Blumenstingl wrote:
>> On Fri, Apr 14, 2023 at 5:24 PM Dmitry Rokosov <ddrokosov@sberdevices.ru> wrote:
>> [...]
>>>   static const struct dwc3_meson_g12a_drvdata a1_drvdata = {
>>> -       .otg_switch_supported = false,
>>> +       .otg_switch_supported = true,
>> it would be great if you could also follow up with a patch that
>> removes otg_switch_supported.
>> A1 was the only variant that needed it and after this patch it's just dead code.
> 
> It makes sense. I thought about it before sending the first version, but
> I found a counter-argument: future SoCs may use this parameter.
> But if you ask, I will remove 'otg_switch_supported' in the next version
> 

Please remove it, it's easy to add it again if needed.

Neil
Dmitry Rokosov April 17, 2023, 12:29 p.m. UTC | #4 
Hello Neil,

Thank you for review, appreciate it!

On Mon, Apr 17, 2023 at 02:22:26PM +0200, neil.armstrong@linaro.org wrote:
> On 17/04/2023 13:47, Dmitry Rokosov wrote:
> > Hello Martin,
> > 
> > Thank you for quick review, appreciate it!
> > Please find my comments below and in the other replies.
> > 
> > On Sun, Apr 16, 2023 at 10:56:36PM +0200, Martin Blumenstingl wrote:
> > > On Fri, Apr 14, 2023 at 5:24 PM Dmitry Rokosov <ddrokosov@sberdevices.ru> wrote:
> > > [...]
> > > >   static const struct dwc3_meson_g12a_drvdata a1_drvdata = {
> > > > -       .otg_switch_supported = false,
> > > > +       .otg_switch_supported = true,
> > > it would be great if you could also follow up with a patch that
> > > removes otg_switch_supported.
> > > A1 was the only variant that needed it and after this patch it's just dead code.
> > 
> > It makes sense. I thought about it before sending the first version, but
> > I found a counter-argument: future SoCs may use this parameter.
> > But if you ask, I will remove 'otg_switch_supported' in the next version
> > 
> 
> Please remove it, it's easy to add it again if needed.

Sure, no problem. It will be removed in the next version.
diff mbox series

Patch

diff --git a/drivers/usb/dwc3/dwc3-meson-g12a.c b/drivers/usb/dwc3/dwc3-meson-g12a.c
index b282ad0e69c6..10469b95deb9 100644
--- a/drivers/usb/dwc3/dwc3-meson-g12a.c
+++ b/drivers/usb/dwc3/dwc3-meson-g12a.c
@@ -242,7 +242,7 @@  static const struct dwc3_meson_g12a_drvdata g12a_drvdata = {
 };
 
 static const struct dwc3_meson_g12a_drvdata a1_drvdata = {
-	.otg_switch_supported = false,
+	.otg_switch_supported = true,
 	.clks = meson_a1_clocks,
 	.num_clks = ARRAY_SIZE(meson_a1_clocks),
 	.phy_names = meson_a1_phy_names,