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

[Biju Das <biju.das@bp.renesas.com>]

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;
> >   }
> >