* [PATCH 0/3] usb: host: pci: PCI ID consolidation
@ 2018-03-14 10:29 Richard Leitner
2018-03-14 10:29 ` [1/3] " Richard Leitner
` (2 more replies)
0 siblings, 3 replies; 31+ messages in thread
From: Richard Leitner @ 2018-03-14 10:29 UTC (permalink / raw)
To: linux-usb, linux-kernel, linux-pci
Cc: gregkh, mathias.nyman, bhelgaas, richard.leitner
From: Richard Leitner <richard.leitner@skidata.com>
Centralize some hardcoded PCI IDs as definitions in the global
include/linux/pci_ids.h file. This is done to reduce the amount of
scattered PCI ID definitions and hardcoded values across the kernel.
Richard Leitner (3):
usb: host: pci: use existing Intel PCI ID macros
usb: host: pci: introduce PCI vendor ID for Netlogic
usb: host: pci: replace hardcoded renesas PCI IDs
drivers/usb/host/pci-quirks.c | 16 +++++++++-------
drivers/usb/host/xhci-pci.c | 7 ++++---
include/linux/pci_ids.h | 5 +++++
3 files changed, 18 insertions(+), 10 deletions(-)
--
2.11.0
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 1/3] usb: host: pci: use existing Intel PCI ID macros
@ 2018-03-14 10:29 ` Richard Leitner
0 siblings, 0 replies; 31+ messages in thread
From: Richard Leitner @ 2018-03-14 10:29 UTC (permalink / raw)
To: linux-usb, linux-kernel, linux-pci
Cc: gregkh, mathias.nyman, bhelgaas, richard.leitner
From: Richard Leitner <richard.leitner@skidata.com>
Instead of the hardcoded hexadecimal PCI IDs use the existing macros
from pci_ids.h for Intel IDs.
Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
---
drivers/usb/host/pci-quirks.c | 10 +++++-----
drivers/usb/host/xhci-pci.c | 3 ++-
include/linux/pci_ids.h | 1 +
3 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 67ad4bb6919a..4f4a9f36a68e 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -858,8 +858,8 @@ static void ehci_bios_handoff(struct pci_dev *pdev,
*
* The HASEE E200 hangs when the semaphore is set (bugzilla #77021).
*/
- if (pdev->vendor == 0x8086 && (pdev->device == 0x283a ||
- pdev->device == 0x27cc)) {
+ if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
+ (pdev->device == 0x283a || pdev->device == 0x27cc)) {
if (dmi_check_system(ehci_dmi_nohandoff_table))
try_handoff = 0;
}
@@ -1168,9 +1168,9 @@ static void quirk_usb_handoff_xhci(struct pci_dev *pdev)
val = readl(base + ext_cap_offset);
/* Auto handoff never worked for these devices. Force it and continue */
- if ((pdev->vendor == PCI_VENDOR_ID_TI && pdev->device == 0x8241) ||
- (pdev->vendor == PCI_VENDOR_ID_RENESAS
- && pdev->device == 0x0014)) {
+ if ((pdev->vendor == PCI_VENDOR_ID_TI &&
+ pdev->device == PCI_DEVICE_ID_TI_TUSB73X0) ||
+ (pdev->vendor == PCI_VENDOR_ID_RENESAS && pdev->device == 0x0014)) {
val = (val | XHCI_HC_OS_OWNED) & ~XHCI_HC_BIOS_OWNED;
writel(val, base + ext_cap_offset);
}
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 5262fa571a5d..a5bfd890190c 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -213,7 +213,8 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
pdev->device == PCI_DEVICE_ID_ASMEDIA_1042A_XHCI)
xhci->quirks |= XHCI_ASMEDIA_MODIFY_FLOWCONTROL;
- 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 (xhci->quirks & XHCI_RESET_ON_RESUME)
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index a6b30667a331..e8d1af82a688 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -838,6 +838,7 @@
#define PCI_DEVICE_ID_TI_XX12 0x8039
#define PCI_DEVICE_ID_TI_XX12_FM 0x803b
#define PCI_DEVICE_ID_TI_XIO2000A 0x8231
+#define PCI_DEVICE_ID_TI_TUSB73X0 0x8241
#define PCI_DEVICE_ID_TI_1130 0xac12
#define PCI_DEVICE_ID_TI_1031 0xac13
#define PCI_DEVICE_ID_TI_1131 0xac15
--
2.11.0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [1/3] usb: host: pci: use existing Intel PCI ID macros
@ 2018-03-14 10:29 ` Richard Leitner
0 siblings, 0 replies; 31+ messages in thread
From: Richard Leitner @ 2018-03-14 10:29 UTC (permalink / raw)
To: linux-usb, linux-kernel, linux-pci
Cc: gregkh, mathias.nyman, bhelgaas, richard.leitner
From: Richard Leitner <richard.leitner@skidata.com>
Instead of the hardcoded hexadecimal PCI IDs use the existing macros
from pci_ids.h for Intel IDs.
Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
---
drivers/usb/host/pci-quirks.c | 10 +++++-----
drivers/usb/host/xhci-pci.c | 3 ++-
include/linux/pci_ids.h | 1 +
3 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 67ad4bb6919a..4f4a9f36a68e 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -858,8 +858,8 @@ static void ehci_bios_handoff(struct pci_dev *pdev,
*
* The HASEE E200 hangs when the semaphore is set (bugzilla #77021).
*/
- if (pdev->vendor == 0x8086 && (pdev->device == 0x283a ||
- pdev->device == 0x27cc)) {
+ if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
+ (pdev->device == 0x283a || pdev->device == 0x27cc)) {
if (dmi_check_system(ehci_dmi_nohandoff_table))
try_handoff = 0;
}
@@ -1168,9 +1168,9 @@ static void quirk_usb_handoff_xhci(struct pci_dev *pdev)
val = readl(base + ext_cap_offset);
/* Auto handoff never worked for these devices. Force it and continue */
- if ((pdev->vendor == PCI_VENDOR_ID_TI && pdev->device == 0x8241) ||
- (pdev->vendor == PCI_VENDOR_ID_RENESAS
- && pdev->device == 0x0014)) {
+ if ((pdev->vendor == PCI_VENDOR_ID_TI &&
+ pdev->device == PCI_DEVICE_ID_TI_TUSB73X0) ||
+ (pdev->vendor == PCI_VENDOR_ID_RENESAS && pdev->device == 0x0014)) {
val = (val | XHCI_HC_OS_OWNED) & ~XHCI_HC_BIOS_OWNED;
writel(val, base + ext_cap_offset);
}
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 5262fa571a5d..a5bfd890190c 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -213,7 +213,8 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
pdev->device == PCI_DEVICE_ID_ASMEDIA_1042A_XHCI)
xhci->quirks |= XHCI_ASMEDIA_MODIFY_FLOWCONTROL;
- 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 (xhci->quirks & XHCI_RESET_ON_RESUME)
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index a6b30667a331..e8d1af82a688 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -838,6 +838,7 @@
#define PCI_DEVICE_ID_TI_XX12 0x8039
#define PCI_DEVICE_ID_TI_XX12_FM 0x803b
#define PCI_DEVICE_ID_TI_XIO2000A 0x8231
+#define PCI_DEVICE_ID_TI_TUSB73X0 0x8241
#define PCI_DEVICE_ID_TI_1130 0xac12
#define PCI_DEVICE_ID_TI_1031 0xac13
#define PCI_DEVICE_ID_TI_1131 0xac15
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 2/3] usb: host: pci: introduce PCI vendor ID for Netlogic
@ 2018-03-14 10:29 ` Richard Leitner
0 siblings, 0 replies; 31+ messages in thread
From: Richard Leitner @ 2018-03-14 10:29 UTC (permalink / raw)
To: linux-usb, linux-kernel, linux-pci
Cc: gregkh, mathias.nyman, bhelgaas, richard.leitner
From: Richard Leitner <richard.leitner@skidata.com>
Replace the hardcoded PCI vendor ID of Netlogic with a definition in
pci_ids.h
Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
---
drivers/usb/host/pci-quirks.c | 2 +-
include/linux/pci_ids.h | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 4f4a9f36a68e..39d163729b89 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -1243,7 +1243,7 @@ static void quirk_usb_early_handoff(struct pci_dev *pdev)
/* Skip Netlogic mips SoC's internal PCI USB controller.
* This device does not need/support EHCI/OHCI handoff
*/
- if (pdev->vendor == 0x184e) /* vendor Netlogic */
+ if (pdev->vendor == PCI_VENDOR_ID_NETLOGIC)
return;
if (pdev->class != PCI_CLASS_SERIAL_USB_UHCI &&
pdev->class != PCI_CLASS_SERIAL_USB_OHCI &&
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index e8d1af82a688..d23a97868dee 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -3067,4 +3067,6 @@
#define PCI_VENDOR_ID_OCZ 0x1b85
+#define PCI_VENDOR_ID_NETLOGIC 0x184e
+
#endif /* _LINUX_PCI_IDS_H */
--
2.11.0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [2/3] usb: host: pci: introduce PCI vendor ID for Netlogic
@ 2018-03-14 10:29 ` Richard Leitner
0 siblings, 0 replies; 31+ messages in thread
From: Richard Leitner @ 2018-03-14 10:29 UTC (permalink / raw)
To: linux-usb, linux-kernel, linux-pci
Cc: gregkh, mathias.nyman, bhelgaas, richard.leitner
From: Richard Leitner <richard.leitner@skidata.com>
Replace the hardcoded PCI vendor ID of Netlogic with a definition in
pci_ids.h
Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
---
drivers/usb/host/pci-quirks.c | 2 +-
include/linux/pci_ids.h | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 4f4a9f36a68e..39d163729b89 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -1243,7 +1243,7 @@ static void quirk_usb_early_handoff(struct pci_dev *pdev)
/* Skip Netlogic mips SoC's internal PCI USB controller.
* This device does not need/support EHCI/OHCI handoff
*/
- if (pdev->vendor == 0x184e) /* vendor Netlogic */
+ if (pdev->vendor == PCI_VENDOR_ID_NETLOGIC)
return;
if (pdev->class != PCI_CLASS_SERIAL_USB_UHCI &&
pdev->class != PCI_CLASS_SERIAL_USB_OHCI &&
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index e8d1af82a688..d23a97868dee 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -3067,4 +3067,6 @@
#define PCI_VENDOR_ID_OCZ 0x1b85
+#define PCI_VENDOR_ID_NETLOGIC 0x184e
+
#endif /* _LINUX_PCI_IDS_H */
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 3/3] usb: host: pci: replace hardcoded renesas PCI IDs
@ 2018-03-14 10:29 ` Richard Leitner
0 siblings, 0 replies; 31+ messages in thread
From: Richard Leitner @ 2018-03-14 10:29 UTC (permalink / raw)
To: linux-usb, linux-kernel, linux-pci
Cc: gregkh, mathias.nyman, bhelgaas, richard.leitner
From: Richard Leitner <richard.leitner@skidata.com>
Introduce Renesas uPD72020{1,2} PCI device IDs in pci_ids.h and replace
the harcoded values with them.
Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
---
drivers/usb/host/pci-quirks.c | 6 ++++--
drivers/usb/host/xhci-pci.c | 4 ++--
include/linux/pci_ids.h | 2 ++
3 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 39d163729b89..5e1ad523622e 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -1170,7 +1170,8 @@ static void quirk_usb_handoff_xhci(struct pci_dev *pdev)
/* Auto handoff never worked for these devices. Force it and continue */
if ((pdev->vendor == PCI_VENDOR_ID_TI &&
pdev->device == PCI_DEVICE_ID_TI_TUSB73X0) ||
- (pdev->vendor == PCI_VENDOR_ID_RENESAS && pdev->device == 0x0014)) {
+ (pdev->vendor == PCI_VENDOR_ID_RENESAS &&
+ pdev->device == PCI_DEVICE_ID_RENESAS_UPD720201)) {
val = (val | XHCI_HC_OS_OWNED) & ~XHCI_HC_BIOS_OWNED;
writel(val, base + ext_cap_offset);
}
@@ -1282,7 +1283,8 @@ bool usb_xhci_needs_pci_reset(struct pci_dev *pdev)
* quirk, or the system will be in a rather bad state.
*/
if (pdev->vendor == PCI_VENDOR_ID_RENESAS &&
- (pdev->device == 0x0014 || pdev->device == 0x0015))
+ (pdev->device == PCI_DEVICE_ID_RENESAS_UPD720201 ||
+ pdev->device == PCI_DEVICE_ID_RENESAS_UPD720202))
return true;
return false;
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index a5bfd890190c..a453e4c35ac7 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -189,10 +189,10 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
xhci->quirks |= XHCI_BROKEN_STREAMS;
}
if (pdev->vendor == PCI_VENDOR_ID_RENESAS &&
- pdev->device == 0x0014)
+ pdev->device == PCI_DEVICE_ID_RENESAS_UPD720201)
xhci->quirks |= XHCI_TRUST_TX_LENGTH;
if (pdev->vendor == PCI_VENDOR_ID_RENESAS &&
- pdev->device == 0x0015)
+ pdev->device == PCI_DEVICE_ID_RENESAS_UPD720202)
xhci->quirks |= XHCI_RESET_ON_RESUME;
if (pdev->vendor == PCI_VENDOR_ID_VIA)
xhci->quirks |= XHCI_RESET_ON_RESUME;
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index d23a97868dee..eb52f0e9b651 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2427,6 +2427,8 @@
#define PCI_DEVICE_ID_RENESAS_SH7763 0x0004
#define PCI_DEVICE_ID_RENESAS_SH7785 0x0007
#define PCI_DEVICE_ID_RENESAS_SH7786 0x0010
+#define PCI_DEVICE_ID_RENESAS_UPD720201 0x0014
+#define PCI_DEVICE_ID_RENESAS_UPD720202 0x0015
#define PCI_VENDOR_ID_SOLARFLARE 0x1924
#define PCI_DEVICE_ID_SOLARFLARE_SFC4000A_0 0x0703
--
2.11.0
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [3/3] usb: host: pci: replace hardcoded renesas PCI IDs
@ 2018-03-14 10:29 ` Richard Leitner
0 siblings, 0 replies; 31+ messages in thread
From: Richard Leitner @ 2018-03-14 10:29 UTC (permalink / raw)
To: linux-usb, linux-kernel, linux-pci
Cc: gregkh, mathias.nyman, bhelgaas, richard.leitner
From: Richard Leitner <richard.leitner@skidata.com>
Introduce Renesas uPD72020{1,2} PCI device IDs in pci_ids.h and replace
the harcoded values with them.
Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
---
drivers/usb/host/pci-quirks.c | 6 ++++--
drivers/usb/host/xhci-pci.c | 4 ++--
include/linux/pci_ids.h | 2 ++
3 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 39d163729b89..5e1ad523622e 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -1170,7 +1170,8 @@ static void quirk_usb_handoff_xhci(struct pci_dev *pdev)
/* Auto handoff never worked for these devices. Force it and continue */
if ((pdev->vendor == PCI_VENDOR_ID_TI &&
pdev->device == PCI_DEVICE_ID_TI_TUSB73X0) ||
- (pdev->vendor == PCI_VENDOR_ID_RENESAS && pdev->device == 0x0014)) {
+ (pdev->vendor == PCI_VENDOR_ID_RENESAS &&
+ pdev->device == PCI_DEVICE_ID_RENESAS_UPD720201)) {
val = (val | XHCI_HC_OS_OWNED) & ~XHCI_HC_BIOS_OWNED;
writel(val, base + ext_cap_offset);
}
@@ -1282,7 +1283,8 @@ bool usb_xhci_needs_pci_reset(struct pci_dev *pdev)
* quirk, or the system will be in a rather bad state.
*/
if (pdev->vendor == PCI_VENDOR_ID_RENESAS &&
- (pdev->device == 0x0014 || pdev->device == 0x0015))
+ (pdev->device == PCI_DEVICE_ID_RENESAS_UPD720201 ||
+ pdev->device == PCI_DEVICE_ID_RENESAS_UPD720202))
return true;
return false;
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index a5bfd890190c..a453e4c35ac7 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -189,10 +189,10 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
xhci->quirks |= XHCI_BROKEN_STREAMS;
}
if (pdev->vendor == PCI_VENDOR_ID_RENESAS &&
- pdev->device == 0x0014)
+ pdev->device == PCI_DEVICE_ID_RENESAS_UPD720201)
xhci->quirks |= XHCI_TRUST_TX_LENGTH;
if (pdev->vendor == PCI_VENDOR_ID_RENESAS &&
- pdev->device == 0x0015)
+ pdev->device == PCI_DEVICE_ID_RENESAS_UPD720202)
xhci->quirks |= XHCI_RESET_ON_RESUME;
if (pdev->vendor == PCI_VENDOR_ID_VIA)
xhci->quirks |= XHCI_RESET_ON_RESUME;
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index d23a97868dee..eb52f0e9b651 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2427,6 +2427,8 @@
#define PCI_DEVICE_ID_RENESAS_SH7763 0x0004
#define PCI_DEVICE_ID_RENESAS_SH7785 0x0007
#define PCI_DEVICE_ID_RENESAS_SH7786 0x0010
+#define PCI_DEVICE_ID_RENESAS_UPD720201 0x0014
+#define PCI_DEVICE_ID_RENESAS_UPD720202 0x0015
#define PCI_VENDOR_ID_SOLARFLARE 0x1924
#define PCI_DEVICE_ID_SOLARFLARE_SFC4000A_0 0x0703
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH 1/3] usb: host: pci: use existing Intel PCI ID macros
@ 2018-03-14 10:48 ` Greg Kroah-Hartman
0 siblings, 0 replies; 31+ messages in thread
From: Greg KH @ 2018-03-14 10:48 UTC (permalink / raw)
To: Richard Leitner
Cc: linux-usb, linux-kernel, linux-pci, mathias.nyman, bhelgaas,
richard.leitner
On Wed, Mar 14, 2018 at 11:29:31AM +0100, Richard Leitner wrote:
> From: Richard Leitner <richard.leitner@skidata.com>
>
> Instead of the hardcoded hexadecimal PCI IDs use the existing macros
> from pci_ids.h for Intel IDs.
You also did something else in this patch, yet failed to mention it
here:
> Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
> ---
> drivers/usb/host/pci-quirks.c | 10 +++++-----
> drivers/usb/host/xhci-pci.c | 3 ++-
> include/linux/pci_ids.h | 1 +
> 3 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
> index 67ad4bb6919a..4f4a9f36a68e 100644
> --- a/drivers/usb/host/pci-quirks.c
> +++ b/drivers/usb/host/pci-quirks.c
> @@ -858,8 +858,8 @@ static void ehci_bios_handoff(struct pci_dev *pdev,
> *
> * The HASEE E200 hangs when the semaphore is set (bugzilla #77021).
> */
> - if (pdev->vendor == 0x8086 && (pdev->device == 0x283a ||
> - pdev->device == 0x27cc)) {
> + if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
> + (pdev->device == 0x283a || pdev->device == 0x27cc)) {
> if (dmi_check_system(ehci_dmi_nohandoff_table))
> try_handoff = 0;
> }
> @@ -1168,9 +1168,9 @@ static void quirk_usb_handoff_xhci(struct pci_dev *pdev)
> val = readl(base + ext_cap_offset);
>
> /* Auto handoff never worked for these devices. Force it and continue */
> - if ((pdev->vendor == PCI_VENDOR_ID_TI && pdev->device == 0x8241) ||
> - (pdev->vendor == PCI_VENDOR_ID_RENESAS
> - && pdev->device == 0x0014)) {
> + if ((pdev->vendor == PCI_VENDOR_ID_TI &&
> + pdev->device == PCI_DEVICE_ID_TI_TUSB73X0) ||
> + (pdev->vendor == PCI_VENDOR_ID_RENESAS && pdev->device == 0x0014)) {
> val = (val | XHCI_HC_OS_OWNED) & ~XHCI_HC_BIOS_OWNED;
> writel(val, base + ext_cap_offset);
> }
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index 5262fa571a5d..a5bfd890190c 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -213,7 +213,8 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
> pdev->device == PCI_DEVICE_ID_ASMEDIA_1042A_XHCI)
> xhci->quirks |= XHCI_ASMEDIA_MODIFY_FLOWCONTROL;
>
> - 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 (xhci->quirks & XHCI_RESET_ON_RESUME)
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index a6b30667a331..e8d1af82a688 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -838,6 +838,7 @@
> #define PCI_DEVICE_ID_TI_XX12 0x8039
> #define PCI_DEVICE_ID_TI_XX12_FM 0x803b
> #define PCI_DEVICE_ID_TI_XIO2000A 0x8231
> +#define PCI_DEVICE_ID_TI_TUSB73X0 0x8241
You shared a TI device id.
Please break this up into 2 patches.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 31+ messages in thread
* [1/3] usb: host: pci: use existing Intel PCI ID macros
@ 2018-03-14 10:48 ` Greg Kroah-Hartman
0 siblings, 0 replies; 31+ messages in thread
From: Greg Kroah-Hartman @ 2018-03-14 10:48 UTC (permalink / raw)
To: Richard Leitner
Cc: linux-usb, linux-kernel, linux-pci, mathias.nyman, bhelgaas,
richard.leitner
On Wed, Mar 14, 2018 at 11:29:31AM +0100, Richard Leitner wrote:
> From: Richard Leitner <richard.leitner@skidata.com>
>
> Instead of the hardcoded hexadecimal PCI IDs use the existing macros
> from pci_ids.h for Intel IDs.
You also did something else in this patch, yet failed to mention it
here:
> Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
> ---
> drivers/usb/host/pci-quirks.c | 10 +++++-----
> drivers/usb/host/xhci-pci.c | 3 ++-
> include/linux/pci_ids.h | 1 +
> 3 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
> index 67ad4bb6919a..4f4a9f36a68e 100644
> --- a/drivers/usb/host/pci-quirks.c
> +++ b/drivers/usb/host/pci-quirks.c
> @@ -858,8 +858,8 @@ static void ehci_bios_handoff(struct pci_dev *pdev,
> *
> * The HASEE E200 hangs when the semaphore is set (bugzilla #77021).
> */
> - if (pdev->vendor == 0x8086 && (pdev->device == 0x283a ||
> - pdev->device == 0x27cc)) {
> + if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
> + (pdev->device == 0x283a || pdev->device == 0x27cc)) {
> if (dmi_check_system(ehci_dmi_nohandoff_table))
> try_handoff = 0;
> }
> @@ -1168,9 +1168,9 @@ static void quirk_usb_handoff_xhci(struct pci_dev *pdev)
> val = readl(base + ext_cap_offset);
>
> /* Auto handoff never worked for these devices. Force it and continue */
> - if ((pdev->vendor == PCI_VENDOR_ID_TI && pdev->device == 0x8241) ||
> - (pdev->vendor == PCI_VENDOR_ID_RENESAS
> - && pdev->device == 0x0014)) {
> + if ((pdev->vendor == PCI_VENDOR_ID_TI &&
> + pdev->device == PCI_DEVICE_ID_TI_TUSB73X0) ||
> + (pdev->vendor == PCI_VENDOR_ID_RENESAS && pdev->device == 0x0014)) {
> val = (val | XHCI_HC_OS_OWNED) & ~XHCI_HC_BIOS_OWNED;
> writel(val, base + ext_cap_offset);
> }
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index 5262fa571a5d..a5bfd890190c 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -213,7 +213,8 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
> pdev->device == PCI_DEVICE_ID_ASMEDIA_1042A_XHCI)
> xhci->quirks |= XHCI_ASMEDIA_MODIFY_FLOWCONTROL;
>
> - 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 (xhci->quirks & XHCI_RESET_ON_RESUME)
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index a6b30667a331..e8d1af82a688 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -838,6 +838,7 @@
> #define PCI_DEVICE_ID_TI_XX12 0x8039
> #define PCI_DEVICE_ID_TI_XX12_FM 0x803b
> #define PCI_DEVICE_ID_TI_XIO2000A 0x8231
> +#define PCI_DEVICE_ID_TI_TUSB73X0 0x8241
You shared a TI device id.
Please break this up into 2 patches.
thanks,
greg k-h
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/3] usb: host: pci: introduce PCI vendor ID for Netlogic
@ 2018-03-14 10:48 ` Greg Kroah-Hartman
0 siblings, 0 replies; 31+ messages in thread
From: Greg KH @ 2018-03-14 10:48 UTC (permalink / raw)
To: Richard Leitner
Cc: linux-usb, linux-kernel, linux-pci, mathias.nyman, bhelgaas,
richard.leitner
On Wed, Mar 14, 2018 at 11:29:32AM +0100, Richard Leitner wrote:
> From: Richard Leitner <richard.leitner@skidata.com>
>
> Replace the hardcoded PCI vendor ID of Netlogic with a definition in
> pci_ids.h
Why? It's only being used in one file, so it should not be in
pci_ids.h, right?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 31+ messages in thread
* [2/3] usb: host: pci: introduce PCI vendor ID for Netlogic
@ 2018-03-14 10:48 ` Greg Kroah-Hartman
0 siblings, 0 replies; 31+ messages in thread
From: Greg Kroah-Hartman @ 2018-03-14 10:48 UTC (permalink / raw)
To: Richard Leitner
Cc: linux-usb, linux-kernel, linux-pci, mathias.nyman, bhelgaas,
richard.leitner
On Wed, Mar 14, 2018 at 11:29:32AM +0100, Richard Leitner wrote:
> From: Richard Leitner <richard.leitner@skidata.com>
>
> Replace the hardcoded PCI vendor ID of Netlogic with a definition in
> pci_ids.h
Why? It's only being used in one file, so it should not be in
pci_ids.h, right?
thanks,
greg k-h
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 3/3] usb: host: pci: replace hardcoded renesas PCI IDs
@ 2018-03-14 10:49 ` Greg Kroah-Hartman
0 siblings, 0 replies; 31+ messages in thread
From: Greg KH @ 2018-03-14 10:49 UTC (permalink / raw)
To: Richard Leitner
Cc: linux-usb, linux-kernel, linux-pci, mathias.nyman, bhelgaas,
richard.leitner
On Wed, Mar 14, 2018 at 11:29:33AM +0100, Richard Leitner wrote:
> From: Richard Leitner <richard.leitner@skidata.com>
>
> Introduce Renesas uPD72020{1,2} PCI device IDs in pci_ids.h and replace
> the harcoded values with them.
>
> Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
> ---
> drivers/usb/host/pci-quirks.c | 6 ++++--
> drivers/usb/host/xhci-pci.c | 4 ++--
> include/linux/pci_ids.h | 2 ++
> 3 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
> index 39d163729b89..5e1ad523622e 100644
> --- a/drivers/usb/host/pci-quirks.c
> +++ b/drivers/usb/host/pci-quirks.c
> @@ -1170,7 +1170,8 @@ static void quirk_usb_handoff_xhci(struct pci_dev *pdev)
> /* Auto handoff never worked for these devices. Force it and continue */
> if ((pdev->vendor == PCI_VENDOR_ID_TI &&
> pdev->device == PCI_DEVICE_ID_TI_TUSB73X0) ||
> - (pdev->vendor == PCI_VENDOR_ID_RENESAS && pdev->device == 0x0014)) {
> + (pdev->vendor == PCI_VENDOR_ID_RENESAS &&
> + pdev->device == PCI_DEVICE_ID_RENESAS_UPD720201)) {
> val = (val | XHCI_HC_OS_OWNED) & ~XHCI_HC_BIOS_OWNED;
> writel(val, base + ext_cap_offset);
> }
> @@ -1282,7 +1283,8 @@ bool usb_xhci_needs_pci_reset(struct pci_dev *pdev)
> * quirk, or the system will be in a rather bad state.
> */
> if (pdev->vendor == PCI_VENDOR_ID_RENESAS &&
> - (pdev->device == 0x0014 || pdev->device == 0x0015))
> + (pdev->device == PCI_DEVICE_ID_RENESAS_UPD720201 ||
> + pdev->device == PCI_DEVICE_ID_RENESAS_UPD720202))
> return true;
>
> return false;
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index a5bfd890190c..a453e4c35ac7 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -189,10 +189,10 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
> xhci->quirks |= XHCI_BROKEN_STREAMS;
> }
> if (pdev->vendor == PCI_VENDOR_ID_RENESAS &&
> - pdev->device == 0x0014)
> + pdev->device == PCI_DEVICE_ID_RENESAS_UPD720201)
> xhci->quirks |= XHCI_TRUST_TX_LENGTH;
> if (pdev->vendor == PCI_VENDOR_ID_RENESAS &&
> - pdev->device == 0x0015)
> + pdev->device == PCI_DEVICE_ID_RENESAS_UPD720202)
> xhci->quirks |= XHCI_RESET_ON_RESUME;
> if (pdev->vendor == PCI_VENDOR_ID_VIA)
> xhci->quirks |= XHCI_RESET_ON_RESUME;
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index d23a97868dee..eb52f0e9b651 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -2427,6 +2427,8 @@
> #define PCI_DEVICE_ID_RENESAS_SH7763 0x0004
> #define PCI_DEVICE_ID_RENESAS_SH7785 0x0007
> #define PCI_DEVICE_ID_RENESAS_SH7786 0x0010
> +#define PCI_DEVICE_ID_RENESAS_UPD720201 0x0014
> +#define PCI_DEVICE_ID_RENESAS_UPD720202 0x0015
Now this patch was fine :)
Care to redo this series?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 31+ messages in thread
* [3/3] usb: host: pci: replace hardcoded renesas PCI IDs
@ 2018-03-14 10:49 ` Greg Kroah-Hartman
0 siblings, 0 replies; 31+ messages in thread
From: Greg Kroah-Hartman @ 2018-03-14 10:49 UTC (permalink / raw)
To: Richard Leitner
Cc: linux-usb, linux-kernel, linux-pci, mathias.nyman, bhelgaas,
richard.leitner
On Wed, Mar 14, 2018 at 11:29:33AM +0100, Richard Leitner wrote:
> From: Richard Leitner <richard.leitner@skidata.com>
>
> Introduce Renesas uPD72020{1,2} PCI device IDs in pci_ids.h and replace
> the harcoded values with them.
>
> Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
> ---
> drivers/usb/host/pci-quirks.c | 6 ++++--
> drivers/usb/host/xhci-pci.c | 4 ++--
> include/linux/pci_ids.h | 2 ++
> 3 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
> index 39d163729b89..5e1ad523622e 100644
> --- a/drivers/usb/host/pci-quirks.c
> +++ b/drivers/usb/host/pci-quirks.c
> @@ -1170,7 +1170,8 @@ static void quirk_usb_handoff_xhci(struct pci_dev *pdev)
> /* Auto handoff never worked for these devices. Force it and continue */
> if ((pdev->vendor == PCI_VENDOR_ID_TI &&
> pdev->device == PCI_DEVICE_ID_TI_TUSB73X0) ||
> - (pdev->vendor == PCI_VENDOR_ID_RENESAS && pdev->device == 0x0014)) {
> + (pdev->vendor == PCI_VENDOR_ID_RENESAS &&
> + pdev->device == PCI_DEVICE_ID_RENESAS_UPD720201)) {
> val = (val | XHCI_HC_OS_OWNED) & ~XHCI_HC_BIOS_OWNED;
> writel(val, base + ext_cap_offset);
> }
> @@ -1282,7 +1283,8 @@ bool usb_xhci_needs_pci_reset(struct pci_dev *pdev)
> * quirk, or the system will be in a rather bad state.
> */
> if (pdev->vendor == PCI_VENDOR_ID_RENESAS &&
> - (pdev->device == 0x0014 || pdev->device == 0x0015))
> + (pdev->device == PCI_DEVICE_ID_RENESAS_UPD720201 ||
> + pdev->device == PCI_DEVICE_ID_RENESAS_UPD720202))
> return true;
>
> return false;
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index a5bfd890190c..a453e4c35ac7 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -189,10 +189,10 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
> xhci->quirks |= XHCI_BROKEN_STREAMS;
> }
> if (pdev->vendor == PCI_VENDOR_ID_RENESAS &&
> - pdev->device == 0x0014)
> + pdev->device == PCI_DEVICE_ID_RENESAS_UPD720201)
> xhci->quirks |= XHCI_TRUST_TX_LENGTH;
> if (pdev->vendor == PCI_VENDOR_ID_RENESAS &&
> - pdev->device == 0x0015)
> + pdev->device == PCI_DEVICE_ID_RENESAS_UPD720202)
> xhci->quirks |= XHCI_RESET_ON_RESUME;
> if (pdev->vendor == PCI_VENDOR_ID_VIA)
> xhci->quirks |= XHCI_RESET_ON_RESUME;
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index d23a97868dee..eb52f0e9b651 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -2427,6 +2427,8 @@
> #define PCI_DEVICE_ID_RENESAS_SH7763 0x0004
> #define PCI_DEVICE_ID_RENESAS_SH7785 0x0007
> #define PCI_DEVICE_ID_RENESAS_SH7786 0x0010
> +#define PCI_DEVICE_ID_RENESAS_UPD720201 0x0014
> +#define PCI_DEVICE_ID_RENESAS_UPD720202 0x0015
Now this patch was fine :)
Care to redo this series?
thanks,
greg k-h
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/3] usb: host: pci: introduce PCI vendor ID for Netlogic
@ 2018-03-14 11:36 ` Richard Leitner
0 siblings, 0 replies; 31+ messages in thread
From: Richard Leitner @ 2018-03-14 11:36 UTC (permalink / raw)
To: Greg KH, Richard Leitner
Cc: linux-usb, linux-kernel, linux-pci, mathias.nyman, bhelgaas
On 03/14/2018 11:48 AM, Greg KH wrote:
> On Wed, Mar 14, 2018 at 11:29:32AM +0100, Richard Leitner wrote:
>> From: Richard Leitner <richard.leitner@skidata.com>
>>
>> Replace the hardcoded PCI vendor ID of Netlogic with a definition in
>> pci_ids.h
>
> Why? It's only being used in one file, so it should not be in
> pci_ids.h, right?
It's also used as PCI_VENDOR_NETLOGIC in arch/mips/netlogic/xlp/.
Should this be replaced with PCI_VENDOR_ID_NETLOGIC from pci_ids.h?
>
> thanks,
>
> greg k-h
>
thank you!
regards;Richard.L
^ permalink raw reply [flat|nested] 31+ messages in thread
* [2/3] usb: host: pci: introduce PCI vendor ID for Netlogic
@ 2018-03-14 11:36 ` Richard Leitner
0 siblings, 0 replies; 31+ messages in thread
From: Richard Leitner @ 2018-03-14 11:36 UTC (permalink / raw)
To: Greg KH, Richard Leitner
Cc: linux-usb, linux-kernel, linux-pci, mathias.nyman, bhelgaas
On 03/14/2018 11:48 AM, Greg KH wrote:
> On Wed, Mar 14, 2018 at 11:29:32AM +0100, Richard Leitner wrote:
>> From: Richard Leitner <richard.leitner@skidata.com>
>>
>> Replace the hardcoded PCI vendor ID of Netlogic with a definition in
>> pci_ids.h
>
> Why? It's only being used in one file, so it should not be in
> pci_ids.h, right?
It's also used as PCI_VENDOR_NETLOGIC in arch/mips/netlogic/xlp/.
Should this be replaced with PCI_VENDOR_ID_NETLOGIC from pci_ids.h?
>
> thanks,
>
> greg k-h
>
thank you!
regards;Richard.L
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 3/3] usb: host: pci: replace hardcoded renesas PCI IDs
@ 2018-03-14 11:38 ` Richard Leitner
0 siblings, 0 replies; 31+ messages in thread
From: Richard Leitner @ 2018-03-14 11:38 UTC (permalink / raw)
To: Greg KH, Richard Leitner
Cc: linux-usb, linux-kernel, linux-pci, mathias.nyman, bhelgaas
On 03/14/2018 11:49 AM, Greg KH wrote:
> On Wed, Mar 14, 2018 at 11:29:33AM +0100, Richard Leitner wrote:
>> From: Richard Leitner <richard.leitner@skidata.com>
>>
>> Introduce Renesas uPD72020{1,2} PCI device IDs in pci_ids.h and replace
>> the harcoded values with them.
>>
>> Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
>> ---
>> drivers/usb/host/pci-quirks.c | 6 ++++--
>> drivers/usb/host/xhci-pci.c | 4 ++--
>> include/linux/pci_ids.h | 2 ++
>> 3 files changed, 8 insertions(+), 4 deletions(-)
>>
> Now this patch was fine :)
>
> Care to redo this series?
Sure. Will send a v2 soon.
>
> thanks,
>
> greg k-h
>
thank you!
regards;Richard.L
^ permalink raw reply [flat|nested] 31+ messages in thread
* [3/3] usb: host: pci: replace hardcoded renesas PCI IDs
@ 2018-03-14 11:38 ` Richard Leitner
0 siblings, 0 replies; 31+ messages in thread
From: Richard Leitner @ 2018-03-14 11:38 UTC (permalink / raw)
To: Greg KH, Richard Leitner
Cc: linux-usb, linux-kernel, linux-pci, mathias.nyman, bhelgaas
On 03/14/2018 11:49 AM, Greg KH wrote:
> On Wed, Mar 14, 2018 at 11:29:33AM +0100, Richard Leitner wrote:
>> From: Richard Leitner <richard.leitner@skidata.com>
>>
>> Introduce Renesas uPD72020{1,2} PCI device IDs in pci_ids.h and replace
>> the harcoded values with them.
>>
>> Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
>> ---
>> drivers/usb/host/pci-quirks.c | 6 ++++--
>> drivers/usb/host/xhci-pci.c | 4 ++--
>> include/linux/pci_ids.h | 2 ++
>> 3 files changed, 8 insertions(+), 4 deletions(-)
>>
> Now this patch was fine :)
>
> Care to redo this series?
Sure. Will send a v2 soon.
>
> thanks,
>
> greg k-h
>
thank you!
regards;Richard.L
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/3] usb: host: pci: introduce PCI vendor ID for Netlogic
@ 2018-03-14 11:49 ` Greg Kroah-Hartman
0 siblings, 0 replies; 31+ messages in thread
From: Greg KH @ 2018-03-14 11:49 UTC (permalink / raw)
To: Richard Leitner
Cc: Richard Leitner, linux-usb, linux-kernel, linux-pci,
mathias.nyman, bhelgaas
On Wed, Mar 14, 2018 at 12:36:17PM +0100, Richard Leitner wrote:
>
> On 03/14/2018 11:48 AM, Greg KH wrote:
> > On Wed, Mar 14, 2018 at 11:29:32AM +0100, Richard Leitner wrote:
> >> From: Richard Leitner <richard.leitner@skidata.com>
> >>
> >> Replace the hardcoded PCI vendor ID of Netlogic with a definition in
> >> pci_ids.h
> >
> > Why? It's only being used in one file, so it should not be in
> > pci_ids.h, right?
>
> It's also used as PCI_VENDOR_NETLOGIC in arch/mips/netlogic/xlp/.
>
> Should this be replaced with PCI_VENDOR_ID_NETLOGIC from pci_ids.h?
Yes, if you are going to add it to pci_ids.h, it had better be used by
multiple files, otherwise it does not belong in there.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 31+ messages in thread
* [2/3] usb: host: pci: introduce PCI vendor ID for Netlogic
@ 2018-03-14 11:49 ` Greg Kroah-Hartman
0 siblings, 0 replies; 31+ messages in thread
From: Greg Kroah-Hartman @ 2018-03-14 11:49 UTC (permalink / raw)
To: Richard Leitner
Cc: Richard Leitner, linux-usb, linux-kernel, linux-pci,
mathias.nyman, bhelgaas
On Wed, Mar 14, 2018 at 12:36:17PM +0100, Richard Leitner wrote:
>
> On 03/14/2018 11:48 AM, Greg KH wrote:
> > On Wed, Mar 14, 2018 at 11:29:32AM +0100, Richard Leitner wrote:
> >> From: Richard Leitner <richard.leitner@skidata.com>
> >>
> >> Replace the hardcoded PCI vendor ID of Netlogic with a definition in
> >> pci_ids.h
> >
> > Why? It's only being used in one file, so it should not be in
> > pci_ids.h, right?
>
> It's also used as PCI_VENDOR_NETLOGIC in arch/mips/netlogic/xlp/.
>
> Should this be replaced with PCI_VENDOR_ID_NETLOGIC from pci_ids.h?
Yes, if you are going to add it to pci_ids.h, it had better be used by
multiple files, otherwise it does not belong in there.
thanks,
greg k-h
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/3] usb: host: pci: introduce PCI vendor ID for Netlogic
@ 2018-03-14 12:17 ` Oliver Neukum
0 siblings, 0 replies; 31+ messages in thread
From: Oliver Neukum @ 2018-03-14 12:17 UTC (permalink / raw)
To: Richard Leitner, linux-kernel, linux-pci, linux-usb
Cc: bhelgaas, mathias.nyman, gregkh, richard.leitner
Am Mittwoch, den 14.03.2018, 11:29 +0100 schrieb Richard Leitner:
> From: Richard Leitner <richard.leitner@skidata.com>
>
> Replace the hardcoded PCI vendor ID of Netlogic with a definition in
> pci_ids.h
Hi,
in general, why?
Does this patch generate any benefit for any developer
reading the source? I don't see it. Does it cause an
issue for anybody who has a log file with the nummerical
ID and needs to grep for it? Yes it does.
Where is the point of this patch?
Regards
Oliver
^ permalink raw reply [flat|nested] 31+ messages in thread
* [2/3] usb: host: pci: introduce PCI vendor ID for Netlogic
@ 2018-03-14 12:17 ` Oliver Neukum
0 siblings, 0 replies; 31+ messages in thread
From: Oliver Neukum @ 2018-03-14 12:17 UTC (permalink / raw)
To: Richard Leitner, linux-kernel, linux-pci, linux-usb
Cc: bhelgaas, mathias.nyman, gregkh, richard.leitner
Am Mittwoch, den 14.03.2018, 11:29 +0100 schrieb Richard Leitner:
> From: Richard Leitner <richard.leitner@skidata.com>
>
> Replace the hardcoded PCI vendor ID of Netlogic with a definition in
> pci_ids.h
Hi,
in general, why?
Does this patch generate any benefit for any developer
reading the source? I don't see it. Does it cause an
issue for anybody who has a log file with the nummerical
ID and needs to grep for it? Yes it does.
Where is the point of this patch?
Regards
Oliver
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/3] usb: host: pci: introduce PCI vendor ID for Netlogic
@ 2018-03-14 13:31 ` Richard Leitner
0 siblings, 0 replies; 31+ messages in thread
From: Richard Leitner @ 2018-03-14 13:31 UTC (permalink / raw)
To: Oliver Neukum, Richard Leitner, linux-kernel, linux-pci, linux-usb
Cc: bhelgaas, mathias.nyman, gregkh
Hi Oliver,
thank you for your feedback!
On 03/14/2018 01:17 PM, Oliver Neukum wrote:
> Am Mittwoch, den 14.03.2018, 11:29 +0100 schrieb Richard Leitner:
>> From: Richard Leitner <richard.leitner@skidata.com>
>>
>> Replace the hardcoded PCI vendor ID of Netlogic with a definition in
>> pci_ids.h
>
> Hi,
>
> in general, why?
> Does this patch generate any benefit for any developer
> reading the source? I don't see it. Does it cause an
> issue for anybody who has a log file with the nummerical
> ID and needs to grep for it? Yes it does.
I'll send a v2 where I also use this definition in
arch/mips/netlogic/xlp/ instead of PCI_VENDOR_NETLOGIC from
arch/mips/include/asm/netlogic/xlp-hal/iomap.h.
Therefore it will remove this definition from the iomap.h
and move it to pci_ids.h
This will IMHO be a clear benefit as it removes a redundant
definition.
>
> Where is the point of this patch?
>
> Regards
> Oliver
>
regards;Richard.L
^ permalink raw reply [flat|nested] 31+ messages in thread
* [2/3] usb: host: pci: introduce PCI vendor ID for Netlogic
@ 2018-03-14 13:31 ` Richard Leitner
0 siblings, 0 replies; 31+ messages in thread
From: Richard Leitner @ 2018-03-14 13:31 UTC (permalink / raw)
To: Oliver Neukum, Richard Leitner, linux-kernel, linux-pci, linux-usb
Cc: bhelgaas, mathias.nyman, gregkh
Hi Oliver,
thank you for your feedback!
On 03/14/2018 01:17 PM, Oliver Neukum wrote:
> Am Mittwoch, den 14.03.2018, 11:29 +0100 schrieb Richard Leitner:
>> From: Richard Leitner <richard.leitner@skidata.com>
>>
>> Replace the hardcoded PCI vendor ID of Netlogic with a definition in
>> pci_ids.h
>
> Hi,
>
> in general, why?
> Does this patch generate any benefit for any developer
> reading the source? I don't see it. Does it cause an
> issue for anybody who has a log file with the nummerical
> ID and needs to grep for it? Yes it does.
I'll send a v2 where I also use this definition in
arch/mips/netlogic/xlp/ instead of PCI_VENDOR_NETLOGIC from
arch/mips/include/asm/netlogic/xlp-hal/iomap.h.
Therefore it will remove this definition from the iomap.h
and move it to pci_ids.h
This will IMHO be a clear benefit as it removes a redundant
definition.
>
> Where is the point of this patch?
>
> Regards
> Oliver
>
regards;Richard.L
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/3] usb: host: pci: introduce PCI vendor ID for Netlogic
@ 2018-03-14 15:27 ` Oliver Neukum
0 siblings, 0 replies; 31+ messages in thread
From: Oliver Neukum @ 2018-03-14 15:27 UTC (permalink / raw)
To: Richard Leitner, Richard Leitner, linux-kernel, linux-pci, linux-usb
Cc: bhelgaas, mathias.nyman, gregkh
Am Mittwoch, den 14.03.2018, 14:31 +0100 schrieb Richard Leitner:
> Hi Oliver,
> thank you for your feedback!
>
> On 03/14/2018 01:17 PM, Oliver Neukum wrote:
> > Am Mittwoch, den 14.03.2018, 11:29 +0100 schrieb Richard Leitner:
> > > From: Richard Leitner <richard.leitner@skidata.com>
> > >
> > > Replace the hardcoded PCI vendor ID of Netlogic with a definition in
> > > pci_ids.h
> >
> > Hi,
> >
> > in general, why?
> > Does this patch generate any benefit for any developer
> > reading the source? I don't see it. Does it cause an
> > issue for anybody who has a log file with the nummerical
> > ID and needs to grep for it? Yes it does.
>
> I'll send a v2 where I also use this definition in
> arch/mips/netlogic/xlp/ instead of PCI_VENDOR_NETLOGIC from
> arch/mips/include/asm/netlogic/xlp-hal/iomap.h.
>
> Therefore it will remove this definition from the iomap.h
> and move it to pci_ids.h
>
> This will IMHO be a clear benefit as it removes a redundant
> definition.
Well, but it does not. Removing a redundant definition is a clear
benefit. But you are not removing a definition. You are introducing
a preprocessor constant. Why?
What is its benefit?
Regards
Oliver
^ permalink raw reply [flat|nested] 31+ messages in thread
* [2/3] usb: host: pci: introduce PCI vendor ID for Netlogic
@ 2018-03-14 15:27 ` Oliver Neukum
0 siblings, 0 replies; 31+ messages in thread
From: Oliver Neukum @ 2018-03-14 15:27 UTC (permalink / raw)
To: Richard Leitner, Richard Leitner, linux-kernel, linux-pci, linux-usb
Cc: bhelgaas, mathias.nyman, gregkh
Am Mittwoch, den 14.03.2018, 14:31 +0100 schrieb Richard Leitner:
> Hi Oliver,
> thank you for your feedback!
>
> On 03/14/2018 01:17 PM, Oliver Neukum wrote:
> > Am Mittwoch, den 14.03.2018, 11:29 +0100 schrieb Richard Leitner:
> > > From: Richard Leitner <richard.leitner@skidata.com>
> > >
> > > Replace the hardcoded PCI vendor ID of Netlogic with a definition in
> > > pci_ids.h
> >
> > Hi,
> >
> > in general, why?
> > Does this patch generate any benefit for any developer
> > reading the source? I don't see it. Does it cause an
> > issue for anybody who has a log file with the nummerical
> > ID and needs to grep for it? Yes it does.
>
> I'll send a v2 where I also use this definition in
> arch/mips/netlogic/xlp/ instead of PCI_VENDOR_NETLOGIC from
> arch/mips/include/asm/netlogic/xlp-hal/iomap.h.
>
> Therefore it will remove this definition from the iomap.h
> and move it to pci_ids.h
>
> This will IMHO be a clear benefit as it removes a redundant
> definition.
Well, but it does not. Removing a redundant definition is a clear
benefit. But you are not removing a definition. You are introducing
a preprocessor constant. Why?
What is its benefit?
Regards
Oliver
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/3] usb: host: pci: introduce PCI vendor ID for Netlogic
@ 2018-03-14 15:44 ` Richard Leitner
0 siblings, 0 replies; 31+ messages in thread
From: Richard Leitner @ 2018-03-14 15:44 UTC (permalink / raw)
To: Oliver Neukum, Richard Leitner, linux-kernel, linux-pci, linux-usb
Cc: bhelgaas, mathias.nyman, gregkh
On 03/14/2018 04:27 PM, Oliver Neukum wrote:
> Am Mittwoch, den 14.03.2018, 14:31 +0100 schrieb Richard Leitner:
>> Hi Oliver,
>> thank you for your feedback!
>>
>> On 03/14/2018 01:17 PM, Oliver Neukum wrote:
>>> Am Mittwoch, den 14.03.2018, 11:29 +0100 schrieb Richard Leitner:
>>>> From: Richard Leitner <richard.leitner@skidata.com>
>>>>
>>>> Replace the hardcoded PCI vendor ID of Netlogic with a definition in
>>>> pci_ids.h
>>>
>>> Hi,
>>>
>>> in general, why?
>>> Does this patch generate any benefit for any developer
>>> reading the source? I don't see it. Does it cause an
>>> issue for anybody who has a log file with the nummerical
>>> ID and needs to grep for it? Yes it does.
>>
>> I'll send a v2 where I also use this definition in
>> arch/mips/netlogic/xlp/ instead of PCI_VENDOR_NETLOGIC from
>> arch/mips/include/asm/netlogic/xlp-hal/iomap.h.
>>
>> Therefore it will remove this definition from the iomap.h
>> and move it to pci_ids.h
>>
>> This will IMHO be a clear benefit as it removes a redundant
>> definition.
>
> Well, but it does not. Removing a redundant definition is a clear
> benefit. But you are not removing a definition. You are introducing
> a preprocessor constant. Why?
> What is its benefit?
AFAIK pci_ids.h collects PCI vendor and device IDs in one single
point. As the PCI vendor ID of Netlogic is used in multiple files
IMHO it would be a good idea to add it to pci_ids.h and furthermore
remove it from arch/mips/include/asm/netlogic/xlp-hal/iomap.h (where
it's currently defined).
Or am I getting things wrong?
regards;richard.l
^ permalink raw reply [flat|nested] 31+ messages in thread
* [2/3] usb: host: pci: introduce PCI vendor ID for Netlogic
@ 2018-03-14 15:44 ` Richard Leitner
0 siblings, 0 replies; 31+ messages in thread
From: Richard Leitner @ 2018-03-14 15:44 UTC (permalink / raw)
To: Oliver Neukum, Richard Leitner, linux-kernel, linux-pci, linux-usb
Cc: bhelgaas, mathias.nyman, gregkh
On 03/14/2018 04:27 PM, Oliver Neukum wrote:
> Am Mittwoch, den 14.03.2018, 14:31 +0100 schrieb Richard Leitner:
>> Hi Oliver,
>> thank you for your feedback!
>>
>> On 03/14/2018 01:17 PM, Oliver Neukum wrote:
>>> Am Mittwoch, den 14.03.2018, 11:29 +0100 schrieb Richard Leitner:
>>>> From: Richard Leitner <richard.leitner@skidata.com>
>>>>
>>>> Replace the hardcoded PCI vendor ID of Netlogic with a definition in
>>>> pci_ids.h
>>>
>>> Hi,
>>>
>>> in general, why?
>>> Does this patch generate any benefit for any developer
>>> reading the source? I don't see it. Does it cause an
>>> issue for anybody who has a log file with the nummerical
>>> ID and needs to grep for it? Yes it does.
>>
>> I'll send a v2 where I also use this definition in
>> arch/mips/netlogic/xlp/ instead of PCI_VENDOR_NETLOGIC from
>> arch/mips/include/asm/netlogic/xlp-hal/iomap.h.
>>
>> Therefore it will remove this definition from the iomap.h
>> and move it to pci_ids.h
>>
>> This will IMHO be a clear benefit as it removes a redundant
>> definition.
>
> Well, but it does not. Removing a redundant definition is a clear
> benefit. But you are not removing a definition. You are introducing
> a preprocessor constant. Why?
> What is its benefit?
AFAIK pci_ids.h collects PCI vendor and device IDs in one single
point. As the PCI vendor ID of Netlogic is used in multiple files
IMHO it would be a good idea to add it to pci_ids.h and furthermore
remove it from arch/mips/include/asm/netlogic/xlp-hal/iomap.h (where
it's currently defined).
Or am I getting things wrong?
regards;richard.l
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/3] usb: host: pci: introduce PCI vendor ID for Netlogic
@ 2018-03-15 9:26 ` Oliver Neukum
0 siblings, 0 replies; 31+ messages in thread
From: Oliver Neukum @ 2018-03-15 9:26 UTC (permalink / raw)
To: Richard Leitner, Richard Leitner, linux-kernel, linux-pci, linux-usb
Cc: bhelgaas, mathias.nyman, gregkh
Am Mittwoch, den 14.03.2018, 16:44 +0100 schrieb Richard Leitner:
> On 03/14/2018 04:27 PM, Oliver Neukum wrote:
> > Am Mittwoch, den 14.03.2018, 14:31 +0100 schrieb Richard Leitner:
> > >
> > Well, but it does not. Removing a redundant definition is a clear
> > benefit. But you are not removing a definition. You are introducing
> > a preprocessor constant. Why?
> > What is its benefit?
>
> AFAIK pci_ids.h collects PCI vendor and device IDs in one single
> point. As the PCI vendor ID of Netlogic is used in multiple files
> IMHO it would be a good idea to add it to pci_ids.h and furthermore
> remove it from arch/mips/include/asm/netlogic/xlp-hal/iomap.h (where
> it's currently defined).
>
> Or am I getting things wrong?
I think so, yes. We are giving names to constants as a form
of comment or to change them at multiple places at once and
consistently.
So
#define XYZ_NETDEV_RESET_RETRIES 2
makes clearly sense. So does
#define XYZ_MAGIC_VALUE1 0xab4e
because it tells you that you have a magic value.
But you will never redefine a PCI vendor ID. In fact you
must not. And if you have a comparison like
dev->vID == 0x1234
if you change this to
dev->vID == SOME_VENDOR_ID
what good does this to you? You already knew it was a vendor ID.
Now you can name it at a glance. So what? If you have a device
you will have to check whether you have some OEM version. You
will always go and check the raw number. And if you have a log
and need to check whether the check will be true, you will have
a number.
Using a constant there is nothing but trouble. Yet one more grep.
Regards
Oliver
^ permalink raw reply [flat|nested] 31+ messages in thread
* [2/3] usb: host: pci: introduce PCI vendor ID for Netlogic
@ 2018-03-15 9:26 ` Oliver Neukum
0 siblings, 0 replies; 31+ messages in thread
From: Oliver Neukum @ 2018-03-15 9:26 UTC (permalink / raw)
To: Richard Leitner, Richard Leitner, linux-kernel, linux-pci, linux-usb
Cc: bhelgaas, mathias.nyman, gregkh
Am Mittwoch, den 14.03.2018, 16:44 +0100 schrieb Richard Leitner:
> On 03/14/2018 04:27 PM, Oliver Neukum wrote:
> > Am Mittwoch, den 14.03.2018, 14:31 +0100 schrieb Richard Leitner:
> > >
> > Well, but it does not. Removing a redundant definition is a clear
> > benefit. But you are not removing a definition. You are introducing
> > a preprocessor constant. Why?
> > What is its benefit?
>
> AFAIK pci_ids.h collects PCI vendor and device IDs in one single
> point. As the PCI vendor ID of Netlogic is used in multiple files
> IMHO it would be a good idea to add it to pci_ids.h and furthermore
> remove it from arch/mips/include/asm/netlogic/xlp-hal/iomap.h (where
> it's currently defined).
>
> Or am I getting things wrong?
I think so, yes. We are giving names to constants as a form
of comment or to change them at multiple places at once and
consistently.
So
#define XYZ_NETDEV_RESET_RETRIES 2
makes clearly sense. So does
#define XYZ_MAGIC_VALUE1 0xab4e
because it tells you that you have a magic value.
But you will never redefine a PCI vendor ID. In fact you
must not. And if you have a comparison like
dev->vID == 0x1234
if you change this to
dev->vID == SOME_VENDOR_ID
what good does this to you? You already knew it was a vendor ID.
Now you can name it at a glance. So what? If you have a device
you will have to check whether you have some OEM version. You
will always go and check the raw number. And if you have a log
and need to check whether the check will be true, you will have
a number.
Using a constant there is nothing but trouble. Yet one more grep.
Regards
Oliver
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/3] usb: host: pci: introduce PCI vendor ID for Netlogic
@ 2018-03-15 9:47 ` Richard Leitner
0 siblings, 0 replies; 31+ messages in thread
From: Richard Leitner @ 2018-03-15 9:47 UTC (permalink / raw)
To: Oliver Neukum, Richard Leitner, linux-kernel, linux-pci, linux-usb
Cc: bhelgaas, mathias.nyman, gregkh
On 03/15/2018 10:26 AM, Oliver Neukum wrote:
> Am Mittwoch, den 14.03.2018, 16:44 +0100 schrieb Richard Leitner:
>> On 03/14/2018 04:27 PM, Oliver Neukum wrote:
>>> Am Mittwoch, den 14.03.2018, 14:31 +0100 schrieb Richard Leitner:
>>>>
>>> Well, but it does not. Removing a redundant definition is a clear
>>> benefit. But you are not removing a definition. You are introducing
>>> a preprocessor constant. Why?
>>> What is its benefit?
>>
>> AFAIK pci_ids.h collects PCI vendor and device IDs in one single
>> point. As the PCI vendor ID of Netlogic is used in multiple files
>> IMHO it would be a good idea to add it to pci_ids.h and furthermore
>> remove it from arch/mips/include/asm/netlogic/xlp-hal/iomap.h (where
>> it's currently defined).
>>
>> Or am I getting things wrong?
>
> I think so, yes. We are giving names to constants as a form
> of comment or to change them at multiple places at once and
> consistently.
>
> So
>
> #define XYZ_NETDEV_RESET_RETRIES 2
>
> makes clearly sense. So does
>
> #define XYZ_MAGIC_VALUE1 0xab4e
>
> because it tells you that you have a magic value.
> But you will never redefine a PCI vendor ID. In fact you
> must not. And if you have a comparison like
>
> dev->vID == 0x1234
>
> if you change this to
>
> dev->vID == SOME_VENDOR_ID
>
> what good does this to you? You already knew it was a vendor ID.
> Now you can name it at a glance. So what? If you have a device
> you will have to check whether you have some OEM version. You
> will always go and check the raw number. And if you have a log
> and need to check whether the check will be true, you will have
> a number.
> Using a constant there is nothing but trouble. Yet one more grep.
Thank you for that explanation. But IMHO it was clearer with a
human-readable name in such comparisons...
For example in the following I see at the first glance which
device from which vendor is affected and I don't need any
additional comments or ID databases...
if (pdev->vendor == PCI_VENDOR_ID_TI &&
pdev->device == PCI_DEVICE_ID_TI_TUSB73X0)
...but if that's not the preferred way of doing things I'm
perfectly fine with that.
Furthermore to me it sounds you are saying that the complete
pci_ids.h should be thrown over-board?!?
regards;richard.l
^ permalink raw reply [flat|nested] 31+ messages in thread
* [2/3] usb: host: pci: introduce PCI vendor ID for Netlogic
@ 2018-03-15 9:47 ` Richard Leitner
0 siblings, 0 replies; 31+ messages in thread
From: Richard Leitner @ 2018-03-15 9:47 UTC (permalink / raw)
To: Oliver Neukum, Richard Leitner, linux-kernel, linux-pci, linux-usb
Cc: bhelgaas, mathias.nyman, gregkh
On 03/15/2018 10:26 AM, Oliver Neukum wrote:
> Am Mittwoch, den 14.03.2018, 16:44 +0100 schrieb Richard Leitner:
>> On 03/14/2018 04:27 PM, Oliver Neukum wrote:
>>> Am Mittwoch, den 14.03.2018, 14:31 +0100 schrieb Richard Leitner:
>>>>
>>> Well, but it does not. Removing a redundant definition is a clear
>>> benefit. But you are not removing a definition. You are introducing
>>> a preprocessor constant. Why?
>>> What is its benefit?
>>
>> AFAIK pci_ids.h collects PCI vendor and device IDs in one single
>> point. As the PCI vendor ID of Netlogic is used in multiple files
>> IMHO it would be a good idea to add it to pci_ids.h and furthermore
>> remove it from arch/mips/include/asm/netlogic/xlp-hal/iomap.h (where
>> it's currently defined).
>>
>> Or am I getting things wrong?
>
> I think so, yes. We are giving names to constants as a form
> of comment or to change them at multiple places at once and
> consistently.
>
> So
>
> #define XYZ_NETDEV_RESET_RETRIES 2
>
> makes clearly sense. So does
>
> #define XYZ_MAGIC_VALUE1 0xab4e
>
> because it tells you that you have a magic value.
> But you will never redefine a PCI vendor ID. In fact you
> must not. And if you have a comparison like
>
> dev->vID == 0x1234
>
> if you change this to
>
> dev->vID == SOME_VENDOR_ID
>
> what good does this to you? You already knew it was a vendor ID.
> Now you can name it at a glance. So what? If you have a device
> you will have to check whether you have some OEM version. You
> will always go and check the raw number. And if you have a log
> and need to check whether the check will be true, you will have
> a number.
> Using a constant there is nothing but trouble. Yet one more grep.
Thank you for that explanation. But IMHO it was clearer with a
human-readable name in such comparisons...
For example in the following I see at the first glance which
device from which vendor is affected and I don't need any
additional comments or ID databases...
if (pdev->vendor == PCI_VENDOR_ID_TI &&
pdev->device == PCI_DEVICE_ID_TI_TUSB73X0)
...but if that's not the preferred way of doing things I'm
perfectly fine with that.
Furthermore to me it sounds you are saying that the complete
pci_ids.h should be thrown over-board?!?
regards;richard.l
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2018-03-15 9:57 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-14 10:29 [PATCH 0/3] usb: host: pci: PCI ID consolidation Richard Leitner
2018-03-14 10:29 ` [PATCH 1/3] usb: host: pci: use existing Intel PCI ID macros Richard Leitner
2018-03-14 10:29 ` [1/3] " Richard Leitner
2018-03-14 10:48 ` [PATCH 1/3] " Greg KH
2018-03-14 10:48 ` [1/3] " Greg Kroah-Hartman
2018-03-14 10:29 ` [PATCH 2/3] usb: host: pci: introduce PCI vendor ID for Netlogic Richard Leitner
2018-03-14 10:29 ` [2/3] " Richard Leitner
2018-03-14 10:48 ` [PATCH 2/3] " Greg KH
2018-03-14 10:48 ` [2/3] " Greg Kroah-Hartman
2018-03-14 11:36 ` [PATCH 2/3] " Richard Leitner
2018-03-14 11:36 ` [2/3] " Richard Leitner
2018-03-14 11:49 ` [PATCH 2/3] " Greg KH
2018-03-14 11:49 ` [2/3] " Greg Kroah-Hartman
2018-03-14 12:17 ` [PATCH 2/3] " Oliver Neukum
2018-03-14 12:17 ` [2/3] " Oliver Neukum
2018-03-14 13:31 ` [PATCH 2/3] " Richard Leitner
2018-03-14 13:31 ` [2/3] " Richard Leitner
2018-03-14 15:27 ` [PATCH 2/3] " Oliver Neukum
2018-03-14 15:27 ` [2/3] " Oliver Neukum
2018-03-14 15:44 ` [PATCH 2/3] " Richard Leitner
2018-03-14 15:44 ` [2/3] " Richard Leitner
2018-03-15 9:26 ` [PATCH 2/3] " Oliver Neukum
2018-03-15 9:26 ` [2/3] " Oliver Neukum
2018-03-15 9:47 ` [PATCH 2/3] " Richard Leitner
2018-03-15 9:47 ` [2/3] " Richard Leitner
2018-03-14 10:29 ` [PATCH 3/3] usb: host: pci: replace hardcoded renesas PCI IDs Richard Leitner
2018-03-14 10:29 ` [3/3] " Richard Leitner
2018-03-14 10:49 ` [PATCH 3/3] " Greg KH
2018-03-14 10:49 ` [3/3] " Greg Kroah-Hartman
2018-03-14 11:38 ` [PATCH 3/3] " Richard Leitner
2018-03-14 11:38 ` [3/3] " Richard Leitner
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.