| Message ID | 20240809093442.646545-5-yanfei.xu@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fixes for hdm docoder initialization from DVSEC ranges | expand |
Yanfei Xu wrote: > When HDM decoders exist but is not enabled, the cases can be divided into > two categories: DVSEC range enabled and not enabled. Extract the check of > mem_enabled out to improve code readability. No functional change. I am not convinced that this is clear win, and that there is no functional change.
On 8/10/2024 3:15 AM, Dan Williams wrote: > Yanfei Xu wrote: >> When HDM decoders exist but is not enabled, the cases can be divided into >> two categories: DVSEC range enabled and not enabled. Extract the check of >> mem_enabled out to improve code readability. No functional change. > > I am not convinced that this is clear win, and that there is no > functional change. Yeah, no functional change. Before the patch2 which moves the check of dvsec_range_allowed() earlier, the previous codes in cxl_hdm_decode_init() is like below: .... for (i = 0, allowed = 0; info->mem_enabled && i < info->ranges; i++) { .... } if (!allowed && info->mem_enabled) { .... } .... if (info->mem_enabled) return 0; It checks "info->mem_enabled" consecutively three times, so if extracting the check and replace it with one check of "!info->mem_enabled" can improve code readability I was thinking. However with applying of patch2, maybe this patch is not much necessary, we can drop it. :) Thanks, Yanfei
diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c index e822cc9ce315..09d63a62f05b 100644 --- a/drivers/cxl/core/pci.c +++ b/drivers/cxl/core/pci.c @@ -438,7 +438,15 @@ int cxl_hdm_decode_init(struct cxl_dev_state *cxlds, struct cxl_hdm *cxlhdm, else if (!hdm) return -ENODEV; - if (!info->ranges && info->mem_enabled) { + if (!info->mem_enabled) { + rc = devm_cxl_enable_hdm(&port->dev, cxlhdm); + if (rc) + return rc; + + return devm_cxl_enable_mem(&port->dev, cxlds); + } + + if (!info->ranges) { dev_err(dev, "No available DVSEC register ranges.\n"); return -ENXIO; } @@ -452,14 +460,7 @@ int cxl_hdm_decode_init(struct cxl_dev_state *cxlds, struct cxl_hdm *cxlhdm, * match. If at least one DVSEC range is enabled and allowed, skip HDM * Decoder Capability Enable. */ - if (info->mem_enabled) - return 0; - - rc = devm_cxl_enable_hdm(&port->dev, cxlhdm); - if (rc) - return rc; - - return devm_cxl_enable_mem(&port->dev, cxlds); + return 0; } EXPORT_SYMBOL_NS_GPL(cxl_hdm_decode_init, CXL);
When HDM decoders exist but is not enabled, the cases can be divided into two categories: DVSEC range enabled and not enabled. Extract the check of mem_enabled out to improve code readability. No functional change. Signed-off-by: Yanfei Xu <yanfei.xu@intel.com> --- drivers/cxl/core/pci.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-)