linux-kernel - RE: [PATCH v2 3/4] gpio: imx-rpmsg: add imx-rpmsg GPIO driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID:
 <PAXPR04MB9185CD0AC1396865AE82BF77891BA@PAXPR04MB9185.eurprd04.prod.outlook.com>
Date: Mon, 29 Sep 2025 14:59:56 +0000
From: Shenwei Wang <shenwei.wang@....com>
To: "Peng Fan (OSS)" <peng.fan@....nxp.com>
CC: Bjorn Andersson <andersson@...nel.org>, Mathieu Poirier
	<mathieu.poirier@...aro.org>, Rob Herring <robh@...nel.org>, Krzysztof
 Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Shawn Guo
	<shawnguo@...nel.org>, Sascha Hauer <s.hauer@...gutronix.de>, Linus Walleij
	<linus.walleij@...aro.org>, Bartosz Golaszewski <brgl@...ev.pl>, Pengutronix
 Kernel Team <kernel@...gutronix.de>, Fabio Estevam <festevam@...il.com>, Peng
 Fan <peng.fan@....com>, "linux-remoteproc@...r.kernel.org"
	<linux-remoteproc@...r.kernel.org>, "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>, "imx@...ts.linux.dev" <imx@...ts.linux.dev>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, dl-linux-imx <linux-imx@....com>
Subject: RE: [PATCH v2 3/4] gpio: imx-rpmsg: add imx-rpmsg GPIO driver



> -----Original Message-----
> From: Peng Fan (OSS) <peng.fan@....nxp.com>
> Sent: Sunday, September 28, 2025 10:37 PM
> To: Shenwei Wang <shenwei.wang@....com>
> Cc: Bjorn Andersson <andersson@...nel.org>; Mathieu Poirier
> <mathieu.poirier@...aro.org>; Rob Herring <robh@...nel.org>; Krzysztof
> Kozlowski <krzk+dt@...nel.org>; Conor Dooley <conor+dt@...nel.org>; Shawn
> Guo <shawnguo@...nel.org>; Sascha Hauer <s.hauer@...gutronix.de>; Linus
> Walleij <linus.walleij@...aro.org>; Bartosz Golaszewski <brgl@...ev.pl>;
> Pengutronix Kernel Team <kernel@...gutronix.de>; Fabio Estevam
> <festevam@...il.com>; Peng Fan <peng.fan@....com>; linux-
> remoteproc@...r.kernel.org; devicetree@...r.kernel.org; imx@...ts.linux.dev;
> linux-arm-kernel@...ts.infradead.org; linux-kernel@...r.kernel.org; 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