Le 08/05/2024 à 08:51, Jacky Huang a écrit :
From: Jacky Huang <ychuang3@xxxxxxxxxxx> Add common pinctrl and GPIO driver for Nuvoton MA35 series SoC, and add support for ma35d1 pinctrl. Signed-off-by: Jacky Huang <ychuang3@xxxxxxxxxxx> ---
...
+static int ma35_gpiolib_register(struct platform_device *pdev, struct ma35_pinctrl *npctl) +{ + struct ma35_pin_ctrl *ctrl = npctl->ctrl; + struct ma35_pin_bank *bank = ctrl->pin_banks; + int ret; + int i; + + for (i = 0; i < ctrl->nr_banks; i++, bank++) { + if (!bank->valid) { + dev_warn(&pdev->dev, "%pfw: bank is not valid\n", bank->fwnode); + continue; + } + bank->irqtype = 0; + bank->irqinten = 0; + bank->chip.label = bank->name; + bank->chip.of_gpio_n_cells = 2; + bank->chip.parent = &pdev->dev; + bank->chip.request = ma35_gpio_core_to_request; + bank->chip.direction_input = ma35_gpio_core_direction_in; + bank->chip.direction_output = ma35_gpio_core_direction_out; + bank->chip.get = ma35_gpio_core_get; + bank->chip.set = ma35_gpio_core_set; + bank->chip.base = -1; + bank->chip.ngpio = bank->nr_pins; + bank->chip.can_sleep = false; + + if (bank->irq > 0) { + struct gpio_irq_chip *girq; + + girq = &bank->chip.irq; + gpio_irq_chip_set_chip(girq, &ma35_gpio_irqchip); + girq->parent_handler = ma35_irq_demux_intgroup; + girq->num_parents = 1; + + girq->parents = devm_kcalloc(&pdev->dev, girq->num_parents, + sizeof(*girq->parents), GFP_KERNEL); + if (!girq->parents) { + ret = -ENOMEM; + goto fail; + } + + girq->parents[0] = bank->irq; + girq->default_type = IRQ_TYPE_NONE; + girq->handler = handle_bad_irq; + } + + ret = devm_gpiochip_add_data(&pdev->dev, &bank->chip, bank); + if (ret) { + dev_err(&pdev->dev, "failed to register gpio_chip %s, error code: %d\n", + bank->chip.label, ret); + goto fail; + } + } + return 0; + +fail: + while (i--) { + bank--; + if (!bank->valid) + continue; + gpiochip_remove(&bank->chip); + }
I don't think this is correct. This is to undo the devm_gpiochip_add_data(), right?
Because of the devm_, no need to explicityl call gpiochip_remove() (more over, it should have been --i and not i-- in the while)
+ return ret; +}
...
+static int ma35_pinconf_set_drive_strength(struct ma35_pinctrl *npctl, unsigned int pin, + int strength) +{ + unsigned int port, group_num; + void __iomem *base; + int i, ds_val = -1; + u32 regval; + + if (ma35_pinconf_get_power_source(npctl, pin) == MVOLT_1800) { + for (i = 0; i < ARRAY_SIZE(ds_1800mv_tbl); i++) { + if (ds_1800mv_tbl[i] == strength) { + ds_val = i; + break; + } + } + } else { + for (i = 0; i < ARRAY_SIZE(ds_3300mv_tbl); i++) { + if (ds_3300mv_tbl[i] == strength) { + ds_val = i; + continue;
break; ?
+ } + } + } + if (ds_val == -1) + return -EINVAL; + + ma35_gpio_cla_port(pin, &group_num, &port); + base = npctl->ctrl->pin_banks[group_num].reg_base; + + regval = readl(base + MA35_GP_DS_REG(port)); + regval &= ~MA35_GP_DS_MASK(port); + regval |= field_prep(MA35_GP_DS_MASK(port), ds_val); + + writel(regval, base + MA35_GP_DS_REG(port)); + + return 0; +}
...
+static int ma35_pinctrl_probe_dt(struct platform_device *pdev, struct ma35_pinctrl *npctl) +{ + struct fwnode_handle *child; + u32 idx = 0; + int ret; + + device_for_each_child_node(&pdev->dev, child) { + if (fwnode_property_present(child, "gpio-controller")) + continue; + npctl->nfunctions++; + npctl->ngroups += of_get_child_count(to_of_node(child)); + } + + if (!npctl->nfunctions) + return -EINVAL; + + npctl->functions = devm_kcalloc(&pdev->dev, npctl->nfunctions, + sizeof(*npctl->functions), GFP_KERNEL); + if (!npctl->functions) + return -ENOMEM; + + npctl->groups = devm_kcalloc(&pdev->dev, npctl->ngroups, + sizeof(*npctl->groups), GFP_KERNEL); + if (!npctl->groups) + return -ENOMEM; + + device_for_each_child_node(&pdev->dev, child) { + if (fwnode_property_present(child, "gpio-controller")) + continue; + + ret = ma35_pinctrl_parse_functions(to_of_node(child), npctl, idx++); + if (ret) { + dev_err(&pdev->dev, "failed to parse function\n");
Missing fwnode_handle_put(child); ? (or a scoped version of device_for_each_child_node() if it exists)
+ return ret; + } + } + return 0; +
... CJ