ADP5588 linux driver problem accessing through GPIO char device gpio-event-mon.c tool

Hello,

I am trying to control the ADP558 device using your linux driver:

https://wiki.analog.com/resources/tools-software/linux-drivers/input-keyboard/adp5588

As you know the GPIO sysfs is deprecated now and the right way to access is through the GPIO char device. I want to request a GPIO interrupt using the gpio-event-mon.c tool under /linux/tools/gpio: https://github.com/torvalds/linux/blob/master/tools/gpio/gpio-event-mon.c

The problem is that when i try to use this tool in the driver is followed the next sequence:

[ 4594.022049] adp5588 1-0020: adp5588_gpio_direction input is called
[ 4594.032877] adp5588 1-0020: adp5588_irq_bus_lock is called
[ 4594.043672] adp5588 1-0020: adp5588_irq_bus_sync_unlock is called
[ 4594.066650] adp5588 1-0020: adp5588_irq_bus_lock is called
[ 4594.072008] adp5588 : adp5588_irq_set_type is called
[ 4594.077328] adp5588 : adp5588_irq_unmask is called
[ 4594.083673] adp5588 1-0020: adp5588_irq_bus_sync_unlock is called

My doubt is:

Why the irq_unmask() callback function is called after the irq_bus_sync_unlock() one when the gpio char device access trhough the tool is used? Checking your driver the unmask of the GPIO interrupt is done in the irq_unmask() function.  If irq_unmask() is called after irq_bus_sync_unlock(), then the unmask value is not transmitted to the device through I2C via the  irq_bus_sync_unlock() function.  

To make the interrupts work I should unmask the gpio interrupt in the irq_bus_sync_unlock() function so that the interrupt is launched. Am i right? Could you take a look to this?

Thanks

Alberto

Parents
  • 0
    •  Analog Employees 
    on Oct 13, 2020 11:48 AM
    My doubt is:

    Why the irq_unmask() callback function is called after the irq_bus_sync_unlock() one when the gpio char device access trhough the tool is used? Checking your driver the unmask of the GPIO interrupt is done in the irq_unmask() function.  If irq_unmask() is called after irq_bus_sync_unlock(), then the unmask value is not transmitted to the device through I2C via the  irq_bus_sync_unlock() function.  

    So, is there an observable behavior that is wrong?

    Looking at the driver code, it looks like it is hooking into the correct places for the irq_chip struct.

  • Hi,

    In my opinion irq_unmask() should be called before irq_bus_sync_unlock(). The idea is to unmask the interrupt in irq_unmask() and then transmit the new value via I2C to the device using irq_bus_sync_unlock(). What is the sense of unmasking the interrupt after updating the value to the register of the device via I2C? Maybe i am missing something but i think that this behavior is wrong.

    Best Regards

  • 0
    •  Analog Employees 
    on Oct 14, 2020 12:00 PM in reply to Aliberal

    I think we are debathing here some old Linux interface.

    This forum isn't meant to do that.

    AFAICT, the driver hooks into the Linux kernel, and it is the kernel calling the driver routine.

    static void adp5588_irq_unmask(struct irq_data *d)
    {
            struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
            struct adp5588_gpio *dev = gpiochip_get_data(gc);

            dev->irq_mask[ADP5588_BANK(d->hwirq)] |= ADP5588_BIT(d->hwirq);
    }

    static struct irq_chip adp5588_irq_chip = {
            .name                   = "adp5588",
            .irq_mask               = adp5588_irq_mask,
            .irq_unmask             = adp5588_irq_unmask,
            .irq_bus_lock           = adp5588_irq_bus_lock,
            .irq_bus_sync_unlock    = adp5588_irq_bus_sync_unlock,
            .irq_set_type           = adp5588_irq_set_type,
    };

    If there are doubts about the correctness of this interface, then maybe these are better suited for discussion on the Linux kernel list, not here.

    There are many wonky/funky semantics in the kernel.

    It is how the maintainers designed them in that moment in time.

    One example, is the   regulator_get_optional()  vs   regulator_get(). regulator_get_optional() will return an error if there is no regulator, regulator_get() will create a dummy regulator.

    On the other hand, gpiod_get_optional() will return a NULL gpio object, while gpiod_get() will return an error if there is no GPIO. Similar is for clck_get_optional() and clk_get()

    So, if there are issues with the ADP5588, this is the place to raise issues.

    But, if there are issues/doubts with the Linux interface, the place is the Linux mailing list.

  • Thanks for your comments! I have already solved it. It was a problem in my SW configuration. The Linux driver is working fine. Best Regards. Alberto

Reply Children
No Data