diff mbox series

[3/4] usb: typec: tcpci_maxim: configure charging & data paths

Message ID 20210311100313.3591254-3-badhri@google.com (mailing list archive)
State New, archived
Headers show
Series [1/4] usb: typec: tcpm: Add callback to notify pd_capable partner | expand

Commit Message

Badhri Jagan Sridharan March 11, 2021, 10:03 a.m. UTC
This change allows the driver to configure input current/voltage
limit for the charging path. The driver sets current_max and voltage_max
values of the power supply identified through chg-psy-name.

The change also exposes the data_role and the orientation as a extcon
interface for configuring the USB data controller.

Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
---
 drivers/usb/typec/tcpm/tcpci_maxim.c | 126 ++++++++++++++++++++++++++-
 1 file changed, 125 insertions(+), 1 deletion(-)

Comments

Heikki Krogerus March 11, 2021, 1:33 p.m. UTC | #1
Hi,

On Thu, Mar 11, 2021 at 02:03:12AM -0800, Badhri Jagan Sridharan wrote:
> This change allows the driver to configure input current/voltage
> limit for the charging path. The driver sets current_max and voltage_max
> values of the power supply identified through chg-psy-name.
> 
> The change also exposes the data_role and the orientation as a extcon
> interface for configuring the USB data controller.

This looks wrong to me. Why wouldn't you just register your device as
a separate psy that supplies your charger (which is also a psy, right)?

thanks,
Badhri Jagan Sridharan March 12, 2021, 5:08 a.m. UTC | #2
On Thu, Mar 11, 2021 at 5:33 AM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> Hi,
>
> On Thu, Mar 11, 2021 at 02:03:12AM -0800, Badhri Jagan Sridharan wrote:
> > This change allows the driver to configure input current/voltage
> > limit for the charging path. The driver sets current_max and voltage_max
> > values of the power supply identified through chg-psy-name.
> >
> > The change also exposes the data_role and the orientation as a extcon
> > interface for configuring the USB data controller.
>
> This looks wrong to me. Why wouldn't you just register your device as
> a separate psy that supplies your charger (which is also a psy, right)?

Hi Heikki,

Looks like that would pretty much make it reflect the same values as
"tcpm-source-psy-" exposed
by tcpm. So experimenting with making the charger power supply a supplicant.
However, noticed that the "tcpm-source-psy-" does not have calls to
power_supply_changed().
So the notifiers are not getting invoked.
Trying to fix that to see if just "tcpm-source-psy-" helps the case
without me trying to create another
one which almost would reflect the same values. Let me know if you
think that might not work.

For now, refactored the patches to only include changes related to
data path and sending
them in. Will follow up with patches for the charger path once I am
done with the above approach
and some validation.

Thanks,
Badhri
>
>
> thanks,
>
> --
> heikki
Badhri Jagan Sridharan March 16, 2021, 10:02 p.m. UTC | #3
On Thu, Mar 11, 2021 at 9:08 PM Badhri Jagan Sridharan
<badhri@google.com> wrote:
>
> On Thu, Mar 11, 2021 at 5:33 AM Heikki Krogerus
> <heikki.krogerus@linux.intel.com> wrote:
> >
> > Hi,
> >
> > On Thu, Mar 11, 2021 at 02:03:12AM -0800, Badhri Jagan Sridharan wrote:
> > > This change allows the driver to configure input current/voltage
> > > limit for the charging path. The driver sets current_max and voltage_max
> > > values of the power supply identified through chg-psy-name.
> > >
> > > The change also exposes the data_role and the orientation as a extcon
> > > interface for configuring the USB data controller.
> >
> > This looks wrong to me. Why wouldn't you just register your device as
> > a separate psy that supplies your charger (which is also a psy, right)?
>
> Hi Heikki,
>
> Looks like that would pretty much make it reflect the same values as
> "tcpm-source-psy-" exposed
> by tcpm. So experimenting with making the charger power supply a supplicant.
> However, noticed that the "tcpm-source-psy-" does not have calls to
> power_supply_changed().
> So the notifiers are not getting invoked.
> Trying to fix that to see if just "tcpm-source-psy-" helps the case
> without me trying to create another
> one which almost would reflect the same values. Let me know if you
> think that might not work.
Hi Heikki,

