Re: [alsa-devel] [PATCH v5 10/15] soundwire: Add sysfs for SoundWire DisCo properties

[Greg Kroah-Hartman]

On Fri, Dec 22, 2017 at 05:13:44PM +0530, Vinod Koul wrote:
> On Fri, Dec 22, 2017 at 09:33:44AM +0100, Greg Kroah-Hartman wrote:
> > On Fri, Dec 22, 2017 at 01:56:56PM +0530, Vinod Koul wrote:
> 
> > > Hey Greg,
> > > 
> > > So I have spent couple of days on this but don't have a solution.
> > > 
> > > I can create a subdirectory using dynamic attribute group by giving it a
> > > name. Works as advertised:). But I am unable to create subdirectory under
> > > newly created subdir. The properties defined by MIPI follow a certain
> > > hierarchy so we need subdirectories to represent that.
> > 
> > Yes, you can't create directories 2 levels deep, sorry, didn't realize
> > you wanted to do that.
> > 
> > > Second issue am facing with these is getting the sdw_bus context. I tried
> > > adding it but nothing looks elegant. I tried making sdw_attributes on top of
> > > struct attribute and pass around the bus but doesn't seem to work for me.
> > > 
> > > On the contrary, the current design of creating kobjects looked more
> > > cleaner. It has been used in bunch of other places, I double checked ref
> > > counting and cleanup looks okay to me. Btw I tested with libudev as well,
> > > its works well as we do send the event using kobject_uevent().
> > 
> > Where has it been used in other parts of the kernel like this?  Time to
> > go yell at people :)
> > 
> > And the issue isn't that you will not catch the uevent, but that this is
> > an attribute of the "parent" device, which is now not a device, but a
> > "raw" kobject on no bus at all.
> > 
> > So, just create these as real struct devices, with a different "type"
> > and have them all live on the bus properly.  Look at how the greybus
> > code does it as one example (USB is another one, but it's much more
> > complex.)
> 
> Thanks for the quick reply.
> 
> Are you sure greybus is a right example?

Yes, but:

> Looking at audio_manager.c we seem to create kset thus kobject without a
> device. It lives off global sysfs!

Not the audio_manager.c code :)

That code never really got reviewed all that well before it was merged.
There's a reason it is in staging...

> Probing further I think you wanted to point me to gb_interface_create()
> which I think would be similar to sdw_add_bus_master(). So we should
> create a device without a driver for this and make kobjects live off it.
> 
> So IIUC it is okay to create kbojects but make sure we they live with a
> 'device' right?

No, again, no driver or bus, should ever be creating "raw" kobjects.

Create a new structure, that has a struct device in it, and give it a
type that is unique to what ever this thing is and point the parent at
the device you are managing.  I don't have the time right now to dig
through this sorry.  Look at usb_create_ep_devs() maybe for a better
example.

thanks,

greg k-h