diff mbox series

[v3,10/18] usb: typec: hd3ss3220: Give the connector fwnode to the port device

Message ID 20191025082324.75731-11-heikki.krogerus@linux.intel.com (mailing list archive)
State Superseded
Headers show
Series usb: typec: API improvements | expand

Commit Message

Heikki Krogerus Oct. 25, 2019, 8:23 a.m. UTC 
The driver already finds the node in order to get reference
to the USB role switch.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Tested-by: Biju Das <biju.das@bp.renesas.com>
---
 drivers/usb/typec/hd3ss3220.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

Comments

Guenter Roeck Nov. 2, 2019, 4:11 p.m. UTC | #1 
On 10/25/19 1:23 AM, Heikki Krogerus wrote:
> The driver already finds the node in order to get reference
> to the USB role switch.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Tested-by: Biju Das <biju.das@bp.renesas.com>

The fusb302 driver retains the fwnode and calls fwnode_handle_put()
in the remove function. Assuming this isn't necessary here,

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   drivers/usb/typec/hd3ss3220.c | 12 +++++++++---
>   1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/typec/hd3ss3220.c b/drivers/usb/typec/hd3ss3220.c
> index db09fa0d85f2..323dfa8160ab 100644
> --- a/drivers/usb/typec/hd3ss3220.c
> +++ b/drivers/usb/typec/hd3ss3220.c
> @@ -178,15 +178,17 @@ static int hd3ss3220_probe(struct i2c_client *client,
>   		return -ENODEV;
>   
>   	hd3ss3220->role_sw = fwnode_usb_role_switch_get(connector);
> -	fwnode_handle_put(connector);
> -	if (IS_ERR(hd3ss3220->role_sw))
> -		return PTR_ERR(hd3ss3220->role_sw);
> +	if (IS_ERR(hd3ss3220->role_sw)) {
> +		ret = PTR_ERR(hd3ss3220->role_sw);
> +		goto err_put_fwnode;
> +	}
>   
>   	typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
>   	typec_cap.driver_data = hd3ss3220;
>   	typec_cap.type = TYPEC_PORT_DRP;
>   	typec_cap.data = TYPEC_PORT_DRD;
>   	typec_cap.ops = &hd3ss3220_ops;
> +	typec_cap.fwnode = connector;
>   
>   	hd3ss3220->port = typec_register_port(&client->dev, &typec_cap);
>   	if (IS_ERR(hd3ss3220->port)) {
> @@ -220,6 +222,8 @@ static int hd3ss3220_probe(struct i2c_client *client,
>   	if (ret < 0)
>   		goto err_unreg_port;
>   
> +	fwnode_handle_put(connector);
> +
>   	dev_info(&client->dev, "probed revision=0x%x\n", ret);
>   
>   	return 0;
> @@ -227,6 +231,8 @@ static int hd3ss3220_probe(struct i2c_client *client,
>   	typec_unregister_port(hd3ss3220->port);
>   err_put_role:
>   	usb_role_switch_put(hd3ss3220->role_sw);
> +err_put_fwnode:
> +	fwnode_handle_put(connector);
>   
>   	return ret;
>   }
>
Biju Das Nov. 4, 2019, 7:50 a.m. UTC | #2 
Hi Guenter,

> -----Original Message-----
> From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> Sent: Saturday, November 2, 2019 4:11 PM
> To: Heikki Krogerus <heikki.krogerus@linux.intel.com>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>
> Cc: Ajay Gupta <ajayg@nvidia.com>; linux-usb@vger.kernel.org; Biju Das
> <biju.das@bp.renesas.com>
> Subject: Re: [PATCH v3 10/18] usb: typec: hd3ss3220: Give the connector
> fwnode to the port device
> 
> On 10/25/19 1:23 AM, Heikki Krogerus wrote:
> > The driver already finds the node in order to get reference to the USB
> > role switch.
> >
> > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > Tested-by: Biju Das <biju.das@bp.renesas.com>
> 
> The fusb302 driver retains the fwnode and calls fwnode_handle_put() in the
> remove function. Assuming this isn't necessary here,

It is not required. The purpose is  to get the role switch handle associated with the connecter node.
Once you get the role switch handle,  you don't need the connector handle any more. 

Cheers,
Biju

> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> 
> > ---
> >   drivers/usb/typec/hd3ss3220.c | 12 +++++++++---
> >   1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/usb/typec/hd3ss3220.c
> > b/drivers/usb/typec/hd3ss3220.c index db09fa0d85f2..323dfa8160ab
> > 100644
> > --- a/drivers/usb/typec/hd3ss3220.c
> > +++ b/drivers/usb/typec/hd3ss3220.c
> > @@ -178,15 +178,17 @@ static int hd3ss3220_probe(struct i2c_client
> *client,
> >   		return -ENODEV;
> >
> >   	hd3ss3220->role_sw = fwnode_usb_role_switch_get(connector);
> > -	fwnode_handle_put(connector);
> > -	if (IS_ERR(hd3ss3220->role_sw))
> > -		return PTR_ERR(hd3ss3220->role_sw);
> > +	if (IS_ERR(hd3ss3220->role_sw)) {
> > +		ret = PTR_ERR(hd3ss3220->role_sw);
> > +		goto err_put_fwnode;
> > +	}
> >
> >   	typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
> >   	typec_cap.driver_data = hd3ss3220;
> >   	typec_cap.type = TYPEC_PORT_DRP;
> >   	typec_cap.data = TYPEC_PORT_DRD;
> >   	typec_cap.ops = &hd3ss3220_ops;
> > +	typec_cap.fwnode = connector;
> >
> >   	hd3ss3220->port = typec_register_port(&client->dev, &typec_cap);
> >   	if (IS_ERR(hd3ss3220->port)) {
> > @@ -220,6 +222,8 @@ static int hd3ss3220_probe(struct i2c_client *client,
> >   	if (ret < 0)
> >   		goto err_unreg_port;
> >
> > +	fwnode_handle_put(connector);
> > +
> >   	dev_info(&client->dev, "probed revision=0x%x\n", ret);
> >
> >   	return 0;
> > @@ -227,6 +231,8 @@ static int hd3ss3220_probe(struct i2c_client *client,
> >   	typec_unregister_port(hd3ss3220->port);
> >   err_put_role:
> >   	usb_role_switch_put(hd3ss3220->role_sw);
> > +err_put_fwnode:
> > +	fwnode_handle_put(connector);
> >
> >   	return ret;
> >   }
> >
diff mbox series

Patch

diff --git a/drivers/usb/typec/hd3ss3220.c b/drivers/usb/typec/hd3ss3220.c
index db09fa0d85f2..323dfa8160ab 100644
--- a/drivers/usb/typec/hd3ss3220.c
+++ b/drivers/usb/typec/hd3ss3220.c
@@ -178,15 +178,17 @@  static int hd3ss3220_probe(struct i2c_client *client,
 		return -ENODEV;
 
 	hd3ss3220->role_sw = fwnode_usb_role_switch_get(connector);
-	fwnode_handle_put(connector);
-	if (IS_ERR(hd3ss3220->role_sw))
-		return PTR_ERR(hd3ss3220->role_sw);
+	if (IS_ERR(hd3ss3220->role_sw)) {
+		ret = PTR_ERR(hd3ss3220->role_sw);
+		goto err_put_fwnode;
+	}
 
 	typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
 	typec_cap.driver_data = hd3ss3220;
 	typec_cap.type = TYPEC_PORT_DRP;
 	typec_cap.data = TYPEC_PORT_DRD;
 	typec_cap.ops = &hd3ss3220_ops;
+	typec_cap.fwnode = connector;
 
 	hd3ss3220->port = typec_register_port(&client->dev, &typec_cap);
 	if (IS_ERR(hd3ss3220->port)) {
@@ -220,6 +222,8 @@  static int hd3ss3220_probe(struct i2c_client *client,
 	if (ret < 0)
 		goto err_unreg_port;
 
+	fwnode_handle_put(connector);
+
 	dev_info(&client->dev, "probed revision=0x%x\n", ret);
 
 	return 0;
@@ -227,6 +231,8 @@  static int hd3ss3220_probe(struct i2c_client *client,
 	typec_unregister_port(hd3ss3220->port);
 err_put_role:
 	usb_role_switch_put(hd3ss3220->role_sw);
+err_put_fwnode:
+	fwnode_handle_put(connector);
 
 	return ret;
 }