linux-kernel - Re: [RFC PATCH 1/2] iio: hmc5843 Add channel attribute for bias configuration

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <56B635BF.2090201@kernel.org>
Date:	Sat, 6 Feb 2016 18:04:47 +0000
From:	Jonathan Cameron <jic23@...nel.org>
To:	Daniel Baluta <daniel.baluta@...el.com>,
	Lars-Peter Clausen <lars@...afoo.de>
Cc:	Cristina Moraru <cristina.moraru09@...il.com>,
	Hartmut Knaack <knaack.h@....de>,
	Peter Meerwald-Stadler <pmeerw@...erw.net>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Cristina Georgiana Opriceana <cristina.opriceana@...il.com>,
	marek@...delico.com, sdliyong@...il.com,
	"linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
	driverdev <devel@...verdev.osuosl.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	"octavian.purdila@...el.com" <octavian.purdila@...el.com>,
	shubhrajyoti@...com
Subject: Re: [RFC PATCH 1/2] iio: hmc5843 Add channel attribute for bias
 configuration

On 05/02/16 09:58, Daniel Baluta wrote:
> On Thu, Feb 4, 2016 at 7:45 PM, Lars-Peter Clausen <lars@...afoo.de> wrote:
>> On 02/04/2016 03:50 PM, Cristina Moraru wrote:
>>> Replace non standard meas_conf attribute with the standard IIO
>>> calibbias attribute.
>>>
>>> API for setting bias measurement configuration:
>>>
>>> 0 - Normal measurement configuration (default): In normal measurement
>>>     configuration the device follows normal measurement flow. Pins BP
>>>     and BN are left floating and high impedance.
>>>
>>> 1 - Positive bias configuration: In positive bias configuration, a
>>>     positive current is forced across the resistive load on pins BP
>>>     and BN.
>>>
>>> 2 - Negative bias configuration. In negative bias configuration, a
>>>     negative current is forced across the resistive load on pins BP
>>>     and BN.
>>>
>>> 3 - Only available on HMC5983. Magnetic sensor is disabled.
>>>     Temperature sensor is enabled.
>>>
>>> With this in place, we can think of moving this driver out of staging.
>>
>> Using a standard attribute, but overloading it with custom semantics doesn't
>> do any good either. calibbias is supposed to be a integer that gets added to
>> measurements internally in the device (unit is device specific though).
>>
>> This attribute seems to do something else. In that case it might be better
>> to stay with a custom attribute (as long as it is documented) or come up
>> with a better way to map the device configuration onto standard attributes.
> 
> We missed the RFC tag. So, with this in mind we still have some minor
> issues to fix and then we can move the code out of staging.
> 
> 1) checkpatch.pl -- strict warnings
> 2) suspend/resume functions in hmc5843_core.c.
> 
> int hmc5843_common_suspend(struct device *dev)
> {
>         return hmc5843_set_mode(iio_priv(dev_get_drvdata(dev)),
>                         HMC5843_MODE_CONVERSION_CONTINUOUS);
> }
> 
> int hmc5843_common_resume(struct device *dev)
> {
>         return hmc5843_set_mode(iio_priv(dev_get_drvdata(dev)),
>                                 HMC5843_MODE_SLEEP);
> }
> 
> I think the function names or implementations were swapped.
> The device should go to sleep at suspend and back to continuous
> mode on resume.
Yup, from a quick glance - it otherwise looks sensible to me.
Will as ever attract more reviews once the move out of staging
patch shows up.

Jonathan

> 
> thanks,
> Daniel.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>