Re: [PATCH v1 2/6] usb: chipidea: tegra: Avoid controller/PHY init if bus is externally controlled

[Svyatoslav Ryhel]

вт, 12 трав. 2026 р. о 10:51 Peter Chen (CIX) <peter.chen@xxxxxxxxxx> пише:
>
> On 26-05-12 09:13:40, Svyatoslav Ryhel wrote:
> > вт, 12 трав. 2026 р. о 04:16 Peter Chen (CIX) <peter.chen@xxxxxxxxxx> пише:
> > >
> > > On 26-05-11 16:56:57, Svyatoslav Ryhel wrote:
> > > > If the USB controller and PHY are externally controlled, then the
> > > > registration of the controller and the PHY initialization should be
> > > > skipped, since these configurations must be done by the device that
> > > > controls the bus to work correctly.
> > > >
> > >
> > > I find you only control USB controller device add at PHY driver, most of USB drivers
> > > has PHY control, for chipidea, it has PHY control at core.c, would please try to
> > > adapt for it?
> > >
> >
> > Usually yes, but this is not the case for Tegra unfortunately. As you
> > can see Tegra specific section of Chipidea driver specifically
> > describes why it has to set PHY manually.
> >
> > /*
> >  * USB controller registers shouldn't be touched before PHY is
> >  * initialized, otherwise CPU will hang because clocks are gated.
> >  * PHY driver controls gating of internal USB clocks on Tegra.
> >  */
> >
> > So in order to provide correct work of USB when set by an external
> > device, both PHY and controller init/add must be skipped.
>
> You could call generic PHY APIs at ci_hdrc_tegra.c, after PHY init or power on,
> call controller initialization.
>

I was tinkering with Chipidea driver for Tegra a bit. If you meant to
use flag only to control usb controller device init and leave PHY to
be handled by controller, then yes, that is possible and that is
better approach.

> >
> > > Peter
> > >
> > > > Signed-off-by: Svyatoslav Ryhel <clamor95@xxxxxxxxx>
> > > > ---
> > > >  drivers/usb/chipidea/ci_hdrc_tegra.c | 36 +++++++++++++++++-----------
> > > >  1 file changed, 22 insertions(+), 14 deletions(-)
> > > >
> > > > diff --git a/drivers/usb/chipidea/ci_hdrc_tegra.c b/drivers/usb/chipidea/ci_hdrc_tegra.c
> > > > index 372788f0f970..593390a818d1 100644
> > > > --- a/drivers/usb/chipidea/ci_hdrc_tegra.c
> > > > +++ b/drivers/usb/chipidea/ci_hdrc_tegra.c
> > > > @@ -32,6 +32,7 @@ struct tegra_usb {
> > > >       struct clk *clk;
> > > >
> > > >       bool needs_double_reset;
> > > > +     bool externally_controlled;
> > > >  };
> > > >
> > > >  struct tegra_usb_soc_info {
> > > > @@ -312,20 +313,25 @@ static int tegra_usb_probe(struct platform_device *pdev)
> > > >       if (device_property_present(&pdev->dev, "nvidia,needs-double-reset"))
> > > >               usb->needs_double_reset = true;
> > > >
> > > > +     if (device_property_present(&pdev->dev, "nvidia,external-control"))
> > > > +             usb->externally_controlled = true;
> > > > +
> > > >       err = tegra_usb_reset_controller(&pdev->dev);
> > > >       if (err) {
> > > >               dev_err_probe(&pdev->dev, err, "failed to reset controller");
> > > >               goto fail_power_off;
> > > >       }
> > > >
> > > > -     /*
> > > > -      * USB controller registers shouldn't be touched before PHY is
> > > > -      * initialized, otherwise CPU will hang because clocks are gated.
> > > > -      * PHY driver controls gating of internal USB clocks on Tegra.
> > > > -      */
> > > > -     err = usb_phy_init(usb->phy);
> > > > -     if (err)
> > > > -             goto fail_power_off;
> > > > +     if (!usb->externally_controlled) {
> > > > +             /*
> > > > +              * USB controller registers shouldn't be touched before PHY is
> > > > +              * initialized, otherwise CPU will hang because clocks are gated.
> > > > +              * PHY driver controls gating of internal USB clocks on Tegra.
> > > > +              */
> > > > +             err = usb_phy_init(usb->phy);
> > > > +             if (err)
> > > > +                     goto fail_power_off;
> > > > +     }
> > > >
> > > >       /* setup and register ChipIdea HDRC device */
> > > >       usb->soc = soc;
> > > > @@ -342,12 +348,14 @@ static int tegra_usb_probe(struct platform_device *pdev)
> > > >       if (of_usb_get_phy_mode(pdev->dev.of_node) == USBPHY_INTERFACE_MODE_ULPI)
> > > >               usb->data.flags &= ~CI_HDRC_SUPPORTS_RUNTIME_PM;
> > > >
> > > > -     usb->dev = ci_hdrc_add_device(&pdev->dev, pdev->resource,
> > > > -                                   pdev->num_resources, &usb->data);
> > > > -     if (IS_ERR(usb->dev)) {
> > > > -             err = dev_err_probe(&pdev->dev, PTR_ERR(usb->dev),
> > > > -                                 "failed to add HDRC device");
> > > > -             goto phy_shutdown;
> > > > +     if (!usb->externally_controlled) {
> > > > +             usb->dev = ci_hdrc_add_device(&pdev->dev, pdev->resource,
> > > > +                                           pdev->num_resources, &usb->data);
> > > > +             if (IS_ERR(usb->dev)) {
> > > > +                     err = dev_err_probe(&pdev->dev, PTR_ERR(usb->dev),
> > > > +                                         "failed to add HDRC device");
> > > > +                     goto phy_shutdown;
> > > > +             }
> > > >       }
> > > >
> > > >       return 0;
> > > > --
> > > > 2.51.0
> > > >
> > >
> > > --
> > >
> > > Best regards,
> > > Peter
>
> --
>
> Best regards,
> Peter