From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D9532C433F5 for ; Fri, 15 Oct 2021 08:07:33 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A778F610D2 for ; Fri, 15 Oct 2021 08:07:33 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org A778F610D2 Authentication-Results: mail.kernel.org; dmarc=fail (p=quarantine dis=none) header.from=microchip.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:CC:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=O+9Itf4SAyVcEHAVMWC+mzbV1TvDDSexGE7elTcxEhQ=; b=ea0bwm3S84QXmGZ4ciedFY+DFA mzRsbHGLV2SIEF9rOIBnBZPrT9ep9PLRruG2oqg7B3peTSvOXg/e4IStbm3Ek+5qxljURiesKy13n i9ruDwFncY9NEVmNKWEo38NfeV3vm5XDR48eNZoQZoeCPPMWb5n2qA6QKg5t3+yYE59LDxO1FWkJ1 uQ1bqTzub4xGX2CLzF1yJUWawT4mWj0jIjA4nRVcgHG0a+AL/8KBGbtJNuSp5SJsUR/oI7YG7rQh6 0Vo7Y9DmDkjr4D5csKd1pd1zjnqzQ6139kDcy6+LU5HmK4samZMZYyttZdop3VTwFL86cExwGGuOj yX+liDag==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mbIDc-005r4z-Tk; Fri, 15 Oct 2021 08:05:35 +0000 Received: from esa.microchip.iphmx.com ([68.232.153.233]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mbI9u-005pZN-9g for linux-arm-kernel@lists.infradead.org; Fri, 15 Oct 2021 08:01:45 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=microchip.com; i=@microchip.com; q=dns/txt; s=mchp; t=1634284902; x=1665820902; h=subject:to:cc:references:from:message-id:date: mime-version:in-reply-to:content-transfer-encoding; bh=fzBZhDFv5vrSa4k4S66CHOdw2P6RhkePr5ADNa7Oh9U=; b=U9LWADQSjd3t/E4i9eczQwTWPgu6N1LxKQIhFhg4Eb1/FPaU+ImgUiyZ iB43kGGxW6YdZBVDwBbWPSvY/I1KuVt1ru+CDDe27kjuH4VDL9HAuKbeQ PqiwzhNaWVzo5lxQB5fxpo1hWmEElqA77U9pib+UAvQkDF76Dx2pBCCfG fa/bLw14oSlyUjouT8Ferz+8hOlScovYjbp+89WZAMag2QXPMvD4l/VsC LIABfz2/waYGW10SLJcIVuDJuz6i6KeXGlLH6DFi3JeBbTBo50hKRRNmo gxbya8HoHIz9ekkzcI3jkwiWgLBVq9X8ITY7pFyo6Jzv8hmFMwVzFLpSg A==; IronPort-SDR: cTbzFWpm0mncwZkfGAQtI6gnvJ3Ca1mBv9WK9sMO5hXL8h+1INScr84+n9ApJz7FcVwiSMubO5 hHoLATPsrUJO0Trc7u8Is49qUbpzSxgOLchFTviRcayt/hUWlAPTI1SddtosO6SPwbYnAep5PF n9eLIHbKTP7dtlj3QEiWA7y1BSD+JZMtOu7ByQvWVjlXa4cE946tYvJeBw+eIQKjUVwohSPTYx lQU4htk+oWeH7bhRf9YRKfm1XfXIioIJ6I2NBdhHodsgTjGkFMkg0RUErM46gZGJnwILQf0iTv r0m+PNbJldTy1JwGrHJlBV5E X-IronPort-AV: E=Sophos;i="5.85,375,1624345200"; d="scan'208";a="148208800" Received: from smtpout.microchip.com (HELO email.microchip.com) ([198.175.253.82]) by esa1.microchip.iphmx.com with ESMTP/TLS/AES256-SHA256; 15 Oct 2021 01:01:41 -0700 Received: from chn-vm-ex02.mchp-main.com (10.10.87.72) by chn-vm-ex02.mchp-main.com (10.10.87.72) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2176.14; Fri, 15 Oct 2021 01:01:40 -0700 Received: from [10.12.67.94] (10.10.115.15) by chn-vm-ex02.mchp-main.com (10.10.85.144) with Microsoft SMTP Server id 15.1.2176.14 via Frontend Transport; Fri, 15 Oct 2021 01:01:39 -0700 Subject: Re: [PATCH v5 11/15] clk: at91: clk-sam9x60-pll: add notifier for div part of PLL To: Claudiu Beznea , , , , CC: , , References: <20211011112719.3951784-1-claudiu.beznea@microchip.com> <20211011112719.3951784-12-claudiu.beznea@microchip.com> From: Nicolas Ferre Organization: microchip Message-ID: <339a4356-9d50-e939-2c8a-5399f7d7f528@microchip.com> Date: Fri, 15 Oct 2021 10:01:38 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <20211011112719.3951784-12-claudiu.beznea@microchip.com> Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211015_010142_435783_111E0689 X-CRM114-Status: GOOD ( 39.01 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 11/10/2021 at 13:27, Claudiu Beznea wrote: > SAM9X60's PLL which is also part of SAMA7G5 is composed of 2 parts: > one fractional part and one divider. On SAMA7G5 the CPU PLL could be > changed at run-time to implement DVFS. The hardware clock tree on > SAMA7G5 for CPU PLL is as follows: > > +---- div1 ----------------> cpuck > | > FRAC PLL ---> DIV PLL -+-> prescaler ---> div0 ---> mck0 > > The div1 block is not implemented in Linux; on prescaler block it has > been discovered a bug on some scenarios and will be removed from Linux > in next commits. Thus, the final clock tree that will be used in Linux > will be as follows: > > +-----------> cpuck > | > FRAC PLL ---> DIV PLL -+-> div0 ---> mck0 > > It has been proposed in [1] to not introduce a new CPUFreq driver but > to overload the proper clock drivers with proper operation such that > cpufreq-dt to be used. To accomplish this DIV PLL and div0 implement > clock notifiers which applies safe dividers before FRAC PLL is changed. > The current commit treats only the DIV PLL by adding a notifier that > sets a safe divider on PRE_RATE_CHANGE events. The safe divider is > provided by initialization clock code (sama7g5.c). The div0 is treated > in next commits (to keep the changes as clean as possible). > > [1] https://lore.kernel.org/lkml/20210105104426.4tmgc2l3vyicwedd@vireshk-i7/ > > Signed-off-by: Claudiu Beznea Yes, it's needed: Acked-by: Nicolas Ferre > --- > drivers/clk/at91/clk-sam9x60-pll.c | 102 ++++++++++++++++++++++------- > drivers/clk/at91/pmc.h | 3 +- > drivers/clk/at91/sam9x60.c | 6 +- > drivers/clk/at91/sama7g5.c | 13 +++- > 4 files changed, 95 insertions(+), 29 deletions(-) > > diff --git a/drivers/clk/at91/clk-sam9x60-pll.c b/drivers/clk/at91/clk-sam9x60-pll.c > index a73d7c96ce1d..d757003004cb 100644 > --- a/drivers/clk/at91/clk-sam9x60-pll.c > +++ b/drivers/clk/at91/clk-sam9x60-pll.c > @@ -5,6 +5,7 @@ > */ > > #include > +#include > #include > #include > #include > @@ -47,12 +48,15 @@ struct sam9x60_div { > struct sam9x60_pll_core core; > struct at91_clk_pms pms; > u8 div; > + u8 safe_div; > }; > > #define to_sam9x60_pll_core(hw) container_of(hw, struct sam9x60_pll_core, hw) > #define to_sam9x60_frac(core) container_of(core, struct sam9x60_frac, core) > #define to_sam9x60_div(core) container_of(core, struct sam9x60_div, core) > > +static struct sam9x60_div *notifier_div; > + > static inline bool sam9x60_pll_ready(struct regmap *regmap, int id) > { > unsigned int status; > @@ -329,6 +333,26 @@ static const struct clk_ops sam9x60_frac_pll_ops_chg = { > .restore_context = sam9x60_frac_pll_restore_context, > }; > > +/* This function should be called with spinlock acquired. */ > +static void sam9x60_div_pll_set_div(struct sam9x60_pll_core *core, u32 div, > + bool enable) > +{ > + struct regmap *regmap = core->regmap; > + u32 ena_msk = enable ? core->layout->endiv_mask : 0; > + u32 ena_val = enable ? (1 << core->layout->endiv_shift) : 0; > + > + regmap_update_bits(regmap, AT91_PMC_PLL_CTRL0, > + core->layout->div_mask | ena_msk, > + (div << core->layout->div_shift) | ena_val); > + > + regmap_update_bits(regmap, AT91_PMC_PLL_UPDT, > + AT91_PMC_PLL_UPDT_UPDATE | AT91_PMC_PLL_UPDT_ID_MSK, > + AT91_PMC_PLL_UPDT_UPDATE | core->id); > + > + while (!sam9x60_pll_ready(regmap, core->id)) > + cpu_relax(); > +} > + > static int sam9x60_div_pll_set(struct sam9x60_pll_core *core) > { > struct sam9x60_div *div = to_sam9x60_div(core); > @@ -346,17 +370,7 @@ static int sam9x60_div_pll_set(struct sam9x60_pll_core *core) > if (!!(val & core->layout->endiv_mask) && cdiv == div->div) > goto unlock; > > - regmap_update_bits(regmap, AT91_PMC_PLL_CTRL0, > - core->layout->div_mask | core->layout->endiv_mask, > - (div->div << core->layout->div_shift) | > - (1 << core->layout->endiv_shift)); > - > - regmap_update_bits(regmap, AT91_PMC_PLL_UPDT, > - AT91_PMC_PLL_UPDT_UPDATE | AT91_PMC_PLL_UPDT_ID_MSK, > - AT91_PMC_PLL_UPDT_UPDATE | core->id); > - > - while (!sam9x60_pll_ready(regmap, core->id)) > - cpu_relax(); > + sam9x60_div_pll_set_div(core, div->div, 1); > > unlock: > spin_unlock_irqrestore(core->lock, flags); > @@ -502,16 +516,7 @@ static int sam9x60_div_pll_set_rate_chg(struct clk_hw *hw, unsigned long rate, > if (cdiv == div->div) > goto unlock; > > - regmap_update_bits(regmap, AT91_PMC_PLL_CTRL0, > - core->layout->div_mask, > - (div->div << core->layout->div_shift)); > - > - regmap_update_bits(regmap, AT91_PMC_PLL_UPDT, > - AT91_PMC_PLL_UPDT_UPDATE | AT91_PMC_PLL_UPDT_ID_MSK, > - AT91_PMC_PLL_UPDT_UPDATE | core->id); > - > - while (!sam9x60_pll_ready(regmap, core->id)) > - cpu_relax(); > + sam9x60_div_pll_set_div(core, div->div, 0); > > unlock: > spin_unlock_irqrestore(core->lock, irqflags); > @@ -538,6 +543,48 @@ static void sam9x60_div_pll_restore_context(struct clk_hw *hw) > sam9x60_div_pll_set(core); > } > > +static int sam9x60_div_pll_notifier_fn(struct notifier_block *notifier, > + unsigned long code, void *data) > +{ > + struct sam9x60_div *div = notifier_div; > + struct sam9x60_pll_core core = div->core; > + struct regmap *regmap = core.regmap; > + unsigned long irqflags; > + u32 val, cdiv; > + int ret = NOTIFY_DONE; > + > + if (code != PRE_RATE_CHANGE) > + return ret; > + > + /* > + * We switch to safe divider to avoid overclocking of other domains > + * feed by us while the frac PLL (our parent) is changed. > + */ > + div->div = div->safe_div; > + > + spin_lock_irqsave(core.lock, irqflags); > + regmap_update_bits(regmap, AT91_PMC_PLL_UPDT, AT91_PMC_PLL_UPDT_ID_MSK, > + core.id); > + regmap_read(regmap, AT91_PMC_PLL_CTRL0, &val); > + cdiv = (val & core.layout->div_mask) >> core.layout->div_shift; > + > + /* Stop if nothing changed. */ > + if (cdiv == div->safe_div) > + goto unlock; > + > + sam9x60_div_pll_set_div(&core, div->div, 0); > + ret = NOTIFY_OK; > + > +unlock: > + spin_unlock_irqrestore(core.lock, irqflags); > + > + return ret; > +} > + > +static struct notifier_block sam9x60_div_pll_notifier = { > + .notifier_call = sam9x60_div_pll_notifier_fn, > +}; > + > static const struct clk_ops sam9x60_div_pll_ops = { > .prepare = sam9x60_div_pll_prepare, > .unprepare = sam9x60_div_pll_unprepare, > @@ -647,7 +694,8 @@ struct clk_hw * __init > sam9x60_clk_register_div_pll(struct regmap *regmap, spinlock_t *lock, > const char *name, const char *parent_name, u8 id, > const struct clk_pll_characteristics *characteristics, > - const struct clk_pll_layout *layout, u32 flags) > + const struct clk_pll_layout *layout, u32 flags, > + u32 safe_div) > { > struct sam9x60_div *div; > struct clk_hw *hw; > @@ -656,9 +704,13 @@ sam9x60_clk_register_div_pll(struct regmap *regmap, spinlock_t *lock, > unsigned int val; > int ret; > > - if (id > PLL_MAX_ID || !lock) > + /* We only support one changeable PLL. */ > + if (id > PLL_MAX_ID || !lock || (safe_div && notifier_div)) > return ERR_PTR(-EINVAL); > > + if (safe_div >= PLL_DIV_MAX) > + safe_div = PLL_DIV_MAX - 1; > + > div = kzalloc(sizeof(*div), GFP_KERNEL); > if (!div) > return ERR_PTR(-ENOMEM); > @@ -678,6 +730,7 @@ sam9x60_clk_register_div_pll(struct regmap *regmap, spinlock_t *lock, > div->core.layout = layout; > div->core.regmap = regmap; > div->core.lock = lock; > + div->safe_div = safe_div; > > spin_lock_irqsave(div->core.lock, irqflags); > > @@ -693,6 +746,9 @@ sam9x60_clk_register_div_pll(struct regmap *regmap, spinlock_t *lock, > if (ret) { > kfree(div); > hw = ERR_PTR(ret); > + } else if (div->safe_div) { > + notifier_div = div; > + clk_notifier_register(hw->clk, &sam9x60_div_pll_notifier); > } > > return hw; > diff --git a/drivers/clk/at91/pmc.h b/drivers/clk/at91/pmc.h > index 45df094498ce..207ecccef29f 100644 > --- a/drivers/clk/at91/pmc.h > +++ b/drivers/clk/at91/pmc.h > @@ -214,7 +214,8 @@ struct clk_hw * __init > sam9x60_clk_register_div_pll(struct regmap *regmap, spinlock_t *lock, > const char *name, const char *parent_name, u8 id, > const struct clk_pll_characteristics *characteristics, > - const struct clk_pll_layout *layout, u32 flags); > + const struct clk_pll_layout *layout, u32 flags, > + u32 safe_div); > > struct clk_hw * __init > sam9x60_clk_register_frac_pll(struct regmap *regmap, spinlock_t *lock, > diff --git a/drivers/clk/at91/sam9x60.c b/drivers/clk/at91/sam9x60.c > index 5f6fa89571b7..5c264185f261 100644 > --- a/drivers/clk/at91/sam9x60.c > +++ b/drivers/clk/at91/sam9x60.c > @@ -242,7 +242,7 @@ static void __init sam9x60_pmc_setup(struct device_node *np) > * This feeds CPU. It should not > * be disabled. > */ > - CLK_IS_CRITICAL | CLK_SET_RATE_GATE); > + CLK_IS_CRITICAL | CLK_SET_RATE_GATE, 0); > if (IS_ERR(hw)) > goto err_free; > > @@ -260,7 +260,7 @@ static void __init sam9x60_pmc_setup(struct device_node *np) > &pll_div_layout, > CLK_SET_RATE_GATE | > CLK_SET_PARENT_GATE | > - CLK_SET_RATE_PARENT); > + CLK_SET_RATE_PARENT, 0); > if (IS_ERR(hw)) > goto err_free; > > @@ -279,7 +279,7 @@ static void __init sam9x60_pmc_setup(struct device_node *np) > hw = at91_clk_register_master_div(regmap, "masterck_div", > "masterck_pres", &sam9x60_master_layout, > &mck_characteristics, &mck_lock, > - CLK_SET_RATE_GATE); > + CLK_SET_RATE_GATE, 0); > if (IS_ERR(hw)) > goto err_free; > > diff --git a/drivers/clk/at91/sama7g5.c b/drivers/clk/at91/sama7g5.c > index 970135e19a75..ae52c10af040 100644 > --- a/drivers/clk/at91/sama7g5.c > +++ b/drivers/clk/at91/sama7g5.c > @@ -127,6 +127,8 @@ static const struct clk_pll_characteristics pll_characteristics = { > * @t: clock type > * @f: clock flags > * @eid: export index in sama7g5->chws[] array > + * @safe_div: intermediate divider need to be set on PRE_RATE_CHANGE > + * notification > */ > static const struct { > const char *n; > @@ -136,6 +138,7 @@ static const struct { > unsigned long f; > u8 t; > u8 eid; > + u8 safe_div; > } sama7g5_plls[][PLL_ID_MAX] = { > [PLL_ID_CPU] = { > { .n = "cpupll_fracck", > @@ -156,7 +159,12 @@ static const struct { > .t = PLL_TYPE_DIV, > /* This feeds CPU. It should not be disabled. */ > .f = CLK_IS_CRITICAL | CLK_SET_RATE_PARENT, > - .eid = PMC_CPUPLL, }, > + .eid = PMC_CPUPLL, > + /* > + * Safe div=15 should be safe even for switching b/w 1GHz and > + * 90MHz (frac pll might go up to 1.2GHz). > + */ > + .safe_div = 15, }, > }, > > [PLL_ID_SYS] = { > @@ -967,7 +975,8 @@ static void __init sama7g5_pmc_setup(struct device_node *np) > sama7g5_plls[i][j].p, i, > sama7g5_plls[i][j].c, > sama7g5_plls[i][j].l, > - sama7g5_plls[i][j].f); > + sama7g5_plls[i][j].f, > + sama7g5_plls[i][j].safe_div); > break; > > default: > -- Nicolas Ferre _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel