Re: [PATCH v2 8/8] rtc: abx80x: add support for ABX81X

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Incorrect SPI read/write flag masks invert SPI commands.
- [Medium] Masking of valid error codes from `devm_regmap_init_*()`.
--

--- Patch [8]: [PATCH v2 8/8] rtc: abx80x: add support for ABX81X ---
commit f05e60a6ab0d5bdeb5665a7f2de3be7428abd444
Author: Antoni Pokusinski <apokusinski01@gmail.com>
Subject: rtc: abx80x: add support for ABX81X

This commit adds SPI support for the Abracon ABX81X family of RTCs. It
introduces the required regmap configuration and SPI driver probe logic.

> diff --git a/drivers/rtc/rtc-abx80x.c b/drivers/rtc/rtc-abx80x.c
> index 7cba72656d094..46434f05a1f7a 100644
> --- a/drivers/rtc/rtc-abx80x.c
> +++ b/drivers/rtc/rtc-abx80x.c
[ ... ]
> @@ -1026,14 +1029,107 @@ static void abx80x_unregister_driver(void)
>  
>  #endif /* IS_ENABLED(CONFIG_I2C) */
>  
> +#if IS_ENABLED(CONFIG_SPI_MASTER)
> +
> +static const struct regmap_config abx80x_regmap_config_spi = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = ABX8XX_SRAM_BASE + ABX8XX_SRAM_WIN_SIZE - 1,
> +
> +	.rd_table = &abx80x_read_table,
> +	.wr_table = &abx80x_write_table,
> +
> +	.write_flag_mask = BIT(7),
> +};

[Severity: Medium]
Does the hardware expect MSB=1 for writes and MSB=0 for reads? Since
.read_flag_mask is omitted, regmap defaults it to 0. If the device expects
the opposite (MSB=1 for reads), the SPI commands could be inverted.

[ ... ]
> +static int abx81x_spi_probe(struct spi_device *spi)
> +{
> +	unsigned int part = (uintptr_t)spi_get_device_match_data(spi);
> +	struct regmap *regmap;
> +
> +	regmap = devm_regmap_init_spi(spi, &abx80x_regmap_config_spi);
> +	if (IS_ERR(regmap)) {
> +		dev_err(&spi->dev, "Unable to allocate regmap\n");
> +		return -EIO;
> +	}

[Severity: Medium]
Should this return PTR_ERR(regmap) instead of a hardcoded -EIO? Returning
-EIO here masks the true error code from devm_regmap_init_spi() (like -ENOMEM
or -EPROBE_DEFER), which could disrupt probe deferral logic. (It looks like
the existing abx80x_i2c_probe() might have the same behavior.)

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260725145718.14733-1-apokusinski01@gmail.com?part=8