2023-01-12 08:22:45

by Gene Chen

[permalink] [raw]
Subject: Re: FW: [PATCH 2/2] usb: typec: tcpci_rt1718s: Add Richtek RT1718S tcpci driver

> -----Original Message-----
> From: Guenter Roeck [mailto:[email protected]] On Behalf Of Guenter Roeck
> Sent: Tuesday, January 10, 2023 2:52 AM
> To: gene_chen(陳俊宇) <[email protected]>; [email protected]; [email protected]; [email protected]
> Cc: [email protected]; [email protected]; [email protected]; [email protected]
> Subject: Re: [PATCH 2/2] usb: typec: tcpci_rt1718s: Add Richtek RT1718S tcpci driver
>
> On 1/9/23 01:31, [email protected] wrote:
> > From: Gene Chen <[email protected]>
> >
> > Richtek RT1718S is highly integrated TCPC and Power Delivery (PD)
> > controller with IEC-ESD Protection on SBU/CC/DP/DM, USB2.0 Switch,
> > Charging Port Controller and Power-Path Control.
> >
> > Signed-off-by: Gene Chen <[email protected]>
> > ---
> > drivers/usb/typec/tcpm/Kconfig | 9 +
> > drivers/usb/typec/tcpm/Makefile | 1 +
> > drivers/usb/typec/tcpm/tcpci_rt1718s.c | 349 +++++++++++++++++++++++++++++++++
> > 3 files changed, 359 insertions(+)
> > create mode 100644 drivers/usb/typec/tcpm/tcpci_rt1718s.c
> >
> > diff --git a/drivers/usb/typec/tcpm/Kconfig
> > b/drivers/usb/typec/tcpm/Kconfig index e6b88ca..f0efb34 100644
> > --- a/drivers/usb/typec/tcpm/Kconfig
> > +++ b/drivers/usb/typec/tcpm/Kconfig
> > @@ -27,6 +27,15 @@ config TYPEC_RT1711H
> > Type-C Port Controller Manager to provide USB PD and USB
> > Type-C functionalities.
> >
> > +config TYPEC_RT1718S
> > +tristate "Richtek RT1718S Type-C chip driver"
> > +depends on I2C
> > +help
> > + Richtek RT1718S Type-C chip driver that works with
> > + Type-C Port Controller Manager to provide USB PD and USB
> > + Type-C functionalities.
> > + Additionally, it supports BC1.2 and power-path control.
> > +
> > config TYPEC_MT6360
> > tristate "Mediatek MT6360 Type-C driver"
> > depends on MFD_MT6360
> > diff --git a/drivers/usb/typec/tcpm/Makefile
> > b/drivers/usb/typec/tcpm/Makefile index 906d9dc..db33ffc 100644
> > --- a/drivers/usb/typec/tcpm/Makefile
> > +++ b/drivers/usb/typec/tcpm/Makefile
> > @@ -5,6 +5,7 @@ obj-$(CONFIG_TYPEC_WCOVE)+= typec_wcove.o
> > typec_wcove-y:= wcove.o
> > obj-$(CONFIG_TYPEC_TCPCI)+= tcpci.o
> > obj-$(CONFIG_TYPEC_RT1711H)+= tcpci_rt1711h.o
> > +obj-$(CONFIG_TYPEC_RT1718S)+= tcpci_rt1718s.o
> > obj-$(CONFIG_TYPEC_MT6360)+= tcpci_mt6360.o
> > obj-$(CONFIG_TYPEC_TCPCI_MT6370)+= tcpci_mt6370.o
> > obj-$(CONFIG_TYPEC_TCPCI_MAXIM)+= tcpci_maxim.o
> > diff --git a/drivers/usb/typec/tcpm/tcpci_rt1718s.c
> > b/drivers/usb/typec/tcpm/tcpci_rt1718s.c
> > new file mode 100644
> > index 00000000..305b39c
> > --- /dev/null
> > +++ b/drivers/usb/typec/tcpm/tcpci_rt1718s.c
> > +
> > +if (*(u8 *)reg == RT1718S_P1PREFIX) {
> > +xfer[0].len = 1,
> > +xfer[0].buf = (u8 *)(reg + 1);
>
> Pointer arithmetic on void * (see below).
>

yes, should be xfer[0].buf = (u8 *)reg + 1;

> > +}
>
> There is a lot of context here which needs explanation. The code extracts the upper 8 bit of the register address and drops it if the value is 0.
> So the address is either 8 bit or 16 bit depending on the register address.
> Also, this only works because reg_format_endian is set to big endian.
>
> This really needs to be documented.
>
> Assigning the values to len and buf twice is really bad style.
> Please use if/else.
>

