Re: [RFC/PATCH 3/3] mfd: twl4030-irq: implement bus_*lock

2010-12-30 Thread Mark Brown 
On Wed, Dec 29, 2010 at 02:28:08PM +0200, Felipe Balbi wrote:

 I believe you meant something like below, I'll get back to this in a
 little while. Have lots to test:

Yes, that looks like what I'd expect.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH 3/3] mfd: twl4030-irq: implement bus_*lock

2010-12-30 Thread Felipe Balbi 

On Thu, Dec 30, 2010 at 12:18:04PM +, Mark Brown wrote:

On Wed, Dec 29, 2010 at 02:28:08PM +0200, Felipe Balbi wrote:


I believe you meant something like below, I'll get back to this in a
little while. Have lots to test:


Yes, that looks like what I'd expect.


Good, I'll clean the patches up and wait past the merge window before
sending final versions.

--
balbi
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH 3/3] mfd: twl4030-irq: implement bus_*lock

2010-12-28 Thread Mark Brown 
On Tue, Dec 28, 2010 at 07:14:19PM +0200, Felipe Balbi wrote:

 +static void twl4030_sih_bus_sync_unlock(unsigned int irq)
 +{
 +   struct sih_agent*agent = get_irq_chip_data(irq);
 +
 +   mutex_unlock(agent-irq_lock);
 +}

I suspect you need to do some sort of sync with the hardware here - the
_sync bit of the name comes from the fact that the mask and unmask stuff
is still called with IRQs disabled and so can't touch and I2C chip, this
is called after reenabling them give a chance for the updates done to
be reflected in the hardware.  The implementation everyone else has done
is to update a register cache in the other functions then write that
out here before dropping the mutex.

  static struct irq_chip twl4030_sih_irq_chip = {
   .name   = twl4030,
   .mask   = twl4030_sih_mask,
   .unmask = twl4030_sih_unmask,
   .set_type   = twl4030_sih_set_type,
 + .bus_lock   = twl4030_sih_bus_lock,
 + .bus_sync_unlock = twl4030_sih_bus_sync_unlock,
  };

I just realised that this collides with the conversion to the irq_
versions that has been done on the driver in -next by either myself or
Lennart (we both submitted essentially the same patches and a couple of
his went in) - that was a purely mechanical conversion that didn't
address any of the issues this patch addresses but they're touching the
same code.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH 3/3] mfd: twl4030-irq: implement bus_*lock

2010-12-28 Thread Felipe Balbi 
Hi,

On Tue, 2010-12-28 at 23:58 +, Mark Brown wrote:
 On Tue, Dec 28, 2010 at 07:14:19PM +0200, Felipe Balbi wrote:
 
  +static void twl4030_sih_bus_sync_unlock(unsigned int irq)
  +{
  +   struct sih_agent*agent = get_irq_chip_data(irq);
  +
  +   mutex_unlock(agent-irq_lock);
  +}
 
 I suspect you need to do some sort of sync with the hardware here - the
 _sync bit of the name comes from the fact that the mask and unmask stuff
 is still called with IRQs disabled and so can't touch and I2C chip, this
 is called after reenabling them give a chance for the updates done to
 be reflected in the hardware.  The implementation everyone else has done
 is to update a register cache in the other functions then write that
 out here before dropping the mutex.

now that I look at some gpio chips I see what you're saying, will update
that tomorrow. Thanks

   static struct irq_chip twl4030_sih_irq_chip = {
  .name   = twl4030,
  .mask   = twl4030_sih_mask,
  .unmask = twl4030_sih_unmask,
  .set_type   = twl4030_sih_set_type,
  +   .bus_lock   = twl4030_sih_bus_lock,
  +   .bus_sync_unlock = twl4030_sih_bus_sync_unlock,
   };
 
 I just realised that this collides with the conversion to the irq_
 versions that has been done on the driver in -next by either myself or
 Lennart (we both submitted essentially the same patches and a couple of
 his went in) - that was a purely mechanical conversion that didn't
 address any of the issues this patch addresses but they're touching the
 same code.

no problem. This will actually only be able on 2.6.39 merge window
anyway, so I'll have plenty of time to rebase on 2.6.38 and get these
patches queued.

ps: sorry the mail change, out of the office.

-- 
balbi

--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html