Re: [bug report] usb: typec: Add driver for Thunderbolt 3 Alternate Mode

[Heikki Krogerus <heikki.krogerus@linux.intel.com>]

+Abhishek

On Tue, Jan 07, 2025 at 05:16:43PM +0300, Dan Carpenter wrote:
> Hello Heikki Krogerus,
> 
> Commit 100e25738659 ("usb: typec: Add driver for Thunderbolt 3
> Alternate Mode") from Dec 13, 2024 (linux-next), leads to the
> following (unpublished) Smatch static checker warnings:
> 
> drivers/usb/typec/altmodes/thunderbolt.c:116 tbt_altmode_work() warn: why is zero skipped 'i'
> drivers/usb/typec/altmodes/thunderbolt.c:147 tbt_enter_modes_ordered() warn: why is zero skipped 'i'
> drivers/usb/typec/altmodes/thunderbolt.c:153 tbt_enter_modes_ordered() info: return a literal instead of 'ret'
> drivers/usb/typec/altmodes/thunderbolt.c:328 tbt_altmode_remove() warn: why is zero skipped 'i'
> drivers/usb/typec/altmodes/thunderbolt.c:354 tbt_ready() warn: 'plug' is not an error pointer
> 
> drivers/usb/typec/altmodes/thunderbolt.c
>     66 static void tbt_altmode_work(struct work_struct *work)
>     67 {
>     68         struct tbt_altmode *tbt = container_of(work, struct tbt_altmode, work);
>     69         int ret;
>     70 
>     71         mutex_lock(&tbt->lock);
>     72 
>     73         switch (tbt->state) {
>     74         case TBT_STATE_SOP_P_ENTER:
>     75                 ret = typec_cable_altmode_enter(tbt->alt, TYPEC_PLUG_SOP_P, NULL);
>     76                 if (ret) {
>     77                         dev_dbg(&tbt->plug[TYPEC_PLUG_SOP_P]->dev,
>     78                                 "failed to enter mode (%d)\n", ret);
>     79                         goto disable_plugs;
>     80                 }
>     81                 break;
>     82         case TBT_STATE_SOP_PP_ENTER:
>     83                 ret = typec_cable_altmode_enter(tbt->alt, TYPEC_PLUG_SOP_PP,  NULL);
>     84                 if (ret) {
>     85                         dev_dbg(&tbt->plug[TYPEC_PLUG_SOP_PP]->dev,
>     86                                 "failed to enter mode (%d)\n", ret);
>     87                         goto disable_plugs;
>     88                 }
>     89                 break;
>     90         case TBT_STATE_ENTER:
>     91                 ret = tbt_enter_mode(tbt);
>     92                 if (ret)
>     93                         dev_dbg(&tbt->alt->dev, "failed to enter mode (%d)\n",
>     94                                 ret);
>     95                 break;
>     96         case TBT_STATE_EXIT:
>     97                 typec_altmode_exit(tbt->alt);
>     98                 break;
>     99         case TBT_STATE_SOP_PP_EXIT:
>     100                 typec_cable_altmode_exit(tbt->alt, TYPEC_PLUG_SOP_PP);
>     101                 break;
>     102         case TBT_STATE_SOP_P_EXIT:
>     103                 typec_cable_altmode_exit(tbt->alt, TYPEC_PLUG_SOP_P);
>     104                 break;
>     105         default:
>     106                 break;
>     107         }
>     108 
>     109         tbt->state = TBT_STATE_IDLE;
>     110 
>     111         mutex_unlock(&tbt->lock);
>     112         return;
>     113 
>     114 disable_plugs:
>     115         for (int i = TYPEC_PLUG_SOP_PP; i > 0; --i) {
>                                                 ^^^^^
> These should be >= 0.  Humans are bad at counting backwards.
> 
> --> 116                 if (tbt->plug[i])
>     117                         typec_altmode_put_plug(tbt->plug[i]);
>     118 
>     119                 tbt->plug[i] = NULL;
>     120         }
>     121 
>     122         tbt->state = TBT_STATE_ENTER;
>     123         schedule_work(&tbt->work);
>     124         mutex_unlock(&tbt->lock);
>     125 }

Abhishek, this looks straighforward, but just in case, can you take
look?

thanks,

-- 
heikki