From patchwork Fri Aug 30 06:11:51 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Umang Jain X-Patchwork-Id: 13784323 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) (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 193151531C9 for ; Fri, 30 Aug 2024 06:12:12 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=213.167.242.64 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724998334; cv=none; b=iapt2uCX12bFqJLZbs8Mg3gp7PCGFHMHz16vF9KyfK9XmS4b9BI7xN1CsHsUDoLO1z2CJ0+HtX4eltJvxjTAmDWadZXoqVu1luvKq19bl2d53OcTohWOJ3r0FIGla/hLk7PkIBR56L6OTHYQXpLHI5Rbb50PDc7RttBIo5OaqZw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724998334; c=relaxed/simple; bh=A5lJjAthj7edefbpVSZ1vxzA9GwnHb4ZMwTPz2YGRBY=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=hwxfjUGHGkOVs+/oVSyUUPcAPt72DuB/dJ7MW5KR8eDUMwNdIDS8MKLGVnBnF/YbwGoshqIkejAqAHX0rlAm2x6wxTRxrKNlRtj2PfRMrrmqFLm9ZD0roe1IFVEkao0er9T5O8vKDKcCZtHDJ2uCawQvaaJfb6L3bfVbP4e5OME= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ideasonboard.com; spf=pass smtp.mailfrom=ideasonboard.com; dkim=pass (1024-bit key) header.d=ideasonboard.com header.i=@ideasonboard.com header.b=u/91umad; arc=none smtp.client-ip=213.167.242.64 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ideasonboard.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=ideasonboard.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="u/91umad" Received: from [192.168.29.25] (unknown [IPv6:2405:201:2015:f873:55f8:639e:8e9f:12ec]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 324FF524; Fri, 30 Aug 2024 08:10:59 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1724998262; bh=A5lJjAthj7edefbpVSZ1vxzA9GwnHb4ZMwTPz2YGRBY=; h=From:Date:Subject:References:In-Reply-To:To:Cc:From; b=u/91umadaVk2kcMeThKWsNphlUtZpHhxeMCHXESH97tGfZImcRsu52Rv7d9BWVTar +qRcnajlS1en5BEMRQSAr07uZyAS9lIQAfKgaxEtsvLFHNBoDjWt4SI8Ydw0udnps/ FmzqMMQ1IQXDPvVP4xuUAikcEM5oNvFQasrmEztY= From: Umang Jain Date: Fri, 30 Aug 2024 11:41:51 +0530 Subject: [PATCH v4 1/2] dt-bindings: media: imx335: Add reset-gpios to the DT example Precedence: bulk X-Mailing-List: imx@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20240830-imx335-vflip-v4-1-cb9f20fc7b87@ideasonboard.com> References: <20240830-imx335-vflip-v4-0-cb9f20fc7b87@ideasonboard.com> In-Reply-To: <20240830-imx335-vflip-v4-0-cb9f20fc7b87@ideasonboard.com> To: Mauro Carvalho Chehab , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Shawn Guo , Sascha Hauer , Pengutronix Kernel Team , Fabio Estevam , "Paul J. Murphy" , Daniele Alessandrelli , Sakari Ailus , Martina Krasteva Cc: linux-media@vger.kernel.org, devicetree@vger.kernel.org, imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Mauro Carvalho Chehab , Kieran Bingham , Laurent Pinchart , Umang Jain , Krzysztof Kozlowski X-Mailer: b4 0.13.0 X-Developer-Signature: v=1; a=ed25519-sha256; t=1724998323; l=1487; i=umang.jain@ideasonboard.com; s=20240731; h=from:subject:message-id; bh=A5lJjAthj7edefbpVSZ1vxzA9GwnHb4ZMwTPz2YGRBY=; b=222YGFQbzKPw/p+ZhX2x9k1K8y+nLZd0q1+lOMFuTcH/52XeXGFUImn45SBUJ3HINrtRdwAFo YyaKhvM+HKwCm2q2GsAgqenDszct3DJFtSJFQcB5t4aYlH0mQt2JIXa X-Developer-Key: i=umang.jain@ideasonboard.com; a=ed25519; pk=7pvnIBNsDpFUMiph0Vlhrr01+rAn5fSIn/QtDeLeXL0= It's easy to get the polarity of GPIOs in the device tree wrong, as shown by a recently fixed bug in the imx335 driver. To lower the chance of future mistakes, especially in new bindings that would take the imx335 binding as a starting point, add the reset-gpios property to the DT example. This showcases the correct polarity of the XCLR signal for Sony sensors in the most common case of the signal not being inverted on the board. Acked-by: Krzysztof Kozlowski Signed-off-by: Umang Jain --- Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml index 106c36ee966d..77bf3a4ee89d 100644 --- a/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml @@ -75,6 +75,8 @@ additionalProperties: false examples: - | + #include + i2c { #address-cells = <1>; #size-cells = <0>; @@ -92,6 +94,8 @@ examples: ovdd-supply = <&camera_vddo_1v8>; dvdd-supply = <&camera_vddd_1v2>; + reset-gpios = <&gpio 50 GPIO_ACTIVE_LOW>; + port { imx335: endpoint { remote-endpoint = <&cam>; From patchwork Fri Aug 30 06:11:52 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Umang Jain X-Patchwork-Id: 13784324 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) (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 092ED481DB for ; Fri, 30 Aug 2024 06:12:15 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=213.167.242.64 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724998337; cv=none; b=mUUJBiV6LYi8syH6l9eBmYLZm5aWvlWD85MI9Q9RkPcUZL0U4ls92hTwoI0l2Z7Nezmc4WDXQ5agjDvOk8lFagMvjkKQ7cW+SYAudyLLgvw/4vn2t0mE18TtvDYuzcdF5huDkCcKpTKLkfTJqBGnbrNI2+71v0jbueuIo/pD+6Q= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724998337; c=relaxed/simple; bh=RS94icNT3Eg1ti5P4VHowp58hHjlEiQghewC7c9ZRdY=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=OOvBop5fNxta2Hnp772s3meWMwP75Qhze3V1cwNpekPEms5pRXoABK2UZ8jPIxnDcvhIzuQLkCpLFAzPT+Aj/oMJIjYEiXBsVklON/vYMP8mYto+uHid/4TsdEKQB6mxSljVeoqYADu6fJp1jcSDBOeeZKXAatYSQa3GRQv7j+o= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ideasonboard.com; spf=pass smtp.mailfrom=ideasonboard.com; dkim=pass (1024-bit key) header.d=ideasonboard.com header.i=@ideasonboard.com header.b=s4/sHg8A; arc=none smtp.client-ip=213.167.242.64 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ideasonboard.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=ideasonboard.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="s4/sHg8A" Received: from [192.168.29.25] (unknown [IPv6:2405:201:2015:f873:55f8:639e:8e9f:12ec]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id C059AAD8; Fri, 30 Aug 2024 08:11:02 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1724998266; bh=RS94icNT3Eg1ti5P4VHowp58hHjlEiQghewC7c9ZRdY=; h=From:Date:Subject:References:In-Reply-To:To:Cc:From; b=s4/sHg8ALwJUHm5ZOWL54xlfL0miILM7yRdGp8SIFrYZk9gIqMrChE8nFIghJt2Sj 6vMCCV+PnOhBrcSIpiawk6l3b1Wj2vuw+CUepu00LqfzxRB8um52J3UxKj9u3cGn6C fL3WFQRB1Hf/Nn1FP9/GLUNrx0bcq/ZD/1tcohfw= From: Umang Jain Date: Fri, 30 Aug 2024 11:41:52 +0530 Subject: [PATCH v4 2/2] media: imx335: Fix reset-gpio handling Precedence: bulk X-Mailing-List: imx@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20240830-imx335-vflip-v4-2-cb9f20fc7b87@ideasonboard.com> References: <20240830-imx335-vflip-v4-0-cb9f20fc7b87@ideasonboard.com> In-Reply-To: <20240830-imx335-vflip-v4-0-cb9f20fc7b87@ideasonboard.com> To: Mauro Carvalho Chehab , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Shawn Guo , Sascha Hauer , Pengutronix Kernel Team , Fabio Estevam , "Paul J. Murphy" , Daniele Alessandrelli , Sakari Ailus , Martina Krasteva Cc: linux-media@vger.kernel.org, devicetree@vger.kernel.org, imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Mauro Carvalho Chehab , Kieran Bingham , Laurent Pinchart , Umang Jain , stable@vger.kernel.org X-Mailer: b4 0.13.0 X-Developer-Signature: v=1; a=ed25519-sha256; t=1724998323; l=4046; i=umang.jain@ideasonboard.com; s=20240731; h=from:subject:message-id; bh=RS94icNT3Eg1ti5P4VHowp58hHjlEiQghewC7c9ZRdY=; b=WS3Iz29bSo0Je4ynMwl7AtWMEvgu1kXD2Vj39Q3XQoJvlK12wlS4WynKSj2FSGR1XrSLK0I1r oTr5rI22EBFDNJcjICZjBu0skjOJ8Qr7EeCK5ZZKmwFgTil6Z+Hhg5+ X-Developer-Key: i=umang.jain@ideasonboard.com; a=ed25519; pk=7pvnIBNsDpFUMiph0Vlhrr01+rAn5fSIn/QtDeLeXL0= Rectify the logical value of reset-gpio so that it is set to 0 (disabled) during power-on and to 1 (enabled) during power-off. Set the reset-gpio to GPIO_OUT_HIGH at initialization time to make sure it starts off in reset. Also drop the "Set XCLR" comment which is not-so-informative. The existing usage of imx335 had reset-gpios polarity inverted (GPIO_ACTIVE_HIGH) in their device-tree sources. With this patch included, those DTS will not be able to stream imx335 anymore. The reset-gpio polarity will need to be rectified in the device-tree sources as shown in [1] example, in order to get imx335 functional again (as it remains in reset prior to this fix). Cc: stable@vger.kernel.org Fixes: 45d19b5fb9ae ("media: i2c: Add imx335 camera sensor driver") Reviewed-by: Laurent Pinchart Link: https://lore.kernel.org/linux-media/20240729110437.199428-1-umang.jain@ideasonboard.com/ Signed-off-by: Umang Jain --- Following conclusions has been observed and discussed [2]: - Original driver was reviewed [3] but, the improper handling of reset-gpios was missed in review. - Commit fea91ee73b7c ("media: i2c: imx335: Enable regulator supplies") shows the driver didn't had regulator enablement support. The driver would have only worked for cases when the supplies were always-on. - Commit 14a60786d72e ("media: imx335: Set reserved register to default value") reflects that the imx335 driver was un-usable due to a reserved register not been set to default. - The original author is no longer using the driver nor it is used for its original purpose any more (confirmed by Sakari Ailus). - It's extremely unlikely the driver has been or continues to be in use on ACPI based systems (comment by Sakari Ailus). The above discussion points in a direction that driver does not cater to a large user-base. Nonetheless, the breakage will be noticed by a few users (if at all) hence, this explanation would help resolve the breakage as soon as noticed (by using correct reset-gpio polarity as mentioned in [1]). [1]: Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml [2]: https://lore.kernel.org/linux-media/20240729110437.199428-1-umang.jain@ideasonboard.com/ [3]: https://lore.kernel.org/all/20210527142145.173-3-martinax.krasteva@linux.intel.com/ --- --- drivers/media/i2c/imx335.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/media/i2c/imx335.c b/drivers/media/i2c/imx335.c index 990d74214cc2..54a1de53d497 100644 --- a/drivers/media/i2c/imx335.c +++ b/drivers/media/i2c/imx335.c @@ -997,7 +997,7 @@ static int imx335_parse_hw_config(struct imx335 *imx335) /* Request optional reset pin */ imx335->reset_gpio = devm_gpiod_get_optional(imx335->dev, "reset", - GPIOD_OUT_LOW); + GPIOD_OUT_HIGH); if (IS_ERR(imx335->reset_gpio)) { dev_err(imx335->dev, "failed to get reset gpio %ld\n", PTR_ERR(imx335->reset_gpio)); @@ -1110,8 +1110,7 @@ static int imx335_power_on(struct device *dev) usleep_range(500, 550); /* Tlow */ - /* Set XCLR */ - gpiod_set_value_cansleep(imx335->reset_gpio, 1); + gpiod_set_value_cansleep(imx335->reset_gpio, 0); ret = clk_prepare_enable(imx335->inclk); if (ret) { @@ -1124,7 +1123,7 @@ static int imx335_power_on(struct device *dev) return 0; error_reset: - gpiod_set_value_cansleep(imx335->reset_gpio, 0); + gpiod_set_value_cansleep(imx335->reset_gpio, 1); regulator_bulk_disable(ARRAY_SIZE(imx335_supply_name), imx335->supplies); return ret; @@ -1141,7 +1140,7 @@ static int imx335_power_off(struct device *dev) struct v4l2_subdev *sd = dev_get_drvdata(dev); struct imx335 *imx335 = to_imx335(sd); - gpiod_set_value_cansleep(imx335->reset_gpio, 0); + gpiod_set_value_cansleep(imx335->reset_gpio, 1); clk_disable_unprepare(imx335->inclk); regulator_bulk_disable(ARRAY_SIZE(imx335_supply_name), imx335->supplies);