From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2D4CCC433DB for ; Mon, 25 Jan 2021 16:52:59 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id DDE7B22795 for ; Mon, 25 Jan 2021 16:52:58 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728431AbhAYQww (ORCPT ); Mon, 25 Jan 2021 11:52:52 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47328 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727834AbhAYQvo (ORCPT ); Mon, 25 Jan 2021 11:51:44 -0500 Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2A99BC06178A for ; Mon, 25 Jan 2021 08:50:37 -0800 (PST) Received: from [IPv6:2804:214:81d7:a6cc:f83c:66c3:4225:e59d] (unknown [IPv6:2804:214:81d7:a6cc:f83c:66c3:4225:e59d]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: koike) by bhuna.collabora.co.uk (Postfix) with ESMTPSA id B61251F45362; Mon, 25 Jan 2021 16:50:30 +0000 (GMT) Subject: Re: [PATCH v3 08/14] media: renesas-ceu: Use v4l2_async_notifier_add_*_subdev To: Sakari Ailus , linux-media@vger.kernel.org Cc: Hans Verkuil , kernel@collabora.com, Laurent Pinchart , Kieran Bingham , Jacopo Mondi , niklas.soderlund+renesas@ragnatech.se, Dafna Hirschfeld , Hugues Fruchet , Nicolas Ferre , Yong Zhi , Sylwester Nawrocki , Maxime Ripard , Robert Foss , Philipp Zabel , Ezequiel Garcia References: <20210125132230.6600-1-sakari.ailus@linux.intel.com> <20210125132230.6600-23-sakari.ailus@linux.intel.com> From: Helen Koike Message-ID: Date: Mon, 25 Jan 2021 13:50:25 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.1 MIME-Version: 1.0 In-Reply-To: <20210125132230.6600-23-sakari.ailus@linux.intel.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org On 1/25/21 10:22 AM, Sakari Ailus wrote: > From: Ezequiel Garcia > > The use of v4l2_async_notifier_add_subdev will be discouraged. > Drivers are instead encouraged to use a helper such as > v4l2_async_notifier_add_i2c_subdev. > > This fixes a misuse of the API, as v4l2_async_notifier_add_subdev > should get a kmalloc'ed struct v4l2_async_subdev, > removing some boilerplate code while at it. > > Use the appropriate helper: v4l2_async_notifier_add_i2c_subdev > or v4l2_async_notifier_add_fwnode_remote_subdev, which handles > the needed setup, instead of open-coding it. > > Using v4l2-async to allocate the driver-specific structs, > requires to change struct ceu_subdev so the embedded > struct v4l2_async_subdev is now the first element. > > Signed-off-by: Ezequiel Garcia > Reviewed-by: Jacopo Mondi > Signed-off-by: Sakari Ailus > --- > drivers/media/platform/renesas-ceu.c | 60 +++++++++++++--------------- > 1 file changed, 27 insertions(+), 33 deletions(-) > > diff --git a/drivers/media/platform/renesas-ceu.c b/drivers/media/platform/renesas-ceu.c > index 4a633ad0e8fa..0298d08b39e4 100644 > --- a/drivers/media/platform/renesas-ceu.c > +++ b/drivers/media/platform/renesas-ceu.c > @@ -152,8 +152,8 @@ static inline struct ceu_buffer *vb2_to_ceu(struct vb2_v4l2_buffer *vbuf) > * ceu_subdev - Wraps v4l2 sub-device and provides async subdevice. > */ > struct ceu_subdev { > - struct v4l2_subdev *v4l2_sd; > struct v4l2_async_subdev asd; > + struct v4l2_subdev *v4l2_sd; > > /* per-subdevice mbus configuration options */ > unsigned int mbus_flags; > @@ -174,7 +174,7 @@ struct ceu_device { > struct v4l2_device v4l2_dev; > > /* subdevices descriptors */ > - struct ceu_subdev *subdevs; > + struct ceu_subdev **subdevs; > /* the subdevice currently in use */ > struct ceu_subdev *sd; > unsigned int sd_index; > @@ -1195,7 +1195,7 @@ static int ceu_enum_input(struct file *file, void *priv, > if (inp->index >= ceudev->num_sd) > return -EINVAL; > > - ceusd = &ceudev->subdevs[inp->index]; > + ceusd = ceudev->subdevs[inp->index]; > > inp->type = V4L2_INPUT_TYPE_CAMERA; > inp->std = 0; > @@ -1230,7 +1230,7 @@ static int ceu_s_input(struct file *file, void *priv, unsigned int i) > return 0; > > ceu_sd_old = ceudev->sd; > - ceudev->sd = &ceudev->subdevs[i]; > + ceudev->sd = ceudev->subdevs[i]; > > /* > * Make sure we can generate output image formats and apply > @@ -1423,7 +1423,7 @@ static int ceu_notify_complete(struct v4l2_async_notifier *notifier) > * ceu formats. > */ > if (!ceudev->sd) { > - ceudev->sd = &ceudev->subdevs[0]; > + ceudev->sd = ceudev->subdevs[0]; > ceudev->sd_index = 0; > } > > @@ -1467,8 +1467,8 @@ static const struct v4l2_async_notifier_operations ceu_notify_ops = { > > /* > * ceu_init_async_subdevs() - Initialize CEU subdevices and async_subdevs in > - * ceu device. Both DT and platform data parsing use > - * this routine. > + * ceu device. Both DT and platform data parsing use > + * this routine. Maybe doc alignment fix should be sent in another patch. With or without this: Reviewed-by: Helen Koike Regards, Helen > * > * Returns 0 for success, -ENOMEM for failure. > */ > @@ -1495,6 +1495,7 @@ static int ceu_parse_platform_data(struct ceu_device *ceudev, > const struct ceu_platform_data *pdata) > { > const struct ceu_async_subdev *async_sd; > + struct v4l2_async_subdev *asd; > struct ceu_subdev *ceu_sd; > unsigned int i; > int ret; > @@ -1510,21 +1511,17 @@ static int ceu_parse_platform_data(struct ceu_device *ceudev, > > /* Setup the ceu subdevice and the async subdevice. */ > async_sd = &pdata->subdevs[i]; > - ceu_sd = &ceudev->subdevs[i]; > - > - INIT_LIST_HEAD(&ceu_sd->asd.list); > - > - ceu_sd->mbus_flags = async_sd->flags; > - ceu_sd->asd.match_type = V4L2_ASYNC_MATCH_I2C; > - ceu_sd->asd.match.i2c.adapter_id = async_sd->i2c_adapter_id; > - ceu_sd->asd.match.i2c.address = async_sd->i2c_address; > - > - ret = v4l2_async_notifier_add_subdev(&ceudev->notifier, > - &ceu_sd->asd); > - if (ret) { > + asd = v4l2_async_notifier_add_i2c_subdev(&ceudev->notifier, > + async_sd->i2c_adapter_id, > + async_sd->i2c_address, > + sizeof(*ceu_sd)); > + if (IS_ERR(asd)) { > v4l2_async_notifier_cleanup(&ceudev->notifier); > - return ret; > + return PTR_ERR(asd); > } > + ceu_sd = to_ceu_subdev(asd); > + ceu_sd->mbus_flags = async_sd->flags; > + ceudev->subdevs[i] = ceu_sd; > } > > return pdata->num_subdevs; > @@ -1536,7 +1533,8 @@ static int ceu_parse_platform_data(struct ceu_device *ceudev, > static int ceu_parse_dt(struct ceu_device *ceudev) > { > struct device_node *of = ceudev->dev->of_node; > - struct device_node *ep, *remote; > + struct device_node *ep; > + struct v4l2_async_subdev *asd; > struct ceu_subdev *ceu_sd; > unsigned int i; > int num_ep; > @@ -1578,20 +1576,16 @@ static int ceu_parse_dt(struct ceu_device *ceudev) > } > > /* Setup the ceu subdevice and the async subdevice. */ > - ceu_sd = &ceudev->subdevs[i]; > - INIT_LIST_HEAD(&ceu_sd->asd.list); > - > - remote = of_graph_get_remote_port_parent(ep); > - ceu_sd->mbus_flags = fw_ep.bus.parallel.flags; > - ceu_sd->asd.match_type = V4L2_ASYNC_MATCH_FWNODE; > - ceu_sd->asd.match.fwnode = of_fwnode_handle(remote); > - > - ret = v4l2_async_notifier_add_subdev(&ceudev->notifier, > - &ceu_sd->asd); > - if (ret) { > - of_node_put(remote); > + asd = v4l2_async_notifier_add_fwnode_remote_subdev( > + &ceudev->notifier, of_fwnode_handle(ep), > + sizeof(*ceu_sd)); > + if (IS_ERR(asd)) { > + ret = PTR_ERR(asd); > goto error_cleanup; > } > + ceu_sd = to_ceu_subdev(asd); > + ceu_sd->mbus_flags = fw_ep.bus.parallel.flags; > + ceudev->subdevs[i] = ceu_sd; > > of_node_put(ep); > } >