The following series adds port-partner registration when
connects/disconnects are detected. In order to do that, we also register
a listener for the cros-usbpd-notifier, which will inform the driver of
EC PD events. While we are here, separate out the Type C port data
objects into a separate struct.
v2: https://lkml.org/lkml/2020/4/9/861
v1: https://lkml.org/lkml/2020/4/6/1155
Changes in v3:
- Fixed trivial Kconfig dependency ordering error.
Prashant Malani (3):
platform/chrome: typec: Use notifier for updates
platform/chrome: typec: Add struct for port data
platform/chrome: typec: Register port partner
drivers/platform/chrome/Kconfig | 1 +
drivers/platform/chrome/cros_ec_typec.c | 119 ++++++++++++++++++++----
2 files changed, 103 insertions(+), 17 deletions(-)
--
2.26.0.110.g2183baf09c-goog
Register a listener for the cros-usbpd-notifier, and update port state
when a notification comes in.
Signed-off-by: Prashant Malani <[email protected]>
---
Changes in v3:
- Fixed ordering issue in Kconfig dependency.
Changes in v2:
- Added Kconfig dependency to CROS_USBPD_NOTIFY.
- Fixed spacing error.
- Removed superfluous devm_warn() call.
drivers/platform/chrome/Kconfig | 1 +
drivers/platform/chrome/cros_ec_typec.c | 23 +++++++++++++++++++++++
2 files changed, 24 insertions(+)
diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index 03ea5129ed0c3..a484ab2c91ff0 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -217,6 +217,7 @@ config CROS_EC_SYSFS
config CROS_EC_TYPEC
tristate "ChromeOS EC Type-C Connector Control"
depends on MFD_CROS_EC_DEV && TYPEC
+ depends on CROS_USBPD_NOTIFY
default MFD_CROS_EC_DEV
help
If you say Y here, you get support for accessing Type C connector
diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index 874269c070739..d444dd7422a2a 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -11,6 +11,7 @@
#include <linux/of.h>
#include <linux/platform_data/cros_ec_commands.h>
#include <linux/platform_data/cros_ec_proto.h>
+#include <linux/platform_data/cros_usbpd_notify.h>
#include <linux/platform_device.h>
#include <linux/usb/typec.h>
@@ -26,6 +27,7 @@ struct cros_typec_data {
struct typec_port *ports[EC_USB_PD_MAX_PORTS];
/* Initial capabilities for each port. */
struct typec_capability *caps[EC_USB_PD_MAX_PORTS];
+ struct notifier_block nb;
};
static int cros_typec_parse_port_props(struct typec_capability *cap,
@@ -272,6 +274,22 @@ static int cros_typec_get_cmd_version(struct cros_typec_data *typec)
return 0;
}
+static int cros_ec_typec_event(struct notifier_block *nb,
+ unsigned long host_event, void *_notify)
+{
+ struct cros_typec_data *typec = container_of(nb, struct cros_typec_data,
+ nb);
+ int ret, i;
+
+ for (i = 0; i < typec->num_ports; i++) {
+ ret = cros_typec_port_update(typec, i);
+ if (ret < 0)
+ dev_warn(typec->dev, "Update failed for port: %d\n", i);
+ }
+
+ return NOTIFY_OK;
+}
+
#ifdef CONFIG_ACPI
static const struct acpi_device_id cros_typec_acpi_id[] = {
{ "GOOG0014", 0 },
@@ -332,6 +350,11 @@ static int cros_typec_probe(struct platform_device *pdev)
goto unregister_ports;
}
+ typec->nb.notifier_call = cros_ec_typec_event;
+ ret = cros_usbpd_register_notify(&typec->nb);
+ if (ret < 0)
+ goto unregister_ports;
+
return 0;
unregister_ports:
--
2.26.0.110.g2183baf09c-goog
Register (and unregister) the port partner when a connect (and
disconnect) is detected.
Co-developed-by: Jon Flatley <[email protected]>
Signed-off-by: Prashant Malani <[email protected]>
---
Changes in v3:
- No changes.
Changes in v2:
- Fixed error pointer return value.
drivers/platform/chrome/cros_ec_typec.c | 48 +++++++++++++++++++++++++
1 file changed, 48 insertions(+)
diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index 56ded09a60ffb..304e0b20f279b 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -22,6 +22,9 @@ struct cros_typec_port {
struct typec_port *port;
/* Initial capabilities for the port. */
struct typec_capability caps;
+ struct typec_partner *partner;
+ /* Port partner PD identity info. */
+ struct usb_pd_identity p_identity;
};
/* Platform-specific data for the Chrome OS EC Type C controller. */
@@ -190,6 +193,30 @@ static int cros_typec_ec_command(struct cros_typec_data *typec,
return ret;
}
+static int cros_typec_add_partner(struct cros_typec_data *typec, int port_num,
+ bool pd_en)
+{
+ struct cros_typec_port *port = typec->ports[port_num];
+ struct typec_partner_desc p_desc = {
+ .usb_pd = pd_en,
+ };
+ int ret = 0;
+
+ /*
+ * Fill an initial PD identity, which will then be updated with info
+ * from the EC.
+ */
+ p_desc.identity = &port->p_identity;
+
+ port->partner = typec_register_partner(port->port, &p_desc);
+ if (IS_ERR_OR_NULL(port->partner)) {
+ ret = PTR_ERR(port->partner);
+ port->partner = NULL;
+ }
+
+ return ret;
+}
+
static void cros_typec_set_port_params_v0(struct cros_typec_data *typec,
int port_num, struct ec_response_usb_pd_control *resp)
{
@@ -212,6 +239,8 @@ static void cros_typec_set_port_params_v1(struct cros_typec_data *typec,
{
struct typec_port *port = typec->ports[port_num]->port;
enum typec_orientation polarity;
+ bool pd_en;
+ int ret;
if (!(resp->enabled & PD_CTRL_RESP_ENABLED_CONNECTED))
polarity = TYPEC_ORIENTATION_NONE;
@@ -226,6 +255,25 @@ static void cros_typec_set_port_params_v1(struct cros_typec_data *typec,
TYPEC_SOURCE : TYPEC_SINK);
typec_set_vconn_role(port, resp->role & PD_CTRL_RESP_ROLE_VCONN ?
TYPEC_SOURCE : TYPEC_SINK);
+
+ /* Register/remove partners when a connect/disconnect occurs. */
+ if (resp->enabled & PD_CTRL_RESP_ENABLED_CONNECTED) {
+ if (typec->ports[port_num]->partner)
+ return;
+
+ pd_en = resp->enabled & PD_CTRL_RESP_ENABLED_PD_CAPABLE;
+ ret = cros_typec_add_partner(typec, port_num, pd_en);
+ if (!ret)
+ dev_warn(typec->dev,
+ "Failed to register partner on port: %d\n",
+ port_num);
+ } else {
+ if (!typec->ports[port_num]->partner)
+ return;
+
+ typec_unregister_partner(typec->ports[port_num]->partner);
+ typec->ports[port_num]->partner = NULL;
+ }
}
static int cros_typec_port_update(struct cros_typec_data *typec, int port_num)
--
2.26.0.110.g2183baf09c-goog
Add a separate struct for storing port data, including Type C connector
class struct pointers and caps.
Signed-off-by: Prashant Malani <[email protected]>
---
Changes in v3:
- No changes.
Changes in v2:
- Removed unrequired devm_kfree() call.
drivers/platform/chrome/cros_ec_typec.c | 48 ++++++++++++++++---------
1 file changed, 31 insertions(+), 17 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index d444dd7422a2a..56ded09a60ffb 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -17,6 +17,13 @@
#define DRV_NAME "cros-ec-typec"
+/* Per port data. */
+struct cros_typec_port {
+ struct typec_port *port;
+ /* Initial capabilities for the port. */
+ struct typec_capability caps;
+};
+
/* Platform-specific data for the Chrome OS EC Type C controller. */
struct cros_typec_data {
struct device *dev;
@@ -24,9 +31,7 @@ struct cros_typec_data {
int num_ports;
unsigned int cmd_ver;
/* Array of ports, indexed by port number. */
- struct typec_port *ports[EC_USB_PD_MAX_PORTS];
- /* Initial capabilities for each port. */
- struct typec_capability *caps[EC_USB_PD_MAX_PORTS];
+ struct cros_typec_port *ports[EC_USB_PD_MAX_PORTS];
struct notifier_block nb;
};
@@ -76,14 +81,25 @@ static int cros_typec_parse_port_props(struct typec_capability *cap,
return 0;
}
+static void cros_unregister_ports(struct cros_typec_data *typec)
+{
+ int i;
+
+ for (i = 0; i < typec->num_ports; i++) {
+ if (!typec->ports[i])
+ continue;
+ typec_unregister_port(typec->ports[i]->port);
+ }
+}
+
static int cros_typec_init_ports(struct cros_typec_data *typec)
{
struct device *dev = typec->dev;
struct typec_capability *cap;
struct fwnode_handle *fwnode;
+ struct cros_typec_port *cros_port;
const char *port_prop;
int ret;
- int i;
int nports;
u32 port_num = 0;
@@ -115,22 +131,23 @@ static int cros_typec_init_ports(struct cros_typec_data *typec)
dev_dbg(dev, "Registering port %d\n", port_num);
- cap = devm_kzalloc(dev, sizeof(*cap), GFP_KERNEL);
- if (!cap) {
+ cros_port = devm_kzalloc(dev, sizeof(*cros_port), GFP_KERNEL);
+ if (!cros_port) {
ret = -ENOMEM;
goto unregister_ports;
}
- typec->caps[port_num] = cap;
+ typec->ports[port_num] = cros_port;
+ cap = &cros_port->caps;
ret = cros_typec_parse_port_props(cap, fwnode, dev);
if (ret < 0)
goto unregister_ports;
- typec->ports[port_num] = typec_register_port(dev, cap);
- if (IS_ERR(typec->ports[port_num])) {
+ cros_port->port = typec_register_port(dev, cap);
+ if (IS_ERR(cros_port->port)) {
dev_err(dev, "Failed to register port %d\n", port_num);
- ret = PTR_ERR(typec->ports[port_num]);
+ ret = PTR_ERR(cros_port->port);
goto unregister_ports;
}
}
@@ -138,8 +155,7 @@ static int cros_typec_init_ports(struct cros_typec_data *typec)
return 0;
unregister_ports:
- for (i = 0; i < typec->num_ports; i++)
- typec_unregister_port(typec->ports[i]);
+ cros_unregister_ports(typec);
return ret;
}
@@ -177,7 +193,7 @@ static int cros_typec_ec_command(struct cros_typec_data *typec,
static void cros_typec_set_port_params_v0(struct cros_typec_data *typec,
int port_num, struct ec_response_usb_pd_control *resp)
{
- struct typec_port *port = typec->ports[port_num];
+ struct typec_port *port = typec->ports[port_num]->port;
enum typec_orientation polarity;
if (!resp->enabled)
@@ -194,7 +210,7 @@ static void cros_typec_set_port_params_v0(struct cros_typec_data *typec,
static void cros_typec_set_port_params_v1(struct cros_typec_data *typec,
int port_num, struct ec_response_usb_pd_control_v1 *resp)
{
- struct typec_port *port = typec->ports[port_num];
+ struct typec_port *port = typec->ports[port_num]->port;
enum typec_orientation polarity;
if (!(resp->enabled & PD_CTRL_RESP_ENABLED_CONNECTED))
@@ -358,9 +374,7 @@ static int cros_typec_probe(struct platform_device *pdev)
return 0;
unregister_ports:
- for (i = 0; i < typec->num_ports; i++)
- if (typec->ports[i])
- typec_unregister_port(typec->ports[i]);
+ cros_unregister_ports(typec);
return ret;
}
--
2.26.0.110.g2183baf09c-goog
Hi Prashant,
Thanks for your patches.
On 10/4/20 2:23, Prashant Malani wrote:
> The following series adds port-partner registration when
> connects/disconnects are detected. In order to do that, we also register
> a listener for the cros-usbpd-notifier, which will inform the driver of
> EC PD events. While we are here, separate out the Type C port data
> objects into a separate struct.
>
> v2: https://lkml.org/lkml/2020/4/9/861
> v1: https://lkml.org/lkml/2020/4/6/1155
>
> Changes in v3:
> - Fixed trivial Kconfig dependency ordering error.
>
> Prashant Malani (3):
> platform/chrome: typec: Use notifier for updates
> platform/chrome: typec: Add struct for port data
> platform/chrome: typec: Register port partner
>
> drivers/platform/chrome/Kconfig | 1 +
> drivers/platform/chrome/cros_ec_typec.c | 119 ++++++++++++++++++++----
> 2 files changed, 103 insertions(+), 17 deletions(-)
>
Would be nice have the Heikki review on those.
Thanks,
Enric
Thanks for the review Enric,
On Tue, Apr 14, 2020 at 7:08 AM Enric Balletbo i Serra
<[email protected]> wrote:
>
> Hi Prashant,
>
> Thank you for your patch.
>
> On 10/4/20 2:23, Prashant Malani wrote:
> > Register (and unregister) the port partner when a connect (and
> > disconnect) is detected.
> >
> > Co-developed-by: Jon Flatley <[email protected]>
> > Signed-off-by: Prashant Malani <[email protected]>
> > ---
> >
> > Changes in v3:
> > - No changes.
> >
> > Changes in v2:
> > - Fixed error pointer return value.
> >
> > drivers/platform/chrome/cros_ec_typec.c | 48 +++++++++++++++++++++++++
> > 1 file changed, 48 insertions(+)
> >
> > diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> > index 56ded09a60ffb..304e0b20f279b 100644
> > --- a/drivers/platform/chrome/cros_ec_typec.c
> > +++ b/drivers/platform/chrome/cros_ec_typec.c
> > @@ -22,6 +22,9 @@ struct cros_typec_port {
> > struct typec_port *port;
> > /* Initial capabilities for the port. */
> > struct typec_capability caps;
> > + struct typec_partner *partner;
> > + /* Port partner PD identity info. */
> > + struct usb_pd_identity p_identity;
> > };
> >
> > /* Platform-specific data for the Chrome OS EC Type C controller. */
> > @@ -190,6 +193,30 @@ static int cros_typec_ec_command(struct cros_typec_data *typec,
> > return ret;
> > }
> >
> > +static int cros_typec_add_partner(struct cros_typec_data *typec, int port_num,
> > + bool pd_en)
> > +{
> > + struct cros_typec_port *port = typec->ports[port_num];
> > + struct typec_partner_desc p_desc = {
> > + .usb_pd = pd_en,
> > + };
> > + int ret = 0;
> > +
> > + /*
> > + * Fill an initial PD identity, which will then be updated with info
> > + * from the EC.
> > + */
> > + p_desc.identity = &port->p_identity;
> > +
> > + port->partner = typec_register_partner(port->port, &p_desc);
> > + if (IS_ERR_OR_NULL(port->partner)) {
> > + ret = PTR_ERR(port->partner);
>
> If you're checking for IS_ERR_OR_NULL that means that port->partner can be NULL,
> so PTR_ERR(NULL) is 0 returning something that you don't really want.
>
> But looking at the typec_register_partner, NULL is not an option as returns a
> handle to the partner on success or ERR_PTR on failure. So, the code should just do:
>
> if (IS_ERR(port->partner))
> return PTR_ERR(port->partner);
Will update the patch to do this on the next version, thanks.
>
> And AFAICS you don't need to set port->partner to NULL.
>
> Can you double check this?
I think we will need it, since we check for it in
cros_typec_set_port_params_v1() to avoid double registration of a
partner:
+ /* Register/remove partners when a connect/disconnect occurs. */
+ if (resp->enabled & PD_CTRL_RESP_ENABLED_CONNECTED) {
+ if (typec->ports[port_num]->partner)
+ return;
So if it is not set to NULL on error earlier, the registration won't
take place here (in case another PD notification comes in while the
PD_CTRL_RESP_ENABLED_CONNECTED bit is still set). Kindly correct me if
this understanding is incorrect.
Best regards,
-Prashant
>
> Thanks,
> Enric
>
> > + port->partner = NULL;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > static void cros_typec_set_port_params_v0(struct cros_typec_data *typec,
> > int port_num, struct ec_response_usb_pd_control *resp)
> > {
> > @@ -212,6 +239,8 @@ static void cros_typec_set_port_params_v1(struct cros_typec_data *typec,
> > {
> > struct typec_port *port = typec->ports[port_num]->port;
> > enum typec_orientation polarity;
> > + bool pd_en;
> > + int ret;
> >
> > if (!(resp->enabled & PD_CTRL_RESP_ENABLED_CONNECTED))
> > polarity = TYPEC_ORIENTATION_NONE;
> > @@ -226,6 +255,25 @@ static void cros_typec_set_port_params_v1(struct cros_typec_data *typec,
> > TYPEC_SOURCE : TYPEC_SINK);
> > typec_set_vconn_role(port, resp->role & PD_CTRL_RESP_ROLE_VCONN ?
> > TYPEC_SOURCE : TYPEC_SINK);
> > +
> > + /* Register/remove partners when a connect/disconnect occurs. */
> > + if (resp->enabled & PD_CTRL_RESP_ENABLED_CONNECTED) {
> > + if (typec->ports[port_num]->partner)
> > + return;
> > +
> > + pd_en = resp->enabled & PD_CTRL_RESP_ENABLED_PD_CAPABLE;
> > + ret = cros_typec_add_partner(typec, port_num, pd_en);
> > + if (!ret)
> > + dev_warn(typec->dev,
> > + "Failed to register partner on port: %d\n",
> > + port_num);
> > + } else {
> > + if (!typec->ports[port_num]->partner)
> > + return;
> > +
> > + typec_unregister_partner(typec->ports[port_num]->partner);
> > + typec->ports[port_num]->partner = NULL;
> > + }
> > }
> >
> > static int cros_typec_port_update(struct cros_typec_data *typec, int port_num)
> >
On Thu, Apr 09, 2020 at 05:23:16PM -0700, Prashant Malani wrote:
> Add a separate struct for storing port data, including Type C connector
> class struct pointers and caps.
>
> Signed-off-by: Prashant Malani <[email protected]>
Reviewed-by: Heikki Krogerus <[email protected]>
> ---
>
> Changes in v3:
> - No changes.
>
> Changes in v2:
> - Removed unrequired devm_kfree() call.
>
> drivers/platform/chrome/cros_ec_typec.c | 48 ++++++++++++++++---------
> 1 file changed, 31 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> index d444dd7422a2a..56ded09a60ffb 100644
> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -17,6 +17,13 @@
>
> #define DRV_NAME "cros-ec-typec"
>
> +/* Per port data. */
> +struct cros_typec_port {
> + struct typec_port *port;
> + /* Initial capabilities for the port. */
> + struct typec_capability caps;
> +};
> +
> /* Platform-specific data for the Chrome OS EC Type C controller. */
> struct cros_typec_data {
> struct device *dev;
> @@ -24,9 +31,7 @@ struct cros_typec_data {
> int num_ports;
> unsigned int cmd_ver;
> /* Array of ports, indexed by port number. */
> - struct typec_port *ports[EC_USB_PD_MAX_PORTS];
> - /* Initial capabilities for each port. */
> - struct typec_capability *caps[EC_USB_PD_MAX_PORTS];
> + struct cros_typec_port *ports[EC_USB_PD_MAX_PORTS];
> struct notifier_block nb;
> };
>
> @@ -76,14 +81,25 @@ static int cros_typec_parse_port_props(struct typec_capability *cap,
> return 0;
> }
>
> +static void cros_unregister_ports(struct cros_typec_data *typec)
> +{
> + int i;
> +
> + for (i = 0; i < typec->num_ports; i++) {
> + if (!typec->ports[i])
> + continue;
> + typec_unregister_port(typec->ports[i]->port);
> + }
> +}
> +
> static int cros_typec_init_ports(struct cros_typec_data *typec)
> {
> struct device *dev = typec->dev;
> struct typec_capability *cap;
> struct fwnode_handle *fwnode;
> + struct cros_typec_port *cros_port;
> const char *port_prop;
> int ret;
> - int i;
> int nports;
> u32 port_num = 0;
>
> @@ -115,22 +131,23 @@ static int cros_typec_init_ports(struct cros_typec_data *typec)
>
> dev_dbg(dev, "Registering port %d\n", port_num);
>
> - cap = devm_kzalloc(dev, sizeof(*cap), GFP_KERNEL);
> - if (!cap) {
> + cros_port = devm_kzalloc(dev, sizeof(*cros_port), GFP_KERNEL);
> + if (!cros_port) {
> ret = -ENOMEM;
> goto unregister_ports;
> }
>
> - typec->caps[port_num] = cap;
> + typec->ports[port_num] = cros_port;
> + cap = &cros_port->caps;
>
> ret = cros_typec_parse_port_props(cap, fwnode, dev);
> if (ret < 0)
> goto unregister_ports;
>
> - typec->ports[port_num] = typec_register_port(dev, cap);
> - if (IS_ERR(typec->ports[port_num])) {
> + cros_port->port = typec_register_port(dev, cap);
> + if (IS_ERR(cros_port->port)) {
> dev_err(dev, "Failed to register port %d\n", port_num);
> - ret = PTR_ERR(typec->ports[port_num]);
> + ret = PTR_ERR(cros_port->port);
> goto unregister_ports;
> }
> }
> @@ -138,8 +155,7 @@ static int cros_typec_init_ports(struct cros_typec_data *typec)
> return 0;
>
> unregister_ports:
> - for (i = 0; i < typec->num_ports; i++)
> - typec_unregister_port(typec->ports[i]);
> + cros_unregister_ports(typec);
> return ret;
> }
>
> @@ -177,7 +193,7 @@ static int cros_typec_ec_command(struct cros_typec_data *typec,
> static void cros_typec_set_port_params_v0(struct cros_typec_data *typec,
> int port_num, struct ec_response_usb_pd_control *resp)
> {
> - struct typec_port *port = typec->ports[port_num];
> + struct typec_port *port = typec->ports[port_num]->port;
> enum typec_orientation polarity;
>
> if (!resp->enabled)
> @@ -194,7 +210,7 @@ static void cros_typec_set_port_params_v0(struct cros_typec_data *typec,
> static void cros_typec_set_port_params_v1(struct cros_typec_data *typec,
> int port_num, struct ec_response_usb_pd_control_v1 *resp)
> {
> - struct typec_port *port = typec->ports[port_num];
> + struct typec_port *port = typec->ports[port_num]->port;
> enum typec_orientation polarity;
>
> if (!(resp->enabled & PD_CTRL_RESP_ENABLED_CONNECTED))
> @@ -358,9 +374,7 @@ static int cros_typec_probe(struct platform_device *pdev)
> return 0;
>
> unregister_ports:
> - for (i = 0; i < typec->num_ports; i++)
> - if (typec->ports[i])
> - typec_unregister_port(typec->ports[i]);
> + cros_unregister_ports(typec);
> return ret;
> }
>
> --
> 2.26.0.110.g2183baf09c-goog
thanks,
--
heikki
On Thu, Apr 09, 2020 at 05:23:17PM -0700, Prashant Malani wrote:
> Register (and unregister) the port partner when a connect (and
> disconnect) is detected.
>
> Co-developed-by: Jon Flatley <[email protected]>
> Signed-off-by: Prashant Malani <[email protected]>
Reviewed-by: Heikki Krogerus <[email protected]>
> ---
>
> Changes in v3:
> - No changes.
>
> Changes in v2:
> - Fixed error pointer return value.
>
> drivers/platform/chrome/cros_ec_typec.c | 48 +++++++++++++++++++++++++
> 1 file changed, 48 insertions(+)
>
> diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> index 56ded09a60ffb..304e0b20f279b 100644
> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -22,6 +22,9 @@ struct cros_typec_port {
> struct typec_port *port;
> /* Initial capabilities for the port. */
> struct typec_capability caps;
> + struct typec_partner *partner;
> + /* Port partner PD identity info. */
> + struct usb_pd_identity p_identity;
> };
>
> /* Platform-specific data for the Chrome OS EC Type C controller. */
> @@ -190,6 +193,30 @@ static int cros_typec_ec_command(struct cros_typec_data *typec,
> return ret;
> }
>
> +static int cros_typec_add_partner(struct cros_typec_data *typec, int port_num,
> + bool pd_en)
> +{
> + struct cros_typec_port *port = typec->ports[port_num];
> + struct typec_partner_desc p_desc = {
> + .usb_pd = pd_en,
> + };
> + int ret = 0;
> +
> + /*
> + * Fill an initial PD identity, which will then be updated with info
> + * from the EC.
> + */
> + p_desc.identity = &port->p_identity;
> +
> + port->partner = typec_register_partner(port->port, &p_desc);
> + if (IS_ERR_OR_NULL(port->partner)) {
> + ret = PTR_ERR(port->partner);
> + port->partner = NULL;
> + }
> +
> + return ret;
> +}
> +
> static void cros_typec_set_port_params_v0(struct cros_typec_data *typec,
> int port_num, struct ec_response_usb_pd_control *resp)
> {
> @@ -212,6 +239,8 @@ static void cros_typec_set_port_params_v1(struct cros_typec_data *typec,
> {
> struct typec_port *port = typec->ports[port_num]->port;
> enum typec_orientation polarity;
> + bool pd_en;
> + int ret;
>
> if (!(resp->enabled & PD_CTRL_RESP_ENABLED_CONNECTED))
> polarity = TYPEC_ORIENTATION_NONE;
> @@ -226,6 +255,25 @@ static void cros_typec_set_port_params_v1(struct cros_typec_data *typec,
> TYPEC_SOURCE : TYPEC_SINK);
> typec_set_vconn_role(port, resp->role & PD_CTRL_RESP_ROLE_VCONN ?
> TYPEC_SOURCE : TYPEC_SINK);
> +
> + /* Register/remove partners when a connect/disconnect occurs. */
> + if (resp->enabled & PD_CTRL_RESP_ENABLED_CONNECTED) {
> + if (typec->ports[port_num]->partner)
> + return;
> +
> + pd_en = resp->enabled & PD_CTRL_RESP_ENABLED_PD_CAPABLE;
> + ret = cros_typec_add_partner(typec, port_num, pd_en);
> + if (!ret)
> + dev_warn(typec->dev,
> + "Failed to register partner on port: %d\n",
> + port_num);
> + } else {
> + if (!typec->ports[port_num]->partner)
> + return;
> +
> + typec_unregister_partner(typec->ports[port_num]->partner);
> + typec->ports[port_num]->partner = NULL;
> + }
> }
>
> static int cros_typec_port_update(struct cros_typec_data *typec, int port_num)
> --
> 2.26.0.110.g2183baf09c-goog
thanks,
--
heikki
On Thu, Apr 09, 2020 at 05:23:15PM -0700, Prashant Malani wrote:
> Register a listener for the cros-usbpd-notifier, and update port state
> when a notification comes in.
>
> Signed-off-by: Prashant Malani <[email protected]>
Reviewed-by: Heikki Krogerus <[email protected]>
> ---
>
> Changes in v3:
> - Fixed ordering issue in Kconfig dependency.
>
> Changes in v2:
> - Added Kconfig dependency to CROS_USBPD_NOTIFY.
> - Fixed spacing error.
> - Removed superfluous devm_warn() call.
>
> drivers/platform/chrome/Kconfig | 1 +
> drivers/platform/chrome/cros_ec_typec.c | 23 +++++++++++++++++++++++
> 2 files changed, 24 insertions(+)
>
> diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
> index 03ea5129ed0c3..a484ab2c91ff0 100644
> --- a/drivers/platform/chrome/Kconfig
> +++ b/drivers/platform/chrome/Kconfig
> @@ -217,6 +217,7 @@ config CROS_EC_SYSFS
> config CROS_EC_TYPEC
> tristate "ChromeOS EC Type-C Connector Control"
> depends on MFD_CROS_EC_DEV && TYPEC
> + depends on CROS_USBPD_NOTIFY
> default MFD_CROS_EC_DEV
> help
> If you say Y here, you get support for accessing Type C connector
> diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> index 874269c070739..d444dd7422a2a 100644
> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -11,6 +11,7 @@
> #include <linux/of.h>
> #include <linux/platform_data/cros_ec_commands.h>
> #include <linux/platform_data/cros_ec_proto.h>
> +#include <linux/platform_data/cros_usbpd_notify.h>
> #include <linux/platform_device.h>
> #include <linux/usb/typec.h>
>
> @@ -26,6 +27,7 @@ struct cros_typec_data {
> struct typec_port *ports[EC_USB_PD_MAX_PORTS];
> /* Initial capabilities for each port. */
> struct typec_capability *caps[EC_USB_PD_MAX_PORTS];
> + struct notifier_block nb;
> };
>
> static int cros_typec_parse_port_props(struct typec_capability *cap,
> @@ -272,6 +274,22 @@ static int cros_typec_get_cmd_version(struct cros_typec_data *typec)
> return 0;
> }
>
> +static int cros_ec_typec_event(struct notifier_block *nb,
> + unsigned long host_event, void *_notify)
> +{
> + struct cros_typec_data *typec = container_of(nb, struct cros_typec_data,
> + nb);
> + int ret, i;
> +
> + for (i = 0; i < typec->num_ports; i++) {
> + ret = cros_typec_port_update(typec, i);
> + if (ret < 0)
> + dev_warn(typec->dev, "Update failed for port: %d\n", i);
> + }
> +
> + return NOTIFY_OK;
> +}
> +
> #ifdef CONFIG_ACPI
> static const struct acpi_device_id cros_typec_acpi_id[] = {
> { "GOOG0014", 0 },
> @@ -332,6 +350,11 @@ static int cros_typec_probe(struct platform_device *pdev)
> goto unregister_ports;
> }
>
> + typec->nb.notifier_call = cros_ec_typec_event;
> + ret = cros_usbpd_register_notify(&typec->nb);
> + if (ret < 0)
> + goto unregister_ports;
> +
> return 0;
>
> unregister_ports:
> --
> 2.26.0.110.g2183baf09c-goog
thanks,
--
heikki
Hi Prashant,
Thank you for your patch.
On 10/4/20 2:23, Prashant Malani wrote:
> Register (and unregister) the port partner when a connect (and
> disconnect) is detected.
>
> Co-developed-by: Jon Flatley <[email protected]>
> Signed-off-by: Prashant Malani <[email protected]>
> ---
>
> Changes in v3:
> - No changes.
>
> Changes in v2:
> - Fixed error pointer return value.
>
> drivers/platform/chrome/cros_ec_typec.c | 48 +++++++++++++++++++++++++
> 1 file changed, 48 insertions(+)
>
> diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> index 56ded09a60ffb..304e0b20f279b 100644
> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -22,6 +22,9 @@ struct cros_typec_port {
> struct typec_port *port;
> /* Initial capabilities for the port. */
> struct typec_capability caps;
> + struct typec_partner *partner;
> + /* Port partner PD identity info. */
> + struct usb_pd_identity p_identity;
> };
>
> /* Platform-specific data for the Chrome OS EC Type C controller. */
> @@ -190,6 +193,30 @@ static int cros_typec_ec_command(struct cros_typec_data *typec,
> return ret;
> }
>
> +static int cros_typec_add_partner(struct cros_typec_data *typec, int port_num,
> + bool pd_en)
> +{
> + struct cros_typec_port *port = typec->ports[port_num];
> + struct typec_partner_desc p_desc = {
> + .usb_pd = pd_en,
> + };
> + int ret = 0;
> +
> + /*
> + * Fill an initial PD identity, which will then be updated with info
> + * from the EC.
> + */
> + p_desc.identity = &port->p_identity;
> +
> + port->partner = typec_register_partner(port->port, &p_desc);
> + if (IS_ERR_OR_NULL(port->partner)) {
> + ret = PTR_ERR(port->partner);
If you're checking for IS_ERR_OR_NULL that means that port->partner can be NULL,
so PTR_ERR(NULL) is 0 returning something that you don't really want.
But looking at the typec_register_partner, NULL is not an option as returns a
handle to the partner on success or ERR_PTR on failure. So, the code should just do:
if (IS_ERR(port->partner))
return PTR_ERR(port->partner);
And AFAICS you don't need to set port->partner to NULL.
Can you double check this?
Thanks,
Enric
> + port->partner = NULL;
> + }
> +
> + return ret;
> +}
> +
> static void cros_typec_set_port_params_v0(struct cros_typec_data *typec,
> int port_num, struct ec_response_usb_pd_control *resp)
> {
> @@ -212,6 +239,8 @@ static void cros_typec_set_port_params_v1(struct cros_typec_data *typec,
> {
> struct typec_port *port = typec->ports[port_num]->port;
> enum typec_orientation polarity;
> + bool pd_en;
> + int ret;
>
> if (!(resp->enabled & PD_CTRL_RESP_ENABLED_CONNECTED))
> polarity = TYPEC_ORIENTATION_NONE;
> @@ -226,6 +255,25 @@ static void cros_typec_set_port_params_v1(struct cros_typec_data *typec,
> TYPEC_SOURCE : TYPEC_SINK);
> typec_set_vconn_role(port, resp->role & PD_CTRL_RESP_ROLE_VCONN ?
> TYPEC_SOURCE : TYPEC_SINK);
> +
> + /* Register/remove partners when a connect/disconnect occurs. */
> + if (resp->enabled & PD_CTRL_RESP_ENABLED_CONNECTED) {
> + if (typec->ports[port_num]->partner)
> + return;
> +
> + pd_en = resp->enabled & PD_CTRL_RESP_ENABLED_PD_CAPABLE;
> + ret = cros_typec_add_partner(typec, port_num, pd_en);
> + if (!ret)
> + dev_warn(typec->dev,
> + "Failed to register partner on port: %d\n",
> + port_num);
> + } else {
> + if (!typec->ports[port_num]->partner)
> + return;
> +
> + typec_unregister_partner(typec->ports[port_num]->partner);
> + typec->ports[port_num]->partner = NULL;
> + }
> }
>
> static int cros_typec_port_update(struct cros_typec_data *typec, int port_num)
>