Re: [PATCH v3 2/2] i2c: i2c-ocores: Add support for the GRLIB port of the controller and custom getreg and setreg functions

2012-11-15 Thread Andreas Larsson

On 2012-11-13 23:45, Peter Korsgaard wrote:

"Andreas" == Andreas Larsson  writes:


Hi,

  Andreas> The registers in the GRLIB port of the controller are 32-bit
  Andreas> and in big endian byte order. The PRELOW and PREHIGH registers
  Andreas> are merged into one register. The subsequent registers have
  Andreas> their offset decreased accordingly. Hence the register access
  Andreas> needs to be handled in a non-standard manner using custom
  Andreas> getreg and setreg functions.

  Andreas> @@ -257,6 +300,14 @@ static int ocores_i2c_of_probe(struct 
platform_device *pdev,

  Andreas>   of_property_read_u32(pdev->dev.of_node, "reg-io-width",
  Andreas>   &i2c->reg_io_width);
  Andreas> +
  Andreas> + if (of_device_is_compatible(pdev->dev.of_node,
  Andreas> + "aeroflexgaisler,i2cmst")) {
  Andreas> + dev_dbg(&pdev->dev, "GRLIB variant of i2c-ocores\n");
  Andreas> + i2c->setreg = oc_setreg_grlib;
  Andreas> + i2c->getreg = oc_getreg_grlib;
  Andreas> + }
  Andreas> +

Please also update the bindings documentation under
Documentation/devicetree/bindings/i2c.


Sure!



With this core you need to add both aeroflexgaisler,i2cmst and
opencores,i2c-ocores to the compatible property, but the grlib variant
is NOT compatible with i2c-ocores, so that's not really nice.

Adding a type define (TYPE_OCORES / TYPE_GRLIB) and a 2nd of_device_id
entry with .data = TYPE_GRLIB, and then using that in the probe routine
would be nicer. Have a look at i2c-at91.c for an example of a driver
doing something like that.


Yes, that is a good idea. Do you think casting to and from void * in the 
following solution is too ugly and rather have a struct pointed to, or do you 
think that would be unnecessary?


static struct of_device_id ocores_i2c_match[] = {
{
.compatible = "opencores,i2c-ocores",
.data = (void *)TYPE_OCORES,
},
{
.compatible = "aeroflexgaisler,i2cmst",
.data = (void *)TYPE_GRLIB,
},
{},
};
MODULE_DEVICE_TABLE(of, ocores_i2c_match);

static int ocores_i2c_get_type(struct platform_device *pdev)
{
const struct of_device_id *match;

match = of_match_node(ocores_i2c_match, pdev->dev.of_node);
if (match)
return (int)match->data;
else
return TYPE_OCORES;
}

Cheers,
Andreas

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH v4 0/2] i2c: i2c-ocores: Add support for sparc, custom set and get functions, and the GRLIB port of the controller

2012-11-15 Thread Andreas Larsson
On sparc, irqs are not present as an IORESOURCE in the struct platform_device
representation. By using platform_get_irq instead of platform_get_resource the
driver works for sparc.

The GRLIB port of the ocores i2c controller needs custom getreg and setreg
functions to allow for big endian register access and to deal with the fact that
the PRELOW and PREHIGH registers have been merged into one register.

Signed-off-by: Andreas Larsson 

 Changes since v3:
 - Use a separate entry in the of match table for the grlib variant and trigger
   grlib function usage on type put in the data field of that table entry

Andreas Larsson (2):
  i2c: i2c-ocores: Add irq support for sparc
  i2c: i2c-ocores: Add support for the GRLIB port of the controller and
custom getreg and setreg functions

 .../devicetree/bindings/i2c/i2c-ocores.txt |2 +-
 drivers/i2c/busses/i2c-ocores.c|   90 ++--
 2 files changed, 83 insertions(+), 9 deletions(-)

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH v4 1/2] i2c: i2c-ocores: Add irq support for sparc

2012-11-15 Thread Andreas Larsson
Add sparc support by using platform_get_irq instead of platform_get_resource.
There are no platform resources of type IORESOURCE_IRQ for sparc, but
platform_get_irq works for sparc. In the non-sparc case platform_get_irq
internally uses platform_get_resource.

Signed-off-by: Andreas Larsson 
Acked-by: Peter Korsgaard 
---
 Changes since v3:
 - None

 drivers/i2c/busses/i2c-ocores.c |   11 ++-
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index bffd550..1d204cb 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -267,7 +267,8 @@ static int __devinit ocores_i2c_probe(struct 
platform_device *pdev)
 {
struct ocores_i2c *i2c;
struct ocores_i2c_platform_data *pdata;
-   struct resource *res, *res2;
+   struct resource *res;
+   int irq;
int ret;
int i;
 
@@ -275,9 +276,9 @@ static int __devinit ocores_i2c_probe(struct 
platform_device *pdev)
if (!res)
return -ENODEV;
 
-   res2 = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-   if (!res2)
-   return -ENODEV;
+   irq = platform_get_irq(pdev, 0);
+   if (irq < 0)
+   return irq;
 
i2c = devm_kzalloc(&pdev->dev, sizeof(*i2c), GFP_KERNEL);
if (!i2c)
@@ -313,7 +314,7 @@ static int __devinit ocores_i2c_probe(struct 
platform_device *pdev)
ocores_init(i2c);
 
init_waitqueue_head(&i2c->wait);
-   ret = devm_request_irq(&pdev->dev, res2->start, ocores_isr, 0,
+   ret = devm_request_irq(&pdev->dev, irq, ocores_isr, 0,
   pdev->name, i2c);
if (ret) {
dev_err(&pdev->dev, "Cannot claim IRQ\n");
-- 
1.7.0.4

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH v4 2/2] i2c: i2c-ocores: Add support for the GRLIB port of the controller and custom getreg and setreg functions

2012-11-15 Thread Andreas Larsson
The registers in the GRLIB port of the controller are 32-bit and in big endian
byte order. The PRELOW and PREHIGH registers are merged into one register. The
subsequent registers have their offset decreased accordingly. Hence the register
access needs to be handled in a non-standard manner using custom getreg and
setreg functions.

A type is added as the data of the of match table entries. A new entry with a
different compatible string is added to the table. The type of that entry
triggers usage of the grlib functions.

Signed-off-by: Andreas Larsson 
---
 Changes since v3:
 - Use a separate entry in the of match table for the grlib variant and trigger
   grlib function usage on type put in the data field of that table entry

 .../devicetree/bindings/i2c/i2c-ocores.txt |2 +-
 drivers/i2c/busses/i2c-ocores.c|   79 +++-
 2 files changed, 77 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-ocores.txt 
b/Documentation/devicetree/bindings/i2c/i2c-ocores.txt
index c15781f..1637c29 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-ocores.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-ocores.txt
@@ -1,7 +1,7 @@
 Device tree configuration for i2c-ocores
 
 Required properties:
-- compatible  : "opencores,i2c-ocores"
+- compatible  : "opencores,i2c-ocores" or "aeroflexgaisler,i2cmst"
 - reg : bus address start and address range size of device
 - interrupts  : interrupt number
 - clock-frequency : frequency of bus clock in Hz
diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index 1d204cb..fc6e6e7 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -4,6 +4,9 @@
  *
  * Peter Korsgaard 
  *
+ * Support for the GRLIB port of the controller by
+ * Andreas Larsson 
+ *
  * This file is licensed under the terms of the GNU General Public License
  * version 2.  This program is licensed "as is" without any warranty of any
  * kind, whether express or implied.
@@ -38,6 +41,8 @@ struct ocores_i2c {
int nmsgs;
int state; /* see STATE_ */
int clock_khz;
+   void (*setreg)(struct ocores_i2c *i2c, int reg, u8 value);
+   u8 (*getreg)(struct ocores_i2c *i2c, int reg);
 };
 
 /* registers */
@@ -71,9 +76,14 @@ struct ocores_i2c {
 #define STATE_READ 3
 #define STATE_ERROR4
 
+#define TYPE_OCORES0
+#define TYPE_GRLIB 1
+
 static inline void oc_setreg(struct ocores_i2c *i2c, int reg, u8 value)
 {
-   if (i2c->reg_io_width == 4)
+   if (i2c->setreg)
+   i2c->setreg(i2c, reg, value);
+   else if (i2c->reg_io_width == 4)
iowrite32(value, i2c->base + (reg << i2c->reg_shift));
else if (i2c->reg_io_width == 2)
iowrite16(value, i2c->base + (reg << i2c->reg_shift));
@@ -83,7 +93,9 @@ static inline void oc_setreg(struct ocores_i2c *i2c, int reg, 
u8 value)
 
 static inline u8 oc_getreg(struct ocores_i2c *i2c, int reg)
 {
-   if (i2c->reg_io_width == 4)
+   if (i2c->getreg)
+   return i2c->getreg(i2c, reg);
+   else if (i2c->reg_io_width == 4)
return ioread32(i2c->base + (reg << i2c->reg_shift));
else if (i2c->reg_io_width == 2)
return ioread16(i2c->base + (reg << i2c->reg_shift));
@@ -91,6 +103,40 @@ static inline u8 oc_getreg(struct ocores_i2c *i2c, int reg)
return ioread8(i2c->base + (reg << i2c->reg_shift));
 }
 
+/* Read and write functions for the GRLIB port of the controller. Registers are
+ * 32-bit big endian and the PRELOW and PREHIGH registers are merged into one
+ * register. The subsequent registers has their offset decreased accordingly. 
*/
+static u8 oc_getreg_grlib(struct ocores_i2c *i2c, int reg)
+{
+   u32 rd;
+   int rreg = reg;
+   if (reg != OCI2C_PRELOW)
+   rreg--;
+   rd = ioread32be(i2c->base + (rreg << i2c->reg_shift));
+   if (reg == OCI2C_PREHIGH)
+   return (u8)(rd >> 8);
+   else
+   return (u8)rd;
+}
+
+static void oc_setreg_grlib(struct ocores_i2c *i2c, int reg, u8 value)
+{
+   u32 curr, wr;
+   int rreg = reg;
+   if (reg != OCI2C_PRELOW)
+   rreg--;
+   if (reg == OCI2C_PRELOW || reg == OCI2C_PREHIGH) {
+   curr = ioread32be(i2c->base + (rreg << i2c->reg_shift));
+   if (reg == OCI2C_PRELOW)
+   wr = (curr & 0xff00) | value;
+   else
+   wr = (((u32)value) << 8) | (curr & 0xff);
+   } else {
+   wr = value;
+   }
+   iowrite32be(wr, i2c->base + (rreg << i2c->reg_shift));
+}
+
 static void ocores_process(struct ocores_i2c *i2c)
 {
   

[PATCH v5 0/2] i2c: i2c-ocores: Add support for sparc, custom set and get functions, and the GRLIB port of the controller

2012-11-15 Thread Andreas Larsson
On sparc, irqs are not present as an IORESOURCE in the struct platform_device
representation. By using platform_get_irq instead of platform_get_resource the
driver works for sparc.

The GRLIB port of the ocores i2c controller needs custom getreg and setreg
functions to allow for big endian register access and to deal with the fact that
the PRELOW and PREHIGH registers have been merged into one register.

Signed-off-by: Andreas Larsson 

Changes since v4:
- Replace the ocores_get_type function with inline code in the probe function
- Replace the oc_(get|set)reg functions with oc_(get|set)reg_(8|16|32)
  functions and always use the function pointers to call them.

Andreas Larsson (2):
  i2c: i2c-ocores: Add irq support for sparc
  i2c: i2c-ocores: Add support for the GRLIB port of the controller and
use function pointers for getreg and setreg functions

 .../devicetree/bindings/i2c/i2c-ocores.txt |2 +-
 drivers/i2c/busses/i2c-ocores.c|  187 ++--
 2 files changed, 138 insertions(+), 51 deletions(-)

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH v5 2/2] i2c: i2c-ocores: Add support for the GRLIB port of the controller and use function pointers for getreg and setreg functions

2012-11-15 Thread Andreas Larsson
The registers in the GRLIB port of the controller are 32-bit and in big endian
byte order. The PRELOW and PREHIGH registers are merged into one register. The
subsequent registers have their offset decreased accordingly. Hence the register
access needs to be handled in a non-standard manner using custom getreg and
setreg functions.

The single oc_getreg and one oc_setreg functions, that branches and call
different ioread and iowrite functions, are replaced by specific functions that
uses the appropriate ioread and iowrite functions and function pointers are
initiated and used to call the approproate functions.

A type is added as the data of the of match table entries. A new entry with a
different compatible string is added to the table. The type of that entry
triggers usage of the custom grlib functions by setting the new getreg and
setreg function pointers to these functions.

Signed-off-by: Andreas Larsson 
---
 Changes since v4:
 - Replace the ocores_get_type function with inline code in the probe function
 - Replace the oc_(get|set)reg functions with oc_(get|set)reg_(8|16|32)
   functions and always use the function pointers to call them.

 .../devicetree/bindings/i2c/i2c-ocores.txt |2 +-
 drivers/i2c/busses/i2c-ocores.c|  176 +++-
 2 files changed, 132 insertions(+), 46 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-ocores.txt 
b/Documentation/devicetree/bindings/i2c/i2c-ocores.txt
index c15781f..1637c29 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-ocores.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-ocores.txt
@@ -1,7 +1,7 @@
 Device tree configuration for i2c-ocores
 
 Required properties:
-- compatible  : "opencores,i2c-ocores"
+- compatible  : "opencores,i2c-ocores" or "aeroflexgaisler,i2cmst"
 - reg : bus address start and address range size of device
 - interrupts  : interrupt number
 - clock-frequency : frequency of bus clock in Hz
diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index 1d204cb..566193d 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -4,6 +4,9 @@
  *
  * Peter Korsgaard 
  *
+ * Support for the GRLIB port of the controller by
+ * Andreas Larsson 
+ *
  * This file is licensed under the terms of the GNU General Public License
  * version 2.  This program is licensed "as is" without any warranty of any
  * kind, whether express or implied.
@@ -38,6 +41,8 @@ struct ocores_i2c {
int nmsgs;
int state; /* see STATE_ */
int clock_khz;
+   void (*setreg)(struct ocores_i2c *i2c, int reg, u8 value);
+   u8 (*getreg)(struct ocores_i2c *i2c, int reg);
 };
 
 /* registers */
@@ -71,34 +76,82 @@ struct ocores_i2c {
 #define STATE_READ 3
 #define STATE_ERROR4
 
-static inline void oc_setreg(struct ocores_i2c *i2c, int reg, u8 value)
+#define TYPE_OCORES0
+#define TYPE_GRLIB 1
+
+static void oc_setreg_8(struct ocores_i2c *i2c, int reg, u8 value)
 {
-   if (i2c->reg_io_width == 4)
-   iowrite32(value, i2c->base + (reg << i2c->reg_shift));
-   else if (i2c->reg_io_width == 2)
-   iowrite16(value, i2c->base + (reg << i2c->reg_shift));
-   else
-   iowrite8(value, i2c->base + (reg << i2c->reg_shift));
+   iowrite8(value, i2c->base + (reg << i2c->reg_shift));
+}
+
+static void oc_setreg_16(struct ocores_i2c *i2c, int reg, u8 value)
+{
+   iowrite16(value, i2c->base + (reg << i2c->reg_shift));
 }
 
-static inline u8 oc_getreg(struct ocores_i2c *i2c, int reg)
+static void oc_setreg_32(struct ocores_i2c *i2c, int reg, u8 value)
 {
-   if (i2c->reg_io_width == 4)
-   return ioread32(i2c->base + (reg << i2c->reg_shift));
-   else if (i2c->reg_io_width == 2)
-   return ioread16(i2c->base + (reg << i2c->reg_shift));
+   iowrite32(value, i2c->base + (reg << i2c->reg_shift));
+}
+
+static inline u8 oc_getreg_8(struct ocores_i2c *i2c, int reg)
+{
+   return ioread8(i2c->base + (reg << i2c->reg_shift));
+}
+
+static inline u8 oc_getreg_16(struct ocores_i2c *i2c, int reg)
+{
+   return ioread16(i2c->base + (reg << i2c->reg_shift));
+}
+
+static inline u8 oc_getreg_32(struct ocores_i2c *i2c, int reg)
+{
+   return ioread32(i2c->base + (reg << i2c->reg_shift));
+}
+
+
+/* Read and write functions for the GRLIB port of the controller. Registers are
+ * 32-bit big endian and the PRELOW and PREHIGH registers are merged into one
+ * register. The subsequent registers has their offset decreased accordingly. 
*/
+static u8 oc_getreg_grlib(struct ocores_i2c *i2c, int reg)
+{
+   u32 rd;
+   int rreg = reg;
+   if (reg != OCI2C_PRELOW)
+   rreg--;
+   rd 

[PATCH v5 1/2] i2c: i2c-ocores: Add irq support for sparc

2012-11-15 Thread Andreas Larsson
Add sparc support by using platform_get_irq instead of platform_get_resource.
There are no platform resources of type IORESOURCE_IRQ for sparc, but
platform_get_irq works for sparc. In the non-sparc case platform_get_irq
internally uses platform_get_resource.

Signed-off-by: Andreas Larsson 
Acked-by: Peter Korsgaard 
---
 Changes since v4:
 - None

 drivers/i2c/busses/i2c-ocores.c |   11 ++-
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index bffd550..1d204cb 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -267,7 +267,8 @@ static int __devinit ocores_i2c_probe(struct 
platform_device *pdev)
 {
struct ocores_i2c *i2c;
struct ocores_i2c_platform_data *pdata;
-   struct resource *res, *res2;
+   struct resource *res;
+   int irq;
int ret;
int i;
 
@@ -275,9 +276,9 @@ static int __devinit ocores_i2c_probe(struct 
platform_device *pdev)
if (!res)
return -ENODEV;
 
-   res2 = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-   if (!res2)
-   return -ENODEV;
+   irq = platform_get_irq(pdev, 0);
+   if (irq < 0)
+   return irq;
 
i2c = devm_kzalloc(&pdev->dev, sizeof(*i2c), GFP_KERNEL);
if (!i2c)
@@ -313,7 +314,7 @@ static int __devinit ocores_i2c_probe(struct 
platform_device *pdev)
ocores_init(i2c);
 
init_waitqueue_head(&i2c->wait);
-   ret = devm_request_irq(&pdev->dev, res2->start, ocores_isr, 0,
+   ret = devm_request_irq(&pdev->dev, irq, ocores_isr, 0,
   pdev->name, i2c);
if (ret) {
dev_err(&pdev->dev, "Cannot claim IRQ\n");
-- 
1.7.0.4

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH v6 0/2] i2c: i2c-ocores: Add support for sparc, custom set and get functions, and the GRLIB port of the controller

2012-11-15 Thread Andreas Larsson
On sparc, irqs are not present as an IORESOURCE in the struct platform_device
representation. By using platform_get_irq instead of platform_get_resource the
driver works for sparc.

The GRLIB port of the ocores i2c controller needs custom getreg and setreg
functions to allow for big endian register access and to deal with the fact that
the PRELOW and PREHIGH registers have been merged into one register.

Signed-off-by: Andreas Larsson 

Changes since v5:
- Function pointers for different widths are set together
- oc_setreg and oc_getreg are kept as wrappers

Andreas Larsson (2):
  i2c: i2c-ocores: Add irq support for sparc
  i2c: i2c-ocores: Add support for the GRLIB port of the controller and
use function pointers for getreg and setreg functions

 .../devicetree/bindings/i2c/i2c-ocores.txt |2 +-
 drivers/i2c/busses/i2c-ocores.c|  149 +---
 2 files changed, 127 insertions(+), 24 deletions(-)

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH v6 1/2] i2c: i2c-ocores: Add irq support for sparc

2012-11-15 Thread Andreas Larsson
Add sparc support by using platform_get_irq instead of platform_get_resource.
There are no platform resources of type IORESOURCE_IRQ for sparc, but
platform_get_irq works for sparc. In the non-sparc case platform_get_irq
internally uses platform_get_resource.

Signed-off-by: Andreas Larsson 
Acked-by: Peter Korsgaard 
---
 Changes since v5:
 - None

 drivers/i2c/busses/i2c-ocores.c |   11 ++-
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index bffd550..1d204cb 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -267,7 +267,8 @@ static int __devinit ocores_i2c_probe(struct 
platform_device *pdev)
 {
struct ocores_i2c *i2c;
struct ocores_i2c_platform_data *pdata;
-   struct resource *res, *res2;
+   struct resource *res;
+   int irq;
int ret;
int i;
 
@@ -275,9 +276,9 @@ static int __devinit ocores_i2c_probe(struct 
platform_device *pdev)
if (!res)
return -ENODEV;
 
-   res2 = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-   if (!res2)
-   return -ENODEV;
+   irq = platform_get_irq(pdev, 0);
+   if (irq < 0)
+   return irq;
 
i2c = devm_kzalloc(&pdev->dev, sizeof(*i2c), GFP_KERNEL);
if (!i2c)
@@ -313,7 +314,7 @@ static int __devinit ocores_i2c_probe(struct 
platform_device *pdev)
ocores_init(i2c);
 
init_waitqueue_head(&i2c->wait);
-   ret = devm_request_irq(&pdev->dev, res2->start, ocores_isr, 0,
+   ret = devm_request_irq(&pdev->dev, irq, ocores_isr, 0,
   pdev->name, i2c);
if (ret) {
dev_err(&pdev->dev, "Cannot claim IRQ\n");
-- 
1.7.0.4

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH v6 2/2] i2c: i2c-ocores: Add support for the GRLIB port of the controller and use function pointers for getreg and setreg functions

2012-11-15 Thread Andreas Larsson
The registers in the GRLIB port of the controller are 32-bit and in big endian
byte order. The PRELOW and PREHIGH registers are merged into one register. The
subsequent registers have their offset decreased accordingly. Hence the register
access needs to be handled in a non-standard manner using custom getreg and
setreg functions.

Add setreg and getreg functions for different register widths and let oc_setreg
and oc_getreg use function pointers to call the appropriate functions.

A type is added as the data of the of match table entries. A new entry with a
different compatible string is added to the table. The type of that entry
triggers usage of the custom grlib functions by setting the setreg and getreg
function pointers.

Signed-off-by: Andreas Larsson 
---
 Changes since v5:
 - Function pointers for different widths are set together
 - oc_setreg and oc_getreg are kept as wrappers

 .../devicetree/bindings/i2c/i2c-ocores.txt |2 +-
 drivers/i2c/busses/i2c-ocores.c|  138 +---
 2 files changed, 121 insertions(+), 19 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-ocores.txt 
b/Documentation/devicetree/bindings/i2c/i2c-ocores.txt
index c15781f..1637c29 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-ocores.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-ocores.txt
@@ -1,7 +1,7 @@
 Device tree configuration for i2c-ocores
 
 Required properties:
-- compatible  : "opencores,i2c-ocores"
+- compatible  : "opencores,i2c-ocores" or "aeroflexgaisler,i2cmst"
 - reg : bus address start and address range size of device
 - interrupts  : interrupt number
 - clock-frequency : frequency of bus clock in Hz
diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index 1d204cb..2ef318b 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -4,6 +4,9 @@
  *
  * Peter Korsgaard 
  *
+ * Support for the GRLIB port of the controller by
+ * Andreas Larsson 
+ *
  * This file is licensed under the terms of the GNU General Public License
  * version 2.  This program is licensed "as is" without any warranty of any
  * kind, whether express or implied.
@@ -38,6 +41,8 @@ struct ocores_i2c {
int nmsgs;
int state; /* see STATE_ */
int clock_khz;
+   void (*setreg)(struct ocores_i2c *i2c, int reg, u8 value);
+   u8 (*getreg)(struct ocores_i2c *i2c, int reg);
 };
 
 /* registers */
@@ -71,24 +76,81 @@ struct ocores_i2c {
 #define STATE_READ 3
 #define STATE_ERROR4
 
+#define TYPE_OCORES0
+#define TYPE_GRLIB 1
+
+static void oc_setreg_8(struct ocores_i2c *i2c, int reg, u8 value)
+{
+   iowrite8(value, i2c->base + (reg << i2c->reg_shift));
+}
+
+static void oc_setreg_16(struct ocores_i2c *i2c, int reg, u8 value)
+{
+   iowrite16(value, i2c->base + (reg << i2c->reg_shift));
+}
+
+static void oc_setreg_32(struct ocores_i2c *i2c, int reg, u8 value)
+{
+   iowrite32(value, i2c->base + (reg << i2c->reg_shift));
+}
+
+static inline u8 oc_getreg_8(struct ocores_i2c *i2c, int reg)
+{
+   return ioread8(i2c->base + (reg << i2c->reg_shift));
+}
+
+static inline u8 oc_getreg_16(struct ocores_i2c *i2c, int reg)
+{
+   return ioread16(i2c->base + (reg << i2c->reg_shift));
+}
+
+static inline u8 oc_getreg_32(struct ocores_i2c *i2c, int reg)
+{
+   return ioread32(i2c->base + (reg << i2c->reg_shift));
+}
+
+/* Read and write functions for the GRLIB port of the controller. Registers are
+ * 32-bit big endian and the PRELOW and PREHIGH registers are merged into one
+ * register. The subsequent registers has their offset decreased accordingly. 
*/
+static u8 oc_getreg_grlib(struct ocores_i2c *i2c, int reg)
+{
+   u32 rd;
+   int rreg = reg;
+   if (reg != OCI2C_PRELOW)
+   rreg--;
+   rd = ioread32be(i2c->base + (rreg << i2c->reg_shift));
+   if (reg == OCI2C_PREHIGH)
+   return (u8)(rd >> 8);
+   else
+   return (u8)rd;
+}
+
+static void oc_setreg_grlib(struct ocores_i2c *i2c, int reg, u8 value)
+{
+   u32 curr, wr;
+   int rreg = reg;
+   if (reg != OCI2C_PRELOW)
+   rreg--;
+   if (reg == OCI2C_PRELOW || reg == OCI2C_PREHIGH) {
+   curr = ioread32be(i2c->base + (rreg << i2c->reg_shift));
+   if (reg == OCI2C_PRELOW)
+   wr = (curr & 0xff00) | value;
+   else
+   wr = (((u32)value) << 8) | (curr & 0xff);
+   } else {
+   wr = value;
+   }
+   iowrite32be(wr, i2c->base + (rreg << i2c->reg_shift));
+}
+
 static inline void oc_setreg(struct ocores_i2c *i2c, int reg, u8 value)
 {
-   if (i2c->reg_io_width == 4)
-   iowrite3

Re: [PATCH v9] can: grcan: Add device driver for GRCAN and GRHCAN cores

2012-11-15 Thread Andreas Larsson

On 2012-11-15 21:32, Marc Kleine-Budde wrote:

On 11/15/2012 08:47 AM, Andreas Larsson wrote:

This driver supports GRCAN and CRHCAN CAN controllers available in the GRLIB
VHDL IP core library.

Signed-off-by: Andreas Larsson 
Acked-by: Wolfgang Grandegger 


Applied to can-next/master - and included in the pull-reqeust I just
sent to David.


Thank you, both for applying it and for the feedback.

Cheers,
Andreas

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH] of/address: sparc: Declare of_iomap as an extern function for sparc again

2012-11-23 Thread Andreas Larsson
This bug-fix makes sure that of_iomap is defined extern for sparc so that the
sparc-specific implementation of_iomap is once again used when including
include/linux/of_address.h in a sparc context. OF_GPIO that is now available for
sparc relies on this.

The bug was inadvertently introduced in a850a75, "of/address: add empty static
inlines for !CONFIG_OF", that added a static dummy inline for of_iomap when
!CONFIG_OF_ADDRESS. However, CONFIG_OF_ADDRESS is never defined for sparc, but
there is a sparc-specific implementation /arch/sparc/kernel/of_device_common.c.

This fix takes the same approach as 0bce04b that solved the equivalent problem
for of_address_to_resource.

Signed-off-by: Andreas Larsson 
---
 arch/sparc/include/asm/prom.h |5 -
 include/linux/of_address.h|2 ++
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/arch/sparc/include/asm/prom.h b/arch/sparc/include/asm/prom.h
index f930031..67c6257 100644
--- a/arch/sparc/include/asm/prom.h
+++ b/arch/sparc/include/asm/prom.h
@@ -63,10 +63,13 @@ extern char *of_console_options;
 extern void irq_trans_init(struct device_node *dp);
 extern char *build_path_component(struct device_node *dp);
 
-/* SPARC has a local implementation */
+/* SPARC has local implementations */
 extern int of_address_to_resource(struct device_node *dev, int index,
  struct resource *r);
 #define of_address_to_resource of_address_to_resource
 
+void __iomem *of_iomap(struct device_node *node, int index);
+#define of_iomap of_iomap
+
 #endif /* __KERNEL__ */
 #endif /* _SPARC_PROM_H */
diff --git a/include/linux/of_address.h b/include/linux/of_address.h
index e20e3af..0506eb5 100644
--- a/include/linux/of_address.h
+++ b/include/linux/of_address.h
@@ -42,10 +42,12 @@ static inline struct device_node 
*of_find_matching_node_by_address(
 {
return NULL;
 }
+#ifndef of_iomap
 static inline void __iomem *of_iomap(struct device_node *device, int index)
 {
return NULL;
 }
+#endif
 static inline const __be32 *of_get_address(struct device_node *dev, int index,
u64 *size, unsigned int *flags)
 {
-- 
1.7.0.4

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH 2/2] spi: sparc: Allow of_register_spi_devices for sparc

2012-12-04 Thread Andreas Larsson
Signed-off-by: Andreas Larsson 
---
 drivers/spi/spi.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index c4f7d71..d8339fe 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -812,7 +812,7 @@ err_init_queue:
 
 /*-*/
 
-#if defined(CONFIG_OF) && !defined(CONFIG_SPARC)
+#if defined(CONFIG_OF)
 /**
  * of_register_spi_devices() - Register child devices onto the SPI bus
  * @master:Pointer to spi_master device
-- 
1.7.0.4

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH 0/2] sparc: Enable OF functionality for sparc for i2c and spi

2012-12-04 Thread Andreas Larsson
This series removes the dependency on !SPARC for OF_I2C and removes the
depencency of !defined(CONFIG_SPARC) for the function
of_register_spi_devices. I find no reason for these to be unavailable
for sparc.

I am not sure if these should go through the sparc tree or the
corresponding subsystem trees.

Andreas Larsson (2):
  of_i2c: sparc: Allow OF_I2C for sparc
  spi: sparc: Allow of_register_spi_devices for sparc

 drivers/of/Kconfig |2 +-
 drivers/spi/spi.c  |2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH 1/2] of_i2c: sparc: Allow OF_I2C for sparc

2012-12-04 Thread Andreas Larsson
Signed-off-by: Andreas Larsson 
---
 drivers/of/Kconfig |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index dfba3e6..d37bfcf 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -53,7 +53,7 @@ config OF_DEVICE
 
 config OF_I2C
def_tristate I2C
-   depends on I2C && !SPARC
+   depends on I2C
help
  OpenFirmware I2C accessors
 
-- 
1.7.0.4

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH 0/2] sparc: Enable OF functionality for sparc for i2c and spi

2012-12-04 Thread Andreas Larsson

On 2012-12-04 15:44, Grant Likely wrote:

On Tue, Dec 4, 2012 at 2:09 PM, Andreas Larsson  wrote:

This series removes the dependency on !SPARC for OF_I2C and removes the
depencency of !defined(CONFIG_SPARC) for the function
of_register_spi_devices. I find no reason for these to be unavailable
for sparc.

I am not sure if these should go through the sparc tree or the
corresponding subsystem trees.


They should go through the subsystem trees.

What build testing have you done on these patches?


I have built under sparc32 (leon sparc to be specific) and have 
confirmed that of_register_spi_devices and of_i2c_register_devices works 
fine.


Cheers,
Andreas

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH 0/6] of, of_gpio, of_spi: Fix and improve of_parse_phandle_with_args, of_gpio_named_count and of_spi_register_master

2012-12-27 Thread Andreas Larsson
This patch series fixes a bug where of_gpio_named count relied upon a return
value that was no longer returned from of_parse_phandle_with_args and adds the
possibility for of_gpio_named_count to return error values.

In addition, for of_spi_register_master it fixes a bug, adds documentation,
adds fetching of gpio flags and initializes gpio values to be consistent with
return values from of_parse_phandle_with_args.

Tested on sparc. Build tested on x86, arm and ppc.

Andreas Larsson (6):
  of: Return -EEXIST from of_parse_phandle_with_args for holes in
phandle list
  of: Return -ENXIO from of_parse_phandle_with_args for too large index
and return errors from of_gpio_named_count
  of_spi: Initialize cs_gpios properly
  of_spi: Document cs_gpios and cs_gpio in kernel-doc
  of_spi: Add fetching of of_gpio flags to of_spi_register_master
  of_spi: Initialize cs_gpios and cs_gpio with -EEXIST

 Documentation/devicetree/bindings/spi/spi-bus.txt |3 +-
 drivers/gpio/gpiolib-of.c |8 +++--
 drivers/hwmon/gpio-fan.c  |2 +-
 drivers/of/base.c |9 +++--
 drivers/of/selftest.c |2 +-
 drivers/spi/spi.c |   40 -
 include/linux/spi/spi.h   |   10 +
 7 files changed, 56 insertions(+), 18 deletions(-)

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH 1/6] of: Return -EEXIST from of_parse_phandle_with_args for holes in phandle list

2012-12-27 Thread Andreas Larsson
Return value for an empty phandle was -EEXIST before commit 15c9a0ac, that
changed the return value in this case to -ENOENT. However, of_gpio_named_count
relies upon the return value to be -EEXIST and relies upon being able to
distinguish this case from the case of no list at all which also returns
-ENOENT.

Also change the of selftest to expect -EEXIST in this case.

Signed-off-by: Andreas Larsson 
---

I have not run-tested the selftest, not having appropriate hardware around for 
that.

 drivers/of/base.c |4 ++--
 drivers/of/selftest.c |2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 2390ddb..986afd7 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1083,11 +1083,11 @@ int of_parse_phandle_with_args(const struct device_node 
*np, const char *list_na
 * All of the error cases above bail out of the loop, so at
 * this point, the parsing is successful. If the requested
 * index matches, then fill the out_args structure and return,
-* or return -ENOENT for an empty entry.
+* or return -EEXIST for an empty entry.
 */
if (cur_index == index) {
if (!phandle)
-   return -ENOENT;
+   return -EEXIST;
 
if (out_args) {
int i;
diff --git a/drivers/of/selftest.c b/drivers/of/selftest.c
index f24ffd7..b1c2ae9 100644
--- a/drivers/of/selftest.c
+++ b/drivers/of/selftest.c
@@ -54,7 +54,7 @@ static void __init of_selftest_parse_phandle_with_args(void)
passed &= (args.args[1] == 0);
break;
case 2:
-   passed &= (rc == -ENOENT);
+   passed &= (rc == -EEXIST);
break;
case 3:
passed &= !rc;
-- 
1.7.0.4

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH 2/6] of: Return -ENXIO from of_parse_phandle_with_args for too large index and return errors from of_gpio_named_count

2012-12-27 Thread Andreas Larsson
This lets of_gpio_named_count return an errno on errors by being able to
distinguish between reaching the end of the phandle list and getting some other
error from of_parse_phandle_with_args.

Return error from of_spi_register_master when there is an "cs-gpios" list for
which gp_gpio_named_count fails.

Adjust drivers/hwmon/gpio-fan.c to cope with error return.

Signed-off-by: Andreas Larsson 
---

Uses -ENXIO modelled after uses such as in platform_get_irq, but maybe some
other errno than ENXIO is more appropriate for this case.

 drivers/gpio/gpiolib-of.c |8 +---
 drivers/hwmon/gpio-fan.c  |2 +-
 drivers/of/base.c |5 -
 drivers/spi/spi.c |   11 +++
 4 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index d542a14..28f24a6 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -107,11 +107,10 @@ EXPORT_SYMBOL(of_get_named_gpio_flags);
  */
 unsigned int of_gpio_named_count(struct device_node *np, const char* propname)
 {
+   int ret;
unsigned int cnt = 0;
 
do {
-   int ret;
-
ret = of_parse_phandle_with_args(np, propname, "#gpio-cells",
 cnt, NULL);
/* A hole in the gpios = <> counts anyway. */
@@ -119,7 +118,10 @@ unsigned int of_gpio_named_count(struct device_node *np, 
const char* propname)
break;
} while (++cnt);
 
-   return cnt;
+   if (ret == -ENXIO)
+   return cnt;
+   else
+   return ret;
 }
 EXPORT_SYMBOL(of_gpio_named_count);
 
diff --git a/drivers/hwmon/gpio-fan.c b/drivers/hwmon/gpio-fan.c
index 4e04c12..9ccd92f 100644
--- a/drivers/hwmon/gpio-fan.c
+++ b/drivers/hwmon/gpio-fan.c
@@ -477,7 +477,7 @@ static int gpio_fan_get_of_pdata(struct device *dev,
pdata->speed = speed;
 
/* Alarm GPIO if one exists */
-   if (of_gpio_named_count(node, "alarm-gpios")) {
+   if (of_gpio_named_count(node, "alarm-gpios") > 0) {
struct gpio_fan_alarm *alarm;
int val;
enum of_gpio_flags flags;
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 986afd7..1f16629 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1110,7 +1110,10 @@ int of_parse_phandle_with_args(const struct device_node 
*np, const char *list_na
/* Loop exited without finding a valid entry; return an error */
if (node)
of_node_put(node);
-   return -EINVAL;
+   if (list == list_end)
+   return -ENXIO; /* Index beyond end of list */
+   else
+   return -EINVAL;
 }
 EXPORT_SYMBOL(of_parse_phandle_with_args);
 
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 19ee901..9c2acf1 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1059,7 +1059,7 @@ EXPORT_SYMBOL_GPL(spi_alloc_master);
 #ifdef CONFIG_OF
 static int of_spi_register_master(struct spi_master *master)
 {
-   u16 nb;
+   int nb;
int i, *cs;
struct device_node *np = master->dev.of_node;
 
@@ -1067,10 +1067,13 @@ static int of_spi_register_master(struct spi_master 
*master)
return 0;
 
nb = of_gpio_named_count(np, "cs-gpios");
-   master->num_chipselect = max(nb, master->num_chipselect);
-
-   if (nb < 1)
+   if (nb == 0 || nb == -ENOENT) /* No error if cs-gpios does not exist */
return 0;
+   else if (nb < 0)
+   return nb;
+
+   if (nb > master->num_chipselect)
+   master->num_chipselect = (u16)nb;
 
cs = devm_kzalloc(&master->dev,
  sizeof(int) * master->num_chipselect,
-- 
1.7.0.4

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH 3/6] of_spi: Initialize cs_gpios properly

2012-12-27 Thread Andreas Larsson
Using memset does not set an array of integers properly

Signed-off-by: Andreas Larsson 
---
 drivers/spi/spi.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 9c2acf1..a4baa0a 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1083,7 +1083,8 @@ static int of_spi_register_master(struct spi_master 
*master)
if (!master->cs_gpios)
return -ENOMEM;
 
-   memset(cs, -EINVAL, master->num_chipselect);
+   for (i = 0; i < master->num_chipselect; i++)
+   cs[i] = -EINVAL;
 
for (i = 0; i < nb; i++)
cs[i] = of_get_named_gpio(np, "cs-gpios", i);
-- 
1.7.0.4

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH 4/6] of_spi: Document cs_gpios and cs_gpio in kernel-doc

2012-12-27 Thread Andreas Larsson
This adds missing kernel-doc entries for cs_gpios in struct spi_master and
cs_gpio in struct spi_device.

Signed-off-by: Andreas Larsson 
---
 include/linux/spi/spi.h |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index f629189..88a669c 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -57,6 +57,7 @@ extern struct bus_type spi_bus_type;
  * @modalias: Name of the driver to use with this device, or an alias
  * for that name.  This appears in the sysfs "modalias" attribute
  * for driver coldplugging, and in uevents used for hotplugging
+ * @cs_gpio: Negative or gpio for chip select.
  *
  * A @spi_device is used to interchange data between an SPI slave
  * (usually a discrete chip) and CPU memory.
@@ -258,6 +259,7 @@ static inline void spi_unregister_driver(struct spi_driver 
*sdrv)
  * @unprepare_transfer_hardware: there are currently no more messages on the
  * queue so the subsystem notifies the driver that it may relax the
  * hardware by issuing this call
+ * @cs_gpios: possible array of negative values or gpios for chip selects
  *
  * Each SPI master controller can communicate with one or more @spi_device
  * children.  These make a small bus, sharing MOSI, MISO and SCK signals
-- 
1.7.0.4

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH 5/6] of_spi: Add fetching of of_gpio flags to of_spi_register_master

2012-12-27 Thread Andreas Larsson
When using a gpio chip select with a OF_GPIO_ACTIVE_LOW flag, this needs to be
known to the controller driver.

Signed-off-by: Andreas Larsson 
---
 Documentation/devicetree/bindings/spi/spi-bus.txt |3 +-
 drivers/spi/spi.c |   24 ++--
 include/linux/spi/spi.h   |5 
 3 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/spi-bus.txt 
b/Documentation/devicetree/bindings/spi/spi-bus.txt
index 296015e..a4950a6 100644
--- a/Documentation/devicetree/bindings/spi/spi-bus.txt
+++ b/Documentation/devicetree/bindings/spi/spi-bus.txt
@@ -57,7 +57,8 @@ contain the following properties.
3-wire mode.
 
 If a gpio chipselect is used for the SPI slave the gpio number will be passed
-via the cs_gpio
+via the cs_gpio and the corresponding of_gpio_flags will be passed via
+cs_gpio_flags.
 
 SPI example for an MPC5200 SPI bus:
spi@f00 {
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index a4baa0a..6f1b717 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -385,8 +385,10 @@ int spi_add_device(struct spi_device *spi)
goto done;
}
 
-   if (master->cs_gpios)
+   if (master->cs_gpios) {
spi->cs_gpio = master->cs_gpios[spi->chip_select];
+   spi->cs_gpio_flags = master->cs_gpio_flags[spi->chip_select];
+   }
 
/* Drivers may modify this initial i/o setup, but will
 * normally rely on the device being setup.  Devices
@@ -1060,7 +1062,8 @@ EXPORT_SYMBOL_GPL(spi_alloc_master);
 static int of_spi_register_master(struct spi_master *master)
 {
int nb;
-   int i, *cs;
+   int ret, i, *cs;
+   enum of_gpio_flags *flags;
struct device_node *np = master->dev.of_node;
 
if (!np)
@@ -1083,13 +1086,28 @@ static int of_spi_register_master(struct spi_master 
*master)
if (!master->cs_gpios)
return -ENOMEM;
 
+   flags = devm_kzalloc(&master->dev,
+sizeof(*flags) * master->num_chipselect,
+GFP_KERNEL);
+   master->cs_gpio_flags = flags;
+
+   if (!master->cs_gpio_flags) {
+   ret = -ENOMEM;
+   goto err_alloc_flags;
+   }
+
for (i = 0; i < master->num_chipselect; i++)
cs[i] = -EINVAL;
 
for (i = 0; i < nb; i++)
-   cs[i] = of_get_named_gpio(np, "cs-gpios", i);
+   cs[i] = of_get_named_gpio_flags(np, "cs-gpios", i, &flags[i]);
 
return 0;
+
+err_alloc_flags:
+   devm_kfree(&master->dev, master->cs_gpios);
+   master->cs_gpios = NULL;
+   return ret;
 }
 #else
 static int of_spi_register_master(struct spi_master *master)
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 88a669c..96b1055 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /*
  * INTERFACES between SPI master-side drivers and SPI infrastructure.
@@ -58,6 +59,7 @@ extern struct bus_type spi_bus_type;
  * for that name.  This appears in the sysfs "modalias" attribute
  * for driver coldplugging, and in uevents used for hotplugging
  * @cs_gpio: Negative or gpio for chip select.
+ * @cs_gpio_flags: of_gpio_flags corresponding to cs_gpio
  *
  * A @spi_device is used to interchange data between an SPI slave
  * (usually a discrete chip) and CPU memory.
@@ -92,6 +94,7 @@ struct spi_device {
void*controller_data;
charmodalias[SPI_NAME_SIZE];
int cs_gpio;/* chip select gpio */
+   enum of_gpio_flags  cs_gpio_flags;  /* chip select of_gpio_flags */
 
/*
 * likely need more hooks for more protocol options affecting how
@@ -260,6 +263,7 @@ static inline void spi_unregister_driver(struct spi_driver 
*sdrv)
  * queue so the subsystem notifies the driver that it may relax the
  * hardware by issuing this call
  * @cs_gpios: possible array of negative values or gpios for chip selects
+ * @cs_gpio_flags: possible array of of_gpio_flags corresponding to cs_gpios
  *
  * Each SPI master controller can communicate with one or more @spi_device
  * children.  These make a small bus, sharing MOSI, MISO and SCK signals
@@ -367,6 +371,7 @@ struct spi_master {
int (*unprepare_transfer_hardware)(struct spi_master *master);
/* gpio chip select */
int *cs_gpios;
+   enum of_gpio_flags  *cs_gpio_flags;
 };
 
 static inline void *spi_master_get_devdata(struct spi_master *master)
-- 
1.7.0.4

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH 6/6] of_spi: Initialize cs_gpios and cs_gpio with -EEXIST

2012-12-27 Thread Andreas Larsson
Holes in the cs-gpios DT phandle list is supposed to mark that native
chipselects is to be used. The value returned from of_get_named_gpio_flags in
this case is -EEXIST. By initializing cs_gpios and cs_gpio with -EEXIST, this
and only this errno will indicate to a spi controller driver that a native
chipselect is to be used.

Signed-off-by: Andreas Larsson 
---
 drivers/spi/spi.c   |4 ++--
 include/linux/spi/spi.h |7 +--
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 6f1b717..7494bad 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -334,7 +334,7 @@ struct spi_device *spi_alloc_device(struct spi_master 
*master)
spi->dev.parent = &master->dev;
spi->dev.bus = &spi_bus_type;
spi->dev.release = spidev_release;
-   spi->cs_gpio = -EINVAL;
+   spi->cs_gpio = -EEXIST;
device_initialize(&spi->dev);
return spi;
 }
@@ -1097,7 +1097,7 @@ static int of_spi_register_master(struct spi_master 
*master)
}
 
for (i = 0; i < master->num_chipselect; i++)
-   cs[i] = -EINVAL;
+   cs[i] = -EEXIST;
 
for (i = 0; i < nb; i++)
cs[i] = of_get_named_gpio_flags(np, "cs-gpios", i, &flags[i]);
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 96b1055..0701882 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -58,7 +58,8 @@ extern struct bus_type spi_bus_type;
  * @modalias: Name of the driver to use with this device, or an alias
  * for that name.  This appears in the sysfs "modalias" attribute
  * for driver coldplugging, and in uevents used for hotplugging
- * @cs_gpio: Negative or gpio for chip select.
+ * @cs_gpio: Negative or gpio for chip select.  Set to -EEXIST when chipselect
+ * should be handled natively by the controller driver
  * @cs_gpio_flags: of_gpio_flags corresponding to cs_gpio
  *
  * A @spi_device is used to interchange data between an SPI slave
@@ -262,7 +263,9 @@ static inline void spi_unregister_driver(struct spi_driver 
*sdrv)
  * @unprepare_transfer_hardware: there are currently no more messages on the
  * queue so the subsystem notifies the driver that it may relax the
  * hardware by issuing this call
- * @cs_gpios: possible array of negative values or gpios for chip selects
+ * @cs_gpios: possible array of negative values or gpios for chip selects.  A
+ * chipselect that should be handled natively by the controller driver is
+ * set to -EEXIST.
  * @cs_gpio_flags: possible array of of_gpio_flags corresponding to cs_gpios
  *
  * Each SPI master controller can communicate with one or more @spi_device
-- 
1.7.0.4

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH 2/6] of: Return -ENXIO from of_parse_phandle_with_args for too large index and return errors from of_gpio_named_count

2012-12-28 Thread Andreas Larsson

On 2012-12-27 13:10, Andreas Larsson wrote:

This lets of_gpio_named_count return an errno on errors by being able to
distinguish between reaching the end of the phandle list and getting some other
error from of_parse_phandle_with_args.

Return error from of_spi_register_master when there is an "cs-gpios" list for
which gp_gpio_named_count fails.

Adjust drivers/hwmon/gpio-fan.c to cope with error return.

Signed-off-by: Andreas Larsson 


The callers of of_gpio_count of course also needs to handle errno 
returns. I'll take care of that in V2 of the patch-set. But I'll wait 
for more comments on the patch set before sending V2 unless someone insists.


Cheers,
Andreas

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH v2 0/6] of, of_gpio, of_spi: Bugfix and improve of_parse_phandle_with_args, of_gpio_named_count and of_spi_register_master

2013-01-29 Thread Andreas Larsson
This patch series fixes a bug where of_gpio_named count relied upon a return
value that was no longer returned from of_parse_phandle_with_args and adds the
possibility for of_gpio_named_count to return error values.

In addition, for of_spi_register_master it fixes a bug, adds documentation,
adds fetching of gpio flags and initializes gpio values to be consistent with
return values from of_parse_phandle_with_args.

Tested on sparc (excluding the changes to drivers gpio-fan, i2c-mux-gpio,
matrix_keypad, mdio-mux-gpio, spi-mpc52xx, spi-oc-tiny, spi-ppc4xx, selftest). 
Compile tested on x86, arm and ppc (all changed source files, when appropriate
for platform)

Changes since v1:
- PATCH 2/6: Handle error return values from calls to of_gpio_count

Andreas Larsson (6):
  of: Return -EEXIST from of_parse_phandle_with_args for holes in
phandle list
  of: Return -ENXIO from of_parse_phandle_with_args for too large index
and return errors from of_gpio_named_count
  of_spi: Initialize cs_gpios properly
  of_spi: Document cs_gpios and cs_gpio in kernel-doc
  of_spi: Add fetching of of_gpio flags to of_spi_register_master
  of_spi: Initialize cs_gpios and cs_gpio with -EEXIST

 Documentation/devicetree/bindings/spi/spi-bus.txt |3 +-
 drivers/gpio/gpiolib-of.c |8 +++--
 drivers/hwmon/gpio-fan.c  |6 ++--
 drivers/i2c/muxes/i2c-mux-gpio.c  |3 +-
 drivers/input/keyboard/matrix_keypad.c|2 +-
 drivers/net/phy/mdio-mux-gpio.c   |2 +-
 drivers/of/base.c |9 +++--
 drivers/of/selftest.c |2 +-
 drivers/spi/spi-fsl-spi.c |4 ++-
 drivers/spi/spi-mpc52xx.c |5 +++
 drivers/spi/spi-oc-tiny.c |4 ++-
 drivers/spi/spi-ppc4xx.c  |6 +++-
 drivers/spi/spi.c |   40 -
 include/linux/spi/spi.h   |   10 +
 14 files changed, 78 insertions(+), 26 deletions(-)

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH v2 1/6] of: Return -EEXIST from of_parse_phandle_with_args for holes in phandle list

2013-01-29 Thread Andreas Larsson
Return value for an empty phandle was -EEXIST before commit 15c9a0ac, that
changed the return value in this case to -ENOENT. However, of_gpio_named_count
relies upon the return value to be -EEXIST and relies upon being able to
distinguish this case from the case of no list at all which also returns
-ENOENT.

Also change the of selftest to expect -EEXIST in this case.

Signed-off-by: Andreas Larsson 
---

I have only compile tested the selftest, not having appropriate hardware around 
for running it.

 drivers/of/base.c |4 ++--
 drivers/of/selftest.c |2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index 2390ddb..986afd7 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1083,11 +1083,11 @@ int of_parse_phandle_with_args(const struct device_node 
*np, const char *list_na
 * All of the error cases above bail out of the loop, so at
 * this point, the parsing is successful. If the requested
 * index matches, then fill the out_args structure and return,
-* or return -ENOENT for an empty entry.
+* or return -EEXIST for an empty entry.
 */
if (cur_index == index) {
if (!phandle)
-   return -ENOENT;
+   return -EEXIST;
 
if (out_args) {
int i;
diff --git a/drivers/of/selftest.c b/drivers/of/selftest.c
index f24ffd7..b1c2ae9 100644
--- a/drivers/of/selftest.c
+++ b/drivers/of/selftest.c
@@ -54,7 +54,7 @@ static void __init of_selftest_parse_phandle_with_args(void)
passed &= (args.args[1] == 0);
break;
case 2:
-   passed &= (rc == -ENOENT);
+   passed &= (rc == -EEXIST);
break;
case 3:
passed &= !rc;
-- 
1.7.0.4

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH v2 2/6] of: Return -ENXIO from of_parse_phandle_with_args for too large index and return errors from of_gpio_named_count

2013-01-29 Thread Andreas Larsson
This lets of_gpio_named_count return an errno on errors by being able to
distinguish between reaching the end of the phandle list and getting some other
error from of_parse_phandle_with_args.

Return error from of_spi_register_master when there is an "cs-gpios" list for
which gp_gpio_named_count fails.

Adjust various drivers cope with error return from of_gpio_named_count,
including via of_gpio_count.

Signed-off-by: Andreas Larsson 
---
Changes since v1:
- Handle error return values from calls to of_gpio_count

 drivers/gpio/gpiolib-of.c  |8 +---
 drivers/hwmon/gpio-fan.c   |6 +++---
 drivers/i2c/muxes/i2c-mux-gpio.c   |3 ++-
 drivers/input/keyboard/matrix_keypad.c |2 +-
 drivers/net/phy/mdio-mux-gpio.c|2 +-
 drivers/of/base.c  |5 -
 drivers/spi/spi-fsl-spi.c  |4 +++-
 drivers/spi/spi-mpc52xx.c  |5 +
 drivers/spi/spi-oc-tiny.c  |4 +++-
 drivers/spi/spi-ppc4xx.c   |6 +-
 drivers/spi/spi.c  |   11 +++
 11 files changed, 39 insertions(+), 17 deletions(-)

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index d542a14..28f24a6 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -107,11 +107,10 @@ EXPORT_SYMBOL(of_get_named_gpio_flags);
  */
 unsigned int of_gpio_named_count(struct device_node *np, const char* propname)
 {
+   int ret;
unsigned int cnt = 0;
 
do {
-   int ret;
-
ret = of_parse_phandle_with_args(np, propname, "#gpio-cells",
 cnt, NULL);
/* A hole in the gpios = <> counts anyway. */
@@ -119,7 +118,10 @@ unsigned int of_gpio_named_count(struct device_node *np, 
const char* propname)
break;
} while (++cnt);
 
-   return cnt;
+   if (ret == -ENXIO)
+   return cnt;
+   else
+   return ret;
 }
 EXPORT_SYMBOL(of_gpio_named_count);
 
diff --git a/drivers/hwmon/gpio-fan.c b/drivers/hwmon/gpio-fan.c
index 4e04c12..9b92d34 100644
--- a/drivers/hwmon/gpio-fan.c
+++ b/drivers/hwmon/gpio-fan.c
@@ -422,8 +422,8 @@ static int gpio_fan_get_of_pdata(struct device *dev,
 
/* Fill GPIO pin array */
pdata->num_ctrl = of_gpio_count(node);
-   if (!pdata->num_ctrl) {
-   dev_err(dev, "gpios DT property empty / missing");
+   if (pdata->num_ctrl <= 0) {
+   dev_err(dev, "gpios DT property broken / empty / missing");
return -ENODEV;
}
ctrl = devm_kzalloc(dev, pdata->num_ctrl * sizeof(unsigned),
@@ -477,7 +477,7 @@ static int gpio_fan_get_of_pdata(struct device *dev,
pdata->speed = speed;
 
/* Alarm GPIO if one exists */
-   if (of_gpio_named_count(node, "alarm-gpios")) {
+   if (of_gpio_named_count(node, "alarm-gpios") > 0) {
struct gpio_fan_alarm *alarm;
int val;
enum of_gpio_flags flags;
diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index 9272743..a3ddb36 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -106,7 +106,8 @@ static int i2c_mux_gpio_probe_dt(struct gpiomux *mux,
 
mux->data.n_gpios = of_gpio_named_count(np, "mux-gpios");
if (mux->data.n_gpios < 0) {
-   dev_err(&pdev->dev, "Missing mux-gpios property in the DT.\n");
+   dev_err(&pdev->dev,
+   "Missing or broken mux-gpios property in the DT.\n");
return -EINVAL;
}
 
diff --git a/drivers/input/keyboard/matrix_keypad.c 
b/drivers/input/keyboard/matrix_keypad.c
index f4ff0dd..bc7cec5 100644
--- a/drivers/input/keyboard/matrix_keypad.c
+++ b/drivers/input/keyboard/matrix_keypad.c
@@ -418,7 +418,7 @@ matrix_keypad_parse_dt(struct device *dev)
 
pdata->num_row_gpios = of_gpio_named_count(np, "row-gpios");
pdata->num_col_gpios = of_gpio_named_count(np, "col-gpios");
-   if (!pdata->num_row_gpios || !pdata->num_col_gpios) {
+   if (pdata->num_row_gpios <= 0 || !pdata->num_col_gpios <= 0) {
dev_err(dev, "number of keypad rows/columns not specified\n");
return ERR_PTR(-EINVAL);
}
diff --git a/drivers/net/phy/mdio-mux-gpio.c b/drivers/net/phy/mdio-mux-gpio.c
index 0c9accb..3e1d285 100644
--- a/drivers/net/phy/mdio-mux-gpio.c
+++ b/drivers/net/phy/mdio-mux-gpio.c
@@ -61,7 +61,7 @@ static int mdio_mux_gpio_probe(struct platform_device *pdev)
return -ENODEV;
 
num_gpios = of_gpio_count(pdev->dev.of_node);
-   if (num_gpios == 0 || num_gpios > MDIO_MUX_GPIO_MAX_BITS)
+

[PATCH v2 3/6] of_spi: Initialize cs_gpios properly

2013-01-29 Thread Andreas Larsson
Using memset does not set an array of integers properly

Signed-off-by: Andreas Larsson 
---
 drivers/spi/spi.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 9c2acf1..a4baa0a 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1083,7 +1083,8 @@ static int of_spi_register_master(struct spi_master 
*master)
if (!master->cs_gpios)
return -ENOMEM;
 
-   memset(cs, -EINVAL, master->num_chipselect);
+   for (i = 0; i < master->num_chipselect; i++)
+   cs[i] = -EINVAL;
 
for (i = 0; i < nb; i++)
cs[i] = of_get_named_gpio(np, "cs-gpios", i);
-- 
1.7.0.4

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH v2 6/6] of_spi: Initialize cs_gpios and cs_gpio with -EEXIST

2013-01-29 Thread Andreas Larsson
Holes in the cs-gpios DT phandle list is supposed to mark that native
chipselects is to be used. The value returned from of_get_named_gpio_flags in
this case is -EEXIST. By initializing cs_gpios and cs_gpio with -EEXIST, this
and only this errno will indicate to a spi controller driver that a native
chipselect is to be used.

Signed-off-by: Andreas Larsson 
---
 drivers/spi/spi.c   |4 ++--
 include/linux/spi/spi.h |7 +--
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 6f1b717..7494bad 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -334,7 +334,7 @@ struct spi_device *spi_alloc_device(struct spi_master 
*master)
spi->dev.parent = &master->dev;
spi->dev.bus = &spi_bus_type;
spi->dev.release = spidev_release;
-   spi->cs_gpio = -EINVAL;
+   spi->cs_gpio = -EEXIST;
device_initialize(&spi->dev);
return spi;
 }
@@ -1097,7 +1097,7 @@ static int of_spi_register_master(struct spi_master 
*master)
}
 
for (i = 0; i < master->num_chipselect; i++)
-   cs[i] = -EINVAL;
+   cs[i] = -EEXIST;
 
for (i = 0; i < nb; i++)
cs[i] = of_get_named_gpio_flags(np, "cs-gpios", i, &flags[i]);
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 96b1055..0701882 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -58,7 +58,8 @@ extern struct bus_type spi_bus_type;
  * @modalias: Name of the driver to use with this device, or an alias
  * for that name.  This appears in the sysfs "modalias" attribute
  * for driver coldplugging, and in uevents used for hotplugging
- * @cs_gpio: Negative or gpio for chip select.
+ * @cs_gpio: Negative or gpio for chip select.  Set to -EEXIST when chipselect
+ * should be handled natively by the controller driver
  * @cs_gpio_flags: of_gpio_flags corresponding to cs_gpio
  *
  * A @spi_device is used to interchange data between an SPI slave
@@ -262,7 +263,9 @@ static inline void spi_unregister_driver(struct spi_driver 
*sdrv)
  * @unprepare_transfer_hardware: there are currently no more messages on the
  * queue so the subsystem notifies the driver that it may relax the
  * hardware by issuing this call
- * @cs_gpios: possible array of negative values or gpios for chip selects
+ * @cs_gpios: possible array of negative values or gpios for chip selects.  A
+ * chipselect that should be handled natively by the controller driver is
+ * set to -EEXIST.
  * @cs_gpio_flags: possible array of of_gpio_flags corresponding to cs_gpios
  *
  * Each SPI master controller can communicate with one or more @spi_device
-- 
1.7.0.4

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH v2 5/6] of_spi: Add fetching of of_gpio flags to of_spi_register_master

2013-01-29 Thread Andreas Larsson
When using a gpio chip select with a OF_GPIO_ACTIVE_LOW flag, this needs to be
known to the controller driver.

Signed-off-by: Andreas Larsson 
---
 Documentation/devicetree/bindings/spi/spi-bus.txt |3 +-
 drivers/spi/spi.c |   24 ++--
 include/linux/spi/spi.h   |5 
 3 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/spi-bus.txt 
b/Documentation/devicetree/bindings/spi/spi-bus.txt
index 296015e..a4950a6 100644
--- a/Documentation/devicetree/bindings/spi/spi-bus.txt
+++ b/Documentation/devicetree/bindings/spi/spi-bus.txt
@@ -57,7 +57,8 @@ contain the following properties.
3-wire mode.
 
 If a gpio chipselect is used for the SPI slave the gpio number will be passed
-via the cs_gpio
+via the cs_gpio and the corresponding of_gpio_flags will be passed via
+cs_gpio_flags.
 
 SPI example for an MPC5200 SPI bus:
spi@f00 {
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index a4baa0a..6f1b717 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -385,8 +385,10 @@ int spi_add_device(struct spi_device *spi)
goto done;
}
 
-   if (master->cs_gpios)
+   if (master->cs_gpios) {
spi->cs_gpio = master->cs_gpios[spi->chip_select];
+   spi->cs_gpio_flags = master->cs_gpio_flags[spi->chip_select];
+   }
 
/* Drivers may modify this initial i/o setup, but will
 * normally rely on the device being setup.  Devices
@@ -1060,7 +1062,8 @@ EXPORT_SYMBOL_GPL(spi_alloc_master);
 static int of_spi_register_master(struct spi_master *master)
 {
int nb;
-   int i, *cs;
+   int ret, i, *cs;
+   enum of_gpio_flags *flags;
struct device_node *np = master->dev.of_node;
 
if (!np)
@@ -1083,13 +1086,28 @@ static int of_spi_register_master(struct spi_master 
*master)
if (!master->cs_gpios)
return -ENOMEM;
 
+   flags = devm_kzalloc(&master->dev,
+sizeof(*flags) * master->num_chipselect,
+GFP_KERNEL);
+   master->cs_gpio_flags = flags;
+
+   if (!master->cs_gpio_flags) {
+   ret = -ENOMEM;
+   goto err_alloc_flags;
+   }
+
for (i = 0; i < master->num_chipselect; i++)
cs[i] = -EINVAL;
 
for (i = 0; i < nb; i++)
-   cs[i] = of_get_named_gpio(np, "cs-gpios", i);
+   cs[i] = of_get_named_gpio_flags(np, "cs-gpios", i, &flags[i]);
 
return 0;
+
+err_alloc_flags:
+   devm_kfree(&master->dev, master->cs_gpios);
+   master->cs_gpios = NULL;
+   return ret;
 }
 #else
 static int of_spi_register_master(struct spi_master *master)
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 88a669c..96b1055 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /*
  * INTERFACES between SPI master-side drivers and SPI infrastructure.
@@ -58,6 +59,7 @@ extern struct bus_type spi_bus_type;
  * for that name.  This appears in the sysfs "modalias" attribute
  * for driver coldplugging, and in uevents used for hotplugging
  * @cs_gpio: Negative or gpio for chip select.
+ * @cs_gpio_flags: of_gpio_flags corresponding to cs_gpio
  *
  * A @spi_device is used to interchange data between an SPI slave
  * (usually a discrete chip) and CPU memory.
@@ -92,6 +94,7 @@ struct spi_device {
void*controller_data;
charmodalias[SPI_NAME_SIZE];
int cs_gpio;/* chip select gpio */
+   enum of_gpio_flags  cs_gpio_flags;  /* chip select of_gpio_flags */
 
/*
 * likely need more hooks for more protocol options affecting how
@@ -260,6 +263,7 @@ static inline void spi_unregister_driver(struct spi_driver 
*sdrv)
  * queue so the subsystem notifies the driver that it may relax the
  * hardware by issuing this call
  * @cs_gpios: possible array of negative values or gpios for chip selects
+ * @cs_gpio_flags: possible array of of_gpio_flags corresponding to cs_gpios
  *
  * Each SPI master controller can communicate with one or more @spi_device
  * children.  These make a small bus, sharing MOSI, MISO and SCK signals
@@ -367,6 +371,7 @@ struct spi_master {
int (*unprepare_transfer_hardware)(struct spi_master *master);
/* gpio chip select */
int *cs_gpios;
+   enum of_gpio_flags  *cs_gpio_flags;
 };
 
 static inline void *spi_master_get_devdata(struct spi_master *master)
-- 
1.7.0.4

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH v2 4/6] of_spi: Document cs_gpios and cs_gpio in kernel-doc

2013-01-29 Thread Andreas Larsson
This adds missing kernel-doc entries for cs_gpios in struct spi_master and
cs_gpio in struct spi_device.

Signed-off-by: Andreas Larsson 
---
 include/linux/spi/spi.h |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index f629189..88a669c 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -57,6 +57,7 @@ extern struct bus_type spi_bus_type;
  * @modalias: Name of the driver to use with this device, or an alias
  * for that name.  This appears in the sysfs "modalias" attribute
  * for driver coldplugging, and in uevents used for hotplugging
+ * @cs_gpio: Negative or gpio for chip select.
  *
  * A @spi_device is used to interchange data between an SPI slave
  * (usually a discrete chip) and CPU memory.
@@ -258,6 +259,7 @@ static inline void spi_unregister_driver(struct spi_driver 
*sdrv)
  * @unprepare_transfer_hardware: there are currently no more messages on the
  * queue so the subsystem notifies the driver that it may relax the
  * hardware by issuing this call
+ * @cs_gpios: possible array of negative values or gpios for chip selects
  *
  * Each SPI master controller can communicate with one or more @spi_device
  * children.  These make a small bus, sharing MOSI, MISO and SCK signals
-- 
1.7.0.4

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH] gpio: Add device driver for GRGPIO cores

2013-01-30 Thread Andreas Larsson
This driver supports GRGPIO gpio cores available in the GRLIB VHDL IP core
library.

Signed-off-by: Andreas Larsson 
---
 drivers/gpio/Kconfig   |7 +
 drivers/gpio/Makefile  |1 +
 drivers/gpio/gpio-grgpio.c |  313 
 3 files changed, 321 insertions(+), 0 deletions(-)
 create mode 100644 drivers/gpio/gpio-grgpio.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 682de75..4ce4626 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -298,6 +298,13 @@ config GPIO_GE_FPGA
  and write pin state) for GPIO implemented in a number of GE single
  board computers.
 
+config GPIO_GRGPIO
+   tristate "Aeroflex Gaisler GRGPIO support"
+   depends on OF
+   help
+ Select this to support Aeroflex Gaisler GRGPIO cores from the GRLIB
+ VHDL IP core library.
+
 comment "I2C GPIO expanders:"
 
 config GPIO_ARIZONA
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index c5aebd0..ca5d7a3 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_ARCH_DAVINCI)+= gpio-davinci.o
 obj-$(CONFIG_GPIO_EM)  += gpio-em.o
 obj-$(CONFIG_GPIO_EP93XX)  += gpio-ep93xx.o
 obj-$(CONFIG_GPIO_GE_FPGA) += gpio-ge.o
+obj-$(CONFIG_GPIO_GRGPIO)  += gpio-grgpio.o
 obj-$(CONFIG_GPIO_ICH) += gpio-ich.o
 obj-$(CONFIG_GPIO_IT8761E) += gpio-it8761e.o
 obj-$(CONFIG_GPIO_JANZ_TTL)+= gpio-janz-ttl.o
diff --git a/drivers/gpio/gpio-grgpio.c b/drivers/gpio/gpio-grgpio.c
new file mode 100644
index 000..1e367c0
--- /dev/null
+++ b/drivers/gpio/gpio-grgpio.c
@@ -0,0 +1,313 @@
+/*
+ * Driver for Aeroflex Gaisler GRGPIO General Purpose I/O cores.
+ *
+ * 2013 (c) Aeroflex Gaisler AB
+ *
+ * This driver supports the GRGPIO GPIO core available in the GRLIB VHDL IP 
core
+ * library.
+ *
+ * Full documentation of the GRGPIO core can be found here:
+ * http://www.gaisler.com/products/grlib/grip.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.
+ *
+ * Contributors: Andreas Larsson 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DRV_NAME   "grgpio"
+
+#define GRGPIO_MAX_NGPIO 32
+
+struct grgpio_regs {
+   u32 data;   /* 0x00 */
+   u32 output; /* 0x04 */
+   u32 dir;/* 0x08 */
+   u32 imask;  /* 0x0c */
+   u32 ipol;   /* 0x10  */
+   u32 iedge;  /* 0x14 */
+   u32 bypass; /* 0x18 */
+   u32 __reserved; /* 0x1c */
+   u32 imap[8];/* 0x20-0x3c */
+};
+
+#define GRGPIO_IN  0
+#define GRGPIO_OUT 1
+
+struct grgpio_priv {
+   struct gpio_chip gc;
+   struct device *dev;
+   struct grgpio_regs __iomem *regs;
+
+   u32 output; /* output shadow register */
+   u32 dir;/* direction shadow register */
+   u32 imask;  /* irq mask shadow register */
+
+   s32 *irqmap;
+
+   spinlock_t lock; /* Protecting concurrent shadow & register changes */
+};
+
+static inline struct grgpio_priv *grgpio_chip_to_priv(struct gpio_chip *gc)
+{
+   return container_of(gc, struct grgpio_priv, gc);
+}
+
+#if defined(__BIG_ENDIAN)
+static inline u32 grgpio_read_reg(u32 __iomem *reg)
+{
+   return ioread32be(reg);
+}
+
+static inline void grgpio_write_reg(u32 __iomem *reg, u32 val)
+{
+   iowrite32be(val, reg);
+}
+#else
+static inline u32 grgpio_read_reg(u32 __iomem *reg)
+{
+   return ioread32(reg);
+}
+
+static inline void grgpio_write_reg(u32 __iomem *reg, u32 val)
+{
+   iowrite32(val, reg);
+}
+#endif
+
+static void grgpio_set_sbit(struct grgpio_priv *priv, u32 __iomem *reg,
+  unsigned offset, int val, u32 *shadow)
+{
+   unsigned long flags;
+
+   spin_lock_irqsave(&priv->lock, flags);
+
+   if (val)
+   *shadow |= (1 << offset);
+   else
+   *shadow &= ~(1 << offset);
+   grgpio_write_reg(reg, *shadow);
+
+   spin_unlock_irqrestore(&priv->lock, flags);
+}
+
+/*  */
+
+static int grgpio_get(struct gpio_chip *gc, unsigned offset)
+{
+   struct grgpio_priv *priv = grgpio_chip_to_priv(gc);
+
+   return (grgpio_read_reg(&priv->regs->data) >> offset) & 1;
+}
+
+static void grgpio_set(struct gpio_chip *gc, unsigned offset, int val)
+{
+   struct grgpio_priv *priv = grgpio_chip_to_priv(gc);
+
+   grgpio_set_sbit(priv, &priv->regs->output, offset, val, &priv->output);
+}
+
+
+static int grgpio_get_dir(struct gpio_chip *gc, unsigned offset)
+{
+   struct grgpio_priv *priv = grgpi

[PATCH 1/2] i2c: i2c-ocores: Add irq support for sparc

2012-10-23 Thread Andreas Larsson
There are no platform resources of type IORESOURCE_IRQ on sparc, so the irq
number is acquired in a different manner for sparc. The general case uses
platform_get_irq, that internally still uses platform_get_resource.

Signed-off-by: Andreas Larsson 
---
 drivers/i2c/busses/i2c-ocores.c |   13 +
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index bffd550..1eb8a65 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -267,16 +267,21 @@ static int __devinit ocores_i2c_probe(struct 
platform_device *pdev)
 {
struct ocores_i2c *i2c;
struct ocores_i2c_platform_data *pdata;
-   struct resource *res, *res2;
+   struct resource *res;
int ret;
int i;
+   int irq;
 
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res)
return -ENODEV;
 
-   res2 = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-   if (!res2)
+#ifdef CONFIG_SPARC
+   irq = pdev->archdata.irqs[0];
+#else
+   irq = platform_get_irq(pdev, 0);
+#endif
+   if (irq < 0)
return -ENODEV;
 
i2c = devm_kzalloc(&pdev->dev, sizeof(*i2c), GFP_KERNEL);
@@ -313,7 +318,7 @@ static int __devinit ocores_i2c_probe(struct 
platform_device *pdev)
ocores_init(i2c);
 
init_waitqueue_head(&i2c->wait);
-   ret = devm_request_irq(&pdev->dev, res2->start, ocores_isr, 0,
+   ret = devm_request_irq(&pdev->dev, irq, ocores_isr, 0,
   pdev->name, i2c);
if (ret) {
dev_err(&pdev->dev, "Cannot claim IRQ\n");
-- 
1.7.0.4

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH 0/2] i2c: i2c-ocores: Add support for sparc, custom set and get functions, and the GRLIB port of the controller

2012-10-23 Thread Andreas Larsson
On sparc, irqs are not present as an IORESOURCE in the struct platform_device
representation. Therefore, for sparc the irq needs to be fetched in a different
manner.

The GRLIB port of the ocores i2c controller needs custom getreg and setreg
functions to allow for big endian register access and to deal with the fact that
the PRELOW and PREHIGH registers have been merged into one register.

Signed-off-by: Andreas Larsson 

Andreas Larsson (2):
  i2c: i2c-ocores: Add irq support for sparc
  i2c: i2c-ocores: Add support for the GRLIB port of the controller and
custom getreg and setreg functions

 drivers/i2c/busses/i2c-ocores.c |   70 +++---
 1 files changed, 64 insertions(+), 6 deletions(-)

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH 2/2] i2c: i2c-ocores: Add support for the GRLIB port of the controller and custom getreg and setreg functions

2012-10-23 Thread Andreas Larsson
The registers in the GRLIB port of the controller are 32-bit and in big endian
byte order. The PRELOW and PREHIGH registers are merged into one register. The
subsequent registers have their offset decreased accordingly. Hence the register
access needs to be handled in a non-standard manner using custom getreg and
setreg functions.

Signed-off-by: Andreas Larsson 
---
 drivers/i2c/busses/i2c-ocores.c |   57 +-
 1 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index 1eb8a65..e3df62f 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -4,6 +4,9 @@
  *
  * Peter Korsgaard 
  *
+ * Support for the GRLIB port of the controller by
+ * Andreas Larsson 
+ *
  * This file is licensed under the terms of the GNU General Public License
  * version 2.  This program is licensed "as is" without any warranty of any
  * kind, whether express or implied.
@@ -38,6 +41,8 @@ struct ocores_i2c {
int nmsgs;
int state; /* see STATE_ */
int clock_khz;
+   void (*setreg)(struct ocores_i2c *i2c, int reg, u8 value);
+   u8 (*getreg)(struct ocores_i2c *i2c, int reg);
 };
 
 /* registers */
@@ -73,7 +78,9 @@ struct ocores_i2c {
 
 static inline void oc_setreg(struct ocores_i2c *i2c, int reg, u8 value)
 {
-   if (i2c->reg_io_width == 4)
+   if (i2c->setreg)
+   i2c->setreg(i2c, reg, value);
+   else if (i2c->reg_io_width == 4)
iowrite32(value, i2c->base + (reg << i2c->reg_shift));
else if (i2c->reg_io_width == 2)
iowrite16(value, i2c->base + (reg << i2c->reg_shift));
@@ -83,7 +90,9 @@ static inline void oc_setreg(struct ocores_i2c *i2c, int reg, 
u8 value)
 
 static inline u8 oc_getreg(struct ocores_i2c *i2c, int reg)
 {
-   if (i2c->reg_io_width == 4)
+   if (i2c->getreg)
+   return i2c->getreg(i2c, reg);
+   else if (i2c->reg_io_width == 4)
return ioread32(i2c->base + (reg << i2c->reg_shift));
else if (i2c->reg_io_width == 2)
return ioread16(i2c->base + (reg << i2c->reg_shift));
@@ -91,6 +100,40 @@ static inline u8 oc_getreg(struct ocores_i2c *i2c, int reg)
return ioread8(i2c->base + (reg << i2c->reg_shift));
 }
 
+/* Read and write functions for the GRLIB port of the controller. Registers are
+ * 32-bit big endian and the PRELOW and PREHIGH registers are merged into one
+ * register. The subsequent registers has their offset decreased accordingly. 
*/
+static u8 oc_getreg_grlib(struct ocores_i2c *i2c, int reg)
+{
+   u32 rd;
+   int rreg = reg;
+   if (reg != OCI2C_PRELOW)
+   rreg--;
+   rd = ioread32be(i2c->base + (rreg << i2c->reg_shift));
+   if (reg == OCI2C_PREHIGH)
+   return (u8)rd >> 8;
+   else
+   return (u8)rd;
+}
+
+static void oc_setreg_grlib(struct ocores_i2c *i2c, int reg, u8 value)
+{
+   u32 curr, wr;
+   int rreg = reg;
+   if (reg != OCI2C_PRELOW)
+   rreg--;
+   if (reg == OCI2C_PRELOW || reg == OCI2C_PREHIGH) {
+   curr = ioread32be(i2c->base + (rreg << i2c->reg_shift));
+   if (reg == OCI2C_PRELOW)
+   wr = (curr & 0xff00) | value;
+   else
+   wr = (((u32)value) << 8) | (curr & 0xff);
+   } else {
+   wr = value;
+   }
+   iowrite32be(wr, i2c->base + (rreg << i2c->reg_shift));
+}
+
 static void ocores_process(struct ocores_i2c *i2c)
 {
struct i2c_msg *msg = i2c->msg;
@@ -233,6 +276,7 @@ static int ocores_i2c_of_probe(struct platform_device *pdev,
 {
struct device_node *np = pdev->dev.of_node;
u32 val;
+   const char *name;
 
if (of_property_read_u32(np, "reg-shift", &i2c->reg_shift)) {
/* no 'reg-shift', check for deprecated 'regstep' */
@@ -257,6 +301,15 @@ static int ocores_i2c_of_probe(struct platform_device 
*pdev,
 
of_property_read_u32(pdev->dev.of_node, "reg-io-width",
&i2c->reg_io_width);
+
+   name = of_get_property(pdev->dev.of_node, "name", NULL);
+   if (name && (!strcmp(name, "GAISLER_I2CMST") ||
+!strcmp(name, "01_028"))) {
+   dev_dbg(&pdev->dev, "GRLIB variant of i2c-ocores\n");
+   i2c->setreg = oc_setreg_grlib;
+   i2c->getreg = oc_getreg_grlib;
+   }
+
return 0;
 }
 #else
-- 
1.7.0.4

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH 1/2] i2c: i2c-ocores: Add irq support for sparc

2012-10-24 Thread Andreas Larsson

On 10/23/2012 10:13 PM, Peter Korsgaard wrote:

"Andreas" == Andreas Larsson  writes:


  Andreas> There are no platform resources of type IORESOURCE_IRQ on
  Andreas> sparc, so the irq number is acquired in a different manner for
  Andreas> sparc. The general case uses platform_get_irq, that internally
  Andreas> still uses platform_get_resource.

I have no idea why sparc is being odd in this regard, but assuming this
is how it's done, I'm fine with this change.

A quick grep doesn't find any other drivers doing this though:

git grep -l archdata.irqs drivers | xargs grep platform_get_irq

Acked-by: Peter Korsgaard 


Other drivers that work both on sparc and on other platforms usually use 
irq_of_parse_and_map on a corresponding device_node. For non-sparc 
architectures irq_of_parse_and_map sets up mappings that needs to be 
teared down on module exit. Sparc however has its own version of 
irq_of_parse_and_map that just returns the irq number using archdata.irq[].


I am trying to get through a patch platform_get_irq to work for sparc as 
well. If that eventually goes through, the CONFIG_SPARC stuff can then 
be removed cleanly from this driver withouth having to mess with 
irq_of_parse_and_map and tearing mappings down.


Another solution is to use irq_of_parse_and_map for the of-case if no 
irq was found using platform_get_irq. But that would make for more 
rearrangements and add the need for irq_dispose_mapping to be added on 
module exit as well (even though the disposing would do nothing for sparc).


Cheers,
Andreas Larsson

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH 2/2] i2c: i2c-ocores: Add support for the GRLIB port of the controller and custom getreg and setreg functions

2012-10-24 Thread Andreas Larsson

On 10/23/2012 10:24 PM, Peter Korsgaard wrote:

"Andreas" == Andreas Larsson  writes:

  [...]
  Andreas> +/* Read and write functions for the GRLIB port of the controller. 
Registers are
  Andreas> + * 32-bit big endian and the PRELOW and PREHIGH registers are 
merged into one
  Andreas> + * register. The subsequent registers has their offset decreased 
accordingly. */
  Andreas> +static u8 oc_getreg_grlib(struct ocores_i2c *i2c, int reg)
  Andreas> +{
  Andreas> + u32 rd;
  Andreas> + int rreg = reg;
  Andreas> + if (reg != OCI2C_PRELOW)
  Andreas> + rreg--;
  Andreas> + rd = ioread32be(i2c->base + (rreg << i2c->reg_shift));
  Andreas> + if (reg == OCI2C_PREHIGH)
  Andreas> + return (u8)rd >> 8;
  Andreas> + else
  Andreas> + return (u8)rd;
  Andreas> +}
  Andreas> +
  Andreas> +static void oc_setreg_grlib(struct ocores_i2c *i2c, int reg, u8 
value)
  Andreas> +{
  Andreas> + u32 curr, wr;
  Andreas> + int rreg = reg;
  Andreas> + if (reg != OCI2C_PRELOW)
  Andreas> + rreg--;
  Andreas> + if (reg == OCI2C_PRELOW || reg == OCI2C_PREHIGH) {
  Andreas> + curr = ioread32be(i2c->base + (rreg << i2c->reg_shift));
  Andreas> + if (reg == OCI2C_PRELOW)
  Andreas> + wr = (curr & 0xff00) | value;
  Andreas> + else
  Andreas> + wr = (((u32)value) << 8) | (curr & 0xff);
  Andreas> + } else {
  Andreas> + wr = value;
  Andreas> + }
  Andreas> + iowrite32be(wr, i2c->base + (rreg << i2c->reg_shift));

Are all platforms using i2c-ocores guaranteed to provide ioread32be /
iowrite32be or should we stick an #ifdef CONFIG_SPARC around it?


As far as I can see, after digging around, the only platforms that have 
ioread/write32, but not ioread/write32be are frv and mn10300. Do you 
know if those platforms are using i2c-ocores?


Cheers,
Andreas Larsson

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH v2] can: grcan: Add device driver for GRCAN and GRHCAN cores

2012-10-24 Thread Andreas Larsson

On 10/23/2012 06:26 PM, Wolfgang Grandegger wrote:

Hi,

before I have a closer look I have some general questions, especially
about the heavy usage of SysFS for configuring the IP core module.
Generally, we are not allowed to use SysFS for CAN device configuration.

- Why may the user want to configure the resources on a per device base?


The GRCAN core supports selecting between two different hardware interfaces. The 
parameters output0, output1 and selection configures these interfaces and 
selects which one to use. This can not be done in general. For output0 and 
output1 what value means what depends on what hardware is connected to the core. 
If a board has many GRCAN cores they might need different settings for these 
variables. In addition, switching between the two hardware interfaces is 
something that might be wanted to be done at runtime.


For the others module level configuration would work fine.



- Aren't there good default values which work just fine for 99% of the
  users?


For output0, output1 and selection, the answer is no due to the reasons pointed 
out above. For the bpr and buffer sizes, that is probably true.




- Why could the resources not be configured via device tree (or platform
  code)?
- Are there other IP cores already supported by mainline Linux, e.g.
  uart. How are they configured?


See further down on Documentation/devicetree/bindings/net/can/grcan.txt for 
discussion on the device tree matter, or do you something else than configuring 
via bsp files and the like?




+What:  /sys/class/net//grcan/rxcode
+Date:  October 2012
+KernelVersion: 3.8
+Contact:   Andreas Larsson
+Description:
+   Configures the rxcode of the hardware filter. Received messages
+   for which ((message_id ^ rxcode) & rxmask) != 0 holds will be
+   filtered out in harware. Possible values in [0, 0x1fff].
+   The default value is set by the module parameter rxcode and can
+   be read at /sys/module/grcan/parameters/rxcode.
+
+What:  /sys/class/net//grcan/rxmask
+Date:  October 2012
+KernelVersion: 3.8
+Contact:   Andreas Larsson
+Description:
+   Configures the rxmask of the hardware filter. Received messages
+   for which ((message_id ^ rxcode) & rxmask) != 0 holds will be
+   filtered out in harware. Possible values in [0, 0x1fff].
+   The default value is set by the module parameter rxmask and can
+   be read at /sys/module/grcan/parameters/rxmask.


Hardware filters should definitely not be defined via SysFS. We do not
have an interface yet, mainly because it does not fit into a multi user
concept. Anyway, we need such an interface *sooner* than later. Needs
some further thoughts.


OK, I'll get rid of that then and wait for such an interface in the future. It 
would be a pity if hardware filtering, that is a feature that would probably be 
used on an embedded system without multiple users, could never be realized 
because of the socket interface being a multi user concept. If root is the only 
one that can set it up it should be fine in my opinion. Nevertheless, I totally 
agree that a well defined API to enable it would be much nicer than going 
through SysFS.




--- /dev/null
+++ b/Documentation/devicetree/bindings/net/can/grcan.txt
@@ -0,0 +1,27 @@
+CAN controller for Aeroflex Gaisler GRCAN and GRHCAN.
+
+The GRCAN and CRHCAN CAN controllers are available in the GRLIB VHDL IP core
+library.
+
+Note: These properties are built from the AMBA plug&play in a Leon SPARC
+system (the ordinary environment for GRCAN and GRHCAN). There are no bsp
+files for sparc.
+
+Required properties:
+
+- name : Should be "GAISLER_GRCAN", "01_03d", "GAISLER_GRHCAN" or "01_034"


A name should be "vendor,device", e.g. gaisler,grcan. It's also unusual
to add release numbers. Also, you do not distinguish between the devices
above. Then, just use one name and the others are compatible to that one
(even if they are named differently). Well, I realized that the device
tree police is less strict than it was 1..2 years ago... anyway, please
have a look to the many examples around, also for CAN.


As I stated in the file, there are no bsp files for sparc. The device trees are 
generated by a boot loader or a prom. For a leon sparc system the boot loader 
gets information from the hardware, the AMBA plug&play, and generates the device 
trees accordingly.


As for 01_03d and 01_034, they are are the names, based on what is found in the 
plug&play, that are generated by a boot loader that does not have a mapping from 
the plug and play to a name. They are not release numbers. They are based on 
vendor and device numbers as found in the plug&play on a leon sparc system.


Compare with these drivers that all use these kind of names to match with the 
hardware:


[PATCH v3] can: grcan: Add device driver for GRCAN and GRHCAN cores

2012-10-30 Thread Andreas Larsson
This driver supports GRCAN and CRHCAN CAN controllers available in the GRLIB
VHDL IP core library.

Signed-off-by: Andreas Larsson 
---
 Documentation/ABI/testing/sysfs-class-net-grcan|   35 +
 .../devicetree/bindings/net/can/grcan.txt  |   27 +
 Documentation/kernel-parameters.txt|   25 +
 drivers/net/can/Kconfig|9 +
 drivers/net/can/Makefile   |1 +
 drivers/net/can/grcan.c| 1616 
 6 files changed, 1713 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-net-grcan
 create mode 100644 Documentation/devicetree/bindings/net/can/grcan.txt
 create mode 100644 drivers/net/can/grcan.c

diff --git a/Documentation/ABI/testing/sysfs-class-net-grcan 
b/Documentation/ABI/testing/sysfs-class-net-grcan
new file mode 100644
index 000..8fa2f90
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-net-grcan
@@ -0,0 +1,35 @@
+
+What:  /sys/class/net//grcan/enable0
+Date:  October 2012
+KernelVersion: 3.8
+Contact:   Andreas Larsson 
+Description:
+   Hardware configuration of physical interface 0. This file reads
+   and writes the "Enable 0" bit of the configuration register.
+   Possible values: 0 or 1. See the GRCAN chapter of the GRLIB IP
+   core library documentation for details.
+   The default value is set by the module parameter enable0 and can
+   be read at /sys/module/grcan/parameters/enable0.
+
+What:  /sys/class/net//grcan/enable1
+Date:  October 2012
+KernelVersion: 3.8
+Contact:   Andreas Larsson 
+Description:
+   Hardware configuration of physical interface 1. This file reads
+   and writes the "Enable 1" bit of the configuration register.
+   Possible values: 0 or 1. See the GRCAN chapter of the GRLIB IP
+   core library documentation for details.
+   The default value is set by the module parameter enable1 and can
+   be read at /sys/module/grcan/parameters/enable1.
+
+What:  /sys/class/net//grcan/selection
+Date:  October 2012
+KernelVersion: 3.8
+Contact:   Andreas Larsson 
+Description:
+   Configuration of which physical interface to be used. Possible
+   values: 0 or 1. See the GRCAN chapter of the GRLIB IP core
+   library documentation for details.
+   The default value is set by the module parameter selection and 
can
+   be read at /sys/module/grcan/parameters/selection.
diff --git a/Documentation/devicetree/bindings/net/can/grcan.txt 
b/Documentation/devicetree/bindings/net/can/grcan.txt
new file mode 100644
index 000..a7180f1
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/can/grcan.txt
@@ -0,0 +1,27 @@
+CAN controller for Aeroflex Gaisler GRCAN and GRHCAN.
+
+The GRCAN and CRHCAN CAN controllers are available in the GRLIB VHDL IP core
+library.
+
+Note: These properties are built from the AMBA plug&play in a Leon SPARC
+system (the ordinary environment for GRCAN and GRHCAN). There are no bsp
+files for sparc.
+
+Required properties:
+
+- name : Should be "GAISLER_GRCAN", "01_03d", "GAISLER_GRHCAN" or "01_034"
+
+- reg : Address and length of the register set for the device
+
+- freq : Frequency of the external oscillator clock in Hz (the frequency of
+   the amba bus in the ordinary case)
+
+- interrupts : Interrupt number for this device
+
+Optional properties:
+
+- systemid : If not present or if the value of the least significant 16 bits
+   of this 32-bit property is smaller than GRCAN_TXBUG_SAFE_GRLIB_VERSION
+   a bug workaround is activated.
+
+For further information look in the documentation for the GLIB IP core library.
diff --git a/Documentation/kernel-parameters.txt 
b/Documentation/kernel-parameters.txt
index 9776f06..3a82433 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -905,6 +905,31 @@ bytes respectively. Such letter suffixes can also be 
entirely omitted.
gpt [EFI] Forces disk with valid GPT signature but
invalid Protective MBR to be treated as GPT.
 
+   grcan.enable0=  [HW] The "Enable 0" bit of the configuration
+   register. For more documentation, see
+   Documentation/ABI/testing/sysfs-class-net-grcan
+   Format: 0 | 1
+   Default: 0
+   grcan.enable1=  [HW] The "Enable 1" bit of the configuration
+   register. For more documentation, see
+   Documentation/ABI/testing/sysfs-class-net-grcan.
+   Format: 0 | 1
+   Default: 0
+   grcan.selection=
+ 

Re: [PATCH v3] can: grcan: Add device driver for GRCAN and GRHCAN cores

2012-10-30 Thread Andreas Larsson

On 10/30/2012 11:07 AM, Wolfgang Grandegger wrote:

+   /* AHB bus error interrupts (not CAN bus errors) - shut down the
+* device.
+*/
+   if (sources & (GRCAN_IRQ_TXAHBERR | GRCAN_IRQ_RXAHBERR)) {
+   if (sources & GRCAN_IRQ_TXAHBERR) {
+   netdev_err(dev, "got AHB bus error on tx\n");
+   stats->tx_errors++;
+   } else {
+   netdev_err(dev, "got AHB bus error on rx\n");
+   stats->rx_errors++;
+   }
+   netdev_err(dev, "halting device\n");
+
+   /* Prevent anything to be enabled again and halt device */
+   SPIN_LOCK(&priv->lock);
+   priv->closing = true;
+   netif_stop_queue(dev);
+   grcan_stop(dev);
+   SPIN_UNLOCK(&priv->lock);


Hm, does that really happen? How can the user/app realized the problem
and recover?


My understanding of it is that if you get amba bus errors something is seriously 
wrong and nothing can be done at driver level to recover. Shutting down the 
device is to prevent the driver from spamming console messages. I used to have a 
sysfs indication of such errors. Now dmesg is the way to find out about the 
problem. The user can always bring the interface down and up again and try again 
after such an error.




Furthermore, why is a spin_clock enough here? THe interrupt may run a
thread.


It should be enough because the function is only called
directly from the interrupt handler, right? Or did I miss something?



+   priv->can.ctrlmode_supported  =
+   CAN_CTRLMODE_LISTENONLY | CAN_CTRLMODE_ONE_SHOT;


What about triple-sampling?


That is not supported by the hardware.



I curious how the device behaves on bus errors and state changes. Could
you please show the output of "candump -e any,0:0,#" while
sending a CAN message with no other node on the bus (not connected) and
with going bus off (by short-circuiting CAN high and low).



Here is the output (with long sequences of similar error frames
where only one counter is increasing cut out) from the the upcoming v4. let me 
know if you see any problems with this.


  can0  2006  [8] 00 00 00 00 00 00 10 00   ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{}
error-counter-tx-rx{{16}{0}}
  can0  2004  [8] 00 20 00 00 00 00 88 00   ERRORFRAME
controller-problem{tx-error-passive}
error-counter-tx-rx{{136}{0}}
  can0  2006  [8] 00 20 00 00 00 00 90 00   ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{tx-error-passive}
error-counter-tx-rx{{144}{0}}
  [...]
  can0  2006  [8] 00 20 00 00 00 00 F0 00   ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{tx-error-passive}
error-counter-tx-rx{{240}{0}}
  can0  2006  [8] 00 20 00 00 00 00 F8 00   ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{tx-error-passive}
error-counter-tx-rx{{248}{0}}
  can0  2042  [8] 00 00 00 00 00 00 80 00   ERRORFRAME
lost-arbitration{at bit 0}
bus-off
error-counter-tx-rx{{128}{0}}
  can0  2006  [8] 00 00 00 00 00 00 18 00   ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{}
error-counter-tx-rx{{24}{0}}
  can0  2004  [8] 00 10 00 00 00 00 4F 80   ERRORFRAME
controller-problem{rx-error-passive}
error-counter-tx-rx{{79}{128}}
  [...]
  can0  2006  [8] 00 10 00 00 00 00 77 80   ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{rx-error-passive}
error-counter-tx-rx{{119}{128}}
  can0  2006  [8] 00 30 00 00 00 00 7F 80   ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{rx-error-passive,tx-error-passive}
error-counter-tx-rx{{127}{128}}
  can0  2006  [8] 00 30 00 00 00 00 87 80   ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{rx-error-passive,tx-error-passive}
error-counter-tx-rx{{135}{128}}
  [...]
  can0  2006  [8] 00 30 00 00 00 00 F7 80   ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{rx-error-passive,tx-error-passive}
error-counter-tx-rx{{247}{128}}
  can0  2006  [8] 00 30 00 00 00 00 FF 80   ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{rx-error-passive,tx-error-passive}
error-counter-tx-rx{{255}{128}}
  can0  2042  [8] 00 00 00 00 00 00 80 00   ERRORFRAME
lost-arbitration{at bit 0}
bus-off
error-counter-tx-rx{{128}{0}}
  can0  2006  [8] 00 00 00 00 00 00 18 00   ERRORFRAME
lost-arbitration{at bit 0}
controller-problem{}
error-counter-tx-rx{{24}{0}}
  can0  2004  [8] 00 10 00 00 00 00 4F 80   ERRORFRAME
controller-problem{rx-error-passive}
error-counter-tx-rx{{79}{128}}
  can0  2006  [8] 00 10 00 00 00 00 57 80   E

Re: [PATCH v3] can: grcan: Add device driver for GRCAN and GRHCAN cores

2012-10-31 Thread Andreas Larsson

On 2012-10-31 13:51, Wolfgang Grandegger wrote:

On 10/30/2012 05:24 PM, Andreas Larsson wrote:

On 10/30/2012 11:07 AM, Wolfgang Grandegger wrote:

+/* AHB bus error interrupts (not CAN bus errors) - shut down the
+ * device.
+ */
+if (sources & (GRCAN_IRQ_TXAHBERR | GRCAN_IRQ_RXAHBERR)) {
+if (sources & GRCAN_IRQ_TXAHBERR) {
+netdev_err(dev, "got AHB bus error on tx\n");
+stats->tx_errors++;
+} else {
+netdev_err(dev, "got AHB bus error on rx\n");
+stats->rx_errors++;
+}
+netdev_err(dev, "halting device\n");
+
+/* Prevent anything to be enabled again and halt device */
+SPIN_LOCK(&priv->lock);
+priv->closing = true;
+netif_stop_queue(dev);
+grcan_stop(dev);
+SPIN_UNLOCK(&priv->lock);


Hm, does that really happen? How can the user/app realized the problem
and recover?


My understanding of it is that if you get amba bus errors something is
seriously wrong and nothing can be done at driver level to recover.
Shutting down the device is to prevent the driver from spamming console
messages. I used to have a sysfs indication of such errors. Now dmesg is
the way to find out about the problem. The user can always bring the
interface down and up again and try again after such an error.


Well, sounds like a fatal error. Did you ever saw it? If that happens
frequently, or even once, the device/system seems not really usable.


Fatal and yes if this is ever seen something is very bad with the system. I have 
never seen it. However the hardware reports if it would happen, so why not try 
to take care of it in some manner. If this is seen the can device is probably 
useless but it is possible that the rest of the system might be usable. I don't 
really know in what circumstances this would trigger.




Furthermore, why is a spin_clock enough here? THe interrupt may run a
thread.


It should be enough because the function is only called
directly from the interrupt handler, right? Or did I miss something?


The interrupt handler may run as thread, e.g. if the -rt patch has been
applied. Therefore it's safer to use the irqsave/restore variants of the
spin_lock functions. At least that's my understanding. See also
"Documentation/spinlock.txt".


I'll look into this. Seems strange if the same cpu can be interrupted in an 
interrupt handler to run the very same interrupt handler. Sounds like a horrible 
situation for any driver that does not lock down most of its interrupt handler. 
Or maybe there is some situation that I don't foresee.




+priv->can.ctrlmode_supported  =
+CAN_CTRLMODE_LISTENONLY | CAN_CTRLMODE_ONE_SHOT;


What about triple-sampling?


That is not supported by the hardware.


Well, now it will be in future versions of the controller, so support is added 
to the driver.




I curious how the device behaves on bus errors and state changes. Could
you please show the output of "candump -e any,0:0,#" while
sending a CAN message with no other node on the bus (not connected) and
with going bus off (by short-circuiting CAN high and low).



Here is the output (with long sequences of similar error frames
where only one counter is increasing cut out) from the the upcoming v4.
let me know if you see any problems with this.


How did your trigger that sequence? Short-circuit or not cable
connected? The latter, I assume, as bus-off is not reached.


Triggered by short-circuiting, just as asked.




   can0  2006  [8] 00 00 00 00 00 00 10 00   ERRORFRAME
 lost-arbitration{at bit 0}
 controller-problem{}
 error-counter-tx-rx{{16}{0}}


This is most probably an ack slot bus error similar to. You seem not to
handle bus errors and it's not a controller problem as the state did not
change.


No, the hardware has no support for can-bus error reporting.



http://lxr.linux.no/#linux+v3.6.4/drivers/net/can/flexcan.c#L353


   can0  2004  [8] 00 20 00 00 00 00 88 00   ERRORFRAME
 controller-problem{tx-error-passive}
 error-counter-tx-rx{{136}{0}}


OK, a state change to error passive. The device seems not to report
changes to error warning.


No, there is no interrupts or anything that alert about the error states. That 
is why I set the sate based on the error counters in the error handler.




   can0  2006  [8] 00 20 00 00 00 00 90 00   ERRORFRAME
 lost-arbitration{at bit 0}
 controller-problem{tx-error-passive}
 error-counter-tx-rx{{144}{0}}


State changes should be reported only once.


Do you mean that an error message should only be sent on state change?



But it did not change. This
is then a bus error (CAN_ERR_PROT | CAN_ERR_BUSERROR). See also above.


   [...]
   can0  2006  [8] 00 20 00 00 00 00 F0 00   ERRORFRAME
 lost-arbitration{at bit 0}

[PATCH v4] can: grcan: Add device driver for GRCAN and GRHCAN cores

2012-10-31 Thread Andreas Larsson
This driver supports GRCAN and CRHCAN CAN controllers available in the GRLIB
VHDL IP core library.

Signed-off-by: Andreas Larsson 
---
 Documentation/ABI/testing/sysfs-class-net-grcan|   35 +
 .../devicetree/bindings/net/can/grcan.txt  |   27 +
 Documentation/kernel-parameters.txt|   25 +
 drivers/net/can/Kconfig|9 +
 drivers/net/can/Makefile   |1 +
 drivers/net/can/grcan.c| 1667 
 6 files changed, 1764 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-net-grcan
 create mode 100644 Documentation/devicetree/bindings/net/can/grcan.txt
 create mode 100644 drivers/net/can/grcan.c

diff --git a/Documentation/ABI/testing/sysfs-class-net-grcan 
b/Documentation/ABI/testing/sysfs-class-net-grcan
new file mode 100644
index 000..8fa2f90
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-net-grcan
@@ -0,0 +1,35 @@
+
+What:  /sys/class/net//grcan/enable0
+Date:  October 2012
+KernelVersion: 3.8
+Contact:   Andreas Larsson 
+Description:
+   Hardware configuration of physical interface 0. This file reads
+   and writes the "Enable 0" bit of the configuration register.
+   Possible values: 0 or 1. See the GRCAN chapter of the GRLIB IP
+   core library documentation for details.
+   The default value is set by the module parameter enable0 and can
+   be read at /sys/module/grcan/parameters/enable0.
+
+What:  /sys/class/net//grcan/enable1
+Date:  October 2012
+KernelVersion: 3.8
+Contact:   Andreas Larsson 
+Description:
+   Hardware configuration of physical interface 1. This file reads
+   and writes the "Enable 1" bit of the configuration register.
+   Possible values: 0 or 1. See the GRCAN chapter of the GRLIB IP
+   core library documentation for details.
+   The default value is set by the module parameter enable1 and can
+   be read at /sys/module/grcan/parameters/enable1.
+
+What:  /sys/class/net//grcan/selection
+Date:  October 2012
+KernelVersion: 3.8
+Contact:   Andreas Larsson 
+Description:
+   Configuration of which physical interface to be used. Possible
+   values: 0 or 1. See the GRCAN chapter of the GRLIB IP core
+   library documentation for details.
+   The default value is set by the module parameter selection and 
can
+   be read at /sys/module/grcan/parameters/selection.
diff --git a/Documentation/devicetree/bindings/net/can/grcan.txt 
b/Documentation/devicetree/bindings/net/can/grcan.txt
new file mode 100644
index 000..a7180f1
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/can/grcan.txt
@@ -0,0 +1,27 @@
+CAN controller for Aeroflex Gaisler GRCAN and GRHCAN.
+
+The GRCAN and CRHCAN CAN controllers are available in the GRLIB VHDL IP core
+library.
+
+Note: These properties are built from the AMBA plug&play in a Leon SPARC
+system (the ordinary environment for GRCAN and GRHCAN). There are no bsp
+files for sparc.
+
+Required properties:
+
+- name : Should be "GAISLER_GRCAN", "01_03d", "GAISLER_GRHCAN" or "01_034"
+
+- reg : Address and length of the register set for the device
+
+- freq : Frequency of the external oscillator clock in Hz (the frequency of
+   the amba bus in the ordinary case)
+
+- interrupts : Interrupt number for this device
+
+Optional properties:
+
+- systemid : If not present or if the value of the least significant 16 bits
+   of this 32-bit property is smaller than GRCAN_TXBUG_SAFE_GRLIB_VERSION
+   a bug workaround is activated.
+
+For further information look in the documentation for the GLIB IP core library.
diff --git a/Documentation/kernel-parameters.txt 
b/Documentation/kernel-parameters.txt
index 9776f06..3a82433 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -905,6 +905,31 @@ bytes respectively. Such letter suffixes can also be 
entirely omitted.
gpt [EFI] Forces disk with valid GPT signature but
invalid Protective MBR to be treated as GPT.
 
+   grcan.enable0=  [HW] The "Enable 0" bit of the configuration
+   register. For more documentation, see
+   Documentation/ABI/testing/sysfs-class-net-grcan
+   Format: 0 | 1
+   Default: 0
+   grcan.enable1=  [HW] The "Enable 1" bit of the configuration
+   register. For more documentation, see
+   Documentation/ABI/testing/sysfs-class-net-grcan.
+   Format: 0 | 1
+   Default: 0
+   grcan.selection=
+ 

Re: [PATCH v3] can: grcan: Add device driver for GRCAN and GRHCAN cores

2012-11-01 Thread Andreas Larsson

On 2012-10-31 21:21, Wolfgang Grandegger wrote:
>>> Well, sounds like a fatal error. Did you ever saw it? If that happens
>>> frequently, or even once, the device/system seems not really usable.
>>
>> Fatal and yes if this is ever seen something is very bad with the
>> system. I have never seen it. However the hardware reports if it would
>> happen, so why not try to take care of it in some manner. If this is
>> seen the can device is probably useless but it is possible that the rest
>> of the system might be usable. I don't really know in what circumstances
>> this would trigger.
>
> OK, maybe a more serious message would just be fine.

Yes, I have now made it clearer that this is a very serious error.



Furthermore, why is a spin_clock enough here? THe interrupt may run a
thread.


It should be enough because the function is only called
directly from the interrupt handler, right? Or did I miss something?


The interrupt handler may run as thread, e.g. if the -rt patch has been
applied. Therefore it's safer to use the irqsave/restore variants of the
spin_lock functions. At least that's my understanding. See also
"Documentation/spinlock.txt".


I'll look into this. Seems strange if the same cpu can be interrupted in
an interrupt handler to run the very same interrupt handler. Sounds like
a horrible situation for any driver that does not lock down most of its
interrupt handler. Or maybe there is some situation that I don't foresee.


As I said, if you use CONFIG_PREEMPT_RT all interrupt isr will be
threaded (running by threads).


I have now added it, even though I am still not sure if it is needed. Under 
CONFIG_PREEMPT_RT_FULL a spin_lock_irqsave is just a plain spin_lock anyway 
(plus type checking on the flags argument).




This is most probably an ack slot bus error similar to. You seem not to
handle bus errors and it's not a controller problem as the state did not
change.


No, the hardware has no support for can-bus error reporting.


OK, but why is CAN_ERR_CRTL set?


Maybe I am just confused with the terminology. What I mean is that the only 
error reporting that is supported by the hardware (apart from AMBA bus errors, 
but that is unrelated to the discussion) is the error counter related errors.


From what I can see CAN_ERR_BUSERROR is not reported in the error counter 
related cases by the other drivers I looked in. The grcan hardware does not 
support reporting errors like form and stuff errors and which bits the errors 
occured in and the like.


Have I understood it correctly that reporting about protocol errors like that is 
what it is to support CAN_CTRLMODE_BERR_REPORTING? This seems to be the case but 
the janz-ican3 driver connects CAN_CTRLMODE_BERR_REPORTING to all error frame 
reporting, so I am not entirely sure.




http://lxr.linux.no/#linux+v3.6.4/drivers/net/can/flexcan.c#L353


can0  2004  [8] 00 20 00 00 00 00 88 00   ERRORFRAME
  controller-problem{tx-error-passive}
  error-counter-tx-rx{{136}{0}}


OK, a state change to error passive. The device seems not to report
changes to error warning.


No, there is no interrupts or anything that alert about the error
states. That is why I set the sate based on the error counters in the
error handler.


Who does trigger the message above?


Sorry for confusing the matter. There is no interrupt for error warning, but 
there are interrupts for increases of the error counters and the other state 
changes. So to be able to report on error warning I check the error counters and 
do it manually.




Obviously the device restarts automatically after bus-off. Can this be
switched off. The normal procedure is to call "ip ... type can restart"
or to set "restart-ms" for automatic restart after the specified delay.


Yes, the hardware becomes error active after having monitored 11 consecutive 
recessive bits on the bus 128 times (as allowed by the 2.0 CAN spec). There is 
no way of turning this off, so to conform to the normal linux procedure of not 
doing this, I shut down the device on bus off interrupt.


In addition I have thrown out the arbitration lost error frame generation as 
arbitration errors can not be singled out. The TXLOSS interrupt might be due to 
arbitration error, but is also triggered in great numbers when there is no one 
else on the can bus or when there is a problem with the hardware interface or 
the bus itself.


This is how things look currently with no one else on the bus:
~ # cansend can0 123#45
  can0  2004  [8] 00 08 00 00 00 00 60 00   ERRORFRAME
controller-problem{tx-error-warning}
error-counter-tx-rx{{96}{0}}
  can0  2004  [8] 00 20 00 00 00 00 80 00   ERRORFRAME
controller-problem{tx-error-passive}
error-counter-tx-rx{{128}{0}}
~ #

And this is how it looks with a short-circuited bus:
~ # cansend can0 123#45
  can0  2004  [8] 00 08 00 00 00 00 90 00   ERRORFRAME
controller-problem{tx-error-warning}
error-counter-tx-rx{{144}{0}}

[PATCH v5] can: grcan: Add device driver for GRCAN and GRHCAN cores

2012-11-02 Thread Andreas Larsson
This driver supports GRCAN and CRHCAN CAN controllers available in the GRLIB
VHDL IP core library.

Signed-off-by: Andreas Larsson 
---
 Documentation/ABI/testing/sysfs-class-net-grcan|   35 +
 .../devicetree/bindings/net/can/grcan.txt  |   27 +
 Documentation/kernel-parameters.txt|   25 +
 drivers/net/can/Kconfig|9 +
 drivers/net/can/Makefile   |1 +
 drivers/net/can/grcan.c| 1712 
 6 files changed, 1809 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-net-grcan
 create mode 100644 Documentation/devicetree/bindings/net/can/grcan.txt
 create mode 100644 drivers/net/can/grcan.c

diff --git a/Documentation/ABI/testing/sysfs-class-net-grcan 
b/Documentation/ABI/testing/sysfs-class-net-grcan
new file mode 100644
index 000..8fa2f90
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-net-grcan
@@ -0,0 +1,35 @@
+
+What:  /sys/class/net//grcan/enable0
+Date:  October 2012
+KernelVersion: 3.8
+Contact:   Andreas Larsson 
+Description:
+   Hardware configuration of physical interface 0. This file reads
+   and writes the "Enable 0" bit of the configuration register.
+   Possible values: 0 or 1. See the GRCAN chapter of the GRLIB IP
+   core library documentation for details.
+   The default value is set by the module parameter enable0 and can
+   be read at /sys/module/grcan/parameters/enable0.
+
+What:  /sys/class/net//grcan/enable1
+Date:  October 2012
+KernelVersion: 3.8
+Contact:   Andreas Larsson 
+Description:
+   Hardware configuration of physical interface 1. This file reads
+   and writes the "Enable 1" bit of the configuration register.
+   Possible values: 0 or 1. See the GRCAN chapter of the GRLIB IP
+   core library documentation for details.
+   The default value is set by the module parameter enable1 and can
+   be read at /sys/module/grcan/parameters/enable1.
+
+What:  /sys/class/net//grcan/selection
+Date:  October 2012
+KernelVersion: 3.8
+Contact:   Andreas Larsson 
+Description:
+   Configuration of which physical interface to be used. Possible
+   values: 0 or 1. See the GRCAN chapter of the GRLIB IP core
+   library documentation for details.
+   The default value is set by the module parameter selection and 
can
+   be read at /sys/module/grcan/parameters/selection.
diff --git a/Documentation/devicetree/bindings/net/can/grcan.txt 
b/Documentation/devicetree/bindings/net/can/grcan.txt
new file mode 100644
index 000..a7180f1
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/can/grcan.txt
@@ -0,0 +1,27 @@
+CAN controller for Aeroflex Gaisler GRCAN and GRHCAN.
+
+The GRCAN and CRHCAN CAN controllers are available in the GRLIB VHDL IP core
+library.
+
+Note: These properties are built from the AMBA plug&play in a Leon SPARC
+system (the ordinary environment for GRCAN and GRHCAN). There are no bsp
+files for sparc.
+
+Required properties:
+
+- name : Should be "GAISLER_GRCAN", "01_03d", "GAISLER_GRHCAN" or "01_034"
+
+- reg : Address and length of the register set for the device
+
+- freq : Frequency of the external oscillator clock in Hz (the frequency of
+   the amba bus in the ordinary case)
+
+- interrupts : Interrupt number for this device
+
+Optional properties:
+
+- systemid : If not present or if the value of the least significant 16 bits
+   of this 32-bit property is smaller than GRCAN_TXBUG_SAFE_GRLIB_VERSION
+   a bug workaround is activated.
+
+For further information look in the documentation for the GLIB IP core library.
diff --git a/Documentation/kernel-parameters.txt 
b/Documentation/kernel-parameters.txt
index 9776f06..3a82433 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -905,6 +905,31 @@ bytes respectively. Such letter suffixes can also be 
entirely omitted.
gpt [EFI] Forces disk with valid GPT signature but
invalid Protective MBR to be treated as GPT.
 
+   grcan.enable0=  [HW] The "Enable 0" bit of the configuration
+   register. For more documentation, see
+   Documentation/ABI/testing/sysfs-class-net-grcan
+   Format: 0 | 1
+   Default: 0
+   grcan.enable1=  [HW] The "Enable 1" bit of the configuration
+   register. For more documentation, see
+   Documentation/ABI/testing/sysfs-class-net-grcan.
+   Format: 0 | 1
+   Default: 0
+   grcan.selection=
+ 

Re: [PATCH v3] can: grcan: Add device driver for GRCAN and GRHCAN cores

2012-11-06 Thread Andreas Larsson

On 11/05/2012 10:28 AM, Wolfgang Grandegger wrote:

On 11/01/2012 05:08 PM, Andreas Larsson wrote:

On 2012-10-31 21:21, Wolfgang Grandegger wrote:

...

Yes, the hardware becomes error active after having monitored 11
consecutive recessive bits on the bus 128 times (as allowed by the 2.0
CAN spec). There is no way of turning this off, so to conform to the
normal linux procedure of not doing this, I shut down the device on bus
off interrupt.


This should be handled in the following way:

1. If priv->can.restart_ms == 0: do *not* allow automatic restart
That's what you alredy have implemented.

2. If priv->can.restart_ms  > 0 : do allow automatic restart.
This requires to send CAN_ERR_RESTARTED when the system goes
bus-on. See at91_can and mcp251x as example.


In addition I have thrown out the arbitration lost error frame
generation as arbitration errors can not be singled out. The TXLOSS
interrupt might be due to arbitration error, but is also triggered in
great numbers when there is no one else on the can bus or when there is
a problem with the hardware interface or the bus itself.

This is how things look currently with no one else on the bus:
~ # cansend can0 123#45
   can0  2004  [8] 00 08 00 00 00 00 60 00   ERRORFRAME
 controller-problem{tx-error-warning}
 error-counter-tx-rx{{96}{0}}
   can0  2004  [8] 00 20 00 00 00 00 80 00   ERRORFRAME
 controller-problem{tx-error-passive}
 error-counter-tx-rx{{128}{0}}
~ #

And this is how it looks with a short-circuited bus:
~ # cansend can0 123#45
   can0  2004  [8] 00 08 00 00 00 00 90 00   ERRORFRAME
 controller-problem{tx-error-warning}
 error-counter-tx-rx{{144}{0}}
   can0  2004  [8] 00 20 00 00 00 00 98 00   ERRORFRAME
 controller-problem{tx-error-passive}
 error-counter-tx-rx{{152}{0}}
   can0  2040  [8] 00 00 00 00 00 00 00 00   ERRORFRAME
 bus-off
~ #


This looks good now. Just the automatic restart is missing as described
above.


When doing the bus_off handling as in at91_can, on a short-circuited bus with 
restart-ms != 0, the result of a cansend is an endless and frequent stream of


  can0  2004  [8] 00 20 00 00 00 00 88 00   ERRORFRAME
controller-problem{tx-error-passive}
error-counter-tx-rx{{136}{0}}
  can0  2040  [8] 00 00 00 00 00 00 80 00   ERRORFRAME
bus-off
error-counter-tx-rx{{128}{0}}
  can0  2104  [8] 00 00 00 00 00 00 10 00   ERRORFRAME
controller-problem{}
restarted-after-bus-off
error-counter-tx-rx{{16}{0}}
  can0  2004  [8] 00 10 00 00 00 00 57 80   ERRORFRAME
controller-problem{rx-error-passive}
error-counter-tx-rx{{87}{128}}
  can0  2040  [8] 00 00 00 00 00 00 80 00   ERRORFRAME
bus-off
error-counter-tx-rx{{128}{0}}
  can0  2104  [8] 00 00 00 00 00 00 08 00   ERRORFRAME
controller-problem{}
restarted-after-bus-off
error-counter-tx-rx{{8}{0}}
  can0  2004  [8] 00 10 00 00 00 00 57 80   ERRORFRAME
controller-problem{rx-error-passive}
error-counter-tx-rx{{87}{128}}
  can0  2040  [8] 00 00 00 00 00 00 80 00   ERRORFRAME
bus-off
error-counter-tx-rx{{128}{0}}
  can0  2104  [8] 00 00 00 00 00 00 08 00   ERRORFRAME
controller-problem{}
restarted-after-bus-off
error-counter-tx-rx{{8}{0}}
  [...]

as the grcan core continues to try to resend the frame when it comes back again. 
To mimic the sja1000 behavior as closely as possible, I guess that the driver 
also would need to make sure that the tx and rx buffers are cleaned out so that 
this resending does not happen, right?


To do this, the hardware needs to be stopped anyway. Therefore, in my opinion it 
is much simpler to handle it as it is in v5: always shut down the hardware on 
bus off and, in the case of a non-zero restart_ms, let restart timer trigger 
can_restart that will call grcan_set_mode which will restart the hardware with 
empty buffers. Do you see any problems with this approach?


The added benefit of this approach is that then the actual millisecond value of 
the non-zero restart_ms is used instead of having the hardware quickly restart 
regardless of the value.


In any case I have some other fixes for v6.

Cheers,
Andreas
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH v3] can: grcan: Add device driver for GRCAN and GRHCAN cores

2012-11-07 Thread Andreas Larsson

On 11/07/2012 12:15 PM, Wolfgang Grandegger wrote:

On 11/07/2012 08:32 AM, Andreas Larsson wrote:

On 11/05/2012 10:28 AM, Wolfgang Grandegger wrote:

...

This looks good now. Just the automatic restart is missing as described
above.


When doing the bus_off handling as in at91_can, on a short-circuited bus
with restart-ms != 0, the result of a cansend is an endless and frequent
stream of

   can0  2004  [8] 00 20 00 00 00 00 88 00   ERRORFRAME
 controller-problem{tx-error-passive}
 error-counter-tx-rx{{136}{0}}
   can0  2040  [8] 00 00 00 00 00 00 80 00   ERRORFRAME
 bus-off
 error-counter-tx-rx{{128}{0}}
   can0  2104  [8] 00 00 00 00 00 00 10 00   ERRORFRAME
 controller-problem{}
 restarted-after-bus-off
 error-counter-tx-rx{{16}{0}}
   can0  2004  [8] 00 10 00 00 00 00 57 80   ERRORFRAME
 controller-problem{rx-error-passive}
 error-counter-tx-rx{{87}{128}}
   can0  2040  [8] 00 00 00 00 00 00 80 00   ERRORFRAME
 bus-off
 error-counter-tx-rx{{128}{0}}
   can0  2104  [8] 00 00 00 00 00 00 08 00   ERRORFRAME
 controller-problem{}
 restarted-after-bus-off
 error-counter-tx-rx{{8}{0}}
   can0  2004  [8] 00 10 00 00 00 00 57 80   ERRORFRAME
 controller-problem{rx-error-passive}
 error-counter-tx-rx{{87}{128}}
   can0  2040  [8] 00 00 00 00 00 00 80 00   ERRORFRAME
 bus-off
 error-counter-tx-rx{{128}{0}}
   can0  2104  [8] 00 00 00 00 00 00 08 00   ERRORFRAME
 controller-problem{}
 restarted-after-bus-off
 error-counter-tx-rx{{8}{0}}
   [...]

as the grcan core continues to try to resend the frame when it comes
back again. To mimic the sja1000 behavior as closely as possible, I
guess that the driver also would need to make sure that the tx and rx
buffers are cleaned out so that this resending does not happen, right?


No, what you see is the normal behavior for automatic restart by the
hardware. A bus-off recovery is *not* the same than a controller restart.


OK, if these automatic restarts are an OK behavior when restart_ms is non-zero I 
am happy with taking the at91_can approach to things.


Cheers,
Andreas

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH v6] can: grcan: Add device driver for GRCAN and GRHCAN cores

2012-11-07 Thread Andreas Larsson
This driver supports GRCAN and CRHCAN CAN controllers available in the GRLIB
VHDL IP core library.

Signed-off-by: Andreas Larsson 
---
 Documentation/ABI/testing/sysfs-class-net-grcan|   35 +
 .../devicetree/bindings/net/can/grcan.txt  |   28 +
 Documentation/kernel-parameters.txt|   19 +
 drivers/net/can/Kconfig|9 +
 drivers/net/can/Makefile   |1 +
 drivers/net/can/grcan.c| 1739 
 6 files changed, 1831 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-net-grcan
 create mode 100644 Documentation/devicetree/bindings/net/can/grcan.txt
 create mode 100644 drivers/net/can/grcan.c

diff --git a/Documentation/ABI/testing/sysfs-class-net-grcan 
b/Documentation/ABI/testing/sysfs-class-net-grcan
new file mode 100644
index 000..2c4acfb
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-net-grcan
@@ -0,0 +1,35 @@
+
+What:  /sys/class/net//grcan/enable0
+Date:  October 2012
+KernelVersion: 3.8
+Contact:   Andreas Larsson 
+Description:
+   Hardware configuration of physical interface 0. This file reads
+   and writes the "Enable 0" bit of the configuration register.
+   Possible values: 0 or 1. See the GRCAN chapter of the GRLIB IP
+   core library documentation for details. The default value is 0
+   or set by the module parameter grcan.enable0 and can be read at
+   /sys/module/grcan/parameters/enable0.
+
+What:  /sys/class/net//grcan/enable1
+Date:  October 2012
+KernelVersion: 3.8
+Contact:   Andreas Larsson 
+Description:
+   Hardware configuration of physical interface 1. This file reads
+   and writes the "Enable 1" bit of the configuration register.
+   Possible values: 0 or 1. See the GRCAN chapter of the GRLIB IP
+   core library documentation for details. The default value is 0
+   or set by the module parameter grcan.enable1 and can be read at
+   /sys/module/grcan/parameters/enable1.
+
+What:  /sys/class/net//grcan/selection
+Date:  October 2012
+KernelVersion: 3.8
+Contact:   Andreas Larsson 
+Description:
+   Configuration of which physical interface to be used. Possible
+   values: 0 or 1. See the GRCAN chapter of the GRLIB IP core
+   library documentation for details. The default value is 0 or is
+   set by the module parameter grcan.selection and can be read at
+   /sys/module/grcan/parameters/selection.
diff --git a/Documentation/devicetree/bindings/net/can/grcan.txt 
b/Documentation/devicetree/bindings/net/can/grcan.txt
new file mode 100644
index 000..34ef349
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/can/grcan.txt
@@ -0,0 +1,28 @@
+Aeroflex Gaisler GRCAN and GRHCAN CAN controllers.
+
+The GRCAN and CRHCAN CAN controllers are available in the GRLIB VHDL IP core
+library.
+
+Note: These properties are built from the AMBA plug&play in a Leon SPARC system
+(the ordinary environment for GRCAN and GRHCAN). There are no dts files for
+sparc.
+
+Required properties:
+
+- name : Should be "GAISLER_GRCAN", "01_03d", "GAISLER_GRHCAN" or "01_034"
+
+- reg : Address and length of the register set for the device
+
+- freq : Frequency of the external oscillator clock in Hz (the frequency of
+   the amba bus in the ordinary case)
+
+- interrupts : Interrupt number for this device
+
+Optional properties:
+
+- systemid : If not present or if the value of the least significant 16 bits
+   of this 32-bit property is smaller than GRCAN_TXBUG_SAFE_GRLIB_VERSION
+   a bug workaround is activated.
+
+For further information look in the documentation for the GLIB IP core library:
+http://www.gaisler.com/products/grlib/grip.pdf
diff --git a/Documentation/kernel-parameters.txt 
b/Documentation/kernel-parameters.txt
index 9776f06..006fa75 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -905,6 +905,25 @@ bytes respectively. Such letter suffixes can also be 
entirely omitted.
gpt [EFI] Forces disk with valid GPT signature but
invalid Protective MBR to be treated as GPT.
 
+   grcan.enable0=  [HW] Configuration of physical interface 0. Determines
+   the "Enable 0" bit of the configuration register.
+   Format: 0 | 1
+   Default: 0
+   grcan.enable1=  [HW] Configuration of physical interface 1. Determines
+   the "Enable 0" bit of the configuration register.
+   Format: 0 | 1
+   Default: 0
+   grcan.selection=
+   [HW]

Re: [PATCH v6] can: grcan: Add device driver for GRCAN and GRHCAN cores

2012-11-08 Thread Andreas Larsson

On 11/08/2012 09:29 AM, Wolfgang Grandegger wrote:

On 11/07/2012 04:20 PM, Andreas Larsson wrote:

This driver supports GRCAN and CRHCAN CAN controllers available in the GRLIB
VHDL IP core library.

Signed-off-by: Andreas Larsson 


Acked-by: Wolfgang Grandegger 

Looks good now. Thanks for your patience.


Thanks a lot for all the feedback!

Cheers,
Andreas

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH v6] can: grcan: Add device driver for GRCAN and GRHCAN cores

2012-11-08 Thread Andreas Larsson

On 11/08/2012 10:27 AM, Marc Kleine-Budde wrote:

On 11/08/2012 09:29 AM, Wolfgang Grandegger wrote:

On 11/07/2012 04:20 PM, Andreas Larsson wrote:

This driver supports GRCAN and CRHCAN CAN controllers available in the GRLIB
VHDL IP core library.

Signed-off-by: Andreas Larsson 


Acked-by: Wolfgang Grandegger 

Looks good now. Thanks for your patience.


I'm still fighting against the flexcan, I'll have a look at the driver
this afternoon.


Excellent! I have since v6 added a new bit-timing parameter restriction needed 
for the newly added triple sampling mode, so I'll send in a v7.


Cheers,
Andreas

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH v7] can: grcan: Add device driver for GRCAN and GRHCAN cores

2012-11-08 Thread Andreas Larsson
This driver supports GRCAN and CRHCAN CAN controllers available in the GRLIB
VHDL IP core library.

Signed-off-by: Andreas Larsson 
Acked-by: Wolfgang Grandegger 
---
Changes since v6: bittiming parameter checks, variable/constant renames, 
comment edits

 Documentation/ABI/testing/sysfs-class-net-grcan|   35 +
 .../devicetree/bindings/net/can/grcan.txt  |   28 +
 Documentation/kernel-parameters.txt|   18 +
 drivers/net/can/Kconfig|9 +
 drivers/net/can/Makefile   |1 +
 drivers/net/can/grcan.c| 1732 
 6 files changed, 1823 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-net-grcan
 create mode 100644 Documentation/devicetree/bindings/net/can/grcan.txt
 create mode 100644 drivers/net/can/grcan.c

diff --git a/Documentation/ABI/testing/sysfs-class-net-grcan 
b/Documentation/ABI/testing/sysfs-class-net-grcan
new file mode 100644
index 000..f418c92
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-net-grcan
@@ -0,0 +1,35 @@
+
+What:  /sys/class/net//grcan/enable0
+Date:  October 2012
+KernelVersion: 3.8
+Contact:   Andreas Larsson 
+Description:
+   Hardware configuration of physical interface 0. This file reads
+   and writes the "Enable 0" bit of the configuration register.
+   Possible values: 0 or 1. See the GRCAN chapter of the GRLIB IP
+   core library documentation for details. The default value is 0
+   or set by the module parameter grcan.enable0 and can be read at
+   /sys/module/grcan/parameters/enable0.
+
+What:  /sys/class/net//grcan/enable1
+Date:  October 2012
+KernelVersion: 3.8
+Contact:   Andreas Larsson 
+Description:
+   Hardware configuration of physical interface 1. This file reads
+   and writes the "Enable 1" bit of the configuration register.
+   Possible values: 0 or 1. See the GRCAN chapter of the GRLIB IP
+   core library documentation for details. The default value is 0
+   or set by the module parameter grcan.enable1 and can be read at
+   /sys/module/grcan/parameters/enable1.
+
+What:  /sys/class/net//grcan/select
+Date:  October 2012
+KernelVersion: 3.8
+Contact:   Andreas Larsson 
+Description:
+   Configuration of which physical interface to be used. Possible
+   values: 0 or 1. See the GRCAN chapter of the GRLIB IP core
+   library documentation for details. The default value is 0 or is
+   set by the module parameter grcan.select and can be read at
+   /sys/module/grcan/parameters/select.
diff --git a/Documentation/devicetree/bindings/net/can/grcan.txt 
b/Documentation/devicetree/bindings/net/can/grcan.txt
new file mode 100644
index 000..34ef349
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/can/grcan.txt
@@ -0,0 +1,28 @@
+Aeroflex Gaisler GRCAN and GRHCAN CAN controllers.
+
+The GRCAN and CRHCAN CAN controllers are available in the GRLIB VHDL IP core
+library.
+
+Note: These properties are built from the AMBA plug&play in a Leon SPARC system
+(the ordinary environment for GRCAN and GRHCAN). There are no dts files for
+sparc.
+
+Required properties:
+
+- name : Should be "GAISLER_GRCAN", "01_03d", "GAISLER_GRHCAN" or "01_034"
+
+- reg : Address and length of the register set for the device
+
+- freq : Frequency of the external oscillator clock in Hz (the frequency of
+   the amba bus in the ordinary case)
+
+- interrupts : Interrupt number for this device
+
+Optional properties:
+
+- systemid : If not present or if the value of the least significant 16 bits
+   of this 32-bit property is smaller than GRCAN_TXBUG_SAFE_GRLIB_VERSION
+   a bug workaround is activated.
+
+For further information look in the documentation for the GLIB IP core library:
+http://www.gaisler.com/products/grlib/grip.pdf
diff --git a/Documentation/kernel-parameters.txt 
b/Documentation/kernel-parameters.txt
index 9776f06..3da4f96 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -905,6 +905,24 @@ bytes respectively. Such letter suffixes can also be 
entirely omitted.
gpt [EFI] Forces disk with valid GPT signature but
invalid Protective MBR to be treated as GPT.
 
+   grcan.enable0=  [HW] Configuration of physical interface 0. Determines
+   the "Enable 0" bit of the configuration register.
+   Format: 0 | 1
+   Default: 0
+   grcan.enable1=  [HW] Configuration of physical interface 1. Determines
+   the "Enable 0" bit of the configuration register.
+   Format

[PATCH v2 2/2] i2c: i2c-ocores: Add support for the GRLIB port of the controller and custom getreg and setreg functions

2012-11-12 Thread Andreas Larsson
The registers in the GRLIB port of the controller are 32-bit and in big endian
byte order. The PRELOW and PREHIGH registers are merged into one register. The
subsequent registers have their offset decreased accordingly. Hence the register
access needs to be handled in a non-standard manner using custom getreg and
setreg functions.

Signed-off-by: Andreas Larsson 
Acked-by: Peter Korsgaard 
---
Changes since v1:
 - Acked by Peter Korsgaard 

 drivers/i2c/busses/i2c-ocores.c |   57 +-
 1 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index 0b71dc6..a4dc5c4 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -4,6 +4,9 @@
  *
  * Peter Korsgaard 
  *
+ * Support for the GRLIB port of the controller by
+ * Andreas Larsson 
+ *
  * This file is licensed under the terms of the GNU General Public License
  * version 2.  This program is licensed "as is" without any warranty of any
  * kind, whether express or implied.
@@ -38,6 +41,8 @@ struct ocores_i2c {
int nmsgs;
int state; /* see STATE_ */
int clock_khz;
+   void (*setreg)(struct ocores_i2c *i2c, int reg, u8 value);
+   u8 (*getreg)(struct ocores_i2c *i2c, int reg);
 };
 
 /* registers */
@@ -73,7 +78,9 @@ struct ocores_i2c {
 
 static inline void oc_setreg(struct ocores_i2c *i2c, int reg, u8 value)
 {
-   if (i2c->reg_io_width == 4)
+   if (i2c->setreg)
+   i2c->setreg(i2c, reg, value);
+   else if (i2c->reg_io_width == 4)
iowrite32(value, i2c->base + (reg << i2c->reg_shift));
else if (i2c->reg_io_width == 2)
iowrite16(value, i2c->base + (reg << i2c->reg_shift));
@@ -83,7 +90,9 @@ static inline void oc_setreg(struct ocores_i2c *i2c, int reg, 
u8 value)
 
 static inline u8 oc_getreg(struct ocores_i2c *i2c, int reg)
 {
-   if (i2c->reg_io_width == 4)
+   if (i2c->getreg)
+   return i2c->getreg(i2c, reg);
+   else if (i2c->reg_io_width == 4)
return ioread32(i2c->base + (reg << i2c->reg_shift));
else if (i2c->reg_io_width == 2)
return ioread16(i2c->base + (reg << i2c->reg_shift));
@@ -91,6 +100,40 @@ static inline u8 oc_getreg(struct ocores_i2c *i2c, int reg)
return ioread8(i2c->base + (reg << i2c->reg_shift));
 }
 
+/* Read and write functions for the GRLIB port of the controller. Registers are
+ * 32-bit big endian and the PRELOW and PREHIGH registers are merged into one
+ * register. The subsequent registers has their offset decreased accordingly. 
*/
+static u8 oc_getreg_grlib(struct ocores_i2c *i2c, int reg)
+{
+   u32 rd;
+   int rreg = reg;
+   if (reg != OCI2C_PRELOW)
+   rreg--;
+   rd = ioread32be(i2c->base + (rreg << i2c->reg_shift));
+   if (reg == OCI2C_PREHIGH)
+   return (u8)rd >> 8;
+   else
+   return (u8)rd;
+}
+
+static void oc_setreg_grlib(struct ocores_i2c *i2c, int reg, u8 value)
+{
+   u32 curr, wr;
+   int rreg = reg;
+   if (reg != OCI2C_PRELOW)
+   rreg--;
+   if (reg == OCI2C_PRELOW || reg == OCI2C_PREHIGH) {
+   curr = ioread32be(i2c->base + (rreg << i2c->reg_shift));
+   if (reg == OCI2C_PRELOW)
+   wr = (curr & 0xff00) | value;
+   else
+   wr = (((u32)value) << 8) | (curr & 0xff);
+   } else {
+   wr = value;
+   }
+   iowrite32be(wr, i2c->base + (rreg << i2c->reg_shift));
+}
+
 static void ocores_process(struct ocores_i2c *i2c)
 {
struct i2c_msg *msg = i2c->msg;
@@ -233,6 +276,7 @@ static int ocores_i2c_of_probe(struct platform_device *pdev,
 {
struct device_node *np = pdev->dev.of_node;
u32 val;
+   const char *name;
 
if (of_property_read_u32(np, "reg-shift", &i2c->reg_shift)) {
/* no 'reg-shift', check for deprecated 'regstep' */
@@ -257,6 +301,15 @@ static int ocores_i2c_of_probe(struct platform_device 
*pdev,
 
of_property_read_u32(pdev->dev.of_node, "reg-io-width",
&i2c->reg_io_width);
+
+   name = of_get_property(pdev->dev.of_node, "name", NULL);
+   if (name && (!strcmp(name, "GAISLER_I2CMST") ||
+!strcmp(name, "01_028"))) {
+   dev_dbg(&pdev->dev, "GRLIB variant of i2c-ocores\n");
+   i2c->setreg = oc_setreg_grlib;
+   i2c->getreg = oc_getreg_grlib;
+   }
+
return 0;
 }
 #else
-- 
1.7.0.4

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH v2 1/2] i2c: i2c-ocores: Add irq support for sparc

2012-11-12 Thread Andreas Larsson
Add sparc support by using platform_get_irq instead of platform_get_resource.
There are no platform resources of type IORESOURCE_IRQ for sparc, but
platform_get_irq works for sparc. In the non-sparc case platform_get_irq
internally uses platform_get_resource.

Signed-off-by: Andreas Larsson 
Acked-by: Peter Korsgaard 
---
Changes since v1:
 - platform_get_irq now works for sparc, so that is used for all platforms
 - Acked by Peter Korsgaard 

 drivers/i2c/busses/i2c-ocores.c |9 +
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index bffd550..0b71dc6 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -267,7 +267,8 @@ static int __devinit ocores_i2c_probe(struct 
platform_device *pdev)
 {
struct ocores_i2c *i2c;
struct ocores_i2c_platform_data *pdata;
-   struct resource *res, *res2;
+   struct resource *res;
+   int irq;
int ret;
int i;
 
@@ -275,8 +276,8 @@ static int __devinit ocores_i2c_probe(struct 
platform_device *pdev)
if (!res)
return -ENODEV;
 
-   res2 = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-   if (!res2)
+   irq = platform_get_irq(pdev, 0);
+   if (irq < 0)
return -ENODEV;
 
i2c = devm_kzalloc(&pdev->dev, sizeof(*i2c), GFP_KERNEL);
@@ -313,7 +314,7 @@ static int __devinit ocores_i2c_probe(struct 
platform_device *pdev)
ocores_init(i2c);
 
init_waitqueue_head(&i2c->wait);
-   ret = devm_request_irq(&pdev->dev, res2->start, ocores_isr, 0,
+   ret = devm_request_irq(&pdev->dev, irq, ocores_isr, 0,
   pdev->name, i2c);
if (ret) {
dev_err(&pdev->dev, "Cannot claim IRQ\n");
-- 
1.7.0.4

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH v2 0/2] i2c: i2c-ocores: Add support for sparc, custom set and get functions, and the GRLIB port of the controller

2012-11-12 Thread Andreas Larsson
On sparc, irqs are not present as an IORESOURCE in the struct platform_device
representation. By using platform_get_irq instead of platform_get_resource the
driver works for sparc.

The GRLIB port of the ocores i2c controller needs custom getreg and setreg
functions to allow for big endian register access and to deal with the fact that
the PRELOW and PREHIGH registers have been merged into one register.

Signed-off-by: Andreas Larsson 

Changes since v1:
 - platform_get_irq now works for sparc, so that is used for all platforms
 - Acks by Peter Korsgaard 

Andreas Larsson (2):
  i2c: i2c-ocores: Add irq support for sparc
  i2c: i2c-ocores: Add support for the GRLIB port of the controller and
custom getreg and setreg functions

 drivers/i2c/busses/i2c-ocores.c |   66 +++---
 1 files changed, 60 insertions(+), 6 deletions(-)

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH v3 0/2] i2c: i2c-ocores: Add support for sparc, custom set and get functions, and the GRLIB port of the controller

2012-11-13 Thread Andreas Larsson
On sparc, irqs are not present as an IORESOURCE in the struct platform_device
representation. By using platform_get_irq instead of platform_get_resource the
driver works for sparc.

The GRLIB port of the ocores i2c controller needs custom getreg and setreg
functions to allow for big endian register access and to deal with the fact that
the PRELOW and PREHIGH registers have been merged into one register.

Signed-off-by: Andreas Larsson 

 Changes since v2:
 - Return error from platform_get_irq on error
 - Trigger usage of the grlib specific functions on compatible property instead
   of name


Andreas Larsson (2):
  i2c: i2c-ocores: Add irq support for sparc
  i2c: i2c-ocores: Add support for the GRLIB port of the controller and
custom getreg and setreg functions

 drivers/i2c/busses/i2c-ocores.c |   66 ++
 1 files changed, 59 insertions(+), 7 deletions(-)

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH v3 1/2] i2c: i2c-ocores: Add irq support for sparc

2012-11-13 Thread Andreas Larsson
Add sparc support by using platform_get_irq instead of platform_get_resource.
There are no platform resources of type IORESOURCE_IRQ for sparc, but
platform_get_irq works for sparc. In the non-sparc case platform_get_irq
internally uses platform_get_resource.

Signed-off-by: Andreas Larsson 
Acked-by: Peter Korsgaard 
---
 Changes since v2:
 - Return error from platform_get_irq on error

 drivers/i2c/busses/i2c-ocores.c |   11 ++-
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index bffd550..1d204cb 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -267,7 +267,8 @@ static int __devinit ocores_i2c_probe(struct 
platform_device *pdev)
 {
struct ocores_i2c *i2c;
struct ocores_i2c_platform_data *pdata;
-   struct resource *res, *res2;
+   struct resource *res;
+   int irq;
int ret;
int i;
 
@@ -275,9 +276,9 @@ static int __devinit ocores_i2c_probe(struct 
platform_device *pdev)
if (!res)
return -ENODEV;
 
-   res2 = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-   if (!res2)
-   return -ENODEV;
+   irq = platform_get_irq(pdev, 0);
+   if (irq < 0)
+   return irq;
 
i2c = devm_kzalloc(&pdev->dev, sizeof(*i2c), GFP_KERNEL);
if (!i2c)
@@ -313,7 +314,7 @@ static int __devinit ocores_i2c_probe(struct 
platform_device *pdev)
ocores_init(i2c);
 
init_waitqueue_head(&i2c->wait);
-   ret = devm_request_irq(&pdev->dev, res2->start, ocores_isr, 0,
+   ret = devm_request_irq(&pdev->dev, irq, ocores_isr, 0,
   pdev->name, i2c);
if (ret) {
dev_err(&pdev->dev, "Cannot claim IRQ\n");
-- 
1.7.0.4

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH v3 2/2] i2c: i2c-ocores: Add support for the GRLIB port of the controller and custom getreg and setreg functions

2012-11-13 Thread Andreas Larsson
The registers in the GRLIB port of the controller are 32-bit and in big endian
byte order. The PRELOW and PREHIGH registers are merged into one register. The
subsequent registers have their offset decreased accordingly. Hence the register
access needs to be handled in a non-standard manner using custom getreg and
setreg functions.

Signed-off-by: Andreas Larsson 
Acked-by: Peter Korsgaard 
---
 Changes since v2:
 - Trigger usage of the the grlib specific functions on compatible property
   instead of name

 drivers/i2c/busses/i2c-ocores.c |   55 +-
 1 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
index 1d204cb..7abf560 100644
--- a/drivers/i2c/busses/i2c-ocores.c
+++ b/drivers/i2c/busses/i2c-ocores.c
@@ -4,6 +4,9 @@
  *
  * Peter Korsgaard 
  *
+ * Support for the GRLIB port of the controller by
+ * Andreas Larsson 
+ *
  * This file is licensed under the terms of the GNU General Public License
  * version 2.  This program is licensed "as is" without any warranty of any
  * kind, whether express or implied.
@@ -38,6 +41,8 @@ struct ocores_i2c {
int nmsgs;
int state; /* see STATE_ */
int clock_khz;
+   void (*setreg)(struct ocores_i2c *i2c, int reg, u8 value);
+   u8 (*getreg)(struct ocores_i2c *i2c, int reg);
 };
 
 /* registers */
@@ -73,7 +78,9 @@ struct ocores_i2c {
 
 static inline void oc_setreg(struct ocores_i2c *i2c, int reg, u8 value)
 {
-   if (i2c->reg_io_width == 4)
+   if (i2c->setreg)
+   i2c->setreg(i2c, reg, value);
+   else if (i2c->reg_io_width == 4)
iowrite32(value, i2c->base + (reg << i2c->reg_shift));
else if (i2c->reg_io_width == 2)
iowrite16(value, i2c->base + (reg << i2c->reg_shift));
@@ -83,7 +90,9 @@ static inline void oc_setreg(struct ocores_i2c *i2c, int reg, 
u8 value)
 
 static inline u8 oc_getreg(struct ocores_i2c *i2c, int reg)
 {
-   if (i2c->reg_io_width == 4)
+   if (i2c->getreg)
+   return i2c->getreg(i2c, reg);
+   else if (i2c->reg_io_width == 4)
return ioread32(i2c->base + (reg << i2c->reg_shift));
else if (i2c->reg_io_width == 2)
return ioread16(i2c->base + (reg << i2c->reg_shift));
@@ -91,6 +100,40 @@ static inline u8 oc_getreg(struct ocores_i2c *i2c, int reg)
return ioread8(i2c->base + (reg << i2c->reg_shift));
 }
 
+/* Read and write functions for the GRLIB port of the controller. Registers are
+ * 32-bit big endian and the PRELOW and PREHIGH registers are merged into one
+ * register. The subsequent registers has their offset decreased accordingly. 
*/
+static u8 oc_getreg_grlib(struct ocores_i2c *i2c, int reg)
+{
+   u32 rd;
+   int rreg = reg;
+   if (reg != OCI2C_PRELOW)
+   rreg--;
+   rd = ioread32be(i2c->base + (rreg << i2c->reg_shift));
+   if (reg == OCI2C_PREHIGH)
+   return (u8)rd >> 8;
+   else
+   return (u8)rd;
+}
+
+static void oc_setreg_grlib(struct ocores_i2c *i2c, int reg, u8 value)
+{
+   u32 curr, wr;
+   int rreg = reg;
+   if (reg != OCI2C_PRELOW)
+   rreg--;
+   if (reg == OCI2C_PRELOW || reg == OCI2C_PREHIGH) {
+   curr = ioread32be(i2c->base + (rreg << i2c->reg_shift));
+   if (reg == OCI2C_PRELOW)
+   wr = (curr & 0xff00) | value;
+   else
+   wr = (((u32)value) << 8) | (curr & 0xff);
+   } else {
+   wr = value;
+   }
+   iowrite32be(wr, i2c->base + (rreg << i2c->reg_shift));
+}
+
 static void ocores_process(struct ocores_i2c *i2c)
 {
struct i2c_msg *msg = i2c->msg;
@@ -257,6 +300,14 @@ static int ocores_i2c_of_probe(struct platform_device 
*pdev,
 
of_property_read_u32(pdev->dev.of_node, "reg-io-width",
&i2c->reg_io_width);
+
+   if (of_device_is_compatible(pdev->dev.of_node,
+   "aeroflexgaisler,i2cmst")) {
+   dev_dbg(&pdev->dev, "GRLIB variant of i2c-ocores\n");
+   i2c->setreg = oc_setreg_grlib;
+   i2c->getreg = oc_getreg_grlib;
+   }
+
return 0;
 }
 #else
-- 
1.7.0.4

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH v8] can: grcan: Add device driver for GRCAN and GRHCAN cores

2012-11-13 Thread Andreas Larsson

On 11/13/2012 10:15 PM, Marc Kleine-Budde wrote:

[...]


On 11/12/2012 03:57 PM, Andreas Larsson wrote:

>+   bpr = 0; /* Note bpr and brp are different concepts */
>+   rsj = bt->sjw;
>+   ps1 = (bt->prop_seg + bt->phase_seg1) - 1; /* tseg1 - 1 */
>+   ps2 = bt->phase_seg2;
>+   scaler = (bt->brp - 1);
>+   timing |= (bpr << GRCAN_CONF_BPR_BIT) & GRCAN_CONF_BPR;
>+   timing |= (rsj << GRCAN_CONF_RSJ_BIT) & GRCAN_CONF_RSJ;
>+   timing |= (ps1 << GRCAN_CONF_PS1_BIT) & GRCAN_CONF_PS1;
>+   timing |= (ps2 << GRCAN_CONF_PS2_BIT) & GRCAN_CONF_PS2;
>+   timing |= (scaler << GRCAN_CONF_SCALER_BIT) & GRCAN_CONF_SCALER;
>+
>+   netdev_info(dev, "setting timing=0x%x\n", timing);

what about moving the sanity check before putting together the "timing"
variable and doing the netdev_info()?


The idea was for the user to have the full context of the problem when getting 
the error (e.g., when using the bitrate method to set the timing parameters, the 
calculated parameters are not otherwise known to the user). But I can do that 
with a separate netdev_dbg and move the sanity check as suggested.



>+   if (!(ps1 > ps2)) {
>+   netdev_err(dev, "PS1 > PS2 must hold: PS1=%d, PS2=%d\n",
>+  ps1, ps2);
>+   return -EINVAL;
>+   }
>+   if (!(ps2 >= rsj)) {
>+   netdev_err(dev, "PS2 >= RSJ must hold: PS2=%d, RSJ=%d\n",
>+  ps2, rsj);
>+   return -EINVAL;
>+   }
>+
>+   grcan_write_bits(®s->conf, timing, GRCAN_CONF_TIMING);
>+   return 0;
>+}


[...]


>+static int grcan_poll(struct napi_struct *napi, int rx_budget)
>+{
>+   struct grcan_priv *priv = container_of(napi, struct grcan_priv, napi);
>+   struct net_device *dev = priv->dev;
>+   struct grcan_registers __iomem *regs = priv->regs;
>+   int rx_work_done;
>+   unsigned long flags;
>+
>+   /* Receive according to given budget */
>+   rx_work_done = grcan_receive(dev, rx_budget);
>+
>+   /* Catch up echo skb according to separate budget to get the benefits of
>+* napi for tx as well. The given rx_budget might not be appropriate for
>+* the tx side.
>+*/
>+   grcan_transmit_catch_up(dev, GRCAN_TX_BUDGET);
>+
>+   spin_lock_irqsave(&priv->lock, flags);
>+
>+   if (grcan_poll_all_done(dev)) {

Just make it:
if (work_done < budget) {
napi_complete();
enable_interrupts();
}

If there are CAN frames pending, an interrupt will kick in and
reschedule the NAPI.


Sure, I can do that for the first check (and add back checking tx_work_done as 
well). That misses to call napi_complete and start interrupts in the case in 
which handling of frames are complete work_done == budget though. But on the 
other hand, then grcan_poll will be triggered once again and then detect that 
nothing is to be done if that is still the case.


However, the problem with skipping the check after turning on interrupts is that 
more frames can have arrived and/or have been sent after calculating work_done 
and before turning on interrupts. For those frames, unless I have misunderstood 
something, interrupts will not be raised and they can get stuck until (if ever) 
later frames once again trigger rescheduling of napi.



>+   bool complete = true;
>+
>+   if (!priv->closing) {
>+   /* Enable tx and rx interrupts again */
>+   grcan_set_bits(®s->imr, GRCAN_IRQ_TX | GRCAN_IRQ_RX);
>+
>+   /* If more work arrived between detecting completion and
>+* turning on interrupts, we need to keep napi running
>+*/
>+   if (!grcan_poll_all_done(dev)) {
>+   complete = false;
>+   grcan_clear_bits(®s->imr,
>+GRCAN_IRQ_TX | GRCAN_IRQ_RX);
>+   }
>+   }
>+   if (complete)
>+   napi_complete(napi);
>+   }
>+
>+   spin_unlock_irqrestore(&priv->lock, flags);
>+
>+   return rx_work_done;
>+}



Thanks for the feedback!

Cheers,
Andreas


___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH v8] can: grcan: Add device driver for GRCAN and GRHCAN cores

2012-11-14 Thread Andreas Larsson

On 2012-11-14 09:43, Marc Kleine-Budde wrote:

Handle incoming events (rx or tx-complete) until:
a) number of handled events == budget
or
b) no more events pending.

while (work_done < budget && interrupts_pending()) {
work_done += handle_rx(budget - work_done);
work_done += handle_tx(budget - work_done);
}


That could starve handle_tx completely though under high rx pressure, but I can 
prevent that by making sure that half of the budget is held back in the first 
call to handle_rx.



Then, if you have handled less events then budget:
1) call napi_complete()
then
2) enable interrupts.

if (work_done < budget) {
napi_complete();
enable_interrupts();
}

Then, return number of handled events:

return work_done;


Any additional remarks on the following implementation of the poll function?

static int grcan_poll(struct napi_struct *napi, int budget)
{
struct grcan_priv *priv = container_of(napi, struct grcan_priv, napi);
struct net_device *dev = priv->dev;
struct grcan_registers __iomem *regs = priv->regs;
unsigned long flags;
int work_done = 0;
int reserved = budget / 2;

while (work_done < budget) {
int old_work_done = work_done;

/* Prevent grcan_transmit_catch_up from starving by reserving
 * part of the budget in the first iteration when calling
 * grcan_receive.
 */
work_done += grcan_receive(dev, budget - reserved - work_done);
reserved = 0;

/* Catch up echo skb according to same budget, as
 * grcan_transmit_catch_up can trigger echo frames being
 * received.
 */
work_done += grcan_transmit_catch_up(dev, budget - work_done);

/* Break out if nothing was done */
if (work_done == old_work_done)
break;
}

if (work_done < budget) {
napi_complete(napi);

/* Guarantee no interference with a running reset that otherwise
 * could turn off interrupts.
 */
spin_lock_irqsave(&priv->lock, flags);

/* Enable tx and rx interrupts again. No need to check
 * priv->closing as napi_disable in grcan_close is waiting for
 * scheduled napi calls to finish.
 */
grcan_set_bits(®s->imr, GRCAN_IRQ_TX | GRCAN_IRQ_RX);

spin_unlock_irqrestore(&priv->lock, flags);
}

return work_done;
}


Cheers,
Andreas

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH v8] can: grcan: Add device driver for GRCAN and GRHCAN cores

2012-11-14 Thread Andreas Larsson

On 2012-11-14 12:22, Marc Kleine-Budde wrote:

On 11/14/2012 12:02 PM, Andreas Larsson wrote:

On 2012-11-14 09:43, Marc Kleine-Budde wrote:

Handle incoming events (rx or tx-complete) until:
a) number of handled events == budget
or
b) no more events pending.

 while (work_done < budget && interrupts_pending()) {
 work_done += handle_rx(budget - work_done);
 work_done += handle_tx(budget - work_done);
 }


That could starve handle_tx completely though under high rx pressure,
but I can prevent that by making sure that half of the budget is held
back in the first call to handle_rx.


What about making the budget big enough to handle both rx and tx in one
napi call. Have a look at the marvell driver [1] for inspiration.


Even if I set the budget to something large, unless I limit the rx side in some 
way it could go on multiple rounds around the circular buffer until it have used 
all the budget. So in some way or another, grcan_receive must be hindered from 
using all the budget.


Sorry, but I am not sure what aspect of the marvell driver poll handling you 
want me to mimic. Without having analyzed exactly how the queueing works yet, it 
seems to make sure that every function that is called from the poll function 
gets ample opportunity to do work by not letting one function hogging all the 
budget. If you want me to mimic it in the aspect of doing work in series of 16 
or something like that, sure, no problem. If it is something else you want to 
point to, please let me know what.



The simplest way to make sure that both tx and rx gets to run is to take 
inspiration from the ethoc driver [1] and just let the tx side get just as much 
budget as the rx side and be done with it. With the echo frames for can, the 
budget should be halved for each I guess to make sure no more frames are 
delivered than the budget, but apart from that I don't see the problem with such 
a simple approach.


static int ethoc_poll(struct napi_struct *napi, int budget)
{
struct ethoc *priv = container_of(napi, struct ethoc, napi);
int rx_work_done = 0;
int tx_work_done = 0;

rx_work_done = ethoc_rx(priv->netdev, budget);
tx_work_done = ethoc_tx(priv->netdev, budget);

if (rx_work_done < budget && tx_work_done < budget) {
napi_complete(napi);
ethoc_enable_irq(priv, INT_MASK_TX | INT_MASK_RX);
}

return rx_work_done;
}

[1]
http://lxr.free-electrons.com/source/drivers/net/ethernet/ethoc.c#L598

Cheers,
Andreas

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH] gpio: Add device driver for GRGPIO cores

2013-02-04 Thread Andreas Larsson

On 2013-02-02 16:16, Linus Walleij wrote:

+#if defined(__BIG_ENDIAN)
+static inline u32 grgpio_read_reg(u32 __iomem *reg)
+{
+   return ioread32be(reg);
+}
+
+static inline void grgpio_write_reg(u32 __iomem *reg, u32 val)
+{
+   iowrite32be(val, reg);
+}
+#else
[...]


Where is this __BIG_ENDIAN flag coming from?

And do you really have and test this regularly on both LE and BE hardware?
I am worrying a bit about maintenance...


I am more than happy to drop that. I will most probably never test this 
on LE hardware.




+static void grgpio_set_sbit(struct grgpio_priv *priv, u32 __iomem *reg,
+  unsigned offset, int val, u32 *shadow)
+{
+   unsigned long flags;
+
+   spin_lock_irqsave(&priv->lock, flags);
+
+   if (val)
+   *shadow |= (1 << offset);
+   else
+   *shadow &= ~(1 << offset);
+   grgpio_write_reg(reg, *shadow);
+
+   spin_unlock_irqrestore(&priv->lock, flags);
+}


This is all very basic stuff. Please make a best effort to reuse or
augment and reuse this:
drivers/gpio/gpio-generic.c

IIRC this one also handles endianness issues, but I could be wrong.


Sure, many of the initial functions do small basic tasks and resembles 
functions in gpio-generic. But that does not make the hardware itself a 
good candidate to use gpio-generic IMHO:


1) This core is generally running on SPARC, that is BE, but even on 
SPARC, readl and writel (that would be used in gprio-generic) deals with 
LE accesses and my registers are BE.


2) The grgpio_to_irq function is very hardware specific, and there is of 
course no gpio_to_irq support in gpio-generic.


3) Running on SPARC, I get Open Firmware information from prom, so there 
is no platform data to access in the probe function. Of course general 
Open Firmware support could be added to gpio-generic, but in addition my 
probe needs to set up very hardware specific things for gpio_to_irq.



Thank you for the feedback!

Cheers,
Andreas Larsson

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH] gpio: Add device driver for GRGPIO cores

2013-02-04 Thread Andreas Larsson

On 2013-02-04 10:24, Linus Walleij wrote:

And do you really have and test this regularly on both LE and BE hardware?
I am worrying a bit about maintenance...


I am more than happy to drop that. I will most probably never test this on
LE hardware.


Will someone else? I'm more thinking whether it is customary in
the SPARC drivers to do things like this, then we should follow
that pattern of course.


It is definitely not customary in SPARC drivers. The module is marked 
"depends on OF" in Kconfig though and there seems to be no way to depend 
on an endianness. Unless someone instantiates the core in a LE system 
there is no reason for it.




2) The grgpio_to_irq function is very hardware specific, and there is of
course no gpio_to_irq support in gpio-generic.


Well, the idea about gpio-generic is to use the pieces you need
IIRC. You may override.


Ah, I see, bgpio_init is exported, so I might be able use that from my 
driver to get rid of some functions. There is support for BE I saw now. 
It seems broken to me (flips bits, not bytes), but that can be fixed.




3) Running on SPARC, I get Open Firmware information from prom, so there is
no platform data to access in the probe function. Of course general Open
Firmware support could be added to gpio-generic, but in addition my probe
needs to set up very hardware specific things for gpio_to_irq.


We should probably add some way to handle generic GPIO
with compatible strings etc, but that's way outside my competence
so OK. Maybe Rob or Grant can say something.


Might be a good idea. However, by just using bgpio_init in a separate 
driver (like most other users of bgpio_init), that would not be required 
or used by me anyhow.



I'll look into using bgpio_init from my driver.

Cheers,
Andreas Larsson

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH 0/2] gpio: Fix gpio-generic bug and add driver for GRGPIO cores

2013-02-05 Thread Andreas Larsson
The second patch is v2 of the earlier single GRGPIO patch

Andreas Larsson (2):
  gpio: gpio-generic: Fix bug in big endian bit conversion
  gpio: Add device driver for GRGPIO cores

 drivers/gpio/Kconfig|8 ++
 drivers/gpio/Makefile   |1 +
 drivers/gpio/gpio-generic.c |5 +-
 drivers/gpio/gpio-grgpio.c  |  244 +++
 4 files changed, 257 insertions(+), 1 deletions(-)
 create mode 100644 drivers/gpio/gpio-grgpio.c

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH 1/2] gpio: gpio-generic: Fix bug in big endian bit conversion

2013-02-05 Thread Andreas Larsson
The swap to convert LE to BE in bgpio_pin2mask_be should be on byte level, not
on bit level.

Signed-off-by: Andreas Larsson 
---
 drivers/gpio/gpio-generic.c |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/gpio/gpio-generic.c b/drivers/gpio/gpio-generic.c
index 05fcc0f..7f11537 100644
--- a/drivers/gpio/gpio-generic.c
+++ b/drivers/gpio/gpio-generic.c
@@ -112,7 +112,10 @@ static unsigned long bgpio_pin2mask(struct bgpio_chip 
*bgc, unsigned int pin)
 static unsigned long bgpio_pin2mask_be(struct bgpio_chip *bgc,
   unsigned int pin)
 {
-   return 1 << (bgc->bits - 1 - pin);
+   unsigned int bit = pin & 0x7; /* Bit number within byte */
+   unsigned int base = pin - bit; /* Pin that is bit 0 within byte */
+
+   return 1 << ((bgc->bits - base - 8) + bit); /* shifted base + bit */
 }
 
 static int bgpio_get(struct gpio_chip *gc, unsigned int gpio)
-- 
1.7.0.4

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH 2/2] gpio: Add device driver for GRGPIO cores

