[PATCH v5 4/8] perf: Add group reads to perf_event_read()
From: Peter Zijlstra
Enable perf_event_read() to update entire groups at once, this will be
useful for read transactions.
Cc: Ingo Molnar
Cc: Arnaldo Carvalho de Melo
Cc: Michael Ellerman
Cc: Sukadev Bhattiprolu
Signed-off-by: Peter Zijlstra (Intel)
Link:
http://lkml.kernel.org/r/20150723080435.ge25...@twins.programming.kicks-ass.net
---
kernel/events/core.c | 39 ---
1 file changed, 32 insertions(+), 7 deletions(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 02095f4..31ec842 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3174,12 +3174,18 @@ void perf_event_exec(void)
rcu_read_unlock();
}
+struct perf_read_data {
+ struct perf_event *event;
+ bool group;
+};
+
/*
* Cross CPU call to read the hardware event
*/
static void __perf_event_read(void *info)
{
- struct perf_event *event = info;
+ struct perf_read_data *data = info;
+ struct perf_event *sub, *event = data->event;
struct perf_event_context *ctx = event->ctx;
struct perf_cpu_context *cpuctx = __get_cpu_context(ctx);
@@ -3198,9 +3204,21 @@ static void __perf_event_read(void *info)
update_context_time(ctx);
update_cgrp_time_from_event(event);
}
+
update_event_times(event);
if (event->state == PERF_EVENT_STATE_ACTIVE)
event->pmu->read(event);
+
+ if (!data->group)
+ goto unlock;
+
+ list_for_each_entry(sub, &event->sibling_list, group_entry) {
+ update_event_times(sub);
+ if (sub->state == PERF_EVENT_STATE_ACTIVE)
+ sub->pmu->read(sub);
+ }
+
+unlock:
raw_spin_unlock(&ctx->lock);
}
@@ -3212,15 +3230,19 @@ static inline u64 perf_event_count(struct perf_event
*event)
return __perf_event_count(event);
}
-static void perf_event_read(struct perf_event *event)
+static void perf_event_read(struct perf_event *event, bool group)
{
/*
* If event is enabled and currently active on a CPU, update the
* value in the event structure:
*/
if (event->state == PERF_EVENT_STATE_ACTIVE) {
+ struct perf_read_data data = {
+ .event = event,
+ .group = group,
+ };
smp_call_function_single(event->oncpu,
-__perf_event_read, event, 1);
+__perf_event_read, &data, 1);
} else if (event->state == PERF_EVENT_STATE_INACTIVE) {
struct perf_event_context *ctx = event->ctx;
unsigned long flags;
@@ -3235,7 +3257,10 @@ static void perf_event_read(struct perf_event *event)
update_context_time(ctx);
update_cgrp_time_from_event(event);
}
- update_event_times(event);
+ if (group)
+ update_group_times(event);
+ else
+ update_event_times(event);
raw_spin_unlock_irqrestore(&ctx->lock, flags);
}
}
@@ -3750,7 +3775,7 @@ u64 perf_event_read_value(struct perf_event *event, u64
*enabled, u64 *running)
mutex_lock(&event->child_mutex);
- perf_event_read(event);
+ perf_event_read(event, false);
total += perf_event_count(event);
*enabled += event->total_time_enabled +
@@ -3759,7 +3784,7 @@ u64 perf_event_read_value(struct perf_event *event, u64
*enabled, u64 *running)
atomic64_read(&event->child_total_time_running);
list_for_each_entry(child, &event->child_list, child_list) {
- perf_event_read(child);
+ perf_event_read(child, false);
total += perf_event_count(child);
*enabled += child->total_time_enabled;
*running += child->total_time_running;
@@ -3920,7 +3945,7 @@ static unsigned int perf_poll(struct file *file,
poll_table *wait)
static void _perf_event_reset(struct perf_event *event)
{
- perf_event_read(event);
+ perf_event_read(event, false);
local64_set(&event->count, 0);
perf_event_update_userpage(event);
}
--
1.7.9.5
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v5 8/8] powerpc/perf/hv-24x7: Use PERF_PMU_TXN_READ interface
The 24x7 counters in Powerpc allow monitoring a large number of counters
simultaneously. They also allow reading several counters in a single
HCALL so we can get a more consistent snapshot of the system.
Use the PMU's transaction interface to monitor and read several event
counters at once. The idea is that users can group several 24x7 events
into a single group of events. We use the following logic to submit
the group of events to the PMU and read the values:
pmu->start_txn()// Initialize before first event
for each event in group
pmu->read(event); // Queue each event to be read
pmu->commit_txn() // Read/update all queuedcounters
The ->commit_txn() also updates the event counts in the respective
perf_event objects. The perf subsystem can then directly get the
event counts from the perf_event and can avoid submitting a new
->read() request to the PMU.
Thanks to input from Peter Zijlstra.
Signed-off-by: Sukadev Bhattiprolu
---
Changelog[v3]
- [Peter Zijlstra] Save the transaction state in ->start_txn() and
remove the flags parameter from ->commit_txn() and ->cancel_txn().
---
arch/powerpc/perf/hv-24x7.c | 166 ++-
1 file changed, 164 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c
index 4d1a8d1..c4eee39 100644
--- a/arch/powerpc/perf/hv-24x7.c
+++ b/arch/powerpc/perf/hv-24x7.c
@@ -142,6 +142,15 @@ static struct attribute_group event_long_desc_group = {
static struct kmem_cache *hv_page_cache;
+DEFINE_PER_CPU(int, hv_24x7_txn_flags);
+DEFINE_PER_CPU(int, hv_24x7_txn_err);
+
+struct hv_24x7_hw {
+ struct perf_event *events[255];
+};
+
+DEFINE_PER_CPU(struct hv_24x7_hw, hv_24x7_hw);
+
/*
* request_buffer and result_buffer are not required to be 4k aligned,
* but are not allowed to cross any 4k boundary. Aligning them to 4k is
@@ -1233,9 +1242,48 @@ static void update_event_count(struct perf_event *event,
u64 now)
static void h_24x7_event_read(struct perf_event *event)
{
u64 now;
+ struct hv_24x7_request_buffer *request_buffer;
+ struct hv_24x7_hw *h24x7hw;
+ int txn_flags;
+
+ txn_flags = __this_cpu_read(hv_24x7_txn_flags);
+
+ /*
+* If in a READ transaction, add this counter to the list of
+* counters to read during the next HCALL (i.e commit_txn()).
+* If not in a READ transaction, go ahead and make the HCALL
+* to read this counter by itself.
+*/
+
+ if (txn_flags & PERF_PMU_TXN_READ) {
+ int i;
+ int ret;
- now = h_24x7_get_value(event);
- update_event_count(event, now);
+ if (__this_cpu_read(hv_24x7_txn_err))
+ return;
+
+ request_buffer = (void *)get_cpu_var(hv_24x7_reqb);
+
+ ret = add_event_to_24x7_request(event, request_buffer);
+ if (ret) {
+ __this_cpu_write(hv_24x7_txn_err, ret);
+ } else {
+ /*
+* Assoicate the event with the HCALL request index,
+* so ->commit_txn() can quickly find/update count.
+*/
+ i = request_buffer->num_requests - 1;
+
+ h24x7hw = &get_cpu_var(hv_24x7_hw);
+ h24x7hw->events[i] = event;
+ put_cpu_var(h24x7hw);
+ }
+
+ put_cpu_var(hv_24x7_reqb);
+ } else {
+ now = h_24x7_get_value(event);
+ update_event_count(event, now);
+ }
}
static void h_24x7_event_start(struct perf_event *event, int flags)
@@ -1257,6 +1305,117 @@ static int h_24x7_event_add(struct perf_event *event,
int flags)
return 0;
}
+/*
+ * 24x7 counters only support READ transactions. They are
+ * always counting and dont need/support ADD transactions.
+ * Cache the flags, but otherwise ignore transactions that
+ * are not PERF_PMU_TXN_READ.
+ */
+static void h_24x7_event_start_txn(struct pmu *pmu, int flags)
+{
+ struct hv_24x7_request_buffer *request_buffer;
+ struct hv_24x7_data_result_buffer *result_buffer;
+
+ /* We should not be called if we are already in a txn */
+ WARN_ON_ONCE(__this_cpu_read(hv_24x7_txn_flags));
+
+ __this_cpu_write(hv_24x7_txn_flags, flags);
+ if (flags & ~PERF_PMU_TXN_READ)
+ return;
+
+ request_buffer = (void *)get_cpu_var(hv_24x7_reqb);
+ result_buffer = (void *)get_cpu_var(hv_24x7_resb);
+
+ init_24x7_request(request_buffer, result_buffer);
+
+ put_cpu_var(hv_24x7_resb);
+ put_cpu_var(hv_24x7_reqb);
+}
+
+/*
+ * Clean up transaction state.
+ *
+ * NOTE: Ignore state of request and result buffers for now.
+ * We will initialize them during the next read/txn.
+ */
+static void reset_txn(void
[PATCH][RESEND] cxl:Plug irq_bitmap getting leaked in cxl_context
This patch plugs the leak of irq_bitmap, allocated as part of initialization of cxl_context struct; during the call to afu_allocate_irqs. The bitmap is now release during the call to function afu_release_irqs. Reported-by: Matthew R. Ochs Signed-off-by: Vaibhav Jain --- drivers/misc/cxl/irq.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/misc/cxl/irq.c b/drivers/misc/cxl/irq.c index 680cd26..c8f1f9d 100644 --- a/drivers/misc/cxl/irq.c +++ b/drivers/misc/cxl/irq.c @@ -511,4 +511,8 @@ void afu_release_irqs(struct cxl_context *ctx, void *cookie) afu_irq_name_free(ctx); cxl_release_irq_ranges(&ctx->irqs, ctx->afu->adapter); + + kfree(ctx->irq_bitmap); + ctx->irq_bitmap = NULL; + ctx->irq_count = 0; } -- 2.2.1 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/3] powerpc/e6500: hw tablewalk: optimize a bit for tcd lock acquiring codes
On Thu, Aug 13, 2015 at 01:44:43PM -0500, Scott Wood wrote: > On Thu, 2015-08-13 at 19:51 +0800, Kevin Hao wrote: > > It makes no sense to put the instructions for calculating the lock > > value (cpu number + 1) and the clearing of eq bit of cr1 in lbarx/stbcx > > loop. And when the lock is acquired by the other thread, the current > > lock value has no chance to equal with the lock value used by current > > cpu. So we can skip the comparing for these two lock values in the > > lbz/bne loop. > > > > Signed-off-by: Kevin Hao > > --- > > arch/powerpc/mm/tlb_low_64e.S | 10 +- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/arch/powerpc/mm/tlb_low_64e.S b/arch/powerpc/mm/tlb_low_64e.S > > index 765b419883f2..e4185581c5a7 100644 > > --- a/arch/powerpc/mm/tlb_low_64e.S > > +++ b/arch/powerpc/mm/tlb_low_64e.S > > @@ -308,11 +308,11 @@ BEGIN_FTR_SECTION /* CPU_FTR_SMT */ > >* > >* MAS6:IND should be already set based on MAS4 > >*/ > > -1: lbarx r15,0,r11 > > lhz r10,PACAPACAINDEX(r13) > > - cmpdi r15,0 > > - cmpdi cr1,r15,1 /* set cr1.eq = 0 for non-recursive */ > > addir10,r10,1 > > + crclr cr1*4+eq/* set cr1.eq = 0 for non-recursive */ > > +1: lbarx r15,0,r11 > > + cmpdi r15,0 > > bne 2f > > You're optimizing the contended case at the expense of introducing stalls in > the uncontended case. Before the patch, the uncontended case code sequence are: 1: lbarx r15,0,r11 lhz r10,PACAPACAINDEX(r13) cmpdi r15,0 cmpdi cr1,r15,1 /* set cr1.eq = 0 for non-recursive */ addir10,r10,1 bne 2f stbcx. r10,0,r11 bne 1b After the patch: lhz r10,PACAPACAINDEX(r13) addir10,r10,1 crclr cr1*4+eq/* set cr1.eq = 0 for non-recursive */ 1: lbarx r15,0,r11 cmpdi r15,0 bne 2f stbcx. r10,0,r11 bne 1b As you know, the lbarx is a Presync instruction and stbcx is a Presync and Postsync instruction. Putting the unnecessary instructions in the lbarx/stbcx loop also serialize these instructions execution. The execution latency of lbarx is only 3 cycles and there are still two instructions after it. Considering the out of order execution optimization after this patch, do you really think that new uncontended path will become slower due to this additional stall? > Does it really matter if there are more instructions > in the loop? I really think we should minimize the window of lbarx/stbcx, for following two reasons: - The bigger of this window, the more possible conflicts between the two threads run into this loop simultaneously. - The reservation used by lbarx may be cleared by another thread due to store to the same reservation granule. The smaller the window of lbarx/stbcx, the less possibility to be affected by this. > This change just means that you'll spin in the loop for more > iterations (if it even does that -- I think the cycles per loop iteration > might be the same before and after, due to load latency and pairing) while > waiting for the other thread to release the lock. Besides the optimization for the contended case, it also make the code more readable with these changes: - It always seem a bit weird to calculate the lock value for the current cpu in the lbarx/stbcx loop. - The "cmpdi cr1,r15,1" seems pretty confusion. It doesn't always do what the comment said (set cr1.eq = 0). In some cases, it does set the crq.eq = 1, such as when running on cpu 1 with lock is acquired by cpu0. All we need to do just clear the cr1.eq unconditionally. > > Do you have any benchmark results for this patch? I doubt it will get any visible difference. Anyway I will gave it a try. Thanks, Kevin > > -Scott > pgptWm4iwxxv3.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/3] powerpc/e6500: hw tablewalk: order the memory access when acquire/release tcd lock
On Thu, Aug 13, 2015 at 10:39:19PM -0500, Scott Wood wrote: > On Thu, 2015-08-13 at 19:51 +0800, Kevin Hao wrote: > > I didn't find anything unusual. But I think we do need to order the > > load/store of esel_next when acquire/release tcd lock. For acquire, > > add a data dependency to order the loads of lock and esel_next. > > For release, even there already have a "isync" here, but it doesn't > > guarantee any memory access order. So we still need "lwsync" for > > the two stores for lock and esel_next. > > I was going to say that esel_next is just a hint and it doesn't really matter > if we occasionally get the wrong value, unless it happens often enough to > cause more performance degradation than the lwsync causes. However, with the > A-008139 workaround we do need to read the same value from esel_next both > times. It might be less costly to save/restore an additional register > instead of lwsync, though. I will try to get some benchmark number to compare which method is a bit better. Do you have any recommended benchmark for a case this is? Thanks, Kevin > > -Scott > pgpQ3_2SiwY2A.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH V2] QorIQ/TMU: add thermal management support based on TMU
> -Original Message-
> From: Eduardo Valentin [mailto:edubez...@gmail.com]
> Sent: Friday, August 14, 2015 12:29 PM
> To: Jia Hongtao-B38951
> Cc: Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org; linux-
> p...@vger.kernel.org
> Subject: Re: [PATCH V2] QorIQ/TMU: add thermal management support based
> on TMU
>
> Hello Hongtao,
>
> On Fri, Aug 14, 2015 at 03:15:22AM +, Hongtao Jia wrote:
> > Hi Eduardo,
> >
> > In previous mail I asked questions about including header files in
> device tree.
> > Don't bother, I have already figured out the solution.
> >
> > Another questions is about cpu cooling:
> > I found out that there is no explicit calling for registering cpu
> > cooling device in the of-thermal style drivers.
>
> Your understanding is correct.
>
> >
> > And Samsung did it in cpufreq driver: drivers/cpufreq/exynos-cpufreq.c
> >
>
> Yes.
>
> > Should all the of-thermal driver use the same way?
>
> of-thermal won't handle the cooling device registering. It is typically
> registered by the cpufreq driver. Have a look in
> drivers/cpufreq/cpufreq-dt.c
>
> > Or is there any recommendation for registering cpu cooling device?
> > (I enabled the CONFIG_CPUFREQ_DT and still got no cooling device
> > registered)
>
> If your system supports using cpufreq-dt, then it will handle registering
> the cpucooling for you, if you configures the cooling dt properties in
> your DT files.
>
> How does your DT entry look like?
Here is the related code snippet:
cpus {
#address-cells = <1>;
#size-cells = <0>;
cpu0: PowerPC,e5500@0 {
device_type = "cpu";
reg = <0>;
clocks = <&mux0>;
next-level-cache = <&L2_1>;
/*cooling-min-level = <0>;*/
/*cooling-max-level = <1>;*/
#cooling-cells = <2>;
L2_1: l2-cache {
next-level-cache = <&cpc>;
};
};
cpu1: PowerPC,e5500@1 {
device_type = "cpu";
reg = <1>;
clocks = <&mux1>;
next-level-cache = <&L2_2>;
/*cooling-min-level = <0>;*/
/*cooling-max-level = <1>;*/
#cooling-cells = <2>;
L2_2: l2-cache {
next-level-cache = <&cpc>;
};
};
cpu2: PowerPC,e5500@2 {
device_type = "cpu";
reg = <2>;
clocks = <&mux2>;
next-level-cache = <&L2_3>;
/*cooling-min-level = <0>;*/
/*cooling-max-level = <1>;*/
#cooling-cells = <2>;
L2_3: l2-cache {
next-level-cache = <&cpc>;
};
};
cpu3: PowerPC,e5500@3 {
device_type = "cpu";
reg = <3>;
clocks = <&mux3>;
next-level-cache = <&L2_4>;
/*cooling-min-level = <0>;*/
/*cooling-max-level = <1>;*/
#cooling-cells = <2>;
L2_4: l2-cache {
next-level-cache = <&cpc>;
};
};
..
thermal-zones {
cpu_thermal: cpu-thermal {
polling-delay-passive = <1000>;
polling-delay = <5000>;
thermal-sensors = <&tmu>;
trips {
cpu_alert: cpu-alert {
temperature = <45000>;
hysteresis = <2000>;
type = "passive";
};
cpu_crit: cpu-crit {
temperature = <95000>;
hysteresis = <2000>;
type = "critical";
};
};
cooling-maps {
map0 {
trip = <&cpu_alert>;
cooling-device =
<&cpu0 THERMAL_NO_LIMIT
THERMAL_NO_LIMIT>;
};
map1 {
trip = <&cpu_alert>;
cooling-dev
Re: [PATCH] powerpc/eeh: Probe after unbalanced kref check
On Fri, Aug 14, 2015 at 04:03:19PM +1000, Daniel Axtens wrote:
>In the complete hotplug case, EEH PEs are supposed to be released
>and set to NULL. Normally, this is done by eeh_remove_device(),
>which is called from pcibios_release_device().
>
>However, if something is holding a kref to the device, it will not
>be released, and the PE will remain. eeh_add_device_late() has
>a check for this which will explictly destroy the PE in this case.
>
>This check in eeh_add_device_late() occurs after a call to
>eeh_ops->probe(). On PowerNV, probe is a pointer to pnv_eeh_probe(),
>which will exit without probing if there is an existing PE.
>
>This means that on PowerNV, devices with outstanding krefs will not
>be rediscovered by EEH correctly after a complete hotplug. This is
>affecting CXL (CAPI) devices in the field.
>
>Put the probe after the kref check so that the PE is destroyed
>and affected devices are correctly rediscovered by EEH.
>
>Fixes: d91dafc02f42 ("powerpc/eeh: Delay probing EEH device during hotplug")
>Cc: sta...@vger.kernel.org
>Cc: Gavin Shan
>Signed-off-by: Daniel Axtens
Acked-by: Gavin Shan
Thanks,
Gavin
>---
> arch/powerpc/kernel/eeh.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
>index af9b597b10af..8e61d717915e 100644
>--- a/arch/powerpc/kernel/eeh.c
>+++ b/arch/powerpc/kernel/eeh.c
>@@ -1116,9 +1116,6 @@ void eeh_add_device_late(struct pci_dev *dev)
> return;
> }
>
>- if (eeh_has_flag(EEH_PROBE_MODE_DEV))
>- eeh_ops->probe(pdn, NULL);
>-
> /*
>* The EEH cache might not be removed correctly because of
>* unbalanced kref to the device during unplug time, which
>@@ -1142,6 +1139,9 @@ void eeh_add_device_late(struct pci_dev *dev)
> dev->dev.archdata.edev = NULL;
> }
>
>+ if (eeh_has_flag(EEH_PROBE_MODE_DEV))
>+ eeh_ops->probe(pdn, NULL);
>+
> edev->pdev = dev;
> dev->dev.archdata.edev = edev;
>
>--
>2.1.4
>
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v5 00/11] CXL EEH Handling
CXL accelerators are unfortunately not immune from failure. This patch set enables them to particpate in the Extended Error Handling process. This series starts with a number of preparatory patches: - Patch 1 is cleanup: converting macros to static inlines. - Patch 2 makes sure we don't touch the hardware when it has failed. - Patches 3-5 make the 'unplug' functions idempotent, so that if we get part way through recovery and then fail, being completely unplugged as part of removal doesn't cause us to oops out. - Patches 6 and 7 refactor init and teardown paths for the adapter and AFUs, so that they can be configured and deconfigured separately from their allocation and release. - Patch 8 stops cxl_reset from breaking EEH. Patches 9 and 10 are parts of EEH. - Firstly we have a kernel flag that allows us to confidently assert the hardware will not change (be reflashed) when it it reset. We need this in order to be able to safely do EEH recovery. - We then have the EEH support itself. Finally, we add a CONFIG_CXL_EEH symbol. This allows drivers to depend on the API we provide to enable CXL EEH, or to be easily backportable if EEH is optional. Changes from v4 are trivial: - Undo the rc/if split - this will be redone in a follow-up patch. - Rebase on top of mpe's next tree so as to solve some merge conflicts. Changes from v3 are minor: - Clarification of responsibility of CXL driver vs driver bound to vPHB with regards to preventing inappropriate access of hardware during recovery. - Clean up unused rc in cxl_alloc_adapter, thanks David Laight. - Break setting rc and testing rc into different lines, thanks mpe and Cyril. - If we fail to init an AFU, don't try to select the best mode. Changes from v2 are mostly minor cleanups, reflecting some review and further testing. - Use static inlines instead of macros. - Propagate PCI link state to devices on the vPHB. - Various cleanup, thanks Cyril Bur. - Use pci_channel_offline instead of a direct check. - Don't ifdef, just provide the symbol so that drivers know that the new API is available. Thanks to Cyril for patiently explaining this to me about 3 times before I understood. Changes from v1: - More comprehensive link down checks, including vPHB. - Rebased to apply cleanly to 4.2-rc4. - cxl reset changes. - CONFIG_CXL_EEH symbol addition. - add better vPHB support to EEH. Daniel Axtens (11): cxl: Convert MMIO read/write macros to inline functions cxl: Drop commands if the PCI channel is not in normal state cxl: Allocate and release the SPA with the AFU cxl: Make IRQ release idempotent cxl: Clean up adapter MMIO unmap path. cxl: Refactor adaptor init/teardown cxl: Refactor AFU init/teardown cxl: Don't remove AFUs/vPHBs in cxl_reset cxl: Allow the kernel to trust that an image won't change on PERST. cxl: EEH support cxl: Add CONFIG_CXL_EEH symbol Documentation/ABI/testing/sysfs-class-cxl | 10 + drivers/misc/cxl/Kconfig | 6 + drivers/misc/cxl/api.c| 7 + drivers/misc/cxl/context.c| 6 +- drivers/misc/cxl/cxl.h| 84 - drivers/misc/cxl/file.c | 19 ++ drivers/misc/cxl/irq.c| 9 + drivers/misc/cxl/native.c | 105 +- drivers/misc/cxl/pci.c| 525 -- drivers/misc/cxl/sysfs.c | 26 ++ drivers/misc/cxl/vphb.c | 34 ++ include/misc/cxl.h| 10 + 12 files changed, 708 insertions(+), 133 deletions(-) -- 2.1.4 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v5 01/11] cxl: Convert MMIO read/write macros to inline functions
We're about to make these more complex, so make them functions
first.
Signed-off-by: Daniel Axtens
---
drivers/misc/cxl/cxl.h | 51 ++
1 file changed, 35 insertions(+), 16 deletions(-)
diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
index 4fd66cabde1e..6a93bfbcd826 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -537,10 +537,15 @@ static inline void __iomem *_cxl_p1_addr(struct cxl *cxl,
cxl_p1_reg_t reg)
return cxl->p1_mmio + cxl_reg_off(reg);
}
-#define cxl_p1_write(cxl, reg, val) \
- out_be64(_cxl_p1_addr(cxl, reg), val)
-#define cxl_p1_read(cxl, reg) \
- in_be64(_cxl_p1_addr(cxl, reg))
+static inline void cxl_p1_write(struct cxl *cxl, cxl_p1_reg_t reg, u64 val)
+{
+ out_be64(_cxl_p1_addr(cxl, reg), val);
+}
+
+static inline u64 cxl_p1_read(struct cxl *cxl, cxl_p1_reg_t reg)
+{
+ return in_be64(_cxl_p1_addr(cxl, reg));
+}
static inline void __iomem *_cxl_p1n_addr(struct cxl_afu *afu, cxl_p1n_reg_t
reg)
{
@@ -548,26 +553,40 @@ static inline void __iomem *_cxl_p1n_addr(struct cxl_afu
*afu, cxl_p1n_reg_t reg
return afu->p1n_mmio + cxl_reg_off(reg);
}
-#define cxl_p1n_write(afu, reg, val) \
- out_be64(_cxl_p1n_addr(afu, reg), val)
-#define cxl_p1n_read(afu, reg) \
- in_be64(_cxl_p1n_addr(afu, reg))
+static inline void cxl_p1n_write(struct cxl_afu *afu, cxl_p1n_reg_t reg, u64
val)
+{
+ out_be64(_cxl_p1n_addr(afu, reg), val);
+}
+
+static inline u64 cxl_p1n_read(struct cxl_afu *afu, cxl_p1n_reg_t reg)
+{
+ return in_be64(_cxl_p1n_addr(afu, reg));
+}
static inline void __iomem *_cxl_p2n_addr(struct cxl_afu *afu, cxl_p2n_reg_t
reg)
{
return afu->p2n_mmio + cxl_reg_off(reg);
}
-#define cxl_p2n_write(afu, reg, val) \
- out_be64(_cxl_p2n_addr(afu, reg), val)
-#define cxl_p2n_read(afu, reg) \
- in_be64(_cxl_p2n_addr(afu, reg))
+static inline void cxl_p2n_write(struct cxl_afu *afu, cxl_p2n_reg_t reg, u64
val)
+{
+ out_be64(_cxl_p2n_addr(afu, reg), val);
+}
+static inline u64 cxl_p2n_read(struct cxl_afu *afu, cxl_p2n_reg_t reg)
+{
+ return in_be64(_cxl_p2n_addr(afu, reg));
+}
-#define cxl_afu_cr_read64(afu, cr, off) \
- in_le64((afu)->afu_desc_mmio + (afu)->crs_offset + ((cr) *
(afu)->crs_len) + (off))
-#define cxl_afu_cr_read32(afu, cr, off) \
- in_le32((afu)->afu_desc_mmio + (afu)->crs_offset + ((cr) *
(afu)->crs_len) + (off))
+static inline u64 cxl_afu_cr_read64(struct cxl_afu *afu, int cr, u64 off)
+{
+ return in_le64((afu)->afu_desc_mmio + (afu)->crs_offset + ((cr) *
(afu)->crs_len) + (off));
+}
+
+static inline u32 cxl_afu_cr_read32(struct cxl_afu *afu, int cr, u64 off)
+{
+ return in_le32((afu)->afu_desc_mmio + (afu)->crs_offset + ((cr) *
(afu)->crs_len) + (off));
+}
u16 cxl_afu_cr_read16(struct cxl_afu *afu, int cr, u64 off);
u8 cxl_afu_cr_read8(struct cxl_afu *afu, int cr, u64 off);
--
2.1.4
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v5 02/11] cxl: Drop commands if the PCI channel is not in normal state
If the PCI channel has gone down, don't attempt to poke the hardware.
We need to guard every time cxl_whatever_(read|write) is called. This
is because a call to those functions will dereference an offset into an
mmio register, and the mmio mappings get invalidated in the EEH
teardown.
Check in the read/write functions in the header.
We give them the same semantics as usual PCI operations:
- a write to a channel that is down is ignored.
- a read from a channel that is down returns all fs.
Also, we try to access the MMIO space of a vPHB device as part of the
PCI disable path. Because that's a read that bypasses most of our usual
checks, we handle it explicitly.
As far as user visible warnings go:
- Check link state in file ops, return -EIO if down.
- Be reasonably quiet if there's an error in a teardown path,
or when we already know the hardware is going down.
- Throw a big WARN if someone tries to start a CXL operation
while the card is down. This gives a useful stacktrace for
debugging whatever is doing that.
Signed-off-by: Daniel Axtens
---
drivers/misc/cxl/context.c | 6 +++-
drivers/misc/cxl/cxl.h | 44 ++--
drivers/misc/cxl/file.c| 19
drivers/misc/cxl/native.c | 72 --
drivers/misc/cxl/vphb.c| 26 +
5 files changed, 155 insertions(+), 12 deletions(-)
diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c
index 1287148629c0..615842115848 100644
--- a/drivers/misc/cxl/context.c
+++ b/drivers/misc/cxl/context.c
@@ -193,7 +193,11 @@ int __detach_context(struct cxl_context *ctx)
if (status != STARTED)
return -EBUSY;
- WARN_ON(cxl_detach_process(ctx));
+ /* Only warn if we detached while the link was OK.
+* If detach fails when hw is down, we don't care.
+*/
+ WARN_ON(cxl_detach_process(ctx) &&
+ cxl_adapter_link_ok(ctx->afu->adapter));
flush_work(&ctx->fault_work); /* Only needed for dedicated process */
put_pid(ctx->pid);
cxl_ctx_put();
diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
index 6a93bfbcd826..9b9e89fd02cc 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -531,6 +531,14 @@ struct cxl_process_element {
__be32 software_state;
} __packed;
+static inline bool cxl_adapter_link_ok(struct cxl *cxl)
+{
+ struct pci_dev *pdev;
+
+ pdev = to_pci_dev(cxl->dev.parent);
+ return !pci_channel_offline(pdev);
+}
+
static inline void __iomem *_cxl_p1_addr(struct cxl *cxl, cxl_p1_reg_t reg)
{
WARN_ON(!cpu_has_feature(CPU_FTR_HVMODE));
@@ -539,12 +547,16 @@ static inline void __iomem *_cxl_p1_addr(struct cxl *cxl,
cxl_p1_reg_t reg)
static inline void cxl_p1_write(struct cxl *cxl, cxl_p1_reg_t reg, u64 val)
{
- out_be64(_cxl_p1_addr(cxl, reg), val);
+ if (likely(cxl_adapter_link_ok(cxl)))
+ out_be64(_cxl_p1_addr(cxl, reg), val);
}
static inline u64 cxl_p1_read(struct cxl *cxl, cxl_p1_reg_t reg)
{
- return in_be64(_cxl_p1_addr(cxl, reg));
+ if (likely(cxl_adapter_link_ok(cxl)))
+ return in_be64(_cxl_p1_addr(cxl, reg));
+ else
+ return ~0ULL;
}
static inline void __iomem *_cxl_p1n_addr(struct cxl_afu *afu, cxl_p1n_reg_t
reg)
@@ -555,12 +567,16 @@ static inline void __iomem *_cxl_p1n_addr(struct cxl_afu
*afu, cxl_p1n_reg_t reg
static inline void cxl_p1n_write(struct cxl_afu *afu, cxl_p1n_reg_t reg, u64
val)
{
- out_be64(_cxl_p1n_addr(afu, reg), val);
+ if (likely(cxl_adapter_link_ok(afu->adapter)))
+ out_be64(_cxl_p1n_addr(afu, reg), val);
}
static inline u64 cxl_p1n_read(struct cxl_afu *afu, cxl_p1n_reg_t reg)
{
- return in_be64(_cxl_p1n_addr(afu, reg));
+ if (likely(cxl_adapter_link_ok(afu->adapter)))
+ return in_be64(_cxl_p1n_addr(afu, reg));
+ else
+ return ~0ULL;
}
static inline void __iomem *_cxl_p2n_addr(struct cxl_afu *afu, cxl_p2n_reg_t
reg)
@@ -570,22 +586,34 @@ static inline void __iomem *_cxl_p2n_addr(struct cxl_afu
*afu, cxl_p2n_reg_t reg
static inline void cxl_p2n_write(struct cxl_afu *afu, cxl_p2n_reg_t reg, u64
val)
{
- out_be64(_cxl_p2n_addr(afu, reg), val);
+ if (likely(cxl_adapter_link_ok(afu->adapter)))
+ out_be64(_cxl_p2n_addr(afu, reg), val);
}
static inline u64 cxl_p2n_read(struct cxl_afu *afu, cxl_p2n_reg_t reg)
{
- return in_be64(_cxl_p2n_addr(afu, reg));
+ if (likely(cxl_adapter_link_ok(afu->adapter)))
+ return in_be64(_cxl_p2n_addr(afu, reg));
+ else
+ return ~0ULL;
}
static inline u64 cxl_afu_cr_read64(struct cxl_afu *afu, int cr, u64 off)
{
- return in_le64((afu)->afu_desc_mmio + (afu)->crs_offset + ((cr) *
(afu)->crs_len) + (off));
+ if (likely(cxl_adapter_link_ok(afu->adapter)))
+ return in_le6
[PATCH v5 03/11] cxl: Allocate and release the SPA with the AFU
Previously the SPA was allocated and freed upon entering and leaving
AFU-directed mode. This causes some issues for error recovery - contexts
hold a pointer inside the SPA, and they may persist after the AFU has
been detached.
We would ideally like to allocate the SPA when the AFU is allocated, and
release it until the AFU is released. However, we don't know how big the
SPA needs to be until we read the AFU descriptor.
Therefore, restructure the code:
- Allocate the SPA only once, on the first attach.
- Release the SPA only when the entire AFU is being released (not
detached). Guard the release with a NULL check, so we don't free
if it was never allocated (e.g. dedicated mode)
Acked-by: Cyril Bur
Signed-off-by: Daniel Axtens
---
drivers/misc/cxl/cxl.h| 3 +++
drivers/misc/cxl/native.c | 33 ++---
drivers/misc/cxl/pci.c| 2 ++
3 files changed, 27 insertions(+), 11 deletions(-)
diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
index 9b9e89fd02cc..d540542f9931 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -632,6 +632,9 @@ void unregister_cxl_calls(struct cxl_calls *calls);
int cxl_alloc_adapter_nr(struct cxl *adapter);
void cxl_remove_adapter_nr(struct cxl *adapter);
+int cxl_alloc_spa(struct cxl_afu *afu);
+void cxl_release_spa(struct cxl_afu *afu);
+
int cxl_file_init(void);
void cxl_file_exit(void);
int cxl_register_adapter(struct cxl *adapter);
diff --git a/drivers/misc/cxl/native.c b/drivers/misc/cxl/native.c
index 44568dd68bb9..b37f2e8004f5 100644
--- a/drivers/misc/cxl/native.c
+++ b/drivers/misc/cxl/native.c
@@ -183,10 +183,8 @@ static int spa_max_procs(int spa_size)
return ((spa_size / 8) - 96) / 17;
}
-static int alloc_spa(struct cxl_afu *afu)
+int cxl_alloc_spa(struct cxl_afu *afu)
{
- u64 spap;
-
/* Work out how many pages to allocate */
afu->spa_order = 0;
do {
@@ -205,6 +203,13 @@ static int alloc_spa(struct cxl_afu *afu)
pr_devel("spa pages: %i afu->spa_max_procs: %i afu->num_procs: %i\n",
1spa_max_procs, afu->num_procs);
+ return 0;
+}
+
+static void attach_spa(struct cxl_afu *afu)
+{
+ u64 spap;
+
afu->sw_command_status = (__be64 *)((char *)afu->spa +
((afu->spa_max_procs + 3) * 128));
@@ -213,14 +218,19 @@ static int alloc_spa(struct cxl_afu *afu)
spap |= CXL_PSL_SPAP_V;
pr_devel("cxl: SPA allocated at 0x%p. Max processes: %i,
sw_command_status: 0x%p CXL_PSL_SPAP_An=0x%016llx\n", afu->spa,
afu->spa_max_procs, afu->sw_command_status, spap);
cxl_p1n_write(afu, CXL_PSL_SPAP_An, spap);
-
- return 0;
}
-static void release_spa(struct cxl_afu *afu)
+static inline void detach_spa(struct cxl_afu *afu)
{
cxl_p1n_write(afu, CXL_PSL_SPAP_An, 0);
- free_pages((unsigned long) afu->spa, afu->spa_order);
+}
+
+void cxl_release_spa(struct cxl_afu *afu)
+{
+ if (afu->spa) {
+ free_pages((unsigned long) afu->spa, afu->spa_order);
+ afu->spa = NULL;
+ }
}
int cxl_tlb_slb_invalidate(struct cxl *adapter)
@@ -447,8 +457,11 @@ static int activate_afu_directed(struct cxl_afu *afu)
dev_info(&afu->dev, "Activating AFU directed mode\n");
- if (alloc_spa(afu))
- return -ENOMEM;
+ if (afu->spa == NULL) {
+ if (cxl_alloc_spa(afu))
+ return -ENOMEM;
+ }
+ attach_spa(afu);
cxl_p1n_write(afu, CXL_PSL_SCNTL_An, CXL_PSL_SCNTL_An_PM_AFU);
cxl_p1n_write(afu, CXL_PSL_AMOR_An, 0xULL);
@@ -559,8 +572,6 @@ static int deactivate_afu_directed(struct cxl_afu *afu)
cxl_afu_disable(afu);
cxl_psl_purge(afu);
- release_spa(afu);
-
return 0;
}
diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
index 1d314f1f95fe..dc26cdd653ad 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -552,6 +552,8 @@ static void cxl_release_afu(struct device *dev)
pr_devel("cxl_release_afu\n");
idr_destroy(&afu->contexts_idr);
+ cxl_release_spa(afu);
+
kfree(afu);
}
--
2.1.4
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v5 04/11] cxl: Make IRQ release idempotent
Check if an IRQ is mapped before releasing it.
This will simplify future EEH code by allowing unconditional unmapping
of IRQs.
Acked-by: Cyril Bur
Signed-off-by: Daniel Axtens
---
drivers/misc/cxl/irq.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/misc/cxl/irq.c b/drivers/misc/cxl/irq.c
index b6b04374c10c..62823784f68e 100644
--- a/drivers/misc/cxl/irq.c
+++ b/drivers/misc/cxl/irq.c
@@ -341,6 +341,9 @@ int cxl_register_psl_err_irq(struct cxl *adapter)
void cxl_release_psl_err_irq(struct cxl *adapter)
{
+ if (adapter->err_virq != irq_find_mapping(NULL, adapter->err_hwirq))
+ return;
+
cxl_p1_write(adapter, CXL_PSL_ErrIVTE, 0x);
cxl_unmap_irq(adapter->err_virq, adapter);
cxl_release_one_irq(adapter, adapter->err_hwirq);
@@ -374,6 +377,9 @@ int cxl_register_serr_irq(struct cxl_afu *afu)
void cxl_release_serr_irq(struct cxl_afu *afu)
{
+ if (afu->serr_virq != irq_find_mapping(NULL, afu->serr_hwirq))
+ return;
+
cxl_p1n_write(afu, CXL_PSL_SERR_An, 0x);
cxl_unmap_irq(afu->serr_virq, afu);
cxl_release_one_irq(afu->adapter, afu->serr_hwirq);
@@ -400,6 +406,9 @@ int cxl_register_psl_irq(struct cxl_afu *afu)
void cxl_release_psl_irq(struct cxl_afu *afu)
{
+ if (afu->psl_virq != irq_find_mapping(NULL, afu->psl_hwirq))
+ return;
+
cxl_unmap_irq(afu->psl_virq, afu);
cxl_release_one_irq(afu->adapter, afu->psl_hwirq);
kfree(afu->psl_irq_name);
--
2.1.4
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v5 05/11] cxl: Clean up adapter MMIO unmap path.
- MMIO pointer unmapping is guarded by a null pointer check.
However, iounmap doesn't null the pointer, just invalidate it.
Therefore, explicitly null the pointer after unmapping.
- afu_desc_mmio also needs to be unmapped.
- PCI regions are allocated in cxl_map_adapter_regs.
Therefore they should be released in unmap, not elsewhere.
Acked-by: Cyril Bur
Signed-off-by: Daniel Axtens
---
drivers/misc/cxl/pci.c | 24 ++--
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
index dc26cdd653ad..1dae1db3166d 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -539,10 +539,18 @@ err:
static void cxl_unmap_slice_regs(struct cxl_afu *afu)
{
- if (afu->p2n_mmio)
+ if (afu->p2n_mmio) {
iounmap(afu->p2n_mmio);
- if (afu->p1n_mmio)
+ afu->p2n_mmio = NULL;
+ }
+ if (afu->p1n_mmio) {
iounmap(afu->p1n_mmio);
+ afu->p1n_mmio = NULL;
+ }
+ if (afu->afu_desc_mmio) {
+ iounmap(afu->afu_desc_mmio);
+ afu->afu_desc_mmio = NULL;
+ }
}
static void cxl_release_afu(struct device *dev)
@@ -920,10 +928,16 @@ err1:
static void cxl_unmap_adapter_regs(struct cxl *adapter)
{
- if (adapter->p1_mmio)
+ if (adapter->p1_mmio) {
iounmap(adapter->p1_mmio);
- if (adapter->p2_mmio)
+ adapter->p1_mmio = NULL;
+ pci_release_region(to_pci_dev(adapter->dev.parent), 2);
+ }
+ if (adapter->p2_mmio) {
iounmap(adapter->p2_mmio);
+ adapter->p2_mmio = NULL;
+ pci_release_region(to_pci_dev(adapter->dev.parent), 0);
+ }
}
static int cxl_read_vsec(struct cxl *adapter, struct pci_dev *dev)
@@ -1132,8 +1146,6 @@ static void cxl_remove_adapter(struct cxl *adapter)
device_unregister(&adapter->dev);
- pci_release_region(pdev, 0);
- pci_release_region(pdev, 2);
pci_disable_device(pdev);
}
--
2.1.4
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v5 06/11] cxl: Refactor adaptor init/teardown
Some aspects of initialisation are done only once in the lifetime of
an adapter: for example, allocating memory for the adapter,
allocating the adapter number, or setting up sysfs/debugfs files.
However, we may want to be able to do some parts of the
initialisation multiple times: for example, in error recovery we
want to be able to tear down and then re-map IO memory and IRQs.
Therefore, refactor CXL init/teardown as follows.
- Keep the overarching functions 'cxl_init_adapter' and its pair,
'cxl_remove_adapter'.
- Move all 'once only' allocation/freeing steps to the existing
'cxl_alloc_adapter' function, and its pair 'cxl_release_adapter'
(This involves moving allocation of the adapter number out of
cxl_init_adapter.)
- Create two new functions: 'cxl_configure_adapter', and its pair
'cxl_deconfigure_adapter'. These two functions 'wire up' the
hardware --- they (de)configure resources that do not need to
last the entire lifetime of the adapter
Signed-off-by: Daniel Axtens
---
drivers/misc/cxl/pci.c | 139 ++---
1 file changed, 86 insertions(+), 53 deletions(-)
diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
index 1dae1db3166d..feec4800e922 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -966,7 +966,6 @@ static int cxl_read_vsec(struct cxl *adapter, struct
pci_dev *dev)
CXL_READ_VSEC_BASE_IMAGE(dev, vsec, &adapter->base_image);
CXL_READ_VSEC_IMAGE_STATE(dev, vsec, &image_state);
adapter->user_image_loaded = !!(image_state &
CXL_VSEC_USER_IMAGE_LOADED);
- adapter->perst_loads_image = true;
adapter->perst_select_user = !!(image_state &
CXL_VSEC_USER_IMAGE_LOADED);
CXL_READ_VSEC_NAFUS(dev, vsec, &adapter->slices);
@@ -1026,22 +1025,33 @@ static void cxl_release_adapter(struct device *dev)
pr_devel("cxl_release_adapter\n");
+ cxl_remove_adapter_nr(adapter);
+
kfree(adapter);
}
-static struct cxl *cxl_alloc_adapter(struct pci_dev *dev)
+static struct cxl *cxl_alloc_adapter(void)
{
struct cxl *adapter;
if (!(adapter = kzalloc(sizeof(struct cxl), GFP_KERNEL)))
return NULL;
- adapter->dev.parent = &dev->dev;
- adapter->dev.release = cxl_release_adapter;
- pci_set_drvdata(dev, adapter);
spin_lock_init(&adapter->afu_list_lock);
+ if (cxl_alloc_adapter_nr(adapter))
+ goto err1;
+
+ if (dev_set_name(&adapter->dev, "card%i", adapter->adapter_num))
+ goto err2;
+
return adapter;
+
+err2:
+ cxl_remove_adapter_nr(adapter);
+err1:
+ kfree(adapter);
+ return NULL;
}
static int sanitise_adapter_regs(struct cxl *adapter)
@@ -1050,57 +1060,96 @@ static int sanitise_adapter_regs(struct cxl *adapter)
return cxl_tlb_slb_invalidate(adapter);
}
-static struct cxl *cxl_init_adapter(struct pci_dev *dev)
+/* This should contain *only* operations that can safely be done in
+ * both creation and recovery.
+ */
+static int cxl_configure_adapter(struct cxl *adapter, struct pci_dev *dev)
{
- struct cxl *adapter;
- bool free = true;
int rc;
+ adapter->dev.parent = &dev->dev;
+ adapter->dev.release = cxl_release_adapter;
+ pci_set_drvdata(dev, adapter);
- if (!(adapter = cxl_alloc_adapter(dev)))
- return ERR_PTR(-ENOMEM);
+ rc = pci_enable_device(dev);
+ if (rc) {
+ dev_err(&dev->dev, "pci_enable_device failed: %i\n", rc);
+ return rc;
+ }
if ((rc = cxl_read_vsec(adapter, dev)))
- goto err1;
+ return rc;
if ((rc = cxl_vsec_looks_ok(adapter, dev)))
- goto err1;
+ return rc;
if ((rc = setup_cxl_bars(dev)))
- goto err1;
+ return rc;
if ((rc = switch_card_to_cxl(dev)))
- goto err1;
-
- if ((rc = cxl_alloc_adapter_nr(adapter)))
- goto err1;
-
- if ((rc = dev_set_name(&adapter->dev, "card%i", adapter->adapter_num)))
- goto err2;
+ return rc;
if ((rc = cxl_update_image_control(adapter)))
- goto err2;
+ return rc;
if ((rc = cxl_map_adapter_regs(adapter, dev)))
- goto err2;
+ return rc;
if ((rc = sanitise_adapter_regs(adapter)))
- goto err2;
+ goto err;
if ((rc = init_implementation_adapter_regs(adapter, dev)))
- goto err3;
+ goto err;
if ((rc = pnv_phb_to_cxl_mode(dev, OPAL_PHB_CAPI_MODE_CAPI)))
- goto err3;
+ goto err;
/* If recovery happened, the last step is to turn on snooping.
* In the non-recovery case this has no effect */
- if ((rc = pnv_phb_to_cxl_mode(dev, OPAL_PHB_CAPI_MODE_SNOOP_ON))) {
-
[PATCH v5 07/11] cxl: Refactor AFU init/teardown
As with an adapter, some aspects of initialisation are done only once
in the lifetime of an AFU: for example, allocating memory, or setting
up sysfs/debugfs files.
However, we may want to be able to do some parts of the initialisation
multiple times: for example, in error recovery we want to be able to
tear down and then re-map IO memory and IRQs.
Therefore, refactor AFU init/teardown as follows.
- Create two new functions: 'cxl_configure_afu', and its pair
'cxl_deconfigure_afu'. As with the adapter functions,
these (de)configure resources that do not need to last the entire
lifetime of the AFU.
- Allocating and releasing memory remain the task of 'cxl_alloc_afu'
and 'cxl_release_afu'.
- Once-only functions that do not involve allocating/releasing memory
stay in the overarching 'cxl_init_afu'/'cxl_remove_afu' pair.
However, the task of picking an AFU mode and activating it has been
broken out.
Signed-off-by: Daniel Axtens
---
drivers/misc/cxl/pci.c | 95 ++
1 file changed, 57 insertions(+), 38 deletions(-)
diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
index feec4800e922..b63d96023c50 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -753,45 +753,70 @@ ssize_t cxl_afu_read_err_buffer(struct cxl_afu *afu, char
*buf,
return count;
}
-static int cxl_init_afu(struct cxl *adapter, int slice, struct pci_dev *dev)
+static int cxl_configure_afu(struct cxl_afu *afu, struct cxl *adapter, struct
pci_dev *dev)
{
- struct cxl_afu *afu;
- bool free = true;
int rc;
- if (!(afu = cxl_alloc_afu(adapter, slice)))
- return -ENOMEM;
-
- if ((rc = dev_set_name(&afu->dev, "afu%i.%i", adapter->adapter_num,
slice)))
- goto err1;
-
if ((rc = cxl_map_slice_regs(afu, adapter, dev)))
- goto err1;
+ return rc;
if ((rc = sanitise_afu_regs(afu)))
- goto err2;
+ goto err1;
/* We need to reset the AFU before we can read the AFU descriptor */
if ((rc = __cxl_afu_reset(afu)))
- goto err2;
+ goto err1;
if (cxl_verbose)
dump_afu_descriptor(afu);
if ((rc = cxl_read_afu_descriptor(afu)))
- goto err2;
+ goto err1;
if ((rc = cxl_afu_descriptor_looks_ok(afu)))
- goto err2;
+ goto err1;
if ((rc = init_implementation_afu_regs(afu)))
- goto err2;
+ goto err1;
if ((rc = cxl_register_serr_irq(afu)))
- goto err2;
+ goto err1;
if ((rc = cxl_register_psl_irq(afu)))
- goto err3;
+ goto err2;
+
+ return 0;
+
+err2:
+ cxl_release_serr_irq(afu);
+err1:
+ cxl_unmap_slice_regs(afu);
+ return rc;
+}
+
+static void cxl_deconfigure_afu(struct cxl_afu *afu)
+{
+ cxl_release_psl_irq(afu);
+ cxl_release_serr_irq(afu);
+ cxl_unmap_slice_regs(afu);
+}
+
+static int cxl_init_afu(struct cxl *adapter, int slice, struct pci_dev *dev)
+{
+ struct cxl_afu *afu;
+ int rc;
+
+ afu = cxl_alloc_afu(adapter, slice);
+ if (!afu)
+ return -ENOMEM;
+
+ rc = dev_set_name(&afu->dev, "afu%i.%i", adapter->adapter_num, slice);
+ if (rc)
+ goto err_free;
+
+ rc = cxl_configure_afu(afu, adapter, dev);
+ if (rc)
+ goto err_free;
/* Don't care if this fails */
cxl_debugfs_afu_add(afu);
@@ -806,10 +831,6 @@ static int cxl_init_afu(struct cxl *adapter, int slice,
struct pci_dev *dev)
if ((rc = cxl_sysfs_afu_add(afu)))
goto err_put1;
-
- if ((rc = cxl_afu_select_best_mode(afu)))
- goto err_put2;
-
adapter->afu[afu->slice] = afu;
if ((rc = cxl_pci_vphb_add(afu)))
@@ -817,21 +838,16 @@ static int cxl_init_afu(struct cxl *adapter, int slice,
struct pci_dev *dev)
return 0;
-err_put2:
- cxl_sysfs_afu_remove(afu);
err_put1:
- device_unregister(&afu->dev);
- free = false;
+ cxl_deconfigure_afu(afu);
cxl_debugfs_afu_remove(afu);
- cxl_release_psl_irq(afu);
-err3:
- cxl_release_serr_irq(afu);
-err2:
- cxl_unmap_slice_regs(afu);
-err1:
- if (free)
- kfree(afu);
+ device_unregister(&afu->dev);
return rc;
+
+err_free:
+ kfree(afu);
+ return rc;
+
}
static void cxl_remove_afu(struct cxl_afu *afu)
@@ -851,10 +867,7 @@ static void cxl_remove_afu(struct cxl_afu *afu)
cxl_context_detach_all(afu);
cxl_afu_deactivate_mode(afu);
- cxl_release_psl_irq(afu);
- cxl_release_serr_irq(afu);
- cxl_unmap_slice_regs(afu);
-
+ cxl_deconfigure_afu(afu);
device_unregister(&afu->dev);
}
@@ -1206,8 +1219,14 @@ static int cxl_probe(struct
[PATCH v5 08/11] cxl: Don't remove AFUs/vPHBs in cxl_reset
If the driver doesn't participate in EEH, the AFUs will be removed
by cxl_remove, which will be invoked by EEH.
If the driver does particpate in EEH, the vPHB needs to stick around
so that the it can particpate.
In both cases, we shouldn't remove the AFU/vPHB.
Reviewed-by: Cyril Bur
Signed-off-by: Daniel Axtens
---
drivers/misc/cxl/pci.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
index b63d96023c50..2b61cb1ee62c 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -880,11 +880,6 @@ int cxl_reset(struct cxl *adapter)
dev_info(&dev->dev, "CXL reset\n");
- for (i = 0; i < adapter->slices; i++) {
- cxl_pci_vphb_remove(adapter->afu[i]);
- cxl_remove_afu(adapter->afu[i]);
- }
-
/* pcie_warm_reset requests a fundamental pci reset which includes a
* PERST assert/deassert. PERST triggers a loading of the image
* if "user" or "factory" is selected in sysfs */
--
2.1.4
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v5 09/11] cxl: Allow the kernel to trust that an image won't change on PERST.
Provide a kernel API and a sysfs entry which allow a user to specify
that when a card is PERSTed, it's image will stay the same, allowing
it to participate in EEH.
cxl_reset is used to reflash the card. In that case, we cannot safely
assert that the image will not change. Therefore, disallow cxl_reset
if the flag is set.
Signed-off-by: Daniel Axtens
---
Documentation/ABI/testing/sysfs-class-cxl | 10 ++
drivers/misc/cxl/api.c| 7 +++
drivers/misc/cxl/cxl.h| 1 +
drivers/misc/cxl/pci.c| 7 +++
drivers/misc/cxl/sysfs.c | 26 ++
include/misc/cxl.h| 10 ++
6 files changed, 61 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-class-cxl
b/Documentation/ABI/testing/sysfs-class-cxl
index acfe9df83139..b07e86d4597f 100644
--- a/Documentation/ABI/testing/sysfs-class-cxl
+++ b/Documentation/ABI/testing/sysfs-class-cxl
@@ -223,3 +223,13 @@ Description:write only
Writing 1 will issue a PERST to card which may cause the card
to reload the FPGA depending on load_image_on_perst.
Users: https://github.com/ibm-capi/libcxl
+
+What: /sys/class/cxl//perst_reloads_same_image
+Date: July 2015
+Contact: linuxppc-dev@lists.ozlabs.org
+Description: read/write
+ Trust that when an image is reloaded via PERST, it will not
+ have changed.
+ 0 = don't trust, the image may be different (default)
+ 1 = trust that the image will not change.
+Users: https://github.com/ibm-capi/libcxl
diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
index 729e0851167d..6a768a9ad22f 100644
--- a/drivers/misc/cxl/api.c
+++ b/drivers/misc/cxl/api.c
@@ -327,3 +327,10 @@ int cxl_afu_reset(struct cxl_context *ctx)
return cxl_afu_check_and_enable(afu);
}
EXPORT_SYMBOL_GPL(cxl_afu_reset);
+
+void cxl_perst_reloads_same_image(struct cxl_afu *afu,
+ bool perst_reloads_same_image)
+{
+ afu->adapter->perst_same_image = perst_reloads_same_image;
+}
+EXPORT_SYMBOL_GPL(cxl_perst_reloads_same_image);
diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
index d540542f9931..cda02412b01e 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -493,6 +493,7 @@ struct cxl {
bool user_image_loaded;
bool perst_loads_image;
bool perst_select_user;
+ bool perst_same_image;
};
int cxl_alloc_one_irq(struct cxl *adapter);
diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
index 2b61cb1ee62c..bfbd6478c0c5 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -878,6 +878,12 @@ int cxl_reset(struct cxl *adapter)
int i;
u32 val;
+ if (adapter->perst_same_image) {
+ dev_warn(&dev->dev,
+"cxl: refusing to reset/reflash when
perst_reloads_same_image is set.\n");
+ return -EINVAL;
+ }
+
dev_info(&dev->dev, "CXL reset\n");
/* pcie_warm_reset requests a fundamental pci reset which includes a
@@ -1151,6 +1157,7 @@ static struct cxl *cxl_init_adapter(struct pci_dev *dev)
* configure/reconfigure
*/
adapter->perst_loads_image = true;
+ adapter->perst_same_image = false;
rc = cxl_configure_adapter(adapter, dev);
if (rc) {
diff --git a/drivers/misc/cxl/sysfs.c b/drivers/misc/cxl/sysfs.c
index 31f38bc71a3d..6619cf1f6e1f 100644
--- a/drivers/misc/cxl/sysfs.c
+++ b/drivers/misc/cxl/sysfs.c
@@ -112,12 +112,38 @@ static ssize_t load_image_on_perst_store(struct device
*device,
return count;
}
+static ssize_t perst_reloads_same_image_show(struct device *device,
+struct device_attribute *attr,
+char *buf)
+{
+ struct cxl *adapter = to_cxl_adapter(device);
+
+ return scnprintf(buf, PAGE_SIZE, "%i\n", adapter->perst_same_image);
+}
+
+static ssize_t perst_reloads_same_image_store(struct device *device,
+struct device_attribute *attr,
+const char *buf, size_t count)
+{
+ struct cxl *adapter = to_cxl_adapter(device);
+ int rc;
+ int val;
+
+ rc = sscanf(buf, "%i", &val);
+ if ((rc != 1) || !(val == 1 || val == 0))
+ return -EINVAL;
+
+ adapter->perst_same_image = (val == 1 ? true : false);
+ return count;
+}
+
static struct device_attribute adapter_attrs[] = {
__ATTR_RO(caia_version),
__ATTR_RO(psl_revision),
__ATTR_RO(base_image),
__ATTR_RO(image_loaded),
__ATTR_RW(load_image_on_perst),
+ __ATTR_RW(perst_reloads_same_image),
__ATTR(reset, S_IWUSR, NULL, reset_adapter_store),
};
diff --git a/include/misc/cxl.h b/include/misc/cxl.h
index 7a6c1d6cc173..f2ffe5bd720d 100644
[PATCH v5 10/11] cxl: EEH support
EEH (Enhanced Error Handling) allows a driver to recover from the
temporary failure of an attached PCI card. Enable basic CXL support
for EEH.
Signed-off-by: Daniel Axtens
---
drivers/misc/cxl/cxl.h | 1 +
drivers/misc/cxl/pci.c | 253
drivers/misc/cxl/vphb.c | 8 ++
3 files changed, 262 insertions(+)
diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
index cda02412b01e..6f5386653dae 100644
--- a/drivers/misc/cxl/cxl.h
+++ b/drivers/misc/cxl/cxl.h
@@ -726,6 +726,7 @@ int cxl_psl_purge(struct cxl_afu *afu);
void cxl_stop_trace(struct cxl *cxl);
int cxl_pci_vphb_add(struct cxl_afu *afu);
+void cxl_pci_vphb_reconfigure(struct cxl_afu *afu);
void cxl_pci_vphb_remove(struct cxl_afu *afu);
extern struct pci_driver cxl_pci_driver;
diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
index bfbd6478c0c5..03ddb2d8f974 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -24,6 +24,7 @@
#include
#include "cxl.h"
+#include
#define CXL_PCI_VSEC_ID0x1280
@@ -1252,10 +1253,262 @@ static void cxl_remove(struct pci_dev *dev)
cxl_remove_adapter(adapter);
}
+static pci_ers_result_t cxl_vphb_error_detected(struct cxl_afu *afu,
+ pci_channel_state_t state)
+{
+ struct pci_dev *afu_dev;
+ pci_ers_result_t result = PCI_ERS_RESULT_NEED_RESET;
+ pci_ers_result_t afu_result = PCI_ERS_RESULT_NEED_RESET;
+
+ /* There should only be one entry, but go through the list
+* anyway
+*/
+ list_for_each_entry(afu_dev, &afu->phb->bus->devices, bus_list) {
+ if (!afu_dev->driver)
+ continue;
+
+ afu_dev->error_state = state;
+
+ if (afu_dev->driver->err_handler)
+ afu_result =
afu_dev->driver->err_handler->error_detected(afu_dev,
+
state);
+ /* Disconnect trumps all, NONE trumps NEED_RESET */
+ if (afu_result == PCI_ERS_RESULT_DISCONNECT)
+ result = PCI_ERS_RESULT_DISCONNECT;
+ else if ((afu_result == PCI_ERS_RESULT_NONE) &&
+(result == PCI_ERS_RESULT_NEED_RESET))
+ result = PCI_ERS_RESULT_NONE;
+ }
+ return result;
+}
+
+static pci_ers_result_t cxl_pci_error_detected(struct pci_dev *pdev,
+ pci_channel_state_t state)
+{
+ struct cxl *adapter = pci_get_drvdata(pdev);
+ struct cxl_afu *afu;
+ pci_ers_result_t result = PCI_ERS_RESULT_NEED_RESET;
+ int i;
+
+ /* At this point, we could still have an interrupt pending.
+* Let's try to get them out of the way before they do
+* anything we don't like.
+*/
+ schedule();
+
+ /* If we're permanently dead, give up. */
+ if (state == pci_channel_io_perm_failure) {
+ /* Tell the AFU drivers; but we don't care what they
+* say, we're going away.
+*/
+ for (i = 0; i < adapter->slices; i++) {
+ afu = adapter->afu[i];
+ cxl_vphb_error_detected(afu, state);
+ }
+ return PCI_ERS_RESULT_DISCONNECT;
+ }
+
+ /* Are we reflashing?
+*
+* If we reflash, we could come back as something entirely
+* different, including a non-CAPI card. As such, by default
+* we don't participate in the process. We'll be unbound and
+* the slot re-probed. (TODO: check EEH doesn't blindly rebind
+* us!)
+*
+* However, this isn't the entire story: for reliablity
+* reasons, we usually want to reflash the FPGA on PERST in
+* order to get back to a more reliable known-good state.
+*
+* This causes us a bit of a problem: if we reflash we can't
+* trust that we'll come back the same - we could have a new
+* image and been PERSTed in order to load that
+* image. However, most of the time we actually *will* come
+* back the same - for example a regular EEH event.
+*
+* Therefore, we allow the user to assert that the image is
+* indeed the same and that we should continue on into EEH
+* anyway.
+*/
+ if (adapter->perst_loads_image && !adapter->perst_same_image) {
+ /* TODO take the PHB out of CXL mode */
+ dev_info(&pdev->dev, "reflashing, so opting out of EEH!\n");
+ return PCI_ERS_RESULT_NONE;
+ }
+
+ /*
+* At this point, we want to try to recover. We'll always
+* need a complete slot reset: we don't trust any other reset.
+*
+* Now, we go through each AFU:
+* - We send the driver, if bound, an error_detected callback.
+
[PATCH v5 11/11] cxl: Add CONFIG_CXL_EEH symbol
CONFIG_CXL_EEH is for CXL's EEH related code. Other drivers can depend on or #ifdef on this symbol to configure PERST behaviour, allowing CXL to participate in the EEH process. Reviewed-by: Cyril Bur Signed-off-by: Daniel Axtens --- drivers/misc/cxl/Kconfig | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/misc/cxl/Kconfig b/drivers/misc/cxl/Kconfig index b6db9ebd52c2..c151fc1fe14c 100644 --- a/drivers/misc/cxl/Kconfig +++ b/drivers/misc/cxl/Kconfig @@ -11,11 +11,17 @@ config CXL_KERNEL_API bool default n +config CXL_EEH + bool + default n + select EEH + config CXL tristate "Support for IBM Coherent Accelerators (CXL)" depends on PPC_POWERNV && PCI_MSI select CXL_BASE select CXL_KERNEL_API + select CXL_EEH default m help Select this option to enable driver support for IBM Coherent -- 2.1.4 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v4 06/11] cxl: Refactor adaptor init/teardown
> These seem a bit odd here (though perfectly harmless) - not sure these > need to be done again on recovery (but maybe I'm wrong?) - seems more > like something that should be done early in cxl_init_adapter? > I was really trying to get init to do _only_ things that are once-off operations. As such, I put things in configure that were safe to do more than once, even if they didn't have to be. I have respun with the style changes reverted. > Acked-by: Ian Munsie > -- Regards, Daniel signature.asc Description: This is a digitally signed message part ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v3] powerpc: Add an inline function to update POWER8 HID0
On 08/05/2015 12:38 PM, Gautham R. Shenoy wrote: > Section 3.7 of Version 1.2 of the Power8 Processor User's Manual > prescribes that updates to HID0 be preceded by a SYNC instruction and > followed by an ISYNC instruction (Page 91). > > Create an inline function name update_power8_hid0() which follows this > recipe and invoke it from the static split core path. > > Signed-off-by: Gautham R. Shenoy Reviewed-by: Shreyas B. Prabhu ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [v2,5/5] powerpc/pseries: re-use code from of_helpers module
On Tue, 2015-11-08 at 11:23:09 UTC, Andy Shevchenko wrote:
> The derive_parent() has similar semantics to what we have in newly introduced
> of_helpers module. The replacement reduces code base and propagates the actual
> error code to the caller.
>
> Signed-off-by: Andy Shevchenko
> ---
> arch/powerpc/platforms/pseries/dlpar.c | 31 +--
> 1 file changed, 5 insertions(+), 26 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/dlpar.c
> b/arch/powerpc/platforms/pseries/dlpar.c
> index 47d9cebe..b7f243c 100644
> --- a/arch/powerpc/platforms/pseries/dlpar.c
> +++ b/arch/powerpc/platforms/pseries/dlpar.c
> @@ -18,6 +18,8 @@
> #include
> #include
> #include
> +
> +#include "of_helpers.h"
> #include "offline_states.h"
> #include "pseries.h"
>
> @@ -244,36 +246,13 @@ cc_error:
> return first_dn;
> }
>
> -static struct device_node *derive_parent(const char *path)
> -{
> - struct device_node *parent;
> - char *last_slash;
> -
> - last_slash = strrchr(path, '/');
> - if (last_slash == path) {
> - parent = of_find_node_by_path("/");
> - } else {
> - char *parent_path;
> - int parent_path_len = last_slash - path + 1;
> - parent_path = kmalloc(parent_path_len, GFP_KERNEL);
> - if (!parent_path)
> - return NULL;
> -
> - strlcpy(parent_path, path, parent_path_len);
> - parent = of_find_node_by_path(parent_path);
> - kfree(parent_path);
> - }
> -
> - return parent;
> -}
> -
> int dlpar_attach_node(struct device_node *dn)
> {
> int rc;
>
> - dn->parent = derive_parent(dn->full_name);
> - if (!dn->parent)
> - return -ENOMEM;
> + dn->parent = pseries_of_derive_parent(dn->full_name);
> + if (IS_ERR(dn->parent))
> + return PTR_ERR(dn_parent);
^
?
There are cross compilers on kernel.org, or on Ubuntu you can just:
$ apt-get install gcc-powerpc-linux-gnu
$ make ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu-gcc
cheers
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
PCI not working in 4.1 for p2010rdb(Freescale) based board
I upgraded our kernel for our custom p2010rdb based board from 3.4 to 4.1 and I cannot get the PCI controller to work, it is not initialized as can be seem by the below dmesg fro 3.4 and 4.1. Sorry to say, PCI is not my cup of tea so I don't not know how to find the culprit. I have checked what I can but nothing stands out and I would appreciate some help/pointers dmesg for 4.1: [0.00] Using P2010 RDB machine description [0.00] Memory CAM mapping: 256/256 Mb, residual: 0Mb [0.00] Linux version 4.1.0+ (jocke@gentoo-jocke) (gcc version 4.5.3 (Gentoo 4.5.3-r2 p1.6, pie-0.4.7) ) #102 Thu Aug 13 21:23:04 CEST 2015 [0.00] Found legacy serial port 0 for /soc@ff70/serial@4500 [0.00] mem=ff704500, taddr=ff704500, irq=0, clk=6, speed=0 [0.00] Found legacy serial port 1 for /soc@ff70/serial@4600 [0.00] mem=ff704600, taddr=ff704600, irq=0, clk=6, speed=0 [0.00] bootconsole [udbg0] enabled [0.00] E500 platform from Transmode Systems AB [0.00] Top of RAM: 0x2000, Total RAM: 0x2000 [0.00] Memory hole size: 0MB [0.00] Zone ranges: [0.00] DMA [mem 0x-0x1fff] [0.00] Normal empty [0.00] Movable zone start for each node [0.00] Early memory node ranges [0.00] node 0: [mem 0x-0x1fff] [0.00] Initmem setup node 0 [mem 0x-0x1fff] [0.00] On node 0 totalpages: 131072 [0.00] free_area_init_node: node 0, pgdat c03c8b10, node_mem_map df5ec000 [0.00] DMA zone: 1024 pages used for memmap [0.00] DMA zone: 0 pages reserved [0.00] DMA zone: 131072 pages, LIFO batch:31 [0.00] MMU: Allocated 1088 bytes of context maps for 255 contexts [0.00] pcpu-alloc: s0 r0 d32768 u32768 alloc=1*32768 [0.00] pcpu-alloc: [0] 0 [0.00] Built 1 zonelists in Zone order, mobility grouping on. Total pages: 130048 [0.00] Kernel command line: root=/dev/mtdblock6 rw rootfstype=jffs2 ip=172.18.2.61:192.168.201.63:172.18.0.1:255.255.0.0:P2020RDB:eth0:off console=ttyS0,115200 [0.00] PID hash table entries: 2048 (order: 1, 8192 bytes) [0.00] Dentry cache hash table entries: 65536 (order: 6, 262144 bytes) [0.00] Inode-cache hash table entries: 32768 (order: 5, 131072 bytes) [0.00] Sorting __ex_table... [0.00] Memory: 443652K/524288K available (2984K kernel code, 108K rwdata, 664K rodata, 152K init, 126K bss, 80636K reserved, 0K cma-reserved) [0.00] Kernel virtual memory layout: [0.00] * 0xfffdf000..0xf000 : fixmap [0.00] * 0xfdffe000..0xfe00 : early ioremap [0.00] * 0xe100..0xfdffe000 : vmalloc & ioremap [0.00] NR_IRQS:128 nr_irqs:128 16 [0.00] mpic: Resetting [0.00] mpic: Setting up MPIC " OpenPIC " version 1.2 at ff74, max 1 CPUs [0.00] mpic: ISU size: 256, shift: 8, mask: ff [0.00] mpic: Initializing for 256 sources [0.00] time_init: decrementer frequency = 75.00 MHz [0.00] time_init: processor frequency = 1200.00 MHz [0.07] clocksource timebase: mask: 0x max_cycles: 0x114c1bade8, max_idle_ns: 440795203839 ns [0.010184] clocksource: timebase mult[d55] shift[24] registered [0.016531] clockevent: decrementer mult[1333] shift[32] cpu[0] [0.016616] pid_max: default: 32768 minimum: 301 [0.021202] Mount-cache hash table entries: 1024 (order: 0, 4096 bytes) [0.027754] Mountpoint-cache hash table entries: 1024 (order: 0, 4096 bytes) [0.036855] clocksource jiffies: mask: 0x max_cycles: 0x, max_idle_ns: 764504178510 ns [0.046775] NET: Registered protocol family 16 [0.055827] spidev registred. [0.064391] PCI: Probing PCI hardware [0.067979] PCI: Assigning unassigned resources... [0.068068] Registering qe_ic with sysfs... [0.071912] pps_core: LinuxPPS API ver. 1 registered [0.076804] pps_core: Software ver. 5.3.6 - Copyright 2005-2007 Rodolfo Giometti [0.086152] PTP clock support registered [0.090408] Switched to clocksource timebase [0.095368] NET: Registered protocol family 2 [0.100202] TCP established hash table entries: 4096 (order: 2, 16384 bytes) [0.107199] TCP bind hash table entries: 4096 (order: 2, 16384 bytes) [0.113626] TCP: Hash tables configured (established 4096 bind 4096) [0.120013] UDP hash table entries: 256 (order: 0, 4096 bytes) [0.125781] UDP-Lite hash table entries: 256 (order: 0, 4096 bytes) [0.132109] NET: Registered protocol family 1 [0.136405] PCI: CLS 0 bytes, default 32 [0.141059] Setting up Freescale MSI support [0.141551] Adding PCI host bridge /pcie@ff70a000 [0.141561] PCI memory map start 0xff70a000, size 0x1000 [0.141578] Found FSL PCI
Re: [PATCH v6 20/42] powerpc/powernv: Create PEs dynamically
On 08/06/2015 02:11 PM, Gavin Shan wrote:
Currently, the PEs and their associated resources are assigned
in ppc_md.pcibios_fixup() except those consumed by SRIOV VFs.
The function is called for once after PCI probing and resources
assignment is finished which isn't hotplug friendly.
The patch creates PEs dynamically by ppc_md.pcibios_setup_bridge(),
which is called on the event during system bootup and PCI hotplug:
updating PCI bridge's windows after resource assignment/reassignment
are finished. For partial hotplug case, where not all PCI devices
belonging to the PE are unplugged and plugged again, we just need
unbinding/binding the affected PCI devices with the corresponding
PE without creating new one.
Besides, it might require additional resources (e.g. M32) to the
windows of the PCI bridge when unplugging current adapter, and
insert a different adapter if there is one PCI slot, which is
assumed behind root port, or the downstream bridge of the PCIE
switch behind root port. The parent bridge of the newly plugged
adapter would reject the request to add more resources, leading
to hotplug failure. For the issue, the patch extends the windows
of root port, or the upstream port of the PCIe switch behind root
port to PHB's windows when ppc_md.pcibios_setup_bridge() is called.
There is no upstream bridge for root bus, so we have to fix it up
before any PE is created because the root bus PE is the ancestor
to anyone else.
Signed-off-by: Gavin Shan
---
arch/powerpc/platforms/powernv/pci-ioda.c | 226 ++
arch/powerpc/platforms/powernv/pci.h | 1 +
2 files changed, 137 insertions(+), 90 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c
b/arch/powerpc/platforms/powernv/pci-ioda.c
index 8aa6ab8..37847a3 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1083,6 +1083,13 @@ static void pnv_ioda_setup_same_PE(struct pci_bus *bus,
struct pnv_ioda_pe *pe)
pci_name(dev));
continue;
}
+
+ /* The PCI device might be not detached from the
+* PE in partial hotplug case.
+*/
+ if (pdn->pe_number != IODA_INVALID_PE)
+ continue;
+
pdn->pe_number = pe->pe_number;
pe->dma32_weight += pnv_ioda_dma_weight(dev);
if ((pe->flags & PNV_IODA_PE_BUS_ALL) && dev->subordinate)
@@ -1101,9 +1108,27 @@ static struct pnv_ioda_pe *pnv_ioda_setup_bus_PE(struct
pci_bus *bus, bool all)
struct pci_controller *hose = pci_bus_to_host(bus);
struct pnv_phb *phb = hose->private_data;
struct pnv_ioda_pe *pe = NULL;
+ int pe_num;
+
+ /* For partial hotplug case, the PE instance hasn't been destroyed
+* yet. We shouldn't allocated a new one and assign resources to
+* it. The existing PE instance should be reused, but we should
+* associate the devices to the PE.
+*/
+ pe_num = phb->ioda.pe_rmap[bus->number << 8];
+ if (pe_num != IODA_INVALID_PE) {
+ pe = &phb->ioda.pe_array[pe_num];
+ pnv_ioda_setup_same_PE(bus, pe);
+ return NULL;
+ }
+
+ /* PE number for root bus should have been reserved */
+ if (pci_is_root_bus(bus) &&
+ phb->ioda.root_pe_idx != IODA_INVALID_PE)
+ pe = &phb->ioda.pe_array[phb->ioda.root_pe_idx];
/* Check if PE is determined by M64 */
- if (phb->pick_m64_pe)
+ if (!pe && phb->pick_m64_pe)
else if (phb->pick_m64_pe)
pe = phb->pick_m64_pe(bus, all);
/* The PE number isn't pinned by M64 */
@@ -1150,46 +1175,6 @@ static struct pnv_ioda_pe *pnv_ioda_setup_bus_PE(struct
pci_bus *bus, bool all)
return pe;
}
-static void pnv_ioda_setup_PEs(struct pci_bus *bus)
-{
- struct pci_dev *dev;
-
- pnv_ioda_setup_bus_PE(bus, false);
-
- list_for_each_entry(dev, &bus->devices, bus_list) {
- if (dev->subordinate) {
- if (pci_pcie_type(dev) == PCI_EXP_TYPE_PCI_BRIDGE)
- pnv_ioda_setup_bus_PE(dev->subordinate, true);
- else
- pnv_ioda_setup_PEs(dev->subordinate);
- }
- }
-}
-
-/*
- * Configure PEs so that the downstream PCI buses and devices
- * could have their associated PE#. Unfortunately, we didn't
- * figure out the way to identify the PLX bridge yet. So we
- * simply put the PCI bus and the subordinate behind the root
- * port to PE# here. The game rule here is expected to be changed
- * as soon as we can detected PLX bridge correctly.
- */
-static void pnv_pci_ioda_setup_PEs(void)
-{
- struct pci_controller *hose, *tmp;
- struct pnv_phb *phb;
-
- list_for_each_entry_safe(hose, tmp, &hose_list, list_node) {
- phb = hose->p
Re: [PATCH 2/3] powerpc: PCI: Fix lookup of linux,pci-probe-only property
Hi Marc,
On Fri, Aug 14, 2015 at 03:41:07PM +0100, Marc Zyngier wrote:
> When find_and_init_phbs() looks for the probe-only property, it seems
> to trust the firmware to be correctly written, and assumes that there
> is a parameter to the property.
>
> It is conceivable that the firmware could not be that perfect, and it
> could expose this property naked (at least one arm64 platform seems to
> exhibit this exact behaviour). The setup code the ends up making
> a decision based on whatever the property pointer points to, which
> is likely to be junk.
>
> Instead, let's check for the validity of the property, and ignore
> it if the firmware couldn't make up its mind.
>
> Signed-off-by: Marc Zyngier
> ---
> arch/powerpc/platforms/pseries/setup.c | 15 ++-
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/setup.c
> b/arch/powerpc/platforms/pseries/setup.c
> index df6a704..6bdc1f9 100644
> --- a/arch/powerpc/platforms/pseries/setup.c
> +++ b/arch/powerpc/platforms/pseries/setup.c
> @@ -490,14 +490,19 @@ static void __init find_and_init_phbs(void)
>*/
> if (of_chosen) {
> const int *prop;
> + int len;
>
> prop = of_get_property(of_chosen,
> - "linux,pci-probe-only", NULL);
> + "linux,pci-probe-only", &len);
> if (prop) {
> - if (*prop)
> - pci_add_flags(PCI_PROBE_ONLY);
> - else
> - pci_clear_flags(PCI_PROBE_ONLY);
> + if (len) {
> + if (be32_to_cpup(prop))
> + pci_add_flags(PCI_PROBE_ONLY);
> + else
> + pci_clear_flags(PCI_PROBE_ONLY);
> + } else {
> + pr_warn("linux,pci-probe-only set without
> value, ignoring\n");
> + }
This seems essentially identical to the pci-host-generic version.
Is there a way we can factor it out so there's only one copy?
> }
> }
> }
> --
> 2.1.4
>
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 1/3] PCI: pci-host-generic: Fix lookup of linux, pci-probe-only property
When pci-host-generic looks for the probe-only property, it seems
to trust the DT to be correctly written, and assumes that there
is a parameter to the property.
Unfortunately, this is not always the case, and some firmware expose
this property naked. The driver ends up making a decision based on
whatever the property pointer points to, which is likely to be junk.
Instead, let's check for the validity of the property, and ignore
it if the firmware couldn't make up its mind.
Signed-off-by: Marc Zyngier
---
drivers/pci/host/pci-host-generic.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/host/pci-host-generic.c
b/drivers/pci/host/pci-host-generic.c
index 265dd25..2b2e2ff 100644
--- a/drivers/pci/host/pci-host-generic.c
+++ b/drivers/pci/host/pci-host-generic.c
@@ -211,6 +211,7 @@ static int gen_pci_probe(struct platform_device *pdev)
const char *type;
const struct of_device_id *of_id;
const int *prop;
+ int len;
struct device *dev = &pdev->dev;
struct device_node *np = dev->of_node;
struct gen_pci *pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
@@ -225,12 +226,16 @@ static int gen_pci_probe(struct platform_device *pdev)
return -EINVAL;
}
- prop = of_get_property(of_chosen, "linux,pci-probe-only", NULL);
+ prop = of_get_property(of_chosen, "linux,pci-probe-only", &len);
if (prop) {
- if (*prop)
- pci_add_flags(PCI_PROBE_ONLY);
- else
- pci_clear_flags(PCI_PROBE_ONLY);
+ if (len) {
+ if (be32_to_cpup(prop))
+ pci_add_flags(PCI_PROBE_ONLY);
+ else
+ pci_clear_flags(PCI_PROBE_ONLY);
+ } else {
+ dev_warn(&pdev->dev, "linux,pci-probe-only set without
value, ignoring\n");
+ }
}
of_id = of_match_node(gen_pci_of_match, np);
--
2.1.4
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 0/3] PCI: arm64/powerpc: Fix parsing of linux,pci-probe-only
The pci-host-generic driver parses the linux,pci-probe-only property, and assumes that it will have a boolean parameter. Turns out that the Seattle DTS file has a naked "linux,pci-probe-only" property, which leads to the driver dereferencing some unsuspecting memory location. Nothing really bad happens (we end up reading some other bit of DT, fortunately), but that not a reason to keep it this way. The first patch fixes the driver not to do silly things, and simply give a warning when this happens. The powerpc code from where this code was lifted is also fixed in a second patch. Finally, the bad property is removed from the Seatle DTS, because it is simply not necessary (it actually prevents me from using SR-IOV, which otherwise runs fine without the probe-only thing). Marc Zyngier (3): PCI: pci-host-generic: Fix lookup of linux,pci-probe-only property powerpc: PCI: Fix lookup of linux,pci-probe-only property arm64: dts: Drop linux,pci-probe-only from the Seattle DTS arch/arm64/boot/dts/amd/amd-overdrive.dts | 1 - arch/powerpc/platforms/pseries/setup.c| 15 ++- drivers/pci/host/pci-host-generic.c | 15 ++- 3 files changed, 20 insertions(+), 11 deletions(-) -- 2.1.4 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 2/3] powerpc: PCI: Fix lookup of linux, pci-probe-only property
When find_and_init_phbs() looks for the probe-only property, it seems
to trust the firmware to be correctly written, and assumes that there
is a parameter to the property.
It is conceivable that the firmware could not be that perfect, and it
could expose this property naked (at least one arm64 platform seems to
exhibit this exact behaviour). The setup code the ends up making
a decision based on whatever the property pointer points to, which
is likely to be junk.
Instead, let's check for the validity of the property, and ignore
it if the firmware couldn't make up its mind.
Signed-off-by: Marc Zyngier
---
arch/powerpc/platforms/pseries/setup.c | 15 ++-
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/setup.c
b/arch/powerpc/platforms/pseries/setup.c
index df6a704..6bdc1f9 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -490,14 +490,19 @@ static void __init find_and_init_phbs(void)
*/
if (of_chosen) {
const int *prop;
+ int len;
prop = of_get_property(of_chosen,
- "linux,pci-probe-only", NULL);
+ "linux,pci-probe-only", &len);
if (prop) {
- if (*prop)
- pci_add_flags(PCI_PROBE_ONLY);
- else
- pci_clear_flags(PCI_PROBE_ONLY);
+ if (len) {
+ if (be32_to_cpup(prop))
+ pci_add_flags(PCI_PROBE_ONLY);
+ else
+ pci_clear_flags(PCI_PROBE_ONLY);
+ } else {
+ pr_warn("linux,pci-probe-only set without
value, ignoring\n");
+ }
}
}
}
--
2.1.4
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 3/3] arm64: dts: Drop linux, pci-probe-only from the Seattle DTS
The linux,pci-probe-only property mandates an argument to indicate
whether or not to engage the "probe-only" mode, but the Seattle
DTS just provides a naked property, which is illegal.
Also, it turns out that the board is perfectly happy without
probe-only, so let's drop this altogether.
Signed-off-by: Marc Zyngier
---
arch/arm64/boot/dts/amd/amd-overdrive.dts | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/arm64/boot/dts/amd/amd-overdrive.dts
b/arch/arm64/boot/dts/amd/amd-overdrive.dts
index 564a3f7..128fa94 100644
--- a/arch/arm64/boot/dts/amd/amd-overdrive.dts
+++ b/arch/arm64/boot/dts/amd/amd-overdrive.dts
@@ -14,7 +14,6 @@
chosen {
stdout-path = &serial0;
- linux,pci-probe-only;
};
};
--
2.1.4
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/3] powerpc: PCI: Fix lookup of linux,pci-probe-only property
Hi Bjorn,
On 14/08/15 15:57, Bjorn Helgaas wrote:
> Hi Marc,
>
> On Fri, Aug 14, 2015 at 03:41:07PM +0100, Marc Zyngier wrote:
>> When find_and_init_phbs() looks for the probe-only property, it seems
>> to trust the firmware to be correctly written, and assumes that there
>> is a parameter to the property.
>>
>> It is conceivable that the firmware could not be that perfect, and it
>> could expose this property naked (at least one arm64 platform seems to
>> exhibit this exact behaviour). The setup code the ends up making
>> a decision based on whatever the property pointer points to, which
>> is likely to be junk.
>>
>> Instead, let's check for the validity of the property, and ignore
>> it if the firmware couldn't make up its mind.
>>
>> Signed-off-by: Marc Zyngier
>> ---
>> arch/powerpc/platforms/pseries/setup.c | 15 ++-
>> 1 file changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/pseries/setup.c
>> b/arch/powerpc/platforms/pseries/setup.c
>> index df6a704..6bdc1f9 100644
>> --- a/arch/powerpc/platforms/pseries/setup.c
>> +++ b/arch/powerpc/platforms/pseries/setup.c
>> @@ -490,14 +490,19 @@ static void __init find_and_init_phbs(void)
>> */
>> if (of_chosen) {
>> const int *prop;
>> +int len;
>>
>> prop = of_get_property(of_chosen,
>> -"linux,pci-probe-only", NULL);
>> +"linux,pci-probe-only", &len);
>> if (prop) {
>> -if (*prop)
>> -pci_add_flags(PCI_PROBE_ONLY);
>> -else
>> -pci_clear_flags(PCI_PROBE_ONLY);
>> +if (len) {
>> +if (be32_to_cpup(prop))
>> +pci_add_flags(PCI_PROBE_ONLY);
>> +else
>> +pci_clear_flags(PCI_PROBE_ONLY);
>> +} else {
>> +pr_warn("linux,pci-probe-only set without
>> value, ignoring\n");
>> +}
>
> This seems essentially identical to the pci-host-generic version.
> Is there a way we can factor it out so there's only one copy?
Probably. drivers/of/of_pci.c seems like a good landing place for it.
I'll hack something and repost it.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 29/31] parisc: handle page-less SG entries
On Thu, Aug 13, 2015 at 9:11 PM, David Miller wrote: > From: James Bottomley >> At least on some PA architectures, you have to be very careful. >> Improperly managed, multiple aliases will cause the system to crash >> (actually a machine check in the cache chequerboard). For the most >> temperamental systems, we need the cache line flushed and the alias >> mapping ejected from the TLB cache before we access the same page at an >> inequivalent alias. > > Also, I want to mention that on sparc64 we manage the cache aliasing > state in the page struct. > > Until a page is mapped into userspace, we just record the most recent > cpu to store into that page with kernel side mappings. Once the page > ends up being mapped or the cpu doing kernel side stores changes, we > actually perform the cache flush. > > Generally speaking, I think that all actual physical memory the kernel > operates on should have a struct page backing it. So this whole > discussion of operating on physical memory in scatter lists without > backing page structs feels really foreign to me. So the only way for page-less pfns to enter the system is through the ->direct_access() method provided by a pmem device's struct block_device_operations. Architectures that require struct page for cache management to must disable ->direct_access() in this case. If an arch still wants to support pmem+DAX then it needs something like this patchset (feedback welcome) to map pmem pfns: https://lkml.org/lkml/2015/8/12/970 Effectively this would disable ->direct_access() on /dev/pmem0, but permit ->direct_access() on /dev/pmem0m. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v2 1/4] of/pci: Add of_pci_check_probe_only to parse "linux, pci-probe-only"
Both pci-host-generic and Pseries parse the "linux,pci-probe-only"
to engage the PCI_PROBE_ONLY mode, and both have a subtle bug that
can be triggered if the property has no parameter.
Provide a generic implementation that can be used by both.
Signed-off-by: Marc Zyngier
---
drivers/of/of_pci.c| 30 ++
include/linux/of_pci.h | 3 +++
2 files changed, 33 insertions(+)
diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
index 5751dc5..a4e29ff 100644
--- a/drivers/of/of_pci.c
+++ b/drivers/of/of_pci.c
@@ -118,6 +118,36 @@ int of_get_pci_domain_nr(struct device_node *node)
EXPORT_SYMBOL_GPL(of_get_pci_domain_nr);
/**
+ * of_pci_check_probe_only - Setup probe only mode if linux,pci-probe-only
+ * is present and valid
+ *
+ * @node: device tree node that may contain the property (usually "chosen")
+ *
+ */
+void of_pci_check_probe_only(struct device_node *node)
+{
+ const int *prop;
+ int len;
+
+ if (!node)
+ return;
+
+ prop = of_get_property(node, "linux,pci-probe-only", &len);
+ if (prop) {
+ if (!len) {
+ pr_warn("linux,pci-probe-only set without value,
ignoring\n");
+ return;
+ }
+
+ if (be32_to_cpup(prop))
+ pci_add_flags(PCI_PROBE_ONLY);
+ else
+ pci_clear_flags(PCI_PROBE_ONLY);
+ }
+}
+EXPORT_SYMBOL_GPL(of_pci_check_probe_only);
+
+/**
* of_pci_dma_configure - Setup DMA configuration
* @dev: ptr to pci_dev struct of the PCI device
*
diff --git a/include/linux/of_pci.h b/include/linux/of_pci.h
index 29fd3fe..4c0a617 100644
--- a/include/linux/of_pci.h
+++ b/include/linux/of_pci.h
@@ -17,6 +17,7 @@ int of_irq_parse_and_map_pci(const struct pci_dev *dev, u8
slot, u8 pin);
int of_pci_parse_bus_range(struct device_node *node, struct resource *res);
int of_get_pci_domain_nr(struct device_node *node);
void of_pci_dma_configure(struct pci_dev *pci_dev);
+void of_pci_check_probe_only(struct device_node *node);
#else
static inline int of_irq_parse_pci(const struct pci_dev *pdev, struct
of_phandle_args *out_irq)
{
@@ -53,6 +54,8 @@ of_get_pci_domain_nr(struct device_node *node)
}
static inline void of_pci_dma_configure(struct pci_dev *pci_dev) { }
+
+static inline void of_pci_check_probe_only(struct device_node *node) { }
#endif
#if defined(CONFIG_OF_ADDRESS)
--
2.1.4
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v2 0/4] PCI: arm64/powerpc: Fix parsing of linux, pci-probe-only
The pci-host-generic driver parses the linux,pci-probe-only property, and assumes that it will have a boolean parameter. Turns out that the Seattle DTS file has a naked "linux,pci-probe-only" property, which leads to the driver dereferencing some unsuspecting memory location. Nothing really bad happens (we end up reading some other bit of DT, fortunately), but that not a reason to keep it this way. Turns out that the Pseries code (where this code was lifted from) may suffer from the same issue. The first patch introduces a common (and fixed) version of that check that can be used by drivers and architectures that require it. The two following patches change the pci-host-generic driver and the powerpc code to use it. Finally, the bad property is removed from the Seatle DTS, because it is simply not necessary (it actually prevents me from using SR-IOV, which otherwise runs fine without the probe-only thing). This has been tested on the offending Seattle board. * From v1: - Consolidate the parsing in of_pci.c (Bjorn) Marc Zyngier (4): of/pci: Add of_pci_check_probe_only to parse "linux,pci-probe-only" PCI: pci-host-generic: Fix lookup of linux,pci-probe-only property powerpc: PCI: Fix lookup of linux,pci-probe-only property arm64: dts: Drop linux,pci-probe-only from the Seattle DTS arch/arm64/boot/dts/amd/amd-overdrive.dts | 1 - arch/powerpc/platforms/pseries/setup.c| 14 ++ drivers/of/of_pci.c | 30 ++ drivers/pci/host/pci-host-generic.c | 9 + include/linux/of_pci.h| 3 +++ 5 files changed, 36 insertions(+), 21 deletions(-) -- 2.1.4 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v2 2/4] PCI: pci-host-generic: Fix lookup of linux, pci-probe-only property
When pci-host-generic looks for the probe-only property, it seems
to trust the DT to be correctly written, and assumes that there
is a parameter to the property.
Unfortunately, this is not always the case, and some firmware expose
this property naked. The driver ends up making a decision based on
whatever the property pointer points to, which is likely to be junk.
Switch to the common of_pci.c implementation that doesn't suffer
from this problem.
Signed-off-by: Marc Zyngier
---
drivers/pci/host/pci-host-generic.c | 9 +
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/pci/host/pci-host-generic.c
b/drivers/pci/host/pci-host-generic.c
index 265dd25..545ff4e 100644
--- a/drivers/pci/host/pci-host-generic.c
+++ b/drivers/pci/host/pci-host-generic.c
@@ -210,7 +210,6 @@ static int gen_pci_probe(struct platform_device *pdev)
int err;
const char *type;
const struct of_device_id *of_id;
- const int *prop;
struct device *dev = &pdev->dev;
struct device_node *np = dev->of_node;
struct gen_pci *pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
@@ -225,13 +224,7 @@ static int gen_pci_probe(struct platform_device *pdev)
return -EINVAL;
}
- prop = of_get_property(of_chosen, "linux,pci-probe-only", NULL);
- if (prop) {
- if (*prop)
- pci_add_flags(PCI_PROBE_ONLY);
- else
- pci_clear_flags(PCI_PROBE_ONLY);
- }
+ of_pci_check_probe_only(of_chosen);
of_id = of_match_node(gen_pci_of_match, np);
pci->cfg.ops = of_id->data;
--
2.1.4
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v2 3/4] powerpc: PCI: Fix lookup of linux, pci-probe-only property
When find_and_init_phbs() looks for the probe-only property, it seems
to trust the firmware to be correctly written, and assumes that there
is a parameter to the property.
It is conceivable that the firmware could not be that perfect, and it
could expose this property naked (at least one arm64 platform seems to
exhibit this exact behaviour). The setup code the ends up making
a decision based on whatever the property pointer points to, which
is likely to be junk.
Instead, switch to the common of_pci.c implementation that doesn't
suffer from this problem and ignore the property if the firmware
couldn't make up its mind.
Signed-off-by: Marc Zyngier
---
arch/powerpc/platforms/pseries/setup.c | 14 ++
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/setup.c
b/arch/powerpc/platforms/pseries/setup.c
index df6a704..8434953 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -40,6 +40,7 @@
#include
#include
#include
+#include
#include
#include
@@ -488,18 +489,7 @@ static void __init find_and_init_phbs(void)
* PCI_PROBE_ONLY and PCI_REASSIGN_ALL_BUS can be set via properties
* in chosen.
*/
- if (of_chosen) {
- const int *prop;
-
- prop = of_get_property(of_chosen,
- "linux,pci-probe-only", NULL);
- if (prop) {
- if (*prop)
- pci_add_flags(PCI_PROBE_ONLY);
- else
- pci_clear_flags(PCI_PROBE_ONLY);
- }
- }
+ of_pci_check_probe_only(of_chosen);
}
static void __init pSeries_setup_arch(void)
--
2.1.4
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v2 4/4] arm64: dts: Drop linux, pci-probe-only from the Seattle DTS
The linux,pci-probe-only property mandates an argument to indicate
whether or not to engage the "probe-only" mode, but the Seattle
DTS just provides a naked property, which is illegal.
Also, it turns out that the board is perfectly happy without
probe-only, so let's drop this altogether.
Signed-off-by: Marc Zyngier
---
arch/arm64/boot/dts/amd/amd-overdrive.dts | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/arm64/boot/dts/amd/amd-overdrive.dts
b/arch/arm64/boot/dts/amd/amd-overdrive.dts
index 564a3f7..128fa94 100644
--- a/arch/arm64/boot/dts/amd/amd-overdrive.dts
+++ b/arch/arm64/boot/dts/amd/amd-overdrive.dts
@@ -14,7 +14,6 @@
chosen {
stdout-path = &serial0;
- linux,pci-probe-only;
};
};
--
2.1.4
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: PCI not working in 4.1 for p2010rdb(Freescale) based board
On Fri, 2015-08-14 at 13:15 +, Joakim Tjernlund wrote: > I upgraded our kernel for our custom p2010rdb based board from 3.4 to 4.1 and > I cannot get the > PCI controller to work, it is not initialized as can be seem by the below > dmesg fro 3.4 and 4.1. > Sorry to say, PCI is not my cup of tea so I don't not know how to find the > culprit. > I have checked what I can but nothing stands out and I would appreciate some > help/pointers Naturally I found the problem shortly after sending this :) Stupid copy&paste error ... Jocke ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 2/4] PCI: pci-host-generic: Fix lookup of linux,pci-probe-only property
On Fri, Aug 14, 2015 at 05:19:17PM +0100, Marc Zyngier wrote:
> When pci-host-generic looks for the probe-only property, it seems
> to trust the DT to be correctly written, and assumes that there
> is a parameter to the property.
>
> Unfortunately, this is not always the case, and some firmware expose
> this property naked. The driver ends up making a decision based on
> whatever the property pointer points to, which is likely to be junk.
>
> Switch to the common of_pci.c implementation that doesn't suffer
> from this problem.
>
> Signed-off-by: Marc Zyngier
> ---
> drivers/pci/host/pci-host-generic.c | 9 +
> 1 file changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/drivers/pci/host/pci-host-generic.c
> b/drivers/pci/host/pci-host-generic.c
> index 265dd25..545ff4e 100644
> --- a/drivers/pci/host/pci-host-generic.c
> +++ b/drivers/pci/host/pci-host-generic.c
> @@ -210,7 +210,6 @@ static int gen_pci_probe(struct platform_device *pdev)
> int err;
> const char *type;
> const struct of_device_id *of_id;
> - const int *prop;
> struct device *dev = &pdev->dev;
> struct device_node *np = dev->of_node;
> struct gen_pci *pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
> @@ -225,13 +224,7 @@ static int gen_pci_probe(struct platform_device *pdev)
> return -EINVAL;
> }
>
> - prop = of_get_property(of_chosen, "linux,pci-probe-only", NULL);
> - if (prop) {
> - if (*prop)
> - pci_add_flags(PCI_PROBE_ONLY);
> - else
> - pci_clear_flags(PCI_PROBE_ONLY);
> - }
> + of_pci_check_probe_only(of_chosen);
Do we need support for pci-probe-only in pci-host-generic at all?
You're removing the use in amd-overdrive.dts, and there are no other
DTs in the kernel tree that mention it.
If we can live without it, that would be nice. It seems like a relic from
days when we couldn't reliably assign resources. (I'm not saying we can do
that reliably even today, but I'd rather make it reliable than turn it
off.)
> of_id = of_match_node(gen_pci_of_match, np);
> pci->cfg.ops = of_id->data;
> --
> 2.1.4
>
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 2/4] PCI: pci-host-generic: Fix lookup of linux,pci-probe-only property
On Fri, Aug 14, 2015 at 05:40:51PM +0100, Bjorn Helgaas wrote:
> On Fri, Aug 14, 2015 at 05:19:17PM +0100, Marc Zyngier wrote:
> > When pci-host-generic looks for the probe-only property, it seems
> > to trust the DT to be correctly written, and assumes that there
> > is a parameter to the property.
> >
> > Unfortunately, this is not always the case, and some firmware expose
> > this property naked. The driver ends up making a decision based on
> > whatever the property pointer points to, which is likely to be junk.
> >
> > Switch to the common of_pci.c implementation that doesn't suffer
> > from this problem.
> >
> > Signed-off-by: Marc Zyngier
> > ---
> > drivers/pci/host/pci-host-generic.c | 9 +
> > 1 file changed, 1 insertion(+), 8 deletions(-)
> >
> > diff --git a/drivers/pci/host/pci-host-generic.c
> > b/drivers/pci/host/pci-host-generic.c
> > index 265dd25..545ff4e 100644
> > --- a/drivers/pci/host/pci-host-generic.c
> > +++ b/drivers/pci/host/pci-host-generic.c
> > @@ -210,7 +210,6 @@ static int gen_pci_probe(struct platform_device *pdev)
> > int err;
> > const char *type;
> > const struct of_device_id *of_id;
> > - const int *prop;
> > struct device *dev = &pdev->dev;
> > struct device_node *np = dev->of_node;
> > struct gen_pci *pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
> > @@ -225,13 +224,7 @@ static int gen_pci_probe(struct platform_device *pdev)
> > return -EINVAL;
> > }
> >
> > - prop = of_get_property(of_chosen, "linux,pci-probe-only", NULL);
> > - if (prop) {
> > - if (*prop)
> > - pci_add_flags(PCI_PROBE_ONLY);
> > - else
> > - pci_clear_flags(PCI_PROBE_ONLY);
> > - }
> > + of_pci_check_probe_only(of_chosen);
>
> Do we need support for pci-probe-only in pci-host-generic at all?
> You're removing the use in amd-overdrive.dts, and there are no other
> DTs in the kernel tree that mention it.
>
> If we can live without it, that would be nice. It seems like a relic from
> days when we couldn't reliably assign resources. (I'm not saying we can do
> that reliably even today, but I'd rather make it reliable than turn it
> off.)
Kvmtool certainly uses it (and generates its own DT, hence why you don't
see it in mainline). Not sure about qemu, though.
Will
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 2/4] PCI: pci-host-generic: Fix lookup of linux,pci-probe-only property
On Fri, Aug 14, 2015 at 05:19:17PM +0100, Marc Zyngier wrote:
> When pci-host-generic looks for the probe-only property, it seems
> to trust the DT to be correctly written, and assumes that there
> is a parameter to the property.
>
> Unfortunately, this is not always the case, and some firmware expose
> this property naked. The driver ends up making a decision based on
> whatever the property pointer points to, which is likely to be junk.
>
> Switch to the common of_pci.c implementation that doesn't suffer
> from this problem.
>
> Signed-off-by: Marc Zyngier
> ---
> drivers/pci/host/pci-host-generic.c | 9 +
> 1 file changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/drivers/pci/host/pci-host-generic.c
> b/drivers/pci/host/pci-host-generic.c
> index 265dd25..545ff4e 100644
> --- a/drivers/pci/host/pci-host-generic.c
> +++ b/drivers/pci/host/pci-host-generic.c
> @@ -210,7 +210,6 @@ static int gen_pci_probe(struct platform_device *pdev)
> int err;
> const char *type;
> const struct of_device_id *of_id;
> - const int *prop;
> struct device *dev = &pdev->dev;
> struct device_node *np = dev->of_node;
> struct gen_pci *pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
> @@ -225,13 +224,7 @@ static int gen_pci_probe(struct platform_device *pdev)
> return -EINVAL;
> }
>
> - prop = of_get_property(of_chosen, "linux,pci-probe-only", NULL);
> - if (prop) {
> - if (*prop)
> - pci_add_flags(PCI_PROBE_ONLY);
> - else
> - pci_clear_flags(PCI_PROBE_ONLY);
> - }
> + of_pci_check_probe_only(of_chosen);
You could probably just make this take void, as the probe-only property
is always in the /chosen node.
Either way:
Acked-by: Will Deacon
Will
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 2/4] PCI: pci-host-generic: Fix lookup of linux, pci-probe-only property
> On 14.08.2015, at 09:43, Will Deacon wrote:
>
> On Fri, Aug 14, 2015 at 05:40:51PM +0100, Bjorn Helgaas wrote:
>> On Fri, Aug 14, 2015 at 05:19:17PM +0100, Marc Zyngier wrote:
>>> When pci-host-generic looks for the probe-only property, it seems
>>> to trust the DT to be correctly written, and assumes that there
>>> is a parameter to the property.
>>>
>>> Unfortunately, this is not always the case, and some firmware expose
>>> this property naked. The driver ends up making a decision based on
>>> whatever the property pointer points to, which is likely to be junk.
>>>
>>> Switch to the common of_pci.c implementation that doesn't suffer
>>> from this problem.
>>>
>>> Signed-off-by: Marc Zyngier
>>> ---
>>> drivers/pci/host/pci-host-generic.c | 9 +
>>> 1 file changed, 1 insertion(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/pci/host/pci-host-generic.c
>>> b/drivers/pci/host/pci-host-generic.c
>>> index 265dd25..545ff4e 100644
>>> --- a/drivers/pci/host/pci-host-generic.c
>>> +++ b/drivers/pci/host/pci-host-generic.c
>>> @@ -210,7 +210,6 @@ static int gen_pci_probe(struct platform_device *pdev)
>>> int err;
>>> const char *type;
>>> const struct of_device_id *of_id;
>>> - const int *prop;
>>> struct device *dev = &pdev->dev;
>>> struct device_node *np = dev->of_node;
>>> struct gen_pci *pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
>>> @@ -225,13 +224,7 @@ static int gen_pci_probe(struct platform_device *pdev)
>>> return -EINVAL;
>>> }
>>>
>>> - prop = of_get_property(of_chosen, "linux,pci-probe-only", NULL);
>>> - if (prop) {
>>> - if (*prop)
>>> - pci_add_flags(PCI_PROBE_ONLY);
>>> - else
>>> - pci_clear_flags(PCI_PROBE_ONLY);
>>> - }
>>> + of_pci_check_probe_only(of_chosen);
>>
>> Do we need support for pci-probe-only in pci-host-generic at all?
>> You're removing the use in amd-overdrive.dts, and there are no other
>> DTs in the kernel tree that mention it.
>>
>> If we can live without it, that would be nice. It seems like a relic from
>> days when we couldn't reliably assign resources. (I'm not saying we can do
>> that reliably even today, but I'd rather make it reliable than turn it
>> off.)
>
> Kvmtool certainly uses it (and generates its own DT, hence why you don't
> see it in mainline). Not sure about qemu, though.
QEMU definitely doesn't do proble-only. Is this driver used on real PPC
machines too? In that case you also won't see the dt, because it comes with the
hardware ;).
Alex
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 7/7] pmem, dax: have direct_access use __pmem annotation
On Thu, 2015-08-13 at 14:20 -0700, Dan Williams wrote: > On Thu, Aug 13, 2015 at 9:51 AM, Ross Zwisler > wrote: > > Update the annotation for the kaddr pointer returned by direct_access() > > so that it is a __pmem pointer. This is consistent with the PMEM driver > > and with how this direct_access() pointer is used in the DAX code. > > > > Signed-off-by: Ross Zwisler > > --- > > Documentation/filesystems/Locking | 3 ++- > > arch/powerpc/sysdev/axonram.c | 7 --- > > drivers/block/brd.c | 4 ++-- > > drivers/nvdimm/pmem.c | 4 ++-- > > drivers/s390/block/dcssblk.c | 10 + > > fs/block_dev.c| 2 +- > > fs/dax.c | 44 > > +-- > > include/linux/blkdev.h| 8 +++ > > 8 files changed, 45 insertions(+), 37 deletions(-) > > > > diff --git a/Documentation/filesystems/Locking > > b/Documentation/filesystems/Locking > > index 6a34a0f..06d4434 100644 > > --- a/Documentation/filesystems/Locking > > +++ b/Documentation/filesystems/Locking > > @@ -397,7 +397,8 @@ prototypes: > > int (*release) (struct gendisk *, fmode_t); > > int (*ioctl) (struct block_device *, fmode_t, unsigned, unsigned > > long); > > int (*compat_ioctl) (struct block_device *, fmode_t, unsigned, > > unsigned long); > > - int (*direct_access) (struct block_device *, sector_t, void **, > > unsigned long *); > > + int (*direct_access) (struct block_device *, sector_t, void __pmem > > **, > > + unsigned long *); > > So this collides with the __pfn_t work. I think the we have a > reasonable chance of getting that in to 4.3, so I'd wait to see if we > hit any major roadblocks with that set [1] before merging these. > > [1]: https://lists.01.org/pipermail/linux-nvdimm/2015-August/001803.html Fair enough. Yea, I hadn't merged with that series yet a) because I didn't know when its review cycle would settle down and b) because that series hadn't pulled in changes from Matthew for PMD support, which I was originally using as a baseline. I'll merge with your code for v3. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 7/7] pmem, dax: have direct_access use __pmem annotation
On Fri, Aug 14, 2015 at 9:55 AM, Ross Zwisler wrote: > On Thu, 2015-08-13 at 14:20 -0700, Dan Williams wrote: >> On Thu, Aug 13, 2015 at 9:51 AM, Ross Zwisler >> wrote: >> > Update the annotation for the kaddr pointer returned by direct_access() >> > so that it is a __pmem pointer. This is consistent with the PMEM driver >> > and with how this direct_access() pointer is used in the DAX code. >> > >> > Signed-off-by: Ross Zwisler >> > --- >> > Documentation/filesystems/Locking | 3 ++- >> > arch/powerpc/sysdev/axonram.c | 7 --- >> > drivers/block/brd.c | 4 ++-- >> > drivers/nvdimm/pmem.c | 4 ++-- >> > drivers/s390/block/dcssblk.c | 10 + >> > fs/block_dev.c| 2 +- >> > fs/dax.c | 44 >> > +-- >> > include/linux/blkdev.h| 8 +++ >> > 8 files changed, 45 insertions(+), 37 deletions(-) >> > >> > diff --git a/Documentation/filesystems/Locking >> > b/Documentation/filesystems/Locking >> > index 6a34a0f..06d4434 100644 >> > --- a/Documentation/filesystems/Locking >> > +++ b/Documentation/filesystems/Locking >> > @@ -397,7 +397,8 @@ prototypes: >> > int (*release) (struct gendisk *, fmode_t); >> > int (*ioctl) (struct block_device *, fmode_t, unsigned, unsigned >> > long); >> > int (*compat_ioctl) (struct block_device *, fmode_t, unsigned, >> > unsigned long); >> > - int (*direct_access) (struct block_device *, sector_t, void **, >> > unsigned long *); >> > + int (*direct_access) (struct block_device *, sector_t, void __pmem >> > **, >> > + unsigned long *); >> >> So this collides with the __pfn_t work. I think the we have a >> reasonable chance of getting that in to 4.3, so I'd wait to see if we >> hit any major roadblocks with that set [1] before merging these. >> >> [1]: https://lists.01.org/pipermail/linux-nvdimm/2015-August/001803.html > > Fair enough. Yea, I hadn't merged with that series yet a) because I didn't > know when its review cycle would settle down and b) because that series hadn't > pulled in changes from Matthew for PMD support, which I was originally using > as a baseline. > > I'll merge with your code for v3. Sounds, let me go rebase the __pfn_t patches on -mm so we'all lined up and collision free. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 2/4] PCI: pci-host-generic: Fix lookup of linux,pci-probe-only property
On Fri, Aug 14, 2015 at 11:43 AM, Will Deacon wrote:
> On Fri, Aug 14, 2015 at 05:40:51PM +0100, Bjorn Helgaas wrote:
>> On Fri, Aug 14, 2015 at 05:19:17PM +0100, Marc Zyngier wrote:
>> > When pci-host-generic looks for the probe-only property, it seems
>> > to trust the DT to be correctly written, and assumes that there
>> > is a parameter to the property.
>> >
>> > Unfortunately, this is not always the case, and some firmware expose
>> > this property naked. The driver ends up making a decision based on
>> > whatever the property pointer points to, which is likely to be junk.
>> >
>> > Switch to the common of_pci.c implementation that doesn't suffer
>> > from this problem.
>> >
>> > Signed-off-by: Marc Zyngier
>> > ---
>> > drivers/pci/host/pci-host-generic.c | 9 +
>> > 1 file changed, 1 insertion(+), 8 deletions(-)
>> >
>> > diff --git a/drivers/pci/host/pci-host-generic.c
>> > b/drivers/pci/host/pci-host-generic.c
>> > index 265dd25..545ff4e 100644
>> > --- a/drivers/pci/host/pci-host-generic.c
>> > +++ b/drivers/pci/host/pci-host-generic.c
>> > @@ -210,7 +210,6 @@ static int gen_pci_probe(struct platform_device *pdev)
>> > int err;
>> > const char *type;
>> > const struct of_device_id *of_id;
>> > - const int *prop;
>> > struct device *dev = &pdev->dev;
>> > struct device_node *np = dev->of_node;
>> > struct gen_pci *pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
>> > @@ -225,13 +224,7 @@ static int gen_pci_probe(struct platform_device *pdev)
>> > return -EINVAL;
>> > }
>> >
>> > - prop = of_get_property(of_chosen, "linux,pci-probe-only", NULL);
>> > - if (prop) {
>> > - if (*prop)
>> > - pci_add_flags(PCI_PROBE_ONLY);
>> > - else
>> > - pci_clear_flags(PCI_PROBE_ONLY);
>> > - }
>> > + of_pci_check_probe_only(of_chosen);
>>
>> Do we need support for pci-probe-only in pci-host-generic at all?
>> You're removing the use in amd-overdrive.dts, and there are no other
>> DTs in the kernel tree that mention it.
>>
>> If we can live without it, that would be nice. It seems like a relic from
>> days when we couldn't reliably assign resources. (I'm not saying we can do
>> that reliably even today, but I'd rather make it reliable than turn it
>> off.)
>
> Kvmtool certainly uses it (and generates its own DT, hence why you don't
> see it in mainline). Not sure about qemu, though.
Do you know why kvmtool wants probe-only? Would something break if we
didn't support probe-only? I guess we'd be looking for a case where
Linux assigns resources and that assignment doesn't work with kvmtool?
"pci-probe-only" doesn't appear in qemu
(git://git.qemu-project.org/qemu.git). (I guess Alexander confirmed
that later.)
Alexander wrote:
> QEMU definitely doesn't do proble-only.
> Is this driver used on real PPC machines too?
I think you're asking about pci-host-generic.c, and the answer is
"no," because pci-host-generic.c currently depends on CONFIG_ARM.
Bjorn
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 1/4] of/pci: Add of_pci_check_probe_only to parse "linux, pci-probe-only"
On Fri, Aug 14, 2015 at 11:19 AM, Marc Zyngier wrote:
> Both pci-host-generic and Pseries parse the "linux,pci-probe-only"
> to engage the PCI_PROBE_ONLY mode, and both have a subtle bug that
> can be triggered if the property has no parameter.
Humm, I bet we could break a lot of machines if we fixed the core code
to properly make pp->value NULL when there is no value.
> Provide a generic implementation that can be used by both.
>
> Signed-off-by: Marc Zyngier
> ---
> drivers/of/of_pci.c| 30 ++
> include/linux/of_pci.h | 3 +++
> 2 files changed, 33 insertions(+)
>
> diff --git a/drivers/of/of_pci.c b/drivers/of/of_pci.c
> index 5751dc5..a4e29ff 100644
> --- a/drivers/of/of_pci.c
> +++ b/drivers/of/of_pci.c
> @@ -118,6 +118,36 @@ int of_get_pci_domain_nr(struct device_node *node)
> EXPORT_SYMBOL_GPL(of_get_pci_domain_nr);
>
> /**
> + * of_pci_check_probe_only - Setup probe only mode if linux,pci-probe-only
> + * is present and valid
> + *
> + * @node: device tree node that may contain the property (usually "chosen")
> + *
> + */
> +void of_pci_check_probe_only(struct device_node *node)
> +{
> + const int *prop;
> + int len;
> +
> + if (!node)
> + return;
> +
> + prop = of_get_property(node, "linux,pci-probe-only", &len);
It is preferred to use of_property_read_u32 to avoid just these types
of problems.
Rob
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V2] cxl: Set up and enable PSL Timebase
Hi Philippe,
I was trying to test Timebase Sync today and realised that it hasn't
been applied to mpe's next tree, and that it no longer applies cleanly.
Would you be able to respin it please?
I also have some minor comments inline.
On Wed, 2015-06-10 at 16:01 +0200, Philippe Bergheaud wrote:
> This patch configures the PSL Timebase function and enables it,
> after the CAPP has been initialized by OPAL.
>
This changelog should go after a line like this:
---
> V2:
> - Clear CXL_PSL_ErrIVTE_tberror bit
> - Define the sync count unit
> - Wait 1ms before each test
> - Use negative error code
> - Do not ignore errors
> - Except if timebase is not supported by OPAL
> - Be silent on success
>
> Signed-off-by: Philippe Bergheaud
> ---
> drivers/misc/cxl/cxl.h |5
> drivers/misc/cxl/pci.c | 57
> +++-
> 2 files changed, 61 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
> index a1cee47..38a7cf9 100644
> --- a/drivers/misc/cxl/cxl.h
> +++ b/drivers/misc/cxl/cxl.h
> @@ -82,8 +82,10 @@ static const cxl_p1_reg_t CXL_PSL_AFUSEL = {0x00B0};
> /* 0x00C0:7EFF Implementation dependent area */
> static const cxl_p1_reg_t CXL_PSL_FIR1 = {0x0100};
> static const cxl_p1_reg_t CXL_PSL_FIR2 = {0x0108};
> +static const cxl_p1_reg_t CXL_PSL_Timebase = {0x0110};
> static const cxl_p1_reg_t CXL_PSL_VERSION = {0x0118};
> static const cxl_p1_reg_t CXL_PSL_RESLCKTO = {0x0128};
> +static const cxl_p1_reg_t CXL_PSL_TB_CTLSTAT = {0x0140};
> static const cxl_p1_reg_t CXL_PSL_FIR_CNTL = {0x0148};
> static const cxl_p1_reg_t CXL_PSL_DSNDCTL = {0x0150};
> static const cxl_p1_reg_t CXL_PSL_SNWRALLOC = {0x0158};
> @@ -151,6 +153,9 @@ static const cxl_p2n_reg_t CXL_PSL_WED_An = {0x0A0};
> #define CXL_PSL_SPAP_Size_Shift 4
> #define CXL_PSL_SPAP_V0x0001ULL
>
> +/** CXL_PSL_Control /
> +#define CXL_PSL_Control_tb 0x0001ULL
> +
> /** CXL_PSL_DLCNTL */
> #define CXL_PSL_DLCNTL_D (0x1ull << (63-28))
> #define CXL_PSL_DLCNTL_C (0x1ull << (63-29))
> diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
> index fc938de..ea1a79f 100644
> --- a/drivers/misc/cxl/pci.c
> +++ b/drivers/misc/cxl/pci.c
> @@ -360,6 +360,55 @@ static int init_implementation_adapter_regs(struct cxl
> *adapter, struct pci_dev
> return 0;
> }
>
> +#define TBSYNC_CNT(n) (((u64)n & 0x7) << (63-6))
> +#define _2048_250MHZ_CYCLES 1
> +
I'm not quite clear what these macros do or mean, and I'm also a little
bit uncomfortable with them just sitting in the middle of the file.
In particular, what does _2048_250MHZ_CYCLES mean?
> +static int cxl_setup_psl_timebase(struct cxl *adapter, struct pci_dev *dev)
> +{
> + u64 psl_tb;
> + int delta;
> + unsigned int retry = 0;
> + struct device_node *np;
> +
> + if (!(np = pnv_pci_to_phb_node(dev)))
> + return -ENODEV;
> +
> + /* Do not fail when CAPP timebase sync is not supported by OPAL */
> + of_node_get(np);
> + if (! of_get_property(np, "ibm,capp-timebase-sync", NULL)) {
> + of_node_put(np);
> + pr_err("PSL: Timebase sync: OPAL support missing\n");
> + return 0;
> + }
> + of_node_put(np);
Do we need to grab a reference to the node?
> +
> + /*
> + * Setup PSL Timebase Control and Status register
> + * with the recommended Timebase Sync Count value
> + */
> + cxl_p1_write(adapter, CXL_PSL_TB_CTLSTAT,
> + TBSYNC_CNT(2 * _2048_250MHZ_CYCLES));
> +
> + /* Enable PSL Timebase */
> + cxl_p1_write(adapter, CXL_PSL_Control, 0x);
> + cxl_p1_write(adapter, CXL_PSL_Control, CXL_PSL_Control_tb);
> +
Is there a reason we need to write 0s to PSL control before we write the
timebase control value?
> + /* Wait until CORE TB and PSL TB difference <= 16usecs */
> + do {
> + msleep(1);
> + if (retry++ > 5) {
> + pr_err("PSL: Timebase sync: giving up!\n");
> + return -EIO;
> + }
> + psl_tb = cxl_p1_read(adapter, CXL_PSL_Timebase);
> + delta = mftb() - psl_tb;
> + if (delta < 0)
> + delta = -delta;
> + } while (cputime_to_usecs(delta) > 16);
> +
> + return 0;
> +}
> +
> static int init_implementation_afu_regs(struct cxl_afu *afu)
> {
> /* read/write masks for this slice */
> @@ -952,9 +1001,12 @@ static struct cxl *cxl_alloc_adapter(struct pci_dev
> *dev)
> return adapter;
> }
>
> +#define CXL_PSL_ErrIVTE_tberror (0x1ull << (63-31))
> +
Again, could we put this somewhere else, rather than in the middle of
the file?
> static int sanitise_adapter_regs(struct cxl *adapter)
> {
> - cxl_p1_write(adapter, CXL_PSL_ErrIVTE, 0x
Re: [PATCH 3/3] powerpc/e6500: hw tablewalk: order the memory access when acquire/release tcd lock
On Fri, 2015-08-14 at 15:13 +0800, Kevin Hao wrote: > On Thu, Aug 13, 2015 at 10:39:19PM -0500, Scott Wood wrote: > > On Thu, 2015-08-13 at 19:51 +0800, Kevin Hao wrote: > > > I didn't find anything unusual. But I think we do need to order the > > > load/store of esel_next when acquire/release tcd lock. For acquire, > > > add a data dependency to order the loads of lock and esel_next. > > > For release, even there already have a "isync" here, but it doesn't > > > guarantee any memory access order. So we still need "lwsync" for > > > the two stores for lock and esel_next. > > > > I was going to say that esel_next is just a hint and it doesn't really > > matter > > if we occasionally get the wrong value, unless it happens often enough to > > cause more performance degradation than the lwsync causes. However, with > > the > > A-008139 workaround we do need to read the same value from esel_next both > > times. It might be less costly to save/restore an additional register > > instead of lwsync, though. > > I will try to get some benchmark number to compare which method is a bit > better. > Do you have any recommended benchmark for a case this is? lmbench lat_mem_rd with a stride chosen to maximize TLB misses. For the uncontended case, one instance; for the contended case, two instances, one pinned to each thread of a core. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/3] powerpc/e6500: hw tablewalk: optimize a bit for tcd lock acquiring codes
On Fri, 2015-08-14 at 15:13 +0800, Kevin Hao wrote: > On Thu, Aug 13, 2015 at 01:44:43PM -0500, Scott Wood wrote: > > On Thu, 2015-08-13 at 19:51 +0800, Kevin Hao wrote: > > > It makes no sense to put the instructions for calculating the lock > > > value (cpu number + 1) and the clearing of eq bit of cr1 in lbarx/stbcx > > > loop. And when the lock is acquired by the other thread, the current > > > lock value has no chance to equal with the lock value used by current > > > cpu. So we can skip the comparing for these two lock values in the > > > lbz/bne loop. > > > > > > Signed-off-by: Kevin Hao > > > --- > > > arch/powerpc/mm/tlb_low_64e.S | 10 +- > > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > > > diff --git a/arch/powerpc/mm/tlb_low_64e.S > > > b/arch/powerpc/mm/tlb_low_64e.S > > > index 765b419883f2..e4185581c5a7 100644 > > > --- a/arch/powerpc/mm/tlb_low_64e.S > > > +++ b/arch/powerpc/mm/tlb_low_64e.S > > > @@ -308,11 +308,11 @@ BEGIN_FTR_SECTION /* CPU_FTR_SMT */ > > >* > > >* MAS6:IND should be already set based on MAS4 > > >*/ > > > -1: lbarx r15,0,r11 > > > lhz r10,PACAPACAINDEX(r13) > > > - cmpdi r15,0 > > > - cmpdi cr1,r15,1 /* set cr1.eq = 0 for non-recursive */ > > > addir10,r10,1 > > > + crclr cr1*4+eq/* set cr1.eq = 0 for non-recursive */ > > > +1: lbarx r15,0,r11 > > > + cmpdi r15,0 > > > bne 2f > > > > You're optimizing the contended case at the expense of introducing stalls > > in > > the uncontended case. > > Before the patch, the uncontended case code sequence are: > 1:lbarx r15,0,r11 > lhz r10,PACAPACAINDEX(r13) > cmpdi r15,0 > cmpdi cr1,r15,1 /* set cr1.eq = 0 for non-recursive */ > addir10,r10,1 > bne 2f > stbcx. r10,0,r11 > bne 1b > > > After the patch: > lhz r10,PACAPACAINDEX(r13) > addir10,r10,1 > crclr cr1*4+eq/* set cr1.eq = 0 for non-recursive */ > 1:lbarx r15,0,r11 > cmpdi r15,0 > bne 2f > stbcx. r10,0,r11 > bne 1b > > As you know, the lbarx is a Presync instruction and stbcx is a Presync and > Postsync instruction. Yes, so don't we want to move instructions after the lbarx if possible, so that the presync condition is achieved sooner? > Putting the unnecessary instructions in the lbarx/stbcx > loop also serialize these instructions execution. Again, the common case should be that the loop executes only once. The two cmpdi instructions should pair, the addi should pair with the bne, and the lhz should happen while waiting for the lbarx result. My understanding of how to model this stuff is certainly imperfect/incomplete, so I generally try to confirm by testing, but I think both loops take the same number of cycles per iteration. > The execution latency of > lbarx is only 3 cycles and there are still two instructions after it. > Considering the out of order execution optimization after this patch, do you > really think that new uncontended path will become slower due to this > additional stall? The (theoretical) additional time is before the loop, not during it. > > Does it really matter if there are more instructions > > in the loop? > > I really think we should minimize the window of lbarx/stbcx, for following > two > reasons: > - The bigger of this window, the more possible conflicts between the two > threads run into this loop simultaneously. That's more true of the total time the lock is held, not the lbarx/stbcx section. > - The reservation used by lbarx may be cleared by another thread due to > store to the same reservation granule. The smaller the window of > lbarx/stbcx, the less possibility to be affected by this. There's only one other thread that should be touching that reservation granule, and it's the one we're waiting for. In any case, if there is a difference in loop iteration execution time, it's small. > > This change just means that you'll spin in the loop for more > > iterations (if it even does that -- I think the cycles per loop iteration > > might be the same before and after, due to load latency and pairing) > > while > > waiting for the other thread to release the lock. > > Besides the optimization for the contended case, it also make the code more > readable with these changes: > - It always seem a bit weird to calculate the lock value for the current > cpu in the lbarx/stbcx loop. > - The "cmpdi cr1,r15,1" seems pretty confusion. It doesn't always do > what > the comment said (set cr1.eq = 0). In some cases, it does set the > crq.eq = 1, such as when running on cpu 1 with lock is acquired by cpu0. > All we need to do just clear the cr1.eq unconditionally. We only care about cr1.eq when we break out of the loop, in which case r15 will have been zero. But yes, crclr is better. > >
Re: [PATCH v6 42/42] pci/hotplug: PowerPC PowerNV PCI hotplug driver
On 08/06/2015 02:11 PM, Gavin Shan wrote:
The patch intends to add standalone driver to support PCI hotplug
for PowerPC PowerNV platform, which runs on top of skiboot firmware.
The firmware identified hotpluggable slots and marked their device
tree node with proper "ibm,slot-pluggable" and "ibm,reset-by-firmware".
The driver simply scans device-tree to create/register PCI hotplug slot
accordingly.
If the skiboot firmware doesn't support slot status retrieval, the PCI
slot device node shouldn't have property "ibm,reset-by-firmware". In
that case, none of valid PCI slots will be detected from device tree.
The skiboot firmware doesn't export the capability to access attention
LEDs yet and it's something for TBD.
Signed-off-by: Gavin Shan
Acked-by: Bjorn Helgaas
---
MAINTAINERS| 6 +
drivers/pci/hotplug/Kconfig| 12 +
drivers/pci/hotplug/Makefile | 4 +
drivers/pci/hotplug/powernv_php.c | 140 +++
drivers/pci/hotplug/powernv_php.h | 92 +
drivers/pci/hotplug/powernv_php_slot.c | 722 +
6 files changed, 976 insertions(+)
create mode 100644 drivers/pci/hotplug/powernv_php.c
create mode 100644 drivers/pci/hotplug/powernv_php.h
create mode 100644 drivers/pci/hotplug/powernv_php_slot.c
diff --git a/MAINTAINERS b/MAINTAINERS
index fd60784..3b75c92 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7747,6 +7747,12 @@ L: linux-...@vger.kernel.org
S:Supported
F:Documentation/PCI/pci-error-recovery.txt
+PCI HOTPLUG DRIVER FOR POWERNV PLATFORM
+M: Gavin Shan
+L: linux-...@vger.kernel.org
+S: Supported
+F: drivers/pci/hotplug/powernv_php*
+
PCI SUBSYSTEM
M:Bjorn Helgaas
L:linux-...@vger.kernel.org
diff --git a/drivers/pci/hotplug/Kconfig b/drivers/pci/hotplug/Kconfig
index df8caec..ef55dae 100644
--- a/drivers/pci/hotplug/Kconfig
+++ b/drivers/pci/hotplug/Kconfig
@@ -113,6 +113,18 @@ config HOTPLUG_PCI_SHPC
When in doubt, say N.
+config HOTPLUG_PCI_POWERNV
+ tristate "PowerPC PowerNV PCI Hotplug driver"
+ depends on PPC_POWERNV && EEH
+ help
+ Say Y here if you run PowerPC PowerNV platform that supports
+ PCI Hotplug
+
+ To compile this driver as a module, choose M here: the
+ module will be called powernv-php.
+
+ When in doubt, say N.
+
config HOTPLUG_PCI_RPA
tristate "RPA PCI Hotplug driver"
depends on PPC_PSERIES && EEH
diff --git a/drivers/pci/hotplug/Makefile b/drivers/pci/hotplug/Makefile
index b616e75..fd51d65 100644
--- a/drivers/pci/hotplug/Makefile
+++ b/drivers/pci/hotplug/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_HOTPLUG_PCI_PCIE)+= pciehp.o
obj-$(CONFIG_HOTPLUG_PCI_CPCI_ZT5550) += cpcihp_zt5550.o
obj-$(CONFIG_HOTPLUG_PCI_CPCI_GENERIC)+= cpcihp_generic.o
obj-$(CONFIG_HOTPLUG_PCI_SHPC)+= shpchp.o
+obj-$(CONFIG_HOTPLUG_PCI_POWERNV) += powernv-php.o
obj-$(CONFIG_HOTPLUG_PCI_RPA) += rpaphp.o
obj-$(CONFIG_HOTPLUG_PCI_RPA_DLPAR) += rpadlpar_io.o
obj-$(CONFIG_HOTPLUG_PCI_SGI) += sgi_hotplug.o
@@ -50,6 +51,9 @@ ibmphp-objs := ibmphp_core.o \
acpiphp-objs := acpiphp_core.o \
acpiphp_glue.o
+powernv-php-objs := powernv_php.o \
+ powernv_php_slot.o
+
rpaphp-objs := rpaphp_core.o \
rpaphp_pci.o\
rpaphp_slot.o
diff --git a/drivers/pci/hotplug/powernv_php.c
b/drivers/pci/hotplug/powernv_php.c
new file mode 100644
index 000..4cbff7a
--- /dev/null
+++ b/drivers/pci/hotplug/powernv_php.c
@@ -0,0 +1,140 @@
+/*
+ * PCI Hotplug Driver for PowerPC PowerNV platform.
+ *
+ * Copyright Gavin Shan, IBM Corporation 2015.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include
+
+#include
+#include
+
+#include "powernv_php.h"
+
+#define DRIVER_VERSION "0.1"
+#define DRIVER_AUTHOR "Gavin Shan, IBM Corporation"
+#define DRIVER_DESC"PowerPC PowerNV PCI Hotplug Driver"
Align all or none.
+
+static struct notifier_block php_msg_nb = {
+ .notifier_call = powernv_php_msg_handler,
+ .next = NULL,
+ .priority = 0,
+};
+
+static int powernv_php_register_one(struct device_node *dn)
+{
+ struct powernv_php_slot *slot;
+ const __be32 *prop32;
+ int ret;
+
+ /* Check if it's hotpluggable slot */
+ prop32 = of_get_property(dn, "ibm,slot-pluggable", NULL);
+ if (!prop32 || !of_read_number(prop32, 1))
+ return -ENXIO;
+
+ prop32 = of_get_property(dn, "ibm,reset-by-firmware", NULL);
+ if (!prop32 ||
Re: [PATCH v2] video: fbdev: fsl: Fix the sleep function for FSL DIU module
Dongsheng Wang wrote: For deep sleep, the diu module will power off, when wake up from the deep sleep, the registers need to be reinitialized. Signed-off-by: Jason Jin Signed-off-by: Wang Dongsheng Acked-by: Timur Tabi ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v6 42/42] pci/hotplug: PowerPC PowerNV PCI hotplug driver
On Sat, Aug 15, 2015 at 01:13:21PM +1000, Alexey Kardashevskiy wrote:
>On 08/06/2015 02:11 PM, Gavin Shan wrote:
>>The patch intends to add standalone driver to support PCI hotplug
>>for PowerPC PowerNV platform, which runs on top of skiboot firmware.
>>The firmware identified hotpluggable slots and marked their device
>>tree node with proper "ibm,slot-pluggable" and "ibm,reset-by-firmware".
>>The driver simply scans device-tree to create/register PCI hotplug slot
>>accordingly.
>>
>>If the skiboot firmware doesn't support slot status retrieval, the PCI
>>slot device node shouldn't have property "ibm,reset-by-firmware". In
>>that case, none of valid PCI slots will be detected from device tree.
>>The skiboot firmware doesn't export the capability to access attention
>>LEDs yet and it's something for TBD.
>>
>>Signed-off-by: Gavin Shan
>>Acked-by: Bjorn Helgaas
>>---
>> MAINTAINERS| 6 +
>> drivers/pci/hotplug/Kconfig| 12 +
>> drivers/pci/hotplug/Makefile | 4 +
>> drivers/pci/hotplug/powernv_php.c | 140 +++
>> drivers/pci/hotplug/powernv_php.h | 92 +
>> drivers/pci/hotplug/powernv_php_slot.c | 722
>> +
>> 6 files changed, 976 insertions(+)
>> create mode 100644 drivers/pci/hotplug/powernv_php.c
>> create mode 100644 drivers/pci/hotplug/powernv_php.h
>> create mode 100644 drivers/pci/hotplug/powernv_php_slot.c
>>
>>diff --git a/MAINTAINERS b/MAINTAINERS
>>index fd60784..3b75c92 100644
>>--- a/MAINTAINERS
>>+++ b/MAINTAINERS
>>@@ -7747,6 +7747,12 @@ L: linux-...@vger.kernel.org
>> S: Supported
>> F: Documentation/PCI/pci-error-recovery.txt
>>
>>+PCI HOTPLUG DRIVER FOR POWERNV PLATFORM
>>+M: Gavin Shan
>>+L: linux-...@vger.kernel.org
>>+S: Supported
>>+F: drivers/pci/hotplug/powernv_php*
>>+
>> PCI SUBSYSTEM
>> M: Bjorn Helgaas
>> L: linux-...@vger.kernel.org
>>diff --git a/drivers/pci/hotplug/Kconfig b/drivers/pci/hotplug/Kconfig
>>index df8caec..ef55dae 100644
>>--- a/drivers/pci/hotplug/Kconfig
>>+++ b/drivers/pci/hotplug/Kconfig
>>@@ -113,6 +113,18 @@ config HOTPLUG_PCI_SHPC
>>
>>When in doubt, say N.
>>
>>+config HOTPLUG_PCI_POWERNV
>>+ tristate "PowerPC PowerNV PCI Hotplug driver"
>>+ depends on PPC_POWERNV && EEH
>>+ help
>>+ Say Y here if you run PowerPC PowerNV platform that supports
>>+ PCI Hotplug
>>+
>>+ To compile this driver as a module, choose M here: the
>>+ module will be called powernv-php.
>>+
>>+ When in doubt, say N.
>>+
>> config HOTPLUG_PCI_RPA
>> tristate "RPA PCI Hotplug driver"
>> depends on PPC_PSERIES && EEH
>>diff --git a/drivers/pci/hotplug/Makefile b/drivers/pci/hotplug/Makefile
>>index b616e75..fd51d65 100644
>>--- a/drivers/pci/hotplug/Makefile
>>+++ b/drivers/pci/hotplug/Makefile
>>@@ -14,6 +14,7 @@ obj-$(CONFIG_HOTPLUG_PCI_PCIE) += pciehp.o
>> obj-$(CONFIG_HOTPLUG_PCI_CPCI_ZT5550) += cpcihp_zt5550.o
>> obj-$(CONFIG_HOTPLUG_PCI_CPCI_GENERIC) += cpcihp_generic.o
>> obj-$(CONFIG_HOTPLUG_PCI_SHPC) += shpchp.o
>>+obj-$(CONFIG_HOTPLUG_PCI_POWERNV)+= powernv-php.o
>> obj-$(CONFIG_HOTPLUG_PCI_RPA) += rpaphp.o
>> obj-$(CONFIG_HOTPLUG_PCI_RPA_DLPAR) += rpadlpar_io.o
>> obj-$(CONFIG_HOTPLUG_PCI_SGI) += sgi_hotplug.o
>>@@ -50,6 +51,9 @@ ibmphp-objs := ibmphp_core.o \
>> acpiphp-objs:= acpiphp_core.o \
>> acpiphp_glue.o
>>
>>+powernv-php-objs := powernv_php.o \
>>+ powernv_php_slot.o
>>+
>> rpaphp-objs := rpaphp_core.o \
>> rpaphp_pci.o\
>> rpaphp_slot.o
>>diff --git a/drivers/pci/hotplug/powernv_php.c
>>b/drivers/pci/hotplug/powernv_php.c
>>new file mode 100644
>>index 000..4cbff7a
>>--- /dev/null
>>+++ b/drivers/pci/hotplug/powernv_php.c
>>@@ -0,0 +1,140 @@
>>+/*
>>+ * PCI Hotplug Driver for PowerPC PowerNV platform.
>>+ *
>>+ * Copyright Gavin Shan, IBM Corporation 2015.
>>+ *
>>+ * This program is free software; you can redistribute it and/or modify
>>+ * it under the terms of the GNU General Public License as published by
>>+ * the Free Software Foundation; either version 2 of the License, or
>>+ * (at your option) any later version.
>>+ */
>>+
>>+#include
>>+
>>+#include
>>+#include
>>+
>>+#include "powernv_php.h"
>>+
>>+#define DRIVER_VERSION "0.1"
>>+#define DRIVER_AUTHOR"Gavin Shan, IBM Corporation"
>>+#define DRIVER_DESC "PowerPC PowerNV PCI Hotplug Driver"
>
>
>Align all or none.
>
All of them are already aligned well. Please check your email setting.
>>+
>>+static struct notifier_block php_msg_nb = {
>>+ .notifier_call = powernv_php_msg_handler,
>>+ .next = NULL,
>>+ .priority = 0,
>>+};
>>+
>>+static int powernv_php_register_one(struct device_node *dn)
>>+{
>>+ str
Re: [PATCH v6 20/42] powerpc/powernv: Create PEs dynamically
On Fri, Aug 14, 2015 at 11:52:44PM +1000, Alexey Kardashevskiy wrote:
>On 08/06/2015 02:11 PM, Gavin Shan wrote:
>>Currently, the PEs and their associated resources are assigned
>>in ppc_md.pcibios_fixup() except those consumed by SRIOV VFs.
>>The function is called for once after PCI probing and resources
>>assignment is finished which isn't hotplug friendly.
>>
>>The patch creates PEs dynamically by ppc_md.pcibios_setup_bridge(),
>>which is called on the event during system bootup and PCI hotplug:
>>updating PCI bridge's windows after resource assignment/reassignment
>>are finished. For partial hotplug case, where not all PCI devices
>>belonging to the PE are unplugged and plugged again, we just need
>>unbinding/binding the affected PCI devices with the corresponding
>>PE without creating new one.
>>
>>Besides, it might require additional resources (e.g. M32) to the
>>windows of the PCI bridge when unplugging current adapter, and
>>insert a different adapter if there is one PCI slot, which is
>>assumed behind root port, or the downstream bridge of the PCIE
>>switch behind root port. The parent bridge of the newly plugged
>>adapter would reject the request to add more resources, leading
>>to hotplug failure. For the issue, the patch extends the windows
>>of root port, or the upstream port of the PCIe switch behind root
>>port to PHB's windows when ppc_md.pcibios_setup_bridge() is called.
>>
>>There is no upstream bridge for root bus, so we have to fix it up
>>before any PE is created because the root bus PE is the ancestor
>>to anyone else.
>>
>>Signed-off-by: Gavin Shan
>>---
>> arch/powerpc/platforms/powernv/pci-ioda.c | 226
>> ++
>> arch/powerpc/platforms/powernv/pci.h | 1 +
>> 2 files changed, 137 insertions(+), 90 deletions(-)
>>
>>diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c
>>b/arch/powerpc/platforms/powernv/pci-ioda.c
>>index 8aa6ab8..37847a3 100644
>>--- a/arch/powerpc/platforms/powernv/pci-ioda.c
>>+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
>>@@ -1083,6 +1083,13 @@ static void pnv_ioda_setup_same_PE(struct pci_bus
>>*bus, struct pnv_ioda_pe *pe)
>> pci_name(dev));
>> continue;
>> }
>>+
>>+ /* The PCI device might be not detached from the
>>+ * PE in partial hotplug case.
>>+ */
>>+ if (pdn->pe_number != IODA_INVALID_PE)
>>+ continue;
>>+
>> pdn->pe_number = pe->pe_number;
>> pe->dma32_weight += pnv_ioda_dma_weight(dev);
>> if ((pe->flags & PNV_IODA_PE_BUS_ALL) && dev->subordinate)
>>@@ -1101,9 +1108,27 @@ static struct pnv_ioda_pe
>>*pnv_ioda_setup_bus_PE(struct pci_bus *bus, bool all)
>> struct pci_controller *hose = pci_bus_to_host(bus);
>> struct pnv_phb *phb = hose->private_data;
>> struct pnv_ioda_pe *pe = NULL;
>>+ int pe_num;
>>+
>>+ /* For partial hotplug case, the PE instance hasn't been destroyed
>>+ * yet. We shouldn't allocated a new one and assign resources to
>>+ * it. The existing PE instance should be reused, but we should
>>+ * associate the devices to the PE.
>>+ */
>>+ pe_num = phb->ioda.pe_rmap[bus->number << 8];
>>+ if (pe_num != IODA_INVALID_PE) {
>>+ pe = &phb->ioda.pe_array[pe_num];
>>+ pnv_ioda_setup_same_PE(bus, pe);
>>+ return NULL;
>>+ }
>>+
>>+ /* PE number for root bus should have been reserved */
>>+ if (pci_is_root_bus(bus) &&
>>+ phb->ioda.root_pe_idx != IODA_INVALID_PE)
>>+ pe = &phb->ioda.pe_array[phb->ioda.root_pe_idx];
>>
>> /* Check if PE is determined by M64 */
>>- if (phb->pick_m64_pe)
>>+ if (!pe && phb->pick_m64_pe)
>
>
>else if (phb->pick_m64_pe)
>
No. When this function is called for the root of root bus, the PE
should have been reserved. So we still have to check @pe.
>
>
>> pe = phb->pick_m64_pe(bus, all);
>>
>> /* The PE number isn't pinned by M64 */
>>@@ -1150,46 +1175,6 @@ static struct pnv_ioda_pe
>>*pnv_ioda_setup_bus_PE(struct pci_bus *bus, bool all)
>> return pe;
>> }
>>
>>-static void pnv_ioda_setup_PEs(struct pci_bus *bus)
>>-{
>>- struct pci_dev *dev;
>>-
>>- pnv_ioda_setup_bus_PE(bus, false);
>>-
>>- list_for_each_entry(dev, &bus->devices, bus_list) {
>>- if (dev->subordinate) {
>>- if (pci_pcie_type(dev) == PCI_EXP_TYPE_PCI_BRIDGE)
>>- pnv_ioda_setup_bus_PE(dev->subordinate, true);
>>- else
>>- pnv_ioda_setup_PEs(dev->subordinate);
>>- }
>>- }
>>-}
>>-
>>-/*
>>- * Configure PEs so that the downstream PCI buses and devices
>>- * could have their associated PE#. Unfortunately, we didn't
>>- * figure out the way to identify the PLX bridge yet. So we
>>- * simply put the PCI bus and the subordinate behind the root
>>- * port to
[PATCH v2 0/5] clk: qoriq: Move chip-specific knowledge into driver
The existing device tree bindings are error-prone and inflexible. Correct the mistake by moving the knowledge into the driver, which has more flexibility in describing the quirks of each chip. This leaves the device tree to its proper role of identifying a programming interface rather than describing its individual registers. For more detail, see the commit message of patch 3. Scott Wood (5): cpufreq: qoriq: Don't look at clock implementation details powerpc/fsl: Move fsl_guts.h out of arch/powerpc clk: qoriq: Move chip-specific knowledge into driver cpufreq: qoriq: Remove frequency masking and minimum powerpc/fsl: Use new clockgen binding .../devicetree/bindings/clock/qoriq-clock.txt | 61 +- arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi |4 +- arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi |8 +- arch/powerpc/boot/dts/fsl/b4si-post.dtsi | 15 - arch/powerpc/boot/dts/fsl/p2041si-post.dtsi| 18 - arch/powerpc/boot/dts/fsl/p2041si-pre.dtsi |8 +- arch/powerpc/boot/dts/fsl/p3041si-post.dtsi| 18 - arch/powerpc/boot/dts/fsl/p3041si-pre.dtsi |8 +- arch/powerpc/boot/dts/fsl/p4080si-post.dtsi| 70 -- arch/powerpc/boot/dts/fsl/p4080si-pre.dtsi | 16 +- arch/powerpc/boot/dts/fsl/p5020si-pre.dtsi |4 +- arch/powerpc/boot/dts/fsl/p5040si-post.dtsi| 18 - arch/powerpc/boot/dts/fsl/p5040si-pre.dtsi |8 +- arch/powerpc/boot/dts/fsl/qoriq-clockgen1.dtsi | 50 +- arch/powerpc/boot/dts/fsl/qoriq-clockgen2.dtsi | 33 +- arch/powerpc/boot/dts/fsl/t1023si-post.dtsi| 16 - arch/powerpc/boot/dts/fsl/t102xsi-pre.dtsi |4 +- arch/powerpc/boot/dts/fsl/t1040si-post.dtsi| 44 - arch/powerpc/boot/dts/fsl/t104xsi-pre.dtsi |8 +- arch/powerpc/boot/dts/fsl/t2081si-post.dtsi| 22 - arch/powerpc/boot/dts/fsl/t208xsi-pre.dtsi |8 +- arch/powerpc/boot/dts/fsl/t4240si-post.dtsi| 61 - arch/powerpc/boot/dts/fsl/t4240si-pre.dtsi | 24 +- arch/powerpc/platforms/85xx/mpc85xx_mds.c |2 +- arch/powerpc/platforms/85xx/mpc85xx_rdb.c |2 +- arch/powerpc/platforms/85xx/p1022_ds.c |2 +- arch/powerpc/platforms/85xx/p1022_rdk.c|2 +- arch/powerpc/platforms/85xx/smp.c |2 +- arch/powerpc/platforms/85xx/twr_p102x.c|2 +- arch/powerpc/platforms/86xx/mpc8610_hpcd.c |2 +- drivers/clk/clk-qoriq.c| 1260 drivers/cpufreq/qoriq-cpufreq.c| 139 +-- drivers/iommu/fsl_pamu.c |2 +- .../asm/fsl_guts.h => include/linux/fsl/guts.h |6 +- sound/soc/fsl/mpc8610_hpcd.c |2 +- sound/soc/fsl/p1022_ds.c |2 +- sound/soc/fsl/p1022_rdk.c |2 +- 37 files changed, 1176 insertions(+), 777 deletions(-) rename arch/powerpc/include/asm/fsl_guts.h => include/linux/fsl/guts.h (98%) -- 2.1.4 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v2 1/5] cpufreq: qoriq: Don't look at clock implementation details
Get the CPU clock's potential parent clocks from the clock interface
itself, rather than manually parsing the clocks property to find a
phandle, looking at the clock-names property of that, and assuming that
those are valid parent clocks for the cpu clock.
This is necessary for cpufreq to continue working once the clocks
are generated based on the clock driver's knowledge of the chip
rather than a fragile device-tree description of the mux options.
Signed-off-by: Scott Wood
---
v2: non-RFC
drivers/cpufreq/qoriq-cpufreq.c | 47 -
1 file changed, 18 insertions(+), 29 deletions(-)
diff --git a/drivers/cpufreq/qoriq-cpufreq.c b/drivers/cpufreq/qoriq-cpufreq.c
index 358f075..32ab99e 100644
--- a/drivers/cpufreq/qoriq-cpufreq.c
+++ b/drivers/cpufreq/qoriq-cpufreq.c
@@ -11,6 +11,7 @@
#define pr_fmt(fmt)KBUILD_MODNAME ": " fmt
#include
+#include
#include
#include
#include
@@ -99,9 +100,10 @@ static u32 get_bus_freq(void)
return sysfreq;
}
-static struct device_node *cpu_to_clk_node(int cpu)
+static struct clk *cpu_to_clk(int cpu)
{
- struct device_node *np, *clk_np;
+ struct device_node *np;
+ struct clk *clk;
if (!cpu_present(cpu))
return NULL;
@@ -110,37 +112,32 @@ static struct device_node *cpu_to_clk_node(int cpu)
if (!np)
return NULL;
- clk_np = of_parse_phandle(np, "clocks", 0);
- if (!clk_np)
- return NULL;
-
+ clk = of_clk_get(np, 0);
of_node_put(np);
-
- return clk_np;
+ return clk;
}
/* traverse cpu nodes to get cpu mask of sharing clock wire */
static void set_affected_cpus(struct cpufreq_policy *policy)
{
- struct device_node *np, *clk_np;
struct cpumask *dstp = policy->cpus;
+ struct clk *clk;
+ const char *ourname, *theirname;
int i;
- np = cpu_to_clk_node(policy->cpu);
- if (!np)
- return;
+ ourname = __clk_get_name(policy->clk);
for_each_present_cpu(i) {
- clk_np = cpu_to_clk_node(i);
- if (!clk_np)
+ clk = cpu_to_clk(i);
+ if (IS_ERR(clk)) {
+ pr_err("%s: no clock for cpu %d\n", __func__, i);
continue;
+ }
- if (clk_np == np)
+ theirname = __clk_get_name(clk);
+ if (!strcmp(ourname, theirname))
cpumask_set_cpu(i, dstp);
-
- of_node_put(clk_np);
}
- of_node_put(np);
}
/* reduce the duplicated frequencies in frequency table */
@@ -219,17 +216,12 @@ static int qoriq_cpufreq_cpu_init(struct cpufreq_policy
*policy)
goto err_nomem2;
}
- pnode = of_parse_phandle(np, "clocks", 0);
- if (!pnode) {
- pr_err("%s: could not get clock information\n", __func__);
- goto err_nomem2;
- }
+ count = __clk_get_num_parents(policy->clk);
- count = of_property_count_strings(pnode, "clock-names");
data->pclk = kcalloc(count, sizeof(struct clk *), GFP_KERNEL);
if (!data->pclk) {
pr_err("%s: no memory\n", __func__);
- goto err_node;
+ goto err_nomem2;
}
table = kcalloc(count + 1, sizeof(*table), GFP_KERNEL);
@@ -244,7 +236,7 @@ static int qoriq_cpufreq_cpu_init(struct cpufreq_policy
*policy)
mask = 0x0;
for (i = 0; i < count; i++) {
- clk = of_clk_get(pnode, i);
+ clk = clk_get_parent_by_index(policy->clk, i);
data->pclk[i] = clk;
freq = clk_get_rate(clk);
/*
@@ -288,10 +280,7 @@ err_nomem1:
kfree(table);
err_pclk:
kfree(data->pclk);
-err_node:
- of_node_put(pnode);
err_nomem2:
- policy->driver_data = NULL;
kfree(data);
err_np:
of_node_put(np);
--
2.1.4
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v2 2/5] powerpc/fsl: Move fsl_guts.h out of arch/powerpc
Freescale's Layerscape ARM chips use the same structure. Signed-off-by: Scott Wood --- v2: non-RFC arch/powerpc/platforms/85xx/mpc85xx_mds.c | 2 +- arch/powerpc/platforms/85xx/mpc85xx_rdb.c | 2 +- arch/powerpc/platforms/85xx/p1022_ds.c | 2 +- arch/powerpc/platforms/85xx/p1022_rdk.c | 2 +- arch/powerpc/platforms/85xx/smp.c | 2 +- arch/powerpc/platforms/85xx/twr_p102x.c | 2 +- arch/powerpc/platforms/86xx/mpc8610_hpcd.c | 2 +- drivers/iommu/fsl_pamu.c| 2 +- arch/powerpc/include/asm/fsl_guts.h => include/linux/fsl/guts.h | 6 ++ sound/soc/fsl/mpc8610_hpcd.c| 2 +- sound/soc/fsl/p1022_ds.c| 2 +- sound/soc/fsl/p1022_rdk.c | 2 +- 12 files changed, 13 insertions(+), 15 deletions(-) rename arch/powerpc/include/asm/fsl_guts.h => include/linux/fsl/guts.h (98%) diff --git a/arch/powerpc/platforms/85xx/mpc85xx_mds.c b/arch/powerpc/platforms/85xx/mpc85xx_mds.c index a392e94..f0be439 100644 --- a/arch/powerpc/platforms/85xx/mpc85xx_mds.c +++ b/arch/powerpc/platforms/85xx/mpc85xx_mds.c @@ -34,6 +34,7 @@ #include #include #include +#include #include #include @@ -51,7 +52,6 @@ #include #include #include -#include #include "smp.h" #include "mpc85xx.h" diff --git a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c index e358bed..50dcc00 100644 --- a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c +++ b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c @@ -17,6 +17,7 @@ #include #include #include +#include #include #include @@ -27,7 +28,6 @@ #include #include #include -#include #include #include diff --git a/arch/powerpc/platforms/85xx/p1022_ds.c b/arch/powerpc/platforms/85xx/p1022_ds.c index 6ac986d3..371df82 100644 --- a/arch/powerpc/platforms/85xx/p1022_ds.c +++ b/arch/powerpc/platforms/85xx/p1022_ds.c @@ -16,6 +16,7 @@ * kind, whether express or implied. */ +#include #include #include #include @@ -25,7 +26,6 @@ #include #include #include -#include #include #include "smp.h" diff --git a/arch/powerpc/platforms/85xx/p1022_rdk.c b/arch/powerpc/platforms/85xx/p1022_rdk.c index 680232d..5087bec 100644 --- a/arch/powerpc/platforms/85xx/p1022_rdk.c +++ b/arch/powerpc/platforms/85xx/p1022_rdk.c @@ -12,6 +12,7 @@ * kind, whether express or implied. */ +#include #include #include #include @@ -21,7 +22,6 @@ #include #include #include -#include #include "smp.h" #include "mpc85xx.h" diff --git a/arch/powerpc/platforms/85xx/smp.c b/arch/powerpc/platforms/85xx/smp.c index b8b8216..6ac7786 100644 --- a/arch/powerpc/platforms/85xx/smp.c +++ b/arch/powerpc/platforms/85xx/smp.c @@ -19,6 +19,7 @@ #include #include #include +#include #include #include @@ -26,7 +27,6 @@ #include #include #include -#include #include #include diff --git a/arch/powerpc/platforms/85xx/twr_p102x.c b/arch/powerpc/platforms/85xx/twr_p102x.c index 30e002f..892e613 100644 --- a/arch/powerpc/platforms/85xx/twr_p102x.c +++ b/arch/powerpc/platforms/85xx/twr_p102x.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -23,7 +24,6 @@ #include #include #include -#include #include #include diff --git a/arch/powerpc/platforms/86xx/mpc8610_hpcd.c b/arch/powerpc/platforms/86xx/mpc8610_hpcd.c index 55413a5..437a9c3 100644 --- a/arch/powerpc/platforms/86xx/mpc8610_hpcd.c +++ b/arch/powerpc/platforms/86xx/mpc8610_hpcd.c @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -38,7 +39,6 @@ #include #include #include -#include #include "mpc86xx.h" diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c index abeedc9..24705c2 100644 --- a/drivers/iommu/fsl_pamu.c +++ b/drivers/iommu/fsl_pamu.c @@ -20,11 +20,11 @@ #include "fsl_pamu.h" +#include #include #include #include -#include /* define indexes for each operation mapping scenario */ #define OMI_QMAN0x00 diff --git a/arch/powerpc/include/asm/fsl_guts.h b/include/linux/fsl/guts.h similarity index 98% rename from arch/powerpc/include/asm/fsl_guts.h rename to include/linux/fsl/guts.h index 43b6bb1..3acf613 100644 --- a/arch/powerpc/include/asm/fsl_guts.h +++ b/include/linux/fsl/guts.h @@ -12,9 +12,8 @@ * option) any later version. */ -#ifndef __ASM_POWERPC_FSL_GUTS_H__ -#define __ASM_POWERPC_FSL_GUTS_H__ -#ifdef __KERNEL__ +#ifndef __FSL_GUTS_H__ +#define __FSL_GUTS_H__ /** * Global Utility Registers. @@ -189,4 +188,3 @@ static inline void guts_set_pmuxcr_dma(struct ccsr_guts __iomem *guts, #endif #endif -#endif diff --git a/sound/soc/fsl/mpc8610_hpcd.c b/sound/soc/fsl/mpc8610_hpcd.c index 9621b91..6f236f1
[PATCH v2 3/5] clk: qoriq: Move chip-specific knowledge into driver
The device tree should describe the chips (or chip-like subblocks) in the system, but it generally does not describe individual registers -- it should identify, rather than describe, a programming interface. This has not been the case with the QorIQ clockgen nodes. The knowledge of what each bit setting of CLKCnCSR means is encoded in three places (binding, pll node, and mux node), and the last also needs to know which options are valid on a particular chip. All three of these locations are considered stable ABI, making it difficult to fix mistakes (of which I have found several), much less refactor the abstraction to be able to address problems, limitations, or new chips. Under the current binding, a pll clock specifier of 2 means that the PLL is divided by 4 -- and the driver implements this, unless there happen to be four clock-output-names rather than 3, in which case it interprets it as PLL divided by 3. This does not appear in the binding documentation at all. That hack is now considered stable ABI. The current device tree nodes contain errors, such as saying that T1040 can set a core clock to PLL/4 when only PLL and PLL/2 are options. The current binding also ignores some restrictions on clock selection, such as p5020's requirement that if a core uses the "wrong" PLL, that PLL must be clocked lower than the "correct" PLL and be at most 80% of the rated CPU frequency. Possibly because of the lack of the ability to express such nuance in the binding, some valid options are omitted from the device trees, such as the ability on p4080 to run cores 0-3 from PLL3 and cores 4-7 from PLL1 (again, only if they are at most 80% of rated CPU frequency). This omission, combined with excessive caution in the cpufreq driver (addressed in a subsequent patch), means that currently on a 1500 MHz p4080 with typical PLL configuration, cpufreq can lower the frequency to 1200 MHz on half the CPUs and do nothing on the others. With this patchset, all CPUs can be lowered to 1200 MHz on a rev2 p4080, and on a rev3 p4080 half can be lowered to 750 MHz and the other half to 600 MHz. The current binding only deals with CPU clocks. To describe FMan in the device tree, we need to describe its clock. Some chips have additional muxes that work like the CPU muxes, but are not described in the device tree. Others require inspecting the Reset Control Word to determine which PLL is used. Rather than continue to extend this mess, replace it. Have the driver bind to the chip-specific clockgen compatible, and keep the detailed description of quirky chip variations in the driver, where it can be easily fixed, refactored, and extended. Older device trees will continue to work (including a workaround for old ls1021a device trees that are missing compatible and reg in the clockgen node, which even the old binding required). The pll/mux details in old device trees will be ignored, but "clocks" properties pointing at the old nodes will still work, and be directed at the corresponding new clock. Signed-off-by: Scott Wood --- v2: Improved legacy device tree compatibility, particularly for ls1021a. .../devicetree/bindings/clock/qoriq-clock.txt | 61 +- drivers/clk/clk-qoriq.c| 1260 2 files changed, 1089 insertions(+), 232 deletions(-) diff --git a/Documentation/devicetree/bindings/clock/qoriq-clock.txt b/Documentation/devicetree/bindings/clock/qoriq-clock.txt index df4a259..16a3ec4 100644 --- a/Documentation/devicetree/bindings/clock/qoriq-clock.txt +++ b/Documentation/devicetree/bindings/clock/qoriq-clock.txt @@ -1,6 +1,6 @@ * Clock Block on Freescale QorIQ Platforms -Freescale qoriq chips take primary clocking input from the external +Freescale QorIQ chips take primary clocking input from the external SYSCLK signal. The SYSCLK input (frequency) is multiplied using multiple phase locked loops (PLL) to create a variety of frequencies which can then be passed to a variety of internal logic, including @@ -13,14 +13,16 @@ which the chip complies. Chassis VersionExample Chips ---- 1.0p4080, p5020, p5040 -2.0t4240, b4860, t1040 +2.0t4240, b4860 1. Clock Block Binding Required properties: -- compatible: Should contain a specific clock block compatible string - and a single chassis clock compatible string. - Clock block strings include, but not limited to, one of the: +- compatible: Should contain a chip-specific clock block compatible + string and (if applicable) may contain a chassis-version clock + compatible string. + + Chip-specific strings are of the form "fsl,-clockgen", such as: * "fsl,p2041-clockgen" * "fsl,p3041-clockgen" * "fsl,p4080-clockgen" @@ -30,15 +32,14 @@ Required properties: * "fsl,b4420-clockgen" * "fsl,b4860-clockgen" * "fsl,ls1021a-clockgen" -
[PATCH v2 4/5] cpufreq: qoriq: Remove frequency masking and minimum
The clock driver now takes care of ensuring that the mux only
exposes options that are valid. The driver was currently being overly
conservative in some cases -- for example, the "min_cpufreq =
get_bus_freq()" restriction only applies to chips with erratum
A-004510, and whether the freq_mask used on p5020 is needed depends on
the actual frequencies of the PLLs (FWIW, p5040 has a similar
limitation but its .freq_mask was zero).
Further, the freq_mask mechanism makes assumptions about the number and
order of the clock parents. The new driver does not adhere to those
assumptions (in particular, it removes invalid options).
Signed-off-by: Scott Wood
---
v2: non-RFC
drivers/cpufreq/qoriq-cpufreq.c | 92 +
1 file changed, 2 insertions(+), 90 deletions(-)
diff --git a/drivers/cpufreq/qoriq-cpufreq.c b/drivers/cpufreq/qoriq-cpufreq.c
index 32ab99e..514395f 100644
--- a/drivers/cpufreq/qoriq-cpufreq.c
+++ b/drivers/cpufreq/qoriq-cpufreq.c
@@ -36,53 +36,6 @@ struct cpu_data {
struct cpufreq_frequency_table *table;
};
-/**
- * struct soc_data - SoC specific data
- * @freq_mask: mask the disallowed frequencies
- * @flag: unique flags
- */
-struct soc_data {
- u32 freq_mask[4];
- u32 flag;
-};
-
-#define FREQ_MASK 1
-/* see hardware specification for the allowed frqeuencies */
-static const struct soc_data sdata[] = {
- { /* used by p2041 and p3041 */
- .freq_mask = {0x8, 0x8, 0x2, 0x2},
- .flag = FREQ_MASK,
- },
- { /* used by p5020 */
- .freq_mask = {0x8, 0x2},
- .flag = FREQ_MASK,
- },
- { /* used by p4080, p5040 */
- .freq_mask = {0},
- .flag = 0,
- },
-};
-
-/*
- * the minimum allowed core frequency, in Hz
- * for chassis v1.0, >= platform frequency
- * for chassis v2.0, >= platform frequency / 2
- */
-static u32 min_cpufreq;
-static const u32 *fmask;
-
-#if defined(CONFIG_ARM)
-static int get_cpu_physical_id(int cpu)
-{
- return topology_core_id(cpu);
-}
-#else
-static int get_cpu_physical_id(int cpu)
-{
- return get_hard_smp_processor_id(cpu);
-}
-#endif
-
static u32 get_bus_freq(void)
{
struct device_node *soc;
@@ -195,7 +148,7 @@ static int qoriq_cpufreq_cpu_init(struct cpufreq_policy
*policy)
{
struct device_node *np, *pnode;
int i, count, ret;
- u32 freq, mask;
+ u32 freq;
struct clk *clk;
struct cpufreq_frequency_table *table;
struct cpu_data *data;
@@ -230,23 +183,11 @@ static int qoriq_cpufreq_cpu_init(struct cpufreq_policy
*policy)
goto err_pclk;
}
- if (fmask)
- mask = fmask[get_cpu_physical_id(cpu)];
- else
- mask = 0x0;
-
for (i = 0; i < count; i++) {
clk = clk_get_parent_by_index(policy->clk, i);
data->pclk[i] = clk;
freq = clk_get_rate(clk);
- /*
-* the clock is valid if its frequency is not masked
-* and large than minimum allowed frequency.
-*/
- if (freq < min_cpufreq || (mask & (1 << i)))
- table[i].frequency = CPUFREQ_ENTRY_INVALID;
- else
- table[i].frequency = freq / 1000;
+ table[i].frequency = freq / 1000;
table[i].driver_data = i;
}
freq_table_redup(table, count);
@@ -321,38 +262,9 @@ static struct cpufreq_driver qoriq_cpufreq_driver = {
.attr = cpufreq_generic_attr,
};
-static const struct of_device_id node_matches[] __initconst = {
- { .compatible = "fsl,p2041-clockgen", .data = &sdata[0], },
- { .compatible = "fsl,p3041-clockgen", .data = &sdata[0], },
- { .compatible = "fsl,p5020-clockgen", .data = &sdata[1], },
- { .compatible = "fsl,p4080-clockgen", .data = &sdata[2], },
- { .compatible = "fsl,p5040-clockgen", .data = &sdata[2], },
- { .compatible = "fsl,qoriq-clockgen-2.0", },
- {}
-};
-
static int __init qoriq_cpufreq_init(void)
{
int ret;
- struct device_node *np;
- const struct of_device_id *match;
- const struct soc_data *data;
-
- np = of_find_matching_node(NULL, node_matches);
- if (!np)
- return -ENODEV;
-
- match = of_match_node(node_matches, np);
- data = match->data;
- if (data) {
- if (data->flag)
- fmask = data->freq_mask;
- min_cpufreq = get_bus_freq();
- } else {
- min_cpufreq = get_bus_freq() / 2;
- }
-
- of_node_put(np);
ret = cpufreq_register_driver(&qoriq_cpufreq_driver);
if (!ret)
--
2.1.4
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v2 5/5] powerpc/fsl: Use new clockgen binding
The driver retains compatibility with old device trees, but we don't
want the old nodes lying around to be copied, or used as a reference
(some of the mux options are incorrect), or even just being clutter.
We will also need the #clock-cells in the clockgen node in order to
add fman nodes.
Signed-off-by: Scott Wood
---
v2: non-RFC
arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi | 4 +-
arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi | 8 +--
arch/powerpc/boot/dts/fsl/b4si-post.dtsi | 15 --
arch/powerpc/boot/dts/fsl/p2041si-post.dtsi| 18 ---
arch/powerpc/boot/dts/fsl/p2041si-pre.dtsi | 8 +--
arch/powerpc/boot/dts/fsl/p3041si-post.dtsi| 18 ---
arch/powerpc/boot/dts/fsl/p3041si-pre.dtsi | 8 +--
arch/powerpc/boot/dts/fsl/p4080si-post.dtsi| 70 --
arch/powerpc/boot/dts/fsl/p4080si-pre.dtsi | 16 +++---
arch/powerpc/boot/dts/fsl/p5020si-pre.dtsi | 4 +-
arch/powerpc/boot/dts/fsl/p5040si-post.dtsi| 18 ---
arch/powerpc/boot/dts/fsl/p5040si-pre.dtsi | 8 +--
arch/powerpc/boot/dts/fsl/qoriq-clockgen1.dtsi | 50 +-
arch/powerpc/boot/dts/fsl/qoriq-clockgen2.dtsi | 33 +---
arch/powerpc/boot/dts/fsl/t1023si-post.dtsi| 16 --
arch/powerpc/boot/dts/fsl/t102xsi-pre.dtsi | 4 +-
arch/powerpc/boot/dts/fsl/t1040si-post.dtsi| 44
arch/powerpc/boot/dts/fsl/t104xsi-pre.dtsi | 8 +--
arch/powerpc/boot/dts/fsl/t2081si-post.dtsi| 22
arch/powerpc/boot/dts/fsl/t208xsi-pre.dtsi | 8 +--
arch/powerpc/boot/dts/fsl/t4240si-post.dtsi| 61 --
arch/powerpc/boot/dts/fsl/t4240si-pre.dtsi | 24 -
22 files changed, 54 insertions(+), 411 deletions(-)
diff --git a/arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi
b/arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi
index 338af7e..9cfeaef 100644
--- a/arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi
+++ b/arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi
@@ -64,14 +64,14 @@
cpu0: PowerPC,e6500@0 {
device_type = "cpu";
reg = <0 1>;
- clocks = <&mux0>;
+ clocks = <&clockgen 1 0>;
next-level-cache = <&L2>;
fsl,portid-mapping = <0x8000>;
};
cpu1: PowerPC,e6500@2 {
device_type = "cpu";
reg = <2 3>;
- clocks = <&mux0>;
+ clocks = <&clockgen 1 0>;
next-level-cache = <&L2>;
fsl,portid-mapping = <0x8000>;
};
diff --git a/arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi
b/arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi
index 1948f73..bc914f2 100644
--- a/arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi
+++ b/arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi
@@ -64,28 +64,28 @@
cpu0: PowerPC,e6500@0 {
device_type = "cpu";
reg = <0 1>;
- clocks = <&mux0>;
+ clocks = <&clockgen 1 0>;
next-level-cache = <&L2>;
fsl,portid-mapping = <0x8000>;
};
cpu1: PowerPC,e6500@2 {
device_type = "cpu";
reg = <2 3>;
- clocks = <&mux0>;
+ clocks = <&clockgen 1 0>;
next-level-cache = <&L2>;
fsl,portid-mapping = <0x8000>;
};
cpu2: PowerPC,e6500@4 {
device_type = "cpu";
reg = <4 5>;
- clocks = <&mux0>;
+ clocks = <&clockgen 1 0>;
next-level-cache = <&L2>;
fsl,portid-mapping = <0x8000>;
};
cpu3: PowerPC,e6500@6 {
device_type = "cpu";
reg = <6 7>;
- clocks = <&mux0>;
+ clocks = <&clockgen 1 0>;
next-level-cache = <&L2>;
fsl,portid-mapping = <0x8000>;
};
diff --git a/arch/powerpc/boot/dts/fsl/b4si-post.dtsi
b/arch/powerpc/boot/dts/fsl/b4si-post.dtsi
index 603910a..004477d 100644
--- a/arch/powerpc/boot/dts/fsl/b4si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/b4si-post.dtsi
@@ -398,21 +398,6 @@
};
/include/ "qoriq-clockgen2.dtsi"
- clockgen: global-utilities@e1000 {
- compatible = "fsl,b4-clockgen", "fsl,qoriq-clockgen-2.0";
- reg = <0xe1000 0x1000>;
-
- mux0: mux0@0 {
- #clock-cells = <0>;
- reg = <0x0 0x4>;
- compatible = "fsl,qoriq-core-mux-2.0";
- clocks = <&pll0 0>, <&pll0 1>, <&pll0 2>,
Re: [RFC PATCH 0/8] clk: qoriq: Move chip-specific knowledge into driver
On Tue, 2015-08-11 at 11:25 -0700, Michael Turquette wrote: > Hi Scott, > > Quoting Scott Wood (2015-06-18 19:49:10) > > The existing device tree bindings are error-prone and inflexible. > > Correct the mistake by moving the knowledge into the driver, which > > has more flexibility in describing the quirks of each chip. This leaves > > the device tree to its proper role of identifying a programming interface > > rather than describing its individual registers. > > Sorry for not responding to this one sooner. Fell through the cracks. > > All of the changes to drives/clk/clk-qoriq.c look great to me. I assume > you need to keep all of these patches together and want to the take > through the freescale tree? If so feel free to add, > > Acked-by: Michael Turquette I just sent a non-RFC v2, with improved compatibility with old device trees (especially ls1021a). It depends on the cpufreq patch though (at least, to avoid breaking qoriq-cpufreq until that patch is merged), so I'll also need an ack from Rafael for that if I'm taking it through my tree. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
