* [PATCH 0/2] iio: humidity: hdc100x: update driver locking @ 2016-07-10 21:29 Alison Schofield 2016-07-10 21:30 ` [PATCH 1/2] iio: humidity: hdc100x: move lock on config updates to single function Alison Schofield 2016-07-10 21:31 ` [PATCH 2/2] iio: humidity: hdc100x: remove lock on heater configuration read Alison Schofield 0 siblings, 2 replies; 4+ messages in thread From: Alison Schofield @ 2016-07-10 21:29 UTC (permalink / raw) To: jic23; +Cc: mranostay, knaack.h, lars, pmeerw, linux-iio, linux-kernel This patchset intends to tidy up the driver locking. The global data lock needs to to protect writes to the configuration register and single channel reads which can be of temp or humidity. First patch moves the config register locking to the config update function. This continues to protect updates to heater and integration times. It puts the lock in one place, right where it needs to occur. Second patch removes the lock on configuration reads of data stored in global data. While the write lock prevents two simultaneous writes (ie. Heater and int_time) to the register, which could return false success status, the read doesn't have the same issue. You get the status at the moment, at it's guaranteed for that moment only. This also aligns with drivers non-locking of other reads of global data configuration status - ie integration time. I believe these changes stand alone as good cleanups to the locking. Alas, my ulterior motive is to prep it so that I can cleanly apply the locks for triggered buffer mode. Alison Schofield (2): iio: humidity: hdc100x: move lock on config updates to single function iio: humidity: hdc100x: remove lock on heater configuration read drivers/iio/humidity/hdc100x.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) -- 2.1.4 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/2] iio: humidity: hdc100x: move lock on config updates to single function 2016-07-10 21:29 [PATCH 0/2] iio: humidity: hdc100x: update driver locking Alison Schofield @ 2016-07-10 21:30 ` Alison Schofield 2016-07-16 5:23 ` Alison Schofield 2016-07-10 21:31 ` [PATCH 2/2] iio: humidity: hdc100x: remove lock on heater configuration read Alison Schofield 1 sibling, 1 reply; 4+ messages in thread From: Alison Schofield @ 2016-07-10 21:30 UTC (permalink / raw) To: jic23; +Cc: mranostay, knaack.h, lars, pmeerw, linux-iio, linux-kernel Move the config register locking to the config update function. This continues to protect updates to heater and integration times. It puts the lock in one place, right where it needs to occur. Add the checkpatch required comment on this lock declaration. Signed-off-by: Alison Schofield <amsfield22@gmail.com> Cc: Daniel Baluta <daniel.baluta@gmail.com> --- drivers/iio/humidity/hdc100x.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/drivers/iio/humidity/hdc100x.c b/drivers/iio/humidity/hdc100x.c index a03832a..ad5a12a 100644 --- a/drivers/iio/humidity/hdc100x.c +++ b/drivers/iio/humidity/hdc100x.c @@ -31,7 +31,7 @@ struct hdc100x_data { struct i2c_client *client; - struct mutex lock; + struct mutex lock; /* protect config updates & raw measurements */ u16 config; /* integration time of the sensor */ @@ -108,10 +108,12 @@ static int hdc100x_update_config(struct hdc100x_data *data, int mask, int val) int tmp = (~mask & data->config) | val; int ret; + mutex_lock(&data->lock); ret = i2c_smbus_write_word_swapped(data->client, HDC100X_REG_CONFIG, tmp); if (!ret) data->config = tmp; + mutex_unlock(&data->lock); return ret; } @@ -234,26 +236,20 @@ static int hdc100x_write_raw(struct iio_dev *indio_dev, int val, int val2, long mask) { struct hdc100x_data *data = iio_priv(indio_dev); - int ret = -EINVAL; switch (mask) { case IIO_CHAN_INFO_INT_TIME: if (val != 0) return -EINVAL; - mutex_lock(&data->lock); - ret = hdc100x_set_it_time(data, chan->address, val2); - mutex_unlock(&data->lock); - return ret; + return hdc100x_set_it_time(data, chan->address, val2); + case IIO_CHAN_INFO_RAW: if (chan->type != IIO_CURRENT || val2 != 0) return -EINVAL; - mutex_lock(&data->lock); - ret = hdc100x_update_config(data, HDC100X_REG_CONFIG_HEATER_EN, + return hdc100x_update_config(data, HDC100X_REG_CONFIG_HEATER_EN, val ? HDC100X_REG_CONFIG_HEATER_EN : 0); - mutex_unlock(&data->lock); - return ret; default: return -EINVAL; } -- 2.1.4 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] iio: humidity: hdc100x: move lock on config updates to single function 2016-07-10 21:30 ` [PATCH 1/2] iio: humidity: hdc100x: move lock on config updates to single function Alison Schofield @ 2016-07-16 5:23 ` Alison Schofield 0 siblings, 0 replies; 4+ messages in thread From: Alison Schofield @ 2016-07-16 5:23 UTC (permalink / raw) To: jic23; +Cc: mranostay, knaack.h, lars, pmeerw, linux-iio, linux-kernel On Sun, Jul 10, 2016 at 02:30:01PM -0700, Alison Schofield wrote: > Move the config register locking to the config update function. This > continues to protect updates to heater and integration times. It puts > the lock in one place, right where it needs to occur. Since creating this patch, I've learned that it is a design choice in kernel drivers to keep the locking at a higher level - ie point of entry. This patch goes against that design, so, I get that it's not do-able. alisons > > Add the checkpatch required comment on this lock declaration. > > Signed-off-by: Alison Schofield <amsfield22@gmail.com> > Cc: Daniel Baluta <daniel.baluta@gmail.com> > --- > drivers/iio/humidity/hdc100x.c | 16 ++++++---------- > 1 file changed, 6 insertions(+), 10 deletions(-) > > diff --git a/drivers/iio/humidity/hdc100x.c b/drivers/iio/humidity/hdc100x.c > index a03832a..ad5a12a 100644 > --- a/drivers/iio/humidity/hdc100x.c > +++ b/drivers/iio/humidity/hdc100x.c > @@ -31,7 +31,7 @@ > > struct hdc100x_data { > struct i2c_client *client; > - struct mutex lock; > + struct mutex lock; /* protect config updates & raw measurements */ > u16 config; > > /* integration time of the sensor */ > @@ -108,10 +108,12 @@ static int hdc100x_update_config(struct hdc100x_data *data, int mask, int val) > int tmp = (~mask & data->config) | val; > int ret; > > + mutex_lock(&data->lock); > ret = i2c_smbus_write_word_swapped(data->client, > HDC100X_REG_CONFIG, tmp); > if (!ret) > data->config = tmp; > + mutex_unlock(&data->lock); > > return ret; > } > @@ -234,26 +236,20 @@ static int hdc100x_write_raw(struct iio_dev *indio_dev, > int val, int val2, long mask) > { > struct hdc100x_data *data = iio_priv(indio_dev); > - int ret = -EINVAL; > > switch (mask) { > case IIO_CHAN_INFO_INT_TIME: > if (val != 0) > return -EINVAL; > > - mutex_lock(&data->lock); > - ret = hdc100x_set_it_time(data, chan->address, val2); > - mutex_unlock(&data->lock); > - return ret; > + return hdc100x_set_it_time(data, chan->address, val2); > + > case IIO_CHAN_INFO_RAW: > if (chan->type != IIO_CURRENT || val2 != 0) > return -EINVAL; > > - mutex_lock(&data->lock); > - ret = hdc100x_update_config(data, HDC100X_REG_CONFIG_HEATER_EN, > + return hdc100x_update_config(data, HDC100X_REG_CONFIG_HEATER_EN, > val ? HDC100X_REG_CONFIG_HEATER_EN : 0); > - mutex_unlock(&data->lock); > - return ret; > default: > return -EINVAL; > } > -- > 2.1.4 > ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 2/2] iio: humidity: hdc100x: remove lock on heater configuration read 2016-07-10 21:29 [PATCH 0/2] iio: humidity: hdc100x: update driver locking Alison Schofield 2016-07-10 21:30 ` [PATCH 1/2] iio: humidity: hdc100x: move lock on config updates to single function Alison Schofield @ 2016-07-10 21:31 ` Alison Schofield 1 sibling, 0 replies; 4+ messages in thread From: Alison Schofield @ 2016-07-10 21:31 UTC (permalink / raw) To: jic23; +Cc: mranostay, knaack.h, lars, pmeerw, linux-iio, linux-kernel The lock around the read (from global data) of the heater configuration status is not needed. Move the lock in IIO_CHAN_INFO_RAW case to only protect the get measurement function. Signed-off-by: Alison Schofield <amsfield22@gmail.com> Cc: Daniel Baluta <daniel.baluta@gmail.com> --- drivers/iio/humidity/hdc100x.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/iio/humidity/hdc100x.c b/drivers/iio/humidity/hdc100x.c index ad5a12a..8195b0c 100644 --- a/drivers/iio/humidity/hdc100x.c +++ b/drivers/iio/humidity/hdc100x.c @@ -193,18 +193,18 @@ static int hdc100x_read_raw(struct iio_dev *indio_dev, case IIO_CHAN_INFO_RAW: { int ret; - mutex_lock(&data->lock); if (chan->type == IIO_CURRENT) { *val = hdc100x_get_heater_status(data); ret = IIO_VAL_INT; } else { + mutex_lock(&data->lock); ret = hdc100x_get_measurement(data, chan); + mutex_unlock(&data->lock); if (ret >= 0) { *val = ret; ret = IIO_VAL_INT; } } - mutex_unlock(&data->lock); return ret; } case IIO_CHAN_INFO_INT_TIME: -- 2.1.4 ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-07-16 5:23 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-07-10 21:29 [PATCH 0/2] iio: humidity: hdc100x: update driver locking Alison Schofield 2016-07-10 21:30 ` [PATCH 1/2] iio: humidity: hdc100x: move lock on config updates to single function Alison Schofield 2016-07-16 5:23 ` Alison Schofield 2016-07-10 21:31 ` [PATCH 2/2] iio: humidity: hdc100x: remove lock on heater configuration read Alison Schofield
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).