Re: [PATCH v2 2/8] rtc: abx80x: move part autodetection before RV1805 configuration

[Antoni Pokusinski <apokusinski01@gmail.com>]

On Sat, Jul 25, 2026 at 03:07:52PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The patch fails to resolve the issue it describes because part autodetection for the RV1805 incorrectly resolves to AB1805.
> --
> 
> --- Patch [2]: [PATCH v2 2/8] rtc: abx80x: move part autodetection before RV1805 configuration ---
> commit e37ea6c1257442ff3423e6bacb742916beb85bbb
> Author: Antoni Pokusinski <apokusinski01@gmail.com>
> 
> rtc: abx80x: move part autodetection before RV1805 configuration
> 
> This commit moves the part autodetection logic before the RV1805
> configuration block. The intention is to ensure the RV1805 specifics are
> applied even when the driver is bound using a generic compatible string.
> 
> > diff --git a/drivers/rtc/rtc-abx80x.c b/drivers/rtc/rtc-abx80x.c
> > index 5486d9d0b1e5d..ca3af91c29977 100644
> > --- a/drivers/rtc/rtc-abx80x.c
> > +++ b/drivers/rtc/rtc-abx80x.c
> > @@ -814,6 +814,25 @@ static int abx80x_probe(struct i2c_client *client)
> >  		return -EIO;
> >  	}
> >  
> > +	/* part autodetection */
> > +	if (part == ABX80X) {
> > +		for (i = 0; abx80x_caps[i].pn; i++)
> > +			if (partnumber == abx80x_caps[i].pn)
> > +				break;
> 
> [Severity: Medium]
> Does this code introduce a regression by preventing the RV1805 configuration
> from running during autodetection?
> 
> Since AB1805 and RV1805 share the same part number (0x1805), and AB1805
> appears earlier in the abx80x_caps array, it appears this loop will always
> match AB1805 first.
> 
> If the driver is bound using the generic abracon,abx80x compatible string,
> part will be set to AB1805 instead of RV1805.
> 
Ok so I missed the fact that "AB1805 and RV1805 share the same part number".
Hence, I see no way to make the driver run the RV1805 configuration when
it is bound using the generic ABX80X, because it may happen that the
detected part is RV1805 even though the real one is AB1805 or the
reverse (depending on the order that we define in `abx80x_caps[]`).
Therefore, I'll drop this patch in v3 and leave things as they were
(i.e. autodetecton after the `if (part == RV1805)` ).
 
> > +		if (abx80x_caps[i].pn == 0) {
> > +			dev_err(&client->dev, "Unknown part: %04x\n",
> > +				partnumber);
> > +			return -EINVAL;
> > +		}
> > +		part = i;
> > +	}
> > +
> > +	if (partnumber != abx80x_caps[part].pn) {
> > +		dev_err(&client->dev, "partnumber mismatch %04x != %04x\n",
> > +			partnumber, abx80x_caps[part].pn);
> > +		return -EINVAL;
> > +	}
> > +
> >  	/* Configure RV1805 specifics */
> >  	if (part == RV1805) {
> 
> [Severity: Medium]
> Will this check now evaluate to false due to the autodetection resolving to
> AB1805 above?
> 
> If part is overwritten with AB1805 during autodetection, the RV1805 specifics
> will continue to be silently skipped. This leaves the device vulnerable to
> accidentally entering test mode and leaking extra power, meaning the patch
> might not resolve the issue it describes.
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260725145718.14733-1-apokusinski01@gmail.com?part=2