Hi Umang and Laurent On Fri, 23 Jul 2021 at 12:52, Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> wrote: > > On Fri, Jul 23, 2021 at 02:44:40PM +0300, Laurent Pinchart wrote: > > Hi Umang, > > > > Thank you for the patch. > > > > CC'ing Sakari. For future kernel patches, you can use the > > ./scripts/get_maintainer.pl script in the kernel sources to get a list > > of appropriate recipients. The list should be taken with a grain of salt > > though, it has a tendency to return too many recipients. For this > > particular patch, for instance, it also recommends Mauro and LKML. > > Whether to CC the subsystem maintainer on every patch depends on the > > subsystem (it's more common for small ones than big ones) and on the > > maintainer's preferences. LKML is a catch-all mailing list with very > > high traffic, and when more appropriate venues exist for patches, I > > usually recommend skipping LKML. > > And expanding the CC list further to include Dave (for his contribution > to the discussion), and Krzysztof and Bingbu (for their contributions to > the driver, as reported by git log). > > > On Fri, Jul 23, 2021 at 04:52:33PM +0530, Umang Jain wrote: > > > The range for analog gain mentioned in the datasheet is [0, 480]. > > > The real gain formula mentioned in the datasheet is: > > > > > > Gain = 512 / (512 – X) > > > > > > Hence, values larger than 511 clearly makes no sense. The gain > > > register field is also documented to be of 9-bits in the datasheet. > > > > > > Certainly, it is enough to infer that, the kernel driver currently > > > advertises an arbitrary analog gain max. Fix it by rectifying the > > > value as per the data sheet i.e. 480. > > > > > > Signed-off-by: Umang Jain <umang.jain@xxxxxxxxxxxxxxxx> > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> That certainly follows the datasheet that I have. Gains up to code 480 work as expected. Up to 496 seems to work, but going beyond that causes issues. Adopting the documented maximum value is the safest approach. Reviewed-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx> > > > > > --- > > > drivers/media/i2c/imx258.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c > > > index 4e695096e5d0..81cdf37216ca 100644 > > > --- a/drivers/media/i2c/imx258.c > > > +++ b/drivers/media/i2c/imx258.c > > > @@ -47,7 +47,7 @@ > > > /* Analog gain control */ > > > #define IMX258_REG_ANALOG_GAIN 0x0204 > > > #define IMX258_ANA_GAIN_MIN 0 > > > -#define IMX258_ANA_GAIN_MAX 0x1fff > > > +#define IMX258_ANA_GAIN_MAX 480 > > > #define IMX258_ANA_GAIN_STEP 1 > > > #define IMX258_ANA_GAIN_DEFAULT 0x0 > > > > > -- > Regards, > > Laurent Pinchart
