* [bug report] pinctrl: core: Handling pinmux and pinconf separately
@ 2021-03-12 6:51 Dan Carpenter
2021-03-12 6:56 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2021-03-12 6:51 UTC (permalink / raw)
To: michal.simek; +Cc: linux-gpio
Hello Michal Simek,
The patch 0952b7ec1614: "pinctrl: core: Handling pinmux and pinconf
separately" from Mar 10, 2021, leads to the following static checker
warning:
drivers/pinctrl/core.c:1275 pinctrl_commit_state()
error: uninitialized symbol 'ret'.
drivers/pinctrl/core.c
1239 static int pinctrl_commit_state(struct pinctrl *p, struct pinctrl_state *state)
1240 {
1241 struct pinctrl_setting *setting, *setting2;
1242 struct pinctrl_state *old_state = p->state;
1243 int ret;
1244
1245 if (p->state) {
1246 /*
1247 * For each pinmux setting in the old state, forget SW's record
1248 * of mux owner for that pingroup. Any pingroups which are
1249 * still owned by the new state will be re-acquired by the call
1250 * to pinmux_enable_setting() in the loop below.
1251 */
1252 list_for_each_entry(setting, &p->state->settings, node) {
1253 if (setting->type != PIN_MAP_TYPE_MUX_GROUP)
1254 continue;
1255 pinmux_disable_setting(setting);
1256 }
1257 }
1258
1259 p->state = NULL;
1260
1261 /* Apply all the settings for the new state - pinmux first */
1262 list_for_each_entry(setting, &state->settings, node) {
1263 switch (setting->type) {
1264 case PIN_MAP_TYPE_MUX_GROUP:
1265 ret = pinmux_enable_setting(setting);
1266 break;
1267 case PIN_MAP_TYPE_CONFIGS_PIN:
1268 case PIN_MAP_TYPE_CONFIGS_GROUP:
1269 break;
"ret" not set on these cases.
1270 default:
1271 ret = -EINVAL;
1272 break;
1273 }
1274
1275 if (ret < 0)
1276 goto unapply_new_state;
1277
1278 /* Do not link hogs (circular dependency) */
1279 if (p != setting->pctldev->p)
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] pinctrl: core: Handling pinmux and pinconf separately
2021-03-12 6:51 [bug report] pinctrl: core: Handling pinmux and pinconf separately Dan Carpenter
@ 2021-03-12 6:56 ` Dan Carpenter
2021-03-12 7:33 ` Michal Simek
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2021-03-12 6:56 UTC (permalink / raw)
To: michal.simek; +Cc: linux-gpio
It also complains about the next loop but that's a false positive
because dealing with lists of data is tricky.
drivers/pinctrl/core.c:1297 pinctrl_commit_state() error: uninitialized symbol 'ret'.
I saw these two warnings initially from zero day bot but didn't forward
it on because I only noticed the issue with second loop and not the
first one. #LifeLessonOrSomething
regards,
dan carpenter
On Fri, Mar 12, 2021 at 09:51:09AM +0300, Dan Carpenter wrote:
> Hello Michal Simek,
>
> The patch 0952b7ec1614: "pinctrl: core: Handling pinmux and pinconf
> separately" from Mar 10, 2021, leads to the following static checker
> warning:
>
> drivers/pinctrl/core.c:1275 pinctrl_commit_state()
> error: uninitialized symbol 'ret'.
>
> drivers/pinctrl/core.c
> 1239 static int pinctrl_commit_state(struct pinctrl *p, struct pinctrl_state *state)
> 1240 {
> 1241 struct pinctrl_setting *setting, *setting2;
> 1242 struct pinctrl_state *old_state = p->state;
> 1243 int ret;
> 1244
> 1245 if (p->state) {
> 1246 /*
> 1247 * For each pinmux setting in the old state, forget SW's record
> 1248 * of mux owner for that pingroup. Any pingroups which are
> 1249 * still owned by the new state will be re-acquired by the call
> 1250 * to pinmux_enable_setting() in the loop below.
> 1251 */
> 1252 list_for_each_entry(setting, &p->state->settings, node) {
> 1253 if (setting->type != PIN_MAP_TYPE_MUX_GROUP)
> 1254 continue;
> 1255 pinmux_disable_setting(setting);
> 1256 }
> 1257 }
> 1258
> 1259 p->state = NULL;
> 1260
> 1261 /* Apply all the settings for the new state - pinmux first */
> 1262 list_for_each_entry(setting, &state->settings, node) {
> 1263 switch (setting->type) {
> 1264 case PIN_MAP_TYPE_MUX_GROUP:
> 1265 ret = pinmux_enable_setting(setting);
> 1266 break;
> 1267 case PIN_MAP_TYPE_CONFIGS_PIN:
> 1268 case PIN_MAP_TYPE_CONFIGS_GROUP:
> 1269 break;
>
> "ret" not set on these cases.
>
> 1270 default:
> 1271 ret = -EINVAL;
> 1272 break;
> 1273 }
> 1274
> 1275 if (ret < 0)
> 1276 goto unapply_new_state;
> 1277
> 1278 /* Do not link hogs (circular dependency) */
> 1279 if (p != setting->pctldev->p)
>
> regards,
> dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] pinctrl: core: Handling pinmux and pinconf separately
2021-03-12 6:56 ` Dan Carpenter
@ 2021-03-12 7:33 ` Michal Simek
0 siblings, 0 replies; 3+ messages in thread
From: Michal Simek @ 2021-03-12 7:33 UTC (permalink / raw)
To: Dan Carpenter, michal.simek, Colin King; +Cc: linux-gpio
Hi,
On 3/12/21 7:56 AM, Dan Carpenter wrote:
> It also complains about the next loop but that's a false positive
> because dealing with lists of data is tricky.
>
> drivers/pinctrl/core.c:1297 pinctrl_commit_state() error: uninitialized symbol 'ret'.
>
> I saw these two warnings initially from zero day bot but didn't forward
> it on because I only noticed the issue with second loop and not the
> first one. #LifeLessonOrSomething
It was reported by Colin yesterday.
lore.kernel.org/r/34c61597-a90b-5d90-f9fb-4ede3ece3b4c@canonical.com
Definitely thanks for report.
Thanks,
Michal
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-03-12 7:34 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-03-12 6:51 [bug report] pinctrl: core: Handling pinmux and pinconf separately Dan Carpenter
2021-03-12 6:56 ` Dan Carpenter
2021-03-12 7:33 ` Michal Simek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).