| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <46f3f44b-5c25-8193-70b5-59303be04ad0@roeck-us.net>
Date: Tue, 25 Jul 2023 07:21:09 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Naresh Solanki <naresh.solanki@...ements.com>,
Jean Delvare <jdelvare@...e.com>,
krzysztof.kozlowski+dt@...aro.org
Cc: linux-hwmon@...r.kernel.org,
Patrick Rudolph <patrick.rudolph@...ements.com>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] hwmon: (pmbus/tda38640) Add workaround for bug in
SVID mode
On 7/25/23 04:40, Naresh Solanki wrote:
> From: Patrick Rudolph <patrick.rudolph@...ements.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@...ements.com>
> Signed-off-by: Naresh Solanki <Naresh.Solanki@...ements.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[] = {
Powered by blists - more mailing lists