Hi Benjamin. On Thu, 12 May 2022 at 13:11, Benjamin Marty <info@xxxxxxxxxxxxxxxx> wrote: > > This bit is marked as reserved in the ADV Hardware Reference Manual. > > Resetting this bit seems to cause increased video noise. Setting this > bit according to the Hardware Reference Manual reduces the video noise > immediately. > > Signed-off-by: Benjamin Marty <info@xxxxxxxxxxxxxxxx> > --- > version 2: > - Fixed Kieran's remarks > > drivers/media/i2c/adv7180.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c > index 4f5db195e66d..992111fe249e 100644 > --- a/drivers/media/i2c/adv7180.c > +++ b/drivers/media/i2c/adv7180.c > @@ -43,6 +43,7 @@ > #define ADV7180_INPUT_CONTROL_INSEL_MASK 0x0f > > #define ADV7182_REG_INPUT_VIDSEL 0x0002 > +#define ADV7182_REG_INPUT_RESERVED BIT(2) Responding to Kieran's comment on V1: > If the bit is documented with a better name, then use that of course, > otherwise perhaps even a comment in the code saying that failing to set > the bit increases visible noise would be suitable. (or that setting the > bit reduces noise, I guess it depends on if you think this bit is > performing noise reduction, or if not setting it is introducing noise) I went digging through the datasheet for this info as I care about ADV728[0|1|2]M. https://www.analog.com/media/en/technical-documentation/data-sheets/ADV7182.pdf page 68 defines bits 0-3 as reserved, and "set to default" which is 0100b. https://www.analog.com/media/en/technical-documentation/user-guides/ADV7280_7281_7282_7283_UG-637.pdf page 70 says the same for ADV7280/ADV7281/ADV7282/ADV7283. So no name or detail in the docs over what the bits do. The patch does mean the driver more closely follows the datasheet, so it looks good to me. Reviewed-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx> I'll try to find a couple of minutes to get my hardware out and confirm I see the change in video noise. Dave > #define ADV7180_REG_OUTPUT_CONTROL 0x0003 > #define ADV7180_REG_EXTENDED_OUTPUT_CONTROL 0x0004 > @@ -1014,7 +1015,9 @@ static int adv7182_init(struct adv7180_state *state) > > static int adv7182_set_std(struct adv7180_state *state, unsigned int std) > { > - return adv7180_write(state, ADV7182_REG_INPUT_VIDSEL, std << 4); > + /* Failing to set the reserved bit can result in increased video noise */ > + return adv7180_write(state, ADV7182_REG_INPUT_VIDSEL, > + (std << 4) | ADV7182_REG_INPUT_RESERVED); > } > > enum adv7182_input_type { > -- > 2.36.1 >
