All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] (no subject)
       [not found] <http://lists.denx.de/pipermail/u-boot/2012-March/120899.html>
@ 2012-03-25 23:00 ` Eric Nelson
  2012-03-25 23:00   ` [U-Boot] [PATCH 1/2] i.MX6: add enable_sata_clock() Eric Nelson
                     ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Eric Nelson @ 2012-03-25 23:00 UTC (permalink / raw)
  To: u-boot

These two patches split the previous patch 
	http://lists.denx.de/pipermail/u-boot/2012-March/120150.html
into two parts.

The first is board-independent and will be useful on all i.MX6 boards.
The second is specific to the mx6qsabrelite board.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 1/2] i.MX6: add enable_sata_clock()
  2012-03-25 23:00 ` [U-Boot] (no subject) Eric Nelson
@ 2012-03-25 23:00   ` Eric Nelson
  2012-03-25 23:00   ` [U-Boot] [PATCH 2/2] i.MX6: mx6q_sabrelite: add SATA bindings Eric Nelson
  2012-03-25 23:10   ` [U-Boot] [PATCH V3 0/2] " Eric Nelson
  2 siblings, 0 replies; 9+ messages in thread
From: Eric Nelson @ 2012-03-25 23:00 UTC (permalink / raw)
  To: u-boot

Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
---
This patch requires Stefano's driver for MX5/MX6 to be useful.
	http://lists.denx.de/pipermail/u-boot/2012-February/118530.html

This is the first and board-independent part of what's needed to enable
SATA on an i.MX6 board as discussed in this thread:
	http://lists.denx.de/pipermail/u-boot/2012-March/120919.html

 arch/arm/cpu/armv7/mx6/clock.c           |   31 ++++++++
 arch/arm/include/asm/arch-mx6/clock.h    |    1 +
 arch/arm/include/asm/arch-mx6/imx-regs.h |    9 +++
 arch/arm/include/asm/arch-mx6/iomux-v3.h |  111 ++++++++++++++++++++++++++++++
 4 files changed, 152 insertions(+), 0 deletions(-)

diff --git a/arch/arm/cpu/armv7/mx6/clock.c b/arch/arm/cpu/armv7/mx6/clock.c
index ef98563..0ec4c15 100644
--- a/arch/arm/cpu/armv7/mx6/clock.c
+++ b/arch/arm/cpu/armv7/mx6/clock.c
@@ -303,6 +303,37 @@ u32 imx_get_fecclk(void)
 	return decode_pll(PLL_ENET, CONFIG_SYS_MX6_HCLK);
 }
 
