8000 I2C readfrom_mem throws EPERM on Portenta C33 · Issue #13280 · micropython/micropython · GitHub
[go: up one dir, main page]

Skip to content

I2C readfrom_mem throws EPERM on Portenta C33 #13280

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
sebromero opened this issue Dec 27, 2023 · 10 comments
Closed

I2C readfrom_mem throws EPERM on Portenta C33 #13280

sebromero opened this issue Dec 27, 2023 · 10 comments

Comments

@sebromero
Copy link
Contributor
sebromero commented Dec 27, 2023

I'm on MicroPython v1.22.0 on 2023-12-27; Arduino Portenta C33 with RA6M5 and I just ran
a simple I2C read that worked fine under 1.21 (after merging #12978). It now throws an exception EPERM. It does however still work with SoftI2C.
I wonder what changed in the mean time in the Renesas port 🤔
To reproduce it, run the following with the values adjusted for you I2C peripheral:

from machine import I2C, Pin, SoftI2C

buses = [I2C(0), SoftI2C(scl=Pin('P408') , sda=Pin('P407'))]

for bus in buses:
    print("Trying bus", bus)
    try:
        data = bus.readfrom_mem(0x21, 0x1c, 0x1) 
        print(data)
        print("Success on bus", bus)
    except Exception as e:
        print(f"Read failed on bus {bus}: {e}")

With my setup it prints the following:

Trying bus I2C(0, freq=400000, scl=P408, sda=P407)
Read failed on bus I2C(0, freq=400000, scl=P408, sda=P407): [Errno 1] EPERM
Trying bus SoftI2C(scl=P408, sda=P407, freq=500000)
b'\xb8'
Success on bus SoftI2C(scl=P408, sda=P407, freq=500000)
@sebromero sebromero added the bug label Dec 27, 2023
@sebromero
Copy link
Contributor Author

@iabdalkader Can you confirm this issue?

@iabdalkader
Copy link
Contributor

@iabdalkader Can you confirm this issue?

I don't have any I2C sensor that I can easily use for testing. Do you have pull-ups on the device ? The board doesn't have any on that bus.

@dpgeorge
Copy link
Member

@sebromero can you run a git-bisect between 6117aa6 (assuming that commit works) and master, and run your above script? If there is a commit that introduced the regression, git-bisect will find it.

The only thing that touched I2C on renesas-ra was 7b2f13f, but I can't see how that would break hardware I2C.

And as @iabdalkader mentions, make sure you have pull-up resistors on both SCL and SDA.

@sebromero sebromero changed the title Regression: I2C readfrom_mem throws EPERM on Portenta C33 I2C readfrom_mem throws EPERM on Portenta C33 Jan 8, 2024
@sebromero
Copy link
Contributor Author
sebromero commented Jan 8, 2024

My bad, it's not a regression, I didn't test carefully enough. But it's still an issue.
The pull up resistors are mounted on the client device (10k).
image

I tested once again and there is some weird behaviour that I'm not sure how to interpret. After a power cycle an I2C scan over hardware I2C works. But if I then call readfrom_mem(33, 13, 1) it fails. A subsequent scan then keeps failing.
However, a software I2C scan and read still works after the hardware one fails. See example below:

>>> from machine import I2C, Pin, SoftI2C
>>> I2C(0).scan()
[33]
>>> I2C(0).readfrom_mem(33, 13, 1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OSError: [Errno 1] EPERM
>>> I2C(0).scan()
[]
>>> SoftI2C(scl=Pin('P408') , sda=Pin('P407')).scan()
[33]
>>> SoftI2C(scl=Pin('P408') , sda=Pin('P407')).readfrom_mem(33, 13, 1)
b'\x01'
>>> 

Not sure what could be the problem. It does work with a regular Arduino sketch.

@iabdalkader
Copy link
Contributor
iabdalkader commented Jan 9, 2024

I think this is a bug in ra/ra_i2c.c, readfrom_mem does a write followed by a read, they both succeed but the bus times out waiting on the last TEI. This is just some debug printfs:

i2c txi 
i2c txi 
i2c eri 
i2c txi 
i2c txi 
i2c tei 
i2c stopped 
i2c No error 
last stop 0 
i2c txi 
i2c txi 
i2c eri 
i2c rxi 
i2c rxi 
i2c timeout 

If you ignore the last TEI you can actually see the register data. Note after this timeout, the state/bus or something locks up (you can't repeat this test without a power cycle).

@TakeoTakahashi2020 Might want to take a look at this.

@TakeoTakahashi2020
Copy link
Contributor

I am sorry for noticing this so late. I will investigate.

@sebromero
Copy link
Contributor Author

@TakeoTakahashi2020 Thanks a lot and please let me know if I can help testing something! 🙏

TakeoTakahashi2020 added a commit to renesas/micropython that referenced this issue Jan 29, 2024
…3280)

Signed-off-by: Takeo Takahashi <takeo.takahashi.xv@renesas.com>
TakeoTakahashi2020 added a commit to renesas/micropython that referenced this issue Jan 29, 2024
…3280).

Signed-off-by: Takeo Takahashi <takeo.takahashi.xv@renesas.com>
TakeoTakahashi2020 added a commit to renesas/micropython that referenced this issue Jan 29, 2024
…3280).

Signed-off-by: Takeo Takahashi <takeo.takahashi.xv@renesas.com>
@TakeoTakahashi2020
Copy link
Contributor

Hi @sebromero san,
Thank you for your issue report.
I have posted a PR #13552 to fix I2C readfrom_mem.
I would appreciate it if you could check with updated file in the PR.

buses = [I2C(0), SoftI2C(scl=Pin('P408') , sda=Pin('P407'))]

Please note that the same pin cannot be assigned to hardware I2C and soft I2C at the same time.

@sebromero
Copy link
Contributor Author

@TakeoTakahashi2020 Thanks a lot for the fix, now it works as expected 🙌

dpgeorge pushed a commit that referenced this issue Jan 31, 2024
Tested on Portenta C33 with AT24256B (addrsize=16) and SSD1306.

Fixes issue #13280.

Signed-off-by: Takeo Takahashi <takeo.takahashi.xv@renesas.com>
@dpgeorge
Copy link
Member

Fixed by 81049ed.

dpgeorge pushed a commit that referenced this issue Feb 20, 2024
Tested on Portenta C33 with AT24256B (addrsize=16) and SSD1306.

Fixes issue #13280.

Signed-off-by: Takeo Takahashi <takeo.takahashi.xv@renesas.com>
tytgatlieven pushed a commit to tytgatlieven/micropython-esp32c6 that referenced this issue Mar 19, 2024
Tested on Portenta C33 with AT24256B (addrsize=16) and SSD1306.

Fixes issue micropython#13280.

Signed-off-by: Takeo Takahashi <takeo.takahashi.xv@renesas.com>
graeme-winter pushed a commit to winter-special-projects/micropython that referenced this issue Sep 21, 2024
Tested on Portenta C33 with AT24256B (addrsize=16) and SSD1306.

Fixes issue micropython#13280.

Signed-off-by: Takeo Takahashi <takeo.takahashi.xv@renesas.com>
ThomasHornschuh added a commit to bonfireprocessor/micropython that referenced this issue Feb 15, 2025
* rp2/rp2_flash: Lockout second core only when doing flash erase/write.

Using the multicore lockout feature in the general atomic section makes it
much more difficult to get correct.

Signed-off-by: Damien George <damien@micropython.org>

* rp2/mutex_extra: Implement additional mutex functions.

These allow entering/exiting a mutex and also disabling/restoring
interrupts, in an atomic way.

Signed-off-by: Damien George <damien@micropython.org>

* rp2/mpthreadport: Fix race with IRQ when entering atomic section.

Prior to this commit there is a potential deadlock in
mp_thread_begin_atomic_section(), when obtaining the atomic_mutex, in the
following situation:
- main thread calls mp_thread_begin_atomic_section() (for whatever reason,
  doesn't matter)
- the second core is running so the main thread grabs the mutex via the
  call mp_thread_mutex_lock(&atomic_mutex, 1), and this succeeds
- before the main thread has a chance to run save_and_disable_interrupts()
  a USB IRQ comes in and the main thread jumps off to process this IRQ
- that USB processing triggers a call to the dcd_event_handler() wrapper
  from commit bcbdee2
- that then calls mp_sched_schedule_node()
- that then attempts to obtain the atomic section, calling
  mp_thread_begin_atomic_section()
- that call then blocks trying to obtain atomic_mutex
- core0 is now deadlocked on itself, because the main thread has the mutex
  but the IRQ handler (which preempted the main thread) is blocked waiting
  for the mutex, which will never be free

The solution in this commit is to use mutex enter/exit functions that also
atomically disable/restore interrupts.

Fixes issues micropython#12980 and micropython#13288.

Signed-off-by: Damien George <damien@micropython.org>

* all: Bump version to 1.22.1.

Signed-off-by: Damien George <damien@micropython.org>

* Generic STM32F401CD Port Compiles (not working yet...)

* rp2/rp2_dma: Fix fetching 'write' buffers for writing not reading.

Signed-off-by: Nicko van Someren <nicko@nicko.org>

* rp2/machine_uart: Fix event wait in uart.flush() and uart.read().

Do not wait in the worst case up to the timeout.

Fixes issue micropython#13377.

Signed-off-by: robert-hh <robert@hammelrath.com>

* renesas-ra/ra: Fix SysTick clock source.

The SysTick_Config function must use the system/CPU clock to configure the
ticks.

Signed-off-by: iabdalkader <i.abdalkader@gmail.com>

* renesas-ra/boards/ARDUINO_PORTENTA_C33: Fix the RTC clock source.

Switch the RTC clock source to Sub-clock (XCIN). This board has an
accurate LSE crystal, and it should be used for the RTC clock
source.

Signed-off-by: iabdalkader <i.abdalkader@gmail.com>

* extmod/asyncio: Support gather of tasks that finish early.

Adds support to asyncio.gather() for the case that one or more (or all)
sub-tasks finish and/or raise an exception before the gather starts.

Signed-off-by: Damien George <damien@micropython.org>

* mimxrt/modmachine: Fix deepsleep wakeup pin ifdef.

Signed-off-by: Kwabena W. Agyeman <kwagyeman@live.com>

* extmod/modssl_mbedtls: Fix cipher iteration in SSLContext.get_ciphers.

Prior to this commit it would skip every second cipher returned from
mbedtls.

The corresponding test is also updated and now passes on esp32, rp2, stm32
and unix.

Signed-off-by: Damien George <damien@micropython.org>

* rp2: Change machine.I2S and rp2.DMA to use shared DMA IRQ handlers.

These separate drivers must share the DMA resource with each other.

Fixes issue micropython#13380.

Signed-off-by: Damien George <damien@micropython.org>

* py/compile: Fix potential Py-stack overflow in try-finally with return.

If a return is executed within the try block of a try-finally then the
return value is stored on the top of the Python stack during the execution
of the finally block.  In this case the Python stack is one larger than it
normally would be in the finally block.

Prior to this commit, the compiler was not taking this case into account
and could have a Python stack overflow if the Python stack used by the
finally block was more than that used elsewhere in the function.  In such
a scenario the last argument of the function would be clobbered by the
top-most temporary value used in the deepest Python expression/statement.

This commit fixes that case by making sure enough Python stack is allocated
to the function.

Fixes issue micropython#13562.

Signed-off-by: Damien George <damien@micropython.org>

* renesas-ra/ra/ra_i2c: Fix 1 byte and 2 bytes read issue.

Tested on Portenta C33 with AT24256B (addrsize=16) and SSD1306.

Fixes issue micropython#13280.

Signed-off-by: Takeo Takahashi <takeo.takahashi.xv@renesas.com>

* extmod/btstack: Reset pending_value_handle before calling write-done cb.

The pending_value_handle needs to be freed and reset before calling
mp_bluetooth_gattc_on_read_write_status(), which will call the Python IRQ
handler, which may in turn call back into BTstack to perform an action like
a write.  In that case the pending_value_handle will need to be available
for the write/read/etc to proceed.

Fixes issue micropython#13611.

Signed-off-by: Damien George <damien@micropython.org>

* extmod/btstack: Reset pending_value_handle before calling read-done cb.

Similar to the previous commit but for MP_BLUETOOTH_IRQ_GATTC_READ_DONE:
the pending_value_handle needs to be reset before calling
mp_bluetooth_gattc_on_read_write_status(), which will call the Python IRQ
handler, which may in turn call back into BTstack to perform an action like
a write.  In that case the pending_value_handle will need to be available
for the write/read/etc to proceed.

Fixes issue micropython#13634.

Signed-off-by: Damien George <damien@micropython.org>

* esp32/mpnimbleport: Release the GIL while doing NimBLE port deinit.

In case callbacks must run (eg a disconnect event happens during the
deinit) and the GIL must be obtained to run the callback.

Fixes part of issue micropython#12349.

Signed-off-by: Damien George <damien@micropython.org>

* esp32: Increase NimBLE task stack size and overflow detection headroom.

The Python BLE IRQ handler will most likely run on the NimBLE task, so its
C stack must be large enough to accommodate reasonably complicated Python
code (eg a few call depths).  So increase this stack size.

Also increase the headroom from 1024 to 2048 bytes.  This is needed because
(1) the esp32 architecture uses a fair amount of stack in general; and (2)
by the time execution gets to setting the Python stack top via
`mp_stack_set_top()` in this interlock code, about 600 bytes of stack are
already used, which reduces the amount available for Python.

Fixes issue micropython#12349.

Signed-off-by: Damien George <damien@micropython.org>

* all: Bump version to 1.22.2.

Signed-off-by: Damien George <damien@micropython.org>

* Submodule update

---------

Signed-off-by: Damien George <damien@micropython.org>
Signed-off-by: Nicko van Someren <nicko@nicko.org>
Signed-off-by: robert-hh <robert@hammelrath.com>
Signed-off-by: iabdalkader <i.abdalkader@gmail.com>
Signed-off-by: Kwabena W. Agyeman <kwagyeman@live.com>
Signed-off-by: Takeo Takahashi <takeo.takahashi.xv@renesas.com>
Co-authored-by: Damien George <damien@micropython.org>
Co-authored-by: Nicko van Someren <nicko@nicko.org>
Co-authored-by: robert-hh <robert@hammelrath.com>
Co-authored-by: iabdalkader <i.abdalkader@gmail.com>
Co-authored-by: Kwabena W. Agyeman <kwagyeman@live.com>
Co-authored-by: Takeo Takahashi <takeo.takahashi.xv@renesas.com>
ThomasHornschuh added a commit to bonfireprocessor/micropython that referenced this issue Feb 16, 2025
* rp2/rp2_flash: Lockout second core only when doing flash erase/write.

Using the multicore lockout feature in the general atomic section makes it
much more difficult to get correct.

Signed-off-by: Damien George <damien@micropython.org>

* rp2/mutex_extra: Implement additional mutex functions.

These allow entering/exiting a mutex and also disabling/restoring
interrupts, in an atomic way.

Signed-off-by: Damien George <damien@micropython.org>

* rp2/mpthreadport: Fix race with IRQ when entering atomic section.

Prior to this commit there is a potential deadlock in
mp_thread_begin_atomic_section(), when obtaining the atomic_mutex, in the
following situation:
- main thread calls mp_thread_begin_atomic_section() (for whatever reason,
  doesn't matter)
- the second core is running so the main thread grabs the mutex via the
  call mp_thread_mutex_lock(&atomic_mutex, 1), and this succeeds
- before the main thread has a chance to run save_and_disable_interrupts()
  a USB IRQ comes in and the main thread jumps off to process this IRQ
- that USB processing triggers a call to the dcd_event_handler() wrapper
  from commit bcbdee2
- that then calls mp_sched_schedule_node()
- that then attempts to obtain the atomic section, calling
  mp_thread_begin_atomic_section()
- that call then blocks trying to obtain atomic_mutex
- core0 is now deadlocked on itself, because the main thread has the mutex
  but the IRQ handler (which preempted the main thread) is blocked waiting
  for the mutex, which will never be free

The solution in this commit is to use mutex enter/exit functions that also
atomically disable/restore interrupts.

Fixes issues micropython#12980 and micropython#13288.

Signed-off-by: Damien George <damien@micropython.org>

* all: Bump version to 1.22.1.

Signed-off-by: Damien George <damien@micropython.org>

* Generic STM32F401CD Port Compiles (not working yet...)

* rp2/rp2_dma: Fix fetching 'write' buffers for writing not reading.

Signed-off-by: Nicko van Someren <nicko@nicko.org>

* rp2/machine_uart: Fix event wait in uart.flush() and uart.read().

Do not wait in the worst case up to the timeout.

Fixes issue micropython#13377.

Signed-off-by: robert-hh <robert@hammelrath.com>

* renesas-ra/ra: Fix SysTick clock source.

The SysTick_Config function must use the system/CPU clock to configure the
ticks.

Signed-off-by: iabdalkader <i.abdalkader@gmail.com>

* renesas-ra/boards/ARDUINO_PORTENTA_C33: Fix the RTC clock source.

Switch the RTC clock source to Sub-clock (XCIN). This board has an
accurate LSE crystal, and it should be used for the RTC clock
source.

Signed-off-by: iabdalkader <i.abdalkader@gmail.com>

* extmod/asyncio: Support gather of tasks that finish early.

Adds support to asyncio.gather() for the case that one or more (or all)
sub-tasks finish and/or raise an exception before the gather starts.

Signed-off-by: Damien George <damien@micropython.org>

* mimxrt/modmachine: Fix deepsleep wakeup pin ifdef.

Signed-off-by: Kwabena W. Agyeman <kwagyeman@live.com>

* extmod/modssl_mbedtls: Fix cipher iteration in SSLContext.get_ciphers.

Prior to this commit it would skip every second cipher returned from
mbedtls.

The corresponding test is also updated and now passes on esp32, rp2, stm32
and unix.

Signed-off-by: Damien George <damien@micropython.org>

* rp2: Change machine.I2S and rp2.DMA to use shared DMA IRQ handlers.

These separate drivers must share the DMA resource with each other.

Fixes issue micropython#13380.

Signed-off-by: Damien George <damien@micropython.org>

* py/compile: Fix potential Py-stack overflow in try-finally with return.

If a return is executed within the try block of a try-finally then the
return value is stored on the top of the Python stack during the execution
of the finally block.  In this case the Python stack is one larger than it
normally would be in the finally block.

Prior to this commit, the compiler was not taking this case into account
and could have a Python stack overflow if the Python stack used by the
finally block was more than that used elsewhere in the function.  In such
a scenario the last argument of the function would be clobbered by the
top-most temporary value used in the deepest Python expression/statement.

This commit fixes that case by making sure enough Python stack is allocated
to the function.

Fixes issue micropython#13562.

Signed-off-by: Damien George <damien@micropython.org>

* renesas-ra/ra/ra_i2c: Fix 1 byte and 2 bytes read issue.

Tested on Portenta C33 with AT24256B (addrsize=16) and SSD1306.

Fixes issue micropython#13280.

Signed-off-by: Takeo Takahashi <takeo.takahashi.xv@renesas.com>

* extmod/btstack: Reset pending_value_handle before calling write-done cb.

The pending_value_handle needs to be freed and reset before calling
mp_bluetooth_gattc_on_read_write_status(), which will call the Python IRQ
handler, which may in turn call back into BTstack to perform an action like
a write.  In that case the pending_value_handle will need to be available
for the write/read/etc to proceed.

Fixes issue micropython#13611.

Signed-off-by: Damien George <damien@micropython.org>

* extmod/btstack: Reset pending_value_handle before calling read-done cb.

Similar to the previous commit but for MP_BLUETOOTH_IRQ_GATTC_READ_DONE:
the pending_value_handle needs to be reset before calling
mp_bluetooth_gattc_on_read_write_status(), which will call the Python IRQ
handler, which may in turn call back into BTstack to perform an action like
a write.  In that case the pending_value_handle will need to be available
for the write/read/etc to proceed.

Fixes issue micropython#13634.

Signed-off-by: Damien George <damien@micropython.org>

* esp32/mpnimbleport: Release the GIL while doing NimBLE port deinit.

In case callbacks must run (eg a disconnect event happens during the
deinit) and the GIL must be obtained to run the callback.

Fixes part of issue micropython#12349.

Signed-off-by: Damien George <damien@micropython.org>

* esp32: Increase NimBLE task stack size and overflow detection headroom.

The Python BLE IRQ handler will most likely run on the NimBLE task, so its
C stack must be large enough to accommodate reasonably complicated Python
code (eg a few call depths).  So increase this stack size.

Also increase the headroom from 1024 to 2048 bytes.  This is needed because
(1) the esp32 architecture uses a fair amount of stack in general; and (2)
by the time execution gets to setting the Python stack top via
`mp_stack_set_top()` in this interlock code, about 600 bytes of stack are
already used, which reduces the amount available for Python.

Fixes issue micropython#12349.

Signed-off-by: Damien George <damien@micropython.org>

* all: Bump version to 1.22.2.

Signed-off-by: Damien George <damien@micropython.org>

* Submodule update

---------

Signed-off-by: Damien George <damien@micropython.org>
Signed-off-by: Nicko van Someren <nicko@nicko.org>
Signed-off-by: robert-hh <robert@hammelrath.com>
Signed-off-by: iabdalkader <i.abdalkader@gmail.com>
Signed-off-by: Kwabena W. Agyeman <kwagyeman@live.com>
Signed-off-by: Takeo Takahashi <takeo.takahashi.xv@renesas.com>
Co-authored-by: Damien George <damien@micropython.org>
Co-authored-by: Nicko van Someren <nicko@nicko.org>
Co-authored-by: robert-hh <robert@hammelrath.com>
Co-authored-by: iabdalkader <i.abdalkader@gmail.com>
Co-authored-by: Kwabena W. Agyeman <kwagyeman@live.com>
Co-authored-by: Takeo Takahashi <takeo.takahashi.xv@renesas.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants
0