Re: [PATCH v6] phy: Add USB3 PHY support to Google Tensor SoC USB PHY driver
From: RD Babiera
Date: Tue Sep 08 2026 - 19:32:14 EST
Hi Neill, thanks for taking a look. Sorry for the delay here, the feedback
arrived right before I had left for vacation so I wanted to make sure I had
proper time to look at it:
On Tue, Aug 25, 2026 at 11:24 AM Neill Kapron <nkapron@xxxxxxxxxx> wrote:
> Do we really need a 2.5s timeout?
2.5s is actually the hardware default for this transaction, which I agree is
unnecessarily long—I set it here for correctness. We use software polling
instead of the hardware interrupt, and in the product's lifetime 100ms is
more than enough time for the ACK signal to arrive. I'll update the next
patch version to use this value. I'll also program
TCA_CTRLSYNCMODE_CFG1.xa_timeout_val to match the SW
poll timeout so we don't let the operation hang on the PHY when SW
finishes its poll.
> The dev_* calls throughout this patch should have trailing newline
> characters.
Thanks for the heads-up, I will add them.
> > +
> > + pm_runtime_put_autosuspend(gphy->dev);
>
> Here you are using the _autosuspend variant, but autosuspend is never
> configured in probe(). Should this be changed to the plain
> pm_runtime_put(), or actually enable autosuspend in the probe function?
>
> > +
> > + return ret;
> > }
That's fair, I'll commit to pm_runtime_put() here given we don't implement
suspend/resume on the PHY itself.
> I think prior to this we should add a check if phy_state is
> COMBO_PHY_IDLE, return an error and change this check to phy_state ==
> COMBO_PHY_INIT_DONE to make sure we are explicit in our state handling
> to prevent issues with any potential refactoring.
Makes sense to me, this will also allow power_on to only run once in the
PHY init/exit lifetime as well.
Best,
RD