arm: odroid: pmic77686: allow bucket voltage settings

PATCH Superseded Suriyan Ramasami 5 participants 15 messages Oct 20, 2014 – Oct 28, 2014

Subsystems (4)

Arm (32-bit) USB Arm Samsung Exynos USB ehci driver

lore

Share & tools

lore

Suriyan Ramasami

author

2014-10-20 17:52 UTC #0 lore raw

PATCH v2 1/3 arm: odroid: pmic77686: allow bucket voltage settings

Allow to set the bucket voltage for the max77686.
This will be used to reset the SMC LAN9730 ethernet on the odroids.

Signed-off-by Suriyan Ramasami <[email protected]>

---
 drivers/power/pmic/pmic_max77686.c | 48 +++++++++++++++++++++++++++++++++++++-
 include/power/max77686_pmic.h      |  3 +++
 2 files changed, 50 insertions(+), 1 deletion(-)

drivers/power/pmic/pmic_max77686.c +47 -1

index df1fd91..1aeadb4 100644
@@ -42,6 +42,25 @@ static unsigned int max77686_ldo_volt2hex(int ldo, ulong uV)
 	return 0;
 }
 
+static unsigned int max77686_buck_volt2hex(int buck, ulong uV)
+{
+	unsigned int hex = 0;
+
+	if (buck < 5 || buck > 9) {
+		debug("%s: buck %d is not supported\n", __func__, buck);
+		return 0;
+	}
+
+	hex = (uV - 750000) / 50000;
+
+	if (hex >= 0 && hex <= MAX77686_BUCK_VOLT_MAX_HEX)
+		return hex;
+
+	debug("%s: %ld is wrong voltage value for BUCK%d\n",
+	      __func__, uV, buck);
+	return MAX77686_BUCK_VOLT_MAX_HEX + 1;
+}
+
 int max77686_set_ldo_voltage(struct pmic *p, int ldo, ulong uV)
 {
 	unsigned int val, ret, hex, adr;

@@ -68,6 +87,33 @@ int max77686_set_ldo_voltage(struct pmic *p, int ldo, ulong uV)
 	return ret;
 }
 
