[PATCH V6] cxl: Fix timebase synchronization status on P9

[PATCH V6] cxl: Fix timebase synchronization status on P9

[Christophe Lombard]

The PSL Timebase register is updated by the PSL to maintain the
timebase.
On P9, the Timebase value is only provided by the CAPP as received
the last time a timebase request was performed.
The timebase requests are initiated through the adapter configuration or
application registers.
The specific sysfs entry "/sys/class/cxl/cardxx/psl_timebase_synced" is
now dynamically updated according the content of the PSL Timebase
register.

Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
Reviewed-by: Vaibhav Jain <vaibhav@linux.vnet.ibm.com>
Acked-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

---
This patch applies on top of this patch:
 http://patchwork.ozlabs.org/patch/873663/

Changelog[v6]
 - Rebased to latest upstream.
 - Recompute the status only in native mode.

Changelog[v5]
 - Rebased to latest upstream.
 - Changed the type of 'delta'

Changelog[v4]
 - Rebased to latest upstream.
 - Added log message.

Changelog[v3]
 - Rebased to latest upstream.
 - Dynamic update is now applied to P8.

Changelog[v2]
 - Missing Signed-off-by.
 - Spaces required around the ':'.
---
 drivers/misc/cxl/pci.c   | 17 -----------------
 drivers/misc/cxl/sysfs.c | 12 ++++++++++++
 2 files changed, 12 insertions(+), 17 deletions(-)

diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
index 66eed6a..3247eaf 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -606,9 +606,6 @@ static u64 timebase_read_xsl(struct cxl *adapter)
 
 static void cxl_setup_psl_timebase(struct cxl *adapter, struct pci_dev *dev)
 {
-	u64 psl_tb;
-	int delta;
-	unsigned int retry = 0;
 	struct device_node *np;
 
 	adapter->psl_timebase_synced = false;
@@ -636,20 +633,6 @@ static void cxl_setup_psl_timebase(struct cxl *adapter, struct pci_dev *dev)
 	cxl_p1_write(adapter, CXL_PSL_Control, 0x0000000000000000);
 	cxl_p1_write(adapter, CXL_PSL_Control, CXL_PSL_Control_tb);
 
-	/* Wait until CORE TB and PSL TB difference <= 16usecs */
-	do {
-		msleep(1);
-		if (retry++ > 5) {
-			dev_info(&dev->dev, "PSL timebase can't synchronize\n");
-			return;
-		}
-		psl_tb = adapter->native->sl_ops->timebase_read(adapter);
-		delta = mftb() - psl_tb;
-		if (delta < 0)
-			delta = -delta;
-	} while (tb_to_ns(delta) > 16000);
-
-	adapter->psl_timebase_synced = true;
 	return;
 }
 
diff --git a/drivers/misc/cxl/sysfs.c b/drivers/misc/cxl/sysfs.c
index a8b6d6a..95285b7 100644
--- a/drivers/misc/cxl/sysfs.c
+++ b/drivers/misc/cxl/sysfs.c
@@ -62,7 +62,19 @@ static ssize_t psl_timebase_synced_show(struct device *device,
 					char *buf)
 {
 	struct cxl *adapter = to_cxl_adapter(device);
+	u64 psl_tb, delta;
 
+	/* Recompute the status only in native mode */
+	if (cpu_has_feature(CPU_FTR_HVMODE)) {
+		psl_tb = adapter->native->sl_ops->timebase_read(adapter);
+		delta = abs(mftb() - psl_tb);
+
+		/* CORE TB and PSL TB difference <= 16usecs ? */
+		adapter->psl_timebase_synced = (tb_to_ns(delta) < 16000) ? true : false;
+		pr_devel("PSL timebase %s - delta: 0x%016llx\n",
+			 (tb_to_ns(delta) < 16000) ? "synchronized" :
+			 "not synchronized", tb_to_ns(delta));
+	}
 	return scnprintf(buf, PAGE_SIZE, "%i\n", adapter->psl_timebase_synced);
 }
 
-- 
2.7.4

Re: [PATCH V6] cxl: Fix timebase synchronization status on P9

[Michael Ellerman]

Christophe Lombard <clombard@linux.vnet.ibm.com> writes:

> The PSL Timebase register is updated by the PSL to maintain the
> timebase.
> On P9, the Timebase value is only provided by the CAPP as received
> the last time a timebase request was performed.
> The timebase requests are initiated through the adapter configuration or
> application registers.
> The specific sysfs entry "/sys/class/cxl/cardxx/psl_timebase_synced" is
> now dynamically updated according the content of the PSL Timebase
> register.
>
> Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
> Reviewed-by: Vaibhav Jain <vaibhav@linux.vnet.ibm.com>
> Acked-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

