Re: [PATCH next] cxl: Allow PSL timebase to not sync

[Michael Neuling <mikey@neuling.org>]

On Mon, 2016-03-14 at 20:29 +0100, Frederic Barrat wrote:
> CXL driver synchronizes the PSL timebase with the CAPP during
> initialization. If it can't synchronize, then the driver currently
> fails and the cxl adapter is not usable. That behavior is a bit
> extreme for the time being, as some adapters are known to have
> troubles syncing their PSL timebase and there are no known use of it.
>=20
> Introduce a psl_timebase module parameter to control whether PSL
> timebase is required or not. Default is to allow initializaton even
> if
> syncing failed.
> Default behavior will be changed when current issues with some cxl
> adapters are resolved.

I'm not happy with doing this unless we add something which advertises
that it's synced or not to userspace.

If we do that, I'm happy to just fail without the need of the parameter
but advertise it to userspace.

The parameter is a bit of a PITA too, as it's a driver level config not
card level.  You really want to turn it on/off based on the card, not
the whole system.

Mikey

>=20
> Signed-off-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
> ---
>=20
> Same as before, but this patch applies on 'next'
>=20
>  drivers/misc/cxl/cxl.h  | 1 +
>  drivers/misc/cxl/main.c | 4 ++++
>  drivers/misc/cxl/pci.c  | 7 +++++--
>  3 files changed, 10 insertions(+), 2 deletions(-)
>=20
> diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
> index 38e21cf..84b563c 100644
> --- a/drivers/misc/cxl/cxl.h
> +++ b/drivers/misc/cxl/cxl.h
> @@ -27,6 +27,7 @@
>  #include <uapi/misc/cxl.h>
> =20
>  extern uint cxl_verbose;
> +extern uint cxl_psl_timebase;
> =20
>  #define CXL_TIMEOUT 5
> =20
> diff --git a/drivers/misc/cxl/main.c b/drivers/misc/cxl/main.c
> index ae68c32..0344010 100644
> --- a/drivers/misc/cxl/main.c
> +++ b/drivers/misc/cxl/main.c
> @@ -32,6 +32,10 @@ uint cxl_verbose;
>  module_param_named(verbose, cxl_verbose, uint, 0600);
>  MODULE_PARM_DESC(verbose, "Enable verbose dmesg output");
> =20
> +uint cxl_psl_timebase;
> +module_param_named(psl_timebase, cxl_psl_timebase, uint, 0600);

Can we make this a bool?

> +MODULE_PARM_DESC(psl_timebase, "Require PSL timebase
> synchronization");
> +
>  const struct cxl_backend_ops *cxl_ops;
> =20
>  int cxl_afu_slbia(struct cxl_afu *afu)
> diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
> index 2844e97..02fd1f8 100644
> --- a/drivers/misc/cxl/pci.c
> +++ b/drivers/misc/cxl/pci.c
> @@ -1144,8 +1144,11 @@ static int cxl_configure_adapter(struct cxl
> *adapter, struct pci_dev *dev)
>  	if ((rc =3D pnv_phb_to_cxl_mode(dev,
> OPAL_PHB_CAPI_MODE_SNOOP_ON)))
>  		goto err;
> =20
> -	if ((rc =3D cxl_setup_psl_timebase(adapter, dev)))
> -		goto err;
> +	if ((rc =3D cxl_setup_psl_timebase(adapter, dev))) {
> +		if (cxl_psl_timebase)
> +			goto err;
> +		pr_err("PSL: Timebase sync: ignoring error\n");
> +	}
> =20
>  	if ((rc =3D cxl_native_register_psl_err_irq(adapter)))
>  		goto err;