Because of rt1718s address which is more than 1 page(0x00-0xFF), hw
design i2c pattern with register has 2 byte to access another
page(0xF200-0xF2FF).
The upper byte is set to 0xF2 indicated page 2.

> > +
> > +if (*(u8 *)val == RT1718S_P1PREFIX) {
> > +xfer.len = val_size - 1;
> > +xfer.buf = (u8 *)(val + 1);
> > +}
> > +
>
> Same comments as above. Also, typecast seems wrong. Shouldn't it be
> ((u8 *)val) + 1) ? My memory may defeat me, but I think that pointer arithmetic on void * is undefined or even illegal.
>
>

You are right, I will fix it, thanks.

> > +if (ret < 0)
> > +return ret;
> > +if (ret != 1)
> > +return -EIO;
> > +
> > +return 0;
> > +}
> > +
> > +static const struct regmap_bus rt1718s_regmap_bus = {
> > +.read= rt1718s_regmap_read,
> > +.write= rt1718s_regmap_write,
> > +};
> > +
>
> This needs documentation: Why not use standard regmap functions ?
> Yes, I know, it is because of the ubusual addressing format used by the chip, but it still needs to be explained. Not everyone should have to read the datasheet to understand the code.
>

Should I add comment before declare rt1718s_regmap_bus?

/*
* Because of rt1718s address which is more than 1 page(0x00-0xFF),
* hw design i2c pattern with register has 2 byte to access another
page(0xF200-0xF2FF).
* The upper byte is set to 0xF2 indicated page 2.
*/

