* Re: [PATCH] pinctrl: core: Handling pinmux and pinconf separately
@ 2021-03-10 15:50 kernel test robot
0 siblings, 0 replies; 3+ messages in thread
From: kernel test robot @ 2021-03-10 15:50 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 9679 bytes --]
CC: kbuild-all(a)lists.01.org
In-Reply-To: <cfbe01f791c2dd42a596cbda57e15599969b57aa.1615364211.git.michal.simek@xilinx.com>
References: <cfbe01f791c2dd42a596cbda57e15599969b57aa.1615364211.git.michal.simek@xilinx.com>
TO: Michal Simek <monstr@monstr.eu>
TO: linux-kernel(a)vger.kernel.org
TO: monstr(a)monstr.eu
TO: michal.simek(a)xilinx.com
TO: git(a)xilinx.com
TO: Linus Walleij <linus.walleij@linaro.org>
CC: linux-gpio(a)vger.kernel.org
Hi Michal,
I love your patch! Perhaps something to improve:
[auto build test WARNING on pinctrl/devel]
[also build test WARNING on linux/master linus/master v5.12-rc2 next-20210310]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Michal-Simek/pinctrl-core-Handling-pinmux-and-pinconf-separately/20210310-162420
base: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel
:::::: branch date: 7 hours ago
:::::: commit date: 7 hours ago
config: x86_64-randconfig-m001-20210309 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
New smatch warnings:
drivers/pinctrl/core.c:1275 pinctrl_commit_state() error: uninitialized symbol 'ret'.
Old smatch warnings:
drivers/pinctrl/core.c:1297 pinctrl_commit_state() error: uninitialized symbol 'ret'.
vim +/ret +1275 drivers/pinctrl/core.c
036f394dd77f81 Benjamin Gaignard 2019-05-22 1233
befe5bdfbb698b Linus Walleij 2012-02-09 1234 /**
981ed1bfbc6c46 Florian Fainelli 2017-03-01 1235 * pinctrl_commit_state() - select/activate/program a pinctrl state to HW
42fed7ba44e4e8 Patrice Chotard 2013-04-11 1236 * @p: the pinctrl handle for the device that requests configuration
42fed7ba44e4e8 Patrice Chotard 2013-04-11 1237 * @state: the state handle to select/activate/program
befe5bdfbb698b Linus Walleij 2012-02-09 1238 */
981ed1bfbc6c46 Florian Fainelli 2017-03-01 1239 static int pinctrl_commit_state(struct pinctrl *p, struct pinctrl_state *state)
befe5bdfbb698b Linus Walleij 2012-02-09 1240 {
6e5e959dde0d92 Stephen Warren 2012-03-02 1241 struct pinctrl_setting *setting, *setting2;
50cf7c8ab324de Richard Genoud 2013-03-25 1242 struct pinctrl_state *old_state = p->state;
6e5e959dde0d92 Stephen Warren 2012-03-02 1243 int ret;
7ecdb16fe63e5b Stephen Warren 2012-03-02 1244
6e5e959dde0d92 Stephen Warren 2012-03-02 1245 if (p->state) {
6e5e959dde0d92 Stephen Warren 2012-03-02 1246 /*
2243a87d90b42e Fan Wu 2014-06-09 1247 * For each pinmux setting in the old state, forget SW's record
2243a87d90b42e Fan Wu 2014-06-09 1248 * of mux owner for that pingroup. Any pingroups which are
2243a87d90b42e Fan Wu 2014-06-09 1249 * still owned by the new state will be re-acquired by the call
2243a87d90b42e Fan Wu 2014-06-09 1250 * to pinmux_enable_setting() in the loop below.
6e5e959dde0d92 Stephen Warren 2012-03-02 1251 */
6e5e959dde0d92 Stephen Warren 2012-03-02 1252 list_for_each_entry(setting, &p->state->settings, node) {
1e2082b5207217 Stephen Warren 2012-03-02 1253 if (setting->type != PIN_MAP_TYPE_MUX_GROUP)
1e2082b5207217 Stephen Warren 2012-03-02 1254 continue;
7ecdb16fe63e5b Stephen Warren 2012-03-02 1255 pinmux_disable_setting(setting);
befe5bdfbb698b Linus Walleij 2012-02-09 1256 }
57b676f9c1b7cd Stephen Warren 2012-03-02 1257 }
57b676f9c1b7cd Stephen Warren 2012-03-02 1258
3102a76cfbf9ac Richard Genoud 2013-03-25 1259 p->state = NULL;
6e5e959dde0d92 Stephen Warren 2012-03-02 1260
501d24ec47c0e4 Michal Simek 2021-03-10 1261 /* Apply all the settings for the new state - pinmux first */
6e5e959dde0d92 Stephen Warren 2012-03-02 1262 list_for_each_entry(setting, &state->settings, node) {
1e2082b5207217 Stephen Warren 2012-03-02 1263 switch (setting->type) {
1e2082b5207217 Stephen Warren 2012-03-02 1264 case PIN_MAP_TYPE_MUX_GROUP:
6e5e959dde0d92 Stephen Warren 2012-03-02 1265 ret = pinmux_enable_setting(setting);
1e2082b5207217 Stephen Warren 2012-03-02 1266 break;
1e2082b5207217 Stephen Warren 2012-03-02 1267 case PIN_MAP_TYPE_CONFIGS_PIN:
501d24ec47c0e4 Michal Simek 2021-03-10 1268 case PIN_MAP_TYPE_CONFIGS_GROUP:
501d24ec47c0e4 Michal Simek 2021-03-10 1269 break;
501d24ec47c0e4 Michal Simek 2021-03-10 1270 default:
501d24ec47c0e4 Michal Simek 2021-03-10 1271 ret = -EINVAL;
501d24ec47c0e4 Michal Simek 2021-03-10 1272 break;
501d24ec47c0e4 Michal Simek 2021-03-10 1273 }
501d24ec47c0e4 Michal Simek 2021-03-10 1274
501d24ec47c0e4 Michal Simek 2021-03-10 @1275 if (ret < 0)
501d24ec47c0e4 Michal Simek 2021-03-10 1276 goto unapply_new_state;
501d24ec47c0e4 Michal Simek 2021-03-10 1277
501d24ec47c0e4 Michal Simek 2021-03-10 1278 /* Do not link hogs (circular dependency) */
501d24ec47c0e4 Michal Simek 2021-03-10 1279 if (p != setting->pctldev->p)
501d24ec47c0e4 Michal Simek 2021-03-10 1280 pinctrl_link_add(setting->pctldev, p->dev);
501d24ec47c0e4 Michal Simek 2021-03-10 1281 }
501d24ec47c0e4 Michal Simek 2021-03-10 1282
501d24ec47c0e4 Michal Simek 2021-03-10 1283 /* Apply all the settings for the new state - pinconf after */
501d24ec47c0e4 Michal Simek 2021-03-10 1284 list_for_each_entry(setting, &state->settings, node) {
501d24ec47c0e4 Michal Simek 2021-03-10 1285 switch (setting->type) {
501d24ec47c0e4 Michal Simek 2021-03-10 1286 case PIN_MAP_TYPE_MUX_GROUP:
501d24ec47c0e4 Michal Simek 2021-03-10 1287 break;
501d24ec47c0e4 Michal Simek 2021-03-10 1288 case PIN_MAP_TYPE_CONFIGS_PIN:
1e2082b5207217 Stephen Warren 2012-03-02 1289 case PIN_MAP_TYPE_CONFIGS_GROUP:
1e2082b5207217 Stephen Warren 2012-03-02 1290 ret = pinconf_apply_setting(setting);
1e2082b5207217 Stephen Warren 2012-03-02 1291 break;
1e2082b5207217 Stephen Warren 2012-03-02 1292 default:
1e2082b5207217 Stephen Warren 2012-03-02 1293 ret = -EINVAL;
1e2082b5207217 Stephen Warren 2012-03-02 1294 break;
1e2082b5207217 Stephen Warren 2012-03-02 1295 }
3102a76cfbf9ac Richard Genoud 2013-03-25 1296
42fed7ba44e4e8 Patrice Chotard 2013-04-11 1297 if (ret < 0) {
3102a76cfbf9ac Richard Genoud 2013-03-25 1298 goto unapply_new_state;
6e5e959dde0d92 Stephen Warren 2012-03-02 1299 }
036f394dd77f81 Benjamin Gaignard 2019-05-22 1300
b672a87ae5ab07 Linus Walleij 2019-05-24 1301 /* Do not link hogs (circular dependency) */
b672a87ae5ab07 Linus Walleij 2019-05-24 1302 if (p != setting->pctldev->p)
036f394dd77f81 Benjamin Gaignard 2019-05-22 1303 pinctrl_link_add(setting->pctldev, p->dev);
42fed7ba44e4e8 Patrice Chotard 2013-04-11 1304 }
6e5e959dde0d92 Stephen Warren 2012-03-02 1305
3102a76cfbf9ac Richard Genoud 2013-03-25 1306 p->state = state;
3102a76cfbf9ac Richard Genoud 2013-03-25 1307
6e5e959dde0d92 Stephen Warren 2012-03-02 1308 return 0;
3102a76cfbf9ac Richard Genoud 2013-03-25 1309
3102a76cfbf9ac Richard Genoud 2013-03-25 1310 unapply_new_state:
da58751ca2490d Richard Genoud 2013-03-28 1311 dev_err(p->dev, "Error applying setting, reverse things back\n");
3102a76cfbf9ac Richard Genoud 2013-03-25 1312
3102a76cfbf9ac Richard Genoud 2013-03-25 1313 list_for_each_entry(setting2, &state->settings, node) {
3102a76cfbf9ac Richard Genoud 2013-03-25 1314 if (&setting2->node == &setting->node)
3102a76cfbf9ac Richard Genoud 2013-03-25 1315 break;
af606177713163 Richard Genoud 2013-03-29 1316 /*
af606177713163 Richard Genoud 2013-03-29 1317 * All we can do here is pinmux_disable_setting.
af606177713163 Richard Genoud 2013-03-29 1318 * That means that some pins are muxed differently now
af606177713163 Richard Genoud 2013-03-29 1319 * than they were before applying the setting (We can't
af606177713163 Richard Genoud 2013-03-29 1320 * "unmux a pin"!), but it's not a big deal since the pins
af606177713163 Richard Genoud 2013-03-29 1321 * are free to be muxed by another apply_setting.
af606177713163 Richard Genoud 2013-03-29 1322 */
af606177713163 Richard Genoud 2013-03-29 1323 if (setting2->type == PIN_MAP_TYPE_MUX_GROUP)
af606177713163 Richard Genoud 2013-03-29 1324 pinmux_disable_setting(setting2);
3102a76cfbf9ac Richard Genoud 2013-03-25 1325 }
8009d5ff00df6a Richard Genoud 2013-03-28 1326
385d94246b05f7 Richard Genoud 2013-03-29 1327 /* There's no infinite recursive loop here because p->state is NULL */
385d94246b05f7 Richard Genoud 2013-03-29 1328 if (old_state)
42fed7ba44e4e8 Patrice Chotard 2013-04-11 1329 pinctrl_select_state(p, old_state);
6e5e959dde0d92 Stephen Warren 2012-03-02 1330
6e5e959dde0d92 Stephen Warren 2012-03-02 1331 return ret;
befe5bdfbb698b Linus Walleij 2012-02-09 1332 }
981ed1bfbc6c46 Florian Fainelli 2017-03-01 1333
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 41414 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] pinctrl: core: Handling pinmux and pinconf separately
2021-03-10 8:16 Michal Simek
@ 2021-03-11 0:01 ` Linus Walleij
0 siblings, 0 replies; 3+ messages in thread
From: Linus Walleij @ 2021-03-11 0:01 UTC (permalink / raw)
To: Michal Simek; +Cc: linux-kernel, Michal Simek, git, open list:GPIO SUBSYSTEM
On Wed, Mar 10, 2021 at 9:16 AM Michal Simek <michal.simek@xilinx.com> wrote:
> Right now the handling order depends on how entries are coming which is
> corresponding with order in DT. We have reached the case with DT overlays
> where conf and mux descriptions are exchanged which ends up in sequence
> that firmware has been asked to perform configuration before requesting the
> pin.
> The patch is enforcing the order that pin is requested all the time first
> followed by pin configuration. This change will ensure that firmware gets
> requests in the right order.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
This looks right to me so I simply applied the patch so it gets some
testing in linux-next.
If there are problems on some platform(s) we will get to know.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] pinctrl: core: Handling pinmux and pinconf separately
@ 2021-03-10 8:16 Michal Simek
2021-03-11 0:01 ` Linus Walleij
0 siblings, 1 reply; 3+ messages in thread
From: Michal Simek @ 2021-03-10 8:16 UTC (permalink / raw)
To: linux-kernel, monstr, michal.simek, git, Linus Walleij; +Cc: linux-gpio
Right now the handling order depends on how entries are coming which is
corresponding with order in DT. We have reached the case with DT overlays
where conf and mux descriptions are exchanged which ends up in sequence
that firmware has been asked to perform configuration before requesting the
pin.
The patch is enforcing the order that pin is requested all the time first
followed by pin configuration. This change will ensure that firmware gets
requests in the right order.
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
drivers/pinctrl/core.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index 7d3370289938..f5c32d2a3c91 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -1258,13 +1258,34 @@ static int pinctrl_commit_state(struct pinctrl *p, struct pinctrl_state *state)
p->state = NULL;
- /* Apply all the settings for the new state */
+ /* Apply all the settings for the new state - pinmux first */
list_for_each_entry(setting, &state->settings, node) {
switch (setting->type) {
case PIN_MAP_TYPE_MUX_GROUP:
ret = pinmux_enable_setting(setting);
break;
case PIN_MAP_TYPE_CONFIGS_PIN:
+ case PIN_MAP_TYPE_CONFIGS_GROUP:
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+
+ if (ret < 0)
+ goto unapply_new_state;
+
+ /* Do not link hogs (circular dependency) */
+ if (p != setting->pctldev->p)
+ pinctrl_link_add(setting->pctldev, p->dev);
+ }
+
+ /* Apply all the settings for the new state - pinconf after */
+ list_for_each_entry(setting, &state->settings, node) {
+ switch (setting->type) {
+ case PIN_MAP_TYPE_MUX_GROUP:
+ break;
+ case PIN_MAP_TYPE_CONFIGS_PIN:
case PIN_MAP_TYPE_CONFIGS_GROUP:
ret = pinconf_apply_setting(setting);
break;
--
2.30.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-03-11 0:02 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-10 15:50 [PATCH] pinctrl: core: Handling pinmux and pinconf separately kernel test robot
-- strict thread matches above, loose matches on Subject: below --
2021-03-10 8:16 Michal Simek
2021-03-11 0:01 ` Linus Walleij
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.