[U-Boot] [PATCH 1/4] dm: gpio: vybrid_gpio: Correct driver's use of bind() method
It does not look like this driver needs to use a bind() method. It does not manually create devices with device_bind() nor does it create devices using U_BOOT_DEVICE(). It seems to only use device tree.
Therefore the manual allocation of platform data is not needed and is confusing. Also platform data should be set up by the ofdata_to_platdata() method, not bind().
Update the driver in case others use it as a model in future.
Signed-off-by: Simon Glass sjg@chromium.org Reported-by: Adam Ford aford173@gmail.com ---
drivers/gpio/vybrid_gpio.c | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-)
diff --git a/drivers/gpio/vybrid_gpio.c b/drivers/gpio/vybrid_gpio.c index 89918e48dd..030e8d08a4 100644 --- a/drivers/gpio/vybrid_gpio.c +++ b/drivers/gpio/vybrid_gpio.c @@ -105,32 +105,18 @@ static int vybrid_gpio_probe(struct udevice *dev) return 0; }
-static int vybrid_gpio_bind(struct udevice *dev) +static int vybrid_gpio_odata_to_platdata(struct udevice *dev) { - struct vybrid_gpio_platdata *plat = dev->platdata; + struct vybrid_gpio_platdata *plat = dev_get_platdata(dev); fdt_addr_t base_addr;
- if (plat) - return 0; - base_addr = devfdt_get_addr(dev); if (base_addr == FDT_ADDR_T_NONE) - return -ENODEV; - - /* - * TODO: - * When every board is converted to driver model and DT is - * supported, this can be done by auto-alloc feature, but - * not using calloc to alloc memory for platdata. - */ - plat = calloc(1, sizeof(*plat)); - if (!plat) - return -ENOMEM; + return -EINVAL;
plat->base = base_addr; plat->chip = dev->req_seq; plat->port_name = fdt_get_name(gd->fdt_blob, dev_of_offset(dev), NULL); - dev->platdata = plat;
return 0; } @@ -144,8 +130,9 @@ U_BOOT_DRIVER(gpio_vybrid) = { .name = "gpio_vybrid", .id = UCLASS_GPIO, .ops = &gpio_vybrid_ops, + .of_match = vybrid_gpio_ids, + .ofdata_to_platdata = vybrid_gpio_odata_to_platdata, .probe = vybrid_gpio_probe, .priv_auto_alloc_size = sizeof(struct vybrid_gpios), - .of_match = vybrid_gpio_ids, - .bind = vybrid_gpio_bind, + .platdata_auto_alloc_size = sizeof(struct vybrid_gpio_platdata), };
These three drivers all use U_BOOT_DEVICE rather than device tree to create devices, so have to do manual allocation of platform data. This is not true for new platforms.
Add a more explicit comment so that people do not copy this approach with new boards.
Signed-off-by: Simon Glass sjg@chromium.org Reported-by: Adam Ford aford173@gmail.com ---
drivers/gpio/imx_rgpio2p.c | 5 +++++ drivers/gpio/mxc_gpio.c | 5 +++++ drivers/gpio/omap_gpio.c | 9 +++++---- 3 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/gpio/imx_rgpio2p.c b/drivers/gpio/imx_rgpio2p.c index 5abc88ba54..e60e9d2a01 100644 --- a/drivers/gpio/imx_rgpio2p.c +++ b/drivers/gpio/imx_rgpio2p.c @@ -175,6 +175,11 @@ static int imx_rgpio2p_bind(struct udevice *dev) * When every board is converted to driver model and DT is supported, * this can be done by auto-alloc feature, but not using calloc * to alloc memory for platdata. + * + * For example imx_rgpio2p_plat uses platform data rather than device + * tree. + * + * NOTE: DO NOT COPY this code if you are using device tree. */ plat = calloc(1, sizeof(*plat)); if (!plat) diff --git a/drivers/gpio/mxc_gpio.c b/drivers/gpio/mxc_gpio.c index 0eb6c600f1..0c42bd6cec 100644 --- a/drivers/gpio/mxc_gpio.c +++ b/drivers/gpio/mxc_gpio.c @@ -311,6 +311,11 @@ static int mxc_gpio_bind(struct udevice *dev) * When every board is converted to driver model and DT is supported, * this can be done by auto-alloc feature, but not using calloc * to alloc memory for platdata. + * + * For example mxc_plat below uses platform data rather than device + * tree. + * + * NOTE: DO NOT COPY this code if you are using device tree. */ plat = calloc(1, sizeof(*plat)); if (!plat) diff --git a/drivers/gpio/omap_gpio.c b/drivers/gpio/omap_gpio.c index b423e34ca4..5c55e6929e 100644 --- a/drivers/gpio/omap_gpio.c +++ b/drivers/gpio/omap_gpio.c @@ -299,12 +299,9 @@ static int omap_gpio_probe(struct udevice *dev)
static int omap_gpio_bind(struct udevice *dev) { - struct omap_gpio_platdata *plat = dev->platdata; + struct omap_gpio_platdata *plat = dev_get_platdata(dev); fdt_addr_t base_addr;
- if (plat) - return 0; - base_addr = devfdt_get_addr(dev); if (base_addr == FDT_ADDR_T_NONE) return -ENODEV; @@ -314,6 +311,10 @@ static int omap_gpio_bind(struct udevice *dev) * When every board is converted to driver model and DT is * supported, this can be done by auto-alloc feature, but * not using calloc to alloc memory for platdata. + * + * For example am33xx_gpio uses platform data rather than device tree. + * + * NOTE: DO NOT COPY this code if you are using device tree. */ plat = calloc(1, sizeof(*plat)); if (!plat)
These three drivers all use U_BOOT_DEVICE rather than device tree to create devices, so have to do manual allocation of platform data. This is not true for new platforms.
Add a more explicit comment so that people do not copy this approach with new boards.
Signed-off-by: Simon Glass sjg@chromium.org Reported-by: Adam Ford aford173@gmail.com ---
drivers/gpio/imx_rgpio2p.c | 5 +++++ drivers/gpio/mxc_gpio.c | 5 +++++ drivers/gpio/omap_gpio.c | 9 +++++---- 3 files changed, 15 insertions(+), 4 deletions(-)
Applied to u-boot-dm thanks!
In U-Boot -ENODEV means that there is no device. When there is a problem with the device, drivers should return an error like -ENXIO or -EREMOTEIO. When the device tree properties cannot be read correct , they should return -EINVAL.
Update various GPIO drivers to follow this rule, to help with consistency for future driver writers.
Signed-off-by: Simon Glass sjg@chromium.org Reported-by: Adam Ford aford173@gmail.com ---
drivers/gpio/adi_gpio2.c | 2 +- drivers/gpio/atmel_pio4.c | 12 ++++++------ drivers/gpio/imx_rgpio2p.c | 2 +- drivers/gpio/mxc_gpio.c | 2 +- drivers/gpio/omap_gpio.c | 2 +- drivers/gpio/tegra186_gpio.c | 2 +- drivers/i2c/imx_lpi2c.c | 2 +- drivers/i2c/mxc_i2c.c | 12 ++++++------ drivers/i2c/tegra186_bpmp_i2c.c | 2 +- 9 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/drivers/gpio/adi_gpio2.c b/drivers/gpio/adi_gpio2.c index 4db08a344a..1012f2d8eb 100644 --- a/drivers/gpio/adi_gpio2.c +++ b/drivers/gpio/adi_gpio2.c @@ -138,7 +138,7 @@ int peripheral_request(unsigned short per, const char *label) return 0;
if (!(per & P_DEFINED)) - return -ENODEV; + return -EINVAL;
BUG_ON(ident >= MAX_RESOURCES);
diff --git a/drivers/gpio/atmel_pio4.c b/drivers/gpio/atmel_pio4.c index f3689467f0..30bc4296e3 100644 --- a/drivers/gpio/atmel_pio4.c +++ b/drivers/gpio/atmel_pio4.c @@ -50,11 +50,11 @@ static int atmel_pio4_config_io_func(u32 port, u32 pin, u32 reg, mask;
if (pin >= ATMEL_PIO_NPINS_PER_BANK) - return -ENODEV; + return -EINVAL;
port_base = atmel_pio4_port_base(port); if (!port_base) - return -ENODEV; + return -EINVAL;
mask = 1 << pin; reg = func; @@ -128,11 +128,11 @@ int atmel_pio4_set_pio_output(u32 port, u32 pin, u32 value) u32 reg, mask;
if (pin >= ATMEL_PIO_NPINS_PER_BANK) - return -ENODEV; + return -EINVAL;
port_base = atmel_pio4_port_base(port); if (!port_base) - return -ENODEV; + return -EINVAL;
mask = 0x01 << pin; reg = ATMEL_PIO_CFGR_FUNC_GPIO | ATMEL_PIO_DIR_MASK; @@ -154,11 +154,11 @@ int atmel_pio4_get_pio_input(u32 port, u32 pin) u32 reg, mask;
if (pin >= ATMEL_PIO_NPINS_PER_BANK) - return -ENODEV; + return -EINVAL;
port_base = atmel_pio4_port_base(port); if (!port_base) - return -ENODEV; + return -EINVAL;
mask = 0x01 << pin; reg = ATMEL_PIO_CFGR_FUNC_GPIO; diff --git a/drivers/gpio/imx_rgpio2p.c b/drivers/gpio/imx_rgpio2p.c index e60e9d2a01..7825714e80 100644 --- a/drivers/gpio/imx_rgpio2p.c +++ b/drivers/gpio/imx_rgpio2p.c @@ -168,7 +168,7 @@ static int imx_rgpio2p_bind(struct udevice *dev)
addr = devfdt_get_addr_index(dev, 1); if (addr == FDT_ADDR_T_NONE) - return -ENODEV; + return -EINVAL;
/* * TODO: diff --git a/drivers/gpio/mxc_gpio.c b/drivers/gpio/mxc_gpio.c index 0c42bd6cec..c480eba940 100644 --- a/drivers/gpio/mxc_gpio.c +++ b/drivers/gpio/mxc_gpio.c @@ -304,7 +304,7 @@ static int mxc_gpio_bind(struct udevice *dev)
addr = devfdt_get_addr(dev); if (addr == FDT_ADDR_T_NONE) - return -ENODEV; + return -EINVAL;
/* * TODO: diff --git a/drivers/gpio/omap_gpio.c b/drivers/gpio/omap_gpio.c index 5c55e6929e..91a3b4672a 100644 --- a/drivers/gpio/omap_gpio.c +++ b/drivers/gpio/omap_gpio.c @@ -304,7 +304,7 @@ static int omap_gpio_bind(struct udevice *dev)
base_addr = devfdt_get_addr(dev); if (base_addr == FDT_ADDR_T_NONE) - return -ENODEV; + return -EINVAL;
/* * TODO: diff --git a/drivers/gpio/tegra186_gpio.c b/drivers/gpio/tegra186_gpio.c index c5a7e13cce..deb59e8b32 100644 --- a/drivers/gpio/tegra186_gpio.c +++ b/drivers/gpio/tegra186_gpio.c @@ -181,7 +181,7 @@ static int tegra186_gpio_bind(struct udevice *parent)
regs = (uint32_t *)devfdt_get_addr_name(parent, "gpio"); if (regs == (uint32_t *)FDT_ADDR_T_NONE) - return -ENODEV; + return -EINVAL;
for (port = 0; port < ctlr_data->port_count; port++) { struct tegra186_gpio_platdata *plat; diff --git a/drivers/i2c/imx_lpi2c.c b/drivers/i2c/imx_lpi2c.c index aa97196e23..e7ec17fe9e 100644 --- a/drivers/i2c/imx_lpi2c.c +++ b/drivers/i2c/imx_lpi2c.c @@ -412,7 +412,7 @@ static int imx_lpi2c_probe(struct udevice *bus)
addr = devfdt_get_addr(bus); if (addr == FDT_ADDR_T_NONE) - return -ENODEV; + return -EINVAL;
i2c_bus->base = addr; i2c_bus->index = bus->seq; diff --git a/drivers/i2c/mxc_i2c.c b/drivers/i2c/mxc_i2c.c index b7bb76c0ed..abf1da2ae3 100644 --- a/drivers/i2c/mxc_i2c.c +++ b/drivers/i2c/mxc_i2c.c @@ -176,7 +176,7 @@ static int bus_i2c_set_bus_speed(struct mxc_i2c_bus *i2c_bus, int speed) int reg_shift = quirk ? VF610_I2C_REGSHIFT : IMX_I2C_REGSHIFT;
if (!base) - return -ENODEV; + return -EINVAL;
/* Store divider value */ writeb(idx, base + (IFDR << reg_shift)); @@ -239,7 +239,7 @@ static int tx_byte(struct mxc_i2c_bus *i2c_bus, u8 byte) if (ret < 0) return ret; if (ret & I2SR_RX_NO_AK) - return -ENODEV; + return -EREMOTEIO; return 0; }
@@ -418,14 +418,14 @@ static int i2c_init_transfer(struct mxc_i2c_bus *i2c_bus, u8 chip, VF610_I2C_REGSHIFT : IMX_I2C_REGSHIFT;
if (!i2c_bus->base) - return -ENODEV; + return -EINVAL;
for (retry = 0; retry < 3; retry++) { ret = i2c_init_transfer_(i2c_bus, chip, addr, alen); if (ret >= 0) return 0; i2c_imx_stop(i2c_bus); - if (ret == -ENODEV) + if (ret == -EREMOTEIO) return ret;
printf("%s: failed for chip 0x%x retry=%d\n", __func__, chip, @@ -754,7 +754,7 @@ static int mxc_i2c_probe(struct udevice *bus)
addr = devfdt_get_addr(bus); if (addr == FDT_ADDR_T_NONE) - return -ENODEV; + return -EINVAL;
i2c_bus->base = addr; i2c_bus->index = bus->seq; @@ -783,7 +783,7 @@ static int mxc_i2c_probe(struct udevice *bus) !dm_gpio_is_valid(&i2c_bus->scl_gpio) | ret | ret2) { dev_err(dev, "i2c bus %d at %lu, fail to request scl/sda gpio\n", bus->seq, i2c_bus->base); - return -ENODEV; + return -EINVAL; } }
diff --git a/drivers/i2c/tegra186_bpmp_i2c.c b/drivers/i2c/tegra186_bpmp_i2c.c index 931c6de508..b46a09a4e0 100644 --- a/drivers/i2c/tegra186_bpmp_i2c.c +++ b/drivers/i2c/tegra186_bpmp_i2c.c @@ -94,7 +94,7 @@ static int tegra186_bpmp_i2c_probe(struct udevice *dev) "nvidia,bpmp-bus-id", U32_MAX); if (priv->bpmp_bus_id == U32_MAX) { debug("%s: could not parse nvidia,bpmp-bus-id\n", __func__); - return -ENODEV; + return -EINVAL; }
return 0;
In U-Boot -ENODEV means that there is no device. When there is a problem with the device, drivers should return an error like -ENXIO or -EREMOTEIO. When the device tree properties cannot be read correct , they should return -EINVAL.
Update various GPIO drivers to follow this rule, to help with consistency for future driver writers.
Signed-off-by: Simon Glass sjg@chromium.org Reported-by: Adam Ford aford173@gmail.com ---
drivers/gpio/adi_gpio2.c | 2 +- drivers/gpio/atmel_pio4.c | 12 ++++++------ drivers/gpio/imx_rgpio2p.c | 2 +- drivers/gpio/mxc_gpio.c | 2 +- drivers/gpio/omap_gpio.c | 2 +- drivers/gpio/tegra186_gpio.c | 2 +- drivers/i2c/imx_lpi2c.c | 2 +- drivers/i2c/mxc_i2c.c | 12 ++++++------ drivers/i2c/tegra186_bpmp_i2c.c | 2 +- 9 files changed, 19 insertions(+), 19 deletions(-)
Applied to u-boot-dm thanks!
These checks cannot fail since driver model will not call a driver's method if it cannot fully create the driver data structures.
It is confusing to have these checks and others might copy them. Drop this code.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/gpio/pca953x_gpio.c | 11 ----------- 1 file changed, 11 deletions(-)
diff --git a/drivers/gpio/pca953x_gpio.c b/drivers/gpio/pca953x_gpio.c index 4962f25230..791d1d1516 100644 --- a/drivers/gpio/pca953x_gpio.c +++ b/drivers/gpio/pca953x_gpio.c @@ -249,22 +249,11 @@ static int pca953x_probe(struct udevice *dev) { struct pca953x_info *info = dev_get_platdata(dev); struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev); - struct dm_i2c_chip *chip = dev_get_parent_platdata(dev); char name[32], *str; int addr; ulong driver_data; int ret;
- if (!info) { - dev_err(dev, "platdata not ready\n"); - return -ENOMEM; - } - - if (!chip) { - dev_err(dev, "i2c not ready\n"); - return -ENODEV; - } - addr = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev), "reg", 0); if (addr == 0) return -ENODEV;
These checks cannot fail since driver model will not call a driver's method if it cannot fully create the driver data structures.
It is confusing to have these checks and others might copy them. Drop this code.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/gpio/pca953x_gpio.c | 11 ----------- 1 file changed, 11 deletions(-)
Applied to u-boot-dm thanks!
It does not look like this driver needs to use a bind() method. It does not manually create devices with device_bind() nor does it create devices using U_BOOT_DEVICE(). It seems to only use device tree.
Therefore the manual allocation of platform data is not needed and is confusing. Also platform data should be set up by the ofdata_to_platdata() method, not bind().
Update the driver in case others use it as a model in future.
Signed-off-by: Simon Glass sjg@chromium.org Reported-by: Adam Ford aford173@gmail.com ---
drivers/gpio/vybrid_gpio.c | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-)
Applied to u-boot-dm thanks!
participants (2)
-
Simon Glass -
sjg＠google.com