On Wed, Aug 9, 2017 at 7:26 PM, Corey Minyard <tcminyard@xxxxxxxxx> wrote: > On 08/09/2017 08:04 PM, Brendan Higgins wrote: >>> >>> Perhaps that is some level of abuse, but it's pretty common. I'm not >>> against it. >>> >>> There is standard IPMI firmware NetFN (though no commands defined) that >>> if >>> you use >>> the driver automatically goes into "Maintenance mode" and modified the >>> timeouts >>> and handling to some extent to help with this. >> >> That is a really good point, I missed that. >> ... >>> >>> >>> There are ways to accomplish this that aren't that complex. You can >>> create >>> an OEM >>> command that can query the maximum message size and the ability to do >>> sequence >>> numbers in the messages. >>> >>> If messages larger than 32-bytes are supported, and the host I2C/SMBus >>> driver >>> supports it, you could use the standard SSIF SMBus commands to do this, >>> they >>> have an 8-bit length field. >>> >>> If sequence numbers are supported, The SSIF could use different SMBus >>> commands >>> to do the write and read requests. Since this is only if you get an OEM >>> command, >>> and if you put the sequence numbers at the end where they are easy to add >>> on >>> the send side, this is a small change to the driver. >> >> What if we just had an OEM command that changed the message structure from >> that point on? We could abuse the "maintenance mode" NetFN to get back >> into >> normal SSIF if necessary. > > > Actually, I wouldn't have a separate "openbmc mode". I would have OpenBMC > always > work with standard SSIF, and have separate SMBus commands for messages with > the sequence number and messages larger than 32 bytes. > > I've attached a patch with what I would expect the changes to be to the host > driver. > It doesn't handle multiple outstanding messages, but it shows what detection > and a > separate SMBus command would look like. I took a look at the patch, it seems reasonable. If I was maintaining SSIF, I probably would not want that kind of clutter for my admittedly weird use case, but if you're okay with it, then so am I. > > >>> So I think the changes would be small and contained. I'm actually ok >>> with a >>> different driver, but I think it would be more valuable to the OpenBMC >>> project >>> to have a standardized interface that would work (in a not quite as >>> efficient >>> mode) with software that does not use the Linux IPMI driver. >> >> I guess I see the all of my asks as hacky things which we can hopefully >> remove >> at some point. Hopefully, most OpenBMC users won't want or need these >> things. >> ... >>>> >>>> Regardless of what we do with the "BT-I2C" stuff, I am still interested >>>> in >>>> what >>>> you think about this. >>> >>> >>> I think you are right, it probably belongs some place else. The way that >>> makes the most >>> sense to me would be to have an "ipmi" directory with a "host" and >>> "slave" >>> side, and since >>> ipmi is not really a char driver, to move it to the main driver >>> directory. >>> That might be >>> fairly disruptive, though. >> >> That was my thinking exactly. >> >>> The other option that makes sense to me would be to add a >>> drivers/char/ipmi_slave directory, >>> or something like that, and put the slave code there. That would be less >>> disruptive. >> >> Right that is the approach I took, except I called it >> drivers/char/ipmi_bmc. >> >> I originally thought doing the less disruptive thing is best; however, I >> know >> there are also some OpenBMC people who are interested in implementing >> IPMB. So maybe now is the time to bite the bullet and create an ipmi >> directory under drivers/. > > > I'm not sure IPMB would make much difference, there's no host side change as > it's > already supported. I don't think there would be any significant code > sharing > between the two. No, I don't expect much code sharing between them. I just thought it would be a reasonable place to put IPMB, sort of like how we have a bunch of "character" device drivers in drivers/char, but I suppose that might be somewhat of an anti-pattern ;-) > > If there end up being a significant amount of common code, then it would > definitely be worth the effort to move it. > >>> -corey >> >> In summary, I think I can live with making it a mangled form of SSIF, but >> I would prefer to put it in its own driver. > > > You can look at the patch and consider it, and consider that you would need > to > implement flag and event handling. On an x86 host there would be SMBIOS > and ACPI stuff to deal with somehow for discovery. There's probably few > other > things to deal with. > >> In any case, I think I would rather focus on the the BMC side IPMI >> framework >> now, since it is a bigger change and would also reduce the work of >> implementing a BMC side SSIF driver. >> >> Here is what I propose: we focus on the BMC side IPMI framework RFC that >> I sent out the other day: >> https://www.mail-archive.com/linux-kernel@xxxxxxxxxxxxxxx/msg1463473.html >> I will add a change to the BMC side IPMI framework patchset to move all >> the >> IPMI stuff to the new drivers/ipmi directory as discussed and then drop >> the >> patch in that patchset that depends on this patchset. >> >> Let me know what you think > > > Let's hold off on the new directory, there's probably some convincing of the > "powers > that be" for that. > > I'll look at the patch set tomorrow, unless something critical comes up. Sounds good > > Thanks, > > -corey > Cheers -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
