Sam Bobroff <sbobr...@linux.ibm.com> writes:
> On Thu, May 03, 2018 at 11:27:12PM +1000, Michael Ellerman wrote:
>> Sam Bobroff <sbobr...@linux.ibm.com> writes:
>> > diff --git a/arch/powerpc/kernel/eeh_driver.c
>> > b/arch/powerpc/kernel/eeh_driver.c
>> > index eb4feee81ff4..1c4336dcf9f5 100644
>> > --- a/arch/powerpc/kernel/eeh_driver.c
>> > +++ b/arch/powerpc/kernel/eeh_driver.c
>> > @@ -54,6 +54,25 @@ static int eeh_result_priority(enum pci_ers_result
>> > result)
>> > }
>> > };
>> >
>> > +const char *pci_ers_result_name(enum pci_ers_result r)
>> > +{
>> > + switch (r) {
>> > + case PCI_ERS_RESULT_NONE: return "none";
>> > + case PCI_ERS_RESULT_CAN_RECOVER: return "can recover";
>> > + case PCI_ERS_RESULT_NEED_RESET: return "need reset";
>> > + case PCI_ERS_RESULT_DISCONNECT: return "disconnect";
>> > + case PCI_ERS_RESULT_RECOVERED: return "recovered";
>> > + case PCI_ERS_RESULT_NO_AER_DRIVER: return "no AER driver";
>> > + default:
>> > + WARN_ONCE(1, "Unknown result type");
>> > + return "unknown";
>> > + }
>> > +};
>> > +
>> > +#define eeh_infoline(EDEV, FMT, ...) \
>> > +pr_info("EEH: PE#%x (PCI %s): " pr_fmt(FMT) "\n", EDEV->pe_config_addr, \
>> > +((EDEV->pdev) ? dev_name(&EDEV->pdev->dev) : "NONE"), ##__VA_ARGS__)
>>
>> Ooof, I guess.
>>
>> It would be nicer as a function.
>
> OK (I'd prefer to avoid macros too), but I'm not sure what kind of
> function you mean. I initially tried to use a function, but the existing
> pr_*() type macros seemed to be macros all the way down to printk, so there
> didn't seem to be anywhere to hook into, and I ended up having to
> open-code the varargs and allocate a buffer to print into. Then it can
> overflow etc. and it ended up seeming worse. Is there a better way I'm
> missing here?
Something like (not tested):
void eeh_edev_info(const struct eeh_info *info, const char *fmt, ...)
{
struct va_format vaf;
va_list args;
va_start(args, fmt);
vaf.fmt = fmt;
vaf.va = &args;
printk(KERN_INFO "EEH: PE#%x (PCI %s): %pV\n",
info->pe_config_addr,
info->pdev ? dev_name(&info->pdev->dev) : "none",
&vaf);
va_end(args);
}
>
>> "infoline" annoys me for some reason, "eeh_info" ?
>
> OK. How about eeh_edev_info(), to indicate that it acts on an 'edev'?
Sounds good.
cheers
