Hans, would you like to take this through your tree, or should we send a pull
> request directly to Mauro ? I'd like the two patches to be merged in v4.18 if
> possible.
I've applied the patches to my tree as discussed with Hans previously.
--
Sakari Ailus
sakari.ai...@linux.intel.com
Hi Niklas,
On Sun, Apr 15, 2018 at 10:47:37PM +0200, Niklas Söderlund wrote:
> Hi Sakari,
>
> Thanks for your feedback.
>
> On 2018-04-04 23:13:57 +0300, Sakari Ailus wrote:
>
> [snip]
>
> > > > + pm_runtime_enable(>dev);
> > >
> >
/bindings/pci/hisilicon-pcie.txt:- status: Either "ok"
or "disabled".
Documentation/devicetree/bindings/pci/xgene-pci.txt:- status: Either "ok" or
"disabled".
Documentation/devicetree/bindings/pci/xgene-pci.txt:status = "ok";
Documentation/devicetree/bindings/phy/apm-xgene-phy.txt:- status
: Shall be "ok" if enabled or "disabled" if disabled.
--
Kind regards,
Sakari Ailus
e-mail: sakari.ai...@iki.fi
struct mt9t112_priv *priv = to_mt9t112(client);
> +
> + if (soc_camera_apply_board_flags(ssdd, cfg) &
> V4L2_MBUS_PCLK_SAMPLE_RISING)
> + priv->flags |= PCLK_RISING;
> +
> + return 0;
> +}
Do you have a DT based system where you use this? Then I think
c/mt9t112.h
> +
> MT9V032 APTINA CAMERA SENSOR
> M: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> L: linux-me...@vger.kernel.org
--
Regards,
Sakari Ailus
e-mail: sakari.ai...@iki.fi
er with v4l2-compliance.
Granted that the drivers will themselves handle the colour space
information correctly, it'd still provide a way for the user to gain the
knowledge of the colour space which I believe is what matters.
--
Kind regards,
Sakari Ailus
e-mail: sakari.ai...@iki.fi
> Signed-off-by: Niklas Söderlund <niklas.soderlund+rene...@ragnatech.se>
Acked-by: Sakari Ailus <sakari.ai...@linux.intel.com>
> ---
> include/uapi/linux/videodev2.h | 4
> 1 file changed, 4 insertions(+)
>
> Hi,
>
> I hope this is the correct he
[-Wtype-limits]
>
> return V4L2_COLORSPACE_IS_VALID(colorspace);
>
> Dropping the first check would fix that, but wouldn't catch invalid values
> when operating on a signed type, such as int or enum v4l2_colorspace.
How about simply casting it to u32 first (and removing the first test)?
--
Regards,
Sakari Ailus
e-mail: sakari.ai...@iki.fi
ntity;
> vdev = media_entity_to_video_device(entity);
>
> Fix the collision by renaming the macro argument to '__entity'.
>
> Fixes: 69b925c5fc36d8f1 ("media: v4l2-dev.h: add kernel-doc to two macros")
> Signed-off-by: Niklas Söderlund <niklas.soderlund+rene...@ra
uil <hans.verk...@cisco.com>
>
> Un-acked.
>
> I just noticed that this sensor driver has no enum_frame_interval and
> g/s_parm support. How would a driver ever know the frame rate of the
> sensor without that?
s/_parm/_frame_interval/ ?
We should have wrappers for this or rather to convert g/s_parm users to
g/s_frame_interval so drivers don't need to implement both.
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi
n is fine.
Acked-by: Sakari Ailus <sakari.ai...@linux.intel.com>
>
> On Wed, Jan 17, 2018 at 09:59:59AM +0200, Sakari Ailus wrote:
> > Hi Jacopo,
> >
> > On Tue, Jan 16, 2018 at 10:44:53PM +0100, Jacopo Mondi wrote:
> > > Add bindings documentation for Renesas C
nctrl-names = "default";
> + pinctrl-0 = <_pins>;
> +
> + reset-gpios = < 11 GPIO_ACTIVE_LOW>;
> + powerdown-gpios = < 12 GPIO_ACTIVE_HIGH>;
> +
> + port {
> + ov7670_out: endpoint {
> + remote-endpoint = <_in>;
> +
> + hsync-active = <1>;
> + vsync-active = <0>;
> + };
> + };
> + };
> +};
> --
> 2.7.4
>
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi
onboard.com>
Acked-by: Sakari Ailus <sakari.ai...@linux.intel.com>
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi
pix_format. So no, I can't see what would be the point in making such
a reference.
> * @code:data format code (from enum v4l2_mbus_pixelcode)
> * @field: used interlacing type (from enum v4l2_field)
> * @colorspace: colorspace of the data (from enum v4l2_colorspace)
--
Regards,
Sakari Ailus
e-mail: sakari.ai...@iki.fi
Hejssan,
On Tue, Dec 19, 2017 at 12:38:51AM +0100, Niklas Söderlund wrote:
> Hi Sakari,
>
> Tack för dina kommentarer.
>
> On 2017-12-15 14:25:27 +0200, Sakari Ailus wrote:
> > On Thu, Dec 14, 2017 at 08:08:25PM +0100, Niklas Söderlund wrote:
> > > The R-Car CSI-
On Tue, Dec 19, 2017 at 12:08:56AM +0100, Niklas Söderlund wrote:
> Hej Sakari,
>
> Tack för dina kommentarer.
>
> On 2017-12-15 13:54:02 +0200, Sakari Ailus wrote:
> > On Thu, Dec 14, 2017 at 08:08:22PM +0100, Niklas Söderlund wrote:
> > > The pipeline will be mov
arse_dt() and parse_platform_data() behaves differently.
> > The former returns error if no subdevices are connected to CEU, the
> > latter returns 0. That's wrong.
> >
> > I wonder what's the correct behavior here. Other mainline drivers I
> > looked into (pxa_camera and atmel-isc) behaves differently from each
> > other, so I guess this is up to each platform to decide.
>
> No, what it means is that we've failed to standardize it, not that it
> shouldn't be standardized :-)
>
> > Also, the CEU can accept one single input (and I made it clear
> > in DT bindings documentation saying it accepts a single endpoint,
> > while I'm parsing all the available ones in driver, I will fix this)
> > but as it happens on Migo-R, there could be HW hacks to share the input
> > lines between multiple subdevices. Should I accept it from dts as well?
> >
> > So:
> > 1) Should we fail to probe if no subdevices are connected?
>
> While the CEU itself would be fully functional without a subdev, in practice
> it would be of no use. I would thus fail probing.
>
> > 2) Should we accept more than 1 subdevice from dts as it happens right
> > now for platform data?
>
> We need to support multiple connected devices, as some of the boards require
> that. What I'm not sure about is whether the multiplexer on the Migo-R board
> should be modeled as a subdevice. We could in theory connect multiple sensors
> to the CEU input signals without any multiplexer as long as all but one are
> in
> reset with their outputs in a high impedance state. As that wouldn' require a
> multiplexer we would need to support multiple endpoints in the CEU port. We
> could then support Migo-R the same way, making the multiplexer transparent.
>
> Sakari, what would you do here ?
We do have:
drivers/media/platform/video-mux.c
What is not addressed right now are the CSI-2 bus parameters, if the mux is
just a passive switch. This could be done using the frame descriptors.
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi
rocki <s.nawro...@samsung.com>
> Signed-off-by: Mauro Carvalho Chehab <mche...@s-opensource.com>
I'm not sure this is needed but it doesn't break anything either.
Feel free to add:
Acked-by: Sakari Ailus <sakari.ai...@linux.intel.com>
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi
Hi Jacopo,
On Sun, Dec 17, 2017 at 05:42:54PM +0100, jacopo mondi wrote:
> Hi Sakari,
>
> On Fri, Dec 15, 2017 at 06:17:04PM +0200, Sakari Ailus wrote:
> > Hi Jacopo,
> >
> > On Wed, Dec 13, 2017 at 07:26:20PM +0100, Jacopo Mondi wrote:
> > > The v4l2-async
}
> > + }
>
> This is the part I like the least in this patch set. The
> v4l2_subdev::subdev_notifier field should really disappear, there's no reason
> to limit subdevs to a single notifier. Implicit registration of notifiers is
> a
> dirty hack in my opinion.
>
> > mutex_lock(_lock);
> >
> > INIT_LIST_HEAD(>async_list);
>
> [snip]
>
> --
> Regards,
>
> Laurent Pinchart
>
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi
; v4l2_async_notifier *notifier,
> static int v4l2_async_notifier_try_all_subdevs(
> struct v4l2_async_notifier *notifier)
> {
> + struct device *dev = v4l2_async_notifier_dev(notifier);
> struct v4l2_device *v4l2_dev =
> v4l2_async_notifier_find_v4l2_dev(notifier);
> struct v4l2_subdev *sd;
> @@ -247,6 +322,9 @@ static int v4l2_async_notifier_try_all_subdevs(
> if (!v4l2_dev)
> return 0;
>
> + dev_dbg(dev, "Testing notifier \"%s\" against all subdevices\n",
> + fwnode_get_name(notifier->owner));
> +
> again:
> list_for_each_entry(sd, _list, async_list) {
> struct v4l2_async_subdev *asd;
> @@ -378,6 +456,9 @@ static int __v4l2_async_notifier_register(struct
> v4l2_async_notifier *notifier)
>
> notifier->owner = dev_fwnode(dev);
>
> + dev_dbg(dev, "Registering notifier \"%s\"\n",
> + fwnode_get_name(notifier->owner));
> +
> mutex_lock(_lock);
>
> for (i = 0; i < notifier->num_subdevs; i++) {
> --
> 2.7.4
>
--
Sakari Ailus
sakari.ai...@linux.intel.com
v4l2_async_notifier_unbind_all_subdevs(subdev_notifier);
>
> diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> index a15c01d..6ab04ad 100644
> --- a/include/media/v4l2-async.h
> +++ b/include/media/v4l2-async.h
> @@ -110,6 +110,7 @@ struct v4l2_async_notifier_operations {
> * @waiting: list of struct v4l2_async_subdev, waiting for their drivers
> * @done:list of struct v4l2_subdev, already probed
> * @list:member in a global list of notifiers
> + * @registered: notifier registered complete flag
> */
> struct v4l2_async_notifier {
> const struct v4l2_async_notifier_operations *ops;
> @@ -123,6 +124,7 @@ struct v4l2_async_notifier {
> struct list_head waiting;
> struct list_head done;
> struct list_head list;
> + bool registered;
> };
>
> /**
> --
> 2.7.4
>
--
Sakari Ailus
sakari.ai...@linux.intel.com
t is root
> or not, add a 'struct fwnode_handle *owner' field, whose name can be
> printed out for debug purposes.
>
> Signed-off-by: Jacopo Mondi <jacopo+rene...@jmondi.org>
You'll have struct device either through the v4l2_device or v4l2_subdev. Do
you need an additional field for this?
t struct
> fwnode_handle *fwnode,
> unsigned int nargs, unsigned int index,
> struct fwnode_reference_args *args);
>
> +const char *fwnode_get_name(const struct fwnode_handle *fwnode);
> struct fwnode_handle *fwnode_get_parent(const struct fwnode_handle *fwnode);
> struct fwnode_handle *fwnode_get_next_parent(
> struct fwnode_handle *fwnode);
--
Kind regards,
Sakari Ailus
sakari.ai...@linux.intel.com
}
>
> if (enable && priv->stream_count[vc] == 0) {
> - ret = v4l2_subdev_call(nextsd, video, s_stream, 1);
> + ret = v4l2_subdev_call(nextsd, pad, s_stream, rpad, rstream, 1);
> if (ret) {
> rcar_csi2_stop(priv);
> pm_runtime_put(priv->dev);
> goto out;
> }
> } else if (!enable && priv->stream_count[vc] == 1) {
> - ret = v4l2_subdev_call(nextsd, video, s_stream, 0);
> + ret = v4l2_subdev_call(nextsd, pad, s_stream, rpad, rstream, 0);
> }
>
> priv->stream_count[vc] += enable ? 1 : -1;
> --
> 2.15.1
>
--
Sakari Ailus
sakari.ai...@linux.intel.com
t;stream_count[vc] += enable ? 1 : -1;
> out:
> mutex_unlock(>lock);
>
> @@ -919,7 +939,9 @@ static int rcar_csi2_probe(struct platform_device *pdev)
> priv->dev = >dev;
>
> mutex_init(>lock);
> - priv->stream_count = 0;
> +
> + for (i = 0; i < 4; i++)
> + priv->stream_count[i] = 0;
>
> ret = rcar_csi2_probe_resources(priv, pdev);
> if (ret) {
> --
> 2.15.1
>
--
Sakari Ailus
sakari.ai...@linux.intel.com
>
> + vin_dbg(vin, "pad: %u stream: 0 enable: %d\n", pad->index, on);
> +
> return ret;
> }
>
> --
> 2.15.1
>
--
Sakari Ailus
sakari.ai...@linux.intel.com
return -EPIPE;
>
> ret = v4l2_subdev_call(sd, video, s_stream, 1);
> if (ret == -ENOIOCTLCMD)
> ret = 0;
> if (ret)
> - media_pipeline_stop(>vdev.entity);
> + media_pipeline_stop(vin->vdev.entity.pads);
>
> return ret;
> }
> --
> 2.15.1
>
--
Sakari Ailus
sakari.ai...@linux.intel.com
truct v4l2_subdev_routing *route);
> int (*set_routing)(struct v4l2_subdev *sd,
> struct v4l2_subdev_routing *route);
> + int (*s_stream)(struct v4l2_subdev *sd, unsigned int pad,
> + unsigned int stream, int enable);
How about bool for enable?
> };
>
> /**
--
Sakari Ailus
sakari.ai...@linux.intel.com
a bit from the fact that V4L2 has never standardized a naming
> scheme for the devices. It wouldn't be fair to ask you to fix that as a
> prerequisite to get the VIN driver merged, but we clearly have to work on
> that
> at some point.
Agreed, this needs to be stable and I think aligning to what omap3isp or
vsp1 do would be a good fix here.
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi
which store their default parent device
> node.
>
> Signed-off-by: Kieran Bingham <kieran.bingham+rene...@ideasonboard.com>
> Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>
>
> ---
>
> Hi Sakari,
>
> Since you signed-off on this patch - it has
klas.soderlund+rene...@ragnatech.se>
> Acked-by: Rob Herring <r...@kernel.org>
Acked-by: Sakari Ailus <sakari.ai...@linux.intel.com>
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi
Hejssan!!
On Sat, Dec 02, 2017 at 03:50:21PM +0100, Niklas Söderlund wrote:
> Hej Sakari,
>
> Thanks for your feedback.
>
> On 2017-12-02 16:05:08 +0200, Sakari Ailus wrote:
> > Hejssan,
> >
> > On Sat, Dec 02, 2017 at 12:08:21PM +0100, Niklas Söderlund wro
> > +
> > > + ret = v4l2_async_register_subdev(>subdev);
> > > + if (ret < 0)
> > > + goto error;
> > > +
> > > + pm_runtime_enable(>dev);
> >
> > Is this enough for platform devices? Just wondering.
>
> As far as I can tell from the documentation this should be enough. I'm
> no expert on PM so if I'm wrong please let me know.
>
> Geert: do I understand the documentation correctly?
>
> >
> > > +
> > > + dev_info(priv->dev, "%d lanes found\n", priv->lanes);
> >
> > I'd use dev_dbg.
>
> I'm thorn about this one. I agree that the information printed is not
> critical. But I have found this useful when receiving logs from users
> who have misconfigured there DTS with the wrong number of lines.
>
> I'm open to changing this, but if it's a matter of taste I prefer to
> keep it at a info level.
No objections. There are a bunch of stuff that can go wrong, this is just
one small piece of that. Thinking about it, it might be nice to add debug
prints for endpoint parsing so we'd have a generic way to print this
information.
--
Regards,
Sakari Ailus
e-mail: sakari.ai...@iki.fi
.data = _csi2_info_r8a7796,
> + },
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, rcar_csi2_of_table);
> +
> +static const struct soc_device_attribute r8a7795es1[] = {
> + {
> + .soc_id = "r8a7795", .revision = "ES1.*",
> + .data = _csi2_info_r8a7795es1,
> + },
> + { /* sentinel */}
> +};
> +
> +static int rcar_csi2_probe(struct platform_device *pdev)
> +{
> + const struct soc_device_attribute *attr;
> + struct rcar_csi2 *priv;
> + unsigned int i;
> + int ret;
> +
> + priv = devm_kzalloc(>dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->info = of_device_get_match_data(>dev);
> +
> + /* r8a7795 ES1.x behaves different then ES2.0+ but no own compat */
> + attr = soc_device_match(r8a7795es1);
> + if (attr)
> + priv->info = attr->data;
> +
> + priv->dev = >dev;
> +
> + mutex_init(>lock);
> + priv->stream_count = 0;
> +
> + ret = rcar_csi2_probe_resources(priv, pdev);
> + if (ret) {
> + dev_err(priv->dev, "Failed to get resources\n");
> + return ret;
> + }
> +
> + platform_set_drvdata(pdev, priv);
> +
> + ret = rcar_csi2_parse_dt(priv);
> + if (ret)
> + return ret;
> +
> + priv->subdev.owner = THIS_MODULE;
> + priv->subdev.dev = >dev;
> + v4l2_subdev_init(>subdev, _csi2_subdev_ops);
> + v4l2_set_subdevdata(>subdev, >dev);
> + snprintf(priv->subdev.name, V4L2_SUBDEV_NAME_SIZE, "%s %s",
> + KBUILD_MODNAME, dev_name(>dev));
> + priv->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
> +
> + priv->subdev.entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;
> + priv->subdev.entity.ops = _csi2_entity_ops;
> +
> + priv->pads[RCAR_CSI2_SINK].flags = MEDIA_PAD_FL_SINK;
> + for (i = RCAR_CSI2_SOURCE_VC0; i < NR_OF_RCAR_CSI2_PAD; i++)
> + priv->pads[i].flags = MEDIA_PAD_FL_SOURCE;
> +
> + ret = media_entity_pads_init(>subdev.entity, NR_OF_RCAR_CSI2_PAD,
> + priv->pads);
> + if (ret)
> + goto error;
> +
> + ret = v4l2_async_register_subdev(>subdev);
> + if (ret < 0)
> + goto error;
> +
> + pm_runtime_enable(>dev);
Is this enough for platform devices? Just wondering.
> +
> + dev_info(priv->dev, "%d lanes found\n", priv->lanes);
I'd use dev_dbg.
> +
> + return 0;
> +
> +error:
> + v4l2_async_notifier_unregister(>notifier);
> + v4l2_async_notifier_cleanup(>notifier);
> +
> + return ret;
> +}
> +
> +static int rcar_csi2_remove(struct platform_device *pdev)
> +{
> + struct rcar_csi2 *priv = platform_get_drvdata(pdev);
> +
> + v4l2_async_notifier_unregister(>notifier);
> + v4l2_async_notifier_cleanup(>notifier);
> + v4l2_async_unregister_subdev(>subdev);
> +
> + pm_runtime_disable(>dev);
> +
> + return 0;
> +}
> +
> +static struct platform_driver __refdata rcar_csi2_pdrv = {
> + .remove = rcar_csi2_remove,
> + .probe = rcar_csi2_probe,
> + .driver = {
> + .name = "rcar-csi2",
> + .of_match_table = of_match_ptr(rcar_csi2_of_table),
> + },
> +};
> +
> +module_platform_driver(rcar_csi2_pdrv);
> +
> +MODULE_AUTHOR("Niklas Söderlund <niklas.soderl...@ragnatech.se>");
> +MODULE_DESCRIPTION("Renesas R-Car MIPI CSI-2 receiver");
> +MODULE_LICENSE("GPL v2");
--
Hälsningar,
Sakari Ailus
e-mail: sakari.ai...@iki.fi
On Wed, Nov 29, 2017 at 01:04:30PM +0200, Sakari Ailus wrote:
> Hi Jacopo,
>
> On Mon, Nov 27, 2017 at 11:26:53AM +0100, Jacopo Mondi wrote:
> > ov7670 currently supports configuration of a few parameters only through
> > platform data. Implement media bus configuration by
> + else if (ret > 0)
> + info->pclk_hb_disable = true;
> + else
> + info->pclk_hb_disable = false;
> +
> + ret = ov7670_parse_dt_prop(>dev, "ov7670,pll-bypass");
> + if (ret < 0 && ret != -EINVAL)
> + return ret;
> + else if (ret > 0)
> + info->pll_bypass = true;
> + else
> + info->pll_bypass = false;
> +
> + } else if (client->dev.platform_data) {
> struct ov7670_config *config = client->dev.platform_data;
>
> /*
> @@ -1649,9 +1746,6 @@ static int ov7670_probe(struct i2c_client *client,
> tpf.denominator = 30;
> info->devtype->set_framerate(sd, );
>
> - if (info->pclk_hb_disable)
> - ov7670_write(sd, REG_COM10, COM10_PCLK_HB);
> -
> v4l2_ctrl_handler_init(>hdl, 10);
> v4l2_ctrl_new_std(>hdl, _ctrl_ops,
> V4L2_CID_BRIGHTNESS, 0, 255, 1, 128);
> --
> 2.7.4
>
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi
On Fri, Nov 17, 2017 at 10:14:51AM +0100, jacopo mondi wrote:
> Hi Sakari!
>
> On Fri, Nov 17, 2017 at 02:43:15AM +0200, Sakari Ailus wrote:
> > Hi Jacopo,
> >
> > On Wed, Nov 15, 2017 at 11:56:01AM +0100, Jacopo Mondi wrote:
> > >
>
> [snip]
>
>
On Fri, Nov 17, 2017 at 10:33:55AM +0100, jacopo mondi wrote:
> Hi Sakari!
>
> On Fri, Nov 17, 2017 at 02:36:51AM +0200, Sakari Ailus wrote:
> > Hi Jacopo,
> >
> > On Wed, Nov 15, 2017 at 03:25:11PM +0100, jacopo mondi wrote:
> > > Hi Sakari,
> > >
priv->clk);
> +error_clk_enable:
> + v4l2_ctrl_handler_free(>hdl);
>
> return ret;
> }
> @@ -1096,7 +1127,8 @@ static int ov772x_remove(struct i2c_client *client)
> {
> struct ov772x_priv *priv = to_ov772x(i2c_get_clientdata(client));
>
> - v4l2_clk_put(priv->clk);
> + if (priv->clk)
> + clk_put(priv->clk);
> v4l2_device_unregister_subdev(>subdev);
> v4l2_ctrl_handler_free(>hdl);
> return 0;
> diff --git a/include/media/i2c/ov772x.h b/include/media/i2c/ov772x.h
> index 00dbb7c..5896dff 100644
> --- a/include/media/i2c/ov772x.h
> +++ b/include/media/i2c/ov772x.h
> @@ -54,6 +54,9 @@ struct ov772x_edge_ctrl {
> struct ov772x_camera_info {
> unsigned long flags;
> struct ov772x_edge_ctrl edgectrl;
> +
> + int (*platform_enable)(void);
> + void (*platform_disable)(void);
> };
>
> #endif /* __OV772X_H__ */
> --
> 2.7.4
>
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi
Hi Jacopo,
On Wed, Nov 15, 2017 at 03:25:11PM +0100, jacopo mondi wrote:
> Hi Sakari,
>thanks for review!
You're welcome!
> On Wed, Nov 15, 2017 at 02:45:51PM +0200, Sakari Ailus wrote:
> > Hi Jacopo,
> >
> > Could you remove the original driver and send the pat
runtime_disable(>dev);
>
> + video_device_unplug(>vdev);
Does this depend on another patch?
>
> if (!vin->info->use_mc) {
> v4l2_async_notifier_unregister(>notifier);
> --
> Regards,
>
> Laurent Pinchart
>
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi
of a section entered with
> video_device_enter().
> + * It wakes up all tasks waiting on video_device_unplug() for device access
> + * sections to be exited.
> + */
> +void video_device_exit(struct video_device *vdev);
> +
> +/**
> + * video_device_unplug - mark a device as unplugged
> + * @vdev: the video device
> + *
> + * Mark a device as unplugged, causing all subsequent calls to
> + * video_device_enter() to return an error. If a device access section is
> + * currently being executed the function waits until the section is exited as
> + * marked by a call to video_device_exit().
> + */
> +void video_device_unplug(struct video_device *vdev);
> +
> #endif /* _V4L2_DEV_H */
> --
> Regards,
>
> Laurent Pinchart
>
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi
vbuf = _entry(buf_head, struct sh_mobile_ceu_buffer,
> +queue)->vb;
> + vb2_buffer_done(>vb2_buf, VB2_BUF_STATE_DONE);
This should be VB2_BUF_STATE_ERROR, as the hardware hasn't actually
processed them, right?
> list_del_init(buf_head);
> + }
>
> spin_unlock_irq(>lock);
>
--
Regards,
Sakari Ailus
e-mail: sakari.ai...@iki.fi
Hi Jacopo,
Could you remove the original driver and send the patch using git
send-email -C ? That way a single patch would address converting it to a
proper V4L2 driver as well as move it to the correct location. The changes
would be easier to review that way since then, well, it'd be easier to
idth;
> + unsigned char bus_shift;
> + unsigned int i2c_adapter_id;
> + unsigned int i2c_address;
> +};
> +
> +struct ceu_info {
> + unsigned int num_subdevs;
> + struct ceu_async_subdev subdevs[CEU_MAX_SENS];
> +};
> +
> +#endif /* __ASM_RENESAS_CEU_H__ */
> --
> 2.7.4
>
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi
ot;fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <2400>;
> + };
What's the purpose of the fixed_clk node here?
> +
> + port {
> + ov7670_out: endpoint {
> + remote-endpoint = <_in>;
> +
> + bus-width = <8>;
> + hsync-active = <1>;
> + vsync-active = <1>;
> + pclk-sample = <1>;
> + data-active = <1>;
> + };
> + };
> + };
--
Regards,
Sakari Ailus
e-mail: sakari.ai...@iki.fi
r manages calling the s_power callback so there's
hardly a need to do so here. It's just that the master drivers still need
that as long as there are sub-device drivers that depend on it.
--
Regards,
Sakari Ailus
e-mail: sakari.ai...@iki.fi
Hejssan, Niklas!
On Sat, Nov 11, 2017 at 01:11:13AM +0100, Niklas Söderlund wrote:
> Hej Sakari,
>
> On 2017-11-11 00:32:27 +0200, Sakari Ailus wrote:
> > Hej Niklas,
> >
> > Tack för uppdaterade lappar! Jag har några kommentar nedan... det ser bra
> > ut öve
t; + .hsfreqrange = hsfreqrange_m3w_h3es1,
> +};
> +
> +static const struct of_device_id rcar_csi2_of_table[] = {
> + {
> + .compatible = "renesas,r8a7795-csi2",
> + .data = _csi2_info_r8a7795,
> + },
> + {
> + .compatible = "renesas,r8a7796-csi2",
> + .data = _csi2_info_r8a7796,
> + },
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, rcar_csi2_of_table);
> +
> +static const struct soc_device_attribute r8a7795es1[] = {
> + {
> + .soc_id = "r8a7795", .revision = "ES1.*",
> + .data = _csi2_info_r8a7795es1,
> + },
> + { /* sentinel */}
> +};
> +
> +static int rcar_csi2_probe(struct platform_device *pdev)
> +{
> + const struct soc_device_attribute *attr;
> + struct rcar_csi2 *priv;
> + unsigned int i;
> + int ret;
> +
> + priv = devm_kzalloc(>dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->info = of_device_get_match_data(>dev);
> +
> + /* r8a7795 ES1.x behaves different then ES2.0+ but no own compat */
> + attr = soc_device_match(r8a7795es1);
> + if (attr)
> + priv->info = attr->data;
> +
> + priv->dev = >dev;
> +
> + mutex_init(>lock);
> + priv->stream_count = 0;
> +
> + ret = rcar_csi2_probe_resources(priv, pdev);
> + if (ret) {
> + dev_err(priv->dev, "Failed to get resources\n");
> + return ret;
> + }
> +
> + platform_set_drvdata(pdev, priv);
> +
> + ret = rcar_csi2_parse_dt(priv);
> + if (ret)
> + return ret;
> +
> + priv->subdev.owner = THIS_MODULE;
> + priv->subdev.dev = >dev;
> + v4l2_subdev_init(>subdev, _csi2_subdev_ops);
> + v4l2_set_subdevdata(>subdev, >dev);
> + snprintf(priv->subdev.name, V4L2_SUBDEV_NAME_SIZE, "%s %s",
> + KBUILD_MODNAME, dev_name(>dev));
> + priv->subdev.flags = V4L2_SUBDEV_FL_HAS_DEVNODE;
> +
> + priv->subdev.entity.function = MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER;
> + priv->subdev.entity.ops = _csi2_entity_ops;
> +
> + priv->pads[RCAR_CSI2_SINK].flags = MEDIA_PAD_FL_SINK;
> + for (i = RCAR_CSI2_SOURCE_VC0; i < NR_OF_RCAR_CSI2_PAD; i++)
> + priv->pads[i].flags = MEDIA_PAD_FL_SOURCE;
> +
> + ret = media_entity_pads_init(>subdev.entity, NR_OF_RCAR_CSI2_PAD,
> + priv->pads);
> + if (ret)
> + goto error;
> +
> + ret = v4l2_async_register_subdev(>subdev);
> + if (ret < 0)
> + goto error;
> +
> + pm_runtime_enable(>dev);
> +
> + dev_info(priv->dev, "%d lanes found\n", priv->lanes);
> +
> + return 0;
> +
> +error:
> + v4l2_async_notifier_cleanup(>notifier);
> +
> + return ret;
> +}
> +
> +static int rcar_csi2_remove(struct platform_device *pdev)
> +{
> + struct rcar_csi2 *priv = platform_get_drvdata(pdev);
> +
> + v4l2_async_notifier_cleanup(>notifier);
> + v4l2_async_unregister_subdev(>subdev);
> +
> + pm_runtime_disable(>dev);
> +
> + return 0;
> +}
> +
> +static struct platform_driver __refdata rcar_csi2_pdrv = {
> + .remove = rcar_csi2_remove,
> + .probe = rcar_csi2_probe,
> + .driver = {
> + .name = "rcar-csi2",
> + .of_match_table = of_match_ptr(rcar_csi2_of_table),
> + },
> +};
> +
> +module_platform_driver(rcar_csi2_pdrv);
> +
> +MODULE_AUTHOR("Niklas Söderlund <niklas.soderl...@ragnatech.se>");
> +MODULE_DESCRIPTION("Renesas R-Car MIPI CSI-2 receiver");
> +MODULE_LICENSE("GPL v2");
--
Kind regards,
Sakari Ailus
e-mail: sakari.ai...@iki.fi
reg = <7>;
> + remote-endpoint = <>;
> + };
> + };
> + };
> + };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index adbf69306e9ee3d2..fa81ee6e80274646 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8565,6 +8565,7 @@ L: linux-me...@vger.kernel.org
> L: linux-renesas-soc@vger.kernel.org
> T: git git://linuxtv.org/media_tree.git
> S: Supported
> +F: Documentation/devicetree/bindings/media/rcar-csi2.txt
> F: Documentation/devicetree/bindings/media/rcar_vin.txt
> F: drivers/media/platform/rcar-vin/
>
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi
Hi Mauro,
(Removing the non-list recipients.)
On Fri, Sep 29, 2017 at 06:27:13AM -0300, Mauro Carvalho Chehab wrote:
> Em Thu, 28 Sep 2017 15:09:21 +0300
> Sakari Ailus <sakari.ai...@iki.fi> escreveu:
>
> > Hi Mauro,
> >
> > On Wed, Sep 27, 2017 at 06:46:56PM -
roperty API is just an API.
--
Kind regards,
Sakari Ailus
e-mail: sakari.ai...@iki.fi
ount by using of_get_parent() instead of of_get_next_parent() which
> >> > don't decrement the usecount of the node passed to it.
> >> >
> >> > Fixes: 3b27d00e7b6d7c88 ("device property: Move fwnode graph ops to
> >> > firmware specific locatio
dev[i++] = d;
> - else
> - put_device(d);
> + dev[i++] = d;
> }
>
> mutex_unlock(_lock);
>
> /*
>* Call device_attach() to reprobe devices
> - *
> - * NOTE: If dev allocation fails, i is
feedback.
> >>
> >> On 2017-08-18 14:20:08 +0300, Laurent Pinchart wrote:
> >>> Hello,
> >>>
> >>> On Tuesday 15 Aug 2017 19:09:33 Sakari Ailus wrote:
> >>>> On Mon, Jul 31, 2017 at 12:31:58AM +0200, Niklas Söderlund wrote:
> &g
Hejssan Niklas,
Niklas Söderlund wrote:
Hi Sakari,
On 2017-08-21 22:03:02 +0300, Sakari Ailus wrote:
Hi Niklas,
Niklas Söderlund wrote:
Hi Sakari,
On 2017-08-21 16:30:17 +0300, Sakari Ailus wrote:
Hi Niklas,
Niklas Söderlund wrote:
Using CONFIG_OF_DYNAMIC=y uncovered an imbalance
Hi Niklas,
Niklas Söderlund wrote:
Hi Sakari,
On 2017-08-21 16:30:17 +0300, Sakari Ailus wrote:
Hi Niklas,
Niklas Söderlund wrote:
Using CONFIG_OF_DYNAMIC=y uncovered an imbalance in the usecount of the
node being passed to of_fwnode_graph_get_port_parent(). Preserve the
usecount just like
device_node *np;
+ /*
+* Preserve usecount for passed in node as of_get_next_parent()
+* will do of_node_put() on it.
+*/
+ of_node_get(to_of_node(fwnode));
+
/* Get the parent of the port */
np = of_get_next_parent(to_of_node(fwnode));
if (!n
On Fri, Aug 18, 2017 at 03:42:07PM +0200, Niklas Söderlund wrote:
> Hi,
>
> On 2017-08-18 14:15:26 +0300, Laurent Pinchart wrote:
> > Hi Sakari,
> >
> > On Tuesday 15 Aug 2017 19:16:14 Sakari Ailus wrote:
> > > On Mon, Jul 31, 2017 at 12:31:55AM +0200, Nikl
ing changes to the order in which things work, and
that's not necessary to achieve the objective of passing the async subdev
pointer to the notifier.
With that changed,
Acked-by: Sakari Ailus <sakari.ai...@linux.intel.com>
> ---
> drivers/media/v4l2-core/v4l2-async.c | 6 +++---
&g
the same lock, so using it here could lead to a
> + * dead lock situation.
> + */
> +
> for (i = 0; i < count; i++) {
> /* If we handled USB devices, we'd have to lock the parent too
> */
> device_release_driver(dev[i]);
--
Regards,
Sakari Ailus
e-mail: sakari.ai...@iki.fi
and use it. No need to look it up or check whether it was found.
> + if (!ctrl)
> + return -EINVAL;
> +
> + return v4l2_ctrl_s_ctrl_int64(ctrl, rate);
> +}
--
Kind regards,
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
tion is that the initialisation of all the related
devices is required to be completed until any device nodes are registered.
This as itself is a part of the problem problem.
How that can be solved is by registering the device nodes of sub-devices
when the sub-devices are registered rather than at the time when all of
them are registered. This will have the effect of making partial device
states visible to the user space during driver probing.
I'd like to claim that it is not knowable whether a device the probing of
which has failed is going to come up in a fraction of a second or is not
going to come up during system uptime. -EPROBE_DEFER probably means that
the device will be around but there's no guarantee about it either, whereas
another error could be possibly fixed e.g. by the user.
Besides, the device nodes are even currently not created at the same
moment; they are rather created within a small window of time during which
the user will have access to the device nodes already created --- which do
not (yet) represent the complete device.
--
Kind regards,
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
Hejssan Niklas,
On Thu, Jun 15, 2017 at 10:48:20AM +0200, Niklas Söderlund wrote:
> Hi Sakari,
>
> Thanks for your comments.
>
> On 2017-05-29 14:35:16 +0300, Sakari Ailus wrote:
> > Hi Niklas,
> >
> > A few comments below.
> >
> > On Wed, May 2
Hi Niklas,
On Tue, Jun 13, 2017 at 06:50:14PM +0200, Niklas Söderlund wrote:
> Hi Sakari,
>
> Thanks for your feedback.
>
> On 2017-05-29 14:16:25 +0300, Sakari Ailus wrote:
> > Hi Niklas,
> >
> > On Wed, May 24, 2017 at 02:13:52AM +0200, Niklas Söderlund wr
e. I think you should call this
e.g. direction_flags or pad_direction_flags. It'd be clear that it's about
pad flags and only the direction matters.
> +{
> + struct fwnode_endpoint endpoint;
> + int i, ret;
media entity num_pads field is u16. I guess unsigned int is fine, but
iklas Söderlund <niklas.soderlund+rene...@ragnatech.se>
Thanks!
Acked-by: Sakari Ailus <sakari.ai...@linux.intel.com>
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
d. This is a recurring pattern, and making this change
avoids an additional check.
Having something along these lines in the patch description wouldn't hurt.
--
Regards,
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
On Wed, Jun 07, 2017 at 10:52:07AM +0100, Kieran Bingham wrote:
> From: Kieran Bingham <kieran.bingham+rene...@ideasonboard.com>
>
> Return NULL, if a null entity is parsed for it's v4l2_subdev
>
> Signed-off-by: Kieran Bingham <kieran.bingham+rene...@ideasonboard.com>
_ep.bus.mipi_csi2.data_lanes[i] : i;
> +
> + /* Check for valid lane number */
> + if (priv->lane_swap[i] < 1 || priv->lane_swap[i] > 4) {
Do you also support for three lanes? Based on the code elsewhere, it doesn't
seem like that.
> +
csi20vin7: endpoint@7 {
> + reg = <7>;
> + remote-endpoint = <>;
> + };
> + };
> + };
> + };
> +
> +/* Board propert
uld be quite clean, you'd remove one variable which is just serving as a
condition to restart the loop:
again:
list_for_each_entry_safe() {
ret = test_notify();
if (ret < 0)
return ret;
goto again;
}
Hejssan,
On Wed, May 24, 2017 at 04:07:36PM +0200, Niklas Söderlund wrote:
> Hi Sakari,
>
> Thanks for your feedback.
>
> On 2017-05-24 16:21:37 +0300, Sakari Ailus wrote:
> > Hi Niklas,
> >
> > On Wed, May 24, 2017 at 02:09:06AM +0200, Niklas Söderlund wr
of your branch. I
> see it is not there in media-tree master yet. Please let me know if there
> is a change in plan.
I've sent a pull request to Mauro here and my expectation is it'll reach
media tree master in not too distant future:
<URL:http://www.spinics.net/lists/linux-media/msg11570
ify().
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+rene...@ragnatech.se>
Acked-by: Sakari Ailus <sakari.ai...@linux.intel.com>
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
is function searches the entity for the first pad that matches
> + * the @direction.
> + *
> + * Return: return 0 on success.
> + */
> +int media_entity_pad_from_fwnode(struct media_entity *entity,
> + struct fwnode_handle *fwnode,
> + int direction, unsigned int *pad);
> +
> +/**
> * media_graph_walk_init - Allocate resources used by graph walk.
> *
> * @graph: Media graph structure that will be used to walk the graph
--
Regards,
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
ill, and a negative value on error. I
think we won't have more than INT_MAX pads. :-)
> int (*link_setup)(struct media_entity *entity,
> const struct media_pad *local,
> const struct media_pad *remote, u32 flags);
--
Regards,
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
On Tue, May 23, 2017 at 10:47:58PM +0100, Kieran Bingham wrote:
> On 23/05/17 22:43, Sakari Ailus wrote:
> > On Wed, May 24, 2017 at 12:40:19AM +0300, Sakari Ailus wrote:
> >>> * When all devices use endpoint matching, this code can be simplified,
> >>> and th
On Wed, May 24, 2017 at 12:40:19AM +0300, Sakari Ailus wrote:
> > * When all devices use endpoint matching, this code can be simplified, and
> > the
> > * parent comparisons can be removed.
Oh, and this I'm not so sure about --- we'll need to match lens controllers
Hi Kieran,
On Tue, May 23, 2017 at 06:40:08PM +0100, Kieran Bingham wrote:
> On 23/05/17 14:02, Sakari Ailus wrote:
> > Hi Kieran,
> >
> > On Mon, May 22, 2017 at 06:36:38PM +0100, Kieran Bingham wrote:
> >> From: Kieran Bingham <kieran.bingham+rene...@id
Hi Kieran,
On Tue, May 23, 2017 at 06:15:42PM +0100, Kieran Bingham wrote:
>
>
> On 23/05/17 13:58, Sakari Ailus wrote:
> > Hi Kieran,
> >
> > On Mon, May 22, 2017 at 06:36:37PM +0100, Kieran Bingham wrote:
> >> From: Kieran Bingham <kieran.bingham+rene
match_fwnode_of(sd_parent, asd_fwnode) ||
> +match_fwnode_of(sd->fwnode, asd_parent);
> }
>
> static bool match_custom(struct v4l2_subdev *sd, struct v4l2_async_subdev
> *asd)
Would this become easier to read if you handled all matching in what is
called
struct fwnode_handle *fwnode, struct fwnode_handle *prev);
> +struct fwnode_handle *fwnode_graph_get_port_parent(
> + struct fwnode_handle *fwnode);
> struct fwnode_handle *fwnode_graph_get_remote_port_parent(
> struct fwnode_handle *fwnode);
> struct fwnode_handle *fwnode_graph_get_remote_port(
--
Regards,
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
> fwnode_graph_get_port_parent(struct fwnode_handle *endpoint)
> > * Returns firmware node of the device the @endpoint belongs to.
> >
> >
> > I guess those could be changed too ...
>
> My point is that the kerneldoc format documents return values with a
> "Return:"
> tag. The documentation for the function can still provide extra information.
Yeah, let's do this right and then fix the rest. I've already submitted the
pull request on this one --- the origin of that text is actually the V4L2 OF
framework. Feel free to submit a patch that fixes it, I can do it as well.
--
Regards,
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
ent(endpoint),
graph_get_port_parent);
Or something like that.
--
Regards,
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
Hi Laurent,
On Fri, May 19, 2017 at 12:59:12AM +0300, Laurent Pinchart wrote:
> Hi Sakari,
>
> On Friday 19 May 2017 00:05:17 Sakari Ailus wrote:
> > On Thu, May 18, 2017 at 11:54:46PM +0300, Laurent Pinchart wrote:
> > > On Thursday 18 May 2017 23:50:34 Sakari Ailus wrot
be encountered at
some point in driver initialisation.
Anyway, I don't think this situation should be a lasting one, and that
endpoint matching is the way to go.
>
> I have posted an implementation of this as:
> [PATCH v1 3/3] v4l: async: Match parent devices [0]
>
> I believe this to be correct - but I'm aware that I'm only really considering
> the OF side here... Please let me know if there's something I'm not taking
> into
> account.
--
Regards,
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
Hi Laurent,
On Thu, May 18, 2017 at 11:54:46PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
>
> On Thursday 18 May 2017 23:50:34 Sakari Ailus wrote:
> > On Thu, May 18, 2017 at 07:08:00PM +0300, Laurent Pinchart wrote:
> > > On Wednesday 17 May 2017 22:20:57 Sakari Ailus w
Hi Laurent,
On Thu, May 18, 2017 at 07:08:00PM +0300, Laurent Pinchart wrote:
> Hi Sakari,
>
> On Wednesday 17 May 2017 22:20:57 Sakari Ailus wrote:
> > On Wed, May 17, 2017 at 04:38:14PM +0100, Kieran Bingham wrote:
> > > From: Kieran Bingham <kieran.bingh
t.
<URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=acpi-graph-cleaned>
This function becomes quite simple, see
fwnode_graph_get_remote_port_parent().
--
Kind regards,
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
there's little use for supporting for const and
non-const arguments presumably. A simple static inline function should do.
--
Regards,
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
tic const struct v4l2_dv_timings_cap adv748x_hdmi_timings_cap = {
> >>>> +.type = V4L2_DV_BT_656_1120,
> >>>> +/* keep this initialization for compatibility with GCC < 4.4.6
> >>>> */
> >>>> +.reserved = { 0 },
> >>>
> >>> Is this necessary? Aren't the uninitialised fields zeroed if any field in
> >>> a
> >>> struct is initialised, also on those versions of GCC? (Of course there
> >>> have
> >>> been bugs, but nothing rings a bell.)
> >>
> >> Looks like it is necessary:
> >> https://patchwork.kernel.org/patch/2851851/
> >
> > This is related to the anonymous union. What I'm saying appears unrelated
> > --- if any struct field is initialised, then the rest of the fields of the
> > struct are initialised to zero by the compiler (unless explicitly
> > initialised to something else).
> >
> > I.e. you can drop ".reserved = { 0 }," line.
>
> In that patch I referenced, (several of the drivers were all updated in a
> similar fashion) Gianluca mentioned the following:
>
> >
> > Please note that I have also to init the reserved space as otherwise GCC
> > fails with this error:
> >
> > CC [M] adv7842.o
> > adv7842.c:549: error: field name not in record or union initializer
> > adv7842.c:549: error: (near initialization for
> > 'adv7842_timings_cap_analog.reserved')
> > adv7842.c:549: warning: braces around scalar initializer
> > adv7842.c:549: warning: (near initialization for
> > 'adv7842_timings_cap_analog.reserved[0]')
I had to test that here. :-) Ouch. And I'm left wondering how does the
initialisation of the reserved field get things working again...
Anyway, please ignore the original comment.
>
> Therefore my understanding was that it was necessary.
>
> If not - then we can remove from the following drivers too:
>
> git grep ".reserved = { 0 }"
> drivers/media/i2c/ad9389b.c:.reserved = { 0 },
> drivers/media/i2c/adv7511.c:.reserved = { 0 },
> drivers/media/i2c/adv7604.c:.reserved = { 0 },
> drivers/media/i2c/adv7604.c:.reserved = { 0 },
> drivers/media/i2c/adv7842.c:.reserved = { 0 },
> drivers/media/i2c/adv7842.c:.reserved = { 0 },
> drivers/media/i2c/tc358743.c: .reserved = { 0 },
> drivers/media/i2c/ths8200.c:.reserved = { 0 },
> drivers/media/platform/vivid/vivid-vid-common.c:.reserved = { 0 },
> drivers/media/spi/gs1662.c: .reserved = { 0 },
> samples/v4l/v4l2-pci-skeleton.c:.reserved = { 0 },
>
> But maybe we should test with an older compiler to verify this...
>
> Is there a cut-off point where we would stop supporting older compilers?
I guess when people stop submitting patches to support them? :-)
Documentation/admin-guide/README.rst says that at least GCC 3.2 should be
used. That's very, very old. I wonder if anyone has recently used such an
old version...
--
Regards,
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
Hi Kieran,
On Mon, May 15, 2017 at 01:32:41PM +0100, Kieran Bingham wrote:
> Hi Sakari
>
> On 12/05/17 17:46, Sakari Ailus wrote:
> > Hi Kieran,
> >
> > Thanks for the patches!
>
> Thankyou for the review!
You're welcome! :-)
>
> > Would you ha
b/arch/arm/mm/dma-mapping.c
> > index ab4f745..a40f03e 100644
> > --- a/arch/arm/mm/dma-mapping.c
> > +++ b/arch/arm/mm/dma-mapping.c
> > @@ -2358,6 +2358,7 @@ static void arm_teardown_iommu_dma_ops(struct device
> *dev)
> > __arm_iommu_detach_device(dev);
> > arm_iommu_release_mapping(mapping);
> > + set_dma_ops(dev, NULL);
> > }
> > #else
>
> The subject mentions arch_teardown_dma_ops(), which I think is correct, but
> the patch adds the set_dma_ops() call to arm_teardown_iommu_dma_ops().
>
> However, the situation is perhaps more complex. Note the check at the
> beginning of arch_setup_dma_ops():
>
> /*
>* Don't override the dma_ops if they have already been set. Ideally
>* this should be the only location where dma_ops are set, remove this
>* check when all other callers of set_dma_ops will have disappeared.
>*/
> if (dev->dma_ops)
> return;
>
> If you set the dma_ops to NULL in arm_teardown_iommu_dma_ops() or
> arch_teardown_dma_ops(), the next call to arch_setup_dma_ops() will override
> them. To be safe you should only set them to NULL if they have been set by
> arch_setup_dma_ops(). More than that, arch_teardown_dma_ops() should probably
> not call arm_teardown_iommu_dma_ops() at all if the dma_ops were set by
> arm_iommu_attach_device() and not arch_teardown_dma_ops(). One option would
> be
> to add a field to struct dev_archdata to store that information. To avoid
> growing the structure, which is embedded in every struct device, you could
> possibly turn the dma_coherent bool into a bitfield.
>
> @@ -19,7 +19,8 @@ struct dev_archdata {
> #ifdef CONFIG_XEN
> const struct dma_map_ops *dev_dma_ops;
> #endif
> - bool dma_coherent;
> + bool dma_coherent:1;
> + bool dma_ops_setup:1;
> };
>
> struct omap_device;
>
> I haven't checked, however, whether the dma_coherent field would need to be
> accessed atomically, so this might be a bad idea.
A bool bit field? :-)
I think I'd just use bool for both. I wouldn't expect dma_coherent change
once it has been set before device driver probe though.
If you like a bit field, then I'd propose making it unsigned int.
--
Regards,
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
Hi Kieran,
Thanks for the patches!
Would you have a media-ctl -p && media-ctl --print-dot (or the PS file) to
see how this looks like in practice?
Please see my comments below.
On Thu, May 11, 2017 at 06:21:20PM +0100, Kieran Bingham wrote:
> From: Kieran Bingham
e my local v4l2-acpi branch and what I have in
my git.linuxtv.org tree. :-) So the change is there, embedded in the same
patch that converts the users.
I'll upload it later tonight.
--
Regards,
Sakari Ailus
e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
r patches which will reach media-tree master in next rc1,
for now I've merged them here:
<URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=v4l2-acpi-merge>
I'll send a pull request for media-tree once we have 4.12rc1 in media-tree
master.
Thanks.
--
Kind regards,
Sakari Ai
Hi Ramesh,
On Tue, May 09, 2017 at 02:37:34PM +0100, Ramesh Shanmugasundaram wrote:
> This patch adds driver support for the MAX2175 chip. This is Maxim
> Integrated's RF to Bits tuner front end chip designed for software-defined
> radio solutions. This driver exposes the tuner as a sub-device
remote-endpoint = <_rx_device>;
> + };
> + };
> +
> +};
> diff --git a/Documentation/devicetree/bindings/property-units.txt
> b/Documentation/devicetree/bindings/property-units.txt
> index 0849618a9df0..2d1d28843c96 100644
> --- a/Do
1 - 100 of 143 matches
Mail list logo
