> > +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.

> > +   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