Re: [PATCH v5 3/3] xhci: tegra: USB2 pad power controls

[Jim Lin]

On Thu, 2022-10-27 at 16:00 +0200, Greg KH wrote:
> External email: Use caution opening links or attachments
> 
> 
> On Thu, Oct 27, 2022 at 09:31:27PM +0800, Jim Lin wrote:
> > Program USB2 pad PD controls during port connect/disconnect, port
> > suspend/resume, and test mode, to reduce power consumption on
> > disconnect or suspend.
> > 
> > Signed-off-by: Petlozu Pravareshwar <petlozup@xxxxxxxxxx>
> > Signed-off-by: JC Kuo <jckuo@xxxxxxxxxx>
> > Signed-off-by: Jim Lin <jilin@xxxxxxxxxx>
> 
> Who is the author here?  These do not seem to be in the correct order
> if
> you are the author, right?
> > This is an old patch. Each time went with some small modification.


Petlozu is author for local Kernel 3.18

Then JC for local Kernel 4.4
Now my turn for Kernel 5.xx


> 
> > 
> > ---
> > v2: Fix issue that wrong tegra->phys[] may be accessed on tegra124
> > v3: No change on copyright
> > v4: Remove hcd_to_tegra_xusb() function which is used only once.
> > v5: Update .hub_control in tegra_xhci_overrides (xhci-tegra.c)
> >     Invoke xhci_hub_control() directly (xhci-tegra.c)
> > 
> >  drivers/usb/host/xhci-tegra.c | 131
> > +++++++++++++++++++++++++++++++++-
> >  1 file changed, 130 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-
> > tegra.c
> > index c8af2cd2216d..f685bb7459ba 100644
> > --- a/drivers/usb/host/xhci-tegra.c
> > +++ b/drivers/usb/host/xhci-tegra.c
> > @@ -189,6 +189,13 @@ struct tegra_xusb_context_soc {
> >       } fpci;
> >  };
> > 
> > +enum tegra_xhci_phy_type {
> > +     USB3_PHY,
> > +     USB2_PHY,
> > +     HSIC_PHY,
> > +     MAX_PHY_TYPES,
> > +};
> > +
> >  struct tegra_xusb_soc {
> >       const char *firmware;
> >       const char * const *supply_names;
> > @@ -274,6 +281,7 @@ struct tegra_xusb {
> > 
> >       bool suspended;
> >       struct tegra_xusb_context context;
> > +     u32 enable_utmi_pad_after_lp0_exit;
> 
> This is a bitfield, how do we know it will fit in a u32?  What is the
> range you are putting in here?
> 
> thanks,
> 
> greg k-h
static void tegra_xhci_program_utmi_power_lp0_exit(struct tegra_xusb
*tegra)
{
	unsigned int i;

	for (i = 0; i < tegra->soc->phy_types[USB2_PHY].num; i++) {
		if (!is_host_mode_phy(tegra, USB2_PHY, i))
			continue;
		/* USB2 */
		if (tegra->enable_utmi_pad_after_lp0_exit & BIT(i))
:
How many bits to be used is based on tegra->soc-
>phy_types[USB2_PHY].num which is defined like

static const struct tegra_xusb_phy_type tegra210_phy_types[] = {
:
	{ .name = "usb2", .num = 4, },
:
};

static const struct tegra_xusb_phy_type tegra194_phy_types[] = {
:
	{ .name = "usb2", .num = 4, },
};
, so far at most 4.

Therefore u8 for enable_utmi_pad_after_lp0_exit is long enough.

--nvpublic