On Fri, 13 Sep 2024 17:36:31 +0300
Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx> wrote:
> Add pcie_print_tlp_log() helper to print TLP Header and Prefix Log.
> Print End-End Prefixes only if they are non-zero.
>
> Consolidate the few places which currently print TLP using custom
> formatting.
>
> The first attempt used pr_cont() instead of building a string first but
> it turns out pr_cont() is not compatible with pci_err() and prints on a
> separate line. When I asked about this, Andy Shevchenko suggested
> pr_cont() should not be used in the first place (to eventually get rid
> of it) so pr_cont() is now replaced with building the string first.
>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
A couple of trivial things inline but looks good to me either way.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> diff --git a/drivers/pci/pcie/tlp.c b/drivers/pci/pcie/tlp.c
> index def9dd7b73e8..097ac8514e96 100644
> --- a/drivers/pci/pcie/tlp.c
> +++ b/drivers/pci/pcie/tlp.c
> @@ -6,6 +6,7 @@
> */
>
> #include <linux/aer.h>
> +#include <linux/array_size.h>
> #include <linux/pci.h>
> #include <linux/string.h>
>
> @@ -76,3 +77,33 @@ int pcie_read_tlp_log(struct pci_dev *dev, int where, int where2,
>
> return 0;
> }
> +
> +/**
> + * pcie_print_tlp_log - Print TLP Header / Prefix Log contents
> + * @dev: PCIe device
> + * @log: TLP Log structure
> + * @pfx: String prefix (for print out indentation)
Code doesn't care if it is indentation or ponies. So does it make
sense to say anything beyond String prefix?
> + *
> + * Prints TLP Header and Prefix Log information held by @log.
> + */
> +void pcie_print_tlp_log(const struct pci_dev *dev,
> + const struct pcie_tlp_log *log, const char *pfx)
> +{
> + char buf[(10 + 1) * (4 + ARRAY_SIZE(log->prefix)) + 14 + 1];
Can we associate the 14 with the prefixes string by having that as a
const char * and using strlen() It was a tiny bit irritating to count
the characters whilst reviewing this ;)
> + unsigned int i;
> + int len;
> +
> + len = scnprintf(buf, sizeof(buf), "%#010x %#010x %#010x %#010x",
> + log->dw[0], log->dw[1], log->dw[2], log->dw[3]);
> +
> + if (log->prefix[0])
> + len += scnprintf(buf + len, sizeof(buf) - len, " E-E Prefixes:");
> + for (i = 0; i < ARRAY_SIZE(log->prefix); i++) {
> + if (!log->prefix[i])
> + break;
> + len += scnprintf(buf + len, sizeof(buf) - len,
> + " %#010x", log->prefix[i]);
> + }
> +
> + pci_err(dev, "%sTLP Header: %s\n", pfx, buf);
> +}