+int enable_sata_clock(void)
+{
+	u32 reg = 0;
+	s32 timeout = 100000;
+	struct imx_ccm_reg *const imx_ccm
+		= (struct imx_ccm_reg *) CCM_BASE_ADDR;
+
+	/* Enable sata clock */
+	reg = readl(&imx_ccm->CCGR5); /* CCGR5 */
+	reg |= MXC_CCM_CCGR5_CG2_MASK;
+	writel(reg, &imx_ccm->CCGR5);
+
+	/* Enable PLLs */
+	reg = readl(&imx_ccm->analog_pll_enet);
+	reg &= ~BM_ANADIG_PLL_SYS_POWERDOWN;
+	writel(reg, &imx_ccm->analog_pll_enet);
+	reg |= BM_ANADIG_PLL_SYS_ENABLE;
+	while (timeout--) {
+		if (readl(&imx_ccm->analog_pll_enet) & BM_ANADIG_PLL_SYS_LOCK)
+			break;
+	}
+	if (timeout <= 0)
+		return -1;
+	reg &= ~BM_ANADIG_PLL_SYS_BYPASS;
+	writel(reg, &imx_ccm->analog_pll_enet);
+	reg |= BM_ANADIG_PLL_ENET_ENABLE_SATA;
+	writel(reg, &imx_ccm->analog_pll_enet);
+
+	return 0 ;
+}
+
 unsigned int mxc_get_clock(enum mxc_clock clk)
 {
 	switch (clk) {
diff --git a/arch/arm/include/asm/arch-mx6/clock.h b/arch/arm/include/asm/arch-mx6/clock.h
index 613809b..b91d8bf 100644
--- a/arch/arm/include/asm/arch-mx6/clock.h
+++ b/arch/arm/include/asm/arch-mx6/clock.h
@@ -47,5 +47,6 @@ u32 imx_get_uartclk(void);
 u32 imx_get_fecclk(void);
 unsigned int mxc_get_clock(enum mxc_clock clk);
 void enable_usboh3_clk(unsigned char enable);
+int enable_sata_clock(void);
 
 #endif /* __ASM_ARCH_CLOCK_H */
diff --git a/arch/arm/include/asm/arch-mx6/imx-regs.h b/arch/arm/include/asm/arch-mx6/imx-regs.h
index 1033d05..f72e753 100644
--- a/arch/arm/include/asm/arch-mx6/imx-regs.h
+++ b/arch/arm/include/asm/arch-mx6/imx-regs.h
@@ -438,5 +438,14 @@ struct anatop_regs {
 	u32	digprog;		/* 0x260 */
 };
 
+struct iomuxc_base_regs {
+	u32     gpr[14];        /* 0x000 */
+	u32     obsrv[5];       /* 0x038 */
+	u32     swmux_ctl[197]; /* 0x04c */
+	u32     swpad_ctl[250]; /* 0x360 */
+	u32     swgrp[26];      /* 0x748 */
+	u32     daisy[104];     /* 0x7b0..94c */
+};
+
 #endif /* __ASSEMBLER__*/
 #endif /* __ASM_ARCH_MX6_IMX_REGS_H__ */
diff --git a/arch/arm/include/asm/arch-mx6/iomux-v3.h b/arch/arm/include/asm/arch-mx6/iomux-v3.h
index 4558f4f..788b413 100644
--- a/arch/arm/include/asm/arch-mx6/iomux-v3.h
+++ b/arch/arm/include/asm/arch-mx6/iomux-v3.h
@@ -100,4 +100,115 @@ typedef u64 iomux_v3_cfg_t;
 int imx_iomux_v3_setup_pad(iomux_v3_cfg_t pad);
 int imx_iomux_v3_setup_multiple_pads(iomux_v3_cfg_t *pad_list, unsigned count);
 
+/*
+ * IOMUXC_GPR13 bit fields
+ */
+#define IOMUXC_GPR13_SDMA_STOP_REQ	(1<<30)
+#define IOMUXC_GPR13_CAN2_STOP_REQ	(1<<29)
+#define IOMUXC_GPR13_CAN1_STOP_REQ	(1<<28)
+#define IOMUXC_GPR13_ENET_STOP_REQ	(1<<27)
+#define IOMUXC_GPR13_SATA_PHY_8_MASK	(7<<24)
+#define IOMUXC_GPR13_SATA_PHY_7_MASK	(0x1f<<19)
+#define IOMUXC_GPR13_SATA_PHY_6_SHIFT	16
+#define IOMUXC_GPR13_SATA_PHY_6_MASK	(7<<IOMUXC_GPR13_SATA_PHY_6_SHIFT)
+#define IOMUXC_GPR13_SATA_SPEED_MASK	(1<<15)
+#define IOMUXC_GPR13_SATA_PHY_5_MASK	(1<<14)
+#define IOMUXC_GPR13_SATA_PHY_4_MASK	(7<<11)
+#define IOMUXC_GPR13_SATA_PHY_3_MASK	(0x1f<<7)
+#define IOMUXC_GPR13_SATA_PHY_2_MASK	(0x1f<<2)
+#define IOMUXC_GPR13_SATA_PHY_1_MASK	(3<<0)
+
+#define IOMUXC_GPR13_SATA_PHY_8_RXEQ_0P5DB	(0b000<<24)
+#define IOMUXC_GPR13_SATA_PHY_8_RXEQ_1P0DB	(0b001<<24)
+#define IOMUXC_GPR13_SATA_PHY_8_RXEQ_1P5DB	(0b010<<24)
+#define IOMUXC_GPR13_SATA_PHY_8_RXEQ_2P0DB	(0b011<<24)
+#define IOMUXC_GPR13_SATA_PHY_8_RXEQ_2P5DB	(0b100<<24)
+#define IOMUXC_GPR13_SATA_PHY_8_RXEQ_3P0DB	(0b101<<24)
+#define IOMUXC_GPR13_SATA_PHY_8_RXEQ_3P5DB	(0b110<<24)
+#define IOMUXC_GPR13_SATA_PHY_8_RXEQ_4P0DB	(0b111<<24)
+
+#define IOMUXC_GPR13_SATA_PHY_7_SATA1I	(0b10000<<19)
+#define IOMUXC_GPR13_SATA_PHY_7_SATA1M	(0b10000<<19)
+#define IOMUXC_GPR13_SATA_PHY_7_SATA1X	(0b11010<<19)
+#define IOMUXC_GPR13_SATA_PHY_7_SATA2I	(0b10010<<19)
+#define IOMUXC_GPR13_SATA_PHY_7_SATA2M	(0b10010<<19)
+#define IOMUXC_GPR13_SATA_PHY_7_SATA2X	(0b11010<<19)
+
+#define IOMUXC_GPR13_SATA_SPEED_1P5G	(0<<15)
+#define IOMUXC_GPR13_SATA_SPEED_3G	(1<<15)
+
+#define IOMUXC_GPR13_SATA_SATA_PHY_5_SS_DISABLED	(0<<14)
+#define IOMUXC_GPR13_SATA_SATA_PHY_5_SS_ENABLED		(1<<14)
+
+#define IOMUXC_GPR13_SATA_SATA_PHY_4_ATTEN_16_16	(0<<11)
+#define IOMUXC_GPR13_SATA_SATA_PHY_4_ATTEN_14_16	(1<<11)
+#define IOMUXC_GPR13_SATA_SATA_PHY_4_ATTEN_12_16	(2<<11)
+#define IOMUXC_GPR13_SATA_SATA_PHY_4_ATTEN_10_16	(3<<11)
+#define IOMUXC_GPR13_SATA_SATA_PHY_4_ATTEN_9_16		(4<<11)
+#define IOMUXC_GPR13_SATA_SATA_PHY_4_ATTEN_8_16		(5<<11)
+
+#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_0P00_DB	(0b0000<<7)
+#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_0P37_DB	(0b0001<<7)
+#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_0P74_DB	(0b0010<<7)
+#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_1P11_DB	(0b0011<<7)
+#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_1P48_DB	(0b0100<<7)
+#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_1P85_DB	(0b0101<<7)
+#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_2P22_DB	(0b0110<<7)
+#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_2P59_DB	(0b0111<<7)
+#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_2P96_DB	(0b1000<<7)
+#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_3P33_DB	(0b1001<<7)
+#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_3P70_DB	(0b1010<<7)
+#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_4P07_DB	(0b1011<<7)
+#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_4P44_DB	(0b1100<<7)
+#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_4P81_DB	(0b1101<<7)
+#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_5P28_DB	(0b1110<<7)
+#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_5P75_DB	(0b1111<<7)
+
+#define IOMUXC_GPR13_SATA_PHY_2_TX_0P937V	(0b00000<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_0P947V	(0b00001<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_0P957V	(0b00010<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_0P966V	(0b00011<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_0P976V	(0b00100<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_0P986V	(0b00101<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_0P996V	(0b00110<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P005V	(0b00111<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P015V	(0b01000<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P025V	(0b01001<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P035V	(0b01010<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P045V	(0b01011<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P054V	(0b01100<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P064V	(0b01101<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P074V	(0b01110<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P084V	(0b01111<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P094V	(0b10000<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P104V	(0b10001<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P113V	(0b10010<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P123V	(0b10011<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P133V	(0b10100<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P143V	(0b10101<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P152V	(0b10110<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P162V	(0b10111<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P172V	(0b11000<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P182V	(0b11001<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P191V	(0b11010<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P201V	(0b11011<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P211V	(0b11100<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P221V	(0b11101<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P230V	(0b11110<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P240V	(0b11111<<2)
+
+#define IOMUXC_GPR13_SATA_PHY_1_FAST	0
+#define IOMUXC_GPR13_SATA_PHY_1_MEDIUM	1
+#define IOMUXC_GPR13_SATA_PHY_1_SLOW	2
+
+#define IOMUXC_GPR13_SATA_MASK (IOMUXC_GPR13_SATA_PHY_8_MASK \
+				|IOMUXC_GPR13_SATA_PHY_7_MASK \
+				|IOMUXC_GPR13_SATA_PHY_6_MASK \
+				|IOMUXC_GPR13_SATA_SPEED_MASK \
+				|IOMUXC_GPR13_SATA_PHY_5_MASK \
+				|IOMUXC_GPR13_SATA_PHY_4_MASK \
+				|IOMUXC_GPR13_SATA_PHY_3_MASK \
+				|IOMUXC_GPR13_SATA_PHY_2_MASK \
+				|IOMUXC_GPR13_SATA_PHY_1_MASK)
+
 #endif	/* __MACH_IOMUX_V3_H__*/
-- 
1.7.9

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 2/2] i.MX6: mx6q_sabrelite: add SATA bindings
  2012-03-25 23:00 ` [U-Boot] (no subject) Eric Nelson
  2012-03-25 23:00   ` [U-Boot] [PATCH 1/2] i.MX6: add enable_sata_clock() Eric Nelson
@ 2012-03-25 23:00   ` Eric Nelson
  2012-03-26  8:35     ` Stefano Babic
  2012-03-25 23:10   ` [U-Boot] [PATCH V3 0/2] " Eric Nelson
  2 siblings, 1 reply; 9+ messages in thread
From: Eric Nelson @ 2012-03-25 23:00 UTC (permalink / raw)
  To: u-boot

V2 has been stripped of the board-independent changes and
uses clrsetbits_le32() instead of twiddling bits by hand.

Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
---
 board/freescale/mx6qsabrelite/mx6qsabrelite.c |   32 +++++++++++++++++++++++++
 include/configs/mx6qsabrelite.h               |   13 ++++++++++
 2 files changed, 45 insertions(+), 0 deletions(-)

diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
index 1d09a72..afb1245 100644
--- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
+++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
@@ -25,6 +25,8 @@
 #include <asm/arch/imx-regs.h>
 #include <asm/arch/mx6x_pins.h>
 #include <asm/arch/iomux-v3.h>
+#include <asm/arch/ccm_regs.h>
+#include <asm/arch/clock.h>
 #include <asm/errno.h>
 #include <asm/gpio.h>
 #include <mmc.h>
@@ -267,6 +269,32 @@ int board_eth_init(bd_t *bis)
 	return 0;
 }
 
+#ifdef CONFIG_CMD_SATA
+
+int setup_sata(void)
+{
+	int rval = enable_sata_clock();
+	if (rval == 0) {
+		struct iomuxc_base_regs *const iomuxc_regs
+			= (struct iomuxc_base_regs *) IOMUXC_BASE_ADDR;
+		clrsetbits_le32(&iomuxc_regs->gpr[13],
+				IOMUXC_GPR13_SATA_MASK,
+				IOMUXC_GPR13_SATA_PHY_8_RXEQ_3P0DB
+				|IOMUXC_GPR13_SATA_PHY_7_SATA2M
+				|IOMUXC_GPR13_SATA_SPEED_3G
+				|(3<<IOMUXC_GPR13_SATA_PHY_6_SHIFT)
+				|IOMUXC_GPR13_SATA_SATA_PHY_5_SS_DISABLED
+				|IOMUXC_GPR13_SATA_SATA_PHY_4_ATTEN_9_16
+				|IOMUXC_GPR13_SATA_PHY_3_TXBOOST_0P00_DB
+				|IOMUXC_GPR13_SATA_PHY_2_TX_1P104V
+				|IOMUXC_GPR13_SATA_PHY_1_SLOW);
+		rval = 0;
+	}
+
+	return rval;
+}
+#endif
+
 int board_early_init_f(void)
 {
        setup_iomux_uart();
@@ -283,6 +311,10 @@ int board_init(void)
 	setup_spi();
 #endif
 
+#ifdef CONFIG_CMD_SATA
+	setup_sata();
+#endif
+
        return 0;
 }
 
diff --git a/include/configs/mx6qsabrelite.h b/include/configs/mx6qsabrelite.h
index c851559..a3c97c6 100644
--- a/include/configs/mx6qsabrelite.h
+++ b/include/configs/mx6qsabrelite.h
@@ -71,6 +71,19 @@
 #define CONFIG_CMD_FAT
 #define CONFIG_DOS_PARTITION
 
+#define CONFIG_CMD_SATA
+/*
+ * SATA Configs
+ */
+#ifdef CONFIG_CMD_SATA
+#define CONFIG_DWC_AHSATA
+#define CONFIG_SYS_SATA_MAX_DEVICE	1
+#define CONFIG_DWC_AHSATA_PORT_ID	0
+#define CONFIG_DWC_AHSATA_BASE_ADDR	SATA_ARB_BASE_ADDR
+#define CONFIG_LBA48
+#define CONFIG_LIBATA
+#endif
+
 #define CONFIG_CMD_PING
 #define CONFIG_CMD_DHCP
 #define CONFIG_CMD_MII
-- 
1.7.9

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH V3 0/2] i.MX6: mx6q_sabrelite: add SATA bindings
  2012-03-25 23:00 ` [U-Boot] (no subject) Eric Nelson
  2012-03-25 23:00   ` [U-Boot] [PATCH 1/2] i.MX6: add enable_sata_clock() Eric Nelson
  2012-03-25 23:00   ` [U-Boot] [PATCH 2/2] i.MX6: mx6q_sabrelite: add SATA bindings Eric Nelson
