[RFC 0/2] misc: bt: added Block Transfer over I2C

[Brendan Higgins]

@Joel, thanks! I appreciate that :-)

> yes. The main question is about the use of the ioctl to propagate
> the SMS ATN event to the host. I think this is probably the best

You are changing the behavior of the interface in some respect, so I think
ioctl makes sense.

> method but as this is a user API, we should be careful with what we
> merge. I don't know any other implementation of a IPMI BT driver
> for the BMC side.

Fair point, that is something I am not sure about either. I think
upstreaming my patch would be contingent on whether we wanted to upstream
yours. On the other hand, if people want your IPMI BT driver from the BMC
side, it would be worth seeing if upstream wanted mine: bt-i2c addresses an
Aspeed specific issue (cannot NACK arbitrary messages makes SSIF support
possibly impossible), but we will see. I am more interested in seeing how
you guys feel about using Block Transfer for more things in a possibly
somewhat modified manner.

It seems like OpenBMC assumes all IPMI messages are assumed to be framed as
BT messages; maybe we want to consider formalizing around them? Just a
thought.

> Also, the drivers should probably be under char/ipmi. I will change
> that.

Makes sense, I will fix this in my next update.

> Finaly, the device name should contain "ipmi" IMO. Let's find a
> name ! or a pattern at least.

Also makes sense; I definitely want to follow the same pattern as you since
this is intended to provide the same API to the user. To throw my 2 cents
in, I do not like the "host" part of "bt-host", just because it feels
confusing. I get it from the standpoint of the userland (it is a port to
the host), but from a driver standpoint it seems confusing. I like maybe
use the term "bmc" or "slave" instead. Maybe the prefix could be
"ipmi-bt-xxx-cpu" and "ipmi-bt-xxx-bmc"?

Cheers

On Fri, Sep 9, 2016 at 9:30 AM Cédric Le Goater <clg at kaod.org> wrote:

> On 09/09/2016 07:22 AM, Joel Stanley wrote:
> > Hey Brendan,
> >
> > On Fri, Sep 9, 2016 at 6:58 AM, Brendan Higgins
> > <brendanhiggins at google.com> wrote:
> >> Looking for feedback on my new bt-i2c (Block Transfer over I2C)
> interface.
> >
> > Thanks for sending this out. You're setting a great example for the rest
> of us!
> >
> > I don't have an specific feedback for you, other than to say I think
> > what you are doing is reasonable.
> >
> >> I based the slave side (BMC) driver on the bt-host driver; the file
> system
> >> interface is fully compatible and has been tested with btbridged and
> ipmid
> >> running on an ast2500 evb.
> >
> > This is timely. Cedric has recently sent out the bt-host patches[1].
> > One of the questions raised was the suitability of the userspace API
> > for other users. It would be useful for you to put forward your
> > thoughts on that discussion.
> >
> > [1] http://www.spinics.net/lists/arm-kernel/msg527834.html
>
> yes. The main question is about the use of the ioctl to propagate
> the SMS ATN event to the host. I think this is probably the best
> method but as this is a user API, we should be careful with what we
> merge. I don't know any other implementation of a IPMI BT driver
> for the BMC side.
>
> Also, the drivers should probably be under char/ipmi. I will change
> that.
>
> Finaly, the device name should contain "ipmi" IMO. Let's find a
> name ! or a pattern at least.
>
> Thanks,
>
> C.
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20160909/2cbbd7e4/attachment.html>