Re: [PATCH 1/3] i2c: append hardware lock with bus lock

[Jean Delvare]

Hi Eric,

On Thu, 28 Apr 2011 16:36:02 +0800, Eric Miao wrote:
> On Thu, Apr 28, 2011 at 4:22 PM, Jean Delvare <khali@xxxxxxxxxxxx> wrote:
> > Hi Haojian,
> >
> > On Thu, 28 Apr 2011 12:02:36 +0800, Haojian Zhuang wrote:
> >> 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?
> >
> 
> 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 <khali@xxxxxxxxxxxx>

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.

-- 
Jean Delvare
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/