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 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(-)
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 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
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 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 <[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;
@@ -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
>>>>> "Andreas" == Andreas Larsson <[email protected]> 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.
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.
--
Bye, Peter Korsgaard
On 2012-11-13 23:45, Peter Korsgaard wrote:
>>>>>> "Andreas" == Andreas Larsson <[email protected]> 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
>>>>> "Andreas" == Andreas Larsson <[email protected]> writes:
Hi,
>> 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.
Andreas> Yes, that is a good idea. Do you think casting to and from
Andreas> void * in the following solution is too ugly and rather have a
Andreas> struct pointed to, or do you think that would be unnecessary?
I find the casting OK.
Thanks.
--
Bye, Peter Korsgaard