linux-kernel - Re: [PATCH 3/4] irqchip: imgpdc: Set sys wake polarities to active high

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: <20171004131453.GF25320@jhogan-linux.le.imgtec.org>
Date:   Wed, 4 Oct 2017 14:14:53 +0100
From:   James Hogan <james.hogan@...tec.com>
To:     Ed Blake <ed.blake@...drel.com>
CC:     <tglx@...utronix.de>, <jason@...edaemon.net>,
        <marc.zyngier@....com>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/4] irqchip: imgpdc: Set sys wake polarities to active
 high

Hi Ed,

On Mon, Oct 02, 2017 at 10:55:58AM +0100, Ed Blake wrote:
> 
> Set all sys wake polarities to active high during initial setup.  The
> default is active low, which currently causes the 'flow_type' passed
> into the set_type function to be effectively inverted.
> 
> Signed-off-by: Ed Blake <ed.blake@...drel.com>
> ---
>  drivers/irqchip/irq-imgpdc.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-imgpdc.c b/drivers/irqchip/irq-imgpdc.c
> index 22d8f8a..d1bcfef 100644
> --- a/drivers/irqchip/irq-imgpdc.c
> +++ b/drivers/irqchip/irq-imgpdc.c
> @@ -49,6 +49,8 @@
>  #define PDC_IRQ_ROUTE_EXT_EN_WD		0x00000004
>  #define PDC_IRQ_ROUTE_EXT_EN_IR		0x00000002
>  #define PDC_IRQ_ROUTE_EXT_EN_RTC	0x00000001
> +#define PDC_SYS_WAKE_POL		0x00000020
> +#define PDC_SYS_WAKE_POL_SHIFT		5

I don't see this bit in the documentation for the PDC in TZ1090, so it
must be new in later versions of the IP. Maybe that can be mentioned in
the commit message or a code comment.

Does it need backporting to any stable branches (presumably v4.1+ for
Pistachio's sake)?

>  #define PDC_SYS_WAKE_RESET		0x00000010
>  #define PDC_SYS_WAKE_INT_MODE		0x0000000e
>  #define PDC_SYS_WAKE_INT_MODE_SHIFT	1
> @@ -63,6 +65,9 @@
>  #define PDC_SYS_WAKE_INT_CHANGE		0x6
>  #define PDC_SYS_WAKE_INT_NONE		0x4
>  
> +#define PDC_SYS_WAKE_ACTIVE_LOW		0x0
> +#define PDC_SYS_WAKE_ACTIVE_HIGH	0x1

Its a pity this HW isn't compatible with the older version which I
presume treated the INT_MODEs as active high but would have reported 0
in this bit. Never mind.

> +
>  /**
>   * struct pdc_intc_priv - private pdc interrupt data.
>   * @nr_perips:			Number of peripheral interrupt signals.
> @@ -335,8 +340,9 @@ static void pdc_intc_setup(struct pdc_intc_priv *priv)
>  	for (i = 0; i < priv->nr_syswakes; ++i) {
>  		/* set the IRQ mode to none */
>  		soc_sys_wake_regoff = PDC_SYS_WAKE_BASE + i*PDC_SYS_WAKE_STRIDE;
> -		soc_sys_wake = PDC_SYS_WAKE_INT_NONE
> -				<< PDC_SYS_WAKE_INT_MODE_SHIFT;
> +		soc_sys_wake =
> +			PDC_SYS_WAKE_ACTIVE_HIGH << PDC_SYS_WAKE_POL_SHIFT |
> +			PDC_SYS_WAKE_INT_NONE << PDC_SYS_WAKE_INT_MODE_SHIFT;

Looks reasonable. With a tweaked commit message or comment as mentioned
above:

Acked-by: James Hogan <jhogan@...nel.org>

Cheers
James

>  		pdc_write(priv, soc_sys_wake_regoff, soc_sys_wake);
>  	}
>  }
> -- 
> 1.9.1
> 

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)