Re: [RFC 2/3] regmap: Use the enhancement of i2c API to address circular dependency problem

From: Mark Brown
Date: Fri Jan 16 2015 - 13:36:59 EST


On Fri, Jan 16, 2015 at 06:36:14PM +0100, Paul Osmialowski wrote:
> On Fri, 16 Jan 2015, Mark Brown wrote:

> >I don't know what this means, sorry. I'm also very worried about the
> >fact that this is being discussed purely in terms of I2C - why would
> >this not affect other buses?

> I tried to open some gate for further extension to any bus that is used for
> regmap communications. Currently it goes down to regmap-i2c.c since I
> enhanced i2c API for this. Anyone who feels it is useful or saves oneself
> from locking troubles can voluntarily adapt other regmap-i2c.* places (as
> needed?).

> My whole point is that I proposed a way to solve nasty deadlock which is
> better to fix than just leave as it is. I got a feeling that situation I
> adressed here may occur others too, so I proposed this extension that allows
> future adaptations. I don't expect it to be accepted easily (i.e. I'm new
> here and have mixed feelins about proposing changes that go so far),
> therefore I prepared other solution for this particular deadlock that occurs
> on this particular device.

What I'm saying is that I want to understand this change from a point of
view that isn't tied to I2C - at the regmap level what is this doing,
I2C is a bus that has some properties which you're saying needs some
changes, what are those properties and those changes?

> >>+ void (*reg_unprepare_sync_io)(void *context);

> >The first question here is why this only affects synchronous I/O or
> >alternatively why these operations have _sync in the name if they aren't
> >for synchronous I/O.

> IMHO this whole idea is against asynchronous I/O.

Can you be more specific please? If something needs preparing it seems
like it'd need preparing over an async transaction just as much as over
a synchronous one.

> >>+ if (bus) {
> >>+ map->reg_prepare_sync_io = regmap_bus_prepare_sync_io;
> >>+ map->reg_unprepare_sync_io = regmap_bus_unprepare_sync_io;
> >>+ }

> >Why are we using these indirections instead of assigning the operation
> >directly? They...

> I followed the pattern used throughout this file.

Not in this pattern where the caller needs to check too.

Attachment: signature.asc
Description: Digital signature