[PATCH v5 4/5] gpio: imx-rpmsg: add imx-rpmsg GPIO driver

[Shenwei Wang]

> -----Original Message-----
> From: Zhongqiu Han <zhongqiu.han at oss.qualcomm.com>
> Sent: Thursday, November 6, 2025 6:31 AM
> To: Shenwei Wang <shenwei.wang at nxp.com>; Bjorn Andersson
> <andersson at kernel.org>; Mathieu Poirier <mathieu.poirier at linaro.org>; Rob
> Herring <robh at kernel.org>; Krzysztof Kozlowski <krzk+dt at kernel.org>; Conor
> Dooley <conor+dt at kernel.org>; Shawn Guo <shawnguo at kernel.org>; Sascha
> Hauer <s.hauer at pengutronix.de>; Jonathan Corbet <corbet at lwn.net>; Linus
> Walleij <linus.walleij at linaro.org>; Bartosz Golaszewski <brgl at bgdev.pl>
> Cc: Pengutronix Kernel Team <kernel at pengutronix.de>; Fabio Estevam
> <festevam at gmail.com>; Peng Fan <peng.fan at nxp.com>; linux-
> remoteproc at vger.kernel.org; devicetree at vger.kernel.org; imx at lists.linux.dev;
> linux-arm-kernel at lists.infradead.org; linux-kernel at vger.kernel.org; linux-
> doc at vger.kernel.org; dl-linux-imx <linux-imx at nxp.com>; Andrew Lunn
> <andrew at lunn.ch>; zhongqiu.han at oss.qualcomm.com
> Subject: [EXT] Re: [PATCH v5 4/5] gpio: imx-rpmsg: add imx-rpmsg GPIO driver
> On 11/5/2025 4:33 AM, Shenwei Wang wrote:
> > On i.MX SoCs, the system may include two processors:
> >       - An MCU running an RTOS
> >       - An MPU running Linux
> >
> > +
> > +struct imx_rpmsg_gpio_port {
> > +     struct gpio_chip gc;
> > +     struct imx_rpmsg_gpio_pin gpio_pins[IMX_RPMSG_GPIO_PER_PORT];
> > +     struct imx_gpio_rpmsg_info info;
> > +     int idx;
> > +};
> > +
> 
> Hello Shenwei,
> I'd like to go over a few aspects of this patch.
> 

Hi Zhongqiu,

Thank you for the review. I'll address the two issues you reported below in the next revision.

Thanks,
Shenwei

> 
> > +static int gpio_send_message(struct imx_rpmsg_gpio_port *port,
> > +                          struct gpio_rpmsg_data *msg,
> > +                          bool sync)
> > +{
> > +     struct imx_gpio_rpmsg_info *info = &port->info;
> > +     int err;
> > +
> > +     if (!info->rpdev) {
> > +             dev_dbg(&info->rpdev->dev,
> 
> 1.NULL pointer dereference here.
> 
> > +                     "rpmsg channel doesn't exist, is remote core ready?\n");
> > +             return -EINVAL;
> > +     }
> > +
> > +     reinit_completion(&info->cmd_complete);
> > +     err = rpmsg_send(info->rpdev->ept, (void *)msg,
> > +                      sizeof(struct gpio_rpmsg_data));
> > +static void imx_rpmsg_irq_bus_sync_unlock(struct irq_data *d) {
> > +     struct imx_rpmsg_gpio_port *port = irq_data_get_irq_chip_data(d);
> > +     struct gpio_rpmsg_data *msg = NULL;
> > +     u32 gpio_idx = d->hwirq;
> > +
> > +     if (port == NULL) {
> > +             mutex_unlock(&port->info.lock);
> > +             return;
> > +     }
> > +
> 
> 2.Unlocking port->info.lock when port is NULL will crash due to a NULL
> pointer dereference. Please fix the logic as well.> +   /*
> > +      * For mask irq, do nothing here.
> > +      * M core will mask interrupt after a interrupt occurred, and then
> > +      * sends a notify to A core.
> > +      * After A core dealt with the notify, A core will send a rpmsg to
> > +      * M core to unmask this interrupt again.
> > +      */
> > +
> > +     if (port->gpio_pins[gpio_idx].irq_mask && !port-
> >gpio_pins[gpio_idx].irq_unmask) {
> > +             port->gpio_pins[gpio_idx].irq_mask = 0;
> > +             mutex_unlock(&port->info.lock);
> > +             return;
> > +     }
> > +
> > +
> > +MODULE_AUTHOR("Shenwei Wang <shenwei.wang at nxp.com>");
> > +MODULE_DESCRIPTION("NXP i.MX SoC rpmsg gpio driver");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.43.0
> >
> >
> 
> 
> --
> Thx and BRs,
> Zhongqiu Han