+int max77686_set_buck_voltage(struct pmic *p, int buck, ulong uV)
+{
+	unsigned int val, ret, hex, adr;
+
+	if (buck < 5 && buck > 9) {
+		printf("%s: %d is an unsupported bucket number\n",
+		       __func__, buck);
+		return -1;
+	}
+
+	adr = max77686_buck_addr[buck] + 1;
+	hex = max77686_buck_volt2hex(buck, uV);
+
+	if (hex == MAX77686_BUCK_VOLT_MAX_HEX + 1)
+		return -1;
+
+	ret = pmic_reg_read(p, adr, &val);
+	if (ret)
+		return ret;
+
+	val &= ~MAX77686_BUCK_VOLT_MASK;
+	val |= hex;
+	ret |= pmic_reg_write(p, adr, val);
+
+	return ret;
+}
+
 int max77686_set_ldo_mode(struct pmic *p, int ldo, char opmode)
 {
 	unsigned int val, ret, adr, mode;

@@ -157,7 +203,7 @@ int max77686_set_buck_mode(struct pmic *p, int buck, char opmode)
 	/* mode */
 	switch (opmode) {
 	case OPMODE_OFF:
-		mode = MAX77686_BUCK_MODE_OFF;
+		mode = MAX77686_BUCK_MODE_OFF << mode_shift;
 		break;
 	case OPMODE_STANDBY:
 		switch (buck) {

include/power/max77686_pmic.h +3 -0

index c2a772a..b0e4255 100644
@@ -150,6 +150,7 @@ enum {
 
 int max77686_set_ldo_voltage(struct pmic *p, int ldo, ulong uV);
 int max77686_set_ldo_mode(struct pmic *p, int ldo, char opmode);
+int max77686_set_buck_voltage(struct pmic *p, int buck, ulong uV);
 int max77686_set_buck_mode(struct pmic *p, int buck, char opmode);
 
 #define MAX77686_LDO_VOLT_MAX_HEX	0x3f

@@ -159,6 +160,8 @@ int max77686_set_buck_mode(struct pmic *p, int buck, char opmode);
 #define MAX77686_LDO_MODE_STANDBY	(0x01 << 0x06)
 #define MAX77686_LDO_MODE_LPM		(0x02 << 0x06)
 #define MAX77686_LDO_MODE_ON		(0x03 << 0x06)
+#define MAX77686_BUCK_VOLT_MAX_HEX	0x3f
+#define MAX77686_BUCK_VOLT_MASK		0x3f
 #define MAX77686_BUCK_MODE_MASK		0x03
 #define MAX77686_BUCK_MODE_SHIFT_1	0x00
 #define MAX77686_BUCK_MODE_SHIFT_2	0x04

Signature · 1 line

1.9.1

Suriyan Ramasami

author

2014-10-20 17:52 UTC #1 lore raw

PATCH v2 2/3 arm: odroid: enable/disable usb host phy for exynos4412

Enable/disable the usb host phy on the odroid U/X2 boards which are based on the Exynos4412 SOC.

Signed-off-by Suriyan Ramasami <[email protected]>

---
 arch/arm/cpu/armv7/exynos/power.c        | 26 ++++++++++++++++++++++++++
 arch/arm/include/asm/arch-exynos/power.h |  7 +++++++
 2 files changed, 33 insertions(+)

arch/arm/cpu/armv7/exynos/power.c +26 -0

index e1ab3d6..6578a07 100644
@@ -53,10 +53,36 @@ void exynos5_set_usbhost_phy_ctrl(unsigned int enable)
 	}
 }
 
+void exynos4412_set_usbhost_phy_ctrl(unsigned int enable)
+{
+	struct exynos4412_power *power =
+		(struct exynos4412_power *)samsung_get_base_power();
+
+	if (enable) {
+		/* Enabling USBHOST_PHY */
+		setbits_le32(&power->usbhost_phy_control,
+			     POWER_USB_HOST_PHY_CTRL_EN);
+		setbits_le32(&power->hsic1_phy_control,
+			     POWER_USB_HOST_PHY_CTRL_EN);
+		setbits_le32(&power->hsic2_phy_control,
+			     POWER_USB_HOST_PHY_CTRL_EN);
+	} else {
+		/* Disabling USBHOST_PHY */
+		clrbits_le32(&power->usbhost_phy_control,
+			     POWER_USB_HOST_PHY_CTRL_EN);
+		clrbits_le32(&power->hsic1_phy_control,
+			     POWER_USB_HOST_PHY_CTRL_EN);
+		clrbits_le32(&power->hsic2_phy_control,
+			     POWER_USB_HOST_PHY_CTRL_EN);
+	}
+}
+
 void set_usbhost_phy_ctrl(unsigned int enable)
 {
 	if (cpu_is_exynos5())
 		exynos5_set_usbhost_phy_ctrl(enable);
+	else if (proid_is_exynos4412())
+		exynos4412_set_usbhost_phy_ctrl(enable);
 }
 
 static void exynos5_set_usbdrd_phy_ctrl(unsigned int enable)

arch/arm/include/asm/arch-exynos/power.h +7 -0

index e8a98a5..3f97b31 100644
@@ -210,6 +210,13 @@ struct exynos4_power {
 	unsigned int	gps_alive_option;
 };
 
+struct exynos4412_power {
+	unsigned char	res1[0x0704];
+	unsigned int	usbhost_phy_control;
+	unsigned int	hsic1_phy_control;
+	unsigned int	hsic2_phy_control;
+};
+
 struct exynos5_power {
 	unsigned int	om_stat;
 	unsigned char	res1[0x18];

Signature · 1 line

1.9.1

Suriyan Ramasami

author

2014-10-20 17:52 UTC #2 lore raw

PATCH v2 3/3 arm: odroid: usb: add support for usb host including ethernet

In reply to Suriyan Ramasami

This change adds support for enabling the USB host features of the board. This includes the USB3503A hub and the SMC LAN9730 ethernet controller as well.

Credit goes to Tushar Berara for the function set_usb_ethaddr().

Signed-off-by Suriyan Ramasami <[email protected]>

---
v2:
* Removed an unneeded header file from ehci-exynos.c
* Fix indentation in the dts file

---
 arch/arm/dts/exynos4412-odroid.dts      | 11 +++++++
 arch/arm/include/asm/arch-exynos/cpu.h  |  2 ++
 arch/arm/include/asm/arch-exynos/ehci.h | 13 ++++++++
 board/samsung/odroid/odroid.c           | 55 +++++++++++++++++++++++++++++++++
 drivers/usb/host/ehci-exynos.c          | 51 +++++++++++++++++++++++++-----
 include/configs/odroid.h                | 13 ++++++++
 6 files changed, 137 insertions(+), 8 deletions(-)

arch/arm/dts/exynos4412-odroid.dts +11 -0

index 24d0bf1..ac5fece 100644
@@ -67,4 +67,15 @@
 		div = <0x3>;
 		index = <4>;
 	};
+
+	ehci at 12580000 {
+		compatible = "samsung,exynos-ehci";
+		reg = <0x12580000 0x100>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		phy {
+			compatible = "samsung,exynos-usb-phy";
+			reg = <0x125B0000 0x100>;
+		};
+	};
 };

arch/arm/include/asm/arch-exynos/cpu.h +2 -0

index ba71714..fda21fb 100644
@@ -18,6 +18,8 @@
 
 #define EXYNOS4_GPIO_PART3_BASE		0x03860000
 #define EXYNOS4_PRO_ID			0x10000000
+#define EXYNOS4_GUID_LOW		0x10000014
+#define EXYNOS4_GUID_HIGH		0x10000018
 #define EXYNOS4_SYSREG_BASE		0x10010000
 #define EXYNOS4_POWER_BASE		0x10020000
 #define EXYNOS4_SWRESET			0x10020400

arch/arm/include/asm/arch-exynos/ehci.h +13 -0

index d2d70bd..3800fa9 100644
@@ -12,6 +12,13 @@
 
 #define CLK_24MHZ		5
 
+#define PHYPWR_NORMAL_MASK_PHY0                 (0x39 << 0)
+#define PHYPWR_NORMAL_MASK_PHY1                 (0x7 << 6)
+#define PHYPWR_NORMAL_MASK_HSIC0                (0x7 << 9)
+#define PHYPWR_NORMAL_MASK_HSIC1                (0x7 << 12)
+#define RSTCON_HOSTPHY_SWRST                    (0xf << 3)
+#define RSTCON_SWRST                            (0x1 << 0)
+
 #define HOST_CTRL0_PHYSWRSTALL			(1 << 31)
 #define HOST_CTRL0_COMMONON_N			(1 << 9)
 #define HOST_CTRL0_SIDDQ			(1 << 6)

@@ -61,6 +68,12 @@ struct exynos_usb_phy {
 	unsigned int usbotgtune;
 };
 
+struct exynos4412_usb_phy {
+	unsigned int usbphyctrl;
+	unsigned int usbphyclk;
+	unsigned int usbphyrstcon;
+};
+
 /* Switch on the VBUS power. */
 int board_usb_vbus_init(void);

board/samsung/odroid/odroid.c +55 -0

64 lines not on this page · raw message

drivers/usb/host/ehci-exynos.c +43 -8

72 lines not on this page · raw message

include/configs/odroid.h +13 -0

index b928af8..807e96b 100644
@@ -198,6 +198,19 @@
 
 #define CONFIG_CMD_GPIO
 
+/* USB */
+#define CONFIG_CMD_USB
+#define CONFIG_USB_EHCI
+#define CONFIG_USB_EHCI_EXYNOS
+#define CONFIG_USB_STORAGE
+
+#define CONFIG_SYS_USB_EHCI_MAX_ROOT_PORTS	3
+#define CONFIG_CMD_NET
+#define CONFIG_CMD_PING
+#define CONFIG_CMD_DHCP
+#define CONFIG_USB_HOST_ETHER
+#define CONFIG_USB_ETHER_SMSC95XX
+
 /*
  * Supported Odroid boards: X3, U3
  * TODO: Add Odroid X support

Signature · 1 line

1.9.1

Suriyan Ramasami

author

2014-10-24 16:08 UTC #4 lore raw

PATCH v2 3/3 arm: odroid: usb: add support for usb host including ethernet

Hello Minkyu Kang,

Quoted 103 lines · Minkyu Kang

Dear Suriyan Ramasami,

Quoted 100 lines · Suriyan Ramasami

This change adds support for enabling the USB host features of the board. This includes the USB3503A hub and the SMC LAN9730 ethernet controller as well.

Credit goes to Tushar Berara for the function set_usb_ethaddr().

Signed-off-by Suriyan Ramasami <[email protected]>

---
v2:
* Removed an unneeded header file from ehci-exynos.c
* Fix indentation in the dts file

---
 arch/arm/dts/exynos4412-odroid.dts      | 11 +++++++
 arch/arm/include/asm/arch-exynos/cpu.h  |  2 ++
 arch/arm/include/asm/arch-exynos/ehci.h | 13 ++++++++
 board/samsung/odroid/odroid.c           | 55 +++++++++++++++++++++++++++++++++
 drivers/usb/host/ehci-exynos.c          | 51 +++++++++++++++++++++++++-----
 include/configs/odroid.h                | 13 ++++++++
 6 files changed, 137 insertions(+), 8 deletions(-)

arch/arm/dts/exynos4412-odroid.dts +11 -0

index 24d0bf1..ac5fece 100644
@@ -67,4 +67,15 @@
              div = <0x3>;
              index = <4>;
      };
+
+     ehci at 12580000 {
+             compatible = "samsung,exynos-ehci";
+             reg = <0x12580000 0x100>;
+             #address-cells = <1>;
+             #size-cells = <1>;
+             phy {
+                     compatible = "samsung,exynos-usb-phy";
+                     reg = <0x125B0000 0x100>;
+             };
+     };
 };

arch/arm/include/asm/arch-exynos/cpu.h +2 -0

index ba71714..fda21fb 100644
@@ -18,6 +18,8 @@
 
 #define EXYNOS4_GPIO_PART3_BASE              0x03860000
 #define EXYNOS4_PRO_ID                       0x10000000
+#define EXYNOS4_GUID_LOW             0x10000014
+#define EXYNOS4_GUID_HIGH            0x10000018
 #define EXYNOS4_SYSREG_BASE          0x10010000
 #define EXYNOS4_POWER_BASE           0x10020000
 #define EXYNOS4_SWRESET                      0x10020400

arch/arm/include/asm/arch-exynos/ehci.h +13 -0

index d2d70bd..3800fa9 100644
@@ -12,6 +12,13 @@
 
 #define CLK_24MHZ            5
 
+#define PHYPWR_NORMAL_MASK_PHY0                 (0x39 << 0)
+#define PHYPWR_NORMAL_MASK_PHY1                 (0x7 << 6)
+#define PHYPWR_NORMAL_MASK_HSIC0                (0x7 << 9)
+#define PHYPWR_NORMAL_MASK_HSIC1                (0x7 << 12)
+#define RSTCON_HOSTPHY_SWRST                    (0xf << 3)
+#define RSTCON_SWRST                            (0x1 << 0)
+
 #define HOST_CTRL0_PHYSWRSTALL                       (1 << 31)
 #define HOST_CTRL0_COMMONON_N                        (1 << 9)
 #define HOST_CTRL0_SIDDQ                     (1 << 6)

@@ -61,6 +68,12 @@ struct exynos_usb_phy {
      unsigned int usbotgtune;
 };
 
+struct exynos4412_usb_phy {
+     unsigned int usbphyctrl;
+     unsigned int usbphyclk;
+     unsigned int usbphyrstcon;
+};
+
 /* Switch on the VBUS power. */
 int board_usb_vbus_init(void);

board/samsung/odroid/odroid.c +7 -0

index 5edb250..6c78b67 100644
@@ -453,9 +453,64 @@ struct s3c_plat_otg_data s5pc210_otg_data = {
      .usb_phy_ctrl   = EXYNOS4X12_USBPHY_CONTROL,
      .usb_flags      = PHY0_SLEEP,
 };
+#endif
+
+#if defined(CONFIG_USB_GADGET) || defined(CONFIG_CMD_USB)
+
+#ifdef CONFIG_CMD_USB
+static void set_usb_ethaddr(void)
+{

Quoted 9 lines

board/samsung/odroid/odroid.c struct s3c_plat_otg_data s5pc210_otg_data = { · line 466
Quoted 4 lines
board/samsung/odroid/odroid.c +4 -0
+     int i;
+     uchar mac[6];
+     unsigned int guid_high = readl(EXYNOS4_GUID_HIGH);
+     unsigned int guid_low = readl(EXYNOS4_GUID_LOW);
We don't allow direct access.
Is it special register? I can't find this register on TRM. If so you can make inline function at cpu.h instead.

This register is not in the TRM. This register is possibly documented in the TRM for the Exynos5250. Through experimentation I found that it behaves the same on Exynos4412 prime as well - I checked this with one X2, 1 U2 and 2 U3s, and they do indeed do the job of being unique and hence can be used to generate the mac address which will be unique across all U2s/U3s/X2s and possibly other Exynos SoCs like Exynos4212 etc.

Regarding direct access, I am a bit confused. In odroid.c I see quite a many places which is doing a readl() of registers. Here we are readl(addr) into guid_* similarly, and cooking up a mac address in a local char array.

I fail to see your point. Can you please elaborate more, so I can comprehend?

Quoted 9 lines

board/samsung/odroid/odroid.c struct s3c_plat_otg_data s5pc210_otg_data = { · line 472
Quoted 6 lines
board/samsung/odroid/odroid.c +6 -0
+
+     for (i = 0; i < 2; i++)
+             mac[i] = (guid_high >> (8 * (1 - i))) & 0xFF;
+
+     for (i = 0; i < 4; i++)
+             mac[i+2] = (guid_low >> (8 * (3 - i))) & 0xFF;
please add space. i + 2

Shall do so.

Quoted 96 lines

Quoted 83 lines

board/samsung/odroid/odroid.c +38 -0

+
+     /* mark it as not multicast and outside official 80211 MAC namespace */
+     mac[0] = (mac[0] & ~0x1) | 0x2;
+
+     eth_setenv_enetaddr("ethaddr", mac);
+     eth_setenv_enetaddr("usbethaddr", mac);
+}
+#endif
 
 int board_usb_init(int index, enum usb_init_type init)
 {
+#ifdef CONFIG_CMD_USB
+     struct pmic *p_pmic;
+
+     /* Set Ref freq 0 => 24MHz, 1 => 26MHz*/
+     /* Odroid Us have it at 24MHz, Odroid Xs at 26MHz */
+     if (gd->board_type == ODROID_TYPE_U3)
+             gpio_direction_output(EXYNOS4X12_GPIO_X30, 0);
+     else
+             gpio_direction_output(EXYNOS4X12_GPIO_X30, 1);
+
+     /* Disconnect, Reset, Connect */
+     gpio_direction_output(EXYNOS4X12_GPIO_X34, 0);
+     gpio_direction_output(EXYNOS4X12_GPIO_X35, 0);
+     gpio_direction_output(EXYNOS4X12_GPIO_X35, 1);
+     gpio_direction_output(EXYNOS4X12_GPIO_X34, 1);
+
+     /* Power off and on BUCK8 for LAN9730 */
+     debug("LAN9730 - Turning power buck 8 OFF and ON.\n");
+
+     p_pmic = pmic_get("MAX77686_PMIC");
+     if (p_pmic && !pmic_probe(p_pmic)) {
+             max77686_set_buck_mode(p_pmic, 8, OPMODE_OFF);
+             max77686_set_buck_voltage(p_pmic, 8, 750000);
+             max77686_set_buck_voltage(p_pmic, 8, 3300000);
+             max77686_set_buck_mode(p_pmic, 8, OPMODE_ON);
+     }
+
+     set_usb_ethaddr();
+#endif
+
      debug("USB_udc_probe\n");
      return s3c_udc_probe(&s5pc210_otg_data);
 }

drivers/usb/host/ehci-exynos.c +13 -8

index edd91a8..9c479b1 100644
@@ -85,15 +85,10 @@ static int exynos_usb_parse_dt(const void *blob, struct exynos_ehci *exynos)
 }
 #endif
 
-/* Setup the EHCI host controller. */
-static void setup_usb_phy(struct exynos_usb_phy *usb)
+static void exynos5_setup_usb_phy(struct exynos_usb_phy *usb)
 {
      u32 hsic_ctrl;
 
-     set_usbhost_mode(USB20_PHY_CFG_HOST_LINK_EN);
-
-     set_usbhost_phy_ctrl(POWER_USB_HOST_PHY_CTRL_EN);
-
      clrbits_le32(&usb->usbphyctrl0,
                      HOST_CTRL0_FSEL_MASK |
                      HOST_CTRL0_COMMONON_N |

@@ -150,8 +145,32 @@ static void setup_usb_phy(struct exynos_usb_phy *usb)
                      EHCICTRL_ENAINCR16);
 }
 
-/* Reset the EHCI host controller. */
-static void reset_usb_phy(struct exynos_usb_phy *usb)
+static void exynos4412_setup_usb_phy(struct exynos4412_usb_phy *usb)
+{
+     writel(CLK_24MHZ, &usb->usbphyclk);
+
+     clrbits_le32(&usb->usbphyctrl, (PHYPWR_NORMAL_MASK_HSIC0 |
+             PHYPWR_NORMAL_MASK_HSIC1 | PHYPWR_NORMAL_MASK_PHY1 |
+             PHYPWR_NORMAL_MASK_PHY0));
+
+     setbits_le32(&usb->usbphyrstcon, (RSTCON_HOSTPHY_SWRST | RSTCON_SWRST));
+     udelay(10);
+     clrbits_le32(&usb->usbphyrstcon, (RSTCON_HOSTPHY_SWRST | RSTCON_SWRST));
+}

drivers/usb/host/ehci-exynos.c static void setup_usb_phy(struct exynos_usb_phy *usb) · line 169

Quoted 10 lines

drivers/usb/host/ehci-exynos.c +10 -0

+
+static void setup_usb_phy(struct exynos_usb_phy *usb)
+{
+     set_usbhost_mode(USB20_PHY_CFG_HOST_LINK_EN);
+
+     set_usbhost_phy_ctrl(POWER_USB_HOST_PHY_CTRL_EN);
+
+     if (cpu_is_exynos5())
+             exynos5_setup_usb_phy(usb);
+     else if (proid_is_exynos4412())

please don't mix cpu_is.. and proid_is...

Quoted 9 lines

it should be
if (cpu_is_exynos5()) {
}
else if (cpu_is_exynos4()) {
    if (proid_is_exynos4412()) {
    }
}

OK, I shall do so.

Quoted 20 lines

drivers/usb/host/ehci-exynos.c +13 -0

+             exynos4412_setup_usb_phy((struct exynos4412_usb_phy *)usb);
+}
+
+static void exynos5_reset_usb_phy(struct exynos_usb_phy *usb)
 {
      u32 hsic_ctrl;

@@ -171,6 +190,22 @@ static void reset_usb_phy(struct exynos_usb_phy *usb)
 
      setbits_le32(&usb->hsicphyctrl1, hsic_ctrl);
      setbits_le32(&usb->hsicphyctrl2, hsic_ctrl);
+}
+
+static void exynos4412_reset_usb_phy(struct exynos4412_usb_phy *usb)
+{
+     setbits_le32(&usb->usbphyctrl, (PHYPWR_NORMAL_MASK_HSIC0 |
+             PHYPWR_NORMAL_MASK_HSIC1 | PHYPWR_NORMAL_MASK_PHY1 |
+             PHYPWR_NORMAL_MASK_PHY0));
+}
+

Quoted 9 lines

drivers/usb/host/ehci-exynos.c static void reset_usb_phy(struct exynos_usb_phy *usb) · line 207
Quoted 6 lines
drivers/usb/host/ehci-exynos.c +6 -0
+/* Reset the EHCI host controller. */
+static void reset_usb_phy(struct exynos_usb_phy *usb)
+{
+     if (cpu_is_exynos5())
+             exynos5_reset_usb_phy(usb);
+     else if (proid_is_exynos4412())
ditto.

OK, shall change this as well.

Thanks for taking a look!
- Suriyan

Quoted 32 lines

Quoted 19 lines

drivers/usb/host/ehci-exynos.c +1 -0

+             exynos4412_reset_usb_phy((struct exynos4412_usb_phy *)usb);
 
      set_usbhost_phy_ctrl(POWER_USB_HOST_PHY_CTRL_DISABLE);
 }

include/configs/odroid.h +6 -0

index b928af8..807e96b 100644
@@ -198,6 +198,19 @@
 
 #define CONFIG_CMD_GPIO
 
+/* USB */
+#define CONFIG_CMD_USB
+#define CONFIG_USB_EHCI
+#define CONFIG_USB_EHCI_EXYNOS
+#define CONFIG_USB_STORAGE
+

include/configs/odroid.h line 216

Quoted 10 lines

include/configs/odroid.h +7 -0

+#define CONFIG_SYS_USB_EHCI_MAX_ROOT_PORTS   3
+#define CONFIG_CMD_NET
+#define CONFIG_CMD_PING
+#define CONFIG_CMD_DHCP
+#define CONFIG_USB_HOST_ETHER
+#define CONFIG_USB_ETHER_SMSC95XX
+
 /*
  * Supported Odroid boards: X3, U3
  * TODO: Add Odroid X support

Thanks,
Minkyu Kang.

Przemyslaw Marczak

2014-10-27 11:08 UTC #5 lore raw

PATCH v2 3/3 arm: odroid: usb: add support for usb host including ethernet

Hello Suriyan,

Quoted 117 lines · Suriyan Ramasami

Hello Minkyu Kang,

Quoted 103 lines · Minkyu Kang

Dear Suriyan Ramasami,

Quoted 100 lines · Suriyan Ramasami

This change adds support for enabling the USB host features of the board. This includes the USB3503A hub and the SMC LAN9730 ethernet controller as well.

Credit goes to Tushar Berara for the function set_usb_ethaddr().

Signed-off-by Suriyan Ramasami <[email protected]>

---
v2:
* Removed an unneeded header file from ehci-exynos.c
* Fix indentation in the dts file

---
  arch/arm/dts/exynos4412-odroid.dts      | 11 +++++++
  arch/arm/include/asm/arch-exynos/cpu.h  |  2 ++
  arch/arm/include/asm/arch-exynos/ehci.h | 13 ++++++++
  board/samsung/odroid/odroid.c           | 55 +++++++++++++++++++++++++++++++++
  drivers/usb/host/ehci-exynos.c          | 51 +++++++++++++++++++++++++-----
  include/configs/odroid.h                | 13 ++++++++
  6 files changed, 137 insertions(+), 8 deletions(-)

arch/arm/dts/exynos4412-odroid.dts +11 -0

index 24d0bf1..ac5fece 100644
@@ -67,4 +67,15 @@
               div = <0x3>;
               index = <4>;
       };
+
+     ehci at 12580000 {
+             compatible = "samsung,exynos-ehci";
+             reg = <0x12580000 0x100>;
+             #address-cells = <1>;
+             #size-cells = <1>;
+             phy {
+                     compatible = "samsung,exynos-usb-phy";
+                     reg = <0x125B0000 0x100>;
+             };
+     };
  };

arch/arm/include/asm/arch-exynos/cpu.h +2 -0

index ba71714..fda21fb 100644
@@ -18,6 +18,8 @@
 
  #define EXYNOS4_GPIO_PART3_BASE              0x03860000
  #define EXYNOS4_PRO_ID                       0x10000000
+#define EXYNOS4_GUID_LOW             0x10000014
+#define EXYNOS4_GUID_HIGH            0x10000018
  #define EXYNOS4_SYSREG_BASE          0x10010000
  #define EXYNOS4_POWER_BASE           0x10020000
  #define EXYNOS4_SWRESET                      0x10020400

arch/arm/include/asm/arch-exynos/ehci.h +13 -0

index d2d70bd..3800fa9 100644
@@ -12,6 +12,13 @@
 
  #define CLK_24MHZ            5
 
+#define PHYPWR_NORMAL_MASK_PHY0                 (0x39 << 0)
+#define PHYPWR_NORMAL_MASK_PHY1                 (0x7 << 6)
+#define PHYPWR_NORMAL_MASK_HSIC0                (0x7 << 9)
+#define PHYPWR_NORMAL_MASK_HSIC1                (0x7 << 12)
+#define RSTCON_HOSTPHY_SWRST                    (0xf << 3)
+#define RSTCON_SWRST                            (0x1 << 0)
+
  #define HOST_CTRL0_PHYSWRSTALL                       (1 << 31)
  #define HOST_CTRL0_COMMONON_N                        (1 << 9)
  #define HOST_CTRL0_SIDDQ                     (1 << 6)

@@ -61,6 +68,12 @@ struct exynos_usb_phy {
       unsigned int usbotgtune;
  };
 
+struct exynos4412_usb_phy {
+     unsigned int usbphyctrl;
+     unsigned int usbphyclk;
+     unsigned int usbphyrstcon;
+};
+
  /* Switch on the VBUS power. */
  int board_usb_vbus_init(void);

board/samsung/odroid/odroid.c +7 -0

index 5edb250..6c78b67 100644
@@ -453,9 +453,64 @@ struct s3c_plat_otg_data s5pc210_otg_data = {
       .usb_phy_ctrl   = EXYNOS4X12_USBPHY_CONTROL,
       .usb_flags      = PHY0_SLEEP,
  };
+#endif
+
+#if defined(CONFIG_USB_GADGET) || defined(CONFIG_CMD_USB)
+
+#ifdef CONFIG_CMD_USB
+static void set_usb_ethaddr(void)
+{

Quoted 9 lines

board/samsung/odroid/odroid.c struct s3c_plat_otg_data s5pc210_otg_data = { · line 466
Quoted 4 lines
board/samsung/odroid/odroid.c +4 -0
+     int i;
+     uchar mac[6];
+     unsigned int guid_high = readl(EXYNOS4_GUID_HIGH);
+     unsigned int guid_low = readl(EXYNOS4_GUID_LOW);
We don't allow direct access.
Is it special register? I can't find this register on TRM. If so you can make inline function at cpu.h instead.

Quoted 8 lines

This register is not in the TRM. This register is possibly documented in the TRM for the Exynos5250. Through experimentation I found that it behaves the same on Exynos4412 prime as well - I checked this with one X2, 1 U2 and 2 U3s, and they do indeed do the job of being unique and hence can be used to generate the mac address which will be unique across all U2s/U3s/X2s and possibly other Exynos SoCs like Exynos4212 etc.

Those register addresses are not documented anywhere. So use of them is not a good idea and can cause unpredictable results, even if was tested on a few devices.

Quoted 7 lines

Regarding direct access, I am a bit confused. In odroid.c I see quite a many places which is doing a readl() of registers. Here we are readl(addr) into guid_* similarly, and cooking up a mac address in a local char array.
I fail to see your point. Can you please elaborate more, so I can comprehend?

The MAC address is defined by IEEE standard, so we can't use magic numbers from undefined registers.
I see two ways to do it:
- put the MAC into environment
- use of UUID generator from: lib/uuid.c

On the Odroid you will get the hardware acceleration, because the hardware random number generator is enabled.

Quoted 150 lines

Quoted 9 lines

board/samsung/odroid/odroid.c struct s3c_plat_otg_data s5pc210_otg_data = { · line 472
Quoted 6 lines
board/samsung/odroid/odroid.c +6 -0
+
+     for (i = 0; i < 2; i++)
+             mac[i] = (guid_high >> (8 * (1 - i))) & 0xFF;
+
+     for (i = 0; i < 4; i++)
+             mac[i+2] = (guid_low >> (8 * (3 - i))) & 0xFF;
please add space. i + 2

Shall do so.

Quoted 96 lines

Quoted 83 lines

board/samsung/odroid/odroid.c +38 -0

+
+     /* mark it as not multicast and outside official 80211 MAC namespace */
+     mac[0] = (mac[0] & ~0x1) | 0x2;
+
+     eth_setenv_enetaddr("ethaddr", mac);
+     eth_setenv_enetaddr("usbethaddr", mac);
+}
+#endif
 
  int board_usb_init(int index, enum usb_init_type init)
  {
+#ifdef CONFIG_CMD_USB
+     struct pmic *p_pmic;
+
+     /* Set Ref freq 0 => 24MHz, 1 => 26MHz*/
+     /* Odroid Us have it at 24MHz, Odroid Xs at 26MHz */
+     if (gd->board_type == ODROID_TYPE_U3)
+             gpio_direction_output(EXYNOS4X12_GPIO_X30, 0);
+     else
+             gpio_direction_output(EXYNOS4X12_GPIO_X30, 1);
+
+     /* Disconnect, Reset, Connect */
+     gpio_direction_output(EXYNOS4X12_GPIO_X34, 0);
+     gpio_direction_output(EXYNOS4X12_GPIO_X35, 0);
+     gpio_direction_output(EXYNOS4X12_GPIO_X35, 1);
+     gpio_direction_output(EXYNOS4X12_GPIO_X34, 1);
+
+     /* Power off and on BUCK8 for LAN9730 */
+     debug("LAN9730 - Turning power buck 8 OFF and ON.\n");
+
+     p_pmic = pmic_get("MAX77686_PMIC");
+     if (p_pmic && !pmic_probe(p_pmic)) {
+             max77686_set_buck_mode(p_pmic, 8, OPMODE_OFF);
+             max77686_set_buck_voltage(p_pmic, 8, 750000);
+             max77686_set_buck_voltage(p_pmic, 8, 3300000);
+             max77686_set_buck_mode(p_pmic, 8, OPMODE_ON);
+     }
+
+     set_usb_ethaddr();
+#endif
+
       debug("USB_udc_probe\n");
       return s3c_udc_probe(&s5pc210_otg_data);
  }

drivers/usb/host/ehci-exynos.c +13 -8

index edd91a8..9c479b1 100644
@@ -85,15 +85,10 @@ static int exynos_usb_parse_dt(const void *blob, struct exynos_ehci *exynos)
  }
  #endif
 
-/* Setup the EHCI host controller. */
-static void setup_usb_phy(struct exynos_usb_phy *usb)
+static void exynos5_setup_usb_phy(struct exynos_usb_phy *usb)
  {
       u32 hsic_ctrl;
 
-     set_usbhost_mode(USB20_PHY_CFG_HOST_LINK_EN);
-
-     set_usbhost_phy_ctrl(POWER_USB_HOST_PHY_CTRL_EN);
-
       clrbits_le32(&usb->usbphyctrl0,
                       HOST_CTRL0_FSEL_MASK |
                       HOST_CTRL0_COMMONON_N |

@@ -150,8 +145,32 @@ static void setup_usb_phy(struct exynos_usb_phy *usb)
                       EHCICTRL_ENAINCR16);
  }
 
-/* Reset the EHCI host controller. */
-static void reset_usb_phy(struct exynos_usb_phy *usb)
+static void exynos4412_setup_usb_phy(struct exynos4412_usb_phy *usb)
+{
+     writel(CLK_24MHZ, &usb->usbphyclk);
+
+     clrbits_le32(&usb->usbphyctrl, (PHYPWR_NORMAL_MASK_HSIC0 |
+             PHYPWR_NORMAL_MASK_HSIC1 | PHYPWR_NORMAL_MASK_PHY1 |
+             PHYPWR_NORMAL_MASK_PHY0));
+
+     setbits_le32(&usb->usbphyrstcon, (RSTCON_HOSTPHY_SWRST | RSTCON_SWRST));
+     udelay(10);
+     clrbits_le32(&usb->usbphyrstcon, (RSTCON_HOSTPHY_SWRST | RSTCON_SWRST));
+}

drivers/usb/host/ehci-exynos.c static void setup_usb_phy(struct exynos_usb_phy *usb) · line 169

Quoted 10 lines

drivers/usb/host/ehci-exynos.c +10 -0

+
+static void setup_usb_phy(struct exynos_usb_phy *usb)
+{
+     set_usbhost_mode(USB20_PHY_CFG_HOST_LINK_EN);
+
+     set_usbhost_phy_ctrl(POWER_USB_HOST_PHY_CTRL_EN);
+
+     if (cpu_is_exynos5())
+             exynos5_setup_usb_phy(usb);
+     else if (proid_is_exynos4412())

please don't mix cpu_is.. and proid_is...

Quoted 9 lines

it should be
if (cpu_is_exynos5()) {
}
else if (cpu_is_exynos4()) {
     if (proid_is_exynos4412()) {
     }
}

OK, I shall do so.

Quoted 20 lines

drivers/usb/host/ehci-exynos.c +13 -0

+             exynos4412_setup_usb_phy((struct exynos4412_usb_phy *)usb);
+}
+
+static void exynos5_reset_usb_phy(struct exynos_usb_phy *usb)
  {
       u32 hsic_ctrl;

@@ -171,6 +190,22 @@ static void reset_usb_phy(struct exynos_usb_phy *usb)
 
       setbits_le32(&usb->hsicphyctrl1, hsic_ctrl);
       setbits_le32(&usb->hsicphyctrl2, hsic_ctrl);
+}
+
+static void exynos4412_reset_usb_phy(struct exynos4412_usb_phy *usb)
+{
+     setbits_le32(&usb->usbphyctrl, (PHYPWR_NORMAL_MASK_HSIC0 |
+             PHYPWR_NORMAL_MASK_HSIC1 | PHYPWR_NORMAL_MASK_PHY1 |
+             PHYPWR_NORMAL_MASK_PHY0));
+}
+

Quoted 9 lines

drivers/usb/host/ehci-exynos.c static void reset_usb_phy(struct exynos_usb_phy *usb) · line 207
Quoted 6 lines
drivers/usb/host/ehci-exynos.c +6 -0
+/* Reset the EHCI host controller. */
+static void reset_usb_phy(struct exynos_usb_phy *usb)
+{
+     if (cpu_is_exynos5())
+             exynos5_reset_usb_phy(usb);
+     else if (proid_is_exynos4412())
ditto.

Quoted 37 lines

OK, shall change this as well.

Thanks for taking a look!
- Suriyan

Quoted 32 lines

Quoted 19 lines

drivers/usb/host/ehci-exynos.c +1 -0

+             exynos4412_reset_usb_phy((struct exynos4412_usb_phy *)usb);
 
       set_usbhost_phy_ctrl(POWER_USB_HOST_PHY_CTRL_DISABLE);
  }

include/configs/odroid.h +6 -0

index b928af8..807e96b 100644
@@ -198,6 +198,19 @@
 
  #define CONFIG_CMD_GPIO
 
+/* USB */
+#define CONFIG_CMD_USB
+#define CONFIG_USB_EHCI
+#define CONFIG_USB_EHCI_EXYNOS
+#define CONFIG_USB_STORAGE
+

include/configs/odroid.h line 216

Quoted 10 lines

include/configs/odroid.h +7 -0

+#define CONFIG_SYS_USB_EHCI_MAX_ROOT_PORTS   3
+#define CONFIG_CMD_NET
+#define CONFIG_CMD_PING
+#define CONFIG_CMD_DHCP
+#define CONFIG_USB_HOST_ETHER
+#define CONFIG_USB_ETHER_SMSC95XX
+
  /*
   * Supported Odroid boards: X3, U3
   * TODO: Add Odroid X support

Thanks,
Minkyu Kang.

Best regards,

Signature · 4 lines

Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com

Suriyan Ramasami

author

2014-10-27 19:34 UTC #6 lore raw

PATCH v2 3/3 arm: odroid: usb: add support for usb host including ethernet

Hello Przemyslaw Marczak,

Quoted 141 lines · Przemyslaw Marczak

Hello Suriyan,

Quoted 128 lines · Suriyan Ramasami

Hello Minkyu Kang,

Quoted 113 lines · Minkyu Kang

Dear Suriyan Ramasami,

Quoted 109 lines · Suriyan Ramasami

This change adds support for enabling the USB host features of the board.
This includes the USB3503A hub and the SMC LAN9730 ethernet controller as well.

Credit goes to Tushar Berara for the function set_usb_ethaddr().

Signed-off-by Suriyan Ramasami <[email protected]>

---
v2:
* Removed an unneeded header file from ehci-exynos.c
* Fix indentation in the dts file

---
  arch/arm/dts/exynos4412-odroid.dts      | 11 +++++++
  arch/arm/include/asm/arch-exynos/cpu.h  |  2 ++
  arch/arm/include/asm/arch-exynos/ehci.h | 13 ++++++++
  board/samsung/odroid/odroid.c           | 55
+++++++++++++++++++++++++++++++++
  drivers/usb/host/ehci-exynos.c          | 51
+++++++++++++++++++++++++-----
  include/configs/odroid.h                | 13 ++++++++
  6 files changed, 137 insertions(+), 8 deletions(-)

arch/arm/dts/exynos4412-odroid.dts +11 -0

index 24d0bf1..ac5fece 100644
@@ -67,4 +67,15 @@
               div = <0x3>;
               index = <4>;
       };
+
+     ehci at 12580000 {
+             compatible = "samsung,exynos-ehci";
+             reg = <0x12580000 0x100>;
+             #address-cells = <1>;
+             #size-cells = <1>;
+             phy {
+                     compatible = "samsung,exynos-usb-phy";
+                     reg = <0x125B0000 0x100>;
+             };
+     };
  };

arch/arm/include/asm/arch-exynos/cpu.h +2 -0

index ba71714..fda21fb 100644
@@ -18,6 +18,8 @@
 
  #define EXYNOS4_GPIO_PART3_BASE              0x03860000
  #define EXYNOS4_PRO_ID                       0x10000000
+#define EXYNOS4_GUID_LOW             0x10000014
+#define EXYNOS4_GUID_HIGH            0x10000018
  #define EXYNOS4_SYSREG_BASE          0x10010000
  #define EXYNOS4_POWER_BASE           0x10020000
  #define EXYNOS4_SWRESET                      0x10020400

arch/arm/include/asm/arch-exynos/ehci.h +13 -0

index d2d70bd..3800fa9 100644
@@ -12,6 +12,13 @@
 
  #define CLK_24MHZ            5
 
+#define PHYPWR_NORMAL_MASK_PHY0                 (0x39 << 0)
+#define PHYPWR_NORMAL_MASK_PHY1                 (0x7 << 6)
+#define PHYPWR_NORMAL_MASK_HSIC0                (0x7 << 9)
+#define PHYPWR_NORMAL_MASK_HSIC1                (0x7 << 12)
+#define RSTCON_HOSTPHY_SWRST                    (0xf << 3)
+#define RSTCON_SWRST                            (0x1 << 0)
+
  #define HOST_CTRL0_PHYSWRSTALL                       (1 << 31)
  #define HOST_CTRL0_COMMONON_N                        (1 << 9)
  #define HOST_CTRL0_SIDDQ                     (1 << 6)

@@ -61,6 +68,12 @@ struct exynos_usb_phy {
       unsigned int usbotgtune;
  };
 
+struct exynos4412_usb_phy {
+     unsigned int usbphyctrl;
+     unsigned int usbphyclk;
+     unsigned int usbphyrstcon;
+};
+
  /* Switch on the VBUS power. */
  int board_usb_vbus_init(void);

board/samsung/odroid/odroid.c +8 -0

index 5edb250..6c78b67 100644
@@ -453,9 +453,64 @@ struct s3c_plat_otg_data s5pc210_otg_data = {
       .usb_phy_ctrl   = EXYNOS4X12_USBPHY_CONTROL,
       .usb_flags      = PHY0_SLEEP,
  };
+#endif
+
+#if defined(CONFIG_USB_GADGET) || defined(CONFIG_CMD_USB)
+
+#ifdef CONFIG_CMD_USB
+static void set_usb_ethaddr(void)
+{
+     int i;

Quoted 9 lines

board/samsung/odroid/odroid.c struct s3c_plat_otg_data s5pc210_otg_data = { · line 466
Quoted 3 lines
board/samsung/odroid/odroid.c +3 -0
+     uchar mac[6];
+     unsigned int guid_high = readl(EXYNOS4_GUID_HIGH);
+     unsigned int guid_low = readl(EXYNOS4_GUID_LOW);
We don't allow direct access.
Is it special register? I can't find this register on TRM. If so you can make inline function at cpu.h instead.

Quoted 8 lines

This register is not in the TRM. This register is possibly documented in the TRM for the Exynos5250. Through experimentation I found that it behaves the same on Exynos4412 prime as well - I checked this with one X2, 1 U2 and 2 U3s, and they do indeed do the job of being unique and hence can be used to generate the mac address which will be unique across all U2s/U3s/X2s and possibly other Exynos SoCs like Exynos4212 etc.

Quoted 4 lines

Those register addresses are not documented anywhere. So use of them is not a good idea and can cause unpredictable results, even if was tested on a few devices.

OK, fair enough. I thought somehow it would be documented in the Samsung internal user manual. I guess it is not!

Quoted 11 lines

Quoted 8 lines
Regarding direct access, I am a bit confused. In odroid.c I see quite a many places which is doing a readl() of registers. Here we are readl(addr) into guid_* similarly, and cooking up a mac address in a local char array.
I fail to see your point. Can you please elaborate more, so I can comprehend?
The MAC address is defined by IEEE standard, so we can't use magic numbers from undefined registers.

I am aware of that. In the code by Tushar Behera you will see that he does mark it - "not multicast and outside official 80211 MAC namespace".

Quoted 7 lines

I see two ways to do it:
- put the MAC into environment
- use of UUID generator from: lib/uuid.c
On the Odroid you will get the hardware acceleration, because the hardware random number generator is enabled.

Let me research a bit - if a random MAC address can indeed be cooked up. If not, as you suggest, we can leave it as an onus on the user to set it in the env. (which is what is being done currently).

Do you have any pointers in using the hardware random generator in the Odroid? That does seem promising.

Quoted 192 lines

Quoted 11 lines

board/samsung/odroid/odroid.c struct s3c_plat_otg_data s5pc210_otg_data = { · line 472
Quoted 7 lines
board/samsung/odroid/odroid.c +6 -0
+
+     for (i = 0; i < 2; i++)
+             mac[i] = (guid_high >> (8 * (1 - i))) & 0xFF;
+
+     for (i = 0; i < 4; i++)
+             mac[i+2] = (guid_low >> (8 * (3 - i))) & 0xFF;
please add space. i + 2

Shall do so.

Quoted 103 lines

Quoted 89 lines

board/samsung/odroid/odroid.c +2 -0

+
+     /* mark it as not multicast and outside official 80211 MAC

namespace */

board/samsung/odroid/odroid.c +36 -0

+     mac[0] = (mac[0] & ~0x1) | 0x2;
+
+     eth_setenv_enetaddr("ethaddr", mac);
+     eth_setenv_enetaddr("usbethaddr", mac);
+}
+#endif
 
  int board_usb_init(int index, enum usb_init_type init)
  {
+#ifdef CONFIG_CMD_USB
+     struct pmic *p_pmic;
+
+     /* Set Ref freq 0 => 24MHz, 1 => 26MHz*/
+     /* Odroid Us have it at 24MHz, Odroid Xs at 26MHz */
+     if (gd->board_type == ODROID_TYPE_U3)
+             gpio_direction_output(EXYNOS4X12_GPIO_X30, 0);
+     else
+             gpio_direction_output(EXYNOS4X12_GPIO_X30, 1);
+
+     /* Disconnect, Reset, Connect */
+     gpio_direction_output(EXYNOS4X12_GPIO_X34, 0);
+     gpio_direction_output(EXYNOS4X12_GPIO_X35, 0);
+     gpio_direction_output(EXYNOS4X12_GPIO_X35, 1);
+     gpio_direction_output(EXYNOS4X12_GPIO_X34, 1);
+
+     /* Power off and on BUCK8 for LAN9730 */
+     debug("LAN9730 - Turning power buck 8 OFF and ON.\n");
+
+     p_pmic = pmic_get("MAX77686_PMIC");
+     if (p_pmic && !pmic_probe(p_pmic)) {
+             max77686_set_buck_mode(p_pmic, 8, OPMODE_OFF);
+             max77686_set_buck_voltage(p_pmic, 8, 750000);
+             max77686_set_buck_voltage(p_pmic, 8, 3300000);
+             max77686_set_buck_mode(p_pmic, 8, OPMODE_ON);
+     }
+
+     set_usb_ethaddr();
+#endif
+
       debug("USB_udc_probe\n");
       return s3c_udc_probe(&s5pc210_otg_data);
  }

drivers/usb/host/ehci-exynos.c +1 -6

index edd91a8..9c479b1 100644
@@ -85,15 +85,10 @@ static int exynos_usb_parse_dt(const void *blob,
 struct exynos_ehci *exynos)
  }
  #endif
 
-/* Setup the EHCI host controller. */
-static void setup_usb_phy(struct exynos_usb_phy *usb)
+static void exynos5_setup_usb_phy(struct exynos_usb_phy *usb)
  {
       u32 hsic_ctrl;
 
-     set_usbhost_mode(USB20_PHY_CFG_HOST_LINK_EN);
-
-     set_usbhost_phy_ctrl(POWER_USB_HOST_PHY_CTRL_EN);
-
       clrbits_le32(&usb->usbphyctrl0,
                       HOST_CTRL0_FSEL_MASK |

HOST_CTRL0_COMMONON_N |

drivers/usb/host/ehci-exynos.c +12 -2

@@ -150,8 +145,32 @@ static void setup_usb_phy(struct exynos_usb_phy
 *usb)
                       EHCICTRL_ENAINCR16);
  }
 
-/* Reset the EHCI host controller. */
-static void reset_usb_phy(struct exynos_usb_phy *usb)
+static void exynos4412_setup_usb_phy(struct exynos4412_usb_phy *usb)
+{
+     writel(CLK_24MHZ, &usb->usbphyclk);
+
+     clrbits_le32(&usb->usbphyctrl, (PHYPWR_NORMAL_MASK_HSIC0 |
+             PHYPWR_NORMAL_MASK_HSIC1 | PHYPWR_NORMAL_MASK_PHY1 |
+             PHYPWR_NORMAL_MASK_PHY0));
+
+     setbits_le32(&usb->usbphyrstcon, (RSTCON_HOSTPHY_SWRST |
 RSTCON_SWRST));
+     udelay(10);
+     clrbits_le32(&usb->usbphyrstcon, (RSTCON_HOSTPHY_SWRST |
 RSTCON_SWRST));
+}

drivers/usb/host/ehci-exynos.c static void setup_usb_phy(struct exynos_usb_phy · line 172

Quoted 10 lines

drivers/usb/host/ehci-exynos.c +10 -0

+
+static void setup_usb_phy(struct exynos_usb_phy *usb)
+{
+     set_usbhost_mode(USB20_PHY_CFG_HOST_LINK_EN);
+
+     set_usbhost_phy_ctrl(POWER_USB_HOST_PHY_CTRL_EN);
+
+     if (cpu_is_exynos5())
+             exynos5_setup_usb_phy(usb);
+     else if (proid_is_exynos4412())

please don't mix cpu_is.. and proid_is...

Quoted 9 lines

it should be
if (cpu_is_exynos5()) {
}
else if (cpu_is_exynos4()) {
     if (proid_is_exynos4412()) {
     }
}

OK, I shall do so.

Quoted 23 lines

drivers/usb/host/ehci-exynos.c +1 -0

+             exynos4412_setup_usb_phy((struct exynos4412_usb_phy

*)usb);

drivers/usb/host/ehci-exynos.c +13 -0

+}
+
+static void exynos5_reset_usb_phy(struct exynos_usb_phy *usb)
  {
       u32 hsic_ctrl;

@@ -171,6 +190,22 @@ static void reset_usb_phy(struct exynos_usb_phy
 *usb)
 
       setbits_le32(&usb->hsicphyctrl1, hsic_ctrl);
       setbits_le32(&usb->hsicphyctrl2, hsic_ctrl);
+}
+
+static void exynos4412_reset_usb_phy(struct exynos4412_usb_phy *usb)
+{
+     setbits_le32(&usb->usbphyctrl, (PHYPWR_NORMAL_MASK_HSIC0 |
+             PHYPWR_NORMAL_MASK_HSIC1 | PHYPWR_NORMAL_MASK_PHY1 |
+             PHYPWR_NORMAL_MASK_PHY0));
+}
+
+/* Reset the EHCI host controller. */

Quoted 9 lines

drivers/usb/host/ehci-exynos.c static void reset_usb_phy(struct exynos_usb_phy *usb) · line 207
Quoted 5 lines
drivers/usb/host/ehci-exynos.c +5 -0
+static void reset_usb_phy(struct exynos_usb_phy *usb)
+{
+     if (cpu_is_exynos5())
+             exynos5_reset_usb_phy(usb);
+     else if (proid_is_exynos4412())
ditto.

OK, shall change this as well.

Thanks for taking a look!
- Suriyan

Quoted 25 lines

drivers/usb/host/ehci-exynos.c +1 -0

+             exynos4412_reset_usb_phy((struct exynos4412_usb_phy
 *)usb);

       set_usbhost_phy_ctrl(POWER_USB_HOST_PHY_CTRL_DISABLE);

}

include/configs/odroid.h +12 -0

index b928af8..807e96b 100644
@@ -198,6 +198,19 @@
 
  #define CONFIG_CMD_GPIO
 
+/* USB */
+#define CONFIG_CMD_USB
+#define CONFIG_USB_EHCI
+#define CONFIG_USB_EHCI_EXYNOS
+#define CONFIG_USB_STORAGE
+
+#define CONFIG_SYS_USB_EHCI_MAX_ROOT_PORTS   3
+#define CONFIG_CMD_NET
+#define CONFIG_CMD_PING
+#define CONFIG_CMD_DHCP
+#define CONFIG_USB_HOST_ETHER
+#define CONFIG_USB_ETHER_SMSC95XX

Quoted 10 lines

Quoted 8 lines
Quoted 8 lines
include/configs/odroid.h line 216
Quoted 5 lines
include/configs/odroid.h +1 -0
+
  /*
   * Supported Odroid boards: X3, U3
   * TODO: Add Odroid X support
Thanks,
Minkyu Kang.

Thanks!
- Suriyan

Quoted 6 lines

Best regards,
--
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com

Przemyslaw Marczak

2014-10-28 07:47 UTC #7 lore raw

PATCH v2 3/3 arm: odroid: usb: add support for usb host including ethernet

Hello Suriyan Ramasami,

Quoted 145 lines · Suriyan Ramasami

Hello Przemyslaw Marczak,

Quoted 141 lines · Przemyslaw Marczak

Hello Suriyan,

Quoted 136 lines · Suriyan Ramasami

Hello Minkyu Kang,

Quoted 113 lines · Minkyu Kang

Dear Suriyan Ramasami,

Quoted 109 lines · Suriyan Ramasami

This change adds support for enabling the USB host features of the board.
This includes the USB3503A hub and the SMC LAN9730 ethernet controller as well.

Credit goes to Tushar Berara for the function set_usb_ethaddr().

Signed-off-by Suriyan Ramasami <[email protected]>

---
v2:
* Removed an unneeded header file from ehci-exynos.c
* Fix indentation in the dts file

---
   arch/arm/dts/exynos4412-odroid.dts      | 11 +++++++
   arch/arm/include/asm/arch-exynos/cpu.h  |  2 ++
   arch/arm/include/asm/arch-exynos/ehci.h | 13 ++++++++
   board/samsung/odroid/odroid.c           | 55
+++++++++++++++++++++++++++++++++
   drivers/usb/host/ehci-exynos.c          | 51
+++++++++++++++++++++++++-----
   include/configs/odroid.h                | 13 ++++++++
   6 files changed, 137 insertions(+), 8 deletions(-)

arch/arm/dts/exynos4412-odroid.dts +11 -0

index 24d0bf1..ac5fece 100644
@@ -67,4 +67,15 @@
                div = <0x3>;
                index = <4>;
        };
+
+     ehci at 12580000 {
+             compatible = "samsung,exynos-ehci";
+             reg = <0x12580000 0x100>;
+             #address-cells = <1>;
+             #size-cells = <1>;
+             phy {
+                     compatible = "samsung,exynos-usb-phy";
+                     reg = <0x125B0000 0x100>;
+             };
+     };
   };

arch/arm/include/asm/arch-exynos/cpu.h +2 -0

index ba71714..fda21fb 100644
@@ -18,6 +18,8 @@
 
   #define EXYNOS4_GPIO_PART3_BASE              0x03860000
   #define EXYNOS4_PRO_ID                       0x10000000
+#define EXYNOS4_GUID_LOW             0x10000014
+#define EXYNOS4_GUID_HIGH            0x10000018
   #define EXYNOS4_SYSREG_BASE          0x10010000
   #define EXYNOS4_POWER_BASE           0x10020000
   #define EXYNOS4_SWRESET                      0x10020400

arch/arm/include/asm/arch-exynos/ehci.h +13 -0

index d2d70bd..3800fa9 100644
@@ -12,6 +12,13 @@
 
   #define CLK_24MHZ            5
 
+#define PHYPWR_NORMAL_MASK_PHY0                 (0x39 << 0)
+#define PHYPWR_NORMAL_MASK_PHY1                 (0x7 << 6)
+#define PHYPWR_NORMAL_MASK_HSIC0                (0x7 << 9)
+#define PHYPWR_NORMAL_MASK_HSIC1                (0x7 << 12)
+#define RSTCON_HOSTPHY_SWRST                    (0xf << 3)
+#define RSTCON_SWRST                            (0x1 << 0)
+
   #define HOST_CTRL0_PHYSWRSTALL                       (1 << 31)
   #define HOST_CTRL0_COMMONON_N                        (1 << 9)
   #define HOST_CTRL0_SIDDQ                     (1 << 6)

@@ -61,6 +68,12 @@ struct exynos_usb_phy {
        unsigned int usbotgtune;
   };
 
+struct exynos4412_usb_phy {
+     unsigned int usbphyctrl;
+     unsigned int usbphyclk;
+     unsigned int usbphyrstcon;
+};
+
   /* Switch on the VBUS power. */
   int board_usb_vbus_init(void);

board/samsung/odroid/odroid.c +8 -0

index 5edb250..6c78b67 100644
@@ -453,9 +453,64 @@ struct s3c_plat_otg_data s5pc210_otg_data = {
        .usb_phy_ctrl   = EXYNOS4X12_USBPHY_CONTROL,
        .usb_flags      = PHY0_SLEEP,
   };
+#endif
+
+#if defined(CONFIG_USB_GADGET) || defined(CONFIG_CMD_USB)
+
+#ifdef CONFIG_CMD_USB
+static void set_usb_ethaddr(void)
+{
+     int i;

Quoted 9 lines

board/samsung/odroid/odroid.c struct s3c_plat_otg_data s5pc210_otg_data = { · line 466
Quoted 3 lines
board/samsung/odroid/odroid.c +3 -0
+     uchar mac[6];
+     unsigned int guid_high = readl(EXYNOS4_GUID_HIGH);
+     unsigned int guid_low = readl(EXYNOS4_GUID_LOW);
We don't allow direct access.
Is it special register? I can't find this register on TRM. If so you can make inline function at cpu.h instead.

Quoted 8 lines

Quoted 4 lines
Those register addresses are not documented anywhere. So use of them is not a good idea and can cause unpredictable results, even if was tested on a few devices.
OK, fair enough. I thought somehow it would be documented in the Samsung internal user manual. I guess it is not!

I checked some of the Exynos4 and Exynos5 documentation, and unfortunately I didn't found any information about those registers. Maybe there is some meaning of them, but rather for the vendor, than for the user.

Quoted 9 lines

Quoted 9 lines
Quoted 8 lines
Regarding direct access, I am a bit confused. In odroid.c I see quite a many places which is doing a readl() of registers. Here we are readl(addr) into guid_* similarly, and cooking up a mac address in a local char array.
I fail to see your point. Can you please elaborate more, so I can comprehend?

Quoted 7 lines

Quoted 2 lines
The MAC address is defined by IEEE standard, so we can't use magic numbers from undefined registers.
I am aware of that. In the code by Tushar Behera you will see that he does mark it - "not multicast and outside official 80211 MAC namespace".

Ok, but this is an open source code, and I assume that it should keeps some of the standards?

Quoted 8 lines

Quoted 7 lines
I see two ways to do it:
- put the MAC into environment
- use of UUID generator from: lib/uuid.c
On the Odroid you will get the hardware acceleration, because the hardware random number generator is enabled.

Quoted 7 lines

Let me research a bit - if a random MAC address can indeed be cooked up. If not, as you suggest, we can leave it as an onus on the user to set it in the env. (which is what is being done currently).
Do you have any pointers in using the hardware random generator in the Odroid? That does seem promising.

The random MAC in the meaning that 3-bytes for the vendor and 3 random generated(unique) - for the automation.
But the environment variable with MAC is simpler to do and don't require
hardcode the vendor's 3 bytes of MAC.

Please look into the lib/uuid.c:
- gen_rand_uuid()
- gen_rand_uuid_str()
This is also used by "gpt" command.
If you enable CONFIG_CMD_UUID, then you can see how it works on the console.

Quoted 203 lines

Quoted 192 lines

Quoted 11 lines

board/samsung/odroid/odroid.c struct s3c_plat_otg_data s5pc210_otg_data = { · line 472
Quoted 7 lines
board/samsung/odroid/odroid.c +6 -0
+
+     for (i = 0; i < 2; i++)
+             mac[i] = (guid_high >> (8 * (1 - i))) & 0xFF;
+
+     for (i = 0; i < 4; i++)
+             mac[i+2] = (guid_low >> (8 * (3 - i))) & 0xFF;
please add space. i + 2

Shall do so.

Quoted 103 lines

Quoted 89 lines

board/samsung/odroid/odroid.c +2 -0

+
+     /* mark it as not multicast and outside official 80211 MAC

namespace */

board/samsung/odroid/odroid.c +36 -0

+     mac[0] = (mac[0] & ~0x1) | 0x2;
+
+     eth_setenv_enetaddr("ethaddr", mac);
+     eth_setenv_enetaddr("usbethaddr", mac);
+}
+#endif
 
   int board_usb_init(int index, enum usb_init_type init)
   {
+#ifdef CONFIG_CMD_USB
+     struct pmic *p_pmic;
+
+     /* Set Ref freq 0 => 24MHz, 1 => 26MHz*/
+     /* Odroid Us have it at 24MHz, Odroid Xs at 26MHz */
+     if (gd->board_type == ODROID_TYPE_U3)
+             gpio_direction_output(EXYNOS4X12_GPIO_X30, 0);
+     else
+             gpio_direction_output(EXYNOS4X12_GPIO_X30, 1);
+
+     /* Disconnect, Reset, Connect */
+     gpio_direction_output(EXYNOS4X12_GPIO_X34, 0);
+     gpio_direction_output(EXYNOS4X12_GPIO_X35, 0);
+     gpio_direction_output(EXYNOS4X12_GPIO_X35, 1);
+     gpio_direction_output(EXYNOS4X12_GPIO_X34, 1);
+
+     /* Power off and on BUCK8 for LAN9730 */
+     debug("LAN9730 - Turning power buck 8 OFF and ON.\n");
+
+     p_pmic = pmic_get("MAX77686_PMIC");
+     if (p_pmic && !pmic_probe(p_pmic)) {
+             max77686_set_buck_mode(p_pmic, 8, OPMODE_OFF);
+             max77686_set_buck_voltage(p_pmic, 8, 750000);
+             max77686_set_buck_voltage(p_pmic, 8, 3300000);
+             max77686_set_buck_mode(p_pmic, 8, OPMODE_ON);
+     }
+
+     set_usb_ethaddr();
+#endif
+
        debug("USB_udc_probe\n");
        return s3c_udc_probe(&s5pc210_otg_data);
   }

drivers/usb/host/ehci-exynos.c +1 -6

index edd91a8..9c479b1 100644
@@ -85,15 +85,10 @@ static int exynos_usb_parse_dt(const void *blob,
 struct exynos_ehci *exynos)
   }
   #endif
 
-/* Setup the EHCI host controller. */
-static void setup_usb_phy(struct exynos_usb_phy *usb)
+static void exynos5_setup_usb_phy(struct exynos_usb_phy *usb)
   {
        u32 hsic_ctrl;
 
-     set_usbhost_mode(USB20_PHY_CFG_HOST_LINK_EN);
-
-     set_usbhost_phy_ctrl(POWER_USB_HOST_PHY_CTRL_EN);
-
        clrbits_le32(&usb->usbphyctrl0,
                        HOST_CTRL0_FSEL_MASK |

HOST_CTRL0_COMMONON_N |

drivers/usb/host/ehci-exynos.c +12 -2

@@ -150,8 +145,32 @@ static void setup_usb_phy(struct exynos_usb_phy
 *usb)
                        EHCICTRL_ENAINCR16);
   }
 
-/* Reset the EHCI host controller. */
-static void reset_usb_phy(struct exynos_usb_phy *usb)
+static void exynos4412_setup_usb_phy(struct exynos4412_usb_phy *usb)
+{
+     writel(CLK_24MHZ, &usb->usbphyclk);
+
+     clrbits_le32(&usb->usbphyctrl, (PHYPWR_NORMAL_MASK_HSIC0 |
+             PHYPWR_NORMAL_MASK_HSIC1 | PHYPWR_NORMAL_MASK_PHY1 |
+             PHYPWR_NORMAL_MASK_PHY0));
+
+     setbits_le32(&usb->usbphyrstcon, (RSTCON_HOSTPHY_SWRST |
 RSTCON_SWRST));
+     udelay(10);
+     clrbits_le32(&usb->usbphyrstcon, (RSTCON_HOSTPHY_SWRST |
 RSTCON_SWRST));
+}

drivers/usb/host/ehci-exynos.c static void setup_usb_phy(struct exynos_usb_phy · line 172

Quoted 10 lines

drivers/usb/host/ehci-exynos.c +10 -0

+
+static void setup_usb_phy(struct exynos_usb_phy *usb)
+{
+     set_usbhost_mode(USB20_PHY_CFG_HOST_LINK_EN);
+
+     set_usbhost_phy_ctrl(POWER_USB_HOST_PHY_CTRL_EN);
+
+     if (cpu_is_exynos5())
+             exynos5_setup_usb_phy(usb);
+     else if (proid_is_exynos4412())

please don't mix cpu_is.. and proid_is...

Quoted 9 lines

it should be
if (cpu_is_exynos5()) {
}
else if (cpu_is_exynos4()) {
      if (proid_is_exynos4412()) {
      }
}

OK, I shall do so.

Quoted 23 lines

drivers/usb/host/ehci-exynos.c +1 -0

+             exynos4412_setup_usb_phy((struct exynos4412_usb_phy

*)usb);

drivers/usb/host/ehci-exynos.c +13 -0

+}
+
+static void exynos5_reset_usb_phy(struct exynos_usb_phy *usb)
   {
        u32 hsic_ctrl;

@@ -171,6 +190,22 @@ static void reset_usb_phy(struct exynos_usb_phy
 *usb)
 
        setbits_le32(&usb->hsicphyctrl1, hsic_ctrl);
        setbits_le32(&usb->hsicphyctrl2, hsic_ctrl);
+}
+
+static void exynos4412_reset_usb_phy(struct exynos4412_usb_phy *usb)
+{
+     setbits_le32(&usb->usbphyctrl, (PHYPWR_NORMAL_MASK_HSIC0 |
+             PHYPWR_NORMAL_MASK_HSIC1 | PHYPWR_NORMAL_MASK_PHY1 |
+             PHYPWR_NORMAL_MASK_PHY0));
+}
+
+/* Reset the EHCI host controller. */

Quoted 9 lines

drivers/usb/host/ehci-exynos.c static void reset_usb_phy(struct exynos_usb_phy *usb) · line 207
Quoted 5 lines
drivers/usb/host/ehci-exynos.c +5 -0
+static void reset_usb_phy(struct exynos_usb_phy *usb)
+{
+     if (cpu_is_exynos5())
+             exynos5_reset_usb_phy(usb);
+     else if (proid_is_exynos4412())
ditto.

OK, shall change this as well.

Thanks for taking a look!
- Suriyan

Quoted 25 lines

drivers/usb/host/ehci-exynos.c +1 -0

+             exynos4412_reset_usb_phy((struct exynos4412_usb_phy
 *)usb);

        set_usbhost_phy_ctrl(POWER_USB_HOST_PHY_CTRL_DISABLE);

}

include/configs/odroid.h +12 -0

index b928af8..807e96b 100644
@@ -198,6 +198,19 @@
 
   #define CONFIG_CMD_GPIO
 
+/* USB */
+#define CONFIG_CMD_USB
+#define CONFIG_USB_EHCI
+#define CONFIG_USB_EHCI_EXYNOS
+#define CONFIG_USB_STORAGE
+
+#define CONFIG_SYS_USB_EHCI_MAX_ROOT_PORTS   3
+#define CONFIG_CMD_NET
+#define CONFIG_CMD_PING
+#define CONFIG_CMD_DHCP
+#define CONFIG_USB_HOST_ETHER
+#define CONFIG_USB_ETHER_SMSC95XX

Quoted 10 lines

Quoted 8 lines
Quoted 8 lines
include/configs/odroid.h line 216
Quoted 5 lines
include/configs/odroid.h +1 -0
+
   /*
    * Supported Odroid boards: X3, U3
    * TODO: Add Odroid X support
Thanks,
Minkyu Kang.

Quoted 10 lines

Thanks!
- Suriyan
Quoted 6 lines
Best regards,
--
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com

Best regards,

Signature · 4 lines

Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com

Albert ARIBAUD

2014-10-28 08:08 UTC #8 lore raw

PATCH v2 3/3 arm: odroid: usb: add support for usb host including ethernet

Hello Przemyslaw,

Quoted 5 lines · Przemyslaw Marczak

Hello Suriyan Ramasami,
Quoted 3 lines
Quoted 2 lines
The MAC address is defined by IEEE standard, so we can't use magic numbers from undefined registers.

Quoted 7 lines

Quoted 4 lines
I am aware of that. In the code by Tushar Behera you will see that he does mark it - "not multicast and outside official 80211 MAC namespace".
Ok, but this is an open source code, and I assume that it should keeps some of the standards?

At least the code could and should make sure the Locally Administered flag is set in the randomly generated MAC -- that would keep the MAC standard and ensure it won't conflict with universally administered (iow, 'normal') MAC addresses.

Amicalement,

Signature · 1 line

Albert.

Minkyu Kang

2014-10-27 13:03 UTC #9 lore raw

PATCH v2 3/3 arm: odroid: usb: add support for usb host including ethernet

In reply to Suriyan Ramasami

Dear Suriyan Ramasami,

Quoted 125 lines · Suriyan Ramasami

Hello Minkyu Kang,

Quoted 103 lines · Minkyu Kang

Dear Suriyan Ramasami,

Quoted 100 lines · Suriyan Ramasami

This change adds support for enabling the USB host features of the board. This includes the USB3503A hub and the SMC LAN9730 ethernet controller as well.

Credit goes to Tushar Berara for the function set_usb_ethaddr().

Signed-off-by Suriyan Ramasami <[email protected]>

---
v2:
* Removed an unneeded header file from ehci-exynos.c
* Fix indentation in the dts file

---
 arch/arm/dts/exynos4412-odroid.dts      | 11 +++++++
 arch/arm/include/asm/arch-exynos/cpu.h  |  2 ++
 arch/arm/include/asm/arch-exynos/ehci.h | 13 ++++++++
 board/samsung/odroid/odroid.c           | 55 +++++++++++++++++++++++++++++++++
 drivers/usb/host/ehci-exynos.c          | 51 +++++++++++++++++++++++++-----
 include/configs/odroid.h                | 13 ++++++++
 6 files changed, 137 insertions(+), 8 deletions(-)

arch/arm/dts/exynos4412-odroid.dts +11 -0

index 24d0bf1..ac5fece 100644
@@ -67,4 +67,15 @@
              div = <0x3>;
              index = <4>;
      };
+
+     ehci at 12580000 {
+             compatible = "samsung,exynos-ehci";
+             reg = <0x12580000 0x100>;
+             #address-cells = <1>;
+             #size-cells = <1>;
+             phy {
+                     compatible = "samsung,exynos-usb-phy";
+                     reg = <0x125B0000 0x100>;
+             };
+     };
 };

arch/arm/include/asm/arch-exynos/cpu.h +2 -0

index ba71714..fda21fb 100644
@@ -18,6 +18,8 @@
 
 #define EXYNOS4_GPIO_PART3_BASE              0x03860000
 #define EXYNOS4_PRO_ID                       0x10000000
+#define EXYNOS4_GUID_LOW             0x10000014
+#define EXYNOS4_GUID_HIGH            0x10000018
 #define EXYNOS4_SYSREG_BASE          0x10010000
 #define EXYNOS4_POWER_BASE           0x10020000
 #define EXYNOS4_SWRESET                      0x10020400

arch/arm/include/asm/arch-exynos/ehci.h +13 -0

index d2d70bd..3800fa9 100644
@@ -12,6 +12,13 @@
 
 #define CLK_24MHZ            5
 
+#define PHYPWR_NORMAL_MASK_PHY0                 (0x39 << 0)
+#define PHYPWR_NORMAL_MASK_PHY1                 (0x7 << 6)
+#define PHYPWR_NORMAL_MASK_HSIC0                (0x7 << 9)
+#define PHYPWR_NORMAL_MASK_HSIC1                (0x7 << 12)
+#define RSTCON_HOSTPHY_SWRST                    (0xf << 3)
+#define RSTCON_SWRST                            (0x1 << 0)
+
 #define HOST_CTRL0_PHYSWRSTALL                       (1 << 31)
 #define HOST_CTRL0_COMMONON_N                        (1 << 9)
 #define HOST_CTRL0_SIDDQ                     (1 << 6)

@@ -61,6 +68,12 @@ struct exynos_usb_phy {
      unsigned int usbotgtune;
 };
 
+struct exynos4412_usb_phy {
+     unsigned int usbphyctrl;
+     unsigned int usbphyclk;
+     unsigned int usbphyrstcon;
+};
+
 /* Switch on the VBUS power. */
 int board_usb_vbus_init(void);

board/samsung/odroid/odroid.c +7 -0

index 5edb250..6c78b67 100644
@@ -453,9 +453,64 @@ struct s3c_plat_otg_data s5pc210_otg_data = {
      .usb_phy_ctrl   = EXYNOS4X12_USBPHY_CONTROL,
      .usb_flags      = PHY0_SLEEP,
 };
+#endif
+
+#if defined(CONFIG_USB_GADGET) || defined(CONFIG_CMD_USB)
+
+#ifdef CONFIG_CMD_USB
+static void set_usb_ethaddr(void)
+{

Quoted 9 lines

board/samsung/odroid/odroid.c struct s3c_plat_otg_data s5pc210_otg_data = { · line 466
Quoted 4 lines
board/samsung/odroid/odroid.c +4 -0
+     int i;
+     uchar mac[6];
+     unsigned int guid_high = readl(EXYNOS4_GUID_HIGH);
+     unsigned int guid_low = readl(EXYNOS4_GUID_LOW);
We don't allow direct access.
Is it special register? I can't find this register on TRM. If so you can make inline function at cpu.h instead.

Quoted 7 lines

Regarding direct access, I am a bit confused. In odroid.c I see quite a many places which is doing a readl() of registers. Here we are readl(addr) into guid_* similarly, and cooking up a mac address in a local char array.
I fail to see your point. Can you please elaborate more, so I can comprehend?

Samsung SoCs always get the address of register using accessor functions(samsung_get_base_xxx).

Thanks,
Minkyu Kang.

Suriyan Ramasami

author

2014-10-27 19:36 UTC #10 lore raw

PATCH v2 3/3 arm: odroid: usb: add support for usb host including ethernet

Hello Minkyu Kang,

Quoted 133 lines · Minkyu Kang

Dear Suriyan Ramasami,

Quoted 130 lines · Suriyan Ramasami

Hello Minkyu Kang,

Quoted 103 lines · Minkyu Kang

Dear Suriyan Ramasami,

Quoted 100 lines · Suriyan Ramasami

This change adds support for enabling the USB host features of the board. This includes the USB3503A hub and the SMC LAN9730 ethernet controller as well.

Credit goes to Tushar Berara for the function set_usb_ethaddr().

Signed-off-by Suriyan Ramasami <[email protected]>

---
v2:
* Removed an unneeded header file from ehci-exynos.c
* Fix indentation in the dts file

---
 arch/arm/dts/exynos4412-odroid.dts      | 11 +++++++
 arch/arm/include/asm/arch-exynos/cpu.h  |  2 ++
 arch/arm/include/asm/arch-exynos/ehci.h | 13 ++++++++
 board/samsung/odroid/odroid.c           | 55 +++++++++++++++++++++++++++++++++
 drivers/usb/host/ehci-exynos.c          | 51 +++++++++++++++++++++++++-----
 include/configs/odroid.h                | 13 ++++++++
 6 files changed, 137 insertions(+), 8 deletions(-)

arch/arm/dts/exynos4412-odroid.dts +11 -0

index 24d0bf1..ac5fece 100644
@@ -67,4 +67,15 @@
              div = <0x3>;
              index = <4>;
      };
+
+     ehci at 12580000 {
+             compatible = "samsung,exynos-ehci";
+             reg = <0x12580000 0x100>;
+             #address-cells = <1>;
+             #size-cells = <1>;
+             phy {
+                     compatible = "samsung,exynos-usb-phy";
+                     reg = <0x125B0000 0x100>;
+             };
+     };
 };

arch/arm/include/asm/arch-exynos/cpu.h +2 -0

index ba71714..fda21fb 100644
@@ -18,6 +18,8 @@
 
 #define EXYNOS4_GPIO_PART3_BASE              0x03860000
 #define EXYNOS4_PRO_ID                       0x10000000
+#define EXYNOS4_GUID_LOW             0x10000014
+#define EXYNOS4_GUID_HIGH            0x10000018
 #define EXYNOS4_SYSREG_BASE          0x10010000
 #define EXYNOS4_POWER_BASE           0x10020000
 #define EXYNOS4_SWRESET                      0x10020400

arch/arm/include/asm/arch-exynos/ehci.h +13 -0

index d2d70bd..3800fa9 100644
@@ -12,6 +12,13 @@
 
 #define CLK_24MHZ            5
 
+#define PHYPWR_NORMAL_MASK_PHY0                 (0x39 << 0)
+#define PHYPWR_NORMAL_MASK_PHY1                 (0x7 << 6)
+#define PHYPWR_NORMAL_MASK_HSIC0                (0x7 << 9)
+#define PHYPWR_NORMAL_MASK_HSIC1                (0x7 << 12)
+#define RSTCON_HOSTPHY_SWRST                    (0xf << 3)
+#define RSTCON_SWRST                            (0x1 << 0)
+
 #define HOST_CTRL0_PHYSWRSTALL                       (1 << 31)
 #define HOST_CTRL0_COMMONON_N                        (1 << 9)
 #define HOST_CTRL0_SIDDQ                     (1 << 6)

@@ -61,6 +68,12 @@ struct exynos_usb_phy {
      unsigned int usbotgtune;
 };
 
+struct exynos4412_usb_phy {
+     unsigned int usbphyctrl;
+     unsigned int usbphyclk;
+     unsigned int usbphyrstcon;
+};
+
 /* Switch on the VBUS power. */
 int board_usb_vbus_init(void);

board/samsung/odroid/odroid.c +7 -0

index 5edb250..6c78b67 100644
@@ -453,9 +453,64 @@ struct s3c_plat_otg_data s5pc210_otg_data = {
      .usb_phy_ctrl   = EXYNOS4X12_USBPHY_CONTROL,
      .usb_flags      = PHY0_SLEEP,
 };
+#endif
+
+#if defined(CONFIG_USB_GADGET) || defined(CONFIG_CMD_USB)
+
+#ifdef CONFIG_CMD_USB
+static void set_usb_ethaddr(void)
+{

Quoted 9 lines

board/samsung/odroid/odroid.c struct s3c_plat_otg_data s5pc210_otg_data = { · line 466
Quoted 4 lines
board/samsung/odroid/odroid.c +4 -0
+     int i;
+     uchar mac[6];
+     unsigned int guid_high = readl(EXYNOS4_GUID_HIGH);
+     unsigned int guid_low = readl(EXYNOS4_GUID_LOW);
We don't allow direct access.
Is it special register? I can't find this register on TRM. If so you can make inline function at cpu.h instead.

Quoted 5 lines

Quoted 2 lines
I fail to see your point. Can you please elaborate more, so I can comprehend?
Samsung SoCs always get the address of register using accessor functions(samsung_get_base_xxx).

Ah! I see your point.

Unfortunately, use of these beloved registers has been thwarted by Przemyslaw Marczak.

Regards
- Suriyan

Quoted 3 lines

Thanks,
Minkyu Kang.

Jaehoon Chung

2014-10-24 06:52 UTC #11 lore raw

PATCH v2 1/3 arm: odroid: pmic77686: allow bucket voltage settings

In reply to Suriyan Ramasami

Hi.

Quoted 41 lines · Suriyan Ramasami

Allow to set the bucket voltage for the max77686.
This will be used to reset the SMC LAN9730 ethernet on the odroids.

Signed-off-by Suriyan Ramasami <[email protected]>

---
 drivers/power/pmic/pmic_max77686.c | 48 +++++++++++++++++++++++++++++++++++++-
 include/power/max77686_pmic.h      |  3 +++
 2 files changed, 50 insertions(+), 1 deletion(-)

drivers/power/pmic/pmic_max77686.c +19 -0

index df1fd91..1aeadb4 100644
@@ -42,6 +42,25 @@ static unsigned int max77686_ldo_volt2hex(int ldo, ulong uV)
 	return 0;
 }
 
+static unsigned int max77686_buck_volt2hex(int buck, ulong uV)
+{
+	unsigned int hex = 0;
+
+	if (buck < 5 || buck > 9) {
+		debug("%s: buck %d is not supported\n", __func__, buck);
+		return 0;
+	}
+
+	hex = (uV - 750000) / 50000;
+
+	if (hex >= 0 && hex <= MAX77686_BUCK_VOLT_MAX_HEX)
+		return hex;
+
+	debug("%s: %ld is wrong voltage value for BUCK%d\n",
+	      __func__, uV, buck);
+	return MAX77686_BUCK_VOLT_MAX_HEX + 1;
+}
+
 int max77686_set_ldo_voltage(struct pmic *p, int ldo, ulong uV)
 {
 	unsigned int val, ret, hex, adr;

@@ -68,6 +87,33 @@ int max77686_set_ldo_voltage(struct pmic *p, int ldo, ulong uV)
 	return ret;

drivers/power/pmic/pmic_max77686.c int max77686_set_ldo_voltage(struct pmic *p, int ldo, ulong uV) · line 97

Quoted 10 lines

drivers/power/pmic/pmic_max77686.c +8 -0

 }
 
+int max77686_set_buck_voltage(struct pmic *p, int buck, ulong uV)
+{
+	unsigned int val, ret, hex, adr;
+
+	if (buck < 5 && buck > 9) {
+		printf("%s: %d is an unsupported bucket number\n",
+		       __func__, buck);
+		return -1;

How about using error number instead of "-1"?

drivers/power/pmic/pmic_max77686.c int max77686_set_ldo_voltage(struct pmic *p, int ldo, ulong uV) · line 104

Quoted 7 lines

drivers/power/pmic/pmic_max77686.c +7 -0

+	}
+
+	adr = max77686_buck_addr[buck] + 1;
+	hex = max77686_buck_volt2hex(buck, uV);
+
+	if (hex == MAX77686_BUCK_VOLT_MAX_HEX + 1)
+		return -1;

Ditto.

drivers/power/pmic/pmic_max77686.c int max77686_set_ldo_voltage(struct pmic *p, int ldo, ulong uV) · line 112

Quoted 8 lines

drivers/power/pmic/pmic_max77686.c +8 -0

+
+	ret = pmic_reg_read(p, adr, &val);
+	if (ret)
+		return ret;
+
+	val &= ~MAX77686_BUCK_VOLT_MASK;
+	val |= hex;
+	ret |= pmic_reg_write(p, adr, val);

ret |= pmic_reg_write(p, addr, val | hex); ?

Best Regards,
Jaehoon Chung

Quoted 38 lines

drivers/power/pmic/pmic_max77686.c +5 -1

+
+	return ret;
+}
+
 int max77686_set_ldo_mode(struct pmic *p, int ldo, char opmode)
 {
 	unsigned int val, ret, adr, mode;

@@ -157,7 +203,7 @@ int max77686_set_buck_mode(struct pmic *p, int buck, char opmode)
 	/* mode */
 	switch (opmode) {
 	case OPMODE_OFF:
-		mode = MAX77686_BUCK_MODE_OFF;
+		mode = MAX77686_BUCK_MODE_OFF << mode_shift;
 		break;
 	case OPMODE_STANDBY:
 		switch (buck) {

include/power/max77686_pmic.h +3 -0

index c2a772a..b0e4255 100644
@@ -150,6 +150,7 @@ enum {
 
 int max77686_set_ldo_voltage(struct pmic *p, int ldo, ulong uV);
 int max77686_set_ldo_mode(struct pmic *p, int ldo, char opmode);
+int max77686_set_buck_voltage(struct pmic *p, int buck, ulong uV);
 int max77686_set_buck_mode(struct pmic *p, int buck, char opmode);
 
 #define MAX77686_LDO_VOLT_MAX_HEX	0x3f

@@ -159,6 +160,8 @@ int max77686_set_buck_mode(struct pmic *p, int buck, char opmode);
 #define MAX77686_LDO_MODE_STANDBY	(0x01 << 0x06)
 #define MAX77686_LDO_MODE_LPM		(0x02 << 0x06)
 #define MAX77686_LDO_MODE_ON		(0x03 << 0x06)
+#define MAX77686_BUCK_VOLT_MAX_HEX	0x3f
+#define MAX77686_BUCK_VOLT_MASK		0x3f
 #define MAX77686_BUCK_MODE_MASK		0x03
 #define MAX77686_BUCK_MODE_SHIFT_1	0x00
 #define MAX77686_BUCK_MODE_SHIFT_2	0x04

Suriyan Ramasami

author

2014-10-24 15:53 UTC #12 lore raw

PATCH v2 1/3 arm: odroid: pmic77686: allow bucket voltage settings

Hello Jaehoon Chung,

Quoted 20 lines · Jaehoon Chung

Hi.
Quoted 17 lines · Suriyan Ramasami
Allow to set the bucket voltage for the max77686.
This will be used to reset the SMC LAN9730 ethernet on the odroids.
Signed-off-by Suriyan Ramasami <[email protected]>
---
 drivers/power/pmic/pmic_max77686.c | 48 +++++++++++++++++++++++++++++++++++++-
 include/power/max77686_pmic.h      |  3 +++
 2 files changed, 50 insertions(+), 1 deletion(-)
drivers/power/pmic/pmic_max77686.c +0 -0
index df1fd91..1aeadb4 100644
@@ -42,6 +42,25 @@ static unsigned int max77686_ldo_volt2hex(int ldo, ulong uV)
      return 0;
 }
 

Quoted 7 lines

drivers/power/pmic/pmic_max77686.c static unsigned int max77686_ldo_volt2hex(int ldo, ulong uV) · line 51
Quoted 7 lines
drivers/power/pmic/pmic_max77686.c +7 -0
+static unsigned int max77686_buck_volt2hex(int buck, ulong uV)
+{
+     unsigned int hex = 0;
+
+     if (buck < 5 || buck > 9) {
+             debug("%s: buck %d is not supported\n", __func__, buck);
+             return 0;

Here, I should return MAX77686_BUCK_VOLT_MAX_HEX + 1 instead of 0 to signal an error condition, as 0 is a valid non error value.

Quoted 19 lines

drivers/power/pmic/pmic_max77686.c +12 -0

+     }
+
+     hex = (uV - 750000) / 50000;
+
+     if (hex >= 0 && hex <= MAX77686_BUCK_VOLT_MAX_HEX)
+             return hex;
+
+     debug("%s: %ld is wrong voltage value for BUCK%d\n",
+           __func__, uV, buck);
+     return MAX77686_BUCK_VOLT_MAX_HEX + 1;
+}
+
 int max77686_set_ldo_voltage(struct pmic *p, int ldo, ulong uV)
 {
      unsigned int val, ret, hex, adr;

@@ -68,6 +87,33 @@ int max77686_set_ldo_voltage(struct pmic *p, int ldo, ulong uV)
      return ret;
 }

Quoted 10 lines

drivers/power/pmic/pmic_max77686.c int max77686_set_ldo_voltage(struct pmic *p, int ldo, ulong uV) · line 97
Quoted 8 lines
drivers/power/pmic/pmic_max77686.c +8 -0
+int max77686_set_buck_voltage(struct pmic *p, int buck, ulong uV)
+{
+     unsigned int val, ret, hex, adr;
+
+     if (buck < 5 && buck > 9) {
+             printf("%s: %d is an unsupported bucket number\n",
+                    __func__, buck);
+             return -1;
How about using error number instead of "-1"?

Could you please elaborate on this? This function always returns -1 on failure just like the function max77686_set_ldo_voltate() which I have tried to mimic as much as I can.
I am guessing that you want me to return -EINVAL in this case? Please let me know, and I am OK to change it, just that this function will deviate from the rest of the functions in this file.

Quoted 10 lines

drivers/power/pmic/pmic_max77686.c int max77686_set_ldo_voltage(struct pmic *p, int ldo, ulong uV) · line 104
Quoted 8 lines
drivers/power/pmic/pmic_max77686.c +7 -0
+     }
+
+     adr = max77686_buck_addr[buck] + 1;
+     hex = max77686_buck_volt2hex(buck, uV);
+
+     if (hex == MAX77686_BUCK_VOLT_MAX_HEX + 1)
+             return -1;
Ditto.

I believe, I return -EINVAL here, but please look at my reasoning above.

Quoted 12 lines

drivers/power/pmic/pmic_max77686.c int max77686_set_ldo_voltage(struct pmic *p, int ldo, ulong uV) · line 112
Quoted 9 lines
drivers/power/pmic/pmic_max77686.c +8 -0
+
+     ret = pmic_reg_read(p, adr, &val);
+     if (ret)
+             return ret;
+
+     val &= ~MAX77686_BUCK_VOLT_MASK;
+     val |= hex;
+     ret |= pmic_reg_write(p, adr, val);
ret |= pmic_reg_write(p, addr, val | hex); ?

OK, will change that. Again, this was done to be as close to max77686_set_ldo_voltate()

Thanks and Regards!
- Suriyan

Quoted 41 lines

Best Regards,
Jaehoon Chung

Quoted 38 lines

drivers/power/pmic/pmic_max77686.c +5 -1

+
+     return ret;
+}
+
 int max77686_set_ldo_mode(struct pmic *p, int ldo, char opmode)
 {
      unsigned int val, ret, adr, mode;

@@ -157,7 +203,7 @@ int max77686_set_buck_mode(struct pmic *p, int buck, char opmode)
      /* mode */
      switch (opmode) {
      case OPMODE_OFF:
-             mode = MAX77686_BUCK_MODE_OFF;
+             mode = MAX77686_BUCK_MODE_OFF << mode_shift;
              break;
      case OPMODE_STANDBY:
              switch (buck) {

include/power/max77686_pmic.h +3 -0

index c2a772a..b0e4255 100644
@@ -150,6 +150,7 @@ enum {
 
 int max77686_set_ldo_voltage(struct pmic *p, int ldo, ulong uV);
 int max77686_set_ldo_mode(struct pmic *p, int ldo, char opmode);
+int max77686_set_buck_voltage(struct pmic *p, int buck, ulong uV);
 int max77686_set_buck_mode(struct pmic *p, int buck, char opmode);
 
 #define MAX77686_LDO_VOLT_MAX_HEX    0x3f

@@ -159,6 +160,8 @@ int max77686_set_buck_mode(struct pmic *p, int buck, char opmode);
 #define MAX77686_LDO_MODE_STANDBY    (0x01 << 0x06)
 #define MAX77686_LDO_MODE_LPM                (0x02 << 0x06)
 #define MAX77686_LDO_MODE_ON         (0x03 << 0x06)
+#define MAX77686_BUCK_VOLT_MAX_HEX   0x3f
+#define MAX77686_BUCK_VOLT_MASK              0x3f
 #define MAX77686_BUCK_MODE_MASK              0x03
 #define MAX77686_BUCK_MODE_SHIFT_1   0x00
 #define MAX77686_BUCK_MODE_SHIFT_2   0x04

Przemyslaw Marczak

2014-10-27 11:08 UTC #13 lore raw

PATCH v2 1/3 arm: odroid: pmic77686: allow bucket voltage settings

Hello Suriyan,

Quoted 24 lines · Suriyan Ramasami

Hello Jaehoon Chung,
Quoted 20 lines · Jaehoon Chung
Hi.
Quoted 17 lines · Suriyan Ramasami
Allow to set the bucket voltage for the max77686.
This will be used to reset the SMC LAN9730 ethernet on the odroids.
Signed-off-by Suriyan Ramasami <[email protected]>
---
  drivers/power/pmic/pmic_max77686.c | 48 +++++++++++++++++++++++++++++++++++++-
  include/power/max77686_pmic.h      |  3 +++
  2 files changed, 50 insertions(+), 1 deletion(-)
drivers/power/pmic/pmic_max77686.c +0 -0
index df1fd91..1aeadb4 100644
@@ -42,6 +42,25 @@ static unsigned int max77686_ldo_volt2hex(int ldo, ulong uV)
       return 0;
  }
 

Quoted 11 lines

Quoted 7 lines
drivers/power/pmic/pmic_max77686.c static unsigned int max77686_ldo_volt2hex(int ldo, ulong uV) · line 51
Quoted 7 lines
drivers/power/pmic/pmic_max77686.c +7 -0
+static unsigned int max77686_buck_volt2hex(int buck, ulong uV)
+{
+     unsigned int hex = 0;
+
+     if (buck < 5 || buck > 9) {
+             debug("%s: buck %d is not supported\n", __func__, buck);
+             return 0;
Here, I should return MAX77686_BUCK_VOLT_MAX_HEX + 1 instead of 0 to signal an error condition, as 0 is a valid non error value.

He 'hex' value has at most 1 byte of len, so you can return the 'int' value and use the negative errno numbers - and there is no value conflicts.

Quoted 30 lines

Quoted 19 lines

drivers/power/pmic/pmic_max77686.c +12 -0

+     }
+
+     hex = (uV - 750000) / 50000;
+
+     if (hex >= 0 && hex <= MAX77686_BUCK_VOLT_MAX_HEX)
+             return hex;
+
+     debug("%s: %ld is wrong voltage value for BUCK%d\n",
+           __func__, uV, buck);
+     return MAX77686_BUCK_VOLT_MAX_HEX + 1;
+}
+
  int max77686_set_ldo_voltage(struct pmic *p, int ldo, ulong uV)
  {
       unsigned int val, ret, hex, adr;

@@ -68,6 +87,33 @@ int max77686_set_ldo_voltage(struct pmic *p, int ldo, ulong uV)
       return ret;
  }

Quoted 10 lines

drivers/power/pmic/pmic_max77686.c int max77686_set_ldo_voltage(struct pmic *p, int ldo, ulong uV) · line 97
Quoted 8 lines
drivers/power/pmic/pmic_max77686.c +8 -0
+int max77686_set_buck_voltage(struct pmic *p, int buck, ulong uV)
+{
+     unsigned int val, ret, hex, adr;
+
+     if (buck < 5 && buck > 9) {
+             printf("%s: %d is an unsupported bucket number\n",
+                    __func__, buck);
+             return -1;
How about using error number instead of "-1"?

Quoted 7 lines

Could you please elaborate on this? This function always returns -1 on failure just like the function max77686_set_ldo_voltate() which I have tried to mimic as much as I can.
I am guessing that you want me to return -EINVAL in this case? Please let me know, and I am OK to change it, just that this function will deviate from the rest of the functions in this file.

Yes, this will be better.

Quoted 6 lines

Quoted 6 lines
drivers/power/pmic/pmic_max77686.c int max77686_set_ldo_voltage(struct pmic *p, int ldo, ulong uV) · line 102
Quoted 6 lines
drivers/power/pmic/pmic_max77686.c +5 -0
+     }
+
+     adr = max77686_buck_addr[buck] + 1;
+     hex = max77686_buck_volt2hex(buck, uV);
+

if (hex < 0)

	return hex;

Quoted 20 lines

Quoted 4 lines
drivers/power/pmic/pmic_max77686.c int max77686_set_ldo_voltage(struct pmic *p, int ldo, ulong uV) · line 104
Quoted 2 lines
drivers/power/pmic/pmic_max77686.c +2 -0
+     if (hex == MAX77686_BUCK_VOLT_MAX_HEX + 1)
+             return -1;
Ditto.
I believe, I return -EINVAL here, but please look at my reasoning above.
Quoted 12 lines
drivers/power/pmic/pmic_max77686.c int max77686_set_ldo_voltage(struct pmic *p, int ldo, ulong uV) · line 112
Quoted 9 lines
drivers/power/pmic/pmic_max77686.c +8 -0
+
+     ret = pmic_reg_read(p, adr, &val);
+     if (ret)
+             return ret;
+
+     val &= ~MAX77686_BUCK_VOLT_MASK;
+     val |= hex;
+     ret |= pmic_reg_write(p, adr, val);
ret |= pmic_reg_write(p, addr, val | hex); ?

Quoted 47 lines

OK, will change that. Again, this was done to be as close to max77686_set_ldo_voltate()

Thanks and Regards!
- Suriyan

Quoted 41 lines

Best Regards,
Jaehoon Chung

Quoted 38 lines

drivers/power/pmic/pmic_max77686.c +5 -1

+
+     return ret;
+}
+
  int max77686_set_ldo_mode(struct pmic *p, int ldo, char opmode)
  {
       unsigned int val, ret, adr, mode;

@@ -157,7 +203,7 @@ int max77686_set_buck_mode(struct pmic *p, int buck, char opmode)
       /* mode */
       switch (opmode) {
       case OPMODE_OFF:
-             mode = MAX77686_BUCK_MODE_OFF;
+             mode = MAX77686_BUCK_MODE_OFF << mode_shift;
               break;
       case OPMODE_STANDBY:
               switch (buck) {

include/power/max77686_pmic.h +3 -0

index c2a772a..b0e4255 100644
@@ -150,6 +150,7 @@ enum {
 
  int max77686_set_ldo_voltage(struct pmic *p, int ldo, ulong uV);
  int max77686_set_ldo_mode(struct pmic *p, int ldo, char opmode);
+int max77686_set_buck_voltage(struct pmic *p, int buck, ulong uV);
  int max77686_set_buck_mode(struct pmic *p, int buck, char opmode);
 
  #define MAX77686_LDO_VOLT_MAX_HEX    0x3f

@@ -159,6 +160,8 @@ int max77686_set_buck_mode(struct pmic *p, int buck, char opmode);
  #define MAX77686_LDO_MODE_STANDBY    (0x01 << 0x06)
  #define MAX77686_LDO_MODE_LPM                (0x02 << 0x06)
  #define MAX77686_LDO_MODE_ON         (0x03 << 0x06)
+#define MAX77686_BUCK_VOLT_MAX_HEX   0x3f
+#define MAX77686_BUCK_VOLT_MASK              0x3f
  #define MAX77686_BUCK_MODE_MASK              0x03
  #define MAX77686_BUCK_MODE_SHIFT_1   0x00
  #define MAX77686_BUCK_MODE_SHIFT_2   0x04

Best regards,

Signature · 4 lines

Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com

Suriyan Ramasami

author

2014-10-27 19:02 UTC #14 lore raw

PATCH v2 1/3 arm: odroid: pmic77686: allow bucket voltage settings

Hello Przemyslaw Marczak,

Quoted 47 lines · Przemyslaw Marczak

Hello Suriyan,
Quoted 31 lines · Suriyan Ramasami
Hello Jaehoon Chung,
Quoted 25 lines · Jaehoon Chung
Hi.
Quoted 21 lines · Suriyan Ramasami
Allow to set the bucket voltage for the max77686.
This will be used to reset the SMC LAN9730 ethernet on the odroids.
Signed-off-by Suriyan Ramasami <[email protected]>
---
  drivers/power/pmic/pmic_max77686.c | 48
+++++++++++++++++++++++++++++++++++++-
  include/power/max77686_pmic.h      |  3 +++
  2 files changed, 50 insertions(+), 1 deletion(-)
drivers/power/pmic/pmic_max77686.c +0 -0
index df1fd91..1aeadb4 100644
@@ -42,6 +42,25 @@ static unsigned int max77686_ldo_volt2hex(int ldo,
 ulong uV)
       return 0;
  }
 
Quoted 12 lines
Quoted 7 lines
drivers/power/pmic/pmic_max77686.c static unsigned int max77686_ldo_volt2hex(int ldo, ulong uV) · line 51
Quoted 7 lines
drivers/power/pmic/pmic_max77686.c +7 -0
+static unsigned int max77686_buck_volt2hex(int buck, ulong uV)
+{
+     unsigned int hex = 0;
+
+     if (buck < 5 || buck > 9) {
+             debug("%s: buck %d is not supported\n", __func__, buck);
+             return 0;
Here, I should return MAX77686_BUCK_VOLT_MAX_HEX + 1 instead of 0 to signal an error condition, as 0 is a valid non error value.

Quoted 3 lines

He 'hex' value has at most 1 byte of len, so you can return the 'int' value and use the negative errno numbers - and there is no value conflicts.

OK, Shall use negative errno values.

Quoted 34 lines

Quoted 32 lines

Quoted 19 lines

drivers/power/pmic/pmic_max77686.c +12 -0

+     }
+
+     hex = (uV - 750000) / 50000;
+
+     if (hex >= 0 && hex <= MAX77686_BUCK_VOLT_MAX_HEX)
+             return hex;
+
+     debug("%s: %ld is wrong voltage value for BUCK%d\n",
+           __func__, uV, buck);
+     return MAX77686_BUCK_VOLT_MAX_HEX + 1;
+}
+
  int max77686_set_ldo_voltage(struct pmic *p, int ldo, ulong uV)
  {

       unsigned int val, ret, hex, adr;

drivers/power/pmic/pmic_max77686.c +0 -0

@@ -68,6 +87,33 @@ int max77686_set_ldo_voltage(struct pmic *p, int ldo,
 ulong uV)
       return ret;

drivers/power/pmic/pmic_max77686.c int max77686_set_ldo_voltage(struct pmic *p, int ldo, · line 98

Quoted 10 lines

drivers/power/pmic/pmic_max77686.c +8 -0

  }
 
+int max77686_set_buck_voltage(struct pmic *p, int buck, ulong uV)
+{
+     unsigned int val, ret, hex, adr;
+
+     if (buck < 5 && buck > 9) {
+             printf("%s: %d is an unsupported bucket number\n",
+                    __func__, buck);
+             return -1;

How about using error number instead of "-1"?

Quoted 8 lines

Quoted 7 lines
Could you please elaborate on this? This function always returns -1 on failure just like the function max77686_set_ldo_voltate() which I have tried to mimic as much as I can.
I am guessing that you want me to return -EINVAL in this case? Please let me know, and I am OK to change it, just that this function will deviate from the rest of the functions in this file.
Yes, this will be better.

OK, shall do so.

Quoted 10 lines

Quoted 7 lines
Quoted 7 lines
drivers/power/pmic/pmic_max77686.c int max77686_set_ldo_voltage(struct pmic *p, int ldo, ulong uV) · line 102
Quoted 7 lines
drivers/power/pmic/pmic_max77686.c +5 -0
+     }
+
+     adr = max77686_buck_addr[buck] + 1;
+     hex = max77686_buck_volt2hex(buck, uV);
+
if (hex < 0)

Quoted 3 lines

        return hex;

OK, will do so.

Quoted 61 lines

Quoted 61 lines
Quoted 5 lines
drivers/power/pmic/pmic_max77686.c int max77686_set_ldo_voltage(struct pmic *p, int ldo, ulong uV) · line 104
Quoted 2 lines
drivers/power/pmic/pmic_max77686.c +2 -0
+     if (hex == MAX77686_BUCK_VOLT_MAX_HEX + 1)
+             return -1;
Ditto.
I believe, I return -EINVAL here, but please look at my reasoning above.
Quoted 13 lines
drivers/power/pmic/pmic_max77686.c int max77686_set_ldo_voltage(struct pmic *p, int ldo, ulong uV) · line 112
Quoted 9 lines
drivers/power/pmic/pmic_max77686.c +8 -0
+
+     ret = pmic_reg_read(p, adr, &val);
+     if (ret)
+             return ret;
+
+     val &= ~MAX77686_BUCK_VOLT_MASK;
+     val |= hex;
+     ret |= pmic_reg_write(p, adr, val);
ret |= pmic_reg_write(p, addr, val | hex); ?
OK, will change that. Again, this was done to be as close to max77686_set_ldo_voltate()
Thanks and Regards!
- Suriyan
Quoted 32 lines
Best Regards,
Jaehoon Chung
Quoted 30 lines
drivers/power/pmic/pmic_max77686.c +4 -0
+
+     return ret;
+}
+
  int max77686_set_ldo_mode(struct pmic *p, int ldo, char opmode)
  {
       unsigned int val, ret, adr, mode;
drivers/power/pmic/pmic_max77686.c +1 -1
@@ -157,7 +203,7 @@ int max77686_set_buck_mode(struct pmic *p, int buck,
 char opmode)
       /* mode */
       switch (opmode) {
       case OPMODE_OFF:
-             mode = MAX77686_BUCK_MODE_OFF;
+             mode = MAX77686_BUCK_MODE_OFF << mode_shift;
               break;
       case OPMODE_STANDBY:
               switch (buck) {
include/power/max77686_pmic.h +1 -0
index c2a772a..b0e4255 100644
@@ -150,6 +150,7 @@ enum {
 
  int max77686_set_ldo_voltage(struct pmic *p, int ldo, ulong uV);
  int max77686_set_ldo_mode(struct pmic *p, int ldo, char opmode);
+int max77686_set_buck_voltage(struct pmic *p, int buck, ulong uV);
  int max77686_set_buck_mode(struct pmic *p, int buck, char opmode);
 

Quoted 14 lines

Quoted 5 lines

include/power/max77686_pmic.h +0 -0

  #define MAX77686_LDO_VOLT_MAX_HEX    0x3f

@@ -159,6 +160,8 @@ int max77686_set_buck_mode(struct pmic *p, int buck,
 char opmode);
  #define MAX77686_LDO_MODE_STANDBY    (0x01 << 0x06)
  #define MAX77686_LDO_MODE_LPM                (0x02 << 0x06)

Quoted 8 lines

Quoted 7 lines

include/power/max77686_pmic.h +2 -0

  #define MAX77686_LDO_MODE_ON         (0x03 << 0x06)
+#define MAX77686_BUCK_VOLT_MAX_HEX   0x3f
+#define MAX77686_BUCK_VOLT_MASK              0x3f
  #define MAX77686_BUCK_MODE_MASK              0x03
  #define MAX77686_BUCK_MODE_SHIFT_1   0x00

  #define MAX77686_BUCK_MODE_SHIFT_2   0x04

Thanks!
- Suriyan

Quoted 6 lines

Best regards,
--
Przemyslaw Marczak
Samsung R&D Institute Poland
Samsung Electronics
p.marczak at samsung.com