[U-Boot] [PATCH 1/4] ata: ahci: Loop over the actual number of ports, not the maximum
The loop in ahci_start_ports() is looping over the maximum number of SCSI devices in the system, which can be larger than the amount of ports a particular AHCI controller has. The extra looping isn't directly harmful because the link_port_map bitmap won't have the bit set for a nonexistent port, but it is wasteful. Replace the loop limit with the port count of the AHCI controller instead.
Signed-off-by: Tuomas Tynkkynen tuomas.tynkkynen@iki.fi --- drivers/ata/ahci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c index c35912bd33..333f0457f6 100644 --- a/drivers/ata/ahci.c +++ b/drivers/ata/ahci.c @@ -980,7 +980,7 @@ static int ahci_start_ports(struct ahci_uc_priv *uc_priv)
linkmap = uc_priv->link_port_map;
- for (i = 0; i < CONFIG_SYS_SCSI_MAX_SCSI_ID; i++) { + for (i = 0; i < uc_priv->n_ports; i++) { if (((linkmap >> i) & 0x01)) { if (ahci_port_start(uc_priv, (u8) i)) { printf("Can not start port %d\n", i);
When using device model this sort of hardcoded limits aren't used or necessary.
Signed-off-by: Tuomas Tynkkynen tuomas.tynkkynen@iki.fi --- drivers/ata/ahci.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c index 333f0457f6..5fafb63aeb 100644 --- a/drivers/ata/ahci.c +++ b/drivers/ata/ahci.c @@ -230,8 +230,10 @@ static int ahci_host_init(struct ahci_uc_priv *uc_priv) debug("cap 0x%x port_map 0x%x n_ports %d\n", uc_priv->cap, uc_priv->port_map, uc_priv->n_ports);
+#if !defined(CONFIG_DM_SCSI) if (uc_priv->n_ports > CONFIG_SYS_SCSI_MAX_SCSI_ID) uc_priv->n_ports = CONFIG_SYS_SCSI_MAX_SCSI_ID; +#endif
for (i = 0; i < uc_priv->n_ports; i++) { if (!(port_map & (1 << i)))
On 13 September 2018 at 00:28, Tuomas Tynkkynen tuomas.tynkkynen@iki.fi wrote:
When using device model this sort of hardcoded limits aren't used or necessary.
Signed-off-by: Tuomas Tynkkynen tuomas.tynkkynen@iki.fi
drivers/ata/ahci.c | 2 ++ 1 file changed, 2 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
But please see below
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c index 333f0457f6..5fafb63aeb 100644 --- a/drivers/ata/ahci.c +++ b/drivers/ata/ahci.c @@ -230,8 +230,10 @@ static int ahci_host_init(struct ahci_uc_priv *uc_priv) debug("cap 0x%x port_map 0x%x n_ports %d\n", uc_priv->cap, uc_priv->port_map, uc_priv->n_ports);
+#if !defined(CONFIG_DM_SCSI)
Can you use this instead?
if (IS_DEFINED(CONFIG_DM_SCSI))
if (uc_priv->n_ports > CONFIG_SYS_SCSI_MAX_SCSI_ID) uc_priv->n_ports = CONFIG_SYS_SCSI_MAX_SCSI_ID;+#endif
for (i = 0; i < uc_priv->n_ports; i++) { if (!(port_map & (1 << i)))-- 2.16.3
Hi,
On 09/14/2018 01:55 PM, Simon Glass wrote:
On 13 September 2018 at 00:28, Tuomas Tynkkynen tuomas.tynkkynen@iki.fi wrote:
When using device model this sort of hardcoded limits aren't used or necessary.
Signed-off-by: Tuomas Tynkkynen tuomas.tynkkynen@iki.fi
drivers/ata/ahci.c | 2 ++ 1 file changed, 2 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
But please see below
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c index 333f0457f6..5fafb63aeb 100644 --- a/drivers/ata/ahci.c +++ b/drivers/ata/ahci.c @@ -230,8 +230,10 @@ static int ahci_host_init(struct ahci_uc_priv *uc_priv) debug("cap 0x%x port_map 0x%x n_ports %d\n", uc_priv->cap, uc_priv->port_map, uc_priv->n_ports);
+#if !defined(CONFIG_DM_SCSI)
Can you use this instead?
if (IS_DEFINED(CONFIG_DM_SCSI))
No, that won't work because after patch 3 CONFIG_SYS_SCSI_MAX_SCSI_ID won't be defined for boards using CONFIG_DM_SCSI, so using the preprocessor is necessary.
On Thu, Sep 13, 2018 at 01:28:55AM +0300, Tuomas Tynkkynen wrote:
When using device model this sort of hardcoded limits aren't used or necessary.
Signed-off-by: Tuomas Tynkkynen tuomas.tynkkynen@iki.fi Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
These options are not used or necessary when device model is being used for SCSI. Just drop them.
Signed-off-by: Tuomas Tynkkynen tuomas.tynkkynen@iki.fi --- include/configs/dra7xx_evm.h | 4 ---- include/configs/qemu-arm.h | 3 --- include/configs/x86-common.h | 4 ---- include/configs/xilinx_zynqmp.h | 7 ------- 4 files changed, 18 deletions(-)
diff --git a/include/configs/dra7xx_evm.h b/include/configs/dra7xx_evm.h index fcaf3a1e13..d8d6d2f6b0 100644 --- a/include/configs/dra7xx_evm.h +++ b/include/configs/dra7xx_evm.h @@ -113,10 +113,6 @@
/* SATA */ #define CONFIG_SCSI_AHCI_PLAT -#define CONFIG_SYS_SCSI_MAX_SCSI_ID 1 -#define CONFIG_SYS_SCSI_MAX_LUN 1 -#define CONFIG_SYS_SCSI_MAX_DEVICE (CONFIG_SYS_SCSI_MAX_SCSI_ID * \ - CONFIG_SYS_SCSI_MAX_LUN)
/* NAND support */ #ifdef CONFIG_NAND diff --git a/include/configs/qemu-arm.h b/include/configs/qemu-arm.h index 66729b7d4f..5a12a32d39 100644 --- a/include/configs/qemu-arm.h +++ b/include/configs/qemu-arm.h @@ -20,9 +20,6 @@ /* For timer, QEMU emulates an ARMv7/ARMv8 architected timer */ #define CONFIG_SYS_HZ 1000
-/* For block devices, QEMU emulates an ICH9 AHCI controller over PCI */ -#define CONFIG_SYS_SCSI_MAX_SCSI_ID 6 - /* QEMU emulates the ARM AMBA PL031 RTC */ #define CONFIG_SYS_RTC_PL031_BASE 0x09010000
diff --git a/include/configs/x86-common.h b/include/configs/x86-common.h index f0b027e69c..ab345326e8 100644 --- a/include/configs/x86-common.h +++ b/include/configs/x86-common.h @@ -28,10 +28,6 @@ #define CONFIG_LBA48 #define CONFIG_SYS_64BIT_LBA
-#define CONFIG_SYS_SCSI_MAX_SCSI_ID 2 -#define CONFIG_SYS_SCSI_MAX_LUN 1 -#define CONFIG_SYS_SCSI_MAX_DEVICE (CONFIG_SYS_SCSI_MAX_SCSI_ID * \ - CONFIG_SYS_SCSI_MAX_LUN) #endif
/* Generic TPM interfaced through LPC bus */ diff --git a/include/configs/xilinx_zynqmp.h b/include/configs/xilinx_zynqmp.h index a65e8fedff..0ab32611ce 100644 --- a/include/configs/xilinx_zynqmp.h +++ b/include/configs/xilinx_zynqmp.h @@ -122,13 +122,6 @@ # define CONFIG_SYS_EEPROM_SIZE (64 * 1024) #endif
-#ifdef CONFIG_SATA_CEVA -#define CONFIG_SYS_SCSI_MAX_SCSI_ID 2 -#define CONFIG_SYS_SCSI_MAX_LUN 1 -#define CONFIG_SYS_SCSI_MAX_DEVICE (CONFIG_SYS_SCSI_MAX_SCSI_ID * \ - CONFIG_SYS_SCSI_MAX_LUN) -#endif - #define CONFIG_SYS_BOOTM_LEN (60 * 1024 * 1024)
#define CONFIG_CLOCKS
On 13 September 2018 at 00:28, Tuomas Tynkkynen tuomas.tynkkynen@iki.fi wrote:
These options are not used or necessary when device model is being used for SCSI. Just drop them.
Signed-off-by: Tuomas Tynkkynen tuomas.tynkkynen@iki.fi
include/configs/dra7xx_evm.h | 4 ---- include/configs/qemu-arm.h | 3 --- include/configs/x86-common.h | 4 ---- include/configs/xilinx_zynqmp.h | 7 ------- 4 files changed, 18 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
On Thu, Sep 13, 2018 at 01:28:56AM +0300, Tuomas Tynkkynen wrote:
These options are not used or necessary when device model is being used for SCSI. Just drop them.
Signed-off-by: Tuomas Tynkkynen tuomas.tynkkynen@iki.fi Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
This option has never been used for anything. Drop it.
Signed-off-by: Tuomas Tynkkynen tuomas.tynkkynen@iki.fi --- include/configs/MPC8544DS.h | 3 +-- include/configs/MPC8572DS.h | 1 - include/configs/MPC8610HPCD.h | 1 - include/configs/MPC8641HPCN.h | 1 - include/configs/sbc8641d.h | 1 - scripts/config_whitelist.txt | 1 - 6 files changed, 1 insertion(+), 7 deletions(-)
diff --git a/include/configs/MPC8544DS.h b/include/configs/MPC8544DS.h index 2568e95270..d825f0fc33 100644 --- a/include/configs/MPC8544DS.h +++ b/include/configs/MPC8544DS.h @@ -280,8 +280,7 @@ extern unsigned long get_board_sys_clk(unsigned long dummy); #define CONFIG_SYS_SCSI_MAX_SCSI_ID 4 #define CONFIG_SYS_SCSI_MAX_LUN 1 #define CONFIG_SYS_SCSI_MAX_DEVICE (CONFIG_SYS_SCSI_MAX_SCSI_ID * CONFIG_SYS_SCSI_MAX_LUN) -#define CONFIG_SYS_SCSI_MAXDEVICE CONFIG_SYS_SCSI_MAX_DEVICE -#endif /* SCSCI */ +#endif /* CONFIG_SCSI_AHCI */
#endif /* CONFIG_PCI */
diff --git a/include/configs/MPC8572DS.h b/include/configs/MPC8572DS.h index 8c92c3f832..dd081e8c12 100644 --- a/include/configs/MPC8572DS.h +++ b/include/configs/MPC8572DS.h @@ -464,7 +464,6 @@ #define CONFIG_SYS_SCSI_MAX_SCSI_ID 4 #define CONFIG_SYS_SCSI_MAX_LUN 1 #define CONFIG_SYS_SCSI_MAX_DEVICE (CONFIG_SYS_SCSI_MAX_SCSI_ID * CONFIG_SYS_SCSI_MAX_LUN) -#define CONFIG_SYS_SCSI_MAXDEVICE CONFIG_SYS_SCSI_MAX_DEVICE #endif /* SCSI */
#endif /* CONFIG_PCI */ diff --git a/include/configs/MPC8610HPCD.h b/include/configs/MPC8610HPCD.h index cfb7135870..02fd864727 100644 --- a/include/configs/MPC8610HPCD.h +++ b/include/configs/MPC8610HPCD.h @@ -278,7 +278,6 @@ #define CONFIG_SYS_SCSI_MAX_SCSI_ID 4 #define CONFIG_SYS_SCSI_MAX_LUN 1 #define CONFIG_SYS_SCSI_MAX_DEVICE (CONFIG_SYS_SCSI_MAX_SCSI_ID * CONFIG_SYS_SCSI_MAX_LUN) -#define CONFIG_SYS_SCSI_MAXDEVICE CONFIG_SYS_SCSI_MAX_DEVICE #endif
#endif /* CONFIG_PCI */ diff --git a/include/configs/MPC8641HPCN.h b/include/configs/MPC8641HPCN.h index 68bc710b02..bc69efbbe6 100644 --- a/include/configs/MPC8641HPCN.h +++ b/include/configs/MPC8641HPCN.h @@ -373,7 +373,6 @@ extern unsigned long get_board_sys_clk(unsigned long dummy); #define CONFIG_SYS_SCSI_MAX_SCSI_ID 4 #define CONFIG_SYS_SCSI_MAX_LUN 1 #define CONFIG_SYS_SCSI_MAX_DEVICE (CONFIG_SYS_SCSI_MAX_SCSI_ID * CONFIG_SYS_SCSI_MAX_LUN) -#define CONFIG_SYS_SCSI_MAXDEVICE CONFIG_SYS_SCSI_MAX_DEVICE #endif
#endif /* CONFIG_PCI */ diff --git a/include/configs/sbc8641d.h b/include/configs/sbc8641d.h index c509822814..d777e7a36a 100644 --- a/include/configs/sbc8641d.h +++ b/include/configs/sbc8641d.h @@ -298,7 +298,6 @@ #define CONFIG_SYS_SCSI_MAX_SCSI_ID 4 #define CONFIG_SYS_SCSI_MAX_LUN 1 #define CONFIG_SYS_SCSI_MAX_DEVICE (CONFIG_SYS_SCSI_MAX_SCSI_ID * CONFIG_SYS_SCSI_MAX_LUN) -#define CONFIG_SYS_SCSI_MAXDEVICE CONFIG_SYS_SCSI_MAX_DEVICE #endif
#endif /* CONFIG_PCI */ diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt index 30c79a643e..6a4a20ac3a 100644 --- a/scripts/config_whitelist.txt +++ b/scripts/config_whitelist.txt @@ -4101,7 +4101,6 @@ CONFIG_SYS_SCCR_USBDRCM CONFIG_SYS_SCCR_USBMPHCM CONFIG_SYS_SCR CONFIG_SYS_SCRATCH_VA -CONFIG_SYS_SCSI_MAXDEVICE CONFIG_SYS_SCSI_MAX_DEVICE CONFIG_SYS_SCSI_MAX_LUN CONFIG_SYS_SCSI_MAX_SCSI_ID
On 13 September 2018 at 00:28, Tuomas Tynkkynen tuomas.tynkkynen@iki.fi wrote:
This option has never been used for anything. Drop it.
Signed-off-by: Tuomas Tynkkynen tuomas.tynkkynen@iki.fi
include/configs/MPC8544DS.h | 3 +-- include/configs/MPC8572DS.h | 1 - include/configs/MPC8610HPCD.h | 1 - include/configs/MPC8641HPCN.h | 1 - include/configs/sbc8641d.h | 1 - scripts/config_whitelist.txt | 1 - 6 files changed, 1 insertion(+), 7 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
On Thu, Sep 13, 2018 at 01:28:57AM +0300, Tuomas Tynkkynen wrote:
This option has never been used for anything. Drop it.
Signed-off-by: Tuomas Tynkkynen tuomas.tynkkynen@iki.fi Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
On 13 September 2018 at 00:28, Tuomas Tynkkynen tuomas.tynkkynen@iki.fi wrote:
The loop in ahci_start_ports() is looping over the maximum number of SCSI devices in the system, which can be larger than the amount of ports a particular AHCI controller has. The extra looping isn't directly harmful because the link_port_map bitmap won't have the bit set for a nonexistent port, but it is wasteful. Replace the loop limit with the port count of the AHCI controller instead.
Signed-off-by: Tuomas Tynkkynen tuomas.tynkkynen@iki.fi
drivers/ata/ahci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org
On Thu, Sep 13, 2018 at 01:28:54AM +0300, Tuomas Tynkkynen wrote:
The loop in ahci_start_ports() is looping over the maximum number of SCSI devices in the system, which can be larger than the amount of ports a particular AHCI controller has. The extra looping isn't directly harmful because the link_port_map bitmap won't have the bit set for a nonexistent port, but it is wasteful. Replace the loop limit with the port count of the AHCI controller instead.
Signed-off-by: Tuomas Tynkkynen tuomas.tynkkynen@iki.fi Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
participants (3)
-
Simon Glass -
Tom Rini -
Tuomas Tynkkynen