> > +static int rt1718s_sw_reset(struct rt1718s_chip *chip) {
> > +int ret;
> > +
> > +ret = regmap_update_bits(chip->tdata.regmap, RT1718S_SYS_CTRL3,
> > + RT1718S_SWRESET_MASK, RT1718S_SWRESET_MASK);
> > +if (ret < 0)
> > +return ret;
> > +
> > +/* Wait for IC to reset done*/
> > +usleep_range(1000, 2000);
> > +
>
> "RT1718S will not response to the I2C commands for 2ms after writing SOFT_RESET"
>
> Timeout needs to be at least 2 ms.
>

ACK

> > +return 0;
> > +}
> > +
> > +static int rt1718s_check_chip_exist(struct i2c_client *i2c) {
> > +int ret;
> > +
> > +ret = i2c_smbus_read_word_data(i2c, TCPC_VENDOR_ID);
> > +if (ret < 0)
> > +return ret;
> > +if (ret != RT1718S_VID) {
> > +dev_err(&i2c->dev, "vid is not correct, 0x%04x\n", ret);
> > +return -ENODEV;
> > +}
> > +ret = i2c_smbus_read_word_data(i2c, TCPC_PRODUCT_ID);
> > +if (ret < 0)
> > +return ret;
> > +if (ret != RT1718S_PID) {
> > +dev_err(&i2c->dev, "pid is not correct, 0x%04x\n", ret);
> > +return -ENODEV;
> > +}
> > +return 0;
> > +}
> > +
> > +static int rt1718s_probe(struct i2c_client *i2c) {
> > +struct rt1718s_chip *chip;
> > +struct gpio_desc *gpiod;
> > +int ret;
> > +u16 clr_events = 0;
> > +
> > +ret = rt1718s_check_chip_exist(i2c);
> > +if (ret < 0) {
> > +dev_err(&i2c->dev, "check vid/pid fail(%d)\n", ret);
>
> Double error message.
>

ACK, I will remove it.

> > +return ret;
> > +}
> > +
> > +chip = devm_kzalloc(&i2c->dev, sizeof(*chip), GFP_KERNEL);
> > +if (!chip)
> > +return -ENOMEM;
> > +
> > +chip->dev = &i2c->dev;
> > +
> > +chip->tdata.regmap = devm_regmap_init(&i2c->dev,
> > + &rt1718s_regmap_bus, &i2c->dev,
> > + &rt1718s_regmap_config);
> > +if (IS_ERR(chip->tdata.regmap))
> > +return dev_err_probe(&i2c->dev, PTR_ERR(chip->tdata.regmap),
> > + "Failed to init regmap\n");
> > +
> > +chip->vbus = devm_regulator_get(&i2c->dev, "vbus");
> > +if (IS_ERR(chip->vbus))
> > +return dev_err_probe(&i2c->dev, PTR_ERR(chip->vbus),
> > + "Failed to get vbus regulator\n");
> > +
> > +ret = rt1718s_sw_reset(chip);
> > +if (ret < 0)
> > +return ret;
> > +
> > +ret = regmap_raw_write(chip->tdata.regmap, TCPC_ALERT_MASK, &clr_events,
> > + sizeof(clr_events));
> > +
> > +chip->tdata.init = rt1718s_init;
> > +chip->tdata.set_vbus = rt1718s_set_vbus;
> > +chip->tcpci = tcpci_register_port(&i2c->dev, &chip->tdata);
> > +if (IS_ERR(chip->tcpci))
> > +return dev_err_probe(&i2c->dev, PTR_ERR(chip->tcpci),
> > + "Failed to register tcpci port\n");
> > +
> > +/* for platform set gpio default inpull_high */
> > +gpiod = devm_gpiod_get(&i2c->dev, NULL, GPIOD_IN);
> > +if (IS_ERR(gpiod))
> > +return dev_err_probe(&i2c->dev, PTR_ERR(gpiod),
> > + "Failed to get gpio\n");
> > +
> > +ret = devm_request_threaded_irq(&i2c->dev, i2c->irq, NULL,
> > +rt1718s_irq, IRQF_ONESHOT,
> > +dev_name(&i2c->dev), chip);
> > +if (ret) {
> > +dev_err(chip->dev, "Failed to register irq\n");
> > +tcpci_unregister_port(chip->tcpci);
> > +return ret;
> > +}
> > +
> > +device_init_wakeup(&i2c->dev, true);
> > +i2c_set_clientdata(i2c, chip);
> > +
> > +dev_info(&i2c->dev, "%s:successfully\n", __func__);
>
> Nore logging noise.
>

ACK

> > +return 0;
> > +}
> > +
> > +static void rt1718s_remove(struct i2c_client *i2c) {
> > +struct rt1718s_chip *chip = i2c_get_clientdata(i2c);
> > +
> > +tcpci_unregister_port(chip->tcpci);
> > +}
> > +
> > +static int __maybe_unused rt1718s_suspend(struct device *dev) {
> > +struct i2c_client *i2c = to_i2c_client(dev);
> > +
> > +if (device_may_wakeup(dev))
> > +enable_irq_wake(i2c->irq);
> > +disable_irq(i2c->irq);
> > +
> > +return 0;
> > +}
> > +
> > +static int __maybe_unused rt1718s_resume(struct device *dev) {
> > +struct i2c_client *i2c = to_i2c_client(dev);
> > +
> > +if (device_may_wakeup(dev))
> > +disable_irq_wake(i2c->irq);
> > +enable_irq(i2c->irq);
> > +
> > +return 0;
> > +}
> > +
> > +static SIMPLE_DEV_PM_OPS(rt1718s_pm_ops, rt1718s_suspend,
> > +rt1718s_resume);
> > +
> > +static const struct of_device_id __maybe_unused rt1718s_of_id[] = {
> > +{ .compatible = "richtek,rt1718s", },
> > +{},
> > +};
> > +MODULE_DEVICE_TABLE(of, rt1718s_of_id);
> > +
> > +static struct i2c_driver rt1718s_i2c_driver = {
> > +.driver = {
> > +.name = "rt1718s",
> > +.pm = &rt1718s_pm_ops,
> > +.of_match_table = rt1718s_of_id,
> > +},
> > +.probe_new = rt1718s_probe,
> > +.remove = rt1718s_remove,
> > +};
> > +module_i2c_driver(rt1718s_i2c_driver);
> > +
> > +MODULE_AUTHOR("Gene Chen <[email protected]>");
> > +MODULE_DESCRIPTION("RT1718S USB Type-C Port Controller Interface
> > +Driver"); MODULE_LICENSE("GPL");


2023-01-12 15:09:56

by Guenter Roeck

[permalink] [raw]
Subject: Re: FW: [PATCH 2/2] usb: typec: tcpci_rt1718s: Add Richtek RT1718S tcpci driver

On 1/12/23 00:05, Gene Chen wrote:
>> -----Original Message-----
>> From: Guenter Roeck [mailto:[email protected]] On Behalf Of Guenter Roeck
>> Sent: Tuesday, January 10, 2023 2:52 AM
>> To: gene_chen(陳俊宇) <[email protected]>; [email protected]; [email protected]; [email protected]
>> Cc: [email protected]; [email protected]; [email protected]; [email protected]
>> Subject: Re: [PATCH 2/2] usb: typec: tcpci_rt1718s: Add Richtek RT1718S tcpci driver
>>
>> On 1/9/23 01:31, [email protected] wrote:
>>> From: Gene Chen <[email protected]>
>>>
>>> Richtek RT1718S is highly integrated TCPC and Power Delivery (PD)
>>> controller with IEC-ESD Protection on SBU/CC/DP/DM, USB2.0 Switch,
>>> Charging Port Controller and Power-Path Control.
>>>
>>> Signed-off-by: Gene Chen <[email protected]>
>>> ---
>>> drivers/usb/typec/tcpm/Kconfig | 9 +
>>> drivers/usb/typec/tcpm/Makefile | 1 +
>>> drivers/usb/typec/tcpm/tcpci_rt1718s.c | 349 +++++++++++++++++++++++++++++++++
>>> 3 files changed, 359 insertions(+)
>>> create mode 100644 drivers/usb/typec/tcpm/tcpci_rt1718s.c
>>>
>>> diff --git a/drivers/usb/typec/tcpm/Kconfig
>>> b/drivers/usb/typec/tcpm/Kconfig index e6b88ca..f0efb34 100644
>>> --- a/drivers/usb/typec/tcpm/Kconfig
>>> +++ b/drivers/usb/typec/tcpm/Kconfig
>>> @@ -27,6 +27,15 @@ config TYPEC_RT1711H
>>> Type-C Port Controller Manager to provide USB PD and USB
>>> Type-C functionalities.
>>>
>>> +config TYPEC_RT1718S
>>> +tristate "Richtek RT1718S Type-C chip driver"
>>> +depends on I2C
>>> +help
>>> + Richtek RT1718S Type-C chip driver that works with
>>> + Type-C Port Controller Manager to provide USB PD and USB
>>> + Type-C functionalities.
>>> + Additionally, it supports BC1.2 and power-path control.
>>> +
>>> config TYPEC_MT6360
>>> tristate "Mediatek MT6360 Type-C driver"
>>> depends on MFD_MT6360
>>> diff --git a/drivers/usb/typec/tcpm/Makefile
>>> b/drivers/usb/typec/tcpm/Makefile index 906d9dc..db33ffc 100644
>>> --- a/drivers/usb/typec/tcpm/Makefile
>>> +++ b/drivers/usb/typec/tcpm/Makefile
>>> @@ -5,6 +5,7 @@ obj-$(CONFIG_TYPEC_WCOVE)+= typec_wcove.o
>>> typec_wcove-y:= wcove.o
>>> obj-$(CONFIG_TYPEC_TCPCI)+= tcpci.o
>>> obj-$(CONFIG_TYPEC_RT1711H)+= tcpci_rt1711h.o
>>> +obj-$(CONFIG_TYPEC_RT1718S)+= tcpci_rt1718s.o
>>> obj-$(CONFIG_TYPEC_MT6360)+= tcpci_mt6360.o
>>> obj-$(CONFIG_TYPEC_TCPCI_MT6370)+= tcpci_mt6370.o
>>> obj-$(CONFIG_TYPEC_TCPCI_MAXIM)+= tcpci_maxim.o
>>> diff --git a/drivers/usb/typec/tcpm/tcpci_rt1718s.c
>>> b/drivers/usb/typec/tcpm/tcpci_rt1718s.c
>>> new file mode 100644
>>> index 00000000..305b39c
>>> --- /dev/null
>>> +++ b/drivers/usb/typec/tcpm/tcpci_rt1718s.c
>>> +
>>> +if (*(u8 *)reg == RT1718S_P1PREFIX) {
>>> +xfer[0].len = 1,
>>> +xfer[0].buf = (u8 *)(reg + 1);
>>
>> Pointer arithmetic on void * (see below).
>>
>
> yes, should be xfer[0].buf = (u8 *)reg + 1;
>
>>> +}
>>
>> There is a lot of context here which needs explanation. The code extracts the upper 8 bit of the register address and drops it if the value is 0.
>> So the address is either 8 bit or 16 bit depending on the register address.
>> Also, this only works because reg_format_endian is set to big endian.
>>
>> This really needs to be documented.
>>
>> Assigning the values to len and buf twice is really bad style.
>> Please use if/else.
>>
>
> Because of rt1718s address which is more than 1 page(0x00-0xFF), hw
> design i2c pattern with register has 2 byte to access another
> page(0xF200-0xF2FF).
> The upper byte is set to 0xF2 indicated page 2.
>

Sure. I meant something like

if (*(u8 *)reg == RT1718S_P1PREFIX) {
xfer[0].len = 1; <<== that should be a semicolon, not ','
xfer[0].buf = (u8 *)reg + 1;
} else {
...
}

to avoid the double assignment.

>>> +
>>> +if (*(u8 *)val == RT1718S_P1PREFIX) {
>>> +xfer.len = val_size - 1;
>>> +xfer.buf = (u8 *)(val + 1);
>>> +}
>>> +
>>
>> Same comments as above. Also, typecast seems wrong. Shouldn't it be
>> ((u8 *)val) + 1) ? My memory may defeat me, but I think that pointer arithmetic on void * is undefined or even illegal.
>>
>>
>
> You are right, I will fix it, thanks.
>
>>> +if (ret < 0)
>>> +return ret;
>>> +if (ret != 1)
>>> +return -EIO;
>>> +
>>> +return 0;
>>> +}
>>> +
>>> +static const struct regmap_bus rt1718s_regmap_bus = {
>>> +.read= rt1718s_regmap_read,
>>> +.write= rt1718s_regmap_write,
>>> +};
>>> +
>>
>> This needs documentation: Why not use standard regmap functions ?
>> Yes, I know, it is because of the ubusual addressing format used by the chip, but it still needs to be explained. Not everyone should have to read the datasheet to understand the code.
>>
>
> Should I add comment before declare rt1718s_regmap_bus?
>
> /*
> * Because of rt1718s address which is more than 1 page(0x00-0xFF),
> * hw design i2c pattern with register has 2 byte to access another
> page(0xF200-0xF2FF).
> * The upper byte is set to 0xF2 indicated page 2.
> */
>

