sun6i: s/SUNXI_GPL0_R_P2WI/SUN6I_GPL0_R_P2WI/

sun8i (A23) introduces a new bus for communicating with the pmic, the rsb, the rsb is also used to communicate with the pmic on the A80, and is documented in the A80 user manual.

This commit adds support for this based on the rsb driver from the allwinner u-boot sources.

Note: Not yet ready for upstream, code needs some significant cleanups.

Signed-off-by Hans de Goede <[email protected]>

---
 arch/arm/cpu/armv7/sunxi/Makefile      |   1 +
 arch/arm/cpu/armv7/sunxi/rsb.c         | 158 +++++++++++++++++++++++++++++++++
 arch/arm/include/asm/arch-sunxi/cpu.h  |   3 +-
 arch/arm/include/asm/arch-sunxi/gpio.h |   2 +
 arch/arm/include/asm/arch-sunxi/prcm.h |   3 +-
 arch/arm/include/asm/arch-sunxi/rsb.h  |  55 ++++++++++++
 6 files changed, 220 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm/cpu/armv7/sunxi/rsb.c
 create mode 100644 arch/arm/include/asm/arch-sunxi/rsb.h

arch/arm/cpu/armv7/sunxi/Makefile +1 -0

index 1337b60..3e8975a 100644
@@ -15,6 +15,7 @@ obj-y += pinmux.o
 obj-$(CONFIG_MACH_SUN6I)       += prcm.o
 obj-$(CONFIG_MACH_SUN8I)       += prcm.o
 obj-$(CONFIG_MACH_SUN6I)       += p2wi.o
+obj-$(CONFIG_MACH_SUN8I)       += rsb.o
 obj-$(CONFIG_MACH_SUN4I)       += clock_sun4i.o
 obj-$(CONFIG_MACH_SUN5I)       += clock_sun4i.o
 obj-$(CONFIG_MACH_SUN6I)       += clock_sun6i.o

arch/arm/cpu/armv7/sunxi/rsb.c new file +45 -0

new file mode 100644
index 0000000..b72bb9d
@@ -0,0 +1,158 @@
+/*
+ * (C) Copyright 2014 Hans de Goede <[email protected]>
+ *
+ * Based on allwinner u-boot sources rsb code which is:
+ * (C) Copyright 2007-2013
+ * Allwinner Technology Co., Ltd. <www.allwinnertech.com>
+ * lixiang <[email protected]>
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+
+#include <common.h>
+#include <errno.h>
+#include <asm/arch/cpu.h>
+#include <asm/arch/gpio.h>
+#include <asm/arch/prcm.h>
+#include <asm/arch/rsb.h>
+
+static void rsb_cfg_io(void)
+{
+       sunxi_gpio_set_cfgpin(SUNXI_GPL(0), SUN8I_GPL0_R_RSB_SCK);
+       sunxi_gpio_set_cfgpin(SUNXI_GPL(1), SUN8I_GPL1_R_RSB_SDA);
+       sunxi_gpio_set_pull(SUNXI_GPL(0), 1);
+       sunxi_gpio_set_pull(SUNXI_GPL(1), 1);
+       sunxi_gpio_set_drv(SUNXI_GPL(0), 2);
+       sunxi_gpio_set_drv(SUNXI_GPL(1), 2);
+}
+
+static void rsb_set_clk(void)
+{
+       struct sunxi_rsb_reg * const rsb =
+               (struct sunxi_rsb_reg *)SUNXI_RSB_BASE;
+       u32 div = 0;
+       u32 cd_odly = 0;
+
+       /* Source is Hosc24M, set RSB clk to 3Mhz */
+       div = 24000000 / 3000000 / 2 - 1;
+       cd_odly = div >> 1;
+       if (!cd_odly)
+               cd_odly = 1;
+
+       writel((cd_odly << 8) | div, &rsb->ccr);
+}
+
+void rsb_init(void)

arch/arm/cpu/armv7/sunxi/rsb.c new file +103 -0

+       rsb_set_clk();
+}
+
+static int rsb_await_trans(void)
+{
+       struct sunxi_rsb_reg * const rsb =
+               (struct sunxi_rsb_reg *)SUNXI_RSB_BASE;
+       unsigned long tmo = timer_get_us() + 1000000;
+       u32 stat;
+       int ret;
+
+       while (1) {
+               stat = readl(&rsb->stat);
+               if (stat & RSB_STAT_LBSY_INT) {
+                       ret = -EBUSY;
+                       break;
+               }
+               if (stat & RSB_STAT_TERR_INT) {
+                       ret = -EIO;
+                       break;
+               }
+               if (stat & RSB_STAT_TOVER_INT) {
+                       ret = 0;
+                       break;
+               }
+               if (timer_get_us() > tmo) {
+                       ret = -ETIME;
+                       break;
+               }
+       }
+       writel(stat, &rsb->stat); /* Clear status bits */
+
+       return ret;
+}
+
+int rsb_set_device_mode(u32 device_mode_data)
+{
+       struct sunxi_rsb_reg * const rsb =
+               (struct sunxi_rsb_reg *)SUNXI_RSB_BASE;
+       unsigned long tmo = timer_get_us() + 1000000;
+
+       writel(RSB_DMCR_DEVICE_MODE_START | device_mode_data, &rsb->dmcr);
+
+       while (readl(&rsb->dmcr) & RSB_DMCR_DEVICE_MODE_START) {
+               if (timer_get_us() > tmo)
+                       return -ETIME;
+       }
+
+       return rsb_await_trans();
+}
+
+static int rsb_do_trans(void)
+{
+       struct sunxi_rsb_reg * const rsb =
+               (struct sunxi_rsb_reg *)SUNXI_RSB_BASE;
+
+       setbits_le32(&rsb->ctrl, RSB_CTRL_START_TRANS);
+       return rsb_await_trans();
+}
+
+int rsb_set_device_address(u16 device_addr, u16 runtime_addr)
+{
+       struct sunxi_rsb_reg * const rsb =
+               (struct sunxi_rsb_reg *)SUNXI_RSB_BASE;
+
+       writel(RSB_DEVADDR_RUNTIME_ADDR(runtime_addr) |
+              RSB_DEVADDR_DEVICE_ADDR(device_addr), &rsb->devaddr);
+       writel(RSB_CMD_SET_RTSADDR, &rsb->cmd);
+
+       return rsb_do_trans();
+}
+
+int rsb_write(const u16 runtime_device_addr, const u8 reg_addr, u8 data)
+{
+       struct sunxi_rsb_reg * const rsb =
+               (struct sunxi_rsb_reg *)SUNXI_RSB_BASE;
+
+       writel(RSB_DEVADDR_RUNTIME_ADDR(runtime_device_addr), &rsb->devaddr);
+       writel(reg_addr, &rsb->addr);
+       writel(data, &rsb->data);
+       writel(RSB_CMD_BYTE_WRITE, &rsb->cmd);
+
+       return rsb_do_trans();
+}
+
+int rsb_read(const u16 runtime_device_addr, const u8 reg_addr, u8 *data)
+{
+       struct sunxi_rsb_reg * const rsb =
+               (struct sunxi_rsb_reg *)SUNXI_RSB_BASE;
+       int ret;
+
+       writel(RSB_DEVADDR_RUNTIME_ADDR(runtime_device_addr), &rsb->devaddr);
+       writel(reg_addr, &rsb->addr);
+       writel(RSB_CMD_BYTE_READ, &rsb->cmd);
+
+       ret = rsb_do_trans();
+       if (ret)
+               return ret;
+
+       *data = readl(&rsb->data) & 0xff;
+
+       return 0;
+}

arch/arm/include/asm/arch-sunxi/cpu.h +0 -0

index 9500262..49c1614 100644

arch/arm/include/asm/arch-sunxi/cpu.h +0 -0

 /* CoreSight Debug Module */
 #define SUNXI_CSDM_BASE                        0x3f500000

arch/arm/include/asm/arch-sunxi/gpio.h +2 -0

index 9f972ce..6623f15 100644
@@ -176,6 +176,8 @@ enum sunxi_gpio_number {
 #define SUN6I_GPL0_R_P2WI_SCK  3
 #define SUN6I_GPL1_R_P2WI_SDA  3
 
+#define SUN8I_GPL0_R_RSB_SCK   2
+#define SUN8I_GPL1_R_RSB_SDA   2
 #define SUN8I_GPL2_R_UART_TX   2
 #define SUN8I_GPL3_R_UART_RX   2
 

arch/arm/include/asm/arch-sunxi/prcm.h +2 -1

index 88de1ff..82ed541 100644
@@ -50,7 +50,8 @@
 #define PRCM_APB0_GATE_PIO (0x1 << 0)
 #define PRCM_APB0_GATE_IR (0x1 << 1)
 #define PRCM_APB0_GATE_TIMER01 (0x1 << 2)
-#define PRCM_APB0_GATE_P2WI (0x1 << 3)
+#define PRCM_APB0_GATE_P2WI (0x1 << 3)         /* sun6i */
+#define PRCM_APB0_GATE_RSB (0x1 << 3)          /* sun8i */
 #define PRCM_APB0_GATE_UART (0x1 << 4)
 #define PRCM_APB0_GATE_1WIRE (0x1 << 5)
 #define PRCM_APB0_GATE_I2C (0x1 << 6)

arch/arm/include/asm/arch-sunxi/rsb.h new file +55 -1

new file mode 100644
index 0000000..95a595a
@@ -0,0 +1,55 @@
+/*
+ * (C) Copyright 2014 Hans de Goede <[email protected]>
+ *
+ * Based on allwinner u-boot sources rsb code which is:
+ * (C) Copyright 2007-2013
+ * Allwinner Technology Co., Ltd. <www.allwinnertech.com>
+ * lixiang <[email protected]>
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+
+#ifndef __SUNXI_RSB_H
+#define __SUNXI_RSB_H
+
+#include <common.h>
+#include <asm/io.h>
+
+struct sunxi_rsb_reg {
+       u32 ctrl;       /* 0x00 */
+       u32 ccr;        /* 0x04 */
+       u32 inte;       /* 0x08 */
+       u32 stat;       /* 0x0c */
+       u32 addr;       /* 0x10 */
+       u8 res0[8];     /* 0x14 */
+       u32 data;       /* 0x1c */
+       u8 res1[4];     /* 0x20 */
+       u32 lcr;        /* 0x24 */
+       u32 dmcr;       /* 0x28 */
+       u32 cmd;        /* 0x2c */
+       u32 devaddr;    /* 0x30 */
+};
+
+#define RSB_CTRL_SOFT_RST              (1 << 0)
+#define RSB_CTRL_START_TRANS           (1 << 7)
+
+#define RSB_STAT_TOVER_INT             (1 << 0)
+#define RSB_STAT_TERR_INT              (1 << 1)
+#define RSB_STAT_LBSY_INT              (1 << 2)
+
+#define RSB_DMCR_DEVICE_MODE_START     (1 << 31)
+
+#define RSB_CMD_BYTE_WRITE             0x4e
+#define RSB_CMD_BYTE_READ              0x8b
+#define RSB_CMD_SET_RTSADDR            0xe8
+
+#define RSB_DEVADDR_RUNTIME_ADDR(x)    ((x) << 16)
+#define RSB_DEVADDR_DEVICE_ADDR(x)     ((x) << 0)
+
+void rsb_init(void);
+int rsb_set_device_mode(u32 device_mode_data);
+int rsb_set_device_address(u16 device_addr, u16 runtime_addr);
+int rsb_write(const u16 runtime_device_addr, const u8 reg_addr, u8 data);
+int rsb_read(const u16 runtime_device_addr, const u8 reg_addr, u8 *data);
+
+#endif
--

