Re: Fwd: [PATCH 7/8] watchdog: davinci: add "clocks" property

2013-11-16 Thread Guenter Roeck

On 11/06/2013 03:32 AM, ivan.khoronzhuk wrote:

The Keystone arch is using clocks in DT and source clock for watchdog
has to be specified, so add this to binding.

Signed-off-by: Ivan Khoronzhuk 
---
  .../devicetree/bindings/watchdog/davinci-wdt.txt   |5 +
  1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/watchdog/davinci-wdt.txt 
b/Documentation/devicetree/bindings/watchdog/davinci-wdt.txt
index fddced9..4db4d0e 100644
--- a/Documentation/devicetree/bindings/watchdog/davinci-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/davinci-wdt.txt
@@ -7,6 +7,10 @@ Required properties:

  - reg : Should contain WDT registers location and length

+- clocks: phandle reference to the controller clock.
+ Required only for Keystone arch.
+ See clock-bindings.txt
+


Yet another form of formatting. Also, wonder if it makes sense to merge this 
with the patch adding keystone support.


  Optional properties:

  - timeout-sec:Contains the watchdog timeout in seconds
@@ -21,4 +25,5 @@ wdt: wdt@232 {
compatible = "ti,davinci-wdt";
reg = <0x0232 0x80>;
timeout-sec = <30>;
+   clocks = <&clkwdtimer0>;
  };



--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Fwd: [PATCH 8/8] arm: dts: keystone: add watchdog entry

2013-11-16 Thread Guenter Roeck

On 11/06/2013 03:33 AM, ivan.khoronzhuk wrote:

Add watchdog entry to keystone device tree.

Signed-off-by: Ivan Khoronzhuk 


Acked-by: Guenter Roeck 


---
  arch/arm/boot/dts/keystone.dts |6 ++
  1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/keystone.dts b/arch/arm/boot/dts/keystone.dts
index 100bdf5..a6e5f91 100644
--- a/arch/arm/boot/dts/keystone.dts
+++ b/arch/arm/boot/dts/keystone.dts
@@ -179,5 +179,11 @@
interrupts = ;
clocks = <&clkspi>;
};
+
+   wdt: wdt@022f0080 {
+   compatible = "ti,keystone-wdt";
+   reg = <0x022f0080 0x80>;
+   clocks = <&clkwdtimer0>;
+   };
};
  };



--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Fwd: [PATCH 6/8] watchdog: davinci: reuse driver for keystone arch

2013-11-16 Thread Guenter Roeck

On 11/06/2013 03:32 AM, ivan.khoronzhuk wrote:

The keystone arch use the same IP watchdog, so add "ti,keystone-wdt"
compatible and correct identity.

Signed-off-by: Ivan Khoronzhuk 
---
  .../devicetree/bindings/watchdog/davinci-wdt.txt   |   11 +--
  drivers/watchdog/Kconfig   |4 ++--
  drivers/watchdog/davinci_wdt.c |3 ++-
  3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/davinci-wdt.txt 
b/Documentation/devicetree/bindings/watchdog/davinci-wdt.txt
index 1668b6e..fddced9 100644
--- a/Documentation/devicetree/bindings/watchdog/davinci-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/davinci-wdt.txt
@@ -1,13 +1,20 @@
-DaVinci Watchdog Timer (WDT) Controller
+Texas Instruments DaVinci/Keystone Watchdog Timer (WDT) Controller

  Required properties:
-- compatible : Should be "ti,davinci-wdt"
+
+- compatible:  "ti,davinci-wdt"
+   "ti,keystone-wdt"
+
  - reg : Should contain WDT registers location and length


Please use consistent formatting. If you change it, at least change it to be 
consistent.



  Optional properties:

  - timeout-sec:Contains the watchdog timeout in seconds

+Documentation:
+Davinci DM646x - http://www.ti.com/lit/ug/spruer5b/spruer5b.pdf
+Keystone - http://www.ti.com/lit/ug/sprugv5a/sprugv5a.pdf
+
  Examples:

  wdt: wdt@232 {
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 2c954b5..a4fe130 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -270,11 +270,11 @@ config IOP_WATCHDOG

  config DAVINCI_WATCHDOG
tristate "DaVinci watchdog"
-   depends on ARCH_DAVINCI
+   depends on ARCH_DAVINCI || ARCH_KEYSTONE
select WATCHDOG_CORE
help
  Say Y here if to include support for the watchdog timer
- in the DaVinci DM644x/DM646x processors.
+ in the DaVinci DM644x/DM646x or Keystone processors.
  To compile this driver as a module, choose M here: the
  module will be called davinci_wdt.

diff --git a/drivers/watchdog/davinci_wdt.c b/drivers/watchdog/davinci_wdt.c
index a371b2d..e51fd2e 100644
--- a/drivers/watchdog/davinci_wdt.c
+++ b/drivers/watchdog/davinci_wdt.c
@@ -158,7 +158,7 @@ static unsigned int davinci_wdt_status(struct 
watchdog_device *wdd)

  static const struct watchdog_info davinci_wdt_info = {
.options = WDIOF_KEEPALIVEPING,
-   .identity = "DaVinci Watchdog",
+   .identity = "DaVinci/Keystone Watchdog",
  };

  static const struct watchdog_ops davinci_wdt_ops = {
@@ -229,6 +229,7 @@ static int davinci_wdt_remove(struct platform_device *pdev)

  static const struct of_device_id davinci_wdt_of_match[] = {
{ .compatible = "ti,davinci-wdt", },
+   { .compatible = "ti,keystone-wdt", },
{},
  };
  MODULE_DEVICE_TABLE(of, davinci_wdt_of_match);



--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Fwd: [PATCH 5/8] watchdog: davinci: add "timeout-sec" property

2013-11-16 Thread Guenter Roeck

On 11/06/2013 03:32 AM, ivan.khoronzhuk wrote:

Since Davinci WDT has been switched to use WDT core, it became able
to support timeout-sec property, so add it to it's binding description.

Signed-off-by: Ivan Khoronzhuk 
---
  .../devicetree/bindings/watchdog/davinci-wdt.txt   |5 +
  1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/watchdog/davinci-wdt.txt 
b/Documentation/devicetree/bindings/watchdog/davinci-wdt.txt
index 75558cc..1668b6e 100644
--- a/Documentation/devicetree/bindings/watchdog/davinci-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/davinci-wdt.txt
@@ -4,9 +4,14 @@ Required properties:
  - compatible : Should be "ti,davinci-wdt"
  - reg : Should contain WDT registers location and length

+Optional properties:
+


empty line not needed here (and inconsistent).


+- timeout-sec: Contains the watchdog timeout in seconds
+


Formatting is different to other bindings.


  Examples:

  wdt: wdt@232 {
compatible = "ti,davinci-wdt";
reg = <0x0232 0x80>;
+   timeout-sec = <30>;
  };



--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Fwd: [PATCH 4/8] watchdog: davinci: add GET_STATUS option support

2013-11-16 Thread Guenter Roeck

On 11/06/2013 03:32 AM, ivan.khoronzhuk wrote:

When watchdog timer is expired we can know about it thought


thought -> through or with


GET_STATUS ioctl option.

Signed-off-by: Ivan Khoronzhuk 
---
  drivers/watchdog/davinci_wdt.c |   13 +
  1 file changed, 13 insertions(+)

diff --git a/drivers/watchdog/davinci_wdt.c b/drivers/watchdog/davinci_wdt.c
index 6cbf2e1..a371b2d 100644
--- a/drivers/watchdog/davinci_wdt.c
+++ b/drivers/watchdog/davinci_wdt.c
@@ -144,6 +144,18 @@ static unsigned int davinci_wdt_get_timeleft(struct 
watchdog_device *wdd)
return wdd->timeout - timer_counter;
  }

+static unsigned int davinci_wdt_status(struct watchdog_device *wdd)
+{
+   u32 val;
+   struct davinci_wdt_device *davinci_wdt = watchdog_get_drvdata(wdd);
+
+   val = ioread32(davinci_wdt->base + WDTCR);
+   if (val & WDFLAG)
+   return WDIOF_CARDRESET;
+

"Card previously reset the CPU"

Is this really accurate / correct ?

My understanding is that the status is supposed to return the reason for a 
previous reset/reboot,
not the curent condition.


+   return 0;
+}
+
  static const struct watchdog_info davinci_wdt_info = {
.options = WDIOF_KEEPALIVEPING,
.identity = "DaVinci Watchdog",
@@ -155,6 +167,7 @@ static const struct watchdog_ops davinci_wdt_ops = {
.stop   = davinci_wdt_ping,
.ping   = davinci_wdt_ping,
.get_timeleft   = davinci_wdt_get_timeleft,
+   .status = davinci_wdt_status,
  };

  static int davinci_wdt_probe(struct platform_device *pdev)



--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Fwd: [PATCH 3/8] watchdog: davinci: add GET_TIMELEFT option support

2013-11-16 Thread Guenter Roeck

On 11/06/2013 03:31 AM, ivan.khoronzhuk wrote:

Currently, the davinci watchdog can be read while counting,
so we can add ability to report the remaining time before
the system will reboot.

Signed-off-by: Ivan Khoronzhuk 
---
  drivers/watchdog/davinci_wdt.c |   28 
  1 file changed, 28 insertions(+)

diff --git a/drivers/watchdog/davinci_wdt.c b/drivers/watchdog/davinci_wdt.c
index 1fc2093..6cbf2e1 100644
--- a/drivers/watchdog/davinci_wdt.c
+++ b/drivers/watchdog/davinci_wdt.c
@@ -117,6 +117,33 @@ static int davinci_wdt_ping(struct watchdog_device *wdd)
return 0;
  }

+static unsigned int davinci_wdt_get_timeleft(struct watchdog_device *wdd)
+{
+   u64 timer_counter;
+   unsigned long freq;
+   u32 val;
+   struct davinci_wdt_device *davinci_wdt = watchdog_get_drvdata(wdd);
+
+   /* if timeout is occured then return 0 */


is -> has ?


+   val = ioread32(davinci_wdt->base + WDTCR);
+   if (val & WDFLAG)
+   return 0;
+
+   freq = clk_get_rate(davinci_wdt->clk);
+
+   if (!freq) {
+   dev_err(wdd->dev, "clock freq is not set\n");
+   return 0;
+   }
+

This error check doesn't make sense to me; elsewhere the clock rate is not 
validated.
I would suggest to just return 0 here.


+   timer_counter = ioread32(davinci_wdt->base + TIM12);
+   timer_counter |= ((u64)ioread32(davinci_wdt->base + TIM34) << 32);
+
+   do_div(timer_counter, freq);
+
+   return wdd->timeout - timer_counter;
+}
+
  static const struct watchdog_info davinci_wdt_info = {
.options = WDIOF_KEEPALIVEPING,
.identity = "DaVinci Watchdog",
@@ -127,6 +154,7 @@ static const struct watchdog_ops davinci_wdt_ops = {
.start  = davinci_wdt_start,
.stop   = davinci_wdt_ping,
.ping   = davinci_wdt_ping,
+   .get_timeleft   = davinci_wdt_get_timeleft,
  };

  static int davinci_wdt_probe(struct platform_device *pdev)



--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Fwd: [PATCH 2/8] watchdog: davinci: use davinci_wdt_device structure to hold device data

2013-11-16 Thread Guenter Roeck

On 11/06/2013 03:31 AM, ivan.khoronzhuk wrote:

Some SoCs, like Keystone 2, can support more than one WDT and each
watchdog device has to use it's own base address, clock source,
wdd device, so add new davinci_wdt_device structure to hold device
data.

Signed-off-by: Ivan Khoronzhuk 


Reviewed-by: Guenter roeck 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 01/31] ARM: tegra: add missing clock documentation to DT bindings

2013-11-16 Thread Marc Dietrich
Hi Stephen,

On Friday 15 November 2013 13:53:56 Stephen Warren wrote:
> From: Stephen Warren 
> 
> Many of the Tegra DT binding documents say nothing about the clocks or
> clock-names properties, yet those are present and required in DT files.
> This patch simply updates the documentation file to match the implicit
> definition of the binding, based on real-world DT content.
> 
> All Tegra bindings that mention clocks are updated to have consistent
> wording and formatting of the clock-related properties.
> 
> Cc: tred...@nvidia.com
> Cc: pdeschrij...@nvidia.com
> Cc: linux-te...@vger.kernel.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: Rob Herring 
> Cc: Pawel Moll 
> Cc: Mark Rutland 
> Cc: Ian Campbell 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Stephen Warren 
> ---
>  .../bindings/arm/tegra/nvidia,tegra20-pmc.txt  |  1 +
>  .../devicetree/bindings/dma/tegra20-apbdma.txt |  3 ++
>  .../bindings/gpu/nvidia,tegra20-host1x.txt | 61
> ++ .../devicetree/bindings/i2c/nvidia,tegra20-i2c.txt |
> 14 ++---
>  .../bindings/input/nvidia,tegra20-kbc.txt  |  3 ++
>  .../bindings/mmc/nvidia,tegra20-sdhci.txt  |  3 ++
>  .../devicetree/bindings/nvec/nvidia,nvec.txt   |  8 +++
>  .../bindings/pci/nvidia,tegra20-pcie.txt   | 16 +++---
>  .../devicetree/bindings/pwm/nvidia,tegra20-pwm.txt |  3 ++
>  .../devicetree/bindings/rtc/nvidia,tegra20-rtc.txt |  3 ++
>  .../bindings/serial/nvidia,tegra20-hsuart.txt  |  3 ++
>  .../bindings/sound/nvidia,tegra-audio-alc5632.txt  |  7 +--
>  .../bindings/sound/nvidia,tegra-audio-rt5640.txt   |  7 +--
>  .../bindings/sound/nvidia,tegra-audio-wm8753.txt   |  7 +--
>  .../bindings/sound/nvidia,tegra-audio-wm8903.txt   |  7 +--
>  .../bindings/sound/nvidia,tegra-audio-wm9712.txt   |  7 +--
>  .../bindings/sound/nvidia,tegra20-ac97.txt |  4 ++
>  .../bindings/sound/nvidia,tegra20-i2s.txt  |  3 ++
>  .../bindings/sound/nvidia,tegra30-ahub.txt | 21 ++--
>  .../bindings/sound/nvidia,tegra30-i2s.txt  |  5 +-
>  .../bindings/spi/nvidia,tegra114-spi.txt   |  8 ++-
>  .../bindings/spi/nvidia,tegra20-sflash.txt |  4 +-
>  .../bindings/spi/nvidia,tegra20-slink.txt  |  4 +-
>  .../bindings/timer/nvidia,tegra20-timer.txt|  3 ++
>  .../bindings/timer/nvidia,tegra30-timer.txt|  3 ++
>  .../bindings/usb/nvidia,tegra20-ehci.txt   |  3 +-
>  26 files changed, 172 insertions(+), 39 deletions(-)
> 
> diff --git
> a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
> b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt index
> 1608a54e90e1..68ac65f82a1c 100644
> --- a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
> +++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
> @@ -9,6 +9,7 @@ Required properties:
>  - compatible : Should contain "nvidia,tegra-pmc".
>  - reg : Offset and length of the register set for the device
>  - clocks : Must contain an entry for each entry in clock-names.
> +  See ../clocks/clock-bindings.txt for details.
>  - clock-names : Must include the following entries:
>"pclk" (The Tegra clock of that name),
>"clk32k_in" (The 32KHz clock input to Tegra).
> diff --git a/Documentation/devicetree/bindings/dma/tegra20-apbdma.txt
> b/Documentation/devicetree/bindings/dma/tegra20-apbdma.txt index
> 90fa7da525b8..74bfc54bb184 100644
> --- a/Documentation/devicetree/bindings/dma/tegra20-apbdma.txt
> +++ b/Documentation/devicetree/bindings/dma/tegra20-apbdma.txt
> @@ -5,6 +5,8 @@ Required properties:
>  - reg: Should contain DMA registers location and length. This shuld include
> all of the per-channel registers.
>  - interrupts: Should contain all of the per-channel DMA interrupts.
> +- clocks : Must contain one entry, for the module clock.
> +  See ../clocks/clock-bindings.txt for details.
> 
>  Examples:
> 
> @@ -27,4 +29,5 @@ apbdma: dma@6000a000 {
>  0 149 0x04
>  0 150 0x04
>  0 151 0x04 >;
> + clocks = <&tegra_car 34>;
>  };
> diff --git a/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt
> b/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt index
> b4fa934ae3a2..c9a715a75f60 100644
> --- a/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt
> +++ b/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt
> @@ -9,6 +9,8 @@ Required properties:
>  - #size-cells: The number of cells used to represent the size of an address
> range in the host1x address space. Should be 1.
>  - ranges: The mapping of the host1x address space to the CPU address space.
> +- clocks : Must contain one entry, for the module clock.
> +  See ../clocks/clock-bindings.txt for details.
> 
>  The host1x top-level node defines a number of children, each representing
> one of the following host1x client modules:
> @@ -19,6 +21,8 @@ of the following host1x client modul

Re: Fwd: [PATCH 1/8] watchdog: davinci: change driver to use WDT core

2013-11-16 Thread Guenter Roeck

On 11/06/2013 03:31 AM, ivan.khoronzhuk wrote:

To reduce code duplicate and increase code readability use WDT core
code to handle WDT interface.

Remove io_lock as the WDT core uses mutex to lock each wdt device.
Remove wdt_state as the WDT core track state with its own variable.

The watchdog_init_timeout() can read timeout value from timeout-sec
property if the passed value is out of bounds. So set initial
heartbeat value more than max value in order to initialize heartbeat
in next way. If heartbeat is not set thought module parameter, try
to read it's value from WDT node timeout-sec property. If node has
no one, use default value.

The heartbeat is hold in wdd->timeout by WDT core, so use it in
order to set timeout period.

Signed-off-by: Ivan Khoronzhuk 
---
  drivers/watchdog/Kconfig   |1 +
  drivers/watchdog/davinci_wdt.c |  150 ++--
  2 files changed, 38 insertions(+), 113 deletions(-)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index d1d53f3..2c954b5 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -271,6 +271,7 @@ config IOP_WATCHDOG
  config DAVINCI_WATCHDOG
tristate "DaVinci watchdog"
depends on ARCH_DAVINCI
+   select WATCHDOG_CORE
help
  Say Y here if to include support for the watchdog timer
  in the DaVinci DM644x/DM646x processors.
diff --git a/drivers/watchdog/davinci_wdt.c b/drivers/watchdog/davinci_wdt.c
index bead774..a6eef71 100644
--- a/drivers/watchdog/davinci_wdt.c
+++ b/drivers/watchdog/davinci_wdt.c
@@ -3,7 +3,7 @@
   *
   * Watchdog driver for DaVinci DM644x/DM646x processors
   *
- * Copyright (C) 2006 Texas Instruments.
+ * Copyright (C) 2013 Texas Instruments.


2006-2013


   *
   * 2007 (c) MontaVista Software, Inc. This file is licensed under
   * the terms of the GNU General Public License version 2. This program
@@ -15,18 +15,12 @@
  #include 
  #include 
  #include 
-#include 
-#include 
  #include 
  #include 
-#include 
  #include 
-#include 
-#include 
  #include 
  #include 
  #include 
-#include 
  #include 

  #define MODULE_NAME "DAVINCI-WDT: "
@@ -61,31 +55,13 @@
  #define WDKEY_SEQ0(0xa5c6 << 16)
  #define WDKEY_SEQ1(0xda7e << 16)

-static int heartbeat = DEFAULT_HEARTBEAT;
-
-static DEFINE_SPINLOCK(io_lock);
-static unsigned long wdt_status;
-#define WDT_IN_USE0
-#define WDT_OK_TO_CLOSE   1
-#define WDT_REGION_INITED 2
-#define WDT_DEVICE_INITED 3
-
+static int heartbeat = MAX_HEARTBEAT + 1;


Initializing it with 0 (ie not at all) would be just as good. Also see below.


  static void __iomem   *wdt_base;
  struct clk*wdt_clk;
+static struct watchdog_device  wdt_wdd;

-static void wdt_service(void)
-{
-   spin_lock(&io_lock);
-
-   /* put watchdog in service state */
-   iowrite32(WDKEY_SEQ0, wdt_base + WDTCR);
-   /* put watchdog in active state */
-   iowrite32(WDKEY_SEQ1, wdt_base + WDTCR);
-
-   spin_unlock(&io_lock);
-}
-
-static void wdt_enable(void)
+/* davinci_wdt_start - enable watchdog */


That comment doesn't really provide much value.


+static int davinci_wdt_start(struct watchdog_device *wdd)
  {
u32 tgcr;
u32 timer_margin;
@@ -93,8 +69,6 @@ static void wdt_enable(void)

wdt_freq = clk_get_rate(wdt_clk);

-   spin_lock(&io_lock);
-
/* disable, internal clock source */
iowrite32(0, wdt_base + TCR);
/* reset timer, set mode to 64-bit watchdog, and unreset */
@@ -105,9 +79,9 @@ static void wdt_enable(void)
iowrite32(0, wdt_base + TIM12);
iowrite32(0, wdt_base + TIM34);
/* set timeout period */
-   timer_margin = (((u64)heartbeat * wdt_freq) & 0x);
+   timer_margin = (((u64)wdd->timeout * wdt_freq) & 0x);
iowrite32(timer_margin, wdt_base + PRD12);
-   timer_margin = (((u64)heartbeat * wdt_freq) >> 32);
+   timer_margin = (((u64)wdd->timeout * wdt_freq) >> 32);
iowrite32(timer_margin, wdt_base + PRD34);
/* enable run continuously */
iowrite32(ENAMODE12_PERIODIC, wdt_base + TCR);
@@ -119,84 +93,28 @@ static void wdt_enable(void)
iowrite32(WDKEY_SEQ0 | WDEN, wdt_base + WDTCR);
/* put watchdog in active state */
iowrite32(WDKEY_SEQ1 | WDEN, wdt_base + WDTCR);
-
-   spin_unlock(&io_lock);
-}
-
-static int davinci_wdt_open(struct inode *inode, struct file *file)
-{
-   if (test_and_set_bit(WDT_IN_USE, &wdt_status))
-   return -EBUSY;
-
-   wdt_enable();
-
-   return nonseekable_open(inode, file);
+   return 0;
  }

-static ssize_t
-davinci_wdt_write(struct file *file, const char *data, size_t len,
- loff_t *ppos)
+static int davinci_wdt_ping(struct watchdog_device *wdd)
  {
-   if (len)
-   wdt_service();
-
-   return len;
+   /* put watchdog in service state */
+   iowrite32(WDKEY_SEQ0, wdt_base + WDTCR);
+   /* put

Re: [PATCH v5 2/3] of/selftest: Add self tests for manipulation of properties

2013-11-16 Thread Rob Herring
On Fri, Nov 15, 2013 at 11:46 AM, Grant Likely  wrote:
> Adds a few simple test cases to ensure that addition, update and removal
> of device tree node properties works correctly.
>
> Signed-off-by: Grant Likely 
> Cc: Rob Herring 
> Cc: Benjamin Herrenschmidt 
> Cc: David S. Miller 
> Cc: Nathan Fontenot 
> Cc: Pantelis Antoniou 
> ---
>  drivers/of/selftest.c | 62 
> +++
>  1 file changed, 62 insertions(+)

Does this need to depend on or select OF_DYNAMIC?

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] ARM: OMAP: dts: add n8x0 onenand

2013-11-16 Thread Aaro Koskinen
Convert onenand to DT on n8x0 boards.

Signed-off-by: Aaro Koskinen 
---

This patch is based on top of Tony's 2420 DT patches.

 arch/arm/boot/dts/omap2420-n8x0-common.dtsi | 65 +
 arch/arm/mach-omap2/board-n8x0.c| 44 ---
 2 files changed, 65 insertions(+), 44 deletions(-)

diff --git a/arch/arm/boot/dts/omap2420-n8x0-common.dtsi 
b/arch/arm/boot/dts/omap2420-n8x0-common.dtsi
index c539693..89608b2 100644
--- a/arch/arm/boot/dts/omap2420-n8x0-common.dtsi
+++ b/arch/arm/boot/dts/omap2420-n8x0-common.dtsi
@@ -32,3 +32,68 @@
 &i2c2 {
clock-frequency = <40>;
 };
+
+&gpmc {
+   ranges = <0 0 0x0400 0x1000>;
+
+   /* gpio-irq for dma: 26 */
+
+   onenand@0,0 {
+   #address-cells = <1>;
+   #size-cells = <1>;
+   reg = <0 0 0x1000>;
+
+   gpmc,sync-read;
+   gpmc,burst-length = <16>;
+   gpmc,burst-read;
+   gpmc,burst-wrap;
+   gpmc,device-width = <2>;
+   gpmc,mux-add-data = <2>;
+   gpmc,cs-on-ns = <0>;
+   gpmc,cs-rd-off-ns = <127>;
+   gpmc,cs-wr-off-ns = <109>;
+   gpmc,adv-on-ns = <0>;
+   gpmc,adv-rd-off-ns = <18>;
+   gpmc,adv-wr-off-ns = <18>;
+   gpmc,oe-on-ns = <27>;
+   gpmc,oe-off-ns = <127>;
+   gpmc,we-on-ns = <27>;
+   gpmc,we-off-ns = <72>;
+   gpmc,rd-cycle-ns = <145>;
+   gpmc,wr-cycle-ns = <136>;
+   gpmc,access-ns = <118>;
+   gpmc,page-burst-access-ns = <27>;
+   gpmc,bus-turnaround-ns = <0>;
+   gpmc,cycle2cycle-delay-ns = <0>;
+   gpmc,wait-monitoring-ns = <0>;
+   gpmc,clk-activation-ns = <9>;
+   gpmc,sync-clk-ps = <27000>;
+
+   /* MTD partition table corresponding to old board-n8x0 file. */
+   partition@0 {
+   label = "bootloader";
+   reg = <0x 0x0002>;
+   read-only;
+   };
+   partition@1 {
+   label = "config";
+   reg = <0x0002 0x0006>;
+   };
+   partition@2 {
+   label = "kernel";
+   reg = <0x0008 0x0020>;
+   };
+   partition@3 {
+   label = "initfs";
+   reg = <0x0028 0x0040>;
+   };
+   partition@4 {
+   label = "rootfs";
+   reg = <0x0068 0x0f98>;
+   };
+   partition@5 {
+   label = "omap2-onenand";
+   reg = <0x 0x1000>;
+   };
+   };
+};
diff --git a/arch/arm/mach-omap2/board-n8x0.c b/arch/arm/mach-omap2/board-n8x0.c
index 5e04bdc..f7996194 100644
--- a/arch/arm/mach-omap2/board-n8x0.c
+++ b/arch/arm/mach-omap2/board-n8x0.c
@@ -162,49 +162,6 @@ static struct spi_board_info n800_spi_board_info[] 
__initdata = {
},
 };
 
-#if defined(CONFIG_MTD_ONENAND_OMAP2) || \
-   defined(CONFIG_MTD_ONENAND_OMAP2_MODULE)
-
-static struct mtd_partition onenand_partitions[] = {
-   {
-   .name   = "bootloader",
-   .offset = 0,
-   .size   = 0x2,
-   .mask_flags = MTD_WRITEABLE,/* Force read-only */
-   },
-   {
-   .name   = "config",
-   .offset = MTDPART_OFS_APPEND,
-   .size   = 0x6,
-   },
-   {
-   .name   = "kernel",
-   .offset = MTDPART_OFS_APPEND,
-   .size   = 0x20,
-   },
-   {
-   .name   = "initfs",
-   .offset = MTDPART_OFS_APPEND,
-   .size   = 0x40,
-   },
-   {
-   .name   = "rootfs",
-   .offset = MTDPART_OFS_APPEND,
-   .size   = MTDPART_SIZ_FULL,
-   },
-};
-
-static struct omap_onenand_platform_data board_onenand_data[] = {
-   {
-   .cs = 0,
-   .gpio_irq   = 26,
-   .parts  = onenand_partitions,
-   .nr_parts   = ARRAY_SIZE(onenand_partitions),
-   .flags  = ONENAND_SYNC_READ,
-   }
-};
-#endif
-
 #if defined(CONFIG_MENELAUS) &&
\
(defined(CONFIG_MMC_OMAP) || defined(CONFIG_MMC_OMAP_MODULE))
 
@@ -639,7 +596,6 @@ static int __init n8x0_late_initcall(void)
if (!board_caps)
return -ENODEV;
 
-   gpmc_onenand_init(board_onenand_data);
n8x0_mmc_init();
n8x0_usb_init();
 
-- 
1.8.4.3

-

Re: [PATCH v5 2/3] of/selftest: Add self tests for manipulation of properties

2013-11-16 Thread Pantelis Antoniou
Hi Grant,

On Nov 15, 2013, at 7:46 PM, Grant Likely wrote:

> Adds a few simple test cases to ensure that addition, update and removal
> of device tree node properties works correctly.
> 
> Signed-off-by: Grant Likely 
> Cc: Rob Herring 
> Cc: Benjamin Herrenschmidt 
> Cc: David S. Miller 
> Cc: Nathan Fontenot 
> Cc: Pantelis Antoniou 
> ---
> drivers/of/selftest.c | 62 +++
> 1 file changed, 62 insertions(+)
> 
> diff --git a/drivers/of/selftest.c b/drivers/of/selftest.c
> index e21012bde639..cb8d3e722f76 100644
> --- a/drivers/of/selftest.c
> +++ b/drivers/of/selftest.c
> @@ -30,6 +30,67 @@ static struct selftest_results {
>   } \
> }
> 
> +static void __init of_selftest_dynamic(void)
> +{
> + struct device_node *np;
> + struct property *prop;
> +
> + np = of_find_node_by_path("/testcase-data");
> + if (!np) {
> + pr_err("missing testcase data\n");
> + return;
> + }
> +
> + /* Array of 4 properties for the purpose of testing */
> + prop = kzalloc(sizeof(*prop) * 4, GFP_KERNEL);
> + if (!prop) {
> + selftest(0, "kzalloc() failed\n");
> + return;
> + }
> +
> + /* Add a new property - should pass*/
> + prop->name = "new-property";
> + prop->value = "new-property-data";
> + prop->length = strlen(prop->value);
> + selftest(of_add_property(np, prop) == 0, "Adding a new property 
> failed\n");
> +
> + /* Try to add an existing property - should fail */
> + prop++;
> + prop->name = "new-property";
> + prop->value = "new-property-data-should-fail";
> + prop->length = strlen(prop->value);
> + selftest(of_add_property(np, prop) != 0,
> +  "Adding an existing property should have failed\n");
> +
> + /* Try to modify an existing property - should pass */
> + prop->value = "modify-property-data-should-pass";
> + prop->length = strlen(prop->value);
> + selftest(of_update_property(np, prop) == 0,
> +  "Updating an existing property should have passed\n");
> +
> + /* Try to modify non-existent property - should pass*/
> + prop++;
> + prop->name = "modify-property";
> + prop->value = "modify-missing-property-data-should-pass";
> + prop->length = strlen(prop->value);
> + selftest(of_update_property(np, prop) == 0,
> +  "Updating a missing property should have passed\n");
> +
> + /* Remove property - should pass */
> + selftest(of_remove_property(np, prop) == 0,
> +  "Removing a property should have passed\n");
> +
> + /* Adding very large property - should pass */
> + prop++;
> + prop->name = "large-property-PAGE_SIZEx8";
> + prop->length = PAGE_SIZE * 8;
> + prop->value = kzalloc(prop->length, GFP_KERNEL);
> + selftest(prop->value != NULL, "Unable to allocate large buffer\n");
> + if (prop->value)
> + selftest(of_add_property(np, prop) == 0,
> +  "Adding a large property should have passed\n");
> +}
> +
> static void __init of_selftest_parse_phandle_with_args(void)
> {
>   struct device_node *np;
> @@ -312,6 +373,7 @@ static int __init of_selftest(void)
>   of_node_put(np);
> 
>   pr_info("start of selftest - you will see error messages\n");
> + of_selftest_dynamic();
>   of_selftest_parse_phandle_with_args();
>   of_selftest_property_match_string();
>   of_selftest_parse_interrupts();
> -- 
> 1.8.3.2
> 

I like this. I'll add my overlay test cases somewhere around here.

I might need to introduce some new helper functions in order to generate
the overlay completely programmatically.

Regards

-- Pantelis



--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] dtc: Dynamic symbols & fixup support (v2)

2013-11-16 Thread Pantelis Antoniou
Hi Grant,

On Nov 15, 2013, at 9:01 AM, Grant Likely wrote:

> On Tue, 12 Nov 2013 11:20:04 +0100, Pantelis Antoniou 
>  wrote:
>> Hi Grant,
>> 
>> On Nov 11, 2013, at 6:56 PM, Grant Likely wrote:
>> 
>>> On Tue, 29 Oct 2013 21:01:06 +0200, Pantelis Antoniou 
>>>  wrote:
 Enable the generation of symbol & fixup information for
 usage with dynamic DT loading.
 
 Passing the -@ option generates a __symbols__ node at the
 root node of the resulting blob for any node labels used.
 
 When using the /plugin/ tag all unresolved label references
 be tracked in the __fixups__ node, while all local phandle
 references will the tracked in the __local_fixups__ node.
 
 This is sufficient to implement a dynamic DT object loader.
 
 Changes since v1:
 
 * Forward ported to 1.4 version
 * Added manual entries for the -@ option
 
 Signed-off-by: Pantelis Antoniou 
>>> 
>>> Hi Pantelis,
>>> 
>>> This looks pretty good on first look. Comments below.
>>> 
 ---
 Documentation/dt-object-internal.txt | 295 
 +++
 Documentation/manual.txt |  10 ++
>> 
>> [snip]
>> 
 +-
 +
 +Since the device tree is used for booting a number of very different 
 hardware
 +platforms it is imperative that we tread very carefully.
 +
 +2.a) No changes to the Device Tree binary format. We cannot modify the 
 tree
 +format at all and all the information we require should be encoded using 
 device
 +tree itself. We can add nodes that can be safely ignored by both 
 bootloaders and
 +the kernel.
>>> 
>>> Not /entirely/ true. It would be completely fine to bump up the binary
>>> format version for the overlays since they will never be loaded
>>> standalone. If there are specific things that you would like to have in
>>> the marshalled format then go ahead and propose them. Then the overlay
>>> would use the new protocol version, but the base tree would use the
>>> existing one.
>> 
>> OK, I see. I am very hesitant to bump the format version, because both the
>> base DTS and the overlay need to be compiled with symbol support.
>> We can bump the version format for the overlay fragment, but bumping the
>> version for the base DTS will affect bootloaders badly.
> 
> Hmmm, I missed that detail. Why does the base tree require symbol
> support? It looked to me like the overlay had all the resolution
> information needed.
> 

It is because the symbols are in our case the labels of the DTS. We need
to generate a node containing all the labels so that the overlays can use in 
order
to get the target of their insertion point.

> g.
> 

Regards

-- Pantelis

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/8] net: smc91x: Fix device tree based configuration so it's usable

2013-11-16 Thread Tony Lindgren
* Tony Lindgren  [131114 08:09]:
> * Mark Rutland  [131114 03:04]:
> > 
> > In the driver the supported access sizes are not mutually exclusive.  It
> > would be nice for the binding to have the same property.
> 
> Hmm indeed. How about we add reg-io-width-mask:
> 
>   1 = 8-bit access
>   2 = 16-bit access
>   4 = 32-bit access
>   ...
> 
> So for a driver to support 8, 16 and 32-bit access the mask would
> be:
>   reg-io-width-mask = <7>;
> 
> Although the values for reg-io-width would support masks too, it
> might be better to have reg-io-width-mask to avoid confusion.
> 
> Or do you have any better ideas?
>  
> > > +- smsc,nowait : Setup for fast register access with no waits
> > 
> > I'm confused by what this means. When would this be selected, and when
> > wouldn't it be?
> 
> The driver has a module parameter for it and the comments say:
> 
> "nowait  = 0 for normal wait states, 1 eliminates additional wait states"
> 
> Most platforms seem to set it, but the default is to not set it.
> I guess we could that be a module parameter for now as that's a
> timing optimization.

Here's what I was thinking with the reg-io-width-mask. Anybody
have comments on using reg-io-width vs reg-io-width-mask?

Regards,

Tony


From: Tony Lindgren 
Date: Wed, 13 Nov 2013 16:36:37 -0800
Subject: [PATCH] net: smc91x: Fix device tree based configuration so it's usable

Commit 89ce376c6bdc (drivers/net: Use of_match_ptr() macro in smc91x.c)
added minimal device tree support to smc91x, but it's not working on
many platforms because of the lack of some key configuration bits.

Fix the issue by parsing the necessary configuration like the
smc911x driver is doing.

Cc: Nicolas Pitre 
Cc: "David S. Miller" 
Cc: net...@vger.kernel.org
Cc: devicetree@vger.kernel.org
Signed-off-by: Tony Lindgren 

--- a/Documentation/devicetree/bindings/net/smsc-lan91c111.txt
+++ b/Documentation/devicetree/bindings/net/smsc-lan91c111.txt
@@ -8,3 +8,7 @@ Required properties:
 Optional properties:
 - phy-device : phandle to Ethernet phy
 - local-mac-address : Ethernet mac address to use
+- reg-io-width-mask : Mask of sizes (in bytes) of the IO accesses that
+  are supported on the device.  Valid value for SMSC LAN91c111 are
+  1, 2 or 4.  If it's omitted or invalid, the size would be 2 meaning
+  16-bit access only.
--- a/drivers/net/ethernet/smsc/smc91x.c
+++ b/drivers/net/ethernet/smsc/smc91x.c
@@ -82,6 +82,7 @@ static const char version[] =
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -2189,6 +2190,15 @@ static void smc_release_datacs(struct platform_device 
*pdev, struct net_device *
}
 }
 
+#if IS_BUILTIN(CONFIG_OF)
+static const struct of_device_id smc91x_match[] = {
+   { .compatible = "smsc,lan91c94", },
+   { .compatible = "smsc,lan91c111", },
+   {},
+};
+MODULE_DEVICE_TABLE(of, smc91x_match);
+#endif
+
 /*
  * smc_init(void)
  *   Input parameters:
@@ -2203,6 +2213,8 @@ static void smc_release_datacs(struct platform_device 
*pdev, struct net_device *
 static int smc_drv_probe(struct platform_device *pdev)
 {
struct smc91x_platdata *pd = dev_get_platdata(&pdev->dev);
+   struct device_node *np = pdev->dev.of_node;
+   const struct of_device_id *match = NULL;
struct smc_local *lp;
struct net_device *ndev;
struct resource *res, *ires;
@@ -,11 +2234,31 @@ static int smc_drv_probe(struct platform_device *pdev)
 */
 
lp = netdev_priv(ndev);
+   lp->cfg.flags = 0;
 
if (pd) {
memcpy(&lp->cfg, pd, sizeof(lp->cfg));
lp->io_shift = SMC91X_IO_SHIFT(lp->cfg.flags);
-   } else {
+   }
+
+#if IS_BUILTIN(CONFIG_OF)
+   match = of_match_device(of_match_ptr(smc91x_match), &pdev->dev);
+   if (match) {
+   u32 val;
+
+   of_property_read_u32(np, "reg-io-width", &val);
+   if (val == 0)
+   lp->cfg.flags |= SMC91X_USE_16BIT;
+   if (val & 1)
+   lp->cfg.flags |= SMC91X_USE_8BIT;
+   if (val & 2)
+   lp->cfg.flags |= SMC91X_USE_16BIT;
+   if (val & 4)
+   lp->cfg.flags |= SMC91X_USE_32BIT;
+   }
+#endif
+
+   if (!pd && !match) {
lp->cfg.flags |= (SMC_CAN_USE_8BIT)  ? SMC91X_USE_8BIT  : 0;
lp->cfg.flags |= (SMC_CAN_USE_16BIT) ? SMC91X_USE_16BIT : 0;
lp->cfg.flags |= (SMC_CAN_USE_32BIT) ? SMC91X_USE_32BIT : 0;
@@ -2375,15 +2407,6 @@ static int smc_drv_resume(struct device *dev)
return 0;
 }
 
-#ifdef CONFIG_OF
-static const struct of_device_id smc91x_match[] = {
-   { .compatible = "smsc,lan91c94", },
-   { .compatible = "smsc,lan91c111", },
-   {},
-};
-MODULE_DEVICE_TABLE(of, smc91x_match);
-#endif
-
 static struct dev_pm_ops smc_drv_pm_ops = {
.suspend= smc_drv_suspend,
.resume = smc_drv_resume,
--
To unsubscribe from 

Re: [PATCH 4/5] ARM: dts: Add basic support for omap3 LDP zoom1 labrador

2013-11-16 Thread Tony Lindgren
* Sebastian Reichel  [131116 00:05]:
> On Fri, Nov 15, 2013 at 04:36:06PM -0800, Tony Lindgren wrote:
> > Basic things like serial, Ethernet, MMC, NAND, DSS, touchscreen
> > and GPIO keys work.
> > 
> > For twl4030-keypad we're still missing the binding, but
> > support for that should be trivial to add once the driver
> > has been updated.
> 
> DT bindings for twl4030-keypad are being worked on:
> 
> https://lkml.org/lkml/2013/11/8/463
> 
> > MUSB I'm pretty sure I got got to enumerate once, but I
> > suspect the battery charging somehow disrupts it and it's
> > not enumerating in general for some reason.
> 
> Try to add this:
> 
> &usb_otg_hs {
>   phys = <&usb2_phy>;
>   phy-names = "usb2-phy";
> };
> 
> Those were introduced by the new generic PHY framework, see
> for example: https://lkml.org/lkml/2013/9/27/37

Thanks yeah those should be added. No luck with those either
though, I do have MUSB working fine on the 37xx-evm with the
same image.

I seem to get twl4030_usb interrupts on cable insert and remove,
then enabling debugging I see the following:

connect
HW_CONDITIONS 0xe0/224; link 3

disconnect
HW_CONDITIONS 0x60/96; link 4

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] of: make of_get_phy_mode parse 'phy-connection-type'

2013-11-16 Thread Grant Likely
On Fri, 15 Nov 2013 06:23:32 +, Florian Fainelli  
wrote:
> Per the ePAPR v1.1 specification, 'phy-connection-type' is the canonical
> property name for describing an Ethernet to PHY connection type. Make
> sure that of_get_phy_mode() also attempts to parse that property and
> update the comments mentioning 'phy-mode' to also include
> 'phy-connection-type'.
> 
> Acked-by: Grant Likely 
> Signed-off-by: Florian Fainelli 

Applied, thanks

g.

> ---
> Changes since v2:
> - reworked the error condition to look nicer per Grant's suggestion
> - added Grant's Acked-by tag
> - fixed a typo in the commit message on "mentioning"
> 
>  drivers/of/of_net.c | 11 +++
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/of/of_net.c b/drivers/of/of_net.c
> index 8f9be2e..651e249 100644
> --- a/drivers/of/of_net.c
> +++ b/drivers/of/of_net.c
> @@ -13,8 +13,8 @@
>  
>  /**
>   * It maps 'enum phy_interface_t' found in include/linux/phy.h
> - * into the device tree binding of 'phy-mode', so that Ethernet
> - * device driver can get phy interface from device tree.
> + * into the device tree binding of 'phy-mode' or 'phy-connection-type',
> + * so that Ethernet device driver can get phy interface from device tree.
>   */
>  static const char *phy_modes[] = {
>   [PHY_INTERFACE_MODE_NA] = "",
> @@ -36,8 +36,9 @@ static const char *phy_modes[] = {
>   * of_get_phy_mode - Get phy mode for given device_node
>   * @np:  Pointer to the given device_node
>   *
> - * The function gets phy interface string from property 'phy-mode',
> - * and return its index in phy_modes table, or errno in error case.
> + * The function gets phy interface string from property 'phy-mode' or
> + * 'phy-connection-type', and return its index in phy_modes table, or errno 
> in
> + * error case.
>   */
>  int of_get_phy_mode(struct device_node *np)
>  {
> @@ -46,6 +47,8 @@ int of_get_phy_mode(struct device_node *np)
>  
>   err = of_property_read_string(np, "phy-mode", &pm);
>   if (err < 0)
> + err = of_property_read_string(np, "phy-connection-type", &pm);
> + if (err < 0)
>   return err;
>  
>   for (i = 0; i < ARRAY_SIZE(phy_modes); i++)
> -- 
> 1.8.3.2
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 5/5] OF: Introduce utility helper functions

2013-11-16 Thread Pantelis Antoniou
Hi Grant,

On Nov 15, 2013, at 8:27 AM, Grant Likely wrote:

> On Thu, 14 Nov 2013 10:51:05 +0100, Pantelis Antoniou 
>  wrote:
>> Hi Grant,
>> 
>> On Nov 14, 2013, at 2:44 AM, Grant Likely wrote:
>> 
>>> On Wed, 13 Nov 2013 10:03:37 +0100, Pantelis Antoniou 
>>>  wrote:
 On Nov 13, 2013, at 2:34 AM, Grant Likely wrote:
> On Tue, 12 Nov 2013 11:39:08 +0100, Pantelis Antoniou 
>  wrote:
>>> On Tue,  5 Nov 2013 19:50:16 +0200, Pantelis Antoniou 
>>>  wrote:
 +  } else {
 +  pr_warn("%s: node %p cannot be freed; memory is gone\n",
 +  __func__, node);
 +  }
 +}
>>> 
>>> All of the above is potentially dangerous. There is no way to determine
>>> if anything still holds a reference to a node. The proper way to handle
>>> removal of properties is to have a release method when the last
>>> of_node_put is called.
>>> 
>> 
>> This is safe, and expected to be called only on a dynamically created 
>> tree,
>> that's what all the checks against OF_DYNAMIC guard against.
>> 
>> It is not ever meant to be called on an arbitrary tree, created by 
>> unflattening
>> a blob.
> 
> I am talking about when being used on a dynamic tree. The problem is
> when a driver or other code holds a reference to a dynamic nodes, but
> doesn't release it correctly. The memory must not be freed until all of
> the references are relased. OF_DYNAMIC doesn't actually help in that
> case, and it is the reason for of_node_get()/of_node_put()
> 
 
 I know, but even that is not enough. of_node_get()/of_node_put() handles 
 the
 case of references to the nodes, but not what happens with references to
 properties. deadprops is mitigating the problem somewhat, but if we're 
 going
 to go to all the trouble of kobjectification let's do the props as well.
 
 of_get_property could be modified to return a devm_kmalloced copy of the 
 real
 property and that would deal with most of the callers. Of course for
 the small sized scalar data we can avoid the copy.
 
 By using the devm_* interface we also avoid having to mess too much with 
 the callers.
 
 I.e. what about something like devm_of_get_property()?
>>> 
>>> Reference counting is already a horrible pain to keep correct. I don't
>>> see a better way to handle it in the dynamic case, so we're stuck with
>>> it, but I don't want to make it any harder. Adding ref counting to
>>> properties will make it harder than it already is to get the code right.
>>> I'm absolutely fine with a little bit of wasted memory in the form of
>>> deadprops when the alternative is so horrible. References at the node
>>> level is enough granularity.
>>> 
>>> I don't think kduping the property is the solution either. I strongly
>>> suspect that will be far more expensive than the deadprop solution.
>>> 
>> 
>> As long as we can live with deadprops all is fine. Perhaps a 
>> devm_of_get_property()
>> makes sense for new drivers though? What do you think? Perhaps copying to a
>> user supplied buffer as well?
> 
> I still don't think it is necessary. The device lifetime should always
> be shorter than the node lifetime.
> 
>> It's a kind of drag. That means you get handed a device_node pointer you are 
>> not
>> able to free it without having the blob along with the container/accessor of 
>> it.
>> I.e. For the normal case where the blob comes from a request_firmware() call
>> You have to keep the firmware structure around.
>> 
>> Depending on what other method you're going to use tends to make the code a 
>> little
>> bit messier. 
> 
> Understood. Stick with keeping the blob around for now. It can be
> reworkd in the future if necessary since there are no associated
> userspace ABI issues.
> 
> g.

OK, will do.

Regards

-- Pantelis--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [alsa-devel] [RFCv2] ASoC: Add support for BCM2835

2013-11-16 Thread Lars-Peter Clausen
On 11/12/2013 07:41 PM, Florian Meier wrote:
> This driver adds support for digital audio (I2S)
> for the BCM2835 SoC that is used by the
> Raspberry Pi. External audio codecs can be
> connected to the Raspberry Pi via P5 header.
> 
> It relies on cyclic DMA engine support for BCM2835.
> 
> Signed-off-by: Florian Meier 

Looks mostly good in my opinion. A few comments on minor bits and pieces.

> diff --git a/sound/soc/bcm/Kconfig b/sound/soc/bcm/Kconfig
> new file mode 100644
> index 000..93619ec
> --- /dev/null
> +++ b/sound/soc/bcm/Kconfig
> @@ -0,0 +1,15 @@
> +config SND_BCM2835_SOC_I2S
> + tristate
> +
> +config SND_BCM2835_SOC
> + tristate "SoC Audio support for the Broadcom BCM2835 I2S module"
> + depends on ARCH_BCM2835

I think there is no compile time dependency on ARCH_BCM2835. Changing this
to 'depends on ARCH_BCM2835 || COMPILE_TEST' allows to archive better build
test coverage for the driver.

> + select SND_SOC_DMAENGINE_PCM
> + select DMADEVICES
> + select DMA_BCM2835

I don't think its a good idea to select DMADEVICES or DMA_BCM2835 here,
since those are user selectable symbols from another subsystem. Either
'depends on DMA_BCM2835' or nothing. Will I think nothing is better since
there is no compile time dependency.

> + select SND_SOC_GENERIC_DMAENGINE_PCM
> + select REGMAP_MMIO
> + help
> +   Say Y or M if you want to add support for codecs attached to
> +   the BCM2835 I2S interface. You will also need
> +   to select the audio interfaces to support below.
[...]

> +static void bcm2835_i2s_clear_fifos(struct bcm2835_i2s_dev *dev,
> + bool tx, bool rx)
> +{
> + int timeout = 1000;
> + uint32_t syncval;
> + uint32_t csreg;
> + uint32_t i2s_active_state;
> + uint32_t clkreg;
> + uint32_t clk_active_state;
> + uint32_t off;
> + uint32_t clr;
> +
> + off =  tx ? BCM2835_I2S_TXON : 0;
> + off |= rx ? BCM2835_I2S_RXON : 0;
> +
> + clr =  tx ? BCM2835_I2S_TXCLR : 0;
> + clr |= rx ? BCM2835_I2S_RXCLR : 0;
> +
> + /* Backup the current state */
> + regmap_read(dev->i2s_regmap, BCM2835_I2S_CS_A_REG, &csreg);
> + i2s_active_state = csreg & (BCM2835_I2S_RXON | BCM2835_I2S_TXON);
> +
> + regmap_read(dev->clk_regmap, BCM2835_CLK_PCMCTL_REG, &clkreg);
> + clk_active_state = clkreg & BCM2835_CLK_ENAB;
> +
> + /* Start clock if not running */
> + if (!clk_active_state) {
> + regmap_update_bits(dev->clk_regmap, BCM2835_CLK_PCMCTL_REG,
> + BCM2835_CLK_PASSWD_MASK | BCM2835_CLK_ENAB,
> + BCM2835_CLK_PASSWD | BCM2835_CLK_ENAB);
> + }
> +
> + /* Stop I2S module */
> + regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG, off, 0);
> +
> + /*
> +  * Clear the FIFOs
> +  * Requires at least 2 PCM clock cycles to take effect
> +  */
> + regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG, clr, -1);

