From: Guenter Roeck <linux@roeck-us.net>
To: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: linux-usb@vger.kernel.org
Subject: Re: [PATCH v3 3/9] usb: typec: Separate the operations vector
Date: Tue, 8 Oct 2019 14:38:04 -0700 [thread overview]
Message-ID: <20191008213804.GC16138@roeck-us.net> (raw)
In-Reply-To: <20191008111350.68581-4-heikki.krogerus@linux.intel.com>
On Tue, Oct 08, 2019 at 02:13:44PM +0300, Heikki Krogerus wrote:
> Introducing struct typec_operations which has the same
> callbacks as struct typec_capability. The old callbacks are
> kept for now, but after all users have been converted, they
> will be removed.
>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/usb/typec/class.c | 39 +++++++++++++++++++++++++++++----------
> include/linux/usb/typec.h | 20 ++++++++++++++++++++
> 2 files changed, 49 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 89ffe370e426..11ed3dc6fc49 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -54,6 +54,7 @@ struct typec_port {
>
> const struct typec_capability *orig_cap; /* to be removed */
> const struct typec_capability *cap;
> + const struct typec_operations *ops;
> };
>
> #define to_typec_port(_dev_) container_of(_dev_, struct typec_port, dev)
> @@ -956,7 +957,7 @@ preferred_role_store(struct device *dev, struct device_attribute *attr,
> return -EOPNOTSUPP;
> }
>
> - if (!port->cap->try_role) {
> + if (!port->cap->try_role && (!port->ops || !port->ops->try_role)) {
> dev_dbg(dev, "Setting preferred role not supported\n");
> return -EOPNOTSUPP;
> }
> @@ -969,7 +970,10 @@ preferred_role_store(struct device *dev, struct device_attribute *attr,
> return -EINVAL;
> }
>
> - ret = port->cap->try_role(port->orig_cap, role);
> + if (port->ops && port->ops->try_role)
> + ret = port->ops->try_role(port, role);
> + else
> + ret = port->cap->try_role(port->orig_cap, role);
> if (ret)
> return ret;
>
> @@ -1000,7 +1004,7 @@ static ssize_t data_role_store(struct device *dev,
> struct typec_port *port = to_typec_port(dev);
> int ret;
>
> - if (!port->cap->dr_set) {
> + if (!port->cap->dr_set && (!port->ops || !port->ops->dr_set)) {
> dev_dbg(dev, "data role swapping not supported\n");
> return -EOPNOTSUPP;
> }
> @@ -1015,7 +1019,10 @@ static ssize_t data_role_store(struct device *dev,
> goto unlock_and_ret;
> }
>
> - ret = port->cap->dr_set(port->orig_cap, ret);
> + if (port->ops && port->ops->dr_set)
> + ret = port->ops->dr_set(port, ret);
> + else
> + ret = port->cap->dr_set(port->orig_cap, ret);
> if (ret)
> goto unlock_and_ret;
>
> @@ -1050,7 +1057,7 @@ static ssize_t power_role_store(struct device *dev,
> return -EOPNOTSUPP;
> }
>
> - if (!port->cap->pr_set) {
> + if (!port->cap->pr_set && (!port->ops || !port->ops->pr_set)) {
> dev_dbg(dev, "power role swapping not supported\n");
> return -EOPNOTSUPP;
> }
> @@ -1072,7 +1079,10 @@ static ssize_t power_role_store(struct device *dev,
> goto unlock_and_ret;
> }
>
> - ret = port->cap->pr_set(port->orig_cap, ret);
> + if (port->ops && port->ops->dr_set)
> + ret = port->ops->pr_set(port, ret);
> + else
> + ret = port->cap->pr_set(port->orig_cap, ret);
> if (ret)
> goto unlock_and_ret;
>
> @@ -1103,7 +1113,8 @@ port_type_store(struct device *dev, struct device_attribute *attr,
> int ret;
> enum typec_port_type type;
>
> - if (!port->cap->port_type_set || port->cap->type != TYPEC_PORT_DRP) {
> + if (port->cap->type != TYPEC_PORT_DRP || (!port->cap->port_type_set &&
> + (!port->ops || !port->ops->port_type_set))) {
> dev_dbg(dev, "changing port type not supported\n");
> return -EOPNOTSUPP;
> }
> @@ -1120,7 +1131,10 @@ port_type_store(struct device *dev, struct device_attribute *attr,
> goto unlock_and_ret;
> }
>
> - ret = port->cap->port_type_set(port->orig_cap, type);
> + if (port->ops && port->ops->port_type_set)
> + ret = port->ops->port_type_set(port, type);
> + else
> + ret = port->cap->port_type_set(port->orig_cap, type);
> if (ret)
> goto unlock_and_ret;
>
> @@ -1176,7 +1190,7 @@ static ssize_t vconn_source_store(struct device *dev,
> return -EOPNOTSUPP;
> }
>
> - if (!port->cap->vconn_set) {
> + if (!port->cap->vconn_set && (!port->ops || !port->ops->vconn_set)) {
> dev_dbg(dev, "VCONN swapping not supported\n");
> return -EOPNOTSUPP;
> }
> @@ -1185,7 +1199,11 @@ static ssize_t vconn_source_store(struct device *dev,
> if (ret)
> return ret;
>
> - ret = port->cap->vconn_set(port->orig_cap, (enum typec_role)source);
> + if (port->ops && port->ops->vconn_set)
> + ret = port->ops->vconn_set(port, (enum typec_role)source);
> + else
> + ret = port->cap->vconn_set(port->orig_cap,
> + (enum typec_role)source);
> if (ret)
> return ret;
>
> @@ -1591,6 +1609,7 @@ struct typec_port *typec_register_port(struct device *parent,
> mutex_init(&port->port_type_lock);
>
> port->id = id;
> + port->ops = cap->ops;
> port->orig_cap = cap;
> port->port_type = cap->type;
> port->prefer_role = cap->prefer_role;
> diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
> index 8b90cd77331c..c9bef128453b 100644
> --- a/include/linux/usb/typec.h
> +++ b/include/linux/usb/typec.h
> @@ -168,6 +168,23 @@ struct typec_partner_desc {
> struct usb_pd_identity *identity;
> };
>
> +/**
> + * struct typec_operations - USB Type-C Port Operations
> + * @try_role: Set data role preference for DRP port
> + * @dr_set: Set Data Role
> + * @pr_set: Set Power Role
> + * @vconn_set: Source VCONN
> + * @port_type_set: Set port type
> + */
> +struct typec_operations {
> + int (*try_role)(struct typec_port *port, int role);
> + int (*dr_set)(struct typec_port *port, enum typec_data_role role);
> + int (*pr_set)(struct typec_port *port, enum typec_role role);
> + int (*vconn_set)(struct typec_port *port, enum typec_role role);
> + int (*port_type_set)(struct typec_port *port,
> + enum typec_port_type type);
> +};
> +
> /*
> * struct typec_capability - USB Type-C Port Capabilities
> * @type: Supported power role of the port
> @@ -180,6 +197,7 @@ struct typec_partner_desc {
> * @mux: Multiplexer switch for Alternate/Accessory Modes
> * @fwnode: Optional fwnode of the port
> * @driver_data: Private pointer for driver specific info
> + * @ops: Port operations vector
> * @try_role: Set data role preference for DRP port
> * @dr_set: Set Data Role
> * @pr_set: Set Power Role
> @@ -201,6 +219,8 @@ struct typec_capability {
> struct fwnode_handle *fwnode;
> void *driver_data;
>
> + const struct typec_operations *ops;
> +
> int (*try_role)(const struct typec_capability *,
> int role);
>
> --
> 2.23.0
>
next prev parent reply other threads:[~2019-10-08 21:38 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-08 11:13 [PATCH v3 0/9] usb: typec: Small API improvement Heikki Krogerus
2019-10-08 11:13 ` [PATCH v3 1/9] usb: typec: Copy everything from struct typec_capability during registration Heikki Krogerus
2019-10-08 20:44 ` Guenter Roeck
2019-10-09 7:58 ` Heikki Krogerus
2019-10-08 11:13 ` [PATCH v3 2/9] usb: typec: Introduce typec_get_drvdata() Heikki Krogerus
2019-10-08 21:36 ` Guenter Roeck
2019-10-08 11:13 ` [PATCH v3 3/9] usb: typec: Separate the operations vector Heikki Krogerus
2019-10-08 21:38 ` Guenter Roeck [this message]
2019-10-08 11:13 ` [PATCH v3 4/9] usb: typec: tcpm: Start using struct typec_operations Heikki Krogerus
2019-10-08 11:13 ` [PATCH v3 5/9] usb: typec: tps6598x: " Heikki Krogerus
2019-10-08 11:13 ` [PATCH v3 6/9] usb: typec: ucsi: " Heikki Krogerus
2019-10-08 11:13 ` [PATCH v3 7/9] usb: typec: hd3ss3220: " Heikki Krogerus
2019-10-08 21:39 ` Guenter Roeck
2019-10-08 11:13 ` [PATCH v3 8/9] usb: typec: Remove the callback members from struct typec_capability Heikki Krogerus
2019-10-08 21:40 ` Guenter Roeck
2019-10-08 11:13 ` [PATCH v3 9/9] usb: typec: Remove unused " Heikki Krogerus
2019-10-08 21:40 ` Guenter Roeck
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191008213804.GC16138@roeck-us.net \
--to=linux@roeck-us.net \
--cc=heikki.krogerus@linux.intel.com \
--cc=linux-usb@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.