Re: [PATCH v4 2/2] hwmon: (pmbus/tda38740) Add driver for Infineon TDA38740/TDA38725

[Colin Huang]

Guenter Roeck <linux@xxxxxxxxxxxx> 於 2026年9月10日週四 下午11:19寫道：
>
> On 9/10/26 05:35, Colin Huang wrote:
> > From: Colin Huang <u8813345@xxxxxxxxx>
> >
> > Add a PMBus driver for Infineon TDA38740 and TDA38725
> > single-voltage synchronous buck regulators.
> >
> > Signed-off-by: Colin Huang <u8813345@xxxxxxxxx>
> > ---
> >   Documentation/hwmon/index.rst    |  1 +
> >   Documentation/hwmon/tda38740.rst | 65 +++++++++++++++++++++++++++
> >   drivers/hwmon/pmbus/Kconfig      |  9 ++++
> >   drivers/hwmon/pmbus/Makefile     |  1 +
> >   drivers/hwmon/pmbus/tda38740.c   | 94 ++++++++++++++++++++++++++++++++++++++++
> >   5 files changed, 170 insertions(+)
> >
> > diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
> > index f07977a20271..aeff87b3d46d 100644
> > --- a/Documentation/hwmon/index.rst
> > +++ b/Documentation/hwmon/index.rst
> > @@ -265,6 +265,7 @@ Hardware Monitoring Kernel Drivers
> >      sy7636a-hwmon
> >      tc654
> >      tc74
> > +   tda38740
> >      thmc50
> >      tmp102
> >      tmp103
> > diff --git a/Documentation/hwmon/tda38740.rst b/Documentation/hwmon/tda38740.rst
> > new file mode 100644
> > index 000000000000..55ce12f14914
> > --- /dev/null
> > +++ b/Documentation/hwmon/tda38740.rst
> > @@ -0,0 +1,65 @@
> > +.. SPDX-License-Identifier: GPL-2.0-only
> > +
> > +====================================
> > +Kernel driver tda38740
> > +====================================
> > +
> > +Supported chips:
> > +  * Infineon TDA38740
> > +  * Infineon TDA38725
> > +
> > +    Prefix: 'tda38740'
> > +
> > +    Addresses scanned: -
> > +
> > +    Datasheet:
> > +      https://www.infineon.com/row/public/documents/24/49/infineon-tda38740-tda38725-datasheet-en.pdf
> > +
> > +Author: Colin Huang <u8813345@xxxxxxxxx>
> > +
> > +Description
> > +-----------
> > +
> > +The TDA38740 and TDA38725 are single-output synchronous buck regulators with a
> > +PMBus interface. They expose basic voltage, current, power and temperature
> > +telemetry through the standard PMBus hwmon interfaces.
> > +
> > +Sysfs entries
> > +-------------
> > +
> > +======================= =======================================================
> > +curr1_alarm          Input current alarm
> > +curr1_input          Measured input current
> > +curr1_label          "iin"
> > +
> > +curr2_crit           Critical maximum output current
> > +curr2_crit_alarm     Output current critical high alarm
> > +curr2_input          Measured output current
> > +curr2_label          "iout"
> > +
> > +in1_crit             Critical maximum input voltage
> > +in1_crit_alarm               Input voltage critical high alarm
> > +in1_input            Measured input voltage
> > +in1_label            "vin"
> > +
> > +in2_crit             Critical maximum output voltage
> > +in2_crit_alarm               Output voltage critical high alarm
> > +in2_input            Measured output voltage
> > +in2_label            "vout"
> > +in2_lcrit            Critical minimum output voltage
> > +in2_lcrit_alarm              Output voltage critical low alarm
> > +
> > +power1_alarm         Input power alarm
> > +power1_input         Measured input power
> > +power1_label         "pin"
> > +
> > +power2_input         Measured output power
> > +power2_label         "pout"
> > +
> > +temp1_crit           Critical high temperature
> > +temp1_crit_alarm     Chip temperature critical high alarm
> > +temp1_input          Measured temperature
> > +temp1_max            Maximum temperature
> > +temp1_max_alarm              Chip temperature high alarm
> > +======================= =======================================================
> > +
> > diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
> > index bcfdc4ce4c10..e4ca80dd0574 100644
> > --- a/drivers/hwmon/pmbus/Kconfig
> > +++ b/drivers/hwmon/pmbus/Kconfig
> > @@ -763,6 +763,15 @@ config SENSORS_TDA38640_REGULATOR
> >         If you say yes here you get regulator support for Infineon
> >         TDA38640 as regulator.
> >
> > +config SENSORS_TDA38740
> > +     tristate "Infineon TDA38725/TDA38740"
> > +     help
> > +       If you say yes here you get hardware monitoring support for Infineon
> > +       TDA38725 and TDA38740.
> > +
> > +       This driver can also be built as a module. If so, the module will
> > +       be called tda38740.
> > +
> >   config SENSORS_TPS25990
> >       tristate "TI TPS25990"
> >       help
> > diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
> > index e288fe72a437..eb06d47816fd 100644
> > --- a/drivers/hwmon/pmbus/Makefile
> > +++ b/drivers/hwmon/pmbus/Makefile
> > @@ -70,6 +70,7 @@ obj-$(CONFIG_SENSORS_STEF48H28)     += stef48h28.o
> >   obj-$(CONFIG_SENSORS_SQ24860)       += sq24860.o
> >   obj-$(CONFIG_SENSORS_STPDDC60)      += stpddc60.o
> >   obj-$(CONFIG_SENSORS_TDA38640)      += tda38640.o
> > +obj-$(CONFIG_SENSORS_TDA38740)       += tda38740.o
> >   obj-$(CONFIG_SENSORS_TPS25990)      += tps25990.o
> >   obj-$(CONFIG_SENSORS_TPS40422)      += tps40422.o
> >   obj-$(CONFIG_SENSORS_TPS53679)      += tps53679.o
> > diff --git a/drivers/hwmon/pmbus/tda38740.c b/drivers/hwmon/pmbus/tda38740.c
> > new file mode 100644
> > index 000000000000..6d3e9884f733
> > --- /dev/null
> > +++ b/drivers/hwmon/pmbus/tda38740.c
> > @@ -0,0 +1,94 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Hardware monitoring driver for Infineon TDA38725/TDA38740
> > + *
> > + * Copyright (c) 2023 9elements GmbH
> > + *
> > + */
> > +
> > +#include <linux/err.h>
> > +#include <linux/i2c.h>
> > +#include <linux/init.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include "pmbus.h"
> > +
> > +/*
> > + * TDA38725/TDA38740 only support Linear format for VOUT related commands,
> > + * with exponents in the range of -8 to -12 (see datasheet VOUT_MODE
> > + * description). Direct format is not supported by this device.
> > + */
> > +static int tda38740_identify(struct i2c_client *client,
> > +                          struct pmbus_driver_info *info)
> > +{
> > +     int vout_mode;
> > +
> > +     vout_mode = pmbus_read_byte_data(client, 0, PMBUS_VOUT_MODE);
> > +     if (vout_mode < 0 || vout_mode == 0xff)
> > +             return vout_mode < 0 ? vout_mode : -ENODEV;
> > +
> > +     if ((vout_mode >> 5) != 0)
> > +             return -ENODEV;
> > +
>
> What is the point of this code ? The PMBus core, in pmbus_identify_common(),
> already compares the configured mode (info->format[PSC_VOLTAGE_OUT]) with
> the value in the PMBUS_VOUT_MODE register and bails out if it does not match.
> The code here just seems to duplicate that. Am I missing something ?
Hi Guenter,
  Thanks for your review.
  Yes, code is duplicating.
  I will remove this function in next.

Best regards,
Colin Huang

> > +     info->format[PSC_VOLTAGE_OUT] = linear;
> > +
> > +     return 0;
> > +}
> > +
> > +static struct pmbus_driver_info tda38740_info = {
> > +     .pages = 1,
> > +     .format[PSC_VOLTAGE_IN] = linear,
> > +     .format[PSC_CURRENT_OUT] = linear,
> > +     .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
> > +         | PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT
> > +         | PMBUS_HAVE_IOUT | PMBUS_HAVE_STATUS_IOUT
> > +         | PMBUS_HAVE_POUT | PMBUS_HAVE_PIN,
> > +     .identify = tda38740_identify,
> > +};
> > +
> > +static int tda38740_probe(struct i2c_client *client)
> > +{
> > +     struct tda38740_data *data;
> > +
>
> This definition got lost, and I don't immediately see why it would be
> needed.
>
> Thanks,
> Guenter
>
Thanks for the review.
I plan to change it to:
```
static int tda38740_probe(struct i2c_client *client)
{
return pmbus_do_probe(client, &tda38740_info);
}
```
Best regards,
Colin Huang


> > +     data = devm_kzalloc(&client->dev, sizeof(*data), GFP_KERNEL);
> > +     if (!data)
> > +             return -ENOMEM;
> > +
> > +     memcpy(&data->info, &tda38740_info, sizeof(tda38740_info));
> > +
> > +     return pmbus_do_probe(client, &data->info);
> > +}
> > +
> > +static const struct i2c_device_id tda38740_id[] = {
> > +     { .name = "tda38725"},
> > +     { .name = "tda38740"},
> > +     {}
> > +};
> > +MODULE_DEVICE_TABLE(i2c, tda38740_id);
> > +
> > +static const struct of_device_id __maybe_unused tda38740_of_match[] = {
> > +     { .compatible = "infineon,tda38725"},
> > +     { .compatible = "infineon,tda38740"},
> > +     {}
> > +};
> > +MODULE_DEVICE_TABLE(of, tda38740_of_match);
> > +
> > +static struct i2c_driver tda38740_driver = {
> > +     .driver = {
> > +             .name = "tda38740",
> > +             .of_match_table = of_match_ptr(tda38740_of_match),
> > +     },
> > +     .probe = tda38740_probe,
> > +     .id_table = tda38740_id,
> > +};
> > +
> > +module_i2c_driver(tda38740_driver);
> > +
> > +MODULE_DESCRIPTION("PMBus driver for Infineon TDA38725/TDA38740");
> > +MODULE_LICENSE("GPL");
> > +MODULE_IMPORT_NS("PMBUS");
> >
>