Message ID | 168428297113.2205351.16197713607932380832.stgit@djiang5-mobl3 |
---|---|
State | Superseded |
Headers | show |
Series | cxl: Move operations after memory is ready | expand |
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 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; > > > > > >
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 --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;