On Tue, Jan 7, 2025 at 6:24 AM Heikki Krogerus
<heikki.krogerus@xxxxxxxxxxxxxxx> wrote:
>
> +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
+Benson Leung +Łukasz Bartosik can help with a patch to address this
while I'm out on baby bonding leave. As you noted, they look pretty
straightforward.
