From patchwork Wed Sep 13 09:24:31 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sakari Ailus X-Patchwork-Id: 9950821 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id CB5E06024A for ; Wed, 13 Sep 2017 09:24:38 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id BC255290A2 for ; Wed, 13 Sep 2017 09:24:38 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B0EAA290B6; Wed, 13 Sep 2017 09:24:38 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id EA827290A2 for ; Wed, 13 Sep 2017 09:24:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752075AbdIMJYf (ORCPT ); Wed, 13 Sep 2017 05:24:35 -0400 Received: from nblzone-211-213.nblnetworks.fi ([83.145.211.213]:47064 "EHLO hillosipuli.retiisi.org.uk" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751380AbdIMJYe (ORCPT ); Wed, 13 Sep 2017 05:24:34 -0400 Received: from valkosipuli.localdomain (valkosipuli.retiisi.org.uk [IPv6:2001:1bc8:1a6:d3d5::80:2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by hillosipuli.retiisi.org.uk (Postfix) with ESMTPS id ED0CE600D8; Wed, 13 Sep 2017 12:24:31 +0300 (EEST) Received: from sakke by valkosipuli.localdomain with local (Exim 4.89) (envelope-from ) id 1ds3uJ-00046C-FB; Wed, 13 Sep 2017 12:24:31 +0300 Date: Wed, 13 Sep 2017 12:24:31 +0300 From: Sakari Ailus To: Hans Verkuil Cc: Sakari Ailus , linux-media@vger.kernel.org, niklas.soderlund@ragnatech.se, maxime.ripard@free-electrons.com, robh@kernel.org, laurent.pinchart@ideasonboard.com, devicetree@vger.kernel.org, pavel@ucw.cz, sre@kernel.org Subject: Re: [PATCH v12 18/26] v4l: fwnode: Add a helper function for parsing generic references Message-ID: <20170913092430.cbdgerkhiuxakbxv@valkosipuli.retiisi.org.uk> References: <20170912134200.19556-1-sakari.ailus@linux.intel.com> <20170912134200.19556-19-sakari.ailus@linux.intel.com> <020b9c86-dd73-3516-4a0e-827db9680b55@xs4all.nl> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <020b9c86-dd73-3516-4a0e-827db9680b55@xs4all.nl> User-Agent: NeoMutt/20170113 (1.7.2) Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Hans, Thanks for the review! On Wed, Sep 13, 2017 at 09:27:34AM +0200, Hans Verkuil wrote: > On 09/12/2017 03:41 PM, Sakari Ailus wrote: > > Add function v4l2_fwnode_reference_count() for counting external > > ???? There is no function v4l2_fwnode_reference_count()?! It got removed during the revisions but the commit message was not changed accordingly, I do that now. > > > references and v4l2_fwnode_reference_parse() for parsing them as async > > sub-devices. > > > > This can be done on e.g. flash or lens async sub-devices that are not part > > of but are associated with a sensor. > > > > struct v4l2_async_notifier.max_subdevs field is added to contain the > > maximum number of sub-devices in a notifier to reflect the memory > > allocated for the subdevs array. > > You forgot to remove this outdated paragraph. Oops. Removed it now. > > > > > Signed-off-by: Sakari Ailus > > --- > > drivers/media/v4l2-core/v4l2-fwnode.c | 69 +++++++++++++++++++++++++++++++++++ > > 1 file changed, 69 insertions(+) > > > > diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c > > index 44ee35f6aad5..a32473f95be1 100644 > > --- a/drivers/media/v4l2-core/v4l2-fwnode.c > > +++ b/drivers/media/v4l2-core/v4l2-fwnode.c > > @@ -498,6 +498,75 @@ int v4l2_async_notifier_parse_fwnode_endpoints_by_port( > > } > > EXPORT_SYMBOL_GPL(v4l2_async_notifier_parse_fwnode_endpoints_by_port); > > > > +/* > > + * v4l2_fwnode_reference_parse - parse references for async sub-devices > > + * @dev: the device node the properties of which are parsed for references > > + * @notifier: the async notifier where the async subdevs will be added > > + * @prop: the name of the property > > + * > > + * Return: 0 on success > > + * -ENOENT if no entries were found > > + * -ENOMEM if memory allocation failed > > + * -EINVAL if property parsing failed > > + */ > > +static int v4l2_fwnode_reference_parse( > > + struct device *dev, struct v4l2_async_notifier *notifier, > > + const char *prop) > > +{ > > + struct fwnode_reference_args args; > > + unsigned int index; > > + int ret; > > + > > + for (index = 0; > > + !(ret = fwnode_property_get_reference_args( > > + dev_fwnode(dev), prop, NULL, 0, index, &args)); > > + index++) > > + fwnode_handle_put(args.fwnode); > > + > > + if (!index) > > + return -ENOENT; > > + > > + /* > > + * To-do: handle -ENODATA when "device property: Align return > > + * codes of acpi_fwnode_get_reference_with_args" is merged. > > + */ > > + if (ret != -ENOENT && ret != -ENODATA) > > So while that patch referenced in the To-do above is not merged yet, > what does fwnode_property_get_reference_args return? ENOENT or ENODATA? > Or ENOENT now and ENODATA later? Or vice versa? > > I can't tell based on that information whether this code is correct or not. > > The comment needs to explain this a bit better. I'll add this: diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c index a32473f95be1..74fcc3ba9ebd 100644 --- a/drivers/media/v4l2-core/v4l2-fwnode.c +++ b/drivers/media/v4l2-core/v4l2-fwnode.c @@ -529,6 +529,9 @@ static int v4l2_fwnode_reference_parse( /* * To-do: handle -ENODATA when "device property: Align return * codes of acpi_fwnode_get_reference_with_args" is merged. + * Right now, both -ENODATA and -ENOENT signal the end of + * references where only a single error code should be used + * for the purpose. */ if (ret != -ENOENT && ret != -ENODATA) return ret;