> -----Original Message-----
> From: Zhongqiu Han <[email protected]>
> Sent: Thursday, November 6, 2025 6:31 AM
> To: Shenwei Wang <[email protected]>; Bjorn Andersson
> <[email protected]>; Mathieu Poirier <[email protected]>; Rob
> Herring <[email protected]>; Krzysztof Kozlowski <[email protected]>; Conor
> Dooley <[email protected]>; Shawn Guo <[email protected]>; Sascha
> Hauer <[email protected]>; Jonathan Corbet <[email protected]>; Linus
> Walleij <[email protected]>; Bartosz Golaszewski <[email protected]>
> Cc: Pengutronix Kernel Team <[email protected]>; Fabio Estevam
> <[email protected]>; Peng Fan <[email protected]>; linux-
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; dl-linux-imx <[email protected]>; Andrew Lunn
> <[email protected]>; [email protected]
> 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 <[email protected]>");
> > +MODULE_DESCRIPTION("NXP i.MX SoC rpmsg gpio driver");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.43.0
> >
> >
>
>
> --
> Thx and BRs,
> Zhongqiu Han
