<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    <br>
    <br>
    <div class="moz-cite-prefix">Le 06/04/2023 à 19:16, Frederic Barrat
      a écrit :<br>
    </div>
    <blockquote type="cite"
      cite="mid:f23c6f27-a761-287f-5077-d65c8cbb5045@linux.ibm.com">
      <br>
      <br>
      On 13/09/2022 12:26, Christophe Lombard wrote:
      <br>
      <blockquote type="cite">The Management Component Transport
        Protocol (MCTP) defines a communication
        <br>
        model intended to facilitate communication.
        <br>
        <br>
        This patch initialize MCTP binding over LPC Bus interface.
        <br>
        <br>
        Several steps must be performed:
        <br>
        - Initialize the MCTP core (mctp_init()).
        <br>
        - Initialize a hardware binding as AST LPC mode host
        (mctp_astlpc_init()).
        <br>
        - Register the hardware binding with the core
        (mctp_register_bus()), using
        <br>
        a predefined EID (Host default is 9).
        <br>
        <br>
        To transmit a MCTP message, mctp_message_tx() is used.
        <br>
        To receive a MCTP message, a callback need to be provided and
        registered
        <br>
        through mctp_set_rx_all().
        <br>
        <br>
        For the transfer of MCTP messages, two basics components are
        used:
        <br>
        - A window of the LPC FW address space, where reads and writes
        are
        <br>
        forwarded to BMC memory.
        <br>
        - An interrupt mechanism using the KCS interface.
        <br>
        <br>
        hw/ast-bmc/ast-mctp.c is compilated if the compiler flag
        CONFIG_PLDM is
        <br>
        set.
        <br>
        <br>
        Signed-off-by: Christophe Lombard
        <a class="moz-txt-link-rfc2396E" href="mailto:clombard@linux.vnet.ibm.com"><clombard@linux.vnet.ibm.com></a>
        <br>
        ---
        <br>
          hw/ast-bmc/Makefile.inc |   7 +
        <br>
          hw/ast-bmc/ast-mctp.c   | 386
        ++++++++++++++++++++++++++++++++++++++++
        <br>
          include/ast.h           |  11 ++
        <br>
          3 files changed, 404 insertions(+)
        <br>
          create mode 100644 hw/ast-bmc/ast-mctp.c
        <br>
        <br>
        diff --git a/hw/ast-bmc/Makefile.inc b/hw/ast-bmc/Makefile.inc
        <br>
        index e7ded0e8..546f2bc7 100644
        <br>
        --- a/hw/ast-bmc/Makefile.inc
        <br>
        +++ b/hw/ast-bmc/Makefile.inc
        <br>
        @@ -2,5 +2,12 @@
        <br>
          SUBDIRS += hw/ast-bmc
        <br>
            AST_BMC_OBJS  = ast-io.o ast-sf-ctrl.o
        <br>
        +
        <br>
        +ifeq ($(CONFIG_PLDM),1)
        <br>
        +CPPFLAGS += -I$(SRC)/libmctp/
        <br>
        +#CFLAGS += -DAST_MCTP_DEBUG
        <br>
        +AST_BMC_OBJS += ast-mctp.o
        <br>
        +endif
        <br>
        +
        <br>
          AST_BMC = hw/ast-bmc/built-in.a
        <br>
          $(AST_BMC): $(AST_BMC_OBJS:%=hw/ast-bmc/%)
        <br>
        diff --git a/hw/ast-bmc/ast-mctp.c b/hw/ast-bmc/ast-mctp.c
        <br>
        new file mode 100644
        <br>
        index 00000000..1e7bdb51
        <br>
        --- /dev/null
        <br>
        +++ b/hw/ast-bmc/ast-mctp.c
        <br>
        @@ -0,0 +1,386 @@
        <br>
        +// SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later
        <br>
        +// Copyright 2022 IBM Corp.
        <br>
        +
        <br>
        +#define pr_fmt(fmt) "AST-MCTP: " fmt
        <br>
        +
        <br>
        +#include <lpc.h>
        <br>
        +#include <interrupts.h>
        <br>
        +#include <timer.h>
        <br>
        +#include <timebase.h>
        <br>
        +#include <debug_descriptor.h>
        <br>
        +#include <ast.h>
        <br>
        +#include <console.h>
        <br>
        +#include <libmctp.h>
        <br>
        +#include <libmctp-cmds.h>
        <br>
        +#include <libmctp-log.h>
        <br>
        +#include <libmctp-astlpc.h>
        <br>
        +
        <br>
        +/*
        <br>
        + * For the EIDs: the valid range is 8-254.
        <br>
        + * host default = 9
        <br>
        + */
        <br>
        +uint8_t HOST_EID = 9;
        <br>
        +
        <br>
        +struct timer ast_boot_timer;
        <br>
        +static struct mctp *mctp;
        <br>
        +
        <br>
        +struct ast_mctp_backend {
        <br>
        +    bool (*ready)(void);
        <br>
        +    int (*binding)(void);
        <br>
        +    void (*destroy)(void);
        <br>
        +};
        <br>
        +const struct ast_mctp_backend *mctp_backend;
        <br>
        +
        <br>
        +/*
        <br>
        + * The AST LPC binding is described here:
        <br>
        + *
        <br>
        + *
<a class="moz-txt-link-freetext" href="https://github.com/openbmc/libmctp/blob/master/docs/bindings/vendor-astlpc.md">https://github.com/openbmc/libmctp/blob/master/docs/bindings/vendor-astlpc.md</a><br>
      </blockquote>
      <br>
      <br>
      The correct URL is now:
      <br>
<a class="moz-txt-link-freetext" href="https://github.com/openbmc/libmctp/blob/master/docs/bindings/vendor-ibm-astlpc.md">https://github.com/openbmc/libmctp/blob/master/docs/bindings/vendor-ibm-astlpc.md</a>
      <br>
      <br>
    </blockquote>
    <br>
    Thanks.<br>
    <br>
    <blockquote type="cite"
      cite="mid:f23c6f27-a761-287f-5077-d65c8cbb5045@linux.ibm.com">
      <br>
      <blockquote type="cite">+ *
        <br>
        + * Most of the binding is implemented in libmctp, but we need
        to provide
        <br>
        + * accessors for the LPC FW space (for the packet buffer) and
        for the KCS
        <br>
        + * peripheral (for the interrupt mechanism).
        <br>
        + */
        <br>
        +
        <br>
        +struct astlpc_ops_data {
        <br>
        +    uint16_t kcs_data_addr; /* LPC IO space offset for the data
        register */
        <br>
        +    uint16_t kcs_stat_addr;
        <br>
        +
        <br>
        +    /* address of the packet exchange buffer in FW space */
        <br>
        +    uint32_t lpc_fw_addr;
        <br>
        +};
        <br>
        +
        <br>
        +static int astlpc_kcs_reg_read(void *binding_data,
        <br>
        +                   enum mctp_binding_astlpc_kcs_reg reg,
        <br>
        +                   uint8_t *val)
        <br>
        +{
        <br>
        +    struct astlpc_ops_data *ops_data = binding_data;
        <br>
        +    uint32_t addr, tmp;
        <br>
        +    int rc;
        <br>
        +
        <br>
        +    if (reg == MCTP_ASTLPC_KCS_REG_STATUS)
        <br>
        +        addr = ops_data->kcs_stat_addr;
        <br>
        +    else if (reg == MCTP_ASTLPC_KCS_REG_DATA)
        <br>
        +        addr = ops_data->kcs_data_addr;
        <br>
        +    else
        <br>
        +        return OPAL_PARAMETER;
        <br>
        +
        <br>
        +    rc = lpc_read(OPAL_LPC_IO, addr, &tmp, 1);
        <br>
        +    if (!rc)
        <br>
        +        *val = tmp & 0xff;
        <br>
        +    else
        <br>
        +        *val = 0xff;
        <br>
        +
        <br>
        +    mctp_prdebug("%s 0x%hhx from %s (%d)",
        <br>
        +             __func__, *val, reg ? "status" : "data", rc);
        <br>
        +
        <br>
        +    return rc;
        <br>
        +}
        <br>
        +
        <br>
        +static int astlpc_kcs_reg_write(void *binding_data,
        <br>
        +                enum mctp_binding_astlpc_kcs_reg reg,
        <br>
        +                uint8_t val)
        <br>
        +{
        <br>
        +    struct astlpc_ops_data *ops_data = binding_data;
        <br>
        +    uint32_t addr;
        <br>
        +
        <br>
        +    mctp_prdebug("%s 0x%hhx to %s",
        <br>
        +             __func__, val, reg ? "status" : "data");
        <br>
        +
        <br>
        +    if (reg == MCTP_ASTLPC_KCS_REG_STATUS)
        <br>
        +        addr = ops_data->kcs_stat_addr;
        <br>
        +    else if (reg == MCTP_ASTLPC_KCS_REG_DATA)
        <br>
        +        addr = ops_data->kcs_data_addr;
        <br>
        +    else
        <br>
        +        return OPAL_PARAMETER;
        <br>
        +
        <br>
        +    return lpc_write(OPAL_LPC_IO, addr, val, 1);
        <br>
        +}
        <br>
        +
        <br>
        +static int astlpc_read(void *binding_data, void *buf, long
        offset,
        <br>
        +               size_t len)
        <br>
        +{
        <br>
        +    struct astlpc_ops_data *ops_data = binding_data;
        <br>
        +
        <br>
        +    mctp_prdebug("%s %zu bytes from 0x%lx (lpc: 0x%lx)",
        <br>
        +             __func__, len, offset,
        <br>
        +             ops_data->lpc_fw_addr + offset);
        <br>
        +    return lpc_fw_read(ops_data->lpc_fw_addr + offset, buf,
        len);
        <br>
        +}
        <br>
        +
        <br>
        +static int astlpc_write(void *binding_data, const void *buf,
        <br>
        +            long offset, size_t len)
        <br>
        +{
        <br>
        +    struct astlpc_ops_data *ops_data = binding_data;
        <br>
        +
        <br>
        +    mctp_prdebug("%s %zu bytes to offset 0x%lx (lpc: 0x%lx)",
        <br>
        +             __func__, len, offset,
        <br>
        +             ops_data->lpc_fw_addr + offset);
        <br>
        +    return lpc_fw_write(ops_data->lpc_fw_addr + offset, buf,
        len);
        <br>
        +}
        <br>
        +
        <br>
        +static const struct mctp_binding_astlpc_ops astlpc_ops = {
        <br>
        +    .kcs_read = astlpc_kcs_reg_read,
        <br>
        +    .kcs_write = astlpc_kcs_reg_write,
        <br>
        +    .lpc_read = astlpc_read,
        <br>
        +    .lpc_write = astlpc_write,
        <br>
        +};
        <br>
        +
        <br>
        +static struct mctp_binding_astlpc *astlpc;
        <br>
        +static struct astlpc_ops_data *ops_data;
        <br>
        +
        <br>
        +/* Keyboard Controller Style (KCS) data register address */
        <br>
        +#define KCS_DATA_REG 0xca2
        <br>
        +
        <br>
        +/* Keyboard Controller Style (KCS) status register address */
        <br>
        +#define KCS_STATUS_REG 0xca3
        <br>
        +
        <br>
        +/* Serial IRQ number for KCS interface */
        <br>
        +#define KCS_SERIAL_IRQ 11
        <br>
      </blockquote>
      <br>
      <br>
      These should probably be defined in platforms/astbmc/common.c and
      added to the device tree and the value read here from the device
      tree, like it's done for other LPC clients (UART, BT, MBOX).
      <br>
      <br>
    </blockquote>
    <br>
    ok will do.<br>
    <br>
    <blockquote type="cite"
      cite="mid:f23c6f27-a761-287f-5077-d65c8cbb5045@linux.ibm.com">
      <br>
      <blockquote type="cite">+
        <br>
        +#define KCS_STATUS_BMC_READY 0x80
        <br>
        +
        <br>
        +bool irq_active;
        <br>
        +
        <br>
        +/* we need a poller to crank the mctp state machine during boot
        */
        <br>
        +static void astlpc_poller(struct timer *t __unused,
        <br>
        +              void *data __unused, uint64_t now __unused)
        <br>
        +{
        <br>
        +    mctp_astlpc_poll(astlpc);
        <br>
        +
        <br>
        +    /* once booted we can rely on the KCS interrupt */
        <br>
        +    if ((opal_booting() && !irq_active))
        <br>
      </blockquote>
      <br>
      <br>
      We shoult talk about interrupts, I'm pretty convinced the call to
      opal_booting() is useless, since external interrupts are not going
      to fire while we are in skiboot. But this could all be moot if we
      use an opal poller (see below).
      <br>
      <br>
      <br>
      <blockquote type="cite">+       
        schedule_timer(&ast_boot_timer, msecs_to_tb(5));
        <br>
        +}
        <br>
        +
        <br>
        +/* at runtime the interrupt should handle it */
        <br>
        +static void astlpc_interrupt(uint32_t chip_id __unused,
        <br>
        +                 uint32_t irq_msk __unused)
        <br>
        +{
        <br>
        +    if (!irq_active) {
        <br>
        +        mctp_prerr("IRQ Active! Disabling boot poller...");
        <br>
        +        irq_active = true;
        <br>
        +    }
        <br>
        +
        <br>
        +    mctp_astlpc_poll(astlpc);
        <br>
        +}
        <br>
        +
        <br>
        +static struct lpc_client kcs_lpc_client = {
        <br>
        +    .reset = NULL,
        <br>
        +    .interrupt = astlpc_interrupt,
        <br>
        +    .interrupts = LPC_IRQ(KCS_SERIAL_IRQ),
        <br>
        +};
        <br>
        +
        <br>
        +static bool astlpc_ready(void)
        <br>
        +{
        <br>
        +    uint32_t tmp = 0;
        <br>
        +
        <br>
        +    /* Probe for MCTP support by reading the STAT register of
        KCS#4 */
        <br>
        +    if (lpc_probe_read(OPAL_LPC_IO, KCS_STATUS_REG, &tmp,
        1)) {
        <br>
      </blockquote>
      <br>
      <br>
      We've talked about this, I've done some reading and I don't
      understand the need for lpc_probe_read() here. My understanding is
      that the "probe" version is more forgiving in case of errors (it
      temporarily masks the interrupt, does a read, then checks the
      error status register to report errors) and it is meant to be used
      with invalid addresses for example. It's not the case here. I've
      been running with a simple lpc_read() and it's working as expected
      for me.
      <br>
      <br>
      <br>
    </blockquote>
    <br>
    you have probably reason. We will use lpc_read() instead.<br>
    <br>
    <blockquote type="cite"
      cite="mid:f23c6f27-a761-287f-5077-d65c8cbb5045@linux.ibm.com">
      <blockquote type="cite">+        mctp_prdebug("KCS4 probe failed,
        skipping MCTP init");
        <br>
        +        return false;
        <br>
        +    }
        <br>
        +
        <br>
        +    return !!(tmp & KCS_STATUS_BMC_READY); /* high bit
        indicates BMC is listening */
        <br>
        +}
        <br>
        +
        <br>
        +#define DESIRED_MTU 32768
        <br>
        +
        <br>
        +static void astlpc_destroy(void)
        <br>
        +{
        <br>
        +    if (astlpc) {
        <br>
        +        mctp_astlpc_destroy(astlpc);
        <br>
        +        cancel_timer(&ast_boot_timer);
        <br>
        +        astlpc = NULL;
        <br>
        +    }
        <br>
        +}
        <br>
        +
        <br>
        +static int astlpc_binding(void)
        <br>
        +{
        <br>
        +    ops_data = zalloc(sizeof(struct astlpc_ops_data));
        <br>
        +    if (!ops_data)
        <br>
        +        return OPAL_NO_MEM;
        <br>
        +
        <br>
        +    /*
        <br>
        +     * Current OpenBMC systems put the MCTP buffer 1MB down
        from
        <br>
        +     * the end of the LPC FW range.
        <br>
        +     *
        <br>
        +     * The size of the FW range is: 0x1000_0000 so the window
        be at:
        <br>
        +     *
        <br>
        +     *   0x1000_0000 - 2**20 == 0xff00000
        <br>
        +     */
        <br>
        +    ops_data->lpc_fw_addr = 0xff00000;
        <br>
      </blockquote>
      <br>
      <br>
      The fw address is already defined in HDAT! Look for "mctp_base" in
      bmc_create_node(). Maybe it could be added to the device tree from
      HDAT and read here?
      <br>
      <br>
      <br>
    </blockquote>
    <br>
    Thanks.<br>
    <br>
    <blockquote type="cite"
      cite="mid:f23c6f27-a761-287f-5077-d65c8cbb5045@linux.ibm.com">
      <blockquote type="cite">+
        <br>
        +    /* values chosen by the OpenBMC driver */
        <br>
        +    ops_data->kcs_data_addr = KCS_DATA_REG;
        <br>
        +    ops_data->kcs_stat_addr = KCS_STATUS_REG;
        <br>
        +
        <br>
        +    /* Initialise the binding */
        <br>
        +    astlpc = mctp_astlpc_init(MCTP_BINDING_ASTLPC_MODE_HOST,
        <br>
        +                  DESIRED_MTU,
        <br>
        +                  NULL,
        <br>
        +                  &astlpc_ops,
        <br>
        +                  ops_data);
        <br>
        +    if (!astlpc) {
        <br>
        +        mctp_prerr("binding init failed");
        <br>
        +        return OPAL_HARDWARE;
        <br>
        +    }
        <br>
        +
        <br>
        +    /* register an lpc client so we get an interrupt */
        <br>
        +    lpc_register_client(0, &kcs_lpc_client,
        IRQ_ATTR_TARGET_OPAL);
        <br>
        +
        <br>
        +    init_timer(&ast_boot_timer, astlpc_poller, astlpc);
        <br>
        +    schedule_timer(&ast_boot_timer, msecs_to_tb(5));
        <br>
      </blockquote>
      <br>
      <br>
      I'm thinking we should convert this to an opal poller (by calling
      opal_add_poller()) instead of defining our own timer. It would
      call our polling function whenever opal_run_pollers() is called
      and we don't need to manage the timer (by re-arming it each time
      astlpc_poller() is called).
      <br>
      <br>
      <br>
      <blockquote type="cite">+
        <br>
        +    /* Register the binding to the LPC bus we are using for
        this
        <br>
        +     * MCTP configuration.
        <br>
        +     */
        <br>
        +    if (mctp_register_bus(mctp,
        <br>
        +                  mctp_binding_astlpc_core(astlpc),
        <br>
        +                  HOST_EID)) {
        <br>
        +        mctp_prerr("failed to register bus");
        <br>
        +        goto err;
        <br>
        +    }
        <br>
        +
        <br>
        +    return OPAL_SUCCESS;
        <br>
        +
        <br>
        +err:
        <br>
      </blockquote>
      <br>
      <br>
      The timer needs to be cleaned up on the error path.
      <br>
      <br>
    </blockquote>
    <br>
    <span>Good catch!<br>
      <br>
    </span>
    <blockquote type="cite"
      cite="mid:f23c6f27-a761-287f-5077-d65c8cbb5045@linux.ibm.com">
      <br>
      <blockquote type="cite">+    astlpc_destroy();
        <br>
        +    free(ops_data);
        <br>
        +
        <br>
        +    return OPAL_HARDWARE;
        <br>
        +}
        <br>
        +
        <br>
        +static const struct ast_mctp_backend astlpc_backend = {
        <br>
        +    .ready = astlpc_ready,
        <br>
        +    .binding = astlpc_binding,
        <br>
        +    .destroy = astlpc_destroy,
        <br>
        +};
        <br>
        +
        <br>
        +static void *mctp_malloc(size_t size) { return malloc(size); }
        <br>
        +static void mctp_free(void *ptr) { return free(ptr); }
        <br>
        +static void *mctp_realloc(void *ptr, size_t size)
        <br>
        +{
        <br>
        +    return realloc(ptr, size);
        <br>
        +}
        <br>
        +
        <br>
        +#ifdef AST_MCTP_DEBUG
        <br>
        +char buffer[320];
        <br>
        +static void mctp_log(int log_lvl, const char *fmt, va_list va)
        <br>
        +{
        <br>
        +    snprintf(buffer, sizeof(buffer), "%s\n", fmt);
        <br>
        +    vprlog(log_lvl, buffer, va);
        <br>
        +}
        <br>
        +#endif
        <br>
      </blockquote>
      <br>
      <br>
      I don't really like how logging is done in this file. We always
      reuse the libmctp trace macro, which means:
      <br>
      - if we don't define AST_MCTP_DEBUG, we get no traces at all, not
      even errors.
      <br>
      - if we define AST_MCTP_DEBUG, then we also get the traces from
      libmctp, which floods the in-memory skiboot trace buffer with
      traces such as:
      <br>
      [  371.500175890,7] AST-MCTP: astlpc_kcs_reg_read 0xc0 from status
      (0)
      <br>
      [  371.500219079,7] astlpc: host: mctp_astlpc_poll: status: 0xc0
      <br>
      <br>
      Why not use the usual prlog() for traces from this file?
      <br>
      We can keep AST_MCTP_DEBUG just as a mean to control the libmctp
      tracing only.
      <br>
      <br>
    </blockquote>
    <br>
    ast-mctp.c is <span class="HwtZe" lang="en"><span class="jCAhz
        ChMk0b"><span class="ryNqvb">intrinsically linked to libmctp.
          For this reason we used the same way of tracing. <br>
        </span></span></span><span class="HwtZe" lang="en"><span
        class="jCAhz ChMk0b"><span class="ryNqvb">But no problem to use
          prolog.<br>
          <br>
        </span></span></span><span class="HwtZe" lang="en"><span
        class="jCAhz ChMk0b"><span class="ryNqvb"></span></span></span>
    <blockquote type="cite"
      cite="mid:f23c6f27-a761-287f-5077-d65c8cbb5045@linux.ibm.com">
      <br>
      <blockquote type="cite">+
        <br>
        +int ast_mctp_ready(void)
        <br>
        +{
        <br>
        +    if (!mctp_backend->ready())
        <br>
      </blockquote>
      <br>
      <br>
      That's an exported API, so it would be better to check if
      mctp_backend and mctp_backend->ready are non-NULL before
      calling it.
      <br>
      <br>
    </blockquote>
    <br>
    Correct.<br>
    <br>
    <blockquote type="cite"
      cite="mid:f23c6f27-a761-287f-5077-d65c8cbb5045@linux.ibm.com">
      <br>
      <blockquote type="cite">+        return OPAL_BUSY;
        <br>
        +
        <br>
        +    return OPAL_SUCCESS;
        <br>
        +}
        <br>
        +
        <br>
        +int ast_mctp_message_tx(uint8_t eid, uint8_t *msg, int len)
        <br>
        +{
        <br>
        +    uint8_t tag = 0;
        <br>
        +
        <br>
        +    return mctp_message_tx(mctp, eid, MCTP_MESSAGE_TO_DST, tag,
        msg, len);
        <br>
        +}
        <br>
        +
        <br>
        +void ast_mctp_stop_polling(void)
        <br>
        +{
        <br>
      </blockquote>
      <br>
      <br>
      This function looks like dead code. Leftover from debug?
      <br>
      <br>
    </blockquote>
    <br>
    Certainly, yes !<br>
    <br>
    <blockquote type="cite"
      cite="mid:f23c6f27-a761-287f-5077-d65c8cbb5045@linux.ibm.com">
      <br>
      <blockquote type="cite">+    irq_active = true;
        <br>
        +}
        <br>
        +
        <br>
        +#define HOST_MAX_INCOMING_MESSAGE_ALLOCATION 131072
        <br>
        +
        <br>
        +/*
        <br>
        + * Initialize mctp binding for hbrt and provide interfaces for
        sending
        <br>
        + * and receiving mctp messages.
        <br>
        + */
        <br>
        +int ast_mctp_init(uint8_t mode, void (*fn)(uint8_t src_eid,
        bool tag_owner,
        <br>
        +          uint8_t msg_tag, void *data, void *msg, size_t len))
        <br>
        +{
        <br>
        +    uint64_t start;
        <br>
        +
        <br>
        +    if (mode == MCTP_BINDING_ASTLPC_MODE)
        <br>
        +        mctp_backend = &astlpc_backend;
        <br>
        +    else {
        <br>
        +        mctp_prerr("Unknown AST binding mode: %u", mode);
        <br>
        +        return OPAL_PARAMETER;
        <br>
        +    }
        <br>
        +
        <br>
        +    if (!mctp_backend->ready())
        <br>
        +        return OPAL_HARDWARE;
        <br>
        +
        <br>
        +    /* skiboot's malloc/free/realloc are macros so they need
        <br>
        +     * wrappers
        <br>
        +     */
        <br>
        +    mctp_set_alloc_ops(mctp_malloc, mctp_free, mctp_realloc);
        <br>
        +
        <br>
        +    /*
        <br>
        +     * /-----\                                 /---------\
        <br>
        +     * | bmc | (eid: 8) <- lpc/kcs -> (eid: 9) | skiboot
        |
        <br>
        +     * \-----/               pcie              \---------/
        <br>
        +     *           astlpc kcs / astpcie binding
        <br>
        +     */
        <br>
        +    mctp = mctp_init();
        <br>
        +    if (!mctp) {
        <br>
        +        mctp_prerr("mctp init failed");
        <br>
        +        return OPAL_HARDWARE;
        <br>
        +    }
        <br>
        +
        <br>
        +#ifdef AST_MCTP_DEBUG
        <br>
        +    /* Setup the trace hook */
        <br>
        +    mctp_set_log_custom(mctp_log);
        <br>
        +#endif
        <br>
        +
        <br>
        +    mctp_prinfo("libmctp initialized");
        <br>
        +
        <br>
        +    /* Set the max message size to be large enough */
        <br>
        +    mctp_set_max_message_size(mctp,
        HOST_MAX_INCOMING_MESSAGE_ALLOCATION);
        <br>
        +
        <br>
        +    /* Setup the message rx callback */
        <br>
        +    mctp_set_rx_all(mctp, fn, NULL);
        <br>
        +
        <br>
        +    /* Initialise the binding */
        <br>
        +    if (mctp_backend->binding())
        <br>
        +        goto err;
        <br>
        +
        <br>
        +    /* Initializing the channel requires running the poll
        function
        <br>
        +     * a few times to handle the initial setup since we need
        the
        <br>
        +     * BMC to reply.
        <br>
        +     *
        <br>
        +     * Unfortuntately there's no good way to determine if this
        <br>
        +     * worked or not so poll for a bit with a timeout. :(
        <br>
        +     *
        <br>
        +     * We have to check (bus->state ==
        mctp_bus_state_constructed)
        <br>
        +     * otherwise mctp_message_tx() will reject the message.
        <br>
        +     */
        <br>
        +    start = mftb();
        <br>
        +    while (tb_compare(mftb(), start + msecs_to_tb(250)) ==
        TB_ABEFOREB)
        <br>
        +        time_wait_ms(10);
        <br>
      </blockquote>
      <br>
      <br>
      Why not a simple time_wait_ms() for the full duration? It would
      call the opal_run_pollers(), which check the timers.
      <br>
      <br>
        Fred
      <br>
      <br>
      <br>
      <br>
      <blockquote type="cite">+
        <br>
        +    mctp_prwarn("%s - The mctp bus state should be correct now
        !!!",
        <br>
        +            __func__);
        <br>
        +
        <br>
        +    return OPAL_SUCCESS;
        <br>
        +
        <br>
        +err:
        <br>
        +    mctp_prerr("Unable to initialize MCTP");
        <br>
        +    mctp_destroy(mctp);
        <br>
        +    mctp = NULL;
        <br>
        +
        <br>
        +    return OPAL_HARDWARE;
        <br>
        +}
        <br>
        +
        <br>
        +void ast_mctp_exit(void)
        <br>
        +{
        <br>
        +    if (mctp_backend && mctp_backend->destroy)
        <br>
        +        mctp_backend->destroy();
        <br>
        +
        <br>
        +    if (mctp)
        <br>
        +        mctp_destroy(mctp);
        <br>
        +
        <br>
        +    mctp = NULL;
        <br>
        +}
        <br>
        diff --git a/include/ast.h b/include/ast.h
        <br>
        index 5e932398..55eeae28 100644
        <br>
        --- a/include/ast.h
        <br>
        +++ b/include/ast.h
        <br>
        @@ -91,6 +91,17 @@ void ast_setup_ibt(uint16_t io_base, uint8_t
        irq);
        <br>
          /* MBOX configuration */
        <br>
          void ast_setup_sio_mbox(uint16_t io_base, uint8_t irq);
        <br>
          +/* AST MCTP configuration */
        <br>
        +#define MCTP_BINDING_ASTLPC_MODE 0
        <br>
        +#define MCTP_BINDING_ASTPCIE_MODE 1
        <br>
        +
        <br>
        +int ast_mctp_ready(void);
        <br>
        +int ast_mctp_init(uint8_t mode, void (*fn)(uint8_t src_eid,
        bool tag_owner,
        <br>
        +          uint8_t msg_tag, void *data, void *msg, size_t len));
        <br>
        +void ast_mctp_exit(void);
        <br>
        +void ast_mctp_stop_polling(void);
        <br>
        +int ast_mctp_message_tx(uint8_t eid, uint8_t *msg, int len);
        <br>
        +
        <br>
          #endif /* __SKIBOOT__ */
        <br>
            /*
        <br>
      </blockquote>
    </blockquote>
    <br>
  </body>
</html>