Re: [PATCH v12 3/5] gpio: rpmsg: add generic rpmsg GPIO driver

[Shenwei Wang]

> -----Original Message-----
> From: Andrew Lunn <andrew@xxxxxxx>
> Sent: Tuesday, March 17, 2026 9:22 AM
> To: Arnaud POULIQUEN <arnaud.pouliquen@xxxxxxxxxxx>
> Cc: Shenwei Wang <shenwei.wang@xxxxxxx>; Linus Walleij
> <linusw@xxxxxxxxxx>; Bartosz Golaszewski <brgl@xxxxxxxxxx>; Jonathan Corbet
> <corbet@xxxxxxx>; Rob Herring <robh@xxxxxxxxxx>; Krzysztof Kozlowski
> <krzk+dt@xxxxxxxxxx>; Conor Dooley <conor+dt@xxxxxxxxxx>; Bjorn Andersson
> <andersson@xxxxxxxxxx>; Mathieu Poirier <mathieu.poirier@xxxxxxxxxx>; Frank Li
> <frank.li@xxxxxxx>; Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>; Shuah Khan
> <skhan@xxxxxxxxxxxxxxxxxxx>; linux-gpio@xxxxxxxxxxxxxxx; linux-
> doc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Pengutronix Kernel Team
> <kernel@xxxxxxxxxxxxxx>; Fabio Estevam <festevam@xxxxxxxxx>; Peng Fan
> <peng.fan@xxxxxxx>; devicetree@xxxxxxxxxxxxxxx; linux-
> remoteproc@xxxxxxxxxxxxxxx; imx@xxxxxxxxxxxxxxx; linux-arm-
> kernel@xxxxxxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>; Bartosz
> Golaszewski <brgl@xxxxxxxx>
> Subject: [EXT] Re: [PATCH v12 3/5] gpio: rpmsg: add generic rpmsg GPIO driver
> 
> > > +static int rpmsg_gpio_send_message(struct rpmsg_gpio_port *port,
> > > +                              struct rpmsg_gpio_packet *msg,
> > > +                              bool sync) {
> > > +   struct rpmsg_gpio_info *info = &port->info;
> > > +   struct rpdev_drvdata *drvdata;
> > > +   int ret;
> > > +
> > > +   drvdata = dev_get_drvdata(&info->rpdev->dev);
> > > +   reinit_completion(&info->cmd_complete);
> > > +
> > > +   if (drvdata->protocol_fixed_up)
> > > +           ret = drvdata->protocol_fixed_up->send_fixed_up(info,
> > > + msg);
> >
> > Seems not part of a generic implementation
> 
> Agreed. Lets have a clean generic implementation first, and then patches on top
> for legacy stuff.
> 

Adding fixed_up hooks doesn't make the implementation non-generic-this pattern is 
widely used in Linux drivers.

Shenwei

> > > +   ret = of_property_read_u32(np, "ngpios", &port->ngpios);
> >
> > The number of GPIOs should be obtained from the remote side, as done
> > in virtio_gpio. In virtio_gpio, this is retrieved via a get_config operation.
> > Here, you could implement a specific RPMsg to retrieve the remote topology.
> 
> The DT property ngpios is pretty standard, so i think it makes a lot of sense to
> have. But i also agree that asking the other side makes sense. What is being
> implemented here with rpmsg is not that different to greybus, and the greybus
> GPIO driver also ask the other side how many GPIO lines it has. So yes, it should
> be part of the protocol, but maybe optional?
> 
>           Andrew