* RE: [PATCH v4 4/4] powerpc/85xx: add sysfs for pw20 state and altivec idle
From: Bhushan Bharat-R65777 @ 2013-09-25 8:33 UTC (permalink / raw)
To: Wang Dongsheng-B40534, Wood Scott-B07421; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <ABB05CD9C9F68C46A5CEDC7F154392590108FF41@039-SN2MPN1-021.039d.mgd.msft.net>
> -----Original Message-----
> From: Wang Dongsheng-B40534
> Sent: Wednesday, September 25, 2013 1:40 PM
> To: Bhushan Bharat-R65777; Wood Scott-B07421
> Cc: linuxppc-dev@lists.ozlabs.org
> Subject: RE: [PATCH v4 4/4] powerpc/85xx: add sysfs for pw20 state and al=
tivec
> idle
>=20
>=20
>=20
> > -----Original Message-----
> > From: Bhushan Bharat-R65777
> > Sent: Wednesday, September 25, 2013 2:23 PM
> > To: Wang Dongsheng-B40534; Wood Scott-B07421
> > Cc: linuxppc-dev@lists.ozlabs.org; Wang Dongsheng-B40534
> > Subject: RE: [PATCH v4 4/4] powerpc/85xx: add sysfs for pw20 state and
> > altivec idle
> >
> >
> >
> > > -----Original Message-----
> > > From: Linuxppc-dev [mailto:linuxppc-dev-
> > > bounces+bharat.bhushan=3Dfreescale.com@lists.ozlabs.org] On Behalf Of
> > > bounces+Dongsheng
> > > Wang
> > > Sent: Tuesday, September 24, 2013 2:59 PM
> > > To: Wood Scott-B07421
> > > Cc: linuxppc-dev@lists.ozlabs.org; Wang Dongsheng-B40534
> > > Subject: [PATCH v4 4/4] powerpc/85xx: add sysfs for pw20 state and
> > > altivec idle
> > >
> > > From: Wang Dongsheng <dongsheng.wang@freescale.com>
> > >
> > > Add a sys interface to enable/diable pw20 state or altivec idle, and
> > > control the wait entry time.
> > >
> > > Enable/Disable interface:
> > > 0, disable. 1, enable.
> > > /sys/devices/system/cpu/cpuX/pw20_state
> > > /sys/devices/system/cpu/cpuX/altivec_idle
> > >
> > > Set wait time interface:(Nanosecond)
> > > /sys/devices/system/cpu/cpuX/pw20_wait_time
> > > /sys/devices/system/cpu/cpuX/altivec_idle_wait_time
> > > Example: Base on TBfreq is 41MHZ.
> > > 1~47(ns): TB[63]
> > > 48~95(ns): TB[62]
> > > 96~191(ns): TB[61]
> > > 192~383(ns): TB[62]
> > > 384~767(ns): TB[60]
> > > ...
> > >
> > > Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
> > > ---
> > > *v4:
> > > Move code from 85xx/common.c to kernel/sysfs.c.
> > >
> > > Remove has_pw20_altivec_idle function.
> > >
> > > Change wait "entry_bit" to wait time.
> > >
> > > arch/powerpc/kernel/sysfs.c | 291
> > > ++++++++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 291 insertions(+)
> > >
> > > diff --git a/arch/powerpc/kernel/sysfs.c
> > > b/arch/powerpc/kernel/sysfs.c index 27a90b9..23fece6 100644
> > > --- a/arch/powerpc/kernel/sysfs.c
> > > +++ b/arch/powerpc/kernel/sysfs.c
> > > @@ -85,6 +85,279 @@ __setup("smt-snooze-delay=3D",
> > > setup_smt_snooze_delay);
> > >
> > > #endif /* CONFIG_PPC64 */
> > >
> > > +#ifdef CONFIG_FSL_SOC
> > > +#define MAX_BIT 63
> > > +
> > > +static u64 pw20_wt;
> > > +static u64 altivec_idle_wt;
> > > +
> > > +static unsigned int get_idle_ticks_bit(u64 ns) {
> > > + u64 cycle;
> > > +
> > > + cycle =3D div_u64(ns, 1000 / tb_ticks_per_usec);
> >
> > When tb_ticks_per_usec > 1000 (timebase frequency > 1GHz) then this
> > will always be ns, which is not correct, no?
> >
> "1000 / tb_ticks_per_usec" means nsec_ticks_per_tb
>=20
> If timebase frequency > 1GHz, this should be "tb_ticks_per_usec / 1000" a=
nd to
> get tb_ticks_per_nsec.
> This should be changed to "cycle =3D ns * tb_ticks_per_nsec;"
Yes, we need to change this to two line.
>=20
> But at present we do not have such a platform that timebase frequency mor=
e than
> 1GHz. And I think it is not need to support such a situation. Because we =
have no
> environment to test it.
>=20
> If later there will be more than 1GHZ platform at that time to add this s=
upport.
Would like to leave it to Scott, but personally I think that if there is so=
mething simple to fix then it must be fixed rather than waiting for some er=
ror to happen and then fixing.
-Bharat
>=20
> Thanks.
>=20
> -dongsheng
^ permalink raw reply
* Re: [PATCH v2 4/4] hotplug, powerpc, x86: Remove cpu_hotplug_driver_lock()
From: Rafael J. Wysocki @ 2013-09-25 8:27 UTC (permalink / raw)
To: Toshi Kani
Cc: fenghua.yu, bp, gregkh, x86, linux-kernel, linux-acpi,
isimatu.yasuaki, mingo, srivatsa.bhat, nfont, tglx, hpa,
linuxppc-dev
In-Reply-To: <1377822129-4143-5-git-send-email-toshi.kani@hp.com>
Hi,
On Thursday, August 29, 2013 06:22:09 PM Toshi Kani wrote:
> cpu_hotplug_driver_lock() serializes CPU online/offline operations
> when ARCH_CPU_PROBE_RELEASE is set. This lock interface is no longer
> necessary with the following reason:
>
> - lock_device_hotplug() now protects CPU online/offline operations,
> including the probe & release interfaces enabled by
> ARCH_CPU_PROBE_RELEASE. The use of cpu_hotplug_driver_lock() is
> redundant.
> - cpu_hotplug_driver_lock() is only valid when ARCH_CPU_PROBE_RELEASE
> is defined, which is misleading and is only enabled on powerpc.
>
> This patch removes the cpu_hotplug_driver_lock() interface. As
> a result, ARCH_CPU_PROBE_RELEASE only enables / disables the cpu
> probe & release interface as intended. There is no functional change
> in this patch.
>
> Signed-off-by: Toshi Kani <toshi.kani@hp.com>
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Reviewed-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
Can you please rebase this patch on top of 3.12-rc2? It doesn't apply for
me any more.
Thanks,
Rafael
> ---
> Performed build test only on powerpc.
> ---
> arch/powerpc/kernel/smp.c | 12 ----------
> arch/powerpc/platforms/pseries/dlpar.c | 40 ++++++++++++--------------------
> arch/x86/kernel/topology.c | 2 --
> drivers/base/cpu.c | 10 +-------
> include/linux/cpu.h | 13 ----------
> 5 files changed, 16 insertions(+), 61 deletions(-)
>
> diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
> index 38b0ba6..1667269 100644
> --- a/arch/powerpc/kernel/smp.c
> +++ b/arch/powerpc/kernel/smp.c
> @@ -763,18 +763,6 @@ void __cpu_die(unsigned int cpu)
> smp_ops->cpu_die(cpu);
> }
>
> -static DEFINE_MUTEX(powerpc_cpu_hotplug_driver_mutex);
> -
> -void cpu_hotplug_driver_lock()
> -{
> - mutex_lock(&powerpc_cpu_hotplug_driver_mutex);
> -}
> -
> -void cpu_hotplug_driver_unlock()
> -{
> - mutex_unlock(&powerpc_cpu_hotplug_driver_mutex);
> -}
> -
> void cpu_die(void)
> {
> if (ppc_md.cpu_die)
> diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
> index a1a7b9a..e39325d 100644
> --- a/arch/powerpc/platforms/pseries/dlpar.c
> +++ b/arch/powerpc/platforms/pseries/dlpar.c
> @@ -387,18 +387,13 @@ static ssize_t dlpar_cpu_probe(const char *buf, size_t count)
> char *cpu_name;
> int rc;
>
> - cpu_hotplug_driver_lock();
> rc = strict_strtoul(buf, 0, &drc_index);
> - if (rc) {
> - rc = -EINVAL;
> - goto out;
> - }
> + if (rc)
> + return -EINVAL;
>
> dn = dlpar_configure_connector(drc_index);
> - if (!dn) {
> - rc = -EINVAL;
> - goto out;
> - }
> + if (!dn)
> + return -EINVAL;
>
> /* configure-connector reports cpus as living in the base
> * directory of the device tree. CPUs actually live in the
> @@ -407,8 +402,7 @@ static ssize_t dlpar_cpu_probe(const char *buf, size_t count)
> cpu_name = kasprintf(GFP_KERNEL, "/cpus%s", dn->full_name);
> if (!cpu_name) {
> dlpar_free_cc_nodes(dn);
> - rc = -ENOMEM;
> - goto out;
> + return -ENOMEM;
> }
>
> kfree(dn->full_name);
> @@ -417,22 +411,21 @@ static ssize_t dlpar_cpu_probe(const char *buf, size_t count)
> rc = dlpar_acquire_drc(drc_index);
> if (rc) {
> dlpar_free_cc_nodes(dn);
> - rc = -EINVAL;
> - goto out;
> + return -EINVAL;
> }
>
> rc = dlpar_attach_node(dn);
> if (rc) {
> dlpar_release_drc(drc_index);
> dlpar_free_cc_nodes(dn);
> - goto out;
> + return rc;
> }
>
> rc = dlpar_online_cpu(dn);
> -out:
> - cpu_hotplug_driver_unlock();
> + if (rc)
> + return rc;
>
> - return rc ? rc : count;
> + return count;
> }
>
> static int dlpar_offline_cpu(struct device_node *dn)
> @@ -505,30 +498,27 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t count)
> return -EINVAL;
> }
>
> - cpu_hotplug_driver_lock();
> rc = dlpar_offline_cpu(dn);
> if (rc) {
> of_node_put(dn);
> - rc = -EINVAL;
> - goto out;
> + return -EINVAL;
> }
>
> rc = dlpar_release_drc(*drc_index);
> if (rc) {
> of_node_put(dn);
> - goto out;
> + return rc;
> }
>
> rc = dlpar_detach_node(dn);
> if (rc) {
> dlpar_acquire_drc(*drc_index);
> - goto out;
> + return rc;
> }
>
> of_node_put(dn);
> -out:
> - cpu_hotplug_driver_unlock();
> - return rc ? rc : count;
> +
> + return count;
> }
>
> static int __init pseries_dlpar_init(void)
> diff --git a/arch/x86/kernel/topology.c b/arch/x86/kernel/topology.c
> index a3f35eb..649b010 100644
> --- a/arch/x86/kernel/topology.c
> +++ b/arch/x86/kernel/topology.c
> @@ -66,7 +66,6 @@ int __ref _debug_hotplug_cpu(int cpu, int action)
> return -EINVAL;
>
> lock_device_hotplug();
> - cpu_hotplug_driver_lock();
>
> switch (action) {
> case 0:
> @@ -91,7 +90,6 @@ int __ref _debug_hotplug_cpu(int cpu, int action)
> ret = -EINVAL;
> }
>
> - cpu_hotplug_driver_unlock();
> unlock_device_hotplug();
>
> return ret;
> diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
> index dc78e45..9745f03 100644
> --- a/drivers/base/cpu.c
> +++ b/drivers/base/cpu.c
> @@ -46,8 +46,6 @@ static int __ref cpu_subsys_online(struct device *dev)
> int from_nid, to_nid;
> int ret;
>
> - cpu_hotplug_driver_lock();
> -
> from_nid = cpu_to_node(cpuid);
> ret = cpu_up(cpuid);
> /*
> @@ -58,18 +56,12 @@ static int __ref cpu_subsys_online(struct device *dev)
> if (from_nid != to_nid)
> change_cpu_under_node(cpu, from_nid, to_nid);
>
> - cpu_hotplug_driver_unlock();
> return ret;
> }
>
> static int cpu_subsys_offline(struct device *dev)
> {
> - int ret;
> -
> - cpu_hotplug_driver_lock();
> - ret = cpu_down(dev->id);
> - cpu_hotplug_driver_unlock();
> - return ret;
> + return cpu_down(dev->id);
> }
>
> void unregister_cpu(struct cpu *cpu)
> diff --git a/include/linux/cpu.h b/include/linux/cpu.h
> index 801ff9e..3434ef7 100644
> --- a/include/linux/cpu.h
> +++ b/include/linux/cpu.h
> @@ -185,19 +185,6 @@ extern void cpu_hotplug_enable(void);
> void clear_tasks_mm_cpumask(int cpu);
> int cpu_down(unsigned int cpu);
>
> -#ifdef CONFIG_ARCH_CPU_PROBE_RELEASE
> -extern void cpu_hotplug_driver_lock(void);
> -extern void cpu_hotplug_driver_unlock(void);
> -#else
> -static inline void cpu_hotplug_driver_lock(void)
> -{
> -}
> -
> -static inline void cpu_hotplug_driver_unlock(void)
> -{
> -}
> -#endif
> -
> #else /* CONFIG_HOTPLUG_CPU */
>
> static inline void cpu_hotplug_begin(void) {}
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply
* RE: [PATCH v4 4/4] powerpc/85xx: add sysfs for pw20 state and altivec idle
From: Wang Dongsheng-B40534 @ 2013-09-25 8:10 UTC (permalink / raw)
To: Bhushan Bharat-R65777, Wood Scott-B07421; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <6A3DF150A5B70D4F9B66A25E3F7C888D0717F8F9@039-SN2MPN1-011.039d.mgd.msft.net>
> -----Original Message-----
> From: Bhushan Bharat-R65777
> Sent: Wednesday, September 25, 2013 2:23 PM
> To: Wang Dongsheng-B40534; Wood Scott-B07421
> Cc: linuxppc-dev@lists.ozlabs.org; Wang Dongsheng-B40534
> Subject: RE: [PATCH v4 4/4] powerpc/85xx: add sysfs for pw20 state and
> altivec idle
>=20
>=20
>=20
> > -----Original Message-----
> > From: Linuxppc-dev [mailto:linuxppc-dev-
> > bounces+bharat.bhushan=3Dfreescale.com@lists.ozlabs.org] On Behalf Of
> > bounces+Dongsheng
> > Wang
> > Sent: Tuesday, September 24, 2013 2:59 PM
> > To: Wood Scott-B07421
> > Cc: linuxppc-dev@lists.ozlabs.org; Wang Dongsheng-B40534
> > Subject: [PATCH v4 4/4] powerpc/85xx: add sysfs for pw20 state and
> > altivec idle
> >
> > From: Wang Dongsheng <dongsheng.wang@freescale.com>
> >
> > Add a sys interface to enable/diable pw20 state or altivec idle, and
> > control the wait entry time.
> >
> > Enable/Disable interface:
> > 0, disable. 1, enable.
> > /sys/devices/system/cpu/cpuX/pw20_state
> > /sys/devices/system/cpu/cpuX/altivec_idle
> >
> > Set wait time interface:(Nanosecond)
> > /sys/devices/system/cpu/cpuX/pw20_wait_time
> > /sys/devices/system/cpu/cpuX/altivec_idle_wait_time
> > Example: Base on TBfreq is 41MHZ.
> > 1~47(ns): TB[63]
> > 48~95(ns): TB[62]
> > 96~191(ns): TB[61]
> > 192~383(ns): TB[62]
> > 384~767(ns): TB[60]
> > ...
> >
> > Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
> > ---
> > *v4:
> > Move code from 85xx/common.c to kernel/sysfs.c.
> >
> > Remove has_pw20_altivec_idle function.
> >
> > Change wait "entry_bit" to wait time.
> >
> > arch/powerpc/kernel/sysfs.c | 291
> > ++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 291 insertions(+)
> >
> > diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
> > index 27a90b9..23fece6 100644
> > --- a/arch/powerpc/kernel/sysfs.c
> > +++ b/arch/powerpc/kernel/sysfs.c
> > @@ -85,6 +85,279 @@ __setup("smt-snooze-delay=3D",
> > setup_smt_snooze_delay);
> >
> > #endif /* CONFIG_PPC64 */
> >
> > +#ifdef CONFIG_FSL_SOC
> > +#define MAX_BIT 63
> > +
> > +static u64 pw20_wt;
> > +static u64 altivec_idle_wt;
> > +
> > +static unsigned int get_idle_ticks_bit(u64 ns) {
> > + u64 cycle;
> > +
> > + cycle =3D div_u64(ns, 1000 / tb_ticks_per_usec);
>=20
> When tb_ticks_per_usec > 1000 (timebase frequency > 1GHz) then this will
> always be ns, which is not correct, no?
>=20
"1000 / tb_ticks_per_usec" means nsec_ticks_per_tb
If timebase frequency > 1GHz, this should be "tb_ticks_per_usec / 1000" and=
to get tb_ticks_per_nsec.
This should be changed to "cycle =3D ns * tb_ticks_per_nsec;"
But at present we do not have such a platform that timebase frequency more =
than 1GHz. And I think it is not need to support such a situation. Because =
we have no environment to test it.
If later there will be more than 1GHZ platform at that time to add this sup=
port.
Thanks.
-dongsheng
^ permalink raw reply
* Re: [PATCH v10 2/3] DMA: Freescale: Add new 8-channel DMA engine device tree nodes
From: Hongbo Zhang @ 2013-09-25 7:35 UTC (permalink / raw)
To: Stephen Warren
Cc: mark.rutland, devicetree, ian.campbell, pawel.moll, vinod.koul,
linux-kernel, rob.herring, djbw, linuxppc-dev
In-Reply-To: <5241CC60.5070204@wwwdotorg.org>
On 09/25/2013 01:31 AM, Stephen Warren wrote:
> On 09/24/2013 04:30 AM, Hongbo Zhang wrote:
>> On 09/24/2013 01:04 AM, Stephen Warren wrote:
>>> On 09/18/2013 04:15 AM, hongbo.zhang@freescale.com wrote:
>>>> From: Hongbo Zhang <hongbo.zhang@freescale.com>
>>>>
>>>> Freescale QorIQ T4 and B4 introduce new 8-channel DMA engines, this
>>>> patch adds
>>>> the device tree nodes for them.
>>>> diff --git a/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
>>>> b/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
>>>> +Required properties:
>>>> +
>>>> +- compatible : must include "fsl,elo3-dma"
>>>> +- reg : DMA General Status Registers, i.e. DGSR0 which
>>>> contains
>>>> + status for channel 1~4, and DGSR1 for channel 5~8
>>> Is that a single entry, which is large enough to cover both registers,
>>> or a pair of entries, one per register? Reading the text, I might assume
>>> the former, but looking at the examples, it's the latter.
>> My impression is that I cannot tell it is one larger entry or two
>> entries by reading the description text, but the example gives the answer.
>> Is it so important to specify it is only one entry or entries list?
>> I prefer language as concise as possible, especially for the common
>> properties such as reg and interrupt (eg the reg is implicitly offset
>> and length of registers, can be continuous or not), it is difficult or
>> unnecessary or impossible to describe much details, the example can also
>> work as a complementary description, otherwise no need to put an example
>> in the binding document.
> The description of the properties should fully describe them. The
> example is just an example, not a specification of the properties.
>
It is OK for me to update the description like this:
reg: containing two entries for DMA General Status Registers, i.e.
DGSR0 which contains + status for channel 1~4, and DGSR1 for channel 5~8
and let me wait one or more days to see if other reviewers/maintainers
have further comments before I send our another iteration.
By the way, I know maybe it is difficult, but why not introduce a
document of maintaining rules for the dt binding docs? we have dedicated
maintainers for this part now. Description language from one submitter
cannot satisfy every reviewer/maintainer, for a reg property, is it
necessary to say "offset and length", to say "how many entries", to say
"register functions and even names"? If there is specific rules (even
with good examples), it will be convenient for both submitter and
reviewers. Without rules/guidelines, new submitter would like to follow
old bad samples.
^ permalink raw reply
* Re: mm: insure topdown mmap chooses addresses above security minimum
From: Ingo Molnar @ 2013-09-25 7:30 UTC (permalink / raw)
To: Timothy Pepper
Cc: linux-mips, Russell King, Andrew Morton, Paul Mundt, linux-sh,
x86, Ralf Baechle, linux-mm, Linus Torvalds, Ingo Molnar,
Paul Mackerras, H. Peter Anvin, sparclinux, Thomas Gleixner,
linuxppc-dev, David S. Miller, linux-arm-kernel
In-Reply-To: <1380057811-5352-1-git-send-email-timothy.c.pepper@linux.intel.com>
* Timothy Pepper <timothy.c.pepper@linux.intel.com> wrote:
> A security check is performed on mmap addresses in
> security/security.c:security_mmap_addr(). It uses mmap_min_addr to insure
> mmaps don't get addresses lower than a user configurable guard value
> (/proc/sys/vm/mmap_min_addr). The arch specific mmap topdown searches
> look for a map candidate address all the way down to a low_limit that is
> currently hard coded as PAGE_SIZE. Depending on compile time options
> and userspace setting the procfs tunable, the security check's view of
> the minimum allowable address may be something greater than PAGE_SIZE.
> This leaves a gap where get_unmapped_area()'s call to get_area() might
> return an address above PAGE_SIZE, but below mmap_min_addr, and thus
> get_unmapped_area() fails.
>
> This was seen on x86_64 in the case of a topdown address space and a large
> stack rlimit, with mmap_min_addr having been set to 32k by the distro.
> This left a 28k gap where the get area search intends to place a small
> mmap, but then get_unmapped_area() stumbles at the security check.
>
> What should have happened is the address search wraps back to a higher
> address, the search continues and perhaps succeeds. Indeed an mmap of
> a larger size gets a topdown search that does wrap around back up into
> the rlimit stack reserve and succeeds assuming suitable free space.
> But a small mmap fits in the low gap and always fails. It becomes
> possible to make large mmaps but not small ones.
>
> When an explicit address hint is given, mm/mmap.c's round_hint_to_min()
> will round up to mmap_min_addr.
>
> A topdown search's low_limit should similarly consider mmap_min_addr
> instead of just PAGE_SIZE.
>
> Signed-off-by: Tim Pepper <timothy.c.pepper@linux.intel.com>
> Cc: linux-mm@kvack.org
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: x86@kernel.org
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: linux-mips@linux-mips.org
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: Paul Mundt <lethal@linux-sh.org>
> Cc: linux-sh@vger.kernel.org
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: sparclinux@vger.kernel.org
> --
> arch/arm/mm/mmap.c | 3 ++-
> arch/mips/mm/mmap.c | 3 ++-
> arch/powerpc/mm/slice.c | 3 ++-
> arch/sh/mm/mmap.c | 3 ++-
> arch/sparc/kernel/sys_sparc_64.c | 3 ++-
> arch/x86/kernel/sys_x86_64.c | 3 ++-
> 6 files changed, 12 insertions(+), 6 deletions(-)
>
> + info.low_limit = max(PAGE_SIZE, PAGE_ALIGN(mmap_min_addr));
> info.high_limit = mm->mmap_base;
> info.align_mask = do_align ? (PAGE_MASK & (SHMLBA - 1)) : 0;
> info.align_offset = pgoff << PAGE_SHIFT;
> info.flags = VM_UNMAPPED_AREA_TOPDOWN;
> - info.low_limit = PAGE_SIZE;
> + info.low_limit = max(PAGE_SIZE, PAGE_ALIGN(mmap_min_addr));
> info.high_limit = mm->mmap_base;
> addr = vm_unmapped_area(&info);
> - info.low_limit = addr;
> + info.low_limit = max(addr, PAGE_ALIGN(mmap_min_addr));
> info.flags = VM_UNMAPPED_AREA_TOPDOWN;
> info.length = len;
> - info.low_limit = PAGE_SIZE;
> + info.low_limit = max(PAGE_SIZE, PAGE_ALIGN(mmap_min_addr));
> info.high_limit = mm->mmap_base;
> info.align_mask = do_colour_align ? (PAGE_MASK & shm_align_mask) : 0;
> info.align_offset = pgoff << PAGE_SHIFT;
> info.flags = VM_UNMAPPED_AREA_TOPDOWN;
> info.length = len;
> - info.low_limit = PAGE_SIZE;
> + info.low_limit = max(PAGE_SIZE, PAGE_ALIGN(mmap_min_addr));
> info.high_limit = mm->mmap_base;
> info.align_mask = do_color_align ? (PAGE_MASK & (SHMLBA - 1)) : 0;
> info.align_offset = pgoff << PAGE_SHIFT;
> info.flags = VM_UNMAPPED_AREA_TOPDOWN;
> info.length = len;
> - info.low_limit = PAGE_SIZE;
> + info.low_limit = max(PAGE_SIZE, PAGE_ALIGN(mmap_min_addr));
> info.high_limit = mm->mmap_base;
> info.align_mask = filp ? get_align_mask() : 0;
> info.align_offset = pgoff << PAGE_SHIFT;
There appears to be a lot of repetition in these methods - instead of
changing 6 places it would be more future-proof to first factor out the
common bits and then to apply the fix to the shared implementation.
Thanks,
Ingo
^ permalink raw reply
* [PATCH v2] powerpc/83xx: gianfar_ptp: select 1588 clock source through dts file
From: Aida Mynzhasova @ 2013-09-25 7:24 UTC (permalink / raw)
To: linuxppc-dev; +Cc: devicetree, richardcochran, netdev
Currently IEEE 1588 timer reference clock source is determined through
hard-coded value in gianfar_ptp driver. This patch allows to select ptp
clock source by means of device tree file node.
For instance:
fsl,cksel = <0>;
for using external (TSEC_TMR_CLK input) high precision timer
reference clock.
Other acceptable values:
<1> : eTSEC system clock
<2> : eTSEC1 transmit clock
<3> : RTC clock input
When this attribute isn't used, eTSEC system clock will serve as
IEEE 1588 timer reference clock.
Signed-off-by: Aida Mynzhasova <aida.mynzhasova@skitlab.ru>
---
Documentation/devicetree/bindings/net/fsl-tsec-phy.txt | 16 +++++++++++++++-
drivers/net/ethernet/freescale/gianfar_ptp.c | 4 +++-
2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt b/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
index 2c6be03..eb06059 100644
--- a/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
+++ b/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
@@ -86,6 +86,7 @@ General Properties:
Clock Properties:
+ - fsl,cksel Timer reference clock source.
- fsl,tclk-period Timer reference clock period in nanoseconds.
- fsl,tmr-prsc Prescaler, divides the output clock.
- fsl,tmr-add Frequency compensation value.
@@ -97,7 +98,7 @@ Clock Properties:
clock. You must choose these carefully for the clock to work right.
Here is how to figure good values:
- TimerOsc = system clock MHz
+ TimerOsc = selected reference clock MHz
tclk_period = desired clock period nanoseconds
NominalFreq = 1000 / tclk_period MHz
FreqDivRatio = TimerOsc / NominalFreq (must be greater that 1.0)
@@ -114,6 +115,18 @@ Clock Properties:
Pulse Per Second (PPS) signal, since this will be offered to the PPS
subsystem to synchronize the Linux clock.
+ "fsl,cksel" property allows to select different reference clock
+ sources:
+
+ <0> - external high precision timer reference clock (TSEC_TMR_CLK
+ input is used for this purpose);
+ <1> - eTSEC system clock;
+ <2> - eTSEC1 transmit clock;
+ <3> - RTC clock input.
+
+ When this attribute is not used, eTSEC system clock will serve as
+ IEEE 1588 timer reference clock.
+
Example:
ptp_clock@24E00 {
@@ -121,6 +134,7 @@ Example:
reg = <0x24E00 0xB0>;
interrupts = <12 0x8 13 0x8>;
interrupt-parent = < &ipic >;
+ fsl,cksel = <1>;
fsl,tclk-period = <10>;
fsl,tmr-prsc = <100>;
fsl,tmr-add = <0x999999A4>;
diff --git a/drivers/net/ethernet/freescale/gianfar_ptp.c b/drivers/net/ethernet/freescale/gianfar_ptp.c
index 098f133..e006a09 100644
--- a/drivers/net/ethernet/freescale/gianfar_ptp.c
+++ b/drivers/net/ethernet/freescale/gianfar_ptp.c
@@ -452,7 +452,9 @@ static int gianfar_ptp_probe(struct platform_device *dev)
err = -ENODEV;
etsects->caps = ptp_gianfar_caps;
- etsects->cksel = DEFAULT_CKSEL;
+
+ if (get_of_u32(node, "fsl,cksel", &etsects->cksel))
+ etsects->cksel = DEFAULT_CKSEL;
if (get_of_u32(node, "fsl,tclk-period", &etsects->tclk_period) ||
get_of_u32(node, "fsl,tmr-prsc", &etsects->tmr_prsc) ||
--
1.8.1.2
^ permalink raw reply related
* RE: [PATCH v4 4/4] powerpc/85xx: add sysfs for pw20 state and altivec idle
From: Bhushan Bharat-R65777 @ 2013-09-25 6:22 UTC (permalink / raw)
To: Wang Dongsheng-B40534, Wood Scott-B07421
Cc: Wang Dongsheng-B40534, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1380014925-23300-4-git-send-email-dongsheng.wang@freescale.com>
> -----Original Message-----
> From: Linuxppc-dev [mailto:linuxppc-dev-
> bounces+bharat.bhushan=3Dfreescale.com@lists.ozlabs.org] On Behalf Of Don=
gsheng
> Wang
> Sent: Tuesday, September 24, 2013 2:59 PM
> To: Wood Scott-B07421
> Cc: linuxppc-dev@lists.ozlabs.org; Wang Dongsheng-B40534
> Subject: [PATCH v4 4/4] powerpc/85xx: add sysfs for pw20 state and altive=
c idle
>=20
> From: Wang Dongsheng <dongsheng.wang@freescale.com>
>=20
> Add a sys interface to enable/diable pw20 state or altivec idle, and
> control the wait entry time.
>=20
> Enable/Disable interface:
> 0, disable. 1, enable.
> /sys/devices/system/cpu/cpuX/pw20_state
> /sys/devices/system/cpu/cpuX/altivec_idle
>=20
> Set wait time interface:(Nanosecond)
> /sys/devices/system/cpu/cpuX/pw20_wait_time
> /sys/devices/system/cpu/cpuX/altivec_idle_wait_time
> Example: Base on TBfreq is 41MHZ.
> 1~47(ns): TB[63]
> 48~95(ns): TB[62]
> 96~191(ns): TB[61]
> 192~383(ns): TB[62]
> 384~767(ns): TB[60]
> ...
>=20
> Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
> ---
> *v4:
> Move code from 85xx/common.c to kernel/sysfs.c.
>=20
> Remove has_pw20_altivec_idle function.
>=20
> Change wait "entry_bit" to wait time.
>=20
> arch/powerpc/kernel/sysfs.c | 291 ++++++++++++++++++++++++++++++++++++++=
++++++
> 1 file changed, 291 insertions(+)
>=20
> diff --git a/arch/powerpc/kernel/sysfs.c b/arch/powerpc/kernel/sysfs.c
> index 27a90b9..23fece6 100644
> --- a/arch/powerpc/kernel/sysfs.c
> +++ b/arch/powerpc/kernel/sysfs.c
> @@ -85,6 +85,279 @@ __setup("smt-snooze-delay=3D", setup_smt_snooze_delay=
);
>=20
> #endif /* CONFIG_PPC64 */
>=20
> +#ifdef CONFIG_FSL_SOC
> +#define MAX_BIT 63
> +
> +static u64 pw20_wt;
> +static u64 altivec_idle_wt;
> +
> +static unsigned int get_idle_ticks_bit(u64 ns)
> +{
> + u64 cycle;
> +
> + cycle =3D div_u64(ns, 1000 / tb_ticks_per_usec);
When tb_ticks_per_usec > 1000 (timebase frequency > 1GHz) then this will a=
lways be ns, which is not correct, no?=20
> + if (!cycle)
> + return 0;
> +
> + return ilog2(cycle);
> +}
> +
> +static void do_show_pwrmgtcr0(void *val)
> +{
> + u32 *value =3D val;
> +
> + *value =3D mfspr(SPRN_PWRMGTCR0);
> +}
> +
> +static ssize_t show_pw20_state(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + u32 value;
> + unsigned int cpu =3D dev->id;
> +
> + smp_call_function_single(cpu, do_show_pwrmgtcr0, &value, 1);
> +
> + value &=3D PWRMGTCR0_PW20_WAIT;
> +
> + return sprintf(buf, "%u\n", value ? 1 : 0);
> +}
> +
> +static void do_store_pw20_state(void *val)
> +{
> + u32 *value =3D val;
> + u32 pw20_state;
> +
> + pw20_state =3D mfspr(SPRN_PWRMGTCR0);
> +
> + if (*value)
> + pw20_state |=3D PWRMGTCR0_PW20_WAIT;
> + else
> + pw20_state &=3D ~PWRMGTCR0_PW20_WAIT;
> +
> + mtspr(SPRN_PWRMGTCR0, pw20_state);
> +}
> +
> +static ssize_t store_pw20_state(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + u32 value;
> + unsigned int cpu =3D dev->id;
> +
> + if (kstrtou32(buf, 0, &value))
> + return -EINVAL;
> +
> + if (value > 1)
> + return -EINVAL;
> +
> + smp_call_function_single(cpu, do_store_pw20_state, &value, 1);
> +
> + return count;
> +}
> +
> +static ssize_t show_pw20_wait_time(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + u32 value;
> + u64 tb_cycle;
> + u64 time;
> +
> + unsigned int cpu =3D dev->id;
> +
> + if (!pw20_wt) {
> + smp_call_function_single(cpu, do_show_pwrmgtcr0, &value, 1);
> + value =3D (value & PWRMGTCR0_PW20_ENT) >>
> + PWRMGTCR0_PW20_ENT_SHIFT;
> +
> + tb_cycle =3D (1 << (MAX_BIT - value)) * 2;
> + time =3D tb_cycle * (1000 / tb_ticks_per_usec) - 1;
Similar to above comment.
-Bharat
> + } else {
> + time =3D pw20_wt;
> + }
> +
> + return sprintf(buf, "%llu\n", time);
> +}
> +
> +static void set_pw20_wait_entry_bit(void *val)
> +{
> + u32 *value =3D val;
> + u32 pw20_idle;
> +
> + pw20_idle =3D mfspr(SPRN_PWRMGTCR0);
> +
> + /* Set Automatic PW20 Core Idle Count */
> + /* clear count */
> + pw20_idle &=3D ~PWRMGTCR0_PW20_ENT;
> +
> + /* set count */
> + pw20_idle |=3D ((MAX_BIT - *value) << PWRMGTCR0_PW20_ENT_SHIFT);
> +
> + mtspr(SPRN_PWRMGTCR0, pw20_idle);
> +}
> +
> +static ssize_t store_pw20_wait_time(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + u32 entry_bit;
> + u64 value;
> +
> + unsigned int cpu =3D dev->id;
> +
> + if (kstrtou64(buf, 0, &value))
> + return -EINVAL;
> +
> + if (!value)
> + return -EINVAL;
> +
> + entry_bit =3D get_idle_ticks_bit(value);
> + if (entry_bit > MAX_BIT)
> + return -EINVAL;
> +
> + pw20_wt =3D value;
> + smp_call_function_single(cpu, set_pw20_wait_entry_bit,
> + &entry_bit, 1);
> +
> + return count;
> +}
> +
> +static ssize_t show_altivec_idle(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + u32 value;
> + unsigned int cpu =3D dev->id;
> +
> + smp_call_function_single(cpu, do_show_pwrmgtcr0, &value, 1);
> +
> + value &=3D PWRMGTCR0_AV_IDLE_PD_EN;
> +
> + return sprintf(buf, "%u\n", value ? 1 : 0);
> +}
> +
> +static void do_store_altivec_idle(void *val)
> +{
> + u32 *value =3D val;
> + u32 altivec_idle;
> +
> + altivec_idle =3D mfspr(SPRN_PWRMGTCR0);
> +
> + if (*value)
> + altivec_idle |=3D PWRMGTCR0_AV_IDLE_PD_EN;
> + else
> + altivec_idle &=3D ~PWRMGTCR0_AV_IDLE_PD_EN;
> +
> + mtspr(SPRN_PWRMGTCR0, altivec_idle);
> +}
> +
> +static ssize_t store_altivec_idle(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + u32 value;
> + unsigned int cpu =3D dev->id;
> +
> + if (kstrtou32(buf, 0, &value))
> + return -EINVAL;
> +
> + if (value > 1)
> + return -EINVAL;
> +
> + smp_call_function_single(cpu, do_store_altivec_idle, &value, 1);
> +
> + return count;
> +}
> +
> +static ssize_t show_altivec_idle_wait_time(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + u32 value;
> + u64 tb_cycle;
> + u64 time;
> +
> + unsigned int cpu =3D dev->id;
> +
> + if (!altivec_idle_wt) {
> + smp_call_function_single(cpu, do_show_pwrmgtcr0, &value, 1);
> + value =3D (value & PWRMGTCR0_AV_IDLE_CNT) >>
> + PWRMGTCR0_AV_IDLE_CNT_SHIFT;
> +
> + tb_cycle =3D (1 << (MAX_BIT - value)) * 2;
> + time =3D tb_cycle * (1000 / tb_ticks_per_usec) - 1;
> + } else {
> + time =3D altivec_idle_wt;
> + }
> +
> + return sprintf(buf, "%llu\n", time);
> +}
> +
> +static void set_altivec_idle_wait_entry_bit(void *val)
> +{
> + u32 *value =3D val;
> + u32 altivec_idle;
> +
> + altivec_idle =3D mfspr(SPRN_PWRMGTCR0);
> +
> + /* Set Automatic AltiVec Idle Count */
> + /* clear count */
> + altivec_idle &=3D ~PWRMGTCR0_AV_IDLE_CNT;
> +
> + /* set count */
> + altivec_idle |=3D ((MAX_BIT - *value) << PWRMGTCR0_AV_IDLE_CNT_SHIFT);
> +
> + mtspr(SPRN_PWRMGTCR0, altivec_idle);
> +}
> +
> +static ssize_t store_altivec_idle_wait_time(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + u32 entry_bit;
> + u64 value;
> +
> + unsigned int cpu =3D dev->id;
> +
> + if (kstrtou64(buf, 0, &value))
> + return -EINVAL;
> +
> + if (!value)
> + return -EINVAL;
> +
> + entry_bit =3D get_idle_ticks_bit(value);
> + if (entry_bit > MAX_BIT)
> + return -EINVAL;
> +
> + altivec_idle_wt =3D value;
> + smp_call_function_single(cpu, set_altivec_idle_wait_entry_bit,
> + &entry_bit, 1);
> +
> + return count;
> +}
> +
> +/*
> + * Enable/Disable interface:
> + * 0, disable. 1, enable.
> + */
> +static DEVICE_ATTR(pw20_state, 0600, show_pw20_state, store_pw20_state);
> +static DEVICE_ATTR(altivec_idle, 0600, show_altivec_idle, store_altivec_=
idle);
> +
> +/*
> + * Set wait time interface:(Nanosecond)
> + * Example: Base on TBfreq is 41MHZ.
> + * 1~47(ns): TB[63]
> + * 48~95(ns): TB[62]
> + * 96~191(ns): TB[61]
> + * 192~383(ns): TB[62]
> + * 384~767(ns): TB[60]
> + * ...
> + */
> +static DEVICE_ATTR(pw20_wait_time, 0600,
> + show_pw20_wait_time,
> + store_pw20_wait_time);
> +static DEVICE_ATTR(altivec_idle_wait_time, 0600,
> + show_altivec_idle_wait_time,
> + store_altivec_idle_wait_time);
> +#endif
> +
> /*
> * Enabling PMCs will slow partition context switch times so we only do
> * it the first time we write to the PMCs.
> @@ -407,6 +680,15 @@ static void register_cpu_online(unsigned int cpu)
> device_create_file(s, &dev_attr_pir);
> #endif /* CONFIG_PPC64 */
>=20
> +#ifdef CONFIG_FSL_SOC
> + if (PVR_VER(cur_cpu_spec->pvr_value) =3D=3D PVR_VER_E6500) {
> + device_create_file(s, &dev_attr_pw20_state);
> + device_create_file(s, &dev_attr_pw20_wait_time);
> +
> + device_create_file(s, &dev_attr_altivec_idle);
> + device_create_file(s, &dev_attr_altivec_idle_wait_time);
> + }
> +#endif
> cacheinfo_cpu_online(cpu);
> }
>=20
> @@ -479,6 +761,15 @@ static void unregister_cpu_online(unsigned int cpu)
> device_remove_file(s, &dev_attr_pir);
> #endif /* CONFIG_PPC64 */
>=20
> +#ifdef CONFIG_FSL_SOC
> + if (PVR_VER(cur_cpu_spec->pvr_value) =3D=3D PVR_VER_E6500) {
> + device_remove_file(s, &dev_attr_pw20_state);
> + device_remove_file(s, &dev_attr_pw20_wait_time);
> +
> + device_remove_file(s, &dev_attr_altivec_idle);
> + device_remove_file(s, &dev_attr_altivec_idle_wait_time);
> + }
> +#endif
> cacheinfo_cpu_offline(cpu);
> }
>=20
> --
> 1.8.0
>=20
>=20
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply
* Re: [PATCH V2 0/6] perf: New conditional branch filter
From: Anshuman Khandual @ 2013-09-25 6:15 UTC (permalink / raw)
To: Michael Ellerman
Cc: LKML, Stephane Eranian, Arnaldo Carvalho de Melo, Linux PPC dev,
Sukadev Bhattiprolu, Michael Neuling
In-Reply-To: <1380075585.14938.4.camel@concordia>
On 09/25/2013 07:49 AM, Michael Ellerman wrote:
> On Mon, 2013-09-23 at 14:45 +0530, Anshuman Khandual wrote:
>> On 09/21/2013 12:25 PM, Stephane Eranian wrote:
>>> On Tue, Sep 10, 2013 at 4:06 AM, Michael Ellerman
>>> <michael@ellerman.id.au> wrote:
>>>>>
>>>>> On Fri, 2013-08-30 at 09:54 +0530, Anshuman Khandual wrote:
>>>>>>> This patchset is the re-spin of the original branch stack sampling
>>>>>>> patchset which introduced new PERF_SAMPLE_BRANCH_COND filter. This patchset
>>>>>>> also enables SW based branch filtering support for PPC64 platforms which have
>>>>>>> branch stack sampling support. With this new enablement, the branch filter support
>>>>>>> for PPC64 platforms have been extended to include all these combinations discussed
>>>>>>> below with a sample test application program.
>>>>>
>>>>> ...
>>>>>
>>>>>>> Mixed filters
>>>>>>> -------------
>>>>>>> (6) perf record -e branch-misses:u -j any_call,any_ret ./cprog
>>>>>>> Error:
>>>>>>> The perf.data file has no samples!
>>>>>>>
>>>>>>> NOTE: As expected. The HW filters all the branches which are calls and SW tries to find return
>>>>>>> branches in that given set. Both the filters are mutually exclussive, so obviously no samples
>>>>>>> found in the end profile.
>>>>>
>>>>> The semantics of multiple filters is not clear to me. It could be an OR,
>>>>> or an AND. You have implemented AND, does that match existing behaviour
>>>>> on x86 for example?
>>>
>>> The semantic on the API is OR. AND does not make sense: CALL & RETURN?
>>> On x86, the HW filter is an OR (default: ALL, set bit to disable a
>>> type). I suspect
>>> it is similar on PPC.
>>
>> Given the situation as explained here, which semantic would be better for single
>> HW and multiple SW filters. Accordingly validate_instruction() function will have
>> to be re-implemented. But I believe OR-ing the SW filters will be preferable.
>>
>> (1) (HW_FILTER_1) && (SW_FILTER_1) && (SW_FILTER_2)
>> or
>> (2) (HW_FILTER_1) && (SW_FILTER_1 || SW_FILTER_2)
>>
>> Please let me know your inputs and suggestions on this. Thank you.
>
> You need to implement the correct semantics, regardless of how the
> hardware happens to work.
>
> That means if multiple filters are specified you need to do all the
> filtering in software.
Hello Stephane,
I looked at the X86 code on branch filtering implementation.
(1) During event creation intel_pmu_hw_config calls intel_pmu_setup_lbr_filter when LBR sampling
is required, intel_pmu_setup_lbr_filter calls these two functions
(a) intel_pmu_setup_sw_lbr_filter
"event->hw.branch_reg.reg" contains all the SW filter masks which can be
supported for the user requested filters event->attr.branch_sample_type (even
if some of them could implemented in PMU HW)
(b) intel_pmu_setup_hw_lbr_filter (when HW filtering is present)
"event->hw.branch_reg.config" contains all the PMU HW filter masks corresponding
to the requested filters in event->attr.branch_sample_type. One point to note
here is that if the user has requested for some branch filter which is not supported
in the HW LBR filter, the event creation request is rejected with EOPNOTSUPP. This
not true for the filters which can be ignored in the PMU.
(2) When the event is enabled in the PMU
(a) cpuc->lbr_sel->config gets into the HW register to enable the filtering of branches
which was determined in the function intel_pmu_setup_hw_lbr_filter.
(3) After the IRQ happened, intel_pmu_lbr_read reads all the entries from the LBR HW and then
applies the filter in the function intel_pmu_lbr_filter.
(a) intel_pmu_lbr_filter functions take into account cpuc->br_sel (which is nothing but
event->hw.branch_reg.reg as determined in the function intel_pmu_setup_sw_lbr_filter)
which contains the entire branch filter request set in terms applicable SW filter. Here
the semantic is OR when we look at from SW filter implementation point of view.
BUT what branch record set we are working on right now ? A set which was captured with LBR HW
with cpuc->lbr_sel->config filters enabled on it. So to me the X86 implementation of the semantics
look something like this.
A - Branch filter set requested by the user
B - Subset of A which can be supported in HW
C - Subset of A which can be supported in SW
(B) && (C)
NOTE: Individual filters are OR-ed inside both B and C sets.
So here the semantics is not a true OR. This is my understanding till now which may be wrong. Please
help me understand if the semantics is something otherwise than what is explained above.
In POWER8 because we cannot OR individual HW PMU supported filters, till now the semantics looked a bit odd.
But as Michael has pointed out here that if there are multiple branch filter requests implement all of them
in SW. Only in case where the user requests for an individual filter and if it happen to be supported in HW
PMU, we will use the PMU filters.
Regards
Anshuman
^ permalink raw reply
* [PATCH 2/2] powerpc: Add real mode cache inhibited IO accessors
From: Michael Ellerman @ 2013-09-25 5:38 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <1380087532-20219-1-git-send-email-michael@ellerman.id.au>
These accessors allow us to do cache inhibited accesses when in real
mode. They should only be used in real mode.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
arch/powerpc/include/asm/io.h | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h
index 8b3bd66..1cac8d3 100644
--- a/arch/powerpc/include/asm/io.h
+++ b/arch/powerpc/include/asm/io.h
@@ -178,10 +178,24 @@ DEF_MMIO_OUT_DFORM(out_be32, 32, stw);
DEF_MMIO_OUT_XFORM(out_le16, 16, sthbrx);
DEF_MMIO_OUT_XFORM(out_le32, 32, stwbrx);
+/*
+ * Cache inhibitied accessors for use in real mode, you don't want to use these
+ * unless you know what you're doing.
+ */
+DEF_MMIO_OUT_XFORM(out_rm8, 8, stbcix);
+DEF_MMIO_OUT_XFORM(out_rm16, 16, sthcix);
+DEF_MMIO_OUT_XFORM(out_rm32, 32, stwcix);
+DEF_MMIO_IN_XFORM(in_rm8, 8, lbzcix);
+DEF_MMIO_IN_XFORM(in_rm16, 16, lhzcix);
+DEF_MMIO_IN_XFORM(in_rm32, 32, lwzcix);
+
#ifdef __powerpc64__
DEF_MMIO_OUT_DFORM(out_be64, 64, std);
DEF_MMIO_IN_DFORM(in_be64, 64, ld);
+DEF_MMIO_OUT_XFORM(out_rm64, 64, stdcix);
+DEF_MMIO_IN_XFORM(in_rm64, 64, ldcix);
+
/* There is no asm instructions for 64 bits reverse loads and stores */
static inline u64 in_le64(const volatile u64 __iomem *addr)
{
--
1.8.1.2
^ permalink raw reply related
* [PATCH 1/2] powerpc: Rename IO accessor generation macros
From: Michael Ellerman @ 2013-09-25 5:38 UTC (permalink / raw)
To: linuxppc-dev
In io.h we have macros to generate our IO accessors. These are currently
named FOO_BE() and FOO_LE() to indicate big & little endian. However the
distinction between the macros is not so much the endianess as the form
of the instruction that is used.
Rename the macros to reflect that, so we end up with XFORM and DFORM
variants, and use them appropriately.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
arch/powerpc/include/asm/io.h | 36 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h
index 5a64757..8b3bd66 100644
--- a/arch/powerpc/include/asm/io.h
+++ b/arch/powerpc/include/asm/io.h
@@ -113,7 +113,7 @@ extern bool isa_io_special;
/* gcc 4.0 and older doesn't have 'Z' constraint */
#if __GNUC__ < 4 || (__GNUC__ == 4 && __GNUC_MINOR__ == 0)
-#define DEF_MMIO_IN_LE(name, size, insn) \
+#define DEF_MMIO_IN_XFORM(name, size, insn) \
static inline u##size name(const volatile u##size __iomem *addr) \
{ \
u##size ret; \
@@ -122,7 +122,7 @@ static inline u##size name(const volatile u##size __iomem *addr) \
return ret; \
}
-#define DEF_MMIO_OUT_LE(name, size, insn) \
+#define DEF_MMIO_OUT_XFORM(name, size, insn) \
static inline void name(volatile u##size __iomem *addr, u##size val) \
{ \
__asm__ __volatile__("sync;"#insn" %1,0,%2" \
@@ -130,7 +130,7 @@ static inline void name(volatile u##size __iomem *addr, u##size val) \
IO_SET_SYNC_FLAG(); \
}
#else /* newer gcc */
-#define DEF_MMIO_IN_LE(name, size, insn) \
+#define DEF_MMIO_IN_XFORM(name, size, insn) \
static inline u##size name(const volatile u##size __iomem *addr) \
{ \
u##size ret; \
@@ -139,7 +139,7 @@ static inline u##size name(const volatile u##size __iomem *addr) \
return ret; \
}
-#define DEF_MMIO_OUT_LE(name, size, insn) \
+#define DEF_MMIO_OUT_XFORM(name, size, insn) \
static inline void name(volatile u##size __iomem *addr, u##size val) \
{ \
__asm__ __volatile__("sync;"#insn" %1,%y0" \
@@ -148,7 +148,7 @@ static inline void name(volatile u##size __iomem *addr, u##size val) \
}
#endif
-#define DEF_MMIO_IN_BE(name, size, insn) \
+#define DEF_MMIO_IN_DFORM(name, size, insn) \
static inline u##size name(const volatile u##size __iomem *addr) \
{ \
u##size ret; \
@@ -157,7 +157,7 @@ static inline u##size name(const volatile u##size __iomem *addr) \
return ret; \
}
-#define DEF_MMIO_OUT_BE(name, size, insn) \
+#define DEF_MMIO_OUT_DFORM(name, size, insn) \
static inline void name(volatile u##size __iomem *addr, u##size val) \
{ \
__asm__ __volatile__("sync;"#insn"%U0%X0 %1,%0" \
@@ -166,21 +166,21 @@ static inline void name(volatile u##size __iomem *addr, u##size val) \
}
-DEF_MMIO_IN_BE(in_8, 8, lbz);
-DEF_MMIO_IN_BE(in_be16, 16, lhz);
-DEF_MMIO_IN_BE(in_be32, 32, lwz);
-DEF_MMIO_IN_LE(in_le16, 16, lhbrx);
-DEF_MMIO_IN_LE(in_le32, 32, lwbrx);
+DEF_MMIO_IN_DFORM(in_8, 8, lbz);
+DEF_MMIO_IN_DFORM(in_be16, 16, lhz);
+DEF_MMIO_IN_DFORM(in_be32, 32, lwz);
+DEF_MMIO_IN_XFORM(in_le16, 16, lhbrx);
+DEF_MMIO_IN_XFORM(in_le32, 32, lwbrx);
-DEF_MMIO_OUT_BE(out_8, 8, stb);
-DEF_MMIO_OUT_BE(out_be16, 16, sth);
-DEF_MMIO_OUT_BE(out_be32, 32, stw);
-DEF_MMIO_OUT_LE(out_le16, 16, sthbrx);
-DEF_MMIO_OUT_LE(out_le32, 32, stwbrx);
+DEF_MMIO_OUT_DFORM(out_8, 8, stb);
+DEF_MMIO_OUT_DFORM(out_be16, 16, sth);
+DEF_MMIO_OUT_DFORM(out_be32, 32, stw);
+DEF_MMIO_OUT_XFORM(out_le16, 16, sthbrx);
+DEF_MMIO_OUT_XFORM(out_le32, 32, stwbrx);
#ifdef __powerpc64__
-DEF_MMIO_OUT_BE(out_be64, 64, std);
-DEF_MMIO_IN_BE(in_be64, 64, ld);
+DEF_MMIO_OUT_DFORM(out_be64, 64, std);
+DEF_MMIO_IN_DFORM(in_be64, 64, ld);
/* There is no asm instructions for 64 bits reverse loads and stores */
static inline u64 in_le64(const volatile u64 __iomem *addr)
--
1.8.1.2
^ permalink raw reply related
* RE: [PATCH v4 1/4] powerpc/fsl: add E6500 PVR and SPRN_PWRMGTCR0 define
From: Wang Dongsheng-B40534 @ 2013-09-25 5:08 UTC (permalink / raw)
To: Bhushan Bharat-R65777, Kumar Gala
Cc: Wood Scott-B07421, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <6A3DF150A5B70D4F9B66A25E3F7C888D0717F7DC@039-SN2MPN1-011.039d.mgd.msft.net>
> -----Original Message-----
> From: Bhushan Bharat-R65777
> Sent: Wednesday, September 25, 2013 11:43 AM
> To: Kumar Gala
> Cc: Wang Dongsheng-B40534; Wood Scott-B07421; linuxppc-
> dev@lists.ozlabs.org
> Subject: RE: [PATCH v4 1/4] powerpc/fsl: add E6500 PVR and SPRN_PWRMGTCR0
> define
>=20
>=20
>=20
> > -----Original Message-----
> > From: Kumar Gala [mailto:galak@kernel.crashing.org]
> > Sent: Tuesday, September 24, 2013 9:19 PM
> > To: Bhushan Bharat-R65777
> > Cc: Wang Dongsheng-B40534; Wood Scott-B07421;
> > linuxppc-dev@lists.ozlabs.org
> > Subject: Re: [PATCH v4 1/4] powerpc/fsl: add E6500 PVR and
> > SPRN_PWRMGTCR0 define
> >
> >
> > On Sep 24, 2013, at 6:21 AM, Bhushan Bharat-R65777 wrote:
> >
> > >
> > >
> > >> -----Original Message-----
> > >> From: Linuxppc-dev [mailto:linuxppc-dev-
> > >> bounces+bharat.bhushan=3Dfreescale.com@lists.ozlabs.org] On Behalf O=
f
> > >> bounces+Dongsheng
> > >> Wang
> > >> Sent: Tuesday, September 24, 2013 2:58 PM
> > >> To: Wood Scott-B07421
> > >> Cc: linuxppc-dev@lists.ozlabs.org; Wang Dongsheng-B40534
> > >> Subject: [PATCH v4 1/4] powerpc/fsl: add E6500 PVR and
> > >> SPRN_PWRMGTCR0 define
> > >>
> > >> From: Wang Dongsheng <dongsheng.wang@freescale.com>
> > >>
> > >> E6500 PVR and SPRN_PWRMGTCR0 will be used in subsequent
> > >> pw20/altivec idle patches.
> > >>
> > >> Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
> > >> ---
> > >> *v3:
> > >> Add bit definitions for PWRMGTCR0.
> > >>
> > >> arch/powerpc/include/asm/reg.h | 2 ++
> > >> arch/powerpc/include/asm/reg_booke.h | 9 +++++++++
> > >> 2 files changed, 11 insertions(+)
> > >>
> > >> diff --git a/arch/powerpc/include/asm/reg.h
> > >> b/arch/powerpc/include/asm/reg.h index 64264bf..d4160ca 100644
> > >> --- a/arch/powerpc/include/asm/reg.h
> > >> +++ b/arch/powerpc/include/asm/reg.h
> > >> @@ -1053,6 +1053,8 @@
> > >> #define PVR_8560 0x80200000
> > >> #define PVR_VER_E500V1 0x8020
> > >> #define PVR_VER_E500V2 0x8021
> > >> +#define PVR_VER_E6500 0x8040
> > >> +
> > >> /*
> > >> * For the 8xx processors, all of them report the same PVR family
> > >> for
> > >> * the PowerPC core. The various versions of these processors must
> > >> be diff -- git a/arch/powerpc/include/asm/reg_booke.h
> > >> b/arch/powerpc/include/asm/reg_booke.h
> > >> index ed8f836..4a6457e 100644
> > >> --- a/arch/powerpc/include/asm/reg_booke.h
> > >> +++ b/arch/powerpc/include/asm/reg_booke.h
> > >> @@ -170,6 +170,7 @@
> > >> #define SPRN_L2CSR1 0x3FA /* L2 Data Cache Control and Status
> Register 1
> > >> */
> > >> #define SPRN_DCCR 0x3FA /* Data Cache Cacheability Register */
> > >> #define SPRN_ICCR 0x3FB /* Instruction Cache Cacheability Register
> */
> > >> +#define SPRN_PWRMGTCR0 0x3FB /* Power management control register
> 0 */
> > >
> > > Is this generic for booke or e6500 specific? I can't see this
> > > register either
> > in ISA and EREF.
> > > Also I can see SPRN_ICCR also with same SPRN, how that is possible?
> >
> > Its possibly because the register maybe in implementation specific
> > region. I'm guessing ICCR is a 40x specific register.
>=20
> Kumar, this seems to create confusion?=20
I don't think this define will create a confusion, because this is only SPR=
number
definition and we already have a document(like EREF, ISA, this register def=
ine in
E6500-EREF) to describe these registers. There are no conflicts and other p=
latform
and different platforms for the same register have different purposes, it l=
ooks normal.
Instead we should put together, so as to remind that the SPR will be reuse =
from other platforms.
-dongsheng
^ permalink raw reply
* [git pull] Please pull powerpc.git merge branch
From: Benjamin Herrenschmidt @ 2013-09-25 4:39 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linuxppc-dev, Linux Kernel list
Hi Linus !
Here are a few things for -rc2, this time it's all written by me so it
can only be perfect .... right ? :)
So we have the fix to call irq_enter/exit on the irq stack we've been
discussing, plus a cleanup on top to remove an unused (and broken)
stack limit tracking feature (well, make it 32-bit only in fact where
it is used and works properly).
Then we have two things that I wrote over the last couple of days and
made the executive decision to include just because I can (and I'm sure
you won't object .... right ?).
They fix a couple of annoying and long standing "issues":
- We had separate zImages for when booting via Open Firmware vs.
booting via a flat device-tree, while it's trivial to make one that
deals with both
- We wasted a ton of cycles spinning secondary CPUs uselessly at boot
instead of starting them when needed on pseries, thus contributing
significantly to global warming.
Cheers,
Ben.
The following changes since commit 4a10c2ac2f368583138b774ca41fac4207911983:
Linux 3.12-rc2 (2013-09-23 15:41:09 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/benh/powerpc.git merge
for you to fetch changes up to dbe78b40118636f2d5d276144239dd4bfd5f04f9:
powerpc/pseries: Do not start secondaries in Open Firmware (2013-09-25 14:19:00 +1000)
----------------------------------------------------------------
Benjamin Herrenschmidt (4):
powerpc/irq: Run softirqs off the top of the irq stack
powerpc: Remove ksp_limit on ppc64
powerpc/zImage: make the "OF" wrapper support ePAPR boot
powerpc/pseries: Do not start secondaries in Open Firmware
arch/powerpc/boot/Makefile | 4 +-
arch/powerpc/boot/epapr-wrapper.c | 9 ++++
arch/powerpc/boot/epapr.c | 4 +-
arch/powerpc/boot/of.c | 16 +++++-
arch/powerpc/boot/wrapper | 9 ++--
arch/powerpc/include/asm/irq.h | 4 +-
arch/powerpc/include/asm/processor.h | 4 +-
arch/powerpc/kernel/asm-offsets.c | 3 +-
arch/powerpc/kernel/irq.c | 100 +++++++++++++++--------------------
arch/powerpc/kernel/misc_32.S | 25 +++++++--
arch/powerpc/kernel/misc_64.S | 10 ++--
arch/powerpc/kernel/process.c | 3 +-
arch/powerpc/kernel/prom_init.c | 21 ++++++++
arch/powerpc/lib/sstep.c | 3 +-
arch/powerpc/platforms/pseries/smp.c | 26 +++++----
15 files changed, 147 insertions(+), 94 deletions(-)
create mode 100644 arch/powerpc/boot/epapr-wrapper.c
^ permalink raw reply
* [PATCH] powerpc/pseries: Do not start secondaries in Open Firmware
From: Benjamin Herrenschmidt @ 2013-09-25 4:06 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Paul Mackerras
Starting secondary CPUs early on from Open Firmware and placing them
in a holding spin loop slows down the boot process significantly under
some hypervisors such as KVM.
This is also unnecessary when RTAS supports querying the CPU state
So let's not do it.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
arch/powerpc/kernel/prom_init.c | 21 +++++++++++++++++++++
arch/powerpc/platforms/pseries/smp.c | 26 ++++++++++++++++----------
2 files changed, 37 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index 12e656f..5fe2842 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -196,6 +196,8 @@ static int __initdata mem_reserve_cnt;
static cell_t __initdata regbuf[1024];
+static bool rtas_has_query_cpu_stopped;
+
/*
* Error results ... some OF calls will return "-1" on error, some
@@ -1574,6 +1576,11 @@ static void __init prom_instantiate_rtas(void)
prom_setprop(rtas_node, "/rtas", "linux,rtas-entry",
&val, sizeof(val));
+ /* Check if it supports "query-cpu-stopped-state" */
+ if (prom_getprop(rtas_node, "query-cpu-stopped-state",
+ &val, sizeof(val)) != PROM_ERROR)
+ rtas_has_query_cpu_stopped = true;
+
#if defined(CONFIG_PPC_POWERNV) && defined(__BIG_ENDIAN__)
/* PowerVN takeover hack */
prom_rtas_data = base;
@@ -1815,6 +1822,18 @@ static void __init prom_hold_cpus(void)
= (void *) LOW_ADDR(__secondary_hold_acknowledge);
unsigned long secondary_hold = LOW_ADDR(__secondary_hold);
+ /*
+ * On pseries, if RTAS supports "query-cpu-stopped-state",
+ * we skip this stage, the CPUs will be started by the
+ * kernel using RTAS.
+ */
+ if ((of_platform == PLATFORM_PSERIES ||
+ of_platform == PLATFORM_PSERIES_LPAR) &&
+ rtas_has_query_cpu_stopped) {
+ prom_printf("prom_hold_cpus: skipped\n");
+ return;
+ }
+
prom_debug("prom_hold_cpus: start...\n");
prom_debug(" 1) spinloop = 0x%x\n", (unsigned long)spinloop);
prom_debug(" 1) *spinloop = 0x%x\n", *spinloop);
@@ -3011,6 +3030,8 @@ unsigned long __init prom_init(unsigned long r3, unsigned long r4,
* On non-powermacs, put all CPUs in spin-loops.
*
* PowerMacs use a different mechanism to spin CPUs
+ *
+ * (This must be done after instanciating RTAS)
*/
if (of_platform != PLATFORM_POWERMAC &&
of_platform != PLATFORM_OPAL)
diff --git a/arch/powerpc/platforms/pseries/smp.c b/arch/powerpc/platforms/pseries/smp.c
index 1c1771a..24f58cb 100644
--- a/arch/powerpc/platforms/pseries/smp.c
+++ b/arch/powerpc/platforms/pseries/smp.c
@@ -233,18 +233,24 @@ static void __init smp_init_pseries(void)
alloc_bootmem_cpumask_var(&of_spin_mask);
- /* Mark threads which are still spinning in hold loops. */
- if (cpu_has_feature(CPU_FTR_SMT)) {
- for_each_present_cpu(i) {
- if (cpu_thread_in_core(i) == 0)
- cpumask_set_cpu(i, of_spin_mask);
- }
- } else {
- cpumask_copy(of_spin_mask, cpu_present_mask);
+ /*
+ * Mark threads which are still spinning in hold loops
+ *
+ * We know prom_init will not have started them if RTAS supports
+ * query-cpu-stopped-state.
+ */
+ if (rtas_token("query-cpu-stopped-state") == RTAS_UNKNOWN_SERVICE) {
+ if (cpu_has_feature(CPU_FTR_SMT)) {
+ for_each_present_cpu(i) {
+ if (cpu_thread_in_core(i) == 0)
+ cpumask_set_cpu(i, of_spin_mask);
+ }
+ } else
+ cpumask_copy(of_spin_mask, cpu_present_mask);
+
+ cpumask_clear_cpu(boot_cpuid, of_spin_mask);
}
- cpumask_clear_cpu(boot_cpuid, of_spin_mask);
-
/* Non-lpar has additional take/give timebase */
if (rtas_token("freeze-time-base") != RTAS_UNKNOWN_SERVICE) {
smp_ops->give_timebase = rtas_give_timebase;
^ permalink raw reply related
* RE: [PATCH v4 1/4] powerpc/fsl: add E6500 PVR and SPRN_PWRMGTCR0 define
From: Bhushan Bharat-R65777 @ 2013-09-25 3:42 UTC (permalink / raw)
To: Kumar Gala
Cc: Wood Scott-B07421, Wang Dongsheng-B40534,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <D3D5E237-6C31-424C-A340-E79F6AF5B0B2@kernel.crashing.org>
> -----Original Message-----
> From: Kumar Gala [mailto:galak@kernel.crashing.org]
> Sent: Tuesday, September 24, 2013 9:19 PM
> To: Bhushan Bharat-R65777
> Cc: Wang Dongsheng-B40534; Wood Scott-B07421; linuxppc-dev@lists.ozlabs.o=
rg
> Subject: Re: [PATCH v4 1/4] powerpc/fsl: add E6500 PVR and SPRN_PWRMGTCR0=
define
>=20
>=20
> On Sep 24, 2013, at 6:21 AM, Bhushan Bharat-R65777 wrote:
>=20
> >
> >
> >> -----Original Message-----
> >> From: Linuxppc-dev [mailto:linuxppc-dev-
> >> bounces+bharat.bhushan=3Dfreescale.com@lists.ozlabs.org] On Behalf Of
> >> bounces+Dongsheng
> >> Wang
> >> Sent: Tuesday, September 24, 2013 2:58 PM
> >> To: Wood Scott-B07421
> >> Cc: linuxppc-dev@lists.ozlabs.org; Wang Dongsheng-B40534
> >> Subject: [PATCH v4 1/4] powerpc/fsl: add E6500 PVR and SPRN_PWRMGTCR0
> >> define
> >>
> >> From: Wang Dongsheng <dongsheng.wang@freescale.com>
> >>
> >> E6500 PVR and SPRN_PWRMGTCR0 will be used in subsequent pw20/altivec
> >> idle patches.
> >>
> >> Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
> >> ---
> >> *v3:
> >> Add bit definitions for PWRMGTCR0.
> >>
> >> arch/powerpc/include/asm/reg.h | 2 ++
> >> arch/powerpc/include/asm/reg_booke.h | 9 +++++++++
> >> 2 files changed, 11 insertions(+)
> >>
> >> diff --git a/arch/powerpc/include/asm/reg.h
> >> b/arch/powerpc/include/asm/reg.h index 64264bf..d4160ca 100644
> >> --- a/arch/powerpc/include/asm/reg.h
> >> +++ b/arch/powerpc/include/asm/reg.h
> >> @@ -1053,6 +1053,8 @@
> >> #define PVR_8560 0x80200000
> >> #define PVR_VER_E500V1 0x8020
> >> #define PVR_VER_E500V2 0x8021
> >> +#define PVR_VER_E6500 0x8040
> >> +
> >> /*
> >> * For the 8xx processors, all of them report the same PVR family for
> >> * the PowerPC core. The various versions of these processors must be
> >> diff -- git a/arch/powerpc/include/asm/reg_booke.h
> >> b/arch/powerpc/include/asm/reg_booke.h
> >> index ed8f836..4a6457e 100644
> >> --- a/arch/powerpc/include/asm/reg_booke.h
> >> +++ b/arch/powerpc/include/asm/reg_booke.h
> >> @@ -170,6 +170,7 @@
> >> #define SPRN_L2CSR1 0x3FA /* L2 Data Cache Control and Status Register=
1
> >> */
> >> #define SPRN_DCCR 0x3FA /* Data Cache Cacheability Register */
> >> #define SPRN_ICCR 0x3FB /* Instruction Cache Cacheability Register */
> >> +#define SPRN_PWRMGTCR0 0x3FB /* Power management control register 0 *=
/
> >
> > Is this generic for booke or e6500 specific? I can't see this register =
either
> in ISA and EREF.
> > Also I can see SPRN_ICCR also with same SPRN, how that is possible?
>=20
> Its possibly because the register maybe in implementation specific region=
. I'm
> guessing ICCR is a 40x specific register.
Kumar, this seems to create confusion? Although I do not like so many heade=
r files but still I think we can have reg_4xx.h, reg_fsl_booke.h etc for im=
plementation specific definitions.
-Bharat
>=20
> - k
>=20
^ permalink raw reply
* RE: [PATCH v4 1/4] powerpc/fsl: add E6500 PVR and SPRN_PWRMGTCR0 define
From: Wang Dongsheng-B40534 @ 2013-09-25 2:34 UTC (permalink / raw)
To: Kumar Gala, Bhushan Bharat-R65777
Cc: Wood Scott-B07421, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <D3D5E237-6C31-424C-A340-E79F6AF5B0B2@kernel.crashing.org>
> >> /*
> >> * For the 8xx processors, all of them report the same PVR family for
> >> * the PowerPC core. The various versions of these processors must be
> >> diff -- git a/arch/powerpc/include/asm/reg_booke.h
> >> b/arch/powerpc/include/asm/reg_booke.h
> >> index ed8f836..4a6457e 100644
> >> --- a/arch/powerpc/include/asm/reg_booke.h
> >> +++ b/arch/powerpc/include/asm/reg_booke.h
> >> @@ -170,6 +170,7 @@
> >> #define SPRN_L2CSR1 0x3FA /* L2 Data Cache Control and Status
> Register 1
> >> */
> >> #define SPRN_DCCR 0x3FA /* Data Cache Cacheability Register */
> >> #define SPRN_ICCR 0x3FB /* Instruction Cache Cacheability Register
> */
> >> +#define SPRN_PWRMGTCR0 0x3FB /* Power management control register
> 0 */
> >
> > Is this generic for booke or e6500 specific? I can't see this register
> either in ISA and EREF.
Yes, now only e6500 have this register. There is no problem in this definit=
ion,
because no conflict in FSL platform.
> > Also I can see SPRN_ICCR also with same SPRN, how that is possible?
>=20
> Its possibly because the register maybe in implementation specific region=
.
> I'm guessing ICCR is a 40x specific register.
Yes, kumar is right. Its use only in 4xx series of chips.
ICTC(arch/powerpc/include/asm/reg.h) also use 0x3FB, Its use only in 6xx se=
ries of chips.
-dongsheng
^ permalink raw reply
* Re: [PATCH V2 0/6] perf: New conditional branch filter
From: Michael Ellerman @ 2013-09-25 2:19 UTC (permalink / raw)
To: Anshuman Khandual
Cc: Sukadev Bhattiprolu, LKML, Stephane Eranian,
Arnaldo Carvalho de Melo, Linux PPC dev, Michael Neuling
In-Reply-To: <524006C4.3010006@linux.vnet.ibm.com>
On Mon, 2013-09-23 at 14:45 +0530, Anshuman Khandual wrote:
> On 09/21/2013 12:25 PM, Stephane Eranian wrote:
> > On Tue, Sep 10, 2013 at 4:06 AM, Michael Ellerman
> > <michael@ellerman.id.au> wrote:
> >> >
> >> > On Fri, 2013-08-30 at 09:54 +0530, Anshuman Khandual wrote:
> >>> > > This patchset is the re-spin of the original branch stack sampling
> >>> > > patchset which introduced new PERF_SAMPLE_BRANCH_COND filter. This patchset
> >>> > > also enables SW based branch filtering support for PPC64 platforms which have
> >>> > > branch stack sampling support. With this new enablement, the branch filter support
> >>> > > for PPC64 platforms have been extended to include all these combinations discussed
> >>> > > below with a sample test application program.
> >> >
> >> > ...
> >> >
> >>> > > Mixed filters
> >>> > > -------------
> >>> > > (6) perf record -e branch-misses:u -j any_call,any_ret ./cprog
> >>> > > Error:
> >>> > > The perf.data file has no samples!
> >>> > >
> >>> > > NOTE: As expected. The HW filters all the branches which are calls and SW tries to find return
> >>> > > branches in that given set. Both the filters are mutually exclussive, so obviously no samples
> >>> > > found in the end profile.
> >> >
> >> > The semantics of multiple filters is not clear to me. It could be an OR,
> >> > or an AND. You have implemented AND, does that match existing behaviour
> >> > on x86 for example?
> >
> > The semantic on the API is OR. AND does not make sense: CALL & RETURN?
> > On x86, the HW filter is an OR (default: ALL, set bit to disable a
> > type). I suspect
> > it is similar on PPC.
>
> Given the situation as explained here, which semantic would be better for single
> HW and multiple SW filters. Accordingly validate_instruction() function will have
> to be re-implemented. But I believe OR-ing the SW filters will be preferable.
>
> (1) (HW_FILTER_1) && (SW_FILTER_1) && (SW_FILTER_2)
> or
> (2) (HW_FILTER_1) && (SW_FILTER_1 || SW_FILTER_2)
>
> Please let me know your inputs and suggestions on this. Thank you.
You need to implement the correct semantics, regardless of how the
hardware happens to work.
That means if multiple filters are specified you need to do all the
filtering in software.
cheers
^ permalink raw reply
* Re: linux-next: build failure after merge of the akpm tree
From: Timur Tabi @ 2013-09-25 1:21 UTC (permalink / raw)
To: Stephen Rothwell
Cc: Greg KH, lkml, Sergei Trofimovich, linux-next@vger.kernel.org,
Andrew Morton, ppc-dev, Timur Tabi
In-Reply-To: <20130925110643.db5fa154bea3838ed6affa45@canb.auug.org.au>
On Tue, Sep 24, 2013 at 8:06 PM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> Hi Andrew,
>
> After merging the akpm tree, linux-next builds (powerpc allmodconfig)
> fail like this:
>
> drivers/tty/ehv_bytechan.c:362:1: error: type defaults to 'int' in declaration of 'console_initcall' [-Werror=implicit-int]
>
> Caused by commit 0f01cf96c2d4 ("./Makefile: enable -Werror=implicit-int
> and -Werror=strict-prototypes by default") which has bee in linux-next
> since Aug 16. This commit exposed that fact that
> drivers/tty/ehv_bytechan.c can be built as a module, but has a
> console_initcall (which is not available to modules).
Is this something new? This code hasn't changed in over two years, so
I'm surprised it suddenly broke.
> This was
> originally introduced in commit dcd83aaff1c8 ("tty/powerpc: introduce the
> ePAPR embedded hypervisor byte channel driver") in v3.2.
>
> Anyone got a good solution?
How about:
#ifndef MODULE
static int __init ehv_bc_console_init(void)
{
...
}
console_initcall(ehv_bc_console_init);
#endif
^ permalink raw reply
* linux-next: build failure after merge of the akpm tree
From: Stephen Rothwell @ 2013-09-25 1:06 UTC (permalink / raw)
To: Andrew Morton
Cc: Greg KH, linux-kernel, Sergei Trofimovich, linux-next, ppc-dev,
Timur Tabi
[-- Attachment #1: Type: text/plain, Size: 792 bytes --]
Hi Andrew,
After merging the akpm tree, linux-next builds (powerpc allmodconfig)
fail like this:
drivers/tty/ehv_bytechan.c:362:1: error: type defaults to 'int' in declaration of 'console_initcall' [-Werror=implicit-int]
Caused by commit 0f01cf96c2d4 ("./Makefile: enable -Werror=implicit-int
and -Werror=strict-prototypes by default") which has bee in linux-next
since Aug 16. This commit exposed that fact that
drivers/tty/ehv_bytechan.c can be built as a module, but has a
console_initcall (which is not available to modules). This was
originally introduced in commit dcd83aaff1c8 ("tty/powerpc: introduce the
ePAPR embedded hypervisor byte channel driver") in v3.2.
Anyone got a good solution?
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v3 3/3] powerpc/85xx: use one kernel option for all the CoreNet_Generic boards
From: Kevin Hao @ 2013-09-25 0:58 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc
In-Reply-To: <1380062492.24959.123.camel@snotra.buserror.net>
[-- Attachment #1: Type: text/plain, Size: 609 bytes --]
On Tue, Sep 24, 2013 at 05:41:32PM -0500, Scott Wood wrote:
> On Tue, 2013-09-24 at 11:03 +0800, Kevin Hao wrote:
> > + and B4 QDS boards
[...]
>
> Is there any difference between the 32-bit and 64-bit versions of this
> config symbol, other than the help text?
No. As you know some of these boards only support 32bit kernel and some of
them only support 64bit kernel. It will definitely cause confusion when using
only one kernel option for all these boards. So I divide this into two options
(even the same name) for 32bit and 64bit respectively.
Thanks,
Kevin
>
> -Scott
>
>
>
[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2][v3] powerpc/fsl-booke: Add initial T104x_QDS board support
From: Timur Tabi @ 2013-09-25 0:18 UTC (permalink / raw)
To: Scott Wood
Cc: Wood Scott-B07421, Jain Priyanka-B32167, Aggrwal Poonam-B10812,
linuxppc-dev@lists.ozlabs.org, Kushwaha Prabhakar-B32579
In-Reply-To: <1380065132.24959.144.camel@snotra.buserror.net>
Scott Wood wrote:
>> >I think it should be okay to leave the DIU node. I think the kernel
>> >will crash if you try to enable a DIU console (video= on the kernel
>> >command line), but I think it's okay to ignore that for the moment.
> Sounds like a bug in the DIU driver. It should fail gracefully in the
> absence of platform support.
I agree. And one day, I'll get a chance to look at the code to see if
it's broken.
^ permalink raw reply
* Re: [PATCH 1/2][v4] powerpc/fsl-booke: Add initial T104x_QDS board support
From: Scott Wood @ 2013-09-25 0:01 UTC (permalink / raw)
To: Prabhakar Kushwaha; +Cc: Priyanka Jain, linuxppc-dev, Poonam Aggrwal
In-Reply-To: <1379908288-7804-1-git-send-email-prabhakar@freescale.com>
On Mon, 2013-09-23 at 09:21 +0530, Prabhakar Kushwaha wrote:
> +/* coreint doesn't play nice with lazy EE, use legacy mpic for now */
> + .get_irq = mpic_get_coreint_irq,
Remove the comment.
Also note Kevin's patch to consolidate on a common corenet board file.
-Scott
^ permalink raw reply
* Re: [PATCH 1/7] powerpc: Add interface to get msi region information
From: Bjorn Helgaas @ 2013-09-24 23:58 UTC (permalink / raw)
To: Bharat Bhushan
Cc: agraf, joro, linux-kernel, iommu, Bharat Bhushan, alex.williamson,
linux-pci, scottwood, linuxppc-dev
In-Reply-To: <1379575763-2091-2-git-send-email-Bharat.Bhushan@freescale.com>
On Thu, Sep 19, 2013 at 12:59:17PM +0530, Bharat Bhushan wrote:
> This patch adds interface to get following information
> - Number of MSI regions (which is number of MSI banks for powerpc).
> - Get the region address range: Physical page which have the
> address/addresses used for generating MSI interrupt
> and size of the page.
>
> These are required to create IOMMU (Freescale PAMU) mapping for
> devices which are directly assigned using VFIO.
>
> Signed-off-by: Bharat Bhushan <bharat.bhushan@freescale.com>
> ---
> arch/powerpc/include/asm/machdep.h | 8 +++++++
> arch/powerpc/include/asm/pci.h | 2 +
> arch/powerpc/kernel/msi.c | 18 ++++++++++++++++
> arch/powerpc/sysdev/fsl_msi.c | 39 +++++++++++++++++++++++++++++++++--
> arch/powerpc/sysdev/fsl_msi.h | 11 ++++++++-
> drivers/pci/msi.c | 26 ++++++++++++++++++++++++
> include/linux/msi.h | 8 +++++++
> include/linux/pci.h | 13 ++++++++++++
> 8 files changed, 120 insertions(+), 5 deletions(-)
>
> ...
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index aca7578..6d85c15 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -30,6 +30,20 @@ static int pci_msi_enable = 1;
>
> /* Arch hooks */
>
> +#ifndef arch_msi_get_region_count
> +int arch_msi_get_region_count(void)
> +{
> + return 0;
> +}
> +#endif
> +
> +#ifndef arch_msi_get_region
> +int arch_msi_get_region(int region_num, struct msi_region *region)
> +{
> + return 0;
> +}
> +#endif
This #define strategy is gone; see 4287d824 ("PCI: use weak functions for
MSI arch-specific functions"). Please use the weak function strategy
for your new MSI region functions.
> +
> #ifndef arch_msi_check_device
> int arch_msi_check_device(struct pci_dev *dev, int nvec, int type)
> {
> @@ -903,6 +917,18 @@ void pci_disable_msi(struct pci_dev *dev)
> }
> EXPORT_SYMBOL(pci_disable_msi);
>
> +int msi_get_region_count(void)
> +{
> + return arch_msi_get_region_count();
> +}
> +EXPORT_SYMBOL(msi_get_region_count);
> +
> +int msi_get_region(int region_num, struct msi_region *region)
> +{
> + return arch_msi_get_region(region_num, region);
> +}
> +EXPORT_SYMBOL(msi_get_region);
Please split these interface additions, i.e., the drivers/pci/msi.c,
include/linux/msi.h, and include/linux/pci.h changes, into a separate
patch.
I don't know enough about VFIO to understand why these new interfaces
are needed. Is this the first VFIO IOMMU driver? I see
vfio_iommu_spapr_tce.c and vfio_iommu_type1.c but I don't know if
they're comparable to the Freescale PAMU. Do other VFIO IOMMU
implementations support MSI? If so, do they handle the problem of
mapping the MSI regions in a different way?
> /**
> * pci_msix_table_size - return the number of device's MSI-X table entries
> * @dev: pointer to the pci_dev data structure of MSI-X device function
> diff --git a/include/linux/msi.h b/include/linux/msi.h
> index ee66f3a..ae32601 100644
> --- a/include/linux/msi.h
> +++ b/include/linux/msi.h
> @@ -50,6 +50,12 @@ struct msi_desc {
> struct kobject kobj;
> };
>
> +struct msi_region {
> + int region_num;
> + dma_addr_t addr;
> + size_t size;
> +};
This needs some sort of explanatory comment.
> /*
> * The arch hook for setup up msi irqs
> */
> @@ -58,5 +64,7 @@ void arch_teardown_msi_irq(unsigned int irq);
> int arch_setup_msi_irqs(struct pci_dev *dev, int nvec, int type);
> void arch_teardown_msi_irqs(struct pci_dev *dev);
> int arch_msi_check_device(struct pci_dev* dev, int nvec, int type);
> +int arch_msi_get_region_count(void);
> +int arch_msi_get_region(int region_num, struct msi_region *region);
>
> #endif /* LINUX_MSI_H */
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 186540d..2b26a59 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1126,6 +1126,7 @@ struct msix_entry {
> u16 entry; /* driver uses to specify entry, OS writes */
> };
>
> +struct msi_region;
>
> #ifndef CONFIG_PCI_MSI
> static inline int pci_enable_msi_block(struct pci_dev *dev, unsigned int nvec)
> @@ -1168,6 +1169,16 @@ static inline int pci_msi_enabled(void)
> {
> return 0;
> }
> +
> +static inline int msi_get_region_count(void)
> +{
> + return 0;
> +}
> +
> +static inline int msi_get_region(int region_num, struct msi_region *region)
> +{
> + return 0;
> +}
> #else
> int pci_enable_msi_block(struct pci_dev *dev, unsigned int nvec);
> int pci_enable_msi_block_auto(struct pci_dev *dev, unsigned int *maxvec);
> @@ -1180,6 +1191,8 @@ void pci_disable_msix(struct pci_dev *dev);
> void msi_remove_pci_irq_vectors(struct pci_dev *dev);
> void pci_restore_msi_state(struct pci_dev *dev);
> int pci_msi_enabled(void);
> +int msi_get_region_count(void);
> +int msi_get_region(int region_num, struct msi_region *region);
> #endif
>
> #ifdef CONFIG_PCIEPORTBUS
> --
> 1.7.0.4
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" 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 2/2][RFC][v3] pci: fsl: rework PCI driver compatible with Layerscape
From: Scott Wood @ 2013-09-24 23:40 UTC (permalink / raw)
To: Lian Minghuan-b31939; +Cc: Minghuan Lian, linuxppc-dev, Zang Roy-R61911
In-Reply-To: <52381F9C.200@freescale.com>
On Tue, 2013-09-17 at 17:23 +0800, Lian Minghuan-b31939 wrote:
> >> diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
> >> index a189ff0..4cb12e8 100644
> >> --- a/arch/powerpc/sysdev/fsl_pci.c
> >> +++ b/arch/powerpc/sysdev/fsl_pci.c
> >> @@ -62,7 +62,11 @@ static void quirk_fsl_pcie_header(struct pci_dev *dev)
> >> #if defined(CONFIG_FSL_SOC_BOOKE) || defined(CONFIG_PPC_86xx)
> >>
> >> #define MAX_PHYS_ADDR_BITS 40
> >> -static u64 pci64_dma_offset = 1ull << MAX_PHYS_ADDR_BITS;
> >> +
> >> +u64 fsl_arch_pci64_dma_offset(void)
> >> +{
> >> + return 1ull << MAX_PHYS_ADDR_BITS;
> >> +}
> >>
> >> static int fsl_pci_dma_set_mask(struct device *dev, u64 dma_mask)
> >> {
> >> @@ -77,17 +81,43 @@ static int fsl_pci_dma_set_mask(struct device *dev, u64 dma_mask)
> >> if ((dev->bus == &pci_bus_type) &&
> >> dma_mask >= DMA_BIT_MASK(MAX_PHYS_ADDR_BITS)) {
> >> set_dma_ops(dev, &dma_direct_ops);
> >> - set_dma_offset(dev, pci64_dma_offset);
> >> + set_dma_offset(dev, fsl_arch_pci64_dma_offset());
> >> }
> > Is the intent for fsl_arch_pci64_dma_offset() to eventually do something
> > that isn't calculable at compile time?
> >
> [Minghuan] fsl_arch_pci64_dma_offset() is also called by pci-fsl.c to
> setup inbound ATMU.
> I think different platform or architecture(LS1) may use different dma
> offset(maybe I am wrong
> they can use the same offset 1ull << MAX_PHYS_ADDR_BITS). I selected
> u64 fsl_arch_pci64_dma_offset(void) not extern u64 pci64_dma_offset to
> share the global
> value between /driver/pci/host/pci-fsl.c and
> arch/powerpc/sysdev/fsl_pci.c or / arch/arm/..../fsl_pci.c
> 'extern' variable will cause the warning when checking patch.
It will only warn if you're doing it wrong. :-P
Put the extern in a header and the actual declaration in the
arch-specific C file.
> >> *dev->dma_mask = dma_mask;
> >> return 0;
> >> }
> >>
> >> +struct fsl_pci *fsl_arch_sys_to_pci(void *sys)
> >> +{
> >> + struct pci_controller *hose = sys;
> >> + struct fsl_pci *pci = hose->private_data;
> > If this were just to convert to fsl_pci, that seems like header
> > material.
> [Minghuan] In arm architecture it will be implemented like this:
> struct fsl_pci *fsl_arch_sys_to_pci(void *sys) {
> struct pci_sys_data *sys_data = sys;
> return sys_data->private_data;
> }
Right, so make it an inline function in each architecture's header file.
> driver/pci/host/pci-fsl.c should not include any arch specific header file.
include/linux/fsl/pci.h could include <asm/fsl/pci.h>. If this is the
only arch-specific thing that you need in a header, though, then I guess
leave it in the C file.
> >> + /* Update the first bus number */
> >> + if (pci->first_busno != hose->first_busno)
> >> + pci->first_busno = hose->first_busno;
> > This isn't part of the interface description in the header...
> [Minghuan] Yes. host->first_busno will be reassigned if defined
> PCI_REASSIGN_ALL_BUS.
> and I can not find a chance to update pci->first_busno. this will cause
> we can not
> read/write pci configuration space when the hose->first_busno is changed
> but pci->first_busno
> is not updated synchronously.
>
> the following code to check first_busno when access the configuration space.
>
> if (pci->indirect_type & INDIRECT_TYPE_NO_PCIE_LINK) {
> if (bus != pci->first_busno)
> return PCIBIOS_DEVICE_NOT_FOUND;
> ...
> }
>
> bus_no = (bus == pci->first_busno) ? pci->self_busno : bus;
>
> So I added the sentences to this function to fix the issue.
How was this handled in the old PPC code? I don't see anything similar.
Is it only an issue on ARM?
At the very least this side-effect should be mentioned in the function
interface documentation, but I'd rather see a less hacky solution.
> >> @@ -260,14 +259,6 @@ int mpc85xx_pci_err_probe(struct platform_device *op)
> >> /* we only need the error registers */
> >> r.start += 0xe00;
> >>
> >> - if (!devm_request_mem_region(&op->dev, r.start, resource_size(&r),
> >> - pdata->name)) {
> >> - printk(KERN_ERR "%s: Error while requesting mem region\n",
> >> - __func__);
> >> - res = -EBUSY;
> >> - goto err;
> >> - }
> > Why? If the relationship between the edac driver and the main pci
> > driver is changing, explain that.
> [Minghuan] Ok.
> The main pci driver used devm_ioremap_resource() to map regester space.
And it didn't before? It'd be nice if this "rework" patch could be
split into digestible chunks, each explained on its own.
> >> +config PCI_FSL
> >> + bool "Freescale PCI/PCIe controller"
> >> + depends on FSL_SOC_BOOKE || PPC_86xx
> > Needs help text.
> >
> > Make it clear that this is for 85xx/86xx/QorIQ/Layerscape, not all
> > Freescale chips with PCI/PCIe.
> [Minghuan] Ok. I will add help text.
> >> no_bridge:
> >> - iounmap(hose->private_data);
> >> - /* unmap cfg_data & cfg_addr separately if not on same page */
> >> - if (((unsigned long)hose->cfg_data & PAGE_MASK) !=
> >> - ((unsigned long)hose->cfg_addr & PAGE_MASK))
> >> - iounmap(hose->cfg_data);
> >> - iounmap(hose->cfg_addr);
> >> - pcibios_free_controller(hose);
> >> - return -ENODEV;
> >> + dev_info(&pdev->dev, "It works as EP mode\n");
> >> + return -EPERM;
> > This is a poorly phrased message. In any case, what does this change
> > have to do with making the PCI driver compatible with layerscape?
> [Minghuan] I can not quite understand what you mean.
> Should I remove the "dev_info(&pdev->dev, "It works as EP mode\n");"
> and not change ENODEV to EPERM?
> we do not really need this change.
> If the controller is in EP mode, we only need to return an error,
> because at this time
> 'hose' has not been created.
I don't have a strong opinion on the error code, but if you do change it
to EPERM it should be a separate patch with its own explanation.
"dev_info" seems inappropriate here -- either it indicates the caller
did something wrong, and thus should be dev_err() with a clearer message
(e.g. avoid words like "it" and unnecessary abbreviation), or this is
the normal point at which we detect that the hardware isn't configured
in host mode (in which case it should be at most dev_dbg).
-Scott
^ permalink raw reply
* Re: [PATCH 1/2][v3] powerpc/fsl-booke: Add initial T104x_QDS board support
From: Scott Wood @ 2013-09-24 23:25 UTC (permalink / raw)
To: Timur Tabi
Cc: Wood Scott-B07421, Jain Priyanka-B32167, Aggrwal Poonam-B10812,
linuxppc-dev@lists.ozlabs.org, Kushwaha Prabhakar-B32579
In-Reply-To: <523BBC15.8030607@tabi.org>
On Thu, 2013-09-19 at 22:08 -0500, Timur Tabi wrote:
> Kushwaha Prabhakar-B32579 wrote:
> > My primary object is to put base patch in Linux. once it done other things can be enabled one by one.
>
> Any features which are not enabled must be specified in the patch
> description. The patch says that the board supports DIU, but the code
> doesn't, so that's misleading.
>
> > Also, I am not familiar with DIU driver:(.
>
> I can help you with the DIU driver.
>
> > shall I remove the DIU node, and while adding support of DIU, all modification will be sent.
>
> I think it should be okay to leave the DIU node. I think the kernel
> will crash if you try to enable a DIU console (video= on the kernel
> command line), but I think it's okay to ignore that for the moment.
Sounds like a bug in the DIU driver. It should fail gracefully in the
absence of platform support.
-Scott
^ permalink raw reply
* Re: [PATCH V4 3/3] powerpc/85xx: Add TWR-P1025 board support
From: Scott Wood @ 2013-09-24 23:22 UTC (permalink / raw)
To: Xie Xiaobo; +Cc: linuxppc-dev, Michael Johnston
In-Reply-To: <1380019739-8196-3-git-send-email-X.Xie@freescale.com>
On Tue, 2013-09-24 at 18:48 +0800, Xie Xiaobo wrote:
> + partition@80000 {
> + /* 3.5 MB for Linux Kernel Image */
> + reg = <0x00080000 0x00380000>;
> + label = "NOR Linux Kernel Image";
> + };
Is this enough?
> + partition@400000 {
> + /* 58.75MB for JFFS2 based Root file System */
> + reg = <0x00400000 0x03ac0000>;
> + label = "NOR Root File System";
> + };
Don't specify jffs2.
> + /* CS2 for Display */
> + ssd1289@2,0 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "ssd1289";
> + reg = <0x2 0x0000 0x0002
> + 0x2 0x0002 0x0002>;
> + };
Node names should be generic. What does ssd1289 do? If this is
actually the display device, then it should be called "display@2,0".
How about a vendor prefix on that compatible? Why
#address-cells/#size-cells despite no child nodes? Where is a binding
that says what each of those two reg resources mean?
> diff --git a/arch/powerpc/boot/dts/p1025twr_32b.dts b/arch/powerpc/boot/dts/p1025twr_32b.dts
> new file mode 100644
> index 0000000..ccb173f
> --- /dev/null
> +++ b/arch/powerpc/boot/dts/p1025twr_32b.dts
> @@ -0,0 +1,135 @@
> +/*
> + * P1025 TWR Device Tree Source (32-bit address map)
> + *
> + * Copyright 2013 Freescale Semiconductor Inc.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are met:
> + * * Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * * Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + * * Neither the name of Freescale Semiconductor nor the
> + * names of its contributors may be used to endorse or promote products
> + * derived from this software without specific prior written permission.
> + *
> + *
> + * ALTERNATIVELY, this software may be distributed under the terms of the
> + * GNU General Public License ("GPL") as published by the Free Software
> + * Foundation, either version 2 of that License or (at your option) any
> + * later version.
> + *
> + * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS'' AND ANY
> + * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
> + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> + * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE FOR ANY
> + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
> + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
> + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +/include/ "fsl/p1021si-pre.dtsi"
> +/ {
> + model = "fsl,P1025";
> + compatible = "fsl,TWR-P1025";
> +
> + memory {
> + device_type = "memory";
> + };
> +
> + lbc: localbus@ffe05000 {
> + reg = <0 0xffe05000 0 0x1000>;
> +
> + /* NOR Flash and SSD1289 */
> + ranges = <0x0 0x0 0x0 0xec000000 0x04000000
> + 0x2 0x0 0x0 0xe0000000 0x00020000>;
> + };
> +
> + soc: soc@ffe00000 {
> + ranges = <0x0 0x0 0xffe00000 0x100000>;
> + };
> +
> + pci0: pcie@ffe09000 {
> + ranges = <0x2000000 0x0 0xa0000000 0 0xa0000000 0x0 0x20000000
> + 0x1000000 0x0 0x00000000 0 0xffc10000 0x0 0x10000>;
> + reg = <0 0xffe09000 0 0x1000>;
> + pcie@0 {
> + ranges = <0x2000000 0x0 0xa0000000
> + 0x2000000 0x0 0xa0000000
> + 0x0 0x20000000
> +
> + 0x1000000 0x0 0x0
> + 0x1000000 0x0 0x0
> + 0x0 0x100000>;
> + };
> + };
> +
> + pci1: pcie@ffe0a000 {
> + reg = <0 0xffe0a000 0 0x1000>;
> + ranges = <0x2000000 0x0 0x80000000 0 0x80000000 0x0 0x20000000
> + 0x1000000 0x0 0x00000000 0 0xffc00000 0x0 0x10000>;
> + pcie@0 {
> + ranges = <0x2000000 0x0 0x80000000
> + 0x2000000 0x0 0x80000000
> + 0x0 0x20000000
> +
> + 0x1000000 0x0 0x0
> + 0x1000000 0x0 0x0
> + 0x0 0x100000>;
> + };
> + };
> +
> + qe: qe@ffe80000 {
> + ranges = <0x0 0x0 0xffe80000 0x40000>;
> + reg = <0 0xffe80000 0 0x480>;
> + brg-frequency = <0>;
> + bus-frequency = <0>;
> + status = "disabled"; /* no firmware loaded */
> +
> + enet3: ucc@2000 {
> + device_type = "network";
> + compatible = "ucc_geth";
> + rx-clock-name = "clk12";
> + tx-clock-name = "clk9";
> + pio-handle = <&pio1>;
> + phy-handle = <&qe_phy0>;
> + phy-connection-type = "mii";
> + };
> +
> + mdio@2120 {
> + qe_phy0: ethernet-phy@18 {
> + interrupt-parent = <&mpic>;
> + interrupts = <4 1 0 0>;
> + reg = <0x18>;
> + device_type = "ethernet-phy";
> + };
> + qe_phy1: ethernet-phy@19 {
> + interrupt-parent = <&mpic>;
> + interrupts = <5 1 0 0>;
> + reg = <0x19>;
> + device_type = "ethernet-phy";
> + };
> + tbi-phy@11 {
> + reg = <0x11>;
> + device_type = "tbi-phy";
> + };
> + };
> +
> + enet4: ucc@2400 {
> + device_type = "network";
> + compatible = "ucc_geth";
> + rx-clock-name = "none";
> + tx-clock-name = "clk13";
> + pio-handle = <&pio2>;
> + phy-handle = <&qe_phy1>;
> + phy-connection-type = "rmii";
> + };
> + };
> +};
Don't duplicate all this just for 32/36 bit. Use a dtsi for (e.g.) the
contents of the QE node.
Is there a strong need to support both 32 and 36 bit in the first place?
> +/* ************************************************************************
> + *
> + * Setup the architecture
> + *
> + */
> +static void __init twr_p1025_setup_arch(void)
> +{
> +#ifdef CONFIG_QUICC_ENGINE
> + struct device_node *np;
> +#endif
> +
> + if (ppc_md.progress)
> + ppc_md.progress("twr_p1025_setup_arch()", 0);
> +
> + mpc85xx_smp_init();
> +
> + fsl_pci_assign_primary();
> +
> +#ifdef CONFIG_QUICC_ENGINE
> + mpc85xx_qe_init();
> +
> +#if defined(CONFIG_UCC_GETH) || defined(CONFIG_SERIAL_QE)
> + if (machine_is(twr_p1025)) {
> + struct ccsr_guts __iomem *guts;
> +
> + np = of_find_node_by_name(NULL, "global-utilities");
Look for it by compatible.
> + if (np) {
> + guts = of_iomap(np, 0);
> + if (!guts)
> + pr_err("twr_p1025: could not map"
> + "global utilities register\n");
Don't linewrap printed string constants (this is an exception to the
80-column rule).
> + else {
> + /* P1025 has pins muxed for QE and other functions. To
> + * enable QE UEC mode, we need to set bit QE0 for UCC1
> + * in Eth mode, QE0 and QE3 for UCC5 in Eth mode, QE9
> + * and QE12 for QE MII management signals in PMUXCR
> + * register.
> + */
> +
> + printk(KERN_INFO "P1025 pinmux configured for QE\n");
Bad indentation, and use pr_info() (or better, just remove it; it's
implied by the absence of the error on the other branch of the if).
> +
> + /* Set QE mux bits in PMUXCR */
> + setbits32(&guts->pmuxcr, MPC85xx_PMUXCR_QE(0) |
> + MPC85xx_PMUXCR_QE(3) |
> + MPC85xx_PMUXCR_QE(9) |
> + MPC85xx_PMUXCR_QE(12));
> + iounmap(guts);
> +
> +#if defined(CONFIG_SERIAL_QE)
> + /* On P1025TWR board, the UCC7 acted as UART port.
> + * However, The UCC7's CTS pin is low level in default,
> + * it will impact the transmission in full duplex
> + * communication. So disable the Flow control pin PA18.
> + * The UCC7 UART just can use RXD and TXD pins.
> + */
> + par_io_config_pin(0, 18, 0, 0, 0, 0);
> +#endif
Any reason not to do this unconditionally?
-Scott
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox