| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 28 Feb 2012 09:26:22 -0800
From: Stephen Warren <swarren@...dia.com>
To: Dong Aisheng <aisheng.dong@...escale.com>
CC: Linus Walleij <linus.walleij@...ricsson.com>,
Linus Walleij <linus.walleij@...aro.org>,
"B29396@...escale.com" <B29396@...escale.com>,
"s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
"dongas86@...il.com" <dongas86@...il.com>,
"shawn.guo@...aro.org" <shawn.guo@...aro.org>,
"thomas.abraham@...aro.org" <thomas.abraham@...aro.org>,
"tony@...mide.com" <tony@...mide.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH V2 1/2] pinctrl: Introduce PINCTRL_STATE_DEFAULT, define
hogs as that state
Dong Aisheng wrote at Tuesday, February 28, 2012 2:30 AM:
> On Mon, Feb 27, 2012 at 04:55:08PM -0700, Stephen Warren wrote:
> > This provides a single centralized name for the default state.
> >
> > Update PIN_MAP_* macros to use this state name, instead of requiring the
> > user to pass a state name in.
...
> > diff --git a/Documentation/pinctrl.txt b/Documentation/pinctrl.txt
> > static struct pinctrl_map __initdata mapping[] = {
> > - PIN_MAP("I2CMAP", "pinctrl-foo", "i2c0", "foo-i2c.0"),
> > + PIN_MAP(PINCTRL_STATE_DEFAULT, "pinctrl-foo", "i2c0", "foo-i2c.0"),
>
> It seemed Linus already applied your "re-order struct pinctrl_map".
> This may need change a bit according to that patch.
Yes, that patch is applied.
However, note that I deliberately did not change PIN_MAP()'s parameter
order in that patch. I deferred as much rework of those macros to the
later patch "pinctrl: Enhance mapping table to support pin config
operations" to avoid repeatedly changing those macros where possible.
For reference, the definition of PIN_MAP() at this point in the series
is:
#define PIN_MAP(a, b, c, d) \
{ .name = a, .ctrl_dev_name = b, .function = c, .dev_name = d }
> > @@ -989,7 +989,7 @@ of the pin controller itself, like this:
> >
> > {
> > .dev_name = "pinctrl-foo",
> > - .name = "POWERMAP"
> > + .name = PINCTRL_STATE_DEFAULT,
> > .ctrl_dev_name = "pinctrl-foo",
> > .function = "power_func",
> > },
>
> ditto
That already matches the order of fields in the struct definition,
which for reference is as follows at this point in the series:
struct pinctrl_map {
const char *dev_name;
const char *name;
const char *ctrl_dev_name;
const char *function;
const char *group;
};
(.group is still allowed to be NULL)
> > @@ -1078,8 +944,6 @@ static void pinctrl_init_device_debugfs(struct pinctrl_dev *pctldev)
> > device_root, pctldev, &pinctrl_groups_ops);
> > debugfs_create_file("gpio-ranges", S_IFREG | S_IRUGO,
> > device_root, pctldev, &pinctrl_gpioranges_ops);
> > - debugfs_create_file("pinmux-hogs", S_IFREG | S_IRUGO,
> > - device_root, pctldev, &pinmux_hogs_ops);
>
> I see you remove the pinnmux-hogs sysfs file here.
> I guess you may want to merge it into pinctrl-maps sysfs file since
> they're almost same(only difference is device name).
>
> Do you think if we can add a special flag for that type of map in sysfs
> (e.g. a "Hog" flag behind the regular map debug info)?
> Then users do not need to check device's name to see if it's a hogged function.
>
> (Anyway, it should be in another patch)
I'd rather not myself; I don't see why "hog" should be a special-case;
it's just that the pin controller driver's mapping table entries have
it set up certain pin mux/config settings.
That said, feel free to submit a patch for this. I'd prefer that any
text you add to the debugfs files to indicate this "hogging" be in
addition to anything that's already present rather than replacing it
(as has unfortunately happened already in "pinmux-pins"), otherwise
it obscures information.
> > diff --git a/include/linux/pinctrl/machine.h b/include/linux/pinctrl/machine.h
> > index 73fbb27..20e9735 100644
> > --- a/include/linux/pinctrl/machine.h
> > +++ b/include/linux/pinctrl/machine.h
> > @@ -12,6 +12,8 @@
> > #ifndef __LINUX_PINCTRL_MACHINE_H
> > #define __LINUX_PINCTRL_MACHINE_H
> >
> > +#include "pinctrl.h"
> > +
> > /**
> > * struct pinctrl_map - boards/machines shall provide this map for devices
> > * @dev_name: the name of the device using this specific mapping, the name
> > @@ -49,17 +51,18 @@ struct pinctrl_map {
> > * Convenience macro to map a system function onto a certain pinctrl device,
> > * to be hogged by the pin control core until the system shuts down.
> > */
> > -#define PIN_MAP_SYS_HOG(a, b, c) \
> > - { .name = a, .ctrl_dev_name = b, .dev_name = b, .function = c, }
> > +#define PIN_MAP_SYS_HOG(a, b) \
> > + { .name = PINCTRL_STATE_DEFAULT, .ctrl_dev_name = a, .dev_name = a, \
> > + .function = b, }
> >
> > /*
> > * Convenience macro to map a system function onto a certain pinctrl device
> > * using a specified group, to be hogged by the pin control core until the
> > * system shuts down.
> > */
> > -#define PIN_MAP_SYS_HOG_GROUP(a, b, c, d) \
> > - { .name = a, .ctrl_dev_name = b, .dev_name = b, .function = c, \
> > - .group = d, }
> > +#define PIN_MAP_SYS_HOG_GROUP(a, b, c) \
> > + { .name = PINCTRL_STATE_DEFAULT, .ctrl_dev_name = a, .dev_name = a, \
> > + .function = b, .group = c, }
> >
>
> In your v1 patch, we discussed about the possible requirement of different state
> support for hog functions(it seemed still no conclusion, right?).
I agreed to fully support that. However as I mentioned, the support will be
actually implemented in patch "pinctrl: Enhance mapping table to support
pin config operations" since I was already reworking all the mapping
table macros there, it felt better to do the rework once in that patch.
I'm waiting on resolution of the locking patch issue before I repost an
updated version of the mapping table rework patch that includes this
support.
> Acked-by: Dong Aisheng <dong.aisheng@...aro.org>
Thanks.
--
nvpublic
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists