Re: [PATCH 3/4] usb: typec: tcpci_rt1711h: Add support for Hynetek HUSB311

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3319 bytes --]

On Thu, Mar 12, 2026 at 11:09:43AM +0400, Alexey Charkov wrote:
> On Wed, Mar 11, 2026 at 10:33 PM Conor Dooley <conor@kernel.org> wrote:
> >
> > On Wed, Mar 11, 2026 at 08:20:46PM +0400, Alexey Charkov wrote:
> > > HUSB311 is a pin-compatible and register-compatible drop-in replacement
> > > for RT1711H, so add its IDs to the existing driver.
> > >
> > > Link: https://www.hynetek.com/uploadfiles/site/219/news/0863c0c7-f535-4f09-bacd-0440d2c21088.pdf
> > > Link: https://dl.xkwy2018.com/downloads/RK3588S/03_Product%20Line%20Branch_Tablet/02_Key%20Device%20Specifications/HUSB311%20introduction%2020210526.pdf
> > > Signed-off-by: Alexey Charkov <alchark@flipper.net>
> > > ---
> > >  drivers/usb/typec/tcpm/tcpci_rt1711h.c | 21 +++++++++++++++++++--
> > >  1 file changed, 19 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/usb/typec/tcpm/tcpci_rt1711h.c b/drivers/usb/typec/tcpm/tcpci_rt1711h.c
> > > index 88c50b984e8a..ac5917c1e253 100644
> > > --- a/drivers/usb/typec/tcpm/tcpci_rt1711h.c
> > > +++ b/drivers/usb/typec/tcpm/tcpci_rt1711h.c
> > > @@ -18,6 +18,9 @@
> > >  #include <linux/regmap.h>
> > >  #include <linux/regulator/consumer.h>
> > >
> > > +#define HUSB311_VID          0x2E99
> > > +#define HUSB311_PID          0x0311
> > > +#define HUSB311_DID          0x0000
> > >  #define RT1711H_VID          0x29CF
> > >  #define RT1711H_PID          0x1711
> > >  #define RT1711H_DID          0x2171
> > > @@ -55,6 +58,8 @@
> > >
> > >  struct rt1711h_chip_info {
> > >       u32 rxdz_sel;
> > > +     u16 vid;
> > > +     u16 pid;
> > >       u16 did;
> > >       bool enable_pd30_extended_message;
> > >  };
> > > @@ -308,14 +313,14 @@ static int rt1711h_check_revision(struct i2c_client *i2c, struct rt1711h_chip *c
> > >       ret = i2c_smbus_read_word_data(i2c, TCPC_VENDOR_ID);
> > >       if (ret < 0)
> > >               return ret;
> > > -     if (ret != RT1711H_VID) {
> > > +     if (ret != chip->info->vid) {
> > >               dev_err(&i2c->dev, "vid is not correct, 0x%04x\n", ret);
> >
> > Why are we checking vids and pids here? Rejecting a non-match prevents
> > using fallback compatibles, so I'd like to know why the code exists.
> >
> > Is it just here for the sake of it, or to prevent some actual issues?
> > Not really familiar with USB devices unfortunately.
> 
> Hi Conor,
> 
> It looks like a relic of some original vendor provided driver code.
> Rockchip's implementation of a HUSB311 driver [1] contains similar
> checks but I don't think it adds practical value in the world of
> Device Tree (after all, it's just an I2C device - I haven't seen many
> I2C drivers reject a DT match based on hardware IDs found in

There's been a lot of drivers in drivers/iio that do this kind of
although they're gradually being modified over time to support using
fallback compatibles.

> registers). I chose to avoid removing them with my patch though,
> because I don't have any Richtek hardware to test such a change.
> 
> Maybe the error could be downgraded to a warning though.

To make use of a fallback, it can't produce a warning. If you wanna keep
it, just make them dev_dbg() I think.

Also, the devicetree binding should then be modified to support using
the fallback.

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]