[U-Boot] [PATCH] Canyonlands SATA harddisk driver

[Stefan Roese]

On Wednesday 13 May 2009 10:56:00 Kazuaki Ichinohe wrote:
> This patch adds a SATA harddisk driver for the canyonlands.
> This patch is kernel driver's porting.
> This pach corresponded to not cmd_scsi but cmd_sata.
>
> [environment variable, boot script]
> setenv bootargs root=/dev/sda7 rw
> setenv bootargs ${bootargs} console=ttyS0,115200
> ext2load sata 0:2 0x400000 /canyonlands/uImage
> ext2load sata 0:2 0x800000 /canyonlands/canyonlands.dtb
> fdt addr 0x800000 0x4000
> bootm 0x400000 - 0x800000
>
> If you drive SATA-2 disk on Canyonlands, you must change parts from
> PI2PCIE212 to PI2PCIE2212 on U25. We confirmed to boot by using following
> disk.
>
> 1.Vender: Fujitsu Type: MHW2040BS
> 2.Vender: Fujitsu Type: MHW2060BK
> 3.Vendor: HAGIWARA SYS-COM:HFD25S-032GT
> 4.Vender: WesternDigital Type: WD3200BJKT (CONFIG_LBA48 required)
> 5.Vender: WesternDigital Type: WD3200BEVT (CONFIG_LBA48 required)
> 6.Vender: hitachi Type: HTS543232L9A300 (CONFIG_LBA48 required)
> 7.Vender: Seagate Type: ST31000333AS (CONFIG_LBA48 required)
> 8.Vender: Transcend Type: TS32GSSD25S-M
> 9.Vender: MTRON Type: MSD-SATA1525-016
>
> Signed-off-by: Kazuaki Ichinohe <kazuichi at fsi.co.jp>

This finally looks good. :)

Some comments below.

> +++ u-boot-sata/drivers/block/sata_dwc.c	2009-05-07 09:37:53.000000000
> +0900 @@ -0,0 +1,2110 @@
> +/*
> + * sata_dwc.c

<snip>

> +	if ((nsect == 0x55) && (lbal == 0xaa)) {
> +		printf("we found a device\n");

		printf("Device found\n");

> +	} else {
> +		printf("Not found a device.\n");

		printf("No device found\n");

> +		dev_state = SATA_NODEVICE;
> +		return FALSE;
> +	}
> +
> +	tmp = ATA_DEVICE_OBS;
> +	writeb(tmp,ioaddr->device_addr);

Add a space after the ",".

> +	writeb(ap->ctl,ioaddr->ctl_addr);

Here again...

> +
> +	udelay(200);
> +
> +	writeb(ap->ctl | ATA_SRST, ioaddr->ctl_addr);
> +
> +	udelay(200);
> +	writeb(ap->ctl,ioaddr->ctl_addr);

And here again. I won't mention this again. Please check the whole source for 
such missing spaces.

<snip>

> +int scan_sata (int dev)

Please remove the space before "(". Use a consistent coding style in this 
file. So check for other occurrences too.

> +{
> +	int i;
> +	int rc;
> +	u8 status;
> +	const u16 *id;
> +	struct ata_device *ata_dev = &ata_device;
> +	unsigned long pio_mask, mwdma_mask, udma_mask;
> +	unsigned long xfer_mask;
> +	char revbuf[7];
> +	u16 iobuf[ATA_SECTOR_WORDS];
> +
> +	memset ( iobuf, 0, sizeof(iobuf));

	memset(iobuf, 0, sizeof(iobuf));

> +
> +	if (dev_state == SATA_NODEVICE)
> +		return 1;
> +
> +	printf("waitng for device ready.");

	printf("Waiting for device...");

There are two places where "waiting" is misspelled. Please fix both.

Thanks.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================