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

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 <[email protected]>
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)
> =20
>  #endif /* IS_ENABLED(CONFIG_I2C) */
> =20
> +#if IS_ENABLED(CONFIG_SPI_MASTER)
> +
> +static const struct regmap_config abx80x_regmap_config_spi =3D {
> +	.reg_bits =3D 8,
> +	.val_bits =3D 8,
> +	.max_register =3D ABX8XX_SRAM_BASE + ABX8XX_SRAM_WIN_SIZE - 1,
> +
> +	.rd_table =3D &abx80x_read_table,
> +	.wr_table =3D &abx80x_write_table,
> +
> +	.write_flag_mask =3D BIT(7),
> +};

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

[ ... ]
> +static int abx81x_spi_probe(struct spi_device *spi)
> +{
> +	unsigned int part =3D (uintptr_t)spi_get_device_match_data(spi);
> +	struct regmap *regmap;
> +
> +	regmap =3D 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 -ENOM=
EM
or -EPROBE_DEFER), which could disrupt probe deferral logic. (It looks like
the existing abx80x_i2c_probe() might have the same behavior.)

[ ... ]

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260725145718.1473=
[email protected]?part=3D8