usb: xhci-pci: Add TUSB73x0 definitions
Subsystems (2)
PATCH 1/3 usb: xhci-pci: Add TUSB73x0 definitions
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.
--- drivers/usb/host/xhci-pci.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
drivers/usb/host/xhci-pci.c +9 -2
@@ -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 ;
Signature · 1 line
2.43.0
PATCH 2/3 usb: xhci-pci: Add run-time check for TUSB73X0
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.
--- drivers/usb/host/xhci-pci.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
drivers/usb/host/xhci-pci.c +5 -3
@@ -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 ;
Signature · 1 line
2.43.0
PATCH 3/3 usb: xhci-pci: Put PWRON_POLARITY on a defined state
In reply to Fabio EstevamCurrently, 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.
--- drivers/usb/host/xhci-pci.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
drivers/usb/host/xhci-pci.c +9 -3
@@ -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 ;
Signature · 1 line
2.43.0
Re: [PATCH 3/3] usb: xhci-pci: Put PWRON_POLARITY on a defined state
Quoted 5 lines · Fabio Estevam
Currently,
TUSB73X0_PWRON_POLARITYis 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?
Quoted 2 lines
To make the state of
PWRON_POLARITYdeterministic, always write it when the "ti,pwron-active-high" is present.
That wouldn't be too dangereous, but...
Quoted 16 lines
Signed-off-by Fabio Estevam <[email protected]>--- drivers/usb/host/xhci-pci.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)drivers/usb/host/xhci-pci.c +2
-1@@ -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 ;
drivers/usb/host/xhci-pci.c int xhci_pci_common_probe(struct pci_dev *dev, const struct pci_device_id *id) · line 696
Quoted 10 lines
drivers/usb/host/xhci-pci.c +7
-2+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
Re: [PATCH 3/3] usb: xhci-pci: Put PWRON_POLARITY on a defined state
Hi Michal,
Quoted 11 lines · Michal Pecio
Quoted 5 lines · Fabio Estevam
Currently,
TUSB73X0_PWRON_POLARITYis 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.
Re: [PATCH 3/3] usb: xhci-pci: Put PWRON_POLARITY on a defined state
Quoted 12 lines · Fabio Estevam
Hi Michal,
Quoted 9 lines · Michal Pecio
Quoted 6 lines · Fabio Estevam
Currently,
TUSB73X0_PWRON_POLARITYis 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.
Quoted 7 lines
Quoted 4 lines
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