+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
