Re: [U-Boot] [PATCH] Canyonlands SATA harddisk driver
Dear Kazuaki Ichinohe,
In message 4a321b74.8040...@fsi.co.jp you wrote:
This patch adds a SATA harddisk driver for the canyonlands.
This patch is kernel driver's porting.
This patch corresponded to not cmd_scsi but cmd_sata.
This patch divided an unused member with ifndef __U_BOOT__ in the structure.
[environment variable, boot script]
setenv bootargs root=/dev/sda7 rw
setenv bootargs ${bootargs} console=ttyS0,115200
ext2load sata 0:2 0x40 /canyonlands/uImage
ext2load sata 0:2 0x80 /canyonlands/canyonlands.dtb
fdt addr 0x80 0x4000
bootm 0x40 - 0x80
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
Applied, thanks.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
War isn't a good life, but it's life.
-- Kirk, A Private Little War, stardate 4211.8
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Canyonlands SATA harddisk driver
Dear Kazuaki Ichinohe, In message 4a247247.70...@fsi.co.jp you wrote: ... But, other local, private, resource management structures are encouraged to be shrinked/optimized, as it's just waste of ROM space. Originally U-boot of PowerPC is not importance for the size because the size is large. Size *is* important. Please read the docs - it's pointed out again and again. It is important to maintain interchangeability with the Linux kernel driver. It is important that maintenance is good. Agreed. But we can easily disable / #define out unused parts of the code and data structures as Jean-Christophe suggested, without adding any significant maintenance problems. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Men will always be men -- no matter where they are. -- Harry Mudd, Mudd's Women, stardate 1329.8 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Canyonlands SATA harddisk driver
Kuribayashi-san, Do you think that you should include the Acked SATA driver in merge window of the next release (2009/08 or -09?)? I want to open the ACKed SATA driver to the public to the CanyonLands user. Regards, Kazuaki Ichinohe ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Canyonlands SATA harddisk driver
Hello Kuribayashi-san,
Please make sure I'm not talking about register definition structures.
They're harmless, and no need to be cleaned up.
The definition of the register of DMA is the following.
However, the structure is used a little for the usage in which DMA is disabled.
struct dmareg {
u32 low;
u32 high;
};
struct dma_chan_regs {
struct dmareg sar;
struct dmareg dar;
struct dmareg llp;
struct dmareg ctl;
struct dmareg sstat;
struct dmareg dstat;
struct dmareg sstatar;
struct dmareg dstatar;
struct dmareg cfg;
struct dmareg sgr;
struct dmareg dsr;
};
struct dma_interrupt_regs {
struct dmareg tfr;
struct dmareg block;
struct dmareg srctran;
struct dmareg dsttran;
struct dmareg error;
};
struct ahb_dma_regs {
struct dma_chan_regschan_regs[DMA_NUM_CHAN_REGS];
struct dma_interrupt_regs interrupt_raw;
struct dma_interrupt_regs interrupt_status;
struct dma_interrupt_regs interrupt_mask;
struct dma_interrupt_regs interrupt_clear;
struct dmareg statusInt;
struct dmareg rq_srcreg;
struct dmareg rq_dstreg;
struct dmareg rq_sgl_srcreg;
struct dmareg rq_sgl_dstreg;
struct dmareg rq_lst_srcreg;
struct dmareg rq_lst_dstreg;
struct dmareg dma_cfg;
struct dmareg dma_chan_en;
struct dmareg dma_id;
struct dmareg dma_test;
struct dmareg res1;
struct dmareg res2;
/* DMA Comp Params
* Param 6 = dma_param[0], Param 5 = dma_param[1],
* Param 4 = dma_param[2] ...
*/
struct dmareg dma_params[6];
};
But, other local, private, resource management structures are encouraged
to be shrinked/optimized, as it's just waste of ROM space.
Originally U-boot of PowerPC is not importance for the size because the size is
large.
I will point out and object about you.
It is important to maintain interchangeability with the Linux kernel driver.
It is important that maintenance is good.
Regards,
Kazuaki Ichinohe
Shinya Kuribayashi wrote:
Kazuaki Ichinohe wrote:
DMA function was scheduled to be developed as my schedule.
However, the development of the DMA function is discontinued once now.
The structure of the register that controls DMA has not been used any
longer.
I will e-mail the source code ( removed the struct of DMA register )
later.
Please make sure I'm not talking about register definition structures.
They're harmless, and no need to be cleaned up.
But, other local, private, resource management structures are encouraged
to be shrinked/optimized, as it's just waste of ROM space.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Canyonlands SATA harddisk driver
Kazuaki Ichinohe wrote: But, other local, private, resource management structures are encouraged to be shrinked/optimized, as it's just waste of ROM space. Originally U-boot of PowerPC is not importance for the size because the size is large. I will point out and object about you. As said before, I'm fine with this driver as is. I've just pointed out it has bunch of unused struct members in it. It is important to maintain interchangeability with the Linux kernel driver. It is important that maintenance is good. Hm, I don't see any good aspects with doing so, but that's also fine with me. -- Shinya Kuribayashi NEC Electronics ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Canyonlands SATA harddisk driver
On 18:07 Fri 29 May , Shinya Kuribayashi wrote: Kazuaki Ichinohe wrote: DMA function was scheduled to be developed as my schedule. However, the development of the DMA function is discontinued once now. The structure of the register that controls DMA has not been used any longer. I will e-mail the source code ( removed the struct of DMA register ) later. Please make sure I'm not talking about register definition structures. They're harmless, and no need to be cleaned up. But, other local, private, resource management structures are encouraged to be shrinked/optimized, as it's just waste of ROM space. if we remove it from the source, it will be harder to integrate upgrade on both side. As done for the nand and ubi add ifndef __U_BOOT__ will allow to reduce the size impact without the integration difficult Best Regards, J. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Canyonlands SATA harddisk driver
Hello, The SATA harddisk driver is based on the Linux kernel source code. Therefore, the software structure is the same structure as the Linux kernel driver. DMA function was scheduled to be developed as my schedule. However, the development of the DMA function is discontinued once now. The structure of the register that controls DMA has not been used any longer. I will e-mail the source code ( removed the struct of DMA register ) later. Regards, Kazuaki Ichinohe 2009/5/29 Wolfgang Denk w...@denx.de: Dear Kazuaki Ichinohe, In message 4a0d36aa.5000...@fsi.co.jp you 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. Stefan Roese acked your patch, but then Shinya Kuribayashi remarked: | I'm fine with this patch applied or not, but this driver is too big. | It has a lot of unused struct members, I'm not sure they'll be used | in the future, though. Can you please comment on this? Is this stuff indeed provisions for later extensions that you have in your queue, or is it dead code that can be removed? Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de I'd like to meet the man who invented sex and see what he's working on now. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Canyonlands SATA harddisk driver
On Friday 15 May 2009 11:32:26 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. Looks good now. Thanks for all your effort here. So: Acked-by: Stefan Roese s...@denx.de 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: off...@denx.de = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Canyonlands SATA harddisk driver
Stefan Roese wrote: On Friday 15 May 2009 11:32:26 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. Looks good now. Thanks for all your effort here. So: Acked-by: Stefan Roese s...@denx.de I'm fine with this patch applied or not, but this driver is too big. It has a lot of unused struct members, I'm not sure they'll be used in the future, though. Shinya ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Canyonlands SATA harddisk driver
Hello Denk, Stefan, I sent the patch source again on May 13. Could you review the patch source again ? Regards, Kazuaki Ichinohe ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Canyonlands SATA harddisk driver
On Thursday 14 May 2009 10:28:08 Kazuaki Ichinohe wrote: I sent the patch source again on May 13. Could you review the patch source again ? Yes, will do. 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: off...@denx.de = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Canyonlands SATA harddisk driver
Dear Kazuaki Ichinohe, In message 4a0947cd.6050...@fsi.co.jp you wrote: I re-send the patch source. The following three points were corrected. 1.--- was adjusted to add git history. 2.The following lines of /drivers/block/Makefile were matched to sorting. You really don't get it. All such comments like the lines above must go *BELOW* the --- line. Only the commit message, and nothing elase, is allowed above the ---. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Minds are like parachutes - they only function when open. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Canyonlands SATA harddisk driver
Hi Kazuaki Ichinohe,
first the good news. This patch version now applies clean. Thanks.
But please find still a few comments below.
On Friday 08 May 2009 08:27:27 Kazuaki Ichinohe wrote:
Hello Denk, Stefan,
The confirmed patch is sent again.
Please review this patch.
Move the lines above below the --- line below. This way those lines will not
be added to the git history.
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 0x40 /canyonlands/uImage
ext2load sata 0:2 0x80 /canyonlands/canyonlands.dtb
fdt addr 0x80 0x4000
bootm 0x40 - 0x80
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
---
[patch]
diff -uprN u-boot-0507/drivers/block/Makefile
u-boot-sata/drivers/block/Makefile ---
u-boot-0507/drivers/block/Makefile2009-05-07 09:25:48.0 +0900 +++
u-boot-sata/drivers/block/Makefile2009-05-07 09:43:56.0 +0900 @@
-35,6 +35,7 @@ COBJS-$(CONFIG_SATA_SIL3114) += sata_sil
COBJS-$(CONFIG_SCSI_AHCI) += ahci.o
COBJS-$(CONFIG_SCSI_SYM53C8XX) += sym53c8xx.o
COBJS-$(CONFIG_SYSTEMACE) += systemace.o
+COBJS-$(CONFIG_SATA_DWC) += sata_dwc.o
Please keep the list sorted.
snip
diff -uprN u-boot-0507/include/configs/canyonlands.h
u-boot-sata/include/configs/canyonlands.h ---
u-boot-0507/include/configs/canyonlands.h 2009-05-07 09:25:50.0
+0900 +++ u-boot-sata/include/configs/canyonlands.h 2009-05-07
09:49:10.0 +0900 @@ -454,6 +454,7 @@
#define CONFIG_CMD_SDRAM
#define CONFIG_CMD_SNTP
#define CONFIG_CMD_USB
+#define CONFIG_CMD_SATA
You enable SATA commands on Canyonlands here which is ok. But...
#elif defined(CONFIG_GLACIER)
#define CONFIG_CMD_DATE
#define CONFIG_CMD_DTT
@@ -517,6 +518,21 @@
#endif /* CONFIG_460GT */
/*---
+ * S-ATA driver setup
+ *--*/
+#define CONFIG_SATA_DWC
... you enable the DWC SATA driver for all 460 board variants here, including
Glacier and Arches.
+#ifdef CONFIG_SATA_DWC
+#define CONFIG_LIBATA
+
+#define SATA_BASE_ADDR 0xe20d1000 /* PPC460EX SATA Base
Address */
+#define SATA_DMA_REG_ADDR0xe20d0800 /* PPC460EX SATA Base Address */
+#define CONFIG_SYS_SATA_MAX_DEVICE 1 /* SATA MAX DEVICE */
+/* Convert sectorsize to wordsize */
+#define ATA_SECTOR_WORDS (ATA_SECT_SIZE/2)
#define ATA_SECTOR_WORDS(ATA_SECT_SIZE / 2)
+#endif
I suggest to do it this way:
/*
* SATA driver setup
*/
#ifdef CONFIG_CMD_SATA
#define CONFIG_SATA_DWC
#define CONFIG_LIBATA
#define SATA_BASE_ADDR 0xe20d1000 /* PPC460EX SATA Base Address */
#define SATA_DMA_REG_ADDR 0xe20d0800 /* PPC460EX SATA Base Address */
#define CONFIG_SYS_SATA_MAX_DEVICE 1 /* SATA MAX DEVICE */
/* Convert sectorsize to wordsize */
#define ATA_SECTOR_WORDS(ATA_SECT_SIZE / 2)
#endif /* CONFIG_CMD_SATA */
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: off...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Canyonlands SATA harddisk driver
Dear Kazuaki Ichinohe, In message 4a027e44.5080...@fsi.co.jp you wrote: Hello Denk, Stefan, Could you please rebase your patch against the current top-of-tree mainline repository and resumbmit? Thank you for the reply. I made the patch from the source obtained with git in 5/7. The confirmed patch is sent again. Please review this patch. --- Please put comments BELOW the --- line, not above. Only put the commit message ABOVE the --- line. And note that there is only ONE --- line in a patch. [patch] diff -uprN u-boot-0507/drivers/block/Makefile u-boot-sata/drivers/block/Makefile --- u-boot-0507/drivers/block/Makefile2009-05-07 09:25:48.0 +0900 +++ u-boot-sata/drivers/block/Makefile2009-05-07 09:43:56.0 +0900 @@ -35,6 +35,7 @@ COBJS-$(CONFIG_SATA_SIL3114) += sata_sil COBJS-$(CONFIG_SCSI_AHCI) += ahci.o COBJS-$(CONFIG_SCSI_SYM53C8XX) += sym53c8xx.o COBJS-$(CONFIG_SYSTEMACE) += systemace.o +COBJS-$(CONFIG_SATA_DWC) += sata_dwc.o Sorry, your patch is white-space corrupted: patching file drivers/block/Makefile Hunk #1 FAILED at 35. 1 out of 1 hunk FAILED -- saving rejects to file drivers/block/Makefile.rej patching file drivers/block/sata_dwc.c patching file drivers/block/sata_dwc.h patching file include/configs/canyonlands.h Hunk #1 FAILED at 454. patch: malformed patch at line 2718: Thanks. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Nearly everyone is in favor of going to heaven but too many are hoping they'll live long enough to see an easing of the entrance requirements. Never appeal to a man's better nature. he might not have one. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Canyonlands SATA harddisk driver
On Monday 27 April 2009, Kazuaki Ichinohe wrote: I sent the SATA patch mail on April 17. However, the following content is not confirmed, and there is not a reply either. Could you confirm it? I re-send the SATA driver patch. I failed applying your patch: Applying: Canyonlands SATA harddisk driver error: patch failed: drivers/block/Makefile:35 error: drivers/block/Makefile: patch does not apply error: patch failed: include/configs/canyonlands.h:454 error: include/configs/canyonlands.h: patch does not apply Patch failed at 0001 Canyonlands SATA harddisk driver When you have resolved this problem run git am --resolved. If you would prefer to skip this patch, instead run git am --skip. To restore the original branch and stop patching run git am --abort. Could you please rebase your patch against the current top-of-tree mainline repository and resumbmit? 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: off...@denx.de = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Canyonlands SATA harddisk driver
Hi Kazuaki Ichinohe, On Monday 27 April 2009, Kazuaki Ichinohe wrote: I sent the SATA patch mail on April 17. However, the following content is not confirmed, and there is not a reply either. Could you confirm it? I re-send the SATA driver patch. Sorry, I forgot about reviewing it. I'll do this today or tomorrow. 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: off...@denx.de = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Canyonlands SATA harddisk driver
Hello Denk, After confirming operation, I'll send sata_dwc.c with cmd_sata.c again. Regards, Kazuaki Ichinohe Wolfgang Denk wrote: Dear Kazuaki Ichinohe, In message 49cb5f77.1060...@fsi.co.jp you wrote: ... Why do you add this 460EX SATA support to cmd_scsi.c? Wouldn't cmd_sata.c be a better place? Or did I miss something here? The cmd_scsi.c which define CONFIG_CMD_SCSI are modified because I want to use scsiboot command of U-BOOT. This makes no sense. It is a S-ATA driver, not a SCSI driver, and as such it has to be supported in the context of the S-ATA support. I explicitly NAK the changes to the SCSI related files. Best regards, Wolfgang Denk ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Canyonlands SATA harddisk driver
On Thursday 26 March 2009, Kazuaki Ichinohe wrote: Thank you for the reply. My answer is described in the beginning. OK, I just tried to apply your patch for basic testing. Didn't work though. Here the error log: [ste...@kubuntu u-boot (master)]$ patch -p1 patches_sata/\[U-Boot\]\ \[PATCH\]\ Canyonlands\ SATA\ harddisk\ driver.mbox patching file common/cmd_scsi.c Hunk #1 FAILED at 47. Hunk #2 FAILED at 181. Hunk #3 FAILED at 192. Hunk #4 FAILED at 453. 4 out of 4 hunks FAILED -- saving rejects to file common/cmd_scsi.c.rej patching file drivers/block/Makefile Hunk #1 FAILED at 34. 1 out of 1 hunk FAILED -- saving rejects to file drivers/block/Makefile.rej patching file drivers/block/sata_dwc.c patching file drivers/block/sata_dwc.h patching file include/configs/canyonlands.h Hunk #1 FAILED at 454. Hunk #2 FAILED at 518. 2 out of 2 hunks FAILED -- saving rejects to file include/configs/canyonlands.h.rej Seems that you didn't base your patch on the latest git version of U-Boot. Please rebase your patch again that it applies cleanly. And again, please use the git tools to generate the patch. Or at least send the patch in a git compatible way, so that it can be applied using git am. 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: off...@denx.de = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Canyonlands SATA harddisk driver
Dear Kazuaki Ichinohe, In message 49cb5f77.1060...@fsi.co.jp you wrote: ... Why do you add this 460EX SATA support to cmd_scsi.c? Wouldn't cmd_sata.c be a better place? Or did I miss something here? The cmd_scsi.c which define CONFIG_CMD_SCSI are modified because I want to use scsiboot command of U-BOOT. This makes no sense. It is a S-ATA driver, not a SCSI driver, and as such it has to be supported in the context of the S-ATA support. I explicitly NAK the changes to the SCSI related files. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de A quarrel is quickly settled when deserted by one party; there is no battle unless there be two. - Seneca ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] Canyonlands SATA harddisk driver
On Wednesday 25 March 2009, Kazuaki Ichinohe wrote:
Hi Stefan,
Thank you for the reply.
This is the patch that corrects the point.
Signed-off-by: Kazuaki Ichinohe kazuichi at fsi.co.jp
OK, now you added your Signed-off-by line and the patch itself doesn't seem to
be line wrapped. But the commit text above is not really describing the
patch. Your first version was better.
Please find some more comments below.
---
[patch]
diff -purN u-boot-2009.03/common/cmd_scsi.c
u-boot-2009.03-sata/common/cmd_scsi.c ---
u-boot-2009.03/common/cmd_scsi.c 2009-03-22 06:04:41.0 +0900 +++
u-boot-2009.03-sata/common/cmd_scsi.c 2009-03-25 18:53:29.0 +0900
@@ -47,8 +47,10 @@
#define SCSI_DEV_ID 0x5288
#else
+#ifndef CONFIG_SATA_DWC
#error no scsi device defined
#endif
+#endif
Why do you add this 460EX SATA support to cmd_scsi.c? Wouldn't cmd_sata.c be a
better place? Or did I miss something here?
snip
+++ u-boot-2009.03-sata/drivers/block/sata_dwc.c 2009-03-25
18:02:07.0 +0900 @@ -0,0 +1,2175 @@
+/*
+ * sata_dwc.c
+ *
+ * Synopsys DesignWare Cores (DWC) SATA host driver
+ *
+ * Author: Mark Miesfeld mmiesf...@amcc.com
+ *
+ * Ported from 2.6.19.2 to 2.6.25/26 by Stefan Roese s...@denx.de
+ * Copyright 2008 DENX Software Engineering
+ *
+ * Based on versions provided by AMCC and Synopsys which are:
+ * Copyright 2006 Applied Micro Circuits Corporation
+ * COPYRIGHT (C) 2005 SYNOPSYS, INC. ALL RIGHTS RESERVED
+ *
+ * This program is free software; you can redistribute
+ * it and/or modify it under the terms of the GNU
+ * General Public License as published by the
+ * Free Software Foundation; either version 2 of the License,
+ * or (at your option) any later version.
+ *
+ */
As it seems you added the Linux driver version with some changes so that it
fits into U-Boot. You should document this in the commit text of this patch.
+
+#include common.h
+#include command.h
+#include pci.h
+#include asm/processor.h
+#include asm/errno.h
+#include asm/io.h
+#include malloc.h
+#include scsi.h
+#include ata.h
+#include linux/ctype.h
+
+/* sata_dwc.h */
+#include sata_dwc.h
+
+/* Base Address */
+#define SATA_BASE_ADDR 0xe20d1000
+#define SATA_DMA_REG_ADDR0xe20d0800
These defines should be moved to another (Canyonlands specific) header. It
could be that at some time another platform/board will be added using the
same DesignWare SATA core but with different addresses.
snip
+u8 ata_check_altstatus(struct ata_port *ap)
+{
+ u8 val = 0;
+ val = readb(ap-ioaddr.altstatus_addr);
+ return val;
+}
+
+int check_sata_dev_state(void)
+{
+ static ccb tempccb; /* temporary scsi command buffer */
+ ccb *pccb = (ccb *)tempccb;
+ int ret = 0;
+ int i = 0;
+
+ while(1){
Space after while please. Please check this file again. Here are multiple
places where you don't have a space after the while or if statement.
+ udelay (1); /* 10 ms */
+
+ pccb-cmd[0] = SCSI_READ10;
+ pccb-cmd[1] = 0;
+ pccb-cmd[2] = 0;
+ pccb-cmd[3] = 0;
+ pccb-cmd[4] = 0;
+ pccb-cmd[5] = 0;
+ pccb-cmd[6] = 0;
+ pccb-cmd[7] = 0;
+ pccb-cmd[8] = 1;
+ pccb-cmdlen = 10;
+ pccb-pdata = temp_data_buf[0];/* dummy */
+ pccb-datalen = 512;
+
+ /* Send Read Command */
+ ret = ata_dev_read_sectors(pccb);
+
+ /* result TRUE = break */
+ if(ret == 0){
Here again.
+ break;
+ }
And these curly braces are not needed (because of a single line statement).
Please remove them,
+
+ i++;
+ if (i (ATA_RESET_TIME * 100)){
+ printf(** TimeOUT **\n);
+ dev_state = SATA_NODEVICE; /* set device status
flag */
+ return FALSE;
+ }
+
+ if ((i = 100) ((i%100)==0)) {
+ printf(.);
+ }
Again, single line statement.
+ }
+
+ /* Set device status flag */
+ dev_state = SATA_READY;
+
+ return TRUE;
+}
+
+void ata_id_string(const u16 *id, unsigned char *s,
+ unsigned int ofs, unsigned int len)
+{
+ unsigned int c;
+
+ while (len 0) {
+ c = id[ofs] 8;
+ *s = c;
+ s++;
+
+ c = id[ofs] 0xff;
+ *s = c;
+ s++;
+
+ ofs++;
+ len -= 2;
+ }
+}
+
+static int waiting_for_reg_state(volatile u8 *offset,
+ int timeout_msec,
+ u32 sign)
+{
+ int i;
+ u32 status;
+
+ for (i = 0; i timeout_msec; i++){
+ status = readl(offset);
+
