On Tue, Mar 29, 2016 at 03:53:19PM +1000, Russell Currey wrote:

On Tue, 2016-03-29 at 16:26 +1100, Gavin Shan wrote:

quoted

On Tue, Mar 29, 2016 at 12:51:50PM +1000, Russell Currey wrote:

<snip>

quoted

quoted

	/*
	 * Necessary sanity check. We needn't check "get-config-addr-info"

@@ -93,8 +98,7 @@ static int pseries_eeh_init(void)

	=A0=A0=A0=A0(ibm_read_slot_reset_state2 =3D=3D RTAS_UNKNOWN_SERVICE=

 &&

quoted

quoted

	=A0=A0=A0=A0=A0ibm_read_slot_reset_state =3D=3D RTAS_UNKNOWN_SERVIC=

E)	||

quoted

quoted

	=A0=A0=A0=A0ibm_slot_error_detail =3D=3D RTAS_UNKNOWN_SERVICE	||
-	=A0=A0=A0=A0(ibm_configure_pe =3D=3D RTAS_UNKNOWN_SERVICE		&
&
-	=A0=A0=A0=A0=A0ibm_configure_bridge =3D=3D RTAS_UNKNOWN_SERVICE)) =

{

quoted

quoted

+	=A0=A0=A0=A0ibm_configure_pe =3D=3D RTAS_UNKNOWN_SERVICE) {
		pr_info("EEH functionality not supported\n");
		return -EINVAL;
	}

Since you're here, you can do similar thing to @ibm_read_slot_reset_st=

ate

quoted

and @ibm_read_slot_reset_state?

Ah, didn't notice there was a similar thing going on there. =A0Will fix.

Ok.

quoted

=20

quoted

=20

@@ -621,18 +625,9 @@ static int pseries_eeh_configure_bridge(struct

eeh_pe *pe)
	if (pe->addr)
		config_addr =3D pe->addr;
=20
-	/* Use new configure-pe function, if supported */
-	if (ibm_configure_pe !=3D RTAS_UNKNOWN_SERVICE) {
-		ret =3D 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 !=3D RTAS_UNKNOWN_SERVICE) {
-		ret =3D rtas_call(ibm_configure_bridge, 3, 1, NULL,
-				config_addr, BUID_HI(pe->phb->buid),
-				BUID_LO(pe->phb->buid));
-	} else {
-		return -EFAULT;
-	}
+	ret =3D rtas_call(ibm_configure_pe, 3, 1, NULL,
+			config_addr, BUID_HI(pe->phb->buid),
+			BUID_LO(pe->phb->buid));
=20

Russell, it seems not working if "ibm,configure-pe" and "ibm,configure=

-

quoted

bridge" are all
missed from "/rtas".

If they're both missing, then the init should fail as=A0ibm_configure_pe=

 will

be RTAS_UNKNOWN_SERVICE, so this code should never be called.

Yeah, I missed the point, thanks.

quoted

 Also, I don't think we need backport it to 3.10+ as it's not fixing
any bugs if I'm correct enough.

This patch doesn't, but the second patch does.

Ok. In the commit log of this patch, you have something like below and th=
at
means it needs by stable kernels. I agree the next one is needed by stabl=
e
kernels, so the two patches would have inversed order if you agree. In th=
at
case, the next one (to be in stable kernels) won't depend on current on w=
hich
isn't required by stable kernels.

Cc: <redacted> # 3.10-   <<< The format would be 3.10+

Thanks,
Gavin

quoted

quoted

=20
	if (ret)
		pr_warn("%s: Unable to configure bridge PHB#%d-PE#%x
(%d)\n",