| 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 May 2013 08:48:57 +0200
From: Linus Walleij <linus.walleij@...aro.org>
To: James Hogan <james.hogan@...tec.com>,
Laurent Pinchart <laurent.pinchart+renesas@...asonboard.com>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Grant Likely <grant.likely@...retlab.ca>,
Rob Herring <rob.herring@...xeda.com>,
Rob Landley <rob@...dley.net>,
"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
"devicetree-discuss@...ts.ozlabs.org"
<devicetree-discuss@...ts.ozlabs.org>
Subject: Re: [PATCH v2 6/9] pinctrl-tz1090: add TZ1090 pinctrl driver
Hi James, I want to route this patch by Laurent Pinchart, as he's also
adding device tree support for a SoC using generic pinconf.
Creating new bindings for every pin controller defining the same thing
and implementing the same OF parsers is not sane.
We need to:
- Define generic pinconf bindings
- Implement a common parser for these in drivers/pinctrl/pinconf-generic.c
I'll merge your patches to <linux/pinctrl/pinctrl-generic.h> as a
baseline.
On Fri, May 24, 2013 at 6:21 PM, James Hogan <james.hogan@...tec.com> wrote:
(...)
> +Optional subnode-properties:
> +- function: A string containing the name of the function to mux to the pin or
> + group. Valid values for function names are listed below, including which
> + pingroups can be muxed to them.
> +- tristate: Flag, put pin into high impedance state.
> +- pull-up: Flag, pull pin high.
> +- pull-down: Flag, pull pin low.
> +- bus-hold: Flag, weak latch last value on tristate bus.
> +- schmitt: Integer, enable or disable Schmitt trigger mode for the pins.
> + 0: no hysteresis
> + 1: schmitt trigger
> +- slew-rate: Integer, control slew rate of pins.
> + 0: slow (half frequency)
> + 1: fast
> +- drive-strength: Integer, control drive strength of pins in mA.
> + 2: 2mA
> + 4: 4mA
> + 8: 8mA
> + 12: 12mA
So if you want to use these names they shall be made generic.
Else you have to prefix everything with "tz1090,<property>"
> +/* Describes pinconf properties/flags available from device tree */
> +static const struct cfg_param {
> + const char *property;
> + enum pin_config_param param;
> + bool flag;
> +} cfg_params[] = {
> + {"tristate", PIN_CONFIG_BIAS_HIGH_IMPEDANCE, true},
> + {"pull-up", PIN_CONFIG_BIAS_PULL_UP, true},
> + {"pull-down", PIN_CONFIG_BIAS_PULL_DOWN, true},
> + {"bus-hold", PIN_CONFIG_BIAS_BUS_HOLD, true},
> + {"schmitt", PIN_CONFIG_INPUT_SCHMITT_ENABLE, true},
> + {"slew-rate", PIN_CONFIG_SLEW_RATE, false},
> + {"drive-strength", PIN_CONFIG_DRIVE_STRENGTH, false},
> +};
> +
> +int tz1090_pinctrl_dt_subnode_to_map(struct device *dev,
> + struct device_node *np,
> + struct pinctrl_map **map,
> + unsigned *reserved_maps,
> + unsigned *num_maps)
> +{
> + int ret, i;
> + const char *function;
> + u32 val;
> + unsigned long config;
> + unsigned long *configs = NULL;
> + unsigned num_configs = 0;
> + unsigned reserve;
> + struct property *prop;
> + const char *group;
> +
> + ret = of_property_read_string(np, "function", &function);
> + if (ret < 0) {
> + /* EINVAL=missing, which is fine since it's optional */
> + if (ret != -EINVAL)
> + dev_err(dev, "could not parse property function\n");
> + function = NULL;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(cfg_params); i++) {
> + ret = of_property_read_u32(np, cfg_params[i].property, &val);
> + /* flags don't have to have a value */
> + if (ret == -EOVERFLOW && cfg_params[i].flag) {
> + val = 1;
> + ret = 0;
> + }
> + if (!ret) {
> + config = pinconf_to_config_packed(cfg_params[i].param,
> + val);
> + ret = add_config(dev, &configs, &num_configs, config);
> + if (ret < 0)
> + goto exit;
> + /* EINVAL=missing, which is fine since it's optional */
> + } else if (ret != -EINVAL) {
> + dev_err(dev, "could not parse property %s (%d)\n",
> + cfg_params[i].property, ret);
> + }
> + }
> +
> + reserve = 0;
> + if (function != NULL)
> + reserve++;
> + if (num_configs)
> + reserve++;
> + ret = of_property_count_strings(np, "pins");
> + if (ret < 0) {
> + dev_err(dev, "could not parse property pins\n");
> + goto exit;
> + }
> + reserve *= ret;
> +
> + ret = reserve_map(dev, map, reserved_maps, num_maps, reserve);
> + if (ret < 0)
> + goto exit;
> +
> + of_property_for_each_string(np, "pins", prop, group) {
> + if (function) {
> + ret = add_map_mux(map, reserved_maps, num_maps,
> + group, function);
> + if (ret < 0)
> + goto exit;
> + }
> +
> + if (num_configs) {
> + ret = add_map_configs(dev, map, reserved_maps,
> + num_maps, group, configs,
> + num_configs);
> + if (ret < 0)
> + goto exit;
> + }
> + }
> +
> + ret = 0;
> +
> +exit:
> + kfree(configs);
> + return ret;
> +}
This is looking good. Can you split out the pin config mapping in
some way and put that into pinconf-generic.c?
Yours,
Linus Walleij
--
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