Add Richtek RT1719 USBPD controller support

Quoted 1 line

drivers/usb/typec/rt1719.c:492:65: warning: suggest parentheses around arithmetic in operand of '|' [-Wparentheses]

From: ChiYuan Huang <[email protected]>

Add binding for Richtek RT1719.

Signed-off-by ChiYuan Huang <[email protected]>

---
 .../devicetree/bindings/usb/richtek,rt1719.yaml    | 85 ++++++++++++++++++++++
 1 file changed, 85 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/richtek,rt1719.yaml

drivers/usb/typec/rt1719.c new file +31 -0

+static int rt1719_probe(struct i2c_client *i2c)
+{
+	struct rt1719_data *data;
+	struct fwnode_handle *fwnode;
+	struct typec_capability typec_cap = { };
+	int ret;
+
+	data = devm_kzalloc(&i2c->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->dev = &i2c->dev;
+	init_completion(&data->req_completion);
+
+	data->regmap = devm_regmap_init_i2c(i2c, &rt1719_regmap_config);
+	if (IS_ERR(data->regmap)) {
+		ret = PTR_ERR(data->regmap);
+		dev_err(&i2c->dev, "Failed to init regmap (%d)\n", ret);
+		return ret;
+	}
+
+	ret = rt1719_check_exist(data);
+	if (ret)
+		return ret;
+
+	ret = rt1719_get_caps(data);
+	if (ret)
+		return ret;
+
+	/*
+	 * This fwnode has a "compatible" property, but is never populated as a

drivers/usb/typec/rt1719.c new file +76 -0

+	data->role_sw = fwnode_usb_role_switch_get(fwnode);
+	if (IS_ERR(data->role_sw)) {
+		ret = PTR_ERR(data->role_sw);
+		dev_err(&i2c->dev, "Failed to get usb role switch (%d)\n", ret);
+		goto err_fwnode_put;
+	}
+
+	ret = devm_rt1719_psy_register(data);
+	if (ret) {
+		dev_err(&i2c->dev, "Failed to register psy (%d)\n", ret);
+		goto err_role_put;
+	}
+
+	typec_cap.revision = USB_TYPEC_REV_1_2;
+	typec_cap.pd_revision = 0x300;	/* USB-PD spec release 3.0 */
+	typec_cap.type = TYPEC_PORT_SNK;
+	typec_cap.data = TYPEC_PORT_DRD;
+	typec_cap.ops = &rt1719_port_ops;
+	typec_cap.fwnode = fwnode;
+	typec_cap.driver_data = data;
+	typec_cap.accessory[0] = TYPEC_ACCESSORY_DEBUG;
+
+	data->partner_desc.identity = &data->partner_ident;
+
+	data->port = typec_register_port(&i2c->dev, &typec_cap);
+	if (IS_ERR(data->port)) {
+		ret = PTR_ERR(data->port);
+		dev_err(&i2c->dev, "Failed to register typec port (%d)\n", ret);
+		goto err_role_put;
+	}
+
+	ret = rt1719_init_attach_state(data);
+	if (ret) {
+		dev_err(&i2c->dev, "Failed to init attach state (%d)\n", ret);
+		goto err_role_put;
+	}
+
+	ret = rt1719_irq_init(data);
+	if (ret) {
+		dev_err(&i2c->dev, "Failed to init irq\n");
+		goto err_role_put;
+	}
+
+	fwnode_handle_put(fwnode);
+
+	i2c_set_clientdata(i2c, data);
+
+	return 0;
+
+err_role_put:
+	usb_role_switch_put(data->role_sw);
+err_fwnode_put:
+	fwnode_handle_put(fwnode);
+
+	return ret;
+}
+
+static int rt1719_remove(struct i2c_client *i2c)
+{
+	struct rt1719_data *data = i2c_get_clientdata(i2c);
+
+	typec_unregister_port(data->port);
+	usb_role_switch_put(data->role_sw);
+
+	return 0;
+}
+
+static const struct of_device_id __maybe_unused rt1719_device_table[] = {
+	{ .compatible = "richtek,rt1719", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, rt1719_device_table);
+
+static struct i2c_driver rt1719_driver = {
+	.driver = {
+		.name = "rt1719",

Hi,

Sorry, there is still one more problem that I realised - not your problem, but a problem with fw_devlink_purge_absent_suppliers()...

Quoted 31 lines · cy_huang

drivers/usb/typec/rt1719.c new file +31 -0

+static int rt1719_probe(struct i2c_client *i2c)
+{
+     struct rt1719_data *data;
+     struct fwnode_handle *fwnode;
+     struct typec_capability typec_cap = { };
+     int ret;
+
+     data = devm_kzalloc(&i2c->dev, sizeof(*data), GFP_KERNEL);
+     if (!data)
+             return -ENOMEM;
+
+     data->dev = &i2c->dev;
+     init_completion(&data->req_completion);
+
+     data->regmap = devm_regmap_init_i2c(i2c, &rt1719_regmap_config);
+     if (IS_ERR(data->regmap)) {
+             ret = PTR_ERR(data->regmap);
+             dev_err(&i2c->dev, "Failed to init regmap (%d)\n", ret);
+             return ret;
+     }
+
+     ret = rt1719_check_exist(data);
+     if (ret)
+             return ret;
+
+     ret = rt1719_get_caps(data);
+     if (ret)
+             return ret;
+
+     /*
+      * This fwnode has a "compatible" property, but is never populated as a

drivers/usb/typec/rt1719.c line 883

Quoted 10 lines

drivers/usb/typec/rt1719.c new file +10 -0

+      * struct device. Instead we simply parse it to read the properties.
+      * This it breaks fw_devlink=on. To maintain backward compatibility
+      * with existing DT files, we work around this by deleting any
+      * fwnode_links to/from this fwnode.
+      */
+     fwnode = device_get_named_child_node(&i2c->dev, "connector");
+     if (!fwnode)
+             return -ENODEV;
+
+     fw_devlink_purge_absent_suppliers(fwnode);

So don't use that function unless you really see some issue that it's fixing yourself.

That function is broken. It breaks at least if the fwnode is shared - fwnodes can be, and are, shared - most likely it's broken in some other ways too. That function seems to be a hack for some individual problem that was caused by some deeper problem with the device links.

We really need to figure out a fix for the core problem now instead of trying to come up with these hacks for every single separate scenario that is breaking because of the core problem, what ever that core problem may be.

Quoted 81 lines

drivers/usb/typec/rt1719.c new file +81 -0

+     data->role_sw = fwnode_usb_role_switch_get(fwnode);
+     if (IS_ERR(data->role_sw)) {
+             ret = PTR_ERR(data->role_sw);
+             dev_err(&i2c->dev, "Failed to get usb role switch (%d)\n", ret);
+             goto err_fwnode_put;
+     }
+
+     ret = devm_rt1719_psy_register(data);
+     if (ret) {
+             dev_err(&i2c->dev, "Failed to register psy (%d)\n", ret);
+             goto err_role_put;
+     }
+
+     typec_cap.revision = USB_TYPEC_REV_1_2;
+     typec_cap.pd_revision = 0x300;  /* USB-PD spec release 3.0 */
+     typec_cap.type = TYPEC_PORT_SNK;
+     typec_cap.data = TYPEC_PORT_DRD;
+     typec_cap.ops = &rt1719_port_ops;
+     typec_cap.fwnode = fwnode;
+     typec_cap.driver_data = data;
+     typec_cap.accessory[0] = TYPEC_ACCESSORY_DEBUG;
+
+     data->partner_desc.identity = &data->partner_ident;
+
+     data->port = typec_register_port(&i2c->dev, &typec_cap);
+     if (IS_ERR(data->port)) {
+             ret = PTR_ERR(data->port);
+             dev_err(&i2c->dev, "Failed to register typec port (%d)\n", ret);
+             goto err_role_put;
+     }
+
+     ret = rt1719_init_attach_state(data);
+     if (ret) {
+             dev_err(&i2c->dev, "Failed to init attach state (%d)\n", ret);
+             goto err_role_put;
+     }
+
+     ret = rt1719_irq_init(data);
+     if (ret) {
+             dev_err(&i2c->dev, "Failed to init irq\n");
+             goto err_role_put;
+     }
+
+     fwnode_handle_put(fwnode);
+
+     i2c_set_clientdata(i2c, data);
+
+     return 0;
+
+err_role_put:
+     usb_role_switch_put(data->role_sw);
+err_fwnode_put:
+     fwnode_handle_put(fwnode);
+
+     return ret;
+}
+
+static int rt1719_remove(struct i2c_client *i2c)
+{
+     struct rt1719_data *data = i2c_get_clientdata(i2c);
+
+     typec_unregister_port(data->port);
+     usb_role_switch_put(data->role_sw);
+
+     return 0;
+}
+
+static const struct of_device_id __maybe_unused rt1719_device_table[] = {
+     { .compatible = "richtek,rt1719", },
+     { }
+};
+MODULE_DEVICE_TABLE(of, rt1719_device_table);
+
+static struct i2c_driver rt1719_driver = {
+     .driver = {
+             .name = "rt1719",
+             .of_match_table = rt1719_device_table,
+     },
+     .probe_new = rt1719_probe,
+     .remove = rt1719_remove,
+};

drivers/usb/typec/rt1719.c line 970

Quoted 5 lines

drivers/usb/typec/rt1719.c new file +5 -0

+module_i2c_driver(rt1719_driver);
+
+MODULE_AUTHOR("ChiYuan Huang <[email protected]>");
+MODULE_DESCRIPTION("Richtek RT1719 Sink Only USBPD Controller Driver");
+MODULE_LICENSE("GPL v2");

Oh, you probable want the make that "GPL" instead. See Documentation/process/license-rules.rst.

thanks,

--
heikki