linux-kernel - Re: [PATCH v3 2/4] mfd: max8997: handle IRQs using regmap

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-id: <5322BEDF.4080408@samsung.com>
Date:	Fri, 14 Mar 2014 09:33:35 +0100
From:	Robert Baldyga <r.baldyga@...sung.com>
To:	Chanwoo Choi <cw00.choi@...sung.com>
Cc:	sameo@...ux.intel.com, lee.jones@...aro.org,
	myungjoo.ham@...sung.com, dmitry.torokhov@...il.com,
	cooloney@...il.com, rpurdie@...ys.net, dbaryshkov@...il.com,
	dwmw2@...radead.org, lgirdwood@...il.com, broonie@...nel.org,
	a.zummo@...ertech.it, paul.gortmaker@...driver.com,
	linux-kernel@...r.kernel.org, linux-input@...r.kernel.org,
	linux-leds@...r.kernel.org, rtc-linux@...glegroups.com,
	m.szyprowski@...sung.com, k.kozlowski@...sung.com
Subject: Re: [PATCH v3 2/4] mfd: max8997: handle IRQs using regmap

On 03/13/2014 11:45 AM, Chanwoo Choi wrote:
> Hi Robert,
> 
> On 03/13/2014 06:38 PM, Robert Baldyga wrote:
>> This patch modifies mfd driver to use regmap for handling interrupts.
>> It allows to simplify irq handling process. This modifications needed
>> to make small changes in function drivers, which use interrupts.
>>
>> Signed-off-by: Robert Baldyga <r.baldyga@...sung.com>

(...)

>> @@ -468,8 +560,11 @@ static int max8997_suspend(struct device *dev)
>>  	struct i2c_client *i2c = container_of(dev, struct i2c_client, dev);
>>  	struct max8997_dev *max8997 = i2c_get_clientdata(i2c);
>>  
>> -	if (device_may_wakeup(dev))
>> -		irq_set_irq_wake(max8997->irq, 1);
>> +	if (device_may_wakeup(dev)) {
>> +		enable_irq_wake(max8997->irq);
>> +		disable_irq(max8997->irq);
> 
> irq_set_irq_wake() is same as enable_irq_wake().

It's more intuitive name and makes code reading easier.

> Why is it necessary for disable_irq(max8997->irq)?

It's because we don't want to get interrupts before i2c controller will
be ready. So we disable irq in drivers suspend, and enable in resume.

If some iqr will come before enable_irq() call, it will be noticed, and
IRQ_PENDING flag will be set. In this case irq will be handled
immediately after enable_irq() call.

In previous version there was call of max8997_irq_resume() in resume
function, which caused forced interrupt handling, which has similar effect.

> 
>> +	}
>> +
>>  	return 0;
>>  }
>>  
>> @@ -478,9 +573,12 @@ static int max8997_resume(struct device *dev)
>>  	struct i2c_client *i2c = container_of(dev, struct i2c_client, dev);
>>  	struct max8997_dev *max8997 = i2c_get_clientdata(i2c);
>>  
>> -	if (device_may_wakeup(dev))
>> -		irq_set_irq_wake(max8997->irq, 0);
>> -	return max8997_irq_resume(max8997);
>> +	if (device_may_wakeup(dev)) {
>> +		disable_irq_wake(max8997->irq);
>> +		enable_irq(max8997->irq);
> 
> ditto.
> 
>> +	}
>> +
>> +	return 0;
>>  }
>>  

Thanks,
Robert Baldyga
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/