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

On 25/07/2026 21:19:36+0200, Antoni Pokusinski wrote:
> On Sat, Jul 25, 2026 at 03:07:52PM +0000, [email protected] 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 <[email protected]>
> > 
> > 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)` ).

Yes, don't bother. Autodetection was a bad idea anyway and will never be
used with an rv1805.


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com