Messages in this thread Patch in this message | | | Date | Wed, 19 Jan 2022 13:31:58 +0000 | From | Marc Zyngier <> | Subject | Re: [PATCH v2 2/4] irqchip/meson-gpio: support more than 8 channels gpio irq line |
| |
On Wed, 19 Jan 2022 07:08:07 +0000, Qianggui Song <qianggui.song@amlogic.com> wrote: > > Current meson gpio irqchip driver only support 8 channels for gpio irq > line, later chips may have more then 8 channels, so need to modify code > to support more. > > Signed-off-by: Qianggui Song <qianggui.song@amlogic.com> > --- > drivers/irqchip/irq-meson-gpio.c | 33 +++++++++++++++++++++++--------- > 1 file changed, 24 insertions(+), 9 deletions(-) > > diff --git a/drivers/irqchip/irq-meson-gpio.c b/drivers/irqchip/irq-meson-gpio.c > index d90ff0b92480..eefe15e1b3a6 100644 > --- a/drivers/irqchip/irq-meson-gpio.c > +++ b/drivers/irqchip/irq-meson-gpio.c > @@ -16,7 +16,6 @@ > #include <linux/of.h> > #include <linux/of_address.h> > > -#define NUM_CHANNEL 8 > #define MAX_INPUT_MUX 256 > > #define REG_EDGE_POL 0x00 > @@ -60,6 +59,7 @@ struct irq_ctl_ops { > > struct meson_gpio_irq_params { > unsigned int nr_hwirq; > + unsigned int nr_channels; > bool support_edge_both; > unsigned int edge_both_offset; > unsigned int edge_single_offset; > @@ -81,6 +81,7 @@ struct meson_gpio_irq_params { > .edge_single_offset = 0, \ > .pol_low_offset = 16, \ > .pin_sel_mask = 0xff, \ > + .nr_channels = 8, \ > > #define INIT_MESON_A1_COMMON_DATA(irqs) \ > INIT_MESON_COMMON(irqs, meson_a1_gpio_irq_init, \ > @@ -90,6 +91,7 @@ struct meson_gpio_irq_params { > .edge_single_offset = 8, \ > .pol_low_offset = 0, \ > .pin_sel_mask = 0x7f, \ > + .nr_channels = 8, \ > > static const struct meson_gpio_irq_params meson8_params = { > INIT_MESON8_COMMON_DATA(134) > @@ -136,8 +138,8 @@ static const struct of_device_id meson_irq_gpio_matches[] = { > struct meson_gpio_irq_controller { > const struct meson_gpio_irq_params *params; > void __iomem *base; > - u32 channel_irqs[NUM_CHANNEL]; > - DECLARE_BITMAP(channel_map, NUM_CHANNEL); > + u32 *channel_irqs; > + unsigned long *channel_map;
This really is over-engineering at its best.
With your new fancy HW, you have at most 12 bits being used in this bitmap. So why not have a single unsigned long, no dynamic allocation, and simply an assertion somewhere that checks that nr_channel is never bigger than BITS_PER_LONG? Less code, less memory wasted, less problems.
M.
-- Without deviation from the norm, progress is not possible.
| |