Re: [PATCH v2] usb: typec: tps6598x: Add USB role switching logic

[Heikki Krogerus]

Hi,

On Thu, May 07, 2020 at 10:59:38PM +0100, Bryan O'Donoghue wrote:
> @@ -452,6 +471,7 @@ static int tps6598x_probe(struct i2c_client *client)
>  {
>       struct typec_capability typec_cap = { };
>       struct tps6598x *tps;
> +     struct fwnode_handle *fwnode;
>       u32 status;
>       u32 conf;
>       u32 vid;
> @@ -495,11 +515,21 @@ static int tps6598x_probe(struct i2c_client *client)
>       if (ret < 0)
>               return ret;
>  
> +     fwnode = device_get_named_child_node(&client->dev, "connector");
> +     if (!IS_ERR_OR_NULL(fwnode)) {
> +             tps->role_sw = fwnode_usb_role_switch_get(fwnode);
> +             if (IS_ERR(tps->role_sw)) {
> +                     ret = PTR_ERR(tps->role_sw);
> +                     goto err_fwnode_put;
> +             }
> +     }

        fwnode = device_get_named_child_node(&client->dev, "connector");
        if (IS_ERR(fwnode))
                return PTR_ERR(fwnode);

        tps->role_sw = fwnode_usb_role_switch_get(fwnode);
        if (IS_ERR(tps->role_sw)) {
                ret = PTR_ERR(tps->role_sw);
                goto err_fwnode_put;
        }

>       typec_cap.revision = USB_TYPEC_REV_1_2;
>       typec_cap.pd_revision = 0x200;
>       typec_cap.prefer_role = TYPEC_NO_PREFERRED_ROLE;
>       typec_cap.driver_data = tps;
>       typec_cap.ops = &tps6598x_ops;
> +     typec_cap.fwnode = fwnode;
>  
>       switch (TPS_SYSCONF_PORTINFO(conf)) {
>       case TPS_PORTINFO_SINK_ACCESSORY:
> @@ -525,12 +555,16 @@ static int tps6598x_probe(struct i2c_client *client)
>               typec_cap.data = TYPEC_PORT_DFP;
>               break;
>       default:
> -             return -ENODEV;
> +             ret = -ENODEV;
> +             goto err_role_put;
>       }
>  
>       tps->port = typec_register_port(&client->dev, &typec_cap);
> -     if (IS_ERR(tps->port))
> -             return PTR_ERR(tps->port);
> +     if (IS_ERR(tps->port)) {
> +             ret = PTR_ERR(tps->port);
> +             goto err_role_put;
> +     }
> +     fwnode_handle_put(fwnode);
>  
>       if (status & TPS_STATUS_PLUG_PRESENT) {
>               ret = tps6598x_connect(tps, status);
> @@ -545,12 +579,19 @@ static int tps6598x_probe(struct i2c_client *client)
>       if (ret) {
>               tps6598x_disconnect(tps, 0);
>               typec_unregister_port(tps->port);
> -             return ret;
> +             goto err_role_put;
>       }
>  
>       i2c_set_clientdata(client, tps);
>  
>       return 0;
> +
> +err_role_put:
> +     usb_role_switch_put(tps->role_sw);
> +err_fwnode_put:
> +     fwnode_handle_put(fwnode);
> +
> +     return ret;
>  }

thanks,

-- 
heikki