lkml.org 
[lkml]   [2022]   [Jan]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 4/4] irqchip/meson-gpio: Add support for meson s4 SoCs
On Sat, 08 Jan 2022 08:42:18 +0000,
Qianggui Song <qianggui.song@amlogic.com> wrote:
>
> The meson s4 SoCs support 12 gpio irq lines compared with previous
> serial chips and have something different, details are as below.
>
> IRQ Number:
> - 80:68 13 pins on bank Z
> - 67:48 20 pins on bank X
> - 47:36 12 pins on bank H
> - 35:24 12 pins on bank D
> - 23:22 2 pins on bank E
> - 21:14 8 pins on bank C
> - 13:0 13 pins on bank B
>
> - PADCTRL_GPIO_IRQ_CTRL0
> bit[31]: enable/disable the whole irq lines

s/the whole/all the/

> bit[12-23]: single edge trigger
> bit[0-11]: poll trigger
>
> - PADCTRL_GPIO_IRQ_CTRL[X]
> - bit[0-16]: 7 bits to chooge gpio source for irq line 2*[X] - 2

choose?

> - bit[16-22]:7 bits to chooge gpio source for irq line 2*[X] - 1
> where X = 1-6
>
> - PADCTRL_GPIO_IRQ_CTRL[7]
> bit[0-11]: both edge trigger

This information would fit better in the code than in the commit
message.

>
> Signed-off-by: Qianggui Song <qianggui.song@amlogic.com>
> ---
> drivers/irqchip/irq-meson-gpio.c | 51 ++++++++++++++++++++++++++++++++
> 1 file changed, 51 insertions(+)
>
> diff --git a/drivers/irqchip/irq-meson-gpio.c b/drivers/irqchip/irq-meson-gpio.c
> index 98419428fcbd..c5d20a866c37 100644
> --- a/drivers/irqchip/irq-meson-gpio.c
> +++ b/drivers/irqchip/irq-meson-gpio.c
> @@ -42,6 +42,11 @@
> #define REG_PIN_SEL_SHIFT(x) (((x) % 4) * 8)
> #define REG_FILTER_SEL_SHIFT(x) ((x) * 4)
>
> +/* use for s4 chips */

s/use/Used/

> +#define REG_EDGE_POL_S4 0x1c
> +#define REG_EDGE_POL_MASK_S4(x) \
> + ({typeof(x) _x = (x); BIT(_x) | BIT(12 + (_x)); })

Why on Earth should this macro handle multiple types?

> +
> struct meson_gpio_irq_controller;
> static void meson8_gpio_irq_sel_pin(struct meson_gpio_irq_controller *ctl,
> unsigned int channel, unsigned long hwirq);
> @@ -50,6 +55,9 @@ static void meson_a1_gpio_irq_sel_pin(struct meson_gpio_irq_controller *ctl,
> unsigned int channel,
> unsigned long hwirq);
> static void meson_a1_gpio_irq_init(struct meson_gpio_irq_controller *ctl);
> +static unsigned int
> +meson_s4_gpio_irq_sel_type(struct meson_gpio_irq_controller *ctl,
> + unsigned int idx, u32 val);
>
> struct irq_ctl_ops {
> void (*gpio_irq_sel_pin)(struct meson_gpio_irq_controller *ctl,
> @@ -96,6 +104,17 @@ struct meson_gpio_irq_params {
> .pin_sel_mask = 0x7f, \
> .channel_num = 8, \
>
> +#define INIT_MESON_S4_COMMON_DATA(irqs) \
> + INIT_MESON_COMMON(irqs, meson_a1_gpio_irq_init, \
> + meson_a1_gpio_irq_sel_pin, \
> + meson_s4_gpio_irq_sel_type) \
> + .support_edge_both = true, \
> + .edge_both_offset = 0, \
> + .edge_single_offset = 12, \
> + .pol_low_offset = 0, \
> + .pin_sel_mask = 0xff, \
> + .channel_num = 12, \
> +
> static const struct meson_gpio_irq_params meson8_params = {
> INIT_MESON8_COMMON_DATA(134)
> };
> @@ -126,6 +145,10 @@ static const struct meson_gpio_irq_params a1_params = {
> INIT_MESON_A1_COMMON_DATA(62)
> };
>
> +static const struct meson_gpio_irq_params s4_params = {
> + INIT_MESON_S4_COMMON_DATA(82)
> +};
> +
> static const struct of_device_id meson_irq_gpio_matches[] = {
> { .compatible = "amlogic,meson8-gpio-intc", .data = &meson8_params },
> { .compatible = "amlogic,meson8b-gpio-intc", .data = &meson8b_params },
> @@ -135,6 +158,7 @@ static const struct of_device_id meson_irq_gpio_matches[] = {
> { .compatible = "amlogic,meson-g12a-gpio-intc", .data = &axg_params },
> { .compatible = "amlogic,meson-sm1-gpio-intc", .data = &sm1_params },
> { .compatible = "amlogic,meson-a1-gpio-intc", .data = &a1_params },
> + { .compatible = "amlogic,meson-s4-gpio-intc", .data = &s4_params },
> { }
> };
>
> @@ -202,6 +226,33 @@ static void meson_a1_gpio_irq_init(struct meson_gpio_irq_controller *ctl)
> meson_gpio_irq_update_bits(ctl, REG_EDGE_POL, BIT(31), BIT(31));
> }
>
> +static unsigned int
> +meson_s4_gpio_irq_sel_type(struct meson_gpio_irq_controller *ctl,
> + unsigned int idx, unsigned int type)

An 'unsigned int' return type, directly returned by a caller that
has 'int' as its return type. What could possibly go wrong?

> +{
> + unsigned int val = 0;
> +
> + meson_gpio_irq_update_bits(ctl, REG_EDGE_POL_S4, BIT(0 + (idx)), 0);

Drop the 0 + as well as the useless bracketing all over this function.

> +
> + if (type == IRQ_TYPE_EDGE_BOTH) {
> + val |= BIT(ctl->params->edge_both_offset + (idx));
> + meson_gpio_irq_update_bits(ctl, REG_EDGE_POL_S4,
> + BIT(ctl->params->edge_both_offset + (idx)), val);
> + return 0;
> + }
> +
> + if (type & (IRQ_TYPE_LEVEL_LOW | IRQ_TYPE_EDGE_FALLING))
> + val |= BIT(ctl->params->pol_low_offset + (idx));
> +
> + if (type & (IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING))
> + val |= BIT(ctl->params->edge_single_offset + (idx));
> +
> + meson_gpio_irq_update_bits(ctl, REG_EDGE_POL,
> + REG_EDGE_POL_MASK_S4(idx), val);
> +
> + return 0;
> +};
> +
> static int
> meson_gpio_irq_request_channel(struct meson_gpio_irq_controller *ctl,
> unsigned long hwirq,

M.

--
Without deviation from the norm, progress is not possible.
\
 
 \ /
  Last update: 2022-01-08 12:07    [W:0.048 / U:1.328 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site