Messages in this thread | | | Subject | Re: [PATCH v5] media: Driver for Toshiba et8ek8 5MP sensor | From | Ivaylo Dimitrov <> | Date | Wed, 14 Dec 2016 17:52:59 +0200 |
| |
Hi
On 14.12.2016 15:03, Pali Rohár wrote: > Hi! See inlined some my notes. >
>> + >> +#ifdef USE_CRC >> + rval = et8ek8_i2c_read_reg(client, ET8EK8_REG_8BIT, 0x1263, &val); >> + if (rval) >> + goto out; >> +#if USE_CRC /* TODO get crc setting from DT */ >> + val |= BIT(4); >> +#else >> + val &= ~BIT(4); >> +#endif >> + rval = et8ek8_i2c_write_reg(client, ET8EK8_REG_8BIT, 0x1263, val); >> + if (rval) >> + goto out; >> +#endif > > USE_CRC is defined to 1. Do we need that #ifdef check at all? > > What with above TODO? > >> +
It becomes a bit more complicated :) - on n900, front camera doesn't use CRC, while back camera does. Right now there is no way, even if we use video bus switch driver, to tell ISP to have 2 ports with different settings, not only for CRC, but for strobe, etc. Look at that commit https://github.com/freemangordon/linux-n900/commit/e5582fa56bbc0161d6c567157df42433829ee4de. What I think here is that ISP should take settings from the remote endpoints rather from it's local port. So far it does not.
So, until there is a way for ISP to have more than one CCP channel with different settings, I can't think of anything we can do here but to place TODO.
Ivo
| |