Hi! > > I believe probability someone uses that with more than 4 CPUs is < > > 5%. > > So you even didn't bother to check: > > $ git grep "default-trigger = \"cpu[4-9]" > arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: linux,default-trigger = "cpu4"; > arch/arm/boot/dts/vexpress-v2m-rs1.dtsi: linux,default-trigger = "cpu5"; > arch/arm/boot/dts/vexpress-v2m.dtsi: linux,default-trigger = "cpu4"; > arch/arm/boot/dts/vexpress-v2m.dtsi: linux,default-trigger = "cpu5"; > > cpus are enumerated starting from 0, so there are more reasons for which > your patch is broken: > > 1. There are mainline users. > 2. You claim that you limit trigger use to 4 cpus, while the number is > actually 5, due to your condition: > + if (cpu > 4) > + continue; Ok, fixed. > 3. For platforms exceeding the limit the number of triggers registered > would not match the number all available cpus, for no obvious reason. > Better solution would be to prevent use of the trigger entirely > in such cases, which would need only to alter first instruction in > ledtrig_cpu_init(), which currently is: > > BUILD_BUG_ON(CONFIG_NR_CPUS > 9999); Hmm. If I do that I'll get complains from various build bots... But I might do dependency in Kconfig... > The correct approach would be to create new trigger with better > interface and then advise people switching to it. Patch would be accepted. > > Probability that someone uses it with more than 100 CPUs is << 1% > > I'd say. Systems just don't have that many LEDs. I'll take the risk. > > > > If I broke someone's real, existing setup, I'll raise the limit. > > Is this professional approach - throw a potential bug at users and > check if it will hit them? :-) And for no reason - you're not fixing > anything. I'm sorry I failed to meet your expectations. I raised limit to 8. Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Attachment:
signature.asc
Description: PGP signature
