From mboxrd@z Thu Jan 1 00:00:00 1970 From: Rob Herring Date: Mon, 04 Jul 2011 09:51:30 -0500 Subject: [U-Boot] [PATCH 5/6] scsi/ahci: add support for non-PCI controllers In-Reply-To: <20110704101739.8561F15794D5@gemini.denx.de> References: <1309275583-11763-1-git-send-email-robherring2@gmail.com> <1309275583-11763-6-git-send-email-robherring2@gmail.com> <20110704101739.8561F15794D5@gemini.denx.de> Message-ID: <4E11D372.8090708@calxeda.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Wolfgang, On 07/04/2011 05:17 AM, Wolfgang Denk wrote: > Dear Rob Herring, > > In message <1309275583-11763-6-git-send-email-robherring2@gmail.com> you wrote: >> From: Rob Herring >> >> Add support for AHCI controllers that are not PCI based. >> >> Signed-off-by: Rob Herring >> Cc: Wolfgang Denk >> --- >> common/cmd_scsi.c | 6 +++- >> drivers/block/ahci.c | 70 +++++++++++++++++++++++++++++++++++++++++++------ >> include/ahci.h | 4 +++ >> include/scsi.h | 1 + >> 4 files changed, 70 insertions(+), 11 deletions(-) >> >> diff --git a/common/cmd_scsi.c b/common/cmd_scsi.c >> index be4fe74..25a8299 100644 >> --- a/common/cmd_scsi.c >> +++ b/common/cmd_scsi.c >> @@ -47,7 +47,8 @@ >> #define SCSI_DEV_ID 0x5288 >> >> #else >> -#error no scsi device defined >> +#define SCSI_VEND_ID 0 >> +#define SCSI_DEV_ID 0 >> #endif > > I'm not sure if this is a good idea. Please explain. This is the PCI ID and is only used here: common/cmd_scsi.c:183: busdevfunc=pci_find_device(SCSI_VEND_ID,SCSI_DEV_ID,0); /* get PCI Device ID */ For a non-PCI AHCI controller, there is no id. For PCI, I don't think 0 is a valid vendor ID anyway. > > Also, checkpatch says: > > ERROR: trailing whitespace > WARNING: please, no spaces at the start of a line > #287: FILE: include/ahci.h:193: > + $ > >> +#ifdef CONFIG_PCI >> pci_read_config_word(pdev, 0x0a, &cc); >> if (cc == 0x0101) >> scc_s = "IDE"; >> @@ -222,7 +227,9 @@ static void ahci_print_info(struct ahci_probe_ent *probe_ent) >> scc_s = "RAID"; >> else >> scc_s = "unknown"; >> - >> +#else >> + scc_s = "SATA"; >> +#endif > > Is SATA really the only possible option here? Well I suppose anything is possible, but for embbedded SOCs with AHCI I've seen, they are SATA only. It's purely informational. > >> +int ahci_init(u32 base) >> +{ > ... >> +} > > Should this always be compiled in? I can add a config option CONFIG_SCSI_AHCI_PLAT. Perhaps this would be better than using CONFIG_PCI as I suppose you could have non-PCI AHCI controller on a platform with PCI. Rob