Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754948AbcJNFOg (ORCPT ); Fri, 14 Oct 2016 01:14:36 -0400 Received: from mail-pa0-f45.google.com ([209.85.220.45]:35453 "EHLO mail-pa0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754070AbcJNFOc (ORCPT ); Fri, 14 Oct 2016 01:14:32 -0400 Date: Thu, 13 Oct 2016 22:14:17 -0700 From: Bjorn Andersson To: loic pallardy Cc: Matt Redfearn , Ohad Ben-Cohen , linux-remoteproc@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 3/4] remoteproc: Add a sysfs interface for firmware and state Message-ID: <20161014051417.GD8247@tuxbot> References: <1476193185-32107-1-git-send-email-matt.redfearn@imgtec.com> <1476193185-32107-4-git-send-email-matt.redfearn@imgtec.com> <45167a9c-80ee-2af1-2ec5-db1eb1eef883@st.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <45167a9c-80ee-2af1-2ec5-db1eb1eef883@st.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2019 Lines: 61 On Thu 13 Oct 07:39 PDT 2016, loic pallardy wrote: > > > On 10/13/2016 04:25 PM, Matt Redfearn wrote: > >Hi Loic, > > > > > >On 13/10/16 14:56, loic pallardy wrote: > >> > >> > >>On 10/11/2016 03:39 PM, Matt Redfearn wrote: [..] > >>>diff --git a/drivers/remoteproc/remoteproc_internal.h > >>>b/drivers/remoteproc/remoteproc_internal.h > >>>index 837faf2677a6..2beb86ddfacc 100644 > >>>--- a/drivers/remoteproc/remoteproc_internal.h > >>>+++ b/drivers/remoteproc/remoteproc_internal.h > >>>@@ -64,6 +64,11 @@ void rproc_create_debug_dir(struct rproc *rproc); > >>> void rproc_init_debugfs(void); > >>> void rproc_exit_debugfs(void); > >>> > >>>+/* from remoteproc_sysfs.c */ > >>>+extern struct class rproc_class; > >>struct class rproc_class should be static to remoteproc_sysfs.c file. > >>It will be better to create a new interface like rproc_register_sysfs > >>to associate rproc_class to rproc device. > > > >It would be nice if that were possible, but since the class has to > >associated with the devices created within remoteproc_core, as it stands > >we must have visibility of this symbol there, see above the change to > >drivers/remoteproc/remoteproc_core.c line 1455. > >The alternative would be a utility function in remoteproc_sysfs.c to > >associate the class with an rproc device, it depends what the preference > >would be. > > Yes it will be my preference. remoteproc_sysfs.c file to provide a new > service like: > void rproc_register_sysfs(struct rproc *rproc) { > rproc->dev.class = &rproc_class; > } > > and to call this function from rproc_alloc > Compare: rproc->dev.class = &rproc_class; to: rproc_register_sysfs(rproc); The meaning of the prior is obvious, the latter require the jump to a different file to follow. And we would only be trading one global variable for a global function. A viable alternative would be to keep a local pointer to the class in core.c, that we assign during the call to rproc_init_sysfs(). But I'm fine with the way it's written. Regards, Bjorn