On 11/30/2016 10:52 AM, Thomas Renninger wrote: > On Tuesday, November 29, 2016 10:40:34 PM Martin Wilck wrote: >> Hi Thomas, >> >> On Tue, 2016-11-29 at 19:02 +0100, Thomas Renninger wrote: >>> On Monday, November 28, 2016 04:01:14 PM Corey Minyard wrote: >>>> Thanks, this has been on my todo list for a while, but I didn't >>>> know exactly >>>> how to accomplish it. >>> Why using a SOFTDEP? >>> If you always want this code in, when ipmi_si or other ipmi HW >>> drivers are >>> loaded, why not compile it into this impi_msghandler module? >> I prefer the softdep solution. It's a one-line patch, simple and clean, >> and expresses the intention nicely. > One line, does not mean simple and clean. > > The fact that MODULE_SOFTDEP is used at 5 places in the whole kernel only > and this is the first post: setup, already tells me that this is an uncommon > setup that should be avoided if possible. > > >> I didn't cover other architectures >> because that's not my area of expertise. >>> So my idea was and still is to compile the rather small: >>> ipmi_devintf into ipmi_msghandler >>> driver. impi_msghandler always gets loaded when ipmi HW drivers gets >>> successfully loaded. The device interface should always be created as >>> well. So >>> why do we need complicated and error prone SOFTDEPS, module >>> aliases... >> IMO this is precisely what softdeps were made for. > The character device should be put into the generic driver part. > I guess this softdeps stuff is made for really ugly dependencies. > I recently heard of gpio platform drivers needed to open sd card lock. > In this case this would make sense? > >> What's the actual >> user benefit of merging the code of ipmi_msghandler and ipmi_devintf? > A HW driver typically provides the character device. Is there any reason > for not doing so here? > Why should you want to have a mcelog_devintf, tty_devintf, ...
There are several in-kernel users of IPMI, including ACPI, a watchdog, and some poweroff code for systems that couldn't do it any other way. There are some others, I think, as people have asked how to do it, but they may not be in-tree. I2C has a separate device interface, and that's what I modelled this after. It doesn't have a softdep or anything like that. It is similar in size to ipmi_devintf.c >> In fact, IMO it's a bonus that this is a *soft* dep, as it provides >> users the freedom *not* to have /dev/ipmi0 in the system (for security >> reasons perhaps), by blacklisting or removing ipmi_devintf.ko. > Why? Keep it simple and straightforward if possible. > > There is no need to create a special line in: > /lib/modules/`uname -r`/modules.softdep > and rely on make modules and modprobe magic. > > I can provide new patch(es) if others agree and my idea is preferred. Doing this is not hard, the question is: What is the best policy? There are users that need the device interface, but there are plenty that use IPMI indirectly through ACPI or the watchdog and don't care about the device interface. The device interface code is not huge, but it's not small, either. There are minor security implications. I'd like to give the user the freedom to not have the device but the convenience of it working out of the box most of the time. The softdep seems to me to provide that, though thinking about it, it should probably be in ipmi_msghandler.c, not in the individual interfaces. -corey > Thomas > > ------------------------------------------------------------------------------ > _______________________________________________ > Openipmi-developer mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/openipmi-developer ------------------------------------------------------------------------------ _______________________________________________ Openipmi-developer mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openipmi-developer
