[PATCH] clk/arc: Add I2S PLL clock driver
Jose Abreu
Jose.Abreu at synopsys.com
Tue Mar 29 05:48:43 PDT 2016
Hi Alexey,
On 29-03-2016 12:25, Alexey Brodkin wrote:
> Hi Jose,
>
> On Tue, 2016-03-29 at 11:56 +0100, Jose Abreu wrote:
>> The ARC SDP I2S clock can be programmed using a
>> specific PLL.
>>
>> This patch has the goal of adding a clock driver
>> that programs this PLL.
>>
>> At this moment the rate values are hardcoded in
>> a table but in the future it would be ideal to
>> use a function which determines the PLL values
>> given the desired rate.
>>
>> Signed-off-by: Jose Abreu <joabreu at synopsys.com>
>> ---
> I believe these kind of patches worth sending to
> linux-snps mailing list (at least add it in Cc) so
> they won't be lost in time and could be used later as
> useful references.
Agree.
>
>> arch/arc/boot/dts/axs10x_mb.dtsi | 5 ++
>> drivers/clk/Makefile | 1 +
>> drivers/clk/arc/Makefile | 1 +
>> drivers/clk/arc/i2s_pll_clock.c | 143 +++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 150 insertions(+)
>> create mode 100644 drivers/clk/arc/Makefile
>> create mode 100644 drivers/clk/arc/i2s_pll_clock.c
>>
>> diff --git a/arch/arc/boot/dts/axs10x_mb.dtsi b/arch/arc/boot/dts/axs10x_mb.dtsi
>> index ab5d570..9c68226 100644
>> --- a/arch/arc/boot/dts/axs10x_mb.dtsi
>> +++ b/arch/arc/boot/dts/axs10x_mb.dtsi
>> @@ -23,6 +23,11 @@
>> #clock-cells = <0>;
>> };
>>
>> + i2sclk: i2sclk {
>> + compatible = "snps,i2s-pll-clock";
>> + #clock-cells = <0>;
>> + };
>> +
>> apbclk: apbclk {
>> compatible = "fixed-clock";
>> clock-frequency = <50000000>;
>> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
>> index 46869d6..620e47f 100644
>> --- a/drivers/clk/Makefile
>> +++ b/drivers/clk/Makefile
>> @@ -84,3 +84,4 @@ obj-$(CONFIG_X86) += x86/
>> obj-$(CONFIG_ARCH_ZX) += zte/
>> obj-$(CONFIG_ARCH_ZYNQ) += zynq/
>> obj-$(CONFIG_H8300) += h8300/
>> +obj-$(CONFIG_ARC) += arc/
> Two things here:
> [1] This clock is relevant to only one board but not SoC, CPU core
> or whole architecture, so it makes sense to make it dependent on
> the AXS platform, i.e. CONFIG_ARC_PLAT_AXS10X.
>
> [2] Something similar is applicable to folder name, I would suggest to
> use "drivers/clk/axs10x"
I will rename to axs10x and use CONFIG_ARC_PLAT_AXS10X.
>> diff --git a/drivers/clk/arc/Makefile b/drivers/clk/arc/Makefile
>> new file mode 100644
>> index 0000000..01996b8
>> --- /dev/null
>> +++ b/drivers/clk/arc/Makefile
>> @@ -0,0 +1 @@
>> +obj-y += i2s_pll_clock.o
>> diff --git a/drivers/clk/arc/i2s_pll_clock.c b/drivers/clk/arc/i2s_pll_clock.c
>> new file mode 100644
>> index 0000000..8c401f1
>> --- /dev/null
>> +++ b/drivers/clk/arc/i2s_pll_clock.c
>> @@ -0,0 +1,143 @@
>> +/*
>> + * Synopsys I2S PLL clock driver
> Again that's not generic SNPS I2S clock but clock driver for a particular board.
>
>> + *
>> + * drivers/clk/arc/i2s_pll_clock.c
> BTW not sure why this path could be needed here.
> IMHO that might be safely dropped.
>
>> + *
>> + * Copyright (C) 2016 Synopsys
>> + *
>> + * This file is licensed under the terms of the GNU General Public
>> + * License version 2. This program is licensed "as is" without any
>> + * warranty of any kind, whether express or implied.
>> + */
>> +#include <linux/clk-provider.h>
>> +#include <linux/err.h>
>> +#include <linux/device.h>
>> +#include <linux/of_address.h>
>> +#include <linux/slab.h>
>> +
>> +/* FPGA Version Info */
>> +#define FPGA_VER_INFO 0xE0011230
>> +#define FPGA_VER_27M 0x000FBED9
> This is a little bit tricky.
> We'll need to do that check in each and every clock driver for AXS10x
> and so it worth putting this code in some common location (I mean common
> for all axs10x clock drivers).
>
This is not common to all clocks. We are expecting a new firmware release which
will change the reference clock input for the I2S PLL from 27Mhz to 28.224 Mhz.
To support both the old and the new firmware we check this version info register
so that we can determine which firmware version is being used. Due to this
change the PLL dividers are different. As the remaining clocks will be
unaffected I think this can remain only in this driver.
>> +/* PLL registers addresses */
>> +#define PLL_IDIV_ADDR 0xE00100A0
>> +#define PLL_FBDIV_ADDR 0xE00100A4
>> +#define PLL_ODIV0_ADDR 0xE00100A8
>> +#define PLL_ODIV1_ADDR 0xE00100AC
>>
>> +struct i2s_pll_clk {
>> + struct clk_hw hw;
>> +};
>> +
>> +struct i2s_pll_cfg {
>> + unsigned int rate;
>> + unsigned int idiv;
>> + unsigned int fbdiv;
>> + unsigned int odiv0;
>> + unsigned int odiv1;
>> +};
>> +
>> +static const struct i2s_pll_cfg i2s_pll_cfg_27m[] = {
>> + /* 27 Mhz */
>> + { 1024000, 0x104, 0x451, 0x10E38, 0x2000 },
>> + { 1411200, 0x104, 0x596, 0x10D35, 0x2000 },
>> + { 1536000, 0x208, 0xA28, 0x10B2C, 0x2000 },
>> + { 2048000, 0x82, 0x451, 0x10E38, 0x2000 },
>> + { 2822400, 0x82, 0x596, 0x10D35, 0x2000 },
>> + { 3072000, 0x104, 0xA28, 0x10B2C, 0x2000 },
>> + { 0, 0, 0, 0, 0 },
>> +};
>> +
>> +static const struct i2s_pll_cfg i2s_pll_cfg_28m[] = {
>> + /* 28.224 Mhz */
>> + { 1024000, 0x82, 0x105, 0x107DF, 0x2000 },
>> + { 1411200, 0x28A, 0x1, 0x10001, 0x2000 },
>> + { 1536000, 0xA28, 0x187, 0x10042, 0x2000 },
>> + { 2048000, 0x41, 0x105, 0x107DF, 0x2000 },
>> + { 2822400, 0x145, 0x1, 0x10001, 0x2000 },
>> + { 3072000, 0x514, 0x187, 0x10042, 0x2000 },
>> + { 0, 0, 0, 0, 0 },
>> +};
>> +
>> +static unsigned long i2s_pll_recalc_rate(struct clk_hw *hw,
>> + unsigned long parent_rate)
>> +{
>> + /* TODO */
>> + pr_info("%s: parent_rate=%ld\n", __func__, parent_rate);
>> + return parent_rate;
>> +}
>> +
>> +static long i2s_pll_round_rate(struct clk_hw *hw, unsigned long rate,
>> + unsigned long *prate)
>> +{
>> + /* TODO: Round rate to nearest valid rate */
>> + *prate = rate;
>> + pr_info("%s: rate=%ld, prate=%ld\n", __func__, rate, *prate);
>> + return *prate;
>> +}
> These are now just dummy functions so probably they could be dropped.
Agree but I will have to check if the clock core needs this functions to be defined.
>> +static int i2s_pll_set_rate(struct clk_hw *hw, unsigned long rate,
>> + unsigned long parent_rate)
>> +{
>> + const struct i2s_pll_cfg *pll_cfg;
>> + int i;
>> +
>> + if (readl((void *)FPGA_VER_INFO) <= FPGA_VER_27M)
>> + pll_cfg = i2s_pll_cfg_27m;
>> + else
>> + pll_cfg = i2s_pll_cfg_28m;
> As I mentioned above we need to check board version in common place and then
> just use some variable or structure member for comparison.
>
> Otherwise it looks pretty nice!
>
> -Alexey
Thanks for your comments!
Best regards,
Jose Miguel Abreu
More information about the linux-snps-arc
mailing list