Read/write to multiple IO region doesn't seem to work
Closed this issue · 7 comments
Hello,
I have a device with 2 blocks of memory-mapped register that appear as 2 IO regions within a single device.
I've opened the device (metal_device_open()), mapped the 2 IO regions (metal_device_io_region() and checked their physical base addresses (metal_io_phys()). All looks good.
However, when I try to read or write using metal_io_read32() and metal_io_write32() to either IO region, I only seem to be accessing the 1st region. For example, if I write to IO region 1 it affects the phsyical memory for IO region 0; if I read from IO region 1 it appears to actually reads from IO region 0. Verified with devmem. I've got debug code which prints out the physical address just before I call metal_io_read32() or metal_io_write32() and it is correct.
I should add this is on Linux with QEMU session.
Any ideas what the problem could be?
Hi @sjd-xlnx thanks for raising this. can you show the sample metal_device_io_region, read and write code blocks?
Minimal code example here. The device "a40d0000.my_device" has 2 IO regions, at address a40d0000 and a4030000.
#include <inttypes.h>
#include <stdio.h>
#include <stdlib.h>
#include <metal/sys.h>
#include <metal/device.h>
#include <metal/io.h>
#include <metal/irq.h>
struct device_info
{
struct metal_device *dev;
struct metal_io_region *io[2];
};
struct device_info device;
int main(void)
{
// Parameters define log handlers and log level for libmetal
struct metal_init_params init_param =
{
.log_handler = metal_default_log_handler,
.log_level = METAL_LOG_INFO,
};
// Initialize libmetal
if (metal_init(&init_param))
{
printf("Failed to initialize libmetal\n");
return 1;
}
// Open device
if (metal_device_open("platform", "a40d0000.my_device", &(device.dev)))
{
printf("Failed to open device\n");
return 1;
}
// Map both IO regions
for (int i = 0; i < 2; ++i)
{
// Map IO region
device.io[i] = metal_device_io_region(device.dev, i);
if (device.io[i] != NULL)
{
printf("Mapped IO region (%d)\n", i);
printf("Physical base adddress = 0x%lX\n", metal_io_phys(device.io[i], 0));
printf("base addr of IO region %lx\n", metal_io_virt_to_phys(device.io[i], device.io[i]->virt));
}
else
{
printf("Failed to map IO region (%d)\n", i);
}
}
// Now try to write to IO regions
metal_io_write32(device.io[0], 0x2100, 1); // write 1 to IO region #0
metal_io_write32(device.io[1], 0x2100, 2); // write 2 to IO region #1
// Now try to read from the IO regions
uint32_t val1 = metal_io_read32(device.io[0], 0x2100);
uint32_t val2 = metal_io_read32(device.io[1], 0x2100);
printf("val1 = 0x%X, val2 = 0x%X\n", val1, val2);
if ((val1 == 1) && (val2 == 2))
{
printf("PASS\n");
}
else
{
printf("FAIL\n");
}
return 0;
}
I think I found the problem. Please take a look in /lib/system/linux/device.c, function metal_uio_dev_open(). It seems there was a big in the mmap() call, which was fixed in the OpenAMP/libmetal version, but not the Xilinx/libmetal fork that I'm using. I'm currently patching to see if this fixes the issue.
I have tested my build with this one-line fix from OpenAMP/libmetal, and I can confirm that this fixes the issue, and with it, I can read/write to both IO regions. I am therefore closing this Issue on OpenAMP/libmetal repo, and will pursue a fix in the Xilinx/libmetal fork. Thanks.
@sjd-xlnx I know this is probably not the place to ask this but not sure where else: Could you show what your device looked like in the system-user.dtsi? I am trying to map a single slave device to multiple io regions but am having some issues. When I make a call to metal_device_io_region(my_device, index) it succeeds in mapping for index 0 but not for index 1. Thank you.
@r-kyle15 Here's an extract for our device. I've obfuscated bits that are commercially sensitive, but you can get the gist.
QQQ@a4030000 {
clock-names = "AAA_clk\0BBB_clk\0CCC_clk\0DDD_clk\0EEE_clk\0S_FFF_aclk";
clocks = <0x13 0x14 0x14 0x15 0x14 0x11>;
compatible = "ZZZ,QQQ-1.2";
interrupt-names = "interrupt";
interrupt-parent = <0x16>;
interrupts = <0x01 0x02>;
reg = <0x00 0xa4030000 0x00 0x10000 0x00 0xa4040000 0x00 0x10000>;
phandle = <0x62>;
};
The important bit is the "reg" line, i.e. reg = <0x00 0xa4030000 0x00 0x10000 0x00 0xa4040000 0x00 0x10000>;
It's defining that the device has 2 memory mapped regions; the first starting at 0xa4030000 and the second starting at 0xa4040000.
With this configuration, I was able to "metal_device_open" the device QQQ@a4030000, and then use "metal_device_io_region" on that device with an index 0 and index 1. Code fragment below...
// Open the device
metal_device_open(bus_name, "QQQ@a4030000", &(device->dev)))
for (int i = 0; i < 2; ++i)
{
// Map the IO region
device->io[i] = metal_device_io_region(device->dev, i);
Then the rest of the code can access either device through the device->io pointer. Does that make sense?
I'm not sure from your description if your design is like that? My situation was 1 device with 2 IO regions. In HW this was basically 2 identical devices (each with own register bank) instantiated inside a top-level block with some additional logic. When generating the device tree from this HW configuration it ends up as 1 device with 2 IO regions.
Hope this helps.
@sjn-xlnx thank you for your response!
That does make sense, my situation is a single port slave device(essentially just a register bank) with contiguous IO region. I wanted to split it up into 3 separate devices based on functionality, but seems like the metal_device_io_region() call finds the first region(the one at the base address of the entire register bank) but unable to open the other 2. I believe I updated the device tree properly because I can see 3 mappings at the offsets when I drill down into the device in /sys/devices. But, maybe splitting up a single slave contiguous region is not supported by libmetal?
I have ended up making a single device and "modularizing" it in my Linux application code. Not my preference but I could not get past io region mapping issue.