Re: [PATCH v4 0/3] Append new variables to vmcoreinfo (TCR_EL1.T1SZ for arm64 and MAX_PHYSMEM_BITS for all archs)
Hi Akashi, On Fri, Nov 15, 2019 at 7:29 AM AKASHI Takahiro wrote: > > Bhupesh, > > On Fri, Nov 15, 2019 at 01:24:17AM +0530, Bhupesh Sharma wrote: > > Hi Akashi, > > > > On Wed, Nov 13, 2019 at 12:11 PM AKASHI Takahiro > > wrote: > > > > > > Hi Bhupesh, > > > > > > Do you have a corresponding patch for userspace tools, > > > including crash util and/or makedumpfile? > > > Otherwise, we can't verify that a generated core file is > > > correctly handled. > > > > Sure. I am still working on the crash-utility related changes, but you > > can find the makedumpfile changes I posted a couple of days ago here > > (see [0]) and the github link for the makedumpfile changes can be seen > > via [1]. > > > > I will post the crash-util changes shortly as well. > > Thanks for having a look at the same. > > Thank you. > I have tested my kdump patch with a hacked version of crash > where VA_BITS_ACTUAL is calculated from tcr_el1_t1sz in vmcoreinfo. > I also did hack to calculate VA_BITS_ACTUAL is calculated from tcr_el1_t1sz in vmcoreinfo. Now i am getting error same as mentioned by you in other thread last month. https://www.mail-archive.com/crash-utility@redhat.com/msg07385.html how this error was overcome? I am using - crashkernel: https://github.com/crash-utility/crash.git commit: babd7ae62d4e8fd6f93fd30b88040d9376522aa3 and - Linux: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git commit: af42d3466bdc8f39806b26f593604fdc54140bcb --pk
[PATCH 4/5] powerpc/powernv: Remove set but not used variable 'pdn'
Fixes gcc '-Wunused-but-set-variable' warning:
arch/powerpc/platforms/powernv/pci-ioda.c: In function pnv_ioda_release_vf_PE:
arch/powerpc/platforms/powernv/pci-ioda.c:1468:25: warning: variable pdn set
but not used [-Wunused-but-set-variable]
It is introduced by commit 781a868f3136 ("powerpc/powernv:
Shift VF resource with an offset"), but never used, so remove it.
Reported-by: Hulk Robot
Signed-off-by: zhengbin
---
arch/powerpc/platforms/powernv/pci-ioda.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c
b/arch/powerpc/platforms/powernv/pci-ioda.c
index 846843b..47ed443 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1461,12 +1461,10 @@ static void pnv_ioda_release_vf_PE(struct pci_dev *pdev)
struct pci_controller *hose;
struct pnv_phb*phb;
struct pnv_ioda_pe*pe, *pe_n;
- struct pci_dn *pdn;
bus = pdev->bus;
hose = pci_bus_to_host(bus);
phb = hose->private_data;
- pdn = pci_get_pdn(pdev);
if (!pdev->is_physfn)
return;
--
2.7.4
[PATCH 2/5] powerpc/perf: Remove set but not used variable 'target'
Fixes gcc '-Wunused-but-set-variable' warning:
arch/powerpc/perf/imc-pmu.c: In function trace_imc_event_init:
arch/powerpc/perf/imc-pmu.c:1292:22: warning: variable target set but not used
[-Wunused-but-set-variable]
It is introduced by commit 012ae244845f ("powerpc/perf:
Trace imc PMU functions"), but never used, so remove it.
Reported-by: Hulk Robot
Signed-off-by: zhengbin
---
arch/powerpc/perf/imc-pmu.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c
index cb50a9e..83f0908 100644
--- a/arch/powerpc/perf/imc-pmu.c
+++ b/arch/powerpc/perf/imc-pmu.c
@@ -1302,8 +1302,6 @@ static void trace_imc_event_del(struct perf_event *event,
int flags)
static int trace_imc_event_init(struct perf_event *event)
{
- struct task_struct *target;
-
if (event->attr.type != event->pmu->type)
return -ENOENT;
@@ -1315,7 +1313,6 @@ static int trace_imc_event_init(struct perf_event *event)
return -ENOENT;
event->hw.idx = -1;
- target = event->hw.target;
event->pmu->task_ctx_nr = perf_hw_context;
return 0;
--
2.7.4
[PATCH 5/5] powerpc/powernv: Remove set but not used variable 'parent'
Fixes gcc '-Wunused-but-set-variable' warning:
arch/powerpc/platforms/powernv/pci-ioda.c: In function pnv_ioda_configure_pe:
arch/powerpc/platforms/powernv/pci-ioda.c:867:18: warning: variable parent set
but not used [-Wunused-but-set-variable]
It is not used since commit b131a8425c34 ("powerpc/powernv:
Set PELTV for compound PEs")
Reported-by: Hulk Robot
Signed-off-by: zhengbin
---
arch/powerpc/platforms/powernv/pci-ioda.c | 8
1 file changed, 8 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c
b/arch/powerpc/platforms/powernv/pci-ioda.c
index 47ed443..ae2db65 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -862,7 +862,6 @@ static int pnv_ioda_deconfigure_pe(struct pnv_phb *phb,
struct pnv_ioda_pe *pe)
static int pnv_ioda_configure_pe(struct pnv_phb *phb, struct pnv_ioda_pe *pe)
{
- struct pci_dev *parent;
uint8_t bcomp, dcomp, fcomp;
long rc, rid_end, rid;
@@ -872,7 +871,6 @@ static int pnv_ioda_configure_pe(struct pnv_phb *phb,
struct pnv_ioda_pe *pe)
dcomp = OPAL_IGNORE_RID_DEVICE_NUMBER;
fcomp = OPAL_IGNORE_RID_FUNCTION_NUMBER;
- parent = pe->pbus->self;
if (pe->flags & PNV_IODA_PE_BUS_ALL)
count = pe->pbus->busn_res.end -
pe->pbus->busn_res.start + 1;
else
@@ -893,12 +891,6 @@ static int pnv_ioda_configure_pe(struct pnv_phb *phb,
struct pnv_ioda_pe *pe)
}
rid_end = pe->rid + (count << 8);
} else {
-#ifdef CONFIG_PCI_IOV
- if (pe->flags & PNV_IODA_PE_VF)
- parent = pe->parent_dev;
- else
-#endif /* CONFIG_PCI_IOV */
- parent = pe->pdev->bus->self;
bcomp = OpalPciBusAll;
dcomp = OPAL_COMPARE_RID_DEVICE_NUMBER;
fcomp = OPAL_COMPARE_RID_FUNCTION_NUMBER;
--
2.7.4
[PATCH 1/5] powerpc/fadump: Remove set but not used variable 'elf'
Fixes gcc '-Wunused-but-set-variable' warning:
arch/powerpc/kernel/fadump.c: In function fadump_update_elfcore_header:
arch/powerpc/kernel/fadump.c:790:17: warning: variable elf set but not used
[-Wunused-but-set-variable]
It is introduced by commit ebaeb5ae2437 ("fadump:
Convert firmware-assisted cpu state dump data into elf notes."),
but never used, so remove it.
Reported-by: Hulk Robot
Signed-off-by: zhengbin
---
arch/powerpc/kernel/fadump.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
index ff0114a..66f6bf0 100644
--- a/arch/powerpc/kernel/fadump.c
+++ b/arch/powerpc/kernel/fadump.c
@@ -654,10 +654,8 @@ u32 *fadump_regs_to_elf_notes(u32 *buf, struct pt_regs
*regs)
void fadump_update_elfcore_header(char *bufp)
{
- struct elfhdr *elf;
struct elf_phdr *phdr;
- elf = (struct elfhdr *)bufp;
bufp += sizeof(struct elfhdr);
/* First note is a place holder for cpu notes info. */
--
2.7.4
[PATCH 0/5] powerpc: Remove five unused variables
zhengbin (5): powerpc/fadump: Remove set but not used variable 'elf' powerpc/perf: Remove set but not used variable 'target' powerpc/powernv: Remove set but not used variable 'total_vfs' powerpc/powernv: Remove set but not used variable 'pdn' powerpc/powernv: Remove set but not used variable 'parent' arch/powerpc/kernel/fadump.c | 2 -- arch/powerpc/perf/imc-pmu.c | 3 --- arch/powerpc/platforms/powernv/pci-ioda.c | 12 3 files changed, 17 deletions(-) -- 2.7.4
[PATCH 3/5] powerpc/powernv: Remove set but not used variable 'total_vfs'
Fixes gcc '-Wunused-but-set-variable' warning:
arch/powerpc/platforms/powernv/pci-ioda.c: In function pnv_pci_vf_assign_m64:
arch/powerpc/platforms/powernv/pci-ioda.c:1346:25: warning: variable total_vfs
set but not used [-Wunused-but-set-variable]
It is introduced by commit 02639b0e1326 ("powerpc/powernv:
Group VF PE when IOV BAR is big on PHB3"), but never used, so remove it.
Reported-by: Hulk Robot
Signed-off-by: zhengbin
---
arch/powerpc/platforms/powernv/pci-ioda.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c
b/arch/powerpc/platforms/powernv/pci-ioda.c
index da1068a..846843b 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1341,7 +1341,6 @@ static int pnv_pci_vf_assign_m64(struct pci_dev *pdev,
u16 num_vfs)
struct resource *res;
inti, j;
int64_trc;
- inttotal_vfs;
resource_size_tsize, start;
intpe_num;
intm64_bars;
@@ -1350,7 +1349,6 @@ static int pnv_pci_vf_assign_m64(struct pci_dev *pdev,
u16 num_vfs)
hose = pci_bus_to_host(bus);
phb = hose->private_data;
pdn = pci_get_pdn(pdev);
- total_vfs = pci_sriov_get_totalvfs(pdev);
if (pdn->m64_single_mode)
m64_bars = num_vfs;
--
2.7.4
[PATCH] KVM: PPC: Remove set but not used variable 'ra','rs','rt'
Fixes gcc '-Wunused-but-set-variable' warning:
arch/powerpc/kvm/emulate_loadstore.c: In function kvmppc_emulate_loadstore:
arch/powerpc/kvm/emulate_loadstore.c:87:6: warning: variable ra set but not
used [-Wunused-but-set-variable]
arch/powerpc/kvm/emulate_loadstore.c: In function kvmppc_emulate_loadstore:
arch/powerpc/kvm/emulate_loadstore.c:87:10: warning: variable rs set but not
used [-Wunused-but-set-variable]
arch/powerpc/kvm/emulate_loadstore.c: In function kvmppc_emulate_loadstore:
arch/powerpc/kvm/emulate_loadstore.c:87:14: warning: variable rt set but not
used [-Wunused-but-set-variable]
They are not used since commit 2b33cb585f94 ("KVM: PPC: Reimplement
LOAD_FP/STORE_FP instruction mmio emulation with analyse_instr() input")
Reported-by: Hulk Robot
Signed-off-by: zhengbin
---
arch/powerpc/kvm/emulate_loadstore.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/arch/powerpc/kvm/emulate_loadstore.c
b/arch/powerpc/kvm/emulate_loadstore.c
index 2e496eb..1139bc5 100644
--- a/arch/powerpc/kvm/emulate_loadstore.c
+++ b/arch/powerpc/kvm/emulate_loadstore.c
@@ -73,7 +73,6 @@ int kvmppc_emulate_loadstore(struct kvm_vcpu *vcpu)
{
struct kvm_run *run = vcpu->run;
u32 inst;
- int ra, rs, rt;
enum emulation_result emulated = EMULATE_FAIL;
int advance = 1;
struct instruction_op op;
@@ -85,10 +84,6 @@ int kvmppc_emulate_loadstore(struct kvm_vcpu *vcpu)
if (emulated != EMULATE_DONE)
return emulated;
- ra = get_ra(inst);
- rs = get_rs(inst);
- rt = get_rt(inst);
-
vcpu->arch.mmio_vsx_copy_nums = 0;
vcpu->arch.mmio_vsx_offset = 0;
vcpu->arch.mmio_copy_type = KVMPPC_VSX_COPY_NONE;
--
2.7.4
Re: [PATCH v4 0/3] Append new variables to vmcoreinfo (TCR_EL1.T1SZ for arm64 and MAX_PHYSMEM_BITS for all archs)
Hi Prabhakar,
On Tue, Nov 19, 2019 at 12:02:46PM +0530, Prabhakar Kushwaha wrote:
> Hi Akashi,
>
> On Fri, Nov 15, 2019 at 7:29 AM AKASHI Takahiro
> wrote:
> >
> > Bhupesh,
> >
> > On Fri, Nov 15, 2019 at 01:24:17AM +0530, Bhupesh Sharma wrote:
> > > Hi Akashi,
> > >
> > > On Wed, Nov 13, 2019 at 12:11 PM AKASHI Takahiro
> > > wrote:
> > > >
> > > > Hi Bhupesh,
> > > >
> > > > Do you have a corresponding patch for userspace tools,
> > > > including crash util and/or makedumpfile?
> > > > Otherwise, we can't verify that a generated core file is
> > > > correctly handled.
> > >
> > > Sure. I am still working on the crash-utility related changes, but you
> > > can find the makedumpfile changes I posted a couple of days ago here
> > > (see [0]) and the github link for the makedumpfile changes can be seen
> > > via [1].
> > >
> > > I will post the crash-util changes shortly as well.
> > > Thanks for having a look at the same.
> >
> > Thank you.
> > I have tested my kdump patch with a hacked version of crash
> > where VA_BITS_ACTUAL is calculated from tcr_el1_t1sz in vmcoreinfo.
> >
>
> I also did hack to calculate VA_BITS_ACTUAL is calculated from
> tcr_el1_t1sz in vmcoreinfo. Now i am getting error same as mentioned
> by you in other thread last month.
> https://www.mail-archive.com/crash-utility@redhat.com/msg07385.html
>
> how this error was overcome?
>
> I am using
> - crashkernel: https://github.com/crash-utility/crash.git commit:
> babd7ae62d4e8fd6f93fd30b88040d9376522aa3
> and
> - Linux: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> commit: af42d3466bdc8f39806b26f593604fdc54140bcb
# I am rather reluctant to cross-post non-kernel patch to lkml/lakml,
The only change I made to crash utility was:
===8<===
diff --git a/arm64.c b/arm64.c
index 5ee5f1a29a41..84e40aeb561b 100644
--- a/arm64.c
+++ b/arm64.c
@@ -3857,8 +3857,8 @@ arm64_calc_VA_BITS(void)
} else if (ACTIVE())
error(FATAL, "cannot determine VA_BITS_ACTUAL: please
use /proc/kcore\n");
else {
- if ((string =
pc->read_vmcoreinfo("NUMBER(VA_BITS_ACTUAL)"))) {
- value = atol(string);
+ if ((string =
pc->read_vmcoreinfo("NUMBER(tcr_el1_t1sz)"))) {
+ value = 64 - strtoll(string, NULL, 0);
free(string);
machdep->machspec->VA_BITS_ACTUAL = value;
machdep->machspec->VA_BITS = value;
===>8===
Thanks,
-Takahiro Akashi
> --pk
Re: [PATCH 04/11] powerpc/powernv/ioda: Release opencapi device
On 10/9/19 1:45 am, Frederic Barrat wrote: With hotplug, an opencapi device can now go away. It needs to be released, mostly to clean up its PE state. We were previously not defining any device callback. We can reuse the standard PCI release callback, it does a bit too much for an opencapi device, but it's harmless, and only needs minor tuning. Also separate the undo of the PELT-V code in a separate function, it is not needed for NPU devices and it improves a bit the readability of the code. Signed-off-by: Frederic Barrat Reviewed-by: Andrew Donnellan -- Andrew Donnellan OzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH v5 02/24] mm/gup: factor out duplicate code from four routines
On 11/18/19 1:46 AM, Jan Kara wrote:
> On Thu 14-11-19 21:53:18, John Hubbard wrote:
>> There are four locations in gup.c that have a fair amount of code
>> duplication. This means that changing one requires making the same
>> changes in four places, not to mention reading the same code four
>> times, and wondering if there are subtle differences.
>>
>> Factor out the common code into static functions, thus reducing the
>> overall line count and the code's complexity.
>>
>> Also, take the opportunity to slightly improve the efficiency of the
>> error cases, by doing a mass subtraction of the refcount, surrounded
>> by get_page()/put_page().
>>
>> Also, further simplify (slightly), by waiting until the the successful
>> end of each routine, to increment *nr.
>>
>> Reviewed-by: Jérôme Glisse
>> Cc: Jan Kara
>> Cc: Ira Weiny
>> Cc: Christoph Hellwig
>> Cc: Aneesh Kumar K.V
>> Signed-off-by: John Hubbard
>> ---
>> mm/gup.c | 95
>> 1 file changed, 40 insertions(+), 55 deletions(-)
>>
>> diff --git a/mm/gup.c b/mm/gup.c
>> index 85caf76b3012..858541ea30ce 100644
>> --- a/mm/gup.c
>> +++ b/mm/gup.c
>> @@ -1969,6 +1969,29 @@ static int __gup_device_huge_pud(pud_t pud, pud_t
>> *pudp, unsigned long addr,
>> }
>> #endif
>>
>> +static int __record_subpages(struct page *page, unsigned long addr,
>> + unsigned long end, struct page **pages)
>> +{
>> +int nr = 0;
>> +int nr_recorded_pages = 0;
>> +
>> +do {
>> +pages[nr] = page;
>> +nr++;
>> +page++;
>> +nr_recorded_pages++;
>> +} while (addr += PAGE_SIZE, addr != end);
>> +return nr_recorded_pages;
>
> nr == nr_recorded_pages so no need for both... BTW, structuring this as a
> for loop would be probably more logical and shorter now:
>
> for (nr = 0; addr != end; addr += PAGE_SIZE)
> pages[nr++] = page++;
> return nr;
>
Nice touch, I've applied it.
thanks,
--
John Hubbard
NVIDIA
> The rest of the patch looks good to me.
>
> Honza
>
Re: [PATCH v3 15/15] powerpc/32s: Activate CONFIG_VMAP_STACK
Christophe Leroy writes: > A few changes to retrieve DAR and DSISR from struct regs > instead of retrieving them directly, as they may have > changed due to a TLB miss. > > Also modifies hash_page() and friends to work with virtual > data addresses instead of physical ones. > > Signed-off-by: Christophe Leroy > --- > arch/powerpc/kernel/entry_32.S | 4 +++ > arch/powerpc/kernel/head_32.S | 19 +++--- > arch/powerpc/kernel/head_32.h | 4 ++- > arch/powerpc/mm/book3s32/hash_low.S| 46 > +- > arch/powerpc/mm/book3s32/mmu.c | 9 +-- > arch/powerpc/platforms/Kconfig.cputype | 2 ++ > 6 files changed, 61 insertions(+), 23 deletions(-) This is faulting with qemu mac99 model: Key type id_resolver registered Key type id_legacy registered BUG: Unable to handle kernel data access on read at 0x2f0db684 Faulting instruction address: 0x4130 Oops: Kernel access of bad area, sig: 11 [#1] BE PAGE_SIZE=4K MMU=Hash PowerMac Modules linked in: CPU: 0 PID: 65 Comm: modprobe Not tainted 5.4.0-rc2-gcc49+ #63 NIP: 4130 LR: 08c8 CTR: b7eb86e0 REGS: f106de80 TRAP: 0300 Not tainted (5.4.0-rc2-gcc49+) MSR: 3012 CR: 4106df38 XER: 2000 DAR: 2f0db684 DSISR: 4000 GPR00: b7ec5d64 f106df38 bf988a70 2f0db540 b7ec3620 bf988d38 GPR08: 1880 d032 72656773 f106df38 b7ed10ec b7ed3d38 b7ee8900 GPR16: bf988d10 0001 bf988d10 b7ec3620 bf988d50 b7ee76ec b7ee7320 GPR24: 1878 b7ee8900 10029f00 1879 b7ee7ff4 bf988d30 NIP [4130] 0x4130 LR [08c8] 0x8c8 Call Trace: [f106df38] [c0016224] ret_from_syscall+0x0/0x34 (unreliable) --- interrupt: c01 at 0xb7ed0f50 LR = 0xb7ec5d64 Instruction dump: db8300e0 fc00048e 60a52000 80850144 ---[ end trace 265da51c6d8b86c5 ]--- I think I'll have to drop this series for now. cheers
Re: [PATCH v3 15/15] powerpc/32s: Activate CONFIG_VMAP_STACK
Michael Ellerman writes: > Christophe Leroy writes: >> A few changes to retrieve DAR and DSISR from struct regs >> instead of retrieving them directly, as they may have >> changed due to a TLB miss. >> >> Also modifies hash_page() and friends to work with virtual >> data addresses instead of physical ones. >> >> Signed-off-by: Christophe Leroy >> --- >> arch/powerpc/kernel/entry_32.S | 4 +++ >> arch/powerpc/kernel/head_32.S | 19 +++--- >> arch/powerpc/kernel/head_32.h | 4 ++- >> arch/powerpc/mm/book3s32/hash_low.S| 46 >> +- >> arch/powerpc/mm/book3s32/mmu.c | 9 +-- >> arch/powerpc/platforms/Kconfig.cputype | 2 ++ >> 6 files changed, 61 insertions(+), 23 deletions(-) > > If I build pmac32_defconfig with KVM enabled this causes a build break: > > arch/powerpc/kernel/head_32.S: Assembler messages: > arch/powerpc/kernel/head_32.S:324: Error: attempt to move .org backwards > scripts/Makefile.build:357: recipe for target > 'arch/powerpc/kernel/head_32.o' failed > make[2]: *** [arch/powerpc/kernel/head_32.o] Error 1 > > In the interests of getting the series merged I'm inclined to just make > VMAP_STACK and KVM incompatible for now with: > > diff --git a/arch/powerpc/platforms/Kconfig.cputype > b/arch/powerpc/platforms/Kconfig.cputype > index 15c9097dc4f7..5074fe77af40 100644 > --- a/arch/powerpc/platforms/Kconfig.cputype > +++ b/arch/powerpc/platforms/Kconfig.cputype > @@ -31,7 +31,7 @@ config PPC_BOOK3S_6xx > select PPC_HAVE_PMU_SUPPORT > select PPC_HAVE_KUEP > select PPC_HAVE_KUAP > - select HAVE_ARCH_VMAP_STACK > + select HAVE_ARCH_VMAP_STACK if !KVM_BOOK3S_32 For some reason this needs to be !KVM. > config PPC_BOOK3S_601 > bool "PowerPC 601" > > > Thoughts? cheers
Re: [PATCH 02/11] powerpc/powernv/ioda: Protect PE list
On 10/9/19 1:45 am, Frederic Barrat wrote: Protect the PHB's list of PE. Probably not needed as long as it was populated during PHB creation, but it feels right and will become required once we can add/remove opencapi devices on hotplug. Signed-off-by: Frederic Barrat Reviewed-by: Andrew Donnellan -- Andrew Donnellan OzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH] powerpc/pseries: remove variable 'status' set but not used
Chen Wandun writes:
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> arch/powerpc/platforms/pseries/ras.c: In function ras_epow_interrupt:
> arch/powerpc/platforms/pseries/ras.c:319:6: warning: variable status set but
> not used [-Wunused-but-set-variable]
Thanks for the patch.
But it almost certainly is wrong to not check the status.
It's calling firmware and just assuming that the call succeeded. It then
goes on to use the result that should have been written by firmware, but
is now potentially random junk.
So I'd much rather a patch to change it to check the status.
> diff --git a/arch/powerpc/platforms/pseries/ras.c
> b/arch/powerpc/platforms/pseries/ras.c
> index 1d7f973..4a61d0f 100644
> --- a/arch/powerpc/platforms/pseries/ras.c
> +++ b/arch/powerpc/platforms/pseries/ras.c
> @@ -316,12 +316,11 @@ static irqreturn_t ras_hotplug_interrupt(int irq, void
> *dev_id)
> /* Handle environmental and power warning (EPOW) interrupts. */
> static irqreturn_t ras_epow_interrupt(int irq, void *dev_id)
> {
> - int status;
> int state;
> int critical;
>
> - status = rtas_get_sensor_fast(EPOW_SENSOR_TOKEN, EPOW_SENSOR_INDEX,
> - &state);
> + rtas_get_sensor_fast(EPOW_SENSOR_TOKEN, EPOW_SENSOR_INDEX,
> + &state);
This is calling a helper which already does some translation of the
return value, any value < 0 indicates an error.
> @@ -330,12 +329,12 @@ static irqreturn_t ras_epow_interrupt(int irq, void
> *dev_id)
>
> spin_lock(&ras_log_buf_lock);
>
> - status = rtas_call(ras_check_exception_token, 6, 1, NULL,
> -RTAS_VECTOR_EXTERNAL_INTERRUPT,
> -virq_to_hw(irq),
> -RTAS_EPOW_WARNING,
> -critical, __pa(&ras_log_buf),
> - rtas_get_error_log_max());
> + rtas_call(ras_check_exception_token, 6, 1, NULL,
> + RTAS_VECTOR_EXTERNAL_INTERRUPT,
> + virq_to_hw(irq),
> + RTAS_EPOW_WARNING,
> + critical, __pa(&ras_log_buf),
> + rtas_get_error_log_max());
This is directly calling firmware.
As documented in LoPAPR, a negative status indicates an error, 0
indicates a new error log was found (ie. the function should continue),
or 1 there was no error log (ie. nothing to do).
cheers
> log_error(ras_log_buf, ERR_TYPE_RTAS_LOG, 0);
>
> --
> 2.7.4
[PATCH 4.14 145/239] libfdt: Ensure INT_MAX is defined in libfdt_env.h
From: Rob Herring [ Upstream commit 53dd9dce6979bc54d64a3a09a2fb20187a025be7 ] The next update of libfdt has a new dependency on INT_MAX. Update the instances of libfdt_env.h in the kernel to either include the necessary header with the definition or define it locally. Cc: Russell King Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Cc: Michael Ellerman Cc: linux-arm-ker...@lists.infradead.org Cc: linuxppc-dev@lists.ozlabs.org Signed-off-by: Rob Herring Signed-off-by: Sasha Levin --- arch/arm/boot/compressed/libfdt_env.h | 2 ++ arch/powerpc/boot/libfdt_env.h| 2 ++ include/linux/libfdt_env.h| 1 + 3 files changed, 5 insertions(+) diff --git a/arch/arm/boot/compressed/libfdt_env.h b/arch/arm/boot/compressed/libfdt_env.h index 07437816e0986..b36c0289a308e 100644 --- a/arch/arm/boot/compressed/libfdt_env.h +++ b/arch/arm/boot/compressed/libfdt_env.h @@ -6,6 +6,8 @@ #include #include +#define INT_MAX((int)(~0U>>1)) + typedef __be16 fdt16_t; typedef __be32 fdt32_t; typedef __be64 fdt64_t; diff --git a/arch/powerpc/boot/libfdt_env.h b/arch/powerpc/boot/libfdt_env.h index f52c31b1f48fa..39155d3b2cefa 100644 --- a/arch/powerpc/boot/libfdt_env.h +++ b/arch/powerpc/boot/libfdt_env.h @@ -5,6 +5,8 @@ #include #include +#define INT_MAX((int)(~0U>>1)) + #include "of.h" typedef u32 uint32_t; diff --git a/include/linux/libfdt_env.h b/include/linux/libfdt_env.h index 14997285e53d3..1aa707ab19bbf 100644 --- a/include/linux/libfdt_env.h +++ b/include/linux/libfdt_env.h @@ -2,6 +2,7 @@ #ifndef _LIBFDT_ENV_H #define _LIBFDT_ENV_H +#include /* For INT_MAX */ #include #include -- 2.20.1
[PATCH 4.19 270/422] libfdt: Ensure INT_MAX is defined in libfdt_env.h
From: Rob Herring [ Upstream commit 53dd9dce6979bc54d64a3a09a2fb20187a025be7 ] The next update of libfdt has a new dependency on INT_MAX. Update the instances of libfdt_env.h in the kernel to either include the necessary header with the definition or define it locally. Cc: Russell King Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Cc: Michael Ellerman Cc: linux-arm-ker...@lists.infradead.org Cc: linuxppc-dev@lists.ozlabs.org Signed-off-by: Rob Herring Signed-off-by: Sasha Levin --- arch/arm/boot/compressed/libfdt_env.h | 2 ++ arch/powerpc/boot/libfdt_env.h| 2 ++ include/linux/libfdt_env.h| 1 + 3 files changed, 5 insertions(+) diff --git a/arch/arm/boot/compressed/libfdt_env.h b/arch/arm/boot/compressed/libfdt_env.h index 07437816e0986..b36c0289a308e 100644 --- a/arch/arm/boot/compressed/libfdt_env.h +++ b/arch/arm/boot/compressed/libfdt_env.h @@ -6,6 +6,8 @@ #include #include +#define INT_MAX((int)(~0U>>1)) + typedef __be16 fdt16_t; typedef __be32 fdt32_t; typedef __be64 fdt64_t; diff --git a/arch/powerpc/boot/libfdt_env.h b/arch/powerpc/boot/libfdt_env.h index 2a0c8b1bf1479..2abc8e83b95e9 100644 --- a/arch/powerpc/boot/libfdt_env.h +++ b/arch/powerpc/boot/libfdt_env.h @@ -5,6 +5,8 @@ #include #include +#define INT_MAX((int)(~0U>>1)) + #include "of.h" typedef unsigned long uintptr_t; diff --git a/include/linux/libfdt_env.h b/include/linux/libfdt_env.h index c6ac1fe7ec68a..edb0f0c309044 100644 --- a/include/linux/libfdt_env.h +++ b/include/linux/libfdt_env.h @@ -2,6 +2,7 @@ #ifndef LIBFDT_ENV_H #define LIBFDT_ENV_H +#include /* For INT_MAX */ #include #include -- 2.20.1
Re: [-merge] BUG followed by oops running ndctl tests
> On 16-Nov-2019, at 12:25 AM, Aneesh Kumar K.V > wrote: > > On 11/15/19 11:36 AM, Sachin Sant wrote: >> Following Oops is seen on latest (commit 3b4852888d) powerpc merge branch >> code while running ndctl (test_namespace) tests >> 85c5b0984e was good. > > > > Are the namespace size created with size that is multiple of 16M size? > > Wondering whether this is related to > https://patchwork.kernel.org/patch/11215049/ This patch series doesn’t seem to help. I can still recreate the problem with the patches applied. Thanks -Sachin > > -aneesh
Re: [PATCH 08/11] pci/hotplug/pnv-php: Register opencapi slots
On 10/9/19 1:45 am, Frederic Barrat wrote: Add the opencapi PHBs to the list of PHBs being scanned to look for slots. Signed-off-by: Frederic Barrat --- drivers/pci/hotplug/pnv_php.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/pci/hotplug/pnv_php.c b/drivers/pci/hotplug/pnv_php.c index 304bdbcdb77c..f0a7360154e7 100644 --- a/drivers/pci/hotplug/pnv_php.c +++ b/drivers/pci/hotplug/pnv_php.c @@ -954,7 +954,8 @@ static int __init pnv_php_init(void) pr_info(DRIVER_DESC " version: " DRIVER_VERSION "\n"); for_each_compatible_node(dn, NULL, "ibm,ioda2-phb") pnv_php_register(dn); - + for_each_compatible_node(dn, NULL, "ibm,ioda2-npu2-opencapi-phb") + pnv_php_register_one(dn); return 0; } @@ -964,6 +965,8 @@ static void __exit pnv_php_exit(void) for_each_compatible_node(dn, NULL, "ibm,ioda2-phb") pnv_php_unregister(dn); + for_each_compatible_node(dn, NULL, "ibm,ioda2-npu2-opencapi-phb") + pnv_php_unregister(dn); } Why do we use register_one to register and unregister rather than unregister_one to unregister? -- Andrew Donnellan OzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH v5 10/24] mm/gup: introduce pin_user_pages*() and FOLL_PIN
On 11/18/19 2:16 AM, Jan Kara wrote: > On Thu 14-11-19 21:53:26, John Hubbard wrote: >> /* >> - * NOTE on FOLL_LONGTERM: >> + * FOLL_PIN and FOLL_LONGTERM may be used in various combinations with each >> + * other. Here is what they mean, and how to use them: >> * >> * FOLL_LONGTERM indicates that the page will be held for an indefinite time >> - * period _often_ under userspace control. This is contrasted with >> - * iov_iter_get_pages() where usages which are transient. >> + * period _often_ under userspace control. This is in contrast to >> + * iov_iter_get_pages(), where usages which are transient. > ^^^ when you touch this, please fix also the > second sentense. It doesn't quite make sense to me... I'd probably write > there "whose usages are transient" but maybe you can come up with something > even better. Fixed, using your wording, as I didn't see any obvious improvements beyond that. thanks, -- John Hubbard NVIDIA > > Otherwise the patch looks good to me so feel free to add: > > Reviewed-by: Jan Kara > > Honza >
Re: [PATCH 06/11] pci/hotplug/pnv-php: Remove erroneous warning
On 10/9/19 1:45 am, Frederic Barrat wrote: On powernv, when removing a device through hotplug, the following warning is logged: Invalid refcount <.> on <...> It may be incorrect, the refcount may be set to a higher value than 1 and be valid. of_detach_node() can drop more than one reference. As it doesn't seem trivial to assert the correct value, let's remove the warning. Signed-off-by: Frederic Barrat Reviewed-by: Andrew Donnellan -- Andrew Donnellan OzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
[PATCH v5 3/3] powerpc/prom_init: Use -ffreestanding to avoid a reference to bcmp
r374662 gives LLVM the ability to convert certain loops into a reference
to bcmp as an optimization; this breaks prom_init_check.sh:
CALLarch/powerpc/kernel/prom_init_check.sh
Error: External symbol 'bcmp' referenced from prom_init.c
make[2]: *** [arch/powerpc/kernel/Makefile:196: prom_init_check] Error 1
bcmp is defined in lib/string.c as a wrapper for memcmp so this could be
added to the whitelist. However, commit 450e7dd4001f ("powerpc/prom_init:
don't use string functions from lib/") copied memcmp as prom_memcmp to
avoid KASAN instrumentation so having bcmp be resolved to regular memcmp
would break that assumption. Furthermore, because the compiler is the
one that inserted bcmp, we cannot provide something like prom_bcmp.
To prevent LLVM from being clever with optimizations like this, use
-ffreestanding to tell LLVM we are not hosted so it is not free to make
transformations like this.
Link: https://github.com/ClangBuiltLinux/linux/issues/647
Link:
https://github.com/llvm/llvm-project/commit/76cdcf25b883751d83402baea6316772aa73865c
Reviewed-by: Nick Desaulneris
Signed-off-by: Nathan Chancellor
---
v1 -> v3:
* New patch in the series
v3 -> v4:
* Rebase on v5.4-rc3.
* Add Nick's reviewed-by tag.
* Update the LLVM commit reference to the latest applied version (r374662)
as it was originally committed as r370454, reverted in r370788, and
reapplied as r374662.
v4 -> v5:
* Rebase on next-20191118 to avoid a conflict with commit
6266a4dadb1d ("powerpc/64s: Always disable branch profiling for prom_init.o")
arch/powerpc/kernel/Makefile | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 3c113ae0de2b..82170c155cb6 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -23,6 +23,7 @@ CFLAGS_prom.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
CFLAGS_prom_init.o += $(call cc-option, -fno-stack-protector)
CFLAGS_prom_init.o += -DDISABLE_BRANCH_PROFILING
+CFLAGS_prom_init.o += -ffreestanding
ifdef CONFIG_FUNCTION_TRACER
# Do not trace early boot code
--
2.24.0
[PATCH v5 2/3] powerpc: Avoid clang warnings around setjmp and longjmp
Commit aea447141c7e ("powerpc: Disable -Wbuiltin-requires-header when
setjmp is used") disabled -Wbuiltin-requires-header because of a warning
about the setjmp and longjmp declarations.
r367387 in clang added another diagnostic around this, complaining that
there is no jmp_buf declaration.
In file included from ../arch/powerpc/xmon/xmon.c:47:
../arch/powerpc/include/asm/setjmp.h:10:13: error: declaration of
built-in function 'setjmp' requires the declaration of the 'jmp_buf'
type, commonly provided in the header .
[-Werror,-Wincomplete-setjmp-declaration]
extern long setjmp(long *);
^
../arch/powerpc/include/asm/setjmp.h:11:13: error: declaration of
built-in function 'longjmp' requires the declaration of the 'jmp_buf'
type, commonly provided in the header .
[-Werror,-Wincomplete-setjmp-declaration]
extern void longjmp(long *, long);
^
2 errors generated.
We are not using the standard library's longjmp/setjmp implementations
for obvious reasons; make this clear to clang by using -ffreestanding
on these files.
Cc: sta...@vger.kernel.org # 4.14+
Link: https://github.com/ClangBuiltLinux/linux/issues/625
Link:
https://github.com/llvm/llvm-project/commit/3be25e79477db2d31ac46493d97eca8c20592b07
Link: https://godbolt.org/z/B2oQnl
Suggested-by: Segher Boessenkool
Reviewed-by: Nick Desaulniers
Signed-off-by: Nathan Chancellor
---
v1 -> v3 (I skipped v2 because the first patch in the series already
had a v2):
* Use -ffreestanding instead of outright disabling the warning because
it is legitimate.
v3 -> v4:
* Rebase on v5.4-rc3
* Add Nick's reviewed-by and Compiler Explorer link.
v4 -> v5:
* Rebase on next-20191118
arch/powerpc/kernel/Makefile | 4 ++--
arch/powerpc/xmon/Makefile | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index bb57d168d6f4..3c113ae0de2b 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -5,8 +5,8 @@
CFLAGS_ptrace.o+= -DUTS_MACHINE='"$(UTS_MACHINE)"'
-# Disable clang warning for using setjmp without setjmp.h header
-CFLAGS_crash.o += $(call cc-disable-warning, builtin-requires-header)
+# Avoid clang warnings around longjmp/setjmp declarations
+CFLAGS_crash.o += -ffreestanding
ifdef CONFIG_PPC64
CFLAGS_prom_init.o += $(NO_MINIMAL_TOC)
diff --git a/arch/powerpc/xmon/Makefile b/arch/powerpc/xmon/Makefile
index f142570ad860..c3842dbeb1b7 100644
--- a/arch/powerpc/xmon/Makefile
+++ b/arch/powerpc/xmon/Makefile
@@ -1,8 +1,8 @@
# SPDX-License-Identifier: GPL-2.0
# Makefile for xmon
-# Disable clang warning for using setjmp without setjmp.h header
-subdir-ccflags-y := $(call cc-disable-warning, builtin-requires-header)
+# Avoid clang warnings around longjmp/setjmp declarations
+subdir-ccflags-y := -ffreestanding
GCOV_PROFILE := n
KCOV_INSTRUMENT := n
--
2.24.0
[PATCH v5 1/3] powerpc: Don't add -mabi= flags when building with Clang
When building pseries_defconfig, building vdso32 errors out:
error: unknown target ABI 'elfv1'
This happens because -m32 in clang changes the target to 32-bit,
which does not allow the ABI to be changed, as the setABI virtual
function is not overridden:
https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0/clang/include/clang/Basic/TargetInfo.h#L1073-L1078
https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0/clang/lib/Basic/Targets/PPC.h#L327-L365
Commit 4dc831aa8813 ("powerpc: Fix compiling a BE kernel with a
powerpc64le toolchain") added these flags to fix building big endian
kernels with a little endian GCC.
Clang doesn't need -mabi because the target triple controls the default
value. -mlittle-endian and -mbig-endian manipulate the triple into
either powerpc64-* or powerpc64le-*, which properly sets the default
ABI:
https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0/clang/lib/Driver/Driver.cpp#L450-L463
https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0/llvm/lib/Support/Triple.cpp#L1432-L1516
https://github.com/llvm/llvm-project/blob/llvmorg-9.0.0/clang/lib/Basic/Targets/PPC.h#L377-L383
Adding a debug print out in the PPC64TargetInfo constructor after line
383 above shows this:
$ echo | ./clang -E --target=powerpc64-linux -mbig-endian -o /dev/null -
Default ABI: elfv1
$ echo | ./clang -E --target=powerpc64-linux -mlittle-endian -o /dev/null -
Default ABI: elfv2
$ echo | ./clang -E --target=powerpc64le-linux -mbig-endian -o /dev/null -
Default ABI: elfv1
$ echo | ./clang -E --target=powerpc64le-linux -mlittle-endian -o /dev/null -
Default ABI: elfv2
Don't specify -mabi when building with clang to avoid the build error
with -m32 and not change any code generation.
-mcall-aixdesc is not an implemented flag in clang so it can be
safely excluded as well, see commit 238abecde8ad ("powerpc: Don't
use gcc specific options on clang").
pseries_defconfig successfully builds after this patch and
powernv_defconfig and ppc44x_defconfig don't regress.
Link: https://github.com/ClangBuiltLinux/linux/issues/240
Reviewed-by: Daniel Axtens
Signed-off-by: Nathan Chancellor
---
v1 -> v2:
* Improve commit message
v2 -> v3:
* Rebase and merge into a single series.
v3 -> v4:
* Rebase on v5.4-rc3.
* Update links to point to llvmorg-9.0.0 instead of llvmorg-9.0.0-rc2.
v4 -> v5:
* Rebase on next-20191118
arch/powerpc/Makefile | 4
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index a3ed4f168607..f35730548e42 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -91,11 +91,13 @@ MULTIPLEWORD:= -mmultiple
endif
ifdef CONFIG_PPC64
+ifndef CONFIG_CC_IS_CLANG
cflags-$(CONFIG_CPU_BIG_ENDIAN)+= $(call cc-option,-mabi=elfv1)
cflags-$(CONFIG_CPU_BIG_ENDIAN)+= $(call
cc-option,-mcall-aixdesc)
aflags-$(CONFIG_CPU_BIG_ENDIAN)+= $(call cc-option,-mabi=elfv1)
aflags-$(CONFIG_CPU_LITTLE_ENDIAN) += -mabi=elfv2
endif
+endif
ifndef CONFIG_CC_IS_CLANG
cflags-$(CONFIG_CPU_LITTLE_ENDIAN) += -mno-strict-align
@@ -141,6 +143,7 @@ endif
endif
CFLAGS-$(CONFIG_PPC64) := $(call cc-option,-mtraceback=no)
+ifndef CONFIG_CC_IS_CLANG
ifdef CONFIG_CPU_LITTLE_ENDIAN
CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mabi=elfv2,$(call
cc-option,-mcall-aixdesc))
AFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mabi=elfv2)
@@ -149,6 +152,7 @@ CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mabi=elfv1)
CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mcall-aixdesc)
AFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mabi=elfv1)
endif
+endif
CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mcmodel=medium,$(call
cc-option,-mminimal-toc))
CFLAGS-$(CONFIG_PPC64) += $(call cc-option,-mno-pointers-to-nested-functions)
--
2.24.0
[PATCH v5 0/3] LLVM/Clang fixes for a few defconfigs
Hi all, This series includes a set of fixes for LLVM/Clang when building a few defconfigs (powernv, ppc44x, and pseries are the ones that our CI configuration tests [1]). The first patch fixes pseries_defconfig, which has never worked in mainline. The second and third patches fixes issues with all of these configs due to internal changes to LLVM, which point out issues with the kernel. These have been broken since July/August, it would be nice to get these reviewed and applied. Please let me know what I can do to get these applied soon so we can stop applying them out of tree. [1]: https://github.com/ClangBuiltLinux/continuous-integration Previous versions: v3: https://lore.kernel.org/lkml/20190911182049.77853-1-natechancel...@gmail.com/ v4: https://lore.kernel.org/lkml/20191014025101.18567-1-natechancel...@gmail.com/ Cheers, Nathan
Re: [PATCH 08/10] nvdimm: Add driver for OpenCAPI Storage Class Memory
On Mon, Nov 18, 2019 at 7:29 PM Andrew Donnellan wrote: > > On 19/11/19 1:48 pm, Alastair D'Silva wrote: > > On Tue, 2019-11-19 at 10:47 +1100, Andrew Donnellan wrote: > >> On 15/11/19 3:35 am, Dan Williams wrote: > Have you discussed with the directory owner if it's ok to split > the > driver over several files? > >>> > >>> My thought is to establish drivers/opencapi/ and move this and the > >>> existing drivers/misc/ocxl/ bits there. > >> > >> Is there any other justification for this we can think of apart from > >> not > >> wanting to put this driver in the nvdimm directory? OpenCAPI drivers > >> aren't really a category of driver unto themselves. > >> > > > > There is a precedent for bus-based dirs, eg. drivers/(ide|w1|spi) all > > contain drivers for both controllers & connected devices. > > > > Fred, how do you feel about moving the generic OpenCAPI driver out of > > drivers/misc? > > Instinctively I don't like the idea of creating a whole opencapi > directory, as OpenCAPI is a generic bus which is not tightly coupled to > any particular application area, and drivers for other OpenCAPI devices > are already spread throughout the tree (e.g. cxlflash in drivers/scsi). I'm not suggesting all opencapi drivers go there, nor the entirety of this driver, just common infrastructure. That said, it's hard to talk about specifics given the current state of the patch set. I have not even taken a deeper look past the changelog as this 3K lines-of-code submission needs to be broken up into smaller pieces before we settle on what pieces belong where. Just looking at the diffstat, at a minimum it's not appropriate for them to live in drivers/nvdimm/ directly, drivers/nvdimm/oxcl/ would be an acceptable starting point.
Re: [PATCH 08/10] nvdimm: Add driver for OpenCAPI Storage Class Memory
On 19/11/19 1:48 pm, Alastair D'Silva wrote: On Tue, 2019-11-19 at 10:47 +1100, Andrew Donnellan wrote: On 15/11/19 3:35 am, Dan Williams wrote: Have you discussed with the directory owner if it's ok to split the driver over several files? My thought is to establish drivers/opencapi/ and move this and the existing drivers/misc/ocxl/ bits there. Is there any other justification for this we can think of apart from not wanting to put this driver in the nvdimm directory? OpenCAPI drivers aren't really a category of driver unto themselves. There is a precedent for bus-based dirs, eg. drivers/(ide|w1|spi) all contain drivers for both controllers & connected devices. Fred, how do you feel about moving the generic OpenCAPI driver out of drivers/misc? Instinctively I don't like the idea of creating a whole opencapi directory, as OpenCAPI is a generic bus which is not tightly coupled to any particular application area, and drivers for other OpenCAPI devices are already spread throughout the tree (e.g. cxlflash in drivers/scsi). -- Andrew Donnellan OzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH 08/10] nvdimm: Add driver for OpenCAPI Storage Class Memory
On Tue, 2019-11-19 at 10:47 +1100, Andrew Donnellan wrote: > On 15/11/19 3:35 am, Dan Williams wrote: > > > Have you discussed with the directory owner if it's ok to split > > > the > > > driver over several files? > > > > My thought is to establish drivers/opencapi/ and move this and the > > existing drivers/misc/ocxl/ bits there. > > Is there any other justification for this we can think of apart from > not > wanting to put this driver in the nvdimm directory? OpenCAPI drivers > aren't really a category of driver unto themselves. > There is a precedent for bus-based dirs, eg. drivers/(ide|w1|spi) all contain drivers for both controllers & connected devices. Fred, how do you feel about moving the generic OpenCAPI driver out of drivers/misc? -- Alastair D'Silva Open Source Developer Linux Technology Centre, IBM Australia mob: 0423 762 819
Re: [PATCH 07/11] pci/hotplug/pnv-php: Improve error msg on power state change failure
On 10/9/19 1:45 am, Frederic Barrat wrote: When changing the slot state, if opal hits an error and tells as such in the asynchronous reply, the warning "Wrong msg" is logged, which is rather confusing. Instead we can reuse the better message which is already used when we couldn't submit the asynchronous opal request initially. Signed-off-by: Frederic Barrat Reviewed-by: Andrew Donnellan -- Andrew Donnellan OzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
[Bug 205327] kmemleak reports various leaks in "swapper/0"
https://bugzilla.kernel.org/show_bug.cgi?id=205327 --- Comment #6 from Michael Ellerman (mich...@ellerman.id.au) --- I replied with directions on what to do. -- You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH 05/11] powerpc/powernv/ioda: Find opencapi slot for a device node
On 10/9/19 1:45 am, Frederic Barrat wrote:
diff --git a/arch/powerpc/platforms/powernv/pci.c
b/arch/powerpc/platforms/powernv/pci.c
index 6104418c9ad5..00a79f3c989f 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -48,13 +48,14 @@ int pnv_pci_get_slot_id(struct device_node *np, uint64_t
*id)
return -ENXIO;
bdfn = ((bdfn & 0x0000) >> 8);
- while ((parent = of_get_parent(parent))) {
+ for (parent = np; parent; parent = of_get_parent(parent)) {
I think we should rename this pointer from "parent" as it no longer
means that.
--
Andrew Donnellan OzLabs, ADL Canberra
a...@linux.ibm.com IBM Australia Limited
Re: [PATCH] of: unittest: fix memory leak in attach_node_and_children
Erhard Furtner writes:
> In attach_node_and_children memory is allocated for full_name via
> kasprintf. If the condition of the 1st if is not met the function
> returns early without freeing the memory. Add a kfree() to fix that.
It would be good to mention that this was detected with kmemleak.
It looks like the leak was introduced by this commit:
Fixes: 5babefb7f7ab ("of: unittest: allow base devicetree to have symbol
metadata")
> Signed-off-by: Erhard Furtner
> ---
> drivers/of/unittest.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
Reviewed-by: Michael Ellerman
Because this patch is to drivers/of, you need to send it to the right
folks. You can work out who with:
$ ./scripts/get_maintainer.pl -f drivers/of/unittest.c
robh...@kernel.org
frowand.l...@gmail.com
devicet...@vger.kernel.org
linux-ker...@vger.kernel.org
So to get it merged you should send a v2 (ie. with "PATCH v2" in the
subject), and Cc those people above as well as linuxppc-dev.
You should include the Fixes and Reviewed-by tags I've posted above in
your v2.
cheers
> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
> index 92e895d86458..ca7823eef2b4 100644
> --- a/drivers/of/unittest.c
> +++ b/drivers/of/unittest.c
> @@ -1146,8 +1146,10 @@ static void attach_node_and_children(struct
> device_node *np)
> full_name = kasprintf(GFP_KERNEL, "%pOF", np);
>
> if (!strcmp(full_name, "/__local_fixups__") ||
> - !strcmp(full_name, "/__fixups__"))
> + !strcmp(full_name, "/__fixups__")) {
> + kfree(full_name);
> return;
> + }
>
> dup = of_find_node_by_path(full_name);
> kfree(full_name);
> --
> 2.23.0
Re: [PATCH 03/12] powerpc: Replace cpu_up/down with device_online/offline
Qais Yousef writes:
> The core device API performs extra housekeeping bits that are missing
> from directly calling cpu_up/down.
>
> See commit a6717c01ddc2 ("powerpc/rtas: use device model APIs and
> serialization during LPM") for an example description of what might go
> wrong.
>
> This also prepares to make cpu_up/down a private interface for anything
> but the cpu subsystem.
>
> Signed-off-by: Qais Yousef
> CC: Benjamin Herrenschmidt
> CC: Paul Mackerras
> CC: Michael Ellerman
> CC: Enrico Weigelt
> CC: Ram Pai
> CC: Nicholas Piggin
> CC: Thiago Jung Bauermann
> CC: Christophe Leroy
> CC: Thomas Gleixner
> CC: linuxppc-dev@lists.ozlabs.org
> CC: linux-ker...@vger.kernel.org
> ---
> arch/powerpc/kernel/machine_kexec_64.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
My initial though is "what about kdump", but that function is not called
during kdump, so there should be no issue with the extra locking leading
to deadlocks in a crash.
Acked-by: Michael Ellerman
I assume you haven't actually tested it?
cheers
> diff --git a/arch/powerpc/kernel/machine_kexec_64.c
> b/arch/powerpc/kernel/machine_kexec_64.c
> index 04a7cba58eff..ebf8cc7acc4d 100644
> --- a/arch/powerpc/kernel/machine_kexec_64.c
> +++ b/arch/powerpc/kernel/machine_kexec_64.c
> @@ -208,13 +208,15 @@ static void wake_offline_cpus(void)
> {
> int cpu = 0;
>
> + lock_device_hotplug();
> for_each_present_cpu(cpu) {
> if (!cpu_online(cpu)) {
> printk(KERN_INFO "kexec: Waking offline cpu %d.\n",
> cpu);
> - WARN_ON(cpu_up(cpu));
> + WARN_ON(device_online(get_cpu_device(cpu)));
> }
> }
> + unlock_device_hotplug();
> }
>
> static void kexec_prepare_cpus(void)
> --
> 2.17.1
Re: [PATCH v5 17/24] mm/gup: track FOLL_PIN pages
On 11/18/19 3:58 AM, Jan Kara wrote:
> On Thu 14-11-19 21:53:33, John Hubbard wrote:
>> Add tracking of pages that were pinned via FOLL_PIN.
>>
>> As mentioned in the FOLL_PIN documentation, callers who effectively set
>> FOLL_PIN are required to ultimately free such pages via put_user_page().
>> The effect is similar to FOLL_GET, and may be thought of as "FOLL_GET
>> for DIO and/or RDMA use".
>>
>> Pages that have been pinned via FOLL_PIN are identifiable via a
>> new function call:
>>
>>bool page_dma_pinned(struct page *page);
>>
>> What to do in response to encountering such a page, is left to later
>> patchsets. There is discussion about this in [1].
> ^^ missing this reference
> in the changelog...
I'll add that.
>
>> This also changes a BUG_ON(), to a WARN_ON(), in follow_page_mask().
>>
>> Suggested-by: Jan Kara
>> Suggested-by: Jérôme Glisse
>> Signed-off-by: John Hubbard
>> ---
>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>> index 6588d2e02628..db872766480f 100644
>> --- a/include/linux/mm.h
>> +++ b/include/linux/mm.h
>> @@ -1054,6 +1054,8 @@ static inline __must_check bool try_get_page(struct
>> page *page)
>> return true;
>> }
>>
>> +__must_check bool user_page_ref_inc(struct page *page);
>> +
>> static inline void put_page(struct page *page)
>> {
>> page = compound_head(page);
>> @@ -1071,29 +1073,70 @@ static inline void put_page(struct page *page)
>> __put_page(page);
>> }
>>
>> -/**
>> - * put_user_page() - release a gup-pinned page
>> - * @page:pointer to page to be released
>> +/*
>> + * GUP_PIN_COUNTING_BIAS, and the associated functions that use it, overload
>> + * the page's refcount so that two separate items are tracked: the original
>> page
>> + * reference count, and also a new count of how many get_user_pages() calls
>> were
> ^^ pin_user_pages()
>
>> + * made against the page. ("gup-pinned" is another term for the latter).
>> + *
>> + * With this scheme, get_user_pages() becomes special: such pages are marked
> ^^^ pin_user_pages()
>
>> + * as distinct from normal pages. As such, the put_user_page() call (and its
>> + * variants) must be used in order to release gup-pinned pages.
>> + *
>> + * Choice of value:
>> *
>> - * Pages that were pinned via pin_user_pages*() must be released via either
>> - * put_user_page(), or one of the put_user_pages*() routines. This is so
>> that
>> - * eventually such pages can be separately tracked and uniquely handled. In
>> - * particular, interactions with RDMA and filesystems need special handling.
>> + * By making GUP_PIN_COUNTING_BIAS a power of two, debugging of page
>> reference
>> + * counts with respect to get_user_pages() and put_user_page() becomes
>> simpler,
> ^^^ pin_user_pages()
>
Yes.
>> + * due to the fact that adding an even power of two to the page refcount has
>> + * the effect of using only the upper N bits, for the code that counts up
>> using
>> + * the bias value. This means that the lower bits are left for the exclusive
>> + * use of the original code that increments and decrements by one (or at
>> least,
>> + * by much smaller values than the bias value).
>> *
>> - * put_user_page() and put_page() are not interchangeable, despite this
>> early
>> - * implementation that makes them look the same. put_user_page() calls must
>> - * be perfectly matched up with pin*() calls.
>> + * Of course, once the lower bits overflow into the upper bits (and this is
>> + * OK, because subtraction recovers the original values), then visual
>> inspection
>> + * no longer suffices to directly view the separate counts. However, for
>> normal
>> + * applications that don't have huge page reference counts, this won't be an
>> + * issue.
>> + *
>> + * Locking: the lockless algorithm described in page_cache_get_speculative()
>> + * and page_cache_gup_pin_speculative() provides safe operation for
>> + * get_user_pages and page_mkclean and other calls that race to set up page
>> + * table entries.
>> */
> ...
>> @@ -2070,9 +2191,16 @@ static int gup_hugepte(pte_t *ptep, unsigned long sz,
>> unsigned long addr,
>> page = head + ((addr & (sz-1)) >> PAGE_SHIFT);
>> refs = __record_subpages(page, addr, end, pages + *nr);
>>
>> -head = try_get_compound_head(head, refs);
>> -if (!head)
>> -return 0;
>> +if (flags & FOLL_PIN) {
>> +head = page;
>> +if (unlikely(!user_page_ref_inc(head)))
>> +return 0;
>> +head = page;
>
> Why do you assign 'head' twice? Also the refcounting logic is repeated
> several times so perhaps you can factor it out in to a helper function or
> even move it to __record_subpages()?
OK.
>
>> +} else {
>> +head = try_get_compound_head(head, refs);
>> +if (!head)
>> +
Re: [PATCH 08/10] nvdimm: Add driver for OpenCAPI Storage Class Memory
On Mon, Nov 18, 2019 at 3:48 PM Andrew Donnellan wrote: > > On 15/11/19 3:35 am, Dan Williams wrote: > >> Have you discussed with the directory owner if it's ok to split the > >> driver over several files? > > > > My thought is to establish drivers/opencapi/ and move this and the > > existing drivers/misc/ocxl/ bits there. > > Is there any other justification for this we can think of apart from not > wanting to put this driver in the nvdimm directory? OpenCAPI drivers > aren't really a category of driver unto themselves. The concern is less about adding to drivers/nvdimm/ and more about the proper location to house opencapi specific transport and enumeration details. The organization I'm looking for is to group platform transport and enumeration code together similar to how drivers/pci/ exists independent of all pci drivers that use that common core. For libnvdimm the enumeration is platform specific and calls into the nvdimm core. This is why the x86 platform persistent memory bus driver lives under drivers/acpi/nfit/ instead of drivers/nvdimm/. The nfit driver is an ACPI extension that translates ACPI details into libnvdimm core objects. The usage of "ocxl" in the source leads me to think part of this driver belongs in a directory that has other opencapi specific considerations.
Re: [PATCH 08/10] nvdimm: Add driver for OpenCAPI Storage Class Memory
On 15/11/19 3:35 am, Dan Williams wrote: Have you discussed with the directory owner if it's ok to split the driver over several files? My thought is to establish drivers/opencapi/ and move this and the existing drivers/misc/ocxl/ bits there. Is there any other justification for this we can think of apart from not wanting to put this driver in the nvdimm directory? OpenCAPI drivers aren't really a category of driver unto themselves. -- Andrew Donnellan OzLabs, ADL Canberra a...@linux.ibm.com IBM Australia Limited
Re: [PATCH 08/10] nvdimm: Add driver for OpenCAPI Storage Class Memory
On Thu, 2019-11-14 at 14:41 +0100, Frederic Barrat wrote: > Hi Alastair, > > The patch is huge and could/should probably be split in smaller > pieces > to ease the review. However, having sinned on that same topic in the > past, I made a first pass anyway. I haven't covered everything but > tried > to focus on the general setup of the driver for now. > Since the patch is very long, I'm writing all the comments in one > chunk > here instead of spreading them over a few thousand lines, where some > would be easy to miss. > > > Update MAINTAINERS for the new files > > Have you discussed with the directory owner if it's ok to split the > driver over several files? > Not yet, I do have a request as to whether drivers/nvdimm is actually the right place for this though. > > Kconfig > === > Does it make sense to keep OCXL_SCM_DEBUG separate? Why not enabling > it > by default? I think so, these features are a bit dangerous for general users, but very useful for developers. > > > ocxl_scm.c > == > > scm_file_init() > --- > on error paths, should call idr_destroy() (same pb found in base > ocxl > driver) > Ok. > > scm_probe() and _function_0() > - > > The different init between function 0 and 1 looks a bit off and it > seems > they could be unified. Function 1 does the same thing as function 0 > (call ocxl_function_open()) then some AFU-specific init, which we > could > skip if there's no AFU found on the function. And log a WARN if we > find > an AFU on something else than function 1, since, unlike ocxl, we > know > exactly what the device is like for SCM. But keep an common probe() > function. It would also simplify the flow on scm_remove() and avoid > problems (currently ocxl_function_close() is not called for function > 1). Hmm, the 2 functions do very different things. Function 0 only exists for link negotiation, and there is a huge amount of other bits & pieces in the scm_data struct that will never be used for it. ocxl_function_close() is called in free_scm() via the release handler for the device. > > scm-data->timeouts: array of size 9 declared, we only init 7 > members. > With the zalloc() the others are at 0, is that correct? > Yes, these will be queried dynamically from the card in a later patch, but that feature is not yet ready for testing. The timeouts that are currently 0 are never read in the current implementation of the driver. > > Something looks wrong regarding data release in the error path(s). > IIUC, > we register the device early and rely on the release callback of the > device to free all resources (in free_scm_dev()). We should probably > have a comment in probe() to make it obvious. Or maybe better, have > a I'll add a comment. > subfunction to keep doing the rest of the inits which are dependent > on > the device release to be cleaned up. In the subsequent error paths > in > scm_probe(), we are missing a device_unregister() > This is intentional, I want to keep the device online (but not registered with libnvdimm) in the event of an error as the card can be interrogated via IOCTLs to find out what wrong. > Could log 120 times the same "Waiting for SCM to become usable" > message, > which is not really helping much. > I'll quieten that, it was useful during development to identify whether the machine had locked up or was still waiting on the card. > > free_scm() > - > Related to above comment in probe(), it would help to be able to > easily > match the what's done in probe vs. undone here. For example, in > probe(), > there's scm_setup_irq(), where we do all things related to > interrupts. > But we don't have a subfunction to clean the interrupts state. It > would > help for readability and track potential misses. I didn't tried to > match > all of them, but the following calls seem missing: > > ocxl_context_detach() > ocxl_afu_irq_free() Hmm, we call ocxl_context_detach_all() in ocxl/core.c:remove_afu() (via ocxl_function_close), but by that stage, I've already called ocxl_context_free(), so that's clearly a bug. I'll add in the missing ocxl_context_detach call in the scm_driver, and in a seperate patch, free the context in ocxl_context_detach_all(). I'll also add in the missing calls to ocxl_afu_irq_free(), but I wonder whether we should also clean all the IRQ allocations in remove_afu() too? > > > ocxl_remove() > - > see comment above about unifying function 0 and 1 case. > Why is nvdimm_bus_unregister() treated separately? Can't it be part > of > the "normal" freeing of resources done implicitly when calling > device_unregister() in the free_scm() callback? > Yeah, good observation. > > scm_setup_device_metadata() > --- > function doesn't do any setup, so the name is misleading. > renamed to read_device_metadata(). > for (i = 0; i < 8; i++) > scm_data->fw_version[i] = (val >> (i * 8)) & 0xff;
Re: [PATCH 00/12] Convert cpu_up/down to device_online/offline
On Mon, 18 Nov 2019, Qais Yousef wrote:
> I had to make an educated guess that you're probably the 'maintainer' of cpu
> hotplug - but there's no explicit entry that says that. Please let me know if
> I need to bring the attention of others too.
:)
> The series do have few rough edges to address, but it's relatively
> straightforward and I think does offer a nice improvement in the form of
> consolidating the API for bringing up/down cpus from external
> subsystems/drivers. Beside fix the inconsistency of device's core view of the
> state of the cpu which can happen when cpu_{up/down} are called directly.
>
> The downside I see is that the external API to bring cpus up/down for
> suspend/resume and at boot seem to have grown a bit organically (I've added
> a couple in this series to address 2 direct users of cpu_{up,down}). We might
> need to rethink this API, but I think this is outside the scope of this
> series.
>
> Any thoughts/feedback would be appreciated.
Let me have a look.
[Bug 205327] kmemleak reports various leaks in "swapper/0"
https://bugzilla.kernel.org/show_bug.cgi?id=205327 --- Comment #5 from Erhard F. (erhar...@mailbox.org) --- (In reply to Michael Ellerman from comment #3) > Do you want to send a patch? I already sent the patch to linuxppc-dev@lists.ozlabs.org (https://patchwork.ozlabs.org/patch/1195212/) but don't know how to proceed further to get this patch into stable, etc. -- You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH 01/11] powerpc/powernv/ioda: Fix ref count for devices with their own PE
Le 18/11/2019 à 03:36, Alistair Popple a écrit : On Monday, 18 November 2019 12:24:24 PM AEDT Oliver O'Halloran wrote: On Mon, Nov 18, 2019 at 12:06 PM Alistair Popple wrote: On Wednesday, 13 November 2019 4:38:21 AM AEDT Frederic Barrat wrote: However, one question is whether this patch breaks nvlink and if nvlink assumes the devices won’t go away because we explicitly take a reference forever. In npu_dma.c, there are 2 functions which allow to find the GPU associated to a npu device, and vice-versa. Both functions return a pointer to a struct pci_dev, but they don’t take a reference on the device being returned. So that seems dangerous. I’m probably missing something. Alexey, Alistair: what, if anything, guarantees, that the npu or gpu devices stay valid. Is it because we simply don’t provide any means to get rid of them ? Otherwise, don’t we need the callers of pnv_pci_get_gpu_dev() and pnv_pci_get_npu_dev() to worry about reference counting ? I’ve started looking into it and the changes are scary, which explains Greg’s related commit 02c5f5394918b. To be honest the reference counting looks like it has evolved into something quite suspect and I don't think you're missing anything. In practice though we likely haven't hit any issues because the original callers didn't store references to the pdev which would make the window quite small (although the pass through work may have changed that). And as you say there simply wasn't any means to get rid of them anyway (EEH, hotplug, etc. has never been implemented or supported for GPUs and all sorts of terrible things happen if you try). In other words: leaking a ref is the only safe thing to do. Correct. The best solution would likely be to review the reference counting and to teach callers of get_*_dev() to call pci_put_dev(), etc. The issue is that the two callers of get_pci_dev() are non-GPL exported symbols so we don't know what's calling them or what their expectations are. We be doing whatever makes sense for OpenCAPI and if that happens to cause a problem for someone else, then they can deal with it. The issue isn't that it's exported non-GPL vs. GPL, rather that there are probably out-of-tree modules like the NVIDIA driver using it. However as you say supporting out-of-tree modules is not generally a concern for kernel developers so we certainly shouldn't let that prevent us doing a fix. Thanks Alistair and Oliver. I'll bite the bullet and try do the right thing wrt ref counting in npu-dma.c Fred - Alistair Oliver
Re: powerpc ftrace broken due to "manual merge of the ftrace tree with the arm64 tree"
On Mon, 2019-11-18 at 10:16 -0500, Steven Rostedt wrote: > On Mon, 18 Nov 2019 09:58:42 -0500 > Steven Rostedt wrote: > > > On Mon, 18 Nov 2019 09:51:04 -0500 > > Steven Rostedt wrote: > > > > > > > Test this commit please: b83b43ffc6e4b514ca034a0fbdee01322e2f7022 > > > > > > > > > > > > > # git reset --hard b83b43ffc6e4b514ca034a0fbdee01322e2f7022 > > > > > > > > Yes, that one is bad. > > > > > > Can you see if this patch fixes the issue for you? > > > > Don't bother. This isn't the right fix, I know see the real issue. > > > > New fix coming shortly. > > > > Can you try this? Yes, it works fine. > > It appears that I picked a name "ftrace_graph_stub", that was already in > use by powerpc. This just renames the function stub I used. > > -- Steve > > diff --git a/include/asm-generic/vmlinux.lds.h > b/include/asm-generic/vmlinux.lds.h > index 0f358be551cd..996db32c491b 100644 > --- a/include/asm-generic/vmlinux.lds.h > +++ b/include/asm-generic/vmlinux.lds.h > @@ -112,7 +112,7 @@ > #ifdef CONFIG_FTRACE_MCOUNT_RECORD > #ifdef CC_USING_PATCHABLE_FUNCTION_ENTRY > /* > - * Need to also make ftrace_graph_stub point to ftrace_stub > + * Need to also make ftrace_stub_graph point to ftrace_stub > * so that the same stub location may have different protocols > * and not mess up with C verifiers. > */ > @@ -120,17 +120,17 @@ > __start_mcount_loc = .; \ > KEEP(*(__patchable_function_entries)) \ > __stop_mcount_loc = .; \ > - ftrace_graph_stub = ftrace_stub; > + ftrace_stub_graph = ftrace_stub; > #else > #define MCOUNT_REC() . = ALIGN(8); \ > __start_mcount_loc = .; \ > KEEP(*(__mcount_loc)) \ > __stop_mcount_loc = .; \ > - ftrace_graph_stub = ftrace_stub; > + ftrace_stub_graph = ftrace_stub; > #endif > #else > # ifdef CONFIG_FUNCTION_TRACER > -# define MCOUNT_REC() ftrace_graph_stub = ftrace_stub; > +# define MCOUNT_REC() ftrace_stub_graph = ftrace_stub; > # else > # define MCOUNT_REC() > # endif > diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c > index fa3ce10d0405..67e0c462b059 100644 > --- a/kernel/trace/fgraph.c > +++ b/kernel/trace/fgraph.c > @@ -336,10 +336,10 @@ int ftrace_graph_entry_stub(struct ftrace_graph_ent > *trace) > * Simply points to ftrace_stub, but with the proper protocol. > * Defined by the linker script in linux/vmlinux.lds.h > */ > -extern void ftrace_graph_stub(struct ftrace_graph_ret *); > +extern void ftrace_stub_graph(struct ftrace_graph_ret *); > > /* The callbacks that hook a function */ > -trace_func_graph_ret_t ftrace_graph_return = ftrace_graph_stub; > +trace_func_graph_ret_t ftrace_graph_return = ftrace_stub_graph; > trace_func_graph_ent_t ftrace_graph_entry = ftrace_graph_entry_stub; > static trace_func_graph_ent_t __ftrace_graph_entry = ftrace_graph_entry_stub; > > @@ -619,7 +619,7 @@ void unregister_ftrace_graph(struct fgraph_ops *gops) > goto out; > > ftrace_graph_active--; > - ftrace_graph_return = ftrace_graph_stub; > + ftrace_graph_return = ftrace_stub_graph; > ftrace_graph_entry = ftrace_graph_entry_stub; > __ftrace_graph_entry = ftrace_graph_entry_stub; > ftrace_shutdown(&graph_ops, FTRACE_STOP_FUNC_RET);
Re: [PATCH 00/12] Convert cpu_up/down to device_online/offline
Hi Thomas
On 10/30/19 15:38, Qais Yousef wrote:
> Using cpu_up/down directly to bring cpus online/offline loses synchronization
> with sysfs and could suffer from a race similar to what is described in
> commit a6717c01ddc2 ("powerpc/rtas: use device model APIs and serialization
> during LPM").
>
> cpu_up/down seem to be more of a internal implementation detail for the cpu
> subsystem to use to boot up cpus, perform suspend/resume and low level hotplug
> operations. Users outside of the cpu subsystem would be better using the
> device
> core API to bring a cpu online/offline which is the interface used to hotplug
> memory and other system devices.
>
> Several users have already migrated to use the device core API, this series
> converts the remaining users and hides cpu_up/down from internal users at the
> end.
>
> I still need to update the documentation to remove references to cpu_up/down
> and advocate for device_online/offline instead if this series will make its
> way
> through.
>
> I noticed this problem while working on a hack to disable offlining
> a particular CPU but noticed that setting the offline_disabled attribute in
> the
> device struct isn't enough because users can easily bypass the device core.
> While my hack isn't a valid use case but it did highlight the inconsistency in
> the way cpus are being onlined/offlined and this attempt hopefully improves on
> this.
>
> The first 6 patches fixes arch users.
>
> The next 5 patches fixes generic code users. Particularly creating a new
> special exported API for the device core to use instead of cpu_up/down.
> Maybe we can do something more restrictive than that.
>
> The last patch removes cpu_up/down from cpu.h and unexport the functions.
>
> In some cases where the use of cpu_up/down seemed legitimate, I encapsulated
> the logic in a higher level - special purposed function; and converted the
> code
> to use that instead.
>
> I did run the rcu torture, lock torture and psci checker tests and no problem
> was noticed. I did perform build tests on all arch affected except for parisc.
>
> Hopefully I got the CC list right for all the patches. Apologies in advance if
> some people were omitted from some patches but they should have been CCed.
I had to make an educated guess that you're probably the 'maintainer' of cpu
hotplug - but there's no explicit entry that says that. Please let me know if
I need to bring the attention of others too.
The series do have few rough edges to address, but it's relatively
straightforward and I think does offer a nice improvement in the form of
consolidating the API for bringing up/down cpus from external
subsystems/drivers. Beside fix the inconsistency of device's core view of the
state of the cpu which can happen when cpu_{up/down} are called directly.
The downside I see is that the external API to bring cpus up/down for
suspend/resume and at boot seem to have grown a bit organically (I've added
a couple in this series to address 2 direct users of cpu_{up,down}). We might
need to rethink this API, but I think this is outside the scope of this series.
Any thoughts/feedback would be appreciated.
Thanks
--
Qais Yousef
Re: powerpc ftrace broken due to "manual merge of the ftrace tree with the arm64 tree"
On Mon, 18 Nov 2019 09:58:42 -0500 Steven Rostedt wrote: > On Mon, 18 Nov 2019 09:51:04 -0500 > Steven Rostedt wrote: > > > > > Test this commit please: b83b43ffc6e4b514ca034a0fbdee01322e2f7022 > > > > > > # git reset --hard b83b43ffc6e4b514ca034a0fbdee01322e2f7022 > > > > > > Yes, that one is bad. > > > > Can you see if this patch fixes the issue for you? > > Don't bother. This isn't the right fix, I know see the real issue. > > New fix coming shortly. > Can you try this? It appears that I picked a name "ftrace_graph_stub", that was already in use by powerpc. This just renames the function stub I used. -- Steve diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index 0f358be551cd..996db32c491b 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -112,7 +112,7 @@ #ifdef CONFIG_FTRACE_MCOUNT_RECORD #ifdef CC_USING_PATCHABLE_FUNCTION_ENTRY /* - * Need to also make ftrace_graph_stub point to ftrace_stub + * Need to also make ftrace_stub_graph point to ftrace_stub * so that the same stub location may have different protocols * and not mess up with C verifiers. */ @@ -120,17 +120,17 @@ __start_mcount_loc = .; \ KEEP(*(__patchable_function_entries)) \ __stop_mcount_loc = .; \ - ftrace_graph_stub = ftrace_stub; + ftrace_stub_graph = ftrace_stub; #else #define MCOUNT_REC() . = ALIGN(8); \ __start_mcount_loc = .; \ KEEP(*(__mcount_loc)) \ __stop_mcount_loc = .; \ - ftrace_graph_stub = ftrace_stub; + ftrace_stub_graph = ftrace_stub; #endif #else # ifdef CONFIG_FUNCTION_TRACER -# define MCOUNT_REC() ftrace_graph_stub = ftrace_stub; +# define MCOUNT_REC() ftrace_stub_graph = ftrace_stub; # else # define MCOUNT_REC() # endif diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c index fa3ce10d0405..67e0c462b059 100644 --- a/kernel/trace/fgraph.c +++ b/kernel/trace/fgraph.c @@ -336,10 +336,10 @@ int ftrace_graph_entry_stub(struct ftrace_graph_ent *trace) * Simply points to ftrace_stub, but with the proper protocol. * Defined by the linker script in linux/vmlinux.lds.h */ -extern void ftrace_graph_stub(struct ftrace_graph_ret *); +extern void ftrace_stub_graph(struct ftrace_graph_ret *); /* The callbacks that hook a function */ -trace_func_graph_ret_t ftrace_graph_return = ftrace_graph_stub; +trace_func_graph_ret_t ftrace_graph_return = ftrace_stub_graph; trace_func_graph_ent_t ftrace_graph_entry = ftrace_graph_entry_stub; static trace_func_graph_ent_t __ftrace_graph_entry = ftrace_graph_entry_stub; @@ -619,7 +619,7 @@ void unregister_ftrace_graph(struct fgraph_ops *gops) goto out; ftrace_graph_active--; - ftrace_graph_return = ftrace_graph_stub; + ftrace_graph_return = ftrace_stub_graph; ftrace_graph_entry = ftrace_graph_entry_stub; __ftrace_graph_entry = ftrace_graph_entry_stub; ftrace_shutdown(&graph_ops, FTRACE_STOP_FUNC_RET);
Re: powerpc ftrace broken due to "manual merge of the ftrace tree with the arm64 tree"
On Mon, 18 Nov 2019 09:51:04 -0500 Steven Rostedt wrote: > > > Test this commit please: b83b43ffc6e4b514ca034a0fbdee01322e2f7022 > > > > # git reset --hard b83b43ffc6e4b514ca034a0fbdee01322e2f7022 > > > > Yes, that one is bad. > > Can you see if this patch fixes the issue for you? Don't bother. This isn't the right fix, I know see the real issue. New fix coming shortly. -- Steve
Re: powerpc ftrace broken due to "manual merge of the ftrace tree with the arm64 tree"
On Fri, 15 Nov 2019 16:06:34 -0500 Qian Cai wrote: > > Test this commit please: b83b43ffc6e4b514ca034a0fbdee01322e2f7022 > > # git reset --hard b83b43ffc6e4b514ca034a0fbdee01322e2f7022 > > Yes, that one is bad. Can you see if this patch fixes the issue for you? Thanks! -- Steve diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index 0f358be551cd..fd8f4dc661dc 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -109,6 +109,13 @@ #define MEM_DISCARD(sec) *(.mem##sec) #endif +/* PowerPC defines ftrace_graph_stub in the code */ +#ifndef CONFIG_PPC +# define DEFINE_FTRACE_GRAPH_STUB ftrace_graph_stub = ftrace_stub; +#else +# define DEFINE_FTRACE_GRAPH_STUB +#endif + #ifdef CONFIG_FTRACE_MCOUNT_RECORD #ifdef CC_USING_PATCHABLE_FUNCTION_ENTRY /* @@ -120,17 +127,17 @@ __start_mcount_loc = .; \ KEEP(*(__patchable_function_entries)) \ __stop_mcount_loc = .; \ - ftrace_graph_stub = ftrace_stub; + DEFINE_FTRACE_GRAPH_STUB #else #define MCOUNT_REC() . = ALIGN(8); \ __start_mcount_loc = .; \ KEEP(*(__mcount_loc)) \ __stop_mcount_loc = .; \ - ftrace_graph_stub = ftrace_stub; + DEFINE_FTRACE_GRAPH_STUB #endif #else # ifdef CONFIG_FUNCTION_TRACER -# define MCOUNT_REC() ftrace_graph_stub = ftrace_stub; +# define MCOUNT_REC() DEFINE_FTRACE_GRAPH_STUB # else # define MCOUNT_REC() # endif
Re: [PATCH v5 11/24] goldish_pipe: convert to pin_user_pages() and put_user_page()
On Thu 14-11-19 21:53:27, John Hubbard wrote:
> 1. Call the new global pin_user_pages_fast(), from pin_goldfish_pages().
>
> 2. As required by pin_user_pages(), release these pages via
> put_user_page(). In this case, do so via put_user_pages_dirty_lock().
>
> That has the side effect of calling set_page_dirty_lock(), instead
> of set_page_dirty(). This is probably more accurate.
>
> As Christoph Hellwig put it, "set_page_dirty() is only safe if we are
> dealing with a file backed page where we have reference on the inode it
> hangs off." [1]
>
> Another side effect is that the release code is simplified because
> the page[] loop is now in gup.c instead of here, so just delete the
> local release_user_pages() entirely, and call
> put_user_pages_dirty_lock() directly, instead.
>
> [1] https://lore.kernel.org/r/20190723153640.gb...@lst.de
>
> Reviewed-by: Ira Weiny
> Signed-off-by: John Hubbard
Looks good to me. You can add:
Reviewed-by: Jan Kara
Honza
> ---
> drivers/platform/goldfish/goldfish_pipe.c | 17 +++--
> 1 file changed, 3 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/platform/goldfish/goldfish_pipe.c
> b/drivers/platform/goldfish/goldfish_pipe.c
> index 7ed2a21a0bac..635a8bc1b480 100644
> --- a/drivers/platform/goldfish/goldfish_pipe.c
> +++ b/drivers/platform/goldfish/goldfish_pipe.c
> @@ -274,7 +274,7 @@ static int pin_goldfish_pages(unsigned long first_page,
> *iter_last_page_size = last_page_size;
> }
>
> - ret = get_user_pages_fast(first_page, requested_pages,
> + ret = pin_user_pages_fast(first_page, requested_pages,
> !is_write ? FOLL_WRITE : 0,
> pages);
> if (ret <= 0)
> @@ -285,18 +285,6 @@ static int pin_goldfish_pages(unsigned long first_page,
> return ret;
> }
>
> -static void release_user_pages(struct page **pages, int pages_count,
> -int is_write, s32 consumed_size)
> -{
> - int i;
> -
> - for (i = 0; i < pages_count; i++) {
> - if (!is_write && consumed_size > 0)
> - set_page_dirty(pages[i]);
> - put_page(pages[i]);
> - }
> -}
> -
> /* Populate the call parameters, merging adjacent pages together */
> static void populate_rw_params(struct page **pages,
> int pages_count,
> @@ -372,7 +360,8 @@ static int transfer_max_buffers(struct goldfish_pipe
> *pipe,
>
> *consumed_size = pipe->command_buffer->rw_params.consumed_size;
>
> - release_user_pages(pipe->pages, pages_count, is_write, *consumed_size);
> + put_user_pages_dirty_lock(pipe->pages, pages_count,
> + !is_write && *consumed_size > 0);
>
> mutex_unlock(&pipe->lock);
> return 0;
> --
> 2.24.0
>
--
Jan Kara
SUSE Labs, CR
Re: [PATCH v5 15/24] fs/io_uring: set FOLL_PIN via pin_user_pages()
On Thu 14-11-19 21:53:31, John Hubbard wrote:
> Convert fs/io_uring to use the new pin_user_pages() call, which sets
> FOLL_PIN. Setting FOLL_PIN is now required for code that requires
> tracking of pinned pages, and therefore for any code that calls
> put_user_page().
>
> In partial anticipation of this work, the io_uring code was already
> calling put_user_page() instead of put_page(). Therefore, in order to
> convert from the get_user_pages()/put_page() model, to the
> pin_user_pages()/put_user_page() model, the only change required
> here is to change get_user_pages() to pin_user_pages().
>
> Signed-off-by: John Hubbard
Looks good to me. You can add:
Reviewed-by: Jan Kara
Honza
> ---
> fs/io_uring.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index f9a38998f2fc..cff64bd00db9 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -3433,7 +3433,7 @@ static int io_sqe_buffer_register(struct io_ring_ctx
> *ctx, void __user *arg,
>
> ret = 0;
> down_read(¤t->mm->mmap_sem);
> - pret = get_user_pages(ubuf, nr_pages,
> + pret = pin_user_pages(ubuf, nr_pages,
> FOLL_WRITE | FOLL_LONGTERM,
> pages, vmas);
> if (pret == nr_pages) {
> --
> 2.24.0
>
--
Jan Kara
SUSE Labs, CR
Re: [PATCH v5 06/24] goldish_pipe: rename local pin_user_pages() routine
On Thu 14-11-19 21:53:22, John Hubbard wrote:
> 1. Avoid naming conflicts: rename local static function from
> "pin_user_pages()" to "pin_goldfish_pages()".
>
> An upcoming patch will introduce a global pin_user_pages()
> function.
>
> Reviewed-by: Jérôme Glisse
> Reviewed-by: Ira Weiny
> Signed-off-by: John Hubbard
Looks good to me. You can add:
Reviewed-by: Jan Kara
Honza
> ---
> drivers/platform/goldfish/goldfish_pipe.c | 18 +-
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/platform/goldfish/goldfish_pipe.c
> b/drivers/platform/goldfish/goldfish_pipe.c
> index cef0133aa47a..7ed2a21a0bac 100644
> --- a/drivers/platform/goldfish/goldfish_pipe.c
> +++ b/drivers/platform/goldfish/goldfish_pipe.c
> @@ -257,12 +257,12 @@ static int goldfish_pipe_error_convert(int status)
> }
> }
>
> -static int pin_user_pages(unsigned long first_page,
> - unsigned long last_page,
> - unsigned int last_page_size,
> - int is_write,
> - struct page *pages[MAX_BUFFERS_PER_COMMAND],
> - unsigned int *iter_last_page_size)
> +static int pin_goldfish_pages(unsigned long first_page,
> + unsigned long last_page,
> + unsigned int last_page_size,
> + int is_write,
> + struct page *pages[MAX_BUFFERS_PER_COMMAND],
> + unsigned int *iter_last_page_size)
> {
> int ret;
> int requested_pages = ((last_page - first_page) >> PAGE_SHIFT) + 1;
> @@ -354,9 +354,9 @@ static int transfer_max_buffers(struct goldfish_pipe
> *pipe,
> if (mutex_lock_interruptible(&pipe->lock))
> return -ERESTARTSYS;
>
> - pages_count = pin_user_pages(first_page, last_page,
> - last_page_size, is_write,
> - pipe->pages, &iter_last_page_size);
> + pages_count = pin_goldfish_pages(first_page, last_page,
> + last_page_size, is_write,
> + pipe->pages, &iter_last_page_size);
> if (pages_count < 0) {
> mutex_unlock(&pipe->lock);
> return pages_count;
> --
> 2.24.0
>
--
Jan Kara
SUSE Labs, CR
Re: [PATCH v5 13/24] mm/process_vm_access: set FOLL_PIN via pin_user_pages_remote()
On Thu 14-11-19 21:53:29, John Hubbard wrote: > Convert process_vm_access to use the new pin_user_pages_remote() > call, which sets FOLL_PIN. Setting FOLL_PIN is now required for > code that requires tracking of pinned pages. > > Also, release the pages via put_user_page*(). > > Also, rename "pages" to "pinned_pages", as this makes for > easier reading of process_vm_rw_single_vec(). > > Reviewed-by: Jérôme Glisse > Reviewed-by: Ira Weiny > Signed-off-by: John Hubbard > --- > mm/process_vm_access.c | 28 +++- > 1 file changed, 15 insertions(+), 13 deletions(-) The patch looks good to me. You can add: Reviewed-by: Jan Kara Honza -- Jan Kara SUSE Labs, CR
Re: [PATCH v5 10/24] mm/gup: introduce pin_user_pages*() and FOLL_PIN
On Thu 14-11-19 21:53:26, John Hubbard wrote: > /* > - * NOTE on FOLL_LONGTERM: > + * FOLL_PIN and FOLL_LONGTERM may be used in various combinations with each > + * other. Here is what they mean, and how to use them: > * > * FOLL_LONGTERM indicates that the page will be held for an indefinite time > - * period _often_ under userspace control. This is contrasted with > - * iov_iter_get_pages() where usages which are transient. > + * period _often_ under userspace control. This is in contrast to > + * iov_iter_get_pages(), where usages which are transient. ^^^ when you touch this, please fix also the second sentense. It doesn't quite make sense to me... I'd probably write there "whose usages are transient" but maybe you can come up with something even better. Otherwise the patch looks good to me so feel free to add: Reviewed-by: Jan Kara Honza -- Jan Kara SUSE Labs, CR
Re: [PATCH v5 07/24] IB/umem: use get_user_pages_fast() to pin DMA pages
On Thu 14-11-19 21:53:23, John Hubbard wrote:
> And get rid of the mmap_sem calls, as part of that. Note
> that get_user_pages_fast() will, if necessary, fall back to
> __gup_longterm_unlocked(), which takes the mmap_sem as needed.
>
> Reviewed-by: Jason Gunthorpe
> Reviewed-by: Ira Weiny
> Signed-off-by: John Hubbard
Looks good to me. You can add:
Reviewed-by: Jan Kara
Honza
> ---
> drivers/infiniband/core/umem.c | 17 ++---
> 1 file changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/infiniband/core/umem.c b/drivers/infiniband/core/umem.c
> index 24244a2f68cc..3d664a2539eb 100644
> --- a/drivers/infiniband/core/umem.c
> +++ b/drivers/infiniband/core/umem.c
> @@ -271,16 +271,13 @@ struct ib_umem *ib_umem_get(struct ib_udata *udata,
> unsigned long addr,
> sg = umem->sg_head.sgl;
>
> while (npages) {
> - down_read(&mm->mmap_sem);
> - ret = get_user_pages(cur_base,
> - min_t(unsigned long, npages,
> -PAGE_SIZE / sizeof (struct page *)),
> - gup_flags | FOLL_LONGTERM,
> - page_list, NULL);
> - if (ret < 0) {
> - up_read(&mm->mmap_sem);
> + ret = get_user_pages_fast(cur_base,
> + min_t(unsigned long, npages,
> + PAGE_SIZE /
> + sizeof(struct page *)),
> + gup_flags | FOLL_LONGTERM, page_list);
> + if (ret < 0)
> goto umem_release;
> - }
>
> cur_base += ret * PAGE_SIZE;
> npages -= ret;
> @@ -288,8 +285,6 @@ struct ib_umem *ib_umem_get(struct ib_udata *udata,
> unsigned long addr,
> sg = ib_umem_add_sg_table(sg, page_list, ret,
> dma_get_max_seg_size(context->device->dma_device),
> &umem->sg_nents);
> -
> - up_read(&mm->mmap_sem);
> }
>
> sg_mark_end(sg);
> --
> 2.24.0
>
--
Jan Kara
SUSE Labs, CR
Re: [PATCH v5 02/24] mm/gup: factor out duplicate code from four routines
On Thu 14-11-19 21:53:18, John Hubbard wrote:
> There are four locations in gup.c that have a fair amount of code
> duplication. This means that changing one requires making the same
> changes in four places, not to mention reading the same code four
> times, and wondering if there are subtle differences.
>
> Factor out the common code into static functions, thus reducing the
> overall line count and the code's complexity.
>
> Also, take the opportunity to slightly improve the efficiency of the
> error cases, by doing a mass subtraction of the refcount, surrounded
> by get_page()/put_page().
>
> Also, further simplify (slightly), by waiting until the the successful
> end of each routine, to increment *nr.
>
> Reviewed-by: Jérôme Glisse
> Cc: Jan Kara
> Cc: Ira Weiny
> Cc: Christoph Hellwig
> Cc: Aneesh Kumar K.V
> Signed-off-by: John Hubbard
> ---
> mm/gup.c | 95
> 1 file changed, 40 insertions(+), 55 deletions(-)
>
> diff --git a/mm/gup.c b/mm/gup.c
> index 85caf76b3012..858541ea30ce 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -1969,6 +1969,29 @@ static int __gup_device_huge_pud(pud_t pud, pud_t
> *pudp, unsigned long addr,
> }
> #endif
>
> +static int __record_subpages(struct page *page, unsigned long addr,
> + unsigned long end, struct page **pages)
> +{
> + int nr = 0;
> + int nr_recorded_pages = 0;
> +
> + do {
> + pages[nr] = page;
> + nr++;
> + page++;
> + nr_recorded_pages++;
> + } while (addr += PAGE_SIZE, addr != end);
> + return nr_recorded_pages;
nr == nr_recorded_pages so no need for both... BTW, structuring this as a
for loop would be probably more logical and shorter now:
for (nr = 0; addr != end; addr += PAGE_SIZE)
pages[nr++] = page++;
return nr;
The rest of the patch looks good to me.
Honza
--
Jan Kara
SUSE Labs, CR
Re: [PATCH v5 17/24] mm/gup: track FOLL_PIN pages
On Thu 14-11-19 21:53:33, John Hubbard wrote:
> Add tracking of pages that were pinned via FOLL_PIN.
>
> As mentioned in the FOLL_PIN documentation, callers who effectively set
> FOLL_PIN are required to ultimately free such pages via put_user_page().
> The effect is similar to FOLL_GET, and may be thought of as "FOLL_GET
> for DIO and/or RDMA use".
>
> Pages that have been pinned via FOLL_PIN are identifiable via a
> new function call:
>
>bool page_dma_pinned(struct page *page);
>
> What to do in response to encountering such a page, is left to later
> patchsets. There is discussion about this in [1].
^^ missing this reference
in the changelog...
> This also changes a BUG_ON(), to a WARN_ON(), in follow_page_mask().
>
> Suggested-by: Jan Kara
> Suggested-by: Jérôme Glisse
> Signed-off-by: John Hubbard
> ---
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 6588d2e02628..db872766480f 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1054,6 +1054,8 @@ static inline __must_check bool try_get_page(struct
> page *page)
> return true;
> }
>
> +__must_check bool user_page_ref_inc(struct page *page);
> +
> static inline void put_page(struct page *page)
> {
> page = compound_head(page);
> @@ -1071,29 +1073,70 @@ static inline void put_page(struct page *page)
> __put_page(page);
> }
>
> -/**
> - * put_user_page() - release a gup-pinned page
> - * @page:pointer to page to be released
> +/*
> + * GUP_PIN_COUNTING_BIAS, and the associated functions that use it, overload
> + * the page's refcount so that two separate items are tracked: the original
> page
> + * reference count, and also a new count of how many get_user_pages() calls
> were
^^ pin_user_pages()
> + * made against the page. ("gup-pinned" is another term for the latter).
> + *
> + * With this scheme, get_user_pages() becomes special: such pages are marked
^^^ pin_user_pages()
> + * as distinct from normal pages. As such, the put_user_page() call (and its
> + * variants) must be used in order to release gup-pinned pages.
> + *
> + * Choice of value:
> *
> - * Pages that were pinned via pin_user_pages*() must be released via either
> - * put_user_page(), or one of the put_user_pages*() routines. This is so that
> - * eventually such pages can be separately tracked and uniquely handled. In
> - * particular, interactions with RDMA and filesystems need special handling.
> + * By making GUP_PIN_COUNTING_BIAS a power of two, debugging of page
> reference
> + * counts with respect to get_user_pages() and put_user_page() becomes
> simpler,
^^^ pin_user_pages()
> + * due to the fact that adding an even power of two to the page refcount has
> + * the effect of using only the upper N bits, for the code that counts up
> using
> + * the bias value. This means that the lower bits are left for the exclusive
> + * use of the original code that increments and decrements by one (or at
> least,
> + * by much smaller values than the bias value).
> *
> - * put_user_page() and put_page() are not interchangeable, despite this early
> - * implementation that makes them look the same. put_user_page() calls must
> - * be perfectly matched up with pin*() calls.
> + * Of course, once the lower bits overflow into the upper bits (and this is
> + * OK, because subtraction recovers the original values), then visual
> inspection
> + * no longer suffices to directly view the separate counts. However, for
> normal
> + * applications that don't have huge page reference counts, this won't be an
> + * issue.
> + *
> + * Locking: the lockless algorithm described in page_cache_get_speculative()
> + * and page_cache_gup_pin_speculative() provides safe operation for
> + * get_user_pages and page_mkclean and other calls that race to set up page
> + * table entries.
> */
...
> @@ -2070,9 +2191,16 @@ static int gup_hugepte(pte_t *ptep, unsigned long sz,
> unsigned long addr,
> page = head + ((addr & (sz-1)) >> PAGE_SHIFT);
> refs = __record_subpages(page, addr, end, pages + *nr);
>
> - head = try_get_compound_head(head, refs);
> - if (!head)
> - return 0;
> + if (flags & FOLL_PIN) {
> + head = page;
> + if (unlikely(!user_page_ref_inc(head)))
> + return 0;
> + head = page;
Why do you assign 'head' twice? Also the refcounting logic is repeated
several times so perhaps you can factor it out in to a helper function or
even move it to __record_subpages()?
> + } else {
> + head = try_get_compound_head(head, refs);
> + if (!head)
> + return 0;
> + }
>
> if (unlikely(pte_val(pte) != pte_val(*ptep))) {
> put_compound_head(head, refs);
So this will do the wrong thing for FOLL_PIN
Re: HPT allocation failures on POWER8 KVM hosts
On Mon, Nov 18, 2019 at 01:02:00PM +1100, Daniel Axtens wrote:
> Hi Roman,
>
> > We're running a lot of KVM virtual machines on POWER8 hosts and
> > sometimes new VMs can't be started because there are no contiguous
> > regions for HPT because of CMA region fragmentation.
> >
> > The issue is covered in the LWN article: https://lwn.net/Articles/684611/
> > The article points that you raised the problem on LSFMM 2016. However I
> > couldn't find a follow up article on the issue.
> >
> > Looking at the kernel commit log I've identified a few commits that
> > might reduce CMA fragmentaiton and overcome HPT allocation failure:
> > - bd2e75633c801 ("dma-contiguous: use fallback alloc_pages for single
> > pages")
> > - 678e174c4c16a ("powerpc/mm/iommu: allow migration of cma allocated
> > pages during mm_iommu_do_alloc")
> > - 9a4e9f3b2d739 ("mm: update get_user_pages_longterm to migrate pages
> > allocated from
> > CMA region")
> > - d7fefcc8de914 ("mm/cma: add PF flag to force non cma alloc")
> >
> > Are there any other commits that address the issue? What is the first
> > kernel version that shouldn't have the HPT allocation problem due to CMA
> > fragmentation?
>
> I've had some success increasing the CMA allocation with the
> kvm_cma_resv_ratio boot parameter - see
> arch/powerpc/kvm/book3s_hv_builtin.c
>
> The default is 5%. In a support case in a former job we had a customer
> who increased this to I think 7 or 8% and saw the symptoms subside
> dramatically.
>
Hi Daniel,
Thank you, I'll try to increase kvm_cma_resv_ratio for now, but even 5%
CMA reserve should be more than enough, given the size of HPT as 1/128th
of VM max memory.
For a 16GB RAM VM without balloon device, only 128MB is going to be
reserved for HPT using CMA. So, 5% CMA reserve should allow to provision
VMs with over 1.5TB of RAM on 256GB RAM host. In other words the default
CMA reserve allows to overprovision 6 times more memory for VMs than
presented on a host.
We rarely add balloon device and sometimes don't add it at all. Therefore
I'm still looking for commits that would help to avoid the issue with
the default CMA reserve.
Thank you,
Roman
[PATCH v5 48/48] soc: fsl: qe: remove PPC32 dependency from CONFIG_QUICC_ENGINE
There are also PPC64, ARM and ARM64 based SOCs with a QUICC Engine, and the core QE code as well as net/wan/fsl_ucc_hdlc and tty/serial/ucc_uart has now been modified to not rely on ppcisms. So extend the architectures that can select QUICC_ENGINE, and add the rather modest requirements of OF && HAS_IOMEM. The core code as well as the ucc_uart driver has been tested on an LS1021A (arm), and it has also been tested that the QE code still works on an mpc8309 (ppc). Qiang Zhao has tested that the QE-HDLC code that gets enabled with this works on ARM64. Signed-off-by: Rasmus Villemoes --- drivers/soc/fsl/qe/Kconfig | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/soc/fsl/qe/Kconfig b/drivers/soc/fsl/qe/Kconfig index cfa4b2939992..357c5800b112 100644 --- a/drivers/soc/fsl/qe/Kconfig +++ b/drivers/soc/fsl/qe/Kconfig @@ -5,7 +5,8 @@ config QUICC_ENGINE bool "QUICC Engine (QE) framework support" - depends on FSL_SOC && PPC32 + depends on OF && HAS_IOMEM + depends on PPC || ARM || ARM64 || COMPILE_TEST select GENERIC_ALLOCATOR select CRC32 help -- 2.23.0
[PATCH v5 47/48] net: ethernet: freescale: make UCC_GETH explicitly depend on PPC32
Currently, QUICC_ENGINE depends on PPC32, so this in itself does not change anything. In order to allow removing the PPC32 dependency from QUICC_ENGINE and avoid allmodconfig build failures, add this explicit dependency. Also, the QE Ethernet has never been integrated on any non-PowerPC SoC and most likely will not be in the future. Signed-off-by: Rasmus Villemoes --- drivers/net/ethernet/freescale/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/freescale/Kconfig b/drivers/net/ethernet/freescale/Kconfig index 6a7e8993119f..2bd7ace0a953 100644 --- a/drivers/net/ethernet/freescale/Kconfig +++ b/drivers/net/ethernet/freescale/Kconfig @@ -74,7 +74,7 @@ config FSL_XGMAC_MDIO config UCC_GETH tristate "Freescale QE Gigabit Ethernet" - depends on QUICC_ENGINE + depends on QUICC_ENGINE && PPC32 select FSL_PQ_MDIO select PHYLIB ---help--- -- 2.23.0
[PATCH v5 46/48] net/wan/fsl_ucc_hdlc: reject muram offsets above 64K
Qiang Zhao points out that these offsets get written to 16-bit
registers, and there are some QE platforms with more than 64K
muram. So it is possible that qe_muram_alloc() gives us an allocation
that can't actually be used by the hardware, so detect and reject
that.
Reported-by: Qiang Zhao
Signed-off-by: Rasmus Villemoes
---
drivers/net/wan/fsl_ucc_hdlc.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index 8d13586bb774..f029eaa7cfc0 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -245,6 +245,11 @@ static int uhdlc_init(struct ucc_hdlc_private *priv)
ret = -ENOMEM;
goto free_riptr;
}
+ if (riptr != (u16)riptr || tiptr != (u16)tiptr) {
+ dev_err(priv->dev, "MURAM allocation out of addressable
range\n");
+ ret = -ENOMEM;
+ goto free_tiptr;
+ }
/* Set RIPTR, TIPTR */
iowrite16be(riptr, &priv->ucc_pram->riptr);
--
2.23.0
[PATCH v5 45/48] net/wan/fsl_ucc_hdlc: fix reading of __be16 registers
When releasing the allocated muram resource, we rely on reading back
the offsets from the riptr/tiptr registers. But those registers are
__be16 (and we indeed write them using iowrite16be), so we can't just
read them back with a normal C dereference.
This is not currently a real problem, since for now the driver is
PPC32-only. But it will soon be allowed to be used on arm and arm64 as
well.
Signed-off-by: Rasmus Villemoes
---
drivers/net/wan/fsl_ucc_hdlc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index 405b24a5a60d..8d13586bb774 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -732,8 +732,8 @@ static int uhdlc_open(struct net_device *dev)
static void uhdlc_memclean(struct ucc_hdlc_private *priv)
{
- qe_muram_free(priv->ucc_pram->riptr);
- qe_muram_free(priv->ucc_pram->tiptr);
+ qe_muram_free(ioread16be(&priv->ucc_pram->riptr));
+ qe_muram_free(ioread16be(&priv->ucc_pram->tiptr));
if (priv->rx_bd_base) {
dma_free_coherent(priv->dev,
--
2.23.0
[PATCH v5 44/48] net/wan/fsl_ucc_hdlc: avoid use of IS_ERR_VALUE()
When building this on a 64-bit platform gcc rightly warns that the
error checking is broken (-ENOMEM stored in an u32 does not compare
greater than (unsigned long)-MAX_ERRNO). Instead, now that
qe_muram_alloc() returns s32, use that type to store the return value
and use standard kernel style "ret < 0".
Signed-off-by: Rasmus Villemoes
---
drivers/net/wan/fsl_ucc_hdlc.c | 10 +-
drivers/net/wan/fsl_ucc_hdlc.h | 2 +-
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index ce6af7d5380f..405b24a5a60d 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -84,8 +84,8 @@ static int uhdlc_init(struct ucc_hdlc_private *priv)
int ret, i;
void *bd_buffer;
dma_addr_t bd_dma_addr;
- u32 riptr;
- u32 tiptr;
+ s32 riptr;
+ s32 tiptr;
u32 gumr;
ut_info = priv->ut_info;
@@ -195,7 +195,7 @@ static int uhdlc_init(struct ucc_hdlc_private *priv)
priv->ucc_pram_offset = qe_muram_alloc(sizeof(struct ucc_hdlc_param),
ALIGNMENT_OF_UCC_HDLC_PRAM);
- if (IS_ERR_VALUE(priv->ucc_pram_offset)) {
+ if (priv->ucc_pram_offset < 0) {
dev_err(priv->dev, "Can not allocate MURAM for hdlc
parameter.\n");
ret = -ENOMEM;
goto free_tx_bd;
@@ -233,14 +233,14 @@ static int uhdlc_init(struct ucc_hdlc_private *priv)
/* Alloc riptr, tiptr */
riptr = qe_muram_alloc(32, 32);
- if (IS_ERR_VALUE(riptr)) {
+ if (riptr < 0) {
dev_err(priv->dev, "Cannot allocate MURAM mem for Receive
internal temp data pointer\n");
ret = -ENOMEM;
goto free_tx_skbuff;
}
tiptr = qe_muram_alloc(32, 32);
- if (IS_ERR_VALUE(tiptr)) {
+ if (tiptr < 0) {
dev_err(priv->dev, "Cannot allocate MURAM mem for Transmit
internal temp data pointer\n");
ret = -ENOMEM;
goto free_riptr;
diff --git a/drivers/net/wan/fsl_ucc_hdlc.h b/drivers/net/wan/fsl_ucc_hdlc.h
index 8b3507ae1781..71d5ad0a7b98 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.h
+++ b/drivers/net/wan/fsl_ucc_hdlc.h
@@ -98,7 +98,7 @@ struct ucc_hdlc_private {
unsigned short tx_ring_size;
unsigned short rx_ring_size;
- u32 ucc_pram_offset;
+ s32 ucc_pram_offset;
unsigned short encoding;
unsigned short parity;
--
2.23.0
[PATCH v5 43/48] soc: fsl: qe: avoid IS_ERR_VALUE in ucc_fast.c
When building this on a 64-bit platform gcc rightly warns that the
error checking is broken (-ENOMEM stored in an u32 does not compare
greater than (unsigned long)-MAX_ERRNO). Instead, change the
ucc_fast_[tr]x_virtual_fifo_base_offset members to s32 and use an
ordinary check-for-negative. Also, this avoids treating 0 as "this
cannot have been returned from qe_muram_alloc() so don't free it".
Signed-off-by: Rasmus Villemoes
---
drivers/soc/fsl/qe/ucc_fast.c | 15 ++-
include/soc/fsl/qe/ucc_fast.h | 4 ++--
2 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/drivers/soc/fsl/qe/ucc_fast.c b/drivers/soc/fsl/qe/ucc_fast.c
index ca0452497a20..ad6193ea4597 100644
--- a/drivers/soc/fsl/qe/ucc_fast.c
+++ b/drivers/soc/fsl/qe/ucc_fast.c
@@ -197,6 +197,8 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct
ucc_fast_private ** ucc
__func__);
return -ENOMEM;
}
+ uccf->ucc_fast_tx_virtual_fifo_base_offset = -1;
+ uccf->ucc_fast_rx_virtual_fifo_base_offset = -1;
/* Fill fast UCC structure */
uccf->uf_info = uf_info;
@@ -265,10 +267,9 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct
ucc_fast_private ** ucc
/* Allocate memory for Tx Virtual Fifo */
uccf->ucc_fast_tx_virtual_fifo_base_offset =
qe_muram_alloc(uf_info->utfs, UCC_FAST_VIRT_FIFO_REGS_ALIGNMENT);
- if (IS_ERR_VALUE(uccf->ucc_fast_tx_virtual_fifo_base_offset)) {
+ if (uccf->ucc_fast_tx_virtual_fifo_base_offset < 0) {
printk(KERN_ERR "%s: cannot allocate MURAM for TX FIFO\n",
__func__);
- uccf->ucc_fast_tx_virtual_fifo_base_offset = 0;
ucc_fast_free(uccf);
return -ENOMEM;
}
@@ -278,10 +279,9 @@ int ucc_fast_init(struct ucc_fast_info * uf_info, struct
ucc_fast_private ** ucc
qe_muram_alloc(uf_info->urfs +
UCC_FAST_RECEIVE_VIRTUAL_FIFO_SIZE_FUDGE_FACTOR,
UCC_FAST_VIRT_FIFO_REGS_ALIGNMENT);
- if (IS_ERR_VALUE(uccf->ucc_fast_rx_virtual_fifo_base_offset)) {
+ if (uccf->ucc_fast_rx_virtual_fifo_base_offset < 0) {
printk(KERN_ERR "%s: cannot allocate MURAM for RX FIFO\n",
__func__);
- uccf->ucc_fast_rx_virtual_fifo_base_offset = 0;
ucc_fast_free(uccf);
return -ENOMEM;
}
@@ -384,11 +384,8 @@ void ucc_fast_free(struct ucc_fast_private * uccf)
if (!uccf)
return;
- if (uccf->ucc_fast_tx_virtual_fifo_base_offset)
- qe_muram_free(uccf->ucc_fast_tx_virtual_fifo_base_offset);
-
- if (uccf->ucc_fast_rx_virtual_fifo_base_offset)
- qe_muram_free(uccf->ucc_fast_rx_virtual_fifo_base_offset);
+ qe_muram_free(uccf->ucc_fast_tx_virtual_fifo_base_offset);
+ qe_muram_free(uccf->ucc_fast_rx_virtual_fifo_base_offset);
if (uccf->uf_regs)
iounmap(uccf->uf_regs);
diff --git a/include/soc/fsl/qe/ucc_fast.h b/include/soc/fsl/qe/ucc_fast.h
index e9cc46042a83..ba0e838f962a 100644
--- a/include/soc/fsl/qe/ucc_fast.h
+++ b/include/soc/fsl/qe/ucc_fast.h
@@ -188,9 +188,9 @@ struct ucc_fast_private {
int stopped_tx; /* Whether channel has been stopped for Tx
(STOP_TX, etc.) */
int stopped_rx; /* Whether channel has been stopped for Rx */
- u32 ucc_fast_tx_virtual_fifo_base_offset;/* pointer to base of Tx
+ s32 ucc_fast_tx_virtual_fifo_base_offset;/* pointer to base of Tx
virtual fifo */
- u32 ucc_fast_rx_virtual_fifo_base_offset;/* pointer to base of Rx
+ s32 ucc_fast_rx_virtual_fifo_base_offset;/* pointer to base of Rx
virtual fifo */
#ifdef STATISTICS
u32 tx_frames; /* Transmitted frames counter. */
--
2.23.0
[PATCH v5 42/48] soc: fsl: qe: drop pointless check in qe_sdma_init()
The sdma member of struct qe_immap is not at offset zero, so even if
qe_immr wasn't initialized yet (i.e. NULL), &qe_immr->sdma would not
be NULL.
Signed-off-by: Rasmus Villemoes
---
drivers/soc/fsl/qe/qe.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/soc/fsl/qe/qe.c b/drivers/soc/fsl/qe/qe.c
index 5bf279c679ef..96c2057d8d8e 100644
--- a/drivers/soc/fsl/qe/qe.c
+++ b/drivers/soc/fsl/qe/qe.c
@@ -367,9 +367,6 @@ static int qe_sdma_init(void)
struct sdma __iomem *sdma = &qe_immr->sdma;
static s32 sdma_buf_offset = -ENOMEM;
- if (!sdma)
- return -ENODEV;
-
/* allocate 2 internal temporary buffers (512 bytes size each) for
* the SDMA */
if (sdma_buf_offset < 0) {
--
2.23.0
[PATCH v5 40/48] soc: fsl: qe: avoid IS_ERR_VALUE in ucc_slow.c
When trying to build this for a 64-bit platform, one gets warnings
from using IS_ERR_VALUE on something which is not sizeof(long).
Instead, change the various *_offset fields to store a signed integer,
and simply check for a negative return from qe_muram_alloc(). Since
qe_muram_free() now accepts and ignores a negative argument, we only
need to make sure these fields are initialized with -1, and we can
just unconditionally call qe_muram_free() in ucc_slow_free().
Note that the error case for us_pram_offset failed to set that field
to 0 (which, as noted earlier, is anyway a bogus sentinel value).
Signed-off-by: Rasmus Villemoes
---
drivers/soc/fsl/qe/ucc_slow.c | 22 +-
include/soc/fsl/qe/ucc_slow.h | 6 +++---
2 files changed, 12 insertions(+), 16 deletions(-)
diff --git a/drivers/soc/fsl/qe/ucc_slow.c b/drivers/soc/fsl/qe/ucc_slow.c
index 9b55fd0f50c6..274d34449846 100644
--- a/drivers/soc/fsl/qe/ucc_slow.c
+++ b/drivers/soc/fsl/qe/ucc_slow.c
@@ -154,6 +154,9 @@ int ucc_slow_init(struct ucc_slow_info * us_info, struct
ucc_slow_private ** ucc
__func__);
return -ENOMEM;
}
+ uccs->rx_base_offset = -1;
+ uccs->tx_base_offset = -1;
+ uccs->us_pram_offset = -1;
/* Fill slow UCC structure */
uccs->us_info = us_info;
@@ -179,7 +182,7 @@ int ucc_slow_init(struct ucc_slow_info * us_info, struct
ucc_slow_private ** ucc
/* Get PRAM base */
uccs->us_pram_offset =
qe_muram_alloc(UCC_SLOW_PRAM_SIZE, ALIGNMENT_OF_UCC_SLOW_PRAM);
- if (IS_ERR_VALUE(uccs->us_pram_offset)) {
+ if (uccs->us_pram_offset < 0) {
printk(KERN_ERR "%s: cannot allocate MURAM for PRAM", __func__);
ucc_slow_free(uccs);
return -ENOMEM;
@@ -206,10 +209,9 @@ int ucc_slow_init(struct ucc_slow_info * us_info, struct
ucc_slow_private ** ucc
uccs->rx_base_offset =
qe_muram_alloc(us_info->rx_bd_ring_len * sizeof(struct qe_bd),
QE_ALIGNMENT_OF_BD);
- if (IS_ERR_VALUE(uccs->rx_base_offset)) {
+ if (uccs->rx_base_offset < 0) {
printk(KERN_ERR "%s: cannot allocate %u RX BDs\n", __func__,
us_info->rx_bd_ring_len);
- uccs->rx_base_offset = 0;
ucc_slow_free(uccs);
return -ENOMEM;
}
@@ -217,9 +219,8 @@ int ucc_slow_init(struct ucc_slow_info * us_info, struct
ucc_slow_private ** ucc
uccs->tx_base_offset =
qe_muram_alloc(us_info->tx_bd_ring_len * sizeof(struct qe_bd),
QE_ALIGNMENT_OF_BD);
- if (IS_ERR_VALUE(uccs->tx_base_offset)) {
+ if (uccs->tx_base_offset < 0) {
printk(KERN_ERR "%s: cannot allocate TX BDs", __func__);
- uccs->tx_base_offset = 0;
ucc_slow_free(uccs);
return -ENOMEM;
}
@@ -352,14 +353,9 @@ void ucc_slow_free(struct ucc_slow_private * uccs)
if (!uccs)
return;
- if (uccs->rx_base_offset)
- qe_muram_free(uccs->rx_base_offset);
-
- if (uccs->tx_base_offset)
- qe_muram_free(uccs->tx_base_offset);
-
- if (uccs->us_pram)
- qe_muram_free(uccs->us_pram_offset);
+ qe_muram_free(uccs->rx_base_offset);
+ qe_muram_free(uccs->tx_base_offset);
+ qe_muram_free(uccs->us_pram_offset);
if (uccs->us_regs)
iounmap(uccs->us_regs);
diff --git a/include/soc/fsl/qe/ucc_slow.h b/include/soc/fsl/qe/ucc_slow.h
index 8696fdea2ae9..d187a6be83bc 100644
--- a/include/soc/fsl/qe/ucc_slow.h
+++ b/include/soc/fsl/qe/ucc_slow.h
@@ -185,7 +185,7 @@ struct ucc_slow_private {
struct ucc_slow_info *us_info;
struct ucc_slow __iomem *us_regs; /* Ptr to memory map of UCC regs */
struct ucc_slow_pram *us_pram; /* a pointer to the parameter RAM */
- u32 us_pram_offset;
+ s32 us_pram_offset;
int enabled_tx; /* Whether channel is enabled for Tx (ENT) */
int enabled_rx; /* Whether channel is enabled for Rx (ENR) */
int stopped_tx; /* Whether channel has been stopped for Tx
@@ -194,8 +194,8 @@ struct ucc_slow_private {
struct list_head confQ; /* frames passed to chip waiting for tx */
u32 first_tx_bd_mask; /* mask is used in Tx routine to save status
and length for first BD in a frame */
- u32 tx_base_offset; /* first BD in Tx BD table offset (In MURAM) */
- u32 rx_base_offset; /* first BD in Rx BD table offset (In MURAM) */
+ s32 tx_base_offset; /* first BD in Tx BD table offset (In MURAM) */
+ s32 rx_base_offset; /* first BD in Rx BD table offset (In MURAM) */
struct qe_bd *confBd; /* next BD for confirm after Tx */
struct qe_bd *tx_bd;/* next BD for new Tx request */
struct
[PATCH v5 38/48] soc: fsl: qe: drop broken lazy call of cpm_muram_init()
cpm_muram_alloc_common() tries to support a kind of lazy initialization - if the muram_pool has not been created yet, it calls cpm_muram_init(). Now, cpm_muram_alloc_common() is always called under spin_lock_irqsave(&cpm_muram_lock, flags); and cpm_muram_init() does gen_pool_create() (which implies a GFP_KERNEL allocation) and ioremap(), not to mention the fun that ensues from cpm_muram_init() doing spin_lock_init(&cpm_muram_lock); In other words, this has never worked, so nobody can have been relying on it. cpm_muram_init() is called from a subsys_initcall (either from cpm_init() in arch/powerpc/sysdev/cpm_common.c or, via qe_reset(), from qe_init() in drivers/soc/fsl/qe/qe.c). Signed-off-by: Rasmus Villemoes --- drivers/soc/fsl/qe/qe_common.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/soc/fsl/qe/qe_common.c b/drivers/soc/fsl/qe/qe_common.c index 48c77bb92846..dcb267567d76 100644 --- a/drivers/soc/fsl/qe/qe_common.c +++ b/drivers/soc/fsl/qe/qe_common.c @@ -119,9 +119,6 @@ static s32 cpm_muram_alloc_common(unsigned long size, struct muram_block *entry; s32 start; - if (!muram_pool && cpm_muram_init()) - goto out2; - start = gen_pool_alloc_algo(muram_pool, size, algo, data); if (!start) goto out2; -- 2.23.0
[PATCH v5 41/48] soc: fsl: qe: drop use of IS_ERR_VALUE in qe_sdma_init()
Now that qe_muram_alloc() returns s32, adapt qe_sdma_init() and avoid
another few IS_ERR_VALUE() uses.
Signed-off-by: Rasmus Villemoes
---
drivers/soc/fsl/qe/qe.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/soc/fsl/qe/qe.c b/drivers/soc/fsl/qe/qe.c
index ec511840db3c..5bf279c679ef 100644
--- a/drivers/soc/fsl/qe/qe.c
+++ b/drivers/soc/fsl/qe/qe.c
@@ -365,16 +365,16 @@ EXPORT_SYMBOL(qe_put_snum);
static int qe_sdma_init(void)
{
struct sdma __iomem *sdma = &qe_immr->sdma;
- static unsigned long sdma_buf_offset = (unsigned long)-ENOMEM;
+ static s32 sdma_buf_offset = -ENOMEM;
if (!sdma)
return -ENODEV;
/* allocate 2 internal temporary buffers (512 bytes size each) for
* the SDMA */
- if (IS_ERR_VALUE(sdma_buf_offset)) {
+ if (sdma_buf_offset < 0) {
sdma_buf_offset = qe_muram_alloc(512 * 2, 4096);
- if (IS_ERR_VALUE(sdma_buf_offset))
+ if (sdma_buf_offset < 0)
return -ENOMEM;
}
--
2.23.0
[PATCH v5 39/48] soc: fsl: qe: refactor cpm_muram_alloc_common to prevent BUG on error path
If the kmalloc() fails, we try to undo the gen_pool allocation we've
just done. Unfortunately, start has already been modified to subtract
the GENPOOL_OFFSET bias, so we're freeing something that very likely
doesn't exist in the gen_pool, meaning we hit the
kernel BUG at lib/genalloc.c:399!
Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
...
[<803fd0e8>] (gen_pool_free) from [<80426bc8>]
(cpm_muram_alloc_common+0xb0/0xc8)
[<80426bc8>] (cpm_muram_alloc_common) from [<80426c28>]
(cpm_muram_alloc+0x48/0x80)
[<80426c28>] (cpm_muram_alloc) from [<80428214>] (ucc_slow_init+0x110/0x4f0)
[<80428214>] (ucc_slow_init) from [<8044a718>]
(qe_uart_request_port+0x3c/0x1d8)
(this was tested by just injecting a random failure by adding
"|| (get_random_int()&7) == 0" to the "if (!entry)" condition).
Refactor the code so we do the kmalloc() first, meaning that's the
thing that needs undoing in case gen_pool_alloc_algo() then
fails. This allows a later cleanup to move the locking from the
callers into the _common function, keeping the kmalloc() out of the
critical region and then, hopefully (if all the muram_alloc callers
allow) change it to a GFP_KERNEL allocation.
Signed-off-by: Rasmus Villemoes
---
drivers/soc/fsl/qe/qe_common.c | 16 +++-
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/soc/fsl/qe/qe_common.c b/drivers/soc/fsl/qe/qe_common.c
index dcb267567d76..a81a1a79f1ca 100644
--- a/drivers/soc/fsl/qe/qe_common.c
+++ b/drivers/soc/fsl/qe/qe_common.c
@@ -119,23 +119,21 @@ static s32 cpm_muram_alloc_common(unsigned long size,
struct muram_block *entry;
s32 start;
+ entry = kmalloc(sizeof(*entry), GFP_ATOMIC);
+ if (!entry)
+ return -ENOMEM;
start = gen_pool_alloc_algo(muram_pool, size, algo, data);
- if (!start)
- goto out2;
+ if (!start) {
+ kfree(entry);
+ return -ENOMEM;
+ }
start = start - GENPOOL_OFFSET;
memset_io(cpm_muram_addr(start), 0, size);
- entry = kmalloc(sizeof(*entry), GFP_ATOMIC);
- if (!entry)
- goto out1;
entry->start = start;
entry->size = size;
list_add(&entry->head, &muram_block_list);
return start;
-out1:
- gen_pool_free(muram_pool, start, size);
-out2:
- return -ENOMEM;
}
/*
--
2.23.0
[PATCH v5 37/48] soc: fsl: qe: make cpm_muram_free() ignore a negative offset
This allows one to simplify callers since they can store a negative
value as a sentinel to indicate "this was never allocated" (or store
the -ENOMEM from an allocation failure) and then call cpm_muram_free()
unconditionally.
Signed-off-by: Rasmus Villemoes
---
drivers/soc/fsl/qe/qe_common.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/soc/fsl/qe/qe_common.c b/drivers/soc/fsl/qe/qe_common.c
index 962835488f66..48c77bb92846 100644
--- a/drivers/soc/fsl/qe/qe_common.c
+++ b/drivers/soc/fsl/qe/qe_common.c
@@ -176,6 +176,9 @@ void cpm_muram_free(s32 offset)
int size;
struct muram_block *tmp;
+ if (offset < 0)
+ return;
+
size = 0;
spin_lock_irqsave(&cpm_muram_lock, flags);
list_for_each_entry(tmp, &muram_block_list, head) {
--
2.23.0
[PATCH v5 36/48] soc: fsl: qe: make cpm_muram_free() return void
Nobody uses the return value from cpm_muram_free, and functions that
free resources usually return void. One could imagine a use for a "how
much have I allocated" a la ksize(), but knowing how much one had
access to after the fact is useless.
Signed-off-by: Rasmus Villemoes
---
drivers/soc/fsl/qe/qe_common.c | 3 +--
include/soc/fsl/qe/qe.h| 5 ++---
2 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/soc/fsl/qe/qe_common.c b/drivers/soc/fsl/qe/qe_common.c
index 84c90105e588..962835488f66 100644
--- a/drivers/soc/fsl/qe/qe_common.c
+++ b/drivers/soc/fsl/qe/qe_common.c
@@ -170,7 +170,7 @@ EXPORT_SYMBOL(cpm_muram_alloc);
* cpm_muram_free - free a chunk of multi-user ram
* @offset: The beginning of the chunk as returned by cpm_muram_alloc().
*/
-int cpm_muram_free(s32 offset)
+void cpm_muram_free(s32 offset)
{
unsigned long flags;
int size;
@@ -188,7 +188,6 @@ int cpm_muram_free(s32 offset)
}
gen_pool_free(muram_pool, offset + GENPOOL_OFFSET, size);
spin_unlock_irqrestore(&cpm_muram_lock, flags);
- return size;
}
EXPORT_SYMBOL(cpm_muram_free);
diff --git a/include/soc/fsl/qe/qe.h b/include/soc/fsl/qe/qe.h
index f589ae3f1216..e282ac01ec08 100644
--- a/include/soc/fsl/qe/qe.h
+++ b/include/soc/fsl/qe/qe.h
@@ -99,7 +99,7 @@ int cpm_muram_init(void);
#if defined(CONFIG_CPM) || defined(CONFIG_QUICC_ENGINE)
s32 cpm_muram_alloc(unsigned long size, unsigned long align);
-int cpm_muram_free(s32 offset);
+void cpm_muram_free(s32 offset);
s32 cpm_muram_alloc_fixed(unsigned long offset, unsigned long size);
void __iomem *cpm_muram_addr(unsigned long offset);
unsigned long cpm_muram_offset(void __iomem *addr);
@@ -111,9 +111,8 @@ static inline s32 cpm_muram_alloc(unsigned long size,
return -ENOSYS;
}
-static inline int cpm_muram_free(s32 offset)
+static inline void cpm_muram_free(s32 offset)
{
- return -ENOSYS;
}
static inline s32 cpm_muram_alloc_fixed(unsigned long offset,
--
2.23.0
[PATCH v5 35/48] soc: fsl: qe: change return type of cpm_muram_alloc() to s32
There are a number of problems with cpm_muram_alloc() and its
callers. Most callers assign the return value to some variable and
then use IS_ERR_VALUE to check for allocation failure. However, when
that variable is not sizeof(long), this leads to warnings - and it is
indeed broken to do e.g.
u32 foo = cpm_muram_alloc();
if (IS_ERR_VALUE(foo))
on a 64-bit platform, since the condition
foo >= (unsigned long)-ENOMEM
is tautologically false. There are also callers that ignore the
possibility of error, and then there are those that check for error by
comparing the return value to 0...
One could fix that by changing all callers to store the return value
temporarily in an "unsigned long" and test that. However, use of
IS_ERR_VALUE() is error-prone and should be restricted to things which
are inherently long-sized (stuff in pt_regs etc.). Instead, let's aim
for changing to the standard kernel style
int foo = cpm_muram_alloc();
if (foo < 0)
deal_with_it()
some->where = foo;
Changing the return type from unsigned long to s32 (aka signed int)
doesn't change the value that gets stored into any of the callers'
variables except if the caller was storing the result in a u64 _and_
the allocation failed, so in itself this patch should be a no-op.
Another problem with cpm_muram_alloc() is that it can certainly
validly return 0 - and except if some cpm_muram_alloc_fixed() call
interferes, the very first cpm_muram_alloc() call will return just
that. But that shows that both ucc_slow_free() and ucc_fast_free() are
buggy, since they assume that a value of 0 means "that field was never
allocated". We'll later change cpm_muram_free() to accept (and ignore)
a negative offset, so callers can use a sentinel of -1 instead of 0
and just unconditionally call cpm_muram_free().
Signed-off-by: Rasmus Villemoes
---
drivers/soc/fsl/qe/qe_common.c | 29 -
include/soc/fsl/qe/qe.h| 16
2 files changed, 24 insertions(+), 21 deletions(-)
diff --git a/drivers/soc/fsl/qe/qe_common.c b/drivers/soc/fsl/qe/qe_common.c
index 83e85e61669f..84c90105e588 100644
--- a/drivers/soc/fsl/qe/qe_common.c
+++ b/drivers/soc/fsl/qe/qe_common.c
@@ -32,7 +32,7 @@ static phys_addr_t muram_pbase;
struct muram_block {
struct list_head head;
- unsigned long start;
+ s32 start;
int size;
};
@@ -110,13 +110,14 @@ int cpm_muram_init(void)
* @algo: algorithm for alloc.
* @data: data for genalloc's algorithm.
*
- * This function returns an offset into the muram area.
+ * This function returns a non-negative offset into the muram area, or
+ * a negative errno on failure.
*/
-static unsigned long cpm_muram_alloc_common(unsigned long size,
- genpool_algo_t algo, void *data)
+static s32 cpm_muram_alloc_common(unsigned long size,
+ genpool_algo_t algo, void *data)
{
struct muram_block *entry;
- unsigned long start;
+ s32 start;
if (!muram_pool && cpm_muram_init())
goto out2;
@@ -137,7 +138,7 @@ static unsigned long cpm_muram_alloc_common(unsigned long
size,
out1:
gen_pool_free(muram_pool, start, size);
out2:
- return (unsigned long)-ENOMEM;
+ return -ENOMEM;
}
/*
@@ -145,13 +146,14 @@ static unsigned long cpm_muram_alloc_common(unsigned long
size,
* @size: number of bytes to allocate
* @align: requested alignment, in bytes
*
- * This function returns an offset into the muram area.
+ * This function returns a non-negative offset into the muram area, or
+ * a negative errno on failure.
* Use cpm_dpram_addr() to get the virtual address of the area.
* Use cpm_muram_free() to free the allocation.
*/
-unsigned long cpm_muram_alloc(unsigned long size, unsigned long align)
+s32 cpm_muram_alloc(unsigned long size, unsigned long align)
{
- unsigned long start;
+ s32 start;
unsigned long flags;
struct genpool_data_align muram_pool_data;
@@ -168,7 +170,7 @@ EXPORT_SYMBOL(cpm_muram_alloc);
* cpm_muram_free - free a chunk of multi-user ram
* @offset: The beginning of the chunk as returned by cpm_muram_alloc().
*/
-int cpm_muram_free(unsigned long offset)
+int cpm_muram_free(s32 offset)
{
unsigned long flags;
int size;
@@ -194,13 +196,14 @@ EXPORT_SYMBOL(cpm_muram_free);
* cpm_muram_alloc_fixed - reserve a specific region of multi-user ram
* @offset: offset of allocation start address
* @size: number of bytes to allocate
- * This function returns an offset into the muram area
+ * This function returns @offset if the area was available, a negative
+ * errno otherwise.
* Use cpm_dpram_addr() to get the virtual address of the area.
* Use cpm_muram_free() to free the allocation.
*/
-unsigned long cpm_muram_alloc_fixed(unsigned long offset, unsigned long size)
+s32 cpm_muram_alloc_fixed(unsigned long offset, unsigned long size)
{
- unsigned long start;
+ s32 start;
[PATCH v5 34/48] serial: ucc_uart: access __be32 field using be32_to_cpu
The buf member of struct qe_bd is a __be32, so to make this work on
little-endian hosts, use be32_to_cpu when reading it.
Signed-off-by: Rasmus Villemoes
---
drivers/tty/serial/ucc_uart.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/ucc_uart.c b/drivers/tty/serial/ucc_uart.c
index 9436b93d5cfa..afc2a5d69202 100644
--- a/drivers/tty/serial/ucc_uart.c
+++ b/drivers/tty/serial/ucc_uart.c
@@ -343,7 +343,7 @@ static int qe_uart_tx_pump(struct uart_qe_port *qe_port)
/* Pick next descriptor and fill from buffer */
bdp = qe_port->tx_cur;
- p = qe2cpu_addr(bdp->buf, qe_port);
+ p = qe2cpu_addr(be32_to_cpu(bdp->buf), qe_port);
*p++ = port->x_char;
qe_iowrite16be(1, &bdp->length);
@@ -371,7 +371,7 @@ static int qe_uart_tx_pump(struct uart_qe_port *qe_port)
while (!(qe_ioread16be(&bdp->status) & BD_SC_READY) &&
(xmit->tail != xmit->head)) {
count = 0;
- p = qe2cpu_addr(bdp->buf, qe_port);
+ p = qe2cpu_addr(be32_to_cpu(bdp->buf), qe_port);
while (count < qe_port->tx_fifosize) {
*p++ = xmit->buf[xmit->tail];
xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
@@ -491,7 +491,7 @@ static void qe_uart_int_rx(struct uart_qe_port *qe_port)
}
/* get pointer */
- cp = qe2cpu_addr(bdp->buf, qe_port);
+ cp = qe2cpu_addr(be32_to_cpu(bdp->buf), qe_port);
/* loop through the buffer */
while (i-- > 0) {
--
2.23.0
[PATCH v5 33/48] serial: ucc_uart: limit brg-frequency workaround to PPC32
According to Timur Tabi
This bug in older U-Boots is definitely PowerPC-specific
So before allowing this driver to be built for platforms other than
PPC32, make sure that we don't accept malformed device trees on those
other platforms.
Suggested-by: Timur Tabi
Signed-off-by: Rasmus Villemoes
---
drivers/tty/serial/ucc_uart.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/tty/serial/ucc_uart.c b/drivers/tty/serial/ucc_uart.c
index c055abf4c919..9436b93d5cfa 100644
--- a/drivers/tty/serial/ucc_uart.c
+++ b/drivers/tty/serial/ucc_uart.c
@@ -1392,6 +1392,13 @@ static int ucc_uart_probe(struct platform_device *ofdev)
if (val)
qe_port->port.uartclk = val;
else {
+ if (!IS_ENABLED(CONFIG_PPC32)) {
+ dev_err(&ofdev->dev,
+ "invalid brg-frequency in device tree\n");
+ ret = -EINVAL;
+ goto out_np;
+ }
+
/*
* Older versions of U-Boot do not initialize the brg-frequency
* property, so in this case we assume the BRG frequency is
--
2.23.0
[PATCH v5 32/48] serial: ucc_uart: use of_property_read_u32() in ucc_uart_probe()
For this to work correctly on little-endian hosts, don't access the
device-tree properties directly in native endianness, but use the
of_property_read_u32() helper.
Signed-off-by: Rasmus Villemoes
---
drivers/tty/serial/ucc_uart.c | 36 +++
1 file changed, 15 insertions(+), 21 deletions(-)
diff --git a/drivers/tty/serial/ucc_uart.c b/drivers/tty/serial/ucc_uart.c
index 313697842e24..c055abf4c919 100644
--- a/drivers/tty/serial/ucc_uart.c
+++ b/drivers/tty/serial/ucc_uart.c
@@ -1256,10 +1256,10 @@ static int soft_uart_init(struct platform_device *ofdev)
static int ucc_uart_probe(struct platform_device *ofdev)
{
struct device_node *np = ofdev->dev.of_node;
- const unsigned int *iprop; /* Integer OF properties */
const char *sprop; /* String OF properties */
struct uart_qe_port *qe_port = NULL;
struct resource res;
+ u32 val;
int ret;
/*
@@ -1290,23 +1290,20 @@ static int ucc_uart_probe(struct platform_device *ofdev)
/* Get the UCC number (device ID) */
/* UCCs are numbered 1-7 */
- iprop = of_get_property(np, "cell-index", NULL);
- if (!iprop) {
- iprop = of_get_property(np, "device-id", NULL);
- if (!iprop) {
- dev_err(&ofdev->dev, "UCC is unspecified in "
- "device tree\n");
+ if (of_property_read_u32(np, "cell-index", &val)) {
+ if (of_property_read_u32(np, "device-id", &val)) {
+ dev_err(&ofdev->dev, "UCC is unspecified in device
tree\n");
ret = -EINVAL;
goto out_free;
}
}
- if ((*iprop < 1) || (*iprop > UCC_MAX_NUM)) {
- dev_err(&ofdev->dev, "no support for UCC%u\n", *iprop);
+ if (val < 1 || val > UCC_MAX_NUM) {
+ dev_err(&ofdev->dev, "no support for UCC%u\n", val);
ret = -ENODEV;
goto out_free;
}
- qe_port->ucc_num = *iprop - 1;
+ qe_port->ucc_num = val - 1;
/*
* In the future, we should not require the BRG to be specified in the
@@ -1350,13 +1347,12 @@ static int ucc_uart_probe(struct platform_device *ofdev)
}
/* Get the port number, numbered 0-3 */
- iprop = of_get_property(np, "port-number", NULL);
- if (!iprop) {
+ if (of_property_read_u32(np, "port-number", &val)) {
dev_err(&ofdev->dev, "missing port-number in device tree\n");
ret = -EINVAL;
goto out_free;
}
- qe_port->port.line = *iprop;
+ qe_port->port.line = val;
if (qe_port->port.line >= UCC_MAX_UART) {
dev_err(&ofdev->dev, "port-number must be 0-%u\n",
UCC_MAX_UART - 1);
@@ -1386,31 +1382,29 @@ static int ucc_uart_probe(struct platform_device *ofdev)
}
}
- iprop = of_get_property(np, "brg-frequency", NULL);
- if (!iprop) {
+ if (of_property_read_u32(np, "brg-frequency", &val)) {
dev_err(&ofdev->dev,
"missing brg-frequency in device tree\n");
ret = -EINVAL;
goto out_np;
}
- if (*iprop)
- qe_port->port.uartclk = *iprop;
+ if (val)
+ qe_port->port.uartclk = val;
else {
/*
* Older versions of U-Boot do not initialize the brg-frequency
* property, so in this case we assume the BRG frequency is
* half the QE bus frequency.
*/
- iprop = of_get_property(np, "bus-frequency", NULL);
- if (!iprop) {
+ if (of_property_read_u32(np, "bus-frequency", &val)) {
dev_err(&ofdev->dev,
"missing QE bus-frequency in device tree\n");
ret = -EINVAL;
goto out_np;
}
- if (*iprop)
- qe_port->port.uartclk = *iprop / 2;
+ if (val)
+ qe_port->port.uartclk = val / 2;
else {
dev_err(&ofdev->dev,
"invalid QE bus-frequency in device tree\n");
--
2.23.0
[PATCH v5 30/48] serial: ucc_uart: factor out soft_uart initialization
The "soft uart" mechanism is a workaround for a silicon bug which (as
far as I know) only affects some PPC-based SOCs.
The code that determines which microcode blob to request relies on
some powerpc-specific bits (e.g. the mfspr(SPRN_SVR) and hence also
the asm/reg.h header). This makes it a little awkward to allow this
driver to be built for non-PPC based SOCs with a QE, even if they are
not affected by that silicon bug and thus don't need any of the Soft
UART logic.
There's no way around guarding those bits with some ifdeffery, so to
keep that isolated, factor out the
do-we-need-soft-uart-and-if-so-handle-the-firmware to a separate
function, which we can then easily stub out for non-PPC.
Signed-off-by: Rasmus Villemoes
---
drivers/tty/serial/ucc_uart.c | 110 ++
1 file changed, 58 insertions(+), 52 deletions(-)
diff --git a/drivers/tty/serial/ucc_uart.c b/drivers/tty/serial/ucc_uart.c
index 8a378ee5d34f..f286e91714cb 100644
--- a/drivers/tty/serial/ucc_uart.c
+++ b/drivers/tty/serial/ucc_uart.c
@@ -1183,70 +1183,76 @@ static void uart_firmware_cont(const struct firmware
*fw, void *context)
release_firmware(fw);
}
-static int ucc_uart_probe(struct platform_device *ofdev)
+static int soft_uart_init(struct platform_device *ofdev)
{
struct device_node *np = ofdev->dev.of_node;
- const unsigned int *iprop; /* Integer OF properties */
- const char *sprop; /* String OF properties */
- struct uart_qe_port *qe_port = NULL;
- struct resource res;
+ struct qe_firmware_info *qe_fw_info;
int ret;
- /*
-* Determine if we need Soft-UART mode
-*/
if (of_find_property(np, "soft-uart", NULL)) {
dev_dbg(&ofdev->dev, "using Soft-UART mode\n");
soft_uart = 1;
+ } else {
+ return 0;
}
- /*
-* If we are using Soft-UART, determine if we need to upload the
-* firmware, too.
-*/
- if (soft_uart) {
- struct qe_firmware_info *qe_fw_info;
-
- qe_fw_info = qe_get_firmware_info();
-
- /* Check if the firmware has been uploaded. */
- if (qe_fw_info && strstr(qe_fw_info->id, "Soft-UART")) {
- firmware_loaded = 1;
- } else {
- char filename[32];
- unsigned int soc;
- unsigned int rev_h;
- unsigned int rev_l;
-
- soc = soc_info(&rev_h, &rev_l);
- if (!soc) {
- dev_err(&ofdev->dev, "unknown CPU model\n");
- return -ENXIO;
- }
- sprintf(filename, "fsl_qe_ucode_uart_%u_%u%u.bin",
- soc, rev_h, rev_l);
-
- dev_info(&ofdev->dev, "waiting for firmware %s\n",
- filename);
+ qe_fw_info = qe_get_firmware_info();
- /*
-* We call request_firmware_nowait instead of
-* request_firmware so that the driver can load and
-* initialize the ports without holding up the rest of
-* the kernel. If hotplug support is enabled in the
-* kernel, then we use it.
-*/
- ret = request_firmware_nowait(THIS_MODULE,
- FW_ACTION_HOTPLUG, filename, &ofdev->dev,
- GFP_KERNEL, &ofdev->dev, uart_firmware_cont);
- if (ret) {
- dev_err(&ofdev->dev,
- "could not load firmware %s\n",
- filename);
- return ret;
- }
+ /* Check if the firmware has been uploaded. */
+ if (qe_fw_info && strstr(qe_fw_info->id, "Soft-UART")) {
+ firmware_loaded = 1;
+ } else {
+ char filename[32];
+ unsigned int soc;
+ unsigned int rev_h;
+ unsigned int rev_l;
+
+ soc = soc_info(&rev_h, &rev_l);
+ if (!soc) {
+ dev_err(&ofdev->dev, "unknown CPU model\n");
+ return -ENXIO;
+ }
+ sprintf(filename, "fsl_qe_ucode_uart_%u_%u%u.bin",
+ soc, rev_h, rev_l);
+
+ dev_info(&ofdev->dev, "waiting for firmware %s\n",
+filename);
+
+ /*
+* We call request_firmware_nowait instead of
+* request_firmware so that the driver can load and
+* initialize the ports without holding up the rest of
+* the kernel. If hotplug support is enabled in
[PATCH v5 31/48] serial: ucc_uart: stub out soft_uart_init for !CONFIG_PPC32
The Soft UART hack is only needed for some PPC-based SOCs. To allow
building this driver for non-PPC, guard soft_uart_init() and its
helpers by CONFIG_PPC32, and use a no-op soft_uart_init() otherwise.
Signed-off-by: Rasmus Villemoes
---
drivers/tty/serial/ucc_uart.c | 17 -
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/ucc_uart.c b/drivers/tty/serial/ucc_uart.c
index f286e91714cb..313697842e24 100644
--- a/drivers/tty/serial/ucc_uart.c
+++ b/drivers/tty/serial/ucc_uart.c
@@ -33,7 +33,10 @@
#include
#include
-#include
+
+#ifdef CONFIG_PPC32
+#include /* mfspr, SPRN_SVR */
+#endif
/*
* The GUMR flag for Soft UART. This would normally be defined in qe.h,
@@ -1096,6 +1099,8 @@ static const struct uart_ops qe_uart_pops = {
.verify_port= qe_uart_verify_port,
};
+
+#ifdef CONFIG_PPC32
/*
* Obtain the SOC model number and revision level
*
@@ -1238,6 +1243,16 @@ static int soft_uart_init(struct platform_device *ofdev)
return 0;
}
+#else /* !CONFIG_PPC32 */
+
+static int soft_uart_init(struct platform_device *ofdev)
+{
+ return 0;
+}
+
+#endif
+
+
static int ucc_uart_probe(struct platform_device *ofdev)
{
struct device_node *np = ofdev->dev.of_node;
--
2.23.0
[PATCH v5 29/48] serial: ucc_uart: replace ppc-specific IO accessors
Some ARM-based SOCs (e.g. LS1021A) also have a QUICC engine. As
preparation for allowing this driver to build on ARM, replace the
ppc-specific in_be16() etc. by the qe_io* helpers. Done via
coccinelle.
Signed-off-by: Rasmus Villemoes
---
drivers/tty/serial/ucc_uart.c | 210 +-
1 file changed, 102 insertions(+), 108 deletions(-)
diff --git a/drivers/tty/serial/ucc_uart.c b/drivers/tty/serial/ucc_uart.c
index 7e802616cba8..8a378ee5d34f 100644
--- a/drivers/tty/serial/ucc_uart.c
+++ b/drivers/tty/serial/ucc_uart.c
@@ -258,11 +258,11 @@ static unsigned int qe_uart_tx_empty(struct uart_port
*port)
struct qe_bd *bdp = qe_port->tx_bd_base;
while (1) {
- if (in_be16(&bdp->status) & BD_SC_READY)
+ if (qe_ioread16be(&bdp->status) & BD_SC_READY)
/* This BD is not done, so return "not done" */
return 0;
- if (in_be16(&bdp->status) & BD_SC_WRAP)
+ if (qe_ioread16be(&bdp->status) & BD_SC_WRAP)
/*
* This BD is done and it's the last one, so return
* "done"
@@ -308,7 +308,7 @@ static void qe_uart_stop_tx(struct uart_port *port)
struct uart_qe_port *qe_port =
container_of(port, struct uart_qe_port, port);
- clrbits16(&qe_port->uccp->uccm, UCC_UART_UCCE_TX);
+ qe_clrbits_be16(&qe_port->uccp->uccm, UCC_UART_UCCE_TX);
}
/*
@@ -343,10 +343,10 @@ static int qe_uart_tx_pump(struct uart_qe_port *qe_port)
p = qe2cpu_addr(bdp->buf, qe_port);
*p++ = port->x_char;
- out_be16(&bdp->length, 1);
- setbits16(&bdp->status, BD_SC_READY);
+ qe_iowrite16be(1, &bdp->length);
+ qe_setbits_be16(&bdp->status, BD_SC_READY);
/* Get next BD. */
- if (in_be16(&bdp->status) & BD_SC_WRAP)
+ if (qe_ioread16be(&bdp->status) & BD_SC_WRAP)
bdp = qe_port->tx_bd_base;
else
bdp++;
@@ -365,7 +365,7 @@ static int qe_uart_tx_pump(struct uart_qe_port *qe_port)
/* Pick next descriptor and fill from buffer */
bdp = qe_port->tx_cur;
- while (!(in_be16(&bdp->status) & BD_SC_READY) &&
+ while (!(qe_ioread16be(&bdp->status) & BD_SC_READY) &&
(xmit->tail != xmit->head)) {
count = 0;
p = qe2cpu_addr(bdp->buf, qe_port);
@@ -378,11 +378,11 @@ static int qe_uart_tx_pump(struct uart_qe_port *qe_port)
break;
}
- out_be16(&bdp->length, count);
- setbits16(&bdp->status, BD_SC_READY);
+ qe_iowrite16be(count, &bdp->length);
+ qe_setbits_be16(&bdp->status, BD_SC_READY);
/* Get next BD. */
- if (in_be16(&bdp->status) & BD_SC_WRAP)
+ if (qe_ioread16be(&bdp->status) & BD_SC_WRAP)
bdp = qe_port->tx_bd_base;
else
bdp++;
@@ -415,12 +415,12 @@ static void qe_uart_start_tx(struct uart_port *port)
container_of(port, struct uart_qe_port, port);
/* If we currently are transmitting, then just return */
- if (in_be16(&qe_port->uccp->uccm) & UCC_UART_UCCE_TX)
+ if (qe_ioread16be(&qe_port->uccp->uccm) & UCC_UART_UCCE_TX)
return;
/* Otherwise, pump the port and start transmission */
if (qe_uart_tx_pump(qe_port))
- setbits16(&qe_port->uccp->uccm, UCC_UART_UCCE_TX);
+ qe_setbits_be16(&qe_port->uccp->uccm, UCC_UART_UCCE_TX);
}
/*
@@ -431,7 +431,7 @@ static void qe_uart_stop_rx(struct uart_port *port)
struct uart_qe_port *qe_port =
container_of(port, struct uart_qe_port, port);
- clrbits16(&qe_port->uccp->uccm, UCC_UART_UCCE_RX);
+ qe_clrbits_be16(&qe_port->uccp->uccm, UCC_UART_UCCE_RX);
}
/* Start or stop sending break signal
@@ -470,14 +470,14 @@ static void qe_uart_int_rx(struct uart_qe_port *qe_port)
*/
bdp = qe_port->rx_cur;
while (1) {
- status = in_be16(&bdp->status);
+ status = qe_ioread16be(&bdp->status);
/* If this one is empty, then we assume we've read them all */
if (status & BD_SC_EMPTY)
break;
/* get number of characters, and check space in RX buffer */
- i = in_be16(&bdp->length);
+ i = qe_ioread16be(&bdp->length);
/* If we don't have enough room in RX buffer for the entire BD,
* then we try later, which will be the next RX interrupt.
@@ -508,9 +508,10 @@ static void qe_uart_int_rx(struct uart_qe_port *qe_port)
}
/* This BD is ready to be used again. Clear status.
[PATCH v5 28/48] serial: ucc_uart: explicitly include soc/fsl/cpm.h
This driver uses #defines from soc/fsl/cpm.h, so instead of relying on some other header pulling that in, do that explicitly. This is preparation for allowing this driver to build on ARM. Signed-off-by: Rasmus Villemoes --- drivers/tty/serial/ucc_uart.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/tty/serial/ucc_uart.c b/drivers/tty/serial/ucc_uart.c index a0555ae2b1ef..7e802616cba8 100644 --- a/drivers/tty/serial/ucc_uart.c +++ b/drivers/tty/serial/ucc_uart.c @@ -32,6 +32,7 @@ #include #include +#include #include /* -- 2.23.0
[PATCH v5 26/48] soc: fsl: move cpm.h from powerpc/include/asm to include/soc/fsl
Some drivers, e.g. ucc_uart, need definitions from cpm.h. In order to
allow building those drivers for non-ppc based SOCs, move the header
to include/soc/fsl. For now, leave a trivial wrapper at the old
location so drivers can be updated one by one.
Signed-off-by: Rasmus Villemoes
---
arch/powerpc/include/asm/cpm.h | 172 +
include/soc/fsl/cpm.h | 171
2 files changed, 172 insertions(+), 171 deletions(-)
create mode 100644 include/soc/fsl/cpm.h
diff --git a/arch/powerpc/include/asm/cpm.h b/arch/powerpc/include/asm/cpm.h
index 4c24ea8209bb..ce483b0f8a4d 100644
--- a/arch/powerpc/include/asm/cpm.h
+++ b/arch/powerpc/include/asm/cpm.h
@@ -1,171 +1 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef __CPM_H
-#define __CPM_H
-
-#include
-#include
-#include
-#include
-#include
-
-/*
- * SPI Parameter RAM common to QE and CPM.
- */
-struct spi_pram {
- __be16 rbase; /* Rx Buffer descriptor base address */
- __be16 tbase; /* Tx Buffer descriptor base address */
- u8 rfcr; /* Rx function code */
- u8 tfcr; /* Tx function code */
- __be16 mrblr; /* Max receive buffer length */
- __be32 rstate; /* Internal */
- __be32 rdp;/* Internal */
- __be16 rbptr; /* Internal */
- __be16 rbc;/* Internal */
- __be32 rxtmp; /* Internal */
- __be32 tstate; /* Internal */
- __be32 tdp;/* Internal */
- __be16 tbptr; /* Internal */
- __be16 tbc;/* Internal */
- __be32 txtmp; /* Internal */
- __be32 res;/* Tx temp. */
- __be16 rpbase; /* Relocation pointer (CPM1 only) */
- __be16 res1; /* Reserved */
-};
-
-/*
- * USB Controller pram common to QE and CPM.
- */
-struct usb_ctlr {
- u8 usb_usmod;
- u8 usb_usadr;
- u8 usb_uscom;
- u8 res1[1];
- __be16 usb_usep[4];
- u8 res2[4];
- __be16 usb_usber;
- u8 res3[2];
- __be16 usb_usbmr;
- u8 res4[1];
- u8 usb_usbs;
- /* Fields down below are QE-only */
- __be16 usb_ussft;
- u8 res5[2];
- __be16 usb_usfrn;
- u8 res6[0x22];
-} __attribute__ ((packed));
-
-/*
- * Function code bits, usually generic to devices.
- */
-#ifdef CONFIG_CPM1
-#define CPMFCR_GBL ((u_char)0x00) /* Flag doesn't exist in CPM1 */
-#define CPMFCR_TC2 ((u_char)0x00) /* Flag doesn't exist in CPM1 */
-#define CPMFCR_DTB ((u_char)0x00) /* Flag doesn't exist in CPM1 */
-#define CPMFCR_BDB ((u_char)0x00) /* Flag doesn't exist in CPM1 */
-#else
-#define CPMFCR_GBL ((u_char)0x20) /* Set memory snooping */
-#define CPMFCR_TC2 ((u_char)0x04) /* Transfer code 2 value */
-#define CPMFCR_DTB ((u_char)0x02) /* Use local bus for data when set */
-#define CPMFCR_BDB ((u_char)0x01) /* Use local bus for BD when set */
-#endif
-#define CPMFCR_EB ((u_char)0x10) /* Set big endian byte order */
-
-/* Opcodes common to CPM1 and CPM2
-*/
-#define CPM_CR_INIT_TRX((ushort)0x)
-#define CPM_CR_INIT_RX ((ushort)0x0001)
-#define CPM_CR_INIT_TX ((ushort)0x0002)
-#define CPM_CR_HUNT_MODE ((ushort)0x0003)
-#define CPM_CR_STOP_TX ((ushort)0x0004)
-#define CPM_CR_GRA_STOP_TX ((ushort)0x0005)
-#define CPM_CR_RESTART_TX ((ushort)0x0006)
-#define CPM_CR_CLOSE_RX_BD ((ushort)0x0007)
-#define CPM_CR_SET_GADDR ((ushort)0x0008)
-#define CPM_CR_SET_TIMER ((ushort)0x0008)
-#define CPM_CR_STOP_IDMA ((ushort)0x000b)
-
-/* Buffer descriptors used by many of the CPM protocols. */
-typedef struct cpm_buf_desc {
- ushort cbd_sc; /* Status and Control */
- ushort cbd_datlen; /* Data length in buffer */
- uintcbd_bufaddr;/* Buffer address in host memory */
-} cbd_t;
-
-/* Buffer descriptor control/status used by serial
- */
-
-#define BD_SC_EMPTY(0x8000)/* Receive is empty */
-#define BD_SC_READY(0x8000)/* Transmit is ready */
-#define BD_SC_WRAP (0x2000)/* Last buffer descriptor */
-#define BD_SC_INTRPT (0x1000)/* Interrupt on change */
-#define BD_SC_LAST (0x0800)/* Last buffer in frame */
-#define BD_SC_TC (0x0400)/* Transmit CRC */
-#define BD_SC_CM (0x0200)/* Continuous mode */
-#define BD_SC_ID (0x0100)/* Rec'd too many idles */
-#define BD_SC_P(0x0100)/* xmt preamble */
-#define BD_SC_BR (0x0020)/* Break received */
-#define BD_SC_FR (0x0010)/* Framing error */
-#define BD_SC_PR (0x0008)/* Parity error */
-#define BD_SC_NAK (0x0004)/* NAK - did not respond */
-#define BD_SC_OV (0x0002)/* Overrun */
-#define BD_SC_UN (0x0002)/* Underrun */
-#define BD_SC_CD (0x0001)/* */
-#define BD_SC_CL (0
[PATCH v5 27/48] soc/fsl/qe/qe.h: update include path for cpm.h
asm/cpm.h under arch/powerpc is now just a wrapper for including soc/fsl/cpm.h. In order to make the qe.h header usable on other architectures, use the latter path directly. Signed-off-by: Rasmus Villemoes --- include/soc/fsl/qe/qe.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/soc/fsl/qe/qe.h b/include/soc/fsl/qe/qe.h index 9cac04c692fd..521fa3a177e0 100644 --- a/include/soc/fsl/qe/qe.h +++ b/include/soc/fsl/qe/qe.h @@ -17,7 +17,7 @@ #include #include #include -#include +#include #include #include #include -- 2.23.0
[PATCH v5 25/48] soc: fsl: qe: qe_io.c: use of_property_read_u32() in par_io_init()
This is necessary for this to work on little-endian hosts.
Signed-off-by: Rasmus Villemoes
---
drivers/soc/fsl/qe/qe_io.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/soc/fsl/qe/qe_io.c b/drivers/soc/fsl/qe/qe_io.c
index 61dd8eb8c0fe..11ea08e97db7 100644
--- a/drivers/soc/fsl/qe/qe_io.c
+++ b/drivers/soc/fsl/qe/qe_io.c
@@ -28,7 +28,7 @@ int par_io_init(struct device_node *np)
{
struct resource res;
int ret;
- const u32 *num_ports;
+ u32 num_ports;
/* Map Parallel I/O ports registers */
ret = of_address_to_resource(np, 0, &res);
@@ -36,9 +36,8 @@ int par_io_init(struct device_node *np)
return ret;
par_io = ioremap(res.start, resource_size(&res));
- num_ports = of_get_property(np, "num-ports", NULL);
- if (num_ports)
- num_par_io_ports = *num_ports;
+ if (!of_property_read_u32(np, "num-ports", &num_ports))
+ num_par_io_ports = num_ports;
return 0;
}
--
2.23.0
[PATCH v5 22/48] soc: fsl: qe: qe.c: use of_property_read_* helpers
Instead of manually doing of_get_property/of_find_property and reading
the value by assigning to a u32* or u64* and dereferencing, use the
of_property_read_* functions.
This make the code more readable, and more importantly, is required
for this to work correctly on little-endian platforms.
Signed-off-by: Rasmus Villemoes
---
drivers/soc/fsl/qe/qe.c | 33 -
1 file changed, 8 insertions(+), 25 deletions(-)
diff --git a/drivers/soc/fsl/qe/qe.c b/drivers/soc/fsl/qe/qe.c
index a4763282ea68..ec511840db3c 100644
--- a/drivers/soc/fsl/qe/qe.c
+++ b/drivers/soc/fsl/qe/qe.c
@@ -159,8 +159,7 @@ static unsigned int brg_clk = 0;
unsigned int qe_get_brg_clk(void)
{
struct device_node *qe;
- int size;
- const u32 *prop;
+ u32 brg;
unsigned int mod;
if (brg_clk)
@@ -170,9 +169,8 @@ unsigned int qe_get_brg_clk(void)
if (!qe)
return brg_clk;
- prop = of_get_property(qe, "brg-frequency", &size);
- if (prop && size == sizeof(*prop))
- brg_clk = *prop;
+ if (!of_property_read_u32(qe, "brg-frequency", &brg))
+ brg_clk = brg;
of_node_put(qe);
@@ -571,11 +569,9 @@ EXPORT_SYMBOL(qe_upload_firmware);
struct qe_firmware_info *qe_get_firmware_info(void)
{
static int initialized;
- struct property *prop;
struct device_node *qe;
struct device_node *fw = NULL;
const char *sprop;
- unsigned int i;
/*
* If we haven't checked yet, and a driver hasn't uploaded a firmware
@@ -609,20 +605,11 @@ struct qe_firmware_info *qe_get_firmware_info(void)
strlcpy(qe_firmware_info.id, sprop,
sizeof(qe_firmware_info.id));
- prop = of_find_property(fw, "extended-modes", NULL);
- if (prop && (prop->length == sizeof(u64))) {
- const u64 *iprop = prop->value;
-
- qe_firmware_info.extended_modes = *iprop;
- }
+ of_property_read_u64(fw, "extended-modes",
+&qe_firmware_info.extended_modes);
- prop = of_find_property(fw, "virtual-traps", NULL);
- if (prop && (prop->length == 32)) {
- const u32 *iprop = prop->value;
-
- for (i = 0; i < ARRAY_SIZE(qe_firmware_info.vtraps); i++)
- qe_firmware_info.vtraps[i] = iprop[i];
- }
+ of_property_read_u32_array(fw, "virtual-traps", qe_firmware_info.vtraps,
+ ARRAY_SIZE(qe_firmware_info.vtraps));
of_node_put(fw);
@@ -633,17 +620,13 @@ EXPORT_SYMBOL(qe_get_firmware_info);
unsigned int qe_get_num_of_risc(void)
{
struct device_node *qe;
- int size;
unsigned int num_of_risc = 0;
- const u32 *prop;
qe = qe_get_device_node();
if (!qe)
return num_of_risc;
- prop = of_get_property(qe, "fsl,qe-num-riscs", &size);
- if (prop && size == sizeof(*prop))
- num_of_risc = *prop;
+ of_property_read_u32(qe, "fsl,qe-num-riscs", &num_of_risc);
of_node_put(qe);
--
2.23.0
[PATCH v5 24/48] soc: fsl: qe: qe_io.c: access device tree property using be32_to_cpu
We need to apply be32_to_cpu to make this work correctly on
little-endian hosts.
Signed-off-by: Rasmus Villemoes
---
drivers/soc/fsl/qe/qe_io.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/soc/fsl/qe/qe_io.c b/drivers/soc/fsl/qe/qe_io.c
index 99aeb01586bd..61dd8eb8c0fe 100644
--- a/drivers/soc/fsl/qe/qe_io.c
+++ b/drivers/soc/fsl/qe/qe_io.c
@@ -142,7 +142,7 @@ int par_io_of_config(struct device_node *np)
{
struct device_node *pio;
int pio_map_len;
- const unsigned int *pio_map;
+ const __be32 *pio_map;
if (par_io == NULL) {
printk(KERN_ERR "par_io not initialized\n");
@@ -167,9 +167,15 @@ int par_io_of_config(struct device_node *np)
}
while (pio_map_len > 0) {
- par_io_config_pin((u8) pio_map[0], (u8) pio_map[1],
- (int) pio_map[2], (int) pio_map[3],
- (int) pio_map[4], (int) pio_map[5]);
+ u8 port= be32_to_cpu(pio_map[0]);
+ u8 pin = be32_to_cpu(pio_map[1]);
+ int dir= be32_to_cpu(pio_map[2]);
+ int open_drain = be32_to_cpu(pio_map[3]);
+ int assignment = be32_to_cpu(pio_map[4]);
+ int has_irq= be32_to_cpu(pio_map[5]);
+
+ par_io_config_pin(port, pin, dir, open_drain,
+ assignment, has_irq);
pio_map += 6;
pio_map_len -= 6;
}
--
2.23.0
[PATCH v5 23/48] soc: fsl: qe: qe_io.c: don't open-code of_parse_phandle()
Signed-off-by: Rasmus Villemoes
---
drivers/soc/fsl/qe/qe_io.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/soc/fsl/qe/qe_io.c b/drivers/soc/fsl/qe/qe_io.c
index f6b10f38b2f4..99aeb01586bd 100644
--- a/drivers/soc/fsl/qe/qe_io.c
+++ b/drivers/soc/fsl/qe/qe_io.c
@@ -141,7 +141,6 @@ EXPORT_SYMBOL(par_io_data_set);
int par_io_of_config(struct device_node *np)
{
struct device_node *pio;
- const phandle *ph;
int pio_map_len;
const unsigned int *pio_map;
@@ -150,14 +149,12 @@ int par_io_of_config(struct device_node *np)
return -1;
}
- ph = of_get_property(np, "pio-handle", NULL);
- if (ph == NULL) {
+ pio = of_parse_phandle(np, "pio-handle", 0);
+ if (pio == NULL) {
printk(KERN_ERR "pio-handle not available\n");
return -1;
}
- pio = of_find_node_by_phandle(*ph);
-
pio_map = of_get_property(pio, "pio-map", &pio_map_len);
if (pio_map == NULL) {
printk(KERN_ERR "pio-map is not set!\n");
--
2.23.0
[PATCH v5 21/48] soc: fsl: qe: merge qe_ic.h headers into qe_ic.c
The public qe_ic.h header is no longer included by anything but
qe_ic.c. Merge both headers into qe_ic.c, and drop the unused
constants.
Signed-off-by: Rasmus Villemoes
---
drivers/soc/fsl/qe/qe_ic.c | 52 +++-
drivers/soc/fsl/qe/qe_ic.h | 99 --
include/soc/fsl/qe/qe_ic.h | 56 -
3 files changed, 50 insertions(+), 157 deletions(-)
delete mode 100644 drivers/soc/fsl/qe/qe_ic.h
delete mode 100644 include/soc/fsl/qe/qe_ic.h
diff --git a/drivers/soc/fsl/qe/qe_ic.c b/drivers/soc/fsl/qe/qe_ic.c
index 4832884da5bb..0dd5bdb04a14 100644
--- a/drivers/soc/fsl/qe/qe_ic.c
+++ b/drivers/soc/fsl/qe/qe_ic.c
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -25,9 +26,56 @@
#include
#include
#include
-#include
-#include "qe_ic.h"
+#define NR_QE_IC_INTS 64
+
+/* QE IC registers offset */
+#define QEIC_CICR 0x00
+#define QEIC_CIVEC 0x04
+#define QEIC_CIPXCC0x10
+#define QEIC_CIPYCC0x14
+#define QEIC_CIPWCC0x18
+#define QEIC_CIPZCC0x1c
+#define QEIC_CIMR 0x20
+#define QEIC_CRIMR 0x24
+#define QEIC_CIPRTA0x30
+#define QEIC_CIPRTB0x34
+#define QEIC_CHIVEC0x60
+
+struct qe_ic {
+ /* Control registers offset */
+ u32 __iomem *regs;
+
+ /* The remapper for this QEIC */
+ struct irq_domain *irqhost;
+
+ /* The "linux" controller struct */
+ struct irq_chip hc_irq;
+
+ /* VIRQ numbers of QE high/low irqs */
+ unsigned int virq_high;
+ unsigned int virq_low;
+};
+
+/*
+ * QE interrupt controller internal structure
+ */
+struct qe_ic_info {
+ /* Location of this source at the QIMR register */
+ u32 mask;
+
+ /* Mask register offset */
+ u32 mask_reg;
+
+ /*
+* For grouped interrupts sources - the interrupt code as
+* appears at the group priority register
+*/
+ u8 pri_code;
+
+ /* Group priority register offset */
+ u32 pri_reg;
+};
static DEFINE_RAW_SPINLOCK(qe_ic_lock);
diff --git a/drivers/soc/fsl/qe/qe_ic.h b/drivers/soc/fsl/qe/qe_ic.h
deleted file mode 100644
index 9420378d9b6b..
--- a/drivers/soc/fsl/qe/qe_ic.h
+++ /dev/null
@@ -1,99 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-or-later */
-/*
- * drivers/soc/fsl/qe/qe_ic.h
- *
- * QUICC ENGINE Interrupt Controller Header
- *
- * Copyright (C) 2006 Freescale Semiconductor, Inc. All rights reserved.
- *
- * Author: Li Yang
- * Based on code from Shlomi Gridish
- */
-#ifndef _POWERPC_SYSDEV_QE_IC_H
-#define _POWERPC_SYSDEV_QE_IC_H
-
-#include
-
-#define NR_QE_IC_INTS 64
-
-/* QE IC registers offset */
-#define QEIC_CICR 0x00
-#define QEIC_CIVEC 0x04
-#define QEIC_CRIPNR0x08
-#define QEIC_CIPNR 0x0c
-#define QEIC_CIPXCC0x10
-#define QEIC_CIPYCC0x14
-#define QEIC_CIPWCC0x18
-#define QEIC_CIPZCC0x1c
-#define QEIC_CIMR 0x20
-#define QEIC_CRIMR 0x24
-#define QEIC_CICNR 0x28
-#define QEIC_CIPRTA0x30
-#define QEIC_CIPRTB0x34
-#define QEIC_CRICR 0x3c
-#define QEIC_CHIVEC0x60
-
-/* Interrupt priority registers */
-#define CIPCC_SHIFT_PRI0 29
-#define CIPCC_SHIFT_PRI1 26
-#define CIPCC_SHIFT_PRI2 23
-#define CIPCC_SHIFT_PRI3 20
-#define CIPCC_SHIFT_PRI4 13
-#define CIPCC_SHIFT_PRI5 10
-#define CIPCC_SHIFT_PRI6 7
-#define CIPCC_SHIFT_PRI7 4
-
-/* CICR priority modes */
-#define CICR_GWCC 0x0004
-#define CICR_GXCC 0x0002
-#define CICR_GYCC 0x0001
-#define CICR_GZCC 0x0008
-#define CICR_GRTA 0x0020
-#define CICR_GRTB 0x0040
-#define CICR_HPIT_SHIFT8
-#define CICR_HPIT_MASK 0x0300
-#define CICR_HP_SHIFT 24
-#define CICR_HP_MASK 0x3f00
-
-/* CICNR */
-#define CICNR_WCC1T_SHIFT 20
-#define CICNR_ZCC1T_SHIFT 28
-#define CICNR_YCC1T_SHIFT 12
-#define CICNR_XCC1T_SHIFT 4
-
-/* CRICR */
-#define CRICR_RTA1T_SHIFT 20
-#define CRICR_RTB1T_SHIFT 28
-
-/* Signal indicator */
-#define SIGNAL_MASK3
-#define SIGNAL_HIGH2
-#define SIGNAL_LOW 0
-
-struct qe_ic {
- /* Control registers offset */
- u32 __iomem *regs;
-
- /* The remapper for this QEIC */
- struct irq_domain *irqhost;
-
- /* The "linux" controller struct */
- struct irq_chip hc_irq;
-
- /* VIRQ numbers of QE high/low irqs */
- unsigned int virq_high;
- unsigned int virq_low;
-};
-
-/*
- * QE interrupt controller internal structure
- */
-struct qe_ic_info {
- u32 mask; /* location of this source at
Re: [PATCH v4 2/2] powerpc/kexec: move kexec files into a dedicated subdir.
Christophe Leroy writes:
> arch/powerpc/kernel/ contains 8 files dedicated to kexec.
>
> Move them into a dedicated subdirectory.
>
> Signed-off-by: Christophe Leroy
>
> ---
> v2: moved crash.c as well as it's part of kexec suite.
> v3: renamed files to remove 'kexec' keyword from names.
> v4: removed a ifdef in kexec/Makefile
> ---
> arch/powerpc/kernel/Makefile | 19 +---
> arch/powerpc/kernel/kexec/Makefile | 25
> ++
> arch/powerpc/kernel/{ => kexec}/crash.c| 0
> .../kernel/{kexec_elf_64.c => kexec/elf_64.c} | 0
> arch/powerpc/kernel/{ima_kexec.c => kexec/ima.c} | 0
> .../kernel/{machine_kexec.c => kexec/machine.c}| 0
> .../{machine_kexec_32.c => kexec/machine_32.c} | 0
> .../{machine_kexec_64.c => kexec/machine_64.c} | 0
> .../machine_file_64.c} | 0
> .../{kexec_relocate_32.S => kexec/relocate_32.S} | 2 +-
> 10 files changed, 27 insertions(+), 19 deletions(-)
> create mode 100644 arch/powerpc/kernel/kexec/Makefile
> rename arch/powerpc/kernel/{ => kexec}/crash.c (100%)
> rename arch/powerpc/kernel/{kexec_elf_64.c => kexec/elf_64.c} (100%)
> rename arch/powerpc/kernel/{ima_kexec.c => kexec/ima.c} (100%)
> rename arch/powerpc/kernel/{machine_kexec.c => kexec/machine.c} (100%)
> rename arch/powerpc/kernel/{machine_kexec_32.c => kexec/machine_32.c} (100%)
> rename arch/powerpc/kernel/{machine_kexec_64.c => kexec/machine_64.c} (100%)
> rename arch/powerpc/kernel/{machine_kexec_file_64.c =>
> kexec/machine_file_64.c} (100%)
> rename arch/powerpc/kernel/{kexec_relocate_32.S => kexec/relocate_32.S} (99%)
I'm inclined to move the directory out of kernel, ie. up a level with mm
and so on.
And I also don't think the "machine" naming is useful anymore. It comes
from the naming of the arch functions, eg. machine_kexec(), which was
named to be analogous to machine_restart().
So how about:
arch/powerpc/{kernel/machine_kexec.c => kexec/core.c}
arch/powerpc/{kernel/machine_kexec_32.c => kexec/core_32.c}
arch/powerpc/{kernel/machine_kexec_64.c => kexec/core_64.c}
arch/powerpc/{kernel => kexec}/crash.c
arch/powerpc/{kernel/kexec_elf_64.c => kexec/elf_64.c}
arch/powerpc/{kernel/machine_kexec_file_64.c => kexec/file_load.c}
arch/powerpc/{kernel/ima_kexec.c => kexec/ima.c}
arch/powerpc/{kernel/kexec_relocate_32.S => kexec/relocate_32.S}
And we end up with:
$ find arch/powerpc/kexec
arch/powerpc/kexec/
arch/powerpc/kexec/file_load.c
arch/powerpc/kexec/relocate_32.S
arch/powerpc/kexec/core_64.c
arch/powerpc/kexec/ima.c
arch/powerpc/kexec/core.c
arch/powerpc/kexec/core_32.c
arch/powerpc/kexec/Makefile
arch/powerpc/kexec/crash.c
arch/powerpc/kexec/elf_64.c
cheers
[PATCH v5 20/48] soc: fsl: qe: simplify qe_ic_init()
qe_ic_init() takes a flags parameter, but all callers (including the
sole remaining one) have always passed 0. So remove that parameter and
simplify the body accordingly. We still explicitly initialize the
Interrupt Configuration Register (CICR) to its reset value of
all-zeroes, just in case the bootloader has played funny games.
Signed-off-by: Rasmus Villemoes
---
drivers/soc/fsl/qe/qe_ic.c | 27 ---
1 file changed, 4 insertions(+), 23 deletions(-)
diff --git a/drivers/soc/fsl/qe/qe_ic.c b/drivers/soc/fsl/qe/qe_ic.c
index 23b457e884d8..4832884da5bb 100644
--- a/drivers/soc/fsl/qe/qe_ic.c
+++ b/drivers/soc/fsl/qe/qe_ic.c
@@ -356,13 +356,13 @@ static void qe_ic_cascade_muxed_mpic(struct irq_desc
*desc)
chip->irq_eoi(&desc->irq_data);
}
-static void __init qe_ic_init(struct device_node *node, unsigned int flags)
+static void __init qe_ic_init(struct device_node *node)
{
void (*low_handler)(struct irq_desc *desc);
void (*high_handler)(struct irq_desc *desc);
struct qe_ic *qe_ic;
struct resource res;
- u32 temp = 0, ret;
+ u32 ret;
ret = of_address_to_resource(node, 0, &res);
if (ret)
@@ -399,26 +399,7 @@ static void __init qe_ic_init(struct device_node *node,
unsigned int flags)
high_handler = NULL;
}
- /* default priority scheme is grouped. If spread mode is*/
- /* required, configure cicr accordingly.*/
- if (flags & QE_IC_SPREADMODE_GRP_W)
- temp |= CICR_GWCC;
- if (flags & QE_IC_SPREADMODE_GRP_X)
- temp |= CICR_GXCC;
- if (flags & QE_IC_SPREADMODE_GRP_Y)
- temp |= CICR_GYCC;
- if (flags & QE_IC_SPREADMODE_GRP_Z)
- temp |= CICR_GZCC;
- if (flags & QE_IC_SPREADMODE_GRP_RISCA)
- temp |= CICR_GRTA;
- if (flags & QE_IC_SPREADMODE_GRP_RISCB)
- temp |= CICR_GRTB;
-
- /* choose destination signal for highest priority interrupt */
- if (flags & QE_IC_HIGH_SIGNAL)
- temp |= (SIGNAL_HIGH << CICR_HPIT_SHIFT);
-
- qe_ic_write(qe_ic->regs, QEIC_CICR, temp);
+ qe_ic_write(qe_ic->regs, QEIC_CICR, 0);
irq_set_handler_data(qe_ic->virq_low, qe_ic);
irq_set_chained_handler(qe_ic->virq_low, low_handler);
@@ -439,7 +420,7 @@ static int __init qe_ic_of_init(void)
if (!np)
return -ENODEV;
}
- qe_ic_init(np, 0);
+ qe_ic_init(np);
of_node_put(np);
return 0;
}
--
2.23.0
[PATCH v5 19/48] soc: fsl: qe: make qe_ic_get_{low,high}_irq static
These are only called from within qe_ic.c, so make them static.
Signed-off-by: Rasmus Villemoes
---
drivers/soc/fsl/qe/qe_ic.c | 4 ++--
include/soc/fsl/qe/qe_ic.h | 10 --
2 files changed, 2 insertions(+), 12 deletions(-)
diff --git a/drivers/soc/fsl/qe/qe_ic.c b/drivers/soc/fsl/qe/qe_ic.c
index 8f74bc6efd5c..23b457e884d8 100644
--- a/drivers/soc/fsl/qe/qe_ic.c
+++ b/drivers/soc/fsl/qe/qe_ic.c
@@ -283,7 +283,7 @@ static const struct irq_domain_ops qe_ic_host_ops = {
};
/* Return an interrupt vector or 0 if no interrupt is pending. */
-unsigned int qe_ic_get_low_irq(struct qe_ic *qe_ic)
+static unsigned int qe_ic_get_low_irq(struct qe_ic *qe_ic)
{
int irq;
@@ -299,7 +299,7 @@ unsigned int qe_ic_get_low_irq(struct qe_ic *qe_ic)
}
/* Return an interrupt vector or 0 if no interrupt is pending. */
-unsigned int qe_ic_get_high_irq(struct qe_ic *qe_ic)
+static unsigned int qe_ic_get_high_irq(struct qe_ic *qe_ic)
{
int irq;
diff --git a/include/soc/fsl/qe/qe_ic.h b/include/soc/fsl/qe/qe_ic.h
index d47eb231519e..70bb5a0f6535 100644
--- a/include/soc/fsl/qe/qe_ic.h
+++ b/include/soc/fsl/qe/qe_ic.h
@@ -53,14 +53,4 @@ enum qe_ic_grp_id {
QE_IC_GRP_RISCB /* QE interrupt controller RISC group B */
};
-#ifdef CONFIG_QUICC_ENGINE
-unsigned int qe_ic_get_low_irq(struct qe_ic *qe_ic);
-unsigned int qe_ic_get_high_irq(struct qe_ic *qe_ic);
-#else
-static inline unsigned int qe_ic_get_low_irq(struct qe_ic *qe_ic)
-{ return 0; }
-static inline unsigned int qe_ic_get_high_irq(struct qe_ic *qe_ic)
-{ return 0; }
-#endif /* CONFIG_QUICC_ENGINE */
-
#endif /* _ASM_POWERPC_QE_IC_H */
--
2.23.0
[PATCH v5 17/48] soc: fsl: qe: remove unused qe_ic_set_* functions
There are no current callers of these functions, and they use the
ppc-specific virq_to_hw(). So removing them gets us one step closer to
building QE support for ARM.
If the functionality is ever actually needed, the code can be dug out
of git and then adapted to work on all architectures, but for future
reference please note that I believe qe_ic_set_priority is buggy: The
"priority < 4" should be "priority <= 4", and in the else branch 24
should be replaced by 28, at least if I'm reading the data sheet right.
Signed-off-by: Rasmus Villemoes
---
drivers/soc/fsl/qe/qe_ic.c | 94 --
include/soc/fsl/qe/qe_ic.h | 4 --
2 files changed, 98 deletions(-)
diff --git a/drivers/soc/fsl/qe/qe_ic.c b/drivers/soc/fsl/qe/qe_ic.c
index de2ca2e3a648..4839dcd5c5d3 100644
--- a/drivers/soc/fsl/qe/qe_ic.c
+++ b/drivers/soc/fsl/qe/qe_ic.c
@@ -445,97 +445,3 @@ static int __init qe_ic_of_init(void)
return 0;
}
subsys_initcall(qe_ic_of_init);
-
-void qe_ic_set_highest_priority(unsigned int virq, int high)
-{
- struct qe_ic *qe_ic = qe_ic_from_irq(virq);
- unsigned int src = virq_to_hw(virq);
- u32 temp = 0;
-
- temp = qe_ic_read(qe_ic->regs, QEIC_CICR);
-
- temp &= ~CICR_HP_MASK;
- temp |= src << CICR_HP_SHIFT;
-
- temp &= ~CICR_HPIT_MASK;
- temp |= (high ? SIGNAL_HIGH : SIGNAL_LOW) << CICR_HPIT_SHIFT;
-
- qe_ic_write(qe_ic->regs, QEIC_CICR, temp);
-}
-
-/* Set Priority level within its group, from 1 to 8 */
-int qe_ic_set_priority(unsigned int virq, unsigned int priority)
-{
- struct qe_ic *qe_ic = qe_ic_from_irq(virq);
- unsigned int src = virq_to_hw(virq);
- u32 temp;
-
- if (priority > 8 || priority == 0)
- return -EINVAL;
- if (WARN_ONCE(src >= ARRAY_SIZE(qe_ic_info),
- "%s: Invalid hw irq number for QEIC\n", __func__))
- return -EINVAL;
- if (qe_ic_info[src].pri_reg == 0)
- return -EINVAL;
-
- temp = qe_ic_read(qe_ic->regs, qe_ic_info[src].pri_reg);
-
- if (priority < 4) {
- temp &= ~(0x7 << (32 - priority * 3));
- temp |= qe_ic_info[src].pri_code << (32 - priority * 3);
- } else {
- temp &= ~(0x7 << (24 - priority * 3));
- temp |= qe_ic_info[src].pri_code << (24 - priority * 3);
- }
-
- qe_ic_write(qe_ic->regs, qe_ic_info[src].pri_reg, temp);
-
- return 0;
-}
-
-/* Set a QE priority to use high irq, only priority 1~2 can use high irq */
-int qe_ic_set_high_priority(unsigned int virq, unsigned int priority, int high)
-{
- struct qe_ic *qe_ic = qe_ic_from_irq(virq);
- unsigned int src = virq_to_hw(virq);
- u32 temp, control_reg = QEIC_CICNR, shift = 0;
-
- if (priority > 2 || priority == 0)
- return -EINVAL;
- if (WARN_ONCE(src >= ARRAY_SIZE(qe_ic_info),
- "%s: Invalid hw irq number for QEIC\n", __func__))
- return -EINVAL;
-
- switch (qe_ic_info[src].pri_reg) {
- case QEIC_CIPZCC:
- shift = CICNR_ZCC1T_SHIFT;
- break;
- case QEIC_CIPWCC:
- shift = CICNR_WCC1T_SHIFT;
- break;
- case QEIC_CIPYCC:
- shift = CICNR_YCC1T_SHIFT;
- break;
- case QEIC_CIPXCC:
- shift = CICNR_XCC1T_SHIFT;
- break;
- case QEIC_CIPRTA:
- shift = CRICR_RTA1T_SHIFT;
- control_reg = QEIC_CRICR;
- break;
- case QEIC_CIPRTB:
- shift = CRICR_RTB1T_SHIFT;
- control_reg = QEIC_CRICR;
- break;
- default:
- return -EINVAL;
- }
-
- shift += (2 - priority) * 2;
- temp = qe_ic_read(qe_ic->regs, control_reg);
- temp &= ~(SIGNAL_MASK << shift);
- temp |= (high ? SIGNAL_HIGH : SIGNAL_LOW) << shift;
- qe_ic_write(qe_ic->regs, control_reg, temp);
-
- return 0;
-}
diff --git a/include/soc/fsl/qe/qe_ic.h b/include/soc/fsl/qe/qe_ic.h
index 43e4ce95c6a0..d47eb231519e 100644
--- a/include/soc/fsl/qe/qe_ic.h
+++ b/include/soc/fsl/qe/qe_ic.h
@@ -63,8 +63,4 @@ static inline unsigned int qe_ic_get_high_irq(struct qe_ic
*qe_ic)
{ return 0; }
#endif /* CONFIG_QUICC_ENGINE */
-void qe_ic_set_highest_priority(unsigned int virq, int high);
-int qe_ic_set_priority(unsigned int virq, unsigned int priority);
-int qe_ic_set_high_priority(unsigned int virq, unsigned int priority, int
high);
-
#endif /* _ASM_POWERPC_QE_IC_H */
--
2.23.0
[PATCH v5 18/48] soc: fsl: qe: don't use NO_IRQ in qe_ic.c
This driver is currently PPC-only, and on powerpc, NO_IRQ is 0, so
this doesn't change functionality. However, not every architecture
defines NO_IRQ, and some define it as -1, so the detection of a failed
irq_of_parse_and_map() (which returns 0 on failure) would be wrong on
those. So to prepare for allowing this driver to build on other
architectures, drop all references to NO_IRQ.
Signed-off-by: Rasmus Villemoes
---
drivers/soc/fsl/qe/qe_ic.c | 21 ++---
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/drivers/soc/fsl/qe/qe_ic.c b/drivers/soc/fsl/qe/qe_ic.c
index 4839dcd5c5d3..8f74bc6efd5c 100644
--- a/drivers/soc/fsl/qe/qe_ic.c
+++ b/drivers/soc/fsl/qe/qe_ic.c
@@ -282,7 +282,7 @@ static const struct irq_domain_ops qe_ic_host_ops = {
.xlate = irq_domain_xlate_onetwocell,
};
-/* Return an interrupt vector or NO_IRQ if no interrupt is pending. */
+/* Return an interrupt vector or 0 if no interrupt is pending. */
unsigned int qe_ic_get_low_irq(struct qe_ic *qe_ic)
{
int irq;
@@ -293,12 +293,12 @@ unsigned int qe_ic_get_low_irq(struct qe_ic *qe_ic)
irq = qe_ic_read(qe_ic->regs, QEIC_CIVEC) >> 26;
if (irq == 0)
- return NO_IRQ;
+ return 0;
return irq_linear_revmap(qe_ic->irqhost, irq);
}
-/* Return an interrupt vector or NO_IRQ if no interrupt is pending. */
+/* Return an interrupt vector or 0 if no interrupt is pending. */
unsigned int qe_ic_get_high_irq(struct qe_ic *qe_ic)
{
int irq;
@@ -309,7 +309,7 @@ unsigned int qe_ic_get_high_irq(struct qe_ic *qe_ic)
irq = qe_ic_read(qe_ic->regs, QEIC_CHIVEC) >> 26;
if (irq == 0)
- return NO_IRQ;
+ return 0;
return irq_linear_revmap(qe_ic->irqhost, irq);
}
@@ -320,7 +320,7 @@ static void qe_ic_cascade_low(struct irq_desc *desc)
unsigned int cascade_irq = qe_ic_get_low_irq(qe_ic);
struct irq_chip *chip = irq_desc_get_chip(desc);
- if (cascade_irq != NO_IRQ)
+ if (cascade_irq != 0)
generic_handle_irq(cascade_irq);
if (chip->irq_eoi)
@@ -333,7 +333,7 @@ static void qe_ic_cascade_high(struct irq_desc *desc)
unsigned int cascade_irq = qe_ic_get_high_irq(qe_ic);
struct irq_chip *chip = irq_desc_get_chip(desc);
- if (cascade_irq != NO_IRQ)
+ if (cascade_irq != 0)
generic_handle_irq(cascade_irq);
if (chip->irq_eoi)
@@ -347,10 +347,10 @@ static void qe_ic_cascade_muxed_mpic(struct irq_desc
*desc)
struct irq_chip *chip = irq_desc_get_chip(desc);
cascade_irq = qe_ic_get_high_irq(qe_ic);
- if (cascade_irq == NO_IRQ)
+ if (cascade_irq == 0)
cascade_irq = qe_ic_get_low_irq(qe_ic);
- if (cascade_irq != NO_IRQ)
+ if (cascade_irq != 0)
generic_handle_irq(cascade_irq);
chip->irq_eoi(&desc->irq_data);
@@ -386,7 +386,7 @@ static void __init qe_ic_init(struct device_node *node,
unsigned int flags)
qe_ic->virq_high = irq_of_parse_and_map(node, 0);
qe_ic->virq_low = irq_of_parse_and_map(node, 1);
- if (qe_ic->virq_low == NO_IRQ) {
+ if (!qe_ic->virq_low) {
printk(KERN_ERR "Failed to map QE_IC low IRQ\n");
kfree(qe_ic);
return;
@@ -423,8 +423,7 @@ static void __init qe_ic_init(struct device_node *node,
unsigned int flags)
irq_set_handler_data(qe_ic->virq_low, qe_ic);
irq_set_chained_handler(qe_ic->virq_low, low_handler);
- if (qe_ic->virq_high != NO_IRQ &&
- qe_ic->virq_high != qe_ic->virq_low) {
+ if (qe_ic->virq_high && qe_ic->virq_high != qe_ic->virq_low) {
irq_set_handler_data(qe_ic->virq_high, qe_ic);
irq_set_chained_handler(qe_ic->virq_high, high_handler);
}
--
2.23.0
[PATCH v5 16/48] soc: fsl: qe: rename qe_ic_cascade_low_mpic -> qe_ic_cascade_low
The qe_ic_cascade_{low,high}_mpic functions are now used as handlers
both when the interrupt parent is mpic as well as ipic, so remove the
_mpic suffix.
Signed-off-by: Rasmus Villemoes
---
drivers/soc/fsl/qe/qe_ic.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/soc/fsl/qe/qe_ic.c b/drivers/soc/fsl/qe/qe_ic.c
index 94ccbeeb1ad1..de2ca2e3a648 100644
--- a/drivers/soc/fsl/qe/qe_ic.c
+++ b/drivers/soc/fsl/qe/qe_ic.c
@@ -314,7 +314,7 @@ unsigned int qe_ic_get_high_irq(struct qe_ic *qe_ic)
return irq_linear_revmap(qe_ic->irqhost, irq);
}
-static void qe_ic_cascade_low_mpic(struct irq_desc *desc)
+static void qe_ic_cascade_low(struct irq_desc *desc)
{
struct qe_ic *qe_ic = irq_desc_get_handler_data(desc);
unsigned int cascade_irq = qe_ic_get_low_irq(qe_ic);
@@ -327,7 +327,7 @@ static void qe_ic_cascade_low_mpic(struct irq_desc *desc)
chip->irq_eoi(&desc->irq_data);
}
-static void qe_ic_cascade_high_mpic(struct irq_desc *desc)
+static void qe_ic_cascade_high(struct irq_desc *desc)
{
struct qe_ic *qe_ic = irq_desc_get_handler_data(desc);
unsigned int cascade_irq = qe_ic_get_high_irq(qe_ic);
@@ -392,8 +392,8 @@ static void __init qe_ic_init(struct device_node *node,
unsigned int flags)
return;
}
if (qe_ic->virq_high != qe_ic->virq_low) {
- low_handler = qe_ic_cascade_low_mpic;
- high_handler = qe_ic_cascade_high_mpic;
+ low_handler = qe_ic_cascade_low;
+ high_handler = qe_ic_cascade_high;
} else {
low_handler = qe_ic_cascade_muxed_mpic;
high_handler = NULL;
--
2.23.0
[PATCH v5 15/48] soc: fsl: qe: move qe_ic_cascade_* functions to qe_ic.c
These functions are only ever called through a function pointer, and
therefore it makes no sense for them to be "static inline" - gcc has
no choice but to emit a copy in each translation unit that takes the
address of one of these. Since they are now only referenced from
qe_ic.c, just make them local to that file.
Signed-off-by: Rasmus Villemoes
---
drivers/soc/fsl/qe/qe_ic.c | 42 ++
include/soc/fsl/qe/qe_ic.h | 42 --
2 files changed, 42 insertions(+), 42 deletions(-)
diff --git a/drivers/soc/fsl/qe/qe_ic.c b/drivers/soc/fsl/qe/qe_ic.c
index a062efac398b..94ccbeeb1ad1 100644
--- a/drivers/soc/fsl/qe/qe_ic.c
+++ b/drivers/soc/fsl/qe/qe_ic.c
@@ -314,6 +314,48 @@ unsigned int qe_ic_get_high_irq(struct qe_ic *qe_ic)
return irq_linear_revmap(qe_ic->irqhost, irq);
}
+static void qe_ic_cascade_low_mpic(struct irq_desc *desc)
+{
+ struct qe_ic *qe_ic = irq_desc_get_handler_data(desc);
+ unsigned int cascade_irq = qe_ic_get_low_irq(qe_ic);
+ struct irq_chip *chip = irq_desc_get_chip(desc);
+
+ if (cascade_irq != NO_IRQ)
+ generic_handle_irq(cascade_irq);
+
+ if (chip->irq_eoi)
+ chip->irq_eoi(&desc->irq_data);
+}
+
+static void qe_ic_cascade_high_mpic(struct irq_desc *desc)
+{
+ struct qe_ic *qe_ic = irq_desc_get_handler_data(desc);
+ unsigned int cascade_irq = qe_ic_get_high_irq(qe_ic);
+ struct irq_chip *chip = irq_desc_get_chip(desc);
+
+ if (cascade_irq != NO_IRQ)
+ generic_handle_irq(cascade_irq);
+
+ if (chip->irq_eoi)
+ chip->irq_eoi(&desc->irq_data);
+}
+
+static void qe_ic_cascade_muxed_mpic(struct irq_desc *desc)
+{
+ struct qe_ic *qe_ic = irq_desc_get_handler_data(desc);
+ unsigned int cascade_irq;
+ struct irq_chip *chip = irq_desc_get_chip(desc);
+
+ cascade_irq = qe_ic_get_high_irq(qe_ic);
+ if (cascade_irq == NO_IRQ)
+ cascade_irq = qe_ic_get_low_irq(qe_ic);
+
+ if (cascade_irq != NO_IRQ)
+ generic_handle_irq(cascade_irq);
+
+ chip->irq_eoi(&desc->irq_data);
+}
+
static void __init qe_ic_init(struct device_node *node, unsigned int flags)
{
void (*low_handler)(struct irq_desc *desc);
diff --git a/include/soc/fsl/qe/qe_ic.h b/include/soc/fsl/qe/qe_ic.h
index a47a0d26acbd..43e4ce95c6a0 100644
--- a/include/soc/fsl/qe/qe_ic.h
+++ b/include/soc/fsl/qe/qe_ic.h
@@ -67,46 +67,4 @@ void qe_ic_set_highest_priority(unsigned int virq, int high);
int qe_ic_set_priority(unsigned int virq, unsigned int priority);
int qe_ic_set_high_priority(unsigned int virq, unsigned int priority, int
high);
-static inline void qe_ic_cascade_low_mpic(struct irq_desc *desc)
-{
- struct qe_ic *qe_ic = irq_desc_get_handler_data(desc);
- unsigned int cascade_irq = qe_ic_get_low_irq(qe_ic);
- struct irq_chip *chip = irq_desc_get_chip(desc);
-
- if (cascade_irq != NO_IRQ)
- generic_handle_irq(cascade_irq);
-
- if (chip->irq_eoi)
- chip->irq_eoi(&desc->irq_data);
-}
-
-static inline void qe_ic_cascade_high_mpic(struct irq_desc *desc)
-{
- struct qe_ic *qe_ic = irq_desc_get_handler_data(desc);
- unsigned int cascade_irq = qe_ic_get_high_irq(qe_ic);
- struct irq_chip *chip = irq_desc_get_chip(desc);
-
- if (cascade_irq != NO_IRQ)
- generic_handle_irq(cascade_irq);
-
- if (chip->irq_eoi)
- chip->irq_eoi(&desc->irq_data);
-}
-
-static inline void qe_ic_cascade_muxed_mpic(struct irq_desc *desc)
-{
- struct qe_ic *qe_ic = irq_desc_get_handler_data(desc);
- unsigned int cascade_irq;
- struct irq_chip *chip = irq_desc_get_chip(desc);
-
- cascade_irq = qe_ic_get_high_irq(qe_ic);
- if (cascade_irq == NO_IRQ)
- cascade_irq = qe_ic_get_low_irq(qe_ic);
-
- if (cascade_irq != NO_IRQ)
- generic_handle_irq(cascade_irq);
-
- chip->irq_eoi(&desc->irq_data);
-}
-
#endif /* _ASM_POWERPC_QE_IC_H */
--
2.23.0
[PATCH v5 14/48] powerpc/85xx: remove mostly pointless mpc85xx_qe_init()
Since commit 302c059f2e7b (QE: use subsys_initcall to init qe),
mpc85xx_qe_init() has done nothing apart from possibly emitting a
pr_err(). As part of reducing the amount of QE-related code in
arch/powerpc/ (and eventually support QE on other architectures),
remove this low-hanging fruit.
Signed-off-by: Rasmus Villemoes
---
arch/powerpc/platforms/85xx/common.c | 23 ---
arch/powerpc/platforms/85xx/corenet_generic.c | 2 --
arch/powerpc/platforms/85xx/mpc85xx.h | 2 --
arch/powerpc/platforms/85xx/mpc85xx_mds.c | 1 -
arch/powerpc/platforms/85xx/mpc85xx_rdb.c | 1 -
arch/powerpc/platforms/85xx/twr_p102x.c | 1 -
6 files changed, 30 deletions(-)
diff --git a/arch/powerpc/platforms/85xx/common.c
b/arch/powerpc/platforms/85xx/common.c
index fe0606439b5a..a554b6d87cf7 100644
--- a/arch/powerpc/platforms/85xx/common.c
+++ b/arch/powerpc/platforms/85xx/common.c
@@ -86,29 +86,6 @@ void __init mpc85xx_cpm2_pic_init(void)
#endif
#ifdef CONFIG_QUICC_ENGINE
-void __init mpc85xx_qe_init(void)
-{
- struct device_node *np;
-
- np = of_find_compatible_node(NULL, NULL, "fsl,qe");
- if (!np) {
- np = of_find_node_by_name(NULL, "qe");
- if (!np) {
- pr_err("%s: Could not find Quicc Engine node\n",
- __func__);
- return;
- }
- }
-
- if (!of_device_is_available(np)) {
- of_node_put(np);
- return;
- }
-
- of_node_put(np);
-
-}
-
void __init mpc85xx_qe_par_io_init(void)
{
struct device_node *np;
diff --git a/arch/powerpc/platforms/85xx/corenet_generic.c
b/arch/powerpc/platforms/85xx/corenet_generic.c
index 8c1bb3941642..27ac38f7e1a9 100644
--- a/arch/powerpc/platforms/85xx/corenet_generic.c
+++ b/arch/powerpc/platforms/85xx/corenet_generic.c
@@ -56,8 +56,6 @@ void __init corenet_gen_setup_arch(void)
swiotlb_detect_4g();
pr_info("%s board\n", ppc_md.name);
-
- mpc85xx_qe_init();
}
static const struct of_device_id of_device_ids[] = {
diff --git a/arch/powerpc/platforms/85xx/mpc85xx.h
b/arch/powerpc/platforms/85xx/mpc85xx.h
index fa23f9b0592c..cb84c5c56c36 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx.h
+++ b/arch/powerpc/platforms/85xx/mpc85xx.h
@@ -10,10 +10,8 @@ static inline void __init mpc85xx_cpm2_pic_init(void) {}
#endif /* CONFIG_CPM2 */
#ifdef CONFIG_QUICC_ENGINE
-extern void mpc85xx_qe_init(void);
extern void mpc85xx_qe_par_io_init(void);
#else
-static inline void __init mpc85xx_qe_init(void) {}
static inline void __init mpc85xx_qe_par_io_init(void) {}
#endif
diff --git a/arch/powerpc/platforms/85xx/mpc85xx_mds.c
b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
index 4bc49e5ec0b6..fb05b4d5bf1e 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_mds.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
@@ -237,7 +237,6 @@ static void __init mpc85xx_mds_qe_init(void)
{
struct device_node *np;
- mpc85xx_qe_init();
mpc85xx_qe_par_io_init();
mpc85xx_mds_reset_ucc_phys();
diff --git a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
index 14b5a61d49c1..80a80174768c 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
@@ -72,7 +72,6 @@ static void __init mpc85xx_rdb_setup_arch(void)
fsl_pci_assign_primary();
#ifdef CONFIG_QUICC_ENGINE
- mpc85xx_qe_init();
mpc85xx_qe_par_io_init();
#if defined(CONFIG_UCC_GETH) || defined(CONFIG_SERIAL_QE)
if (machine_is(p1025_rdb)) {
diff --git a/arch/powerpc/platforms/85xx/twr_p102x.c
b/arch/powerpc/platforms/85xx/twr_p102x.c
index b099f5607120..9abb1e9f73c4 100644
--- a/arch/powerpc/platforms/85xx/twr_p102x.c
+++ b/arch/powerpc/platforms/85xx/twr_p102x.c
@@ -57,7 +57,6 @@ static void __init twr_p1025_setup_arch(void)
fsl_pci_assign_primary();
#ifdef CONFIG_QUICC_ENGINE
- mpc85xx_qe_init();
mpc85xx_qe_par_io_init();
#if IS_ENABLED(CONFIG_UCC_GETH) || IS_ENABLED(CONFIG_SERIAL_QE)
--
2.23.0
[PATCH v5 13/48] powerpc/83xx: remove mpc83xx_ipic_and_qe_init_IRQ
This is now exactly the same as mpc83xx_ipic_init_IRQ, so just use
that directly.
Signed-off-by: Rasmus Villemoes
---
arch/powerpc/platforms/83xx/km83xx.c | 2 +-
arch/powerpc/platforms/83xx/misc.c| 7 ---
arch/powerpc/platforms/83xx/mpc832x_mds.c | 2 +-
arch/powerpc/platforms/83xx/mpc832x_rdb.c | 2 +-
arch/powerpc/platforms/83xx/mpc836x_mds.c | 2 +-
arch/powerpc/platforms/83xx/mpc836x_rdk.c | 2 +-
arch/powerpc/platforms/83xx/mpc83xx.h | 5 -
7 files changed, 5 insertions(+), 17 deletions(-)
diff --git a/arch/powerpc/platforms/83xx/km83xx.c
b/arch/powerpc/platforms/83xx/km83xx.c
index 5c6227f7bc37..3d89569e9e71 100644
--- a/arch/powerpc/platforms/83xx/km83xx.c
+++ b/arch/powerpc/platforms/83xx/km83xx.c
@@ -177,7 +177,7 @@ define_machine(mpc83xx_km) {
.name = "mpc83xx-km-platform",
.probe = mpc83xx_km_probe,
.setup_arch = mpc83xx_km_setup_arch,
- .init_IRQ = mpc83xx_ipic_and_qe_init_IRQ,
+ .init_IRQ = mpc83xx_ipic_init_IRQ,
.get_irq= ipic_get_irq,
.restart= mpc83xx_restart,
.time_init = mpc83xx_time_init,
diff --git a/arch/powerpc/platforms/83xx/misc.c
b/arch/powerpc/platforms/83xx/misc.c
index 6935a5b9fbd1..1d8306eb2958 100644
--- a/arch/powerpc/platforms/83xx/misc.c
+++ b/arch/powerpc/platforms/83xx/misc.c
@@ -88,13 +88,6 @@ void __init mpc83xx_ipic_init_IRQ(void)
ipic_set_default_priority();
}
-#ifdef CONFIG_QUICC_ENGINE
-void __init mpc83xx_ipic_and_qe_init_IRQ(void)
-{
- mpc83xx_ipic_init_IRQ();
-}
-#endif /* CONFIG_QUICC_ENGINE */
-
static const struct of_device_id of_bus_ids[] __initconst = {
{ .type = "soc", },
{ .compatible = "soc", },
diff --git a/arch/powerpc/platforms/83xx/mpc832x_mds.c
b/arch/powerpc/platforms/83xx/mpc832x_mds.c
index 1c73af104d19..6fa5402ebf20 100644
--- a/arch/powerpc/platforms/83xx/mpc832x_mds.c
+++ b/arch/powerpc/platforms/83xx/mpc832x_mds.c
@@ -101,7 +101,7 @@ define_machine(mpc832x_mds) {
.name = "MPC832x MDS",
.probe = mpc832x_sys_probe,
.setup_arch = mpc832x_sys_setup_arch,
- .init_IRQ = mpc83xx_ipic_and_qe_init_IRQ,
+ .init_IRQ = mpc83xx_ipic_init_IRQ,
.get_irq= ipic_get_irq,
.restart= mpc83xx_restart,
.time_init = mpc83xx_time_init,
diff --git a/arch/powerpc/platforms/83xx/mpc832x_rdb.c
b/arch/powerpc/platforms/83xx/mpc832x_rdb.c
index 87f68ca06255..622c625d5ce4 100644
--- a/arch/powerpc/platforms/83xx/mpc832x_rdb.c
+++ b/arch/powerpc/platforms/83xx/mpc832x_rdb.c
@@ -219,7 +219,7 @@ define_machine(mpc832x_rdb) {
.name = "MPC832x RDB",
.probe = mpc832x_rdb_probe,
.setup_arch = mpc832x_rdb_setup_arch,
- .init_IRQ = mpc83xx_ipic_and_qe_init_IRQ,
+ .init_IRQ = mpc83xx_ipic_init_IRQ,
.get_irq= ipic_get_irq,
.restart= mpc83xx_restart,
.time_init = mpc83xx_time_init,
diff --git a/arch/powerpc/platforms/83xx/mpc836x_mds.c
b/arch/powerpc/platforms/83xx/mpc836x_mds.c
index 5b484da9533e..219a83ab6c00 100644
--- a/arch/powerpc/platforms/83xx/mpc836x_mds.c
+++ b/arch/powerpc/platforms/83xx/mpc836x_mds.c
@@ -208,7 +208,7 @@ define_machine(mpc836x_mds) {
.name = "MPC836x MDS",
.probe = mpc836x_mds_probe,
.setup_arch = mpc836x_mds_setup_arch,
- .init_IRQ = mpc83xx_ipic_and_qe_init_IRQ,
+ .init_IRQ = mpc83xx_ipic_init_IRQ,
.get_irq= ipic_get_irq,
.restart= mpc83xx_restart,
.time_init = mpc83xx_time_init,
diff --git a/arch/powerpc/platforms/83xx/mpc836x_rdk.c
b/arch/powerpc/platforms/83xx/mpc836x_rdk.c
index b7119e443920..b4aac2cde849 100644
--- a/arch/powerpc/platforms/83xx/mpc836x_rdk.c
+++ b/arch/powerpc/platforms/83xx/mpc836x_rdk.c
@@ -41,7 +41,7 @@ define_machine(mpc836x_rdk) {
.name = "MPC836x RDK",
.probe = mpc836x_rdk_probe,
.setup_arch = mpc836x_rdk_setup_arch,
- .init_IRQ = mpc83xx_ipic_and_qe_init_IRQ,
+ .init_IRQ = mpc83xx_ipic_init_IRQ,
.get_irq= ipic_get_irq,
.restart= mpc83xx_restart,
.time_init = mpc83xx_time_init,
diff --git a/arch/powerpc/platforms/83xx/mpc83xx.h
b/arch/powerpc/platforms/83xx/mpc83xx.h
index d343f6ce2599..f37d04332fc7 100644
--- a/arch/powerpc/platforms/83xx/mpc83xx.h
+++ b/arch/powerpc/platforms/83xx/mpc83xx.h
@@ -72,11 +72,6 @@ extern int mpc837x_usb_cfg(void);
extern int mpc834x_usb_cfg(void);
extern int mpc831x_usb_cfg(void);
extern void mpc83xx_ipic_init_IRQ(void);
-#ifdef CONFIG_QUICC_ENGINE
-extern void mpc83xx_ipic_and_qe_init_IRQ(void);
-#else
-#define mpc83xx_ipic_and_qe_init_IRQ mpc83xx_ipic_init_IRQ
-#endif /* CONFIG_QUICC_ENGINE */
#ifdef CONFIG_PCI
extern void mpc83xx_setup_pci(void);
--
[PATCH v5 12/48] soc: fsl: qe: move calls of qe_ic_init out of arch/powerpc/
Having to call qe_ic_init() from platform-specific code makes it
awkward to allow building the QE drivers for ARM. It's also a needless
duplication of code, and slightly error-prone: Instead of the caller
needing to know the details of whether the QUICC Engine High and QUICC
Engine Low are actually the same interrupt (see e.g. the machine_is()
in mpc85xx_mds_qeic_init), just let the init function choose the
appropriate handlers after it has parsed the DT and figured it out. If
the two interrupts are distinct, use separate handlers, otherwise use
the handler which first checks the CHIVEC register (for the high
priority interrupts), then the CIVEC.
All existing callers pass 0 for flags, so continue to do that from the
new single caller. Later cleanups will remove that argument
from qe_ic_init and simplify the body, as well as make qe_ic_init into
a proper init function for an IRQCHIP_DECLARE, eliminating the need to
manually look up the fsl,qe-ic node.
Signed-off-by: Rasmus Villemoes
---
arch/powerpc/platforms/83xx/km83xx.c | 1 -
arch/powerpc/platforms/83xx/misc.c| 16 --
arch/powerpc/platforms/83xx/mpc832x_mds.c | 1 -
arch/powerpc/platforms/83xx/mpc832x_rdb.c | 1 -
arch/powerpc/platforms/83xx/mpc836x_mds.c | 1 -
arch/powerpc/platforms/83xx/mpc836x_rdk.c | 1 -
arch/powerpc/platforms/83xx/mpc83xx.h | 2 --
arch/powerpc/platforms/85xx/corenet_generic.c | 10 ---
arch/powerpc/platforms/85xx/mpc85xx_mds.c | 27 -
arch/powerpc/platforms/85xx/mpc85xx_rdb.c | 17 ---
arch/powerpc/platforms/85xx/twr_p102x.c | 15 --
drivers/soc/fsl/qe/qe_ic.c| 29 +--
include/soc/fsl/qe/qe_ic.h| 7 -
13 files changed, 26 insertions(+), 102 deletions(-)
diff --git a/arch/powerpc/platforms/83xx/km83xx.c
b/arch/powerpc/platforms/83xx/km83xx.c
index 273145aed90a..5c6227f7bc37 100644
--- a/arch/powerpc/platforms/83xx/km83xx.c
+++ b/arch/powerpc/platforms/83xx/km83xx.c
@@ -34,7 +34,6 @@
#include
#include
#include
-#include
#include "mpc83xx.h"
diff --git a/arch/powerpc/platforms/83xx/misc.c
b/arch/powerpc/platforms/83xx/misc.c
index 779791c0570f..6935a5b9fbd1 100644
--- a/arch/powerpc/platforms/83xx/misc.c
+++ b/arch/powerpc/platforms/83xx/misc.c
@@ -14,7 +14,6 @@
#include
#include
#include
-#include
#include
#include
@@ -90,24 +89,9 @@ void __init mpc83xx_ipic_init_IRQ(void)
}
#ifdef CONFIG_QUICC_ENGINE
-void __init mpc83xx_qe_init_IRQ(void)
-{
- struct device_node *np;
-
- np = of_find_compatible_node(NULL, NULL, "fsl,qe-ic");
- if (!np) {
- np = of_find_node_by_type(NULL, "qeic");
- if (!np)
- return;
- }
- qe_ic_init(np, 0, qe_ic_cascade_low_mpic, qe_ic_cascade_high_mpic);
- of_node_put(np);
-}
-
void __init mpc83xx_ipic_and_qe_init_IRQ(void)
{
mpc83xx_ipic_init_IRQ();
- mpc83xx_qe_init_IRQ();
}
#endif /* CONFIG_QUICC_ENGINE */
diff --git a/arch/powerpc/platforms/83xx/mpc832x_mds.c
b/arch/powerpc/platforms/83xx/mpc832x_mds.c
index b428835e5919..1c73af104d19 100644
--- a/arch/powerpc/platforms/83xx/mpc832x_mds.c
+++ b/arch/powerpc/platforms/83xx/mpc832x_mds.c
@@ -33,7 +33,6 @@
#include
#include
#include
-#include
#include "mpc83xx.h"
diff --git a/arch/powerpc/platforms/83xx/mpc832x_rdb.c
b/arch/powerpc/platforms/83xx/mpc832x_rdb.c
index 4588ce632484..87f68ca06255 100644
--- a/arch/powerpc/platforms/83xx/mpc832x_rdb.c
+++ b/arch/powerpc/platforms/83xx/mpc832x_rdb.c
@@ -22,7 +22,6 @@
#include
#include
#include
-#include
#include
#include
diff --git a/arch/powerpc/platforms/83xx/mpc836x_mds.c
b/arch/powerpc/platforms/83xx/mpc836x_mds.c
index 4a4efa906d35..5b484da9533e 100644
--- a/arch/powerpc/platforms/83xx/mpc836x_mds.c
+++ b/arch/powerpc/platforms/83xx/mpc836x_mds.c
@@ -41,7 +41,6 @@
#include
#include
#include
-#include
#include "mpc83xx.h"
diff --git a/arch/powerpc/platforms/83xx/mpc836x_rdk.c
b/arch/powerpc/platforms/83xx/mpc836x_rdk.c
index 9923059cb111..b7119e443920 100644
--- a/arch/powerpc/platforms/83xx/mpc836x_rdk.c
+++ b/arch/powerpc/platforms/83xx/mpc836x_rdk.c
@@ -17,7 +17,6 @@
#include
#include
#include
-#include
#include
#include
diff --git a/arch/powerpc/platforms/83xx/mpc83xx.h
b/arch/powerpc/platforms/83xx/mpc83xx.h
index 459145623334..d343f6ce2599 100644
--- a/arch/powerpc/platforms/83xx/mpc83xx.h
+++ b/arch/powerpc/platforms/83xx/mpc83xx.h
@@ -73,10 +73,8 @@ extern int mpc834x_usb_cfg(void);
extern int mpc831x_usb_cfg(void);
extern void mpc83xx_ipic_init_IRQ(void);
#ifdef CONFIG_QUICC_ENGINE
-extern void mpc83xx_qe_init_IRQ(void);
extern void mpc83xx_ipic_and_qe_init_IRQ(void);
#else
-static inline void __init mpc83xx_qe_init_IRQ(void) {}
#define mpc83xx_ipic_and_qe_init_IRQ mpc83xx_ipic_init_IRQ
#endif /* CONFIG_QUICC_ENGINE */
diff --git a/ar
[PATCH v5 11/48] soc: fsl: qe: use qe_ic_cascade_{low, high}_mpic also on 83xx
The *_ipic and *_mpic handlers are almost identical - the only
difference is that the latter end with an unconditional
chip->irq_eoi() call. Since IPIC does not have ->irq_eoi, we can
reduce some code duplication by calling irq_eoi conditionally.
This is similar to what is already done in mpc8xxx_gpio_irq_cascade().
This leaves the functions slightly misnamed, but that will be fixed in
a subsequent patch.
Signed-off-by: Rasmus Villemoes
---
arch/powerpc/platforms/83xx/misc.c | 2 +-
include/soc/fsl/qe/qe_ic.h | 24
2 files changed, 5 insertions(+), 21 deletions(-)
diff --git a/arch/powerpc/platforms/83xx/misc.c
b/arch/powerpc/platforms/83xx/misc.c
index f46d7bf3b140..779791c0570f 100644
--- a/arch/powerpc/platforms/83xx/misc.c
+++ b/arch/powerpc/platforms/83xx/misc.c
@@ -100,7 +100,7 @@ void __init mpc83xx_qe_init_IRQ(void)
if (!np)
return;
}
- qe_ic_init(np, 0, qe_ic_cascade_low_ipic, qe_ic_cascade_high_ipic);
+ qe_ic_init(np, 0, qe_ic_cascade_low_mpic, qe_ic_cascade_high_mpic);
of_node_put(np);
}
diff --git a/include/soc/fsl/qe/qe_ic.h b/include/soc/fsl/qe/qe_ic.h
index 714a9b890d8d..bfaa233d8328 100644
--- a/include/soc/fsl/qe/qe_ic.h
+++ b/include/soc/fsl/qe/qe_ic.h
@@ -74,24 +74,6 @@ void qe_ic_set_highest_priority(unsigned int virq, int high);
int qe_ic_set_priority(unsigned int virq, unsigned int priority);
int qe_ic_set_high_priority(unsigned int virq, unsigned int priority, int
high);
-static inline void qe_ic_cascade_low_ipic(struct irq_desc *desc)
-{
- struct qe_ic *qe_ic = irq_desc_get_handler_data(desc);
- unsigned int cascade_irq = qe_ic_get_low_irq(qe_ic);
-
- if (cascade_irq != NO_IRQ)
- generic_handle_irq(cascade_irq);
-}
-
-static inline void qe_ic_cascade_high_ipic(struct irq_desc *desc)
-{
- struct qe_ic *qe_ic = irq_desc_get_handler_data(desc);
- unsigned int cascade_irq = qe_ic_get_high_irq(qe_ic);
-
- if (cascade_irq != NO_IRQ)
- generic_handle_irq(cascade_irq);
-}
-
static inline void qe_ic_cascade_low_mpic(struct irq_desc *desc)
{
struct qe_ic *qe_ic = irq_desc_get_handler_data(desc);
@@ -101,7 +83,8 @@ static inline void qe_ic_cascade_low_mpic(struct irq_desc
*desc)
if (cascade_irq != NO_IRQ)
generic_handle_irq(cascade_irq);
- chip->irq_eoi(&desc->irq_data);
+ if (chip->irq_eoi)
+ chip->irq_eoi(&desc->irq_data);
}
static inline void qe_ic_cascade_high_mpic(struct irq_desc *desc)
@@ -113,7 +96,8 @@ static inline void qe_ic_cascade_high_mpic(struct irq_desc
*desc)
if (cascade_irq != NO_IRQ)
generic_handle_irq(cascade_irq);
- chip->irq_eoi(&desc->irq_data);
+ if (chip->irq_eoi)
+ chip->irq_eoi(&desc->irq_data);
}
static inline void qe_ic_cascade_muxed_mpic(struct irq_desc *desc)
--
2.23.0
[PATCH v5 10/48] soc: fsl: qe: remove pointless sysfs registration in qe_ic.c
There's no point in registering with sysfs when that doesn't actually
allow any interaction with the device or driver (no uevents, no sysfs
files that provide information or allow configuration, no nothing).
Signed-off-by: Rasmus Villemoes
---
drivers/soc/fsl/qe/qe_ic.c | 31 ---
1 file changed, 31 deletions(-)
diff --git a/drivers/soc/fsl/qe/qe_ic.c b/drivers/soc/fsl/qe/qe_ic.c
index 4b03060d8079..f170926ce4d1 100644
--- a/drivers/soc/fsl/qe/qe_ic.c
+++ b/drivers/soc/fsl/qe/qe_ic.c
@@ -474,34 +474,3 @@ int qe_ic_set_high_priority(unsigned int virq, unsigned
int priority, int high)
return 0;
}
-
-static struct bus_type qe_ic_subsys = {
- .name = "qe_ic",
- .dev_name = "qe_ic",
-};
-
-static struct device device_qe_ic = {
- .id = 0,
- .bus = &qe_ic_subsys,
-};
-
-static int __init init_qe_ic_sysfs(void)
-{
- int rc;
-
- printk(KERN_DEBUG "Registering qe_ic with sysfs...\n");
-
- rc = subsys_system_register(&qe_ic_subsys, NULL);
- if (rc) {
- printk(KERN_ERR "Failed registering qe_ic sys class\n");
- return -ENODEV;
- }
- rc = device_register(&device_qe_ic);
- if (rc) {
- printk(KERN_ERR "Failed registering qe_ic sys device\n");
- return -ENODEV;
- }
- return 0;
-}
-
-subsys_initcall(init_qe_ic_sysfs);
--
2.23.0
[PATCH v5 09/48] soc: fsl: qe: drop assign-only high_active in qe_ic_init
high_active is only assigned to but never used. Remove it.
Signed-off-by: Rasmus Villemoes
---
drivers/soc/fsl/qe/qe_ic.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/soc/fsl/qe/qe_ic.c b/drivers/soc/fsl/qe/qe_ic.c
index 8c874372416b..4b03060d8079 100644
--- a/drivers/soc/fsl/qe/qe_ic.c
+++ b/drivers/soc/fsl/qe/qe_ic.c
@@ -320,7 +320,7 @@ void __init qe_ic_init(struct device_node *node, unsigned
int flags,
{
struct qe_ic *qe_ic;
struct resource res;
- u32 temp = 0, ret, high_active = 0;
+ u32 temp = 0, ret;
ret = of_address_to_resource(node, 0, &res);
if (ret)
@@ -366,10 +366,8 @@ void __init qe_ic_init(struct device_node *node, unsigned
int flags,
temp |= CICR_GRTB;
/* choose destination signal for highest priority interrupt */
- if (flags & QE_IC_HIGH_SIGNAL) {
+ if (flags & QE_IC_HIGH_SIGNAL)
temp |= (SIGNAL_HIGH << CICR_HPIT_SHIFT);
- high_active = 1;
- }
qe_ic_write(qe_ic->regs, QEIC_CICR, temp);
--
2.23.0
[PATCH v5 08/48] soc: fsl: qe: drop unneeded #includes
These includes are not actually needed, and asm/rheap.h and sysdev/fsl_soc.h are PPC-specific, hence prevent compiling QE for other architectures. Signed-off-by: Rasmus Villemoes --- drivers/soc/fsl/qe/qe.c| 5 - drivers/soc/fsl/qe/qe_io.c | 2 -- 2 files changed, 7 deletions(-) diff --git a/drivers/soc/fsl/qe/qe.c b/drivers/soc/fsl/qe/qe.c index 1d8aa62c7ddf..a4763282ea68 100644 --- a/drivers/soc/fsl/qe/qe.c +++ b/drivers/soc/fsl/qe/qe.c @@ -26,13 +26,8 @@ #include #include #include -#include -#include -#include #include #include -#include -#include static void qe_snums_init(void); static int qe_sdma_init(void); diff --git a/drivers/soc/fsl/qe/qe_io.c b/drivers/soc/fsl/qe/qe_io.c index 5e3471ac09dd..f6b10f38b2f4 100644 --- a/drivers/soc/fsl/qe/qe_io.c +++ b/drivers/soc/fsl/qe/qe_io.c @@ -18,8 +18,6 @@ #include #include -#include -#include #undef DEBUG -- 2.23.0
[PATCH v5 07/48] soc: fsl: qe: qe.c: guard use of pvr_version_is() with CONFIG_PPC32
Commit e5c5c8d23fef (soc/fsl/qe: only apply QE_General4 workaround on
affected SoCs) introduced use of pvr_version_is(), saying
The QE_General4 workaround is only valid for the MPC832x and MPC836x
SoCs. The other SoCs that embed a QUICC engine are not affected by this
hardware bug and thus can use the computed divisors (this was
successfully tested on the T1040).
I'm reading the above as saying that the errata does not apply to the
ARM-based SOCs with QUICC engine. In any case, use of pvr_version_is()
must be guarded by CONFIG_PPC32 before we can remove the PPC32
dependency from CONFIG_QUICC_ENGINE, so introduce qe_general4_errata()
to keep the necessary #ifdeffery localized to a trivial helper.
Signed-off-by: Rasmus Villemoes
---
drivers/soc/fsl/qe/qe.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/soc/fsl/qe/qe.c b/drivers/soc/fsl/qe/qe.c
index 85737e6f5b62..1d8aa62c7ddf 100644
--- a/drivers/soc/fsl/qe/qe.c
+++ b/drivers/soc/fsl/qe/qe.c
@@ -197,6 +197,14 @@ EXPORT_SYMBOL(qe_get_brg_clk);
#define PVR_VER_836x 0x8083
#define PVR_VER_832x 0x8084
+static bool qe_general4_errata(void)
+{
+#ifdef CONFIG_PPC32
+ return pvr_version_is(PVR_VER_836x) || pvr_version_is(PVR_VER_832x);
+#endif
+ return false;
+}
+
/* Program the BRG to the given sampling rate and multiplier
*
* @brg: the BRG, QE_BRG1 - QE_BRG16
@@ -223,7 +231,7 @@ int qe_setbrg(enum qe_clock brg, unsigned int rate,
unsigned int multiplier)
/* Errata QE_General4, which affects some MPC832x and MPC836x SOCs, says
that the BRG divisor must be even if you're not using divide-by-16
mode. */
- if (pvr_version_is(PVR_VER_836x) || pvr_version_is(PVR_VER_832x))
+ if (qe_general4_errata())
if (!div16 && (divisor & 1) && (divisor > 3))
divisor++;
--
2.23.0
[PATCH v5 06/48] soc: fsl: qe: replace spin_event_timeout by readx_poll_timeout_atomic
In preparation for allowing QE to be built for architectures other
than ppc, use the generic readx_poll_timeout_atomic() helper from
iopoll.h rather than the ppc-only spin_event_timeout().
Signed-off-by: Rasmus Villemoes
---
drivers/soc/fsl/qe/qe.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/soc/fsl/qe/qe.c b/drivers/soc/fsl/qe/qe.c
index 456bd7416876..85737e6f5b62 100644
--- a/drivers/soc/fsl/qe/qe.c
+++ b/drivers/soc/fsl/qe/qe.c
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -108,7 +109,8 @@ int qe_issue_cmd(u32 cmd, u32 device, u8 mcn_protocol, u32
cmd_input)
{
unsigned long flags;
u8 mcn_shift = 0, dev_shift = 0;
- u32 ret;
+ u32 val;
+ int ret;
spin_lock_irqsave(&qe_lock, flags);
if (cmd == QE_RESET) {
@@ -135,13 +137,12 @@ int qe_issue_cmd(u32 cmd, u32 device, u8 mcn_protocol,
u32 cmd_input)
}
/* wait for the QE_CR_FLG to clear */
- ret = spin_event_timeout((qe_ioread32be(&qe_immr->cp.cecr) & QE_CR_FLG)
== 0,
-100, 0);
- /* On timeout (e.g. failure), the expression will be false (ret == 0),
- otherwise it will be true (ret == 1). */
+ ret = readx_poll_timeout_atomic(qe_ioread32be, &qe_immr->cp.cecr, val,
+ (val & QE_CR_FLG) == 0, 0, 100);
+ /* On timeout, ret is -ETIMEDOUT, otherwise it will be 0. */
spin_unlock_irqrestore(&qe_lock, flags);
- return ret == 1;
+ return ret == 0;
}
EXPORT_SYMBOL(qe_issue_cmd);
--
2.23.0
[PATCH v5 02/48] soc: fsl: qe: drop volatile qualifier of struct qe_ic::regs
The actual io accessors (e.g. in_be32) implicitly add a volatile
qualifier to their address argument. Remove volatile from the struct
definition and the qe_ic_(read/write) helpers, in preparation for
switching from the ppc-specific io accessors to generic ones.
Signed-off-by: Rasmus Villemoes
---
drivers/soc/fsl/qe/qe_ic.c | 4 ++--
drivers/soc/fsl/qe/qe_ic.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/soc/fsl/qe/qe_ic.c b/drivers/soc/fsl/qe/qe_ic.c
index 9bac546998d3..791adcd121d1 100644
--- a/drivers/soc/fsl/qe/qe_ic.c
+++ b/drivers/soc/fsl/qe/qe_ic.c
@@ -171,12 +171,12 @@ static struct qe_ic_info qe_ic_info[] = {
},
};
-static inline u32 qe_ic_read(volatile __be32 __iomem * base, unsigned int reg)
+static inline u32 qe_ic_read(__be32 __iomem *base, unsigned int reg)
{
return in_be32(base + (reg >> 2));
}
-static inline void qe_ic_write(volatile __be32 __iomem * base, unsigned int
reg,
+static inline void qe_ic_write(__be32 __iomem *base, unsigned int reg,
u32 value)
{
out_be32(base + (reg >> 2), value);
diff --git a/drivers/soc/fsl/qe/qe_ic.h b/drivers/soc/fsl/qe/qe_ic.h
index 08c695672a03..9420378d9b6b 100644
--- a/drivers/soc/fsl/qe/qe_ic.h
+++ b/drivers/soc/fsl/qe/qe_ic.h
@@ -72,7 +72,7 @@
struct qe_ic {
/* Control registers offset */
- volatile u32 __iomem *regs;
+ u32 __iomem *regs;
/* The remapper for this QEIC */
struct irq_domain *irqhost;
--
2.23.0
[PATCH v5 03/48] soc: fsl: qe: rename qe_(clr/set/clrset)bit* helpers
Make it clear that these operate on big-endian registers (i.e. use the
iowrite*be primitives) before we introduce more uses of them and allow
the QE drivers to be built for platforms other than ppc32.
Signed-off-by: Rasmus Villemoes
---
drivers/net/wan/fsl_ucc_hdlc.c | 4 ++--
drivers/soc/fsl/qe/ucc.c | 10 +-
include/soc/fsl/qe/qe.h| 18 +-
3 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/net/wan/fsl_ucc_hdlc.c b/drivers/net/wan/fsl_ucc_hdlc.c
index ca0f3be2b6bf..ce6af7d5380f 100644
--- a/drivers/net/wan/fsl_ucc_hdlc.c
+++ b/drivers/net/wan/fsl_ucc_hdlc.c
@@ -623,8 +623,8 @@ static int ucc_hdlc_poll(struct napi_struct *napi, int
budget)
if (howmany < budget) {
napi_complete_done(napi, howmany);
- qe_setbits32(priv->uccf->p_uccm,
-(UCCE_HDLC_RX_EVENTS | UCCE_HDLC_TX_EVENTS) << 16);
+ qe_setbits_be32(priv->uccf->p_uccm,
+ (UCCE_HDLC_RX_EVENTS | UCCE_HDLC_TX_EVENTS) <<
16);
}
return howmany;
diff --git a/drivers/soc/fsl/qe/ucc.c b/drivers/soc/fsl/qe/ucc.c
index 024d239ac1e1..ae9f2cf560cb 100644
--- a/drivers/soc/fsl/qe/ucc.c
+++ b/drivers/soc/fsl/qe/ucc.c
@@ -540,8 +540,8 @@ int ucc_set_tdm_rxtx_clk(u32 tdm_num, enum qe_clock clock,
cmxs1cr = (tdm_num < 4) ? &qe_mux_reg->cmxsi1cr_l :
&qe_mux_reg->cmxsi1cr_h;
- qe_clrsetbits32(cmxs1cr, QE_CMXUCR_TX_CLK_SRC_MASK << shift,
- clock_bits << shift);
+ qe_clrsetbits_be32(cmxs1cr, QE_CMXUCR_TX_CLK_SRC_MASK << shift,
+ clock_bits << shift);
return 0;
}
@@ -650,9 +650,9 @@ int ucc_set_tdm_rxtx_sync(u32 tdm_num, enum qe_clock clock,
shift = ucc_get_tdm_sync_shift(mode, tdm_num);
- qe_clrsetbits32(&qe_mux_reg->cmxsi1syr,
- QE_CMXUCR_TX_CLK_SRC_MASK << shift,
- source << shift);
+ qe_clrsetbits_be32(&qe_mux_reg->cmxsi1syr,
+ QE_CMXUCR_TX_CLK_SRC_MASK << shift,
+ source << shift);
return 0;
}
diff --git a/include/soc/fsl/qe/qe.h b/include/soc/fsl/qe/qe.h
index c1036d16ed03..a1aa4eb28f0c 100644
--- a/include/soc/fsl/qe/qe.h
+++ b/include/soc/fsl/qe/qe.h
@@ -241,20 +241,20 @@ static inline int qe_alive_during_sleep(void)
#define qe_muram_offset cpm_muram_offset
#define qe_muram_dma cpm_muram_dma
-#define qe_setbits32(_addr, _v) iowrite32be(ioread32be(_addr) | (_v), (_addr))
-#define qe_clrbits32(_addr, _v) iowrite32be(ioread32be(_addr) & ~(_v), (_addr))
+#define qe_setbits_be32(_addr, _v) iowrite32be(ioread32be(_addr) | (_v),
(_addr))
+#define qe_clrbits_be32(_addr, _v) iowrite32be(ioread32be(_addr) & ~(_v),
(_addr))
-#define qe_setbits16(_addr, _v) iowrite16be(ioread16be(_addr) | (_v), (_addr))
-#define qe_clrbits16(_addr, _v) iowrite16be(ioread16be(_addr) & ~(_v), (_addr))
+#define qe_setbits_be16(_addr, _v) iowrite16be(ioread16be(_addr) | (_v),
(_addr))
+#define qe_clrbits_be16(_addr, _v) iowrite16be(ioread16be(_addr) & ~(_v),
(_addr))
-#define qe_setbits8(_addr, _v) iowrite8(ioread8(_addr) | (_v), (_addr))
-#define qe_clrbits8(_addr, _v) iowrite8(ioread8(_addr) & ~(_v), (_addr))
+#define qe_setbits_8(_addr, _v) iowrite8(ioread8(_addr) | (_v), (_addr))
+#define qe_clrbits_8(_addr, _v) iowrite8(ioread8(_addr) & ~(_v), (_addr))
-#define qe_clrsetbits32(addr, clear, set) \
+#define qe_clrsetbits_be32(addr, clear, set) \
iowrite32be((ioread32be(addr) & ~(clear)) | (set), (addr))
-#define qe_clrsetbits16(addr, clear, set) \
+#define qe_clrsetbits_be16(addr, clear, set) \
iowrite16be((ioread16be(addr) & ~(clear)) | (set), (addr))
-#define qe_clrsetbits8(addr, clear, set) \
+#define qe_clrsetbits_8(addr, clear, set) \
iowrite8((ioread8(addr) & ~(clear)) | (set), (addr))
/* Structure that defines QE firmware binary files.
--
2.23.0
