Messages in this thread | | | Date | Tue, 25 Jul 2023 07:21:09 -0700 | Subject | Re: [PATCH 3/3] hwmon: (pmbus/tda38640) Add workaround for bug in SVID mode | From | Guenter Roeck <> |
| |
On 7/25/23 04:40, Naresh Solanki wrote: > From: Patrick Rudolph <patrick.rudolph@9elements.com> > > The TDA38640 supports two operating modes to set the output voltage: > - PMBUS > - SVID > > Due to an undocumented bug the regulator cannot be enabled using BIT7 > of OPERATION(01h) register when in SVID mode. > It works when in PMBUS mode. In SVID mode the regulator only cares > about the ENABLE pin. > > Add a workaround that needs the ENABLE pin to be left floating or > tied to a fixed level. The DT must contain the newly introduced > property 'infineon,en-pin-fixed-level' to enable this workaround. >
Why is that property even needed ? Isn't the workaround always (and only) required if the chip is in SVID mode ?
Guenter
> The workaround will map the PB_OPERATION_CONTROL_ON bit to the > PB_ON_OFF_CONFIG_EN_PIN_REQ bit of the ON_OFF_CONFIG register. > In combination with the fixed level on the ENABLE pin the regulator > can be controlled as expected. > > Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com> > Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com> > --- > drivers/hwmon/pmbus/tda38640.c | 95 +++++++++++++++++++++++++++++++++- > 1 file changed, 93 insertions(+), 2 deletions(-) > > diff --git a/drivers/hwmon/pmbus/tda38640.c b/drivers/hwmon/pmbus/tda38640.c > index 450b0273fb59..9d3b89d9845c 100644 > --- a/drivers/hwmon/pmbus/tda38640.c > +++ b/drivers/hwmon/pmbus/tda38640.c > @@ -18,6 +18,72 @@ static const struct regulator_desc __maybe_unused tda38640_reg_desc[] = { > PMBUS_REGULATOR("vout", 0), > }; > > +struct tda38640_data { > + struct pmbus_driver_info info; > + u32 en_pin_lvl; > +}; > + > +#define to_tda38640_data(x) container_of(x, struct tda38640_data, info) > + > +/* > + * Map PB_ON_OFF_CONFIG_POLARITY_HIGH to PB_OPERATION_CONTROL_ON. > + */ > +static int tda38640_read_byte_data(struct i2c_client *client, int page, int reg) > +{ > + const struct pmbus_driver_info *info = pmbus_get_driver_info(client); > + struct tda38640_data *data = to_tda38640_data(info); > + int ret, on_off_config, enabled; > + > + if (reg != PMBUS_OPERATION) > + return -ENODATA; > + > + ret = pmbus_read_byte_data(client, page, reg); > + if (ret < 0) > + return ret; > + > + on_off_config = pmbus_read_byte_data(client, page, > + PMBUS_ON_OFF_CONFIG); > + if (on_off_config < 0) > + return on_off_config; > + > + enabled = !!(on_off_config & PB_ON_OFF_CONFIG_POLARITY_HIGH); > + > + enabled ^= data->en_pin_lvl; > + if (enabled) > + ret &= ~PB_OPERATION_CONTROL_ON; > + else > + ret |= PB_OPERATION_CONTROL_ON; > + > + return ret; > +} > + > +/* > + * Map PB_OPERATION_CONTROL_ON to PB_ON_OFF_CONFIG_POLARITY_HIGH. > + */ > +static int tda38640_write_byte_data(struct i2c_client *client, int page, > + int reg, u8 byte) > +{ > + const struct pmbus_driver_info *info = pmbus_get_driver_info(client); > + struct tda38640_data *data = to_tda38640_data(info); > + int enable, ret; > + > + if (reg != PMBUS_OPERATION) > + return -ENODATA; > + > + enable = !!(byte & PB_OPERATION_CONTROL_ON); > + > + byte &= ~PB_OPERATION_CONTROL_ON; > + ret = pmbus_write_byte_data(client, page, reg, byte); > + if (ret < 0) > + return ret; > + > + enable ^= data->en_pin_lvl; > + > + return pmbus_update_byte_data(client, page, PMBUS_ON_OFF_CONFIG, > + PB_ON_OFF_CONFIG_POLARITY_HIGH, > + enable ? 0 : PB_ON_OFF_CONFIG_POLARITY_HIGH); > +} > + > static struct pmbus_driver_info tda38640_info = { > .pages = 1, > .format[PSC_VOLTAGE_IN] = linear, > @@ -26,7 +92,6 @@ static struct pmbus_driver_info tda38640_info = { > .format[PSC_CURRENT_IN] = linear, > .format[PSC_POWER] = linear, > .format[PSC_TEMPERATURE] = linear, > - > .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_STATUS_INPUT > | PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP > | PMBUS_HAVE_IIN > @@ -41,7 +106,33 @@ static struct pmbus_driver_info tda38640_info = { > > static int tda38640_probe(struct i2c_client *client) > { > - return pmbus_do_probe(client, &tda38640_info); > + struct device *dev = &client->dev; > + struct device_node *np = dev_of_node(dev); > + struct tda38640_data *data; > + > + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + memcpy(&data->info, &tda38640_info, sizeof(tda38640_info)); > + > + if (!CONFIG_SENSORS_TDA38640_REGULATOR || !np || > + of_property_read_u32(np, "infineon,en-pin-fixed-level", &data->en_pin_lvl)) > + return pmbus_do_probe(client, &data->info); > + > + /* > + * Apply ON_OFF_CONFIG workaround as enabling the regulator using the > + * OPERATION register doesn't work in SVID mode. > + */ > + data->info.read_byte_data = tda38640_read_byte_data; > + data->info.write_byte_data = tda38640_write_byte_data; > + /* > + * One should configure PMBUS_ON_OFF_CONFIG here, but > + * PB_ON_OFF_CONFIG_POWERUP_CONTROL, PB_ON_OFF_CONFIG_EN_PIN_REQ and > + * PB_ON_OFF_CONFIG_EN_PIN_REQ are ignored by the device. > + * Only PB_ON_OFF_CONFIG_POLARITY_HIGH has an effect. > + */ > + > + return pmbus_do_probe(client, &data->info); > } > > static const struct i2c_device_id tda38640_id[] = {
| |