@ 2012-03-25 23:10   ` Eric Nelson
  2 siblings, 0 replies; 9+ messages in thread
From: Eric Nelson @ 2012-03-25 23:10 UTC (permalink / raw)
  To: u-boot

On 03/25/2012 04:00 PM, Eric Nelson wrote:
> These two patches split the previous patch
> 	http://lists.denx.de/pipermail/u-boot/2012-March/120150.html
> into two parts.
>
> The first is board-independent and will be useful on all i.MX6 boards.
> The second is specific to the mx6qsabrelite board.
>

Sorry about the prior e-mail. It looks like I botched the header.

I'm still trying to figure out this 'git send-email' thing.

This should say that the following two patches are V3 of a patch
originally titled "i.MX6: mx6q_sabrelite: add SATA bindings", though
the title doesn't match because only the second is board-specific.

	http://lists.denx.de/pipermail/u-boot/2012-March#120150

Regards,


Eric

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 2/2] i.MX6: mx6q_sabrelite: add SATA bindings
  2012-03-25 23:00   ` [U-Boot] [PATCH 2/2] i.MX6: mx6q_sabrelite: add SATA bindings Eric Nelson
@ 2012-03-26  8:35     ` Stefano Babic
  2012-03-26 13:35       ` Eric Nelson
  0 siblings, 1 reply; 9+ messages in thread
