All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] usb: typec: tcpm: Add callback to notify pd_capable partner
@ 2021-03-11 10:03 Badhri Jagan Sridharan
  2021-03-11 10:03 ` [PATCH 2/4] usb: typec: tcpci: Add tcpc chip level callbacks Badhri Jagan Sridharan
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Badhri Jagan Sridharan @ 2021-03-11 10:03 UTC (permalink / raw)
  To: Guenter Roeck, Heikki Krogerus, Greg Kroah-Hartman, Rob Herring
  Cc: linux-usb, linux-kernel, Kyle Tso, devicetree, Badhri Jagan Sridharan

This change informs lower level tcpc drivers of pd_capable
partner. This is useful while setting current limit for the
charging path.

Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
---
 drivers/usb/typec/tcpm/tcpm.c | 20 +++++++++++++++-----
 include/linux/usb/tcpm.h      |  3 +++
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 11d0c40bc47d..e9886e850b84 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -952,6 +952,16 @@ static int tcpm_set_current_limit(struct tcpm_port *port, u32 max_ma, u32 mv)
 	return ret;
 }
 
+static void tcpm_set_pd_capable(struct tcpm_port *port, bool capable)
+{
+	tcpm_log(port, "Partner pd capable %s", capable ? "true" : "false");
+
+	if (port->tcpc->set_pd_capable)
+		port->tcpc->set_pd_capable(port->tcpc, capable);
+
+	port->pd_capable = capable;
+}
+
 static int tcpm_set_attached_state(struct tcpm_port *port, bool attached)
 {
 	return port->tcpc->set_roles(port->tcpc, attached, port->pwr_role,
@@ -3444,7 +3454,7 @@ static int tcpm_src_attach(struct tcpm_port *port)
 	if (ret < 0)
 		goto out_disable_vconn;
 
-	port->pd_capable = false;
+	tcpm_set_pd_capable(port, false);
 
 	port->partner = NULL;
 
@@ -3509,7 +3519,7 @@ static void tcpm_reset_port(struct tcpm_port *port)
 	tcpm_unregister_altmodes(port);
 	tcpm_typec_disconnect(port);
 	port->attached = false;
-	port->pd_capable = false;
+	tcpm_set_pd_capable(port, false);
 	port->pps_data.supported = false;
 	tcpm_set_partner_usb_comm_capable(port, false);
 
@@ -3583,7 +3593,7 @@ static int tcpm_snk_attach(struct tcpm_port *port)
 	if (ret < 0)
 		return ret;
 
-	port->pd_capable = false;
+	tcpm_set_pd_capable(port, false);
 
 	port->partner = NULL;
 
@@ -3813,7 +3823,7 @@ static void run_state_machine(struct tcpm_port *port)
 			 */
 			/* port->hard_reset_count = 0; */
 			port->caps_count = 0;
-			port->pd_capable = true;
+			tcpm_set_pd_capable(port, true);
 			tcpm_set_state_cond(port, SRC_SEND_CAPABILITIES_TIMEOUT,
 					    PD_T_SEND_SOURCE_CAP);
 		}
@@ -4074,7 +4084,7 @@ static void run_state_machine(struct tcpm_port *port)
 		}
 		break;
 	case SNK_NEGOTIATE_CAPABILITIES:
-		port->pd_capable = true;
+		tcpm_set_pd_capable(port, true);
 		tcpm_set_partner_usb_comm_capable(port,
 						  !!(port->source_caps[0] & PDO_FIXED_USB_COMM));
 		port->hard_reset_count = 0;
diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
index 42fcfbe10590..d5d7293bc34d 100644
--- a/include/linux/usb/tcpm.h
+++ b/include/linux/usb/tcpm.h
@@ -112,6 +112,8 @@ enum tcpm_transmit_type {
  *              Optional; The USB Communications Capable bit indicates if port
  *              partner is capable of communication over the USB data lines
  *              (e.g. D+/- or SS Tx/Rx). Called to notify the status of the bit.
+ * @set_pd_capable:
+ *		Optional; Called to notify if the partner is pd capable.
  */
 struct tcpc_dev {
 	struct fwnode_handle *fwnode;
@@ -144,6 +146,7 @@ struct tcpc_dev {
 						 bool pps_active, u32 requested_vbus_voltage);
 	bool (*is_vbus_vsafe0v)(struct tcpc_dev *dev);
 	void (*set_partner_usb_comm_capable)(struct tcpc_dev *dev, bool enable);
+	void (*set_pd_capable)(struct tcpc_dev *dev, bool enable);
 };
 
 struct tcpm_port;
-- 
2.31.0.rc2.261.g7f71774620-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/4] usb: typec: tcpci: Add tcpc chip level callbacks
  2021-03-11 10:03 [PATCH 1/4] usb: typec: tcpm: Add callback to notify pd_capable partner Badhri Jagan Sridharan
@ 2021-03-11 10:03 ` Badhri Jagan Sridharan
  2021-03-11 10:03 ` [PATCH 3/4] usb: typec: tcpci_maxim: configure charging & data paths Badhri Jagan Sridharan
  2021-03-11 10:03 ` [PATCH 4/4] dt-bindings: usb: Add chg-psy-name property Maxim 33359 binding Badhri Jagan Sridharan
  2 siblings, 0 replies; 9+ messages in thread
From: Badhri Jagan Sridharan @ 2021-03-11 10:03 UTC (permalink / raw)
  To: Guenter Roeck, Heikki Krogerus, Greg Kroah-Hartman, Rob Herring
  Cc: linux-usb, linux-kernel, Kyle Tso, devicetree, Badhri Jagan Sridharan

This change adds chip callbacks for the following operations:
1. Setting/getting vbus voltage and current limits.
2. Notifying presence of PD capable partner
3. Notifying port role
4. Notifying orientation

Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
---
 drivers/usb/typec/tcpm/tcpci.c | 45 +++++++++++++++++++++++++++++++++-
 drivers/usb/typec/tcpm/tcpci.h |  7 ++++++
 2 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
index 027afd7dfdce..35f229d1dde8 100644
--- a/drivers/usb/typec/tcpm/tcpci.c
+++ b/drivers/usb/typec/tcpm/tcpci.c
@@ -265,9 +265,16 @@ static int tcpci_set_polarity(struct tcpc_dev *tcpc,
 	if (ret < 0)
 		return ret;
 
-	return regmap_write(tcpci->regmap, TCPC_TCPC_CTRL,
+	ret = regmap_write(tcpci->regmap, TCPC_TCPC_CTRL,
 			   (polarity == TYPEC_POLARITY_CC2) ?
 			   TCPC_TCPC_CTRL_ORIENTATION : 0);
+	if (ret < 0)
+		return ret;
+
+	if (tcpci->data->set_cc_polarity)
+		tcpci->data->set_cc_polarity(tcpci, tcpci->data, polarity);
+
+	return ret;
 }
 
 static void tcpci_set_partner_usb_comm_capable(struct tcpc_dev *tcpc, bool capable)
@@ -395,6 +402,9 @@ static int tcpci_set_roles(struct tcpc_dev *tcpc, bool attached,
 	if (ret < 0)
 		return ret;
 
+	if (tcpci->data->set_roles)
+		tcpci->data->set_roles(tcpci, tcpci->data, attached, role, data);
+
 	return 0;
 }
 
@@ -439,6 +449,36 @@ static bool tcpci_is_vbus_vsafe0v(struct tcpc_dev *tcpc)
 	return !!(reg & TCPC_EXTENDED_STATUS_VSAFE0V);
 }
 
