Skip to contentSkip to conversation

i2c: append hardware lock with bus lock

PATCH In review Haojian Zhuang 5 participants 15 messages Apr 28, 2011 – May 10, 2011 1 message not in the archive

Subsystems (5)
Arm (32-bit) Arm sub-architectures I2C subsystem host drivers I2C +1 more subsystems
lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): 2011042801

Haojian Zhuang

author
#1 lore raw

PATCH 1/3 i2c: append hardware lock with bus lock

Both AP and CP are contained in Marvell PXA910 silicon. These two ARM cores are sharing one pair of I2C pins.

In order to keep I2C transaction operated with atomic, hardware lock (RIPC) is required. Because of this, bus lock in AP side can't afford this requirement. Now hardware lock is appended.

Signed-off-by Haojian Zhuang <[email protected]>
Cc Ben Dooks <[email protected]>
---
 drivers/i2c/i2c-core.c |   22 ++++++++++++++++++----
 include/linux/i2c.h    |    3 +++
 2 files changed, 21 insertions(+), 4 deletions(-)
drivers/i2c/i2c-core.c +18 -4
index 045ba6e..412c7a5 100644
@@ -448,8 +448,11 @@ void i2c_lock_adapter(struct i2c_adapter *adapter)
 
 	if (parent)
 		i2c_lock_adapter(parent);
-	else
+	else {
 		rt_mutex_lock(&adapter->bus_lock);
+		if (adapter->hardware_lock)
+			adapter->hardware_lock();
+	}
 }
 EXPORT_SYMBOL_GPL(i2c_lock_adapter);
 

@@ -460,11 +463,19 @@ EXPORT_SYMBOL_GPL(i2c_lock_adapter);
 static int i2c_trylock_adapter(struct i2c_adapter *adapter)
 {
 	struct i2c_adapter *parent = i2c_parent_is_i2c_adapter(adapter);
+	int ret = 0;
 
 	if (parent)
 		return i2c_trylock_adapter(parent);
-	else
-		return rt_mutex_trylock(&adapter->bus_lock);
+	else {
+		ret = rt_mutex_trylock(&adapter->bus_lock);
+		if (ret && adapter->hardware_trylock) {
+			ret = adapter->hardware_trylock();
+			if (!ret)
+				i2c_unlock_adapter(adapter);
+		}
+		return ret;
+	}
 }
 
 /**

@@ -477,8 +488,11 @@ void i2c_unlock_adapter(struct i2c_adapter *adapter)
 
 	if (parent)
 		i2c_unlock_adapter(parent);
-	else
+	else {
+		if (adapter->hardware_unlock)
+			adapter->hardware_unlock();
 		rt_mutex_unlock(&adapter->bus_lock);
+	}
 }
 EXPORT_SYMBOL_GPL(i2c_unlock_adapter);
include/linux/i2c.h +3 -0
index 06a8d9c..b283b4e 100644
@@ -361,6 +361,9 @@ struct i2c_adapter {
 
 	/* data fields that are valid for all devices	*/
 	struct rt_mutex bus_lock;
+	void (*hardware_lock)(void);
+	void (*hardware_unlock)(void);
+	int (*hardware_trylock)(void);
 
 	int timeout;			/* in jiffies */
 	int retries;
Signature · 1 line
1.7.1
#2 lore raw

Hi Haojian,

Quoted 6 lines · Haojian Zhuang

Both AP and CP are contained in Marvell PXA910 silicon. These two ARM cores are sharing one pair of I2C pins.

In order to keep I2C transaction operated with atomic, hardware lock (RIPC) is required. Because of this, bus lock in AP side can't afford this requirement. Now hardware lock is appended.

I have no objection to the idea, but one question: when using the hardware lock, isn't the software mutex redundant? I would expect that you call the hardware_lock/unlock functions _instead_ of rt_mutex_lock/unlock, rather than in addition to it. Or do you still need the rt_mutex to prevent priority inversion?

Quoted 74 lines
Signed-off-by Haojian Zhuang <[email protected]>
Cc Ben Dooks <[email protected]>
---
 drivers/i2c/i2c-core.c |   22 ++++++++++++++++++----
 include/linux/i2c.h    |    3 +++
 2 files changed, 21 insertions(+), 4 deletions(-)
