| 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: <d5c85ba7-77ec-47f4-8ba1-39199e96da11@kernel.org>
Date: Wed, 27 Aug 2025 11:07:25 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Gary Yang <gary.yang@...tech.com>, linus.walleij@...aro.org,
robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org
Cc: linux-gpio@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, cix-kernel-upstream@...tech.com
Subject: Re: [PATCH 1/3] pinctrl: cix: Add pin-controller support for sky1
On 27/08/2025 04:42, Gary Yang wrote:
> +
> +static int sky1_pinctrl_probe_dt(struct platform_device *pdev,
> + struct sky1_pinctrl *spctl)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct device_node *child;
> + struct pinctrl_dev *pctl = spctl->pctl;
> + u32 nfuncs = 0;
> + u32 i = 0;
> + bool flat_funcs;
> +
> + if (!np)
> + return -ENODEV;
> +
> + flat_funcs = sky1_pinctrl_dt_is_flat_functions(np);
> + if (flat_funcs) {
> + nfuncs = 1;
> + } else {
> + nfuncs = of_get_child_count(np);
> + if (nfuncs == 0) {
> + dev_err(&pdev->dev, "no functions defined\n");
> + return -EINVAL;
> + }
> + }
> +
> + for (i = 0; i < nfuncs; i++) {
> + struct function_desc *function;
> +
> + function = devm_kzalloc(&pdev->dev, sizeof(*function),
> + GFP_KERNEL);
> + if (!function)
> + return -ENOMEM;
> +
> + mutex_lock(&spctl->mutex);
> + radix_tree_insert(&pctl->pin_function_tree, i, function);
> + mutex_unlock(&spctl->mutex);
> + }
> + pctl->num_functions = nfuncs;
> +
> + spctl->group_index = 0;
> + if (flat_funcs) {
> + pctl->num_groups = of_get_child_count(np);
> + } else {
> + pctl->num_groups = 0;
> + for_each_child_of_node(np, child)
> + pctl->num_groups += of_get_child_count(child);
> + }
> +
> + if (flat_funcs) {
> + sky1_pinctrl_parse_functions(np, spctl, 0);
> + } else {
> + i = 0;
> + for_each_child_of_node(np, child)
> + sky1_pinctrl_parse_functions(child, spctl, i++);
> + }
> +
> + return 0;
> +}
> +
> +int sky1_base_pinctrl_probe(struct platform_device *pdev,
> + const struct sky1_pinctrl_soc_info *info)
> +{
> + struct pinctrl_desc *sky1_pinctrl_desc;
> + struct sky1_pinctrl *spctl;
> + int ret, i;
> +
> + if (!info || !info->pins || !info->npins) {
> + dev_err(&pdev->dev, "wrong pinctrl info\n");
> + return -EINVAL;
> + }
> +
> + /* Create state holders etc for this driver */
> + spctl = devm_kzalloc(&pdev->dev, sizeof(*spctl), GFP_KERNEL);
> + if (!spctl)
> + return -ENOMEM;
> +
> + spctl->pin_regs = devm_kmalloc_array(&pdev->dev, info->npins,
> + sizeof(*spctl->pin_regs),
> + GFP_KERNEL);
> + if (!spctl->pin_regs)
> + return -ENOMEM;
> +
> + for (i = 0; i < info->npins; i++)
> + spctl->pin_regs[i] = -1;
> +
> + spctl->base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(spctl->base))
> + return PTR_ERR(spctl->base);
> +
> + sky1_pinctrl_desc = devm_kzalloc(&pdev->dev, sizeof(*sky1_pinctrl_desc),
> + GFP_KERNEL);
> + if (!sky1_pinctrl_desc)
> + return -ENOMEM;
> +
> + sky1_pinctrl_desc->name = dev_name(&pdev->dev);
> + sky1_pinctrl_desc->pins = info->pins;
> + sky1_pinctrl_desc->npins = info->npins;
> + sky1_pinctrl_desc->pctlops = &sky1_pctrl_ops;
> + sky1_pinctrl_desc->pmxops = &sky1_pmx_ops;
> + sky1_pinctrl_desc->confops = &sky1_pinconf_ops;
> + sky1_pinctrl_desc->owner = THIS_MODULE;
> +
> + mutex_init(&spctl->mutex);
> +
> + spctl->info = info;
> + spctl->dev = &pdev->dev;
> + platform_set_drvdata(pdev, spctl);
> + ret = devm_pinctrl_register_and_init(&pdev->dev,
> + sky1_pinctrl_desc, spctl,
> + &spctl->pctl);
> + if (ret) {
> + dev_err(&pdev->dev, "could not register SKY1 pinctrl driver\n");
> + return ret;
> + }
> +
> + ret = sky1_pinctrl_probe_dt(pdev, spctl);
> +
No blank line here.
> + if (ret) {
> + dev_err(&pdev->dev, "fail to probe dt properties\n");
You are printing same error twice. Drop this and just handle error
printing in sky1_pinctrl_probe_dt().
Especially that you now print errors on ENOMEM.
> + return ret;
> + }
> +
> + pinctrl_provide_dummies();
> + dev_info(&pdev->dev, "initialized SKY1 pinctrl driver\n");
No, please drop. Drivers should be silent on success.
> +
> + return pinctrl_enable(spctl->pctl);
> +}
> +EXPORT_SYMBOL_GPL(sky1_base_pinctrl_probe);
> +
..
> +
> +static struct platform_driver sky1_pinctrl_driver = {
> + .driver = {
> + .name = "sky1-pinctrl",
> + .of_match_table = of_match_ptr(sky1_pinctrl_of_match),
You have a warning here - please drop of_match_ptr.
> + .pm = &sky1_pinctrl_pm_ops,
> + },
> + .probe = sky1_pinctrl_probe,
> +};
> +
> +static int __init sky1_pinctrl_init(void)
> +{
> + return platform_driver_register(&sky1_pinctrl_driver);
> +}
> +arch_initcall(sky1_pinctrl_init);
> +
> +MODULE_AUTHOR("Jerry Zhu <Jerry.Zhu@...tech.com>");
> +MODULE_DESCRIPTION("Cix Sky1 pinctrl driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/pinctrl/cix/pinctrl-sky1.h b/drivers/pinctrl/cix/pinctrl-sky1.h
> new file mode 100644
> index 000000000000..09b25dbb6db3
> --- /dev/null
> +++ b/drivers/pinctrl/cix/pinctrl-sky1.h
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Author: Jerry Zhu <Jerry.Zhu@...tech.com>
> + */
> +
> +#ifndef __DRIVERS_PINCTRL_SKY1_H
> +#define __DRIVERS_PINCTRL_SKY1_H
> +
> +#include <linux/pinctrl/pinconf-generic.h>
> +#include <linux/pinctrl/pinmux.h>
Are you sure you use both headers in this header?
Best regards,
Krzysztof
Powered by blists - more mailing lists