On 7/20/22 23:11, Gene Chen wrote:
From: Gene Chen <gene_chen@xxxxxxxxxxx>
Add compatible id with rt1715, and add initial setting for
specific support PD30 command.
Signed-off-by: Gene Chen <gene_chen@xxxxxxxxxxx>
---
drivers/usb/typec/tcpm/tcpci_rt1711h.c | 24 ++++++++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/typec/tcpm/tcpci_rt1711h.c b/drivers/usb/typec/tcpm/tcpci_rt1711h.c
index e65b568959e9..3316dfaeee0d 100644
--- a/drivers/usb/typec/tcpm/tcpci_rt1711h.c
+++ b/drivers/usb/typec/tcpm/tcpci_rt1711h.c
@@ -17,6 +17,7 @@
#define RT1711H_VID 0x29CF
#define RT1711H_PID 0x1711
+#define RT1715_DID 0x2173
#define RT1711H_PHYCTRL1 0x80
#define RT1711H_PHYCTRL2 0x81
@@ -28,6 +29,7 @@
(((ck300) << 7) | ((ship_off) << 5) | \
((auto_idle) << 3) | ((tout) & 0x07))
#define RT1711H_AUTOIDLEEN_MASK BIT(3)
+#define RT1711H_ENEXTMSG_MASK BIT(4)
I would suggest to drop _MASK.
#define RT1711H_RTCTRL11 0x9E
@@ -46,6 +48,7 @@ struct rt1711h_chip {
struct device *dev;
struct regulator *vbus;
bool src_en;
+ u16 did;
};
static int rt1711h_read16(struct rt1711h_chip *chip, unsigned int reg, u16 *val)
@@ -82,8 +85,9 @@ static struct rt1711h_chip *tdata_to_rt1711h(struct tcpci_data *tdata)
static int rt1711h_init(struct tcpci *tcpci, struct tcpci_data *tdata)
{
- int ret;
struct rt1711h_chip *chip = tdata_to_rt1711h(tdata);
+ struct regmap *regmap = chip->data.regmap;
+ int ret;
/* CK 300K from 320K, shipping off, auto_idle enable, tout = 32ms */
ret = rt1711h_write8(chip, RT1711H_RTCTRL8,
@@ -91,6 +95,14 @@ static int rt1711h_init(struct tcpci *tcpci, struct tcpci_data *tdata)
if (ret < 0)
return ret;
+ /* Enable PD30 extended message for RT1715 */
+ if (chip->did == RT1715_DID) {
+ ret = regmap_update_bits(regmap, RT1711H_RTCTRL8,
+ RT1711H_ENEXTMSG_MASK, 0xFF);
0xFF -> RT1711H_ENEXTMSG
+ if (ret < 0)
+ return ret;
+ }
+
/* I2C reset : (val + 1) * 12.5ms */
ret = rt1711h_write8(chip, RT1711H_RTCTRL11,
RT1711H_RTCTRL11_SET(1, 0x0F));
@@ -246,7 +258,11 @@ static int rt1711h_check_revision(struct i2c_client *i2c)
dev_err(&i2c->dev, "pid is not correct, 0x%04x\n", ret);
return -ENODEV;
}
- return 0;
+ ret = i2c_smbus_read_word_data(i2c, TCPC_BCD_DEV);
+ if (ret < 0)
+ return ret;
+ dev_info(&i2c->dev, "did is 0x%04x\n", ret);
Unnecessary noise. If needed for testing, please make it dev_dbg.
+ return ret;
I think it would make sense to pass chip as parameter and set chip->did here.
Also, validation is missing. This function is supposed to check/validate
revision data, but it just accepts all DIDs. Then later DID values are used
to distinguish functionality. At the same time, the new device ID and OF
compatible strings are not used for that purpose and thus have no real value.
Since there can be chips with different DIDs which require different functionality,
DID values should be validated, and only chips with supported DIDs should be
accepted. Also, since there are separate device IDs and devicetree compatible
properties, the DIDs associated with supported chips should be referenced there.
Thanks,
Guenter
}
static int rt1711h_probe(struct i2c_client *client,
@@ -265,6 +281,8 @@ static int rt1711h_probe(struct i2c_client *client,
if (!chip)
return -ENOMEM;
+ chip->did = ret;
+
chip->data.regmap = devm_regmap_init_i2c(client,
&rt1711h_regmap_config);
if (IS_ERR(chip->data.regmap))
@@ -315,6 +333,7 @@ static int rt1711h_remove(struct i2c_client *client)
static const struct i2c_device_id rt1711h_id[] = {
{ "rt1711h", 0 },
+ { "rt1715", 0 },
{ }
};
MODULE_DEVICE_TABLE(i2c, rt1711h_id);
@@ -322,6 +341,7 @@ MODULE_DEVICE_TABLE(i2c, rt1711h_id);
#ifdef CONFIG_OF
static const struct of_device_id rt1711h_of_match[] = {
{ .compatible = "richtek,rt1711h", },
+ { .compatible = "richtek,rt1715", },
{},
};
MODULE_DEVICE_TABLE(of, rt1711h_of_match);