drivers/i2c/i2c-core.c +18 -4
index 045ba6e..412c7a5 100644
@@ -448,8 +448,11 @@ void i2c_lock_adapter(struct i2c_adapter *adapter)
 
 	if (parent)
 		i2c_lock_adapter(parent);
-	else
+	else {
 		rt_mutex_lock(&adapter->bus_lock);
+		if (adapter->hardware_lock)
+			adapter->hardware_lock();
+	}
 }
 EXPORT_SYMBOL_GPL(i2c_lock_adapter);
 

@@ -460,11 +463,19 @@ EXPORT_SYMBOL_GPL(i2c_lock_adapter);
 static int i2c_trylock_adapter(struct i2c_adapter *adapter)
 {
 	struct i2c_adapter *parent = i2c_parent_is_i2c_adapter(adapter);
+	int ret = 0;
 
 	if (parent)
 		return i2c_trylock_adapter(parent);
-	else
-		return rt_mutex_trylock(&adapter->bus_lock);
+	else {
+		ret = rt_mutex_trylock(&adapter->bus_lock);
+		if (ret && adapter->hardware_trylock) {
+			ret = adapter->hardware_trylock();
+			if (!ret)
+				i2c_unlock_adapter(adapter);
+		}
+		return ret;
+	}
 }
 
 /**

@@ -477,8 +488,11 @@ void i2c_unlock_adapter(struct i2c_adapter *adapter)
 
 	if (parent)
 		i2c_unlock_adapter(parent);
-	else
+	else {
+		if (adapter->hardware_unlock)
+			adapter->hardware_unlock();
 		rt_mutex_unlock(&adapter->bus_lock);
+	}
 }
 EXPORT_SYMBOL_GPL(i2c_unlock_adapter);
include/linux/i2c.h +3 -0
index 06a8d9c..b283b4e 100644
@@ -361,6 +361,9 @@ struct i2c_adapter {
 
 	/* data fields that are valid for all devices	*/
 	struct rt_mutex bus_lock;
+	void (*hardware_lock)(void);
+	void (*hardware_unlock)(void);
+	int (*hardware_trylock)(void);
 
 	int timeout;			/* in jiffies */
 	int retries;
Signature · 1 line
Jean Delvare
#3 lore raw
Quoted 10 lines · Jean Delvare

Hi Haojian,

Quoted 6 lines · Haojian Zhuang

Both AP and CP are contained in Marvell PXA910 silicon. These two ARM cores are sharing one pair of I2C pins.

In order to keep I2C transaction operated with atomic, hardware lock (RIPC) is required. Because of this, bus lock in AP side can't afford this requirement. Now hardware lock is appended.

Quoted 6 lines

I have no objection to the idea, but one question: when using the hardware lock, isn't the software mutex redundant? I would expect that you call the hardware_lock/unlock functions _instead_ of rt_mutex_lock/unlock, rather than in addition to it. Or do you still need the rt_mutex to prevent priority inversion?

Jean,

It's actually not redundant. The hardware lock is used to protect access to the same register regions between two processors (AP and CP so called), while the software lock is used to protect access from within the AP side.

Quoted 79 lines
Quoted 72 lines
Signed-off-by Haojian Zhuang <[email protected]>
Cc Ben Dooks <[email protected]>

---
?drivers/i2c/i2c-core.c | ? 22 ++++++++++++++++++---- ?include/linux/i2c.h ? ?| ? ?3 +++
?2 files changed, 21 insertions(+), 4 deletions(-)

drivers/i2c/i2c-core.c +14 -3
index 045ba6e..412c7a5 100644
@@ -448,8 +448,11 @@ void i2c_lock_adapter(struct i2c_adapter *adapter)
 
 ? ? ? if (parent)
 ? ? ? ? ? ? ? i2c_lock_adapter(parent);
- ? ? else
+ ? ? else {
 ? ? ? ? ? ? ? rt_mutex_lock(&adapter->bus_lock);
+ ? ? ? ? ? ? if (adapter->hardware_lock)
+ ? ? ? ? ? ? ? ? ? ? adapter->hardware_lock();
+ ? ? }
 ?}
 ?EXPORT_SYMBOL_GPL(i2c_lock_adapter);
 

@@ -460,11 +463,19 @@ EXPORT_SYMBOL_GPL(i2c_lock_adapter);
 ?static int i2c_trylock_adapter(struct i2c_adapter *adapter)
 ?{
 ? ? ? struct i2c_adapter *parent = i2c_parent_is_i2c_adapter(adapter);
+ ? ? int ret = 0;
 
 ? ? ? if (parent)
 ? ? ? ? ? ? ? return i2c_trylock_adapter(parent);
- ? ? else
- ? ? ? ? ? ? return rt_mutex_trylock(&adapter->bus_lock);
+ ? ? else {
+ ? ? ? ? ? ? ret = rt_mutex_trylock(&adapter->bus_lock);
+ ? ? ? ? ? ? if (ret && adapter->hardware_trylock) {
+ ? ? ? ? ? ? ? ? ? ? ret = adapter->hardware_trylock();
+ ? ? ? ? ? ? ? ? ? ? if (!ret)
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? i2c_unlock_adapter(adapter);
+ ? ? ? ? ? ? }
+ ? ? ? ? ? ? return ret;
+ ? ? }
 ?}

?/**

drivers/i2c/i2c-core.c +4 -1
@@ -477,8 +488,11 @@ void i2c_unlock_adapter(struct i2c_adapter *adapter)
 
 ? ? ? if (parent)
 ? ? ? ? ? ? ? i2c_unlock_adapter(parent);
- ? ? else
+ ? ? else {
+ ? ? ? ? ? ? if (adapter->hardware_unlock)
+ ? ? ? ? ? ? ? ? ? ? adapter->hardware_unlock();
 ? ? ? ? ? ? ? rt_mutex_unlock(&adapter->bus_lock);
+ ? ? }
 ?}
 ?EXPORT_SYMBOL_GPL(i2c_unlock_adapter);
include/linux/i2c.h +0 -0
index 06a8d9c..b283b4e 100644
@@ -361,6 +361,9 @@ struct i2c_adapter {

? ? ? /* data fields that are valid for all devices ? */ ? ? ? struct rt_mutex bus_lock;
+ ? ? void (*hardware_lock)(void);
+ ? ? void (*hardware_unlock)(void);
+ ? ? int (*hardware_trylock)(void);

Quoted 2 lines

? ? ? int timeout; ? ? ? ? ? ? ? ? ? ?/* in jiffies */ ? ? ? int retries;

--
Jean Delvare

Acked-by
#4 lore raw

Hi Eric,

Quoted 18 lines · Eric Miao
Quoted 10 lines · Jean Delvare

Hi Haojian,

Quoted 6 lines · Haojian Zhuang

Both AP and CP are contained in Marvell PXA910 silicon. These two ARM cores are sharing one pair of I2C pins.

In order to keep I2C transaction operated with atomic, hardware lock (RIPC) is required. Because of this, bus lock in AP side can't afford this requirement. Now hardware lock is appended.

Quoted 6 lines

I have no objection to the idea, but one question: when using the hardware lock, isn't the software mutex redundant? I would expect that you call the hardware_lock/unlock functions _instead_ of rt_mutex_lock/unlock, rather than in addition to it. Or do you still need the rt_mutex to prevent priority inversion?

Quoted 6 lines

Jean,

It's actually not redundant. The hardware lock is used to protect access to the same register regions between two processors (AP and CP so called), while the software lock is used to protect access from within the AP side.

Are you suggesting that the hardware lock wouldn't mind being taken twice by the AP side? If it is the case, then indeed the software mutex is still needed to prevent it from happening.

That being said... I guess that avoiding a priority inversion is a good enough reason to always take the rt_mutex, regardless of the hardware lock implementation.

So, this patch is

Acked-by Jean Delvare <[email protected]>

I guess it makes more sense for me to let Ben apply it, as the other two patches in the series are for him too. This will avoid a dependency between our trees.

Signature · 1 line
Jean Delvare
#5 lore raw
Quoted 8 lines · Jean Delvare

Are you suggesting that the hardware lock wouldn't mind being taken twice by the AP side? If it is the case, then indeed the software mutex is still needed to prevent it from happening.

That being said... I guess that avoiding a priority inversion is a good enough reason to always take the rt_mutex, regardless of the hardware lock implementation.

Quoted 7 lines

So, this patch is

Acked-by Jean Delvare <[email protected]>

I guess it makes more sense for me to let Ben apply it, as the other two patches in the series are for him too. This will avoid a dependency between our trees.

Only change I'd suggest is passing adapter to the hardware_lock/unlock methods. Having no arguments what so ever in generic code for this kind of stuff looks rather strange and limiting.

author
#6 lore raw
Quoted 13 lines · Russell King - ARM Linux
Quoted 12 lines · Jean Delvare

Are you suggesting that the hardware lock wouldn't mind being taken twice by the AP side? If it is the case, then indeed the software mutex is still needed to prevent it from happening.

That being said... I guess that avoiding a priority inversion is a good enough reason to always take the rt_mutex, regardless of the hardware lock implementation.

So, this patch is

Acked-by Jean Delvare <[email protected]>
Quoted 8 lines
Quoted 3 lines

I guess it makes more sense for me to let Ben apply it, as the other two patches in the series are for him too. This will avoid a dependency between our trees.

Only change I'd suggest is passing adapter to the hardware_lock/unlock methods. ?Having no arguments what so ever in generic code for this kind of stuff looks rather strange and limiting.

OK. I'll update it.

author
#7 lore raw
In reply to Eric Miao
Quoted 18 lines · Eric Miao
Quoted 10 lines · Jean Delvare

Hi Haojian,

Quoted 6 lines · Haojian Zhuang

Both AP and CP are contained in Marvell PXA910 silicon. These two ARM cores are sharing one pair of I2C pins.

In order to keep I2C transaction operated with atomic, hardware lock (RIPC) is required. Because of this, bus lock in AP side can't afford this requirement. Now hardware lock is appended.

Quoted 6 lines

I have no objection to the idea, but one question: when using the hardware lock, isn't the software mutex redundant? I would expect that you call the hardware_lock/unlock functions _instead_ of rt_mutex_lock/unlock, rather than in addition to it. Or do you still need the rt_mutex to prevent priority inversion?

Quoted 7 lines

Jean,

It's actually not redundant. The hardware lock is used to protect access to the same register regions between two processors (AP and CP so called), while the software lock is used to protect access from within the AP side.

Jean,

It's not redundant. Reading RIPC register will try to get the lock. We're always using __raw_readl() API to read register. I think the read operation couldn't be
atomic and finished in one instruction cycle. If two processes in AP side try to
get the RIPC lock with __raw_readl(), it may result dead lock. If we fetch the RIPC lock behind software bus lock, it's safe.

If process on AP try to get the RIPC lock and compete with CP, it won't be an issue. It should always be atomic.

author
#8 lore raw

PATCH 1/3 i2c: append hardware lock with bus lock

In reply to a message not in the archive

Both AP and CP are contained in Marvell PXA910 silicon. These two ARM cores are sharing one pair of I2C pins.

In order to keep I2C transaction operated with atomic, hardware lock (RIPC) is required. Because of this, bus lock in AP side can't afford this requirement. Now hardware lock is appended.

Signed-off-by Haojian Zhuang <[email protected]>
Cc Ben Dooks <[email protected]>
Cc Jean Delvare <[email protected]>
---
 drivers/i2c/i2c-core.c |   22 ++++++++++++++++++----
 include/linux/i2c.h    |    3 +++
 2 files changed, 21 insertions(+), 4 deletions(-)
drivers/i2c/i2c-core.c +18 -4
index 045ba6e..132d46c 100644
@@ -448,8 +448,11 @@ void i2c_lock_adapter(struct i2c_adapter *adapter)
 
 	if (parent)
 		i2c_lock_adapter(parent);
-	else
+	else {
 		rt_mutex_lock(&adapter->bus_lock);
+		if (adapter->hardware_lock)
+			adapter->hardware_lock(adapter);
+	}
 }
 EXPORT_SYMBOL_GPL(i2c_lock_adapter);
 

