[PATCH 2/3] pinctrl: sophgo: add support for SG2042 SoC

[Inochi Amaoto]

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 at outlook.com>
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/of.h>
> 
> Sort in alphabetic.
> 
> [......]
> 
>