LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v2] PowerPC: kernel: compiling issue, make additional room in exception vector area
From: Chen Gang @ 2013-04-26  1:36 UTC (permalink / raw)
  To: Michael Neuling
  Cc: sfr, matt, linux-kernel, Paul Mackerras, Aneesh Kumar K.V,
	Mike Qiu, linuxppc-dev
In-Reply-To: <5179D5D8.8080102@asianux.com>

On 2013年04月26日 09:18, Chen Gang wrote:
> On 2013年04月26日 09:06, Chen Gang wrote:
>>> CFAR is the Come From Register.  It saves the location of the last
>>>> branch and is hence overwritten by any branch.
>>>>
>> Do we process it just like others done (e.g. 0x300, 0xe00, 0xe20 ...) ?
>> 	. = 0x900
>> 	.globl decrementer_pSeries
>> decrementer_pSeries:
>>  	HMT_MEDIUM_PPR_DISCARD
>> 	SET_SCRATCH0(r13)
>> 	b decrementer_pSeries_0
>>
>> 	...
>>
>>

Oh, it seems EXCEPTION_PROLOG_1 will save the regesters which related
with CFAR, so I think need move EXCEPTION_PROLOG_1 to near 0x900.

---------------------------------diff v2 begin-----------------------------

diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index e789ee7..f0489c4 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -254,7 +254,15 @@ hardware_interrupt_hv:
 	STD_EXCEPTION_PSERIES(0x800, 0x800, fp_unavailable)
 	KVM_HANDLER_PR(PACA_EXGEN, EXC_STD, 0x800)
 
-	MASKABLE_EXCEPTION_PSERIES(0x900, 0x900, decrementer)
+	. = 0x900
+	.globl decrementer_pSeries
+decrementer_pSeries:
+	HMT_MEDIUM_PPR_DISCARD
+	SET_SCRATCH0(r13)		/* save r13 */
+	EXCEPTION_PROLOG_0(PACA_EXGEN)
+	EXCEPTION_PROLOG_1(PACA_EXGEN, SOFTEN_TEST_PR, 0x900)
+	b	decrementer_pSeries_0
+
 	STD_EXCEPTION_HV(0x980, 0x982, hdecrementer)
 
 	MASKABLE_EXCEPTION_PSERIES(0xa00, 0xa00, doorbell_super)
@@ -536,6 +544,11 @@ ALT_FTR_SECTION_END_IFCLR(CPU_FTR_ARCH_206)
 #endif
 
 	.align	7
+	/* moved from 0x900 */
+decrementer_pSeries_0:
+	EXCEPTION_PROLOG_PSERIES_1(decrementer_common, EXC_STD)
+
+	.align	7
 	/* moved from 0xe00 */
 	STD_EXCEPTION_HV_OOL(0xe02, h_data_storage)
 	KVM_HANDLER_SKIP(PACA_EXGEN, EXC_HV, 0xe02)


---------------------------------diff v2 end-------------------------------


> 
> Such as the fix below, is it OK (just like 0x300 or 0x200 has done) ?
> 
> Please check, thanks.
> 
> ---------------------------diff begin-------------------------------------
> 
> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
> index e789ee7..a0a5ff2 100644
> --- a/arch/powerpc/kernel/exceptions-64s.S
> +++ b/arch/powerpc/kernel/exceptions-64s.S
> @@ -254,7 +254,14 @@ hardware_interrupt_hv:
>  	STD_EXCEPTION_PSERIES(0x800, 0x800, fp_unavailable)
>  	KVM_HANDLER_PR(PACA_EXGEN, EXC_STD, 0x800)
>  
> -	MASKABLE_EXCEPTION_PSERIES(0x900, 0x900, decrementer)
> +	. = 0x900
> +	.globl decrementer_pSeries
> +decrementer_pSeries:
> +	HMT_MEDIUM_PPR_DISCARD
> +	SET_SCRATCH0(r13)		/* save r13 */
> +	EXCEPTION_PROLOG_0(PACA_EXGEN)
> +	b	decrementer_pSeries_0
> +
>  	STD_EXCEPTION_HV(0x980, 0x982, hdecrementer)
>  
>  	MASKABLE_EXCEPTION_PSERIES(0xa00, 0xa00, doorbell_super)
> @@ -536,6 +543,12 @@ ALT_FTR_SECTION_END_IFCLR(CPU_FTR_ARCH_206)
>  #endif
>  
>  	.align	7
> +	/* moved from 0x900 */
> +decrementer_pSeries_0:
> +	EXCEPTION_PROLOG_1(PACA_EXGEN, SOFTEN_TEST_PR, 0x900)
> +	EXCEPTION_PROLOG_PSERIES_1(decrementer_common, EXC_STD)
> +
> +	.align	7
>  	/* moved from 0xe00 */
>  	STD_EXCEPTION_HV_OOL(0xe02, h_data_storage)
>  	KVM_HANDLER_SKIP(PACA_EXGEN, EXC_HV, 0xe02)
> 
> ---------------------------diff end---------------------------------------
> 


-- 
Chen Gang

Asianux Corporation

^ permalink raw reply related

* [PATCH] powerpc: Fix hardware IRQs with MMU on exceptions when HV=0
From: Michael Neuling @ 2013-04-26  1:30 UTC (permalink / raw)
  To: benh; +Cc: Linux PPC dev, Nishanth Aravamudan

POWER8 allows us to take interrupts with the MMU on.  This gives us a
second set of vectors offset at 0x4000.  

Unfortunately when coping these vectors we missed checking for MSR HV
for hardware interrupts (0x500).  This results in us trying to use
HSRR0/1 when HV=0, rather than SRR0/1 on HW IRQs

The below fixes this to check CPU_FTR_HVMODE when patching the code at
0x4500.  

Also we remove the check for CPU_FTR_ARCH_206 since relocation on IRQs
are only available in arch 2.07 and beyond.

Thanks to benh for helping find this.

Signed-off-by: Michael Neuling <mikey@neuling.org>
cc: stable@kernel.org

diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 56bd923..3bbe7ed 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -797,7 +797,7 @@ hardware_interrupt_relon_hv:
 		_MASKABLE_RELON_EXCEPTION_PSERIES(0x502, hardware_interrupt, EXC_HV, SOFTEN_TEST_HV)
 	FTR_SECTION_ELSE
 		_MASKABLE_RELON_EXCEPTION_PSERIES(0x500, hardware_interrupt, EXC_STD, SOFTEN_TEST_PR)
-	ALT_FTR_SECTION_END_IFSET(CPU_FTR_ARCH_206)
+	ALT_FTR_SECTION_END_IFSET(CPU_FTR_HVMODE)
 	STD_RELON_EXCEPTION_PSERIES(0x4600, 0x600, alignment)
 	STD_RELON_EXCEPTION_PSERIES(0x4700, 0x700, program_check)
 	STD_RELON_EXCEPTION_PSERIES(0x4800, 0x800, fp_unavailable)

^ permalink raw reply related

* Re: [PATCH v2] PowerPC: kernel: compiling issue, make additional room in exception vector area
From: Chen Gang @ 2013-04-26  1:18 UTC (permalink / raw)
  To: Michael Neuling
  Cc: sfr, matt, linux-kernel, Paul Mackerras, Aneesh Kumar K.V,
	Mike Qiu, linuxppc-dev
In-Reply-To: <5179D302.9060203@asianux.com>

On 2013年04月26日 09:06, Chen Gang wrote:
>> CFAR is the Come From Register.  It saves the location of the last
>> > branch and is hence overwritten by any branch.
>> > 
> Do we process it just like others done (e.g. 0x300, 0xe00, 0xe20 ...) ?
> 	. = 0x900
> 	.globl decrementer_pSeries
> decrementer_pSeries:
>  	HMT_MEDIUM_PPR_DISCARD
> 	SET_SCRATCH0(r13)
> 	b decrementer_pSeries_0
> 
> 	...
> 
> 

Such as the fix below, is it OK (just like 0x300 or 0x200 has done) ?

Please check, thanks.

---------------------------diff begin-------------------------------------

diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index e789ee7..a0a5ff2 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -254,7 +254,14 @@ hardware_interrupt_hv:
 	STD_EXCEPTION_PSERIES(0x800, 0x800, fp_unavailable)
 	KVM_HANDLER_PR(PACA_EXGEN, EXC_STD, 0x800)
 
-	MASKABLE_EXCEPTION_PSERIES(0x900, 0x900, decrementer)
+	. = 0x900
+	.globl decrementer_pSeries
+decrementer_pSeries:
+	HMT_MEDIUM_PPR_DISCARD
+	SET_SCRATCH0(r13)		/* save r13 */
+	EXCEPTION_PROLOG_0(PACA_EXGEN)
+	b	decrementer_pSeries_0
+
 	STD_EXCEPTION_HV(0x980, 0x982, hdecrementer)
 
 	MASKABLE_EXCEPTION_PSERIES(0xa00, 0xa00, doorbell_super)
@@ -536,6 +543,12 @@ ALT_FTR_SECTION_END_IFCLR(CPU_FTR_ARCH_206)
 #endif
 
 	.align	7
+	/* moved from 0x900 */
+decrementer_pSeries_0:
+	EXCEPTION_PROLOG_1(PACA_EXGEN, SOFTEN_TEST_PR, 0x900)
+	EXCEPTION_PROLOG_PSERIES_1(decrementer_common, EXC_STD)
+
+	.align	7
 	/* moved from 0xe00 */
 	STD_EXCEPTION_HV_OOL(0xe02, h_data_storage)
 	KVM_HANDLER_SKIP(PACA_EXGEN, EXC_HV, 0xe02)

---------------------------diff end---------------------------------------

-- 
Chen Gang

Asianux Corporation

^ permalink raw reply related

* Re: [PATCH v2] PowerPC: kernel: compiling issue, make additional room in exception vector area
From: Chen Gang @ 2013-04-26  1:06 UTC (permalink / raw)
  To: Michael Neuling
  Cc: sfr, matt, linux-kernel, Paul Mackerras, Aneesh Kumar K.V,
	Mike Qiu, linuxppc-dev
In-Reply-To: <30269.1366931768@ale.ozlabs.ibm.com>

On 2013年04月26日 07:16, Michael Neuling wrote:
>> > diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
>> > index e789ee7..8997de2 100644
>> > --- a/arch/powerpc/kernel/exceptions-64s.S
>> > +++ b/arch/powerpc/kernel/exceptions-64s.S
>> > @@ -254,7 +254,11 @@ hardware_interrupt_hv:
>> >  	STD_EXCEPTION_PSERIES(0x800, 0x800, fp_unavailable)
>> >  	KVM_HANDLER_PR(PACA_EXGEN, EXC_STD, 0x800)
>> >  
>> > -	MASKABLE_EXCEPTION_PSERIES(0x900, 0x900, decrementer)
>> > +	. = 0x900
>> > +	.globl decrementer_pSeries
>> > +decrementer_pSeries:
>> > +	b	decrementer_pSeries_0
>> > +
> Unfortunately you can't do this ether as we need to save the CFAR[1]
> before it's overwritten by any branch. MASKABLE_EXCEPTION_PSERIES does
> this.
> 

Thanks for your checking.

> CFAR is the Come From Register.  It saves the location of the last
> branch and is hence overwritten by any branch.
> 

Do we process it just like others done (e.g. 0x300, 0xe00, 0xe20 ...) ?
	. = 0x900
	.globl decrementer_pSeries
decrementer_pSeries:
 	HMT_MEDIUM_PPR_DISCARD
	SET_SCRATCH0(r13)
	b decrementer_pSeries_0

	...


> Thanks for trying.
> 

Not at all, before get fixed by other members, I should continue trying.

-- 
Chen Gang

Asianux Corporation

^ permalink raw reply

* Re: [PATCH v2 12/15] powerpc/85xx: add time base sync support for e6500
From: Scott Wood @ 2013-04-26  0:07 UTC (permalink / raw)
  To: Zhao Chenhui; +Cc: linuxppc-dev, linux-kernel, r58472
In-Reply-To: <20130425002818.GE3172@localhost.localdomain>

On 04/24/2013 07:28:18 PM, Zhao Chenhui wrote:
> On Wed, Apr 24, 2013 at 05:38:16PM -0500, Scott Wood wrote:
> > On 04/24/2013 06:29:29 AM, Zhao Chenhui wrote:
> > >On Tue, Apr 23, 2013 at 07:04:06PM -0500, Scott Wood wrote:
> > >> On 04/19/2013 05:47:45 AM, Zhao Chenhui wrote:
> > >> >From: Chen-Hui Zhao <chenhui.zhao@freescale.com>
> > >> >
> > >> >For e6500, two threads in one core share one time base. Just =20
> need
> > >> >to do time base sync on first thread of one core, and skip it on
> > >> >the other thread.
> > >> >
> > >> >Signed-off-by: Zhao Chenhui <chenhui.zhao@freescale.com>
> > >> >Signed-off-by: Li Yang <leoli@freescale.com>
> > >> >Signed-off-by: Andy Fleming <afleming@freescale.com>
> > >> >---
> > >> > arch/powerpc/platforms/85xx/smp.c |   52
> > >> >+++++++++++++++++++++++++++++++-----
> > >> > 1 files changed, 44 insertions(+), 8 deletions(-)
> > >> >
> > >> >diff --git a/arch/powerpc/platforms/85xx/smp.c
> > >> >b/arch/powerpc/platforms/85xx/smp.c
> > >> >index 74d8cde..5f3eee3 100644
> > >> >--- a/arch/powerpc/platforms/85xx/smp.c
> > >> >+++ b/arch/powerpc/platforms/85xx/smp.c
> > >> >@@ -53,26 +55,40 @@ static inline u32 get_phy_cpu_mask(void)
> > >> > 	u32 mask;
> > >> > 	int cpu;
> > >> >
> > >> >-	mask =3D 1 << cur_booting_core;
> > >> >-	for_each_online_cpu(cpu)
> > >> >-		mask |=3D 1 << get_hard_smp_processor_id(cpu);
> > >> >+	if (smt_capable()) {
> > >> >+		/* two threads in one core share one time base =20
> */
> > >> >+		mask =3D 1 << =20
> cpu_core_index_of_thread(cur_booting_core);
> > >> >+		for_each_online_cpu(cpu)
> > >> >+			mask |=3D 1 << cpu_core_index_of_thread(
> > >> >+					=20
> get_hard_smp_processor_id(cpu));
> > >> >+	} else {
> > >> >+		mask =3D 1 << cur_booting_core;
> > >> >+		for_each_online_cpu(cpu)
> > >> >+			mask |=3D 1 << =20
> get_hard_smp_processor_id(cpu);
> > >> >+	}
> > >>
> > >> Where is smt_capable defined()?  I assume somewhere in the =20
> patchset
> > >> but it's a pain to search 12 patches...
> > >>
> > >
> > >It is defined in arch/powerpc/include/asm/topology.h.
> > >	#define smt_capable()           (cpu_has_feature(CPU_FTR_SMT))
> > >
> > >Thanks for your review again.
> >
> > We shouldn't base it on CPU_FTR_SMT.  For example, e6500 doesn't
> > claim that feature yet, except in our SDK kernel.  That doesn't
> > change the topology of CPU numbering.
> >
>=20
> Then, where can I get the thread information? dts?
> Or, wait for upstream of the thread suppport of e6500.

It's an inherent property of e6500 (outside of some virtualization =20
scenarios, but you wouldn't run this code under a hypervisor) that you =20
have two threads per core (whether Linux uses them or not).  Or you =20
could read TMCFG0[NTHRD] if you know you're on a chip that has TMRs but =20
aren't positive it's an e6500, but I wouldn't bother.  If we do ever =20
have such a chip, there are probably other things that will need =20
updating.

> > >static inline u32 get_phy_cpu_mask(void)
> > >{
> > >	u32 mask;
> > >	int cpu;
> > >
> > >	mask =3D 1 << cpu_core_index_of_thread(cur_booting_core);
> > >	for_each_online_cpu(cpu)
> > >		mask |=3D 1 << cpu_core_index_of_thread(
> > >				get_hard_smp_processor_id(cpu));
> > >
> > >	return mask;
> > >}
> >
> > Likewise, this will get it wrong if SMT is disabled or not yet
> > implemented on a core.
> >
> > -Scott
>=20
> Let's look into cpu_core_index_of_thread() in =20
> arch/powerpc/kernel/smp.c.
>=20
>   int cpu_core_index_of_thread(int cpu)
>   {
>       return cpu >> threads_shift;
>   }
>=20
> If no thread, the threads_shift is equal to 0. It can work with no
> thread.

My point is that if threads are disabled, threads_shift will be 0, but =20
e6500 cores will still be numbered 0, 2, 4, etc.

> Perhaps, I should submit this patch after the thread patches for =20
> e6500.

Why?

-Scott=

^ permalink raw reply

* why does CONFIG_NONSTATIC_KERNEL affect the boot kernel?
From: Chris Friesen @ 2013-04-25 23:33 UTC (permalink / raw)
  To: linuxppc-dev, Benjamin Herrenschmidt, Paul Mackerras


Hi,

Looking at the kdump code for powerpc, I see that if 
CONFIG_NONSTATIC_KERNEL is not set then the load address for the capture 
kernel is fixed at 32MB.

Why is this?

When using a separate capture kernel I don't see why that should 
restrict where I allocate space in the boot kernel.

Chris

-- 

Chris Friesen
Software Designer

500 Palladium Drive, Suite 2100
Ottawa, Ontario K2N 1C2, Canada
www.genband.com
office:+1.343.883.2717
chris.friesen@genband.com

^ permalink raw reply

* Re: [PATCH v2] PowerPC: kernel: compiling issue, make additional room in exception vector area
From: Michael Neuling @ 2013-04-25 23:16 UTC (permalink / raw)
  To: Chen Gang
  Cc: sfr, matt, linux-kernel, Paul Mackerras, Aneesh Kumar K.V,
	Mike Qiu, linuxppc-dev
In-Reply-To: <517918AB.4020508@asianux.com>

Chen Gang <gang.chen@asianux.com> wrote:

> 
> When CONFIG_KVM_BOOK3S_64_PR is enabled,
> MASKABLE_EXCEPTION_PSERIES(0x900 ...) will includes __KVMTEST, it will
> exceed 0x980 which STD_EXCEPTION_HV(0x980 ...) will use, it will cause
> compiling issue.
> 
> The related errors:
> arch/powerpc/kernel/exceptions-64s.S: Assembler messages:
> arch/powerpc/kernel/exceptions-64s.S:258: Error: attempt to move .org backwards
> make[1]: *** [arch/powerpc/kernel/head_64.o] Error 1
> 
> The position 0x900 and 0x980 are solid, so can not move the position
> to make room larger. The final solution is to jump to another area to
> execute the related code.
> 
> 
> Signed-off-by: Chen Gang <gang.chen@asianux.com>
> ---
>  arch/powerpc/kernel/exceptions-64s.S |   12 +++++++++++-
>  1 files changed, 11 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
> index e789ee7..8997de2 100644
> --- a/arch/powerpc/kernel/exceptions-64s.S
> +++ b/arch/powerpc/kernel/exceptions-64s.S
> @@ -254,7 +254,11 @@ hardware_interrupt_hv:
>  	STD_EXCEPTION_PSERIES(0x800, 0x800, fp_unavailable)
>  	KVM_HANDLER_PR(PACA_EXGEN, EXC_STD, 0x800)
>  
> -	MASKABLE_EXCEPTION_PSERIES(0x900, 0x900, decrementer)
> +	. = 0x900
> +	.globl decrementer_pSeries
> +decrementer_pSeries:
> +	b	decrementer_pSeries_0
> +

Unfortunately you can't do this ether as we need to save the CFAR[1]
before it's overwritten by any branch. MASKABLE_EXCEPTION_PSERIES does
this.

CFAR is the Come From Register.  It saves the location of the last
branch and is hence overwritten by any branch.

Thanks for trying.

Mikey

>  	STD_EXCEPTION_HV(0x980, 0x982, hdecrementer)
>  
>  	MASKABLE_EXCEPTION_PSERIES(0xa00, 0xa00, doorbell_super)
> @@ -536,6 +540,12 @@ ALT_FTR_SECTION_END_IFCLR(CPU_FTR_ARCH_206)
>  #endif
>  
>  	.align	7
> +	/* moved from 0x900 */
> +decrementer_pSeries_0:
> +	_MASKABLE_EXCEPTION_PSERIES(0x900, decrementer,
> +				    EXC_STD, SOFTEN_TEST_PR)
> +
> +	.align	7
>  	/* moved from 0xe00 */
>  	STD_EXCEPTION_HV_OOL(0xe02, h_data_storage)
>  	KVM_HANDLER_SKIP(PACA_EXGEN, EXC_HV, 0xe02)
> -- 
> 1.7.7.6
> 
> 

^ permalink raw reply

* RE: [PATCH 1/3] rapidio: make enumeration/discovery configurable
From: Bounine, Alexandre @ 2013-04-25 21:00 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linuxppc-dev@lists.ozlabs.org, Micha Nelissen,
	linux-kernel@vger.kernel.org, Andre van Herk
In-Reply-To: <20130424143714.4911405aa979bff27887765a@linux-foundation.org>

