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

["Peter Chen (CIX)" <peter.chen@kernel.org>]

On 26-05-12 09:13:40, Svyatoslav Ryhel wrote:
> вт, 12 трав. 2026 р. о 04:16 Peter Chen (CIX) <peter.chen@kernel.org> пише:
> >
> > 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.

> 
> > Peter
> >
> > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > ---
> > >  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