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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Thu, 18 Sep 2014 10:49:43 +0300
From:	Mika Westerberg <mika.westerberg@...el.com>
To:	Loic Poulain <loic.poulain@...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

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