> -----Original Message-----
> From: Peng Fan (OSS) <[email protected]>
> Sent: Sunday, September 28, 2025 10:37 PM
> To: Shenwei Wang <[email protected]>
> Cc: 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]>; Linus
> Walleij <[email protected]>; Bartosz Golaszewski <[email protected]>;
> Pengutronix Kernel Team <[email protected]>; Fabio Estevam
> <[email protected]>; Peng Fan <[email protected]>; linux-
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> dl-linux-imx
> >+ if (!port)
> >+ return -ENODEV;
> >+
> >+ if (msg->header.type == GPIO_RPMSG_REPLY) {
> >+ port->info.reply_msg = msg;
> >+ complete(&port->info.cmd_complete);
> >+ } else if (msg->header.type == GPIO_RPMSG_NOTIFY) {
> >+ port->info.notify_msg = msg;
> >+ local_irq_save(flags);
>
> Would you explain a bit on why need to disable IRQ on current core.
>
The generic_handle_domain_irq is required to be called in an IRQ context.
Seems it can
be replaced with the generic_handle_domain_irq_safe here.
Thanks,
Shenwei
> >+ generic_handle_domain_irq(port->gc.irq.domain, msg->pin_idx);
> >+ local_irq_restore(flags);
> >+ } else
> >+ dev_err(&rpdev->dev, "wrong command type!\n");
> >+
> >+ return 0;
> >+}
> >+
> >+static void imx_rpmsg_gpio_remove_action(void *data) {
> >+ struct imx_rpmsg_gpio_port *port = data;
> >+
> >+ port->info.port_store[port->idx] = 0; }
> >+
> >+static int imx_rpmsg_gpio_probe(struct platform_device *pdev) {
> >+ struct imx_rpmsg_driver_data *pltdata = pdev->dev.platform_data;
> >+ struct device_node *np = pdev->dev.of_node;
> >+ struct imx_rpmsg_gpio_port *port;
> >+ struct gpio_irq_chip *girq;
> >+ struct gpio_chip *gc;
> >+ int ret;
> >+
> >+ if (!pltdata)
> >+ return -EPROBE_DEFER;
> >+
> >+ port = devm_kzalloc(&pdev->dev, sizeof(*port), GFP_KERNEL);
> >+ if (!port)
> >+ return -ENOMEM;
> >+
> >+ ret = of_property_read_u32(np, "reg", &port->idx);
>
> "device_property_read_u32" should be better.
>
> >+ if (ret)
> >+ return ret;
> >+
> >+ if (port->idx > MAX_DEV_PER_CHANNEL)
> >+ return -EINVAL;
> >+
> >+ mutex_init(&port->info.lock);
> >+ init_completion(&port->info.cmd_complete);
> >+ port->info.rpdev = pltdata->rpdev;
> >+ port->info.port_store = pltdata->channel_devices;
> >+ port->info.port_store[port->idx] = port;
> >+ if (!pltdata->rx_callback)
> >+ pltdata->rx_callback = imx_rpmsg_gpio_callback;
> >+
> >+ gc = &port->gc;
> >+ gc->owner = THIS_MODULE;
> >+ gc->parent = &pltdata->rpdev->dev;
> >+ gc->label = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s-gpio%d",
> >+ pltdata->rproc_name, port->idx);
> >+ gc->ngpio = IMX_RPMSG_GPIO_PER_PORT;
> >+ gc->base = -1;
> >+
> >+ gc->direction_input = imx_rpmsg_gpio_direction_input;
> >+ gc->direction_output = imx_rpmsg_gpio_direction_output;
> >+ gc->get = imx_rpmsg_gpio_get;
> >+ gc->set = imx_rpmsg_gpio_set;
> >+
> >+ platform_set_drvdata(pdev, port);
> >+ girq = &gc->irq;
> >+ gpio_irq_chip_set_chip(girq, &imx_rpmsg_irq_chip);
> >+ girq->parent_handler = NULL;
> >+ girq->num_parents = 0;
> >+ girq->parents = NULL;
> >+ girq->chip->name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s-
> gpio%d",
> >+ pltdata->rproc_name, port->idx);
>
> Align pltdata->rproc_name with the upper line '('.
>
> >+
> >+ devm_add_action_or_reset(&pdev->dev,
> imx_rpmsg_gpio_remove_action,
> >+port);
>
> return value should be checked.
>
> >+
> >+ return devm_gpiochip_add_data(&pdev->dev, gc, port); }
> >+
>
> Thanks,
> Peng
