From mboxrd@z Thu Jan 1 00:00:00 1970 From: Neil Armstrong Subject: Re: [PATCH v2 3/3] spi: meson-axg: add a linear clock divider support Date: Thu, 13 Dec 2018 09:55:11 +0100 Message-ID: <3cc699dc-4021-b993-2b47-b00b40f380f8@baylibre.com> References: <1544690354-16409-1-git-send-email-sunny.luo@amlogic.com> <1544690354-16409-4-git-send-email-sunny.luo@amlogic.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: Yixun Lan , Jerome Brunet , Kevin Hilman , Carlo Caione , Jianxin Pan , Xingyu Chen , linux-spi@vger.kernel.org, linux-amlogic@lists.infradead.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org To: Sunny Luo , Mark Brown Return-path: In-Reply-To: <1544690354-16409-4-git-send-email-sunny.luo@amlogic.com> Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-Id: linux-spi.vger.kernel.org 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 > Signed-off-by: Yixun Lan > --- > 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[] = { >