Re: [PATCH 1/1] i2c-omap: add mpu wake up latency constraint in i2c

Tony Lindgren <tony@atomide.com> writes:

> * Kevin Hilman <khilman@deeprootsystems.com> [100426 15:57]:
>> Tony Lindgren <tony@atomide.com> writes:
>>
>> +#ifdef CONFIG_ARCH_OMAP3
>> +/*
>> + * omap_i2c_set_wfc_mpu_wkup_lat - sets mpu wake up constraint
>> + * @dev:	i2c bus device pointer
>> + * @val:	latency constraint to set, -1 to disable constraint
>> + *
>> + * When waiting for completion of a i2c transfer, we need to set a wake up
>> + * latency constraint for the MPU. This is to ensure quick enough wakeup from
>> + * idle, when transfer completes.
>> + */
>> +static void omap_i2c_set_wfc_mpu_wkup_lat(struct device *dev, int val)
>> +{
>> +	omap_pm_set_max_mpu_wakeup_lat(dev, val);
>> +}
>> +#endif
>> +
>
> Can't you leave out the above too?
>
>> +	struct omap_i2c_bus_platform_data *pd;
>>  	struct resource *res;
>>  	resource_size_t base, irq;
>>
>>  	pdev = &omap_i2c_devices[bus_id - 1];
>> +	pd = pdev->dev.platform_data;
>>  	if (bus_id == 1) {
>>  		res = pdev->resource;
>>  		if (cpu_class_is_omap1()) {
>> @@ -122,6 +143,8 @@ static int __init omap_i2c_add_bus(int bus_id)
>>  		omap1_i2c_mux_pins(bus_id);
>>  	if (cpu_class_is_omap2())
>>  		omap2_i2c_mux_pins(bus_id);
>> +	if (cpu_is_omap34xx())
>> +		pd->set_mpu_wkup_lat = omap_i2c_set_wfc_mpu_wkup_lat;
>
> And then just set
> 		pd->set_mpu_wkup_lat = omap_pm_set_max_mpu_wakeup_lat;
>
> Here instead?

Sure.

> BTW, in your version looks like compile will break if CONFIG_ARCH_OMAP3
> is not set.

it shouldn't since if !CONFIG_ARCH_OMAP3, cpu_is_omap34xx() is zero
and that code will not be compiled.

Anyways, here's updated version which gets rid of both problems.

Kevin

From 4dadd7a580c347d6d93ca3cdb7fbf6bd1bab9440 Mon Sep 17 00:00:00 2001
From: Kalle Jokiniemi <kalle.jokiniemi@digia.com>
Date: Wed, 21 Oct 2009 14:51:21 +0300
Subject: [PATCH] i2c-omap: add mpu wake up latency constraint in i2c

While waiting for completion of the i2c transfer, the
MPU could hit OFF mode and cause several msecs of
delay that made i2c transfers fail more often. The
extra delays and subsequent re-trys cause i2c clocks
to be active more often. This has also an negative
effect on power consumption.

Created a mechanism for passing and using the
constraint setting function in driver code. The used
mpu wake up latency constraints are now set individually
per bus, and they are calculated based on clock rate
and fifo size.

Thanks to Jarkko Nikula, Moiz Sonasath, Paul Walmsley,
and Nishanth Menon for tuning out the details of
this patch.

Updates by Kevin as requested by Tony:

- Remove omap_set_i2c_constraint_func() in favor of conditionally
  adding the flag in omap_i2c_add_bus() in order to keep all the OMAP
  conditional checking in a single location.
- Update set_mpu_wkup_lat prototypes to match OMAP PM layer so
  OMAP PM function can be used directly in pdata.

Cc: Moiz Sonasath <m-sonasath@ti.com>
Cc: Jarkko Nikula <jhnikula@gmail.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Nishanth Menon <nm@ti.com>
Signed-off-by: Kalle Jokiniemi <kalle.jokiniemi@digia.com>
Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>