From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kishon Vijay Abraham I Date: Wed, 20 Aug 2014 14:54:47 +0000 Subject: Re: [PATCH v5] phy: Renesas R-Car Gen2 PHY driver Message-Id: <53F4B3E7.3030700@ti.com> List-Id: References: <201407222327.15179.sergei.shtylyov@cogentembedded.com> In-Reply-To: <201407222327.15179.sergei.shtylyov@cogentembedded.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Sergei Shtylyov , robh+dt@kernel.org, pawel.moll@arm.com, mark.rutland@arm.com, ijc+devicetree@hellion.org.uk, galak@codeaurora.org, grant.likely@linaro.org, devicetree@vger.kernel.org Cc: rdunlap@infradead.org, linux-doc@vger.kernel.org, linux-sh@vger.kernel.org, linux-usb@vger.kernel.org Hi, On Wednesday 23 July 2014 12:57 AM, Sergei Shtylyov wrote: . . . . > Index: linux-phy/drivers/phy/phy-rcar-gen2.c > =================================> --- /dev/null > +++ linux-phy/drivers/phy/phy-rcar-gen2.c > @@ -0,0 +1,341 @@ > +/* > + * Renesas R-Car Gen2 PHY driver > + * > + * Copyright (C) 2014 Renesas Solutions Corp. > + * Copyright (C) 2014 Cogent Embedded, Inc. > + * > + * 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. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > + > +#define USBHS_LPSTS 0x02 > +#define USBHS_UGCTRL 0x80 > +#define USBHS_UGCTRL2 0x84 > +#define USBHS_UGSTS 0x88 /* The manuals have 0x90 */ > + > +/* Low Power Status register (LPSTS) */ > +#define USBHS_LPSTS_SUSPM 0x4000 > + > +/* USB General control register (UGCTRL) */ > +#define USBHS_UGCTRL_CONNECT 0x00000004 > +#define USBHS_UGCTRL_PLLRESET 0x00000001 > + > +/* USB General control register 2 (UGCTRL2) */ > +#define USBHS_UGCTRL2_USB2SEL 0x80000000 > +#define USBHS_UGCTRL2_USB2SEL_PCI 0x00000000 > +#define USBHS_UGCTRL2_USB2SEL_USB30 0x80000000 > +#define USBHS_UGCTRL2_USB0SEL 0x00000030 > +#define USBHS_UGCTRL2_USB0SEL_PCI 0x00000010 > +#define USBHS_UGCTRL2_USB0SEL_HS_USB 0x00000030 > + > +/* USB General status register (UGSTS) */ > +#define USBHS_UGSTS_LOCK 0x00000300 /* The manuals have 0x3 */ > + > +#define PHYS_PER_CHANNEL 2 > + > +struct rcar_gen2_phy { > + struct phy *phy; > + struct rcar_gen2_channel *channel; > + int number; > + u32 select_value; > +}; > + > +struct rcar_gen2_channel { > + struct device_node *of_node; > + struct rcar_gen2_phy_driver *drv; > + struct rcar_gen2_phy phys[PHYS_PER_CHANNEL]; > + int selected_phy; > + u32 select_mask; > +}; > + > +struct rcar_gen2_phy_driver { > + void __iomem *base; > + struct clk *clk; > + spinlock_t lock; > + int num_channels; > + struct rcar_gen2_channel *channels; > +}; > + > +static int rcar_gen2_phy_init(struct phy *p) > +{ > + struct rcar_gen2_phy *phy = phy_get_drvdata(p); > + struct rcar_gen2_channel *channel = phy->channel; > + struct rcar_gen2_phy_driver *drv = channel->drv; > + unsigned long flags; > + u32 ugctrl2; > + > + /* > + * Try to acquire exclusive access to PHY. The first driver calling > + * phy_init() on a given channel wins, and all attempts to use another > + * PHY on this channel will fail until phy_exit() is called by the first > + * driver. Achieving this with cmpxcgh() should be SMP-safe. > + */ > + if (cmpxchg(&channel->selected_phy, -1, phy->number) != -1) > + return -EBUSY; This should be done in phy_get no? Can we add this in phy-core? There might be other users who want to have exclusive access within the same phy provider. Rest of it looks fine to me. Thanks Kishon From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kishon Vijay Abraham I Subject: Re: [PATCH v5] phy: Renesas R-Car Gen2 PHY driver Date: Wed, 20 Aug 2014 20:12:47 +0530 Message-ID: <53F4B3E7.3030700@ti.com> References: <201407222327.15179.sergei.shtylyov@cogentembedded.com> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <201407222327.15179.sergei.shtylyov@cogentembedded.com> Sender: linux-doc-owner@vger.kernel.org To: Sergei Shtylyov , robh+dt@kernel.org, pawel.moll@arm.com, mark.rutland@arm.com, ijc+devicetree@hellion.org.uk, galak@codeaurora.org, grant.likely@linaro.org, devicetree@vger.kernel.org Cc: rdunlap@infradead.org, linux-doc@vger.kernel.org, linux-sh@vger.kernel.org, linux-usb@vger.kernel.org List-Id: devicetree@vger.kernel.org Hi, On Wednesday 23 July 2014 12:57 AM, Sergei Shtylyov wrote: . . . . > Index: linux-phy/drivers/phy/phy-rcar-gen2.c > =================================================================== > --- /dev/null > +++ linux-phy/drivers/phy/phy-rcar-gen2.c > @@ -0,0 +1,341 @@ > +/* > + * Renesas R-Car Gen2 PHY driver > + * > + * Copyright (C) 2014 Renesas Solutions Corp. > + * Copyright (C) 2014 Cogent Embedded, Inc. > + * > + * 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. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > + > +#define USBHS_LPSTS 0x02 > +#define USBHS_UGCTRL 0x80 > +#define USBHS_UGCTRL2 0x84 > +#define USBHS_UGSTS 0x88 /* The manuals have 0x90 */ > + > +/* Low Power Status register (LPSTS) */ > +#define USBHS_LPSTS_SUSPM 0x4000 > + > +/* USB General control register (UGCTRL) */ > +#define USBHS_UGCTRL_CONNECT 0x00000004 > +#define USBHS_UGCTRL_PLLRESET 0x00000001 > + > +/* USB General control register 2 (UGCTRL2) */ > +#define USBHS_UGCTRL2_USB2SEL 0x80000000 > +#define USBHS_UGCTRL2_USB2SEL_PCI 0x00000000 > +#define USBHS_UGCTRL2_USB2SEL_USB30 0x80000000 > +#define USBHS_UGCTRL2_USB0SEL 0x00000030 > +#define USBHS_UGCTRL2_USB0SEL_PCI 0x00000010 > +#define USBHS_UGCTRL2_USB0SEL_HS_USB 0x00000030 > + > +/* USB General status register (UGSTS) */ > +#define USBHS_UGSTS_LOCK 0x00000300 /* The manuals have 0x3 */ > + > +#define PHYS_PER_CHANNEL 2 > + > +struct rcar_gen2_phy { > + struct phy *phy; > + struct rcar_gen2_channel *channel; > + int number; > + u32 select_value; > +}; > + > +struct rcar_gen2_channel { > + struct device_node *of_node; > + struct rcar_gen2_phy_driver *drv; > + struct rcar_gen2_phy phys[PHYS_PER_CHANNEL]; > + int selected_phy; > + u32 select_mask; > +}; > + > +struct rcar_gen2_phy_driver { > + void __iomem *base; > + struct clk *clk; > + spinlock_t lock; > + int num_channels; > + struct rcar_gen2_channel *channels; > +}; > + > +static int rcar_gen2_phy_init(struct phy *p) > +{ > + struct rcar_gen2_phy *phy = phy_get_drvdata(p); > + struct rcar_gen2_channel *channel = phy->channel; > + struct rcar_gen2_phy_driver *drv = channel->drv; > + unsigned long flags; > + u32 ugctrl2; > + > + /* > + * Try to acquire exclusive access to PHY. The first driver calling > + * phy_init() on a given channel wins, and all attempts to use another > + * PHY on this channel will fail until phy_exit() is called by the first > + * driver. Achieving this with cmpxcgh() should be SMP-safe. > + */ > + if (cmpxchg(&channel->selected_phy, -1, phy->number) != -1) > + return -EBUSY; This should be done in phy_get no? Can we add this in phy-core? There might be other users who want to have exclusive access within the same phy provider. Rest of it looks fine to me. Thanks Kishon