diff mbox series

[3/3] cxl: Move read_cdat_data() to after media is ready

Message ID 168428297113.2205351.16197713607932380832.stgit@djiang5-mobl3
State Superseded
Headers show
Series cxl: Move operations after memory is ready | expand

Commit Message

Dave Jiang May 17, 2023, 12:22 a.m. UTC
CDAT data is only valid after the media is ready. Move read_cdat_data() to
after cxl_await_media_read() is successful.

Fixes: 32ce3f185bbb ("cxl/port: Split endpoint and switch port probe")
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/cxl/port.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Ira Weiny May 18, 2023, 2:55 a.m. UTC | #1
Dave Jiang wrote:
> CDAT data is only valid after the media is ready. Move read_cdat_data() to
> after cxl_await_media_read() is successful.
> 
> Fixes: 32ce3f185bbb ("cxl/port: Split endpoint and switch port probe")
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/cxl/port.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
> index 40e7b7de8bf6..bebc297fde5c 100644
> --- a/drivers/cxl/port.c
> +++ b/drivers/cxl/port.c
> @@ -96,9 +96,6 @@ static int cxl_endpoint_port_probe(struct cxl_port *port)
>  	if (IS_ERR(cxlhdm))
>  		return PTR_ERR(cxlhdm);
>  
> -	/* Cache the data early to ensure is_visible() works */
> -	read_cdat_data(port);
> -
>  	get_device(&cxlmd->dev);
>  	rc = devm_add_action_or_reset(&port->dev, schedule_detach, cxlmd);
>  	if (rc)
> @@ -114,6 +111,9 @@ static int cxl_endpoint_port_probe(struct cxl_port *port)
>  		return rc;
>  	}
>  
> +	/* Cache the data early to ensure is_visible() works */
> +	read_cdat_data(port);
> +

Does this apply to the latest master?  I'm not seeing
cxl_dev_state_identify() in the current cxl_endpoint_port_probe()?

I think this is good and needs to go in but I'm not sure where this call
is landing now.  I assume it is right after cxl_await_media_ready()?

Ira

>  	rc = cxl_dev_state_identify(cxlds);
>  	if (rc)
>  		return rc;
> 
>
Ira Weiny May 18, 2023, 2:59 a.m. UTC | #2
Ira Weiny wrote:
> Dave Jiang wrote:
> > CDAT data is only valid after the media is ready. Move read_cdat_data() to
> > after cxl_await_media_read() is successful.
> > 
> > Fixes: 32ce3f185bbb ("cxl/port: Split endpoint and switch port probe")
> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> > ---
> >  drivers/cxl/port.c |    6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
> > index 40e7b7de8bf6..bebc297fde5c 100644
> > --- a/drivers/cxl/port.c
> > +++ b/drivers/cxl/port.c
> > @@ -96,9 +96,6 @@ static int cxl_endpoint_port_probe(struct cxl_port *port)
> >  	if (IS_ERR(cxlhdm))
> >  		return PTR_ERR(cxlhdm);
> >  
> > -	/* Cache the data early to ensure is_visible() works */
> > -	read_cdat_data(port);
> > -
> >  	get_device(&cxlmd->dev);
> >  	rc = devm_add_action_or_reset(&port->dev, schedule_detach, cxlmd);
> >  	if (rc)
> > @@ -114,6 +111,9 @@ static int cxl_endpoint_port_probe(struct cxl_port *port)
> >  		return rc;
> >  	}
> >  
> > +	/* Cache the data early to ensure is_visible() works */
> > +	read_cdat_data(port);
> > +
> 
> Does this apply to the latest master?  I'm not seeing
> cxl_dev_state_identify() in the current cxl_endpoint_port_probe()?
> 
> I think this is good and needs to go in but I'm not sure where this call
> is landing now.  I assume it is right after cxl_await_media_ready()?

<sigh> Never mind...  I see now that patch 2/3 added
cxl_await_media_ready()

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

> 
> Ira
> 
> >  	rc = cxl_dev_state_identify(cxlds);
> >  	if (rc)
> >  		return rc;
> > 
> > 
> 
>
Dan Williams May 18, 2023, 3:45 p.m. UTC | #3
Dave Jiang wrote:
> CDAT data is only valid after the media is ready. Move read_cdat_data() to
> after cxl_await_media_read() is successful.

s/cxl_await_media_read()/cxl_await_media_ready()/

> Fixes: 32ce3f185bbb ("cxl/port: Split endpoint and switch port probe")

No, c97006046c79 ("cxl/port: Read CDAT table")

...but I think because of the realization of where await-media-ready
needs to move to make sure that basic capacity information is settled relative
to when the attributes become visible. This call can stay where its at
and assume that the port would not have been registered unless someone
up the stack had successfully probed for capacity.

> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/cxl/port.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
> index 40e7b7de8bf6..bebc297fde5c 100644
> --- a/drivers/cxl/port.c
> +++ b/drivers/cxl/port.c
> @@ -96,9 +96,6 @@ static int cxl_endpoint_port_probe(struct cxl_port *port)
>  	if (IS_ERR(cxlhdm))
>  		return PTR_ERR(cxlhdm);
>  
> -	/* Cache the data early to ensure is_visible() works */
> -	read_cdat_data(port);
> -
>  	get_device(&cxlmd->dev);
>  	rc = devm_add_action_or_reset(&port->dev, schedule_detach, cxlmd);
>  	if (rc)
> @@ -114,6 +111,9 @@ static int cxl_endpoint_port_probe(struct cxl_port *port)
>  		return rc;
>  	}
>  
> +	/* Cache the data early to ensure is_visible() works */
> +	read_cdat_data(port);
> +
>  	rc = cxl_dev_state_identify(cxlds);
>  	if (rc)
>  		return rc;
> 
>
diff mbox series

Patch

diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
index 40e7b7de8bf6..bebc297fde5c 100644
--- a/drivers/cxl/port.c
+++ b/drivers/cxl/port.c
@@ -96,9 +96,6 @@  static int cxl_endpoint_port_probe(struct cxl_port *port)
 	if (IS_ERR(cxlhdm))
 		return PTR_ERR(cxlhdm);
 
-	/* Cache the data early to ensure is_visible() works */
-	read_cdat_data(port);
-
 	get_device(&cxlmd->dev);
 	rc = devm_add_action_or_reset(&port->dev, schedule_detach, cxlmd);
 	if (rc)
@@ -114,6 +111,9 @@  static int cxl_endpoint_port_probe(struct cxl_port *port)
 		return rc;
 	}
 
+	/* Cache the data early to ensure is_visible() works */
+	read_cdat_data(port);
+
 	rc = cxl_dev_state_identify(cxlds);
 	if (rc)
 		return rc;