Re: [PATCH] i2c: at91: add dma support

2012-10-17 Thread Nicolas Ferre
On 10/10/2012 03:43 PM, ludovic.desroc...@atmel.com :
 From: Ludovic Desroches ludovic.desroc...@atmel.com
 
 Add dma support for Atmel TWI which is available on sam9x5 and later.
 
 When using dma for reception, you have to read only n-2 bytes. The last
 two bytes are read manually. Don't doing this should cause to send the
 STOP command too late and then to get extra data in the receive
 register.
 
 Signed-off-by: Ludovic Desroches ludovic.desroc...@atmel.com

Acked-by: Nicolas Ferre nicolas.fe...@atmel.com

Nice work Ludo ;-) !

Bye,

 ---
  drivers/i2c/busses/i2c-at91.c | 326 
 --
  1 file changed, 314 insertions(+), 12 deletions(-)
 
 diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
 index aa59a25..33219f8 100644
 --- a/drivers/i2c/busses/i2c-at91.c
 +++ b/drivers/i2c/busses/i2c-at91.c
 @@ -19,6 +19,8 @@
  
  #include linux/clk.h
  #include linux/completion.h
 +#include linux/dma-mapping.h
 +#include linux/dmaengine.h
  #include linux/err.h
  #include linux/i2c.h
  #include linux/interrupt.h
 @@ -30,6 +32,8 @@
  #include linux/platform_device.h
  #include linux/slab.h
  
 +#include mach/at_hdmac.h
 +
  #define TWI_CLK_HZ   10  /* max 400 Kbits/s */
  #define AT91_I2C_TIMEOUT msecs_to_jiffies(100)   /* transfer timeout */
  
 @@ -65,9 +69,21 @@
  #define  AT91_TWI_THR0x0034  /* Transmit Holding Register */
  
  struct at91_twi_pdata {
 - unsignedclk_max_div;
 - unsignedclk_offset;
 - boolhas_unre_flag;
 + unsignedclk_max_div;
 + unsignedclk_offset;
 + boolhas_unre_flag;
 + boolhas_dma_support;
 + struct at_dma_slave dma_slave;
 +};
 +
 +struct at91_twi_dma {
 + struct dma_chan *chan_rx;
 + struct dma_chan *chan_tx;
 + struct scatterlist  sg;
 + struct dma_async_tx_descriptor  *data_desc;
 + enum dma_data_direction direction;
 + boolbuf_mapped;
 + boolxfer_in_progress;
  };
  
  struct at91_twi_dev {
 @@ -79,10 +95,13 @@ struct at91_twi_dev {
   size_t  buf_len;
   struct i2c_msg  *msg;
   int irq;
 + unsignedimr;
   unsignedtransfer_status;
   struct i2c_adapter  adapter;
   unsignedtwi_cwgr_reg;
   struct at91_twi_pdata   *pdata;
 + booluse_dma;
 + struct at91_twi_dma dma;
  };
  
  static unsigned at91_twi_read(struct at91_twi_dev *dev, unsigned reg)
 @@ -98,7 +117,18 @@ static void at91_twi_write(struct at91_twi_dev *dev, 
 unsigned reg, unsigned val)
  static void at91_disable_twi_interrupts(struct at91_twi_dev *dev)
  {
   at91_twi_write(dev, AT91_TWI_IDR,
 -AT91_TWI_TXCOMP | AT91_TWI_RXRDY | AT91_TWI_TXRDY);
 + AT91_TWI_TXCOMP | AT91_TWI_RXRDY | AT91_TWI_TXRDY);
 +}
 +
 +static void at91_twi_irq_save(struct at91_twi_dev *dev)
 +{
 + dev-imr = at91_twi_read(dev, AT91_TWI_IMR)  0x7;
 + at91_disable_twi_interrupts(dev);
 +}
 +
 +static void at91_twi_irq_restore(struct at91_twi_dev *dev)
 +{
 + at91_twi_write(dev, AT91_TWI_IER, dev-imr);
  }
  
  static void at91_init_twi_bus(struct at91_twi_dev *dev)
 @@ -137,6 +167,30 @@ static void __devinit at91_calc_twi_clock(struct 
 at91_twi_dev *dev, int twi_clk)
   dev_dbg(dev-dev, cdiv %d ckdiv %d\n, cdiv, ckdiv);
  }
  
 +static void at91_twi_dma_cleanup(struct at91_twi_dev *dev)
 +{
 + struct at91_twi_dma *dma = dev-dma;
 +
 + at91_twi_irq_save(dev);
 +
 + if (dma-xfer_in_progress) {
 + if (dma-direction == DMA_FROM_DEVICE)
 + dma-chan_rx-device-device_control(dma-chan_rx,
 + DMA_TERMINATE_ALL, 0);
 + else
 + dma-chan_tx-device-device_control(dma-chan_tx,
 + DMA_TERMINATE_ALL, 0);
 + dma-xfer_in_progress = false;
 + }
 + if (dma-buf_mapped) {
 + dma_unmap_single(dev-dev, sg_dma_address(dma-sg),
 +  dev-buf_len, dma-direction);
 + dma-buf_mapped = false;
 + }
 +
 + at91_twi_irq_restore(dev);
 +}
 +
  static void at91_twi_write_next_byte(struct at91_twi_dev *dev)
  {
   if (dev-buf_len = 0)
 @@ -153,6 +207,65 @@ static void at91_twi_write_next_byte(struct at91_twi_dev 
 *dev)
   ++dev-buf;
  }
  
 +static void at91_twi_write_data_dma_callback(void *data)
 +{
 + struct at91_twi_dev *dev = (struct at91_twi_dev *)data;
 +
 + dma_unmap_single(dev-dev, sg_dma_address(dev-dma.sg),
 +  dev-buf_len, DMA_TO_DEVICE);
 +
 + at91_twi_write(dev, AT91_TWI_CR, AT91_TWI_STOP);
 +}
 +
 +static void at91_twi_write_data_dma(struct 

Re: [PATCH] i2c: at91: add dma support

2012-10-17 Thread Russell King - ARM Linux
On Wed, Oct 10, 2012 at 03:43:07PM +0200, ludovic.desroc...@atmel.com wrote:
 + txdesc = chan_tx-device-device_prep_slave_sg(chan_tx, dma-sg,
 + 1, DMA_TO_DEVICE, DMA_PREP_INTERRUPT | DMA_CTRL_ACK, NULL);

No, a while back the DMA engine API changed.  It no longer takes
DMA_TO_DEVICE/DMA_FROM_DEVICE but DMA_MEM_TO_DEV and DMA_DEV_TO_MEM.

 + /* Keep in mind that we won't use dma to read the last two bytes */
 + at91_twi_irq_save(dev);
 + dma_addr = dma_map_single(dev-dev, dev-buf, dev-buf_len - 2,
 +   DMA_FROM_DEVICE);

Ditto.

 + dma-xfer_in_progress = true;
 + cookie = rxdesc-tx_submit(rxdesc);
 + if (dma_submit_error(cookie)) {

tx_submit never errors (anymore.)

 + slave_config.direction = DMA_TO_DEVICE;

Same comment as for the other directions.  Note that DMA engine drivers
really should ignore this parameter now, and DMA engine users should phase
it out.

 + if (dmaengine_slave_config(dma-chan_tx, slave_config)) {
 + dev_err(dev-dev, failed to configure tx channel\n);
 + ret = -EINVAL;
 + goto error;
 + }
 +
 + slave_config.direction = DMA_FROM_DEVICE;

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


Re: i2cset question

2012-10-17 Thread Jean Delvare
Hi Javi,

On Wed, 17 Oct 2012 12:07:40 + (UTC), Javi wrote:
 I have the same problem than Chris.

Actually I suspect this is a different problem.

 I am working with an ALIX motherboard with 
 voyage linux distribution and I have connected a 24LC64 eeprom. When I try to 
 write the epprom with i2cset command it doesn't work.

Because the 24LC64 uses a two-byte addressing model, while SMBus uses a
one-byte addressing model. This means you can't use this type of EEPROM
with an SMBus controller [1]. You need a full-featured I2C controller.
You may be able to achieve that on your system by using the scx200_i2c
driver instead of scx200_acb driver, or by using an extra pair of GPIO
pins with i2c-gpio.

If this isn't an option for you then you'll have to use a different
EEPROM model. AFAIK the 24C16 is the largest one using single-byte
addressing (it uses all 8 I2C addresses 0x50-0x57, and 256 * 8 * 8 =
16K.)

Note that you can't use i2cdump, i2cget and i2cset with 2-byte
addressed EEPROMs. You'll have to try the dedicated tools under
eepromer in the i2c-tools package [2]. For 2-byte addressed EEPROMs you
want either eeprog or eepromer. I seem to recall eeprog implements the
tricks described below for 2-byte addressed EEPROMs over SMBus.

[1] Actually you can write to such an EEPROM by abusing SMBus write
transactions. All you have to do is pass the second address byte as
the first data byte of either an SMBus write word transaction or,
if your SMBus controller supports it, an I2C block write
transaction. Then shift all data bytes by one position. For reads,
you could use an SMBus write byte transaction to set the address
and then SMBus receive byte transactions in a loop to retrieve the
data bytes one by one. However this is racy and slow.

[2] Note that the primary hosting site for i2c-tools is currently down,
you can use my mirror if needed:
http://khali.linux-fr.org/mirror/i2c-tools/

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


Re: RT throttling and suspend/resume (was Re: [PATCH] i2c: omap: revert i2c: omap: switch to threaded IRQ support)

2012-10-17 Thread Felipe Balbi
Hi,

On Tue, Oct 16, 2012 at 02:39:50PM -0700, Kevin Hilman wrote:
 + peterz, tglx
 
 Felipe Balbi ba...@ti.com writes:
 
 [...]
 
  The problem I see is that even though we properly return IRQ_WAKE_THREAD
  and wake_up_process() manages to wakeup the IRQ thread (it returns 1),
  the thread is never scheduled. To make things even worse, ouw irq thread
  runs once, but doesn't run on a consecutive call. Here's some (rather
  nasty) debug prints showing the problem:
 
 [...]
 
  [   88.721923] try_to_wake_up 1411
  [   88.725189] === irq_wake_thread 139: IRQ 72 wake_up_process 0
  [   88.731292] [sched_delayed] sched: RT throttling activated
 
 This throttling message is the key one.
 
 With RT throttling activated, the IRQ thread will not be run (it
 eventually will be allowed much later on, but by then, the I2C xfers
 have timed out.)
 
 As a quick hack, the throttling can be disabled by seeting the
 sched_rt_runtime to RUNTIME_INF:
 
 # sysctl -w kernel.sched_rt_runtime_us=-1
 
 and a quick test shows that things go back to working as expected.  But
 we still need to figure out why the throttling is hapenning...
 
 So I started digging into why the RT runtime was so high, and noticed
 that time spent in suspend was being counted as RT runtime!
 
 So spending time in suspend anywhere near sched_rt_runtime (0.95s) will
 cause the RT throttling to always be triggered, and thus prevent IRQ
 threads from running in the resume path.  Ouch.
 
 I think I'm already in over my head in the RT runtime stuff, but
 counting the time spent in suspend as RT runtime smells like a bug to
 me. no?
 
 Peter? Thomas?

it looks like removing console output completely (echo 0 
/proc/sysrq-trigger) I don't see the issue anymore. Let me just run for
a few more iterations to make sure what I'm saying is correct.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH v3] ARM: OMAP: i2c: fix interrupt flood during resume

2012-10-17 Thread Felipe Balbi
Hi,

On Thu, Oct 11, 2012 at 02:08:25PM -0700, Kevin Hilman wrote:
 Hi Kalle,
 
 Kalle Jokiniemi kalle.jokini...@jollamobile.com writes:
 
  The resume_noirq enables interrupts one-by-one starting from
  first one. Now if the wake up event for suspend came from i2c
  device, the i2c bus irq gets enabled before the threaded
  i2c device irq, causing a flood of i2c bus interrupts as the
  threaded irq that should clear the event is not enabled yet.
 
  Fixed the issue by adding suspend_noirq and resume_early
  functions that keep i2c bus interrupts disabled until
  resume_noirq has run completely.
 
  Issue was detected doing a wake up from autosleep with
  twl4030 power key on N9. Patch tested on N9.
 
  Signed-off-by: Kalle Jokiniemi kalle.jokini...@jollamobile.com
 
 This version looks good, thanks for the extra comments.
 
 Reviewed-by: Kevin Hilman khil...@ti.com
 Tested-by: Kevin Hilman khil...@ti.com
 
 Wolfram, This should also probably be Cc'd to stable since it affects
 earlier kernels as well.  Thanks,

just to make sure we're not fixing the wrong problem... does [1] help in
any way ?

[1] http://marc.info/?l=linux-omapm=135048839915719w=2

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH] i2c: omap: adopt pinctrl support

2012-10-17 Thread Shubhrajyoti Datta
On Tue, Oct 16, 2012 at 8:53 PM, Sebastien Guiriec s-guir...@ti.com wrote:
 Some GPIO expanders need some early pin control muxing. Due to
 legacy boards sometimes the driver uses subsys_initcall instead of
 module_init. This patch takes advantage of defer probe feature
 and pin control in order to wait until pin control probing before
 GPIO driver probing. It has been tested on OMAP5 board with TCA6424
 driver.

 log:

 [0.482299] omap_i2c i2c.15: could not find pctldev for node /ocp/pinmux@4a00
 2840/pinmux_i2c5_pins, deferring probe
 [0.482330] platform i2c.15: Driver omap_i2c requests probe deferral
 [0.484466] Advanced Linux Sound Architecture Driver Initialized.

 [4.746917] omap_i2c i2c.15: bus 4 rev2.4.0 at 100 kHz
 [4.755279] gpiochip_find_base: found new base at 477
 [4.761169] gpiochip_add: registered GPIOs 477 to 500 on device: tca6424a

Thanks,

Acked-by: Shubhrajyoti D shubhrajy...@ti.com
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: RT throttling and suspend/resume (was Re: [PATCH] i2c: omap: revert i2c: omap: switch to threaded IRQ support)

2012-10-17 Thread Kevin Hilman
Felipe Balbi ba...@ti.com writes:

 On Wed, Oct 17, 2012 at 05:00:02PM +0300, Felipe Balbi wrote:
 
 On Tue, Oct 16, 2012 at 02:39:50PM -0700, Kevin Hilman wrote:
  + peterz, tglx
  
  Felipe Balbi ba...@ti.com writes:
  
  [...]
  
   The problem I see is that even though we properly return IRQ_WAKE_THREAD
   and wake_up_process() manages to wakeup the IRQ thread (it returns 1),
   the thread is never scheduled. To make things even worse, ouw irq thread
   runs once, but doesn't run on a consecutive call. Here's some (rather
   nasty) debug prints showing the problem:
  
  [...]
  
   [   88.721923] try_to_wake_up 1411
   [   88.725189] === irq_wake_thread 139: IRQ 72 wake_up_process 0
   [   88.731292] [sched_delayed] sched: RT throttling activated
  
  This throttling message is the key one.
  
  With RT throttling activated, the IRQ thread will not be run (it
  eventually will be allowed much later on, but by then, the I2C xfers
  have timed out.)
  
  As a quick hack, the throttling can be disabled by seeting the
  sched_rt_runtime to RUNTIME_INF:
  
  # sysctl -w kernel.sched_rt_runtime_us=-1
  
  and a quick test shows that things go back to working as expected.  But
  we still need to figure out why the throttling is hapenning...
  
  So I started digging into why the RT runtime was so high, and noticed
  that time spent in suspend was being counted as RT runtime!
  
  So spending time in suspend anywhere near sched_rt_runtime (0.95s) will
  cause the RT throttling to always be triggered, and thus prevent IRQ
  threads from running in the resume path.  Ouch.
  
  I think I'm already in over my head in the RT runtime stuff, but
  counting the time spent in suspend as RT runtime smells like a bug to
  me. no?
  
  Peter? Thomas?
 
 it looks like removing console output completely (echo 0 
 /proc/sysrq-trigger) I don't see the issue anymore. Let me just run for
 a few more iterations to make sure what I'm saying is correct.

 Yeah, really looks like removing console output makes the problem go
 away. Ran a few iterations and it always worked fine. Full logs attached

Removing console output during resume is going to significantly change
the timing of what is happening during suspend/resume, so I suspect that
combined with all your other debug prints is somehow masking the
problem.  How log are you letting the system stay in suspend?

That being said, I can still easily reproduce the problem, even with
console output disabled.

With vanilla v3.7-rc1 + the debug patch below[1], with and without
console output, I see RT throttling kicking in on resume, and the RT
runtime on resume corresponds to the time spent in suspend.  Here's an
example of debug output of my patch below after ~3 sec in suspend:

[   43.198028] sched_rt_runtime_exceeded: rt_time 2671752930  runtime 95000
[   43.198028] update_curr_rt: RT runtime exceeded: irq/72-omap_i2c
[   43.198059] update_curr_rt: RT runtime exceeded: irq/72-omap_i2c
[   43.203704] [sched_delayed] sched: RT throttling activated

I see this rather consistently, and the rt_time value is always roughly the
time I spent in suspend.

So the primary question remains: is RT runtime supposed to include the
time spent suspended?  I suspect not. 

Kevin

[1]
diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 418feb0..39de750 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -891,6 +891,8 @@ static int sched_rt_runtime_exceeded(struct rt_rq *rt_rq)
if (!once) {
once = true;
printk_sched(sched: RT throttling 
activated\n);
+   pr_warn(%s: rt_time %llu  runtime %llu\n,
+   __func__, rt_rq-rt_time, runtime);
}
} else {
/*
@@ -948,8 +950,11 @@ static void update_curr_rt(struct rq *rq)
if (sched_rt_runtime(rt_rq) != RUNTIME_INF) {
raw_spin_lock(rt_rq-rt_runtime_lock);
rt_rq-rt_time += delta_exec;
-   if (sched_rt_runtime_exceeded(rt_rq))
+   if (sched_rt_runtime_exceeded(rt_rq)) {
+   pr_warn(%s: RT runtime exceeded: %s\n,
+   __func__, curr-comm);
resched_task(curr);
+   }
raw_spin_unlock(rt_rq-rt_runtime_lock);
}
}
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html