Skip to contentSkip to conversation

add USB coldplay support into udevstart

PATCH In review Thierry Vignaud 6 participants 12 messages Aug 7, 2005 – Aug 10, 2005

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 add USB coldplay support into udevstart

the following patch from Olivier Blin adds USB coldplay support into udevstart (patch is against udev-065):

udevstart.c renamed from udevstart.c.usb-coldplug +66 -3
@@ -138,11 +138,20 @@
 
 	udev_init_device(&udev, devpath, subsystem, "add");
 	udev.devt = get_devt(class_dev);
-	if (!udev.devt && udev.type != DEV_NET) {
-		dbg("sysfs_open_class_device_path failed");
+	if (udev.devt || udev.type == DEV_NET) {
+		udev_rules_get_name(&rules, &udev, class_dev);
+	} else if (udev.type == DEV_DEVICE) {
+		struct sysfs_device *sysfs_dev;
+		sysfs_dev = sysfs_open_device_path(path);
+		if (sysfs_dev == NULL) {
+			dbg("sysfs_open_device_path failed");
+			return -1;
+		}
+		udev_rules_get_run(&rules, &udev, NULL, sysfs_dev);
+	} else {
+		dbg("get_devt failed");
 		return -1;
 	}
-	udev_rules_get_name(&rules, &udev, class_dev);
 	if (udev.ignore_device) {
 		dbg("device event will be ignored");
 		goto exit;
@@ -329,6 +338,59 @@
 	exec_list(&device_list);
 }
 
+static int has_modalias(const char *directory)
+{
+	char filename[PATH_SIZE];
+	struct stat statbuf;
+
+	snprintf(filename, sizeof(filename), "%s/modalias", directory);
+	filename[sizeof(filename)-1] = '\0';
+
+	if (stat(filename, &statbuf) == 0)
+		return 1;
+
+	return 0;
+}
+
+static void udev_scan_usb(void)
+{
+	char base[PATH_SIZE];
+	DIR *dir;
+	struct dirent *dent;
+	LIST_HEAD(device_list);
+
+	snprintf(base, sizeof(base), "%s/bus/usb/devices", sysfs_path);
+	base[sizeof(base)-1] = '\0';
+
+	dir = opendir(base);
+	if (dir != NULL) {
+		for (dent = readdir(dir); dent != NULL; dent = readdir(dir)) {
+			char linkname[PATH_SIZE];
+			char dirname_short[PATH_SIZE];
+			char *dirname = NULL;
+
+			if (dent->d_name[0] == '.')
+				continue;
+
+			snprintf(linkname, sizeof(linkname), "%s/%s", base, dent->d_name);
+			linkname[sizeof(linkname)-1] = '\0';
+
+			if (has_modalias(linkname)) {
+				int ret = readlink(linkname, dirname_short, PATH_SIZE);
+				if (ret > 8) {
+					dirname_short[ret] = '\0';
+					/* replace ../../.. with /sys */
+					dirname = dirname_short + 4;
+					memcpy(dirname, "/sys", 4);
+					device_list_insert(dirname, "usb", &device_list);
+				}
+			}
+		}
+		closedir(dir);
+	}
+	exec_list(&device_list);
+}
+
 static void asmlinkage sig_handler(int signum)
 {
 	switch (signum) {
@@ -371,6 +433,7 @@
 
 	udev_scan_block();
 	udev_scan_class();
+	udev_scan_usb();
 
 	udev_rules_close(&rules);
 	logging_close();

btw, note that according to olivier, there's code from udev_event.c duplicated in udevstartc.c

#1 lore raw
Quoted 2 lines · Thierry Vignaud

the following patch from Olivier Blin adds USB coldplay support into udevstart (patch is against udev-065):

Care to just change this to work for all busses that might have "modalias" files in them? USB isn't the only one...

thanks,

greg k-h

-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#2 lore raw
In reply to Thierry Vignaud
Quoted 2 lines · Thierry Vignaud

udevstart already performs coldplug regarding classes & block devices. it's exactly what udevstart was designed for IMHO.

No. udevstart was to do "coldplug" for devices that udev supports. Adding the modalias stuff to it makes it start supporting events that udev traditionally doesn't care about.

So I agree with Kay, we should do this in multiple steps, with different helpers to do this, to keep distros sane.

thanks,

greg k-h

-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#3 lore raw
In reply to Thierry Vignaud
Quoted 3 lines · On Mon, Aug 08, 2005 at 09:40:33AM -0700, [email protected] wrote:

(*) And come to think of it, I2C. But I2C isn't going to
be hotpluggable ... there are too many devices that can't safely be probed;

But I'm working on providing this option. If you look, we pretty much always do this today in the userspace scripts that people run to try to determine what drivers to load for a specific system.

Yes, some devices can't be probed, but they are in the minority.

thanks,

greg k-h

-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#4 lore raw
In reply to Thierry Vignaud
Quoted 6 lines · Greg KH
Quoted 2 lines · Thierry Vignaud

the following patch from Olivier Blin adds USB coldplay support into udevstart (patch is against udev-065):

Care to just change this to work for all busses that might have "modalias" files in them? USB isn't the only one...

I don't think, that we want to do this in udevstart, at least at it's current state. It will break to many setups, cause all the distros have their own way to handle the coldplug case.

Everything in udevstart is serialized, which may not be optimal for all the things to do during a coldplug session, cause every sleeping script will delay the whole process.

In general I like the idea and we should experiment what the best solution would be: event-replay, coldplug-binary, udevstart + coldplug, synthesizing events and pass them to udevd for parallel execution...
But this should happen in a separate binary, so we don't break too many things at once. :)

Thanks,
Kay

-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#5 lore raw
In reply to Thierry Vignaud
Quoted 6 lines · Kay Sievers
Quoted 5 lines
Quoted 2 lines

the following patch from Olivier Blin adds USB coldplay support into udevstart (patch is against udev-065):

Care to just change this to work for all busses that might have "modalias" files in them? USB isn't the only one...

Quoted 7 lines

I don't think, that we want to do this in udevstart, at least at it's current state. It will break to many setups, cause all the distros have their own way to handle the coldplug case.

Everything in udevstart is serialized, which may not be optimal for all the things to do during a coldplug session, cause every sleeping script will delay the whole process.

udev'll serialize event processing anyway...

Quoted 6 lines

In general I like the idea and we should experiment what the best solution would be: event-replay, coldplug-binary, udevstart + coldplug, synthesizing events and pass them to udevd for parallel execution...
But this should happen in a separate binary, so we don't break too many things at once. :)

udevstart already performs coldplug regarding classes & block devices. it's exactly what udevstart was designed for IMHO.

-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#6 lore raw
In reply to Thierry Vignaud
Quoted 12 lines · Thierry Vignaud
Quoted 10 lines · Kay Sievers
Quoted 5 lines
Quoted 2 lines

the following patch from Olivier Blin adds USB coldplay support into udevstart (patch is against udev-065):

Care to just change this to work for all busses that might have "modalias" files in them? USB isn't the only one...

I don't think, that we want to do this in udevstart, at least at it's current state. It will break to many setups, cause all the distros have their own way to handle the coldplug case.

Quoted 5 lines
Quoted 3 lines

Everything in udevstart is serialized, which may not be optimal for all the things to do during a coldplug session, cause every sleeping script will delay the whole process.

udev'll serialize event processing anyway...

udev is a event process! udevd will not serialize the most cases!

Quoted 9 lines
Quoted 6 lines

In general I like the idea and we should experiment what the best solution would be: event-replay, coldplug-binary, udevstart + coldplug, synthesizing events and pass them to udevd for parallel execution...
But this should happen in a separate binary, so we don't break too many things at once. :)

udevstart already performs coldplug regarding classes & block devices. it's exactly what udevstart was designed for IMHO.

Yeah, great, tell me what it is designed for.

Thanks,
Kay

-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#7 lore raw
In reply to Thierry Vignaud
Quoted 3 lines · Kay Sievers

I don't think, that we want to do this in udevstart, at least at it's current state. It will break to many setups, cause all the distros have their own way to handle the coldplug case.

Please rename 'udevstart' to 'udev_fill_dev_dir_with_device_node_entries', to reflect what the real purpose is (or was, back in the old days).

-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#8 lore raw
In reply to Thierry Vignaud
Quoted 11 lines · On Mon, Aug 08, 2005 at 12:28:40PM -0700, [email protected] wrote:
Quoted 9 lines

Date: Mon, 8 Aug 2005 07:21:20 -0700
From: Greg KH <[email protected]>

Quoted 3 lines · On Mon, Aug 08, 2005 at 09:40:33AM -0700, [email protected] wrote:

(*) And come to think of it, I2C. But I2C isn't going to
be hotpluggable ... there are too many devices that can't safely be probed;

But I'm working on providing this option.

You mean, "modalias" for I2C?

Yes.

Quoted 6 lines
Quoted 3 lines

If you look, we pretty much
always do this today in the userspace scripts that people run to try to determine what drivers to load for a specific system.

That won't work for drivers that need to be running in order for the kernel to boot far enough to run userspace.

Userspace starts up before _any_ device probing ever happens today. So this really isn't an issue.

Quoted 6 lines

I have in mind power
management chips ... things that do power switching and maybe voltage regulation. Also, turnkey systems where it's unreasonable to expect any user to ever do such configuration. Modular drivers aren't always going to be a system option, either... and requiring lots of extra kernel boot arguments (for I2C probe/force options) is error prone.

I don't want to change the way the code currently works at all, just want to expose to userspace the device addresses the i2c drivers support, so that userspace can, if they want to, load the proper drivers without relying on trying them all, like they have to today.

Quoted 3 lines
Quoted 1 line

Yes, some devices can't be probed, but they are in the minority.

Right, but that doesn't mean it's appropriate to ignore them!

I'm not going to, see above.

Quoted 7 lines

Even if you can detect "Bus 2 address 3 has a chip of type X", probing is often unable to uncover the specific chip variant, which can be essential. There needs to be some boot-time way to get that information across. It'd be nice if that didn't involve adding lots of board-specific code to device drivers, or error-prone manual configuration; for now, board-specific driver code seems to be the solution that works best.

Any proposals are walcome.

thanks,

greg k-h

-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#9 lore raw
In reply to Thierry Vignaud
Quoted 8 lines · Olaf Hering
Quoted 3 lines · Kay Sievers

I don't think, that we want to do this in udevstart, at least at it's current state. It will break to many setups, cause all the distros have their own way to handle the coldplug case.

Please rename 'udevstart' to 'udev_fill_dev_dir_with_device_node_entries', to reflect what the real purpose is (or was, back in the old days).

I understand the initial goal of udevstart is to populate /dev, but it can be extended to replay other events that would have been missed before udev was started.

Maybe we could add a --coldplug= option with a comma-separated list of buses. Thus it will be backward compatible for distros already handling the coldplug case, and it will allow to easily switch to udevstart for coldplug. Should I write the patch?

By the way, it would probably be cleaner to use udev_process_event() in add_device() (from udevstart), unless there is some special case I miss.

And this piece of code:

	if (strcmp(dent->d_name, "net") = 0)
		device_list_insert(dirname2, "net", &device_list);
	else if (has_devt(dirname2))
		device_list_insert(dirname2, dent->d_name, &device_list);
can be simplified as:
	if (strcmp(dent->d_name, "net") = 0 || has_devt(dirname2))
		device_list_insert(dirname2, dent->d_name, &device_list);

Regards

Signature · 15 lines
Olivier Blin
Mandriva
Mandrakesoft becomes Mandriva



-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel
#10 lore raw
In reply to Thierry Vignaud
Quoted 11 lines · Olivier Blin
Quoted 8 lines · Olaf Hering
Quoted 3 lines · Kay Sievers

I don't think, that we want to do this in udevstart, at least at it's current state. It will break to many setups, cause all the distros have their own way to handle the coldplug case.

Please rename 'udevstart' to 'udev_fill_dev_dir_with_device_node_entries', to reflect what the real purpose is (or was, back in the old days).

Quoted 8 lines

I understand the initial goal of udevstart is to populate /dev, but it can be extended to replay other events that would have been missed before udev was started.

Maybe we could add a --coldplug= option with a comma-separated list of buses. Thus it will be backward compatible for distros already handling the coldplug case, and it will allow to easily switch to udevstart for coldplug. Should I write the patch?

Sure, that would be nice. But start it as it's own program. Maybe copy udevstart for it, but leave it as it is for now. I don't want to touch it with experimental stuff, also not with alternate code paths, it is the most critical part of udev for most of its users.

There are many options to try and I don't think that serializing the complete coldplug with a single process makes a lot of sense. It would execute all udev rules for every device it finds, external stuff is called that may take a unpredictable amount of time to run. This must be some async operation, similar to what we do with udevd, I think.

Quoted 3 lines

By the way, it would probably be cleaner to use udev_process_event() in add_device() (from udevstart), unless there is some special case I miss.

We don't want the wait_for_sysfs crap with udevstart, it is much faster that way. It may change with a future udev version, which dependend on a specific kernel version, that does not need most of the wait_for_sysfs anymore.

Quoted 8 lines

And this piece of code:

	if (strcmp(dent->d_name, "net") = 0)
		device_list_insert(dirname2, "net", &device_list);
	else if (has_devt(dirname2))
		device_list_insert(dirname2, dent->d_name, &device_list);
can be simplified as:
	if (strcmp(dent->d_name, "net") = 0 || has_devt(dirname2))
		device_list_insert(dirname2, dent->d_name, &device_list);

Yeah, that's right.

Thanks,
Kay

-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

#11 lore raw
In reply to Thierry Vignaud
Quoted 3 lines · Olivier Blin

I understand the initial goal of udevstart is to populate /dev, but it can be extended to replay other events that would have been missed before udev was started.

I think that the goal of udevstart should be to generate all (and only) the events that would be captured in the initramfs or received later directly from the kernel.
Since udev now handles every kind of events for modalias, firmwares loading, etc, then udevstart should be able to generate them.

Signature · 13 lines
ciao,
Marco


-------------------------------------------------------
SF.Net email is Sponsored by the Better Software Conference & EXPO
September 19-22, 2005 * San Francisco, CA * Development Lifecycle Practices
Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel