From: qianggui.song <Qianggui.Song@amlogic.com> To: Marc Zyngier <maz@kernel.org> Cc: Thomas Gleixner <tglx@linutronix.de>, Kevin Hilman <khilman@baylibre.com>, Neil Armstrong <narmstrong@baylibre.com>, Jerome Brunet <jbrunet@baylibre.com>, Martin Blumenstingl <martin.blumenstingl@googlemail.com>, <linux-kernel@vger.kernel.org>, <linux-arm-kernel@lists.infradead.org>, <linux-amlogic@lists.infradead.org> Subject: Re: [PATCH 2/4] irqchip/meson-gpio: support more than 8 channels gpio irq line Date: Mon, 10 Jan 2022 20:27:03 +0800 [thread overview] Message-ID: <3235dfc6-413c-7582-8204-2ac8ad0952d4@amlogic.com> (raw) In-Reply-To: <87tueetsvz.wl-maz@kernel.org> On 1/8/22 6:37 PM, Marc Zyngier wrote: > > On Sat, 08 Jan 2022 08:42:16 +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..6a7b4fb13452 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 channel_num; > > For consistency, please name this nr_channels. Okay > >> 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, \ >> + .channel_num = 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, \ >> + .channel_num = 8, \ >> >> static const struct meson_gpio_irq_params meson8_params = { >> INIT_MESON8_COMMON_DATA(134) >> @@ -136,8 +138,9 @@ 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; >> + u8 channel_num; > > Same thing. Though this is completely superfluous, see below. I will remove this variable > >> spinlock_t lock; >> }; >> >> @@ -207,8 +210,8 @@ meson_gpio_irq_request_channel(struct meson_gpio_irq_controller *ctl, >> spin_lock_irqsave(&ctl->lock, flags); >> >> /* Find a free channel */ >> - idx = find_first_zero_bit(ctl->channel_map, NUM_CHANNEL); >> - if (idx >= NUM_CHANNEL) { >> + idx = find_first_zero_bit(ctl->channel_map, ctl->channel_num); >> + if (idx >= ctl->channel_num) { >> spin_unlock_irqrestore(&ctl->lock, flags); >> pr_err("No channel available\n"); >> return -ENOSPC; >> @@ -447,13 +450,25 @@ static int meson_gpio_irq_parse_dt(struct device_node *node, struct meson_gpio_i >> >> ctl->params = match->data; >> >> + ctl->channel_num = ctl->params->channel_num; > > Since you already have a pointer to params, why do you need to > duplicate this information? will remove it next patch > >> + ctl->channel_irqs = kcalloc(ctl->channel_num, >> + sizeof(*ctl->channel_irqs), GFP_KERNEL); >> + if (!ctl->channel_irqs) >> + return -ENOMEM; >> + >> + ctl->channel_map = bitmap_zalloc(ctl->params->channel_num, GFP_KERNEL); >> + if (!ctl->channel_map) { >> + kfree(ctl->channel_irqs); >> + return -ENOMEM; >> + } >> + >> ret = of_property_read_variable_u32_array(node, >> "amlogic,channel-interrupts", >> ctl->channel_irqs, >> - NUM_CHANNEL, >> - NUM_CHANNEL); >> + ctl->channel_num, >> + ctl->channel_num); >> if (ret < 0) { >> - pr_err("can't get %d channel interrupts\n", NUM_CHANNEL); >> + pr_err("can't get %d channel interrupts\n", ctl->channel_num); >> return ret; > > You are now leaking the bitmap and channel_map allocations. will fix it next patch > > M. > _______________________________________________ linux-amlogic mailing list linux-amlogic@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-amlogic
WARNING: multiple messages have this Message-ID (diff)
From: qianggui.song <Qianggui.Song@amlogic.com> To: Marc Zyngier <maz@kernel.org> Cc: Thomas Gleixner <tglx@linutronix.de>, Kevin Hilman <khilman@baylibre.com>, Neil Armstrong <narmstrong@baylibre.com>, Jerome Brunet <jbrunet@baylibre.com>, Martin Blumenstingl <martin.blumenstingl@googlemail.com>, <linux-kernel@vger.kernel.org>, <linux-arm-kernel@lists.infradead.org>, <linux-amlogic@lists.infradead.org> Subject: Re: [PATCH 2/4] irqchip/meson-gpio: support more than 8 channels gpio irq line Date: Mon, 10 Jan 2022 20:27:03 +0800 [thread overview] Message-ID: <3235dfc6-413c-7582-8204-2ac8ad0952d4@amlogic.com> (raw) In-Reply-To: <87tueetsvz.wl-maz@kernel.org> On 1/8/22 6:37 PM, Marc Zyngier wrote: > > On Sat, 08 Jan 2022 08:42:16 +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..6a7b4fb13452 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 channel_num; > > For consistency, please name this nr_channels. Okay > >> 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, \ >> + .channel_num = 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, \ >> + .channel_num = 8, \ >> >> static const struct meson_gpio_irq_params meson8_params = { >> INIT_MESON8_COMMON_DATA(134) >> @@ -136,8 +138,9 @@ 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; >> + u8 channel_num; > > Same thing. Though this is completely superfluous, see below. I will remove this variable > >> spinlock_t lock; >> }; >> >> @@ -207,8 +210,8 @@ meson_gpio_irq_request_channel(struct meson_gpio_irq_controller *ctl, >> spin_lock_irqsave(&ctl->lock, flags); >> >> /* Find a free channel */ >> - idx = find_first_zero_bit(ctl->channel_map, NUM_CHANNEL); >> - if (idx >= NUM_CHANNEL) { >> + idx = find_first_zero_bit(ctl->channel_map, ctl->channel_num); >> + if (idx >= ctl->channel_num) { >> spin_unlock_irqrestore(&ctl->lock, flags); >> pr_err("No channel available\n"); >> return -ENOSPC; >> @@ -447,13 +450,25 @@ static int meson_gpio_irq_parse_dt(struct device_node *node, struct meson_gpio_i >> >> ctl->params = match->data; >> >> + ctl->channel_num = ctl->params->channel_num; > > Since you already have a pointer to params, why do you need to > duplicate this information? will remove it next patch > >> + ctl->channel_irqs = kcalloc(ctl->channel_num, >> + sizeof(*ctl->channel_irqs), GFP_KERNEL); >> + if (!ctl->channel_irqs) >> + return -ENOMEM; >> + >> + ctl->channel_map = bitmap_zalloc(ctl->params->channel_num, GFP_KERNEL); >> + if (!ctl->channel_map) { >> + kfree(ctl->channel_irqs); >> + return -ENOMEM; >> + } >> + >> ret = of_property_read_variable_u32_array(node, >> "amlogic,channel-interrupts", >> ctl->channel_irqs, >> - NUM_CHANNEL, >> - NUM_CHANNEL); >> + ctl->channel_num, >> + ctl->channel_num); >> if (ret < 0) { >> - pr_err("can't get %d channel interrupts\n", NUM_CHANNEL); >> + pr_err("can't get %d channel interrupts\n", ctl->channel_num); >> return ret; > > You are now leaking the bitmap and channel_map allocations. will fix it next patch > > M. > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-01-10 12:26 UTC|newest] Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-01-08 8:42 [PATCH 0/4] irqchip/meson-gpio: Add support for Meson-S4 SoC Qianggui Song 2022-01-08 8:42 ` Qianggui Song 2022-01-08 8:42 ` Qianggui Song 2022-01-08 8:42 ` [PATCH 1/4] dt-bindings: interrupt-controller: New binding for Meson-S4 SoCs Qianggui Song 2022-01-08 8:42 ` Qianggui Song 2022-01-08 8:42 ` Qianggui Song 2022-01-09 15:27 ` Christian Hewitt 2022-01-09 15:27 ` Christian Hewitt 2022-01-09 15:27 ` Christian Hewitt 2022-01-10 12:33 ` qianggui.song 2022-01-10 12:33 ` qianggui.song 2022-01-08 8:42 ` [PATCH 2/4] irqchip/meson-gpio: support more than 8 channels gpio irq line Qianggui Song 2022-01-08 8:42 ` Qianggui Song 2022-01-08 8:42 ` Qianggui Song 2022-01-08 10:37 ` Marc Zyngier 2022-01-08 10:37 ` Marc Zyngier 2022-01-08 10:37 ` Marc Zyngier 2022-01-10 12:27 ` qianggui.song [this message] 2022-01-10 12:27 ` qianggui.song 2022-01-08 8:42 ` [PATCH 3/4] irqchip/meson-gpio: add select trigger type callback Qianggui Song 2022-01-08 8:42 ` Qianggui Song 2022-01-08 8:42 ` Qianggui Song 2022-01-08 10:44 ` Marc Zyngier 2022-01-08 10:44 ` Marc Zyngier 2022-01-08 10:44 ` Marc Zyngier 2022-01-10 12:28 ` qianggui.song 2022-01-10 12:28 ` qianggui.song 2022-01-08 8:42 ` [PATCH 4/4] irqchip/meson-gpio: Add support for meson s4 SoCs Qianggui Song 2022-01-08 8:42 ` Qianggui Song 2022-01-08 8:42 ` Qianggui Song 2022-01-08 11:06 ` Marc Zyngier 2022-01-08 11:06 ` Marc Zyngier 2022-01-08 11:06 ` Marc Zyngier 2022-01-10 12:32 ` qianggui.song 2022-01-10 12:32 ` qianggui.song 2022-01-10 14:54 ` Marc Zyngier 2022-01-10 14:54 ` Marc Zyngier
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=3235dfc6-413c-7582-8204-2ac8ad0952d4@amlogic.com \ --to=qianggui.song@amlogic.com \ --cc=jbrunet@baylibre.com \ --cc=khilman@baylibre.com \ --cc=linux-amlogic@lists.infradead.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=martin.blumenstingl@googlemail.com \ --cc=maz@kernel.org \ --cc=narmstrong@baylibre.com \ --cc=tglx@linutronix.de \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.