On Wed, Aug 11, 2021 at 10:03 AM Andy Shevchenko
[off-list ref] wrote:

On Wed, Aug 11, 2021 at 8:46 AM Mugilraj Dhavachelvan
[off-list ref] wrote:

quoted

On Tue, Aug 10, 2021 at 03:49:52PM +0300, Andy Shevchenko wrote:

quoted

On Mon, Aug 9, 2021 at 10:59 AM Mugilraj Dhavachelvan
[off-list ref] wrote:

...

quoted

quoted

quoted

+       data->tol = data->cfg->kohms * (val & GENMASK(6, 0)) * 10 / 8;
+       if (!(val & BIT(7)))
+               data->tol *= -1;

Shouldn't you simple use corresponding sign_extend*()?

I'm not able see any sign_extend for 16 bit. Is there any other way?

So, then add it in bitops.h the same way it's done for s32 and s64.

In case you are afraid that this will delay patches and you are in a
hurry, you may define it locally (but in the very same way, so
somebody can make it generic). Nevertheless, I think this one can be
added directly to the generic header, it will definitely have more
than one user.

...

quoted

quoted

quoted

+       if (!data->cfg)
+               data->cfg = &ad5110_cfg[i2c_match_id(ad5110_id, client)->driver_data];

Not sure this is not a dead code since you are using ->probe_new().

Even I'm suspecting that and also removing id_table. But I'm not sure of
it so just left as it is.

I²C ID table is good to have without direct use, but ->probe_new() is
called if and only if there is a compatible string or ACPI ID match.
In such case data->cfg may be NULL if and only if the corresponding
table missed it, but this will be a bug anyway, so the above code will
rather hide the bug. Hence, please remove it.

-- 
With Best Regards,
Andy Shevchenko