@@ -460,11 +463,19 @@ EXPORT_SYMBOL_GPL(i2c_lock_adapter);
 static int i2c_trylock_adapter(struct i2c_adapter *adapter)
 {
 	struct i2c_adapter *parent = i2c_parent_is_i2c_adapter(adapter);
+	int ret = 0;
 
 	if (parent)
 		return i2c_trylock_adapter(parent);
-	else
-		return rt_mutex_trylock(&adapter->bus_lock);
+	else {
+		ret = rt_mutex_trylock(&adapter->bus_lock);
+		if (ret && adapter->hardware_trylock) {
+			ret = adapter->hardware_trylock(adapter);
+			if (!ret)
+				i2c_unlock_adapter(adapter);
+		}
+		return ret;
+	}
 }
 
 /**

@@ -477,8 +488,11 @@ void i2c_unlock_adapter(struct i2c_adapter *adapter)
 
 	if (parent)
 		i2c_unlock_adapter(parent);
-	else
+	else {
+		if (adapter->hardware_unlock)
+			adapter->hardware_unlock(adapter);
 		rt_mutex_unlock(&adapter->bus_lock);
+	}
 }
 EXPORT_SYMBOL_GPL(i2c_unlock_adapter);
include/linux/i2c.h +3 -0
index 06a8d9c..f4ef5b0 100644
@@ -361,6 +361,9 @@ struct i2c_adapter {
 
 	/* data fields that are valid for all devices	*/
 	struct rt_mutex bus_lock;
+	void (*hardware_lock)(struct i2c_adapter *);
+	void (*hardware_unlock)(struct i2c_adapter *);
+	int (*hardware_trylock)(struct i2c_adapter *);
 
 	int timeout;			/* in jiffies */
 	int retries;
Signature · 1 line
1.7.1
#9 lore raw
Quoted 10 lines · Haojian Zhuang

Both AP and CP are contained in Marvell PXA910 silicon. These two ARM cores are sharing one pair of I2C pins.

In order to keep I2C transaction operated with atomic, hardware lock (RIPC) is required. Because of this, bus lock in AP side can't afford this requirement. Now hardware lock is appended.

Signed-off-by Haojian Zhuang <[email protected]>
Cc Ben Dooks <[email protected]>
Cc Jean Delvare <[email protected]>

Right, this looks like a reasonable explanation of what is going on here and if Jean is happy with the core changes I'll look at where the driver change can go.

Quoted 78 lines
---
 drivers/i2c/i2c-core.c |   22 ++++++++++++++++++----
 include/linux/i2c.h    |    3 +++
 2 files changed, 21 insertions(+), 4 deletions(-)
drivers/i2c/i2c-core.c +18 -4
index 045ba6e..132d46c 100644
@@ -448,8 +448,11 @@ void i2c_lock_adapter(struct i2c_adapter *adapter)
 
 	if (parent)
 		i2c_lock_adapter(parent);
-	else
+	else {
 		rt_mutex_lock(&adapter->bus_lock);
+		if (adapter->hardware_lock)
+			adapter->hardware_lock(adapter);
+	}
 }
 EXPORT_SYMBOL_GPL(i2c_lock_adapter);
 

@@ -460,11 +463,19 @@ EXPORT_SYMBOL_GPL(i2c_lock_adapter);
 static int i2c_trylock_adapter(struct i2c_adapter *adapter)
 {
 	struct i2c_adapter *parent = i2c_parent_is_i2c_adapter(adapter);
+	int ret = 0;
 
 	if (parent)
 		return i2c_trylock_adapter(parent);
-	else
-		return rt_mutex_trylock(&adapter->bus_lock);
+	else {
+		ret = rt_mutex_trylock(&adapter->bus_lock);
+		if (ret && adapter->hardware_trylock) {
+			ret = adapter->hardware_trylock(adapter);
+			if (!ret)
+				i2c_unlock_adapter(adapter);
+		}
+		return ret;
+	}
 }
 
 /**

@@ -477,8 +488,11 @@ void i2c_unlock_adapter(struct i2c_adapter *adapter)
 
 	if (parent)
 		i2c_unlock_adapter(parent);
-	else
+	else {
+		if (adapter->hardware_unlock)
+			adapter->hardware_unlock(adapter);
 		rt_mutex_unlock(&adapter->bus_lock);
+	}
 }
 EXPORT_SYMBOL_GPL(i2c_unlock_adapter);
include/linux/i2c.h +3 -0
index 06a8d9c..f4ef5b0 100644
@@ -361,6 +361,9 @@ struct i2c_adapter {
 
 	/* data fields that are valid for all devices	*/
 	struct rt_mutex bus_lock;
