linux-kernel - Re: [PATCH v5 3/3] iio: magnetometer: Add driver support for PNI RM3100

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <716779d7-4fcc-9914-cf73-0b852902c9bd@gmail.com>
Date:   Mon, 5 Nov 2018 09:19:29 +0800
From:   Song Qiang <songqiang1304521@...il.com>
To:     Jonathan Cameron <jic23@...nel.org>
Cc:     knaack.h@....de, lars@...afoo.de, pmeerw@...erw.net,
        robh+dt@...nel.org, mark.rutland@....com, preid@...ctromag.com.au,
        himanshujha199640@...il.com, linux-iio@...r.kernel.org,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v5 3/3] iio: magnetometer: Add driver support for PNI
 RM3100


On 2018/11/4 上午1:19, Jonathan Cameron wrote:
> On Fri,  2 Nov 2018 15:42:09 +0800
> Song Qiang <songqiang1304521@...il.com> wrote:
>
>
...
>> +int rm3100_common_probe(struct device *dev, struct regmap *regmap, int irq)
>> +{
>> +	struct iio_dev *indio_dev;
>> +	struct rm3100_data *data;
>> +	unsigned int tmp;
>> +	int ret;
>> +
>> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
>> +	if (!indio_dev)
>> +		return -ENOMEM;
>> +
>> +	data = iio_priv(indio_dev);
>> +	data->regmap = regmap;
>> +
>> +	mutex_init(&data->lock);
>> +
>> +	indio_dev->dev.parent = dev;
>> +	indio_dev->name = "rm3100";
>> +	indio_dev->info = &rm3100_info;
>> +	indio_dev->channels = rm3100_channels;
>> +	indio_dev->num_channels = ARRAY_SIZE(rm3100_channels);
>> +	indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_TRIGGERED;
>> +	indio_dev->currentmode = INDIO_DIRECT_MODE;
> Huh. That's an interesting one... Looks like a bug in the core to me that
> somehow has never bitten us as no driver has ever done anything other
> than explicitly match against other modes.  No one sets currentmode
> directly so we should be good just putting this in the core as long
> as a device supports direct mode.  I'll look at that when I get a moment.
>
Hi Jonathan,

Last patch series I submit had 'case 0' for threaded function of interrupt 
handler. That's just because of this. I greped INDIO_DIRECT_MODE in iio folder, 
and found out that the iio core only set the driver mode to INDIO_DIRECT_MODE 
when it leaves buffer mode. Originally I thought when I call 
'iio_claim_direct_mode()' the currentmode should be set to INDIO_DIRECT_MODE.

This problem would lead to a potential bug if someone installed the device and 
use single-shot mode before any buffered read(I met this).

Though it seems like not many drivers use the same architecture I'm using, they 
don't need to check current mode, I'll look into more drivers.

st_sensors_core.c:589 seems like to choose to treat this as it's a 
bi-conditional situation with 'if it's in INDIO_BUFFER_TRIGGERED mode, if it's 
not, no matter what value it is, it's in INDIO_DIRECT_MODE.'


yours,

Song Qiang

...