[v1,1/2] irqchip/tegra: Remove everything related to COP
diff mbox series

Message ID 20190811183044.13925-1-digetx@gmail.com
State New
Headers show
Series
  • [v1,1/2] irqchip/tegra: Remove everything related to COP
Related show

Commit Message

Dmitry Osipenko Aug. 11, 2019, 6:30 p.m. UTC
There is no point in touching of the COP (ARM7TDMI auxiliary boot/firmware
CPU) because COP's interrupts should be related only to an old multimedia
firmware that is not applicable to the upstream kernel. Hence let's remove
everything related to the COP, for consistency.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/irqchip/irq-tegra.c | 17 -----------------
 1 file changed, 17 deletions(-)

Comments

Marc Zyngier Aug. 13, 2019, 2:25 p.m. UTC | #1
On Sun, 11 Aug 2019 19:30:43 +0100,
Dmitry Osipenko <digetx@gmail.com> wrote:
> 
> There is no point in touching of the COP (ARM7TDMI auxiliary boot/firmware
> CPU) because COP's interrupts should be related only to an old multimedia
> firmware that is not applicable to the upstream kernel. Hence let's remove
> everything related to the COP, for consistency.

We've had that exact discussion a few weeks ago, and I objected to
this change as there is no possible way you can know for sure people
don't use mainline with an old firmware. The whole point of the
firmware is to be an abstraction for the kernel, for better or worse.

If you really want to do something useful here, consider detecting
such firmware (even better, detect the new ones that don't require
this stuff) and document the various requirements. At least we'll know
where we stand, because the changes you're making are just as random
as what we have so far.

Thanks,

	M.
Dmitry Osipenko Aug. 13, 2019, 3:09 p.m. UTC | #2
13.08.2019 17:25, Marc Zyngier пишет:
> On Sun, 11 Aug 2019 19:30:43 +0100,
> Dmitry Osipenko <digetx@gmail.com> wrote:
>>
>> There is no point in touching of the COP (ARM7TDMI auxiliary boot/firmware
>> CPU) because COP's interrupts should be related only to an old multimedia
>> firmware that is not applicable to the upstream kernel. Hence let's remove
>> everything related to the COP, for consistency.
> 
> We've had that exact discussion a few weeks ago, and I objected to
> this change as there is no possible way you can know for sure people
> don't use mainline with an old firmware. The whole point of the
> firmware is to be an abstraction for the kernel, for better or worse.

I think you're misunderstanding what kind of firmware it is. It absolutely can't be used
with upstream kernel without of porting the whole downstream driver stack (which is a very
old crusty v3.1 kernel, a typical non-upstreamable downstream hackery). In upstream we now
have a proper DRM graphics driver, video decoder driver and etc. Few years ago those
downstream drivers were obsoleted and not supported by the NVIDIA anymore.

> If you really want to do something useful here, consider detecting
> such firmware (even better, detect the new ones that don't require
> this stuff) and document the various requirements. At least we'll know
> where we stand, because the changes you're making are just as random
> as what we have so far.

As I just said in the above, that old firmware is not usable with the upstream in any shape
or form. Hence there is nothing to detect, it is safe to assume that only newer hardware has
something running in background. Peter De Schrijver suggested that kernel shouldn't touch
the COP's interrupts for the newer firmware, so this patch should be fine if Peter or
somebody else from NVIDIA will give an ACK.

Patch
diff mbox series

diff --git a/drivers/irqchip/irq-tegra.c b/drivers/irqchip/irq-tegra.c
index e1f771c72fc4..14dcacc2ad38 100644
--- a/drivers/irqchip/irq-tegra.c
+++ b/drivers/irqchip/irq-tegra.c
@@ -33,11 +33,6 @@ 
 #define ICTLR_CPU_IER_CLR	0x28
 #define ICTLR_CPU_IEP_CLASS	0x2C
 
-#define ICTLR_COP_IER		0x30
-#define ICTLR_COP_IER_SET	0x34
-#define ICTLR_COP_IER_CLR	0x38
-#define ICTLR_COP_IEP_CLASS	0x3c
-
 #define TEGRA_MAX_NUM_ICTLRS	6
 
 static unsigned int num_ictlrs;
@@ -68,8 +63,6 @@  static const struct of_device_id ictlr_matches[] = {
 struct tegra_ictlr_info {
 	void __iomem *base[TEGRA_MAX_NUM_ICTLRS];
 #ifdef CONFIG_PM_SLEEP
-	u32 cop_ier[TEGRA_MAX_NUM_ICTLRS];
-	u32 cop_iep[TEGRA_MAX_NUM_ICTLRS];
 	u32 cpu_ier[TEGRA_MAX_NUM_ICTLRS];
 	u32 cpu_iep[TEGRA_MAX_NUM_ICTLRS];
 
@@ -144,11 +137,6 @@  static int tegra_ictlr_suspend(void)
 		/* Save interrupt state */
 		lic->cpu_ier[i] = readl_relaxed(ictlr + ICTLR_CPU_IER);
 		lic->cpu_iep[i] = readl_relaxed(ictlr + ICTLR_CPU_IEP_CLASS);
-		lic->cop_ier[i] = readl_relaxed(ictlr + ICTLR_COP_IER);
-		lic->cop_iep[i] = readl_relaxed(ictlr + ICTLR_COP_IEP_CLASS);
-
-		/* Disable COP interrupts */
-		writel_relaxed(~0ul, ictlr + ICTLR_COP_IER_CLR);
 
 		/* Disable CPU interrupts */
 		writel_relaxed(~0ul, ictlr + ICTLR_CPU_IER_CLR);
@@ -175,11 +163,6 @@  static void tegra_ictlr_resume(void)
 		writel_relaxed(~0ul, ictlr + ICTLR_CPU_IER_CLR);
 		writel_relaxed(lic->cpu_ier[i],
 			       ictlr + ICTLR_CPU_IER_SET);
-		writel_relaxed(lic->cop_iep[i],
-			       ictlr + ICTLR_COP_IEP_CLASS);
-		writel_relaxed(~0ul, ictlr + ICTLR_COP_IER_CLR);
-		writel_relaxed(lic->cop_ier[i],
-			       ictlr + ICTLR_COP_IER_SET);
 	}
 	local_irq_restore(flags);
 }