Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) client-ip=2620:137:e000::3:1;
From:   =?utf-8?Q?Krzysztof_Ha=C5=82asa?= <khalasa@piap.pl>
To:     Stefan Lengfeld <stefan@lengfeld.xyz>
Cc:     linux-media <linux-media@vger.kernel.org>,
        lkml <linux-kernel@vger.kernel.org>,
        Dave Stevenson <dave.stevenson@raspberrypi.com>,
        Oleksij Rempel <linux@rempel-privat.de>,
        Pengutronix Kernel Team <kernel@pengutronix.de>,
        Shawn Guo <shawnguo@kernel.org>,
        Sascha Hauer <s.hauer@pengutronix.de>,
        Fabio Estevam <festevam@gmail.com>,
        NXP Linux Team <linux-imx@nxp.com>, linux-i2c@vger.kernel.org
Subject: Re: Sony IMX290/462 image sensors I2C xfer peculiarity
References: <m3y1gpw8ri.fsf@t19.piap.pl>
        <CAPY8ntASwh3AcRqE+2zF4Df=u+=wJ5K9icAeOrXTMJGDd1+caw@mail.gmail.com>
        <m3o7hfx3ob.fsf@t19.piap.pl> <m37cnuvmhn.fsf@t19.piap.pl>
        <m3o7h5tthf.fsf@t19.piap.pl> <m3jzrttrmz.fsf@t19.piap.pl>
        <20231011101553.we3r73xejvqdql5j@porty> <m3fs2htn7g.fsf@t19.piap.pl>
        <20231012220127.GB27838@stcim.de>
Date:   Fri, 13 Oct 2023 12:39:09 +0200
In-Reply-To: <20231012220127.GB27838@stcim.de> (Stefan Lengfeld's message of
        "Fri, 13 Oct 2023 00:01:27 +0200")
Message-ID: <m3y1g6st6a.fsf@t19.piap.pl>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?eDlSNlBQNVZjSkZScXZMbnEvWFk3aDhUOHpnS1QwUGl5YWlUenFwakE4aEhH?=
 =?utf-8?B?ZnAyRjlHVWVUMEk3OWNLdHUvZDZhQ3dBbGgxQkdtcTdpYXZwZSs1aDZlOHNu?=
 =?utf-8?B?aVNHeUhJUDhFQ0trZitKc00rQVBOVGRBcThOb2l4elBMMWYyeWJLR2dDODV0?=
 =?utf-8?B?NkRJdWVkTit5OFlMTVp5ZTlCeDRGOUV4WVhSSE9TU05pQjlzem5MY1NYaHUv?=
 =?utf-8?B?SzNCQmcxMEV1SDdWS1k1UmFZMkx2ZEhOcE5GT3NKNDVESkhxWGdNODl1MkIx?=
 =?utf-8?B?Ujd4VkF2R1pzb1lrM1NEMHMrcnVVWmVSNnBuNGtwTVdsemN2S0dZOWQ0RzNQ?=
 =?utf-8?B?UDZvVWhBWXJGOGNOcVE3bXZnZ1NIS25lOWtFRzhWUVJEek01bzVTUG50VGlV?=
 =?utf-8?B?WVlqT2VGR2Zncm8zdGdRaTZ4QmZ4SHpjblR0Z1F0c3ZTSFlkYlQ3UW04Ym96?=
 =?utf-8?B?cDBXbXdKSzdHR3I3djJQZENDQ0xPS0xYdXNUWlRyMFhYODN1cG1lQXg0Q05w?=
 =?utf-8?B?N2g3UVVyNTNKRk9Hc2Q2VlhLQ3hrMERvUE9SVlN1TXdxd3FjOXdyU1ZpcS95?=
 =?utf-8?B?eEp1MDVwNXJNVWxlMW9vZU9nSElNdlVhK1VJclgxNSt6dUJxWS9ZV1ZVY1Bh?=
 =?utf-8?B?am03NEwyQ3dPNzlobGpKOWdYYjk3WXdCQ0JqOXdQTmM5S1d4ekViUWxFYUpO?=
 =?utf-8?B?alhibjZOMFNzVHFXWVl2R1FDbGp3d2E1cDlMRFZLOWtzY0dJWnZtVldYMld4?=
 =?utf-8?B?YWFsUDR2SmxtRFRzTjIyMjJDVXh3ckFJMWxvRy96cTdlWVRCRGlGZ1VQVlgz?=
 =?utf-8?B?b1o1NTJYN201RTl2eDZmdWQydFFrZkdFNmgxT0RSUmRmWDk4TTVaQmFQeE1m?=
 =?utf-8?B?TTQ0a09Hejh2S1kydG9EY3dHMXdWSDhlS2Q3eWg4MTdvWm5GN2dqTkxLYXpU?=
 =?utf-8?B?VHZndXNaMC9MQkxOYW1KOEo5ZjVsRlBIYnR4QXJxN2pMVGV1aS9XWk9COFFF?=
 =?utf-8?B?N3g3QVVnQURXWWtiQVVROU1BRGQ3RDFxeW16VERpTHEreWhFakJud2ZyazBi?=
 =?utf-8?B?MEM4OEorellmWXhuZ0RiamVLby94UDE4R0E2eFJLbk4rZGp3MVQ2T3RSOEdn?=
 =?utf-8?B?Rkt3SFNjWWJLaGllQjRSbzlxTWVMdW0wUVVZUzBMRitMVXhRb1hqem4xeVlz?=
 =?utf-8?B?NnFQcHM3aklMV1B2ZTVYRVlPUWljU0N6L0lTTGFpWmYvaGpsSnVkZ0hOYVRt?=
 =?utf-8?B?cUp2M3ZFb0dsU3ZTOEZYcnlab3JQWGZEeWNMQjF1SlNraGVBaUJZaDVkNGNq?=
 =?utf-8?B?NlRLdlZrN3pzNTdJaHUyOTlPUzcyM2p6MllYSE9SSHJPVUhXZit0dmtBcU41?=
 =?utf-8?B?cGw4MXYyZ3Zpekt2UmFjL05Tanh0MHJKNGFIbjJrakZhNGpXdGc2OFdGRCtv?=
 =?utf-8?B?SEFOc0YwVzVyOXRiWUdwMnJ2dzlIdm5LUHFFNHRReGh2RWhNcEtjcWoxN09w?=
 =?utf-8?B?b1ZZYUNTbWovTFZXMkhUMXNTcW1tWjcyTXBIM2RCZlNXSW1GVlBmSzJtL0Rx?=
 =?utf-8?B?SFZSdDBtT2JSeXNiTi9RaSsxdE8yZzFaOHFiMGxDeEUwQW1GZWZGa2VxSDJl?=
 =?utf-8?B?cDVFWFpnR0YvUkV0QmhJT3lucFdaUlZrQ1NYKzZrOXcyU01GL2hVZnZtdzZF?=
 =?utf-8?B?bEIyWms2KzdnU2dwdFdXQmlPVG5PU0FUQyt3emxnVkMzemZjdFVLL0VZeUJP?=
 =?utf-8?B?WjJsMGVlK04wLzVBU3hsWVQvWGxuMFhmMHdYNHZsUm1qNW9GNW9vZlhLVnll?=
 =?utf-8?B?aFNVUFp4bWFkZ3F2VUNpSERhYjJHQlUrL0VTeXJDZ2w2UVJVdVByRXlHTW5Y?=
 =?utf-8?B?d0tWcEJSdTh0cVBrOGdaMmxUTmcyLzdEV1crNHVOc0FuTkF2aUM1dTBqRUtl?=
 =?utf-8?B?NHB4c05GWW9qM09EY2xwYU1HZ0RyVXdkbHBGUHlDQytxZVlROVlEc1pIK0pj?=
 =?utf-8?B?ZWZFd1JzeGo0SnpnVTRLRkxKKzhDWTdvR0V2ZEJQa0FtcWJJb1FsQ2tZSUNU?=
 =?utf-8?B?Zm1WaXFzZ2hseUJieVo1d3BuRURsM2NIS2lPSFdFK2F0dk1GWXhiQzE2TEtp?=
 =?utf-8?B?VXJwZmtHR2JzTUEwWXBRM1Nzdk8rTXYvUXBob05sS01JcjAvREdYbVF1TGJS?=
 =?utf-8?Q?WACJ3cNuFAlwE2jCIWYlzzWXd9hdfLyYNDphWvu3rHZj?=
X-OriginatorOrg: piap.pl
X-MS-Exchange-CrossTenant-Network-Message-Id: f212bc5f-a398-4047-a585-08dbcbd8a2c9
X-MS-Exchange-CrossTenant-AuthSource: VI1P193MB0685.EURP193.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Oct 2023 10:39:11.0252
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3e05b101-c6fe-47e5-82e1-c6a410bb95c0
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: p2Mx+LKWUrEtPlDH8DIClLugtcFyimSCZM5FdxMEgMCWctdbFiziAF21GpGMfGel69Uw4B/Fw1gh+mbzgtLzqs/evM/yhYDx0S2lmTfBPYIXuVKT+xTA5QTzyo8qAg6my7p3KZrMB5H9i2N5DvDHug==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS8P193MB2302
X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,
	SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on morse.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (morse.vger.email [0.0.0.0]); Fri, 13 Oct 2023 03:39:46 -0700 (PDT)

Hi Stefan,

> Maybe you just hit a corner case or a bug, that can be avoid, in the I2C
> device.  Maybe check with the manufacturer directly?

I don't have direct contact at Sony, I guess I can try to escalate this
through the part supplier, but I won't hold my breath.

> Do you know the I2C repeated start feature [1]? This allows to batch toge=
ther
> multiple I2C read/writes in a single transfer. And in the best case, this
> transfer is executed in one go without a delay in between. At least in th=
e
> kernel it's guaranteed that no other driver can go in between with anothe=
r
> transfer.

Sure, imx290.c sensor driver use repeated STARTs. In fact, it only makes
things worse.

The timeout counter seems to start with the regular START (falling edge
of SDA), repeated STARTs don't reset it. After 2^18 + 8 or + 9 MCLK
cycles, the sensor simply disconnects from the bus, generating
pseudo-STOP if it was in the middle of its 0 bit (0->1 SDA change while
SCL high) or starting sending pseudo-1 bits otherwise (0xFF data on read
or negative ACK on write). This way repeated START -> longer transfer ->
higher probability of failure. Not that it really matters.

I don't know about in-sensor race conditions, for example on WRITE to
the chip, when the ACK it interrupted by the timeout (this can be
detected by the CPU, but not reliably, depending on actual timings).

OTOH with my "always use atomic xfers with these sensors" hack to the
i.MX I2C driver, it seems to work correctly (at least as far as I2C is
concerned).

I wonder if we could/should add some special handling of these sensors
in the mainline kernel. local_irq_save() and the atomic path do the
trick, but it would have to be done in all I2C drivers (or at least in
ones used with these sensors). If no other devices need such treatment,
well... Everyone can (possibly) use a non-official hack.

Thanks for your input,
--=20
Krzysztof "Chris" Ha=C5=82asa

Sie=C4=87 Badawcza =C5=81ukasiewicz
Przemys=C5=82owy Instytut Automatyki i Pomiar=C3=B3w PIAP
Al. Jerozolimskie 202, 02-486 Warszawa