linux-kernel - Re: [PATCH v3 09/13] clk: imx: Add audiomix clock controller support

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <158758072945.163502.17393357985434075011@swboyd.mtv.corp.google.com>
Date:   Wed, 22 Apr 2020 11:38:49 -0700
From:   Stephen Boyd <sboyd@...nel.org>
To:     Abel Vesa <abel.vesa@....com>, Anson Huang <anson.huang@....com>,
        Jacky Bai <ping.bai@....com>, Lee Jones <lee.jones@...aro.org>,
        Leonard Crestez <leonard.crestez@....com>,
        Peng Fan <peng.fan@....com>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Sascha Hauer <kernel@...gutronix.de>,
        Shawn Guo <shawnguo@...nel.org>
Cc:     NXP Linux Team <linux-imx@....com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        devicetree@...r.kernel.org, linux-clk@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, Abel Vesa <abel.vesa@....com>
Subject: Re: [PATCH v3 09/13] clk: imx: Add audiomix clock controller support

Quoting Abel Vesa (2020-04-15 01:02:49)
> diff --git a/drivers/clk/imx/clk-audiomix.c b/drivers/clk/imx/clk-audiomix.c
> new file mode 100644
> index 00000000..aa48b06
> --- /dev/null
> +++ b/drivers/clk/imx/clk-audiomix.c
> @@ -0,0 +1,175 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright 2019 NXP.
> + */
> +
> +#include <dt-bindings/clock/imx8mp-clock.h>
> +#include <linux/clk.h>

Include <linux/clk-provider.h> because this is a clk provider.

> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>

Is this include used?

> +#include <linux/slab.h>
> +#include <linux/types.h>
> +
> +#include "clk.h"
> +
> +static int shared_count_pdm;
> +static struct clk_hw **hws;
> +static struct clk_hw_onecell_data *clk_hw_data;
> +static uint32_t audiomix_clk_saved_regs[14];
> +static struct clk *clk_audio_root;
> +static struct clk *clk_audio_ahb;
> +static struct clk *clk_audio_axi_div;
> +
> +static const struct imx_pll14xx_rate_table imx_audiomix_sai_pll_tbl[] = {
> +       PLL_1443X_RATE(650000000U, 325, 3, 2, 0),
> +};
> +
> +static const struct imx_pll14xx_clk imx_audiomix_sai_pll = {
> +       .type = PLL_1443X,
> +       .rate_table = imx_audiomix_sai_pll_tbl,
> +};
> +
> +static const char * const imx_sai_mclk2_sels[] = {"sai1", "sai2", "sai3", "dummy",
> +                                       "sai5", "sai6", "sai7", "dummy",
> +                                       "dummy", "dummy", "dummy",
> +                                       "dummy", "dummy", "dummy", "dummy"};
> +static const char * const imx_sai1_mclk1_sels[] = {"sai1", "dummy", };
> +static const char * const imx_sai2_mclk1_sels[] = {"sai2", "dummy", };
> +static const char * const imx_sai3_mclk1_sels[] = {"sai3", "dummy", };
> +static const char * const imx_sai5_mclk1_sels[] = {"sai5", "dummy", };
> +static const char * const imx_sai6_mclk1_sels[] = {"sai6", "dummy", };
> +static const char * const imx_sai7_mclk1_sels[] = {"sai7", "dummy", };
> +static const char * const imx_pdm_sels[] = {"pdm", "sai_pll_div2", "dummy", "dummy" };
> +static const char * const imx_sai_pll_ref_sels[] = {"osc_24m", "dummy", "dummy", "dummy", };
> +static const char * const imx_sai_pll_bypass_sels[] = {"sai_pll", "sai_pll_ref_sel", };
> +
> +static int imx_audiomix_clk_probe(struct platform_device *pdev)
[..]
> +
> +       /* unbypass the pll */
> +       clk_hw_set_parent(hws[IMX8MP_CLK_AUDIOMIX_SAI_PLL_BYPASS],
> +                               hws[IMX8MP_CLK_AUDIOMIX_SAI_PLL]);
> +
> +       imx_check_clk_hws(hws, IMX8MP_CLK_AUDIOMIX_END);
> +
> +       of_clk_add_hw_provider(dev->of_node, of_clk_hw_onecell_get,
> +                               clk_hw_data);

What if this fails?

> +
> +       return 0;
> +}
> +
> +static const struct of_device_id imx_audiomix_clk_of_match[] = {
> +       { .compatible = "fsl,imx8mp-audiomix-clk" },
> +       { /* Sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, imx_audiomix_clk_of_match);
> +
> +

Nitpick: Remove double newline?

> +static struct platform_driver imx_audiomix_clk_driver = {
> +       .probe = imx_audiomix_clk_probe,
> +       .driver = {
> +               .name = "imx-audiomix-clk",
> +               .of_match_table = of_match_ptr(imx_audiomix_clk_of_match),

Add suppress_bind_attr here so that this can't be removed from
userspace?

> +       },
> +};
> +module_platform_driver(imx_audiomix_clk_driver);