[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a0677c8f-08f6-4b0d-8393-8086972475f2@wanadoo.fr>
Date: Sun, 11 May 2025 09:57:22 +0200
From: Christophe JAILLET <christophe.jaillet@...adoo.fr>
To: waqar.hameed@...s.com
Cc: jic23@...nel.org, kernel@...s.com, lars@...afoo.de,
linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] iio: Add driver for Nicera D3-323-AA PIR sensor
Le 09/05/2025 à 17:03, Waqar Hameed a écrit :
> Nicera D3-323-AA is a PIR sensor for human detection. It has support for
> raw data measurements and detection notification. The communication
> protocol is custom made and therefore needs to be GPIO bit banged.
>
> The device has two main settings that can be configured: a threshold
> value for detection and a band-pass filter. The configurable parameters
> for the band-pass filter are the high-pass and low-pass cutoff
> frequencies and its peak gain. Map these settings to the corresponding
> parameters in the `iio` framework.
>
> The low-pass and high-pass cutoff frequencies are pairwise for the
> different available filter types. Because of this, only allow to set the
> low-pass cutoff frequency from `sysfs` and use that to configure the
> corresponding high-pass cutoff frequency. This is sufficient to
> unambiguously choose a filter type.
>
> Raw data measurements can be obtained from the device. However, since we
> rely on bit banging, it will be rather cumbersome with buffer support.
> The main reason being that the data protocol has strict timing
> requirements (it's serial like UART), and it's mainly used during
> debugging since in real-world applications only the event notification
> is of importance. Therefore, only add support for events (for now).
>
> Signed-off-by: Waqar Hameed <waqar.hameed-VrBV9hrLPhE@...lic.gmane.org>
> ---
Hi,
...
> +static int d3323aa_set_lp_filter_freq(unsigned long *regmap, const int val,
> + int val2)
> +{
> + size_t idx;
> +
> + /* Truncate fractional part to one digit. */
> + val2 /= 100000;
> +
> + for (idx = 0; idx < ARRAY_SIZE(d3323aa_lp_filter_freq); ++idx) {
> + int integer = d3323aa_lp_filter_freq[idx][0] /
> + d3323aa_lp_filter_freq[idx][1];
> + int fract = d3323aa_lp_filter_freq[idx][0] %
> + d3323aa_lp_filter_freq[idx][1];
Missing newline.
> + if (val == integer && val2 == fract)
> + break;
> + }
> +
> + if (idx == ARRAY_SIZE(d3323aa_lp_filter_freq))
> + return -ERANGE;
> +
> + bitmap_write(regmap, d3323aa_lp_filter_regval[idx],
> + D3323AA_REG_BIT_FILSEL0, D3323AA_FILTER_TYPE_NR_BITS);
> +
> + return 0;
> +}
...
> +static int d3323aa_probe(struct platform_device *pdev)
> +{
> + DECLARE_BITMAP(default_regmap, D3323AA_REG_NR_BITS);
> + struct d3323aa_data *data;
> + struct iio_dev *indio_dev;
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*data));
> + if (!indio_dev)
> + return dev_err_probe(&pdev->dev, -ENOMEM,
> + "Could not allocate iio device\n");
> +
> + data = iio_priv(indio_dev);
> + data->dev = &pdev->dev;
> + platform_set_drvdata(pdev, indio_dev);
> +
> + init_completion(&data->reset_completion);
> +
> + ret = devm_mutex_init(data->dev, &data->regmap_lock);
> + if (ret)
> + return dev_err_probe(data->dev, ret,
> + "Could not initialize mutex (%d)\n", ret);
No need to repeat the error code in the message, when using dev_err_probe().
Same for all calls below.
> +
> + /* Request GPIOs. */
> + data->gpiod_vdd = devm_gpiod_get(data->dev, "vdd", GPIOD_OUT_LOW);
> + if (IS_ERR(data->gpiod_vdd))
> + return dev_err_probe(data->dev, PTR_ERR(data->gpiod_vdd),
> + "Could not get GPIO vdd (%ld)\n",
> + PTR_ERR(data->gpiod_vdd));
> +
> + data->gpiod_clk_vout =
> + devm_gpiod_get(data->dev, "clk-vout", GPIOD_OUT_LOW);
> + if (IS_ERR(data->gpiod_clk_vout))
> + return dev_err_probe(data->dev, PTR_ERR(data->gpiod_clk_vout),
> + "Could not get GPIO clk-vout (%ld)\n",
> + PTR_ERR(data->gpiod_clk_vout));
> +
> + data->gpiod_data = devm_gpiod_get(data->dev, "data", GPIOD_OUT_LOW);
> + if (IS_ERR(data->gpiod_data))
> + return dev_err_probe(data->dev, PTR_ERR(data->gpiod_data),
> + "Could not get GPIO data (%ld)\n",
> + PTR_ERR(data->gpiod_data));
> +
> + ret = gpiod_to_irq(data->gpiod_clk_vout);
> + if (ret < 0)
> + return dev_err_probe(data->dev, ret, "Could not get IRQ (%d)\n",
> + ret);
> +
> + data->irq = ret;
> +
> + /* Do one setup with the default values. */
> + bitmap_zero(default_regmap, D3323AA_REG_NR_BITS);
> + d3323aa_set_threshold(default_regmap, D3323AA_THRESH_DEFAULT_VAL);
> + d3323aa_set_filter_gain(default_regmap,
> + D3323AA_FILTER_GAIN_DEFAULT_VAL);
> + ret = d3323aa_setup(indio_dev, default_regmap);
> + if (ret)
> + return ret;
> +
> + indio_dev->info = &d3323aa_info;
> + indio_dev->name = D3323AA_DRV_NAME;
> + indio_dev->channels = d3323aa_channels;
> + indio_dev->num_channels = ARRAY_SIZE(d3323aa_channels);
> +
> + ret = devm_iio_device_register(data->dev, indio_dev);
> + if (ret)
> + return dev_err_probe(data->dev, ret,
> + "Could not register iio device (%d)\n",
> + ret);
> +
> + return 0;
> +}
...
CJ
Powered by blists - more mailing lists