From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ezequiel Garcia Subject: Re: [PATCH v2 14/18] ARM: mvebu: Add support for USB cluster on the Armada 375 SoC Date: Fri, 25 Apr 2014 12:43:20 -0300 Message-ID: <20140425154320.GA20801@arch.cereza> References: <1398434836-18908-1-git-send-email-gregory.clement@free-electrons.com> <1398434836-18908-15-git-send-email-gregory.clement@free-electrons.com> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: Content-Disposition: inline In-Reply-To: <1398434836-18908-15-git-send-email-gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> Sender: linux-usb-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Gregory CLEMENT Cc: Mathias Nyman , Greg Kroah-Hartman , Felipe Balbi , linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Jason Cooper , Andrew Lunn , Sebastian Hesselbarth , Thomas Petazzoni , linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, Lior Amsalem , Tawfik Bayouk , Nadav Haklai , Grant Likely , Rob Herring , devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: devicetree@vger.kernel.org On Apr 25, Gregory CLEMENT wrote: > The Armada 375 SoC comes with an USB2 host and device controller and > an USB3 controller. The USB cluster control register allows to manage > common features of both USB controllers. >=20 > Signed-off-by: Gregory CLEMENT > --- > arch/arm/mach-mvebu/Makefile | 2 +- > arch/arm/mach-mvebu/usb-cluster.c | 96 +++++++++++++++++++++++++++++= ++++++++++ > 2 files changed, 97 insertions(+), 1 deletion(-) > create mode 100644 arch/arm/mach-mvebu/usb-cluster.c >=20 > diff --git a/arch/arm/mach-mvebu/Makefile b/arch/arm/mach-mvebu/Makef= ile > index a63e43b6b451..dec05e7e1802 100644 > --- a/arch/arm/mach-mvebu/Makefile > +++ b/arch/arm/mach-mvebu/Makefile > @@ -4,7 +4,7 @@ ccflags-$(CONFIG_ARCH_MULTIPLATFORM) :=3D -I$(srctree= )/$(src)/include \ > AFLAGS_coherency_ll.o :=3D -Wa,-march=3Darmv7-a > =20 > obj-y +=3D system-controller.o mvebu-soc-id.o > -obj-$(CONFIG_MACH_MVEBU_V7) +=3D board-v7.o > +obj-$(CONFIG_MACH_MVEBU_V7) +=3D board-v7.o usb-cluster.o > obj-$(CONFIG_MACH_DOVE) +=3D dove.o > obj-$(CONFIG_ARCH_MVEBU) +=3D coherency.o coherency_ll.o pmsu.o > obj-$(CONFIG_SMP) +=3D platsmp.o headsmp.o > diff --git a/arch/arm/mach-mvebu/usb-cluster.c b/arch/arm/mach-mvebu/= usb-cluster.c > new file mode 100644 > index 000000000000..4c15d282db23 > --- /dev/null > +++ b/arch/arm/mach-mvebu/usb-cluster.c > @@ -0,0 +1,96 @@ > +/* > + * USB cluster support for Armada 375 platform. > + * > + * Copyright (C) 2014 Marvell > + * > + * Gregory CLEMENT > + * > + * This file is licensed under the terms of the GNU General Public > + * License version 2. This program is licensed "as is" without any > + * warranty of any kind, whether express or implied. > + * > + * Armada 375 comes with an USB2 host and device controller and an > + * USB3 controller. The USB cluster control register allows to manag= e > + * common features of both USB controller. > + */ > + > +#include > +#include > +#include > +#include > +#include > + > +#define USB2_PHY_CONFIG_ENABLE BIT(0) /* active low */ > + If it's active low, maybe you can simply call this USB2_PHY_DISABLE? > +static struct of_device_id of_usb_cluster_table[] =3D { > + { .compatible =3D "marvell,armada-375-usb-cluster", }, > + { /* end of list */ }, > +}; > + > +static int __init mvebu_usb_cluster_init(void) > +{ > + struct device_node *np; > + > + np =3D of_find_matching_node(NULL, of_usb_cluster_table); > + if (np) { I think that writing: if (!np) return 0; Will allow to write the rest with less indentation, which is usually cl= eaner. > + void __iomem *usb_cluster_base; > + u32 reg; > + struct device_node *ehci_node, *xhci_node; > + struct property *ehci_status; > + bool use_usb3 =3D false; > + > + usb_cluster_base =3D of_iomap(np, 0); > + BUG_ON(!usb_cluster_base); > + > + xhci_node =3D of_find_compatible_node(NULL, NULL, > + "marvell,armada-375-xhci"); > + > + if (xhci_node && of_device_is_available(xhci_node)) > + use_usb3 =3D true; > + > + ehci_node =3D of_find_compatible_node(NULL, NULL, > + "marvell,orion-ehci"); > + > + if (ehci_node && of_device_is_available(ehci_node) > + && use_usb3) { > + /* > + * We can't use usb2 and usb3 in the same time, so let's > + * disbale usb2 and complain about it to the user askinf typos: disable, asking > + * to fix the device tree. > + */ > + > + ehci_status =3D kzalloc(sizeof(struct property), > + GFP_KERNEL); > + WARN_ON(!ehci_status); > + > + ehci_status->value =3D kstrdup("disabled", GFP_KERNEL); Unless I'm missing something, warning about allocation error but then s= till using the pointer will cause very nasty problems. Maybe you can simply bail out? > + WARN_ON(!ehci_status->value); > + > + ehci_status->length =3D 8; > + ehci_status->name =3D kstrdup("status", GFP_KERNEL); > + WARN_ON(!ehci_status->name); > + > + of_update_property(ehci_node, ehci_status); > + pr_err("%s: armada-375-xhci and orion-ehci are incompatible for t= his SoC.\n", > + __func__); > + pr_err("Please fix your dts!\n"); > + pr_err("orion-ehci have been disabled by default...\n"); > + We've been using a WARN(1, FW_BUG "") to notify the user of a broken DT= =2E Arnd suggested to use WARN() to really catch the user's attention. > + } > + > + reg =3D readl(usb_cluster_base); > + if (use_usb3) > + reg |=3D USB2_PHY_CONFIG_ENABLE; > + else > + reg &=3D ~USB2_PHY_CONFIG_ENABLE; > + writel(reg, usb_cluster_base); > + > + of_node_put(ehci_node); > + of_node_put(xhci_node); > + of_node_put(np); > + iounmap(usb_cluster_base); > + } > + > + return 0; > +} > +postcore_initcall(mvebu_usb_cluster_init); > --=20 > 1.8.1.2 >=20 --=20 Ezequiel Garc=EDa, Free Electrons Embedded Linux, Kernel and Android Engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html