On Tue, Apr 21, 2020 at 10:39 PM Ivan Mikhaylov <i.mikhaylov@xxxxxxxxx> wrote:
>
> Proximity sensor driver based on light/vcnl4000.c code.
> For now supports only the single on-demand measurement.
>
> The VCNL3020 is a fully integrated proximity sensor. Fully
> integrated means that the infrared emitter is included in the
> package. It has 16-bit resolution. It includes a signal
> processing IC and features standard I2C communication
> interface. It features an interrupt function.
...
> +static int vcnl3020_get_and_apply_property(struct vcnl3020_data *data,
> + const char *prop, u32 reg)
> +{
> + int rc;
> + u32 val;
> +
> + rc = device_property_read_u32(data->dev, prop, &val);
> + if (rc)
> + return 0;
> +
> + /* An example of conversion from uA to reg val:
> + * 200000 uA == 200 mA == 20
> + */
> + if (!strcmp(prop, "vishay,led-current-microamp"))
> + val /= 10000;
I probably missed the point why this function is needed at all, since
we always call only for a single property.
On top of that, why do we have this nasty strcmp()? Can't we simple do
something like
static int vcnl3020_get_and_apply_property(struct vcnl3020_data *data,
const char *prop, u32 reg, u32 div)
{
...
val /= div;
...
}
static int vcnl3020_get_and_apply_led_current_property(struct
vcnl3020_data *data)
{
/*
* An example of conversion from uA to reg val:
* 200000 uA == 200 mA == 20
*/
// Note by the way comments style
return vcnl3020_get_and_apply_property(data, "vishay,led-current-microamp",
VCNL_LED_CURRENT, 10000);
}
?
> + rc = regmap_write(data->regmap, reg, val);
> + if (rc) {
> + dev_err(data->dev, "Error (%d) setting property (%s)\n",
> + rc, prop);
> + }
> +
> + return rc;
> +}
--
With Best Regards,
Andy Shevchenko