+static void tcpci_set_pd_capable(struct tcpc_dev *tcpc, bool capable)
+{
+	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
+
+	if (tcpci->data->set_pd_capable)
+		tcpci->data->set_pd_capable(tcpci, tcpci->data, capable);
+}
+
+static int tcpci_get_current_limit(struct tcpc_dev *tcpc)
+{
+	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
+
+	if (tcpci->data->get_current_limit)
+		return tcpci->data->get_current_limit(tcpci, tcpci->data);
+
+	return 0;
+}
+
+static int tcpci_set_current_limit(struct tcpc_dev *tcpc, u32 max_ma, u32 mv)
+{
+	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
+	int ret = 0;
+
+	if (tcpci->data->set_current_limit)
+		ret = tcpci->data->set_current_limit(tcpci, tcpci->data, max_ma,
+						     mv);
+
+	return ret;
+}
+
 static int tcpci_set_vbus(struct tcpc_dev *tcpc, bool source, bool sink)
 {
 	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
@@ -744,6 +784,9 @@ struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data)
 	tcpci->tcpc.enable_frs = tcpci_enable_frs;
 	tcpci->tcpc.frs_sourcing_vbus = tcpci_frs_sourcing_vbus;
 	tcpci->tcpc.set_partner_usb_comm_capable = tcpci_set_partner_usb_comm_capable;
+	tcpci->tcpc.set_pd_capable = tcpci_set_pd_capable;
+	tcpci->tcpc.set_current_limit = tcpci_set_current_limit;
+	tcpci->tcpc.get_current_limit = tcpci_get_current_limit;
 
 	if (tcpci->data->auto_discharge_disconnect) {
 		tcpci->tcpc.enable_auto_vbus_discharge = tcpci_enable_auto_vbus_discharge;
diff --git a/drivers/usb/typec/tcpm/tcpci.h b/drivers/usb/typec/tcpm/tcpci.h
index 57b6e24e0a0c..6fdad15ce1f2 100644
--- a/drivers/usb/typec/tcpm/tcpci.h
+++ b/drivers/usb/typec/tcpm/tcpci.h
@@ -181,6 +181,13 @@ struct tcpci_data {
 	void (*frs_sourcing_vbus)(struct tcpci *tcpci, struct tcpci_data *data);
 	void (*set_partner_usb_comm_capable)(struct tcpci *tcpci, struct tcpci_data *data,
 					     bool capable);
+	void (*set_pd_capable)(struct tcpci *tcpci, struct tcpci_data *data, bool capable);
+	int (*get_current_limit)(struct tcpci *tcpci, struct tcpci_data *data);
+	int (*set_current_limit)(struct tcpci *tcpci, struct tcpci_data *data, u32 max_ma, u32 mv);
+	void (*set_roles)(struct tcpci *tcpci, struct tcpci_data *data, bool attached,
+			  enum typec_role role, enum typec_data_role data_role);
+	void (*set_cc_polarity)(struct tcpci *tcpci, struct tcpci_data *data,
+				enum typec_cc_polarity polarity);
 };
 
 struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data);
-- 
2.31.0.rc2.261.g7f71774620-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/4] usb: typec: tcpci_maxim: configure charging & data paths
  2021-03-11 10:03 [PATCH 1/4] usb: typec: tcpm: Add callback to notify pd_capable partner Badhri Jagan Sridharan
  2021-03-11 10:03 ` [PATCH 2/4] usb: typec: tcpci: Add tcpc chip level callbacks Badhri Jagan Sridharan
@ 2021-03-11 10:03 ` Badhri Jagan Sridharan
  2021-03-11 13:33   ` Heikki Krogerus
  2021-03-11 10:03 ` [PATCH 4/4] dt-bindings: usb: Add chg-psy-name property Maxim 33359 binding Badhri Jagan Sridharan
  2 siblings, 1 reply; 9+ messages in thread
From: Badhri Jagan Sridharan @ 2021-03-11 10:03 UTC (permalink / raw)
  To: Guenter Roeck, Heikki Krogerus, Greg Kroah-Hartman, Rob Herring
  Cc: linux-usb, linux-kernel, Kyle Tso, devicetree, Badhri Jagan Sridharan

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(-)

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;
 }
-- 
2.31.0.rc2.261.g7f71774620-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/4] dt-bindings: usb: Add chg-psy-name property Maxim 33359 binding
  2021-03-11 10:03 [PATCH 1/4] usb: typec: tcpm: Add callback to notify pd_capable partner Badhri Jagan Sridharan
  2021-03-11 10:03 ` [PATCH 2/4] usb: typec: tcpci: Add tcpc chip level callbacks Badhri Jagan Sridharan
  2021-03-11 10:03 ` [PATCH 3/4] usb: typec: tcpci_maxim: configure charging & data paths Badhri Jagan Sridharan
@ 2021-03-11 10:03 ` Badhri Jagan Sridharan
  2021-03-24 14:50   ` Rob Herring
  2 siblings, 1 reply; 9+ messages in thread
From: Badhri Jagan Sridharan @ 2021-03-11 10:03 UTC (permalink / raw)
  To: Guenter Roeck, Heikki Krogerus, Greg Kroah-Hartman, Rob Herring
  Cc: linux-usb, linux-kernel, Kyle Tso, devicetree, Badhri Jagan Sridharan

chg-psy-name is an optional string property used to indicate the
power supply object for which the current/voltage_max limits have
to be set.

Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
---
 Documentation/devicetree/bindings/usb/maxim,max33359.yaml | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
index 93a19eda610b..3a278969109e 100644
--- a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
+++ b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
@@ -28,6 +28,11 @@ properties:
     description:
       Properties for usb c connector.
 
+  chg-psy-name:
+    description: Power supply whose current/voltage_max values to be
+      configured.
+    $ref: /schemas/types.yaml#definitions/string
+
 required:
   - compatible
   - reg
@@ -49,7 +54,7 @@ examples:
             reg = <0x25>;
             interrupt-parent = <&gpa8>;
             interrupts = <2 IRQ_TYPE_LEVEL_LOW>;
-
+            chg-psy-name = "main_charger";
             connector {
                 compatible = "usb-c-connector";
                 label = "USB-C";
-- 
2.31.0.rc2.261.g7f71774620-goog


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/4] usb: typec: tcpci_maxim: configure charging & data paths
  2021-03-11 10:03 ` [PATCH 3/4] usb: typec: tcpci_maxim: configure charging & data paths Badhri Jagan Sridharan
@ 2021-03-11 13:33   ` Heikki Krogerus
  2021-03-12  5:08     ` Badhri Jagan Sridharan
  0 siblings, 1 reply; 9+ messages in thread
From: Heikki Krogerus @ 2021-03-11 13:33 UTC (permalink / raw)
  To: Badhri Jagan Sridharan
  Cc: Guenter Roeck, Greg Kroah-Hartman, Rob Herring, linux-usb,
	linux-kernel, Kyle Tso, devicetree

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,

-- 
heikki

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/4] usb: typec: tcpci_maxim: configure charging & data paths
  2021-03-11 13:33   ` Heikki Krogerus
@ 2021-03-12  5:08     ` Badhri Jagan Sridharan
  2021-03-16 22:02       ` Badhri Jagan Sridharan
  0 siblings, 1 reply; 9+ messages in thread
From: Badhri Jagan Sridharan @ 2021-03-12  5:08 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Guenter Roeck, Greg Kroah-Hartman, Rob Herring, USB, LKML,
	Kyle Tso,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/4] usb: typec: tcpci_maxim: configure charging & data paths
  2021-03-12  5:08     ` Badhri Jagan Sridharan
@ 2021-03-16 22:02       ` Badhri Jagan Sridharan
  0 siblings, 0 replies; 9+ messages in thread
From: Badhri Jagan Sridharan @ 2021-03-16 22:02 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Guenter Roeck, Greg Kroah-Hartman, Rob Herring, USB, LKML,
	Kyle Tso,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 4/4] dt-bindings: usb: Add chg-psy-name property Maxim 33359 binding
  2021-03-11 10:03 ` [PATCH 4/4] dt-bindings: usb: Add chg-psy-name property Maxim 33359 binding Badhri Jagan Sridharan
@ 2021-03-24 14:50   ` Rob Herring
  2021-03-24 20:01     ` Badhri Jagan Sridharan
  0 siblings, 1 reply; 9+ messages in thread
From: Rob Herring @ 2021-03-24 14:50 UTC (permalink / raw)
  To: Badhri Jagan Sridharan
  Cc: Guenter Roeck, Heikki Krogerus, Greg Kroah-Hartman, linux-usb,
	linux-kernel, Kyle Tso, devicetree

On Thu, Mar 11, 2021 at 02:03:13AM -0800, Badhri Jagan Sridharan wrote:
> chg-psy-name is an optional string property used to indicate the
> power supply object for which the current/voltage_max limits have
> to be set.
> 
> Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
> ---
>  Documentation/devicetree/bindings/usb/maxim,max33359.yaml | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
> index 93a19eda610b..3a278969109e 100644
> --- a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
> +++ b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
> @@ -28,6 +28,11 @@ properties:
>      description:
>        Properties for usb c connector.
>  
> +  chg-psy-name:
> +    description: Power supply whose current/voltage_max values to be
> +      configured.
> +    $ref: /schemas/types.yaml#definitions/string

If you want a non-vendor specific property, this needs to be documented 
in a common binding. I think this needs a better explaination and 
examples of multiple chargers.

Rob

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 4/4] dt-bindings: usb: Add chg-psy-name property Maxim 33359 binding
  2021-03-24 14:50   ` Rob Herring
@ 2021-03-24 20:01     ` Badhri Jagan Sridharan
  0 siblings, 0 replies; 9+ messages in thread
From: Badhri Jagan Sridharan @ 2021-03-24 20:01 UTC (permalink / raw)
  To: Rob Herring
  Cc: Guenter Roeck, Heikki Krogerus, Greg Kroah-Hartman, USB, LKML,
	Kyle Tso,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

Hi Rob,

Thanks for the feedback !
From the feedback that I received from the other patches in the stack,
we have identified an alternate approach of doing this without
introducing this device tree addition.
So, for now this patch is no longer needed. While the alternate
approach is still being validated, will resurface this patch if I
identify any drawbacks of the alternate approach.

Regards,
Badhri


On Wed, Mar 24, 2021 at 7:50 AM Rob Herring <robh@kernel.org> wrote:
>
> On Thu, Mar 11, 2021 at 02:03:13AM -0800, Badhri Jagan Sridharan wrote:
> > chg-psy-name is an optional string property used to indicate the
> > power supply object for which the current/voltage_max limits have
> > to be set.
> >
> > Signed-off-by: Badhri Jagan Sridharan <badhri@google.com>
> > ---
> >  Documentation/devicetree/bindings/usb/maxim,max33359.yaml | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
> > index 93a19eda610b..3a278969109e 100644
> > --- a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
> > +++ b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
> > @@ -28,6 +28,11 @@ properties:
> >      description:
> >        Properties for usb c connector.
> >
> > +  chg-psy-name:
> > +    description: Power supply whose current/voltage_max values to be
> > +      configured.
> > +    $ref: /schemas/types.yaml#definitions/string
>
> If you want a non-vendor specific property, this needs to be documented
> in a common binding. I think this needs a better explaination and
> examples of multiple chargers.
>
> Rob

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-03-24 20:03 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-11 10:03 [PATCH 1/4] usb: typec: tcpm: Add callback to notify pd_capable partner Badhri Jagan Sridharan
2021-03-11 10:03 ` [PATCH 2/4] usb: typec: tcpci: Add tcpc chip level callbacks Badhri Jagan Sridharan
2021-03-11 10:03 ` [PATCH 3/4] usb: typec: tcpci_maxim: configure charging & data paths Badhri Jagan Sridharan
2021-03-11 13:33   ` Heikki Krogerus
2021-03-12  5:08     ` Badhri Jagan Sridharan
2021-03-16 22:02       ` Badhri Jagan Sridharan
2021-03-11 10:03 ` [PATCH 4/4] dt-bindings: usb: Add chg-psy-name property Maxim 33359 binding Badhri Jagan Sridharan
2021-03-24 14:50   ` Rob Herring
2021-03-24 20:01     ` Badhri Jagan Sridharan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.