Messages in this thread |  | | From | Shenwei Wang <> | | Subject | Re: [PATCH v13 3/4] gpio: rpmsg: add generic rpmsg GPIO driver | | Date | Thu, 7 May 2026 19:43:33 +0000 |
| |
> -----Original Message----- > From: Mathieu Poirier <mathieu.poirier@linaro.org> > Sent: Thursday, May 7, 2026 12:13 PM > To: Arnaud POULIQUEN <arnaud.pouliquen@foss.st.com> > Cc: Beleswar Prasad Padhi <b-padhi@ti.com>; Shenwei Wang > <shenwei.wang@nxp.com>; Andrew Lunn <andrew@lunn.ch>; Linus Walleij > <linusw@kernel.org>; Bartosz Golaszewski <brgl@kernel.org>; Jonathan Corbet > <corbet@lwn.net>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski > <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Bjorn Andersson > <andersson@kernel.org>; Frank Li <frank.li@nxp.com>; Sascha Hauer > <s.hauer@pengutronix.de>; Shuah Khan <skhan@linuxfoundation.org>; linux- > gpio@vger.kernel.org; linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org; > Pengutronix Kernel Team <kernel@pengutronix.de>; Fabio Estevam > <festevam@gmail.com>; Peng Fan <peng.fan@nxp.com>; > devicetree@vger.kernel.org; linux-remoteproc@vger.kernel.org; > imx@lists.linux.dev; linux-arm-kernel@lists.infradead.org; dl-linux-imx <linux- > imx@nxp.com>; Bartosz Golaszewski <brgl@bgdev.pl> > Subject: [EXT] Re: [PATCH v13 3/4] gpio: rpmsg: add generic rpmsg GPIO driver > > > > From my perspective, based on your proposal: > > > > 1) Linux should send a get_config message to the remote proc (0x405 -> > 0xD). 2) The remote processor would respond with the list of ports, associated > > > > with an remote endpoint addresses. > > > > > > > > > Agreed, we can scale it for multiple remote endpoints like this. > > > > > > > 3) Linux would parse the response, compare it with the DT, enable the > GPIO > > > > ports accordingly, creating it local endpoint and associating it with > > > > the remote endpoint. > > > > Using name service to identify the ports should avoid step 1 & 2 ... > > > > > > > > > Yes, but won't that make a lot of hard-codings in the driver? > > > > > > +static struct rpmsg_device_id rpmsg_gpio_channel_id_table[] = { > > > + { .name = "rpmsg-io-25" }, > > > + { .name = "rpmsg-io-32" }, > > > + { .name = "rpmsg-io-35" }, > > > + { }, > > > +}; > > > > > > What if tomorrow another vendor decides to add more remoteproc > > > controlled GPIO ports to Linux, they would have to update this > > > struct in the driver everytime. And the port indexes (25/32/35) > > > could also differ between vendors. We should make the driver dynamic > > > i.e. vendor agnostic. > > > > > > I think querying the remote firmware at runtime (step 1 & 2 above) > > > is a common design pattern and makes the driver vendor agnostic. But > > > feel free to correct me. > > > > > > > You are right. My proposal would require a patch in rpmsg-core. The > > idea of allowing a postfix in the compatible string has been discussed > > before, but, if I remember correctly, it was not concluded. > > > > I also remember discussing this. I even reviewed one of Arnaud's patch and > submitted one myself. This must have been in 2020 and the reason why it wasn't > merged has escaped my memory. > > > /* rpmsg devices and drivers are matched using the service name */ > > static inline int rpmsg_id_match(const struct rpmsg_device *rpdev, > > const struct rpmsg_device_id *id) { > > size_t len; > > > > + len = strnlen(id->name, RPMSG_NAME_SIZE); > > + if (len && id->name[len - 1] == '*') > > + return !strncmp(id->name, rpdev->id.name, len - 1); > > > > return strncmp(id->name, rpdev->id.name, RPMSG_NAME_SIZE) == 0; > > } > > > > Then, in rpmsg-gpio, and possibly in other drivers such as rpmsg-tty > > and a future rpmsg-i2c, we could use: > > static struct rpmsg_device_id rpmsg_gpio_channel_id_table[] = { > > { .name = "rpmsg-io" }, > > { .name = "rpmsg-io-*" }, > > { }, > > }; > > That was my initial approach. We don't even need an additional "rpmsg-io-*" in > rpmsg_gpio_channel_id_table[]. All we need is: > > /* rpmsg devices and drivers are matched using the service name */ static inline > int rpmsg_id_match(const struct rpmsg_device *rpdev, > const struct rpmsg_device_id *id) { > + size_t len = strnlen(id->name, RPMSG_NAME_SIZE); > > - return strncmp(id->name, rpdev->id.name, RPMSG_NAME_SIZE) == 0; > + return strncmp(id->name, rpdev->id.name, len) == 0; > } >
If we encode the port index directly into ept->src, for example:
ept->src = (baseaddr << 8) | port_index;
where baseaddr can be derived from the channel address, we can avoid the possible address conflict.
With this approach, the patch to rpmsg-core would no longer be necessary.
Thanks, Shenwei
> And let the rpmsg-virtio-gpio driver parse @rpdev->id.name to match with a > GPIO controller in the DT. > > > > > If exact name matching is strongly required, then this proposal would > > not be suitablea. > > > > A third option would be a combination of both approaches: instantiate > > the device using the same name service from the remote side, as done > > in rpmsg-tty. In that case, a get_config message, or a similar > > mechanism, would also be needed to retrieve the port information from the > remote side. > > > > I'm not overly fond of a get_config message because it is one more thing we have > to define and maintain. > > Arnaud: is there a get_config message already defined for rpmsg_tty? > > Beleswar: Can you provide a link to a virtio device that would use a get_config > message? > > > Tanmaya also proposed another alternative based on reserved addresses. > > > > At this point, I suggest letting Mathieu review the discussion and > > recommend the most suitable approach. > > > > Thanks, > > Arnaud > > > > > > > > > > At the end, whatever solution is implemented, my main concern is > > > > that the Linux driver design should, if possible, avoid adding > > > > unnecessary complexity or limitations on the remote side (for instance in > openAMP project). > > > > > > > > > Yes definitely, I want the same. Feel free to let me know if this > > > does not suit with the OpenAMP project. > > > > > > Thanks, > > > Beleswar > > > > > > > > > > > Thanks, > > > > Arnaud > > > > > > > > > > > > > So Linux does not need to send the port idx everytime while > > > > > sending a gpio message anymore. > > > > > > > > > > Thanks, > > > > > Beleswar > > > > > > > > > > [...] > > > > > > > > > > >
|  |