Re: [PATCH v3] dma-buf: Add a capabilities directory

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, May 30, 2022 at 08:15:04AM +0000, Simon Ser wrote:
> On Monday, May 30th, 2022 at 09:20, Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> 
> > > > +static struct attribute *dma_buf_caps_attrs[] = {
> > > > +	&dma_buf_sync_file_import_export_attr.attr,
> > > > +	NULL,
> > > > +};
> > > > +
> > > > +static const struct attribute_group dma_buf_caps_attr_group = {
> > > > +	.attrs = dma_buf_caps_attrs,
> > > > +};
> > >
> > > Didn't we had macros for those? I think I have seen something for that.
> >
> > Yes, please use ATTRIBUTE_GROUPS()
> 
> This doesn't allow the user to set a group name, and creates an unused
> "_groups" variable, causing warnings.

Then set a group name.

But you really want to almost always be using lists of groups, which is
why that macro works that way.

> 
> > > > +
> > > > +static struct kobject *dma_buf_caps_kobj;
> > > > +
> > > > +int dma_buf_init_sysfs_capabilities(struct kset *kset)
> > > > +{
> > > > +	int ret;
> > > > +
> > > > +	dma_buf_caps_kobj = kobject_create_and_add("caps", &kset->kobj);
> > > > +	if (!dma_buf_caps_kobj)
> > > > +		return -ENOMEM;
> > > > +
> > > > +	ret = sysfs_create_group(dma_buf_caps_kobj, &dma_buf_caps_attr_group);
> >
> > Why do we have "raw" kobjects here?
> >
> > A group can have a name, which puts it in the subdirectory of the object
> > it is attached to.  Please do that and do not create a new kobject.
> 
> I see, I'll switch to sysfs_create_group with a group name in the next version.

No, do not do that, add it to the list of groups for the original
kobject.

thanks,

greg k-h



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux