Re: [PATCH v3 3/5] spi: sunxi: Add Allwinner A31 SPI controller driver

2014-02-04 Thread Mark Brown
On Tue, Feb 04, 2014 at 10:09:26AM +0100, Maxime Ripard wrote:
> On Tue, Feb 04, 2014 at 12:21:10AM +, Mark Brown wrote:

> > It isn't awesome, no.  Ideally the runtime PM code would do this but
> > then you couldn't ifdef the operations which as far as I can tell is the
> > main thing people want from disabling it and it gets complicated for
> > devices that genuinely do power up on startup so here we are.

> We discussed it with Kevin on IRC, and he suggested that we move that
> pm_runtime initialization to the SPI core, but I guess that would also
> mean that all drivers shouldn't ifdef the operations, so that the core
> can call the runtime_resume callback directly.

No, that's not going to be robust - it means drivers can't do any power
sequencing of their own.

> However, I don't really get why any driver should be doing so, since
> you still need these functions to at least to the device
> suspend/resume in the probe/remove, and you don't really want to
> duplicate the code.

I don't think it's particularly useful to support disabling runtime PM
in the first place but some drivers do different things when doing
runtime management to those they do on first init - for example there
may be additional steps that only need to be done during first power up.


signature.asc
Description: Digital signature


Re: [PATCH v3 3/5] spi: sunxi: Add Allwinner A31 SPI controller driver

2014-02-04 Thread Maxime Ripard
Hi Mark,

On Tue, Feb 04, 2014 at 12:21:10AM +, Mark Brown wrote:
> On Fri, Jan 31, 2014 at 11:47:04PM +0100, Maxime Ripard wrote:
> > On Fri, Jan 31, 2014 at 12:48:09PM +, Mark Brown wrote:
> > > On Fri, Jan 31, 2014 at 11:55:50AM +0100, Maxime Ripard wrote:
> 
> > > > +   pm_runtime_enable(>dev);
> > > > +   if (!pm_runtime_enabled(>dev)) {
> > > > +   ret = sun6i_spi_runtime_resume(>dev);
> > > > +   if (ret) {
> > > > +   dev_err(>dev, "Couldn't resume the 
> > > > device\n");
> > > > +   return ret;
> > > > +   }
> > > > +   }
> 
> > > No, as discussed don't do this - notice how other drivers aren't written
> > > this way either.  Like I said leave the device powered on startup and
> > > then let it be idled by runtime PM.
> 
> > Well, some SPI drivers are actually written like that (all the tegra
> 
> It's not been done consistently, no - that should be fixed.
> 
> > SPI drivers for example). It's not an excuse, but waking up the device
> > only to put it back in suspend right away seems kind of
> 
> It isn't awesome, no.  Ideally the runtime PM code would do this but
> then you couldn't ifdef the operations which as far as I can tell is the
> main thing people want from disabling it and it gets complicated for
> devices that genuinely do power up on startup so here we are.

We discussed it with Kevin on IRC, and he suggested that we move that
pm_runtime initialization to the SPI core, but I guess that would also
mean that all drivers shouldn't ifdef the operations, so that the core
can call the runtime_resume callback directly.

However, I don't really get why any driver should be doing so, since
you still need these functions to at least to the device
suspend/resume in the probe/remove, and you don't really want to
duplicate the code.

Right now, about half of the SPI drivers using auto_runtime_pm are
using a ifdef, the other half is not.

> > inefficient. Plus, the pm_runtime_idle callback you suggested are
> > actually calling runtime_idle, while we want to call runtime_suspend.
> 
> Yeah, I didn't actually check if I was looking at the right call there.

I was actually wrong, it does so in its very last line.

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


signature.asc
Description: Digital signature


Re: [PATCH v3 3/5] spi: sunxi: Add Allwinner A31 SPI controller driver

2014-02-04 Thread Maxime Ripard
Hi Mark,

On Tue, Feb 04, 2014 at 12:21:10AM +, Mark Brown wrote:
 On Fri, Jan 31, 2014 at 11:47:04PM +0100, Maxime Ripard wrote:
  On Fri, Jan 31, 2014 at 12:48:09PM +, Mark Brown wrote:
   On Fri, Jan 31, 2014 at 11:55:50AM +0100, Maxime Ripard wrote:
 
+   pm_runtime_enable(pdev-dev);
+   if (!pm_runtime_enabled(pdev-dev)) {
+   ret = sun6i_spi_runtime_resume(pdev-dev);
+   if (ret) {
+   dev_err(pdev-dev, Couldn't resume the 
device\n);
+   return ret;
+   }
+   }
 
   No, as discussed don't do this - notice how other drivers aren't written
   this way either.  Like I said leave the device powered on startup and
   then let it be idled by runtime PM.
 
  Well, some SPI drivers are actually written like that (all the tegra
 
 It's not been done consistently, no - that should be fixed.
 
  SPI drivers for example). It's not an excuse, but waking up the device
  only to put it back in suspend right away seems kind of
 
 It isn't awesome, no.  Ideally the runtime PM code would do this but
 then you couldn't ifdef the operations which as far as I can tell is the
 main thing people want from disabling it and it gets complicated for
 devices that genuinely do power up on startup so here we are.

We discussed it with Kevin on IRC, and he suggested that we move that
pm_runtime initialization to the SPI core, but I guess that would also
mean that all drivers shouldn't ifdef the operations, so that the core
can call the runtime_resume callback directly.

However, I don't really get why any driver should be doing so, since
you still need these functions to at least to the device
suspend/resume in the probe/remove, and you don't really want to
duplicate the code.

Right now, about half of the SPI drivers using auto_runtime_pm are
using a ifdef, the other half is not.

  inefficient. Plus, the pm_runtime_idle callback you suggested are
  actually calling runtime_idle, while we want to call runtime_suspend.
 
 Yeah, I didn't actually check if I was looking at the right call there.

I was actually wrong, it does so in its very last line.

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


signature.asc
Description: Digital signature


Re: [PATCH v3 3/5] spi: sunxi: Add Allwinner A31 SPI controller driver

2014-02-04 Thread Mark Brown
On Tue, Feb 04, 2014 at 10:09:26AM +0100, Maxime Ripard wrote:
 On Tue, Feb 04, 2014 at 12:21:10AM +, Mark Brown wrote:

  It isn't awesome, no.  Ideally the runtime PM code would do this but
  then you couldn't ifdef the operations which as far as I can tell is the
  main thing people want from disabling it and it gets complicated for
  devices that genuinely do power up on startup so here we are.

 We discussed it with Kevin on IRC, and he suggested that we move that
 pm_runtime initialization to the SPI core, but I guess that would also
 mean that all drivers shouldn't ifdef the operations, so that the core
 can call the runtime_resume callback directly.

No, that's not going to be robust - it means drivers can't do any power
sequencing of their own.

 However, I don't really get why any driver should be doing so, since
 you still need these functions to at least to the device
 suspend/resume in the probe/remove, and you don't really want to
 duplicate the code.

I don't think it's particularly useful to support disabling runtime PM
in the first place but some drivers do different things when doing
runtime management to those they do on first init - for example there
may be additional steps that only need to be done during first power up.


signature.asc
Description: Digital signature


Re: [PATCH v3 3/5] spi: sunxi: Add Allwinner A31 SPI controller driver

2014-02-03 Thread Mark Brown
On Fri, Jan 31, 2014 at 11:47:04PM +0100, Maxime Ripard wrote:
> On Fri, Jan 31, 2014 at 12:48:09PM +, Mark Brown wrote:
> > On Fri, Jan 31, 2014 at 11:55:50AM +0100, Maxime Ripard wrote:

> > > + pm_runtime_enable(>dev);
> > > + if (!pm_runtime_enabled(>dev)) {
> > > + ret = sun6i_spi_runtime_resume(>dev);
> > > + if (ret) {
> > > + dev_err(>dev, "Couldn't resume the device\n");
> > > + return ret;
> > > + }
> > > + }

> > No, as discussed don't do this - notice how other drivers aren't written
> > this way either.  Like I said leave the device powered on startup and
> > then let it be idled by runtime PM.

> Well, some SPI drivers are actually written like that (all the tegra

It's not been done consistently, no - that should be fixed.

> SPI drivers for example). It's not an excuse, but waking up the device
> only to put it back in suspend right away seems kind of

It isn't awesome, no.  Ideally the runtime PM code would do this but
then you couldn't ifdef the operations which as far as I can tell is the
main thing people want from disabling it and it gets complicated for
devices that genuinely do power up on startup so here we are.

> inefficient. Plus, the pm_runtime_idle callback you suggested are
> actually calling runtime_idle, while we want to call runtime_suspend.

Yeah, I didn't actually check if I was looking at the right call there.


signature.asc
Description: Digital signature


Re: [PATCH v3 3/5] spi: sunxi: Add Allwinner A31 SPI controller driver

2014-02-03 Thread Mark Brown
On Fri, Jan 31, 2014 at 11:47:04PM +0100, Maxime Ripard wrote:
 On Fri, Jan 31, 2014 at 12:48:09PM +, Mark Brown wrote:
  On Fri, Jan 31, 2014 at 11:55:50AM +0100, Maxime Ripard wrote:

   + pm_runtime_enable(pdev-dev);
   + if (!pm_runtime_enabled(pdev-dev)) {
   + ret = sun6i_spi_runtime_resume(pdev-dev);
   + if (ret) {
   + dev_err(pdev-dev, Couldn't resume the device\n);
   + return ret;
   + }
   + }

  No, as discussed don't do this - notice how other drivers aren't written
  this way either.  Like I said leave the device powered on startup and
  then let it be idled by runtime PM.

 Well, some SPI drivers are actually written like that (all the tegra

It's not been done consistently, no - that should be fixed.

 SPI drivers for example). It's not an excuse, but waking up the device
 only to put it back in suspend right away seems kind of

It isn't awesome, no.  Ideally the runtime PM code would do this but
then you couldn't ifdef the operations which as far as I can tell is the
main thing people want from disabling it and it gets complicated for
devices that genuinely do power up on startup so here we are.

 inefficient. Plus, the pm_runtime_idle callback you suggested are
 actually calling runtime_idle, while we want to call runtime_suspend.

Yeah, I didn't actually check if I was looking at the right call there.


signature.asc
Description: Digital signature


Re: [PATCH v3 3/5] spi: sunxi: Add Allwinner A31 SPI controller driver

2014-01-31 Thread Maxime Ripard
On Fri, Jan 31, 2014 at 12:48:09PM +, Mark Brown wrote:
> On Fri, Jan 31, 2014 at 11:55:50AM +0100, Maxime Ripard wrote:
> 
> > +   master = devm_spi_alloc_master(>dev, sizeof(struct sun6i_spi));
> > +   if (!master) {
> > +   dev_err(>dev, "Unable to allocate SPI Master\n");
> > +   return -ENOMEM;
> > +   }
> 
> This now depends on your other series which as I said doesn't look like
> the best approach.

Indeed, I forgot to mention it in the cover letter. My bad.

> 
> > +   pm_runtime_enable(>dev);
> > +   if (!pm_runtime_enabled(>dev)) {
> > +   ret = sun6i_spi_runtime_resume(>dev);
> > +   if (ret) {
> > +   dev_err(>dev, "Couldn't resume the device\n");
> > +   return ret;
> > +   }
> > +   }
> 
> No, as discussed don't do this - notice how other drivers aren't written
> this way either.  Like I said leave the device powered on startup and
> then let it be idled by runtime PM.

Well, some SPI drivers are actually written like that (all the tegra
SPI drivers for example). It's not an excuse, but waking up the device
only to put it back in suspend right away seems kind of
inefficient. Plus, the pm_runtime_idle callback you suggested are
actually calling runtime_idle, while we want to call runtime_suspend.


-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


signature.asc
Description: Digital signature


Re: [PATCH v3 3/5] spi: sunxi: Add Allwinner A31 SPI controller driver

2014-01-31 Thread Mark Brown
On Fri, Jan 31, 2014 at 11:55:50AM +0100, Maxime Ripard wrote:

> + master = devm_spi_alloc_master(>dev, sizeof(struct sun6i_spi));
> + if (!master) {
> + dev_err(>dev, "Unable to allocate SPI Master\n");
> + return -ENOMEM;
> + }

This now depends on your other series which as I said doesn't look like
the best approach.

> + pm_runtime_enable(>dev);
> + if (!pm_runtime_enabled(>dev)) {
> + ret = sun6i_spi_runtime_resume(>dev);
> + if (ret) {
> + dev_err(>dev, "Couldn't resume the device\n");
> + return ret;
> + }
> + }

No, as discussed don't do this - notice how other drivers aren't written
this way either.  Like I said leave the device powered on startup and
then let it be idled by runtime PM.


signature.asc
Description: Digital signature


[PATCH v3 3/5] spi: sunxi: Add Allwinner A31 SPI controller driver

2014-01-31 Thread Maxime Ripard
The Allwinner A31 has a new SPI controller IP compared to the older Allwinner
SoCs.

It supports DMA, but the driver only does PIO for now, and DMA will be
supported eventually.

Signed-off-by: Maxime Ripard 
---
 .../devicetree/bindings/spi/spi-sun6i.txt  |  24 ++
 drivers/spi/Kconfig|   6 +
 drivers/spi/Makefile   |   1 +
 drivers/spi/spi-sun6i.c| 473 +
 4 files changed, 504 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/spi-sun6i.txt
 create mode 100644 drivers/spi/spi-sun6i.c

diff --git a/Documentation/devicetree/bindings/spi/spi-sun6i.txt 
b/Documentation/devicetree/bindings/spi/spi-sun6i.txt
new file mode 100644
index 000..21de73d
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-sun6i.txt
@@ -0,0 +1,24 @@
+Allwinner A31 SPI controller
+
+Required properties:
+- compatible: Should be "allwinner,sun6i-a31-spi".
+- reg: Should contain register location and length.
+- interrupts: Should contain interrupt.
+- clocks: phandle to the clocks feeding the SPI controller. Two are
+  needed:
+  - "ahb": the gated AHB parent clock
+  - "mod": the parent module clock
+- clock-names: Must contain the clock names described just above
+- resets: phandle to the reset controller asserting this device in
+  reset
+
+Example:
+
+spi1: spi@01c69000 {
+   compatible = "allwinner,sun6i-a31-spi";
+   reg = <0x01c69000 0x1000>;
+   interrupts = <0 66 4>;
+   clocks = <_gates 21>, <_clk>;
+   clock-names = "ahb", "mod";
+   resets = <_rst 21>;
+};
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 5072b71..24f3b85 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -446,6 +446,12 @@ config SPI_SIRF
help
  SPI driver for CSR SiRFprimaII SoCs
 
+config SPI_SUN6I
+   tristate "Allwinner A31 SPI controller"
+   depends on ARCH_SUNXI || COMPILE_TEST
+   help
+ This enables using the SPI controller on the Allwinner A31 SoCs.
+
 config SPI_MXS
tristate "Freescale MXS SPI controller"
depends on ARCH_MXS
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 95af48d..13b6ccf 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -70,6 +70,7 @@ obj-$(CONFIG_SPI_SH_HSPI) += spi-sh-hspi.o
 obj-$(CONFIG_SPI_SH_MSIOF) += spi-sh-msiof.o
 obj-$(CONFIG_SPI_SH_SCI)   += spi-sh-sci.o
 obj-$(CONFIG_SPI_SIRF) += spi-sirf.o
+obj-$(CONFIG_SPI_SUN6I)+= spi-sun6i.o
 obj-$(CONFIG_SPI_TEGRA114) += spi-tegra114.o
 obj-$(CONFIG_SPI_TEGRA20_SFLASH)   += spi-tegra20-sflash.o
 obj-$(CONFIG_SPI_TEGRA20_SLINK)+= spi-tegra20-slink.o
diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
new file mode 100644
index 000..1747892
--- /dev/null
+++ b/drivers/spi/spi-sun6i.c
@@ -0,0 +1,473 @@
+/*
+ * Copyright (C) 2012 - 2014 Allwinner Tech
+ * Pan Nan 
+ *
+ * Copyright (C) 2014 Maxime Ripard
+ * Maxime Ripard 
+ *
+ * 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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#define SUN6I_FIFO_DEPTH   128
+
+#define SUN6I_GBL_CTL_REG  0x04
+#define SUN6I_GBL_CTL_BUS_ENABLE   BIT(0)
+#define SUN6I_GBL_CTL_MASTER   BIT(1)
+#define SUN6I_GBL_CTL_TP   BIT(7)
+#define SUN6I_GBL_CTL_RST  BIT(31)
+
+#define SUN6I_TFR_CTL_REG  0x08
+#define SUN6I_TFR_CTL_CPHA BIT(0)
+#define SUN6I_TFR_CTL_CPOL BIT(1)
+#define SUN6I_TFR_CTL_SPOL BIT(2)
+#define SUN6I_TFR_CTL_CS_MASK  0x3
+#define SUN6I_TFR_CTL_CS(cs)   (((cs) & SUN6I_TFR_CTL_CS_MASK) 
<< 4)
+#define SUN6I_TFR_CTL_CS_MANUALBIT(6)
+#define SUN6I_TFR_CTL_CS_LEVEL BIT(7)
+#define SUN6I_TFR_CTL_DHB  BIT(8)
+#define SUN6I_TFR_CTL_FBS  BIT(12)
+#define SUN6I_TFR_CTL_XCH  BIT(31)
+
+#define SUN6I_INT_CTL_REG  0x10
+#define SUN6I_INT_CTL_RF_OVF   BIT(8)
+#define SUN6I_INT_CTL_TC   BIT(12)
+
+#define SUN6I_INT_STA_REG  0x14
+
+#define SUN6I_FIFO_CTL_REG 0x18
+#define SUN6I_FIFO_CTL_RF_RST  BIT(15)
+#define SUN6I_FIFO_CTL_TF_RST  BIT(31)
+
+#define SUN6I_FIFO_STA_REG 0x1c
+#define SUN6I_FIFO_STA_RF_CNT_MASK 0x7f
+#define SUN6I_FIFO_STA_RF_CNT_BITS 0
+#define SUN6I_FIFO_STA_TF_CNT_MASK  

Re: [PATCH v3 3/5] spi: sunxi: Add Allwinner A31 SPI controller driver

2014-01-31 Thread Maxime Ripard
On Fri, Jan 31, 2014 at 12:48:09PM +, Mark Brown wrote:
 On Fri, Jan 31, 2014 at 11:55:50AM +0100, Maxime Ripard wrote:
 
  +   master = devm_spi_alloc_master(pdev-dev, sizeof(struct sun6i_spi));
  +   if (!master) {
  +   dev_err(pdev-dev, Unable to allocate SPI Master\n);
  +   return -ENOMEM;
  +   }
 
 This now depends on your other series which as I said doesn't look like
 the best approach.

Indeed, I forgot to mention it in the cover letter. My bad.

 
  +   pm_runtime_enable(pdev-dev);
  +   if (!pm_runtime_enabled(pdev-dev)) {
  +   ret = sun6i_spi_runtime_resume(pdev-dev);
  +   if (ret) {
  +   dev_err(pdev-dev, Couldn't resume the device\n);
  +   return ret;
  +   }
  +   }
 
 No, as discussed don't do this - notice how other drivers aren't written
 this way either.  Like I said leave the device powered on startup and
 then let it be idled by runtime PM.

Well, some SPI drivers are actually written like that (all the tegra
SPI drivers for example). It's not an excuse, but waking up the device
only to put it back in suspend right away seems kind of
inefficient. Plus, the pm_runtime_idle callback you suggested are
actually calling runtime_idle, while we want to call runtime_suspend.


-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com


signature.asc
Description: Digital signature


[PATCH v3 3/5] spi: sunxi: Add Allwinner A31 SPI controller driver

2014-01-31 Thread Maxime Ripard
The Allwinner A31 has a new SPI controller IP compared to the older Allwinner
SoCs.

It supports DMA, but the driver only does PIO for now, and DMA will be
supported eventually.

Signed-off-by: Maxime Ripard maxime.rip...@free-electrons.com
---
 .../devicetree/bindings/spi/spi-sun6i.txt  |  24 ++
 drivers/spi/Kconfig|   6 +
 drivers/spi/Makefile   |   1 +
 drivers/spi/spi-sun6i.c| 473 +
 4 files changed, 504 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/spi-sun6i.txt
 create mode 100644 drivers/spi/spi-sun6i.c

diff --git a/Documentation/devicetree/bindings/spi/spi-sun6i.txt 
b/Documentation/devicetree/bindings/spi/spi-sun6i.txt
new file mode 100644
index 000..21de73d
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-sun6i.txt
@@ -0,0 +1,24 @@
+Allwinner A31 SPI controller
+
+Required properties:
+- compatible: Should be allwinner,sun6i-a31-spi.
+- reg: Should contain register location and length.
+- interrupts: Should contain interrupt.
+- clocks: phandle to the clocks feeding the SPI controller. Two are
+  needed:
+  - ahb: the gated AHB parent clock
+  - mod: the parent module clock
+- clock-names: Must contain the clock names described just above
+- resets: phandle to the reset controller asserting this device in
+  reset
+
+Example:
+
+spi1: spi@01c69000 {
+   compatible = allwinner,sun6i-a31-spi;
+   reg = 0x01c69000 0x1000;
+   interrupts = 0 66 4;
+   clocks = ahb1_gates 21, spi1_clk;
+   clock-names = ahb, mod;
+   resets = ahb1_rst 21;
+};
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 5072b71..24f3b85 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -446,6 +446,12 @@ config SPI_SIRF
help
  SPI driver for CSR SiRFprimaII SoCs
 
+config SPI_SUN6I
+   tristate Allwinner A31 SPI controller
+   depends on ARCH_SUNXI || COMPILE_TEST
+   help
+ This enables using the SPI controller on the Allwinner A31 SoCs.
+
 config SPI_MXS
tristate Freescale MXS SPI controller
depends on ARCH_MXS
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 95af48d..13b6ccf 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -70,6 +70,7 @@ obj-$(CONFIG_SPI_SH_HSPI) += spi-sh-hspi.o
 obj-$(CONFIG_SPI_SH_MSIOF) += spi-sh-msiof.o
 obj-$(CONFIG_SPI_SH_SCI)   += spi-sh-sci.o
 obj-$(CONFIG_SPI_SIRF) += spi-sirf.o
+obj-$(CONFIG_SPI_SUN6I)+= spi-sun6i.o
 obj-$(CONFIG_SPI_TEGRA114) += spi-tegra114.o
 obj-$(CONFIG_SPI_TEGRA20_SFLASH)   += spi-tegra20-sflash.o
 obj-$(CONFIG_SPI_TEGRA20_SLINK)+= spi-tegra20-slink.o
diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c
new file mode 100644
index 000..1747892
--- /dev/null
+++ b/drivers/spi/spi-sun6i.c
@@ -0,0 +1,473 @@
+/*
+ * Copyright (C) 2012 - 2014 Allwinner Tech
+ * Pan Nan pan...@allwinnertech.com
+ *
+ * Copyright (C) 2014 Maxime Ripard
+ * Maxime Ripard maxime.rip...@free-electrons.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/clk.h
+#include linux/delay.h
+#include linux/device.h
+#include linux/interrupt.h
+#include linux/io.h
+#include linux/module.h
+#include linux/platform_device.h
+#include linux/pm_runtime.h
+#include linux/reset.h
+#include linux/workqueue.h
+
+#include linux/spi/spi.h
+
+#define SUN6I_FIFO_DEPTH   128
+
+#define SUN6I_GBL_CTL_REG  0x04
+#define SUN6I_GBL_CTL_BUS_ENABLE   BIT(0)
+#define SUN6I_GBL_CTL_MASTER   BIT(1)
+#define SUN6I_GBL_CTL_TP   BIT(7)
+#define SUN6I_GBL_CTL_RST  BIT(31)
+
+#define SUN6I_TFR_CTL_REG  0x08
+#define SUN6I_TFR_CTL_CPHA BIT(0)
+#define SUN6I_TFR_CTL_CPOL BIT(1)
+#define SUN6I_TFR_CTL_SPOL BIT(2)
+#define SUN6I_TFR_CTL_CS_MASK  0x3
+#define SUN6I_TFR_CTL_CS(cs)   (((cs)  SUN6I_TFR_CTL_CS_MASK) 
 4)
+#define SUN6I_TFR_CTL_CS_MANUALBIT(6)
+#define SUN6I_TFR_CTL_CS_LEVEL BIT(7)
+#define SUN6I_TFR_CTL_DHB  BIT(8)
+#define SUN6I_TFR_CTL_FBS  BIT(12)
+#define SUN6I_TFR_CTL_XCH  BIT(31)
+
+#define SUN6I_INT_CTL_REG  0x10
+#define SUN6I_INT_CTL_RF_OVF   BIT(8)
+#define SUN6I_INT_CTL_TC   BIT(12)
+
+#define SUN6I_INT_STA_REG  0x14
+
+#define SUN6I_FIFO_CTL_REG 0x18
+#define SUN6I_FIFO_CTL_RF_RST  BIT(15)
+#define 

Re: [PATCH v3 3/5] spi: sunxi: Add Allwinner A31 SPI controller driver

2014-01-31 Thread Mark Brown
On Fri, Jan 31, 2014 at 11:55:50AM +0100, Maxime Ripard wrote:

 + master = devm_spi_alloc_master(pdev-dev, sizeof(struct sun6i_spi));
 + if (!master) {
 + dev_err(pdev-dev, Unable to allocate SPI Master\n);
 + return -ENOMEM;
 + }

This now depends on your other series which as I said doesn't look like
the best approach.

 + pm_runtime_enable(pdev-dev);
 + if (!pm_runtime_enabled(pdev-dev)) {
 + ret = sun6i_spi_runtime_resume(pdev-dev);
 + if (ret) {
 + dev_err(pdev-dev, Couldn't resume the device\n);
 + return ret;
 + }
 + }

No, as discussed don't do this - notice how other drivers aren't written
this way either.  Like I said leave the device powered on startup and
then let it be idled by runtime PM.


signature.asc
Description: Digital signature