Hi Krzysztof
On Fri, Oct 07, 2022 at 07:20:51AM +0200, Krzysztof Hałasa wrote:
> Hi Jacopo and Co,
>
> Jacopo Mondi <jacopo@xxxxxxxxxx> writes:
>
> > +static int ar0521_read_reg(struct ar0521_dev *sensor, u16 reg, u16 *val)
> > +{
> > + struct i2c_client *client = sensor->i2c_client;
> > + unsigned char buf[2];
> > + struct i2c_msg msg;
> > + int ret;
> > +
> > + msg.addr = client->addr;
> > + msg.flags = client->flags;
> > + msg.len = sizeof(u16);
> > + msg.buf = buf;
> > + put_unaligned_be16(reg, buf);
> > +
> > + ret = i2c_transfer(client->adapter, &msg, 1);
> > + if (ret < 0)
> > + return ret;
> > +
> > + msg.len = sizeof(u16);
> > + msg.flags = client->flags | I2C_M_RD;
> > + msg.buf = buf;
> > +
> > + ret = i2c_transfer(client->adapter, &msg, 1);
> > + if (ret < 0)
> > + return ret;
> > +
> > + *val = get_unaligned_be16(buf);
> > +
> > + return 0;
> > +}
>
> Why not simply use a shadow register?
>
Sorry I didn't get you. Care to expand ?
> > +static int ar0521_set_analog_gain(struct ar0521_dev *sensor)
> > +{
> > + u16 global_gain;
> > + int ret;
> > +
> > + ret = ar0521_read_reg(sensor, AR0521_REG_GLOBAL_GAIN, &global_gain);
> > + if (ret)
> > + return ret;
> > +
> > + global_gain &= ~AR0521_REG_GLOBAL_GAIN_ANA_MASK;
> > + global_gain |= sensor->ctrls.ana_gain->val & AR0521_REG_GLOBAL_GAIN_ANA_MASK;
> > +
> > + return ar0521_write_reg(sensor, AR0521_REG_GLOBAL_GAIN, global_gain);
>
> This one is simple: you can't write to AR0521_REG_GLOBAL_GAIN.
Uh
I can guarantee you it works :)
> You can write to individual color gain registers (any will do for analog
> gain), but writing to AR0521_REG_GLOBAL_GAIN will reset all the digital
> gains as well. Reading the register doesn't give you anything
I think that's ok, isn't it ? If one wants to control the global gain
it goes through this register, if individual gains need to be
configured one should not set the global gain ?
> interesting, either (the analog gain which you overwrite anyway).
The high bits are the global digital gain, and I need to read its value in
order not to overwrite them.
>
> BTW ISP can't really do that color balancing for you, since the sensor
> operates at its native bit resolution and ISP is limited to the output
> format, which is currently only 8-bit.
I'm not sure what do you mean here either :)
> --
> Krzysztof "Chris" Hałasa
>
> Sieć Badawcza Łukasiewicz
> Przemysłowy Instytut Automatyki i Pomiarów PIAP
> Al. Jerozolimskie 202, 02-486 Warszawa
