[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2a44fe91-b4a5-4842-8abc-f30c532f14e0@linaro.org>
Date: Sat, 23 Dec 2023 15:19:21 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Tzuyi Chang <tychang@...ltek.com>,
Linus Walleij <linus.walleij@...aro.org>, Bartosz Golaszewski
<brgl@...ev.pl>, Andy Shevchenko <andy@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>
Cc: linux-gpio@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 2/2] Add GPIO support for Realtek DHC(Digital Home
Center) RTD SoCs.
On 22/12/2023 08:58, Tzuyi Chang wrote:
> This driver enables configuration of GPIO direction, GPIO values, GPIO
> debounce settings and handles GPIO interrupts.
>
> Signed-off-by: Tzuyi Chang <tychang@...ltek.com>
> Reviewed-by: Linus Walleij <linus.walleij@...aro.org>
> ---
...
> +static int rtd_gpio_irq_set_type(struct irq_data *d, unsigned int type)
> +{
> + struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> + struct rtd_gpio *data = gpiochip_get_data(gc);
> + irq_hw_number_t hwirq = irqd_to_hwirq(d);
> + u32 mask = BIT(hwirq % 32);
> + unsigned long flags;
> + int dp_reg_offset;
> + bool polarity;
> + u32 val;
> +
> + dp_reg_offset = rtd_gpio_dp_offset(data, hwirq);
> +
> + switch (type & IRQ_TYPE_SENSE_MASK) {
> + case IRQ_TYPE_EDGE_RISING:
> + polarity = 1;
> + break;
> +
> + case IRQ_TYPE_EDGE_FALLING:
> + polarity = 0;
> + break;
> +
> + case IRQ_TYPE_EDGE_BOTH:
> + polarity = 1;
> + break;
> +
> + default:
> + return -EINVAL;
> + }
> +
> + raw_spin_lock_irqsave(&data->lock, flags);
Why are you using raw spinlock? This question applies to entire driver.
> +
> + val = readl_relaxed(data->base + dp_reg_offset);
> + if (polarity)
> + val |= mask;
> + else
> + val &= ~mask;
> + writel_relaxed(val, data->base + dp_reg_offset);
> +
> + raw_spin_unlock_irqrestore(&data->lock, flags);
> +
> + return 0;
> +}
...
}
> +
> +module_init(rtd_gpio_init);
> +
> +static void __exit rtd_gpio_exit(void)
> +{
> + platform_driver_unregister(&rtd_gpio_platform_driver);
> +}
> +module_exit(rtd_gpio_exit);
Why not using module_platform_driver?
Best regards,
Krzysztof
Powered by blists - more mailing lists