Can you give me a Fixes: tag for this?

Does it need to go to stable? I can't tell from the change log how bad
the bug this fixes is.

cheers

> ---
> This patch applies on top of this patch:
>  http://patchwork.ozlabs.org/patch/873663/
>
> Changelog[v6]
>  - Rebased to latest upstream.
>  - Recompute the status only in native mode.
>
> Changelog[v5]
>  - Rebased to latest upstream.
>  - Changed the type of 'delta'
>
> Changelog[v4]
>  - Rebased to latest upstream.
>  - Added log message.
>
> Changelog[v3]
>  - Rebased to latest upstream.
>  - Dynamic update is now applied to P8.
>
> Changelog[v2]
>  - Missing Signed-off-by.
>  - Spaces required around the ':'.
> ---
>  drivers/misc/cxl/pci.c   | 17 -----------------
>  drivers/misc/cxl/sysfs.c | 12 ++++++++++++
>  2 files changed, 12 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
> index 66eed6a..3247eaf 100644
> --- a/drivers/misc/cxl/pci.c
> +++ b/drivers/misc/cxl/pci.c
> @@ -606,9 +606,6 @@ static u64 timebase_read_xsl(struct cxl *adapter)
>  
>  static void cxl_setup_psl_timebase(struct cxl *adapter, struct pci_dev *dev)
>  {
> -	u64 psl_tb;
> -	int delta;
> -	unsigned int retry = 0;
>  	struct device_node *np;
>  
>  	adapter->psl_timebase_synced = false;
> @@ -636,20 +633,6 @@ static void cxl_setup_psl_timebase(struct cxl *adapter, struct pci_dev *dev)
>  	cxl_p1_write(adapter, CXL_PSL_Control, 0x0000000000000000);
>  	cxl_p1_write(adapter, CXL_PSL_Control, CXL_PSL_Control_tb);
>  
> -	/* Wait until CORE TB and PSL TB difference <= 16usecs */
> -	do {
> -		msleep(1);
> -		if (retry++ > 5) {
> -			dev_info(&dev->dev, "PSL timebase can't synchronize\n");
> -			return;
> -		}
> -		psl_tb = adapter->native->sl_ops->timebase_read(adapter);
> -		delta = mftb() - psl_tb;
> -		if (delta < 0)
> -			delta = -delta;
> -	} while (tb_to_ns(delta) > 16000);
> -
> -	adapter->psl_timebase_synced = true;
>  	return;
>  }
>  
> diff --git a/drivers/misc/cxl/sysfs.c b/drivers/misc/cxl/sysfs.c
> index a8b6d6a..95285b7 100644
> --- a/drivers/misc/cxl/sysfs.c
> +++ b/drivers/misc/cxl/sysfs.c
> @@ -62,7 +62,19 @@ static ssize_t psl_timebase_synced_show(struct device *device,
>  					char *buf)
>  {
>  	struct cxl *adapter = to_cxl_adapter(device);
> +	u64 psl_tb, delta;
>  
> +	/* Recompute the status only in native mode */
> +	if (cpu_has_feature(CPU_FTR_HVMODE)) {
> +		psl_tb = adapter->native->sl_ops->timebase_read(adapter);
> +		delta = abs(mftb() - psl_tb);
> +
> +		/* CORE TB and PSL TB difference <= 16usecs ? */
> +		adapter->psl_timebase_synced = (tb_to_ns(delta) < 16000) ? true : false;
> +		pr_devel("PSL timebase %s - delta: 0x%016llx\n",
> +			 (tb_to_ns(delta) < 16000) ? "synchronized" :
> +			 "not synchronized", tb_to_ns(delta));
> +	}
>  	return scnprintf(buf, PAGE_SIZE, "%i\n", adapter->psl_timebase_synced);
>  }
>  
> -- 
> 2.7.4

Re: [PATCH V6] cxl: Fix timebase synchronization status on P9

[christophe lombard]

Le 21/02/2018 à 03:43, Michael Ellerman a écrit :
> Christophe Lombard <clombard@linux.vnet.ibm.com> writes:
> 
>> The PSL Timebase register is updated by the PSL to maintain the
>> timebase.
>> On P9, the Timebase value is only provided by the CAPP as received
>> the last time a timebase request was performed.
>> The timebase requests are initiated through the adapter configuration or
>> application registers.
>> The specific sysfs entry "/sys/class/cxl/cardxx/psl_timebase_synced" is
>> now dynamically updated according the content of the PSL Timebase
>> register.
>>
>> Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
>> Reviewed-by: Vaibhav Jain <vaibhav@linux.vnet.ibm.com>
>> Acked-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> 
> Can you give me a Fixes: tag for this?
> 
> Does it need to go to stable? I can't tell from the change log how bad
> the bug this fixes is.
> 

Sorry, I completely forgot to add it:
Fixes: f24be42aab37 ("cxl: Add psl9 specific code")

With the current code, the state of the PSL timebase, on P9, is not
displayed correctly because the feature works differently from
what was expected.
This patch fixes this behavior and introduces a new way to get 
dynamically the state of the PSL timebase.

Thanks

> cheers
> 
>> ---
>> This patch applies on top of this patch:
>>   http://patchwork.ozlabs.org/patch/873663/
>>
>> Changelog[v6]
>>   - Rebased to latest upstream.
>>   - Recompute the status only in native mode.
>>
>> Changelog[v5]
>>   - Rebased to latest upstream.
>>   - Changed the type of 'delta'
>>
>> Changelog[v4]
>>   - Rebased to latest upstream.
>>   - Added log message.
>>
>> Changelog[v3]
>>   - Rebased to latest upstream.
>>   - Dynamic update is now applied to P8.
>>
>> Changelog[v2]
>>   - Missing Signed-off-by.
>>   - Spaces required around the ':'.
>> ---
>>   drivers/misc/cxl/pci.c   | 17 -----------------
>>   drivers/misc/cxl/sysfs.c | 12 ++++++++++++
>>   2 files changed, 12 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
>> index 66eed6a..3247eaf 100644
>> --- a/drivers/misc/cxl/pci.c
>> +++ b/drivers/misc/cxl/pci.c
>> @@ -606,9 +606,6 @@ static u64 timebase_read_xsl(struct cxl *adapter)
>>   
>>   static void cxl_setup_psl_timebase(struct cxl *adapter, struct pci_dev *dev)
>>   {
>> -	u64 psl_tb;
>> -	int delta;
>> -	unsigned int retry = 0;
>>   	struct device_node *np;
>>   
>>   	adapter->psl_timebase_synced = false;
>> @@ -636,20 +633,6 @@ static void cxl_setup_psl_timebase(struct cxl *adapter, struct pci_dev *dev)
>>   	cxl_p1_write(adapter, CXL_PSL_Control, 0x0000000000000000);
>>   	cxl_p1_write(adapter, CXL_PSL_Control, CXL_PSL_Control_tb);
>>   
>> -	/* Wait until CORE TB and PSL TB difference <= 16usecs */
>> -	do {
>> -		msleep(1);
>> -		if (retry++ > 5) {
>> -			dev_info(&dev->dev, "PSL timebase can't synchronize\n");
>> -			return;
>> -		}
>> -		psl_tb = adapter->native->sl_ops->timebase_read(adapter);
>> -		delta = mftb() - psl_tb;
>> -		if (delta < 0)
>> -			delta = -delta;
>> -	} while (tb_to_ns(delta) > 16000);
>> -
>> -	adapter->psl_timebase_synced = true;
>>   	return;
>>   }
>>   
>> diff --git a/drivers/misc/cxl/sysfs.c b/drivers/misc/cxl/sysfs.c
>> index a8b6d6a..95285b7 100644
>> --- a/drivers/misc/cxl/sysfs.c
>> +++ b/drivers/misc/cxl/sysfs.c
>> @@ -62,7 +62,19 @@ static ssize_t psl_timebase_synced_show(struct device *device,
>>   					char *buf)
>>   {
>>   	struct cxl *adapter = to_cxl_adapter(device);
>> +	u64 psl_tb, delta;
>>   
>> +	/* Recompute the status only in native mode */
>> +	if (cpu_has_feature(CPU_FTR_HVMODE)) {
>> +		psl_tb = adapter->native->sl_ops->timebase_read(adapter);
>> +		delta = abs(mftb() - psl_tb);
>> +
>> +		/* CORE TB and PSL TB difference <= 16usecs ? */
>> +		adapter->psl_timebase_synced = (tb_to_ns(delta) < 16000) ? true : false;
>> +		pr_devel("PSL timebase %s - delta: 0x%016llx\n",
>> +			 (tb_to_ns(delta) < 16000) ? "synchronized" :
>> +			 "not synchronized", tb_to_ns(delta));
>> +	}
>>   	return scnprintf(buf, PAGE_SIZE, "%i\n", adapter->psl_timebase_synced);
>>   }
>>   
>> -- 
>> 2.7.4
> 

Re: [PATCH V6] cxl: Fix timebase synchronization status on P9

[Frederic Barrat]

Le 20/02/2018 à 14:48, Christophe Lombard a écrit :
> The PSL Timebase register is updated by the PSL to maintain the
> timebase.
> On P9, the Timebase value is only provided by the CAPP as received
> the last time a timebase request was performed.
> The timebase requests are initiated through the adapter configuration or
> application registers.
> The specific sysfs entry "/sys/class/cxl/cardxx/psl_timebase_synced" is
> now dynamically updated according the content of the PSL Timebase
> register.
> 
> Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
> Reviewed-by: Vaibhav Jain <vaibhav@linux.vnet.ibm.com>
> Acked-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> 
> ---


Thanks!
Acked-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>


> This patch applies on top of this patch:
>   http://patchwork.ozlabs.org/patch/873663/
> 
> Changelog[v6]
>   - Rebased to latest upstream.
>   - Recompute the status only in native mode.
> 
> Changelog[v5]
>   - Rebased to latest upstream.
>   - Changed the type of 'delta'
> 
> Changelog[v4]
>   - Rebased to latest upstream.
>   - Added log message.
> 
> Changelog[v3]
>   - Rebased to latest upstream.
>   - Dynamic update is now applied to P8.
> 
> Changelog[v2]
>   - Missing Signed-off-by.
>   - Spaces required around the ':'.
> ---
>   drivers/misc/cxl/pci.c   | 17 -----------------
>   drivers/misc/cxl/sysfs.c | 12 ++++++++++++
>   2 files changed, 12 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
> index 66eed6a..3247eaf 100644
> --- a/drivers/misc/cxl/pci.c
> +++ b/drivers/misc/cxl/pci.c
> @@ -606,9 +606,6 @@ static u64 timebase_read_xsl(struct cxl *adapter)
> 
>   static void cxl_setup_psl_timebase(struct cxl *adapter, struct pci_dev *dev)
>   {
> -	u64 psl_tb;
> -	int delta;
> -	unsigned int retry = 0;
>   	struct device_node *np;
> 
>   	adapter->psl_timebase_synced = false;
> @@ -636,20 +633,6 @@ static void cxl_setup_psl_timebase(struct cxl *adapter, struct pci_dev *dev)
>   	cxl_p1_write(adapter, CXL_PSL_Control, 0x0000000000000000);
>   	cxl_p1_write(adapter, CXL_PSL_Control, CXL_PSL_Control_tb);
> 
> -	/* Wait until CORE TB and PSL TB difference <= 16usecs */
> -	do {
> -		msleep(1);
> -		if (retry++ > 5) {
> -			dev_info(&dev->dev, "PSL timebase can't synchronize\n");
> -			return;
> -		}
> -		psl_tb = adapter->native->sl_ops->timebase_read(adapter);
> -		delta = mftb() - psl_tb;
> -		if (delta < 0)
> -			delta = -delta;
> -	} while (tb_to_ns(delta) > 16000);
> -
> -	adapter->psl_timebase_synced = true;
>   	return;
>   }
> 
> diff --git a/drivers/misc/cxl/sysfs.c b/drivers/misc/cxl/sysfs.c
> index a8b6d6a..95285b7 100644
> --- a/drivers/misc/cxl/sysfs.c
> +++ b/drivers/misc/cxl/sysfs.c
> @@ -62,7 +62,19 @@ static ssize_t psl_timebase_synced_show(struct device *device,
>   					char *buf)
>   {
>   	struct cxl *adapter = to_cxl_adapter(device);
> +	u64 psl_tb, delta;
> 
> +	/* Recompute the status only in native mode */
> +	if (cpu_has_feature(CPU_FTR_HVMODE)) {
> +		psl_tb = adapter->native->sl_ops->timebase_read(adapter);
> +		delta = abs(mftb() - psl_tb);
> +
> +		/* CORE TB and PSL TB difference <= 16usecs ? */
> +		adapter->psl_timebase_synced = (tb_to_ns(delta) < 16000) ? true : false;
> +		pr_devel("PSL timebase %s - delta: 0x%016llx\n",
> +			 (tb_to_ns(delta) < 16000) ? "synchronized" :
> +			 "not synchronized", tb_to_ns(delta));
> +	}
>   	return scnprintf(buf, PAGE_SIZE, "%i\n", adapter->psl_timebase_synced);
>   }
> 

Re: [V6] cxl: Fix timebase synchronization status on P9

[Michael Ellerman]

On Tue, 2018-02-20 at 13:48:56 UTC, Christophe Lombard wrote:
> The PSL Timebase register is updated by the PSL to maintain the
> timebase.
> On P9, the Timebase value is only provided by the CAPP as received
> the last time a timebase request was performed.
> The timebase requests are initiated through the adapter configuration or
> application registers.
> The specific sysfs entry "/sys/class/cxl/cardxx/psl_timebase_synced" is
> now dynamically updated according the content of the PSL Timebase
> register.
> 
> Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
> Reviewed-by: Vaibhav Jain <vaibhav@linux.vnet.ibm.com>
> Acked-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> Acked-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/c2be663d5307fb9751a562ac664fa7

cheers