| Message ID | 50c75be8ecab225a1dd49628a173d211a02755b2.1459791946.git.joabreu@synopsys.com |
|---|---|
| State | New |
| Headers | show |
Hi Jose, On Mon, 2016-04-11 at 11:41 +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@synopsys.com> > --- > > Changes v3 -> v4: > * Added binding document (as suggested by Stephen Boyd) > * Minor code style fixes (as suggested by Stephen Boyd) > * Use ioremap (as suggested by Stephen Boyd) > * Implement round_rate (as suggested by Stephen Boyd) > * Change to platform driver (as suggested by Stephen Boyd) > * Use {readl/writel}_relaxed (as suggested by Vineet Gupta) > > Changes v2 -> v3: > * Implemented recalc_rate > > Changes v1 -> v2: > * Renamed folder to axs10x (as suggested by Alexey Brodkin) > * Added more supported rates [snip] > diff --git a/Documentation/devicetree/bindings/clock/i2s-pll-clock.txt b/Documentation/devicetree/bindings/clock/i2s- > pll-clock.txt > new file mode 100644 > index 0000000..ff86a41 > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/i2s-pll-clock.txt > @@ -0,0 +1,17 @@ > +Binding for the AXS10X I2S PLL clock > + > +This binding uses the common clock binding[1]. > + > +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt > + > +Required properties: > +- compatible: shall be "snps,i2s-pll-clock" > +- #clock-cells: from common clock binding; Should always be set to 0. > +- reg : Address and length of the I2S PLL register set. > + > +Example: > + clock@0x100a0 { Please remove "0x" from node name. > + compatible = "snps,i2s-pll-clock"; > + reg = <0x100a0 0x10>; > + #clock-cells = <0>; > + }; > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile > index 46869d6..2ca62dc6 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_PLAT_AXS10X) += axs10x/ > diff --git a/drivers/clk/axs10x/Makefile b/drivers/clk/axs10x/Makefile > new file mode 100644 > index 0000000..01996b8 > --- /dev/null > +++ b/drivers/clk/axs10x/Makefile > @@ -0,0 +1 @@ > +obj-y += i2s_pll_clock.o > diff --git a/drivers/clk/axs10x/i2s_pll_clock.c b/drivers/clk/axs10x/i2s_pll_clock.c > new file mode 100644 > index 0000000..3ba4e2f > --- /dev/null > +++ b/drivers/clk/axs10x/i2s_pll_clock.c > @@ -0,0 +1,217 @@ > +/* > + * Synopsys AXS10X SDP I2S PLL clock driver > + * > + * 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/platform_device.h> > +#include <linux/module.h> > +#include <linux/clk-provider.h> > +#include <linux/err.h> > +#include <linux/device.h> "linux/platform_device.h" includes "linux/device.h" so you may make this list of headers a little bit shorter. > +#include <linux/of_address.h> > +#include <linux/slab.h> > +#include <linux/of.h> "linux/of_address.h" already includes "linux/of.h". [snip] > + > +static const struct of_device_id i2s_pll_clk_id[] = { > + { .compatible = "snps,i2s-pll-clock", }, I would think that it makes sense to add the board name in this compatible string. So something like "snps,axs10x-i2s-pll-clock" IMHO looks much more informative. Also adding Rob Herring and DT mailing list in Cc. Please make sure Rod acks your bindings and corresponding docs. -Alexey
On 04/11, Alexey Brodkin wrote: > On Mon, 2016-04-11 at 11:41 +0100, Jose Abreu wrote: > > + * warranty of any kind, whether express or implied. > > + */ > > + > > +#include <linux/platform_device.h> > > +#include <linux/module.h> > > +#include <linux/clk-provider.h> > > +#include <linux/err.h> > > +#include <linux/device.h> > > "linux/platform_device.h" includes "linux/device.h" so you may make this list of headers > a little bit shorter. > > > +#include <linux/of_address.h> > > +#include <linux/slab.h> > > +#include <linux/of.h> > > "linux/of_address.h" already includes "linux/of.h". It's ok to include things twice. In fact, its better to avoid any implicit includes so that if we ever want to remove includes from other headers we can do so without disturbing this driver.
Hi Stephen, On Mon, 2016-04-11 at 15:03 -0700, sboyd@codeaurora.org wrote: > On 04/11, Alexey Brodkin wrote: > > > > On Mon, 2016-04-11 at 11:41 +0100, Jose Abreu wrote: > > > > > > + * warranty of any kind, whether express or implied. > > > + */ > > > + > > > +#include <linux/platform_device.h> > > > +#include <linux/module.h> > > > +#include <linux/clk-provider.h> > > > +#include <linux/err.h> > > > +#include <linux/device.h> > > "linux/platform_device.h" includes "linux/device.h" so you may make this list of headers > > a little bit shorter. > > > > > > > > +#include <linux/of_address.h> > > > +#include <linux/slab.h> > > > +#include <linux/of.h> > > "linux/of_address.h" already includes "linux/of.h". > It's ok to include things twice. In fact, its better to avoid any > implicit includes so that if we ever want to remove includes from > other headers we can do so without disturbing this driver. IMHO approach with minimal amount of headers is nice just because it's easier to check if everything is in place. I mean attempt to compile will immediately reveal a missing header. So that's what I do - I remove as many inclusions as I may until stuff compiles. But with approach of explicit inclusion it's much easier to include much more headers than really needed. The only way to figure out if header is really required is to manually check all used functions in the current source which is way too unreliable and probably nobody will do it ever anyways. And that's how we'll get more stale and pointless inclusions. -Alexey
On 04/15, Alexey Brodkin wrote: > Hi Stephen, > > On Mon, 2016-04-11 at 15:03 -0700, sboyd@codeaurora.org wrote: > > On 04/11, Alexey Brodkin wrote: > > > > > > On Mon, 2016-04-11 at 11:41 +0100, Jose Abreu wrote: > > > > > > > > + * warranty of any kind, whether express or implied. > > > > + */ > > > > + > > > > +#include <linux/platform_device.h> > > > > +#include <linux/module.h> > > > > +#include <linux/clk-provider.h> > > > > +#include <linux/err.h> > > > > +#include <linux/device.h> > > > "linux/platform_device.h" includes "linux/device.h" so you may make this list of headers > > > a little bit shorter. > > > > > > > > > > > +#include <linux/of_address.h> > > > > +#include <linux/slab.h> > > > > +#include <linux/of.h> > > > "linux/of_address.h" already includes "linux/of.h". > > It's ok to include things twice. In fact, its better to avoid any > > implicit includes so that if we ever want to remove includes from > > other headers we can do so without disturbing this driver. > > IMHO approach with minimal amount of headers is nice just because > it's easier to check if everything is in place. I mean attempt to compile > will immediately reveal a missing header. > > So that's what I do - I remove as many inclusions as I may until stuff compiles. Please don't. People try to break header includes cycles by moving things in one header to another header, and then those changes may need to fix random drivers because they are now missing an implicit include they were relying on, etc. > > But with approach of explicit inclusion it's much easier to include > much more headers than really needed. The only way to figure out if header is really > required is to manually check all used functions in the current source > which is way too unreliable and probably nobody will do it ever anyways. > And that's how we'll get more stale and pointless inclusions. Sounds like a job for a script!
On 04/11, Jose Abreu wrote: > new file mode 100644 > index 0000000..3ba4e2f > --- /dev/null > +++ b/drivers/clk/axs10x/i2s_pll_clock.c > @@ -0,0 +1,217 @@ > + > +static int i2s_pll_clk_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct device_node *node = dev->of_node; > + const char *clk_name; > + struct clk *clk; > + struct i2s_pll_clk *pll_clk; > + struct clk_init_data init; > + struct resource *mem; > + > + if (!node) > + return -ENODEV; Does this ever happen? Looks like dead code. > + > + pll_clk = devm_kzalloc(dev, sizeof(*pll_clk), GFP_KERNEL); > + if (!pll_clk) > + return -ENOMEM; > + > + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + pll_clk->base = devm_ioremap_resource(dev, mem); > + if (IS_ERR(pll_clk->base)) > + return PTR_ERR(pll_clk->base); > + > + clk_name = node->name; > + init.name = clk_name; > + init.ops = &i2s_pll_ops; > + init.num_parents = 0; > + pll_clk->hw.init = &init; > + > + clk = clk_register(NULL, &pll_clk->hw); Pass dev as first argument. Also use devm_clk_register() instead. > + if (IS_ERR(clk)) { > + dev_err(dev, "failed to register %s div clock (%ld)\n", > + clk_name, PTR_ERR(clk)); > + return PTR_ERR(clk); > + } > + > + if (readl((void *)FPGA_VER_INFO) <= FPGA_VER_27M) { Please don't readl directly from addresses. I think I mentioned that before and didn't get back to you when you replied asking for other solutions. I still think a proper DT is in order instead of doing this check for ref_clk. > + pll_clk->ref_clk = 27000000; > + pll_clk->pll_cfg = i2s_pll_cfg_27m; > + } else { > + pll_clk->ref_clk = 28224000; > + pll_clk->pll_cfg = i2s_pll_cfg_28m; > + } We should do this before registering the clk with the framework. > + > + return of_clk_add_provider(node, of_clk_src_simple_get, clk); > +} > + > +static int i2s_pll_clk_remove(struct platform_device *pdev) > +{ > + of_clk_del_provider(pdev->dev.of_node); > + return 0; > +} > + > +static const struct of_device_id i2s_pll_clk_id[] = { > + { .compatible = "snps,i2s-pll-clock", }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, i2s_pll_clk_id); > + > +static struct platform_driver i2s_pll_clk_driver = { > + .driver = { > + .name = "i2s-pll-clock", > + .of_match_table = of_match_ptr(i2s_pll_clk_id), You can drop of_match_ptr(), it doesn't have much use besides introducing compilation warnings. > + }, > + .probe = i2s_pll_clk_probe, > + .remove = i2s_pll_clk_remove, > +}; > +module_platform_driver(i2s_pll_clk_driver); >
Hi Stephen, On 16-04-2016 00:46, Stephen Boyd wrote: > On 04/11, Jose Abreu wrote: >> new file mode 100644 >> index 0000000..3ba4e2f >> --- /dev/null >> +++ b/drivers/clk/axs10x/i2s_pll_clock.c >> @@ -0,0 +1,217 @@ >> + >> +static int i2s_pll_clk_probe(struct platform_device *pdev) >> +{ >> + struct device *dev = &pdev->dev; >> + struct device_node *node = dev->of_node; >> + const char *clk_name; >> + struct clk *clk; >> + struct i2s_pll_clk *pll_clk; >> + struct clk_init_data init; >> + struct resource *mem; >> + >> + if (!node) >> + return -ENODEV; > Does this ever happen? Looks like dead code. Will remove. >> + >> + pll_clk = devm_kzalloc(dev, sizeof(*pll_clk), GFP_KERNEL); >> + if (!pll_clk) >> + return -ENOMEM; >> + >> + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> + pll_clk->base = devm_ioremap_resource(dev, mem); >> + if (IS_ERR(pll_clk->base)) >> + return PTR_ERR(pll_clk->base); >> + >> + clk_name = node->name; >> + init.name = clk_name; >> + init.ops = &i2s_pll_ops; >> + init.num_parents = 0; >> + pll_clk->hw.init = &init; >> + >> + clk = clk_register(NULL, &pll_clk->hw); > Pass dev as first argument. Also use devm_clk_register() instead. Ok. >> + if (IS_ERR(clk)) { >> + dev_err(dev, "failed to register %s div clock (%ld)\n", >> + clk_name, PTR_ERR(clk)); >> + return PTR_ERR(clk); >> + } >> + >> + if (readl((void *)FPGA_VER_INFO) <= FPGA_VER_27M) { > Please don't readl directly from addresses. I think I mentioned > that before and didn't get back to you when you replied asking > for other solutions. I still think a proper DT is in order > instead of doing this check for ref_clk. I think that the DT approach would be better but I also think that using two DT files with only one change between them is not viable. I can see some alternatives: 1) Pass the region of FPGA version in reg field of DT so that writel is not directly used; 2) Create a dummy parent clock driver that reads from FPGA version register and returns the rate; 3) Last resort: Use two DT files for each FPGA version. @Vineet, @Alexey: Can you give some suggestions? Some background: We are expecting a new firmware release for the AXS board that will change the reference clock value of the I2S PLL from 27MHz to 28.224MHz. Due to this change the dividers of this PLL will change. Right now I am directly reading from the FPGA version register but Stephen suggested to use a DT approach so that this rate is declared as parent clock. This would be a good solution but would require the usage of two different DT files (one for the current firmware and another for the new firmware), which I think is not ideal. What is your opinion? Some other solutions are listed above. >> + pll_clk->ref_clk = 27000000; >> + pll_clk->pll_cfg = i2s_pll_cfg_27m; >> + } else { >> + pll_clk->ref_clk = 28224000; >> + pll_clk->pll_cfg = i2s_pll_cfg_28m; >> + } > We should do this before registering the clk with the framework. Ok. >> + >> + return of_clk_add_provider(node, of_clk_src_simple_get, clk); >> +} >> + >> +static int i2s_pll_clk_remove(struct platform_device *pdev) >> +{ >> + of_clk_del_provider(pdev->dev.of_node); >> + return 0; >> +} >> + >> +static const struct of_device_id i2s_pll_clk_id[] = { >> + { .compatible = "snps,i2s-pll-clock", }, >> + { }, >> +}; >> +MODULE_DEVICE_TABLE(of, i2s_pll_clk_id); >> + >> +static struct platform_driver i2s_pll_clk_driver = { >> + .driver = { >> + .name = "i2s-pll-clock", >> + .of_match_table = of_match_ptr(i2s_pll_clk_id), > You can drop of_match_ptr(), it doesn't have much use besides > introducing compilation warnings. Ok. >> + }, >> + .probe = i2s_pll_clk_probe, >> + .remove = i2s_pll_clk_remove, >> +}; >> +module_platform_driver(i2s_pll_clk_driver); >> Best regards, Jose Miguel Abreu
On Monday 18 April 2016 04:00 PM, Jose Abreu wrote: >>> + if (readl((void *)FPGA_VER_INFO) <= FPGA_VER_27M) { >> > Please don't readl directly from addresses. I think I mentioned >> > that before and didn't get back to you when you replied asking >> > for other solutions. I still think a proper DT is in order >> > instead of doing this check for ref_clk. > I think that the DT approach would be better but I also think that using two DT > files with only one change between them is not viable. I can see some alternatives: > 1) Pass the region of FPGA version in reg field of DT so that writel is not > directly used; > 2) Create a dummy parent clock driver that reads from FPGA version register > and returns the rate; > 3) Last resort: Use two DT files for each FPGA version. > > @Vineet, @Alexey: Can you give some suggestions? > > Some background: > We are expecting a new firmware release for the AXS board that will change the > reference clock value of the I2S PLL from 27MHz to 28.224MHz. Due to this change > the dividers of this PLL will change. Right now I am directly reading from the > FPGA version register but Stephen suggested to use a DT approach so that this > rate is declared as parent clock. This would be a good solution but would > require the usage of two different DT files (one for the current firmware and > another for the new firmware), which I think is not ideal. What is your opinion? > Some other solutions are listed above. Consider this my ignorance of clk drivers, what exactly is the problem with that readl() for FPGA ver. Having 2 versions of DT is annoyance for sure, but the bigger headache is that it still won't help cases of users mixing and matching boards and DT. IMO this runtime check is pretty nice and will support both types of boards with exact same code/DT ! FWIW, both solutions #1 and #3 seem to imply a different DT - no ? And I really don't see how #2 makes things more elegant/abstracted w.r.t clk framework ? So I prefer what you had before. -Vineet
Hi Vineet, On 18-04-2016 12:49, Vineet Gupta wrote: > On Monday 18 April 2016 04:00 PM, Jose Abreu wrote: >>>> + if (readl((void *)FPGA_VER_INFO) <= FPGA_VER_27M) { >>>> Please don't readl directly from addresses. I think I mentioned >>>> that before and didn't get back to you when you replied asking >>>> for other solutions. I still think a proper DT is in order >>>> instead of doing this check for ref_clk. >> I think that the DT approach would be better but I also think that using two DT >> files with only one change between them is not viable. I can see some alternatives: >> 1) Pass the region of FPGA version in reg field of DT so that writel is not >> directly used; >> 2) Create a dummy parent clock driver that reads from FPGA version register >> and returns the rate; >> 3) Last resort: Use two DT files for each FPGA version. >> >> @Vineet, @Alexey: Can you give some suggestions? >> >> Some background: >> We are expecting a new firmware release for the AXS board that will change the >> reference clock value of the I2S PLL from 27MHz to 28.224MHz. Due to this change >> the dividers of this PLL will change. Right now I am directly reading from the >> FPGA version register but Stephen suggested to use a DT approach so that this >> rate is declared as parent clock. This would be a good solution but would >> require the usage of two different DT files (one for the current firmware and >> another for the new firmware), which I think is not ideal. What is your opinion? >> Some other solutions are listed above. > Consider this my ignorance of clk drivers, what exactly is the problem with that > readl() for FPGA ver. Having 2 versions of DT is annoyance for sure, but the > bigger headache is that it still won't help cases of users mixing and matching > boards and DT. IMO this runtime check is pretty nice and will support both types > of boards with exact same code/DT ! > > FWIW, both solutions #1 and #3 seem to imply a different DT - no ? Solution 1 only requires that the FPGA version register is declared in the DT, something like this: i2s_clock@100a0 { compatible = "snps,axs10x-i2s-pll-clock"; reg = <0x100a0 0x10 0x11230 0x04>; #clock-cells = <0>; }; And then the region is io-remapped. This solution would discard the direct readl from the address and would still be compatible with the different firmwares using the same DT. Solution 3 is the alternative that Stephen suggested which requires two different DT's. > > And I really don't see how #2 makes things more elegant/abstracted w.r.t clk > framework ? Yes, solution 2 is more of a workaround and is not the best by far. > > So I prefer what you had before. > -Vineet @Stephen: can you give some input so that I can submit a v6? Best regards, Jose Miguel Abreu
On 04/19, Jose Abreu wrote: > > @Stephen: can you give some input so that I can submit a v6? > I don't prefer putting the second register in the same DT node, but that's really up to the DT reviewers to approve such a design. The current binding has been acked by Rob right? Assuming the new binding is acked/reviewed then that solution is fine. Otherwise, I still prefer two DTS files for the two different FPGA versions. At the least, please use ioremap for any pointers that you readl/writel here. Beyond that, we should have a fixed rate source clk somewhere in the software view of the clk tree, because that reflects reality. Hardcoding the parent rate in the structure works, but doesn't properly express the clk tree.
Hi Stephen, On 20-04-2016 02:54, Stephen Boyd wrote: > On 04/19, Jose Abreu wrote: >> @Stephen: can you give some input so that I can submit a v6? >> > I don't prefer putting the second register in the same DT node, > but that's really up to the DT reviewers to approve such a > design. The current binding has been acked by Rob right? Yes. > Assuming the new binding is acked/reviewed then that solution is > fine. Ok, will then use the DT to pass the FPGA version register. > Otherwise, I still prefer two DTS files for the two different FPGA > versions. At the least, please use ioremap for any pointers that > you readl/writel here. > > Beyond that, we should have a fixed rate source clk somewhere in > the software view of the clk tree, because that reflects reality. > Hardcoding the parent rate in the structure works, but doesn't > properly express the clk tree. > Can I use a property in the DT to pass this reference clock? something like this: snps,parent-freq = <0xFBED9 27000000>, <0x0 28224000>; /* Tuple <fpga-version reference-clock-freq>, fpga-version = 0 is default */ Or use a parent clock? like: clk { compatible = "fixed-clock"; clock-frequency = <27000000>; #clock-cells = <0>; snps,fpga-version = <0xFBED9>; } It is important to distinguish between the different versions automatically, is any of these solutions ok? Best regards, Jose Miguel Abreu
Hi Jose, Stephen, On Wed, 2016-04-20 at 10:47 +0100, Jose Abreu wrote: > Hi Stephen, > > > On 20-04-2016 02:54, Stephen Boyd wrote: > > > > On 04/19, Jose Abreu wrote: > > > > > > @Stephen: can you give some input so that I can submit a v6? > > > > > I don't prefer putting the second register in the same DT node, > > but that's really up to the DT reviewers to approve such a > > design. The current binding has been acked by Rob right? > Yes. > > > > > Assuming the new binding is acked/reviewed then that solution is > > fine. > Ok, will then use the DT to pass the FPGA version register. We won't need to know FPGA version at all I think. Read my comment below. > > > > Otherwise, I still prefer two DTS files for the two different FPGA > > versions. At the least, please use ioremap for any pointers that > > you readl/writel here. > > > > Beyond that, we should have a fixed rate source clk somewhere in > > the software view of the clk tree, because that reflects reality. > > Hardcoding the parent rate in the structure works, but doesn't > > properly express the clk tree. > > > Can I use a property in the DT to pass this reference clock? something like this: > snps,parent-freq = <0xFBED9 27000000>, <0x0 28224000>; /* Tuple > <fpga-version reference-clock-freq>, fpga-version = 0 is default */ > > Or use a parent clock? like: > clk { > compatible = "fixed-clock"; > clock-frequency = <27000000>; > #clock-cells = <0>; > snps,fpga-version = <0xFBED9>; > } > > It is important to distinguish between the different versions automatically, is > any of these solutions ok? I do like that solution with a master clock but with some fine-tuning for simplification. We'll add master clock node for I2S as a fixed clock like that: ------------------->8------------------ i2s_master_clock: clk { #clock-cells = <0>; compatible = "fixed-clock"; clock-frequency = <27000000>; }; ------------------->8------------------ Note there's no mention of MB version, just a value of the frequency. And in the driver itself value of that master clock will be used for population of "pll_clk->ref_clk" directly. These are benefits we'll get with that approach: [1] We escape any IOs not related to our clock device (I mean "snps,i2s-pll-clock") itself. [2] We'll use whatever reference clock value is given. I.e. we'll be able to do a fix-up of that reference clock value early in platform code depending on HW we're running on. That's what people do here and there. [3] Remember another clock driver for AXS10x board is right around the corner. I mean the one for ARC PGU which uses exactly the same master clock. So one fixup as mentioned above will work at once for 2 clock drivers. Let me know if above makes sense. -Alexey
Hi Alexey, On 20-04-2016 17:12, Alexey Brodkin wrote: > Hi Jose, Stephen, > > On Wed, 2016-04-20 at 10:47 +0100, Jose Abreu wrote: >> Hi Stephen, >> >> >> On 20-04-2016 02:54, Stephen Boyd wrote: >>> On 04/19, Jose Abreu wrote: >>>> @Stephen: can you give some input so that I can submit a v6? >>>> >>> I don't prefer putting the second register in the same DT node, >>> but that's really up to the DT reviewers to approve such a >>> design. The current binding has been acked by Rob right? >> Yes. >> >>> Assuming the new binding is acked/reviewed then that solution is >>> fine. >> Ok, will then use the DT to pass the FPGA version register. > We won't need to know FPGA version at all I think. > Read my comment below. > >>> Otherwise, I still prefer two DTS files for the two different FPGA >>> versions. At the least, please use ioremap for any pointers that >>> you readl/writel here. >>> >>> Beyond that, we should have a fixed rate source clk somewhere in >>> the software view of the clk tree, because that reflects reality. >>> Hardcoding the parent rate in the structure works, but doesn't >>> properly express the clk tree. >>> >> Can I use a property in the DT to pass this reference clock? something like this: >> snps,parent-freq = <0xFBED9 27000000>, <0x0 28224000>; /* Tuple >> <fpga-version reference-clock-freq>, fpga-version = 0 is default */ >> >> Or use a parent clock? like: >> clk { >> compatible = "fixed-clock"; >> clock-frequency = <27000000>; >> #clock-cells = <0>; >> snps,fpga-version = <0xFBED9>; >> } >> >> It is important to distinguish between the different versions automatically, is >> any of these solutions ok? > I do like that solution with a master clock but with some fine-tuning > for simplification. > > We'll add master clock node for I2S as a fixed clock like that: > ------------------->8------------------ > i2s_master_clock: clk { > #clock-cells = <0>; > compatible = "fixed-clock"; > clock-frequency = <27000000>; > }; > ------------------->8------------------ > > Note there's no mention of MB version, just a value of the frequency. > And in the driver itself value of that master clock will be used for > population of "pll_clk->ref_clk" directly. > > These are benefits we'll get with that approach: > [1] We escape any IOs not related to our clock device (I mean > "snps,i2s-pll-clock") itself. > [2] We'll use whatever reference clock value is given. > I.e. we'll be able to do a fix-up of that reference clock > value early in platform code depending on HW we're running on. > That's what people do here and there. > [3] Remember another clock driver for AXS10x board is right around > the corner. I mean the one for ARC PGU which uses exactly the same > master clock. So one fixup as mentioned above will work > at once for 2 clock drivers. > > Let me know if above makes sense. That approach can't be used because the reference clock value will change in the next firmware release. The new release will have a reference clock of 28224000 Hz instead of the usual 27000000 Hz, so we need to have a way to distinguish between them. Because of that we can't have only one master clock unless you state to users that they have to change the reference clock value when using the new firmware release. Stephen suggested to use two DT files (one for each firmware release), but as Vineet said this would be annoying to the user so I am trying to use another solution so that only one DT file is required. > > -Alexey Best regards, Jose Miguel Abreu
Hi Jose, On Thu, 2016-04-21 at 10:51 +0100, Jose Abreu wrote: > Hi Alexey, > > > > Otherwise, I still prefer two DTS files for the two different FPGA > > > > versions. At the least, please use ioremap for any pointers that > > > > you readl/writel here. > > > > > > > > Beyond that, we should have a fixed rate source clk somewhere in > > > > the software view of the clk tree, because that reflects reality. > > > > Hardcoding the parent rate in the structure works, but doesn't > > > > properly express the clk tree. > > > > > > > Can I use a property in the DT to pass this reference clock? something like this: > > > snps,parent-freq = <0xFBED9 27000000>, <0x0 28224000>; /* Tuple > > > <fpga-version reference-clock-freq>, fpga-version = 0 is default */ > > > > > > Or use a parent clock? like: > > > clk { > > > compatible = "fixed-clock"; > > > clock-frequency = <27000000>; > > > #clock-cells = <0>; > > > snps,fpga-version = <0xFBED9>; > > > } > > > > > > It is important to distinguish between the different versions automatically, is > > > any of these solutions ok? > > I do like that solution with a master clock but with some fine-tuning > > for simplification. > > > > We'll add master clock node for I2S as a fixed clock like that: > > ------------------->8------------------ > > i2s_master_clock: clk { > > #clock-cells = <0>; > > compatible = "fixed-clock"; > > clock-frequency = <27000000>; > > }; > > ------------------->8------------------ > > > > Note there's no mention of MB version, just a value of the frequency. > > And in the driver itself value of that master clock will be used for > > population of "pll_clk->ref_clk" directly. > > > > These are benefits we'll get with that approach: > > [1] We escape any IOs not related to our clock device (I mean > > "snps,i2s-pll-clock") itself. > > [2] We'll use whatever reference clock value is given. > > I.e. we'll be able to do a fix-up of that reference clock > > value early in platform code depending on HW we're running on. > > That's what people do here and there. > > [3] Remember another clock driver for AXS10x board is right around > > the corner. I mean the one for ARC PGU which uses exactly the same > > master clock. So one fixup as mentioned above will work > > at once for 2 clock drivers. > > > > Let me know if above makes sense. > That approach can't be used because the reference clock value will change in the > next firmware release. The new release will have a reference clock of 28224000 > Hz instead of the usual 27000000 Hz, so we need to have a way to distinguish > between them. Because of that we can't have only one master clock unless you > state to users that they have to change the reference clock value when using the > new firmware release. Stephen suggested to use two DT files (one for each > firmware release), but as Vineet said this would be annoying to the user so I am > trying to use another solution so that only one DT file is required. Ok reference clock will change. But I may guess we'll still be able to determine at least that new firmware version in run-time, right? If so we'll update a fix-up in early axs10x platform code so that reference clock will be set as 28224000 Hz. And indeed 2 DT files is a no go - we want to run the same one binary (with built-in .dtb) on all flavors of AXS boards. And fix-up I'm talking about will actually do transformation of .dtb early on kernel boot process so that will be a complete equivalent of different DT files. -Alexey
Hi Alexey, On 21-04-2016 13:18, Alexey Brodkin wrote: > Hi Jose, > > On Thu, 2016-04-21 at 10:51 +0100, Jose Abreu wrote: >> Hi Alexey, > >>>>> Otherwise, I still prefer two DTS files for the two different FPGA >>>>> versions. At the least, please use ioremap for any pointers that >>>>> you readl/writel here. >>>>> >>>>> Beyond that, we should have a fixed rate source clk somewhere in >>>>> the software view of the clk tree, because that reflects reality. >>>>> Hardcoding the parent rate in the structure works, but doesn't >>>>> properly express the clk tree. >>>>> >>>> Can I use a property in the DT to pass this reference clock? something like this: >>>> snps,parent-freq = <0xFBED9 27000000>, <0x0 28224000>; /* Tuple >>>> <fpga-version reference-clock-freq>, fpga-version = 0 is default */ >>>> >>>> Or use a parent clock? like: >>>> clk { >>>> compatible = "fixed-clock"; >>>> clock-frequency = <27000000>; >>>> #clock-cells = <0>; >>>> snps,fpga-version = <0xFBED9>; >>>> } >>>> >>>> It is important to distinguish between the different versions automatically, is >>>> any of these solutions ok? >>> I do like that solution with a master clock but with some fine-tuning >>> for simplification. >>> >>> We'll add master clock node for I2S as a fixed clock like that: >>> ------------------->8------------------ >>> i2s_master_clock: clk { >>> #clock-cells = <0>; >>> compatible = "fixed-clock"; >>> clock-frequency = <27000000>; >>> }; >>> ------------------->8------------------ >>> >>> Note there's no mention of MB version, just a value of the frequency. >>> And in the driver itself value of that master clock will be used for >>> population of "pll_clk->ref_clk" directly. >>> >>> These are benefits we'll get with that approach: >>> [1] We escape any IOs not related to our clock device (I mean >>> "snps,i2s-pll-clock") itself. >>> [2] We'll use whatever reference clock value is given. >>> I.e. we'll be able to do a fix-up of that reference clock >>> value early in platform code depending on HW we're running on. >>> That's what people do here and there. >>> [3] Remember another clock driver for AXS10x board is right around >>> the corner. I mean the one for ARC PGU which uses exactly the same >>> master clock. So one fixup as mentioned above will work >>> at once for 2 clock drivers. >>> >>> Let me know if above makes sense. >> That approach can't be used because the reference clock value will change in the >> next firmware release. The new release will have a reference clock of 28224000 >> Hz instead of the usual 27000000 Hz, so we need to have a way to distinguish >> between them. Because of that we can't have only one master clock unless you >> state to users that they have to change the reference clock value when using the >> new firmware release. Stephen suggested to use two DT files (one for each >> firmware release), but as Vineet said this would be annoying to the user so I am >> trying to use another solution so that only one DT file is required. > Ok reference clock will change. > But I may guess we'll still be able to determine at least that new > firmware version in run-time, right? If so we'll update a fix-up in > early axs10x platform code so that reference clock will be set as 28224000 Hz. Yes, there is a register where the FPGA version date is encoded, we can use that to check which firmware is used (if date <= old_firmware_date then clock=27000000; else clock=28224000). If that fix is acceptable it could be a good solution without having to use custom parameters in the DT (no need to encode the different clocks and we would only use one master clock) but I am not sure where and how this can be encoded and I don't know how to change the DT on runtime. Can you give me some guidelines? > > And indeed 2 DT files is a no go - we want to run the same one binary > (with built-in .dtb) on all flavors of AXS boards. And fix-up I'm talking about > will actually do transformation of .dtb early on kernel boot process so that will > be a complete equivalent of different DT files. And doing modifications on the DT can cause some misdirections to users. Besides, we would have clock specific functions in init procedures which is precisely what we are trying to avoid by submitting this driver. > > -Alexey Best regards, Jose Miguel Abreu
Hi Jose, On Thu, 2016-04-21 at 14:10 +0100, Jose Abreu wrote: > Hi Alexey, > > > On 21-04-2016 13:18, Alexey Brodkin wrote: > > > > Hi Jose, > > > > On Thu, 2016-04-21 at 10:51 +0100, Jose Abreu wrote: > > > > > > Hi Alexey, > > > > > Ok reference clock will change. > > But I may guess we'll still be able to determine at least that new > > firmware version in run-time, right? If so we'll update a fix-up in > > early axs10x platform code so that reference clock will be set as 28224000 Hz. > Yes, there is a register where the FPGA version date is encoded, we can use that > to check which firmware is used (if date <= old_firmware_date then > clock=27000000; else clock=28224000). If that fix is acceptable it could be a > good solution without having to use custom parameters in the DT (no need to > encode the different clocks and we would only use one master clock) but I am not > sure where and how this can be encoded and I don't know how to change the DT on > runtime. Can you give me some guidelines? Take a look here - http://git.kernel.org/cgit/linux/kernel/git/vgupta/arc.git/commit/arch/arc/plat-axs10x/axs10x.c?h=for -next&id=5cd0f5102753a7405548d0c66c11a2a0a05bbf2e We do something very similar here - we're patching in run-time core frequency that was specified in .dts. And in the very same way one will be able to do fix-ups for other clocks. Moreover I would propose to think about that fix-up as of completely separate topic. I.e. in your driver for AXS' I2S clock just use a new reference "fixed-clock" (that you'll add in "axs10x_mb.dtsi" as a part of your driver submission). And once your driver gets accepted we'll work on fix-up in axs10x platform. This way we'll move with smaller steps and hopefully will get things done sooner. > > And indeed 2 DT files is a no go - we want to run the same one binary > > (with built-in .dtb) on all flavors of AXS boards. And fix-up I'm talking about > > will actually do transformation of .dtb early on kernel boot process so that will > > be a complete equivalent of different DT files. > And doing modifications on the DT can cause some misdirections to users. What do you mean here? What kind of problems do you expect to face? > Besides, we would have clock specific functions in init procedures which is > precisely what we are trying to avoid by submitting this driver. You're talking about fixups above here? -Alexey
On Thursday 21 April 2016 05:48 PM, Alexey Brodkin wrote: > Hi Jose, > > On Thu, 2016-04-21 at 10:51 +0100, Jose Abreu wrote: >> Hi Alexey, > > >>>>> Otherwise, I still prefer two DTS files for the two different FPGA >>>>> versions. At the least, please use ioremap for any pointers that >>>>> you readl/writel here. >>>>> >>>>> Beyond that, we should have a fixed rate source clk somewhere in >>>>> the software view of the clk tree, because that reflects reality. >>>>> Hardcoding the parent rate in the structure works, but doesn't >>>>> properly express the clk tree. >>>>> >>>> Can I use a property in the DT to pass this reference clock? something like this: >>>> snps,parent-freq = <0xFBED9 27000000>, <0x0 28224000>; /* Tuple >>>> <fpga-version reference-clock-freq>, fpga-version = 0 is default */ >>>> >>>> Or use a parent clock? like: >>>> clk { >>>> compatible = "fixed-clock"; >>>> clock-frequency = <27000000>; >>>> #clock-cells = <0>; >>>> snps,fpga-version = <0xFBED9>; >>>> } >>>> >>>> It is important to distinguish between the different versions automatically, is >>>> any of these solutions ok? >>> I do like that solution with a master clock but with some fine-tuning >>> for simplification. >>> >>> We'll add master clock node for I2S as a fixed clock like that: >>> ------------------->8------------------ >>> i2s_master_clock: clk { >>> #clock-cells = <0>; >>> compatible = "fixed-clock"; >>> clock-frequency = <27000000>; >>> }; >>> ------------------->8------------------ >>> >>> Note there's no mention of MB version, just a value of the frequency. >>> And in the driver itself value of that master clock will be used for >>> population of "pll_clk->ref_clk" directly. >>> >>> These are benefits we'll get with that approach: >>> [1] We escape any IOs not related to our clock device (I mean >>> "snps,i2s-pll-clock") itself. >>> [2] We'll use whatever reference clock value is given. >>> I.e. we'll be able to do a fix-up of that reference clock >>> value early in platform code depending on HW we're running on. >>> That's what people do here and there. >>> [3] Remember another clock driver for AXS10x board is right around >>> the corner. I mean the one for ARC PGU which uses exactly the same >>> master clock. So one fixup as mentioned above will work >>> at once for 2 clock drivers. >>> >>> Let me know if above makes sense. >> That approach can't be used because the reference clock value will change in the >> next firmware release. The new release will have a reference clock of 28224000 >> Hz instead of the usual 27000000 Hz, so we need to have a way to distinguish >> between them. Because of that we can't have only one master clock unless you >> state to users that they have to change the reference clock value when using the >> new firmware release. Stephen suggested to use two DT files (one for each >> firmware release), but as Vineet said this would be annoying to the user so I am >> trying to use another solution so that only one DT file is required. > > Ok reference clock will change. > But I may guess we'll still be able to determine at least that new > firmware version in run-time, right? If so we'll update a fix-up in > early axs10x platform code so that reference clock will be set as 28224000 Hz. Please no - lets not bolt-in more hacks and instead try do this cleanly if possible. And from other discusions it seems there might be a way. The readl approach seems fine to me (with ioremap) if that is what it takes. > And indeed 2 DT files is a no go - we want to run the same one binary > (with built-in .dtb) on all flavors of AXS boards. Right - 2 DT is not acceptable unless we are feeling bored and want more emails for AXS board support :-) And fix-up I'm talking about > will actually do transformation of .dtb early on kernel boot process so that will > be a complete equivalent of different DT files. > > -Alexey-- > To unsubscribe from this list: send the line "unsubscribe linux-clk" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
diff --git a/Documentation/devicetree/bindings/clock/i2s-pll-clock.txt b/Documentation/devicetree/bindings/clock/i2s-pll-clock.txt new file mode 100644 index 0000000..ff86a41 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/i2s-pll-clock.txt @@ -0,0 +1,17 @@ +Binding for the AXS10X I2S PLL clock + +This binding uses the common clock binding[1]. + +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt + +Required properties: +- compatible: shall be "snps,i2s-pll-clock" +- #clock-cells: from common clock binding; Should always be set to 0. +- reg : Address and length of the I2S PLL register set. + +Example: + clock@0x100a0 { + compatible = "snps,i2s-pll-clock"; + reg = <0x100a0 0x10>; + #clock-cells = <0>; + }; diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index 46869d6..2ca62dc6 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_PLAT_AXS10X) += axs10x/ diff --git a/drivers/clk/axs10x/Makefile b/drivers/clk/axs10x/Makefile new file mode 100644 index 0000000..01996b8 --- /dev/null +++ b/drivers/clk/axs10x/Makefile @@ -0,0 +1 @@ +obj-y += i2s_pll_clock.o diff --git a/drivers/clk/axs10x/i2s_pll_clock.c b/drivers/clk/axs10x/i2s_pll_clock.c new file mode 100644 index 0000000..3ba4e2f --- /dev/null +++ b/drivers/clk/axs10x/i2s_pll_clock.c @@ -0,0 +1,217 @@ +/* + * Synopsys AXS10X SDP I2S PLL clock driver + * + * 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/platform_device.h> +#include <linux/module.h> +#include <linux/clk-provider.h> +#include <linux/err.h> +#include <linux/device.h> +#include <linux/of_address.h> +#include <linux/slab.h> +#include <linux/of.h> + +/* FPGA Version Info */ +#define FPGA_VER_INFO 0xE0011230 +#define FPGA_VER_27M 0x000FBED9 + +/* PLL registers addresses */ +#define PLL_IDIV_REG 0x0 +#define PLL_FBDIV_REG 0x4 +#define PLL_ODIV0_REG 0x8 +#define PLL_ODIV1_REG 0xC + +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 }, + { 2116800, 0x82, 0x3CF, 0x10C30, 0x2000 }, + { 2304000, 0x104, 0x79E, 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 }, + { 2116800, 0x514, 0x42, 0x10001, 0x2000 }, + { 2304000, 0x619, 0x82, 0x10001, 0x2000 }, + { 0, 0, 0, 0, 0 }, +}; + +struct i2s_pll_clk { + void __iomem *base; + struct clk_hw hw; + unsigned int ref_clk; + const struct i2s_pll_cfg *pll_cfg; +}; + +static inline void i2s_pll_write(struct i2s_pll_clk *clk, unsigned int reg, + unsigned int val) +{ + writel_relaxed(val, clk->base + reg); +} + +static inline unsigned int i2s_pll_read(struct i2s_pll_clk *clk, + unsigned int reg) +{ + return readl_relaxed(clk->base + reg); +} + +static inline struct i2s_pll_clk *to_i2s_pll_clk(struct clk_hw *hw) +{ + return container_of(hw, struct i2s_pll_clk, hw); +} + +static unsigned int i2s_pll_get_value(unsigned int val) +{ + return (val & 0x3F) + ((val >> 6) & 0x3F); +} + +static unsigned long i2s_pll_recalc_rate(struct clk_hw *hw, + unsigned long parent_rate) +{ + struct i2s_pll_clk *clk = to_i2s_pll_clk(hw); + unsigned int idiv, fbdiv, odiv; + + idiv = i2s_pll_get_value(i2s_pll_read(clk, PLL_IDIV_REG)); + fbdiv = i2s_pll_get_value(i2s_pll_read(clk, PLL_FBDIV_REG)); + odiv = i2s_pll_get_value(i2s_pll_read(clk, PLL_ODIV0_REG)); + + return ((clk->ref_clk / idiv) * fbdiv) / odiv; +} + +static long i2s_pll_round_rate(struct clk_hw *hw, unsigned long rate, + unsigned long *prate) +{ + const struct i2s_pll_cfg *pll_cfg = to_i2s_pll_clk(hw)->pll_cfg; + int i; + + for (i = 0; pll_cfg[i].rate != 0; i++) + if (pll_cfg[i].rate == rate) + return rate; + + return -EINVAL; +} + +static int i2s_pll_set_rate(struct clk_hw *hw, unsigned long rate, + unsigned long parent_rate) +{ + struct i2s_pll_clk *clk = to_i2s_pll_clk(hw); + const struct i2s_pll_cfg *pll_cfg = clk->pll_cfg; + int i; + + for (i = 0; pll_cfg[i].rate != 0; i++) { + if (pll_cfg[i].rate == rate) { + i2s_pll_write(clk, PLL_IDIV_REG, pll_cfg[i].idiv); + i2s_pll_write(clk, PLL_FBDIV_REG, pll_cfg[i].fbdiv); + i2s_pll_write(clk, PLL_ODIV0_REG, pll_cfg[i].odiv0); + i2s_pll_write(clk, PLL_ODIV1_REG, pll_cfg[i].odiv1); + return 0; + } + } + + pr_err("%s: invalid rate=%ld, parent_rate=%ld\n", __func__, + rate, parent_rate); + return -EINVAL; +} + +static const struct clk_ops i2s_pll_ops = { + .recalc_rate = i2s_pll_recalc_rate, + .round_rate = i2s_pll_round_rate, + .set_rate = i2s_pll_set_rate, +}; + +static int i2s_pll_clk_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct device_node *node = dev->of_node; + const char *clk_name; + struct clk *clk; + struct i2s_pll_clk *pll_clk; + struct clk_init_data init; + struct resource *mem; + + if (!node) + return -ENODEV; + + pll_clk = devm_kzalloc(dev, sizeof(*pll_clk), GFP_KERNEL); + if (!pll_clk) + return -ENOMEM; + + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); + pll_clk->base = devm_ioremap_resource(dev, mem); + if (IS_ERR(pll_clk->base)) + return PTR_ERR(pll_clk->base); + + clk_name = node->name; + init.name = clk_name; + init.ops = &i2s_pll_ops; + init.num_parents = 0; + pll_clk->hw.init = &init; + + clk = clk_register(NULL, &pll_clk->hw); + if (IS_ERR(clk)) { + dev_err(dev, "failed to register %s div clock (%ld)\n", + clk_name, PTR_ERR(clk)); + return PTR_ERR(clk); + } + + if (readl((void *)FPGA_VER_INFO) <= FPGA_VER_27M) { + pll_clk->ref_clk = 27000000; + pll_clk->pll_cfg = i2s_pll_cfg_27m; + } else { + pll_clk->ref_clk = 28224000; + pll_clk->pll_cfg = i2s_pll_cfg_28m; + } + + return of_clk_add_provider(node, of_clk_src_simple_get, clk); +} + +static int i2s_pll_clk_remove(struct platform_device *pdev) +{ + of_clk_del_provider(pdev->dev.of_node); + return 0; +} + +static const struct of_device_id i2s_pll_clk_id[] = { + { .compatible = "snps,i2s-pll-clock", }, + { }, +}; +MODULE_DEVICE_TABLE(of, i2s_pll_clk_id); + +static struct platform_driver i2s_pll_clk_driver = { + .driver = { + .name = "i2s-pll-clock", + .of_match_table = of_match_ptr(i2s_pll_clk_id), + }, + .probe = i2s_pll_clk_probe, + .remove = i2s_pll_clk_remove, +}; +module_platform_driver(i2s_pll_clk_driver); + +MODULE_AUTHOR("Jose Abreu <joabreu@synopsys.com>"); +MODULE_DESCRIPTION("Synopsys AXS10X SDP I2S PLL Clock Driver"); +MODULE_LICENSE("GPL v2");
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@synopsys.com> --- Changes v3 -> v4: * Added binding document (as suggested by Stephen Boyd) * Minor code style fixes (as suggested by Stephen Boyd) * Use ioremap (as suggested by Stephen Boyd) * Implement round_rate (as suggested by Stephen Boyd) * Change to platform driver (as suggested by Stephen Boyd) * Use {readl/writel}_relaxed (as suggested by Vineet Gupta) Changes v2 -> v3: * Implemented recalc_rate Changes v1 -> v2: * Renamed folder to axs10x (as suggested by Alexey Brodkin) * Added more supported rates .../devicetree/bindings/clock/i2s-pll-clock.txt | 17 ++ drivers/clk/Makefile | 1 + drivers/clk/axs10x/Makefile | 1 + drivers/clk/axs10x/i2s_pll_clock.c | 217 +++++++++++++++++++++ 4 files changed, 236 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/i2s-pll-clock.txt create mode 100644 drivers/clk/axs10x/Makefile create mode 100644 drivers/clk/axs10x/i2s_pll_clock.c