From: Jonathan Cameron <jic23@kernel.org>
To: "ludovic.tancerel@maplehightech.com"
<ludovic.tancerel@maplehightech.com>
Cc: knaack.h@gmx.de, lars@metafoo.de, pmeerw@pmeerw.net,
linux-iio@vger.kernel.org, William.Markezana@meas-spec.com
Subject: Re: [PATCH V2 6/6] iio: Add ms8607 meas-spec driver support
Date: Sun, 27 Sep 2015 17:01:30 +0100 [thread overview]
Message-ID: <560812DA.7030004@kernel.org> (raw)
In-Reply-To: <939F5CDA-AEBD-4060-8B15-FCD4D9424B6F@maplehightech.com>
On 25/09/15 10:31, ludovic.tancerel@maplehightech.com wrote:
> Resend as for some reason, there was a problem with mail server
>
> Regards,
> Ludovic
>
>> Jonathan,
>> some quick feedback on one of the comments below.
>>
>> Please let me know if you disagree.
>>
>> Regards,
>> Ludovic
>>
>>
>> Le 8 août 2015 à 19:22, Jonathan Cameron <jic23@kernel.org <mailto:jic23@kernel.org>> a écrit :
>>
>>> On 30/07/15 10:25, Ludovic Tancerel wrote:
>>>> Signed-off-by: Ludovic Tancerel <ludovic.tancerel@maplehightech.com <mailto:ludovic.tancerel@maplehightech.com>>
>>> Little bits inline.
>>>
>>> A nice looking patch set. I was unsure the division made sense between the
>>> core and the drivers, but it seems to work well.
>>> Will be interesting to see what other parts measurement specialties comes out
>>> with in the future!
>>>
>>> Jonathan
>>>> ---
>>>> Documentation/ABI/testing/sysfs-bus-iio-meas-spec | 1 +
>>>> drivers/iio/humidity/htu21.c | 32 ++++++++++++++++++++---
>>>> drivers/iio/pressure/Kconfig | 13 +++++++++
>>>> drivers/iio/pressure/ms5637.c | 6 ++++-
>>>> 4 files changed, 48 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-meas-spec b/Documentation/ABI/testing/sysfs-bus-iio-meas-spec
>>>> index 7b09d3a..df70dda6 100644
>>>> --- a/Documentation/ABI/testing/sysfs-bus-iio-meas-spec
>>>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-meas-spec
>>>> @@ -12,3 +12,4 @@ Description:
>>>> Enable or disable heater function by writing either
>>>> '1' or '0'.
>>>> Same reading values apply
>>>> +This ABI is available for htu21, ms8607
>>>> diff --git a/drivers/iio/humidity/htu21.c b/drivers/iio/humidity/htu21.c
>>>> index 870b631..4720cbc 100644
>>>> --- a/drivers/iio/humidity/htu21.c
>>>> +++ b/drivers/iio/humidity/htu21.c
>>>> @@ -1,6 +1,7 @@
>>>> /*
>>>> * htu21.c - Support for Measurement-Specialties
>>>> * htu21 temperature & humidity sensor
>>>> + * and humidity part of MS8607 sensor
>>>> *
>>>> * Copyright (c) 2014 Measurement-Specialties
>>>> *
>>>> @@ -10,6 +11,8 @@
>>>> *
>>>> * Datasheet:
>>>> * http://www.meas-spec.com/downloads/HTU21D.pdf
>>>> + * Datasheet:
>>>> + * http://www.meas-spec.com/downloads/MS8607-02BA01.pdf
>>>> *
>>>> */
>>>>
>>>> @@ -25,6 +28,11 @@
>>>>
>>>> #define HTU21_RESET0xFE
>>>>
>>>> +enum {
>>>> +HTU21,
>>>> +MS8607
>>>> +};
>>>> +
>>>> static const int htu21_samp_freq[4] = { 20, 40, 70, 120 };
>>>> /* String copy of the above const for readability purpose */
>>>> static const char htu21_show_samp_freq[] = "20 40 70 120";
>>>> @@ -107,6 +115,17 @@ static const struct iio_chan_spec htu21_channels[] = {
>>>> }
>>>> };
>>>>
>>>> +/* Meas Spec recommendation is to not read temperature
>>> /*
>>> * Meas
>>>> + * on this driver part for MS8607
>>>> + */
>>>> +static const struct iio_chan_spec ms8607_channels[] = {
>>>> +{
>>>> +.type = IIO_HUMIDITYRELATIVE,
>>>> +.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_PROCESSED),
>>>> +.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
>>>> + }
>>>> +};
>>>> +
>>>> static ssize_t htu21_show_battery_low(struct device *dev,
>>>> struct device_attribute *attr, char *buf)
>>>> {
>>>> @@ -189,8 +208,14 @@ int htu21_probe(struct i2c_client *client,
>>>> indio_dev->name = id->name;
>>>> indio_dev->dev.parent = &client->dev;
>>>> indio_dev->modes = INDIO_DIRECT_MODE;
>>>> -indio_dev->channels = htu21_channels;
>>>> -indio_dev->num_channels = ARRAY_SIZE(htu21_channels);
>>>> +
>>>> +if (id->driver_data == MS8607) {
>>>> +indio_dev->channels = ms8607_channels;
>>>> +indio_dev->num_channels = ARRAY_SIZE(ms8607_channels);
>>>> +} else {
>>>> +indio_dev->channels = htu21_channels;
>>>> +indio_dev->num_channels = ARRAY_SIZE(htu21_channels);
>>>> +}
>>>>
>>>> i2c_set_clientdata(client, indio_dev);
>>>>
>>>> @@ -207,7 +232,8 @@ int htu21_probe(struct i2c_client *client,
>>>> }
>>>>
>>>> static const struct i2c_device_id htu21_id[] = {
>>>> -{"htu21", 0},
>>>> +{"htu21", HTU21},
>>>> +{"ms8607-h", MS8607},
>>> perhaps -humidity for clarity if not already in use out in the field.
>>
>> h stands for humidity
>> tp for temperature and pressure.
>>
>> ms8607-temperaturepressure seems too long to me, so I prefer to keep ms8607-tp and keep ms8607-h for consistancy
hmm. Rather feels like there might be a middle ground such as -temppres which still makes it more obvious than the
single letter!
>>
>>>> {}
>>>> };
>>>>
>>>> diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig
>>>> index 8142cfe..e6a7fd5 100644
>>>> --- a/drivers/iio/pressure/Kconfig
>>>> +++ b/drivers/iio/pressure/Kconfig
>>>> @@ -90,6 +90,19 @@ config MS5637
>>>> This driver can also be built as a module. If so, the module will
>>>> be called ms5637.
>>>>
>>>> +config MS8607
>>>> +tristate "Measurement Specialties MS8607 pressure, temperature & humidity sensor"
>>>> +depends on I2C
>>>> + select IIO_MS_SENSORS_I2C
>>>> + select HTU21
>>>> + select MS5637
>>>> +help
>>>> + If you say yes here you get support for the Measurement Specialties
>>>> + MS8607 pressure, temperature and humidity sensor.
>>>> +
>>>> + This is based on HTU21 and MS5637 drivers. If built as a module,
>>>> + modules to load will be htu21 and ms5637.
>>>> +
>>> Don't bother with the separate kconfig entry. Just make sure the help texts for
>>> the other two make it clear that they also support part of this device.
>>>
>>>> config IIO_ST_PRESS
>>>> tristate "STMicroelectronics pressure sensor Driver"
>>>> depends on (I2C || SPI_MASTER) && SYSFS
>>>> diff --git a/drivers/iio/pressure/ms5637.c b/drivers/iio/pressure/ms5637.c
>>>> index e2073fd..c185aa0 100644
>>>> --- a/drivers/iio/pressure/ms5637.c
>>>> +++ b/drivers/iio/pressure/ms5637.c
>>>> @@ -1,5 +1,5 @@
>>>> /*
>>>> - * ms5637.c - Support for Measurement-Specialties ms5637
>>>> + * ms5637.c - Support for Measurement-Specialties ms5637 and ms8607
>>>> * pressure & temperature sensor
>>>> *
>>>> * Copyright (c) 2015 Measurement-Specialties
>>>> @@ -10,8 +10,11 @@
>>>> *
>>>> * Datasheet:
>>>> * http://www.meas-spec.com/downloads/MS5637-02BA03.pdf
>>>> + * Datasheet:
>>>> + * http://www.meas-spec.com/downloads/MS8607-02BA01.pdf
>>>> *
>>>> */
>>>> +
>>>> #include <linux/init.h>
>>>> #include <linux/device.h>
>>>> #include <linux/kernel.h>
>>>> @@ -168,6 +171,7 @@ static int ms5637_probe(struct i2c_client *client,
>>>>
>>>> static const struct i2c_device_id ms5637_id[] = {
>>>> {"ms5637", 0},
>>>> +{"ms8607-tp", 1},
>>> Perhaps -temp for extra clarity (or are there devices out there already
>>> using this naming?)
>>>> {}
>>>> };
>>>>
>>>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>>> the body of a message to majordomo@vger.kernel.org <mailto:majordomo@vger.kernel.org>
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
prev parent reply other threads:[~2015-09-27 16:01 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-30 9:25 [PATCH V2 0/6] iio: TSYS01, TSYS02D, HTU21, MS5637, MS8607 Measurement Specialties driver development Ludovic Tancerel
2015-07-30 9:25 ` [PATCH V2 1/6] iio: Add meas-spec sensors common part Ludovic Tancerel
2015-08-08 16:58 ` Jonathan Cameron
2015-08-10 7:43 ` Crt Mori
2015-08-10 11:07 ` Crt Mori
2015-08-15 20:09 ` Jonathan Cameron
2015-09-14 12:56 ` ludovic.tancerel
2015-09-20 19:22 ` Jonathan Cameron
2015-09-23 20:55 ` ludovic.tancerel
2015-07-30 9:25 ` [PATCH V2 2/6] iio: Add tsys01 meas-spec driver support Ludovic Tancerel
2015-08-08 17:03 ` Jonathan Cameron
2015-08-08 17:04 ` Jonathan Cameron
2015-07-30 9:25 ` [PATCH V2 3/6] iio: Add tsys02d " Ludovic Tancerel
2015-08-08 17:15 ` Jonathan Cameron
2015-08-09 12:33 ` Guenter Roeck
2015-08-09 20:13 ` Hartmut Knaack
2015-08-09 20:21 ` Guenter Roeck
2015-07-30 9:25 ` [PATCH V2 4/6] iio: Add htu21 " Ludovic Tancerel
2015-08-08 17:17 ` Jonathan Cameron
2015-07-30 9:25 ` [PATCH V2 5/6] iio: Add ms5637 " Ludovic Tancerel
2015-07-30 9:25 ` [PATCH V2 6/6] iio: Add ms8607 " Ludovic Tancerel
2015-08-08 17:22 ` Jonathan Cameron
2015-09-25 9:25 ` ludovic.tancerel
2015-09-25 9:31 ` ludovic.tancerel
2015-09-27 16:01 ` Jonathan Cameron [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=560812DA.7030004@kernel.org \
--to=jic23@kernel.org \
--cc=William.Markezana@meas-spec.com \
--cc=knaack.h@gmx.de \
--cc=lars@metafoo.de \
--cc=linux-iio@vger.kernel.org \
--cc=ludovic.tancerel@maplehightech.com \
--cc=pmeerw@pmeerw.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.