From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id EDB3219B586; Wed, 8 Jan 2025 17:44:49 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.70.183.197 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736358292; cv=none; b=SLMuxNUMOYdQNRKbsbaJzIpXPTomRT/AyoukuPzYcIIn4lYVBu3xwu0HXEsbiRCLxju3RR7ymGTJkoelt8x3eZaj5RC90ZTsyD+KGLzLiRnnzhwGuEUGKQ1Hjlq4ddROHDH6N1X6g8Fvjxm9bOyIM1i1y6eVV2k0LoGb00cxt14= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736358292; c=relaxed/simple; bh=oxIY8To//tDnkJG7ZagTIT37bBiPqu2UlpxhpeXyo9E=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=mrXsNbNgfo884QS2fooff1IRGCh5eTo7ttezXsQsh8ZZ3UGLLoKCqVn7D6nkfDx8Te7of7s2hq0Q0MxS/BrveTE8eAeK3nLc7n/pPlMeV6q4cggwO0CGfJxruLgqd+AVJdwSGHPMqNvvEr4Hsx+47U6DtnJpEuFD7RN9wpp7wwA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com; spf=pass smtp.mailfrom=bootlin.com; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b=OhzF/v3e; arc=none smtp.client-ip=217.70.183.197 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=bootlin.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b="OhzF/v3e" Received: by mail.gandi.net (Postfix) with ESMTPSA id 14DDD1C0003; Wed, 8 Jan 2025 17:44:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=gm1; t=1736358286; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=nVX6wM/yvs9o7iBktHEAa+W2nk3G2s2HD+9v+CoiRkk=; b=OhzF/v3e/veSuhWNWeHA4zZZiEmhJtHWKroYPkZIQjQGdq76uUK7jNPyA1f0MO9pv//8hw AqtTSYg52KBcv6fCr0kLCYk9vBNSt3WkZZQRH1Sokz76DmiXIvLyfSBSv3XzrJei/rzbrH MXyap3cfVJSBSEyAMENcoYHdjWFY6TFwL47z0M6kgOVoS9bIVDmoBpxChGYPQEKHqhI5Eq OOS2b2drhmpxXkne0Qo8ClU8fchuxgIvieWxH/ign1fa0pGlxMZnWlB8GCGYHbhRrXP6oX i5d1cGOGxfzd8n8PYrqVQab3O9TkrfdYthiObV+qfjhfubc2hzat0PoTczt8+A== Date: Wed, 8 Jan 2025 18:44:42 +0100 From: Herve Codina To: Alexander Stein Cc: Andrzej Hajda , Neil Armstrong , Robert Foss , Laurent Pinchart , Jonas Karlman , Jernej Skrabec , David Airlie , Simona Vetter , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Marek Vasut , dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Louis Chauvet , Luca Ceresoli , Thomas Petazzoni Subject: Re: [PATCH v3 3/3] drm: bridge: ti-sn65dsi83: Add error recovery mechanism Message-ID: <20250108184442.64f38fbc@bootlin.com> In-Reply-To: <115787605.nniJfEyVGO@steina-w> References: <20250108101907.410456-1-herve.codina@bootlin.com> <20250108101907.410456-4-herve.codina@bootlin.com> <115787605.nniJfEyVGO@steina-w> Organization: Bootlin X-Mailer: Claws Mail 4.3.0 (GTK 3.24.43; x86_64-redhat-linux-gnu) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-GND-Sasl: herve.codina@bootlin.com Hi Alexander, On Wed, 08 Jan 2025 11:54:49 +0100 Alexander Stein wrote: [...] > > #include > > #include > > +#include /* DRM_MODESET_LOCK_ALL_BEGIN() needs drm_drv_uses_atomic_modeset() */ > > Shouldn't this include be added to include/drm/drm_modeset_lock.h instead? Yes indeed. I will change that in the next iteration. > > > #include > > #include > > #include > > @@ -147,6 +150,9 @@ struct sn65dsi83 { > > struct regulator *vcc; > > bool lvds_dual_link; > > bool lvds_dual_link_even_odd_swap; > > + bool use_irq; > > + struct delayed_work monitor_work; > > + struct work_struct reset_work; > > Can you please rebase? You are missing commit d2b8c6d549570 > ("drm/bridge: ti-sn65dsi83: Add ti,lvds-vod-swing optional properties") Sure, I will rebase. [...] > > +static void sn65dsi83_handle_errors(struct sn65dsi83 *ctx) > > +{ > > + unsigned int irq_stat; > > + int ret; > > + > > + /* > > + * Schedule a reset in case of: > > + * - the bridge doesn't answer > > + * - the bridge signals an error > > + */ > > + > > + ret = regmap_read(ctx->regmap, REG_IRQ_STAT, &irq_stat); > > + if (ret || irq_stat) > > + schedule_work(&ctx->reset_work); > > Shouldn't you clear the error bits as well? Thanks for pointing that. I can clear the error bit but further more, I probably need to simply disable the interrupt. In some cases, we observed i2c access failure. In that cases clearing error bits is simply not possible. To avoid some possible interrupt storms (the chip detect a failure, set its interrupt line but could be not accessible anymore), the best thing to do is to disable the interrupt line here, let the reset work to do its job performing a full reset of the device and re-enabling the interrupt line when needed, probably in sn65dsi83_atomic_enable(). What do you think about that? Best regards, Hervé