Re: [PATCH v4 2/2] Add GPIO support for Realtek DHC(Digital Home Center) RTD SoCs.

[Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>]

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@xxxxxxxxxxx>
> Reviewed-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---


...

> +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