From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934474AbcIFMqg (ORCPT ); Tue, 6 Sep 2016 08:46:36 -0400 Received: from mout.kundenserver.de ([212.227.126.134]:49184 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932406AbcIFMqe (ORCPT ); Tue, 6 Sep 2016 08:46:34 -0400 From: Arnd Bergmann To: Ulf Hansson Cc: Yangbo Lu , linux-mmc , Scott Wood , "linuxppc-dev@lists.ozlabs.org" , "devicetree@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , linux-clk , "linux-i2c@vger.kernel.org" , iommu@lists.linux-foundation.org, netdev@vger.kernel.org, Mark Rutland , Rob Herring , Russell King , Jochen Friedrich , Joerg Roedel , Claudiu Manoil , Bhupesh Sharma , Qiang Zhao , Kumar Gala , Santosh Shilimkar , Yang-Leo Li , Xiaobo Xie Subject: Re: [v11, 7/8] base: soc: introduce soc_device_match() interface Date: Tue, 06 Sep 2016 14:46:08 +0200 Message-ID: <6515108.aCEls7dUuW@wuerfel> User-Agent: KMail/5.1.3 (Linux/4.4.0-34-generic; KDE/5.18.0; x86_64; ; ) In-Reply-To: References: <1473150503-9550-1-git-send-email-yangbo.lu@nxp.com> <1473150503-9550-8-git-send-email-yangbo.lu@nxp.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:EyxaNc93WpreRupHWCux6/cs89oW235gkyGhvtlh0Ys70QnGfms EhAA0nJfbpJssA1wOXByI4EuYAMeZk0HYs5mjphWt8HU1xDScn8OQgJQ5HiGP3LK06AIaGl wEC9jCMMRSxpZ3fwjyQIYVG4GD+U+AnVbLhO5WceD4bfdyhXZOtbtnQNF4NlSSgAS1ixxw3 HXTpNs1YfloBSntiHayJw== X-UI-Out-Filterresults: notjunk:1;V01:K0:U+UaGsDX2IY=:u3sZxe8ogWR/kxeIHxaQv9 zlRpnGbGiZrnuWn+yojombQugibPQYc0kZQsPJmUZHqFkBn85UNAgLmH3cvuM1QGIkDr0XerN vueZg6SHpiI6W5eqfx7oSkiomOyMcZABhv+CSk7fFtHqsRZvgrfa6WeMjz1b1DYkEfC3F7lTA wShC18FCVoE6yArv1nsAaC5BU7d8ui1n7bvnrbUS2vYMl10eH/4FUxsn8CK4wt1ONLQBV3spH mFk18ZI1cWB+Vqs1XGpn/Wg6OtHvJGG3Np46V3cnqs8C6Ctk46uGbYjk4kAPvFjmdgqWq606W 2nq7RMvsksrGmH1wCCcvgtFKKpPllyaWChts/XZrjt/DJbYbPZmhcRqNkxZzJuEz8/biYe/nl umYZqFyVXbPTv5bVxHQ8FE1Oa5mPsDsK5/a0xjxr+Ua+WmBrZrvBH648AwPLX7HbAJpmE/W2N tF5ZQC4qGr3IHIvbMLPUBhf8WDq/Ramzz/2JyGINGmcwcZv73kNsdLLuF+Ecx+nCoTJ7voW7F flXtWguO2qh0cwkA1jgkwW336TbiaSEyOcWhA+qyChcOtkL3lm5lur3teFVZusb54kmlMpUi3 zlClvt/ueWQ1v7TYBLJyXCIbivZzC8CiOqO3ez7qdtHN1Bt2XAy7tLAcauCRv8McCb5U327dN ebuyIyeYxufdNr+N9CAqwb/gv5IfInMJJ4GBFrSx7DQiT4hRR1/5Mlr9edwn/I1NMFzbSVoJf uJWkYwRwrYv9Dc1c Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tuesday, September 6, 2016 1:44:23 PM CEST Ulf Hansson wrote: > On 6 September 2016 at 10:28, Yangbo Lu wrote: > > We keep running into cases where device drivers want to know the exact > > version of the a SoC they are currently running on. In the past, this has > > usually been done through a vendor specific API that can be called by a > > driver, or by directly accessing some kind of version register that is > > not part of the device itself but that belongs to a global register area > > of the chip. Please add "From: Arnd Bergmann " as the first line, to preserve authorship. If you use "git send-email" or "git format-patch", that should happen automatically if the author field is set right (if not, use 'git commit --amend --author="Arnd Bergmann "' to fix it). > > + > > +/* > > + * soc_device_match - identify the SoC in the machine > > + * @matches: zero-terminated array of possible matches > > Perhaps also express the constraint on the matching entries. As you > need at least one of the ->machine(), ->family(), ->revision() or > ->soc_id() callbacks implemented, right!? They are not callbacks, just strings. Having an empty entry indicates the end of the array, and this is not called. > > + * > > + * returns the first matching entry of the argument array, or NULL > > + * if none of them match. > > + * > > + * This function is meant as a helper in place of of_match_node() > > + * in cases where either no device tree is available or the information > > + * in a device node is insufficient to identify a particular variant > > + * by its compatible strings or other properties. For new devices, > > + * the DT binding should always provide unique compatible strings > > + * that allow the use of of_match_node() instead. > > + * > > + * The calling function can use the .data entry of the > > + * soc_device_attribute to pass a structure or function pointer for > > + * each entry. > > I don't get the use case behind this, could you elaborate? > > Perhaps we should postpone adding the .data entry until we actually > see a need for it? I think the interface is rather useless without a way to figure out which entry you got. Almost all users of of_match_node() actually use the returned ->data field, and I expect this to be the same here. > > + */ > > +const struct soc_device_attribute *soc_device_match( > > + const struct soc_device_attribute *matches) > > +{ > > + struct device *dev; > > + int ret; > > + > > + for (ret = 0; ret == 0; matches++) { > > This loop looks a bit weird and unsafe. Ah, and I thought I was being clever ;-) > 1) Perhaps using a while loop makes this more readable? > 2) As this is an exported API, I guess validation of the ->matches > pointer needs to be done before accessing it. Sounds fine. > > + if (!(matches->machine || matches->family || > > + matches->revision || matches->soc_id)) > > + return NULL; > > + dev = NULL; > > There's no need to use a struct device just to assign it to NULL. > Instead just provide the function below with NULL. > > > + ret = bus_for_each_dev(&soc_bus_type, dev, (void *)matches, > > + soc_device_match_one); I don't remember what led to this, I think you are right, we should just pass NULL as most other callers. Thanks for the review. ARnd