On Sat, Nov 21, 2015 at 11:26 PM, Jonathan Cameron <jic23@xxxxxxxxxx> wrote: > On 20/11/15 16:40, Peter Meerwald-Stadler wrote: >> On Wed, 18 Nov 2015, Matt Ranostay wrote: >> >>> MAX30100 is an heart rate and pulse oximeter sensor that works using >>> two LEDS of different wavelengths, and detecting the light reflected >>> back. >>> >>> This patchset adds support for both IR and RED LED channels which can >> >> red? this refers to a LED with visible red color? >> more nitpicking below > As I'm here, I'll answer that one. Yes. Absolutely, it's 700->800nm light > which pretty much makes it mostly visible red. Yeah this device uses a "red" light (aka close enough to wavelength) mainly for the oxygen reading. >> >>> be processed in userspace to determine heart rate and blood oxygen >>> levels. >>> >>> Signed-off-by: Matt Ranostay <mranostay@xxxxxxxxx> > Few additional comments from me. Coming along nicely. > > Guessing we want to hold this until you have real hardware tested with > the final version anyway. Should be plenty of time. > > Jonathan >>> --- >>> .../devicetree/bindings/iio/light/max30100.txt | 22 ++ >>> drivers/iio/Kconfig | 1 + >>> drivers/iio/Makefile | 1 + >>> drivers/iio/health/Kconfig | 21 + >>> drivers/iio/health/Makefile | 7 + >>> drivers/iio/health/max30100.c | 423 +++++++++++++++++++++ >>> 6 files changed, 475 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/iio/light/max30100.txt >>> create mode 100644 drivers/iio/health/Kconfig >>> create mode 100644 drivers/iio/health/Makefile >>> create mode 100644 drivers/iio/health/max30100.c >>> >>> diff --git a/Documentation/devicetree/bindings/iio/light/max30100.txt b/Documentation/devicetree/bindings/iio/light/max30100.txt >>> new file mode 100644 >>> index 0000000..435d6fc >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/iio/light/max30100.txt >>> @@ -0,0 +1,22 @@ >>> +* Maxim MAX30100 heart rate and pulse oximeter sensor >>> + >>> +https://datasheets.maximintegrated.com/en/ds/MAX30100.pdf >>> + >>> +Required properties: >>> + >>> + - compatible: must be "maxim,max30100" >>> + - reg: the I2c address of the sensor >> >> I2C >> >>> + - interrupt-parent: should be the phandle for the interrupt controller >>> + - interrupts : the sole interrupt generated by the device >> >> no space before : for consistency >> >>> + >>> + Refer to interrupt-controller/interrupts.txt for generic interrupt client >>> + node bindings. >>> + >>> +Example: >>> + >>> +max30100@57 { >>> + compatible = "maxim,max30100"; >>> + reg = <0x57>; >>> + interrupt-parent = <&gpio1>; >>> + interrupts = <16 2>; >>> +}; >>> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig >>> index 5dfb4f1..b846873 100644 >>> --- a/drivers/iio/Kconfig >>> +++ b/drivers/iio/Kconfig >>> @@ -68,6 +68,7 @@ source "drivers/iio/common/Kconfig" >>> source "drivers/iio/dac/Kconfig" >>> source "drivers/iio/frequency/Kconfig" >>> source "drivers/iio/gyro/Kconfig" >>> +source "drivers/iio/health/Kconfig" >>> source "drivers/iio/humidity/Kconfig" >>> source "drivers/iio/imu/Kconfig" >>> source "drivers/iio/light/Kconfig" >>> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile >>> index dc73c1f..7371791 100644 >>> --- a/drivers/iio/Makefile >>> +++ b/drivers/iio/Makefile >>> @@ -20,6 +20,7 @@ obj-y += common/ >>> obj-y += dac/ >>> obj-y += gyro/ >>> obj-y += frequency/ >>> +obj-y += health/ >>> obj-y += humidity/ >>> obj-y += imu/ >>> obj-y += light/ >>> diff --git a/drivers/iio/health/Kconfig b/drivers/iio/health/Kconfig >>> new file mode 100644 >>> index 0000000..30d4ca0 >>> --- /dev/null >>> +++ b/drivers/iio/health/Kconfig >>> @@ -0,0 +1,21 @@ >>> +# >>> +# Health sensors >>> +# >>> +# When adding new entries keep the list in alphabetical order >>> + >>> +menu "Health sensors" >>> + >>> +config MAX30100 >>> + tristate "MAX30100 heart rate and pulse oximeter sensor" >>> + depends on I2C >>> + select REGMAP_I2C >>> + select IIO_BUFFER >>> + select IIO_KFIFO_BUF >>> + help >>> + Say Y here to build I2C interface support for the Maxim >>> + MAX30100 heart rate, and pulse oximeter sensor. >>> + >>> + To compile this driver as a module, choose M here: the >>> + module will be called max30100 >> >> put a full stop (.) at the end of the sentence maybe :) >> >>> + >>> +endmenu >>> diff --git a/drivers/iio/health/Makefile b/drivers/iio/health/Makefile >>> new file mode 100644 >>> index 0000000..7c475d7 >>> --- /dev/null >>> +++ b/drivers/iio/health/Makefile >>> @@ -0,0 +1,7 @@ >>> +# >>> +# Makefile for IIO Health sensors >>> +# >>> + >>> +# When adding new entries keep the list in alphabetical order >>> + >>> +obj-$(CONFIG_MAX30100) += max30100.o >>> diff --git a/drivers/iio/health/max30100.c b/drivers/iio/health/max30100.c >>> new file mode 100644 >>> index 0000000..3ad6b31 >>> --- /dev/null >>> +++ b/drivers/iio/health/max30100.c >>> @@ -0,0 +1,423 @@ >>> +/* >>> + * max30100.c - Support for MAX30100 heart rate and pulse oximeter sensor >>> + * >>> + * Copyright (C) 2015 Matt Ranostay <mranostay@xxxxxxxxx> >>> + * >>> + * 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 >>> + * the Free Software Foundation; either version 2 of the License, or >>> + * (at your option) any later version. >>> + * >>> + * This program is distributed in the hope that it will be useful, >>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>> + * GNU General Public License for more details. >>> + * >>> + */ >>> + >>> +#include <linux/module.h> >>> +#include <linux/init.h> >>> +#include <linux/interrupt.h> >>> +#include <linux/delay.h> >>> +#include <linux/err.h> >>> +#include <linux/irq.h> >>> +#include <linux/i2c.h> >>> +#include <linux/mutex.h> >>> +#include <linux/regmap.h> >>> +#include <linux/iio/iio.h> >>> +#include <linux/iio/buffer.h> >>> +#include <linux/iio/kfifo_buf.h> >>> + >>> +#define MAX30100_REGMAP_NAME "max30100_regmap" >>> +#define MAX30100_DRV_NAME "max30100" >>> + >>> +#define MAX30100_REG_INT_STATUS 0x00 >>> +#define MAX30100_REG_INT_STATUS_PWR_RDY BIT(0) >>> +#define MAX30100_REG_INT_STATUS_SPO2_RDY BIT(4) >>> +#define MAX30100_REG_INT_STATUS_HR_RDY BIT(5) >>> + >>> +#define MAX30100_REG_INT_ENABLE 0x01 >>> +#define MAX30100_REG_INT_ENABLE_HR_EN BIT(0) >>> +#define MAX30100_REG_INT_ENABLE_SPO2_EN BIT(1) >>> +#define MAX30100_REG_INT_ENABLE_MASK 0xf0 >>> +#define MAX30100_REG_INT_ENABLE_MASK_SHIFT 4 >>> + >>> +#define MAX30100_REG_FIFO_DATA 0x05 >>> + >>> +#define MAX30100_REG_MODE_CONFIG 0x06 >>> +#define MAX30100_REG_MODE_CONFIG_MODE_HR_EN BIT(0) >>> +#define MAX30100_REG_MODE_CONFIG_MODE_SPO2_EN BIT(1) >>> +#define MAX30100_REG_MODE_CONFIG_MODE_MASK 0x03 >>> +#define MAX30100_REG_MODE_CONFIG_TEMP_EN BIT(3) >>> +#define MAX30100_REG_MODE_CONFIG_PWR BIT(7) >>> + >>> +#define MAX30100_REG_SPO2_CONFIG 0x07 >>> +#define MAX30100_REG_SPO2_CONFIG_HI_RES_EN BIT(6) >>> + >>> +#define MAX30100_REG_SPO2_CONFIG_100HZ BIT(2) >>> +#define MAX30100_REG_SPO2_CONFIG_1600US 0x3 >>> + >>> +#define MAX30100_REG_LED_CONFIG 0x09 >>> +#define MAX30100_REG_LED_CONFIG_RED_LED_SHIFT 4 >>> + >>> +#define MAX30100_REG_LED_CONFIG_24MA 0x07 >>> +#define MAX30100_REG_LED_CONFIG_50MA 0x0f >>> + >>> +#define MAX30100_REG_TEMP_INTEGER 0x16 >>> +#define MAX30100_REG_TEMP_FRACTION 0x17 >>> + >>> +struct max30100_data { >>> + struct iio_dev *indio_dev; > This is only used in one place and there is a perfectly good pointer to > this in the interrupt anyway so drop it from here. (note this is a > common one to spot, there is very rarely a good reason to have a pointer > to the struct iio_dev in the private data. Everything should be the other > way around!) Ah right :) > > >>> + struct mutex lock; >>> + struct regmap *regmap; >>> + >>> + u8 buffer[4]; /* 4 8-bit channels */ >>> +}; >>> + >>> +static bool max30100_is_volatile_reg(struct device *dev, unsigned int reg) >>> +{ >>> + switch (reg) { >>> + case MAX30100_REG_INT_STATUS: >>> + case MAX30100_REG_MODE_CONFIG: >>> + case MAX30100_REG_FIFO_DATA: >>> + case MAX30100_REG_TEMP_INTEGER: >>> + case MAX30100_REG_TEMP_FRACTION: >>> + return true; >>> + default: >>> + return false; >>> + } >>> +} >>> + >>> +static const struct regmap_config max30100_regmap_config = { >>> + .name = MAX30100_REGMAP_NAME, >>> + >>> + .reg_bits = 8, >>> + .val_bits = 8, >>> + >>> + .max_register = MAX30100_REG_LED_CONFIG, >>> + .cache_type = REGCACHE_FLAT, >>> + >>> + .volatile_reg = max30100_is_volatile_reg, >>> +}; >>> + >>> +static const unsigned long max30100_scan_masks[] = {0x3, 0}; >>> + >>> +static const struct iio_chan_spec max30100_channels[] = { >>> + { >>> + .type = IIO_INTENSITY, >>> + .channel2 = IIO_MOD_LIGHT_IR, >>> + >>> + .scan_index = 0, >>> + .scan_type = { >>> + .sign = 'u', >>> + .realbits = 16, >>> + .storagebits = 16, >>> + .endianness = IIO_BE, >>> + }, >>> + }, >>> + { >>> + .type = IIO_INTENSITY, >>> + .channel2 = IIO_MOD_LIGHT_RED, >>> + >>> + .scan_index = 1, >>> + .scan_type = { >>> + .sign = 'u', >>> + .realbits = 16, >> >> max30100_interrupt_handler() reads 4 bytes?! >> >> description of buffer in max30100_data: >> u8 buffer[4]; /* 4 8-bit channels */ >> this is confusing; maybe __be16 buffer[2]? > Agreed, __be16 version preferred. Makes it explicit > what we have and fix the description (good spot Peter!) >> >>> + .storagebits = 16, >>> + .endianness = IIO_BE, >>> + }, >>> + }, >>> + { >>> + .type = IIO_TEMP, >>> + .info_mask_separate = >>> + BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE), >>> + .scan_index = -1, >>> + }, >>> +}; >>> + >>> +static int max30100_set_powermode(struct max30100_data *data, bool state) >>> +{ >>> + return regmap_update_bits(data->regmap, MAX30100_REG_MODE_CONFIG, >>> + MAX30100_REG_MODE_CONFIG_PWR, >>> + state ? 0 : MAX30100_REG_MODE_CONFIG_PWR); >>> +} >>> + >>> +static int max30100_buffer_postenable(struct iio_dev *indio_dev) >>> +{ >>> + struct max30100_data *data = iio_priv(indio_dev); >>> + >>> + return max30100_set_powermode(data, true); >>> +} >>> + >>> +static int max30100_buffer_predisable(struct iio_dev *indio_dev) >>> +{ >>> + struct max30100_data *data = iio_priv(indio_dev); >>> + >>> + return max30100_set_powermode(data, false); >>> +} >>> + >>> +static const struct iio_buffer_setup_ops max30100_buffer_setup_ops = { >>> + .postenable = max30100_buffer_postenable, >>> + .predisable = max30100_buffer_predisable, >>> +}; >>> + >>> +static inline int max30100_fifo_is_empty(struct max30100_data *data) >>> +{ >>> + int ret, val; >>> + >>> + ret = regmap_read(data->regmap, MAX30100_REG_INT_STATUS, &val); >>> + if (ret) >>> + return ret; >>> + >>> + return !!(val >> 4); > You don't really need the !! as you only check it's != 0 later... > I guess you could argue this makes it obvious it is a boolean though... >>> +} >>> + >>> +static irqreturn_t max30100_interrupt_handler(int irq, void *private) >>> +{ >>> + struct iio_dev *indio_dev = private; >>> + struct max30100_data *data = iio_priv(indio_dev); >>> + int ret; >>> + >>> + mutex_lock(&data->lock); >>> + >>> + while (max30100_fifo_is_empty(data) != 0) { >>> + ret = regmap_bulk_read(data->regmap, MAX30100_REG_FIFO_DATA, >>> + &data->buffer, 4); >>> + >>> + if (ret) >>> + goto err_read; >>> + > use the local copy (ultimately private) instead of the version in data. >>> + iio_push_to_buffers(data->indio_dev, data->buffer); >>> + } >>> + >>> +err_read: >>> + mutex_unlock(&data->lock); >>> + >>> + return IRQ_HANDLED; >>> +} >>> + >>> +static int max30100_chip_init(struct max30100_data *data) >>> +{ >>> + int ret; >>> + >>> + /* RED IR LED = 24mA, IR LED = 50mA */ >>> + ret = regmap_write(data->regmap, MAX30100_REG_LED_CONFIG, >>> + (MAX30100_REG_LED_CONFIG_24MA << >>> + MAX30100_REG_LED_CONFIG_RED_LED_SHIFT) | >>> + MAX30100_REG_LED_CONFIG_50MA); >>> + if (ret) >>> + return ret; >>> + >>> + /* enable hi-res SPO2 readings with 100 samples a second */ >>> + ret = regmap_write(data->regmap, MAX30100_REG_SPO2_CONFIG, >>> + MAX30100_REG_SPO2_CONFIG_HI_RES_EN | >>> + MAX30100_REG_SPO2_CONFIG_100HZ | >>> + MAX30100_REG_SPO2_CONFIG_1600US); >>> + if (ret) >>> + return ret; >>> + >>> + /* enable SPO2 + HR mode */ >>> + ret = regmap_update_bits(data->regmap, MAX30100_REG_MODE_CONFIG, >>> + MAX30100_REG_MODE_CONFIG_MODE_MASK, >>> + MAX30100_REG_MODE_CONFIG_MODE_HR_EN | >>> + MAX30100_REG_MODE_CONFIG_MODE_SPO2_EN); >>> + if (ret) >>> + return ret; >>> + >>> + /* enable SPO2 + HR interrupts */ >>> + return regmap_update_bits(data->regmap, MAX30100_REG_INT_ENABLE, >>> + MAX30100_REG_INT_ENABLE_MASK, >>> + (MAX30100_REG_INT_ENABLE_HR_EN | >>> + MAX30100_REG_INT_ENABLE_SPO2_EN) >>> + << MAX30100_REG_INT_ENABLE_MASK_SHIFT); >>> +} >>> + >>> +static int max30100_read_temp(struct max30100_data *data, int *val) >>> +{ >>> + int ret; >>> + int reg; >>> + >>> + ret = regmap_read(data->regmap, MAX30100_REG_TEMP_INTEGER, ®); >>> + if (ret < 0) >>> + return ret; >>> + *val = reg << 4; >> >> note that this is a signed value; I think negative temperatures are not >> processed correctly > Definitely looks like it's missing a sign extend to me as well! D'oh! Yeah you are correct! >> >>> + >>> + ret = regmap_read(data->regmap, MAX30100_REG_TEMP_FRACTION, ®); >>> + if (ret < 0) >>> + return ret; >>> + *val |= reg & 0xf; >>> + >>> + return 0; >>> +} >>> + >>> +static int max30100_get_temp(struct max30100_data *data, int *val) >>> +{ >>> + int ret; >>> + int tries = 3; >>> + >>> + /* start acquisition */ >>> + ret = regmap_update_bits(data->regmap, MAX30100_REG_MODE_CONFIG, >>> + MAX30100_REG_MODE_CONFIG_TEMP_EN, >>> + MAX30100_REG_MODE_CONFIG_TEMP_EN); >>> + if (ret) >>> + return ret; >>> + >>> + while (tries--) { >>> + int reg; > Could use the interrupt that indicates temperature is ready... > > Why have you chosen not to? (might well be sensible but you want > to have documentation somewhere here to explain why.) > > I guess it's because it is likely to be an occasional out of band reading > and you don't want to complicate the interrupt handling? Yeah I didn't want to complicate the interrupt handler. Pretty much the only reason I did it that way... Should I document that in a comment? >>> + >>> + usleep_range(29000, 40000); >>> + >>> + ret = regmap_read(data->regmap, MAX30100_REG_MODE_CONFIG, ®); >>> + if (ret < 0) >>> + break; >>> + >>> + /* bit reset to zero when reading is ready */ >>> + if (!(reg & MAX30100_REG_MODE_CONFIG_TEMP_EN)) { >>> + ret = max30100_read_temp(data, val); >>> + break; >>> + } >>> + ret = -EIO; >>> + } >>> + >>> + return ret; >>> +} >>> + >>> +static int max30100_read_raw(struct iio_dev *indio_dev, >>> + struct iio_chan_spec const *chan, >>> + int *val, int *val2, long mask) >>> +{ >>> + struct max30100_data *data = iio_priv(indio_dev); >>> + int ret = -EINVAL; >>> + >>> + mutex_lock(&indio_dev->mlock); >>> + >>> + /* temperature reading can only be acquired while engine is running */ >>> + if (!iio_buffer_enabled(indio_dev) && mask == IIO_CHAN_INFO_RAW) { >>> + ret = -EAGAIN; > Possibly -EBUSY. EAGAIN tends to lead to usespace software immediately > trying to read again whereas EBUSY tends to imply there is a 'reason' beyond > just 'not now'. > > Not a particularly clear cut case however. > > Reorder this to be under the switch (mask) case IIO_CHAN_INFO_RAW though > and the code flow will be simpler as you won't need to check we are trying > a raw read twice. Got it! > >>> + goto error_again; >>> + } >>> + >>> + switch (mask) { >>> + case IIO_CHAN_INFO_RAW: >>> + ret = max30100_get_temp(data, val); >>> + if (!ret) >> >> what if ret < 0? >> _get_temp() checks for ret < 0 >> >> INTENSITY channels cannot be raw_read? > It's not in the channel spec, so Matt's right here. However > given this is 'unusual' it might act as a form of 'documentation' > to explicitly verify we have a temperature channel here. > > (Peter, for why they have no _raw access see the previous version - > basically there is no point in getting a single reading on this sort > of device - it's the processing of the waveform that allows you to > establish what the device is meant to be measuring. Makes sense to > me). >> >>> + ret = IIO_VAL_INT; >>> + break; >>> + case IIO_CHAN_INFO_SCALE: >>> + *val = 1; /* 0.0625 */ >>> + *val2 = 16; >>> + ret = IIO_VAL_FRACTIONAL; >>> + break; >>> + } >>> + >>> +error_again: >>> + mutex_unlock(&indio_dev->mlock); >>> + >>> + return ret; >>> +} >>> + >>> +static const struct iio_info max30100_info = { >>> + .driver_module = THIS_MODULE, >>> + .read_raw = max30100_read_raw, >>> +}; >>> + >>> +static int max30100_probe(struct i2c_client *client, >>> + const struct i2c_device_id *id) >>> +{ >>> + struct max30100_data *data; >>> + struct iio_buffer *buffer; >>> + struct iio_dev *indio_dev; >>> + int ret; >>> + >>> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data)); >>> + if (!indio_dev) >>> + return -ENOMEM; >>> + >>> + buffer = devm_iio_kfifo_allocate(&client->dev); >>> + if (!buffer) >>> + return -ENOMEM; >>> + >>> + iio_device_attach_buffer(indio_dev, buffer); >>> + >>> + indio_dev->name = MAX30100_DRV_NAME; >>> + indio_dev->channels = max30100_channels; >>> + indio_dev->info = &max30100_info; >>> + indio_dev->num_channels = ARRAY_SIZE(max30100_channels); >>> + indio_dev->available_scan_masks = max30100_scan_masks; >>> + indio_dev->modes = (INDIO_BUFFER_SOFTWARE | INDIO_DIRECT_MODE); >>> + indio_dev->setup_ops = &max30100_buffer_setup_ops; >>> + >>> + data = iio_priv(indio_dev); >>> + data->indio_dev = indio_dev; >>> + >>> + mutex_init(&data->lock); >>> + i2c_set_clientdata(client, indio_dev); >>> + >>> + data->regmap = devm_regmap_init_i2c(client, &max30100_regmap_config); >>> + if (IS_ERR(data->regmap)) { >>> + dev_err(&client->dev, "regmap initialization failed.\n"); >>> + return PTR_ERR(data->regmap); >>> + } >>> + max30100_set_powermode(data, false); >>> + >>> + ret = max30100_chip_init(data); >>> + if (ret) >>> + goto error_out; >>> + >>> + if (client->irq <= 0) { >>> + dev_err(&client->dev, "no valid irq defined\n"); >>> + ret = -EINVAL; >>> + goto error_out; >>> + } >>> + ret = devm_request_threaded_irq(&client->dev, client->irq, >>> + NULL, max30100_interrupt_handler, >>> + IRQF_TRIGGER_FALLING | IRQF_ONESHOT, >>> + "max30100_irq", indio_dev); >>> + if (ret) { >>> + dev_err(&client->dev, "request irq (%d) failed\n", client->irq); >>> + goto error_out; >>> + } >>> + >>> + return iio_device_register(indio_dev); >> >> newline here >> >>> +error_out: >> >> drop newline >> >>> + >>> + return ret; >>> +} >>> + >>> +static int max30100_remove(struct i2c_client *client) >>> +{ >>> + struct iio_dev *indio_dev = i2c_get_clientdata(client); >>> + struct max30100_data *data = iio_priv(indio_dev); >>> + >>> + iio_device_unregister(indio_dev); >>> + max30100_set_powermode(data, false); >>> + >>> + return 0; >>> +} >>> + >>> +static const struct i2c_device_id max30100_id[] = { >>> + { "max30100", 0 }, >>> + {} >>> +}; >>> +MODULE_DEVICE_TABLE(i2c, max30100_id); >>> + >>> +static const struct of_device_id max30100_dt_ids[] = { >>> + { .compatible = "maxim,max30100" }, >>> + { } >>> +}; >>> +MODULE_DEVICE_TABLE(of, max30100_dt_ids); >>> + >>> +static struct i2c_driver max30100_driver = { >>> + .driver = { >>> + .name = MAX30100_DRV_NAME, >>> + .of_match_table = of_match_ptr(max30100_dt_ids), >>> + }, >>> + .probe = max30100_probe, >>> + .remove = max30100_remove, >>> + .id_table = max30100_id, >>> +}; >>> +module_i2c_driver(max30100_driver); >>> + >>> +MODULE_AUTHOR("Matt Ranostay <mranostay@xxxxxxxxx>"); >>> +MODULE_DESCRIPTION("MAX30100 heart rate and pulse oximeter sensor"); >>> +MODULE_LICENSE("GPL"); >>> >> > -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html