* Barebox x86 IDE support
@ 2014-03-14 17:01 Stam, Michel [FINT]
0 siblings, 0 replies; 4+ messages in thread
From: Stam, Michel [FINT] @ 2014-03-14 17:01 UTC (permalink / raw)
To: barebox
[-- Attachment #1: Type: text/plain, Size: 1294 bytes --]
Dear list,
While working on barebox recently, I decided to port a couple of patches I made to an old barebox tree a few years ago, to fix the IDE ports on the x86 platform.
I had to work on the resource allocation a bit, as the current GIT repository does not allow IORESOURCE_IO type to be passed to dev_request_region( ) and friends. I tried to keep the interface the same as much as possible by using macros etc.
Please find that attached in 0001-common-driver-Allow-for-I-O-mapped-I-O-in-resource-r.patch.
Next, I added support to the platform IDE interface to use I/O-mapped I/O, and added the necessary allocation to the generic_pc board.
I did have to add a call to ata_port_detect( ) to ide_port_register to actually get the drives to be probed. Without it the interface seems to be stuck in some sort of limbo. This patch is in 0002-x86-Add-support-for-IDE-on-the-legacy-I-O-ports.patch.
Lastly, I added an extra serial port to the generic_pc board. Most (legacy) PCs have support for 2 serial ports, it seemed useful to me.
What does puzzle me is that the arch/x86/boot/Kconfig mentions X86_VESA and X86_VGA support, but the files (console_vga.c and console_vesa.c) do not nor have ever existed. Is this a remnant from U-boot v1?
Best regards,
Michel Stam
[-- Attachment #2: 0001-common-driver-Allow-for-I-O-mapped-I-O-in-resource-r.patch --]
[-- Type: application/octet-stream, Size: 8506 bytes --]
From bd3ba83d233fd28edef074831e9130170556feb2 Mon Sep 17 00:00:00 2001
From: Michel Stam <m.stam@fugro.nl>
Date: Fri, 7 Mar 2014 15:56:52 +0100
Subject: [PATCH 1/3] common: driver: Allow for I/O mapped I/O in resource
requests
The current resource framework is fully geared towards memory
mapped I/O.
This patch adds support for other types of I/O in the resource
structures / function calls.
---
common/memory.c | 2 +-
common/resource.c | 18 ++++++++++++++++--
drivers/base/driver.c | 36 ++++++++++++++++++------------------
include/driver.h | 24 +++++++++++++++++-------
include/linux/ioport.h | 5 ++++-
5 files changed, 56 insertions(+), 29 deletions(-)
diff --git a/common/memory.c b/common/memory.c
index c82bbaa..d36c8df 100644
--- a/common/memory.c
+++ b/common/memory.c
@@ -148,7 +148,7 @@ struct resource *request_sdram_region(const char *name, resource_size_t start,
for_each_memory_bank(bank) {
struct resource *res;
- res = request_region(bank->res, name, start, start + size - 1);
+ res = request_region(bank->res, name, start, start + size - 1, IORESOURCE_MEM);
if (res)
return res;
}
diff --git a/common/resource.c b/common/resource.c
index 5795e79..6600444 100644
--- a/common/resource.c
+++ b/common/resource.c
@@ -38,7 +38,7 @@ static int init_resource(struct resource *res, const char *name)
*/
struct resource *request_region(struct resource *parent,
const char *name, resource_size_t start,
- resource_size_t end)
+ resource_size_t end, int type)
{
struct resource *r, *new;
@@ -70,6 +70,10 @@ struct resource *request_region(struct resource *parent,
goto ok;
if (start > r->end)
continue;
+
+ if (type != resource_type(r->parent))
+ continue;
+
debug("%s: 0x%08llx:0x%08llx conflicts with 0x%08llx:0x%08llx\n",
__func__,
(unsigned long long)start,
@@ -89,6 +93,7 @@ ok:
new->start = start;
new->end = end;
new->parent = parent;
+ new->flags = type;
list_add_tail(&new->sibling, &r->sibling);
return new;
@@ -123,5 +128,14 @@ struct resource iomem_resource = {
struct resource *request_iomem_region(const char *name,
resource_size_t start, resource_size_t end)
{
- return request_region(&iomem_resource, name, start, end);
+ return request_region(&iomem_resource, name, start, end, IORESOURCE_MEM);
+}
+
+/*
+ * request an io region inside the io space
+ */
+struct resource *request_io_region(const char *name,
+ resource_size_t start, resource_size_t end,int type)
+{
+ return request_region(&iomem_resource, name, start, end, type);
}
diff --git a/drivers/base/driver.c b/drivers/base/driver.c
index 37560fd..560579a 100644
--- a/drivers/base/driver.c
+++ b/drivers/base/driver.c
@@ -241,13 +241,13 @@ int register_driver(struct driver_d *drv)
}
EXPORT_SYMBOL(register_driver);
-struct resource *dev_get_resource(struct device_d *dev, int num)
+struct resource *dev_get_resource(struct device_d *dev, int num, int type)
{
int i, n = 0;
for (i = 0; i < dev->num_resources; i++) {
struct resource *res = &dev->resource[i];
- if (resource_type(res) == IORESOURCE_MEM) {
+ if (!type || (resource_type(res) == type)) {
if (n == num)
return res;
n++;
@@ -257,26 +257,26 @@ struct resource *dev_get_resource(struct device_d *dev, int num)
return NULL;
}
-void *dev_get_mem_region(struct device_d *dev, int num)
+void *dev_get_region(struct device_d *dev, int num, int type)
{
struct resource *res;
- res = dev_get_resource(dev, num);
+ res = dev_get_resource(dev, num, type);
if (!res)
return NULL;
return (void __force *)res->start;
}
-EXPORT_SYMBOL(dev_get_mem_region);
+EXPORT_SYMBOL(dev_get_region);
struct resource *dev_get_resource_by_name(struct device_d *dev,
- const char *name)
+ const char *name, int type)
{
int i;
for (i = 0; i < dev->num_resources; i++) {
struct resource *res = &dev->resource[i];
- if (resource_type(res) != IORESOURCE_MEM)
+ if (!type && (resource_type(res) != type))
continue;
if (!res->name)
continue;
@@ -287,49 +287,49 @@ struct resource *dev_get_resource_by_name(struct device_d *dev,
return NULL;
}
-void *dev_get_mem_region_by_name(struct device_d *dev, const char *name)
+void *dev_get_region_by_name(struct device_d *dev, const char *name, int type)
{
struct resource *res;
- res = dev_get_resource_by_name(dev, name);
+ res = dev_get_resource_by_name(dev, name, type);
if (!res)
return NULL;
return (void __force *)res->start;
}
-EXPORT_SYMBOL(dev_get_mem_region_by_name);
+EXPORT_SYMBOL(dev_get_region_by_name);
-void __iomem *dev_request_mem_region_by_name(struct device_d *dev, const char *name)
+void __iomem *dev_request_region_by_name(struct device_d *dev, const char *name, int type)
{
struct resource *res;
- res = dev_get_resource_by_name(dev, name);
+ res = dev_get_resource_by_name(dev, name, type);
if (!res)
return NULL;
- res = request_iomem_region(dev_name(dev), res->start, res->end);
+ res = request_io_region(dev_name(dev), res->start, res->end, type);
if (!res)
return NULL;
return (void __force __iomem *)res->start;
}
-EXPORT_SYMBOL(dev_request_mem_region_by_name);
+EXPORT_SYMBOL(dev_request_region_by_name);
-void __iomem *dev_request_mem_region(struct device_d *dev, int num)
+void __iomem *dev_request_region(struct device_d *dev, int num,int type)
{
struct resource *res;
- res = dev_get_resource(dev, num);
+ res = dev_get_resource(dev, num, type);
if (!res)
return NULL;
- res = request_iomem_region(dev_name(dev), res->start, res->end);
+ res = request_io_region(dev_name(dev), res->start, res->end, type);
if (!res)
return NULL;
return (void __force __iomem *)res->start;
}
-EXPORT_SYMBOL(dev_request_mem_region);
+EXPORT_SYMBOL(dev_request_region);
int dev_protect(struct device_d *dev, size_t count, unsigned long offset, int prot)
{
diff --git a/include/driver.h b/include/driver.h
index 01b181d..2eb4cf8 100644
--- a/include/driver.h
+++ b/include/driver.h
@@ -203,31 +203,41 @@ static inline const char *dev_name(const struct device_d *dev)
/*
* get resource 'num' for a device
*/
-struct resource *dev_get_resource(struct device_d *dev, int num);
+struct resource *dev_get_resource(struct device_d *dev, int num, int type);
/*
* get resource base 'name' for a device
*/
struct resource *dev_get_resource_by_name(struct device_d *dev,
- const char *name);
+ const char *name, int type);
+
/*
+ *
* get register base 'name' for a device
*/
-void *dev_get_mem_region_by_name(struct device_d *dev, const char *name);
+void *dev_get_region_by_name(struct device_d *dev, const char *name, int type);
/*
* exlusively request register base 'name' for a device
*/
-void __iomem *dev_request_mem_region_by_name(struct device_d *dev,
- const char *name);
+void __iomem *dev_request_region_by_name(struct device_d *dev,
+ const char *name, int type);
/*
* get register base 'num' for a device
*/
-void *dev_get_mem_region(struct device_d *dev, int num);
+void *dev_get_region(struct device_d *dev, int num, int type);
/*
* exlusively request register base 'num' for a device
*/
-void __iomem *dev_request_mem_region(struct device_d *dev, int num);
+void __iomem *dev_request_region(struct device_d *dev, int num, int type);
+
+/*
+ * Macros for requesting memory resources
+ */
+#define dev_get_mem_region(dev, num) dev_get_region(dev, num, IORESOURCE_MEM)
+#define dev_get_mem_region_by_name(dev, name) dev_get_region_by_name(dev, name, IORESOURCE_MEM)
+#define dev_request_mem_region_by_name(dev, name) dev_request_region_by_name(dev, name, IORESOURCE_MEM)
+#define dev_request_mem_region(dev, num) dev_request_region(dev, num, IORESOURCE_MEM)
struct device_d *device_alloc(const char *devname, int id);
diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index ff0cba0..3fc2665 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -138,9 +138,12 @@ static inline unsigned long resource_type(const struct resource *res)
struct resource *request_iomem_region(const char *name,
resource_size_t start, resource_size_t end);
+struct resource *request_io_region(const char *name,
+ resource_size_t start, resource_size_t end, int type);
+
struct resource *request_region(struct resource *parent,
const char *name, resource_size_t end,
- resource_size_t size);
+ resource_size_t size,int type);
int release_region(struct resource *res);
--
1.8.4
[-- Attachment #3: 0002-x86-Add-support-for-IDE-on-the-legacy-I-O-ports.patch --]
[-- Type: application/octet-stream, Size: 9238 bytes --]
From f39fe4c00782947092304b84215e5bf583e9c756 Mon Sep 17 00:00:00 2001
From: Michel Stam <m.stam@fugro.nl>
Date: Mon, 10 Mar 2014 10:25:35 +0100
Subject: [PATCH 2/3] x86: Add support for IDE on the legacy I/O ports
---
arch/x86/boards/x86_generic/generic_pc.c | 71 ++++++++++++++++++++++++
drivers/ata/ide-sff.c | 94 ++++++++++++++++++++++++++------
drivers/ata/intf_platform_ide.c | 14 ++++-
include/ata_drive.h | 1 +
4 files changed, 161 insertions(+), 19 deletions(-)
diff --git a/arch/x86/boards/x86_generic/generic_pc.c b/arch/x86/boards/x86_generic/generic_pc.c
index 74a7224..6152afc 100644
--- a/arch/x86/boards/x86_generic/generic_pc.c
+++ b/arch/x86/boards/x86_generic/generic_pc.c
@@ -27,6 +27,10 @@
#include <ns16550.h>
#include <linux/err.h>
+#ifdef CONFIG_DISK_INTF_PLATFORM_IDE
+#include <platform_ide.h>
+#endif
+
/*
* These datas are from the MBR, created by the linker and filled by the
* setup tool while installing barebox on the disk drive
@@ -41,6 +45,55 @@ extern uint8_t pers_env_drive;
*/
#define PATCH_AREA_PERS_SIZE_UNUSED 0x000
+#ifdef CONFIG_DISK_INTF_PLATFORM_IDE
+
+static struct ide_port_info ide_plat = {
+ .ioport_shift = 0,
+ .dataif_be = 0,
+};
+
+static struct resource primary_ide_resources[] = {
+ {
+ .name = "base",
+ .start = 0x1f0,
+ .end = 0x1f8,
+ .flags = IORESOURCE_IO
+ },
+ {
+ .name = "alt",
+ .start = 0x3f6,
+ .end = 0x3f8,
+ .flags = IORESOURCE_IO
+ }
+};
+
+static struct resource secondary_ide_resources[] = {
+ {
+ .name = "base",
+ .start = 0x170,
+ .end = 0x177,
+ .flags = IORESOURCE_IO
+ },
+};
+
+static struct device_d primary_ide_device = {
+ .name = "ide_intf",
+ .id = 0,
+ .platform_data = &ide_plat,
+ .resource = primary_ide_resources,
+ .num_resources = ARRAY_SIZE( primary_ide_resources ),
+};
+
+static struct device_d secondary_ide_device = {
+ .name = "ide_intf",
+ .id = 1,
+ .platform_data = &ide_plat,
+ .resource = secondary_ide_resources,
+ .num_resources = ARRAY_SIZE( secondary_ide_resources ),
+};
+
+#endif /* CONFIG_DISK_INTF_PLATFORM_IDE */
+
static int devices_init(void)
{
struct cdev *cdev;
@@ -48,9 +101,26 @@ static int devices_init(void)
/* extended memory only */
add_mem_device("ram0", 0x0, bios_get_memsize() << 10,
IORESOURCE_MEM_WRITEABLE);
+#ifdef CONFIG_DISK_BIOS
add_generic_device("biosdrive", DEVICE_ID_DYNAMIC, NULL, 0, 0, IORESOURCE_MEM,
NULL);
+#endif
+
+#ifdef CONFIG_DISK_INTF_PLATFORM_IDE
+ platform_device_register( &primary_ide_device );
+ platform_device_register( &secondary_ide_device );
+
+ if (pers_env_size != PATCH_AREA_PERS_SIZE_UNUSED) {
+ cdev = devfs_add_partition("ata0",
+ pers_env_storage * 512,
+ (unsigned)pers_env_size * 512,
+ DEVFS_PARTITION_FIXED, "env0");
+ printf("Partition: %ld\n", IS_ERR(cdev) ? PTR_ERR(cdev) : 0);
+ } else
+ printf("No persistent storage defined\n");
+#endif /* CONFIG_DISK_INTF_PLATFORM_IDE */
+#ifdef CONFIG_DISK_BIOS
if (pers_env_size != PATCH_AREA_PERS_SIZE_UNUSED) {
cdev = devfs_add_partition("biosdisk0",
pers_env_storage * 512,
@@ -59,6 +129,7 @@ static int devices_init(void)
printf("Partition: %ld\n", IS_ERR(cdev) ? PTR_ERR(cdev) : 0);
} else
printf("No persistent storage defined\n");
+#endif
return 0;
}
diff --git a/drivers/ata/ide-sff.c b/drivers/ata/ide-sff.c
index 3d5932e..809a129 100644
--- a/drivers/ata/ide-sff.c
+++ b/drivers/ata/ide-sff.c
@@ -15,13 +15,71 @@
#define DISK_SLAVE 1
/**
+ * Read a byte from the ATA controller
+ * @param ide IDE port structure
+ * @param addr Register adress
+ * @return Register's content
+ */
+static inline uint8_t ata_rd_byte(struct ide_port *ide, void __iomem *addr )
+{
+ if (!ide->io.mmio)
+ return (uint8_t) inb((int) addr);
+ else
+ return readb(addr);
+}
+
+/**
+ * Write a byte to the ATA controller
+ * @param ide IDE port structure
+ * @param value Value to write
+ * @param addr Register adress
+ * @return Register's content
+ */
+static inline void ata_wr_byte(struct ide_port *ide, uint8_t value, void __iomem *addr )
+{
+ if (!ide->io.mmio)
+ outb(value, (int) addr);
+ else
+ writeb(value,addr);
+}
+
+/**
+ * Read a word from the ATA controller
+ * @param ide IDE port structure
+ * @param addr Register adress
+ * @return Register's content
+ */
+static inline uint16_t ata_rd_word(struct ide_port *ide, void __iomem *addr )
+{
+ if (!ide->io.mmio)
+ return (uint16_t) inw((int) addr);
+ else
+ return readw(addr);
+}
+
+/**
+ * Write a word to the ATA controller
+ * @param ide IDE port structure
+ * @param value Value to write
+ * @param addr Register adress
+ * @return Register's content
+ */
+static inline void ata_wr_word(struct ide_port *ide, uint16_t value, void __iomem *addr )
+{
+ if (!ide->io.mmio)
+ outw(value, (int) addr);
+ else
+ writew(value,addr);
+}
+
+/**
* Read the status register of the ATA drive
* @param io Register file
* @return Register's content
*/
static uint8_t ata_rd_status(struct ide_port *ide)
{
- return readb(ide->io.status_addr);
+ return ata_rd_byte(ide,ide->io.status_addr);
}
/**
@@ -83,12 +141,12 @@ static int ata_set_lba_sector(struct ide_port *ide, unsigned drive, uint64_t num
if (num > 0x0FFFFFFF || drive > 1)
return -EINVAL;
- writeb(0xA0 | LBA_FLAG | drive << 4 | num >> 24, ide->io.device_addr);
- writeb(0x00, ide->io.error_addr);
- writeb(0x01, ide->io.nsect_addr);
- writeb(num, ide->io.lbal_addr); /* 0 ... 7 */
- writeb(num >> 8, ide->io.lbam_addr); /* 8 ... 15 */
- writeb(num >> 16, ide->io.lbah_addr); /* 16 ... 23 */
+ ata_wr_byte(ide, 0xA0 | LBA_FLAG | drive << 4 | num >> 24, ide->io.device_addr);
+ ata_wr_byte(ide, 0x00, ide->io.error_addr);
+ ata_wr_byte(ide, 0x01, ide->io.nsect_addr);
+ ata_wr_byte(ide, num, ide->io.lbal_addr); /* 0 ... 7 */
+ ata_wr_byte(ide, num >> 8, ide->io.lbam_addr); /* 8 ... 15 */
+ ata_wr_byte(ide, num >> 16, ide->io.lbah_addr); /* 16 ... 23 */
return 0;
}
@@ -107,7 +165,7 @@ static int ata_wr_cmd(struct ide_port *ide, uint8_t cmd)
if (rc != 0)
return rc;
- writeb(cmd, ide->io.command_addr);
+ ata_wr_byte(ide, cmd, ide->io.command_addr);
return 0;
}
@@ -118,7 +176,7 @@ static int ata_wr_cmd(struct ide_port *ide, uint8_t cmd)
*/
static void ata_wr_dev_ctrl(struct ide_port *ide, uint8_t val)
{
- writeb(val, ide->io.ctl_addr);
+ ata_wr_byte(ide, val, ide->io.ctl_addr);
}
/**
@@ -133,10 +191,10 @@ static void ata_rd_sector(struct ide_port *ide, void *buf)
if (ide->io.dataif_be) {
for (; u > 0; u--)
- *b++ = be16_to_cpu(readw(ide->io.data_addr));
+ *b++ = be16_to_cpu(ata_rd_word(ide, ide->io.data_addr));
} else {
for (; u > 0; u--)
- *b++ = le16_to_cpu(readw(ide->io.data_addr));
+ *b++ = le16_to_cpu(ata_rd_word(ide, ide->io.data_addr));
}
}
@@ -152,10 +210,10 @@ static void ata_wr_sector(struct ide_port *ide, const void *buf)
if (ide->io.dataif_be) {
for (; u > 0; u--)
- writew(cpu_to_be16(*b++), ide->io.data_addr);
+ ata_wr_word(ide, cpu_to_be16(*b++), ide->io.data_addr);
} else {
for (; u > 0; u--)
- writew(cpu_to_le16(*b++), ide->io.data_addr);
+ ata_wr_word(ide, cpu_to_le16(*b++), ide->io.data_addr);
}
}
@@ -169,10 +227,10 @@ static int ide_read_id(struct ata_port *port, void *buf)
struct ide_port *ide = to_ata_drive_access(port);
int rc;
- writeb(0xA0, ide->io.device_addr); /* FIXME drive */
- writeb(0x00, ide->io.lbal_addr);
- writeb(0x00, ide->io.lbam_addr);
- writeb(0x00, ide->io.lbah_addr);
+ ata_wr_byte(ide, 0xA0, ide->io.device_addr); /* FIXME drive */
+ ata_wr_byte(ide, 0x00, ide->io.lbal_addr);
+ ata_wr_byte(ide, 0x00, ide->io.lbam_addr);
+ ata_wr_byte(ide, 0x00, ide->io.lbah_addr);
rc = ata_wr_cmd(ide, ATA_CMD_ID_ATA);
if (rc != 0)
@@ -327,6 +385,8 @@ int ide_port_register(struct ide_port *ide)
ide->port.ops = &ide_ops;
ret = ata_port_register(&ide->port);
+ if ( !ret )
+ ata_port_detect(&ide->port);
if (ret)
free(ide);
diff --git a/drivers/ata/intf_platform_ide.c b/drivers/ata/intf_platform_ide.c
index 8ae0f05..24f78f4 100644
--- a/drivers/ata/intf_platform_ide.c
+++ b/drivers/ata/intf_platform_ide.c
@@ -89,8 +89,18 @@ static int platform_ide_probe(struct device_d *dev)
}
ide = xzalloc(sizeof(*ide));
- reg_base = dev_request_mem_region(dev, 0);
- alt_base = dev_request_mem_region(dev, 1);
+ reg_base = dev_request_region(dev, 0, IORESOURCE_MEM);
+ if (reg_base != NULL)
+ {
+ alt_base = dev_request_region(dev, 1, IORESOURCE_MEM);
+ ide->io.mmio = 1;
+ }
+ else
+ {
+ reg_base = dev_request_region(dev, 0, IORESOURCE_IO);
+ alt_base = dev_request_region(dev, 1, IORESOURCE_IO);
+ }
+
platform_ide_setup_port(reg_base, alt_base, &ide->io, pdata->ioport_shift);
ide->io.reset = pdata->reset;
ide->io.dataif_be = pdata->dataif_be;
diff --git a/include/ata_drive.h b/include/ata_drive.h
index 6d6cca4..44073cb 100644
--- a/include/ata_drive.h
+++ b/include/ata_drive.h
@@ -119,6 +119,7 @@ struct ata_ioports {
/* hard reset line handling */
void (*reset)(int); /* true: assert reset, false: de-assert reset */
int dataif_be; /* true if 16 bit data register is big endian */
+ int mmio; /* true if memory-mapped io */
};
struct ata_port;
--
1.8.4
[-- Attachment #4: 0003-x86-Most-systems-support-a-second-serial-port.-Add-i.patch --]
[-- Type: application/octet-stream, Size: 781 bytes --]
From 3784340f1e85e16595df3ab414a995165f67edf8 Mon Sep 17 00:00:00 2001
From: Michel Stam <m.stam@fugro.nl>
Date: Mon, 10 Mar 2014 10:26:28 +0100
Subject: [PATCH 3/3] x86: Most systems support a second serial port. Add it.
---
arch/x86/boards/x86_generic/generic_pc.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/x86/boards/x86_generic/generic_pc.c b/arch/x86/boards/x86_generic/generic_pc.c
index 6152afc..4031a7e 100644
--- a/arch/x86/boards/x86_generic/generic_pc.c
+++ b/arch/x86/boards/x86_generic/generic_pc.c
@@ -150,6 +150,7 @@ static int pc_console_init(void)
/* Register the serial port */
add_ns16550_device(DEVICE_ID_DYNAMIC, 0x3f8, 8, 0, &serial_plat);
+ add_ns16550_device(DEVICE_ID_DYNAMIC, 0x2f8, 8, 0, &serial_plat);
return 0;
}
--
1.8.4
[-- Attachment #5: Type: text/plain, Size: 149 bytes --]
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Barebox x86 IDE support
2014-03-25 8:18 ` Sascha Hauer
@ 2014-03-25 11:04 ` Michel Stam
0 siblings, 0 replies; 4+ messages in thread
From: Michel Stam @ 2014-03-25 11:04 UTC (permalink / raw)
To: Sascha Hauer; +Cc: barebox
[-- Attachment #1.1: Type: text/plain, Size: 2892 bytes --]
Hello Sascha,
I will repost the patches separately as you requested.
With regard to your comment below, this was somewhat intentional, as I
understood:
> struct resource iomem_resource = {
> .start = 0,
> .end = 0xffffffff,
> .name = "iomem",
> .children = LIST_HEAD_INIT(iomem_resource.children),
> };
as "I/O and memory resource". This is also why I check on the resource
type in request_region( ):
> /*
> * We keep the list of child resources ordered which helps
> * us searching for conflicts here.
> */
> list_for_each_entry(r, &parent->children, sibling) {
> if (end < r->start)
> goto ok;
> if (start > r->end)
> continue;
>
> if (type != resource_type(r->parent))
> continue;
I retain one resource tree that way, also for if people decide to add
DMA or IRQ's in the future. The iteration function skips the resource if
the types do not match. To me the discussion seems the difference
between one linked list for every resource type (== more work if a new
type is added), or a single linked list with a type field (new resource
type == new integer constant, no more).
I would like to hear your thoughts on that.
to be continued after the posting.
Cheers,
Michel
On 03/25/2014 09:18 AM, Sascha Hauer wrote:
> Hi Michel,
>
> On Mon, Mar 24, 2014 at 10:37:48AM +0100, Stam, Michel [FINT] wrote:
>> Hello maintainers,
>>
>> I was wondering if one of you has had time to verify these patches and apply them to trunk?
> Could you send the patches as a series so that it's easier to comment on
> the patches on the list?
>
> There are some things to comment on, I think the most important one is
> this:
>
>> +/*
>> + * request an io region inside the io space
>> + */
>> +struct resource *request_io_region(const char *name,
>> + resource_size_t start, resource_size_t end,int type)
>> +{
>> + return request_region(&iomem_resource, name, start, end, type);
>> +}
>>
> You request here from the iomem resource, but ioports are a completely
> separate resource, so you have to use a new toplevel resource like
> this:
>
> /* The root resource for the whole io space */
> struct resource io_resource = {
> .start = 0,
> .end = 0xffffffff,
> .name = "ioport",
> .children = LIST_HEAD_INIT(io_resource.children),
> };
>
> /*
> * request an io region inside the io space
> */
> struct resource *request_io_region(const char *name,
> resource_size_t start, resource_size_t end)
> {
> return request_region(&io_resource, name, start, end, IORESOURCE_IO);
> }
>
> The 'type' argument to request_io_region is unnecessary since the
> function name already implies it, right?
>
> Sascha
>
[-- Attachment #1.2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4278 bytes --]
[-- Attachment #2: Type: text/plain, Size: 149 bytes --]
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Barebox x86 IDE support
2014-03-24 9:37 Stam, Michel [FINT]
@ 2014-03-25 8:18 ` Sascha Hauer
2014-03-25 11:04 ` Michel Stam
0 siblings, 1 reply; 4+ messages in thread
From: Sascha Hauer @ 2014-03-25 8:18 UTC (permalink / raw)
To: Stam, Michel [FINT]; +Cc: barebox
Hi Michel,
On Mon, Mar 24, 2014 at 10:37:48AM +0100, Stam, Michel [FINT] wrote:
> Hello maintainers,
>
> I was wondering if one of you has had time to verify these patches and apply them to trunk?
Could you send the patches as a series so that it's easier to comment on
the patches on the list?
There are some things to comment on, I think the most important one is
this:
> +/*
> + * request an io region inside the io space
> + */
> +struct resource *request_io_region(const char *name,
> + resource_size_t start, resource_size_t end,int type)
> +{
> + return request_region(&iomem_resource, name, start, end, type);
> +}
>
You request here from the iomem resource, but ioports are a completely
separate resource, so you have to use a new toplevel resource like
this:
/* The root resource for the whole io space */
struct resource io_resource = {
.start = 0,
.end = 0xffffffff,
.name = "ioport",
.children = LIST_HEAD_INIT(io_resource.children),
};
/*
* request an io region inside the io space
*/
struct resource *request_io_region(const char *name,
resource_size_t start, resource_size_t end)
{
return request_region(&io_resource, name, start, end, IORESOURCE_IO);
}
The 'type' argument to request_io_region is unnecessary since the
function name already implies it, right?
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: Barebox x86 IDE support
@ 2014-03-24 9:37 Stam, Michel [FINT]
2014-03-25 8:18 ` Sascha Hauer
0 siblings, 1 reply; 4+ messages in thread
From: Stam, Michel [FINT] @ 2014-03-24 9:37 UTC (permalink / raw)
To: barebox
Hello maintainers,
I was wondering if one of you has had time to verify these patches and apply them to trunk?
Kind regards,
Michel Stam
--
From: Stam, Michel [FINT]
Sent: Friday, March 14, 2014 17:31 PM
To: 'barebox@lists.infradead.org'
Subject: Barebox x86 IDE support
Dear list,
While working on barebox recently, I decided to port a couple of patches I made to an old barebox tree a few years ago, to fix the IDE ports on the x86 platform.
I had to work on the resource allocation a bit, as the current GIT repository does not allow IORESOURCE_IO type to be passed to dev_request_region( ) and friends. I tried to keep the interface the same as much as possible by using macros etc.
Please find that attached in 0001-common-driver-Allow-for-I-O-mapped-I-O-in-resource-r.patch.
Next, I added support to the platform IDE interface to use I/O-mapped I/O, and added the necessary allocation to the generic_pc board.
I did have to add a call to ata_port_detect( ) to ide_port_register to actually get the drives to be probed. Without it the interface seems to be stuck in some sort of limbo. This patch is in 0002-x86-Add-support-for-IDE-on-the-legacy-I-O-ports.patch.
Lastly, I added an extra serial port to the generic_pc board. Most (legacy) PCs have support for 2 serial ports, it seemed useful to me.
What does puzzle me is that the arch/x86/boot/Kconfig mentions X86_VESA and X86_VGA support, but the files (console_vga.c and console_vesa.c) do not nor have ever existed. Is this a remnant from U-boot v1?
Best regards,
Michel Stam
_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-03-25 11:04 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-14 17:01 Barebox x86 IDE support Stam, Michel [FINT]
2014-03-24 9:37 Stam, Michel [FINT]
2014-03-25 8:18 ` Sascha Hauer
2014-03-25 11:04 ` Michel Stam
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox