linux-kernel - Re: [PATCH] base: soc: Handle custom soc information sysfs entries

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20191003182832.GA3579521@kroah.com>
Date:   Thu, 3 Oct 2019 20:28:32 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     mnalajal@...eaurora.org
Cc:     rafael@...nel.org, linux-arm-msm@...r.kernel.org,
        linux-kernel@...r.kernel.org, bjorn.andersson@...aro.org
Subject: Re: [PATCH] base: soc: Handle custom soc information sysfs entries

On Thu, Oct 03, 2019 at 11:17:39AM -0700, mnalajal@...eaurora.org wrote:
> On 2019-10-03 00:05, Greg KH wrote:
> > On Wed, Oct 02, 2019 at 05:06:14PM -0700, Murali Nalajala wrote:
> > > Soc framework exposed sysfs entries are not sufficient for some
> > > of the h/w platforms. Currently there is no interface where soc
> > > drivers can expose further information about their SoCs via soc
> > > framework. This change address this limitation where clients can
> > > pass their custom entries as attribute group and soc framework
> > > would expose them as sysfs properties.
> > > 
> > > Signed-off-by: Murali Nalajala <mnalajal@...eaurora.org>
> > > ---
> > >  drivers/base/soc.c      | 26 ++++++++++++++++++--------
> > >  include/linux/sys_soc.h |  1 +
> > >  2 files changed, 19 insertions(+), 8 deletions(-)
> > 
> > Can you change a soc driver to use this?  I don't think that this patch
> > works because:
> > 
> > > 
> > > diff --git a/drivers/base/soc.c b/drivers/base/soc.c
> > > index 7c0c5ca..ec70a58 100644
> > > --- a/drivers/base/soc.c
> > > +++ b/drivers/base/soc.c
> > > @@ -15,6 +15,8 @@
> > >  #include <linux/err.h>
> > >  #include <linux/glob.h>
> > > 
> > > +#define NUM_ATTR_GROUPS 3
> > > +
> > >  static DEFINE_IDA(soc_ida);
> > > 
> > >  static ssize_t soc_info_get(struct device *dev,
> > > @@ -104,11 +106,6 @@ static ssize_t soc_info_get(struct device *dev,
> > >  	.is_visible = soc_attribute_mode,
> > >  };
> > > 
> > > -static const struct attribute_group *soc_attr_groups[] = {
> > > -	&soc_attr_group,
> > > -	NULL,
> > > -};
> > > -
> > >  static void soc_release(struct device *dev)
> > >  {
> > >  	struct soc_device *soc_dev = container_of(dev, struct soc_device,
> > > dev);
> > > @@ -121,6 +118,7 @@ static void soc_release(struct device *dev)
> > >  struct soc_device *soc_device_register(struct soc_device_attribute
> > > *soc_dev_attr)
> > >  {
> > >  	struct soc_device *soc_dev;
> > > +	const struct attribute_group **soc_attr_groups = NULL;
> > >  	int ret;
> > > 
> > >  	if (!soc_bus_type.p) {
> > > @@ -136,10 +134,20 @@ struct soc_device *soc_device_register(struct
> > > soc_device_attribute *soc_dev_attr
> > >  		goto out1;
> > >  	}
> > > 
> > > +	soc_attr_groups = kzalloc(sizeof(*soc_attr_groups) *
> > > +						NUM_ATTR_GROUPS, GFP_KERNEL);
> > > +	if (!soc_attr_groups) {
> > > +		ret = -ENOMEM;
> > > +		goto out2;
> > > +	}
> > > +	soc_attr_groups[0] = &soc_attr_group;
> > > +	soc_attr_groups[1] = soc_dev_attr->custom_attr_group;
> > > +	soc_attr_groups[2] = NULL;
> > 
> > You set this, but never do anything with it that I can see.  What am I
> > missing?
> no, since i am using the "soc_attr_groups" name as it here you do not see
> the assignment below.
> It is something like this soc_dev->dev.groups = soc_attr_groups;

Ah, I see that now.  Tricky :)

nice work,

greg k-h