On Tue, 2014-07-08 at 22:21 +0200, Roman Byshko wrote:
> diff --git a/drivers/usb/host/ehci-sunxi.c b/drivers/usb/host/ehci-sunxi.c
> new file mode 100644
> index 0000000..5817fc7
> --- /dev/null
> +++ b/drivers/usb/host/ehci-sunxi.c
> @@ -0,0 +1,236 @@
> +/*
> + * Copyright (C) 2014 arokux
> + *
> + * arokux <aro...@gmail.com>

The authorship info here contradicts the authorship suggested by the
commit email metadata and the S-o-b.

If arokux wrote this patch then the commit message should begin with 

        From: Arokux X <aro...@gmail.com>
        
on the first line and arokux needs to have Signed-off on the patch.

I'm not sure what u-boot policy says about pseudonyms.

> + *
> + * Based on code from
> + * Allwinner Technology Co., Ltd. <www.allwinnertech.com>
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.

Please use the SPDX tag instead of the full license.

> +     dat = readl(&pio->dat);
> +     if (val)
> +             dat |= 0x1 << num;
> +     else
> +             dat &= ~(0x1 << num);
> +
> +     writel(dat, &pio->dat);

This is 
        if (val)
                setbits_le32(&pio->dat, 1<<num);
        else
                clrbits_le32(&pio->dat, 1<<num);

It seems like there are a bunch of similar constructs around the place.

> +     return 0;
> +}
> +
> +static void usb_phy_write(struct sunxi_ehci_hcd *sunxi_ehci, int addr,
> +                       int data, int len)
> +{
> +     int temp = 0, j = 0, usbc_bit = 0;
> +     void *dest = sunxi_ehci->csr;
> +
> +     usbc_bit = BIT(sunxi_ehci->id * 2);
> +     for (j = 0; j < len; j++) {
> +             /* set the bit address to be written */
> +             temp = readl(dest);
> +             temp &= ~(0xff << 8);
> +             temp |= ((addr + j) << 8);
> +             writel(temp, dest);

clrsetbits?

> +
> +             clrbits_le32(dest, usbc_bit);
> +             /* set data bit */
> +             if (data & 0x1)
> +                     temp |= BIT(7);
> +             else
> +                     temp &= ~BIT(7);
> +             writeb(temp, dest);

AFAICT this will clobber the clearing of usbc_bit which you just did,
since temp was read before that, is that right?

This should probably use the clr/setbits_le32 stuff and if it really is
deliberately clobbering usbc_bit I think it needs a comment.

> +
> +             setbits_le32(dest, usbc_bit);
> +
> +             clrbits_le32( dest, usbc_bit);

Extra space in this line.

> +     /* gpio_direction_output(sunxi_ehci->gpio_vbus, 1); */

???

> +static void sunxi_ehci_disable(struct sunxi_ehci_hcd *sunxi_ehci)
> +{
> +     struct sunxi_ccm_reg *ccm = (struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
> +
> +     /* gpio_direction_output(sunxi_ehci->gpio_vbus, 0); */

???

Ian.

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to