Patchwork [U-Boot,03/11] usb: ehci: generic PCI support

login
register
mail settings
Submitter Simon Glass
Date Dec. 13, 2012, 1:55 a.m.
Message ID <1355363731-10103-4-git-send-email-sjg@chromium.org>
Download mbox | patch
Permalink /patch/205703/
State Changes Requested, archived
Delegated to: Marek Vasut
Headers show

Comments

Simon Glass - Dec. 13, 2012, 1:55 a.m.
From: Vincent Palatin <vpalatin@chromium.org>

Instead of hardcoding the PCI IDs on the USB controller, use the PCI
class to detect them.

Ensure the busmaster bit is properly set in the PCI configuration.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 drivers/usb/host/ehci-pci.c |   37 +++++++++++++++++++++++++++++++++++++
 1 files changed, 37 insertions(+), 0 deletions(-)
Marek Vasut - Dec. 13, 2012, 5:30 p.m.
Dear Simon Glass,

> From: Vincent Palatin <vpalatin@chromium.org>
> 
> Instead of hardcoding the PCI IDs on the USB controller, use the PCI
> class to detect them.
> 
> Ensure the busmaster bit is properly set in the PCI configuration.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>  drivers/usb/host/ehci-pci.c |   37 +++++++++++++++++++++++++++++++++++++
>  1 files changed, 37 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
> index 29af02d..0dc0e6e 100644
> --- a/drivers/usb/host/ehci-pci.c
> +++ b/drivers/usb/host/ehci-pci.c
> @@ -32,6 +32,34 @@ static struct pci_device_id ehci_pci_ids[] = {
>  	{0x12D8, 0x400F},	/* Pericom */
>  	{0, 0}
>  };
> +#else
> +static pci_dev_t ehci_find_class(void)
> +{
> +	int bus;
> +	int devnum;
> +	pci_dev_t bdf;
> +	uint32_t class;
> +
> +	for (bus = 0; bus < pci_last_busno(); bus++) {
> +		for (devnum = 0; devnum < PCI_MAX_PCI_DEVICES-1; devnum++) {
> +			pci_read_config_dword(PCI_BDF(bus, devnum, 0),
> +					      PCI_CLASS_REVISION, &class);
> +			if (class >> 16 == 0xffff)
> +				continue;
> +
> +			for (bdf = PCI_BDF(bus, devnum, 0);
> +					bdf <= PCI_BDF(bus, devnum,
> +						PCI_MAX_PCI_FUNCTIONS - 1);
> +					bdf += PCI_BDF(0, 0, 1)) {
> +				pci_read_config_dword(bdf, PCI_CLASS_REVISION,
> +						      &class);
> +				if (class >> 8 == 0x0c0320)

Can we at least describe this magic please?

> +					return bdf;
> +			}
> +		}
> +	}
> +	return -1;

Let's try to use errno.h please. It won't be consistent for a while, but we'll 
get there.

> +}
>  #endif
> 
>  /*
> @@ -41,8 +69,13 @@ static struct pci_device_id ehci_pci_ids[] = {
>  int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor
> **hcor) {
>  	pci_dev_t pdev;
> +	uint32_t cmd;
> 
> +#ifdef CONFIG_PCI_EHCI_DEVICE
>  	pdev = pci_find_devices(ehci_pci_ids, CONFIG_PCI_EHCI_DEVICE);
> +#else
> +	pdev = ehci_find_class();
> +#endif
>  	if (pdev == -1) {
>  		printf("EHCI host controller not found\n");
>  		return -1;
> @@ -57,6 +90,10 @@ int ehci_hcd_init(int index, struct ehci_hccr **hccr,
> struct ehci_hcor **hcor) (uint32_t)*hccr, (uint32_t)*hcor,
>  			(uint32_t)HC_LENGTH(ehci_readl(&(*hccr)->cr_capbase)));
> 
> +	/* enable busmaster */
> +	pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
> +	cmd |= PCI_COMMAND_MASTER;
> +	pci_write_config_dword(pdev, PCI_COMMAND, cmd);
>  	return 0;
>  }

Best regards,
Marek Vasut
Vincent Palatin - Dec. 13, 2012, 5:49 p.m.
On Thu, Dec 13, 2012 at 9:30 AM, Marek Vasut <marex@denx.de> wrote:
> Dear Simon Glass,
>
>> From: Vincent Palatin <vpalatin@chromium.org>
>>
>> Instead of hardcoding the PCI IDs on the USB controller, use the PCI
>> class to detect them.
>>
>> Ensure the busmaster bit is properly set in the PCI configuration.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>  drivers/usb/host/ehci-pci.c |   37 +++++++++++++++++++++++++++++++++++++
>>  1 files changed, 37 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
>> index 29af02d..0dc0e6e 100644
>> --- a/drivers/usb/host/ehci-pci.c
>> +++ b/drivers/usb/host/ehci-pci.c
>> @@ -32,6 +32,34 @@ static struct pci_device_id ehci_pci_ids[] = {
>>       {0x12D8, 0x400F},       /* Pericom */
>>       {0, 0}
>>  };
>> +#else
>> +static pci_dev_t ehci_find_class(void)
>> +{
>> +     int bus;
>> +     int devnum;
>> +     pci_dev_t bdf;
>> +     uint32_t class;
>> +
>> +     for (bus = 0; bus < pci_last_busno(); bus++) {
>> +             for (devnum = 0; devnum < PCI_MAX_PCI_DEVICES-1; devnum++) {
>> +                     pci_read_config_dword(PCI_BDF(bus, devnum, 0),
>> +                                           PCI_CLASS_REVISION, &class);
>> +                     if (class >> 16 == 0xffff)
>> +                             continue;
>> +
>> +                     for (bdf = PCI_BDF(bus, devnum, 0);
>> +                                     bdf <= PCI_BDF(bus, devnum,
>> +                                             PCI_MAX_PCI_FUNCTIONS - 1);
>> +                                     bdf += PCI_BDF(0, 0, 1)) {
>> +                             pci_read_config_dword(bdf, PCI_CLASS_REVISION,
>> +                                                   &class);
>> +                             if (class >> 8 == 0x0c0320)
>
> Can we at least describe this magic please?

The PCI class word is something like that :
class = [31:24] class code [23:16] subclass [15:8] Prog if [7:0] Revision ID
For this use case :
class code = 0x0C (Serial bus controller)
subclass = 0x03 (USB controller)
Program interface = 0x20 (EHCI)

This is available in include/pci_ids.h as "#define
PCI_CLASS_SERIAL_USB_EHCI       0x0c0320"
so just replacing the numeric constant by this define should be ok ?

>> +                                     return bdf;
>> +                     }
>> +             }
>> +     }
>> +     return -1;
>
> Let's try to use errno.h please. It won't be consistent for a while, but we'll
> get there.
>
>> +}
>>  #endif
>>
>>  /*
>> @@ -41,8 +69,13 @@ static struct pci_device_id ehci_pci_ids[] = {
>>  int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor
>> **hcor) {
>>       pci_dev_t pdev;
>> +     uint32_t cmd;
>>
>> +#ifdef CONFIG_PCI_EHCI_DEVICE
>>       pdev = pci_find_devices(ehci_pci_ids, CONFIG_PCI_EHCI_DEVICE);
>> +#else
>> +     pdev = ehci_find_class();
>> +#endif
>>       if (pdev == -1) {
>>               printf("EHCI host controller not found\n");
>>               return -1;
>> @@ -57,6 +90,10 @@ int ehci_hcd_init(int index, struct ehci_hccr **hccr,
>> struct ehci_hcor **hcor) (uint32_t)*hccr, (uint32_t)*hcor,
>>                       (uint32_t)HC_LENGTH(ehci_readl(&(*hccr)->cr_capbase)));
>>
>> +     /* enable busmaster */
>> +     pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
>> +     cmd |= PCI_COMMAND_MASTER;
>> +     pci_write_config_dword(pdev, PCI_COMMAND, cmd);
>>       return 0;
>>  }
>
> Best regards,
> Marek Vasut
Marek Vasut - Dec. 13, 2012, 5:54 p.m.
Dear Vincent Palatin,

> On Thu, Dec 13, 2012 at 9:30 AM, Marek Vasut <marex@denx.de> wrote:
> > Dear Simon Glass,
> > 
> >> From: Vincent Palatin <vpalatin@chromium.org>
> >> 
> >> Instead of hardcoding the PCI IDs on the USB controller, use the PCI
> >> class to detect them.
> >> 
> >> Ensure the busmaster bit is properly set in the PCI configuration.
> >> 
> >> Signed-off-by: Simon Glass <sjg@chromium.org>
> >> ---
> >> 
> >>  drivers/usb/host/ehci-pci.c |   37
> >>  +++++++++++++++++++++++++++++++++++++ 1 files changed, 37
> >>  insertions(+), 0 deletions(-)
> >> 
> >> diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
> >> index 29af02d..0dc0e6e 100644
> >> --- a/drivers/usb/host/ehci-pci.c
> >> +++ b/drivers/usb/host/ehci-pci.c
> >> @@ -32,6 +32,34 @@ static struct pci_device_id ehci_pci_ids[] = {
> >> 
> >>       {0x12D8, 0x400F},       /* Pericom */
> >>       {0, 0}
> >>  
> >>  };
> >> 
> >> +#else
> >> +static pci_dev_t ehci_find_class(void)
> >> +{
> >> +     int bus;
> >> +     int devnum;
> >> +     pci_dev_t bdf;
> >> +     uint32_t class;
> >> +
> >> +     for (bus = 0; bus < pci_last_busno(); bus++) {
> >> +             for (devnum = 0; devnum < PCI_MAX_PCI_DEVICES-1; devnum++)
> >> { +                     pci_read_config_dword(PCI_BDF(bus, devnum, 0),
> >> +                                           PCI_CLASS_REVISION,
> >> &class); +                     if (class >> 16 == 0xffff)
> >> +                             continue;
> >> +
> >> +                     for (bdf = PCI_BDF(bus, devnum, 0);
> >> +                                     bdf <= PCI_BDF(bus, devnum,
> >> +                                             PCI_MAX_PCI_FUNCTIONS -
> >> 1); +                                     bdf += PCI_BDF(0, 0, 1)) { + 
> >>                            pci_read_config_dword(bdf,
> >> PCI_CLASS_REVISION, +                                                  
> >> &class);
> >> +                             if (class >> 8 == 0x0c0320)
> > 
> > Can we at least describe this magic please?
> 
> The PCI class word is something like that :
> class = [31:24] class code [23:16] subclass [15:8] Prog if [7:0] Revision
> ID For this use case :
> class code = 0x0C (Serial bus controller)
> subclass = 0x03 (USB controller)
> Program interface = 0x20 (EHCI)
> 
> This is available in include/pci_ids.h as "#define
> PCI_CLASS_SERIAL_USB_EHCI       0x0c0320"
> so just replacing the numeric constant by this define should be ok ?

Of course :)
Simon Glass - Dec. 14, 2012, 1:41 a.m.
Hi Marek,

On Thu, Dec 13, 2012 at 9:30 AM, Marek Vasut <marex@denx.de> wrote:
> Dear Simon Glass,
>
>> From: Vincent Palatin <vpalatin@chromium.org>
>>
>> Instead of hardcoding the PCI IDs on the USB controller, use the PCI
>> class to detect them.
>>
>> Ensure the busmaster bit is properly set in the PCI configuration.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>  drivers/usb/host/ehci-pci.c |   37 +++++++++++++++++++++++++++++++++++++
>>  1 files changed, 37 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
>> index 29af02d..0dc0e6e 100644
>> --- a/drivers/usb/host/ehci-pci.c
>> +++ b/drivers/usb/host/ehci-pci.c
>> @@ -32,6 +32,34 @@ static struct pci_device_id ehci_pci_ids[] = {
>>       {0x12D8, 0x400F},       /* Pericom */
>>       {0, 0}
>>  };
>> +#else
>> +static pci_dev_t ehci_find_class(void)
>> +{
>> +     int bus;
>> +     int devnum;
>> +     pci_dev_t bdf;
>> +     uint32_t class;
>> +
>> +     for (bus = 0; bus < pci_last_busno(); bus++) {
>> +             for (devnum = 0; devnum < PCI_MAX_PCI_DEVICES-1; devnum++) {
>> +                     pci_read_config_dword(PCI_BDF(bus, devnum, 0),
>> +                                           PCI_CLASS_REVISION, &class);
>> +                     if (class >> 16 == 0xffff)
>> +                             continue;
>> +
>> +                     for (bdf = PCI_BDF(bus, devnum, 0);
>> +                                     bdf <= PCI_BDF(bus, devnum,
>> +                                             PCI_MAX_PCI_FUNCTIONS - 1);
>> +                                     bdf += PCI_BDF(0, 0, 1)) {
>> +                             pci_read_config_dword(bdf, PCI_CLASS_REVISION,
>> +                                                   &class);
>> +                             if (class >> 8 == 0x0c0320)
>
> Can we at least describe this magic please?

Changed to PCI_CLASS_SERIAL_USB_EHCI.

>
>> +                                     return bdf;
>> +                     }
>> +             }
>> +     }
>> +     return -1;
>
> Let's try to use errno.h please. It won't be consistent for a while, but we'll
> get there.

Yes, although pci_find_devices() doesn't, so I will just check for < 0 below.

>
>> +}
>>  #endif
>>
>>  /*
>> @@ -41,8 +69,13 @@ static struct pci_device_id ehci_pci_ids[] = {
>>  int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor
>> **hcor) {
>>       pci_dev_t pdev;
>> +     uint32_t cmd;
>>
>> +#ifdef CONFIG_PCI_EHCI_DEVICE
>>       pdev = pci_find_devices(ehci_pci_ids, CONFIG_PCI_EHCI_DEVICE);
>> +#else
>> +     pdev = ehci_find_class();
>> +#endif
>>       if (pdev == -1) {
>>               printf("EHCI host controller not found\n");
>>               return -1;
>> @@ -57,6 +90,10 @@ int ehci_hcd_init(int index, struct ehci_hccr **hccr,
>> struct ehci_hcor **hcor) (uint32_t)*hccr, (uint32_t)*hcor,
>>                       (uint32_t)HC_LENGTH(ehci_readl(&(*hccr)->cr_capbase)));
>>
>> +     /* enable busmaster */
>> +     pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
>> +     cmd |= PCI_COMMAND_MASTER;
>> +     pci_write_config_dword(pdev, PCI_COMMAND, cmd);
>>       return 0;
>>  }
>
> Best regards,
> Marek Vasut

Regards,
Simon

Patch

diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
index 29af02d..0dc0e6e 100644
--- a/drivers/usb/host/ehci-pci.c
+++ b/drivers/usb/host/ehci-pci.c
@@ -32,6 +32,34 @@  static struct pci_device_id ehci_pci_ids[] = {
 	{0x12D8, 0x400F},	/* Pericom */
 	{0, 0}
 };
+#else
+static pci_dev_t ehci_find_class(void)
+{
+	int bus;
+	int devnum;
+	pci_dev_t bdf;
+	uint32_t class;
+
+	for (bus = 0; bus < pci_last_busno(); bus++) {
+		for (devnum = 0; devnum < PCI_MAX_PCI_DEVICES-1; devnum++) {
+			pci_read_config_dword(PCI_BDF(bus, devnum, 0),
+					      PCI_CLASS_REVISION, &class);
+			if (class >> 16 == 0xffff)
+				continue;
+
+			for (bdf = PCI_BDF(bus, devnum, 0);
+					bdf <= PCI_BDF(bus, devnum,
+						PCI_MAX_PCI_FUNCTIONS - 1);
+					bdf += PCI_BDF(0, 0, 1)) {
+				pci_read_config_dword(bdf, PCI_CLASS_REVISION,
+						      &class);
+				if (class >> 8 == 0x0c0320)
+					return bdf;
+			}
+		}
+	}
+	return -1;
+}
 #endif
 
 /*
@@ -41,8 +69,13 @@  static struct pci_device_id ehci_pci_ids[] = {
 int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor **hcor)
 {
 	pci_dev_t pdev;
+	uint32_t cmd;
 
+#ifdef CONFIG_PCI_EHCI_DEVICE
 	pdev = pci_find_devices(ehci_pci_ids, CONFIG_PCI_EHCI_DEVICE);
+#else
+	pdev = ehci_find_class();
+#endif
 	if (pdev == -1) {
 		printf("EHCI host controller not found\n");
 		return -1;
@@ -57,6 +90,10 @@  int ehci_hcd_init(int index, struct ehci_hccr **hccr, struct ehci_hcor **hcor)
 			(uint32_t)*hccr, (uint32_t)*hcor,
 			(uint32_t)HC_LENGTH(ehci_readl(&(*hccr)->cr_capbase)));
 
+	/* enable busmaster */
+	pci_read_config_dword(pdev, PCI_COMMAND, &cmd);
+	cmd |= PCI_COMMAND_MASTER;
+	pci_write_config_dword(pdev, PCI_COMMAND, cmd);
 	return 0;
 }