linux-kernel - Re: [PATCHv2] pinctrl: baytrail: Clear DIRECT

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 for Android: free password hash cracker in your pocket

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

Message-ID: <541AA677.7050907@intel.com>
Date:	Thu, 18 Sep 2014 11:31:35 +0200
From:	Loic Poulain <loic.poulain@...el.com>
To:	Mika Westerberg <mika.westerberg@...el.com>
CC:	linus.walleij@...aro.org, heikki.krogerus@...ux.intel.com,
	mathias.nyman@...ux.intel.com, samuel.ortiz@...el.com,
	linux-kernel@...r.kernel.org,
	Eric Ernst <eric.ernst@...ux.intel.com>
Subject: Re: [PATCHv2] pinctrl: baytrail: Clear DIRECT_IRQ bit

Warn seems necessary because we unconditionally change
the pin behavior, I didn't meet any case where direct irq is truly
used on our platform. But maybe it could happen?
Don't want to cause any hidden regression.

Moreover if it is confirmed that is an hardware issue (BIOS),
We can just keep this patch locally as a workaround and revert
it later once our BIOS fixed.

Regards,
Loic

On 18/09/2014 09:49, Mika Westerberg wrote:
> On Wed, Sep 17, 2014 at 03:47:01PM +0200, Loic Poulain wrote:
>> Direct Irq En bit can be initialized to a bad value.
>> This bit has to be cleared for io access mode.
> +Eric
>
> I would like to have a bit better explanation *why* this bit needs to be
> cleared.
>
> Also want to ask Eric (who added the WARN()), is there something
> preventing us to do this? I remember last time you said that we are not
> supposed to change this bit runtime.
>
> My preference is that we get rid of the WARN() and just unconditionally
> clear the bit.
>
>> Signed-off-by: Loic Poulain <loic.poulain@...el.com>
>> ---
>>   v2: Apply over ff998356b644ebe723127bd9eec6040b59a4a4f6 + add Warning
>>
>>   drivers/pinctrl/pinctrl-baytrail.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pinctrl/pinctrl-baytrail.c b/drivers/pinctrl/pinctrl-baytrail.c
>> index 701a646..77db3db 100644
>> --- a/drivers/pinctrl/pinctrl-baytrail.c
>> +++ b/drivers/pinctrl/pinctrl-baytrail.c
>> @@ -230,10 +230,13 @@ static int byt_irq_type(struct irq_data *d, unsigned type)
>>   	spin_lock_irqsave(&vg->lock, flags);
>>   	value = readl(reg);
>>
>> +	WARN(value & BYT_DIRECT_IRQ_EN, "Clearing direct_irq_en bit");
>> +
> I wonder how the average user react when this pops up in the dmesg :-)
>
>>   	/* For level trigges the BYT_TRIG_POS and BYT_TRIG_NEG bits
>>   	 * are used to indicate high and low level triggering
>>   	 */
>> -	value &= ~(BYT_TRIG_POS | BYT_TRIG_NEG | BYT_TRIG_LVL);
>> +	value &= ~(BYT_DIRECT_IRQ_EN | BYT_TRIG_POS | BYT_TRIG_NEG |
>> +		   BYT_TRIG_LVL);
>>
>>   	switch (type) {
>>   	case IRQ_TYPE_LEVEL_HIGH:
>> --
>> 1.8.3.2

-- 
Intel Open Source Technology Center
http://oss.intel.com/

--
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/