Hi Mark,
On 2022/7/24 上午3:32, Mark Brown wrote:
On Sun, Jul 24, 2022 at 01:06:16AM +0800, Zhou Yanjie wrote:
On 2022/7/23 上午2:38, Mark Brown wrote:
+++ b/drivers/spi/spi-ingenic-sfc.c
@@ -0,0 +1,662 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+/*
+ * Ingenic SoCs SPI Flash Controller Driver
Please make the entire comment a C++ one so things look more
intentional.
I'm sorry, I didn't understand well what you meant :(
Could you please explain a little more detail?
The above comment block uses both C /* */ and C++ // style comments,
please make it just use the C++ style.
Sure, will do in the next version.
+static irqreturn_t ingenic_sfc_irq_handler(int irq, void *data)
+{
+ struct ingenic_sfc *sfc = data;
+
+ writel(0x1f, sfc->base + SFC_REG_INTC);
+
+ complete(&sfc->completion);
+
+ return IRQ_HANDLED;
+}
This doesn't pay any attention to any status registers in the chip so
won't work if the interrupt is shared and won't notice any error reports
from the device...
This interrupt is exclusively owned by SFC, do we still
need to perform the operation you said? I haven't done
these operations before because I want to minimize the
overhead and avoid affecting performance.
Even if the device is not shared is there no possibility that the
device can report an unexpected interrupt status? It's not just
the sharing case, it's also the fact that it looks like there's a
status being reported but we're not checking it so if anything
goes wrong then we're less likely to notice. I'd worry about
data corruption.
Sure, I will change this in the next version.
Thanks and best regards!
