Couldn't wait for my Pyra, so I bought a OMAP5432 devboard

[Risca]

Went on a bit of an online hunt for information about the TWL6037 and here's what I gathered:

1. The Linux device tree documentation lists the following chips as compatible with the "Palmas" series:
   • twl6035 (palmas)
   • twl6037 (palmas)
   • tps65913 (palmas)
   • tps65914 (palmas)
   • tps659038
   • tps65917
2. The "register map" of tps65913 and tps65914 is available from TI: https://e2e.ti.com/cfs-file/__key/communityserver-discussions-components-files/196/swcu132.pdf
3. A brief overview of this register map seem to match the register values in the official driver
4. I was able to find a copy of the datasheet for tps659038. Not sure if I'm allowed to link here since it's not available from TI
   
5. I also found a helpful application note on using the ADC: https://www.ti.com/lit/an/slia087a/slia087a.pdf

It looks like it should be possible to configure the TWL6037 to do periodic readings of the ADC. It's called AUTO mode in the docs. IIUC, the latest reading should always be available. It's even possible to trigger an interrupt when a certain threshold have been passed. Unfortunately, only 1 threshold and not an upper+lower (window) threshold.

 

[Djoga'Ro]

So, A15 and M4 cannot perceive the other talking to the device? How do the device's answers get routed?
If respecting each other isn't in the cards due to perception issues, I'd say the device should exclusively be talked to by only one of the cpus. That one would then have to proxy for the other - a caching proxy, if adequate/desired.

 

[Risca]

So, A15 and M4 cannot perceive the other talking to the device? How do the device's answers get routed?
If respecting each other isn't in the cards due to perception issues, I'd say the device should exclusively be talked to by only one of the cpus. That one would then have to proxy for the other - a caching proxy, if adequate/desired.

The A15 and M4 cannot perceive the other reading and writing to the memory registers that are mapped to the I2C HW. The reads and writes to the L3 main interconnect bus are synchronized by the L3 bus arbiter, but there is nothing indicating that someone else is "using" the i2c HW right now.

How this is normally solved is that each processor gets exclusive access to their own dedicated HW blocks. I.e., A15 gets the first 3 I2C blocks, 2 serial ports, the USB block, and a couple of GPIO blocks; and the M4 gets to use the 4th I2C block, the 3rd serial port, and maybe 1 or 2 GPIO blocks. You avoid it by design. Unfortunately, our (current) design doesn't allow this and we have to share (and synchronize) access. The A15 needs to access the TWL6037 for power management and the M4 needs to access it for reading the ADC. At least that's my plan

EDIT:
Failed my reading comprehension a bit. Using either the A15 or the M4 as an I2C proxy is an interesting idea. It would solve the synchronization issue, depending on how you do it. Having the A15 as the main i2c cache kinda defeats the purpose of this experiment since then both CPUs has to be awake for reading the ADC. The other way around, you could write a custom i2c driver that just forwards data to the M4. You would still have the same problem with synchronization, unless the M4 is smart enough to inspect the i2c transactions and block itself from talking to the TWL6037 when the A15 is. It's a bit complicated, but I think it would work.

I also think that HW semaphores would work, and they seem much easier to deal with. Support is already in the Linux kernel, and I believe TI has support for them in the M4s.

 

[Risca]

For those of you who are a stranger to how CPUs talk to hardware, I can say that it's not really that complicated. There are exceptions of course, and this is a bit of a simplification, but the vast majority of hardware is done by connecting everything to the same big address bus.

So, you wanna write to memory? That's address 0x80000000 to 0xFFFFFFFF (2 GB memory)
So, you wanna read the boot rom? That's address 0x00040000 to 0x00080000
So, you wanna configure the first I2C HW? That's address 0x002C04A to 0x002C080

(the above addresses are merely examples. I didn't look them up)

So, to actually send any data over I2C, you perform memory writes to the address that is associated with the I2C HW "output buffer". These HW blocks usually have internal FIFO buffers so that every time you write to the "output buffer" address, the data gets appended to this FIFO. An example how a very, very low level (fictional) driver can look like:

volatile int *i2c_config = (char*)0x002C04C;
volatile int *i2c_buffer = (char*)0x002C050;
// configure i2c hw to expect 3 bytes to address 0x42
*i2c_config = 0x42 << 8 | 3;
// copy the bytes
*i2c_buffer = 'f';
*i2c_buffer = 'o';
*i2c_buffer = 'o';
// start the transfer
*i2c_config |= 1 << 15

First we write to the configuration address that corresponds to the HW block we want to use. These addresses are written in the datasheet for the processor you're using, as well as what bits at each address means.
Secondly, we write to the "output buffer" address three times. Note that we're not increasing the address after each write but writing to the very same address 3 times. This is the part that might look strange for a pure software coder, but each write will actually be copied to the HW block's internal FIFO.
Lastly, we do a read-modify-write operation at the configuration address so that bit 15 is changed from 0 to 1. This triggers the I2C HW to start clocking out the data on the actual physical wires.

This is a very simple example with made-up values and lots of missing details, but it illustrates how a driver works in principle and hopefully it is clear enough to illustrate why it is very hard/impossible for each separate CPU to know when the other CPU is running the above code. Both CPUs will be writing to the same address (sort of - I've left out the MMU part).

 

[Djoga'Ro]

The other way around, you could write a custom i2c driver that just forwards data to the M4. You would still have the same problem with synchronization, unless the M4 is smart enough to inspect the i2c transactions and block itself from talking to the TWL6037 when the A15 is. It's a bit complicated, but I think it would work.

A := A15, M := M4, D := TWL6037
I thought more like, A knows to talk to D directly and to go through M to do so. Under normal circumstances it only does the latter.
M knows to talk to D about stuff M's interested in and also to talk to it as A would, and M provides an interface for A to use to talk to D. But M would also gather information for A regularly just to cache them and keep the cache up to date - to either cut latency short or even as a watchdog to alert/wake up A in certain situations.
Then, ideally, there was some hw facility/some address that could be set high by M frequently, but would fade if not set after a time serving for some hardbeat functionality. This way A could sleep all it wants, but always determine, if M is up and running. So A could decide, if it talks to D directly or goes through M and also decide to reset/restart M.

Note that we're not increasing the address after each write but writing to the very same address 3 times. This is the part that might look strange for a pure software coder, but each write will actually be copied to the HW block's internal FIFO.

I guess, that's what the keyword volatile is for?

 

[Risca]

I guess, that's what the keyword volatile is for?

Crap! While writing I was thinking "I need to remember to change those variables to volatile", then I completely forgot I've updated the example now

A := A15, M := M4, D := TWL6037
I thought more like, A knows to talk to D directly and to go through M to do so. Under normal circumstances it only does the latter.
M knows to talk to D about stuff M's interested in and also to talk to it as A would, and M provides an interface for A to use to talk to D. But M would also gather information for A regularly just to cache them and keep the cache up to date - to either cut latency short or even as a watchdog to alert/wake up A in certain situations.
Then, ideally, there was some hw facility/some address that could be set high by M frequently, but would fade if not set after a time serving for some hardbeat functionality. This way A could sleep all it wants, but always determine, if M is up and running. So A could decide, if it talks to D directly or goes through M and also decide to reset/restart M.

That's an interesting design! I didn't think about using M as a cache for A to avoid unnecessary reads.
The remoteproc subsystem in Linux knows when M is up or down, and can detect crashes of M.

There are a couple of challenges though:

• There are more devices than the TWL6037 connected to the I2C bus... I think. More on this later.
• All drivers that need to go through M for I2C will have to be modified. I think it might be possible to write a generic i2c driver that abstracts this so we don't have to modify every other driver, but then the M code gets more complex instead. Instead of handling the Change this TWL6037 setting command, it needs to handle Send these X bytes to I2C address 0xYYY commands instead and have to inspect each such command for possible conflicts with what M itself is doing.
• Modifying mainline drivers is a good way to create extra work for oneself when updating to newer kernel release. Things always change within the kernel itself and it can sometimes be a challenge to port patches. Kernel changes should preferably be kept at a minimum, and I find it hard to believe that mainline Linux would accept this kind of driver changes. A generic, custom I2C driver would be easier to maintain, IMO, than changes to each and every driver that talks to devices on this I2C bus

While looking through the schematics for the Pyra, I saw that there were actually two I2C connections to the TWL6037:

1. One shared I2C bus that's connected to:
   • TWL6037 - the Pyra PMIC
   • TWL6040 - audio circuit control channel.
   • AT24C01D - an optional EEPROM for reading HW revision.
   • USB3503A - an USB multiplexer.
2. One direct connection!

I did some more researching and I think this direct connection is for "Smart-Reflex Class 3 control commands (SR-I2C)". I haven't found a pinout datasheet for the OMAP5432 to verify, but it seems likely. I don't think we should be messing with that interface

 

[Djoga'Ro]

Could one "just" remap, who the drivers are addressing, via changes in the device tree? So that without changing the drivers and without them even being aware, they'd talk to M instead of the i2c interface? (And patching that might maybe safely be done with search and replace. Like, count the occurences of the string representing the address to replace; count as expected -> replace; not as expected -> call for maintainer intervention. And of course check, that new stuff in the tree doesn't conflict with replacemeant strings.)
Though I guess, that might mean M has to be able to analyze i2c communication going to D - not only synthesize such things - to be smart about what it itself is doing.

 

[Confuzzled]

Since you don't know if the 'other' cpu is reading from, or writing to the critical memory location, the obvious thing to do is co-operative locking using other shared memory locations, or a shared register. If there is no shared memory address space or registers, you can do locking by sending messages on a convenient bus. It requires cooperative locking code running on both/all processors.

Example: https://www.ibm.com/docs/en/aix/7.2?topic=programming-using-readwrite-locks

I used to do (a looooonnnggg time ago) system programming on VAXclusters where the cluster-wide locking system (the Distributed Lock Manager) was built-in and very nice to use. The OMAP 5432 with two A15 and two M4 processors is amazing when I think of clusters VAX 11/780s

VAX 11/780 - by definition is 1 Dhrystone MIPS. As it ran at 5 MHz, the DMIPS/MHz score is 0.2
Cortex M4 is 1.25 Dhrystone DMIPS/MHz
Cortex A15 is 3.5 DMIPS/MHz

I don't know if convenient Linux equivalent of the Distributed Lock Manager exists...looks like there is: https://en.wikipedia.org/wiki/Distributed_lock_manager#Linux_clustering. It might be overkill, or (I hope) it is a lean and efficient implementation...

 

[Risca]

I still think the easiest solution is to use the HW semaphores. On the Linux side, it would be something like this to synchronize the i2c driver:

--- drivers/i2c/busses/i2c-omap.c.orig    2022-08-18 23:22:38.784631762 +0200
+++ drivers/i2c/busses/i2c-omap.c    2022-08-18 23:43:04.655767062 +0200
@@ -30,6 +30,7 @@
 #include <linux/platform_data/i2c-omap.h>
 #include <linux/pm_runtime.h>
 #include <linux/pinctrl/consumer.h>
+#include <linux/hwspinlock.h>
 
 /* I2C controller revisions */
 #define OMAP_I2C_OMAP1_REV_2        0x20
@@ -211,6 +212,8 @@ struct omap_i2c_dev
     u16            syscstate;
     u16            westate;
     u16            errata;
+
+    struct hwspinlock *hwlock; /* optional hw spinlock to sync with remoteprocs */
 };
 
 static const u8 reg_map_ip_v1[] = {
@@ -800,13 +803,19 @@ omap_i2c_xfer_common(struct i2c_adapter *adap, struct i2c_msg msg
     if (r < 0)
         goto out;
 
+    if (omap->hwlock) {
+        r = hwspin_lock_timeout(omap->hwlock, 1000);
+        if (r < 0)
+            goto out;
+    }
+
     r = omap_i2c_wait_for_bb_valid(omap);
     if (r < 0)
-        goto out;
+        goto out_unlock;
 
     r = omap_i2c_wait_for_bb(omap);
     if (r < 0)
-        goto out;
+        goto out_unlock;
 
     if (omap->set_mpu_wkup_lat != NULL)
         omap->set_mpu_wkup_lat(omap->dev, omap->latency);
@@ -826,6 +835,9 @@ omap_i2c_xfer_common(struct i2c_adapter *adap, struct i2c_msg msg
     if (omap->set_mpu_wkup_lat != NULL)
         omap->set_mpu_wkup_lat(omap->dev, -1);
 
+out_unlock:
+    if (omap->hwlock)
+        hwspin_unlock(omap->hwlock);
 out:
     pm_runtime_mark_last_busy(omap->dev);
     pm_runtime_put_autosuspend(omap->dev);
@@ -1378,6 +1390,7 @@ omap_i2c_probe(struct platform_device *pdev)
 
     match = of_match_device(of_match_ptr(omap_i2c_of_match), &pdev->dev);
     if (match) {
+        u8 spinlock;
         u32 freq = I2C_MAX_STANDARD_MODE_FREQ;
 
         pdata = match->data;
@@ -1386,6 +1399,14 @@ omap_i2c_probe(struct platform_device *pdev)
         of_property_read_u32(node, "clock-frequency", &freq);
         /* convert DT freq value in Hz into kHz for speed */
         omap->speed = freq / 1000;
+
+        if (0 == of_property_read_u8(node, "spinlock-id", &spinlock)) {
+            omap->hwlock = hwspin_lock_request_specific(spinlock);
+            if (omap->hwlock == NULL) {
+                r = -ENODEV;
+                goto err_disable_pm;
+            }
+        }
     } else if (pdata != NULL) {
         omap->speed = pdata->clkrate;
         omap->flags = pdata->flags;
@@ -1524,6 +1545,9 @@ omap_i2c_remove(struct platform_device *pdev)
     struct omap_i2c_dev    *omap = platform_get_drvdata(pdev);
     int ret;
 
+    if (omap->hwlock)
+        hwspin_lock_free(omap->hwlock);
+
     i2c_del_adapter(&omap->adapter);
     ret = pm_runtime_resume_and_get(&pdev->dev);
     if (ret < 0)

HW spinlocks are already supported by the Linux kernel, so let's use that! The above patch compiles, but needs a device tree update and testing. It probably needs a few changes for interrupts and such

 

[Risca]

Could one "just" remap, who the drivers are addressing, via changes in the device tree? So that without changing the drivers and without them even being aware, they'd talk to M instead of the i2c interface? (And patching that might maybe safely be done with search and replace. Like, count the occurences of the string representing the address to replace; count as expected -> replace; not as expected -> call for maintainer intervention. And of course check, that new stuff in the tree doesn't conflict with replacemeant strings.)
Though I guess, that might mean M has to be able to analyze i2c communication going to D - not only synthesize such things - to be smart about what it itself is doing.

This is exactly the things I've tried to talk about in my previous replies.

Could one "just" remap, who the drivers are addressing, via changes in the device tree? So that without changing the drivers and without them even being aware, they'd talk to M instead of the i2c interface?

This would be solved by writing a custom I2C driver. It would be transparent for every driver that needs to talk I2C.

Though I guess, that might mean M has to be able to analyze i2c communication going to D - not only synthesize such things - to be smart about what it itself is doing.

Exactly! It increases the complexity for M.

 

[Djoga'Ro]

Do you want to talk to a device (from A and M) that another, not-to-be-changed driver already also talks to? That would complicate any attempt on synchronization, too.
If not, what the redirecting i2c driver sends to M, M would just relay/synthesize. No, you wouldn't even need it, for there'd be no potential for inconsistency. For what your code on A wants it would not go through the i2c driver, but talk on a higher abstraction level to M. ?
Or maybe I just got lost in the problem space.

 

[Risca]

Do you want to talk to a device (from A and M) that another, not-to-be-changed driver already also talks to? That would complicate any attempt on synchronization, too.
If not, what the redirecting i2c driver sends to M, M would just relay/synthesize. No, you wouldn't even need it, for there'd be no potential for inconsistency. For what your code on A wants it would not go through the i2c driver, but talk on a higher abstraction level to M. ?
Or maybe I just got lost in the problem space.

There is no question that any driver on A that talks to a device D1, which M also want to talk to, will have to be synchronized. However, for device D2, D3, etc., I don't see a need for synchronizing their drivers unless M also wants to talk to those devices.
At the bare minimum, the i2c driver needs to be synchronized, and I think the way to go is the patch I showed above.

 

[Risca]

So, wiggling GPIO pins from the M4 works!

I couldn't find any documentation on how to do it properly or find any official drivers, so it was super dirty instead Just look up the registers I need to manipulate in the datasheet, convert their addresses to M4 address space, and voila:

// 0xA8000000 in our world is 0x48000000 in outside world
#define GPIO5_DATAOUT       *(UInt32*)(0xA805B13C)
#define GPIO5_CLEARDATAOUT  *(UInt32*)(0xA805B190)
#define GPIO5_SETDATAOUT    *(UInt32*)(0xA805B194)

static Void pingCallbackFxn(RPMessage_Handle h, UArg arg, Ptr data,
    UInt16 len, UInt32 src)
{
    static UInt32 counter = 0;
    if (counter++ % 2) {
        GPIO5_CLEARDATAOUT = (1 << LED_PIN);
    }
    else {
        GPIO5_SETDATAOUT = (1 << LED_PIN);
    }
}

I modified the ping_rpmsg example code to toggle the blue LED on the board every time it received a ping.

It's super hacky, but works! I updated my old repo with the new code: https://github.com/Risca/hello_rpmsg

 

[Risca]

I took a quick look at the Linux I2C driver and it shouldn't be that hard to implement a simple driver in the M4. We fortunately doesn't have to cover all the little corner cases that the Linux driver has to, and we can focus on the polling path and ignore interrupts.

I'm busy this weekend, so we'll see when I have time to look at this again

 

[Djoga'Ro]

So, wiggling GPIO pins from the M4 works!

I couldn't find any documentation on how to do it properly or find any official drivers, so it was super dirty instead Just look up the registers I need to manipulate in the datasheet, convert their addresses to M4 address space, and voila:

// 0xA8000000 in our world is 0x48000000 in outside world
#define GPIO5_DATAOUT       *(UInt32*)(0xA805B13C)
#define GPIO5_CLEARDATAOUT  *(UInt32*)(0xA805B190)
#define GPIO5_SETDATAOUT    *(UInt32*)(0xA805B194)

static Void pingCallbackFxn(RPMessage_Handle h, UArg arg, Ptr data,
    UInt16 len, UInt32 src)
{
    static UInt32 counter = 0;
    if (counter++ % 2) {
        GPIO5_CLEARDATAOUT = (1 << LED_PIN);
    }
    else {
        GPIO5_SETDATAOUT = (1 << LED_PIN);
    }
}

I modified the ping_rpmsg example code to toggle the blue LED on the board every time it received a ping.

It's super hacky, but works! I updated my old repo with the new code: https://github.com/Risca/hello_rpmsg

What's the hack?
Are all relevant device addresses offset like that in M in respect to how they are in A? You could use same source files for both, if you incorporate a #define for the offset - separate sources for the actual writing and reading devices, that is. Or if offsets vary, apply it always, but with a #defined factor of 0 for A and one of 1 for M.

 

[Risca]

What's the hack?
Are all relevant device addresses offset like that in M in respect to how they are in A? You could use same source files for both, if you incorporate a #define for the offset - separate sources for the actual writing and reading devices, that is. Or if offsets vary, apply it always, but with a #defined factor of 0 for A and one of 1 for M.

First of all, my defines doesn't use the values written in the datasheet. That's a source of confusion. Sure, I left a comment in there, but that comment fail to explain how that address mapping happened.

The address mapping (or offset) is part of my firmware. Linux will read my firmware and setup the MMU with this mapping, and then launch my program on the M4. It's all part of the resource table I've written about earlier in this thread. This example project is using a default resource table from TI but if I decide to use my own custom resource table, and change this mapping, my defines would be wrong again. TI has code that can read the resource table and look up this address mapping, so I could change my defines to what the datasheet says and then do runtime lookup what the address would be on the M4, but I was too lazy. I might do that when I start to get serious

Lastly, these addresses are only guaranteed to be valid for OMAP5432, and running this code on e.g. OMAP5430 might end up setting my monitor color depth to 12 instead of flipping a pin! Software vendors tend to provide big header files for each SoC they sell where all peripheral addresses are listed, and my program only needs to know the name of the define. Then, it's only a compile time config which header file to include and I'll get the correct address. I couldn't find any such defines in TI/SYSBIOS, XDC tools, or IPC (though I know later MCU+SDK releases from TI has them, but for other processors), so I got impatient and just wrote the raw addresses I knew would work for my particular program on this particular SoC

 

[Risca]

Another common technique is to define the whole peripheral register space as a struct and then cast the base address of the peripheral to this struct. In my case it would look something like this:

struct __attribute__((__packed__)) GPIO {
    UInt8 padding1[0x13C];
    UInt32 DATAOUT;
    UInt8 padding2[0x50];
    UInt32 CLEARDATAOUT;
    UInt32 SETDATAOUT;
};

#define GPIO5 (struct GPIO*)(0x4805B000)

GPIO5->CLEARDATAOUT = (1 << LED_PIN);

The OMAP5432 has some rather funny jumps in its address table it seems. Most MCUs I've worked with, all registers were linearly aligned after one another but here you have big jumps Funny

 

[Letalis Sonus]

The OMAP5432 has some rather funny jumps in its address table it seems. Most MCUs I've worked with, all registers were linearly aligned after one another but here you have big jumps Funny

Eh, it's pretty common on 32-bit MCUs. Even ancient e200 PowerPC MCUs tend to have some fairly sizeable gaps in their modules' memory maps. Sometimes it makes things easier because it makes code more compatible across module revisions with heavy changes, sometimes the developers simply exploit that the available memory ranges are just large enough that you don't need to care much about it.

Most of the time the packed attribute should not even be necessary, because on non-x86 hardware the hard alignment requirements require all registers to be properly aligned anyway. As long as you use 8-bit data types for the padding there should be absolutely no reason for the compiler to insert any gaps by itself.

 

[Risca]

So, after some datasheet reading and pondering, I have an idea!

The datasheets that I've read for various "palmas" type devices (i.e. TWL6037) says that when the device is in "AUTO" mode then 1 or 2 channels can be periodically sampled. For each channel being sampled, a threshold can be set and if the current voltage level reaches above or below that threshold then an interrupt is triggered. I originally dismissed this threshold functionality because we want to be able to detect both above and below, but what if the same channel can be configured twice?! There is basically 2 "slots" to program channel, threshold value, and whether the threshold is high or low. Maybe we can:

• program "slot 1" with
  • channel GPADC2,
  • a threshold value slightly higher than current value, and
  • to trigger an interrupt when the current value reaches above this threshold
• program "slot 2" with
  • channel GPADC2,
  • a threshold value slightly lower than current value, and
  • to trigger an interrupt when the current value reaches below this threshold

If this is allowed and well defined behavior of the TWL6037, then we should get notified automatically by the TWL6037 if the audio wheel is moved in either direction.

Unfortunately, there is a serious lack of public documentation for the TWL6037 and also no schematics for my devboard so I have a bit of a hard time testing this. I have no idea where GPADC2 is routed on my board maybe that doesn't matter? The automatic conversion should show the same value as a manually triggered conversion. I just can't test the trigger functionality. I might give this a shot

 

[Risca]

Okay, so this was easier to test than I first anticipated I wasn't able to test the thresholds, but I can confirm that it's possible to program both "slots" with the same channel! I noticed that channel 7 on my devboard was changing values up and down a bit so I setup TWL6037 to continuously convert that channel:

# cat /sys/bus/iio/devices/iio:device0/in_voltage7_input 
[  209.807210] palmas-gpadc 48070000.i2c:palmas@48:gpadc: AUTO_CONV0_LSB read: 3052
[  209.815290] palmas-gpadc 48070000.i2c:palmas@48:gpadc: AUTO_CONV1_LSB read: 3052
3725
# cat /sys/bus/iio/devices/iio:device0/in_voltage7_input 
[  210.247882] palmas-gpadc 48070000.i2c:palmas@48:gpadc: AUTO_CONV0_LSB read: 3052
[  210.255814] palmas-gpadc 48070000.i2c:palmas@48:gpadc: AUTO_CONV1_LSB read: 3052
3725
# cat /sys/bus/iio/devices/iio:device0/in_voltage7_input
[  210.727360] palmas-gpadc 48070000.i2c:palmas@48:gpadc: AUTO_CONV0_LSB read: 3053
[  210.735477] palmas-gpadc 48070000.i2c:palmas@48:gpadc: AUTO_CONV1_LSB read: 3020
3686

That last reading should indicate that both registers are updated individually!