On 2020-06-11 2:35 a.m., Bartosz Golaszewski wrote:
sob., 6 cze 2020 o 01:57 Robert Hancock <hancock@xxxxxxxxxxxxx> napisał(a):
Adds interrupt support to the Xilinx GPIO driver so that rising and
falling edge line events can be supported. Since interrupt support is
an optional feature in the Xilinx IP, the driver continues to support
devices which have no interrupt provided.
These changes are based on a patch in the Xilinx Linux Git tree,
"gpio: xilinx: Add irq support to the driver" from Srinivas Neeli, but
include a number of fixes and improvements such as supporting both
rising and falling edge events.
Signed-off-by: Robert Hancock <hancock@xxxxxxxxxxxxx>
---
drivers/gpio/Kconfig | 1 +
drivers/gpio/gpio-xilinx.c | 247 ++++++++++++++++++++++++++++++++++---
2 files changed, 233 insertions(+), 15 deletions(-)
(snip)
/* Read/Write access to the GPIO registers */
#if defined(CONFIG_ARCH_ZYNQ) || defined(CONFIG_X86)
# define xgpio_readreg(offset) readl(offset)
@@ -35,17 +43,27 @@
* @gc: GPIO chip
* @regs: register block
* @gpio_width: GPIO width for every channel
- * @gpio_state: GPIO state shadow register
+ * @gpio_state: GPIO write state shadow register
+ * @gpio_last_irq_read: GPIO read state register from last interrupt
* @gpio_dir: GPIO direction shadow register
* @gpio_lock: Lock used for synchronization
+ * @irq: IRQ used by GPIO device
+ * @irq_enable: GPIO irq enable/disable bitfield
+ * @irq_rising_edge: GPIO irq rising edge enable/disable bitfield
+ * @irq_falling_edge: GPIO irq rising edge enable/disable bitfield
*/
struct xgpio_instance {
struct gpio_chip gc;
void __iomem *regs;
unsigned int gpio_width[2];
u32 gpio_state[2];
+ u32 gpio_last_irq_read[2];
u32 gpio_dir[2];
- spinlock_t gpio_lock[2];
+ spinlock_t gpio_lock;
+ int irq;
+ u32 irq_enable[2];
+ u32 irq_rising_edge[2];
+ u32 irq_falling_edge[2];
I don't know this driver very well. Could you explain why the two
instances of these fields and why are you removing the second lock?
The Xilinx IP has either one or two banks of GPIO inputs, which is why
there are two instances of most of these fields. However, the interrupt
status and interrupt mask registers are shared between the banks (there
are separate bits for each bank), which would make the locking difficult
to manage if separate locks were used. There seemed to be no real
downside in using a single lock as there shouldn't be significant
contention.
chip->gc.base = -1;
@@ -336,6 +530,29 @@ static int xgpio_probe(struct platform_device *pdev)
xgpio_save_regs(chip);
+ chip->irq = platform_get_irq(pdev, 0);
Why not simply: platform_get_irq_optional()?
Can do, will update for v2.
--
Robert Hancock
Senior Hardware Designer
SED Systems, a division of Calian Ltd.
Email: hancock@xxxxxxxxxxxxx