From: Stefano Babic @ 2012-03-26  8:35 UTC (permalink / raw)
  To: u-boot

On 26/03/2012 01:00, Eric Nelson wrote:
> V2 has been stripped of the board-independent changes and
> uses clrsetbits_le32() instead of twiddling bits by hand.
> 
> Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
> ---

Hi Eric,

>  board/freescale/mx6qsabrelite/mx6qsabrelite.c |   32 +++++++++++++++++++++++++
>  include/configs/mx6qsabrelite.h               |   13 ++++++++++
>  2 files changed, 45 insertions(+), 0 deletions(-)
> 
> diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> index 1d09a72..afb1245 100644
> --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> @@ -25,6 +25,8 @@
>  #include <asm/arch/imx-regs.h>
>  #include <asm/arch/mx6x_pins.h>
>  #include <asm/arch/iomux-v3.h>
> +#include <asm/arch/ccm_regs.h>
> +#include <asm/arch/clock.h>
>  #include <asm/errno.h>
>  #include <asm/gpio.h>
>  #include <mmc.h>
> @@ -267,6 +269,32 @@ int board_eth_init(bd_t *bis)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_CMD_SATA
> +
> +int setup_sata(void)
> +{
> +	int rval = enable_sata_clock();

What about to return at this point if there is an error ?

Best regards,
Stefano Babic

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 2/2] i.MX6: mx6q_sabrelite: add SATA bindings
  2012-03-26  8:35     ` Stefano Babic
@ 2012-03-26 13:35       ` Eric Nelson
  2012-03-26 13:42         ` Stefano Babic
  0 siblings, 1 reply; 9+ messages in thread
From: Eric Nelson @ 2012-03-26 13:35 UTC (permalink / raw)
  To: u-boot

On 03/26/2012 01:35 AM, Stefano Babic wrote:
> On 26/03/2012 01:00, Eric Nelson wrote:
>> V2 has been stripped of the board-independent changes and
>> uses clrsetbits_le32() instead of twiddling bits by hand.
>>
>> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com>
>> ---
>
> Hi Eric,
>
>>   board/freescale/mx6qsabrelite/mx6qsabrelite.c |   32 +++++++++++++++++++++++++
>>   include/configs/mx6qsabrelite.h               |   13 ++++++++++
>>   2 files changed, 45 insertions(+), 0 deletions(-)
>>
>> diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>> index 1d09a72..afb1245 100644
>> --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>> +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>> @@ -25,6 +25,8 @@
>>   #include<asm/arch/imx-regs.h>
>>   #include<asm/arch/mx6x_pins.h>
>>   #include<asm/arch/iomux-v3.h>
>> +#include<asm/arch/ccm_regs.h>
>> +#include<asm/arch/clock.h>
>>   #include<asm/errno.h>
>>   #include<asm/gpio.h>
>>   #include<mmc.h>
>> @@ -267,6 +269,32 @@ int board_eth_init(bd_t *bis)
>>   	return 0;
>>   }
>>
>> +#ifdef CONFIG_CMD_SATA
>> +
>> +int setup_sata(void)
>> +{
>> +	int rval = enable_sata_clock();
>
> What about to return at this point if there is an error ?
>

I'm not sure I understand. Do you mean re-structure the code with
two returns like this?

diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c 
b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
index afb1245..0d625af 100644
--- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
+++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
@@ -273,23 +273,23 @@ int board_eth_init(bd_t *bis)

  int setup_sata(void)
  {
+       struct iomuxc_base_regs *const iomuxc_regs
+               = (struct iomuxc_base_regs *) IOMUXC_BASE_ADDR;
         int rval = enable_sata_clock();
-       if (rval == 0) {
-               struct iomuxc_base_regs *const iomuxc_regs
-                       = (struct iomuxc_base_regs *) IOMUXC_BASE_ADDR;
-               clrsetbits_le32(&iomuxc_regs->gpr[13],
-                               IOMUXC_GPR13_SATA_MASK,
-                               IOMUXC_GPR13_SATA_PHY_8_RXEQ_3P0DB
-                               |IOMUXC_GPR13_SATA_PHY_7_SATA2M
-                               |IOMUXC_GPR13_SATA_SPEED_3G
-                               |(3<<IOMUXC_GPR13_SATA_PHY_6_SHIFT)
-                               |IOMUXC_GPR13_SATA_SATA_PHY_5_SS_DISABLED
-                               |IOMUXC_GPR13_SATA_SATA_PHY_4_ATTEN_9_16
-                               |IOMUXC_GPR13_SATA_PHY_3_TXBOOST_0P00_DB
-                               |IOMUXC_GPR13_SATA_PHY_2_TX_1P104V
-                               |IOMUXC_GPR13_SATA_PHY_1_SLOW);
-               rval = 0;
-       }
+       if (rval)
+               return rval ;
+
+       clrsetbits_le32(&iomuxc_regs->gpr[13],
+                       IOMUXC_GPR13_SATA_MASK,
+                       IOMUXC_GPR13_SATA_PHY_8_RXEQ_3P0DB
+                       |IOMUXC_GPR13_SATA_PHY_7_SATA2M
+                       |IOMUXC_GPR13_SATA_SPEED_3G
+                       |(3<<IOMUXC_GPR13_SATA_PHY_6_SHIFT)
+                       |IOMUXC_GPR13_SATA_SATA_PHY_5_SS_DISABLED
+                       |IOMUXC_GPR13_SATA_SATA_PHY_4_ATTEN_9_16
+                       |IOMUXC_GPR13_SATA_PHY_3_TXBOOST_0P00_DB
+                       |IOMUXC_GPR13_SATA_PHY_2_TX_1P104V
+                       |IOMUXC_GPR13_SATA_PHY_1_SLOW);

         return rval;
  }

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 2/2] i.MX6: mx6q_sabrelite: add SATA bindings
  2012-03-26 13:35       ` Eric Nelson
@ 2012-03-26 13:42         ` Stefano Babic
  2012-03-26 16:59           ` Eric Nelson
  0 siblings, 1 reply; 9+ messages in thread
From: Stefano Babic @ 2012-03-26 13:42 UTC (permalink / raw)
  To: u-boot

On 26/03/2012 15:35, Eric Nelson wrote:
> On 03/26/2012 01:35 AM, Stefano Babic wrote:
>> On 26/03/2012 01:00, Eric Nelson wrote:
>>> V2 has been stripped of the board-independent changes and
>>> uses clrsetbits_le32() instead of twiddling bits by hand.
>>>
>>> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com>
>>> ---
>>
>> Hi Eric,
>>
>>>   board/freescale/mx6qsabrelite/mx6qsabrelite.c |   32
>>> +++++++++++++++++++++++++
>>>   include/configs/mx6qsabrelite.h               |   13 ++++++++++
>>>   2 files changed, 45 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>>> b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>>> index 1d09a72..afb1245 100644
>>> --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>>> +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>>> @@ -25,6 +25,8 @@
>>>   #include<asm/arch/imx-regs.h>
>>>   #include<asm/arch/mx6x_pins.h>
>>>   #include<asm/arch/iomux-v3.h>
>>> +#include<asm/arch/ccm_regs.h>
>>> +#include<asm/arch/clock.h>
>>>   #include<asm/errno.h>
>>>   #include<asm/gpio.h>
>>>   #include<mmc.h>
>>> @@ -267,6 +269,32 @@ int board_eth_init(bd_t *bis)
>>>       return 0;
>>>   }
>>>
>>> +#ifdef CONFIG_CMD_SATA
>>> +
>>> +int setup_sata(void)
>>> +{
>>> +    int rval = enable_sata_clock();
>>
>> What about to return at this point if there is an error ?
>>
> 
> I'm not sure I understand. Do you mean re-structure the code with
> two returns like this?


No, much easier - I find the code is easy to understand if it looks like
if the function returns immediately in case of error.

	if (do_something())
		return ERROR;

	< code when no error happens>

Your  enable_sata_clock() return only -1 in case of error. You could
easy write:

	if (enable_sata_clock())
		return -1 (or better a value in errno.h)
        struct iomuxc_base_regs *const iomuxc_regs
                   = (struct iomuxc_base_regs *) IOMUXC_BASE_ADDR;
        clrsetbits_le32(&iomuxc_regs->gpr[13],
                       IOMUXC_GPR13_SATA_MASK,
                       IOMUXC_GPR13_SATA_PHY_8_RXEQ_3P0DB
                       |IOMUXC_GPR13_SATA_PHY_7_SATA2M
                       |IOMUXC_GPR13_SATA_SPEED_3G
                       |(3<<IOMUXC_GPR13_SATA_PHY_6_SHIFT)
                       |IOMUXC_GPR13_SATA_SATA_PHY_5_SS_DISABLED
                       |IOMUXC_GPR13_SATA_SATA_PHY_4_ATTEN_9_16
                       |IOMUXC_GPR13_SATA_PHY_3_TXBOOST_0P00_DB
                       |IOMUXC_GPR13_SATA_PHY_2_TX_1P104V
                       |IOMUXC_GPR13_SATA_PHY_1_SLOW);

	return 0;	


Best regards,
Stefano Babic

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 2/2] i.MX6: mx6q_sabrelite: add SATA bindings
  2012-03-26 13:42         ` Stefano Babic
@ 2012-03-26 16:59           ` Eric Nelson
  2012-04-25 23:12             ` Eric Nelson
  0 siblings, 1 reply; 9+ messages in thread
From: Eric Nelson @ 2012-03-26 16:59 UTC (permalink / raw)
  To: u-boot

On 03/26/2012 06:42 AM, Stefano Babic wrote:
> On 26/03/2012 15:35, Eric Nelson wrote:
>> On 03/26/2012 01:35 AM, Stefano Babic wrote:
>>> On 26/03/2012 01:00, Eric Nelson wrote:
>>>> V2 has been stripped of the board-independent changes and
>>>>
 >>>> <snip>
 >>>>
>>>> +#ifdef CONFIG_CMD_SATA
>>>> +
>>>> +int setup_sata(void)
>>>> +{
>>>> +    int rval = enable_sata_clock();
>>>
>>> What about to return at this point if there is an error ?
>>>
>> I'm not sure I understand. Do you mean re-structure the code with
>> two returns like this?
>
> No, much easier - I find the code is easy to understand if it looks like
> if the function returns immediately in case of error.
>
> 	if (do_something())
> 		return ERROR;
>
> 	<  code when no error happens>
>
> Your  enable_sata_clock() return only -1 in case of error. You could
> easy write:
>
> 	if (enable_sata_clock())
> 		return -1 (or better a value in errno.h)
 >
The choice of error code is better made inside enable_sata_clock(),
although I'm not really sure what error code to choose.

The error occurs if the PLL fails to lock and would indicate that
something's horribly wrong.

I'm guessing -EIO is probably the right choice.

If you agree, I'll send V2 of "i.MX6: add enable_sata_clock()" and
V3 of "i.MX6: mx6q_sabrelite: add SATA bindings".

Please advise,


Eric

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 2/2] i.MX6: mx6q_sabrelite: add SATA bindings
  2012-03-26 16:59           ` Eric Nelson
@ 2012-04-25 23:12             ` Eric Nelson
  0 siblings, 0 replies; 9+ messages in thread
From: Eric Nelson @ 2012-04-25 23:12 UTC (permalink / raw)
  To: u-boot

On 03/26/2012 09:59 AM, Eric Nelson wrote:
> On 03/26/2012 06:42 AM, Stefano Babic wrote:
>> On 26/03/2012 15:35, Eric Nelson wrote:
>>> On 03/26/2012 01:35 AM, Stefano Babic wrote:
>>>> On 26/03/2012 01:00, Eric Nelson wrote:
>>>>> V2 has been stripped of the board-independent changes and
>>>>>
>  >>>> <snip>
>  >>>>
>>>>> +#ifdef CONFIG_CMD_SATA
>>>>> +
>>>>> +int setup_sata(void)
>>>>> +{
>>>>> + int rval = enable_sata_clock();
>>>>
>>>> What about to return at this point if there is an error ?
>>>>
>>> I'm not sure I understand. Do you mean re-structure the code with
>>> two returns like this?
>>
>> No, much easier - I find the code is easy to understand if it looks like
>> if the function returns immediately in case of error.
>>
>> if (do_something())
>> return ERROR;
>>
>> < code when no error happens>
>>
>> Your enable_sata_clock() return only -1 in case of error. You could
>> easy write:
>>
>> if (enable_sata_clock())
>> return -1 (or better a value in errno.h)
>  >
> The choice of error code is better made inside enable_sata_clock(),
> although I'm not really sure what error code to choose.
>
> The error occurs if the PLL fails to lock and would indicate that
> something's horribly wrong.
>
> I'm guessing -EIO is probably the right choice.
>
> If you agree, I'll send V2 of "i.MX6: add enable_sata_clock()" and
> V3 of "i.MX6: mx6q_sabrelite: add SATA bindings".
>
Hi Stefano,

It looks like I was waiting for you and never did send V3 of this patch.

I'll forward one shortly.

Regards,


Eric

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2012-04-25 23:12 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <http://lists.denx.de/pipermail/u-boot/2012-March/120899.html>
2012-03-25 23:00 ` [U-Boot] (no subject) Eric Nelson
2012-03-25 23:00   ` [U-Boot] [PATCH 1/2] i.MX6: add enable_sata_clock() Eric Nelson
2012-03-25 23:00   ` [U-Boot] [PATCH 2/2] i.MX6: mx6q_sabrelite: add SATA bindings Eric Nelson
2012-03-26  8:35     ` Stefano Babic
2012-03-26 13:35       ` Eric Nelson
2012-03-26 13:42         ` Stefano Babic
2012-03-26 16:59           ` Eric Nelson
2012-04-25 23:12             ` Eric Nelson
2012-03-25 23:10   ` [U-Boot] [PATCH V3 0/2] " Eric Nelson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.