On Mon, Apr 04, 2016 at 10:38:37AM -0400, Alan Stern wrote:
> On Mon, 4 Apr 2016, Navin P.S wrote:
>
> > I was proposing something like the attached.
> > When you have an oppurtunity to fix or be complaint let us use that so.
> >
> > I finally leave this here for you to accept or reject.
>
> It's hard to comment on attachments. You should put your patches in
> the body of the email.
>
> Here's your patch:
>
> diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
> index ffc9029..8e7e4a7 100644
> --- a/drivers/usb/host/ehci-hub.c
> +++ b/drivers/usb/host/ehci-hub.c
> @@ -872,9 +872,8 @@ int ehci_hub_control(
> ) {
> struct ehci_hcd *ehci = hcd_to_ehci (hcd);
> int ports = HCS_N_PORTS (ehci->hcs_params);
> - u32 __iomem *status_reg = &ehci->regs->port_status[
> - (wIndex & 0xff) - 1];
> - u32 __iomem *hostpc_reg = &ehci->regs->hostpc[(wIndex & 0xff) - 1];
> + u32 __iomem *status_reg = NULL;
> + u32 __iomem *hostpc_reg = NULL;
> u32 temp, temp1, status;
> unsigned long flags;
> int retval = 0;
> @@ -902,6 +901,9 @@ int ehci_hub_control(
> case ClearPortFeature:
> if (!wIndex || wIndex > ports)
> goto error;
> +
> + status_reg = &ehci->regs->port_status[ (wIndex & 0xff) - 1];
> + hostpc_reg = &ehci->regs->hostpc[(wIndex & 0xff) - 1];
> wIndex--;
> temp = ehci_readl(ehci, status_reg);
> temp &= ~PORT_RWC_BITS;
> @@ -990,6 +992,8 @@ int ehci_hub_control(
> case GetPortStatus:
> if (!wIndex || wIndex > ports)
> goto error;
> + status_reg = &ehci->regs->port_status[ (wIndex & 0xff) - 1];
> + hostpc_reg = &ehci->regs->hostpc[(wIndex & 0xff) - 1];
> wIndex--;
> status = 0;
> temp = ehci_readl(ehci, status_reg);
> @@ -1159,6 +1163,8 @@ int ehci_hub_control(
> }
> if (!wIndex || wIndex > ports)
> goto error;
> + status_reg = &ehci->regs->port_status[ (wIndex & 0xff) - 1];
> + hostpc_reg = &ehci->regs->hostpc[(wIndex & 0xff) - 1];
> wIndex--;
> temp = ehci_readl(ehci, status_reg);
> if (temp & PORT_OWNER)
>
> You want to replace 2 statements with 8 statements that do exactly
> the same thing? This does not seem like an improvement.
>
I think Yes i have replaced 2 incorrect statements with 8 correct ones
due to below reason.
Ubsan is a feature that is enabled .config , it was supposed to
fix things that were caught at runtime. If ubsan produces an error at
runtime, it gives us an oppurtunity to fix . Ignoring it would defeat
the purpose of ubsan=y in .config.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
