Re: [PATCH v5 3/3] iio:proximity:hx9023s: Add TYHX HX9023S sensor driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 2024/6/18 03:22, Jonathan Cameron wrote:
On Mon, 17 Jun 2024 19:34:30 +0800
kernel test robot <lkp@xxxxxxxxx> 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@xxxxxxxxx/config)
compiler: aarch64-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20240617/202406171946.qe83Tde0-lkp@xxxxxxxxx/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@xxxxxxxxx>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406171946.qe83Tde0-lkp@xxxxxxxxx/

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	





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux