mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 1/3] usb: xhci-pci: Add TUSB73x0 definitions
@ 2026-09-09 12:58 Fabio Estevam
  2026-09-09 12:58 ` [PATCH 2/3] usb: xhci-pci: Add run-time check for TUSB73X0 Fabio Estevam
  2026-09-09 12:58 ` [PATCH 3/3] usb: xhci-pci: Put PWRON_POLARITY on a defined state Fabio Estevam
  0 siblings, 2 replies; 6+ messages in thread
From: Fabio Estevam @ 2026-09-09 12:58 UTC (permalink / raw)
  To: gregkh
  Cc: mathias.nyman, linux-usb, linux-kernel, parth.pancholi,
	francesco.dolcini, Fabio Estevam

Instead of hard-coding the TUSB73X0 PCI ID, USB_CTRL register address
and the PWRON_POLARITY, introduce definitions for them to make the code
easier to read.

No functional change.

Signed-off-by: Fabio Estevam <festevam@gmail.com>
---
 drivers/usb/host/xhci-pci.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index a8889081ae82..c580e0b86011 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -28,6 +28,9 @@
 #define SPARSE_DISABLE_BIT	17
 #define SPARSE_CNTL_ENABLE	0xC12C
 
+#define TUSB73X0_USB_CTRL			0xe0
+#define TUSB73X0_PWRON_POLARITY			BIT(22)
+
 /* Device for a quirk */
 #define PCI_VENDOR_ID_FRESCO_LOGIC		0x1b73
 #define PCI_DEVICE_ID_FRESCO_LOGIC_PDK		0x1000
@@ -95,6 +98,8 @@
 #define PCI_DEVICE_ID_ASMEDIA_3042_XHCI			0x3042
 #define PCI_DEVICE_ID_ASMEDIA_3242_XHCI			0x3242
 
+#define PCI_DEVICE_ID_TI_TUSB73X0			0x8241
+
 static const char hcd_name[] = "xhci_hcd";
 
 static struct hc_driver __read_mostly xhci_pci_hc_driver;
@@ -479,7 +484,8 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
 	    pdev->device == PCI_DEVICE_ID_ASMEDIA_3042_XHCI)
 		xhci->quirks |= XHCI_RESET_ON_RESUME;
 
