| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <0d0448c7-c33b-8960-d2ed-0a22e2f7fb3d@tuxon.dev>
Date: Thu, 5 Oct 2023 07:24:30 +0300
From: claudiu beznea <claudiu.beznea@...on.dev>
To: Geert Uytterhoeven <geert@...ux-m68k.org>
Cc: mturquette@...libre.com, sboyd@...nel.org, robh+dt@...nel.org,
krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
linus.walleij@...aro.org, gregkh@...uxfoundation.org,
jirislaby@...nel.org, magnus.damm@...il.com,
catalin.marinas@....com, will@...nel.org,
quic_bjorande@...cinc.com, konrad.dybcio@...aro.org, arnd@...db.de,
neil.armstrong@...aro.org, prabhakar.mahadev-lad.rj@...renesas.com,
biju.das.jz@...renesas.com, linux-renesas-soc@...r.kernel.org,
linux-clk@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-gpio@...r.kernel.org,
linux-serial@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2 10/28] clk: renesas: rzg2l: refactor sd mux driver
Hi, Geert,
On 04.10.2023 14:30, Geert Uytterhoeven wrote:
> Hi Claudiu,
>
> On Fri, Sep 29, 2023 at 7:39 AM Claudiu <claudiu.beznea@...on.dev> wrote:
>> From: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
>>
>> Refactor SD MUX driver to be able to reuse the same code on RZ/G3S.
>> RZ/G2{L, UL} has a limitation with regards to switching the clock source
>> for SD MUX (MUX clock source has to be switched to 266MHz before switching
>> b/w 533MHz and 400MHz). This limitation has been introduced as a clock
>> notifier that is registered on platform based initialization data thus the
>> SD MUX code could be reused on RZ/G3S.
>>
>> As both RZ/G2{L, UL} and RZ/G3S has specific bits in specific registers
>> to check if the clock switching has been done, this configuration (register
>> offset, register bits and bits width) is now passed though
>> struct cpg_core_clk::sconf (status configuration) from platform specific
>> initialization code.
>>
>> Along with struct cpg_core_clk::sconf the mux table indices are also
>> passed from platform specific initialization code.
>>
>> Also, mux flags are now passed to DEF_SD_MUX() as they will be later
>> used by RZ/G3S.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
>> ---
>>
>> Changes in v2:
>> - s/indexes/indices in commit description
>> - mentioned in commit description that mux flags can now be passed to
>> driver though DEF_SD_MUX() macro
>> - removed SoC specific names from macros' names
>> - added spaces after { and before } when initializing arrays
>> - preserved the order of .[gs]set_parent() API definitions for simpler
>> diff b/w versions
>> - removed SD_MUX_NOTIF macro
>
> Thanks for the update!
>
>> --- a/drivers/clk/renesas/rzg2l-cpg.c
>> +++ b/drivers/clk/renesas/rzg2l-cpg.c
>
>> @@ -142,6 +146,77 @@ static void rzg2l_cpg_del_clk_provider(void *data)
>> of_clk_del_provider(data);
>> }
>>
>> +/* Must be called in atomic context. */
>> +static int rzg2l_cpg_wait_clk_update_done(void __iomem *base, u32 conf)
>> +{
>> + u32 bitmask = GENMASK(GET_WIDTH(conf) - 1, 0) << GET_SHIFT(conf);
>> + u32 off = GET_REG_OFFSET(conf);
>> + u32 val;
>> +
>> + return readl_poll_timeout_atomic(base + off, val, !(val & bitmask), 10, 200);
>> +}
>> +
>> +int rzg2l_cpg_sd_mux_clk_notifier(struct notifier_block *nb, unsigned long event,
>> + void *data)
>> +{
>> + struct clk_notifier_data *cnd = data;
>> + struct clk_hw *hw = __clk_get_hw(cnd->clk);
>> + struct clk_hw_data *clk_hw_data = to_clk_hw_data(hw);
>> + struct rzg2l_cpg_priv *priv = clk_hw_data->priv;
>> + u32 off = GET_REG_OFFSET(clk_hw_data->conf);
>> + u32 shift = GET_SHIFT(clk_hw_data->conf);
>> + const u32 clk_src_266 = 3;
>> + unsigned long flags;
>> + u32 bitmask;
>> + int ret;
>> +
>> + if (event != PRE_RATE_CHANGE || (cnd->new_rate / MEGA == 266))
>> + return 0;
>
> include/linux/clk.h:
>
> * PRE_RATE_CHANGE - called immediately before the clk rate is changed,
> * to indicate that the rate change will proceed. Drivers must
> * immediately terminate any operations that will be affected by the
> * rate change. Callbacks may either return NOTIFY_DONE, NOTIFY_OK,
> * NOTIFY_STOP or NOTIFY_BAD.
Indeed I missed these.
>
>> +
>> + spin_lock_irqsave(&priv->rmw_lock, flags);
>> +
>> + /*
>> + * As per the HW manual, we should not directly switch from 533 MHz to
>> + * 400 MHz and vice versa. To change the setting from 2’b01 (533 MHz)
>> + * to 2’b10 (400 MHz) or vice versa, Switch to 2’b11 (266 MHz) first,
>> + * and then switch to the target setting (2’b01 (533 MHz) or 2’b10
>> + * (400 MHz)).
>> + * Setting a value of '0' to the SEL_SDHI0_SET or SEL_SDHI1_SET clock
>> + * switching register is prohibited.
>> + * The clock mux has 3 input clocks(533 MHz, 400 MHz, and 266 MHz), and
>> + * the index to value mapping is done by adding 1 to the index.
>> + */
>> + bitmask = (GENMASK(GET_WIDTH(clk_hw_data->conf) - 1, 0) << shift) << 16;
>> + writel(bitmask | (clk_src_266 << shift), priv->base + off);
>> +
>> + /* Wait for the update done. */
>> + ret = rzg2l_cpg_wait_clk_update_done(priv->base, clk_hw_data->sconf);
>> +
>> + spin_unlock_irqrestore(&priv->rmw_lock, flags);
>> +
>> + if (ret)
>> + dev_err(priv->dev, "failed to switch to safe clk source\n");
>> +
>> + return ret;
>
> Likewise.
>
>> +}
>
>>
>> static const struct clk_ops rzg2l_cpg_sd_clk_mux_ops = {
>> .determine_rate = __clk_mux_determine_rate_closest,
>> - .set_parent = rzg2l_cpg_sd_clk_mux_set_parent,
>> - .get_parent = rzg2l_cpg_sd_clk_mux_get_parent,
>> + .set_parent = rzg2l_cpg_sd_mux_clk_set_parent,
>> + .get_parent = rzg2l_cpg_sd_mux_clk_get_parent,
>
> Please keep the old names, for consistency with
> __clk_mux_determine_rate_closest() and drivers/clk/clk-mux.c, and to
> reduce the diff.
>
> Any existing inconsistent use of "clk_mux" vs. "mux_clk" can be resolved
> later with a separate patch, if anyone cares.
ok
>
>> --- a/drivers/clk/renesas/rzg2l-cpg.h
>> +++ b/drivers/clk/renesas/rzg2l-cpg.h
>
>> @@ -272,4 +276,6 @@ extern const struct rzg2l_cpg_info r9a07g044_cpg_info;
>> extern const struct rzg2l_cpg_info r9a07g054_cpg_info;
>> extern const struct rzg2l_cpg_info r9a09g011_cpg_info;
>>
>> +int rzg2l_cpg_sd_mux_clk_notifier(struct notifier_block *nb, unsigned long event, void *data);
>
> rzg2l_cpg_sd_clk_mux_notifier()?
ok
>
>> +
>> #endif
>
> The rest LGTM.
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
Powered by blists - more mailing lists