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=-11.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable 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 E644FC433E7 for ; Fri, 16 Oct 2020 15:16:43 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 6C39620897 for ; Fri, 16 Oct 2020 15:16:43 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="hGVWs7qm"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=cirrus.com header.i=@cirrus.com header.b="lmVCs/iv" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6C39620897 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=opensource.cirrus.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Type: Content-Transfer-Encoding:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=0hPsFP3g2IXr36/WTPqqbc/xaBPqeY7RgTbP5Ni6yBE=; b=hGVWs7qmnILD5/XXot7IsdwIp z+7E8802+SYL9r/92BBINNSMmwPtme4e+yIwWcMAP/Gpj4sVFzKUGguW6RViUo5OibayPBFFDX2Gc mhEivm1PfX29g2/PTHB4EwRpe7Bg5sAwBD2nZZGJRmMUUC2rXDpJLUDykah7Z3lENHzGM4kwZUcqf eYbG6lQYDZ9BIi4RqIVT+3nO3ZEI3a6X217ggmKcnQPAzSr3Xr1Agqa31nY8pqKiT3Zg8QNCMFUPx FozmT8ol9UTlOQTf9sZ3q2K9Rnrk8utTOpcJ1wvKdZy/Rd9zwK+qdGJ4ZV8kIrgh81zEEAhZNlF3/ CnrJYG6Fg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kTRRd-0004vy-4t; Fri, 16 Oct 2020 15:15:01 +0000 Received: from mx0b-001ae601.pphosted.com ([67.231.152.168]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kTRRZ-0004ut-6R; Fri, 16 Oct 2020 15:14:58 +0000 Received: from pps.filterd (m0077474.ppops.net [127.0.0.1]) by mx0b-001ae601.pphosted.com (8.16.0.43/8.16.0.43) with SMTP id 09GFAilX018346; Fri, 16 Oct 2020 10:14:46 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cirrus.com; h=subject : to : cc : references : from : message-id : date : mime-version : in-reply-to : content-type : content-transfer-encoding; s=PODMain02222019; bh=xqJa7ke2bej+Y2/yTZCs2JgbiMjO2DRRIlRYVcz8XVI=; b=lmVCs/iv5yXyzLP36G1sZy/Gmdxojo/mafF7AdQAH6vCS2wF8tUbMEhyiJuUS8grLEN2 DHZrIY5kwqkVYp01pPgpiq32BIsMwK/PVVfC3uUIfW52MjYKgQebYEr134yHOtt+TVGU VHntl0Zu7aBONdahAsWTWT7BUr0zI8e6TjtpdZ0lFu4JjiirsAol58ysuqF4DLW5oJtE G9ZXqxsNJagA6vek7sMd1znVWR3Si19q8P9DpTxASchg6UJISQBZMAbW2Ur74uTL9rmY 7e4vUt3oOMWpRW2IKnuX0vVU+/T5OfHw8s7dJ8A6dcOAoRQM88KR+3/3QjTz+lGQvdml uw== Received: from ediex01.ad.cirrus.com ([87.246.76.36]) by mx0b-001ae601.pphosted.com with ESMTP id 3439cngaya-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NOT); Fri, 16 Oct 2020 10:14:46 -0500 Received: from EDIEX01.ad.cirrus.com (198.61.84.80) by EDIEX01.ad.cirrus.com (198.61.84.80) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1913.5; Fri, 16 Oct 2020 16:14:45 +0100 Received: from ediswmail.ad.cirrus.com (198.61.86.93) by EDIEX01.ad.cirrus.com (198.61.84.80) with Microsoft SMTP Server id 15.1.1913.5 via Frontend Transport; Fri, 16 Oct 2020 16:14:45 +0100 Received: from [10.0.2.15] (ausnpc0lsnw1.ad.cirrus.com [198.61.64.143]) by ediswmail.ad.cirrus.com (Postfix) with ESMTP id A8A4345; Fri, 16 Oct 2020 15:14:44 +0000 (UTC) Subject: Re: [PATCH 1/7] of: base: Add of_count_phandle_with_fixed_args() To: Rob Herring , Robin Murphy References: <20201014145418.31838-1-rf@opensource.cirrus.com> <20201014145418.31838-2-rf@opensource.cirrus.com> <90600a67-25e4-7933-35c3-f515deaee94f@arm.com> From: Richard Fitzgerald Message-ID: <474edf9d-e15a-cc20-1b56-2fe1d7fccf55@opensource.cirrus.com> Date: Fri, 16 Oct 2020 16:14:44 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.3.1 MIME-Version: 1.0 In-Reply-To: Content-Language: en-US X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxscore=0 bulkscore=0 priorityscore=1501 malwarescore=0 adultscore=0 spamscore=0 impostorscore=0 mlxlogscore=999 lowpriorityscore=0 phishscore=0 clxscore=1015 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2010160116 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201016_111457_692606_9ED1E3FC X-CRM114-Status: GOOD ( 25.86 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: devicetree@vger.kernel.org, Linux-ALSA , - , "linux-kernel@vger.kernel.org" , Mark Brown , linux-arm-kernel , Nicolas Saenz Julienne , "moderated list:BROADCOM BCM2835 ARM ARCHITECTURE" Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 16/10/2020 14:31, Rob Herring wrote: > On Thu, Oct 15, 2020 at 11:52 AM Robin Murphy wrote: >> >> On 2020-10-14 19:39, Rob Herring wrote: >>> On Wed, Oct 14, 2020 at 9:54 AM Richard Fitzgerald >>> wrote: >>>> >>>> Add an equivalent of of_count_phandle_with_args() for fixed argument >>>> sets, to pair with of_parse_phandle_with_fixed_args(). >>>> >>>> Signed-off-by: Richard Fitzgerald >>>> --- >>>> drivers/of/base.c | 42 ++++++++++++++++++++++++++++++++++++++++++ >>>> include/linux/of.h | 9 +++++++++ >>>> 2 files changed, 51 insertions(+) >>>> >>>> diff --git a/drivers/of/base.c b/drivers/of/base.c >>>> index ea44fea99813..45d8b0e65345 100644 >>>> --- a/drivers/of/base.c >>>> +++ b/drivers/of/base.c >>>> @@ -1772,6 +1772,48 @@ int of_count_phandle_with_args(const struct device_node *np, const char *list_na >>>> } >>>> EXPORT_SYMBOL(of_count_phandle_with_args); >>>> >>>> +/** >>>> + * of_count_phandle_with_fixed_args() - Find the number of phandles references in a property >>>> + * @np: pointer to a device tree node containing a list >>>> + * @list_name: property name that contains a list >>>> + * @cell_count: number of argument cells following the phandle >>>> + * >>>> + * Returns the number of phandle + argument tuples within a property. It >>>> + * is a typical pattern to encode a list of phandle and variable >>>> + * arguments into a single property. >>>> + */ >>>> +int of_count_phandle_with_fixed_args(const struct device_node *np, >>>> + const char *list_name, >>>> + int cells_count) >>>> +{ >>> >>> Looks to me like you can refactor of_count_phandle_with_args to handle >>> both case and then make this and of_count_phandle_with_args simple >>> wrapper functions. >> >> Although for just counting the number of phandles each with n arguments >> that a property contains, isn't that simply a case of dividing the >> property length by n + 1? The phandles themselves will be validated by >> any subsequent of_parse_phandle*() call anyway, so there doesn't seem >> much point in doing more work then necessary here. >> >>>> + struct of_phandle_iterator it; >>>> + int rc, cur_index = 0; >>>> + >>>> + if (!cells_count) { >>>> + const __be32 *list; >>>> + int size; >>>> + >>>> + list = of_get_property(np, list_name, &size); >>>> + if (!list) >>>> + return -ENOENT; >>>> + >>>> + return size / sizeof(*list); >> >> Case in point - if it's OK to do exactly that for n == 0, then clearly >> we're *aren't* fussed about validating anything, so the n > 0 code below >> is nothing more than a massively expensive way to check for a nonzero >> remainder :/ > > Indeed. We should just generalize this. It can still be refactored to > shared code. > > It's probably worthwhile to check for a remainder here IMO. > Ok, I looked at the implementation of of_phandle_iterator_next() and it is in fact simply incrementing by 'count' 32-bit words. So as Robin said the count_phandle_with_x_args()functions could simply divide the length by count+1. However, may I suggest that should be done in a separate patch after my patch to add count_phandle_with_fixed_args()? That way, if replacing the iteration with the simple length divide causes any unforeseen problems the patch can just be reverted. > Rob > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel