On Tuesday, July 15, 2014 at 11:56:49 PM, Roman Byshko wrote:
Please start writing commit messages for the patches. [...]
diff --git a/drivers/usb/host/ehci-sunxi.c b/drivers/usb/host/ehci-sunxi.c new file mode 100644 index 0000000..8e2baa9 --- /dev/null +++ b/drivers/usb/host/ehci-sunxi.c @@ -0,0 +1,212 @@ +/*
- Copyright (C) 2014 Roman Byshko
- Roman Byshko rbyshko@gmail.com
- Based on code from
- Allwinner Technology Co., Ltd. <www.allwinnertech.com>
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <asm/arch/clock.h> +#include <asm/arch/clock.h> +#include <asm/arch/cpu.h> +#include <asm/arch/gpio.h> +#include <asm/io.h> +#include <common.h> +#include "ehci.h"
+#define BIT(x) (1 << (x))
Please remove this code obfuscation, just use (1 << n) in the definitions below.
+#define SUNXI_USB1_IO_BASE 0x01c14000 +#define SUNXI_USB2_IO_BASE 0x01c1c000
Please implement an get_io_base() or such function, since these addresses are likely to change eventually.
+#define SUNXI_USB_PMU_IRQ_ENABLE 0x800 +#define SUNXI_USB_CSR 0x01c13404 +#define SUNXI_USB_PASSBY_EN 1
+#define SUNXI_EHCI_AHB_ICHR8_EN BIT(10) +#define SUNXI_EHCI_AHB_INCR4_BURST_EN BIT(9) +#define SUNXI_EHCI_AHB_INCRX_ALIGN_EN BIT(8) +#define SUNXI_EHCI_ULPI_BYPASS_EN BIT(0)
+static struct sunxi_ehci_hcd {
- void *ehci_base;
- struct usb_hcd *hcd;
- int usb_rst_mask;
- int ahb_clk_mask;
- int gpio_vbus;
- void *csr;
- int irq;
- int id;
+} sunxi_echi_hcd[CONFIG_USB_MAX_CONTROLLER_COUNT] = {
No need to use this [CONFIG...] , just use [] and the compiler will calculate the size.
- [0] = {
No need for such explicit enumeration.
.ehci_base = (void *) SUNXI_USB1_IO_BASE,.usb_rst_mask = CCM_USB_CTRL_PHY1_RST,.ahb_clk_mask = BIT(AHB_GATE_OFFSET_USB_EHCI0),.gpio_vbus = CONFIG_SUNXI_USB_VBUS0_GPIO,.csr = (void*) SUNXI_USB_CSR,.irq = 39,.id = 1,- },
+#if (CONFIG_USB_MAX_CONTROLLER_COUNT > 1)
- [1] = {
.ehci_base = (void *) SUNXI_USB2_IO_BASE,.usb_rst_mask = CCM_USB_CTRL_PHY2_RST,.ahb_clk_mask = BIT(AHB_GATE_OFFSET_USB_EHCI1),.gpio_vbus = CONFIG_SUNXI_USB_VBUS1_GPIO,.csr = (void*) SUNXI_USB_CSR,.irq = 40,.id = 2,- }
+#endif +};
+static int sunxi_gpio_output(u32 pin, u32 val) +{
- u32 bank = GPIO_BANK(pin);
- u32 num = GPIO_NUM(pin);
- struct sunxi_gpio *pio =
&((struct sunxi_gpio_reg *)SUNXI_PIO_BASE)->gpio_bank[bank];
Is this still an USB driver or is this now a GPIO driver ?
- if (val)
setbits_le32(&pio->dat, 0x1 << num);- else
clrbits_le32(&pio->dat, 0x1 << num);- return 0;
+}
[...]
+static void sunxi_ehci_enable(struct sunxi_ehci_hcd *sunxi_ehci) +{
- struct sunxi_ccm_reg *ccm = (struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
- setbits_le32(&ccm->usb_clk_cfg, sunxi_ehci->usb_rst_mask);
- setbits_le32(&ccm->ahb_gate0, sunxi_ehci->ahb_clk_mask);
- sunxi_usb_phy_init(sunxi_ehci);
- sunxi_usb_passby(sunxi_ehci, SUNXI_USB_PASSBY_EN);
- /* this should be used instead of next two lines if
* sunxi_gpio.c is merged upstream* gpio_direction_output(sunxi_ehci->gpio_vbus, 1); */
Please fix the comment ( http://www.denx.de/wiki/U-Boot/CodingStyle )
- sunxi_gpio_set_cfgpin(sunxi_ehci->gpio_vbus, SUNXI_GPIO_OUTPUT);
- sunxi_gpio_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;
- /* this should be used instead of next two lines if
* sunxi_gpio.c is merged upstream* gpio_direction_output(sunxi_ehci->gpio_vbus, 0); */
DTTO.
- sunxi_gpio_set_cfgpin(sunxi_ehci->gpio_vbus, SUNXI_GPIO_OUTPUT);
- sunxi_gpio_output(sunxi_ehci->gpio_vbus, 0);
- sunxi_usb_passby(sunxi_ehci, !SUNXI_USB_PASSBY_EN);
- clrbits_le32(&ccm->ahb_gate0, sunxi_ehci->ahb_clk_mask);
- clrbits_le32(&ccm->usb_clk_cfg, sunxi_ehci->usb_rst_mask);
+}
+int ehci_hcd_init(int index, enum usb_init_type init, struct ehci_hccr **hccr, + struct ehci_hcor **hcor) +{
- struct sunxi_ccm_reg *ccm = (struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
- struct sunxi_ehci_hcd *sunxi_ehci = &sunxi_echi_hcd[index];
- /* enable common PHY only once */
- if (index == 0)
setbits_le32(&ccm->usb_clk_cfg, CCM_USB_CTRL_PHYGATE);
This would fail if I enabled only controller #1 , which is perfectly legal operation. Just add a counter here and disable the clock upon last call of ehci_hcd_stop() .
- sunxi_ehci_enable(sunxi_ehci);
- *hccr = sunxi_ehci->ehci_base;
- *hcor = (struct ehci_hcor *)((uint32_t) *hccr
+ HC_LENGTH(ehci_readl(&(*hccr)->cr_capbase)));- debug("sunxi-ehci: init hccr %x and hcor %x hc_length %d\n",
(uint32_t)*hccr, (uint32_t)*hcor,(uint32_t)HC_LENGTH(ehci_readl(&(*hccr)->cr_capbase)));- return 0;
+}
+int ehci_hcd_stop(int index) +{
- struct sunxi_ccm_reg *ccm = (struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
- struct sunxi_ehci_hcd *sunxi_ehci = &sunxi_echi_hcd[index];
- sunxi_ehci_disable(sunxi_ehci);
- /* disable common PHY only once, for the last hcd */
- if (index == CONFIG_USB_MAX_CONTROLLER_COUNT - 1)
clrbits_le32(&ccm->usb_clk_cfg, CCM_USB_CTRL_PHYGATE);- return 0;
+}