| 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: <3cc699dc-4021-b993-2b47-b00b40f380f8@baylibre.com>
Date: Thu, 13 Dec 2018 09:55:11 +0100
From: Neil Armstrong <narmstrong@...libre.com>
To: Sunny Luo <sunny.luo@...ogic.com>, Mark Brown <broonie@...nel.org>
Cc: Yixun Lan <yixun.lan@...ogic.com>,
Jerome Brunet <jbrunet@...libre.com>,
Kevin Hilman <khilman@...libre.com>,
Carlo Caione <carlo@...one.org>,
Jianxin Pan <jianxin.pan@...ogic.com>,
Xingyu Chen <xingyu.chen@...ogic.com>,
linux-spi@...r.kernel.org, linux-amlogic@...ts.infradead.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/3] spi: meson-axg: add a linear clock divider support
Hi Sunny,
On 13/12/2018 09:39, Sunny Luo wrote:
> The SPICC controller in Meson-AXG SoC is capable of using
> a linear clock divider to reach a much fine tuned range of clocks,
> while the old controller only use a power of two clock divider,
> result at a more coarse clock range.
This patch should definitely go before patch 1.
>
> Also convert the clock registration into Common Clock Framework.
>
> Signed-off-by: Sunny Luo <sunny.luo@...ogic.com>
> Signed-off-by: Yixun Lan <yixun.lan@...ogic.com>
> ---
> drivers/spi/Kconfig | 2 +-
> drivers/spi/spi-meson-spicc.c | 209 ++++++++++++++++++++++++++++++++++--------
> 2 files changed, 170 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index 9f89cb1..75f7362 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -384,7 +384,7 @@ config SPI_FSL_ESPI
>
> config SPI_MESON_SPICC
> tristate "Amlogic Meson SPICC controller"
> - depends on ARCH_MESON || COMPILE_TEST
> + depends on OF && COMMON_CLK && (ARCH_MESON || COMPILE_TEST)
> help
> This enables master mode support for the SPICC (SPI communication
> controller) available in Amlogic Meson SoCs.
> diff --git a/drivers/spi/spi-meson-spicc.c b/drivers/spi/spi-meson-spicc.c
> index 0384c28..59af18e 100644
> --- a/drivers/spi/spi-meson-spicc.c
> +++ b/drivers/spi/spi-meson-spicc.c
> @@ -116,6 +116,9 @@
> #define SPICC_DWADDR 0x24 /* Write Address of DMA */
>
> #define SPICC_ENH_CTL0 0x38 /* Enhanced Feature */
> +#define SPICC_ENH_CLK_CS_DELAY_MASK GENMASK(15, 0)
> +#define SPICC_ENH_DATARATE_MASK GENMASK(23, 16)
> +#define SPICC_ENH_DATARATE_EN BIT(24)
> #define SPICC_ENH_MOSI_OEN BIT(25)
> #define SPICC_ENH_CLK_OEN BIT(26)
> #define SPICC_ENH_CS_OEN BIT(27)
> @@ -131,6 +134,7 @@
> struct meson_spicc_data {
> unsigned int max_speed_hz;
> bool has_oen;
> + bool has_enhance_clk_div;
> };
>
> struct meson_spicc_device {
> @@ -138,6 +142,7 @@ struct meson_spicc_device {
> struct platform_device *pdev;
> void __iomem *base;
> struct clk *core;
> + struct clk *clk;
> struct spi_message *message;
> struct spi_transfer *xfer;
> const struct meson_spicc_data *data;
> @@ -325,40 +330,6 @@ static irqreturn_t meson_spicc_irq(int irq, void *data)
> return IRQ_HANDLED;
> }
>
> -static u32 meson_spicc_setup_speed(struct meson_spicc_device *spicc, u32 conf,
> - u32 speed)
> -{
> - unsigned long parent, value;
> - unsigned int i, div;
> -
> - parent = clk_get_rate(spicc->core);
> -
> - /* Find closest inferior/equal possible speed */
> - for (i = 0 ; i < 7 ; ++i) {
> - /* 2^(data_rate+2) */
> - value = parent >> (i + 2);
> -
> - if (value <= speed)
> - break;
> - }
> -
> - /* If provided speed it lower than max divider, use max divider */
> - if (i > 7) {
> - div = 7;
> - dev_warn_once(&spicc->pdev->dev, "unable to get close to speed %u\n",
> - speed);
> - } else
> - div = i;
> -
> - dev_dbg(&spicc->pdev->dev, "parent %lu, speed %u -> %lu (%u)\n",
> - parent, speed, value, div);
> -
> - conf &= ~SPICC_DATARATE_MASK;
> - conf |= FIELD_PREP(SPICC_DATARATE_MASK, div);
> -
> - return conf;
> -}
> -
> static void meson_spicc_setup_xfer(struct meson_spicc_device *spicc,
> struct spi_transfer *xfer)
> {
> @@ -367,9 +338,6 @@ static void meson_spicc_setup_xfer(struct meson_spicc_device *spicc,
> /* Read original configuration */
> conf = conf_orig = readl_relaxed(spicc->base + SPICC_CONREG);
>
> - /* Select closest divider */
> - conf = meson_spicc_setup_speed(spicc, conf, xfer->speed_hz);
> -
> /* Setup word width */
> conf &= ~SPICC_BITLENGTH_MASK;
> conf |= FIELD_PREP(SPICC_BITLENGTH_MASK,
> @@ -378,6 +346,8 @@ static void meson_spicc_setup_xfer(struct meson_spicc_device *spicc,
> /* Ignore if unchanged */
> if (conf != conf_orig)
> writel_relaxed(conf, spicc->base + SPICC_CONREG);
> +
> + clk_set_rate(spicc->clk, xfer->speed_hz);
> }
>
> static int meson_spicc_transfer_one(struct spi_master *master,
> @@ -486,9 +456,6 @@ static int meson_spicc_unprepare_transfer(struct spi_master *master)
> /* Disable all IRQs */
> writel(0, spicc->base + SPICC_INTREG);
>
> - /* Disable controller */
> - writel_bits_relaxed(SPICC_ENABLE, 0, spicc->base + SPICC_CONREG);
> -
> device_reset_optional(&spicc->pdev->dev);
>
> return 0;
> @@ -528,6 +495,157 @@ static void meson_spicc_cleanup(struct spi_device *spi)
> spi->controller_state = NULL;
> }
>
> +/*
> + * The Clock Mux
> + * x-----------------x x------------x x------\
> + * |---| 0) fixed factor |---| 1) old div |----| |
> + * | x-----------------x x------------x | |
> + * src ---| |5) mux|-- out
> + * | x-----------------x x------------x | |
> + * |---| 2) fixed factor |---| 3) new div |0---| |
> + * x-----------------x x------------x x------/
> + *
> + * Clk path for GX series:
> + * src -> 0 -> 1 -> out
> + *
> + * Clk path for AXG series:
> + * src -> 0 -> 1 -> 5 -> out
> + * src -> 2 -> 3 -> 5 -> out
> + */
> +
> +/* algorithm for div0 + div1: rate = freq / 4 / (2 ^ N) */
> +static struct clk_fixed_factor meson_spicc_div0 = {
> + .mult = 1,
> + .div = 4,
> +};
> +
> +static struct clk_divider meson_spicc_div1 = {
> + .reg = (void *)SPICC_CONREG,
> + .shift = 16,
> + .width = 3,
> + .flags = CLK_DIVIDER_POWER_OF_TWO,
> +};
> +
> +/* algorithm for div2 + div3: rate = freq / 2 / (N + 1) */
> +static struct clk_fixed_factor meson_spicc_div2 = {
> + .mult = 1,
> + .div = 2,
> +};
> +
> +static struct clk_divider meson_spicc_div3 = {
> + .reg = (void *)SPICC_ENH_CTL0,
> + .shift = 16,
> + .width = 8,
> +};
> +
> +static struct clk_mux meson_spicc_sel = {
> + .reg = (void *)SPICC_ENH_CTL0,
> + .mask = 0x1,
> + .shift = 24,
> +};
> +
> +static int meson_spicc_clk_init(struct meson_spicc_device *spicc)
> +{
> + struct device *dev = &spicc->pdev->dev;
> + struct clk_fixed_factor *div0;
> + struct clk_divider *div1;
> + struct clk_mux *mux;
> + struct clk_init_data init;
> + struct clk *clk;
> + const char *parent_names[1];
> + const char *mux_parent_names[2];
> + char name[32];
> +
> + div0 = &meson_spicc_div0;
> + snprintf(name, sizeof(name), "%s#_div0", dev_name(dev));
> + init.name = name;
> + init.ops = &clk_fixed_factor_ops;
> + init.flags = 0;
> + parent_names[0] = __clk_get_name(spicc->core);
> + init.parent_names = parent_names;
> + init.num_parents = 1;
> +
> + div0->hw.init = &init;
> +
> + clk = devm_clk_register(dev, &div0->hw);
> + if (WARN_ON(IS_ERR(clk)))
> + return PTR_ERR(clk);
> +
> + div1 = &meson_spicc_div1;
> + snprintf(name, sizeof(name), "%s#_div1", dev_name(dev));
> + init.name = name;
> + init.ops = &clk_divider_ops;
> + init.flags = CLK_SET_RATE_PARENT;
> + parent_names[0] = __clk_get_name(clk);
> + init.parent_names = parent_names;
> + init.num_parents = 1;
> +
> + div1->reg = spicc->base + (u64)div1->reg;
> + div1->hw.init = &init;
> +
> + clk = devm_clk_register(dev, &div1->hw);
> + if (WARN_ON(IS_ERR(clk)))
> + return PTR_ERR(clk);
> +
> + if (!spicc->data->has_enhance_clk_div) {
> + spicc->clk = clk;
> + return 0;
> + }
> +
> + mux_parent_names[0] = __clk_get_name(clk);
> +
> + div0 = &meson_spicc_div2;
> + snprintf(name, sizeof(name), "%s#_div2", dev_name(dev));
> + init.name = name;
> + init.ops = &clk_fixed_factor_ops;
> + init.flags = 0;
> + parent_names[0] = __clk_get_name(spicc->core);
> + init.parent_names = parent_names;
> + init.num_parents = 1;
> +
> + div0->hw.init = &init;
> +
> + clk = devm_clk_register(dev, &div0->hw);
> + if (WARN_ON(IS_ERR(clk)))
> + return PTR_ERR(clk);
> +
> + div1 = &meson_spicc_div3;
> + snprintf(name, sizeof(name), "%s#_div3", dev_name(dev));
> + init.name = name;
> + init.ops = &clk_divider_ops;
> + init.flags = CLK_SET_RATE_PARENT;
> + parent_names[0] = __clk_get_name(clk);
> + init.parent_names = parent_names;
> + init.num_parents = 1;
> +
> + div1->reg = spicc->base + (u64)div1->reg;
> + div1->hw.init = &init;
> +
> + clk = devm_clk_register(dev, &div1->hw);
> + if (WARN_ON(IS_ERR(clk)))
> + return PTR_ERR(clk);
> +
> + mux_parent_names[1] = __clk_get_name(clk);
> +
> + mux = &meson_spicc_sel;
> + snprintf(name, sizeof(name), "%s#_sel", dev_name(dev));
> + init.name = name;
> + init.ops = &clk_mux_ops;
> + init.parent_names = mux_parent_names;
> + init.num_parents = 2;
> + init.flags = CLK_SET_RATE_PARENT | CLK_SET_RATE_NO_REPARENT;
> +
> + mux->reg = spicc->base + (u64)mux->reg;
> + mux->hw.init = &init;
> +
> + spicc->clk = devm_clk_register(dev, &mux->hw);
> + if (WARN_ON(IS_ERR(spicc->clk)))
> + return PTR_ERR(spicc->clk);
> +
> + clk_set_parent(spicc->clk, clk);
> + return 0;
> +}
I'll let Jerome review this !
> +
> static int meson_spicc_probe(struct platform_device *pdev)
> {
> struct spi_master *master;
> @@ -557,6 +675,10 @@ static int meson_spicc_probe(struct platform_device *pdev)
> goto out_master;
> }
>
> + /* Set master mode and enable controller */
> + writel_relaxed(SPICC_ENABLE | SPICC_MODE_MASTER,
> + spicc->base + SPICC_CONREG);
Please remove it from meson_spicc_prepare_message() now.
> +
> /* Disable all IRQs */
> writel_relaxed(0, spicc->base + SPICC_INTREG);
>
> @@ -603,6 +725,12 @@ static int meson_spicc_probe(struct platform_device *pdev)
> master->max_speed_hz = min_t(unsigned int, rate >> 1,
> spicc->data->max_speed_hz);
>
> + ret = meson_spicc_clk_init(spicc);
> + if (ret) {
> + dev_err(&pdev->dev, "clock registration failed\n");
> + goto out_master;
> + }
> +
> ret = devm_spi_register_master(&pdev->dev, master);
> if (ret) {
> dev_err(&pdev->dev, "spi master registration failed\n");
> @@ -639,6 +767,7 @@ static const struct meson_spicc_data meson_spicc_gx_data = {
> static const struct meson_spicc_data meson_spicc_axg_data = {
> .max_speed_hz = 80000000,
> .has_oen = true,
> + .has_enhance_clk_div = true,
> };
>
> static const struct of_device_id meson_spicc_of_match[] = {
>
Powered by blists - more mailing lists