Re: [PATCH v5 3/3] pinctrl: aspeed: Add AST2700 SoC1 support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pin_config_get() and pin_config_set() return -EINVAL instead of -ENOTSUPP for virtual pins, causing fatal probe failures for groups containing these pins.
--

commit d4d214b81bd3876c9753372b7321c3f7575490ed
Author: Billy Tsai <billy_tsai@aspeedtech.com>

pinctrl: aspeed: Add AST2700 SoC1 support

This commit implements pin multiplexing and pin configuration for the
AST2700 SoC1 SCU pinctrl block using static SoC data tables. The driver
registers with the generic pinctrl, pinmux, and pinconf frameworks.

> diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g7-soc1.c b/drivers/pinctrl/aspeed/pinctrl-aspeed-g7-soc1.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g7-soc1.c

[ ... ]

> +static int aspeed_g7_soc1_pin_config_get(struct pinctrl_dev *pctldev,
> +					 unsigned int pin,
> +					 unsigned long *config)
> +{
> +	struct aspeed_g7_soc1_pinctrl *pctl = pinctrl_dev_get_drvdata(pctldev);
> +	enum pin_config_param param = pinconf_to_config_param(*config);
> +	struct aspeed_g7_field field;
> +	unsigned int val, val_raw;
> +	int ret, ds_idx;
> +
> +	if (pin > AC24)
> +		return -EINVAL;

[Severity: High]
Should this return -ENOTSUPP instead of -EINVAL for virtual pins?

The pinctrl core treats -EINVAL as a fatal error, but gracefully ignores
-ENOTSUPP for unsupported configurations. If a device tree applies generic
pin configurations (like bias-disable or drive-strength) to groups containing
a mix of physical and virtual pins, returning -EINVAL could abort the entire
group configuration and cause the consumer device's probe to fail.

[ ... ]

> +static int aspeed_g7_soc1_pin_config_set(struct pinctrl_dev *pctldev,
> +					 unsigned int pin,
> +					 unsigned long *configs,
> +					 unsigned int num_configs)
> +{
> +	struct aspeed_g7_soc1_pinctrl *pctl = pinctrl_dev_get_drvdata(pctldev);
> +	struct aspeed_g7_field field;
> +	enum pin_config_param param;
> +	int i, ret, ds_idx;
> +	unsigned int val;
> +	u32 arg;
> +
> +	if (pin > AC24)
> +		return -EINVAL;

[Severity: High]
Does this code also need to return -ENOTSUPP for virtual pins to avoid
failing pinconf_apply_setting() during device probe?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521-pinctrl-single-bit-v5-0-308be2c160fc@aspeedtech.com?part=3