| 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:
<PUZPR06MB5887BFF27AAD64ACA625126BEF3BA@PUZPR06MB5887.apcprd06.prod.outlook.com>
Date: Thu, 28 Aug 2025 06:44:16 +0000
From: Gary Yang <gary.yang@...tech.com>
To: Krzysztof Kozlowski <krzk@...nel.org>, "linus.walleij@...aro.org"
<linus.walleij@...aro.org>, "robh@...nel.org" <robh@...nel.org>,
"krzk+dt@...nel.org" <krzk+dt@...nel.org>, "conor+dt@...nel.org"
<conor+dt@...nel.org>
CC: "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
cix-kernel-upstream <cix-kernel-upstream@...tech.com>
Subject:
回复: [PATCH 1/3] pinctrl: cix: Add pin-controller support for sky1
Hi Krzysztof,
>
> 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.
OK, we will delete this blank line next version
>
> > + 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.
>
Sorry, this print message is only once, not twice, please give more information
> > + return ret;
> > + }
> > +
> > + pinctrl_provide_dummies();
> > + dev_info(&pdev->dev, "initialized SKY1 pinctrl driver\n");
>
>
> No, please drop. Drivers should be silent on success.
>
Ok, replace dev_info with dev_dbg next version
> > +
> > + 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.
>
We will delete it on next version
> > + .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?
>
We will delete them on next version
> Best regards,
> Krzysztof
Best wishes
Gary
Powered by blists - more mailing lists