Re: [PATCH v8 3/4] gpio: rpmsg: add generic rpmsg GPIO driver

[Shenwei Wang]

> -----Original Message-----
> From: Andrew Lunn <andrew@xxxxxxx>
> Sent: Friday, February 20, 2026 7:49 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 v8 3/4] gpio: rpmsg: add generic rpmsg GPIO driver
> >
> > On 2/19/26 14:42, Andrew Lunn wrote:
> > > > > +       u8 id;          /* Message ID Code */
> > > > > +       u8 vendor;      /* Vendor ID number */
> > > >
> > > > Does this fields above are mandatory, seems that it is just some
> > > > constant values that are useless.
> > > >
> > > > > +       u8 version;     /* Vendor-specific version number */
> > > >
> > > > Why it is vendor specific? the version should represent the
> > > > rpmsg-tty protocol version.
> > > >
> > > > > +       u8 type;        /* Message type */
> > > > > +       u8 cmd;         /* Command code */
> > > > > +       u8 reserved[5];
> > > >
> > > > What is the purpose of this reserved field?
> > >
> > > They have an implementation of the other end running on there
> > > systems, and it sounds like it is widely deployed, and they are
> > > trying to keep backwards compatibility. The protocol also implements
> > > more than GPIO. There is also I2C, maybe watchdog, i don't remember,
> > > but early versions of this patchset had a list. Some of these fields
> > > are used for some of these other devices.
> > >
> > > I've been arguing it should be a clean design, with the protocol
> > > focusing on GPIO. And that the rpmsg channel makes it clear this is
> > > a GPIO device, the protocol itself does not need to include fields
> > > to differentiate between GPIO, I2C etc.
> > >
> > > When they start submitting I2C over rpmsg, i expect the same sort of
> > > discussion will start again, so the likelihood of keeping backwards
> > > compatible with there firmware seems low to me.
> >
> > Agree with you.
> 
> Hi Shenwei
> 
> We now have two Maintainers who think you should make a clean design...
> 
> You should go talk with your Management and decide what they want to do.
> Drop this patch series and only support the out of tree driver? Or rework this
> driver and the firmware to the liking of the mainline Maintainers.
> 

The driver is meant to support a specific piece of hardware, and its design follows the 
company’s existing IP, which is already widely deployed. While we're open to sharing 
the IP details with anyone who wants to adopt the technology, changing the IP itself 
isn’t the goal of this patch series.

From the driver perspective, the implementation is clean and generic, and it can support 
multiple hardware variants—even those based on other vendors’ IP—without requiring 
architectural changes. I think it would be more productive if the review focused on the 
driver’s implementation and integration into the subsystem, rather than asking to redesign 
or replace an existing, widely‑released IP block. The intention here is to upstream support 
for hardware/firmware that already exists, not to redefine it.

If there are concerns about specific design elements within the driver, I’m happy to address 
those, but redesigning the hardware/firmware interface is not something this series can solve.

Thanks,
Shenwei


>         Andrew