Skip to content

PCF8523 RTC Driver Sends I2C Double Start While Executing RTC Read Time #5614

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Ben-Rosenfeld-VPS opened this issue Sep 25, 2023 · 6 comments

Comments

@Ben-Rosenfeld-VPS
Copy link

Describe the bug

The PCF8523 datasheet (https://www.nxp.com/docs/en/data-sheet/PCF8523.pdf) states in section 8.11.2:
"For this device, a repeated START is not allowed. Therefore, a STOP has to be released before the next START."
It also states in Figure 33 the Bus Protocol for Read Mode which explicitly shows a Stop/Start.

However, in pcf8523_rtc_read_time the array of msgs is defined for msgs[0] and msgs[1] and then passed in as a parameter to i2c_transfer. Per the i2c_transfer implementation msgs is "One or more messages to execute before STOP is issued to terminate the operation; each message begins with a START.".

Steps to reproduce the behaviour

Configure kernel for use of PCF8523 driver. Connect PCF8523. Analyze with Logic Analyzer.

Repeated Start

Device (s)

Raspberry Pi 4 Mod. B

System

Raspberry Pi reference 2022-09-22
Generated using pi-gen, https://github.com/RPi-Distro/pi-gen, 8a42abcd1dbd8c9c1fdfca4e0c3778255b2f9cc4, stage2

Jan 5 2023 10:46:54
Copyright (c) 2012 Broadcom
version 8ba17717fbcedd4c3b6d4bce7e50c7af4155cba9 (clean) (release) (start)

Linux pcp 5.15.84-v7l+ #1613 SMP Thu Jan 5 12:01:26 GMT 2023 armv7l GNU/Linux

Logs

No response

Additional context

No response

@pelwell
Copy link
Contributor

pelwell commented Sep 25, 2023

How does this misbehaviour manifest itself, other than in a scope trace?

@Ben-Rosenfeld-VPS
Copy link
Author

How does this misbehaviour manifest itself, other than in a scope trace?

I have asked NXP the same question in their forums, here, a few days ago: https://community.nxp.com/t5/Other-NXP-Products/I2C-specification-clarification-mixing-transaction-to-different/m-p/1727431#M19344

Currently it "appears" to work and read the time from the RTC. However, given the implementation of the driver is not to the spec of the IC it does not guarantee this implementation will continue to work in the future.

I leave it to the development team to determine the severity and whether they want to address this.

@pelwell
Copy link
Contributor

pelwell commented Sep 25, 2023

You're currently running a 5.15 kernel. In 6.1 the driver has been rewritten to use a regmap abstraction, and I suspect this bug has disappeared in the process. Since this aspect of the code is unmodified from the upstream version, you would need to report it to the upstream kernel devs, and without any obvious failures it will be hard to get traction for a change to an old LTS kernel.

@Ben-Rosenfeld-VPS
Copy link
Author

Ok, thanks. I'll try updating to 6.1 and see if this issue resolves.

@Ben-Rosenfeld-VPS
Copy link
Author

Hi,
Just a note. I updated.

Raspberry Pi reference 2022-09-22
Generated using pi-gen, https://github.com/RPi-Distro/pi-gen, 8a42abcd1dbd8c9c1fdfca4e0c3778255b2f9cc4, stage2

Mar 17 2023 10:50:39
Copyright (c) 2012 Broadcom
version 82f3750a65fadae9a38077e3c2e217ad158c8d54 (clean) (release) (start)

Linux pcp 6.1.21-v8+ #1642 SMP PREEMPT Mon Apr 3 17:24:16 BST 2023 aarch64 GNU/Linux

The same issue exists.

image

@6by9
Copy link
Contributor

6by9 commented Sep 26, 2023

Indeed regmap_i2c will use a repeated start for a read - https://elixir.bootlin.com/linux/latest/source/drivers/base/regmap/regmap-i2c.c#L172

In theory if the I2C bus driver reports I2C_FUNC_PROTOCOL_MANGLING, then it would appear that addng I2C_M_STOP to the ````xfer[1].flags``` would force a stop between the messages, however I see no way of requesting regmap_i2c to do that.

It's one to take up with the mainline Linux devs.

~/linux$ ./scripts/get_maintainer.pl drivers/rtc/rtc-pcf8523.c 
Alessandro Zummo <[email protected]> (maintainer:REAL TIME CLOCK (RTC) SUBSYSTEM)
Alexandre Belloni <[email protected]> (maintainer:REAL TIME CLOCK (RTC) SUBSYSTEM)
[email protected] (open list:REAL TIME CLOCK (RTC) SUBSYSTEM)
[email protected] (open list)

It looks like the Linux driver has used a repeated start for the 10 years since the driver was added in f803f0d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants