| 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
| ||
|
Message-ID: <CACxGe6tG8CF5AwD0Xn5_O-Ddw-waFbBWB-gubov4DfZY76Ak8A@mail.gmail.com>
Date: Wed, 14 Dec 2011 14:47:18 -0700
From: Grant Likely <grant.likely@...retlab.ca>
To: Linus Walleij <linus.walleij@...aro.org>
Cc: Dong Aisheng <b29396@...escale.com>, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linus.walleij@...ricsson.com,
s.hauer@...gutronix.de, shawn.guo@...escale.com,
kernel@...gutronix.de, rob.herring@...xeda.com
Subject: Re: [RFC PATCH v2 2/4] pinctrl: imx: add pinmux imx driver
On Wed, Dec 14, 2011 at 11:00 AM, Linus Walleij
<linus.walleij@...aro.org> wrote:
> On Wed, Dec 14, 2011 at 5:03 PM, Dong Aisheng <b29396@...escale.com> wrote:
>
>> The driver contains the initial support for imx53 and
>> imx6q.
>
> Cool! Comments below...
>
>> --- /dev/null
>> +++ b/drivers/pinctrl/pinmux-imx-core.c
>
> We should start naming these pinctrl-* rather than pinmux-*
> so just rename it.
>
> Until here it looks like business as usual..
>
>> +#ifdef CONFIG_OF
>> +static int __devinit imx_pmx_parse_functions(struct device_node *np,
>> + struct imx_pinctrl_info *info, u32 num)
>> +{
>> + struct imx_pmx_func *function;
>> + struct imx_pin_group *group;
>> + int ret, len;
>> +
>> + dev_dbg(info->dev, "parse function %d\n", num);
>> +
>> + group = &info->groups[num];
>> + function = &info->functions[num];
>> +
>> + /* Initialise group */
>> + ret = of_property_read_string(np, "grp_name", &group->name);
>> + if (ret) {
>> + dev_err(info->dev, "failed to get grp_name\n");
>> + return ret;
>> + }
>> +
>> + ret = of_property_read_u32(np, "num_pins", &group->num_pins);
>> + if (ret) {
>> + dev_err(info->dev, "failed to get num_pins\n");
>> + return ret;
>> + }
>> +
>> + ret = of_property_read_u32(np, "num_mux", &group->num_mux);
>> + if (ret) {
>> + dev_err(info->dev, "failed to get num_mux\n");
>> + return ret;
>> + }
>> +
>> + if (group->num_pins != group->num_mux)
>> + return -EINVAL;
>> +
>> + group->pins = devm_kzalloc(info->dev, group->num_pins * sizeof(unsigned int),
>> + GFP_KERNEL);
>> + group->mux_mode = devm_kzalloc(info->dev, group->num_mux * sizeof(unsigned int),
>> + GFP_KERNEL);
>> + if (!group->pins || !group->mux_mode)
>> + return -ENOMEM;
>> +
>> + /* sanity check */
>> + if (of_get_property(np, "grp_pins", &len) &&
>> + len != group->num_pins * sizeof(unsigned int)) {
>> + dev_err(info->dev, "wrong pins number?\n");
>> + return -EINVAL;
>> + }
>> +
>> + if (of_get_property(np, "grp_mux", &len) &&
>> + len != group->num_mux * sizeof(unsigned int)) {
>> + dev_err(info->dev, "wrong pin mux number?\n");
>> + return -EINVAL;
>> + }
>> +
>> + ret = of_property_read_u32_array(np, "grp_pins",
>> + group->pins, group->num_pins);
>> + if (ret) {
>> + dev_err(info->dev, "failed to get grp_pins\n");
>> + return ret;
>> + }
>> +
>> + ret = of_property_read_u32_array(np, "grp_mux",
>> + group->mux_mode, group->num_mux);
>> + if (ret) {
>> + dev_err(info->dev, "failed to get grp_mux\n");
>> + return ret;
>> + }
>> +
>> + /* Initialise function */
>> + ret = of_property_read_string(np, "func_name", &function->name);
>> + if (ret) {
>> + dev_err(info->dev, "failed to get func_name\n");
>> + return ret;
>> + }
>> +
>> + function->groups = devm_kzalloc(info->dev, sizeof(char **), GFP_KERNEL);
>> + function->num_groups = 1;
>> + function->groups[0] = group->name;
>> +
>> + dev_dbg(info->dev, "func_name %s grp_name %s num_groups %d\n",
>> + function->name, function->groups[0],
>> + function->num_groups);
>> +
>> + return 0;
>> +}
>> +
>> +static int __devinit imx_pmx_probe_dt(struct platform_device *pdev,
>> + struct imx_pinctrl_info *info)
>> +{
>> + struct device_node *np = pdev->dev.of_node;
>> + struct device_node *child = NULL;
>> + int ret, i;
>> + u32 nfuncs = 0;
>> +
>> + if (!np)
>> + return -ENODEV;
>> +
>> + nfuncs = of_get_child_number(np);
>> + if (nfuncs <= 0) {
>> + dev_err(&pdev->dev, "no functions defined\n");
>> + return -EINVAL;
>> + }
>> +
>> + info->nfunctions = nfuncs;
>> + info->functions = devm_kzalloc(&pdev->dev, nfuncs * sizeof(struct imx_pmx_func),
>> + GFP_KERNEL);
>> + if (!info->functions)
>> + return -ENOMEM;
>> +
>> + /* DT file only passes one group per one function */
>> + info->ngroups = nfuncs;
>> + info->groups = devm_kzalloc(&pdev->dev, nfuncs * sizeof(struct imx_pin_group),
>> + GFP_KERNEL);
>> + if (!info->groups)
>> + return -ENOMEM;
>> +
>> + child = NULL;
>> + i = 0;
>> + for_each_child_of_node(np, child) {
>> + ret = imx_pmx_parse_functions(child, info, i++);
>> + if (ret) {
>> + dev_err(&pdev->dev, "failed to parse function\n");
>> + return ret;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +#else
>> +static int __devinit imx_pmx_probe_dt(struct platform_device *pdev,
>> + struct imx_pinctrl_info *info)
>> +{
>> + return -ENODEV;
>> +}
>> +#endif
>
> Looks clever to me! And I think we have agreed that this probing
> must be inside each driver.
>
> Can we have a look at the corresponding device tree?
... and binding documentation. While on that topic, the convention is
for property names to use dash '-' characters, not underscores '_'.
The properties I see here are using the later.
g.
--
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