[U-Boot] [PATCH 1/3] usb: ehci-pci: Clarify and cleanup the EHCI controller detection

The detection function of the EHCI PCI controller was really cryptic, add a beefy comment and clean the portion of the code up a bit. No change in the logic of the code.
Signed-off-by: Marek Vasut marex@denx.de Cc: Simon Glass sjg@chromium.org --- drivers/usb/host/ehci-pci.c | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c index 7a1ffe5..991b199 100644 --- a/drivers/usb/host/ehci-pci.c +++ b/drivers/usb/host/ehci-pci.c @@ -54,9 +54,31 @@ static pci_dev_t ehci_find_class(int index) bdf += PCI_BDF(0, 0, 1)) { pci_read_config_dword(bdf, PCI_CLASS_REVISION, &class); - if ((class >> 8 == PCI_CLASS_SERIAL_USB_EHCI) - && !index--) - return bdf; + class >>= 8; + /* + * Here be dragons! In case we have multiple + * PCI EHCI controllers, this function will + * be called multiple times as well. This + * function will scan the PCI busses, always + * starting from bus 0, device 0, function 0, + * until it finds an USB controller. The USB + * stack gives us an 'index' of a controller + * that is currently being registered, which + * is a number, starting from 0 and growing + * in ascending order as controllers are added. + * To avoid probing the same controller in tne + * subsequent runs of this function, we will + * skip 'index - 1' detected controllers and + * report the index'th controller. + */ + if (class != PCI_CLASS_SERIAL_USB_EHCI) + continue; + if (index) { + index--; + continue; + } + /* Return index'th controller. */ + return bdf; } } }

In case the controller is not initialized, we shall not de-initialize it. As the control structure will not be filled, we will produce a null ptr dereference if the controller is not inited.
Signed-off-by: Marek Vasut marex@denx.de Cc: Simon Glass sjg@chromium.org --- drivers/usb/host/ehci-hcd.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index 8bd1eb8..3ef204d 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -201,6 +201,9 @@ static int ehci_shutdown(struct ehci_ctrl *ctrl) int i, ret = 0; uint32_t cmd, reg;
+ if (!ctrl || !ctrl->hcor) + return -EINVAL; + cmd = ehci_readl(&ctrl->hcor->or_usbcmd); cmd &= ~(CMD_PSE | CMD_ASE); ehci_writel(&ctrl->hcor->or_usbcmd, cmd);

On 13 December 2013 21:55, Marek Vasut marex@denx.de wrote:
In case the controller is not initialized, we shall not de-initialize it. As the control structure will not be filled, we will produce a null ptr dereference if the controller is not inited.
Signed-off-by: Marek Vasut marex@denx.de Cc: Simon Glass sjg@chromium.org
Acked-by: Simon Glass sjg@chromium.org
drivers/usb/host/ehci-hcd.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index 8bd1eb8..3ef204d 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -201,6 +201,9 @@ static int ehci_shutdown(struct ehci_ctrl *ctrl) int i, ret = 0; uint32_t cmd, reg;
if (!ctrl || !ctrl->hcor)
return -EINVAL;
cmd = ehci_readl(&ctrl->hcor->or_usbcmd); cmd &= ~(CMD_PSE | CMD_ASE); ehci_writel(&ctrl->hcor->or_usbcmd, cmd);
-- 1.8.4.3

Fix the register access in EHCI HCD. We need to use address of the register as an ehci_writel() argument.
Signed-off-by: Marek Vasut marex@denx.de Cc: Simon Glass sjg@chromium.org --- drivers/usb/host/ehci-hcd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index 3ef204d..17187ca 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -948,7 +948,7 @@ int usb_lowlevel_init(int index, enum usb_init_type init, void **controller) #endif /* Set the high address word (aka segment) for 64-bit controller */ if (ehci_readl(&ehcic[index].hccr->cr_hccparams) & 1) - ehci_writel(ehcic[index].hcor->or_ctrldssegment, 0); + ehci_writel(&ehcic[index].hcor->or_ctrldssegment, 0);
qh_list = &ehcic[index].qh_list;

On 13 December 2013 21:55, Marek Vasut marex@denx.de wrote:
Fix the register access in EHCI HCD. We need to use address of the register as an ehci_writel() argument.
Signed-off-by: Marek Vasut marex@denx.de Cc: Simon Glass sjg@chromium.org
Acked-by: Simon Glass sjg@chromium.org
drivers/usb/host/ehci-hcd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c index 3ef204d..17187ca 100644 --- a/drivers/usb/host/ehci-hcd.c +++ b/drivers/usb/host/ehci-hcd.c @@ -948,7 +948,7 @@ int usb_lowlevel_init(int index, enum usb_init_type init, void **controller) #endif /* Set the high address word (aka segment) for 64-bit controller */ if (ehci_readl(&ehcic[index].hccr->cr_hccparams) & 1)
ehci_writel(ehcic[index].hcor->or_ctrldssegment, 0);
ehci_writel(&ehcic[index].hcor->or_ctrldssegment, 0); qh_list = &ehcic[index].qh_list;
-- 1.8.4.3

On 13 December 2013 21:55, Marek Vasut marex@denx.de wrote:
The detection function of the EHCI PCI controller was really cryptic, add a beefy comment and clean the portion of the code up a bit. No change in the logic of the code.
Signed-off-by: Marek Vasut marex@denx.de Cc: Simon Glass sjg@chromium.org
Acked-by: Simon Glass sjg@chromium.org
drivers/usb/host/ehci-pci.c | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c index 7a1ffe5..991b199 100644 --- a/drivers/usb/host/ehci-pci.c +++ b/drivers/usb/host/ehci-pci.c @@ -54,9 +54,31 @@ static pci_dev_t ehci_find_class(int index) bdf += PCI_BDF(0, 0, 1)) { pci_read_config_dword(bdf, PCI_CLASS_REVISION, &class);
if ((class >> 8 == PCI_CLASS_SERIAL_USB_EHCI)
&& !index--)
return bdf;
class >>= 8;
/*
* Here be dragons! In case we have multiple
* PCI EHCI controllers, this function will
* be called multiple times as well. This
* function will scan the PCI busses, always
* starting from bus 0, device 0, function 0,
* until it finds an USB controller. The USB
* stack gives us an 'index' of a controller
* that is currently being registered, which
* is a number, starting from 0 and growing
* in ascending order as controllers are added.
* To avoid probing the same controller in tne
* subsequent runs of this function, we will
* skip 'index - 1' detected controllers and
* report the index'th controller.
*/
if (class != PCI_CLASS_SERIAL_USB_EHCI)
continue;
if (index) {
index--;
continue;
}
/* Return index'th controller. */
return bdf; } } }
-- 1.8.4.3
participants (2)
-
Marek Vasut
-
Simon Glass