Message ID | 20211004085842.255813-1-songkai01@inspur.com (mailing list archive) |
---|---|
State | Changes Requested |
Headers | show |
Series | powerpc/eeh:Fix docstrings in eeh | expand |
Context | Check | Description |
---|---|---|
snowpatch_ozlabs/github-powerpc_ppctests | success | Successfully ran 8 jobs. |
snowpatch_ozlabs/github-powerpc_selftests | success | Successfully ran 8 jobs. |
snowpatch_ozlabs/github-powerpc_sparse | success | Successfully ran 4 jobs. |
snowpatch_ozlabs/github-powerpc_clang | success | Successfully ran 7 jobs. |
snowpatch_ozlabs/github-powerpc_kernel_qemu | success | Successfully ran 24 jobs. |
Hi Kai, Thank you for your patch! I have 3 very minor tweaks and I am otherwise very happy with it. Firstly, in your commit name, there should be a space between "powerpc/eeh:" and "Fix docstrings". You might also want to say "in eeh.c" rather than "in eeh" because there is eeh code in a number of other files too. > We fix the following warnings when building kernel with W=1: > arch/powerpc/kernel/eeh.c:598: warning: Function parameter or member 'function' not described in 'eeh_pci_enable' > arch/powerpc/kernel/eeh.c:774: warning: Function parameter or member 'edev' not described in 'eeh_set_dev_freset' > arch/powerpc/kernel/eeh.c:774: warning: expecting prototype for eeh_set_pe_freset(). Prototype was for eeh_set_dev_freset() instead > arch/powerpc/kernel/eeh.c:814: warning: Function parameter or member 'include_passed' not described in 'eeh_pe_reset_full' > arch/powerpc/kernel/eeh.c:944: warning: Function parameter or member 'ops' not described in 'eeh_init' > arch/powerpc/kernel/eeh.c:1451: warning: Function parameter or member 'include_passed' not described in 'eeh_pe_reset' > arch/powerpc/kernel/eeh.c:1526: warning: Function parameter or member 'func' not described in 'eeh_pe_inject_err' > arch/powerpc/kernel/eeh.c:1526: warning: Excess function parameter 'function' described in 'eeh_pe_inject_err' > > Signed-off-by: Kai Song <songkai01@inspur.com> > --- > arch/powerpc/kernel/eeh.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c > index e9b597ed423c..57a6868a41ab 100644 > --- a/arch/powerpc/kernel/eeh.c > +++ b/arch/powerpc/kernel/eeh.c > @@ -589,6 +589,7 @@ EXPORT_SYMBOL(eeh_check_failure); > /** > * eeh_pci_enable - Enable MMIO or DMA transfers for this slot > * @pe: EEH PE > + * @function : EEH function I don't think there should be a space between '@function' and ':'. I know the parameter is called 'function' and I think "EEH function" was a good guess for the docstring. However, if we look at the way 'function' is used, it is compared with EEH_OPT_xxx constants, and then it's passed to eeh_ops->set_option(), eeh_pci_enable() is also called in eeh_pe_set_option() with a parameter called 'option'. So I think maybe 'function' should be described as the "EEH option"? This is still very unsatisfactory but it's not the fault of your patch - the EEH codebase is very messy and it's worth fixing the W=1 warnings even if we don't fully clean up the EEH codebase. > - * eeh_set_pe_freset - Check the required reset for the indicated device > - * @data: EEH device > + * eeh_set_dev_freset - Check the required reset for the indicated device > + * @edev: EEH device > * @flag: return value > * This is good. > /** > * eeh_pe_reset_full - Complete a full reset process on the indicated PE > * @pe: EEH PE > + * @include_passed: include passed-through devices? > * > * This function executes a full reset procedure on a PE, including setting > * the appropriate flags, performing a fundamental or hot reset, and then > @@ -937,6 +939,7 @@ static struct notifier_block eeh_device_nb = { This is OK. > /** > * eeh_init - System wide EEH initialization > + * @ops: struct to trace EEH operation callback functions I think "@ops: platform-specific functions for EEH operations" is probably clearer? > * > * It's the platform's job to call this from an arch_initcall(). > */ > @@ -1442,6 +1445,7 @@ static int eeh_pe_reenable_devices(struct eeh_pe *pe, bool include_passed) > * eeh_pe_reset - Issue PE reset according to specified type > * @pe: EEH PE > * @option: reset type > + * @include_passed: include passed-through devices? > * This is OK. > * The routine is called to reset the specified PE with the > * indicated type, either fundamental reset or hot reset. > @@ -1513,12 +1517,12 @@ EXPORT_SYMBOL_GPL(eeh_pe_configure); > * eeh_pe_inject_err - Injecting the specified PCI error to the indicated PE > * @pe: the indicated PE > * @type: error type > - * @function: error function > + * @func: error function This is good. > * @addr: address > * @mask: address mask > * > * The routine is called to inject the specified PCI error, which > - * is determined by @type and @function, to the indicated PE for > + * is determined by @type and @func, to the indicated PE for This is good. When you resend, you can include: Reviewed-by: Daniel Axtens <dja@axtens.net> Kind regards, Daniel
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c index e9b597ed423c..57a6868a41ab 100644 --- a/arch/powerpc/kernel/eeh.c +++ b/arch/powerpc/kernel/eeh.c @@ -589,6 +589,7 @@ EXPORT_SYMBOL(eeh_check_failure); /** * eeh_pci_enable - Enable MMIO or DMA transfers for this slot * @pe: EEH PE + * @function : EEH function * * This routine should be called to reenable frozen MMIO or DMA * so that it would work correctly again. It's useful while doing @@ -761,8 +762,8 @@ int pcibios_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state stat } /** - * eeh_set_pe_freset - Check the required reset for the indicated device - * @data: EEH device + * eeh_set_dev_freset - Check the required reset for the indicated device + * @edev: EEH device * @flag: return value * * Each device might have its preferred reset type: fundamental or @@ -801,6 +802,7 @@ static void eeh_pe_refreeze_passed(struct eeh_pe *root) /** * eeh_pe_reset_full - Complete a full reset process on the indicated PE * @pe: EEH PE + * @include_passed: include passed-through devices? * * This function executes a full reset procedure on a PE, including setting * the appropriate flags, performing a fundamental or hot reset, and then @@ -937,6 +939,7 @@ static struct notifier_block eeh_device_nb = { /** * eeh_init - System wide EEH initialization + * @ops: struct to trace EEH operation callback functions * * It's the platform's job to call this from an arch_initcall(). */ @@ -1442,6 +1445,7 @@ static int eeh_pe_reenable_devices(struct eeh_pe *pe, bool include_passed) * eeh_pe_reset - Issue PE reset according to specified type * @pe: EEH PE * @option: reset type + * @include_passed: include passed-through devices? * * The routine is called to reset the specified PE with the * indicated type, either fundamental reset or hot reset. @@ -1513,12 +1517,12 @@ EXPORT_SYMBOL_GPL(eeh_pe_configure); * eeh_pe_inject_err - Injecting the specified PCI error to the indicated PE * @pe: the indicated PE * @type: error type - * @function: error function + * @func: error function * @addr: address * @mask: address mask * * The routine is called to inject the specified PCI error, which - * is determined by @type and @function, to the indicated PE for + * is determined by @type and @func, to the indicated PE for * testing purpose. */ int eeh_pe_inject_err(struct eeh_pe *pe, int type, int func,
We fix the following warnings when building kernel with W=1: arch/powerpc/kernel/eeh.c:598: warning: Function parameter or member 'function' not described in 'eeh_pci_enable' arch/powerpc/kernel/eeh.c:774: warning: Function parameter or member 'edev' not described in 'eeh_set_dev_freset' arch/powerpc/kernel/eeh.c:774: warning: expecting prototype for eeh_set_pe_freset(). Prototype was for eeh_set_dev_freset() instead arch/powerpc/kernel/eeh.c:814: warning: Function parameter or member 'include_passed' not described in 'eeh_pe_reset_full' arch/powerpc/kernel/eeh.c:944: warning: Function parameter or member 'ops' not described in 'eeh_init' arch/powerpc/kernel/eeh.c:1451: warning: Function parameter or member 'include_passed' not described in 'eeh_pe_reset' arch/powerpc/kernel/eeh.c:1526: warning: Function parameter or member 'func' not described in 'eeh_pe_inject_err' arch/powerpc/kernel/eeh.c:1526: warning: Excess function parameter 'function' described in 'eeh_pe_inject_err' Signed-off-by: Kai Song <songkai01@inspur.com> --- arch/powerpc/kernel/eeh.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)