[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20111122195714.GA2815@xanatos>
Date: Tue, 22 Nov 2011 11:57:14 -0800
From: Sarah Sharp <sarah.a.sharp@...ux.intel.com>
To: Aman Deep <amandeep3986@...il.com>
Cc: Greg Kroah-Hartman <gregkh@...e.de>, linux-usb@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] xHCI: Adding #define values used for hub descriptor
On Tue, Nov 22, 2011 at 07:33:36PM +0530, Aman Deep wrote:
> xhci-hub used some numerical values for initialisation of root hub
> descriptors. #define values are addded in usb 2.0 hub specification
> file and these values are used for root hub characteristics
> initialisation.
>
> Also use some #defines in places where magic numbers are being used.
Looks fine to me for the xHCI changes. I copied most of the hub
descriptor code from EHCI, so it will need the same treatment.
Acked-by: Sarah Sharp <sarah.a.sharp@...ux.intel.com>
The #defines look a bit odd to me, like you're using spaces instead of
tabs to separate the name and value, or your mail program is converting
tabs to spaces. Do the numbers line up for you?
> Signed-off-by: Aman Deep <amandeep3986@...il.com>
> ---
> drivers/usb/host/xhci-hub.c | 18 ++++++++----------
> include/linux/usb/ch11.h | 19 ++++++++++++++-----
> 2 files changed, 22 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
> index 430e88f..35e257f 100644
> --- a/drivers/usb/host/xhci-hub.c
> +++ b/drivers/usb/host/xhci-hub.c
> @@ -57,17 +57,15 @@ static void xhci_common_hub_descriptor(struct xhci_hcd *xhci,
> desc->bHubContrCurrent = 0;
>
> desc->bNbrPorts = ports;
> - /* Ugh, these should be #defines, FIXME */
> - /* Using table 11-13 in USB 2.0 spec. */
> temp = 0;
> - /* Bits 1:0 - support port power switching, or power always on */
> + /* Bits 1:0 - support per-port power switching, or power always on */
> if (HCC_PPC(xhci->hcc_params))
> - temp |= 0x0001;
> + temp |= HUB_CHAR_INDV_PORT_LPSM;
> else
> - temp |= 0x0002;
> + temp |= HUB_CHAR_NO_LPSM;
> /* Bit 2 - root hubs are not part of a compound device */
> /* Bits 4:3 - individual port over current protection */
> - temp |= 0x0008;
> + temp |= HUB_CHAR_INDV_PORT_OCPM;
> /* Bits 6:5 - no TTs in root ports */
> /* Bit 7 - no port indicators */
> desc->wHubCharacteristics = cpu_to_le16(temp);
> @@ -86,9 +84,9 @@ static void xhci_usb2_hub_descriptor(struct usb_hcd *hcd, struct xhci_hcd *xhci,
> ports = xhci->num_usb2_ports;
>
> xhci_common_hub_descriptor(xhci, desc, ports);
> - desc->bDescriptorType = 0x29;
> + desc->bDescriptorType = USB_DT_HUB;
> temp = 1 + (ports / 8);
> - desc->bDescLength = 7 + 2 * temp;
> + desc->bDescLength = USB_DT_HUB_NONVAR_SIZE + 2 * temp;
>
> /* The Device Removable bits are reported on a byte granularity.
> * If the port doesn't exist within that byte, the bit is set to 0.
> @@ -137,8 +135,8 @@ static void xhci_usb3_hub_descriptor(struct usb_hcd *hcd, struct xhci_hcd *xhci,
>
> ports = xhci->num_usb3_ports;
> xhci_common_hub_descriptor(xhci, desc, ports);
> - desc->bDescriptorType = 0x2a;
> - desc->bDescLength = 12;
> + desc->bDescriptorType = USB_DT_SS_HUB;
> + desc->bDescLength = USB_DT_SS_HUB_SIZE;
>
> /* header decode latency should be zero for roothubs,
> * see section 4.23.5.2.
> diff --git a/include/linux/usb/ch11.h b/include/linux/usb/ch11.h
> index 4ebaf08..537f354 100644
> --- a/include/linux/usb/ch11.h
> +++ b/include/linux/usb/ch11.h
> @@ -165,11 +165,20 @@ struct usb_port_status {
> * wHubCharacteristics (masks)
> * See USB 2.0 spec Table 11-13, offset 3
> */
> -#define HUB_CHAR_LPSM 0x0003 /* D1 .. D0 */
> -#define HUB_CHAR_COMPOUND 0x0004 /* D2 */
> -#define HUB_CHAR_OCPM 0x0018 /* D4 .. D3 */
> -#define HUB_CHAR_TTTT 0x0060 /* D6 .. D5 */
> -#define HUB_CHAR_PORTIND 0x0080 /* D7 */
> +#define HUB_CHAR_LPSM 0x0003 /* Logical Power Switching Mode mask */
> +#define HUB_CHAR_COMMON_LPSM 0x0000 /* All ports power control at once */
> +#define HUB_CHAR_INDV_PORT_LPSM 0x0001 /* per-port power control */
> +#define HUB_CHAR_NO_LPSM 0x0002 /* no power switching */
> +
> +#define HUB_CHAR_COMPOUND 0x0004 /* hub is part of a compound device */
> +
> +#define HUB_CHAR_OCPM 0x0018 /* Over-Current Protection Mode mask */
> +#define HUB_CHAR_COMMON_OCPM 0x0000 /* All ports Over-Current reporting */
> +#define HUB_CHAR_INDV_PORT_OCPM 0x0008 /* per-port Over-current reporting */
> +#define HUB_CHAR_NO_OCPM 0x0010 /* No Over-current Protection support */
> +
> +#define HUB_CHAR_TTTT 0x0060 /* TT Think Time mask */
> +#define HUB_CHAR_PORTIND 0x0080 /* per-port indicators (LEDs) */
>
> struct usb_hub_status {
> __le16 wHubStatus;
> --
> 1.7.6
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists