On Monday, July 29, 2013 6:50:59 pm Sean Bruno wrote: > [sbruno_comment_blocks == 4] > > > > > The identify function in 7.x has no such check: > > > > static void > > ipmi_isa_identify(driver_t *driver, device_t parent) > > { > > struct ipmi_get_info info; > > uint32_t devid; > > > > if (ipmi_smbios_identify(&info) && info.iface_type != SSIF_MODE && > > device_find_child(parent, "ipmi", -1) == NULL) { > > Ok then what is this ^^^^^^^^^ ? Doesn't this mean that if > device_find_child() returns a child node that we should abort? Is that > not the same as what I'm going on about?
This makes it only add at most one child device. It is a common idiom in identify routines so that if you kldunload and re-kldload you don't end up with two "ipmi" devices added by the identify routine. > > I'd rather be sure this is the right fix, and if it isn't I'd prefer to > > revert this as I don't think it is actually fixing anything. > > > It definitely does *not* have the effect that I advertised in my commit > message. > > the commit DOES: > -- remove any attempt to do anything in ipmi_isa_* functions. > -- does not emit any errors on attach failure (which are noisy and > distracting). For these, the better fix would be to check ipmi_attached in ipmi_isa_probe(). This is what happens in all the other bus front ends. > -- make attaching to ipmi0 more "reliable" by blindly raising the > timeout value to 6 seconds. (6 seconds is the totally empirical > value I came up with in testing that never failed to attach across > 100+ reboots). This is valid, and I don't think that should be reverted. > I disagree that it should be reverted. We can argue about it if you > wish and I'm open to modifying back to the original code. I don't think > I'd agree with removing the error messages on attachment failure though. > I view the attachment failures as "sysadmin noise" but they should be > there *if* there is a real attach failure. How about just moving the ipmi_attached check into the probe routine to match all the other uses (grep for ipmi_attached in the dir to see what I mean). Also, when you MFC, don't claim it fixes NMIs from bce(4), just that it removes noise and expands the timeout. :) -- John Baldwin _______________________________________________ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"