> > > +struct rpmsg_gpio_packet {
> > > + u8 type; /* Message type */
> > > + u8 cmd; /* Command code */
> > > + u8 port_idx;
> > > + u8 line;
> > > + u8 val1;
> > > + u8 val2;
> > > +};
> >
> >
> > Could you please document the fields in these structs (and the below ones
> > too)?
> > From the code, it looks like while sending a message from Linux to
> > Firmware; val1
> > and val2 are used to describe the values to set. Whereas while receiving a
> > response, val1 represents a possible error code, and val2 represents the
> > actual
> > message of get type queries. If that is so, you might want to change the
> > variable
> > names to be more descriptive and also use a union.
> >
>
> The fields in the two structs are fairly self-explanatory. Do we really need
> the additional comments?
> The previous version of the patch used a union, which was updated to support
> the fixed_up hooks.
> Now that the fixed_up hooks have been removed, I can revert this back to the
> union-based implementation.
I thought you had already adopted the virtio message format?
/* Possible values of the status field */
#define VIRTIO_GPIO_STATUS_OK 0x0
#define VIRTIO_GPIO_STATUS_ERR 0x1
struct virtio_gpio_response {
__u8 status;
__u8 value;
};
Seems pretty obvious what status means. value depends on the request,
get_direction actually uses it, and it can be one of
#define VIRTIO_GPIO_DIRECTION_NONE 0x00
#define VIRTIO_GPIO_DIRECTION_OUT 0x01
#define VIRTIO_GPIO_DIRECTION_IN 0x02
and gpio_get uses it as a bool for the state of the GPIO.
Why do we need all the complexity for val1, val2, etc?
Andrew
