| Message ID | 1460010507-24777-2-git-send-email-ruscur@russell.cc (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Thu, Apr 07, 2016 at 04:28:27PM +1000, Russell Currey wrote: >The RTAS calls "ibm,configure-pe" and "ibm,configure-bridge" perform the >same actions, however the former can skip configuration if unnecessary. >The existing code treats them as different tokens even though only one >will ever be called. Refactor this by making a single token that is >assigned during init. > >Signed-off-by: Russell Currey <ruscur@russell.cc> Acked-by: Gavin Shan <gwshan@linux.vnet.ibm.com> >--- >V3: Reorder commits so the previous patch doesn't depend on this > >I had a look at doing the same with some other duplicated tokens but >they had slight differences in semantics so it wasn't helping clarity. >--- > arch/powerpc/platforms/pseries/eeh_pseries.c | 28 ++++++++++++---------------- > 1 file changed, 12 insertions(+), 16 deletions(-) > >diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c >index 405baaf..3998e0f 100644 >--- a/arch/powerpc/platforms/pseries/eeh_pseries.c >+++ b/arch/powerpc/platforms/pseries/eeh_pseries.c >@@ -53,7 +53,6 @@ static int ibm_read_slot_reset_state2; > static int ibm_slot_error_detail; > static int ibm_get_config_addr_info; > static int ibm_get_config_addr_info2; >-static int ibm_configure_bridge; > static int ibm_configure_pe; > > /* >@@ -81,7 +80,14 @@ static int pseries_eeh_init(void) > ibm_get_config_addr_info2 = rtas_token("ibm,get-config-addr-info2"); > ibm_get_config_addr_info = rtas_token("ibm,get-config-addr-info"); > ibm_configure_pe = rtas_token("ibm,configure-pe"); >- ibm_configure_bridge = rtas_token("ibm,configure-bridge"); >+ >+ /* >+ * ibm,configure-pe and ibm,configure-bridge have the same semantics, >+ * however ibm,configure-pe can be faster. If we can't find >+ * ibm,configure-pe then fall back to using ibm,configure-bridge. >+ */ >+ if (ibm_configure_pe == RTAS_UNKNOWN_SERVICE) >+ ibm_configure_pe = rtas_token("ibm,configure-bridge"); > > /* > * Necessary sanity check. We needn't check "get-config-addr-info" >@@ -93,8 +99,7 @@ static int pseries_eeh_init(void) > (ibm_read_slot_reset_state2 == RTAS_UNKNOWN_SERVICE && > ibm_read_slot_reset_state == RTAS_UNKNOWN_SERVICE) || > ibm_slot_error_detail == RTAS_UNKNOWN_SERVICE || >- (ibm_configure_pe == RTAS_UNKNOWN_SERVICE && >- ibm_configure_bridge == RTAS_UNKNOWN_SERVICE)) { >+ ibm_configure_pe == RTAS_UNKNOWN_SERVICE) { > pr_info("EEH functionality not supported\n"); > return -EINVAL; > } >@@ -624,18 +629,9 @@ static int pseries_eeh_configure_bridge(struct eeh_pe *pe) > config_addr = pe->addr; > > while (max_wait > 0) { >- /* Use new configure-pe function, if supported */ >- if (ibm_configure_pe != RTAS_UNKNOWN_SERVICE) { >- ret = rtas_call(ibm_configure_pe, 3, 1, NULL, >- config_addr, BUID_HI(pe->phb->buid), >- BUID_LO(pe->phb->buid)); >- } else if (ibm_configure_bridge != RTAS_UNKNOWN_SERVICE) { >- ret = rtas_call(ibm_configure_bridge, 3, 1, NULL, >- config_addr, BUID_HI(pe->phb->buid), >- BUID_LO(pe->phb->buid)); >- } else { >- return -EFAULT; >- } >+ ret = rtas_call(ibm_configure_pe, 3, 1, NULL, >+ config_addr, BUID_HI(pe->phb->buid), >+ BUID_LO(pe->phb->buid)); > > if (!ret) > return ret; >-- >2.8.0 > >_______________________________________________ >Linuxppc-dev mailing list >Linuxppc-dev@lists.ozlabs.org >https://lists.ozlabs.org/listinfo/linuxppc-dev
On Thu, 2016-07-04 at 06:28:27 UTC, Russell Currey wrote: > The RTAS calls "ibm,configure-pe" and "ibm,configure-bridge" perform the > same actions, however the former can skip configuration if unnecessary. > The existing code treats them as different tokens even though only one > will ever be called. Refactor this by making a single token that is > assigned during init. > > Signed-off-by: Russell Currey <ruscur@russell.cc> > Acked-by: Gavin Shan <gwshan@linux.vnet.ibm.com> Applied to powerpc fixes, thanks. https://git.kernel.org/powerpc/c/bd000b82e86503d5e8b9e6d40a cheers
diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c index 405baaf..3998e0f 100644 --- a/arch/powerpc/platforms/pseries/eeh_pseries.c +++ b/arch/powerpc/platforms/pseries/eeh_pseries.c @@ -53,7 +53,6 @@ static int ibm_read_slot_reset_state2; static int ibm_slot_error_detail; static int ibm_get_config_addr_info; static int ibm_get_config_addr_info2; -static int ibm_configure_bridge; static int ibm_configure_pe; /* @@ -81,7 +80,14 @@ static int pseries_eeh_init(void) ibm_get_config_addr_info2 = rtas_token("ibm,get-config-addr-info2"); ibm_get_config_addr_info = rtas_token("ibm,get-config-addr-info"); ibm_configure_pe = rtas_token("ibm,configure-pe"); - ibm_configure_bridge = rtas_token("ibm,configure-bridge"); + + /* + * ibm,configure-pe and ibm,configure-bridge have the same semantics, + * however ibm,configure-pe can be faster. If we can't find + * ibm,configure-pe then fall back to using ibm,configure-bridge. + */ + if (ibm_configure_pe == RTAS_UNKNOWN_SERVICE) + ibm_configure_pe = rtas_token("ibm,configure-bridge"); /* * Necessary sanity check. We needn't check "get-config-addr-info" @@ -93,8 +99,7 @@ static int pseries_eeh_init(void) (ibm_read_slot_reset_state2 == RTAS_UNKNOWN_SERVICE && ibm_read_slot_reset_state == RTAS_UNKNOWN_SERVICE) || ibm_slot_error_detail == RTAS_UNKNOWN_SERVICE || - (ibm_configure_pe == RTAS_UNKNOWN_SERVICE && - ibm_configure_bridge == RTAS_UNKNOWN_SERVICE)) { + ibm_configure_pe == RTAS_UNKNOWN_SERVICE) { pr_info("EEH functionality not supported\n"); return -EINVAL; } @@ -624,18 +629,9 @@ static int pseries_eeh_configure_bridge(struct eeh_pe *pe) config_addr = pe->addr; while (max_wait > 0) { - /* Use new configure-pe function, if supported */ - if (ibm_configure_pe != RTAS_UNKNOWN_SERVICE) { - ret = rtas_call(ibm_configure_pe, 3, 1, NULL, - config_addr, BUID_HI(pe->phb->buid), - BUID_LO(pe->phb->buid)); - } else if (ibm_configure_bridge != RTAS_UNKNOWN_SERVICE) { - ret = rtas_call(ibm_configure_bridge, 3, 1, NULL, - config_addr, BUID_HI(pe->phb->buid), - BUID_LO(pe->phb->buid)); - } else { - return -EFAULT; - } + ret = rtas_call(ibm_configure_pe, 3, 1, NULL, + config_addr, BUID_HI(pe->phb->buid), + BUID_LO(pe->phb->buid)); if (!ret) return ret;
The RTAS calls "ibm,configure-pe" and "ibm,configure-bridge" perform the same actions, however the former can skip configuration if unnecessary. The existing code treats them as different tokens even though only one will ever be called. Refactor this by making a single token that is assigned during init. Signed-off-by: Russell Currey <ruscur@russell.cc> --- V3: Reorder commits so the previous patch doesn't depend on this I had a look at doing the same with some other duplicated tokens but they had slight differences in semantics so it wasn't helping clarity. --- arch/powerpc/platforms/pseries/eeh_pseries.c | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-)