Re: [alsa-devel] [RFC PATCH 1/7] soundwire: Add sysfs support for master(s)

From: Pierre-Louis Bossart
Date: Mon May 06 2019 - 22:25:07 EST

+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
/sys/bus/acpi/devices/PRP00001:00/int-sdw.0/mipi-disco# more clock_gears

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?