I think the -1 can be confusing. Better use either clr or 0x. Same
applies to a few other places in the driver.

> +
> + /* Wait for 2 PCM clock cycles */
> +
> + /*
> +  * Toggle the SYNC flag - after 2 PCM clock cycles it can be read back
> +  * FIXME: This does not seem to work for slave mode!
> +  */
> + regmap_read(dev->i2s_regmap, BCM2835_I2S_CS_A_REG, &syncval);
> + syncval &= BCM2835_I2S_SYNC;
> +
> + regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG,
> + BCM2835_I2S_SYNC, ~syncval);
> +
> + /* Wait for the SYNC flag changing it's state */
> + while (timeout > 0) {
> + timeout--;
> +
> + regmap_read(dev->i2s_regmap, BCM2835_I2S_CS_A_REG, &csreg);
> + if ((csreg & BCM2835_I2S_SYNC) != syncval)
> + break;
> + }
> +
> + if (timeout <= 0)
> + dev_err(dev->dev, "I2S SYNC error!\n");
> +
> + /* Stop clock if it was not running before */
> + if (!clk_active_state)
> + bcm2835_i2s_stop_clock(dev);
> +
> + /* Restore I2S state */
> + regmap_update_bits(dev->i2s_regmap, BCM2835_I2S_CS_A_REG,
> + BCM2835_I2S_RXON | BCM2835_I2S_TXON, i2s_active_state);
> +}
> +
[...]
> +static int bcm2835_i2s_dai_probe(struct snd_soc_dai *dai)
> +{
> + struct bcm2835_i2s_dev *dev = snd_soc_dai_get_drvdata(dai);
> +
> + dev->dma_data[SNDRV_PCM_STREAM_PLAYBACK].addr_width =
> + DMA_SLAVE_BUSWIDTH_4_BYTES;
> + dev->dma_data[SNDRV_PCM_STREAM_CAPTURE].addr_width =
> + DMA_SLAVE_BUSWIDTH_4_BYTES;
> +
> + /* TODO other burst parameters possible? */
> + dev->dma_data[SNDRV_PCM_STREAM_PLAYBACK].maxburst = 2;
> + dev->dma_data[SNDRV_PCM_STREAM_CAPTURE].maxburst = 2;

I'd move the initialization of dma_data to the driver probe() function.

> +
> + dai->playback_dma_data = &dev->dma_data[SNDRV_PCM_STREAM_PLAYBACK];
> + dai->capture_dma_data = &dev->dm

Re: [ORLinux] [PATCH] openrisc: Add DTS and defconfig for DE0-Nano

2013-11-16 Thread Olof Kindgren
2013/11/16 Stefan Kristiansson 
>
> On Fri, Nov 15, 2013 at 10:50:18AM +0100, Jonas Bonn wrote:
> > >+
> > >+i2c0: ocores@a000 {
> > >+#address-cells = <1>;
> > >+#size-cells = <0>;
> > >+compatible = "opencores,i2c-ocores";
> >
> > Version number needed.  OpenCores wanted "projectname-rtlsvn###"
> > where ### is the SVN commit number of the RTL directory in the
> > project's source repository.
> >
>
> That will also require a change to the driver.
>
> Using svn commit ids as version info seems a bit too fine grained to me,
> but if that's what's agreed on, then it should be the commit id from the
> projects official repository at opencores.org I think.
>
> Stefan
> ___
> Linux mailing list
> li...@lists.openrisc.net
> http://lists.openrisc.net/listinfo/linux


I agree. I don't like doing versioning with revision numbers. It's too
closely tied to the currently used VCS. The problem is that no one has
bothered to do proper releases of the cores for the last ten years or
so.

But since we are talking about a relatively small amount of cores
here, I think it could be worth to take a quick glance to see if the
latest SVN version is compatible with the latest tagged release. I
would suspect that is the case for the majority of the cores and the
we can just use the latest tag as version. For the other cores we
could
1. Use latest tag + 1 (a bit ugly if the maintainer wants to do a proper release
2. Take over maintainership/fork and just release what's in the trunk
(taking over maintenance is preferrable here to avoid  more repo
confusion)
3. Use SVN revisions

option 2 would be my preferred choice here, given that we get someone
to do the actual work. I could probably help out with a few of the
cores

Jonas, you said that opencores wanted projectname-svnversion. Do you
know where that comes from? My proposal here would conflict with that

//Olof
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] openrisc: Add DTS and defconfig for DE0-Nano

2013-11-16 Thread Stefan Kristiansson
On Fri, Nov 15, 2013 at 10:50:18AM +0100, Jonas Bonn wrote:
> >+
> >+i2c0: ocores@a000 {
> >+#address-cells = <1>;
> >+#size-cells = <0>;
> >+compatible = "opencores,i2c-ocores";
> 
> Version number needed.  OpenCores wanted "projectname-rtlsvn###"
> where ### is the SVN commit number of the RTL directory in the
> project's source repository.
> 

That will also require a change to the driver.

Using svn commit ids as version info seems a bit too fine grained to me,
but if that's what's agreed on, then it should be the commit id from the
projects official repository at opencores.org I think.

Stefan
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3] misc: bmp085: Add missing platform data.

2013-11-16 Thread Dr. H. Nikolaus Schaller

Am 15.11.2013 um 14:58 schrieb Arnd Bergmann:

> On Thursday 14 November 2013, Marek Belisko wrote:
>> DT bindings contains more parameters to set so add them to platform data also
>> to have possibility to use on arch where DT isn't available yet.
>> 
>> Signed-off-by: Marek Belisko 
> 
> Can you give an example of a platform that uses this chip and cannot
> yet use DT in the mainline kernel? 

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/omap3-gta04.dts

exists but is far from being complete, because the transition to DT is really 
complex.

We still need some (private) board file for 3.13 and hope to have everything 
ready
for 3.14. But that depends on speed of acceptance of other drivers because
all DT things are still constantly moving.

So we will have to mix DT+board file for a while.

> If it's only for out-of-tree platforms, I'd prefer to leave this
> patch out of tree as well and put the burden on whoever maintains
> a non-DT platform in a private kernel.
> 
> 
>> diff --git a/include/linux/i2c/bmp085.h b/include/linux/i2c/bmp085.h
>> index b66cb98..addb972 100644
>> --- a/include/linux/i2c/bmp085.h
>> +++ b/include/linux/i2c/bmp085.h
> 
> Shouldn't this be in include/linux/platform_data? 
> 
>   Arnd

-- hns--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/5] ARM: dts: Add basic support for omap3 LDP zoom1 labrador

2013-11-16 Thread Sebastian Reichel
On Fri, Nov 15, 2013 at 04:36:06PM -0800, Tony Lindgren wrote:
> Basic things like serial, Ethernet, MMC, NAND, DSS, touchscreen
> and GPIO keys work.
> 
> For twl4030-keypad we're still missing the binding, but
> support for that should be trivial to add once the driver
> has been updated.

DT bindings for twl4030-keypad are being worked on:

https://lkml.org/lkml/2013/11/8/463

> MUSB I'm pretty sure I got got to enumerate once, but I
> suspect the battery charging somehow disrupts it and it's
> not enumerating in general for some reason.

Try to add this:

&usb_otg_hs {
phys = <&usb2_phy>;
phy-names = "usb2-phy";
};

Those were introduced by the new generic PHY framework, see
for example: https://lkml.org/lkml/2013/9/27/37

> [...]

-- Sebastian


signature.asc
Description: Digital signature