-	if (pdev->vendor == PCI_VENDOR_ID_TI && pdev->device == 0x8241)
+	if (pdev->vendor == PCI_VENDOR_ID_TI &&
+	    pdev->device == PCI_DEVICE_ID_TI_TUSB73X0)
 		xhci->quirks |= XHCI_LIMIT_ENDPOINT_INTERVAL_7;
 
 	if ((pdev->vendor == PCI_VENDOR_ID_BROADCOM ||
@@ -678,7 +684,8 @@ int xhci_pci_common_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	dma_set_max_seg_size(&dev->dev, UINT_MAX);
 
 	if (device_property_read_bool(&dev->dev, "ti,pwron-active-high"))
-		pci_clear_and_set_config_dword(dev, 0xE0, 0, 1 << 22);
+		pci_clear_and_set_config_dword(dev, TUSB73X0_USB_CTRL, 0,
+					       TUSB73X0_PWRON_POLARITY);
 
 	return 0;
 
-- 
2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/3] usb: xhci-pci: Add run-time check for TUSB73X0
  2026-09-09 12:58 [PATCH 1/3] usb: xhci-pci: Add TUSB73x0 definitions Fabio Estevam
@ 2026-09-09 12:58 ` Fabio Estevam
  2026-09-09 12:58 ` [PATCH 3/3] usb: xhci-pci: Put PWRON_POLARITY on a defined state Fabio Estevam
  1 sibling, 0 replies; 6+ messages in thread
From: Fabio Estevam @ 2026-09-09 12:58 UTC (permalink / raw)
  To: gregkh
  Cc: mathias.nyman, linux-usb, linux-kernel, parth.pancholi,
	francesco.dolcini, Fabio Estevam

The "ti,pwron-active-high" property is only valid for TUSB73X0.

Protect its code so that eventual specific TUSB73X0 register write applies
only on a TUSB73X0 device.

Signed-off-by: Fabio Estevam <festevam@gmail.com>
---
 drivers/usb/host/xhci-pci.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index c580e0b86011..75aa679f7d79 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -683,9 +683,11 @@ int xhci_pci_common_probe(struct pci_dev *dev, const struct pci_device_id *id)
 
 	dma_set_max_seg_size(&dev->dev, UINT_MAX);
 
-	if (device_property_read_bool(&dev->dev, "ti,pwron-active-high"))
-		pci_clear_and_set_config_dword(dev, TUSB73X0_USB_CTRL, 0,
-					       TUSB73X0_PWRON_POLARITY);
+	if (dev->vendor == PCI_VENDOR_ID_TI &&
+	    dev->device == PCI_DEVICE_ID_TI_TUSB73X0)
+		if (device_property_read_bool(&dev->dev, "ti,pwron-active-high"))
+			pci_clear_and_set_config_dword(dev, TUSB73X0_USB_CTRL, 0,
+						       TUSB73X0_PWRON_POLARITY);
 
 	return 0;
 
-- 
2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 3/3] usb: xhci-pci: Put PWRON_POLARITY on a defined state
  2026-09-09 12:58 [PATCH 1/3] usb: xhci-pci: Add TUSB73x0 definitions Fabio Estevam
  2026-09-09 12:58 ` [PATCH 2/3] usb: xhci-pci: Add run-time check for TUSB73X0 Fabio Estevam
@ 2026-09-09 12:58 ` Fabio Estevam
  2026-09-09 14:43   ` Michal Pecio
  1 sibling, 1 reply; 6+ messages in thread
From: Fabio Estevam @ 2026-09-09 12:58 UTC (permalink / raw)
  To: gregkh
  Cc: mathias.nyman, linux-usb, linux-kernel, parth.pancholi,
	francesco.dolcini, Fabio Estevam

Currently, TUSB73X0_PWRON_POLARITY is set when the "ti,pwron-active-high"
is present.

However, when "ti,pwron-active-high" is absent, its status could have been
set previously by a bootloader or by a previous running kernel.

To make the state of PWRON_POLARITY deterministic, always write it
when the "ti,pwron-active-high" is present.

Signed-off-by: Fabio Estevam <festevam@gmail.com>
---
 drivers/usb/host/xhci-pci.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 75aa679f7d79..833c55c3213a 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -684,10 +684,16 @@ int xhci_pci_common_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	dma_set_max_seg_size(&dev->dev, UINT_MAX);
 
 	if (dev->vendor == PCI_VENDOR_ID_TI &&
-	    dev->device == PCI_DEVICE_ID_TI_TUSB73X0)
+	    dev->device == PCI_DEVICE_ID_TI_TUSB73X0) {
+		u32 pwron_polarity = 0;
+
 		if (device_property_read_bool(&dev->dev, "ti,pwron-active-high"))
-			pci_clear_and_set_config_dword(dev, TUSB73X0_USB_CTRL, 0,
-						       TUSB73X0_PWRON_POLARITY);
+			pwron_polarity = TUSB73X0_PWRON_POLARITY;
+
+		pci_clear_and_set_config_dword(dev, TUSB73X0_USB_CTRL,
+					       TUSB73X0_PWRON_POLARITY,
+					       pwron_polarity);
+	}
 
 	return 0;
 
-- 
2.43.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/3] usb: xhci-pci: Put PWRON_POLARITY on a defined state
  2026-09-09 12:58 ` [PATCH 3/3] usb: xhci-pci: Put PWRON_POLARITY on a defined state Fabio Estevam
@ 2026-09-09 14:43   ` Michal Pecio
  2026-09-11 13:58     ` Fabio Estevam
  0 siblings, 1 reply; 6+ messages in thread
From: Michal Pecio @ 2026-09-09 14:43 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: gregkh, mathias.nyman, linux-usb, linux-kernel, parth.pancholi,
	francesco.dolcini

On Wed,  9 Sep 2026 09:58:42 -0300, Fabio Estevam wrote:
> Currently, TUSB73X0_PWRON_POLARITY is set when the
> "ti,pwron-active-high" is present.
>
> However, when "ti,pwron-active-high" is absent, its status could have
> been set previously by a bootloader or by a previous running kernel.

or by EEPROM.

Do they bother to put the chip in a wrong, non-default state at boot?

> To make the state of PWRON_POLARITY deterministic, always write it
> when the "ti,pwron-active-high" is present.

That wouldn't be too dangereous, but...

