| Message ID | 20250103083520.1764441-1-a909204013@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drivers:gpio: support multiple virtio-gpio controller instances | expand |
On 03-01-25, 16:35, hlleng wrote: > Modify the virtio-gpio driver to support multiple virtual GPIO controller > instances. The previous static global irq_chip structure caused conflicts > between multiple virtio-gpio device instances as they shared the same > interrupt controller configuration. What is the conflict you are getting since all it has is callbacks only, I wonder why do we need to duplicate it.
On 06-01-25, 10:05, Viresh Kumar wrote: > On 03-01-25, 16:35, hlleng wrote: > > Modify the virtio-gpio driver to support multiple virtual GPIO controller > > instances. The previous static global irq_chip structure caused conflicts > > between multiple virtio-gpio device instances as they shared the same > > interrupt controller configuration. > > What is the conflict you are getting since all it has is callbacks only, I > wonder why do we need to duplicate it. Ahh, so irq chip should be shared actually.. static void gpiochip_set_irq_hooks(struct gpio_chip *gc) { ... /* Check if the irqchip already has this hook... */ if (irqchip->irq_enable == gpiochip_irq_enable || irqchip->irq_mask == gpiochip_irq_mask) { /* * ...and if so, give a gentle warning that this is bad * practice. */ chip_info(gc, "detected irqchip that is shared with multiple gpiochips: please fix the driver.\n"); return; } ... } For your patch: Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
ping Viresh Kumar <viresh.kumar@linaro.org> 于2025年1月7日周二 14:00写道： > > On 06-01-25, 10:05, Viresh Kumar wrote: > On 03-01-25, 16:35, hlleng wrote: > > Modify the virtio-gpio driver to support multiple virtual GPIO controller > > instances. The previous static global irq_chip structure caused conflicts > > between multiple virtio-gpio device instances as they shared the same > > interrupt controller configuration. > > What is the conflict you are getting since all it has is callbacks only, I > wonder why do we need to duplicate it. Ahh, so irq chip should be shared actually.. static void gpiochip_set_irq_hooks(struct gpio_chip *gc) { ... /* Check if the irqchip already has this hook... */ if (irqchip->irq_enable == gpiochip_irq_enable || irqchip->irq_mask == gpiochip_irq_mask) { /* * ...and if so, give a gentle warning that this is bad * practice. */ chip_info(gc, "detected irqchip that is shared with multiple gpiochips: please fix the driver.\n"); return; } ... } For your patch: Acked-by: Viresh Kumar <viresh.kumar@linaro.org> -- viresh
+GPIO maintainers. On 08-02-25, 15:54, Leng Cold wrote: > ping > > Viresh Kumar <viresh.kumar@linaro.org> 于2025年1月7日周二 14:00写道： > > > On 06-01-25, 10:05, Viresh Kumar wrote: > > > On 03-01-25, 16:35, hlleng wrote: > > > > Modify the virtio-gpio driver to support multiple virtual GPIO > > controller > > > > instances. The previous static global irq_chip structure caused > > conflicts > > > > between multiple virtio-gpio device instances as they shared the same > > > > interrupt controller configuration. > > > > > > What is the conflict you are getting since all it has is callbacks only, > > I > > > wonder why do we need to duplicate it. > > > > Ahh, so irq chip should be shared actually.. > > > > static void gpiochip_set_irq_hooks(struct gpio_chip *gc) > > { > > ... > > > > /* Check if the irqchip already has this hook... */ > > if (irqchip->irq_enable == gpiochip_irq_enable || > > irqchip->irq_mask == gpiochip_irq_mask) { > > /* > > * ...and if so, give a gentle warning that this is bad > > * practice. > > */ > > chip_info(gc, > > "detected irqchip that is shared with multiple > > gpiochips: please fix the driver.\n"); > > return; > > } > > > > ... > > } > > > > For your patch: > > > > Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
On Mon, Feb 10, 2025 at 5:56 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > +GPIO maintainers. > > On 08-02-25, 15:54, Leng Cold wrote: > > ping > > > > Viresh Kumar <viresh.kumar@linaro.org> 于2025年1月7日周二 14:00写道： > > > > > On 06-01-25, 10:05, Viresh Kumar wrote: > > > > On 03-01-25, 16:35, hlleng wrote: > > > > > Modify the virtio-gpio driver to support multiple virtual GPIO > > > controller > > > > > instances. The previous static global irq_chip structure caused > > > conflicts > > > > > between multiple virtio-gpio device instances as they shared the same > > > > > interrupt controller configuration. > > > > > > > > What is the conflict you are getting since all it has is callbacks only, > > > I > > > > wonder why do we need to duplicate it. > > > > > > Ahh, so irq chip should be shared actually.. > > > > > > static void gpiochip_set_irq_hooks(struct gpio_chip *gc) > > > { > > > ... > > > > > > /* Check if the irqchip already has this hook... */ > > > if (irqchip->irq_enable == gpiochip_irq_enable || > > > irqchip->irq_mask == gpiochip_irq_mask) { > > > /* > > > * ...and if so, give a gentle warning that this is bad > > > * practice. > > > */ > > > chip_info(gc, > > > "detected irqchip that is shared with multiple > > > gpiochips: please fix the driver.\n"); > > > return; > > > } > > > > > > ... > > > } > > > > > > For your patch: > > > > > > Acked-by: Viresh Kumar <viresh.kumar@linaro.org> > > -- > viresh The author would have had more success getting our attention if they used get_maintainer.pl. Also: the subject is wrong, should be "gpio: virtio: ...". Please resend a proper version with Viresh' tag collected. Bart
diff --git a/drivers/gpio/gpio-virtio.c b/drivers/gpio/gpio-virtio.c index 93544ff62513..ac39da17a29b 100644 --- a/drivers/gpio/gpio-virtio.c +++ b/drivers/gpio/gpio-virtio.c @@ -350,19 +350,6 @@ static void virtio_gpio_irq_bus_sync_unlock(struct irq_data *d) mutex_unlock(&vgpio->irq_lock); } -static struct irq_chip vgpio_irq_chip = { - .name = "virtio-gpio", - .irq_enable = virtio_gpio_irq_enable, - .irq_disable = virtio_gpio_irq_disable, - .irq_mask = virtio_gpio_irq_mask, - .irq_unmask = virtio_gpio_irq_unmask, - .irq_set_type = virtio_gpio_irq_set_type, - - /* These are required to implement irqchip for slow busses */ - .irq_bus_lock = virtio_gpio_irq_bus_lock, - .irq_bus_sync_unlock = virtio_gpio_irq_bus_sync_unlock, -}; - static bool ignore_irq(struct virtio_gpio *vgpio, int gpio, struct vgpio_irq_line *irq_line) { @@ -542,6 +529,7 @@ static int virtio_gpio_probe(struct virtio_device *vdev) struct virtio_gpio_config config; struct device *dev = &vdev->dev; struct virtio_gpio *vgpio; + struct irq_chip *gpio_irq_chip; u32 gpio_names_size; u16 ngpio; int ret, i; @@ -591,13 +579,26 @@ static int virtio_gpio_probe(struct virtio_device *vdev) if (!vgpio->irq_lines) return -ENOMEM; + gpio_irq_chip = devm_kzalloc(dev, sizeof(*gpio_irq_chip), GFP_KERNEL); + if (!gpio_irq_chip) + return -ENOMEM; + + gpio_irq_chip->name = dev_name(dev); + gpio_irq_chip->irq_enable = virtio_gpio_irq_enable; + gpio_irq_chip->irq_disable = virtio_gpio_irq_disable; + gpio_irq_chip->irq_mask = virtio_gpio_irq_mask; + gpio_irq_chip->irq_unmask = virtio_gpio_irq_unmask; + gpio_irq_chip->irq_set_type = virtio_gpio_irq_set_type; + gpio_irq_chip->irq_bus_lock = virtio_gpio_irq_bus_lock; + gpio_irq_chip->irq_bus_sync_unlock = virtio_gpio_irq_bus_sync_unlock; + /* The event comes from the outside so no parent handler */ vgpio->gc.irq.parent_handler = NULL; vgpio->gc.irq.num_parents = 0; vgpio->gc.irq.parents = NULL; vgpio->gc.irq.default_type = IRQ_TYPE_NONE; vgpio->gc.irq.handler = handle_level_irq; - vgpio->gc.irq.chip = &vgpio_irq_chip; + vgpio->gc.irq.chip = gpio_irq_chip; for (i = 0; i < ngpio; i++) { vgpio->irq_lines[i].type = VIRTIO_GPIO_IRQ_TYPE_NONE;
Modify the virtio-gpio driver to support multiple virtual GPIO controller instances. The previous static global irq_chip structure caused conflicts between multiple virtio-gpio device instances as they shared the same interrupt controller configuration. Fix this by: 1. Remove the static global vgpio_irq_chip structure 2. Dynamically allocate a dedicated irq_chip for each virtio-gpio instance 3. Use device-specific names for each instance's irq_chip Signed-off-by: hlleng <a909204013@gmail.com> --- drivers/gpio/gpio-virtio.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-)