| Message ID | df3a7ca31d2002ca447ab062f5b5e32ced9bec85.camel@perches.com (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | powerpc/powernv: Rename pe_level_printk to pe_printk and embed KERN_LEVEL in format | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch next (e610a466d16a086e321f0bd421e2fc75cff28605) |
| snowpatch_ozlabs/build-ppc64le | success | Build succeeded |
| snowpatch_ozlabs/build-ppc64be | success | Build succeeded |
| snowpatch_ozlabs/build-ppc64e | success | Build succeeded |
| snowpatch_ozlabs/build-pmac32 | success | Build succeeded |
| snowpatch_ozlabs/checkpatch | warning | total: 0 errors, 0 warnings, 1 checks, 53 lines checked |
Hi Joe Le 21/06/2019 à 07:36, Joe Perches a écrit : > Remove the separate KERN_<LEVEL> from each pe_level_printk and > instead add the KERN_<LEVEL> to the format. > > pfix in pe_level_printk could also be used uninitialized so > add a new else and set pfx to the hex value of pe->flags. > > Rename pe_level_printk to pe_printk and update the pe_<level> > macros. > > Signed-off-by: Joe Perches <joe@perches.com> > --- > arch/powerpc/platforms/powernv/pci-ioda.c | 14 ++++++++++++-- > arch/powerpc/platforms/powernv/pci.h | 11 +++++------ > 2 files changed, 17 insertions(+), 8 deletions(-) I can't see the added value of this patch, it adds more lines than it removes. Christophe > > diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c > index 10cc42b9e541..60fc36ae626a 100644 > --- a/arch/powerpc/platforms/powernv/pci-ioda.c > +++ b/arch/powerpc/platforms/powernv/pci-ioda.c > @@ -50,15 +50,23 @@ > static const char * const pnv_phb_names[] = { "IODA1", "IODA2", "NPU_NVLINK", > "NPU_OCAPI" }; > > -void pe_level_printk(const struct pnv_ioda_pe *pe, const char *level, > - const char *fmt, ...) > +void pe_printk(const struct pnv_ioda_pe *pe, const char *fmt, ...) > { > struct va_format vaf; > va_list args; > char pfix[32]; > + char level[PRINTK_MAX_SINGLE_HEADER_LEN + 1] = "\0"; > > va_start(args, fmt); > > + while (printk_get_level(fmt)) { > + size_t size = printk_skip_level(fmt) - fmt; > + > + memcpy(level, fmt, size); > + level[size] = '\0'; > + fmt += size; > + } > + > vaf.fmt = fmt; > vaf.va = &args; > > @@ -74,6 +82,8 @@ void pe_level_printk(const struct pnv_ioda_pe *pe, const char *level, > (pe->rid & 0xff00) >> 8, > PCI_SLOT(pe->rid), PCI_FUNC(pe->rid)); > #endif /* CONFIG_PCI_IOV*/ > + else > + sprintf(pfix, "(flags: 0x%lx)", pe->flags); > > printk("%spci %s: [PE# %.2x] %pV", > level, pfix, pe->pe_number, &vaf); > diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h > index be26ab3d99e0..870b21f55b3f 100644 > --- a/arch/powerpc/platforms/powernv/pci.h > +++ b/arch/powerpc/platforms/powernv/pci.h > @@ -205,15 +205,14 @@ extern unsigned long pnv_pci_ioda2_get_table_size(__u32 page_shift, > __u64 window_size, __u32 levels); > extern int pnv_eeh_post_init(void); > > -__printf(3, 4) > -extern void pe_level_printk(const struct pnv_ioda_pe *pe, const char *level, > - const char *fmt, ...); > +__printf(2, 3) > +extern void pe_printk(const struct pnv_ioda_pe *pe, const char *fmt, ...); > #define pe_err(pe, fmt, ...) \ > - pe_level_printk(pe, KERN_ERR, fmt, ##__VA_ARGS__) > + pe_printk(pe, KERN_ERR fmt, ##__VA_ARGS__) > #define pe_warn(pe, fmt, ...) \ > - pe_level_printk(pe, KERN_WARNING, fmt, ##__VA_ARGS__) > + pe_printk(pe, KERN_WARNING fmt, ##__VA_ARGS__) > #define pe_info(pe, fmt, ...) \ > - pe_level_printk(pe, KERN_INFO, fmt, ##__VA_ARGS__) > + pe_printk(pe, KERN_INFO fmt, ##__VA_ARGS__) > > /* Nvlink functions */ > extern void pnv_npu_try_dma_set_bypass(struct pci_dev *gpdev, bool bypass);
On Wed, 2025-08-27 at 18:42 +0200, Christophe Leroy wrote: > Hi Joe > > Le 21/06/2019 à 07:36, Joe Perches a écrit : > > Remove the separate KERN_<LEVEL> from each pe_level_printk and > > instead add the KERN_<LEVEL> to the format. Hello Christophe. It's over 6 years since I wrote that. Is that the typical review time? ;) I would expect it doesn't apply anyway though it should make the object size a tiny bit smaller. > > > > pfix in pe_level_printk could also be used uninitialized so > > add a new else and set pfx to the hex value of pe->flags. > > > > Rename pe_level_printk to pe_printk and update the pe_<level> > > macros. > > > > Signed-off-by: Joe Perches <joe@perches.com> > > --- > > arch/powerpc/platforms/powernv/pci-ioda.c | 14 ++++++++++++-- > > arch/powerpc/platforms/powernv/pci.h | 11 +++++------ > > 2 files changed, 17 insertions(+), 8 deletions(-) > > I can't see the added value of this patch, it adds more lines than it > removes. > > Christophe > > > > > diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c > > index 10cc42b9e541..60fc36ae626a 100644 > > --- a/arch/powerpc/platforms/powernv/pci-ioda.c > > +++ b/arch/powerpc/platforms/powernv/pci-ioda.c > > @@ -50,15 +50,23 @@ > > static const char * const pnv_phb_names[] = { "IODA1", "IODA2", "NPU_NVLINK", > > "NPU_OCAPI" }; > > > > -void pe_level_printk(const struct pnv_ioda_pe *pe, const char *level, > > - const char *fmt, ...) > > +void pe_printk(const struct pnv_ioda_pe *pe, const char *fmt, ...) > > { > > struct va_format vaf; > > va_list args; > > char pfix[32]; > > + char level[PRINTK_MAX_SINGLE_HEADER_LEN + 1] = "\0"; > > > > va_start(args, fmt); > > > > + while (printk_get_level(fmt)) { > > + size_t size = printk_skip_level(fmt) - fmt; > > + > > + memcpy(level, fmt, size); > > + level[size] = '\0'; > > + fmt += size; > > + } > > + > > vaf.fmt = fmt; > > vaf.va = &args; > > > > @@ -74,6 +82,8 @@ void pe_level_printk(const struct pnv_ioda_pe *pe, const char *level, > > (pe->rid & 0xff00) >> 8, > > PCI_SLOT(pe->rid), PCI_FUNC(pe->rid)); > > #endif /* CONFIG_PCI_IOV*/ > > + else > > + sprintf(pfix, "(flags: 0x%lx)", pe->flags); > > > > printk("%spci %s: [PE# %.2x] %pV", > > level, pfix, pe->pe_number, &vaf); > > diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h > > index be26ab3d99e0..870b21f55b3f 100644 > > --- a/arch/powerpc/platforms/powernv/pci.h > > +++ b/arch/powerpc/platforms/powernv/pci.h > > @@ -205,15 +205,14 @@ extern unsigned long pnv_pci_ioda2_get_table_size(__u32 page_shift, > > __u64 window_size, __u32 levels); > > extern int pnv_eeh_post_init(void); > > > > -__printf(3, 4) > > -extern void pe_level_printk(const struct pnv_ioda_pe *pe, const char *level, > > - const char *fmt, ...); > > +__printf(2, 3) > > +extern void pe_printk(const struct pnv_ioda_pe *pe, const char *fmt, ...); > > #define pe_err(pe, fmt, ...) \ > > - pe_level_printk(pe, KERN_ERR, fmt, ##__VA_ARGS__) > > + pe_printk(pe, KERN_ERR fmt, ##__VA_ARGS__) > > #define pe_warn(pe, fmt, ...) \ > > - pe_level_printk(pe, KERN_WARNING, fmt, ##__VA_ARGS__) > > + pe_printk(pe, KERN_WARNING fmt, ##__VA_ARGS__) > > #define pe_info(pe, fmt, ...) \ > > - pe_level_printk(pe, KERN_INFO, fmt, ##__VA_ARGS__) > > + pe_printk(pe, KERN_INFO fmt, ##__VA_ARGS__) > > > > /* Nvlink functions */ > > extern void pnv_npu_try_dma_set_bypass(struct pci_dev *gpdev, bool bypass);
Le 28/08/2025 à 02:04, Joe Perches a écrit : > [Vous ne recevez pas souvent de courriers de joe@perches.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ] > > On Wed, 2025-08-27 at 18:42 +0200, Christophe Leroy wrote: >> Hi Joe >> >> Le 21/06/2019 à 07:36, Joe Perches a écrit : >>> Remove the separate KERN_<LEVEL> from each pe_level_printk and >>> instead add the KERN_<LEVEL> to the format. > > Hello Christophe. > > It's over 6 years since I wrote that. > Is that the typical review time? ;) :) I'm trying to clean patchwork and drop stale patches that are still there. At the moment we have 240 'new' patches. 88 of them are older than one year, with a few older ones being almost 10 years old. > > I would expect it doesn't apply anyway > though it should make the object size > a tiny bit smaller. It still applies when doing a 3-way merge indeed. Christophe
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c index 10cc42b9e541..60fc36ae626a 100644 --- a/arch/powerpc/platforms/powernv/pci-ioda.c +++ b/arch/powerpc/platforms/powernv/pci-ioda.c @@ -50,15 +50,23 @@ static const char * const pnv_phb_names[] = { "IODA1", "IODA2", "NPU_NVLINK", "NPU_OCAPI" }; -void pe_level_printk(const struct pnv_ioda_pe *pe, const char *level, - const char *fmt, ...) +void pe_printk(const struct pnv_ioda_pe *pe, const char *fmt, ...) { struct va_format vaf; va_list args; char pfix[32]; + char level[PRINTK_MAX_SINGLE_HEADER_LEN + 1] = "\0"; va_start(args, fmt); + while (printk_get_level(fmt)) { + size_t size = printk_skip_level(fmt) - fmt; + + memcpy(level, fmt, size); + level[size] = '\0'; + fmt += size; + } + vaf.fmt = fmt; vaf.va = &args; @@ -74,6 +82,8 @@ void pe_level_printk(const struct pnv_ioda_pe *pe, const char *level, (pe->rid & 0xff00) >> 8, PCI_SLOT(pe->rid), PCI_FUNC(pe->rid)); #endif /* CONFIG_PCI_IOV*/ + else + sprintf(pfix, "(flags: 0x%lx)", pe->flags); printk("%spci %s: [PE# %.2x] %pV", level, pfix, pe->pe_number, &vaf); diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h index be26ab3d99e0..870b21f55b3f 100644 --- a/arch/powerpc/platforms/powernv/pci.h +++ b/arch/powerpc/platforms/powernv/pci.h @@ -205,15 +205,14 @@ extern unsigned long pnv_pci_ioda2_get_table_size(__u32 page_shift, __u64 window_size, __u32 levels); extern int pnv_eeh_post_init(void); -__printf(3, 4) -extern void pe_level_printk(const struct pnv_ioda_pe *pe, const char *level, - const char *fmt, ...); +__printf(2, 3) +extern void pe_printk(const struct pnv_ioda_pe *pe, const char *fmt, ...); #define pe_err(pe, fmt, ...) \ - pe_level_printk(pe, KERN_ERR, fmt, ##__VA_ARGS__) + pe_printk(pe, KERN_ERR fmt, ##__VA_ARGS__) #define pe_warn(pe, fmt, ...) \ - pe_level_printk(pe, KERN_WARNING, fmt, ##__VA_ARGS__) + pe_printk(pe, KERN_WARNING fmt, ##__VA_ARGS__) #define pe_info(pe, fmt, ...) \ - pe_level_printk(pe, KERN_INFO, fmt, ##__VA_ARGS__) + pe_printk(pe, KERN_INFO fmt, ##__VA_ARGS__) /* Nvlink functions */ extern void pnv_npu_try_dma_set_bypass(struct pci_dev *gpdev, bool bypass);
Remove the separate KERN_<LEVEL> from each pe_level_printk and instead add the KERN_<LEVEL> to the format. pfix in pe_level_printk could also be used uninitialized so add a new else and set pfx to the hex value of pe->flags. Rename pe_level_printk to pe_printk and update the pe_<level> macros. Signed-off-by: Joe Perches <joe@perches.com> --- arch/powerpc/platforms/powernv/pci-ioda.c | 14 ++++++++++++-- arch/powerpc/platforms/powernv/pci.h | 11 +++++------ 2 files changed, 17 insertions(+), 8 deletions(-)