With "[PATCH] usb: typec: tcpm: Invoke power_supply_changed for
tcpm-source-psy-"
which I just sent, "tcpm-source-psy-" seems to do the job. So the
set/get_current_limit
and pd_capable callbacks are not needed. Please do review
"[PATCH] usb: typec: tcpm: Invoke power_supply_changed for tcpm-source-psy-".

Thanks,
Badhri

>
> For now, refactored the patches to only include changes related to
> data path and sending
> them in. Will follow up with patches for the charger path once I am
> done with the above approach
> and some validation.
>
> Thanks,
> Badhri
> >
> >
> > thanks,
> >
> > --
> > heikki
diff mbox series

Patch

diff --git a/drivers/usb/typec/tcpm/tcpci_maxim.c b/drivers/usb/typec/tcpm/tcpci_maxim.c
index 041a1c393594..365ff4c18146 100644
--- a/drivers/usb/typec/tcpm/tcpci_maxim.c
+++ b/drivers/usb/typec/tcpm/tcpci_maxim.c
@@ -7,8 +7,11 @@ 
 
 #include <linux/interrupt.h>
 #include <linux/i2c.h>
+#include <linux/extcon.h>
+#include <linux/extcon-provider.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/power_supply.h>
 #include <linux/regmap.h>
 #include <linux/usb/pd.h>
 #include <linux/usb/tcpm.h>
@@ -46,6 +49,10 @@  struct max_tcpci_chip {
 	struct device *dev;
 	struct i2c_client *client;
 	struct tcpm_port *port;
+	bool pd_capable;
+	bool attached;
+	struct power_supply *chg_psy;
+	struct extcon_dev *extcon;
 };
 
 static const struct regmap_range max_tcpci_tcpci_range[] = {
@@ -439,11 +446,92 @@  static int tcpci_init(struct tcpci *tcpci, struct tcpci_data *data)
 	return -1;
 }
 
+static int max_tcpci_get_current_limit(struct tcpci *tcpci, struct tcpci_data *data)
+{
+	struct max_tcpci_chip *chip = tdata_to_max_tcpci(data);
+	union power_supply_propval val = {0};
+	int ret;
+
+	if (!chip->chg_psy)
+		return 0;
+
+	ret = power_supply_get_property(chip->chg_psy, POWER_SUPPLY_PROP_CURRENT_MAX, &val);
+	return ret < 0 ? ret : val.intval;
+}
+
+static int max_tcpci_set_current_limit(struct tcpci *tcpci, struct tcpci_data *data, u32 max_ma,
+				       u32 mv)
+{
+	struct max_tcpci_chip *chip = tdata_to_max_tcpci(data);
+	union power_supply_propval val;
+	int ret;
+
+	if (!chip->chg_psy)
+		return 0;
+
+	val.intval = mv;
+	ret = power_supply_set_property(chip->chg_psy, POWER_SUPPLY_PROP_VOLTAGE_MAX, &val);
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * Disregard 0mA vote for Rp-default value. Rp-default current values
+	 * would be inferred from other sources such BC1.2 and data stack.
+	 */
+	if (!chip->pd_capable && max_ma == 0 && chip->attached)
+		return 0;
+
+	val.intval = max_ma;
+	return power_supply_set_property(chip->chg_psy, POWER_SUPPLY_PROP_CURRENT_MAX, &val);
+}
+
+static void max_tcpci_set_pd_capable(struct tcpci *tcpci, struct tcpci_data *data, bool capable)
+{
+	struct max_tcpci_chip *chip = tdata_to_max_tcpci(data);
+
+	chip->pd_capable = capable;
+}
+
+static void max_tcpci_set_roles(struct tcpci *tcpci, struct tcpci_data *data, bool attached,
+				enum typec_role role, enum typec_data_role data_role)
+{
+	struct max_tcpci_chip *chip = tdata_to_max_tcpci(data);
+
+	chip->attached = attached;
+
+	if (!attached) {
+		extcon_set_state_sync(chip->extcon, EXTCON_USB_HOST, 0);
+		extcon_set_state_sync(chip->extcon, EXTCON_USB, 0);
+		return;
+	}
+
+	extcon_set_state_sync(chip->extcon, data_role == TYPEC_HOST ? EXTCON_USB_HOST : EXTCON_USB,
+			      1);
+}
+
+static void max_tcpci_set_cc_polarity(struct tcpci *tcpci, struct tcpci_data *data,
+				      enum typec_cc_polarity polarity)
+{
+	struct max_tcpci_chip *chip = tdata_to_max_tcpci(data);
+
+	extcon_set_property(chip->extcon, EXTCON_USB, EXTCON_PROP_USB_TYPEC_POLARITY,
+			    (union extcon_property_value)(int)polarity);
+	extcon_set_property(chip->extcon, EXTCON_USB_HOST, EXTCON_PROP_USB_TYPEC_POLARITY,
+			    (union extcon_property_value)(int)polarity);
+}
+
+static const unsigned int usbpd_extcon[] = {
+	EXTCON_USB,
+	EXTCON_USB_HOST,
+};
+
 static int max_tcpci_probe(struct i2c_client *client, const struct i2c_device_id *i2c_id)
 {
 	int ret;
 	struct max_tcpci_chip *chip;
 	u8 power_status;
+	char *chg_psy_name;
+	struct device_node *dn;
 
 	chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL);
 	if (!chip)
