Messages in this thread | | | Date | Wed, 22 Mar 2023 10:10:23 +0100 | Subject | Re: [PATCH INTERNAL v1 3/3] regulator: tps6594-regulator: Add driver for TI TPS6594 regulators | From | Julien Panis <> |
| |
On 2/24/23 14:31, Esteban Blanc wrote: > 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. > > Signed-off-by: Jerome Neanne <jneanne@baylibre.com> > ---
(...)
> +static int tps6594_regulator_probe(struct platform_device *pdev) > +{ > + struct tps6594 *tps = dev_get_drvdata(pdev->dev.parent); > + struct regulator_dev *rdev; > + struct regulator_config config = {}; > + u8 buck_configured[BUCK_NB] = { 0 }; > + u8 buck_multi[MULTI_PHASE_NB] = { 0 }; > + int i; > + int error; > + int irq; > + int ext_reg_irq_nb = 2; > + struct tps6594_regulator_irq_data *irq_data; > + struct tps6594_ext_regulator_irq_data *irq_ext_reg_data; > + struct tps6594_regulator_irq_type *irq_type; > + struct regulator_dev *rdevbucktbl[BUCK_NB]; > + struct regulator_dev *rdevmultitbl[MULTI_PHASE_NB]; > + struct regulator_dev *rdevldotbl[LDO_NB]; > + > + int multi_phase_id; > + int multi_phase_case = 0xFFFF; > + > + config.dev = tps->dev; > + config.driver_data = tps; > + config.regmap = tps->regmap; > + > + /* > + * Switch case defines different possible multi phase config > + * This is based on dts custom property: multi-phase-id > + * Using compatible or device rev is a too complex alternative > + * Default case is no Multiphase buck. > + * In case of Multiphase configuration, value should be defined for > + * buck_configured to avoid creating bucks for every buck in multiphase > + */ > + > + if (device_property_present(tps->dev, "ti,multi-phase-id")) {
Question @ Mark/Liam: Shouldn't we use the generic 'regulator-coupled-with' property instead of 'ti,multi-phase-id' ? I am in charge of upstreaming dt-bindings and maintainers pointed out the similarity between 'multi-phase' and 'coupled' regulator concepts. Does 'regulator-coupled-with' mean that outputs of buck converters are combined ? If so, this generic property should replace our specific 'ti,multi-phase-id' prop, I guess.
> + device_property_read_u32(tps->dev, "ti,multi-phase-id", &multi_phase_id); > + switch (multi_phase_id) { > + case 12: > + buck_multi[0] = 1; > + buck_configured[0] = 1; > + buck_configured[1] = 1; > + multi_phase_case = TPS6594_BUCK_12; > + break; > + case 34: > + buck_multi[1] = 1; > + buck_configured[2] = 1; > + buck_configured[3] = 1; > + multi_phase_case = TPS6594_BUCK_34; > + break; > + case 123: > + buck_multi[2] = 1; > + buck_configured[0] = 1; > + buck_configured[1] = 1; > + buck_configured[2] = 1; > + multi_phase_case = TPS6594_BUCK_123; > + break; > + case 1234: > + buck_multi[3] = 1; > + buck_configured[0] = 1; > + buck_configured[1] = 1; > + buck_configured[2] = 1; > + buck_configured[3] = 1; > + multi_phase_case = TPS6594_BUCK_1234; > + break; > + } > + } > + > + for (i = 0; i < MULTI_PHASE_NB; i++) { > + if (buck_multi[i] == 0) > + continue; > + > + 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); > + } > + rdevmultitbl[i] = rdev; > + } > +
(...)
| |