[PATCH 0/2] net: mv643xx_eth: use managed clk and allocation

2013-04-11 Thread Sebastian Hesselbarth
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

2013-04-11 Thread Sebastian Hesselbarth
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

2013-04-11 Thread Sebastian Hesselbarth
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

2013-04-11 Thread Sebastian Hesselbarth
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

2013-04-10 Thread Sebastian Hesselbarth
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

2013-04-10 Thread Sebastian Hesselbarth

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

2013-04-10 Thread Sebastian Hesselbarth
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

2013-04-10 Thread Sebastian Hesselbarth
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

2013-04-09 Thread Sebastian Hesselbarth
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

2013-04-08 Thread Sebastian Hesselbarth

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

2013-04-08 Thread Sebastian Hesselbarth
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

2013-04-08 Thread Sebastian Hesselbarth

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

2013-04-08 Thread Sebastian Hesselbarth

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

2013-04-07 Thread Sebastian Hesselbarth
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

2013-04-07 Thread Sebastian Hesselbarth

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

2013-04-05 Thread Sebastian Hesselbarth
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

2013-04-05 Thread Sebastian Hesselbarth

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

2013-04-04 Thread Sebastian Hesselbarth
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

2013-04-03 Thread Sebastian Hesselbarth
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

2013-03-25 Thread Sebastian Hesselbarth
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

2013-03-23 Thread Sebastian Hesselbarth
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

2013-03-21 Thread Sebastian Hesselbarth

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

2013-03-21 Thread Sebastian Hesselbarth

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

2013-03-20 Thread Sebastian Hesselbarth

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

2013-03-19 Thread Sebastian Hesselbarth

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

2013-03-18 Thread Sebastian Hesselbarth
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

2013-03-16 Thread Sebastian Hesselbarth
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

2013-03-16 Thread Sebastian Hesselbarth
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

2013-02-27 Thread Sebastian Hesselbarth
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

2013-02-11 Thread Sebastian Hesselbarth

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

2013-02-09 Thread Sebastian Hesselbarth
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

2013-02-08 Thread Sebastian Hesselbarth
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

2013-02-08 Thread Sebastian Hesselbarth

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

2013-02-08 Thread Sebastian Hesselbarth
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

2013-02-06 Thread Sebastian Hesselbarth
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

2013-02-06 Thread Sebastian Hesselbarth

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

2013-02-03 Thread Sebastian Hesselbarth
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

2012-07-31 Thread Sebastian Hesselbarth
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

2012-07-30 Thread Sebastian Hesselbarth
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

2012-07-12 Thread Sebastian Hesselbarth

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

2012-07-08 Thread Sebastian Hesselbarth
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

<    1   2