Yes, something like that.

>>> +static int rt1718s_sw_reset(struct rt1718s_chip *chip) {
>>> +int ret;
>>> +
>>> +ret = regmap_update_bits(chip->tdata.regmap, RT1718S_SYS_CTRL3,
>>> + RT1718S_SWRESET_MASK, RT1718S_SWRESET_MASK);
>>> +if (ret < 0)
>>> +return ret;
>>> +
>>> +/* Wait for IC to reset done*/
>>> +usleep_range(1000, 2000);
>>> +
>>
>> "RT1718S will not response to the I2C commands for 2ms after writing SOFT_RESET"
>>
>> Timeout needs to be at least 2 ms.
>>
>
> ACK
>
>>> +return 0;
>>> +}
>>> +
>>> +static int rt1718s_check_chip_exist(struct i2c_client *i2c) {
>>> +int ret;
>>> +
>>> +ret = i2c_smbus_read_word_data(i2c, TCPC_VENDOR_ID);
>>> +if (ret < 0)
>>> +return ret;
>>> +if (ret != RT1718S_VID) {
>>> +dev_err(&i2c->dev, "vid is not correct, 0x%04x\n", ret);
>>> +return -ENODEV;
>>> +}
>>> +ret = i2c_smbus_read_word_data(i2c, TCPC_PRODUCT_ID);
>>> +if (ret < 0)
>>> +return ret;
>>> +if (ret != RT1718S_PID) {
>>> +dev_err(&i2c->dev, "pid is not correct, 0x%04x\n", ret);
>>> +return -ENODEV;
>>> +}
>>> +return 0;
>>> +}
>>> +
>>> +static int rt1718s_probe(struct i2c_client *i2c) {
>>> +struct rt1718s_chip *chip;
>>> +struct gpio_desc *gpiod;
>>> +int ret;
>>> +u16 clr_events = 0;
>>> +
>>> +ret = rt1718s_check_chip_exist(i2c);
>>> +if (ret < 0) {
>>> +dev_err(&i2c->dev, "check vid/pid fail(%d)\n", ret);
>>
>> Double error message.
>>
>
> ACK, I will remove it.
>
>>> +return ret;
>>> +}
>>> +
>>> +chip = devm_kzalloc(&i2c->dev, sizeof(*chip), GFP_KERNEL);
>>> +if (!chip)
>>> +return -ENOMEM;
>>> +
>>> +chip->dev = &i2c->dev;
>>> +
>>> +chip->tdata.regmap = devm_regmap_init(&i2c->dev,
>>> + &rt1718s_regmap_bus, &i2c->dev,
>>> + &rt1718s_regmap_config);
>>> +if (IS_ERR(chip->tdata.regmap))
>>> +return dev_err_probe(&i2c->dev, PTR_ERR(chip->tdata.regmap),
>>> + "Failed to init regmap\n");
>>> +
>>> +chip->vbus = devm_regulator_get(&i2c->dev, "vbus");
>>> +if (IS_ERR(chip->vbus))
>>> +return dev_err_probe(&i2c->dev, PTR_ERR(chip->vbus),
>>> + "Failed to get vbus regulator\n");
>>> +
>>> +ret = rt1718s_sw_reset(chip);
>>> +if (ret < 0)
>>> +return ret;
>>> +
>>> +ret = regmap_raw_write(chip->tdata.regmap, TCPC_ALERT_MASK, &clr_events,
>>> + sizeof(clr_events));
>>> +
>>> +chip->tdata.init = rt1718s_init;
>>> +chip->tdata.set_vbus = rt1718s_set_vbus;
>>> +chip->tcpci = tcpci_register_port(&i2c->dev, &chip->tdata);
>>> +if (IS_ERR(chip->tcpci))
>>> +return dev_err_probe(&i2c->dev, PTR_ERR(chip->tcpci),
>>> + "Failed to register tcpci port\n");
>>> +
>>> +/* for platform set gpio default inpull_high */
>>> +gpiod = devm_gpiod_get(&i2c->dev, NULL, GPIOD_IN);
>>> +if (IS_ERR(gpiod))
>>> +return dev_err_probe(&i2c->dev, PTR_ERR(gpiod),
>>> + "Failed to get gpio\n");
>>> +
>>> +ret = devm_request_threaded_irq(&i2c->dev, i2c->irq, NULL,
>>> +rt1718s_irq, IRQF_ONESHOT,
>>> +dev_name(&i2c->dev), chip);
>>> +if (ret) {
>>> +dev_err(chip->dev, "Failed to register irq\n");
>>> +tcpci_unregister_port(chip->tcpci);
>>> +return ret;
>>> +}
>>> +
>>> +device_init_wakeup(&i2c->dev, true);
>>> +i2c_set_clientdata(i2c, chip);
>>> +
>>> +dev_info(&i2c->dev, "%s:successfully\n", __func__);
>>
>> Nore logging noise.
>>
>
> ACK
>
>>> +return 0;
>>> +}
>>> +
>>> +static void rt1718s_remove(struct i2c_client *i2c) {
>>> +struct rt1718s_chip *chip = i2c_get_clientdata(i2c);
>>> +
>>> +tcpci_unregister_port(chip->tcpci);
>>> +}
>>> +
>>> +static int __maybe_unused rt1718s_suspend(struct device *dev) {
>>> +struct i2c_client *i2c = to_i2c_client(dev);
>>> +
>>> +if (device_may_wakeup(dev))
>>> +enable_irq_wake(i2c->irq);
>>> +disable_irq(i2c->irq);
>>> +
>>> +return 0;
>>> +}
>>> +
>>> +static int __maybe_unused rt1718s_resume(struct device *dev) {
>>> +struct i2c_client *i2c = to_i2c_client(dev);
>>> +
>>> +if (device_may_wakeup(dev))
>>> +disable_irq_wake(i2c->irq);
>>> +enable_irq(i2c->irq);
>>> +
>>> +return 0;
>>> +}
>>> +
>>> +static SIMPLE_DEV_PM_OPS(rt1718s_pm_ops, rt1718s_suspend,
>>> +rt1718s_resume);
>>> +
>>> +static const struct of_device_id __maybe_unused rt1718s_of_id[] = {
>>> +{ .compatible = "richtek,rt1718s", },
>>> +{},
>>> +};
>>> +MODULE_DEVICE_TABLE(of, rt1718s_of_id);
>>> +
>>> +static struct i2c_driver rt1718s_i2c_driver = {
>>> +.driver = {
>>> +.name = "rt1718s",
>>> +.pm = &rt1718s_pm_ops,
>>> +.of_match_table = rt1718s_of_id,
>>> +},
>>> +.probe_new = rt1718s_probe,
>>> +.remove = rt1718s_remove,
>>> +};
>>> +module_i2c_driver(rt1718s_i2c_driver);
>>> +
>>> +MODULE_AUTHOR("Gene Chen <[email protected]>");
>>> +MODULE_DESCRIPTION("RT1718S USB Type-C Port Controller Interface
>>> +Driver"); MODULE_LICENSE("GPL");