On 03.01.2023 01:26, Marek Vasut wrote:
On 12/23/22 13:34, Sergiu Moga wrote:
[...]
diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c index 9b955c1bd6..9ae55c6e5d 100644 --- a/drivers/usb/host/ohci-at91.c +++ b/drivers/usb/host/ohci-at91.c @@ -5,6 +5,9 @@ */
#include <common.h>
+#if !(CONFIG_IS_ENABLED(DM_USB))
#include <asm/arch/clk.h>
int usb_cpu_init(void) @@ -62,3 +65,183 @@ int usb_cpu_init_fail(void) { return usb_cpu_stop(); }
Would it be possible to just remove the non-DM functionality ?
Unfortunately, the other older boards would not build and work anymore if I were to remove this.
+#elif CONFIG_IS_ENABLED(DM_GPIO)
I think you want plain '#else' here, and then make the driver select DM_GPIO in case DM_USB is enabled in Kconfig entry.
+#include <clk.h> +#include <dm.h> +#include <asm/gpio.h> +#include <usb.h> +#include "ohci.h"
+#define AT91_MAX_USBH_PORTS 3
+#define at91_for_each_port(index) \ + for ((index) = 0; (index) < AT91_MAX_USBH_PORTS; (index)++)
+struct at91_usbh_data { + enum usb_init_type init_type; + struct gpio_desc vbus_pin[AT91_MAX_USBH_PORTS];
drivers/usb/host/ehci-generic.c: ret = device_get_supply_regulator(dev, "vbus-supply",
I wonder if we can instead use regulator for vbus-supply here too ?
I would rather keep the same DT properties as those in Linux and have them be in concordance with our USB DT binding, which specifies the atmel,vbus-gpio property and no vbus-supply.
Other than that, I am fine with the other requested changes.
+ u8 ports; /* number of ports on root hub */ +};
+struct ohci_at91_priv { + struct clk *iclk; + struct clk *fclk; + struct clk *hclk;
Have a look at clk.*bulk functions in U-Boot , then you can do clk_get_bulk() / clk_enable_bulk() etc. below.
+ bool clocked; +};
+static void at91_start_clock(struct ohci_at91_priv *ohci_at91) +{ + if (ohci_at91->clocked) + return;
+ clk_set_rate(ohci_at91->fclk, 48000000); + clk_prepare_enable(ohci_at91->hclk); + clk_prepare_enable(ohci_at91->iclk); + clk_prepare_enable(ohci_at91->fclk);
E.g. here you can use clk_enable_bulk() .
+ ohci_at91->clocked = true;
Why is this here, are you suffering from clock inbalance ? You shouldn't, so remove this and the if (ohci_at91->clocked) test above.
+}
+static void at91_stop_clock(struct ohci_at91_priv *ohci_at91) +{ + if (!ohci_at91->clocked)
Is this really needed ?
+ return;
+ clk_disable_unprepare(ohci_at91->fclk); + clk_disable_unprepare(ohci_at91->iclk); + clk_disable_unprepare(ohci_at91->hclk); + ohci_at91->clocked = false; +}
+static void ohci_at91_set_power(struct at91_usbh_data *pdata, int port, + bool enable) +{ + if (!dm_gpio_is_valid(&pdata->vbus_pin[port])) + return;
This could be regulator instead.
+ if (enable) + dm_gpio_set_dir_flags(&pdata->vbus_pin[port], + GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE); + else + dm_gpio_set_dir_flags(&pdata->vbus_pin[port], 0); +}
+static void at91_start_hc(struct udevice *dev) +{ + struct ohci_at91_priv *ohci_at91 = dev_get_priv(dev);
+ at91_start_clock(ohci_at91); +}
+static void at91_stop_hc(struct udevice *dev) +{ + struct ohci_at91_priv *ohci_at91 = dev_get_priv(dev);
+ at91_stop_clock(ohci_at91); +}
+static int ohci_atmel_deregister(struct udevice *dev) +{ + struct at91_usbh_data *pdata = dev_get_plat(dev); + int i;
+ at91_stop_hc(dev);
+ at91_for_each_port(i) { + if (i >= pdata->ports)
You can just wrap this test into the at91_for_each_port() macro , just use "index < min(AT91_MAX_USBH_PORTS, pdata->ports)" or so.
+ break;
+ ohci_at91_set_power(pdata, i, false); + }
+ return ohci_deregister(dev); +}
+static int ohci_atmel_child_pre_probe(struct udevice *dev) +{ + struct udevice *ohci_controller = dev_get_parent(dev); + struct at91_usbh_data *pdata = dev_get_plat(ohci_controller); + int i;
+ at91_for_each_port(i) { + if (i >= pdata->ports) + break;
+ ohci_at91_set_power(pdata, i, true); + }
+ return 0; +}
+static int ohci_atmel_probe(struct udevice *dev) +{ + struct at91_usbh_data *pdata = dev_get_plat(dev); + struct ohci_at91_priv *ohci_at91 = dev_get_priv(dev); + int i; + int ret; + u32 ports; + struct ohci_regs *regs = (struct ohci_regs *)dev_read_addr(dev);
+ if (!dev_read_u32(dev, "num-ports", &ports)) + pdata->ports = ports;
dev_read_u32_default() ?
+ at91_for_each_port(i) { + if (i >= pdata->ports) + break;
+ gpio_request_by_name(dev, "atmel,vbus-gpio", i, + &pdata->vbus_pin[i], GPIOD_IS_OUT | + GPIOD_IS_OUT_ACTIVE); + }
+ ohci_at91->iclk = devm_clk_get(dev, "ohci_clk"); + if (IS_ERR(ohci_at91->iclk)) { + ret = PTR_ERR(ohci_at91->iclk); + goto fail; + }
+ ohci_at91->fclk = devm_clk_get(dev, "uhpck"); + if (IS_ERR(ohci_at91->fclk)) { + ret = PTR_ERR(ohci_at91->fclk); + goto fail; + }
+ ohci_at91->hclk = devm_clk_get(dev, "hclk"); + if (IS_ERR(ohci_at91->hclk)) { + ret = PTR_ERR(ohci_at91->hclk);
clk_get_bulk() would simplify this.
[...]