| Message ID | 20230512141755.1712358-1-eblanc@baylibre.com |
|---|---|
| Headers | show |
| Series | TI TPS6594 PMIC support (RTC, pinctrl, regulators) | expand |
Fri, May 12, 2023 at 04:17:55PM +0200, Esteban Blanc kirjoitti: > From: Jerome Neanne <jneanne@baylibre.com> > > This patch adds support for TPS6594 regulators (bucks and LDOs). > The output voltages are configurable and are meant to supply power > to the main processor and other components. > Bucks can be used in single or multiphase mode, depending on PMIC > part number. ... > + enum { > + MULTI_BUCK12, > + MULTI_BUCK123, > + MULTI_BUCK1234, > + MULTI_BUCK12_34, > + MULTI_FIRST = MULTI_BUCK12, > + MULTI_LAST = MULTI_BUCK12_34, > + MULTI_NUM = MULTI_LAST - MULTI_FIRST + 1 Missing TABs? > + }; ... > + for (multi = MULTI_FIRST ; multi < MULTI_NUM ; multi++) { Just a remark: spaces before ; are not standard. > + np = of_find_node_by_name(tps->dev->of_node, multiphases[multi]); > + npname = of_node_full_name(np); > + np_pmic_parent = of_get_parent(of_get_parent(np)); > + if (strcmp((of_node_full_name(np_pmic_parent)), tps->dev->of_node->full_name)) > + continue; Isn't there an API to compare OF node name with a given parameter? > + delta = strcmp(npname, multiphases[multi]); > + if (!delta) { > + switch (multi) { > + case MULTI_BUCK12: > + buck_multi[0] = 1; > + buck_configured[0] = 1; > + buck_configured[1] = 1; > + break; > + /* multiphase buck34 is supported only with buck12 */ > + case MULTI_BUCK12_34: > + buck_multi[0] = 1; > + buck_configured[0] = 1; > + buck_configured[1] = 1; > + buck_multi[1] = 1; Might be easier to read if this is grouped with [0] assignment above. > + buck_configured[2] = 1; > + buck_configured[3] = 1; > + break; > + case MULTI_BUCK123: > + buck_multi[2] = 1; > + buck_configured[0] = 1; > + buck_configured[1] = 1; > + buck_configured[2] = 1; > + break; > + case MULTI_BUCK1234: > + buck_multi[3] = 1; > + buck_configured[0] = 1; > + buck_configured[1] = 1; > + buck_configured[2] = 1; > + buck_configured[3] = 1; > + break; > + } > + } > + } ... > + irq_data = devm_kmalloc(tps->dev, > + ARRAY_SIZE(tps6594_bucks_irq_types) * > + REGS_INT_NB * > + sizeof(struct tps6594_regulator_irq_data) + > + ARRAY_SIZE(tps6594_ldos_irq_types) * > + REGS_INT_NB * > + sizeof(struct tps6594_regulator_irq_data), We have respective macros in overflow.h that can be used here. > + GFP_KERNEL); > + if (!irq_data) > + return -ENOMEM; ... > + rdev = devm_regulator_register(&pdev->dev, &multi_regs[i], &config); > + if (IS_ERR(rdev)) { > + dev_err(tps->dev, "failed to register %s regulator\n", > + pdev->name); > + return PTR_ERR(rdev); return dev_err_probe(...); ? > + } ... > + rdev = devm_regulator_register(&pdev->dev, &buck_regs[i], &config); > + if (IS_ERR(rdev)) { > + dev_err(tps->dev, "failed to register %s regulator\n", > + pdev->name); > + return PTR_ERR(rdev); Same question. > + } ... > + rdev = devm_regulator_register(&pdev->dev, &ldo_regs[i], &config); > + if (IS_ERR(rdev)) { > + dev_err(tps->dev, > + "failed to register %s regulator\n", > + pdev->name); > + return PTR_ERR(rdev); Same question. > + } > + irq_ext_reg_data = devm_kmalloc(tps->dev, > + ext_reg_irq_nb * > + sizeof(struct tps6594_ext_regulator_irq_data), > + GFP_KERNEL); devm_kmalloc_array() > + if (!irq_ext_reg_data) > + return -ENOMEM; ... > + error = devm_request_threaded_irq(tps->dev, irq, NULL, > + tps6594_regulator_irq_handler, > + IRQF_ONESHOT, > + irq_type->irq_name, > + &irq_ext_reg_data[i]); > + if (error) { > + dev_err(tps->dev, "failed to request %s IRQ %d: %d\n", > + irq_type->irq_name, irq, error); > + return error; return dev_err_probe(...); ? > + } > + } > + return 0; > +}