From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752641AbdLMK1Q (ORCPT ); Wed, 13 Dec 2017 05:27:16 -0500 Received: from mail-pf0-f181.google.com ([209.85.192.181]:36407 "EHLO mail-pf0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752612AbdLMK1L (ORCPT ); Wed, 13 Dec 2017 05:27:11 -0500 X-Google-Smtp-Source: ACJfBoswqKuXNdDcdqhb3pLY5U3mc0MTbjcpJmea0CoDYN6c/eaS0EqPXMIrmZdqNb4f4Mx08AFeWQ== Date: Wed, 13 Dec 2017 15:57:07 +0530 From: Viresh Kumar To: Greg Kroah-Hartman Cc: Vincent Guittot , Stephen Boyd , Rajendra Nayak , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, robdclark@gmail.com, s.hauer@pengutronix.de, l.stach@pengutronix.de, shawnguo@kernel.org, fabio.estevam@nxp.com, nm@ti.com, xuwei5@hisilicon.com, robh+dt@kernel.org Subject: Re: [PATCH V4 08/12] boot_constraint: Manage deferrable constraints Message-ID: <20171213102707.GD3322@vireshk-i7> References: <88c7a0a6421d267c118f501ea1e920b04649002d.1509284255.git.viresh.kumar@linaro.org> <20171213095345.GJ13194@kroah.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20171213095345.GJ13194@kroah.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 13-12-17, 10:53, Greg Kroah-Hartman wrote: > On Sun, Oct 29, 2017 at 07:18:56PM +0530, Viresh Kumar wrote: > > +static void add_deferrable_of_single(struct device_node *np, > > + struct dev_boot_constraint *constraints, > > + int count) > > +{ > > + struct device *dev; > > + int ret; > > + > > + if (!of_device_is_available(np)) > > + return; > > + > > + ret = of_platform_bus_create(np, NULL, NULL, NULL, false); > > + if (ret) > > + return; > > + > > + if (of_device_is_compatible(np, "arm,primecell")) { > > Why is "arm,primecell" in the core code here? All we need here is a struct device pointer to add constraints. But how we get the device node depends on what bus type the device corresponds to. Currently this only support amba and platform devices, but we may need to get spi, i2c, etc later on. How do you suggest to keep this stuff out of core here ? Are you asking me to add a generic API in the OF core to find the struct device pointer using a node pointer ? > > + struct amba_device *adev = of_find_amba_device_by_node(np); > > + > > + if (!adev) { > > + pr_err("Failed to find amba dev: %s\n", np->full_name); > > Never use pr_* when you have a valid struct device to use. Sure. I agree. > Don't you > have one from the struct device_node * passed in here? The struct device_node doesn't contain a struct device * unfortunately. Will it be acceptable to add one ? That will solve some controversial part of this function for sure :) -- viresh