2012-11-12 17:00:29

by Andreas Larsson

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

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 <[email protected]>

Changes since v1:
- platform_get_irq now works for sparc, so that is used for all platforms
- Acks by Peter Korsgaard <[email protected]>

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(-)


2012-11-12 17:00:07

by Andreas Larsson

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

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 <[email protected]>
Acked-by: Peter Korsgaard <[email protected]>
---
Changes since v1:
- platform_get_irq now works for sparc, so that is used for all platforms
- Acked by Peter Korsgaard <[email protected]>

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

2012-11-12 17:00:05

by Andreas Larsson

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

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 <[email protected]>
Acked-by: Peter Korsgaard <[email protected]>
---
Changes since v1:
- Acked by Peter Korsgaard <[email protected]>

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 <[email protected]>
*
+ * Support for the GRLIB port of the controller by
+ * Andreas Larsson <[email protected]>
+ *
* 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

2012-11-13 11:11:16

by Wolfram Sang

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

Hi,

On Mon, Nov 12, 2012 at 05:59:50PM +0100, Andreas Larsson wrote:
> 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 <[email protected]>
> Acked-by: Peter Korsgaard <[email protected]>
> ---
> Changes since v1:
> - platform_get_irq now works for sparc, so that is used for all platforms
> - Acked by Peter Korsgaard <[email protected]>
>
> 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;

Why not pass the error code you got?

>
> 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");

Rest looks good.

--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |


Attachments:
(No filename) (2.05 kB)
signature.asc (198.00 B)
Digital signature
Download all attachments

2012-11-13 11:13:56

by Wolfram Sang

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

Hi,

On Mon, Nov 12, 2012 at 05:59:51PM +0100, Andreas Larsson wrote:

> @@ -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;
> + }
> +

I'd think we should handle this via a seperate compatible-entry and
match->data?

Regards,

Wolfram

--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |


Attachments:
(No filename) (1.08 kB)
signature.asc (198.00 B)
Digital signature
Download all attachments

2012-11-13 13:18:45

by Rob Herring

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



On 11/13/2012 05:10 AM, Wolfram Sang wrote:
> Hi,
>
> On Mon, Nov 12, 2012 at 05:59:50PM +0100, Andreas Larsson wrote:
>> 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 <[email protected]>
>> Acked-by: Peter Korsgaard <[email protected]>
>> ---
>> Changes since v1:
>> - platform_get_irq now works for sparc, so that is used for all platforms
>> - Acked by Peter Korsgaard <[email protected]>
>>
>> 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;
>
> Why not pass the error code you got?

I believe that should actually be a check for 'irq <= 0' as it returns
NO_IRQ on error.

>>
>> 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");
>
> Rest looks good.
>

2012-11-13 16:31:31

by Wolfram Sang

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

Hi,

> >> @@ -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;
> >
> > Why not pass the error code you got?
>
> I believe that should actually be a check for 'irq <= 0' as it returns
> NO_IRQ on error.

I wish it did, but (from 3.7-rc5):

79 /**
80 * platform_get_irq - get an IRQ for a device
81 * @dev: platform device
82 * @num: IRQ number index
83 */
84 int platform_get_irq(struct platform_device *dev, unsigned int num)
85 {
86 #ifdef CONFIG_SPARC
87 /* sparc does not have irqs represented as IORESOURCE_IRQ resources */
88 if (!dev || num >= dev->archdata.num_irqs)
89 return -ENXIO;
90 return dev->archdata.irqs[num];
91 #else
92 struct resource *r = platform_get_resource(dev, IORESOURCE_IRQ, num);
93
94 return r ? r->start : -ENXIO;
95 #endif
96 }
97 EXPORT_SYMBOL_GPL(platform_get_irq);

--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |


Attachments:
(No filename) (1.27 kB)
signature.asc (198.00 B)
Digital signature
Download all attachments