[PATCH 2/3] x86: Add support for IDE on the legacy I/O ports

[PATCH 2/3] x86: Add support for IDE on the legacy I/O ports

[Michel Stam]

---
  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


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/3] x86: Add support for IDE on the legacy I/O ports

[Jean-Christophe PLAGNIOL-VILLARD]

On 12:12 Tue 25 Mar     , Michel Stam wrote:
> 
> ---
>  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
why the ifdef?
> +#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,
> +	w	.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 ),
> +};
please create the device on the fly
> +
> +#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
if (IS_ENABLED(CONFxx))
>  	add_generic_device("biosdrive", DEVICE_ID_DYNAMIC, NULL, 0, 0,
> IORESOURCE_MEM,
>  			NULL);
> +#endif
> +
> +#ifdef CONFIG_DISK_INTF_PLATFORM_IDE
ditto and no space after ( and before )
> +	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);
please check your coding style
> +	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 */
so use a boolean
>  };
>   struct ata_port;
> -- 
> 1.8.4
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/3] x86: Add support for IDE on the legacy I/O ports

[Alexander Aring]

On Tue, Mar 25, 2014 at 12:12:00PM +0100, Michel Stam wrote:
> ---
>  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

Here is also a diff inconsitent... there is a space at beginning of the
line?? Did you use git send-email? Check [0] for a introduction.

- Alex

[0] https://home.regit.org/technical-articles/git-for-the-newbie/

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/3] x86: Add support for IDE on the legacy I/O ports

[Michel Stam]

[-- Attachment #1.1: Type: text/plain, Size: 13333 bytes --]

Hello Jean,

Forget about the ifdefs; my patches there were meant to let the code compile depending on what was selected in configuration, (this was not the case previously), but the whole thing became an ifdef mess in the process. I've completely rewritten this, splitting initialisation over multiple files which are selected from the Makefile.

>> 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 */
> so use a boolean
look at the line immediately preceeding the mmio line: I'm following the style of the original driver. I was not intending to do a code cleanup of the driver, the goal is to get it to work on the x86.

>> 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
> why the ifdef?
i discussed that in an email last night; it is a convention I use to prevent badly behaving header files from breaking compiles.

>> secondary_ide_device = {
>> +    .name = "ide_intf",
>> +    .id = 1,
>> +    .platform_data = &ide_plat,
>> +    .resource = secondary_ide_resources,
>> +    .num_resources = ARRAY_SIZE( secondary_ide_resources ),
>> +};
> please create the device on the fly


Again I'm following the style of the original driver; the serial ports in the generic_pc.c behave identically. 

Seems to me that these files were due for an overhaul, given the number of comments I get by following the style of the original driver...

Ive found the perl script that checks patches, next version of the patches will have spacing etc fixed as well, i want to test that everything works first.

Michel Stam


> On 5 apr. 2014, at 05:11, Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> wrote:
> 
>> On 12:12 Tue 25 Mar     , Michel Stam wrote:
>> 
>> ---
>> 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
> why the ifdef?
>> +#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,
>> +    w    .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 ),
>> +};
> please create the device on the fly
>> +
>> +#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
> if (IS_ENABLED(CONFxx))
>>    add_generic_device("biosdrive", DEVICE_ID_DYNAMIC, NULL, 0, 0,
>> IORESOURCE_MEM,
>>            NULL);
>> +#endif
>> +
>> +#ifdef CONFIG_DISK_INTF_PLATFORM_IDE
> ditto and no space after ( and before )
>> +    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);
> please check your coding style
>> +    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 */
> so use a boolean
>> };
>>  struct ata_port;
>> -- 
>> 1.8.4
>> 
>> 
>> _______________________________________________
>> barebox mailing list
>> barebox@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/barebox

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 6165 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/3] x86: Add support for IDE on the legacy I/O ports

[Michel Stam]

[-- Attachment #1.1: Type: text/plain, Size: 1296 bytes --]

Hello Alex,

Yes, I used send-email. I did notice that my editor changed some tabs to spaces, that is already fixed in my local repository. Ill send it after I tested whether everything still works.

Michel Stam


> On 5 apr. 2014, at 07:01, Alexander Aring <alex.aring@gmail.com> wrote:
> 
>> On Tue, Mar 25, 2014 at 12:12:00PM +0100, Michel Stam wrote:
>> ---
>> 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
> 
> Here is also a diff inconsitent... there is a space at beginning of the
> line?? Did you use git send-email? Check [0] for a introduction.
> 
> - Alex
> 
> [0] https://home.regit.org/technical-articles/git-for-the-newbie/

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 6165 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/3] x86: Add support for IDE on the legacy I/O ports

[Alexander Aring]

Hi Michel,

On Sat, Apr 05, 2014 at 08:39:32AM +0200, Michel Stam wrote:
> Hello Alex,
> 
> Yes, I used send-email. I did notice that my editor changed some tabs to spaces, that is already fixed in my local repository. Ill send it after I tested whether everything still works.
> 
you should not do any changes to a patch file except maybe for the
subject to add "v2" or after the "---" lines. I don't saw any changes to
that. <stupid_question> Why you did changes with an editor?</stupid_question>

- Alex

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/3] x86: Add support for IDE on the legacy I/O ports

[Michel Stam]

[-- Attachment #1.1: Type: text/plain, Size: 1079 bytes --]

Hello Alex,

I think we misunderstood eachother; I was talking about space changes in .c files caused by an editor. send-email came after the editing. 

Reading back I think you were talking about directly editing patches, which I did not do. 

The patches as they were sent to this list are the immediate result/output of the git send-email command, if theres inconsistencies in the patch file itself I have no explanation for that.

Michel Stam


> On 5 apr. 2014, at 08:45, Alexander Aring <alex.aring@gmail.com> wrote:
> 
> Hi Michel,
> 
>> On Sat, Apr 05, 2014 at 08:39:32AM +0200, Michel Stam wrote:
>> Hello Alex,
>> 
>> Yes, I used send-email. I did notice that my editor changed some tabs to spaces, that is already fixed in my local repository. Ill send it after I tested whether everything still works.
>> 
> you should not do any changes to a patch file except maybe for the
> subject to add "v2" or after the "---" lines. I don't saw any changes to
> that. <stupid_question> Why you did changes with an editor?</stupid_question>
> 
> - Alex

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 6165 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/3] x86: Add support for IDE on the legacy I/O ports

[Alexander Aring]

On Sat, Apr 05, 2014 at 08:56:25AM +0200, Michel Stam wrote:
> Hello Alex,
> 
> I think we misunderstood eachother; I was talking about space changes in .c files caused by an editor. send-email came after the editing. 
> 
> Reading back I think you were talking about directly editing patches, which I did not do. 
> 
> The patches as they were sent to this list are the immediate result/output of the git send-email command, if theres inconsistencies in the patch file itself I have no explanation for that.

ok, but then I don't unerstand why it is " +#..." instead "+ #..." the +
comming from diff and should always at the beginnen of line.

I think we should stop this discussion now. Nevertheless I saw there is
already a v2. :-)

- Alex

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/3] x86: Add support for IDE on the legacy I/O ports

[Michel Stam]

[-- Attachment #1.1: Type: text/plain, Size: 940 bytes --]

Hey Alex,

Therell be a v3 soon as well...

Cheers,

Michel Stam


> On 5 apr. 2014, at 08:58, Alexander Aring <alex.aring@gmail.com> wrote:
> 
>> On Sat, Apr 05, 2014 at 08:56:25AM +0200, Michel Stam wrote:
>> Hello Alex,
>> 
>> I think we misunderstood eachother; I was talking about space changes in .c files caused by an editor. send-email came after the editing. 
>> 
>> Reading back I think you were talking about directly editing patches, which I did not do. 
>> 
>> The patches as they were sent to this list are the immediate result/output of the git send-email command, if theres inconsistencies in the patch file itself I have no explanation for that.
> 
> ok, but then I don't unerstand why it is " +#..." instead "+ #..." the +
> comming from diff and should always at the beginnen of line.
> 
> I think we should stop this discussion now. Nevertheless I saw there is
> already a v2. :-)
> 
> - Alex

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 6165 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/3] x86: Add support for IDE on the legacy I/O ports

[Alexander Aring]

On Sat, Apr 05, 2014 at 09:12:15AM +0200, Michel Stam wrote:
> Hey Alex,
> 
> Therell be a v3 soon as well...
> 

I don't see that, I also don't see v3 on the mailinglist archives [0].

- Alex

[0] http://lists.infradead.org/pipermail/barebox/

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/3] x86: Add support for IDE on the legacy I/O ports

[Michel Stam]

[-- Attachment #1.1: Type: text/plain, Size: 487 bytes --]

:) the keyword is soon. I want to test that I didnt break anything first, but theyre in my local repository.

cheers

Michel Stam


> On 5 apr. 2014, at 09:18, Alexander Aring <alex.aring@gmail.com> wrote:
> 
>> On Sat, Apr 05, 2014 at 09:12:15AM +0200, Michel Stam wrote:
>> Hey Alex,
>> 
>> Therell be a v3 soon as well...
> 
> I don't see that, I also don't see v3 on the mailinglist archives [0].
> 
> - Alex
> 
> [0] http://lists.infradead.org/pipermail/barebox/

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 6165 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/3] x86: Add support for IDE on the legacy I/O ports

[Alexander Aring]

On Sat, Apr 05, 2014 at 09:20:10AM +0200, Michel Stam wrote:
> :) the keyword is soon. I want to test that I didnt break anything first, but theyre in my local repository.

ah, sorry. Ok sounds good.

- Alex

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/3] x86: Add support for IDE on the legacy I/O ports

[michel]

From: Michel Stam <m.stam@fugro.nl>

---
 arch/x86/boards/x86_generic/generic_pc.c |   73 +++++++++++++++++++++++
 drivers/ata/ide-sff.c                    |   94 ++++++++++++++++++++++++-----
 drivers/ata/intf_platform_ide.c          |   33 +++++++++-
 include/ata_drive.h                      |    1 +
 4 files changed, 180 insertions(+), 21 deletions(-)

diff --git a/arch/x86/boards/x86_generic/generic_pc.c b/arch/x86/boards/x86_generic/generic_pc.c
index 74a7224..895b88d 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,17 +45,85 @@ 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 =  0x1f7,
+		.flags = IORESOURCE_IO
+	},
+	{
+		.name = "alt",
+		.start = 0x3f6,
+		.end =  0x3f7,
+		.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)
 {
+#ifdef CONFIG_DISK_INTF_PLATFORM_IDE
 	struct cdev *cdev;
+#endif
 
 	/* 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,
 				(unsigned)pers_env_size * 512,
@@ -59,6 +131,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..f3a5cbd 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 readb(addr);
+	else
+		return (uint8_t) inb((int) 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)
+		writeb(value, addr);
+	else
+		outb(value, (int) 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 readw(addr);
+	else
+		return (uint16_t) inw((int) 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)
+		writew(value, addr);
+	else
+		outw(value, (int) 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..5ad4c3d 100644
--- a/drivers/ata/intf_platform_ide.c
+++ b/drivers/ata/intf_platform_ide.c
@@ -81,16 +81,43 @@ static int platform_ide_probe(struct device_d *dev)
 	int rc;
 	struct ide_port_info *pdata = dev->platform_data;
 	struct ide_port *ide;
+	struct resource *res;
 	void *reg_base, *alt_base;
+	int mmio;
 
 	if (pdata == NULL) {
 		dev_err(dev, "No platform data. Cannot continue\n");
 		return -EINVAL;
 	}
 
+	reg_base = NULL;
+	alt_base = NULL;
+	res = dev_get_resource(dev, IORESOURCE_MEM, 0);
+	mmio = (res != NULL);
+	if (res && (res = request_iomem_region(dev_name(dev),res->start,res->end)))
+	{
+		reg_base = (void __force __iomem*) res->start;
+		res = dev_get_resource(dev, IORESOURCE_MEM, 1);
+		if (res && (res = request_iomem_region(dev_name(dev),res->start,res->end)))
+			alt_base = (void __force __iomem*) res->start;
+	}
+	else
+	{
+		res = dev_get_resource(dev, IORESOURCE_IO, 0);
+		if (res && (res = request_ioport_region(dev_name(dev),res->start,res->end)))
+		{
+			reg_base = (void __force *) res->start;
+			res = dev_get_resource(dev, IORESOURCE_IO, 1);
+			if (res && (res = request_ioport_region(dev_name(dev),res->start,res->end)))
+				alt_base = (void __force *) res->start;
+		}
+        }
+        if (!reg_base)
+		return -ENODEV;
+
 	ide = xzalloc(sizeof(*ide));
-	reg_base = dev_request_mem_region(dev, 0);
-	alt_base = dev_request_mem_region(dev, 1);
+	ide->io.mmio = mmio;
+
 	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;
@@ -125,6 +152,4 @@ device_platform_driver(platform_ide_driver);
  *
  * This driver does not change any access timings due to the fact it has no idea
  * how to do so. So, do not expect an impressive data throughput.
- *
- * @todo Support also the IO port access method, the x86 architecture is using
  */
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.7.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox