Re: [PATCH 04/13] media: i2c: ds90ub960: Fix logging SP & EQ status only for UB9702

[Andy Shevchenko]

On Fri, Oct 04, 2024 at 05:46:35PM +0300, Tomi Valkeinen wrote:
> UB9702 does not have SP and EQ registers, but the driver uses them in
> log_status(). Fix this by separating the SP and EQ related log_status()
> work into a separate function (for clarity) and calling that function
> only for UB960.

...

> +		dev_info(dev, "\tStrobe range [%d, %d]\n",
> +			 ((v >> UB960_XR_SFILTER_CFG_SFILTER_MIN_SHIFT) & 0xf) -
> +				 7,
> +			 ((v >> UB960_XR_SFILTER_CFG_SFILTER_MAX_SHIFT) & 0xf) -
> +				 7);

I believe the code is much more readable if those 7:s moved to the previous
lines. Btw, does driver use bitfield.h? And why not?


...

> -			dev_info(dev, "\tStrobe range [%d, %d]\n",
> -				 ((v >> UB960_XR_SFILTER_CFG_SFILTER_MIN_SHIFT) & 0xf) - 7,
> -				 ((v >> UB960_XR_SFILTER_CFG_SFILTER_MAX_SHIFT) & 0xf) - 7);

It will be even shorter in the new code!

-- 
With Best Regards,
Andy Shevchenko