2013-02-05 Thread Andreas Larsson
This driver supports GRGPIO gpio cores available in the GRLIB VHDL IP core
library.

Signed-off-by: Andreas Larsson 
---
v2 of the eariler single GRGPIO patch

 drivers/gpio/Kconfig   |8 ++
 drivers/gpio/Makefile  |1 +
 drivers/gpio/gpio-grgpio.c |  244 
 3 files changed, 253 insertions(+), 0 deletions(-)
 create mode 100644 drivers/gpio/gpio-grgpio.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 682de75..8437ab7 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -298,6 +298,14 @@ config GPIO_GE_FPGA
  and write pin state) for GPIO implemented in a number of GE single
  board computers.
 
+config GPIO_GRGPIO
+   tristate "Aeroflex Gaisler GRGPIO support"
+   depends on OF
+   select GPIO_GENERIC
+   help
+ Select this to support Aeroflex Gaisler GRGPIO cores from the GRLIB
+ VHDL IP core library.
+
 comment "I2C GPIO expanders:"
 
 config GPIO_ARIZONA
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index c5aebd0..ca5d7a3 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_ARCH_DAVINCI)+= gpio-davinci.o
 obj-$(CONFIG_GPIO_EM)  += gpio-em.o
 obj-$(CONFIG_GPIO_EP93XX)  += gpio-ep93xx.o
 obj-$(CONFIG_GPIO_GE_FPGA) += gpio-ge.o
+obj-$(CONFIG_GPIO_GRGPIO)  += gpio-grgpio.o
 obj-$(CONFIG_GPIO_ICH) += gpio-ich.o
 obj-$(CONFIG_GPIO_IT8761E) += gpio-it8761e.o
 obj-$(CONFIG_GPIO_JANZ_TTL)+= gpio-janz-ttl.o
diff --git a/drivers/gpio/gpio-grgpio.c b/drivers/gpio/gpio-grgpio.c
new file mode 100644
index 000..87eca73
--- /dev/null
+++ b/drivers/gpio/gpio-grgpio.c
@@ -0,0 +1,244 @@
+/*
+ * Driver for Aeroflex Gaisler GRGPIO General Purpose I/O cores.
+ *
+ * 2013 (c) Aeroflex Gaisler AB
+ *
+ * This driver supports the GRGPIO GPIO core available in the GRLIB VHDL IP 
core
+ * library.
+ *
+ * Full documentation of the GRGPIO core can be found here:
+ * http://www.gaisler.com/products/grlib/grip.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.
+ *
+ * Contributors: Andreas Larsson 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DRV_NAME   "grgpio"
+
+#define GRGPIO_MAX_NGPIO 32
+
+struct grgpio_regs {
+   u32 data;   /* 0x00 */
+   u32 output; /* 0x04 */
+   u32 dir;/* 0x08 */
+   u32 imask;  /* 0x0c */
+   u32 ipol;   /* 0x10  */
+   u32 iedge;  /* 0x14 */
+   u32 bypass; /* 0x18 */
+   u32 __reserved; /* 0x1c */
+   u32 imap[8];/* 0x20-0x3c */
+};
+
+struct grgpio_priv {
+   struct bgpio_chip bgc;
+   struct grgpio_regs __iomem *regs;
+
+   u32 imask;  /* irq mask shadow register */
+   s32 *irqmap;/* maps offset to irq or -1 if no irq */
+};
+
+static inline struct grgpio_priv *grgpio_gc_to_priv(struct gpio_chip *gc)
+{
+   struct bgpio_chip *bgc = to_bgpio_chip(gc);
+   return container_of(bgc, struct grgpio_priv, bgc);
+}
+
+static void grgpio_set_imask(struct grgpio_priv *priv, unsigned int offset,
+int val)
+{
+   struct bgpio_chip *bgc = &priv->bgc;
+   unsigned long mask = bgc->pin2mask(bgc, offset);
+   unsigned long flags;
+
+   spin_lock_irqsave(&bgc->lock, flags);
+
+   if (val)
+   priv->imask |= mask;
+   else
+   priv->imask &= ~mask;
+   bgc->write_reg(&priv->regs->imask, priv->imask);
+
+   spin_unlock_irqrestore(&bgc->lock, flags);
+}
+
+static int grgpio_to_irq(struct gpio_chip *gc, unsigned offset)
+{
+   struct grgpio_priv *priv = grgpio_gc_to_priv(gc);
+   int index, irq;
+
+   if (!priv->irqmap || offset > gc->ngpio)
+   return -ENXIO;
+
+   index = priv->irqmap[offset];
+   if (index < 0)
+   return -ENXIO;
+
+   irq = irq_of_parse_and_map(priv->bgc.gc.dev->of_node, index);
+   if (irq) {
+   /* Enable interrupt and return irq */
+   grgpio_set_imask(priv, offset, 1);
+   return irq;
+   } else {
+   return -ENXIO;
+   }
+}
+
+static void grgpio_free(struct gpio_chip *gc, unsigned offset)
+{
+   struct grgpio_priv *priv = grgpio_gc_to_priv(gc);
+   struct bgpio_chip *bgc = &priv->bgc;
+   unsigned long mask = bgc->pin2mask(bgc, offset);
+
+   if (unlikely(priv->imask & mask))
+   grgpio_set_imask(priv, offset, 0);
+}
+
+static int grgpio_probe(struct platform_device *ofdev)
+{
+   struct device

Re: [PATCH] of: Create function for counting number of phandles in a property

2013-02-11 Thread Andreas Larsson
   const char* propname);
+/**
+ * of_gpio_named_count - Count GPIOs for a device
+ * @np:device node to count GPIOs for
+ * @propname:  property name containing gpio specifier(s)
+ *
+ * The function returns the count of GPIOs specified for a node.
+ *
+ * Note that the empty GPIO specifiers counts too. For example,
+ *
+ * gpios = <0
+ *  &pio1 1 2
+ *  0
+ *  &pio2 3 4>;
+ *
+ * defines four GPIOs (so this function will return 4), two of which
+ * are not specified. Returns -EINVAL for an incorrectly formed gpios
+ * property.
+ */
+static int of_gpio_named_count(struct device_node *np, const char* propname)
+{
+   return of_count_phandle_with_args(np, propname, "#gpio-cells");
+}


Should this be static inline int?

I think it would be good to also document that it also returns -ENOENT 
when the propname property is missing, which might be an important case 
to distinguish from the -EINVAL case.



Cheers,
Andreas Larsson

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH v3] gpio: Add device driver for GRGPIO cores and support custom accessors with gpio-generic

2013-02-11 Thread Andreas Larsson
This driver supports GRGPIO gpio cores available in the GRLIB VHDL IP
core library from Aeroflex Gaisler.

This also adds support to gpio-generic for using custom accessor
functions. The grgpio driver uses this to use ioread32be and iowrite32be
for big endian register accesses.

Signed-off-by: Andreas Larsson 
---
 .../devicetree/bindings/gpio/gpio-grgpio.txt   |   29 +++
 drivers/gpio/Kconfig   |8 +
 drivers/gpio/Makefile  |1 +
 drivers/gpio/gpio-generic.c|   26 ++-
 drivers/gpio/gpio-grgpio.c |  255 
 5 files changed, 310 insertions(+), 9 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-grgpio.txt
 create mode 100644 drivers/gpio/gpio-grgpio.c

diff --git a/Documentation/devicetree/bindings/gpio/gpio-grgpio.txt 
b/Documentation/devicetree/bindings/gpio/gpio-grgpio.txt
new file mode 100644
index 000..36f456f
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-grgpio.txt
@@ -0,0 +1,29 @@
+Aeroflex Gaisler GRGPIO General Purpose I/O cores.
+
+The GRGPIO GPIO core is available in the GRLIB VHDL IP core library.
+
+Note: In the ordinary ordinary environment for the GRGPIO core, a Leon SPARC
+system, these properties are built from information in the AMBA plug&play.
+
+Required properties:
+
+- name : Should be "GAISLER_GPIO" or "01_01a"
+
+- reg : Address and length of the register set for the device
+
+- interrupts : Interrupt numbers for this device
+
+Optional properties:
+
+- base : The base gpio number for the core. A dynamic base is used if not
+   present
+
+- nbits : The number of gpio lines. If not present driver assumes 32 lines.
+
+- irqmap : An array with an index for each gpio line. An index is either a 
valid
+   index into the interrupts property array, or 0x that indicates
+   no irq for that line. Driver provides no interrupt support if not
+   present.
+
+For further information look in the documentation for the GLIB IP core library:
+http://www.gaisler.com/products/grlib/grip.pdf
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index ab97eb8..5472778 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -309,6 +309,14 @@ config GPIO_LYNXPOINT
  driver for GPIO functionality on Intel Lynxpoint PCH chipset
  Requires ACPI device enumeration code to set up a platform device.
 
+config GPIO_GRGPIO
+   tristate "Aeroflex Gaisler GRGPIO support"
+   depends on OF
+   select GPIO_GENERIC
+   help
+ Select this to support Aeroflex Gaisler GRGPIO cores from the GRLIB
+ VHDL IP core library.
+
 comment "I2C GPIO expanders:"
 
 config GPIO_ARIZONA
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 4398034..f3b49a2 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_ARCH_DAVINCI)+= gpio-davinci.o
 obj-$(CONFIG_GPIO_EM)  += gpio-em.o
 obj-$(CONFIG_GPIO_EP93XX)  += gpio-ep93xx.o
 obj-$(CONFIG_GPIO_GE_FPGA) += gpio-ge.o
