2021-03-11 10:04:57

by Badhri Jagan Sridharan

[permalink] [raw]
Subject: [PATCH 1/4] usb: typec: tcpm: Add callback to notify pd_capable partner

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 <[email protected]>
---
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


2021-03-11 10:05:00

by Badhri Jagan Sridharan

[permalink] [raw]
Subject: [PATCH 3/4] usb: typec: tcpci_maxim: configure charging & data paths

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 <[email protected]>
---
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

2021-03-11 10:05:07

by Badhri Jagan Sridharan

[permalink] [raw]
Subject: [PATCH 4/4] dt-bindings: usb: Add chg-psy-name property Maxim 33359 binding

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 <[email protected]>
---
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

2021-03-11 13:36:01

by Heikki Krogerus

[permalink] [raw]
Subject: Re: [PATCH 3/4] usb: typec: tcpci_maxim: configure charging & data paths

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

2021-03-12 06:35:47

by Badhri Jagan Sridharan

[permalink] [raw]
Subject: Re: [PATCH 3/4] usb: typec: tcpci_maxim: configure charging & data paths

On Thu, Mar 11, 2021 at 5:33 AM Heikki Krogerus
<[email protected]> 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

2021-03-16 22:56:47

by Badhri Jagan Sridharan

[permalink] [raw]
Subject: Re: [PATCH 3/4] usb: typec: tcpci_maxim: configure charging & data paths

On Thu, Mar 11, 2021 at 9:08 PM Badhri Jagan Sridharan
<[email protected]> wrote:
>
> On Thu, Mar 11, 2021 at 5:33 AM Heikki Krogerus
> <[email protected]> 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

2021-03-24 14:52:45

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH 4/4] dt-bindings: usb: Add chg-psy-name property Maxim 33359 binding

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 <[email protected]>
> ---
> 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

2021-03-25 03:26:20

by Badhri Jagan Sridharan

[permalink] [raw]
Subject: Re: [PATCH 4/4] dt-bindings: usb: Add chg-psy-name property Maxim 33359 binding

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 <[email protected]> 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 <[email protected]>
> > ---
> > 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