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

Shenwei Wang
2025-11-06T17:00:36Z

> -----Original Message-----
> From: Zhongqiu Han <zhongqiu.han@oss.qualcomm.com>
> Sent: Thursday, November 6, 2025 6:31 AM
> To: Shenwei Wang <shenwei.wang@nxp.com>; Bjorn Andersson
> <andersson@kernel.org>; Mathieu Poirier <mathieu.poirier@linaro.org>; Rob
> Herring <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor
> Dooley <conor+dt@kernel.org>; Shawn Guo <shawnguo@kernel.org>; Sascha
> Hauer <s.hauer@pengutronix.de>; Jonathan Corbet <corbet@lwn.net>; Linus
> Walleij <linus.walleij@linaro.org>; Bartosz Golaszewski <brgl@bgdev.pl>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam
> <festevam@gmail.com>; Peng Fan <peng.fan@nxp.com>; linux-
> remoteproc@vger.kernel.org; devicetree@vger.kernel.org; imx@lists.linux.dev;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
> doc@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Andrew Lunn
> <andrew@lunn.ch>; zhongqiu.han@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@nxp.com>");
> > +MODULE_DESCRIPTION("NXP i.MX SoC rpmsg gpio driver");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.43.0
> >
> >
>
>
> --
> Thx and BRs,
> Zhongqiu Han