+obj-$(CONFIG_GPIO_GRGPIO)  += gpio-grgpio.o
 obj-$(CONFIG_GPIO_ICH) += gpio-ich.o
 obj-$(CONFIG_GPIO_IT8761E) += gpio-it8761e.o
 obj-$(CONFIG_GPIO_JANZ_TTL)+= gpio-janz-ttl.o
diff --git a/drivers/gpio/gpio-generic.c b/drivers/gpio/gpio-generic.c
index 05fcc0f..f854799 100644
--- a/drivers/gpio/gpio-generic.c
+++ b/drivers/gpio/gpio-generic.c
@@ -251,24 +251,25 @@ static int bgpio_setup_accessors(struct device *dev,
 struct bgpio_chip *bgc,
 bool be)
 {
+   struct bgpio_chip def;
 
switch (bgc->bits) {
case 8:
-   bgc->read_reg   = bgpio_read8;
-   bgc->write_reg  = bgpio_write8;
+   def.read_reg= bgpio_read8;
+   def.write_reg   = bgpio_write8;
break;
case 16:
-   bgc->read_reg   = bgpio_read16;
-   bgc->write_reg  = bgpio_write16;
+   def.read_reg= bgpio_read16;
+   def.write_reg   = bgpio_write16;
break;
case 32:
-   bgc->read_reg   = bgpio_read32;
-   bgc->write_reg  = bgpio_write32;
+   def.read_reg= bgpio_read32;
+   def.write_reg   = bgpio_write32;
break;
 #if BITS_PER_LONG >= 64
case 64:
-   bgc->read_reg   = bgpio_read64;
-   bgc->write_reg  = bgpio_write64;
+   def.read_reg= bgpio_read64;
+   def.write_reg   = bgpio_write64;
break;
 #endif /* BITS_PER_LONG >= 64 */
default:
@@ -276,7 +277,14 @@ static int bgpio_setup_accessors(struct device *dev,
return -EINVAL;
}
 
-   bgc->pin2mask = be ? bgpio_pin2mask_be : bgpio_pin2mask;
+ 

Re: [PATCH] of: Create function for counting number of phandles in a property

2013-02-12 Thread Andreas Larsson

On 2013-02-11 00:58, Grant Likely wrote:

This patch creates of_count_phandle_with_args(), a new function for
counting the number of phandle+argument tuples in a given property. This
is better than the existing method of parsing each phandle individually
until parsing fails which is a horribly slow way to do the count.

It also converts of_gpio_named_count() to use the new function instead
of using the above described horrible method.

This also requires the return value of of_gpio_count() &
of_gpio_named_count() from 'unsigned int' to 'int' so that it can return
an error code. All the users of that function are fixed up to correctly
handle a negative return value.


One more thing: In of_spi_register_master() in drivers/spi.c the error 
code is put in the unsigned variable nb, leading to a huge nb and 
master->num_chipselect with following problems when of_gpio_named_count 
returns an error code.


Cheers,
Andreas Larsson

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH v2 5/6] of_spi: Add fetching of of_gpio flags to of_spi_register_master

2013-02-12 Thread Andreas Larsson

On 2013-02-11 01:22, Grant Likely wrote:

On Tue, 29 Jan 2013 15:53:42 +0100, Andreas Larsson  wrote:

When using a gpio chip select with a OF_GPIO_ACTIVE_LOW flag, this needs to be
known to the controller driver.

Signed-off-by: Andreas Larsson 


Out of curiosity, what do you need the flags for? Polarity of the CS
signal? I think it is long past time to revisit baking polarity control
into core of gpiolib. Would you mind investigating doing that instead of
having to manage it manually in each and every driver?


I don't even need it on my hardware. It seemed like a good thing to have 
via of_spi_register_master. I am OK with just dropping this.


Cheers,
Andreas

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH v2 6/6] of_spi: Initialize cs_gpios and cs_gpio with -EEXIST

2013-02-12 Thread Andreas Larsson

On 2013-02-11 01:23, Grant Likely wrote:

On Tue, 29 Jan 2013 15:53:43 +0100, Andreas Larsson  wrote:

Holes in the cs-gpios DT phandle list is supposed to mark that native
chipselects is to be used. The value returned from of_get_named_gpio_flags in
this case is -EEXIST. By initializing cs_gpios and cs_gpio with -EEXIST, this
and only this errno will indicate to a spi controller driver that a native
chipselect is to be used.

Signed-off-by: Andreas Larsson 


I've left this one off for now. Take a look at the patch I posted and
let me know if you think this one should still be applied.


I think that of_spi_register_master should return an error when the 
cs-gpios property is broken (part of patch 2) and that cs_gpios and 
cs_gpio should be initialized to the same value as a hole in the plist.


This patch does not work without 1 and 2, so I'll submit a modified 
patch once "of: Create function for counting number of phandles in a 
property" has stabilized. Would it have to go through the gpio branch then?


Cheers,
Andreas

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH v3 4/5] of: Create function for counting number of phandles in a property

2013-02-13 Thread Andreas Larsson

On 2013-02-13 00:06, Grant Likely wrote:

This patch creates of_count_phandle_with_args(), a new function for
counting the number of phandle+argument tuples in a given property. This
is better than the existing method of parsing each phandle individually
until parsing fails which is a horribly slow way to do the count.

Tested on ARM using the selftest code.

v3: - Rebased on top of selftest code cleanup patch
v2: - fix bug where of_parse_phandle_with_args() could behave like _count_.
 - made of_gpio_named_count() into a static inline regardless of 
CONFIG_OF_GPIO

Signed-off-by: Grant Likely 
Cc: Linus Walleij 
Cc: Rob Herring 
Cc: Andreas Larsson 


Tested-by: Andreas Larsson 

Cheers,
Andreas Larsson

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH v3 5/5] gpio: Make of_count_named_gpios() use new of_count_phandle_with_args()

2013-02-13 Thread Andreas Larsson

On 2013-02-13 00:06, Grant Likely wrote:

This patch replaces the horribly coded of_count_named_gpios() with a
call to of_count_phandle_with_args() which is far more efficient. This
also changes the return value of of_gpio_count() & of_gpio_named_count()
from 'unsigned int' to 'int' so that it can return an error code. All
the users of that function are fixed up to correctly handle a negative
return value.

v2: Split GPIO portion into a separate patch

Signed-off-by: Grant Likely 
Cc: Linus Walleij 
Cc: Rob Herring 
Cc: Andreas Larsson 


For gpiolib-of.c, of_gpio.h and spi.c:
Tested-by: Andreas Larsson 

Cheers,
Andreas Larsson

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH] spi: Initialize cs_gpio and cs_gpios with -ENOENT

2013-02-13 Thread Andreas Larsson
The return value from of_get_named_gpio is -ENOENT when the given index
matches a hole in the "cs-gpios" property phandle list. However, the
default value of cs_gpio in struct spi_device and entries of cs_gpios in
struct spi_master is -EINVAL, which is documented to indicate that a
GPIO line should not be used for the given spi_device.

This sets the default value of cs_gpio in struct spi_device and entries
of cs_gpios in struct spi_master to -ENOENT. Thus, -ENOENT is the only
value used to indicate that no GPIO line should be used.

Signed-off-by: Andreas Larsson 
---

To be applied to spi/next

 drivers/spi/spi.c   |4 ++--
 include/linux/spi/spi.h |4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 4bca50c..b730a50 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -334,7 +334,7 @@ struct spi_device *spi_alloc_device(struct spi_master 
*master)
spi->dev.parent = &master->dev;
spi->dev.bus = &spi_bus_type;
spi->dev.release = spidev_release;
-   spi->cs_gpio = -EINVAL;
+   spi->cs_gpio = -ENOENT;
device_initialize(&spi->dev);
return spi;
 }
@@ -1083,7 +1083,7 @@ static int of_spi_register_master(struct spi_master 
*master)
return -ENOMEM;
 
for (i = 0; i < master->num_chipselect; i++)
-   cs[i] = -EINVAL;
+   cs[i] = -ENOENT;
 
for (i = 0; i < nb; i++)
cs[i] = of_get_named_gpio(np, "cs-gpios", i);
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 30e9c50..b0ae8b8 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -57,7 +57,7 @@ extern struct bus_type spi_bus_type;
  * @modalias: Name of the driver to use with this device, or an alias
  * for that name.  This appears in the sysfs "modalias" attribute
  * for driver coldplugging, and in uevents used for hotplugging
- * @cs_gpio: gpio number of the chipselect line (optional, -EINVAL when
+ * @cs_gpio: gpio number of the chipselect line (optional, -ENOENT when
  * when not using a GPIO line)
  *
  * A @spi_device is used to interchange data between an SPI slave
@@ -261,7 +261,7 @@ static inline void spi_unregister_driver(struct spi_driver 
*sdrv)
  * queue so the subsystem notifies the driver that it may relax the
  * hardware by issuing this call
  * @cs_gpios: Array of GPIOs to use as chip select lines; one per CS
- * number. Any individual value may be -EINVAL for CS lines that
+ * number. Any individual value may be -ENOENT for CS lines that
  * are not GPIOs (driven by the SPI controller itself).
  *
  * Each SPI master controller can communicate with one or more @spi_device
-- 
1.7.0.4

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH] of: spi: Return error from of_spi_register_master on bad "cs-gpios" property

2013-02-13 Thread Andreas Larsson
This makes sure that an error is returned on an incorrectly formed
"cs-gpios" property, but reports success when the "cs-gpios" property is
well formed or missing.

When holes in the cs-gpios property phandle list is used to indicate
that some other form of chipselect is to be used it is important that
failure to read a broken "cs-gpios" property does not silently fail
leading to the spi controller to use an unintended chipselect.

Signed-off-by: Andreas Larsson 
---

Can only be applied to devicetree/next as it builds upon the
"of: Add helper for counting phandle refernces" patch series.

 drivers/spi/spi.c |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 21c4748..9b5f024 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1068,8 +1068,11 @@ static int of_spi_register_master(struct spi_master 
*master)
nb = of_gpio_named_count(np, "cs-gpios");
master->num_chipselect = max(nb, (int)master->num_chipselect);
 
-   if (nb < 1)
+   /* Return error only for an incorrectly formed cs-gpios property */
+   if (nb == 0 || nb == -ENOENT)
return 0;
+   else if (nb < 0)
+   return nb;
 
cs = devm_kzalloc(&master->dev,
  sizeof(int) * master->num_chipselect,
-- 
1.7.0.4

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH v3] gpio: Add device driver for GRGPIO cores and support custom accessors with gpio-generic

2013-02-13 Thread Andreas Larsson

On 2013-02-13 08:05, Anton Vorontsov wrote:

On Tue, Feb 12, 2013 at 08:24:33AM +0100, Andreas Larsson wrote:

+   res = platform_get_resource(ofdev, IORESOURCE_MEM, 0);
+   regs = devm_request_and_ioremap(&ofdev->dev, res);


Just wonder, is it safe to pass null res to devm_request_and_ioremap()?


Yes, and it is the preferred sequence of calls according to the 
documentation of devm_request_and_ioremap.




+   label = kstrdup(np->full_name, GFP_KERNEL);
+   if (label)
+   gc->label = label;


Do we need to free label? If not, having a comment would be awesome. :)
And should we print a warning for !label case?


Yes, it needs to be freed, thanks! The !label case is kind of OK as the 
gpio system assigns some other label if it is NULL, but I'll require it 
instead to make things easier.




+static int grgpio_remove(struct platform_device *ofdev)
+{
+   dev_set_drvdata(&ofdev->dev, NULL);


Is this really needed?


I guess not.


Thanks for the feedback! I take care of the other comments as well.

Cheers,
Andreas Larsson

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH] gpio: Add device driver for GRGPIO cores and support custom accessors with gpio-generic

2013-02-14 Thread Andreas Larsson
This driver supports GRGPIO gpio cores available in the GRLIB VHDL IP
core library from Aeroflex Gaisler.

This also adds support to gpio-generic for using custom accessor
functions. The grgpio driver uses this to use ioread32be and iowrite32be
for big endian register accesses.

Reviewed-by: Anton Vorontsov 
Signed-off-by: Andreas Larsson 
---

Changes since v3:
- Add Reveiwed-by
- Fix pointed out style issues
- Use np->full_name directly instead of kstrdup'ing it as it is a const char*
- Call gpiochip_remove in grgpio_remove

 .../devicetree/bindings/gpio/gpio-grgpio.txt   |   29 +++
 drivers/gpio/Kconfig   |8 +
 drivers/gpio/Makefile  |1 +
 drivers/gpio/gpio-generic.c|   26 ++-
 drivers/gpio/gpio-grgpio.c |  253 
 5 files changed, 308 insertions(+), 9 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-grgpio.txt
 create mode 100644 drivers/gpio/gpio-grgpio.c

diff --git a/Documentation/devicetree/bindings/gpio/gpio-grgpio.txt 
b/Documentation/devicetree/bindings/gpio/gpio-grgpio.txt
new file mode 100644
index 000..36f456f
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-grgpio.txt
@@ -0,0 +1,29 @@
+Aeroflex Gaisler GRGPIO General Purpose I/O cores.
+
+The GRGPIO GPIO core is available in the GRLIB VHDL IP core library.
+
+Note: In the ordinary ordinary environment for the GRGPIO core, a Leon SPARC
+system, these properties are built from information in the AMBA plug&play.
+
+Required properties:
+
+- name : Should be "GAISLER_GPIO" or "01_01a"
+
+- reg : Address and length of the register set for the device
+
+- interrupts : Interrupt numbers for this device
+
+Optional properties:
+
+- base : The base gpio number for the core. A dynamic base is used if not
+   present
+
+- nbits : The number of gpio lines. If not present driver assumes 32 lines.
+
+- irqmap : An array with an index for each gpio line. An index is either a 
valid
+   index into the interrupts property array, or 0x that indicates
+   no irq for that line. Driver provides no interrupt support if not
+   present.
+
+For further information look in the documentation for the GLIB IP core library:
+http://www.gaisler.com/products/grlib/grip.pdf
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index ab97eb8..5472778 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -309,6 +309,14 @@ config GPIO_LYNXPOINT
  driver for GPIO functionality on Intel Lynxpoint PCH chipset
  Requires ACPI device enumeration code to set up a platform device.
 
+config GPIO_GRGPIO
+   tristate "Aeroflex Gaisler GRGPIO support"
+   depends on OF
+   select GPIO_GENERIC
+   help
+ Select this to support Aeroflex Gaisler GRGPIO cores from the GRLIB
+ VHDL IP core library.
+
 comment "I2C GPIO expanders:"
 
 config GPIO_ARIZONA
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 4398034..f3b49a2 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_ARCH_DAVINCI)+= gpio-davinci.o
 obj-$(CONFIG_GPIO_EM)  += gpio-em.o
 obj-$(CONFIG_GPIO_EP93XX)  += gpio-ep93xx.o
 obj-$(CONFIG_GPIO_GE_FPGA) += gpio-ge.o
+obj-$(CONFIG_GPIO_GRGPIO)  += gpio-grgpio.o
 obj-$(CONFIG_GPIO_ICH) += gpio-ich.o
 obj-$(CONFIG_GPIO_IT8761E) += gpio-it8761e.o
 obj-$(CONFIG_GPIO_JANZ_TTL)+= gpio-janz-ttl.o
diff --git a/drivers/gpio/gpio-generic.c b/drivers/gpio/gpio-generic.c
index 05fcc0f..f854799 100644
--- a/drivers/gpio/gpio-generic.c
+++ b/drivers/gpio/gpio-generic.c
@@ -251,24 +251,25 @@ static int bgpio_setup_accessors(struct device *dev,
 struct bgpio_chip *bgc,
 bool be)
 {
+   struct bgpio_chip def;
 
switch (bgc->bits) {
case 8:
-   bgc->read_reg   = bgpio_read8;
-   bgc->write_reg  = bgpio_write8;
+   def.read_reg= bgpio_read8;
+   def.write_reg   = bgpio_write8;
break;
case 16:
-   bgc->read_reg   = bgpio_read16;
-   bgc->write_reg  = bgpio_write16;
+   def.read_reg= bgpio_read16;
+   def.write_reg   = bgpio_write16;
break;
case 32:
-   bgc->read_reg   = bgpio_read32;
-   bgc->write_reg  = bgpio_write32;
+   def.read_reg= bgpio_read32;
+   def.write_reg   = bgpio_write32;
break;
 #if BITS_PER_LONG >= 64
case 64:
-   bgc->read_reg   = bgpio_read64;
-   bgc->write_reg  = bgpio_write64;
+   def.read_reg= bgpio_read64;
+   def.write_reg   = bgpio_write64;
break;
 #endif /* BITS_PER_LO

[PATCH v4 RESEND] gpio: Add device driver for GRGPIO cores and support custom accessors with gpio-generic

2013-02-14 Thread Andreas Larsson
This driver supports GRGPIO gpio cores available in the GRLIB VHDL IP
core library from Aeroflex Gaisler.

This also adds support to gpio-generic for using custom accessor
functions. The grgpio driver uses this to use ioread32be and iowrite32be
for big endian register accesses.

Reviewed-by: Anton Vorontsov 
Signed-off-by: Andreas Larsson 
---

[Resent as I forgot to mark it as v4]

Changes since v3:
- Add Reveiwed-by
- Fix pointed out style issues
- Use np->full_name directly instead of kstrdup'ing it as it is a const char*
- Call gpiochip_remove in grgpio_remove

 .../devicetree/bindings/gpio/gpio-grgpio.txt   |   29 +++
 drivers/gpio/Kconfig   |8 +
 drivers/gpio/Makefile  |1 +
 drivers/gpio/gpio-generic.c|   26 ++-
 drivers/gpio/gpio-grgpio.c |  253 
 5 files changed, 308 insertions(+), 9 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-grgpio.txt
 create mode 100644 drivers/gpio/gpio-grgpio.c

diff --git a/Documentation/devicetree/bindings/gpio/gpio-grgpio.txt 
b/Documentation/devicetree/bindings/gpio/gpio-grgpio.txt
new file mode 100644
index 000..36f456f
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-grgpio.txt
@@ -0,0 +1,29 @@
+Aeroflex Gaisler GRGPIO General Purpose I/O cores.
+
+The GRGPIO GPIO core is available in the GRLIB VHDL IP core library.
+
+Note: In the ordinary ordinary environment for the GRGPIO core, a Leon SPARC
+system, these properties are built from information in the AMBA plug&play.
+
+Required properties:
+
+- name : Should be "GAISLER_GPIO" or "01_01a"
+
+- reg : Address and length of the register set for the device
+
+- interrupts : Interrupt numbers for this device
+
+Optional properties:
+
+- base : The base gpio number for the core. A dynamic base is used if not
+   present
+
+- nbits : The number of gpio lines. If not present driver assumes 32 lines.
+
+- irqmap : An array with an index for each gpio line. An index is either a 
valid
+   index into the interrupts property array, or 0x that indicates
+   no irq for that line. Driver provides no interrupt support if not
+   present.
+
+For further information look in the documentation for the GLIB IP core library:
+http://www.gaisler.com/products/grlib/grip.pdf
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index ab97eb8..5472778 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -309,6 +309,14 @@ config GPIO_LYNXPOINT
  driver for GPIO functionality on Intel Lynxpoint PCH chipset
  Requires ACPI device enumeration code to set up a platform device.
 
+config GPIO_GRGPIO
+   tristate "Aeroflex Gaisler GRGPIO support"
+   depends on OF
+   select GPIO_GENERIC
+   help
+ Select this to support Aeroflex Gaisler GRGPIO cores from the GRLIB
+ VHDL IP core library.
+
 comment "I2C GPIO expanders:"
 
 config GPIO_ARIZONA
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 4398034..f3b49a2 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_ARCH_DAVINCI)+= gpio-davinci.o
 obj-$(CONFIG_GPIO_EM)  += gpio-em.o
 obj-$(CONFIG_GPIO_EP93XX)  += gpio-ep93xx.o
 obj-$(CONFIG_GPIO_GE_FPGA) += gpio-ge.o
+obj-$(CONFIG_GPIO_GRGPIO)  += gpio-grgpio.o
 obj-$(CONFIG_GPIO_ICH) += gpio-ich.o
 obj-$(CONFIG_GPIO_IT8761E) += gpio-it8761e.o
 obj-$(CONFIG_GPIO_JANZ_TTL)+= gpio-janz-ttl.o
diff --git a/drivers/gpio/gpio-generic.c b/drivers/gpio/gpio-generic.c
index 05fcc0f..f854799 100644
--- a/drivers/gpio/gpio-generic.c
+++ b/drivers/gpio/gpio-generic.c
@@ -251,24 +251,25 @@ static int bgpio_setup_accessors(struct device *dev,
 struct bgpio_chip *bgc,
 bool be)
 {
+   struct bgpio_chip def;
 
switch (bgc->bits) {
case 8:
-   bgc->read_reg   = bgpio_read8;
-   bgc->write_reg  = bgpio_write8;
+   def.read_reg= bgpio_read8;
+   def.write_reg   = bgpio_write8;
break;
case 16:
-   bgc->read_reg   = bgpio_read16;
-   bgc->write_reg  = bgpio_write16;
+   def.read_reg= bgpio_read16;
+   def.write_reg   = bgpio_write16;
break;
case 32:
-   bgc->read_reg   = bgpio_read32;
-   bgc->write_reg  = bgpio_write32;
+   def.read_reg= bgpio_read32;
+   def.write_reg   = bgpio_write32;
break;
 #if BITS_PER_LONG >= 64
case 64:
-   bgc->read_reg   = bgpio_read64;
-   bgc->write_reg  = bgpio_write64;
+   def.read_reg= bgpio_read64;
+   def.write_reg   = bgpio_write64;
 

Re: [PATCH 1/2] gpio: gpio-generic: Fix bug in big endian bit conversion

2013-02-26 Thread Andreas Larsson

On 2013-02-26 08:52, Grant Likely wrote:

On Sat, 09 Feb 2013 14:58:55 +, Grant Likely  
wrote:

On Tue,  5 Feb 2013 11:33:02 +0100, Andreas Larsson  wrote:

The swap to convert LE to BE in bgpio_pin2mask_be should be on byte level, not
on bit level.

Signed-off-by: Andreas Larsson 
---
  drivers/gpio/gpio-generic.c |5 -
  1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/gpio/gpio-generic.c b/drivers/gpio/gpio-generic.c
index 05fcc0f..7f11537 100644
--- a/drivers/gpio/gpio-generic.c
+++ b/drivers/gpio/gpio-generic.c
@@ -112,7 +112,10 @@ static unsigned long bgpio_pin2mask(struct bgpio_chip 
*bgc, unsigned int pin)
  static unsigned long bgpio_pin2mask_be(struct bgpio_chip *bgc,
   unsigned int pin)
  {
-   return 1 << (bgc->bits - 1 - pin);
+   unsigned int bit = pin & 0x7; /* Bit number within byte */
+   unsigned int base = pin - bit; /* Pin that is bit 0 within byte */
+
+   return 1 << ((bgc->bits - base - 8) + bit); /* shifted base + bit */


Ah, sorry for my previous reply. I see you have seen gpio-generic.  :-)

No, the original calculation is correct. BE and LE bit numbering are
opposite, bit Linux always uses LE numbers as far as bit masks are
concerned. Therefore pin 0 is the most significant bit, and
pin (nr_bits-1) is the least significant bit.


Hi Andreas,

Actually I'm wrong here (at least partially) after looking closer at the
generic gpio driver. The problem is that things get messed up on 16 or
32 bit access depending on how the hardware expects to be accessed.
bgpio always uses iowrite/ioread for register access which is inherently
little endian, but if the hardware is wired up as a big-endian device
then you have to do the fiddly bit you did above to get the bit
positions to line up where you what then. So, there could potentially be
4 different ways to count bits on a value ioread() from a gpio register:

little-endian, LSB = 0 (sane)
little-endian, MSB = 0 (odd)
big-endian (bytes swapped), MSB = 0 (common on BE platforms)
big-endian (bytes swapped), LSB = 0 (why are you making my life hard?)


Yes, in v4 of my patch I solved it using custom accessors set outside of 
gpio-generic internally using ioread32be/iowrite32be instead of adding a 
whole set of be variants in gpio-generic.




We /could/ have a ioread32be/write32be mode in the driver, but I don't
think that is the right approach. It means we need yet another set of
accessors for register except using the 'be' variants. Blech. What I'd
actually like to do is add a bitmask field to the gpio_desc which can be
calculated ahead of time to whatever madness is required from the way
the device is wired. Then the access routines don't need to even care.
they just apply the bitmask to ioread/iowrite and it doesn't even need
to know what the bit number actually is. The new support for gpio_desc
in the core code makes this feasable.


I am not sure I understand what you mean here or what new support for 
gpio_desc you are referring to (looking in gpio/next at 
git://git.secretlab.ca/git/linux-2.6).


Do you mean to add something like an 'unsigned long bitmask[64]' bitmap 
array with one bitmap for each gpio line to struct gpio_desc and use 
this primarily by gpio-generic.c, populated in bgpio_init? Is gpio_desc 
now available outside of gpiolib.c in some repository/branch that I 
might be unaware of?


Cheers,
Andreas

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH 1/2] gpio: gpio-generic: Fix bug in big endian bit conversion

2013-02-26 Thread Andreas Larsson

On 2013-02-26 11:46, Andreas Larsson wrote:

On 2013-02-26 08:52, Grant Likely wrote:

On Sat, 09 Feb 2013 14:58:55 +, Grant Likely
 wrote:
We /could/ have a ioread32be/write32be mode in the driver, but I don't
think that is the right approach. It means we need yet another set of
accessors for register except using the 'be' variants. Blech. What I'd
actually like to do is add a bitmask field to the gpio_desc which can be
calculated ahead of time to whatever madness is required from the way
the device is wired. Then the access routines don't need to even care.
they just apply the bitmask to ioread/iowrite and it doesn't even need
to know what the bit number actually is. The new support for gpio_desc
in the core code makes this feasable.


I am not sure I understand what you mean here or what new support for
gpio_desc you are referring to (looking in gpio/next at
git://git.secretlab.ca/git/linux-2.6).

Do you mean to add something like an 'unsigned long bitmask[64]' bitmap
array with one bitmap for each gpio line to struct gpio_desc and use
this primarily by gpio-generic.c, populated in bgpio_init? Is gpio_desc
now available outside of gpiolib.c in some repository/branch that I
might be unaware of?


Ah, I realize that it is the "gpiolib: remove gpio_desc[] static array" 
patch series you refer to, not yet pushed to gpio/next at 
git://git.secretlab.ca/git/linux-2.6.


The question on how you intend the bitmap field to be 
defined/used/populated remains.


Cheers,
Andreas

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH v3] gpio: Add device driver for GRGPIO cores and support custom accessors with gpio-generic

2013-03-04 Thread Andreas Larsson

On 2013-03-01 01:24, Linus Walleij wrote:

On Tue, Feb 12, 2013 at 8:24 AM, Andreas Larsson  wrote:


This driver supports GRGPIO gpio cores available in the GRLIB VHDL IP
core library from Aeroflex Gaisler.

This also adds support to gpio-generic for using custom accessor
functions. The grgpio driver uses this to use ioread32be and iowrite32be
for big endian register accesses.

Signed-off-by: Andreas Larsson 


Can you split this in two patches: one that adds the custom accessors
and one that adds the driver?


Sure!


Grant is currently thinking about optimizations on the call graph
depths of the GPIO functions and may want to take this opportunity
to alter something there.


+++ b/drivers/gpio/gpio-grgpio.c

(...)

+struct grgpio_priv {
+   struct bgpio_chip bgc;
+   struct grgpio_regs __iomem *regs;
+
+   u32 imask;  /* irq mask shadow register */
+   s32 *irqmap;/* maps offset to irq or -1 if no irq */


irqmap? Argh what is this... I think you want to use irqdomain
for this instead. (Documentation/IRQ-domain.txt)


Yeah, that comment is not clear. An entry in the irqmap array (for a 
gpio line) can be either -1 indicating no irq for that line or an index 
into the array of irq:s for the of device. Thus it is simply either -1 
or a valid second argument to irq_of_parse_and_map.


Given that this is generally running on SPARC, it seems irqdomain is not 
an option (IRQ_DOMAIN is not selected by SPARC).


Given this, is just a better formulated comment OK with you in this case?



Check other GPIO drivers (e.g. STMPE or TC3589x) for some
example of how to use irqdomain.


+static int grgpio_to_irq(struct gpio_chip *gc, unsigned offset)
+{
+   struct grgpio_priv *priv = grgpio_gc_to_priv(gc);
+   int index, irq;


This is wher you should use irq_create_mapping(domain, hwirq);


Thanks for the feedback!

Cheers,
Andreas

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH v3] gpio: Add device driver for GRGPIO cores and support custom accessors with gpio-generic

2013-03-08 Thread Andreas Larsson
On Thu, 2013-03-07 at 04:44 +0100, Linus Walleij wrote:
> On Mon, Mar 4, 2013 at 10:46 AM, Andreas Larsson  wrote:
> > [Me]
> >>> +struct grgpio_priv {
> >>> +   struct bgpio_chip bgc;
> >>> +   struct grgpio_regs __iomem *regs;
> >>> +
> >>> +   u32 imask;  /* irq mask shadow register */
> >>> +   s32 *irqmap;/* maps offset to irq or -1 if no irq */
> >>
> >>
> >> irqmap? Argh what is this... I think you want to use irqdomain
> >> for this instead. (Documentation/IRQ-domain.txt)
> >
> >
> > Yeah, that comment is not clear. An entry in the irqmap array (for a gpio
> > line) can be either -1 indicating no irq for that line or an index into the
> > array of irq:s for the of device. Thus it is simply either -1 or a valid
> > second argument to irq_of_parse_and_map.
> 
> So just make the mapping function in the irqdomain handle that?
> 
> Maybe I'm talking weird, I'm not really familiar with
> irq_of_parse_and_map().
> 
> > Given that this is generally running on SPARC, it seems irqdomain is not an
> > option (IRQ_DOMAIN is not selected by SPARC).
> 
> That has nothing to do with this. This driver can just select IRQ_DOMAIN
> in *it's* Kconfig entry.

Oh, excellent! I'll look into an irqdomain solution then.

Thanks for the feedback!

Cheers,
Andreas Larsson


___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH v5 0/3] gpio: Add device driver for GRGPIO cores and support custom accessors with gpio-generic

2013-03-15 Thread Andreas Larsson
Differences since v4:
- Split out changes to gpio-generic into patch 1
- Make the basic driver without any irq support into patch 2, so that
  things can be applied so far if more revisions needs to be done for
  the irq support parts.
- Change irq support to use irq domain and put it in patch 3

Signed-off-by: Andreas Larsson 

Andreas Larsson (3):
  gpio: gpio-generic: Add 16 and 32 bit big endian byte order support
  gpio: grgpio: Add device driver for GRGPIO cores
  gpio: grgpio: Add irq support

 .../devicetree/bindings/gpio/gpio-grgpio.txt   |   29 ++
 drivers/gpio/Kconfig   |9 +
 drivers/gpio/Makefile  |1 +
 drivers/gpio/gpio-generic.c|   56 ++-
 drivers/gpio/gpio-grgpio.c |  489 
 include/linux/basic_mmio_gpio.h|1 +
 6 files changed, 576 insertions(+), 9 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-grgpio.txt
 create mode 100644 drivers/gpio/gpio-grgpio.c

-- 
1.7.10.4

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH v5 1/3] gpio: gpio-generic: Add 16 and 32 bit big endian byte order support

2013-03-15 Thread Andreas Larsson
There is no general support for 64-bit big endian accesses, so that is
left unsupported.

Signed-off-by: Andreas Larsson 
---
 drivers/gpio/gpio-generic.c |   56 ---
 include/linux/basic_mmio_gpio.h |1 +
 2 files changed, 48 insertions(+), 9 deletions(-)

diff --git a/drivers/gpio/gpio-generic.c b/drivers/gpio/gpio-generic.c
index 05fcc0f..42d4706 100644
--- a/drivers/gpio/gpio-generic.c
+++ b/drivers/gpio/gpio-generic.c
@@ -104,6 +104,26 @@ static unsigned long bgpio_read64(void __iomem *reg)
 }
 #endif /* BITS_PER_LONG >= 64 */
 
+static void bgpio_write16be(void __iomem *reg, unsigned long data)
+{
+   iowrite16be(data, reg);
+}
+
+static unsigned long bgpio_read16be(void __iomem *reg)
+{
+   return ioread16be(reg);
+}
+
+static void bgpio_write32be(void __iomem *reg, unsigned long data)
+{
+   iowrite32be(data, reg);
+}
+
+static unsigned long bgpio_read32be(void __iomem *reg)
+{
+   return ioread32be(reg);
+}
+
 static unsigned long bgpio_pin2mask(struct bgpio_chip *bgc, unsigned int pin)
 {
return 1 << pin;
@@ -249,7 +269,8 @@ static int bgpio_dir_out_inv(struct gpio_chip *gc, unsigned 
int gpio, int val)
 
 static int bgpio_setup_accessors(struct device *dev,
 struct bgpio_chip *bgc,
-bool be)
+bool bit_be,
+bool byte_be)
 {
 
switch (bgc->bits) {
@@ -258,17 +279,33 @@ static int bgpio_setup_accessors(struct device *dev,
bgc->write_reg  = bgpio_write8;
break;
case 16:
-   bgc->read_reg   = bgpio_read16;
-   bgc->write_reg  = bgpio_write16;
+   if (byte_be) {
+   bgc->read_reg   = bgpio_read16be;
+   bgc->write_reg  = bgpio_write16be;
+   } else {
+   bgc->read_reg   = bgpio_read16;
+   bgc->write_reg  = bgpio_write16;
+   }
break;
case 32:
-   bgc->read_reg   = bgpio_read32;
-   bgc->write_reg  = bgpio_write32;
+   if (byte_be) {
+   bgc->read_reg   = bgpio_read32be;
+   bgc->write_reg  = bgpio_write32be;
+   } else {
+   bgc->read_reg   = bgpio_read32;
+   bgc->write_reg  = bgpio_write32;
+   }
break;
 #if BITS_PER_LONG >= 64
case 64:
-   bgc->read_reg   = bgpio_read64;
-   bgc->write_reg  = bgpio_write64;
+   if (byte_be) {
+   dev_err(dev,
+   "64 bit big endian byte order unsupported\n");
+   return -EINVAL;
+   } else {
+   bgc->read_reg   = bgpio_read64;
+   bgc->write_reg  = bgpio_write64;
+   }
break;
 #endif /* BITS_PER_LONG >= 64 */
default:
@@ -276,7 +313,7 @@ static int bgpio_setup_accessors(struct device *dev,
return -EINVAL;
}
 
-   bgc->pin2mask = be ? bgpio_pin2mask_be : bgpio_pin2mask;
+   bgc->pin2mask = bit_be ? bgpio_pin2mask_be : bgpio_pin2mask;
 
return 0;
 }
@@ -385,7 +422,8 @@ int bgpio_init(struct bgpio_chip *bgc, struct device *dev,
if (ret)
return ret;
 
-   ret = bgpio_setup_accessors(dev, bgc, flags & BGPIOF_BIG_ENDIAN);
+   ret = bgpio_setup_accessors(dev, bgc, flags & BGPIOF_BIG_ENDIAN,
+   flags & BGPIOF_BIG_ENDIAN_BYTE_ORDER);
if (ret)
return ret;
 
diff --git a/include/linux/basic_mmio_gpio.h b/include/linux/basic_mmio_gpio.h
index 1c504ca..d8a97ec 100644
--- a/include/linux/basic_mmio_gpio.h
+++ b/include/linux/basic_mmio_gpio.h
@@ -72,5 +72,6 @@ int bgpio_init(struct bgpio_chip *bgc, struct device *dev,
 #define BGPIOF_BIG_ENDIAN  BIT(0)
 #define BGPIOF_UNREADABLE_REG_SET  BIT(1) /* reg_set is unreadable */
 #define BGPIOF_UNREADABLE_REG_DIR  BIT(2) /* reg_dir is unreadable */
+#define BGPIOF_BIG_ENDIAN_BYTE_ORDER   BIT(3)
 
 #endif /* __BASIC_MMIO_GPIO_H */
-- 
1.7.10.4

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH v5 2/3] gpio: grgpio: Add device driver for GRGPIO cores

2013-03-15 Thread Andreas Larsson
This driver supports GRGPIO gpio cores available in the GRLIB VHDL IP
core library from Aeroflex Gaisler.

Signed-off-by: Andreas Larsson 
---
 .../devicetree/bindings/gpio/gpio-grgpio.txt   |   24 +++
 drivers/gpio/Kconfig   |9 ++
 drivers/gpio/Makefile  |1 +
 drivers/gpio/gpio-grgpio.c |  164 
 4 files changed, 198 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-grgpio.txt
 create mode 100644 drivers/gpio/gpio-grgpio.c

diff --git a/Documentation/devicetree/bindings/gpio/gpio-grgpio.txt 
b/Documentation/devicetree/bindings/gpio/gpio-grgpio.txt
new file mode 100644
index 000..1050dc8
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-grgpio.txt
@@ -0,0 +1,24 @@
+Aeroflex Gaisler GRGPIO General Purpose I/O cores.
+
+The GRGPIO GPIO core is available in the GRLIB VHDL IP core library.
+
+Note: In the ordinary environment for the GRGPIO core, a Leon SPARC system,
+these properties are built from information in the AMBA plug&play.
+
+Required properties:
+
+- name : Should be "GAISLER_GPIO" or "01_01a"
+
+- reg : Address and length of the register set for the device
+
+- interrupts : Interrupt numbers for this device
+
+Optional properties:
+
+- base : The base gpio number for the core. A dynamic base is used if not
+   present
+
+- nbits : The number of gpio lines. If not present driver assumes 32 lines.
+
+For further information look in the documentation for the GLIB IP core library:
+http://www.gaisler.com/products/grlib/grip.pdf
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 93aaadf..32f068d 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -309,6 +309,15 @@ config GPIO_LYNXPOINT
  driver for GPIO functionality on Intel Lynxpoint PCH chipset
  Requires ACPI device enumeration code to set up a platform device.
 
+config GPIO_GRGPIO
+   tristate "Aeroflex Gaisler GRGPIO support"
+   depends on OF
+   select GPIO_GENERIC
+   select IRQ_DOMAIN
+   help
+ Select this to support Aeroflex Gaisler GRGPIO cores from the GRLIB
+ VHDL IP core library.
+
 comment "I2C GPIO expanders:"
 
 config GPIO_ARIZONA
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 22e07bc..3e6be51 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_ARCH_DAVINCI)+= gpio-davinci.o
 obj-$(CONFIG_GPIO_EM)  += gpio-em.o
 obj-$(CONFIG_GPIO_EP93XX)  += gpio-ep93xx.o
 obj-$(CONFIG_GPIO_GE_FPGA) += gpio-ge.o
+obj-$(CONFIG_GPIO_GRGPIO)  += gpio-grgpio.o
 obj-$(CONFIG_GPIO_ICH) += gpio-ich.o
 obj-$(CONFIG_GPIO_IT8761E) += gpio-it8761e.o
 obj-$(CONFIG_GPIO_JANZ_TTL)+= gpio-janz-ttl.o
diff --git a/drivers/gpio/gpio-grgpio.c b/drivers/gpio/gpio-grgpio.c
new file mode 100644
index 000..f8902da
--- /dev/null
+++ b/drivers/gpio/gpio-grgpio.c
@@ -0,0 +1,164 @@
+/*
+ * Driver for Aeroflex Gaisler GRGPIO General Purpose I/O cores.
+ *
+ * 2013 (c) Aeroflex Gaisler AB
+ *
+ * This driver supports the GRGPIO GPIO core available in the GRLIB VHDL
+ * IP core library.
+ *
+ * Full documentation of the GRGPIO core can be found here:
+ * http://www.gaisler.com/products/grlib/grip.pdf
+ *
+ * See "Documentation/devicetree/bindings/gpio/gpio-grgpio.txt" for
+ * information on open firmware properties.
+ *
+ * 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.
+ *
+ * Contributors: Andreas Larsson 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define GRGPIO_MAX_NGPIO 32
+
+struct grgpio_regs {
+   u32 data;   /* 0x00 */
+   u32 output; /* 0x04 */
+   u32 dir;/* 0x08 */
+   u32 imask;  /* 0x0c */
+   u32 ipol;   /* 0x10  */
+   u32 iedge;  /* 0x14 */
+   u32 bypass; /* 0x18 */
+   u32 __reserved; /* 0x1c */
+   u32 imap[8];/* 0x20-0x3c */
+};
+
+struct grgpio_priv {
+   struct bgpio_chip bgc;
+   struct grgpio_regs __iomem *regs;
+   struct device *dev;
+};
+
+static inline struct grgpio_priv *grgpio_gc_to_priv(struct gpio_chip *gc)
+{
+   struct bgpio_chip *bgc = to_bgpio_chip(gc);
+
+   return container_of(bgc, struct grgpio_priv, bgc);
+}
+
+static int grgpio_to_irq(struct gpio_chip *gc, unsigned offset)
+{
+   return -ENXIO;
+}
+
+static int grgpio_probe(struct platform_device *ofdev)
+{
+   struct device_node *np = ofdev->dev.of_node;
+   struct grgpio_regs __iomem *regs;
+   struct gpio_chip *gc;
+   struct bgpio_chip *bgc;
+   struct grgpio_priv *priv;
+ 

[PATCH v5 3/3] gpio: grgpio: Add irq support

2013-03-15 Thread Andreas Larsson
The drivers sets up an irq domain and hands out unique virqs to irq
capable gpio lines regardless of which underlying irqs maps to which gpio
line.

Signed-off-by: Andreas Larsson 
---
 .../devicetree/bindings/gpio/gpio-grgpio.txt   |5 +
 drivers/gpio/gpio-grgpio.c |  333 +++-
 2 files changed, 334 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpio/gpio-grgpio.txt 
b/Documentation/devicetree/bindings/gpio/gpio-grgpio.txt
index 1050dc8..e28bc3c 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-grgpio.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-grgpio.txt
@@ -20,5 +20,10 @@ Optional properties:
 
 - nbits : The number of gpio lines. If not present driver assumes 32 lines.
 
+- irqmap : An array with an index for each gpio line. An index is either a 
valid
+   index into the interrupts property array, or 0x that indicates
+   no irq for that line. Driver provides no interrupt support if not
+   present.
+
 For further information look in the documentation for the GLIB IP core library:
 http://www.gaisler.com/products/grlib/grip.pdf
diff --git a/drivers/gpio/gpio-grgpio.c b/drivers/gpio/gpio-grgpio.c
index f8902da..3acff77 100644
--- a/drivers/gpio/gpio-grgpio.c
+++ b/drivers/gpio/gpio-grgpio.c
@@ -32,6 +32,9 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
 
 #define GRGPIO_MAX_NGPIO 32
 
@@ -47,10 +50,41 @@ struct grgpio_regs {
u32 imap[8];/* 0x20-0x3c */
 };
 
+struct grgpio_uirq {
+   u8 refcnt; /* Reference counter to manage requesting/freeing of uirq */
+   u8 uirq; /* Underlying virq of the gpio driver */
+};
+
+struct grgpio_lirq {
+   s8 index; /* Index into struct grgpio_priv's uirqs, or -1 */
+   u8 virq; /* virq for the gpio line */
+};
+
 struct grgpio_priv {
struct bgpio_chip bgc;
struct grgpio_regs __iomem *regs;
struct device *dev;
+
+   u32 imask; /* irq mask shadow register */
+
+   /* The grgpio core can have multiple irqs. Severeal gpio lines can
+* potentially be mapped to the same irq. This driver sets up an
+* irq domain and hands out separate virqs to each gpio line
+*/
+   struct irq_domain *domain;
+
+   /* This array contains information on each underlying irq, each
+* irq of the grgpio core itself.
+*/
+   struct grgpio_uirq uirqs[GRGPIO_MAX_NGPIO];
+
+   /* This array contains information for each gpio line on the
+* virtual irqs obtains from this driver. An index value of -1 for
+* a certain gpio line indicates that the line has no
+* irq. Otherwise the index connects the virq to the underlying
+* irq by pointing into the uirqs array.
+*/
+   struct grgpio_lirq lirqs[GRGPIO_MAX_NGPIO];
 };
 
 static inline struct grgpio_priv *grgpio_gc_to_priv(struct gpio_chip *gc)
@@ -60,11 +94,231 @@ static inline struct grgpio_priv *grgpio_gc_to_priv(struct 
gpio_chip *gc)
return container_of(bgc, struct grgpio_priv, bgc);
 }
 
+static void grgpio_set_imask(struct grgpio_priv *priv, unsigned int offset,
+int val)
+{
+   struct bgpio_chip *bgc = &priv->bgc;
+   unsigned long mask = bgc->pin2mask(bgc, offset);
+   unsigned long flags;
+
+   spin_lock_irqsave(&bgc->lock, flags);
+
+   if (val)
+   priv->imask |= mask;
+   else
+   priv->imask &= ~mask;
+   bgc->write_reg(&priv->regs->imask, priv->imask);
+
+   spin_unlock_irqrestore(&bgc->lock, flags);
+}
+
 static int grgpio_to_irq(struct gpio_chip *gc, unsigned offset)
 {
-   return -ENXIO;
+   struct grgpio_priv *priv = grgpio_gc_to_priv(gc);
+
+   if (offset > gc->ngpio)
+   return -ENXIO;
+
+   if (priv->lirqs[offset].index < 0)
+   return -ENXIO;
+
+   return irq_create_mapping(priv->domain, offset);
+}
+
+/*  IRQ chip functions  */
+
+static int grgpio_irq_set_type(struct irq_data *d, unsigned int type)
+{
+   struct grgpio_priv *priv = irq_data_get_irq_chip_data(d);
+   unsigned long flags;
+   u32 mask = BIT(d->hwirq);
+   u32 ipol;
+   u32 iedge;
+   u32 pol;
+   u32 edge;
+
+   switch (type) {
+   case IRQ_TYPE_LEVEL_LOW:
+   pol = 0;
+   edge = 0;
+   break;
+   case IRQ_TYPE_LEVEL_HIGH:
+   pol = mask;
+   edge = 0;
+   break;
+   case IRQ_TYPE_EDGE_FALLING:
+   pol = 0;
+   edge = mask;
+   break;
+   case IRQ_TYPE_EDGE_RISING:
+   pol = mask;
+   edge = mask;
+   break;
+   default:
+   return -EINVAL;
+   }
+
+   spin_lock_irqsave(&priv->bgc.lock, flags);
+
+   ipol = p

[PATCH v6 0/2] gpio: Add device driver for GRGPIO cores

2013-04-17 Thread Andreas Larsson
Changes since v5:
- register accesses via base + constant
- no base irq number from device tree
- no empty irq-handler in patch 1
- change multi line comment style
- no talk of "virtual" irq or virq
- better description of the situation with irqs and underlying irqs
- add missing lock in irq-handler
- add warning printout in irq-handler if no gpio line matches irq

Andreas Larsson (2):
  gpio: grgpio: Add device driver for GRGPIO cores
  gpio: grgpio: Add irq support

 .../devicetree/bindings/gpio/gpio-grgpio.txt   |   26 +
 drivers/gpio/Kconfig   |9 +
 drivers/gpio/Makefile  |1 +
 drivers/gpio/gpio-grgpio.c |  505 
 4 files changed, 541 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-grgpio.txt
 create mode 100644 drivers/gpio/gpio-grgpio.c

-- 
1.7.10.4

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


[PATCH v6 1/2] gpio: grgpio: Add device driver for GRGPIO cores

2013-04-17 Thread Andreas Larsson
This driver supports GRGPIO gpio cores available in the GRLIB VHDL IP
core library from Aeroflex Gaisler.

Signed-off-by: Andreas Larsson 
---

Changes since v5:
- register accesses via base + constant
- no base irq number from device tree
- no empty irq-handler

 .../devicetree/bindings/gpio/gpio-grgpio.txt   |   21 +++
 drivers/gpio/Kconfig   |9 ++
 drivers/gpio/Makefile  |1 +
 drivers/gpio/gpio-grgpio.c |  149 
 4 files changed, 180 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-grgpio.txt
 create mode 100644 drivers/gpio/gpio-grgpio.c

diff --git a/Documentation/devicetree/bindings/gpio/gpio-grgpio.txt 
b/Documentation/devicetree/bindings/gpio/gpio-grgpio.txt
new file mode 100644
index 000..1ec46fd
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-grgpio.txt
@@ -0,0 +1,21 @@
+Aeroflex Gaisler GRGPIO General Purpose I/O cores.
+
+The GRGPIO GPIO core is available in the GRLIB VHDL IP core library.
+
+Note: In the ordinary environment for the GRGPIO core, a Leon SPARC system,
+these properties are built from information in the AMBA plug&play.
+
+Required properties:
+
+- name : Should be "GAISLER_GPIO" or "01_01a"
+
+- reg : Address and length of the register set for the device
+
+- interrupts : Interrupt numbers for this device
+
+Optional properties:
+
+- nbits : The number of gpio lines. If not present driver assumes 32 lines.
+
+For further information look in the documentation for the GLIB IP core library:
+http://www.gaisler.com/products/grlib/grip.pdf
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 93aaadf..32f068d 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -309,6 +309,15 @@ config GPIO_LYNXPOINT
  driver for GPIO functionality on Intel Lynxpoint PCH chipset
  Requires ACPI device enumeration code to set up a platform device.
 
+config GPIO_GRGPIO
+   tristate "Aeroflex Gaisler GRGPIO support"
+   depends on OF
+   select GPIO_GENERIC
+   select IRQ_DOMAIN
+   help
+ Select this to support Aeroflex Gaisler GRGPIO cores from the GRLIB
+ VHDL IP core library.
+
 comment "I2C GPIO expanders:"
 
 config GPIO_ARIZONA
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 22e07bc..3e6be51 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_ARCH_DAVINCI)+= gpio-davinci.o
 obj-$(CONFIG_GPIO_EM)  += gpio-em.o
 obj-$(CONFIG_GPIO_EP93XX)  += gpio-ep93xx.o
 obj-$(CONFIG_GPIO_GE_FPGA) += gpio-ge.o
+obj-$(CONFIG_GPIO_GRGPIO)  += gpio-grgpio.o
 obj-$(CONFIG_GPIO_ICH) += gpio-ich.o
 obj-$(CONFIG_GPIO_IT8761E) += gpio-it8761e.o
 obj-$(CONFIG_GPIO_JANZ_TTL)+= gpio-janz-ttl.o
diff --git a/drivers/gpio/gpio-grgpio.c b/drivers/gpio/gpio-grgpio.c
new file mode 100644
index 000..466a1c7
--- /dev/null
+++ b/drivers/gpio/gpio-grgpio.c
@@ -0,0 +1,149 @@
+/*
+ * Driver for Aeroflex Gaisler GRGPIO General Purpose I/O cores.
+ *
+ * 2013 (c) Aeroflex Gaisler AB
+ *
+ * This driver supports the GRGPIO GPIO core available in the GRLIB VHDL
+ * IP core library.
+ *
+ * Full documentation of the GRGPIO core can be found here:
+ * http://www.gaisler.com/products/grlib/grip.pdf
+ *
+ * See "Documentation/devicetree/bindings/gpio/gpio-grgpio.txt" for
+ * information on open firmware properties.
+ *
+ * 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.
+ *
+ * Contributors: Andreas Larsson 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define GRGPIO_MAX_NGPIO 32
+
+#define GRGPIO_DATA0x00
+#define GRGPIO_OUTPUT  0x04
+#define GRGPIO_DIR 0x08
+#define GRGPIO_IMASK   0x0c
+#define GRGPIO_IPOL0x10
+#define GRGPIO_IEDGE   0x14
+#define GRGPIO_BYPASS  0x18
+#define GRGPIO_IMAP_BASE   0x20
+
+struct grgpio_priv {
+   struct bgpio_chip bgc;
+   void __iomem *regs;
+   struct device *dev;
+};
+
+static inline struct grgpio_priv *grgpio_gc_to_priv(struct gpio_chip *gc)
+{
+   struct bgpio_chip *bgc = to_bgpio_chip(gc);
+
+   return container_of(bgc, struct grgpio_priv, bgc);
+}
+
+static int grgpio_probe(struct platform_device *ofdev)
+{
+   struct device_node *np = ofdev->dev.of_node;
+   void  __iomem *regs;
+   struct gpio_chip *gc;
+   struct bgpio_chip *bgc;
+   struct grgpio_priv *priv;
+   struct resource *res;
+   int err;
+   u32 prop;
+
+   priv = devm_kzalloc(&ofdev->dev, sizeof(*priv), GFP_KERNEL);

[PATCH v6 2/2] gpio: grgpio: Add irq support

2013-04-17 Thread Andreas Larsson
The drivers sets up an irq domain and hands out unique irqs to irq
capable gpio lines regardless of how underlying irq maps to gpio
lines. Any gpio line can map to any one or none of the irqs of the
core, independently of each other.

Signed-off-by: Andreas Larsson 
---

Changes since v5:
- register accesses via base + constant
- change multi line comment style
- no talk of "virtual" irq or virq
- better description of the situation with irqs and underlying irqs
- add missing lock in irq-handler
- add warning printout in irq-handler if no gpio line matches irq

 .../devicetree/bindings/gpio/gpio-grgpio.txt   |5 +
 drivers/gpio/gpio-grgpio.c |  362 +++-
 2 files changed, 364 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpio/gpio-grgpio.txt 
b/Documentation/devicetree/bindings/gpio/gpio-grgpio.txt
index 1ec46fd..e466598 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-grgpio.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-grgpio.txt
@@ -17,5 +17,10 @@ Optional properties:
 
 - nbits : The number of gpio lines. If not present driver assumes 32 lines.
 
+- irqmap : An array with an index for each gpio line. An index is either a 
valid
+   index into the interrupts property array, or 0x that indicates
+   no irq for that line. Driver provides no interrupt support if not
+   present.
+
 For further information look in the documentation for the GLIB IP core library:
 http://www.gaisler.com/products/grlib/grip.pdf
diff --git a/drivers/gpio/gpio-grgpio.c b/drivers/gpio/gpio-grgpio.c
index 466a1c7..8e08b86 100644
--- a/drivers/gpio/gpio-grgpio.c
+++ b/drivers/gpio/gpio-grgpio.c
@@ -32,6 +32,9 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
 
 #define GRGPIO_MAX_NGPIO 32
 
@@ -44,10 +47,49 @@
 #define GRGPIO_BYPASS  0x18
 #define GRGPIO_IMAP_BASE   0x20
 
+/* Structure for an irq of the core - called an underlying irq */
+struct grgpio_uirq {
+   u8 refcnt; /* Reference counter to manage requesting/freeing of uirq */
+   u8 uirq; /* Underlying irq of the gpio driver */
+};
+
+/*
+ * Structure for an irq of a gpio line handed out by this driver. The index is
+ * used to map to the corresponding underlying irq.
+ */
+struct grgpio_lirq {
+   s8 index; /* Index into struct grgpio_priv's uirqs, or -1 */
+   u8 irq; /* irq for the gpio line */
+};
+
 struct grgpio_priv {
struct bgpio_chip bgc;
void __iomem *regs;
struct device *dev;
+
+   u32 imask; /* irq mask shadow register */
+
+   /*
+* The grgpio core can have multiple "underlying" irqs. The gpio lines
+* can be mapped to any one or none of these underlying irqs
+* independently of each other. This driver sets up an irq domain and
+* hands out separate irqs to each gpio line
+*/
+   struct irq_domain *domain;
+
+   /*
+* This array contains information on each underlying irq, each
+* irq of the grgpio core itself.
+*/
+   struct grgpio_uirq uirqs[GRGPIO_MAX_NGPIO];
+
+   /*
+* This array contains information for each gpio line on the irqs
+* obtains from this driver. An index value of -1 for a certain gpio
+* line indicates that the line has no irq. Otherwise the index connects
+* the irq to the underlying irq by pointing into the uirqs array.
+*/
+   struct grgpio_lirq lirqs[GRGPIO_MAX_NGPIO];
 };
 
 static inline struct grgpio_priv *grgpio_gc_to_priv(struct gpio_chip *gc)
@@ -57,6 +99,246 @@ static inline struct grgpio_priv *grgpio_gc_to_priv(struct 
gpio_chip *gc)
return container_of(bgc, struct grgpio_priv, bgc);
 }
 
+static void grgpio_set_imask(struct grgpio_priv *priv, unsigned int offset,
+int val)
+{
+   struct bgpio_chip *bgc = &priv->bgc;
+   unsigned long mask = bgc->pin2mask(bgc, offset);
+   unsigned long flags;
+
+   spin_lock_irqsave(&bgc->lock, flags);
+
+   if (val)
+   priv->imask |= mask;
+   else
+   priv->imask &= ~mask;
+   bgc->write_reg(priv->regs + GRGPIO_IMASK, priv->imask);
+
+   spin_unlock_irqrestore(&bgc->lock, flags);
+}
+
+static int grgpio_to_irq(struct gpio_chip *gc, unsigned offset)
+{
+   struct grgpio_priv *priv = grgpio_gc_to_priv(gc);
+
+   if (offset > gc->ngpio)
+   return -ENXIO;
+
+   if (priv->lirqs[offset].index < 0)
+   return -ENXIO;
+
+   return irq_create_mapping(priv->domain, offset);
+}
+
+/*  IRQ chip functions  */
+
+static int grgpio_irq_set_type(struct irq_data *d, unsigned int type)
+{
+   struct grgpio_priv *priv = irq_data_get_irq_chip_data(d);
+   unsigned long flags;
+   u32 mask = BIT(d->hwirq);
+   u32 ipol;
+