| Message ID | 20240827045755.1837473-1-ming4.li@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/3] cxl/pci: Rename cxl_setup_parent_dport() and cxl_dport_map_regs() | expand |
On Tue, 27 Aug 2024 04:57:53 +0000 Li Ming <ming4.li@intel.com> wrote: > The name of cxl_setup_parent_dport() function is not clear, the function > is used to initialize AER and RAS capabilities on a dport, therefore, > rename the function to cxl_dport_init_aer(), it is easier for user to > understand what the function does. Besides, adjust the order of the > function parameters, the subject of cxl_dport_init_aer() is a cxl port, Hmm. It's not just aer, so maybe cxl_dport_init_ras_reporting() is broader naming that incorporates the fact this includes the CXL specific stuff. Obvious that might be used to get more detail on an AER error report, but it's not part of aer. Don't want to just use _ras() because of all the ras control stuff that isn't related to this function. Otherwise LGTM. Jonathan > so a struct cxl_dport as the first parameter of the function should be > better. > > cxl_dport_map_regs() is used to map CXL RAS capability on a cxl dport, > using cxl_dport_map_ras() as the function name. > > Signed-off-by: Li Ming <ming4.li@intel.com>
On 8/27/2024 10:56 PM, Jonathan Cameron wrote: > On Tue, 27 Aug 2024 04:57:53 +0000 > Li Ming <ming4.li@intel.com> wrote: > >> The name of cxl_setup_parent_dport() function is not clear, the function >> is used to initialize AER and RAS capabilities on a dport, therefore, >> rename the function to cxl_dport_init_aer(), it is easier for user to >> understand what the function does. Besides, adjust the order of the >> function parameters, the subject of cxl_dport_init_aer() is a cxl port, > Hmm. It's not just aer, so maybe cxl_dport_init_ras_reporting() is broader > naming that incorporates the fact this includes the CXL specific stuff. > Obvious that might be used to get more detail on an AER error report, > but it's not part of aer. > > Don't want to just use _ras() because of all the ras control stuff that > isn't related to this function. > > Otherwise LGTM. > > Jonathan Thank you for review, will do it in v2. >> so a struct cxl_dport as the first parameter of the function should be >> better. >> >> cxl_dport_map_regs() is used to map CXL RAS capability on a cxl dport, >> using cxl_dport_map_ras() as the function name. >> >> Signed-off-by: Li Ming <ming4.li@intel.com>
diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c index 51132a575b27..9761178372b6 100644 --- a/drivers/cxl/core/pci.c +++ b/drivers/cxl/core/pci.c @@ -787,7 +787,7 @@ static void cxl_dport_map_rch_aer(struct cxl_dport *dport) dport->regs.dport_aer = dport_aer; } -static void cxl_dport_map_regs(struct cxl_dport *dport) +static void cxl_dport_map_ras(struct cxl_dport *dport) { struct cxl_register_map *map = &dport->reg_map; struct device *dev = dport->dport_dev; @@ -831,7 +831,12 @@ static void cxl_disable_rch_root_ints(struct cxl_dport *dport) } } -void cxl_setup_parent_dport(struct device *host, struct cxl_dport *dport) +/** + * cxl_dport_init_aer - Setup AER on this dport + * @dport: the cxl_dport that needs to be initialized + * @host: host device for devm operations + */ +void cxl_dport_init_aer(struct cxl_dport *dport, struct device *host) { struct device *dport_dev = dport->dport_dev; @@ -843,12 +848,12 @@ void cxl_setup_parent_dport(struct device *host, struct cxl_dport *dport) } dport->reg_map.host = host; - cxl_dport_map_regs(dport); + cxl_dport_map_ras(dport); if (dport->rch) cxl_disable_rch_root_ints(dport); } -EXPORT_SYMBOL_NS_GPL(cxl_setup_parent_dport, CXL); +EXPORT_SYMBOL_NS_GPL(cxl_dport_init_aer, CXL); static void cxl_handle_rdport_cor_ras(struct cxl_dev_state *cxlds, struct cxl_dport *dport) diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h index 9afb407d438f..c4b7a0d6bb22 100644 --- a/drivers/cxl/cxl.h +++ b/drivers/cxl/cxl.h @@ -761,10 +761,10 @@ struct cxl_dport *devm_cxl_add_rch_dport(struct cxl_port *port, resource_size_t rcrb); #ifdef CONFIG_PCIEAER_CXL -void cxl_setup_parent_dport(struct device *host, struct cxl_dport *dport); +void cxl_dport_init_aer(struct cxl_dport *dport, struct device *host); #else -static inline void cxl_setup_parent_dport(struct device *host, - struct cxl_dport *dport) { } +static inline void cxl_dport_init_aer(struct cxl_dport *dport, + struct device *host) { } #endif struct cxl_decoder *to_cxl_decoder(struct device *dev); diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c index 7de232eaeb17..e464ec3681bb 100644 --- a/drivers/cxl/mem.c +++ b/drivers/cxl/mem.c @@ -166,7 +166,7 @@ static int cxl_mem_probe(struct device *dev) else endpoint_parent = &parent_port->dev; - cxl_setup_parent_dport(dev, dport); + cxl_dport_init_aer(dport, dev); device_lock(endpoint_parent); if (!endpoint_parent->driver) { diff --git a/tools/testing/cxl/Kbuild b/tools/testing/cxl/Kbuild index 3d1ca9e38b1f..f9eb12eb3f1b 100644 --- a/tools/testing/cxl/Kbuild +++ b/tools/testing/cxl/Kbuild @@ -14,7 +14,7 @@ ldflags-y += --wrap=cxl_dvsec_rr_decode ldflags-y += --wrap=devm_cxl_add_rch_dport ldflags-y += --wrap=cxl_rcd_component_reg_phys ldflags-y += --wrap=cxl_endpoint_parse_cdat -ldflags-y += --wrap=cxl_setup_parent_dport +ldflags-y += --wrap=cxl_dport_init_aer DRIVERS := ../../../drivers CXL_SRC := $(DRIVERS)/cxl diff --git a/tools/testing/cxl/test/mock.c b/tools/testing/cxl/test/mock.c index d619672faa49..6cf89e16fa44 100644 --- a/tools/testing/cxl/test/mock.c +++ b/tools/testing/cxl/test/mock.c @@ -299,17 +299,17 @@ void __wrap_cxl_endpoint_parse_cdat(struct cxl_port *port) } EXPORT_SYMBOL_NS_GPL(__wrap_cxl_endpoint_parse_cdat, CXL); -void __wrap_cxl_setup_parent_dport(struct device *host, struct cxl_dport *dport) +void __wrap_cxl_dport_init_aer(struct cxl_dport *dport, struct device *host) { int index; struct cxl_mock_ops *ops = get_cxl_mock_ops(&index); if (!ops || !ops->is_mock_port(dport->dport_dev)) - cxl_setup_parent_dport(host, dport); + cxl_dport_init_aer(dport, host); put_cxl_mock_ops(index); } -EXPORT_SYMBOL_NS_GPL(__wrap_cxl_setup_parent_dport, CXL); +EXPORT_SYMBOL_NS_GPL(__wrap_cxl_dport_init_aer, CXL); MODULE_LICENSE("GPL v2"); MODULE_IMPORT_NS(ACPI);
The name of cxl_setup_parent_dport() function is not clear, the function is used to initialize AER and RAS capabilities on a dport, therefore, rename the function to cxl_dport_init_aer(), it is easier for user to understand what the function does. Besides, adjust the order of the function parameters, the subject of cxl_dport_init_aer() is a cxl port, so a struct cxl_dport as the first parameter of the function should be better. cxl_dport_map_regs() is used to map CXL RAS capability on a cxl dport, using cxl_dport_map_ras() as the function name. Signed-off-by: Li Ming <ming4.li@intel.com> --- drivers/cxl/core/pci.c | 13 +++++++++---- drivers/cxl/cxl.h | 6 +++--- drivers/cxl/mem.c | 2 +- tools/testing/cxl/Kbuild | 2 +- tools/testing/cxl/test/mock.c | 6 +++--- 5 files changed, 17 insertions(+), 12 deletions(-)