| 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
| ||
|
Message-ID: <1592894173.10773.42.camel@mtkswgap22>
Date: Tue, 23 Jun 2020 14:36:13 +0800
From: Hanks Chen <hanks.chen@...iatek.com>
To: Marc Zyngier <maz@...nel.org>
CC: CC Hwang <cc.hwang@...iatek.com>,
Jason Cooper <jason@...edaemon.net>,
<wsd_upstream@...iatek.com>, Loda Chou <loda.chou@...iatek.com>,
<linux-kernel@...r.kernel.org>,
<linux-mediatek@...ts.infradead.org>,
Matthias Brugger <matthias.bgg@...il.com>,
Thomas Gleixner <tglx@...utronix.de>,
<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH 1/1] irqchip: Add config MTK_SYSIRQ and MTK_CIRQ
On Thu, 2020-06-18 at 22:22 +0800, Hanks Chen wrote:
> On Thu, 2020-06-18 at 13:41 +0100, Marc Zyngier wrote:
> > On 2020-06-18 13:31, Hanks Chen wrote:
> > > Mediatek sysirq and cirq drivers as-is were bound together to the
> > > config
> > > of ARCH_MEDIATEK. These two drivers should be able to be configured
> > > separately. For example, on new Mediatek mobile chips such as
> > > Dimensity
> > > 820, the sysirq driver is not used since the hardware module is
> > > removed.
> > >
> > > Add two new configs to sysirq and cirq drivers.
> > > - config MTK_SYSIRQ for the interrupt polarity controller driver:
> > > sysirq
> > > - config MTK_CIRQ for the low-power interrupt driver: cirq
> > >
> > > Signed-off-by: cc.hwang <cc.hwang@...iatek.com>
> > > Signed-off-by: Hanks Chen <hanks.chen@...iatek.com>
> > > ---
> > > drivers/irqchip/Kconfig | 12 ++++++++++++
> > > drivers/irqchip/Makefile | 3 ++-
> > > 2 files changed, 14 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> > > index 29fead2..cc9aa18 100644
> > > --- a/drivers/irqchip/Kconfig
> > > +++ b/drivers/irqchip/Kconfig
> > > @@ -572,4 +572,16 @@ config LOONGSON_PCH_MSI
> > > help
> > > Support for the Loongson PCH MSI Controller.
> > >
> > > +config MTK_SYSIRQ
> > > + tristate "Mediatek interrupt polarity controller"
> >
> > How do you expect this to work as a module?
>
> My fault, I missed some patches.I'll add them in next patches.
>
> These should be three patches:
> 1. break existing configurations and create new config for them
> 2. Change Kconfig to be loadable as a module and ensure all platforms
> are executable.
> 3. Make the irq-mtk-sysirq driver as a loadable kernel module for GKI
> and flexibility.
> .
> Thank you for your comment
>
> >
> > > + help
> > > + Interrupt polarity controller driver to swap polarity for
> > > + interrupts for Mediatek mobile chips.
> > > +
> > > +config MTK_CIRQ
> > > + bool "Mediatek low-power interrupt controller"
> > > + help
> > > + Low-power interrupt controller driver to monitor IRQS
> > > + in the sleep mode for Mediatek mobile chips.
> > > +
> > > endmenu
> > > diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> > > index 133f9c4..30421d2 100644
> > > --- a/drivers/irqchip/Makefile
> > > +++ b/drivers/irqchip/Makefile
> > > @@ -69,7 +69,8 @@ obj-$(CONFIG_BCM7120_L2_IRQ) += irq-bcm7120-l2.o
> > > obj-$(CONFIG_BRCMSTB_L2_IRQ) += irq-brcmstb-l2.o
> > > obj-$(CONFIG_KEYSTONE_IRQ) += irq-keystone.o
> > > obj-$(CONFIG_MIPS_GIC) += irq-mips-gic.o
> > > -obj-$(CONFIG_ARCH_MEDIATEK) += irq-mtk-sysirq.o irq-mtk-cirq.o
> > > +obj-$(CONFIG_MTK_SYSIRQ) += irq-mtk-sysirq.o
> > > +obj-$(CONFIG_MTK_CIRQ) += irq-mtk-cirq.o
> > > obj-$(CONFIG_ARCH_DIGICOLOR) += irq-digicolor.o
> > > obj-$(CONFIG_RENESAS_H8300H_INTC) += irq-renesas-h8300h.o
> > > obj-$(CONFIG_RENESAS_H8S_INTC) += irq-renesas-h8s.o
> >
> > In general, this approach doesn't look right. As it stands, this is just
> > breaking existing configurations
> >
> > Do you really expect users to know exactly which interrupt controllers
> > their system is going to use? This seems like the wrong assumption. If
> > you really want to save the handful of bytes these drivers take in your
> > image, then add the relevant dependency information in Kconfig.
> >
>
> 1. This patch will add a dependency on it in Kconfig, which ensures all
> platforms are executable.
> 2. We want to follow the Android GKI and according to the requirement of
> GKI(General Kernel Image), all SoC and Board Peripheral drivers are
> loadable kernel modules.so I need to breaking existing configurations
> and create new config for them.
> Thank you for your comment
Hi Marc,
I want to break the dependency between ARCH_MEDIATEK and CIRQ/SYSIRQ,
because we need to follow the GKI.
Could I add the MTK_CIRQ and MTK_SYSIRQ into defconfig?
(arch/arm64/config/defconfig)
It would ensures all platform are executable.
e.g.
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
CONFIG_QCOM_PDC=y
+CONFIG_MTK_SYSIRQ=m
+CONFIG_MTK_CIRQ=y
CONFIG_RESET_QCOM_AOSS=y
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -572,4 +572,18 @@ config LOONGSON_PCH_MSI
help
Support for the Loongson PCH MSI Controller.
+config MTK_SYSIRQ
+ tristate "Mediatek interrupt polarity controller"
+ depends on ARCH_MEDIATEK || COMPILE_TEST
+ help
+ Interrupt polarity controller driver to swap polarity for
+ interrupts for Mediatek mobile chips.
+
+config MTK_CIRQ
+ bool "Mediatek low-power interrupt controller"
+ depends on ARCH_MEDIATEK || COMPILE_TEST
+ help
+ Low-power interrupt controller driver to monitor IRQS
+ in the sleep mode for Mediatek mobile chips.
+
P.S I'll make the irq-mtk-sysirq driver as a loadable kernel module for
GKI
Thanks
> > M.
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Powered by blists - more mailing lists