+	void (*hardware_lock)(struct i2c_adapter *);
+	void (*hardware_unlock)(struct i2c_adapter *);
+	int (*hardware_trylock)(struct i2c_adapter *);
 
 	int timeout;			/* in jiffies */
 	int retries;
Signature · 6 lines
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Signature · 3 lines
Ben Dooks, ben at fluff.org, http://www.fluff.org/ben/

Large Hadron Colada: A large Pina Colada that makes the universe disappear.
#10 lore raw
Quoted 8 lines · Ben Dooks
Quoted 7 lines · Haojian Zhuang

Both AP and CP are contained in Marvell PXA910 silicon. These two ARM cores are sharing one pair of I2C pins.

In order to keep I2C transaction operated with atomic, hardware lock (RIPC) is required. Because of this, bus lock in AP side can't afford this requirement. Now hardware lock is appended.

Quoted 7 lines
Quoted 3 lines
Signed-off-by Haojian Zhuang <[email protected]>
Cc Ben Dooks <[email protected]>
Cc Jean Delvare <[email protected]>

Right, this looks like a reasonable explanation of what is going on here and if Jean is happy with the core changes I'll look at where the driver change can go.

Yes I am!

Let me know if you want me to take the core change in my tree or if you prefer to have it in yours to avoid a dependency.

Signature · 1 line
Jean Delvare
#11 lore raw
Quoted 13 lines · Jean Delvare
Quoted 12 lines · Ben Dooks
Quoted 10 lines · Haojian Zhuang

Both AP and CP are contained in Marvell PXA910 silicon. These two ARM cores are sharing one pair of I2C pins.

In order to keep I2C transaction operated with atomic, hardware lock (RIPC) is required. Because of this, bus lock in AP side can't afford this requirement. Now hardware lock is appended.

Signed-off-by Haojian Zhuang <[email protected]>
Cc Ben Dooks <[email protected]>
Cc Jean Delvare <[email protected]>
Quoted 8 lines
Quoted 3 lines

Right, this looks like a reasonable explanation of what is going on here and if Jean is happy with the core changes I'll look at where the driver change can go.

Yes I am!

Let me know if you want me to take the core change in my tree or if you prefer to have it in yours to avoid a dependency.

if it'll go in my tree ok, then yes.
Will publish a new tree soon.

author
#12 lore raw

PATCH 2/3 i2c: pxa: support hardware lock

In reply to a message not in the archive

Append hardware lock support since it's required by Marvell PXA910.

Signed-off-by Haojian Zhuang <[email protected]>
Cc Ben Dooks <[email protected]>
---
 drivers/i2c/busses/i2c-pxa.c |    3 +++
 include/linux/i2c/pxa-i2c.h  |    3 +++
 2 files changed, 6 insertions(+), 0 deletions(-)
drivers/i2c/busses/i2c-pxa.c +3 -0
index cab529d..e9a5dd8 100644
@@ -1120,6 +1120,9 @@ static int i2c_pxa_probe(struct platform_device *dev)
 
 	if (plat) {
 		i2c->adap.class = plat->class;
+		i2c->adap.hardware_lock = plat->hardware_lock;
+		i2c->adap.hardware_unlock = plat->hardware_unlock;
+		i2c->adap.hardware_trylock = plat->hardware_trylock;
 		i2c->use_pio = plat->use_pio;
 		i2c->fast_mode = plat->fast_mode;
 	}
include/linux/i2c/pxa-i2c.h +3 -0
index 1a9f65e..eec9954 100644
@@ -67,6 +67,9 @@ struct i2c_pxa_platform_data {
 	unsigned int		class;
 	unsigned int		use_pio :1;
 	unsigned int		fast_mode :1;
+	void			(*hardware_lock)(void);
+	void			(*hardware_unlock)(void);
+	int			(*hardware_trylock)(void);
 };
 
 extern void pxa_set_i2c_info(struct i2c_pxa_platform_data *info);
Signature · 1 line
1.7.1
author
#13 lore raw

