Message ID | 20240501152442.1072627-22-git@luigi311.com (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | Sakari Ailus |
Headers |
Received: from ny.mirrors.kernel.org ([147.75.199.223]) by linuxtv.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from <linux-media+bounces-10561-patchwork=linuxtv.org@vger.kernel.org>) id 1s2C0W-0006pg-0U for patchwork@linuxtv.org; Wed, 01 May 2024 15:36:33 +0000 Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ny.mirrors.kernel.org (Postfix) with ESMTPS id E01951C22F11 for <patchwork@linuxtv.org>; Wed, 1 May 2024 15:36:30 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id B5316136651; Wed, 1 May 2024 15:32:15 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=luigi311.com header.i=@luigi311.com header.b="CnC+cnQl" X-Original-To: linux-media@vger.kernel.org Received: from mail-108-mta190.mxroute.com (mail-108-mta190.mxroute.com [136.175.108.190]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id EB89B1350EF for <linux-media@vger.kernel.org>; Wed, 1 May 2024 15:32:12 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=136.175.108.190 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714577535; cv=none; b=e7tUyrUqwX8PQvK4LBzcbQv3ywi32wM8YD8jn0X1a6u6FrbyjSoh9vZnwKMXnPTEMmbBmnbCLJ/TSLit3kDMp/xtTSsWUrTX8tX/wR/BNSuy4+JQLjALkgzlHzOpi42LJymv/W0CF0HAtPLBd/YCCzr5QFotmYRR0XFxBljXwoY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1714577535; c=relaxed/simple; bh=cy2aWmrWmWiUHsiEVhlHLdnMObXYvjQ2zBk5thr1UPg=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=S7qe6x8anMdWcXzlE3N+AHWZfUDtMu6KCB8/7QtHpEhA1SlzOb42M57NSkKdusi2LJcIG7Ux6SsMTjUUL+EjXM2hiHHEX0e5w1hx9HiIkt2k5sNHpCjSApN8fWMAMz/o6G/333XBUFv+XminWEcyfZ3T2I6R5LWt0ZNg84SapQw= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=luigi311.com; spf=pass smtp.mailfrom=luigi311.com; dkim=pass (2048-bit key) header.d=luigi311.com header.i=@luigi311.com header.b=CnC+cnQl; arc=none smtp.client-ip=136.175.108.190 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=luigi311.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=luigi311.com Received: from filter006.mxroute.com ([136.175.111.2] filter006.mxroute.com) (Authenticated sender: mN4UYu2MZsgR) by mail-108-mta190.mxroute.com (ZoneMTA) with ESMTPSA id 18f34c2fda00008ca2.012 for <linux-media@vger.kernel.org> (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384); Wed, 01 May 2024 15:25:45 +0000 X-Zone-Loop: 13a44c13bb17a3757f03f497008e6aca04a8c9c0b244 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=luigi311.com; s=x; h=Content-Transfer-Encoding:MIME-Version:References: In-Reply-To:Message-ID:Date:Subject:Cc:To:From:Sender:Reply-To:Content-Type: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=Qrzk3wtzTAL6vILXNad62uNrTzT00IUmuGyKQ40D23U=; b=CnC+cnQl5j2zYk5kiv4/HYytuq 0GKCkhCKHsLadAqx2Cso4NC+cRO6KpTmr3QsAviBgg//ghwYvIirI1elHtjDGEnJpcEqztKEYouaR X0kAHRfUNlRZKce2K42aWo3qCJHRlgbC210yJGH+uv2vPKALwqYL4xHJV/TAElEW02ujYMghD9FHk UP42HhhAGvoQLB5zREnuiFBPTteOybjPR+BaRi5BC5+YclO/abkUzcYtHeYmlgNcbgqlFscT01GC9 fSz6Z3vkznWx8xUZ6ziqo/v/80u9/LkhQmAy55raY8/VDIqbkbe+H1ZzkxegNPq30E9sAjmHQPhIn v4u8+xWA==; From: git@luigi311.com To: linux-media@vger.kernel.org Cc: dave.stevenson@raspberrypi.com, jacopo.mondi@ideasonboard.com, mchehab@kernel.org, robh@kernel.org, krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, shawnguo@kernel.org, s.hauer@pengutronix.de, kernel@pengutronix.de, festevam@gmail.com, sakari.ailus@linux.intel.com, devicetree@vger.kernel.org, imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, pavel@ucw.cz, phone-devel@vger.kernel.org, Ondrej Jirman <megi@xff.cz>, Luis Garcia <git@luigi311.com>, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Subject: [PATCH v5 21/25] dt-bindings: media: imx258: Add binding for powerdown-gpio Date: Wed, 1 May 2024 09:24:38 -0600 Message-ID: <20240501152442.1072627-22-git@luigi311.com> In-Reply-To: <20240501152442.1072627-1-git@luigi311.com> References: <20240501152442.1072627-1-git@luigi311.com> Precedence: bulk X-Mailing-List: linux-media@vger.kernel.org List-Id: <linux-media.vger.kernel.org> List-Subscribe: <mailto:linux-media+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-media+unsubscribe@vger.kernel.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Authenticated-Id: personal@luigi311.com X-LSpam-Score: -2.3 (--) X-LSpam-Report: No, score=-2.3 required=5.0 tests=ARC_SIGNED=0.001,ARC_VALID=-0.1,BAYES_00=-1.9,DKIM_INVALID=0.1,DKIM_SIGNED=0.1,DMARC_MISSING=0.001,HEADER_FROM_DIFFERENT_DOMAINS=0.5,MAILING_LIST_MULTI=-1,SPF_HELO_NONE=0.001,SPF_PASS=-0.001 autolearn=unavailable autolearn_force=no |
Series |
imx258 improvement series
|
|
Commit Message
Luis Garcia
May 1, 2024, 3:24 p.m. UTC
From: Ondrej Jirman <megi@xff.cz> Add powerdown-gpio binding as it is required for some boards. Signed-off-by: Ondrej Jirman <megi@xff.cz> Signed-off-by: Luis Garcia <git@luigi311.com> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Reviewed-by: Pavel Machek <pavel@ucw.cz> --- Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml | 4 ++++ 1 file changed, 4 insertions(+)
Comments
Hi Luis, On Wed, May 01, 2024 at 09:24:38AM -0600, git@luigi311.com wrote: > From: Ondrej Jirman <megi@xff.cz> > > Add powerdown-gpio binding as it is required for some boards. I thought the conclusion was that this wasn't a property of the sensor? If it needs to be controlled, then this should take place somewhere else than in the sensor driver. > > Signed-off-by: Ondrej Jirman <megi@xff.cz> > Signed-off-by: Luis Garcia <git@luigi311.com> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Reviewed-by: Pavel Machek <pavel@ucw.cz> > --- > Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml > index c978abc0cdb3..33338139e6e8 100644 > --- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml > @@ -36,6 +36,10 @@ properties: > reg: > maxItems: 1 > > + powerdown-gpios: > + description: > + Reference to the GPIO connected to the PWDN pin, if any. > + > reset-gpios: > description: |- > Reference to the GPIO connected to the XCLR pin, if any.
On Fri, May 17, 2024 at 08:31:35AM +0000, Sakari Ailus wrote: > Hi Luis, > > On Wed, May 01, 2024 at 09:24:38AM -0600, git@luigi311.com wrote: > > From: Ondrej Jirman <megi@xff.cz> > > > > Add powerdown-gpio binding as it is required for some boards. > > I thought the conclusion was that this wasn't a property of the sensor? If > it needs to be controlled, then this should take place somewhere else than > in the sensor driver. I'll take patches up to the previous and then 24 and 25 from this version. The rest will need some rework.
On Fri, May 17, 2024 at 08:31:35AM GMT, Sakari Ailus wrote: > Hi Luis, > > On Wed, May 01, 2024 at 09:24:38AM -0600, git@luigi311.com wrote: > > From: Ondrej Jirman <megi@xff.cz> > > > > Add powerdown-gpio binding as it is required for some boards. > > I thought the conclusion was that this wasn't a property of the sensor? If > it needs to be controlled, then this should take place somewhere else than > in the sensor driver. It's a property of the sensor modules. It's just optional on some, eg. (pin 8): https://assets-global.website-files.com/63b65bd4974577341e1fe194/654290d4d0fb173e87f754ed_IMX_258_FF_drawing.png Where else should it be so that the module is described properly in the DT and the powerdown signal can be used as part of powerup/down sequence of the sensor? regards, o. > > > > Signed-off-by: Ondrej Jirman <megi@xff.cz> > > Signed-off-by: Luis Garcia <git@luigi311.com> > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > Reviewed-by: Pavel Machek <pavel@ucw.cz> > > --- > > Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml > > index c978abc0cdb3..33338139e6e8 100644 > > --- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml > > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml > > @@ -36,6 +36,10 @@ properties: > > reg: > > maxItems: 1 > > > > + powerdown-gpios: > > + description: > > + Reference to the GPIO connected to the PWDN pin, if any. > > + > > reset-gpios: > > description: |- > > Reference to the GPIO connected to the XCLR pin, if any. > > -- > Regards, > > Sakari Ailus
Hi Ondřej, On Mon, May 20, 2024 at 02:55:26PM +0200, Ondřej Jirman wrote: > On Fri, May 17, 2024 at 08:31:35AM GMT, Sakari Ailus wrote: > > Hi Luis, > > > > On Wed, May 01, 2024 at 09:24:38AM -0600, git@luigi311.com wrote: > > > From: Ondrej Jirman <megi@xff.cz> > > > > > > Add powerdown-gpio binding as it is required for some boards. > > > > I thought the conclusion was that this wasn't a property of the sensor? If > > it needs to be controlled, then this should take place somewhere else than > > in the sensor driver. > > It's a property of the sensor modules. It's just optional on > some, eg. (pin 8): > > https://assets-global.website-files.com/63b65bd4974577341e1fe194/654290d4d0fb173e87f754ed_IMX_258_FF_drawing.png > > Where else should it be so that the module is described properly in the > DT and the powerdown signal can be used as part of powerup/down sequence > of the sensor? That's an interesting case. The document does suggest the PWDN pin isn't connected though. Is this pin used for something? If so, what is it? The sensor doesn't have it and generally the module in this respect just contains wiring. Someone earlier suggested this could have been related to the VCM.
Hi Ondřej On Mon, 20 May 2024 at 13:55, Ondřej Jirman <megi@xff.cz> wrote: > > On Fri, May 17, 2024 at 08:31:35AM GMT, Sakari Ailus wrote: > > Hi Luis, > > > > On Wed, May 01, 2024 at 09:24:38AM -0600, git@luigi311.com wrote: > > > From: Ondrej Jirman <megi@xff.cz> > > > > > > Add powerdown-gpio binding as it is required for some boards. > > > > I thought the conclusion was that this wasn't a property of the sensor? If > > it needs to be controlled, then this should take place somewhere else than > > in the sensor driver. > > It's a property of the sensor modules. It's just optional on > some, eg. (pin 8): > > https://assets-global.website-files.com/63b65bd4974577341e1fe194/654290d4d0fb173e87f754ed_IMX_258_FF_drawing.png > > Where else should it be so that the module is described properly in the > DT and the powerdown signal can be used as part of powerup/down sequence > of the sensor? From v3 [1] Luis reported testing dropping the powerdown-gpio on a PPP and it working fine. I linked to the IMX258 datasheet in the same thread[2], and that datasheet does not include such a signal on the imx258 sensor itself. If your module has a powerdown gpio, then you'll have to ask the module vendor what it is actually connected to. Potentially it relates to the VCM driver rather than the sensor. Dave [1] https://www.spinics.net/lists/linux-media/msg252519.html [2] https://www.spinics.net/lists/linux-media/msg252496.html > regards, > o. > > > > > > > Signed-off-by: Ondrej Jirman <megi@xff.cz> > > > Signed-off-by: Luis Garcia <git@luigi311.com> > > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > > > Reviewed-by: Pavel Machek <pavel@ucw.cz> > > > --- > > > Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml > > > index c978abc0cdb3..33338139e6e8 100644 > > > --- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml > > > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml > > > @@ -36,6 +36,10 @@ properties: > > > reg: > > > maxItems: 1 > > > > > > + powerdown-gpios: > > > + description: > > > + Reference to the GPIO connected to the PWDN pin, if any. > > > + > > > reset-gpios: > > > description: |- > > > Reference to the GPIO connected to the XCLR pin, if any. > > > > -- > > Regards, > > > > Sakari Ailus
Hi Dave, On Mon, May 20, 2024 at 02:20:28PM GMT, Dave Stevenson wrote: > Hi Ondřej > > [...] > > From v3 [1] Luis reported testing dropping the powerdown-gpio on a PPP > and it working fine. > > I linked to the IMX258 datasheet in the same thread[2], and that > datasheet does not include such a signal on the imx258 sensor itself. > > If your module has a powerdown gpio, then you'll have to ask the > module vendor what it is actually connected to. Potentially it relates > to the VCM driver rather than the sensor. I've tested it in various ways (inverting the signal, etc.) and it doesn't seem to do anything. So these patches related to powerdown-gpio can be dropped. Kind regards, o. > Dave > > [1] https://www.spinics.net/lists/linux-media/msg252519.html > [2] https://www.spinics.net/lists/linux-media/msg252496.html
On 5/20/24 06:50, Sakari Ailus wrote: > On Fri, May 17, 2024 at 08:31:35AM +0000, Sakari Ailus wrote: >> Hi Luis, >> >> On Wed, May 01, 2024 at 09:24:38AM -0600, git@luigi311.com wrote: >>> From: Ondrej Jirman <megi@xff.cz> >>> >>> Add powerdown-gpio binding as it is required for some boards. >> >> I thought the conclusion was that this wasn't a property of the sensor? If >> it needs to be controlled, then this should take place somewhere else than >> in the sensor driver. > > I'll take patches up to the previous and then 24 and 25 from this version. > The rest will need some rework. > Last time i looked at the comments for these two patch discussion was still going on so i was submitting fixes for the other patches while this continued. Looks like the final consensuses is for these two to be dropped so I just submitted v6 with them both dropped. I also tested it on my PPP and it still continues to work as expected just like it did before. Take a look at v6 and let me know what else requires rework on there.
diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml index c978abc0cdb3..33338139e6e8 100644 --- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml @@ -36,6 +36,10 @@ properties: reg: maxItems: 1 + powerdown-gpios: + description: + Reference to the GPIO connected to the PWDN pin, if any. + reset-gpios: description: |- Reference to the GPIO connected to the XCLR pin, if any.