* [U-Boot] [PATCH v2 01/11] dm: linker_lists: Add a way to declare multiple objects
2014-09-17 3:51 [U-Boot] [PATCH v2 0/11] dm: imx: Add driver model support for GPIO and serial on cm_fx6 Simon Glass
@ 2014-09-17 3:51 ` Simon Glass
2014-09-17 3:51 ` [U-Boot] [PATCH v2 02/11] dm: core: Allow a list of devices to be declared in one step Simon Glass
` (9 subsequent siblings)
10 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2014-09-17 3:51 UTC (permalink / raw)
To: u-boot
The existing ll_entry_declare() permits a single element of the list to
be added to a linker list. Sometimes we want to add several objects at
once. To avoid lots of messy declarations, add a macro to support this.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None
include/linker_lists.h | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/include/linker_lists.h b/include/linker_lists.h
index 557e627..32cc9fc 100644
--- a/include/linker_lists.h
+++ b/include/linker_lists.h
@@ -141,6 +141,27 @@
section(".u_boot_list_2_"#_list"_2_"#_name)))
/**
+ * ll_entry_declare_list() - Declare a list of link-generated array entries
+ * @_type: Data type of each entry
+ * @_name: Name of the entry
+ * @_list: name of the list. Should contain only characters allowed
+ * in a C variable name!
+ *
+ * This is like ll_entry_declare() but creates multiple entries. It should
+ * be assigned to an array.
+ *
+ * ll_entry_declare_list(struct my_sub_cmd, my_sub_cmd, cmd_sub, cmd.sub) = {
+ * { .x = 3, .y = 4 },
+ * { .x = 8, .y = 2 },
+ * { .x = 1, .y = 7 }
+ * };
+ */
+#define ll_entry_declare_list(_type, _name, _list) \
+ _type _u_boot_list_2_##_list##_2_##_name[] __aligned(4) \
+ __attribute__((unused, \
+ section(".u_boot_list_2_"#_list"_2_"#_name)))
+
+/**
* We need a 0-byte-size type for iterator symbols, and the compiler
* does not allow defining objects of C type 'void'. Using an empty
* struct is allowed by the compiler, but causes gcc versions 4.4 and
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related [flat|nested] 25+ messages in thread* [U-Boot] [PATCH v2 02/11] dm: core: Allow a list of devices to be declared in one step
2014-09-17 3:51 [U-Boot] [PATCH v2 0/11] dm: imx: Add driver model support for GPIO and serial on cm_fx6 Simon Glass
2014-09-17 3:51 ` [U-Boot] [PATCH v2 01/11] dm: linker_lists: Add a way to declare multiple objects Simon Glass
@ 2014-09-17 3:51 ` Simon Glass
2014-09-17 3:51 ` [U-Boot] [PATCH v2 03/11] dm: core: Allow device_bind() to used without CONFIG_OF_CONTROL Simon Glass
` (8 subsequent siblings)
10 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2014-09-17 3:51 UTC (permalink / raw)
To: u-boot
The U_BOOT_DEVICE macro allows the declaration of a single U-Boot device.
Add an equivalent macro to declare an array of devices, for convenience.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None
include/dm/platdata.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/include/dm/platdata.h b/include/dm/platdata.h
index 2bc8b14..9e47e51 100644
--- a/include/dm/platdata.h
+++ b/include/dm/platdata.h
@@ -25,4 +25,8 @@ struct driver_info {
#define U_BOOT_DEVICE(__name) \
ll_entry_declare(struct driver_info, __name, driver_info)
+/* Declare a list of devices. The argument is a driver_info[] array */
+#define U_BOOT_DEVICES(__name) \
+ ll_entry_declare_list(struct driver_info, __name, driver_info)
+
#endif
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related [flat|nested] 25+ messages in thread* [U-Boot] [PATCH v2 03/11] dm: core: Allow device_bind() to used without CONFIG_OF_CONTROL
2014-09-17 3:51 [U-Boot] [PATCH v2 0/11] dm: imx: Add driver model support for GPIO and serial on cm_fx6 Simon Glass
2014-09-17 3:51 ` [U-Boot] [PATCH v2 01/11] dm: linker_lists: Add a way to declare multiple objects Simon Glass
2014-09-17 3:51 ` [U-Boot] [PATCH v2 02/11] dm: core: Allow a list of devices to be declared in one step Simon Glass
@ 2014-09-17 3:51 ` Simon Glass
2014-09-17 3:51 ` [U-Boot] [PATCH v2 04/11] initcall: Display error number when an error occurs Simon Glass
` (7 subsequent siblings)
10 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2014-09-17 3:51 UTC (permalink / raw)
To: u-boot
The sequence number support in driver model requires device tree control.
It should be skipped if CONFIG_OF_CONTROL is not defined, and should not
require functions from fdtdec.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None
drivers/core/device.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/core/device.c b/drivers/core/device.c
index 166b073..ef41a9b 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -106,13 +106,16 @@ int device_bind(struct udevice *parent, struct driver *drv, const char *name,
* a 'requested' sequence, and will be resolved (and ->seq updated)
* when the device is probed.
*/
- dev->req_seq = fdtdec_get_int(gd->fdt_blob, of_offset, "reg", -1);
dev->seq = -1;
+#ifdef CONFIG_OF_CONTROL
+ dev->req_seq = fdtdec_get_int(gd->fdt_blob, of_offset, "reg", -1);
if (uc->uc_drv->name && of_offset != -1) {
fdtdec_get_alias_seq(gd->fdt_blob, uc->uc_drv->name, of_offset,
&dev->req_seq);
}
-
+#else
+ dev->req_seq = -1;
+#endif
if (!dev->platdata && drv->platdata_auto_alloc_size)
dev->flags |= DM_FLAG_ALLOC_PDATA;
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related [flat|nested] 25+ messages in thread* [U-Boot] [PATCH v2 04/11] initcall: Display error number when an error occurs
2014-09-17 3:51 [U-Boot] [PATCH v2 0/11] dm: imx: Add driver model support for GPIO and serial on cm_fx6 Simon Glass
` (2 preceding siblings ...)
2014-09-17 3:51 ` [U-Boot] [PATCH v2 03/11] dm: core: Allow device_bind() to used without CONFIG_OF_CONTROL Simon Glass
@ 2014-09-17 3:51 ` Simon Glass
2014-09-17 7:40 ` Igor Grinberg
2014-09-17 3:51 ` [U-Boot] [PATCH v2 05/11] dm: serial: Don't require device tree to configure a console Simon Glass
` (6 subsequent siblings)
10 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2014-09-17 3:51 UTC (permalink / raw)
To: u-boot
Now that some initcall functions return a useful error number, display it
when something goes wrong.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Add patch to display error number when an error occurs in initcall
lib/initcall.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/lib/initcall.c b/lib/initcall.c
index 7597bad..39f4b3f 100644
--- a/lib/initcall.c
+++ b/lib/initcall.c
@@ -15,14 +15,16 @@ int initcall_run_list(const init_fnc_t init_sequence[])
for (init_fnc_ptr = init_sequence; *init_fnc_ptr; ++init_fnc_ptr) {
unsigned long reloc_ofs = 0;
+ int ret;
if (gd->flags & GD_FLG_RELOC)
reloc_ofs = gd->reloc_off;
debug("initcall: %p\n", (char *)*init_fnc_ptr - reloc_ofs);
- if ((*init_fnc_ptr)()) {
- printf("initcall sequence %p failed at call %p\n",
+ ret = (*init_fnc_ptr)();
+ if (ret) {
+ printf("initcall sequence %p failed at call %p (err=%d)\n",
init_sequence,
- (char *)*init_fnc_ptr - reloc_ofs);
+ (char *)*init_fnc_ptr - reloc_ofs, ret);
return -1;
}
}
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related [flat|nested] 25+ messages in thread* [U-Boot] [PATCH v2 05/11] dm: serial: Don't require device tree to configure a console
2014-09-17 3:51 [U-Boot] [PATCH v2 0/11] dm: imx: Add driver model support for GPIO and serial on cm_fx6 Simon Glass
` (3 preceding siblings ...)
2014-09-17 3:51 ` [U-Boot] [PATCH v2 04/11] initcall: Display error number when an error occurs Simon Glass
@ 2014-09-17 3:51 ` Simon Glass
2014-09-17 3:51 ` [U-Boot] [PATCH v2 06/11] dm: serial: Put common code into separate functions Simon Glass
` (5 subsequent siblings)
10 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2014-09-17 3:51 UTC (permalink / raw)
To: u-boot
Allow serial_find_console_or_panic() to work without a device tree.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None
drivers/serial/serial-uclass.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
index d04104e..1ac943f 100644
--- a/drivers/serial/serial-uclass.c
+++ b/drivers/serial/serial-uclass.c
@@ -25,6 +25,7 @@ struct udevice *cur_dev __attribute__ ((section(".data")));
static void serial_find_console_or_panic(void)
{
+#ifdef CONFIG_OF_CONTROL
int node;
/* Check for a chosen console */
@@ -44,7 +45,7 @@ static void serial_find_console_or_panic(void)
return;
cur_dev = NULL;
}
-
+#endif
/*
* Failing that, get the device with sequence number 0, or in extremis
* just the first serial device we can find. But we insist on having
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related [flat|nested] 25+ messages in thread* [U-Boot] [PATCH v2 06/11] dm: serial: Put common code into separate functions
2014-09-17 3:51 [U-Boot] [PATCH v2 0/11] dm: imx: Add driver model support for GPIO and serial on cm_fx6 Simon Glass
` (4 preceding siblings ...)
2014-09-17 3:51 ` [U-Boot] [PATCH v2 05/11] dm: serial: Don't require device tree to configure a console Simon Glass
@ 2014-09-17 3:51 ` Simon Glass
2014-09-17 3:51 ` [U-Boot] [PATCH v2 07/11] dm: imx: Add error checking to setup_i2c() Simon Glass
` (4 subsequent siblings)
10 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2014-09-17 3:51 UTC (permalink / raw)
To: u-boot
Avoid duplicating the code which deals with getc() and putc(). It is fairly
simple, but may expand later.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None
drivers/serial/serial-uclass.c | 32 +++++++++++++++++---------------
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
index 1ac943f..e93c624 100644
--- a/drivers/serial/serial-uclass.c
+++ b/drivers/serial/serial-uclass.c
@@ -71,7 +71,7 @@ void serial_initialize(void)
serial_find_console_or_panic();
}
-void serial_putc(char ch)
+static void serial_putc_dev(struct udevice *dev, char ch)
{
struct dm_serial_ops *ops = serial_get_ops(cur_dev);
int err;
@@ -83,6 +83,11 @@ void serial_putc(char ch)
serial_putc('\r');
}
+void serial_putc(char ch)
+{
+ serial_putc_dev(cur_dev, ch);
+}
+
void serial_setbrg(void)
{
struct dm_serial_ops *ops = serial_get_ops(cur_dev);
@@ -107,28 +112,32 @@ int serial_tstc(void)
return 1;
}
-int serial_getc(void)
+static int serial_getc_dev(struct udevice *dev)
{
- struct dm_serial_ops *ops = serial_get_ops(cur_dev);
+ struct dm_serial_ops *ops = serial_get_ops(dev);
int err;
do {
- err = ops->getc(cur_dev);
+ err = ops->getc(dev);
} while (err == -EAGAIN);
return err >= 0 ? err : 0;
}
+int serial_getc(void)
+{
+ return serial_getc_dev(cur_dev);
+}
+
void serial_stdio_init(void)
{
}
-void serial_stub_putc(struct stdio_dev *sdev, const char ch)
+static void serial_stub_putc(struct stdio_dev *sdev, const char ch)
{
struct udevice *dev = sdev->priv;
- struct dm_serial_ops *ops = serial_get_ops(dev);
- ops->putc(dev, ch);
+ serial_putc_dev(dev, ch);
}
void serial_stub_puts(struct stdio_dev *sdev, const char *str)
@@ -140,15 +149,8 @@ void serial_stub_puts(struct stdio_dev *sdev, const char *str)
int serial_stub_getc(struct stdio_dev *sdev)
{
struct udevice *dev = sdev->priv;
- struct dm_serial_ops *ops = serial_get_ops(dev);
-
- int err;
- do {
- err = ops->getc(dev);
- } while (err == -EAGAIN);
-
- return err >= 0 ? err : 0;
+ return serial_getc_dev(dev);
}
int serial_stub_tstc(struct stdio_dev *sdev)
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related [flat|nested] 25+ messages in thread* [U-Boot] [PATCH v2 07/11] dm: imx: Add error checking to setup_i2c()
2014-09-17 3:51 [U-Boot] [PATCH v2 0/11] dm: imx: Add driver model support for GPIO and serial on cm_fx6 Simon Glass
` (5 preceding siblings ...)
2014-09-17 3:51 ` [U-Boot] [PATCH v2 06/11] dm: serial: Put common code into separate functions Simon Glass
@ 2014-09-17 3:51 ` Simon Glass
2014-09-17 8:37 ` Igor Grinberg
2014-09-17 3:51 ` [U-Boot] [PATCH v2 08/11] dm: imx: Use gpio_request() to request GPIOs Simon Glass
` (3 subsequent siblings)
10 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2014-09-17 3:51 UTC (permalink / raw)
To: u-boot
Since this function can fail, check its return value.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Add new patch to add error checking to setup_i2c()
arch/arm/imx-common/i2c-mxv7.c | 24 ++++++++++++++++-----
arch/arm/include/asm/imx-common/mxc_i2c.h | 4 ++--
board/compulab/cm_fx6/cm_fx6.c | 35 +++++++++++++++++++++----------
3 files changed, 45 insertions(+), 18 deletions(-)
diff --git a/arch/arm/imx-common/i2c-mxv7.c b/arch/arm/imx-common/i2c-mxv7.c
index a580873..70cff5c 100644
--- a/arch/arm/imx-common/i2c-mxv7.c
+++ b/arch/arm/imx-common/i2c-mxv7.c
@@ -69,15 +69,29 @@ static void * const i2c_bases[] = {
};
/* i2c_index can be from 0 - 2 */
-void setup_i2c(unsigned i2c_index, int speed, int slave_addr,
- struct i2c_pads_info *p)
+int setup_i2c(unsigned i2c_index, int speed, int slave_addr,
+ struct i2c_pads_info *p)
{
+ int ret;
+
if (i2c_index >= ARRAY_SIZE(i2c_bases))
- return;
+ return -EINVAL;
/* Enable i2c clock */
- enable_i2c_clk(1, i2c_index);
+ ret = enable_i2c_clk(1, i2c_index);
+ if (ret)
+ goto err_clk;
+
/* Make sure bus is idle */
- force_idle_bus(p);
+ ret = force_idle_bus(p);
+ if (ret)
+ goto err_idle;
+
bus_i2c_init(i2c_bases[i2c_index], speed, slave_addr,
force_idle_bus, p);
+
+ return 0;
+
+err_idle:
+err_clk:
+ return ret;
}
diff --git a/arch/arm/include/asm/imx-common/mxc_i2c.h b/arch/arm/include/asm/imx-common/mxc_i2c.h
index 182c2f3..af86163 100644
--- a/arch/arm/include/asm/imx-common/mxc_i2c.h
+++ b/arch/arm/include/asm/imx-common/mxc_i2c.h
@@ -52,8 +52,8 @@ struct i2c_pads_info {
&mx6q_##name : &mx6s_##name
#endif
-void setup_i2c(unsigned i2c_index, int speed, int slave_addr,
- struct i2c_pads_info *p);
+int setup_i2c(unsigned i2c_index, int speed, int slave_addr,
+ struct i2c_pads_info *p);
void bus_i2c_init(void *base, int speed, int slave_addr,
int (*idle_bus_fn)(void *p), void *p);
int bus_i2c_read(void *base, uchar chip, uint addr, int alen, uchar *buf,
diff --git a/board/compulab/cm_fx6/cm_fx6.c b/board/compulab/cm_fx6/cm_fx6.c
index fdb8ebf..62c625a 100644
--- a/board/compulab/cm_fx6/cm_fx6.c
+++ b/board/compulab/cm_fx6/cm_fx6.c
@@ -69,7 +69,7 @@ static iomux_v3_cfg_t const sata_pads[] = {
IOMUX_PADS(PAD_EIM_BCLK__GPIO6_IO31 | MUX_PAD_CTRL(NO_PAD_CTRL)),
};
-static void cm_fx6_setup_issd(void)
+static int cm_fx6_setup_issd(void)
{
SETUP_IOMUX_PADS(sata_pads);
/* Make sure this gpio has logical 0 value */
@@ -79,14 +79,18 @@ static void cm_fx6_setup_issd(void)
cm_fx6_sata_power(0);
mdelay(250);
cm_fx6_sata_power(1);
+
+ return 0;
}
#define CM_FX6_SATA_INIT_RETRIES 10
int sata_initialize(void)
{
- int err, i;
+ int err, i, ret;
- cm_fx6_setup_issd();
+ ret = cm_fx6_setup_issd();
+ if (ret)
+ return ret;
for (i = 0; i < CM_FX6_SATA_INIT_RETRIES; i++) {
err = setup_sata();
if (err) {
@@ -141,14 +145,25 @@ I2C_PADS(i2c2_pads,
IMX_GPIO_NR(1, 6));
-static void cm_fx6_setup_i2c(void)
+static int cm_fx6_setup_i2c(void)
{
- setup_i2c(0, CONFIG_SYS_I2C_SPEED, 0x7f, I2C_PADS_INFO(i2c0_pads));
- setup_i2c(1, CONFIG_SYS_I2C_SPEED, 0x7f, I2C_PADS_INFO(i2c1_pads));
- setup_i2c(2, CONFIG_SYS_I2C_SPEED, 0x7f, I2C_PADS_INFO(i2c2_pads));
+ int ret;
+
+ ret = setup_i2c(0, CONFIG_SYS_I2C_SPEED, 0x7f,
+ I2C_PADS_INFO(i2c0_pads));
+ if (!ret) {
+ ret = setup_i2c(1, CONFIG_SYS_I2C_SPEED, 0x7f,
+ I2C_PADS_INFO(i2c1_pads));
+ }
+ if (!ret) {
+ ret = setup_i2c(2, CONFIG_SYS_I2C_SPEED, 0x7f,
+ I2C_PADS_INFO(i2c2_pads));
+ }
+
+ return ret;
}
#else
-static void cm_fx6_setup_i2c(void) { }
+static int cm_fx6_setup_i2c(void) { return 0; }
#endif
#ifdef CONFIG_USB_EHCI_MX6
@@ -411,9 +426,7 @@ int board_init(void)
{
gd->bd->bi_boot_params = PHYS_SDRAM_1 + 0x100;
cm_fx6_setup_gpmi_nand();
- cm_fx6_setup_i2c();
-
- return 0;
+ return cm_fx6_setup_i2c();
}
int checkboard(void)
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related [flat|nested] 25+ messages in thread* [U-Boot] [PATCH v2 07/11] dm: imx: Add error checking to setup_i2c()
2014-09-17 3:51 ` [U-Boot] [PATCH v2 07/11] dm: imx: Add error checking to setup_i2c() Simon Glass
@ 2014-09-17 8:37 ` Igor Grinberg
2014-09-17 13:56 ` Simon Glass
0 siblings, 1 reply; 25+ messages in thread
From: Igor Grinberg @ 2014-09-17 8:37 UTC (permalink / raw)
To: u-boot
Hi Simon,
On 09/17/14 06:51, Simon Glass wrote:
> Since this function can fail, check its return value.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v2:
> - Add new patch to add error checking to setup_i2c()
>
> arch/arm/imx-common/i2c-mxv7.c | 24 ++++++++++++++++-----
> arch/arm/include/asm/imx-common/mxc_i2c.h | 4 ++--
> board/compulab/cm_fx6/cm_fx6.c | 35 +++++++++++++++++++++----------
> 3 files changed, 45 insertions(+), 18 deletions(-)
>
> diff --git a/arch/arm/imx-common/i2c-mxv7.c b/arch/arm/imx-common/i2c-mxv7.c
> index a580873..70cff5c 100644
> --- a/arch/arm/imx-common/i2c-mxv7.c
> +++ b/arch/arm/imx-common/i2c-mxv7.c
> @@ -69,15 +69,29 @@ static void * const i2c_bases[] = {
> };
>
> /* i2c_index can be from 0 - 2 */
> -void setup_i2c(unsigned i2c_index, int speed, int slave_addr,
> - struct i2c_pads_info *p)
> +int setup_i2c(unsigned i2c_index, int speed, int slave_addr,
> + struct i2c_pads_info *p)
> {
> + int ret;
> +
> if (i2c_index >= ARRAY_SIZE(i2c_bases))
> - return;
> + return -EINVAL;
> /* Enable i2c clock */
> - enable_i2c_clk(1, i2c_index);
> + ret = enable_i2c_clk(1, i2c_index);
> + if (ret)
> + goto err_clk;
> +
> /* Make sure bus is idle */
> - force_idle_bus(p);
> + ret = force_idle_bus(p);
> + if (ret)
> + goto err_idle;
> +
> bus_i2c_init(i2c_bases[i2c_index], speed, slave_addr,
> force_idle_bus, p);
> +
> + return 0;
> +
> +err_idle:
> +err_clk:
> + return ret;
> }
> diff --git a/arch/arm/include/asm/imx-common/mxc_i2c.h b/arch/arm/include/asm/imx-common/mxc_i2c.h
> index 182c2f3..af86163 100644
> --- a/arch/arm/include/asm/imx-common/mxc_i2c.h
> +++ b/arch/arm/include/asm/imx-common/mxc_i2c.h
> @@ -52,8 +52,8 @@ struct i2c_pads_info {
> &mx6q_##name : &mx6s_##name
> #endif
>
> -void setup_i2c(unsigned i2c_index, int speed, int slave_addr,
> - struct i2c_pads_info *p);
> +int setup_i2c(unsigned i2c_index, int speed, int slave_addr,
> + struct i2c_pads_info *p);
> void bus_i2c_init(void *base, int speed, int slave_addr,
> int (*idle_bus_fn)(void *p), void *p);
> int bus_i2c_read(void *base, uchar chip, uint addr, int alen, uchar *buf,
> diff --git a/board/compulab/cm_fx6/cm_fx6.c b/board/compulab/cm_fx6/cm_fx6.c
> index fdb8ebf..62c625a 100644
> --- a/board/compulab/cm_fx6/cm_fx6.c
> +++ b/board/compulab/cm_fx6/cm_fx6.c
> @@ -69,7 +69,7 @@ static iomux_v3_cfg_t const sata_pads[] = {
> IOMUX_PADS(PAD_EIM_BCLK__GPIO6_IO31 | MUX_PAD_CTRL(NO_PAD_CTRL)),
> };
>
> -static void cm_fx6_setup_issd(void)
> +static int cm_fx6_setup_issd(void)
> {
> SETUP_IOMUX_PADS(sata_pads);
> /* Make sure this gpio has logical 0 value */
> @@ -79,14 +79,18 @@ static void cm_fx6_setup_issd(void)
> cm_fx6_sata_power(0);
> mdelay(250);
> cm_fx6_sata_power(1);
> +
> + return 0;
> }
>
> #define CM_FX6_SATA_INIT_RETRIES 10
> int sata_initialize(void)
> {
> - int err, i;
> + int err, i, ret;
>
> - cm_fx6_setup_issd();
> + ret = cm_fx6_setup_issd();
> + if (ret)
> + return ret;
Hmm.. cm-fx6 may have iSSD not assembled and in this case
it has bypasses for a (m)SATA socket on the baseboard.
The socketed device of course is not controlled by those GPIOs.
Therefore, I think, it would be incorrect to fail the boot process
if there is a problem with an iSSD GPIO...
Instead a warning message will be enough. Something like:
ret = cm_fx6_setup_issd();
if (ret)
printf("Warning: iSSD setup failed!\n");
and then continue to the rest of SATA init.
> for (i = 0; i < CM_FX6_SATA_INIT_RETRIES; i++) {
> err = setup_sata();
> if (err) {
> @@ -141,14 +145,25 @@ I2C_PADS(i2c2_pads,
> IMX_GPIO_NR(1, 6));
>
>
> -static void cm_fx6_setup_i2c(void)
> +static int cm_fx6_setup_i2c(void)
> {
> - setup_i2c(0, CONFIG_SYS_I2C_SPEED, 0x7f, I2C_PADS_INFO(i2c0_pads));
> - setup_i2c(1, CONFIG_SYS_I2C_SPEED, 0x7f, I2C_PADS_INFO(i2c1_pads));
> - setup_i2c(2, CONFIG_SYS_I2C_SPEED, 0x7f, I2C_PADS_INFO(i2c2_pads));
> + int ret;
> +
> + ret = setup_i2c(0, CONFIG_SYS_I2C_SPEED, 0x7f,
> + I2C_PADS_INFO(i2c0_pads));
> + if (!ret) {
> + ret = setup_i2c(1, CONFIG_SYS_I2C_SPEED, 0x7f,
> + I2C_PADS_INFO(i2c1_pads));
> + }
> + if (!ret) {
> + ret = setup_i2c(2, CONFIG_SYS_I2C_SPEED, 0x7f,
> + I2C_PADS_INFO(i2c2_pads));
> + }
Almost same here, the fact that one of the (or even all) i2c buses
fails to initialize, should not lead to hang()...
The decision if this is a critical error or not should be decided
by the board code.
In this case, this is not a critical error and if one of the buses
fails to initialize, the others should still try.
So, here also, a warning message would be appropriate instead of
abort.
> +
> + return ret;
> }
> #else
> -static void cm_fx6_setup_i2c(void) { }
> +static int cm_fx6_setup_i2c(void) { return 0; }
> #endif
>
> #ifdef CONFIG_USB_EHCI_MX6
> @@ -411,9 +426,7 @@ int board_init(void)
> {
> gd->bd->bi_boot_params = PHYS_SDRAM_1 + 0x100;
> cm_fx6_setup_gpmi_nand();
> - cm_fx6_setup_i2c();
> -
> - return 0;
> + return cm_fx6_setup_i2c();
Same here, on cm-fx6, i2c is not that critical to U-Boot boot
process, and should not lead to hang().
> }
>
> int checkboard(void)
>
--
Regards,
Igor.
^ permalink raw reply [flat|nested] 25+ messages in thread* [U-Boot] [PATCH v2 07/11] dm: imx: Add error checking to setup_i2c()
2014-09-17 8:37 ` Igor Grinberg
@ 2014-09-17 13:56 ` Simon Glass
2014-09-17 14:16 ` Igor Grinberg
0 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2014-09-17 13:56 UTC (permalink / raw)
To: u-boot
Hi Igor,
On 17 September 2014 02:37, Igor Grinberg <grinberg@compulab.co.il> wrote:
> Hi Simon,
>
> On 09/17/14 06:51, Simon Glass wrote:
> > Since this function can fail, check its return value.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> > Changes in v2:
> > - Add new patch to add error checking to setup_i2c()
> >
> > arch/arm/imx-common/i2c-mxv7.c | 24 ++++++++++++++++-----
> > arch/arm/include/asm/imx-common/mxc_i2c.h | 4 ++--
> > board/compulab/cm_fx6/cm_fx6.c | 35
> +++++++++++++++++++++----------
> > 3 files changed, 45 insertions(+), 18 deletions(-)
> >
> > diff --git a/arch/arm/imx-common/i2c-mxv7.c
> b/arch/arm/imx-common/i2c-mxv7.c
> > index a580873..70cff5c 100644
> > --- a/arch/arm/imx-common/i2c-mxv7.c
> > +++ b/arch/arm/imx-common/i2c-mxv7.c
> > @@ -69,15 +69,29 @@ static void * const i2c_bases[] = {
> > };
> >
> > /* i2c_index can be from 0 - 2 */
> > -void setup_i2c(unsigned i2c_index, int speed, int slave_addr,
> > - struct i2c_pads_info *p)
> > +int setup_i2c(unsigned i2c_index, int speed, int slave_addr,
> > + struct i2c_pads_info *p)
> > {
> > + int ret;
> > +
> > if (i2c_index >= ARRAY_SIZE(i2c_bases))
> > - return;
> > + return -EINVAL;
> > /* Enable i2c clock */
> > - enable_i2c_clk(1, i2c_index);
> > + ret = enable_i2c_clk(1, i2c_index);
> > + if (ret)
> > + goto err_clk;
> > +
> > /* Make sure bus is idle */
> > - force_idle_bus(p);
> > + ret = force_idle_bus(p);
> > + if (ret)
> > + goto err_idle;
> > +
> > bus_i2c_init(i2c_bases[i2c_index], speed, slave_addr,
> > force_idle_bus, p);
> > +
> > + return 0;
> > +
> > +err_idle:
> > +err_clk:
> > + return ret;
> > }
> > diff --git a/arch/arm/include/asm/imx-common/mxc_i2c.h
> b/arch/arm/include/asm/imx-common/mxc_i2c.h
> > index 182c2f3..af86163 100644
> > --- a/arch/arm/include/asm/imx-common/mxc_i2c.h
> > +++ b/arch/arm/include/asm/imx-common/mxc_i2c.h
> > @@ -52,8 +52,8 @@ struct i2c_pads_info {
> > &mx6q_##name : &mx6s_##name
> > #endif
> >
> > -void setup_i2c(unsigned i2c_index, int speed, int slave_addr,
> > - struct i2c_pads_info *p);
> > +int setup_i2c(unsigned i2c_index, int speed, int slave_addr,
> > + struct i2c_pads_info *p);
> > void bus_i2c_init(void *base, int speed, int slave_addr,
> > int (*idle_bus_fn)(void *p), void *p);
> > int bus_i2c_read(void *base, uchar chip, uint addr, int alen, uchar
> *buf,
> > diff --git a/board/compulab/cm_fx6/cm_fx6.c
> b/board/compulab/cm_fx6/cm_fx6.c
> > index fdb8ebf..62c625a 100644
> > --- a/board/compulab/cm_fx6/cm_fx6.c
> > +++ b/board/compulab/cm_fx6/cm_fx6.c
> > @@ -69,7 +69,7 @@ static iomux_v3_cfg_t const sata_pads[] = {
> > IOMUX_PADS(PAD_EIM_BCLK__GPIO6_IO31 | MUX_PAD_CTRL(NO_PAD_CTRL)),
> > };
> >
> > -static void cm_fx6_setup_issd(void)
> > +static int cm_fx6_setup_issd(void)
> > {
> > SETUP_IOMUX_PADS(sata_pads);
> > /* Make sure this gpio has logical 0 value */
> > @@ -79,14 +79,18 @@ static void cm_fx6_setup_issd(void)
> > cm_fx6_sata_power(0);
> > mdelay(250);
> > cm_fx6_sata_power(1);
> > +
> > + return 0;
> > }
> >
> > #define CM_FX6_SATA_INIT_RETRIES 10
> > int sata_initialize(void)
> > {
> > - int err, i;
> > + int err, i, ret;
> >
> > - cm_fx6_setup_issd();
> > + ret = cm_fx6_setup_issd();
> > + if (ret)
> > + return ret;
>
> Hmm.. cm-fx6 may have iSSD not assembled and in this case
> it has bypasses for a (m)SATA socket on the baseboard.
> The socketed device of course is not controlled by those GPIOs.
> Therefore, I think, it would be incorrect to fail the boot process
> if there is a problem with an iSSD GPIO...
> Instead a warning message will be enough. Something like:
>
> ret = cm_fx6_setup_issd();
> if (ret)
> printf("Warning: iSSD setup failed!\n");
>
OK, my intent here was to add the missing gpio_request() calls so that
things would work with DM. I think I am being drawn into board error
policy! I'll give it another spin based on your comments.
>
> and then continue to the rest of SATA init.
>
> > for (i = 0; i < CM_FX6_SATA_INIT_RETRIES; i++) {
> > err = setup_sata();
> > if (err) {
> > @@ -141,14 +145,25 @@ I2C_PADS(i2c2_pads,
> > IMX_GPIO_NR(1, 6));
> >
> >
> > -static void cm_fx6_setup_i2c(void)
> > +static int cm_fx6_setup_i2c(void)
> > {
> > - setup_i2c(0, CONFIG_SYS_I2C_SPEED, 0x7f, I2C_PADS_INFO(i2c0_pads));
> > - setup_i2c(1, CONFIG_SYS_I2C_SPEED, 0x7f, I2C_PADS_INFO(i2c1_pads));
> > - setup_i2c(2, CONFIG_SYS_I2C_SPEED, 0x7f, I2C_PADS_INFO(i2c2_pads));
> > + int ret;
> > +
> > + ret = setup_i2c(0, CONFIG_SYS_I2C_SPEED, 0x7f,
> > + I2C_PADS_INFO(i2c0_pads));
> > + if (!ret) {
> > + ret = setup_i2c(1, CONFIG_SYS_I2C_SPEED, 0x7f,
> > + I2C_PADS_INFO(i2c1_pads));
> > + }
> > + if (!ret) {
> > + ret = setup_i2c(2, CONFIG_SYS_I2C_SPEED, 0x7f,
> > + I2C_PADS_INFO(i2c2_pads));
> > + }
>
> Almost same here, the fact that one of the (or even all) i2c buses
> fails to initialize, should not lead to hang()...
> The decision if this is a critical error or not should be decided
> by the board code.
> In this case, this is not a critical error and if one of the buses
> fails to initialize, the others should still try.
> So, here also, a warning message would be appropriate instead of
> abort.
>
> > +
> > + return ret;
> > }
> > #else
> > -static void cm_fx6_setup_i2c(void) { }
> > +static int cm_fx6_setup_i2c(void) { return 0; }
> > #endif
> >
> > #ifdef CONFIG_USB_EHCI_MX6
> > @@ -411,9 +426,7 @@ int board_init(void)
> > {
> > gd->bd->bi_boot_params = PHYS_SDRAM_1 + 0x100;
> > cm_fx6_setup_gpmi_nand();
> > - cm_fx6_setup_i2c();
> > -
> > - return 0;
> > + return cm_fx6_setup_i2c();
>
> Same here, on cm-fx6, i2c is not that critical to U-Boot boot
> process, and should not lead to hang().
>
> > }
> >
> > int checkboard(void)
> >
>
> --
> Regards,
> Igor.
>
Regards,
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread* [U-Boot] [PATCH v2 07/11] dm: imx: Add error checking to setup_i2c()
2014-09-17 13:56 ` Simon Glass
@ 2014-09-17 14:16 ` Igor Grinberg
0 siblings, 0 replies; 25+ messages in thread
From: Igor Grinberg @ 2014-09-17 14:16 UTC (permalink / raw)
To: u-boot
Hi Simon,
On 09/17/14 16:56, Simon Glass wrote:
> Hi Igor,
>
> On 17 September 2014 02:37, Igor Grinberg <grinberg at compulab.co.il <mailto:grinberg@compulab.co.il>> wrote:
>
> Hi Simon,
>
> On 09/17/14 06:51, Simon Glass wrote:
> > Since this function can fail, check its return value.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org <mailto:sjg@chromium.org>>
> > ---
> >
> > Changes in v2:
> > - Add new patch to add error checking to setup_i2c()
> >
> > arch/arm/imx-common/i2c-mxv7.c | 24 ++++++++++++++++-----
> > arch/arm/include/asm/imx-common/mxc_i2c.h | 4 ++--
> > board/compulab/cm_fx6/cm_fx6.c | 35 +++++++++++++++++++++----------
> > 3 files changed, 45 insertions(+), 18 deletions(-)
> >
> > diff --git a/arch/arm/imx-common/i2c-mxv7.c b/arch/arm/imx-common/i2c-mxv7.c
> > index a580873..70cff5c 100644
> > --- a/arch/arm/imx-common/i2c-mxv7.c
> > +++ b/arch/arm/imx-common/i2c-mxv7.c
> > @@ -69,15 +69,29 @@ static void * const i2c_bases[] = {
> > };
> >
> > /* i2c_index can be from 0 - 2 */
> > -void setup_i2c(unsigned i2c_index, int speed, int slave_addr,
> > - struct i2c_pads_info *p)
> > +int setup_i2c(unsigned i2c_index, int speed, int slave_addr,
> > + struct i2c_pads_info *p)
> > {
> > + int ret;
> > +
> > if (i2c_index >= ARRAY_SIZE(i2c_bases))
> > - return;
> > + return -EINVAL;
> > /* Enable i2c clock */
> > - enable_i2c_clk(1, i2c_index);
> > + ret = enable_i2c_clk(1, i2c_index);
> > + if (ret)
> > + goto err_clk;
> > +
> > /* Make sure bus is idle */
> > - force_idle_bus(p);
> > + ret = force_idle_bus(p);
> > + if (ret)
> > + goto err_idle;
> > +
> > bus_i2c_init(i2c_bases[i2c_index], speed, slave_addr,
> > force_idle_bus, p);
> > +
> > + return 0;
> > +
> > +err_idle:
> > +err_clk:
> > + return ret;
> > }
> > diff --git a/arch/arm/include/asm/imx-common/mxc_i2c.h b/arch/arm/include/asm/imx-common/mxc_i2c.h
> > index 182c2f3..af86163 100644
> > --- a/arch/arm/include/asm/imx-common/mxc_i2c.h
> > +++ b/arch/arm/include/asm/imx-common/mxc_i2c.h
> > @@ -52,8 +52,8 @@ struct i2c_pads_info {
> > &mx6q_##name : &mx6s_##name
> > #endif
> >
> > -void setup_i2c(unsigned i2c_index, int speed, int slave_addr,
> > - struct i2c_pads_info *p);
> > +int setup_i2c(unsigned i2c_index, int speed, int slave_addr,
> > + struct i2c_pads_info *p);
> > void bus_i2c_init(void *base, int speed, int slave_addr,
> > int (*idle_bus_fn)(void *p), void *p);
> > int bus_i2c_read(void *base, uchar chip, uint addr, int alen, uchar *buf,
> > diff --git a/board/compulab/cm_fx6/cm_fx6.c b/board/compulab/cm_fx6/cm_fx6.c
> > index fdb8ebf..62c625a 100644
> > --- a/board/compulab/cm_fx6/cm_fx6.c
> > +++ b/board/compulab/cm_fx6/cm_fx6.c
> > @@ -69,7 +69,7 @@ static iomux_v3_cfg_t const sata_pads[] = {
> > IOMUX_PADS(PAD_EIM_BCLK__GPIO6_IO31 | MUX_PAD_CTRL(NO_PAD_CTRL)),
> > };
> >
> > -static void cm_fx6_setup_issd(void)
> > +static int cm_fx6_setup_issd(void)
> > {
> > SETUP_IOMUX_PADS(sata_pads);
> > /* Make sure this gpio has logical 0 value */
> > @@ -79,14 +79,18 @@ static void cm_fx6_setup_issd(void)
> > cm_fx6_sata_power(0);
> > mdelay(250);
> > cm_fx6_sata_power(1);
> > +
> > + return 0;
> > }
> >
> > #define CM_FX6_SATA_INIT_RETRIES 10
> > int sata_initialize(void)
> > {
> > - int err, i;
> > + int err, i, ret;
> >
> > - cm_fx6_setup_issd();
> > + ret = cm_fx6_setup_issd();
> > + if (ret)
> > + return ret;
>
> Hmm.. cm-fx6 may have iSSD not assembled and in this case
> it has bypasses for a (m)SATA socket on the baseboard.
> The socketed device of course is not controlled by those GPIOs.
> Therefore, I think, it would be incorrect to fail the boot process
> if there is a problem with an iSSD GPIO...
> Instead a warning message will be enough. Something like:
>
> ret = cm_fx6_setup_issd();
> if (ret)
> printf("Warning: iSSD setup failed!\n");
>
>
> OK, my intent here was to add the missing gpio_request() calls so that things would work with DM. I think I am being drawn into board error policy! I'll give it another spin based on your comments.
I understand. Thanks for going for another round.
If you feel like it gets too much board specific, we can send a follow up.
>
>
>
> and then continue to the rest of SATA init.
>
> > for (i = 0; i < CM_FX6_SATA_INIT_RETRIES; i++) {
> > err = setup_sata();
> > if (err) {
> > @@ -141,14 +145,25 @@ I2C_PADS(i2c2_pads,
> > IMX_GPIO_NR(1, 6));
> >
> >
> > -static void cm_fx6_setup_i2c(void)
> > +static int cm_fx6_setup_i2c(void)
> > {
> > - setup_i2c(0, CONFIG_SYS_I2C_SPEED, 0x7f, I2C_PADS_INFO(i2c0_pads));
> > - setup_i2c(1, CONFIG_SYS_I2C_SPEED, 0x7f, I2C_PADS_INFO(i2c1_pads));
> > - setup_i2c(2, CONFIG_SYS_I2C_SPEED, 0x7f, I2C_PADS_INFO(i2c2_pads));
> > + int ret;
> > +
> > + ret = setup_i2c(0, CONFIG_SYS_I2C_SPEED, 0x7f,
> > + I2C_PADS_INFO(i2c0_pads));
> > + if (!ret) {
> > + ret = setup_i2c(1, CONFIG_SYS_I2C_SPEED, 0x7f,
> > + I2C_PADS_INFO(i2c1_pads));
> > + }
> > + if (!ret) {
> > + ret = setup_i2c(2, CONFIG_SYS_I2C_SPEED, 0x7f,
> > + I2C_PADS_INFO(i2c2_pads));
> > + }
>
> Almost same here, the fact that one of the (or even all) i2c buses
> fails to initialize, should not lead to hang()...
> The decision if this is a critical error or not should be decided
> by the board code.
> In this case, this is not a critical error and if one of the buses
> fails to initialize, the others should still try.
> So, here also, a warning message would be appropriate instead of
> abort.
>
> > +
> > + return ret;
> > }
> > #else
> > -static void cm_fx6_setup_i2c(void) { }
> > +static int cm_fx6_setup_i2c(void) { return 0; }
> > #endif
> >
> > #ifdef CONFIG_USB_EHCI_MX6
> > @@ -411,9 +426,7 @@ int board_init(void)
> > {
> > gd->bd->bi_boot_params = PHYS_SDRAM_1 + 0x100;
> > cm_fx6_setup_gpmi_nand();
> > - cm_fx6_setup_i2c();
> > -
> > - return 0;
> > + return cm_fx6_setup_i2c();
>
> Same here, on cm-fx6, i2c is not that critical to U-Boot boot
> process, and should not lead to hang().
>
> > }
> >
> > int checkboard(void)
> >
>
> --
> Regards,
> Igor.
>
>
>
> Regards,
> Simon
>
--
Regards,
Igor.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [U-Boot] [PATCH v2 08/11] dm: imx: Use gpio_request() to request GPIOs
2014-09-17 3:51 [U-Boot] [PATCH v2 0/11] dm: imx: Add driver model support for GPIO and serial on cm_fx6 Simon Glass
` (6 preceding siblings ...)
2014-09-17 3:51 ` [U-Boot] [PATCH v2 07/11] dm: imx: Add error checking to setup_i2c() Simon Glass
@ 2014-09-17 3:51 ` Simon Glass
2014-09-17 12:13 ` Igor Grinberg
2014-09-17 3:51 ` [U-Boot] [PATCH v2 09/11] dm: imx: gpio: Support driver model in MXC gpio driver Simon Glass
` (2 subsequent siblings)
10 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2014-09-17 3:51 UTC (permalink / raw)
To: u-boot
GPIOs should be requested before use. Without this, driver model will not
permit the GPIO to be used.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Check return values of gpio_request()
arch/arm/imx-common/i2c-mxv7.c | 24 ++++++++++++++++++++++++
board/compulab/cm_fx6/cm_fx6.c | 15 +++++++++++++++
board/compulab/cm_fx6/common.c | 7 +++++++
3 files changed, 46 insertions(+)
diff --git a/arch/arm/imx-common/i2c-mxv7.c b/arch/arm/imx-common/i2c-mxv7.c
index 70cff5c..aaf6936 100644
--- a/arch/arm/imx-common/i2c-mxv7.c
+++ b/arch/arm/imx-common/i2c-mxv7.c
@@ -4,6 +4,7 @@
* SPDX-License-Identifier: GPL-2.0+
*/
#include <common.h>
+#include <malloc.h>
#include <asm/arch/clock.h>
#include <asm/arch/imx-regs.h>
#include <asm/errno.h>
@@ -72,10 +73,26 @@ static void * const i2c_bases[] = {
int setup_i2c(unsigned i2c_index, int speed, int slave_addr,
struct i2c_pads_info *p)
{
+ char *name1, *name2;
int ret;
if (i2c_index >= ARRAY_SIZE(i2c_bases))
return -EINVAL;
+
+ name1 = malloc(9);
+ name2 = malloc(9);
+ if (!name1 || !name2)
+ return -ENOMEM;
+ sprintf(name1, "i2c_sda%d", i2c_index);
+ sprintf(name2, "i2c_scl%d", i2c_index);
+ ret = gpio_request(p->sda.gp, name1);
+ if (ret)
+ goto err_req1;
+
+ ret = gpio_request(p->scl.gp, name2);
+ if (ret)
+ goto err_req2;
+
/* Enable i2c clock */
ret = enable_i2c_clk(1, i2c_index);
if (ret)
@@ -93,5 +110,12 @@ int setup_i2c(unsigned i2c_index, int speed, int slave_addr,
err_idle:
err_clk:
+ gpio_free(p->scl.gp);
+err_req2:
+ gpio_free(p->sda.gp);
+err_req1:
+ free(name1);
+ free(name2);
+
return ret;
}
diff --git a/board/compulab/cm_fx6/cm_fx6.c b/board/compulab/cm_fx6/cm_fx6.c
index 62c625a..10e31b6 100644
--- a/board/compulab/cm_fx6/cm_fx6.c
+++ b/board/compulab/cm_fx6/cm_fx6.c
@@ -71,8 +71,21 @@ static iomux_v3_cfg_t const sata_pads[] = {
static int cm_fx6_setup_issd(void)
{
+ int ret;
+ int i;
+
SETUP_IOMUX_PADS(sata_pads);
+
+ for (i = 0; i < ARRAY_SIZE(cm_fx6_issd_gpios); i++) {
+ ret = gpio_request(cm_fx6_issd_gpios[i], "sata");
+ if (ret)
+ return ret;
+ }
+
/* Make sure this gpio has logical 0 value */
+ ret = gpio_request(CM_FX6_SATA_PWLOSS_INT, "sata_pwloss_int");
+ if (ret)
+ return ret;
gpio_direction_output(CM_FX6_SATA_PWLOSS_INT, 0);
udelay(100);
@@ -182,6 +195,7 @@ static int cm_fx6_usb_hub_reset(void)
}
SETUP_IOMUX_PAD(PAD_SD3_RST__GPIO7_IO08 | MUX_PAD_CTRL(NO_PAD_CTRL));
+ gpio_request(CM_FX6_USB_HUB_RST, "usb_hub_rst");
gpio_direction_output(CM_FX6_USB_HUB_RST, 0);
udelay(10);
gpio_direction_output(CM_FX6_USB_HUB_RST, 1);
@@ -339,6 +353,7 @@ int board_eth_init(bd_t *bis)
SETUP_IOMUX_PADS(enet_pads);
/* phy reset */
+ gpio_request(CM_FX6_ENET_NRST, "enet_nrst");
gpio_direction_output(CM_FX6_ENET_NRST, 0);
udelay(500);
gpio_set_value(CM_FX6_ENET_NRST, 1);
diff --git a/board/compulab/cm_fx6/common.c b/board/compulab/cm_fx6/common.c
index 1f39679..e4d7e2e 100644
--- a/board/compulab/cm_fx6/common.c
+++ b/board/compulab/cm_fx6/common.c
@@ -79,6 +79,13 @@ void cm_fx6_set_ecspi_iomux(void)
int board_spi_cs_gpio(unsigned bus, unsigned cs)
{
+#ifndef CONFIG_SPL_BUILD
+ int ret;
+
+ ret = gpio_request(CM_FX6_ECSPI_BUS0_CS0, "ecspi_bus0_cs0");
+ if (ret)
+ return ret;
+#endif
return (bus == 0 && cs == 0) ? (CM_FX6_ECSPI_BUS0_CS0) : -1;
}
#endif
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related [flat|nested] 25+ messages in thread* [U-Boot] [PATCH v2 08/11] dm: imx: Use gpio_request() to request GPIOs
2014-09-17 3:51 ` [U-Boot] [PATCH v2 08/11] dm: imx: Use gpio_request() to request GPIOs Simon Glass
@ 2014-09-17 12:13 ` Igor Grinberg
2014-09-17 14:00 ` Simon Glass
0 siblings, 1 reply; 25+ messages in thread
From: Igor Grinberg @ 2014-09-17 12:13 UTC (permalink / raw)
To: u-boot
Hi Simon,
On 09/17/14 06:51, Simon Glass wrote:
> GPIOs should be requested before use. Without this, driver model will not
> permit the GPIO to be used.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v2:
> - Check return values of gpio_request()
>
> arch/arm/imx-common/i2c-mxv7.c | 24 ++++++++++++++++++++++++
> board/compulab/cm_fx6/cm_fx6.c | 15 +++++++++++++++
> board/compulab/cm_fx6/common.c | 7 +++++++
> 3 files changed, 46 insertions(+)
>
> diff --git a/arch/arm/imx-common/i2c-mxv7.c b/arch/arm/imx-common/i2c-mxv7.c
> index 70cff5c..aaf6936 100644
> --- a/arch/arm/imx-common/i2c-mxv7.c
> +++ b/arch/arm/imx-common/i2c-mxv7.c
> @@ -4,6 +4,7 @@
> * SPDX-License-Identifier: GPL-2.0+
> */
> #include <common.h>
> +#include <malloc.h>
> #include <asm/arch/clock.h>
> #include <asm/arch/imx-regs.h>
> #include <asm/errno.h>
> @@ -72,10 +73,26 @@ static void * const i2c_bases[] = {
> int setup_i2c(unsigned i2c_index, int speed, int slave_addr,
> struct i2c_pads_info *p)
> {
> + char *name1, *name2;
> int ret;
>
> if (i2c_index >= ARRAY_SIZE(i2c_bases))
> return -EINVAL;
> +
> + name1 = malloc(9);
> + name2 = malloc(9);
> + if (!name1 || !name2)
> + return -ENOMEM;
> + sprintf(name1, "i2c_sda%d", i2c_index);
> + sprintf(name2, "i2c_scl%d", i2c_index);
> + ret = gpio_request(p->sda.gp, name1);
> + if (ret)
> + goto err_req1;
> +
> + ret = gpio_request(p->scl.gp, name2);
> + if (ret)
> + goto err_req2;
> +
> /* Enable i2c clock */
> ret = enable_i2c_clk(1, i2c_index);
> if (ret)
> @@ -93,5 +110,12 @@ int setup_i2c(unsigned i2c_index, int speed, int slave_addr,
>
> err_idle:
> err_clk:
> + gpio_free(p->scl.gp);
> +err_req2:
> + gpio_free(p->sda.gp);
> +err_req1:
> + free(name1);
> + free(name2);
> +
> return ret;
> }
> diff --git a/board/compulab/cm_fx6/cm_fx6.c b/board/compulab/cm_fx6/cm_fx6.c
> index 62c625a..10e31b6 100644
> --- a/board/compulab/cm_fx6/cm_fx6.c
> +++ b/board/compulab/cm_fx6/cm_fx6.c
> @@ -71,8 +71,21 @@ static iomux_v3_cfg_t const sata_pads[] = {
>
> static int cm_fx6_setup_issd(void)
> {
> + int ret;
> + int i;
> +
> SETUP_IOMUX_PADS(sata_pads);
> +
> + for (i = 0; i < ARRAY_SIZE(cm_fx6_issd_gpios); i++) {
> + ret = gpio_request(cm_fx6_issd_gpios[i], "sata");
> + if (ret)
> + return ret;
> + }
> +
> /* Make sure this gpio has logical 0 value */
> + ret = gpio_request(CM_FX6_SATA_PWLOSS_INT, "sata_pwloss_int");
> + if (ret)
> + return ret;
> gpio_direction_output(CM_FX6_SATA_PWLOSS_INT, 0);
> udelay(100);
>
> @@ -182,6 +195,7 @@ static int cm_fx6_usb_hub_reset(void)
> }
>
> SETUP_IOMUX_PAD(PAD_SD3_RST__GPIO7_IO08 | MUX_PAD_CTRL(NO_PAD_CTRL));
> + gpio_request(CM_FX6_USB_HUB_RST, "usb_hub_rst");
This is confusing... This GPIO should be already requested
several lines above.
> gpio_direction_output(CM_FX6_USB_HUB_RST, 0);
> udelay(10);
> gpio_direction_output(CM_FX6_USB_HUB_RST, 1);
> @@ -339,6 +353,7 @@ int board_eth_init(bd_t *bis)
>
> SETUP_IOMUX_PADS(enet_pads);
> /* phy reset */
> + gpio_request(CM_FX6_ENET_NRST, "enet_nrst");
Error check?
This one also should not fail the boot process, just abort the
Ethernet init.
> gpio_direction_output(CM_FX6_ENET_NRST, 0);
> udelay(500);
> gpio_set_value(CM_FX6_ENET_NRST, 1);
> diff --git a/board/compulab/cm_fx6/common.c b/board/compulab/cm_fx6/common.c
> index 1f39679..e4d7e2e 100644
> --- a/board/compulab/cm_fx6/common.c
> +++ b/board/compulab/cm_fx6/common.c
> @@ -79,6 +79,13 @@ void cm_fx6_set_ecspi_iomux(void)
>
> int board_spi_cs_gpio(unsigned bus, unsigned cs)
> {
> +#ifndef CONFIG_SPL_BUILD
> + int ret;
> +
> + ret = gpio_request(CM_FX6_ECSPI_BUS0_CS0, "ecspi_bus0_cs0");
> + if (ret)
> + return ret;
> +#endif
Is there a reason for excluding this request from spl builds?
We do build with CONFIG_SPL_GPIO_SUPPORT and we don't have a
problem with spl size.
So, if there is no strong reason to exclude it from spl build,
I'd like to remove the #ifndef.
> return (bus == 0 && cs == 0) ? (CM_FX6_ECSPI_BUS0_CS0) : -1;
> }
> #endif
>
--
Regards,
Igor.
^ permalink raw reply [flat|nested] 25+ messages in thread* [U-Boot] [PATCH v2 08/11] dm: imx: Use gpio_request() to request GPIOs
2014-09-17 12:13 ` Igor Grinberg
@ 2014-09-17 14:00 ` Simon Glass
2014-09-17 14:31 ` Igor Grinberg
0 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2014-09-17 14:00 UTC (permalink / raw)
To: u-boot
Hi Igor,
On 17 September 2014 06:13, Igor Grinberg <grinberg@compulab.co.il> wrote:
> Hi Simon,
>
> On 09/17/14 06:51, Simon Glass wrote:
> > GPIOs should be requested before use. Without this, driver model will not
> > permit the GPIO to be used.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> > Changes in v2:
> > - Check return values of gpio_request()
> >
> > arch/arm/imx-common/i2c-mxv7.c | 24 ++++++++++++++++++++++++
> > board/compulab/cm_fx6/cm_fx6.c | 15 +++++++++++++++
> > board/compulab/cm_fx6/common.c | 7 +++++++
> > 3 files changed, 46 insertions(+)
> >
> > diff --git a/arch/arm/imx-common/i2c-mxv7.c
> b/arch/arm/imx-common/i2c-mxv7.c
> > index 70cff5c..aaf6936 100644
> > --- a/arch/arm/imx-common/i2c-mxv7.c
> > +++ b/arch/arm/imx-common/i2c-mxv7.c
> > @@ -4,6 +4,7 @@
> > * SPDX-License-Identifier: GPL-2.0+
> > */
> > #include <common.h>
> > +#include <malloc.h>
> > #include <asm/arch/clock.h>
> > #include <asm/arch/imx-regs.h>
> > #include <asm/errno.h>
> > @@ -72,10 +73,26 @@ static void * const i2c_bases[] = {
> > int setup_i2c(unsigned i2c_index, int speed, int slave_addr,
> > struct i2c_pads_info *p)
> > {
> > + char *name1, *name2;
> > int ret;
> >
> > if (i2c_index >= ARRAY_SIZE(i2c_bases))
> > return -EINVAL;
> > +
> > + name1 = malloc(9);
> > + name2 = malloc(9);
> > + if (!name1 || !name2)
> > + return -ENOMEM;
> > + sprintf(name1, "i2c_sda%d", i2c_index);
> > + sprintf(name2, "i2c_scl%d", i2c_index);
> > + ret = gpio_request(p->sda.gp, name1);
> > + if (ret)
> > + goto err_req1;
> > +
> > + ret = gpio_request(p->scl.gp, name2);
> > + if (ret)
> > + goto err_req2;
> > +
> > /* Enable i2c clock */
> > ret = enable_i2c_clk(1, i2c_index);
> > if (ret)
> > @@ -93,5 +110,12 @@ int setup_i2c(unsigned i2c_index, int speed, int
> slave_addr,
> >
> > err_idle:
> > err_clk:
> > + gpio_free(p->scl.gp);
> > +err_req2:
> > + gpio_free(p->sda.gp);
> > +err_req1:
> > + free(name1);
> > + free(name2);
> > +
> > return ret;
> > }
> > diff --git a/board/compulab/cm_fx6/cm_fx6.c
> b/board/compulab/cm_fx6/cm_fx6.c
> > index 62c625a..10e31b6 100644
> > --- a/board/compulab/cm_fx6/cm_fx6.c
> > +++ b/board/compulab/cm_fx6/cm_fx6.c
> > @@ -71,8 +71,21 @@ static iomux_v3_cfg_t const sata_pads[] = {
> >
> > static int cm_fx6_setup_issd(void)
> > {
> > + int ret;
> > + int i;
> > +
> > SETUP_IOMUX_PADS(sata_pads);
> > +
> > + for (i = 0; i < ARRAY_SIZE(cm_fx6_issd_gpios); i++) {
> > + ret = gpio_request(cm_fx6_issd_gpios[i], "sata");
> > + if (ret)
> > + return ret;
> > + }
> > +
> > /* Make sure this gpio has logical 0 value */
> > + ret = gpio_request(CM_FX6_SATA_PWLOSS_INT, "sata_pwloss_int");
> > + if (ret)
> > + return ret;
> > gpio_direction_output(CM_FX6_SATA_PWLOSS_INT, 0);
> > udelay(100);
> >
> > @@ -182,6 +195,7 @@ static int cm_fx6_usb_hub_reset(void)
> > }
> >
> > SETUP_IOMUX_PAD(PAD_SD3_RST__GPIO7_IO08 |
> MUX_PAD_CTRL(NO_PAD_CTRL));
> > + gpio_request(CM_FX6_USB_HUB_RST, "usb_hub_rst");
>
> This is confusing... This GPIO should be already requested
> several lines above.
>
OK
>
> > gpio_direction_output(CM_FX6_USB_HUB_RST, 0);
> > udelay(10);
> > gpio_direction_output(CM_FX6_USB_HUB_RST, 1);
> > @@ -339,6 +353,7 @@ int board_eth_init(bd_t *bis)
> >
> > SETUP_IOMUX_PADS(enet_pads);
> > /* phy reset */
> > + gpio_request(CM_FX6_ENET_NRST, "enet_nrst");
>
> Error check?
> This one also should not fail the boot process, just abort the
> Ethernet init.
>
OK
>
> > gpio_direction_output(CM_FX6_ENET_NRST, 0);
> > udelay(500);
> > gpio_set_value(CM_FX6_ENET_NRST, 1);
> > diff --git a/board/compulab/cm_fx6/common.c
> b/board/compulab/cm_fx6/common.c
> > index 1f39679..e4d7e2e 100644
> > --- a/board/compulab/cm_fx6/common.c
> > +++ b/board/compulab/cm_fx6/common.c
> > @@ -79,6 +79,13 @@ void cm_fx6_set_ecspi_iomux(void)
> >
> > int board_spi_cs_gpio(unsigned bus, unsigned cs)
> > {
> > +#ifndef CONFIG_SPL_BUILD
> > + int ret;
> > +
> > + ret = gpio_request(CM_FX6_ECSPI_BUS0_CS0, "ecspi_bus0_cs0");
> > + if (ret)
> > + return ret;
> > +#endif
>
> Is there a reason for excluding this request from spl builds?
> We do build with CONFIG_SPL_GPIO_SUPPORT and we don't have a
> problem with spl size.
> So, if there is no strong reason to exclude it from spl build,
> I'd like to remove the #ifndef.
>
It isn't supported by DM yet.
>
> > return (bus == 0 && cs == 0) ? (CM_FX6_ECSPI_BUS0_CS0) : -1;
> > }
> > #endif
> >
>
> --
> Regards,
> Igor.
>
Regards,
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread* [U-Boot] [PATCH v2 08/11] dm: imx: Use gpio_request() to request GPIOs
2014-09-17 14:00 ` Simon Glass
@ 2014-09-17 14:31 ` Igor Grinberg
2014-09-17 14:34 ` Simon Glass
0 siblings, 1 reply; 25+ messages in thread
From: Igor Grinberg @ 2014-09-17 14:31 UTC (permalink / raw)
To: u-boot
On 09/17/14 17:00, Simon Glass wrote:
> Hi Igor,
>
> On 17 September 2014 06:13, Igor Grinberg <grinberg at compulab.co.il <mailto:grinberg@compulab.co.il>> wrote:
>
> Hi Simon,
>
> On 09/17/14 06:51, Simon Glass wrote:
> > GPIOs should be requested before use. Without this, driver model will not
> > permit the GPIO to be used.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org <mailto:sjg@chromium.org>>
> > ---
> >
> > Changes in v2:
> > - Check return values of gpio_request()
> >
> > arch/arm/imx-common/i2c-mxv7.c | 24 ++++++++++++++++++++++++
> > board/compulab/cm_fx6/cm_fx6.c | 15 +++++++++++++++
> > board/compulab/cm_fx6/common.c | 7 +++++++
> > 3 files changed, 46 insertions(+)
> >
[...]
> > diff --git a/board/compulab/cm_fx6/common.c b/board/compulab/cm_fx6/common.c
> > index 1f39679..e4d7e2e 100644
> > --- a/board/compulab/cm_fx6/common.c
> > +++ b/board/compulab/cm_fx6/common.c
> > @@ -79,6 +79,13 @@ void cm_fx6_set_ecspi_iomux(void)
> >
> > int board_spi_cs_gpio(unsigned bus, unsigned cs)
> > {
> > +#ifndef CONFIG_SPL_BUILD
> > + int ret;
> > +
> > + ret = gpio_request(CM_FX6_ECSPI_BUS0_CS0, "ecspi_bus0_cs0");
> > + if (ret)
> > + return ret;
> > +#endif
>
> Is there a reason for excluding this request from spl builds?
> We do build with CONFIG_SPL_GPIO_SUPPORT and we don't have a
> problem with spl size.
> So, if there is no strong reason to exclude it from spl build,
> I'd like to remove the #ifndef.
>
>
> It isn't supported by DM yet.
AFAICS, gpio_request() is available for SPL.
So in such case the non-DM version will be used and
I can't see any problem with this (unless it does not work for some reason).
>
>
>
> > return (bus == 0 && cs == 0) ? (CM_FX6_ECSPI_BUS0_CS0) : -1;
> > }
> > #endif
> >
>
> --
> Regards,
> Igor.
>
>
> Regards,
> Simon
>
--
Regards,
Igor.
^ permalink raw reply [flat|nested] 25+ messages in thread* [U-Boot] [PATCH v2 08/11] dm: imx: Use gpio_request() to request GPIOs
2014-09-17 14:31 ` Igor Grinberg
@ 2014-09-17 14:34 ` Simon Glass
2014-09-18 7:23 ` Igor Grinberg
0 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2014-09-17 14:34 UTC (permalink / raw)
To: u-boot
Hi Igor,
On 17 September 2014 08:31, Igor Grinberg <grinberg@compulab.co.il> wrote:
> On 09/17/14 17:00, Simon Glass wrote:
> > Hi Igor,
> >
> > On 17 September 2014 06:13, Igor Grinberg <grinberg@compulab.co.il
> <mailto:grinberg@compulab.co.il>> wrote:
> >
> > Hi Simon,
> >
> > On 09/17/14 06:51, Simon Glass wrote:
> > > GPIOs should be requested before use. Without this, driver model
> will not
> > > permit the GPIO to be used.
> > >
> > > Signed-off-by: Simon Glass <sjg@chromium.org <mailto:
> sjg at chromium.org>>
> > > ---
> > >
> > > Changes in v2:
> > > - Check return values of gpio_request()
> > >
> > > arch/arm/imx-common/i2c-mxv7.c | 24 ++++++++++++++++++++++++
> > > board/compulab/cm_fx6/cm_fx6.c | 15 +++++++++++++++
> > > board/compulab/cm_fx6/common.c | 7 +++++++
> > > 3 files changed, 46 insertions(+)
> > >
>
> [...]
>
> > > diff --git a/board/compulab/cm_fx6/common.c
> b/board/compulab/cm_fx6/common.c
> > > index 1f39679..e4d7e2e 100644
> > > --- a/board/compulab/cm_fx6/common.c
> > > +++ b/board/compulab/cm_fx6/common.c
> > > @@ -79,6 +79,13 @@ void cm_fx6_set_ecspi_iomux(void)
> > >
> > > int board_spi_cs_gpio(unsigned bus, unsigned cs)
> > > {
> > > +#ifndef CONFIG_SPL_BUILD
> > > + int ret;
> > > +
> > > + ret = gpio_request(CM_FX6_ECSPI_BUS0_CS0, "ecspi_bus0_cs0");
> > > + if (ret)
> > > + return ret;
> > > +#endif
> >
> > Is there a reason for excluding this request from spl builds?
> > We do build with CONFIG_SPL_GPIO_SUPPORT and we don't have a
> > problem with spl size.
> > So, if there is no strong reason to exclude it from spl build,
> > I'd like to remove the #ifndef.
> >
> >
> > It isn't supported by DM yet.
>
> AFAICS, gpio_request() is available for SPL.
> So in such case the non-DM version will be used and
> I can't see any problem with this (unless it does not work for some
> reason).
>
I suggest that if you wanted the gpio_request() in the non-DM case you
would already have added it?
I would prefer not to have an #ifdef CONFIG_DM_GPIO here also. Can we
revisit when DM supports SPL?
Regards,
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread* [U-Boot] [PATCH v2 08/11] dm: imx: Use gpio_request() to request GPIOs
2014-09-17 14:34 ` Simon Glass
@ 2014-09-18 7:23 ` Igor Grinberg
0 siblings, 0 replies; 25+ messages in thread
From: Igor Grinberg @ 2014-09-18 7:23 UTC (permalink / raw)
To: u-boot
On 09/17/14 17:34, Simon Glass wrote:
> Hi Igor,
>
> On 17 September 2014 08:31, Igor Grinberg <grinberg at compulab.co.il <mailto:grinberg@compulab.co.il>> wrote:
>
> On 09/17/14 17:00, Simon Glass wrote:
> > Hi Igor,
> >
> > On 17 September 2014 06:13, Igor Grinberg <grinberg at compulab.co.il <mailto:grinberg@compulab.co.il> <mailto:grinberg at compulab.co.il <mailto:grinberg@compulab.co.il>>> wrote:
> >
> > Hi Simon,
> >
> > On 09/17/14 06:51, Simon Glass wrote:
> > > GPIOs should be requested before use. Without this, driver model will not
> > > permit the GPIO to be used.
> > >
> > > Signed-off-by: Simon Glass <sjg at chromium.org <mailto:sjg@chromium.org> <mailto:sjg at chromium.org <mailto:sjg@chromium.org>>>
> > > ---
> > >
> > > Changes in v2:
> > > - Check return values of gpio_request()
> > >
> > > arch/arm/imx-common/i2c-mxv7.c | 24 ++++++++++++++++++++++++
> > > board/compulab/cm_fx6/cm_fx6.c | 15 +++++++++++++++
> > > board/compulab/cm_fx6/common.c | 7 +++++++
> > > 3 files changed, 46 insertions(+)
> > >
>
> [...]
>
> > > diff --git a/board/compulab/cm_fx6/common.c b/board/compulab/cm_fx6/common.c
> > > index 1f39679..e4d7e2e 100644
> > > --- a/board/compulab/cm_fx6/common.c
> > > +++ b/board/compulab/cm_fx6/common.c
> > > @@ -79,6 +79,13 @@ void cm_fx6_set_ecspi_iomux(void)
> > >
> > > int board_spi_cs_gpio(unsigned bus, unsigned cs)
> > > {
> > > +#ifndef CONFIG_SPL_BUILD
> > > + int ret;
> > > +
> > > + ret = gpio_request(CM_FX6_ECSPI_BUS0_CS0, "ecspi_bus0_cs0");
> > > + if (ret)
> > > + return ret;
> > > +#endif
> >
> > Is there a reason for excluding this request from spl builds?
> > We do build with CONFIG_SPL_GPIO_SUPPORT and we don't have a
> > problem with spl size.
> > So, if there is no strong reason to exclude it from spl build,
> > I'd like to remove the #ifndef.
> >
> >
> > It isn't supported by DM yet.
>
> AFAICS, gpio_request() is available for SPL.
> So in such case the non-DM version will be used and
> I can't see any problem with this (unless it does not work for some reason).
>
>
> I suggest that if you wanted the gpio_request() in the non-DM case you would already have added it?
I would thought so too... Unfortunately, it has slipped through...
>
> I would prefer not to have an #ifdef CONFIG_DM_GPIO here also. Can we revisit when DM supports SPL?
Yes, no problem.
--
Regards,
Igor.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [U-Boot] [PATCH v2 09/11] dm: imx: gpio: Support driver model in MXC gpio driver
2014-09-17 3:51 [U-Boot] [PATCH v2 0/11] dm: imx: Add driver model support for GPIO and serial on cm_fx6 Simon Glass
` (7 preceding siblings ...)
2014-09-17 3:51 ` [U-Boot] [PATCH v2 08/11] dm: imx: Use gpio_request() to request GPIOs Simon Glass
@ 2014-09-17 3:51 ` Simon Glass
2014-09-17 13:00 ` Igor Grinberg
2014-09-17 3:51 ` [U-Boot] [PATCH v2 10/11] dm: imx: serial: Support driver model in the MXC serial driver Simon Glass
2014-09-17 3:51 ` [U-Boot] [PATCH v2 11/11] dm: imx: Move cm_fx6 to use driver model for serial and GPIO Simon Glass
10 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2014-09-17 3:51 UTC (permalink / raw)
To: u-boot
Add driver model support with this driver. In this case the platform data
is in the driver. It would be better to put this into an SOC-specific file,
but this is best attempted when more boards are moved over to use driver
model.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Change 'reserved' to 'requested'
- Add an internal function to check if a GPIO is requested
- Tidy up confusing code that creates names for gpio_request()
drivers/gpio/mxc_gpio.c | 302 +++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 301 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/mxc_gpio.c b/drivers/gpio/mxc_gpio.c
index 6a572d5..9435d2f 100644
--- a/drivers/gpio/mxc_gpio.c
+++ b/drivers/gpio/mxc_gpio.c
@@ -8,16 +8,31 @@
* SPDX-License-Identifier: GPL-2.0+
*/
#include <common.h>
+#include <errno.h>
+#include <dm.h>
+#include <malloc.h>
#include <asm/arch/imx-regs.h>
#include <asm/gpio.h>
#include <asm/io.h>
-#include <errno.h>
enum mxc_gpio_direction {
MXC_GPIO_DIRECTION_IN,
MXC_GPIO_DIRECTION_OUT,
};
+#define GPIO_NAME_SIZE 20
+#define GPIO_PER_BANK 32
+
+struct mxc_gpio_plat {
+ struct gpio_regs *regs;
+};
+
+struct mxc_bank_info {
+ char label[GPIO_PER_BANK][GPIO_NAME_SIZE];
+ struct gpio_regs *regs;
+};
+
+#ifndef CONFIG_DM_GPIO
#define GPIO_TO_PORT(n) (n / 32)
/* GPIO port description */
@@ -134,3 +149,288 @@ int gpio_direction_output(unsigned gpio, int value)
return mxc_gpio_direction(gpio, MXC_GPIO_DIRECTION_OUT);
}
+#endif
+
+#ifdef CONFIG_DM_GPIO
+/**
+ * gpio_is_requested() - check if a GPIO has been requested
+ *
+ * @bank: Bank to check
+ * @offset: GPIO offset within bank to check
+ * @return true if marked as requested, false if not
+ */
+static inline bool gpio_is_requested(struct mxc_bank_info *bank, int offset)
+{
+ return *bank->label[offset] != '\0';
+}
+
+static int mxc_gpio_is_output(struct gpio_regs *regs, int offset)
+{
+ u32 val;
+
+ val = readl(®s->gpio_dir);
+
+ return val & (1 << offset) ? 1 : 0;
+}
+
+static void mxc_gpio_bank_direction(struct gpio_regs *regs, int offset,
+ enum mxc_gpio_direction direction)
+{
+ u32 l;
+
+ l = readl(®s->gpio_dir);
+
+ switch (direction) {
+ case MXC_GPIO_DIRECTION_OUT:
+ l |= 1 << offset;
+ break;
+ case MXC_GPIO_DIRECTION_IN:
+ l &= ~(1 << offset);
+ }
+ writel(l, ®s->gpio_dir);
+}
+
+static void mxc_gpio_bank_set_value(struct gpio_regs *regs, int offset,
+ int value)
+{
+ u32 l;
+
+ l = readl(®s->gpio_dr);
+ if (value)
+ l |= 1 << offset;
+ else
+ l &= ~(1 << offset);
+ writel(l, ®s->gpio_dr);
+}
+
+static int mxc_gpio_bank_get_value(struct gpio_regs *regs, int offset)
+{
+ return (readl(®s->gpio_psr) >> offset) & 0x01;
+}
+
+static int mxc_gpio_bank_get_output_value(struct gpio_regs *regs, int offset)
+{
+ return (readl(®s->gpio_dr) >> offset) & 0x01;
+}
+
+static int check_requested(struct udevice *dev, unsigned offset,
+ const char *func)
+{
+ struct mxc_bank_info *bank = dev_get_priv(dev);
+ struct gpio_dev_priv *uc_priv = dev->uclass_priv;
+
+ if (!gpio_is_requested(bank, offset)) {
+ printf("mxc_gpio: %s: error: gpio %s%d not requested\n",
+ func, uc_priv->bank_name, offset);
+ return -EPERM;
+ }
+
+ return 0;
+}
+
+/* set GPIO pin 'gpio' as an input */
+static int mxc_gpio_direction_input(struct udevice *dev, unsigned offset)
+{
+ struct mxc_bank_info *bank = dev_get_priv(dev);
+ int ret;
+
+ ret = check_requested(dev, offset, __func__);
+ if (ret)
+ return ret;
+
+ /* Configure GPIO direction as input. */
+ mxc_gpio_bank_direction(bank->regs, offset, MXC_GPIO_DIRECTION_IN);
+
+ return 0;
+}
+
+/* set GPIO pin 'gpio' as an output, with polarity 'value' */
+static int mxc_gpio_direction_output(struct udevice *dev, unsigned offset,
+ int value)
+{
+ struct mxc_bank_info *bank = dev_get_priv(dev);
+ int ret;
+
+ ret = check_requested(dev, offset, __func__);
+ if (ret)
+ return ret;
+
+ /* Configure GPIO output value. */
+ mxc_gpio_bank_set_value(bank->regs, offset, value);
+
+ /* Configure GPIO direction as output. */
+ mxc_gpio_bank_direction(bank->regs, offset, MXC_GPIO_DIRECTION_OUT);
+
+ return 0;
+}
+
+/* read GPIO IN value of pin 'gpio' */
+static int mxc_gpio_get_value(struct udevice *dev, unsigned offset)
+{
+ struct mxc_bank_info *bank = dev_get_priv(dev);
+ int ret;
+
+ ret = check_requested(dev, offset, __func__);
+ if (ret)
+ return ret;
+
+ return mxc_gpio_bank_get_value(bank->regs, offset);
+}
+
+/* write GPIO OUT value to pin 'gpio' */
+static int mxc_gpio_set_value(struct udevice *dev, unsigned offset,
+ int value)
+{
+ struct mxc_bank_info *bank = dev_get_priv(dev);
+ int ret;
+
+ ret = check_requested(dev, offset, __func__);
+ if (ret)
+ return ret;
+
+ mxc_gpio_bank_set_value(bank->regs, offset, value);
+
+ return 0;
+}
+
+static int mxc_gpio_get_state(struct udevice *dev, unsigned int offset,
+ char *buf, int bufsize)
+{
+ struct gpio_dev_priv *uc_priv = dev->uclass_priv;
+ struct mxc_bank_info *bank = dev_get_priv(dev);
+ const char *label;
+ bool is_output;
+ int size;
+
+ label = bank->label[offset];
+ is_output = mxc_gpio_is_output(bank->regs, offset);
+ size = snprintf(buf, bufsize, "%s%d: ",
+ uc_priv->bank_name ? uc_priv->bank_name : "", offset);
+ buf += size;
+ bufsize -= size;
+ snprintf(buf, bufsize, "%s: %d [%c]%s%s",
+ is_output ? "out" : " in",
+ is_output ?
+ mxc_gpio_bank_get_output_value(bank->regs, offset) :
+ mxc_gpio_bank_get_value(bank->regs, offset),
+ *label ? 'x' : ' ',
+ *label ? " " : "",
+ label);
+
+ return 0;
+}
+
+static int mxc_gpio_request(struct udevice *dev, unsigned offset,
+ const char *label)
+{
+ struct mxc_bank_info *bank = dev_get_priv(dev);
+
+ if (gpio_is_requested(bank, offset))
+ return -EBUSY;
+
+ strncpy(bank->label[offset], label, GPIO_NAME_SIZE);
+ bank->label[offset][GPIO_NAME_SIZE - 1] = '\0';
+
+ return 0;
+}
+
+static int mxc_gpio_free(struct udevice *dev, unsigned offset)
+{
+ struct mxc_bank_info *bank = dev_get_priv(dev);
+ int ret;
+
+ ret = check_requested(dev, offset, __func__);
+ if (ret)
+ return ret;
+ bank->label[offset][0] = '\0';
+
+ return 0;
+}
+
+static int mxc_gpio_get_function(struct udevice *dev, unsigned offset)
+{
+ struct mxc_bank_info *bank = dev_get_priv(dev);
+
+ if (!gpio_is_requested(bank, offset))
+ return GPIOF_UNUSED;
+
+ /* GPIOF_FUNC is not implemented yet */
+ if (mxc_gpio_is_output(bank->regs, offset))
+ return GPIOF_OUTPUT;
+ else
+ return GPIOF_INPUT;
+}
+
+static const struct dm_gpio_ops gpio_mxc_ops = {
+ .request = mxc_gpio_request,
+ .free = mxc_gpio_free,
+ .direction_input = mxc_gpio_direction_input,
+ .direction_output = mxc_gpio_direction_output,
+ .get_value = mxc_gpio_get_value,
+ .set_value = mxc_gpio_set_value,
+ .get_function = mxc_gpio_get_function,
+ .get_state = mxc_gpio_get_state,
+};
+
+static const struct mxc_gpio_plat mxc_plat[] = {
+ { (struct gpio_regs *)GPIO1_BASE_ADDR },
+ { (struct gpio_regs *)GPIO2_BASE_ADDR },
+ { (struct gpio_regs *)GPIO3_BASE_ADDR },
+#if defined(CONFIG_MX25) || defined(CONFIG_MX27) || defined(CONFIG_MX51) || \
+ defined(CONFIG_MX53) || defined(CONFIG_MX6)
+ { (struct gpio_regs *)GPIO4_BASE_ADDR },
+#endif
+#if defined(CONFIG_MX27) || defined(CONFIG_MX53) || defined(CONFIG_MX6)
+ { (struct gpio_regs *)GPIO5_BASE_ADDR },
+ { (struct gpio_regs *)GPIO6_BASE_ADDR },
+#endif
+#if defined(CONFIG_MX53) || defined(CONFIG_MX6)
+ { (struct gpio_regs *)GPIO7_BASE_ADDR },
+#endif
+};
+
+static int mxc_gpio_probe(struct udevice *dev)
+{
+ struct mxc_bank_info *bank = dev_get_priv(dev);
+ struct mxc_gpio_plat *plat = dev_get_platdata(dev);
+ struct gpio_dev_priv *uc_priv = dev->uclass_priv;
+ int banknum;
+ char name[18], *str;
+
+ banknum = plat - mxc_plat;
+ sprintf(name, "GPIO%d_", banknum + 1);
+ str = strdup(name);
+ if (!str)
+ return -ENOMEM;
+ uc_priv->bank_name = str;
+ uc_priv->gpio_count = GPIO_PER_BANK;
+ bank->regs = plat->regs;
+
+ return 0;
+}
+
+U_BOOT_DRIVER(gpio_mxc) = {
+ .name = "gpio_mxc",
+ .id = UCLASS_GPIO,
+ .ops = &gpio_mxc_ops,
+ .probe = mxc_gpio_probe,
+ .priv_auto_alloc_size = sizeof(struct mxc_bank_info),
+};
+
+U_BOOT_DEVICES(mxc_gpios) = {
+ { "gpio_mxc", &mxc_plat[0] },
+ { "gpio_mxc", &mxc_plat[1] },
+ { "gpio_mxc", &mxc_plat[2] },
+#if defined(CONFIG_MX25) || defined(CONFIG_MX27) || defined(CONFIG_MX51) || \
+ defined(CONFIG_MX53) || defined(CONFIG_MX6)
+ { "gpio_mxc", &mxc_plat[3] },
+#endif
+#if defined(CONFIG_MX27) || defined(CONFIG_MX53) || defined(CONFIG_MX6)
+ { "gpio_mxc", &mxc_plat[4] },
+ { "gpio_mxc", &mxc_plat[5] },
+#endif
+#if defined(CONFIG_MX53) || defined(CONFIG_MX6)
+ { "gpio_mxc", &mxc_plat[6] },
+#endif
+};
+#endif
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related [flat|nested] 25+ messages in thread* [U-Boot] [PATCH v2 09/11] dm: imx: gpio: Support driver model in MXC gpio driver
2014-09-17 3:51 ` [U-Boot] [PATCH v2 09/11] dm: imx: gpio: Support driver model in MXC gpio driver Simon Glass
@ 2014-09-17 13:00 ` Igor Grinberg
2014-09-17 14:03 ` Simon Glass
0 siblings, 1 reply; 25+ messages in thread
From: Igor Grinberg @ 2014-09-17 13:00 UTC (permalink / raw)
To: u-boot
On 09/17/14 06:51, Simon Glass wrote:
> Add driver model support with this driver. In this case the platform data
> is in the driver. It would be better to put this into an SOC-specific file,
> but this is best attempted when more boards are moved over to use driver
> model.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v2:
> - Change 'reserved' to 'requested'
> - Add an internal function to check if a GPIO is requested
> - Tidy up confusing code that creates names for gpio_request()
>
> drivers/gpio/mxc_gpio.c | 302 +++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 301 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/mxc_gpio.c b/drivers/gpio/mxc_gpio.c
> index 6a572d5..9435d2f 100644
> --- a/drivers/gpio/mxc_gpio.c
> +++ b/drivers/gpio/mxc_gpio.c
[...]
> +static int mxc_gpio_get_state(struct udevice *dev, unsigned int offset,
> + char *buf, int bufsize)
> +{
> + struct gpio_dev_priv *uc_priv = dev->uclass_priv;
> + struct mxc_bank_info *bank = dev_get_priv(dev);
> + const char *label;
> + bool is_output;
> + int size;
> +
> + label = bank->label[offset];
> + is_output = mxc_gpio_is_output(bank->regs, offset);
> + size = snprintf(buf, bufsize, "%s%d: ",
> + uc_priv->bank_name ? uc_priv->bank_name : "", offset);
> + buf += size;
> + bufsize -= size;
> + snprintf(buf, bufsize, "%s: %d [%c]%s%s",
> + is_output ? "out" : " in",
> + is_output ?
> + mxc_gpio_bank_get_output_value(bank->regs, offset) :
> + mxc_gpio_bank_get_value(bank->regs, offset),
> + *label ? 'x' : ' ',
> + *label ? " " : "",
This is a check if the gpio is requested, right?
I think the new function gpio_is_requested() can be of hand here
instead of open coding this.
> + label);
> +
> + return 0;
> +}
> +
[...]
> +static int mxc_gpio_free(struct udevice *dev, unsigned offset)
> +{
> + struct mxc_bank_info *bank = dev_get_priv(dev);
> + int ret;
> +
> + ret = check_requested(dev, offset, __func__);
> + if (ret)
> + return ret;
In case of not requested gpio,
should we really print the error message and return -EPERM?
Or may be adopt free() behavior and just return silently?
Linux gpiolib gpio_free() uses WARN_ON(extra_checks) for
"not requested" cases, so it shouts only in DEBUG cases.
> + bank->label[offset][0] = '\0';
> +
> + return 0;
> +}
> +
[...]
--
Regards,
Igor.
^ permalink raw reply [flat|nested] 25+ messages in thread* [U-Boot] [PATCH v2 09/11] dm: imx: gpio: Support driver model in MXC gpio driver
2014-09-17 13:00 ` Igor Grinberg
@ 2014-09-17 14:03 ` Simon Glass
2014-09-17 14:34 ` Igor Grinberg
0 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2014-09-17 14:03 UTC (permalink / raw)
To: u-boot
Hi Igor,
On 17 September 2014 07:00, Igor Grinberg <grinberg@compulab.co.il> wrote:
> On 09/17/14 06:51, Simon Glass wrote:
> > Add driver model support with this driver. In this case the platform data
> > is in the driver. It would be better to put this into an SOC-specific
> file,
> > but this is best attempted when more boards are moved over to use driver
> > model.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> > Changes in v2:
> > - Change 'reserved' to 'requested'
> > - Add an internal function to check if a GPIO is requested
> > - Tidy up confusing code that creates names for gpio_request()
> >
> > drivers/gpio/mxc_gpio.c | 302
> +++++++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 301 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpio/mxc_gpio.c b/drivers/gpio/mxc_gpio.c
> > index 6a572d5..9435d2f 100644
> > --- a/drivers/gpio/mxc_gpio.c
> > +++ b/drivers/gpio/mxc_gpio.c
>
> [...]
>
> > +static int mxc_gpio_get_state(struct udevice *dev, unsigned int offset,
> > + char *buf, int bufsize)
> > +{
> > + struct gpio_dev_priv *uc_priv = dev->uclass_priv;
> > + struct mxc_bank_info *bank = dev_get_priv(dev);
> > + const char *label;
> > + bool is_output;
> > + int size;
> > +
> > + label = bank->label[offset];
> > + is_output = mxc_gpio_is_output(bank->regs, offset);
> > + size = snprintf(buf, bufsize, "%s%d: ",
> > + uc_priv->bank_name ? uc_priv->bank_name : "",
> offset);
> > + buf += size;
> > + bufsize -= size;
> > + snprintf(buf, bufsize, "%s: %d [%c]%s%s",
> > + is_output ? "out" : " in",
> > + is_output ?
> > + mxc_gpio_bank_get_output_value(bank->regs, offset)
> :
> > + mxc_gpio_bank_get_value(bank->regs, offset),
> > + *label ? 'x' : ' ',
> > + *label ? " " : "",
>
> This is a check if the gpio is requested, right?
> I think the new function gpio_is_requested() can be of hand here
> instead of open coding this.
>
OK
>
> > + label);
> > +
> > + return 0;
> > +}
> > +
>
> [...]
>
> > +static int mxc_gpio_free(struct udevice *dev, unsigned offset)
> > +{
> > + struct mxc_bank_info *bank = dev_get_priv(dev);
> > + int ret;
> > +
> > + ret = check_requested(dev, offset, __func__);
> > + if (ret)
> > + return ret;
>
> In case of not requested gpio,
> should we really print the error message and return -EPERM?
> Or may be adopt free() behavior and just return silently?
> Linux gpiolib gpio_free() uses WARN_ON(extra_checks) for
> "not requested" cases, so it shouts only in DEBUG cases.
>
I'm not sure - I intend to push this up to the DM layer at some point. I
feel that keeping track of GPIO requested/free should be a common feature
of the uclass rather than implemented in each driver. But until we have
enough drivers to make it clear that this is acceptable, I'm leaving it as
it is.
So for at least now I think this is correct. We might consider bringing in
some sort of WARN system to U-Boot. The caller can ignore the return value
anyway.
Regards,
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread* [U-Boot] [PATCH v2 09/11] dm: imx: gpio: Support driver model in MXC gpio driver
2014-09-17 14:03 ` Simon Glass
@ 2014-09-17 14:34 ` Igor Grinberg
0 siblings, 0 replies; 25+ messages in thread
From: Igor Grinberg @ 2014-09-17 14:34 UTC (permalink / raw)
To: u-boot
On 09/17/14 17:03, Simon Glass wrote:
> Hi Igor,
>
> On 17 September 2014 07:00, Igor Grinberg <grinberg at compulab.co.il <mailto:grinberg@compulab.co.il>> wrote:
>
> On 09/17/14 06:51, Simon Glass wrote:
> > Add driver model support with this driver. In this case the platform data
> > is in the driver. It would be better to put this into an SOC-specific file,
> > but this is best attempted when more boards are moved over to use driver
> > model.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org <mailto:sjg@chromium.org>>
> > ---
> >
> > Changes in v2:
> > - Change 'reserved' to 'requested'
> > - Add an internal function to check if a GPIO is requested
> > - Tidy up confusing code that creates names for gpio_request()
> >
> > drivers/gpio/mxc_gpio.c | 302 +++++++++++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 301 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpio/mxc_gpio.c b/drivers/gpio/mxc_gpio.c
> > index 6a572d5..9435d2f 100644
> > --- a/drivers/gpio/mxc_gpio.c
> > +++ b/drivers/gpio/mxc_gpio.c
>
> [...]
[...]
> > +static int mxc_gpio_free(struct udevice *dev, unsigned offset)
> > +{
> > + struct mxc_bank_info *bank = dev_get_priv(dev);
> > + int ret;
> > +
> > + ret = check_requested(dev, offset, __func__);
> > + if (ret)
> > + return ret;
>
> In case of not requested gpio,
> should we really print the error message and return -EPERM?
> Or may be adopt free() behavior and just return silently?
> Linux gpiolib gpio_free() uses WARN_ON(extra_checks) for
> "not requested" cases, so it shouts only in DEBUG cases.
>
>
> I'm not sure - I intend to push this up to the DM layer at some point.
> I feel that keeping track of GPIO requested/free should be a common
> feature of the uclass rather than implemented in each driver.
Agreed. I've got the same feeling while looking at this.
> But until we have enough drivers to make it clear that this is acceptable,
> I'm leaving it as it is.
>
> So for at least now I think this is correct. We might consider bringing
> in some sort of WARN system to U-Boot. The caller can ignore the return
> value anyway.
Ok.
--
Regards,
Igor.
^ permalink raw reply [flat|nested] 25+ messages in thread
* [U-Boot] [PATCH v2 10/11] dm: imx: serial: Support driver model in the MXC serial driver
2014-09-17 3:51 [U-Boot] [PATCH v2 0/11] dm: imx: Add driver model support for GPIO and serial on cm_fx6 Simon Glass
` (8 preceding siblings ...)
2014-09-17 3:51 ` [U-Boot] [PATCH v2 09/11] dm: imx: gpio: Support driver model in MXC gpio driver Simon Glass
@ 2014-09-17 3:51 ` Simon Glass
2014-09-17 3:51 ` [U-Boot] [PATCH v2 11/11] dm: imx: Move cm_fx6 to use driver model for serial and GPIO Simon Glass
10 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2014-09-17 3:51 UTC (permalink / raw)
To: u-boot
Add driver model support with this driver. Boards which use this driver
should define platform data in their board files.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2: None
drivers/serial/serial_mxc.c | 170 +++++++++++++++++++++++++++++++++++++-------
include/serial_mxc.h | 14 ++++
2 files changed, 159 insertions(+), 25 deletions(-)
create mode 100644 include/serial_mxc.h
diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c
index 313d560..9ce24f9 100644
--- a/drivers/serial/serial_mxc.c
+++ b/drivers/serial/serial_mxc.c
@@ -5,37 +5,15 @@
*/
#include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <serial_mxc.h>
#include <watchdog.h>
#include <asm/arch/imx-regs.h>
#include <asm/arch/clock.h>
#include <serial.h>
#include <linux/compiler.h>
-#define __REG(x) (*((volatile u32 *)(x)))
-
-#ifndef CONFIG_MXC_UART_BASE
-#error "define CONFIG_MXC_UART_BASE to use the MXC UART driver"
-#endif
-
-#define UART_PHYS CONFIG_MXC_UART_BASE
-
-/* Register definitions */
-#define URXD 0x0 /* Receiver Register */
-#define UTXD 0x40 /* Transmitter Register */
-#define UCR1 0x80 /* Control Register 1 */
-#define UCR2 0x84 /* Control Register 2 */
-#define UCR3 0x88 /* Control Register 3 */
-#define UCR4 0x8c /* Control Register 4 */
-#define UFCR 0x90 /* FIFO Control Register */
-#define USR1 0x94 /* Status Register 1 */
-#define USR2 0x98 /* Status Register 2 */
-#define UESC 0x9c /* Escape Character Register */
-#define UTIM 0xa0 /* Escape Timer Register */
-#define UBIR 0xa4 /* BRM Incremental Register */
-#define UBMR 0xa8 /* BRM Modulator Register */
-#define UBRC 0xac /* Baud Rate Count Register */
-#define UTS 0xb4 /* UART Test Register (mx31) */
-
/* UART Control Register Bit Fields.*/
#define URXD_CHARRDY (1<<15)
#define URXD_ERR (1<<14)
@@ -128,6 +106,33 @@
#define UTS_RXFULL (1<<3) /* RxFIFO full */
#define UTS_SOFTRST (1<<0) /* Software reset */
+#ifndef CONFIG_DM_SERIAL
+
+#ifndef CONFIG_MXC_UART_BASE
+#error "define CONFIG_MXC_UART_BASE to use the MXC UART driver"
+#endif
+
+#define UART_PHYS CONFIG_MXC_UART_BASE
+
+#define __REG(x) (*((volatile u32 *)(x)))
+
+/* Register definitions */
+#define URXD 0x0 /* Receiver Register */
+#define UTXD 0x40 /* Transmitter Register */
+#define UCR1 0x80 /* Control Register 1 */
+#define UCR2 0x84 /* Control Register 2 */
+#define UCR3 0x88 /* Control Register 3 */
+#define UCR4 0x8c /* Control Register 4 */
+#define UFCR 0x90 /* FIFO Control Register */
+#define USR1 0x94 /* Status Register 1 */
+#define USR2 0x98 /* Status Register 2 */
+#define UESC 0x9c /* Escape Character Register */
+#define UTIM 0xa0 /* Escape Timer Register */
+#define UBIR 0xa4 /* BRM Incremental Register */
+#define UBMR 0xa8 /* BRM Modulator Register */
+#define UBRC 0xac /* Baud Rate Count Register */
+#define UTS 0xb4 /* UART Test Register (mx31) */
+
DECLARE_GLOBAL_DATA_PTR;
static void mxc_serial_setbrg(void)
@@ -222,3 +227,118 @@ __weak struct serial_device *default_serial_console(void)
{
return &mxc_serial_drv;
}
+#endif
+
+#ifdef CONFIG_DM_SERIAL
+
+struct mxc_uart {
+ u32 rxd;
+ u32 spare0[15];
+
+ u32 txd;
+ u32 spare1[15];
+
+ u32 cr1;
+ u32 cr2;
+ u32 cr3;
+ u32 cr4;
+
+ u32 fcr;
+ u32 sr1;
+ u32 sr2;
+ u32 esc;
+
+ u32 tim;
+ u32 bir;
+ u32 bmr;
+ u32 brc;
+
+ u32 onems;
+ u32 ts;
+};
+
+int mxc_serial_setbrg(struct udevice *dev, int baudrate)
+{
+ struct mxc_serial_platdata *plat = dev->platdata;
+ struct mxc_uart *const uart = plat->reg;
+ u32 clk = imx_get_uartclk();
+
+ writel(4 << 7, &uart->fcr); /* divide input clock by 2 */
+ writel(0xf, &uart->bir);
+ writel(clk / (2 * baudrate), &uart->bmr);
+
+ writel(UCR2_WS | UCR2_IRTS | UCR2_RXEN | UCR2_TXEN | UCR2_SRST,
+ &uart->cr2);
+ writel(UCR1_UARTEN, &uart->cr1);
+
+ return 0;
+}
+
+static int mxc_serial_probe(struct udevice *dev)
+{
+ struct mxc_serial_platdata *plat = dev->platdata;
+ struct mxc_uart *const uart = plat->reg;
+
+ writel(0, &uart->cr1);
+ writel(0, &uart->cr2);
+ while (!(readl(&uart->cr2) & UCR2_SRST));
+ writel(0x704 | UCR3_ADNIMP, &uart->cr3);
+ writel(0x8000, &uart->cr4);
+ writel(0x2b, &uart->esc);
+ writel(0, &uart->tim);
+ writel(0, &uart->ts);
+
+ return 0;
+}
+
+static int mxc_serial_getc(struct udevice *dev)
+{
+ struct mxc_serial_platdata *plat = dev->platdata;
+ struct mxc_uart *const uart = plat->reg;
+
+ if (readl(&uart->ts) & UTS_RXEMPTY)
+ return -EAGAIN;
+
+ return readl(&uart->rxd) & URXD_RX_DATA;
+}
+
+static int mxc_serial_putc(struct udevice *dev, const char ch)
+{
+ struct mxc_serial_platdata *plat = dev->platdata;
+ struct mxc_uart *const uart = plat->reg;
+
+ if (!(readl(&uart->ts) & UTS_TXEMPTY))
+ return -EAGAIN;
+
+ writel(ch, &uart->txd);
+
+ return 0;
+}
+
+static int mxc_serial_pending(struct udevice *dev, bool input)
+{
+ struct mxc_serial_platdata *plat = dev->platdata;
+ struct mxc_uart *const uart = plat->reg;
+ uint32_t sr2 = readl(&uart->sr2);
+
+ if (input)
+ return sr2 & USR2_RDR ? 1 : 0;
+ else
+ return sr2 & USR2_TXDC ? 0 : 1;
+}
+
+static const struct dm_serial_ops mxc_serial_ops = {
+ .putc = mxc_serial_putc,
+ .pending = mxc_serial_pending,
+ .getc = mxc_serial_getc,
+ .setbrg = mxc_serial_setbrg,
+};
+
+U_BOOT_DRIVER(serial_mxc) = {
+ .name = "serial_mxc",
+ .id = UCLASS_SERIAL,
+ .probe = mxc_serial_probe,
+ .ops = &mxc_serial_ops,
+ .flags = DM_FLAG_PRE_RELOC,
+};
+#endif
diff --git a/include/serial_mxc.h b/include/serial_mxc.h
new file mode 100644
index 0000000..7d3ace2
--- /dev/null
+++ b/include/serial_mxc.h
@@ -0,0 +1,14 @@
+/*
+ * Copyright (c) 2014 Google, Inc
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#ifndef __serial_mxc_h
+#define __serial_mxc_h
+
+/* Information about a serial port */
+struct mxc_serial_platdata {
+ struct mxc_uart *reg; /* address of registers in physical memory */
+};
+
+#endif
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related [flat|nested] 25+ messages in thread* [U-Boot] [PATCH v2 11/11] dm: imx: Move cm_fx6 to use driver model for serial and GPIO
2014-09-17 3:51 [U-Boot] [PATCH v2 0/11] dm: imx: Add driver model support for GPIO and serial on cm_fx6 Simon Glass
` (9 preceding siblings ...)
2014-09-17 3:51 ` [U-Boot] [PATCH v2 10/11] dm: imx: serial: Support driver model in the MXC serial driver Simon Glass
@ 2014-09-17 3:51 ` Simon Glass
2014-09-17 13:07 ` Igor Grinberg
10 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2014-09-17 3:51 UTC (permalink / raw)
To: u-boot
Now that serial and GPIO are available for iMX.6, move cm_fx6 over as an
example.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Use the correct namespace for the platform data
board/compulab/cm_fx6/cm_fx6.c | 10 ++++++++++
include/configs/cm_fx6.h | 11 +++++++++++
2 files changed, 21 insertions(+)
diff --git a/board/compulab/cm_fx6/cm_fx6.c b/board/compulab/cm_fx6/cm_fx6.c
index 10e31b6..d79cd5f 100644
--- a/board/compulab/cm_fx6/cm_fx6.c
+++ b/board/compulab/cm_fx6/cm_fx6.c
@@ -9,11 +9,13 @@
*/
#include <common.h>
+#include <dm.h>
#include <fsl_esdhc.h>
#include <miiphy.h>
#include <netdev.h>
#include <fdt_support.h>
#include <sata.h>
+#include <serial_mxc.h>
#include <asm/arch/crm_regs.h>
#include <asm/arch/sys_proto.h>
#include <asm/arch/iomux.h>
@@ -509,3 +511,11 @@ u32 get_board_rev(void)
return cl_eeprom_get_board_rev();
}
+static struct mxc_serial_platdata cm_fx6_mxc_serial_plat = {
+ .reg = (struct mxc_uart *)UART4_BASE,
+};
+
+U_BOOT_DEVICE(cm_fx6_serial) = {
+ .name = "serial_mxc",
+ .platdata = &cm_fx6_mxc_serial_plat,
+};
diff --git a/include/configs/cm_fx6.h b/include/configs/cm_fx6.h
index 10d02b4..1f55150 100644
--- a/include/configs/cm_fx6.h
+++ b/include/configs/cm_fx6.h
@@ -21,6 +21,17 @@
#define CONFIG_MACH_TYPE 4273
#define CONFIG_SYS_HZ 1000
+#ifndef CONFIG_SPL_BUILD
+#define CONFIG_DM
+#define CONFIG_CMD_DM
+
+#define CONFIG_DM_GPIO
+#define CONFIG_CMD_GPIO
+
+#define CONFIG_DM_SERIAL
+#define CONFIG_SYS_MALLOC_F_LEN (1 << 10)
+#endif
+
/* Display information on boot */
#define CONFIG_DISPLAY_CPUINFO
#define CONFIG_DISPLAY_BOARDINFO
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related [flat|nested] 25+ messages in thread* [U-Boot] [PATCH v2 11/11] dm: imx: Move cm_fx6 to use driver model for serial and GPIO
2014-09-17 3:51 ` [U-Boot] [PATCH v2 11/11] dm: imx: Move cm_fx6 to use driver model for serial and GPIO Simon Glass
@ 2014-09-17 13:07 ` Igor Grinberg
0 siblings, 0 replies; 25+ messages in thread
From: Igor Grinberg @ 2014-09-17 13:07 UTC (permalink / raw)
To: u-boot
On 09/17/14 06:51, Simon Glass wrote:
> Now that serial and GPIO are available for iMX.6, move cm_fx6 over as an
> example.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
Thanks for doing this.
Acked-by: Igor Grinberg <grinberg@compulab.co.il>
> ---
>
> Changes in v2:
> - Use the correct namespace for the platform data
>
> board/compulab/cm_fx6/cm_fx6.c | 10 ++++++++++
> include/configs/cm_fx6.h | 11 +++++++++++
> 2 files changed, 21 insertions(+)
>
> diff --git a/board/compulab/cm_fx6/cm_fx6.c b/board/compulab/cm_fx6/cm_fx6.c
> index 10e31b6..d79cd5f 100644
> --- a/board/compulab/cm_fx6/cm_fx6.c
> +++ b/board/compulab/cm_fx6/cm_fx6.c
> @@ -9,11 +9,13 @@
> */
>
> #include <common.h>
> +#include <dm.h>
> #include <fsl_esdhc.h>
> #include <miiphy.h>
> #include <netdev.h>
> #include <fdt_support.h>
> #include <sata.h>
> +#include <serial_mxc.h>
> #include <asm/arch/crm_regs.h>
> #include <asm/arch/sys_proto.h>
> #include <asm/arch/iomux.h>
> @@ -509,3 +511,11 @@ u32 get_board_rev(void)
> return cl_eeprom_get_board_rev();
> }
>
> +static struct mxc_serial_platdata cm_fx6_mxc_serial_plat = {
> + .reg = (struct mxc_uart *)UART4_BASE,
> +};
> +
> +U_BOOT_DEVICE(cm_fx6_serial) = {
> + .name = "serial_mxc",
> + .platdata = &cm_fx6_mxc_serial_plat,
> +};
> diff --git a/include/configs/cm_fx6.h b/include/configs/cm_fx6.h
> index 10d02b4..1f55150 100644
> --- a/include/configs/cm_fx6.h
> +++ b/include/configs/cm_fx6.h
> @@ -21,6 +21,17 @@
> #define CONFIG_MACH_TYPE 4273
> #define CONFIG_SYS_HZ 1000
>
> +#ifndef CONFIG_SPL_BUILD
> +#define CONFIG_DM
> +#define CONFIG_CMD_DM
> +
> +#define CONFIG_DM_GPIO
> +#define CONFIG_CMD_GPIO
> +
> +#define CONFIG_DM_SERIAL
> +#define CONFIG_SYS_MALLOC_F_LEN (1 << 10)
> +#endif
> +
> /* Display information on boot */
> #define CONFIG_DISPLAY_CPUINFO
> #define CONFIG_DISPLAY_BOARDINFO
>
--
Regards,
Igor.
^ permalink raw reply [flat|nested] 25+ messages in thread