Re: [U-Boot] [PATCH V2 2/2] sunxi: gpio: convert bind() to use driver data

Hi Stephen,

On 11 May 2016 at 15:26, Stephen Warren swarren@wwwdotorg.org wrote:

From: Stephen Warren swarren@nvidia.com

Now that the DM core sets driver_data before calling bind(), this driver can make use of driver_data to determine the set of child devices to create, rather than manually re-implementing the matching logic in code.

Cc: Hans de Goede hdegoede@redhat.com Signed-off-by: Stephen Warren swarren@nvidia.com

---

v2: New patch.

drivers/gpio/sunxi_gpio.c | 90 ++++++++++++++++++++++++++--------------------- 1 file changed, 50 insertions(+), 40 deletions(-)

Reviewed-by: Simon Glass sjg@chromium.org

Comment below.

diff --git a/drivers/gpio/sunxi_gpio.c b/drivers/gpio/sunxi_gpio.c index a7cec18d57fb..94abbeb39adc 100644 --- a/drivers/gpio/sunxi_gpio.c +++ b/drivers/gpio/sunxi_gpio.c @@ -258,43 +258,30 @@ static int gpio_sunxi_probe(struct udevice *dev)

    return 0;

}

+struct sunxi_gpio_soc_data {

•   int start;
  

•   int no_banks;
  

+};

/**

• We have a top-level GPIO device with no actual GPIOs. It has a child
• device for each Sunxi bank.

*/ static int gpio_sunxi_bind(struct udevice *parent) {

•   struct sunxi_gpio_soc_data *soc_data =
  

•           (struct sunxi_gpio_soc_data *)dev_get_driver_data(parent);
    struct sunxi_gpio_platdata *plat = parent->platdata;
    struct sunxi_gpio_reg *ctlr;
  

•   int bank, no_banks, ret, start;
  

•   int bank, ret;
  
    /* If this is a child device, there is nothing to do here */
    if (plat)
            return 0;
  
  

•   if (fdt_node_check_compatible(gd->fdt_blob, parent->of_offset,
  

•                           "allwinner,sun6i-a31-r-pinctrl") == 0) {
  

•           start = 'L' - 'A';
  

•           no_banks = 2; /* L & M */
  

•   } else if (fdt_node_check_compatible(gd->fdt_blob, parent->of_offset,
  

•                           "allwinner,sun8i-a23-r-pinctrl") == 0 ||
  

•              fdt_node_check_compatible(gd->fdt_blob, parent->of_offset,
  

•                           "allwinner,sun8i-a83t-r-pinctrl") == 0 ||
  

•              fdt_node_check_compatible(gd->fdt_blob, parent->of_offset,
  

•                           "allwinner,sun8i-h3-r-pinctrl") == 0) {
  

•           start = 'L' - 'A';
  

•           no_banks = 1; /* L only */
  

•   } else if (fdt_node_check_compatible(gd->fdt_blob, parent->of_offset,
  

•                           "allwinner,sun9i-a80-r-pinctrl") == 0) {
  

•           start = 'L' - 'A';
  

•           no_banks = 3; /* L, M & N */
  

•   } else {
  

•           start = 0;
  

•           no_banks = SUNXI_GPIO_BANKS;
  

•   }
  

•   ctlr = (struct sunxi_gpio_reg *)dev_get_addr(parent);
  

•   for (bank = 0; bank < no_banks; bank++) {
  

•   for (bank = 0; bank < soc_data->no_banks; bank++) {
            struct sunxi_gpio_platdata *plat;
            struct udevice *dev;
  
  

@@ -302,7 +289,7 @@ static int gpio_sunxi_bind(struct udevice *parent) if (!plat) return -ENOMEM; plat->regs = &ctlr->gpio_bank[bank];

•           plat->bank_name = gpio_bank_name(start + bank);
  

•           plat->bank_name = gpio_bank_name(soc_data->start + bank);
            plat->gpio_count = SUNXI_GPIOS_PER_BANK;
  
            ret = device_bind(parent, parent->driver,
  

@@ -315,23 +302,46 @@ static int gpio_sunxi_bind(struct udevice *parent) return 0; }

+static const struct sunxi_gpio_soc_data soc_data_a_all = {

•   .start = 0,
  

•   .no_banks = SUNXI_GPIO_BANKS,
  

+};

+static const struct sunxi_gpio_soc_data soc_data_l_1 = {

•   .start = 'L' - 'A',
  

•   .no_banks = 1,
  

+};

+static const struct sunxi_gpio_soc_data soc_data_l_2 = {

•   .start = 'L' - 'A',
  

•   .no_banks = 2,
  

+};

+static const struct sunxi_gpio_soc_data soc_data_l_3 = {

•   .start = 'L' - 'A',
  

•   .no_banks = 3,
  

+};

+#define ID(_compat_, _soc_data_) \

•   { .compatible = _compat_, .data = (ulong)&soc_data_##_soc_data_ }
  

static const struct udevice_id sunxi_gpio_ids[] = {

•   { .compatible = "allwinner,sun4i-a10-pinctrl" },
  

•   { .compatible = "allwinner,sun5i-a10s-pinctrl" },
  

•   { .compatible = "allwinner,sun5i-a13-pinctrl" },
  

•   { .compatible = "allwinner,sun6i-a31-pinctrl" },
  

•   { .compatible = "allwinner,sun6i-a31s-pinctrl" },
  

•   { .compatible = "allwinner,sun7i-a20-pinctrl" },
  

•   { .compatible = "allwinner,sun8i-a23-pinctrl" },
  

•   { .compatible = "allwinner,sun8i-a33-pinctrl" },
  

•   { .compatible = "allwinner,sun8i-a83t-pinctrl", },
  

•   { .compatible = "allwinner,sun8i-h3-pinctrl" },
  

•   { .compatible = "allwinner,sun9i-a80-pinctrl" },
  

•   { .compatible = "allwinner,sun6i-a31-r-pinctrl" },
  

•   { .compatible = "allwinner,sun8i-a23-r-pinctrl" },
  

•   { .compatible = "allwinner,sun8i-a83t-r-pinctrl" },
  

•   { .compatible = "allwinner,sun8i-h3-r-pinctrl", },
  

•   { .compatible = "allwinner,sun9i-a80-r-pinctrl", },
  

•   ID("allwinner,sun4i-a10-pinctrl",       a_all),
  

•   ID("allwinner,sun5i-a10s-pinctrl",      a_all),
  

•   ID("allwinner,sun5i-a13-pinctrl",       a_all),
  

•   ID("allwinner,sun6i-a31-pinctrl",       a_all),
  

•   ID("allwinner,sun6i-a31s-pinctrl",      a_all),
  

•   ID("allwinner,sun7i-a20-pinctrl",       a_all),
  

•   ID("allwinner,sun8i-a23-pinctrl",       a_all),
  

•   ID("allwinner,sun8i-a33-pinctrl",       a_all),
  

•   ID("allwinner,sun8i-a83t-pinctrl",      a_all),
  

•   ID("allwinner,sun8i-h3-pinctrl",        a_all),
  

•   ID("allwinner,sun9i-a80-pinctrl",       a_all),
  

•   ID("allwinner,sun6i-a31-r-pinctrl",     l_2),
  

•   ID("allwinner,sun8i-a23-r-pinctrl",     l_1),
  

•   ID("allwinner,sun8i-a83t-r-pinctrl",    l_1),
  

•   ID("allwinner,sun8i-h3-r-pinctrl",      l_1),
  

•   ID("allwinner,sun9i-a80-r-pinctrl",     l_3),
  

I don't think the #define adds a lot of value - consider removing it an writing things out in full?

    { }

};

-- 2.8.2

Regards, Simon