| 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
| ||
|
Date: Tue, 18 Jun 2024 20:05:13 +0800
From: Yasin Lee <yasin.lee.x@...look.com>
To: Jonathan Cameron <jic23@...nel.org>, kernel test robot <lkp@...el.com>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Lars-Peter Clausen <lars@...afoo.de>,
oe-kbuild-all@...ts.linux.dev, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-iio@...r.kernel.org,
Yasin Lee <yasin.lee.x@...il.com>
Subject: Re: [PATCH v5 3/3] iio:proximity:hx9023s: Add TYHX HX9023S sensor
driver
On 2024/6/18 03:22, Jonathan Cameron wrote:
> On Mon, 17 Jun 2024 19:34:30 +0800
> kernel test robot <lkp@...el.com> wrote:
>
>> Hi Yasin,
>>
>> kernel test robot noticed the following build warnings:
>>
>> [auto build test WARNING on jic23-iio/togreg]
>> [also build test WARNING on robh/for-next linus/master v6.10-rc4 next-20240613]
>> [If your patch is applied to the wrong git tree, kindly drop us a note.
>> And when submitting patch, we suggest to use '--base' as documented in
>> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>>
>> url: https://github.com/intel-lab-lkp/linux/commits/Yasin-Lee/dt-bindings-iio-proximity-Add-hx9023s-binding/20240616-154122
>> base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
>> patch link: https://lore.kernel.org/r/SN7PR12MB8101D4BC788B5954608D677DA4CC2%40SN7PR12MB8101.namprd12.prod.outlook.com
>> patch subject: [PATCH v5 3/3] iio:proximity:hx9023s: Add TYHX HX9023S sensor driver
>> config: arm64-randconfig-r132-20240617 (https://download.01.org/0day-ci/archive/20240617/202406171946.qe83Tde0-lkp@intel.com/config)
>> compiler: aarch64-linux-gcc (GCC) 13.2.0
>> reproduce: (https://download.01.org/0day-ci/archive/20240617/202406171946.qe83Tde0-lkp@intel.com/reproduce)
>>
>> If you fix the issue in a separate patch/commit (i.e. not just a new version of
>> the same patch/commit), kindly add following tags
>> | Reported-by: kernel test robot <lkp@...el.com>
>> | Closes: https://lore.kernel.org/oe-kbuild-all/202406171946.qe83Tde0-lkp@intel.com/
>>
>> sparse warnings: (new ones prefixed by >>)
>>>> drivers/iio/proximity/hx9023s.c:955:44: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __be16 @@ got int diff @@
>> drivers/iio/proximity/hx9023s.c:955:44: sparse: expected restricted __be16
>> drivers/iio/proximity/hx9023s.c:955:44: sparse: got int diff
>>
>> vim +955 drivers/iio/proximity/hx9023s.c
>>
>> 931
>> 932 static irqreturn_t hx9023s_trigger_handler(int irq, void *private)
>> 933 {
>> 934 struct iio_poll_func *pf = private;
>> 935 struct iio_dev *indio_dev = pf->indio_dev;
>> 936 struct hx9023s_data *data = iio_priv(indio_dev);
>> 937 struct device *dev = regmap_get_device(data->regmap);
>> 938 int ret;
>> 939 unsigned int bit, i = 0;
>> 940
>> 941 guard(mutex)(&data->mutex);
>> 942 ret = hx9023s_sample(data);
>> 943 if (ret) {
>> 944 dev_warn(dev, "sampling failed\n");
>> 945 goto out;
>> 946 }
>> 947
>> 948 ret = hx9023s_get_prox_state(data);
>> 949 if (ret) {
>> 950 dev_warn(dev, "get prox failed\n");
>> 951 goto out;
>> 952 }
>> 953
>> 954 for_each_set_bit(bit, indio_dev->active_scan_mask, indio_dev->masklength)
>> > 955 data->buffer.channels[i++] = data->ch_data[indio_dev->channels[bit].channel].diff;
>> 956
> This looks very odd. Diff is an int filled with get_unaligned_le16()
> which you then write to a __be16 here.
>
> It should remain little endian, if that is appropriate, throughout.
>
> Also, very long line. Use a local variable for
> indio_dev->channels[bit].channel.
Hi Jonathan,
I reviewed the code and saw that data->buffer.channels[i] needs to be
filled with the MSB and LSB of the diff data register. I can read the
two bytes of diff data using regmap_bulk_read and fill
data->buffer.channels[i]. However, the diff data register in this chip
is multiplexed with the low pass data register. Thus, in some cases,
diff data can't be directly read and must be calculated as the
difference between low pass data and baseline data. Therefore, I can't
directly store the register value in data->buffer.channels[i]. I plan to
make the following changes to the code. Do you think this is feasible?
@@ -141,7 +141,7 @@ struct hx9023s_data {
bool trigger_enabled;
struct {
- __be16 channels[HX9023S_CH_NUM];
+ __le16 channels[HX9023S_CH_NUM];
s64 ts __aligned(8);
} buffer;
@@ -936,7 +936,7 @@ static irqreturn_t hx9023s_trigger_handler(int irq,
void *private)
struct hx9023s_data *data = iio_priv(indio_dev);
struct device *dev = regmap_get_device(data->regmap);
int ret;
- unsigned int bit, i = 0;
+ unsigned int bit, index, i = 0;
guard(mutex)(&data->mutex);
ret = hx9023s_sample(data);
@@ -951,8 +951,10 @@ static irqreturn_t hx9023s_trigger_handler(int irq,
void *private)
goto out;
}
- for_each_set_bit(bit, indio_dev->active_scan_mask,
indio_dev->masklength)
- data->buffer.channels[i++] =
data->ch_data[indio_dev->channels[bit].channel].diff;
+ for_each_set_bit(bit, indio_dev->active_scan_mask,
indio_dev->masklength) {
+ index = indio_dev->channels[bit].channel;
+ data->buffer.channels[i++] =
cpu_to_le16(data->ch_data[index].diff);
+ }
iio_push_to_buffers_with_timestamp(indio_dev, &data->buffer,
pf->timestamp);
Best regards,
Yasin Lee
>> 957 iio_push_to_buffers_with_timestamp(indio_dev, &data->buffer, pf->timestamp);
>> 958
>> 959 out:
>> 960 iio_trigger_notify_done(indio_dev->trig);
>> 961
>> 962 return IRQ_HANDLED;
>> 963 }
>> 964
>>
Powered by blists - more mailing lists