| Message ID | 20170713105939.31566-3-martin.blumenstingl@googlemail.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
On Thu, 2017-07-13 at 12:59 +0200, Martin Blumenstingl wrote: > Many SoC platforms have separate devices for the USB PHY which are > registered through the generic PHY framework. These PHYs have to be > enabled to make the USB controller actually work. They also have to be > disabled again on shutdown/suspend. > > Currently (at least) the following HCI platform drivers are using custom > code to obtain all PHYs via devicetree for the roothub/controller and > disable/enable them when required: > - ehci-platform.c has ehci_platform_power_{on,off} > - xhci-mtk.c has xhci_mtk_phy_{init,exit,power_on,power_off} > - ohci-platform.c has ohci_platform_power_{on,off} > > These drivers are not using the generic devicetree USB device bindings > yet which were only introduced recently (documentation is available in > devicetree/bindings/usb/usb-device.txt). > With this new driver the usb2-phy and usb3-phy can be specified directly > in the child-node of the corresponding port of the roothub via > devicetree. This can be extended by not just parsing PHYs (some of the > other drivers listed above are for example also parsing a list of clocks > as well) when required. > > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> > --- > drivers/usb/host/Kconfig | 3 + > drivers/usb/host/Makefile | 2 + > drivers/usb/host/platform-roothub.c | 146 ++++++++++++++++++++++++++++++++++++ > drivers/usb/host/platform-roothub.h | 14 ++++ > 4 files changed, 165 insertions(+) > create mode 100644 drivers/usb/host/platform-roothub.c > create mode 100644 drivers/usb/host/platform-roothub.h > > diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig > index fa5692dec832..b8b05c786b2a 100644 > --- a/drivers/usb/host/Kconfig > +++ b/drivers/usb/host/Kconfig > @@ -805,6 +805,9 @@ config USB_HCD_SSB > > If unsure, say N. > > +config USB_PLATFORM_ROOTHUB > + bool > + > config USB_HCD_TEST_MODE > bool "HCD test mode support" > ---help--- > diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile > index cf2691fffcc0..dc817f82d632 100644 > --- a/drivers/usb/host/Makefile > +++ b/drivers/usb/host/Makefile > @@ -29,6 +29,8 @@ obj-$(CONFIG_USB_WHCI_HCD) += whci/ > > obj-$(CONFIG_USB_PCI) += pci-quirks.o > > +obj-$(CONFIG_USB_PLATFORM_ROOTHUB) += platform-roothub.o > + > obj-$(CONFIG_USB_EHCI_HCD) += ehci-hcd.o > obj-$(CONFIG_USB_EHCI_PCI) += ehci-pci.o > obj-$(CONFIG_USB_EHCI_HCD_PLATFORM) += ehci-platform.o > diff --git a/drivers/usb/host/platform-roothub.c b/drivers/usb/host/platform-roothub.c > new file mode 100644 > index 000000000000..84837e42b006 > --- /dev/null > +++ b/drivers/usb/host/platform-roothub.c > @@ -0,0 +1,146 @@ > +/* > + * platform roothub driver - a virtual PHY device which passes all phy_* > + * function calls to multiple (actual) PHY devices. This is comes handy when > + * initializing all PHYs on a root-hub (to keep them all in the same state). > + * > + * Copyright (C) 2017 Martin Blumenstingl <martin.blumenstingl@googlemail.com> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see <http://www.gnu.org/licenses/>. > + */ > + > +#include <linux/device.h> > +#include <linux/list.h> > +#include <linux/phy/phy.h> > +#include <linux/of.h> > +#include <linux/usb/of.h> > + > +#include "platform-roothub.h" > + > +#define ROOTHUB_PORTNUM 0 > + > +struct platform_roothub { > + struct phy *phy; > + struct list_head list; > +}; > + > +static struct platform_roothub *platform_roothub_alloc(struct device *dev) > +{ > + struct platform_roothub *roothub_entry; > + > + roothub_entry = devm_kzalloc(dev, sizeof(*roothub_entry), GFP_KERNEL); > + if (!roothub_entry) > + return ERR_PTR(-ENOMEM); > + > + INIT_LIST_HEAD(&roothub_entry->list); > + > + return roothub_entry; > +} > + > +static int platform_roothub_add_phy(struct device *dev, > + struct device_node *port_np, > + const char *con_id, struct list_head *list) > +{ > + struct platform_roothub *roothub_entry; > + struct phy *phy = devm_of_phy_get(dev, port_np, con_id); > + > + if (IS_ERR_OR_NULL(phy)) { > + if (!phy || PTR_ERR(phy) == -ENODEV) > + return 0; > + else > + return PTR_ERR(phy); > + } > + > + roothub_entry = platform_roothub_alloc(dev); > + if (IS_ERR(roothub_entry)) > + return PTR_ERR(roothub_entry); > + > + roothub_entry->phy = phy; > + > + list_add_tail(&roothub_entry->list, list); > + > + return 0; > +} > + > +struct platform_roothub *platform_roothub_init(struct device *dev) > +{ > + struct device_node *roothub_np, *port_np; > + struct platform_roothub *plat_roothub; > + int err; > + > + roothub_np = usb_of_get_child_node(dev->of_node, ROOTHUB_PORTNUM); > + if (!of_device_is_available(roothub_np)) > + return NULL; > + > + plat_roothub = platform_roothub_alloc(dev); > + if (IS_ERR(plat_roothub)) > + return plat_roothub; > + > + for_each_available_child_of_node(roothub_np, port_np) { > + err = platform_roothub_add_phy(dev, port_np, "usb2-phy", > + &plat_roothub->list); > + if (err) > + return ERR_PTR(err); > + > + err = platform_roothub_add_phy(dev, port_np, "usb3-phy", > + &plat_roothub->list); > + if (err) > + return ERR_PTR(err); > + } > + > + return plat_roothub; > +} > +EXPORT_SYMBOL_GPL(platform_roothub_init); Can we process the case without phy-names property? as following one, phy-names may be redundant, because the phy type is informed in phys property. port@2 { reg = <2>; phys = <&u2port1 PHY_TYPE_USB2>, <&u3port1 PHY_TYPE_USB3>; phy-names = "usb2-phy", "usb3-phy"; }; > + > +int platform_roothub_power_on(struct platform_roothub *plat_roothub) > +{ > + struct platform_roothub *roothub_entry; > + struct list_head *head; > + int err; > + > + if (!plat_roothub) > + return 0; > + > + head = &plat_roothub->list; > + > + list_for_each_entry(roothub_entry, head, list) { > + err = phy_init(roothub_entry->phy); > + if (err) > + goto err_out; > + > + err = phy_power_on(roothub_entry->phy); > + if (err) { > + phy_exit(roothub_entry->phy); > + goto err_out; > + } > + } > + > + return 0; > + > +err_out: > + list_for_each_entry_continue_reverse(roothub_entry, head, list) { > + phy_power_off(roothub_entry->phy); > + phy_exit(roothub_entry->phy); > + } > + > + return err; > +} > +EXPORT_SYMBOL_GPL(platform_roothub_power_on); > + > +void platform_roothub_power_off(struct platform_roothub *plat_roothub) > +{ > + struct platform_roothub *roothub_entry; > + > + if (!plat_roothub) > + return; > + > + list_for_each_entry_reverse(roothub_entry, &plat_roothub->list, list) { > + phy_power_off(roothub_entry->phy); > + phy_exit(roothub_entry->phy); > + } > +} > +EXPORT_SYMBOL_GPL(platform_roothub_power_off); I test it, and works fine on mt8173. It will be flexible enough if some more APIs are provided, such as porwer_on/off all phys but not init/exit them. e.g. In order to keep link state on mt8173, we just power off all phys(not exit) when system enter suspend, then power on them again (needn't init, otherwise device will be disconnected) when system resume, this can avoid re-enumerating device. > diff --git a/drivers/usb/host/platform-roothub.h b/drivers/usb/host/platform-roothub.h > new file mode 100644 > index 000000000000..bde0bf299e3b > --- /dev/null > +++ b/drivers/usb/host/platform-roothub.h > @@ -0,0 +1,14 @@ > +#ifndef USB_HOST_PLATFORM_ROOTHUB_H > +#define USB_HOST_PLATFORM_ROOTHUB_H > + > +struct phy; > +struct device_node; These two declarations are not needed, when I remove them, and still compile pass. > + > +struct platform_roothub; > + > +struct platform_roothub *platform_roothub_init(struct device *dev); > + > +int platform_roothub_power_on(struct platform_roothub *plat_roothub); > +void platform_roothub_power_off(struct platform_roothub *plat_roothub); > + > +#endif /* USB_HOST_PLATFORM_ROOTHUB_H */
On Tue, Jul 25, 2017 at 8:40 AM, Chunfeng Yun <chunfeng.yun@mediatek.com> wrote: > On Thu, 2017-07-13 at 12:59 +0200, Martin Blumenstingl wrote: >> Many SoC platforms have separate devices for the USB PHY which are >> registered through the generic PHY framework. These PHYs have to be >> enabled to make the USB controller actually work. They also have to be >> disabled again on shutdown/suspend. >> >> Currently (at least) the following HCI platform drivers are using custom >> code to obtain all PHYs via devicetree for the roothub/controller and >> disable/enable them when required: >> - ehci-platform.c has ehci_platform_power_{on,off} >> - xhci-mtk.c has xhci_mtk_phy_{init,exit,power_on,power_off} >> - ohci-platform.c has ohci_platform_power_{on,off} >> >> These drivers are not using the generic devicetree USB device bindings >> yet which were only introduced recently (documentation is available in >> devicetree/bindings/usb/usb-device.txt). >> With this new driver the usb2-phy and usb3-phy can be specified directly >> in the child-node of the corresponding port of the roothub via >> devicetree. This can be extended by not just parsing PHYs (some of the >> other drivers listed above are for example also parsing a list of clocks >> as well) when required. >> >> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> >> --- >> drivers/usb/host/Kconfig | 3 + >> drivers/usb/host/Makefile | 2 + >> drivers/usb/host/platform-roothub.c | 146 ++++++++++++++++++++++++++++++++++++ >> drivers/usb/host/platform-roothub.h | 14 ++++ >> 4 files changed, 165 insertions(+) >> create mode 100644 drivers/usb/host/platform-roothub.c >> create mode 100644 drivers/usb/host/platform-roothub.h >> >> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig >> index fa5692dec832..b8b05c786b2a 100644 >> --- a/drivers/usb/host/Kconfig >> +++ b/drivers/usb/host/Kconfig >> @@ -805,6 +805,9 @@ config USB_HCD_SSB >> >> If unsure, say N. >> >> +config USB_PLATFORM_ROOTHUB >> + bool >> + >> config USB_HCD_TEST_MODE >> bool "HCD test mode support" >> ---help--- >> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile >> index cf2691fffcc0..dc817f82d632 100644 >> --- a/drivers/usb/host/Makefile >> +++ b/drivers/usb/host/Makefile >> @@ -29,6 +29,8 @@ obj-$(CONFIG_USB_WHCI_HCD) += whci/ >> >> obj-$(CONFIG_USB_PCI) += pci-quirks.o >> >> +obj-$(CONFIG_USB_PLATFORM_ROOTHUB) += platform-roothub.o >> + >> obj-$(CONFIG_USB_EHCI_HCD) += ehci-hcd.o >> obj-$(CONFIG_USB_EHCI_PCI) += ehci-pci.o >> obj-$(CONFIG_USB_EHCI_HCD_PLATFORM) += ehci-platform.o >> diff --git a/drivers/usb/host/platform-roothub.c b/drivers/usb/host/platform-roothub.c >> new file mode 100644 >> index 000000000000..84837e42b006 >> --- /dev/null >> +++ b/drivers/usb/host/platform-roothub.c >> @@ -0,0 +1,146 @@ >> +/* >> + * platform roothub driver - a virtual PHY device which passes all phy_* >> + * function calls to multiple (actual) PHY devices. This is comes handy when >> + * initializing all PHYs on a root-hub (to keep them all in the same state). >> + * >> + * Copyright (C) 2017 Martin Blumenstingl <martin.blumenstingl@googlemail.com> >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program. If not, see <http://www.gnu.org/licenses/>. >> + */ >> + >> +#include <linux/device.h> >> +#include <linux/list.h> >> +#include <linux/phy/phy.h> >> +#include <linux/of.h> >> +#include <linux/usb/of.h> >> + >> +#include "platform-roothub.h" >> + >> +#define ROOTHUB_PORTNUM 0 >> + >> +struct platform_roothub { >> + struct phy *phy; >> + struct list_head list; >> +}; >> + >> +static struct platform_roothub *platform_roothub_alloc(struct device *dev) >> +{ >> + struct platform_roothub *roothub_entry; >> + >> + roothub_entry = devm_kzalloc(dev, sizeof(*roothub_entry), GFP_KERNEL); >> + if (!roothub_entry) >> + return ERR_PTR(-ENOMEM); >> + >> + INIT_LIST_HEAD(&roothub_entry->list); >> + >> + return roothub_entry; >> +} >> + >> +static int platform_roothub_add_phy(struct device *dev, >> + struct device_node *port_np, >> + const char *con_id, struct list_head *list) >> +{ >> + struct platform_roothub *roothub_entry; >> + struct phy *phy = devm_of_phy_get(dev, port_np, con_id); >> + >> + if (IS_ERR_OR_NULL(phy)) { >> + if (!phy || PTR_ERR(phy) == -ENODEV) >> + return 0; >> + else >> + return PTR_ERR(phy); >> + } >> + >> + roothub_entry = platform_roothub_alloc(dev); >> + if (IS_ERR(roothub_entry)) >> + return PTR_ERR(roothub_entry); >> + >> + roothub_entry->phy = phy; >> + >> + list_add_tail(&roothub_entry->list, list); >> + >> + return 0; >> +} >> + >> +struct platform_roothub *platform_roothub_init(struct device *dev) >> +{ >> + struct device_node *roothub_np, *port_np; >> + struct platform_roothub *plat_roothub; >> + int err; >> + >> + roothub_np = usb_of_get_child_node(dev->of_node, ROOTHUB_PORTNUM); >> + if (!of_device_is_available(roothub_np)) >> + return NULL; >> + >> + plat_roothub = platform_roothub_alloc(dev); >> + if (IS_ERR(plat_roothub)) >> + return plat_roothub; >> + >> + for_each_available_child_of_node(roothub_np, port_np) { >> + err = platform_roothub_add_phy(dev, port_np, "usb2-phy", >> + &plat_roothub->list); >> + if (err) >> + return ERR_PTR(err); >> + >> + err = platform_roothub_add_phy(dev, port_np, "usb3-phy", >> + &plat_roothub->list); >> + if (err) >> + return ERR_PTR(err); >> + } >> + >> + return plat_roothub; >> +} >> +EXPORT_SYMBOL_GPL(platform_roothub_init); > Can we process the case without phy-names property? > as following one, phy-names may be redundant, because the phy type is > informed in phys property. > > port@2 { > reg = <2>; > phys = <&u2port1 PHY_TYPE_USB2>, <&u3port1 PHY_TYPE_USB3>; > phy-names = "usb2-phy", "usb3-phy"; > }; according to the PHY binding documentation the phy-names property is mandatory: [0] this would have to be discussed with the PHY framework maintainers > >> + >> +int platform_roothub_power_on(struct platform_roothub *plat_roothub) >> +{ >> + struct platform_roothub *roothub_entry; >> + struct list_head *head; >> + int err; >> + >> + if (!plat_roothub) >> + return 0; >> + >> + head = &plat_roothub->list; >> + >> + list_for_each_entry(roothub_entry, head, list) { >> + err = phy_init(roothub_entry->phy); >> + if (err) >> + goto err_out; >> + >> + err = phy_power_on(roothub_entry->phy); >> + if (err) { >> + phy_exit(roothub_entry->phy); >> + goto err_out; >> + } >> + } >> + >> + return 0; >> + >> +err_out: >> + list_for_each_entry_continue_reverse(roothub_entry, head, list) { >> + phy_power_off(roothub_entry->phy); >> + phy_exit(roothub_entry->phy); >> + } >> + >> + return err; >> +} >> +EXPORT_SYMBOL_GPL(platform_roothub_power_on); >> + >> +void platform_roothub_power_off(struct platform_roothub *plat_roothub) >> +{ >> + struct platform_roothub *roothub_entry; >> + >> + if (!plat_roothub) >> + return; >> + >> + list_for_each_entry_reverse(roothub_entry, &plat_roothub->list, list) { >> + phy_power_off(roothub_entry->phy); >> + phy_exit(roothub_entry->phy); >> + } >> +} >> +EXPORT_SYMBOL_GPL(platform_roothub_power_off); > I test it, and works fine on mt8173. many thanks! > It will be flexible enough if some more APIs are provided, such as > porwer_on/off all phys but not init/exit them. > e.g. In order to keep link state on mt8173, we just power off all > phys(not exit) when system enter suspend, then power on > them again (needn't init, otherwise device will be disconnected) when > system resume, this can avoid re-enumerating device. OK, that sounds reasonable - we don't have suspend support on the Amlogic platforms yet so I couldn't test this. I will add separate _init and _exit functions and use them in the appropriate places in the third patch. thanks for this suggestion! > >> diff --git a/drivers/usb/host/platform-roothub.h b/drivers/usb/host/platform-roothub.h >> new file mode 100644 >> index 000000000000..bde0bf299e3b >> --- /dev/null >> +++ b/drivers/usb/host/platform-roothub.h >> @@ -0,0 +1,14 @@ >> +#ifndef USB_HOST_PLATFORM_ROOTHUB_H >> +#define USB_HOST_PLATFORM_ROOTHUB_H >> + >> +struct phy; >> +struct device_node; > These two declarations are not needed, when I remove them, and still > compile pass. thanks for spotting - neither of these is used inside the header, so you are right: they should be removed >> + >> +struct platform_roothub; >> + >> +struct platform_roothub *platform_roothub_init(struct device *dev); >> + >> +int platform_roothub_power_on(struct platform_roothub *plat_roothub); >> +void platform_roothub_power_off(struct platform_roothub *plat_roothub); >> + >> +#endif /* USB_HOST_PLATFORM_ROOTHUB_H */ > > I will fix these issues and send an update next weekend. it would be great if the USB maintainers (especially Mathias Nyman, since you're the xhci-plat.c maintainer) could comment on this series Regards, Martin [0] http://elixir.free-electrons.com/linux/latest/source/Documentation/devicetree/bindings/phy/phy-bindings.txt#L33
Hi, On Tue, Jul 25, 2017 at 8:40 AM, Chunfeng Yun <chunfeng.yun@mediatek.com> wrote: > On Thu, 2017-07-13 at 12:59 +0200, Martin Blumenstingl wrote: >> Many SoC platforms have separate devices for the USB PHY which are >> registered through the generic PHY framework. These PHYs have to be >> enabled to make the USB controller actually work. They also have to be >> disabled again on shutdown/suspend. >> >> Currently (at least) the following HCI platform drivers are using custom >> code to obtain all PHYs via devicetree for the roothub/controller and >> disable/enable them when required: >> - ehci-platform.c has ehci_platform_power_{on,off} >> - xhci-mtk.c has xhci_mtk_phy_{init,exit,power_on,power_off} >> - ohci-platform.c has ohci_platform_power_{on,off} >> >> These drivers are not using the generic devicetree USB device bindings >> yet which were only introduced recently (documentation is available in >> devicetree/bindings/usb/usb-device.txt). >> With this new driver the usb2-phy and usb3-phy can be specified directly >> in the child-node of the corresponding port of the roothub via >> devicetree. This can be extended by not just parsing PHYs (some of the >> other drivers listed above are for example also parsing a list of clocks >> as well) when required. >> >> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> >> --- >> drivers/usb/host/Kconfig | 3 + >> drivers/usb/host/Makefile | 2 + >> drivers/usb/host/platform-roothub.c | 146 ++++++++++++++++++++++++++++++++++++ >> drivers/usb/host/platform-roothub.h | 14 ++++ >> 4 files changed, 165 insertions(+) >> create mode 100644 drivers/usb/host/platform-roothub.c >> create mode 100644 drivers/usb/host/platform-roothub.h >> >> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig >> index fa5692dec832..b8b05c786b2a 100644 >> --- a/drivers/usb/host/Kconfig >> +++ b/drivers/usb/host/Kconfig >> @@ -805,6 +805,9 @@ config USB_HCD_SSB >> >> If unsure, say N. >> >> +config USB_PLATFORM_ROOTHUB >> + bool >> + >> config USB_HCD_TEST_MODE >> bool "HCD test mode support" >> ---help--- >> diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile >> index cf2691fffcc0..dc817f82d632 100644 >> --- a/drivers/usb/host/Makefile >> +++ b/drivers/usb/host/Makefile >> @@ -29,6 +29,8 @@ obj-$(CONFIG_USB_WHCI_HCD) += whci/ >> >> obj-$(CONFIG_USB_PCI) += pci-quirks.o >> >> +obj-$(CONFIG_USB_PLATFORM_ROOTHUB) += platform-roothub.o >> + >> obj-$(CONFIG_USB_EHCI_HCD) += ehci-hcd.o >> obj-$(CONFIG_USB_EHCI_PCI) += ehci-pci.o >> obj-$(CONFIG_USB_EHCI_HCD_PLATFORM) += ehci-platform.o >> diff --git a/drivers/usb/host/platform-roothub.c b/drivers/usb/host/platform-roothub.c >> new file mode 100644 >> index 000000000000..84837e42b006 >> --- /dev/null >> +++ b/drivers/usb/host/platform-roothub.c >> @@ -0,0 +1,146 @@ >> +/* >> + * platform roothub driver - a virtual PHY device which passes all phy_* >> + * function calls to multiple (actual) PHY devices. This is comes handy when >> + * initializing all PHYs on a root-hub (to keep them all in the same state). >> + * >> + * Copyright (C) 2017 Martin Blumenstingl <martin.blumenstingl@googlemail.com> >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program. If not, see <http://www.gnu.org/licenses/>. >> + */ >> + >> +#include <linux/device.h> >> +#include <linux/list.h> >> +#include <linux/phy/phy.h> >> +#include <linux/of.h> >> +#include <linux/usb/of.h> >> + >> +#include "platform-roothub.h" >> + >> +#define ROOTHUB_PORTNUM 0 >> + >> +struct platform_roothub { >> + struct phy *phy; >> + struct list_head list; >> +}; >> + >> +static struct platform_roothub *platform_roothub_alloc(struct device *dev) >> +{ >> + struct platform_roothub *roothub_entry; >> + >> + roothub_entry = devm_kzalloc(dev, sizeof(*roothub_entry), GFP_KERNEL); >> + if (!roothub_entry) >> + return ERR_PTR(-ENOMEM); >> + >> + INIT_LIST_HEAD(&roothub_entry->list); >> + >> + return roothub_entry; >> +} >> + >> +static int platform_roothub_add_phy(struct device *dev, >> + struct device_node *port_np, >> + const char *con_id, struct list_head *list) >> +{ >> + struct platform_roothub *roothub_entry; >> + struct phy *phy = devm_of_phy_get(dev, port_np, con_id); >> + >> + if (IS_ERR_OR_NULL(phy)) { >> + if (!phy || PTR_ERR(phy) == -ENODEV) >> + return 0; >> + else >> + return PTR_ERR(phy); >> + } >> + >> + roothub_entry = platform_roothub_alloc(dev); >> + if (IS_ERR(roothub_entry)) >> + return PTR_ERR(roothub_entry); >> + >> + roothub_entry->phy = phy; >> + >> + list_add_tail(&roothub_entry->list, list); >> + >> + return 0; >> +} >> + >> +struct platform_roothub *platform_roothub_init(struct device *dev) >> +{ >> + struct device_node *roothub_np, *port_np; >> + struct platform_roothub *plat_roothub; >> + int err; >> + >> + roothub_np = usb_of_get_child_node(dev->of_node, ROOTHUB_PORTNUM); >> + if (!of_device_is_available(roothub_np)) >> + return NULL; >> + >> + plat_roothub = platform_roothub_alloc(dev); >> + if (IS_ERR(plat_roothub)) >> + return plat_roothub; >> + >> + for_each_available_child_of_node(roothub_np, port_np) { >> + err = platform_roothub_add_phy(dev, port_np, "usb2-phy", >> + &plat_roothub->list); >> + if (err) >> + return ERR_PTR(err); >> + >> + err = platform_roothub_add_phy(dev, port_np, "usb3-phy", >> + &plat_roothub->list); >> + if (err) >> + return ERR_PTR(err); >> + } >> + >> + return plat_roothub; >> +} >> +EXPORT_SYMBOL_GPL(platform_roothub_init); > Can we process the case without phy-names property? > as following one, phy-names may be redundant, because the phy type is > informed in phys property. > > port@2 { > reg = <2>; > phys = <&u2port1 PHY_TYPE_USB2>, <&u3port1 PHY_TYPE_USB3>; > phy-names = "usb2-phy", "usb3-phy"; > }; > > >> + >> +int platform_roothub_power_on(struct platform_roothub *plat_roothub) >> +{ >> + struct platform_roothub *roothub_entry; >> + struct list_head *head; >> + int err; >> + >> + if (!plat_roothub) >> + return 0; >> + >> + head = &plat_roothub->list; >> + >> + list_for_each_entry(roothub_entry, head, list) { >> + err = phy_init(roothub_entry->phy); >> + if (err) >> + goto err_out; >> + >> + err = phy_power_on(roothub_entry->phy); >> + if (err) { >> + phy_exit(roothub_entry->phy); >> + goto err_out; >> + } >> + } >> + >> + return 0; >> + >> +err_out: >> + list_for_each_entry_continue_reverse(roothub_entry, head, list) { >> + phy_power_off(roothub_entry->phy); >> + phy_exit(roothub_entry->phy); >> + } >> + >> + return err; >> +} >> +EXPORT_SYMBOL_GPL(platform_roothub_power_on); >> + >> +void platform_roothub_power_off(struct platform_roothub *plat_roothub) >> +{ >> + struct platform_roothub *roothub_entry; >> + >> + if (!plat_roothub) >> + return; >> + >> + list_for_each_entry_reverse(roothub_entry, &plat_roothub->list, list) { >> + phy_power_off(roothub_entry->phy); >> + phy_exit(roothub_entry->phy); >> + } >> +} >> +EXPORT_SYMBOL_GPL(platform_roothub_power_off); > I test it, and works fine on mt8173. > It will be flexible enough if some more APIs are provided, such as > porwer_on/off all phys but not init/exit them. > e.g. In order to keep link state on mt8173, we just power off all > phys(not exit) when system enter suspend, then power on > them again (needn't init, otherwise device will be disconnected) when > system resume, this can avoid re-enumerating device. RFCv3 contains a fix for this (which I can't properly test myself, since the Amlogic platform currently does not have suspend support). it would be great if you code give it another try (see [0]) - if it works for you then a Tested-by, Reviewed-By or Acked-By would be awesome! >> diff --git a/drivers/usb/host/platform-roothub.h b/drivers/usb/host/platform-roothub.h >> new file mode 100644 >> index 000000000000..bde0bf299e3b >> --- /dev/null >> +++ b/drivers/usb/host/platform-roothub.h >> @@ -0,0 +1,14 @@ >> +#ifndef USB_HOST_PLATFORM_ROOTHUB_H >> +#define USB_HOST_PLATFORM_ROOTHUB_H >> + >> +struct phy; >> +struct device_node; > These two declarations are not needed, when I remove them, and still > compile pass. > >> + >> +struct platform_roothub; >> + >> +struct platform_roothub *platform_roothub_init(struct device *dev); >> + >> +int platform_roothub_power_on(struct platform_roothub *plat_roothub); >> +void platform_roothub_power_off(struct platform_roothub *plat_roothub); >> + >> +#endif /* USB_HOST_PLATFORM_ROOTHUB_H */ > > Regards, Martin [0] http://lists.infradead.org/pipermail/linux-amlogic/2017-August/004596.html
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index fa5692dec832..b8b05c786b2a 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -805,6 +805,9 @@ config USB_HCD_SSB If unsure, say N. +config USB_PLATFORM_ROOTHUB + bool + config USB_HCD_TEST_MODE bool "HCD test mode support" ---help--- diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile index cf2691fffcc0..dc817f82d632 100644 --- a/drivers/usb/host/Makefile +++ b/drivers/usb/host/Makefile @@ -29,6 +29,8 @@ obj-$(CONFIG_USB_WHCI_HCD) += whci/ obj-$(CONFIG_USB_PCI) += pci-quirks.o +obj-$(CONFIG_USB_PLATFORM_ROOTHUB) += platform-roothub.o + obj-$(CONFIG_USB_EHCI_HCD) += ehci-hcd.o obj-$(CONFIG_USB_EHCI_PCI) += ehci-pci.o obj-$(CONFIG_USB_EHCI_HCD_PLATFORM) += ehci-platform.o diff --git a/drivers/usb/host/platform-roothub.c b/drivers/usb/host/platform-roothub.c new file mode 100644 index 000000000000..84837e42b006 --- /dev/null +++ b/drivers/usb/host/platform-roothub.c @@ -0,0 +1,146 @@ +/* + * platform roothub driver - a virtual PHY device which passes all phy_* + * function calls to multiple (actual) PHY devices. This is comes handy when + * initializing all PHYs on a root-hub (to keep them all in the same state). + * + * Copyright (C) 2017 Martin Blumenstingl <martin.blumenstingl@googlemail.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#include <linux/device.h> +#include <linux/list.h> +#include <linux/phy/phy.h> +#include <linux/of.h> +#include <linux/usb/of.h> + +#include "platform-roothub.h" + +#define ROOTHUB_PORTNUM 0 + +struct platform_roothub { + struct phy *phy; + struct list_head list; +}; + +static struct platform_roothub *platform_roothub_alloc(struct device *dev) +{ + struct platform_roothub *roothub_entry; + + roothub_entry = devm_kzalloc(dev, sizeof(*roothub_entry), GFP_KERNEL); + if (!roothub_entry) + return ERR_PTR(-ENOMEM); + + INIT_LIST_HEAD(&roothub_entry->list); + + return roothub_entry; +} + +static int platform_roothub_add_phy(struct device *dev, + struct device_node *port_np, + const char *con_id, struct list_head *list) +{ + struct platform_roothub *roothub_entry; + struct phy *phy = devm_of_phy_get(dev, port_np, con_id); + + if (IS_ERR_OR_NULL(phy)) { + if (!phy || PTR_ERR(phy) == -ENODEV) + return 0; + else + return PTR_ERR(phy); + } + + roothub_entry = platform_roothub_alloc(dev); + if (IS_ERR(roothub_entry)) + return PTR_ERR(roothub_entry); + + roothub_entry->phy = phy; + + list_add_tail(&roothub_entry->list, list); + + return 0; +} + +struct platform_roothub *platform_roothub_init(struct device *dev) +{ + struct device_node *roothub_np, *port_np; + struct platform_roothub *plat_roothub; + int err; + + roothub_np = usb_of_get_child_node(dev->of_node, ROOTHUB_PORTNUM); + if (!of_device_is_available(roothub_np)) + return NULL; + + plat_roothub = platform_roothub_alloc(dev); + if (IS_ERR(plat_roothub)) + return plat_roothub; + + for_each_available_child_of_node(roothub_np, port_np) { + err = platform_roothub_add_phy(dev, port_np, "usb2-phy", + &plat_roothub->list); + if (err) + return ERR_PTR(err); + + err = platform_roothub_add_phy(dev, port_np, "usb3-phy", + &plat_roothub->list); + if (err) + return ERR_PTR(err); + } + + return plat_roothub; +} +EXPORT_SYMBOL_GPL(platform_roothub_init); + +int platform_roothub_power_on(struct platform_roothub *plat_roothub) +{ + struct platform_roothub *roothub_entry; + struct list_head *head; + int err; + + if (!plat_roothub) + return 0; + + head = &plat_roothub->list; + + list_for_each_entry(roothub_entry, head, list) { + err = phy_init(roothub_entry->phy); + if (err) + goto err_out; + + err = phy_power_on(roothub_entry->phy); + if (err) { + phy_exit(roothub_entry->phy); + goto err_out; + } + } + + return 0; + +err_out: + list_for_each_entry_continue_reverse(roothub_entry, head, list) { + phy_power_off(roothub_entry->phy); + phy_exit(roothub_entry->phy); + } + + return err; +} +EXPORT_SYMBOL_GPL(platform_roothub_power_on); + +void platform_roothub_power_off(struct platform_roothub *plat_roothub) +{ + struct platform_roothub *roothub_entry; + + if (!plat_roothub) + return; + + list_for_each_entry_reverse(roothub_entry, &plat_roothub->list, list) { + phy_power_off(roothub_entry->phy); + phy_exit(roothub_entry->phy); + } +} +EXPORT_SYMBOL_GPL(platform_roothub_power_off); diff --git a/drivers/usb/host/platform-roothub.h b/drivers/usb/host/platform-roothub.h new file mode 100644 index 000000000000..bde0bf299e3b --- /dev/null +++ b/drivers/usb/host/platform-roothub.h @@ -0,0 +1,14 @@ +#ifndef USB_HOST_PLATFORM_ROOTHUB_H +#define USB_HOST_PLATFORM_ROOTHUB_H + +struct phy; +struct device_node; + +struct platform_roothub; + +struct platform_roothub *platform_roothub_init(struct device *dev); + +int platform_roothub_power_on(struct platform_roothub *plat_roothub); +void platform_roothub_power_off(struct platform_roothub *plat_roothub); + +#endif /* USB_HOST_PLATFORM_ROOTHUB_H */
Many SoC platforms have separate devices for the USB PHY which are registered through the generic PHY framework. These PHYs have to be enabled to make the USB controller actually work. They also have to be disabled again on shutdown/suspend. Currently (at least) the following HCI platform drivers are using custom code to obtain all PHYs via devicetree for the roothub/controller and disable/enable them when required: - ehci-platform.c has ehci_platform_power_{on,off} - xhci-mtk.c has xhci_mtk_phy_{init,exit,power_on,power_off} - ohci-platform.c has ohci_platform_power_{on,off} These drivers are not using the generic devicetree USB device bindings yet which were only introduced recently (documentation is available in devicetree/bindings/usb/usb-device.txt). With this new driver the usb2-phy and usb3-phy can be specified directly in the child-node of the corresponding port of the roothub via devicetree. This can be extended by not just parsing PHYs (some of the other drivers listed above are for example also parsing a list of clocks as well) when required. Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> --- drivers/usb/host/Kconfig | 3 + drivers/usb/host/Makefile | 2 + drivers/usb/host/platform-roothub.c | 146 ++++++++++++++++++++++++++++++++++++ drivers/usb/host/platform-roothub.h | 14 ++++ 4 files changed, 165 insertions(+) create mode 100644 drivers/usb/host/platform-roothub.c create mode 100644 drivers/usb/host/platform-roothub.h