linux-kernel - Re: [patch 09/13] irqchip: spear_shirq: Kill the clear

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: <CAOh2x==nhq6OHkd9U17L3r-_7qnnjYLyiqiKYpGdMuSVEj07=g@mail.gmail.com>
Date:	Fri, 20 Jun 2014 12:35:00 +0530
From:	Viresh Kumar <viresh.kumar@...aro.org>
To:	Thomas Gleixner <tglx@...utronix.de>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Jason Cooper <jason@...edaemon.net>,
	Shiraz Hashim <shiraz.linux.kernel@...il.com>,
	spear-devel <spear-devel@...t.st.com>
Subject: Re: [patch 09/13] irqchip: spear_shirq: Kill the clear_reg nonsense

On Fri, Jun 20, 2014 at 3:04 AM, Thomas Gleixner <tglx@...utronix.de> wrote:
> None of the chips has a ACK register.

I need to recheck on this after looking at datasheets. Arranging for
them, will revert by tomorrow.

> The code brainlessly fiddles
> with the enable register, so it might even reenable a disabled
> interrupt at least on spear300.

Ack/Clear register is only configured for SPEAr320, how will it
make a difference to SPEAr300 ?

And for SPEAr320 as well, the offset mentioned in code for clear
register is different then ENABLE register.

> Index: linux/drivers/irqchip/spear-shirq.c
> ===================================================================
> --- linux.orig/drivers/irqchip/spear-shirq.c
> +++ linux/drivers/irqchip/spear-shirq.c
> @@ -33,15 +33,11 @@
>   * reset_to_enb: val 1 indicates, we need to clear bit for enabling interrupt
>   * status_reg: status register offset
>   * status_reg_mask: status register valid mask
> - * clear_reg: clear register offset
> - * reset_to_clear: val 1 indicates, we need to clear bit for clearing interrupt
>   */
>  struct shirq_regs {
>         u32 enb_reg;
>         u32 reset_to_enb;
>         u32 status_reg;
> -       u32 clear_reg;

> -       u32 reset_to_clear;

AFAIR, there was a revision for SPEAr320 which was actually using
reset_to_clear and so was present in code. But later revisions got rid
of it and code never got updated.

> @@ -150,13 +141,6 @@ static struct spear_shirq spear320_shirq
>         .nr_irqs        = 7,
>         .mask           = ((0x1 << 7) - 1) << 0,
>         .disabled       = 1,
> -       .regs = {
> -               .enb_reg = SPEAR320_INT_ENB_MASK_REG,
> -               .reset_to_enb = 1,
> -               .status_reg = SPEAR320_INT_STS_MASK_REG,
> -               .clear_reg = SPEAR320_INT_CLR_MASK_REG,
> -               .reset_to_clear = 1,
> -       },

Was removing .regs completely intentional?

I don't see these registers getting added again in later patches.
--
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/