LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: Vinod Koul <vkoul@kernel.org>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>,
	alsa-devel@alsa-project.org, tiwai@suse.de,
	linux-kernel@vger.kernel.org, liam.r.girdwood@linux.intel.com,
	broonie@kernel.org, srinivas.kandagatla@linaro.org,
	jank@cadence.com, joe@perches.com,
	Sanyog Kale <sanyog.r.kale@intel.com>
Subject: Re: [alsa-devel] [RFC PATCH 1/7] soundwire: Add sysfs support for master(s)
Date: Tue, 7 May 2019 07:54:32 +0200	[thread overview]
Message-ID: <20190507055432.GB17986@kroah.com> (raw)
In-Reply-To: <20190507052732.GD16052@vkoul-mobl>

On Tue, May 07, 2019 at 10:57:32AM +0530, Vinod Koul wrote:
> On 06-05-19, 21:24, Pierre-Louis Bossart wrote:
> > 
> > > > +int sdw_sysfs_bus_init(struct sdw_bus *bus)
> > > > +{
> > > > +	struct sdw_master_sysfs *master;
> > > > +	int err;
> > > > +
> > > > +	if (bus->sysfs) {
> > > > +		dev_err(bus->dev, "SDW sysfs is already initialized\n");
> > > > +		return -EIO;
> > > > +	}
> > > > +
> > > > +	master = kzalloc(sizeof(*master), GFP_KERNEL);
> > > > +	if (!master)
> > > > +		return -ENOMEM;
> > > 
> > > Why are you creating a whole new device to put all of this under?  Is
> > > this needed?  What will the sysfs tree look like when you do this?  Why
> > > can't the "bus" device just get all of these attributes and no second
> > > device be created?
> > 
> > I tried a quick hack and indeed we could simplify the code with something as
> > simple as:
> > 
> > [attributes omitted]
> > 
> > static const struct attribute_group sdw_master_node_group = {
> > 	.attrs = master_node_attrs,
> > 	.name = "mipi-disco"
> > };
> > 
> > int sdw_sysfs_bus_init(struct sdw_bus *bus)
> > {
> > 	return sysfs_create_group(&bus->dev->kobj, &sdw_master_node_group);
> > }
> > 
> > void sdw_sysfs_bus_exit(struct sdw_bus *bus)
> > {
> > 	sysfs_remove_group(&bus->dev->kobj, &sdw_master_node_group);	
> > }
> > 
> > which gives me a simpler structure and doesn't require additional
> > pretend-devices:
> > 
> > /sys/bus/acpi/devices/PRP00001:00/int-sdw.0/mipi-disco# ls
> > clock_gears
> > /sys/bus/acpi/devices/PRP00001:00/int-sdw.0/mipi-disco# more clock_gears
> > 8086
> > 
> > The issue I have is that for the _show() functions, I don't see a way to go
> > from the device argument to bus. In the example above I forced the output
> > but would need a helper.
> > 
> > static ssize_t clock_gears_show(struct device *dev,
> > 				struct device_attribute *attr, char *buf)
> > {
> > 	struct sdw_bus *bus; // this is what I need to find from dev
> > 	ssize_t size = 0;
> > 	int i;
> > 
> > 	return sprintf(buf, "%d \n", 8086);
> > }
> > 
> > my brain is starting to fry, but I don't see how container_of() would work
> > here since the bus structure contains a pointer to the device. I don't also
> > see a way to check for all devices for the bus_type soundwire.
> > For the slaves we do have a macro based on container_of(), so wondering if
> > we made a mistake in the bus definition? Vinod, any thoughts?
> 
> yeah I dont recall a way to get bus fed into create_group, I did look at
> the other examples back then and IIRC and most of them were using a
> global to do the trick (I didn't want to go down that route).
> 
> I think that was the reason I wrote it this way...
> 
> BTW if you do use psedo-device you can create your own struct foo which
> embeds device and then then you can use container approach to get foo
> (and foo contains bus as a member).
> 
> Greg, any thoughts?

Why would you have "bus" attributes on a device?  I don't think you are
using "bus" here like the driver model uses the term "bus", right?

What are you really trying to show here?

And if you need to know the bus pointer from the device, why don't you
have a pointer to it in your device-specific structure?

thanks,

greg k-h

  reply	other threads:[~2019-05-07  5:54 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-04  1:00 [RFC PATCH 0/7] soundwire: add sysfs and debugfs support Pierre-Louis Bossart
2019-05-04  1:00 ` [RFC PATCH 1/7] soundwire: Add sysfs support for master(s) Pierre-Louis Bossart
2019-05-04  6:52   ` Greg KH
2019-05-06 16:43     ` [alsa-devel] " Pierre-Louis Bossart
2019-05-07  2:24     ` Pierre-Louis Bossart
2019-05-07  5:27       ` Vinod Koul
2019-05-07  5:54         ` Greg KH [this message]
2019-05-07 11:03           ` Vinod Koul
2019-05-07 11:19             ` Greg KH
2019-05-07 22:49               ` Pierre-Louis Bossart
2019-05-08  7:46                 ` Vinod Koul
2019-05-08  9:16                   ` Greg KH
2019-05-08 16:42                     ` Pierre-Louis Bossart
2019-05-08 16:59                       ` Greg KH
     [not found]                         ` <0b8d5238-6894-e2b4-5522-28636e40dd63@linux.intel.com>
2019-05-09  4:26                           ` Vinod Koul
2019-05-09 18:18                           ` Greg KH
2019-05-04  1:00 ` [RFC PATCH 2/7] soundwire: add Slave sysfs support Pierre-Louis Bossart
2019-05-04  6:54   ` Greg KH
2019-05-06 14:42     ` [alsa-devel] " Pierre-Louis Bossart
2019-05-06 15:19       ` Greg KH
2019-05-06 16:22         ` Vinod Koul
2019-05-06 16:46           ` Pierre-Louis Bossart
2019-05-07  5:19             ` Vinod Koul
2019-05-07 13:54               ` Pierre-Louis Bossart
2019-05-08  7:40                 ` Vinod Koul
2019-05-08 16:51                   ` Pierre-Louis Bossart
2019-05-04  1:00 ` [RFC PATCH 3/7] ABI: testing: Add description of soundwire master sysfs files Pierre-Louis Bossart
2019-05-04  6:53   ` Greg KH
2019-05-06 16:24   ` Vinod Koul
2019-05-04  1:00 ` [RFC PATCH 4/7] ABI: testing: Add description of soundwire slave " Pierre-Louis Bossart
2019-05-04  1:00 ` [RFC PATCH 5/7] soundwire: add debugfs support Pierre-Louis Bossart
2019-05-04  7:03   ` Greg KH
2019-05-06 14:48     ` [alsa-devel] " Pierre-Louis Bossart
2019-05-06 16:38       ` Vinod Koul
2019-05-06 16:54         ` Pierre-Louis Bossart
2019-05-07  5:56         ` Greg KH
2019-05-04  1:00 ` [RFC PATCH 6/7] soundwire: cadence_master: add debugfs register dump Pierre-Louis Bossart
2019-05-04  7:03   ` Greg KH
2019-05-06 14:50     ` [alsa-devel] " Pierre-Louis Bossart
2019-05-04  7:04   ` Greg KH
2019-05-04  1:00 ` [RFC PATCH 7/7] soundwire: intel: " Pierre-Louis Bossart
2019-05-04  7:04   ` Greg KH
2019-05-06 14:51     ` [alsa-devel] " Pierre-Louis Bossart

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190507055432.GB17986@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=jank@cadence.com \
    --cc=joe@perches.com \
    --cc=liam.r.girdwood@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pierre-louis.bossart@linux.intel.com \
    --cc=sanyog.r.kale@intel.com \
    --cc=srinivas.kandagatla@linaro.org \
    --cc=tiwai@suse.de \
    --cc=vkoul@kernel.org \
    --subject='Re: [alsa-devel] [RFC PATCH 1/7] soundwire: Add sysfs support for master(s)' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).