Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH] drm/arcpgu: Get rid of "encoder-slave" property
From: Liviu.Dudau @ 2017-03-29 13:48 UTC (permalink / raw)
  To: Alexey Brodkin
  Cc: robh@kernel.org, linux-kernel@vger.kernel.org, robh+dt@kernel.org,
	devicetree@vger.kernel.org, linux-snps-arc@lists.infradead.org,
	Eugeniy Paltsev, daniel@ffwll.ch, dri-devel@lists.freedesktop.org,
	airlied@linux.ie
In-Reply-To: <1490794439.7754.88.camel@synopsys.com>

On Wed, Mar 29, 2017 at 01:34:00PM +0000, Alexey Brodkin wrote:
> Hi Liviu, Rob,

Hi Alexey,

> 
> On Fri, 2017-03-03 at 18:21 +0000, Liviu.Dudau@arm.com wrote:
> > On Fri, Mar 03, 2017 at 05:48:19PM +0000, Alexey Brodkin wrote:
> > > 
> > > Hi Liviu,
> > > 
> > > On Fri, 2017-03-03 at 16:28 +0000, Liviu Dudau wrote:
> > > > 
> > > > On Fri, Mar 03, 2017 at 06:19:24PM +0300, Alexey Brodkin wrote:
> > > > > 
> > > > > 
> > > > > -	/* find the encoder node and initialize it */
> > > > > -	encoder_node = of_parse_phandle(drm->dev->of_node, "encoder-slave", 0);
> > > > > -	if (encoder_node) {
> > > > > -		ret = arcpgu_drm_hdmi_init(drm, encoder_node);
> > > > > -		of_node_put(encoder_node);
> > > > > +	/* There is only one output port inside each device, find it */
> > > > > +	port = of_graph_get_next_endpoint(pdev->dev.of_node, NULL);
> > > > > +
> > > > > +	if (port) {
> > > > > +		if (of_device_is_available(port))
> > > > > +			encoder = of_graph_get_remote_port_parent(port);
> > > > > +		of_node_put(port);
> > > > > +	}
> > > > 
> > > > You must've been looking at some old version. Current version in -next uses
> > > > of_graph_get_remote_node() to replace all those lines you have added (see Rob
> > > > Herring's series to introduce of_graph_get_remote_node() function)
> > > 
> > > Hm, I'm not on Linus' master tree [1] and so I thought I was quite up to date :)
> > > Still I made a check of linux-next and don't see any changes in
> > > "drivers/gpu/drm/arm" compared to Linus' tree.
> > > 
> > > [1] https://urldefense.proofpoint.com/v2/url?u=http-3A__git.kernel.org_cgit_linux_kernel_git_torvalds_linux.git_commit_drivers_gpu_drm_arm-3Fid-3D
> > > e4563f6ba71792c77aeccb2092cc23149b44e642&d=DwIDaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=lqdeeSSEes0GFDDl656eViXO7breS55ytWkhpk5R81I&m=SI66ngnnXy33ncb8m5H4La2
> > > T1SzSEiiP7hc_XsRahEc&s=uaswjVXcjYDrUosOkO_UpTMqJMWTT-LLPrg5JE6-t-8&e= 
> > > [2] https://urldefense.proofpoint.com/v2/url?u=http-3A__git.kernel.org_cgit_linux_kernel_git_next_linux-2Dnext.git_commit_drivers_gpu_drm_arm-3Fid
> > > -3De4563f6ba71792c77aeccb2092cc23149b44e642&d=DwIDaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=lqdeeSSEes0GFDDl656eViXO7breS55ytWkhpk5R81I&m=SI66ngnnXy33ncb8m5H4
> > > La2T1SzSEiiP7hc_XsRahEc&s=hl9Y6s3K9LwLL1M2WnL3ODax_V-ZRh8k1iTiyctIqU4&e= 
> > > 
> > > Could you please clarify which exact tree did you mean?
> > 
> > Sorry, I thought the series got pulled by one of the DRM trees, but it looks like
> > I was wrong. I was carrying a private copy in my internal tree, waiting for the
> > moment when it got pulled into drm-next or drm-misc-next.
> > 
> > Rob, do you have an update on your series introducing of_graph_get_remote_node() ?
> 
> For some reason I cannot find any relevant commits in linux-next tree even today.
> Could you please point me to either any random git tree with mentioned above change or
> maybe just mailing list where this patch was sent?

Not sure why Rob hasn't added it to linux-next, but (according to Rob) this is the latest version:

https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git/log/?h=of-graph-helpers

Best regards,
Liviu

> 
> I'd like to implement the same fix in ARCPGU and call it a day finally.
> 
> -Alexey

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

^ permalink raw reply

* Re: [PATCHv6 11/14] ov2640: convert from soc-camera to a standard subdev sensor driver.
From: Hans Verkuil @ 2017-03-29 13:44 UTC (permalink / raw)
  To: Hugues FRUCHET, linux-media@vger.kernel.org
  Cc: Guennadi Liakhovetski, Songjun Wu, Sakari Ailus,
	devicetree@vger.kernel.org, Hans Verkuil
In-Reply-To: <488e156b-2ceb-3895-6bcc-4785b16c93a5@st.com>

On 29/03/17 15:42, Hugues FRUCHET wrote:
> Acked-by: Hugues Fruchet <hugues.fruchet@st.com>
> 
> Tested successfully on STM324x9I-EVAL evaluation board embedding
> an OV2640 camera sensor.
> 
> I don't understand the comment around s_power op that has been dropped 
> (it is there in code), and no problem is observed doing several 
> open/close, tell me if I miss something.

Darn, I forgot to remove that comment in the commit log. It's a leftover from
an earlier version.

Regards,

	Hans

> 
> BR,
> Hugues.
> 
> On 03/28/2017 10:23 AM, Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> Convert ov2640 to a standard subdev driver. The soc-camera driver no longer
>> uses this driver, so it can safely be converted.
>>
>> Note: the s_power op has been dropped: this never worked. When the last open()
>> is closed, then the power is turned off, and when it is opened again the power
>> is turned on again, but the old state isn't restored.
>>
>> Someone else can figure out in the future how to get this working correctly,
>> but I don't want to spend more time on this.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>> ---
>>  drivers/media/i2c/Kconfig                   | 11 ++++
>>  drivers/media/i2c/Makefile                  |  1 +
>>  drivers/media/i2c/{soc_camera => }/ov2640.c | 89 +++++------------------------
>>  drivers/media/i2c/soc_camera/Kconfig        |  6 --
>>  drivers/media/i2c/soc_camera/Makefile       |  1 -
>>  5 files changed, 27 insertions(+), 81 deletions(-)
>>  rename drivers/media/i2c/{soc_camera => }/ov2640.c (94%)
>>
>> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
>> index cee1dae6e014..db2c63f592c5 100644
>> --- a/drivers/media/i2c/Kconfig
>> +++ b/drivers/media/i2c/Kconfig
>> @@ -520,6 +520,17 @@ config VIDEO_APTINA_PLL
>>  config VIDEO_SMIAPP_PLL
>>  	tristate
>>
>> +config VIDEO_OV2640
>> +	tristate "OmniVision OV2640 sensor support"
>> +	depends on VIDEO_V4L2 && I2C && GPIOLIB
>> +	depends on MEDIA_CAMERA_SUPPORT
>> +	help
>> +	  This is a Video4Linux2 sensor-level driver for the OmniVision
>> +	  OV2640 camera.
>> +
>> +	  To compile this driver as a module, choose M here: the
>> +	  module will be called ov2640.
>> +
>>  config VIDEO_OV2659
>>  	tristate "OmniVision OV2659 sensor support"
>>  	depends on VIDEO_V4L2 && I2C
>> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
>> index 5bc7bbeb5499..50af1e11c85a 100644
>> --- a/drivers/media/i2c/Makefile
>> +++ b/drivers/media/i2c/Makefile
>> @@ -57,6 +57,7 @@ obj-$(CONFIG_VIDEO_VP27SMPX) += vp27smpx.o
>>  obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o
>>  obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
>>  obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
>> +obj-$(CONFIG_VIDEO_OV2640) += ov2640.o
>>  obj-$(CONFIG_VIDEO_OV7640) += ov7640.o
>>  obj-$(CONFIG_VIDEO_OV7670) += ov7670.o
>>  obj-$(CONFIG_VIDEO_OV9650) += ov9650.o
>> diff --git a/drivers/media/i2c/soc_camera/ov2640.c b/drivers/media/i2c/ov2640.c
>> similarity index 94%
>> rename from drivers/media/i2c/soc_camera/ov2640.c
>> rename to drivers/media/i2c/ov2640.c
>> index b9a0069f5b33..83f88efbce69 100644
>> --- a/drivers/media/i2c/soc_camera/ov2640.c
>> +++ b/drivers/media/i2c/ov2640.c
>> @@ -24,8 +24,8 @@
>>  #include <linux/v4l2-mediabus.h>
>>  #include <linux/videodev2.h>
>>
>> -#include <media/soc_camera.h>
>>  #include <media/v4l2-clk.h>
>> +#include <media/v4l2-device.h>
>>  #include <media/v4l2-subdev.h>
>>  #include <media/v4l2-ctrls.h>
>>  #include <media/v4l2-image-sizes.h>
>> @@ -287,7 +287,6 @@ struct ov2640_priv {
>>  	struct v4l2_clk			*clk;
>>  	const struct ov2640_win_size	*win;
>>
>> -	struct soc_camera_subdev_desc	ssdd_dt;
>>  	struct gpio_desc *resetb_gpio;
>>  	struct gpio_desc *pwdn_gpio;
>>  };
>> @@ -677,13 +676,8 @@ static int ov2640_reset(struct i2c_client *client)
>>  }
>>
>>  /*
>> - * soc_camera_ops functions
>> + * functions
>>   */
>> -static int ov2640_s_stream(struct v4l2_subdev *sd, int enable)
>> -{
>> -	return 0;
>> -}
>> -
>>  static int ov2640_s_ctrl(struct v4l2_ctrl *ctrl)
>>  {
>>  	struct v4l2_subdev *sd =
>> @@ -744,10 +738,16 @@ static int ov2640_s_register(struct v4l2_subdev *sd,
>>  static int ov2640_s_power(struct v4l2_subdev *sd, int on)
>>  {
>>  	struct i2c_client *client = v4l2_get_subdevdata(sd);
>> -	struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
>>  	struct ov2640_priv *priv = to_ov2640(client);
>>
>> -	return soc_camera_set_power(&client->dev, ssdd, priv->clk, on);
>> +	gpiod_direction_output(priv->pwdn_gpio, !on);
>> +	if (on && priv->resetb_gpio) {
>> +		/* Active the resetb pin to perform a reset pulse */
>> +		gpiod_direction_output(priv->resetb_gpio, 1);
>> +		usleep_range(3000, 5000);
>> +		gpiod_direction_output(priv->resetb_gpio, 0);
>> +	}
>> +	return 0;
>>  }
>>
>>  /* Select the nearest higher resolution for capture */
>> @@ -994,26 +994,6 @@ static struct v4l2_subdev_core_ops ov2640_subdev_core_ops = {
>>  	.s_power	= ov2640_s_power,
>>  };
>>
>> -static int ov2640_g_mbus_config(struct v4l2_subdev *sd,
>> -				struct v4l2_mbus_config *cfg)
>> -{
>> -	struct i2c_client *client = v4l2_get_subdevdata(sd);
>> -	struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
>> -
>> -	cfg->flags = V4L2_MBUS_PCLK_SAMPLE_RISING | V4L2_MBUS_MASTER |
>> -		V4L2_MBUS_VSYNC_ACTIVE_HIGH | V4L2_MBUS_HSYNC_ACTIVE_HIGH |
>> -		V4L2_MBUS_DATA_ACTIVE_HIGH;
>> -	cfg->type = V4L2_MBUS_PARALLEL;
>> -	cfg->flags = soc_camera_apply_board_flags(ssdd, cfg);
>> -
>> -	return 0;
>> -}
>> -
>> -static struct v4l2_subdev_video_ops ov2640_subdev_video_ops = {
>> -	.s_stream	= ov2640_s_stream,
>> -	.g_mbus_config	= ov2640_g_mbus_config,
>> -};
>> -
>>  static const struct v4l2_subdev_pad_ops ov2640_subdev_pad_ops = {
>>  	.enum_mbus_code = ov2640_enum_mbus_code,
>>  	.get_selection	= ov2640_get_selection,
>> @@ -1023,40 +1003,9 @@ static const struct v4l2_subdev_pad_ops ov2640_subdev_pad_ops = {
>>
>>  static struct v4l2_subdev_ops ov2640_subdev_ops = {
>>  	.core	= &ov2640_subdev_core_ops,
>> -	.video	= &ov2640_subdev_video_ops,
>>  	.pad	= &ov2640_subdev_pad_ops,
>>  };
>>
>> -/* OF probe functions */
>> -static int ov2640_hw_power(struct device *dev, int on)
>> -{
>> -	struct i2c_client *client = to_i2c_client(dev);
>> -	struct ov2640_priv *priv = to_ov2640(client);
>> -
>> -	dev_dbg(&client->dev, "%s: %s the camera\n",
>> -			__func__, on ? "ENABLE" : "DISABLE");
>> -
>> -	if (priv->pwdn_gpio)
>> -		gpiod_direction_output(priv->pwdn_gpio, !on);
>> -
>> -	return 0;
>> -}
>> -
>> -static int ov2640_hw_reset(struct device *dev)
>> -{
>> -	struct i2c_client *client = to_i2c_client(dev);
>> -	struct ov2640_priv *priv = to_ov2640(client);
>> -
>> -	if (priv->resetb_gpio) {
>> -		/* Active the resetb pin to perform a reset pulse */
>> -		gpiod_direction_output(priv->resetb_gpio, 1);
>> -		usleep_range(3000, 5000);
>> -		gpiod_direction_output(priv->resetb_gpio, 0);
>> -	}
>> -
>> -	return 0;
>> -}
>> -
>>  static int ov2640_probe_dt(struct i2c_client *client,
>>  		struct ov2640_priv *priv)
>>  {
>> @@ -1076,11 +1025,6 @@ static int ov2640_probe_dt(struct i2c_client *client,
>>  	else if (IS_ERR(priv->pwdn_gpio))
>>  		return PTR_ERR(priv->pwdn_gpio);
>>
>> -	/* Initialize the soc_camera_subdev_desc */
>> -	priv->ssdd_dt.power = ov2640_hw_power;
>> -	priv->ssdd_dt.reset = ov2640_hw_reset;
>> -	client->dev.platform_data = &priv->ssdd_dt;
>> -
>>  	return 0;
>>  }
>>
>> @@ -1091,7 +1035,6 @@ static int ov2640_probe(struct i2c_client *client,
>>  			const struct i2c_device_id *did)
>>  {
>>  	struct ov2640_priv	*priv;
>> -	struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
>>  	struct i2c_adapter	*adapter = to_i2c_adapter(client->dev.parent);
>>  	int			ret;
>>
>> @@ -1112,17 +1055,15 @@ static int ov2640_probe(struct i2c_client *client,
>>  	if (IS_ERR(priv->clk))
>>  		return -EPROBE_DEFER;
>>
>> -	if (!ssdd && !client->dev.of_node) {
>> +	if (!client->dev.of_node) {
>>  		dev_err(&client->dev, "Missing platform_data for driver\n");
>>  		ret = -EINVAL;
>>  		goto err_clk;
>>  	}
>>
>> -	if (!ssdd) {
>> -		ret = ov2640_probe_dt(client, priv);
>> -		if (ret)
>> -			goto err_clk;
>> -	}
>> +	ret = ov2640_probe_dt(client, priv);
>> +	if (ret)
>> +		goto err_clk;
>>
>>  	v4l2_i2c_subdev_init(&priv->subdev, client, &ov2640_subdev_ops);
>>  	v4l2_ctrl_handler_init(&priv->hdl, 2);
>> @@ -1190,6 +1131,6 @@ static struct i2c_driver ov2640_i2c_driver = {
>>
>>  module_i2c_driver(ov2640_i2c_driver);
>>
>> -MODULE_DESCRIPTION("SoC Camera driver for Omni Vision 2640 sensor");
>> +MODULE_DESCRIPTION("Driver for Omni Vision 2640 sensor");
>>  MODULE_AUTHOR("Alberto Panizzo");
>>  MODULE_LICENSE("GPL v2");
>> diff --git a/drivers/media/i2c/soc_camera/Kconfig b/drivers/media/i2c/soc_camera/Kconfig
>> index 7704bcf5cc25..96859f37cb1c 100644
>> --- a/drivers/media/i2c/soc_camera/Kconfig
>> +++ b/drivers/media/i2c/soc_camera/Kconfig
>> @@ -41,12 +41,6 @@ config SOC_CAMERA_MT9V022
>>  	help
>>  	  This driver supports MT9V022 cameras from Micron
>>
>> -config SOC_CAMERA_OV2640
>> -	tristate "ov2640 camera support"
>> -	depends on SOC_CAMERA && I2C
>> -	help
>> -	  This is a ov2640 camera driver
>> -
>>  config SOC_CAMERA_OV5642
>>  	tristate "ov5642 camera support"
>>  	depends on SOC_CAMERA && I2C
>> diff --git a/drivers/media/i2c/soc_camera/Makefile b/drivers/media/i2c/soc_camera/Makefile
>> index 6f994f9353a0..974bdb721dbe 100644
>> --- a/drivers/media/i2c/soc_camera/Makefile
>> +++ b/drivers/media/i2c/soc_camera/Makefile
>> @@ -3,7 +3,6 @@ obj-$(CONFIG_SOC_CAMERA_MT9M001)	+= mt9m001.o
>>  obj-$(CONFIG_SOC_CAMERA_MT9T031)	+= mt9t031.o
>>  obj-$(CONFIG_SOC_CAMERA_MT9T112)	+= mt9t112.o
>>  obj-$(CONFIG_SOC_CAMERA_MT9V022)	+= mt9v022.o
>> -obj-$(CONFIG_SOC_CAMERA_OV2640)		+= ov2640.o
>>  obj-$(CONFIG_SOC_CAMERA_OV5642)		+= ov5642.o
>>  obj-$(CONFIG_SOC_CAMERA_OV6650)		+= ov6650.o
>>  obj-$(CONFIG_SOC_CAMERA_OV772X)		+= ov772x.o
>>

^ permalink raw reply

* Re: [PATCHv6 12/14] ov2640: use standard clk and enable it.
From: Hugues FRUCHET @ 2017-03-29 13:42 UTC (permalink / raw)
  To: Hans Verkuil, linux-media@vger.kernel.org
  Cc: Guennadi Liakhovetski, Songjun Wu, Sakari Ailus,
	devicetree@vger.kernel.org, Hans Verkuil
In-Reply-To: <20170328082347.11159-13-hverkuil@xs4all.nl>

Acked-by: Hugues Fruchet <hugues.fruchet@st.com>

Tested successfully on STM324x9I-EVAL evaluation board embedding
an OV2640 camera sensor.

BR,
Hugues.

On 03/28/2017 10:23 AM, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> Convert v4l2_clk to normal clk and enable the clock.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/i2c/ov2640.c | 31 ++++++++++++++-----------------
>  1 file changed, 14 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/media/i2c/ov2640.c b/drivers/media/i2c/ov2640.c
> index 83f88efbce69..0445963c5fae 100644
> --- a/drivers/media/i2c/ov2640.c
> +++ b/drivers/media/i2c/ov2640.c
> @@ -16,6 +16,7 @@
>  #include <linux/init.h>
>  #include <linux/module.h>
>  #include <linux/i2c.h>
> +#include <linux/clk.h>
>  #include <linux/slab.h>
>  #include <linux/delay.h>
>  #include <linux/gpio.h>
> @@ -24,7 +25,6 @@
>  #include <linux/v4l2-mediabus.h>
>  #include <linux/videodev2.h>
>
> -#include <media/v4l2-clk.h>
>  #include <media/v4l2-device.h>
>  #include <media/v4l2-subdev.h>
>  #include <media/v4l2-ctrls.h>
> @@ -284,7 +284,7 @@ struct ov2640_priv {
>  	struct v4l2_subdev		subdev;
>  	struct v4l2_ctrl_handler	hdl;
>  	u32	cfmt_code;
> -	struct v4l2_clk			*clk;
> +	struct clk			*clk;
>  	const struct ov2640_win_size	*win;
>
>  	struct gpio_desc *resetb_gpio;
> @@ -1051,14 +1051,11 @@ static int ov2640_probe(struct i2c_client *client,
>  		return -ENOMEM;
>  	}
>
> -	priv->clk = v4l2_clk_get(&client->dev, "xvclk");
> -	if (IS_ERR(priv->clk))
> -		return -EPROBE_DEFER;
> -
> -	if (!client->dev.of_node) {
> -		dev_err(&client->dev, "Missing platform_data for driver\n");
> -		ret = -EINVAL;
> -		goto err_clk;
> +	if (client->dev.of_node) {
> +		priv->clk = devm_clk_get(&client->dev, "xvclk");
> +		if (IS_ERR(priv->clk))
> +			return -EPROBE_DEFER;
> +		clk_prepare_enable(priv->clk);
>  	}
>
>  	ret = ov2640_probe_dt(client, priv);
> @@ -1074,25 +1071,25 @@ static int ov2640_probe(struct i2c_client *client,
>  	priv->subdev.ctrl_handler = &priv->hdl;
>  	if (priv->hdl.error) {
>  		ret = priv->hdl.error;
> -		goto err_clk;
> +		goto err_hdl;
>  	}
>
>  	ret = ov2640_video_probe(client);
>  	if (ret < 0)
> -		goto err_videoprobe;
> +		goto err_hdl;
>
>  	ret = v4l2_async_register_subdev(&priv->subdev);
>  	if (ret < 0)
> -		goto err_videoprobe;
> +		goto err_hdl;
>
>  	dev_info(&adapter->dev, "OV2640 Probed\n");
>
>  	return 0;
>
> -err_videoprobe:
> +err_hdl:
>  	v4l2_ctrl_handler_free(&priv->hdl);
>  err_clk:
> -	v4l2_clk_put(priv->clk);
> +	clk_disable_unprepare(priv->clk);
>  	return ret;
>  }
>
> @@ -1101,9 +1098,9 @@ static int ov2640_remove(struct i2c_client *client)
>  	struct ov2640_priv       *priv = to_ov2640(client);
>
>  	v4l2_async_unregister_subdev(&priv->subdev);
> -	v4l2_clk_put(priv->clk);
> -	v4l2_device_unregister_subdev(&priv->subdev);
>  	v4l2_ctrl_handler_free(&priv->hdl);
> +	v4l2_device_unregister_subdev(&priv->subdev);
> +	clk_disable_unprepare(priv->clk);
>  	return 0;
>  }
>
>

^ permalink raw reply

* Re: [PATCHv6 11/14] ov2640: convert from soc-camera to a standard subdev sensor driver.
From: Hugues FRUCHET @ 2017-03-29 13:42 UTC (permalink / raw)
  To: Hans Verkuil, linux-media@vger.kernel.org
  Cc: Guennadi Liakhovetski, Songjun Wu, Sakari Ailus,
	devicetree@vger.kernel.org, Hans Verkuil
In-Reply-To: <20170328082347.11159-12-hverkuil@xs4all.nl>

Acked-by: Hugues Fruchet <hugues.fruchet@st.com>

Tested successfully on STM324x9I-EVAL evaluation board embedding
an OV2640 camera sensor.

I don't understand the comment around s_power op that has been dropped 
(it is there in code), and no problem is observed doing several 
open/close, tell me if I miss something.

BR,
Hugues.

On 03/28/2017 10:23 AM, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> Convert ov2640 to a standard subdev driver. The soc-camera driver no longer
> uses this driver, so it can safely be converted.
>
> Note: the s_power op has been dropped: this never worked. When the last open()
> is closed, then the power is turned off, and when it is opened again the power
> is turned on again, but the old state isn't restored.
>
> Someone else can figure out in the future how to get this working correctly,
> but I don't want to spend more time on this.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/i2c/Kconfig                   | 11 ++++
>  drivers/media/i2c/Makefile                  |  1 +
>  drivers/media/i2c/{soc_camera => }/ov2640.c | 89 +++++------------------------
>  drivers/media/i2c/soc_camera/Kconfig        |  6 --
>  drivers/media/i2c/soc_camera/Makefile       |  1 -
>  5 files changed, 27 insertions(+), 81 deletions(-)
>  rename drivers/media/i2c/{soc_camera => }/ov2640.c (94%)
>
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index cee1dae6e014..db2c63f592c5 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -520,6 +520,17 @@ config VIDEO_APTINA_PLL
>  config VIDEO_SMIAPP_PLL
>  	tristate
>
> +config VIDEO_OV2640
> +	tristate "OmniVision OV2640 sensor support"
> +	depends on VIDEO_V4L2 && I2C && GPIOLIB
> +	depends on MEDIA_CAMERA_SUPPORT
> +	help
> +	  This is a Video4Linux2 sensor-level driver for the OmniVision
> +	  OV2640 camera.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called ov2640.
> +
>  config VIDEO_OV2659
>  	tristate "OmniVision OV2659 sensor support"
>  	depends on VIDEO_V4L2 && I2C
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index 5bc7bbeb5499..50af1e11c85a 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -57,6 +57,7 @@ obj-$(CONFIG_VIDEO_VP27SMPX) += vp27smpx.o
>  obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o
>  obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
>  obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
> +obj-$(CONFIG_VIDEO_OV2640) += ov2640.o
>  obj-$(CONFIG_VIDEO_OV7640) += ov7640.o
>  obj-$(CONFIG_VIDEO_OV7670) += ov7670.o
>  obj-$(CONFIG_VIDEO_OV9650) += ov9650.o
> diff --git a/drivers/media/i2c/soc_camera/ov2640.c b/drivers/media/i2c/ov2640.c
> similarity index 94%
> rename from drivers/media/i2c/soc_camera/ov2640.c
> rename to drivers/media/i2c/ov2640.c
> index b9a0069f5b33..83f88efbce69 100644
> --- a/drivers/media/i2c/soc_camera/ov2640.c
> +++ b/drivers/media/i2c/ov2640.c
> @@ -24,8 +24,8 @@
>  #include <linux/v4l2-mediabus.h>
>  #include <linux/videodev2.h>
>
> -#include <media/soc_camera.h>
>  #include <media/v4l2-clk.h>
> +#include <media/v4l2-device.h>
>  #include <media/v4l2-subdev.h>
>  #include <media/v4l2-ctrls.h>
>  #include <media/v4l2-image-sizes.h>
> @@ -287,7 +287,6 @@ struct ov2640_priv {
>  	struct v4l2_clk			*clk;
>  	const struct ov2640_win_size	*win;
>
> -	struct soc_camera_subdev_desc	ssdd_dt;
>  	struct gpio_desc *resetb_gpio;
>  	struct gpio_desc *pwdn_gpio;
>  };
> @@ -677,13 +676,8 @@ static int ov2640_reset(struct i2c_client *client)
>  }
>
>  /*
> - * soc_camera_ops functions
> + * functions
>   */
> -static int ov2640_s_stream(struct v4l2_subdev *sd, int enable)
> -{
> -	return 0;
> -}
> -
>  static int ov2640_s_ctrl(struct v4l2_ctrl *ctrl)
>  {
>  	struct v4l2_subdev *sd =
> @@ -744,10 +738,16 @@ static int ov2640_s_register(struct v4l2_subdev *sd,
>  static int ov2640_s_power(struct v4l2_subdev *sd, int on)
>  {
>  	struct i2c_client *client = v4l2_get_subdevdata(sd);
> -	struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
>  	struct ov2640_priv *priv = to_ov2640(client);
>
> -	return soc_camera_set_power(&client->dev, ssdd, priv->clk, on);
> +	gpiod_direction_output(priv->pwdn_gpio, !on);
> +	if (on && priv->resetb_gpio) {
> +		/* Active the resetb pin to perform a reset pulse */
> +		gpiod_direction_output(priv->resetb_gpio, 1);
> +		usleep_range(3000, 5000);
> +		gpiod_direction_output(priv->resetb_gpio, 0);
> +	}
> +	return 0;
>  }
>
>  /* Select the nearest higher resolution for capture */
> @@ -994,26 +994,6 @@ static struct v4l2_subdev_core_ops ov2640_subdev_core_ops = {
>  	.s_power	= ov2640_s_power,
>  };
>
> -static int ov2640_g_mbus_config(struct v4l2_subdev *sd,
> -				struct v4l2_mbus_config *cfg)
> -{
> -	struct i2c_client *client = v4l2_get_subdevdata(sd);
> -	struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
> -
> -	cfg->flags = V4L2_MBUS_PCLK_SAMPLE_RISING | V4L2_MBUS_MASTER |
> -		V4L2_MBUS_VSYNC_ACTIVE_HIGH | V4L2_MBUS_HSYNC_ACTIVE_HIGH |
> -		V4L2_MBUS_DATA_ACTIVE_HIGH;
> -	cfg->type = V4L2_MBUS_PARALLEL;
> -	cfg->flags = soc_camera_apply_board_flags(ssdd, cfg);
> -
> -	return 0;
> -}
> -
> -static struct v4l2_subdev_video_ops ov2640_subdev_video_ops = {
> -	.s_stream	= ov2640_s_stream,
> -	.g_mbus_config	= ov2640_g_mbus_config,
> -};
> -
>  static const struct v4l2_subdev_pad_ops ov2640_subdev_pad_ops = {
>  	.enum_mbus_code = ov2640_enum_mbus_code,
>  	.get_selection	= ov2640_get_selection,
> @@ -1023,40 +1003,9 @@ static const struct v4l2_subdev_pad_ops ov2640_subdev_pad_ops = {
>
>  static struct v4l2_subdev_ops ov2640_subdev_ops = {
>  	.core	= &ov2640_subdev_core_ops,
> -	.video	= &ov2640_subdev_video_ops,
>  	.pad	= &ov2640_subdev_pad_ops,
>  };
>
> -/* OF probe functions */
> -static int ov2640_hw_power(struct device *dev, int on)
> -{
> -	struct i2c_client *client = to_i2c_client(dev);
> -	struct ov2640_priv *priv = to_ov2640(client);
> -
> -	dev_dbg(&client->dev, "%s: %s the camera\n",
> -			__func__, on ? "ENABLE" : "DISABLE");
> -
> -	if (priv->pwdn_gpio)
> -		gpiod_direction_output(priv->pwdn_gpio, !on);
> -
> -	return 0;
> -}
> -
> -static int ov2640_hw_reset(struct device *dev)
> -{
> -	struct i2c_client *client = to_i2c_client(dev);
> -	struct ov2640_priv *priv = to_ov2640(client);
> -
> -	if (priv->resetb_gpio) {
> -		/* Active the resetb pin to perform a reset pulse */
> -		gpiod_direction_output(priv->resetb_gpio, 1);
> -		usleep_range(3000, 5000);
> -		gpiod_direction_output(priv->resetb_gpio, 0);
> -	}
> -
> -	return 0;
> -}
> -
>  static int ov2640_probe_dt(struct i2c_client *client,
>  		struct ov2640_priv *priv)
>  {
> @@ -1076,11 +1025,6 @@ static int ov2640_probe_dt(struct i2c_client *client,
>  	else if (IS_ERR(priv->pwdn_gpio))
>  		return PTR_ERR(priv->pwdn_gpio);
>
> -	/* Initialize the soc_camera_subdev_desc */
> -	priv->ssdd_dt.power = ov2640_hw_power;
> -	priv->ssdd_dt.reset = ov2640_hw_reset;
> -	client->dev.platform_data = &priv->ssdd_dt;
> -
>  	return 0;
>  }
>
> @@ -1091,7 +1035,6 @@ static int ov2640_probe(struct i2c_client *client,
>  			const struct i2c_device_id *did)
>  {
>  	struct ov2640_priv	*priv;
> -	struct soc_camera_subdev_desc *ssdd = soc_camera_i2c_to_desc(client);
>  	struct i2c_adapter	*adapter = to_i2c_adapter(client->dev.parent);
>  	int			ret;
>
> @@ -1112,17 +1055,15 @@ static int ov2640_probe(struct i2c_client *client,
>  	if (IS_ERR(priv->clk))
>  		return -EPROBE_DEFER;
>
> -	if (!ssdd && !client->dev.of_node) {
> +	if (!client->dev.of_node) {
>  		dev_err(&client->dev, "Missing platform_data for driver\n");
>  		ret = -EINVAL;
>  		goto err_clk;
>  	}
>
> -	if (!ssdd) {
> -		ret = ov2640_probe_dt(client, priv);
> -		if (ret)
> -			goto err_clk;
> -	}
> +	ret = ov2640_probe_dt(client, priv);
> +	if (ret)
> +		goto err_clk;
>
>  	v4l2_i2c_subdev_init(&priv->subdev, client, &ov2640_subdev_ops);
>  	v4l2_ctrl_handler_init(&priv->hdl, 2);
> @@ -1190,6 +1131,6 @@ static struct i2c_driver ov2640_i2c_driver = {
>
>  module_i2c_driver(ov2640_i2c_driver);
>
> -MODULE_DESCRIPTION("SoC Camera driver for Omni Vision 2640 sensor");
> +MODULE_DESCRIPTION("Driver for Omni Vision 2640 sensor");
>  MODULE_AUTHOR("Alberto Panizzo");
>  MODULE_LICENSE("GPL v2");
> diff --git a/drivers/media/i2c/soc_camera/Kconfig b/drivers/media/i2c/soc_camera/Kconfig
> index 7704bcf5cc25..96859f37cb1c 100644
> --- a/drivers/media/i2c/soc_camera/Kconfig
> +++ b/drivers/media/i2c/soc_camera/Kconfig
> @@ -41,12 +41,6 @@ config SOC_CAMERA_MT9V022
>  	help
>  	  This driver supports MT9V022 cameras from Micron
>
> -config SOC_CAMERA_OV2640
> -	tristate "ov2640 camera support"
> -	depends on SOC_CAMERA && I2C
> -	help
> -	  This is a ov2640 camera driver
> -
>  config SOC_CAMERA_OV5642
>  	tristate "ov5642 camera support"
>  	depends on SOC_CAMERA && I2C
> diff --git a/drivers/media/i2c/soc_camera/Makefile b/drivers/media/i2c/soc_camera/Makefile
> index 6f994f9353a0..974bdb721dbe 100644
> --- a/drivers/media/i2c/soc_camera/Makefile
> +++ b/drivers/media/i2c/soc_camera/Makefile
> @@ -3,7 +3,6 @@ obj-$(CONFIG_SOC_CAMERA_MT9M001)	+= mt9m001.o
>  obj-$(CONFIG_SOC_CAMERA_MT9T031)	+= mt9t031.o
>  obj-$(CONFIG_SOC_CAMERA_MT9T112)	+= mt9t112.o
>  obj-$(CONFIG_SOC_CAMERA_MT9V022)	+= mt9v022.o
> -obj-$(CONFIG_SOC_CAMERA_OV2640)		+= ov2640.o
>  obj-$(CONFIG_SOC_CAMERA_OV5642)		+= ov5642.o
>  obj-$(CONFIG_SOC_CAMERA_OV6650)		+= ov6650.o
>  obj-$(CONFIG_SOC_CAMERA_OV772X)		+= ov772x.o
>

^ permalink raw reply

* Re: [PATCH v7 1/7] clocksource/drivers/clksrc-evt-probe: Describe with the DT both the clocksource and the clockevent
From: Daniel Lezcano @ 2017-03-29 13:41 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Rob Herring, Alexander Kochetkov, Heiko Stuebner,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Thomas Gleixner,
	Russell King, Caesar Wang, Huang Tao
In-Reply-To: <20170329125713.GH23442@leverpostej>

On Wed, Mar 29, 2017 at 01:57:14PM +0100, Mark Rutland wrote:
> On Wed, Mar 29, 2017 at 02:36:38PM +0200, Daniel Lezcano wrote:
> > On Wed, Mar 29, 2017 at 11:49:11AM +0100, Mark Rutland wrote:
> > > On Wed, Mar 29, 2017 at 11:22:10AM +0200, Daniel Lezcano wrote:
> > > > On Tue, Mar 28, 2017 at 08:51:46PM -0500, Rob Herring wrote:
> > > > > On Wed, Mar 22, 2017 at 06:48:28PM +0300, Alexander Kochetkov wrote:
> > 
> > You can have several timers on the system and may want to use the clockevents
> > from one IP block and the clocksource from another IP block. For example, in
> > the case of a bogus clocksource.
> 
> I understand this. However, what I was trying to say is that *how* we
> use a particular device should be a software decision. I have a more
> concrete suggestion on that below.
> 
> > Moreover there are some drivers with a node for a clocksource and
> > another one for the clockevent, and the driver is assuming the clockevent is
> > defined first and then the clocksource.
> > 
> > eg.
> > 
> > arch/arc/boot/dts/abilis_tb10x.dtsi:
> > 
> >         /* TIMER0 with interrupt for clockevent */
> >         timer0 {
> >                 compatible = "snps,arc-timer";
> >                 interrupts = <3>;
> >                 interrupt-parent = <&intc>;
> >                 clocks = <&cpu_clk>;
> >         };
> > 
> >         /* TIMER1 for free running clocksource */
> >         timer1 {
> >                 compatible = "snps,arc-timer";
> >                 clocks = <&cpu_clk>;
> >         };
> > 
> > drivers/clocksource/arc_timer.c:
> > 
> > static int __init arc_of_timer_init(struct device_node *np)
> > {
> >         static int init_count = 0;
> >         int ret;
> > 
> >         if (!init_count) {
> >                 init_count = 1;
> >                 ret = arc_clockevent_setup(np);
> >         } else {
> >                 ret = arc_cs_setup_timer1(np);
> >         }
> > 
> >         return ret;
> > }
> > 
> > Even if that works, it is a fragile mechanism.
> > 
> > So the purpose of these changes is to provide a stronger timer declaration in
> > order to clearly split in the kernel a clocksource and a clockevent
> > initialization.
> 
> I agree that this pattern is not nice. However, I think that splitting
> devices as this level makes the problem *worse*.
> 
> Users care that they have a clocksource and a clockevent device. They
> do not care *which* particular device is used as either. The comments in
> the DT above are at best misleading.

Agree.

And the driver is assuming the first node is the clockevent and the second one
is the clocksource. If the DT invert these nodes, that breaks the driver.

> What we need is for the kernel to understand that devices can be both
> clockevent and clocksource (perhaps mutually exclusively), such that the
> kernel can decide how to make use of devices.
> 
> That way, for the above the kernel can figure out that timer0 could be
> used as clocksource or clockevent, while timer1 can only be used as a
> clocksource due to the lack of an interrupt. Thus, it can choose to use
> timer0 as a clockevent, and timer1 and a clocksource.

Well, 'interrupt' gives an indication the timer can be used as a clockevent and
clocksource, not the clockevent only.

If we take the case of the rockchip, the arm_arch_timer clocksource is stopped
when the CPU is clock gated. So specifically, we don't want to use this
clocksource but we want to use the arch clockevents because they are better.

> > > > > > With this approach, we allow a mechanism to clearly define a clocksource or a
> > > > > > clockevent without aerobatics we can find around in some drivers:
> > > > > > 	timer-sp804.c, arc-timer.c, dw_apb_timer_of.c, mps2-timer.c,
> > > > > > 	renesas-ostm.c, time-efm32.c, time-lpc32xx.c.
> > > > > 
> > > > > These all already have bindings and work. What problem are you trying to 
> > > > > solve other than restructuring Linux?
> > > > 
> > > > Yes, there is already the bindings, but that force to do some hackish
> > > > initialization.
> > > 
> > > Here, you are forcing hackish DT changes that do not truly describe HW.
> > > How is that better?
> > 
> > So if this is hackish DT changes, then the existing DTs should be fixed, no?
> 
> Yes.
> 
> For the above snippet, the only thing that needs to change is the
> comment.
> 
> > > > I would like to give the opportunity to declare separately a clocksource and a
> > > > clockevent, in order to have full control of how this is initialized.
> > > 
> > > To me it sounds like what we need is Linux infrastructure that allows
> > > one to register a device as having both clockevent/clocksource
> > > functionality.
> > 
> > That was the idea. Create a macro CLOCKEVENT_OF and CLOCKSOURCE_OF both of them
> > calling their respective init routines. And in addition a TIMER_OF doing both
> > CLOCKEVENT_OF and CLOCKSOURCE_OF.
> > 
> > It is the DT which does not allow to do this separation.
> > 
> > Would be the following approach more acceptable ?
> > 
> > 1. Replace all CLOCKSOURCE_OF by TIMER_OF (just renaming)
> 
> I am fine with this renaming.
> 
> > 2. A node can have a clockevent and|or a clocksource attributes
> 
> As above, this should not be in the DT given it's describing a
> (Linux-specific) SW policy and not a HW detail.
> 
> So I must disagree with this.

IIUC my discussion with Rob, an attribute is acceptable (btw if
'clocksource'|'clockevent' names are too Linux specific (+1), what
about a more generic name like 'tick' and 'time' ?).

> > 3. The timer_probe pass a flag to the driver's init function, so this one knows
> >    if it should invoke the clockevent/clocksource init functions.
> >    No attribute defaults to clocksource|clockevent.
> > 
> > That would be backward compatible and will let to create drivers with clutch
> > activated device via DT. Also, it will give the opportunity to the existing
> > drivers to change consolidate their initialization routines.
> 
> I think that if anything, we need a combined clocksource+clockevent
> device that we register to the core code. That means all
> clocksource/clockevent drivers have a consolidated routine.
> 
> Subsequently, core code should determine how specifically to use the
> device (e.g. based on what other devices are registered, and their
> capabilities).

IMO, the core code is complex enough and that may imply more heuristics.
Regarding the number of timers, I do believe it is much better to simply tell
which one we want to use via the DT (assuming the DT is a description of the
desired hardware, not all the available hardware).

-- 

 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 1/2] arc: axs10x: Add DT bindings for I2S audio playback
From: Jose Abreu @ 2017-03-29 13:38 UTC (permalink / raw)
  To: Vineet Gupta, robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-snps-arc-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <8b441b9d-bc26-091a-ac48-8594952759e8-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>

Hi Vineet, Rob,


On 20-03-2017 14:02, Jose Abreu wrote:
> Hi Vineet, Alexey, Rob,
>
>
> On 02-03-2017 18:57, Alexey Brodkin wrote:
>> Hi Jose,
>>
>> On Wed, 2017-02-22 at 18:19 +0000, Jose Abreu wrote:
>>> This patch adds the necessary DT bindings to get HDMI audio
>>> output in ARC AXS10x SDP. The bindings for I2S controller were
>>> added as well as the bindings for simple audio card.
>>>
>>> Signed-off-by: Jose Abreu <joabreu-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
>>> Cc: Carlos Palminha <palminha-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
>>> Cc: Alexey Brodkin <abrodkin-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
>>> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>> Cc: Vineet Gupta <vgupta-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
>>> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>> Cc: linux-snps-arc-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
>>> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>> ---
>>>  arch/arc/boot/dts/axs10x_mb.dtsi | 22 ++++++++++++++++++----
>>>  1 file changed, 18 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/arch/arc/boot/dts/axs10x_mb.dtsi b/arch/arc/boot/dts/axs10x_mb.dtsi
>>> index d6c1bbc..9d882b1 100644
>>> --- a/arch/arc/boot/dts/axs10x_mb.dtsi
>>> +++ b/arch/arc/boot/dts/axs10x_mb.dtsi
>>> @@ -149,12 +149,13 @@
>>>  			interrupts = <14>;
>>>  		};
>>>  
>>> -		i2c@0x1e000 {
>>> -			compatible = "snps,designware-i2c";
>>> +		i2s: i2s@1e000 {
>>> +			compatible = "snps,designware-i2s";
>>>  			reg = <0x1e000 0x100>;
>>> -			clock-frequency = <400000>;
>>> -			clocks = <&i2cclk>;
>>> +			clocks = <&i2sclk 0>;
>>> +			clock-names = "i2sclk";
>>>  			interrupts = <15>;
>>> +			#sound-dai-cells = <0>;
>>>  		};
>>>  
>>>  		i2c@0x1f000 {
>>> @@ -174,6 +175,7 @@
>>>  				adi,input-colorspace = "rgb";
>>>  				adi,input-clock = "1x";
>>>  				adi,clock-delay = <0x03>;
>>> +				#sound-dai-cells = <0>;
>>>  
>>>  				ports {
>>>  					#address-cells = <1>;
>>> @@ -295,5 +297,17 @@
>>>  				};
>>>  			};
>>>  		};
>>> +
>>> +		sound_playback {
>>> +			compatible = "simple-audio-card";
>>> +			simple-audio-card,name = "AXS10x HDMI Audio";
>>> +			simple-audio-card,format = "i2s";
>>> +			simple-audio-card,cpu {
>>> +				sound-dai = <&i2s>;
>>> +			};
>>> +			simple-audio-card,codec {
>>> +				sound-dai = <&adv7511>;
>>> +			};
>>> +		};
>>>  	};
>>>  };
>> Just for the sake of history that's my mods to defconfig that allowed me
>> to play .pcm via HDMI from axs103 board:
>> -------------------------------->8-------------------------------
>> diff --git a/arch/arc/configs/axs103_smp_defconfig b/arch/arc/configs/axs103_smp_defconfig
>> index 30a3d4cf53d2..b11362a32e4e 100644
>> --- a/arch/arc/configs/axs103_smp_defconfig
>> +++ b/arch/arc/configs/axs103_smp_defconfig
>> @@ -67,25 +67,29 @@ CONFIG_MOUSE_PS2_TOUCHKIT=y
>>  CONFIG_MOUSE_SERIAL=y
>>  CONFIG_MOUSE_SYNAPTICS_USB=y
>>  # CONFIG_LEGACY_PTYS is not set
>> -# CONFIG_DEVKMEM is not set
>>  CONFIG_SERIAL_8250=y
>>  CONFIG_SERIAL_8250_CONSOLE=y
>>  CONFIG_SERIAL_8250_DW=y
>>  CONFIG_SERIAL_OF_PLATFORM=y
>>  # CONFIG_HW_RANDOM is not set
>> -CONFIG_I2C=y
>>  CONFIG_I2C_CHARDEV=y
>>  CONFIG_I2C_DESIGNWARE_PLATFORM=y
>>  # CONFIG_HWMON is not set
>> -CONFIG_DRM=m
>> -CONFIG_DRM_I2C_ADV7511=m
>> -CONFIG_DRM_ARCPGU=m
>> -CONFIG_FB=y
>> +CONFIG_DRM=y
>> +CONFIG_DRM_I2C_ADV7511=y
>> +CONFIG_DRM_I2C_ADV7511_AUDIO=y
>> +CONFIG_DRM_ARCPGU=y
>>  CONFIG_FRAMEBUFFER_CONSOLE=y
>>  CONFIG_LOGO=y
>>  # CONFIG_LOGO_LINUX_MONO is not set
>>  # CONFIG_LOGO_LINUX_VGA16 is not set
>>  # CONFIG_LOGO_LINUX_CLUT224 is not set
>> +CONFIG_SOUND=y
>> +CONFIG_SND=y
>> +CONFIG_SND_SOC=y
>> +CONFIG_SND_DESIGNWARE_I2S=y
>> +CONFIG_SND_DESIGNWARE_PCM=y
>> +CONFIG_SND_SIMPLE_CARD=y
>>  CONFIG_USB_EHCI_HCD=y
>>  CONFIG_USB_EHCI_HCD_PLATFORM=y
>>  CONFIG_USB_OHCI_HCD=y
>> -------------------------------->8-------------------------------
>>
>> Anyways...
>>
>> Acked-by: Alexey Brodkin <abrodkin-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> Rob, can you please take a look at this patch and at 2/2?
>
> Alexey, do you still maintain your ack in these patches? (I'm
> asking because I remember you were having no video after patch
> 2/2 of this series). If so, Vineet, can you please pick this up
> and patch 2/2 also?
>
> Best regards,
> Jose Miguel Abreu
>
> _______________________________________________
> linux-snps-arc mailing list
> linux-snps-arc-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.infradead.org_mailman_listinfo_linux-2Dsnps-2Darc&d=DwICAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=yaVFU4TjGY0gVF8El1uKcisy6TPsyCl9uN7Wsis-qhY&m=05dGD6sUxxlAGDQnPi7nAbNEBEAQb-u7BnZFDdL6TwI&s=NSYlH7VAuvW7bJx6m50UgCpZmyvijw__WBWmn39sh6Y&e= 

Gentle ping :)

Best regards,
Jose Miguel Abreu
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 2/2] mtd: spi-nor: add driver for STM32 quad spi flash controller
From: Ludovic BARRE @ 2017-03-29 13:35 UTC (permalink / raw)
  To: Marek Vasut, Cyrille Pitchen
  Cc: David Woodhouse, Brian Norris, Boris Brezillon,
	Richard Weinberger, Alexandre Torgue, Rob Herring,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <28454969-0f56-7752-b087-5e02a1a20c23-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On 03/29/2017 03:09 PM, Marek Vasut wrote:
> On 03/29/2017 02:24 PM, Ludovic BARRE wrote:
>> hi Marek
> Hi!
>
>> thanks for review
>> comment below
>>
>> On 03/29/2017 12:54 PM, Marek Vasut wrote:
>>> On 03/27/2017 02:54 PM, Ludovic Barre wrote:
>>>> From: Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>
>>>>
>>>> The quadspi is a specialized communication interface targeting single,
>>>> dual or quad SPI Flash memories.
>>>>
>>>> It can operate in any of the following modes:
>>>> -indirect mode: all the operations are performed using the quadspi
>>>>    registers
>>>> -read memory-mapped mode: the external Flash memory is mapped to the
>>>>    microcontroller address space and is seen by the system as if it was
>>>>    an internal memory
>>>>
>>>> Signed-off-by: Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>
>>> Hi! I presume this is not compatible with the Cadence QSPI or any other
>>> QSPI controller, huh ?
>> it's not a cadence QSPI, it's specific for stm32 platform
> OK, got it. Didn't ST use Cadence IP somewhere too though ?
> That's probably what confused me, oh well ...
>
>>> Mostly minor nits below ...
>>>
>>>> ---
>>>>    drivers/mtd/spi-nor/Kconfig         |   7 +
>>>>    drivers/mtd/spi-nor/Makefile        |   1 +
>>>>    drivers/mtd/spi-nor/stm32-quadspi.c | 679
>>>> ++++++++++++++++++++++++++++++++++++
>>>>    3 files changed, 687 insertions(+)
>>>>    create mode 100644 drivers/mtd/spi-nor/stm32-quadspi.c
>>> [...]
>>>
>>>> +struct stm32_qspi_cmd {
>>>> +    struct {
>>>> +        u32 addr_width:8;
>>>> +        u32 dummy:8;
>>>> +        u32 data:1;
>>>> +    } conf;
>>> This could all be u8 ? Why this awful construct ?
>> yes I could replace each u32 by u8
> Yeah, please do .
>
>>>> +    u8 opcode;
>>>> +    u32 framemode;
>>>> +    u32 qspimode;
>>>> +    u32 addr;
>>>> +    size_t len;
>>>> +    void *buf;
>>>> +};
>>>> +
>>>> +static int stm32_qspi_wait_cmd(struct stm32_qspi *qspi)
>>>> +{
>>>> +    u32 cr;
>>>> +    int err = 0;
>>> Can readl_poll_timeout() or somesuch replace this function ?
>> in fact stm32_qspi_wait_cmd test if the transfer has been complete (TCF
>> flag)
>> else initialize an interrupt completion.
>> like the time may be long I prefer keep the interrupt wait, if you agree.
> I don't really mind if it fits the hardware better and I agree with you
> it does here.
>
>>>> +    if (readl_relaxed(qspi->io_base + QUADSPI_SR) & SR_TCF)
>>>> +        return 0;
>>>> +
>>>> +    reinit_completion(&qspi->cmd_completion);
>>>> +    cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
>>>> +    writel_relaxed(cr | CR_TCIE, qspi->io_base + QUADSPI_CR);
>>>> +
>>>> +    if
>>>> (!wait_for_completion_interruptible_timeout(&qspi->cmd_completion,
>>>> +                               msecs_to_jiffies(1000)))
>>>> +        err = -ETIMEDOUT;
>>>> +
>>>> +    writel_relaxed(cr, qspi->io_base + QUADSPI_CR);
>>>> +    return err;
>>>> +}
>>>> +
>>>> +static int stm32_qspi_wait_nobusy(struct stm32_qspi *qspi)
>>>> +{
>>>> +    u32 sr;
>>>> +
>>>> +    return readl_relaxed_poll_timeout(qspi->io_base + QUADSPI_SR, sr,
>>>> +                      !(sr & SR_BUSY), 10, 100000);
>>>> +}
>>>> +
>>>> +static void stm32_qspi_set_framemode(struct spi_nor *nor,
>>>> +                     struct stm32_qspi_cmd *cmd, bool read)
>>>> +{
>>>> +    u32 dmode = CCR_DMODE_1;
>>>> +
>>>> +    cmd->framemode = CCR_IMODE_1;
>>>> +
>>>> +    if (read) {
>>>> +        switch (nor->flash_read) {
>>>> +        case SPI_NOR_NORMAL:
>>>> +        case SPI_NOR_FAST:
>>>> +            dmode = CCR_DMODE_1;
>>>> +            break;
>>>> +        case SPI_NOR_DUAL:
>>>> +            dmode = CCR_DMODE_2;
>>>> +            break;
>>>> +        case SPI_NOR_QUAD:
>>>> +            dmode = CCR_DMODE_4;
>>>> +            break;
>>>> +        }
>>>> +    }
>>>> +
>>>> +    cmd->framemode |= cmd->conf.data ? dmode : 0;
>>>> +    cmd->framemode |= cmd->conf.addr_width ? CCR_ADMODE_1 : 0;
>>>> +}
>>>> +
>>>> +static void stm32_qspi_read_fifo(u8 *val, void __iomem *addr)
>>>> +{
>>>> +    *val = readb_relaxed(addr);
>>>> +}
>>>> +
>>>> +static void stm32_qspi_write_fifo(u8 *val, void __iomem *addr)
>>>> +{
>>>> +    writeb_relaxed(*val, addr);
>>>> +}
>>>> +
>>>> +static int stm32_qspi_tx_poll(struct stm32_qspi *qspi,
>>>> +                  const struct stm32_qspi_cmd *cmd)
>>>> +{
>>>> +    void (*tx_fifo)(u8 *, void __iomem *);
>>>> +    u32 len = cmd->len, sr;
>>>> +    u8 *buf = cmd->buf;
>>>> +    int ret;
>>>> +
>>>> +    if (cmd->qspimode == CCR_FMODE_INDW)
>>>> +        tx_fifo = stm32_qspi_write_fifo;
>>>> +    else
>>>> +        tx_fifo = stm32_qspi_read_fifo;
>>>> +
>>>> +    while (len--) {
>>>> +        ret = readl_relaxed_poll_timeout(qspi->io_base + QUADSPI_SR,
>>>> +                         sr, (sr & SR_FTF),
>>>> +                         10, 30000);
>>> Add macros for those ad-hoc timeouts.
>> I will add 2 defines (for stm32_qspi_wait_nobusy, stm32_qspi_tx_poll)
>> #define STM32_QSPI_FIFO_TIMEOUT_US 30000
>> #define STM32_QSPI_BUSY_TIMEOUT_US 100000
> Super
>
>>>> +        if (ret) {
>>>> +            dev_err(qspi->dev, "fifo timeout (stat:%#x)\n", sr);
>>>> +            break;
>>>> +        }
>>>> +        tx_fifo(buf++, qspi->io_base + QUADSPI_DR);
>>>> +    }
>>>> +
>>>> +    return ret;
>>>> +}
>>> [...]
>>>
>>>> +static int stm32_qspi_send(struct stm32_qspi_flash *flash,
>>>> +               const struct stm32_qspi_cmd *cmd)
>>>> +{
>>>> +    struct stm32_qspi *qspi = flash->qspi;
>>>> +    u32 ccr, dcr, cr;
>>>> +    int err;
>>>> +
>>>> +    err = stm32_qspi_wait_nobusy(qspi);
>>>> +    if (err)
>>>> +        goto abort;
>>>> +
>>>> +    dcr = readl_relaxed(qspi->io_base + QUADSPI_DCR) & ~DCR_FSIZE_MASK;
>>>> +    dcr |= DCR_FSIZE(flash->fsize);
>>>> +    writel_relaxed(dcr, qspi->io_base + QUADSPI_DCR);
>>>> +
>>>> +    cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
>>>> +    cr &= ~CR_PRESC_MASK & ~CR_FSEL;
>>>> +    cr |= CR_PRESC(flash->presc);
>>>> +    cr |= flash->cs ? CR_FSEL : 0;
>>>> +    writel_relaxed(cr, qspi->io_base + QUADSPI_CR);
>>>> +
>>>> +    if (cmd->conf.data)
>>>> +        writel_relaxed(cmd->len - 1, qspi->io_base + QUADSPI_DLR);
>>>> +
>>>> +    ccr = cmd->framemode | cmd->qspimode;
>>>> +
>>>> +    if (cmd->conf.dummy)
>>>> +        ccr |= CCR_DCYC(cmd->conf.dummy);
>>>> +
>>>> +    if (cmd->conf.addr_width)
>>>> +        ccr |= CCR_ADSIZE(cmd->conf.addr_width - 1);
>>>> +
>>>> +    ccr |= CCR_INST(cmd->opcode);
>>>> +    writel_relaxed(ccr, qspi->io_base + QUADSPI_CCR);
>>>> +
>>>> +    if (cmd->conf.addr_width && cmd->qspimode != CCR_FMODE_MM)
>>>> +        writel_relaxed(cmd->addr, qspi->io_base + QUADSPI_AR);
>>>> +
>>>> +    err = stm32_qspi_tx(qspi, cmd);
>>>> +    if (err)
>>>> +        goto abort;
>>>> +
>>>> +    if (cmd->qspimode != CCR_FMODE_MM) {
>>>> +        err = stm32_qspi_wait_cmd(qspi);
>>>> +        if (err)
>>>> +            goto abort;
>>>> +        writel_relaxed(FCR_CTCF, qspi->io_base + QUADSPI_FCR);
>>>> +    }
>>>> +
>>>> +    return err;
>>>> +
>>>> +abort:
>>>> +    writel_relaxed(readl_relaxed(qspi->io_base + QUADSPI_CR)
>>>> +               | CR_ABORT, qspi->io_base + QUADSPI_CR);
>>> Use a helper variable here too.
>> ok
>>>> +    dev_err(qspi->dev, "%s abort err:%d\n", __func__, err);
>>>> +    return err;
>>>> +}
>>> [...]
>>>
>>>> +static int stm32_qspi_flash_setup(struct stm32_qspi *qspi,
>>>> +                  struct device_node *np)
>>>> +{
>>>> +    u32 width, flash_read, presc, cs_num, max_rate = 0;
>>>> +    struct stm32_qspi_flash *flash;
>>>> +    struct mtd_info *mtd;
>>>> +    int ret;
>>>> +
>>>> +    of_property_read_u32(np, "reg", &cs_num);
>>>> +    if (cs_num >= STM32_MAX_NORCHIP)
>>>> +        return -EINVAL;
>>>> +
>>>> +    of_property_read_u32(np, "spi-max-frequency", &max_rate);
>>>> +    if (!max_rate)
>>>> +        return -EINVAL;
>>>> +
>>>> +    presc = DIV_ROUND_UP(qspi->clk_rate, max_rate) - 1;
>>>> +
>>>> +    if (of_property_read_u32(np, "spi-rx-bus-width", &width))
>>>> +        width = 1;
>>>> +
>>>> +    if (width == 4)
>>>> +        flash_read = SPI_NOR_QUAD;
>>>> +    else if (width == 2)
>>>> +        flash_read = SPI_NOR_DUAL;
>>>> +    else if (width == 1)
>>>> +        flash_read = SPI_NOR_NORMAL;
>>>> +    else
>>>> +        return -EINVAL;
>>>> +
>>>> +    flash = &qspi->flash[cs_num];
>>>> +    flash->qspi = qspi;
>>>> +    flash->cs = cs_num;
>>>> +    flash->presc = presc;
>>>> +
>>>> +    flash->nor.dev = qspi->dev;
>>>> +    spi_nor_set_flash_node(&flash->nor, np);
>>>> +    flash->nor.priv = flash;
>>>> +    mtd = &flash->nor.mtd;
>>>> +    mtd->priv = &flash->nor;
>>>> +
>>>> +    flash->nor.read = stm32_qspi_read;
>>>> +    flash->nor.write = stm32_qspi_write;
>>>> +    flash->nor.erase = stm32_qspi_erase;
>>>> +    flash->nor.read_reg = stm32_qspi_read_reg;
>>>> +    flash->nor.write_reg = stm32_qspi_write_reg;
>>>> +    flash->nor.prepare = stm32_qspi_prep;
>>>> +    flash->nor.unprepare = stm32_qspi_unprep;
>>>> +
>>>> +    writel_relaxed(LPTR_DFT_TIMEOUT, qspi->io_base + QUADSPI_LPTR);
>>>> +
>>>> +    writel_relaxed(CR_PRESC(presc) | CR_FTHRES(3) | CR_TCEN | CR_SSHIFT
>>>> +               | CR_EN, qspi->io_base + QUADSPI_CR);
>>>> +
>>>> +    /* a minimum fsize must be set to sent the command id */
>>>> +    flash->fsize = 25;
>>> I don't understand why this is needed and the comment doesn't make
>>> sense. Please fix.
>> fsize field defines the size of external memory.
> What external memory ? Unclear
oops, fsize field defined the size of "flash memory" in stm32 qspi 
controller.
Number of bytes in Flash memory = 2 ^[FSIZE+1].
To sent a nor cmd this field must be set (hardware issue),
but before "spi_nor_scan" the size of flash nor is not know.
So I set a temporary value (workaround).

After "spi_nor_scan" the fsize is overwritten by the right value
flash->fsize = __fls(mtd->size) - 1;
>> Normaly, this field is used only for memory map mode,
>> but in fact is check in indirect mode.
>> So while flash scan "spi_nor_scan":
>> -I can't let 0.
>> -I not know yet the size of flash.
>> So I fix a temporary value
>>
>> I will update my comment
> Please do, also please consider that I'm reading the comment and I
> barely have any clue about this hardware , so make sure I can understand it.
>
>>>> +    ret = spi_nor_scan(&flash->nor, NULL, flash_read);
>>>> +    if (ret) {
>>>> +        dev_err(qspi->dev, "device scan failed\n");
>>>> +        return ret;
>>>> +    }
>>>> +
>>>> +    flash->fsize = __fls(mtd->size) - 1;
>>>> +
>>>> +    writel_relaxed(DCR_CSHT(1), qspi->io_base + QUADSPI_DCR);
>>>> +
>>>> +    ret = mtd_device_register(mtd, NULL, 0);
>>>> +    if (ret) {
>>>> +        dev_err(qspi->dev, "mtd device parse failed\n");
>>>> +        return ret;
>>>> +    }
>>>> +
>>>> +    dev_dbg(qspi->dev, "read mm:%s cs:%d bus:%d\n",
>>>> +        qspi->read_mode == CCR_FMODE_MM ? "yes" : "no", cs_num, width);
>>>> +
>>>> +    return 0;
>>>> +}
>>> [...]
>>>
>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] drm/arcpgu: Get rid of "encoder-slave" property
From: Alexey Brodkin @ 2017-03-29 13:34 UTC (permalink / raw)
  To: Liviu.Dudau-5wv7dgnIgG8@public.gmane.org,
	robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-snps-arc-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Eugeniy Paltsev, daniel-/w4YWyX8dFk@public.gmane.org,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
	airlied-cv59FeDIM0c@public.gmane.org
In-Reply-To: <20170303182102.GP917-A/Nd4k6kWRHZROr8t4l/smS4ubULX0JqMm0uRHvK7Nw@public.gmane.org>

Hi Liviu, Rob,

On Fri, 2017-03-03 at 18:21 +0000, Liviu.Dudau@arm.com wrote:
> On Fri, Mar 03, 2017 at 05:48:19PM +0000, Alexey Brodkin wrote:
> > 
> > Hi Liviu,
> > 
> > On Fri, 2017-03-03 at 16:28 +0000, Liviu Dudau wrote:
> > > 
> > > On Fri, Mar 03, 2017 at 06:19:24PM +0300, Alexey Brodkin wrote:
> > > > 
> > > > 
> > > > -	/* find the encoder node and initialize it */
> > > > -	encoder_node = of_parse_phandle(drm->dev->of_node, "encoder-slave", 0);
> > > > -	if (encoder_node) {
> > > > -		ret = arcpgu_drm_hdmi_init(drm, encoder_node);
> > > > -		of_node_put(encoder_node);
> > > > +	/* There is only one output port inside each device, find it */
> > > > +	port = of_graph_get_next_endpoint(pdev->dev.of_node, NULL);
> > > > +
> > > > +	if (port) {
> > > > +		if (of_device_is_available(port))
> > > > +			encoder = of_graph_get_remote_port_parent(port);
> > > > +		of_node_put(port);
> > > > +	}
> > > 
> > > You must've been looking at some old version. Current version in -next uses
> > > of_graph_get_remote_node() to replace all those lines you have added (see Rob
> > > Herring's series to introduce of_graph_get_remote_node() function)
> > 
> > Hm, I'm not on Linus' master tree [1] and so I thought I was quite up to date :)
> > Still I made a check of linux-next and don't see any changes in
> > "drivers/gpu/drm/arm" compared to Linus' tree.
> > 
> > [1] https://urldefense.proofpoint.com/v2/url?u=http-3A__git.kernel.org_cgit_linux_kernel_git_torvalds_linux.git_commit_drivers_gpu_drm_arm-3Fid-3D
> > e4563f6ba71792c77aeccb2092cc23149b44e642&d=DwIDaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=lqdeeSSEes0GFDDl656eViXO7breS55ytWkhpk5R81I&m=SI66ngnnXy33ncb8m5H4La2
> > T1SzSEiiP7hc_XsRahEc&s=uaswjVXcjYDrUosOkO_UpTMqJMWTT-LLPrg5JE6-t-8&e= 
> > [2] https://urldefense.proofpoint.com/v2/url?u=http-3A__git.kernel.org_cgit_linux_kernel_git_next_linux-2Dnext.git_commit_drivers_gpu_drm_arm-3Fid
> > -3De4563f6ba71792c77aeccb2092cc23149b44e642&d=DwIDaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=lqdeeSSEes0GFDDl656eViXO7breS55ytWkhpk5R81I&m=SI66ngnnXy33ncb8m5H4
> > La2T1SzSEiiP7hc_XsRahEc&s=hl9Y6s3K9LwLL1M2WnL3ODax_V-ZRh8k1iTiyctIqU4&e= 
> > 
> > Could you please clarify which exact tree did you mean?
> 
> Sorry, I thought the series got pulled by one of the DRM trees, but it looks like
> I was wrong. I was carrying a private copy in my internal tree, waiting for the
> moment when it got pulled into drm-next or drm-misc-next.
> 
> Rob, do you have an update on your series introducing of_graph_get_remote_node() ?

For some reason I cannot find any relevant commits in linux-next tree even today.
Could you please point me to either any random git tree with mentioned above change or
maybe just mailing list where this patch was sent?

I'd like to implement the same fix in ARCPGU and call it a day finally.

-Alexey

^ permalink raw reply

* Re: [RFC 8/8] ARM: dts: imx6qdl-sabresd: Enable fsl,ldo-bypass
From: Leonard Crestez @ 2017-03-29 13:32 UTC (permalink / raw)
  To: Lucas Stach
  Cc: Mark Brown, Liam Girdwood, Viresh Kumar, Rafael J. Wysocki,
	Shawn Guo, Sascha Hauer, Mark Rutland,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Anson Huang, Irina Tirdea,
	linux-pm-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
	Octavian Purdila, Fabio Estevam, Robin Gong,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1490202830.29056.8.camel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

On Wed, 2017-03-22 at 18:13 +0100, Lucas Stach wrote:
> Am Mittwoch, den 22.03.2017, 18:53 +0200 schrieb Leonard Crestez:

> > This enables LDO bypass by default on the imx6qdl-sabresd boards. New
> > dts files with -ldo suffix are added for users who want to run with LDOs
> > enabled on these boards anyway.

> Given that using LDO bypass affects the device lifetime negatively (see
> AN4724), I think the default should still be to use LDO enabled mode and
> have new DTs for people that desire to shorten the lifetime of the SoC
> for a minimal drop in power consumption.

This was based on what the Freescale BSP does, I don't particularly
object to upstream having a different default. It would be nice for
testing if this ldo-bypass path was enabled by default for some boards
but it's not a very good reason.

I will switch the default.

-- 
Regards,
Leonard
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v3 4/7] arm: dts: r7s72100: Add pin controller node
From: jacopo @ 2017-03-29 13:26 UTC (permalink / raw)
  To: Chris Brandt
  Cc: Jacopo Mondi, geert+renesas@glider.be,
	laurent.pinchart@ideasonboard.com, linus.walleij@linaro.org,
	robh+dt@kernel.org, mark.rutland@arm.com, linux@armlinux.org.uk,
	linux-renesas-soc@vger.kernel.org, linux-gpio@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <SG2PR06MB11651E209BC332719059CC3F8A330@SG2PR06MB1165.apcprd06.prod.outlook.com>

Hi Chris,

On Mon, Mar 27, 2017 at 05:12:04PM +0000, Chris Brandt wrote:
> Hi Jacopo,
> 
> 
> On Friday, March 24, 2017, Jacopo Mondi
> > +	pinctrl: pinctrl@fcfe3000 {
> > +		compatible = "renesas,r7s72100-ports";
> > +
> > +		#pinctrl-cells = <1>;
> > +
> > +		reg = <0xfcfe3000 0x42C0>;
> 
> Out of curiosity, why did you change this from 0x4248 to 0x42C0?
> 
> In your update for renesas,rza1-pinctrl.txt, for the 'Example', you changed it from 0x4248 to 0x4230 (which Geert pointed out makes more sense), but then in the actual DT file you changed it to 0x42C0. Typo?
>

Yes, type.
Or inability to perform basic mathematical operations.
Not sure yet.

Thanks for spotting this :)
   j


> 
> Chris
> 

^ permalink raw reply

* [PATCH v1 8/8] ARM: configs: stm32: DCMI + OV2640 camera support
From: Hugues Fruchet @ 2017-03-29 13:25 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Maxime Coquelin, Alexandre Torgue,
	Mauro Carvalho Chehab, Hans Verkuil
  Cc: devicetree, linux-arm-kernel, linux-kernel, linux-media,
	Benjamin Gaignard, Yannick Fertre, Hugues Fruchet
In-Reply-To: <1490793926-6477-1-git-send-email-hugues.fruchet@st.com>

Enable DCMI camera interface and OV2640 camera sensor drivers.

Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
---
 arch/arm/configs/stm32_defconfig | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/configs/stm32_defconfig b/arch/arm/configs/stm32_defconfig
index 84adc88..3f2e4ce 100644
--- a/arch/arm/configs/stm32_defconfig
+++ b/arch/arm/configs/stm32_defconfig
@@ -53,6 +53,13 @@ CONFIG_GPIO_STMPE=y
 CONFIG_MFD_STMPE=y
 CONFIG_REGULATOR_FIXED_VOLTAGE=y
 # CONFIG_USB_SUPPORT is not set
+CONFIG_VIDEO_V4L2=y
+CONFIG_MEDIA_SUBDRV_AUTOSELECT=n
+CONFIG_V4L_PLATFORM_DRIVERS=y
+CONFIG_MEDIA_SUPPORT=y
+CONFIG_MEDIA_CAMERA_SUPPORT=y
+CONFIG_VIDEO_STM32_DCMI=y
+CONFIG_VIDEO_OV2640=y
 CONFIG_NEW_LEDS=y
 CONFIG_LEDS_CLASS=y
 CONFIG_LEDS_GPIO=y
-- 
1.9.1

^ permalink raw reply related

* [PATCH v1 7/8] ARM: configs: stm32: stmpe 1600 GPIO expandor
From: Hugues Fruchet @ 2017-03-29 13:25 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Maxime Coquelin, Alexandre Torgue,
	Mauro Carvalho Chehab, Hans Verkuil
  Cc: devicetree, linux-arm-kernel, linux-kernel, linux-media,
	Benjamin Gaignard, Yannick Fertre, Hugues Fruchet
In-Reply-To: <1490793926-6477-1-git-send-email-hugues.fruchet@st.com>

Enable STMPE1600 GPIO expandor.

Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
---
 arch/arm/configs/stm32_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/configs/stm32_defconfig b/arch/arm/configs/stm32_defconfig
index a9d8e3c..84adc88 100644
--- a/arch/arm/configs/stm32_defconfig
+++ b/arch/arm/configs/stm32_defconfig
@@ -49,6 +49,8 @@ CONFIG_SERIAL_STM32_CONSOLE=y
 # CONFIG_HW_RANDOM is not set
 # CONFIG_HWMON is not set
 CONFIG_REGULATOR=y
+CONFIG_GPIO_STMPE=y
+CONFIG_MFD_STMPE=y
 CONFIG_REGULATOR_FIXED_VOLTAGE=y
 # CONFIG_USB_SUPPORT is not set
 CONFIG_NEW_LEDS=y
-- 
1.9.1

^ permalink raw reply related

* [PATCH v1 6/8] ARM: dts: stm32: Enable ov2640 camera support of STM32F429-EVAL board
From: Hugues Fruchet @ 2017-03-29 13:25 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Maxime Coquelin, Alexandre Torgue,
	Mauro Carvalho Chehab, Hans Verkuil
  Cc: devicetree, linux-arm-kernel, linux-kernel, linux-media,
	Benjamin Gaignard, Yannick Fertre, Hugues Fruchet
In-Reply-To: <1490793926-6477-1-git-send-email-hugues.fruchet@st.com>

Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
---
 arch/arm/boot/dts/stm32429i-eval.dts | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm/boot/dts/stm32429i-eval.dts b/arch/arm/boot/dts/stm32429i-eval.dts
index 7ffcf07..b7d127c 100644
--- a/arch/arm/boot/dts/stm32429i-eval.dts
+++ b/arch/arm/boot/dts/stm32429i-eval.dts
@@ -48,6 +48,7 @@
 /dts-v1/;
 #include "stm32f429.dtsi"
 #include <dt-bindings/input/input.h>
+#include <dt-bindings/gpio/gpio.h>
 
 / {
 	model = "STMicroelectronics STM32429i-EVAL board";
@@ -66,6 +67,14 @@
 		serial0 = &usart1;
 	};
 
+	clocks {
+		clk_ext_camera: clk-ext-camera {
+			#clock-cells = <0>;
+			compatible = "fixed-clock";
+			clock-frequency = <24000000>;
+		};
+	};
+
 	soc {
 		dma-ranges = <0xc0000000 0x0 0x10000000>;
 	};
@@ -146,6 +155,11 @@
 
 	port {
 		dcmi_0: endpoint@0 {
+			remote-endpoint = <&ov2640_0>;
+			bus-width = <8>;
+			hsync-active = <0>;
+			vsync-active = <0>;
+			pclk-sample = <1>;
 		};
 	};
 };
@@ -155,6 +169,22 @@
 	pinctrl-names = "default";
 	status = "okay";
 
+	ov2640: camera@30 {
+		compatible = "ovti,ov2640";
+		reg = <0x30>;
+		resetb-gpios = <&stmpegpio 2 GPIO_ACTIVE_HIGH>;
+		pwdn-gpios = <&stmpegpio 0 GPIO_ACTIVE_LOW>;
+		clocks = <&clk_ext_camera>;
+		clock-names = "xvclk";
+		status = "okay";
+
+		port {
+			ov2640_0: endpoint {
+				remote-endpoint = <&dcmi_0>;
+			};
+		};
+	};
+
 	stmpe1600: stmpe1600@42 {
 		compatible = "st,stmpe1600";
 		reg = <0x42>;
-- 
1.9.1

^ permalink raw reply related

* [PATCH v1 5/8] ARM: dts: stm32: Enable stmpe1600 gpio expandor of STM32F429-EVAL board
From: Hugues Fruchet @ 2017-03-29 13:25 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Maxime Coquelin, Alexandre Torgue,
	Mauro Carvalho Chehab, Hans Verkuil
  Cc: devicetree, linux-arm-kernel, linux-kernel, linux-media,
	Benjamin Gaignard, Yannick Fertre, Hugues Fruchet
In-Reply-To: <1490793926-6477-1-git-send-email-hugues.fruchet@st.com>

Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
---
 arch/arm/boot/dts/stm32429i-eval.dts | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm/boot/dts/stm32429i-eval.dts b/arch/arm/boot/dts/stm32429i-eval.dts
index 87733d3..7ffcf07 100644
--- a/arch/arm/boot/dts/stm32429i-eval.dts
+++ b/arch/arm/boot/dts/stm32429i-eval.dts
@@ -154,6 +154,23 @@
 	pinctrl-0 = <&i2c1_pins>;
 	pinctrl-names = "default";
 	status = "okay";
+
+	stmpe1600: stmpe1600@42 {
+		compatible = "st,stmpe1600";
+		reg = <0x42>;
+		irq-gpio = <&gpioi 8 0>;
+		irq-trigger = <3>;
+		interrupts = <8 3>;
+		interrupt-parent = <&exti>;
+		interrupt-controller;
+		wakeup-source;
+
+		stmpegpio: stmpe_gpio {
+			compatible = "st,stmpe-gpio";
+			gpio-controller;
+			#gpio-cells = <2>;
+		};
+	};
 };
 
 &mac {
-- 
1.9.1

^ permalink raw reply related

* [PATCH v1 4/8] ARM: dts: stm32: Enable DCMI camera interface on STM32F429-EVAL board
From: Hugues Fruchet @ 2017-03-29 13:25 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Maxime Coquelin, Alexandre Torgue,
	Mauro Carvalho Chehab, Hans Verkuil
  Cc: devicetree, linux-arm-kernel, linux-kernel, linux-media,
	Benjamin Gaignard, Yannick Fertre, Hugues Fruchet
In-Reply-To: <1490793926-6477-1-git-send-email-hugues.fruchet@st.com>

Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
---
 arch/arm/boot/dts/stm32429i-eval.dts | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm/boot/dts/stm32429i-eval.dts b/arch/arm/boot/dts/stm32429i-eval.dts
index 3c99466..87733d3 100644
--- a/arch/arm/boot/dts/stm32429i-eval.dts
+++ b/arch/arm/boot/dts/stm32429i-eval.dts
@@ -141,6 +141,15 @@
 	clock-frequency = <25000000>;
 };
 
+&dcmi {
+	status = "okay";
+
+	port {
+		dcmi_0: endpoint@0 {
+		};
+	};
+};
+
 &i2c1 {
 	pinctrl-0 = <&i2c1_pins>;
 	pinctrl-names = "default";
-- 
1.9.1

^ permalink raw reply related

* [PATCH v1 3/8] ARM: dts: stm32: Enable DCMI support on STM32F429 MCU
From: Hugues Fruchet @ 2017-03-29 13:25 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Maxime Coquelin, Alexandre Torgue,
	Mauro Carvalho Chehab, Hans Verkuil
  Cc: devicetree, linux-arm-kernel, linux-kernel, linux-media,
	Benjamin Gaignard, Yannick Fertre, Hugues Fruchet
In-Reply-To: <1490793926-6477-1-git-send-email-hugues.fruchet@st.com>

Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
---
 arch/arm/boot/dts/stm32f429.dtsi | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
index ee0da97..e1ff978 100644
--- a/arch/arm/boot/dts/stm32f429.dtsi
+++ b/arch/arm/boot/dts/stm32f429.dtsi
@@ -736,6 +736,29 @@
 					slew-rate = <3>;
 				};
 			};
+
+			dcmi_pins: dcmi_pins@0 {
+				pins {
+					pinmux = <STM32F429_PA4_FUNC_DCMI_HSYNC>,
+						 <STM32F429_PB7_FUNC_DCMI_VSYNC>,
+						 <STM32F429_PA6_FUNC_DCMI_PIXCLK>,
+						 <STM32F429_PC6_FUNC_DCMI_D0>,
+						 <STM32F429_PC7_FUNC_DCMI_D1>,
+						 <STM32F429_PC8_FUNC_DCMI_D2>,
+						 <STM32F429_PC9_FUNC_DCMI_D3>,
+						 <STM32F429_PC11_FUNC_DCMI_D4>,
+						 <STM32F429_PD3_FUNC_DCMI_D5>,
+						 <STM32F429_PB8_FUNC_DCMI_D6>,
+						 <STM32F429_PE6_FUNC_DCMI_D7>,
+						 <STM32F429_PC10_FUNC_DCMI_D8>,
+						 <STM32F429_PC12_FUNC_DCMI_D9>,
+						 <STM32F429_PD6_FUNC_DCMI_D10>,
+						 <STM32F429_PD2_FUNC_DCMI_D11>;
+					bias-disable;
+					drive-push-pull;
+					slew-rate = <3>;
+				};
+			};
 		};
 
 		rcc: rcc@40023810 {
@@ -805,6 +828,20 @@
 			status = "disabled";
 		};
 
+		dcmi: dcmi@50050000 {
+			compatible = "st,stm32-dcmi";
+			reg = <0x50050000 0x400>;
+			interrupts = <78>;
+			resets = <&rcc STM32F4_AHB2_RESET(DCMI)>;
+			clocks = <&rcc 0 STM32F4_AHB2_CLOCK(DCMI)>;
+			clock-names = "mclk";
+			pinctrl-names = "default";
+			pinctrl-0 = <&dcmi_pins>;
+			dmas = <&dma2 1 1 0x414 0x3>;
+			dma-names = "tx";
+			status = "disabled";
+		};
+
 		rng: rng@50060800 {
 			compatible = "st,stm32-rng";
 			reg = <0x50060800 0x400>;
-- 
1.9.1

^ permalink raw reply related

* [PATCH v1 2/8] [media] stm32-dcmi: STM32 DCMI camera interface driver
From: Hugues Fruchet @ 2017-03-29 13:25 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Maxime Coquelin, Alexandre Torgue,
	Mauro Carvalho Chehab, Hans Verkuil
  Cc: devicetree, linux-arm-kernel, linux-kernel, linux-media,
	Benjamin Gaignard, Yannick Fertre, Hugues Fruchet
In-Reply-To: <1490793926-6477-1-git-send-email-hugues.fruchet@st.com>

This V4L2 subdev driver enables Digital Camera Memory Interface (DCMI)
of STMicroelectronics STM32 SoC series.

Signed-off-by: Yannick Fertre <yannick.fertre@st.com>
Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
---
 drivers/media/platform/Kconfig            |   12 +
 drivers/media/platform/Makefile           |    2 +
 drivers/media/platform/stm32/Makefile     |    1 +
 drivers/media/platform/stm32/stm32-dcmi.c | 1417 +++++++++++++++++++++++++++++
 4 files changed, 1432 insertions(+)
 create mode 100644 drivers/media/platform/stm32/Makefile
 create mode 100644 drivers/media/platform/stm32/stm32-dcmi.c

diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index ab0bb48..3421965 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -114,6 +114,18 @@ config VIDEO_S3C_CAMIF
 	  To compile this driver as a module, choose M here: the module
 	  will be called s3c-camif.
 
+config VIDEO_STM32_DCMI
+	tristate "Digital Camera Memory Interface (DCMI) support"
+	depends on VIDEO_V4L2 && OF && HAS_DMA
+	depends on ARCH_STM32 || COMPILE_TEST
+	select VIDEOBUF2_DMA_CONTIG
+	---help---
+	  This module makes the STM32 Digital Camera Memory Interface (DCMI)
+	  available as a v4l2 device.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called stm32-dcmi.
+
 source "drivers/media/platform/soc_camera/Kconfig"
 source "drivers/media/platform/exynos4-is/Kconfig"
 source "drivers/media/platform/am437x/Kconfig"
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 8959f6e..d747715 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -64,6 +64,8 @@ obj-$(CONFIG_VIDEO_RCAR_VIN)		+= rcar-vin/
 
 obj-$(CONFIG_VIDEO_ATMEL_ISC)		+= atmel/
 
+obj-$(CONFIG_VIDEO_STM32_DCMI)		+= stm32/
+
 ccflags-y += -I$(srctree)/drivers/media/i2c
 
 obj-$(CONFIG_VIDEO_MEDIATEK_VPU)	+= mtk-vpu/
diff --git a/drivers/media/platform/stm32/Makefile b/drivers/media/platform/stm32/Makefile
new file mode 100644
index 0000000..9b606a7
--- /dev/null
+++ b/drivers/media/platform/stm32/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_VIDEO_STM32_DCMI) += stm32-dcmi.o
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c b/drivers/media/platform/stm32/stm32-dcmi.c
new file mode 100644
index 0000000..2f0286b
--- /dev/null
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -0,0 +1,1417 @@
+/*
+ * Driver for STM32 Digital Camera Memory Interface
+ *
+ * Copyright (C) STMicroelectronics SA 2017
+ * Authors: Yannick Fertre <yannick.fertre@st.com>
+ *          Hugues Fruchet <hugues.fruchet@st.com>
+ *          for STMicroelectronics.
+ * License terms:  GNU General Public License (GPL), version 2
+ *
+ * This driver is based on atmel_isi.c
+ *
+ */
+
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/dmaengine.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+#include <linux/videodev2.h>
+
+#include <media/v4l2-ctrls.h>
+#include <media/v4l2-dev.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-event.h>
+#include <media/v4l2-image-sizes.h>
+#include <media/v4l2-ioctl.h>
+#include <media/v4l2-of.h>
+#include <media/videobuf2-dma-contig.h>
+
+#define DRV_NAME "stm32-dcmi"
+
+/* Registers offset for DCMI */
+#define DCMI_CR		0x00 /* Control Register */
+#define DCMI_SR		0x04 /* Status Register */
+#define DCMI_RIS	0x08 /* Raw Interrupt Status register */
+#define DCMI_IER	0x0C /* Interrupt Enable Register */
+#define DCMI_MIS	0x10 /* Masked Interrupt Status register */
+#define DCMI_ICR	0x14 /* Interrupt Clear Register */
+#define DCMI_ESCR	0x18 /* Embedded Synchronization Code Register */
+#define DCMI_ESUR	0x1C /* Embedded Synchronization Unmask Register */
+#define DCMI_CWSTRT	0x20 /* Crop Window STaRT */
+#define DCMI_CWSIZE	0x24 /* Crop Window SIZE */
+#define DCMI_DR		0x28 /* Data Register */
+#define DCMI_IDR	0x2c /* IDentifier Register */
+
+/* Bits definition for control register (DCMI_CR) */
+#define CR_CAPTURE	BIT(0)
+#define CR_CM		BIT(1)
+#define CR_CROP		BIT(2)
+#define CR_JPEG		BIT(3)
+#define CR_ESS		BIT(4)
+#define CR_PCKPOL	BIT(5)
+#define CR_HSPOL	BIT(6)
+#define CR_VSPOL	BIT(7)
+#define CR_FCRC_0	BIT(8)
+#define CR_FCRC_1	BIT(9)
+#define CR_EDM_0	BIT(10)
+#define CR_EDM_1	BIT(11)
+#define CR_ENABLE	BIT(14)
+
+/* Bits definition for status register (DCMI_SR) */
+#define SR_HSYNC	BIT(0)
+#define SR_VSYNC	BIT(1)
+#define SR_FNE		BIT(2)
+
+/*
+ * Bits definition for interrupt registers
+ * (DCMI_RIS, DCMI_IER, DCMI_MIS, DCMI_ICR)
+ */
+#define IT_FRAME	BIT(0)
+#define IT_OVR		BIT(1)
+#define IT_ERR		BIT(2)
+#define IT_VSYNC	BIT(3)
+#define IT_LINE		BIT(4)
+
+enum state {
+	STOPPED = 0,
+	RUNNING,
+	STOPPING,
+};
+
+#define MAX_BUS_WIDTH		14
+#define MAX_SUPPORT_WIDTH	2048
+#define MAX_SUPPORT_HEIGHT	2048
+#define MIN_SUPPORT_WIDTH	16
+#define MIN_SUPPORT_HEIGHT	16
+#define TIMEOUT_MS		1000
+
+struct dcmi_graph_entity {
+	struct device_node *node;
+
+	struct v4l2_async_subdev asd;
+	struct v4l2_subdev *subdev;
+};
+
+struct dcmi_format {
+	u32	fourcc;
+	u32	mbus_code;
+	u8	bpp;
+};
+
+struct dcmi_buf {
+	struct vb2_v4l2_buffer	vb;
+	bool			prepared;
+	dma_addr_t		paddr;
+	size_t			size;
+	struct list_head	list;
+};
+
+struct stm32_dcmi {
+	spinlock_t			irqlock;
+	struct device			*dev;
+	void __iomem			*regs;
+	struct resource			*res;
+	struct reset_control		*rstc;
+	int				sequence;
+	struct list_head		buffers;
+	struct dcmi_buf			*active;
+
+	struct v4l2_device		v4l2_dev;
+	struct video_device		*vdev;
+	struct v4l2_async_notifier	notifier;
+	struct dcmi_graph_entity	entity;
+	struct v4l2_format		fmt;
+
+	const struct dcmi_format	**user_formats;
+	unsigned int			num_user_formats;
+	const struct dcmi_format	*current_fmt;
+
+	struct mutex			lock;
+	struct vb2_queue		queue;
+
+	struct v4l2_of_bus_parallel	bus;
+	struct completion		complete;
+	struct clk			*mclk;
+	enum state			state;
+	struct dma_chan			*dma_chan;
+	dma_cookie_t			dma_cookie;
+	u32				misr;
+	int				errors_count;
+	int				buffers_count;
+};
+
+static inline struct stm32_dcmi *notifier_to_dcmi(struct v4l2_async_notifier *n)
+{
+	return container_of(n, struct stm32_dcmi, notifier);
+}
+
+static inline u32 reg_read(void __iomem *base, u32 reg)
+{
+	return readl_relaxed(base + reg);
+}
+
+static inline void reg_write(void __iomem *base, u32 reg, u32 val)
+{
+	writel_relaxed(val, base + reg);
+}
+
+static inline void reg_set(void __iomem *base, u32 reg, u32 mask)
+{
+	reg_write(base, reg, reg_read(base, reg) | mask);
+}
+
+static inline void reg_clear(void __iomem *base, u32 reg, u32 mask)
+{
+	reg_write(base, reg, reg_read(base, reg) & ~mask);
+}
+
+static int dcmi_start_capture(struct stm32_dcmi *dcmi);
+
+static void dcmi_dma_callback(void *param)
+{
+	struct stm32_dcmi *dcmi = (struct stm32_dcmi *)param;
+	struct dma_chan *chan = dcmi->dma_chan;
+	struct dma_tx_state state;
+	enum dma_status status;
+
+	spin_lock(&dcmi->irqlock);
+
+	/* Check DMA status */
+	status = dmaengine_tx_status(chan, dcmi->dma_cookie, &state);
+
+	switch (status) {
+	case DMA_IN_PROGRESS:
+		dev_dbg(dcmi->dev, "%s: Received DMA_IN_PROGRESS\n", __func__);
+		break;
+	case DMA_PAUSED:
+		dev_err(dcmi->dev, "%s: Received DMA_PAUSED\n", __func__);
+		break;
+	case DMA_ERROR:
+		dev_err(dcmi->dev, "%s: Received DMA_ERROR\n", __func__);
+		break;
+	case DMA_COMPLETE:
+		dev_dbg(dcmi->dev, "%s: Received DMA_COMPLETE\n", __func__);
+
+		if (dcmi->active) {
+			struct dcmi_buf *buf = dcmi->active;
+			struct vb2_v4l2_buffer *vbuf = &dcmi->active->vb;
+
+			vbuf->sequence = dcmi->sequence++;
+			vbuf->field = V4L2_FIELD_NONE;
+			vbuf->vb2_buf.timestamp = ktime_get_ns();
+			vb2_set_plane_payload(&vbuf->vb2_buf, 0, buf->size);
+			vb2_buffer_done(&vbuf->vb2_buf, VB2_BUF_STATE_DONE);
+			dev_dbg(dcmi->dev, "buffer[%d] done seq=%d\n",
+				vbuf->vb2_buf.index, vbuf->sequence);
+
+			dcmi->buffers_count++;
+			dcmi->active = NULL;
+		}
+
+		/* Restart a new DMA transfer with next buffer */
+		if (dcmi->state == RUNNING) {
+			int ret;
+
+			if (list_empty(&dcmi->buffers)) {
+				dev_err(dcmi->dev, "%s: No more buffer queued, cannot capture buffer",
+					__func__);
+				dcmi->errors_count++;
+				dcmi->active = NULL;
+
+				spin_unlock(&dcmi->irqlock);
+				return;
+			}
+
+			dcmi->active = list_entry(dcmi->buffers.next,
+						  struct dcmi_buf, list);
+
+			list_del_init(&dcmi->active->list);
+
+			ret = dcmi_start_capture(dcmi);
+			if (ret) {
+				dev_err(dcmi->dev, "%s: Cannot restart capture on DMA complete",
+					__func__);
+
+				spin_unlock(&dcmi->irqlock);
+				return;
+			}
+
+			/* Enable capture */
+			reg_set(dcmi->regs, DCMI_CR, CR_CAPTURE);
+		}
+
+		break;
+	default:
+		dev_err(dcmi->dev, "%s: Received unknown status\n", __func__);
+		break;
+	}
+
+	spin_unlock(&dcmi->irqlock);
+}
+
+static int dcmi_start_dma(struct stm32_dcmi *dcmi,
+			  struct dcmi_buf *buf)
+{
+	struct dma_async_tx_descriptor *desc = NULL;
+	struct dma_slave_config config;
+	int ret;
+
+	memset(&config, 0, sizeof(config));
+
+	config.src_addr = (dma_addr_t)dcmi->res->start + DCMI_DR;
+	config.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+	config.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
+	config.dst_maxburst = 4;
+
+	/* Configure DMA channel */
+	ret = dmaengine_slave_config(dcmi->dma_chan, &config);
+	if (ret < 0) {
+		dev_err(dcmi->dev, "%s: DMA channel config failed (%d)\n",
+			__func__, ret);
+		return ret;
+	}
+
+	/* Prepare a DMA transaction */
+	desc = dmaengine_prep_slave_single(dcmi->dma_chan, buf->paddr,
+					   buf->size,
+					   DMA_DEV_TO_MEM, DMA_PREP_INTERRUPT);
+	if (!desc) {
+		dev_err(dcmi->dev, "%s: DMA dmaengine_prep_slave_single failed for buffer size %zu\n",
+			__func__, buf->size);
+		return -EINVAL;
+	}
+
+	/* Set completion callback routine for notification */
+	desc->callback = dcmi_dma_callback;
+	desc->callback_param = dcmi;
+
+	/* Push current DMA transaction in the pending queue */
+	dcmi->dma_cookie = dmaengine_submit(desc);
+
+	dma_async_issue_pending(dcmi->dma_chan);
+
+	return 0;
+}
+
+static int dcmi_start_capture(struct stm32_dcmi *dcmi)
+{
+	int ret;
+	struct dcmi_buf *buf = dcmi->active;
+
+	if (!buf)
+		return -EINVAL;
+
+	ret = dcmi_start_dma(dcmi, buf);
+	if (ret) {
+		vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
+		dcmi->errors_count++;
+		dcmi->active = NULL;
+
+		return ret;
+	}
+
+	/* Enable capture */
+	reg_set(dcmi->regs, DCMI_CR, CR_CAPTURE);
+
+	return 0;
+}
+
+static irqreturn_t dcmi_irq_thread(int irq, void *arg)
+{
+	struct stm32_dcmi *dcmi = arg;
+	int ret;
+
+	spin_lock(&dcmi->irqlock);
+
+	/* Stop capture is required */
+	if (dcmi->state == STOPPING) {
+		reg_clear(dcmi->regs, DCMI_IER, IT_FRAME | IT_OVR | IT_ERR);
+
+		dcmi->state = STOPPED;
+
+		complete(&dcmi->complete);
+
+		spin_unlock(&dcmi->irqlock);
+		return IRQ_HANDLED;
+	}
+
+	if ((dcmi->misr & IT_OVR) || (dcmi->misr & IT_ERR)) {
+		/*
+		 * An overflow or an error has been detected,
+		 * stop current DMA transfert & restart it
+		 */
+		dev_warn(dcmi->dev, "%s: Overflow or error detected\n",
+			 __func__);
+
+		dcmi->errors_count++;
+		dmaengine_terminate_all(dcmi->dma_chan);
+
+		reg_set(dcmi->regs, DCMI_ICR, IT_FRAME | IT_OVR | IT_ERR);
+
+		dev_dbg(dcmi->dev, "Restarting capture after DCMI error\n");
+
+		ret = dcmi_start_capture(dcmi);
+		if (ret) {
+			dev_err(dcmi->dev, "%s: Cannot restart capture on overflow or error\n",
+				__func__);
+
+			spin_unlock(&dcmi->irqlock);
+			return IRQ_HANDLED;
+		}
+	}
+
+	spin_unlock(&dcmi->irqlock);
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t dcmi_irq_callback(int irq, void *arg)
+{
+	struct stm32_dcmi *dcmi = arg;
+
+	spin_lock(&dcmi->irqlock);
+
+	dcmi->misr = reg_read(dcmi->regs, DCMI_MIS);
+
+	/* Clear interrupt */
+	reg_set(dcmi->regs, DCMI_ICR, IT_FRAME | IT_OVR | IT_ERR);
+
+	spin_unlock(&dcmi->irqlock);
+
+	return IRQ_WAKE_THREAD;
+}
+
+static int dcmi_queue_setup(struct vb2_queue *vq,
+			    unsigned int *nbuffers,
+			    unsigned int *nplanes,
+			    unsigned int sizes[],
+			    struct device *alloc_devs[])
+{
+	struct stm32_dcmi *dcmi = vb2_get_drv_priv(vq);
+	unsigned long size;
+
+	size = dcmi->fmt.fmt.pix.sizeimage;
+
+	/* Make sure the image size is large enough */
+	if (*nplanes)
+		return sizes[0] < size ? -EINVAL : 0;
+
+	*nplanes = 1;
+	sizes[0] = size;
+
+	dcmi->active = NULL;
+
+	dev_dbg(dcmi->dev, "Setup queue, count=%d, size=%ld\n",
+		*nbuffers, size);
+
+	return 0;
+}
+
+static int dcmi_buf_init(struct vb2_buffer *vb)
+{
+	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
+	struct dcmi_buf *buf = container_of(vbuf, struct dcmi_buf, vb);
+
+	INIT_LIST_HEAD(&buf->list);
+
+	return 0;
+}
+
+static int dcmi_buf_prepare(struct vb2_buffer *vb)
+{
+	struct stm32_dcmi *dcmi =  vb2_get_drv_priv(vb->vb2_queue);
+	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
+	struct dcmi_buf *buf = container_of(vbuf, struct dcmi_buf, vb);
+	unsigned long size;
+
+	size = dcmi->fmt.fmt.pix.sizeimage;
+
+	if (vb2_plane_size(vb, 0) < size) {
+		dev_err(dcmi->dev, "%s data will not fit into plane (%lu < %lu)\n",
+			__func__, vb2_plane_size(vb, 0), size);
+		return -EINVAL;
+	}
+
+	vb2_set_plane_payload(vb, 0, size);
+
+	if (!buf->prepared) {
+		/* Get memory addresses */
+		buf->paddr =
+			vb2_dma_contig_plane_dma_addr(&buf->vb.vb2_buf, 0);
+		buf->size = vb2_plane_size(&buf->vb.vb2_buf, 0);
+		buf->prepared = true;
+
+		vb2_set_plane_payload(&buf->vb.vb2_buf, 0, buf->size);
+
+		dev_dbg(dcmi->dev, "buffer[%d] phy=0x%pad size=%zu\n",
+			vb->index, &buf->paddr, buf->size);
+	}
+
+	return 0;
+}
+
+static void dcmi_buf_queue(struct vb2_buffer *vb)
+{
+	struct stm32_dcmi *dcmi =  vb2_get_drv_priv(vb->vb2_queue);
+	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
+	struct dcmi_buf *buf = container_of(vbuf, struct dcmi_buf, vb);
+	unsigned long flags = 0;
+
+	spin_lock_irqsave(&dcmi->irqlock, flags);
+
+	if ((dcmi->state == RUNNING) && (!dcmi->active)) {
+		int ret;
+
+		dcmi->active = buf;
+
+		dev_dbg(dcmi->dev, "Starting capture on buffer[%d] queued\n",
+			buf->vb.vb2_buf.index);
+
+		ret = dcmi_start_capture(dcmi);
+		if (ret) {
+			dev_err(dcmi->dev, "%s: Cannot restart capture on overflow or error\n",
+				__func__);
+
+			spin_unlock_irqrestore(&dcmi->irqlock, flags);
+			return;
+		}
+	} else {
+		/* Enqueue to video buffers list */
+		list_add_tail(&buf->list, &dcmi->buffers);
+	}
+
+	spin_unlock_irqrestore(&dcmi->irqlock, flags);
+}
+
+static int dcmi_start_streaming(struct vb2_queue *vq, unsigned int count)
+{
+	struct stm32_dcmi *dcmi = vb2_get_drv_priv(vq);
+	struct dcmi_buf *buf, *node;
+	u32 val;
+	int ret;
+
+	/* Enable stream on the sub device */
+	ret = v4l2_subdev_call(dcmi->entity.subdev, video, s_stream, 1);
+	if (ret && ret != -ENOIOCTLCMD) {
+		dev_err(dcmi->dev, "%s: Failed to start streaming, subdev streamon error",
+			__func__);
+		goto err_release_buffers;
+	}
+
+	if (clk_enable(dcmi->mclk)) {
+		dev_err(dcmi->dev, "%s: Failed to start streaming, cannot enable clock",
+			__func__);
+		goto err_subdev_streamoff;
+	}
+
+	spin_lock_irq(&dcmi->irqlock);
+
+	val = reg_read(dcmi->regs, DCMI_CR);
+
+	val &= ~(CR_PCKPOL | CR_HSPOL | CR_VSPOL |
+		 CR_EDM_0 | CR_EDM_1 | CR_FCRC_0 |
+		 CR_FCRC_1 | CR_JPEG | CR_ESS);
+
+	/* Set bus width */
+	switch (dcmi->bus.bus_width) {
+	case 14:
+		val &= CR_EDM_0 + CR_EDM_1;
+		break;
+	case 12:
+		val &= CR_EDM_1;
+		break;
+	case 10:
+		val &= CR_EDM_0;
+		break;
+	default:
+		/* Set bus width to 8 bits by default */
+		break;
+	}
+
+	/* Set vertical synchronization polarity */
+	if (dcmi->bus.flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH)
+		val |= CR_VSPOL;
+
+	/* Set horizontal synchronization polarity */
+	if (dcmi->bus.flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
+		val |= CR_HSPOL;
+
+	/* Set pixel clock polarity */
+	if (dcmi->bus.flags & V4L2_MBUS_PCLK_SAMPLE_RISING)
+		val |= CR_PCKPOL;
+
+	reg_write(dcmi->regs, DCMI_CR, val);
+
+	/* Enable dcmi */
+	reg_set(dcmi->regs, DCMI_CR, CR_ENABLE);
+
+	dcmi->state = RUNNING;
+
+	dcmi->sequence = 0;
+	dcmi->errors_count = 0;
+	dcmi->buffers_count = 0;
+	dcmi->active = NULL;
+
+	/*
+	 * Start transfer if at least one buffer has been queued,
+	 * otherwise transfer is defered at buffer queueing
+	 */
+	if (list_empty(&dcmi->buffers)) {
+		dev_dbg(dcmi->dev, "Start streaming is defered to next buffer queueing\n");
+		spin_unlock_irq(&dcmi->irqlock);
+		return 0;
+	}
+
+	dcmi->active = list_entry(dcmi->buffers.next, struct dcmi_buf, list);
+	list_del_init(&dcmi->active->list);
+
+	/* Enable interruptions */
+	reg_set(dcmi->regs, DCMI_IER, IT_FRAME | IT_OVR | IT_ERR);
+
+	dev_dbg(dcmi->dev, "Start streaming, starting capture\n");
+
+	ret = dcmi_start_capture(dcmi);
+	if (ret) {
+		dev_err(dcmi->dev, "%s: Start streaming failed, cannot start capture",
+			__func__);
+
+		spin_unlock_irq(&dcmi->irqlock);
+		goto err_subdev_streamoff;
+	}
+
+	spin_unlock_irq(&dcmi->irqlock);
+
+	return 0;
+
+err_subdev_streamoff:
+	v4l2_subdev_call(dcmi->entity.subdev, video, s_stream, 0);
+
+err_release_buffers:
+	spin_lock_irq(&dcmi->irqlock);
+	dcmi->active = NULL;
+	/*
+	 * return all buffers to vb2 in QUEUED state.
+	 * This will give ownership back to userspace
+	 */
+	list_for_each_entry_safe(buf, node, &dcmi->buffers, list) {
+		list_del_init(&buf->list);
+		vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_QUEUED);
+	}
+	spin_unlock_irq(&dcmi->irqlock);
+
+	return ret;
+}
+
+static void dcmi_stop_streaming(struct vb2_queue *vq)
+{
+	struct stm32_dcmi *dcmi = vb2_get_drv_priv(vq);
+	struct dcmi_buf *buf, *node;
+	unsigned long time_ms = msecs_to_jiffies(TIMEOUT_MS);
+	long timeout;
+	int ret;
+
+	/* Disable stream on the sub device */
+	ret = v4l2_subdev_call(dcmi->entity.subdev, video, s_stream, 0);
+	if (ret && ret != -ENOIOCTLCMD)
+		dev_err(dcmi->dev, "stream off failed in subdev\n");
+
+	dcmi->state = STOPPING;
+
+	timeout = wait_for_completion_interruptible_timeout(&dcmi->complete,
+							    time_ms);
+
+	spin_lock_irq(&dcmi->irqlock);
+
+	/* Disable interruptions */
+	reg_clear(dcmi->regs, DCMI_IER, IT_FRAME | IT_OVR | IT_ERR);
+
+	/* Disable DCMI */
+	reg_clear(dcmi->regs, DCMI_CR, CR_ENABLE);
+
+	if (!timeout) {
+		dev_err(dcmi->dev, "Timeout during stop streaming\n");
+		dcmi->state = STOPPED;
+	}
+
+	if (dcmi->active) {
+		buf = dcmi->active;
+		vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
+		dcmi->active = NULL;
+	}
+
+	/* Release all queued buffers */
+	list_for_each_entry_safe(buf, node, &dcmi->buffers, list) {
+		list_del_init(&buf->list);
+		vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
+	}
+
+	spin_unlock_irq(&dcmi->irqlock);
+
+	/* Stop all pending DMA operations */
+	dmaengine_terminate_all(dcmi->dma_chan);
+
+	clk_disable(dcmi->mclk);
+
+	dev_dbg(dcmi->dev, "Stop streaming, errors=%d buffers=%d\n",
+		dcmi->errors_count, dcmi->buffers_count);
+}
+
+static struct vb2_ops dcmi_video_qops = {
+	.queue_setup		= dcmi_queue_setup,
+	.buf_init		= dcmi_buf_init,
+	.buf_prepare		= dcmi_buf_prepare,
+	.buf_queue		= dcmi_buf_queue,
+	.start_streaming	= dcmi_start_streaming,
+	.stop_streaming		= dcmi_stop_streaming,
+	.wait_prepare		= vb2_ops_wait_prepare,
+	.wait_finish		= vb2_ops_wait_finish,
+};
+
+static int dcmi_g_fmt_vid_cap(struct file *file, void *priv,
+			      struct v4l2_format *fmt)
+{
+	struct stm32_dcmi *dcmi = video_drvdata(file);
+
+	*fmt = dcmi->fmt;
+
+	return 0;
+}
+
+static const struct dcmi_format *find_format_by_fourcc(struct stm32_dcmi *dcmi,
+						       unsigned int fourcc)
+{
+	unsigned int num_formats = dcmi->num_user_formats;
+	const struct dcmi_format *fmt;
+	unsigned int i;
+
+	for (i = 0; i < num_formats; i++) {
+		fmt = dcmi->user_formats[i];
+		if (fmt->fourcc == fourcc)
+			return fmt;
+	}
+
+	return NULL;
+}
+
+static int dcmi_try_fmt(struct stm32_dcmi *dcmi, struct v4l2_format *f,
+			const struct dcmi_format **current_fmt)
+{
+	const struct dcmi_format *dcmi_fmt;
+	struct v4l2_pix_format *pixfmt = &f->fmt.pix;
+	struct v4l2_subdev_pad_config pad_cfg;
+	struct v4l2_subdev_format format = {
+		.which = V4L2_SUBDEV_FORMAT_TRY,
+	};
+	int ret;
+
+	if (f->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
+		return -EINVAL;
+
+	dcmi_fmt = find_format_by_fourcc(dcmi, pixfmt->pixelformat);
+	if (!dcmi_fmt) {
+		dcmi_fmt = dcmi->user_formats[dcmi->num_user_formats - 1];
+		pixfmt->pixelformat = dcmi_fmt->fourcc;
+	}
+
+	/* Limit to hardware capabilities */
+	if (pixfmt->width > MAX_SUPPORT_WIDTH)
+		pixfmt->width = MAX_SUPPORT_WIDTH;
+	if (pixfmt->height > MAX_SUPPORT_HEIGHT)
+		pixfmt->height = MAX_SUPPORT_HEIGHT;
+	if (pixfmt->width < MIN_SUPPORT_WIDTH)
+		pixfmt->width = MIN_SUPPORT_WIDTH;
+	if (pixfmt->height < MIN_SUPPORT_HEIGHT)
+		pixfmt->height = MIN_SUPPORT_HEIGHT;
+
+	v4l2_fill_mbus_format(&format.format, pixfmt, dcmi_fmt->mbus_code);
+	ret = v4l2_subdev_call(dcmi->entity.subdev, pad, set_fmt,
+			       &pad_cfg, &format);
+	if (ret < 0)
+		return ret;
+
+	v4l2_fill_pix_format(pixfmt, &format.format);
+
+	pixfmt->field = V4L2_FIELD_NONE;
+	pixfmt->bytesperline = pixfmt->width * dcmi_fmt->bpp;
+	pixfmt->sizeimage = pixfmt->bytesperline * pixfmt->height;
+
+	if (current_fmt)
+		*current_fmt = dcmi_fmt;
+
+	return 0;
+}
+
+static int dcmi_set_fmt(struct stm32_dcmi *dcmi, struct v4l2_format *f)
+{
+	struct v4l2_subdev_format format = {
+		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
+	};
+	const struct dcmi_format *current_fmt;
+	int ret;
+
+	ret = dcmi_try_fmt(dcmi, f, &current_fmt);
+	if (ret)
+		return ret;
+
+	v4l2_fill_mbus_format(&format.format, &f->fmt.pix,
+			      current_fmt->mbus_code);
+	ret = v4l2_subdev_call(dcmi->entity.subdev, pad,
+			       set_fmt, NULL, &format);
+	if (ret < 0)
+		return ret;
+
+	dcmi->fmt = *f;
+	dcmi->current_fmt = current_fmt;
+
+	return 0;
+}
+
+static int dcmi_s_fmt_vid_cap(struct file *file, void *priv,
+			      struct v4l2_format *f)
+{
+	struct stm32_dcmi *dcmi = video_drvdata(file);
+
+	if (vb2_is_streaming(&dcmi->queue))
+		return -EBUSY;
+
+	return dcmi_set_fmt(dcmi, f);
+}
+
+static int dcmi_try_fmt_vid_cap(struct file *file, void *priv,
+				struct v4l2_format *f)
+{
+	struct stm32_dcmi *dcmi = video_drvdata(file);
+
+	return dcmi_try_fmt(dcmi, f, NULL);
+}
+
+static int dcmi_enum_fmt_vid_cap(struct file *file, void  *priv,
+				 struct v4l2_fmtdesc *f)
+{
+	struct stm32_dcmi *dcmi = video_drvdata(file);
+
+	if (f->index >= dcmi->num_user_formats)
+		return -EINVAL;
+
+	f->pixelformat = dcmi->user_formats[f->index]->fourcc;
+	return 0;
+}
+
+static int dcmi_querycap(struct file *file, void *priv,
+			 struct v4l2_capability *cap)
+{
+	strlcpy(cap->driver, DRV_NAME, sizeof(cap->driver));
+	strlcpy(cap->card, "STM32 Digital Camera Memory Interface",
+		sizeof(cap->card));
+	strlcpy(cap->bus_info, "platform:dcmi", sizeof(cap->bus_info));
+	return 0;
+}
+
+static int dcmi_enum_input(struct file *file, void *priv,
+			   struct v4l2_input *i)
+{
+	if (i->index != 0)
+		return -EINVAL;
+
+	i->type = V4L2_INPUT_TYPE_CAMERA;
+	strlcpy(i->name, "Camera", sizeof(i->name));
+	return 0;
+}
+
+static int dcmi_g_input(struct file *file, void *priv, unsigned int *i)
+{
+	*i = 0;
+	return 0;
+}
+
+static int dcmi_s_input(struct file *file, void *priv, unsigned int i)
+{
+	if (i > 0)
+		return -EINVAL;
+	return 0;
+}
+
+static int dcmi_enum_framesizes(struct file *file, void *fh,
+				struct v4l2_frmsizeenum *fsize)
+{
+	struct stm32_dcmi *dcmi = video_drvdata(file);
+	const struct dcmi_format *dcmi_fmt;
+	struct v4l2_subdev_frame_size_enum fse = {
+		.index = fsize->index,
+		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
+	};
+	int ret;
+
+	dcmi_fmt = find_format_by_fourcc(dcmi, fsize->pixel_format);
+	if (!dcmi_fmt)
+		return -EINVAL;
+
+	fse.code = dcmi_fmt->mbus_code;
+
+	ret = v4l2_subdev_call(dcmi->entity.subdev, pad, enum_frame_size,
+			       NULL, &fse);
+	if (ret)
+		return ret;
+
+	fsize->type = V4L2_FRMSIZE_TYPE_DISCRETE;
+	fsize->discrete.width = fse.max_width;
+	fsize->discrete.height = fse.max_height;
+
+	return 0;
+}
+
+static int dcmi_enum_frameintervals(struct file *file, void *fh,
+				    struct v4l2_frmivalenum *fival)
+{
+	struct stm32_dcmi *dcmi = video_drvdata(file);
+	const struct dcmi_format *dcmi_fmt;
+	struct v4l2_subdev_frame_interval_enum fie = {
+		.index = fival->index,
+		.width = fival->width,
+		.height = fival->height,
+		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
+	};
+	int ret;
+
+	dcmi_fmt = find_format_by_fourcc(dcmi, fival->pixel_format);
+	if (!dcmi_fmt)
+		return -EINVAL;
+
+	fie.code = dcmi_fmt->mbus_code;
+
+	ret = v4l2_subdev_call(dcmi->entity.subdev, pad,
+			       enum_frame_interval, NULL, &fie);
+	if (ret)
+		return ret;
+
+	fival->type = V4L2_FRMIVAL_TYPE_DISCRETE;
+	fival->discrete = fie.interval;
+
+	return 0;
+}
+
+static const struct of_device_id stm32_dcmi_of_match[] = {
+	{ .compatible = "st,stm32-dcmi"},
+	{ /* end node */ },
+};
+MODULE_DEVICE_TABLE(of, stm32_dcmi_of_match);
+
+static int dcmi_open(struct file *file)
+{
+	struct stm32_dcmi *dcmi = video_drvdata(file);
+	struct v4l2_subdev *sd = dcmi->entity.subdev;
+	int ret;
+
+	if (mutex_lock_interruptible(&dcmi->lock))
+		return -ERESTARTSYS;
+
+	ret = v4l2_fh_open(file);
+	if (ret < 0)
+		goto unlock;
+
+	if (!v4l2_fh_is_singular_file(file))
+		goto fh_rel;
+
+	ret = v4l2_subdev_call(sd, core, s_power, 1);
+	if (ret < 0 && ret != -ENOIOCTLCMD)
+		goto fh_rel;
+
+	ret = dcmi_set_fmt(dcmi, &dcmi->fmt);
+	if (ret)
+		v4l2_subdev_call(sd, core, s_power, 0);
+fh_rel:
+	if (ret)
+		v4l2_fh_release(file);
+unlock:
+	mutex_unlock(&dcmi->lock);
+	return ret;
+}
+
+static int dcmi_release(struct file *file)
+{
+	struct stm32_dcmi *dcmi = video_drvdata(file);
+	struct v4l2_subdev *sd = dcmi->entity.subdev;
+	bool fh_singular;
+	int ret;
+
+	mutex_lock(&dcmi->lock);
+
+	fh_singular = v4l2_fh_is_singular_file(file);
+
+	ret = _vb2_fop_release(file, NULL);
+
+	if (fh_singular)
+		v4l2_subdev_call(sd, core, s_power, 0);
+
+	mutex_unlock(&dcmi->lock);
+
+	return ret;
+}
+
+static const struct v4l2_ioctl_ops dcmi_ioctl_ops = {
+	.vidioc_querycap		= dcmi_querycap,
+
+	.vidioc_try_fmt_vid_cap		= dcmi_try_fmt_vid_cap,
+	.vidioc_g_fmt_vid_cap		= dcmi_g_fmt_vid_cap,
+	.vidioc_s_fmt_vid_cap		= dcmi_s_fmt_vid_cap,
+	.vidioc_enum_fmt_vid_cap	= dcmi_enum_fmt_vid_cap,
+
+	.vidioc_enum_input		= dcmi_enum_input,
+	.vidioc_g_input			= dcmi_g_input,
+	.vidioc_s_input			= dcmi_s_input,
+
+	.vidioc_enum_framesizes		= dcmi_enum_framesizes,
+	.vidioc_enum_frameintervals	= dcmi_enum_frameintervals,
+
+	.vidioc_reqbufs			= vb2_ioctl_reqbufs,
+	.vidioc_create_bufs		= vb2_ioctl_create_bufs,
+	.vidioc_querybuf		= vb2_ioctl_querybuf,
+	.vidioc_qbuf			= vb2_ioctl_qbuf,
+	.vidioc_dqbuf			= vb2_ioctl_dqbuf,
+	.vidioc_expbuf			= vb2_ioctl_expbuf,
+	.vidioc_prepare_buf		= vb2_ioctl_prepare_buf,
+	.vidioc_streamon		= vb2_ioctl_streamon,
+	.vidioc_streamoff		= vb2_ioctl_streamoff,
+
+	.vidioc_log_status		= v4l2_ctrl_log_status,
+	.vidioc_subscribe_event		= v4l2_ctrl_subscribe_event,
+	.vidioc_unsubscribe_event	= v4l2_event_unsubscribe,
+};
+
+static const struct v4l2_file_operations dcmi_fops = {
+	.owner		= THIS_MODULE,
+	.unlocked_ioctl	= video_ioctl2,
+	.open		= dcmi_open,
+	.release	= dcmi_release,
+	.poll		= vb2_fop_poll,
+	.mmap		= vb2_fop_mmap,
+#ifndef CONFIG_MMU
+	.get_unmapped_area = vb2_fop_get_unmapped_area,
+#endif
+	.read		= vb2_fop_read,
+};
+
+static int dcmi_set_default_fmt(struct stm32_dcmi *dcmi)
+{
+	struct v4l2_format f = {
+		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
+		.fmt.pix = {
+			.width		= CIF_WIDTH,
+			.height		= CIF_HEIGHT,
+			.field		= V4L2_FIELD_NONE,
+			.pixelformat	= dcmi->user_formats[0]->fourcc,
+		},
+	};
+	int ret;
+
+	ret = dcmi_try_fmt(dcmi, &f, NULL);
+	if (ret)
+		return ret;
+	dcmi->current_fmt = dcmi->user_formats[0];
+	dcmi->fmt = f;
+	return 0;
+}
+
+static const struct dcmi_format dcmi_formats[] = {
+	{
+		.fourcc = V4L2_PIX_FMT_RGB565,
+		.mbus_code = MEDIA_BUS_FMT_RGB565_2X8_LE,
+		.bpp = 2,
+	}, {
+		.fourcc = V4L2_PIX_FMT_YUYV,
+		.mbus_code = MEDIA_BUS_FMT_YUYV8_2X8,
+		.bpp = 2,
+	}, {
+		.fourcc = V4L2_PIX_FMT_UYVY,
+		.mbus_code = MEDIA_BUS_FMT_UYVY8_2X8,
+		.bpp = 2,
+	},
+};
+
+static int dcmi_formats_init(struct stm32_dcmi *dcmi)
+{
+	const struct dcmi_format *dcmi_fmts[ARRAY_SIZE(dcmi_formats)];
+	unsigned int num_fmts = 0, i, j;
+	struct v4l2_subdev *subdev = dcmi->entity.subdev;
+	struct v4l2_subdev_mbus_code_enum mbus_code = {
+		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
+	};
+
+	while (!v4l2_subdev_call(subdev, pad, enum_mbus_code,
+				 NULL, &mbus_code)) {
+		for (i = 0; i < ARRAY_SIZE(dcmi_formats); i++) {
+			if (dcmi_formats[i].mbus_code != mbus_code.code)
+				continue;
+
+			/* Code supported, have we got this fourcc yet? */
+			for (j = 0; j < num_fmts; j++)
+				if (dcmi_fmts[j]->fourcc ==
+						dcmi_formats[i].fourcc)
+					/* Already available */
+					break;
+			if (j == num_fmts)
+				/* New */
+				dcmi_fmts[num_fmts++] = dcmi_formats + i;
+		}
+		mbus_code.index++;
+	}
+
+	if (!num_fmts)
+		return -ENXIO;
+
+	dcmi->num_user_formats = num_fmts;
+	dcmi->user_formats = devm_kcalloc(dcmi->dev,
+					 num_fmts, sizeof(struct dcmi_format *),
+					 GFP_KERNEL);
+	if (!dcmi->user_formats) {
+		dev_err(dcmi->dev, "could not allocate memory\n");
+		return -ENOMEM;
+	}
+
+	memcpy(dcmi->user_formats, dcmi_fmts,
+	       num_fmts * sizeof(struct dcmi_format *));
+	dcmi->current_fmt = dcmi->user_formats[0];
+
+	return 0;
+}
+
+static int dcmi_graph_notify_complete(struct v4l2_async_notifier *notifier)
+{
+	struct stm32_dcmi *dcmi = notifier_to_dcmi(notifier);
+	int ret;
+
+	dcmi->vdev->ctrl_handler	= dcmi->entity.subdev->ctrl_handler;
+	ret = dcmi_formats_init(dcmi);
+	if (ret) {
+		dev_err(dcmi->dev, "No supported mediabus format found\n");
+		return ret;
+	}
+
+	ret = dcmi_set_default_fmt(dcmi);
+	if (ret) {
+		dev_err(dcmi->dev, "Could not set default format\n");
+		return ret;
+	}
+
+	ret = video_register_device(dcmi->vdev, VFL_TYPE_GRABBER, -1);
+	if (ret) {
+		dev_err(dcmi->dev, "Failed to register video device\n");
+		return ret;
+	}
+
+	dev_dbg(dcmi->dev, "Device registered as %s\n",
+		video_device_node_name(dcmi->vdev));
+	return 0;
+}
+
+static void dcmi_graph_notify_unbind(struct v4l2_async_notifier *notifier,
+				     struct v4l2_subdev *sd,
+				     struct v4l2_async_subdev *asd)
+{
+	struct stm32_dcmi *dcmi = notifier_to_dcmi(notifier);
+
+	dev_dbg(dcmi->dev, "Removing %s\n", video_device_node_name(dcmi->vdev));
+
+	/* Checks internaly if vdev has been init or not */
+	video_unregister_device(dcmi->vdev);
+}
+
+static int dcmi_graph_notify_bound(struct v4l2_async_notifier *notifier,
+				   struct v4l2_subdev *subdev,
+				   struct v4l2_async_subdev *asd)
+{
+	struct stm32_dcmi *dcmi = notifier_to_dcmi(notifier);
+
+	dev_dbg(dcmi->dev, "Subdev %s bound\n", subdev->name);
+
+	dcmi->entity.subdev = subdev;
+
+	return 0;
+}
+
+static int dcmi_graph_parse(struct stm32_dcmi *dcmi, struct device_node *node)
+{
+	struct device_node *ep = NULL;
+	struct device_node *remote;
+
+	while (1) {
+		ep = of_graph_get_next_endpoint(node, ep);
+		if (!ep)
+			return -EINVAL;
+
+		remote = of_graph_get_remote_port_parent(ep);
+		if (!remote) {
+			of_node_put(ep);
+			return -EINVAL;
+		}
+
+		/* Remote node to connect */
+		dcmi->entity.node = remote;
+		dcmi->entity.asd.match_type = V4L2_ASYNC_MATCH_OF;
+		dcmi->entity.asd.match.of.node = remote;
+		return 0;
+	}
+}
+
+static int dcmi_graph_init(struct stm32_dcmi *dcmi)
+{
+	struct v4l2_async_subdev **subdevs = NULL;
+	int ret;
+
+	/* Parse the graph to extract a list of subdevice DT nodes. */
+	ret = dcmi_graph_parse(dcmi, dcmi->dev->of_node);
+	if (ret < 0) {
+		dev_err(dcmi->dev, "Graph parsing failed\n");
+		return ret;
+	}
+
+	/* Register the subdevices notifier. */
+	subdevs = devm_kzalloc(dcmi->dev, sizeof(*subdevs), GFP_KERNEL);
+	if (!subdevs) {
+		of_node_put(dcmi->entity.node);
+		return -ENOMEM;
+	}
+
+	subdevs[0] = &dcmi->entity.asd;
+
+	dcmi->notifier.subdevs = subdevs;
+	dcmi->notifier.num_subdevs = 1;
+	dcmi->notifier.bound = dcmi_graph_notify_bound;
+	dcmi->notifier.unbind = dcmi_graph_notify_unbind;
+	dcmi->notifier.complete = dcmi_graph_notify_complete;
+
+	ret = v4l2_async_notifier_register(&dcmi->v4l2_dev, &dcmi->notifier);
+	if (ret < 0) {
+		dev_err(dcmi->dev, "Notifier registration failed\n");
+		of_node_put(dcmi->entity.node);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int dcmi_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	const struct of_device_id *match = NULL;
+	struct v4l2_of_endpoint ep;
+	struct stm32_dcmi *dcmi;
+	struct vb2_queue *q;
+	struct dma_chan *chan;
+	struct clk *mclk;
+	int irq;
+	int ret;
+
+	match = of_match_device(of_match_ptr(stm32_dcmi_of_match), &pdev->dev);
+	if (!match) {
+		dev_err(&pdev->dev, "Could not find a match in devicetree\n");
+		return -ENODEV;
+	}
+
+	dcmi = devm_kzalloc(&pdev->dev, sizeof(struct stm32_dcmi), GFP_KERNEL);
+	if (!dcmi)
+		return -ENOMEM;
+
+	dcmi->rstc = of_reset_control_get(np, NULL);
+	if (IS_ERR(dcmi->rstc)) {
+		dev_err(&pdev->dev, "Could not get reset control\n");
+		return -ENODEV;
+	}
+
+	/* Get bus characteristics from devicetree */
+	np = of_graph_get_next_endpoint(np, NULL);
+	if (!np) {
+		dev_err(&pdev->dev, "Could not find the endpoint\n");
+		of_node_put(np);
+		goto err_reset_control_put;
+	}
+
+	ret = v4l2_of_parse_endpoint(np, &ep);
+	if (ret) {
+		dev_err(&pdev->dev, "Could not parse the endpoint\n");
+		of_node_put(np);
+		goto err_reset_control_put;
+	}
+
+	if (ep.bus_type == V4L2_MBUS_CSI2) {
+		dev_err(&pdev->dev, "CSI bus not supported\n");
+		of_node_put(np);
+		goto err_reset_control_put;
+	}
+	dcmi->bus.flags = ep.bus.parallel.flags;
+	dcmi->bus.bus_width = ep.bus.parallel.bus_width;
+	dcmi->bus.data_shift = ep.bus.parallel.data_shift;
+
+	of_node_put(np);
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq <= 0) {
+		dev_err(&pdev->dev, "Could not get irq\n");
+		return -ENODEV;
+	}
+
+	dcmi->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!dcmi->res) {
+		dev_err(&pdev->dev, "Could not get resource\n");
+		return -ENODEV;
+	}
+
+	dcmi->regs = devm_ioremap_resource(&pdev->dev, dcmi->res);
+	if (IS_ERR(dcmi->regs)) {
+		dev_err(&pdev->dev, "Could not map registers\n");
+		return PTR_ERR(dcmi->regs);
+	}
+
+	ret = devm_request_threaded_irq(&pdev->dev, irq, dcmi_irq_callback,
+					dcmi_irq_thread, IRQF_ONESHOT,
+					dev_name(&pdev->dev), dcmi);
+	if (ret) {
+		dev_err(&pdev->dev, "Unable to request irq %d\n", irq);
+		return -ENODEV;
+	}
+
+	mclk = devm_clk_get(&pdev->dev, "mclk");
+	if (IS_ERR(mclk)) {
+		dev_err(&pdev->dev, "Unable to get mclk\n");
+		return PTR_ERR(mclk);
+	}
+
+	chan = dma_request_slave_channel(&pdev->dev, "tx");
+	if (!chan) {
+		dev_info(&pdev->dev, "Unable to request DMA channel, defer probing\n");
+		return -EPROBE_DEFER;
+	}
+
+	ret = clk_prepare(mclk);
+	if (ret) {
+		dev_err(&pdev->dev, "Unable to prepare mclk %p\n", mclk);
+		goto err_dma_release;
+	}
+
+	spin_lock_init(&dcmi->irqlock);
+	mutex_init(&dcmi->lock);
+	init_completion(&dcmi->complete);
+	INIT_LIST_HEAD(&dcmi->buffers);
+
+	dcmi->dev = &pdev->dev;
+	dcmi->mclk = mclk;
+	dcmi->state = STOPPED;
+	dcmi->dma_chan = chan;
+
+	q = &dcmi->queue;
+
+	/* Initialize the top-level structure */
+	ret = v4l2_device_register(&pdev->dev, &dcmi->v4l2_dev);
+	if (ret)
+		goto err_clk_unprepare;
+
+	dcmi->vdev = video_device_alloc();
+	if (!dcmi->vdev) {
+		ret = -ENOMEM;
+		goto err_device_unregister;
+	}
+
+	/* Video node */
+	dcmi->vdev->fops = &dcmi_fops;
+	dcmi->vdev->v4l2_dev = &dcmi->v4l2_dev;
+	dcmi->vdev->queue = &dcmi->queue;
+	strlcpy(dcmi->vdev->name, KBUILD_MODNAME, sizeof(dcmi->vdev->name));
+	dcmi->vdev->release = video_device_release;
+	dcmi->vdev->ioctl_ops = &dcmi_ioctl_ops;
+	dcmi->vdev->lock = &dcmi->lock;
+	dcmi->vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_STREAMING |
+				  V4L2_CAP_READWRITE;
+	video_set_drvdata(dcmi->vdev, dcmi);
+
+	/* Buffer queue */
+	q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+	q->io_modes = VB2_MMAP | VB2_READ | VB2_DMABUF;
+	q->lock = &dcmi->lock;
+	q->drv_priv = dcmi;
+	q->buf_struct_size = sizeof(struct dcmi_buf);
+	q->ops = &dcmi_video_qops;
+	q->mem_ops = &vb2_dma_contig_memops;
+	q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
+	q->min_buffers_needed = 2;
+	q->dev = &pdev->dev;
+
+	ret = vb2_queue_init(q);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to initialize vb2 queue\n");
+		goto err_device_release;
+	}
+
+	ret = dcmi_graph_init(dcmi);
+	if (ret < 0)
+		goto err_device_release;
+
+	/* Reset device */
+	ret = reset_control_assert(dcmi->rstc);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to assert the reset line\n");
+		goto err_device_release;
+	}
+
+	usleep_range(3000, 5000);
+
+	ret = reset_control_deassert(dcmi->rstc);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to deassert the reset line\n");
+		goto err_device_release;
+	}
+
+	dev_info(&pdev->dev, "Probe done\n");
+
+	platform_set_drvdata(pdev, dcmi);
+	return 0;
+
+err_reset_control_put:
+	reset_control_put(dcmi->rstc);
+err_device_release:
+	video_device_release(dcmi->vdev);
+err_device_unregister:
+	v4l2_device_unregister(&dcmi->v4l2_dev);
+err_clk_unprepare:
+	clk_unprepare(dcmi->mclk);
+err_dma_release:
+	dma_release_channel(dcmi->dma_chan);
+
+	return ret;
+}
+
+static int dcmi_remove(struct platform_device *pdev)
+{
+	struct stm32_dcmi *dcmi = platform_get_drvdata(pdev);
+
+	v4l2_async_notifier_unregister(&dcmi->notifier);
+	v4l2_device_unregister(&dcmi->v4l2_dev);
+	clk_unprepare(dcmi->mclk);
+	dma_release_channel(dcmi->dma_chan);
+	reset_control_put(dcmi->rstc);
+
+	return 0;
+}
+
+static struct platform_driver stm32_dcmi_driver = {
+	.probe		= dcmi_probe,
+	.remove		= dcmi_remove,
+	.driver		= {
+		.name = DRV_NAME,
+		.of_match_table = of_match_ptr(stm32_dcmi_of_match),
+	},
+};
+
+module_platform_driver(stm32_dcmi_driver);
+
+MODULE_AUTHOR("Yannick Fertre <yannick.fertre@st.com>");
+MODULE_AUTHOR("Hugues Fruchet <hugues.fruchet@st.com>");
+MODULE_DESCRIPTION("STMicroelectronics STM32 Digital Camera Memory Interface driver");
+MODULE_LICENSE("GPL");
+MODULE_SUPPORTED_DEVICE("video");
-- 
1.9.1

^ permalink raw reply related

* [PATCH v1 1/8] dt-bindings: Document STM32 DCMI bindings
From: Hugues Fruchet @ 2017-03-29 13:25 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Maxime Coquelin, Alexandre Torgue,
	Mauro Carvalho Chehab, Hans Verkuil
  Cc: devicetree, linux-arm-kernel, linux-kernel, linux-media,
	Benjamin Gaignard, Yannick Fertre, Hugues Fruchet
In-Reply-To: <1490793926-6477-1-git-send-email-hugues.fruchet@st.com>

This adds documentation of device tree bindings for the STM32 DCMI
(Digital Camera Memory Interface).

Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
---
 .../devicetree/bindings/media/st,stm32-dcmi.txt    | 77 ++++++++++++++++++++++
 1 file changed, 77 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/st,stm32-dcmi.txt

diff --git a/Documentation/devicetree/bindings/media/st,stm32-dcmi.txt b/Documentation/devicetree/bindings/media/st,stm32-dcmi.txt
new file mode 100644
index 0000000..f0dc709
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/st,stm32-dcmi.txt
@@ -0,0 +1,77 @@
+STMicroelectronics STM32 Digital Camera Memory Interface (DCMI)
+
+Required properties:
+- compatible: "st,stm32-dcmi"
+- reg: physical base address and length of the registers set for the device;
+- interrupts: should contain IRQ line for the DCMI;
+- clocks: list of clock specifiers, corresponding to entries in
+          the clock-names property;
+- clock-names: must contain "mclk", which is the DCMI peripherial clock.
+- resets: Reference to a reset controller
+- reset-names: see Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
+
+DCMI supports a single port node with parallel bus. It should contain one
+'port' child node with child 'endpoint' node. Please refer to the bindings
+defined in Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Example:
+
+Device node example
+-------------------
+	dcmi: dcmi@50050000 {
+		compatible = "st,stm32-dcmi";
+		reg = <0x50050000 0x400>;
+		interrupts = <78>;
+		resets = <&rcc STM32F4_AHB2_RESET(DCMI)>;
+		clocks = <&rcc 0 STM32F4_AHB2_CLOCK(DCMI)>;
+		clock-names = "mclk";
+		pinctrl-names = "default";
+		pinctrl-0 = <&dcmi_pins>;
+		dmas = <&dma2 1 1 0x414 0x3>;
+		dma-names = "tx";
+		status = "disabled";
+	};
+
+Board setup example
+-------------------
+
+&dcmi {
+	status = "okay";
+
+	port {
+		dcmi_0: endpoint@0 {
+			remote-endpoint = <&ov2640_0>;
+			bus-width = <8>;
+			hsync-active = <0>;
+			vsync-active = <0>;
+			pclk-sample = <1>;
+		};
+	};
+};
+
+{
+	[...]
+	i2c@0 {
+		ov2640: camera@30 {
+			compatible = "ovti,ov2640";
+			reg = <0x30>;
+			resetb-gpios = <&stmpegpio 2 GPIO_ACTIVE_HIGH>;
+			pwdn-gpios = <&stmpegpio 0 GPIO_ACTIVE_LOW>;
+			clocks = <&clk_ext_camera>;
+			clock-names = "xvclk";
+			status = "okay";
+
+			port {
+				ov2640_0: endpoint {
+					remote-endpoint = <&dcmi_0>;
+				};
+			};
+		};
+	};
+
+	clk_ext_camera: clk-ext-camera {
+		#clock-cells = <0>;
+		compatible = "fixed-clock";
+		clock-frequency = <24000000>;
+	};
+}
-- 
1.9.1

^ permalink raw reply related

* [PATCH v1 0/8] Add support for DCMI camera interface of STMicroelectronics STM32 SoC series
From: Hugues Fruchet @ 2017-03-29 13:25 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, Maxime Coquelin, Alexandre Torgue,
	Mauro Carvalho Chehab, Hans Verkuil
  Cc: devicetree, linux-kernel, Yannick Fertre, Benjamin Gaignard,
	Hugues Fruchet, linux-arm-kernel, linux-media

This patchset introduces a basic support for Digital Camera Memory Interface
(DCMI) of STMicroelectronics STM32 SoC series.

This first basic support implements RGB565 & YUV frame grabbing.
Cropping and JPEG support will be added later on.

This has been tested on STM324x9I-EVAL evaluation board embedding
an OV2640 camera sensor.

This driver depends on:
  - [PATCHv6 00/14] atmel-isi/ov7670/ov2640: convert to standalone drivers http://www.spinics.net/lists/linux-media/msg113480.html

===========
= history =
===========
version 1:
  - Initial submission

===================
= v4l2-compliance =
===================
Below is the v4l2-compliance report for this current version of the DCMI camera interface.
v4l2-compliance has been built from v4l-utils-1.12.3.

v4l2-compliance SHA   : f5f45e17ee98a0ebad7836ade2b34ceec909d751

Driver Info:
        Driver name   : stm32-dcmi
        Card type     : STM32 Digital Camera Memory Int
        Bus info      : platform:dcmi
        Driver version: 4.11.0
        Capabilities  : 0x85200001
                Video Capture
                Read/Write
                Streaming
                Extended Pix Format
                Device Capabilities
        Device Caps   : 0x05200001
                Video Capture
                Read/Write
                Streaming
                Extended Pix Format

Compliance test for device /dev/video0 (not using libv4l2):

Required ioctls:
        test VIDIOC_QUERYCAP: OK

Allow for multiple opens:
        test second video open: OK
        test VIDIOC_QUERYCAP: OK
        test VIDIOC_G/S_PRIORITY: OK
        test for unlimited opens: OK

Debug ioctls:
        test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
        test VIDIOC_LOG_STATUS: OK

Input ioctls:
        test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
        test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
        test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
        test VIDIOC_ENUMAUDIO: OK (Not Supported)
        test VIDIOC_G/S/ENUMINPUT: OK
        test VIDIOC_G/S_AUDIO: OK (Not Supported)
        Inputs: 1 Audio Inputs: 0 Tuners: 0

Output ioctls:
        test VIDIOC_G/S_MODULATOR: OK (Not Supported)
        test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
        test VIDIOC_ENUMAUDOUT: OK (Not Supported)
        test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
        test VIDIOC_G/S_AUDOUT: OK (Not Supported)
        Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
        test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
        test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
        test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
        test VIDIOC_G/S_EDID: OK (Not Supported)

Test input 0:

        Control ioctls:
                test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
                test VIDIOC_QUERYCTRL: OK
                test VIDIOC_G/S_CTRL: OK
                test VIDIOC_G/S/TRY_EXT_CTRLS: OK
                test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
                test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
                Standard Controls: 3 Private Controls: 0

        Format ioctls:
                test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
                test VIDIOC_G/S_PARM: OK (Not Supported)
                test VIDIOC_G_FBUF: OK (Not Supported)
                test VIDIOC_G_FMT: OK
                test VIDIOC_TRY_FMT: OK
                test VIDIOC_S_FMT: OK
                test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
                test Cropping: OK (Not Supported)
                test Composing: OK (Not Supported)
                test Scaling: OK

        Codec ioctls:
                test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
                test VIDIOC_G_ENC_INDEX: OK (Not Supported)
                test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)

        Buffer ioctls:
                test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
                test VIDIOC_EXPBUF: OK

Test input 0:

Streaming ioctls:
        test read/write: OK
        test MMAP: OK
        test USERPTR: OK (Not Supported)
        test DMABUF: Cannot test, specify --expbuf-device


Total: 46, Succeeded: 46, Failed: 0, Warnings: 0

Hugues Fruchet (8):
  dt-bindings: Document STM32 DCMI bindings
  [media] stm32-dcmi: STM32 DCMI camera interface driver
  ARM: dts: stm32: Enable DCMI support on STM32F429 MCU
  ARM: dts: stm32: Enable DCMI camera interface on STM32F429-EVAL board
  ARM: dts: stm32: Enable stmpe1600 gpio expandor of STM32F429-EVAL
    board
  ARM: dts: stm32: Enable ov2640 camera support of STM32F429-EVAL board
  ARM: configs: stm32: stmpe 1600 GPIO expandor
  ARM: configs: stm32: DCMI + OV2640 camera support

 .../devicetree/bindings/media/st,stm32-dcmi.txt    |   77 ++
 arch/arm/boot/dts/stm32429i-eval.dts               |   56 +
 arch/arm/boot/dts/stm32f429.dtsi                   |   37 +
 arch/arm/configs/stm32_defconfig                   |    9 +
 drivers/media/platform/Kconfig                     |   12 +
 drivers/media/platform/Makefile                    |    2 +
 drivers/media/platform/stm32/Makefile              |    1 +
 drivers/media/platform/stm32/stm32-dcmi.c          | 1417 ++++++++++++++++++++
 8 files changed, 1611 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/st,stm32-dcmi.txt
 create mode 100644 drivers/media/platform/stm32/Makefile
 create mode 100644 drivers/media/platform/stm32/stm32-dcmi.c

-- 
1.9.1

^ permalink raw reply

* Re: [PATCH v3 3/7] arm: dts: dt-bindings: Add Renesas RZ pinctrl header
From: Linus Walleij @ 2017-03-29 13:22 UTC (permalink / raw)
  To: Jacopo Mondi, Bjorn Andersson
  Cc: Geert Uytterhoeven, Laurent Pinchart, Chris Brandt, Rob Herring,
	Mark Rutland, Russell King, Linux-Renesas,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1490368934-12494-4-git-send-email-jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>

On Fri, Mar 24, 2017 at 4:22 PM, Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org> wrote:

> Add dt-bindings for Renesas r7s72100 pin controller header file.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas-AW8dsiIh9cEdnm+yROfE0A@public.gmane.org>

> +/*
> + * Pin is bi-directional.
> + * An alternate function that needs both input/output functionalities shall
> + * be configured as bidirectional.
> + * Eg. SDA/SCL pins of an I2c interface.
> + */
> +#define BI_DIR                 (1 << 3)

Any specific reason why this should not simply be added to
include/linux/pinctrl/pinconf-generic.h
as PIN_CONFIG_BIDIRECTIONAL and parsed in
drivers/pinctrl/pinconf-generic.c
from the (new) DT property "bidirectional" simply?

> +/*
> + * Flags used to ask software to drive the pin I/O direction overriding the
> + * alternate function configuration.
> + * Some alternate functions require software to force I/O direction of a pin,
> + * overriding the designated one.
> + * Refer to the HW manual to know when this flag shall be used.
> + */
> +#define SWIO_IN                        (1 << 4)
> +#define SWIO_OUT               (1 << 5)

What is wrong in doing this with generic pin config using
PIN_CONFIG_INPUT_ENABLE and PIN_CONFIG_OUTPUT
(ignoring the argument)?

In the device tree use input-enable and add a new output-enable
(with unspecified value) with proper description and DT bindings?

And if you think these have no general applicability, by the end
of the day they are *still* pin config, not magic flags we can choose to
toss in with the muxing, so you can do what the Qualcomm driver
does and add custom pin configurations extending the generic
pin config, see drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
qcom,pull-up-strength etc.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v7 0/7] Implement clocksource for rockchip SoC using rockchip timer
From: Alexander Kochetkov @ 2017-03-29 13:22 UTC (permalink / raw)
  To: Daniel Lezcano, Heiko Stuebner, linux-kernel, devicetree,
	linux-arm-kernel, linux-rockchip
  Cc: Thomas Gleixner, Mark Rutland, Rob Herring, Russell King,
	Caesar Wang, Huang Tao
In-Reply-To: <1490197714-25415-1-git-send-email-al.kochet@gmail.com>

Hello, Daniel.

Due to recent comments from Mark[1], may be is better to apply v6[2] series instead of v7[3]?
Because my main goal was to fix wall time on rk3188. And I did it the same way how that was
already done for other timer drivers (one of possible solution).

You can rename CLOCKSOURCE_OF to TIMER_OF later. I can help with that, but I don’t
think it is good idea to combine my changes and timer framework cleanups/improvements
into single series.

And I thinks, that probably is is better to drop [3] and [4] and revert 0c8893c9095d ("clockevents: Add a
clkevt-of mechanism like clksrc-of»).

What do you think?

[1] https://lkml.org/lkml/2017/3/29/286
[2] https://lkml.org/lkml/2017/1/31/401
[3] https://lkml.org/lkml/2017/3/22/508
[4] https://lkml.org/lkml/2017/3/22/420
[5] https://lkml.org/lkml/2017/3/22/426

> 22 марта 2017 г., в 18:48, Alexander Kochetkov <al.kochet@gmail.com> написал(а):
> 
> Hello, Daniel, Heiko.
> 
> Here is try 7 :) Could you please take a look into it?
> 
> rockchip_timer init code implemented using CLOCKEVENT_OF_DECLARE()
> introduced in commit 0c8893c9095d ("clockevents: Add a clkevt-of
> mechanism like clksrc-of")
> 
> There is change in the arch/arm/mach-rockchip/rockchip.c.
> 
> This series should be applied after the commit:
> https://lkml.org/lkml/2017/3/22/426
> 
> As without the commit, you will get linker error ("clkevt-probe.c:63:
> undefined reference to `__clkevt_of_table’")
> 
> Regards,
> Alexander.
> 
> 
> Alexander Kochetkov (6):
>  dt-bindings: clarify compatible property for rockchip timers
>  ARM: dts: rockchip: update compatible property for rk322x timer
>  ARM: dts: rockchip: add clockevent attribute to rockchip timers
>  clocksource/drivers/rockchip_timer: implement clocksource timer
>  ARM: dts: rockchip: add timer entries to rk3188 SoC
>  ARM: dts: rockchip: disable arm-global-timer for rk3188
> 
> Daniel Lezcano (1):
>  clocksource/drivers/clksrc-evt-probe: Describe with the DT both the
>    clocksource and the clockevent
> 
> .../bindings/timer/rockchip,rk-timer.txt           |   12 +-
> Documentation/devicetree/bindings/timer/timer.txt  |   38 ++++
> arch/arm/boot/dts/rk3036.dtsi                      |    1 +
> arch/arm/boot/dts/rk3188.dtsi                      |   19 ++
> arch/arm/boot/dts/rk322x.dtsi                      |    3 +-
> arch/arm/boot/dts/rk3288.dtsi                      |    1 +
> arch/arm/mach-rockchip/rockchip.c                  |    2 +
> arch/arm64/boot/dts/rockchip/rk3368.dtsi           |    1 +
> drivers/clocksource/Kconfig                        |    2 +
> drivers/clocksource/clkevt-probe.c                 |    7 +
> drivers/clocksource/clksrc-probe.c                 |    7 +
> drivers/clocksource/rockchip_timer.c               |  215 ++++++++++++++------
> 12 files changed, 241 insertions(+), 67 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/timer/timer.txt
> 
> -- 
> 1.7.9.5
> 

^ permalink raw reply

* Re: [PATCH v3 1/2] PCI: Add tango MSI controller support
From: Mason @ 2017-03-29 13:16 UTC (permalink / raw)
  To: Marc Zyngier, Marc Gonzalez, Bjorn Helgaas, Thomas Gleixner
  Cc: Robin Murphy, Lorenzo Pieralisi, Liviu Dudau, David Laight,
	linux-pci, Linux ARM, Thibaud Cornic, Phuong Nguyen, LKML, DT
In-Reply-To: <a12a79fd-94f9-5aa4-5aa0-5dccb6c5d615@arm.com>

On 29/03/2017 14:29, Marc Zyngier wrote:

> On 29/03/17 12:29, Marc Gonzalez wrote:
>
>> The MSI controller in Tango supports 256 message-signaled interrupts,
>> and a single doorbell address.
>>
>> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
>> ---
>> Changes since v0.2
>> - Support 256 MSIs instead of only 32
>> - Use spinlock_t instead of struct mutex
>> - Add MSI_FLAG_PCI_MSIX flag
>>
>> IRQs are acked in tango_msi_isr because handle_simple_irq leaves
>> ack, clear, mask and unmask up to the driver. For the same reason,
>> interrupt enable mask is updated from tango_irq_domain_alloc/free.
> 
> I've asked you to move this to individual methods. You've decided not
> to, and that's your call. But I now wonder why I'm even bothering to
> review this, as you've so far just wasted my time.

I misunderstood what you wrote. When you pointed out the comment at
the top of handle_simple_irq (which I mentioned in my above blurb)
I took that to mean that I had to follow those instructions.

Judging by what you wrote below, I must replace handle_simple_irq
with handle_edge_irq, which will call the irq_chip callbacks.

But I don't understand how to get my pcie pointer back in irq_ack
or irq_unmask, or the relevant msi. Can you throw me a clue?

>> +static struct irq_chip tango_msi_irq_chip = {
>> +	.name = "MSI",
>> +	.irq_mask = pci_msi_mask_irq,
>> +	.irq_unmask = pci_msi_unmask_irq,
> 
> How do you make that work if the PCI device doesn't support per-MSI masking?

It seems you're saying this code is broken. Is it functional
in the Altera driver, and I did something to break it?

>> +static int find_free_msi(struct irq_domain *dom, unsigned int virq)
>> +{
>> +	u32 val;
>> +	struct tango_pcie *pcie = dom->host_data;
>> +	unsigned int offset, pos;
>> +
>> +	pos = find_first_zero_bit(pcie->bitmap, MSI_MAX);
>> +	if (pos >= MSI_MAX)
>> +		return -ENOSPC;
>> +
>> +	offset = (pos / 32) * 4;
>> +	val = readl_relaxed(pcie->msi_mask + offset);
>> +	writel_relaxed(val | BIT(pos % 32), pcie->msi_mask + offset);
> 
> Great. I'm now in a position where I can take an interrupt (because of
> the broken locking that doesn't disable interrupts), but the bitmap
> doesn't indicate it yet. With a bit of luck, I'll never make any forward
> progress.

Is this the Yoda way to say:
"Hey moron, use spin_lock_irqsave instead of spin_lock"?

>> +	irq_domain_set_info(dom, virq, pos, &tango_msi_chip,
>> +			dom->host_data, handle_simple_irq, NULL, NULL);
> 
> I've told you a number of times that PCI MSIs are edge triggered...

I will register handle_edge_irq.

> So there is not much progress from the previous version. It is just
> broken in a different ways, and ignores most of the work that is already
> done in the irqchip core.

I wish nothing more than to be able to use as much infrastructure
as possible, in order to write as little code as possible.

Regards.

^ permalink raw reply

* Re: [GIT PULL] PCI: Support for configurable PCI endpoint
From: Niklas Cassel @ 2017-03-29 13:13 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, Bjorn Helgaas, Joao Pinto, linux-pci,
	linux-doc, linux-kernel, devicetree, linux-omap, linux-arm-kernel
  Cc: hch, nsekhar
In-Reply-To: <6868a887-a270-96d6-0600-af2a1af383a8@ti.com>

On 03/29/2017 02:36 PM, Kishon Vijay Abraham I wrote:
(snip)
>>>
>>> FWIW:
>>> I've tested Kishon's tag pci-endpoint-for-4.12
>>> and PCIe on artpec6 SoC is still working fine.
>>
>> Thanks for testing it.
>>>
>>> I also included the DRA7xx PCIe driver in my
>>> kernel so that pcie-designware-ep.c gets built.
>>>
>>> My only worry is that the code in pcie-designware-ep.c
>>> is not compile tested if DRA7xx is not selected
>>> (as it is the only driver using PCIE_DW_EP at
>>> the moment).
>>
>> yeah, we should plan to include COMPILE_TEST in all pci drivers but I guess
>> there is some problem with non-ARM builds [1]. As Bjorn mentioned in the
>> thread, we could add #ifdef ARM and then include COMPILE_TEST.
> 
> I think I misunderstood your concern. yeah, there is no direct way to compile
> pcie-designware-ep.c without selecting DRA7xx.

There is no way to compile pcie-designware-host.c + pcie-designware.c
without selecting a driver that selects PCIE_DW_HOST,
so I guess the same "problem" applies there.

As long as kbuild builds PCI_DRA7XX_EP (and any driver that selects
PCIE_DW_HOST), we should be fine.

^ permalink raw reply

* Re: [PATCH v2 2/7] dt-bindings: pinctrl: Add RZ/A1 bindings doc
From: Linus Walleij @ 2017-03-29 13:10 UTC (permalink / raw)
  To: Chris Brandt
  Cc: jacopo, Geert Uytterhoeven, Jacopo Mondi, Geert Uytterhoeven,
	Laurent Pinchart, Rob Herring, Mark Rutland, Russell King,
	Linux-Renesas, linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <SG2PR06MB1165276F957CE63DBE0A55268A350-ESzmfEwOt/xoAsOJh7vwSm0DtJ1/0DrXvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>

On Wed, Mar 29, 2017 at 2:38 PM, Chris Brandt <Chris.Brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org> wrote:

>> I'm strongly in favour of something like
>>     pinmux = <PIN(1, 4) | FUNC# | FLAGS>, .... ;
>>
>> opposed to
>>     pinmux = <PIN1_4_FUNC#_FLAGS>, ... ;
>
>
> I agree. I like "<PIN(1, 4) | FUNC# | FLAGS>".

I don't know if you have seen the Mediatek things really.
include/dt-bindings/pinctrl/mt6397-pinfunc.h

Example:
#define MT6397_PIN_2_SRCLKEN_PERI__FUNC_GPIO2 (MTK_PIN_NO(2) | 0)

If you prefer to use preprocessor macros or whatever to make the
bitmasks or how you want to organize the constants in your
include files is not my concern, do whatever you seem fit, just
pack it into a 32bit thing somehow which makes sense from a
maintenance point of view.

>> Not only because it will save use from having a loong list(*) of macros
>> that has to be kept up to date when/if new RZ hardware will arrive, but
>> also because of readability and simplicity for down-stream and BSP users.
>> Speaking of which, I would like to know what does Chris think of this.
>
> The list of macros would be very long, especially against the different
> packaging version of the RZ/A1 series. 11 ports, 16-pins for each port, 8 different
> function options for each pin....2 different package/pin variations.
>
> And at the end of the day....there is no benefit for the user over just using
> a macro.

I don't know who has this idea that you could not use macros, certainly
not me. Some misunderstanding must be going on. For what I'm concerned
you can write hex numbers in the pinmux = <0x12345678>;

> The reason for the "FLAGS" is to work around a quirky hardware design (in my opinion).

The flags I don't like at all, and think they should be converted to generic
pin config because they have nothing to do with muxing.

But I will point that out in the specific patch adding them.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 2/2] mtd: spi-nor: add driver for STM32 quad spi flash controller
From: Marek Vasut @ 2017-03-29 13:09 UTC (permalink / raw)
  To: Ludovic BARRE, Cyrille Pitchen
  Cc: David Woodhouse, Brian Norris, Boris Brezillon,
	Richard Weinberger, Alexandre Torgue, Rob Herring,
	linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <2c364b99-512c-8eb6-7044-7989ba21d53b-qxv4g6HH51o@public.gmane.org>

On 03/29/2017 02:24 PM, Ludovic BARRE wrote:
> hi Marek

Hi!

> thanks for review
> comment below
> 
> On 03/29/2017 12:54 PM, Marek Vasut wrote:
>> On 03/27/2017 02:54 PM, Ludovic Barre wrote:
>>> From: Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>
>>>
>>> The quadspi is a specialized communication interface targeting single,
>>> dual or quad SPI Flash memories.
>>>
>>> It can operate in any of the following modes:
>>> -indirect mode: all the operations are performed using the quadspi
>>>   registers
>>> -read memory-mapped mode: the external Flash memory is mapped to the
>>>   microcontroller address space and is seen by the system as if it was
>>>   an internal memory
>>>
>>> Signed-off-by: Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>
>> Hi! I presume this is not compatible with the Cadence QSPI or any other
>> QSPI controller, huh ?
> it's not a cadence QSPI, it's specific for stm32 platform

OK, got it. Didn't ST use Cadence IP somewhere too though ?
That's probably what confused me, oh well ...

>> Mostly minor nits below ...
>>
>>> ---
>>>   drivers/mtd/spi-nor/Kconfig         |   7 +
>>>   drivers/mtd/spi-nor/Makefile        |   1 +
>>>   drivers/mtd/spi-nor/stm32-quadspi.c | 679
>>> ++++++++++++++++++++++++++++++++++++
>>>   3 files changed, 687 insertions(+)
>>>   create mode 100644 drivers/mtd/spi-nor/stm32-quadspi.c
>> [...]
>>
>>> +struct stm32_qspi_cmd {
>>> +    struct {
>>> +        u32 addr_width:8;
>>> +        u32 dummy:8;
>>> +        u32 data:1;
>>> +    } conf;
>> This could all be u8 ? Why this awful construct ?
> yes I could replace each u32 by u8

Yeah, please do .

>>> +    u8 opcode;
>>> +    u32 framemode;
>>> +    u32 qspimode;
>>> +    u32 addr;
>>> +    size_t len;
>>> +    void *buf;
>>> +};
>>> +
>>> +static int stm32_qspi_wait_cmd(struct stm32_qspi *qspi)
>>> +{
>>> +    u32 cr;
>>> +    int err = 0;
>> Can readl_poll_timeout() or somesuch replace this function ?
> in fact stm32_qspi_wait_cmd test if the transfer has been complete (TCF
> flag)
> else initialize an interrupt completion.
> like the time may be long I prefer keep the interrupt wait, if you agree.

I don't really mind if it fits the hardware better and I agree with you
it does here.

>>> +    if (readl_relaxed(qspi->io_base + QUADSPI_SR) & SR_TCF)
>>> +        return 0;
>>> +
>>> +    reinit_completion(&qspi->cmd_completion);
>>> +    cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
>>> +    writel_relaxed(cr | CR_TCIE, qspi->io_base + QUADSPI_CR);
>>> +
>>> +    if
>>> (!wait_for_completion_interruptible_timeout(&qspi->cmd_completion,
>>> +                               msecs_to_jiffies(1000)))
>>> +        err = -ETIMEDOUT;
>>> +
>>> +    writel_relaxed(cr, qspi->io_base + QUADSPI_CR);
>>> +    return err;
>>> +}
>>> +
>>> +static int stm32_qspi_wait_nobusy(struct stm32_qspi *qspi)
>>> +{
>>> +    u32 sr;
>>> +
>>> +    return readl_relaxed_poll_timeout(qspi->io_base + QUADSPI_SR, sr,
>>> +                      !(sr & SR_BUSY), 10, 100000);
>>> +}
>>> +
>>> +static void stm32_qspi_set_framemode(struct spi_nor *nor,
>>> +                     struct stm32_qspi_cmd *cmd, bool read)
>>> +{
>>> +    u32 dmode = CCR_DMODE_1;
>>> +
>>> +    cmd->framemode = CCR_IMODE_1;
>>> +
>>> +    if (read) {
>>> +        switch (nor->flash_read) {
>>> +        case SPI_NOR_NORMAL:
>>> +        case SPI_NOR_FAST:
>>> +            dmode = CCR_DMODE_1;
>>> +            break;
>>> +        case SPI_NOR_DUAL:
>>> +            dmode = CCR_DMODE_2;
>>> +            break;
>>> +        case SPI_NOR_QUAD:
>>> +            dmode = CCR_DMODE_4;
>>> +            break;
>>> +        }
>>> +    }
>>> +
>>> +    cmd->framemode |= cmd->conf.data ? dmode : 0;
>>> +    cmd->framemode |= cmd->conf.addr_width ? CCR_ADMODE_1 : 0;
>>> +}
>>> +
>>> +static void stm32_qspi_read_fifo(u8 *val, void __iomem *addr)
>>> +{
>>> +    *val = readb_relaxed(addr);
>>> +}
>>> +
>>> +static void stm32_qspi_write_fifo(u8 *val, void __iomem *addr)
>>> +{
>>> +    writeb_relaxed(*val, addr);
>>> +}
>>> +
>>> +static int stm32_qspi_tx_poll(struct stm32_qspi *qspi,
>>> +                  const struct stm32_qspi_cmd *cmd)
>>> +{
>>> +    void (*tx_fifo)(u8 *, void __iomem *);
>>> +    u32 len = cmd->len, sr;
>>> +    u8 *buf = cmd->buf;
>>> +    int ret;
>>> +
>>> +    if (cmd->qspimode == CCR_FMODE_INDW)
>>> +        tx_fifo = stm32_qspi_write_fifo;
>>> +    else
>>> +        tx_fifo = stm32_qspi_read_fifo;
>>> +
>>> +    while (len--) {
>>> +        ret = readl_relaxed_poll_timeout(qspi->io_base + QUADSPI_SR,
>>> +                         sr, (sr & SR_FTF),
>>> +                         10, 30000);
>> Add macros for those ad-hoc timeouts.
> I will add 2 defines (for stm32_qspi_wait_nobusy, stm32_qspi_tx_poll)
> #define STM32_QSPI_FIFO_TIMEOUT_US 30000
> #define STM32_QSPI_BUSY_TIMEOUT_US 100000

Super

>>> +        if (ret) {
>>> +            dev_err(qspi->dev, "fifo timeout (stat:%#x)\n", sr);
>>> +            break;
>>> +        }
>>> +        tx_fifo(buf++, qspi->io_base + QUADSPI_DR);
>>> +    }
>>> +
>>> +    return ret;
>>> +}
>>
>> [...]
>>
>>> +static int stm32_qspi_send(struct stm32_qspi_flash *flash,
>>> +               const struct stm32_qspi_cmd *cmd)
>>> +{
>>> +    struct stm32_qspi *qspi = flash->qspi;
>>> +    u32 ccr, dcr, cr;
>>> +    int err;
>>> +
>>> +    err = stm32_qspi_wait_nobusy(qspi);
>>> +    if (err)
>>> +        goto abort;
>>> +
>>> +    dcr = readl_relaxed(qspi->io_base + QUADSPI_DCR) & ~DCR_FSIZE_MASK;
>>> +    dcr |= DCR_FSIZE(flash->fsize);
>>> +    writel_relaxed(dcr, qspi->io_base + QUADSPI_DCR);
>>> +
>>> +    cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
>>> +    cr &= ~CR_PRESC_MASK & ~CR_FSEL;
>>> +    cr |= CR_PRESC(flash->presc);
>>> +    cr |= flash->cs ? CR_FSEL : 0;
>>> +    writel_relaxed(cr, qspi->io_base + QUADSPI_CR);
>>> +
>>> +    if (cmd->conf.data)
>>> +        writel_relaxed(cmd->len - 1, qspi->io_base + QUADSPI_DLR);
>>> +
>>> +    ccr = cmd->framemode | cmd->qspimode;
>>> +
>>> +    if (cmd->conf.dummy)
>>> +        ccr |= CCR_DCYC(cmd->conf.dummy);
>>> +
>>> +    if (cmd->conf.addr_width)
>>> +        ccr |= CCR_ADSIZE(cmd->conf.addr_width - 1);
>>> +
>>> +    ccr |= CCR_INST(cmd->opcode);
>>> +    writel_relaxed(ccr, qspi->io_base + QUADSPI_CCR);
>>> +
>>> +    if (cmd->conf.addr_width && cmd->qspimode != CCR_FMODE_MM)
>>> +        writel_relaxed(cmd->addr, qspi->io_base + QUADSPI_AR);
>>> +
>>> +    err = stm32_qspi_tx(qspi, cmd);
>>> +    if (err)
>>> +        goto abort;
>>> +
>>> +    if (cmd->qspimode != CCR_FMODE_MM) {
>>> +        err = stm32_qspi_wait_cmd(qspi);
>>> +        if (err)
>>> +            goto abort;
>>> +        writel_relaxed(FCR_CTCF, qspi->io_base + QUADSPI_FCR);
>>> +    }
>>> +
>>> +    return err;
>>> +
>>> +abort:
>>> +    writel_relaxed(readl_relaxed(qspi->io_base + QUADSPI_CR)
>>> +               | CR_ABORT, qspi->io_base + QUADSPI_CR);
>> Use a helper variable here too.
> ok
>>> +    dev_err(qspi->dev, "%s abort err:%d\n", __func__, err);
>>> +    return err;
>>> +}
>> [...]
>>
>>> +static int stm32_qspi_flash_setup(struct stm32_qspi *qspi,
>>> +                  struct device_node *np)
>>> +{
>>> +    u32 width, flash_read, presc, cs_num, max_rate = 0;
>>> +    struct stm32_qspi_flash *flash;
>>> +    struct mtd_info *mtd;
>>> +    int ret;
>>> +
>>> +    of_property_read_u32(np, "reg", &cs_num);
>>> +    if (cs_num >= STM32_MAX_NORCHIP)
>>> +        return -EINVAL;
>>> +
>>> +    of_property_read_u32(np, "spi-max-frequency", &max_rate);
>>> +    if (!max_rate)
>>> +        return -EINVAL;
>>> +
>>> +    presc = DIV_ROUND_UP(qspi->clk_rate, max_rate) - 1;
>>> +
>>> +    if (of_property_read_u32(np, "spi-rx-bus-width", &width))
>>> +        width = 1;
>>> +
>>> +    if (width == 4)
>>> +        flash_read = SPI_NOR_QUAD;
>>> +    else if (width == 2)
>>> +        flash_read = SPI_NOR_DUAL;
>>> +    else if (width == 1)
>>> +        flash_read = SPI_NOR_NORMAL;
>>> +    else
>>> +        return -EINVAL;
>>> +
>>> +    flash = &qspi->flash[cs_num];
>>> +    flash->qspi = qspi;
>>> +    flash->cs = cs_num;
>>> +    flash->presc = presc;
>>> +
>>> +    flash->nor.dev = qspi->dev;
>>> +    spi_nor_set_flash_node(&flash->nor, np);
>>> +    flash->nor.priv = flash;
>>> +    mtd = &flash->nor.mtd;
>>> +    mtd->priv = &flash->nor;
>>> +
>>> +    flash->nor.read = stm32_qspi_read;
>>> +    flash->nor.write = stm32_qspi_write;
>>> +    flash->nor.erase = stm32_qspi_erase;
>>> +    flash->nor.read_reg = stm32_qspi_read_reg;
>>> +    flash->nor.write_reg = stm32_qspi_write_reg;
>>> +    flash->nor.prepare = stm32_qspi_prep;
>>> +    flash->nor.unprepare = stm32_qspi_unprep;
>>> +
>>> +    writel_relaxed(LPTR_DFT_TIMEOUT, qspi->io_base + QUADSPI_LPTR);
>>> +
>>> +    writel_relaxed(CR_PRESC(presc) | CR_FTHRES(3) | CR_TCEN | CR_SSHIFT
>>> +               | CR_EN, qspi->io_base + QUADSPI_CR);
>>> +
>>> +    /* a minimum fsize must be set to sent the command id */
>>> +    flash->fsize = 25;
>> I don't understand why this is needed and the comment doesn't make
>> sense. Please fix.
> fsize field defines the size of external memory.

What external memory ? Unclear

> Normaly, this field is used only for memory map mode,
> but in fact is check in indirect mode.
> So while flash scan "spi_nor_scan":
> -I can't let 0.
> -I not know yet the size of flash.
> So I fix a temporary value
> 
> I will update my comment

Please do, also please consider that I'm reading the comment and I
barely have any clue about this hardware , so make sure I can understand it.

>>> +    ret = spi_nor_scan(&flash->nor, NULL, flash_read);
>>> +    if (ret) {
>>> +        dev_err(qspi->dev, "device scan failed\n");
>>> +        return ret;
>>> +    }
>>> +
>>> +    flash->fsize = __fls(mtd->size) - 1;
>>> +
>>> +    writel_relaxed(DCR_CSHT(1), qspi->io_base + QUADSPI_DCR);
>>> +
>>> +    ret = mtd_device_register(mtd, NULL, 0);
>>> +    if (ret) {
>>> +        dev_err(qspi->dev, "mtd device parse failed\n");
>>> +        return ret;
>>> +    }
>>> +
>>> +    dev_dbg(qspi->dev, "read mm:%s cs:%d bus:%d\n",
>>> +        qspi->read_mode == CCR_FMODE_MM ? "yes" : "no", cs_num, width);
>>> +
>>> +    return 0;
>>> +}
>> [...]
>>
> 


-- 
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox