From: nsekhar@ti•com (Sekhar Nori)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v2 04/11] ARM: davinci: da8xx: add usb phy clocks
Date: Wed, 23 Mar 2016 22:26:24 +0530 [thread overview]
Message-ID: <56F2CAB8.3090705@ti.com> (raw)
In-Reply-To: <1458181615-27782-5-git-send-email-david@lechnology.com>
On Thursday 17 March 2016 07:56 AM, David Lechner wrote:
> Up to this point, the USB phy clock configuration was handled manually in
> the board files and in the usb drivers. This adds proper clocks so that
> the usb drivers can use clk_get and clk_enable and not have to worry about
> the details. Also, the related code is removed from the board files.
>
> Signed-off-by: David Lechner <david@lechnology•com>
> ---
>
> v2 changes: Move clock mux code to set_parent callback. Also fixed some other
> issues from feedback on the previous patch.
>
>
> arch/arm/mach-davinci/board-da830-evm.c | 12 ---
> arch/arm/mach-davinci/board-omapl138-hawk.c | 7 --
> arch/arm/mach-davinci/da830.c | 143 ++++++++++++++++++++++++++++
> arch/arm/mach-davinci/da850.c | 143 ++++++++++++++++++++++++++++
> 4 files changed, 286 insertions(+), 19 deletions(-)
>
> diff --git a/arch/arm/mach-davinci/board-da830-evm.c b/arch/arm/mach-davinci/board-da830-evm.c
> index 3d8cf8c..f3a8cc9 100644
> --- a/arch/arm/mach-davinci/board-da830-evm.c
> +++ b/arch/arm/mach-davinci/board-da830-evm.c
> @@ -115,18 +115,6 @@ static __init void da830_evm_usb_init(void)
> */
> cfgchip2 = __raw_readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
>
> - /* USB2.0 PHY reference clock is 24 MHz */
> - cfgchip2 &= ~CFGCHIP2_REFFREQ;
> - cfgchip2 |= CFGCHIP2_REFFREQ_24MHZ;
> -
> - /*
> - * Select internal reference clock for USB 2.0 PHY
> - * and use it as a clock source for USB 1.1 PHY
> - * (this is the default setting anyway).
> - */
> - cfgchip2 &= ~CFGCHIP2_USB1PHYCLKMUX;
> - cfgchip2 |= CFGCHIP2_USB2PHYCLKMUX;
> -
> /*
> * We have to override VBUS/ID signals when MUSB is configured into the
> * host-only mode -- ID pin will float if no cable is connected, so the
> diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c b/arch/arm/mach-davinci/board-omapl138-hawk.c
> index ee62486..d27e753 100644
> --- a/arch/arm/mach-davinci/board-omapl138-hawk.c
> +++ b/arch/arm/mach-davinci/board-omapl138-hawk.c
> @@ -251,13 +251,6 @@ static __init void omapl138_hawk_usb_init(void)
> return;
> }
>
> - /* Setup the Ref. clock frequency for the HAWK at 24 MHz. */
> -
> - cfgchip2 = __raw_readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
> - cfgchip2 &= ~CFGCHIP2_REFFREQ;
> - cfgchip2 |= CFGCHIP2_REFFREQ_24MHZ;
> - __raw_writel(cfgchip2, DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
> -
> ret = gpio_request_one(DA850_USB1_VBUS_PIN,
> GPIOF_DIR_OUT, "USB1 VBUS");
> if (ret < 0) {
> diff --git a/arch/arm/mach-davinci/da830.c b/arch/arm/mach-davinci/da830.c
> index 7187e7f..ee942b0 100644
> --- a/arch/arm/mach-davinci/da830.c
> +++ b/arch/arm/mach-davinci/da830.c
> @@ -12,6 +12,7 @@
> #include <linux/init.h>
> #include <linux/clk.h>
> #include <linux/platform_data/gpio-davinci.h>
> +#include <linux/platform_data/usb-davinci.h>
>
> #include <asm/mach/map.h>
>
> @@ -346,6 +347,12 @@ static struct clk i2c1_clk = {
> .gpsc = 1,
> };
>
> +static struct clk usb_ref_clk = {
> + .name = "usb_ref_clk",
> + .rate = 48000000,
> + .set_rate = davinci_simple_set_rate,
> +};
can we call this usb_refclkin so it matches the TRM name? Also, should
this node be not be coming through individual board files as the rate
depends on what is connected to the usb_refclkin pin? Or is the
expectation that boards will call clk_set_rate() on this clock to the
correct value? If yes, I think it is misleading to populate the .rate here.
> +
> static struct clk usb11_clk = {
> .name = "usb11",
> .parent = &pll0_sysclk4,
> @@ -353,6 +360,139 @@ static struct clk usb11_clk = {
> .gpsc = 1,
> };
>
> +static void usb20_phy_clk_enable(struct clk *clk)
> +{
> + u32 val;
> +
> + val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
> +
> + /*
> + * Turn on the USB 2.0 PHY, but just the PLL, and not OTG. The USB 1.1
> + * host may use the PLL clock without USB 2.0 OTG being used.
> + */
> + val &= ~(CFGCHIP2_RESET | CFGCHIP2_PHYPWRDN);
> + val |= CFGCHIP2_PHY_PLLON;
> +
> + writel(val, DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
> +
> + pr_info("Waiting for USB 2.0 PHY clock good...\n");
> + while (!(readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG))
> + & CFGCHIP2_PHYCLKGD))
> + cpu_relax();
I guess this is copying some earlier code, but still, it will be nice to
see a timeout mechanism here, rather than loop endlessly.
> +}
> +
> +static void usb20_phy_clk_disable(struct clk *clk)
> +{
> + u32 val;
> +
> + val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
> + val |= CFGCHIP2_PHYPWRDN;
> + writel(val, DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
> +}
> +
> +static int usb20_phy_clk_set_parent(struct clk *clk, struct clk *parent)
> +{
> + u32 __iomem *cfgchip2;
> + u32 val;
> +
> + /*
> + * Can't use DA8XX_SYSCFG0_VIRT() here since this can be called before
> + * da8xx_syscfg0_base is initialized.
> + */
> + cfgchip2 = ioremap(DA8XX_SYSCFG0_BASE + DA8XX_CFGCHIP2_REG, 4);
Again, not sure if this is juts a theoretical possibility. If yes, I
would rather see you bail out if syscfg0_base is not initialized by the
time you get here than do an ioremap() again.
> + val = readl(cfgchip2);
> +
> + /* Set the mux depending on the parent clock. */
> + if (parent == &pll0_aux_clk)
> + val |= CFGCHIP2_USB2PHYCLKMUX;
> + else if (parent == &usb_ref_clk)
> + val &= ~CFGCHIP2_USB2PHYCLKMUX;
> + else {
> + pr_err("Bad parent on USB 2.0 PHY clock.\n");
> + return -EINVAL;
> + }
> +
> + /* reference frequency also comes from parent clock */
> + val &= ~CFGCHIP2_REFFREQ;
> + switch (clk_get_rate(parent)) {
> + case 12000000:
> + val |= CFGCHIP2_REFFREQ_12MHZ;
> + break;
> + case 13000000:
> + val |= CFGCHIP2_REFFREQ_13MHZ;
> + break;
> + case 19200000:
> + val |= CFGCHIP2_REFFREQ_19_2MHZ;
> + break;
> + case 20000000:
> + val |= CFGCHIP2_REFFREQ_20MHZ;
> + break;
> + case 24000000:
> + val |= CFGCHIP2_REFFREQ_24MHZ;
> + break;
> + case 26000000:
> + val |= CFGCHIP2_REFFREQ_26MHZ;
> + break;
> + case 38400000:
> + val |= CFGCHIP2_REFFREQ_38_4MHZ;
> + break;
> + case 40000000:
> + val |= CFGCHIP2_REFFREQ_40MHZ;
> + break;
> + case 48000000:
> + val |= CFGCHIP2_REFFREQ_48MHZ;
> + break;
> + default:
> + pr_err("Bad parent clock rate on USB 2.0 PHY clock.\n");
> + return -EINVAL;
> + }
> +
> + writel(val, cfgchip2);
> +
> + return 0;
> +}
> +
> +static struct clk usb20_phy_clk = {
> + .name = "usb20_phy",
> + .parent = &pll0_aux_clk,
> + .clk_enable = usb20_phy_clk_enable,
> + .clk_disable = usb20_phy_clk_disable,
> + .set_parent = usb20_phy_clk_set_parent,
> +};
I hope you have checked that all boards in mainline use the AUXCLK as
the reference USB 2.0 frequency?
> +
> +static int usb11_phy_clk_set_parent(struct clk *clk, struct clk *parent)
> +{
> + u32 __iomem *cfgchip2;
> + u32 val;
> +
> + /*
> + * Can't use DA8XX_SYSCFG0_VIRT() here since this can be called before
> + * da8xx_syscfg0_base is initialized.
> + */
> + cfgchip2 = ioremap(DA8XX_SYSCFG0_BASE + DA8XX_CFGCHIP2_REG, 4);
> + val = readl(cfgchip2);
> +
> + /* Set the USB 1.1 PHY clock mux based on the parent clock. */
> + if (parent == &usb20_phy_clk)
> + val &= ~CFGCHIP2_USB1PHYCLKMUX;
> + else if (parent == &usb_ref_clk)
> + val |= CFGCHIP2_USB1PHYCLKMUX;
> + else {
> + pr_err("Bad parent on USB 1.1 PHY clock.\n");
> + return -EINVAL;
> + }
> +
> + writel(val, cfgchip2);
> +
> + return 0;
> +}
> +
> +static struct clk usb11_phy_clk = {
> + .name = "usb11_phy",
> + .parent = &usb20_phy_clk,
> + .set_parent = usb11_phy_clk_set_parent,
> +};
Same thing here. I hope all current boards use USB2.0 clk as reference
for USB 1.1 phy
> +
> static struct clk emif3_clk = {
> .name = "emif3",
> .parent = &pll0_sysclk5,
> @@ -420,7 +560,10 @@ static struct clk_lookup da830_clks[] = {
> CLK("davinci_mdio.0", "fck", &emac_clk),
> CLK(NULL, "gpio", &gpio_clk),
> CLK("i2c_davinci.2", NULL, &i2c1_clk),
> + CLK(NULL, "usb_ref_clk", &usb_ref_clk),
> CLK(NULL, "usb11", &usb11_clk),
> + CLK(NULL, "usb20_phy", &usb20_phy_clk),
> + CLK(NULL, "usb11_phy", &usb11_phy_clk),
> CLK(NULL, "emif3", &emif3_clk),
> CLK(NULL, "arm", &arm_clk),
> CLK(NULL, "rmii", &rmii_clk),
> diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
> index 8c8f31e..8089a82 100644
> --- a/arch/arm/mach-davinci/da850.c
> +++ b/arch/arm/mach-davinci/da850.c
> @@ -19,6 +19,7 @@
> #include <linux/cpufreq.h>
> #include <linux/regulator/consumer.h>
> #include <linux/platform_data/gpio-davinci.h>
> +#include <linux/platform_data/usb-davinci.h>
>
> #include <asm/mach/map.h>
>
> @@ -360,6 +361,12 @@ static struct clk aemif_clk = {
> .flags = ALWAYS_ENABLED,
> };
>
> +static struct clk usb_ref_clk = {
> + .name = "usb_ref_clk",
> + .rate = 48000000,
> + .set_rate = davinci_simple_set_rate,
> +};
> +
> static struct clk usb11_clk = {
> .name = "usb11",
> .parent = &pll0_sysclk4,
> @@ -374,6 +381,139 @@ static struct clk usb20_clk = {
> .gpsc = 1,
> };
>
> +static void usb20_phy_clk_enable(struct clk *clk)
> +{
> + u32 val;
> +
> + val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
> +
> + /*
> + * Turn on the USB 2.0 PHY, but just the PLL, and not OTG. The USB 1.1
> + * host may use the PLL clock without USB 2.0 OTG being used.
> + */
> + val &= ~(CFGCHIP2_RESET | CFGCHIP2_PHYPWRDN);
> + val |= CFGCHIP2_PHY_PLLON;
> +
> + writel(val, DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
> +
> + pr_info("Waiting for USB 2.0 PHY clock good...\n");
> + while (!(readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG))
> + & CFGCHIP2_PHYCLKGD))
> + cpu_relax();
> +}
Looks like this is pretty much going to be the same code repeated for
DA850 and DA830. So can we move these to a common file like da8xx-usb.c?
You can even register these USB clocks from that file by using
clkdev_add() and clk_register(). This way they can remain to be file local.
Thanks,
Sekhar
next prev parent reply other threads:[~2016-03-23 16:56 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-17 2:26 [PATCH v2 00/11] da8xx USB clocks David Lechner
2016-03-17 2:26 ` [PATCH v2 01/11] ARM: davinci: defined missing CFGCHIP2_REFFREQ_* macros for MUSB PHY David Lechner
2016-03-17 2:26 ` [PATCH v2 02/11] ARM: davinci: add set_parent callback for mux clocks David Lechner
2016-03-17 2:26 ` [PATCH v2 03/11] ARM: davinci: da850: use clk->set_parent for async3 David Lechner
2016-03-23 15:56 ` Sekhar Nori
2016-03-23 17:20 ` David Lechner
2016-03-23 17:29 ` Sekhar Nori
2016-03-23 18:32 ` David Lechner
2016-03-24 13:44 ` Sekhar Nori
2016-03-17 2:26 ` [PATCH v2 04/11] ARM: davinci: da8xx: add usb phy clocks David Lechner
2016-03-17 12:12 ` Sergei Shtylyov
2016-03-23 16:56 ` Sekhar Nori [this message]
2016-03-23 17:45 ` David Lechner
2016-03-23 17:54 ` Sekhar Nori
2016-03-17 2:26 ` [PATCH v2 05/11] dt-bindings: Add bindings for phy-da8xx-usb David Lechner
2016-03-19 23:56 ` Rob Herring
2016-03-23 17:06 ` Sekhar Nori
2016-03-23 17:56 ` David Lechner
2016-03-17 2:26 ` [PATCH v2 06/11] phy: da8xx-usb: new driver for DA8XX SoC USB PHY David Lechner
2016-03-17 12:38 ` Sergei Shtylyov
2016-03-23 17:21 ` Sekhar Nori
2016-03-23 18:06 ` David Lechner
2016-03-24 14:01 ` David Laight
2016-04-01 13:16 ` Kishon Vijay Abraham I
2016-04-01 14:45 ` Bin Liu
2016-04-01 16:02 ` David Lechner
2016-04-01 16:19 ` Bin Liu
2016-04-01 19:49 ` Sergei Shtylyov
2016-04-01 19:45 ` Sergei Shtylyov
2016-04-01 19:56 ` Bin Liu
2016-04-13 20:51 ` David Lechner
2016-04-14 12:32 ` Kishon Vijay Abraham I
2016-03-17 2:26 ` [PATCH v2 07/11] ARM: davinci: da8xx: Add USB PHY platform declaration David Lechner
2016-03-17 2:26 ` [PATCH v2 08/11] ARM: dt: da850: Add usb phy node David Lechner
2016-03-17 2:26 ` [PATCH v2 09/11] usb: ohci-da8xx: Remove code that references mach David Lechner
2016-03-17 12:53 ` Sergei Shtylyov
2016-03-17 2:26 ` [PATCH v2 10/11] usb: musb: da8xx: Use devm in probe David Lechner
2016-03-17 11:07 ` Sergei Shtylyov
2016-03-17 2:26 ` [PATCH v2 11/11] usb: musb: da8xx: Remove mach code David Lechner
2016-03-17 13:11 ` Sergei Shtylyov
2016-03-17 17:38 ` David Lechner
2016-03-17 13:39 ` [PATCH v2 00/11] da8xx USB clocks Sergei Shtylyov
2016-03-23 17:26 ` Sekhar Nori
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=56F2CAB8.3090705@ti.com \
--to=nsekhar@ti$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox