Hi Abhishek,
On Wed, Jan 22, 2025 at 04:00:47PM -0800, Abhishek Pandit-Subedi wrote:
> 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.
Thanks Dan for the report on these. I agree, seems simple to fix this one.
I'll take a look at the others in this file too.
+Andrei Kuchynski as well.
Thanks,
Benson
Attachment:
signature.asc
Description: PGP signature