PATCH 2/3 i2c: pxa: support hardware lock

In reply to a message not in the archive

Append hardware lock support since it's required by Marvell PXA910.

Signed-off-by Haojian Zhuang <[email protected]>
Cc Ben Dooks <[email protected]>
Cc Jean Delvare <[email protected]>
---
 drivers/i2c/busses/i2c-pxa.c |    3 +++
 include/linux/i2c/pxa-i2c.h  |    3 +++
 2 files changed, 6 insertions(+), 0 deletions(-)
drivers/i2c/busses/i2c-pxa.c +3 -0
index cab529d..e9a5dd8 100644
@@ -1120,6 +1120,9 @@ static int i2c_pxa_probe(struct platform_device *dev)
 
 	if (plat) {
 		i2c->adap.class = plat->class;
+		i2c->adap.hardware_lock = plat->hardware_lock;
+		i2c->adap.hardware_unlock = plat->hardware_unlock;
+		i2c->adap.hardware_trylock = plat->hardware_trylock;
 		i2c->use_pio = plat->use_pio;
 		i2c->fast_mode = plat->fast_mode;
 	}
include/linux/i2c/pxa-i2c.h +3 -0
index 1a9f65e..076324f 100644
@@ -67,6 +67,9 @@ struct i2c_pxa_platform_data {
 	unsigned int		class;
 	unsigned int		use_pio :1;
 	unsigned int		fast_mode :1;
+	void			(*hardware_lock)(struct i2c_adapter *);
+	void			(*hardware_unlock)(struct i2c_adapter *);
+	int			(*hardware_trylock)(struct i2c_adapter *);
 };
 
 extern void pxa_set_i2c_info(struct i2c_pxa_platform_data *info);
Signature · 1 line
1.7.1
author
#14 lore raw

PATCH 3/3 ARM: mmp: add hardware lock support in PXA910

In reply to a message not in the archive

RIPC register is used to implement hardware lock between CP and AP in Marvell PXA910.

Signed-off-by Haojian Zhuang <[email protected]>
Cc Ben Dooks <[email protected]>
Cc Eric Miao <[email protected]>
Cc Russell King <[email protected]>
---
 arch/arm/mach-mmp/include/mach/pxa910.h |    3 ++
 arch/arm/mach-mmp/pxa910.c              |   21 +++++++++++++-
 arch/arm/mach-mmp/ttc_dkb.c             |   45 +++++++++++++++++++++++++++++++
 3 files changed, 68 insertions(+), 1 deletions(-)
arch/arm/mach-mmp/include/mach/pxa910.h +3 -0
index 91be755..e7659a1 100644
@@ -5,6 +5,9 @@ struct sys_timer;
 
 extern struct sys_timer pxa910_timer;
 extern void __init pxa910_init_irq(void);
+extern void pxa910_ripc_lock(void);
+extern void pxa910_ripc_unlock(void);
+extern int pxa910_ripc_trylock(void);
 
 #include <linux/i2c.h>
 #include <linux/i2c/pxa-i2c.h>
arch/arm/mach-mmp/pxa910.c +20 -1
index 8f92ccd..3190257 100644
@@ -28,7 +28,10 @@
 #include "common.h"
 #include "clock.h"
 
-#define MFPR_VIRT_BASE	(APB_VIRT_BASE + 0x1e000)
+#define MFPR_VIRT_BASE		(APB_VIRT_BASE + 0x1e000)
+
+#define RIPC0_VIRT_BASE		(APB_VIRT_BASE + 0x3D000)
+#define RIPC0_STATUS		(RIPC0_VIRT_BASE + 0x00)
 
 static struct mfp_addr_map pxa910_mfp_addr_map[] __initdata =
 {

@@ -100,6 +103,22 @@ void __init pxa910_init_irq(void)
 	pxa910_init_gpio();
 }
 
+void pxa910_ripc_lock(void)
+{
+	while (__raw_readl(RIPC0_STATUS))
+		cpu_relax();
+}
+
+int pxa910_ripc_trylock(void)
+{
+	return (!__raw_readl(RIPC0_STATUS));
+}
+
+void pxa910_ripc_unlock(void)
+{
+	__raw_writel(1, RIPC0_STATUS);
+}
+
 /* APB peripheral clocks */
 static APBC_CLK(uart1, PXA910_UART0, 1, 14745600);
 static APBC_CLK(uart2, PXA910_UART1, 1, 14745600);
arch/arm/mach-mmp/ttc_dkb.c +45 -0

69 lines not on this page · raw message

Signature · 1 line
1.7.1
author
#15 lore raw

PATCH 3/3 ARM: mmp: add hardware lock support in PXA910

In reply to a message not in the archive

RIPC register is used to implement hardware lock between CP and AP in Marvell PXA910.

Signed-off-by Haojian Zhuang <[email protected]>
Cc Ben Dooks <[email protected]>
Cc Jean Delvare <[email protected]>
Cc Eric Miao <[email protected]>
Cc Russell King <[email protected]>
---
 arch/arm/mach-mmp/include/mach/pxa910.h |   13 +++++---
 arch/arm/mach-mmp/pxa910.c              |   23 +++++++++++++++-
 arch/arm/mach-mmp/ttc_dkb.c             |   45 +++++++++++++++++++++++++++++++
 3 files changed, 75 insertions(+), 6 deletions(-)
arch/arm/mach-mmp/include/mach/pxa910.h +8 -5
index 91be755..2973466 100644
@@ -1,16 +1,19 @@
 #ifndef __ASM_MACH_PXA910_H
 #define __ASM_MACH_PXA910_H
 
-struct sys_timer;
-
-extern struct sys_timer pxa910_timer;
-extern void __init pxa910_init_irq(void);
-
 #include <linux/i2c.h>
 #include <linux/i2c/pxa-i2c.h>
 #include <mach/devices.h>
 #include <plat/pxa3xx_nand.h>
 
+struct sys_timer;
+
+extern struct sys_timer pxa910_timer;
+extern void __init pxa910_init_irq(void);
+extern void pxa910_ripc_lock(struct i2c_adapter *);
+extern void pxa910_ripc_unlock(struct i2c_adapter *);
+extern int pxa910_ripc_trylock(struct i2c_adapter *);
+
 extern struct pxa_device_desc pxa910_device_uart1;
 extern struct pxa_device_desc pxa910_device_uart2;
 extern struct pxa_device_desc pxa910_device_twsi0;
arch/arm/mach-mmp/pxa910.c +22 -1
index 8f92ccd..4fbe485 100644
@@ -13,6 +13,7 @@
 #include <linux/init.h>
 #include <linux/list.h>
 #include <linux/io.h>
+#include <linux/i2c.h>
 
 #include <asm/mach/time.h>
 #include <mach/addr-map.h>

@@ -28,7 +29,10 @@
 #include "common.h"
 #include "clock.h"
 
-#define MFPR_VIRT_BASE	(APB_VIRT_BASE + 0x1e000)
+#define MFPR_VIRT_BASE		(APB_VIRT_BASE + 0x1e000)
+
+#define RIPC0_VIRT_BASE		(APB_VIRT_BASE + 0x3D000)
+#define RIPC0_STATUS		(RIPC0_VIRT_BASE + 0x00)
 
 static struct mfp_addr_map pxa910_mfp_addr_map[] __initdata =
 {

@@ -100,6 +104,23 @@ void __init pxa910_init_irq(void)
 	pxa910_init_gpio();
 }
 
+void pxa910_ripc_lock(struct i2c_adapter *adap)
+{
+	while (__raw_readl(RIPC0_STATUS))
+		cpu_relax();
+}
+
+/* return 1 -- succeed to get lock; 0 -- fail to get lock */
+int pxa910_ripc_trylock(struct i2c_adapter *adap)
+{
+	return (!__raw_readl(RIPC0_STATUS));
+}
+
+void pxa910_ripc_unlock(struct i2c_adapter *adap)
+{
+	__raw_writel(1, RIPC0_STATUS);
+}
+
 /* APB peripheral clocks */
 static APBC_CLK(uart1, PXA910_UART0, 1, 14745600);
 static APBC_CLK(uart2, PXA910_UART1, 1, 14745600);
arch/arm/mach-mmp/ttc_dkb.c +45 -0

69 lines not on this page · raw message

Signature · 1 line
1.7.1