On Tue, Jan 26, 2016 at 01:27:18PM +0100, Jean Delvare wrote:
> But I'd say -EPERM is hardly better. The problem with -ENODEV is that it
> is already returned by this function for several other error causes.
> Also the aer-inject user-space tool will print the error message from
> the error code, and I don't think "No such device" is helpful in that
> case. What about -ENOTSUPP ("Operation not supported") or
> -EEPROTONOSUPPORT ("Protocol not supported")?
Makes sense.
> I can change it if nobody objects. I think the change can be included in
> this patch as it is quite related.
I'd do a separate patch but this is only my opinion. I guess that's
Bjorn's call.
> I'd rather ask, why printk? ;-) Using raw printk is considered bad and
> should be avoided whenever possible.
Hmm, interesting. Why?
> So says checkpatch.pl.
Please don't tell me you believe what checkpatch says.
> > Why not pr_err() and define pr_fmt to "aer_inject: " and then drop
> > that prefix from the messages?
>
> Because I believe that including the device name in the error messages
> makes them more helpful to understand and diagnose the problem. If the
> device where we try to inject the error has a problem, it's PCI name
> will be included in the error message. If the error is with the root
> port, then we include the root port's PCI name. If I used pr_err()
> instead then the device information would be missing.
True, that's a good argument.
However, if you're doing aer injection, you already *know* the device
you're injecting too. Unless you want to inject in multiple devices and
then it is helpful.
So sure, dev_* sounds better as it gives more info about which device
fails, but then please convert the whole driver.
Thanks.
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
