mbox series

[v1,0/5] arm64: meson: support Amlogic A1 USB OTG controller

Message ID 20230414152423.19842-1-ddrokosov@sberdevices.ru
Headers show
Series arm64: meson: support Amlogic A1 USB OTG controller | expand

Message

Dmitry Rokosov April 14, 2023, 3:24 p.m. UTC
This patch series introduces full support for the Amlogic A1 USB controller
in OTG mode (peripheral and host modes switching).

Previously, Amlogic's patch series [1] was applied to the upstream tree,
but it only had USB host mode support.
Furthermore, the device tree patchset [2] wasn't merged due to a missing
clk driver.
Patchset [2] has been completely reworked:
    - changed register base offsets to proper values
    - introduced dwc2 in peripheral mode
    - OTG mode support
    - the SoB of Amlogic authors still remain

Testing:
    - USB OTG role switching between gadget and host - OK
    - Peripheral mode - OK (tested with adb shell/push/pop)
    - Host mode - OK (tested only USB enumeration and detection)

Links:
    [1] https://lore.kernel.org/all/1581990859-135234-1-git-send-email-hanjie.lin@amlogic.com/
    [2] https://lore.kernel.org/all/1581990859-135234-4-git-send-email-hanjie.lin@amlogic.com/

Dmitry Rokosov (5):
  phy: amlogic: during USB PHY clkin obtaining, enable it
  usb: dwc2: support dwc2 IP for Amlogic A1 SoC family
  dt-bindings: usb: dwc2: add support for Amlogic A1 SoC USB peripheral
  usb: dwc3-meson-g12a: support OTG switch
  arm64: dts: meson: a1: support USB controller in OTG mode

 .../devicetree/bindings/usb/dwc2.yaml         |  1 +
 arch/arm64/boot/dts/amlogic/meson-a1.dtsi     | 59 +++++++++++++++++++
 drivers/phy/amlogic/phy-meson-g12a-usb2.c     |  2 +-
 drivers/usb/dwc2/params.c                     | 21 +++++++
 drivers/usb/dwc3/dwc3-meson-g12a.c            |  2 +-
 5 files changed, 83 insertions(+), 2 deletions(-)

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.