[PATCH 0/2] net: mv643xx_eth: use managed clk and allocation
With introduction of common clock framework and the ability to provide gated clocks, mv643xx_eth required calls to get and enable these clock gates on some platforms. Back then, common clock framework api wasn't safe for architectures without support for common clocks. This has changed now and there are also managed (devm_) counterparts for clock related functions. The second patch in this series, also converts kzalloc to devm_kzalloc where applicable. Both patches have been sent to the corresponding mailing lists as individual patches before. To get the order required to apply them right, this patch set combines both patches into one set. Sebastian Hesselbarth (2): net: mv643xx_eth: add shared clk and cleanup existing clk handling net: mv643xx_eth: use managed devm_kzalloc Documentation/devicetree/bindings/marvell.txt |3 ++ drivers/net/ethernet/marvell/mv643xx_eth.c| 44 + 2 files changed, 18 insertions(+), 29 deletions(-) --- Cc: Grant Likely grant.lik...@secretlab.ca Cc: Rob Herring rob.herr...@calxeda.com Cc: Rob Landley r...@landley.net Cc: Lennert Buytenhek buyt...@wantstofly.org Cc: Sebastian Hesselbarth sebastian.hesselba...@gmail.com Cc: Andrew Lunn and...@lunn.ch Cc: Jason Cooper ja...@lakedaemon.net Cc: Florian Fainelli flor...@openwrt.org Cc: Sergei Shtylyov sergei.shtyl...@cogentembedded.com Cc: Benjamin Herrenschmidt b...@kernel.crashing.org Cc: Paul Mackerras pau...@samba.org Cc: linuxppc-...@lists.ozlabs.org Cc: devicetree-discuss@lists.ozlabs.org Cc: linux-...@vger.kernel.org Cc: linux-ker...@vger.kernel.org Cc: net...@vger.kernel.org -- 1.7.10.4 ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH 1/2] net: mv643xx_eth: add shared clk and cleanup existing clk handling
This patch adds an optional shared block clock to avoid lockups on clock gated controllers. Besides the new clock, clock handling for existing clocks is cleaned up and moved to devm_clk_get. Device tree binding documentation is updated for the new clocks property. Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@gmail.com --- Cc: Grant Likely grant.lik...@secretlab.ca Cc: Rob Herring rob.herr...@calxeda.com Cc: Rob Landley r...@landley.net Cc: Lennert Buytenhek buyt...@wantstofly.org Cc: Sebastian Hesselbarth sebastian.hesselba...@gmail.com Cc: Andrew Lunn and...@lunn.ch Cc: Jason Cooper ja...@lakedaemon.net Cc: Florian Fainelli flor...@openwrt.org Cc: Sergei Shtylyov sergei.shtyl...@cogentembedded.com Cc: Benjamin Herrenschmidt b...@kernel.crashing.org Cc: Paul Mackerras pau...@samba.org Cc: linuxppc-...@lists.ozlabs.org Cc: devicetree-discuss@lists.ozlabs.org Cc: linux-...@vger.kernel.org Cc: linux-ker...@vger.kernel.org Cc: net...@vger.kernel.org --- Documentation/devicetree/bindings/marvell.txt |3 +++ drivers/net/ethernet/marvell/mv643xx_eth.c| 27 ++--- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/Documentation/devicetree/bindings/marvell.txt b/Documentation/devicetree/bindings/marvell.txt index f1533d9..f7a0da6 100644 --- a/Documentation/devicetree/bindings/marvell.txt +++ b/Documentation/devicetree/bindings/marvell.txt @@ -115,6 +115,9 @@ prefixed with the string marvell,, for Marvell Technology Group Ltd. - compatible : marvell,mv64360-eth-block - reg : Offset and length of the register set for this block + Optional properties: + - clocks : Phandle to the clock control device and gate bit + Example Discovery Ethernet block node: ethernet-block@2000 { #address-cells = 1; diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c index aedbd82..bbe6104 100644 --- a/drivers/net/ethernet/marvell/mv643xx_eth.c +++ b/drivers/net/ethernet/marvell/mv643xx_eth.c @@ -268,7 +268,7 @@ struct mv643xx_eth_shared_private { int extended_rx_coal_limit; int tx_bw_control; int tx_csum_limit; - + struct clk *clk; }; #define TX_BW_CONTROL_ABSENT 0 @@ -410,9 +410,7 @@ struct mv643xx_eth_private { /* * Hardware-specific parameters. */ -#if defined(CONFIG_HAVE_CLK) struct clk *clk; -#endif unsigned int t_clk; }; @@ -2569,6 +2567,10 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev) if (msp-base == NULL) goto out_free; + msp-clk = devm_clk_get(pdev-dev, NULL); + if (!IS_ERR(msp-clk)) + clk_prepare_enable(msp-clk); + /* * (Re-)program MBUS remapping windows if we are asked to. */ @@ -2595,6 +2597,8 @@ static int mv643xx_eth_shared_remove(struct platform_device *pdev) struct mv643xx_eth_shared_private *msp = platform_get_drvdata(pdev); iounmap(msp-base); + if (!IS_ERR(msp-clk)) + clk_disable_unprepare(msp-clk); kfree(msp); return 0; @@ -2801,13 +2805,12 @@ static int mv643xx_eth_probe(struct platform_device *pdev) * it to override the default. */ mp-t_clk = 13300; -#if defined(CONFIG_HAVE_CLK) - mp-clk = clk_get(pdev-dev, (pdev-id ? 1 : 0)); + mp-clk = devm_clk_get(pdev-dev, NULL); if (!IS_ERR(mp-clk)) { clk_prepare_enable(mp-clk); mp-t_clk = clk_get_rate(mp-clk); } -#endif + set_params(mp, pd); netif_set_real_num_tx_queues(dev, mp-txq_count); netif_set_real_num_rx_queues(dev, mp-rxq_count); @@ -2889,12 +2892,8 @@ static int mv643xx_eth_probe(struct platform_device *pdev) return 0; out: -#if defined(CONFIG_HAVE_CLK) - if (!IS_ERR(mp-clk)) { + if (!IS_ERR(mp-clk)) clk_disable_unprepare(mp-clk); - clk_put(mp-clk); - } -#endif free_netdev(dev); return err; @@ -2909,12 +2908,8 @@ static int mv643xx_eth_remove(struct platform_device *pdev) phy_detach(mp-phy); cancel_work_sync(mp-tx_timeout_task); -#if defined(CONFIG_HAVE_CLK) - if (!IS_ERR(mp-clk)) { + if (!IS_ERR(mp-clk)) clk_disable_unprepare(mp-clk); - clk_put(mp-clk); - } -#endif free_netdev(mp-dev); -- 1.7.10.4 ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH v2 2/2] net: mv643xx_eth: use managed devm_kzalloc
This patch moves shared private data kzalloc to managed devm_kzalloc and cleans now unneccessary kfree and error handling. Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@gmail.com --- Changes from v1: - replaced EADDRNOTAVAIL with ENOMEM on failing ioremap (Reported by Sergei Shtylyov) Cc: Grant Likely grant.lik...@secretlab.ca Cc: Rob Herring rob.herr...@calxeda.com Cc: Rob Landley r...@landley.net Cc: Lennert Buytenhek buyt...@wantstofly.org Cc: Sebastian Hesselbarth sebastian.hesselba...@gmail.com Cc: Andrew Lunn and...@lunn.ch Cc: Jason Cooper ja...@lakedaemon.net Cc: Florian Fainelli flor...@openwrt.org Cc: Sergei Shtylyov sergei.shtyl...@cogentembedded.com Cc: Benjamin Herrenschmidt b...@kernel.crashing.org Cc: Paul Mackerras pau...@samba.org Cc: linuxppc-...@lists.ozlabs.org Cc: devicetree-discuss@lists.ozlabs.org Cc: linux-...@vger.kernel.org Cc: linux-ker...@vger.kernel.org Cc: net...@vger.kernel.org --- drivers/net/ethernet/marvell/mv643xx_eth.c | 17 - 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c index bbe6104..305038f 100644 --- a/drivers/net/ethernet/marvell/mv643xx_eth.c +++ b/drivers/net/ethernet/marvell/mv643xx_eth.c @@ -2547,25 +2547,22 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev) struct mv643xx_eth_shared_private *msp; const struct mbus_dram_target_info *dram; struct resource *res; - int ret; if (!mv643xx_eth_version_printed++) pr_notice(MV-643xx 10/100/1000 ethernet driver version %s\n, mv643xx_eth_driver_version); - ret = -EINVAL; res = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (res == NULL) - goto out; + return -EINVAL; - ret = -ENOMEM; - msp = kzalloc(sizeof(*msp), GFP_KERNEL); + msp = devm_kzalloc(pdev-dev, sizeof(*msp), GFP_KERNEL); if (msp == NULL) - goto out; + return -ENOMEM; msp-base = ioremap(res-start, resource_size(res)); if (msp-base == NULL) - goto out_free; + return -ENOMEM; msp-clk = devm_clk_get(pdev-dev, NULL); if (!IS_ERR(msp-clk)) @@ -2585,11 +2582,6 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev) platform_set_drvdata(pdev, msp); return 0; - -out_free: - kfree(msp); -out: - return ret; } static int mv643xx_eth_shared_remove(struct platform_device *pdev) @@ -2599,7 +2591,6 @@ static int mv643xx_eth_shared_remove(struct platform_device *pdev) iounmap(msp-base); if (!IS_ERR(msp-clk)) clk_disable_unprepare(msp-clk); - kfree(msp); return 0; } -- 1.7.10.4 ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 0/5 v2] mv643xx_eth: device tree bindings
On Thu, Apr 11, 2013 at 6:53 PM, Jason Cooper ja...@lakedaemon.net wrote: On Thu, Apr 04, 2013 at 12:27:10PM +0200, Florian Fainelli wrote: This patch serie implements mv643xx_eth device tree bindings. I opted for the reuse of the bindings already defined in Documentation/devicetree/bindings/marvell.txt so that we do not create any confusion. How is the new version of this looking? Do you think you'll be able to get it up in time for DaveM to take it into v3.10? With this and Thomas' pci series, we will have Kirkwood fully converted to devicetree, can begin removing board files, and finally begin migrating everything over to mach-mvebu/. This will lead to the removal of five directories under arch/arm/ (plat-orion, mach-kirkwood, mach-orion5x, mach-dove, and mach-mv78xx0). Jason, I sent Florian an update of some of his patches earlier, based on cleanup patches I submitted for mv643xx_eth while testing his patch set. I think, the patch set will play nicely on ARM but without PPC guys involved in testing it, I guess it will break something there. Florian is reusing the DT bindings from PPC and they are compatible, but there parsing is also done within arch/ppc/sysdev/mv64360-dev.c and maybe somewhere else. The current DT patches for mv643xx_eth will reparse some properties again (and may break eth on ppc platforms using mv643xx_eth). Sebastian ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v7] clk: add si5351 i2c common clock driver
On Wed, Apr 10, 2013 at 4:48 PM, Michal Bachraty michal.bachr...@streamunlimited.com wrote: Hi Sebastian, This driver doesn't work for me. In my case, u-boot initializes si-5351 and power down unused clocks while booting kernel. there is need for power up clocks as was in previous versions of your driver. See patch, whre the problem is fixed: @@ -992,6 +992,10 @@ static long si5351_clkout_round_rate(struct clk_hw *hw, unsigned long rate, } while (1); } rate = *parent_rate rdiv; + + /* powerup clkout */ + si5351_set_bits(hwdata-drvdata, SI5351_CLK0_CTRL + hwdata-num, + SI5351_CLK_POWERDOWN, 0); dev_dbg(hwdata-drvdata-client-dev, %s - %s: rdiv = %u, parent_rate = %lu, rate = %lu\n, With this lines, driver works well. Hmm, is there any driver using the clock output? Does it clk_prepare_enable() the clock? I tend not to mess with anything the bootloader or eeprom config left disabled. It works for me, but here the driver will prepare/enable the clock prior use. Also, +==Example== + +/* 25MHz reference crystal */ +ref25: ref25M { + compatible = fixed-clock; + #clock-cells = 0; + clock-frequency = 2500; +}; + +i2c-master-node { + + /* Si5351a msop10 i2c clock generator */ + si5351a: clock-generator@60 { + compatible = silabs,si5351a-msop; + reg = 0x60; + #address-cells = 1; + #size-cells = 0; + #clock-cells = 1; + + /* connect xtal input to 25MHz reference */ + clocks = ref25; + + /* connect xtal input as source of pll0 and pll1 */ + silabs,pll-source = 0 0, 1 0; + + /* +* overwrite clkout0 configuration with: +* - 8mA output drive strength +* - pll0 as clock source of multisynth0 +* - multisynth0 as clock source of output divider +* - multisynth0 can change pll0 +* - set initial clock frequency of 74.25MHz +*/ + clkout0 { + reg = 0; + silabs,drive-strength = 8; + silabs,multisynth-source = 0; + silabs,clock-source = 0; + silabs,pll-master; + clock-frequency = 7425; + }; + + /* +* overwrite clkout1 configuration with: +* - 4mA output drive strength +* - pll1 as clock source of multisynth1 +* - multisynth1 as clock source of output divider +* - multisynth1 can change pll1 +*/ + clkout1 { + reg = 1; + silabs,drive-strength = 4; + silabs,multisynth-source = 1; + silabs,clock-source = 0; + pll-master; + }; + Is definition of pll-master in clkout1 correct? should not be silabs,pll- master ? Good catch. Sebastian ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v7] clk: add si5351 i2c common clock driver
Hooray Google! Thanks for removing plain text sending from gmail web-frontend.. *sigh* Sorry for resending this, but HTML mails get rejected by linux mailing lists. On 04/10/2013 04:48 PM, Michal Bachraty wrote: Hi Sebastian, This driver doesn't work for me. In my case, u-boot initializes si-5351 and power down unused clocks while booting kernel. there is need for power up clocks as was in previous versions of your driver. See patch, whre the problem is fixed: @@ -992,6 +992,10 @@ static long si5351_clkout_round_rate(struct clk_hw *hw, unsigned long rate, } while (1); } rate = *parent_rate rdiv; + + /* powerup clkout */ + si5351_set_bits(hwdata-drvdata, SI5351_CLK0_CTRL + hwdata-num, + SI5351_CLK_POWERDOWN, 0); dev_dbg(hwdata-drvdata-client-dev, %s - %s: rdiv = %u, parent_rate = %lu, rate = %lu\n, With this lines, driver works well. Hmm, is there any driver using the clock output? Does it clk_prepare_enable() the clock? I tend not to mess with anything the bootloader or eeprom config left disabled. It works for me, but here the driver will prepare/enable the clock prior use. Also, +==Example== + +/* 25MHz reference crystal */ +ref25: ref25M { + compatible = fixed-clock; + #clock-cells =0; + clock-frequency =2500; +}; + +i2c-master-node { + + /* Si5351a msop10 i2c clock generator */ + si5351a: clock-generator@60 { + compatible = silabs,si5351a-msop; + reg =0x60; + #address-cells =1; + #size-cells =0; + #clock-cells =1; + + /* connect xtal input to 25MHz reference */ + clocks =ref25; + + /* connect xtal input as source of pll0 and pll1 */ + silabs,pll-source =0 0,1 0; + + /* +* overwrite clkout0 configuration with: +* - 8mA output drive strength +* - pll0 as clock source of multisynth0 +* - multisynth0 as clock source of output divider +* - multisynth0 can change pll0 +* - set initial clock frequency of 74.25MHz +*/ + clkout0 { + reg =0; + silabs,drive-strength =8; + silabs,multisynth-source =0; + silabs,clock-source =0; + silabs,pll-master; + clock-frequency =7425; + }; + + /* +* overwrite clkout1 configuration with: +* - 4mA output drive strength +* - pll1 as clock source of multisynth1 +* - multisynth1 as clock source of output divider +* - multisynth1 can change pll1 +*/ + clkout1 { + reg =1; + silabs,drive-strength =4; + silabs,multisynth-source =1; + silabs,clock-source =0; + pll-master; + }; + Is definition of pll-master in clkout1 correct? should not be silabs,pll- master ? Good catch. Sebastian ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH] net: mvmdio: add clocks property to binding documentation
Patch net: mvmdio: get and enable optional clock was missing an update of the corresponding device tree binding documentation. This patch adds the clocks property to mvmdio binding documentation. Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@gmail.com --- Cc: Grant Likely grant.lik...@secretlab.ca Cc: Rob Herring rob.herr...@calxeda.com Cc: Rob Landley r...@landley.net Cc: David S. Miller da...@davemloft.net Cc: Florian Fainelli flor...@openwrt.org Cc: Thomas Petazzoni thomas.petazz...@free-electrons.com Cc: Sebastian Hesselbarth sebastian.hesselba...@gmail.com Cc: Greg Kroah-Hartman gre...@linuxfoundation.org Cc: net...@vger.kernel.org Cc: devicetree-discuss@lists.ozlabs.org Cc: linux-...@vger.kernel.org Cc: linux-ker...@vger.kernel.org --- Documentation/devicetree/bindings/net/marvell-orion-mdio.txt |1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt index 052b5f2..9417e54 100644 --- a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt +++ b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt @@ -11,6 +11,7 @@ Required properties: Optional properties: - interrupts: interrupt line number for the SMI error/done interrupt +- clocks: Phandle to the clock control device and gate bit The child nodes of the MDIO driver are the individual PHY devices connected to this MDIO bus. They must have a reg property given the -- 1.7.10.4 ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH] net: mv643xx_eth: add shared clk and cleanup existing clk handling
This patch adds an optional shared block clock to avoid lockups on clock gated controllers. Besides the new clock, clock handling for existing clocks is cleaned up and moved to devm_clk_get. Device tree binding documentation is updated for the new clocks property. Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@gmail.com --- Cc: Grant Likely grant.lik...@secretlab.ca Cc: Rob Herring rob.herr...@calxeda.com Cc: Rob Landley r...@landley.net Cc: Lennert Buytenhek buyt...@wantstofly.org Cc: Sebastian Hesselbarth sebastian.hesselba...@gmail.com Cc: Andrew Lunn and...@lunn.ch Cc: Jason Cooper ja...@lakedaemon.net Cc: Florian Fainelli flor...@openwrt.org Cc: devicetree-discuss@lists.ozlabs.org Cc: linux-...@vger.kernel.org Cc: linux-ker...@vger.kernel.org Cc: net...@vger.kernel.org --- Documentation/devicetree/bindings/marvell.txt |3 +++ drivers/net/ethernet/marvell/mv643xx_eth.c| 27 ++--- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/Documentation/devicetree/bindings/marvell.txt b/Documentation/devicetree/bindings/marvell.txt index f1533d9..f7a0da6 100644 --- a/Documentation/devicetree/bindings/marvell.txt +++ b/Documentation/devicetree/bindings/marvell.txt @@ -115,6 +115,9 @@ prefixed with the string marvell,, for Marvell Technology Group Ltd. - compatible : marvell,mv64360-eth-block - reg : Offset and length of the register set for this block + Optional properties: + - clocks : Phandle to the clock control device and gate bit + Example Discovery Ethernet block node: ethernet-block@2000 { #address-cells = 1; diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c index aedbd82..bbe6104 100644 --- a/drivers/net/ethernet/marvell/mv643xx_eth.c +++ b/drivers/net/ethernet/marvell/mv643xx_eth.c @@ -268,7 +268,7 @@ struct mv643xx_eth_shared_private { int extended_rx_coal_limit; int tx_bw_control; int tx_csum_limit; - + struct clk *clk; }; #define TX_BW_CONTROL_ABSENT 0 @@ -410,9 +410,7 @@ struct mv643xx_eth_private { /* * Hardware-specific parameters. */ -#if defined(CONFIG_HAVE_CLK) struct clk *clk; -#endif unsigned int t_clk; }; @@ -2569,6 +2567,10 @@ static int mv643xx_eth_shared_probe(struct platform_device *pdev) if (msp-base == NULL) goto out_free; + msp-clk = devm_clk_get(pdev-dev, NULL); + if (!IS_ERR(msp-clk)) + clk_prepare_enable(msp-clk); + /* * (Re-)program MBUS remapping windows if we are asked to. */ @@ -2595,6 +2597,8 @@ static int mv643xx_eth_shared_remove(struct platform_device *pdev) struct mv643xx_eth_shared_private *msp = platform_get_drvdata(pdev); iounmap(msp-base); + if (!IS_ERR(msp-clk)) + clk_disable_unprepare(msp-clk); kfree(msp); return 0; @@ -2801,13 +2805,12 @@ static int mv643xx_eth_probe(struct platform_device *pdev) * it to override the default. */ mp-t_clk = 13300; -#if defined(CONFIG_HAVE_CLK) - mp-clk = clk_get(pdev-dev, (pdev-id ? 1 : 0)); + mp-clk = devm_clk_get(pdev-dev, NULL); if (!IS_ERR(mp-clk)) { clk_prepare_enable(mp-clk); mp-t_clk = clk_get_rate(mp-clk); } -#endif + set_params(mp, pd); netif_set_real_num_tx_queues(dev, mp-txq_count); netif_set_real_num_rx_queues(dev, mp-rxq_count); @@ -2889,12 +2892,8 @@ static int mv643xx_eth_probe(struct platform_device *pdev) return 0; out: -#if defined(CONFIG_HAVE_CLK) - if (!IS_ERR(mp-clk)) { + if (!IS_ERR(mp-clk)) clk_disable_unprepare(mp-clk); - clk_put(mp-clk); - } -#endif free_netdev(dev); return err; @@ -2909,12 +2908,8 @@ static int mv643xx_eth_remove(struct platform_device *pdev) phy_detach(mp-phy); cancel_work_sync(mp-tx_timeout_task); -#if defined(CONFIG_HAVE_CLK) - if (!IS_ERR(mp-clk)) { + if (!IS_ERR(mp-clk)) clk_disable_unprepare(mp-clk); - clk_put(mp-clk); - } -#endif free_netdev(mp-dev); -- 1.7.10.4 ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] clk: si5351: Add gapless tuning for SI5351 PLL
On Tue, Apr 9, 2013 at 10:26 AM, Michal Bachraty michal.bachr...@streamunlimited.com wrote: For gapless tuning, there is no need for PLL reset and clkout power-down when tuning output. silabs,gapless-tuning parameter enables gapless tuning for specific clock output. Michal, does gapless tuning have any constraints? Can we use it for all rate changes or determine the need for pll reset by rate offset? Sebastian ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [v5] clk: add si5351 i2c common clock driver
On 04/08/2013 02:17 AM, Guenter Roeck wrote: On Mon, Apr 08, 2013 at 01:49:24AM +0200, Sebastian Hesselbarth wrote: On 04/08/2013 12:50 AM, Guenter Roeck wrote: On Fri, Apr 05, 2013 at 05:23:35AM -, Sebastian Hesselbarth wrote: This patch adds a common clock driver for Silicon Labs Si5351a/b/c i2c programmable clock generators. Currently, the driver supports DT kernels only and VXCO feature of si5351b is not implemented. DT bindings selectively allow to overwrite stored Si5351 configuration which is very helpful for clock generators with empty eeprom configuration. Corresponding device tree binding documentation is also added. Signed-off-by: Sebastian Hesselbarthsebastian.hesselba...@gmail.com Tested-by: Daniel Mackzon...@gmail.com [ ... ] +static inline void _si5351_msynth_set_pll_master( + struct si5351_driver_data *drvdata, unsigned char num, int is_master) +{ + unsigned long flags; + + if (num 8 || + (drvdata-variant == SI5351_VARIANT_A3 num 3)) + return; + + flags = __clk_get_flags(drvdata-msynth[num].hw.clk); + if (is_master) + flags |= CLK_SET_RATE_PARENT; + else + flags= ~CLK_SET_RATE_PARENT; + __clk_set_flags(drvdata-msynth[num].hw.clk, flags); +} + Unless I am missing something, neither __clk_get_flags() nor the new __clk_set_flags is exported. Did you try to build and load the driver as module ? Well, good catch. I didn't try to build v5 as a module, but I guess it will fail. But I consider this as something that has to be addressed in clk framework itself, not in this patch. There will be other clk-providers built as module in the future for sure. Sure, but you provided the patch to make __clk_set_flags global. To avoid build failures, I would suggest to either submit a patch to export the missing functions, or to remove the ability to build the driver as module. Actually, I knew that __clk_set_flags patch will not be accepted before posting it ;) On a side note, do you happen to know anyone working on drivers for Si5319 or Si5368 ? No. Thanks, Guenter ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [v5] clk: add si5351 i2c common clock driver
On Mon, Apr 8, 2013 at 4:54 PM, Guenter Roeck li...@roeck-us.net wrote: On Mon, Apr 08, 2013 at 08:11:38AM +0200, Sebastian Hesselbarth wrote: On 04/08/2013 02:17 AM, Guenter Roeck wrote: On Mon, Apr 08, 2013 at 01:49:24AM +0200, Sebastian Hesselbarth wrote: On 04/08/2013 12:50 AM, Guenter Roeck wrote: On Fri, Apr 05, 2013 at 05:23:35AM -, Sebastian Hesselbarth wrote: This patch adds a common clock driver for Silicon Labs Si5351a/b/c i2c programmable clock generators. Currently, the driver supports DT kernels only and VXCO feature of si5351b is not implemented. DT bindings selectively allow to overwrite stored Si5351 configuration which is very helpful for clock generators with empty eeprom configuration. Corresponding device tree binding documentation is also added. Signed-off-by: Sebastian Hesselbarthsebastian.hesselba...@gmail.com Tested-by: Daniel Mackzon...@gmail.com [ ... ] +static inline void _si5351_msynth_set_pll_master( + struct si5351_driver_data *drvdata, unsigned char num, int is_master) +{ + unsigned long flags; + + if (num 8 || + (drvdata-variant == SI5351_VARIANT_A3 num 3)) + return; + + flags = __clk_get_flags(drvdata-msynth[num].hw.clk); + if (is_master) + flags |= CLK_SET_RATE_PARENT; + else + flags= ~CLK_SET_RATE_PARENT; + __clk_set_flags(drvdata-msynth[num].hw.clk, flags); +} + Unless I am missing something, neither __clk_get_flags() nor the new __clk_set_flags is exported. Did you try to build and load the driver as module ? Well, good catch. I didn't try to build v5 as a module, but I guess it will fail. But I consider this as something that has to be addressed in clk framework itself, not in this patch. There will be other clk-providers built as module in the future for sure. Sure, but you provided the patch to make __clk_set_flags global. To avoid build failures, I would suggest to either submit a patch to export the missing functions, or to remove the ability to build the driver as module. Actually, I knew that __clk_set_flags patch will not be accepted before posting it ;) Ah, but part of that is to get you to think about it again, and to defend it if it is really needed. After all, it can be abused applies to pretty much every API. Guenter, I already thought about it a lot and I consider clk api broken in a way here. Key question is if you _really_ need run-time flag modifications, or if you can live with initialization-time settings. If you really need it, you'll have to explain the reasons. The question is not if _I_ really need run-time flags but why the api allows to perform run-time modifications of the clock hierarchy without allowing different flags? There is clk_set_parent() so I guess clk api knows about run-time changes already, but you cannot have different flags per parent. And with __clk_set_flags() rejected, you are not allowed to change the flags. I understand that some flags are permanent and required at registration, but CLK_SET_PARENT_RATE is not. It is not limited by hardware but limited by api. One way would be a more generic clk-mux with per parent flags, but for the current implementation, I cannot see how clk-mux can be exploited here. I can live with it, because then dynamic muxing of clock hierarchy within clk-si5351 is just not supported or will break function. Currently, there is no support for dynamic muxing, so everything is fine. On a side note, do you happen to know anyone working on drivers for Si5319 or Si5368 ? No. Too bad ... I may have to write that code myself then. Well, if clk-si5351 will ever get in mainline kernel, feel free to use it as a template ;) Sebastian ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v6] clk: add si5351 i2c common clock driver
On 04/08/2013 07:46 PM, Guenter Roeck wrote: On Mon, Apr 08, 2013 at 06:46:57PM +0200, Sebastian Hesselbarth wrote: This patch adds a common clock driver for Silicon Labs Si5351a/b/c i2c programmable clock generators. Currently, the driver does not support VXCO feature of si5351b. Passing platform_data or DT bindings selectively allow to overwrite stored Si5351 configuration which is very helpful for clock generators with empty eeprom configuration. Corresponding device tree binding documentation is also added. Signed-off-by: Sebastian Hesselbarthsebastian.hesselba...@gmail.com [ ... ] + +/* + * Si5351 i2c probe and DT + */ +#ifdef CONFIG_OF +static const struct of_device_id si5351_dt_ids[] = { + { .compatible = silabs,si5351a, .data = (void *)SI5351_VARIANT_A, }, + { .compatible = silabs,si5351a-msop, +.data = (void *)SI5351_VARIANT_A3, }, + { .compatible = silabs,si5351b, .data = (void *)SI5351_VARIANT_B, }, + { .compatible = silabs,si5351c, .data = (void *)SI5351_VARIANT_C, }, + { } +}; +MODULE_DEVICE_TABLE(of, si5351_dt_ids); + +static int si5351_dt_parse(struct i2c_client *client) +{ + struct device_node *child, *np = client-dev.of_node; + struct si5351_platform_data *pdata; + const struct of_device_id *match; + struct property *prop; + const __be32 *p; + int num = 0; + u32 val; + + if (np == NULL) + return 0; + + match = of_match_node(si5351_dt_ids, np); + if (match == NULL) + return -EINVAL; + + pdata = devm_kzalloc(client-dev, sizeof(*pdata), GFP_KERNEL); + if (!pdata) + return -ENOMEM; + + pdata-variant = (enum si5351_variant)match-data; + pdata-clk_xtal = of_clk_get(np, 0); + if (!IS_ERR(pdata-clk_xtal)) + clk_put(pdata-clk_xtal); + pdata-clk_clkin = of_clk_get(np, 1); + if (!IS_ERR(pdata-clk_clkin)) + clk_put(pdata-clk_clkin); + + /* +* property silabs,pll-source :num src, [..] +* allow to selectively set pll source +*/ + of_property_for_each_u32(np, silabs,pll-source, prop, p, num) { + if (num= 2) { + dev_err(client-dev, + invalid pll %d on pll-source prop\n, num); + break; You report dev_err here, but you don't return an error to the caller. Is this on purpose ? If it is not a fatal error, maybe it should be dev_info ? This shouldn't break but continue with one u32 skipped. Actually, it is a warning because somebody passed an invalid value and should be dev_warn(). But see my note below, I will make them all fatal. + } + + p = of_prop_next_u32(prop, p,val); + if (!p) + break; + + switch (val) { + case 0: + pdata-pll_src[num] = SI5351_PLL_SRC_XTAL; + break; + case 1: + pdata-pll_src[num] = SI5351_PLL_SRC_CLKIN; + break; + default: + dev_warn(client-dev, +invalid parent %d for pll %d\n, val, num); + continue; Same here, and a couple of times below. Given the context, I think it would be better if the error cases would return an error. After all, the condition suggests that the device tree data is wrong, meaning one has to assume it won't work anyway and should be fixed in the device tree and not be ignored by the driver. I am skipping invalid DT data enties here, but I can make them all fatal. I will also add more variant checks in the corresponding _si5351_* functions. + } + } + + /* per clkout properties */ + num = of_get_child_count(np); + + if (num == 0) + return 0; + This doesn't set client-dev.platform_data yet returns no error, meaning the calling code will either use provided platform data or fail after all if it is NULL. That seems to be inconsistent, given that a dt entry was already detected. The user might end up wondering why the provided device tree data is not used, not realizing that it is incomplete. If children are not mandatory, you could just drop the code above and go through for_each_child_of_node() anyway; it won't do anything and set client-dev.platform_data at the end. If children are mandatory, it might make sense to return an eror here (if there is dt information, it should be complete and consistent). Not having children is valid as you only provide them if you want to overwrite the current config stored in the eeprom. But yes, skipping for_each_child_of_node below is a left-over from an implementation where I used dynamically allocated -pll/-clkout. + for_each_child_of_node(np, child) { + if (of_property_read_u32(child, reg,num
Re: [v5] clk: add si5351 i2c common clock driver
On 04/08/2013 07:36 PM, Mike Turquette wrote: Quoting Sebastian Hesselbarth (2013-04-08 08:38:44) On Mon, Apr 8, 2013 at 4:54 PM, Guenter Roeckli...@roeck-us.net wrote: On Mon, Apr 08, 2013 at 08:11:38AM +0200, Sebastian Hesselbarth wrote: On 04/08/2013 02:17 AM, Guenter Roeck wrote: On Mon, Apr 08, 2013 at 01:49:24AM +0200, Sebastian Hesselbarth wrote: On 04/08/2013 12:50 AM, Guenter Roeck wrote: On Fri, Apr 05, 2013 at 05:23:35AM -, Sebastian Hesselbarth wrote: This patch adds a common clock driver for Silicon Labs Si5351a/b/c i2c programmable clock generators. Currently, the driver supports DT kernels only and VXCO feature of si5351b is not implemented. DT bindings selectively allow to overwrite stored Si5351 configuration which is very helpful for clock generators with empty eeprom configuration. Corresponding device tree binding documentation is also added. Signed-off-by: Sebastian Hesselbarthsebastian.hesselba...@gmail.com Tested-by: Daniel Mackzon...@gmail.com [ ... ] +static inline void _si5351_msynth_set_pll_master( + struct si5351_driver_data *drvdata, unsigned char num, int is_master) +{ + unsigned long flags; + + if (num8 || + (drvdata-variant == SI5351_VARIANT_A3num3)) + return; + + flags = __clk_get_flags(drvdata-msynth[num].hw.clk); + if (is_master) + flags |= CLK_SET_RATE_PARENT; + else + flags= ~CLK_SET_RATE_PARENT; + __clk_set_flags(drvdata-msynth[num].hw.clk, flags); +} + Unless I am missing something, neither __clk_get_flags() nor the new __clk_set_flags is exported. Did you try to build and load the driver as module ? Well, good catch. I didn't try to build v5 as a module, but I guess it will fail. But I consider this as something that has to be addressed in clk framework itself, not in this patch. There will be other clk-providers built as module in the future for sure. Sure, but you provided the patch to make __clk_set_flags global. To avoid build failures, I would suggest to either submit a patch to export the missing functions, or to remove the ability to build the driver as module. Actually, I knew that __clk_set_flags patch will not be accepted before posting it ;) Ah, but part of that is to get you to think about it again, and to defend it if it is really needed. After all, it can be abused applies to pretty much every API. Guenter, I already thought about it a lot and I consider clk api broken in a way here. Key question is if you _really_ need run-time flag modifications, or if you can live with initialization-time settings. If you really need it, you'll have to explain the reasons. The question is not if _I_ really need run-time flags but why the api allows to perform run-time modifications of the clock hierarchy without allowing different flags? There is clk_set_parent() so I guess clk api knows about run-time changes already, but you cannot have different flags per parent. And with __clk_set_flags() rejected, you are not allowed to change the flags. I understand that some flags are permanent and required at registration, but CLK_SET_PARENT_RATE is not. It is not limited by hardware but limited by api. One way would be a more generic clk-mux with per parent flags, but for the current implementation, I cannot see how clk-mux can be exploited here. There are a couple of ways to get past this issue. One is removal of some of the flags. I have never liked CLK_SET_RATE_PARENT, since I think the ability to propagate a rate-change request up to the parent should be enabled for all clocks. This is in the interest of the driver author who does not care to know intimate details of the clock hierarchy. When I developed the rate-change propagation code I was likely too hasty in defining a per-clock flag for that behavior. It might have been enough to simply compare thebest_parent_rate value from .round_rate and compare it against clk-parent-rate. This means that no flag is necessary. This assumes that the .round_rate implementation has enough knowledge to know whether or not to propagate the rate-change request up to the parent. This may not not true for the common divider type. I'll make some tests on removing this flag (and potentially other flags) to see how painful it is. Mike, another good thing would be to have implementation independent building blocks for clk drivers. I think clk-fixed-rate, clk-gate, clk-mux and some clk-synth should be enough. The current core clk templates in the api are almost all special implementations. Sebastian ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH] net: of_mdio: scan mdiobus for PHYs without reg property
Using DT for mdiobus and ethernet-phy requires to know the PHY address, which is hard to guess if you don't know it. This patch extends of_mdiobus_register to scan mdiobus for PHYs if reg property of the corresponding node is not set. This also allows to have phy nodes in SoC DT files where the reg property can be overwritten in the board file later. To encourage people to finally set the actual phy address, the mdiobus scan is noisier than required. Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@gmail.com --- Cc: Grant Likely grant.lik...@secretlab.ca Cc: Rob Herring rob.herr...@calxeda.com Cc: Florian Fainelli flor...@openwrt.org Cc: devicetree-discuss@lists.ozlabs.org Cc: net...@vger.kernel.org Cc: linux-ker...@vger.kernel.org --- drivers/of/of_mdio.c | 64 +- 1 file changed, 58 insertions(+), 6 deletions(-) diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c index e3a8b22..23049ae 100644 --- a/drivers/of/of_mdio.c +++ b/drivers/of/of_mdio.c @@ -34,7 +34,10 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np) { struct phy_device *phy; struct device_node *child; - int rc, i; + const __be32 *paddr; + u32 addr; + bool is_c45, scanphys = false; + int rc, i, len; /* Mask out all PHYs from auto probing. Instead the PHYs listed in * the device tree are populated after the bus has been registered */ @@ -54,14 +57,10 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np) /* Loop over the child nodes and register a phy_device for each one */ for_each_available_child_of_node(np, child) { - const __be32 *paddr; - u32 addr; - int len; - bool is_c45; - /* A PHY must have a reg property in the range [0-31] */ paddr = of_get_property(child, reg, len); if (!paddr || len sizeof(*paddr)) { + scanphys = true; dev_err(mdio-dev, %s has invalid PHY address\n, child-full_name); continue; @@ -111,6 +110,59 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np) child-name, addr); } + if (!scanphys) + return 0; + + /* auto scan for PHYs with empty reg property */ + for_each_available_child_of_node(np, child) { + /* Skip PHYs with reg property set */ + paddr = of_get_property(child, reg, len); + if (paddr) + continue; + + is_c45 = of_device_is_compatible(child, +ethernet-phy-ieee802.3-c45); + + for (addr = 0; addr PHY_MAX_ADDR; addr++) { + /* skip already registered PHYs */ + if (mdio-phy_map[addr]) + continue; + + /* be noisy to encourage people to set reg property */ + dev_info(mdio-dev, scan phy %s at address %i\n, +child-name, addr); + + phy = get_phy_device(mdio, addr, is_c45); + if (!phy || IS_ERR(phy)) + continue; + + if (mdio-irq) { + mdio-irq[addr] = + irq_of_parse_and_map(child, 0); + if (!mdio-irq[addr]) + mdio-irq[addr] = PHY_POLL; + } + + /* Associate the OF node with the device structure so it +* can be looked up later */ + of_node_get(child); + phy-dev.of_node = child; + + /* All data is now stored in the phy struct; +* register it */ + rc = phy_device_register(phy); + if (rc) { + phy_device_free(phy); + of_node_put(child); + continue; + } + + dev_info(mdio-dev, registered phy %s at address %i\n, +child-name, addr); + break; + } + } + return 0; } EXPORT_SYMBOL(of_mdiobus_register); -- 1.7.10.4 ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [v5] clk: add si5351 i2c common clock driver
On 04/08/2013 12:50 AM, Guenter Roeck wrote: On Fri, Apr 05, 2013 at 05:23:35AM -, Sebastian Hesselbarth wrote: This patch adds a common clock driver for Silicon Labs Si5351a/b/c i2c programmable clock generators. Currently, the driver supports DT kernels only and VXCO feature of si5351b is not implemented. DT bindings selectively allow to overwrite stored Si5351 configuration which is very helpful for clock generators with empty eeprom configuration. Corresponding device tree binding documentation is also added. Signed-off-by: Sebastian Hesselbarthsebastian.hesselba...@gmail.com Tested-by: Daniel Mackzon...@gmail.com [ ... ] +static inline void _si5351_msynth_set_pll_master( + struct si5351_driver_data *drvdata, unsigned char num, int is_master) +{ + unsigned long flags; + + if (num 8 || + (drvdata-variant == SI5351_VARIANT_A3 num 3)) + return; + + flags = __clk_get_flags(drvdata-msynth[num].hw.clk); + if (is_master) + flags |= CLK_SET_RATE_PARENT; + else + flags= ~CLK_SET_RATE_PARENT; + __clk_set_flags(drvdata-msynth[num].hw.clk, flags); +} + Unless I am missing something, neither __clk_get_flags() nor the new __clk_set_flags is exported. Did you try to build and load the driver as module ? Well, good catch. I didn't try to build v5 as a module, but I guess it will fail. But I consider this as something that has to be addressed in clk framework itself, not in this patch. There will be other clk-providers built as module in the future for sure. Sebastian ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH 1/5 v2] mv643xx_eth: add Device Tree bindings
On Fri, Apr 5, 2013 at 11:56 AM, Florian Fainelli flor...@openwrt.org wrote: [snip] Florian, took me a while to try you patches out on Dove but now I fixed all issues. I will comment on all related patches but first I want to comment here. One general note for Dove related patches: You didn't remove the registration of ge platform_device from mach-dove/board-dt.c. That will lead to double registration of mdio and mv643xx_eth/shared, so you'll never be sure if DT or non-DT code is executed. I haven't checked mach-kirkwood/board-dt.c or orion5x code. if (!mv643xx_eth_version_printed++) pr_notice(MV-643xx 10/100/1000 ethernet driver version %s\n, This is not related to your change, but there is a problem in this function that has already been discussed in the past if I remember correctly: The respective clock needs to be enabled here (at least on Kirkwood), since accesses to the hardware are done below. Enabling the clock only in mv643xx_eth_probe() is too late. As said, this is not a problem introduced by your changes (and which is currently circumvented by enabling the respective clocks in kirkwood_legacy_clk_init() and kirkwood_ge0x_init()), but we might want to fix this now to get rid of unconditionally enabling the GE clocks in the DT case. I think there may have been some confusion between the ethernet-group clock and the actual Ethernet port inside the ethernet-group. The mv643xx_eth driver assumes we have a per-port clock gating scheme, while I think we have a per ethernet-group clock gating scheme instead. Like you said, I think this should be addressed separately. IMHO, there should be a clocks property where ever you try to access registers, i.e. in all three parts mv643xx_eth_shared (group), mv643xx_eth (port) and mdio. Since port depends on shared it would be ok to have it per group but that may collide with other SoCs than Dove/Kirkwood that have per port clocks. Is that separation (group/port) really required for any SoC? [snip] You don't change the clk initialization here: #if defined(CONFIG_HAVE_CLK) mp-clk = clk_get(pdev-dev, (pdev-id ? 1 : 0)); if (!IS_ERR(mp-clk)) { clk_prepare_enable(mp-clk); mp-t_clk = clk_get_rate(mp-clk); } #endif Which, if I understand correctly, works in the DT case because you assign clock-names to the clocks in the DTS. However, I wonder whether this works for any but the first Ethernet device. Yes, it does. Assigned clocks from clocks property get a clock alias for that device name (node name). Using anything else than NULL here is IMHO just wrong. We should rather provide proper clock aliases for non-DT case. In the old platform device setup, the pdev-id was set when initialiazing the platform_device structure in common.c. Where is this done in the DT case? Looks like you are right, in the DT case, I assume that we should lookup the clock using NULL instead of 1 or 0 so we match any clock instead of a specific one. Yes. [snip] In phy_scan(), the phy is searched like this: snprintf(phy_id, sizeof(phy_id), PHY_ID_FMT, orion-mdio-mii, addr); phydev = phy_connect(mp-dev, phy_id, mv643xx_eth_adjust_link, PHY_INTERFACE_MODE_GMII); But orion-mdio-mii:xx is the name of the PHY if MDIO is setup via a platform_device. I could not get this to work if the MDIO device is setup via DT. Am I doing something wrong? I just missed updating this part of the code to probe for PHYs. The board I tested with uses a PHY_NONE configuration. I will add the missing bits for of_phy_connect() to be called here. I don't think that the ethernet controller should probe the PHY's on mdio-bus at all. At least not for DT enabled platforms. I had a look at DT and non-DT mdio-bus sources, and realized that there is a bus scan for non-DT only. of_mdiobus_register requires you to set (and know) the PHY address. I prepared a patch for of_mdio_register that will allow you to probe mdio and assign phy addresses to each node found. Currently, the heuristic for probing is: assign each phy node the next probed phy_addr starting with 0. But that will not allow to e.g. set some PHY addresses and probe the rest. We had a similar discussion whether to probe or not for DT nodes, and I guess there also will be some discussion about the above patch. OTOH we could just (again) ask users of every kirkwood/orion5x/dove board to tell their phy addresses and fail to probe the phy for new boards... I will prepare a proper patch soon and post it on the corresponding lists. Additionally, in phy_scan() there is this: if (phy_addr == MV643XX_ETH_PHY_ADDR_DEFAULT) { start = phy_addr_get(mp) 0x1f; num = 32; } else { ... MV643XX_ETH_PHY_ADDR_DEFAULT is defined as 0. However, many Kirkwood devices use
Re: [PATCH 1/5 v2] mv643xx_eth: add Device Tree bindings
On 04/05/2013 08:04 PM, Jason Gunthorpe wrote: On Fri, Apr 05, 2013 at 03:58:03PM +0200, Sebastian Hesselbarth wrote: I don't think that the ethernet controller should probe the PHY's on mdio-bus at all. At least not for DT enabled platforms. I had a look at DT and non-DT mdio-bus sources, and realized that there is a bus scan for non-DT only. of_mdiobus_register requires you to set (and know) the PHY address. DT platforms should have the option to use the standard phy-phandle connection: mdio@72004 { #address-cells =1; #size-cells =0; compatible = marvell,orion-mdio; reg =0x72004 0x84; status = disabled; +PHY1: ethernet-phy@1 { +reg =1; +device_type = ethernet-phy; +}; }; ethernet-group@72000 { #address-cells =1; #size-cells =0; compatible = marvell,mv643xx-eth-block; reg =0x72000 0x4000; tx-csum-limit =1600; status = disabled; egiga0: ethernet@0 { device_type = network; compatible = marvell,mv643xx-eth; reg =0; interrupts =29; clocks =gate_clk 2; + phy-handle =PHY1; }; }; When phy-handle is present the ethernet driver should not probe/scan for phys. There is standard code to handle all of this - an important gain is that the phy driver now has access to a DT node and can apply phy-specific properties. The above is what I use as a modification of Florian's patches. I compared of_mdiobus_register against mdiobus_register. The difference is that DT version does not scan the bus for attached PHYs. That is ok, if you know the phy_address of the PHY that you pass the handle of. But in most cases there will be only one PHY on the mdiobus and especially for new boards probing will help here. We had a similar discussion whether to probe or not for DT nodes, and I guess there also will be some discussion about the above patch. OTOH we could just (again) ask users of every kirkwood/orion5x/dove board to tell their phy addresses and fail to probe the phy for new boards... Maybe print a warning and call the no-DT phy probe code if phy-handle is nor present? I think it would be best to spam each probing result during mdiobus scan to encourage people to edit the DT and put in the phy_addr directly. IMHO it will be best to have bus scan on mdiobus rather than ethernet driver. Not sure this should be in the common code, phy probing is sketchy, it shouldn't be encouraged, IMHO.. Actually, it is in common code (non-DT case) and I think passing the phy_addr by DT node like above is best. But for boards you don't know the address (yet) probing helps a lot. If all phy nodes have their reg property set, no probing will be performed. For testing mdio bus probe, I used mdio { ... ethphy: ethernet-phy { reg = 32; }; }; And 32 should be defined as OF_PHY_ADDR_AUTOSCAN. It is an invalid address as max phy_addr is 31. I also thought about a bool property but passing an invalid reg property in SoC file allows to overwrite it in board file with the actual address easily. And AFAIK bool properties cannot be removed later on by board specific nodes. Sebastian ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH v5] clk: add si5351 i2c common clock driver
This patch adds a common clock driver for Silicon Labs Si5351a/b/c i2c programmable clock generators. Currently, the driver supports DT kernels only and VXCO feature of si5351b is not implemented. DT bindings selectively allow to overwrite stored Si5351 configuration which is very helpful for clock generators with empty eeprom configuration. Corresponding device tree binding documentation is also added. Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@gmail.com Tested-by: Daniel Mack zon...@gmail.com --- Changes from v4: - move from clk-private.h to clk-provider.h (Reported by Mike Turquette) - use __clk_set_flags() helper Changes from v3: - add silabs prefix to custom DT properties (Reported by Lars-Peter Clausen) - use sizeof(*foo) instead of sizeof(struct bar) (Reported by Lars-Peter Clausen) - check return value of of_clk_add_provider (Reported by Lars-Peter Clausen) - clean up i2c client init (Reported by Lars-Peter Clausen) - silence successful probe (Suggested by Lars-Peter Clausen) - make CONFIG_CLK_SI5351 depend on CONFIG_OF Changes from v2: - add curly brackets to if-else-statements (Reported by Daniel Mack) - fix div-by-zero for clk6/clk7 (Reported by Daniel Mack) - fix parameter address calculation for clk6/clk7 Changes from v1: - remove .is_enabled functions as they read from i2c (Reported by Daniel Mack) - add CLK_SET_RATE_PARENT on clkout reparent if clkout uses its own multisync Cc: Grant Likely grant.lik...@secretlab.ca Cc: Rob Herring rob.herr...@calxeda.com Cc: Rob Landley r...@landley.net Cc: Mike Turquette mturque...@linaro.org Cc: Stephen Warren swar...@nvidia.com Cc: Thierry Reding thierry.red...@avionic-design.de Cc: Dom Cobley popcorn...@gmail.com Cc: Linus Walleij linus.wall...@linaro.org Cc: Arnd Bergmann a...@arndb.de Cc: Andrew Morton a...@linux-foundation.org Cc: Pawel Moll pawel.m...@arm.com Cc: Mark Brown broo...@opensource.wolfsonmicro.com Cc: Russell King - ARM Linux li...@arm.linux.org.uk Cc: Rabeeh Khoury rab...@solid-run.com Cc: Daniel Mack zon...@gmail.com Cc: Jean-Francois Moine moin...@free.fr Cc: Lars-Peter Clausen l...@metafoo.de Cc: devicetree-discuss@lists.ozlabs.org Cc: linux-...@vger.kernel.org Cc: linux-ker...@vger.kernel.org Cc: linux-arm-ker...@lists.infradead.org --- .../devicetree/bindings/clock/silabs,si5351.txt| 114 ++ .../devicetree/bindings/vendor-prefixes.txt|1 + drivers/clk/Kconfig| 10 + drivers/clk/Makefile |1 + drivers/clk/clk-si5351.c | 1429 drivers/clk/clk-si5351.h | 155 +++ 6 files changed, 1710 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/silabs,si5351.txt create mode 100644 drivers/clk/clk-si5351.c create mode 100644 drivers/clk/clk-si5351.h diff --git a/Documentation/devicetree/bindings/clock/silabs,si5351.txt b/Documentation/devicetree/bindings/clock/silabs,si5351.txt new file mode 100644 index 000..cc37465 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/silabs,si5351.txt @@ -0,0 +1,114 @@ +Binding for Silicon Labs Si5351a/b/c programmable i2c clock generator. + +Reference +[1] Si5351A/B/C Data Sheet +http://www.silabs.com/Support%20Documents/TechnicalDocs/Si5351.pdf + +The Si5351a/b/c are programmable i2c clock generators with upto 8 output +clocks. Si5351a also has a reduced pin-count package (MSOP10) where only +3 output clocks are accessible. The internal structure of the clock +generators can be found in [1]. + +==I2C device node== + +Required properties: +- compatible: shall be one of silabs,si5351{a,a-msop,b,c}. +- reg: i2c device address, shall be 0x60 or 0x61. +- #clock-cells: from common clock binding; shall be set to 1. +- clocks: from common clock binding; list of parent clock + handles, shall be xtal reference clock or xtal and clkin for + si5351c only. +- #address-cells: shall be set to 1. +- #size-cells: shall be set to 0. + +Optional properties: +- silabs,pll-source: pair of (number, source) for each pll. Allows + to overwrite clock source of pll A (number=0) or B (number=1). + +==Child nodes== + +Each of the clock outputs can be overwritten individually by +using a child node to the I2C device node. If a child node for a clock +output is not set, the eeprom configuration is not overwritten. + +Required child node properties: +- reg: number of clock output. + +Optional child node properties: +- silabs,clock-source: source clock of the output divider stage N, shall be + 0 = multisynth N + 1 = multisynth 0 for output clocks 0-3, else multisynth4 + 2 = xtal + 3 = clkin (si5351c only) +- silabs,drive-strength: output drive strength in mA, shall be one of {2,4,6,8}. +- silabs,multisynth-source: source pll A(0) or B(1) of corresponding multisynth + divider. +- silabs,pll-master: boolean, multisynth can change pll frequency. + +==Example== + +/* 25MHz reference crystal */ +ref25: ref25M
Re: [PATCH v4] clk: add si5351 i2c common clock driver
On Wed, Apr 3, 2013 at 1:46 AM, Mike Turquette mturque...@linaro.org wrote: Quoting Sebastian Hesselbarth (2013-03-23 07:46:50) diff --git a/drivers/clk/clk-si5351.c b/drivers/clk/clk-si5351.c new file mode 100644 index 000..9d0c210 --- /dev/null +++ b/drivers/clk/clk-si5351.c @@ -0,0 +1,1411 @@ +/* + * clk-si5351.c: Silicon Laboratories Si5351A/B/C I2C Clock Generator + * + * Sebastian Hesselbarth sebastian.hesselba...@gmail.com + * Rabeeh Khoury rab...@solid-run.com + * + * References: + * [1] Si5351A/B/C Data Sheet + * http://www.silabs.com/Support%20Documents/TechnicalDocs/Si5351.pdf + * [2] Manually Generating an Si5351 Register Map + * http://www.silabs.com/Support%20Documents/TechnicalDocs/AN619.pdf + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. + */ + +#include linux/module.h +#include linux/kernel.h +#include linux/clk-private.h I hope that this is supposed to be clk-provider.h. I don't plan on taking in any more clock drivers that depend on clk-private.h. I didn't see any reason for this patch to not use clk-provider.h. True except for two things: - The driver sets clk flags for the hierarchy after parsing the DT and there is no helper __clk_set_flags() - For single parent clks (pxtal, pclkin) I reuse clk-name for .parent_names For the latter I would have to add two static char *[1] somewhere, not a big deal. But the for the flags, I guess there will be no __clk_set_flags helper? Then I'd have to parse the DT in advance to have the correct flags ready at clk registration. Sebastian ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v3] clk: add si5351 i2c common clock driver
On Wed, Mar 20, 2013 at 5:48 PM, Daniel Mack zon...@gmail.com wrote: On 20.03.2013 14:55, michal.bachr...@gmail.com wrote: Thanks for writing this driver! I have tested your si5351 clock driver and his tuning capabilities. It works well, it generates proper clock frequency, but when new frequency is generated, little clock gap (1ms) is generated. Si5351 datasheet and WP claims, clock tuning can be without gaps - http://www.silabs.com/Support%20Documents/TechnicalDocs/Si5350-51-Frequency-Shifting-WP.pdf I made some tests with Si5351A chip and I found that when tuning touch only Multisynth registers, it can tune without gaps. There is no need for soft PLL reset. I found also, accessing Multisynth registers is not atomic, so there can be another frequency at output, while not all registers are written. Writing only to one register seems to be atomic. Michael, if you don't configure the clock output to modify the pll, changing output frequency will not alter pll config and there will be no reset of pll. Yeah, but limiting possible changes to the PLLs to one single register also means that you cannot offer to generate all the frequencies any more. What could probably be done is refine the algorithm so that it stays 'as close as possible' to the former values, but I'm not sure how much work that implies. Can you provide a patch against Sebastian's v3 to do that? Then it can be cleanly applied on top of the driver later. Ack. Feel free to post a patch on top of v4 now. Sebastian ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH v4] clk: add si5351 i2c common clock driver
This patch adds a common clock driver for Silicon Labs Si5351a/b/c i2c programmable clock generators. Currently, the driver supports DT kernels only and VXCO feature of si5351b is not implemented. DT bindings selectively allow to overwrite stored Si5351 configuration which is very helpful for clock generators with empty eeprom configuration. Corresponding device tree binding documentation is also added. Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@gmail.com Tested-by: Daniel Mack zon...@gmail.com --- Changes from v3: - add silabs prefix to custom DT properties (Reported by Lars-Peter Clausen) - use sizeof(*foo) instead of sizeof(struct bar) (Reported by Lars-Peter Clausen) - check return value of of_clk_add_provider (Reported by Lars-Peter Clausen) - clean up i2c client init (Reported by Lars-Peter Clausen) - silence successful probe (Suggested by Lars-Peter Clausen) - make CONFIG_CLK_SI5351 depend on CONFIG_OF Changes from v2: - add curly brackets to if-else-statements (Reported by Daniel Mack) - fix div-by-zero for clk6/clk7 (Reported by Daniel Mack) - fix parameter address calculation for clk6/clk7 Changes from v1: - remove .is_enabled functions as they read from i2c (Reported by Daniel Mack) - add CLK_SET_RATE_PARENT on clkout reparent if clkout uses its own multisync Cc: Grant Likely grant.lik...@secretlab.ca Cc: Rob Herring rob.herr...@calxeda.com Cc: Rob Landley r...@landley.net Cc: Mike Turquette mturque...@linaro.org Cc: Stephen Warren swar...@nvidia.com Cc: Thierry Reding thierry.red...@avionic-design.de Cc: Dom Cobley popcorn...@gmail.com Cc: Linus Walleij linus.wall...@linaro.org Cc: Arnd Bergmann a...@arndb.de Cc: Andrew Morton a...@linux-foundation.org Cc: Russell King - ARM Linux li...@arm.linux.org.uk Cc: Rabeeh Khoury rab...@solid-run.com Cc: Daniel Mack zon...@gmail.com Cc: Jean-Francois Moine moin...@free.fr Cc: Lars-Peter Clausen l...@metafoo.de Cc: devicetree-discuss@lists.ozlabs.org Cc: linux-...@vger.kernel.org Cc: linux-ker...@vger.kernel.org Cc: linux-arm-ker...@lists.infradead.org --- .../devicetree/bindings/clock/silabs,si5351.txt| 114 ++ .../devicetree/bindings/vendor-prefixes.txt|1 + drivers/clk/Kconfig| 10 + drivers/clk/Makefile |1 + drivers/clk/clk-si5351.c | 1411 drivers/clk/clk-si5351.h | 155 +++ 6 files changed, 1692 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/silabs,si5351.txt create mode 100644 drivers/clk/clk-si5351.c create mode 100644 drivers/clk/clk-si5351.h diff --git a/Documentation/devicetree/bindings/clock/silabs,si5351.txt b/Documentation/devicetree/bindings/clock/silabs,si5351.txt new file mode 100644 index 000..cc37465 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/silabs,si5351.txt @@ -0,0 +1,114 @@ +Binding for Silicon Labs Si5351a/b/c programmable i2c clock generator. + +Reference +[1] Si5351A/B/C Data Sheet +http://www.silabs.com/Support%20Documents/TechnicalDocs/Si5351.pdf + +The Si5351a/b/c are programmable i2c clock generators with upto 8 output +clocks. Si5351a also has a reduced pin-count package (MSOP10) where only +3 output clocks are accessible. The internal structure of the clock +generators can be found in [1]. + +==I2C device node== + +Required properties: +- compatible: shall be one of silabs,si5351{a,a-msop,b,c}. +- reg: i2c device address, shall be 0x60 or 0x61. +- #clock-cells: from common clock binding; shall be set to 1. +- clocks: from common clock binding; list of parent clock + handles, shall be xtal reference clock or xtal and clkin for + si5351c only. +- #address-cells: shall be set to 1. +- #size-cells: shall be set to 0. + +Optional properties: +- silabs,pll-source: pair of (number, source) for each pll. Allows + to overwrite clock source of pll A (number=0) or B (number=1). + +==Child nodes== + +Each of the clock outputs can be overwritten individually by +using a child node to the I2C device node. If a child node for a clock +output is not set, the eeprom configuration is not overwritten. + +Required child node properties: +- reg: number of clock output. + +Optional child node properties: +- silabs,clock-source: source clock of the output divider stage N, shall be + 0 = multisynth N + 1 = multisynth 0 for output clocks 0-3, else multisynth4 + 2 = xtal + 3 = clkin (si5351c only) +- silabs,drive-strength: output drive strength in mA, shall be one of {2,4,6,8}. +- silabs,multisynth-source: source pll A(0) or B(1) of corresponding multisynth + divider. +- silabs,pll-master: boolean, multisynth can change pll frequency. + +==Example== + +/* 25MHz reference crystal */ +ref25: ref25M { + compatible = fixed-clock; + #clock-cells = 0; + clock-frequency = 2500; +}; + +i2c-master-node { + + /* Si5351a msop10 i2c clock generator */ + si5351a: clock-generator
Re: [PATCH v3] clk: add si5351 i2c common clock driver
On 03/21/2013 07:09 PM, Lars-Peter Clausen wrote: On 03/18/2013 11:43 AM, Sebastian Hesselbarth wrote: This patch adds a common clock driver for Silicon Labs Si5351a/b/c i2c programmable clock generators. Currently, the driver supports DT kernels only and VXCO feature of si5351b is not implemented. DT bindings selectively allow to overwrite stored Si5351 configuration which is very helpful for clock generators with empty eeprom configuration. Corresponding device tree binding documentation is also added. Signed-off-by: Sebastian Hesselbarthsebastian.hesselba...@gmail.com Hi, Couple of comments inside. Lars-Peter, thanks for the review. --- [...] +==Child nodes== + +Each of the clock outputs can be overwritten individually by +using a child node to the I2C device node. If a child node for a clock +output is not set, the eeprom configuration is not overwritten. + +Required child node properties: +- reg: number of clock output. + +Optional child node properties: +- clock-source: source clock of the output divider stage N, shall be + 0 = multisynth N + 1 = multisynth 0 for output clocks 0-3, else multisynth4 + 2 = xtal + 3 = clkin (si5351c only) +- drive-strength: output drive strength in mA, shall be one of {2,4,6,8}. +- multisynth-source: source pll A(0) or B(1) of corresponding multisynth + divider. +- pll-master: boolean, multisynth can change pll frequency. Custom properties need a vendor prefix. Good catch, I will add silabs prefix here. [...] diff --git a/drivers/clk/clk-si5351.c b/drivers/clk/clk-si5351.c new file mode 100644 index 000..38540e7 --- /dev/null +++ b/drivers/clk/clk-si5351.c @@ -0,0 +1,1429 @@ [...] + +/* + * Si5351 vxco clock input (Si5351B only) + */ + +static int si5351_vxco_prepare(struct clk_hw *hw) +{ + struct si5351_hw_data *hwdata = + container_of(hw, struct si5351_hw_data, hw); + + dev_warn(hwdata-drvdata-client-dev, VXCO currently unsupported\n); Wouldn't it be better to not add the vxco clock if it is not supported? Hmm, I thought about that already and knew someone will suggest to remove it. But, IMHO it is better to leave that functions there for two reasons: 1. It is only a small part of one of three supported si5351 variant and doesn't take much space. 2. The most common user of this driver is a hardware engineer and I want him/her to help to add support. No warning, no note. I can make the clock registration fail, if that is what you suggest. [..] + +static const struct of_device_id si5351_dt_ids[] = { + { .compatible = silabs,si5351a, .data = (void *)SI5351_VARIANT_A, }, + { .compatible = silabs,si5351a-msop, +.data = (void *)SI5351_VARIANT_A3, }, + { .compatible = silabs,si5351b, .data = (void *)SI5351_VARIANT_B, }, + { .compatible = silabs,si5351c, .data = (void *)SI5351_VARIANT_C, }, + { } +}; +MODULE_DEVICE_TABLE(i2c, si5351_dt_ids); MODULE_DEVICE_TABLE(of, Ack. +static int si5351_i2c_probe( + struct i2c_client *client, const struct i2c_device_id *id) This should easily fit in one line. Ack. +{ + struct si5351_driver_data *drvdata; + struct clk_init_data init; + struct clk *clk; + const char *parent_names[4]; + u8 num_parents, num_clocks; + int ret, n; + + drvdata = devm_kzalloc(client-dev, sizeof(struct si5351_driver_data), sizeof(*drvdata) is the preferred way of writing this, same for a few other similar instances. Yeah, I will change that. + GFP_KERNEL); + if (drvdata == NULL) { + dev_err(client-dev, unable to allocate driver data\n); + return -ENOMEM; + } + [...] + of_clk_add_provider(client-dev.of_node, of_clk_src_onecell_get, + drvdata-onecell); You should check the return value of of_clk_add_provider Ack. + + dev_info(client-dev, registered si5351 i2c client\n); + That's just noise, imagine every driver would print such a line, your bootlog would be scrolling for hours ;) I'd either remove it or make it dev_dbg Actually, I understand not to have it, but if you don't want it you can still boot with quiet, can't you? Having one single line that helps you see it has been probed helps a lot. But, I don't have a strong opinion on that and can make it dev_dbg. + return 0; +} + +static int si5351_i2c_remove(struct i2c_client *client) +{ + i2c_set_clientdata(client, NULL); This is not required anymore, the core takes care of it these days. I think you can drop the whole remove callback. Ok. + return 0; +} + +static const struct i2c_device_id si5351_i2c_ids[] = { + { silabs,si5351, SI5351_BUS_BASE_ADDR | 0 }, + { silabs,si5351, SI5351_BUS_BASE_ADDR | 1 }, + { } +}; This is not how it is supposed to be used. The second field is not the i2c address of the device, but device specific data, which you can
Re: [PATCH 5/5] arm: dts: Convert mvebu device tree files to 64 bits
On 03/21/2013 10:41 PM, Jason Gunthorpe wrote: On Thu, Mar 21, 2013 at 10:15:23PM +0100, Thomas Petazzoni wrote: Dear Jason Gunthorpe, On Thu, 21 Mar 2013 14:55:45 -0600, Jason Gunthorpe wrote: Or, better, locate all the internal registers above 8G and use contiguous DRAM mapping from 0 - 8GB I see two potential issues with this idea: *) It only works when LPAE is enabled, so we would have to have different internal register addresses depending on whether LPAE is enabled or not. Probably not impossible, but not very straightforward either. Ideally the internal register space address would come from the DT - we are getting very close to that on Marvell, I think.. Things like earlyprintk should ideally early parse the DT, harder I know :( *) It would require Linux to change the internal registers address (for now the kernel relies on the bootloader). The problem is that we can't do it early enough to preserve the earlyprintk functionality. Maybe you have suggestions on how to achieve that? I can't forsee how Linux could do this reprogramming - not only do you have to move the registers, you'd also have to reprogram the DRAM bases, while running from the DRAM. Not only does that have to be done early, but the DT would need to describe the DRAM ranks, and the code would have to parse it.. On top of that it would have to be very careful not to wack any DRAM that has already been touched by the kernel.. Tricky stuff :) Jason, Lior, at least for Dove you are missing one important fact: The Internal Registers Base Address register is within ... the internal registers. You can't even look it up without knowing where it is. That doesn't mean we can parse the DT for the current register base address and move it to where we expect it. But for DRAM, I suggest not to mess with it. The boot loader is there for a reason. Sebastian ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v3] clk: add si5351 i2c common clock driver
On 03/20/2013 01:26 AM, Mike Turquette wrote: Quoting Sebastian Hesselbarth (2013-03-18 03:43:17) This patch adds a common clock driver for Silicon Labs Si5351a/b/c i2c programmable clock generators. Currently, the driver supports DT kernels only and VXCO feature of si5351b is not implemented. DT bindings selectively allow to overwrite stored Si5351 configuration which is very helpful for clock generators with empty eeprom configuration. Corresponding device tree binding documentation is also added. Signed-off-by: Sebastian Hesselbarthsebastian.hesselba...@gmail.com --- Changes from v2: - add curly brackets to if-else-statements (Reported by Daniel Mack) - fix div-by-zero for clk6/clk7 (Reported by Daniel Mack) - fix parameter address calculation for clk6/clk7 Changes from v1: - remove .is_enabled functions as they read from i2c (Reported by Daniel Mack) - add CLK_SET_RATE_PARENT on clkout reparent if clkout uses its own multisync I've just pushed out a new clk-next branch which includes Ulf Hansson's series to introduce the .is_prepared callback. Perhaps you can use that where you were previously using .is_enabled? Mike, thanks for the info, but I think having .prepare_enable and regmap caching is just fine for now. When .is_prepared is in mainline, I will re-add the callbacks. Sebastian ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH] clk: add DT test clock consumer driver
On 03/19/2013 02:54 AM, Mike Turquette wrote: Quoting Arnd Bergmann (2013-03-16 07:56:54) On Saturday 16 March 2013, Sebastian Hesselbarth wrote: This driver adds a DT test clock consumer that exposes debugfs files to enable/disable and set/get rate of the attached programmable clock. During development of a i2c-attached clock generator I found it useful to debug the clock generator's internal pll settings by enforcing clock rates through debugfs. Signed-off-by: Sebastian Hesselbarthsebastian.hesselba...@gmail.com It sounds a little clumsy to have a device driver to match a device that you create just for matching the driver. Would it be possible to separate the debugging logic from the platform device logic? I think it may be useful to have a debugfs or sysfs inteface for all clocks in the system, even if that is disabled by default or only available after manually loading a module implementing that functionality. I agree that a generic approach is needed here. I have been meaning to break the existing debugfs stuff out into clk-debug.c. I'll do that soon and maybe you can add a new Kconfig entry for COMMON_CLK_DEBUG_USERSPACE (or something like that) which implements this? Mike, I agree with you and Arnd about clumsiness and a generic approach, but this driver is a little different from controlling _all_ clocks within the tree. It just adds one consumer that can _request_ a new rate. Nevertheless, I can have a look at clk-debug and adding the functionality. On the other hand this sort of stuff really scares me. I know for a fact that a debug interface to enable/disable clocks and set clock rate would ship on real devices. Quite likely some android phones out there would be controlling hardware clocks from some horrible userspace utility. *shudder* This will happen for sure. Sebastian, another small nitpick, can you change the enable attribute to be named prepare_enable? This more accurately reflects what is going on. On a generic approach I would rather have a look at the actual ops that are provided and name the files accordingly. That will also allow us _not_ to set the rate of crystal oscillators ;) I also wonder how simple it would be to add a parent attribute here that allows one to call clk_set_parent from the debugfs interface? To make it easy on you, the interface could accept an integer as the index of the clk-parents[] array. This is a bad interface design as it requires the user to look into the code to know which index corresponds to which parent clock; however I do not want people to use this interface for anything other than debug/testing, so I am ok with this interface being a PITA to use. Sure, but it will not help much against userspace hardware clock utilities ;) Sebastian ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH v3] clk: add si5351 i2c common clock driver
This patch adds a common clock driver for Silicon Labs Si5351a/b/c i2c programmable clock generators. Currently, the driver supports DT kernels only and VXCO feature of si5351b is not implemented. DT bindings selectively allow to overwrite stored Si5351 configuration which is very helpful for clock generators with empty eeprom configuration. Corresponding device tree binding documentation is also added. Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@gmail.com --- Changes from v2: - add curly brackets to if-else-statements (Reported by Daniel Mack) - fix div-by-zero for clk6/clk7 (Reported by Daniel Mack) - fix parameter address calculation for clk6/clk7 Changes from v1: - remove .is_enabled functions as they read from i2c (Reported by Daniel Mack) - add CLK_SET_RATE_PARENT on clkout reparent if clkout uses its own multisync Cc: Grant Likely grant.lik...@secretlab.ca Cc: Rob Herring rob.herr...@calxeda.com Cc: Rob Landley r...@landley.net Cc: Mike Turquette mturque...@linaro.org Cc: Stephen Warren swar...@nvidia.com Cc: Thierry Reding thierry.red...@avionic-design.de Cc: Dom Cobley popcorn...@gmail.com Cc: Linus Walleij linus.wall...@linaro.org Cc: Arnd Bergmann a...@arndb.de Cc: Andrew Morton a...@linux-foundation.org Cc: Russell King - ARM Linux li...@arm.linux.org.uk Cc: Rabeeh Khoury rab...@solid-run.com Cc: Daniel Mack zon...@gmail.com Cc: Jean-Francois Moine moin...@free.fr Cc: devicetree-discuss@lists.ozlabs.org Cc: linux-...@vger.kernel.org Cc: linux-ker...@vger.kernel.org Cc: linux-arm-ker...@lists.infradead.org --- .../devicetree/bindings/clock/silabs,si5351.txt| 114 ++ .../devicetree/bindings/vendor-prefixes.txt|1 + drivers/clk/Kconfig|9 + drivers/clk/Makefile |1 + drivers/clk/clk-si5351.c | 1429 drivers/clk/clk-si5351.h | 155 +++ 6 files changed, 1709 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/silabs,si5351.txt create mode 100644 drivers/clk/clk-si5351.c create mode 100644 drivers/clk/clk-si5351.h diff --git a/Documentation/devicetree/bindings/clock/silabs,si5351.txt b/Documentation/devicetree/bindings/clock/silabs,si5351.txt new file mode 100644 index 000..f73d2d2 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/silabs,si5351.txt @@ -0,0 +1,114 @@ +Binding for Silicon Labs Si5351a/b/c programmable i2c clock generator. + +Reference +[1] Si5351A/B/C Data Sheet +http://www.silabs.com/Support%20Documents/TechnicalDocs/Si5351.pdf + +The Si5351a/b/c are programmable i2c clock generators with upto 8 output +clocks. Si5351a also has a reduced pin-count package (MSOP10) where only +3 output clocks are accessible. The internal structure of the clock +generators can be found in [1]. + +==I2C device node== + +Required properties: +- compatible: shall be one of silabs,si5351{a,a-msop,b,c}. +- reg: i2c device address, shall be 0x60 or 0x61. +- #clock-cells: from common clock binding; shall be set to 1. +- clocks: from common clock binding; list of parent clock + handles, shall be xtal reference clock or xtal and clkin for + si5351c only. +- #address-cells: shall be set to 1. +- #size-cells: shall be set to 0. + +Optional properties: +- pll-source: pair of (number, source) for each pll. Allows + to overwrite clock source of pll A (number=0) or B (number=1). + +==Child nodes== + +Each of the clock outputs can be overwritten individually by +using a child node to the I2C device node. If a child node for a clock +output is not set, the eeprom configuration is not overwritten. + +Required child node properties: +- reg: number of clock output. + +Optional child node properties: +- clock-source: source clock of the output divider stage N, shall be + 0 = multisynth N + 1 = multisynth 0 for output clocks 0-3, else multisynth4 + 2 = xtal + 3 = clkin (si5351c only) +- drive-strength: output drive strength in mA, shall be one of {2,4,6,8}. +- multisynth-source: source pll A(0) or B(1) of corresponding multisynth + divider. +- pll-master: boolean, multisynth can change pll frequency. + +==Example== + +/* 25MHz reference crystal */ +ref25: ref25M { + compatible = fixed-clock; + #clock-cells = 0; + clock-frequency = 2500; +}; + +i2c-master-node { + + /* Si5351a msop10 i2c clock generator */ + si5351a: clock-generator@60 { + compatible = silabs,si5351a-msop; + reg = 0x60; + #address-cells = 1; + #size-cells = 0; + #clock-cells = 1; + + /* connect xtal input to 25MHz reference */ + clocks = ref25; + + /* connect xtal input as source of pll0 and pll1 */ + pll-source = 0 0, 1 0; + + /* +* overwrite clkout0 configuration with: +* - 8mA output drive strength
[PATCH] clk: add DT test clock consumer driver
This driver adds a DT test clock consumer that exposes debugfs files to enable/disable and set/get rate of the attached programmable clock. During development of a i2c-attached clock generator I found it useful to debug the clock generator's internal pll settings by enforcing clock rates through debugfs. Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@gmail.com --- Cc: Grant Likely grant.lik...@secretlab.ca Cc: Rob Herring rob.herr...@calxeda.com Cc: Rob Landley r...@landley.net Cc: Mike Turquette mturque...@linaro.org Cc: Linus Walleij linus.wall...@linaro.org Cc: Arnd Bergmann a...@arndb.de Cc: Andrew Morton a...@linux-foundation.org Cc: devicetree-discuss@lists.ozlabs.org Cc: linux-...@vger.kernel.org Cc: linux-ker...@vger.kernel.org Cc: linux-arm-ker...@lists.infradead.org --- .../bindings/clock/test-clock-consumer.txt | 19 ++ drivers/clk/Kconfig|6 + drivers/clk/Makefile |3 + drivers/clk/clk-test.c | 183 4 files changed, 211 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/test-clock-consumer.txt create mode 100644 drivers/clk/clk-test.c diff --git a/Documentation/devicetree/bindings/clock/test-clock-consumer.txt b/Documentation/devicetree/bindings/clock/test-clock-consumer.txt new file mode 100644 index 000..3e228ed --- /dev/null +++ b/Documentation/devicetree/bindings/clock/test-clock-consumer.txt @@ -0,0 +1,19 @@ +* Test Clock Consumer + +The test clock consumer allows to debug clock providers by exposing +debugfs files to enable/disable and set/get rate of attached clock +respectively. It is especially useful for checking programmable clock +generators, e.g. enforcing target clock rates to debug clock generator's +pll settings. Currently, there is only one clock per test clock consumer +supported but more than one test clock consumer can be added. + +Required properties: +- compatible : shall be test-clock-consumer +- clocks : phandle of the clock to debug + +Example: + +clock-consumer { + compatible = test-clock-consumer; + clocks = programmable_clock; +}; diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index a47e6ee..e23e471 100644 --- a/drivers/clk/Kconfig +++ b/drivers/clk/Kconfig @@ -33,6 +33,12 @@ config COMMON_CLK_DEBUG clk_flags, clk_prepare_count, clk_enable_count clk_notifier_count. +config COMMON_CLK_TEST_CONSUMER + tristate Clock consumer test driver + ---help--- + This is a clock consumer test driver to allow testing clock + provider through user space. + config COMMON_CLK_WM831X tristate Clock driver for WM831x/2x PMICs depends on MFD_WM831X diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile index 300d477..095899e 100644 --- a/drivers/clk/Makefile +++ b/drivers/clk/Makefile @@ -34,3 +34,6 @@ obj-$(CONFIG_X86) += x86/ obj-$(CONFIG_COMMON_CLK_WM831X) += clk-wm831x.o obj-$(CONFIG_COMMON_CLK_MAX77686) += clk-max77686.o obj-$(CONFIG_CLK_TWL6040) += clk-twl6040.o + +# Debug drivers +obj-$(CONFIG_COMMON_CLK_TEST_CONSUMER) += clk-test.o diff --git a/drivers/clk/clk-test.c b/drivers/clk/clk-test.c new file mode 100644 index 000..08dd66b --- /dev/null +++ b/drivers/clk/clk-test.c @@ -0,0 +1,183 @@ +/* + * clk-test.c: Common Clock Framework Test Clock Consumer + * + * (c) 2012 Sebastian Hesselbarth sebastian.hesselba...@gmail.com + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. + */ + +#include linux/module.h +#include linux/kernel.h +#include linux/clk.h +#include linux/clk-provider.h +#include linux/debugfs.h +#include linux/err.h +#include linux/errno.h +#include linux/of_platform.h + +struct clock_consumer_data { + struct clk *clk; + struct dentry *clkdir; + const char *name; + unsigned intenabled; +}; + +static struct dentry *rootdir; + +static int clock_consumer_debug_rate_set(void *data, u64 val) +{ + struct clock_consumer_data *ccdata = (struct clock_consumer_data *)data; + + if (!ccdata-enabled) + return -EBUSY; + + return clk_set_rate(ccdata-clk, (unsigned long)val); +} + +static int clock_consumer_debug_rate_get(void *data, u64 *val) +{ + struct clock_consumer_data *ccdata = (struct clock_consumer_data *)data; + + if (!ccdata-enabled) + return -EBUSY; + + *val = clk_get_rate(ccdata-clk); + + return 0; +} + +static int clock_consumer_debug_enable_set(void *data, u64 val) +{ + struct clock_consumer_data *ccdata = (struct clock_consumer_data *)data; + + if ((val == ccdata-enabled) || (!val !ccdata-enabled)) + return 0; + + ccdata-enabled
[PATCH v2] clk: add si5351 i2c common clock driver
This patch adds a common clock driver for Silicon Labs Si5351a/b/c i2c programmable clock generators. Currently, the driver supports DT kernels only and VXCO feature of si5351b is not implemented. DT bindings selectively allow to overwrite stored Si5351 configuration which is very helpful for clock generators with empty eeprom configuration. Corresponding device tree binding documentation is also added. Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@gmail.com --- Changes from v1: - remove .is_enabled functions as they read from i2c (Reported by Daniel Mack) - add CLK_SET_RATE_PARENT on clkout reparent if clkout uses its own multisync Cc: Grant Likely grant.lik...@secretlab.ca Cc: Rob Herring rob.herr...@calxeda.com Cc: Rob Landley r...@landley.net Cc: Mike Turquette mturque...@linaro.org Cc: Stephen Warren swar...@nvidia.com Cc: Thierry Reding thierry.red...@avionic-design.de Cc: Dom Cobley popcorn...@gmail.com Cc: Linus Walleij linus.wall...@linaro.org Cc: Arnd Bergmann a...@arndb.de Cc: Andrew Morton a...@linux-foundation.org Cc: Russell King - ARM Linux li...@arm.linux.org.uk Cc: Rabeeh Khoury rab...@solid-run.com Cc: Daniel Mack zon...@gmail.com Cc: Jean-Francois Moine moin...@free.fr Cc: devicetree-discuss@lists.ozlabs.org Cc: linux-...@vger.kernel.org Cc: linux-ker...@vger.kernel.org Cc: linux-arm-ker...@lists.infradead.org --- .../devicetree/bindings/clock/silabs,si5351.txt| 114 ++ .../devicetree/bindings/vendor-prefixes.txt|1 + drivers/clk/Kconfig|9 + drivers/clk/Makefile |1 + drivers/clk/clk-si5351.c | 1413 drivers/clk/clk-si5351.h | 155 +++ 6 files changed, 1693 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/silabs,si5351.txt create mode 100644 drivers/clk/clk-si5351.c create mode 100644 drivers/clk/clk-si5351.h diff --git a/Documentation/devicetree/bindings/clock/silabs,si5351.txt b/Documentation/devicetree/bindings/clock/silabs,si5351.txt new file mode 100644 index 000..f73d2d2 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/silabs,si5351.txt @@ -0,0 +1,114 @@ +Binding for Silicon Labs Si5351a/b/c programmable i2c clock generator. + +Reference +[1] Si5351A/B/C Data Sheet +http://www.silabs.com/Support%20Documents/TechnicalDocs/Si5351.pdf + +The Si5351a/b/c are programmable i2c clock generators with upto 8 output +clocks. Si5351a also has a reduced pin-count package (MSOP10) where only +3 output clocks are accessible. The internal structure of the clock +generators can be found in [1]. + +==I2C device node== + +Required properties: +- compatible: shall be one of silabs,si5351{a,a-msop,b,c}. +- reg: i2c device address, shall be 0x60 or 0x61. +- #clock-cells: from common clock binding; shall be set to 1. +- clocks: from common clock binding; list of parent clock + handles, shall be xtal reference clock or xtal and clkin for + si5351c only. +- #address-cells: shall be set to 1. +- #size-cells: shall be set to 0. + +Optional properties: +- pll-source: pair of (number, source) for each pll. Allows + to overwrite clock source of pll A (number=0) or B (number=1). + +==Child nodes== + +Each of the clock outputs can be overwritten individually by +using a child node to the I2C device node. If a child node for a clock +output is not set, the eeprom configuration is not overwritten. + +Required child node properties: +- reg: number of clock output. + +Optional child node properties: +- clock-source: source clock of the output divider stage N, shall be + 0 = multisynth N + 1 = multisynth 0 for output clocks 0-3, else multisynth4 + 2 = xtal + 3 = clkin (si5351c only) +- drive-strength: output drive strength in mA, shall be one of {2,4,6,8}. +- multisynth-source: source pll A(0) or B(1) of corresponding multisynth + divider. +- pll-master: boolean, multisynth can change pll frequency. + +==Example== + +/* 25MHz reference crystal */ +ref25: ref25M { + compatible = fixed-clock; + #clock-cells = 0; + clock-frequency = 2500; +}; + +i2c-master-node { + + /* Si5351a msop10 i2c clock generator */ + si5351a: clock-generator@60 { + compatible = silabs,si5351a-msop; + reg = 0x60; + #address-cells = 1; + #size-cells = 0; + #clock-cells = 1; + + /* connect xtal input to 25MHz reference */ + clocks = ref25; + + /* connect xtal input as source of pll0 and pll1 */ + pll-source = 0 0, 1 0; + + /* +* overwrite clkout0 configuration with: +* - 8mA output drive strength +* - pll0 as clock source of multisynth0 +* - multisynth0 as clock source of output divider +* - multisynth0 can change pll0 +* - set initial clock
Re: [PATCH] clk: add si5351 i2c common clock driver
Daniel, first of all sorry for the late answer but thanks for testing the driver. On 2/19/13, Daniel Mack zon...@gmail.com wrote: Hi Sebastian, I did some more tests today and it took me a while to dig for the root cause why things were not working for me in the first place - see below. On 09.02.2013 13:59, Sebastian Hesselbarth wrote: +==Example== + +/* 25MHz reference crystal */ +ref25: ref25M { +compatible = fixed-clock; +#clock-cells = 0; +clock-frequency = 2500; +}; + +i2c-master-node { + +/* Si5351a msop10 i2c clock generator */ +si5351a: clock-generator@60 { +compatible = silabs,si5351a-msop; +reg = 0x60; +#address-cells = 1; +#size-cells = 0; +#clock-cells = 1; + +/* connect xtal input to 25MHz reference */ +clocks = ref25; As referred to in another thread, registering the ref25M clock that way didn't suffice for me on my platform - but that's a different story. I guess fixed-clock isn't registered by OMAP's clock init code? I had to do this on dove, too. Actually, I will come back to clock initialization for dove later and was hoping that there will be some global way of registering core common clock drivers (or at least fixed-clock) until then. +static void si5351_read_parameters(struct si5351_driver_data *drvdata, +unsigned char reg, struct si5351_parameters *params) +{ +unsigned char buf[SI5351_PARAMETERS_LENGTH]; On a general note, I think you can use u8 instead of unsigned char all over the place here, which will save you some indentation trouble. Ok, I guess I was deriving unsigned char usage from other clock drivers and never went to u8. But I ll reconsider using u8 when all issues are worked out. +static inline int _si5351_clkout_reparent(struct si5351_driver_data *drvdata, + unsigned char num, unsigned char parent) +{ +struct clk *pclk; + +if (num 8 || +(drvdata-variant == SI5351_VARIANT_A3 num 3)) +return -EINVAL; + +switch (parent) { +case 0: +pclk = drvdata-msynth[num].hw.clk; +break; +case 1: +pclk = drvdata-msynth[0].hw.clk; +if (num = 4) +pclk = drvdata-msynth[4].hw.clk; +break; +case 2: +pclk = drvdata-xtal.clk; +break; +case 3: +if (drvdata-variant != SI5351_VARIANT_C) +return -EINVAL; +pclk = drvdata-clkin.clk; +break; +default: +return -EINVAL; +} +return clk_set_parent(drvdata-clkout[num].hw.clk, pclk); +} [...] +static int si5351_clkout_set_parent(struct clk_hw *hw, u8 index) +{ +struct si5351_hw_data *hwdata = +container_of(hw, struct si5351_hw_data, hw); +unsigned val; + +val = 0; +hw-clk-flags = ~CLK_SET_RATE_PARENT; +switch (index) { +case 0: +hw-clk-flags |= CLK_SET_RATE_PARENT; +val = SI5351_CLK_INPUT_MULTISYNTH_N; +break; I fugured that _si5351_clkout_reparent() wouldn't actually call -set_parent() on the clock, which leads to the fact that CLK_SET_RATE_PARENT is not set in the flags. That way, only the clkout end leaf is actually supplied with a new rate, which leads to incorrect effective clocks, depending on the current multisynth/pll configuration. Yeah, true. Unfortunately, _clkout_reparent() is more like a dirty hack to allow to reparent the clock output. At registration internal configuration of si5351 is not known and when I parse the DT for clock configuration I might have been already assigned to the same parent clk that you later explicitly configure. What I basically want for si5351 (or any other eeprom based programmable clock gen) is that stored configuration is not touched. But it can be changed after eeprom contents have been copied into device's sram - and this _is_ mandatory for the si5351 that I use on CuBox where there is no useful config stored at all. Anyway, there still seem to be some more issues with doing it right on current common clk framwork - thanks for pointing it out. The reason for this is in clk_set_parent() itself, which bails if the parent is already set to the passed value: if (clk-parent == parent) goto out; I fixed that for now by explicitly setting the clock's parent to NULL before calling clk_set_parent() in _si5351_clkout_reparent(), so the calbacks are triggered. But there might be a nicer way, for example to factor out the CLK_SET_RATE_PARENT handling to some function called from _si5351_clkout_reparent() or so. Anyway, with this hack in place along with the other details I mentioned in my first mail, the driver seems to work for me now, which is great. I will do more extensive tests later that week when I have access to better scopes ... I am
Re: [PATCH] clk: add si5351 i2c common clock driver
On 02/11/2013 06:46 AM, Mike Turquette wrote: Quoting Sebastian Hesselbarth (2013-02-09 04:59:32) This patch adds a common clock driver for Silicon Labs Si5351a/b/c i2c programmable clock generators. Currently, the driver supports DT kernels only and VXCO feature of si5351b is not implemented. DT bindings selectively allow to overwrite stored Si5351 configuration which is very helpful for clock generators with empty eeprom configuration. Corresponding device tree binding documentation is also added. Signed-off-by: Sebastian Hesselbarthsebastian.hesselba...@gmail.com --- Notes: - During development I used a debugfs clock consumer that I can also post if there is interest in it. Please do. I have a set of patches that implement a fake clock subtree for testing the core framework. I've been thinking of pushing this to the list once it is more presentable and your work might fit into that nicely. Mike, then I will clean the debugfs driver and post it together with this patch for 3.9-rc1 as an individual patch. - With current (3.8-rc6) common clock framework there is two (minor) issues: * although clocks are registered with devm_clk_register they are not removed from the clock tree on unloading. That makes reloading of clk-si5351 as module impossible. This is a known issue. clk_unregister is a NOP and defining it has always been deferred until the day that someone needed it. Care to take a crack at it? Ok. I can have a look at it and propose a patch but that will take a while as other stuff came in between. But IMHO, preparing/enabling clocks by clock consumers should increase reference count so referenced modules cannot be unloaded.. but that I have never had a look at, yet ;) * potentially there could be more than one different external si5351 generators but clocks are registered with names that do not refer to e.g. the device name. Maybe common clock framework should prepend the device name for each registered clock, i.e. 0-0060.clk0. That would also avoid name collisions with same clock names from different drivers (clk0 is likely to be used by others ;)) More unfinished work, just like clk_unregister above. I'm sure you are aware that clk_register takes struct device *dev as input, but does nothing with it. It wouldn't take much to concatenate the device name and clock name if dev is present. However a complication here is that the registration code takes a parent string name to match parents up for discrete subtrees; how could statically defined data know about the device name ahead of time? I see. Wrt the above comment about spare time, would prepending DT clocks be sufficient? Or/And use a fallback mechanism that first tries a full match, full match with own device name, and relaxed match for clock name as it is now? The above design decision took place before the big DT push we have today and was short-sighted. It would be better to change the framework to rely less on string name lookups and DT is one way out of that. 3.8-rc7 is already out and I don't plan to take anything that hasn't already been submitted for 3.9 now. Can you resubmit this after 3.9-rc1 comes out? Sure, but I'll be not available next 2 weeks or so. If 3.8 falls within that time, I will re-post it later. It is ok for me, if it has to go in after 3.9 also. Sebastian ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH] clk: add si5351 i2c common clock driver
This patch adds a common clock driver for Silicon Labs Si5351a/b/c i2c programmable clock generators. Currently, the driver supports DT kernels only and VXCO feature of si5351b is not implemented. DT bindings selectively allow to overwrite stored Si5351 configuration which is very helpful for clock generators with empty eeprom configuration. Corresponding device tree binding documentation is also added. Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@gmail.com --- Notes: - During development I used a debugfs clock consumer that I can also post if there is interest in it. - With current (3.8-rc6) common clock framework there is two (minor) issues: * although clocks are registered with devm_clk_register they are not removed from the clock tree on unloading. That makes reloading of clk-si5351 as module impossible. * potentially there could be more than one different external si5351 generators but clocks are registered with names that do not refer to e.g. the device name. Maybe common clock framework should prepend the device name for each registered clock, i.e. 0-0060.clk0. That would also avoid name collisions with same clock names from different drivers (clk0 is likely to be used by others ;)) - The driver has been frequency tested for some common video/audio clocks and manages it to tune in every frequency successfully. A comparison with silabs windows tool shows a different heuristic for vco frequencies. The tests have been comfirmed by visual check on an 500MHz oscilloscope but no jitter measurements have been carried out. I will provide comparison by email on request. Cc: Grant Likely grant.lik...@secretlab.ca Cc: Rob Herring rob.herr...@calxeda.com Cc: Rob Landley r...@landley.net Cc: Mike Turquette mturque...@linaro.org Cc: Stephen Warren swar...@nvidia.com Cc: Thierry Reding thierry.red...@avionic-design.de Cc: Dom Cobley popcorn...@gmail.com Cc: Linus Walleij linus.wall...@linaro.org Cc: Arnd Bergmann a...@arndb.de Cc: Andrew Morton a...@linux-foundation.org Cc: Russell King - ARM Linux li...@arm.linux.org.uk Cc: Rabeeh Khoury rab...@solid-run.com Cc: devicetree-discuss@lists.ozlabs.org Cc: linux-...@vger.kernel.org Cc: linux-ker...@vger.kernel.org Cc: linux-arm-ker...@lists.infradead.org --- .../devicetree/bindings/clock/silabs,si5351.txt| 114 ++ .../devicetree/bindings/vendor-prefixes.txt|1 + drivers/clk/Kconfig|9 + drivers/clk/Makefile |1 + drivers/clk/clk-si5351.c | 1447 drivers/clk/clk-si5351.h | 155 +++ 6 files changed, 1727 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/silabs,si5351.txt create mode 100644 drivers/clk/clk-si5351.c create mode 100644 drivers/clk/clk-si5351.h diff --git a/Documentation/devicetree/bindings/clock/silabs,si5351.txt b/Documentation/devicetree/bindings/clock/silabs,si5351.txt new file mode 100644 index 000..3fa3c3e --- /dev/null +++ b/Documentation/devicetree/bindings/clock/silabs,si5351.txt @@ -0,0 +1,114 @@ +Binding for Silicon Labs Si5351a/b/c programmable i2c clock generator. + +Reference +[1] Si5351A/B/C Data Sheet +http://www.silabs.com/Support%20Documents/TechnicalDocs/Si5351.pdf + +The Si5351a/b/c are programmable i2c clock generators with upto 8 output +clocks. Si5351a also has a reduced pin-count package (MSOP10) where only +3 output clocks are accessible. The internal structure of the clock +generators can be found in [1]. + +==I2C device node== + +Required properties: +- compatible: shall be one of silabs,si5351{a,a-msop,b,c}. +- reg: i2c device address, shall be 0x60 or 0x61. +- #clock-cells: from common clock binding; shall be set to 1. +- clocks: from common clock binding; list of parent clock + handles, shall be xtal reference clock or xtal and clkin for + si5351c only. +- #address-cells: shall be set to 1. +- #size-cells: shall be set to 0. + +Optional properties: +- pll-source: pair of (number, source) for each pll. Allows + to overwrite clock source of pll A (number=0) or B (number=1). + +==Child nodes== + +Each of the clock outputs can be overwritten individually by +using a child node to the I2C device node. If a child node for a clock +output is not set, the eeprom configuration is not overwritten. + +Required child node properties: +- reg: number of clock output. + +Optional child node properties: +- clock-source: source clock of the output divider stage N, shall be + 0 = multisynth N + 1 = multisynth 0 for output clocks 0-3, else multisynth4 + 2 = xtal + 3 = clkin (si5351c only) +- drive-strength: output drive strength in mA, shall be one of {2,4,6,8}. +- multisynth-source: source pll A(0) or B(1) of corresponding multisynth + divider. +- pll-master: boolean, multisynth can change pll frequency. + +==Example== + +/* 25MHz reference crystal */ +ref25: ref25M
Re: [PATCH RESEND] media: rc: gpio-ir-recv: add support for device tree parsing
On Fri, Feb 8, 2013 at 6:57 PM, Mauro Carvalho Chehab mche...@redhat.com wrote: Em Wed, 06 Feb 2013 18:18:22 +0100 Sebastian Hesselbarth sebastian.hesselba...@gmail.com escreveu: On 02/06/2013 02:48 PM, Sylwester Nawrocki wrote: On 02/06/2013 09:03 AM, Sebastian Hesselbarth wrote: This patch adds device tree parsing for gpio_ir_recv platform_data and the mandatory binding documentation. It basically follows what we already have for e.g. gpio_keys. All required device tree properties are OS independent but optional properties allow linux specific support for rc protocols and maps. There was a similar patch sent by Matus Ujhelyi but that discussion died after the first reviews. Signed-off-by: Sebastian Hesselbarthsebastian.hesselba...@gmail.com --- ... diff --git a/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt b/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt new file mode 100644 index 000..937760c --- /dev/null +++ b/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt @@ -0,0 +1,20 @@ +Device-Tree bindings for GPIO IR receiver + +Required properties: + - compatible = gpio-ir-receiver; + - gpios: OF device-tree gpio specification. + +Optional properties: + - linux,allowed-rc-protocols: Linux specific u64 bitmask of allowed + rc protocols. You likely need to specify in these bindings documentation which bit corresponds to which RC protocol. I'm not very familiar with the RC internals, but why it has to be specified statically in the device tree, when decoding seems to be mostly software defined ? I might be missing something though.. Sylwester, I am not familiar with RC internals either. Maybe somebody with more insight in media/rc can clarify the specific needs for the rc subsystem. I was just transferring the DT support approach taken by gpio_keys to gpio_ir_recv as I will be using it on mach-dove/cubox soon. The allowed rc protocol field are there for devices with hardware IR support, where only a limited set of remote protocols can be decoded. For software decoders RC_BIT_ALL is the proper setup. Users of course can change it via sysfs at runtime, or a software decoder may be disabled at compilation time by not selecting its CONFIG_* var. Mauro, thanks for the clarification! So for v2 of the patch, you all agree on removing linux,allowed-rc-protocols from device node properties? Couldn't this be configured at run time, with all protocols allowed as the default ? Actually, this is how the internal rc code works. If there is nothing defined for allowed_protocols it assumes that all protocols are supported. That is why above node properties are optional. About the binding documentation of allowed_protocols, rc_map, or the default behavior of current linux code, I don't think they will stay in-sync for long. Why not? The rc_map name is used either by Kernelspace or by Userspace, in order to provide the IR keycode name that matches a given keytable. There's no plans to change it, even in the long term. Actually, I wasn't referring to changing names or bitmasks but updating the binding documentation with new allowed protocols or supported map names. For linux,rc-map-name property it should be enough to just write that it relates to linux rc subsystem rc_map name - how to actually set it to a useful name is documented in rc subsystem. And if the property is not set at all, DT parsing in gpio_ir_recv assumes the subsystem (or gpio_ir_recv platform) default, IIRC rc-none. I'll respin a v2 without allowed-protocols property soon. Sebastian ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH v2] media: rc: gpio-ir-recv: add support for device tree parsing
On 02/08/2013 10:26 PM, Sylwester Nawrocki wrote: On 02/08/2013 09:38 PM, Sebastian Hesselbarth wrote: This patch adds device tree parsing for gpio_ir_recv platform_data and the mandatory binding documentation. It basically follows what we already have for e.g. gpio_keys. All required device tree properties are OS independent but an optional property allow linux specific support for rc maps. There was a similar patch sent by Matus Ujhelyi but that discussion died after the first reviews. Signed-off-by: Sebastian Hesselbarthsebastian.hesselba...@gmail.com --- Changelog v1-v2: - get rid of ptr returned by _get_devtree_pdata() - check for of_node instead for NULL pdata - remove unneccessary double check for gpios property - remove unneccessary #ifdef CONFIG_OF around match table Cc: Grant Likelygrant.lik...@secretlab.ca Cc: Rob Herringrob.herr...@calxeda.com Cc: Rob Landleyr...@landley.net Cc: Mauro Carvalho Chehabmche...@redhat.com Cc: Sebastian Hesselbarthsebastian.hesselba...@gmail.com Cc: Benoit Thebaudeaubenoit.thebaud...@advansee.com Cc: David Hardemanda...@hardeman.nu Cc: Trilok Sonits...@codeaurora.org Cc: Sylwester Nawrockis.nawro...@samsung.com Cc: Matus Ujhelyiujhely...@gmail.com Cc: devicetree-discuss@lists.ozlabs.org Cc: linux-...@vger.kernel.org Cc: linux-ker...@vger.kernel.org Cc: linux-me...@vger.kernel.org --- .../devicetree/bindings/media/gpio-ir-receiver.txt | 16 ++ drivers/media/rc/gpio-ir-recv.c | 57 2 files changed, 73 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/gpio-ir-receiver.txt diff --git a/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt b/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt new file mode 100644 index 000..8589f30 --- /dev/null +++ b/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt @@ -0,0 +1,16 @@ +Device-Tree bindings for GPIO IR receiver + +Required properties: + - compatible = gpio-ir-receiver; + - gpios: OF device-tree gpio specification. Perhaps: - compatible: should be gpio-ir-receiver; - gpios: specifies GPIO used for IR signal reception. ? Ok, i'll change that. + +Optional properties: + - linux,rc-map-name: Linux specific remote control map name. + +Example node: + + ir: ir-receiver { + compatible = gpio-ir-receiver; + gpios =gpio0 19 1; + linux,rc-map-name = rc-rc6-mce; + }; diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c index 4f71a7d..3c62006 100644 --- a/drivers/media/rc/gpio-ir-recv.c +++ b/drivers/media/rc/gpio-ir-recv.c @@ -16,6 +16,7 @@ #includelinux/interrupt.h #includelinux/gpio.h #includelinux/slab.h +#includelinux/of_gpio.h #includelinux/platform_device.h #includelinux/irq.h #includemedia/rc-core.h @@ -30,6 +31,50 @@ struct gpio_rc_dev { bool active_low; }; +#ifdef CONFIG_OF +/* + * Translate OpenFirmware node properties into platform_data + */ +static int gpio_ir_recv_get_devtree_pdata(struct device *dev, + struct gpio_ir_recv_platform_data *pdata) +{ + struct device_node *np = dev-of_node; + enum of_gpio_flags flags; + int gpio; + + gpio = of_get_gpio_flags(np, 0,flags); + if (gpio 0) { + if (gpio != -EPROBE_DEFER) + dev_err(dev, Failed to get gpio flags, error: %d\n, + gpio); dev_err(dev, Failed to get gpio flags (%d)\n, gpio); ? Ack. + return gpio; + } + + pdata-gpio_nr = gpio; + pdata-active_low = (flags OF_GPIO_ACTIVE_LOW) ? true : false; This could be simplified to: pdata-active_low = (flags OF_GPIO_ACTIVE_LOW); Ack. + /* probe() takes care of map_name == NULL or allowed_protos == 0 */ + pdata-map_name = of_get_property(np, linux,rc-map-name, NULL); + pdata-allowed_protos = 0; + + return 0; +} + +static struct of_device_id gpio_ir_recv_of_match[] = { + { .compatible = gpio-ir-receiver, }, + { }, +}; +MODULE_DEVICE_TABLE(of, gpio_ir_recv_of_match); + +#else /* !CONFIG_OF */ + +static inline struct gpio_ir_recv_platform_data * +gpio_ir_recv_get_devtree_pdata(struct device *dev) Please check how it compiles with CONFIG_OF disabled ;) Grrrml ;) I'll fix that of course... You could also make it: #define gpio_ir_recv_get_devtree_pdata (-ENOSYS) Hmm, does that also play with parameter passing of the CONFIG_OF gpio_ir_recv_get_devtree_pdata() ? +{ + return ERR_PTR(-ENODEV); +} + +#endif + static irqreturn_t gpio_ir_recv_irq(int irq, void *dev_id) { struct gpio_rc_dev *gpio_dev = dev_id; @@ -66,6 +111,17 @@ static int gpio_ir_recv_probe(struct platform_device *pdev) pdev-dev.platform_data; int rc; + if (pdev-dev.of_node) { + struct gpio_ir_recv_platform_data *dtpdata = I think you could use pdata here instead, as previously. But I'm fine with as it is now as well. Yeah, but pdata is const and I will change it within _get_devtree_pdata(). I could cast the const away when passing it to gpio_ir_recv_get_devtree_pdata() but it is almost the same amount of code.. and it is cleaner this way. + devm_kzalloc(pdev-dev, sizeof(*dtpdata), GFP_KERNEL); + if (!dtpdata) + return -ENOMEM; + rc
[PATCH v3] media: rc: gpio-ir-recv: add support for device tree parsing
This patch adds device tree parsing for gpio_ir_recv platform_data and the mandatory binding documentation. It basically follows what we already have for e.g. gpio_keys. All required device tree properties are OS independent but an optional property allows linux specific support for rc maps. Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@gmail.com Reviewed-by: Sylwester Nawrocki s.nawro...@samsung.com --- There was a similar patch sent by Matus Ujhelyi but that discussion died after the first reviews. Changelog v1-v2: - get rid of ptr returned by _get_devtree_pdata() - check for of_node instead for NULL pdata - remove unneccessary double check for gpios property - remove unneccessary #ifdef CONFIG_OF around match table v2-v3: - use define for !OF_CONFIG _get_devtree_pdata() - simplify flags test and of_get_gpio_flags error msg - fix some typos in binding documentation - move note about previous patch submission to non-commit area Cc: Grant Likely grant.lik...@secretlab.ca Cc: Rob Herring rob.herr...@calxeda.com Cc: Rob Landley r...@landley.net Cc: Mauro Carvalho Chehab mche...@redhat.com Cc: Sebastian Hesselbarth sebastian.hesselba...@gmail.com Cc: Benoit Thebaudeau benoit.thebaud...@advansee.com Cc: David Hardeman da...@hardeman.nu Cc: Trilok Soni ts...@codeaurora.org Cc: Sylwester Nawrocki s.nawro...@samsung.com Cc: Matus Ujhelyi ujhely...@gmail.com Cc: devicetree-discuss@lists.ozlabs.org Cc: linux-...@vger.kernel.org Cc: linux-ker...@vger.kernel.org Cc: linux-me...@vger.kernel.org --- .../devicetree/bindings/media/gpio-ir-receiver.txt | 16 ++ drivers/media/rc/gpio-ir-recv.c| 52 2 files changed, 68 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/gpio-ir-receiver.txt diff --git a/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt b/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt new file mode 100644 index 000..56e726e --- /dev/null +++ b/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt @@ -0,0 +1,16 @@ +Device-Tree bindings for GPIO IR receiver + +Required properties: + - compatible: should be gpio-ir-receiver. + - gpios: specifies GPIO used for IR signal reception. + +Optional properties: + - linux,rc-map-name: Linux specific remote control map name. + +Example node: + + ir: ir-receiver { + compatible = gpio-ir-receiver; + gpios = gpio0 19 1; + linux,rc-map-name = rc-rc6-mce; + }; diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c index 4f71a7d..b5cfb44 100644 --- a/drivers/media/rc/gpio-ir-recv.c +++ b/drivers/media/rc/gpio-ir-recv.c @@ -16,6 +16,7 @@ #include linux/interrupt.h #include linux/gpio.h #include linux/slab.h +#include linux/of_gpio.h #include linux/platform_device.h #include linux/irq.h #include media/rc-core.h @@ -30,6 +31,45 @@ struct gpio_rc_dev { bool active_low; }; +#ifdef CONFIG_OF +/* + * Translate OpenFirmware node properties into platform_data + */ +static int gpio_ir_recv_get_devtree_pdata(struct device *dev, + struct gpio_ir_recv_platform_data *pdata) +{ + struct device_node *np = dev-of_node; + enum of_gpio_flags flags; + int gpio; + + gpio = of_get_gpio_flags(np, 0, flags); + if (gpio 0) { + if (gpio != -EPROBE_DEFER) + dev_err(dev, Failed to get gpio flags (%d)\n, gpio); + return gpio; + } + + pdata-gpio_nr = gpio; + pdata-active_low = (flags OF_GPIO_ACTIVE_LOW); + /* probe() takes care of map_name == NULL or allowed_protos == 0 */ + pdata-map_name = of_get_property(np, linux,rc-map-name, NULL); + pdata-allowed_protos = 0; + + return 0; +} + +static struct of_device_id gpio_ir_recv_of_match[] = { + { .compatible = gpio-ir-receiver, }, + { }, +}; +MODULE_DEVICE_TABLE(of, gpio_ir_recv_of_match); + +#else /* !CONFIG_OF */ + +#define gpio_ir_recv_get_devtree_pdata(dev, pdata) (-ENOSYS) + +#endif + static irqreturn_t gpio_ir_recv_irq(int irq, void *dev_id) { struct gpio_rc_dev *gpio_dev = dev_id; @@ -66,6 +106,17 @@ static int gpio_ir_recv_probe(struct platform_device *pdev) pdev-dev.platform_data; int rc; + if (pdev-dev.of_node) { + struct gpio_ir_recv_platform_data *dtpdata = + devm_kzalloc(pdev-dev, sizeof(*dtpdata), GFP_KERNEL); + if (!dtpdata) + return -ENOMEM; + rc = gpio_ir_recv_get_devtree_pdata(pdev-dev, dtpdata); + if (rc) + return rc; + pdata = dtpdata; + } + if (!pdata) return -EINVAL; @@ -192,6 +243,7 @@ static struct platform_driver gpio_ir_recv_driver = { .driver = { .name
[PATCH RESEND] media: rc: gpio-ir-recv: add support for device tree parsing
This patch adds device tree parsing for gpio_ir_recv platform_data and the mandatory binding documentation. It basically follows what we already have for e.g. gpio_keys. All required device tree properties are OS independent but optional properties allow linux specific support for rc protocols and maps. There was a similar patch sent by Matus Ujhelyi but that discussion died after the first reviews. Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@gmail.com --- Cc: Grant Likely grant.lik...@secretlab.ca Cc: Rob Herring rob.herr...@calxeda.com Cc: Rob Landley r...@landley.net Cc: Mauro Carvalho Chehab mche...@redhat.com Cc: Sebastian Hesselbarth sebastian.hesselba...@gmail.com Cc: Benoit Thebaudeau benoit.thebaud...@advansee.com Cc: David Hardeman da...@hardeman.nu Cc: Trilok Soni ts...@codeaurora.org Cc: devicetree-discuss@lists.ozlabs.org Cc: linux-...@vger.kernel.org Cc: linux-ker...@vger.kernel.org Cc: linux-me...@vger.kernel.org --- .../devicetree/bindings/media/gpio-ir-receiver.txt | 20 ++ drivers/media/rc/gpio-ir-recv.c| 68 +++- 2 files changed, 86 insertions(+), 2 deletions(-) create mode 100644 Documentation/devicetree/bindings/media/gpio-ir-receiver.txt diff --git a/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt b/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt new file mode 100644 index 000..937760c --- /dev/null +++ b/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt @@ -0,0 +1,20 @@ +Device-Tree bindings for GPIO IR receiver + +Required properties: + - compatible = gpio-ir-receiver; + - gpios: OF device-tree gpio specification. + +Optional properties: + - linux,allowed-rc-protocols: Linux specific u64 bitmask of allowed + rc protocols. + - linux,rc-map-name: Linux specific remote control map name. + +Example node: + + ir: ir-receiver { + compatible = gpio-ir-receiver; + gpios = gpio0 19 1; + /* allow rc protocols 1-4 */ + linux,allowed-rc-protocols = 0x 0x001e; + linux,rc-map-name = rc-rc6-mce; + }; diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c index 4f71a7d..25e09fa 100644 --- a/drivers/media/rc/gpio-ir-recv.c +++ b/drivers/media/rc/gpio-ir-recv.c @@ -16,6 +16,7 @@ #include linux/interrupt.h #include linux/gpio.h #include linux/slab.h +#include linux/of_gpio.h #include linux/platform_device.h #include linux/irq.h #include media/rc-core.h @@ -30,6 +31,63 @@ struct gpio_rc_dev { bool active_low; }; +#ifdef CONFIG_OF +/* + * Translate OpenFirmware node properties into platform_data + */ +static struct gpio_ir_recv_platform_data * +gpio_ir_recv_get_devtree_pdata(struct device *dev) +{ + struct device_node *np = dev-of_node; + struct gpio_ir_recv_platform_data *pdata; + enum of_gpio_flags flags; + int gpio; + + if (!np) + return ERR_PTR(-ENODEV); + + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); + if (!pdata) + return ERR_PTR(-ENOMEM); + + if (!of_find_property(np, gpios, NULL)) { + dev_err(dev, Found gpio-ir-receiver without gpios\n); + return ERR_PTR(-EINVAL); + } + + gpio = of_get_gpio_flags(np, 0, flags); + if (gpio 0) { + if (gpio != -EPROBE_DEFER) + dev_err(dev, Failed to get gpio flags, error: %d\n, + gpio); + return ERR_PTR(gpio); + } + + pdata-gpio_nr = gpio; + pdata-active_low = (flags OF_GPIO_ACTIVE_LOW) ? true : false; + pdata-map_name = of_get_property(np, linux,rc-map-name, NULL); + of_property_read_u64(np, linux,allowed-rc-protocols, +pdata-allowed_protos); + + return pdata; +} + +static struct of_device_id gpio_ir_recv_of_match[] = { + { .compatible = gpio-ir-receiver, }, + { }, +}; +MODULE_DEVICE_TABLE(of, gpio_ir_recv_of_match); + +#else /* !CONFIG_OF */ + +static inline struct gpio_ir_recv_platform_data * +gpio_ir_recv_get_devtree_pdata(struct device *dev) +{ + return ERR_PTR(-ENODEV); +} + +#endif + static irqreturn_t gpio_ir_recv_irq(int irq, void *dev_id) { struct gpio_rc_dev *gpio_dev = dev_id; @@ -66,8 +124,11 @@ static int gpio_ir_recv_probe(struct platform_device *pdev) pdev-dev.platform_data; int rc; - if (!pdata) - return -EINVAL; + if (!pdata) { + pdata = gpio_ir_recv_get_devtree_pdata(pdev-dev); + if (IS_ERR(pdata)) + return PTR_ERR(pdata); + } if (pdata-gpio_nr 0) return -EINVAL; @@ -195,6 +256,9 @@ static struct platform_driver gpio_ir_recv_driver = { #ifdef CONFIG_PM .pm = gpio_ir_recv_pm_ops, #endif +#ifdef
Re: [PATCH RESEND] media: rc: gpio-ir-recv: add support for device tree parsing
On 02/06/2013 02:48 PM, Sylwester Nawrocki wrote: On 02/06/2013 09:03 AM, Sebastian Hesselbarth wrote: This patch adds device tree parsing for gpio_ir_recv platform_data and the mandatory binding documentation. It basically follows what we already have for e.g. gpio_keys. All required device tree properties are OS independent but optional properties allow linux specific support for rc protocols and maps. There was a similar patch sent by Matus Ujhelyi but that discussion died after the first reviews. Signed-off-by: Sebastian Hesselbarthsebastian.hesselba...@gmail.com --- ... diff --git a/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt b/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt new file mode 100644 index 000..937760c --- /dev/null +++ b/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt @@ -0,0 +1,20 @@ +Device-Tree bindings for GPIO IR receiver + +Required properties: + - compatible = gpio-ir-receiver; + - gpios: OF device-tree gpio specification. + +Optional properties: + - linux,allowed-rc-protocols: Linux specific u64 bitmask of allowed + rc protocols. You likely need to specify in these bindings documentation which bit corresponds to which RC protocol. I'm not very familiar with the RC internals, but why it has to be specified statically in the device tree, when decoding seems to be mostly software defined ? I might be missing something though.. Sylwester, I am not familiar with RC internals either. Maybe somebody with more insight in media/rc can clarify the specific needs for the rc subsystem. I was just transferring the DT support approach taken by gpio_keys to gpio_ir_recv as I will be using it on mach-dove/cubox soon. Couldn't this be configured at run time, with all protocols allowed as the default ? Actually, this is how the internal rc code works. If there is nothing defined for allowed_protocols it assumes that all protocols are supported. That is why above node properties are optional. About the binding documentation of allowed_protocols, rc_map, or the default behavior of current linux code, I don't think they will stay in-sync for long. I'd rather completely remove those os-specific properties from DT, but that hits the above statement about the needs of media/rc subsystem. + - linux,rc-map-name: Linux specific remote control map name. + +Example node: + + ir: ir-receiver { + compatible = gpio-ir-receiver; + gpios =gpio0 19 1; + /* allow rc protocols 1-4 */ + linux,allowed-rc-protocols =0x 0x001e; + linux,rc-map-name = rc-rc6-mce; + }; diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c index 4f71a7d..25e09fa 100644 --- a/drivers/media/rc/gpio-ir-recv.c +++ b/drivers/media/rc/gpio-ir-recv.c @@ -16,6 +16,7 @@ #includelinux/interrupt.h #includelinux/gpio.h #includelinux/slab.h +#includelinux/of_gpio.h #includelinux/platform_device.h #includelinux/irq.h #includemedia/rc-core.h @@ -30,6 +31,63 @@ struct gpio_rc_dev { bool active_low; }; +#ifdef CONFIG_OF +/* + * Translate OpenFirmware node properties into platform_data + */ +static struct gpio_ir_recv_platform_data * +gpio_ir_recv_get_devtree_pdata(struct device *dev) +{ + struct device_node *np = dev-of_node; + struct gpio_ir_recv_platform_data *pdata; + enum of_gpio_flags flags; + int gpio; + + if (!np) + return ERR_PTR(-ENODEV); + + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); + if (!pdata) + return ERR_PTR(-ENOMEM); + + if (!of_find_property(np, gpios, NULL)) { Why do you need this ? Isn't of_get_gpio_flags() sufficient ? Ok. Now that you point at it, I agree that this check and the error below is not needed. It is in gpio_keys, so that explains why it also moved in here. + dev_err(dev, Found gpio-ir-receiver without gpios\n); + return ERR_PTR(-EINVAL); + } + + gpio = of_get_gpio_flags(np, 0,flags); + if (gpio 0) { + if (gpio != -EPROBE_DEFER) + dev_err(dev, Failed to get gpio flags, error: %d\n, + gpio); + return ERR_PTR(gpio); + } + + pdata-gpio_nr = gpio; + pdata-active_low = (flags OF_GPIO_ACTIVE_LOW) ? true : false; + pdata-map_name = of_get_property(np, linux,rc-map-name, NULL); + of_property_read_u64(np, linux,allowed-rc-protocols, + pdata-allowed_protos); + + return pdata; +} + +static struct of_device_id gpio_ir_recv_of_match[] = { + { .compatible = gpio-ir-receiver, }, + { }, +}; +MODULE_DEVICE_TABLE(of, gpio_ir_recv_of_match); + +#else /* !CONFIG_OF */ + +static inline struct gpio_ir_recv_platform_data * +gpio_ir_recv_get_devtree_pdata(struct device *dev) +{ + return ERR_PTR(-ENODEV); +} + +#endif
[PATCH] media: rc: gpio-ir-recv: add support for device tree parsing
This patch adds device tree parsing for gpio_ir_recv platform_data and the mandatory binding documentation. It basically follows what we already have for e.g. gpio_keys. All required device tree properties are OS independent but optional properties allow linux specific support for rc protocols and maps. There was a similar patch sent by Matus Ujhelyi but that discussion died after the first reviews. Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@gmail.com --- Cc: Grant Likely grant.lik...@secretlab.ca Cc: Rob Herring rob.herr...@calxeda.com Cc: Rob Landley r...@landley.net Cc: Mauro Carvalho Chehab mche...@redhat.com Cc: Sebastian Hesselbarth sebastian.hesselba...@gmail.com Cc: Benoit Thebaudeau benoit.thebaud...@advansee.com Cc: David Hardeman da...@hardeman.nu Cc: Trilok Soni ts...@codeaurora.org Cc: devicetree-discuss@lists.ozlabs.org Cc: linux-...@vger.kernel.org Cc: linux-ker...@vger.kernel.org Cc: linux-me...@vger.kernel.org --- .../devicetree/bindings/media/gpio-ir-receiver.txt | 20 ++ drivers/media/rc/gpio-ir-recv.c| 68 +++- 2 files changed, 86 insertions(+), 2 deletions(-) create mode 100644 Documentation/devicetree/bindings/media/gpio-ir-receiver.txt diff --git a/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt b/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt new file mode 100644 index 000..937760c --- /dev/null +++ b/Documentation/devicetree/bindings/media/gpio-ir-receiver.txt @@ -0,0 +1,20 @@ +Device-Tree bindings for GPIO IR receiver + +Required properties: + - compatible = gpio-ir-receiver; + - gpios: OF device-tree gpio specification. + +Optional properties: + - linux,allowed-rc-protocols: Linux specific u64 bitmask of allowed + rc protocols. + - linux,rc-map-name: Linux specific remote control map name. + +Example node: + + ir: ir-receiver { + compatible = gpio-ir-receiver; + gpios = gpio0 19 1; + /* allow rc protocols 1-4 */ + linux,allowed-rc-protocols = 0x 0x001e; + linux,rc-map-name = rc-rc6-mce; + }; diff --git a/drivers/media/rc/gpio-ir-recv.c b/drivers/media/rc/gpio-ir-recv.c index 4f71a7d..25e09fa 100644 --- a/drivers/media/rc/gpio-ir-recv.c +++ b/drivers/media/rc/gpio-ir-recv.c @@ -16,6 +16,7 @@ #include linux/interrupt.h #include linux/gpio.h #include linux/slab.h +#include linux/of_gpio.h #include linux/platform_device.h #include linux/irq.h #include media/rc-core.h @@ -30,6 +31,63 @@ struct gpio_rc_dev { bool active_low; }; +#ifdef CONFIG_OF +/* + * Translate OpenFirmware node properties into platform_data + */ +static struct gpio_ir_recv_platform_data * +gpio_ir_recv_get_devtree_pdata(struct device *dev) +{ + struct device_node *np = dev-of_node; + struct gpio_ir_recv_platform_data *pdata; + enum of_gpio_flags flags; + int gpio; + + if (!np) + return ERR_PTR(-ENODEV); + + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL); + if (!pdata) + return ERR_PTR(-ENOMEM); + + if (!of_find_property(np, gpios, NULL)) { + dev_err(dev, Found gpio-ir-receiver without gpios\n); + return ERR_PTR(-EINVAL); + } + + gpio = of_get_gpio_flags(np, 0, flags); + if (gpio 0) { + if (gpio != -EPROBE_DEFER) + dev_err(dev, Failed to get gpio flags, error: %d\n, + gpio); + return ERR_PTR(gpio); + } + + pdata-gpio_nr = gpio; + pdata-active_low = (flags OF_GPIO_ACTIVE_LOW) ? true : false; + pdata-map_name = of_get_property(np, linux,rc-map-name, NULL); + of_property_read_u64(np, linux,allowed-rc-protocols, +pdata-allowed_protos); + + return pdata; +} + +static struct of_device_id gpio_ir_recv_of_match[] = { + { .compatible = gpio-ir-receiver, }, + { }, +}; +MODULE_DEVICE_TABLE(of, gpio_ir_recv_of_match); + +#else /* !CONFIG_OF */ + +static inline struct gpio_ir_recv_platform_data * +gpio_ir_recv_get_devtree_pdata(struct device *dev) +{ + return ERR_PTR(-ENODEV); +} + +#endif + static irqreturn_t gpio_ir_recv_irq(int irq, void *dev_id) { struct gpio_rc_dev *gpio_dev = dev_id; @@ -66,8 +124,11 @@ static int gpio_ir_recv_probe(struct platform_device *pdev) pdev-dev.platform_data; int rc; - if (!pdata) - return -EINVAL; + if (!pdata) { + pdata = gpio_ir_recv_get_devtree_pdata(pdev-dev); + if (IS_ERR(pdata)) + return PTR_ERR(pdata); + } if (pdata-gpio_nr 0) return -EINVAL; @@ -195,6 +256,9 @@ static struct platform_driver gpio_ir_recv_driver = { #ifdef CONFIG_PM .pm = gpio_ir_recv_pm_ops, #endif +#ifdef
[PATCH v2 1/1] ARM: dove: DT support for sdhci-dove
This patch adds device tree support and binding documentiation for sdhci-dove. v2: extended documentation and removed second interrupt as it is marked 'reserved' in dove datasheet. Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@googlemail.com Cc: Grant Likely grant.lik...@secretlab.ca Cc: Rob Herring rob.herr...@calxeda.com Cc: Rob Landley r...@landley.net Cc: Chris Ball c...@laptop.org Cc: Anton Vorontsov cbouatmai...@gmail.com Cc: Manuel Lauss manuel.la...@googlemail.com Cc: David Brown dav...@codeaurora.org Cc: Andrew Lunn and...@lunn.ch Cc: devicetree-discuss@lists.ozlabs.org Cc: linux-...@vger.kernel.org Cc: linux-ker...@vger.kernel.org Cc: linux-...@vger.kernel.org --- Documentation/devicetree/bindings/mmc/sdhci-dove.txt | 14 ++ drivers/mmc/host/sdhci-dove.c|8 2 files changed, 22 insertions(+) create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-dove.txt diff --git a/Documentation/devicetree/bindings/mmc/sdhci-dove.txt b/Documentation/devicetree/bindings/mmc/sdhci-dove.txt new file mode 100644 index 000..f08bb30 --- /dev/null +++ b/Documentation/devicetree/bindings/mmc/sdhci-dove.txt @@ -0,0 +1,14 @@ +* Marvell sdhci-dove controller + +Required properties: +- compatible: Should be marvell,dove-sdhci. +- reg: Physical base address of the sdhci controller. +- interrupts: interrupt nr of the sdhci controller. + +Example: + +sdio0: sdio@92000 { + compatible = marvell,dove-sdhci; + reg = 0x92000 0x100; + interrupts = 35; +}; diff --git a/drivers/mmc/host/sdhci-dove.c b/drivers/mmc/host/sdhci-dove.c index a6e53a1..90140eb 100644 --- a/drivers/mmc/host/sdhci-dove.c +++ b/drivers/mmc/host/sdhci-dove.c @@ -24,6 +24,7 @@ #include linux/err.h #include linux/module.h #include linux/mmc/host.h +#include linux/of.h #include sdhci-pltfm.h @@ -126,11 +127,18 @@ static int __devexit sdhci_dove_remove(struct platform_device *pdev) return sdhci_pltfm_unregister(pdev); } +static const struct of_device_id sdhci_dove_of_match_table[] __devinitdata = { + { .compatible = marvell,dove-sdhci, }, + {} +}; +MODULE_DEVICE_TABLE(of, sdhci_dove_of_match_table); + static struct platform_driver sdhci_dove_driver = { .driver = { .name = sdhci-dove, .owner = THIS_MODULE, .pm = SDHCI_PLTFM_PMOPS, + .of_match_table = of_match_ptr(sdhci_dove_of_match_table), }, .probe = sdhci_dove_probe, .remove = __devexit_p(sdhci_dove_remove), -- 1.7.10.4 ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH 1/1] ARM: dove: DT support for sdhci-dove
This patch adds device tree support and binding documentiation for sdhci-dove. Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@googlemail.com Cc: Grant Likely grant.lik...@secretlab.ca Cc: Rob Herring rob.herr...@calxeda.com Cc: Rob Landley r...@landley.net Cc: Chris Ball c...@laptop.org Cc: Anton Vorontsov cbouatmai...@gmail.com Cc: Manuel Lauss manuel.la...@googlemail.com Cc: David Brown dav...@codeaurora.org Cc: devicetree-discuss@lists.ozlabs.org Cc: linux-...@vger.kernel.org Cc: linux-ker...@vger.kernel.org Cc: linux-...@vger.kernel.org --- Documentation/devicetree/bindings/mmc/sdhci-dove.txt | 12 drivers/mmc/host/sdhci-dove.c|8 2 files changed, 20 insertions(+) create mode 100644 Documentation/devicetree/bindings/mmc/sdhci-dove.txt diff --git a/Documentation/devicetree/bindings/mmc/sdhci-dove.txt b/Documentation/devicetree/bindings/mmc/sdhci-dove.txt new file mode 100644 index 000..3dd42552 --- /dev/null +++ b/Documentation/devicetree/bindings/mmc/sdhci-dove.txt @@ -0,0 +1,12 @@ +* Marvell sdhci-dove controller + +Required properties: +- compatible: Should be marvell,dove-sdhci. + +Example: + +sdio0: sdio@92000 { + compatible = marvell,dove-sdhci; + reg = 0x92000 0x100; + interrupts = 35, 37; +}; diff --git a/drivers/mmc/host/sdhci-dove.c b/drivers/mmc/host/sdhci-dove.c index a6e53a1..90140eb 100644 --- a/drivers/mmc/host/sdhci-dove.c +++ b/drivers/mmc/host/sdhci-dove.c @@ -24,6 +24,7 @@ #include linux/err.h #include linux/module.h #include linux/mmc/host.h +#include linux/of.h #include sdhci-pltfm.h @@ -126,11 +127,18 @@ static int __devexit sdhci_dove_remove(struct platform_device *pdev) return sdhci_pltfm_unregister(pdev); } +static const struct of_device_id sdhci_dove_of_match_table[] __devinitdata = { + { .compatible = marvell,dove-sdhci, }, + {} +}; +MODULE_DEVICE_TABLE(of, sdhci_dove_of_match_table); + static struct platform_driver sdhci_dove_driver = { .driver = { .name = sdhci-dove, .owner = THIS_MODULE, .pm = SDHCI_PLTFM_PMOPS, + .of_match_table = of_match_ptr(sdhci_dove_of_match_table), }, .probe = sdhci_dove_probe, .remove = __devexit_p(sdhci_dove_remove), -- 1.7.10.4 ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[RESEND PATCH 1/1] clk: add DT support for clock gating control
As Rob's clock binding support patch is now up on clk-next, I'd like to draw attention on this patch again. -- This patch adds support for using clock gates (clk-gate) from DT based on Rob Herrings DT clk binding support for 3.6. It adds a helper function to clk-gate to allocate all resources required by a set of individual clock gates, i.e. register base address and lock. Each clock gate is described as a child of the clock-gating-control in DT and also created by the helper function. Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@googlemail.com Cc: Grant Likely grant.lik...@secretlab.ca Cc: Rob Herring rob.herr...@calxeda.com Cc: Rob Landley r...@landley.net Cc: Mike Turquette mturque...@ti.com Cc: devicetree-discuss@lists.ozlabs.org Cc: linux-...@vger.kernel.org Cc: linux-ker...@vger.kernel.org Cc: linux-arm-ker...@lists.infradead.org --- .../bindings/clock/clock-gating-control.txt| 80 +++ drivers/clk/clk-gate.c | 84 include/linux/clk-provider.h |2 + 3 files changed, 166 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/clock-gating-control.txt diff --git a/Documentation/devicetree/bindings/clock/clock-gating-control.txt b/Documentation/devicetree/bindings/clock/clock-gating-control.txt new file mode 100644 index 000..05f5728 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/clock-gating-control.txt @@ -0,0 +1,80 @@ +Binding for simple clock gating control based on clock gate register with one +bit per clock gate. This is a clock consumer and also a clock provider for a +set of gated clocks that are described as children of this node. Each clock gate +is described by a bit number and polarity to control the gate. + +This binding uses the common clock binding[1]. + +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt + +==Clock gating control== + +Required properties: +- compatible : shall be clock-gating-control. +- reg : should contain the register physical address and length for +the clock gating control. +- clocks : shared parent clock for all gated clocks. +- #clock-cells : from common clock binding; shall be set to 0. +- #address-cells : number of cells required to describe a clock gate; + should be 2. +- #size-cells : should be zero. + +Each individual clock gate bit is described as a child of the +corresponding gating control register with the following properties. + +Required child properties: +- reg : should contain the individual bit and polarity to control +the clock gate. A polarity of 0 means that by setting the +bit to 1 the clock passes through the clock gate while + setting the bit to 0 disables the clock. Any other value + for polarity inverts the meaning of the control bit. + +Optional child properties: +- clocks : overrides the shared parent clock for this clock gate + by the clock passed in this property. +- clock-output-names : from common clock binding; allows to set + a specific name for the gated clock output. + +==Example== + + /* fixed root clock */ + osc: oscillator { + compatible = fixed-clock; + #clock-cells = 0; + clock-frequency = 16667; + }; + + /* sata peripheral clock */ + sata_clk: ext-oscillator { + compatible = fixed-clock; + #clock-cells = 0; + clock-frequency = 2500; + }; + + /* register-based clock gating control */ + gating-control@f10d0038 { + compatible = clock-gating-control; + reg = 0xf10d0038 0x4; + clocks = osc; + #clock-cells = 0; + #address-cells = 2; + #size-cells = 0; + + /* USB0 clock gate on register bit 0 with inverted polarity */ + cg_usb0: clockgate@0 { + reg = 0 1; /* register bit 0, inverted polarity */ + }; + + /* PCIe0 clock gate on register bit 1 with normal polarity +* and named output clock */ + cg_pcie0: clockgate@1 { + reg = 1 0; /* register bit 1, normal polarity */ + clock-output-names = pcie0_clk; + }; + + /* SATA clock gate with different parent clock */ + cg_sata: clockgate@3 { + reg = 3 0; /* register bit 3, normal polarity */ + clocks = sata_clk; + }; + }; diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c index 578465e..1e88907 100644 --- a/drivers/clk/clk-gate.c +++ b/drivers/clk/clk-gate.c @@ -15,6 +15,9 @@ #include linux/io.h #include linux/err.h #include linux/string.h +#include linux/of.h +#include linux/of_address.h +#include linux/of_platform.h /** * DOC: basic gatable clock
[PATCH 1/1] clk: add DT support for clock gating control
This patch adds support for using clock gates (clk-gate) from DT based on Rob Herrings DT clk binding support for 3.6. It adds a helper function to clk-gate to allocate all resources required by a set of individual clock gates, i.e. register base address and lock. Each clock gate is described as a child of the clock-gating-control in DT and also created by the helper function. Signed-off-by: Sebastian Hesselbarth sebastian.hesselba...@googlemail.com Cc: Grant Likely grant.lik...@secretlab.ca Cc: Rob Herring rob.herr...@calxeda.com Cc: Rob Landley r...@landley.net Cc: Mike Turquette mturque...@ti.com Cc: devicetree-discuss@lists.ozlabs.org Cc: linux-...@vger.kernel.org Cc: linux-ker...@vger.kernel.org Cc: linux-arm-ker...@lists.infradead.org --- .../bindings/clock/clock-gating-control.txt| 80 +++ drivers/clk/clk-gate.c | 84 include/linux/clk-provider.h |2 + 3 files changed, 166 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/clock-gating-control.txt diff --git a/Documentation/devicetree/bindings/clock/clock-gating-control.txt b/Documentation/devicetree/bindings/clock/clock-gating-control.txt new file mode 100644 index 000..05f5728 --- /dev/null +++ b/Documentation/devicetree/bindings/clock/clock-gating-control.txt @@ -0,0 +1,80 @@ +Binding for simple clock gating control based on clock gate register with one +bit per clock gate. This is a clock consumer and also a clock provider for a +set of gated clocks that are described as children of this node. Each clock gate +is described by a bit number and polarity to control the gate. + +This binding uses the common clock binding[1]. + +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt + +==Clock gating control== + +Required properties: +- compatible : shall be clock-gating-control. +- reg : should contain the register physical address and length for +the clock gating control. +- clocks : shared parent clock for all gated clocks. +- #clock-cells : from common clock binding; shall be set to 0. +- #address-cells : number of cells required to describe a clock gate; + should be 2. +- #size-cells : should be zero. + +Each individual clock gate bit is described as a child of the +corresponding gating control register with the following properties. + +Required child properties: +- reg : should contain the individual bit and polarity to control +the clock gate. A polarity of 0 means that by setting the +bit to 1 the clock passes through the clock gate while + setting the bit to 0 disables the clock. Any other value + for polarity inverts the meaning of the control bit. + +Optional child properties: +- clocks : overrides the shared parent clock for this clock gate + by the clock passed in this property. +- clock-output-names : from common clock binding; allows to set + a specific name for the gated clock output. + +==Example== + + /* fixed root clock */ + osc: oscillator { + compatible = fixed-clock; + #clock-cells = 0; + clock-frequency = 16667; + }; + + /* sata peripheral clock */ + sata_clk: ext-oscillator { + compatible = fixed-clock; + #clock-cells = 0; + clock-frequency = 2500; + }; + + /* register-based clock gating control */ + gating-control@f10d0038 { + compatible = clock-gating-control; + reg = 0xf10d0038 0x4; + clocks = osc; + #clock-cells = 0; + #address-cells = 2; + #size-cells = 0; + + /* USB0 clock gate on register bit 0 with inverted polarity */ + cg_usb0: clockgate@0 { + reg = 0 1; /* register bit 0, inverted polarity */ + }; + + /* PCIe0 clock gate on register bit 1 with normal polarity +* and named output clock */ + cg_pcie0: clockgate@1 { + reg = 1 0; /* register bit 1, normal polarity */ + clock-output-names = pcie0_clk; + }; + + /* SATA clock gate with different parent clock */ + cg_sata: clockgate@3 { + reg = 3 0; /* register bit 3, normal polarity */ + clocks = sata_clk; + }; + }; diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c index 578465e..1e88907 100644 --- a/drivers/clk/clk-gate.c +++ b/drivers/clk/clk-gate.c @@ -15,6 +15,9 @@ #include linux/io.h #include linux/err.h #include linux/string.h +#include linux/of.h +#include linux/of_address.h +#include linux/of_platform.h /** * DOC: basic gatable clock which can gate and ungate it's ouput @@ -148,3 +151,84 @@ struct clk *clk_register_gate(struct device *dev, const