Re: [PATCH v2 2/2] arch/powerpc/kvm: kvmppc_hv_entry: remove r4 argument
On 2023-03-15 10:48:01, Kautuk Consul wrote: > On 2023-03-15 15:48:53, Michael Ellerman wrote: > > Kautuk Consul writes: > > > kvmppc_hv_entry is called from only 2 locations within > > > book3s_hv_rmhandlers.S. Both of those locations set r4 > > > as HSTATE_KVM_VCPU(r13) before calling kvmppc_hv_entry. > > > So, shift the r4 load instruction to kvmppc_hv_entry and > > > thus modify the calling convention of this function. > > > > > > Signed-off-by: Kautuk Consul > > > --- > > > arch/powerpc/kvm/book3s_hv_rmhandlers.S | 9 - > > > 1 file changed, 4 insertions(+), 5 deletions(-) > > > > > > diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S > > > b/arch/powerpc/kvm/book3s_hv_rmhandlers.S > > > index b81ba4ee0521..da9a15db12fe 100644 > > > --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S > > > +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S > > > @@ -85,7 +85,7 @@ _GLOBAL_TOC(kvmppc_hv_entry_trampoline) > > > RFI_TO_KERNEL > > > > > > kvmppc_call_hv_entry: > > > - ld r4, HSTATE_KVM_VCPU(r13) > > > + /* Enter guest. */ > > > bl kvmppc_hv_entry > > > > > > /* Back from guest - restore host state and return to caller */ > > > @@ -352,9 +352,7 @@ kvm_secondary_got_guest: > > > mtspr SPRN_LDBAR, r0 > > > isync > > > 63: > > > - /* Order load of vcpu after load of vcore */ > > > - lwsync > > > > Where did this barrier go? > > > > I don't see that it's covered by any existing barriers in > > kvmppc_hv_entry, and you don't add it back anywhere. > > My concept about this is that since now the call to kvmppc_hv_entry > is first taken before the load to r4 shouldn't the pending load in the > pipeline of the HSTATE_KVM_VCORE as per the earlier comment be ordered anyway > before-hand ? Or do you mesn to say that pending loads may not be > cleared/flushed across the "bl " boundary ? Sorry, I mean: " shouldn't the pending load in the pipeline (of the HSTATE_KVM_VCORE) as per the earlier comment be ordered anyway before-hand?" Forgot the paranthesis. > > > > > - ld r4, HSTATE_KVM_VCPU(r13) > > > + /* Enter guest. */ > > > bl kvmppc_hv_entry > > > > > > /* Back from the guest, go back to nap */ > > > @@ -506,7 +504,6 @@ SYM_INNER_LABEL(kvmppc_hv_entry, SYM_L_LOCAL) > > > > > > /* Required state: > > >* > > > - * R4 = vcpu pointer (or NULL) > > >* MSR = ~IR|DR > > >* R13 = PACA > > >* R1 = host R1 > > > @@ -524,6 +521,8 @@ SYM_INNER_LABEL(kvmppc_hv_entry, SYM_L_LOCAL) > > > li r6, KVM_GUEST_MODE_HOST_HV > > > stb r6, HSTATE_IN_GUEST(r13) > > > > > > + ld r4, HSTATE_KVM_VCPU(r13) > > > + > > > #ifdef CONFIG_KVM_BOOK3S_HV_P8_TIMING > > > /* Store initial timestamp */ > > > cmpdi r4, 0 > > > > cheers
Re: [PATCH v2 2/2] arch/powerpc/kvm: kvmppc_hv_entry: remove r4 argument
On 2023-03-15 15:48:53, Michael Ellerman wrote: > Kautuk Consul writes: > > kvmppc_hv_entry is called from only 2 locations within > > book3s_hv_rmhandlers.S. Both of those locations set r4 > > as HSTATE_KVM_VCPU(r13) before calling kvmppc_hv_entry. > > So, shift the r4 load instruction to kvmppc_hv_entry and > > thus modify the calling convention of this function. > > > > Signed-off-by: Kautuk Consul > > --- > > arch/powerpc/kvm/book3s_hv_rmhandlers.S | 9 - > > 1 file changed, 4 insertions(+), 5 deletions(-) > > > > diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S > > b/arch/powerpc/kvm/book3s_hv_rmhandlers.S > > index b81ba4ee0521..da9a15db12fe 100644 > > --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S > > +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S > > @@ -85,7 +85,7 @@ _GLOBAL_TOC(kvmppc_hv_entry_trampoline) > > RFI_TO_KERNEL > > > > kvmppc_call_hv_entry: > > - ld r4, HSTATE_KVM_VCPU(r13) > > + /* Enter guest. */ > > bl kvmppc_hv_entry > > > > /* Back from guest - restore host state and return to caller */ > > @@ -352,9 +352,7 @@ kvm_secondary_got_guest: > > mtspr SPRN_LDBAR, r0 > > isync > > 63: > > - /* Order load of vcpu after load of vcore */ > > - lwsync > > Where did this barrier go? > > I don't see that it's covered by any existing barriers in > kvmppc_hv_entry, and you don't add it back anywhere. My concept about this is that since now the call to kvmppc_hv_entry is first taken before the load to r4 shouldn't the pending load in the pipeline of the HSTATE_KVM_VCORE as per the earlier comment be ordered anyway before-hand ? Or do you mesn to say that pending loads may not be cleared/flushed across the "bl " boundary ? > > > - ld r4, HSTATE_KVM_VCPU(r13) > > + /* Enter guest. */ > > bl kvmppc_hv_entry > > > > /* Back from the guest, go back to nap */ > > @@ -506,7 +504,6 @@ SYM_INNER_LABEL(kvmppc_hv_entry, SYM_L_LOCAL) > > > > /* Required state: > > * > > -* R4 = vcpu pointer (or NULL) > > * MSR = ~IR|DR > > * R13 = PACA > > * R1 = host R1 > > @@ -524,6 +521,8 @@ SYM_INNER_LABEL(kvmppc_hv_entry, SYM_L_LOCAL) > > li r6, KVM_GUEST_MODE_HOST_HV > > stb r6, HSTATE_IN_GUEST(r13) > > > > + ld r4, HSTATE_KVM_VCPU(r13) > > + > > #ifdef CONFIG_KVM_BOOK3S_HV_P8_TIMING > > /* Store initial timestamp */ > > cmpdi r4, 0 > > cheers
[PATCH v4 20/36] powerpc: Implement the new page table range API
Add set_ptes(), update_mmu_cache_range() and flush_dcache_folio().
Change the PG_arch_1 (aka PG_dcache_dirty) flag from being per-page to
per-folio.
Signed-off-by: Matthew Wilcox (Oracle)
Cc: Michael Ellerman
Cc: Nicholas Piggin
Cc: Christophe Leroy
Cc: linuxppc-dev@lists.ozlabs.org
---
arch/powerpc/include/asm/book3s/pgtable.h | 10 +
arch/powerpc/include/asm/cacheflush.h | 14 +--
arch/powerpc/include/asm/kvm_ppc.h| 10 ++---
arch/powerpc/include/asm/nohash/pgtable.h | 13 ++
arch/powerpc/include/asm/pgtable.h| 6 +++
arch/powerpc/mm/book3s64/hash_utils.c | 11 ++---
arch/powerpc/mm/cacheflush.c | 40 ++
arch/powerpc/mm/nohash/e500_hugetlbpage.c | 3 +-
arch/powerpc/mm/pgtable.c | 51 +--
9 files changed, 77 insertions(+), 81 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/pgtable.h
b/arch/powerpc/include/asm/book3s/pgtable.h
index d18b748ea3ae..c2ef811505b0 100644
--- a/arch/powerpc/include/asm/book3s/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/pgtable.h
@@ -9,13 +9,6 @@
#endif
#ifndef __ASSEMBLY__
-/* Insert a PTE, top-level function is out of line. It uses an inline
- * low level function in the respective pgtable-* files
- */
-extern void set_pte_at(struct mm_struct *mm, unsigned long addr, pte_t *ptep,
- pte_t pte);
-
-
#define __HAVE_ARCH_PTEP_SET_ACCESS_FLAGS
extern int ptep_set_access_flags(struct vm_area_struct *vma, unsigned long
address,
pte_t *ptep, pte_t entry, int dirty);
@@ -36,7 +29,8 @@ void __update_mmu_cache(struct vm_area_struct *vma, unsigned
long address, pte_t
* corresponding HPTE into the hash table ahead of time, instead of
* waiting for the inevitable extra hash-table miss exception.
*/
-static inline void update_mmu_cache(struct vm_area_struct *vma, unsigned long
address, pte_t *ptep)
+static inline void update_mmu_cache_range(struct vm_area_struct *vma,
+ unsigned long address, pte_t *ptep, unsigned int nr)
{
if (IS_ENABLED(CONFIG_PPC32) && !mmu_has_feature(MMU_FTR_HPTE_TABLE))
return;
diff --git a/arch/powerpc/include/asm/cacheflush.h
b/arch/powerpc/include/asm/cacheflush.h
index 7564dd4fd12b..ef7d2de33b89 100644
--- a/arch/powerpc/include/asm/cacheflush.h
+++ b/arch/powerpc/include/asm/cacheflush.h
@@ -35,13 +35,19 @@ static inline void flush_cache_vmap(unsigned long start,
unsigned long end)
* It just marks the page as not i-cache clean. We do the i-cache
* flush later when the page is given to a user process, if necessary.
*/
-static inline void flush_dcache_page(struct page *page)
+static inline void flush_dcache_folio(struct folio *folio)
{
if (cpu_has_feature(CPU_FTR_COHERENT_ICACHE))
return;
/* avoid an atomic op if possible */
- if (test_bit(PG_dcache_clean, >flags))
- clear_bit(PG_dcache_clean, >flags);
+ if (test_bit(PG_dcache_clean, >flags))
+ clear_bit(PG_dcache_clean, >flags);
+}
+#define flush_dcache_folio flush_dcache_folio
+
+static inline void flush_dcache_page(struct page *page)
+{
+ flush_dcache_folio(page_folio(page));
}
void flush_icache_range(unsigned long start, unsigned long stop);
@@ -51,7 +57,7 @@ void flush_icache_user_page(struct vm_area_struct *vma,
struct page *page,
unsigned long addr, int len);
#define flush_icache_user_page flush_icache_user_page
-void flush_dcache_icache_page(struct page *page);
+void flush_dcache_icache_folio(struct folio *folio);
/**
* flush_dcache_range(): Write any modified data cache blocks out to memory and
diff --git a/arch/powerpc/include/asm/kvm_ppc.h
b/arch/powerpc/include/asm/kvm_ppc.h
index 6bef23d6d0e3..e91dd8e88bb7 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -868,7 +868,7 @@ void kvmppc_init_lpid(unsigned long nr_lpids);
static inline void kvmppc_mmu_flush_icache(kvm_pfn_t pfn)
{
- struct page *page;
+ struct folio *folio;
/*
* We can only access pages that the kernel maps
* as memory. Bail out for unmapped ones.
@@ -877,10 +877,10 @@ static inline void kvmppc_mmu_flush_icache(kvm_pfn_t pfn)
return;
/* Clear i-cache for new pages */
- page = pfn_to_page(pfn);
- if (!test_bit(PG_dcache_clean, >flags)) {
- flush_dcache_icache_page(page);
- set_bit(PG_dcache_clean, >flags);
+ folio = page_folio(pfn_to_page(pfn));
+ if (!test_bit(PG_dcache_clean, >flags)) {
+ flush_dcache_icache_folio(folio);
+ set_bit(PG_dcache_clean, >flags);
}
}
diff --git a/arch/powerpc/include/asm/nohash/pgtable.h
b/arch/powerpc/include/asm/nohash/pgtable.h
index a6cb6f92..69a7dd47a9f0 100644
--- a/arch/powerpc/include/asm/nohash/pgtable.h
+++ b/arch/powerpc/include/asm/nohash/pgtable.h
@@
Re: [PATCH v2 2/2] arch/powerpc/kvm: kvmppc_hv_entry: remove r4 argument
Kautuk Consul writes: > kvmppc_hv_entry is called from only 2 locations within > book3s_hv_rmhandlers.S. Both of those locations set r4 > as HSTATE_KVM_VCPU(r13) before calling kvmppc_hv_entry. > So, shift the r4 load instruction to kvmppc_hv_entry and > thus modify the calling convention of this function. > > Signed-off-by: Kautuk Consul > --- > arch/powerpc/kvm/book3s_hv_rmhandlers.S | 9 - > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S > b/arch/powerpc/kvm/book3s_hv_rmhandlers.S > index b81ba4ee0521..da9a15db12fe 100644 > --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S > +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S > @@ -85,7 +85,7 @@ _GLOBAL_TOC(kvmppc_hv_entry_trampoline) > RFI_TO_KERNEL > > kvmppc_call_hv_entry: > - ld r4, HSTATE_KVM_VCPU(r13) > + /* Enter guest. */ > bl kvmppc_hv_entry > > /* Back from guest - restore host state and return to caller */ > @@ -352,9 +352,7 @@ kvm_secondary_got_guest: > mtspr SPRN_LDBAR, r0 > isync > 63: > - /* Order load of vcpu after load of vcore */ > - lwsync Where did this barrier go? I don't see that it's covered by any existing barriers in kvmppc_hv_entry, and you don't add it back anywhere. > - ld r4, HSTATE_KVM_VCPU(r13) > + /* Enter guest. */ > bl kvmppc_hv_entry > > /* Back from the guest, go back to nap */ > @@ -506,7 +504,6 @@ SYM_INNER_LABEL(kvmppc_hv_entry, SYM_L_LOCAL) > > /* Required state: >* > - * R4 = vcpu pointer (or NULL) >* MSR = ~IR|DR >* R13 = PACA >* R1 = host R1 > @@ -524,6 +521,8 @@ SYM_INNER_LABEL(kvmppc_hv_entry, SYM_L_LOCAL) > li r6, KVM_GUEST_MODE_HOST_HV > stb r6, HSTATE_IN_GUEST(r13) > > + ld r4, HSTATE_KVM_VCPU(r13) > + > #ifdef CONFIG_KVM_BOOK3S_HV_P8_TIMING > /* Store initial timestamp */ > cmpdi r4, 0 cheers
Re: [PATCH] crypto: p10-aes-gcm - remove duplicate include header
On Tue, Mar 14, 2023 at 09:44:52PM +1100, Michael Ellerman wrote: > > Hmm. Seems none of them were ever Cc'ed to linuxppc-dev. So this is the > first I've seen of them. Sorry, I didn't know that you weren't aware of this change. I will be more careful with these ppc patches in future. Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: boot regression on ppc64 with linux 6.2
Andrea Righi writes:
> I'm triggering the following bug when booting my qemu powerpc VM:
I'm not seeing that here :/
Can you give a bit more detail?
- qemu version
- qemu command line
- what userspace are you using?
- full dmesg of the failing case
> event-sources: Unable to request interrupt 23 for
> /event-sources/hot-plug-events
> WARNING: CPU: 0 PID: 1 at arch/powerpc/platforms/pseries/event_sources.c:26
> request_event_sources_irqs+0xbc/0xf0
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper/0 Tainted: GW 6.2.2-kc #1
> Hardware name: IBM pSeries (emulated by qemu) POWER9 (raw) 0x4e1200 0xf05
> of:SLOF,HEAD pSeries
> NIP: c2022eec LR: c2022ee8 CTR:
> REGS: c3483910 TRAP: 0700 Tainted: GW (6.2.2-kc)
> MSR: 82029033 CR: 24483200 XER:
> CFAR: c0180838 IRQMASK: 0
> GPR00: c2022ee8 c3483bb0 c1a5ce00 0050
> GPR04: c2437d78 c2437e28 0001 0001
> GPR08: c2437d00 0001 44483200
> GPR12: c272 c0012758
> GPR16:
> GPR20:
> GPR24: c20033fc cccd c00e07f0
> GPR28: c0db0520 c000fff92ac0 0017
> NIP [c2022eec] request_event_sources_irqs+0xbc/0xf0
> LR [c2022ee8] request_event_sources_irqs+0xb8/0xf0
> Call Trace:
> [c3483bb0] [c2022ee8] request_event_sources_irqs+0xb8/0xf0
> (unreliable)
> [c3483c40] [c2022fa0]
> __machine_initcall_pseries_init_ras_hotplug_IRQ+0x80/0xb0
> [c3483c70] [c00121b8] do_one_initcall+0x98/0x300
> [c3483d50] [c2004b28] kernel_init_freeable+0x2ec/0x370
> [c3483df0] [c0012780] kernel_init+0x30/0x190
> [c3483e50] [c000cf5c] ret_from_kernel_thread+0x5c/0x64
> --- interrupt: 0 at 0x0
>
> I did a bisect it and it seems that the offending commit is:
> baa49d81a94b ("powerpc/pseries: hvcall stack frame overhead")
>
> Reverting that and also dfecd06bc552 ("powerpc: remove
> STACK_FRAME_OVERHEAD"), because we need to re-introduce
> STACK_FRAME_OVERHEAD, seems to fix everything.
That function doesn't make a hcall, so presumably there was some earlier
problem which we only detect here.
cheers
Re: [PATCH v2 4/5] riscv: Select ARCH_DMA_DEFAULT_COHERENT
On Thu, 23 Feb 2023 14:20:27 PST (-0800), Conor Dooley wrote: On Thu, Feb 23, 2023 at 11:36:43AM +, Jiaxun Yang wrote: For riscv our assumption is unless a device states it is non-coherent, we take it to be DMA coherent. Select ARCH_DMA_DEFAULT_COHERENT to ensure dma_default_coherent is always initialized to true. Signed-off-by: Jiaxun Yang --- arch/riscv/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 1d46a268ce16..b71ce992c0c0 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -233,6 +233,7 @@ config LOCKDEP_SUPPORT config RISCV_DMA_NONCOHERENT bool + select ARCH_DMA_DEFAULT_COHERENT Since we are always coherent by default, I feel like you should put this in the main "config RISCV" section, where OF_DMA_DEFAULT_COHERENT currently is, no? Seems reasonable to me. With that Reviewed-by: Palmer Dabbelt Acked-by: Palmer Dabbelt as I'm assuming these should all stay together. Thanks! Wouldn't bother respinning for that unless the dma folk have comments for you. select ARCH_HAS_DMA_PREP_COHERENT select ARCH_HAS_SETUP_DMA_OPS select ARCH_HAS_SYNC_DMA_FOR_CPU -- 2.37.1 (Apple Git-137.1)
Re: [PATCH 14/36] powerpc/pseries: move to use bus_get_dev_root()
Greg Kroah-Hartman writes: > Direct access to the struct bus_type dev_root pointer is going away soon > so replace that with a call to bus_get_dev_root() instead, which is what > it is there for. > > Cc: Michael Ellerman > Cc: Nicholas Piggin > Cc: Christophe Leroy > Cc: Greg Kroah-Hartman > Cc: linuxppc-dev@lists.ozlabs.org > Signed-off-by: Greg Kroah-Hartman > --- > Note, this is a patch that is a prepatory cleanup as part of a larger > series of patches that is working on resolving some old driver core > design mistakes. It will build and apply cleanly on top of 6.3-rc2 on > its own, but I'd prefer if I could take it through my driver-core tree > so that the driver core changes can be taken through there for 6.4-rc1. > > .../platforms/pseries/pseries_energy.c| 28 +++ > arch/powerpc/platforms/pseries/suspend.c | 10 +-- > 2 files changed, 25 insertions(+), 13 deletions(-) Acked-by: Michael Ellerman (powerpc) The other powerpc changes look OK too. cheers
Re: [RFC PATCH v1] powerpc: Add version to install filenames
Nick Child writes: > Rather than replacing the versionless vmlinux and System.map files, > copy to files with the version info appended. > > Additionally, since executing the script is a last resort option, > inform the user about the missing `installkernel` command and the > location of the installation. > > This work is adapted from `arch/s390/boot/install.sh`. > > Signed-off-by: Nick Child > --- > > Hoping I am not breaking someones dependency on targeting /boot/vmlinux > so RFC'ing. It will probably break *someone*'s workflow :) > I typically have kernelinstall on my LPARs and installing and rebooting > goes peacefully. > > Recently, I did not have kernelinstall and `make install` seemed to behave > differently. I got very little output but a succeful return code. After > initramfs issues during boot I dug into the makefiles a bit to figure out > where execution was differing. When `kernelinstall` cannot be found, we > invoke `arch/powerpc/boot/install.sh` instead. I am primarily interested > in getting more information relayed to the user about what is going on. > > The changes to installing with the version appended are more of an > afterthought > that makes sense to me but could understand why someone may depend on > consistent > filenames. > > Opening as RFC for opinions/rejections/concerns. TIL arch/powerpc/boot/install.sh even exists :) I generally netboot kernels, so I don't really use `make install` that much. But I know some folks do, though they probably have `installkernel` installed as a rule. Still this change seems sensible, and putting the version in the file names matches what arm, s390, arm64 and riscv do. See if anyone else has an opinion. cheers
Re: [PATCH] PCI/AER: correctable error message as KERN_INFO
On Tue, Mar 14, 2023 at 12:38 PM Bjorn Helgaas wrote:
>
> On Tue, Feb 28, 2023 at 10:04:53PM -0800, Grant Grundler wrote:
> > Since correctable errors have been corrected (and counted), the dmesg output
> > should not be reported as a warning, but rather as "informational".
> >
> > Otherwise, using a certain well known vendor's PCIe parts in a USB4 docking
> > station, the dmesg buffer can be spammed with correctable errors, 717 bytes
> > per instance, potentially many MB per day.
> >
> > Given the "WARN" priority, these messages have already confused the typical
> > user that stumbles across them, support staff (triaging feedback reports),
> > and more than a few linux kernel devs. Changing to INFO will hide these
> > messages from most audiences.
> >
> > Signed-off-by: Grant Grundler
> > ---
> > This patch will likely conflict with:
> >
> > https://lore.kernel.org/all/20230103165548.570377-1-rajat.khandel...@linux.intel.com/
> >
> > which I'd also like to see upstream. Please let me know to resubmit
> > mine if Rajat's patch lands first. Or feel free to fix up this one.
>
> Yes. I think it makes sense to separate this into two patches:
>
> 1) Log correctable errors as KERN_INFO instead of KERN_WARNING, and
> 2) Rate-limit correctable error logging.
I'm going to look into your comment below. I'll port Rajat's patch on
top of mine to follow the order you've listed above.
> > drivers/pci/pcie/aer.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> > index f6c24ded134c..e4cf3ec40d66 100644
> > --- a/drivers/pci/pcie/aer.c
> > +++ b/drivers/pci/pcie/aer.c
> > @@ -692,7 +692,7 @@ static void __aer_print_error(struct pci_dev *dev,
> >
> > if (info->severity == AER_CORRECTABLE) {
> > strings = aer_correctable_error_string;
> > - level = KERN_WARNING;
> > + level = KERN_INFO;
> > } else {
> > strings = aer_uncorrectable_error_string;
> > level = KERN_ERR;
> > @@ -724,7 +724,7 @@ void aer_print_error(struct pci_dev *dev, struct
> > aer_err_info *info)
> > layer = AER_GET_LAYER_ERROR(info->severity, info->status);
> > agent = AER_GET_AGENT(info->severity, info->status);
> >
> > - level = (info->severity == AER_CORRECTABLE) ? KERN_WARNING : KERN_ERR;
> > + level = (info->severity == AER_CORRECTABLE) ? KERN_INFO : KERN_ERR;
> >
> > pci_printk(level, dev, "PCIe Bus Error: severity=%s, type=%s, (%s)\n",
> > aer_error_severity_string[info->severity],
>
> Shouldn't we do the same in the cper_print_aer() path? That path
> currently uses pci_err() and then calls __aer_print_error(), so the
> initial message will always be KERN_ERR, and the decoding done by
> __aer_print_error() will be KERN_INFO (for correctable) or KERN_ERR.
I was completely unaware of this since it's not causing me any
immediate problems. But I agree the message priority should be
consistent for correctable errors.
> Seems like a shame to do the same test in three places, but would
> require a little more refactoring to avoid that.
I don't mind doing the same test in multiple places. If refactoring
this isn't straight forward, I'll leave the refactoring for someone
more ambitious. :D
cheers,
grant
>
> Bjorn
Re: [PATCH] modpost: support arbitrary symbol length in modversion
> In practice, this is what I'm testing at the moment:
>
> ---
> diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
> index ff045644f13f..ea6c830ed1e7 100644
> --- a/arch/powerpc/kernel/module_64.c
> +++ b/arch/powerpc/kernel/module_64.c
> @@ -234,12 +234,13 @@ static unsigned long get_stubs_size(const Elf64_Ehdr
> *hdr,
> static void dedotify_versions(struct modversion_info *vers,
> unsigned long size)
> {
> - struct modversion_info *end;
> + struct modversion_info *end = (void *)vers + size;
>
> - for (end = (void *)vers + size; vers < end; vers++)
> + for (; vers < end && vers->next; vers = (void *)vers + vers->next) {
> if (vers->name[0] == '.') {
> memmove(vers->name, vers->name+1, strlen(vers->name));
> }
> + }
> }
>
> /*
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 8c5909c0076c..4744901bdf63 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -34,9 +34,11 @@
> #define MODULE_NAME_LEN MAX_PARAM_PREFIX_LEN
>
> struct modversion_info {
> - unsigned long crc;
> - char name[MODULE_NAME_LEN];
> -};
> + /* Offset of the next modversion entry in relation to this one. */
> + u32 next;
> + u32 crc;
> + char name[0];
> +} __packed;
>
> struct module;
> struct exception_table_entry;
> diff --git a/kernel/module/version.c b/kernel/module/version.c
> index 53f43ac5a73e..5528f98c42dc 100644
> --- a/kernel/module/version.c
> +++ b/kernel/module/version.c
> @@ -17,32 +17,30 @@ int check_version(const struct load_info *info,
> {
> Elf_Shdr *sechdrs = info->sechdrs;
> unsigned int versindex = info->index.vers;
> - unsigned int i, num_versions;
> - struct modversion_info *versions;
> + struct modversion_info *versions, *end;
> + u32 crcval;
>
> /* Exporting module didn't supply crcs? OK, we're already tainted. */
> if (!crc)
> return 1;
> + crcval = *crc;
>
> /* No versions at all? modprobe --force does this. */
> if (versindex == 0)
> return try_to_force_load(mod, symname) == 0;
>
> versions = (void *)sechdrs[versindex].sh_addr;
> - num_versions = sechdrs[versindex].sh_size
> - / sizeof(struct modversion_info);
> + end = (void *)versions + sechdrs[versindex].sh_size;
>
> - for (i = 0; i < num_versions; i++) {
> - u32 crcval;
> -
> - if (strcmp(versions[i].name, symname) != 0)
> + for (; versions < end && versions->next;
> +versions = (void *)versions + versions->next) {
> + if (strcmp(versions->name, symname) != 0)
> continue;
>
> - crcval = *crc;
> - if (versions[i].crc == crcval)
> + if (versions->crc == crcval)
> return 1;
> - pr_debug("Found checksum %X vs module %lX\n",
> - crcval, versions[i].crc);
> + pr_debug("Found checksum %X vs module %X\n",
> + crcval, versions->crc);
> goto bad_version;
> }
>
> diff --git a/scripts/export_report.pl b/scripts/export_report.pl
> index feb3d5542a62..1117646f3141 100755
> --- a/scripts/export_report.pl
> +++ b/scripts/export_report.pl
> @@ -116,18 +116,19 @@ foreach my $thismod (@allcfiles) {
> while ( <$module> ) {
> chomp;
> if ($state == 0) {
> - $state = 1 if ($_ =~ /static const struct
> modversion_info/);
> + $state = 1 if ($_ =~ /static const char versions/);
> next;
> }
> if ($state == 1) {
> - $state = 2 if ($_ =~
> /__attribute__\(\(section\("__versions"\)\)\)/);
> + $state = 2 if ($_ =~ /__used
> __section\("__versions"\)/);
> next;
> }
> if ($state == 2) {
> - if ( $_ !~ /0x[0-9a-f]+,/ ) {
> + if ( $_ !~ /\\0"/ ) {
> + last if ($_ =~ /;/);
> next;
> }
> - my $sym = (split /([,"])/,)[4];
> + my $sym = (split /(["\\])/,)[2];
> my ($module, $value, $symbol, $gpl) = @{$SYMBOL{$sym}};
> $SYMBOL{ $sym } = [ $module, $value+1, $symbol, $gpl];
> push(@{$MODULE{$thismod}} , $sym);
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index efff8078e395..55335ae98f4f 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -2046,13 +2046,17 @@ static void add_exported_symbols(struct buffer *buf,
> struct module *mod)
> static void add_versions(struct buffer *b, struct module *mod)
> {
> struct symbol *s;
> + unsigned int name_len;
> +
Re: [PATCH 1/1] PCI: layerscape: Add the workaround for A-010305
On Thu, Jan 12, 2023 at 02:44:33PM -0500, Frank Li wrote:
> From: Xiaowei Bao
>
> When a link down or hot reset event occurs, the PCI Express EP
> controller's Link Capabilities Register should retain the values of
> the Maximum Link Width and Supported Link Speed configured by RCW.
Can you rework this to say what the patch does and why it's necessary?
Apparently it's a workaround for some issue in A-010305? The subject
line could also use more content. What is A-010305? What is the
problem this works around?
I don't see a check for A-010305; do *all* devices handled by this
driver have this problem?
The PCIe Link Capabilities is supposed to be read-only; maybe this
device loses the value on link down or hot reset? And I guess the
device interrupts on link up/down and reset, and you restore the value
then?
Link Capabilities contains several things other than Max Link Width
and Max Link Speed. But they don't need to be restored?
What is RCW?
> Signed-off-by: Xiaowei Bao
> Signed-off-by: Hou Zhiqiang
> Signed-off-by: Frank Li
> ---
> .../pci/controller/dwc/pci-layerscape-ep.c| 112 +-
> 1 file changed, 111 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> index ed5cfc9408d9..1b884854c18e 100644
> --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> @@ -18,6 +18,22 @@
>
> #include "pcie-designware.h"
>
> +#define PCIE_LINK_CAP0x7C/* PCIe Link
> Capabilities*/
Is this something you can find by searching the capability list
instead of hard-coding the config space offset?
> +#define MAX_LINK_SP_MASK 0x0F
> +#define MAX_LINK_W_MASK 0x3F
> +#define MAX_LINK_W_SHIFT 4
These look like they should use PCI_EXP_LNKCAP_SLS and
PCI_EXP_LNKCAP_MLW instead of defining new ones.
> +/* PEX PFa PCIE pme and message interrupt registers*/
> +#define PEX_PF0_PME_MES_DR 0xC0020
> +#define PEX_PF0_PME_MES_DR_LUD (1 << 7)
> +#define PEX_PF0_PME_MES_DR_LDD (1 << 9)
> +#define PEX_PF0_PME_MES_DR_HRD (1 << 10)
> +
> +#define PEX_PF0_PME_MES_IER0xC0028
> +#define PEX_PF0_PME_MES_IER_LUDIE (1 << 7)
> +#define PEX_PF0_PME_MES_IER_LDDIE (1 << 9)
> +#define PEX_PF0_PME_MES_IER_HRDIE (1 << 10)
> +
> #define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
>
> struct ls_pcie_ep_drvdata {
> @@ -30,8 +46,90 @@ struct ls_pcie_ep {
> struct dw_pcie *pci;
> struct pci_epc_features *ls_epc;
> const struct ls_pcie_ep_drvdata *drvdata;
> + u8 max_speed;
> + u8 max_width;
> + boolbig_endian;
> + int irq;
> };
>
> +static u32 ls_lut_readl(struct ls_pcie_ep *pcie, u32 offset)
> +{
> + struct dw_pcie *pci = pcie->pci;
> +
> + if (pcie->big_endian)
> + return ioread32be(pci->dbi_base + offset);
> + else
> + return ioread32(pci->dbi_base + offset);
> +}
> +
> +static void ls_lut_writel(struct ls_pcie_ep *pcie, u32 offset,
> + u32 value)
> +{
> + struct dw_pcie *pci = pcie->pci;
> +
> + if (pcie->big_endian)
> + iowrite32be(value, pci->dbi_base + offset);
> + else
> + iowrite32(value, pci->dbi_base + offset);
> +}
> +
> +static irqreturn_t ls_pcie_ep_event_handler(int irq, void *dev_id)
> +{
> + struct ls_pcie_ep *pcie = (struct ls_pcie_ep *)dev_id;
> + struct dw_pcie *pci = pcie->pci;
> + u32 val;
> +
> + val = ls_lut_readl(pcie, PEX_PF0_PME_MES_DR);
> + if (!val)
> + return IRQ_NONE;
> +
> + if (val & PEX_PF0_PME_MES_DR_LUD)
> + dev_info(pci->dev, "Detect the link up state !\n");
> + else if (val & PEX_PF0_PME_MES_DR_LDD)
> + dev_info(pci->dev, "Detect the link down state !\n");
> + else if (val & PEX_PF0_PME_MES_DR_HRD)
> + dev_info(pci->dev, "Detect the hot reset state !\n");
No space before "!". Seems possibly more verbose than necessary,
since the endpoint may be reset as part of normal operation.
> + dw_pcie_dbi_ro_wr_en(pci);
> + dw_pcie_writew_dbi(pci, PCIE_LINK_CAP,
> +(pcie->max_width << MAX_LINK_W_SHIFT) |
Use FIELD_PREP() so you don't need a shift.
> +pcie->max_speed);
> + dw_pcie_dbi_ro_wr_dis(pci);
> +
> + ls_lut_writel(pcie, PEX_PF0_PME_MES_DR, val);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int ls_pcie_ep_interrupt_init(struct ls_pcie_ep *pcie,
> + struct platform_device *pdev)
> +{
> + u32 val;
> + int ret;
> +
> + pcie->irq = platform_get_irq_byname(pdev, "pme");
> + if (pcie->irq < 0) {
> + dev_err(>dev, "Can't get 'pme'
Re: [PATCH v4 0/7] introduce vm_flags modifier functions
On Thu, 26 Jan 2023 11:37:45 -0800 Suren Baghdasaryan wrote: > This patchset was originally published as a part of per-VMA locking [1] and > was split after suggestion that it's viable on its own and to facilitate > the review process. It is now a preprequisite for the next version of per-VMA > lock patchset, which reuses vm_flags modifier functions to lock the VMA when > vm_flags are being updated. > > VMA vm_flags modifications are usually done under exclusive mmap_lock > protection because this attrubute affects other decisions like VMA merging > or splitting and races should be prevented. Introduce vm_flags modifier > functions to enforce correct locking. > > The patchset applies cleanly over mm-unstable branch of mm tree. With this series, vfio-pci developed a bunch of warnings around not holding the mmap_lock write semaphore while calling io_remap_pfn_range() from our fault handler, vfio_pci_mmap_fault(). I suspect vdpa has the same issue for their use of remap_pfn_range() from their fault handler, JasonW, MST, FYI. It also looks like gru_fault() would have the same issue, Dimitri. In all cases, we're preemptively setting vm_flags to what remap_pfn_range_notrack() uses, so I thought we were safe here as I specifically remember trying to avoid changing vm_flags from the fault handler. But apparently that doesn't take into account track_pfn_remap() where VM_PAT comes into play. The reason for using remap_pfn_range() on fault in vfio-pci is that we're mapping device MMIO to userspace, where that MMIO can be disabled and we'd rather zap the mapping when that occurs so that we can sigbus the user rather than allow the user to trigger potentially fatal bus errors on the host. Peter Xu has suggested offline that a non-lazy approach to reinsert the mappings might be more inline with mm expectations relative to touching vm_flags during fault. What's the right solution here? Can the fault handling be salvaged, is proactive remapping the right approach, or is there something better? Thanks, Alex
RE: [EXT] Re: [External] : [PATCH v3 1/1] PCI: layerscape: Add EP mode support for ls1028a
>
> Reviewed-by: Alok Tiwari
>
> On 2/9/2023 8:40 PM, Frank Li wrote:
> > From: Xiaowei Bao
> >
> > Add PCIe EP mode support for ls1028a.
> >
> > Signed-off-by: Xiaowei Bao
> > Signed-off-by: Hou Zhiqiang
> > Signed-off-by: Frank Li
> > Acked-by: Roy Zang
> > ---
Ping!
there are no feedback for over 1 month.
Just 1 line change.
> >
> > Change from v2 to v3
> > order by .compatible
> >
> > Change from v2 to v2
> > Added
> > Signed-off-by: Frank Li
> > Acked-by: Roy Zang
> >
> >
> > All other patches were already accepte by maintainer in
> >
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Furlde
> fense.com%2Fv3%2F__https%3A%2F%2Flore.kernel.org%2Flkml%2F202
> 2223457.10599-1-
> leoyang.li%40nxp.com%2F__%3B!!ACWV5N9M2RV99hQ!NR9EU4fPDwxdyrb
> 9tdBm9VNIMHSlw6dLgXCAPDSrm7ftWVNrh6JldLGzzrKyiE0xRlP5OdiGBN7PCf
> 9gRaA%24=05%7C01%7CFrank.Li%40nxp.com%7C1d32974e205b4a8591
> 9f08db0ba30b9b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6381
> 16567129733840%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiL
> CJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C
> ata=hGKffPqpE%2Ft66x71Y47ocGbIuFH7vpjLadlAXbnyBOw%3D=0
> >
> > But missed this one.
> >
> > Re-post
> >
> > drivers/pci/controller/dwc/pci-layerscape-ep.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > index ad99707b3b99..c640db60edc6 100644
> > --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > @@ -110,6 +110,7 @@ static const struct ls_pcie_ep_drvdata
> lx2_ep_drvdata = {
> > };
> >
> > static const struct of_device_id ls_pcie_ep_of_match[] = {
> > + { .compatible = "fsl,ls1028a-pcie-ep", .data = _ep_drvdata },
> > { .compatible = "fsl,ls1046a-pcie-ep", .data = _ep_drvdata },
> > { .compatible = "fsl,ls1088a-pcie-ep", .data = _ep_drvdata },
> > { .compatible = "fsl,ls2088a-pcie-ep", .data = _ep_drvdata },
Re: [PATCH v11 03/13] dt-bindings: Convert gpio-mmio to yaml
On 14/03/2023 20:52, Sean Anderson wrote: > On 3/14/23 15:45, Krzysztof Kozlowski wrote: >> On 14/03/2023 19:50, Sean Anderson wrote: >>> On 3/14/23 14:32, Krzysztof Kozlowski wrote: On 14/03/2023 19:09, Sean Anderson wrote: > On 3/14/23 13:56, Krzysztof Kozlowski wrote: >> On 13/03/2023 17:11, Sean Anderson wrote: >> + reg-names: >>> +minItems: 1 >>> +maxItems: 5 >>> +items: >>> + enum: >> >> Why this is in any order? Other bindings were here specific, your 'reg' >> is also specific/fixed. > > Some devicetrees have dirout first, and other have dat first. There is no > mandatory order, and some registers can be included or left out as is > convenient to the devicetree author. > > reg is not specific/fixed either. It is just done that way for > convenience (and to match the names here). The items have order and usually we require strict order from DTS, unless there is a reason. If there is no reason, use fixed order and then fix the DTS. >>> >>> The items do not have order. That is the whole point of having a >>> separate names property. The DTs are not "broken" for taking advantage >>> of a longstanding feature. There is no advantage to rewriting them to >>> use a fixed order, especially when there is no precedent. This is just >>> an area where json schema cannot completely validate devicetrees. >> >> I don't understand "there is no precedent".There is - we rewrite >> hundreds of DTS. Just look at mine and other people commits. > > There is no precedent for a fixed order of registers for this device. > We have always used reg-names to interpret regs. And who is "we"? Bootloader? Firmware? BSD? Because they all matter. It does not matter that one particular driver uses reg-names. The common rule is always the same - entries are ordered and fixed (with exceptions). > >> The reg-names are helper and entries were always expected to be ordered > > This is not the case for this device. Registers may be in any order, and Their physical order does not determine the order of entries in DT. > some registers may be omitted (and not always the same ones). OK, that's the reason. > reg-names is the > only way to determine which registers are present. Best regards, Krzysztof
RE: [PATCH 1/1] PCI: layerscape: Add the workaround for A-010305
>
> > -Original Message-
> > From: Frank Li
> > Subject: RE: [PATCH 1/1] PCI: layerscape: Add the workaround for A-010305
> >
> > > Subject: [PATCH 1/1] PCI: layerscape: Add the workaround for A-010305
> > >
> > > From: Xiaowei Bao
> > >
> > > When a link down or hot reset event occurs, the PCI Express EP
> > > controller's Link Capabilities Register should retain the values of
> > > the Maximum Link Width and Supported Link Speed configured by RCW.
> > >
> > > Signed-off-by: Xiaowei Bao
> > > Signed-off-by: Hou Zhiqiang
> > > Signed-off-by: Frank Li
> > > ---
> >
> > Ping
>
> Friendly ping.
Ping. No feedback for over 1 month!
>
> >
> >
> > > static struct platform_driver ls_pcie_ep_driver = {
> > > --
> > > 2.34.1
Re: [PATCH v11 03/13] dt-bindings: Convert gpio-mmio to yaml
On 3/14/23 15:45, Krzysztof Kozlowski wrote: > On 14/03/2023 19:50, Sean Anderson wrote: >> On 3/14/23 14:32, Krzysztof Kozlowski wrote: >>> On 14/03/2023 19:09, Sean Anderson wrote: On 3/14/23 13:56, Krzysztof Kozlowski wrote: > On 13/03/2023 17:11, Sean Anderson wrote: > + reg-names: >> +minItems: 1 >> +maxItems: 5 >> +items: >> + enum: > > Why this is in any order? Other bindings were here specific, your 'reg' > is also specific/fixed. Some devicetrees have dirout first, and other have dat first. There is no mandatory order, and some registers can be included or left out as is convenient to the devicetree author. reg is not specific/fixed either. It is just done that way for convenience (and to match the names here). >>> >>> The items have order and usually we require strict order from DTS, >>> unless there is a reason. If there is no reason, use fixed order and >>> then fix the DTS. >> >> The items do not have order. That is the whole point of having a >> separate names property. The DTs are not "broken" for taking advantage >> of a longstanding feature. There is no advantage to rewriting them to >> use a fixed order, especially when there is no precedent. This is just >> an area where json schema cannot completely validate devicetrees. > > I don't understand "there is no precedent".There is - we rewrite > hundreds of DTS. Just look at mine and other people commits. There is no precedent for a fixed order of registers for this device. We have always used reg-names to interpret regs. > The reg-names are helper and entries were always expected to be ordered This is not the case for this device. Registers may be in any order, and some registers may be omitted (and not always the same ones). reg-names is the only way to determine which registers are present. --Sean
Re: [PATCH v11 03/13] dt-bindings: Convert gpio-mmio to yaml
On 14/03/2023 19:50, Sean Anderson wrote: > On 3/14/23 14:32, Krzysztof Kozlowski wrote: >> On 14/03/2023 19:09, Sean Anderson wrote: >>> On 3/14/23 13:56, Krzysztof Kozlowski wrote: On 13/03/2023 17:11, Sean Anderson wrote: + reg-names: > +minItems: 1 > +maxItems: 5 > +items: > + enum: Why this is in any order? Other bindings were here specific, your 'reg' is also specific/fixed. >>> >>> Some devicetrees have dirout first, and other have dat first. There is no >>> mandatory order, and some registers can be included or left out as is >>> convenient to the devicetree author. >>> >>> reg is not specific/fixed either. It is just done that way for >>> convenience (and to match the names here). >> >> The items have order and usually we require strict order from DTS, >> unless there is a reason. If there is no reason, use fixed order and >> then fix the DTS. > > The items do not have order. That is the whole point of having a > separate names property. The DTs are not "broken" for taking advantage > of a longstanding feature. There is no advantage to rewriting them to > use a fixed order, especially when there is no precedent. This is just > an area where json schema cannot completely validate devicetrees. I don't understand "there is no precedent". There is - we rewrite hundreds of DTS. Just look at mine and other people commits. The reg-names are helper and entries were always expected to be ordered. On the other hand if different devices use different order, then it cannot be changed obviously (as the order is fixed). Best regards, Krzysztof
Re: [PATCH] PCI/AER: correctable error message as KERN_INFO
On Tue, Feb 28, 2023 at 10:04:53PM -0800, Grant Grundler wrote:
> Since correctable errors have been corrected (and counted), the dmesg output
> should not be reported as a warning, but rather as "informational".
>
> Otherwise, using a certain well known vendor's PCIe parts in a USB4 docking
> station, the dmesg buffer can be spammed with correctable errors, 717 bytes
> per instance, potentially many MB per day.
>
> Given the "WARN" priority, these messages have already confused the typical
> user that stumbles across them, support staff (triaging feedback reports),
> and more than a few linux kernel devs. Changing to INFO will hide these
> messages from most audiences.
>
> Signed-off-by: Grant Grundler
> ---
> This patch will likely conflict with:
>
> https://lore.kernel.org/all/20230103165548.570377-1-rajat.khandel...@linux.intel.com/
>
> which I'd also like to see upstream. Please let me know to resubmit
> mine if Rajat's patch lands first. Or feel free to fix up this one.
Yes. I think it makes sense to separate this into two patches:
1) Log correctable errors as KERN_INFO instead of KERN_WARNING, and
2) Rate-limit correctable error logging.
> drivers/pci/pcie/aer.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index f6c24ded134c..e4cf3ec40d66 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -692,7 +692,7 @@ static void __aer_print_error(struct pci_dev *dev,
>
> if (info->severity == AER_CORRECTABLE) {
> strings = aer_correctable_error_string;
> - level = KERN_WARNING;
> + level = KERN_INFO;
> } else {
> strings = aer_uncorrectable_error_string;
> level = KERN_ERR;
> @@ -724,7 +724,7 @@ void aer_print_error(struct pci_dev *dev, struct
> aer_err_info *info)
> layer = AER_GET_LAYER_ERROR(info->severity, info->status);
> agent = AER_GET_AGENT(info->severity, info->status);
>
> - level = (info->severity == AER_CORRECTABLE) ? KERN_WARNING : KERN_ERR;
> + level = (info->severity == AER_CORRECTABLE) ? KERN_INFO : KERN_ERR;
>
> pci_printk(level, dev, "PCIe Bus Error: severity=%s, type=%s, (%s)\n",
> aer_error_severity_string[info->severity],
Shouldn't we do the same in the cper_print_aer() path? That path
currently uses pci_err() and then calls __aer_print_error(), so the
initial message will always be KERN_ERR, and the decoding done by
__aer_print_error() will be KERN_INFO (for correctable) or KERN_ERR.
Seems like a shame to do the same test in three places, but would
require a little more refactoring to avoid that.
Bjorn
[PATCH v5 1/4] PCI: Introduce pci_dev_for_each_resource()
From: Mika Westerberg
Instead of open-coding it everywhere introduce a tiny helper that can be
used to iterate over each resource of a PCI device, and convert the most
obvious users into it.
While at it drop doubled empty line before pdev_sort_resources().
No functional changes intended.
Suggested-by: Andy Shevchenko
Signed-off-by: Mika Westerberg
Signed-off-by: Andy Shevchenko
Reviewed-by: Krzysztof Wilczyński
---
.clang-format | 2 ++
arch/alpha/kernel/pci.c | 5 ++--
arch/arm/kernel/bios32.c | 16 ++---
arch/arm/mach-dove/pcie.c | 10
arch/arm/mach-mv78xx0/pcie.c | 10
arch/arm/mach-orion5x/pci.c | 10
arch/mips/pci/ops-bcm63xx.c | 8 +++
arch/mips/pci/pci-legacy.c| 3 +--
arch/powerpc/kernel/pci-common.c | 21
arch/powerpc/platforms/4xx/pci.c | 8 +++
arch/powerpc/platforms/52xx/mpc52xx_pci.c | 4 ++--
arch/powerpc/platforms/pseries/pci.c | 16 ++---
arch/sh/drivers/pci/pcie-sh7786.c | 10
arch/sparc/kernel/leon_pci.c | 5 ++--
arch/sparc/kernel/pci.c | 10
arch/sparc/kernel/pcic.c | 5 ++--
drivers/pci/remove.c | 5 ++--
drivers/pci/setup-bus.c | 27 -
drivers/pci/setup-res.c | 4 +---
drivers/pci/vgaarb.c | 17 -
drivers/pci/xen-pcifront.c| 4 +---
drivers/pnp/quirks.c | 29 ---
include/linux/pci.h | 15 ++--
23 files changed, 111 insertions(+), 133 deletions(-)
diff --git a/.clang-format b/.clang-format
index d988e9fa9b26..266abb843654 100644
--- a/.clang-format
+++ b/.clang-format
@@ -520,6 +520,8 @@ ForEachMacros:
- 'of_property_for_each_string'
- 'of_property_for_each_u32'
- 'pci_bus_for_each_resource'
+ - 'pci_dev_for_each_resource'
+ - 'pci_dev_for_each_resource_p'
- 'pci_doe_for_each_off'
- 'pcl_for_each_chunk'
- 'pcl_for_each_segment'
diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
index 64fbfb0763b2..4458eb7f44f0 100644
--- a/arch/alpha/kernel/pci.c
+++ b/arch/alpha/kernel/pci.c
@@ -288,11 +288,10 @@ pcibios_claim_one_bus(struct pci_bus *b)
struct pci_bus *child_bus;
list_for_each_entry(dev, >devices, bus_list) {
+ struct resource *r;
int i;
- for (i = 0; i < PCI_NUM_RESOURCES; i++) {
- struct resource *r = >resource[i];
-
+ pci_dev_for_each_resource(dev, r, i) {
if (r->parent || !r->start || !r->flags)
continue;
if (pci_has_flag(PCI_PROBE_ONLY) ||
diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
index e7ef2b5bea9c..5254734b23e6 100644
--- a/arch/arm/kernel/bios32.c
+++ b/arch/arm/kernel/bios32.c
@@ -142,15 +142,15 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_WINBOND2,
PCI_DEVICE_ID_WINBOND2_89C940F,
*/
static void pci_fixup_dec21285(struct pci_dev *dev)
{
- int i;
-
if (dev->devfn == 0) {
+ struct resource *r;
+
dev->class &= 0xff;
dev->class |= PCI_CLASS_BRIDGE_HOST << 8;
- for (i = 0; i < PCI_NUM_RESOURCES; i++) {
- dev->resource[i].start = 0;
- dev->resource[i].end = 0;
- dev->resource[i].flags = 0;
+ pci_dev_for_each_resource_p(dev, r) {
+ r->start = 0;
+ r->end = 0;
+ r->flags = 0;
}
}
}
@@ -162,13 +162,11 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_DEC,
PCI_DEVICE_ID_DEC_21285, pci_fixup_d
static void pci_fixup_ide_bases(struct pci_dev *dev)
{
struct resource *r;
- int i;
if ((dev->class >> 8) != PCI_CLASS_STORAGE_IDE)
return;
- for (i = 0; i < PCI_NUM_RESOURCES; i++) {
- r = dev->resource + i;
+ pci_dev_for_each_resource_p(dev, r) {
if ((r->start & ~0x80) == 0x374) {
r->start |= 2;
r->end = r->start;
diff --git a/arch/arm/mach-dove/pcie.c b/arch/arm/mach-dove/pcie.c
index 754ca381f600..58cecd79a204 100644
--- a/arch/arm/mach-dove/pcie.c
+++ b/arch/arm/mach-dove/pcie.c
@@ -142,14 +142,14 @@ static struct pci_ops pcie_ops = {
static void rc_pci_fixup(struct pci_dev *dev)
{
if (dev->bus->parent == NULL && dev->devfn == 0) {
- int i;
+ struct resource *r;
dev->class &= 0xff;
dev->class |= PCI_CLASS_BRIDGE_HOST << 8;
- for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
-
[PATCH v5 3/4] EISA: Convert to use pci_bus_for_each_resource_p()
The pci_bus_for_each_resource_p() hides the iterator loop since
it may be not used otherwise. With this, we may drop that iterator
variable definition.
Signed-off-by: Andy Shevchenko
Reviewed-by: Krzysztof Wilczyński
---
drivers/eisa/pci_eisa.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/eisa/pci_eisa.c b/drivers/eisa/pci_eisa.c
index 930c2332c3c4..907b86384396 100644
--- a/drivers/eisa/pci_eisa.c
+++ b/drivers/eisa/pci_eisa.c
@@ -20,8 +20,8 @@ static struct eisa_root_device pci_eisa_root;
static int __init pci_eisa_init(struct pci_dev *pdev)
{
- int rc, i;
struct resource *res, *bus_res = NULL;
+ int rc;
if ((rc = pci_enable_device (pdev))) {
dev_err(>dev, "Could not enable device\n");
@@ -38,7 +38,7 @@ static int __init pci_eisa_init(struct pci_dev *pdev)
* eisa_root_register() can only deal with a single io port resource,
* so we use the first valid io port resource.
*/
- pci_bus_for_each_resource(pdev->bus, res, i)
+ pci_bus_for_each_resource_p(pdev->bus, res)
if (res && (res->flags & IORESOURCE_IO)) {
bus_res = res;
break;
--
2.39.2
[PATCH v5 2/4] PCI: Split pci_bus_for_each_resource_p() out of pci_bus_for_each_resource()
Refactor pci_bus_for_each_resource() in the same way as it's done in
pci_dev_for_each_resource() case. This will allow to hide iterator
inside the loop, where it's not used otherwise.
No functional changes intended.
Signed-off-by: Andy Shevchenko
Reviewed-by: Krzysztof Wilczyński
---
.clang-format | 1 +
drivers/pci/bus.c | 7 +++
drivers/pci/hotplug/shpchp_sysfs.c | 8
drivers/pci/pci.c | 5 ++---
drivers/pci/probe.c| 2 +-
drivers/pci/setup-bus.c| 10 --
include/linux/pci.h| 14 ++
7 files changed, 25 insertions(+), 22 deletions(-)
diff --git a/.clang-format b/.clang-format
index 266abb843654..81c9f055086f 100644
--- a/.clang-format
+++ b/.clang-format
@@ -520,6 +520,7 @@ ForEachMacros:
- 'of_property_for_each_string'
- 'of_property_for_each_u32'
- 'pci_bus_for_each_resource'
+ - 'pci_bus_for_each_resource_p'
- 'pci_dev_for_each_resource'
- 'pci_dev_for_each_resource_p'
- 'pci_doe_for_each_off'
diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index 549c4bd5caec..b0789d332d36 100644
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -182,13 +182,13 @@ static int pci_bus_alloc_from_region(struct pci_bus *bus,
struct resource *res,
void *alignf_data,
struct pci_bus_region *region)
{
- int i, ret;
struct resource *r, avail;
resource_size_t max;
+ int ret;
type_mask |= IORESOURCE_TYPE_BITS;
- pci_bus_for_each_resource(bus, r, i) {
+ pci_bus_for_each_resource_p(bus, r) {
resource_size_t min_used = min;
if (!r)
@@ -289,9 +289,8 @@ bool pci_bus_clip_resource(struct pci_dev *dev, int idx)
struct resource *res = >resource[idx];
struct resource orig_res = *res;
struct resource *r;
- int i;
- pci_bus_for_each_resource(bus, r, i) {
+ pci_bus_for_each_resource_p(bus, r) {
resource_size_t start, end;
if (!r)
diff --git a/drivers/pci/hotplug/shpchp_sysfs.c
b/drivers/pci/hotplug/shpchp_sysfs.c
index 64beed7a26be..ff04f0c5e7c3 100644
--- a/drivers/pci/hotplug/shpchp_sysfs.c
+++ b/drivers/pci/hotplug/shpchp_sysfs.c
@@ -24,16 +24,16 @@
static ssize_t show_ctrl(struct device *dev, struct device_attribute *attr,
char *buf)
{
struct pci_dev *pdev;
- int index, busnr;
struct resource *res;
struct pci_bus *bus;
size_t len = 0;
+ int busnr;
pdev = to_pci_dev(dev);
bus = pdev->subordinate;
len += sysfs_emit_at(buf, len, "Free resources: memory\n");
- pci_bus_for_each_resource(bus, res, index) {
+ pci_bus_for_each_resource_p(bus, res) {
if (res && (res->flags & IORESOURCE_MEM) &&
!(res->flags & IORESOURCE_PREFETCH)) {
len += sysfs_emit_at(buf, len,
@@ -43,7 +43,7 @@ static ssize_t show_ctrl(struct device *dev, struct
device_attribute *attr, char
}
}
len += sysfs_emit_at(buf, len, "Free resources: prefetchable memory\n");
- pci_bus_for_each_resource(bus, res, index) {
+ pci_bus_for_each_resource_p(bus, res) {
if (res && (res->flags & IORESOURCE_MEM) &&
(res->flags & IORESOURCE_PREFETCH)) {
len += sysfs_emit_at(buf, len,
@@ -53,7 +53,7 @@ static ssize_t show_ctrl(struct device *dev, struct
device_attribute *attr, char
}
}
len += sysfs_emit_at(buf, len, "Free resources: IO\n");
- pci_bus_for_each_resource(bus, res, index) {
+ pci_bus_for_each_resource_p(bus, res) {
if (res && (res->flags & IORESOURCE_IO)) {
len += sysfs_emit_at(buf, len,
"start = %8.8llx, length =
%8.8llx\n",
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 7a67611dc5f4..2f8915ab41ef 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -779,9 +779,8 @@ struct resource *pci_find_parent_resource(const struct
pci_dev *dev,
{
const struct pci_bus *bus = dev->bus;
struct resource *r;
- int i;
- pci_bus_for_each_resource(bus, r, i) {
+ pci_bus_for_each_resource_p(bus, r) {
if (!r)
continue;
if (resource_contains(r, res)) {
@@ -799,7 +798,7 @@ struct resource *pci_find_parent_resource(const struct
pci_dev *dev,
* be both a positively-decoded aperture and a
* subtractively-decoded region that contain the BAR.
* We want the positively-decoded one, so this depends
-* on pci_bus_for_each_resource() giving us those
+* on pci_bus_for_each_resource_p() giving us those
*
[PATCH v5 4/4] pcmcia: Convert to use pci_bus_for_each_resource_p()
The pci_bus_for_each_resource_p() hides the iterator loop since
it may be not used otherwise. With this, we may drop that iterator
variable definition.
Signed-off-by: Andy Shevchenko
Reviewed-by: Krzysztof Wilczyński
Acked-by: Dominik Brodowski
---
drivers/pcmcia/rsrc_nonstatic.c | 9 +++--
drivers/pcmcia/yenta_socket.c | 3 +--
2 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/pcmcia/rsrc_nonstatic.c b/drivers/pcmcia/rsrc_nonstatic.c
index ad1141fddb4c..9d92d4bb6239 100644
--- a/drivers/pcmcia/rsrc_nonstatic.c
+++ b/drivers/pcmcia/rsrc_nonstatic.c
@@ -934,7 +934,7 @@ static int adjust_io(struct pcmcia_socket *s, unsigned int
action, unsigned long
static int nonstatic_autoadd_resources(struct pcmcia_socket *s)
{
struct resource *res;
- int i, done = 0;
+ int done = 0;
if (!s->cb_dev || !s->cb_dev->bus)
return -ENODEV;
@@ -960,12 +960,9 @@ static int nonstatic_autoadd_resources(struct
pcmcia_socket *s)
*/
if (s->cb_dev->bus->number == 0)
return -EINVAL;
-
- for (i = 0; i < PCI_BRIDGE_RESOURCE_NUM; i++) {
- res = s->cb_dev->bus->resource[i];
-#else
- pci_bus_for_each_resource(s->cb_dev->bus, res, i) {
#endif
+
+ pci_bus_for_each_resource_p(s->cb_dev->bus, res) {
if (!res)
continue;
diff --git a/drivers/pcmcia/yenta_socket.c b/drivers/pcmcia/yenta_socket.c
index 1365eaa20ff4..2e5bdf3db0ba 100644
--- a/drivers/pcmcia/yenta_socket.c
+++ b/drivers/pcmcia/yenta_socket.c
@@ -673,9 +673,8 @@ static int yenta_search_res(struct yenta_socket *socket,
struct resource *res,
u32 min)
{
struct resource *root;
- int i;
- pci_bus_for_each_resource(socket->dev->bus, root, i) {
+ pci_bus_for_each_resource_p(socket->dev->bus, root) {
if (!root)
continue;
--
2.39.2
[PATCH v5 0/4] PCI: Add pci_dev_for_each_resource() helper and update users
Provide two new helper macros to iterate over PCI device resources and convert users. Looking at it, refactor existing pci_bus_for_each_resource() and convert users accordingly. Changelog v5: - renamed loop variable to minimize the clash (Keith) - addressed smatch warning (Dan) - addressed 0-day bot findings (LKP) Changelog v4: - rebased on top of v6.3-rc1 - added tag (Krzysztof) Changelog v3: - rebased on top of v2 by Mika, see above - added tag to pcmcia patch (Dominik) Changelog v2: - refactor to have two macros - refactor existing pci_bus_for_each_resource() in the same way and convert users Andy Shevchenko (3): PCI: Split pci_bus_for_each_resource_p() out of pci_bus_for_each_resource() EISA: Convert to use pci_bus_for_each_resource_p() pcmcia: Convert to use pci_bus_for_each_resource_p() Mika Westerberg (1): PCI: Introduce pci_dev_for_each_resource() .clang-format | 3 ++ arch/alpha/kernel/pci.c | 5 ++- arch/arm/kernel/bios32.c | 16 +- arch/arm/mach-dove/pcie.c | 10 +++--- arch/arm/mach-mv78xx0/pcie.c | 10 +++--- arch/arm/mach-orion5x/pci.c | 10 +++--- arch/mips/pci/ops-bcm63xx.c | 8 ++--- arch/mips/pci/pci-legacy.c| 3 +- arch/powerpc/kernel/pci-common.c | 21 +++-- arch/powerpc/platforms/4xx/pci.c | 8 ++--- arch/powerpc/platforms/52xx/mpc52xx_pci.c | 4 +-- arch/powerpc/platforms/pseries/pci.c | 16 +- arch/sh/drivers/pci/pcie-sh7786.c | 10 +++--- arch/sparc/kernel/leon_pci.c | 5 ++- arch/sparc/kernel/pci.c | 10 +++--- arch/sparc/kernel/pcic.c | 5 ++- drivers/eisa/pci_eisa.c | 4 +-- drivers/pci/bus.c | 7 ++--- drivers/pci/hotplug/shpchp_sysfs.c| 8 ++--- drivers/pci/pci.c | 5 ++- drivers/pci/probe.c | 2 +- drivers/pci/remove.c | 5 ++- drivers/pci/setup-bus.c | 37 +-- drivers/pci/setup-res.c | 4 +-- drivers/pci/vgaarb.c | 17 +++ drivers/pci/xen-pcifront.c| 4 +-- drivers/pcmcia/rsrc_nonstatic.c | 9 ++ drivers/pcmcia/yenta_socket.c | 3 +- drivers/pnp/quirks.c | 29 ++ include/linux/pci.h | 29 ++ 30 files changed, 142 insertions(+), 165 deletions(-) -- 2.39.2
[PATCH v2] net: Use of_property_read_bool() for boolean properties
It is preferred to use typed property access functions (i.e. of_property_read_ functions) rather than low-level of_get_property/of_find_property functions for reading properties. Convert reading boolean properties to of_property_read_bool(). Reviewed-by: Simon Horman Acked-by: Marc Kleine-Budde # for net/can Acked-by: Kalle Valo Acked-by: Nicolas Ferre Acked-by: Francois Romieu Reviewed-by: Wei Fang Signed-off-by: Rob Herring --- v2: - Convert no_eeprom type to bool. --- drivers/net/can/cc770/cc770_platform.c | 12 ++-- drivers/net/ethernet/cadence/macb_main.c| 2 +- drivers/net/ethernet/davicom/dm9000.c | 4 ++-- drivers/net/ethernet/freescale/fec_main.c | 2 +- drivers/net/ethernet/freescale/fec_mpc52xx.c| 2 +- drivers/net/ethernet/freescale/gianfar.c| 4 ++-- drivers/net/ethernet/ibm/emac/core.c| 8 drivers/net/ethernet/ibm/emac/rgmii.c | 2 +- drivers/net/ethernet/stmicro/stmmac/dwmac-imx.c | 3 +-- drivers/net/ethernet/sun/niu.c | 2 +- drivers/net/ethernet/ti/cpsw-phy-sel.c | 3 +-- drivers/net/ethernet/ti/netcp_ethss.c | 8 +++- drivers/net/ethernet/via/via-velocity.c | 3 +-- drivers/net/ethernet/via/via-velocity.h | 2 +- drivers/net/ethernet/xilinx/ll_temac_main.c | 9 - drivers/net/wan/fsl_ucc_hdlc.c | 11 +++ drivers/net/wireless/ti/wlcore/spi.c| 3 +-- net/ncsi/ncsi-manage.c | 4 ++-- 18 files changed, 36 insertions(+), 48 deletions(-) diff --git a/drivers/net/can/cc770/cc770_platform.c b/drivers/net/can/cc770/cc770_platform.c index 8d916e2ee6c2..8dcc32e4e30e 100644 --- a/drivers/net/can/cc770/cc770_platform.c +++ b/drivers/net/can/cc770/cc770_platform.c @@ -93,20 +93,20 @@ static int cc770_get_of_node_data(struct platform_device *pdev, if (priv->can.clock.freq > 800) priv->cpu_interface |= CPUIF_DMC; - if (of_get_property(np, "bosch,divide-memory-clock", NULL)) + if (of_property_read_bool(np, "bosch,divide-memory-clock")) priv->cpu_interface |= CPUIF_DMC; - if (of_get_property(np, "bosch,iso-low-speed-mux", NULL)) + if (of_property_read_bool(np, "bosch,iso-low-speed-mux")) priv->cpu_interface |= CPUIF_MUX; if (!of_get_property(np, "bosch,no-comperator-bypass", NULL)) priv->bus_config |= BUSCFG_CBY; - if (of_get_property(np, "bosch,disconnect-rx0-input", NULL)) + if (of_property_read_bool(np, "bosch,disconnect-rx0-input")) priv->bus_config |= BUSCFG_DR0; - if (of_get_property(np, "bosch,disconnect-rx1-input", NULL)) + if (of_property_read_bool(np, "bosch,disconnect-rx1-input")) priv->bus_config |= BUSCFG_DR1; - if (of_get_property(np, "bosch,disconnect-tx1-output", NULL)) + if (of_property_read_bool(np, "bosch,disconnect-tx1-output")) priv->bus_config |= BUSCFG_DT1; - if (of_get_property(np, "bosch,polarity-dominant", NULL)) + if (of_property_read_bool(np, "bosch,polarity-dominant")) priv->bus_config |= BUSCFG_POL; prop = of_get_property(np, "bosch,clock-out-frequency", _size); diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c index 6e141a8bbf43..66e30561569e 100644 --- a/drivers/net/ethernet/cadence/macb_main.c +++ b/drivers/net/ethernet/cadence/macb_main.c @@ -4990,7 +4990,7 @@ static int macb_probe(struct platform_device *pdev) bp->jumbo_max_len = macb_config->jumbo_max_len; bp->wol = 0; - if (of_get_property(np, "magic-packet", NULL)) + if (of_property_read_bool(np, "magic-packet")) bp->wol |= MACB_WOL_HAS_MAGIC_PACKET; device_set_wakeup_capable(>dev, bp->wol & MACB_WOL_HAS_MAGIC_PACKET); diff --git a/drivers/net/ethernet/davicom/dm9000.c b/drivers/net/ethernet/davicom/dm9000.c index b21e56de6167..05a89ab6766c 100644 --- a/drivers/net/ethernet/davicom/dm9000.c +++ b/drivers/net/ethernet/davicom/dm9000.c @@ -1393,9 +1393,9 @@ static struct dm9000_plat_data *dm9000_parse_dt(struct device *dev) if (!pdata) return ERR_PTR(-ENOMEM); - if (of_find_property(np, "davicom,ext-phy", NULL)) + if (of_property_read_bool(np, "davicom,ext-phy")) pdata->flags |= DM9000_PLATF_EXT_PHY; - if (of_find_property(np, "davicom,no-eeprom", NULL)) + if (of_property_read_bool(np, "davicom,no-eeprom")) pdata->flags |= DM9000_PLATF_NO_EEPROM; ret = of_get_mac_address(np, pdata->dev_addr); diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index c73e25f8995e..f3b16a6673e2 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -4251,7 +4251,7 @@
Re: [PATCH] net: Use of_property_read_bool() for boolean properties
On Sat, Mar 11, 2023 at 5:50 AM Simon Horman wrote:
>
> On Fri, Mar 10, 2023 at 08:47:16AM -0600, Rob Herring wrote:
> > It is preferred to use typed property access functions (i.e.
> > of_property_read_ functions) rather than low-level
> > of_get_property/of_find_property functions for reading properties.
> > Convert reading boolean properties to to of_property_read_bool().
> >
> > Signed-off-by: Rob Herring
>
> Reviewed-by: Simon Horman
>
> ...
>
> > diff --git a/drivers/net/ethernet/via/via-velocity.c
> > b/drivers/net/ethernet/via/via-velocity.c
> > index a502812ac418..86f7843b4591 100644
> > --- a/drivers/net/ethernet/via/via-velocity.c
> > +++ b/drivers/net/ethernet/via/via-velocity.c
> > @@ -2709,8 +2709,7 @@ static int velocity_get_platform_info(struct
> > velocity_info *vptr)
> > struct resource res;
> > int ret;
> >
> > - if (of_get_property(vptr->dev->of_node, "no-eeprom", NULL))
> > - vptr->no_eeprom = 1;
> > + vptr->no_eeprom = of_property_read_bool(vptr->dev->of_node,
> > "no-eeprom");
>
> As per my comment on "[PATCH] nfc: mrvl: Use of_property_read_bool() for
> boolean properties".
>
> I'm not that enthusiastic about assigning a bool value to a field
> with an integer type. But that is likely a topic for another patch.
>
> > ret = of_address_to_resource(vptr->dev->of_node, 0, );
> > if (ret) {
>
> ...
>
> > diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
> > index 1c53b5546927..47c2ad7a3e42 100644
> > --- a/drivers/net/wan/fsl_ucc_hdlc.c
> > +++ b/drivers/net/wan/fsl_ucc_hdlc.c
> > @@ -1177,14 +1177,9 @@ static int ucc_hdlc_probe(struct platform_device
> > *pdev)
> > uhdlc_priv->dev = >dev;
> > uhdlc_priv->ut_info = ut_info;
> >
> > - if (of_get_property(np, "fsl,tdm-interface", NULL))
> > - uhdlc_priv->tsa = 1;
> > -
> > - if (of_get_property(np, "fsl,ucc-internal-loopback", NULL))
> > - uhdlc_priv->loopback = 1;
> > -
> > - if (of_get_property(np, "fsl,hdlc-bus", NULL))
> > - uhdlc_priv->hdlc_bus = 1;
> > + uhdlc_priv->tsa = of_property_read_bool(np, "fsl,tdm-interface");
>
> Here too.
These are already bool. Turns out the only one that needs changing is
no_eeprom. netdev folks marked this as changes requested, so I'll add
that in v2.
Rob
Re: [PATCH v11 03/13] dt-bindings: Convert gpio-mmio to yaml
On 3/14/23 14:32, Krzysztof Kozlowski wrote: > On 14/03/2023 19:09, Sean Anderson wrote: >> On 3/14/23 13:56, Krzysztof Kozlowski wrote: >>> On 13/03/2023 17:11, Sean Anderson wrote: >>> + reg-names: +minItems: 1 +maxItems: 5 +items: + enum: >>> >>> Why this is in any order? Other bindings were here specific, your 'reg' >>> is also specific/fixed. >> >> Some devicetrees have dirout first, and other have dat first. There is no >> mandatory order, and some registers can be included or left out as is >> convenient to the devicetree author. >> >> reg is not specific/fixed either. It is just done that way for >> convenience (and to match the names here). > > The items have order and usually we require strict order from DTS, > unless there is a reason. If there is no reason, use fixed order and > then fix the DTS. The items do not have order. That is the whole point of having a separate names property. The DTs are not "broken" for taking advantage of a longstanding feature. There is no advantage to rewriting them to use a fixed order, especially when there is no precedent. This is just an area where json schema cannot completely validate devicetrees. --Sean
Re: [PATCH v4 1/4] PCI: Introduce pci_dev_for_each_resource()
On Sat, Mar 11, 2023 at 04:54:32PM +0300, Dan Carpenter wrote: > 059b4a086017fb Mika Westerberg 2023-03-10 246 > unsigned long type = resource_type(r); > 999ed65ad12e37 Rene Herman 2008-07-25 247 > 059b4a086017fb Mika Westerberg 2023-03-10 @248if > (type != IORESOURCE_IO || type != IORESOURCE_MEM || > > ^^ > This || needs to be &&. This loop will always hit the continue path > without doing anything. > > 059b4a086017fb Mika Westerberg 2023-03-10 249 > resource_size(r) == 0) > 0509ad5e1a7d92 Bjorn Helgaas 2008-03-11 250 > continue; Thanks, I'll fix in v5. -- With Best Regards, Andy Shevchenko
Re: [PATCH v4 1/4] PCI: Introduce pci_dev_for_each_resource()
On Fri, Mar 10, 2023 at 03:15:38PM -0700, Keith Busch wrote: > On Fri, Mar 10, 2023 at 07:14:13PM +0200, Andy Shevchenko wrote: ... > > +#define pci_dev_for_each_resource_p(dev, res) > > \ > > + __pci_dev_for_each_resource(dev, res, i, unsigned int) > > It looks dangerous to have a macro declare a variable when starting a new > scope. How do you know the name 'i' won't clash with something defined above? I'll rename. Thank you. -- With Best Regards, Andy Shevchenko
Re: [PATCH v11 03/13] dt-bindings: Convert gpio-mmio to yaml
On 14/03/2023 19:09, Sean Anderson wrote:
> On 3/14/23 13:56, Krzysztof Kozlowski wrote:
>> On 13/03/2023 17:11, Sean Anderson wrote:
>>> This is a generic binding for simple MMIO GPIO controllers. Although we
>>> have a single driver for these controllers, they were previously spread
>>> over several files. Consolidate them. The register descriptions are
>>> adapted from the comments in the source. There is no set order for the
>>> registers, so I have not specified one.
>>>
>>> Rename brcm,bcm6345-gpio to brcm,bcm63xx-gpio to reflect that bcm6345
>>> has moved.
>>>
>>> Signed-off-by: Sean Anderson
>>> Reviewed-by: Linus Walleij
>>> ---
>>> Linus or Bartosz, feel free to pick this up as the rest of this series
>>> may not be merged any time soon.
>>>
>>> Changes in v11:
>>> - Keep empty (or almost-empty) properties on a single line
>>> - Don't use | unnecessarily
>>> - Use gpio as the node name for examples
>>> - Rename brcm,bcm6345-gpio.yaml to brcm,bcm63xx-gpio.yaml
>>>
>>> Changes in v10:
>>> - New
>>>
>>> ...m6345-gpio.yaml => brcm,bcm63xx-gpio.yaml} | 16 +--
>>> .../devicetree/bindings/gpio/gpio-mmio.yaml | 134 ++
>>> .../bindings/gpio/ni,169445-nand-gpio.txt | 38 -
>>> .../devicetree/bindings/gpio/wd,mbl-gpio.txt | 38 -
>>> 4 files changed, 135 insertions(+), 91 deletions(-)
>>> rename Documentation/devicetree/bindings/gpio/{brcm,bcm6345-gpio.yaml =>
>>> brcm,bcm63xx-gpio.yaml} (78%)
>>> create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mmio.yaml
>>> delete mode 100644
>>> Documentation/devicetree/bindings/gpio/ni,169445-nand-gpio.txt
>>> delete mode 100644 Documentation/devicetree/bindings/gpio/wd,mbl-gpio.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/gpio/brcm,bcm6345-gpio.yaml
>>> b/Documentation/devicetree/bindings/gpio/brcm,bcm63xx-gpio.yaml
>>> similarity index 78%
>>> rename from Documentation/devicetree/bindings/gpio/brcm,bcm6345-gpio.yaml
>>> rename to Documentation/devicetree/bindings/gpio/brcm,bcm63xx-gpio.yaml
>>> index 4d69f79df859..e11f4af49c52 100644
>>> --- a/Documentation/devicetree/bindings/gpio/brcm,bcm6345-gpio.yaml
>>> +++ b/Documentation/devicetree/bindings/gpio/brcm,bcm63xx-gpio.yaml
>>
>>
>>> +
>>> +description:
>>> + Some simple GPIO controllers may consist of a single data register or a
>>> pair
>>> + of set/clear-bit registers. Such controllers are common for glue logic in
>>> + FPGAs or ASICs. Commonly, these controllers are accessed over
>>> memory-mapped
>>> + NAND-style parallel busses.
>>> +
>>> +properties:
>>> + big-endian: true
>>> +
>>> + compatible:
>>
>> Keep compatible as first property.
>
> I thought it was alphabetical.
There is no clear rule, except that compatible is always first. In the
DTS reg is second, in bindings usually as well but not always.
>
>>> +enum:
>>> + - brcm,bcm6345-gpio # Broadcom BCM6345 GPIO controller
>>> + - wd,mbl-gpio # Western Digital MyBook Live memory-mapped GPIO
>>> controller
>>> + - ni,169445-nand-gpio # National Instruments 169445 GPIO NAND
>>> controller
>>
>> I think you got comment that these comments are making things
>> unreadable. I don't see here improvement.
>
> That was not the comment I got.
OK
>
> | I think you can inline description: statements in the enum instead of
> | the # hash comments, however IIRC you have to use oneOf and
> | const: to do it, like I do in
> | Documentation/devicetree/bindings/input/touchscreen/cypress,cy8ctma340.yaml
> | but don't overinvest in this if it is cumbersome.
>
> I investigated this and determined it was cumbersome.
So just :
# Western Digital MyBook Live memory-mapped GPIO controller
- wd,mbl-gpio
>
>> For example first comment is useless - you say the same as compatible.
>> Same with last one. So only remaining WD comment should be made in new
>> line so everything is nicely readable.
>
> I don't understand what you mean by "made in new line". Anyway, I will
> leave just the WD comment.
>
>> BTW, order the enum by name.
>
> OK
>
>>> +
>>> + '#gpio-cells':
>>> +const: 2
>>> +
>>> + gpio-controller:
>>> +true
>>
>> I am sure I saw comments here...
>>
>> https://lore.kernel.org/all/20230308231018.ga4039466-r...@kernel.org/
>
> OK
>
>>> +
>>> + reg:
>>> +minItems: 1
>>> +description:
>>> + A list of registers in the controller. The width of each register is
>>> + determined by its size.
>>
>> I don't understand this comment. Aren't you describing now what 'reg' is
>> in DT spec? If so, drop. If not, please share more.
>
> Each register describes exactly one hardware register. In some other
> device, when you see `regs = <0x800 0x100>`, then you may have 64
> 32-bit registers. But for this device, it would be one 2048-bit
> register.
Ah, so you do not mean here address space size? OK then, thanks for
clarification.
>
>>> All registers must have the same width. The number
>>> + of GPIOs is set by the width,
Re: [PATCH v11 03/13] dt-bindings: Convert gpio-mmio to yaml
On 3/14/23 13:56, Krzysztof Kozlowski wrote:
> On 13/03/2023 17:11, Sean Anderson wrote:
>> This is a generic binding for simple MMIO GPIO controllers. Although we
>> have a single driver for these controllers, they were previously spread
>> over several files. Consolidate them. The register descriptions are
>> adapted from the comments in the source. There is no set order for the
>> registers, so I have not specified one.
>>
>> Rename brcm,bcm6345-gpio to brcm,bcm63xx-gpio to reflect that bcm6345
>> has moved.
>>
>> Signed-off-by: Sean Anderson
>> Reviewed-by: Linus Walleij
>> ---
>> Linus or Bartosz, feel free to pick this up as the rest of this series
>> may not be merged any time soon.
>>
>> Changes in v11:
>> - Keep empty (or almost-empty) properties on a single line
>> - Don't use | unnecessarily
>> - Use gpio as the node name for examples
>> - Rename brcm,bcm6345-gpio.yaml to brcm,bcm63xx-gpio.yaml
>>
>> Changes in v10:
>> - New
>>
>> ...m6345-gpio.yaml => brcm,bcm63xx-gpio.yaml} | 16 +--
>> .../devicetree/bindings/gpio/gpio-mmio.yaml | 134 ++
>> .../bindings/gpio/ni,169445-nand-gpio.txt | 38 -
>> .../devicetree/bindings/gpio/wd,mbl-gpio.txt | 38 -
>> 4 files changed, 135 insertions(+), 91 deletions(-)
>> rename Documentation/devicetree/bindings/gpio/{brcm,bcm6345-gpio.yaml =>
>> brcm,bcm63xx-gpio.yaml} (78%)
>> create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mmio.yaml
>> delete mode 100644
>> Documentation/devicetree/bindings/gpio/ni,169445-nand-gpio.txt
>> delete mode 100644 Documentation/devicetree/bindings/gpio/wd,mbl-gpio.txt
>>
>> diff --git a/Documentation/devicetree/bindings/gpio/brcm,bcm6345-gpio.yaml
>> b/Documentation/devicetree/bindings/gpio/brcm,bcm63xx-gpio.yaml
>> similarity index 78%
>> rename from Documentation/devicetree/bindings/gpio/brcm,bcm6345-gpio.yaml
>> rename to Documentation/devicetree/bindings/gpio/brcm,bcm63xx-gpio.yaml
>> index 4d69f79df859..e11f4af49c52 100644
>> --- a/Documentation/devicetree/bindings/gpio/brcm,bcm6345-gpio.yaml
>> +++ b/Documentation/devicetree/bindings/gpio/brcm,bcm63xx-gpio.yaml
>
>
>> +
>> +description:
>> + Some simple GPIO controllers may consist of a single data register or a
>> pair
>> + of set/clear-bit registers. Such controllers are common for glue logic in
>> + FPGAs or ASICs. Commonly, these controllers are accessed over
>> memory-mapped
>> + NAND-style parallel busses.
>> +
>> +properties:
>> + big-endian: true
>> +
>> + compatible:
>
> Keep compatible as first property.
I thought it was alphabetical.
>> +enum:
>> + - brcm,bcm6345-gpio # Broadcom BCM6345 GPIO controller
>> + - wd,mbl-gpio # Western Digital MyBook Live memory-mapped GPIO
>> controller
>> + - ni,169445-nand-gpio # National Instruments 169445 GPIO NAND
>> controller
>
> I think you got comment that these comments are making things
> unreadable. I don't see here improvement.
That was not the comment I got.
| I think you can inline description: statements in the enum instead of
| the # hash comments, however IIRC you have to use oneOf and
| const: to do it, like I do in
| Documentation/devicetree/bindings/input/touchscreen/cypress,cy8ctma340.yaml
| but don't overinvest in this if it is cumbersome.
I investigated this and determined it was cumbersome.
> For example first comment is useless - you say the same as compatible.
> Same with last one. So only remaining WD comment should be made in new
> line so everything is nicely readable.
I don't understand what you mean by "made in new line". Anyway, I will
leave just the WD comment.
> BTW, order the enum by name.
OK
>> +
>> + '#gpio-cells':
>> +const: 2
>> +
>> + gpio-controller:
>> +true
>
> I am sure I saw comments here...
>
> https://lore.kernel.org/all/20230308231018.ga4039466-r...@kernel.org/
OK
>> +
>> + reg:
>> +minItems: 1
>> +description:
>> + A list of registers in the controller. The width of each register is
>> + determined by its size.
>
> I don't understand this comment. Aren't you describing now what 'reg' is
> in DT spec? If so, drop. If not, please share more.
Each register describes exactly one hardware register. In some other
device, when you see `regs = <0x800 0x100>`, then you may have 64
32-bit registers. But for this device, it would be one 2048-bit
register.
>> All registers must have the same width. The number
>> + of GPIOs is set by the width, with bit 0 corresponding to GPIO 0.
>> +items:
>> + - description:
>> + Register to READ the value of the GPIO lines. If GPIO line is
>> high,
>> + the bit will be set. If the GPIO line is low, the bit will be
>> cleared.
>> + This register may also be used to drive GPIOs if the SET register
>> is
>> + omitted.
>> + - description:
>> + Register to SET the value of the GPIO lines. Setting a bit in this
>> + register will drive the GPIO
Re: [PATCH v11 03/13] dt-bindings: Convert gpio-mmio to yaml
On 13/03/2023 17:11, Sean Anderson wrote:
> This is a generic binding for simple MMIO GPIO controllers. Although we
> have a single driver for these controllers, they were previously spread
> over several files. Consolidate them. The register descriptions are
> adapted from the comments in the source. There is no set order for the
> registers, so I have not specified one.
>
> Rename brcm,bcm6345-gpio to brcm,bcm63xx-gpio to reflect that bcm6345
> has moved.
>
> Signed-off-by: Sean Anderson
> Reviewed-by: Linus Walleij
> ---
> Linus or Bartosz, feel free to pick this up as the rest of this series
> may not be merged any time soon.
>
> Changes in v11:
> - Keep empty (or almost-empty) properties on a single line
> - Don't use | unnecessarily
> - Use gpio as the node name for examples
> - Rename brcm,bcm6345-gpio.yaml to brcm,bcm63xx-gpio.yaml
>
> Changes in v10:
> - New
>
> ...m6345-gpio.yaml => brcm,bcm63xx-gpio.yaml} | 16 +--
> .../devicetree/bindings/gpio/gpio-mmio.yaml | 134 ++
> .../bindings/gpio/ni,169445-nand-gpio.txt | 38 -
> .../devicetree/bindings/gpio/wd,mbl-gpio.txt | 38 -
> 4 files changed, 135 insertions(+), 91 deletions(-)
> rename Documentation/devicetree/bindings/gpio/{brcm,bcm6345-gpio.yaml =>
> brcm,bcm63xx-gpio.yaml} (78%)
> create mode 100644 Documentation/devicetree/bindings/gpio/gpio-mmio.yaml
> delete mode 100644
> Documentation/devicetree/bindings/gpio/ni,169445-nand-gpio.txt
> delete mode 100644 Documentation/devicetree/bindings/gpio/wd,mbl-gpio.txt
>
> diff --git a/Documentation/devicetree/bindings/gpio/brcm,bcm6345-gpio.yaml
> b/Documentation/devicetree/bindings/gpio/brcm,bcm63xx-gpio.yaml
> similarity index 78%
> rename from Documentation/devicetree/bindings/gpio/brcm,bcm6345-gpio.yaml
> rename to Documentation/devicetree/bindings/gpio/brcm,bcm63xx-gpio.yaml
> index 4d69f79df859..e11f4af49c52 100644
> --- a/Documentation/devicetree/bindings/gpio/brcm,bcm6345-gpio.yaml
> +++ b/Documentation/devicetree/bindings/gpio/brcm,bcm63xx-gpio.yaml
> +
> +description:
> + Some simple GPIO controllers may consist of a single data register or a
> pair
> + of set/clear-bit registers. Such controllers are common for glue logic in
> + FPGAs or ASICs. Commonly, these controllers are accessed over memory-mapped
> + NAND-style parallel busses.
> +
> +properties:
> + big-endian: true
> +
> + compatible:
Keep compatible as first property.
> +enum:
> + - brcm,bcm6345-gpio # Broadcom BCM6345 GPIO controller
> + - wd,mbl-gpio # Western Digital MyBook Live memory-mapped GPIO
> controller
> + - ni,169445-nand-gpio # National Instruments 169445 GPIO NAND
> controller
I think you got comment that these comments are making things
unreadable. I don't see here improvement.
For example first comment is useless - you say the same as compatible.
Same with last one. So only remaining WD comment should be made in new
line so everything is nicely readable.
BTW, order the enum by name.
> +
> + '#gpio-cells':
> +const: 2
> +
> + gpio-controller:
> +true
I am sure I saw comments here...
https://lore.kernel.org/all/20230308231018.ga4039466-r...@kernel.org/
> +
> + reg:
> +minItems: 1
> +description:
> + A list of registers in the controller. The width of each register is
> + determined by its size.
I don't understand this comment. Aren't you describing now what 'reg' is
in DT spec? If so, drop. If not, please share more.
> All registers must have the same width. The number
> + of GPIOs is set by the width, with bit 0 corresponding to GPIO 0.
> +items:
> + - description:
> + Register to READ the value of the GPIO lines. If GPIO line is high,
> + the bit will be set. If the GPIO line is low, the bit will be
> cleared.
> + This register may also be used to drive GPIOs if the SET register
> is
> + omitted.
> + - description:
> + Register to SET the value of the GPIO lines. Setting a bit in this
> + register will drive the GPIO line high.
> + - description:
> + Register to CLEAR the value of the GPIO lines. Setting a bit in
> this
> + register will drive the GPIO line low. If this register is omitted,
> + the SET register will be used to clear the GPIO lines as well, by
> + actively writing the line with 0.
> + - description:
> + Register to set the line as OUTPUT. Setting a bit in this register
> + will turn that line into an output line. Conversely, clearing a bit
> + will turn that line into an input.
> + - description:
> + Register to set this line as INPUT. Setting a bit in this register
> + will turn that line into an input line. Conversely, clearing a bit
> + will turn that line into an output.
> +
> + reg-names:
> +minItems: 1
> +maxItems: 5
> +items:
> + enum:
Why this is in any order? Other
[RFC PATCH v1] powerpc: Add version to install filenames
Rather than replacing the versionless vmlinux and System.map files,
copy to files with the version info appended.
Additionally, since executing the script is a last resort option,
inform the user about the missing `installkernel` command and the
location of the installation.
This work is adapted from `arch/s390/boot/install.sh`.
Signed-off-by: Nick Child
---
Hoping I am not breaking someones dependency on targeting /boot/vmlinux
so RFC'ing.
I typically have kernelinstall on my LPARs and installing and rebooting
goes peacefully.
Recently, I did not have kernelinstall and `make install` seemed to behave
differently. I got very little output but a succeful return code. After
initramfs issues during boot I dug into the makefiles a bit to figure out
where execution was differing. When `kernelinstall` cannot be found, we
invoke `arch/powerpc/boot/install.sh` instead. I am primarily interested
in getting more information relayed to the user about what is going on.
The changes to installing with the version appended are more of an afterthought
that makes sense to me but could understand why someone may depend on consistent
filenames.
Opening as RFC for opinions/rejections/concerns.
Thanks!
arch/powerpc/boot/install.sh | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/boot/install.sh b/arch/powerpc/boot/install.sh
index 461902c8a46d..101fcb397a0f 100755
--- a/arch/powerpc/boot/install.sh
+++ b/arch/powerpc/boot/install.sh
@@ -21,13 +21,17 @@ set -e
# this should work for both the pSeries zImage and the iSeries vmlinux.sm
image_name=`basename $2`
-if [ -f $4/$image_name ]; then
- mv $4/$image_name $4/$image_name.old
+
+echo "Warning: '${INSTALLKERNEL}' command not available... Copying" \
+ "directly to $4/$image_name-$1" >&2
+
+if [ -f $4/$image_name-$1 ]; then
+ mv $4/$image_name-$1 $4/$image_name-$1.old
fi
-if [ -f $4/System.map ]; then
- mv $4/System.map $4/System.old
+if [ -f $4/System.map-$1 ]; then
+ mv $4/System.map-$1 $4/System-$1.old
fi
-cat $2 > $4/$image_name
-cp $3 $4/System.map
+cat $2 > $4/$image_name-$1
+cp $3 $4/System.map-$1
--
2.31.1
Re: [PATCH v3 01/38] Kconfig: introduce HAS_IOPORT option and select it as necessary
On Tue, 2023-03-14 at 13:11 +0100, Niklas Schnelle wrote: > We introduce a new HAS_IOPORT Kconfig option to indicate support for I/O > Port access. In a future patch HAS_IOPORT=n will disable compilation of > the I/O accessor functions inb()/outb() and friends on architectures > which can not meaningfully support legacy I/O spaces such as s390. Also > add dependencies on HAS_IOPORT for the ISA and HAVE_EISA config options > as these busses always go along with HAS_IOPORT. > > The "depends on" relations on HAS_IOPORT in drivers as well as ifdefs > for HAS_IOPORT specific sections will be added in subsequent patches on > a per subsystem basis. > > Co-developed-by: Arnd Bergmann > Signed-off-by: Niklas Schnelle > --- > @Arnd, I swear I asked you and then added Signed-off-bys for all these Co-developed-bys as suggested by checkpatch. Sadly that must have been during my failed attempt of converting to b4 prep / b4 send before sending this last Friday and then it got lost. It almost worked and is a very nice work flow except that b4 currently can only use a single list of To/Cc fields and for this treewide series that would probably hit mail server limits. Added it now. Thanks, Niklas
Re: [PATCH v3 01/38] Kconfig: introduce HAS_IOPORT option and select it as necessary
On Tue, 2023-03-14 at 14:29 +0100, Arnd Bergmann wrote: > On Tue, Mar 14, 2023, at 13:11, Niklas Schnelle wrote: > > We introduce a new HAS_IOPORT Kconfig option to indicate support for I/O > > Port access. In a future patch HAS_IOPORT=n will disable compilation of > > the I/O accessor functions inb()/outb() and friends on architectures > > which can not meaningfully support legacy I/O spaces such as s390. Also > > add dependencies on HAS_IOPORT for the ISA and HAVE_EISA config options > > as these busses always go along with HAS_IOPORT. > > > > The "depends on" relations on HAS_IOPORT in drivers as well as ifdefs > > for HAS_IOPORT specific sections will be added in subsequent patches on > > a per subsystem basis. > > I think it would be helpful to enumerate which architectures > do not get HAS_IOPORT added, as they will be affected more. > > > Co-developed-by: Arnd Bergmann > > Signed-off-by: Niklas Schnelle > > If there are no objections, I could send this first patch for the > asm-generic tree as a preparation for 6.3, so we are able to merge > the other patches through subsystem maintainer tree for 6.4. > > arch/loongarch/ will now also need to select HAS_IOPORT > uncontitionally, this architecture was added after you > sent v2. Ah right. Added "select HAS_IOPORT" for LoongArch. > > > diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig > > index a98940e64243..5eeacc72e4da 100644 > > --- a/arch/parisc/Kconfig > > +++ b/arch/parisc/Kconfig > > @@ -47,6 +47,7 @@ config PARISC > > select MODULES_USE_ELF_RELA > > select CLONE_BACKWARDS > > select TTY # Needed for pdc_cons.c > > + select HAS_IOPORT if PCI > > It's also needed for EISA and I think you should select it > from CONFIG_GSC in drivers/parisc/Kconfig for this purpose. > > This could also be 'select HAS_IOPORT if PCI || EISA', but > that would require removing the 'depends on HAS_IOPORT' > under drivers/eisa/. I did use "select HAS_IOPORT if PCI || ISA || ATARI_ROM_ISA" in m68k so I think ideally we would handle both in the same way. I don't have a strong preference but I think the "select HAS_IOPORT if ..." puts it all in a single place which is nice. Also I think this would make it more similar architectures with unconditional HAS_IOPORT that thus don't need "depends on HAS_IOPORT" in their "config ISA" either. As also pointed by your comment below for x86. So will try to go this route. > > > select HAVE_DEBUG_STACKOVERFLOW > > select HAVE_ARCH_AUDITSYSCALL > > select HAVE_ARCH_HASH > > @@ -131,6 +132,7 @@ config STACKTRACE_SUPPORT > > > > config ISA_DMA_API > > bool > > + depends on HAS_IOPORT > > > > This line is not really needed since there is no way to > enable ISA_DMA_API. Removed > > > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig > > index a6c4407d3ec8..f7de646c074a 100644 > > --- a/arch/powerpc/Kconfig > > +++ b/arch/powerpc/Kconfig > > @@ -188,6 +188,7 @@ config PPC > > select GENERIC_SMP_IDLE_THREAD > > select GENERIC_TIME_VSYSCALL > > select GENERIC_VDSO_TIME_NS > > + select HAS_IOPORT if PCI > > select HAVE_ARCH_AUDITSYSCALL > > select HAVE_ARCH_HUGE_VMALLOC if HAVE_ARCH_HUGE_VMAP > > select HAVE_ARCH_HUGE_VMAP if PPC_RADIX_MMU || PPC_8xx > > @@ -1070,7 +1071,6 @@ menu "Bus options" > > > > config ISA > > bool "Support for ISA-bus hardware" > > - depends on PPC_CHRP > > select PPC_I8259 > > help > > Find out whether you have ISA slots on your motherboard. ISA is the > > This line looks wrong, I think we should keep that dependency. > Did you get a circular dependency if you leave it in? I don't recall why this was removed. I guess it happened when I was experimenting with adding "depends on HAS_IOPORT" for the ISA config options but that ultimately lead to circular dependencies, must have messed up when removing this here. > > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig > > index a825bf031f49..634dd42532f3 100644 > > --- a/arch/x86/Kconfig > > +++ b/arch/x86/Kconfig > > @@ -162,6 +162,7 @@ config X86 > > select GUP_GET_PXX_LOW_HIGH if X86_PAE > > select HARDIRQS_SW_RESEND > > select HARDLOCKUP_CHECK_TIMESTAMP if X86_64 > > + select HAS_IOPORT > > select HAVE_ACPI_APEI if ACPI > > select HAVE_ACPI_APEI_NMI if ACPI > > select HAVE_ALIGNED_STRUCT_PAGE if SLUB > > @@ -2893,6 +2894,7 @@ if X86_32 > > > > config ISA > > bool "ISA support" > > + depends on HAS_IOPORT > > help > > HAS_IOPORT is selected unconditionally already, so this doesn't > really do anything. Removed. > > > diff --git a/lib/Kconfig.kgdb b/lib/Kconfig.kgdb > > index 3b9a44008433..c68e4d9dcecb 100644 > > --- a/lib/Kconfig.kgdb > > +++ b/lib/Kconfig.kgdb > > @@ -121,7 +121,8 @@ config KDB_DEFAULT_ENABLE > > > > config KDB_KEYBOARD > > bool "KGDB_KDB: keyboard as input device" > > - depends on VT
Re: [PATCH] modpost: support arbitrary symbol length in modversion
On Tue, Mar 14, 2023 at 03:38:24PM +0100, Andrea Righi wrote:
> On Mon, Mar 13, 2023 at 11:09:31PM +0100, Andrea Righi wrote:
> > On Mon, Mar 13, 2023 at 11:02:34PM +0100, Michal Suchánek wrote:
> > > On Mon, Mar 13, 2023 at 10:53:34PM +0100, Andrea Righi wrote:
> > > > On Mon, Mar 13, 2023 at 10:48:53PM +0100, Michal Suchánek wrote:
> > > > > Hello,
> > > > >
> > > > > On Mon, Mar 13, 2023 at 09:32:16PM +0100, Andrea Righi wrote:
> > > > > > On Wed, Jan 11, 2023 at 04:11:51PM +, Gary Guo wrote:
> > > > > > > Currently modversion uses a fixed size array of size (64 -
> > > > > > > sizeof(long))
> > > > > > > to store symbol names, thus placing a hard limit on length of
> > > > > > > symbols.
> > > > > > > Rust symbols (which encodes crate and module names) can be quite
> > > > > > > a bit
> > > > > > > longer. The length limit in kallsyms is increased to 512 for this
> > > > > > > reason.
> > > > > > >
> > > > > > > It's a waste of space to simply expand the fixed array size to
> > > > > > > 512 in
> > > > > > > modversion info entries. I therefore make it variably sized, with
> > > > > > > offset
> > > > > > > to the next entry indicated by the initial "next" field.
> > > > > > >
> > > > > > > In addition to supporting longer-than-56/60 byte symbols, this
> > > > > > > patch also
> > > > > > > reduce the size for short symbols by getting rid of excessive 0
> > > > > > > paddings.
> > > > > > > There are still some zero paddings to ensure "next" and "crc"
> > > > > > > fields are
> > > > > > > properly aligned.
> > > > > > >
> > > > > > > This patch does have a tiny drawback that it makes ".mod.c" files
> > > > > > > generated
> > > > > > > a bit less easy to read, as code like
> > > > > > >
> > > > > > > "\x08\x00\x00\x00\x78\x56\x34\x12"
> > > > > > > "symbol\0\0"
> > > > > > >
> > > > > > > is generated as opposed to
> > > > > > >
> > > > > > > { 0x12345678, "symbol" },
> > > > > > >
> > > > > > > because the structure is now variable-length. But hopefully
> > > > > > > nobody reads
> > > > > > > the generated file :)
> > > > > > >
> > > > > > > Link: b8a94bfb3395 ("kallsyms: increase maximum kernel symbol
> > > > > > > length to 512")
> > > > > > > Link: https://github.com/Rust-for-Linux/linux/pull/379
> > > > > > >
> > > > > > > Signed-off-by: Gary Guo
> > > > > >
> > > > > > Is there any newer version of this patch?
> > > > > >
> > > > > > I'm doing some tests with it, but I'm getting boot failures on ppc64
> > > > > > with this applied (at boot kernel is spitting out lots of oops'es
> > > > > > and
> > > > > > unfortunately it's really hard to copy paste or just read them from
> > > > > > the
> > > > > > console).
> > > > >
> > > > > Are you using the ELF ABI v1 or v2?
> > > > >
> > > > > v1 may have some additional issues when it comes to these symbol
> > > > > tables.
> > > > >
> > > > > Thanks
> > > > >
> > > > > Michal
> > > >
> > > > I have CONFIG_PPC64_ELF_ABI_V2=y in my .config, so I guess I'm using v2.
> > > >
> > > > BTW, the issue seems to be in dedotify_versions(), as a silly test I
> > > > tried to comment out this function completely to be a no-op and now my
> > > > system boots fine (but I guess I'm probably breaking something else).
> > >
> > > Probably not. You should not have the extra leading dot on ABI v2. So if
> > > dedotify does something that means something generates and then expects
> > > back symbols with a leading dot, and this workaround for ABI v1 breaks
> > > that. Or maybe it is called when it shouldn't.
> >
> > Hm.. I'll add some debugging to this function to see what happens exactly.
>
> Alright I've done more tests across different architectures. My problem
> with ppc64 is that this architecture is evaluating sechdrs[i].sh_size
> using get_stubs_size(), that apparently can add some extra padding, so
> doing (vers + vers->next < end) isn't a reliable check to determine the
> end of the variable array, because sometimes "end" can be greater than
> the last "vers + vers->next" entry.
>
> In general I think it'd be more reliable to add a dummy NULL entry at
> the end of the modversion array.
>
> Moreover, I think we also need to enforce struct modversion_info to be
> __packed, just to make sure that no extra padding is added (otherwise it
> may break our logic to determine the offset of the next entry).
>
> > @@ -2062,16 +2066,25 @@ static void add_versions(struct buffer *b, struct
> > module *mod)
> > s->name, mod->name);
> > continue;
> > }
> > - if (strlen(s->name) >= MODULE_NAME_LEN) {
> > - error("too long symbol \"%s\" [%s.ko]\n",
> > - s->name, mod->name);
> > - break;
> > - }
> > - buf_printf(b, "\t{ %#8x, \"%s\" },\n",
> > - s->crc, s->name);
> > + name_len = strlen(s->name);
> > + name_len_padded = (name_len + 1 + 3) &
Re: [PATCH] modpost: support arbitrary symbol length in modversion
On Mon, Mar 13, 2023 at 11:09:31PM +0100, Andrea Righi wrote:
> On Mon, Mar 13, 2023 at 11:02:34PM +0100, Michal Suchánek wrote:
> > On Mon, Mar 13, 2023 at 10:53:34PM +0100, Andrea Righi wrote:
> > > On Mon, Mar 13, 2023 at 10:48:53PM +0100, Michal Suchánek wrote:
> > > > Hello,
> > > >
> > > > On Mon, Mar 13, 2023 at 09:32:16PM +0100, Andrea Righi wrote:
> > > > > On Wed, Jan 11, 2023 at 04:11:51PM +, Gary Guo wrote:
> > > > > > Currently modversion uses a fixed size array of size (64 -
> > > > > > sizeof(long))
> > > > > > to store symbol names, thus placing a hard limit on length of
> > > > > > symbols.
> > > > > > Rust symbols (which encodes crate and module names) can be quite a
> > > > > > bit
> > > > > > longer. The length limit in kallsyms is increased to 512 for this
> > > > > > reason.
> > > > > >
> > > > > > It's a waste of space to simply expand the fixed array size to 512
> > > > > > in
> > > > > > modversion info entries. I therefore make it variably sized, with
> > > > > > offset
> > > > > > to the next entry indicated by the initial "next" field.
> > > > > >
> > > > > > In addition to supporting longer-than-56/60 byte symbols, this
> > > > > > patch also
> > > > > > reduce the size for short symbols by getting rid of excessive 0
> > > > > > paddings.
> > > > > > There are still some zero paddings to ensure "next" and "crc"
> > > > > > fields are
> > > > > > properly aligned.
> > > > > >
> > > > > > This patch does have a tiny drawback that it makes ".mod.c" files
> > > > > > generated
> > > > > > a bit less easy to read, as code like
> > > > > >
> > > > > > "\x08\x00\x00\x00\x78\x56\x34\x12"
> > > > > > "symbol\0\0"
> > > > > >
> > > > > > is generated as opposed to
> > > > > >
> > > > > > { 0x12345678, "symbol" },
> > > > > >
> > > > > > because the structure is now variable-length. But hopefully nobody
> > > > > > reads
> > > > > > the generated file :)
> > > > > >
> > > > > > Link: b8a94bfb3395 ("kallsyms: increase maximum kernel symbol
> > > > > > length to 512")
> > > > > > Link: https://github.com/Rust-for-Linux/linux/pull/379
> > > > > >
> > > > > > Signed-off-by: Gary Guo
> > > > >
> > > > > Is there any newer version of this patch?
> > > > >
> > > > > I'm doing some tests with it, but I'm getting boot failures on ppc64
> > > > > with this applied (at boot kernel is spitting out lots of oops'es and
> > > > > unfortunately it's really hard to copy paste or just read them from
> > > > > the
> > > > > console).
> > > >
> > > > Are you using the ELF ABI v1 or v2?
> > > >
> > > > v1 may have some additional issues when it comes to these symbol tables.
> > > >
> > > > Thanks
> > > >
> > > > Michal
> > >
> > > I have CONFIG_PPC64_ELF_ABI_V2=y in my .config, so I guess I'm using v2.
> > >
> > > BTW, the issue seems to be in dedotify_versions(), as a silly test I
> > > tried to comment out this function completely to be a no-op and now my
> > > system boots fine (but I guess I'm probably breaking something else).
> >
> > Probably not. You should not have the extra leading dot on ABI v2. So if
> > dedotify does something that means something generates and then expects
> > back symbols with a leading dot, and this workaround for ABI v1 breaks
> > that. Or maybe it is called when it shouldn't.
>
> Hm.. I'll add some debugging to this function to see what happens exactly.
Alright I've done more tests across different architectures. My problem
with ppc64 is that this architecture is evaluating sechdrs[i].sh_size
using get_stubs_size(), that apparently can add some extra padding, so
doing (vers + vers->next < end) isn't a reliable check to determine the
end of the variable array, because sometimes "end" can be greater than
the last "vers + vers->next" entry.
In general I think it'd be more reliable to add a dummy NULL entry at
the end of the modversion array.
Moreover, I think we also need to enforce struct modversion_info to be
__packed, just to make sure that no extra padding is added (otherwise it
may break our logic to determine the offset of the next entry).
> @@ -2062,16 +2066,25 @@ static void add_versions(struct buffer *b, struct
> module *mod)
> s->name, mod->name);
> continue;
> }
> - if (strlen(s->name) >= MODULE_NAME_LEN) {
> - error("too long symbol \"%s\" [%s.ko]\n",
> - s->name, mod->name);
> - break;
> - }
> - buf_printf(b, "\t{ %#8x, \"%s\" },\n",
> -s->crc, s->name);
> + name_len = strlen(s->name);
> + name_len_padded = (name_len + 1 + 3) & ~3;
> +
> + /* Offset to next entry */
> + tmp = TO_NATIVE(8 + name_len_padded);
^ Here's another issue that I found, you can't use TO_NATIVE() in this
way, some compilers are complaining (like on s390x this doesn't build).
So we need to do something
Re: [PATCH v3] vdso: Improve cmd_vdso_check to check all dynamic relocations
Fangrui Song writes: > The actual intention is that no dynamic relocation exists. However, some > GNU ld ports produce unneeded R_*_NONE. (If a port fails to determine > the exact .rel[a].dyn size, the trailing zeros become R_*_NONE > relocations. E.g. ld's powerpc port recently fixed > https://sourceware.org/bugzilla/show_bug.cgi?id=29540) R_*_NONE are > generally no-op in the dynamic loaders. So just ignore them. > > With the change, we can remove ARCH_REL_TYPE_ABS. ARCH_REL_TYPE_ABS is a > bit misnomer as ports may check RELAVETIVE/GLOB_DAT/JUMP_SLOT which are > not called "absolute relocations". (The patch is motivated by the arm64 > port missing R_AARCH64_RELATIVE.) > > Signed-off-by: Fangrui Song > Reviewed-by: Christophe Leroy > Reviewed-by: Vincenzo Frascino # for vDSO, aarch64 > Tested-by: Vincenzo Frascino # for aarch64 > --- > Changes from v2: > * rebase > > Changes from v3: > * Add a comment before `include $(srctree)/lib/vdso/Makefile` in every > touched arch Makefile > --- > arch/arm/vdso/Makefile| 4 +--- > arch/arm64/kernel/vdso/Makefile | 4 +--- > arch/arm64/kernel/vdso32/Makefile | 3 --- > arch/csky/kernel/vdso/Makefile| 4 +--- > arch/loongarch/vdso/Makefile | 4 +--- > arch/mips/vdso/Makefile | 4 +--- > arch/powerpc/kernel/vdso/Makefile | 2 +- Acked-by: Michael Ellerman (powerpc) cheers
Re: [PATCH v3 01/38] Kconfig: introduce HAS_IOPORT option and select it as necessary
On Tue, Mar 14, 2023, at 13:11, Niklas Schnelle wrote: > We introduce a new HAS_IOPORT Kconfig option to indicate support for I/O > Port access. In a future patch HAS_IOPORT=n will disable compilation of > the I/O accessor functions inb()/outb() and friends on architectures > which can not meaningfully support legacy I/O spaces such as s390. Also > add dependencies on HAS_IOPORT for the ISA and HAVE_EISA config options > as these busses always go along with HAS_IOPORT. > > The "depends on" relations on HAS_IOPORT in drivers as well as ifdefs > for HAS_IOPORT specific sections will be added in subsequent patches on > a per subsystem basis. I think it would be helpful to enumerate which architectures do not get HAS_IOPORT added, as they will be affected more. > Co-developed-by: Arnd Bergmann > Signed-off-by: Niklas Schnelle If there are no objections, I could send this first patch for the asm-generic tree as a preparation for 6.3, so we are able to merge the other patches through subsystem maintainer tree for 6.4. arch/loongarch/ will now also need to select HAS_IOPORT uncontitionally, this architecture was added after you sent v2. > diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig > index a98940e64243..5eeacc72e4da 100644 > --- a/arch/parisc/Kconfig > +++ b/arch/parisc/Kconfig > @@ -47,6 +47,7 @@ config PARISC > select MODULES_USE_ELF_RELA > select CLONE_BACKWARDS > select TTY # Needed for pdc_cons.c > + select HAS_IOPORT if PCI It's also needed for EISA and I think you should select it from CONFIG_GSC in drivers/parisc/Kconfig for this purpose. This could also be 'select HAS_IOPORT if PCI || EISA', but that would require removing the 'depends on HAS_IOPORT' under drivers/eisa/. > select HAVE_DEBUG_STACKOVERFLOW > select HAVE_ARCH_AUDITSYSCALL > select HAVE_ARCH_HASH > @@ -131,6 +132,7 @@ config STACKTRACE_SUPPORT > > config ISA_DMA_API > bool > + depends on HAS_IOPORT > This line is not really needed since there is no way to enable ISA_DMA_API. > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig > index a6c4407d3ec8..f7de646c074a 100644 > --- a/arch/powerpc/Kconfig > +++ b/arch/powerpc/Kconfig > @@ -188,6 +188,7 @@ config PPC > select GENERIC_SMP_IDLE_THREAD > select GENERIC_TIME_VSYSCALL > select GENERIC_VDSO_TIME_NS > + select HAS_IOPORT if PCI > select HAVE_ARCH_AUDITSYSCALL > select HAVE_ARCH_HUGE_VMALLOC if HAVE_ARCH_HUGE_VMAP > select HAVE_ARCH_HUGE_VMAP if PPC_RADIX_MMU || PPC_8xx > @@ -1070,7 +1071,6 @@ menu "Bus options" > > config ISA > bool "Support for ISA-bus hardware" > - depends on PPC_CHRP > select PPC_I8259 > help > Find out whether you have ISA slots on your motherboard. ISA is the This line looks wrong, I think we should keep that dependency. Did you get a circular dependency if you leave it in? > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig > index a825bf031f49..634dd42532f3 100644 > --- a/arch/x86/Kconfig > +++ b/arch/x86/Kconfig > @@ -162,6 +162,7 @@ config X86 > select GUP_GET_PXX_LOW_HIGH if X86_PAE > select HARDIRQS_SW_RESEND > select HARDLOCKUP_CHECK_TIMESTAMP if X86_64 > + select HAS_IOPORT > select HAVE_ACPI_APEI if ACPI > select HAVE_ACPI_APEI_NMI if ACPI > select HAVE_ALIGNED_STRUCT_PAGE if SLUB > @@ -2893,6 +2894,7 @@ if X86_32 > > config ISA > bool "ISA support" > + depends on HAS_IOPORT > help HAS_IOPORT is selected unconditionally already, so this doesn't really do anything. > diff --git a/lib/Kconfig.kgdb b/lib/Kconfig.kgdb > index 3b9a44008433..c68e4d9dcecb 100644 > --- a/lib/Kconfig.kgdb > +++ b/lib/Kconfig.kgdb > @@ -121,7 +121,8 @@ config KDB_DEFAULT_ENABLE > > config KDB_KEYBOARD > bool "KGDB_KDB: keyboard as input device" > - depends on VT && KGDB_KDB && !PARISC > + depends on HAS_IOPORT > + depends on VT && KGDB_KDB > default n This loses the !PARISC dependency, which I don't think is intentional. The added HAS_IOPORT dependency makes sense here, but I think this should be in a different patch and not in the preparation. Arnd
Re: [PATCH v3 01/38] Kconfig: introduce HAS_IOPORT option and select it as necessary
On Tue, 2023-03-14 at 13:11 +0100, Niklas Schnelle wrote: > --- a/arch/um/Kconfig > +++ b/arch/um/Kconfig > @@ -56,6 +56,7 @@ config NO_IOPORT_MAP > > config ISA > bool > + depends on HAS_IOPORT > config ISA here is already unselectable, and nothing ever does "select ISA" (only in some other architectures), so is there much point in this? I'm not even sure why this exists at all. But anyway, adding a dependency to a always-false symbol doesn't make it less always-false :-) Acked-by: Johannes Berg # for ARCH=um Certainly will be nice to get rid of this cruft for architectures that don't have it. johannes
Re: [PATCH v3 01/38] Kconfig: introduce HAS_IOPORT option and select it as necessary
On Tue, Mar 14, 2023 at 1:13 PM Niklas Schnelle wrote:
> We introduce a new HAS_IOPORT Kconfig option to indicate support for I/O
> Port access. In a future patch HAS_IOPORT=n will disable compilation of
> the I/O accessor functions inb()/outb() and friends on architectures
> which can not meaningfully support legacy I/O spaces such as s390. Also
> add dependencies on HAS_IOPORT for the ISA and HAVE_EISA config options
> as these busses always go along with HAS_IOPORT.
>
> The "depends on" relations on HAS_IOPORT in drivers as well as ifdefs
> for HAS_IOPORT specific sections will be added in subsequent patches on
> a per subsystem basis.
>
> Co-developed-by: Arnd Bergmann
> Signed-off-by: Niklas Schnelle
> arch/m68k/Kconfig | 1 +
Acked-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[PATCH v3 01/38] Kconfig: introduce HAS_IOPORT option and select it as necessary
We introduce a new HAS_IOPORT Kconfig option to indicate support for I/O Port access. In a future patch HAS_IOPORT=n will disable compilation of the I/O accessor functions inb()/outb() and friends on architectures which can not meaningfully support legacy I/O spaces such as s390. Also add dependencies on HAS_IOPORT for the ISA and HAVE_EISA config options as these busses always go along with HAS_IOPORT. The "depends on" relations on HAS_IOPORT in drivers as well as ifdefs for HAS_IOPORT specific sections will be added in subsequent patches on a per subsystem basis. Co-developed-by: Arnd Bergmann Signed-off-by: Niklas Schnelle --- arch/alpha/Kconfig | 1 + arch/arm/Kconfig| 1 + arch/arm64/Kconfig | 1 + arch/ia64/Kconfig | 1 + arch/m68k/Kconfig | 1 + arch/microblaze/Kconfig | 1 + arch/mips/Kconfig | 2 ++ arch/parisc/Kconfig | 2 ++ arch/powerpc/Kconfig| 2 +- arch/riscv/Kconfig | 1 + arch/sh/Kconfig | 1 + arch/sparc/Kconfig | 1 + arch/um/Kconfig | 1 + arch/x86/Kconfig| 2 ++ drivers/bus/Kconfig | 2 +- drivers/eisa/Kconfig| 1 + lib/Kconfig | 4 lib/Kconfig.kgdb| 3 ++- 18 files changed, 25 insertions(+), 3 deletions(-) diff --git a/arch/alpha/Kconfig b/arch/alpha/Kconfig index 780d4673c3ca..a5c2b1aa46b0 100644 --- a/arch/alpha/Kconfig +++ b/arch/alpha/Kconfig @@ -27,6 +27,7 @@ config ALPHA select AUDIT_ARCH select GENERIC_CPU_VULNERABILITIES select GENERIC_SMP_IDLE_THREAD + select HAS_IOPORT select HAVE_ARCH_AUDITSYSCALL select HAVE_MOD_ARCH_SPECIFIC select MODULES_USE_ELF_RELA diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index e24a9820e12f..4acb5bc4b52a 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -70,6 +70,7 @@ config ARM select GENERIC_SCHED_CLOCK select GENERIC_SMP_IDLE_THREAD select HARDIRQS_SW_RESEND + select HAS_IOPORT select HAVE_ARCH_AUDITSYSCALL if AEABI && !OABI_COMPAT select HAVE_ARCH_BITREVERSE if (CPU_32v7M || CPU_32v7) && !CPU_32v6 select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL && !CPU_ENDIAN_BE32 && MMU diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 1023e896d46b..b740019c4aee 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -145,6 +145,7 @@ config ARM64 select GENERIC_GETTIMEOFDAY select GENERIC_VDSO_TIME_NS select HARDIRQS_SW_RESEND + select HAS_IOPORT select HAVE_MOVE_PMD select HAVE_MOVE_PUD select HAVE_PCI diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig index d7e4a24e8644..2e13ec8263b9 100644 --- a/arch/ia64/Kconfig +++ b/arch/ia64/Kconfig @@ -25,6 +25,7 @@ config IA64 select PCI_DOMAINS if PCI select PCI_MSI select PCI_SYSCALL if PCI + select HAS_IOPORT select HAVE_ASM_MODVERSIONS select HAVE_UNSTABLE_SCHED_CLOCK select HAVE_EXIT_THREAD diff --git a/arch/m68k/Kconfig b/arch/m68k/Kconfig index 82154952e574..40198a1ebe27 100644 --- a/arch/m68k/Kconfig +++ b/arch/m68k/Kconfig @@ -18,6 +18,7 @@ config M68K select GENERIC_CPU_DEVICES select GENERIC_IOMAP select GENERIC_IRQ_SHOW + select HAS_IOPORT if PCI || ISA || ATARI_ROM_ISA select HAVE_ARCH_SECCOMP select HAVE_ARCH_SECCOMP_FILTER select HAVE_ASM_MODVERSIONS diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig index cc88af6fa7a4..211f338d6235 100644 --- a/arch/microblaze/Kconfig +++ b/arch/microblaze/Kconfig @@ -21,6 +21,7 @@ config MICROBLAZE select GENERIC_IRQ_SHOW select GENERIC_PCI_IOMAP select GENERIC_SCHED_CLOCK + select HAS_IOPORT if PCI select HAVE_ARCH_HASH select HAVE_ARCH_KGDB select HAVE_ARCH_SECCOMP diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig index e2f3ca73f40d..64760fcd7b52 100644 --- a/arch/mips/Kconfig +++ b/arch/mips/Kconfig @@ -47,6 +47,7 @@ config MIPS select GENERIC_SMP_IDLE_THREAD select GENERIC_TIME_VSYSCALL select GUP_GET_PXX_LOW_HIGH if CPU_MIPS32 && PHYS_ADDR_T_64BIT + select HAS_IOPORT if !NO_IOPORT_MAP select HAVE_ARCH_COMPILER_H select HAVE_ARCH_JUMP_LABEL select HAVE_ARCH_KGDB if MIPS_FP_SUPPORT @@ -3120,6 +3121,7 @@ config PCI_DRIVERS_LEGACY # users to choose the right thing ... # config ISA + depends on HAS_IOPORT bool config TC diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig index a98940e64243..5eeacc72e4da 100644 --- a/arch/parisc/Kconfig +++ b/arch/parisc/Kconfig @@ -47,6 +47,7 @@ config PARISC select MODULES_USE_ELF_RELA select CLONE_BACKWARDS select TTY # Needed for pdc_cons.c + select HAS_IOPORT if PCI select HAVE_DEBUG_STACKOVERFLOW select HAVE_ARCH_AUDITSYSCALL select HAVE_ARCH_HASH @@ -131,6 +132,7 @@ config STACKTRACE_SUPPORT config ISA_DMA_API bool +
Re: [PATCH] crypto: p10-aes-gcm - remove duplicate include header
Herbert Xu writes:
> On Tue, Mar 14, 2023 at 08:47:30AM +, Christophe Leroy wrote:
>>
>> Any reason for resending ?
>
> The p10 patches were reverted, and have only just been re-instated.
Hmm. Seems none of them were ever Cc'ed to linuxppc-dev. So this is the
first I've seen of them.
We seem to have two almost identical copies of ppc-xlate.pl now :/
$ find . -name ppc-xlate.pl
./arch/powerpc/crypto/ppc-xlate.pl
./drivers/crypto/vmx/ppc-xlate.pl
And notably the new one doesn't have the changes from commit
505ea33089dc ("powerpc/64: Add big-endian ELFv2 flavour to crypto VMX
asm generation"), so that probably breaks the build for some configs.
There's also now two new .pl files with identical names, but different
content to the copies in drivers/crypto/vmx:
$ find . -name "*-ppc.pl" | xargs wc -l
370 ./arch/powerpc/crypto/ghashp8-ppc.pl
585 ./arch/powerpc/crypto/aesp8-ppc.pl
243 ./drivers/crypto/vmx/ghashp8-ppc.pl
3846 ./drivers/crypto/vmx/aesp8-ppc.pl
5044 total
Also PPC_MODULE_FEATURE_P10 should be in arch/powerpc/include/asm/cpufeature.h.
And CRYPTO_AES_GCM_P10 should not depend on POWER10_CPU, that restricts
it to being built when the kernel is built to run *only* on Power10,
which basically no one does, certainly no distro.
The code needs to detect at runtime if it's on Power10, and only
register itself if so.
cheers
Re: [PATCH] crypto: p10-aes-gcm - remove duplicate include header
On Tue, Mar 14, 2023 at 08:47:30AM +, Christophe Leroy wrote: > > Any reason for resending ? The p10 patches were reverted, and have only just been re-instated. Cheers, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] crypto: p10-aes-gcm - remove duplicate include header
Hi, Le 14/03/2023 à 09:31, ye.xingc...@zte.com.cn a écrit : > From: Ye Xingchen > > crypto/algapi.h is included more than once. > > Signed-off-by: Ye Xingchen You already sent this patch, see https://patchwork.ozlabs.org/project/linuxppc-dev/patch/202301171601080312...@zte.com.cn/ Any reason for resending ? Christophe > --- > arch/powerpc/crypto/aes-gcm-p10-glue.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/arch/powerpc/crypto/aes-gcm-p10-glue.c > b/arch/powerpc/crypto/aes-gcm-p10-glue.c > index c95f5b7cc456..1533c8cdd26f 100644 > --- a/arch/powerpc/crypto/aes-gcm-p10-glue.c > +++ b/arch/powerpc/crypto/aes-gcm-p10-glue.c > @@ -8,7 +8,6 @@ > #include > #include > #include > -#include > #include > #include > #include
[PATCH] crypto: p10-aes-gcm - remove duplicate include header
From: Ye Xingchen crypto/algapi.h is included more than once. Signed-off-by: Ye Xingchen --- arch/powerpc/crypto/aes-gcm-p10-glue.c | 1 - 1 file changed, 1 deletion(-) diff --git a/arch/powerpc/crypto/aes-gcm-p10-glue.c b/arch/powerpc/crypto/aes-gcm-p10-glue.c index c95f5b7cc456..1533c8cdd26f 100644 --- a/arch/powerpc/crypto/aes-gcm-p10-glue.c +++ b/arch/powerpc/crypto/aes-gcm-p10-glue.c @@ -8,7 +8,6 @@ #include #include #include -#include #include #include #include -- 2.25.1
Re: Question about the dependency on the config SOC_FSL in CPM_QMC
Hi Lukas, Mark,
On Tue, 14 Mar 2023 09:17:18 +0100
Lukas Bulwahn wrote:
> On Tue, Mar 14, 2023 at 8:57 AM Herve Codina wrote:
> >
> > Hi Lukas,
> >
> > On Tue, 14 Mar 2023 08:21:50 +0100
> > Lukas Bulwahn wrote:
> >
> > > Dear Herve,
> > >
> > > In your patch below, you added the config CPM_QMC which depends on the
> > > non-existing config SOC_FSL:
> > >
> > > https://lore.kernel.org/r/20230217145645.1768659-7-herve.cod...@bootlin.com
> > >
> > > Up to my knowledge, the config SOC_FSL never existed in the mainline
> > > tree. Is this dependency really required or can the expression simply
> > > be reduced to COMPILE_TEST and we drop the dependency to SOC_FSL?
> > >
> > > Note: This patch has now shown up in linux-next with commit
> > > 3178d58e0b97. Currently, it would not be possible to compile test this
> > > driver, as the dependency on SOC_FSL is never met.
> > >
> > >
> > > Best regards,
> > >
> > > Lukas
> >
> > My bad :(
> >
> > The dependency must be FSL_SOC instead of SOC_FSL.
>
> Herve, are you going to send a quick fix to your patch or ask Mark if
> your original patch can simply be replaced with a new one with this
> change added?
I have just sent a quick fix.
https://lore.kernel.org/linux-kernel/20230314082157.137176-1-herve.cod...@bootlin.com/
Mark, it can be squashed with
3178d58e0b97 ("soc: fsl: cpm1: Add support for QMC")
Regards
Hervé
>
> Lukas
>
> > I mean:
> > diff --git a/drivers/soc/fsl/qe/Kconfig b/drivers/soc/fsl/qe/Kconfig
> > index f90cfdf0c763..7268c2fbcbc1 100644
> > --- a/drivers/soc/fsl/qe/Kconfig
> > +++ b/drivers/soc/fsl/qe/Kconfig
> > @@ -47,7 +47,7 @@ config CPM_TSA
> > config CPM_QMC
> > tristate "CPM QMC support"
> > depends on OF && HAS_IOMEM
> > - depends on CPM1 || (SOC_FSL && COMPILE_TEST)
> > + depends on CPM1 || (FSL_SOC && COMPILE_TEST)
> > depends on CPM_TSA
> > help
> > Freescale CPM QUICC Multichannel Controller
> >
> >
> >
> > --
> > Hervé Codina, Bootlin
> > Embedded Linux and Kernel engineering
> > https://bootlin.com
[PATCH] soc: fsl: cpm1: qmc: Fix test dependency
The QMC depends on (SOC_FSL && COMPILE_TEST). SOC_FSL does not exist. Fix the dependency using the correct one: FSL_SOC. Signed-off-by: Herve Codina --- drivers/soc/fsl/qe/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/soc/fsl/qe/Kconfig b/drivers/soc/fsl/qe/Kconfig index f90cfdf0c763..7268c2fbcbc1 100644 --- a/drivers/soc/fsl/qe/Kconfig +++ b/drivers/soc/fsl/qe/Kconfig @@ -47,7 +47,7 @@ config CPM_TSA config CPM_QMC tristate "CPM QMC support" depends on OF && HAS_IOMEM - depends on CPM1 || (SOC_FSL && COMPILE_TEST) + depends on CPM1 || (FSL_SOC && COMPILE_TEST) depends on CPM_TSA help Freescale CPM QUICC Multichannel Controller -- 2.39.2
Re: Question about the dependency on the config SOC_FSL in CPM_QMC
On Tue, Mar 14, 2023 at 8:57 AM Herve Codina wrote: > > Hi Lukas, > > On Tue, 14 Mar 2023 08:21:50 +0100 > Lukas Bulwahn wrote: > > > Dear Herve, > > > > In your patch below, you added the config CPM_QMC which depends on the > > non-existing config SOC_FSL: > > > > https://lore.kernel.org/r/20230217145645.1768659-7-herve.cod...@bootlin.com > > > > Up to my knowledge, the config SOC_FSL never existed in the mainline > > tree. Is this dependency really required or can the expression simply > > be reduced to COMPILE_TEST and we drop the dependency to SOC_FSL? > > > > Note: This patch has now shown up in linux-next with commit > > 3178d58e0b97. Currently, it would not be possible to compile test this > > driver, as the dependency on SOC_FSL is never met. > > > > > > Best regards, > > > > Lukas > > My bad :( > > The dependency must be FSL_SOC instead of SOC_FSL. Herve, are you going to send a quick fix to your patch or ask Mark if your original patch can simply be replaced with a new one with this change added? Lukas > I mean: > diff --git a/drivers/soc/fsl/qe/Kconfig b/drivers/soc/fsl/qe/Kconfig > index f90cfdf0c763..7268c2fbcbc1 100644 > --- a/drivers/soc/fsl/qe/Kconfig > +++ b/drivers/soc/fsl/qe/Kconfig > @@ -47,7 +47,7 @@ config CPM_TSA > config CPM_QMC > tristate "CPM QMC support" > depends on OF && HAS_IOMEM > - depends on CPM1 || (SOC_FSL && COMPILE_TEST) > + depends on CPM1 || (FSL_SOC && COMPILE_TEST) > depends on CPM_TSA > help > Freescale CPM QUICC Multichannel Controller > > > > -- > Hervé Codina, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com
Re: Question about the dependency on the config SOC_FSL in CPM_QMC
Hi Lukas Le 14/03/2023 à 08:21, Lukas Bulwahn a écrit : > Dear Herve, > > In your patch below, you added the config CPM_QMC which depends on the > non-existing config SOC_FSL: > > https://lore.kernel.org/r/20230217145645.1768659-7-herve.cod...@bootlin.com > > Up to my knowledge, the config SOC_FSL never existed in the mainline > tree. Is this dependency really required or can the expression simply > be reduced to COMPILE_TEST and we drop the dependency to SOC_FSL? That's a mistake, should be FSL_SOC. See https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20230126083222.374243-7-herve.cod...@bootlin.com/#3058690 Christophe
Re: Question about the dependency on the config SOC_FSL in CPM_QMC
Hi Lukas, On Tue, 14 Mar 2023 08:21:50 +0100 Lukas Bulwahn wrote: > Dear Herve, > > In your patch below, you added the config CPM_QMC which depends on the > non-existing config SOC_FSL: > > https://lore.kernel.org/r/20230217145645.1768659-7-herve.cod...@bootlin.com > > Up to my knowledge, the config SOC_FSL never existed in the mainline > tree. Is this dependency really required or can the expression simply > be reduced to COMPILE_TEST and we drop the dependency to SOC_FSL? > > Note: This patch has now shown up in linux-next with commit > 3178d58e0b97. Currently, it would not be possible to compile test this > driver, as the dependency on SOC_FSL is never met. > > > Best regards, > > Lukas My bad :( The dependency must be FSL_SOC instead of SOC_FSL. I mean: diff --git a/drivers/soc/fsl/qe/Kconfig b/drivers/soc/fsl/qe/Kconfig index f90cfdf0c763..7268c2fbcbc1 100644 --- a/drivers/soc/fsl/qe/Kconfig +++ b/drivers/soc/fsl/qe/Kconfig @@ -47,7 +47,7 @@ config CPM_TSA config CPM_QMC tristate "CPM QMC support" depends on OF && HAS_IOMEM - depends on CPM1 || (SOC_FSL && COMPILE_TEST) + depends on CPM1 || (FSL_SOC && COMPILE_TEST) depends on CPM_TSA help Freescale CPM QUICC Multichannel Controller -- Hervé Codina, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Question about the dependency on the config SOC_FSL in CPM_QMC
Dear Herve, In your patch below, you added the config CPM_QMC which depends on the non-existing config SOC_FSL: https://lore.kernel.org/r/20230217145645.1768659-7-herve.cod...@bootlin.com Up to my knowledge, the config SOC_FSL never existed in the mainline tree. Is this dependency really required or can the expression simply be reduced to COMPILE_TEST and we drop the dependency to SOC_FSL? Note: This patch has now shown up in linux-next with commit 3178d58e0b97. Currently, it would not be possible to compile test this driver, as the dependency on SOC_FSL is never met. Best regards, Lukas
Re: [PATCH v11 09/13] arm64: dts: ls1046a: Add serdes nodes
On Mon, Mar 13, 2023 at 12:11:33PM -0400, Sean Anderson wrote: > This adds nodes for the SerDes devices. They are disabled by default > to prevent any breakage on existing boards. > > Signed-off-by: Sean Anderson The DTS patches look good to me. Let me know if they are ready to be applied. Shawn
