Re: [PATCH V2] i2c-designware: Add Intel Baytrail PMIC I2C bus support

[Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2246 bytes --]

Hi David,

On Tue, Sep 23, 2014 at 12:58:54PM -0700, David E. Box wrote:
> Hi Maxime,
> 
> On Tue, Sep 23, 2014 at 09:00:57PM +0200, Maxime Ripard wrote:
> > Hi David,
> > 
> > On Tue, Sep 23, 2014 at 11:40:26AM -0700, David E. Box wrote:
> > > This patch implements an I2C bus sharing mechanism between the host and platform
> > > hardware on select Intel BayTrail SoC platforms using the X-Powers AXP288 PMIC.
> > > 
> > > On these platforms access to the PMIC must be shared with platform hardware. The
> > > hardware unit assumes full control of the I2C bus and the host must request
> > > access through a special semaphore. Hardware control of the bus also makes it
> > > necessary to disable runtime pm to avoid interfering with hardware transactions.
> > > 
> > > Signed-off-by: David E. Box <david.e.box-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> > 
> > Sorry for stepping in like this without really knowing your platform,
> > but wouldn't using the hwspinlock framework make more sense than
> > hardcoding your own internal functions here?
> 
> I looked into this but didn't see a clear way on our platform to identify the
> semaphore seperately from doing it in the designware platform driver. The way
> we can find it now is through evaluating an ACPI _SEM object on every i2c device
> that gets probed by the dw driver since at probe time we can get the acpi handle.

And you have no way to turn it around and identify which semaphore is
associated to which i2c bus?

If so, there is probably some way to associate a given instance of the
i2c driver to one semaphore.

> Without this handle however there isn't a clear way of evaluating the _SEM
> object which would be needed to register a hwspinlock in separate code.
> 
> Plus it would still require changes to the designware i2c core, though admittedly
> having a generic hwspinlock pointer added to the struct is cleaner.

Not only cleaner, but that could also be used by other platforms that
are using this I2C driver (and since it's a designware IP, there must
be quite a lot) together with hardware locking.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]