@@ -472,6 +560,11 @@  static int max_tcpci_probe(struct i2c_client *client, const struct i2c_device_id
 	chip->data.auto_discharge_disconnect = true;
 	chip->data.vbus_vsafe0v = true;
 	chip->data.set_partner_usb_comm_capable = max_tcpci_set_partner_usb_comm_capable;
+	chip->data.get_current_limit = max_tcpci_get_current_limit;
+	chip->data.set_current_limit = max_tcpci_set_current_limit;
+	chip->data.set_pd_capable = max_tcpci_set_pd_capable;
+	chip->data.set_roles = max_tcpci_set_roles;
+	chip->data.set_cc_polarity = max_tcpci_set_cc_polarity;
 
 	max_tcpci_init_regs(chip);
 	chip->tcpci = tcpci_register_port(chip->dev, &chip->data);
@@ -484,9 +577,38 @@  static int max_tcpci_probe(struct i2c_client *client, const struct i2c_device_id
 	if (ret < 0)
 		goto unreg_port;
 
+	dn = dev_of_node(&client->dev);
+	if (!dn) {
+		dev_err(&client->dev, "of node not found\n");
+		ret = -EINVAL;
+		goto unreg_port;
+	}
+	chg_psy_name = (char *)of_get_property(dn, "chg-psy-name", NULL);
+	if (chg_psy_name)
+		chip->chg_psy = power_supply_get_by_name(chg_psy_name);
+
+	chip->extcon = devm_extcon_dev_allocate(&client->dev, usbpd_extcon);
+	if (IS_ERR(chip->extcon)) {
+		dev_err(&client->dev, "Error allocating extcon: %ld\n", PTR_ERR(chip->extcon));
+		ret = PTR_ERR(chip->extcon);
+		goto psy_put;
+	}
+
+	ret = devm_extcon_dev_register(&client->dev, chip->extcon);
+	if (ret < 0) {
+		dev_err(&client->dev, "failed to register extcon device");
+		goto psy_put;
+	}
+
+	extcon_set_property_capability(chip->extcon, EXTCON_USB, EXTCON_PROP_USB_TYPEC_POLARITY);
+	extcon_set_property_capability(chip->extcon, EXTCON_USB_HOST,
+				       EXTCON_PROP_USB_TYPEC_POLARITY);
+
 	device_init_wakeup(chip->dev, true);
 	return 0;
-
+psy_put:
+	if (chip->chg_psy)
+		power_supply_put(chip->chg_psy);
 unreg_port:
 	tcpci_unregister_port(chip->tcpci);
 
@@ -499,6 +621,8 @@  static int max_tcpci_remove(struct i2c_client *client)
 
 	if (!IS_ERR_OR_NULL(chip->tcpci))
 		tcpci_unregister_port(chip->tcpci);
+	if (chip->chg_psy)
+		power_supply_put(chip->chg_psy);
 
 	return 0;
 }