Hi Sakari,
On Fri, Jul 22, 2022 at 05:37:53PM +0300, Sakari Ailus wrote:
> Hi Alexander,
>
> On Thu, Jul 21, 2022 at 01:43:54PM +0200, Alexander Stein wrote:
> ...
> > Nice the following snippet does the trick already:
> > ---8<---
> > --- a/drivers/media/i2c/imx290.c
> > +++ b/drivers/media/i2c/imx290.c
> > @@ -221,6 +221,7 @@ static const struct imx290_pixfmt imx290_formats[] = {
> > static const struct regmap_config imx290_regmap_config = {
> > .reg_bits = 16,
> > .val_bits = 8,
> > + .use_single_read = true,
> > };
> >
> > static const char * const imx290_test_pattern_menu[] = {
> > ---8<---
> >
> > As this affects the VC OV9281 as well, any suggestions for a common property?
>
> If there's a 1:1 I²C mux in there between the host and the sensor, should
> it be in DT as well? I'm not entirely certain it's necessary.
The microcontroller also the sensor clock and power supplies, so it has
to be modelled in DT in any case. I was trying to avoid exposing it as
an I2C mux, but maybe we'll have to bite the bullet...
I've implement support for two camera modules from Vision Components but
haven't submitted patches yet. See [1] and [2] for DT examples and [3]
for the driver that handles the microcontroller.
Note that one purpose of the microcontroller is to configure the sensor
automatically. It can be queried through I2C for a list of supported
modes, and it can also reconfigure the sensor fully when a mode is
selected. This is meant to enable development of a single driver that
will cover all modules, regardless of which camera sensor it integrates.
I'm not sure what words you will use to voice your opinion on this
design, but I think I already agree :-)
[1] https://gitlab.com/ideasonboard/nxp/linux/-/blob/pinchartl/v5.19/dev/isp/next/arch/arm64/boot/dts/freescale/imx8mp-maivin-csi1-imx296.dts
[2] https://gitlab.com/ideasonboard/nxp/linux/-/blob/pinchartl/v5.19/dev/isp/next/arch/arm64/boot/dts/freescale/imx8mp-maivin-csi1-imx327.dts
[3] https://gitlab.com/ideasonboard/nxp/linux/-/blob/pinchartl/v5.19/dev/isp/next/drivers/media/i2c/vc-mipi.c
> The property could be called e.g. "single-octet-read". I think this should
> probably be documented in I²C bindings (or even regmap).
I like the idea of making it a DT property global to all I2C devices. It
should ideally be parsed by the I2C core or by regmap.
--
Regards,
Laurent Pinchart
