Hi Jonathan,
On 04/12/2011 19:08, Jonathan Cameron wrote:
> Mostly fine, though I'd structure a few corners differently as explained
> inline.
>
> Also, I don't immediately see a reason not to use threaded interrupts
> for much
> of what we have here and that would definitely be preferable if possible!
>
> Review may be more thorough that you wanted at this stage, but given I was
> reading the code I thought I might as well do it along the way!
Thanks :)
> On 12/02/2011 03:35 PM, Maxime Ripard wrote:
>> Signed-off-by: Maxime Ripard <maxime.ripard@xxxxxxxxxxxxxxxxxx>
>>
>> Cc: Patrice Vilchez <patrice.vilchez@xxxxxxxxx>
>> Cc: Thomas Petazzoni <thomas.petazzoni@xxxxxxxxxxxxxxxxxx>
>> Cc: Nicolas Ferre <nicolas.ferre@xxxxxxxxx>
>> ---
>> arch/arm/mach-at91/at91sam9260_devices.c | 3 +
>> arch/arm/mach-at91/board-sam9g20ek.c | 1 +
>> drivers/staging/iio/adc/Kconfig | 11 +-
>> drivers/staging/iio/adc/at91_adc.c | 329 +++++++++++++++++++++++++++++-
>> include/linux/platform_data/at91_adc.h | 2 +
>> 5 files changed, 331 insertions(+), 15 deletions(-)
>>
>> diff --git a/arch/arm/mach-at91/at91sam9260_devices.c b/arch/arm/mach-at91/at91sam9260_devices.c
>> index 95b3127..9dc9f96 100644
>> --- a/arch/arm/mach-at91/at91sam9260_devices.c
>> +++ b/arch/arm/mach-at91/at91sam9260_devices.c
>> @@ -1366,6 +1366,9 @@ void __init at91_add_device_adc(struct at91_adc_data *data)
>> if (test_bit(3, &data->channels_used))
>> at91_set_A_periph(AT91_PIN_PC3, 0);
>>
>> + if (data->use_external)
>> + at91_set_A_periph(AT91_PIN_PA22, 0);
>> +
>> adc_data = *data;
>> platform_device_register(&at91_adc_device);
>> }
>> diff --git a/arch/arm/mach-at91/board-sam9g20ek.c b/arch/arm/mach-at91/board-sam9g20ek.c
>> index 7758f34..f18f6f2 100644
>> --- a/arch/arm/mach-at91/board-sam9g20ek.c
>> +++ b/arch/arm/mach-at91/board-sam9g20ek.c
>> @@ -317,6 +317,7 @@ static void __init ek_add_device_buttons(void) {}
>>
>> static struct at91_adc_data ek_adc_data = {
>> .channels_used = BIT(0) | BIT(1) | BIT(2) | BIT(3),
>> + .use_external = true,
>> .vref = 3300,
>> };
>>
>> diff --git a/drivers/staging/iio/adc/Kconfig b/drivers/staging/iio/adc/Kconfig
>> index 93de64d..9d87c75 100644
>> --- a/drivers/staging/iio/adc/Kconfig
>> +++ b/drivers/staging/iio/adc/Kconfig
>> @@ -170,10 +170,13 @@ config AD7280
>> module will be called ad7280a
>>
>> config AT91_ADC
>> - tristate "Atmel AT91 ADC"
>> - depends on SYSFS && ARCH_AT91
>> - help
>> - Say yes here to build support for Atmel AT91 ADC
>> + tristate "Atmel AT91 ADC"
>> + depends on SYSFS && ARCH_AT91
>> + select IIO_BUFFER
>> + select IIO_SW_RING
>> + select IIO_TRIGGER
> I assume this is true because some of these changes mean
> that it will simply not function without the buffered side?
> Given that one typical usecase for this device is simple
> hwmon type monitoring (fully pull interface in kernel)
> I'd be inclined to see how much extra code would be
> required to allow it to run as before adding buffered support.
> If it is really uggly then fair enough!
I've tried to do that, and found it quite ugly, but I'll resubmit a new
version with this and see how you find it :)
>> + help
>> + Say yes here to build support for Atmel AT91 ADC.
>>
>> config MAX1363
>> tristate "Maxim max1363 ADC driver"
>> diff --git a/drivers/staging/iio/adc/at91_adc.c b/drivers/staging/iio/adc/at91_adc.c
>> index b357363..74baaf3 100644
>> --- a/drivers/staging/iio/adc/at91_adc.c
>> +++ b/drivers/staging/iio/adc/at91_adc.c
>> @@ -23,9 +23,20 @@
>> #include "../iio.h"
>> #include <linux/platform_data/at91_adc.h>
>>
>> +#include "../buffer.h"
>> +#include "../ring_sw.h"
>> +#include "../trigger.h"
>> +#include "../trigger_consumer.h"
>> +
>> #include <mach/at91_adc.h>
>> #include <mach/cpu.h>
>>
> I'd like to see some docs for this. Obviously
> name is self explanatory but value isn't!
You're right. The value here is the value that has to be written in the
mode register of the ADC. From one board to another, as triggers change,
so does the values to put in the register to enable this particular trigger.
>> +struct at91_adc_trigger {
>> + char *name;
>> + u8 value;
>> + bool is_external;
>> +};
>> +
>> struct at91_adc_desc {
>> /* ADC Clock as specified by the datasheet, in Hz. */
>> u32 clock;
>> @@ -37,6 +48,7 @@ struct at91_adc_desc {
>> u8 num_channels;
>> /* Startup time of the ADC, in microseconds. */
>> u8 startup_time;
>> + struct at91_adc_trigger *triggers;
>> };
>>
>> struct at91_adc_state {
>> @@ -51,12 +63,37 @@ struct at91_adc_state {
>> unsigned long channels_mask;
>> bool irq_enabled;
>> struct at91_adc_desc *desc;
>> + struct iio_trigger **trig;
>> +};
>> +
> Trigger names are a little cryptic. I'd be inclined to make them
> a bit longer and hence perhaps self explanatory if you can. Note
> as first device of this type you are pretty much setting the
> naming standard and I think these will appear in some of the user
> space abi, so they won't be easy to change later!
> For that reason we are going to be fussier than normal.
You're right to be. Indeed, the names aren't that explicit. Would
something like "timer-counter-0" be better ?
>
>> +static struct at91_adc_trigger at91_adc_trigger_sam9g20[] = {
>> + [0] = {
>> + .name = "TC0",
>> + .value = AT91_ADC_TRGSEL_TC0 | AT91_ADC_TRGEN,
>> + .is_external = false,
>> + },
>> + [1] = {
>> + .name = "TC1",
>> + .value = AT91_ADC_TRGSEL_TC1 | AT91_ADC_TRGEN,
>> + .is_external = false,
>> + },
>> + [2] = {
>> + .name = "TC2",
>> + .value = AT91_ADC_TRGSEL_TC2 | AT91_ADC_TRGEN,
>> + .is_external = false,
>> + },
>> + [3] = {
>> + .name = "external",
>> + .value = AT91_ADC_TRGSEL_EXTERNAL | AT91_ADC_TRGEN,
>> + .is_external = true,
>> + },
> How does it know how many there are? Null terminate this perhaps?
>> };
>>
>> static struct at91_adc_desc at91_adc_desc_sam9g20 = {
>> .clock = 5000000,
>> .num_channels = 4,
>> .startup_time = 10,
>> + .triggers = at91_adc_trigger_sam9g20,
>> };
>>
>> static int at91_adc_select_soc(struct at91_adc_state *st)
>> @@ -82,27 +119,124 @@ static inline void at91_adc_reg_write(struct at91_adc_state *st,
>> writel_relaxed(val, st->reg_base + reg);
>> }
>>
>> +static irqreturn_t at91_adc_trigger_handler(int irq, void *p)
>> +{
>> + struct iio_poll_func *pf = p;
>> + struct iio_dev *idev = pf->indio_dev;
>> + struct at91_adc_state *st = iio_priv(idev);
>> + struct iio_buffer *buffer = idev->buffer;
>> + int len = 0;
>> +
>> + size_t datasize = buffer->access->get_bytes_per_datum(buffer);
>> + u16 *data = kmalloc(datasize, GFP_KERNEL);
>> + if (data == NULL)
>> + return -ENOMEM;
> I know some of my drivers have done this, but I prefer the approach
> used by some of the ADI ones. They allocate these buffers in the
> chip_state and hence get this kmalloc out of this fast patch.
> That should be fine as we should always be able to make sure it
> is the right size in the preenable function for the buffer.
Ok
>> +
>> + /* Needed to ACK the DRDY interruption */
>> + at91_adc_reg_read(st, AT91_ADC_LCDR);
> Here? Conceptually I'd expect to see this right at the end of the interrupt
> handling. I would imaging acking this means another one can arive. To
> cleanly support capture from other devices on this trigger this would occur
> in the try_reenable callback. That only gets called when the number of
> calls to iio_trigger_notify_done is equal to the number of devices waiting
> on the trigger (i.e. everyone is done).
Ok, so try_reenable should contain both the interrupt ACK and the
re-enabling of the irq ?
>> +
>> + if (buffer->scan_count) {
>> + int i, j;
>> + for (i = 0, j = 0; i < buffer->scan_count; i++) {
> Just as a heads up, this patch will change shortly due to the
> introduction of multiple buffer support so please watch those
> patches as they go through and the driver changes we are making..
>
> Also, this looks like a for_each_set_bit function would make this easier to
> read.
>
>> + j = find_next_bit(buffer->scan_mask,
>> + st->desc->num_channels,
>> + j);
>> + data[i] = at91_adc_reg_read(st, AT91_ADC_CHR(j));
>> + j++;
>> + len += 2;
>> + }
>> + }
>> +
> The utility of ALIGN macro has been pointed out to me recently and makes
> it more apparent what is going on in cases like this.
Oh, didn't know about it either, I will take a look.
> Also, conceptually I'd expect to see the timestamp capture occuring earlier
> to make it nearer to the trigger point (when I guess the read is actually
> occuring - having been lazy and not actually loaded the datasheet ;)
Ok, so basically, in the pollfunc allocation, I would move
at91_adc_trigger_handler to top half, and put iio_pollfunc_store_time as
bottom half ?
>> + if (buffer->scan_timestamp) {
>> + s64 *timestamp = (s64 *)((u8 *)data + round_up(len,
>> + sizeof(s64)));
>> + *timestamp = iio_get_time_ns();
>> + }
>> +
>> + buffer->access->store_to(buffer, (u8 *)data, pf->timestamp);
>> +
>> + kfree(data);
>> +
>> + iio_trigger_notify_done(idev->trig);
>> + st->irq_enabled = true;
>
>> + enable_irq(st->irq);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> static irqreturn_t at91_adc_eoc_trigger(int irq, void *private)
>> {
>> struct iio_dev *idev = private;
>> struct at91_adc_state *st = iio_priv(idev);
>> - unsigned int status = at91_adc_reg_read(st, AT91_ADC_SR);
>>
>> - if (!(status & AT91_ADC_DRDY))
>> - return IRQ_HANDLED;
>> + st->done = true;
> Want to wake this up here? Not after the st->last_value bit below?
> Obviously probably doesn't actually matter is we are in an irq handler
> but it would make for more logical flow to my mind. Data is read first
> and then we tell those waiting it is available...
Indeed.
>> + wake_up_interruptible(&st->wq_data_avail);
>>
>> - if (status & st->channels_mask) {
>> - st->done = true;
>> + if (iio_buffer_enabled(idev)) {
> For ease of flow, can we not make this a one shot interrupt (using
> threaded irqs)? I'm also given we are being notified data is ready
> the read doesn't actually need to happen in the top half. I'd prefer
> seeing just the check that it is this interrupt and the iio_trigger_poll
> happening in the top half.
>
> The trigger_poll is just in case we need to fire a capturing pin on
> something else using this trigger alongside this device.
I'm not sure to understand here. I thought trigger_poll was actually to
trigger an IRQ for the bottom and top half ?
And so, you mean that this handler should be something like
{
if (!(status & AT91_ADC_DRDY))
return IRQ_HANDLED;
if (iio_buffer_enabled(idev))
iio_trigger_poll(idev->trig, ...);
else
st->last_value = ...
return IRQ_HANDLED;
}
with the wake up being moved in the at91_adc_trigger_handler function,
or the other way around, move the read into this handler just before the
wake up ?
>
>> + disable_irq_nosync(irq);
>> + st->irq_enabled = false;
>> + iio_trigger_poll(idev->trig, iio_get_time_ns());
>> + } else {
>> st->last_value = at91_adc_reg_read(st, AT91_ADC_LCDR);
>> }
>>
>> - wake_up_interruptible(&st->wq_data_avail);
>> -
>> return IRQ_HANDLED;
>> }
>>
>> +static const struct iio_buffer_setup_ops at91_adc_buffer_setup_ops = {
>> + .preenable = &iio_sw_buffer_preenable,
>> + .postenable = &iio_triggered_buffer_postenable,
>> + .predisable = &iio_triggered_buffer_predisable,
>> +};
>> +
>> +static int at91_adc_buffer_init(struct iio_dev *idev)
>> +{
>> + struct iio_buffer *buffer;
>> + int ret;
>> +
>> + buffer = iio_sw_rb_allocate(idev);
>> + if (buffer == NULL) {
>> + ret = -ENOMEM;
>> + goto error_ret;
>> + }
>> +
>> + idev->buffer = buffer;
>> + buffer->scan_timestamp = true;
> Killing of bpe is scheduled. Nothing that can't be recovered from
> iio_chan_spec structures and shouldn't be used in capture path anyway.
> The new iio_sw_buffer_preenable is a lot cleverer! Easy to change
> once those patches have merged of course!
Oh, cool :)
I have to admit, I didn't know why I had to set this up after having set
storagebits.
>
>> + buffer->bpe = 2;
>> + buffer->access = &ring_sw_access_funcs;
>> + buffer->setup_ops = &at91_adc_buffer_setup_ops;
>> + buffer->owner = THIS_MODULE;
>> +
> Looking at what you have here, you might save a fair bit of code by
> taking advantage of Lars-Peter's patch of last week that handles all
> this boilerplate under slightly restricted circumstances...
I missed his patch, thanks !
>> + idev->pollfunc = iio_alloc_pollfunc(NULL,
>> + &at91_adc_trigger_handler,
>> + 0,
>> + idev,
>> + "at91_adc-consumer-%d",
>> + idev->id);
>> +
>> + if (idev->pollfunc == NULL) {
>> + ret = -ENOMEM;
>> + goto error_free_buffer;
>> + }
>> +
>> + idev->modes |= INDIO_BUFFER_TRIGGERED;
>> + return 0;
>> +
>> +error_free_buffer:
>> + iio_sw_rb_free(idev->buffer);
>> +error_ret:
>> + return ret;
>> +
>> +}
>> +
>> +static void at91_adc_buffer_remove(struct iio_dev *indio_dev)
>> +{
>> + iio_dealloc_pollfunc(indio_dev->pollfunc);
>> + iio_sw_rb_free(indio_dev->buffer);
>> +}
>> +
>> static int at91_adc_channel_init(struct iio_dev *idev,
>> - struct at91_adc_data *pdata)
>> + struct at91_adc_data *pdata)
> May be a valid whitespace cleanup, but shouldn't be in this patch.
>> {
>> struct at91_adc_state *st = iio_priv(idev);
>> struct iio_chan_spec *chan_array;
>> @@ -121,6 +255,7 @@ static int at91_adc_channel_init(struct iio_dev *idev,
>> chan->type = IIO_VOLTAGE;
>> chan->indexed = 1;
>> chan->channel = bit;
>> + chan->scan_index = idx;
>> chan->scan_type.sign = 'u';
>> chan->scan_type.realbits = 10;
>> chan->scan_type.storagebits = 16;
>> @@ -137,6 +272,145 @@ static void at91_adc_channel_remove(struct iio_dev *idev)
>> kfree(idev->channels);
>> }
>>
>> +static int at91_adc_set_trigger_state(struct iio_trigger *trig, bool state)
>> +{
>> + struct iio_dev *idev = trig->private_data;
>> + struct at91_adc_state *st = iio_priv(idev);
>> + struct iio_buffer *buffer = idev->buffer;
>> + u32 status = at91_adc_reg_read(st, AT91_ADC_MR);
>> + u8 bit;
>> +
>> + if (state) {
>> + int i;
>> +
> Looks suspect. sizeof that is only 4 because it is a pointer, not because
> it has 4 elements I think???
...
You're definitely right here.
>> + for (i = 0; i < sizeof(st->desc->triggers); i++) {
>> + char *name = kasprintf(GFP_KERNEL,
>> + "%s-dev%d-%s",
>> + idev->name,
>> + idev->id,
>> + st->desc->triggers[i].name);
>> +
> Cleaner to verify the kasprintf worked first and return -ENOMEM if it
> failed. Then see if the name matches. Also, !strcmp is a really
> confusing idiom. I at least prefer strcmp() == 0
Ok
>> + if (!strcmp(idev->trig->name, name)) {
>> + status |= st->desc->triggers[i].value;
>> + kfree(name);
>> + break;
>> + }
>> +
>> + kfree(name);
>> + }
>> +
>> + if (i == sizeof(st->desc->triggers))
>> + return -EINVAL;
>> +
>> + at91_adc_reg_write(st, AT91_ADC_MR, status);
>> +
>> + for_each_set_bit(bit, buffer->scan_mask,
>> + st->desc->num_channels) {
>> + struct iio_chan_spec const *chan = idev->channels + bit;
>> + at91_adc_reg_write(st, AT91_ADC_CHER,
>> + AT91_ADC_CH(chan->channel));
>> + }
>> +
>> + at91_adc_reg_write(st, AT91_ADC_IER, AT91_ADC_DRDY);
>> +
>> +
>> + } else {
>> + at91_adc_reg_write(st, AT91_ADC_IDR, AT91_ADC_DRDY);
>> + at91_adc_reg_write(st, AT91_ADC_MR,
>> + status & ~AT91_ADC_TRGEN);
>> +
>> + for_each_set_bit(bit, buffer->scan_mask,
>> + st->desc->num_channels) {
>> + at91_adc_reg_write(st, AT91_ADC_CHDR,
>> + AT91_ADC_CH(bit));
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static const struct iio_trigger_ops at91_adc_trigger_ops = {
>> + .owner = THIS_MODULE,
>> + .set_trigger_state = &at91_adc_set_trigger_state,
>> +};
>> +
>> +
> Name is a little misleading as this also registers the trigger.
> Perhaps at91_adc_configure_trigger?
>> +static struct iio_trigger *at91_adc_allocate_trigger(struct iio_dev *idev,
>> + struct at91_adc_trigger *trigger)
>> +{
>> + struct iio_trigger *trig = iio_allocate_trigger("%s-dev%d-%s",
>> + idev->name,
>> + idev->id,
>> + trigger->name);
>> + int ret;
>> +
>> + if (trig == NULL)
>> + return NULL;
>> +
>> + trig->dev.parent = idev->dev.parent;
>> + trig->owner = THIS_MODULE;
> yikes. That's silly, we still have an owner field in trig and in ops?
> Definitely one for cleaning up.
>> + trig->private_data = idev;
>> + trig->ops = &at91_adc_trigger_ops;
>> +
>> + ret = iio_trigger_register(trig);
>> + if (ret < 0)
>> + return NULL;
>> +
>> + return trig;
>> +}
>> +
>> +static int at91_adc_trigger_init(struct iio_dev *idev,
>> + struct at91_adc_data *pdata)
>> +{
>> + struct at91_adc_state *st = iio_priv(idev);
>> + int i, ret;
>> +
>> + st->trig = kcalloc(sizeof(st->desc->triggers),
>> + sizeof(struct iio_trigger *),
>> + GFP_KERNEL);
>> +
> extra white space...
Hmm, I don't see where it is.
>> + if (st->trig == NULL) {
>> + ret = -ENOMEM;
>> + goto error_ret;
>> + }
>> +
>> + for (i = 0; i < sizeof(st->desc->triggers); i++) {
>> + if (st->desc->triggers[i].is_external && !(pdata->use_external))
>> + continue;
>> +
>> + st->trig[i] = at91_adc_allocate_trigger(idev,
>> + st->desc->triggers + i);
> Check the right trigger ;)
Haha, good catch
Sorry about that, an artifact from the good ol' hardcoded time :)
>> + if (st->trig[0] == NULL) {
>> + ret = -ENOMEM;
>> + goto error_trigger;
>> + }
>> + }
>> +
>> + return 0;
>> +
>> +error_trigger:
>> + for (; i >= 0; i--) {
>> + iio_trigger_unregister(st->trig[i]);
>> + iio_free_trigger(st->trig[i]);
>> + }
>> + kfree(st->trig);
>> +error_ret:
>> + return ret;
>> +}
>> +
>> +static void at91_adc_trigger_remove(struct iio_dev *idev)
>> +{
>> + struct at91_adc_state *st = iio_priv(idev);
>> + int i;
>> +
>> + for (i = 0; i < sizeof(st->trig); i++) {
>> + iio_trigger_unregister(st->trig[i]);
>> + iio_free_trigger(st->trig[i]);
>> + }
>> +
>> + kfree(st->trig);
>> +}
>> +
>> static int at91_adc_read_raw(struct iio_dev *idev,
>> struct iio_chan_spec const *chan,
>> int *val, int *val2, long mask)
>> @@ -309,8 +583,10 @@ static int __devinit at91_adc_probe(struct platform_device *pdev)
>>
>> /* Setup the ADC channels available on the board */
>> ret = at91_adc_channel_init(idev, pdata);
>> - if (ret < 0)
> Shouldn't be in this patch.
>> + if (ret < 0) {
>> + dev_err(&pdev->dev, "Couldn't initialize the channels.\n");
>> goto error_free_clk;
>> + }
>>
>> init_waitqueue_head(&st->wq_data_avail);
>> mutex_init(&st->lock);
>> @@ -318,12 +594,40 @@ static int __devinit at91_adc_probe(struct platform_device *pdev)
>> st->vref_mv = pdata->vref;
>> st->channels_mask = pdata->channels_used;
>>
>> - ret = iio_device_register(idev);
>> - if (ret < 0)
>> + ret = at91_adc_buffer_init(idev);
>> + if (ret < 0) {
>> + dev_err(&pdev->dev, "Couldn't initialize the buffer.\n");
>> goto error_free_channels;
>> + }
>> +
>> + ret = iio_buffer_register(idev,
>> + idev->channels,
>> + idev->num_channels);
>> + if (ret < 0) {
>> + dev_err(&pdev->dev, "Couldn't register the buffer.\n");
>> + goto error_free_buffer;
>> + }
>> +
>> + ret = at91_adc_trigger_init(idev, pdata);
>> + if (ret < 0) {
>> + dev_err(&pdev->dev, "Couldn't setup the triggers.\n");
>> + goto error_unregister_buffer;
>> + }
>> +
>
> Also, not in this patch.
>> + ret = iio_device_register(idev);
>> + if (ret < 0) {
>> + dev_err(&pdev->dev, "Couldn't register the device.\n");
>> + goto error_remove_triggers;
>> + }
>>
>> return 0;
>>
>> +error_remove_triggers:
>> + at91_adc_trigger_remove(idev);
>> +error_unregister_buffer:
>> + iio_buffer_unregister(idev);
>> +error_free_buffer:
>> + at91_adc_buffer_remove(idev);
>> error_free_channels:
>> at91_adc_channel_remove(idev);
>> error_free_clk:
>> @@ -348,6 +652,9 @@ static int __devexit at91_adc_remove(struct platform_device *pdev)
>> struct at91_adc_state *st = iio_priv(idev);
>>
>> iio_device_unregister(idev);
>> + at91_adc_trigger_remove(idev);
>> + iio_buffer_unregister(idev);
>> + at91_adc_buffer_remove(idev);
>> at91_adc_channel_remove(idev);
>> clk_disable(st->clk);
>> clk_put(st->clk);
>> diff --git a/include/linux/platform_data/at91_adc.h b/include/linux/platform_data/at91_adc.h
>> index 9c5a64e..5b65eff 100644
>> --- a/include/linux/platform_data/at91_adc.h
>> +++ b/include/linux/platform_data/at91_adc.h
>> @@ -18,6 +18,8 @@
> Probably worth doing docs here in kernel-doc format.
>> struct at91_adc_data {
>> /* Channels in use on the board as a bitmask */
>> unsigned long channels_used;
>> + /* Do we need the external triggers ? */
>> + bool use_external;
>> /* Reference voltage for the ADC in millivolts */
>> u16 vref;
>> };
>
--
Maxime Ripard, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
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
