DS3231 v. MAX77620

Hi All,

I just plugged a DS3231 (RTC) into a RPi4 running 13.0 Release. Not 
strictly necessary but I had one on my desk and it's a weekend. Added an 
appropriate dtb overlay and loaded the ds3231.ko via loader.conf. Done 
this a few times before on other boards and not expecting any drama.

Instead of showing up during boot as a DS3231, it appears to be probed 
as a MAX77620 (which fails) and  leaves the real device unavailable to 
the ds3231 driver.


Nov  5 17:23:00 427269616e-60 kernel: iic0: <I2C generic I/O> on iicbus0
Nov  5 17:23:00 427269616e-60 kernel: rtc0: <MAX77620 RTC> at addr 0xd0 
on iicbus0

Nov  5 17:23:00 427269616e-60 kernel: rtc0: Error when reading reg 0x02, 
rv: 35
Nov  5 17:23:00 427269616e-60 kernel: rtc0: Failed to configure RTC
Nov  5 17:23:00 427269616e-60 kernel: device_attach: rtc0 attach returned 5

After some investigation I have found that all I need to provoke these 
messages is the dtb overlay loaded. Exactly the same messages are 
generated without the ds3231.ko module and even when no physical device 
is present.

Looking at max77620_rtc_probe in 
sys/arm64/nvidia/tegra210/max77620_rtc.c shows:

static int
max77620_rtc_probe(device_t dev)
{
struct iicbus_ivar *dinfo;
dinfo = device_get_ivars(dev);
if (dinfo == NULL)
return (ENXIO);
if (dinfo->addr != MAX77620_RTC_I2C_ADDR << 1)
return (ENXIO);
device_set_desc(dev, "MAX77620 RTC");
return (BUS_PROBE_DEFAULT);
}

This device will attempt to attach to anything with address == 
MAX77620_RTC_I2C_ADDR (0x68) that is found in the device tree. However, 
https://learn.adafruit.com/i2c-addresses/the-list lists:

0x68 This address is really popular with real time clocks, almost all of 
them use 0x68! AMG8833 IR Thermal Camera Breakout (0x68 or 0x69) DS1307 
RTC (0x68 only) DS3231 RTC (0x68 only) ICM-20649 Accel+Gyro (0x68 or 
0x69) ITG3200 Gyro (0x68 or 0x69) MPU-9250 9-DoF IMU (0x68 or 0x69) 
MPU-60X0 Accel+Gyro (0x68 or 0x69) PCF8523 RTC (0x68 only)

A seven bit device address is clearly not enough to uniquely identify a 
type of device and so shouldn't be used like this in the driver.

Either the driver should use ofw_bus_search_compatible (although I 
believe there is no entry for the rtc in the linux device tree) or at 
least made conditional on the parent device (the MAX77620) being in the 
device tree.

As I said earlier, this doesn't matter a huge amount for me at this 
stage because in my current application time will be configured by ntp. 
In the future it will matter more. While it may well be that the target 
device cannot be identified any other way, this doesn't belong in GENERIC.

Thanks for reading,

Brian Scott