Hi Linus,
On 19. 5. 31. 오전 3:39, Linus Walleij wrote:
> The only thing that makes sense is to request a falling edge interrupt
> if the line is active low and a rising edge interrupt if the line is
> active high, so just do that and get rid of the assignment from
> platform data. The GPIO descriptor knows if the line is active high
> or low.
>
> Also make irq a local variable in probe(), it's not used anywhere else.
>
> Signed-off-by: Linus Walleij <linus.wall...@linaro.org>
> ---
> drivers/extcon/extcon-gpio.c | 27 ++++++++++++++++++---------
> 1 file changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/extcon/extcon-gpio.c b/drivers/extcon/extcon-gpio.c
> index 13ba3a6e81d5..a0674f1f3849 100644
> --- a/drivers/extcon/extcon-gpio.c
> +++ b/drivers/extcon/extcon-gpio.c
> @@ -30,26 +30,22 @@
> /**
> * struct gpio_extcon_data - A simple GPIO-controlled extcon device state
> container.
> * @edev: Extcon device.
> - * @irq: Interrupt line for the external connector.
> * @work: Work fired by the interrupt.
> * @debounce_jiffies: Number of jiffies to wait for the GPIO to
> stabilize, from the debounce
> * value.
> * @gpiod: GPIO descriptor for this external connector.
> * @extcon_id: The unique id of specific external connector.
> * @debounce: Debounce time for GPIO IRQ in ms.
> - * @irq_flags: IRQ Flags (e.g., IRQF_TRIGGER_LOW).
> * @check_on_resume: Boolean describing whether to check the state of gpio
> * while resuming from sleep.
> */
> struct gpio_extcon_data {
> struct extcon_dev *edev;
> - int irq;
> struct delayed_work work;
> unsigned long debounce_jiffies;
> struct gpio_desc *gpiod;
> unsigned int extcon_id;
> unsigned long debounce;
> - unsigned long irq_flags;
> bool check_on_resume;
> };
>
> @@ -77,6 +73,8 @@ static int gpio_extcon_probe(struct platform_device *pdev)
> {
> struct gpio_extcon_data *data;
> struct device *dev = &pdev->dev;
> + unsigned long irq_flags;
> + int irq;
> int ret;
>
> data = devm_kzalloc(dev, sizeof(struct gpio_extcon_data), GFP_KERNEL);
> @@ -96,9 +94,20 @@ static int gpio_extcon_probe(struct platform_device *pdev)
> data->gpiod = devm_gpiod_get(dev, "extcon", GPIOD_IN);
> if (IS_ERR(data->gpiod))
> return PTR_ERR(data->gpiod);
> - data->irq = gpiod_to_irq(data->gpiod);
> - if (data->irq <= 0)
> - return data->irq;
> + irq = gpiod_to_irq(data->gpiod);
> + if (irq <= 0)
> + return irq;
> +
> + /*
> + * It is unlikely that this is an acknowledged interrupt that goes
> + * away after handling, what we are looking for are falling edges
> + * if the signal is active low, and rising edges if the signal is
> + * active high.
> + */
> + if (gpiod_is_active_low(data->gpiod))
> + irq_flags = IRQF_TRIGGER_FALLING;
If gpiod_is_active_low(data->gpiod) is true, irq_flags might be
IRQF_TRIGGER_LOW instead of IRQF_TRIGGER_FALLING. How can we sure
that irq_flags is always IRQF_TRIGGER_FALLING?
> + else
> + irq_flags = IRQF_TRIGGER_RISING;
>
> /* Allocate the memory of extcon devie and register extcon device */
> data->edev = devm_extcon_dev_allocate(dev, &data->extcon_id);
> @@ -117,8 +126,8 @@ static int gpio_extcon_probe(struct platform_device *pdev)
> * Request the interrupt of gpio to detect whether external connector
> * is attached or detached.
> */
> - ret = devm_request_any_context_irq(dev, data->irq,
> - gpio_irq_handler, data->irq_flags,
> + ret = devm_request_any_context_irq(dev, irq,
> + gpio_irq_handler, irq_flags,
> pdev->name, data);
> if (ret < 0)
> return ret;
>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
