diff mbox series

[16/18] cxl: Move reading of CDAT data from device to after media is ready

Message ID 167571670516.587790.14112456054041985666.stgit@djiang5-mobl3.local
State New
Headers show
Series cxl: Add support for QTG ID retrieval for CXL subsystem | expand

Commit Message

Dave Jiang Feb. 6, 2023, 8:51 p.m. UTC
The CDAT data is only valid after the hardware signals the media is ready.
Move the reading to after cxl_await_media_ready() has succeeded.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/cxl/port.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Lukas Wunner Feb. 6, 2023, 10:17 p.m. UTC | #1
On Mon, Feb 06, 2023 at 01:51:46PM -0700, Dave Jiang wrote:
> --- a/drivers/cxl/port.c
> +++ b/drivers/cxl/port.c
> @@ -109,6 +106,8 @@ static int cxl_port_probe(struct device *dev)
>  			return rc;
>  		}
>  
> +		/* Cache the data early to ensure is_visible() works */
> +		read_cdat_data(port);
>  		if (port->cdat.table) {
>  			rc = cdat_table_parse_dsmas(port->cdat.table,
>  						    cxl_dsmas_parse_entry,

Which branch is this patch based on?  I'm not seeing a function
called cdat_table_parse_dsmas() in cxl/next.

cxl_cdat_read_table() could be amended with a switch/case ladder
which compares entry->type to acpi_cdat_type values and stores
a pointer to an entry of interest e.g. in port->cdat->dsmas.
Then you can use that pointer directly to find the dsmas in the
CDAT and parse it.

Note however that cxl_cdat_read_table() is refactored heavily by
my DOE rework series (will submit v3 later this week):

https://github.com/l1k/linux/commits/doe

Thanks,

Lukas
Dave Jiang Feb. 7, 2023, 8:55 p.m. UTC | #2
On 2/6/23 3:17 PM, Lukas Wunner wrote:
> On Mon, Feb 06, 2023 at 01:51:46PM -0700, Dave Jiang wrote:
>> --- a/drivers/cxl/port.c
>> +++ b/drivers/cxl/port.c
>> @@ -109,6 +106,8 @@ static int cxl_port_probe(struct device *dev)
>>   			return rc;
>>   		}
>>   
>> +		/* Cache the data early to ensure is_visible() works */
>> +		read_cdat_data(port);
>>   		if (port->cdat.table) {
>>   			rc = cdat_table_parse_dsmas(port->cdat.table,
>>   						    cxl_dsmas_parse_entry,
> 
> Which branch is this patch based on?  I'm not seeing a function
> called cdat_table_parse_dsmas() in cxl/next.

v6.2-rc7. See commit 4/18. That's where it's introduced. I adapted it 
from ACPI entries parsing code.

> 
> cxl_cdat_read_table() could be amended with a switch/case ladder
> which compares entry->type to acpi_cdat_type values and stores
> a pointer to an entry of interest e.g. in port->cdat->dsmas.
> Then you can use that pointer directly to find the dsmas in the
> CDAT and parse it.

Yes, but we may have more than 1 DSMAS right? Plus having to parse the 
DSLBIS entries as well, may be better to just have a common parsing 
routine to deal with all that.
> 
> Note however that cxl_cdat_read_table() is refactored heavily by
> my DOE rework series (will submit v3 later this week):
> 
> https://github.com/l1k/linux/commits/doe
> 
> Thanks,
> 
> Lukas
Jonathan Cameron Feb. 9, 2023, 3:31 p.m. UTC | #3
On Mon, 06 Feb 2023 13:51:46 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> The CDAT data is only valid after the hardware signals the media is ready.
> Move the reading to after cxl_await_media_ready() has succeeded.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Fix?  Though I doubt we care about backporting this one as until
after this patch series, CDAT was mostly informational so hopefully
no one relies on it.

Jonathan

> ---
>  drivers/cxl/port.c |    5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
> index b7a4a1be2945..6b2ad22487f5 100644
> --- a/drivers/cxl/port.c
> +++ b/drivers/cxl/port.c
> @@ -91,9 +91,6 @@ static int cxl_port_probe(struct device *dev)
>  		struct cxl_memdev *cxlmd = to_cxl_memdev(port->uport);
>  		struct cxl_dev_state *cxlds = cxlmd->cxlds;
>  
> -		/* Cache the data early to ensure is_visible() works */
> -		read_cdat_data(port);
> -
>  		get_device(&cxlmd->dev);
>  		rc = devm_add_action_or_reset(dev, schedule_detach, cxlmd);
>  		if (rc)
> @@ -109,6 +106,8 @@ static int cxl_port_probe(struct device *dev)
>  			return rc;
>  		}
>  
> +		/* Cache the data early to ensure is_visible() works */
> +		read_cdat_data(port);
>  		if (port->cdat.table) {
>  			rc = cdat_table_parse_dsmas(port->cdat.table,
>  						    cxl_dsmas_parse_entry,
> 
>
diff mbox series

Patch

diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
index b7a4a1be2945..6b2ad22487f5 100644
--- a/drivers/cxl/port.c
+++ b/drivers/cxl/port.c
@@ -91,9 +91,6 @@  static int cxl_port_probe(struct device *dev)
 		struct cxl_memdev *cxlmd = to_cxl_memdev(port->uport);
 		struct cxl_dev_state *cxlds = cxlmd->cxlds;
 
-		/* Cache the data early to ensure is_visible() works */
-		read_cdat_data(port);
-
 		get_device(&cxlmd->dev);
 		rc = devm_add_action_or_reset(dev, schedule_detach, cxlmd);
 		if (rc)
@@ -109,6 +106,8 @@  static int cxl_port_probe(struct device *dev)
 			return rc;
 		}
 
+		/* Cache the data early to ensure is_visible() works */
+		read_cdat_data(port);
 		if (port->cdat.table) {
 			rc = cdat_table_parse_dsmas(port->cdat.table,
 						    cxl_dsmas_parse_entry,