On Wed, Nov 29, 2023 at 09:04:49AM +0200, Petre Rodan wrote:
> Adds driver for digital Honeywell TruStability HSC and SSC series
> pressure and temperature sensors.
> Communication is one way. The sensor only requires 4 bytes worth of
> clock pulses on both i2c and spi in order to push the data out.
> The i2c address is hardcoded and depends on the part number.
> There is no additional GPIO control.
Thank you for an update!
Much better now, only one important thing is left unaddressed, see below.
...
> + if (strncmp(triplet, "NA", 2) == 0) {
str_has_prefix()
...
> + for (index = 0; index < ARRAY_SIZE(hsc_range_config); index++) {
> + if (strncmp(hsc_range_config[index].triplet,
> + triplet,
> + HSC_PRESSURE_TRIPLET_LEN - 1) == 0) {
> + hsc->pmin = hsc_range_config[index].pmin;
> + hsc->pmax = hsc_range_config[index].pmax;
> + found = 1;
> + break;
> + }
> + }
> + if (hsc->pmin == hsc->pmax || !found)
> + return dev_err_probe(dev, -EINVAL,
> + "honeywell,pressure-triplet is invalid\n");
This one is important. I think I told already twice that this is NIH
device_property_match_property_string(). Please, use this API directly.
...
> + tmp = div_s64(((s64)(hsc->pmax - hsc->pmin)) * MICRO,
> + hsc->outmax - hsc->outmin);
> + hsc->p_scale = div_s64_rem(tmp, NANO, &hsc->p_scale_dec);
> + tmp = div_s64(((s64)hsc->pmin * (s64)(hsc->outmax - hsc->outmin)) *
> + MICRO, hsc->pmax - hsc->pmin);
I would leave MICRO on the previous line for better understanding the code.
...
> +#ifndef _HSC030PA_H
> +#define _HSC030PA_H
> +#include <linux/mutex.h>
> +#include <linux/property.h>
Is not used here.
> +#include <linux/types.h>
...
> +/*
> + * get all conversions (4 bytes) in one go
> + * since transfers are not address-based
> +*/
Missing space at the last line and missing capitalization and grammar period.
...
> +int hsc_common_probe(struct device *dev, void *client,
> + hsc_recv_fn recv_fn, const char *name);
Indentation is unusual on the second line, also you can use just "recv" as
parameter name. But both are minor and I leave them to you and maintainer.
...
> +static const struct of_device_id hsc_spi_match[] = {
> + {.compatible = "honeywell,hsc030pa"},
I believe Jonathan asks for inner spaces, like { ...foo... }.
> + {}
> +};
...
> +static const struct spi_device_id hsc_spi_id[] = {
> + {"hsc030pa"},
Ditto.
> + {}
> +};
--
With Best Regards,
Andy Shevchenko
