From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Svyatoslav Ryhel <clamor95@gmail.com>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>,
Mauro Carvalho Chehab <mchehab@kernel.org>,
Rob Herring <robh@kernel.org>,
Krzysztof Kozlowski <krzk+dt@kernel.org>,
Conor Dooley <conor+dt@kernel.org>,
linux-media@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1 2/2] media: i2c: mt9m114: add support for Aptina MI1040
Date: Wed, 28 Jan 2026 15:40:15 +0200 [thread overview]
Message-ID: <20260128134015.GD3210848@killaraus> (raw)
In-Reply-To: <CAPVz0n3mG79Q0E=F5a3iSdd9TcVXFXuLj9vFkX3if2zVzpR=eg@mail.gmail.com>
On Mon, Jan 26, 2026 at 01:50:13PM +0200, Svyatoslav Ryhel wrote:
> пн, 26 січ. 2026 р. о 12:05 Laurent Pinchart name:
> > On Mon, Jan 26, 2026 at 11:50:05AM +0200, Svyatoslav Ryhel wrote:
> > > пн, 26 січ. 2026 р. о 11:35 Sakari Ailus <sakari.ailus@linux.intel.com> пише:
> > > > On Mon, Jan 26, 2026 at 10:34:30AM +0200, Svyatoslav Ryhel wrote:
> > > > > Slightly different version of MT9M114 camera module is used in a several
> > > > > devices like ASUS Nexus 7 (2012) or ASUS Transformer Prime TF201 and is
> > > > > called Aptina MI1040. Only difference found so far is lacking ability to
> > > >
> > > > s/Only/The only/
> > > >
> > > > > poll STATUS and COMMAND registers during power on sequence, which causes
> > > > > driver to fail with time out error. Add polling flag to diverge models and
> > > > > address quirk found in MI1040.
> > > > >
> > > > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > > > ---
> > > > > drivers/media/i2c/mt9m114.c | 35 ++++++++++++++++++++++++++++-------
> > > > > 1 file changed, 28 insertions(+), 7 deletions(-)
> > > > >
> > > > > diff --git a/drivers/media/i2c/mt9m114.c b/drivers/media/i2c/mt9m114.c
> > > > > index 4ec033c0ee84..d96a57ebcad4 100644
> > > > > --- a/drivers/media/i2c/mt9m114.c
> > > > > +++ b/drivers/media/i2c/mt9m114.c
> > > > > @@ -368,6 +368,10 @@ enum {
> > > > > * Data Structures
> > > > > */
> > > > >
> > > > > +struct mt9m114_model_info {
> > > > > + bool polling;
> > > > > +};
> > > > > +
> > > > > enum mt9m114_format_flag {
> > > > > MT9M114_FMT_FLAG_PARALLEL = BIT(0),
> > > > > MT9M114_FMT_FLAG_CSI2 = BIT(1),
> > > > > @@ -421,6 +425,8 @@ struct mt9m114 {
> > > > >
> > > > > struct v4l2_ctrl *tpg[4];
> > > > > } ifp;
> > > > > +
> > > > > + const struct mt9m114_model_info *info;
> > > > > };
> > > > >
> > > > > /* -----------------------------------------------------------------------------
> > > > > @@ -2186,9 +2192,11 @@ static int mt9m114_power_on(struct mt9m114 *sensor)
> > > > > */
> > > > > usleep_range(44500, 50000);
> > > > >
> > > > > - ret = mt9m114_poll_command(sensor, MT9M114_COMMAND_REGISTER_SET_STATE);
> > > > > - if (ret < 0)
> > > > > - goto error_clock;
> > > > > + if (sensor->info->polling) {
> > > > > + ret = mt9m114_poll_command(sensor, MT9M114_COMMAND_REGISTER_SET_STATE);
> > > > > + if (ret < 0)
> > > > > + goto error_clock;
> > > > > + }
> > > >
> > > > What does the datasheet say, is there a need to do something else instead?
> > > > As the polling is there to ensure firmware has done its job, the need
> > > > appears to still be there.
> > >
> > > MI1040 has no datasheet available and downstream code does not do this
> > > polling. I have tested on Nexus 7 which has this camera and it seems
> > > to be fully operational without this poling, but as soon it is enabled
> > > camera fails will timeout. I suspect that this camera version has some
> > > quirk regarding early access, but I cannot back it up by any
> > > documentation or additional data.
> > >
> > > I have a device with proper version of mt9m114 too and it works with
> > > his driver without any major issues.
> >
> > Does the device reply to reads of the MT9M114_COMMAND_REGISTER register
> > but never shows the MT9M114_COMMAND_REGISTER_SET_STATE bit being set, or
> > does it not reply to reads at all (timeouts on the I2C bus) ?
>
> I have re-run check on Nexus 7 and TF201 which both have mi1040 cam.
>
> From data I have got:
>
> "mt9m114_poll_command(sensor, MT9M114_COMMAND_REGISTER_SET_STATE);"
>
> passes fine, which is good. Previous time if failed with Timeout
> waiting for state and -ETIMEDOUT. But
>
> "mt9m114_poll_state(sensor, MT9M114_SYS_STATE_STANDBY);"
>
> still gives:
>
> [ 2.921791] mt9m114 2-0048: Timeout waiting for state 0x52
> [ 2.922777] mt9m114 2-0048: error -ETIMEDOUT: Could not power on the device
>
> commenting mt9m114_poll_state made camera work, I was able to take pictures.
>
> If you have any suggestions for testing, let me know.
Does increasing the timeout help ? I'm wondering if this patch could
just work by luck, with the sensor no being ready quickly enough for the
timeout used by mt9m114_poll_command(), but hvaing time to get ready
after power on because the driver doesn't start streaming immediately.
> Additionally, not related to this patch but to mt9m114 device tree
> reflection. The MT9M114 driver exposes the IFP device's sink pad
> first, which causes issues for external devices that rely on the OF
> graph to get the correct pad information. Laurent, may you consider
> swapping sink and source pads of IFP so that source pad is set first
> and correspond to device tree port or at least if you and media
> subsystem maintainers are fine with such change?
That risks breaking existing userspace applications.
The issue may be that the IFP entity doesn't implement the
.get_fwnode_pad() operation. You will also need to make sure that the
downstream driver calls media_entity_get_fwnode_pad() instead of
assuming the pad number to be 0. Could you give that a try ?
--
Regards,
Laurent Pinchart
next prev parent reply other threads:[~2026-01-28 13:40 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-26 8:34 [PATCH v1 0/2] media: i2c: mt9m114: add support for Aptina MI1040 Svyatoslav Ryhel
2026-01-26 8:34 ` [PATCH v1 1/2] dt-bindings: media: mt9m114: document MI1040 sensor Svyatoslav Ryhel
2026-01-26 20:12 ` Conor Dooley
2026-01-26 8:34 ` [PATCH v1 2/2] media: i2c: mt9m114: add support for Aptina MI1040 Svyatoslav Ryhel
2026-01-26 9:35 ` Sakari Ailus
2026-01-26 9:50 ` Svyatoslav Ryhel
2026-01-26 10:05 ` Laurent Pinchart
2026-01-26 11:50 ` Svyatoslav Ryhel
2026-01-28 13:40 ` Laurent Pinchart [this message]
2026-01-28 14:56 ` Svyatoslav Ryhel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260128134015.GD3210848@killaraus \
--to=laurent.pinchart@ideasonboard.com \
--cc=clamor95@gmail.com \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=krzk+dt@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=mchehab@kernel.org \
--cc=robh@kernel.org \
--cc=sakari.ailus@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.