On 05/02/16 09:58, Daniel Baluta wrote:
> On Thu, Feb 4, 2016 at 7:45 PM, Lars-Peter Clausen <l...@metafoo.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 majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
