linux-kernel - Re: [PATCH 3/4] remoteproc: Add a sysfs interface for firmware and state

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20161014051417.GD8247@tuxbot>
Date:   Thu, 13 Oct 2016 22:14:17 -0700
From:   Bjorn Andersson <bjorn.andersson@...aro.org>
To:     loic pallardy <loic.pallardy@...com>
Cc:     Matt Redfearn <matt.redfearn@...tec.com>,
        Ohad Ben-Cohen <ohad@...ery.com>,
        linux-remoteproc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/4] remoteproc: Add a sysfs interface for firmware and
 state

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