On Tue, Oct 29, 2024 at 10:59:04AM +0800, Chen Wang wrote: > > On 2024/10/24 14:43, Inochi Amaoto wrote: > [......] > > diff --git a/drivers/pinctrl/sophgo/pinctrl-sg2042-ops.c b/drivers/pinctrl/sophgo/pinctrl-sg2042-ops.c > > new file mode 100644 > > index 000000000000..f1c33b166d01 > > --- /dev/null > > +++ b/drivers/pinctrl/sophgo/pinctrl-sg2042-ops.c > > @@ -0,0 +1,583 @@ > [......] > > +int sg2042_pinctrl_probe(struct platform_device *pdev) > > +{ > > + struct device *dev = &pdev->dev; > > + struct sg2042_pinctrl *pctrl; > > + const struct sg2042_pinctrl_data *pctrl_data; > > + int ret; > > + > > + pctrl_data = device_get_match_data(dev); > > + if (!pctrl_data) > > + return -ENODEV; > > + > > + if (pctrl_data->npins == 0) > > + return dev_err_probe(dev, -EINVAL, "invalid pin data\n"); > > + > > + pctrl = devm_kzalloc(dev, sizeof(*pctrl), GFP_KERNEL); > > + if (!pctrl) > > + return -ENOMEM; > > + > > + pctrl->regs = devm_platform_ioremap_resource(pdev, 0); > > + if (IS_ERR(pctrl->regs)) > > + return PTR_ERR(pctrl->regs); > > + > > + pctrl->pdesc.name = dev_name(dev); > > + pctrl->pdesc.pins = pctrl_data->pins; > > + pctrl->pdesc.npins = pctrl_data->npins; > > + pctrl->pdesc.pctlops = &sg2042_pctrl_ops; > > + pctrl->pdesc.pmxops = &sg2042_pmx_ops; > > + pctrl->pdesc.confops = &sg2042_pconf_ops; > > + pctrl->pdesc.owner = THIS_MODULE; > > + > > + pctrl->data = pctrl_data; > > + pctrl->dev = dev; > > + raw_spin_lock_init(&pctrl->lock); > > + mutex_init(&pctrl->mutex); > > + > > + platform_set_drvdata(pdev, pctrl); > > + > > + ret = devm_pinctrl_register_and_init(dev, &pctrl->pdesc, > > + pctrl, &pctrl->pctl_dev); > > + if (ret) > > + return dev_err_probe(dev, ret, > > + "fail to register pinctrl driver\n"); > > + > > + return pinctrl_enable(pctrl->pctl_dev); > > +} > > +EXPORT_SYMBOL_GPL(sg2042_pinctrl_probe); > Why EXPORT_SYMBOL_GPL? sg2042_pinctrl_probe looks like just a global > function should be enough. See below for why declare it as a module. > > + > > +MODULE_DESCRIPTION("Pinctrl OPs for the SG2042 SoC"); > > +MODULE_LICENSE("GPL"); > > pinctrl-sg2042-ops.c is just a common file built together with > pinctrl-sg2042.c, right? Why you declare it as a module? > This is for sg2044, which has the same pinctrl ops. Build this as module could save some space for the final image. > > diff --git a/drivers/pinctrl/sophgo/pinctrl-sg2042.c b/drivers/pinctrl/sophgo/pinctrl-sg2042.c > > new file mode 100644 > > index 000000000000..81411670f855 > > --- /dev/null > > +++ b/drivers/pinctrl/sophgo/pinctrl-sg2042.c > > @@ -0,0 +1,642 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Sophgo SG2042 SoC pinctrl driver. > > + * > > + * Copyright (C) 2024 Inochi Amaoto <inochiama@xxxxxxxxxxx> > > + */ > > + > > +#include <linux/module.h> > > +#include <linux/platform_device.h> > > +#include <linux/of.h> > > Sort in alphabetic. > > [......] > >