Hi,
On 09/12/2024 11:11, Sakari Ailus wrote:
Huomenta,
On Fri, Dec 06, 2024 at 10:26:40AM +0200, 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.
Cc: stable@vger.kernel.org Fixes: afe267f2d368 ("media: i2c: add DS90UB960 driver") Reviewed-by: Jai Luthra jai.luthra@ideasonboard.com Signed-off-by: Tomi Valkeinen tomi.valkeinen@ideasonboard.com
drivers/media/i2c/ds90ub960.c | 90 ++++++++++++++++++++++++------------------- 1 file changed, 50 insertions(+), 40 deletions(-)
diff --git a/drivers/media/i2c/ds90ub960.c b/drivers/media/i2c/ds90ub960.c index 24198b803eff..94c8acf171b4 100644 --- a/drivers/media/i2c/ds90ub960.c +++ b/drivers/media/i2c/ds90ub960.c @@ -2950,6 +2950,54 @@ static const struct v4l2_subdev_pad_ops ub960_pad_ops = { .set_fmt = ub960_set_fmt, }; +static void ub960_log_status_ub960_sp_eq(struct ub960_data *priv,
unsigned int nport)+{
- struct device *dev = &priv->client->dev;
- u8 eq_level;
- s8 strobe_pos;
- u8 v = 0;
- /* Strobe */
- ub960_read(priv, UB960_XR_AEQ_CTL1, &v);
How about adding __must_check to the ub960_read()?
Actually, this is just moving code around (behind an if), so I'd rather not add more to this patch, especially as this is a fix.
We'll add the error handling separately on top.
Tomi
- dev_info(dev, "\t%s strobe\n",
(v & UB960_XR_AEQ_CTL1_AEQ_SFILTER_EN) ? "Adaptive" :"Manual");- if (v & UB960_XR_AEQ_CTL1_AEQ_SFILTER_EN) {
ub960_read(priv, UB960_XR_SFILTER_CFG, &v);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);- }
- ub960_rxport_get_strobe_pos(priv, nport, &strobe_pos);
- dev_info(dev, "\tStrobe pos %d\n", strobe_pos);
- /* EQ */
- ub960_rxport_read(priv, nport, UB960_RR_AEQ_BYPASS, &v);
- dev_info(dev, "\t%s EQ\n",
(v & UB960_RR_AEQ_BYPASS_ENABLE) ? "Manual" :"Adaptive");- if (!(v & UB960_RR_AEQ_BYPASS_ENABLE)) {
ub960_rxport_read(priv, nport, UB960_RR_AEQ_MIN_MAX, &v);dev_info(dev, "\tEQ range [%u, %u]\n",(v >> UB960_RR_AEQ_MIN_MAX_AEQ_FLOOR_SHIFT) & 0xf,(v >> UB960_RR_AEQ_MIN_MAX_AEQ_MAX_SHIFT) & 0xf);- }
- if (ub960_rxport_get_eq_level(priv, nport, &eq_level) == 0)
dev_info(dev, "\tEQ level %u\n", eq_level);+}
- static int ub960_log_status(struct v4l2_subdev *sd) { struct ub960_data *priv = sd_to_ub960(sd);
@@ -2997,8 +3045,6 @@ static int ub960_log_status(struct v4l2_subdev *sd) for (nport = 0; nport < priv->hw_data->num_rxports; nport++) { struct ub960_rxport *rxport = priv->rxports[nport];
u8 eq_level;s8 strobe_pos;dev_info(dev, "RX %u\n", nport); @@ -3034,44 +3080,8 @@ static int ub960_log_status(struct v4l2_subdev *sd) ub960_rxport_read(priv, nport, UB960_RR_CSI_ERR_COUNTER, &v); dev_info(dev, "\tcsi_err_counter %u\n", v);
/* Strobe */ub960_read(priv, UB960_XR_AEQ_CTL1, &v);dev_info(dev, "\t%s strobe\n",(v & UB960_XR_AEQ_CTL1_AEQ_SFILTER_EN) ? "Adaptive" :"Manual");if (v & UB960_XR_AEQ_CTL1_AEQ_SFILTER_EN) {ub960_read(priv, UB960_XR_SFILTER_CFG, &v);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);}ub960_rxport_get_strobe_pos(priv, nport, &strobe_pos);dev_info(dev, "\tStrobe pos %d\n", strobe_pos);/* EQ */ub960_rxport_read(priv, nport, UB960_RR_AEQ_BYPASS, &v);dev_info(dev, "\t%s EQ\n",(v & UB960_RR_AEQ_BYPASS_ENABLE) ? "Manual" :"Adaptive");if (!(v & UB960_RR_AEQ_BYPASS_ENABLE)) {ub960_rxport_read(priv, nport, UB960_RR_AEQ_MIN_MAX, &v);dev_info(dev, "\tEQ range [%u, %u]\n",(v >> UB960_RR_AEQ_MIN_MAX_AEQ_FLOOR_SHIFT) & 0xf,(v >> UB960_RR_AEQ_MIN_MAX_AEQ_MAX_SHIFT) & 0xf);}if (ub960_rxport_get_eq_level(priv, nport, &eq_level) == 0)dev_info(dev, "\tEQ level %u\n", eq_level);
if (!priv->hw_data->is_ub9702)ub960_log_status_ub960_sp_eq(priv, nport);/* GPIOs */ for (i = 0; i < UB960_NUM_BC_GPIOS; i++) {