From patchwork Thu Aug 31 10:02:59 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ludovic Desroches X-Patchwork-Id: 9931725 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 31D75602F0 for ; Thu, 31 Aug 2017 10:04:28 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1751D2022C for ; Thu, 31 Aug 2017 10:04:28 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 0BC6E28174; Thu, 31 Aug 2017 10:04:28 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.6 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 0D9652022C for ; Thu, 31 Aug 2017 10:04:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender:Cc:List-Subscribe: List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To: Content-Type:MIME-Version:References:Message-ID:Subject:To:From:Date:Reply-To :Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=SGUCzSxJ7rObRxTPVGCb/fbFRN9y2xwqp2m1gg9nzoA=; b=Jjhk/HMCXlbYRkBALOLjs2Eyo ppyxxOSlcgo0N4PuoiBy4wTz8hXEds9ERDGiPyl4EI/mFEgCEBCEKnPs8jtDsFYbZ3y3fLfZdILWA eoTvbDhnA18arUy1/QDRiUjU8R9QQDBWWX3EkE9pbPZvzatY5EP0enFQr5SOYHQcFaidi9kJCtHwt fiUna2/vneCAymBx/MOXmphkaH/l+2hEqeCthHcaCRDbZ4kdaEz0Z9oXHO1IcAvnmJjhMlqlVRNpC 0pakDF7MSEWyC7gDHJCd+VqyYDPYHUdH9jRfEx6Ij/SkkoWMY6yReP6T4VFdkyYd2JZIN3qVGeVPH 1r0jBKo5w==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1dnMKl-0003i2-OT; Thu, 31 Aug 2017 10:04:23 +0000 Received: from esa5.microchip.iphmx.com ([216.71.150.166]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1dnMKh-0003gY-NL for linux-arm-kernel@lists.infradead.org; Thu, 31 Aug 2017 10:04:21 +0000 X-IronPort-AV: E=Sophos;i="5.41,451,1498546800"; d="scan'208";a="4311245" Received: from exsmtp02.microchip.com (HELO email.microchip.com) ([198.175.253.38]) by esa5.microchip.iphmx.com with ESMTP/TLS/DHE-RSA-AES256-SHA; 31 Aug 2017 03:03:58 -0700 Received: from localhost (10.10.76.4) by chn-sv-exch02.mchp-main.com (10.10.76.38) with Microsoft SMTP Server id 14.3.352.0; Thu, 31 Aug 2017 03:03:56 -0700 Date: Thu, 31 Aug 2017 12:02:59 +0200 From: Ludovic Desroches To: Ingo van Lil Subject: Re: [PATCH] clk: at91: utmi: Support configurable multipliers Message-ID: <20170831100259.qj4cisnu2il6mo5y@rfolt0960.corp.atmel.com> Mail-Followup-To: Ingo van Lil , linux-kernel@vger.kernel.org, Alexandre Belloni , Boris Brezillon , Nicolas Ferre , linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org References: <20170710122409.3017-1-inguin@gmx.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20170710122409.3017-1-inguin@gmx.de> User-Agent: NeoMutt/20170609 (1.8.3) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20170831_030419_826454_FA64BE03 X-CRM114-Status: GOOD ( 30.77 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Boris Brezillon , Nicolas Ferre , linux-kernel@vger.kernel.org, Alexandre Belloni , linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Ingo, I did a similar patch, and I was told that you sent something similar. I missed your patch since I have not subscribed to the linux-clk mailing list. If it's related to at91, you should send it to linux-arm mailing list too. On Mon, Jul 10, 2017 at 02:24:09PM +0200, Ingo van Lil wrote: > The AT91 UTMI clock is a PLL generating the 480 MHz USB clock. > Originally, this PLL always had a fixed x40 multiplier, thus requiring > a 12 MHz input clock. Recent SOCs (sama5d2 and sama5d3) have a special > function register for configuring this multiplier, allowing to use > different main clock frequencies. > > Signed-off-by: Ingo van Lil Unfortunately, replacing my patch by yours, it no longer works. More precisely, USB stuff is broken: utmi rate is weird. > --- > drivers/clk/at91/clk-utmi.c | 105 +++++++++++++++++++++++++++++++++++++++++-- > include/soc/at91/atmel-sfr.h | 2 + > 2 files changed, 104 insertions(+), 3 deletions(-) > > diff --git a/drivers/clk/at91/clk-utmi.c b/drivers/clk/at91/clk-utmi.c > index aadabd9..d41c38b 100644 > --- a/drivers/clk/at91/clk-utmi.c > +++ b/drivers/clk/at91/clk-utmi.c > @@ -14,14 +14,29 @@ > #include > #include > #include > +#include > > #include "pmc.h" > > +/* default multiplier for SOCs that do not allow configuration via SFR */ > #define UTMI_FIXED_MUL 40 > > +/* supported multiplier settings for SOCs that allow configuration via SFR */ > +struct utmi_multipliers { > + const char *sfr_compatible_name; > + u8 multipliers[4]; > +}; > + > +static const struct utmi_multipliers utmi_multipliers[] = { > + { "atmel,sama5d2-sfr", { 40, 30, 20, 40 } }, > + { "atmel,sama5d3-sfr", { 40, 30, 20, 10 } }, > +}; > + I will see if there is no mistake in the datasheet. On my side, I deal only with '40, 30, 20, 10' case, I am not sure we have to take care of '40, 30, 20, 40' even if it's not a mistake in the datasheet. > struct clk_utmi { > struct clk_hw hw; > struct regmap *regmap; > + struct regmap *sfr_regmap; > + const u8 *multipliers; > }; > > #define to_clk_utmi(hw) container_of(hw, struct clk_utmi, hw) > @@ -66,8 +81,67 @@ static void clk_utmi_unprepare(struct clk_hw *hw) > static unsigned long clk_utmi_recalc_rate(struct clk_hw *hw, > unsigned long parent_rate) > { > - /* UTMI clk is a fixed clk multiplier */ > - return parent_rate * UTMI_FIXED_MUL; > + struct clk_utmi *utmi = to_clk_utmi(hw); > + u8 mul = UTMI_FIXED_MUL; > + > + if (utmi->sfr_regmap && utmi->multipliers) { > + u32 regval; > + regmap_read(utmi->sfr_regmap, AT91_SFR_UTMICKTRIM, ®val); > + mul = utmi->multipliers[regval & AT91_UTMICKTRIM_FREQ_MASK]; > + } > + > + return parent_rate * mul; > +} > + > +static long clk_utmi_round_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long *parent_rate) > +{ > + struct clk_utmi *utmi = to_clk_utmi(hw); > + unsigned long bestrate = 0; > + int bestdiff = -1; > + int i; > + > + if (!utmi->sfr_regmap || !utmi->multipliers) > + return *parent_rate * UTMI_FIXED_MUL; > + > + for (i = 0; i < ARRAY_SIZE(utmi_multipliers); i++) { > + unsigned long tmprate = *parent_rate * utmi->multipliers[i]; > + int tmpdiff; > + > + if (tmprate < rate) > + continue; > + > + tmpdiff = tmprate - rate; > + if (bestdiff < 0 || bestdiff > tmpdiff) { > + bestrate = tmprate; > + bestdiff = tmpdiff; > + } > + > + if (!bestdiff) > + break; > + } > + > + return bestrate; > +} > + > +static int clk_utmi_set_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long parent_rate) > +{ > + struct clk_utmi *utmi = to_clk_utmi(hw); > + int i; > + > + if (!utmi->sfr_regmap || !utmi->multipliers) > + return rate == parent_rate * UTMI_FIXED_MUL ? 0 : -EINVAL; > + > + for (i = 0; i < ARRAY_SIZE(utmi_multipliers); i++) { > + if (rate == parent_rate * utmi->multipliers[i]) { > + regmap_update_bits(utmi->sfr_regmap, AT91_SFR_UTMICKTRIM, > + AT91_UTMICKTRIM_FREQ_MASK, i); > + return 0; > + } > + } > + > + return -EINVAL; > } What's the purpose of this addition? Who should set the rate of the utmi clock? I mean you have no choice, the rate is 480 MHz. > > static const struct clk_ops utmi_ops = { > @@ -75,10 +149,13 @@ static const struct clk_ops utmi_ops = { > .unprepare = clk_utmi_unprepare, > .is_prepared = clk_utmi_is_prepared, > .recalc_rate = clk_utmi_recalc_rate, > + .round_rate = clk_utmi_round_rate, > + .set_rate = clk_utmi_set_rate, > }; > > static struct clk_hw * __init > at91_clk_register_utmi(struct regmap *regmap, > + struct regmap *sfr_regmap, const u8 *multipliers, > const char *name, const char *parent_name) > { > struct clk_utmi *utmi; > @@ -98,6 +175,8 @@ at91_clk_register_utmi(struct regmap *regmap, > > utmi->hw.init = &init; > utmi->regmap = regmap; > + utmi->sfr_regmap = sfr_regmap; > + utmi->multipliers = multipliers; > > hw = &utmi->hw; > ret = clk_hw_register(NULL, &utmi->hw); > @@ -115,6 +194,9 @@ static void __init of_at91sam9x5_clk_utmi_setup(struct device_node *np) > const char *parent_name; > const char *name = np->name; > struct regmap *regmap; > + struct regmap *sfr_regmap; > + const u8 *multipliers = NULL; > + size_t i; > > parent_name = of_clk_get_parent_name(np, 0); > > @@ -124,7 +206,24 @@ static void __init of_at91sam9x5_clk_utmi_setup(struct device_node *np) > if (IS_ERR(regmap)) > return; > > - hw = at91_clk_register_utmi(regmap, name, parent_name); > + for (i = 0; i < ARRAY_SIZE(utmi_multipliers); i++) { > + sfr_regmap = syscon_regmap_lookup_by_compatible( > + utmi_multipliers[i].sfr_compatible_name); > + if (!IS_ERR(sfr_regmap)) { > + pr_debug("clk-utmi: found sfr node: %s\n", > + utmi_multipliers[i].sfr_compatible_name); > + multipliers = utmi_multipliers[i].multipliers; > + break; > + } > + } > + > + if (IS_ERR(sfr_regmap)) { > + pr_debug("clk-utmi: failed to find sfr node\n"); > + sfr_regmap = NULL; > + } > + > + hw = at91_clk_register_utmi(regmap, sfr_regmap, multipliers, > + name, parent_name); > if (IS_ERR(hw)) > return; > > diff --git a/include/soc/at91/atmel-sfr.h b/include/soc/at91/atmel-sfr.h > index 506ea8f..763948e 100644 > --- a/include/soc/at91/atmel-sfr.h > +++ b/include/soc/at91/atmel-sfr.h > @@ -17,6 +17,7 @@ > /* 0x08 ~ 0x0c: Reserved */ > #define AT91_SFR_OHCIICR 0x10 /* OHCI INT Configuration Register */ > #define AT91_SFR_OHCIISR 0x14 /* OHCI INT Status Register */ > +#define AT91_SFR_UTMICKTRIM 0x30 /* UTMI Clock Trimming Register */ > #define AT91_SFR_I2SCLKSEL 0x90 /* I2SC Register */ > > /* Field definitions */ > @@ -28,5 +29,6 @@ > AT91_OHCIICR_SUSPEND_B | \ > AT91_OHCIICR_SUSPEND_C) > > +#define AT91_UTMICKTRIM_FREQ_MASK 0x03 You can use GENMASK here. > > #endif /* _LINUX_MFD_SYSCON_ATMEL_SFR_H */ I have attached the patch I did. Regards Ludovic diff --git a/drivers/clk/at91/clk-utmi.c b/drivers/clk/at91/clk-utmi.c index aadabd9d1e2b..0b88147c4ec6 100644 --- a/drivers/clk/at91/clk-utmi.c +++ b/drivers/clk/at91/clk-utmi.c @@ -14,14 +14,15 @@ #include #include #include +#include #include "pmc.h" -#define UTMI_FIXED_MUL 40 - struct clk_utmi { struct clk_hw hw; - struct regmap *regmap; + unsigned int mul; + struct regmap *regmap_pmc; + struct regmap *regmap_sfr; }; #define to_clk_utmi(hw) container_of(hw, struct clk_utmi, hw) @@ -37,13 +38,54 @@ static inline bool clk_utmi_ready(struct regmap *regmap) static int clk_utmi_prepare(struct clk_hw *hw) { + struct clk_hw *hw_parent; struct clk_utmi *utmi = to_clk_utmi(hw); unsigned int uckr = AT91_PMC_UPLLEN | AT91_PMC_UPLLCOUNT | AT91_PMC_BIASEN; + unsigned int utmi_ref_clk_freq; + unsigned long parent_rate; + + /* + * If mainck rate is different from 12 MHz, we have to configure the + * FREQ field of the SFR_UTMICKTRIM register to generate properly + * the utmi clock. + */ + hw_parent = clk_hw_get_parent(hw); + parent_rate = clk_hw_get_rate(hw_parent); + + switch (parent_rate) { + case 12000000: + utmi_ref_clk_freq = 0; + utmi->mul = 40; + break; + case 16000000: + utmi_ref_clk_freq = 1; + utmi->mul = 30; + break; + case 24000000: + utmi_ref_clk_freq = 2; + utmi->mul = 20; + break; + case 48000000: + utmi_ref_clk_freq = 3; + utmi->mul = 10; + break; + default: + pr_err("Unsupported mainck rate to generate the utmi clock\n"); + return -EINVAL; + } + + if (utmi_ref_clk_freq & !utmi->regmap_sfr) { + pr_err("If mainck rate is different from 12 MHz, the sfr node is required to generate the utmi clock\n"); + return -EINVAL; + } - regmap_update_bits(utmi->regmap, AT91_CKGR_UCKR, uckr, uckr); + regmap_update_bits(utmi->regmap_sfr, AT91_SFR_UTMICKTRIM, + AT91_UTMICKTRIM_FREQ, utmi_ref_clk_freq); - while (!clk_utmi_ready(utmi->regmap)) + regmap_update_bits(utmi->regmap_pmc, AT91_CKGR_UCKR, uckr, uckr); + + while (!clk_utmi_ready(utmi->regmap_pmc)) cpu_relax(); return 0; @@ -53,21 +95,22 @@ static int clk_utmi_is_prepared(struct clk_hw *hw) { struct clk_utmi *utmi = to_clk_utmi(hw); - return clk_utmi_ready(utmi->regmap); + return clk_utmi_ready(utmi->regmap_pmc); } static void clk_utmi_unprepare(struct clk_hw *hw) { struct clk_utmi *utmi = to_clk_utmi(hw); - regmap_update_bits(utmi->regmap, AT91_CKGR_UCKR, AT91_PMC_UPLLEN, 0); + regmap_update_bits(utmi->regmap_pmc, AT91_CKGR_UCKR, AT91_PMC_UPLLEN, 0); } static unsigned long clk_utmi_recalc_rate(struct clk_hw *hw, unsigned long parent_rate) { - /* UTMI clk is a fixed clk multiplier */ - return parent_rate * UTMI_FIXED_MUL; + struct clk_utmi *utmi = to_clk_utmi(hw); + + return parent_rate * utmi->mul; } static const struct clk_ops utmi_ops = { @@ -78,7 +121,7 @@ static const struct clk_ops utmi_ops = { }; static struct clk_hw * __init -at91_clk_register_utmi(struct regmap *regmap, +at91_clk_register_utmi(struct regmap *regmap_pmc, struct regmap *regmap_sfr, const char *name, const char *parent_name) { struct clk_utmi *utmi; @@ -97,7 +140,8 @@ at91_clk_register_utmi(struct regmap *regmap, init.flags = CLK_SET_RATE_GATE; utmi->hw.init = &init; - utmi->regmap = regmap; + utmi->regmap_pmc = regmap_pmc; + utmi->regmap_sfr = regmap_sfr; hw = &utmi->hw; ret = clk_hw_register(NULL, &utmi->hw); @@ -114,17 +158,20 @@ static void __init of_at91sam9x5_clk_utmi_setup(struct device_node *np) struct clk_hw *hw; const char *parent_name; const char *name = np->name; - struct regmap *regmap; + struct regmap *regmap_pmc, *regmap_sfr; parent_name = of_clk_get_parent_name(np, 0); of_property_read_string(np, "clock-output-names", &name); - regmap = syscon_node_to_regmap(of_get_parent(np)); - if (IS_ERR(regmap)) + regmap_pmc = syscon_node_to_regmap(of_get_parent(np)); + if (IS_ERR(regmap_pmc)) return; - hw = at91_clk_register_utmi(regmap, name, parent_name); + /* SFR node missing is not necessarily an issue, we'll check it later. */ + regmap_sfr = syscon_regmap_lookup_by_compatible("atmel,sama5d2-sfr"); + + hw = at91_clk_register_utmi(regmap_pmc, regmap_sfr, name, parent_name); if (IS_ERR(hw)) return; diff --git a/include/soc/at91/atmel-sfr.h b/include/soc/at91/atmel-sfr.h index 506ea8ffda19..ad97856f8964 100644 --- a/include/soc/at91/atmel-sfr.h +++ b/include/soc/at91/atmel-sfr.h @@ -17,6 +17,7 @@ /* 0x08 ~ 0x0c: Reserved */ #define AT91_SFR_OHCIICR 0x10 /* OHCI INT Configuration Register */ #define AT91_SFR_OHCIISR 0x14 /* OHCI INT Status Register */ +#define AT91_SFR_UTMICKTRIM 0x30 #define AT91_SFR_I2SCLKSEL 0x90 /* I2SC Register */ /* Field definitions */ @@ -28,5 +29,6 @@ AT91_OHCIICR_SUSPEND_B | \ AT91_OHCIICR_SUSPEND_C) +#define AT91_UTMICKTRIM_FREQ GENMASK(1,0) #endif /* _LINUX_MFD_SYSCON_ATMEL_SFR_H */