| Message ID | 1582681882-15254-1-git-send-email-joy_chu@wistron.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | platform/mihawk: support dynamic PCIe slot table | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch master (f123417068e51842004bdc047c8c5107b70442ef) |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot | fail | Test snowpatch/job/snowpatch-skiboot on branch master |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco | success | Signed-off-by present |
On Wed, Feb 26, 2020 at 12:57 PM Joy Chu <joy_chu@wistron.com> wrote: > > Slot table auto-detection for different riser cards by using IPMI > OEM command to communicate with BMC. > > Signed-off-by: Joy Chu <joy_chu@wistron.com> > --- > platforms/astbmc/mihawk.c | 229 +++++++++++++++++++++++++++++++++++--- > 1 file changed, 214 insertions(+), 15 deletions(-) > > diff --git a/platforms/astbmc/mihawk.c b/platforms/astbmc/mihawk.c > index d33d16bb..28c999aa 100644 > --- a/platforms/astbmc/mihawk.c > +++ b/platforms/astbmc/mihawk.c > @@ -15,8 +15,16 @@ > #include <pci.h> > #include <pci-cfg.h> > > +#include <timebase.h> > + > #include "astbmc.h" > > +/* IPMI message code for Riser-F query (OEM). */ > +#define IPMI_RISERF_QUERY IPMI_CODE(0x32, 0x01) > + > +static bool mihawk_riserF_found = false; > +static bool bmc_query_waiting = false; > + > /* nvme backplane slots */ > static const struct slot_table_entry hdd_bay_slots[] = { > SW_PLUGGABLE("hdd0", 0x0), > @@ -91,7 +99,7 @@ static const struct platform_ocapi mihawk_ocapi = { > .ocapi_slot_label = mihawk_ocapi_slot_label, > }; > > -static const struct slot_table_entry P1E1A_x8_PLX8748_down[] = { > +static const struct slot_table_entry P1E1A_x8_PLX8748_RiserA_down[] = { > SW_PLUGGABLE("Slot7", 0x10), > SW_PLUGGABLE("Slot8", 0x8), > SW_PLUGGABLE("Slot10", 0x9), > @@ -99,25 +107,25 @@ static const struct slot_table_entry P1E1A_x8_PLX8748_down[] = { > { .etype = st_end } > }; > > -static const struct slot_table_entry P1E1A_x8_PLX8748_up[] = { > +static const struct slot_table_entry P1E1A_x8_PLX8748_RiserA_up[] = { > { > .etype = st_builtin_dev, > .location = ST_LOC_DEVFN(0,0), > - .children = P1E1A_x8_PLX8748_down, > + .children = P1E1A_x8_PLX8748_RiserA_down, > }, > { .etype = st_end } > }; > > -static const struct slot_table_entry p1phb1_slot[] = { > +static const struct slot_table_entry p1phb1_rA_slot[] = { > { > .etype = st_builtin_dev, > .location = ST_LOC_DEVFN(0,0), > - .children = P1E1A_x8_PLX8748_up, > + .children = P1E1A_x8_PLX8748_RiserA_up, > }, > { .etype = st_end }, > }; > > -static const struct slot_table_entry P0E1A_x8_PLX8748_down[] = { > +static const struct slot_table_entry P0E1A_x8_PLX8748_RiserA_down[] = { > SW_PLUGGABLE("Slot2", 0x10), > SW_PLUGGABLE("Slot3", 0x8), > SW_PLUGGABLE("Slot5", 0x9), > @@ -125,20 +133,120 @@ static const struct slot_table_entry P0E1A_x8_PLX8748_down[] = { > { .etype = st_end } > }; > > -static const struct slot_table_entry P0E1A_x8_PLX8748_up[] = { > +static const struct slot_table_entry P0E1A_x8_PLX8748_RiserA_up[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P0E1A_x8_PLX8748_RiserA_down, > + }, > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry p0phb1_rA_slot[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P0E1A_x8_PLX8748_RiserA_up, > + }, > + { .etype = st_end }, > +}; > + > +static const struct slot_table_entry P1E1A_x8_PLX8748_RiserF_down[] = { > + SW_PLUGGABLE("Slot7", 0x10), > + SW_PLUGGABLE("Slot10", 0x9), > + > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry P1E1A_x8_PLX8748_RiserF_up[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P1E1A_x8_PLX8748_RiserF_down, > + }, > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry p1phb1_rF_slot[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P1E1A_x8_PLX8748_RiserF_up, > + }, > + { .etype = st_end }, > +}; > + > +static const struct slot_table_entry P0E1A_x8_PLX8748_RiserF_down[] = { > + SW_PLUGGABLE("Slot2", 0x10), > + SW_PLUGGABLE("Slot5", 0x9), > + > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry P0E1A_x8_PLX8748_RiserF_up[] = { > { > .etype = st_builtin_dev, > .location = ST_LOC_DEVFN(0,0), > - .children = P0E1A_x8_PLX8748_down, > + .children = P0E1A_x8_PLX8748_RiserF_down, > }, > { .etype = st_end } > }; > > -static const struct slot_table_entry p0phb1_slot[] = { > +static const struct slot_table_entry p0phb1_rF_slot[] = { > { > .etype = st_builtin_dev, > .location = ST_LOC_DEVFN(0,0), > - .children = P0E1A_x8_PLX8748_up, > + .children = P0E1A_x8_PLX8748_RiserF_up, > + }, > + { .etype = st_end }, > +}; > + > +static const struct slot_table_entry P1E2_x16_Switch_down[] = { > + SW_PLUGGABLE("Slot8", 0x1), > + SW_PLUGGABLE("Slot9", 0x0), > + > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry P1E2_x16_Switch_up[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P1E2_x16_Switch_down, > + }, > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry p1phb3_switch_slot[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P1E2_x16_Switch_up, > + }, > + { .etype = st_end }, > +}; > + > +static const struct slot_table_entry P0E2_x16_Switch_down[] = { > + SW_PLUGGABLE("Slot3", 0x1), > + SW_PLUGGABLE("Slot4", 0x0), > + > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry P0E2_x16_Switch_up[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P0E2_x16_Switch_down, > + }, > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry p0phb3_switch_slot[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P0E2_x16_Switch_up, > }, > { .etype = st_end }, You can use the ST_PLUGGABLE() and ST_BUILTIN_DEV() macros to remove most of the struct boilerplate. There's an example of to use them for the upstream port of a switch, etc platforms/qemu/qemu.c. That said, if you use them and need to backport this patch then you'll need to pick cherry-pick bbe5f0038786 ("platforms/astbmc: Add more slot helper macros") too. > }; > @@ -148,22 +256,38 @@ ST_PLUGGABLE(p0phb3_slot, "Slot4"); > ST_PLUGGABLE(p1phb0_slot, "Slot6"); > ST_PLUGGABLE(p1phb3_slot, "Slot9"); > > -static const struct slot_table_entry mihawk_phb_table[] = { > +static const struct slot_table_entry mihawk_riserA_phb_table[] = { > /* ==== CPU0 ==== */ > ST_PHB_ENTRY(0, 0, p0phb0_slot), /* P0E0_x16_Slot1 */ > - ST_PHB_ENTRY(0, 1, p0phb1_slot), /* P0E1A_x8_PLX8748-1_Slot2-3-5 */ > + ST_PHB_ENTRY(0, 1, p0phb1_rA_slot), /* P0E1A_x8_PLX8748-1_Slot2-3-5 */ > //ST_PHB_ENTRY(0, 2, p0phb2_slot), /* P0E1B_x8_USBTI7340 */ > ST_PHB_ENTRY(0, 3, p0phb3_slot), /* P0E2_x16_Slot4 */ > > /* ==== CPU1 ==== */ > ST_PHB_ENTRY(8, 0, p1phb0_slot), /* P1E0_x16_Slot6 */ > - ST_PHB_ENTRY(8, 1, p1phb1_slot), /* P1E1A_x8_PLX8748-2_Slot7-8-10 */ > + ST_PHB_ENTRY(8, 1, p1phb1_rA_slot), /* P1E1A_x8_PLX8748-2_Slot7-8-10 */ > //ST_PHB_ENTRY(8, 2, p1phb2_slot), /* P1E1B_x8_NA */ > ST_PHB_ENTRY(8, 3, p1phb3_slot), /* P1E2_x16_Slot9 */ > > { .etype = st_end }, > }; > > +static const struct slot_table_entry mihawk_riserF_phb_table[] = { > + /* ==== CPU0 ==== */ > + ST_PHB_ENTRY(0, 0, p0phb0_slot), /* P0E0_x16_Slot1 */ > + ST_PHB_ENTRY(0, 1, p0phb1_rF_slot), /* P0E1A_x8_PLX8748-1_Slot2-5 */ > + //ST_PHB_ENTRY(0, 2, p0phb2_slot), /* P0E1B_x8_USBTI7340 */ > + ST_PHB_ENTRY(0, 3, p0phb3_switch_slot),/* P0E2_x16_SWITCH_Slot3-4 */ > + > + /* ==== CPU1 ==== */ > + ST_PHB_ENTRY(8, 0, p1phb0_slot), /* P1E0_x16_Slot6 */ > + ST_PHB_ENTRY(8, 1, p1phb1_rF_slot), /* P1E1A_x8_PLX8748-2_Slot7-10 */ > + //ST_PHB_ENTRY(8, 2, p1phb2_slot), /* P1E1B_x8_NA */ > + ST_PHB_ENTRY(8, 3, p1phb3_switch_slot),/* P1E2_x16_SWITCH_Slot8-9 */ > + > + { .etype = st_end }, > +}; > + > #define NPU_BASE 0x5011000 > #define NPU_SIZE 0x2c > #define NPU_INDIRECT0 0x8000000009010c3fUL /* OB0 - no OB3 on Mihawk */ > @@ -282,15 +406,90 @@ static bool mihawk_probe(void) > mihawk_create_npu(); > mihawk_create_ocapi_i2c_bus(); > > - slot_table_init(mihawk_phb_table); > + if (mihawk_riserF_found) > + slot_table_init(mihawk_riserF_phb_table); Is setting the slot table here necessary? It'll be overwritten when mihawk_init() is called later on and I don't think anything will reference the slot table until we start doing PCI probing. > return true; > } > > +static void mihawk_riser_query_complete(struct ipmi_msg *msg) > +{ > + if (msg->cc != IPMI_CC_NO_ERROR) { > + prlog(PR_ERR, "Mihawk: IPMI riser query returned error. cmd=0x%02x," > + " netfn=0x%02x, rc=0x%x\n", msg->cmd, msg->netfn, msg->cc); > + bmc_query_waiting = false; > + ipmi_free_msg(msg); > + return; > + } > + > + prlog(PR_DEBUG, "Mihawk: IPMI Got riser query result. p0:%02x, p1:%02x\n" > + , msg->data[0], msg->data[1]); > + > + uint8_t *riser_state = (uint8_t*)msg->user_data; > + lwsync(); > + *riser_state = msg->data[0] << 4 | msg->data[1]; > + > + bmc_query_waiting = false; > + ipmi_free_msg(msg); > +} > + > +static void mihawk_init(void) > +{ > + struct ipmi_msg *ipmi_msg; > + uint8_t riser_state = 0; > + int timeout_ms = 3000; > + > + astbmc_init(); > + > + /* > + * We use IPMI to ask BMC if Riser-F is installed and set up the > + * corresponding slot table. > + */ > + if (!mihawk_riserF_found) { Is mihawk_riserF_found ever going to be true at this point? Looks like its only ever set below > + ipmi_msg = ipmi_mkmsg(IPMI_DEFAULT_INTERFACE, > + IPMI_RISERF_QUERY, > + mihawk_riser_query_complete, > + &riser_state, NULL, 0, 2); > + > + if (!ipmi_msg) { > + prlog(PR_ERR, "Mihawk: Couldn't create ipmi msg."); > + } > + else { > + ipmi_msg->error = mihawk_riser_query_complete; > + ipmi_queue_msg(ipmi_msg); > + bmc_query_waiting = true; > + > + prlog(PR_DEBUG, "Mihawk: Requesting IPMI_RISERF_QUERY (netfn " > + "%02x, cmd %02x)\n", ipmi_msg->netfn, ipmi_msg->cmd); > + > + while (bmc_query_waiting) { > + time_wait_ms(10); > + timeout_ms -= 10; > + > + if (timeout_ms == 0) > + break; > + } > + } Can you use ipmi_queue_msg_sync() instead of the callback and open-coded delay loop? The only problem I can see is that if the BMC doesn't respond we'd hit the timeout in the current code while ipmi_queue_msg_sync() will wait forever. We could always add a timeout to the _sync version though. > + prlog(PR_DEBUG, "Mihawk: IPMI_RISERF_QUERY finish. riser_state: %02x" > + ", waiting: %d\n", riser_state, bmc_query_waiting); > + > + if (riser_state != 0) { > + mihawk_riserF_found = true; > + slot_table_init(mihawk_riserF_phb_table); > + prlog(PR_DEBUG, "Mihawk: Detect Riser-F via IPMI\n"); > + } > + else { no newline > + slot_table_init(mihawk_riserA_phb_table); > + prlog(PR_DEBUG, "Mihawk: No Riser-F found, use Riser-A table\n"); > + } > + } > +} > + > DECLARE_PLATFORM(mihawk) = { > .name = "Mihawk", > .probe = mihawk_probe, > - .init = astbmc_init, > + .init = mihawk_init, > .start_preload_resource = flash_start_preload_resource, > .resource_loaded = flash_resource_loaded, > .bmc = &bmc_plat_ast2500_openbmc, > -- > 2.17.1 > > _______________________________________________ > Skiboot mailing list > Skiboot@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/skiboot
Hi Oliver, The slot helper macros example from qemu looks more simpler and clearer. We'll take this commit for consideration. To use ipmi_queue_msg_sync() with timeout, would you please share some example? We want to add the timeout to prevent from the long waiting and the increase of boot-up time. Best Regards, Joy Chu -----Original Message----- From: Oliver O'Halloran <oohall@gmail.com> Sent: Wednesday, February 26, 2020 12:23 PM To: Joy Chu/WHQ/Wistron <Joy_Chu@wistron.com> Cc: skiboot list <skiboot@lists.ozlabs.org>; Stewart Smith <stewart@linux.vnet.ibm.com>; Oliver OHalloran <oliveroh@au1.ibm.com>; chhank@tw.ibm.com; chcyjoy@gmail.com Subject: Re: [Skiboot] [PATCH] platform/mihawk: support dynamic PCIe slot table On Wed, Feb 26, 2020 at 12:57 PM Joy Chu <joy_chu@wistron.com> wrote: > > Slot table auto-detection for different riser cards by using IPMI OEM > command to communicate with BMC. > > Signed-off-by: Joy Chu <joy_chu@wistron.com> > --- > platforms/astbmc/mihawk.c | 229 > +++++++++++++++++++++++++++++++++++--- > 1 file changed, 214 insertions(+), 15 deletions(-) > > diff --git a/platforms/astbmc/mihawk.c b/platforms/astbmc/mihawk.c > index d33d16bb..28c999aa 100644 > --- a/platforms/astbmc/mihawk.c > +++ b/platforms/astbmc/mihawk.c > @@ -15,8 +15,16 @@ > #include <pci.h> > #include <pci-cfg.h> > > +#include <timebase.h> > + > #include "astbmc.h" > > +/* IPMI message code for Riser-F query (OEM). */ > +#define IPMI_RISERF_QUERY IPMI_CODE(0x32, 0x01) > + > +static bool mihawk_riserF_found = false; static bool > +bmc_query_waiting = false; > + > /* nvme backplane slots */ > static const struct slot_table_entry hdd_bay_slots[] = { > SW_PLUGGABLE("hdd0", 0x0), > @@ -91,7 +99,7 @@ static const struct platform_ocapi mihawk_ocapi = { > .ocapi_slot_label = mihawk_ocapi_slot_label, > }; > > -static const struct slot_table_entry P1E1A_x8_PLX8748_down[] = { > +static const struct slot_table_entry P1E1A_x8_PLX8748_RiserA_down[] = > +{ > SW_PLUGGABLE("Slot7", 0x10), > SW_PLUGGABLE("Slot8", 0x8), > SW_PLUGGABLE("Slot10", 0x9), > @@ -99,25 +107,25 @@ static const struct slot_table_entry P1E1A_x8_PLX8748_down[] = { > { .etype = st_end } > }; > > -static const struct slot_table_entry P1E1A_x8_PLX8748_up[] = { > +static const struct slot_table_entry P1E1A_x8_PLX8748_RiserA_up[] = { > { > .etype = st_builtin_dev, > .location = ST_LOC_DEVFN(0,0), > - .children = P1E1A_x8_PLX8748_down, > + .children = P1E1A_x8_PLX8748_RiserA_down, > }, > { .etype = st_end } > }; > > -static const struct slot_table_entry p1phb1_slot[] = { > +static const struct slot_table_entry p1phb1_rA_slot[] = { > { > .etype = st_builtin_dev, > .location = ST_LOC_DEVFN(0,0), > - .children = P1E1A_x8_PLX8748_up, > + .children = P1E1A_x8_PLX8748_RiserA_up, > }, > { .etype = st_end }, > }; > > -static const struct slot_table_entry P0E1A_x8_PLX8748_down[] = { > +static const struct slot_table_entry P0E1A_x8_PLX8748_RiserA_down[] = > +{ > SW_PLUGGABLE("Slot2", 0x10), > SW_PLUGGABLE("Slot3", 0x8), > SW_PLUGGABLE("Slot5", 0x9), > @@ -125,20 +133,120 @@ static const struct slot_table_entry P0E1A_x8_PLX8748_down[] = { > { .etype = st_end } > }; > > -static const struct slot_table_entry P0E1A_x8_PLX8748_up[] = { > +static const struct slot_table_entry P0E1A_x8_PLX8748_RiserA_up[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P0E1A_x8_PLX8748_RiserA_down, > + }, > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry p0phb1_rA_slot[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P0E1A_x8_PLX8748_RiserA_up, > + }, > + { .etype = st_end }, > +}; > + > +static const struct slot_table_entry P1E1A_x8_PLX8748_RiserF_down[] = { > + SW_PLUGGABLE("Slot7", 0x10), > + SW_PLUGGABLE("Slot10", 0x9), > + > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry P1E1A_x8_PLX8748_RiserF_up[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P1E1A_x8_PLX8748_RiserF_down, > + }, > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry p1phb1_rF_slot[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P1E1A_x8_PLX8748_RiserF_up, > + }, > + { .etype = st_end }, > +}; > + > +static const struct slot_table_entry P0E1A_x8_PLX8748_RiserF_down[] = { > + SW_PLUGGABLE("Slot2", 0x10), > + SW_PLUGGABLE("Slot5", 0x9), > + > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry P0E1A_x8_PLX8748_RiserF_up[] = { > { > .etype = st_builtin_dev, > .location = ST_LOC_DEVFN(0,0), > - .children = P0E1A_x8_PLX8748_down, > + .children = P0E1A_x8_PLX8748_RiserF_down, > }, > { .etype = st_end } > }; > > -static const struct slot_table_entry p0phb1_slot[] = { > +static const struct slot_table_entry p0phb1_rF_slot[] = { > { > .etype = st_builtin_dev, > .location = ST_LOC_DEVFN(0,0), > - .children = P0E1A_x8_PLX8748_up, > + .children = P0E1A_x8_PLX8748_RiserF_up, > + }, > + { .etype = st_end }, > +}; > + > +static const struct slot_table_entry P1E2_x16_Switch_down[] = { > + SW_PLUGGABLE("Slot8", 0x1), > + SW_PLUGGABLE("Slot9", 0x0), > + > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry P1E2_x16_Switch_up[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P1E2_x16_Switch_down, > + }, > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry p1phb3_switch_slot[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P1E2_x16_Switch_up, > + }, > + { .etype = st_end }, > +}; > + > +static const struct slot_table_entry P0E2_x16_Switch_down[] = { > + SW_PLUGGABLE("Slot3", 0x1), > + SW_PLUGGABLE("Slot4", 0x0), > + > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry P0E2_x16_Switch_up[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P0E2_x16_Switch_down, > + }, > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry p0phb3_switch_slot[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P0E2_x16_Switch_up, > }, > { .etype = st_end }, You can use the ST_PLUGGABLE() and ST_BUILTIN_DEV() macros to remove most of the struct boilerplate. There's an example of to use them for the upstream port of a switch, etc platforms/qemu/qemu.c. That said, if you use them and need to backport this patch then you'll need to pick cherry-pick bbe5f0038786 ("platforms/astbmc: Add more slot helper macros") too. > }; > @@ -148,22 +256,38 @@ ST_PLUGGABLE(p0phb3_slot, "Slot4"); > ST_PLUGGABLE(p1phb0_slot, "Slot6"); ST_PLUGGABLE(p1phb3_slot, > "Slot9"); > > -static const struct slot_table_entry mihawk_phb_table[] = { > +static const struct slot_table_entry mihawk_riserA_phb_table[] = { > /* ==== CPU0 ==== */ > ST_PHB_ENTRY(0, 0, p0phb0_slot), /* P0E0_x16_Slot1 */ > - ST_PHB_ENTRY(0, 1, p0phb1_slot), /* P0E1A_x8_PLX8748-1_Slot2-3-5 */ > + ST_PHB_ENTRY(0, 1, p0phb1_rA_slot), /* > + P0E1A_x8_PLX8748-1_Slot2-3-5 */ > //ST_PHB_ENTRY(0, 2, p0phb2_slot), /* P0E1B_x8_USBTI7340 */ > ST_PHB_ENTRY(0, 3, p0phb3_slot), /* P0E2_x16_Slot4 */ > > /* ==== CPU1 ==== */ > ST_PHB_ENTRY(8, 0, p1phb0_slot), /* P1E0_x16_Slot6 */ > - ST_PHB_ENTRY(8, 1, p1phb1_slot), /* P1E1A_x8_PLX8748-2_Slot7-8-10 */ > + ST_PHB_ENTRY(8, 1, p1phb1_rA_slot), /* > + P1E1A_x8_PLX8748-2_Slot7-8-10 */ > //ST_PHB_ENTRY(8, 2, p1phb2_slot), /* P1E1B_x8_NA */ > ST_PHB_ENTRY(8, 3, p1phb3_slot), /* P1E2_x16_Slot9 */ > > { .etype = st_end }, > }; > > +static const struct slot_table_entry mihawk_riserF_phb_table[] = { > + /* ==== CPU0 ==== */ > + ST_PHB_ENTRY(0, 0, p0phb0_slot), /* P0E0_x16_Slot1 */ > + ST_PHB_ENTRY(0, 1, p0phb1_rF_slot), /* P0E1A_x8_PLX8748-1_Slot2-5 */ > + //ST_PHB_ENTRY(0, 2, p0phb2_slot), /* P0E1B_x8_USBTI7340 */ > + ST_PHB_ENTRY(0, 3, p0phb3_switch_slot),/* > +P0E2_x16_SWITCH_Slot3-4 */ > + > + /* ==== CPU1 ==== */ > + ST_PHB_ENTRY(8, 0, p1phb0_slot), /* P1E0_x16_Slot6 */ > + ST_PHB_ENTRY(8, 1, p1phb1_rF_slot), /* P1E1A_x8_PLX8748-2_Slot7-10 */ > + //ST_PHB_ENTRY(8, 2, p1phb2_slot), /* P1E1B_x8_NA */ > + ST_PHB_ENTRY(8, 3, p1phb3_switch_slot),/* > + P1E2_x16_SWITCH_Slot8-9 */ > + > + { .etype = st_end }, > +}; > + > #define NPU_BASE 0x5011000 > #define NPU_SIZE 0x2c > #define NPU_INDIRECT0 0x8000000009010c3fUL /* OB0 - no OB3 on Mihawk > */ @@ -282,15 +406,90 @@ static bool mihawk_probe(void) > mihawk_create_npu(); > mihawk_create_ocapi_i2c_bus(); > > - slot_table_init(mihawk_phb_table); > + if (mihawk_riserF_found) > + slot_table_init(mihawk_riserF_phb_table); Is setting the slot table here necessary? It'll be overwritten when mihawk_init() is called later on and I don't think anything will reference the slot table until we start doing PCI probing. > return true; > } > > +static void mihawk_riser_query_complete(struct ipmi_msg *msg) { > + if (msg->cc != IPMI_CC_NO_ERROR) { > + prlog(PR_ERR, "Mihawk: IPMI riser query returned error. cmd=0x%02x," > + " netfn=0x%02x, rc=0x%x\n", msg->cmd, msg->netfn, msg->cc); > + bmc_query_waiting = false; > + ipmi_free_msg(msg); > + return; > + } > + > + prlog(PR_DEBUG, "Mihawk: IPMI Got riser query result. p0:%02x, p1:%02x\n" > + , msg->data[0], msg->data[1]); > + > + uint8_t *riser_state = (uint8_t*)msg->user_data; > + lwsync(); > + *riser_state = msg->data[0] << 4 | msg->data[1]; > + > + bmc_query_waiting = false; > + ipmi_free_msg(msg); > +} > + > +static void mihawk_init(void) > +{ > + struct ipmi_msg *ipmi_msg; > + uint8_t riser_state = 0; > + int timeout_ms = 3000; > + > + astbmc_init(); > + > + /* > + * We use IPMI to ask BMC if Riser-F is installed and set up the > + * corresponding slot table. > + */ > + if (!mihawk_riserF_found) { Is mihawk_riserF_found ever going to be true at this point? Looks like its only ever set below > + ipmi_msg = ipmi_mkmsg(IPMI_DEFAULT_INTERFACE, > + IPMI_RISERF_QUERY, > + mihawk_riser_query_complete, > + &riser_state, NULL, 0, 2); > + > + if (!ipmi_msg) { > + prlog(PR_ERR, "Mihawk: Couldn't create ipmi msg."); > + } > + else { > + ipmi_msg->error = mihawk_riser_query_complete; > + ipmi_queue_msg(ipmi_msg); > + bmc_query_waiting = true; > + > + prlog(PR_DEBUG, "Mihawk: Requesting IPMI_RISERF_QUERY (netfn " > + "%02x, cmd %02x)\n", ipmi_msg->netfn, > + ipmi_msg->cmd); > + > + while (bmc_query_waiting) { > + time_wait_ms(10); > + timeout_ms -= 10; > + > + if (timeout_ms == 0) > + break; > + } > + } Can you use ipmi_queue_msg_sync() instead of the callback and open-coded delay loop? The only problem I can see is that if the BMC doesn't respond we'd hit the timeout in the current code while ipmi_queue_msg_sync() will wait forever. We could always add a timeout to the _sync version though. > + prlog(PR_DEBUG, "Mihawk: IPMI_RISERF_QUERY finish. riser_state: %02x" > + ", waiting: %d\n", riser_state, > + bmc_query_waiting); > + > + if (riser_state != 0) { > + mihawk_riserF_found = true; > + slot_table_init(mihawk_riserF_phb_table); > + prlog(PR_DEBUG, "Mihawk: Detect Riser-F via IPMI\n"); > + } > + else { no newline > + slot_table_init(mihawk_riserA_phb_table); > + prlog(PR_DEBUG, "Mihawk: No Riser-F found, use Riser-A table\n"); > + } > + } > +} > + > DECLARE_PLATFORM(mihawk) = { > .name = "Mihawk", > .probe = mihawk_probe, > - .init = astbmc_init, > + .init = mihawk_init, > .start_preload_resource = flash_start_preload_resource, > .resource_loaded = flash_resource_loaded, > .bmc = &bmc_plat_ast2500_openbmc, > -- > 2.17.1 > > _______________________________________________ > Skiboot mailing list > Skiboot@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/skiboot
Hi Oliver, As in your previous comment: ""Can you use ipmi_queue_msg_sync() instead of the callback and open-coded delay loop? The only problem I can see is that if the BMC doesn't respond we'd hit the timeout in the current code while ipmi_queue_msg_sync() will wait forever. We could always add a timeout to the _sync version though."" I still have no idea about how to add the timeout when using ipmi_queue_msg_sync(). Would you please give us some hints? Best Regards, Joy Chu -----Original Message----- From: Joy Chu/WHQ/Wistron Sent: Tuesday, March 3, 2020 4:15 PM To: 'Oliver O'Halloran' <oohall@gmail.com> Cc: skiboot list <skiboot@lists.ozlabs.org>; Stewart Smith <stewart@linux.vnet.ibm.com>; Oliver OHalloran <oliveroh@au1.ibm.com>; chhank@tw.ibm.com; chcyjoy@gmail.com Subject: RE: [Skiboot] [PATCH] platform/mihawk: support dynamic PCIe slot table Hi Oliver, The slot helper macros example from qemu looks more simpler and clearer. We'll take this commit for consideration. To use ipmi_queue_msg_sync() with timeout, would you please share some example? We want to add the timeout to prevent from the long waiting and the increase of boot-up time. Best Regards, Joy Chu -----Original Message----- From: Oliver O'Halloran <oohall@gmail.com> Sent: Wednesday, February 26, 2020 12:23 PM To: Joy Chu/WHQ/Wistron <Joy_Chu@wistron.com> Cc: skiboot list <skiboot@lists.ozlabs.org>; Stewart Smith <stewart@linux.vnet.ibm.com>; Oliver OHalloran <oliveroh@au1.ibm.com>; chhank@tw.ibm.com; chcyjoy@gmail.com Subject: Re: [Skiboot] [PATCH] platform/mihawk: support dynamic PCIe slot table On Wed, Feb 26, 2020 at 12:57 PM Joy Chu <joy_chu@wistron.com> wrote: > > Slot table auto-detection for different riser cards by using IPMI OEM > command to communicate with BMC. > > Signed-off-by: Joy Chu <joy_chu@wistron.com> > --- > platforms/astbmc/mihawk.c | 229 > +++++++++++++++++++++++++++++++++++--- > 1 file changed, 214 insertions(+), 15 deletions(-) > > diff --git a/platforms/astbmc/mihawk.c b/platforms/astbmc/mihawk.c > index d33d16bb..28c999aa 100644 > --- a/platforms/astbmc/mihawk.c > +++ b/platforms/astbmc/mihawk.c > @@ -15,8 +15,16 @@ > #include <pci.h> > #include <pci-cfg.h> > > +#include <timebase.h> > + > #include "astbmc.h" > > +/* IPMI message code for Riser-F query (OEM). */ > +#define IPMI_RISERF_QUERY IPMI_CODE(0x32, 0x01) > + > +static bool mihawk_riserF_found = false; static bool > +bmc_query_waiting = false; > + > /* nvme backplane slots */ > static const struct slot_table_entry hdd_bay_slots[] = { > SW_PLUGGABLE("hdd0", 0x0), > @@ -91,7 +99,7 @@ static const struct platform_ocapi mihawk_ocapi = { > .ocapi_slot_label = mihawk_ocapi_slot_label, > }; > > -static const struct slot_table_entry P1E1A_x8_PLX8748_down[] = { > +static const struct slot_table_entry P1E1A_x8_PLX8748_RiserA_down[] = > +{ > SW_PLUGGABLE("Slot7", 0x10), > SW_PLUGGABLE("Slot8", 0x8), > SW_PLUGGABLE("Slot10", 0x9), > @@ -99,25 +107,25 @@ static const struct slot_table_entry P1E1A_x8_PLX8748_down[] = { > { .etype = st_end } > }; > > -static const struct slot_table_entry P1E1A_x8_PLX8748_up[] = { > +static const struct slot_table_entry P1E1A_x8_PLX8748_RiserA_up[] = { > { > .etype = st_builtin_dev, > .location = ST_LOC_DEVFN(0,0), > - .children = P1E1A_x8_PLX8748_down, > + .children = P1E1A_x8_PLX8748_RiserA_down, > }, > { .etype = st_end } > }; > > -static const struct slot_table_entry p1phb1_slot[] = { > +static const struct slot_table_entry p1phb1_rA_slot[] = { > { > .etype = st_builtin_dev, > .location = ST_LOC_DEVFN(0,0), > - .children = P1E1A_x8_PLX8748_up, > + .children = P1E1A_x8_PLX8748_RiserA_up, > }, > { .etype = st_end }, > }; > > -static const struct slot_table_entry P0E1A_x8_PLX8748_down[] = { > +static const struct slot_table_entry P0E1A_x8_PLX8748_RiserA_down[] = > +{ > SW_PLUGGABLE("Slot2", 0x10), > SW_PLUGGABLE("Slot3", 0x8), > SW_PLUGGABLE("Slot5", 0x9), > @@ -125,20 +133,120 @@ static const struct slot_table_entry P0E1A_x8_PLX8748_down[] = { > { .etype = st_end } > }; > > -static const struct slot_table_entry P0E1A_x8_PLX8748_up[] = { > +static const struct slot_table_entry P0E1A_x8_PLX8748_RiserA_up[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P0E1A_x8_PLX8748_RiserA_down, > + }, > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry p0phb1_rA_slot[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P0E1A_x8_PLX8748_RiserA_up, > + }, > + { .etype = st_end }, > +}; > + > +static const struct slot_table_entry P1E1A_x8_PLX8748_RiserF_down[] = { > + SW_PLUGGABLE("Slot7", 0x10), > + SW_PLUGGABLE("Slot10", 0x9), > + > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry P1E1A_x8_PLX8748_RiserF_up[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P1E1A_x8_PLX8748_RiserF_down, > + }, > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry p1phb1_rF_slot[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P1E1A_x8_PLX8748_RiserF_up, > + }, > + { .etype = st_end }, > +}; > + > +static const struct slot_table_entry P0E1A_x8_PLX8748_RiserF_down[] = { > + SW_PLUGGABLE("Slot2", 0x10), > + SW_PLUGGABLE("Slot5", 0x9), > + > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry P0E1A_x8_PLX8748_RiserF_up[] = { > { > .etype = st_builtin_dev, > .location = ST_LOC_DEVFN(0,0), > - .children = P0E1A_x8_PLX8748_down, > + .children = P0E1A_x8_PLX8748_RiserF_down, > }, > { .etype = st_end } > }; > > -static const struct slot_table_entry p0phb1_slot[] = { > +static const struct slot_table_entry p0phb1_rF_slot[] = { > { > .etype = st_builtin_dev, > .location = ST_LOC_DEVFN(0,0), > - .children = P0E1A_x8_PLX8748_up, > + .children = P0E1A_x8_PLX8748_RiserF_up, > + }, > + { .etype = st_end }, > +}; > + > +static const struct slot_table_entry P1E2_x16_Switch_down[] = { > + SW_PLUGGABLE("Slot8", 0x1), > + SW_PLUGGABLE("Slot9", 0x0), > + > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry P1E2_x16_Switch_up[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P1E2_x16_Switch_down, > + }, > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry p1phb3_switch_slot[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P1E2_x16_Switch_up, > + }, > + { .etype = st_end }, > +}; > + > +static const struct slot_table_entry P0E2_x16_Switch_down[] = { > + SW_PLUGGABLE("Slot3", 0x1), > + SW_PLUGGABLE("Slot4", 0x0), > + > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry P0E2_x16_Switch_up[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P0E2_x16_Switch_down, > + }, > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry p0phb3_switch_slot[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P0E2_x16_Switch_up, > }, > { .etype = st_end }, You can use the ST_PLUGGABLE() and ST_BUILTIN_DEV() macros to remove most of the struct boilerplate. There's an example of to use them for the upstream port of a switch, etc platforms/qemu/qemu.c. That said, if you use them and need to backport this patch then you'll need to pick cherry-pick bbe5f0038786 ("platforms/astbmc: Add more slot helper macros") too. > }; > @@ -148,22 +256,38 @@ ST_PLUGGABLE(p0phb3_slot, "Slot4"); > ST_PLUGGABLE(p1phb0_slot, "Slot6"); ST_PLUGGABLE(p1phb3_slot, > "Slot9"); > > -static const struct slot_table_entry mihawk_phb_table[] = { > +static const struct slot_table_entry mihawk_riserA_phb_table[] = { > /* ==== CPU0 ==== */ > ST_PHB_ENTRY(0, 0, p0phb0_slot), /* P0E0_x16_Slot1 */ > - ST_PHB_ENTRY(0, 1, p0phb1_slot), /* P0E1A_x8_PLX8748-1_Slot2-3-5 */ > + ST_PHB_ENTRY(0, 1, p0phb1_rA_slot), /* > + P0E1A_x8_PLX8748-1_Slot2-3-5 */ > //ST_PHB_ENTRY(0, 2, p0phb2_slot), /* P0E1B_x8_USBTI7340 */ > ST_PHB_ENTRY(0, 3, p0phb3_slot), /* P0E2_x16_Slot4 */ > > /* ==== CPU1 ==== */ > ST_PHB_ENTRY(8, 0, p1phb0_slot), /* P1E0_x16_Slot6 */ > - ST_PHB_ENTRY(8, 1, p1phb1_slot), /* P1E1A_x8_PLX8748-2_Slot7-8-10 */ > + ST_PHB_ENTRY(8, 1, p1phb1_rA_slot), /* > + P1E1A_x8_PLX8748-2_Slot7-8-10 */ > //ST_PHB_ENTRY(8, 2, p1phb2_slot), /* P1E1B_x8_NA */ > ST_PHB_ENTRY(8, 3, p1phb3_slot), /* P1E2_x16_Slot9 */ > > { .etype = st_end }, > }; > > +static const struct slot_table_entry mihawk_riserF_phb_table[] = { > + /* ==== CPU0 ==== */ > + ST_PHB_ENTRY(0, 0, p0phb0_slot), /* P0E0_x16_Slot1 */ > + ST_PHB_ENTRY(0, 1, p0phb1_rF_slot), /* P0E1A_x8_PLX8748-1_Slot2-5 */ > + //ST_PHB_ENTRY(0, 2, p0phb2_slot), /* P0E1B_x8_USBTI7340 */ > + ST_PHB_ENTRY(0, 3, p0phb3_switch_slot),/* > +P0E2_x16_SWITCH_Slot3-4 */ > + > + /* ==== CPU1 ==== */ > + ST_PHB_ENTRY(8, 0, p1phb0_slot), /* P1E0_x16_Slot6 */ > + ST_PHB_ENTRY(8, 1, p1phb1_rF_slot), /* P1E1A_x8_PLX8748-2_Slot7-10 */ > + //ST_PHB_ENTRY(8, 2, p1phb2_slot), /* P1E1B_x8_NA */ > + ST_PHB_ENTRY(8, 3, p1phb3_switch_slot),/* > + P1E2_x16_SWITCH_Slot8-9 */ > + > + { .etype = st_end }, > +}; > + > #define NPU_BASE 0x5011000 > #define NPU_SIZE 0x2c > #define NPU_INDIRECT0 0x8000000009010c3fUL /* OB0 - no OB3 on Mihawk > */ @@ -282,15 +406,90 @@ static bool mihawk_probe(void) > mihawk_create_npu(); > mihawk_create_ocapi_i2c_bus(); > > - slot_table_init(mihawk_phb_table); > + if (mihawk_riserF_found) > + slot_table_init(mihawk_riserF_phb_table); Is setting the slot table here necessary? It'll be overwritten when mihawk_init() is called later on and I don't think anything will reference the slot table until we start doing PCI probing. > return true; > } > > +static void mihawk_riser_query_complete(struct ipmi_msg *msg) { > + if (msg->cc != IPMI_CC_NO_ERROR) { > + prlog(PR_ERR, "Mihawk: IPMI riser query returned error. cmd=0x%02x," > + " netfn=0x%02x, rc=0x%x\n", msg->cmd, msg->netfn, msg->cc); > + bmc_query_waiting = false; > + ipmi_free_msg(msg); > + return; > + } > + > + prlog(PR_DEBUG, "Mihawk: IPMI Got riser query result. p0:%02x, p1:%02x\n" > + , msg->data[0], msg->data[1]); > + > + uint8_t *riser_state = (uint8_t*)msg->user_data; > + lwsync(); > + *riser_state = msg->data[0] << 4 | msg->data[1]; > + > + bmc_query_waiting = false; > + ipmi_free_msg(msg); > +} > + > +static void mihawk_init(void) > +{ > + struct ipmi_msg *ipmi_msg; > + uint8_t riser_state = 0; > + int timeout_ms = 3000; > + > + astbmc_init(); > + > + /* > + * We use IPMI to ask BMC if Riser-F is installed and set up the > + * corresponding slot table. > + */ > + if (!mihawk_riserF_found) { Is mihawk_riserF_found ever going to be true at this point? Looks like its only ever set below > + ipmi_msg = ipmi_mkmsg(IPMI_DEFAULT_INTERFACE, > + IPMI_RISERF_QUERY, > + mihawk_riser_query_complete, > + &riser_state, NULL, 0, 2); > + > + if (!ipmi_msg) { > + prlog(PR_ERR, "Mihawk: Couldn't create ipmi msg."); > + } > + else { > + ipmi_msg->error = mihawk_riser_query_complete; > + ipmi_queue_msg(ipmi_msg); > + bmc_query_waiting = true; > + > + prlog(PR_DEBUG, "Mihawk: Requesting IPMI_RISERF_QUERY (netfn " > + "%02x, cmd %02x)\n", ipmi_msg->netfn, > + ipmi_msg->cmd); > + > + while (bmc_query_waiting) { > + time_wait_ms(10); > + timeout_ms -= 10; > + > + if (timeout_ms == 0) > + break; > + } > + } Can you use ipmi_queue_msg_sync() instead of the callback and open-coded delay loop? The only problem I can see is that if the BMC doesn't respond we'd hit the timeout in the current code while ipmi_queue_msg_sync() will wait forever. We could always add a timeout to the _sync version though. > + prlog(PR_DEBUG, "Mihawk: IPMI_RISERF_QUERY finish. riser_state: %02x" > + ", waiting: %d\n", riser_state, > + bmc_query_waiting); > + > + if (riser_state != 0) { > + mihawk_riserF_found = true; > + slot_table_init(mihawk_riserF_phb_table); > + prlog(PR_DEBUG, "Mihawk: Detect Riser-F via IPMI\n"); > + } > + else { no newline > + slot_table_init(mihawk_riserA_phb_table); > + prlog(PR_DEBUG, "Mihawk: No Riser-F found, use Riser-A table\n"); > + } > + } > +} > + > DECLARE_PLATFORM(mihawk) = { > .name = "Mihawk", > .probe = mihawk_probe, > - .init = astbmc_init, > + .init = mihawk_init, > .start_preload_resource = flash_start_preload_resource, > .resource_loaded = flash_resource_loaded, > .bmc = &bmc_plat_ast2500_openbmc, > -- > 2.17.1 > > _______________________________________________ > Skiboot mailing list > Skiboot@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/skiboot
Send my question again as below. > > Slot table auto-detection for different riser cards by using IPMI OEM > command to communicate with BMC. > > Signed-off-by: Joy Chu <joy_chu@wistron.com> > --- > platforms/astbmc/mihawk.c | 229 > +++++++++++++++++++++++++++++++++++--- > 1 file changed, 214 insertions(+), 15 deletions(-) > > diff --git a/platforms/astbmc/mihawk.c b/platforms/astbmc/mihawk.c > index d33d16bb..28c999aa 100644 > --- a/platforms/astbmc/mihawk.c > +++ b/platforms/astbmc/mihawk.c > @@ -15,8 +15,16 @@ > #include <pci.h> > #include <pci-cfg.h> > > +#include <timebase.h> > + > #include "astbmc.h" > > +/* IPMI message code for Riser-F query (OEM). */ > +#define IPMI_RISERF_QUERY IPMI_CODE(0x32, 0x01) > + > +static bool mihawk_riserF_found = false; static bool > +bmc_query_waiting = false; > + > /* nvme backplane slots */ > static const struct slot_table_entry hdd_bay_slots[] = { > SW_PLUGGABLE("hdd0", 0x0), > @@ -91,7 +99,7 @@ static const struct platform_ocapi mihawk_ocapi = { > .ocapi_slot_label = mihawk_ocapi_slot_label, > }; > > -static const struct slot_table_entry P1E1A_x8_PLX8748_down[] = { > +static const struct slot_table_entry P1E1A_x8_PLX8748_RiserA_down[] = > +{ > SW_PLUGGABLE("Slot7", 0x10), > SW_PLUGGABLE("Slot8", 0x8), > SW_PLUGGABLE("Slot10", 0x9), > @@ -99,25 +107,25 @@ static const struct slot_table_entry P1E1A_x8_PLX8748_down[] = { > { .etype = st_end } > }; > > -static const struct slot_table_entry P1E1A_x8_PLX8748_up[] = { > +static const struct slot_table_entry P1E1A_x8_PLX8748_RiserA_up[] = { > { > .etype = st_builtin_dev, > .location = ST_LOC_DEVFN(0,0), > - .children = P1E1A_x8_PLX8748_down, > + .children = P1E1A_x8_PLX8748_RiserA_down, > }, > { .etype = st_end } > }; > > -static const struct slot_table_entry p1phb1_slot[] = { > +static const struct slot_table_entry p1phb1_rA_slot[] = { > { > .etype = st_builtin_dev, > .location = ST_LOC_DEVFN(0,0), > - .children = P1E1A_x8_PLX8748_up, > + .children = P1E1A_x8_PLX8748_RiserA_up, > }, > { .etype = st_end }, > }; > > -static const struct slot_table_entry P0E1A_x8_PLX8748_down[] = { > +static const struct slot_table_entry P0E1A_x8_PLX8748_RiserA_down[] = > +{ > SW_PLUGGABLE("Slot2", 0x10), > SW_PLUGGABLE("Slot3", 0x8), > SW_PLUGGABLE("Slot5", 0x9), > @@ -125,20 +133,120 @@ static const struct slot_table_entry P0E1A_x8_PLX8748_down[] = { > { .etype = st_end } > }; > > -static const struct slot_table_entry P0E1A_x8_PLX8748_up[] = { > +static const struct slot_table_entry P0E1A_x8_PLX8748_RiserA_up[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P0E1A_x8_PLX8748_RiserA_down, > + }, > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry p0phb1_rA_slot[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P0E1A_x8_PLX8748_RiserA_up, > + }, > + { .etype = st_end }, > +}; > + > +static const struct slot_table_entry P1E1A_x8_PLX8748_RiserF_down[] = { > + SW_PLUGGABLE("Slot7", 0x10), > + SW_PLUGGABLE("Slot10", 0x9), > + > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry P1E1A_x8_PLX8748_RiserF_up[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P1E1A_x8_PLX8748_RiserF_down, > + }, > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry p1phb1_rF_slot[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P1E1A_x8_PLX8748_RiserF_up, > + }, > + { .etype = st_end }, > +}; > + > +static const struct slot_table_entry P0E1A_x8_PLX8748_RiserF_down[] = { > + SW_PLUGGABLE("Slot2", 0x10), > + SW_PLUGGABLE("Slot5", 0x9), > + > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry P0E1A_x8_PLX8748_RiserF_up[] = { > { > .etype = st_builtin_dev, > .location = ST_LOC_DEVFN(0,0), > - .children = P0E1A_x8_PLX8748_down, > + .children = P0E1A_x8_PLX8748_RiserF_down, > }, > { .etype = st_end } > }; > > -static const struct slot_table_entry p0phb1_slot[] = { > +static const struct slot_table_entry p0phb1_rF_slot[] = { > { > .etype = st_builtin_dev, > .location = ST_LOC_DEVFN(0,0), > - .children = P0E1A_x8_PLX8748_up, > + .children = P0E1A_x8_PLX8748_RiserF_up, > + }, > + { .etype = st_end }, > +}; > + > +static const struct slot_table_entry P1E2_x16_Switch_down[] = { > + SW_PLUGGABLE("Slot8", 0x1), > + SW_PLUGGABLE("Slot9", 0x0), > + > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry P1E2_x16_Switch_up[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P1E2_x16_Switch_down, > + }, > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry p1phb3_switch_slot[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P1E2_x16_Switch_up, > + }, > + { .etype = st_end }, > +}; > + > +static const struct slot_table_entry P0E2_x16_Switch_down[] = { > + SW_PLUGGABLE("Slot3", 0x1), > + SW_PLUGGABLE("Slot4", 0x0), > + > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry P0E2_x16_Switch_up[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P0E2_x16_Switch_down, > + }, > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry p0phb3_switch_slot[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P0E2_x16_Switch_up, > }, > { .etype = st_end }, You can use the ST_PLUGGABLE() and ST_BUILTIN_DEV() macros to remove most of the struct boilerplate. There's an example of to use them for the upstream port of a switch, etc platforms/qemu/qemu.c. That said, if you use them and need to backport this patch then you'll need to pick cherry-pick bbe5f0038786 ("platforms/astbmc: Add more slot helper macros") too. Joy: The slot helper macros example from qemu looks more simpler and clearer. We'll take this commit for consideration. > }; > @@ -148,22 +256,38 @@ ST_PLUGGABLE(p0phb3_slot, "Slot4"); > ST_PLUGGABLE(p1phb0_slot, "Slot6"); ST_PLUGGABLE(p1phb3_slot, > "Slot9"); > > -static const struct slot_table_entry mihawk_phb_table[] = { > +static const struct slot_table_entry mihawk_riserA_phb_table[] = { > /* ==== CPU0 ==== */ > ST_PHB_ENTRY(0, 0, p0phb0_slot), /* P0E0_x16_Slot1 */ > - ST_PHB_ENTRY(0, 1, p0phb1_slot), /* P0E1A_x8_PLX8748-1_Slot2-3-5 */ > + ST_PHB_ENTRY(0, 1, p0phb1_rA_slot), /* > + P0E1A_x8_PLX8748-1_Slot2-3-5 */ > //ST_PHB_ENTRY(0, 2, p0phb2_slot), /* P0E1B_x8_USBTI7340 */ > ST_PHB_ENTRY(0, 3, p0phb3_slot), /* P0E2_x16_Slot4 */ > > /* ==== CPU1 ==== */ > ST_PHB_ENTRY(8, 0, p1phb0_slot), /* P1E0_x16_Slot6 */ > - ST_PHB_ENTRY(8, 1, p1phb1_slot), /* P1E1A_x8_PLX8748-2_Slot7-8-10 */ > + ST_PHB_ENTRY(8, 1, p1phb1_rA_slot), /* > + P1E1A_x8_PLX8748-2_Slot7-8-10 */ > //ST_PHB_ENTRY(8, 2, p1phb2_slot), /* P1E1B_x8_NA */ > ST_PHB_ENTRY(8, 3, p1phb3_slot), /* P1E2_x16_Slot9 */ > > { .etype = st_end }, > }; > > +static const struct slot_table_entry mihawk_riserF_phb_table[] = { > + /* ==== CPU0 ==== */ > + ST_PHB_ENTRY(0, 0, p0phb0_slot), /* P0E0_x16_Slot1 */ > + ST_PHB_ENTRY(0, 1, p0phb1_rF_slot), /* P0E1A_x8_PLX8748-1_Slot2-5 */ > + //ST_PHB_ENTRY(0, 2, p0phb2_slot), /* P0E1B_x8_USBTI7340 */ > + ST_PHB_ENTRY(0, 3, p0phb3_switch_slot),/* > +P0E2_x16_SWITCH_Slot3-4 */ > + > + /* ==== CPU1 ==== */ > + ST_PHB_ENTRY(8, 0, p1phb0_slot), /* P1E0_x16_Slot6 */ > + ST_PHB_ENTRY(8, 1, p1phb1_rF_slot), /* P1E1A_x8_PLX8748-2_Slot7-10 */ > + //ST_PHB_ENTRY(8, 2, p1phb2_slot), /* P1E1B_x8_NA */ > + ST_PHB_ENTRY(8, 3, p1phb3_switch_slot),/* > + P1E2_x16_SWITCH_Slot8-9 */ > + > + { .etype = st_end }, > +}; > + > #define NPU_BASE 0x5011000 > #define NPU_SIZE 0x2c > #define NPU_INDIRECT0 0x8000000009010c3fUL /* OB0 - no OB3 on Mihawk > */ @@ -282,15 +406,90 @@ static bool mihawk_probe(void) > mihawk_create_npu(); > mihawk_create_ocapi_i2c_bus(); > > - slot_table_init(mihawk_phb_table); > + if (mihawk_riserF_found) > + slot_table_init(mihawk_riserF_phb_table); Is setting the slot table here necessary? It'll be overwritten when mihawk_init() is called later on and I don't think anything will reference the slot table until we start doing PCI probing. Joy: I'll remove this. > return true; > } > > +static void mihawk_riser_query_complete(struct ipmi_msg *msg) { > + if (msg->cc != IPMI_CC_NO_ERROR) { > + prlog(PR_ERR, "Mihawk: IPMI riser query returned error. cmd=0x%02x," > + " netfn=0x%02x, rc=0x%x\n", msg->cmd, msg->netfn, msg->cc); > + bmc_query_waiting = false; > + ipmi_free_msg(msg); > + return; > + } > + > + prlog(PR_DEBUG, "Mihawk: IPMI Got riser query result. p0:%02x, p1:%02x\n" > + , msg->data[0], msg->data[1]); > + > + uint8_t *riser_state = (uint8_t*)msg->user_data; > + lwsync(); > + *riser_state = msg->data[0] << 4 | msg->data[1]; > + > + bmc_query_waiting = false; > + ipmi_free_msg(msg); > +} > + > +static void mihawk_init(void) > +{ > + struct ipmi_msg *ipmi_msg; > + uint8_t riser_state = 0; > + int timeout_ms = 3000; > + > + astbmc_init(); > + > + /* > + * We use IPMI to ask BMC if Riser-F is installed and set up the > + * corresponding slot table. > + */ > + if (!mihawk_riserF_found) { Is mihawk_riserF_found ever going to be true at this point? Looks like its only ever set below Joy: The value check of mihawk_riserF_found is necessary. I'll remove this. > + ipmi_msg = ipmi_mkmsg(IPMI_DEFAULT_INTERFACE, > + IPMI_RISERF_QUERY, > + mihawk_riser_query_complete, > + &riser_state, NULL, 0, 2); > + > + if (!ipmi_msg) { > + prlog(PR_ERR, "Mihawk: Couldn't create ipmi msg."); > + } > + else { > + ipmi_msg->error = mihawk_riser_query_complete; > + ipmi_queue_msg(ipmi_msg); > + bmc_query_waiting = true; > + > + prlog(PR_DEBUG, "Mihawk: Requesting IPMI_RISERF_QUERY (netfn " > + "%02x, cmd %02x)\n", ipmi_msg->netfn, > + ipmi_msg->cmd); > + > + while (bmc_query_waiting) { > + time_wait_ms(10); > + timeout_ms -= 10; > + > + if (timeout_ms == 0) > + break; > + } > + } Can you use ipmi_queue_msg_sync() instead of the callback and open-coded delay loop? The only problem I can see is that if the BMC doesn't respond we'd hit the timeout in the current code while ipmi_queue_msg_sync() will wait forever. We could always add a timeout to the _sync version though. Joy: To use ipmi_queue_msg_sync() with timeout, would you please share some example? We want to add the timeout to prevent from the long waiting and the increase of boot-up time. I still have no idea about how to add the timeout when using ipmi_queue_msg_sync(). Would you please give us some hints? > + prlog(PR_DEBUG, "Mihawk: IPMI_RISERF_QUERY finish. riser_state: %02x" > + ", waiting: %d\n", riser_state, > + bmc_query_waiting); > + > + if (riser_state != 0) { > + mihawk_riserF_found = true; > + slot_table_init(mihawk_riserF_phb_table); > + prlog(PR_DEBUG, "Mihawk: Detect Riser-F via IPMI\n"); > + } > + else { no newline > + slot_table_init(mihawk_riserA_phb_table); > + prlog(PR_DEBUG, "Mihawk: No Riser-F found, use Riser-A table\n"); > + } > + } > +} > + > DECLARE_PLATFORM(mihawk) = { > .name = "Mihawk", > .probe = mihawk_probe, > - .init = astbmc_init, > + .init = mihawk_init, > .start_preload_resource = flash_start_preload_resource, > .resource_loaded = flash_resource_loaded, > .bmc = &bmc_plat_ast2500_openbmc, > -- > 2.17.1 > > _______________________________________________ > Skiboot mailing list > Skiboot@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/skiboot
Send my question again as below and modify the typo from previous mail. > > Slot table auto-detection for different riser cards by using IPMI OEM > command to communicate with BMC. > > Signed-off-by: Joy Chu <joy_chu@wistron.com<mailto:joy_chu@wistron.com>> > --- > platforms/astbmc/mihawk.c | 229 > +++++++++++++++++++++++++++++++++++--- > 1 file changed, 214 insertions(+), 15 deletions(-) > > diff --git a/platforms/astbmc/mihawk.c b/platforms/astbmc/mihawk.c > index d33d16bb..28c999aa 100644 > --- a/platforms/astbmc/mihawk.c > +++ b/platforms/astbmc/mihawk.c > @@ -15,8 +15,16 @@ > #include <pci.h> > #include <pci-cfg.h> > > +#include <timebase.h> > + > #include "astbmc.h" > > +/* IPMI message code for Riser-F query (OEM). */ > +#define IPMI_RISERF_QUERY IPMI_CODE(0x32, 0x01) > + > +static bool mihawk_riserF_found = false; static bool > +bmc_query_waiting = false; > + > /* nvme backplane slots */ > static const struct slot_table_entry hdd_bay_slots[] = { > SW_PLUGGABLE("hdd0", 0x0), > @@ -91,7 +99,7 @@ static const struct platform_ocapi mihawk_ocapi = { > .ocapi_slot_label = mihawk_ocapi_slot_label, > }; > > -static const struct slot_table_entry P1E1A_x8_PLX8748_down[] = { > +static const struct slot_table_entry P1E1A_x8_PLX8748_RiserA_down[] = > +{ > SW_PLUGGABLE("Slot7", 0x10), > SW_PLUGGABLE("Slot8", 0x8), > SW_PLUGGABLE("Slot10", 0x9), > @@ -99,25 +107,25 @@ static const struct slot_table_entry P1E1A_x8_PLX8748_down[] = { > { .etype = st_end } > }; > > -static const struct slot_table_entry P1E1A_x8_PLX8748_up[] = { > +static const struct slot_table_entry P1E1A_x8_PLX8748_RiserA_up[] = { > { > .etype = st_builtin_dev, > .location = ST_LOC_DEVFN(0,0), > - .children = P1E1A_x8_PLX8748_down, > + .children = P1E1A_x8_PLX8748_RiserA_down, > }, > { .etype = st_end } > }; > > -static const struct slot_table_entry p1phb1_slot[] = { > +static const struct slot_table_entry p1phb1_rA_slot[] = { > { > .etype = st_builtin_dev, > .location = ST_LOC_DEVFN(0,0), > - .children = P1E1A_x8_PLX8748_up, > + .children = P1E1A_x8_PLX8748_RiserA_up, > }, > { .etype = st_end }, > }; > > -static const struct slot_table_entry P0E1A_x8_PLX8748_down[] = { > +static const struct slot_table_entry P0E1A_x8_PLX8748_RiserA_down[] = > +{ > SW_PLUGGABLE("Slot2", 0x10), > SW_PLUGGABLE("Slot3", 0x8), > SW_PLUGGABLE("Slot5", 0x9), > @@ -125,20 +133,120 @@ static const struct slot_table_entry P0E1A_x8_PLX8748_down[] = { > { .etype = st_end } > }; > > -static const struct slot_table_entry P0E1A_x8_PLX8748_up[] = { > +static const struct slot_table_entry P0E1A_x8_PLX8748_RiserA_up[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P0E1A_x8_PLX8748_RiserA_down, > + }, > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry p0phb1_rA_slot[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P0E1A_x8_PLX8748_RiserA_up, > + }, > + { .etype = st_end }, > +}; > + > +static const struct slot_table_entry P1E1A_x8_PLX8748_RiserF_down[] = { > + SW_PLUGGABLE("Slot7", 0x10), > + SW_PLUGGABLE("Slot10", 0x9), > + > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry P1E1A_x8_PLX8748_RiserF_up[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P1E1A_x8_PLX8748_RiserF_down, > + }, > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry p1phb1_rF_slot[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P1E1A_x8_PLX8748_RiserF_up, > + }, > + { .etype = st_end }, > +}; > + > +static const struct slot_table_entry P0E1A_x8_PLX8748_RiserF_down[] = { > + SW_PLUGGABLE("Slot2", 0x10), > + SW_PLUGGABLE("Slot5", 0x9), > + > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry P0E1A_x8_PLX8748_RiserF_up[] = { > { > .etype = st_builtin_dev, > .location = ST_LOC_DEVFN(0,0), > - .children = P0E1A_x8_PLX8748_down, > + .children = P0E1A_x8_PLX8748_RiserF_down, > }, > { .etype = st_end } > }; > > -static const struct slot_table_entry p0phb1_slot[] = { > +static const struct slot_table_entry p0phb1_rF_slot[] = { > { > .etype = st_builtin_dev, > .location = ST_LOC_DEVFN(0,0), > - .children = P0E1A_x8_PLX8748_up, > + .children = P0E1A_x8_PLX8748_RiserF_up, > + }, > + { .etype = st_end }, > +}; > + > +static const struct slot_table_entry P1E2_x16_Switch_down[] = { > + SW_PLUGGABLE("Slot8", 0x1), > + SW_PLUGGABLE("Slot9", 0x0), > + > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry P1E2_x16_Switch_up[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P1E2_x16_Switch_down, > + }, > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry p1phb3_switch_slot[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P1E2_x16_Switch_up, > + }, > + { .etype = st_end }, > +}; > + > +static const struct slot_table_entry P0E2_x16_Switch_down[] = { > + SW_PLUGGABLE("Slot3", 0x1), > + SW_PLUGGABLE("Slot4", 0x0), > + > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry P0E2_x16_Switch_up[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P0E2_x16_Switch_down, > + }, > + { .etype = st_end } > +}; > + > +static const struct slot_table_entry p0phb3_switch_slot[] = { > + { > + .etype = st_builtin_dev, > + .location = ST_LOC_DEVFN(0,0), > + .children = P0E2_x16_Switch_up, > }, > { .etype = st_end }, You can use the ST_PLUGGABLE() and ST_BUILTIN_DEV() macros to remove most of the struct boilerplate. There's an example of to use them for the upstream port of a switch, etc platforms/qemu/qemu.c. That said, if you use them and need to backport this patch then you'll need to pick cherry-pick bbe5f0038786 ("platforms/astbmc: Add more slot helper macros") too. Joy: The slot helper macros example from qemu looks more simpler and clearer. We'll take this commit for consideration. > }; > @@ -148,22 +256,38 @@ ST_PLUGGABLE(p0phb3_slot, "Slot4"); > ST_PLUGGABLE(p1phb0_slot, "Slot6"); ST_PLUGGABLE(p1phb3_slot, > "Slot9"); > > -static const struct slot_table_entry mihawk_phb_table[] = { > +static const struct slot_table_entry mihawk_riserA_phb_table[] = { > /* ==== CPU0 ==== */ > ST_PHB_ENTRY(0, 0, p0phb0_slot), /* P0E0_x16_Slot1 */ > - ST_PHB_ENTRY(0, 1, p0phb1_slot), /* P0E1A_x8_PLX8748-1_Slot2-3-5 */ > + ST_PHB_ENTRY(0, 1, p0phb1_rA_slot), /* > + P0E1A_x8_PLX8748-1_Slot2-3-5 */ > //ST_PHB_ENTRY(0, 2, p0phb2_slot), /* P0E1B_x8_USBTI7340 */ > ST_PHB_ENTRY(0, 3, p0phb3_slot), /* P0E2_x16_Slot4 */ > > /* ==== CPU1 ==== */ > ST_PHB_ENTRY(8, 0, p1phb0_slot), /* P1E0_x16_Slot6 */ > - ST_PHB_ENTRY(8, 1, p1phb1_slot), /* P1E1A_x8_PLX8748-2_Slot7-8-10 */ > + ST_PHB_ENTRY(8, 1, p1phb1_rA_slot), /* > + P1E1A_x8_PLX8748-2_Slot7-8-10 */ > //ST_PHB_ENTRY(8, 2, p1phb2_slot), /* P1E1B_x8_NA */ > ST_PHB_ENTRY(8, 3, p1phb3_slot), /* P1E2_x16_Slot9 */ > > { .etype = st_end }, > }; > > +static const struct slot_table_entry mihawk_riserF_phb_table[] = { > + /* ==== CPU0 ==== */ > + ST_PHB_ENTRY(0, 0, p0phb0_slot), /* P0E0_x16_Slot1 */ > + ST_PHB_ENTRY(0, 1, p0phb1_rF_slot), /* P0E1A_x8_PLX8748-1_Slot2-5 */ > + //ST_PHB_ENTRY(0, 2, p0phb2_slot), /* P0E1B_x8_USBTI7340 */ > + ST_PHB_ENTRY(0, 3, p0phb3_switch_slot),/* > +P0E2_x16_SWITCH_Slot3-4 */ > + > + /* ==== CPU1 ==== */ > + ST_PHB_ENTRY(8, 0, p1phb0_slot), /* P1E0_x16_Slot6 */ > + ST_PHB_ENTRY(8, 1, p1phb1_rF_slot), /* P1E1A_x8_PLX8748-2_Slot7-10 */ > + //ST_PHB_ENTRY(8, 2, p1phb2_slot), /* P1E1B_x8_NA */ > + ST_PHB_ENTRY(8, 3, p1phb3_switch_slot),/* > + P1E2_x16_SWITCH_Slot8-9 */ > + > + { .etype = st_end }, > +}; > + > #define NPU_BASE 0x5011000 > #define NPU_SIZE 0x2c > #define NPU_INDIRECT0 0x8000000009010c3fUL /* OB0 - no OB3 on Mihawk > */ @@ -282,15 +406,90 @@ static bool mihawk_probe(void) > mihawk_create_npu(); > mihawk_create_ocapi_i2c_bus(); > > - slot_table_init(mihawk_phb_table); > + if (mihawk_riserF_found) > + slot_table_init(mihawk_riserF_phb_table); Is setting the slot table here necessary? It'll be overwritten when mihawk_init() is called later on and I don't think anything will reference the slot table until we start doing PCI probing. Joy: I'll remove this. > return true; > } > > +static void mihawk_riser_query_complete(struct ipmi_msg *msg) { > + if (msg->cc != IPMI_CC_NO_ERROR) { > + prlog(PR_ERR, "Mihawk: IPMI riser query returned error. cmd=0x%02x," > + " netfn=0x%02x, rc=0x%x\n", msg->cmd, msg->netfn, msg->cc); > + bmc_query_waiting = false; > + ipmi_free_msg(msg); > + return; > + } > + > + prlog(PR_DEBUG, "Mihawk: IPMI Got riser query result. p0:%02x, p1:%02x\n" > + , msg->data[0], msg->data[1]); > + > + uint8_t *riser_state = (uint8_t*)msg->user_data; > + lwsync(); > + *riser_state = msg->data[0] << 4 | msg->data[1]; > + > + bmc_query_waiting = false; > + ipmi_free_msg(msg); > +} > + > +static void mihawk_init(void) > +{ > + struct ipmi_msg *ipmi_msg; > + uint8_t riser_state = 0; > + int timeout_ms = 3000; > + > + astbmc_init(); > + > + /* > + * We use IPMI to ask BMC if Riser-F is installed and set up the > + * corresponding slot table. > + */ > + if (!mihawk_riserF_found) { Is mihawk_riserF_found ever going to be true at this point? Looks like its only ever set below Joy: The value check of mihawk_riserF_found is necessary(*typo; it should be “unnecessary”). I'll remove this. > + ipmi_msg = ipmi_mkmsg(IPMI_DEFAULT_INTERFACE, > + IPMI_RISERF_QUERY, > + mihawk_riser_query_complete, > + &riser_state, NULL, 0, 2); > + > + if (!ipmi_msg) { > + prlog(PR_ERR, "Mihawk: Couldn't create ipmi msg."); > + } > + else { > + ipmi_msg->error = mihawk_riser_query_complete; > + ipmi_queue_msg(ipmi_msg); > + bmc_query_waiting = true; > + > + prlog(PR_DEBUG, "Mihawk: Requesting IPMI_RISERF_QUERY (netfn " > + "%02x, cmd %02x)\n", ipmi_msg->netfn, > + ipmi_msg->cmd); > + > + while (bmc_query_waiting) { > + time_wait_ms(10); > + timeout_ms -= 10; > + > + if (timeout_ms == 0) > + break; > + } > + } Can you use ipmi_queue_msg_sync() instead of the callback and open-coded delay loop? The only problem I can see is that if the BMC doesn't respond we'd hit the timeout in the current code while ipmi_queue_msg_sync() will wait forever. We could always add a timeout to the _sync version though. Joy: To use ipmi_queue_msg_sync() with timeout, would you please share some example? We want to add the timeout to prevent from the long waiting and the increase of boot-up time. I still have no idea about how to add the timeout when using ipmi_queue_msg_sync(). Would you please give us some hints? > + prlog(PR_DEBUG, "Mihawk: IPMI_RISERF_QUERY finish. riser_state: %02x" > + ", waiting: %d\n", riser_state, > + bmc_query_waiting); > + > + if (riser_state != 0) { > + mihawk_riserF_found = true; > + slot_table_init(mihawk_riserF_phb_table); > + prlog(PR_DEBUG, "Mihawk: Detect Riser-F via IPMI\n"); > + } > + else { no newline > + slot_table_init(mihawk_riserA_phb_table); > + prlog(PR_DEBUG, "Mihawk: No Riser-F found, use Riser-A table\n"); > + } > + } > +} > + > DECLARE_PLATFORM(mihawk) = { > .name = "Mihawk", > .probe = mihawk_probe, > - .init = astbmc_init, > + .init = mihawk_init, > .start_preload_resource = flash_start_preload_resource, > .resource_loaded = flash_resource_loaded, > .bmc = &bmc_plat_ast2500_openbmc, > -- > 2.17.1 > > _______________________________________________ > Skiboot mailing list > Skiboot@lists.ozlabs.org<mailto:Skiboot@lists.ozlabs.org> > https://lists.ozlabs.org/listinfo/skiboot
On Thu, Mar 12, 2020 at 8:05 PM Joy Chu/WHQ/Wistron <Joy_Chu@wistron.com> wrote: > > Hi Oliver, > > As in your previous comment: > ""Can you use ipmi_queue_msg_sync() instead of the callback and open-coded delay loop? The only problem I can see is that if the BMC doesn't respond we'd hit the timeout in the current code while > ipmi_queue_msg_sync() will wait forever. We could always add a timeout to the _sync version though."" > > I still have no idea about how to add the timeout when using ipmi_queue_msg_sync(). > Would you please give us some hints? Hi Joy, Sorry about the delay. I thought I sent a reply the other day, but it looks like it didn't actually send. Anyway, currently there is no code to implement a synchronous message with a timeout. What I was thinking was that rather than having the platform code implement one by hand we should provide it as a part of the core API. That said, I'm a bit iffy about the concept. If we have a message that times out what should we be doing with the message on the skiboot side? If the BMC doesn't respond we can't just leave the message sitting on the IPMI queue. What is happening on the BMC side that prevents it sending a timely response? Oliver
Hi Oliver, In our case, we want to setup the timeout to prevent from the wait forever. When it hit the timeout, I need to keep the skiboot process going. We will setup a default setting if we can't get the response from BMC in time. At that time, we could free the ipmi msg from the queue (oh, I forget to handle this in my patch). Then, we will use the default setting and continue the mihawk_init process. Best Regards, Joy Chu -----Original Message----- From: Oliver O'Halloran <oohall@gmail.com> Sent: Wednesday, March 25, 2020 6:28 AM To: Joy Chu/WHQ/Wistron <Joy_Chu@wistron.com> Cc: skiboot list <skiboot@lists.ozlabs.org>; chhank@tw.ibm.com Subject: Re: [Skiboot] [PATCH] platform/mihawk: support dynamic PCIe slot table On Thu, Mar 12, 2020 at 8:05 PM Joy Chu/WHQ/Wistron <Joy_Chu@wistron.com> wrote: > > Hi Oliver, > > As in your previous comment: > ""Can you use ipmi_queue_msg_sync() instead of the callback and > open-coded delay loop? The only problem I can see is that if the BMC > doesn't respond we'd hit the timeout in the current code while > ipmi_queue_msg_sync() will wait forever. We could always add a timeout to the _sync version though."" > > I still have no idea about how to add the timeout when using ipmi_queue_msg_sync(). > Would you please give us some hints? Hi Joy, Sorry about the delay. I thought I sent a reply the other day, but it looks like it didn't actually send. Anyway, currently there is no code to implement a synchronous message with a timeout. What I was thinking was that rather than having the platform code implement one by hand we should provide it as a part of the core API. That said, I'm a bit iffy about the concept. If we have a message that times out what should we be doing with the message on the skiboot side? If the BMC doesn't respond we can't just leave the message sitting on the IPMI queue. What is happening on the BMC side that prevents it sending a timely response? Oliver
Hi Oliver, Sorry to bother you. For the timeout, do you have any other idea? We plan to release a pnor with this dynamic pcie slot table feature next Wednesday. I really hope this feature can be on time. Best Regards, Joy Chu -----Original Message----- From: Joy Chu/WHQ/Wistron Sent: Wednesday, March 25, 2020 5:37 PM To: 'Oliver O'Halloran' <oohall@gmail.com> Cc: skiboot list <skiboot@lists.ozlabs.org>; chhank@tw.ibm.com Subject: RE: [Skiboot] [PATCH] platform/mihawk: support dynamic PCIe slot table Hi Oliver, In our case, we want to setup the timeout to prevent from the wait forever. When it hit the timeout, I need to keep the skiboot process going. We will setup a default setting if we can't get the response from BMC in time. At that time, we could free the ipmi msg from the queue (oh, I forget to handle this in my patch). Then, we will use the default setting and continue the mihawk_init process. Best Regards, Joy Chu -----Original Message----- From: Oliver O'Halloran <oohall@gmail.com> Sent: Wednesday, March 25, 2020 6:28 AM To: Joy Chu/WHQ/Wistron <Joy_Chu@wistron.com> Cc: skiboot list <skiboot@lists.ozlabs.org>; chhank@tw.ibm.com Subject: Re: [Skiboot] [PATCH] platform/mihawk: support dynamic PCIe slot table On Thu, Mar 12, 2020 at 8:05 PM Joy Chu/WHQ/Wistron <Joy_Chu@wistron.com> wrote: > > Hi Oliver, > > As in your previous comment: > ""Can you use ipmi_queue_msg_sync() instead of the callback and > open-coded delay loop? The only problem I can see is that if the BMC > doesn't respond we'd hit the timeout in the current code while > ipmi_queue_msg_sync() will wait forever. We could always add a timeout to the _sync version though."" > > I still have no idea about how to add the timeout when using ipmi_queue_msg_sync(). > Would you please give us some hints? Hi Joy, Sorry about the delay. I thought I sent a reply the other day, but it looks like it didn't actually send. Anyway, currently there is no code to implement a synchronous message with a timeout. What I was thinking was that rather than having the platform code implement one by hand we should provide it as a part of the core API. That said, I'm a bit iffy about the concept. If we have a message that times out what should we be doing with the message on the skiboot side? If the BMC doesn't respond we can't just leave the message sitting on the IPMI queue. What is happening on the BMC side that prevents it sending a timely response? Oliver
Hi Oliver, Do you have any comment for this patch? Best Regards, Joy Chu -----Original Message----- From: Joy Chu/WHQ/Wistron Sent: Thursday, March 26, 2020 6:23 PM To: 'Oliver O'Halloran' <oohall@gmail.com> Cc: 'skiboot list' <skiboot@lists.ozlabs.org>; 'chhank@tw.ibm.com' <chhank@tw.ibm.com> Subject: RE: [Skiboot] [PATCH] platform/mihawk: support dynamic PCIe slot table Hi Oliver, Sorry to bother you. For the timeout, do you have any other idea? We plan to release a pnor with this dynamic pcie slot table feature next Wednesday. I really hope this feature can be on time. Best Regards, Joy Chu -----Original Message----- From: Joy Chu/WHQ/Wistron Sent: Wednesday, March 25, 2020 5:37 PM To: 'Oliver O'Halloran' <oohall@gmail.com> Cc: skiboot list <skiboot@lists.ozlabs.org>; chhank@tw.ibm.com Subject: RE: [Skiboot] [PATCH] platform/mihawk: support dynamic PCIe slot table Hi Oliver, In our case, we want to setup the timeout to prevent from the wait forever. When it hit the timeout, I need to keep the skiboot process going. We will setup a default setting if we can't get the response from BMC in time. At that time, we could free the ipmi msg from the queue (oh, I forget to handle this in my patch). Then, we will use the default setting and continue the mihawk_init process. Best Regards, Joy Chu -----Original Message----- From: Oliver O'Halloran <oohall@gmail.com> Sent: Wednesday, March 25, 2020 6:28 AM To: Joy Chu/WHQ/Wistron <Joy_Chu@wistron.com> Cc: skiboot list <skiboot@lists.ozlabs.org>; chhank@tw.ibm.com Subject: Re: [Skiboot] [PATCH] platform/mihawk: support dynamic PCIe slot table On Thu, Mar 12, 2020 at 8:05 PM Joy Chu/WHQ/Wistron <Joy_Chu@wistron.com> wrote: > > Hi Oliver, > > As in your previous comment: > ""Can you use ipmi_queue_msg_sync() instead of the callback and > open-coded delay loop? The only problem I can see is that if the BMC > doesn't respond we'd hit the timeout in the current code while > ipmi_queue_msg_sync() will wait forever. We could always add a timeout to the _sync version though."" > > I still have no idea about how to add the timeout when using ipmi_queue_msg_sync(). > Would you please give us some hints? Hi Joy, Sorry about the delay. I thought I sent a reply the other day, but it looks like it didn't actually send. Anyway, currently there is no code to implement a synchronous message with a timeout. What I was thinking was that rather than having the platform code implement one by hand we should provide it as a part of the core API. That said, I'm a bit iffy about the concept. If we have a message that times out what should we be doing with the message on the skiboot side? If the BMC doesn't respond we can't just leave the message sitting on the IPMI queue. What is happening on the BMC side that prevents it sending a timely response? Oliver
On Mon, Mar 30, 2020 at 7:13 PM Joy Chu/WHQ/Wistron <Joy_Chu@wistron.com> wrote: > > Hi Oliver, > > Do you have any comment for this patch? Not really. It sounds like the timeout is a just-in-case thing and assuming the BMC is functional you will always get a response? Is that correct? If so we can use the current patch. Oliver
Hi Oliver, Yes. If this solution is OK, I'll send the patch v2 for some modification and the code base update. Best Regards, Joy Chu -----Original Message----- From: Oliver O'Halloran <oohall@gmail.com> Sent: Monday, March 30, 2020 6:27 PM To: Joy Chu/WHQ/Wistron <Joy_Chu@wistron.com> Cc: skiboot list <skiboot@lists.ozlabs.org>; chhank@tw.ibm.com Subject: Re: [Skiboot] [PATCH] platform/mihawk: support dynamic PCIe slot table On Mon, Mar 30, 2020 at 7:13 PM Joy Chu/WHQ/Wistron <Joy_Chu@wistron.com> wrote: > > Hi Oliver, > > Do you have any comment for this patch? Not really. It sounds like the timeout is a just-in-case thing and assuming the BMC is functional you will always get a response? Is that correct? If so we can use the current patch. Oliver
diff --git a/platforms/astbmc/mihawk.c b/platforms/astbmc/mihawk.c index d33d16bb..28c999aa 100644 --- a/platforms/astbmc/mihawk.c +++ b/platforms/astbmc/mihawk.c @@ -15,8 +15,16 @@ #include <pci.h> #include <pci-cfg.h> +#include <timebase.h> + #include "astbmc.h" +/* IPMI message code for Riser-F query (OEM). */ +#define IPMI_RISERF_QUERY IPMI_CODE(0x32, 0x01) + +static bool mihawk_riserF_found = false; +static bool bmc_query_waiting = false; + /* nvme backplane slots */ static const struct slot_table_entry hdd_bay_slots[] = { SW_PLUGGABLE("hdd0", 0x0), @@ -91,7 +99,7 @@ static const struct platform_ocapi mihawk_ocapi = { .ocapi_slot_label = mihawk_ocapi_slot_label, }; -static const struct slot_table_entry P1E1A_x8_PLX8748_down[] = { +static const struct slot_table_entry P1E1A_x8_PLX8748_RiserA_down[] = { SW_PLUGGABLE("Slot7", 0x10), SW_PLUGGABLE("Slot8", 0x8), SW_PLUGGABLE("Slot10", 0x9), @@ -99,25 +107,25 @@ static const struct slot_table_entry P1E1A_x8_PLX8748_down[] = { { .etype = st_end } }; -static const struct slot_table_entry P1E1A_x8_PLX8748_up[] = { +static const struct slot_table_entry P1E1A_x8_PLX8748_RiserA_up[] = { { .etype = st_builtin_dev, .location = ST_LOC_DEVFN(0,0), - .children = P1E1A_x8_PLX8748_down, + .children = P1E1A_x8_PLX8748_RiserA_down, }, { .etype = st_end } }; -static const struct slot_table_entry p1phb1_slot[] = { +static const struct slot_table_entry p1phb1_rA_slot[] = { { .etype = st_builtin_dev, .location = ST_LOC_DEVFN(0,0), - .children = P1E1A_x8_PLX8748_up, + .children = P1E1A_x8_PLX8748_RiserA_up, }, { .etype = st_end }, }; -static const struct slot_table_entry P0E1A_x8_PLX8748_down[] = { +static const struct slot_table_entry P0E1A_x8_PLX8748_RiserA_down[] = { SW_PLUGGABLE("Slot2", 0x10), SW_PLUGGABLE("Slot3", 0x8), SW_PLUGGABLE("Slot5", 0x9), @@ -125,20 +133,120 @@ static const struct slot_table_entry P0E1A_x8_PLX8748_down[] = { { .etype = st_end } }; -static const struct slot_table_entry P0E1A_x8_PLX8748_up[] = { +static const struct slot_table_entry P0E1A_x8_PLX8748_RiserA_up[] = { + { + .etype = st_builtin_dev, + .location = ST_LOC_DEVFN(0,0), + .children = P0E1A_x8_PLX8748_RiserA_down, + }, + { .etype = st_end } +}; + +static const struct slot_table_entry p0phb1_rA_slot[] = { + { + .etype = st_builtin_dev, + .location = ST_LOC_DEVFN(0,0), + .children = P0E1A_x8_PLX8748_RiserA_up, + }, + { .etype = st_end }, +}; + +static const struct slot_table_entry P1E1A_x8_PLX8748_RiserF_down[] = { + SW_PLUGGABLE("Slot7", 0x10), + SW_PLUGGABLE("Slot10", 0x9), + + { .etype = st_end } +}; + +static const struct slot_table_entry P1E1A_x8_PLX8748_RiserF_up[] = { + { + .etype = st_builtin_dev, + .location = ST_LOC_DEVFN(0,0), + .children = P1E1A_x8_PLX8748_RiserF_down, + }, + { .etype = st_end } +}; + +static const struct slot_table_entry p1phb1_rF_slot[] = { + { + .etype = st_builtin_dev, + .location = ST_LOC_DEVFN(0,0), + .children = P1E1A_x8_PLX8748_RiserF_up, + }, + { .etype = st_end }, +}; + +static const struct slot_table_entry P0E1A_x8_PLX8748_RiserF_down[] = { + SW_PLUGGABLE("Slot2", 0x10), + SW_PLUGGABLE("Slot5", 0x9), + + { .etype = st_end } +}; + +static const struct slot_table_entry P0E1A_x8_PLX8748_RiserF_up[] = { { .etype = st_builtin_dev, .location = ST_LOC_DEVFN(0,0), - .children = P0E1A_x8_PLX8748_down, + .children = P0E1A_x8_PLX8748_RiserF_down, }, { .etype = st_end } }; -static const struct slot_table_entry p0phb1_slot[] = { +static const struct slot_table_entry p0phb1_rF_slot[] = { { .etype = st_builtin_dev, .location = ST_LOC_DEVFN(0,0), - .children = P0E1A_x8_PLX8748_up, + .children = P0E1A_x8_PLX8748_RiserF_up, + }, + { .etype = st_end }, +}; + +static const struct slot_table_entry P1E2_x16_Switch_down[] = { + SW_PLUGGABLE("Slot8", 0x1), + SW_PLUGGABLE("Slot9", 0x0), + + { .etype = st_end } +}; + +static const struct slot_table_entry P1E2_x16_Switch_up[] = { + { + .etype = st_builtin_dev, + .location = ST_LOC_DEVFN(0,0), + .children = P1E2_x16_Switch_down, + }, + { .etype = st_end } +}; + +static const struct slot_table_entry p1phb3_switch_slot[] = { + { + .etype = st_builtin_dev, + .location = ST_LOC_DEVFN(0,0), + .children = P1E2_x16_Switch_up, + }, + { .etype = st_end }, +}; + +static const struct slot_table_entry P0E2_x16_Switch_down[] = { + SW_PLUGGABLE("Slot3", 0x1), + SW_PLUGGABLE("Slot4", 0x0), + + { .etype = st_end } +}; + +static const struct slot_table_entry P0E2_x16_Switch_up[] = { + { + .etype = st_builtin_dev, + .location = ST_LOC_DEVFN(0,0), + .children = P0E2_x16_Switch_down, + }, + { .etype = st_end } +}; + +static const struct slot_table_entry p0phb3_switch_slot[] = { + { + .etype = st_builtin_dev, + .location = ST_LOC_DEVFN(0,0), + .children = P0E2_x16_Switch_up, }, { .etype = st_end }, }; @@ -148,22 +256,38 @@ ST_PLUGGABLE(p0phb3_slot, "Slot4"); ST_PLUGGABLE(p1phb0_slot, "Slot6"); ST_PLUGGABLE(p1phb3_slot, "Slot9"); -static const struct slot_table_entry mihawk_phb_table[] = { +static const struct slot_table_entry mihawk_riserA_phb_table[] = { /* ==== CPU0 ==== */ ST_PHB_ENTRY(0, 0, p0phb0_slot), /* P0E0_x16_Slot1 */ - ST_PHB_ENTRY(0, 1, p0phb1_slot), /* P0E1A_x8_PLX8748-1_Slot2-3-5 */ + ST_PHB_ENTRY(0, 1, p0phb1_rA_slot), /* P0E1A_x8_PLX8748-1_Slot2-3-5 */ //ST_PHB_ENTRY(0, 2, p0phb2_slot), /* P0E1B_x8_USBTI7340 */ ST_PHB_ENTRY(0, 3, p0phb3_slot), /* P0E2_x16_Slot4 */ /* ==== CPU1 ==== */ ST_PHB_ENTRY(8, 0, p1phb0_slot), /* P1E0_x16_Slot6 */ - ST_PHB_ENTRY(8, 1, p1phb1_slot), /* P1E1A_x8_PLX8748-2_Slot7-8-10 */ + ST_PHB_ENTRY(8, 1, p1phb1_rA_slot), /* P1E1A_x8_PLX8748-2_Slot7-8-10 */ //ST_PHB_ENTRY(8, 2, p1phb2_slot), /* P1E1B_x8_NA */ ST_PHB_ENTRY(8, 3, p1phb3_slot), /* P1E2_x16_Slot9 */ { .etype = st_end }, }; +static const struct slot_table_entry mihawk_riserF_phb_table[] = { + /* ==== CPU0 ==== */ + ST_PHB_ENTRY(0, 0, p0phb0_slot), /* P0E0_x16_Slot1 */ + ST_PHB_ENTRY(0, 1, p0phb1_rF_slot), /* P0E1A_x8_PLX8748-1_Slot2-5 */ + //ST_PHB_ENTRY(0, 2, p0phb2_slot), /* P0E1B_x8_USBTI7340 */ + ST_PHB_ENTRY(0, 3, p0phb3_switch_slot),/* P0E2_x16_SWITCH_Slot3-4 */ + + /* ==== CPU1 ==== */ + ST_PHB_ENTRY(8, 0, p1phb0_slot), /* P1E0_x16_Slot6 */ + ST_PHB_ENTRY(8, 1, p1phb1_rF_slot), /* P1E1A_x8_PLX8748-2_Slot7-10 */ + //ST_PHB_ENTRY(8, 2, p1phb2_slot), /* P1E1B_x8_NA */ + ST_PHB_ENTRY(8, 3, p1phb3_switch_slot),/* P1E2_x16_SWITCH_Slot8-9 */ + + { .etype = st_end }, +}; + #define NPU_BASE 0x5011000 #define NPU_SIZE 0x2c #define NPU_INDIRECT0 0x8000000009010c3fUL /* OB0 - no OB3 on Mihawk */ @@ -282,15 +406,90 @@ static bool mihawk_probe(void) mihawk_create_npu(); mihawk_create_ocapi_i2c_bus(); - slot_table_init(mihawk_phb_table); + if (mihawk_riserF_found) + slot_table_init(mihawk_riserF_phb_table); return true; } +static void mihawk_riser_query_complete(struct ipmi_msg *msg) +{ + if (msg->cc != IPMI_CC_NO_ERROR) { + prlog(PR_ERR, "Mihawk: IPMI riser query returned error. cmd=0x%02x," + " netfn=0x%02x, rc=0x%x\n", msg->cmd, msg->netfn, msg->cc); + bmc_query_waiting = false; + ipmi_free_msg(msg); + return; + } + + prlog(PR_DEBUG, "Mihawk: IPMI Got riser query result. p0:%02x, p1:%02x\n" + , msg->data[0], msg->data[1]); + + uint8_t *riser_state = (uint8_t*)msg->user_data; + lwsync(); + *riser_state = msg->data[0] << 4 | msg->data[1]; + + bmc_query_waiting = false; + ipmi_free_msg(msg); +} + +static void mihawk_init(void) +{ + struct ipmi_msg *ipmi_msg; + uint8_t riser_state = 0; + int timeout_ms = 3000; + + astbmc_init(); + + /* + * We use IPMI to ask BMC if Riser-F is installed and set up the + * corresponding slot table. + */ + if (!mihawk_riserF_found) { + ipmi_msg = ipmi_mkmsg(IPMI_DEFAULT_INTERFACE, + IPMI_RISERF_QUERY, + mihawk_riser_query_complete, + &riser_state, NULL, 0, 2); + + if (!ipmi_msg) { + prlog(PR_ERR, "Mihawk: Couldn't create ipmi msg."); + } + else { + ipmi_msg->error = mihawk_riser_query_complete; + ipmi_queue_msg(ipmi_msg); + bmc_query_waiting = true; + + prlog(PR_DEBUG, "Mihawk: Requesting IPMI_RISERF_QUERY (netfn " + "%02x, cmd %02x)\n", ipmi_msg->netfn, ipmi_msg->cmd); + + while (bmc_query_waiting) { + time_wait_ms(10); + timeout_ms -= 10; + + if (timeout_ms == 0) + break; + } + } + + prlog(PR_DEBUG, "Mihawk: IPMI_RISERF_QUERY finish. riser_state: %02x" + ", waiting: %d\n", riser_state, bmc_query_waiting); + + if (riser_state != 0) { + mihawk_riserF_found = true; + slot_table_init(mihawk_riserF_phb_table); + prlog(PR_DEBUG, "Mihawk: Detect Riser-F via IPMI\n"); + } + else { + slot_table_init(mihawk_riserA_phb_table); + prlog(PR_DEBUG, "Mihawk: No Riser-F found, use Riser-A table\n"); + } + } +} + DECLARE_PLATFORM(mihawk) = { .name = "Mihawk", .probe = mihawk_probe, - .init = astbmc_init, + .init = mihawk_init, .start_preload_resource = flash_start_preload_resource, .resource_loaded = flash_resource_loaded, .bmc = &bmc_plat_ast2500_openbmc,
Slot table auto-detection for different riser cards by using IPMI OEM command to communicate with BMC. Signed-off-by: Joy Chu <joy_chu@wistron.com> --- platforms/astbmc/mihawk.c | 229 +++++++++++++++++++++++++++++++++++--- 1 file changed, 214 insertions(+), 15 deletions(-)