From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heinrich Schuchardt Date: Mon, 22 Feb 2021 19:59:33 +0100 Subject: [PATCH 2/2] Fix IDE commands issued, fix endian issues, fix non MMIO In-Reply-To: <20210222170512.22601-3-reinoud@NetBSD.org> References: <20210222170512.22601-1-reinoud@NetBSD.org> <20210222170512.22601-3-reinoud@NetBSD.org> Message-ID: <1cee3024-8270-7ac0-9940-5d68a0f9be73@gmx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 2/22/21 6:05 PM, Reinoud Zandijk wrote: A patch without commit message will not be accepted. Please, run scripts/checkpatch.pl on your patch: ERROR: Missing Signed-off-by: line(s) > --- > drivers/block/ide.c | 143 +++++++++++++------------------------------- > include/ata.h | 3 +- > 2 files changed, 42 insertions(+), 104 deletions(-) > > diff --git a/drivers/block/ide.c b/drivers/block/ide.c > index 43a0776099..fa3481e936 100644 > --- a/drivers/block/ide.c Your patch is mal-formed: ERROR: Avoid using diff content in the commit message - patch(1) might not work #126: --- a/drivers/block/ide.c > +++ b/drivers/block/ide.c > @@ -130,56 +130,40 @@ OUT: > * ATAPI Support > */ > > -#if defined(CONFIG_IDE_SWAP_IO) As far as possible replace #if by if. > /* since ATAPI may use commands with not 4 bytes alligned length > * we have our own transfer functions, 2 bytes alligned */ > __weak void ide_output_data_shorts(int dev, ushort *sect_buf, int shorts) > { > ushort *dbuf; > - volatile ushort *pbuf; > + u64 paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG); ERROR: DOS line endings #139: FILE: drivers/block/ide.c:138: +^Iu64 paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG);^M$ > > - pbuf = (ushort *)(ATA_CURR_BASE(dev) + ATA_DATA_REG); > dbuf = (ushort *)sect_buf; > > debug("in output data shorts base for read is %lx\n", > - (unsigned long) pbuf); > + (unsigned long) paddr); > > while (shorts--) { > EIEIO; > - *pbuf = *dbuf++; > + outw(paddr, cpu_to_le16(*dbuf++)); > } > } > > __weak void ide_input_data_shorts(int dev, ushort *sect_buf, int shorts) > { > ushort *dbuf; > - volatile ushort *pbuf; > + u64 paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG); > > - pbuf = (ushort *)(ATA_CURR_BASE(dev) + ATA_DATA_REG); > dbuf = (ushort *)sect_buf; > > debug("in input data shorts base for read is %lx\n", > - (unsigned long) pbuf); > + (unsigned long) paddr); > > while (shorts--) { > EIEIO; > - *dbuf++ = *pbuf; > + *dbuf++ = le16_to_cpu(inw(paddr)); > } > } > > -#else /* ! CONFIG_IDE_SWAP_IO */ > -__weak void ide_output_data_shorts(int dev, ushort *sect_buf, int shorts) > -{ > - outsw(ATA_CURR_BASE(dev) + ATA_DATA_REG, sect_buf, shorts); > -} > - > -__weak void ide_input_data_shorts(int dev, ushort *sect_buf, int shorts) > -{ > - insw(ATA_CURR_BASE(dev) + ATA_DATA_REG, sect_buf, shorts); > -} > - > -#endif /* CONFIG_IDE_SWAP_IO */ > - > /* > * Wait until (Status & mask) == res, or timeout (in ms) > * Return last status > @@ -636,19 +620,6 @@ static void ide_ident(struct blk_desc *dev_desc) > sizeof(dev_desc->vendor)); > ident_cpy((unsigned char *)dev_desc->product, iop.serial_no, > sizeof(dev_desc->product)); > -#ifdef __LITTLE_ENDIAN > - /* > - * firmware revision, model, and serial number have Big Endian Byte > - * order in Word. Convert all three to little endian. > - * > - * See CF+ and CompactFlash Specification Revision 2.0: > - * 6.2.1.6: Identify Drive, Table 39 for more details > - */ > - > - strswab(dev_desc->revision); > - strswab(dev_desc->vendor); > - strswab(dev_desc->product); > -#endif /* __LITTLE_ENDIAN */ > > if ((iop.config & 0x0080) == 0x0080) > dev_desc->removable = 1; > @@ -662,23 +633,19 @@ static void ide_ident(struct blk_desc *dev_desc) > } > #endif /* CONFIG_ATAPI */ > > -#ifdef __BIG_ENDIAN > - /* swap shorts */ > - dev_desc->lba = (iop.lba_capacity << 16) | (iop.lba_capacity >> 16); > -#else /* ! __BIG_ENDIAN */ > - /* > - * do not swap shorts on little endian > - * > - * See CF+ and CompactFlash Specification Revision 2.0: > - * 6.2.1.6: Identfy Drive, Table 39, Word Address 57-58 for details. > - */ > - dev_desc->lba = iop.lba_capacity; > -#endif /* __BIG_ENDIAN */ > + iop.lba_capacity[0] = be16_to_cpu(iop.lba_capacity[0]); > + iop.lba_capacity[1] = be16_to_cpu(iop.lba_capacity[1]); > + dev_desc->lba = > + ((unsigned long) iop.lba_capacity[0]) | > + ((unsigned long) iop.lba_capacity[1] << 16); > > #ifdef CONFIG_LBA48 > if (iop.command_set_2 & 0x0400) { /* LBA 48 support */ > dev_desc->lba48 = 1; > - dev_desc->lba = (unsigned long long) iop.lba48_capacity[0] | > + for (int i = 0; i < 4; i++) > + iop.lba48_capacity[i] = be16_to_cpu(iop.lba48_capacity[i]); > + dev_desc->lba = > + ((unsigned long long) iop.lba48_capacity[0] | > ((unsigned long long) iop.lba48_capacity[1] << 16) | > ((unsigned long long) iop.lba48_capacity[2] << 32) | > ((unsigned long long) iop.lba48_capacity[3] << 48); > @@ -846,90 +813,60 @@ void ide_init(void) > #endif > } > > -/* We only need to swap data if we are running on a big endian cpu. */ > -#if defined(__LITTLE_ENDIAN) > __weak void ide_input_swap_data(int dev, ulong *sect_buf, int words) > { > - ide_input_data(dev, sect_buf, words); > -} > -#else > -__weak void ide_input_swap_data(int dev, ulong *sect_buf, int words) > -{ > - volatile ushort *pbuf = > - (ushort *)(ATA_CURR_BASE(dev) + ATA_DATA_REG); > + u64 paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG); > ushort *dbuf = (ushort *)sect_buf; > > debug("in input swap data base for read is %lx\n", > - (unsigned long) pbuf); > + (unsigned long) paddr); > > while (words--) { > -#ifdef __MIPS__ > - *dbuf++ = swab16p((u16 *)pbuf); > - *dbuf++ = swab16p((u16 *)pbuf); > -#else > - *dbuf++ = ld_le16(pbuf); > - *dbuf++ = ld_le16(pbuf); > -#endif /* !MIPS */ > + EIEIO; > + *dbuf++ = be16_to_cpu(inw(paddr)); > + EIEIO; > + *dbuf++ = be16_to_cpu(inw(paddr)); > } > } > -#endif /* __LITTLE_ENDIAN */ > - > > -#if defined(CONFIG_IDE_SWAP_IO) > __weak void ide_output_data(int dev, const ulong *sect_buf, int words) > { > +#if defined(CONFIG_IDE_AHB) > + ide_write_data(dev, sect_buf, words); > +#else > ushort *dbuf; > - volatile ushort *pbuf; > + u64 paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG); > > - pbuf = (ushort *)(ATA_CURR_BASE(dev) + ATA_DATA_REG); > dbuf = (ushort *)sect_buf; > while (words--) { > EIEIO; > - *pbuf = *dbuf++; > + outw(paddr, cpu_to_le16(*dbuf++)); > EIEIO; > - *pbuf = *dbuf++; > + outw(paddr, cpu_to_le16(*dbuf++)); > } > +#endif /* CONFIG_IDE_AHB */ > } > -#else /* ! CONFIG_IDE_SWAP_IO */ > -__weak void ide_output_data(int dev, const ulong *sect_buf, int words) > -{ > -#if defined(CONFIG_IDE_AHB) > - ide_write_data(dev, sect_buf, words); > -#else > - outsw(ATA_CURR_BASE(dev) + ATA_DATA_REG, sect_buf, words << 1); > -#endif > -} > -#endif /* CONFIG_IDE_SWAP_IO */ > > -#if defined(CONFIG_IDE_SWAP_IO) > __weak void ide_input_data(int dev, ulong *sect_buf, int words) > { > +#if defined(CONFIG_IDE_AHB) > + ide_read_data(dev, sect_buf, words); > +#else > ushort *dbuf; > - volatile ushort *pbuf; > + u64 paddr = (ATA_CURR_BASE(dev) + ATA_DATA_REG); > > - pbuf = (ushort *)(ATA_CURR_BASE(dev) + ATA_DATA_REG); > dbuf = (ushort *)sect_buf; > > - debug("in input data base for read is %lx\n", (unsigned long) pbuf); > + debug("in input data base for read is %lx\n", (unsigned long) paddr); > > while (words--) { > EIEIO; > - *dbuf++ = *pbuf; > + *dbuf++ = le16_to_cpu(inw(paddr)); > EIEIO; > - *dbuf++ = *pbuf; > + *dbuf++ = le16_to_cpu(inw(paddr)); > } > +#endif /* CONFIG_IDE_AHB */ > } > -#else /* ! CONFIG_IDE_SWAP_IO */ > -__weak void ide_input_data(int dev, ulong *sect_buf, int words) > -{ > -#if defined(CONFIG_IDE_AHB) > - ide_read_data(dev, sect_buf, words); > -#else > - insw(ATA_CURR_BASE(dev) + ATA_DATA_REG, sect_buf, words << 1); > -#endif > -} > - > -#endif /* CONFIG_IDE_SWAP_IO */ > > #ifdef CONFIG_BLK > ulong ide_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt, > @@ -1019,14 +956,14 @@ ulong ide_read(struct blk_desc *block_dev, lbaint_t blknr, lbaint_t blkcnt, > if (lba48) { > ide_outb(device, ATA_DEV_HD, > ATA_LBA | ATA_DEVICE(device)); > - ide_outb(device, ATA_COMMAND, ATA_CMD_READ_EXT); > + ide_outb(device, ATA_COMMAND, ATA_CMD_PIO_READ_EXT); > > } else > #endif > { > ide_outb(device, ATA_DEV_HD, ATA_LBA | > ATA_DEVICE(device) | ((blknr >> 24) & 0xF)); > - ide_outb(device, ATA_COMMAND, ATA_CMD_READ); > + ide_outb(device, ATA_COMMAND, ATA_CMD_PIO_READ); > } > > udelay(50); > @@ -1116,14 +1053,14 @@ ulong ide_write(struct blk_desc *block_dev, lbaint_t blknr, lbaint_t blkcnt, > if (lba48) { > ide_outb(device, ATA_DEV_HD, > ATA_LBA | ATA_DEVICE(device)); > - ide_outb(device, ATA_COMMAND, ATA_CMD_WRITE_EXT); > + ide_outb(device, ATA_COMMAND, ATA_CMD_PIO_WRITE_EXT); > > } else > #endif > { > ide_outb(device, ATA_DEV_HD, ATA_LBA | > ATA_DEVICE(device) | ((blknr >> 24) & 0xF)); > - ide_outb(device, ATA_COMMAND, ATA_CMD_WRITE); > + ide_outb(device, ATA_COMMAND, ATA_CMD_PIO_WRITE); > } > > udelay(50); > diff --git a/include/ata.h b/include/ata.h > index 3d870c973f..7c95cfdced 100644 > --- a/include/ata.h > +++ b/include/ata.h > @@ -134,7 +134,8 @@ typedef struct hd_driveid { > unsigned short cur_capacity1; /* (2 words, misaligned int) */ > unsigned char multsect; /* current multiple sector count */ > unsigned char multsect_valid; /* when (bit0==1) multsect is ok */ > - unsigned int lba_capacity; /* total number of sectors */ > + /*unsigned int lba_capacity; /--* total number of sectors */ Please, remove replaced code completely. > + unsigned short lba_capacity[2]; /* 2 16bit values containing lba total number of sectors */ WARNING: line length of 101 exceeds 100 columns #400: FILE: include/ata.h:138: + unsigned short lba_capacity[2]; /* 2 16bit values containing lba total number of sectors */ Best regards Heinrich > unsigned short dma_1word; /* single-word dma info */ > unsigned short dma_mword; /* multiple-word dma info */ > unsigned short eide_pio_modes; /* bits 0:mode3 1:mode4 */ >