Messages in this thread Patch in this message | | | Date | Wed, 26 Jul 2023 17:52:52 +0530 | Subject | Re: [PATCH 3/3] hwmon: (pmbus/tda38640) Add workaround for bug in SVID mode | From | Naresh Solanki <> |
| |
Hi Guenter,
On 25-07-2023 07:51 pm, Guenter Roeck wrote: > 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 ? Will add below function to detect SVID mode. static bool svid_mode(struct i2c_client *client) { /* PMBUS_MFR_READ(0xD0) + Address */ u8 write_buf[] = {0xd0, 0x44, 0x00}; u8 read_buf[2]; int ret;
struct i2c_msg msgs[2] = { { .addr = client->addr, .flags = 0, .buf = write_buf, .len = sizeof(write_buf), }, { .addr = client->addr, .flags = I2C_M_RD, .buf = read_buf, .len = sizeof(read_buf), } };
ret = i2c_transfer(client->adapter, msgs, 2);
if (ret < 0) { dev_err(&client->dev, "%s:%d i2c_transfer failed. %d", __func__, __LINE__, ret); return ret; } /* 0x44[15] determines PMBus Operating Mode */ return !!(read_buf[1] & BIT(7)); }
Based on svid_mode will init accordingly: if (!IS_ENABLED(CONFIG_SENSORS_TDA38640_REGULATOR) || !np || !svid_mode(client)) return pmbus_do_probe(client, &data->info);
dev_dbg(&client->dev, "SVID mode");
Regards, Naresh > > 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[] = { >
| |