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  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:   Sat, 26 Dec 2020 16:02:21 +0100
From:   Bert Vermeulen <bert@...t.com>
To:     Marc Zyngier <maz@...nel.org>
Cc:     Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        Rob Herring <robh+dt@...nel.org>,
        Paul Burton <paulburton@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Damien Le Moal <damien.lemoal@....com>,
        Mateusz Holenko <mholenko@...micro.com>,
        Stafford Horne <shorne@...il.com>,
        Pawel Czarnecki <pczarnecki@...ernships.antmicro.com>,
        Palmer Dabbelt <palmerdabbelt@...gle.com>,
        C├ędric Le Goater <clg@...d.org>,
        Shawn Guo <shawnguo@...nel.org>, Joel Stanley <joel@....id.au>,
        Leonard Crestez <leonard.crestez@....com>,
        Peng Fan <peng.fan@....com>, linux-mips@...r.kernel.org,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v2] Add support for Realtek RTL838x/RTL839x switch SoCs

On 12/23/20 5:18 PM, Marc Zyngier wrote:

Marc,

Thanks for reviewing. We will rework as needed, however:

> On Wed, 23 Dec 2020 15:06:24 +0000,
> Bert Vermeulen <bert@...t.com> wrote:
[...]

>> +/* Interrupt numbers/bits */
>> +#define RTL8380_IRQ_UART0		31
>> +#define RTL8380_IRQ_UART1		30
>> +#define RTL8380_IRQ_TC0			29
>> +#define RTL8380_IRQ_TC1			28
>> +#define RTL8380_IRQ_OCPTO		27
>> +#define RTL8380_IRQ_HLXTO		26
>> +#define RTL8380_IRQ_SLXTO		25
>> +#define RTL8380_IRQ_NIC			24
>> +#define RTL8380_IRQ_GPIO_ABCD		23
>> +#define RTL8380_IRQ_GPIO_EFGH		22
>> +#define RTL8380_IRQ_RTC			21
>> +#define RTL8380_IRQ_SWCORE		20
>> +#define RTL8380_IRQ_WDT_IP1		19
>> +#define RTL8380_IRQ_WDT_IP2		18
> 
> Why do we need any of this? The mapping should be explicit in the DT.
> 
>> +
>> +/* Global Interrupt Mask Register */
>> +#define RTL8380_ICTL_GIMR	0x00
>> +/* Global Interrupt Status Register */
>> +#define RTL8380_ICTL_GISR	0x04
>> +
>> +/* Cascaded interrupts */
>> +#define RTL8380_CPU_IRQ_SHARED0		(MIPS_CPU_IRQ_BASE + 2)
>> +#define RTL8380_CPU_IRQ_UART		(MIPS_CPU_IRQ_BASE + 3)
>> +#define RTL8380_CPU_IRQ_SWITCH		(MIPS_CPU_IRQ_BASE + 4)
>> +#define RTL8380_CPU_IRQ_SHARED1		(MIPS_CPU_IRQ_BASE + 5)
>> +#define RTL8380_CPU_IRQ_EXTERNAL	(MIPS_CPU_IRQ_BASE + 6)
>> +#define RTL8380_CPU_IRQ_COUNTER		(MIPS_CPU_IRQ_BASE + 7)
>> +
>> +
>> +/* Interrupt routing register */
>> +#define RTL8380_IRR0		0x08
>> +#define RTL8380_IRR1		0x0c
>> +#define RTL8380_IRR2		0x10
>> +#define RTL8380_IRR3		0x14
>> +
>> +/* Cascade map */
>> +#define RTL8380_IRQ_CASCADE_UART0	2
>> +#define RTL8380_IRQ_CASCADE_UART1	1
>> +#define RTL8380_IRQ_CASCADE_TC0		5
>> +#define RTL8380_IRQ_CASCADE_TC1		1
>> +#define RTL8380_IRQ_CASCADE_OCPTO	1
>> +#define RTL8380_IRQ_CASCADE_HLXTO	1
>> +#define RTL8380_IRQ_CASCADE_SLXTO	1
>> +#define RTL8380_IRQ_CASCADE_NIC		4
>> +#define RTL8380_IRQ_CASCADE_GPIO_ABCD	4
>> +#define RTL8380_IRQ_CASCADE_GPIO_EFGH	4
>> +#define RTL8380_IRQ_CASCADE_RTC		4
>> +#define RTL8380_IRQ_CASCADE_SWCORE	3
>> +#define RTL8380_IRQ_CASCADE_WDT_IP1	4
>> +#define RTL8380_IRQ_CASCADE_WDT_IP2	5
>> +
>> +/* Pack cascade map into interrupt routing registers */
>> +#define RTL8380_IRR0_SETTING (\
>> +	(RTL8380_IRQ_CASCADE_UART0	<< 28) | \
>> +	(RTL8380_IRQ_CASCADE_UART1	<< 24) | \
>> +	(RTL8380_IRQ_CASCADE_TC0	<< 20) | \
>> +	(RTL8380_IRQ_CASCADE_TC1	<< 16) | \
>> +	(RTL8380_IRQ_CASCADE_OCPTO	<< 12) | \
>> +	(RTL8380_IRQ_CASCADE_HLXTO	<< 8)  | \
>> +	(RTL8380_IRQ_CASCADE_SLXTO	<< 4)  | \
>> +	(RTL8380_IRQ_CASCADE_NIC	<< 0))
>> +#define RTL8380_IRR1_SETTING (\
>> +	(RTL8380_IRQ_CASCADE_GPIO_ABCD	<< 28) | \
>> +	(RTL8380_IRQ_CASCADE_GPIO_EFGH	<< 24) | \
>> +	(RTL8380_IRQ_CASCADE_RTC	<< 20) | \
>> +	(RTL8380_IRQ_CASCADE_SWCORE	<< 16))
>> +#define RTL8380_IRR2_SETTING	0
>> +#define RTL8380_IRR3_SETTING	0

[...]

>> +	/* Set up interrupt routing */
>> +	writel(RTL8380_IRR0_SETTING, REG(RTL8380_IRR0));
>> +	writel(RTL8380_IRR1_SETTING, REG(RTL8380_IRR1));
>> +	writel(RTL8380_IRR2_SETTING, REG(RTL8380_IRR2));
>> +	writel(RTL8380_IRR3_SETTING, REG(RTL8380_IRR3));
> 
> What is this doing?

It's fairly evident considering the comments -- routing of secondary IRQs 
onto the CPU IRQs. But as to packing this into DTS I'm not sure.

DTS syntax being what it is, this would inevitably get more complex and 
harder to understand. Do you have an example where this is done in a better way?

thanks,


-- 
Bert Vermeulen
bert@...t.com

Powered by blists - more mailing lists