All of lore.kernel.org
 help / color / mirror / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: Heikki Krogerus <heikki.krogerus@linux.intel.com>,
	Hans de Goede <hdegoede@redhat.com>
Cc: linux-usb@vger.kernel.org
Subject: Re: [PATCH 5/7] usb: typec: tps6598x: Start using struct typec_operations
Date: Tue, 1 Oct 2019 06:34:14 -0700	[thread overview]
Message-ID: <91ed071d-5b67-bbb2-8640-8d2680d3e3c2@roeck-us.net> (raw)
In-Reply-To: <20191001094858.68643-6-heikki.krogerus@linux.intel.com>

On 10/1/19 2:48 AM, Heikki Krogerus wrote:
> Supplying the operation callbacks as part of a struct
> typec_operations instead of as part of struct
> typec_capability during port registration. After this there
> is not need to keep the capabilities stored anywhere in the
> driver.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>   drivers/usb/typec/tps6598x.c | 49 +++++++++++++++++++-----------------
>   1 file changed, 26 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/usb/typec/tps6598x.c b/drivers/usb/typec/tps6598x.c
> index a38d1409f15b..0698addd1185 100644
> --- a/drivers/usb/typec/tps6598x.c
> +++ b/drivers/usb/typec/tps6598x.c
> @@ -94,7 +94,6 @@ struct tps6598x {
>   	struct typec_port *port;
>   	struct typec_partner *partner;
>   	struct usb_pd_identity partner_identity;
> -	struct typec_capability typec_cap;
>   };
>   
>   /*
> @@ -307,11 +306,10 @@ static int tps6598x_exec_cmd(struct tps6598x *tps, const char *cmd,
>   	return 0;
>   }
>   
> -static int
> -tps6598x_dr_set(const struct typec_capability *cap, enum typec_data_role role)
> +static int tps6598x_dr_set(struct typec_port *port, enum typec_data_role role)
>   {
> -	struct tps6598x *tps = container_of(cap, struct tps6598x, typec_cap);
>   	const char *cmd = (role == TYPEC_DEVICE) ? "SWUF" : "SWDF";
> +	struct tps6598x *tps = typec_get_drvdata(port);
>   	u32 status;
>   	int ret;
>   
> @@ -338,11 +336,10 @@ tps6598x_dr_set(const struct typec_capability *cap, enum typec_data_role role)
>   	return ret;
>   }
>   
> -static int
> -tps6598x_pr_set(const struct typec_capability *cap, enum typec_role role)
> +static int tps6598x_pr_set(struct typec_port *port, enum typec_role role)
>   {
> -	struct tps6598x *tps = container_of(cap, struct tps6598x, typec_cap);
>   	const char *cmd = (role == TYPEC_SINK) ? "SWSk" : "SWSr";
> +	struct tps6598x *tps = typec_get_drvdata(port);
>   	u32 status;
>   	int ret;
>   
> @@ -369,6 +366,11 @@ tps6598x_pr_set(const struct typec_capability *cap, enum typec_role role)
>   	return ret;
>   }
>   
> +static const struct typec_operations tps6598x_ops = {
> +	.dr_set = tps6598x_dr_set,
> +	.pr_set = tps6598x_pr_set,
> +};
> +
>   static irqreturn_t tps6598x_interrupt(int irq, void *data)
>   {
>   	struct tps6598x *tps = data;
> @@ -448,6 +450,7 @@ static const struct regmap_config tps6598x_regmap_config = {
>   
>   static int tps6598x_probe(struct i2c_client *client)
>   {
> +	struct typec_capability typec_cap = { };
>   	struct tps6598x *tps;
>   	u32 status;
>   	u32 conf;
> @@ -492,40 +495,40 @@ static int tps6598x_probe(struct i2c_client *client)
>   	if (ret < 0)
>   		return ret;
>   
> -	tps->typec_cap.revision = USB_TYPEC_REV_1_2;
> -	tps->typec_cap.pd_revision = 0x200;
> -	tps->typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
> -	tps->typec_cap.pr_set = tps6598x_pr_set;
> -	tps->typec_cap.dr_set = tps6598x_dr_set;
> +	typec_cap.revision = USB_TYPEC_REV_1_2;
> +	typec_cap.pd_revision = 0x200;
> +	typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;

Nitpick:
	struct typec_capability typec_cap = {
		.revision = USB_TYPEC_REV_1_2,
		.pd_revision = 0x200,
		.prefer_role = TYPEC_NO_PREFERRED_ROLE,
	};

Your call, though.

> +	typec_cap.driver_data = tps;
> +	typec_cap.ops = &tps6598x_ops;
>   
>   	switch (TPS_SYSCONF_PORTINFO(conf)) {
>   	case TPS_PORTINFO_SINK_ACCESSORY:
>   	case TPS_PORTINFO_SINK:
> -		tps->typec_cap.type = TYPEC_PORT_SNK;
> -		tps->typec_cap.data = TYPEC_PORT_UFP;
> +		typec_cap.type = TYPEC_PORT_SNK;
> +		typec_cap.data = TYPEC_PORT_UFP;
>   		break;
>   	case TPS_PORTINFO_DRP_UFP_DRD:
>   	case TPS_PORTINFO_DRP_DFP_DRD:
> -		tps->typec_cap.type = TYPEC_PORT_DRP;
> -		tps->typec_cap.data = TYPEC_PORT_DRD;
> +		typec_cap.type = TYPEC_PORT_DRP;
> +		typec_cap.data = TYPEC_PORT_DRD;
>   		break;
>   	case TPS_PORTINFO_DRP_UFP:
> -		tps->typec_cap.type = TYPEC_PORT_DRP;
> -		tps->typec_cap.data = TYPEC_PORT_UFP;
> +		typec_cap.type = TYPEC_PORT_DRP;
> +		typec_cap.data = TYPEC_PORT_UFP;
>   		break;
>   	case TPS_PORTINFO_DRP_DFP:
> -		tps->typec_cap.type = TYPEC_PORT_DRP;
> -		tps->typec_cap.data = TYPEC_PORT_DFP;
> +		typec_cap.type = TYPEC_PORT_DRP;
> +		typec_cap.data = TYPEC_PORT_DFP;
>   		break;
>   	case TPS_PORTINFO_SOURCE:
> -		tps->typec_cap.type = TYPEC_PORT_SRC;
> -		tps->typec_cap.data = TYPEC_PORT_DFP;
> +		typec_cap.type = TYPEC_PORT_SRC;
> +		typec_cap.data = TYPEC_PORT_DFP;
>   		break;
>   	default:
>   		return -ENODEV;
>   	}
>   
> -	tps->port = typec_register_port(&client->dev, &tps->typec_cap);
> +	tps->port = typec_register_port(&client->dev, &typec_cap);
>   	if (IS_ERR(tps->port))
>   		return PTR_ERR(tps->port);
>   
> 


  reply	other threads:[~2019-10-01 13:34 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-01  9:48 [PATCH 0/7] usb: typec: Small API improvement Heikki Krogerus
2019-10-01  9:48 ` [PATCH 1/7] usb: typec: Copy everything from struct typec_capability during registration Heikki Krogerus
2019-10-01 13:08   ` Guenter Roeck
2019-10-02 16:06     ` Heikki Krogerus
2019-10-02 16:36       ` Guenter Roeck
2019-10-02 18:29         ` Heikki Krogerus
2019-10-03  3:45           ` Guenter Roeck
2019-10-03  8:03             ` Heikki Krogerus
2019-10-02 19:16       ` Heikki Krogerus
2019-10-03  3:51         ` Guenter Roeck
2019-10-03 13:29           ` Heikki Krogerus
2019-10-01  9:48 ` [PATCH 2/7] usb: typec: Introduce typec_get_drvdata() Heikki Krogerus
2019-10-01  9:48 ` [PATCH 3/7] usb: typec: Separate the operations vector Heikki Krogerus
2019-10-01 13:22   ` Guenter Roeck
2019-10-04  8:45     ` Heikki Krogerus
2019-10-01  9:48 ` [PATCH 4/7] usb: typec: tcpm: Start using struct typec_operations Heikki Krogerus
2019-10-01 13:30   ` Guenter Roeck
2019-10-04  8:46     ` Heikki Krogerus
2019-10-01  9:48 ` [PATCH 5/7] usb: typec: tps6598x: " Heikki Krogerus
2019-10-01 13:34   ` Guenter Roeck [this message]
2019-10-01 13:35   ` Guenter Roeck
2019-10-04  8:49     ` Heikki Krogerus
2019-10-01  9:48 ` [PATCH 6/7] usb: typec: ucsi: " Heikki Krogerus
2019-10-01 13:35   ` Guenter Roeck
2019-10-01  9:48 ` [PATCH 7/7] usb: typec: Remove the callback members from struct typec_capability Heikki Krogerus
2019-10-01 13:37   ` 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=91ed071d-5b67-bbb2-8640-8d2680d3e3c2@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=hdegoede@redhat.com \
    --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.