Message ID | 20240715172835.24757-7-alejandro.lucero-palau@amd.com |
---|---|
State | Superseded |
Headers | show |
Series | cxl: add Type2 device support | expand |
On Mon, 15 Jul 2024 18:28:26 +0100 alejandro.lucero-palau@amd.com wrote: > From: Alejandro Lucero <alucerop@amd.com> > > A Type-2 driver can require to set the memory availability explicitly. > > Add a function to the exported CXL API for accelerator drivers. > > Signed-off-by: Alejandro Lucero <alucerop@amd.com> > --- > drivers/cxl/core/memdev.c | 7 ++++++- > drivers/net/ethernet/sfc/efx_cxl.c | 5 +++++ > include/linux/cxl_accel_mem.h | 2 ++ > 3 files changed, 13 insertions(+), 1 deletion(-) > > diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c > index b4205ecca365..58a51e7fd37f 100644 > --- a/drivers/cxl/core/memdev.c > +++ b/drivers/cxl/core/memdev.c > @@ -714,7 +714,6 @@ static int cxl_memdev_open(struct inode *inode, struct file *file) > return 0; > } > > - Grumpy maintainer time ;) Scrub for this stuff before posting. Move the whitespace cleanup to the earlier patch so we have less noise here. > void cxl_accel_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec) > { > cxlds->cxl_dvsec = dvsec; > @@ -759,6 +758,12 @@ int cxl_accel_request_resource(struct cxl_dev_state *cxlds, bool is_ram) > } > EXPORT_SYMBOL_NS_GPL(cxl_accel_request_resource, CXL); > > +void cxl_accel_set_media_ready(struct cxl_dev_state *cxlds) > +{ > + cxlds->media_ready = true; > +} > +EXPORT_SYMBOL_NS_GPL(cxl_accel_set_media_ready, CXL); > + > static int cxl_memdev_release_file(struct inode *inode, struct file *file) > { > struct cxl_memdev *cxlmd = > diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c > index 37d8bfdef517..a84fe7992c53 100644 > --- a/drivers/net/ethernet/sfc/efx_cxl.c > +++ b/drivers/net/ethernet/sfc/efx_cxl.c > @@ -56,6 +56,11 @@ void efx_cxl_init(struct efx_nic *efx) > > if (cxl_accel_request_resource(cxl->cxlds, true)) > pci_info(pci_dev, "CXL accel resource request failed"); > + > + if (!cxl_await_media_ready(cxl->cxlds)) > + cxl_accel_set_media_ready(cxl->cxlds); > + else > + pci_info(pci_dev, "CXL accel media not active"); Feels fatal. pci_err() and return an error. > } > > > diff --git a/include/linux/cxl_accel_mem.h b/include/linux/cxl_accel_mem.h > index 0ba2195b919b..b883c438a132 100644 > --- a/include/linux/cxl_accel_mem.h > +++ b/include/linux/cxl_accel_mem.h > @@ -24,4 +24,6 @@ void cxl_accel_set_resource(struct cxl_dev_state *cxlds, struct resource res, > enum accel_resource); > int cxl_pci_accel_setup_regs(struct pci_dev *pdev, struct cxl_dev_state *cxlds); > int cxl_accel_request_resource(struct cxl_dev_state *cxlds, bool is_ram); > +void cxl_accel_set_media_ready(struct cxl_dev_state *cxlds); > +int cxl_await_media_ready(struct cxl_dev_state *cxlds); > #endif
On 8/4/24 18:26, Jonathan Cameron wrote: > On Mon, 15 Jul 2024 18:28:26 +0100 > alejandro.lucero-palau@amd.com wrote: > >> From: Alejandro Lucero <alucerop@amd.com> >> >> A Type-2 driver can require to set the memory availability explicitly. >> >> Add a function to the exported CXL API for accelerator drivers. >> >> Signed-off-by: Alejandro Lucero <alucerop@amd.com> >> --- >> drivers/cxl/core/memdev.c | 7 ++++++- >> drivers/net/ethernet/sfc/efx_cxl.c | 5 +++++ >> include/linux/cxl_accel_mem.h | 2 ++ >> 3 files changed, 13 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c >> index b4205ecca365..58a51e7fd37f 100644 >> --- a/drivers/cxl/core/memdev.c >> +++ b/drivers/cxl/core/memdev.c >> @@ -714,7 +714,6 @@ static int cxl_memdev_open(struct inode *inode, struct file *file) >> return 0; >> } >> >> - > Grumpy maintainer time ;) > Scrub for this stuff before posting. Move the whitespace cleanup to the > earlier patch so we have less noise here. > I will avoid this kind of things in v3. >> void cxl_accel_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec) >> { >> cxlds->cxl_dvsec = dvsec; >> @@ -759,6 +758,12 @@ int cxl_accel_request_resource(struct cxl_dev_state *cxlds, bool is_ram) >> } >> EXPORT_SYMBOL_NS_GPL(cxl_accel_request_resource, CXL); >> >> +void cxl_accel_set_media_ready(struct cxl_dev_state *cxlds) >> +{ >> + cxlds->media_ready = true; >> +} >> +EXPORT_SYMBOL_NS_GPL(cxl_accel_set_media_ready, CXL); >> + >> static int cxl_memdev_release_file(struct inode *inode, struct file *file) >> { >> struct cxl_memdev *cxlmd = >> diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c >> index 37d8bfdef517..a84fe7992c53 100644 >> --- a/drivers/net/ethernet/sfc/efx_cxl.c >> +++ b/drivers/net/ethernet/sfc/efx_cxl.c >> @@ -56,6 +56,11 @@ void efx_cxl_init(struct efx_nic *efx) >> >> if (cxl_accel_request_resource(cxl->cxlds, true)) >> pci_info(pci_dev, "CXL accel resource request failed"); >> + >> + if (!cxl_await_media_ready(cxl->cxlds)) >> + cxl_accel_set_media_ready(cxl->cxlds); >> + else >> + pci_info(pci_dev, "CXL accel media not active"); > Feels fatal. pci_err() and return an error. As I commented yesterday when this patch was pointed to in another patch review, this is unnecessary in our case and it will be fixed in next version: cxl_await_media_ready will not be invoked only using the accessor for manually setting the media ready. Thanks >> } >> >> >> diff --git a/include/linux/cxl_accel_mem.h b/include/linux/cxl_accel_mem.h >> index 0ba2195b919b..b883c438a132 100644 >> --- a/include/linux/cxl_accel_mem.h >> +++ b/include/linux/cxl_accel_mem.h >> @@ -24,4 +24,6 @@ void cxl_accel_set_resource(struct cxl_dev_state *cxlds, struct resource res, >> enum accel_resource); >> int cxl_pci_accel_setup_regs(struct pci_dev *pdev, struct cxl_dev_state *cxlds); >> int cxl_accel_request_resource(struct cxl_dev_state *cxlds, bool is_ram); >> +void cxl_accel_set_media_ready(struct cxl_dev_state *cxlds); >> +int cxl_await_media_ready(struct cxl_dev_state *cxlds); >> #endif
diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c index b4205ecca365..58a51e7fd37f 100644 --- a/drivers/cxl/core/memdev.c +++ b/drivers/cxl/core/memdev.c @@ -714,7 +714,6 @@ static int cxl_memdev_open(struct inode *inode, struct file *file) return 0; } - void cxl_accel_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec) { cxlds->cxl_dvsec = dvsec; @@ -759,6 +758,12 @@ int cxl_accel_request_resource(struct cxl_dev_state *cxlds, bool is_ram) } EXPORT_SYMBOL_NS_GPL(cxl_accel_request_resource, CXL); +void cxl_accel_set_media_ready(struct cxl_dev_state *cxlds) +{ + cxlds->media_ready = true; +} +EXPORT_SYMBOL_NS_GPL(cxl_accel_set_media_ready, CXL); + static int cxl_memdev_release_file(struct inode *inode, struct file *file) { struct cxl_memdev *cxlmd = diff --git a/drivers/net/ethernet/sfc/efx_cxl.c b/drivers/net/ethernet/sfc/efx_cxl.c index 37d8bfdef517..a84fe7992c53 100644 --- a/drivers/net/ethernet/sfc/efx_cxl.c +++ b/drivers/net/ethernet/sfc/efx_cxl.c @@ -56,6 +56,11 @@ void efx_cxl_init(struct efx_nic *efx) if (cxl_accel_request_resource(cxl->cxlds, true)) pci_info(pci_dev, "CXL accel resource request failed"); + + if (!cxl_await_media_ready(cxl->cxlds)) + cxl_accel_set_media_ready(cxl->cxlds); + else + pci_info(pci_dev, "CXL accel media not active"); } diff --git a/include/linux/cxl_accel_mem.h b/include/linux/cxl_accel_mem.h index 0ba2195b919b..b883c438a132 100644 --- a/include/linux/cxl_accel_mem.h +++ b/include/linux/cxl_accel_mem.h @@ -24,4 +24,6 @@ void cxl_accel_set_resource(struct cxl_dev_state *cxlds, struct resource res, enum accel_resource); int cxl_pci_accel_setup_regs(struct pci_dev *pdev, struct cxl_dev_state *cxlds); int cxl_accel_request_resource(struct cxl_dev_state *cxlds, bool is_ram); +void cxl_accel_set_media_ready(struct cxl_dev_state *cxlds); +int cxl_await_media_ready(struct cxl_dev_state *cxlds); #endif