[U-Boot] [PATCH] dwc_ahsata: return failure for MX6 if not IMX6Q

The IMX6QUAD/DUAL have SATA, but the IMX6SOLO/DL do not. Return an error indicating no such port instead of attempting a memory access that results in a data abort and reset. This dynamic detection is necessary for bootloaders that support multiple variants of the IMX6 SoC.
Signed-off-by: Tim Harvey tharvey@gateworks.com --- drivers/block/dwc_ahsata.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/drivers/block/dwc_ahsata.c b/drivers/block/dwc_ahsata.c index 3569214..638a585 100644 --- a/drivers/block/dwc_ahsata.c +++ b/drivers/block/dwc_ahsata.c @@ -17,6 +17,7 @@ #include <asm/io.h> #include <linux/bitops.h> #include <asm/arch/clock.h> +#include <asm/arch/sys_proto.h> #include "dwc_ahsata.h"
struct sata_port_regs { @@ -558,6 +559,12 @@ int init_sata(int dev) u32 linkmap; struct ahci_probe_ent *probe_ent = NULL;
+#if defined(CONFIG_MX6) + if (!is_cpu_type(MXC_CPU_MX6Q)) { + printf("No device detected!\n"); + return 1; + } +#endif if (dev < 0 || dev > (CONFIG_SYS_SATA_MAX_DEVICE - 1)) { printf("The sata index %d is out of ranges\n\r", dev); return -1;

Hi Tim,
On 06/05/2014 07:22, Tim Harvey wrote:
The IMX6QUAD/DUAL have SATA, but the IMX6SOLO/DL do not. Return an error indicating no such port instead of attempting a memory access that results in a data abort and reset. This dynamic detection is necessary for bootloaders that support multiple variants of the IMX6 SoC.
Right !
Signed-off-by: Tim Harvey tharvey@gateworks.com
drivers/block/dwc_ahsata.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/drivers/block/dwc_ahsata.c b/drivers/block/dwc_ahsata.c index 3569214..638a585 100644 --- a/drivers/block/dwc_ahsata.c +++ b/drivers/block/dwc_ahsata.c @@ -17,6 +17,7 @@ #include <asm/io.h> #include <linux/bitops.h> #include <asm/arch/clock.h> +#include <asm/arch/sys_proto.h> #include "dwc_ahsata.h"
struct sata_port_regs { @@ -558,6 +559,12 @@ int init_sata(int dev) u32 linkmap; struct ahci_probe_ent *probe_ent = NULL;
+#if defined(CONFIG_MX6)
- if (!is_cpu_type(MXC_CPU_MX6Q)) {
printf("No device detected!\n");
Change only printf with puts. We use puts() in u-boot when we print a fixed string. Really I had drop the whole line. It is not that a SATA device is not attached or not detected, but it is that a SATA is not possible at all. It is like this is an error, but it is not: it is the real use case.
Best regards, Stefano Babic

On 5/5/2014 10:22 PM, Tim Harvey wrote:
The IMX6QUAD/DUAL have SATA, but the IMX6SOLO/DL do not. Return an error indicating no such port instead of attempting a memory access that results in a data abort and reset. This dynamic detection is necessary for bootloaders that support multiple variants of the IMX6 SoC.
Signed-off-by: Tim Harvey tharvey@gateworks.com
drivers/block/dwc_ahsata.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/drivers/block/dwc_ahsata.c b/drivers/block/dwc_ahsata.c index 3569214..638a585 100644 --- a/drivers/block/dwc_ahsata.c +++ b/drivers/block/dwc_ahsata.c @@ -17,6 +17,7 @@ #include <asm/io.h> #include <linux/bitops.h> #include <asm/arch/clock.h> +#include <asm/arch/sys_proto.h> #include "dwc_ahsata.h"
struct sata_port_regs { @@ -558,6 +559,12 @@ int init_sata(int dev) u32 linkmap; struct ahci_probe_ent *probe_ent = NULL;
+#if defined(CONFIG_MX6)
- if (!is_cpu_type(MXC_CPU_MX6Q)) {
&& !is_cpu_type(MXC_CPU_MX6D)
is needed too ?
Troy

On Tue, May 6, 2014 at 12:55 PM, Troy Kisky troy.kisky@boundarydevices.com wrote:
On 5/5/2014 10:22 PM, Tim Harvey wrote:
The IMX6QUAD/DUAL have SATA, but the IMX6SOLO/DL do not. Return an error indicating no such port instead of attempting a memory access that results in a data abort and reset. This dynamic detection is necessary for bootloaders that support multiple variants of the IMX6 SoC.
Signed-off-by: Tim Harvey tharvey@gateworks.com
drivers/block/dwc_ahsata.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/drivers/block/dwc_ahsata.c b/drivers/block/dwc_ahsata.c index 3569214..638a585 100644 --- a/drivers/block/dwc_ahsata.c +++ b/drivers/block/dwc_ahsata.c @@ -17,6 +17,7 @@ #include <asm/io.h> #include <linux/bitops.h> #include <asm/arch/clock.h> +#include <asm/arch/sys_proto.h> #include "dwc_ahsata.h"
struct sata_port_regs { @@ -558,6 +559,12 @@ int init_sata(int dev) u32 linkmap; struct ahci_probe_ent *probe_ent = NULL;
+#if defined(CONFIG_MX6)
if (!is_cpu_type(MXC_CPU_MX6Q)) {
&& !is_cpu_type(MXC_CPU_MX6D)
is needed too ?
Troy
Troy,
Indeed - thanks for the reminder!
Tim

The IMX6QUAD/DUAL have SATA, but the IMX6SOLO/DL do not. Return failure instead of attempting a memory access that results in a data abort and reset.
Signed-off-by: Tim Harvey tharvey@gateworks.com --- v2: - remove print as this condition isn't really an error just something not possible --- drivers/block/dwc_ahsata.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/block/dwc_ahsata.c b/drivers/block/dwc_ahsata.c index 3569214..15d65d7 100644 --- a/drivers/block/dwc_ahsata.c +++ b/drivers/block/dwc_ahsata.c @@ -17,6 +17,7 @@ #include <asm/io.h> #include <linux/bitops.h> #include <asm/arch/clock.h> +#include <asm/arch/sys_proto.h> #include "dwc_ahsata.h"
struct sata_port_regs { @@ -558,6 +559,10 @@ int init_sata(int dev) u32 linkmap; struct ahci_probe_ent *probe_ent = NULL;
+#if defined(CONFIG_MX6) + if (!is_cpu_type(MXC_CPU_MX6Q) && !is_cpu_type(MXC_CPU_MX6D)) + return 1; +#endif if (dev < 0 || dev > (CONFIG_SYS_SATA_MAX_DEVICE - 1)) { printf("The sata index %d is out of ranges\n\r", dev); return -1;

On Wed, May 7, 2014 at 10:23 PM, Tim Harvey tharvey@gateworks.com wrote:
The IMX6QUAD/DUAL have SATA, but the IMX6SOLO/DL do not. Return failure instead of attempting a memory access that results in a data abort and reset.
Signed-off-by: Tim Harvey tharvey@gateworks.com
v2:
- remove print as this condition isn't really an error just something not possible
drivers/block/dwc_ahsata.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/block/dwc_ahsata.c b/drivers/block/dwc_ahsata.c index 3569214..15d65d7 100644 --- a/drivers/block/dwc_ahsata.c +++ b/drivers/block/dwc_ahsata.c @@ -17,6 +17,7 @@ #include <asm/io.h> #include <linux/bitops.h> #include <asm/arch/clock.h> +#include <asm/arch/sys_proto.h> #include "dwc_ahsata.h"
struct sata_port_regs { @@ -558,6 +559,10 @@ int init_sata(int dev) u32 linkmap; struct ahci_probe_ent *probe_ent = NULL;
+#if defined(CONFIG_MX6)
if (!is_cpu_type(MXC_CPU_MX6Q) && !is_cpu_type(MXC_CPU_MX6D))
return 1;
+#endif if (dev < 0 || dev > (CONFIG_SYS_SATA_MAX_DEVICE - 1)) { printf("The sata index %d is out of ranges\n\r", dev); return -1; -- 1.8.3.2
Stefano,
Any comments or does anyone else need to review this?
Regards,
Tim

On 08/05/2014 07:23, Tim Harvey wrote:
The IMX6QUAD/DUAL have SATA, but the IMX6SOLO/DL do not. Return failure instead of attempting a memory access that results in a data abort and reset.
Signed-off-by: Tim Harvey tharvey@gateworks.com
v2:
- remove print as this condition isn't really an error just something not possible
Applied to u-boot-imx, thanks !
Best regards, Stefano Babic

The IMX6QUAD/DUAL have SATA, but the IMX6SOLO/DL do not. Return instead of configuring the SATA clock and GPR13 registers.
Signed-off-by: Tim Harvey tharvey@gateworks.com --- arch/arm/imx-common/sata.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/arm/imx-common/sata.c b/arch/arm/imx-common/sata.c index 2e69486..c10dd28 100644 --- a/arch/arm/imx-common/sata.c +++ b/arch/arm/imx-common/sata.c @@ -8,13 +8,18 @@ #include <asm/arch/iomux.h> #include <asm/io.h> #include <asm/arch/clock.h> +#include <asm/arch/sys_proto.h>
int setup_sata(void) { struct iomuxc_base_regs *const iomuxc_regs = (struct iomuxc_base_regs *)IOMUXC_BASE_ADDR; + int ret;
- int ret = enable_sata_clock(); + if (!is_cpu_type(MXC_CPU_MX6Q) && !is_cpu_type(MXC_CPU_MX6D)) + return 1; + + ret = enable_sata_clock(); if (ret) return ret;

On Wed, May 7, 2014 at 10:24 PM, Tim Harvey tharvey@gateworks.com wrote:
The IMX6QUAD/DUAL have SATA, but the IMX6SOLO/DL do not. Return instead of configuring the SATA clock and GPR13 registers.
Signed-off-by: Tim Harvey tharvey@gateworks.com
arch/arm/imx-common/sata.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/arm/imx-common/sata.c b/arch/arm/imx-common/sata.c index 2e69486..c10dd28 100644 --- a/arch/arm/imx-common/sata.c +++ b/arch/arm/imx-common/sata.c @@ -8,13 +8,18 @@ #include <asm/arch/iomux.h> #include <asm/io.h> #include <asm/arch/clock.h> +#include <asm/arch/sys_proto.h>
int setup_sata(void) { struct iomuxc_base_regs *const iomuxc_regs = (struct iomuxc_base_regs *)IOMUXC_BASE_ADDR;
int ret;
int ret = enable_sata_clock();
if (!is_cpu_type(MXC_CPU_MX6Q) && !is_cpu_type(MXC_CPU_MX6D))
return 1;
ret = enable_sata_clock(); if (ret) return ret;
-- 1.8.3.2
Stefano,
Any comments?
Regards,
Tim

Hi Tim,
On 05/06/2014 12:44, Tim Harvey wrote:
On Wed, May 7, 2014 at 10:24 PM, Tim Harvey tharvey@gateworks.com wrote:
The IMX6QUAD/DUAL have SATA, but the IMX6SOLO/DL do not. Return instead of configuring the SATA clock and GPR13 registers.
Signed-off-by: Tim Harvey tharvey@gateworks.com
arch/arm/imx-common/sata.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/arm/imx-common/sata.c b/arch/arm/imx-common/sata.c index 2e69486..c10dd28 100644 --- a/arch/arm/imx-common/sata.c +++ b/arch/arm/imx-common/sata.c @@ -8,13 +8,18 @@ #include <asm/arch/iomux.h> #include <asm/io.h> #include <asm/arch/clock.h> +#include <asm/arch/sys_proto.h>
int setup_sata(void) { struct iomuxc_base_regs *const iomuxc_regs = (struct iomuxc_base_regs *)IOMUXC_BASE_ADDR;
int ret;
int ret = enable_sata_clock();
if (!is_cpu_type(MXC_CPU_MX6Q) && !is_cpu_type(MXC_CPU_MX6D))
return 1;
ret = enable_sata_clock(); if (ret) return ret;
-- 1.8.3.2
Stefano,
Any comments?
No comments. In my queue it is ready-to-be-applied. I will do it after the whole SPL series. My plan is to apply them until the end of the week.
Best regards, Stefano Babic

Hi Tim,
On 05/06/2014 12:44, Tim Harvey wrote:
On Wed, May 7, 2014 at 10:24 PM, Tim Harvey tharvey@gateworks.com wrote:
The IMX6QUAD/DUAL have SATA, but the IMX6SOLO/DL do not. Return instead of configuring the SATA clock and GPR13 registers.
Signed-off-by: Tim Harvey tharvey@gateworks.com
arch/arm/imx-common/sata.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/arm/imx-common/sata.c b/arch/arm/imx-common/sata.c index 2e69486..c10dd28 100644 --- a/arch/arm/imx-common/sata.c +++ b/arch/arm/imx-common/sata.c @@ -8,13 +8,18 @@ #include <asm/arch/iomux.h> #include <asm/io.h> #include <asm/arch/clock.h> +#include <asm/arch/sys_proto.h>
int setup_sata(void) { struct iomuxc_base_regs *const iomuxc_regs = (struct iomuxc_base_regs *)IOMUXC_BASE_ADDR;
int ret;
int ret = enable_sata_clock();
if (!is_cpu_type(MXC_CPU_MX6Q) && !is_cpu_type(MXC_CPU_MX6D))
return 1;
ret = enable_sata_clock(); if (ret) return ret;
-- 1.8.3.2
Stefano,
Any comments?
Yes: applied to u-boot-imx, thanks !
Regards, Stefano
participants (3)
-
Stefano Babic
-
Tim Harvey
-
Troy Kisky