2.1.0

Hi,

Quoted 4 lines · Hans de Goede

sun8i (A23) introduces a new bus for communicating with the pmic, the rsb, the rsb is also used to communicate with the pmic on the A80, and is documented in the A80 user manual.

Quoted 4 lines

This commit adds support for this based on the rsb driver from the allwinner u-boot sources.

Note: Not yet ready for upstream, code needs some significant cleanups.

Confusing note.

Quoted 81 lines

Signed-off-by Hans de Goede <[email protected]>

---
  arch/arm/cpu/armv7/sunxi/Makefile      |   1 +
  arch/arm/cpu/armv7/sunxi/rsb.c         | 158 +++++++++++++++++++++++++++++++++
  arch/arm/include/asm/arch-sunxi/cpu.h  |   3 +-
  arch/arm/include/asm/arch-sunxi/gpio.h |   2 +
  arch/arm/include/asm/arch-sunxi/prcm.h |   3 +-
  arch/arm/include/asm/arch-sunxi/rsb.h  |  55 ++++++++++++
  6 files changed, 220 insertions(+), 2 deletions(-)

create mode 100644 arch/arm/cpu/armv7/sunxi/rsb.c create mode 100644 arch/arm/include/asm/arch-sunxi/rsb.h

arch/arm/cpu/armv7/sunxi/Makefile +1 -0

index 1337b60..3e8975a 100644
@@ -15,6 +15,7 @@ obj-y += pinmux.o
  obj-$(CONFIG_MACH_SUN6I)       += prcm.o
  obj-$(CONFIG_MACH_SUN8I)       += prcm.o
  obj-$(CONFIG_MACH_SUN6I)       += p2wi.o
+obj-$(CONFIG_MACH_SUN8I)       += rsb.o
  obj-$(CONFIG_MACH_SUN4I)       += clock_sun4i.o
  obj-$(CONFIG_MACH_SUN5I)       += clock_sun4i.o
  obj-$(CONFIG_MACH_SUN6I)       += clock_sun6i.o

arch/arm/cpu/armv7/sunxi/rsb.c new file +49 -0

new file mode 100644
index 0000000..b72bb9d
@@ -0,0 +1,158 @@
+/*
+ * (C) Copyright 2014 Hans de Goede <[email protected]>
+ *
+ * Based on allwinner u-boot sources rsb code which is:
+ * (C) Copyright 2007-2013
+ * Allwinner Technology Co., Ltd. <www.allwinnertech.com>
+ * lixiang <[email protected]>
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+
+#include <common.h>
+#include <errno.h>
+#include <asm/arch/cpu.h>
+#include <asm/arch/gpio.h>
+#include <asm/arch/prcm.h>
+#include <asm/arch/rsb.h>
+
+static void rsb_cfg_io(void)
+{
+       sunxi_gpio_set_cfgpin(SUNXI_GPL(0), SUN8I_GPL0_R_RSB_SCK);
+       sunxi_gpio_set_cfgpin(SUNXI_GPL(1), SUN8I_GPL1_R_RSB_SDA);
+       sunxi_gpio_set_pull(SUNXI_GPL(0), 1);
+       sunxi_gpio_set_pull(SUNXI_GPL(1), 1);
+       sunxi_gpio_set_drv(SUNXI_GPL(0), 2);
+       sunxi_gpio_set_drv(SUNXI_GPL(1), 2);
+}
+
+static void rsb_set_clk(void)
+{
+       struct sunxi_rsb_reg * const rsb =
+               (struct sunxi_rsb_reg *)SUNXI_RSB_BASE;
+       u32 div = 0;
+       u32 cd_odly = 0;
+
+       /* Source is Hosc24M, set RSB clk to 3Mhz */
+       div = 24000000 / 3000000 / 2 - 1;
+       cd_odly = div >> 1;
+       if (!cd_odly)
+               cd_odly = 1;
+
+       writel((cd_odly << 8) | div, &rsb->ccr);
+}
+
+void rsb_init(void)
+{
+       struct sunxi_rsb_reg * const rsb =
+               (struct sunxi_rsb_reg *)SUNXI_RSB_BASE;
+

arch/arm/cpu/armv7/sunxi/rsb.c line 55

Quoted 6 lines

arch/arm/cpu/armv7/sunxi/rsb.c new file +6 -0

+       rsb_cfg_io();
+
+       /* Enable RSB and PIO clk, and de-assert their resets */
+       prcm_apb0_enable(PRCM_APB0_GATE_PIO | PRCM_APB0_GATE_RSB);
+
+       writel(RSB_CTRL_SOFT_RST, &rsb->ctrl);

Do we need a small delay here for the device to finish reset?

Quoted 109 lines

arch/arm/cpu/armv7/sunxi/rsb.c new file +103 -0

+       rsb_set_clk();
+}
+
+static int rsb_await_trans(void)
+{
+       struct sunxi_rsb_reg * const rsb =
+               (struct sunxi_rsb_reg *)SUNXI_RSB_BASE;
+       unsigned long tmo = timer_get_us() + 1000000;
+       u32 stat;
+       int ret;
+
+       while (1) {
+               stat = readl(&rsb->stat);
+               if (stat & RSB_STAT_LBSY_INT) {
+                       ret = -EBUSY;
+                       break;
+               }
+               if (stat & RSB_STAT_TERR_INT) {
+                       ret = -EIO;
+                       break;
+               }
+               if (stat & RSB_STAT_TOVER_INT) {
+                       ret = 0;
+                       break;
+               }
+               if (timer_get_us() > tmo) {
+                       ret = -ETIME;
+                       break;
+               }
+       }
+       writel(stat, &rsb->stat); /* Clear status bits */
+
+       return ret;
+}
+
+int rsb_set_device_mode(u32 device_mode_data)
+{
+       struct sunxi_rsb_reg * const rsb =
+               (struct sunxi_rsb_reg *)SUNXI_RSB_BASE;
+       unsigned long tmo = timer_get_us() + 1000000;
+
+       writel(RSB_DMCR_DEVICE_MODE_START | device_mode_data, &rsb->dmcr);
+
+       while (readl(&rsb->dmcr) & RSB_DMCR_DEVICE_MODE_START) {
+               if (timer_get_us() > tmo)
+                       return -ETIME;
+       }
+
+       return rsb_await_trans();
+}
+
+static int rsb_do_trans(void)
+{
+       struct sunxi_rsb_reg * const rsb =
+               (struct sunxi_rsb_reg *)SUNXI_RSB_BASE;
+
+       setbits_le32(&rsb->ctrl, RSB_CTRL_START_TRANS);
+       return rsb_await_trans();
+}
+
+int rsb_set_device_address(u16 device_addr, u16 runtime_addr)
+{
+       struct sunxi_rsb_reg * const rsb =
+               (struct sunxi_rsb_reg *)SUNXI_RSB_BASE;
+
+       writel(RSB_DEVADDR_RUNTIME_ADDR(runtime_addr) |
+              RSB_DEVADDR_DEVICE_ADDR(device_addr), &rsb->devaddr);
+       writel(RSB_CMD_SET_RTSADDR, &rsb->cmd);
+
+       return rsb_do_trans();
+}
+
+int rsb_write(const u16 runtime_device_addr, const u8 reg_addr, u8 data)
+{
+       struct sunxi_rsb_reg * const rsb =
+               (struct sunxi_rsb_reg *)SUNXI_RSB_BASE;
+
+       writel(RSB_DEVADDR_RUNTIME_ADDR(runtime_device_addr), &rsb->devaddr);
+       writel(reg_addr, &rsb->addr);
+       writel(data, &rsb->data);
+       writel(RSB_CMD_BYTE_WRITE, &rsb->cmd);
+
+       return rsb_do_trans();
+}
+
+int rsb_read(const u16 runtime_device_addr, const u8 reg_addr, u8 *data)
+{
+       struct sunxi_rsb_reg * const rsb =
+               (struct sunxi_rsb_reg *)SUNXI_RSB_BASE;
+       int ret;
+
+       writel(RSB_DEVADDR_RUNTIME_ADDR(runtime_device_addr), &rsb->devaddr);
+       writel(reg_addr, &rsb->addr);
+       writel(RSB_CMD_BYTE_READ, &rsb->cmd);
+
+       ret = rsb_do_trans();
+       if (ret)
+               return ret;
+
+       *data = readl(&rsb->data) & 0xff;
+
+       return 0;
+}

arch/arm/include/asm/arch-sunxi/cpu.h +0 -0

index 9500262..49c1614 100644

arch/arm/include/asm/arch-sunxi/cpu.h line 132

Quoted 6 lines

arch/arm/include/asm/arch-sunxi/cpu.h +2 -1

@@ -128,7 +128,8 @@
  #define SUN6I_CPUCFG_BASE              0x01f01c00
  #define SUNXI_R_UART_BASE              0x01f02800
  #define SUNXI_R_PIO_BASE               0x01f02c00
-#define SUNXI_P2WI_BASE                        0x01f03400
+#define SUNXI_P2WI_BASE                        0x01f03400      /* on sun6i */
+#define SUNXI_RSB_BASE                 0x01f03400      /* on sun8i */

Maybe we just use SUN6I_ SUN8I_ prefixes?
The base address will be different on sun9i.

Quoted 87 lines

arch/arm/include/asm/arch-sunxi/cpu.h +0 -0

  /* CoreSight Debug Module */
  #define SUNXI_CSDM_BASE                        0x3f500000

arch/arm/include/asm/arch-sunxi/gpio.h +2 -0

index 9f972ce..6623f15 100644
@@ -176,6 +176,8 @@ enum sunxi_gpio_number {
  #define SUN6I_GPL0_R_P2WI_SCK  3
  #define SUN6I_GPL1_R_P2WI_SDA  3
 
+#define SUN8I_GPL0_R_RSB_SCK   2
+#define SUN8I_GPL1_R_RSB_SDA   2
  #define SUN8I_GPL2_R_UART_TX   2
  #define SUN8I_GPL3_R_UART_RX   2
 

arch/arm/include/asm/arch-sunxi/prcm.h +2 -1

index 88de1ff..82ed541 100644
@@ -50,7 +50,8 @@
  #define PRCM_APB0_GATE_PIO (0x1 << 0)
  #define PRCM_APB0_GATE_IR (0x1 << 1)
  #define PRCM_APB0_GATE_TIMER01 (0x1 << 2)
-#define PRCM_APB0_GATE_P2WI (0x1 << 3)
+#define PRCM_APB0_GATE_P2WI (0x1 << 3)         /* sun6i */
+#define PRCM_APB0_GATE_RSB (0x1 << 3)          /* sun8i */
  #define PRCM_APB0_GATE_UART (0x1 << 4)
  #define PRCM_APB0_GATE_1WIRE (0x1 << 5)
  #define PRCM_APB0_GATE_I2C (0x1 << 6)

arch/arm/include/asm/arch-sunxi/rsb.h new file +51 -0

new file mode 100644
index 0000000..95a595a
@@ -0,0 +1,55 @@
+/*
+ * (C) Copyright 2014 Hans de Goede <[email protected]>
+ *
+ * Based on allwinner u-boot sources rsb code which is:
+ * (C) Copyright 2007-2013
+ * Allwinner Technology Co., Ltd. <www.allwinnertech.com>
+ * lixiang <[email protected]>
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+
+#ifndef __SUNXI_RSB_H
+#define __SUNXI_RSB_H
+
+#include <common.h>
+#include <asm/io.h>
+
+struct sunxi_rsb_reg {
+       u32 ctrl;       /* 0x00 */
+       u32 ccr;        /* 0x04 */
+       u32 inte;       /* 0x08 */
+       u32 stat;       /* 0x0c */
+       u32 addr;       /* 0x10 */
+       u8 res0[8];     /* 0x14 */
+       u32 data;       /* 0x1c */
+       u8 res1[4];     /* 0x20 */
+       u32 lcr;        /* 0x24 */
+       u32 dmcr;       /* 0x28 */
+       u32 cmd;        /* 0x2c */
+       u32 devaddr;    /* 0x30 */
+};
+
+#define RSB_CTRL_SOFT_RST              (1 << 0)
+#define RSB_CTRL_START_TRANS           (1 << 7)
+
+#define RSB_STAT_TOVER_INT             (1 << 0)
+#define RSB_STAT_TERR_INT              (1 << 1)
+#define RSB_STAT_LBSY_INT              (1 << 2)
+
+#define RSB_DMCR_DEVICE_MODE_START     (1 << 31)
+
+#define RSB_CMD_BYTE_WRITE             0x4e
+#define RSB_CMD_BYTE_READ              0x8b
+#define RSB_CMD_SET_RTSADDR            0xe8
+
+#define RSB_DEVADDR_RUNTIME_ADDR(x)    ((x) << 16)
+#define RSB_DEVADDR_DEVICE_ADDR(x)     ((x) << 0)
+
+void rsb_init(void);
+int rsb_set_device_mode(u32 device_mode_data);
+int rsb_set_device_address(u16 device_addr, u16 runtime_addr);

Quoted 6 lines

arch/arm/include/asm/arch-sunxi/rsb.h new file +4 -1

+int rsb_write(const u16 runtime_device_addr, const u8 reg_addr, u8 data);
+int rsb_read(const u16 runtime_device_addr, const u8 reg_addr, u8 *data);
+
+#endif
--

2.1.0

The rest looks good. Thanks!

Hi,

Quoted 10 lines · Chen-Yu Tsai

Hi,

Quoted 7 lines · Hans de Goede

sun8i (A23) introduces a new bus for communicating with the pmic, the rsb, the rsb is also used to communicate with the pmic on the A80, and is documented in the A80 user manual.

This commit adds support for this based on the rsb driver from the allwinner u-boot sources.

Quoted 3 lines

Quoted 1 line

Note: Not yet ready for upstream, code needs some significant cleanups.

Confusing note.

Ugh, I forgot to remove that note after doing the cleanups, I'll remove it from the version of the patch my personal tree :)

Quoted 10 lines

arch/arm/include/asm/arch-sunxi/cpu.h line 132

Quoted 7 lines

arch/arm/include/asm/arch-sunxi/cpu.h +2 -1

@@ -128,7 +128,8 @@
  #define SUN6I_CPUCFG_BASE              0x01f01c00
  #define SUNXI_R_UART_BASE              0x01f02800
  #define SUNXI_R_PIO_BASE               0x01f02c00
-#define SUNXI_P2WI_BASE                        0x01f03400
+#define SUNXI_P2WI_BASE                        0x01f03400      /* on sun6i */
+#define SUNXI_RSB_BASE                 0x01f03400      /* on sun8i */

Maybe we just use SUN6I_ SUN8I_ prefixes?
The base address will be different on sun9i.

Quoted 10 lines · Hans de Goede

Quoted 10 lines

arch/arm/include/asm/arch-sunxi/cpu.h line 132

Quoted 7 lines

arch/arm/include/asm/arch-sunxi/cpu.h +2 -1

@@ -128,7 +128,8 @@
   #define SUN6I_CPUCFG_BASE              0x01f01c00
   #define SUNXI_R_UART_BASE              0x01f02800
   #define SUNXI_R_PIO_BASE               0x01f02c00
-#define SUNXI_P2WI_BASE                        0x01f03400
+#define SUNXI_P2WI_BASE                        0x01f03400      /* on sun6i */
+#define SUNXI_RSB_BASE                 0x01f03400      /* on sun8i */

Maybe we just use SUN6I_ SUN8I_ prefixes?
The base address will be different on sun9i.

Did you miss this suggestions from Chen-Yu? Seems like a good one.

The axp223 appears to be the same as the axp221, except that it uses the rsb to communicate rather then the p2wi. At least all the registers we use are 100% the same.

Signed-off-by Hans de Goede <[email protected]>

---
 arch/arm/cpu/armv7/sunxi/cpu_info.c |  2 +-
 drivers/power/Kconfig               |  8 ++--
 drivers/power/axp221.c              | 88 ++++++++++++++++++++++++++++---------
 include/axp221.h                    |  4 ++
 4 files changed, 76 insertions(+), 26 deletions(-)

arch/arm/cpu/armv7/sunxi/cpu_info.c +1 -1

index 7a3a4ca..b6cb9de 100644
@@ -76,7 +76,7 @@ int print_cpuinfo(void)
 
 int sunxi_get_sid(unsigned int *sid)
 {
-#ifdef CONFIG_MACH_SUN6I
+#if defined CONFIG_MACH_SUN6I || defined CONFIG_MACH_SUN8I
 #ifdef CONFIG_AXP221_POWER
        return axp221_get_sid(sid);
 #else

drivers/power/Kconfig +4 -4

index 1ec7c0e..7373a79 100644
@@ -1,10 +1,10 @@
 config AXP221_POWER
-       boolean "axp221 pmic support"
-       depends on MACH_SUN6I
+       boolean "axp221 / axp223 pmic support"
+       depends on MACH_SUN6I || MACH_SUN8I
        default y
        ---help---
-       Say y here to enable support for the axp221 pmic found on most sun6i
-       (A31) boards.
+       Say y here to enable support for the axp221 / axp223 pmic found on most
+       sun6i (A31) / sun8i (A23) boards.
 
 config AXP221_DLDO1_VOLT
        int "axp221 dldo1 voltage"

drivers/power/axp221.c +67 -21

index 826567a..717adad 100644
@@ -1,4 +1,10 @@
 /*
+ * AXP221 and AXP223 driver
+ *
+ * IMPORTANT when making changes to this file check that the registers
+ * used are the same for the axp221 and axp223.
+ *
+ * (C) Copyright 2014 Hans de Goede <[email protected]>
  * (C) Copyright 2013 Oliver Schinagl <[email protected]>
  *
  * SPDX-License-Identifier:    GPL-2.0+

@@ -7,8 +13,50 @@
 #include <common.h>
 #include <errno.h>
 #include <asm/arch/p2wi.h>
+#include <asm/arch/rsb.h>
 #include <axp221.h>
 
+/*
+ * The axp221 uses the p2wi bus, the axp223 is identical (for all registers
+ * used sofar) but uses the rsb bus. These functions abstract this.
+ */
+static int pmic_bus_init(void)
+{
+#ifdef CONFIG_MACH_SUN6I
+       p2wi_init();
+       return p2wi_change_to_p2wi_mode(AXP221_CHIP_ADDR, AXP221_CTRL_ADDR,
+                                       AXP221_INIT_DATA);
+#else
+       int ret;
+
+       rsb_init();
+
+       ret = rsb_set_device_mode(AXP223_DEVICE_MODE_DATA);
+       if (ret)
+               return ret;
+
+       return rsb_set_device_address(AXP223_DEVICE_ADDR, AXP223_RUNTIME_ADDR);
+#endif
+}
+
+static int pmic_bus_read(const u8 addr, u8 *data)
+{
+#ifdef CONFIG_MACH_SUN6I
+       return p2wi_read(addr, data);
+#else
+       return rsb_read(AXP223_RUNTIME_ADDR, addr, data);
+#endif
+}
+
+static int pmic_bus_write(const u8 addr, u8 data)
+{
+#ifdef CONFIG_MACH_SUN6I
+       return p2wi_write(addr, data);
+#else
+       return rsb_write(AXP223_RUNTIME_ADDR, addr, data);
+#endif
+}
+
 static u8 axp221_mvolt_to_cfg(int mvolt, int min, int max, int div)
 {
        if (mvolt < min)

@@ -24,12 +72,12 @@ static int axp221_setbits(u8 reg, u8 bits)
        int ret;
        u8 val;
 
-       ret = p2wi_read(reg, &val);
+       ret = pmic_bus_read(reg, &val);
        if (ret)
                return ret;
 
        val |= bits;
-       return p2wi_write(reg, val);
+       return pmic_bus_write(reg, val);
 }
 
 int axp221_set_dcdc1(unsigned int mvolt)

@@ -37,7 +85,7 @@ int axp221_set_dcdc1(unsigned int mvolt)
        int ret;
        u8 cfg = axp221_mvolt_to_cfg(mvolt, 1600, 3400, 100);
 
-       ret = p2wi_write(AXP221_DCDC1_CTRL, cfg);
+       ret = pmic_bus_write(AXP221_DCDC1_CTRL, cfg);
        if (ret)
                return ret;
 

@@ -49,28 +97,28 @@ int axp221_set_dcdc2(unsigned int mvolt)
 {
        u8 cfg = axp221_mvolt_to_cfg(mvolt, 600, 1540, 20);
 
-       return p2wi_write(AXP221_DCDC2_CTRL, cfg);
+       return pmic_bus_write(AXP221_DCDC2_CTRL, cfg);
 }
 
 int axp221_set_dcdc3(unsigned int mvolt)
 {
        u8 cfg = axp221_mvolt_to_cfg(mvolt, 600, 1860, 20);
 
-       return p2wi_write(AXP221_DCDC3_CTRL, cfg);
+       return pmic_bus_write(AXP221_DCDC3_CTRL, cfg);
 }
 
 int axp221_set_dcdc4(unsigned int mvolt)
 {
        u8 cfg = axp221_mvolt_to_cfg(mvolt, 600, 1540, 20);
 
-       return p2wi_write(AXP221_DCDC4_CTRL, cfg);
+       return pmic_bus_write(AXP221_DCDC4_CTRL, cfg);
 }
 
 int axp221_set_dcdc5(unsigned int mvolt)
 {
        u8 cfg = axp221_mvolt_to_cfg(mvolt, 1000, 2550, 50);
 
-       return p2wi_write(AXP221_DCDC5_CTRL, cfg);
+       return pmic_bus_write(AXP221_DCDC5_CTRL, cfg);
 }
 
 int axp221_set_dldo1(unsigned int mvolt)

@@ -78,7 +126,7 @@ int axp221_set_dldo1(unsigned int mvolt)
        int ret;
        u8 cfg = axp221_mvolt_to_cfg(mvolt, 700, 3300, 100);
 
-       ret = p2wi_write(AXP221_DLDO1_CTRL, cfg);
+       ret = pmic_bus_write(AXP221_DLDO1_CTRL, cfg);
        if (ret)
                return ret;
 

@@ -91,7 +139,7 @@ int axp221_set_dldo2(unsigned int mvolt)
        int ret;
        u8 cfg = axp221_mvolt_to_cfg(mvolt, 700, 3300, 100);
 
-       ret = p2wi_write(AXP221_DLDO2_CTRL, cfg);
+       ret = pmic_bus_write(AXP221_DLDO2_CTRL, cfg);
        if (ret)
                return ret;
 

@@ -104,7 +152,7 @@ int axp221_set_dldo3(unsigned int mvolt)
        int ret;
        u8 cfg = axp221_mvolt_to_cfg(mvolt, 700, 3300, 100);
 
-       ret = p2wi_write(AXP221_DLDO3_CTRL, cfg);
+       ret = pmic_bus_write(AXP221_DLDO3_CTRL, cfg);
        if (ret)
                return ret;
 

@@ -117,7 +165,7 @@ int axp221_set_dldo4(unsigned int mvolt)
        int ret;
        u8 cfg = axp221_mvolt_to_cfg(mvolt, 700, 3300, 100);
 
-       ret = p2wi_write(AXP221_DLDO4_CTRL, cfg);
+       ret = pmic_bus_write(AXP221_DLDO4_CTRL, cfg);
        if (ret)
                return ret;
 

@@ -130,7 +178,7 @@ int axp221_set_aldo1(unsigned int mvolt)
        int ret;
        u8 cfg = axp221_mvolt_to_cfg(mvolt, 700, 3300, 100);
 
-       ret = p2wi_write(AXP221_ALDO1_CTRL, cfg);
+       ret = pmic_bus_write(AXP221_ALDO1_CTRL, cfg);
        if (ret)
                return ret;
 

@@ -143,7 +191,7 @@ int axp221_set_aldo2(unsigned int mvolt)
        int ret;
        u8 cfg = axp221_mvolt_to_cfg(mvolt, 700, 3300, 100);
 
-       ret = p2wi_write(AXP221_ALDO2_CTRL, cfg);
+       ret = pmic_bus_write(AXP221_ALDO2_CTRL, cfg);
        if (ret)
                return ret;
 

@@ -156,7 +204,7 @@ int axp221_set_aldo3(unsigned int mvolt)
        int ret;
        u8 cfg = axp221_mvolt_to_cfg(mvolt, 700, 3300, 100);
 
-       ret = p2wi_write(AXP221_ALDO3_CTRL, cfg);
+       ret = pmic_bus_write(AXP221_ALDO3_CTRL, cfg);
        if (ret)
                return ret;
 

@@ -169,13 +217,11 @@ int axp221_init(void)
        u8 axp_chip_id;
        int ret;
 
-       p2wi_init();
-       ret = p2wi_change_to_p2wi_mode(AXP221_CHIP_ADDR, AXP221_CTRL_ADDR,
-                                      AXP221_INIT_DATA);
+       ret = pmic_bus_init();
        if (ret)
                return ret;
 
-       ret = p2wi_read(AXP221_CHIP_ID, &axp_chip_id);
+       ret = pmic_bus_read(AXP221_CHIP_ID, &axp_chip_id);
        if (ret)
                return ret;
 

@@ -194,17 +240,17 @@ int axp221_get_sid(unsigned int *sid)
        if (ret)
                return ret;
 
-       ret = p2wi_write(AXP221_PAGE, 1);
+       ret = pmic_bus_write(AXP221_PAGE, 1);
        if (ret)
                return ret;
 
        for (i = 0; i < 16; i++) {
-               ret = p2wi_read(AXP221_SID + i, &dest[i]);
+               ret = pmic_bus_read(AXP221_SID + i, &dest[i]);
                if (ret)
                        return ret;
        }
 
-       p2wi_write(AXP221_PAGE, 0);
+       pmic_bus_write(AXP221_PAGE, 0);
 
        for (i = 0; i < 4; i++)
                sid[i] = be32_to_cpu(sid[i]);

include/axp221.h +4 -1

index db219c6..10d35e1 100644
@@ -10,6 +10,10 @@
 #define AXP221_CTRL_ADDR 0x3e
 #define AXP221_INIT_DATA 0x3e
 
+#define AXP223_DEVICE_ADDR 0x3a3
+#define AXP223_RUNTIME_ADDR 0x2d
+#define AXP223_DEVICE_MODE_DATA 0x7c3e00
+
 /* Page 0 addresses */
 #define AXP221_CHIP_ID         0x03
 #define AXP221_OUTPUT_CTRL1    0x10
--

2.1.0

Quoted 283 lines · Hans de Goede

The axp223 appears to be the same as the axp221, except that it uses the rsb to communicate rather then the p2wi. At least all the registers we use are 100% the same.

Signed-off-by Hans de Goede <[email protected]>

---
  arch/arm/cpu/armv7/sunxi/cpu_info.c |  2 +-
  drivers/power/Kconfig               |  8 ++--
  drivers/power/axp221.c              | 88 ++++++++++++++++++++++++++++---------
  include/axp221.h                    |  4 ++
  4 files changed, 76 insertions(+), 26 deletions(-)

arch/arm/cpu/armv7/sunxi/cpu_info.c +1 -1

index 7a3a4ca..b6cb9de 100644
@@ -76,7 +76,7 @@ int print_cpuinfo(void)
 
  int sunxi_get_sid(unsigned int *sid)
  {
-#ifdef CONFIG_MACH_SUN6I
+#if defined CONFIG_MACH_SUN6I || defined CONFIG_MACH_SUN8I
  #ifdef CONFIG_AXP221_POWER
         return axp221_get_sid(sid);
  #else

drivers/power/Kconfig +4 -4

index 1ec7c0e..7373a79 100644
@@ -1,10 +1,10 @@
  config AXP221_POWER
-       boolean "axp221 pmic support"
-       depends on MACH_SUN6I
+       boolean "axp221 / axp223 pmic support"
+       depends on MACH_SUN6I || MACH_SUN8I
         default y
         ---help---
-       Say y here to enable support for the axp221 pmic found on most sun6i
-       (A31) boards.
+       Say y here to enable support for the axp221 / axp223 pmic found on most
+       sun6i (A31) / sun8i (A23) boards.
 
  config AXP221_DLDO1_VOLT
         int "axp221 dldo1 voltage"

drivers/power/axp221.c +67 -21

index 826567a..717adad 100644
@@ -1,4 +1,10 @@
  /*
+ * AXP221 and AXP223 driver
+ *
+ * IMPORTANT when making changes to this file check that the registers
+ * used are the same for the axp221 and axp223.
+ *
+ * (C) Copyright 2014 Hans de Goede <[email protected]>
   * (C) Copyright 2013 Oliver Schinagl <[email protected]>
   *
   * SPDX-License-Identifier:    GPL-2.0+

@@ -7,8 +13,50 @@
  #include <common.h>
  #include <errno.h>
  #include <asm/arch/p2wi.h>
+#include <asm/arch/rsb.h>
  #include <axp221.h>
 
+/*
+ * The axp221 uses the p2wi bus, the axp223 is identical (for all registers
+ * used sofar) but uses the rsb bus. These functions abstract this.
+ */
+static int pmic_bus_init(void)
+{
+#ifdef CONFIG_MACH_SUN6I
+       p2wi_init();
+       return p2wi_change_to_p2wi_mode(AXP221_CHIP_ADDR, AXP221_CTRL_ADDR,
+                                       AXP221_INIT_DATA);
+#else
+       int ret;
+
+       rsb_init();
+
+       ret = rsb_set_device_mode(AXP223_DEVICE_MODE_DATA);
+       if (ret)
+               return ret;
+
+       return rsb_set_device_address(AXP223_DEVICE_ADDR, AXP223_RUNTIME_ADDR);
+#endif
+}
+
+static int pmic_bus_read(const u8 addr, u8 *data)
+{
+#ifdef CONFIG_MACH_SUN6I
+       return p2wi_read(addr, data);
+#else
+       return rsb_read(AXP223_RUNTIME_ADDR, addr, data);
+#endif
+}
+
+static int pmic_bus_write(const u8 addr, u8 data)
+{
+#ifdef CONFIG_MACH_SUN6I
+       return p2wi_write(addr, data);
+#else
+       return rsb_write(AXP223_RUNTIME_ADDR, addr, data);
+#endif
+}
+
  static u8 axp221_mvolt_to_cfg(int mvolt, int min, int max, int div)
  {
         if (mvolt < min)

@@ -24,12 +72,12 @@ static int axp221_setbits(u8 reg, u8 bits)
         int ret;
         u8 val;
 
-       ret = p2wi_read(reg, &val);
+       ret = pmic_bus_read(reg, &val);
         if (ret)
                 return ret;
 
         val |= bits;
-       return p2wi_write(reg, val);
+       return pmic_bus_write(reg, val);
  }
 
  int axp221_set_dcdc1(unsigned int mvolt)

@@ -37,7 +85,7 @@ int axp221_set_dcdc1(unsigned int mvolt)
         int ret;
         u8 cfg = axp221_mvolt_to_cfg(mvolt, 1600, 3400, 100);
 
-       ret = p2wi_write(AXP221_DCDC1_CTRL, cfg);
+       ret = pmic_bus_write(AXP221_DCDC1_CTRL, cfg);
         if (ret)
                 return ret;
 

@@ -49,28 +97,28 @@ int axp221_set_dcdc2(unsigned int mvolt)
  {
         u8 cfg = axp221_mvolt_to_cfg(mvolt, 600, 1540, 20);
 
-       return p2wi_write(AXP221_DCDC2_CTRL, cfg);
+       return pmic_bus_write(AXP221_DCDC2_CTRL, cfg);
  }
 
  int axp221_set_dcdc3(unsigned int mvolt)
  {
         u8 cfg = axp221_mvolt_to_cfg(mvolt, 600, 1860, 20);
 
-       return p2wi_write(AXP221_DCDC3_CTRL, cfg);
+       return pmic_bus_write(AXP221_DCDC3_CTRL, cfg);
  }
 
  int axp221_set_dcdc4(unsigned int mvolt)
  {
         u8 cfg = axp221_mvolt_to_cfg(mvolt, 600, 1540, 20);
 
-       return p2wi_write(AXP221_DCDC4_CTRL, cfg);
+       return pmic_bus_write(AXP221_DCDC4_CTRL, cfg);
  }
 
  int axp221_set_dcdc5(unsigned int mvolt)
  {
         u8 cfg = axp221_mvolt_to_cfg(mvolt, 1000, 2550, 50);
 
-       return p2wi_write(AXP221_DCDC5_CTRL, cfg);
+       return pmic_bus_write(AXP221_DCDC5_CTRL, cfg);
  }
 
  int axp221_set_dldo1(unsigned int mvolt)

@@ -78,7 +126,7 @@ int axp221_set_dldo1(unsigned int mvolt)
         int ret;
         u8 cfg = axp221_mvolt_to_cfg(mvolt, 700, 3300, 100);
 
-       ret = p2wi_write(AXP221_DLDO1_CTRL, cfg);
+       ret = pmic_bus_write(AXP221_DLDO1_CTRL, cfg);
         if (ret)
                 return ret;
 

@@ -91,7 +139,7 @@ int axp221_set_dldo2(unsigned int mvolt)
         int ret;
         u8 cfg = axp221_mvolt_to_cfg(mvolt, 700, 3300, 100);
 
-       ret = p2wi_write(AXP221_DLDO2_CTRL, cfg);
+       ret = pmic_bus_write(AXP221_DLDO2_CTRL, cfg);
         if (ret)
                 return ret;
 

@@ -104,7 +152,7 @@ int axp221_set_dldo3(unsigned int mvolt)
         int ret;
         u8 cfg = axp221_mvolt_to_cfg(mvolt, 700, 3300, 100);
 
-       ret = p2wi_write(AXP221_DLDO3_CTRL, cfg);
+       ret = pmic_bus_write(AXP221_DLDO3_CTRL, cfg);
         if (ret)
                 return ret;
 

@@ -117,7 +165,7 @@ int axp221_set_dldo4(unsigned int mvolt)
         int ret;
         u8 cfg = axp221_mvolt_to_cfg(mvolt, 700, 3300, 100);
 
-       ret = p2wi_write(AXP221_DLDO4_CTRL, cfg);
+       ret = pmic_bus_write(AXP221_DLDO4_CTRL, cfg);
         if (ret)
                 return ret;
 

@@ -130,7 +178,7 @@ int axp221_set_aldo1(unsigned int mvolt)
         int ret;
         u8 cfg = axp221_mvolt_to_cfg(mvolt, 700, 3300, 100);
 
-       ret = p2wi_write(AXP221_ALDO1_CTRL, cfg);
+       ret = pmic_bus_write(AXP221_ALDO1_CTRL, cfg);
         if (ret)
                 return ret;
 

@@ -143,7 +191,7 @@ int axp221_set_aldo2(unsigned int mvolt)
         int ret;
         u8 cfg = axp221_mvolt_to_cfg(mvolt, 700, 3300, 100);
 
-       ret = p2wi_write(AXP221_ALDO2_CTRL, cfg);
+       ret = pmic_bus_write(AXP221_ALDO2_CTRL, cfg);
         if (ret)
                 return ret;
 

@@ -156,7 +204,7 @@ int axp221_set_aldo3(unsigned int mvolt)
         int ret;
         u8 cfg = axp221_mvolt_to_cfg(mvolt, 700, 3300, 100);
 
-       ret = p2wi_write(AXP221_ALDO3_CTRL, cfg);
+       ret = pmic_bus_write(AXP221_ALDO3_CTRL, cfg);
         if (ret)
                 return ret;
 

@@ -169,13 +217,11 @@ int axp221_init(void)
         u8 axp_chip_id;
         int ret;
 
-       p2wi_init();
-       ret = p2wi_change_to_p2wi_mode(AXP221_CHIP_ADDR, AXP221_CTRL_ADDR,
-                                      AXP221_INIT_DATA);
+       ret = pmic_bus_init();
         if (ret)
                 return ret;
 
-       ret = p2wi_read(AXP221_CHIP_ID, &axp_chip_id);
+       ret = pmic_bus_read(AXP221_CHIP_ID, &axp_chip_id);
         if (ret)
                 return ret;
 

@@ -194,17 +240,17 @@ int axp221_get_sid(unsigned int *sid)
         if (ret)
                 return ret;
 
-       ret = p2wi_write(AXP221_PAGE, 1);
+       ret = pmic_bus_write(AXP221_PAGE, 1);
         if (ret)
                 return ret;
 
         for (i = 0; i < 16; i++) {
-               ret = p2wi_read(AXP221_SID + i, &dest[i]);
+               ret = pmic_bus_read(AXP221_SID + i, &dest[i]);
                 if (ret)
                         return ret;
         }
 
-       p2wi_write(AXP221_PAGE, 0);
+       pmic_bus_write(AXP221_PAGE, 0);
 
         for (i = 0; i < 4; i++)
                 sid[i] = be32_to_cpu(sid[i]);

include/axp221.h +4 -1

index db219c6..10d35e1 100644
@@ -10,6 +10,10 @@
  #define AXP221_CTRL_ADDR 0x3e
  #define AXP221_INIT_DATA 0x3e
 
+#define AXP223_DEVICE_ADDR 0x3a3
+#define AXP223_RUNTIME_ADDR 0x2d
+#define AXP223_DEVICE_MODE_DATA 0x7c3e00
+
  /* Page 0 addresses */
  #define AXP221_CHIP_ID         0x03
  #define AXP221_OUTPUT_CTRL1    0x10
--

Quoted 2 lines

2.1.0

git am reports:

Applying patch #422068 using 'git am'
Description: [U-Boot,03/14] sunxi: axp221: Add axp223 support
Applying: sunxi: axp221: Add axp223 support /home/wens/sunxi/u-boot/.git/rebase-apply/patch:50: trailing whitespace.
*
/home/wens/sunxi/u-boot/.git/rebase-apply/patch:76: trailing whitespace.

warning: 2 lines add whitespace errors.

The axp223 appears to be the same as the axp221, except that it uses the rsb to communicate rather then the p2wi. At least all the registers we use are 100% the same.

Signed-off-by Hans de Goede <[email protected]>

Some of the ldo-s of the axp221 are used in the same way on most boards, add comments to the Kconfig help text to reflect this, and give them defaults matching their typical usage.

Signed-off-by Hans de Goede <[email protected]>

The dcdc1 voltage is typically used as generic 3.3V IO voltage for things like GPIO-s, sdcard interfaces, etc. On most boards this is undervolted to 3.0V to safe battery, but not on all, make it configurable so that we can use the same settings as the original firmware on all boards.

Signed-off-by Hans de Goede <[email protected]>

Explicitly turn off unused voltages, rather then leaving them as is. Likewise explictly enabled the dcdc convertors, rather then assuming they are already enabled at boot.

Signed-off-by Hans de Goede <[email protected]>

dcdc4 is not used on sun8i, disable it.

While at it also add comments to the other fixed voltages to document what they are used for.

Signed-off-by Hans de Goede <[email protected]>

The sun8i dram code sometimes wants to enable sigma delta mode, add a parameter to allow this.

Signed-off-by Hans de Goede <[email protected]>

The sun8i dram code sometimes wants to enable sigma delta mode, add a parameter to allow this.

Signed-off-by Hans de Goede <[email protected]>

---
 arch/arm/cpu/armv7/sunxi/clock_sun6i.c        | 9 +++++++--
 arch/arm/cpu/armv7/sunxi/dram_sun6i.c         | 2 +-
 arch/arm/include/asm/arch-sunxi/clock.h       | 1 -
 arch/arm/include/asm/arch-sunxi/clock_sun6i.h | 5 +++++
 4 files changed, 13 insertions(+), 4 deletions(-)

arch/arm/cpu/armv7/sunxi/clock_sun6i.c +7 -2

index 8e949c6..193e314 100644
@@ -144,15 +144,20 @@ void clock_set_pll3(unsigned int clk)
 	       &ccm->pll3_cfg);
 }
 
-void clock_set_pll5(unsigned int clk)
+void clock_set_pll5(unsigned int clk, bool sigma_delta_enable)
 {
 	struct sunxi_ccm_reg * const ccm =
 		(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
 	const int k = 2;
 	const int m = 1;
 
+	if (sigma_delta_enable)
+		writel(CCM_PLL5_PATTERN, &ccm->pll5_pattern_cfg);
+
 	/* PLL5 rate = 24000000 * n * k / m */
-	writel(CCM_PLL5_CTRL_EN | CCM_PLL5_CTRL_UPD |
+	writel(CCM_PLL5_CTRL_EN |
+	       (sigma_delta_enable ? CCM_PLL5_CTRL_SIGMA_DELTA_EN : 0) |
+	       CCM_PLL5_CTRL_UPD |
 	       CCM_PLL5_CTRL_N(clk / (24000000 * k / m)) |
 	       CCM_PLL5_CTRL_K(k) | CCM_PLL5_CTRL_M(m), &ccm->pll5_cfg);
 

arch/arm/cpu/armv7/sunxi/dram_sun6i.c +0 -0

index 61bb8d4..bc6428a 100644

arch/arm/cpu/armv7/sunxi/dram_sun6i.c +0 -0

 	clrsetbits_le32(&ccm->dram_clk_cfg, CCM_DRAMCLK_CFG_DIV0_MASK,
 		CCM_DRAMCLK_CFG_DIV0(dram_clk_div) | CCM_DRAMCLK_CFG_RST |

arch/arm/include/asm/arch-sunxi/clock.h +0 -1

index 64acff3..505c363 100644
@@ -26,7 +26,6 @@ int clock_init(void);
 int clock_twi_onoff(int port, int state);
 void clock_set_pll1(unsigned int hz);
 void clock_set_pll3(unsigned int hz);
-void clock_set_pll5(unsigned int hz);
 unsigned int clock_get_pll5p(void);
 unsigned int clock_get_pll6(void);
 void clock_set_de_mod_clock(u32 *clk_cfg, unsigned int hz);

arch/arm/include/asm/arch-sunxi/clock_sun6i.h +5 -0

index 3d4fcd1..f807af3 100644
@@ -185,6 +185,7 @@ struct sunxi_ccm_reg {
 #define CCM_PLL5_CTRL_K(n)		((((n) - 1) & 0x3) << 4)
 #define CCM_PLL5_CTRL_N(n)		((((n) - 1) & 0x1f) << 8)
 #define CCM_PLL5_CTRL_UPD		(0x1 << 20)
+#define CCM_PLL5_CTRL_SIGMA_DELTA_EN	(0x1 << 24)
 #define CCM_PLL5_CTRL_EN		(0x1 << 31)
 
 #define PLL6_CFG_DEFAULT		0x90041811 /* 600 MHz */

@@ -274,6 +275,8 @@ struct sunxi_ccm_reg {
 
 #define MBUS_CLK_DEFAULT		0x81000001 /* PLL6 / 2 */
 
+#define CCM_PLL5_PATTERN		0xd1303333
+
 /* ahb_reset0 offsets */
 #define AHB_RESET_OFFSET_GMAC		17
 #define AHB_RESET_OFFSET_MCTL		14

@@ -308,4 +311,6 @@ struct sunxi_ccm_reg {
 #define CCM_DE_CTRL_PLL10		(5 << 24)
 #define CCM_DE_CTRL_GATE		(1 << 31)
 
+void clock_set_pll5(unsigned int clk, bool sigma_delta_enable);
+
 #endif /* _SUNXI_CLOCK_SUN6I_H */

The A23 (sun8i) requires different values for these then sun6i, so make them function parameters.

Signed-off-by Hans de Goede <[email protected]>

The A23 (sun8i) requires different values for these then sun6i, so make them function parameters.

Signed-off-by Hans de Goede <[email protected]>

---
 arch/arm/cpu/armv7/sunxi/clock_sun6i.c        | 4 +---
 arch/arm/cpu/armv7/sunxi/dram_sun6i.c         | 2 +-
 arch/arm/include/asm/arch-sunxi/clock_sun6i.h | 2 +-
 3 files changed, 3 insertions(+), 5 deletions(-)

arch/arm/cpu/armv7/sunxi/clock_sun6i.c +1 -3

index 193e314..8ef19df 100644
@@ -144,12 +144,10 @@ void clock_set_pll3(unsigned int clk)
 	       &ccm->pll3_cfg);
 }
 
-void clock_set_pll5(unsigned int clk, bool sigma_delta_enable)
+void clock_set_pll5(unsigned int clk, int k, int m, bool sigma_delta_enable)
 {
 	struct sunxi_ccm_reg * const ccm =
 		(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
-	const int k = 2;
-	const int m = 1;
 
 	if (sigma_delta_enable)
 		writel(CCM_PLL5_PATTERN, &ccm->pll5_pattern_cfg);

arch/arm/cpu/armv7/sunxi/dram_sun6i.c +1 -1

index bc6428a..a8bbdfd 100644
@@ -46,7 +46,7 @@ static void mctl_sys_init(void)
 		(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
 	const int dram_clk_div = 2;
 
-	clock_set_pll5(DRAM_CLK * dram_clk_div, false);
+	clock_set_pll5(DRAM_CLK * dram_clk_div, 2, 1, false);
 
 	clrsetbits_le32(&ccm->dram_clk_cfg, CCM_DRAMCLK_CFG_DIV0_MASK,
 		CCM_DRAMCLK_CFG_DIV0(dram_clk_div) | CCM_DRAMCLK_CFG_RST |

arch/arm/include/asm/arch-sunxi/clock_sun6i.h +1 -1

index f807af3..7d61216 100644
@@ -311,6 +311,6 @@ struct sunxi_ccm_reg {
 #define CCM_DE_CTRL_PLL10		(5 << 24)
 #define CCM_DE_CTRL_GATE		(1 << 31)
 
-void clock_set_pll5(unsigned int clk, bool sigma_delta_enable);
+void clock_set_pll5(unsigned int clk, int k, int m, bool sigma_delta_enable);
 
 #endif /* _SUNXI_CLOCK_SUN6I_H */

Quoted 4 lines · On Tue, 16 Dec 2014 21:31:34 +0100 Hans de Goede <[email protected]> wrote:

The A23 (sun8i) requires different values for these then sun6i, so make them function parameters.

Signed-off-by Hans de Goede <[email protected]>

What happens if A23 does not get these special k and m parameters, but the 'clock_set_pll5' function picks some other values for them (with the same resulting target clock speed)?

And if they are really required for A23, then why exposing them as the function arguments instead of hiding this implementation detail inside of the 'clock_set_pll5' function?

Quoted 49 lines

---
  arch/arm/cpu/armv7/sunxi/clock_sun6i.c        | 4 +---
  arch/arm/cpu/armv7/sunxi/dram_sun6i.c         | 2 +-
  arch/arm/include/asm/arch-sunxi/clock_sun6i.h | 2 +-
  3 files changed, 3 insertions(+), 5 deletions(-)

arch/arm/cpu/armv7/sunxi/clock_sun6i.c +1 -3

index 193e314..8ef19df 100644
@@ -144,12 +144,10 @@ void clock_set_pll3(unsigned int clk)
  	       &ccm->pll3_cfg);
  }
 
-void clock_set_pll5(unsigned int clk, bool sigma_delta_enable)
+void clock_set_pll5(unsigned int clk, int k, int m, bool sigma_delta_enable)
  {
  	struct sunxi_ccm_reg * const ccm =
  		(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
-	const int k = 2;
-	const int m = 1;
 
  	if (sigma_delta_enable)
  		writel(CCM_PLL5_PATTERN, &ccm->pll5_pattern_cfg);

arch/arm/cpu/armv7/sunxi/dram_sun6i.c +1 -1

index bc6428a..a8bbdfd 100644
@@ -46,7 +46,7 @@ static void mctl_sys_init(void)
  		(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
  	const int dram_clk_div = 2;
 
-	clock_set_pll5(DRAM_CLK * dram_clk_div, false);
+	clock_set_pll5(DRAM_CLK * dram_clk_div, 2, 1, false);
 
  	clrsetbits_le32(&ccm->dram_clk_cfg, CCM_DRAMCLK_CFG_DIV0_MASK,
  		CCM_DRAMCLK_CFG_DIV0(dram_clk_div) | CCM_DRAMCLK_CFG_RST |

arch/arm/include/asm/arch-sunxi/clock_sun6i.h +1 -1

index f807af3..7d61216 100644
@@ -311,6 +311,6 @@ struct sunxi_ccm_reg {
  #define CCM_DE_CTRL_PLL10		(5 << 24)
  #define CCM_DE_CTRL_GATE		(1 << 31)
 
-void clock_set_pll5(unsigned int clk, bool sigma_delta_enable);
+void clock_set_pll5(unsigned int clk, int k, int m, bool sigma_delta_enable);
 
  #endif /* _SUNXI_CLOCK_SUN6I_H */

Hi,

Quoted 11 lines · Siarhei Siamashka

Quoted 4 lines · On Tue, 16 Dec 2014 21:31:34 +0100 Hans de Goede <[email protected]> wrote:

The A23 (sun8i) requires different values for these then sun6i, so make them function parameters.

Signed-off-by Hans de Goede <[email protected]>

What happens if A23 does not get these special k and m parameters, but the 'clock_set_pll5' function picks some other values for them (with the same resulting target clock speed)?

The major difference is that on the A23 pll5 must be set to dram_clk / 2, where as on A31 it needs to be set to dram_clk * 2. By codifying k and m so that they do the * 2 on A31 and / 2 on A23 we can make any multiple of 24 MHz as DRAM clk without needing any other code to figure out the optimal k and m.

AFAIK using other k and m factors with the same end-result should work fine.

But doing things this way follows the KISS principle and I'm a great fan of KISS.

Regards,

Hans

Quoted 57 lines

And if they are really required for A23, then why exposing them as the function arguments instead of hiding this implementation detail inside of the 'clock_set_pll5' function?

Quoted 49 lines

---
  arch/arm/cpu/armv7/sunxi/clock_sun6i.c        | 4 +---
  arch/arm/cpu/armv7/sunxi/dram_sun6i.c         | 2 +-
  arch/arm/include/asm/arch-sunxi/clock_sun6i.h | 2 +-
  3 files changed, 3 insertions(+), 5 deletions(-)

arch/arm/cpu/armv7/sunxi/clock_sun6i.c +1 -3

index 193e314..8ef19df 100644
@@ -144,12 +144,10 @@ void clock_set_pll3(unsigned int clk)
  	       &ccm->pll3_cfg);
  }
 
-void clock_set_pll5(unsigned int clk, bool sigma_delta_enable)
+void clock_set_pll5(unsigned int clk, int k, int m, bool sigma_delta_enable)
  {
  	struct sunxi_ccm_reg * const ccm =
  		(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
-	const int k = 2;
-	const int m = 1;
 
  	if (sigma_delta_enable)
  		writel(CCM_PLL5_PATTERN, &ccm->pll5_pattern_cfg);

arch/arm/cpu/armv7/sunxi/dram_sun6i.c +1 -1

index bc6428a..a8bbdfd 100644
@@ -46,7 +46,7 @@ static void mctl_sys_init(void)
  		(struct sunxi_ccm_reg *)SUNXI_CCM_BASE;
  	const int dram_clk_div = 2;
 
-	clock_set_pll5(DRAM_CLK * dram_clk_div, false);
+	clock_set_pll5(DRAM_CLK * dram_clk_div, 2, 1, false);
 
  	clrsetbits_le32(&ccm->dram_clk_cfg, CCM_DRAMCLK_CFG_DIV0_MASK,
  		CCM_DRAMCLK_CFG_DIV0(dram_clk_div) | CCM_DRAMCLK_CFG_RST |

arch/arm/include/asm/arch-sunxi/clock_sun6i.h +1 -1

index f807af3..7d61216 100644
@@ -311,6 +311,6 @@ struct sunxi_ccm_reg {
  #define CCM_DE_CTRL_PLL10		(5 << 24)
  #define CCM_DE_CTRL_GATE		(1 << 31)
 
-void clock_set_pll5(unsigned int clk, bool sigma_delta_enable);
+void clock_set_pll5(unsigned int clk, int k, int m, bool sigma_delta_enable);
 
  #endif /* _SUNXI_CLOCK_SUN6I_H */

The await_completion helper is already copy pasted between the sun4i and sun6i dram code, and we need it for sun8i too, so lets make it an inline helper in dram.h, rather then adding yet another copy.

Signed-off-by Hans de Goede <[email protected]>

The await_completion helper is already copy pasted between the sun4i and sun6i dram code, and we need it for sun8i too, so lets make it an inline helper in dram.h, rather then adding yet another copy.

Signed-off-by Hans de Goede <[email protected]>

---
 arch/arm/cpu/armv7/sunxi/dram_sun4i.c  | 17 ++---------------
 arch/arm/cpu/armv7/sunxi/dram_sun6i.c  | 31 +++++++++----------------------
 arch/arm/include/asm/arch-sunxi/dram.h | 14 ++++++++++++++
 3 files changed, 25 insertions(+), 37 deletions(-)

arch/arm/cpu/armv7/sunxi/dram_sun4i.c +2 -15

index ec8aaa7..c736fa3 100644
@@ -36,24 +36,11 @@
 #define CPU_CFG_CHIP_REV_B 0x3
 
 /*
- * Wait up to 1s for value to be set in given part of reg.
- */
-static void await_completion(u32 *reg, u32 mask, u32 val)
-{
-	unsigned long tmo = timer_get_us() + 1000000;
-
-	while ((readl(reg) & mask) != val) {
-		if (timer_get_us() > tmo)
-			panic("Timeout initialising DRAM\n");
-	}
-}
-
-/*
  * Wait up to 1s for mask to be clear in given reg.
  */
 static inline void await_bits_clear(u32 *reg, u32 mask)
 {
-	await_completion(reg, mask, 0);
+	mctl_await_completion(reg, mask, 0);
 }
 
 /*

@@ -61,7 +48,7 @@ static inline void await_bits_clear(u32 *reg, u32 mask)
  */
 static inline void await_bits_set(u32 *reg, u32 mask)
 {
-	await_completion(reg, mask, mask);
+	mctl_await_completion(reg, mask, mask);
 }
 
 /*

arch/arm/cpu/armv7/sunxi/dram_sun6i.c +0 -13

index a8bbdfd..e1670e5 100644
@@ -27,19 +27,6 @@ struct dram_sun6i_para {
 	u16 page_size;
 };
 
-/*
- * Wait up to 1s for value to be set in given part of reg.
- */
-static void await_completion(u32 *reg, u32 mask, u32 val)
-{
-	unsigned long tmo = timer_get_us() + 1000000;
-
-	while ((readl(reg) & mask) != val) {
-		if (timer_get_us() > tmo)
-			panic("Timeout initialising DRAM\n");
-	}
-}
-
 static void mctl_sys_init(void)
 {
 	struct sunxi_ccm_reg * const ccm =

arch/arm/cpu/armv7/sunxi/dram_sun6i.c +8 -8

 	writel(MDFS_CLK_DEFAULT, &ccm->mdfs_clk_cfg);
 

@@ -107,8 +94,8 @@ static bool mctl_rank_detect(u32 *gsr0, int rank)
 	const u32 done = MCTL_DX_GSR0_RANK0_TRAIN_DONE << rank;
 	const u32 err = MCTL_DX_GSR0_RANK0_TRAIN_ERR << rank;
 
-	await_completion(gsr0, done, done);
-	await_completion(gsr0 + 0x10, done, done);
+	mctl_await_completion(gsr0, done, done);
+	mctl_await_completion(gsr0 + 0x10, done, done);
 
 	return !(readl(gsr0) & err) && !(readl(gsr0 + 0x10) & err);
 }

@@ -129,7 +116,7 @@ static void mctl_channel_init(int ch_index, struct dram_sun6i_para *para)
 	}
 
 	writel(MCTL_MCMD_NOP, &mctl_ctl->mcmd);
-	await_completion(&mctl_ctl->mcmd, MCTL_MCMD_BUSY, 0);
+	mctl_await_completion(&mctl_ctl->mcmd, MCTL_MCMD_BUSY, 0);
 
 	/* PHY initialization */
 	writel(MCTL_PGCR, &mctl_phy->pgcr);

@@ -166,14 +153,14 @@ static void mctl_channel_init(int ch_index, struct dram_sun6i_para *para)
 	writel(MCTL_DX_GCR | MCTL_DX_GCR_EN, &mctl_phy->dx2gcr);
 	writel(MCTL_DX_GCR | MCTL_DX_GCR_EN, &mctl_phy->dx3gcr);
 
-	await_completion(&mctl_phy->pgsr, 0x03, 0x03);
+	mctl_await_completion(&mctl_phy->pgsr, 0x03, 0x03);
 
 	writel(CONFIG_DRAM_ZQ, &mctl_phy->zq0cr1);
 
 	setbits_le32(&mctl_phy->pir, MCTL_PIR_CLEAR_STATUS);
 	writel(MCTL_PIR_STEP1, &mctl_phy->pir);
 	udelay(10);
-	await_completion(&mctl_phy->pgsr, 0x1f, 0x1f);
+	mctl_await_completion(&mctl_phy->pgsr, 0x1f, 0x1f);
 
 	/* rank detect */
 	if (!mctl_rank_detect(&mctl_phy->dx0gsr0, 1)) {

@@ -204,14 +191,14 @@ static void mctl_channel_init(int ch_index, struct dram_sun6i_para *para)
 	setbits_le32(&mctl_phy->pir, MCTL_PIR_CLEAR_STATUS);
 	writel(MCTL_PIR_STEP2, &mctl_phy->pir);
 	udelay(10);
-	await_completion(&mctl_phy->pgsr, 0x11, 0x11);
+	mctl_await_completion(&mctl_phy->pgsr, 0x11, 0x11);
 
 	if (readl(&mctl_phy->pgsr) & MCTL_PGSR_TRAIN_ERR_MASK)
 		panic("Training error initialising DRAM\n");
 
 	/* Move to configure state */
 	writel(MCTL_SCTL_CONFIG, &mctl_ctl->sctl);
-	await_completion(&mctl_ctl->sstat, 0x07, 0x01);
+	mctl_await_completion(&mctl_ctl->sstat, 0x07, 0x01);
 
 	/* Set number of clks per micro-second */
 	writel(DRAM_CLK / 1000000, &mctl_ctl->togcnt1u);

@@ -270,7 +257,7 @@ static void mctl_channel_init(int ch_index, struct dram_sun6i_para *para)
 
 	/* Move to access state */
 	writel(MCTL_SCTL_ACCESS, &mctl_ctl->sctl);
-	await_completion(&mctl_ctl->sstat, 0x07, 0x03);
+	mctl_await_completion(&mctl_ctl->sstat, 0x07, 0x03);
 }
 
 static void mctl_com_init(struct dram_sun6i_para *para)

arch/arm/include/asm/arch-sunxi/dram.h +14 -0

index 9072e68..18924f5 100644
@@ -12,6 +12,7 @@
 #ifndef _SUNXI_DRAM_H
 #define _SUNXI_DRAM_H
 
+#include <asm/io.h>
 #include <linux/types.h>
 
 /* dram regs definition */

@@ -23,4 +24,17 @@
 
 unsigned long sunxi_dram_init(void);
 
+/*
+ * Wait up to 1s for value to be set in given part of reg.
+ */
+static inline void mctl_await_completion(u32 *reg, u32 mask, u32 val)
+{
+	unsigned long tmo = timer_get_us() + 1000000;
+
+	while ((readl(reg) & mask) != val) {
+		if (timer_get_us() > tmo)
+			panic("Timeout initialising DRAM\n");
+	}
+}
+
 #endif /* _SUNXI_DRAM_H */

Quoted 5 lines · On Tue, 16 Dec 2014 21:31:35 +0100 Hans de Goede <[email protected]> wrote:

The await_completion helper is already copy pasted between the sun4i and sun6i dram code, and we need it for sun8i too, so lets make it an inline helper in dram.h, rather then adding yet another copy.

Signed-off-by Hans de Goede <[email protected]>

Making this function "static inline" and placing it into a header file encourages the compiler to actually inline it. Which is not great for code size:

== Before the patch and using gcc version 4.8.3 ==

$ arm-none-linux-gnieabi-size spl/arch/arm/cpu/armv7/sunxi/dram_sun4i.o
  text    data     bss     dec     hex filename
  1731       0       0    1731     6c3

$ arm-none-linux-gnieabi-size spl/arch/arm/cpu/armv7/sunxi/dram_sun6i.o
  text    data     bss     dec     hex filename
  1841       0       0    1841     731

== After the patch and using gcc version 4.8.3 ==

$ arm-none-linux-gnieabi-size spl/arch/arm/cpu/armv7/sunxi/dram_sun4i.o
  text    data     bss     dec     hex filename
  1763       0       0    1763     6e3

$ arm-none-linux-gnieabi-size spl/arch/arm/cpu/armv7/sunxi/dram_sun6i.o
  text    data     bss     dec     hex filename
  1983       0       0    1983     7bf

Could we perhaps just introduce something like a new source file "dram_common.c" or even "dram.c"?

As an additional observation, moving await_bits_clear/await_bits_set functions into a common header file and using them in the sun6i dram code might improve readability.

Hi,

Quoted 26 lines · Siarhei Siamashka

Quoted 5 lines · On Tue, 16 Dec 2014 21:31:35 +0100 Hans de Goede <[email protected]> wrote:

The await_completion helper is already copy pasted between the sun4i and sun6i dram code, and we need it for sun8i too, so lets make it an inline helper in dram.h, rather then adding yet another copy.

Signed-off-by Hans de Goede <[email protected]>

Making this function "static inline" and placing it into a header file encourages the compiler to actually inline it. Which is not great for code size:

== Before the patch and using gcc version 4.8.3 ==

$ arm-none-linux-gnieabi-size spl/arch/arm/cpu/armv7/sunxi/dram_sun4i.o
  text    data     bss     dec     hex filename
  1731       0       0    1731     6c3

$ arm-none-linux-gnieabi-size spl/arch/arm/cpu/armv7/sunxi/dram_sun6i.o
  text    data     bss     dec     hex filename
  1841       0       0    1841     731

== After the patch and using gcc version 4.8.3 ==

Quoted 7 lines

$ arm-none-linux-gnieabi-size spl/arch/arm/cpu/armv7/sunxi/dram_sun4i.o
  text    data     bss     dec     hex filename
  1763       0       0    1763     6e3

$ arm-none-linux-gnieabi-size spl/arch/arm/cpu/armv7/sunxi/dram_sun6i.o
  text    data     bss     dec     hex filename
  1983       0       0    1983     7bf

Ah, thanks for catching that, the size increase does not seem to be a problem right now, but it certainly is something to keep in mind.

Quoted 2 lines

Could we perhaps just introduce something like a new source file "dram_common.c" or even "dram.c"?

Sounds like a good idea, patches welcome.

Quoted 3 lines

As an additional observation, moving await_bits_clear/await_bits_set functions into a common header file and using them in the sun6i dram code might improve readability.

Sounds like another good idea, patches welcome.

The sun8i boot0 code fills the DRAM with a "random" pattern before comparing it at different offsets to do columns, etc. detection. The sun6i boot0 code does not do it, but it seems like a good idea to do this regardless.

The new mctl_mem_fill function this introduces is added as an inline helper in dram.h, so that it can be shared with the sun8i dram code.

While at it move mctl_mem_matches to dram.h for re-use in sun8i too.

Signed-off-by Hans de Goede <[email protected]>

---
 arch/arm/cpu/armv7/sunxi/dram_sun6i.c  | 15 +--------------
 arch/arm/include/asm/arch-sunxi/dram.h | 29 +++++++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 14 deletions(-)

arch/arm/cpu/armv7/sunxi/dram_sun6i.c +1 -14

index e1670e5..4675c48 100644
@@ -326,20 +326,6 @@ static void mctl_port_cfg(void)
 	writel(0x00000307, &mctl_com->mbagcr[5]);
 }
 
-static bool mctl_mem_matches(u32 offset)
-{
-	const int match_count = 64;
-	int i, matches = 0;
-
-	for (i = 0; i < match_count; i++) {
-		if (readl(CONFIG_SYS_SDRAM_BASE + i * 4) ==
-		    readl(CONFIG_SYS_SDRAM_BASE + offset + i * 4))
-			matches++;
-	}
-
-	return matches == match_count;
-}
-
 unsigned long sunxi_dram_init(void)
 {
 	struct sunxi_mctl_com_reg * const mctl_com =

@@ -391,6 +377,7 @@ unsigned long sunxi_dram_init(void)
 		    MCTL_CR_BANK(1) | MCTL_CR_RANK(1));
 
 	/* Detect and set page size */
+	mctl_mem_fill();
 	for (columns = 7; columns < 20; columns++) {
 		if (mctl_mem_matches(1 << columns))
 			break;

arch/arm/include/asm/arch-sunxi/dram.h +29 -0

index 18924f5..0bf718c 100644
@@ -22,6 +22,8 @@
 #include <asm/arch/dram_sun4i.h>
 #endif
 
+#define MCTL_MEM_FILL_MATCH_COUNT 64
+
 unsigned long sunxi_dram_init(void);
 
 /*

@@ -37,4 +39,31 @@ static inline void mctl_await_completion(u32 *reg, u32 mask, u32 val)
 	}
 }
 
+/*
+ * Fill beginning of DRAM with "random" data for mctl_mem_matches()
+ */
+static inline void mctl_mem_fill(void)
+{
+	int i;
+
+	for (i = 0; i < MCTL_MEM_FILL_MATCH_COUNT; i++)
+		writel(0xaa55aa55 + i, CONFIG_SYS_SDRAM_BASE + i * 4);
+}
+
+/*
+ * Test if memory at offset offset matches memory at begin of DRAM
+ */
+static inline bool mctl_mem_matches(u32 offset)
+{
+	int i, matches = 0;
+
+	for (i = 0; i < MCTL_MEM_FILL_MATCH_COUNT; i++) {
+		if (readl(CONFIG_SYS_SDRAM_BASE + i * 4) ==
+		    readl(CONFIG_SYS_SDRAM_BASE + offset + i * 4))
+			matches++;
+	}
+
+	return matches == MCTL_MEM_FILL_MATCH_COUNT;
+}
+
 #endif /* _SUNXI_DRAM_H */

Quoted 3 lines · Hans de Goede

The sun8i boot0 code fills the DRAM with a "random" pattern before comparing it at different offsets to do columns, etc. detection. The sun6i boot0 code does not do it, but it seems like a good idea to do this regardless.

Is this the right way round? The existing sun6i code (which you are moving here) seems to _rely_ on something having written a useful pattern, which I would have assumed to have been boot0. Or else how does it work now? Chance?

Quoted 89 lines

The new mctl_mem_fill function this introduces is added as an inline helper in dram.h, so that it can be shared with the sun8i dram code.

While at it move mctl_mem_matches to dram.h for re-use in sun8i too.

Signed-off-by Hans de Goede <[email protected]>

---
 arch/arm/cpu/armv7/sunxi/dram_sun6i.c  | 15 +--------------
 arch/arm/include/asm/arch-sunxi/dram.h | 29 +++++++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 14 deletions(-)

arch/arm/cpu/armv7/sunxi/dram_sun6i.c +1 -14

index e1670e5..4675c48 100644
@@ -326,20 +326,6 @@ static void mctl_port_cfg(void)
 	writel(0x00000307, &mctl_com->mbagcr[5]);
 }
 
-static bool mctl_mem_matches(u32 offset)
-{
-	const int match_count = 64;
-	int i, matches = 0;
-
-	for (i = 0; i < match_count; i++) {
-		if (readl(CONFIG_SYS_SDRAM_BASE + i * 4) ==
-		    readl(CONFIG_SYS_SDRAM_BASE + offset + i * 4))
-			matches++;
-	}
-
-	return matches == match_count;
-}
-
 unsigned long sunxi_dram_init(void)
 {
 	struct sunxi_mctl_com_reg * const mctl_com =

@@ -391,6 +377,7 @@ unsigned long sunxi_dram_init(void)
 		    MCTL_CR_BANK(1) | MCTL_CR_RANK(1));
 
 	/* Detect and set page size */
+	mctl_mem_fill();
 	for (columns = 7; columns < 20; columns++) {
 		if (mctl_mem_matches(1 << columns))
 			break;

arch/arm/include/asm/arch-sunxi/dram.h +29 -0

index 18924f5..0bf718c 100644
@@ -22,6 +22,8 @@
 #include <asm/arch/dram_sun4i.h>
 #endif
 
+#define MCTL_MEM_FILL_MATCH_COUNT 64
+
 unsigned long sunxi_dram_init(void);
 
 /*

@@ -37,4 +39,31 @@ static inline void mctl_await_completion(u32 *reg, u32 mask, u32 val)
 	}
 }
 
+/*
+ * Fill beginning of DRAM with "random" data for mctl_mem_matches()
+ */
+static inline void mctl_mem_fill(void)
+{
+	int i;
+
+	for (i = 0; i < MCTL_MEM_FILL_MATCH_COUNT; i++)
+		writel(0xaa55aa55 + i, CONFIG_SYS_SDRAM_BASE + i * 4);
+}
+
+/*
+ * Test if memory at offset offset matches memory at begin of DRAM
+ */
+static inline bool mctl_mem_matches(u32 offset)
+{
+	int i, matches = 0;
+
+	for (i = 0; i < MCTL_MEM_FILL_MATCH_COUNT; i++) {
+		if (readl(CONFIG_SYS_SDRAM_BASE + i * 4) ==
+		    readl(CONFIG_SYS_SDRAM_BASE + offset + i * 4))
+			matches++;
+	}
+
+	return matches == MCTL_MEM_FILL_MATCH_COUNT;
+}
+
 #endif /* _SUNXI_DRAM_H */

Quoted 9 lines · On Thu, 18 Dec 2014 19:12:13 +0000 Ian Campbell <[email protected]> wrote:

Quoted 3 lines · Hans de Goede

The sun8i boot0 code fills the DRAM with a "random" pattern before comparing it at different offsets to do columns, etc. detection. The sun6i boot0 code does not do it, but it seems like a good idea to do this regardless.

Is this the right way round? The existing sun6i code (which you are moving here) seems to _rely_ on something having written a useful pattern, which I would have assumed to have been boot0. Or else how does it work now? Chance?

It appears that this code is just trying to find the first address line, which is not connected anywhere. If the address line is not connected, then having the corresponding bit set or clear in the memory address does not matter and we are effectively accessing the same location.

Both sun6i and sun8i code is incorrect and only works because of relying on luck.

The sun6i code is incorrect because it is just reading memory without initializing it at all. Probably relying on having some unique garbage there in a natural way.

The newer sun8i code is still incorrect because it is not safeguarding against accidentally encountering the same test pattern at an unrelated memory location. Yes, the probability is extremely low, but still not zero.

Hi,

Quoted 13 lines · Siarhei Siamashka

Quoted 9 lines · On Thu, 18 Dec 2014 19:12:13 +0000 Ian Campbell <[email protected]> wrote:

Quoted 3 lines · Hans de Goede

The sun8i boot0 code fills the DRAM with a "random" pattern before comparing it at different offsets to do columns, etc. detection. The sun6i boot0 code does not do it, but it seems like a good idea to do this regardless.

Is this the right way round? The existing sun6i code (which you are moving here) seems to _rely_ on something having written a useful pattern, which I would have assumed to have been boot0. Or else how does it work now? Chance?

Quoted 4 lines

It appears that this code is just trying to find the first address line, which is not connected anywhere. If the address line is not connected, then having the corresponding bit set or clear in the memory address does not matter and we are effectively accessing the same location.

Correct.

Quoted 7 lines

Both sun6i and sun8i code is incorrect and only works because of relying on luck.

The sun6i code is incorrect because it is just reading memory without initializing it at all. Probably relying on having some unique garbage there in a natural way.

Quoted 4 lines

The newer sun8i code is still incorrect because it is not safeguarding against accidentally encountering the same test pattern at an unrelated memory location. Yes, the probability is extremely low, but still not zero.

The probability is so low that it really does not matter, so I've no intention to fix this.

Quoted 3 lines · Hans de Goede

The sun8i boot0 code fills the DRAM with a "random" pattern before comparing it at different offsets to do columns, etc. detection. The sun6i boot0 code does not do it, but it seems like a good idea to do this regardless.

Is this the right way round?

The existing sun6i code (which you are
moving here) seems to _rely_ on something having written a useful pattern, which I would have assumed to have been boot0. Or else how does it work now? Chance?

Quoted 90 lines

The new mctl_mem_fill function this introduces is added as an inline helper in dram.h, so that it can be shared with the sun8i dram code.

While at it move mctl_mem_matches to dram.h for re-use in sun8i too.

Signed-off-by Hans de Goede <[email protected]>

---
  arch/arm/cpu/armv7/sunxi/dram_sun6i.c  | 15 +--------------
  arch/arm/include/asm/arch-sunxi/dram.h | 29 +++++++++++++++++++++++++++++
  2 files changed, 30 insertions(+), 14 deletions(-)

arch/arm/cpu/armv7/sunxi/dram_sun6i.c +1 -14

index e1670e5..4675c48 100644
@@ -326,20 +326,6 @@ static void mctl_port_cfg(void)
  	writel(0x00000307, &mctl_com->mbagcr[5]);
  }
 
-static bool mctl_mem_matches(u32 offset)
-{
-	const int match_count = 64;
-	int i, matches = 0;
-
-	for (i = 0; i < match_count; i++) {
-		if (readl(CONFIG_SYS_SDRAM_BASE + i * 4) ==
-		    readl(CONFIG_SYS_SDRAM_BASE + offset + i * 4))
-			matches++;
-	}
-
-	return matches == match_count;
-}
-
  unsigned long sunxi_dram_init(void)
  {
  	struct sunxi_mctl_com_reg * const mctl_com =

@@ -391,6 +377,7 @@ unsigned long sunxi_dram_init(void)
  		    MCTL_CR_BANK(1) | MCTL_CR_RANK(1));
 
  	/* Detect and set page size */
+	mctl_mem_fill();
  	for (columns = 7; columns < 20; columns++) {
  		if (mctl_mem_matches(1 << columns))
  			break;

arch/arm/include/asm/arch-sunxi/dram.h +29 -0

index 18924f5..0bf718c 100644
@@ -22,6 +22,8 @@
  #include <asm/arch/dram_sun4i.h>
  #endif
 
+#define MCTL_MEM_FILL_MATCH_COUNT 64
+
  unsigned long sunxi_dram_init(void);
 
  /*

@@ -37,4 +39,31 @@ static inline void mctl_await_completion(u32 *reg, u32 mask, u32 val)
  	}
  }
 
+/*
+ * Fill beginning of DRAM with "random" data for mctl_mem_matches()
+ */
+static inline void mctl_mem_fill(void)
+{
+	int i;
+
+	for (i = 0; i < MCTL_MEM_FILL_MATCH_COUNT; i++)
+		writel(0xaa55aa55 + i, CONFIG_SYS_SDRAM_BASE + i * 4);
+}
+
+/*
+ * Test if memory at offset offset matches memory at begin of DRAM
+ */
+static inline bool mctl_mem_matches(u32 offset)
+{
+	int i, matches = 0;
+
+	for (i = 0; i < MCTL_MEM_FILL_MATCH_COUNT; i++) {
+		if (readl(CONFIG_SYS_SDRAM_BASE + i * 4) ==
+		    readl(CONFIG_SYS_SDRAM_BASE + offset + i * 4))
+			matches++;
+	}
+
+	return matches == MCTL_MEM_FILL_MATCH_COUNT;
+}
+
  #endif /* _SUNXI_DRAM_H */

Hi,

Quoted 6 lines · Ian Campbell

Quoted 3 lines · Hans de Goede

The sun8i boot0 code fills the DRAM with a "random" pattern before comparing it at different offsets to do columns, etc. detection. The sun6i boot0 code does not do it, but it seems like a good idea to do this regardless.

Is this the right way round?

Yes, while working on the sun8i code I noticed that it was prefilling memory before doing wrap-around checks like the sun6i code is doing too, and then I realized that the sun6i code was relying on whatever is in DRAM being random enough for its wrap-around tests to work, without ensuring it is random enough.

Quoted 4 lines

The existing sun6i code (which you are
moving here) seems to _rely_ on something having written a useful pattern, which I would have assumed to have been boot0. Or else how does it work now? Chance?

Yep, it purely works by chance so far.

Quoted 21 lines · Hans de Goede

Hi,

Quoted 6 lines · Ian Campbell

Quoted 3 lines · Hans de Goede

The sun8i boot0 code fills the DRAM with a "random" pattern before comparing it at different offsets to do columns, etc. detection. The sun6i boot0 code does not do it, but it seems like a good idea to do this regardless.

Is this the right way round?

Yes, while working on the sun8i code I noticed that it was prefilling memory before doing wrap-around checks like the sun6i code is doing too, and then I realized that the sun6i code was relying on whatever is in DRAM being random enough for its wrap-around tests to work, without ensuring it is random enough.

Quoted 4 lines

The existing sun6i code (which you are
moving here) seems to _rely_ on something having written a useful pattern, which I would have assumed to have been boot0. Or else how does it work now? Chance?

Quoted 1 line

Yep, it purely works by chance so far.

"excellent". Can you mention that in the commit message please. With
that: Acked-by: Ian Campbell <[email protected]>

Quoted 22 lines · On Mon, 22 Dec 2014 14:19:20 +0000 Ian Campbell <[email protected]> wrote:

Quoted 21 lines · Hans de Goede

Hi,

Quoted 6 lines · Ian Campbell

Quoted 3 lines · Hans de Goede

The sun8i boot0 code fills the DRAM with a "random" pattern before comparing it at different offsets to do columns, etc. detection. The sun6i boot0 code does not do it, but it seems like a good idea to do this regardless.

Is this the right way round?

Yes, while working on the sun8i code I noticed that it was prefilling memory before doing wrap-around checks like the sun6i code is doing too, and then I realized that the sun6i code was relying on whatever is in DRAM being random enough for its wrap-around tests to work, without ensuring it is random enough.

Quoted 4 lines

The existing sun6i code (which you are
moving here) seems to _rely_ on something having written a useful pattern, which I would have assumed to have been boot0. Or else how does it work now? Chance?

Quoted 4 lines

Quoted 1 line

Yep, it purely works by chance so far.

"excellent". Can you mention that in the commit message please. With
that: Acked-by: Ian Campbell <[email protected]>

Ian, are you fine with the fact that it still works by chance even after this "fix"? Albeit with a much higher chance of not running into troubles.

Quoted 16 lines · Hans de Goede

Hi,

Quoted 6 lines · Ian Campbell

Quoted 3 lines · Hans de Goede

The sun8i boot0 code fills the DRAM with a "random" pattern before comparing it at different offsets to do columns, etc. detection. The sun6i boot0 code does not do it, but it seems like a good idea to do this regardless.

Is this the right way round?

Yes, while working on the sun8i code I noticed that it was prefilling memory before doing wrap-around checks like the sun6i code is doing too, and then I realized that the sun6i code was relying on whatever is in DRAM being random enough for its wrap-around tests to work, without ensuring it is random enough.

Quoted 6 lines

Quoted 4 lines

The existing sun6i code (which you are
moving here) seems to _rely_ on something having written a useful pattern, which I would have assumed to have been boot0. Or else how does it work now? Chance?

Yep, it purely works by chance so far.

"excellent". Can you mention that in the commit message please.

With that: Acked-by: Ian Campbell <[email protected]>

Use memcmp for mctl_mem_matches instead of DIY.

Signed-off-by Hans de Goede <[email protected]>