All of lore.kernel.org
 help / color / mirror / Atom feed
From: Maxime Ripard <maxime@cerno.tech>
To: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Cc: linux-media@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
	devel@driverdev.osuosl.org, linux-sunxi@googlegroups.com,
	Yong Deng <yong.deng@magewell.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Rob Herring <robh+dt@kernel.org>, Chen-Yu Tsai <wens@csie.org>,
	Jernej Skrabec <jernej.skrabec@siol.net>,
	Jonathan Corbet <corbet@lwn.net>,
	Kishon Vijay Abraham I <kishon@ti.com>,
	Vinod Koul <vkoul@kernel.org>,
	Helen Koike <helen.koike@collabora.com>,
	Dafna Hirschfeld <dafna.hirschfeld@collabora.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	Hans Verkuil <hans.verkuil@cisco.com>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	kevin.lhopital@hotmail.com
Subject: Re: [PATCH v2 13/19] media: sunxi: Add support for the A31 MIPI CSI-2 controller
Date: Wed, 2 Dec 2020 16:48:18 +0100	[thread overview]
Message-ID: <20201202154818.bf72m2firemyc5ve@gilmour> (raw)
In-Reply-To: <X8eoX+M650sMXqpx@aptenodytes>

[-- Attachment #1: Type: text/plain, Size: 2015 bytes --]

On Wed, Dec 02, 2020 at 03:44:47PM +0100, Paul Kocialkowski wrote:
> > > +static int __maybe_unused sun6i_mipi_csi2_suspend(struct device *dev)
> > > +{
> > > +	struct sun6i_mipi_csi2_dev *cdev = dev_get_drvdata(dev);
> > > +
> > > +	clk_disable_unprepare(cdev->clk_mod);
> > > +	clk_disable_unprepare(cdev->clk_bus);
> > > +	reset_control_assert(cdev->reset);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int __maybe_unused sun6i_mipi_csi2_resume(struct device *dev)
> > > +{
> > > +	struct sun6i_mipi_csi2_dev *cdev = dev_get_drvdata(dev);
> > > +	int ret;
> > > +
> > > +	ret = reset_control_deassert(cdev->reset);
> > > +	if (ret) {
> > > +		dev_err(cdev->dev, "failed to deassert reset\n");
> > > +		return ret;
> > > +	}
> > > +
> > > +	ret = clk_prepare_enable(cdev->clk_bus);
> > > +	if (ret) {
> > > +		dev_err(cdev->dev, "failed to enable bus clock\n");
> > > +		goto error_reset;
> > > +	}
> > > +
> > > +	ret = clk_prepare_enable(cdev->clk_mod);
> > > +	if (ret) {
> > > +		dev_err(cdev->dev, "failed to enable module clock\n");
> > > +		goto error_clk_bus;
> > > +	}
> > > +
> > > +	return 0;
> > > +
> > > +error_clk_bus:
> > > +	clk_disable_unprepare(cdev->clk_bus);
> > > +
> > > +error_reset:
> > > +	reset_control_assert(cdev->reset);
> > > +
> > > +	return ret;
> > > +}
> > 
> > I'm guessing you set the __maybe_unused attribute because you're using
> > SET_RUNTIME_PM_OPS, but what would happen if runtime_pm isn't selected?
> > It looks like you don't handle that case.
> 
> Indeed, __maybe_unused is because of the conditional definition of
> SET_RUNTIME_PM_OPS. If CONFIG_PM is not selected, then I guess the controller
> wouldn't be powered and wouldn't work. So I should definitely add a Kconfig
> dependency on PM then, right?

There's two ways we can do it. What you suggested is one, the other is
to have something like our SPI driver to call directly the resume
function if there's no runtime pm support.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Maxime Ripard <maxime@cerno.tech>
To: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Cc: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>,
	linux-doc@vger.kernel.org, linux-sunxi@googlegroups.com,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	kevin.lhopital@hotmail.com, devel@driverdev.osuosl.org,
	Jonathan Corbet <corbet@lwn.net>,
	Kishon Vijay Abraham I <kishon@ti.com>,
	Chen-Yu Tsai <wens@csie.org>,
	Hans Verkuil <hans.verkuil@cisco.com>,
	linux-media@vger.kernel.org, devicetree@vger.kernel.org,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	Helen Koike <helen.koike@collabora.com>,
	Rob Herring <robh+dt@kernel.org>,
	Yong Deng <yong.deng@magewell.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	linux-arm-kernel@lists.infradead.org,
	Jernej Skrabec <jernej.skrabec@siol.net>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-kernel@vger.kernel.org, Vinod Koul <vkoul@kernel.org>,
	Philipp Zabel <p.zabel@pengutronix.de>
Subject: Re: [PATCH v2 13/19] media: sunxi: Add support for the A31 MIPI CSI-2 controller
Date: Wed, 2 Dec 2020 16:48:18 +0100	[thread overview]
Message-ID: <20201202154818.bf72m2firemyc5ve@gilmour> (raw)
In-Reply-To: <X8eoX+M650sMXqpx@aptenodytes>


[-- Attachment #1.1: Type: text/plain, Size: 2015 bytes --]

On Wed, Dec 02, 2020 at 03:44:47PM +0100, Paul Kocialkowski wrote:
> > > +static int __maybe_unused sun6i_mipi_csi2_suspend(struct device *dev)
> > > +{
> > > +	struct sun6i_mipi_csi2_dev *cdev = dev_get_drvdata(dev);
> > > +
> > > +	clk_disable_unprepare(cdev->clk_mod);
> > > +	clk_disable_unprepare(cdev->clk_bus);
> > > +	reset_control_assert(cdev->reset);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int __maybe_unused sun6i_mipi_csi2_resume(struct device *dev)
> > > +{
> > > +	struct sun6i_mipi_csi2_dev *cdev = dev_get_drvdata(dev);
> > > +	int ret;
> > > +
> > > +	ret = reset_control_deassert(cdev->reset);
> > > +	if (ret) {
> > > +		dev_err(cdev->dev, "failed to deassert reset\n");
> > > +		return ret;
> > > +	}
> > > +
> > > +	ret = clk_prepare_enable(cdev->clk_bus);
> > > +	if (ret) {
> > > +		dev_err(cdev->dev, "failed to enable bus clock\n");
> > > +		goto error_reset;
> > > +	}
> > > +
> > > +	ret = clk_prepare_enable(cdev->clk_mod);
> > > +	if (ret) {
> > > +		dev_err(cdev->dev, "failed to enable module clock\n");
> > > +		goto error_clk_bus;
> > > +	}
> > > +
> > > +	return 0;
> > > +
> > > +error_clk_bus:
> > > +	clk_disable_unprepare(cdev->clk_bus);
> > > +
> > > +error_reset:
> > > +	reset_control_assert(cdev->reset);
> > > +
> > > +	return ret;
> > > +}
> > 
> > I'm guessing you set the __maybe_unused attribute because you're using
> > SET_RUNTIME_PM_OPS, but what would happen if runtime_pm isn't selected?
> > It looks like you don't handle that case.
> 
> Indeed, __maybe_unused is because of the conditional definition of
> SET_RUNTIME_PM_OPS. If CONFIG_PM is not selected, then I guess the controller
> wouldn't be powered and wouldn't work. So I should definitely add a Kconfig
> dependency on PM then, right?

There's two ways we can do it. What you suggested is one, the other is
to have something like our SPI driver to call directly the resume
function if there's no runtime pm support.

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 169 bytes --]

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

WARNING: multiple messages have this Message-ID (diff)
From: Maxime Ripard <maxime@cerno.tech>
To: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Cc: Dafna Hirschfeld <dafna.hirschfeld@collabora.com>,
	linux-doc@vger.kernel.org, linux-sunxi@googlegroups.com,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	kevin.lhopital@hotmail.com, devel@driverdev.osuosl.org,
	Jonathan Corbet <corbet@lwn.net>,
	Kishon Vijay Abraham I <kishon@ti.com>,
	Chen-Yu Tsai <wens@csie.org>,
	Hans Verkuil <hans.verkuil@cisco.com>,
	linux-media@vger.kernel.org, devicetree@vger.kernel.org,
	Sakari Ailus <sakari.ailus@linux.intel.com>,
	Helen Koike <helen.koike@collabora.com>,
	Rob Herring <robh+dt@kernel.org>,
	Yong Deng <yong.deng@magewell.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	linux-arm-kernel@lists.infradead.org,
	Jernej Skrabec <jernej.skrabec@siol.net>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-kernel@vger.kernel.org, Vinod Koul <vkoul@kernel.org>,
	Philipp Zabel <p.zabel@pengutronix.de>
Subject: Re: [PATCH v2 13/19] media: sunxi: Add support for the A31 MIPI CSI-2 controller
Date: Wed, 2 Dec 2020 16:48:18 +0100	[thread overview]
Message-ID: <20201202154818.bf72m2firemyc5ve@gilmour> (raw)
In-Reply-To: <X8eoX+M650sMXqpx@aptenodytes>


[-- Attachment #1.1: Type: text/plain, Size: 2015 bytes --]

On Wed, Dec 02, 2020 at 03:44:47PM +0100, Paul Kocialkowski wrote:
> > > +static int __maybe_unused sun6i_mipi_csi2_suspend(struct device *dev)
> > > +{
> > > +	struct sun6i_mipi_csi2_dev *cdev = dev_get_drvdata(dev);
> > > +
> > > +	clk_disable_unprepare(cdev->clk_mod);
> > > +	clk_disable_unprepare(cdev->clk_bus);
> > > +	reset_control_assert(cdev->reset);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static int __maybe_unused sun6i_mipi_csi2_resume(struct device *dev)
> > > +{
> > > +	struct sun6i_mipi_csi2_dev *cdev = dev_get_drvdata(dev);
> > > +	int ret;
> > > +
> > > +	ret = reset_control_deassert(cdev->reset);
> > > +	if (ret) {
> > > +		dev_err(cdev->dev, "failed to deassert reset\n");
> > > +		return ret;
> > > +	}
> > > +
> > > +	ret = clk_prepare_enable(cdev->clk_bus);
> > > +	if (ret) {
> > > +		dev_err(cdev->dev, "failed to enable bus clock\n");
> > > +		goto error_reset;
> > > +	}
> > > +
> > > +	ret = clk_prepare_enable(cdev->clk_mod);
> > > +	if (ret) {
> > > +		dev_err(cdev->dev, "failed to enable module clock\n");
> > > +		goto error_clk_bus;
> > > +	}
> > > +
> > > +	return 0;
> > > +
> > > +error_clk_bus:
> > > +	clk_disable_unprepare(cdev->clk_bus);
> > > +
> > > +error_reset:
> > > +	reset_control_assert(cdev->reset);
> > > +
> > > +	return ret;
> > > +}
> > 
> > I'm guessing you set the __maybe_unused attribute because you're using
> > SET_RUNTIME_PM_OPS, but what would happen if runtime_pm isn't selected?
> > It looks like you don't handle that case.
> 
> Indeed, __maybe_unused is because of the conditional definition of
> SET_RUNTIME_PM_OPS. If CONFIG_PM is not selected, then I guess the controller
> wouldn't be powered and wouldn't work. So I should definitely add a Kconfig
> dependency on PM then, right?

There's two ways we can do it. What you suggested is one, the other is
to have something like our SPI driver to call directly the resume
function if there's no runtime pm support.

Maxime

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2020-12-02 15:49 UTC|newest]

Thread overview: 117+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-28 14:28 [PATCH v2 00/19] Allwinner MIPI CSI-2 support for A31/V3s/A83T Paul Kocialkowski
2020-11-28 14:28 ` Paul Kocialkowski
2020-11-28 14:28 ` Paul Kocialkowski
2020-11-28 14:28 ` [PATCH v2 01/19] docs: phy: Add a part about PHY mode and submode Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-12-01 10:37   ` Maxime Ripard
2020-12-01 10:37     ` Maxime Ripard
2020-12-01 10:37     ` Maxime Ripard
2020-11-28 14:28 ` [PATCH v2 02/19] phy: Distinguish between Rx and Tx for MIPI D-PHY with submodes Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-11-28 14:28 ` [PATCH v2 03/19] phy: allwinner: phy-sun6i-mipi-dphy: Support D-PHY Rx mode for MIPI CSI-2 Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-12-01 10:38   ` Maxime Ripard
2020-12-01 10:38     ` Maxime Ripard
2020-12-01 10:38     ` Maxime Ripard
2020-11-28 14:28 ` [PATCH v2 04/19] media: sun6i-csi: Use common V4L2 format info for storage bpp Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-12-01 10:39   ` Maxime Ripard
2020-12-01 10:39     ` Maxime Ripard
2020-12-01 10:39     ` Maxime Ripard
2020-11-28 14:28 ` [PATCH v2 05/19] media: sun6i-csi: Only configure the interface data width for parallel Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-11-28 14:28 ` [PATCH v2 06/19] dt-bindings: media: sun6i-a31-csi: Add MIPI CSI-2 input port Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-12-01 10:43   ` Maxime Ripard
2020-12-01 10:43     ` Maxime Ripard
2020-12-01 10:43     ` Maxime Ripard
2020-12-02 14:16     ` Paul Kocialkowski
2020-12-02 14:16       ` Paul Kocialkowski
2020-12-02 14:16       ` Paul Kocialkowski
2020-12-08 19:54       ` Rob Herring
2020-12-08 19:54         ` Rob Herring
2020-12-08 19:54         ` Rob Herring
2020-11-28 14:28 ` [PATCH v2 07/19] media: sun6i-csi: Add support for MIPI CSI-2 bridge input Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-12-01 12:12   ` Maxime Ripard
2020-12-01 12:12     ` Maxime Ripard
2020-12-01 12:12     ` Maxime Ripard
2020-12-02 14:19     ` Paul Kocialkowski
2020-12-02 14:19       ` Paul Kocialkowski
2020-12-02 14:19       ` Paul Kocialkowski
2020-12-02 15:40       ` Maxime Ripard
2020-12-02 15:40         ` Maxime Ripard
2020-12-02 15:40         ` Maxime Ripard
2020-12-02 16:07         ` Paul Kocialkowski
2020-12-02 16:07           ` Paul Kocialkowski
2020-12-02 16:07           ` Paul Kocialkowski
2020-11-28 14:28 ` [PATCH v2 08/19] ARM: dts: sun8i: a83t: Add CSI controller ports Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-11-28 14:28 ` [PATCH v2 09/19] ARM: dts: sunxi: h3/h5: Add CSI controller port for parallel input Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-12-01 12:14   ` Maxime Ripard
2020-12-01 12:14     ` Maxime Ripard
2020-12-01 12:14     ` Maxime Ripard
2020-12-02 15:02     ` Paul Kocialkowski
2020-12-02 15:02       ` Paul Kocialkowski
2020-12-02 15:02       ` Paul Kocialkowski
2020-12-02 15:46       ` Maxime Ripard
2020-12-02 15:46         ` Maxime Ripard
2020-12-02 15:46         ` Maxime Ripard
2020-11-28 14:28 ` [PATCH v2 10/19] ARM: dts: sun8i: v3s: Add CSI1 " Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-11-28 14:28 ` [PATCH v2 11/19] arm64: dts: allwinner: a64: Add CSI " Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-11-28 14:28 ` [PATCH v2 12/19] dt-bindings: media: Add A31 MIPI CSI-2 bindings documentation Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-12-01 12:15   ` Maxime Ripard
2020-12-01 12:15     ` Maxime Ripard
2020-12-01 12:15     ` Maxime Ripard
2020-11-28 14:28 ` [PATCH v2 13/19] media: sunxi: Add support for the A31 MIPI CSI-2 controller Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-12-01 12:20   ` Maxime Ripard
2020-12-01 12:20     ` Maxime Ripard
2020-12-01 12:20     ` Maxime Ripard
2020-12-02 14:44     ` Paul Kocialkowski
2020-12-02 14:44       ` Paul Kocialkowski
2020-12-02 14:44       ` Paul Kocialkowski
2020-12-02 15:48       ` Maxime Ripard [this message]
2020-12-02 15:48         ` Maxime Ripard
2020-12-02 15:48         ` Maxime Ripard
2020-12-02 16:09         ` Paul Kocialkowski
2020-12-02 16:09           ` Paul Kocialkowski
2020-12-02 16:09           ` Paul Kocialkowski
2020-11-28 14:28 ` [PATCH v2 14/19] ARM: dts: sun8i: v3s: Add nodes for MIPI CSI-2 support Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-11-28 14:28 ` [PATCH v2 15/19] MAINTAINERS: Add entry for the Allwinner A31 MIPI CSI-2 bridge Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-11-28 14:28 ` [PATCH v2 16/19] dt-bindings: media: Add A83T MIPI CSI-2 bindings documentation Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-12-11  3:12   ` Rob Herring
2020-12-11  3:12     ` Rob Herring
2020-12-11  3:12     ` Rob Herring
2020-11-28 14:28 ` [PATCH v2 17/19] media: sunxi: Add support for the A83T MIPI CSI-2 controller Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-11-28 14:28 ` [PATCH v2 18/19] ARM: dts: sun8i: a83t: Add MIPI CSI-2 controller node Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-11-28 14:28 ` [PATCH v2 19/19] MAINTAINERS: Add entry for the Allwinner A83T MIPI CSI-2 bridge Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski
2020-11-28 14:28   ` Paul Kocialkowski

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=20201202154818.bf72m2firemyc5ve@gilmour \
    --to=maxime@cerno.tech \
    --cc=corbet@lwn.net \
    --cc=dafna.hirschfeld@collabora.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=devicetree@vger.kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=hans.verkuil@cisco.com \
    --cc=helen.koike@collabora.com \
    --cc=jernej.skrabec@siol.net \
    --cc=kevin.lhopital@hotmail.com \
    --cc=kishon@ti.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-sunxi@googlegroups.com \
    --cc=mchehab@kernel.org \
    --cc=p.zabel@pengutronix.de \
    --cc=paul.kocialkowski@bootlin.com \
    --cc=robh+dt@kernel.org \
    --cc=sakari.ailus@linux.intel.com \
    --cc=thomas.petazzoni@bootlin.com \
    --cc=vkoul@kernel.org \
    --cc=wens@csie.org \
    --cc=yong.deng@magewell.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.