> -----Original Message-----
> From: Andrew Lunn <[email protected]>
> Sent: Tuesday, March 17, 2026 9:22 AM
> To: Arnaud POULIQUEN <[email protected]>
> Cc: Shenwei Wang <[email protected]>; Linus Walleij
> <[email protected]>; Bartosz Golaszewski <[email protected]>; Jonathan Corbet
> <[email protected]>; Rob Herring <[email protected]>; Krzysztof Kozlowski
> <[email protected]>; Conor Dooley <[email protected]>; Bjorn Andersson
> <[email protected]>; Mathieu Poirier <[email protected]>; Frank Li
> <[email protected]>; Sascha Hauer <[email protected]>; Shuah Khan
> <[email protected]>; [email protected]; linux-
> [email protected]; [email protected]; Pengutronix Kernel Team
> <[email protected]>; Fabio Estevam <[email protected]>; Peng Fan
> <[email protected]>; [email protected]; linux-
> [email protected]; [email protected]; linux-arm-
> [email protected]; dl-linux-imx <[email protected]>; Bartosz
> Golaszewski <[email protected]>
> 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

Reply via email to