[PATCH 0/4] Register at91 OHCI into DM and add SAMA7 USB PHY's
This patch series originates from a bigger patch series: https://lists.denx.de/pipermail/u-boot/2022-December/502865.html
Register ohci-at91 driver into Driver Model. In order for the VBUS to stay enabled, a `child_pre_probe` method has been added to overcome the DM core disabling it in `usb_scan_device`: when the generic `device_probe` method is called, the pinctrl is processed once again, undoing whatever changes have been made in our driver's probe method. In order to enable USB on SAMA7G5 the addition of the USB 2.0 PHY drivers were required.
Cristian Birsan (1): usb: ohci-at91: Add `ohci_t` field in `ohci_at91_priv`
Sergiu Moga (3): phy: at91: Add support for the USB 2.0 PHY's of SAMA7 usb: ohci-at91: Enable OHCI functionality and register into DM usb: ohci-at91: Add USB PHY functionality
drivers/phy/Kconfig | 10 ++ drivers/phy/Makefile | 1 + drivers/phy/phy-sama7-usb.c | 90 +++++++++++++ drivers/phy/phy-sama7-utmi-clk.c | 202 +++++++++++++++++++++++++++++ drivers/usb/host/ohci-at91.c | 215 +++++++++++++++++++++++++++++++ 5 files changed, 518 insertions(+) create mode 100644 drivers/phy/phy-sama7-usb.c create mode 100644 drivers/phy/phy-sama7-utmi-clk.c
In order to have USB functionality, drivers for SAMA7's USB 2.0 PHY's have been added. There is one driver for UTMI clock's SFR and RESET required functionalities and one for its three possible subclocks of the phy's themselves. In order for this layout to properly work in conjunction with CCF and DT, the former driver will also act as a clock provider for the three phy's with the help of a custom hook into the driver's of_xlate method.
Signed-off-by: Sergiu Moga sergiu.moga@microchip.com Tested-by: Mihai Sain mihai.sain@microchip.com --- drivers/phy/Kconfig | 10 ++ drivers/phy/Makefile | 1 + drivers/phy/phy-sama7-usb.c | 90 ++++++++++++++ drivers/phy/phy-sama7-utmi-clk.c | 202 +++++++++++++++++++++++++++++++ 4 files changed, 303 insertions(+) create mode 100644 drivers/phy/phy-sama7-usb.c create mode 100644 drivers/phy/phy-sama7-utmi-clk.c
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index cf4d5908d7..9fbb956783 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig @@ -281,6 +281,16 @@ config PHY_XILINX_ZYNQMP Enable this to support ZynqMP High Speed Gigabit Transceiver that is part of ZynqMP SoC.
+config PHY_MICROCHIP_SAMA7_USB + tristate "Microchip SAMA7 USB 2.0 PHY" + depends on PHY && ARCH_AT91 + help + Enable this to support SAMA7 USB 2.0 PHY. + + The USB 2.0 PHY integrates high-speed, full-speed and low-speed + termination and signal switching. With a single resistor, it + requires minimal external components. + source "drivers/phy/rockchip/Kconfig" source "drivers/phy/cadence/Kconfig" source "drivers/phy/ti/Kconfig" diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile index a3b9f3c5b1..9d50affd47 100644 --- a/drivers/phy/Makefile +++ b/drivers/phy/Makefile @@ -38,6 +38,7 @@ obj-$(CONFIG_PHY_MTK_TPHY) += phy-mtk-tphy.o obj-$(CONFIG_PHY_NPCM_USB) += phy-npcm-usb.o obj-$(CONFIG_PHY_IMX8MQ_USB) += phy-imx8mq-usb.o obj-$(CONFIG_PHY_XILINX_ZYNQMP) += phy-zynqmp.o +obj-$(CONFIG_PHY_MICROCHIP_SAMA7_USB) += phy-sama7-utmi-clk.o phy-sama7-usb.o obj-y += cadence/ obj-y += ti/ obj-y += qcom/ diff --git a/drivers/phy/phy-sama7-usb.c b/drivers/phy/phy-sama7-usb.c new file mode 100644 index 0000000000..200324d812 --- /dev/null +++ b/drivers/phy/phy-sama7-usb.c @@ -0,0 +1,90 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Support for Atmel/Microchip USB PHY's. + * + * Copyright (C) 2022 Microchip Technology Inc. and its subsidiaries + * + * Author: Sergiu Moga sergiu.moga@microchip.com + */ + +#include <clk.h> +#include <dm.h> +#include <generic-phy.h> +#include <syscon.h> +#include <regmap.h> +#include <mach/sama7-sfr.h> + +struct sama7_usb_phy { + struct clk *uclk; + struct regmap *sfr; + int port; +}; + +static int sama7_usb_phy_init(struct phy *phy) +{ + struct sama7_usb_phy *sama7_phy = dev_get_priv(phy->dev); + int port = sama7_phy->port; + + regmap_update_bits(sama7_phy->sfr, SAMA7_SFR_UTMI0R(port), + SAMA7_SFR_UTMI_RX_TX_PREEM_AMP_TUNE_1X, + SAMA7_SFR_UTMI_RX_TX_PREEM_AMP_TUNE_1X); + + regmap_update_bits(sama7_phy->sfr, SAMA7_SFR_UTMI0R(port), + SAMA7_SFR_UTMI_RX_VBUS, + SAMA7_SFR_UTMI_RX_VBUS); + + return 0; +} + +static int sama7_phy_power_on(struct phy *phy) +{ + struct sama7_usb_phy *sama7_phy = dev_get_priv(phy->dev); + + clk_prepare_enable(sama7_phy->uclk); + + return 0; +} + +static int sama7_phy_power_off(struct phy *phy) +{ + struct sama7_usb_phy *sama7_phy = dev_get_priv(phy->dev); + + clk_disable_unprepare(sama7_phy->uclk); + + return 0; +} + +static int sama7_usb_phy_probe(struct udevice *dev) +{ + struct sama7_usb_phy *sama7_phy = dev_get_priv(dev); + + sama7_phy->uclk = devm_clk_get(dev, "utmi_clk"); + if (IS_ERR(sama7_phy->uclk)) + return PTR_ERR(sama7_phy->uclk); + + sama7_phy->sfr = syscon_regmap_lookup_by_phandle(dev, "sfr-phandle"); + if (IS_ERR(sama7_phy->sfr)) + return PTR_ERR(sama7_phy->sfr); + + return dev_read_u32(dev, "reg", &sama7_phy->port); +} + +static const struct phy_ops sama7_usb_phy_ops = { + .init = sama7_usb_phy_init, + .power_on = sama7_phy_power_on, + .power_off = sama7_phy_power_off, +}; + +static const struct udevice_id sama7_usb_phy_of_match[] = { + { .compatible = "microchip,sama7g5-usb-phy", }, + { }, +}; + +U_BOOT_DRIVER(sama7_usb_phy_driver) = { + .name = "sama7-usb-phy", + .id = UCLASS_PHY, + .of_match = sama7_usb_phy_of_match, + .ops = &sama7_usb_phy_ops, + .probe = sama7_usb_phy_probe, + .priv_auto = sizeof(struct sama7_usb_phy), +}; diff --git a/drivers/phy/phy-sama7-utmi-clk.c b/drivers/phy/phy-sama7-utmi-clk.c new file mode 100644 index 0000000000..ab9fddccf6 --- /dev/null +++ b/drivers/phy/phy-sama7-utmi-clk.c @@ -0,0 +1,202 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Support for Atmel/Microchip USB PHY's. + * + * Copyright (C) 2022 Microchip Technology Inc. and its subsidiaries + * + * Author: Sergiu Moga sergiu.moga@microchip.com + */ + +#include <dm.h> +#include <linux/clk-provider.h> +#include <syscon.h> +#include <regmap.h> +#include <mach/sama7-sfr.h> +#include <reset.h> +#include <dt-bindings/clk/at91.h> + +struct sama7_utmi_clk { + struct clk uclk; + struct regmap *regmap_sfr; + struct reset_ctl *reset; + u8 id; +}; + +#define to_sama7_utmi_clk(_c) container_of(_c, struct sama7_utmi_clk, uclk) + +#define UBOOT_DM_CLK_MICROCHIP_SAMA7G5_UTMI "sama7-utmi-clk" +#define UBOOT_DM_MICROCHIP_SAMA7G5_UTMI "sama7-utmi" + +#define AT91_TO_CLK_ID(_t, _i) (((_t) << 8) | ((_i) & 0xff)) + +/* + * UTMI clock description + * @n: clock name + * @p: clock parent name + * @id: clock id in RSTC_GRSTR + */ +static struct { + const char *n; + const char *p; + u8 id; +} sama7_utmick[] = { + { .n = "utmi1", .p = "utmick", .id = 0, }, + { .n = "utmi2", .p = "utmi1", .id = 1, }, + { .n = "utmi3", .p = "utmi1", .id = 2, }, +}; + +static int sama7_utmi_clk_enable(struct clk *clk) +{ + int ret; + + struct sama7_utmi_clk *utmi = to_sama7_utmi_clk(clk); + u8 id = utmi->id; + + ret = reset_assert(utmi->reset); + if (ret) + return ret; + + ret = regmap_update_bits(utmi->regmap_sfr, SAMA7_SFR_UTMI0R(id), + SAMA7_SFR_UTMI_COMMONON, 0); + if (ret < 0) + return ret; + + ret = reset_deassert(utmi->reset); + if (ret) + return ret; + + /* Datasheet states a minimum of 45 us before any USB operation */ + udelay(50); + + return 0; +} + +static int sama7_utmi_clk_disable(struct clk *clk) +{ + int ret; + struct sama7_utmi_clk *utmi = to_sama7_utmi_clk(clk); + u8 id = utmi->id; + + ret = reset_assert(utmi->reset); + if (ret) + return ret; + + regmap_update_bits(utmi->regmap_sfr, SAMA7_SFR_UTMI0R(id), + SAMA7_SFR_UTMI_COMMONON, SAMA7_SFR_UTMI_COMMONON); + + return 0; +} + +static ulong sama7_utmi_clk_get_rate(struct clk *clk) +{ + /* Return utmick's rate: 480MHz */ + return clk_get_parent_rate(clk); +} + +static const struct clk_ops sama7_utmi_clk_ops = { + .enable = sama7_utmi_clk_enable, + .disable = sama7_utmi_clk_disable, + .get_rate = sama7_utmi_clk_get_rate, +}; + +static struct clk* +sama7_utmi_clk_register(struct regmap *regmap_sfr, struct reset_ctl *reset, + const char *name, const char *parent_name, u8 id) +{ + struct clk *clk; + struct sama7_utmi_clk *utmi_clk; + int ret; + + if (!regmap_sfr || !reset || !name || !parent_name) + return ERR_PTR(-EINVAL); + + utmi_clk = kzalloc(sizeof(*utmi_clk), GFP_KERNEL); + if (!utmi_clk) + return ERR_PTR(-ENOMEM); + + utmi_clk->reset = reset; + utmi_clk->regmap_sfr = regmap_sfr; + utmi_clk->id = id; + + clk = &utmi_clk->uclk; + ret = clk_register(clk, UBOOT_DM_CLK_MICROCHIP_SAMA7G5_UTMI, + name, parent_name); + if (ret) { + kfree(utmi_clk); + clk = ERR_PTR(ret); + } + + clk_dm(AT91_TO_CLK_ID(UTMI, utmi_clk->id), clk); + + return clk; +} + +static int sama7_utmi_probe(struct udevice *dev) +{ + struct clk *utmi_parent_clk, *utmi_clk; + struct regmap *regmap_sfr; + struct reset_ctl *phy_reset; + int i; + char name[16]; + + utmi_parent_clk = devm_clk_get(dev, "utmi_clk"); + if (IS_ERR(utmi_parent_clk)) + return PTR_ERR(utmi_parent_clk); + + regmap_sfr = syscon_regmap_lookup_by_phandle(dev, "sfr-phandle"); + if (IS_ERR(regmap_sfr)) + return PTR_ERR(regmap_sfr); + + for (i = 0; i < ARRAY_SIZE(sama7_utmick); i++) { + snprintf(name, sizeof(name), "usb%d_reset", i); + phy_reset = devm_reset_control_get(dev, name); + if (IS_ERR(phy_reset)) + return PTR_ERR(phy_reset); + + utmi_clk = sama7_utmi_clk_register(regmap_sfr, phy_reset, + sama7_utmick[i].n, + sama7_utmick[i].p, + sama7_utmick[i].id); + if (IS_ERR(utmi_clk)) + return PTR_ERR(utmi_clk); + } + + return 0; +}; + +static const struct udevice_id sama7_utmi_clk_dt_ids[] = { + { .compatible = "microchip,sama7g5-utmi-clk", }, + { /* sentinel */}, +}; + +static int utmi_clk_of_xlate(struct clk *clk, struct ofnode_phandle_args *args) +{ + if (args->args_count != 1) { + debug("UTMI: clk: Invalid args_count: %d\n", args->args_count); + return -EINVAL; + } + + clk->id = AT91_TO_CLK_ID(UTMI, args->args[0]); + + return 0; +} + +static const struct clk_ops sama7_utmi_ops = { + .of_xlate = utmi_clk_of_xlate, + .enable = ccf_clk_enable, + .disable = ccf_clk_disable, +}; + +U_BOOT_DRIVER(microhip_sama7g5_utmi_clk) = { + .name = UBOOT_DM_CLK_MICROCHIP_SAMA7G5_UTMI, + .id = UCLASS_CLK, + .ops = &sama7_utmi_clk_ops, +}; + +U_BOOT_DRIVER(microhip_sama7g5_utmi) = { + .name = UBOOT_DM_MICROCHIP_SAMA7G5_UTMI, + .of_match = sama7_utmi_clk_dt_ids, + .id = UCLASS_CLK, + .ops = &sama7_utmi_ops, + .probe = sama7_utmi_probe, +};
Register the OHCI driver into DM by properly initializing the required clocks and pins required by the DT node of OHCI. In order for the VBUS to stay enabled, a `child_pre_probe` method has been added to overcome the DM core disabling it in `usb_scan_device`: when the generic `device_probe` method is called, the pinctrl is processed once again, undoing whatever changes have been made in our driver's probe method.
Signed-off-by: Sergiu Moga sergiu.moga@microchip.com --- drivers/usb/host/ohci-at91.c | 183 +++++++++++++++++++++++++++++++++++ 1 file changed, 183 insertions(+)
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(); } + +#elif CONFIG_IS_ENABLED(DM_GPIO) + +#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]; + u8 ports; /* number of ports on root hub */ +}; + +struct ohci_at91_priv { + struct clk *iclk; + struct clk *fclk; + struct clk *hclk; + 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); + ohci_at91->clocked = true; +} + +static void at91_stop_clock(struct ohci_at91_priv *ohci_at91) +{ + if (!ohci_at91->clocked) + 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; + + 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) + 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; + + 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); + goto fail; + } + + at91_start_hc(dev); + + return ohci_register(dev, regs); + +fail: + at91_for_each_port(i) + if (dm_gpio_is_valid(&pdata->vbus_pin[i])) + gpio_free(pdata->vbus_pin[i].offset); + + return ret; +} + +static const struct udevice_id ohci_usb_ids[] = { + { .compatible = "atmel,at91rm9200-ohci", }, + { } +}; + +U_BOOT_DRIVER(ohci_atmel) = { + .name = "ohci_atmel", + .id = UCLASS_USB, + .of_match = ohci_usb_ids, + .probe = ohci_atmel_probe, + .remove = ohci_atmel_deregister, + .child_pre_probe = ohci_atmel_child_pre_probe, + .ops = &ohci_usb_ops, + .plat_auto = sizeof(struct at91_usbh_data), + .priv_auto = sizeof(struct ohci_at91_priv), + .flags = DM_FLAG_ALLOC_PRIV_DMA, +}; + +#endif /* CONFIG_IS_ENABLED(DM_USB) && CONFIG_IS_ENABLED(DM_GPIO) */
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 ?
+#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 ?
- 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.
[...]
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.
[...]
On 1/3/23 13:11, Sergiu.Moga@microchip.com wrote:
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.
Any chance they can be converted to DM ?
+#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.
Oh well, so yes, we're stuck with this GPIO stuff.
[...]
On 03.01.2023 16:34, Marek Vasut wrote:
On 1/3/23 13:11, Sergiu.Moga@microchip.com wrote:
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.
Any chance they can be converted to DM ?
Not at the moment, no.
+#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.
Oh well, so yes, we're stuck with this GPIO stuff.
[...]
Unfortunately yes...
On 1/3/23 16:49, Sergiu.Moga@microchip.com wrote:
On 03.01.2023 16:34, Marek Vasut wrote:
On 1/3/23 13:11, Sergiu.Moga@microchip.com wrote:
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.
Any chance they can be converted to DM ?
Not at the moment, no.
Why ?
On 03.01.2023 17:57, Marek Vasut wrote:
On 1/3/23 16:49, Sergiu.Moga@microchip.com wrote:
On 03.01.2023 16:34, Marek Vasut wrote:
On 1/3/23 13:11, Sergiu.Moga@microchip.com wrote:
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.
Any chance they can be converted to DM ?
Not at the moment, no.
Why ?
My apologies, but I do not have these boards at my disposal to test them and they are not the focus of this patch series. The reason I kept the non-DM part is that so the build on the other boards does not break. Perhaps in the future there will be plans to convert these as well but at the moment I can not say for sure.
On 1/3/23 17:01, Sergiu.Moga@microchip.com wrote:
On 03.01.2023 17:57, Marek Vasut wrote:
On 1/3/23 16:49, Sergiu.Moga@microchip.com wrote:
On 03.01.2023 16:34, Marek Vasut wrote:
On 1/3/23 13:11, Sergiu.Moga@microchip.com wrote:
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.
Any chance they can be converted to DM ?
Not at the moment, no.
Why ?
My apologies, but I do not have these boards at my disposal to test them and they are not the focus of this patch series. The reason I kept the non-DM part is that so the build on the other boards does not break. Perhaps in the future there will be plans to convert these as well but at the moment I can not say for sure.
No need to apologize. Can you add a comment into the commit message, explaining this ?
From: Cristian Birsan cristian.birsan@microchip.com
The `ohci_register` function expects that the OHCI driver's priv is a struct whose first field is of type `ohci_t`. The original conversion to DM did not have it and this inconsistency revealed itself whenever U-Boot required multiple memory allocations resulting in a memory overwrite of where this field would supposedly be.
Thus, add this missing field and automatically increase the implicit size of the driver's priv to avoid whatever future memory allocations may take place from overwriting it.
Fixes: de1cf0a9c6 ("drivers: usb: ohci-at91: Enable OHCI functionality and register into DM") Signed-off-by: Cristian Birsan cristian.birsan@microchip.com Signed-off-by: Sergiu Moga sergiu.moga@microchip.com Tested-by: Mihai Sain mihai.sain@microchip.com --- drivers/usb/host/ohci-at91.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c index 9ae55c6e5d..5cf8f283e5 100644 --- a/drivers/usb/host/ohci-at91.c +++ b/drivers/usb/host/ohci-at91.c @@ -86,6 +86,7 @@ struct at91_usbh_data { };
struct ohci_at91_priv { + ohci_t ohci; struct clk *iclk; struct clk *fclk; struct clk *hclk;
On 12/23/22 13:34, Sergiu Moga wrote:
From: Cristian Birsan cristian.birsan@microchip.com
The `ohci_register` function expects that the OHCI driver's priv is a struct whose first field is of type `ohci_t`. The original conversion to DM did not have it and this inconsistency revealed itself whenever U-Boot required multiple memory allocations resulting in a memory overwrite of where this field would supposedly be.
Thus, add this missing field and automatically increase the implicit size of the driver's priv to avoid whatever future memory allocations may take place from overwriting it.
Fixes: de1cf0a9c6 ("drivers: usb: ohci-at91: Enable OHCI functionality and register into DM") Signed-off-by: Cristian Birsan cristian.birsan@microchip.com Signed-off-by: Sergiu Moga sergiu.moga@microchip.com Tested-by: Mihai Sain mihai.sain@microchip.com
Either squash into 2/4 or -- if this should be applied on current release -- submit as separate patch.
Add the ability to enable/disable whatever USB PHY's are passed to the AT91 OHCI driver through DT.
Signed-off-by: Sergiu Moga sergiu.moga@microchip.com Tested-by: Mihai Sain mihai.sain@microchip.com --- drivers/usb/host/ohci-at91.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c index 5cf8f283e5..217f31b402 100644 --- a/drivers/usb/host/ohci-at91.c +++ b/drivers/usb/host/ohci-at91.c @@ -74,6 +74,10 @@ int usb_cpu_init_fail(void) #include <usb.h> #include "ohci.h"
+#if CONFIG_IS_ENABLED(PHY_MICROCHIP_SAMA7_USB) +#include <generic-phy.h> +#endif + #define AT91_MAX_USBH_PORTS 3
#define at91_for_each_port(index) \ @@ -91,6 +95,10 @@ struct ohci_at91_priv { struct clk *fclk; struct clk *hclk; bool clocked; + +#if CONFIG_IS_ENABLED(PHY_MICROCHIP_SAMA7_USB) + struct phy phy[AT91_MAX_USBH_PORTS]; +#endif };
static void at91_start_clock(struct ohci_at91_priv *ohci_at91) @@ -98,6 +106,13 @@ static void at91_start_clock(struct ohci_at91_priv *ohci_at91) if (ohci_at91->clocked) return;
+#if CONFIG_IS_ENABLED(PHY_MICROCHIP_SAMA7_USB) + int i; + + at91_for_each_port(i) + generic_phy_power_on(&ohci_at91->phy[i]); +#endif + clk_set_rate(ohci_at91->fclk, 48000000); clk_prepare_enable(ohci_at91->hclk); clk_prepare_enable(ohci_at91->iclk); @@ -110,6 +125,13 @@ static void at91_stop_clock(struct ohci_at91_priv *ohci_at91) if (!ohci_at91->clocked) return;
+#if CONFIG_IS_ENABLED(PHY_MICROCHIP_SAMA7_USB) + int i; + + at91_for_each_port(i) + generic_phy_power_off(&ohci_at91->phy[i]); +#endif + clk_disable_unprepare(ohci_at91->fclk); clk_disable_unprepare(ohci_at91->iclk); clk_disable_unprepare(ohci_at91->hclk); @@ -215,6 +237,14 @@ static int ohci_atmel_probe(struct udevice *dev) goto fail; }
+#if CONFIG_IS_ENABLED(PHY_MICROCHIP_SAMA7_USB) + at91_for_each_port(i) { + generic_phy_get_by_index(dev, i, &ohci_at91->phy[i]); + generic_phy_init(&ohci_at91->phy[i]); + generic_phy_configure(&ohci_at91->phy[i], NULL); + } +#endif + at91_start_hc(dev);
return ohci_register(dev, regs); @@ -229,6 +259,7 @@ fail:
static const struct udevice_id ohci_usb_ids[] = { { .compatible = "atmel,at91rm9200-ohci", }, + { .compatible = "microchip,sama7g5-ohci", }, { } };
On 12/23/22 13:34, Sergiu Moga wrote:
Add the ability to enable/disable whatever USB PHY's are passed to the AT91 OHCI driver through DT.
Signed-off-by: Sergiu Moga sergiu.moga@microchip.com Tested-by: Mihai Sain mihai.sain@microchip.com
drivers/usb/host/ohci-at91.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c index 5cf8f283e5..217f31b402 100644 --- a/drivers/usb/host/ohci-at91.c +++ b/drivers/usb/host/ohci-at91.c @@ -74,6 +74,10 @@ int usb_cpu_init_fail(void) #include <usb.h> #include "ohci.h"
+#if CONFIG_IS_ENABLED(PHY_MICROCHIP_SAMA7_USB) +#include <generic-phy.h> +#endif
#define AT91_MAX_USBH_PORTS 3
#define at91_for_each_port(index) \
@@ -91,6 +95,10 @@ struct ohci_at91_priv { struct clk *fclk; struct clk *hclk; bool clocked;
+#if CONFIG_IS_ENABLED(PHY_MICROCHIP_SAMA7_USB)
- struct phy phy[AT91_MAX_USBH_PORTS];
+#endif };
static void at91_start_clock(struct ohci_at91_priv *ohci_at91) @@ -98,6 +106,13 @@ static void at91_start_clock(struct ohci_at91_priv *ohci_at91) if (ohci_at91->clocked) return;
+#if CONFIG_IS_ENABLED(PHY_MICROCHIP_SAMA7_USB)
Use plain:
if (CONFIG_IS_ENABLED(...)) { ... }
instead of the #if ... , the compiler would optimize the code out correctly.
- int i;
- at91_for_each_port(i)
generic_phy_power_on(&ohci_at91->phy[i]);
Look at generic_phy_get_bulk() and generic_phy_power_on_bulk() and co. those should get rid of this loop altogether.
[...]
On 03.01.2023 01:33, Marek Vasut wrote:
On 12/23/22 13:34, Sergiu Moga wrote:
Add the ability to enable/disable whatever USB PHY's are passed to the AT91 OHCI driver through DT.
Signed-off-by: Sergiu Moga sergiu.moga@microchip.com Tested-by: Mihai Sain mihai.sain@microchip.com
drivers/usb/host/ohci-at91.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c index 5cf8f283e5..217f31b402 100644 --- a/drivers/usb/host/ohci-at91.c +++ b/drivers/usb/host/ohci-at91.c @@ -74,6 +74,10 @@ int usb_cpu_init_fail(void) #include <usb.h> #include "ohci.h"
+#if CONFIG_IS_ENABLED(PHY_MICROCHIP_SAMA7_USB) +#include <generic-phy.h> +#endif
#define AT91_MAX_USBH_PORTS 3
#define at91_for_each_port(index) \ @@ -91,6 +95,10 @@ struct ohci_at91_priv { struct clk *fclk; struct clk *hclk; bool clocked;
+#if CONFIG_IS_ENABLED(PHY_MICROCHIP_SAMA7_USB) + struct phy phy[AT91_MAX_USBH_PORTS]; +#endif };
static void at91_start_clock(struct ohci_at91_priv *ohci_at91) @@ -98,6 +106,13 @@ static void at91_start_clock(struct ohci_at91_priv *ohci_at91) if (ohci_at91->clocked) return;
+#if CONFIG_IS_ENABLED(PHY_MICROCHIP_SAMA7_USB)
Use plain:
if (CONFIG_IS_ENABLED(...)) { ... }
instead of the #if ... , the compiler would optimize the code out correctly.
The build system complains that the generic_phy_* methods are not present and, if possible, I would like not to have to enable the PHY related CONFIGs on the boards that do not need it only to avoid these warnings.
+ int i;
+ at91_for_each_port(i) + generic_phy_power_on(&ohci_at91->phy[i]);
Look at generic_phy_get_bulk() and generic_phy_power_on_bulk() and co. those should get rid of this loop altogether.
[...]
On 1/3/23 14:02, Sergiu.Moga@microchip.com wrote:
On 03.01.2023 01:33, Marek Vasut wrote:
On 12/23/22 13:34, Sergiu Moga wrote:
Add the ability to enable/disable whatever USB PHY's are passed to the AT91 OHCI driver through DT.
Signed-off-by: Sergiu Moga sergiu.moga@microchip.com Tested-by: Mihai Sain mihai.sain@microchip.com
drivers/usb/host/ohci-at91.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c index 5cf8f283e5..217f31b402 100644 --- a/drivers/usb/host/ohci-at91.c +++ b/drivers/usb/host/ohci-at91.c @@ -74,6 +74,10 @@ int usb_cpu_init_fail(void) #include <usb.h> #include "ohci.h"
+#if CONFIG_IS_ENABLED(PHY_MICROCHIP_SAMA7_USB) +#include <generic-phy.h> +#endif
#define AT91_MAX_USBH_PORTS 3
#define at91_for_each_port(index) \ @@ -91,6 +95,10 @@ struct ohci_at91_priv { struct clk *fclk; struct clk *hclk; bool clocked;
+#if CONFIG_IS_ENABLED(PHY_MICROCHIP_SAMA7_USB) + struct phy phy[AT91_MAX_USBH_PORTS]; +#endif };
static void at91_start_clock(struct ohci_at91_priv *ohci_at91) @@ -98,6 +106,13 @@ static void at91_start_clock(struct ohci_at91_priv *ohci_at91) if (ohci_at91->clocked) return;
+#if CONFIG_IS_ENABLED(PHY_MICROCHIP_SAMA7_USB)
Use plain:
if (CONFIG_IS_ENABLED(...)) { ... }
instead of the #if ... , the compiler would optimize the code out correctly.
The build system complains that the generic_phy_* methods are not present and, if possible, I would like not to have to enable the PHY related CONFIGs on the boards that do not need it only to avoid these warnings.
Do include <generic-phy.h> . If CONFIG_PHY is not set, then those generic_phy_*() functions become empty inline functions, where are optimized out by the compiler.
On 23.12.2022 14:34, Sergiu Moga wrote:
This patch series originates from a bigger patch series: https://lists.denx.de/pipermail/u-boot/2022-December/502865.html
Register ohci-at91 driver into Driver Model. In order for the VBUS to stay enabled, a `child_pre_probe` method has been added to overcome the DM core disabling it in `usb_scan_device`: when the generic `device_probe` method is called, the pinctrl is processed once again, undoing whatever changes have been made in our driver's probe method. In order to enable USB on SAMA7G5 the addition of the USB 2.0 PHY drivers were required.
Cristian Birsan (1): usb: ohci-at91: Add `ohci_t` field in `ohci_at91_priv`
Sergiu Moga (3): phy: at91: Add support for the USB 2.0 PHY's of SAMA7 usb: ohci-at91: Enable OHCI functionality and register into DM usb: ohci-at91: Add USB PHY functionality
drivers/phy/Kconfig | 10 ++ drivers/phy/Makefile | 1 + drivers/phy/phy-sama7-usb.c | 90 +++++++++++++ drivers/phy/phy-sama7-utmi-clk.c | 202 +++++++++++++++++++++++++++++ drivers/usb/host/ohci-at91.c | 215 +++++++++++++++++++++++++++++++ 5 files changed, 518 insertions(+) create mode 100644 drivers/phy/phy-sama7-usb.c create mode 100644 drivers/phy/phy-sama7-utmi-clk.c
By the way, this also depends on the following patch series: https://lists.denx.de/pipermail/u-boot/2022-December/502979.html
participants (3)
-
Marek Vasut -
Sergiu Moga -
Sergiu.Moga＠microchip.com