* [bug report] usb: typec: Add driver for Thunderbolt 3 Alternate Mode
@ 2025-01-07 14:16 Dan Carpenter
2025-01-07 14:24 ` Heikki Krogerus
2025-01-24 19:56 ` Benson Leung
0 siblings, 2 replies; 6+ messages in thread
From: Dan Carpenter @ 2025-01-07 14:16 UTC (permalink / raw)
To: Heikki Krogerus; +Cc: linux-usb
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 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [bug report] usb: typec: Add driver for Thunderbolt 3 Alternate Mode
2025-01-07 14:16 [bug report] usb: typec: Add driver for Thunderbolt 3 Alternate Mode Dan Carpenter
@ 2025-01-07 14:24 ` Heikki Krogerus
2025-01-23 0:00 ` Abhishek Pandit-Subedi
2025-01-24 19:56 ` Benson Leung
1 sibling, 1 reply; 6+ messages in thread
From: Heikki Krogerus @ 2025-01-07 14:24 UTC (permalink / raw)
To: Dan Carpenter, Abhishek Pandit-Subedi; +Cc: linux-usb
+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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [bug report] usb: typec: Add driver for Thunderbolt 3 Alternate Mode
2025-01-07 14:24 ` Heikki Krogerus
@ 2025-01-23 0:00 ` Abhishek Pandit-Subedi
2025-01-23 0:26 ` Benson Leung
0 siblings, 1 reply; 6+ messages in thread
From: Abhishek Pandit-Subedi @ 2025-01-23 0:00 UTC (permalink / raw)
To: Heikki Krogerus, Benson Leung, Łukasz Bartosik
Cc: Dan Carpenter, linux-usb
On Tue, Jan 7, 2025 at 6:24 AM Heikki Krogerus
<heikki.krogerus@linux.intel.com> 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [bug report] usb: typec: Add driver for Thunderbolt 3 Alternate Mode
2025-01-23 0:00 ` Abhishek Pandit-Subedi
@ 2025-01-23 0:26 ` Benson Leung
0 siblings, 0 replies; 6+ messages in thread
From: Benson Leung @ 2025-01-23 0:26 UTC (permalink / raw)
To: Abhishek Pandit-Subedi
Cc: Heikki Krogerus, Benson Leung, Łukasz Bartosik,
Dan Carpenter, linux-usb, akuchynski
[-- Attachment #1: Type: text/plain, Size: 4597 bytes --]
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@linux.intel.com> 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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [bug report] usb: typec: Add driver for Thunderbolt 3 Alternate Mode
2025-01-07 14:16 [bug report] usb: typec: Add driver for Thunderbolt 3 Alternate Mode Dan Carpenter
2025-01-07 14:24 ` Heikki Krogerus
@ 2025-01-24 19:56 ` Benson Leung
2025-01-25 13:01 ` Dan Carpenter
1 sibling, 1 reply; 6+ messages in thread
From: Benson Leung @ 2025-01-24 19:56 UTC (permalink / raw)
To: Dan Carpenter
Cc: Heikki Krogerus, linux-usb, abhishekpandit, ukaszb, akuchynski,
bleung
[-- Attachment #1: Type: text/plain, Size: 631 bytes --]
Hi Dan,
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:153 tbt_enter_modes_ordered() info: return a literal instead of 'ret'
I posted a couple of patches to fix the other warnings, but wasn't sure what to
with this one since by my reading, we're doing that intentionally. This seems
like an informational message.
Thanks,
Benson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [bug report] usb: typec: Add driver for Thunderbolt 3 Alternate Mode
2025-01-24 19:56 ` Benson Leung
@ 2025-01-25 13:01 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2025-01-25 13:01 UTC (permalink / raw)
To: Benson Leung
Cc: Heikki Krogerus, linux-usb, abhishekpandit, ukaszb, akuchynski,
bleung
On Fri, Jan 24, 2025 at 07:56:38PM +0000, Benson Leung wrote:
> Hi Dan,
>
>
> 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:153 tbt_enter_modes_ordered() info: return a literal instead of 'ret'
>
> I posted a couple of patches to fix the other warnings, but wasn't sure what to
> with this one since by my reading, we're doing that intentionally. This seems
> like an informational message.
Fine fine. It's an unpublished warning.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-01-25 13:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-07 14:16 [bug report] usb: typec: Add driver for Thunderbolt 3 Alternate Mode Dan Carpenter
2025-01-07 14:24 ` Heikki Krogerus
2025-01-23 0:00 ` Abhishek Pandit-Subedi
2025-01-23 0:26 ` Benson Leung
2025-01-24 19:56 ` Benson Leung
2025-01-25 13:01 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox