* [PATCH 1/2] powerpc: Fix warnings for arch/powerpc/mm/numa.c
From: Robert C Jennings @ 2013-10-25 19:25 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Robert C Jennings
In-Reply-To: <1382729107-13560-1-git-send-email-rcj@linux.vnet.ibm.com>
Simple fixes for sparse warnings in this file.
Resolves:
arch/powerpc/mm/numa.c:198:24:
warning: Using plain integer as NULL pointer
arch/powerpc/mm/numa.c:1157:5:
warning: symbol 'hot_add_node_scn_to_nid' was not declared.
Should it be static?
arch/powerpc/mm/numa.c:1238:28:
warning: Using plain integer as NULL pointer
arch/powerpc/mm/numa.c:1538:6:
warning: symbol 'topology_schedule_update' was not declared.
Should it be static?
Signed-off-by: Robert C Jennings <rcj@linux.vnet.ibm.com>
---
arch/powerpc/mm/numa.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index c916127..33d6784 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -195,7 +195,7 @@ static const __be32 *of_get_usable_memory(struct device_node *memory)
u32 len;
prop = of_get_property(memory, "linux,drconf-usable-memory", &len);
if (!prop || len < sizeof(unsigned int))
- return 0;
+ return NULL;
return prop;
}
@@ -1154,7 +1154,7 @@ static int hot_add_drconf_scn_to_nid(struct device_node *memory,
* represented in the device tree as a node (i.e. memory@XXXX) for
* each memblock.
*/
-int hot_add_node_scn_to_nid(unsigned long scn_addr)
+static int hot_add_node_scn_to_nid(unsigned long scn_addr)
{
struct device_node *memory;
int nid = -1;
@@ -1235,7 +1235,7 @@ static u64 hot_add_drconf_memory_max(void)
struct device_node *memory = NULL;
unsigned int drconf_cell_cnt = 0;
u64 lmb_size = 0;
- const __be32 *dm = 0;
+ const __be32 *dm = NULL;
memory = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
if (memory) {
@@ -1535,7 +1535,7 @@ static void topology_work_fn(struct work_struct *work)
}
static DECLARE_WORK(topology_work, topology_work_fn);
-void topology_schedule_update(void)
+static void topology_schedule_update(void)
{
schedule_work(&topology_work);
}
--
1.8.1.2
^ permalink raw reply related
* [PATCH 0/2] powerpc: Sparse fixes for numa.c
From: Robert C Jennings @ 2013-10-25 19:25 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Robert C Jennings
Cleaning out some stashed fixes for sparse warnings found while
working on 3be7db6: (powerpc: VPHN topology change updates all siblings)
I don't see a reason why the declarations in arch/powerpc/kernel/setup.h
can't live in arch/powerpc/include/asm/setup.h and .../mm/numa.c
should include these definitions.
Robert C Jennings (2):
powerpc: Fix warnings for arch/powerpc/mm/numa.c
powerpc: Move local setup.h declarations to arch includes
arch/powerpc/include/asm/setup.h | 5 +++++
arch/powerpc/kernel/module.c | 3 +--
arch/powerpc/kernel/module_32.c | 3 +--
arch/powerpc/kernel/module_64.c | 3 +--
arch/powerpc/kernel/setup-common.c | 2 --
arch/powerpc/kernel/setup.h | 9 ---------
arch/powerpc/kernel/setup_32.c | 2 --
arch/powerpc/kernel/setup_64.c | 2 --
arch/powerpc/kernel/vdso.c | 3 +--
arch/powerpc/mm/numa.c | 8 ++++----
10 files changed, 13 insertions(+), 27 deletions(-)
delete mode 100644 arch/powerpc/kernel/setup.h
--
1.8.1.2
^ permalink raw reply
* Re: perf events ring buffer memory barrier on powerpc
From: Peter Zijlstra @ 2013-10-25 17:37 UTC (permalink / raw)
To: Frederic Weisbecker
Cc: Michael Neuling, Mathieu Desnoyers, linux-kernel, Linux PPC dev,
anton, Victor Kaplansky
In-Reply-To: <20131023141948.GB3566@localhost.localdomain>
On Wed, Oct 23, 2013 at 03:19:51PM +0100, Frederic Weisbecker wrote:
> On Wed, Oct 23, 2013 at 10:54:54AM +1100, Michael Neuling wrote:
> > Frederic,
> >
> > The comment says atomic_dec_and_test() but the code is
> > local_dec_and_test().
> >
> > On powerpc, local_dec_and_test() doesn't have a memory barrier but
> > atomic_dec_and_test() does. Is the comment wrong, or is
> > local_dec_and_test() suppose to imply a memory barrier too and we have
> > it wrongly implemented in powerpc?
My bad; I converted from atomic to local without actually thinking it
seems. Your implementation of the local primitives is fine.
> > diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
> > index cd55144..95768c6 100644
> > --- a/kernel/events/ring_buffer.c
> > +++ b/kernel/events/ring_buffer.c
> > @@ -87,10 +87,10 @@ again:
> > goto out;
> >
> > /*
> > - * Publish the known good head. Rely on the full barrier implied
> > - * by atomic_dec_and_test() order the rb->head read and this
> > - * write.
> > + * Publish the known good head. We need a memory barrier to order the
> > + * order the rb->head read and this write.
> > */
> > + smp_mb ();
> > rb->user_page->data_head = head;
> >
> > /*
Right; so that would indeed be what the comment suggests it should be.
However I think the comment is now actively wrong too :-)
Since on the kernel side the buffer is strictly per-cpu, we don't need
memory barriers there.
> I think we want this ordering:
>
> Kernel User
>
> READ rb->user_page->data_tail READ rb->user_page->data_head
> smp_mb() smp_mb()
> WRITE rb data READ rb data
> smp_mb() smp_mb()
> rb->user_page->data_head WRITE rb->user_page->data_tail
>
I would argue for:
READ ->data_tail READ ->data_head
smp_rmb() (A) smp_rmb() (C)
WRITE $data READ $data
smp_wmb() (B) smp_mb() (D)
STORE ->data_head WRITE ->data_tail
Where A pairs with D, and B pairs with C.
I don't think A needs to be a full barrier because we won't in fact
write data until we see the store from userspace. So we simply don't
issue the data WRITE until we observe it.
OTOH, D needs to be a full barrier since it separates the data READ from
the tail WRITE.
For B a WMB is sufficient since it separates two WRITEs, and for C an
RMB is sufficient since it separates two READs.
---
kernel/events/ring_buffer.c | 29 ++++++++++++++++++++++++++---
1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
index cd55144270b5..c91274ef4e23 100644
--- a/kernel/events/ring_buffer.c
+++ b/kernel/events/ring_buffer.c
@@ -87,10 +87,31 @@ static void perf_output_put_handle(struct perf_output_handle *handle)
goto out;
/*
- * Publish the known good head. Rely on the full barrier implied
- * by atomic_dec_and_test() order the rb->head read and this
- * write.
+ * Since the mmap() consumer (userspace) can run on a different CPU:
+ *
+ * kernel user
+ *
+ * READ ->data_tail READ ->data_head
+ * smp_rmb() (A) smp_rmb() (C)
+ * WRITE $data READ $data
+ * smp_wmb() (B) smp_mb() (D)
+ * STORE ->data_head WRITE ->data_tail
+ *
+ * Where A pairs with D, and B pairs with C.
+ *
+ * I don't think A needs to be a full barrier because we won't in fact
+ * write data until we see the store from userspace. So we simply don't
+ * issue the data WRITE until we observe it.
+ *
+ * OTOH, D needs to be a full barrier since it separates the data READ
+ * from the tail WRITE.
+ *
+ * For B a WMB is sufficient since it separates two WRITEs, and for C
+ * an RMB is sufficient since it separates two READs.
+ *
+ * See perf_output_begin().
*/
+ smp_wmb();
rb->user_page->data_head = head;
/*
@@ -154,6 +175,8 @@ int perf_output_begin(struct perf_output_handle *handle,
* Userspace could choose to issue a mb() before updating the
* tail pointer. So that all reads will be completed before the
* write is issued.
+ *
+ * See perf_output_put_handle().
*/
tail = ACCESS_ONCE(rb->user_page->data_tail);
smp_rmb();
^ permalink raw reply related
* Re: [PATCH] [RFC] Emulate "lwsync" to run standard user land on e500 cores
From: James Yang @ 2013-10-25 15:25 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, Wolfgang Denk
In-Reply-To: <1382697373.3926.36.camel@aoeu.buserror.net>
On Fri, 25 Oct 2013, Scott Wood wrote:
> Has anyone measured how much this slows things down with a typical
> userspace?
Not a measurement of the patch in question but an older similar patch
on 3.0.51 (8572 running Debian 5.0.3):
$ ./test-lwsync
cycles per emulated lwsync = 371
cycles per sync = 36
----------------------------------------------------------
#include <stdio.h>
int main (void) {
unsigned long atb_start, atb_stop;
unsigned long i;
asm volatile ("mfspr %0,526" : "=r" (atb_start));
for (i = 0; i < 100; i++) {
asm volatile ("lwsync");
}
asm volatile ("mfspr %0,526" : "=r" (atb_stop));
printf("cycles per emulated lwsync = %lu\n", (atb_stop -
atb_start) / 100);
asm volatile ("mfspr %0,526" : "=r" (atb_start));
for (i = 0; i < 100; i++) {
asm volatile ("sync");
}
asm volatile ("mfspr %0,526" : "=r" (atb_stop));
printf("cycles per sync = %lu\n", (atb_stop - atb_start) /
100);
return 0;
}
----------------------------------------------------------
^ permalink raw reply
* RE: [PATCH] [RFC] Emulate "lwsync" to run standard user land on e500 cores
From: James Yang @ 2013-10-25 15:13 UTC (permalink / raw)
To: David Laight; +Cc: linuxppc-dev
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B73B1@saturn3.aculab.com>
On Fri, 25 Oct 2013, David Laight wrote:
> > This is not a distro issue. It's a libstdc++ portability issue. libstdc++
> > hardcodes lwsync unless __NO_LWSYNC__ is explicitly defined,
> > which you only get with -mcpu=8540/-mcpu=8548. When compiled
> > for any powerpc target other than -mcpu=8540/-mcpu=8548, including
> > the default -mcpu=common, libstdc++ will end up containing lwsync.
> > There is no way to explicitly request libstdc++ to be built without lwsync
> > with an -mcpu target other than 8540/8548.
> >
> > The issue is easily demonstrated by running a program that throws a
> > C++ exception: __cxa_throw() is called, which has an lwsync. This
> > results in an illegal instruction exception when run on an e500v1/e500v2.
>
> Perhaps libstc++ should be working out at run time whether lwsync is
> valid?
lwsync has been in libstdc++ for over 8 years so there's a substantial
amount of legacy binary code that exists such that changing libstdc++
now won't solve the problem.
This isn't a performance issue, it's a functional issue -- libstdc++
for -mcpu=common targets doesn't work on e500v1/e500v2. (QorIQ P4080
that have e500mc and newer cores support lwsync so this is no longer
an issue.)
If one /really/ cared about losing performance while running common
target binaries, emulating lwsync is an inconsequential performance
loss compared to the kernel doing floating point emulation.
Recompiling libstdc++ (and all your other userland) with -mcpu=8548 is
what you'd have to do to avoid classic FP and use Embedded FP or SPE,
and if you are doing that you'll get sync instead of lwsync in
libstdc++.
^ permalink raw reply
* Re: [PATCH 3/3] sched: Aggressive balance in domains whose groups share package resources
From: Preeti U Murthy @ 2013-10-25 13:19 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Michael Neuling, Mike Galbraith, Paul Turner, linux-kernel,
Anton Blanchard, linuxppc-dev, Ingo Molnar
In-Reply-To: <20131022222326.GL2490@laptop.programming.kicks-ass.net>
Hi Peter,
On 10/23/2013 03:53 AM, Peter Zijlstra wrote:
> On Mon, Oct 21, 2013 at 05:15:02PM +0530, Vaidyanathan Srinivasan wrote:
>> kernel/sched/fair.c | 18 ++++++++++++++++++
>> 1 file changed, 18 insertions(+)
>>
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index 828ed97..bbcd96b 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -5165,6 +5165,8 @@ static int load_balance(int this_cpu, struct rq *this_rq,
>> {
>> int ld_moved, cur_ld_moved, active_balance = 0;
>> struct sched_group *group;
>> + struct sched_domain *child;
>> + int share_pkg_res = 0;
>> struct rq *busiest;
>> unsigned long flags;
>> struct cpumask *cpus = __get_cpu_var(load_balance_mask);
>> @@ -5190,6 +5192,10 @@ static int load_balance(int this_cpu, struct rq *this_rq,
>>
>> schedstat_inc(sd, lb_count[idle]);
>>
>> + child = sd->child;
>> + if (child && child->flags & SD_SHARE_PKG_RESOURCES)
>> + share_pkg_res = 1;
>> +
>> redo:
>> if (!should_we_balance(&env)) {
>> *continue_balancing = 0;
>> @@ -5202,6 +5208,7 @@ redo:
>> goto out_balanced;
>> }
>>
>> +redo_grp:
>> busiest = find_busiest_queue(&env, group);
>> if (!busiest) {
>> schedstat_inc(sd, lb_nobusyq[idle]);
>> @@ -5292,6 +5299,11 @@ more_balance:
>> if (!cpumask_empty(cpus)) {
>> env.loop = 0;
>> env.loop_break = sched_nr_migrate_break;
>> + if (share_pkg_res &&
>> + cpumask_intersects(cpus,
>> + to_cpumask(group->cpumask)))
>
> sched_group_cpus()
>
>> + goto redo_grp;
>> +
>> goto redo;
>> }
>> goto out_balanced;
>> @@ -5318,9 +5330,15 @@ more_balance:
>> */
>> if (!cpumask_test_cpu(this_cpu,
>> tsk_cpus_allowed(busiest->curr))) {
>> + cpumask_clear_cpu(cpu_of(busiest), cpus);
>> raw_spin_unlock_irqrestore(&busiest->lock,
>> flags);
>> env.flags |= LBF_ALL_PINNED;
>> + if (share_pkg_res &&
>> + cpumask_intersects(cpus,
>> + to_cpumask(group->cpumask)))
>> + goto redo_grp;
>> +
>> goto out_one_pinned;
>> }
>
> Man this retry logic is getting annoying.. isn't there anything saner we
> can do?
Maybe we can do this just at the SIBLINGS level? Having the hyper
threads busy due to the scenario described in the changelog is bad for
performance.
Regards
Preeti U Murthy
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
^ permalink raw reply
* Re: [PATCH] [RFC] Emulate "lwsync" to run standard user land on e500 cores
From: Benjamin Herrenschmidt @ 2013-10-25 13:02 UTC (permalink / raw)
To: David Laight; +Cc: Yang James-RA8135, linuxppc-dev
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B73B1@saturn3.aculab.com>
On Fri, 2013-10-25 at 10:58 +0100, David Laight wrote:
> > This is not a distro issue. It's a libstdc++ portability issue. libstdc++
> > hardcodes lwsync unless __NO_LWSYNC__ is explicitly defined,
> > which you only get with -mcpu=8540/-mcpu=8548. When compiled
> > for any powerpc target other than -mcpu=8540/-mcpu=8548, including
> > the default -mcpu=common, libstdc++ will end up containing lwsync.
> > There is no way to explicitly request libstdc++ to be built without lwsync
> > with an -mcpu target other than 8540/8548.
> >
> > The issue is easily demonstrated by running a program that throws a
> > C++ exception: __cxa_throw() is called, which has an lwsync. This
> > results in an illegal instruction exception when run on an e500v1/e500v2.
>
> Perhaps libstc++ should be working out at run time whether lwsync is valid?
Do we have enough coats of paint on this bike shed yet ? :-)
I'm personally tempted to take Scott's approach since that's what we do
for other things as well, it just works and is simple.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH] [RFC] Emulate "lwsync" to run standard user land on e500 cores
From: Scott Wood @ 2013-10-25 10:36 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev, Wolfgang Denk
In-Reply-To: <6BFC8EB0-1A75-41C3-985A-E3ED14846710@kernel.crashing.org>
On Thu, 2013-10-24 at 04:55 -0500, Kumar Gala wrote:
> On Oct 24, 2013, at 4:45 AM, Benjamin Herrenschmidt wrote:
>
> > On Wed, 2013-10-23 at 23:06 -0500, Kumar Gala wrote:
> >> On Oct 23, 2013, at 5:15 AM, Scott Wood wrote:
> >>
> >>> On Wed, 2013-10-23 at 00:07 -0500, Kumar Gala wrote:
> >>>> On Oct 18, 2013, at 2:38 AM, Wolfgang Denk wrote:
> >>>>> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> >>>>> index f783c93..f330374 100644
> >>>>> --- a/arch/powerpc/kernel/traps.c
> >>>>> +++ b/arch/powerpc/kernel/traps.c
> >>>>> @@ -986,6 +986,13 @@ static int emulate_instruction(struct pt_regs *regs)
> >>>>> return 0;
> >>>>> }
> >>>>>
> >>>>> + /* Emulating the lwsync insn as a sync insn */
> >>>>> + if (instword == PPC_INST_LWSYNC) {
> >>>>> + PPC_WARN_EMULATED(lwsync, regs);
> >>>>> + asm volatile("sync" : : : "memory");
> >>>>
> >>>> Do we really need the inline asm? Doesn't the fact of just taking an exception and returning from it equate to a sync.
> >>>
> >>> No, it doesn't equate to a sync. See the discussion here:
> >>> http://patchwork.ozlabs.org/patch/256747/
> >>>
> >>
> >> Thanks.
> >>
> >> I'm not sure I'm a fan of doing this as it silently hides a significant performance impact.
> >>
> >> Could we possible re-write the userspace instruction to be a 'sync' when we hit this?
> >
> > Rewriting user space is a can of worms I wouldn't get into ... is any
> > other arch doing it ?
>
> Fair enough
> >
> > I'm not too worried as long as we warn and account them.
>
> Than, I'd ask this be under a Kconfig option that is disabled by
> default. Users should have to explicitly enable this so they know what
> they are doing.
Why should this be any different than the other emulated instructions,
which are generally either not kconfigized, or on by default in some
configs (like fp emu)?
Making sure users are aware of this is what PPC_WARN_EMULATED is for.
Has anyone measured how much this slows things down with a typical
userspace?
-Scott
^ permalink raw reply
* RE: [PATCH] [RFC] Emulate "lwsync" to run standard user land on e500 cores
From: David Laight @ 2013-10-25 9:58 UTC (permalink / raw)
To: Yang James-RA8135, Kumar Gala; +Cc: linuxppc-dev
In-Reply-To: <CE506A05CAAF2A42B6F5840E6093C92D08895AC8@039-SN1MPN1-005.039d.mgd.msft.net>
> This is not a distro issue. It's a libstdc++ portability issue. =
libstdc++
> hardcodes lwsync unless __NO_LWSYNC__ is explicitly defined,
> which you only get with -mcpu=3D8540/-mcpu=3D8548. When compiled
> for any powerpc target other than -mcpu=3D8540/-mcpu=3D8548, including
> the default -mcpu=3Dcommon, libstdc++ will end up containing lwsync.
> There is no way to explicitly request libstdc++ to be built without =
lwsync
> with an -mcpu target other than 8540/8548.
>=20
> The issue is easily demonstrated by running a program that throws a
> C++ exception: __cxa_throw() is called, which has an lwsync. This
> results in an illegal instruction exception when run on an =
e500v1/e500v2.
Perhaps libstc++ should be working out at run time whether lwsync is =
valid?
David
^ permalink raw reply
* RE: [PATCH V4 3/3] powerpc/85xx: Add TWR-P1025 board support
From: Xie Xiaobo-R63061 @ 2013-10-25 9:49 UTC (permalink / raw)
To: Wood Scott-B07421; +Cc: Johnston Michael-R49610, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1380301430.24959.399.camel@snotra.buserror.net>
SGkgU2NvdHQsDQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogV29vZCBT
Y290dC1CMDc0MjENCj4gU2VudDogU2F0dXJkYXksIFNlcHRlbWJlciAyOCwgMjAxMyAxOjA0IEFN
DQo+IFRvOiBYaWUgWGlhb2JvLVI2MzA2MQ0KPiBDYzogV29vZCBTY290dC1CMDc0MjE7IGxpbnV4
cHBjLWRldkBsaXN0cy5vemxhYnMub3JnOyBKb2huc3RvbiBNaWNoYWVsLQ0KPiBSNDk2MTANCj4g
U3ViamVjdDogUmU6IFtQQVRDSCBWNCAzLzNdIHBvd2VycGMvODV4eDogQWRkIFRXUi1QMTAyNSBi
b2FyZCBzdXBwb3J0DQo+IA0KPiBPbiBXZWQsIDIwMTMtMDktMjUgYXQgMDQ6NTAgLTA1MDAsIFhp
ZSBYaWFvYm8tUjYzMDYxIHdyb3RlOg0KPiA+ID4gPiArI2lmIGRlZmluZWQoQ09ORklHX1NFUklB
TF9RRSkNCj4gPiA+ID4gKwkJCS8qIE9uIFAxMDI1VFdSIGJvYXJkLCB0aGUgVUNDNyBhY3RlZCBh
cyBVQVJUIHBvcnQuDQo+ID4gPiA+ICsJCQkgKiBIb3dldmVyLCBUaGUgVUNDNydzIENUUyBwaW4g
aXMgbG93IGxldmVsIGluDQo+IGRlZmF1bHQsDQo+ID4gPiA+ICsJCQkgKiBpdCB3aWxsIGltcGFj
dCB0aGUgdHJhbnNtaXNzaW9uIGluIGZ1bGwgZHVwbGV4DQo+ID4gPiA+ICsJCQkgKiBjb21tdW5p
Y2F0aW9uLiBTbyBkaXNhYmxlIHRoZSBGbG93IGNvbnRyb2wgcGluDQo+IFBBMTguDQo+ID4gPiA+
ICsJCQkgKiBUaGUgVUNDNyBVQVJUIGp1c3QgY2FuIHVzZSBSWEQgYW5kIFRYRCBwaW5zLg0KPiA+
ID4gPiArCQkJICovDQo+ID4gPiA+ICsJCQlwYXJfaW9fY29uZmlnX3BpbigwLCAxOCwgMCwgMCwg
MCwgMCk7ICNlbmRpZg0KPiA+ID4NCj4gPiA+IEFueSByZWFzb24gbm90IHRvIGRvIHRoaXMgdW5j
b25kaXRpb25hbGx5Pw0KPiA+DQo+ID4gVGhpcyBpcyBhIGJvYXJkIGlzc3VlLCB0aGUgY29kZSBh
bHJlYWR5IGhhdmUgYSBjb25kaXRpb24gLSBkZWZpbmVkDQo+ID4gU0VSSUFMX1FFLCBhbmQgSSB3
aWxsIGFkZCBhIGNvbmRpdGlvbiAiaWYgKG1hY2hpbmVfaXModHdyX3AxMDI1KSkiLg0KPiANCj4g
TXkgcG9pbnQgd2FzLCBpcyB0aGVyZSBhbnkgaGFybSBpbiBkb2luZyB0aGlzIHdpdGhvdXQgcmVn
YXJkIHRvDQo+IENPTkZJR19TRVJJQUxfUUU/DQoNClRoZSBDVFMgcGluIHdpbGwgYmUgdGhlIHBp
biBvZiBwcm9maWJ1cyBpZiBhZGQgYSBwcm9maWJ1cyBleHBhbnNpb24gYm9hcmQgaW4gVFdSIHN5
c3RlbS4gSWYgZGlzYWJsZSB0aGUgcGluIHVuY29uZGl0aW9uYWxseSwgaXQgd2lsbCBhZmZlY3Qg
dGhlIGZ1bmN0aW9uIG9mIHByb2ZpYnVzLiAgDQoNCi1YaWUgWGlhb2JvDQo+IA0KPiAtU2NvdHQN
Cj4gDQoNCg==
^ permalink raw reply
* [PATCH] mv643xx_eth: fix return value check in mv64x60_eth_register_shared_pdev()
From: Wei Yongjun @ 2013-10-25 9:30 UTC (permalink / raw)
To: benh, paulus, grant.likely, rob.herring, davem, florian
Cc: yongjun_wei, linuxppc-dev
From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
In case of error, the function platform_device_register_simple() returns
RR_PTR() and never returns NULL. The NULL test in the return value check
should be replaced with IS_ERR().
Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
arch/powerpc/sysdev/mv64x60_dev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/sysdev/mv64x60_dev.c b/arch/powerpc/sysdev/mv64x60_dev.c
index 4a25c26..a3a8fad 100644
--- a/arch/powerpc/sysdev/mv64x60_dev.c
+++ b/arch/powerpc/sysdev/mv64x60_dev.c
@@ -228,7 +228,7 @@ static struct platform_device * __init mv64x60_eth_register_shared_pdev(
if (id == 0) {
pdev = platform_device_register_simple("orion-mdio", -1, &r[1], 1);
- if (!pdev)
+ if (IS_ERR(pdev))
return pdev;
}
^ permalink raw reply related
* [PATCH] ASoC: fsl_spdif: fix return value check in fsl_spdif_probe()
From: Wei Yongjun @ 2013-10-25 9:29 UTC (permalink / raw)
To: timur, lgirdwood, broonie, perex, tiwai, grant.likely,
rob.herring
Cc: yongjun_wei, linuxppc-dev, alsa-devel
From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
In case of error, the function platform_get_resource() returns NULL
pointer not ERR_PTR(). The IS_ERR() test in the return value check
should be replaced with NULL test.
Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
sound/soc/fsl/fsl_spdif.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/fsl/fsl_spdif.c b/sound/soc/fsl/fsl_spdif.c
index 3920c3e..e5bfafe 100644
--- a/sound/soc/fsl/fsl_spdif.c
+++ b/sound/soc/fsl/fsl_spdif.c
@@ -1107,9 +1107,9 @@ static int fsl_spdif_probe(struct platform_device *pdev)
/* Get the addresses and IRQ */
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (IS_ERR(res)) {
+ if (!res) {
dev_err(&pdev->dev, "could not determine device resources\n");
- return PTR_ERR(res);
+ return -ENXIO;
}
regs = devm_ioremap_resource(&pdev->dev, res);
^ permalink raw reply related
* Re: [PATCH v2 08/10] of/platform: Resolve interrupt references at probe time
From: Thierry Reding @ 2013-10-25 7:35 UTC (permalink / raw)
To: Grant Likely
Cc: devicetree, Russell King, linux-mips, Greg Kroah-Hartman,
linux-kernel, Ralf Baechle, Rob Herring, sparclinux,
Thomas Gleixner, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20131024163749.68D01C403B6@trevor.secretlab.ca>
[-- Attachment #1: Type: text/plain, Size: 5592 bytes --]
On Thu, Oct 24, 2013 at 05:37:49PM +0100, Grant Likely wrote:
> On Wed, 16 Oct 2013 00:24:36 +0100, Grant Likely <grant.likely@linaro.org> wrote:
> > On Wed, 18 Sep 2013 15:24:50 +0200, Thierry Reding <thierry.reding@gmail.com> wrote:
> > > Interrupt references are currently resolved very early (when a device is
> > > created). This has the disadvantage that it will fail in cases where the
> > > interrupt parent hasn't been probed and no IRQ domain for it has been
> > > registered yet. To work around that various drivers use explicit
> > > initcall ordering to force interrupt parents to be probed before devices
> > > that need them are created. That's error prone and doesn't always work.
> > > If a platform device uses an interrupt line connected to a different
> > > platform device (such as a GPIO controller), both will be created in the
> > > same batch, and the GPIO controller won't have been probed by its driver
> > > when the depending platform device is created. Interrupt resolution will
> > > fail in that case.
> >
> > What is the reason for all the rework on the irq parsing return values?
> > A return value of '0' is always an error on irq parsing, regardless of
> > architecture even if NO_IRQ is defined as -1. I may have missed it, but
> > I don't see any checking for specific error values in the return paths
> > of the functions.
> >
> > If the specific return value isn't required (and I don't think it is),
> > then you can simplify the whole series by getting rid of the rework
> > patches.
>
> I've not heard back about the above, but I've just had a conversation
> with Rob about what to do here.
I thought I had sent a reply regarding this about a week ago. Perhaps it
got lost. I'll resend.
> The problem that I have is that it makes a specific return code need
> to traverse several levels of function calls and have a meaning come
> out the other end. It becomes difficult to figure out where that code
> actually comes from when reading the code. That's more of a gut-feel
> reaction rather than pointing out specifics though.
To be honest, I'm not all that happy with that aspect myself, but at the
same time I didn't feel like duplicating a lot of code to get this done
more easily. I imagine that would've caused significant pushback as
well. It's somewhat unfortunate that we have to propagate back through
several level, but that's just the way the code is currently written and
I don't think we can really get the information (EPROBE_DEFER) from any
other place but from the lowest level.
> The other thing that makes me nervous how invasive the series is.
Well, I guess that comes with the territory, doesn't it? Interrupts are
used in a large number of places and they have been used in a very
static manner so far. The end result of this patch series is that for
most devices instantiated from the device tree interrupts end up in the
same category as any other resources such as GPIOs, regulators or
clocks. They become mostly dynamic.
That in itself is a big change, so I don't think it's all that
surprising that the required changes are invasive.
And I think if we really want to solve it properly we need to make even
more invasive changes. For example, Grygorii pointed out that we could
have a setup in the future where the following happens:
1) driver providing interrupts is probed
2) driver using interrupts is probed, interrupt references are
resolved at probe time
3) both drivers are unloaded
4) both drivers are reloaded
In that case with the current set of patches the added core code assumes
that the interrupts have already been resolved and are still valid.
Possibly the easiest way to fix that would be to just zero out the
interrupt resources on remove so that they can be re-resolved on next
probe.
But that's somewhat cumbersome and it seems to me like a better fix
might be to go and change struct platform_device to not use a single
array of resources, but rather a list, or perhaps an array per type of
resource. The current platform_device structure is simple and easy, but
it doesn't work well with all the new dynamicity that we want/need/have
today.
Obviously modifying the innards of struct platform_device will likely
turn out to be a mammoth task of its own, but if that's what it takes
I'm prepared to do that as well. Or at least even try.
> However, even with saying all of the above, I'm not saying outright no.
> I want to get this feature in.
That's good to hear. Last time we talked about it we seemed to have an
agreement that this needs to be done, but you not replying had me
worried that perhaps you had changed your mind. It seems you've been
busy trying to address other issues that maybe are even more pressing so
I can hardly complain. =)
I'm good as long as we can keep moving in the right direction.
> It is obviously needed and I'll even merge the patches piecemeal as the
> look ready (I've already merged 2). Regardless, the current series needs
> to be reworked. It conflicts with the other IRQ rework that I've already
> put into my tree. The best thing to do would probably be respin it
> against my current tree and repost.
Sure, that won't be a problem. I might not get to it immediately, but
I'll get back to it.
> I'll take a fresh look then.... In the mean time, anything you can do to
> /sanely/ reduce the impact will probably help. :-)
I might be able to do that. But I'll mention that in another thread in
the right context.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 02/12][v3] pci: fsl: add structure fsl_pci
From: Lian Minghuan-b31939 @ 2013-10-25 5:58 UTC (permalink / raw)
To: Kumar Gala, Minghuan Lian
Cc: Arnd Bergmann, linux-pci, Zang Roy-R61911, Bjorn Helgaas,
Scott Wood, linuxppc-dev@lists.ozlabs.org list, linux-arm-kernel
In-Reply-To: <2B28E421-9B64-4CEC-9E00-8893502CE12F@kernel.crashing.org>
Hi Kumar,
please see my comment inline.
On 10/24/2013 12:11 PM, Kumar Gala wrote:
> On Oct 23, 2013, at 5:41 AM, Minghuan Lian wrote:
>
>> PowerPC uses structure pci_controller to describe PCI controller,
>> but ARM uses structure pci_sys_data. In order to support PowerPC
>> and ARM simultaneously, the patch adds a structure fsl_pci that
>> contains most of the members of the pci_controller and pci_sys_data.
>> Meanwhile, it defines a interface fsl_arch_sys_to_pci() which should
>> be implemented in architecture-specific PCI controller driver to
>> convert pci_controller or pci_sys_data to fsl_pci.
>>
>> Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
>> ---
>> change log:
>> v1-v3:
>> Derived from http://patchwork.ozlabs.org/patch/278965/
>>
>> Based on upstream master.
>> Based on the discussion of RFC version here
>> http://patchwork.ozlabs.org/patch/274487/
>>
>> include/linux/fsl/pci-common.h | 41 +++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 41 insertions(+)
> NAK.
>
> We discussed this some at the ARM Summit this week and the feeling is we need to move to a common interface between the various ARCHs.
[Minghuan] Do you mean we will use the common interface instead of
arch/powerpc/kernel/pci-common.c...
and arch/arm/kernel/bios32.c? Who will do the code movement and when
will the work be completed? The patches just move the common functions
of FSL PCI controller operation which can be re-used by PowerPC and ARM.
LS1 is coming, I worry about not having enough time to wait for the move
is completed.
> - k
>
>> diff --git a/include/linux/fsl/pci-common.h b/include/linux/fsl/pci-common.h
>> index 5e4f683..e56a040 100644
>> --- a/include/linux/fsl/pci-common.h
>> +++ b/include/linux/fsl/pci-common.h
>> @@ -102,5 +102,46 @@ struct ccsr_pci {
>>
>> };
>>
>> +/*
>> + * Structure of a PCI controller (host bridge)
>> + */
>> +struct fsl_pci {
>> + struct list_head node;
>> + bool is_pcie;
>> + struct device_node *dn;
>> + struct device *dev;
>> +
>> + int first_busno;
>> + int last_busno;
>> + int self_busno;
>> + struct resource busn;
>> +
>> + struct pci_ops *ops;
>> + struct ccsr_pci __iomem *regs;
>> +
>> + u32 indirect_type;
>> +
>> + struct resource io_resource;
>> + resource_size_t io_base_phys;
>> + resource_size_t pci_io_size;
>> +
>> + struct resource mem_resources[3];
>> + resource_size_t mem_offset[3];
>> +
>> + int global_number; /* PCI domain number */
>> +
>> + resource_size_t dma_window_base_cur;
>> + resource_size_t dma_window_size;
>> +
>> + void *sys;
>> +};
>> +
>> +/*
>> + * Convert architecture specific pci controller structure to fsl_pci
>> + * PowerPC uses structure pci_controller and ARM uses structure pci_sys_data
>> + * to describe pci controller.
>> + */
>> +extern struct fsl_pci *fsl_arch_sys_to_pci(void *sys);
>> +
>> #endif /* __PCI_COMMON_H */
>> #endif /* __KERNEL__ */
>> --
>> 1.8.1.2
>>
>
^ permalink raw reply
* RE: [PATCH] [RFC] Emulate "lwsync" to run standard user land on e500 cores
From: Yang James-RA8135 @ 2013-10-25 4:49 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <26976A96-8DF8-44EB-8B9B-E23200738F4E@kernel.crashing.org>
Sorry for the formatting. I'm not at my usual mailer.=0A=
=0A=
From: Kumar Gala:=0A=
>On Oct 24, 2013, at 4:05 PM, James Yang wrote:=0A=
>> On Thu, 24 Oct 2013, Kumar Gala wrote:=0A=
>>> Than, I'd ask this be under a Kconfig option that is disabled by=0A=
>>> default. Users should have to explicitly enable this so they know=0A=
>>> what they are doing.=0A=
>>=0A=
>>=0A=
>> I think it should be enabled by default, rather than disabled, so that=
=0A=
>> users would actually see a warning rather than get a sig 4. Or, let=0A=
>> it not be Kconfig-able so that this doesn't become a problem any more.=
=0A=
>> (It's been 4 years since I sent to you an earlier version of this=0A=
>> patch.)=0A=
>=0A=
>And clearly most users don't seem terrible annoyed enough about =0A=
>this issue to have concerns. I don't see why making it a Kconfig =0A=
>option impacts the small handful of people that happen to try =0A=
> and run a more generic distro on e500 cores.=0A=
=0A=
=0A=
I would have to dispute that qualification of "most". =0A=
=0A=
This is not a distro issue. It's a libstdc++ portability issue. libstdc++ =
=0A=
hardcodes lwsync unless __NO_LWSYNC__ is explicitly defined, =0A=
which you only get with -mcpu=3D8540/-mcpu=3D8548. When compiled =0A=
for any powerpc target other than -mcpu=3D8540/-mcpu=3D8548, including=0A=
the default -mcpu=3Dcommon, libstdc++ will end up containing lwsync. =0A=
There is no way to explicitly request libstdc++ to be built without lwsync=
=0A=
with an -mcpu target other than 8540/8548.=0A=
=0A=
The issue is easily demonstrated by running a program that throws a =0A=
C++ exception: __cxa_throw() is called, which has an lwsync. This=0A=
results in an illegal instruction exception when run on an e500v1/e500v2.=
=0A=
=0A=
Those users who insist on using a generic target for their compiler=0A=
will hit this problem, in particular, those who need to generate =0A=
binary code that targets a wide range of powerpc targets, such as=0A=
generic distro. It's unreasonable to require a user to recompile =0A=
libstdc++ just to get functionality for a C++ program, since they would=0A=
have to provide two libstdc++.so along with some dynamic-linking hacks to=
=0A=
determine at runtime which one should be used. Emulating lwsync =0A=
does not prevent an e500v1/e500v2-targeted libstdc++ from providing=0A=
the optimal performance.=0A=
=0A=
=0A=
citation:=0A=
=0A=
> > lwsync is embedded into the library unless __NO_LWSYNC__ is defined:=0A=
> > http://gcc.gnu.org/viewcvs/gcc/trunk/libstdc%2B%2B-v3/config/cpu/powerp=
c/atomic_word.h?revision=3D195701&view=3Dmarkup#l30=0A=
> > -----------------------------------------------=0A=
> > #define _GLIBCXX_READ_MEM_BARRIER __asm __volatile=0A=
> > ("isync":::"memory")=0A=
> > #ifdef __NO_LWSYNC__=0A=
> > #define _GLIBCXX_WRITE_MEM_BARRIER __asm __volatile=0A=
> > ("sync":::"memory")=0A=
> > #else=0A=
> > #define _GLIBCXX_WRITE_MEM_BARRIER __asm __volatile=0A=
> > ("lwsync":::"memory")=0A=
> > #endif=0A=
> > -----------------------------------------------=0A=
> > =0A=
> > =0A=
=0A=
^ permalink raw reply
* Re: [PATCH] [RFC] Emulate "lwsync" to run standard user land on e500 cores
From: Kumar Gala @ 2013-10-25 4:12 UTC (permalink / raw)
To: James Yang; +Cc: linuxppc-dev
In-Reply-To: <alpine.LRH.2.00.1310241556390.31001@ra8135-ec1.am.freescale.net>
On Oct 24, 2013, at 4:05 PM, James Yang wrote:
> On Thu, 24 Oct 2013, Kumar Gala wrote:
>=20
>> On Oct 24, 2013, at 4:45 AM, Benjamin Herrenschmidt wrote:
>>=20
>>> On Wed, 2013-10-23 at 23:06 -0500, Kumar Gala wrote:
>>>> On Oct 23, 2013, at 5:15 AM, Scott Wood wrote:
>>>>=20
>>>>> On Wed, 2013-10-23 at 00:07 -0500, Kumar Gala wrote:
>>>>>> On Oct 18, 2013, at 2:38 AM, Wolfgang Denk wrote:
>>>>>>> diff --git a/arch/powerpc/kernel/traps.c =
b/arch/powerpc/kernel/traps.c
>>>>>>> index f783c93..f330374 100644
>>>>>>> --- a/arch/powerpc/kernel/traps.c
>>>>>>> +++ b/arch/powerpc/kernel/traps.c
>>>>>>> @@ -986,6 +986,13 @@ static int emulate_instruction(struct =
pt_regs *regs)
>>>>>>> return 0;
>>>>>>> }
>>>>>>>=20
>>>>>>> + /* Emulating the lwsync insn as a sync insn */
>>>>>>> + if (instword =3D=3D PPC_INST_LWSYNC) {
>>>>>>> + PPC_WARN_EMULATED(lwsync, regs);
>>>>>>> + asm volatile("sync" : : : "memory");
>>>>>>=20
>>>>>> Do we really need the inline asm? Doesn't the fact of just =
taking an exception and returning from it equate to a sync.
>>>>>=20
>>>>> No, it doesn't equate to a sync. See the discussion here:
>>>>> http://patchwork.ozlabs.org/patch/256747/
>>>>>=20
>>>>=20
>>>> Thanks.=20
>>>>=20
>>>> I'm not sure I'm a fan of doing this as it silently hides a=20
>>>> significant performance impact.
>>>>=20
>>>> Could we possible re-write the userspace instruction to be a=20
>>>> 'sync' when we hit this?
>>>=20
>>> Rewriting user space is a can of worms I wouldn't get into ... is=20
>>> any other arch doing it ?
>>=20
>> Fair enough
>>>=20
>>> I'm not too worried as long as we warn and account them.
>>=20
>> Than, I'd ask this be under a Kconfig option that is disabled by=20
>> default. Users should have to explicitly enable this so they know=20
>> what they are doing.
>=20
>=20
> I think it should be enabled by default, rather than disabled, so that=20=
> users would actually see a warning rather than get a sig 4. Or, let=20=
> it not be Kconfig-able so that this doesn't become a problem any more.=20=
> (It's been 4 years since I sent to you an earlier version of this=20
> patch.)
And clearly most users don't seem terrible annoyed enough about this =
issue to have concerns. I don't see why making it a Kconfig option =
impacts the small handful of people that happen to try and run a more =
generic distro on e500 cores.
- k=
^ permalink raw reply
* [PATCH 1/2 v2] powerpc/e6500: Include Power ISA properties
From: Lijun Pan @ 2013-10-24 21:11 UTC (permalink / raw)
To: linuxppc-dev, scottwood; +Cc: Lijun Pan
b4420 and b4860 device trees do not have these properties.
Signed-off-by: Lijun Pan <Lijun.Pan@freescale.com>
---
v2: fixed a typo
arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi | 2 ++
arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi | 2 ++
2 files changed, 4 insertions(+)
diff --git a/arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi b/arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi
index 7b4426e..c6e451a 100644
--- a/arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi
+++ b/arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi
@@ -34,6 +34,8 @@
/dts-v1/;
+/include/ "e6500_power_isa.dtsi"
+
/ {
compatible = "fsl,B4420";
#address-cells = <2>;
diff --git a/arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi b/arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi
index 5263fa4..9bc26b1 100644
--- a/arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi
+++ b/arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi
@@ -34,6 +34,8 @@
/dts-v1/;
+/include/ "e6500_power_isa.dtsi"
+
/ {
compatible = "fsl,B4860";
#address-cells = <2>;
--
1.7.9.7
^ permalink raw reply related
* Re: [PATCH] [RFC] Emulate "lwsync" to run standard user land on e500 cores
From: James Yang @ 2013-10-24 21:05 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev
In-Reply-To: <6BFC8EB0-1A75-41C3-985A-E3ED14846710@kernel.crashing.org>
On Thu, 24 Oct 2013, Kumar Gala wrote:
> On Oct 24, 2013, at 4:45 AM, Benjamin Herrenschmidt wrote:
>
> > On Wed, 2013-10-23 at 23:06 -0500, Kumar Gala wrote:
> >> On Oct 23, 2013, at 5:15 AM, Scott Wood wrote:
> >>
> >>> On Wed, 2013-10-23 at 00:07 -0500, Kumar Gala wrote:
> >>>> On Oct 18, 2013, at 2:38 AM, Wolfgang Denk wrote:
> >>>>> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> >>>>> index f783c93..f330374 100644
> >>>>> --- a/arch/powerpc/kernel/traps.c
> >>>>> +++ b/arch/powerpc/kernel/traps.c
> >>>>> @@ -986,6 +986,13 @@ static int emulate_instruction(struct pt_regs *regs)
> >>>>> return 0;
> >>>>> }
> >>>>>
> >>>>> + /* Emulating the lwsync insn as a sync insn */
> >>>>> + if (instword == PPC_INST_LWSYNC) {
> >>>>> + PPC_WARN_EMULATED(lwsync, regs);
> >>>>> + asm volatile("sync" : : : "memory");
> >>>>
> >>>> Do we really need the inline asm? Doesn't the fact of just taking an exception and returning from it equate to a sync.
> >>>
> >>> No, it doesn't equate to a sync. See the discussion here:
> >>> http://patchwork.ozlabs.org/patch/256747/
> >>>
> >>
> >> Thanks.
> >>
> >> I'm not sure I'm a fan of doing this as it silently hides a
> >> significant performance impact.
> >>
> >> Could we possible re-write the userspace instruction to be a
> >> 'sync' when we hit this?
> >
> > Rewriting user space is a can of worms I wouldn't get into ... is
> > any other arch doing it ?
>
> Fair enough
> >
> > I'm not too worried as long as we warn and account them.
>
> Than, I'd ask this be under a Kconfig option that is disabled by
> default. Users should have to explicitly enable this so they know
> what they are doing.
I think it should be enabled by default, rather than disabled, so that
users would actually see a warning rather than get a sig 4. Or, let
it not be Kconfig-able so that this doesn't become a problem any more.
(It's been 4 years since I sent to you an earlier version of this
patch.)
^ permalink raw reply
* [PATCH 1/2] powerpc/e6500: Include Power ISA properties
From: Lijun Pan @ 2013-10-24 21:03 UTC (permalink / raw)
To: linuxppc-dev, scottwood; +Cc: Lijun Pan
b4420 and b4860 device trees do not have these properties.
Signed-off-by: Lijun Pan <Lijun.Pan@freescale.com>
---
arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi | 2 ++
arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi | 2 ++
2 files changed, 4 insertions(+)
diff --git a/arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi b/arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi
index 7b4426e..cbaecf1 100644
--- a/arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi
+++ b/arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi
@@ -34,6 +34,8 @@
/dts-v1/;
+/include/ "e6500_power_isa.dtsi:"
+
/ {
compatible = "fsl,B4420";
#address-cells = <2>;
diff --git a/arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi b/arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi
index 5263fa4..9bc26b1 100644
--- a/arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi
+++ b/arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi
@@ -34,6 +34,8 @@
/dts-v1/;
+/include/ "e6500_power_isa.dtsi"
+
/ {
compatible = "fsl,B4860";
#address-cells = <2>;
--
1.7.9.7
^ permalink raw reply related
* [PATCH 2/2] powerpc/e500v2: Include Power ISA properties
From: Lijun Pan @ 2013-10-24 21:03 UTC (permalink / raw)
To: linuxppc-dev, scottwood; +Cc: Lijun Pan
In-Reply-To: <1382648605-22509-1-git-send-email-Lijun.Pan@freescale.com>
bsc9131 device tree does not have these properties.
Signed-off-by: Lijun Pan <Lijun.Pan@freescale.com>
---
arch/powerpc/boot/dts/fsl/bsc9131si-pre.dtsi | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/powerpc/boot/dts/fsl/bsc9131si-pre.dtsi b/arch/powerpc/boot/dts/fsl/bsc9131si-pre.dtsi
index 743e4ae..f6ec4a6 100644
--- a/arch/powerpc/boot/dts/fsl/bsc9131si-pre.dtsi
+++ b/arch/powerpc/boot/dts/fsl/bsc9131si-pre.dtsi
@@ -33,6 +33,9 @@
*/
/dts-v1/;
+
+/include/ "e500v2_power_isa.dtsi"
+
/ {
compatible = "fsl,BSC9131";
#address-cells = <2>;
--
1.7.9.7
^ permalink raw reply related
* Re: [PATCH v2 08/10] of/platform: Resolve interrupt references at probe time
From: Grant Likely @ 2013-10-24 16:37 UTC (permalink / raw)
To: Thierry Reding, Rob Herring, Greg Kroah-Hartman, Thomas Gleixner
Cc: linux-mips, Russell King, devicetree, linux-kernel, Ralf Baechle,
sparclinux, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20131015232436.19F61C40099@trevor.secretlab.ca>
On Wed, 16 Oct 2013 00:24:36 +0100, Grant Likely <grant.likely@linaro.org> wrote:
> On Wed, 18 Sep 2013 15:24:50 +0200, Thierry Reding <thierry.reding@gmail.com> wrote:
> > Interrupt references are currently resolved very early (when a device is
> > created). This has the disadvantage that it will fail in cases where the
> > interrupt parent hasn't been probed and no IRQ domain for it has been
> > registered yet. To work around that various drivers use explicit
> > initcall ordering to force interrupt parents to be probed before devices
> > that need them are created. That's error prone and doesn't always work.
> > If a platform device uses an interrupt line connected to a different
> > platform device (such as a GPIO controller), both will be created in the
> > same batch, and the GPIO controller won't have been probed by its driver
> > when the depending platform device is created. Interrupt resolution will
> > fail in that case.
>
> What is the reason for all the rework on the irq parsing return values?
> A return value of '0' is always an error on irq parsing, regardless of
> architecture even if NO_IRQ is defined as -1. I may have missed it, but
> I don't see any checking for specific error values in the return paths
> of the functions.
>
> If the specific return value isn't required (and I don't think it is),
> then you can simplify the whole series by getting rid of the rework
> patches.
I've not heard back about the above, but I've just had a conversation
with Rob about what to do here. The problem that I have is that it makes
a specific return code need to traverse several levels of function calls
and have a meaning come out the other end. It becomes difficult to
figure out where that code actually comes from when reading the code.
That's more of a gut-feel reaction rather than pointing out specifics
though.
The other thing that makes me nervous how invasive the series is.
However, even with saying all of the above, I'm not saying outright no.
I want to get this feature in. It is obviously needed and I'll even
merge the patches piecemeal as the look ready (I've already merged 2).
Regardless, the current series needs to be reworked. It conflicts with
the other IRQ rework that I've already put into my tree. The best thing
to do would probably be respin it against my current tree and repost.
I'll take a fresh look then.... In the mean time, anything you can do to
/sanely/ reduce the impact will probably help. :-)
g.
^ permalink raw reply
* Re: [PATCHv1 1/8] ALSA: Add SAI SoC Digital Audio Interface driver.
From: Mark Brown @ 2013-10-24 11:05 UTC (permalink / raw)
To: Xiubo Li
Cc: mark.rutland, alsa-devel, linux-doc, tiwai, b18965, timur, perex,
r65073, LW, linux, b42378, linux-arm-kernel, grant.likely,
devicetree, ian.campbell, pawel.moll, swarren, rob.herring, oskar,
fabio.estevam, lgirdwood, linux-kernel, rob, r64188, shawn.guo,
linuxppc-dev
In-Reply-To: <1382000477-17304-2-git-send-email-Li.Xiubo@freescale.com>
[-- Attachment #1: Type: text/plain, Size: 2866 bytes --]
On Thu, Oct 17, 2013 at 05:01:10PM +0800, Xiubo Li wrote:
> +static struct snd_pcm_hardware snd_fsl_hardware = {
> + .info = SNDRV_PCM_INFO_INTERLEAVED |
> + SNDRV_PCM_INFO_BLOCK_TRANSFER |
> + SNDRV_PCM_INFO_MMAP |
> + SNDRV_PCM_INFO_MMAP_VALID |
> + SNDRV_PCM_INFO_PAUSE |
> + SNDRV_PCM_INFO_RESUME,
> + .formats = SNDRV_PCM_FMTBIT_S16_LE,
> + .rate_min = 8000,
> + .channels_min = 2,
> + .channels_max = 2,
> + .buffer_bytes_max = FSL_SAI_DMABUF_SIZE,
> + .period_bytes_min = 4096,
> + .period_bytes_max = FSL_SAI_DMABUF_SIZE / TCD_NUMBER,
> + .periods_min = TCD_NUMBER,
> + .periods_max = TCD_NUMBER,
> + .fifo_size = 0,
> +};
There's a patch in -next that lets the generic dmaengine code figure out
some settings from the dmacontroller rather than requiring the driver to
explicitly provide configuration - it's "ASoC: dmaengine-pcm: Provide
default config". Please update your driver to use this, or let's work
out what it doesn't do any try to fix it.
> + ret = fsl_sai_set_dai_sysclk_tr(cpu_dai, clk_id, freq,
> + FSL_FMT_TRANSMITTER);
> + if (ret) {
> + dev_err(cpu_dai->dev,
> + "Cannot set sai's transmitter sysclk: %d\n",
> + ret);
> + return ret;
> + }
> +
> + ret = fsl_sai_set_dai_sysclk_tr(cpu_dai, clk_id, freq,
> + FSL_FMT_RECEIVER);
As other people have commented these should be exposed as separate
clocks rather than set in sync, unless there's some hardware reason they
need to be identical. If that is the case then a comment explaining the
limitation would be good.
Similarly with several of the other functions.
> +int fsl_sai_dai_remove(struct snd_soc_dai *dai)
> +{
> + struct fsl_sai *sai = dev_get_drvdata(dai->dev);
> +
> + clk_disable_unprepare(sai->clk);
It'd be a bit nicer to only enable the clock while the driver is
actively being used rather than all the time the system is powered up
but it's not a blocker for merge.
> + ret = snd_soc_register_component(&pdev->dev, &fsl_component,
> + &fsl_sai_dai, 1);
> + if (ret)
> + return ret;
There's a devm_snd_soc_register_component() in -next, please use that.
> +
> + ret = fsl_pcm_dma_init(pdev);
> + if (ret)
> + goto out;
> +
> + platform_set_drvdata(pdev, sai);
These should go before the driver is registered with the subsystem
otherwise you've got a race where something might try to use the driver
before init is finished.
> +static int fsl_sai_remove(struct platform_device *pdev)
> +{
> + struct fsl_sai *sai = platform_get_drvdata(pdev);
> +
> + fsl_pcm_dma_exit(pdev);
> +
> + snd_soc_unregister_component(&pdev->dev);
Similarly here, unregister from the subsystem then clean up after.
> +#define SAI_CR5_FBT(x) ((x) << 8)
> +#define SAI_CR5_FBT_MASK (0x1f << 8)
> +
> +/* SAI audio dividers */
> +#define FSL_SAI_TX_DIV 0
> +#define FSL_SAI_RX_DIV 1
Make the namespacing consistent please - for preference use FSL_SAI
always.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [PATCH] powerpc/powernv: Code update interface
From: Vasant Hegde @ 2013-10-24 10:34 UTC (permalink / raw)
To: linuxppc-dev
Code update interface for powernv platform. This provides
sysfs interface to pass new image, validate, update and
commit images.
This patch includes:
- Below OPAL APIs for code update
- opal_validate_flash()
- opal_manage_flash()
- opal_update_flash()
- Create below sysfs files under /sys/firmware/opal
- image : Interface to pass new FW image
- validate_flash : Validate candidate image
- manage_flash : Commit/Reject operations
- update_flash : Flash new candidate image
Updating Image:
"update_flash" is an interface to indicate flash new FW.
It just passes image SG list to FW. Actual flashing is done
during system reboot time.
Note:
- SG entry format:
I have kept version number to keep this list similar to what
PAPR is defined.
Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/opal.h | 7
arch/powerpc/platforms/powernv/Makefile | 2
arch/powerpc/platforms/powernv/opal-flash.c | 667 ++++++++++++++++++++++++
arch/powerpc/platforms/powernv/opal-wrappers.S | 3
arch/powerpc/platforms/powernv/opal.c | 3
5 files changed, 681 insertions(+), 1 deletion(-)
create mode 100644 arch/powerpc/platforms/powernv/opal-flash.c
diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index f5d8739..60ab3d6 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -129,6 +129,9 @@ extern int opal_enter_rtas(struct rtas_args *args,
#define OPAL_LPC_READ 67
#define OPAL_LPC_WRITE 68
#define OPAL_RETURN_CPU 69
+#define OPAL_FLASH_VALIDATE 76
+#define OPAL_FLASH_MANAGE 77
+#define OPAL_FLASH_UPDATE 78
#ifndef __ASSEMBLY__
@@ -659,6 +662,9 @@ int64_t opal_lpc_write(uint32_t chip_id, enum OpalLPCAddressType addr_type,
uint32_t addr, uint32_t data, uint32_t sz);
int64_t opal_lpc_read(uint32_t chip_id, enum OpalLPCAddressType addr_type,
uint32_t addr, uint32_t *data, uint32_t sz);
+int64_t opal_validate_flash(uint64_t buffer, uint32_t *size, uint32_t *result);
+int64_t opal_manage_flash(uint8_t op);
+int64_t opal_update_flash(uint64_t blk_list);
/* Internal functions */
extern int early_init_dt_scan_opal(unsigned long node, const char *uname, int depth, void *data);
@@ -687,6 +693,7 @@ extern int opal_set_rtc_time(struct rtc_time *tm);
extern void opal_get_rtc_time(struct rtc_time *tm);
extern unsigned long opal_get_boot_time(void);
extern void opal_nvram_init(void);
+extern void opal_flash_init(void);
extern int opal_machine_check(struct pt_regs *regs);
diff --git a/arch/powerpc/platforms/powernv/Makefile b/arch/powerpc/platforms/powernv/Makefile
index 300c437..68e2b7e 100644
--- a/arch/powerpc/platforms/powernv/Makefile
+++ b/arch/powerpc/platforms/powernv/Makefile
@@ -1,5 +1,5 @@
obj-y += setup.o opal-takeover.o opal-wrappers.o opal.o
-obj-y += opal-rtc.o opal-nvram.o opal-lpc.o
+obj-y += opal-rtc.o opal-nvram.o opal-lpc.o opal-flash.o
obj-$(CONFIG_SMP) += smp.o
obj-$(CONFIG_PCI) += pci.o pci-p5ioc2.o pci-ioda.o
diff --git a/arch/powerpc/platforms/powernv/opal-flash.c b/arch/powerpc/platforms/powernv/opal-flash.c
new file mode 100644
index 0000000..6ffa6b1
--- /dev/null
+++ b/arch/powerpc/platforms/powernv/opal-flash.c
@@ -0,0 +1,667 @@
+/*
+ * PowerNV OPAL Firmware Update Interface
+ *
+ * Copyright 2013 IBM Corp.
+ *
+ * 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.
+ */
+
+#define DEBUG
+
+#include <linux/kernel.h>
+#include <linux/reboot.h>
+#include <linux/init.h>
+#include <linux/kobject.h>
+#include <linux/sysfs.h>
+#include <linux/slab.h>
+#include <linux/mm.h>
+#include <linux/vmalloc.h>
+#include <linux/pagemap.h>
+
+#include <asm/opal.h>
+
+/* FLASH status codes */
+#define FLASH_NO_OP -1099 /* No operation initiated by user */
+#define FLASH_NO_AUTH -9002 /* Not a service authority partition */
+
+/* Validate image status values */
+#define VALIDATE_IMG_READY -1001 /* Image ready for validation */
+#define VALIDATE_IMG_INCOMPLETE -1002 /* User copied < VALIDATE_BUF_SIZE */
+
+/* Manage image status values */
+#define MANAGE_ACTIVE_ERR -9001 /* Cannot overwrite active img */
+
+/* Flash image status values */
+#define FLASH_IMG_READY 0 /* Img ready for flash on reboot */
+#define FLASH_INVALID_IMG -1003 /* Flash image shorter than expected */
+#define FLASH_IMG_NULL_DATA -1004 /* Bad data in sg list entry */
+#define FLASH_IMG_BAD_LEN -1005 /* Bad length in sg list entry */
+
+/* Manage operation tokens */
+#define FLASH_REJECT_TMP_SIDE 0 /* Reject temporary fw image */
+#define FLASH_COMMIT_TMP_SIDE 1 /* Commit temporary fw image */
+
+/* Update tokens */
+#define FLASH_UPDATE_CANCEL 0 /* Cancel update request */
+#define FLASH_UPDATE_INIT 1 /* Initiate update */
+
+/* Validate image update result tokens */
+#define VALIDATE_TMP_UPDATE 0 /* T side will be updated */
+#define VALIDATE_FLASH_AUTH 1 /* Partition does not have authority */
+#define VALIDATE_INVALID_IMG 2 /* Candidate image is not valid */
+#define VALIDATE_CUR_UNKNOWN 3 /* Current fixpack level is unknown */
+/*
+ * Current T side will be committed to P side before being replace with new
+ * image, and the new image is downlevel from current image
+ */
+#define VALIDATE_TMP_COMMIT_DL 4
+/*
+ * Current T side will be committed to P side before being replaced with new
+ * image
+ */
+#define VALIDATE_TMP_COMMIT 5
+/*
+ * T side will be updated with a downlevel image
+ */
+#define VALIDATE_TMP_UPDATE_DL 6
+/*
+ * The candidate image's release date is later than the system's firmware
+ * service entitlement date - service warranty period has expired
+ */
+#define VALIDATE_OUT_OF_WRNTY 7
+
+/* Validate buffer size */
+#define VALIDATE_BUF_SIZE 4096
+
+/* XXX: Assume candidate image size is <= 256MB */
+#define MAX_IMAGE_SIZE 0x10000000
+
+/* Flash sg list version */
+#define SG_LIST_VERSION (1UL)
+
+/* Image status */
+enum {
+ IMAGE_INVALID,
+ IMAGE_LOADING,
+ IMAGE_READY,
+};
+
+/* Candidate image data */
+struct image_data_t {
+ int status;
+ void *data;
+ uint32_t size;
+};
+
+/* Candidate image header */
+struct image_header_t {
+ uint16_t magic;
+ uint16_t version;
+ uint32_t size;
+};
+
+/* Scatter/gather entry */
+struct opal_sg_entry {
+ void *data;
+ long length;
+};
+
+/* We calculate number of entries based on PAGE_SIZE */
+#define SG_ENTRIES_PER_NODE ((PAGE_SIZE - 16) / sizeof(struct opal_sg_entry))
+
+/*
+ * This struct is very similar but not identical to that
+ * needed by the opal flash update. All we need to do for
+ * opal is rewrite num_entries into a version/length and
+ * translate the pointers to absolute.
+ */
+struct opal_sg_list {
+ unsigned long num_entries;
+ struct opal_sg_list *next;
+ struct opal_sg_entry entry[SG_ENTRIES_PER_NODE];
+};
+
+struct validate_flash_t {
+ int status; /* Return status */
+ void *buf; /* Candiate image buffer */
+ uint32_t buf_size; /* Image size */
+ uint32_t result; /* Update results token */
+};
+
+struct manage_flash_t {
+ int status; /* Return status */
+};
+
+struct update_flash_t {
+ int status; /* Return status */
+};
+
+static struct image_header_t image_header;
+static struct image_data_t image_data;
+static struct validate_flash_t validate_flash_data;
+static struct manage_flash_t manage_flash_data;
+static struct update_flash_t update_flash_data;
+
+static DEFINE_MUTEX(image_data_mutex);
+
+/*
+ * Validate candidate image
+ */
+static inline void opal_flash_validate(void)
+{
+ struct validate_flash_t *args_buf = &validate_flash_data;
+
+ args_buf->status = opal_validate_flash(__pa(args_buf->buf),
+ &(args_buf->buf_size),
+ &(args_buf->result));
+}
+
+/*
+ * Validate output format:
+ * validate result token
+ * current image version details
+ * new image version details
+ */
+static ssize_t validate_show(struct kobject *kobj,
+ struct kobj_attribute *attr, char *buf)
+{
+ struct validate_flash_t *args_buf = &validate_flash_data;
+ int len;
+
+ /* Candidate image is not validated */
+ if (args_buf->status < VALIDATE_TMP_UPDATE) {
+ len = sprintf(buf, "%d\n", args_buf->status);
+ goto out;
+ }
+
+ /* Result token */
+ len = sprintf(buf, "%d\n", args_buf->result);
+
+ /* Current and candidate image version details */
+ if ((args_buf->result != VALIDATE_TMP_UPDATE) &&
+ (args_buf->result < VALIDATE_CUR_UNKNOWN))
+ goto out;
+
+ if (args_buf->buf_size > (VALIDATE_BUF_SIZE - len)) {
+ memcpy(buf + len, args_buf->buf, VALIDATE_BUF_SIZE - len);
+ len = VALIDATE_BUF_SIZE;
+ } else {
+ memcpy(buf + len, args_buf->buf, args_buf->buf_size);
+ len += args_buf->buf_size;
+ }
+out:
+ /* Set status to default */
+ args_buf->status = FLASH_NO_OP;
+ return len;
+}
+
+/*
+ * Validate candidate firmware image
+ *
+ * Note:
+ * We are only interested in first 4K bytes of the
+ * candidate image.
+ */
+static ssize_t validate_store(struct kobject *kobj,
+ struct kobj_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct validate_flash_t *args_buf = &validate_flash_data;
+
+ if (buf[0] != '1')
+ return -EINVAL;
+
+ mutex_lock(&image_data_mutex);
+
+ if (image_data.status != IMAGE_READY ||
+ image_data.size < VALIDATE_BUF_SIZE) {
+ args_buf->result = VALIDATE_INVALID_IMG;
+ args_buf->status = VALIDATE_IMG_INCOMPLETE;
+ goto out;
+ }
+
+ /* Copy first 4k bytes of candidate image */
+ memcpy(args_buf->buf, image_data.data, VALIDATE_BUF_SIZE);
+
+ args_buf->status = VALIDATE_IMG_READY;
+ args_buf->buf_size = VALIDATE_BUF_SIZE;
+
+ /* Validate candidate image */
+ opal_flash_validate();
+
+out:
+ mutex_unlock(&image_data_mutex);
+ return count;
+}
+
+/*
+ * Manage flash routine
+ */
+static inline void opal_flash_manage(uint8_t op)
+{
+ struct manage_flash_t *const args_buf = &manage_flash_data;
+
+ args_buf->status = opal_manage_flash(op);
+}
+
+/*
+ * Show manage flash status
+ */
+static ssize_t manage_show(struct kobject *kobj,
+ struct kobj_attribute *attr, char *buf)
+{
+ struct manage_flash_t *const args_buf = &manage_flash_data;
+ int rc;
+
+ rc = sprintf(buf, "%d\n", args_buf->status);
+ /* Set status to default*/
+ args_buf->status = FLASH_NO_OP;
+ return rc;
+}
+
+/*
+ * Manage operations:
+ * 0 - Reject
+ * 1 - Commit
+ */
+static ssize_t manage_store(struct kobject *kobj,
+ struct kobj_attribute *attr,
+ const char *buf, size_t count)
+{
+ uint8_t op;
+ switch (buf[0]) {
+ case '0':
+ op = FLASH_REJECT_TMP_SIDE;
+ break;
+ case '1':
+ op = FLASH_COMMIT_TMP_SIDE;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ /* commit/reject temporary image */
+ opal_flash_manage(op);
+ return count;
+}
+
+/*
+ * Free sg list
+ */
+static void free_sg_list(struct opal_sg_list *list)
+{
+ struct opal_sg_list *sg1;
+ while (list) {
+ sg1 = list->next;
+ kfree(list);
+ list = sg1;
+ }
+ list = NULL;
+}
+
+/*
+ * Build candidate image scatter gather list
+ *
+ * list format:
+ * -----------------------------------
+ * | VER (8) | Entry length in bytes |
+ * -----------------------------------
+ * | Pointer to next entry |
+ * -----------------------------------
+ * | Address of memory area 1 |
+ * -----------------------------------
+ * | Length of memory area 1 |
+ * -----------------------------------
+ * | ......... |
+ * -----------------------------------
+ * | ......... |
+ * -----------------------------------
+ * | Address of memory area N |
+ * -----------------------------------
+ * | Length of memory area N |
+ * -----------------------------------
+ */
+static struct opal_sg_list *image_data_to_sglist(void)
+{
+ struct opal_sg_list *sg1, *list = NULL;
+ void *addr;
+ int size;
+
+ addr = image_data.data;
+ size = image_data.size;
+
+ sg1 = kzalloc((sizeof(struct opal_sg_list)), GFP_KERNEL);
+ if (!sg1)
+ return NULL;
+
+ list = sg1;
+ sg1->num_entries = 0;
+ while (size > 0) {
+ /* Translate virtual address to physical address */
+ sg1->entry[sg1->num_entries].data =
+ (void *)(vmalloc_to_pfn(addr) << PAGE_SHIFT);
+
+ if (size > PAGE_SIZE)
+ sg1->entry[sg1->num_entries].length = PAGE_SIZE;
+ else
+ sg1->entry[sg1->num_entries].length = size;
+
+ sg1->num_entries++;
+ if (sg1->num_entries >= SG_ENTRIES_PER_NODE) {
+ sg1->next = kzalloc((sizeof(struct opal_sg_list)),
+ GFP_KERNEL);
+ if (!sg1->next) {
+ pr_err("%s : Failed to allocate memory\n",
+ __func__);
+ goto nomem;
+ }
+
+ sg1 = sg1->next;
+ sg1->num_entries = 0;
+ }
+ addr += PAGE_SIZE;
+ size -= PAGE_SIZE;
+ }
+ return list;
+nomem:
+ free_sg_list(list);
+ return NULL;
+}
+
+/*
+ * OPAL update flash
+ */
+static int opal_flash_update(int op)
+{
+ struct opal_sg_list *sg, *list, *next;
+ unsigned long addr;
+ int64_t rc = OPAL_PARAMETER;
+
+ if (op == FLASH_UPDATE_CANCEL) {
+ pr_alert("FLASH: Image update cancelled\n");
+ addr = '\0';
+ goto flash;
+ }
+
+ list = image_data_to_sglist();
+ if (!list)
+ goto invalid_img;
+
+ /* First entry address */
+ addr = __pa(list);
+
+ /* Translate sg list address to absolute */
+ for (sg = list; sg; sg = next) {
+ next = sg->next;
+ /* Don't translate NULL pointer for last entry */
+ if (sg->next)
+ sg->next = (struct opal_sg_list *)__pa(sg->next);
+ else
+ sg->next = NULL;
+
+ /* Make num_entries into the version/length field */
+ sg->num_entries = (SG_LIST_VERSION << 56) |
+ (sg->num_entries * sizeof(struct opal_sg_entry) + 16);
+ }
+
+ pr_alert("FLASH: Image is %u bytes\n", image_data.size);
+ pr_alert("FLASH: Image update requested\n");
+ pr_alert("FLASH: Image will be updated during system reboot\n");
+ pr_alert("FLASH: This will take several minutes. Do not power off!\n");
+
+flash:
+ rc = opal_update_flash(addr);
+
+invalid_img:
+ return rc;
+}
+
+/*
+ * Show candidate image status
+ */
+static ssize_t update_show(struct kobject *kobj,
+ struct kobj_attribute *attr, char *buf)
+{
+ struct update_flash_t *const args_buf = &update_flash_data;
+ return sprintf(buf, "%d\n", args_buf->status);
+}
+
+/*
+ * Set update image flag
+ * 1 - Flash new image
+ * 0 - Cancel flash request
+ */
+static ssize_t update_store(struct kobject *kobj,
+ struct kobj_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct update_flash_t *const args_buf = &update_flash_data;
+ int rc = count;
+
+ mutex_lock(&image_data_mutex);
+
+ switch (buf[0]) {
+ case '0':
+ if (args_buf->status == FLASH_IMG_READY)
+ opal_flash_update(FLASH_UPDATE_CANCEL);
+ args_buf->status = FLASH_NO_OP;
+ break;
+ case '1':
+ /* Image is loaded? */
+ if (image_data.status == IMAGE_READY)
+ args_buf->status =
+ opal_flash_update(FLASH_UPDATE_INIT);
+ else
+ args_buf->status = FLASH_INVALID_IMG;
+ break;
+ default:
+ rc = -EINVAL;
+ }
+
+ mutex_unlock(&image_data_mutex);
+ return rc;
+}
+
+/*
+ * Free image buffer
+ */
+static void free_image_buf(void)
+{
+ void *addr;
+ int size;
+
+ addr = image_data.data;
+ size = PAGE_ALIGN(image_data.size);
+ while (size > 0) {
+ ClearPageReserved(vmalloc_to_page(addr));
+ addr += PAGE_SIZE;
+ size -= PAGE_SIZE;
+ }
+ vfree(image_data.data);
+ image_data.data = NULL;
+ image_data.status = IMAGE_INVALID;
+}
+
+/*
+ * Allocate image buffer.
+ */
+static int alloc_image_buf(char *buffer, size_t count)
+{
+ void *addr;
+ int size;
+
+ if (count < sizeof(struct image_header_t)) {
+ pr_warn("FLASH: Invalid candidate image\n");
+ return -EINVAL;
+ }
+
+ memcpy(&image_header, (void *)buffer, sizeof(struct image_header_t));
+ image_data.size = be32_to_cpu(image_header.size);
+ pr_debug("FLASH: Candiate image size = %u\n", image_data.size);
+
+ if (image_data.size > MAX_IMAGE_SIZE) {
+ pr_warn("FLASH: Too large image\n");
+ return -EINVAL;
+ }
+ if (image_data.size < VALIDATE_BUF_SIZE) {
+ pr_warn("FLASH: Image is shorter than expected\n");
+ return -EINVAL;
+ }
+
+ image_data.data = vzalloc(PAGE_ALIGN(image_data.size));
+ if (!image_data.data) {
+ pr_err("%s : Failed to allocate memory\n", __func__);
+ return -ENOMEM;
+ }
+
+ /* Pin memory */
+ addr = image_data.data;
+ size = PAGE_ALIGN(image_data.size);
+ while (size > 0) {
+ SetPageReserved(vmalloc_to_page(addr));
+ addr += PAGE_SIZE;
+ size -= PAGE_SIZE;
+ }
+
+ image_data.status = IMAGE_LOADING;
+ return 0;
+}
+
+/*
+ * Copy candidate image
+ *
+ * Parse candidate image header to get total image size
+ * and pre-allocate required memory.
+ */
+static ssize_t image_data_write(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr,
+ char *buffer, loff_t pos, size_t count)
+{
+ int rc;
+
+ mutex_lock(&image_data_mutex);
+
+ /* New image ? */
+ if (pos == 0) {
+ /* Free memory, if already allocated */
+ if (image_data.data)
+ free_image_buf();
+
+ /* Cancel outstanding image update request */
+ if (update_flash_data.status == FLASH_IMG_READY)
+ opal_flash_update(FLASH_UPDATE_CANCEL);
+
+ /* Allocate memory */
+ rc = alloc_image_buf(buffer, count);
+ if (rc)
+ goto out;
+ }
+
+ if (image_data.status != IMAGE_LOADING) {
+ rc = -ENOMEM;
+ goto out;
+ }
+
+ if ((pos + count) > image_data.size) {
+ rc = -EINVAL;
+ goto out;
+ }
+
+ memcpy(image_data.data + pos, (void *)buffer, count);
+ rc = count;
+
+ /* Set image status */
+ if ((pos + count) == image_data.size) {
+ pr_debug("FLASH: Candidate image loaded....\n");
+ image_data.status = IMAGE_READY;
+ }
+
+out:
+ mutex_unlock(&image_data_mutex);
+ return rc;
+}
+
+/*
+ * sysfs interface :
+ * OPAL uses below sysfs files for code update.
+ * We create these files under /sys/firmware/opal.
+ *
+ * image : Interface to load candidate firmware image
+ * validate_flash : Validate firmware image
+ * manage_flash : Commit/Reject firmware image
+ * update_flash : Flash new firmware image
+ *
+ */
+static struct bin_attribute image_data_attr = {
+ .attr = {.name = "image", .mode = 0200},
+ .size = MAX_IMAGE_SIZE, /* Limit image size */
+ .write = image_data_write,
+};
+
+static struct kobj_attribute validate_attribute =
+ __ATTR(validate_flash, 0600, validate_show, validate_store);
+
+static struct kobj_attribute manage_attribute =
+ __ATTR(manage_flash, 0600, manage_show, manage_store);
+
+static struct kobj_attribute update_attribute =
+ __ATTR(update_flash, 0600, update_show, update_store);
+
+static struct attribute *image_op_attrs[] = {
+ &validate_attribute.attr,
+ &manage_attribute.attr,
+ &update_attribute.attr,
+ NULL /* need to NULL terminate the list of attributes */
+};
+
+static struct attribute_group image_op_attr_group = {
+ .attrs = image_op_attrs,
+};
+
+void __init opal_flash_init(void)
+{
+ int ret;
+
+ /* Allocate validate image buffer */
+ validate_flash_data.buf = kzalloc(VALIDATE_BUF_SIZE, GFP_KERNEL);
+ if (!validate_flash_data.buf) {
+ pr_err("%s : Failed to allocate memory\n", __func__);
+ return;
+ }
+
+ /* Make sure /sys/firmware/opal directory is created */
+ if (!opal_kobj) {
+ pr_warn("FLASH: opal kobject is not available\n");
+ goto nokobj;
+ }
+
+ /* Create the sysfs files */
+ ret = sysfs_create_group(opal_kobj, &image_op_attr_group);
+ if (ret) {
+ pr_warn("FLASH: Failed to create sysfs files\n");
+ goto nokobj;
+ }
+
+ ret = sysfs_create_bin_file(opal_kobj, &image_data_attr);
+ if (ret) {
+ pr_warn("FLASH: Failed to create sysfs files\n");
+ goto nosysfs_file;
+ }
+
+ /* Set default status */
+ validate_flash_data.status = FLASH_NO_OP;
+ manage_flash_data.status = FLASH_NO_OP;
+ update_flash_data.status = FLASH_NO_OP;
+ image_data.status = IMAGE_INVALID;
+ return;
+
+nosysfs_file:
+ sysfs_remove_group(opal_kobj, &image_op_attr_group);
+
+nokobj:
+ kfree(validate_flash_data.buf);
+ return;
+}
diff --git a/arch/powerpc/platforms/powernv/opal-wrappers.S b/arch/powerpc/platforms/powernv/opal-wrappers.S
index 8f38445..26d7c96 100644
--- a/arch/powerpc/platforms/powernv/opal-wrappers.S
+++ b/arch/powerpc/platforms/powernv/opal-wrappers.S
@@ -116,3 +116,6 @@ OPAL_CALL(opal_xscom_write, OPAL_XSCOM_WRITE);
OPAL_CALL(opal_lpc_read, OPAL_LPC_READ);
OPAL_CALL(opal_lpc_write, OPAL_LPC_WRITE);
OPAL_CALL(opal_return_cpu, OPAL_RETURN_CPU);
+OPAL_CALL(opal_validate_flash, OPAL_FLASH_VALIDATE);
+OPAL_CALL(opal_manage_flash, OPAL_FLASH_MANAGE);
+OPAL_CALL(opal_update_flash, OPAL_FLASH_UPDATE);
diff --git a/arch/powerpc/platforms/powernv/opal.c b/arch/powerpc/platforms/powernv/opal.c
index 023a789..6293928 100644
--- a/arch/powerpc/platforms/powernv/opal.c
+++ b/arch/powerpc/platforms/powernv/opal.c
@@ -433,6 +433,9 @@ static int __init opal_init(void)
/* Create "opal" kobject under /sys/firmware */
rc = opal_sysfs_init();
+ /* Setup code update interface */
+ opal_flash_init();
+
return rc;
}
subsys_initcall(opal_init);
^ permalink raw reply related
* RE: [PATCH] [RFC] Emulate "lwsync" to run standard user land on e500 cores
From: David Laight @ 2013-10-24 10:18 UTC (permalink / raw)
To: Kumar Gala, Scott Wood; +Cc: linuxppc-dev, Wolfgang Denk
In-Reply-To: <BE5B0245-4ED1-4E3F-ADB1-F6A70C8FE3E1@kernel.crashing.org>
> Subject: Re: [PATCH] [RFC] Emulate "lwsync" to run standard user land =
on e500 cores
>=20
>=20
> On Oct 23, 2013, at 5:15 AM, Scott Wood wrote:
>=20
> > On Wed, 2013-10-23 at 00:07 -0500, Kumar Gala wrote:
> >> On Oct 18, 2013, at 2:38 AM, Wolfgang Denk wrote:
> >>> diff --git a/arch/powerpc/kernel/traps.c =
b/arch/powerpc/kernel/traps.c
> >>> index f783c93..f330374 100644
> >>> --- a/arch/powerpc/kernel/traps.c
> >>> +++ b/arch/powerpc/kernel/traps.c
> >>> @@ -986,6 +986,13 @@ static int emulate_instruction(struct pt_regs =
*regs)
> >>> return 0;
> >>> }
> >>>
> >>> + /* Emulating the lwsync insn as a sync insn */
> >>> + if (instword =3D=3D PPC_INST_LWSYNC) {
> >>> + PPC_WARN_EMULATED(lwsync, regs);
> >>> + asm volatile("sync" : : : "memory");
...
> I'm not sure I'm a fan of doing this as it silently hides a =
significant performance impact.
>=20
> Could we possible re-write the userspace instruction to be a 'sync' =
when we hit this?
You'd need to modify the memory without marking the page dirty.
(Which might be interesting for other cpu-dependant optimisations.)
David
^ permalink raw reply
* Re: [PATCH] [RFC] Emulate "lwsync" to run standard user land on e500 cores
From: Kumar Gala @ 2013-10-24 9:55 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Scott Wood, linuxppc-dev, Wolfgang Denk
In-Reply-To: <1382607919.9395.56.camel@pasglop>
On Oct 24, 2013, at 4:45 AM, Benjamin Herrenschmidt wrote:
> On Wed, 2013-10-23 at 23:06 -0500, Kumar Gala wrote:
>> On Oct 23, 2013, at 5:15 AM, Scott Wood wrote:
>>=20
>>> On Wed, 2013-10-23 at 00:07 -0500, Kumar Gala wrote:
>>>> On Oct 18, 2013, at 2:38 AM, Wolfgang Denk wrote:
>>>>> diff --git a/arch/powerpc/kernel/traps.c =
b/arch/powerpc/kernel/traps.c
>>>>> index f783c93..f330374 100644
>>>>> --- a/arch/powerpc/kernel/traps.c
>>>>> +++ b/arch/powerpc/kernel/traps.c
>>>>> @@ -986,6 +986,13 @@ static int emulate_instruction(struct pt_regs =
*regs)
>>>>> return 0;
>>>>> }
>>>>>=20
>>>>> + /* Emulating the lwsync insn as a sync insn */
>>>>> + if (instword =3D=3D PPC_INST_LWSYNC) {
>>>>> + PPC_WARN_EMULATED(lwsync, regs);
>>>>> + asm volatile("sync" : : : "memory");
>>>>=20
>>>> Do we really need the inline asm? Doesn't the fact of just taking =
an exception and returning from it equate to a sync.
>>>=20
>>> No, it doesn't equate to a sync. See the discussion here:
>>> http://patchwork.ozlabs.org/patch/256747/
>>>=20
>>=20
>> Thanks.=20
>>=20
>> I'm not sure I'm a fan of doing this as it silently hides a =
significant performance impact.
>>=20
>> Could we possible re-write the userspace instruction to be a 'sync' =
when we hit this?
>=20
> Rewriting user space is a can of worms I wouldn't get into ... is any
> other arch doing it ?
Fair enough
>=20
> I'm not too worried as long as we warn and account them.
Than, I'd ask this be under a Kconfig option that is disabled by =
default. Users should have to explicitly enable this so they know what =
they are doing.
- k
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox