On 2022-08-21 13:26, Frank Wunderlich wrote:
From: Peter Geis <pgwipeout@xxxxxxxxx>
RTC sometimes does not respond the first time in init.
Try multiple times to get a response.
Signed-off-by: Peter Geis <pgwipeout@xxxxxxxxx>
Signed-off-by: Frank Wunderlich <frank-w@xxxxxxxxxxxxxxx>
---
discussion from v1
https://patchwork.kernel.org/project/linux-rockchip/patch/20220608161150.58919-2-linux@xxxxxxxxx/
On Fri, Jul 8, 2022 at 12:18 PM Robin Murphy <robin.murphy@xxxxxxx> wrote:
FWIW, given that HYM8563 is fairly common on RK3288 boards - I can't say
I've ever noticed an issue with mine, for instance - it seems dubious
that this would be a general issue of the chip itself. Are you sure it's
not a SoC or board-level issue with the I2C bus being in a funny initial
state, timings being marginal, or suchlike?
Peter Geis <pgwipeout@xxxxxxxxx>:
I don't think this is an SoC issue since this is the first instance
I've encountered it. Mind you we don't have the reset lines hooked up
at all for the Rockchip i2c driver, so it's possible that's the case,
but I'd imagine it would be observed more broadly if that was the
case. I've tried pushing the timings out pretty far as well as bumping
up the drive strength to no change. It seems to occur only with the
hym rtc used on this board. I suspect it's a new variant of the hym
that has slightly different behavior.
Sure, if it's documented somewhere that Hayou (or if the BPI-R2 Pro
schematic is to be believed, AnalogTek) decided to innovate a new
"sometimes doesn't work" feature for a chip that's been in production
for a decade or more, and that 2 retries at 20ms intervals is what's
recommended, then I'm open to believing that this isn't a complete hack.
Or at least if someone can say they've scoped the pins and confirmed
that nothing looks suspect at the protocol level when this happens that
could explain it.
Otherwise, I'll remain unconvinced that it isn't a coincidence that this
has shown up while bringing up a new board with a new SoC, and hacking a
mature common driver to bodge around an issue that isn't fully
understood, and could very conceivably lie elsewhere, is not the right
answer. Especially when it involves a board vendor... let's say, whose
reputation proceeds them.
Since I'm not above wasting 20 minutes of my time to prove a point, for
starters the schematic seems to imply that it's using a variant of RK809
where LDO4, used as the I/O supply for i2c3, is off by default, so on
the face of it it could be something as stupidly simple as the RTC probe
racing with the PMIC or I/O domain probe. Sure, the DT claims it's
already on at boot, but *is* it? Maybe that was true with some
downstream bootloader, but do we know that's what you're using to boot
mainline? Maybe this something so obvious that you've already confirmed
and taken it for granted, but the patch as presented doesn't give me the
confidence to rule *anything* out.
Thanks,
Robin.
---
drivers/rtc/rtc-hym8563.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/rtc/rtc-hym8563.c b/drivers/rtc/rtc-hym8563.c
index cc710d682121..d9d0b6615a07 100644
--- a/drivers/rtc/rtc-hym8563.c
+++ b/drivers/rtc/rtc-hym8563.c
@@ -13,6 +13,7 @@
#include <linux/clk-provider.h>
#include <linux/i2c.h>
#include <linux/bcd.h>
+#include <linux/delay.h>
#include <linux/rtc.h>
#define HYM8563_CTL1 0x00
@@ -438,10 +439,16 @@ static irqreturn_t hym8563_irq(int irq, void *dev_id)
static int hym8563_init_device(struct i2c_client *client)
{
- int ret;
+ int ret, i;
/* Clear stop flag if present */
- ret = i2c_smbus_write_byte_data(client, HYM8563_CTL1, 0);
+ for (i = 0; i < 3; i++) {
+ ret = i2c_smbus_write_byte_data(client, HYM8563_CTL1, 0);
+ if (ret == 0)
+ break;
+ msleep(20);
+ }
+
if (ret < 0)
return ret;
