* [PATCH v3 0/2] rcar-csi2: Use standby mode instead of resetting
@ 2019-03-12 23:49 Niklas Söderlund
2019-03-12 23:49 ` [PATCH v3 1/2] dt-bindings: rcar-csi2: List resets as a mandatory property Niklas Söderlund
2019-03-12 23:49 ` [PATCH v3 2/2] rcar-csi2: Use standby mode instead of resetting Niklas Söderlund
0 siblings, 2 replies; 6+ messages in thread
From: Niklas Söderlund @ 2019-03-12 23:49 UTC (permalink / raw)
To: Laurent Pinchart, linux-media; +Cc: linux-renesas-soc, Niklas Söderlund
Hi,
This small series updates rcar-csi2 to use the standby mode described in
later versions of the datasheet.
* Changes since v2
- Clarify commit message that software reset is replaced with the new
module reset method.
- Enter standby mode before stopping the video source.
* Changes since v1
- Break up enter and exit of standby mode in two separate functions.
- Add "select RESET_CONTROLLER" to Kconfig.
Niklas Söderlund (2):
dt-bindings: rcar-csi2: List resets as a mandatory property
rcar-csi2: Use standby mode instead of resetting
.../bindings/media/renesas,rcar-csi2.txt | 3 +-
drivers/media/platform/rcar-vin/Kconfig | 1 +
drivers/media/platform/rcar-vin/rcar-csi2.c | 69 +++++++++++--------
3 files changed, 45 insertions(+), 28 deletions(-)
--
2.21.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 1/2] dt-bindings: rcar-csi2: List resets as a mandatory property
2019-03-12 23:49 [PATCH v3 0/2] rcar-csi2: Use standby mode instead of resetting Niklas Söderlund
@ 2019-03-12 23:49 ` Niklas Söderlund
2019-04-12 7:03 ` Jacopo Mondi
2019-03-12 23:49 ` [PATCH v3 2/2] rcar-csi2: Use standby mode instead of resetting Niklas Söderlund
1 sibling, 1 reply; 6+ messages in thread
From: Niklas Söderlund @ 2019-03-12 23:49 UTC (permalink / raw)
To: Laurent Pinchart, linux-media
Cc: linux-renesas-soc, Niklas Söderlund, devicetree, Rob Herring
The resets property will become mandatory to operate the device, list it
as such. All device tree source files have always included the reset
property so making it mandatory will not introduce any regressions.
While at it improve the description for the clocks property.
Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
index d63275e17afdd180..9a0d0531c67df48c 100644
--- a/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
+++ b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
@@ -18,7 +18,8 @@ Mandatory properties
- reg: the register base and size for the device registers
- interrupts: the interrupt for the device
- - clocks: reference to the parent clock
+ - clocks: A phandle + clock specifier for the module clock
+ - resets: A phandle + reset specifier for the module reset
The device node shall contain two 'port' child nodes according to the
bindings defined in Documentation/devicetree/bindings/media/
--
2.21.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 2/2] rcar-csi2: Use standby mode instead of resetting
2019-03-12 23:49 [PATCH v3 0/2] rcar-csi2: Use standby mode instead of resetting Niklas Söderlund
2019-03-12 23:49 ` [PATCH v3 1/2] dt-bindings: rcar-csi2: List resets as a mandatory property Niklas Söderlund
@ 2019-03-12 23:49 ` Niklas Söderlund
2019-03-13 0:18 ` Laurent Pinchart
1 sibling, 1 reply; 6+ messages in thread
From: Niklas Söderlund @ 2019-03-12 23:49 UTC (permalink / raw)
To: Laurent Pinchart, linux-media; +Cc: linux-renesas-soc, Niklas Söderlund
Later versions of the datasheet updates the reset procedure to more
closely resemble the standby mode. Update the driver to enter and exit
the standby mode instead of resetting the hardware before and after
streaming is started and stopped. This replaces the software reset
(SRST.SRST) control.
While at it break out the full start and stop procedures from
rcsi2_s_stream() into the existing helper functions.
Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
drivers/media/platform/rcar-vin/Kconfig | 1 +
drivers/media/platform/rcar-vin/rcar-csi2.c | 69 +++++++++++++--------
2 files changed, 43 insertions(+), 27 deletions(-)
diff --git a/drivers/media/platform/rcar-vin/Kconfig b/drivers/media/platform/rcar-vin/Kconfig
index e3eb8fee253658da..f26f47e3bcf44825 100644
--- a/drivers/media/platform/rcar-vin/Kconfig
+++ b/drivers/media/platform/rcar-vin/Kconfig
@@ -3,6 +3,7 @@ config VIDEO_RCAR_CSI2
tristate "R-Car MIPI CSI-2 Receiver"
depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && OF
depends on ARCH_RENESAS || COMPILE_TEST
+ select RESET_CONTROLLER
select V4L2_FWNODE
help
Support for Renesas R-Car MIPI CSI-2 receiver.
diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
index f64528d2be3c95dd..10f1b4978ed7dcc6 100644
--- a/drivers/media/platform/rcar-vin/rcar-csi2.c
+++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
@@ -14,6 +14,7 @@
#include <linux/of_graph.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
+#include <linux/reset.h>
#include <linux/sys_soc.h>
#include <media/v4l2-ctrls.h>
@@ -350,6 +351,7 @@ struct rcar_csi2 {
struct device *dev;
void __iomem *base;
const struct rcar_csi2_info *info;
+ struct reset_control *rstc;
struct v4l2_subdev subdev;
struct media_pad pads[NR_OF_RCAR_CSI2_PAD];
@@ -387,11 +389,19 @@ static void rcsi2_write(struct rcar_csi2 *priv, unsigned int reg, u32 data)
iowrite32(data, priv->base + reg);
}
-static void rcsi2_reset(struct rcar_csi2 *priv)
+static void rcsi2_enter_standby(struct rcar_csi2 *priv)
{
- rcsi2_write(priv, SRST_REG, SRST_SRST);
+ rcsi2_write(priv, PHYCNT_REG, 0);
+ rcsi2_write(priv, PHTC_REG, PHTC_TESTCLR);
+ reset_control_assert(priv->rstc);
usleep_range(100, 150);
- rcsi2_write(priv, SRST_REG, 0);
+ pm_runtime_put(priv->dev);
+}
+
+static void rcsi2_exit_standby(struct rcar_csi2 *priv)
+{
+ pm_runtime_get_sync(priv->dev);
+ reset_control_deassert(priv->rstc);
}
static int rcsi2_wait_phy_start(struct rcar_csi2 *priv)
@@ -462,7 +472,7 @@ static int rcsi2_calc_mbps(struct rcar_csi2 *priv, unsigned int bpp)
return mbps;
}
-static int rcsi2_start(struct rcar_csi2 *priv)
+static int rcsi2_start_receiver(struct rcar_csi2 *priv)
{
const struct rcar_csi2_format *format;
u32 phycnt, vcdt = 0, vcdt2 = 0;
@@ -506,7 +516,6 @@ static int rcsi2_start(struct rcar_csi2 *priv)
/* Init */
rcsi2_write(priv, TREF_REG, TREF_TREF);
- rcsi2_reset(priv);
rcsi2_write(priv, PHTC_REG, 0);
/* Configure */
@@ -564,19 +573,36 @@ static int rcsi2_start(struct rcar_csi2 *priv)
return 0;
}
+static int rcsi2_start(struct rcar_csi2 *priv)
+{
+ int ret;
+
+ rcsi2_exit_standby(priv);
+
+ ret = rcsi2_start_receiver(priv);
+ if (ret) {
+ rcsi2_enter_standby(priv);
+ return ret;
+ }
+
+ ret = v4l2_subdev_call(priv->remote, video, s_stream, 1);
+ if (ret) {
+ rcsi2_enter_standby(priv);
+ return ret;
+ }
+
+ return 0;
+}
+
static void rcsi2_stop(struct rcar_csi2 *priv)
{
- rcsi2_write(priv, PHYCNT_REG, 0);
-
- rcsi2_reset(priv);
-
- rcsi2_write(priv, PHTC_REG, PHTC_TESTCLR);
+ rcsi2_enter_standby(priv);
+ v4l2_subdev_call(priv->remote, video, s_stream, 0);
}
static int rcsi2_s_stream(struct v4l2_subdev *sd, int enable)
{
struct rcar_csi2 *priv = sd_to_csi2(sd);
- struct v4l2_subdev *nextsd;
int ret = 0;
mutex_lock(&priv->lock);
@@ -586,27 +612,12 @@ static int rcsi2_s_stream(struct v4l2_subdev *sd, int enable)
goto out;
}
- nextsd = priv->remote;
-
if (enable && priv->stream_count == 0) {
- pm_runtime_get_sync(priv->dev);
-
ret = rcsi2_start(priv);
- if (ret) {
- pm_runtime_put(priv->dev);
+ if (ret)
goto out;
- }
-
- ret = v4l2_subdev_call(nextsd, video, s_stream, 1);
- if (ret) {
- rcsi2_stop(priv);
- pm_runtime_put(priv->dev);
- goto out;
- }
} else if (!enable && priv->stream_count == 1) {
rcsi2_stop(priv);
- v4l2_subdev_call(nextsd, video, s_stream, 0);
- pm_runtime_put(priv->dev);
}
priv->stream_count += enable ? 1 : -1;
@@ -936,6 +947,10 @@ static int rcsi2_probe_resources(struct rcar_csi2 *priv,
if (irq < 0)
return irq;
+ priv->rstc = devm_reset_control_get(&pdev->dev, NULL);
+ if (IS_ERR(priv->rstc))
+ return PTR_ERR(priv->rstc);
+
return 0;
}
--
2.21.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 2/2] rcar-csi2: Use standby mode instead of resetting
2019-03-12 23:49 ` [PATCH v3 2/2] rcar-csi2: Use standby mode instead of resetting Niklas Söderlund
@ 2019-03-13 0:18 ` Laurent Pinchart
0 siblings, 0 replies; 6+ messages in thread
From: Laurent Pinchart @ 2019-03-13 0:18 UTC (permalink / raw)
To: Niklas Söderlund; +Cc: linux-media, linux-renesas-soc
Hi Niklas,
Thank you for the patch.
On Wed, Mar 13, 2019 at 12:49:30AM +0100, Niklas Söderlund wrote:
> Later versions of the datasheet updates the reset procedure to more
> closely resemble the standby mode. Update the driver to enter and exit
> the standby mode instead of resetting the hardware before and after
> streaming is started and stopped. This replaces the software reset
> (SRST.SRST) control.
>
> While at it break out the full start and stop procedures from
> rcsi2_s_stream() into the existing helper functions.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/media/platform/rcar-vin/Kconfig | 1 +
> drivers/media/platform/rcar-vin/rcar-csi2.c | 69 +++++++++++++--------
> 2 files changed, 43 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/media/platform/rcar-vin/Kconfig b/drivers/media/platform/rcar-vin/Kconfig
> index e3eb8fee253658da..f26f47e3bcf44825 100644
> --- a/drivers/media/platform/rcar-vin/Kconfig
> +++ b/drivers/media/platform/rcar-vin/Kconfig
> @@ -3,6 +3,7 @@ config VIDEO_RCAR_CSI2
> tristate "R-Car MIPI CSI-2 Receiver"
> depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && OF
> depends on ARCH_RENESAS || COMPILE_TEST
> + select RESET_CONTROLLER
> select V4L2_FWNODE
> help
> Support for Renesas R-Car MIPI CSI-2 receiver.
> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
> index f64528d2be3c95dd..10f1b4978ed7dcc6 100644
> --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> @@ -14,6 +14,7 @@
> #include <linux/of_graph.h>
> #include <linux/platform_device.h>
> #include <linux/pm_runtime.h>
> +#include <linux/reset.h>
> #include <linux/sys_soc.h>
>
> #include <media/v4l2-ctrls.h>
> @@ -350,6 +351,7 @@ struct rcar_csi2 {
> struct device *dev;
> void __iomem *base;
> const struct rcar_csi2_info *info;
> + struct reset_control *rstc;
>
> struct v4l2_subdev subdev;
> struct media_pad pads[NR_OF_RCAR_CSI2_PAD];
> @@ -387,11 +389,19 @@ static void rcsi2_write(struct rcar_csi2 *priv, unsigned int reg, u32 data)
> iowrite32(data, priv->base + reg);
> }
>
> -static void rcsi2_reset(struct rcar_csi2 *priv)
> +static void rcsi2_enter_standby(struct rcar_csi2 *priv)
> {
> - rcsi2_write(priv, SRST_REG, SRST_SRST);
> + rcsi2_write(priv, PHYCNT_REG, 0);
> + rcsi2_write(priv, PHTC_REG, PHTC_TESTCLR);
> + reset_control_assert(priv->rstc);
> usleep_range(100, 150);
> - rcsi2_write(priv, SRST_REG, 0);
> + pm_runtime_put(priv->dev);
> +}
> +
> +static void rcsi2_exit_standby(struct rcar_csi2 *priv)
> +{
> + pm_runtime_get_sync(priv->dev);
> + reset_control_deassert(priv->rstc);
> }
>
> static int rcsi2_wait_phy_start(struct rcar_csi2 *priv)
> @@ -462,7 +472,7 @@ static int rcsi2_calc_mbps(struct rcar_csi2 *priv, unsigned int bpp)
> return mbps;
> }
>
> -static int rcsi2_start(struct rcar_csi2 *priv)
> +static int rcsi2_start_receiver(struct rcar_csi2 *priv)
> {
> const struct rcar_csi2_format *format;
> u32 phycnt, vcdt = 0, vcdt2 = 0;
> @@ -506,7 +516,6 @@ static int rcsi2_start(struct rcar_csi2 *priv)
>
> /* Init */
> rcsi2_write(priv, TREF_REG, TREF_TREF);
> - rcsi2_reset(priv);
> rcsi2_write(priv, PHTC_REG, 0);
>
> /* Configure */
> @@ -564,19 +573,36 @@ static int rcsi2_start(struct rcar_csi2 *priv)
> return 0;
> }
>
> +static int rcsi2_start(struct rcar_csi2 *priv)
> +{
> + int ret;
> +
> + rcsi2_exit_standby(priv);
> +
> + ret = rcsi2_start_receiver(priv);
> + if (ret) {
> + rcsi2_enter_standby(priv);
> + return ret;
> + }
> +
> + ret = v4l2_subdev_call(priv->remote, video, s_stream, 1);
> + if (ret) {
> + rcsi2_enter_standby(priv);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> static void rcsi2_stop(struct rcar_csi2 *priv)
> {
> - rcsi2_write(priv, PHYCNT_REG, 0);
> -
> - rcsi2_reset(priv);
> -
> - rcsi2_write(priv, PHTC_REG, PHTC_TESTCLR);
> + rcsi2_enter_standby(priv);
> + v4l2_subdev_call(priv->remote, video, s_stream, 0);
> }
>
> static int rcsi2_s_stream(struct v4l2_subdev *sd, int enable)
> {
> struct rcar_csi2 *priv = sd_to_csi2(sd);
> - struct v4l2_subdev *nextsd;
> int ret = 0;
>
> mutex_lock(&priv->lock);
> @@ -586,27 +612,12 @@ static int rcsi2_s_stream(struct v4l2_subdev *sd, int enable)
> goto out;
> }
>
> - nextsd = priv->remote;
> -
> if (enable && priv->stream_count == 0) {
> - pm_runtime_get_sync(priv->dev);
> -
> ret = rcsi2_start(priv);
> - if (ret) {
> - pm_runtime_put(priv->dev);
> + if (ret)
> goto out;
> - }
> -
> - ret = v4l2_subdev_call(nextsd, video, s_stream, 1);
> - if (ret) {
> - rcsi2_stop(priv);
> - pm_runtime_put(priv->dev);
> - goto out;
> - }
> } else if (!enable && priv->stream_count == 1) {
> rcsi2_stop(priv);
> - v4l2_subdev_call(nextsd, video, s_stream, 0);
> - pm_runtime_put(priv->dev);
> }
>
> priv->stream_count += enable ? 1 : -1;
> @@ -936,6 +947,10 @@ static int rcsi2_probe_resources(struct rcar_csi2 *priv,
> if (irq < 0)
> return irq;
>
> + priv->rstc = devm_reset_control_get(&pdev->dev, NULL);
> + if (IS_ERR(priv->rstc))
> + return PTR_ERR(priv->rstc);
> +
> return 0;
> }
>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: rcar-csi2: List resets as a mandatory property
2019-03-12 23:49 ` [PATCH v3 1/2] dt-bindings: rcar-csi2: List resets as a mandatory property Niklas Söderlund
@ 2019-04-12 7:03 ` Jacopo Mondi
2019-04-12 7:07 ` Jacopo Mondi
0 siblings, 1 reply; 6+ messages in thread
From: Jacopo Mondi @ 2019-04-12 7:03 UTC (permalink / raw)
To: Niklas Söderlund
Cc: Laurent Pinchart, linux-media, linux-renesas-soc, devicetree,
Rob Herring
[-- Attachment #1: Type: text/plain, Size: 1682 bytes --]
Hi Niklas,
On Wed, Mar 13, 2019 at 12:49:29AM +0100, Niklas Söderlund wrote:
> The resets property will become mandatory to operate the device, list it
> as such. All device tree source files have always included the reset
> property so making it mandatory will not introduce any regressions.
>
> While at it improve the description for the clocks property.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
> Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
> index d63275e17afdd180..9a0d0531c67df48c 100644
> --- a/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
> +++ b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
> @@ -18,7 +18,8 @@ Mandatory properties
>
> - reg: the register base and size for the device registers
> - interrupts: the interrupt for the device
> - - clocks: reference to the parent clock
> + - clocks: A phandle + clock specifier for the module clock
> + - resets: A phandle + reset specifier for the module reset
>
Verified that all users in mainline of this bindings already contains
a 'reset' property:
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Thanks
j
> The device node shall contain two 'port' child nodes according to the
> bindings defined in Documentation/devicetree/bindings/media/
> --
> 2.21.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: rcar-csi2: List resets as a mandatory property
2019-04-12 7:03 ` Jacopo Mondi
@ 2019-04-12 7:07 ` Jacopo Mondi
0 siblings, 0 replies; 6+ messages in thread
From: Jacopo Mondi @ 2019-04-12 7:07 UTC (permalink / raw)
To: Niklas Söderlund
Cc: Laurent Pinchart, linux-media, linux-renesas-soc, devicetree,
Rob Herring
[-- Attachment #1: Type: text/plain, Size: 1910 bytes --]
Oh, I missed a pull request was sent already for this series :)
Even better!
On Fri, Apr 12, 2019 at 09:03:38AM +0200, Jacopo Mondi wrote:
> Hi Niklas,
>
> On Wed, Mar 13, 2019 at 12:49:29AM +0100, Niklas Söderlund wrote:
> > The resets property will become mandatory to operate the device, list it
> > as such. All device tree source files have always included the reset
> > property so making it mandatory will not introduce any regressions.
> >
> > While at it improve the description for the clocks property.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > ---
> > Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
> > index d63275e17afdd180..9a0d0531c67df48c 100644
> > --- a/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
> > +++ b/Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt
> > @@ -18,7 +18,8 @@ Mandatory properties
> >
> > - reg: the register base and size for the device registers
> > - interrupts: the interrupt for the device
> > - - clocks: reference to the parent clock
> > + - clocks: A phandle + clock specifier for the module clock
> > + - resets: A phandle + reset specifier for the module reset
> >
>
> Verified that all users in mainline of this bindings already contains
> a 'reset' property:
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
>
> Thanks
> j
>
> > The device node shall contain two 'port' child nodes according to the
> > bindings defined in Documentation/devicetree/bindings/media/
> > --
> > 2.21.0
> >
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-04-12 7:06 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-12 23:49 [PATCH v3 0/2] rcar-csi2: Use standby mode instead of resetting Niklas Söderlund
2019-03-12 23:49 ` [PATCH v3 1/2] dt-bindings: rcar-csi2: List resets as a mandatory property Niklas Söderlund
2019-04-12 7:03 ` Jacopo Mondi
2019-04-12 7:07 ` Jacopo Mondi
2019-03-12 23:49 ` [PATCH v3 2/2] rcar-csi2: Use standby mode instead of resetting Niklas Söderlund
2019-03-13 0:18 ` Laurent Pinchart
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).