> Signed-off-by: Fabio Estevam <festevam@gmail.com>
> ---
>  drivers/usb/host/xhci-pci.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index 75aa679f7d79..833c55c3213a 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -684,10 +684,16 @@ int xhci_pci_common_probe(struct pci_dev *dev, const struct pci_device_id *id)
>  	dma_set_max_seg_size(&dev->dev, UINT_MAX);
>  
>  	if (dev->vendor == PCI_VENDOR_ID_TI &&
> -	    dev->device == PCI_DEVICE_ID_TI_TUSB73X0)
> +	    dev->device == PCI_DEVICE_ID_TI_TUSB73X0) {
> +		u32 pwron_polarity = 0;
> +
>  		if (device_property_read_bool(&dev->dev, "ti,pwron-active-high"))
> -			pci_clear_and_set_config_dword(dev, TUSB73X0_USB_CTRL, 0,
> -						       TUSB73X0_PWRON_POLARITY);
> +			pwron_polarity = TUSB73X0_PWRON_POLARITY;
> +
> +		pci_clear_and_set_config_dword(dev, TUSB73X0_USB_CTRL,
> +					       TUSB73X0_PWRON_POLARITY,
> +					       pwron_polarity);
> +	}

... isn't device_property_present() missing here?

Also, I don't know those DT rules, but doesn't "ti,something" imply
that this must be a TI chip, no need to check IDs?

Regards,
Michal

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/3] usb: xhci-pci: Put PWRON_POLARITY on a defined state
  2026-09-09 14:43   ` Michal Pecio
@ 2026-09-11 13:58     ` Fabio Estevam
  2026-09-11 18:11       ` Michal Pecio
  0 siblings, 1 reply; 6+ messages in thread
From: Fabio Estevam @ 2026-09-11 13:58 UTC (permalink / raw)
  To: Michal Pecio
  Cc: gregkh, mathias.nyman, linux-usb, linux-kernel, parth.pancholi,
	francesco.dolcini

Hi Michal,

On Wed, Sep 9, 2026 at 11:43 AM Michal Pecio <michal.pecio@gmail.com> wrote:
>
> On Wed,  9 Sep 2026 09:58:42 -0300, Fabio Estevam wrote:
> > Currently, TUSB73X0_PWRON_POLARITY is set when the
> > "ti,pwron-active-high" is present.
> >
> > However, when "ti,pwron-active-high" is absent, its status could have
> > been set previously by a bootloader or by a previous running kernel.
>
> or by EEPROM.
>
> Do they bother to put the chip in a wrong, non-default state at boot?

I was working on a hardware bring-up with the TUSB7320, and I never
saw it happen.

Since this is more theoretical, let's discard patches 2 and 3 for now.

I'll resend only the first patch of this series.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 3/3] usb: xhci-pci: Put PWRON_POLARITY on a defined state
  2026-09-11 13:58     ` Fabio Estevam
@ 2026-09-11 18:11       ` Michal Pecio
  0 siblings, 0 replies; 6+ messages in thread
From: Michal Pecio @ 2026-09-11 18:11 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: gregkh, mathias.nyman, linux-usb, linux-kernel, parth.pancholi,
	francesco.dolcini

On Fri, 11 Sep 2026 10:58:27 -0300, Fabio Estevam wrote:
> Hi Michal,
> 
> On Wed, Sep 9, 2026 at 11:43 AM Michal Pecio <michal.pecio@gmail.com> wrote:
> >
> > On Wed,  9 Sep 2026 09:58:42 -0300, Fabio Estevam wrote:  
> > > Currently, TUSB73X0_PWRON_POLARITY is set when the
> > > "ti,pwron-active-high" is present.
> > >
> > > However, when "ti,pwron-active-high" is absent, its status could
> > > have been set previously by a bootloader or by a previous running
> > > kernel.  
> >
> > or by EEPROM.
> >
> > Do they bother to put the chip in a wrong, non-default state at
> > boot?  
> 
> I was working on a hardware bring-up with the TUSB7320, and I never
> saw it happen.

That could practically only happen due to a bug. Seems more likely
that EEPROM or boot FW may set the bit because it's required by the
board and then the kernel could clear it for lack of knowledge.

If there is ever a need to support clearing the bit, it would better
be done by checking if the DT property is specified at all, and only
then tweaking the bit accordingly. Otherwise, leave it alone.

Regards,
Michal

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-09-11 18:11 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-09 12:58 [PATCH 1/3] usb: xhci-pci: Add TUSB73x0 definitions Fabio Estevam
2026-09-09 12:58 ` [PATCH 2/3] usb: xhci-pci: Add run-time check for TUSB73X0 Fabio Estevam
2026-09-09 12:58 ` [PATCH 3/3] usb: xhci-pci: Put PWRON_POLARITY on a defined state Fabio Estevam
2026-09-09 14:43   ` Michal Pecio
2026-09-11 13:58     ` Fabio Estevam
2026-09-11 18:11       ` Michal Pecio

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®