| Message ID | 1525983970-14139-1-git-send-email-ppaidipe@linux.vnet.ibm.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | p9dsu: detect variant in init only if probe fails to found. | expand |
On Fri, 2018-05-11 at 01:56 +0530, Pridhiviraj Paidipeddi wrote: > Currently the slot table init happens twice in both probe and init > functions due to the variant detection logic called with in-correct > condition check. > > Fixes: d32ddea9 ("p9dsu: detect p9dsu variant even when hostboot > doesn't tell us") > > Signed-off-by: Pridhiviraj Paidipeddi <ppaidipe@linux.vnet.ibm.com> > --- > platforms/astbmc/p9dsu.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/platforms/astbmc/p9dsu.c b/platforms/astbmc/p9dsu.c > index e4fef5e..ead693f 100644 > --- a/platforms/astbmc/p9dsu.c > +++ b/platforms/astbmc/p9dsu.c > @@ -28,6 +28,8 @@ > > #include "astbmc.h" > > +static bool variant_found = false; Can you call this something more descriptive than just "variant"? > + > static const struct slot_table_entry p9dsu1u_phb0_0_slot[] = { > { > .etype = st_pluggable_slot, > @@ -590,6 +592,8 @@ static bool p9dsu_probe(void) > dt_node_is_compatible(dt_root, "supermicro,p9dsu2uess"))) > return false; > > + variant_found = true; > + > /* Lot of common early inits here */ > astbmc_early_init(); > > @@ -607,7 +611,7 @@ static bool p9dsu_probe(void) > } else if (dt_node_is_compatible(dt_root, "supermicro,p9dsu2uess")) { > prlog(PR_INFO, "Detected p9dsu2uess variant\n"); > slot_table_init(p9dsu2uess_phb_table); > - } > + } else { > /* > * else we need to ask the BMC what subtype we are, but we need IPMI > * which we don't get until astbmc_init(), so we delay setting up the > @@ -616,6 +620,8 @@ static bool p9dsu_probe(void) > * This only applies if you're using a Hostboot that doesn't do this > * for us. > */ > + variant_found = false; > + } > > return true; > } > @@ -642,7 +648,7 @@ static void p9dsu_init(void) > * variant we are if Hostboot isn't the patched one that does this > * for us. > */ > - if (dt_node_is_compatible(dt_root, "supermicro,p9dsu")) { > + if (!variant_found) { > ipmi_msg = ipmi_mkmsg(IPMI_DEFAULT_INTERFACE, > IPMI_CODE(IPMI_NETFN_APP, 0x52), > p9dsu_riser_query_complete,
On 2018-05-11 11:41, Michael Neuling wrote: > On Fri, 2018-05-11 at 01:56 +0530, Pridhiviraj Paidipeddi wrote: >> Currently the slot table init happens twice in both probe and init >> functions due to the variant detection logic called with in-correct >> condition check. >> >> Fixes: d32ddea9 ("p9dsu: detect p9dsu variant even when hostboot >> doesn't tell us") >> >> Signed-off-by: Pridhiviraj Paidipeddi <ppaidipe@linux.vnet.ibm.com> >> --- >> platforms/astbmc/p9dsu.c | 10 ++++++++-- >> 1 file changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/platforms/astbmc/p9dsu.c b/platforms/astbmc/p9dsu.c >> index e4fef5e..ead693f 100644 >> --- a/platforms/astbmc/p9dsu.c >> +++ b/platforms/astbmc/p9dsu.c >> @@ -28,6 +28,8 @@ >> >> #include "astbmc.h" >> >> +static bool variant_found = false; > > Can you call this something more descriptive than just "variant"? > How about p9dsu_riser_found? >> + >> static const struct slot_table_entry p9dsu1u_phb0_0_slot[] = { >> { >> .etype = st_pluggable_slot, >> @@ -590,6 +592,8 @@ static bool p9dsu_probe(void) >> dt_node_is_compatible(dt_root, "supermicro,p9dsu2uess"))) >> return false; >> >> + variant_found = true; >> + >> /* Lot of common early inits here */ >> astbmc_early_init(); >> >> @@ -607,7 +611,7 @@ static bool p9dsu_probe(void) >> } else if (dt_node_is_compatible(dt_root, "supermicro,p9dsu2uess")) >> { >> prlog(PR_INFO, "Detected p9dsu2uess variant\n"); >> slot_table_init(p9dsu2uess_phb_table); >> - } >> + } else { >> /* >> * else we need to ask the BMC what subtype we are, but we need IPMI >> * which we don't get until astbmc_init(), so we delay setting up >> the >> @@ -616,6 +620,8 @@ static bool p9dsu_probe(void) >> * This only applies if you're using a Hostboot that doesn't do this >> * for us. >> */ >> + variant_found = false; >> + } >> >> return true; >> } >> @@ -642,7 +648,7 @@ static void p9dsu_init(void) >> * variant we are if Hostboot isn't the patched one that does this >> * for us. >> */ >> - if (dt_node_is_compatible(dt_root, "supermicro,p9dsu")) { >> + if (!variant_found) { >> ipmi_msg = ipmi_mkmsg(IPMI_DEFAULT_INTERFACE, >> IPMI_CODE(IPMI_NETFN_APP, 0x52), >> p9dsu_riser_query_complete,
ppaidipe <ppaidipe@linux.vnet.ibm.com> writes: > On 2018-05-11 11:41, Michael Neuling wrote: >> On Fri, 2018-05-11 at 01:56 +0530, Pridhiviraj Paidipeddi wrote: >>> Currently the slot table init happens twice in both probe and init >>> functions due to the variant detection logic called with in-correct >>> condition check. >>> >>> Fixes: d32ddea9 ("p9dsu: detect p9dsu variant even when hostboot >>> doesn't tell us") >>> >>> Signed-off-by: Pridhiviraj Paidipeddi <ppaidipe@linux.vnet.ibm.com> >>> --- >>> platforms/astbmc/p9dsu.c | 10 ++++++++-- >>> 1 file changed, 8 insertions(+), 2 deletions(-) >>> >>> diff --git a/platforms/astbmc/p9dsu.c b/platforms/astbmc/p9dsu.c >>> index e4fef5e..ead693f 100644 >>> --- a/platforms/astbmc/p9dsu.c >>> +++ b/platforms/astbmc/p9dsu.c >>> @@ -28,6 +28,8 @@ >>> >>> #include "astbmc.h" >>> >>> +static bool variant_found = false; >> >> Can you call this something more descriptive than just "variant"? >> > > How about p9dsu_riser_found? I think that's okay - I did the change and merged to master as of d6317227c2d1b0ddbb331c250729d8a3b07180d1
On 2018-05-12 01:37, Stewart Smith wrote: > ppaidipe <ppaidipe@linux.vnet.ibm.com> writes: >> On 2018-05-11 11:41, Michael Neuling wrote: >>> On Fri, 2018-05-11 at 01:56 +0530, Pridhiviraj Paidipeddi wrote: >>>> Currently the slot table init happens twice in both probe and init >>>> functions due to the variant detection logic called with in-correct >>>> condition check. >>>> >>>> Fixes: d32ddea9 ("p9dsu: detect p9dsu variant even when hostboot >>>> doesn't tell us") >>>> >>>> Signed-off-by: Pridhiviraj Paidipeddi <ppaidipe@linux.vnet.ibm.com> >>>> --- >>>> platforms/astbmc/p9dsu.c | 10 ++++++++-- >>>> 1 file changed, 8 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/platforms/astbmc/p9dsu.c b/platforms/astbmc/p9dsu.c >>>> index e4fef5e..ead693f 100644 >>>> --- a/platforms/astbmc/p9dsu.c >>>> +++ b/platforms/astbmc/p9dsu.c >>>> @@ -28,6 +28,8 @@ >>>> >>>> #include "astbmc.h" >>>> >>>> +static bool variant_found = false; >>> >>> Can you call this something more descriptive than just "variant"? >>> >> >> How about p9dsu_riser_found? > > I think that's okay - I did the change and merged to master as of > d6317227c2d1b0ddbb331c250729d8a3b07180d1 Thanks.
diff --git a/platforms/astbmc/p9dsu.c b/platforms/astbmc/p9dsu.c index e4fef5e..ead693f 100644 --- a/platforms/astbmc/p9dsu.c +++ b/platforms/astbmc/p9dsu.c @@ -28,6 +28,8 @@ #include "astbmc.h" +static bool variant_found = false; + static const struct slot_table_entry p9dsu1u_phb0_0_slot[] = { { .etype = st_pluggable_slot, @@ -590,6 +592,8 @@ static bool p9dsu_probe(void) dt_node_is_compatible(dt_root, "supermicro,p9dsu2uess"))) return false; + variant_found = true; + /* Lot of common early inits here */ astbmc_early_init(); @@ -607,7 +611,7 @@ static bool p9dsu_probe(void) } else if (dt_node_is_compatible(dt_root, "supermicro,p9dsu2uess")) { prlog(PR_INFO, "Detected p9dsu2uess variant\n"); slot_table_init(p9dsu2uess_phb_table); - } + } else { /* * else we need to ask the BMC what subtype we are, but we need IPMI * which we don't get until astbmc_init(), so we delay setting up the @@ -616,6 +620,8 @@ static bool p9dsu_probe(void) * This only applies if you're using a Hostboot that doesn't do this * for us. */ + variant_found = false; + } return true; } @@ -642,7 +648,7 @@ static void p9dsu_init(void) * variant we are if Hostboot isn't the patched one that does this * for us. */ - if (dt_node_is_compatible(dt_root, "supermicro,p9dsu")) { + if (!variant_found) { ipmi_msg = ipmi_mkmsg(IPMI_DEFAULT_INTERFACE, IPMI_CODE(IPMI_NETFN_APP, 0x52), p9dsu_riser_query_complete,
Currently the slot table init happens twice in both probe and init functions due to the variant detection logic called with in-correct condition check. Fixes: d32ddea9 ("p9dsu: detect p9dsu variant even when hostboot doesn't tell us") Signed-off-by: Pridhiviraj Paidipeddi <ppaidipe@linux.vnet.ibm.com> --- platforms/astbmc/p9dsu.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)