On Wednesday, April 24, 2013 5:37 PM Andrew Morton wrote:
>=20
> On Wed, 24 Apr 2013 10:31:57 -0400 Alexandre Bounine
> <alexandre.bounine@idt.com> wrote:
>=20
> > Rework to implement RapidIO enumeration/discovery method selection
> > combined with ability to use enumeration/discovery as a kernel module.
> >
> > ...
> >
> > @@ -1421,3 +1295,46 @@ enum_done:
> >  bail:
> >  	return -EBUSY;
> >  }
> > +
> > +struct rio_scan rio_scan_ops =3D {
> > +	.enumerate =3D rio_enum_mport,
> > +	.discover =3D rio_disc_mport,
> > +};
> > +
> > +
> > +#ifdef MODULE
>=20
> Why the `ifdef MODULE'?  The module parameters are still accessible if
> the driver is statically linked and we do want the driver to behave in
> the same way regardless of how it was linked and loaded.

Marked for review.

> > +static bool scan;
> > +module_param(scan, bool, 0);
> > +MODULE_PARM_DESC(scan, "Start RapidIO network enumeration/discovery
> "
> > +			"(default =3D 1)");
> > +
> > +/**
> > + * rio_basic_attach:
> > + *
> > + * When this enumeration/discovery method is loaded as a module this f=
unction
> > + * registers its specific enumeration and discover routines for all av=
ailable
> > + * RapidIO mport devices. The "scan" command line parameter controls a=
bility of
> > + * the module to start RapidIO enumeration/discovery automatically.
> > + *
> > + * Returns 0 for success or -EIO if unable to register itself.
> > + *
> > + * This enumeration/discovery method cannot be unloaded and therefore =
does not
> > + * provide a matching cleanup_module routine.
> > + */
> > +
> > +int __init rio_basic_attach(void)
>=20
> static

My miss. Will fix.

> > +{
> > +	if (rio_register_scan(RIO_MPORT_ANY, &rio_scan_ops))
> > +		return -EIO;
> > +	if (scan)
> > +		rio_init_mports();
> > +	return 0;
> > +}
> > +
> > +module_init(rio_basic_attach);
> > +
> > +MODULE_DESCRIPTION("Basic RapidIO enumeration/discovery");
> > +MODULE_LICENSE("GPL");
> > +
> > +#endif /* MODULE */
> > diff --git a/drivers/rapidio/rio.c b/drivers/rapidio/rio.c
> > index d553b5d..e36628a 100644
> > --- a/drivers/rapidio/rio.c
> > +++ b/drivers/rapidio/rio.c
> > @@ -31,6 +31,9 @@
> >
> >  #include "rio.h"
> >
> > +LIST_HEAD(rio_devices);
>=20
> static?
>=20
> > +DEFINE_SPINLOCK(rio_global_list_lock);
>=20
> static?

These two have been very tempting for me to make them static.
But because a goal was simple code move and they have been visible
from very beginning of RapidIO code, I decided to keep the scope "as is".
While I am against using the device list directly, I am not sure that this
patch is a good place to change the existing scope.

Because keeping them global prompted your comment I will happily make them
static and see if anyone will complain about it.
=20
> > +
> >  static LIST_HEAD(rio_mports);
> >  static unsigned char next_portid;
> >  static DEFINE_SPINLOCK(rio_mmap_lock);
> >
> > ...
> >
> > +/**
> > + * rio_switch_init - Sets switch operations for a particular vendor sw=
itch
> > + * @rdev: RIO device
> > + * @do_enum: Enumeration/Discovery mode flag
> > + *
> > + * Searches the RIO switch ops table for known switch types. If the vi=
d
> > + * and did match a switch table entry, then call switch initialization
> > + * routine to setup switch-specific routines.
> > + */
> > +void rio_switch_init(struct rio_dev *rdev, int do_enum)
> > +{
> > +	struct rio_switch_ops *cur =3D __start_rio_switch_ops;
> > +	struct rio_switch_ops *end =3D __end_rio_switch_ops;
>=20
> huh, I hadn't noticed that RIO has its very own vmlinux section.  How
> peculair.

Yes it is there (since 2.6.15). We will address it at some point later.
At this moment just moving the code from one file to another.
=20
> > +	while (cur < end) {
> > +		if ((cur->vid =3D=3D rdev->vid) && (cur->did =3D=3D rdev->did)) {
> > +			pr_debug("RIO: calling init routine for %s\n",
> > +				 rio_name(rdev));
> > +			cur->init_hook(rdev, do_enum);
> > +			break;
> > +		}
> > +		cur++;
> > +	}
> > +
> > +	if ((cur >=3D end) && (rdev->pef & RIO_PEF_STD_RT)) {
> > +		pr_debug("RIO: adding STD routing ops for %s\n",
> > +			rio_name(rdev));
> > +		rdev->rswitch->add_entry =3D rio_std_route_add_entry;
> > +		rdev->rswitch->get_entry =3D rio_std_route_get_entry;
> > +		rdev->rswitch->clr_table =3D rio_std_route_clr_table;
> > +	}
> > +
> > +	if (!rdev->rswitch->add_entry || !rdev->rswitch->get_entry)
> > +		printk(KERN_ERR "RIO: missing routing ops for %s\n",
> > +		       rio_name(rdev));
> > +}
> > +EXPORT_SYMBOL_GPL(rio_switch_init);
> >
> > ...
> >
> > +int rio_register_scan(int mport_id, struct rio_scan *scan_ops)
> > +{
> > +	struct rio_mport *port;
> > +	int rc =3D -EBUSY;
> > +
> > +	list_for_each_entry(port, &rio_mports, node) {
>=20
> How come the driver has no locking for rio_mports?  If a bugfix isn't
> needed here then a code comment is!

Locking is not needed at this moment, but has to be added sooner or later a=
nyway.
I will add it now to avoid fixing it later.=20
=20
> > +		if (port->id =3D=3D mport_id || mport_id =3D=3D RIO_MPORT_ANY) {
> > +			if (port->nscan && mport_id =3D=3D RIO_MPORT_ANY)
> > +				continue;
> > +			else if (port->nscan)
> > +				break;
> > +
> > +			port->nscan =3D scan_ops;
> > +			rc =3D 0;
> > +
> > +			if (mport_id !=3D RIO_MPORT_ANY)
> > +				break;
> > +		}
> > +	}
> > +
> > +	return rc;
> > +}
> >
> > ...

^ permalink raw reply

* Re: [PATCH 1/5] ibmebus: convert of_platform_driver to platform_driver
From: Rob Herring @ 2013-04-25 20:45 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Roland Dreier, Arnd Bergmann, linux-rdma, netdev,
	devicetree-discuss@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	Christoph Raisch, Hoang-Nam Nguyen, Thadeu Lima de Souza Cascardo,
	Grant Likely, Paul Mackerras, Sean Hefty, linuxppc-dev,
	Hal Rosenstock
In-Reply-To: <1366920741.2869.32.camel@pasglop>

On 04/25/2013 03:12 PM, Benjamin Herrenschmidt wrote:
> On Thu, 2013-04-25 at 14:14 -0500, Rob Herring wrote:
>> On 04/25/2013 12:35 PM, Benjamin Herrenschmidt wrote:

[...]

>> You need patch 2 of this series to fix this:
>>
>> driver core: move to_platform_driver to platform_device.h
>>
>> which as Arnd pointed out needs to come first. I've fixed that in my
>> tree, but I don't think it warrants another post.
> 
> Can I pull you tree from somewhere to test ?

Here you go:

git://sources.calxeda.com/kernel/linux.git of-platform-removal

Rob

^ permalink raw reply

* Re: [PATCH 1/5] ibmebus: convert of_platform_driver to platform_driver
From: Benjamin Herrenschmidt @ 2013-04-25 20:12 UTC (permalink / raw)
  To: Rob Herring
  Cc: Roland Dreier, Arnd Bergmann, linux-rdma, netdev,
	devicetree-discuss@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	Christoph Raisch, Hoang-Nam Nguyen, Thadeu Lima de Souza Cascardo,
	Grant Likely, Paul Mackerras, Sean Hefty, linuxppc-dev,
	Hal Rosenstock
In-Reply-To: <517980B0.3000401@gmail.com>

On Thu, 2013-04-25 at 14:14 -0500, Rob Herring wrote:
> On 04/25/2013 12:35 PM, Benjamin Herrenschmidt wrote:
> > On Thu, 2013-04-25 at 10:23 -0500, Rob Herring wrote:
> >> Ben, Can I have your Ack for this? The change is straightforward and
> >> neither of the 2 drivers used the id parameter that is removed.
> > 
> > Didn't you get my mail about a compile failure caused by this patch ?
> 
> No, and I can't find any evidence of a mail in my inbox or in the list
> archives.

Odd...

> > 
 .../...

> You need patch 2 of this series to fix this:
> 
> driver core: move to_platform_driver to platform_device.h
> 
> which as Arnd pointed out needs to come first. I've fixed that in my
> tree, but I don't think it warrants another post.

Can I pull you tree from somewhere to test ?

Cheers,
Ben.

> Rob
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 1/5] ibmebus: convert of_platform_driver to platform_driver
From: Rob Herring @ 2013-04-25 19:14 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Roland Dreier, Arnd Bergmann, linux-rdma, netdev,
	devicetree-discuss@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	Christoph Raisch, Hoang-Nam Nguyen, Thadeu Lima de Souza Cascardo,
	Grant Likely, Paul Mackerras, Sean Hefty, linuxppc-dev,
	Hal Rosenstock
In-Reply-To: <1366911329.2869.31.camel@pasglop>

On 04/25/2013 12:35 PM, Benjamin Herrenschmidt wrote:
> On Thu, 2013-04-25 at 10:23 -0500, Rob Herring wrote:
>> Ben, Can I have your Ack for this? The change is straightforward and
>> neither of the 2 drivers used the id parameter that is removed.
> 
> Didn't you get my mail about a compile failure caused by this patch ?

No, and I can't find any evidence of a mail in my inbox or in the list
archives.

> 
> A quick build test leads to:
> 
> /home/benh/linux-powerpc-test/arch/powerpc/kernel/ibmebus.c: In function 'ibmebus_bus_device_probe':
> /home/benh/linux-powerpc-test/arch/powerpc/kernel/ibmebus.c:344:2: error: implicit declaration of function 'to_platform_driver' [-Werror=implicit-function-declaration]
> /home/benh/linux-powerpc-test/arch/powerpc/kernel/ibmebus.c:344:6: error: assignment makes pointer from integer without a cast [-Werror]
> /home/benh/linux-powerpc-test/arch/powerpc/kernel/ibmebus.c: In function 'ibmebus_bus_device_remove':
> /home/benh/linux-powerpc-test/arch/powerpc/kernel/ibmebus.c:363:32: error: initialization makes pointer from integer without a cast [-Werror]
> /home/benh/linux-powerpc-test/arch/powerpc/kernel/ibmebus.c: In function 'ibmebus_bus_device_shutdown':
> /home/benh/linux-powerpc-test/arch/powerpc/kernel/ibmebus.c:373:32: error: initialization makes pointer from integer without a cast [-Werror]
> /home/benh/linux-powerpc-test/arch/powerpc/kernel/ibmebus.c: In function 'ibmebus_bus_legacy_suspend':
> /home/benh/linux-powerpc-test/arch/powerpc/kernel/ibmebus.c:420:32: error: initialization makes pointer from integer without a cast [-Werror]
> /home/benh/linux-powerpc-test/arch/powerpc/kernel/ibmebus.c: In function 'ibmebus_bus_legacy_resume':
> /home/benh/linux-powerpc-test/arch/powerpc/kernel/ibmebus.c:431:32: error: initialization makes pointer from integer without a cast [-Werror]
> cc1: all warnings being treated as errors
> make[2]: *** [arch/powerpc/kernel/ibmebus.o] Error 1
> make[2]: *** Waiting for unfinished jobs....
> 
> Haven't had a chance to look too closely today -EJETLAG :-)

You need patch 2 of this series to fix this:

driver core: move to_platform_driver to platform_device.h

which as Arnd pointed out needs to come first. I've fixed that in my
tree, but I don't think it warrants another post.

Rob

^ permalink raw reply

* Re: [RFC PATCH 1/3] powerpc: Move struct pci_controller to asm-generic
From: Bjorn Helgaas @ 2013-04-25 19:08 UTC (permalink / raw)
  To: Andrew Murray
  Cc: Michal Simek, Arnd Bergmann, linux-pci@vger.kernel.org,
	microblaze-uclinux, linux-kernel@vger.kernel.org, linuxppc-dev
In-Reply-To: <1366883360-14061-2-git-send-email-Andrew.Murray@arm.com>

On Thu, Apr 25, 2013 at 3:49 AM, Andrew Murray <Andrew.Murray@arm.com> wrote:
> This patch moves struct pci_controller into asm-generic to allow
> for use by other architectures thus reducing code duplication in
> the kernel.
>
> Signed-off-by: Andrew Murray <Andrew.Murray@arm.com>
> ---
>  arch/powerpc/include/asm/pci-bridge.h |   87 +++++---------------------------
>  include/asm-generic/pci-bridge.h      |   68 +++++++++++++++++++++++++
>  2 files changed, 82 insertions(+), 73 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
> index 205bfba..163bd40 100644
> --- a/arch/powerpc/include/asm/pci-bridge.h
> +++ b/arch/powerpc/include/asm/pci-bridge.h
> @@ -8,7 +8,6 @@
>   * 2 of the License, or (at your option) any later version.
>   */
>  #include <linux/pci.h>
> -#include <linux/list.h>
>  #include <linux/ioport.h>
>  #include <linux/of_pci.h>
>  #include <asm-generic/pci-bridge.h>
> @@ -16,85 +15,27 @@
>  struct device_node;
>
>  /*
> - * Structure of a PCI controller (host bridge)
> + * Used for variants of PCI indirect handling and possible quirks:
> + *  SET_CFG_TYPE - used on 4xx or any PHB that does explicit type0/1
> + *  EXT_REG - provides access to PCI-e extended registers
> + *  SURPRESS_PRIMARY_BUS - we suppress the setting of PCI_PRIMARY_BUS

s/SURPRESS/SUPRESS/ throughout.

I'm not sure how generic these flags will end up being.  If they wind
up in a shared header file, it seems like we might want a PCI_ prefix
to anchor them.

> + *   on Freescale PCI-e controllers since they used the PCI_PRIMARY_BUS
> + *   to determine which bus number to match on when generating type0
> + *   config cycles
> + *  NO_PCIE_LINK - the Freescale PCI-e controllers have issues with
> + *   hanging if we don't have link and try to do config cycles to
> + *   anything but the PHB.  Only allow talking to the PHB if this is
> + *   set.
> + *  BIG_ENDIAN - cfg_addr is a big endian register
> + *  BROKEN_MRM - the 440EPx/GRx chips have an errata that causes hangs on
> + *   the PLB4.  Effectively disable MRM commands by setting this.
>   */
> -struct pci_controller {
> -       struct pci_bus *bus;
> -       char is_dynamic;
> -#ifdef CONFIG_PPC64
> -       int node;
> -#endif
> -       struct device_node *dn;
> -       struct list_head list_node;
> -       struct device *parent;
> -
> -       int first_busno;
> -       int last_busno;
> -       int self_busno;
> -       struct resource busn;
> -
> -       void __iomem *io_base_virt;
> -#ifdef CONFIG_PPC64
> -       void *io_base_alloc;
> -#endif
> -       resource_size_t io_base_phys;
> -       resource_size_t pci_io_size;
> -
> -       /* Some machines (PReP) have a non 1:1 mapping of
> -        * the PCI memory space in the CPU bus space
> -        */
> -       resource_size_t pci_mem_offset;
> -
> -       /* Some machines have a special region to forward the ISA
> -        * "memory" cycles such as VGA memory regions. Left to 0
> -        * if unsupported
> -        */
> -       resource_size_t isa_mem_phys;
> -       resource_size_t isa_mem_size;
> -
> -       struct pci_ops *ops;
> -       unsigned int __iomem *cfg_addr;
> -       void __iomem *cfg_data;
> -
> -       /*
> -        * Used for variants of PCI indirect handling and possible quirks:
> -        *  SET_CFG_TYPE - used on 4xx or any PHB that does explicit type0/1
> -        *  EXT_REG - provides access to PCI-e extended registers
> -        *  SURPRESS_PRIMARY_BUS - we suppress the setting of PCI_PRIMARY_BUS
> -        *   on Freescale PCI-e controllers since they used the PCI_PRIMARY_BUS
> -        *   to determine which bus number to match on when generating type0
> -        *   config cycles
> -        *  NO_PCIE_LINK - the Freescale PCI-e controllers have issues with
> -        *   hanging if we don't have link and try to do config cycles to
> -        *   anything but the PHB.  Only allow talking to the PHB if this is
> -        *   set.
> -        *  BIG_ENDIAN - cfg_addr is a big endian register
> -        *  BROKEN_MRM - the 440EPx/GRx chips have an errata that causes hangs on
> -        *   the PLB4.  Effectively disable MRM commands by setting this.
> -        */
>  #define PPC_INDIRECT_TYPE_SET_CFG_TYPE         0x00000001
>  #define PPC_INDIRECT_TYPE_EXT_REG              0x00000002
>  #define PPC_INDIRECT_TYPE_SURPRESS_PRIMARY_BUS 0x00000004
>  #define PPC_INDIRECT_TYPE_NO_PCIE_LINK         0x00000008
>  #define PPC_INDIRECT_TYPE_BIG_ENDIAN           0x00000010
>  #define PPC_INDIRECT_TYPE_BROKEN_MRM           0x00000020
> -       u32 indirect_type;
> -       /* Currently, we limit ourselves to 1 IO range and 3 mem
> -        * ranges since the common pci_bus structure can't handle more
> -        */
> -       struct resource io_resource;
> -       struct resource mem_resources[3];
> -       int global_number;              /* PCI domain number */
> -
> -       resource_size_t dma_window_base_cur;
> -       resource_size_t dma_window_size;
> -
> -#ifdef CONFIG_PPC64
> -       unsigned long buid;
> -
> -       void *private_data;
> -#endif /* CONFIG_PPC64 */
> -};
>
>  /* These are used for config access before all the PCI probing
>     has been done. */
> diff --git a/include/asm-generic/pci-bridge.h b/include/asm-generic/pci-bridge.h
> index 20db2e5..e58830e 100644
> --- a/include/asm-generic/pci-bridge.h
> +++ b/include/asm-generic/pci-bridge.h
> @@ -9,6 +9,9 @@
>
>  #ifdef __KERNEL__
>
> +#include <linux/pci.h>
> +#include <linux/list.h>
> +
>  enum {
>         /* Force re-assigning all resources (ignore firmware
>          * setup completely)
> @@ -38,6 +41,71 @@ enum {
>         PCI_SCAN_ALL_PCIE_DEVS  = 0x00000040,
>  };
>
> +struct device_node;
> +
> +/*
> + * Structure of a PCI controller (host bridge)
> + */
> +#if defined(CONFIG_PPC32) || defined(CONFIG_PPC64)
> +struct pci_controller {
> +       struct pci_bus *bus;

A lot of the information here (struct pci_bus *bus, first_/last_busno,
busn, window information) is already in the struct pci_host_bridge,
and I'm not sure it makes sense to duplicate it here.  I'd like to use
that structure when it makes sense, and use the sysdata pointer, i.e.,
the pointer to "struct pci_controller" here, for things that are truly
arch-specific.

> +       char is_dynamic;
> +#ifdef CONFIG_PPC64
> +       int node;
> +#endif
> +       struct device_node *dn;
> +       struct list_head list_node;
> +       struct device *parent;
> +
> +       int first_busno;
> +       int last_busno;
> +       int self_busno;
> +#if defined(CONFIG_PPC64) || defined(CONFIG_PPC32)
> +       struct resource busn;
> +#endif
> +
> +       void __iomem *io_base_virt;
> +#ifdef CONFIG_PPC64
> +       void *io_base_alloc;
> +#endif
> +       resource_size_t io_base_phys;
> +       resource_size_t pci_io_size;
> +
> +       /* Some machines (PReP) have a non 1:1 mapping of
> +        * the PCI memory space in the CPU bus space
> +        */
> +       resource_size_t pci_mem_offset;
> +
> +       /* Some machines have a special region to forward the ISA
> +        * "memory" cycles such as VGA memory regions. Left to 0
> +        * if unsupported
> +        */
> +       resource_size_t isa_mem_phys;
> +       resource_size_t isa_mem_size;
> +
> +       struct pci_ops *ops;
> +       unsigned int __iomem *cfg_addr;
> +       void __iomem *cfg_data;
> +
> +       u32 indirect_type;
> +       /* Currently, we limit ourselves to 1 IO range and 3 mem
> +        * ranges since the common pci_bus structure can't handle more
> +        */
> +       struct resource io_resource;
> +       struct resource mem_resources[3];

pci_host_bridge has a list for apertures, which contains this
information (and the offset between CPU and PCI bus addresses).

> +       int global_number;              /* PCI domain number */

The domain is a good candidate for going in the pci_host_bridge
structure.  I'm not sure how to plumb it in there, but it is something
that's pretty generic and I'd like to get rid of all the arch-specific
pci_domain_nr() implementations that are really the same.

> +
> +       resource_size_t dma_window_base_cur;
> +       resource_size_t dma_window_size;
> +
> +#ifdef CONFIG_PPC64
> +       unsigned long buid;
> +
> +       void *private_data;
> +#endif /* CONFIG_PPC64 */
> +};
> +#endif
> +
>  #ifdef CONFIG_PCI
>  extern unsigned int pci_flags;
>
> --
> 1.7.0.4
>

^ permalink raw reply

* Re: [PATCH 4/4] powerpc/perf: Add support for SIER
From: Sukadev Bhattiprolu @ 2013-04-25 18:24 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <1365582765-6939-4-git-send-email-michael@ellerman.id.au>

Michael Ellerman [michael@ellerman.id.au] wrote:
| From: Michael Ellerman <michaele@au1.ibm.com>
| 
| On power8 we have a new SIER (Sampled Instruction Event Register), which
| captures information about instructions when we have random sampling
| enabled.
| 
| Add support for loading the SIER into pt_regs, overloading regs->dar.
| Also set the new NO_SIPR flag in regs->result if we don't have SIPR.
| 
| Update regs_sihv/sipr() to look for SIPR/SIHV in SIER.
| 
| Signed-off-by: Michael Ellerman <michaele@au1.ibm.com>
| ---
|  arch/powerpc/include/asm/perf_event_server.h |    1 +
|  arch/powerpc/perf/core-book3s.c              |   19 +++++++++++++++++++
|  2 files changed, 20 insertions(+)
| 
| diff --git a/arch/powerpc/include/asm/perf_event_server.h b/arch/powerpc/include/asm/perf_event_server.h
| index e287aef..a1a1ad8 100644
| --- a/arch/powerpc/include/asm/perf_event_server.h
| +++ b/arch/powerpc/include/asm/perf_event_server.h
| @@ -53,6 +53,7 @@ struct power_pmu {
|  #define PPMU_NO_CONT_SAMPLING	0x00000008 /* no continuous sampling */
|  #define PPMU_SIAR_VALID		0x00000010 /* Processor has SIAR Valid bit */
|  #define PPMU_HAS_SSLOT		0x00000020 /* Has sampled slot in MMCRA */
| +#define PPMU_HAS_SIER		0x00000040 /* Has SIER */
| 
|  /*
|   * Values for flags to get_alternatives()
| diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
| index 4255b12..a4bbd4d 100644
| --- a/arch/powerpc/perf/core-book3s.c
| +++ b/arch/powerpc/perf/core-book3s.c
| @@ -116,6 +116,9 @@ static bool regs_sihv(struct pt_regs *regs)
|  {
|  	unsigned long sihv = MMCRA_SIHV;
| 
| +	if (ppmu->flags & PPMU_HAS_SIER)
| +		return !!(regs->dar & SIER_SIHV);
| +

Were SIER_SIHV and SIER_SIPR defined in an earlier patch set ?

|  	if (ppmu->flags & PPMU_ALT_SIPR)
|  		sihv = POWER6_MMCRA_SIHV;
| 
| @@ -126,6 +129,9 @@ static bool regs_sipr(struct pt_regs *regs)
|  {
|  	unsigned long sipr = MMCRA_SIPR;
| 
| +	if (ppmu->flags & PPMU_HAS_SIER)
| +		return !!(regs->dar & SIER_SIPR);
| +
|  	if (ppmu->flags & PPMU_ALT_SIPR)
|  		sipr = POWER6_MMCRA_SIPR;
| 
| @@ -184,6 +190,7 @@ static inline u32 perf_get_misc_flags(struct pt_regs *regs)
|  /*
|   * Overload regs->dsisr to store MMCRA so we only need to read it once
|   * on each interrupt.
| + * Overload regs->dar to store SIER if we have it.
|   * Overload regs->result to specify whether we should use the MSR (result
|   * is zero) or the SIAR (result is non zero).
|   */
| @@ -200,6 +207,18 @@ static inline void perf_read_regs(struct pt_regs *regs)
|  		regs->result |= 2;
| 
|  	/*
| +	 * On power8 if we're in random sampling mode, the SIER is updated.
| +	 * If we're in continuous sampling mode, we don't have SIPR.
| +	 */
| +	if (ppmu->flags & PPMU_HAS_SIER) {
| +		if (marked)
| +			regs->dar = mfspr(SPRN_SIER);
| +		else
| +			regs->result |= 2;

Can we use a helper, regs_set_no_sipr() to set this - since we set and
test in more than one place ?

Other than these nits, the patchset looks good.

Sukadev

^ permalink raw reply

* Re: [PATCH 1/5] ibmebus: convert of_platform_driver to platform_driver
From: Benjamin Herrenschmidt @ 2013-04-25 17:35 UTC (permalink / raw)
  To: Rob Herring
  Cc: Roland Dreier, Arnd Bergmann, linux-rdma, netdev,
	devicetree-discuss@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	Christoph Raisch, Hoang-Nam Nguyen, Thadeu Lima de Souza Cascardo,
	Grant Likely, Paul Mackerras, Sean Hefty, linuxppc-dev,
	Hal Rosenstock
In-Reply-To: <CAL_JsqKDF=hPEM0Jz-zZSXBCXBZ=gBjLWL6QDvq+nFWkqY3DMw@mail.gmail.com>

On Thu, 2013-04-25 at 10:23 -0500, Rob Herring wrote:
> Ben, Can I have your Ack for this? The change is straightforward and
> neither of the 2 drivers used the id parameter that is removed.

Didn't you get my mail about a compile failure caused by this patch ?

Or did you send an update that I missed ?

(Copy of the original email below)

Cheers,
Ben.

----

On Sun, 2013-04-21 at 21:13 -0500, Rob Herring wrote:
> From: Rob Herring <rob.herring@calxeda.com>
> 
> ibmebus is the last remaining user of of_platform_driver and the
> conversion to a regular platform driver is trivial.

A quick build test leads to:

/home/benh/linux-powerpc-test/arch/powerpc/kernel/ibmebus.c: In function 'ibmebus_bus_device_probe':
/home/benh/linux-powerpc-test/arch/powerpc/kernel/ibmebus.c:344:2: error: implicit declaration of function 'to_platform_driver' [-Werror=implicit-function-declaration]
/home/benh/linux-powerpc-test/arch/powerpc/kernel/ibmebus.c:344:6: error: assignment makes pointer from integer without a cast [-Werror]
/home/benh/linux-powerpc-test/arch/powerpc/kernel/ibmebus.c: In function 'ibmebus_bus_device_remove':
/home/benh/linux-powerpc-test/arch/powerpc/kernel/ibmebus.c:363:32: error: initialization makes pointer from integer without a cast [-Werror]
/home/benh/linux-powerpc-test/arch/powerpc/kernel/ibmebus.c: In function 'ibmebus_bus_device_shutdown':
/home/benh/linux-powerpc-test/arch/powerpc/kernel/ibmebus.c:373:32: error: initialization makes pointer from integer without a cast [-Werror]
/home/benh/linux-powerpc-test/arch/powerpc/kernel/ibmebus.c: In function 'ibmebus_bus_legacy_suspend':
/home/benh/linux-powerpc-test/arch/powerpc/kernel/ibmebus.c:420:32: error: initialization makes pointer from integer without a cast [-Werror]
/home/benh/linux-powerpc-test/arch/powerpc/kernel/ibmebus.c: In function 'ibmebus_bus_legacy_resume':
/home/benh/linux-powerpc-test/arch/powerpc/kernel/ibmebus.c:431:32: error: initialization makes pointer from integer without a cast [-Werror]
cc1: all warnings being treated as errors
make[2]: *** [arch/powerpc/kernel/ibmebus.o] Error 1
make[2]: *** Waiting for unfinished jobs....

Haven't had a chance to look too closely today -EJETLAG :-)

^ permalink raw reply

* Re: [PATCHv4 1/2] ppc64: perform proper max_bus_speed detection
From: Lucas Kannebley Tavares @ 2013-04-25 17:34 UTC (permalink / raw)
  To: linuxppc-dev, dri-devel, Benjamin Herrenschmidt, Bjorn Helgaas,
	David Airlie <airlied@linux.ie> Michael Ellerman,
	Kleber Sacilotto de Souza, Alex Deucher, Jerome Glisse,
	Thadeu Lima de Souza Cascardo, Brian King
In-Reply-To: <20130424234838.GA1971@thor.bakeyournoodle.com>

On 04/24/2013 08:48 PM, Tony Breeds wrote:
> On Wed, Apr 24, 2013 at 07:54:49PM -0300, lucaskt@linux.vnet.ibm.com wrote:
>> From: Lucas Kannebley Tavares<lucaskt@linux.vnet.ibm.com>
>>
>> On pseries machines the detection for max_bus_speed should be done
>> through an OpenFirmware property. This patch adds a function to perform
>> this detection and a hook to perform dynamic adding of the function only for
>> pseries. This is done by overwriting the weak
>> pcibios_root_bridge_prepare function which is called by pci_create_root_bus().
>>
>> Signed-off-by: Lucas Kannebley Tavares<lucaskt@linux.vnet.ibm.com>
>> ---
>>   arch/powerpc/include/asm/machdep.h       |  2 ++
>>   arch/powerpc/kernel/pci-common.c         |  8 +++++
>>   arch/powerpc/platforms/pseries/pci.c     | 51 ++++++++++++++++++++++++++++++++
>>   arch/powerpc/platforms/pseries/pseries.h |  4 +++
>>   arch/powerpc/platforms/pseries/setup.c   |  2 ++
>>   5 files changed, 67 insertions(+)
>>
>> diff --git a/arch/powerpc/include/asm/machdep.h b/arch/powerpc/include/asm/machdep.h
>> index 3d6b410..8f558bf 100644
>> --- a/arch/powerpc/include/asm/machdep.h
>> +++ b/arch/powerpc/include/asm/machdep.h
>> @@ -107,6 +107,8 @@ struct machdep_calls {
>>   	void		(*pcibios_fixup)(void);
>>   	int		(*pci_probe_mode)(struct pci_bus *);
>>   	void		(*pci_irq_fixup)(struct pci_dev *dev);
>> +	int		(*pcibios_root_bridge_prepare)(struct pci_host_bridge
>> +				*bridge);
>>
>>   	/* To setup PHBs when using automatic OF platform driver for PCI */
>>   	int		(*pci_setup_phb)(struct pci_controller *host);
>> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
>> index fa12ae4..80986cf 100644
>> --- a/arch/powerpc/kernel/pci-common.c
>> +++ b/arch/powerpc/kernel/pci-common.c
>> @@ -844,6 +844,14 @@ int pci_proc_domain(struct pci_bus *bus)
>>   	return 1;
>>   }
>>
>> +int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
>> +{
>> +	if (ppc_md.pcibios_root_bridge_prepare)
>> +		return ppc_md.pcibios_root_bridge_prepare(bridge);
>> +
>> +	return 0;
>> +}
>> +
>>   /* This header fixup will do the resource fixup for all devices as they are
>>    * probed, but not for bridge ranges
>>    */
>> diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c
>> index 0b580f4..7f9c956 100644
>> --- a/arch/powerpc/platforms/pseries/pci.c
>> +++ b/arch/powerpc/platforms/pseries/pci.c
>> @@ -108,3 +108,54 @@ static void fixup_winbond_82c105(struct pci_dev* dev)
>>   }
>>   DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_WINBOND, PCI_DEVICE_ID_WINBOND_82C105,
>>   			 fixup_winbond_82c105);
>> +
>> +int pseries_root_bridge_prepare(struct pci_host_bridge *bridge)
>> +{
>> +	struct device_node *dn, *pdn;
>> +	struct pci_bus *bus;
>> +	const uint32_t *pcie_link_speed_stats;
>> +
>> +	bus = bridge->bus;
>> +
>> +	dn = pcibios_get_phb_of_node(bus);
>> +	if (!dn)
>> +		return 0;
>> +
>> +	for (pdn = dn; pdn != NULL; pdn = pdn->parent) {
>> +		pcie_link_speed_stats = (const uint32_t *) of_get_property(dn,
>> +			"ibm,pcie-link-speed-stats", NULL);
>> +		if (pcie_link_speed_stats)
>> +			break;
>> +	}
>
> Please use the helpers in include/linux/of.h rather than open coding
> this.
>
> Yours Tony


Hi Tony,


This is what I can find as an equivalent code:

	for (pdn = dn; pdn != NULL; pdn = of_get_next_parent(pdn)) {
		pcie_link_speed_stats = (const uint32_t *)
			of_get_property(dn,
				"ibm,pcie-link-speed-stats", NULL);
		if (pcie_link_speed_stats)
			break;
	}

is this your suggestion, or was it another approach that will have the 
same result?

Thanks,

-- 
Lucas Kannebley Tavares
Software Engineer
IBM Linux Technology Center

^ permalink raw reply

* Re: [PATCH 2/2 V7] powerpc/85xx: Add machine check handler to fix PCIe erratum on mpc85xx
From: Segher Boessenkool @ 2013-04-25 17:32 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, Jia Hongtao, B07421
In-Reply-To: <1366909096.30341.3@snotra>

>>> * Remove A variant of load instruction emulation
>> Why is this?  You handle all other simple load insns, there is
>> nothing special about LHA.  (I reviewed the V4 email thread, no
>> reason for the chance is given there).
>
> The LHA implementation in V5 was incorrect (didn't sign-extend).

The history on the current version says that it was _removed_ in V5 :-)
So an off-by-one in the history, I suppose.

It would be totally trivial to implement it correctly, seems a bit
silly to remove it?


Segher

^ permalink raw reply

* Re: [PATCH 1/4] powerpc/perf: Convert mmcra_sipr/sihv() to regs_sipr/sihv()
From: Sukadev Bhattiprolu @ 2013-04-25 17:11 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev, Paul Mackerras
In-Reply-To: <1365582765-6939-1-git-send-email-michael@ellerman.id.au>

Michael Ellerman [michael@ellerman.id.au] wrote:
| From: Michael Ellerman <michaele@au1.ibm.com>
| 
| On power8 the SIPR and SIHV are not in MMCRA, so convert the routines
| to take regs and change the names accordingly.
| 
| Signed-off-by: Michael Ellerman <michaele@au1.ibm.com>
| ---
|  arch/powerpc/perf/core-book3s.c |   20 +++++++++++---------
|  1 file changed, 11 insertions(+), 9 deletions(-)
| 
| diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
| index fcfafa0..cb1618d 100644
| --- a/arch/powerpc/perf/core-book3s.c
| +++ b/arch/powerpc/perf/core-book3s.c
| @@ -112,24 +112,24 @@ static inline void perf_get_data_addr(struct pt_regs *regs, u64 *addrp)
|  		*addrp = mfspr(SPRN_SDAR);
|  }
| 
| -static bool mmcra_sihv(unsigned long mmcra)
| +static bool regs_sihv(struct pt_regs *regs)
|  {
|  	unsigned long sihv = MMCRA_SIHV;
| 
|  	if (ppmu->flags & PPMU_ALT_SIPR)
|  		sihv = POWER6_MMCRA_SIHV;
| 
| -	return !!(mmcra & sihv);
| +	return !!(regs->dsisr & sihv);
|  }
| 
| -static bool mmcra_sipr(unsigned long mmcra)
| +static bool regs_sipr(struct pt_regs *regs)
|  {
|  	unsigned long sipr = MMCRA_SIPR;
| 

Would it make sense to add this here:

	if (ppmu->flags & PPMU_NO_SIPR)
		return 0;
so ....

|  	if (ppmu->flags & PPMU_ALT_SIPR)
|  		sipr = POWER6_MMCRA_SIPR;
| 
| -	return !!(mmcra & sipr);
| +	return !!(regs->dsisr & sipr);
|  }
| 
|  static inline u32 perf_flags_from_msr(struct pt_regs *regs)
| @@ -143,7 +143,6 @@ static inline u32 perf_flags_from_msr(struct pt_regs *regs)
| 
|  static inline u32 perf_get_misc_flags(struct pt_regs *regs)
|  {
| -	unsigned long mmcra = regs->dsisr;
|  	unsigned long use_siar = regs->result;
| 
|  	if (!use_siar)
| @@ -163,10 +162,12 @@ static inline u32 perf_get_misc_flags(struct pt_regs *regs)
|  	}
| 
|  	/* PR has priority over HV, so order below is important */
| -	if (mmcra_sipr(mmcra))
| +	if (regs_sipr(regs))
|  		return PERF_RECORD_MISC_USER;
| -	if (mmcra_sihv(mmcra) && (freeze_events_kernel != MMCR0_FCHV))
| +
| +	if (regs_sihv(regs) && (freeze_events_kernel != MMCR0_FCHV))
|  		return PERF_RECORD_MISC_HYPERVISOR;
| +
|  	return PERF_RECORD_MISC_KERNEL;
|  }
| 
| @@ -182,6 +183,8 @@ static inline void perf_read_regs(struct pt_regs *regs)
|  	int marked = mmcra & MMCRA_SAMPLE_ENABLE;
|  	int use_siar;
| 
| +	regs->dsisr = mmcra;
| +
|  	/*
|  	 * If this isn't a PMU exception (eg a software event) the SIAR is
|  	 * not valid. Use pt_regs.
| @@ -205,12 +208,11 @@ static inline void perf_read_regs(struct pt_regs *regs)
|  		use_siar = 1;
|  	else if ((ppmu->flags & PPMU_NO_CONT_SAMPLING))
|  		use_siar = 0;
| -	else if (!(ppmu->flags & PPMU_NO_SIPR) && mmcra_sipr(mmcra))
| +	else if (!(ppmu->flags & PPMU_NO_SIPR) && regs_sipr(regs))

... this becomes

	else if (regs_sipr(regs))

mmcra_sipr() is used currently in two places and both places check
the NO_SIPR flag.

The reason is that this line gets modified in PATCH 3/4 to:

	else if (!regs_no_sipr(regs) && regs_sipr(regs))

which is kind of hard to read. if (!not_X && X)

Sukadev

^ permalink raw reply

* Re: [PATCH 2/2 V7] powerpc/85xx: Add machine check handler to fix PCIe erratum on mpc85xx
From: Scott Wood @ 2013-04-25 16:58 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: linuxppc-dev, Jia Hongtao, B07421
In-Reply-To: <CA5B8197-1A55-4E6E-AE0C-CD015D97EC83@kernel.crashing.org>

On 04/25/2013 10:31:51 AM, Segher Boessenkool wrote:
>> * Remove A variant of load instruction emulation
>=20
> Why is this?  You handle all other simple load insns, there is
> nothing special about LHA.  (I reviewed the V4 email thread, no
> reason for the chance is given there).

The LHA implementation in V5 was incorrect (didn't sign-extend).

-Scott=

^ permalink raw reply

* Re: [PATCH 2/2 V7] powerpc/85xx: Add machine check handler to fix PCIe erratum on mpc85xx
From: Segher Boessenkool @ 2013-04-25 15:31 UTC (permalink / raw)
  To: Jia Hongtao; +Cc: B07421, linuxppc-dev
In-Reply-To: <1366684776-10946-2-git-send-email-hongtao.jia@freescale.com>

> * Remove A variant of load instruction emulation

Why is this?  You handle all other simple load insns, there is
nothing special about LHA.  (I reviewed the V4 email thread, no
reason for the chance is given there).


Segher

^ permalink raw reply

* Re: [PATCH 1/5] ibmebus: convert of_platform_driver to platform_driver
From: Rob Herring @ 2013-04-25 15:23 UTC (permalink / raw)
  To: linux-kernel@vger.kernel.org, devicetree-discuss@lists.ozlabs.org,
	Benjamin Herrenschmidt
  Cc: Roland Dreier, Arnd Bergmann, linux-rdma, netdev, Rob Herring,
	Christoph Raisch, Hoang-Nam Nguyen, Thadeu Lima de Souza Cascardo,
	Grant Likely, Paul Mackerras, Sean Hefty, linuxppc-dev,
	Hal Rosenstock
In-Reply-To: <1366596798-9457-2-git-send-email-robherring2@gmail.com>

On Sun, Apr 21, 2013 at 9:13 PM, Rob Herring <robherring2@gmail.com> wrote:
> From: Rob Herring <rob.herring@calxeda.com>
>
> ibmebus is the last remaining user of of_platform_driver and the
> conversion to a regular platform driver is trivial.
>
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Hoang-Nam Nguyen <hnguyen@de.ibm.com>
> Cc: Christoph Raisch <raisch@de.ibm.com>
> Cc: Roland Dreier <roland@kernel.org>
> Cc: Sean Hefty <sean.hefty@intel.com>
> Cc: Hal Rosenstock <hal.rosenstock@gmail.com>
> Cc: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
> Cc: Grant Likely <grant.likely@linaro.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-rdma@vger.kernel.org
> Cc: netdev@vger.kernel.org

Ben, Can I have your Ack for this? The change is straightforward and
neither of the 2 drivers used the id parameter that is removed.

Rob

> ---
>  arch/powerpc/include/asm/ibmebus.h        |    4 ++--
>  arch/powerpc/kernel/ibmebus.c             |   22 ++++++++++------------
>  drivers/infiniband/hw/ehca/ehca_main.c    |    5 ++---
>  drivers/net/ethernet/ibm/ehea/ehea_main.c |    8 +++-----
>  4 files changed, 17 insertions(+), 22 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/ibmebus.h b/arch/powerpc/include/asm/ibmebus.h
> index 1a9d9ae..088f95b 100644
> --- a/arch/powerpc/include/asm/ibmebus.h
> +++ b/arch/powerpc/include/asm/ibmebus.h
> @@ -48,8 +48,8 @@
>
>  extern struct bus_type ibmebus_bus_type;
>
> -int ibmebus_register_driver(struct of_platform_driver *drv);
> -void ibmebus_unregister_driver(struct of_platform_driver *drv);
> +int ibmebus_register_driver(struct platform_driver *drv);
> +void ibmebus_unregister_driver(struct platform_driver *drv);
>
>  int ibmebus_request_irq(u32 ist, irq_handler_t handler,
>                         unsigned long irq_flags, const char *devname,
> diff --git a/arch/powerpc/kernel/ibmebus.c b/arch/powerpc/kernel/ibmebus.c
> index 8220baa..16a7c23 100644
> --- a/arch/powerpc/kernel/ibmebus.c
> +++ b/arch/powerpc/kernel/ibmebus.c
> @@ -205,7 +205,7 @@ static int ibmebus_create_devices(const struct of_device_id *matches)
>         return ret;
>  }
>
> -int ibmebus_register_driver(struct of_platform_driver *drv)
> +int ibmebus_register_driver(struct platform_driver *drv)
>  {
>         /* If the driver uses devices that ibmebus doesn't know, add them */
>         ibmebus_create_devices(drv->driver.of_match_table);
> @@ -215,7 +215,7 @@ int ibmebus_register_driver(struct of_platform_driver *drv)
>  }
>  EXPORT_SYMBOL(ibmebus_register_driver);
>
> -void ibmebus_unregister_driver(struct of_platform_driver *drv)
> +void ibmebus_unregister_driver(struct platform_driver *drv)
>  {
>         driver_unregister(&drv->driver);
>  }
> @@ -338,11 +338,10 @@ static int ibmebus_bus_bus_match(struct device *dev, struct device_driver *drv)
>  static int ibmebus_bus_device_probe(struct device *dev)
>  {
>         int error = -ENODEV;
> -       struct of_platform_driver *drv;
> +       struct platform_driver *drv;
>         struct platform_device *of_dev;
> -       const struct of_device_id *match;
>
> -       drv = to_of_platform_driver(dev->driver);
> +       drv = to_platform_driver(dev->driver);
>         of_dev = to_platform_device(dev);
>
>         if (!drv->probe)
> @@ -350,9 +349,8 @@ static int ibmebus_bus_device_probe(struct device *dev)
>
>         of_dev_get(of_dev);
>
> -       match = of_match_device(drv->driver.of_match_table, dev);
> -       if (match)
> -               error = drv->probe(of_dev, match);
> +       if (of_driver_match_device(dev, dev->driver))
> +               error = drv->probe(of_dev);
>         if (error)
>                 of_dev_put(of_dev);
>
> @@ -362,7 +360,7 @@ static int ibmebus_bus_device_probe(struct device *dev)
>  static int ibmebus_bus_device_remove(struct device *dev)
>  {
>         struct platform_device *of_dev = to_platform_device(dev);
> -       struct of_platform_driver *drv = to_of_platform_driver(dev->driver);
> +       struct platform_driver *drv = to_platform_driver(dev->driver);
>
>         if (dev->driver && drv->remove)
>                 drv->remove(of_dev);
> @@ -372,7 +370,7 @@ static int ibmebus_bus_device_remove(struct device *dev)
>  static void ibmebus_bus_device_shutdown(struct device *dev)
>  {
>         struct platform_device *of_dev = to_platform_device(dev);
> -       struct of_platform_driver *drv = to_of_platform_driver(dev->driver);
> +       struct platform_driver *drv = to_platform_driver(dev->driver);
>
>         if (dev->driver && drv->shutdown)
>                 drv->shutdown(of_dev);
> @@ -419,7 +417,7 @@ struct device_attribute ibmebus_bus_device_attrs[] = {
>  static int ibmebus_bus_legacy_suspend(struct device *dev, pm_message_t mesg)
>  {
>         struct platform_device *of_dev = to_platform_device(dev);
> -       struct of_platform_driver *drv = to_of_platform_driver(dev->driver);
> +       struct platform_driver *drv = to_platform_driver(dev->driver);
>         int ret = 0;
>
>         if (dev->driver && drv->suspend)
> @@ -430,7 +428,7 @@ static int ibmebus_bus_legacy_suspend(struct device *dev, pm_message_t mesg)
>  static int ibmebus_bus_legacy_resume(struct device *dev)
>  {
>         struct platform_device *of_dev = to_platform_device(dev);
> -       struct of_platform_driver *drv = to_of_platform_driver(dev->driver);
> +       struct platform_driver *drv = to_platform_driver(dev->driver);
>         int ret = 0;
>
>         if (dev->driver && drv->resume)
> diff --git a/drivers/infiniband/hw/ehca/ehca_main.c b/drivers/infiniband/hw/ehca/ehca_main.c
> index f8a6291..982e3ef 100644
> --- a/drivers/infiniband/hw/ehca/ehca_main.c
> +++ b/drivers/infiniband/hw/ehca/ehca_main.c
> @@ -713,8 +713,7 @@ static struct attribute_group ehca_dev_attr_grp = {
>         .attrs = ehca_dev_attrs
>  };
>
> -static int ehca_probe(struct platform_device *dev,
> -                     const struct of_device_id *id)
> +static int ehca_probe(struct platform_device *dev)
>  {
>         struct ehca_shca *shca;
>         const u64 *handle;
> @@ -937,7 +936,7 @@ static struct of_device_id ehca_device_table[] =
>  };
>  MODULE_DEVICE_TABLE(of, ehca_device_table);
>
> -static struct of_platform_driver ehca_driver = {
> +static struct platform_driver ehca_driver = {
>         .probe       = ehca_probe,
>         .remove      = ehca_remove,
>         .driver = {
> diff --git a/drivers/net/ethernet/ibm/ehea/ehea_main.c b/drivers/net/ethernet/ibm/ehea/ehea_main.c
> index 328f47c..c6ecf70 100644
> --- a/drivers/net/ethernet/ibm/ehea/ehea_main.c
> +++ b/drivers/net/ethernet/ibm/ehea/ehea_main.c
> @@ -98,8 +98,7 @@ static struct ehea_fw_handle_array ehea_fw_handles;
>  static struct ehea_bcmc_reg_array ehea_bcmc_regs;
>
>
> -static int ehea_probe_adapter(struct platform_device *dev,
> -                             const struct of_device_id *id);
> +static int ehea_probe_adapter(struct platform_device *dev);
>
>  static int ehea_remove(struct platform_device *dev);
>
> @@ -112,7 +111,7 @@ static struct of_device_id ehea_device_table[] = {
>  };
>  MODULE_DEVICE_TABLE(of, ehea_device_table);
>
> -static struct of_platform_driver ehea_driver = {
> +static struct platform_driver ehea_driver = {
>         .driver = {
>                 .name = "ehea",
>                 .owner = THIS_MODULE,
> @@ -3255,8 +3254,7 @@ static void ehea_remove_device_sysfs(struct platform_device *dev)
>         device_remove_file(&dev->dev, &dev_attr_remove_port);
>  }
>
> -static int ehea_probe_adapter(struct platform_device *dev,
> -                             const struct of_device_id *id)
> +static int ehea_probe_adapter(struct platform_device *dev)
>  {
>         struct ehea_adapter *adapter;
>         const u64 *adapter_handle;
> --
> 1.7.10.4
>

^ permalink raw reply

* Re: [PATCH 04/28] proc: Supply PDE attribute setting accessor functions [RFC]
From: Vasant Hegde @ 2013-04-25 15:22 UTC (permalink / raw)
  To: David Howells
  Cc: alsa-devel, linux-pci, linux-wireless, linux-kernel,
	netfilter-devel, viro, netdev, linux-fsdevel, linuxppc-dev,
	linux-media
In-Reply-To: <20130416182606.27773.55054.stgit@warthog.procyon.org.uk>

On 04/16/2013 11:56 PM, David Howells wrote:
> Supply accessor functions to set attributes in proc_dir_entry structs.
>
> The following are supplied: proc_set_size() and proc_set_user().
>
> Signed-off-by: David Howells<dhowells@redhat.com>
> cc: linuxppc-dev@lists.ozlabs.org
> cc: linux-media@vger.kernel.org
> cc: netdev@vger.kernel.org
> cc: linux-wireless@vger.kernel.org
> cc: linux-pci@vger.kernel.org
> cc: netfilter-devel@vger.kernel.org
> cc: alsa-devel@alsa-project.org
> ---
>
>   arch/powerpc/kernel/proc_powerpc.c        |    2 +-
>   arch/powerpc/platforms/pseries/reconfig.c |    2 +-

arch/powerpc side changes looks good.

-Vasant

>   drivers/media/pci/ttpci/av7110_ir.c       |    2 +-
>   drivers/net/irda/vlsi_ir.c                |    2 +-
>   drivers/net/wireless/airo.c               |   34 +++++++++--------------------
>   drivers/pci/proc.c                        |    2 +-
>   fs/proc/generic.c                         |   13 +++++++++++
>   include/linux/proc_fs.h                   |    5 ++++
>   kernel/configs.c                          |    2 +-
>   kernel/profile.c                          |    2 +-
>   net/netfilter/xt_recent.c                 |    3 +--
>   sound/core/info.c                         |    2 +-
>   12 files changed, 38 insertions(+), 33 deletions(-)
>
> diff --git a/arch/powerpc/kernel/proc_powerpc.c b/arch/powerpc/kernel/proc_powerpc.c
> index 41d8ee9..feb8580 100644
> --- a/arch/powerpc/kernel/proc_powerpc.c
> +++ b/arch/powerpc/kernel/proc_powerpc.c
> @@ -83,7 +83,7 @@ static int __init proc_ppc64_init(void)
>   			&page_map_fops, vdso_data);
>   	if (!pde)
>   		return 1;
> -	pde->size = PAGE_SIZE;
> +	proc_set_size(pde, PAGE_SIZE);
>
>   	return 0;
>   }
> diff --git a/arch/powerpc/platforms/pseries/reconfig.c b/arch/powerpc/platforms/pseries/reconfig.c
> index d6491bd..f93cdf5 100644
> --- a/arch/powerpc/platforms/pseries/reconfig.c
> +++ b/arch/powerpc/platforms/pseries/reconfig.c
> @@ -452,7 +452,7 @@ static int proc_ppc64_create_ofdt(void)
>
>   	ent = proc_create("powerpc/ofdt", S_IWUSR, NULL,&ofdt_fops);
>   	if (ent)
> -		ent->size = 0;
> +		proc_set_size(ent, 0);
>
>   	return 0;
>   }
> diff --git a/drivers/media/pci/ttpci/av7110_ir.c b/drivers/media/pci/ttpci/av7110_ir.c
> index eb82286..0e763a7 100644
> --- a/drivers/media/pci/ttpci/av7110_ir.c
> +++ b/drivers/media/pci/ttpci/av7110_ir.c
> @@ -375,7 +375,7 @@ int av7110_ir_init(struct av7110 *av7110)
>   	if (av_cnt == 1) {
>   		e = proc_create("av7110_ir", S_IWUSR, NULL,&av7110_ir_proc_fops);
>   		if (e)
> -			e->size = 4 + 256 * sizeof(u16);
> +			proc_set_size(e, 4 + 256 * sizeof(u16));
>   	}
>
>   	tasklet_init(&av7110->ir.ir_tasklet, av7110_emit_key, (unsigned long)&av7110->ir);
> diff --git a/drivers/net/irda/vlsi_ir.c b/drivers/net/irda/vlsi_ir.c
> index e22cd4e..5f47584 100644
> --- a/drivers/net/irda/vlsi_ir.c
> +++ b/drivers/net/irda/vlsi_ir.c
> @@ -1678,7 +1678,7 @@ vlsi_irda_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>   			IRDA_WARNING("%s: failed to create proc entry\n",
>   				     __func__);
>   		} else {
> -			ent->size = 0;
> +			proc_set_size(ent, 0);
>   		}
>   		idev->proc_entry = ent;
>   	}
> diff --git a/drivers/net/wireless/airo.c b/drivers/net/wireless/airo.c
> index 66e398d..21d0233 100644
> --- a/drivers/net/wireless/airo.c
> +++ b/drivers/net/wireless/airo.c
> @@ -4507,73 +4507,63 @@ static int setup_proc_entry( struct net_device *dev,
>   					    airo_entry);
>   	if (!apriv->proc_entry)
>   		goto fail;
> -	apriv->proc_entry->uid = proc_kuid;
> -	apriv->proc_entry->gid = proc_kgid;
> +	proc_set_user(apriv->proc_entry, proc_kuid, proc_kgid);
>
>   	/* Setup the StatsDelta */
>   	entry = proc_create_data("StatsDelta", S_IRUGO&  proc_perm,
>   				 apriv->proc_entry,&proc_statsdelta_ops, dev);
>   	if (!entry)
>   		goto fail_stats_delta;
> -	entry->uid = proc_kuid;
> -	entry->gid = proc_kgid;
> +	proc_set_user(entry, proc_kuid, proc_kgid);
>
>   	/* Setup the Stats */
>   	entry = proc_create_data("Stats", S_IRUGO&  proc_perm,
>   				 apriv->proc_entry,&proc_stats_ops, dev);
>   	if (!entry)
>   		goto fail_stats;
> -	entry->uid = proc_kuid;
> -	entry->gid = proc_kgid;
> +	proc_set_user(entry, proc_kuid, proc_kgid);
>
>   	/* Setup the Status */
>   	entry = proc_create_data("Status", S_IRUGO&  proc_perm,
>   				 apriv->proc_entry,&proc_status_ops, dev);
>   	if (!entry)
>   		goto fail_status;
> -	entry->uid = proc_kuid;
> -	entry->gid = proc_kgid;
> +	proc_set_user(entry, proc_kuid, proc_kgid);
>
>   	/* Setup the Config */
>   	entry = proc_create_data("Config", proc_perm,
>   				 apriv->proc_entry,&proc_config_ops, dev);
>   	if (!entry)
>   		goto fail_config;
> -	entry->uid = proc_kuid;
> -	entry->gid = proc_kgid;
> +	proc_set_user(entry, proc_kuid, proc_kgid);
>
>   	/* Setup the SSID */
>   	entry = proc_create_data("SSID", proc_perm,
>   				 apriv->proc_entry,&proc_SSID_ops, dev);
>   	if (!entry)
>   		goto fail_ssid;
> -	entry->uid = proc_kuid;
> -	entry->gid = proc_kgid;
> +	proc_set_user(entry, proc_kuid, proc_kgid);
>
>   	/* Setup the APList */
>   	entry = proc_create_data("APList", proc_perm,
>   				 apriv->proc_entry,&proc_APList_ops, dev);
>   	if (!entry)
>   		goto fail_aplist;
> -	entry->uid = proc_kuid;
> -	entry->gid = proc_kgid;
> +	proc_set_user(entry, proc_kuid, proc_kgid);
>
>   	/* Setup the BSSList */
>   	entry = proc_create_data("BSSList", proc_perm,
>   				 apriv->proc_entry,&proc_BSSList_ops, dev);
>   	if (!entry)
>   		goto fail_bsslist;
> -	entry->uid = proc_kuid;
> -	entry->gid = proc_kgid;
> +	proc_set_user(entry, proc_kuid, proc_kgid);
>
>   	/* Setup the WepKey */
>   	entry = proc_create_data("WepKey", proc_perm,
>   				 apriv->proc_entry,&proc_wepkey_ops, dev);
>   	if (!entry)
>   		goto fail_wepkey;
> -	entry->uid = proc_kuid;
> -	entry->gid = proc_kgid;
> -
> +	proc_set_user(entry, proc_kuid, proc_kgid);
>   	return 0;
>
>   fail_wepkey:
> @@ -5695,10 +5685,8 @@ static int __init airo_init_module( void )
>
>   	airo_entry = proc_mkdir_mode("driver/aironet", airo_perm, NULL);
>
> -	if (airo_entry) {
> -		airo_entry->uid = proc_kuid;
> -		airo_entry->gid = proc_kgid;
> -	}
> +	if (airo_entry)
> +		proc_set_user(airo_entry, proc_kuid, proc_kgid);
>
>   	for (i = 0; i<  4&&  io[i]&&  irq[i]; i++) {
>   		airo_print_info("", "Trying to configure ISA adapter at irq=%d "
> diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c
> index 12e4fb5..7cde7c1 100644
> --- a/drivers/pci/proc.c
> +++ b/drivers/pci/proc.c
> @@ -419,7 +419,7 @@ int pci_proc_attach_device(struct pci_dev *dev)
>   			&proc_bus_pci_operations, dev);
>   	if (!e)
>   		return -ENOMEM;
> -	e->size = dev->cfg_size;
> +	proc_set_size(e, dev->cfg_size);
>   	dev->procent = e;
>
>   	return 0;
> diff --git a/fs/proc/generic.c b/fs/proc/generic.c
> index 1c07cad..5f6f6c3 100644
> --- a/fs/proc/generic.c
> +++ b/fs/proc/generic.c
> @@ -498,6 +498,19 @@ out:
>   	return NULL;
>   }
>   EXPORT_SYMBOL(proc_create_data);
> +
> +void proc_set_size(struct proc_dir_entry *de, loff_t size)
> +{
> +	de->size = size;
> +}
> +EXPORT_SYMBOL(proc_set_size);
> +
> +void proc_set_user(struct proc_dir_entry *de, kuid_t uid, kgid_t gid)
> +{
> +	de->uid = uid;
> +	de->gid = gid;
> +}
> +EXPORT_SYMBOL(proc_set_user);
>
>   static void free_proc_entry(struct proc_dir_entry *de)
>   {
> diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
> index 805edac..28a4d7e 100644
> --- a/include/linux/proc_fs.h
> +++ b/include/linux/proc_fs.h
> @@ -130,6 +130,9 @@ static inline struct proc_dir_entry *proc_create(const char *name, umode_t mode,
>   extern struct proc_dir_entry *proc_net_mkdir(struct net *net, const char *name,
>   	struct proc_dir_entry *parent);
>
> +extern void proc_set_size(struct proc_dir_entry *, loff_t);
> +extern void proc_set_user(struct proc_dir_entry *, kuid_t, kgid_t);
> +
>   extern struct file *proc_ns_fget(int fd);
>   extern bool proc_ns_inode(struct inode *inode);
>
> @@ -158,6 +161,8 @@ static inline struct proc_dir_entry *proc_mkdir(const char *name,
>   	struct proc_dir_entry *parent) {return NULL;}
>   static inline struct proc_dir_entry *proc_mkdir_mode(const char *name,
>   	umode_t mode, struct proc_dir_entry *parent) { return NULL; }
> +static inline void proc_set_size(struct proc_dir_entry *de, loff_t size) {}
> +static inline void proc_set_user(struct proc_dir_entry *de, kuid_t uid, kgid_t gid) {}
>
>   struct tty_driver;
>   static inline void proc_tty_register_driver(struct tty_driver *driver) {};
> diff --git a/kernel/configs.c b/kernel/configs.c
> index 42e8fa0..c18b1f1 100644
> --- a/kernel/configs.c
> +++ b/kernel/configs.c
> @@ -79,7 +79,7 @@ static int __init ikconfig_init(void)
>   	if (!entry)
>   		return -ENOMEM;
>
> -	entry->size = kernel_config_data_size;
> +	proc_set_size(entry, kernel_config_data_size);
>
>   	return 0;
>   }
> diff --git a/kernel/profile.c b/kernel/profile.c
> index 524ce5e..0bf4007 100644
> --- a/kernel/profile.c
> +++ b/kernel/profile.c
> @@ -600,7 +600,7 @@ int __ref create_proc_profile(void) /* false positive from hotcpu_notifier */
>   			    NULL,&proc_profile_operations);
>   	if (!entry)
>   		return 0;
> -	entry->size = (1+prof_len) * sizeof(atomic_t);
> +	proc_set_size(entry, (1 + prof_len) * sizeof(atomic_t));
>   	hotcpu_notifier(profile_cpu_callback, 0);
>   	return 0;
>   }
> diff --git a/net/netfilter/xt_recent.c b/net/netfilter/xt_recent.c
> index 3db2d38..1e657cf 100644
> --- a/net/netfilter/xt_recent.c
> +++ b/net/netfilter/xt_recent.c
> @@ -401,8 +401,7 @@ static int recent_mt_check(const struct xt_mtchk_param *par,
>   		ret = -ENOMEM;
>   		goto out;
>   	}
> -	pde->uid = uid;
> -	pde->gid = gid;
> +	proc_set_user(pde, uid, gid);
>   #endif
>   	spin_lock_bh(&recent_lock);
>   	list_add_tail(&t->list,&recent_net->tables);
> diff --git a/sound/core/info.c b/sound/core/info.c
> index 3aa8864..c7f41c3 100644
> --- a/sound/core/info.c
> +++ b/sound/core/info.c
> @@ -970,7 +970,7 @@ int snd_info_register(struct snd_info_entry * entry)
>   			mutex_unlock(&info_mutex);
>   			return -ENOMEM;
>   		}
> -		p->size = entry->size;
> +		proc_set_size(p, entry->size);
>   	}
>   	entry->p = p;
>   	if (entry->parent)
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>

^ permalink raw reply

* Re: [PATCH 23/28] ppc: Clean up scanlog [RFC]
From: Vasant Hegde @ 2013-04-25 15:01 UTC (permalink / raw)
  To: David Howells
  Cc: linux-fsdevel, Paul Mackerras, linuxppc-dev, linux-kernel, viro
In-Reply-To: <20130416182722.27773.32105.stgit@warthog.procyon.org.uk>

On 04/16/2013 11:57 PM, David Howells wrote:
> Clean up the pseries scanlog driver's use of procfs:
>
>   (1) Don't need to save the proc_dir_entry pointer as we have the filename to
>       remove with.
>
>   (2) Save the scan log buffer pointer in a static variable (there is only one
>       of it) and don't save it in the PDE (which doesn't have a destructor).

Changes looks good.

-Vasant

>
> Signed-off-by: David Howells<dhowells@redhat.com>
> cc: Benjamin Herrenschmidt<benh@kernel.crashing.org>
> cc: Paul Mackerras<paulus@samba.org>
> cc: linuxppc-dev@lists.ozlabs.org
> ---
>
>   arch/powerpc/platforms/pseries/scanlog.c |   29 +++++++++++------------------
>   1 file changed, 11 insertions(+), 18 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/scanlog.c b/arch/powerpc/platforms/pseries/scanlog.c
> index cc220d2..b502ab6 100644
> --- a/arch/powerpc/platforms/pseries/scanlog.c
> +++ b/arch/powerpc/platforms/pseries/scanlog.c
> @@ -41,12 +41,12 @@
>
>
>   static unsigned int ibm_scan_log_dump;			/* RTAS token */
> -static struct proc_dir_entry *proc_ppc64_scan_log_dump;	/* The proc file */
> +static unsigned int *scanlog_buffer;			/* The data buffer */
>
>   static ssize_t scanlog_read(struct file *file, char __user *buf,
>   			    size_t count, loff_t *ppos)
>   {
> -	unsigned int *data = PDE_DATA(file_inode(file));
> +	unsigned int *data = scanlog_buffer;
>   	int status;
>   	unsigned long len, off;
>   	unsigned int wait_time;
> @@ -134,7 +134,7 @@ static ssize_t scanlog_write(struct file * file, const char __user * buf,
>
>   static int scanlog_open(struct inode * inode, struct file * file)
>   {
> -	unsigned int *data = PDE_DATA(file_inode(file));
> +	unsigned int *data = scanlog_buffer;
>
>   	if (data[0] != 0) {
>   		/* This imperfect test stops a second copy of the
> @@ -150,10 +150,9 @@ static int scanlog_open(struct inode * inode, struct file * file)
>
>   static int scanlog_release(struct inode * inode, struct file * file)
>   {
> -	unsigned int *data = PDE_DATA(file_inode(file));
> +	unsigned int *data = scanlog_buffer;
>
>   	data[0] = 0;
> -
>   	return 0;
>   }
>
> @@ -169,7 +168,6 @@ const struct file_operations scanlog_fops = {
>   static int __init scanlog_init(void)
>   {
>   	struct proc_dir_entry *ent;
> -	void *data;
>   	int err = -ENOMEM;
>
>   	ibm_scan_log_dump = rtas_token("ibm,scan-log-dump");
> @@ -177,29 +175,24 @@ static int __init scanlog_init(void)
>   		return -ENODEV;
>
>   	/* Ideally we could allocate a buffer<  4G */
> -	data = kzalloc(RTAS_DATA_BUF_SIZE, GFP_KERNEL);
> -	if (!data)
> +	scanlog_buffer = kzalloc(RTAS_DATA_BUF_SIZE, GFP_KERNEL);
> +	if (!scanlog_buffer)
>   		goto err;
>
> -	ent = proc_create_data("powerpc/rtas/scan-log-dump", S_IRUSR, NULL,
> -			&scanlog_fops, data);
> +	ent = proc_create("powerpc/rtas/scan-log-dump", S_IRUSR, NULL,
> +			&scanlog_fops);
>   	if (!ent)
>   		goto err;
> -
> -	proc_ppc64_scan_log_dump = ent;
> -
>   	return 0;
>   err:
> -	kfree(data);
> +	kfree(scanlog_buffer);
>   	return err;
>   }
>
>   static void __exit scanlog_cleanup(void)
>   {
> -	if (proc_ppc64_scan_log_dump) {
> -		kfree(proc_ppc64_scan_log_dump->data);
> -		remove_proc_entry("scan-log-dump", proc_ppc64_scan_log_dump->parent);
> -	}
> +	remove_proc_entry("powerpc/rtas/scan-log-dump", NULL);
> +	kfree(scanlog_buffer);
>   }
>
>   module_init(scanlog_init);
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>

^ permalink raw reply

* Re: [PATCH 22/28] ppc: Clean up rtas_flash driver somewhat [RFC]
From: Vasant Hegde @ 2013-04-25 14:33 UTC (permalink / raw)
  To: David Howells
  Cc: linux-kernel, Paul Mackerras, Anton Blanchard, linux-fsdevel,
	linuxppc-dev, viro
In-Reply-To: <20130416182716.27773.74530.stgit@warthog.procyon.org.uk>

On 04/16/2013 11:57 PM, David Howells wrote:
> Clean up some of the problems with the rtas_flash driver:
>
>   (1) It shouldn't fiddle with the internals of the procfs filesystem (altering
>       pde->count).
>
>   (2) If pid namespaces are in effect, then you can get multiple inodes
>       connected to a single pde, thereby rendering the pde->count>  2 test
>       useless.
>
>   (3) The pde->count fudging doesn't work for forked, dup'd or cloned file
>       descriptors, so add static mutexes and use them to wrap access to the
>       driver through read, write and release methods.
>
>   (4) The driver can only handle one device, so allocate most of the data
>       previously attached to the pde->data as static variables instead (though
>       allocate the validation data buffer with kmalloc).
>
>   (5) We don't need to save the pde pointers as long as we have the filenames
>       available for removal.
>
>   (6) Don't try to multiplex what the update file read method does based on the
>       filename.  Instead provide separate file ops and split the function.
>
> Whilst we're at it, tabulate the procfile information and loop through it when
> creating or destroying them rather than manually coding each one.
>
> Signed-off-by: David Howells<dhowells@redhat.com>
> cc: Benjamin Herrenschmidt<benh@kernel.crashing.org>
> cc: Paul Mackerras<paulus@samba.org>
> cc: Anton Blanchard<anton@samba.org>
> cc: linuxppc-dev@lists.ozlabs.org
> ---
>
>   arch/powerpc/kernel/rtas_flash.c |  446 +++++++++++++++++---------------------
>   1 file changed, 200 insertions(+), 246 deletions(-)
>
> diff --git a/arch/powerpc/kernel/rtas_flash.c b/arch/powerpc/kernel/rtas_flash.c
> index c642f01..8196bfb 100644
> --- a/arch/powerpc/kernel/rtas_flash.c
> +++ b/arch/powerpc/kernel/rtas_flash.c
> @@ -102,9 +102,10 @@ static struct kmem_cache *flash_block_cache = NULL;
>
>   #define FLASH_BLOCK_LIST_VERSION (1UL)
>
> -/* Local copy of the flash block list.
> - * We only allow one open of the flash proc file and create this
> - * list as we go.  The rtas_firmware_flash_list varable will be
> +/*
> + * Local copy of the flash block list.
> + *
> + * The rtas_firmware_flash_list varable will be

s/varable/variable/

>    * set once the data is fully read.
>    *
>    * For convenience as we build the list we use virtual addrs,
> @@ -125,23 +126,23 @@ struct rtas_update_flash_t
>   struct rtas_manage_flash_t
>   {
>   	int status;			/* Returned status */
> -	unsigned int op;		/* Reject or commit image */
>   };
>
>   /* Status int must be first member of struct */
>   struct rtas_validate_flash_t
>   {
>   	int status;		 	/* Returned status */	
> -	char buf[VALIDATE_BUF_SIZE]; 	/* Candidate image buffer */
> +	char *buf;			/* Candidate image buffer */
>   	unsigned int buf_size;		/* Size of image buf */
>   	unsigned int update_results;	/* Update results token */
>   };
>
> -static DEFINE_SPINLOCK(flash_file_open_lock);
> -static struct proc_dir_entry *firmware_flash_pde;
> -static struct proc_dir_entry *firmware_update_pde;
> -static struct proc_dir_entry *validate_pde;
> -static struct proc_dir_entry *manage_pde;
> +static struct rtas_update_flash_t rtas_update_flash_data;
> +static struct rtas_manage_flash_t rtas_manage_flash_data;
> +static struct rtas_validate_flash_t rtas_validate_flash_data;
> +static DEFINE_MUTEX(rtas_update_flash_mutex);
> +static DEFINE_MUTEX(rtas_manage_flash_mutex);
> +static DEFINE_MUTEX(rtas_validate_flash_mutex);
>
>   /* Do simple sanity checks on the flash image. */
>   static int flash_list_valid(struct flash_block_list *flist)
> @@ -191,10 +192,10 @@ static void free_flash_list(struct flash_block_list *f)
>
>   static int rtas_flash_release(struct inode *inode, struct file *file)
>   {
> -	struct proc_dir_entry *dp = PDE(file_inode(file));
> -	struct rtas_update_flash_t *uf;
> -	
> -	uf = (struct rtas_update_flash_t *) dp->data;
> +	struct rtas_update_flash_t *const uf =&rtas_update_flash_data;
> +
> +	mutex_lock(&rtas_update_flash_mutex);
> +
>   	if (uf->flist) {
>   		/* File was opened in write mode for a new flash attempt */
>   		/* Clear saved list */
> @@ -214,13 +215,14 @@ static int rtas_flash_release(struct inode *inode, struct file *file)
>   		uf->flist = NULL;
>   	}
>
> -	atomic_dec(&dp->count);
> +	mutex_unlock(&rtas_update_flash_mutex);
>   	return 0;
>   }
>
> -static void get_flash_status_msg(int status, char *buf)
> +static size_t get_flash_status_msg(int status, char *buf)
>   {
> -	char *msg;
> +	const char *msg;
> +	size_t len;
>
>   	switch (status) {
>   	case FLASH_AUTH:
> @@ -242,34 +244,51 @@ static void get_flash_status_msg(int status, char *buf)
>   		msg = "ready: firmware image ready for flash on reboot\n";
>   		break;
>   	default:
> -		sprintf(buf, "error: unexpected status value %d\n", status);
> -		return;
> +		return sprintf(buf, "error: unexpected status value %d\n",
> +			       status);
>   	}
>
> -	strcpy(buf, msg);	
> +	len = strlen(msg);
> +	memcpy(buf, msg, len + 1);
> +	return len;
>   }
>
>   /* Reading the proc file will show status (not the firmware contents) */
> -static ssize_t rtas_flash_read(struct file *file, char __user *buf,
> -			       size_t count, loff_t *ppos)
> +static ssize_t rtas_flash_read_msg(struct file *file, char __user *buf,
> +				   size_t count, loff_t *ppos)
>   {
> -	struct proc_dir_entry *dp = PDE(file_inode(file));
> -	struct rtas_update_flash_t *uf;
> +	struct rtas_update_flash_t *const uf =&rtas_update_flash_data;
>   	char msg[RTAS_MSG_MAXLEN];
> +	size_t len;
> +	int status;
>
> -	uf = dp->data;
> +	mutex_lock(&rtas_update_flash_mutex);
> +	status = uf->status;
> +	mutex_unlock(&rtas_update_flash_mutex);
>
> -	if (!strcmp(dp->name, FIRMWARE_FLASH_NAME)) {
> -		get_flash_status_msg(uf->status, msg);
> -	} else {	   /* FIRMWARE_UPDATE_NAME */
> -		sprintf(msg, "%d\n", uf->status);
> -	}
> +	/* Read as text message */
> +	len = get_flash_status_msg(uf->status, msg);

Above line should be
	len = get_flash_status_msg(status, msg);

> +	return simple_read_from_buffer(buf, count, ppos, msg, len);
> +}
> +
> +static ssize_t rtas_flash_read_num(struct file *file, char __user *buf,
> +				   size_t count, loff_t *ppos)
> +{
> +	struct rtas_update_flash_t *const uf =&rtas_update_flash_data;
> +	char msg[RTAS_MSG_MAXLEN];
> +	int status;
>
> +	mutex_lock(&rtas_update_flash_mutex);
> +	status = uf->status;
> +	mutex_unlock(&rtas_update_flash_mutex);
> +
> +	/* Read as number */
> +	sprintf(msg, "%d\n", status);
>   	return simple_read_from_buffer(buf, count, ppos, msg, strlen(msg));
>   }
>
>   /* constructor for flash_block_cache */
> -void rtas_block_ctor(void *ptr)
> +static void rtas_block_ctor(void *ptr)
>   {
>   	memset(ptr, 0, RTAS_BLK_SIZE);
>   }
> @@ -282,16 +301,15 @@ void rtas_block_ctor(void *ptr)
>   static ssize_t rtas_flash_write(struct file *file, const char __user *buffer,
>   				size_t count, loff_t *off)
>   {
> -	struct proc_dir_entry *dp = PDE(file_inode(file));
> -	struct rtas_update_flash_t *uf;
> +	struct rtas_update_flash_t *const uf =&rtas_update_flash_data;
>   	char *p;
> -	int next_free;
> +	int next_free, rc;
>   	struct flash_block_list *fl;
>
> -	uf = (struct rtas_update_flash_t *) dp->data;
> +	mutex_lock(&rtas_update_flash_mutex);
>
>   	if (uf->status == FLASH_AUTH || count == 0)
> -		return count;	/* discard data */
> +		goto out;	/* discard data */
>
>   	/* In the case that the image is not ready for flashing, the memory
>   	 * allocated for the block list will be freed upon the release of the
> @@ -300,7 +318,7 @@ static ssize_t rtas_flash_write(struct file *file, const char __user *buffer,
>   	if (uf->flist == NULL) {
>   		uf->flist = kmem_cache_alloc(flash_block_cache, GFP_KERNEL);
>   		if (!uf->flist)
> -			return -ENOMEM;
> +			goto nomem;
>   	}
>
>   	fl = uf->flist;
> @@ -311,7 +329,7 @@ static ssize_t rtas_flash_write(struct file *file, const char __user *buffer,
>   		/* Need to allocate another block_list */
>   		fl->next = kmem_cache_alloc(flash_block_cache, GFP_KERNEL);
>   		if (!fl->next)
> -			return -ENOMEM;
> +			goto nomem;
>   		fl = fl->next;
>   		next_free = 0;
>   	}
> @@ -320,52 +338,37 @@ static ssize_t rtas_flash_write(struct file *file, const char __user *buffer,
>   		count = RTAS_BLK_SIZE;
>   	p = kmem_cache_alloc(flash_block_cache, GFP_KERNEL);
>   	if (!p)
> -		return -ENOMEM;
> +		goto nomem;
>   	
>   	if(copy_from_user(p, buffer, count)) {
>   		kmem_cache_free(flash_block_cache, p);
> -		return -EFAULT;
> +		rc = -EFAULT;
> +		goto error;
>   	}
>   	fl->blocks[next_free].data = p;
>   	fl->blocks[next_free].length = count;
>   	fl->num_blocks++;
> -
> +out:
> +	mutex_lock(&rtas_update_flash_mutex);

Above line should be "mutext_unlock(....)".


>   	return count;
> -}
> -
> -static int rtas_excl_open(struct inode *inode, struct file *file)
> -{
> -	struct proc_dir_entry *dp = PDE(inode);
> -
> -	/* Enforce exclusive open with use count of PDE */
> -	spin_lock(&flash_file_open_lock);
> -	if (atomic_read(&dp->count)>  2) {
> -		spin_unlock(&flash_file_open_lock);
> -		return -EBUSY;
> -	}
> -
> -	atomic_inc(&dp->count);
> -	spin_unlock(&flash_file_open_lock);
> -	
> -	return 0;
> -}
> -
> -static int rtas_excl_release(struct inode *inode, struct file *file)
> -{
> -	struct proc_dir_entry *dp = PDE(inode);
>
> -	atomic_dec(&dp->count);
> -
> -	return 0;
> +nomem:
> +	rc = -ENOMEM;
> +error:
> +	mutex_lock(&rtas_update_flash_mutex);

Again, above line should be "mutex_unlock".

> +	return rc;
>   }
>
> -static void manage_flash(struct rtas_manage_flash_t *args_buf)
> +/*
> + * Flash management routines.
> + */
> +static void manage_flash(struct rtas_manage_flash_t *args_buf, unsigned int op)
>   {
>   	s32 rc;
>
>   	do {
> -		rc = rtas_call(rtas_token("ibm,manage-flash-image"), 1,
> -			       1, NULL, args_buf->op);
> +		rc = rtas_call(rtas_token("ibm,manage-flash-image"), 1, 1,
> +			       NULL, op);
>   	} while (rtas_busy_delay(rc));
>
>   	args_buf->status = rc;
> @@ -374,40 +377,38 @@ static void manage_flash(struct rtas_manage_flash_t *args_buf)
>   static ssize_t manage_flash_read(struct file *file, char __user *buf,
>   			       size_t count, loff_t *ppos)
>   {
> -	struct proc_dir_entry *dp = PDE(file_inode(file));
> -	struct rtas_manage_flash_t *args_buf;
> +	struct rtas_manage_flash_t *const args_buf =&rtas_manage_flash_data;
>   	char msg[RTAS_MSG_MAXLEN];
> -	int msglen;
> +	int msglen, status;
>
> -	args_buf = dp->data;
> -	if (args_buf == NULL)
> -		return 0;
> -
> -	msglen = sprintf(msg, "%d\n", args_buf->status);
> +	mutex_lock(&rtas_manage_flash_mutex);
> +	status = args_buf->status;
> +	mutex_unlock(&rtas_manage_flash_mutex);
>
> +	msglen = sprintf(msg, "%d\n", status);
>   	return simple_read_from_buffer(buf, count, ppos, msg, msglen);
>   }
>
>   static ssize_t manage_flash_write(struct file *file, const char __user *buf,
>   				size_t count, loff_t *off)
>   {
> -	struct proc_dir_entry *dp = PDE(file_inode(file));
> -	struct rtas_manage_flash_t *args_buf;
> -	const char reject_str[] = "0";
> -	const char commit_str[] = "1";
> +	struct rtas_manage_flash_t *const args_buf =&rtas_manage_flash_data;
> +	static const char reject_str[] = "0";
> +	static const char commit_str[] = "1";
>   	char stkbuf[10];
> -	int op;
> +	int op, rc;
> +
> +	mutex_lock(&rtas_manage_flash_mutex);
>
> -	args_buf = (struct rtas_manage_flash_t *) dp->data;
>   	if ((args_buf->status == MANAGE_AUTH) || (count == 0))
> -		return count;
> +		goto out;
>   		
>   	op = -1;
>   	if (buf) {
>   		if (count>  9) count = 9;
> -		if (copy_from_user (stkbuf, buf, count)) {
> -			return -EFAULT;
> -		}
> +		rc = -EFAULT;
> +		if (copy_from_user (stkbuf, buf, count))
> +			goto error;
>   		if (strncmp(stkbuf, reject_str, strlen(reject_str)) == 0)
>   			op = RTAS_REJECT_TMP_IMG;
>   		else if (strncmp(stkbuf, commit_str, strlen(commit_str)) == 0)
> @@ -417,12 +418,19 @@ static ssize_t manage_flash_write(struct file *file, const char __user *buf,
>   	if (op == -1)   /* buf is empty, or contains invalid string */
>   		return -EINVAL;
>

We need to release mutex here.

+	if (op == -1) {   /* buf is empty, or contains invalid string */
+		rc = -EINVAL;
+		goto error;
+	}



> -	args_buf->op = op;
> -	manage_flash(args_buf);
> -
> +	manage_flash(args_buf, op);
> +out:
> +	mutex_unlock(&rtas_manage_flash_mutex);
>   	return count;
> +
> +error:
> +	mutex_unlock(&rtas_manage_flash_mutex);
> +	return rc;
>   }
>
> +/*
> + * Validation routines.
> + */
>   static void validate_flash(struct rtas_validate_flash_t *args_buf)
>   {
>   	int token = rtas_token("ibm,validate-flash-image");
> @@ -462,14 +470,14 @@ static int get_validate_flash_msg(struct rtas_validate_flash_t *args_buf,
>   static ssize_t validate_flash_read(struct file *file, char __user *buf,
>   			       size_t count, loff_t *ppos)
>   {
> -	struct proc_dir_entry *dp = PDE(file_inode(file));
> -	struct rtas_validate_flash_t *args_buf;
> +	struct rtas_validate_flash_t *const args_buf =
> +		&rtas_validate_flash_data;
>   	char msg[RTAS_MSG_MAXLEN];
>   	int msglen;
>
> -	args_buf = dp->data;
> -
> +	mutex_lock(&rtas_validate_flash_mutex);
>   	msglen = get_validate_flash_msg(args_buf, msg);
> +	mutex_unlock(&rtas_validate_flash_mutex);
>
>   	return simple_read_from_buffer(buf, count, ppos, msg, msglen);
>   }
> @@ -477,24 +485,18 @@ static ssize_t validate_flash_read(struct file *file, char __user *buf,
>   static ssize_t validate_flash_write(struct file *file, const char __user *buf,
>   				    size_t count, loff_t *off)
>   {
> -	struct proc_dir_entry *dp = PDE(file_inode(file));
> -	struct rtas_validate_flash_t *args_buf;
> +	struct rtas_validate_flash_t *const args_buf =
> +		&rtas_validate_flash_data;
>   	int rc;
>
> -	args_buf = (struct rtas_validate_flash_t *) dp->data;
> -
> -	if (dp->data == NULL) {
> -		dp->data = kmalloc(sizeof(struct rtas_validate_flash_t),
> -				GFP_KERNEL);
> -		if (dp->data == NULL)
> -			return -ENOMEM;
> -	}
> +	mutex_lock(&rtas_validate_flash_mutex);
>
>   	/* We are only interested in the first 4K of the
>   	 * candidate image */
>   	if ((*off>= VALIDATE_BUF_SIZE) ||
>   		(args_buf->status == VALIDATE_AUTH)) {
>   		*off += count;
> +		mutex_unlock(&rtas_validate_flash_mutex);
>   		return count;
>   	}
>
> @@ -517,31 +519,29 @@ static ssize_t validate_flash_write(struct file *file, const char __user *buf,
>   	*off += count;
>   	rc = count;
>   done:
> -	if (rc<  0) {
> -		kfree(dp->data);
> -		dp->data = NULL;
> -	}
> +	mutex_unlock(&rtas_validate_flash_mutex);
>   	return rc;
>   }
>
>   static int validate_flash_release(struct inode *inode, struct file *file)
>   {
> -	struct proc_dir_entry *dp = PDE(file_inode(file));
> -	struct rtas_validate_flash_t *args_buf;
> +	struct rtas_validate_flash_t *const args_buf =
> +		&rtas_validate_flash_data;
>
> -	args_buf = (struct rtas_validate_flash_t *) dp->data;
> +	mutex_lock(&rtas_validate_flash_mutex);
>
>   	if (args_buf->status == VALIDATE_READY) {
>   		args_buf->buf_size = VALIDATE_BUF_SIZE;
>   		validate_flash(args_buf);
>   	}
>
> -	/* The matching atomic_inc was in rtas_excl_open() */
> -	atomic_dec(&dp->count);
> -
> +	mutex_unlock(&rtas_validate_flash_mutex);
>   	return 0;
>   }
>
> +/*
> + * On-reboot flash update applicator.
> + */
>   static void rtas_flash_firmware(int reboot_type)
>   {
>   	unsigned long image_size;
> @@ -634,75 +634,57 @@ static void rtas_flash_firmware(int reboot_type)
>   	spin_unlock(&rtas_data_buf_lock);
>   }
>
> -static void remove_flash_pde(struct proc_dir_entry *dp)
> -{
> -	if (dp) {
> -		kfree(dp->data);
> -		remove_proc_entry(dp->name, dp->parent);
> -	}
> -}
> -
> -static int initialize_flash_pde_data(const char *rtas_call_name,
> -				     size_t buf_size,
> -				     struct proc_dir_entry *dp)
> -{
> +/*
> + * Manifest of proc files to create
> + */
> +struct rtas_flash_file {
> +	const char *filename;
> +	const char *rtas_call_name;
>   	int *status;
> -	int token;
> -
> -	dp->data = kzalloc(buf_size, GFP_KERNEL);
> -	if (dp->data == NULL)
> -		return -ENOMEM;
> -
> -	/*
> -	 * This code assumes that the status int is the first member of the
> -	 * struct
> -	 */
> -	status = (int *) dp->data;
> -	token = rtas_token(rtas_call_name);
> -	if (token == RTAS_UNKNOWN_SERVICE)
> -		*status = FLASH_AUTH;
> -	else
> -		*status = FLASH_NO_OP;
> -
> -	return 0;
> -}
> -
> -static struct proc_dir_entry *create_flash_pde(const char *filename,
> -					       const struct file_operations *fops)
> -{
> -	return proc_create(filename, S_IRUSR | S_IWUSR, NULL, fops);
> -}
> -
> -static const struct file_operations rtas_flash_operations = {
> -	.owner		= THIS_MODULE,
> -	.read		= rtas_flash_read,
> -	.write		= rtas_flash_write,
> -	.open		= rtas_excl_open,
> -	.release	= rtas_flash_release,
> -	.llseek		= default_llseek,
> +	const struct file_operations fops;
>   };
>
> -static const struct file_operations manage_flash_operations = {
> -	.owner		= THIS_MODULE,
> -	.read		= manage_flash_read,
> -	.write		= manage_flash_write,
> -	.open		= rtas_excl_open,
> -	.release	= rtas_excl_release,
> -	.llseek		= default_llseek,
> -};
> -
> -static const struct file_operations validate_flash_operations = {
> -	.owner		= THIS_MODULE,
> -	.read		= validate_flash_read,
> -	.write		= validate_flash_write,
> -	.open		= rtas_excl_open,
> -	.release	= validate_flash_release,
> -	.llseek		= default_llseek,
> +static const struct rtas_flash_file rtas_flash_files[] = {
> +	{
> +		.filename	= "powerpc/rtas/" FIRMWARE_FLASH_NAME,
> +		.rtas_call_name	= "ibm,update-flash-64-and-reboot",
> +		.status		=&rtas_update_flash_data.status,
> +		.fops.read	= rtas_flash_read_msg,
> +		.fops.write	= rtas_flash_write,
> +		.fops.release	= rtas_flash_release,
> +		.fops.llseek	= default_llseek,
> +	},
> +	{
> +		.filename	= "powerpc/rtas/" FIRMWARE_UPDATE_NAME,
> +		.rtas_call_name	= "ibm,update-flash-64-and-reboot",
> +		.status		=&rtas_update_flash_data.status,
> +		.fops.read	= rtas_flash_read_num,
> +		.fops.write	= rtas_flash_write,
> +		.fops.release	= rtas_flash_release,
> +		.fops.llseek	= default_llseek,
> +	},
> +	{
> +		.filename	= "powerpc/rtas/" VALIDATE_FLASH_NAME,
> +		.rtas_call_name	= "ibm,validate-flash-image",
> +		.status		=&rtas_validate_flash_data.status,
> +		.fops.read	= manage_flash_read,
> +		.fops.write	= manage_flash_write,


We have to validate FW here :-) Above two lines needs to be replaced like below.

+		.fops.read	= validate_flash_read,
+		.fops.write	= validate_flash_write,
+		.fops.release	= validate_flash_release,


> +		.fops.llseek	= default_llseek,
> +	},
> +	{
> +		.filename	= "powerpc/rtas/" MANAGE_FLASH_NAME,
> +		.rtas_call_name	= "ibm,manage-flash-image",
> +		.status		=&rtas_manage_flash_data.status,
> +		.fops.read	= validate_flash_read,
> +		.fops.write	= validate_flash_write,
> +		.fops.release	= validate_flash_release,

Here we need to manage FW :-) Above three lines needs to be replaced like below.
+		.fops.read	= manage_flash_read,
+		.fops.write	= manage_flash_write,


I wrote below patch on top of yours to test rtas_flash.

--- rtas_flash.c.org	2013-04-25 04:35:42.000000000 -0400
+++ rtas_flash.c	2013-04-25 05:53:35.000000000 -0400
@@ -267,7 +267,7 @@ static ssize_t rtas_flash_read_msg(struc
  	mutex_unlock(&rtas_update_flash_mutex);

  	/* Read as text message */
-	len = get_flash_status_msg(uf->status, msg);
+	len = get_flash_status_msg(status, msg);
  	return simple_read_from_buffer(buf, count, ppos, msg, len);
  }

@@ -349,13 +349,13 @@ static ssize_t rtas_flash_write(struct f
  	fl->blocks[next_free].length = count;
  	fl->num_blocks++;
  out:
-	mutex_lock(&rtas_update_flash_mutex);
+	mutex_unlock(&rtas_update_flash_mutex);
  	return count;

  nomem:
  	rc = -ENOMEM;
  error:
-	mutex_lock(&rtas_update_flash_mutex);
+	mutex_unlock(&rtas_update_flash_mutex);
  	return rc;
  }

@@ -415,8 +415,10 @@ static ssize_t manage_flash_write(struct
  			op = RTAS_COMMIT_TMP_IMG;
  	}
  	
-	if (op == -1)   /* buf is empty, or contains invalid string */
-		return -EINVAL;
+	if (op == -1) {   /* buf is empty, or contains invalid string */
+		rc = -EINVAL;
+		goto error;
+	}

  	manage_flash(args_buf, op);
  out:
@@ -668,17 +670,17 @@ static const struct rtas_flash_file rtas
  		.filename	= "powerpc/rtas/" VALIDATE_FLASH_NAME,
  		.rtas_call_name	= "ibm,validate-flash-image",
  		.status		= &rtas_validate_flash_data.status,
-		.fops.read	= manage_flash_read,
-		.fops.write	= manage_flash_write,
+		.fops.read	= validate_flash_read,
+		.fops.write	= validate_flash_write,
+		.fops.release	= validate_flash_release,
  		.fops.llseek	= default_llseek,
  	},
  	{
  		.filename	= "powerpc/rtas/" MANAGE_FLASH_NAME,
  		.rtas_call_name	= "ibm,manage-flash-image",
  		.status		= &rtas_manage_flash_data.status,
-		.fops.read	= validate_flash_read,
-		.fops.write	= validate_flash_write,
-		.fops.release	= validate_flash_release,
+		.fops.read	= manage_flash_read,
+		.fops.write	= manage_flash_write,
  		.fops.llseek	= default_llseek,
  	}
  };



-Vasant


> +		.fops.llseek	= default_llseek,
> +	}
>   };
>
>   static int __init rtas_flash_init(void)
>   {
> -	int rc;
> +	int i;
>
>   	if (rtas_token("ibm,update-flash-64-and-reboot") ==
>   		       RTAS_UNKNOWN_SERVICE) {
> @@ -710,93 +692,65 @@ static int __init rtas_flash_init(void)
>   		return 1;
>   	}
>
> -	firmware_flash_pde = create_flash_pde("powerpc/rtas/"
> -					      FIRMWARE_FLASH_NAME,
> -					&rtas_flash_operations);
> -	if (firmware_flash_pde == NULL) {
> -		rc = -ENOMEM;
> -		goto cleanup;
> -	}
> +	rtas_validate_flash_data.buf = kzalloc(VALIDATE_BUF_SIZE, GFP_KERNEL);
> +	if (!rtas_validate_flash_data.buf)
> +		return -ENOMEM;
>
> -	rc = initialize_flash_pde_data("ibm,update-flash-64-and-reboot",
> -			 	       sizeof(struct rtas_update_flash_t),
> -				       firmware_flash_pde);
> -	if (rc != 0)
> -		goto cleanup;
> -
> -	firmware_update_pde = create_flash_pde("powerpc/rtas/"
> -					       FIRMWARE_UPDATE_NAME,
> -					&rtas_flash_operations);
> -	if (firmware_update_pde == NULL) {
> -		rc = -ENOMEM;
> -		goto cleanup;
> +	flash_block_cache = kmem_cache_create("rtas_flash_cache",
> +					      RTAS_BLK_SIZE, RTAS_BLK_SIZE, 0,
> +					      rtas_block_ctor);
> +	if (!flash_block_cache) {
> +		printk(KERN_ERR "%s: failed to create block cache\n",
> +				__func__);
> +		goto enomem_buf;
>   	}
>
> -	rc = initialize_flash_pde_data("ibm,update-flash-64-and-reboot",
> -			 	       sizeof(struct rtas_update_flash_t),
> -				       firmware_update_pde);
> -	if (rc != 0)
> -		goto cleanup;
> -
> -	validate_pde = create_flash_pde("powerpc/rtas/" VALIDATE_FLASH_NAME,
> -			      		&validate_flash_operations);
> -	if (validate_pde == NULL) {
> -		rc = -ENOMEM;
> -		goto cleanup;
> -	}
> +	for (i = 0; i<  ARRAY_SIZE(rtas_flash_files); i++) {
> +		const struct rtas_flash_file *f =&rtas_flash_files[i];
> +		int token;
>
> -	rc = initialize_flash_pde_data("ibm,validate-flash-image",
> -		                       sizeof(struct rtas_validate_flash_t),
> -				       validate_pde);
> -	if (rc != 0)
> -		goto cleanup;
> -
> -	manage_pde = create_flash_pde("powerpc/rtas/" MANAGE_FLASH_NAME,
> -				&manage_flash_operations);
> -	if (manage_pde == NULL) {
> -		rc = -ENOMEM;
> -		goto cleanup;
> -	}
> +		if (!proc_create(f->filename, S_IRUSR | S_IWUSR, NULL,&f->fops))
> +			goto enomem;
>
> -	rc = initialize_flash_pde_data("ibm,manage-flash-image",
> -			               sizeof(struct rtas_manage_flash_t),
> -				       manage_pde);
> -	if (rc != 0)
> -		goto cleanup;
> +		/*
> +		 * This code assumes that the status int is the first member of the
> +		 * struct
> +		 */
> +		token = rtas_token(f->rtas_call_name);
> +		if (token == RTAS_UNKNOWN_SERVICE)
> +			*f->status = FLASH_AUTH;
> +		else
> +			*f->status = FLASH_NO_OP;
> +	}
>
>   	rtas_flash_term_hook = rtas_flash_firmware;
> -
> -	flash_block_cache = kmem_cache_create("rtas_flash_cache",
> -				RTAS_BLK_SIZE, RTAS_BLK_SIZE, 0,
> -				rtas_block_ctor);
> -	if (!flash_block_cache) {
> -		printk(KERN_ERR "%s: failed to create block cache\n",
> -				__func__);
> -		rc = -ENOMEM;
> -		goto cleanup;
> -	}
>   	return 0;
>
> -cleanup:
> -	remove_flash_pde(firmware_flash_pde);
> -	remove_flash_pde(firmware_update_pde);
> -	remove_flash_pde(validate_pde);
> -	remove_flash_pde(manage_pde);
> +enomem:
> +	while (--i>= 0) {
> +		const struct rtas_flash_file *f =&rtas_flash_files[i];
> +		remove_proc_entry(f->filename, NULL);
> +	}
>
> -	return rc;
> +	kmem_cache_destroy(flash_block_cache);
> +enomem_buf:
> +	kfree(rtas_validate_flash_data.buf);
> +	return -ENOMEM;
>   }
>
>   static void __exit rtas_flash_cleanup(void)
>   {
> +	int i;
> +
>   	rtas_flash_term_hook = NULL;
>
> -	if (flash_block_cache)
> -		kmem_cache_destroy(flash_block_cache);
> +	for (i = 0; i<  ARRAY_SIZE(rtas_flash_files); i++) {
> +		const struct rtas_flash_file *f =&rtas_flash_files[i];
> +		remove_proc_entry(f->filename, NULL);
> +	}
>
> -	remove_flash_pde(firmware_flash_pde);
> -	remove_flash_pde(firmware_update_pde);
> -	remove_flash_pde(validate_pde);
> -	remove_flash_pde(manage_pde);
> +	kmem_cache_destroy(flash_block_cache);
> +	kfree(rtas_validate_flash_data.buf);
>   }
>
>   module_init(rtas_flash_init);
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>

^ permalink raw reply

* RE: [PATCH 1/1] kvm:book3e: Fix a build error
From: Caraman Mihai Claudiu-B02008 @ 2013-04-25 12:11 UTC (permalink / raw)
  To: Tiejun Chen, galak@kernel.crashing.org
  Cc: linuxppc-dev@lists.ozlabs.org, kvm@vger.kernel.org,
	kvm-ppc@vger.kernel.org
In-Reply-To: <1366890336-4282-1-git-send-email-tiejun.chen@windriver.com>

> -----Original Message-----
> From: kvm-ppc-owner@vger.kernel.org [mailto:kvm-ppc-
> owner@vger.kernel.org] On Behalf Of Tiejun Chen
> Sent: Thursday, April 25, 2013 2:46 PM
> To: galak@kernel.crashing.org
> Cc: linuxppc-dev@lists.ozlabs.org; kvm-ppc@vger.kernel.org;
> kvm@vger.kernel.org
> Subject: [PATCH 1/1] kvm:book3e: Fix a build error
>=20
> Commit cd66cc2e, "powerpc/85xx: Add AltiVec support for e6500", adds
> support for AltiVec on a Book-E class processor, but while compiling
> in the CONFIG_PPC_BOOK3E_64 and CONFIG_VIRTUALIZATION case, this
> introduce the following error:
>=20
> arch/powerpc/kernel/exceptions-64e.S:402: undefined reference to
> `kvmppc_handler_42_0x01B'
> arch/powerpc/kernel/built-in.o: In function `exc_altivec_assist_book3e':
> arch/powerpc/kernel/exceptions-64e.S:424: undefined reference to
> `kvmppc_handler_43_0x01B'
> make: *** [vmlinux] Error 1
>=20
> Looks we should add these altivec kvm handlers.
>=20
> Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
> ---
>  arch/powerpc/kvm/bookehv_interrupts.S |    5 +++++
>  1 file changed, 5 insertions(+)
>=20
> diff --git a/arch/powerpc/kvm/bookehv_interrupts.S
> b/arch/powerpc/kvm/bookehv_interrupts.S
> index e8ed7d6..fa9c78a 100644
> --- a/arch/powerpc/kvm/bookehv_interrupts.S
> +++ b/arch/powerpc/kvm/bookehv_interrupts.S
> @@ -319,6 +319,11 @@ kvm_handler BOOKE_INTERRUPT_DEBUG, EX_PARAMS(DBG), \
>  	SPRN_DSRR0, SPRN_DSRR1, 0
>  kvm_handler BOOKE_INTERRUPT_DEBUG, EX_PARAMS(CRIT), \
>  	SPRN_CSRR0, SPRN_CSRR1, 0
> +/* altivec */
> +kvm_handler BOOKE_INTERRUPT_ALTIVEC_UNAVAIL, EX_PARAMS(GEN), \
> +	SPRN_SRR0, SPRN_SRR1, 0
> +kvm_handler BOOKE_INTERRUPT_ALTIVEC_ASSIST, EX_PARAMS(GEN), \
> +	SPRN_SRR0, SPRN_SRR1, 0
>  #else
>  /*
>   * For input register values, see
> arch/powerpc/include/asm/kvm_booke_hv_asm.h
> --
=20
It seems that you are not using kvm-ppc-queue branch.

I already have a patch ready for this (and AltiVec support is work
in progress) but we need first to pull e6500 kernel patches from
Linux tree into agraf.git.
=20
-Mike



=20

^ permalink raw reply

* Re: [PATCH v2] PowerPC: kernel: compiling issue, make additional room in exception vector area
From: Chen Gang @ 2013-04-25 11:58 UTC (permalink / raw)
  To: Mike Qiu, Paul Mackerras, Michael Neuling
  Cc: sfr, matt, linux-kernel, Aneesh Kumar K.V, linuxppc-dev
In-Reply-To: <517918AB.4020508@asianux.com>

Hello Mike:

This patch can pass compiling with Mike's config file, under my
cross-compiling environments, but does not run under the real machine,
please try it.

Welcome other members to help check this patch whether valid.

Thanks.


On 2013年04月25日 19:51, Chen Gang wrote:
> 
> When CONFIG_KVM_BOOK3S_64_PR is enabled,
> MASKABLE_EXCEPTION_PSERIES(0x900 ...) will includes __KVMTEST, it will
> exceed 0x980 which STD_EXCEPTION_HV(0x980 ...) will use, it will cause
> compiling issue.
> 
> The related errors:
> arch/powerpc/kernel/exceptions-64s.S: Assembler messages:
> arch/powerpc/kernel/exceptions-64s.S:258: Error: attempt to move .org backwards
> make[1]: *** [arch/powerpc/kernel/head_64.o] Error 1
> 
> The position 0x900 and 0x980 are solid, so can not move the position
> to make room larger. The final solution is to jump to another area to
> execute the related code.
> 
> 
> Signed-off-by: Chen Gang <gang.chen@asianux.com>
> ---
>  arch/powerpc/kernel/exceptions-64s.S |   12 +++++++++++-
>  1 files changed, 11 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
> index e789ee7..8997de2 100644
> --- a/arch/powerpc/kernel/exceptions-64s.S
> +++ b/arch/powerpc/kernel/exceptions-64s.S
> @@ -254,7 +254,11 @@ hardware_interrupt_hv:
>  	STD_EXCEPTION_PSERIES(0x800, 0x800, fp_unavailable)
>  	KVM_HANDLER_PR(PACA_EXGEN, EXC_STD, 0x800)
>  
> -	MASKABLE_EXCEPTION_PSERIES(0x900, 0x900, decrementer)
> +	. = 0x900
> +	.globl decrementer_pSeries
> +decrementer_pSeries:
> +	b	decrementer_pSeries_0
> +
>  	STD_EXCEPTION_HV(0x980, 0x982, hdecrementer)
>  
>  	MASKABLE_EXCEPTION_PSERIES(0xa00, 0xa00, doorbell_super)
> @@ -536,6 +540,12 @@ ALT_FTR_SECTION_END_IFCLR(CPU_FTR_ARCH_206)
>  #endif
>  
>  	.align	7
> +	/* moved from 0x900 */
> +decrementer_pSeries_0:
> +	_MASKABLE_EXCEPTION_PSERIES(0x900, decrementer,
> +				    EXC_STD, SOFTEN_TEST_PR)
> +
> +	.align	7
>  	/* moved from 0xe00 */
>  	STD_EXCEPTION_HV_OOL(0xe02, h_data_storage)
>  	KVM_HANDLER_SKIP(PACA_EXGEN, EXC_HV, 0xe02)
> 


-- 
Chen Gang

Asianux Corporation

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox