* [PATCH 0/3] Add small improvements to iio/magnetometer hmc5843 driver @ 2015-03-19 19:17 Cristina Opriceana 2015-03-19 19:19 ` [PATCH 1/3] Staging: iio: Place driver in sleep mode on error Cristina Opriceana ` (3 more replies) 0 siblings, 4 replies; 9+ messages in thread From: Cristina Opriceana @ 2015-03-19 19:17 UTC (permalink / raw) To: outreachy-kernel This patchset reviews the iio/magnetometer hmc5843 driver and fixes incorrect file header, hardcoded values and reconfigures some small error checks. Cristina Opriceana (3): Staging: iio: Place driver in sleep mode on error Staging: iio: Replace hardcoded value with number of channels Staging: iio: Fix file header to match standards drivers/staging/iio/magnetometer/hmc5843_core.c | 59 +++++++++++++------------ 1 file changed, 30 insertions(+), 29 deletions(-) -- 1.9.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/3] Staging: iio: Place driver in sleep mode on error 2015-03-19 19:17 [PATCH 0/3] Add small improvements to iio/magnetometer hmc5843 driver Cristina Opriceana @ 2015-03-19 19:19 ` Cristina Opriceana 2015-03-20 9:13 ` [Outreachy kernel] " Daniel Baluta 2015-03-20 12:39 ` Greg KH 2015-03-19 19:20 ` [PATCH 2/3] Staging: iio: Replace hardcoded value with number of channels Cristina Opriceana ` (2 subsequent siblings) 3 siblings, 2 replies; 9+ messages in thread From: Cristina Opriceana @ 2015-03-19 19:19 UTC (permalink / raw) To: outreachy-kernel; +Cc: outreachy-kernel Added sleep_mode label, on which the device enters sleep mode if encounters an error after initialization. This helps saving energy consumption. Signed-off-by: Cristina Opriceana <cristina.opriceana@gmail.com> --- drivers/staging/iio/magnetometer/hmc5843_core.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/staging/iio/magnetometer/hmc5843_core.c b/drivers/staging/iio/magnetometer/hmc5843_core.c index 90cc18b..59afede 100644 --- a/drivers/staging/iio/magnetometer/hmc5843_core.c +++ b/drivers/staging/iio/magnetometer/hmc5843_core.c @@ -606,16 +606,17 @@ int hmc5843_common_probe(struct device *dev, struct regmap *regmap, ret = iio_triggered_buffer_setup(indio_dev, NULL, hmc5843_trigger_handler, NULL); if (ret < 0) - return ret; + goto sleep_mode; ret = iio_device_register(indio_dev); - if (ret < 0) - goto buffer_cleanup; - + if (ret < 0) { + iio_triggered_buffer_cleanup(indio_dev); + goto sleep_mode; + } return 0; -buffer_cleanup: - iio_triggered_buffer_cleanup(indio_dev); +sleep_mode: + hmc5843_set_mode(iio_priv(indio_dev), HMC5843_MODE_SLEEP); return ret; } EXPORT_SYMBOL(hmc5843_common_probe); -- 1.9.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Outreachy kernel] [PATCH 1/3] Staging: iio: Place driver in sleep mode on error 2015-03-19 19:19 ` [PATCH 1/3] Staging: iio: Place driver in sleep mode on error Cristina Opriceana @ 2015-03-20 9:13 ` Daniel Baluta 2015-03-20 9:31 ` Cristina Opriceana 2015-03-20 12:39 ` Greg KH 1 sibling, 1 reply; 9+ messages in thread From: Daniel Baluta @ 2015-03-20 9:13 UTC (permalink / raw) To: Cristina Opriceana; +Cc: outreachy-kernel On Thu, Mar 19, 2015 at 9:19 PM, Cristina Opriceana <cristina.opriceana@gmail.com> wrote: > Added sleep_mode label, on which the device enters sleep mode if > encounters an error after initialization. This helps saving energy > consumption. Commit message shouldn't tell "how" it does something, this should be obvious from the code. We should insist more on "why" we need this change. So, the fact that you added "sleep_mode" label shouldn't be present in commit message. Better say: Put device in sleep mode if we encounter an error after initialization to avoid wasting power. > > Signed-off-by: Cristina Opriceana <cristina.opriceana@gmail.com> > --- > drivers/staging/iio/magnetometer/hmc5843_core.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/drivers/staging/iio/magnetometer/hmc5843_core.c b/drivers/staging/iio/magnetometer/hmc5843_core.c > index 90cc18b..59afede 100644 > --- a/drivers/staging/iio/magnetometer/hmc5843_core.c > +++ b/drivers/staging/iio/magnetometer/hmc5843_core.c > @@ -606,16 +606,17 @@ int hmc5843_common_probe(struct device *dev, struct regmap *regmap, > ret = iio_triggered_buffer_setup(indio_dev, NULL, > hmc5843_trigger_handler, NULL); > if (ret < 0) > - return ret; > + goto sleep_mode; > > ret = iio_device_register(indio_dev); > - if (ret < 0) > - goto buffer_cleanup; > - > + if (ret < 0) { > + iio_triggered_buffer_cleanup(indio_dev); > + goto sleep_mode; > + } Why this change here? The initial goto buffer_cleanup will work just fine. > return 0; > > -buffer_cleanup: > - iio_triggered_buffer_cleanup(indio_dev); > +sleep_mode: > + hmc5843_set_mode(iio_priv(indio_dev), HMC5843_MODE_SLEEP); > return ret; > } > EXPORT_SYMBOL(hmc5843_common_probe); > -- Nice patch. A good idea would be to also add module authors/reviewers for this. Thanks, Daniel. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Outreachy kernel] [PATCH 1/3] Staging: iio: Place driver in sleep mode on error 2015-03-20 9:13 ` [Outreachy kernel] " Daniel Baluta @ 2015-03-20 9:31 ` Cristina Opriceana 0 siblings, 0 replies; 9+ messages in thread From: Cristina Opriceana @ 2015-03-20 9:31 UTC (permalink / raw) To: Daniel Baluta; +Cc: outreachy-kernel On Vi, 2015-03-20 at 11:13 +0200, Daniel Baluta wrote: > On Thu, Mar 19, 2015 at 9:19 PM, Cristina Opriceana > <cristina.opriceana@gmail.com> wrote: > > Added sleep_mode label, on which the device enters sleep mode if > > encounters an error after initialization. This helps saving energy > > consumption. > > Commit message shouldn't tell "how" it does something, this should > be obvious from the code. We should insist more on "why" we need > this change. > > So, the fact that you added "sleep_mode" label shouldn't be present > in commit message. > > Better say: > > Put device in sleep mode if we encounter an error after initialization > to avoid wasting power. > > > > > Signed-off-by: Cristina Opriceana <cristina.opriceana@gmail.com> > > --- > > drivers/staging/iio/magnetometer/hmc5843_core.c | 13 +++++++------ > > 1 file changed, 7 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/staging/iio/magnetometer/hmc5843_core.c b/drivers/staging/iio/magnetometer/hmc5843_core.c > > index 90cc18b..59afede 100644 > > --- a/drivers/staging/iio/magnetometer/hmc5843_core.c > > +++ b/drivers/staging/iio/magnetometer/hmc5843_core.c > > @@ -606,16 +606,17 @@ int hmc5843_common_probe(struct device *dev, struct regmap *regmap, > > ret = iio_triggered_buffer_setup(indio_dev, NULL, > > hmc5843_trigger_handler, NULL); > > if (ret < 0) > > - return ret; > > + goto sleep_mode; > > > > ret = iio_device_register(indio_dev); > > - if (ret < 0) > > - goto buffer_cleanup; > > - > > + if (ret < 0) { > > + iio_triggered_buffer_cleanup(indio_dev); > > + goto sleep_mode; > > + } > > Why this change here? The initial goto buffer_cleanup will work just fine. I wanted to avoid duplicating code, but yes, apart from that, the initial goto would work just fine. > > > return 0; > > > > -buffer_cleanup: > > - iio_triggered_buffer_cleanup(indio_dev); > > +sleep_mode: > > + hmc5843_set_mode(iio_priv(indio_dev), HMC5843_MODE_SLEEP); > > return ret; > > } > > EXPORT_SYMBOL(hmc5843_common_probe); > > -- > > Nice patch. A good idea would be to also add module authors/reviewers for this. > > Thanks, > Daniel. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Outreachy kernel] [PATCH 1/3] Staging: iio: Place driver in sleep mode on error 2015-03-19 19:19 ` [PATCH 1/3] Staging: iio: Place driver in sleep mode on error Cristina Opriceana 2015-03-20 9:13 ` [Outreachy kernel] " Daniel Baluta @ 2015-03-20 12:39 ` Greg KH 2015-03-20 15:34 ` Cristina Opriceana 1 sibling, 1 reply; 9+ messages in thread From: Greg KH @ 2015-03-20 12:39 UTC (permalink / raw) To: Cristina Opriceana; +Cc: outreachy-kernel On Thu, Mar 19, 2015 at 09:19:08PM +0200, Cristina Opriceana wrote: > Added sleep_mode label, on which the device enters sleep mode if > encounters an error after initialization. This helps saving energy > consumption. > > Signed-off-by: Cristina Opriceana <cristina.opriceana@gmail.com> > --- > drivers/staging/iio/magnetometer/hmc5843_core.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) This series doesn't apply, and also, if forced, breaks the build :( Please fix up and resend. thanks, greg k-h ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Outreachy kernel] [PATCH 1/3] Staging: iio: Place driver in sleep mode on error 2015-03-20 12:39 ` Greg KH @ 2015-03-20 15:34 ` Cristina Opriceana 0 siblings, 0 replies; 9+ messages in thread From: Cristina Opriceana @ 2015-03-20 15:34 UTC (permalink / raw) To: Greg KH; +Cc: outreachy-kernel On Vi, 2015-03-20 at 13:39 +0100, Greg KH wrote: > On Thu, Mar 19, 2015 at 09:19:08PM +0200, Cristina Opriceana wrote: > > Added sleep_mode label, on which the device enters sleep mode if > > encounters an error after initialization. This helps saving energy > > consumption. > > > > Signed-off-by: Cristina Opriceana <cristina.opriceana@gmail.com> > > --- > > drivers/staging/iio/magnetometer/hmc5843_core.c | 13 +++++++------ > > 1 file changed, 7 insertions(+), 6 deletions(-) > > This series doesn't apply, and also, if forced, breaks the build :( > > Please fix up and resend. > I'll fix and resend. Sorry about that, I changed some .config options and forgot to put them back. > thanks, > > greg k-h ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] Staging: iio: Replace hardcoded value with number of channels 2015-03-19 19:17 [PATCH 0/3] Add small improvements to iio/magnetometer hmc5843 driver Cristina Opriceana 2015-03-19 19:19 ` [PATCH 1/3] Staging: iio: Place driver in sleep mode on error Cristina Opriceana @ 2015-03-19 19:20 ` Cristina Opriceana 2015-03-19 19:22 ` [PATCH 3/3] Staging: iio: Fix file header to match standards Cristina Opriceana 2015-03-20 8:58 ` [Outreachy kernel] [PATCH 0/3] Add small improvements to iio/magnetometer hmc5843 driver Daniel Baluta 3 siblings, 0 replies; 9+ messages in thread From: Cristina Opriceana @ 2015-03-19 19:20 UTC (permalink / raw) To: outreachy-kernel; +Cc: outreachy-kernel This patch replaces the hardcoded value 4 for the number of available channels and computes it with the ARRAY_SIZE macro. Signed-off-by: Cristina Opriceana <cristina.opriceana@gmail.com> --- drivers/staging/iio/magnetometer/hmc5843_core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/iio/magnetometer/hmc5843_core.c b/drivers/staging/iio/magnetometer/hmc5843_core.c index 59afede..3898690 100644 --- a/drivers/staging/iio/magnetometer/hmc5843_core.c +++ b/drivers/staging/iio/magnetometer/hmc5843_core.c @@ -596,7 +596,7 @@ int hmc5843_common_probe(struct device *dev, struct regmap *regmap, indio_dev->info = &hmc5843_info; indio_dev->modes = INDIO_DIRECT_MODE; indio_dev->channels = data->variant->channels; - indio_dev->num_channels = 4; + indio_dev->num_channels = ARRAY_SIZE(data->variant->channels); indio_dev->available_scan_masks = hmc5843_scan_masks; ret = hmc5843_init(data); -- 1.9.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/3] Staging: iio: Fix file header to match standards 2015-03-19 19:17 [PATCH 0/3] Add small improvements to iio/magnetometer hmc5843 driver Cristina Opriceana 2015-03-19 19:19 ` [PATCH 1/3] Staging: iio: Place driver in sleep mode on error Cristina Opriceana 2015-03-19 19:20 ` [PATCH 2/3] Staging: iio: Replace hardcoded value with number of channels Cristina Opriceana @ 2015-03-19 19:22 ` Cristina Opriceana 2015-03-20 8:58 ` [Outreachy kernel] [PATCH 0/3] Add small improvements to iio/magnetometer hmc5843 driver Daniel Baluta 3 siblings, 0 replies; 9+ messages in thread From: Cristina Opriceana @ 2015-03-19 19:22 UTC (permalink / raw) To: outreachy-kernel; +Cc: outreachy-kernel Fix file header to match Linux Kernel style. Remove Free Software Foundation reference to silence the checkpatch.pl warning. Add driver description and correct spelling mistakes. Signed-off-by: Cristina Opriceana <cristina.opriceana@gmail.com> --- drivers/staging/iio/magnetometer/hmc5843_core.c | 44 ++++++++++++------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/drivers/staging/iio/magnetometer/hmc5843_core.c b/drivers/staging/iio/magnetometer/hmc5843_core.c index 3898690..d03833b 100644 --- a/drivers/staging/iio/magnetometer/hmc5843_core.c +++ b/drivers/staging/iio/magnetometer/hmc5843_core.c @@ -1,25 +1,25 @@ -/* Copyright (C) 2010 Texas Instruments - Author: Shubhrajyoti Datta <shubhrajyoti@ti.com> - Acknowledgement: Jonathan Cameron <jic23@kernel.org> for valuable inputs. - - Support for HMC5883 and HMC5883L by Peter Meerwald <pmeerw@pmeerw.net>. - - Split to multiple files by Josef Gajdusek <atx@atx.name> - 2014 - - 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. - - You should have received a copy of the GNU General Public License - along with this program; if not, write to the Free Software - Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. -*/ +/* + * Device driver for the the HMC5843 multi-chip module designed + * for low field magnetic sensing. + * + * Copyright (C) 2010 Texas Instruments + * + * Author: Shubhrajyoti Datta <shubhrajyoti@ti.com> + * Acknowledgment: Jonathan Cameron <jic23@kernel.org> for valuable inputs. + * Support for HMC5883 and HMC5883L by Peter Meerwald <pmeerw@pmeerw.net>. + * Split to multiple files by Josef Gajdusek <atx@atx.name> - 2014 + * + * 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/regmap.h> -- 1.9.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Outreachy kernel] [PATCH 0/3] Add small improvements to iio/magnetometer hmc5843 driver 2015-03-19 19:17 [PATCH 0/3] Add small improvements to iio/magnetometer hmc5843 driver Cristina Opriceana ` (2 preceding siblings ...) 2015-03-19 19:22 ` [PATCH 3/3] Staging: iio: Fix file header to match standards Cristina Opriceana @ 2015-03-20 8:58 ` Daniel Baluta 3 siblings, 0 replies; 9+ messages in thread From: Daniel Baluta @ 2015-03-20 8:58 UTC (permalink / raw) To: Cristina Opriceana; +Cc: outreachy-kernel On Thu, Mar 19, 2015 at 9:17 PM, Cristina Opriceana <cristina.opriceana@gmail.com> wrote: > This patchset reviews the iio/magnetometer hmc5843 driver and fixes incorrect > file header, hardcoded values and reconfigures some small error checks. > > Cristina Opriceana (3): > Staging: iio: Place driver in sleep mode on error > Staging: iio: Replace hardcoded value with number of channels > Staging: iio: Fix file header to match standards Not sure if this patch series depends on [PATCH 0/5] Fix iio/magnetometer style warnings If so, please merge these into one. It will be easier for Greg to merge them. Daniel. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-03-20 15:35 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-03-19 19:17 [PATCH 0/3] Add small improvements to iio/magnetometer hmc5843 driver Cristina Opriceana 2015-03-19 19:19 ` [PATCH 1/3] Staging: iio: Place driver in sleep mode on error Cristina Opriceana 2015-03-20 9:13 ` [Outreachy kernel] " Daniel Baluta 2015-03-20 9:31 ` Cristina Opriceana 2015-03-20 12:39 ` Greg KH 2015-03-20 15:34 ` Cristina Opriceana 2015-03-19 19:20 ` [PATCH 2/3] Staging: iio: Replace hardcoded value with number of channels Cristina Opriceana 2015-03-19 19:22 ` [PATCH 3/3] Staging: iio: Fix file header to match standards Cristina Opriceana 2015-03-20 8:58 ` [Outreachy kernel] [PATCH 0/3] Add small improvements to iio/magnetometer hmc5843 driver Daniel Baluta
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.