* Re: [lm-sensors] [PATCH 1/4] libsensors4: Support more bus types,
2007-08-17 15:14 [lm-sensors] [PATCH 1/4] libsensors4: Support more bus types, part 1 Jean Delvare
@ 2007-08-17 19:44 ` Hans de Goede
2007-08-18 15:05 ` Jean Delvare
` (12 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Hans de Goede @ 2007-08-17 19:44 UTC (permalink / raw)
To: lm-sensors
Jean Delvare wrote:
> Support more bus types (part 1 of 2). Originally libsensors was very
> i2c-centric. Make it more neutral so that we can cleanly support
> additional bus types such as SPI or One-Wire.
>
> This first part introduces sensors_bus_id, and updates
> sensors_chip_name to use it.
>
Looks good to me, can you commit these to svn before the end of the weekend,
monday I'll be back at work and there I have several machines to test with, so
if you can get all your changes into svn before monday then I can give svn a
good testing with various setups @ work.
Regards,
Hans
> ---
> lib/access.c | 43 +++++++++++++++++------------------
> lib/data.c | 62 +++++++++++++++++++++++++++++----------------------
> lib/sensors.h | 22 ++++++++++++------
> lib/sysfs.c | 24 ++++++++++++-------
> prog/sensord/args.c | 3 +-
> prog/sensors/main.c | 7 +++--
> 6 files changed, 93 insertions(+), 68 deletions(-)
>
> --- lm-sensors-3.orig/lib/sensors.h 2007-08-17 09:23:31.000000000 +0200
> +++ lm-sensors-3/lib/sensors.h 2007-08-17 09:24:35.000000000 +0200
> @@ -1,6 +1,7 @@
> /*
> sensors.h - Part of libsensors, a Linux library for reading sensor data.
> Copyright (c) 1998, 1999 Frodo Looijaard <frodol@dds.nl>
> + Copyright (C) 2007 Jean Delvare <khali@linux-fr.org>
>
> This program is free software; you can redistribute it and/or modify
> it under the terms of the GNU General Public License as published by
> @@ -26,22 +27,29 @@
> /* Publicly accessible library functions */
>
> #define SENSORS_CHIP_NAME_PREFIX_ANY NULL
> -#define SENSORS_CHIP_NAME_BUS_ISA -1
> -#define SENSORS_CHIP_NAME_BUS_ANY -2
> -#define SENSORS_CHIP_NAME_BUS_ANY_I2C -3
> -#define SENSORS_CHIP_NAME_BUS_PCI -5
> #define SENSORS_CHIP_NAME_ADDR_ANY -1
>
> +#define SENSORS_BUS_TYPE_ANY (-1)
> +#define SENSORS_BUS_TYPE_I2C 0
> +#define SENSORS_BUS_TYPE_ISA 1
> +#define SENSORS_BUS_TYPE_PCI 2
> +#define SENSORS_BUS_NR_ANY (-1)
> +
> #ifdef __cplusplus
> extern "C" {
> #endif /* __cplusplus */
>
> extern const char *libsensors_version;
>
> +typedef struct sensors_bus_id {
> + short type;
> + short nr;
> +} sensors_bus_id;
> +
> /* A chip name is encoded in this structure */
> typedef struct sensors_chip_name {
> char *prefix;
> - int bus;
> + sensors_bus_id bus;
> int addr;
> char *path;
> } sensors_chip_name;
> @@ -70,10 +78,10 @@ int sensors_snprintf_chip_name(char *str
> int sensors_match_chip(const sensors_chip_name *chip1,
> const sensors_chip_name *chip2);
>
> -/* This function returns the adapter name of a bus number,
> +/* This function returns the adapter name of a bus,
> as used within the sensors_chip_name structure. If it could not be found,
> it returns NULL */
> -const char *sensors_get_adapter_name(int bus_nr);
> +const char *sensors_get_adapter_name(const sensors_bus_id *bus);
>
> /* Look up the label which belongs to this chip. Note that chip should not
> contain wildcard values! *result is newly allocated (free it yourself).
> --- lm-sensors-3.orig/lib/data.c 2007-08-17 09:23:31.000000000 +0200
> +++ lm-sensors-3/lib/data.c 2007-08-17 10:40:49.000000000 +0200
> @@ -92,7 +92,8 @@ int sensors_parse_chip_name(const char *
> /* Then we have either a sole "*" (all chips with this name) or a bus
> type and an address. */
> if (!strcmp(name, "*")) {
> - res->bus = SENSORS_CHIP_NAME_BUS_ANY;
> + res->bus.type = SENSORS_BUS_TYPE_ANY;
> + res->bus.nr = SENSORS_BUS_NR_ANY;
> res->addr = SENSORS_CHIP_NAME_ADDR_ANY;
> return 0;
> }
> @@ -100,11 +101,11 @@ int sensors_parse_chip_name(const char *
> if (!(dash = strchr(name, '-')))
> goto ERROR;
> if (!strncmp(name, "i2c", dash - name))
> - res->bus = SENSORS_CHIP_NAME_BUS_ANY_I2C;
> + res->bus.type = SENSORS_BUS_TYPE_I2C;
> else if (!strncmp(name, "isa", dash - name))
> - res->bus = SENSORS_CHIP_NAME_BUS_ISA;
> + res->bus.type = SENSORS_BUS_TYPE_ISA;
> else if (!strncmp(name, "pci", dash - name))
> - res->bus = SENSORS_CHIP_NAME_BUS_PCI;
> + res->bus.type = SENSORS_BUS_TYPE_PCI;
> else
> goto ERROR;
> name = dash + 1;
> @@ -112,17 +113,21 @@ int sensors_parse_chip_name(const char *
> /* Some bus types (i2c) have an additional bus number. For these, the
> next part is either a "*" (any bus of that type) or a decimal
> number. */
> - switch (res->bus) {
> - case SENSORS_CHIP_NAME_BUS_ANY_I2C:
> + switch (res->bus.type) {
> + case SENSORS_BUS_TYPE_I2C:
> if (!strncmp(name, "*-", 2)) {
> + res->bus.nr = SENSORS_BUS_NR_ANY;
> name += 2;
> break;
> }
>
> - res->bus = strtoul(name, &dash, 10);
> - if (*name = '\0' || *dash != '-' || res->bus < 0)
> + res->bus.nr = strtoul(name, &dash, 10);
> + if (*name = '\0' || *dash != '-' || res->bus.nr < 0)
> goto ERROR;
> name = dash + 1;
> + break;
> + default:
> + res->bus.nr = SENSORS_BUS_NR_ANY;
> }
>
> /* Last part is the chip address, or "*" for any address. */
> @@ -147,17 +152,19 @@ int sensors_snprintf_chip_name(char *str
> if (sensors_chip_name_has_wildcards(chip))
> return -SENSORS_ERR_WILDCARDS;
>
> - switch (chip->bus) {
> - case SENSORS_CHIP_NAME_BUS_ISA:
> + switch (chip->bus.type) {
> + case SENSORS_BUS_TYPE_ISA:
> return snprintf(str, size, "%s-isa-%04x", chip->prefix,
> chip->addr);
> - case SENSORS_CHIP_NAME_BUS_PCI:
> + case SENSORS_BUS_TYPE_PCI:
> return snprintf(str, size, "%s-pci-%04x", chip->prefix,
> chip->addr);
> - default:
> - return snprintf(str, size, "%s-i2c-%d-%02x", chip->prefix,
> - chip->bus, chip->addr);
> + case SENSORS_BUS_TYPE_I2C:
> + return snprintf(str, size, "%s-i2c-%hd-%02x", chip->prefix,
> + chip->bus.nr, chip->addr);
> }
> +
> + return -SENSORS_ERR_CHIP_NAME;
> }
>
> int sensors_parse_i2cbus_name(const char *name, int *res)
> @@ -178,12 +185,13 @@ int sensors_substitute_chip(sensors_chip
> {
> int i, j;
> for (i = 0; i < sensors_config_busses_count; i++)
> - if (sensors_config_busses[i].number = name->bus)
> + if (name->bus.type = SENSORS_BUS_TYPE_I2C &&
> + sensors_config_busses[i].number = name->bus.nr)
> break;
>
> if (i = sensors_config_busses_count) {
> sensors_parse_error("Undeclared i2c bus referenced", lineno);
> - name->bus = sensors_proc_bus_count;
> + name->bus.nr = sensors_proc_bus_count;
> return -SENSORS_ERR_BUS_NAME;
> }
>
> @@ -191,14 +199,14 @@ int sensors_substitute_chip(sensors_chip
> for (j = 0; j < sensors_proc_bus_count; j++) {
> if (!strcmp(sensors_config_busses[i].adapter,
> sensors_proc_bus[j].adapter)) {
> - name->bus = sensors_proc_bus[j].number;
> + name->bus.nr = sensors_proc_bus[j].number;
> return 0;
> }
> }
>
> /* We did not find anything. sensors_proc_bus_count is not
> a valid bus number, so it will never be matched. Good. */
> - name->bus = sensors_proc_bus_count;
> + name->bus.nr = sensors_proc_bus_count;
> return 0;
> }
>
> @@ -211,14 +219,16 @@ int sensors_substitute_busses(void)
> for (i = 0; i < sensors_config_chips_count; i++) {
> lineno = sensors_config_chips[i].lineno;
> chips = &sensors_config_chips[i].chips;
> - for (j = 0; j < chips->fits_count; j++)
> - if (chips->fits[j].bus != SENSORS_CHIP_NAME_BUS_ISA &&
> - chips->fits[j].bus != SENSORS_CHIP_NAME_BUS_PCI &&
> - chips->fits[j].bus != SENSORS_CHIP_NAME_BUS_ANY &&
> - chips->fits[j].bus != SENSORS_CHIP_NAME_BUS_ANY_I2C)
> - if ((err = sensors_substitute_chip(chips->fits+j,
> - lineno)))
> - res = err;
> + for (j = 0; j < chips->fits_count; j++) {
> + /* We can only substitute if a specific bus number
> + is given. */
> + if (chips->fits[j].bus.nr = SENSORS_BUS_NR_ANY)
> + continue;
> +
> + err = sensors_substitute_chip(&chips->fits[j], lineno);
> + if (err)
> + res = err;
> + }
> }
> return res;
> }
> --- lm-sensors-3.orig/lib/access.c 2007-08-17 09:23:31.000000000 +0200
> +++ lm-sensors-3/lib/access.c 2007-08-17 09:24:35.000000000 +0200
> @@ -1,6 +1,7 @@
> /*
> access.c - Part of libsensors, a Linux library for reading sensor data.
> Copyright (c) 1998, 1999 Frodo Looijaard <frodol@dds.nl>
> + Copyright (C) 2007 Jean Delvare <khali@linux-fr.org>
>
> This program is free software; you can redistribute it and/or modify
> it under the terms of the GNU General Public License as published by
> @@ -41,22 +42,15 @@ int sensors_match_chip(const sensors_chi
> strcasecmp(chip1->prefix, chip2->prefix))
> return 0;
>
> - if ((chip1->bus != SENSORS_CHIP_NAME_BUS_ANY) &&
> - (chip2->bus != SENSORS_CHIP_NAME_BUS_ANY) &&
> - (chip1->bus != chip2->bus)) {
> -
> - if ((chip1->bus = SENSORS_CHIP_NAME_BUS_ISA) ||
> - (chip2->bus = SENSORS_CHIP_NAME_BUS_ISA))
> - return 0;
> -
> - if ((chip1->bus = SENSORS_CHIP_NAME_BUS_PCI) ||
> - (chip2->bus = SENSORS_CHIP_NAME_BUS_PCI))
> - return 0;
> -
> - if ((chip1->bus != SENSORS_CHIP_NAME_BUS_ANY_I2C) &&
> - (chip2->bus != SENSORS_CHIP_NAME_BUS_ANY_I2C))
> - return 0;
> - }
> + if ((chip1->bus.type != SENSORS_BUS_TYPE_ANY) &&
> + (chip2->bus.type != SENSORS_BUS_TYPE_ANY) &&
> + (chip1->bus.type != chip2->bus.type))
> + return 0;
> +
> + if ((chip1->bus.nr != SENSORS_BUS_NR_ANY) &&
> + (chip2->bus.nr != SENSORS_BUS_NR_ANY) &&
> + (chip1->bus.nr != chip2->bus.nr))
> + return 0;
>
> if ((chip1->addr != chip2->addr) &&
> (chip1->addr != SENSORS_CHIP_NAME_ADDR_ANY) &&
> @@ -136,8 +130,8 @@ sensors_lookup_feature_name(const sensor
> int sensors_chip_name_has_wildcards(const sensors_chip_name *chip)
> {
> if ((chip->prefix = SENSORS_CHIP_NAME_PREFIX_ANY) ||
> - (chip->bus = SENSORS_CHIP_NAME_BUS_ANY) ||
> - (chip->bus = SENSORS_CHIP_NAME_BUS_ANY_I2C) ||
> + (chip->bus.type = SENSORS_BUS_TYPE_ANY) ||
> + (chip->bus.nr = SENSORS_BUS_NR_ANY) ||
> (chip->addr = SENSORS_CHIP_NAME_ADDR_ANY))
> return 1;
> else
> @@ -318,16 +312,21 @@ const sensors_chip_name *sensors_get_det
> return res;
> }
>
> -const char *sensors_get_adapter_name(int bus_nr)
> +const char *sensors_get_adapter_name(const sensors_bus_id *bus)
> {
> int i;
>
> - if (bus_nr = SENSORS_CHIP_NAME_BUS_ISA)
> + /* bus types with a single instance */
> + switch (bus->type) {
> + case SENSORS_BUS_TYPE_ISA:
> return "ISA adapter";
> - if (bus_nr = SENSORS_CHIP_NAME_BUS_PCI)
> + case SENSORS_BUS_TYPE_PCI:
> return "PCI adapter";
> + }
> +
> + /* bus types with several instances */
> for (i = 0; i < sensors_proc_bus_count; i++)
> - if (sensors_proc_bus[i].number = bus_nr)
> + if (sensors_proc_bus[i].number = bus->nr)
> return sensors_proc_bus[i].adapter;
> return NULL;
> }
> --- lm-sensors-3.orig/lib/sysfs.c 2007-08-17 09:23:31.000000000 +0200
> +++ lm-sensors-3/lib/sysfs.c 2007-08-17 09:24:35.000000000 +0200
> @@ -226,14 +226,16 @@ static int sensors_read_one_sysfs_chip(s
> if (!entry.chip.path)
> sensors_fatal_error(__FUNCTION__, "out of memory");
>
> - if (sscanf(dev->name, "%d-%x", &entry.chip.bus, &entry.chip.addr) = 2) {
> + if (sscanf(dev->name, "%hd-%x", &entry.chip.bus.nr, &entry.chip.addr) = 2) {
> /* find out if legacy ISA or not */
> - if (entry.chip.bus = 9191)
> - entry.chip.bus = SENSORS_CHIP_NAME_BUS_ISA;
> - else {
> + if (entry.chip.bus.nr = 9191) {
> + entry.chip.bus.type = SENSORS_BUS_TYPE_ISA;
> + entry.chip.bus.nr = 0;
> + } else {
> + entry.chip.bus.type = SENSORS_BUS_TYPE_I2C;
> snprintf(bus_path, sizeof(bus_path),
> "%s/class/i2c-adapter/i2c-%d/device/name",
> - sensors_sysfs_mount, entry.chip.bus);
> + sensors_sysfs_mount, entry.chip.bus.nr);
>
> if ((bus_attr = sysfs_open_attribute(bus_path))) {
> if (sysfs_read_attribute(bus_attr)) {
> @@ -242,19 +244,23 @@ static int sensors_read_one_sysfs_chip(s
> }
>
> if (bus_attr->value
> - && !strncmp(bus_attr->value, "ISA ", 4))
> - entry.chip.bus = SENSORS_CHIP_NAME_BUS_ISA;
> + && !strncmp(bus_attr->value, "ISA ", 4)) {
> + entry.chip.bus.type = SENSORS_BUS_TYPE_ISA;
> + entry.chip.bus.nr = 0;
> + }
>
> sysfs_close_attribute(bus_attr);
> }
> }
> } else if (sscanf(dev->name, "%*[a-z0-9_].%d", &entry.chip.addr) = 1) {
> /* must be new ISA (platform driver) */
> - entry.chip.bus = SENSORS_CHIP_NAME_BUS_ISA;
> + entry.chip.bus.type = SENSORS_BUS_TYPE_ISA;
> + entry.chip.bus.nr = 0;
> } else if (sscanf(dev->name, "%x:%x:%x.%x", &domain, &bus, &slot, &fn) = 4) {
> /* PCI */
> entry.chip.addr = (domain << 16) + (bus << 8) + (slot << 3) + fn;
> - entry.chip.bus = SENSORS_CHIP_NAME_BUS_PCI;
> + entry.chip.bus.type = SENSORS_BUS_TYPE_PCI;
> + entry.chip.bus.nr = 0;
> } else
> goto exit_free;
>
> --- lm-sensors-3.orig/prog/sensord/args.c 2007-08-17 09:23:31.000000000 +0200
> +++ lm-sensors-3/prog/sensord/args.c 2007-08-17 09:24:35.000000000 +0200
> @@ -288,7 +288,8 @@ parseChips
> (int argc, char **argv) {
> if (optind = argc) {
> chipNames[0].prefix = SENSORS_CHIP_NAME_PREFIX_ANY;
> - chipNames[0].bus = SENSORS_CHIP_NAME_BUS_ANY;
> + chipNames[0].bus.type = SENSORS_BUS_TYPE_ANY;
> + chipNames[0].bus.nr = SENSORS_BUS_NR_ANY;
> chipNames[0].addr = SENSORS_CHIP_NAME_ADDR_ANY;
> numChipNames = 1;
> } else {
> --- lm-sensors-3.orig/prog/sensors/main.c 2007-08-17 09:23:31.000000000 +0200
> +++ lm-sensors-3/prog/sensors/main.c 2007-08-17 09:24:35.000000000 +0200
> @@ -209,7 +209,8 @@ int main (int argc, char *argv[])
>
> if (optind = argc) {
> chips[0].prefix = SENSORS_CHIP_NAME_PREFIX_ANY;
> - chips[0].bus = SENSORS_CHIP_NAME_BUS_ANY;
> + chips[0].bus.type = SENSORS_BUS_TYPE_ANY;
> + chips[0].bus.nr = SENSORS_BUS_NR_ANY;
> chips[0].addr = SENSORS_CHIP_NAME_ADDR_ANY;
> chips_count = 1;
> } else
> @@ -310,11 +311,11 @@ void do_a_print(const sensors_chip_name
>
> printf("%s\n",sprintf_chip_name(name));
> if (!hide_adapter) {
> - const char *adap = sensors_get_adapter_name(name->bus);
> + const char *adap = sensors_get_adapter_name(&name->bus);
> if (adap)
> printf("Adapter: %s\n", adap);
> else
> - fprintf(stderr, "Can't get adapter name for bus %d\n", name->bus);
> + fprintf(stderr, "Can't get adapter name\n");
> }
> if (do_unknown)
> print_unknown_chip(name);
>
>
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [lm-sensors] [PATCH 1/4] libsensors4: Support more bus types,
2007-08-17 15:14 [lm-sensors] [PATCH 1/4] libsensors4: Support more bus types, part 1 Jean Delvare
2007-08-17 19:44 ` [lm-sensors] [PATCH 1/4] libsensors4: Support more bus types, Hans de Goede
@ 2007-08-18 15:05 ` Jean Delvare
2007-08-18 21:07 ` Henrique de Moraes Holschuh
` (11 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Jean Delvare @ 2007-08-18 15:05 UTC (permalink / raw)
To: lm-sensors
Hi Hans,
On Fri, 17 Aug 2007 21:44:50 +0200, Hans de Goede wrote:
> Jean Delvare wrote:
> > Support more bus types (part 1 of 2). Originally libsensors was very
> > i2c-centric. Make it more neutral so that we can cleanly support
> > additional bus types such as SPI or One-Wire.
> >
> > This first part introduces sensors_bus_id, and updates
> > sensors_chip_name to use it.
>
> Looks good to me, can you commit these to svn before the end of the weekend,
> monday I'll be back at work and there I have several machines to test with, so
> if you can get all your changes into svn before monday then I can give svn a
> good testing with various setups @ work.
Sure, I plan to commit them on Saturday evening or Sunday morning,
unless someone finds problem before then.
If possible, your testing should include passing funky chip names on the
sensors' command line, and/or using a configuration file with tricky
bus and chip statements.
BTW, thanks a lot for reviewing and testing my libsensors4 patches,
this is very much appreciated.
--
Jean Delvare
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [lm-sensors] [PATCH 1/4] libsensors4: Support more bus types,
2007-08-17 15:14 [lm-sensors] [PATCH 1/4] libsensors4: Support more bus types, part 1 Jean Delvare
2007-08-17 19:44 ` [lm-sensors] [PATCH 1/4] libsensors4: Support more bus types, Hans de Goede
2007-08-18 15:05 ` Jean Delvare
@ 2007-08-18 21:07 ` Henrique de Moraes Holschuh
2007-08-19 15:02 ` Jean Delvare
` (10 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-08-18 21:07 UTC (permalink / raw)
To: lm-sensors
On Fri, 17 Aug 2007, Jean Delvare wrote:
> Support more bus types (part 1 of 2). Originally libsensors was very
> i2c-centric. Make it more neutral so that we can cleanly support
> additional bus types such as SPI or One-Wire.
...
> +#define SENSORS_BUS_TYPE_ANY (-1)
> +#define SENSORS_BUS_TYPE_I2C 0
> +#define SENSORS_BUS_TYPE_ISA 1
> +#define SENSORS_BUS_TYPE_PCI 2
> +#define SENSORS_BUS_NR_ANY (-1)
What about BUS_HOST, for drivers doing a firmware<->hwmon bridge, like
thinkpad-acpi, asus-acpi, sonypi... ? They usually attach to the device
tree through one or more platform devices, and we are now trying to export
fan control and thermal monitoring using the hwmon class...
Or am I in the wrong track here?
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [lm-sensors] [PATCH 1/4] libsensors4: Support more bus types,
2007-08-17 15:14 [lm-sensors] [PATCH 1/4] libsensors4: Support more bus types, part 1 Jean Delvare
` (2 preceding siblings ...)
2007-08-18 21:07 ` Henrique de Moraes Holschuh
@ 2007-08-19 15:02 ` Jean Delvare
2007-08-19 19:14 ` Henrique de Moraes Holschuh
` (9 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Jean Delvare @ 2007-08-19 15:02 UTC (permalink / raw)
To: lm-sensors
Hi Henrique,
On Sat, 18 Aug 2007 18:07:49 -0300, Henrique de Moraes Holschuh wrote:
> On Fri, 17 Aug 2007, Jean Delvare wrote:
> > Support more bus types (part 1 of 2). Originally libsensors was very
> > i2c-centric. Make it more neutral so that we can cleanly support
> > additional bus types such as SPI or One-Wire.
> ...
>
> > +#define SENSORS_BUS_TYPE_ANY (-1)
> > +#define SENSORS_BUS_TYPE_I2C 0
> > +#define SENSORS_BUS_TYPE_ISA 1
> > +#define SENSORS_BUS_TYPE_PCI 2
> > +#define SENSORS_BUS_NR_ANY (-1)
>
> What about BUS_HOST, for drivers doing a firmware<->hwmon bridge, like
> thinkpad-acpi, asus-acpi, sonypi... ? They usually attach to the device
> tree through one or more platform devices, and we are now trying to export
> fan control and thermal monitoring using the hwmon class...
>
> Or am I in the wrong track here?
Thanks for pointing this out, indeed we need to make sure that the new
libsensors will work properly with the thinkpad_acpi driver and other
similar drivers. However I do not think that we need a special bus type
for these. The thinkpad_acpi driver instantiates a platform device (as
you mentioned yourself), and platform drivers are covered by the "ISA"
bus type in libsensors (for historical reasons mainly.)
BTW, I just gave a try to the thinkpad_acpi driver (in 2.6.22.3) with
libsensors4. I had to make the following changes to the driver for
libsensors4 to recognize it:
--- linux-2.6.22.orig/drivers/misc/thinkpad_acpi.c
+++ linux-2.6.22/drivers/misc/thinkpad_acpi.c
@@ -4211,6 +4211,13 @@ IBM_PARAM(brightness);
IBM_PARAM(volume);
IBM_PARAM(fan);
+static ssize_t thinkpad_acpi_show_name(struct device *dev, struct
+ device_attribute *devattr, char *buf)
+{
+ return sprintf(buf, "thinkpad\n");
+}
+static DEVICE_ATTR(name, S_IRUGO, thinkpad_acpi_show_name, NULL);
+
static int __init thinkpad_acpi_module_init(void)
{
int ret, i;
@@ -4248,7 +4255,7 @@ static int __init thinkpad_acpi_module_i
/* Device initialization */
- tpacpi_pdev = platform_device_register_simple(IBM_DRVR_NAME, -1,
+ tpacpi_pdev = platform_device_register_simple(IBM_DRVR_NAME, 0,
NULL, 0);
if (IS_ERR(tpacpi_pdev)) {
ret = PTR_ERR(tpacpi_pdev);
@@ -4257,6 +4264,12 @@ static int __init thinkpad_acpi_module_i
thinkpad_acpi_module_exit();
return ret;
}
+ ret = device_create_file(&tpacpi_pdev->dev, &dev_attr_name);
+ if (ret) {
+ printk(IBM_ERR "unable to create name attribute\n");
+ thinkpad_acpi_module_exit();
+ return ret;
+ }
tpacpi_hwmon = hwmon_device_register(&tpacpi_pdev->dev);
if (IS_ERR(tpacpi_hwmon)) {
ret = PTR_ERR(tpacpi_hwmon);
Basically, two changes: add a "name" attribute to the device, and give
the platform device an instance number.
The name attribute is needed and must be added to the thinkpad_acpi
driver now, otherwise it won't work with libsensors (neither current nor
libsensors4). This is where libsensors gets the "prefix" part of a
chip name.
The instance number is required for now, this is where libsensors gets
the "address" part of a chip name for platform devices. However, I
admit that it's not ideal that you have to declare a fake instance
number in the case of the thinkpad_acpi driver, as there can be only
one device almost by definition. Ideally libsensors should default to
address 0 for platform devices with no instance number. This should be
addressed as part of:
http://www.lm-sensors.org/ticket/2240
But I didn't plan to fix this before 3.0.1. Maybe we can have a
temporary workaround before that so that we don't have to hack the
thinkpad_acpi driver.
--
Jean Delvare
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [lm-sensors] [PATCH 1/4] libsensors4: Support more bus types,
2007-08-17 15:14 [lm-sensors] [PATCH 1/4] libsensors4: Support more bus types, part 1 Jean Delvare
` (3 preceding siblings ...)
2007-08-19 15:02 ` Jean Delvare
@ 2007-08-19 19:14 ` Henrique de Moraes Holschuh
2007-08-19 19:21 ` Henrique de Moraes Holschuh
` (8 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-08-19 19:14 UTC (permalink / raw)
To: lm-sensors
On Sun, 19 Aug 2007, Jean Delvare wrote:
> Basically, two changes: add a "name" attribute to the device, and give
> the platform device an instance number.
>
> The name attribute is needed and must be added to the thinkpad_acpi
> driver now, otherwise it won't work with libsensors (neither current nor
> libsensors4). This is where libsensors gets the "prefix" part of a
> chip name.
I will bake up a patch and send it for inclusion in 2.6.23-rc ASAP. If Len
and Linus will take it (they should, it is an obviously safe thing that
affects one driver only...).
> The instance number is required for now, this is where libsensors gets
> the "address" part of a chip name for platform devices. However, I
> admit that it's not ideal that you have to declare a fake instance
> number in the case of the thinkpad_acpi driver, as there can be only
> one device almost by definition. Ideally libsensors should default to
> address 0 for platform devices with no instance number. This should be
> addressed as part of:
>
> http://www.lm-sensors.org/ticket/2240
>
> But I didn't plan to fix this before 3.0.1. Maybe we can have a
> temporary workaround before that so that we don't have to hack the
> thinkpad_acpi driver.
Since this would probably hit problems in other drivers, I think a
work-around in libsensors is probably the best way to go about it.
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [lm-sensors] [PATCH 1/4] libsensors4: Support more bus types,
2007-08-17 15:14 [lm-sensors] [PATCH 1/4] libsensors4: Support more bus types, part 1 Jean Delvare
` (4 preceding siblings ...)
2007-08-19 19:14 ` Henrique de Moraes Holschuh
@ 2007-08-19 19:21 ` Henrique de Moraes Holschuh
2007-08-20 11:53 ` Henrique de Moraes Holschuh
` (7 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-08-19 19:21 UTC (permalink / raw)
To: lm-sensors
On Sun, 19 Aug 2007, Jean Delvare wrote:
> Thanks for pointing this out, indeed we need to make sure that the new
> libsensors will work properly with the thinkpad_acpi driver and other
> similar drivers. However I do not think that we need a special bus type
> for these. The thinkpad_acpi driver instantiates a platform device (as
> you mentioned yourself), and platform drivers are covered by the "ISA"
> bus type in libsensors (for historical reasons mainly.)
This sounds a bit of an ugliness that might be best fixed, since it is a
rare time window where one can actually do those. If it is not too painful,
why not add BUS_HOST to attach stuff that is not on PCI/PCIe, nor ISA, nor
I2C/SMBUS?
I mean, it is weird enough with ACPI, but there is also on-die sensors in
CPUs, and their ilk... and attaching those to BUS_ISA or BUS_PCI sounds like
something out of an acid trip IMHO, never mind that it would work just fine
(unlike most stuff that could come out of an acid trip, I suppose) :-)
I'm glat to know it is only a mostly cosmetic thing, but still, if it is not
too painful to fix...
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [lm-sensors] [PATCH 1/4] libsensors4: Support more bus types,
2007-08-17 15:14 [lm-sensors] [PATCH 1/4] libsensors4: Support more bus types, part 1 Jean Delvare
` (5 preceding siblings ...)
2007-08-19 19:21 ` Henrique de Moraes Holschuh
@ 2007-08-20 11:53 ` Henrique de Moraes Holschuh
2007-08-20 12:19 ` Jean Delvare
` (6 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-08-20 11:53 UTC (permalink / raw)
To: lm-sensors
On Sun, 19 Aug 2007, Henrique de Moraes Holschuh wrote:
> On Sun, 19 Aug 2007, Jean Delvare wrote:
> > Basically, two changes: add a "name" attribute to the device, and give
> > the platform device an instance number.
> >
> > The name attribute is needed and must be added to the thinkpad_acpi
> > driver now, otherwise it won't work with libsensors (neither current nor
> > libsensors4). This is where libsensors gets the "prefix" part of a
> > chip name.
>
> I will bake up a patch and send it for inclusion in 2.6.23-rc ASAP. If Len
> and Linus will take it (they should, it is an obviously safe thing that
> affects one driver only...).
Ok, I have one tentative patch ready, but I better make sure I got it right
on the first try.
First, is the "name" attribute documented anywhere? I may have to change
its contents later, if I break up thinkpad-acpi into various modules. Does
anything else other than libsensors4 uses it?
To avoid changing the contents of the "name" attribute, I'd have to create a
second platform device and dedicate it to hwmon (and name it
"thinkpad-sensors"). This is NOT a problem, but I am unsure if such a
change would be accepted in-tree this late in the game. I will probably try
it anyway, at least it is future-proof. It is probably safer to defer
thinkpad-acpi libsensors4 support to 2.6.24 if the change is not accepted
for -rc4.
Second, the libsensors configuration for thinkpad-acpi depends on the
thinkpad model. I *can* export its model to libsensors4 (I have that
information), but how do I go about it?
See, that's why I'd really like BUS_HOST. Unlike ISA which has port
numbers, or PCI, which has PCI-IDs, BUS_HOST is suitable to model and
version numbers as a way to differentiate various devices.
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [lm-sensors] [PATCH 1/4] libsensors4: Support more bus types,
2007-08-17 15:14 [lm-sensors] [PATCH 1/4] libsensors4: Support more bus types, part 1 Jean Delvare
` (6 preceding siblings ...)
2007-08-20 11:53 ` Henrique de Moraes Holschuh
@ 2007-08-20 12:19 ` Jean Delvare
2007-08-20 12:46 ` Henrique de Moraes Holschuh
` (5 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Jean Delvare @ 2007-08-20 12:19 UTC (permalink / raw)
To: lm-sensors
Hi Henrique,
On Mon, 20 Aug 2007 08:53:30 -0300, Henrique de Moraes Holschuh wrote:
> On Sun, 19 Aug 2007, Henrique de Moraes Holschuh wrote:
> > On Sun, 19 Aug 2007, Jean Delvare wrote:
> > > Basically, two changes: add a "name" attribute to the device, and give
> > > the platform device an instance number.
> > >
> > > The name attribute is needed and must be added to the thinkpad_acpi
> > > driver now, otherwise it won't work with libsensors (neither current nor
> > > libsensors4). This is where libsensors gets the "prefix" part of a
> > > chip name.
> >
> > I will bake up a patch and send it for inclusion in 2.6.23-rc ASAP. If Len
> > and Linus will take it (they should, it is an obviously safe thing that
> > affects one driver only...).
>
> Ok, I have one tentative patch ready, but I better make sure I got it right
> on the first try.
>
> First, is the "name" attribute documented anywhere? I may have to change
> its contents later, if I break up thinkpad-acpi into various modules. Does
> anything else other than libsensors4 uses it?
The name attribute is also used by libsensors3 and pwmconfig.
> To avoid changing the contents of the "name" attribute, I'd have to create a
> second platform device and dedicate it to hwmon (and name it
> "thinkpad-sensors"). This is NOT a problem, but I am unsure if such a
> change would be accepted in-tree this late in the game. I will probably try
> it anyway, at least it is future-proof. It is probably safer to defer
> thinkpad-acpi libsensors4 support to 2.6.24 if the change is not accepted
> for -rc4.
I would aim at 2.6.24. As you can see there are still a number of
things to be discussed, and there's no point in rushing a patch in
2.6.23 anyway, given that the stable version of libsensors still lacks
support for the thinkpad_acpi driver anyway.
> Second, the libsensors configuration for thinkpad-acpi depends on the
> thinkpad model. I *can* export its model to libsensors4 (I have that
> information), but how do I go about it?
Good question. Hans de Goede and myself had a heated discussion about
this some times ago about his abituguru3 driver. He finally decided to
let the driver export the labels to user-space (files are names
temp1_label, fan1_label etc...) because it was easier for him that way
in the case of the Abit µGuru. My own preference would be to handle it
in user-space: get the DMI data for the system, and download the right
configuration file for the given system. However it essentially depends
on whether the DMI data is OK to pick the right configuration file, or
not. If the configuration depends on bits encoded in the chip itself
(as is the case of the µGuru) then in-kernel labelling is probably
better. Ultimately, what we want is to lower the maintenance workload.
> See, that's why I'd really like BUS_HOST. Unlike ISA which has port
> numbers, or PCI, which has PCI-IDs, BUS_HOST is suitable to model and
> version numbers as a way to differentiate various devices.
ISA port numbers and PCI IDs are unrelated. It's more PCI device number
and function which relate to ISA port numbers, in that they uniquely
identify a device within the running system (PCI IDs do not.) This is
what we use the address for in libsensors: unique device identification
in the system, so that you can have different configurations for two
similar devices in a given system. This problem doesn't exist for
devices like thinkpad_acpi, which are unique by nature, so there's no
problem to solve as far as I can see.
--
Jean Delvare
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [lm-sensors] [PATCH 1/4] libsensors4: Support more bus types,
2007-08-17 15:14 [lm-sensors] [PATCH 1/4] libsensors4: Support more bus types, part 1 Jean Delvare
` (7 preceding siblings ...)
2007-08-20 12:19 ` Jean Delvare
@ 2007-08-20 12:46 ` Henrique de Moraes Holschuh
2007-08-20 13:50 ` Jean Delvare
` (4 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-08-20 12:46 UTC (permalink / raw)
To: lm-sensors
On Mon, 20 Aug 2007, Jean Delvare wrote:
> On Mon, 20 Aug 2007 08:53:30 -0300, Henrique de Moraes Holschuh wrote:
> > On Sun, 19 Aug 2007, Henrique de Moraes Holschuh wrote:
> > > On Sun, 19 Aug 2007, Jean Delvare wrote:
> > > > Basically, two changes: add a "name" attribute to the device, and give
> > > > the platform device an instance number.
> > > >
> > > > The name attribute is needed and must be added to the thinkpad_acpi
> > > > driver now, otherwise it won't work with libsensors (neither current nor
> > > > libsensors4). This is where libsensors gets the "prefix" part of a
> > > > chip name.
> > >
> > > I will bake up a patch and send it for inclusion in 2.6.23-rc ASAP. If Len
> > > and Linus will take it (they should, it is an obviously safe thing that
> > > affects one driver only...).
> >
> > Ok, I have one tentative patch ready, but I better make sure I got it right
> > on the first try.
> >
> > First, is the "name" attribute documented anywhere? I may have to change
> > its contents later, if I break up thinkpad-acpi into various modules. Does
> > anything else other than libsensors4 uses it?
>
> The name attribute is also used by libsensors3 and pwmconfig.
Better document it in Documentation/hwmon/sysfs-interface, then... I'd do
it, but I don't know the specifics.
> > it anyway, at least it is future-proof. It is probably safer to defer
> > thinkpad-acpi libsensors4 support to 2.6.24 if the change is not accepted
> > for -rc4.
>
> I would aim at 2.6.24. As you can see there are still a number of
Agreed. Scheduled for 2.6.24, then.
> > Second, the libsensors configuration for thinkpad-acpi depends on the
> > thinkpad model. I *can* export its model to libsensors4 (I have that
> > information), but how do I go about it?
>
> Good question. Hans de Goede and myself had a heated discussion about
> this some times ago about his abituguru3 driver. He finally decided to
> let the driver export the labels to user-space (files are names
> temp1_label, fan1_label etc...) because it was easier for him that way
I was not really thinking about exporting labels, but rather to export
something that can be a match key in the libsensors config file.
> in the case of the Abit µGuru. My own preference would be to handle it
> in user-space: get the DMI data for the system, and download the right
> configuration file for the given system. However it essentially depends
Make it "get the DMI data and allow for AND matches in libsensors config",
and I will agree. And I wouldn't even have to do anything in the kernel,
since there is already support for it both in sysfs, and directly through
userspace.
That would allow one to match a config to "thinkpad-sensors-*" AND "ThinkPad
T43", for example. We even have a *good* template to use when constructing
such strings, see commit 4f5c791a850e5305a5b1b48d0e4b4de248dc96f9 (which is
already in 2.6.23).
> > See, that's why I'd really like BUS_HOST. Unlike ISA which has port
> > numbers, or PCI, which has PCI-IDs, BUS_HOST is suitable to model and
> > version numbers as a way to differentiate various devices.
>
> ISA port numbers and PCI IDs are unrelated. It's more PCI device number
I know. I was trying to convey the idea that what comes after the "name"
for BUS_HOST could be DMI data, instead of ISA port numbers (used in ISA
stuff) or PCI IDs (used in PCI stuff), or I2C IDs (used in I2C/smbus)...
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [lm-sensors] [PATCH 1/4] libsensors4: Support more bus types,
2007-08-17 15:14 [lm-sensors] [PATCH 1/4] libsensors4: Support more bus types, part 1 Jean Delvare
` (8 preceding siblings ...)
2007-08-20 12:46 ` Henrique de Moraes Holschuh
@ 2007-08-20 13:50 ` Jean Delvare
2007-08-20 14:38 ` Jean Delvare
` (3 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Jean Delvare @ 2007-08-20 13:50 UTC (permalink / raw)
To: lm-sensors
Hi Henrique,
On Sun, 19 Aug 2007 16:21:37 -0300, Henrique de Moraes Holschuh wrote:
> On Sun, 19 Aug 2007, Jean Delvare wrote:
> > Thanks for pointing this out, indeed we need to make sure that the new
> > libsensors will work properly with the thinkpad_acpi driver and other
> > similar drivers. However I do not think that we need a special bus type
> > for these. The thinkpad_acpi driver instantiates a platform device (as
> > you mentioned yourself), and platform drivers are covered by the "ISA"
> > bus type in libsensors (for historical reasons mainly.)
>
> This sounds a bit of an ugliness that might be best fixed, since it is a
> rare time window where one can actually do those. If it is not too painful,
> why not add BUS_HOST to attach stuff that is not on PCI/PCIe, nor ISA, nor
> I2C/SMBUS?
I agree that it's the right time to fix things, however I'm not sure
what problem we are trying to solve here. The only purpose of the bus
types in libsensors is to qualify the bus numbers and addresses that
follow them in a chip name (e.g. lm78-i2c-1-2d means the LM78 chip that
is connected to the I2C bus #1 and lives at address 0x2d). The whole
purpose of these chip names being to be able to have different
configurations for similar chips (for example one W83781D chip on the
motherboard connected to ISA and one W83781D chip on the graphics
adapter connected to I2C, or two LM90 chips on the motherboard's
SMBus). In the case of "host" devices such as thinkpad_acpi, the device
is unique almost by definition, so there is no need to differentiate.
Thus naming it thinkpad-isa-0000 or thinkpad-host-0000 or
thinkpad-foo-42 doesn't make any difference.
> I mean, it is weird enough with ACPI, but there is also on-die sensors in
> CPUs, and their ilk... and attaching those to BUS_ISA or BUS_PCI sounds like
> something out of an acid trip IMHO, never mind that it would work just fine
> (unlike most stuff that could come out of an acid trip, I suppose) :-)
The K8 thermal sensors are actually reported through a PCI device, so
k8temp-pci-00c3 is a perfectly correct name.
For Core thermal sensors, admittedly, the coretemp-isa-0000 name has no
physical sense. This is the exact same "problem" as for the thinkpad
driver, with the exception that there can actually be more than one
Core temperature sensor, so we use the "ISA address" to distinguish
between them.
> I'm glat to know it is only a mostly cosmetic thing, but still, if it is not
> too painful to fix...
Actually, this isn't trivial. To build a chip name, libsensors uses the
device's subsystem in sysfs. i2c -> i2c, pci -> pci, platform -> isa.
"Host" sensors are implemented as platform drivers, and this is the
reason why libsensors names them foo-isa-0000. So having libsensors
differentiate between host devices and ISA devices would require that
we add a brand new mechanism. We could, of course... if it was solving
a problem. BTW, most "ISA" hardware monitoring chips these days are
actually connected through the LPC bus and not the ISA bus, but we
have been reporting all of them as isa because it's more simple that
way.
--
Jean Delvare
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [lm-sensors] [PATCH 1/4] libsensors4: Support more bus types,
2007-08-17 15:14 [lm-sensors] [PATCH 1/4] libsensors4: Support more bus types, part 1 Jean Delvare
` (9 preceding siblings ...)
2007-08-20 13:50 ` Jean Delvare
@ 2007-08-20 14:38 ` Jean Delvare
2007-08-20 17:49 ` Henrique de Moraes Holschuh
` (2 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Jean Delvare @ 2007-08-20 14:38 UTC (permalink / raw)
To: lm-sensors
Hi Henrique,
On Mon, 20 Aug 2007 09:46:56 -0300, Henrique de Moraes Holschuh wrote:
> On Mon, 20 Aug 2007, Jean Delvare wrote:
> > On Mon, 20 Aug 2007 08:53:30 -0300, Henrique de Moraes Holschuh wrote:
> > > First, is the "name" attribute documented anywhere? I may have to change
> > > its contents later, if I break up thinkpad-acpi into various modules. Does
> > > anything else other than libsensors4 uses it?
> >
> > The name attribute is also used by libsensors3 and pwmconfig.
>
> Better document it in Documentation/hwmon/sysfs-interface, then... I'd do
> it, but I don't know the specifics.
Good idea. There was nothing to document originally because i2c devices
get this file automatically, and until recently almost all hardware
monitoring drivers were using (or abusing) i2c. Now that a good load of
hardware monitoring drivers are platform drivers, documenting the name
attribute makes a lot of sense. Patch follows.
> > > Second, the libsensors configuration for thinkpad-acpi depends on the
> > > thinkpad model. I *can* export its model to libsensors4 (I have that
> > > information), but how do I go about it?
> >
> > Good question. Hans de Goede and myself had a heated discussion about
> > this some times ago about his abituguru3 driver. He finally decided to
> > let the driver export the labels to user-space (files are names
> > temp1_label, fan1_label etc...) because it was easier for him that way
>
> I was not really thinking about exporting labels, but rather to export
> something that can be a match key in the libsensors config file.
Except that libsensors4 is totally chip-agnostic by design, and I'd
like it to stay that way. We already have two mechanisms to label the
inputs (label files in sysfs and sensors.conf), I'd rather not add a
3rd one.
> > in the case of the Abit µGuru. My own preference would be to handle it
> > in user-space: get the DMI data for the system, and download the right
> > configuration file for the given system. However it essentially depends
>
> Make it "get the DMI data and allow for AND matches in libsensors config",
> and I will agree. And I wouldn't even have to do anything in the kernel,
> since there is already support for it both in sysfs, and directly through
> userspace.
There's no need for AND matching as you described. The idea is to have
one specific configuration file per Thinkpad model. The setup to
support this isn't yet in place but people have been working on it.
I'll look into it more closely after lm-sensors 3.0.0 is released.
> That would allow one to match a config to "thinkpad-sensors-*" AND "ThinkPad
> T43", for example. We even have a *good* template to use when constructing
> such strings, see commit 4f5c791a850e5305a5b1b48d0e4b4de248dc96f9 (which is
> already in 2.6.23).
The mechanism introduced by this commit is useful to autoload drivers
based on DMI data. This isn't what we are discussing here.
> > > See, that's why I'd really like BUS_HOST. Unlike ISA which has port
> > > numbers, or PCI, which has PCI-IDs, BUS_HOST is suitable to model and
> > > version numbers as a way to differentiate various devices.
> >
> > ISA port numbers and PCI IDs are unrelated. It's more PCI device number
>
> I know. I was trying to convey the idea that what comes after the "name"
> for BUS_HOST could be DMI data, instead of ISA port numbers (used in ISA
> stuff) or PCI IDs (used in PCI stuff), or I2C IDs (used in I2C/smbus)...
And I was trying to explain why using the "address" field of
libsensors' chip name wasn't a good idea, because this field is there
to differentiate between similar chips on a given machine, and not to
identify a unique chip configuration. Using one field for different
purposes could quickly lead to confusion IMHO.
--
Jean Delvare
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [lm-sensors] [PATCH 1/4] libsensors4: Support more bus types,
2007-08-17 15:14 [lm-sensors] [PATCH 1/4] libsensors4: Support more bus types, part 1 Jean Delvare
` (10 preceding siblings ...)
2007-08-20 14:38 ` Jean Delvare
@ 2007-08-20 17:49 ` Henrique de Moraes Holschuh
2007-08-20 17:53 ` Henrique de Moraes Holschuh
2007-08-21 8:02 ` Jean Delvare
13 siblings, 0 replies; 15+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-08-20 17:49 UTC (permalink / raw)
To: lm-sensors
On Mon, 20 Aug 2007, Jean Delvare wrote:
> On Mon, 20 Aug 2007 09:46:56 -0300, Henrique de Moraes Holschuh wrote:
> > I was not really thinking about exporting labels, but rather to export
> > something that can be a match key in the libsensors config file.
>
> Except that libsensors4 is totally chip-agnostic by design, and I'd
> like it to stay that way. We already have two mechanisms to label the
> inputs (label files in sysfs and sensors.conf), I'd rather not add a
> 3rd one.
Well, as long as we can have different configurations for libsensors on
different thinkpad models, I don't really care :-)
> > > ISA port numbers and PCI IDs are unrelated. It's more PCI device number
> >
> > I know. I was trying to convey the idea that what comes after the "name"
> > for BUS_HOST could be DMI data, instead of ISA port numbers (used in ISA
> > stuff) or PCI IDs (used in PCI stuff), or I2C IDs (used in I2C/smbus)...
>
> And I was trying to explain why using the "address" field of
> libsensors' chip name wasn't a good idea, because this field is there
> to differentiate between similar chips on a given machine, and not to
> identify a unique chip configuration. Using one field for different
> purposes could quickly lead to confusion IMHO.
I see.
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [lm-sensors] [PATCH 1/4] libsensors4: Support more bus types,
2007-08-17 15:14 [lm-sensors] [PATCH 1/4] libsensors4: Support more bus types, part 1 Jean Delvare
` (11 preceding siblings ...)
2007-08-20 17:49 ` Henrique de Moraes Holschuh
@ 2007-08-20 17:53 ` Henrique de Moraes Holschuh
2007-08-21 8:02 ` Jean Delvare
13 siblings, 0 replies; 15+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-08-20 17:53 UTC (permalink / raw)
To: lm-sensors
On Mon, 20 Aug 2007, Jean Delvare wrote:
> For Core thermal sensors, admittedly, the coretemp-isa-0000 name has no
> physical sense. This is the exact same "problem" as for the thinkpad
> driver, with the exception that there can actually be more than one
> Core temperature sensor, so we use the "ISA address" to distinguish
> between them.
>
> > I'm glat to know it is only a mostly cosmetic thing, but still, if it is not
> > too painful to fix...
>
> Actually, this isn't trivial. To build a chip name, libsensors uses the
> device's subsystem in sysfs. i2c -> i2c, pci -> pci, platform -> isa.
> "Host" sensors are implemented as platform drivers, and this is the
> reason why libsensors names them foo-isa-0000. So having libsensors
Is there a good reason why we couldn't rename the "isa" bus to "platform",
then? It is far more correct to call every hwmon ISA device a platform
device, than to call every platform device an ISA device :p
> a problem. BTW, most "ISA" hardware monitoring chips these days are
> actually connected through the LPC bus and not the ISA bus, but we
> have been reporting all of them as isa because it's more simple that
> way.
It is just cosmetics, indeed. But still, it is the right time to ask about
it.
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [lm-sensors] [PATCH 1/4] libsensors4: Support more bus types,
2007-08-17 15:14 [lm-sensors] [PATCH 1/4] libsensors4: Support more bus types, part 1 Jean Delvare
` (12 preceding siblings ...)
2007-08-20 17:53 ` Henrique de Moraes Holschuh
@ 2007-08-21 8:02 ` Jean Delvare
13 siblings, 0 replies; 15+ messages in thread
From: Jean Delvare @ 2007-08-21 8:02 UTC (permalink / raw)
To: lm-sensors
Hi Henrique,
On Mon, 20 Aug 2007 14:53:08 -0300, Henrique de Moraes Holschuh wrote:
> Is there a good reason why we couldn't rename the "isa" bus to "platform",
> then? It is far more correct to call every hwmon ISA device a platform
> device, than to call every platform device an ISA device :p
Historical reasons, maybe. Users may be using the chip name filtering
feature of sensors(1) in scripts for example, renaming "isa" to
"platform" would break these for ISA/LPC chips. So the change would
make the upgrade path slightly more difficult.
I don't know if this reason is good enough to not do it, though. I
wonder what other developers and users think about the idea? Anyone
wants to argue in one direction or the other? This is the right time.
--
Jean Delvare
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 15+ messages in thread