* Re: [v1] powerpc/mpc512x: silence build warning upon disabled DIU
From: Anatolij Gustschin @ 2013-10-10 16:09 UTC (permalink / raw)
To: Brian Norris; +Cc: Gerhard Sittig, linuxppc-dev
In-Reply-To: <20131009192931.GC23337@ld-irv-0074.broadcom.com>
Hi,
On Wed, 9 Oct 2013 12:29:31 -0700
Brian Norris <computersforpeace@gmail.com> wrote:
...
> > +#else
> > +void __init mpc512x_setup_diu(void) { /* EMPTY */ }
> > +void __init mpc512x_init_diu(void) { /* EMPTY */ }
> > #endif
> >
> > void __init mpc512x_init_IRQ(void)
>
> I see an alternative solution:
>
> Can't almost all of the code in mpc512x_shared.c be declared 'static'?
making mpc512x_setup_diu(), mpc512x_release_bootmem(),
mpc512x_valid_monitor_port() and void mpc512x_set_pixel_clock()
should be okay.
> Then, you can get the real benefit of IS_ENABLED() by removing the
>
> #if IS_ENABLED(CONFIG_FB_FSL_DIU)
>
> from around all the DIU code, and it will automatically be removed by
> the compiler when it is not used.
>
> I think the current patch is necessary for immediate use, and it can be
> sent to stable. But I might suggest a follow-up patch or 2 that makes
> the functions static and kills the #ifdef entirely.
Yes, we also have to remove CONFIG_FB_FSL_DIU ifdef in
arch/powerpc/sysdev/fsl_soc.h and building should work then.
Thanks,
Anatolij
^ permalink raw reply
* Re: [PATCH RFC 50/77] mlx5: Update MSI/MSI-X interrupts enablement code
From: Eli Cohen @ 2013-10-10 15:29 UTC (permalink / raw)
To: Alexander Gordeev
Cc: linux-mips, VMware, Inc., linux-nvme, linux-ide, linux-s390,
Andy King, linux-scsi, linux-rdma, x86, Ingo Molnar, linux-pci,
iss_storagedev, linux-driver, Tejun Heo, Bjorn Helgaas,
Dan Williams, Jon Mason, Solarflare linux maintainers, netdev,
linux-kernel, Ralf Baechle, e1000-devel, Martin Schwidefsky,
linux390, linuxppc-dev
In-Reply-To: <20131003194837.GA27636@dhcp-26-207.brq.redhat.com>
On Thu, Oct 03, 2013 at 09:48:39PM +0200, Alexander Gordeev wrote:
>
> pci_enable_msix() may fail, but it can not return a positive number.
>
That is true according to the current logic but the comment on top of
pci_enable_msix() still says:
"A return of < 0 indicates a failure. Or a return of > 0 indicates
that driver request is exceeding the number of irqs or MSI-X vectors
available"
So you're counting on an implementation that may change in the future.
I think leaving the code as it is now is safer.
^ permalink raw reply
* Re: Elbc device driver
From: Scott Wood @ 2013-10-10 15:14 UTC (permalink / raw)
To: Mercier Ivan; +Cc: linuxppc-dev
In-Reply-To: <CAMc2ieqXg1C6twTaqL9irJoLceSw-fFyTseifW5dmCmnXHWLCw@mail.gmail.com>
On Thu, 2013-10-10 at 16:54 +0200, Mercier Ivan wrote:
> Hi,
> I've got a KERN_INFO msg: "fsl-lbc a3p400.11: failed to get memory
> region" corresponding to my elbc device that I try to map.
> a3p400@3,0 {
> compatible = "fsl,elbc";
> reg = <0 0xe0000000 0x1000000>;
> };
The node name and unit address do not match the contents of the node.
The former describes a device that sits on the localbus, while the
latter has a compatible describes the localbus itself. I don't know
what that reg is, but it doesn't match the unit address.
> The corresponding law on ELBC is 0xe0000000 ->0xf0000000.
> I want to map my device on 0xe0000000 ->0xe8000000 as it has 16 adress bits.
>
> Now the controler registers are properly set,but how do I map the device?
Your eLBC node should look something like this (I'm assuming CCSR is at
0xffe000000, and that you actually meant 0x0e0000000 and not
0xfe0000000, though probably one of those assumptions is wrong):
localbus@ffe124000 {
compatible = "fsl,elbc", "simple-bus";
reg = <0xf 0xfe124000 0 0x1000>;
interrupts = <25 2 0 0>;
#address-cells = <2>;
#size-cells = <1>;
ranges = <3 0 0 0xe0000000 0x08000000>;
/* If at all possible, replace "a3p400" in the node name with
something generic */
a3p400@3,0 {
compatible = "something appropriate here";
reg = <3 0 0x08000000>;
};
};
Note that at the dts level, you should be including
arch/powerpc/boot/dts/fsl/p3041-post.dtsi and thus your board dts would
only need to supply reg, ranges, and the a3p400 node (see
arch/powerpc/boot/dts/p3041ds.dts for an example).
-Scott
^ permalink raw reply
* Re: Elbc device driver
From: Mercier Ivan @ 2013-10-10 14:54 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <1381271655.7979.290.camel@snotra.buserror.net>
Hi,
I've got a KERN_INFO msg: "fsl-lbc a3p400.11: failed to get memory
region" corresponding to my elbc device that I try to map.
a3p400@3,0 {
compatible = "fsl,elbc";
reg = <0 0xe0000000 0x1000000>;
};
The corresponding law on ELBC is 0xe0000000 ->0xf0000000.
I want to map my device on 0xe0000000 ->0xe8000000 as it has 16 adress bits.
Now the controler registers are properly set,but how do I map the device?
Thanks a lot
2013/10/9 Scott Wood <scottwood@freescale.com>:
> On Tue, 2013-10-08 at 16:06 +0200, Mercier Ivan wrote:
>> Hi,
>>
>> I'm working on a powerpc qoriq p3041 and trying to communicate with a
>> device by elbc bus in gpmc mode.
>>
>> I 've integrated CONFIG_FSL_LBC in Linux which provide the basic functions.
>>
>> Now I'm wondering how can I do read and write operations on the
>> bus.Where is mapped my device?
>
> You'll need to use ioremap() or of_iomap() to map it.
>
>> Should I code .read and .write driver functions?How can I start?
>>
>> How integrates my device in the device tree?
>
> See Documentation/devicetree/bindings/powerpc/fsl/lbc.txt and examples
> such as "board-control" in various device trees.
>
> -Scott
>
>
>
^ permalink raw reply
* Re: Gianfar driver crashes in Kernel v3.10
From: Claudiu Manoil @ 2013-10-10 11:07 UTC (permalink / raw)
To: Thomas Hühn; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <E0CAE3C8-5883-4D20-8A95-CCC88B4A811C@net.t-labs.tu-berlin.de>
On 10/4/2013 3:28 PM, Thomas H=FChn wrote:
>
> [code]
> [ 2671.841927] Oops: Exception in kernel mode, sig: 5 [#1]
> [ 2671.847141] Freescale P1014
> [ 2671.849925] Modules linked in: ath9k pppoe ppp_async iptable_nat
> ath9k_common pppox p
> e xt_tcpudp xt_tcpmss xt_string xt_statistic xt_state xt_recent xt_quot=
a
> xt_pkttype xt_o
> mark xt_connbytes xt_comment xt_addrtype xt_TCPMSS xt_REDIRECT xt_NETMA=
P
> xt_LOG xt_IPMAR
> ms_datafab ums_cypress ums_alauda slhc nf_nat_tftp nf_nat_snmp_basic
> nf_nat_sip nf_nat_r
> ntrack_sip nf_conntrack_rtsp nf_conntrack_proto_gre nf_conntrack_irc
> nf_conntrack_h323 n
> compat_xtables compat ath sch_teql sch_tbf sch_sfq sch_red sch_prio
> sch_htb sch_gred sc
> skbedit act_mirred em_u32 cls_u32 cls_tcindex cls_flow cls_route cls_fw
> sch_hfsc sch_ing
> r usb_storage leds_gpio ohci_hcd ehci_platform ehci_hcd sd_mod scsi_mod
> fsl_mph_dr_of gp
> [ 2671.988946] CPU: 0 PID: 5209 Comm: iftop Not tainted 3.10.13 #2
> [ 2671.994859] task: c4b22220 ti: c7ff8000 task.ti: c477e000
> [ 2672.000250] NIP: c018c7a0 LR: c018c794 CTR: c000b070
> [ 2672.005206] REGS: c7ff9f10 TRAP: 3202 Not tainted (3.10.13)
> [ 2672.011028] MSR: 00029000 <CE,EE,ME> CR: 48000024 XER: 20000000
> [ 2672.017125]
> GPR00: 000000ff c477fde0 c4b22220 00000000 00000000 000000ff 00000000
> 70000000
> GPR08: ffffffff 00000008 00000000 ffffffff 00000046 10022248 00000000
> 00000008
> GPR16: c781b3c0 c781b3c0 000000ff 00000000 00000001 0000021c 00000086
> fffff800
> GPR24: c7980300 00000000 00000001 00000040 00000003 c4b33000 00000000
> 00000001
> [ 2672.046832] NIP [c018c7a0] gfar_poll+0x424/0x520
> [ 2672.051442] LR [c018c794] gfar_poll+0x418/0x520
> [ 2672.055962] Call Trace:
> [ 2672.058402] [c477fde0] [c018c674] gfar_poll+0x2f8/0x520 (unreliable)
> [ 2672.064762] [c477fe80] [c01b0ce8] net_rx_action+0x6c/0x158
> [ 2672.070249] [c477feb0] [c0027dc4] __do_softirq+0xbc/0x16c
> [ 2672.075642] [c477ff00] [c0027f7c] irq_exit+0x4c/0x68
> [ 2672.080604] [c477ff10] [c00041f8] do_IRQ+0xf4/0x10c
> [ 2672.085478] [c477ff40] [c000ca3c] ret_from_except+0x0/0x18
> [ 2672.090991] --- Exception: 501 at 0x48083c28
> [ 2672.090991] LR =3D 0x48083bf8
> [ 2672.098378] Instruction dump:
> [ 2672.101338] 7f8f2040 419cfcc4 80900000 38a00000 8061004c 7e118378
> 81c10050 7ffafb78
> [ 2672.109092] 4bf9eaa1 83810034 7c7e1b78 8361003c <83210038> 83a1004c
> 48000060 41a2004c
> [ 2672.117021] ---[ end trace 565fb54528d305fa ]---
> [ 2672.121628]
> [ 2673.103130] Kernel panic - not syncing: Fatal exception in interrupt
> [ 2673.109474] Rebooting in 3 seconds..
>
> U-Boot 2010.12-svn15934 (Dec 11 2012 - 16:23:49)
> [/code]
>
Hi,
Does this show up on a half duplex (100Mb/s) link?
Could you provide following for the gianfar interface, on your setup:
# ethtool ethX
and
# ethtool -d ethX | grep 500
Is there any other indication before this Oops? Like a tx timeout WARN?
Thanks,
Claudiu
^ permalink raw reply
* Re: [PATCH 2/3] powerpc/scom: Replace debugfs interface with cleaner sysfs one
From: Benjamin Herrenschmidt @ 2013-10-10 10:16 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev
In-Reply-To: <20131010100640.GA9906@iris.ozlabs.ibm.com>
On Thu, 2013-10-10 at 21:06 +1100, Paul Mackerras wrote:
> On Thu, Oct 10, 2013 at 07:18:35PM +1100, Benjamin Herrenschmidt wrote:
> > The debugfs interface was essentially unused, and racy for anything
> > other than manual use by a developer. This provides a more useful
> > sysfs based one which can be used by programs without racing with
> > each other essentially by providing a file to read/write with an
> > offset being the scom address << 8.
>
> Don't you mean address << 3 (or address * 8)?
Yes, typo in the comment, the code is fine. I was tired :)
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH RFC 00/77] Re-design MSI/MSI-X interrupts enablement pattern
From: Alexander Gordeev @ 2013-10-10 10:17 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linux-mips, VMware, Inc., linux-pci, linux-nvme, linux-ide,
H. Peter Anvin, linux-s390, Andy King, linux-scsi, linux-rdma,
x86, Ingo Molnar, iss_storagedev, linux-driver, Tejun Heo,
Bjorn Helgaas, Dan Williams, Jon Mason,
Solarflare linux maintainers, netdev, linux-kernel, Ralf Baechle,
e1000-devel, Martin Schwidefsky, linux390, linuxppc-dev
In-Reply-To: <1381292648.645.259.camel@pasglop>
On Wed, Oct 09, 2013 at 03:24:08PM +1100, Benjamin Herrenschmidt wrote:
> On Tue, 2013-10-08 at 20:55 -0700, H. Peter Anvin wrote:
> > Why not add a minimum number to pci_enable_msix(), i.e.:
> >
> > pci_enable_msix(pdev, msix_entries, nvec, minvec)
> >
> > ... which means "nvec" is the number of interrupts *requested*, and
> > "minvec" is the minimum acceptable number (otherwise fail).
>
> Which is exactly what Ben (the other Ben :-) suggested and that I
> supports...
Ok, this suggestion sounded in one or another form by several people.
What about name it pcim_enable_msix_range() and wrap in couple more
helpers to complete an API:
int pcim_enable_msix_range(pdev, msix_entries, nvec, minvec);
<0 - error code
>0 - number of MSIs allocated, where minvec >= result <= nvec
int pcim_enable_msix(pdev, msix_entries, nvec);
<0 - error code
>0 - number of MSIs allocated, where 1 >= result <= nvec
int pcim_enable_msix_exact(pdev, msix_entries, nvec);
<0 - error code
>0 - number of MSIs allocated, where result == nvec
The latter's return value seems odd, but I can not help to make
it consistent with the first two.
(Sorry if you see this message twice - my MUA seems struggle with one of CC).
> Cheers,
> Ben.
>
>
--
Regards,
Alexander Gordeev
agordeev@redhat.com
^ permalink raw reply
* Re: [PATCH 2/3] powerpc/scom: Replace debugfs interface with cleaner sysfs one
From: Paul Mackerras @ 2013-10-10 10:06 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev
In-Reply-To: <1381393115.4330.39.camel@pasglop>
On Thu, Oct 10, 2013 at 07:18:35PM +1100, Benjamin Herrenschmidt wrote:
> The debugfs interface was essentially unused, and racy for anything
> other than manual use by a developer. This provides a more useful
> sysfs based one which can be used by programs without racing with
> each other essentially by providing a file to read/write with an
> offset being the scom address << 8.
Don't you mean address << 3 (or address * 8)?
Paul.
^ permalink raw reply
* Re: [PATCH v5] powerpc/mpc85xx: Update the clock nodes in device tree
From: Mark Rutland @ 2013-10-10 10:03 UTC (permalink / raw)
To: Yuantian.Tang@freescale.com
Cc: devicetree@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1381300704-4238-1-git-send-email-Yuantian.Tang@freescale.com>
On Wed, Oct 09, 2013 at 07:38:24AM +0100, Yuantian.Tang@freescale.com wrote:
> From: Tang Yuantian <yuantian.tang@freescale.com>
>
> The following SoCs will be affected: p2041, p3041, p4080,
> p5020, p5040, b4420, b4860, t4240
>
> Signed-off-by: Tang Yuantian <Yuantian.Tang@freescale.com>
> Signed-off-by: Li Yang <leoli@freescale.com>
> ---
> v5:
> - refine the binding document
> - update the compatible string
> v4:
> - add binding document
> - update compatible string
> - update the reg property
> v3:
> - fix typo
> v2:
> - add t4240, b4420, b4860 support
> - remove pll/4 clock from p2041, p3041 and p5020 board
>
> .../devicetree/bindings/clock/corenet-clock.txt | 111 ++++++++++++++++++++
> arch/powerpc/boot/dts/fsl/b4420si-post.dtsi | 35 +++++++
> arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi | 2 +
> arch/powerpc/boot/dts/fsl/b4860si-post.dtsi | 35 +++++++
> arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi | 4 +
> arch/powerpc/boot/dts/fsl/p2041si-post.dtsi | 60 +++++++++++
> arch/powerpc/boot/dts/fsl/p2041si-pre.dtsi | 4 +
> arch/powerpc/boot/dts/fsl/p3041si-post.dtsi | 60 +++++++++++
> arch/powerpc/boot/dts/fsl/p3041si-pre.dtsi | 4 +
> arch/powerpc/boot/dts/fsl/p4080si-post.dtsi | 112 +++++++++++++++++++++
> arch/powerpc/boot/dts/fsl/p4080si-pre.dtsi | 8 ++
> arch/powerpc/boot/dts/fsl/p5020si-post.dtsi | 42 ++++++++
> arch/powerpc/boot/dts/fsl/p5020si-pre.dtsi | 2 +
> arch/powerpc/boot/dts/fsl/p5040si-post.dtsi | 60 +++++++++++
> arch/powerpc/boot/dts/fsl/p5040si-pre.dtsi | 4 +
> arch/powerpc/boot/dts/fsl/t4240si-post.dtsi | 85 ++++++++++++++++
> arch/powerpc/boot/dts/fsl/t4240si-pre.dtsi | 12 +++
> 17 files changed, 640 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/corenet-clock.txt
>
> diff --git a/Documentation/devicetree/bindings/clock/corenet-clock.txt b/Documentation/devicetree/bindings/clock/corenet-clock.txt
> new file mode 100644
> index 0000000..8efc62d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/corenet-clock.txt
> @@ -0,0 +1,111 @@
> +* Clock Block on Freescale CoreNet Platforms
> +
> +Freescale CoreNet chips take primary clocking input from the external
> +SYSCLK signal. The SYSCLK input (frequency) is multiplied using
> +multiple phase locked loops (PLL) to create a variety of frequencies
> +which can then be passed to a variety of internal logic, including
> +cores and peripheral IP blocks.
> +Please refer to the Reference Manual for details.
> +
> +1. Clock Block Binding
> +
> +Required properties:
> +- compatible: Should include one or more of the following:
> + - "fsl,<chip>-clockgen": for chip specific clock block
> + - "fsl,qoriq-clockgen-[1,2].x": for chassis 1.x and 2.x clock
While I can see that "fsl,<chip>-clockgen" might have a large set of
strings that we may never deal with in th kernel, I'd prefer that the
basic strings (i.e. all the "fsl,qoriq-clockgen-[1,2].x" variants) were
listed explicitly here.
Given they only seem to be "fsl,qoriq-clockgen-1.0" and
"fsl,qoriq-clockgen-2.0" this shouldn't be too difficult to list and
describe.
> +- reg: Offset and length of the clock register set
> +- clock-frequency: Indicates input clock frequency of clock block.
> + Will be set by u-boot
Why does the fact this is set by u-boot matter to the binding?
> +
> +Recommended properties:
> +- #ddress-cells: Specifies the number of cells used to represent
> + physical base addresses. Must be present if the device has
> + sub-nodes and set to 1 if present
Typo: #address-cells
In the example it looks like the address cells of child nodes are
offsets within the unit, rather than absolute physical addresses. Could
the code not treat them as absolute addresses? Then we'd only need to
document that #address-cells, #size-cells and ranges must be present and
have values suitable for mapping child nodes into the address space of
the parent.
> +- #size-cells: Specifies the number of cells used to represent
> + the size of an address. Must be present if the device has
> + sub-nodes and set to 1 if present
It's not really the size of an address, it's the size of a region
identified by a reg entry.
I think this can be simplified by my suggestion above.
> +
> +2. Clock Provider/Consumer Binding
> +
> +Most of the binding are from the common clock binding[1].
> + [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> +
> +Required properties:
> +- compatible : Should include one or more of the following:
> + - "fsl,qoriq-core-pll-[1,2].x": Indicates a core PLL clock device
> + - "fsl,qoriq-core-mux-[1,2].x": Indicates a core multiplexer clock
> + device; divided from the core PLL clock
As above, I'd prefer a complete list of the basic strings we expect.
> + - "fixed-clock": From common clock binding; indicates output clock
> + of oscillator
> + - "fsl,qoriq-sysclk-[1,2].x": Indicates input system clock
Here too.
> +- #clock-cells: From common clock binding; indicates the number of
> + output clock. 0 is for one output clock; 1 for more than one clock
If a clock source has multiple outputs, what those outputs are and what
values in clock-cells they correspond to should be described here.
> +
> +Recommended properties:
> +- clocks: Should be the phandle of input parent clock
> +- clock-names: From common clock binding, indicates the clock name
That description's a bit opaque.
What's the name of the clock input on these units? That's what
clock-names should contain, and that should be documented.
Do we not _always_ need the parent clock?
If we have a clock do we need a clock-names entry for it?
> +- clock-output-names: From common clock binding, indicates the names of
> + output clocks
> +- reg: Should be the offset and length of clock block base address.
> + The length should be 4.
> +
> +Example for clock block and clock provider:
> +/ {
> + clockgen: global-utilities@e1000 {
> + compatible = "fsl,p5020-clockgen", "fsl,qoriq-clockgen-1.0";
> + reg = <0xe1000 0x1000>;
> + clock-frequency = <0>;
That looks odd.
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + sysclk: sysclk {
> + #clock-cells = <0>;
> + compatible = "fsl,qoriq-sysclk-1.0", "fixed-clock";
We didn't mention in the binding that "fsl,qoriq-sysclk-1.0" was
compatible with "fixed-clock" and should have "fixed-clock" in the
compatible list...
> + clock-output-names = "sysclk";
> + }
> +
> + pll0: pll0@800 {
> + #clock-cells = <1>;
> + reg = <0x800 0x4>;
> + compatible = "fsl,qoriq-core-pll-1.0";
> + clocks = <&sysclk>;
> + clock-output-names = "pll0", "pll0-div2";
> + };
> +
> + pll1: pll1@820 {
> + #clock-cells = <1>;
> + reg = <0x820 0x4>;
> + compatible = "fsl,qoriq-core-pll-1.0";
> + clocks = <&sysclk>;
> + clock-output-names = "pll1", "pll1-div2";
> + };
> +
> + mux0: mux0@0 {
> + #clock-cells = <0>;
> + reg = <0x0 0x4>;
> + compatible = "fsl,qoriq-core-mux-1.0";
> + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>;
> + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1";
> + clock-output-names = "cmux0";
> + };
> +
> + mux1: mux1@20 {
> + #clock-cells = <0>;
> + reg = <0x20 0x4>;
> + compatible = "fsl,qoriq-core-mux-1.0";
> + clocks = <&pll0 0>, <&pll0 1>, <&pll1 0>, <&pll1 1>;
> + clock-names = "pll0_0", "pll0_1", "pll1_0", "pll1_1";
> + clock-output-names = "cmux1";
How does the mux choose which input clock to use at a point in time?
Cheers,
Mark.
^ permalink raw reply
* Loading and executing non-Linux kernel outside Linux memory.
From: Sumesh Kaana @ 2013-10-10 8:47 UTC (permalink / raw)
To: linuxppc-dev@lists.ozlabs.org
[-- Attachment #1: Type: text/plain, Size: 369 bytes --]
Hi,
I'm running Linux on a PowerPC-44x based board with 256 Megabytes of RAM at 0x5000.0000 - 0x6000.0000.Upon an error interrupt, I should load my recovery module(non-Linux kernel) at 0x4000.0000 and jump the execution to this.
Any suggestions for this implementation?
Would kexec allow me to load an image ouside linux's memory?
Thanks,Sumesh.
[-- Attachment #2: Type: text/html, Size: 753 bytes --]
^ permalink raw reply
* Re: [PATCH] powerpc, perf: Configure BHRB filter before enabling PMU interrupts
From: Anshuman Khandual @ 2013-10-10 8:50 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, mikey
In-Reply-To: <20131009060321.GB28160@concordia>
On 10/09/2013 11:33 AM, Michael Ellerman wrote:
> On Wed, Oct 09, 2013 at 10:16:32AM +0530, Anshuman Khandual wrote:
>> On 10/09/2013 06:51 AM, Michael Ellerman wrote:
>>> On Tue, Oct 08, 2013 at 12:51:18PM +0530, Anshuman Khandual wrote:
>>>> On 10/08/2013 09:51 AM, Michael Ellerman wrote:
>>>>> On Mon, Oct 07, 2013 at 10:00:26AM +0530, Anshuman Khandual wrote:
>>>>>> Right now the `config_bhrb` PMU specific call happens after write_mmcr0
>>>>>> which actually enables the PMU for event counting and interrupt. So
>>>>>> there is a small window of time where the PMU and BHRB runs without the
>>>>>> required HW branch filter (if any) enabled in BHRB. This can cause some
>>>>>> of the branch samples to be collected through BHRB without any filter
>>>>>> being applied and hence affecting the correctness of the results. This
>>>>>> patch moves the BHRB config function call before enabling the interrupts.
>>>>>
>>>>> Patch looks good.
>>>>>
>>>>> But it reminds me I have an item in my TODO list:
>>>>> - "Why can't config_bhrb() be done in compute_mmcr()" ?
>>>>>
>>>>
>>>> compute_mmcr() function deals with generic MMCR* configs for normal PMU
>>>> events. Even if BHRB config touches MMCRA register, it's configuration
>>>> does not interfere with the PMU config for general events. So its best
>>>> to keep them separate.
>>>
>>> I'm unconvinced. If they'd been together to begin with this bug never
>>> would have happened.
>>
>> This is an ordering of configuration problem. Putting them together in the
>> same function does not rule out the chances of this ordering problem. Could
>> you please kindly explain how this could have been avoided ?
>
> The existing code already makes sure to write MMCRA before MMCR0.
>
Thats not true. One example being here at power_pmu_enable function.
write_mmcr0(cpuhw, mmcr0);
/*
* Enable instruction sampling if necessary
*/
if (cpuhw->mmcr[2] & MMCRA_SAMPLE_ENABLE) {
mb();
mtspr(SPRN_MMCRA, cpuhw->mmcr[2]);
}
Even I think this is not right. Instruction sampling should have been
enabled before we enable PMU interrupts. Else there is a small window
of time where we could have the PMU enabled with events (which requires
sampling) without the sampling itself being enabled in MMCRA.
The only dependency BHRB and generic events have with each other is that
they both are ready for action once the PMU interrupt has been enabled
with MMCR0_PMXE bit.
Regards
Anshuman
^ permalink raw reply
* [PATCH 3/3] powerpc/powernv: Add support for indirect XSCOM via sysfs
From: Benjamin Herrenschmidt @ 2013-10-10 8:19 UTC (permalink / raw)
To: linuxppc-dev
Indirect XSCOM addresses normally have the top bit set (of the 64-bit
address). This doesn't work via the normal sysfs interface, so we use
a different encoding, which we need to convert before calling OPAL.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
arch/powerpc/platforms/powernv/opal-xscom.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/arch/powerpc/platforms/powernv/opal-xscom.c b/arch/powerpc/platforms/powernv/opal-xscom.c
index 09a90d8..23e52f3 100644
--- a/arch/powerpc/platforms/powernv/opal-xscom.c
+++ b/arch/powerpc/platforms/powernv/opal-xscom.c
@@ -71,11 +71,33 @@ static int opal_xscom_err_xlate(int64_t rc)
}
}
+static u64 opal_scom_unmangle(u64 reg)
+{
+ /*
+ * XSCOM indirect addresses have the top bit set. Additionally
+ * the reset of the top 3 nibbles is always 0.
+ *
+ * Because the sysfs interface uses signed offsets and shifts
+ * the address left by 3, we basically cannot use the top 4 bits
+ * of the 64-bit address, and thus cannot use the indirect bit.
+ *
+ * To deal with that, we support the indirect bit being in bit
+ * 4 (IBM notation) instead of bit 0 in this API, we do the
+ * conversion here. To leave room for further xscom address
+ * expansion, we only clear out the top byte
+ *
+ */
+ if (reg & (1ull << 59))
+ reg = (reg & ~(0xffull << 56)) | (1ull << 63);
+ return reg;
+}
+
static int opal_scom_read(scom_map_t map, u64 reg, u64 *value)
{
struct opal_scom_map *m = map;
int64_t rc;
+ reg = opal_scom_unmangle(reg);
rc = opal_xscom_read(m->chip, m->addr + reg, (uint64_t *)__pa(value));
return opal_xscom_err_xlate(rc);
}
@@ -85,6 +107,7 @@ static int opal_scom_write(scom_map_t map, u64 reg, u64 value)
struct opal_scom_map *m = map;
int64_t rc;
+ reg = opal_scom_unmangle(reg);
rc = opal_xscom_write(m->chip, m->addr + reg, value);
return opal_xscom_err_xlate(rc);
}
^ permalink raw reply related
* [PATCH 2/3] powerpc/scom: Replace debugfs interface with cleaner sysfs one
From: Benjamin Herrenschmidt @ 2013-10-10 8:18 UTC (permalink / raw)
To: linuxppc-dev
The debugfs interface was essentially unused, and racy for anything
other than manual use by a developer. This provides a more useful
sysfs based one which can be used by programs without racing with
each other essentially by providing a file to read/write with an
offset being the scom address << 8.
It requires 8 bytes aligned and multiple of 8 bytes sized accesses
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
arch/powerpc/configs/chroma_defconfig | 2 +-
arch/powerpc/sysdev/Kconfig | 6 +-
arch/powerpc/sysdev/scom.c | 210 +++++++++++++++++++++-------------
3 files changed, 135 insertions(+), 83 deletions(-)
diff --git a/arch/powerpc/configs/chroma_defconfig b/arch/powerpc/configs/chroma_defconfig
index 4f35fc4..2891464 100644
--- a/arch/powerpc/configs/chroma_defconfig
+++ b/arch/powerpc/configs/chroma_defconfig
@@ -42,7 +42,7 @@ CONFIG_MODULE_UNLOAD=y
CONFIG_MODULE_FORCE_UNLOAD=y
CONFIG_MODVERSIONS=y
CONFIG_MODULE_SRCVERSION_ALL=y
-CONFIG_SCOM_DEBUGFS=y
+CONFIG_SCOM_SYSFS=y
CONFIG_PPC_A2_DD2=y
CONFIG_KVM_GUEST=y
CONFIG_NO_HZ=y
diff --git a/arch/powerpc/sysdev/Kconfig b/arch/powerpc/sysdev/Kconfig
index 13ec968..30c26bb 100644
--- a/arch/powerpc/sysdev/Kconfig
+++ b/arch/powerpc/sysdev/Kconfig
@@ -26,9 +26,9 @@ source "arch/powerpc/sysdev/xics/Kconfig"
config PPC_SCOM
bool
-config SCOM_DEBUGFS
- bool "Expose SCOM controllers via debugfs"
- depends on PPC_SCOM && DEBUG_FS
+config SCOM_SYSFS
+ bool "Expose SCOM controllers via sysfs"
+ depends on PPC_SCOM
default n
config GE_FPGA
diff --git a/arch/powerpc/sysdev/scom.c b/arch/powerpc/sysdev/scom.c
index 3963d99..72eda2d 100644
--- a/arch/powerpc/sysdev/scom.c
+++ b/arch/powerpc/sysdev/scom.c
@@ -19,9 +19,10 @@
*/
#include <linux/kernel.h>
-#include <linux/debugfs.h>
#include <linux/slab.h>
#include <linux/export.h>
+#include <linux/kobject.h>
+#include <linux/sysfs.h>
#include <asm/debug.h>
#include <asm/prom.h>
#include <asm/scom.h>
@@ -95,116 +96,167 @@ scom_map_t scom_map_device(struct device_node *dev, int index)
}
EXPORT_SYMBOL_GPL(scom_map_device);
-#ifdef CONFIG_SCOM_DEBUGFS
-struct scom_debug_entry {
- struct device_node *dn;
- unsigned long addr;
- scom_map_t map;
- spinlock_t lock;
- char name[8];
- struct debugfs_blob_wrapper blob;
+#ifdef CONFIG_SCOM_SYSFS
+
+struct scom_chip_dir {
+ struct kobject kobj;
+ struct device_node *devnode;
};
-static int scom_addr_set(void *data, u64 val)
+static struct scom_chip_dir *kobj_to_scom_chip_dir(struct kobject *k)
{
- struct scom_debug_entry *ent = data;
-
- ent->addr = 0;
- scom_unmap(ent->map);
-
- ent->map = scom_map(ent->dn, val, 1);
- if (scom_map_ok(ent->map))
- ent->addr = val;
- else
- return -EFAULT;
-
- return 0;
+ return container_of(k, struct scom_chip_dir, kobj);
}
-static int scom_addr_get(void *data, u64 *val)
+static ssize_t scom_devspec_show(struct kobject *k, struct kobj_attribute *attr,
+ char *buf)
{
- struct scom_debug_entry *ent = data;
- *val = ent->addr;
- return 0;
-}
-DEFINE_SIMPLE_ATTRIBUTE(scom_addr_fops, scom_addr_get, scom_addr_set,
- "0x%llx\n");
+ struct scom_chip_dir *dir = kobj_to_scom_chip_dir(k);
-static int scom_val_set(void *data, u64 val)
-{
- struct scom_debug_entry *ent = data;
+ return sprintf(buf, "%s", dir->devnode->full_name);
+}
- if (!scom_map_ok(ent->map))
- return -EFAULT;
+static struct kobj_attribute scom_devspec_attr =
+ __ATTR(devspec, S_IRUGO, scom_devspec_show, NULL);
- scom_write(ent->map, 0, val);
+static struct attribute *scom_dir_default_attrs[] = {
+ &scom_devspec_attr.attr,
+ NULL,
+};
- return 0;
+static void scom_dir_release(struct kobject *kobj)
+{
+ struct scom_chip_dir *dir = kobj_to_scom_chip_dir(kobj);
+ kfree(dir);
}
-static int scom_val_get(void *data, u64 *val)
-{
- struct scom_debug_entry *ent = data;
+static struct kobj_type scom_dir_type = {
+ .release = scom_dir_release,
+ .sysfs_ops = &kobj_sysfs_ops,
+ .default_attrs = scom_dir_default_attrs,
+};
- if (!scom_map_ok(ent->map))
- return -EFAULT;
+static ssize_t scom_sysfs_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr,
+ char *buf, loff_t off, size_t count)
+{
+ struct scom_chip_dir *dir = kobj_to_scom_chip_dir(kobj);
+ u64 reg, reg_cnt, *buf64;
+ scom_map_t map;
+ int rc;
+
+ if (off & 7 || count & 7)
+ return -EINVAL;
+ reg = off >> 3;
+ reg_cnt = count >> 3;
+
+ map = scom_map(dir->devnode, reg, reg_cnt);
+ if (!scom_map_ok(map))
+ return -EINVAL;
+
+ buf64 = (u64 *)buf;
+ for (reg = 0; reg < reg_cnt; reg++) {
+ rc = scom_read(map, reg, buf64++);
+ if (rc) {
+ count = rc;
+ break;
+ }
+ }
+ scom_unmap(map);
+ return count;
+}
- return scom_read(ent->map, 0, val);
+static ssize_t scom_sysfs_write(struct file* filp, struct kobject *kobj,
+ struct bin_attribute *bin_attr,
+ char *buf, loff_t off, size_t count)
+{
+ struct scom_chip_dir *dir = kobj_to_scom_chip_dir(kobj);
+ u64 reg, reg_cnt, *buf64;
+ scom_map_t map;
+ int rc;
+
+ if (off & 7 || count & 7)
+ return -EINVAL;
+ reg = off >> 3;
+ reg_cnt = count >> 3;
+
+ map = scom_map(dir->devnode, reg, reg_cnt);
+ if (!scom_map_ok(map))
+ return -EINVAL;
+
+ buf64 = (u64 *)buf;
+ for (reg = 0; reg < reg_cnt; reg++) {
+ rc = scom_write(map, reg, *(buf64++));
+ if (rc) {
+ count = rc;
+ break;
+ }
+ }
+ scom_unmap(map);
+ return count;
}
-DEFINE_SIMPLE_ATTRIBUTE(scom_val_fops, scom_val_get, scom_val_set,
- "0x%llx\n");
-static int scom_debug_init_one(struct dentry *root, struct device_node *dn,
- int i)
+static struct bin_attribute scom_access_attr = {
+ .attr = {
+ .name = "access",
+ .mode = S_IRUSR | S_IWUSR,
+ },
+ .size = 0,
+ .read = scom_sysfs_read,
+ .write = scom_sysfs_write,
+};
+
+static int scom_sysfs_init_chip(struct kobject *root, struct device_node *dn,
+ int index)
{
- struct scom_debug_entry *ent;
- struct dentry *dir;
+ struct scom_chip_dir *dir;
+ int rc;
- ent = kzalloc(sizeof(*ent), GFP_KERNEL);
- if (!ent)
+ dir = kzalloc(sizeof(*dir), GFP_KERNEL);
+ if (!dir)
return -ENOMEM;
-
- ent->dn = of_node_get(dn);
- ent->map = SCOM_MAP_INVALID;
- spin_lock_init(&ent->lock);
- snprintf(ent->name, 8, "scom%d", i);
- ent->blob.data = (void*) dn->full_name;
- ent->blob.size = strlen(dn->full_name);
-
- dir = debugfs_create_dir(ent->name, root);
- if (!dir) {
- of_node_put(dn);
- kfree(ent);
- return -1;
+ dir->devnode = of_node_get(dn);
+
+ rc = kobject_init_and_add(&dir->kobj, &scom_dir_type,
+ root, "%08x", index);
+ if (rc) {
+ pr_err("scom: Failed to create kobj for %s, err %d\n",
+ dn->full_name, rc);
+ return rc;
}
- debugfs_create_file("addr", 0600, dir, ent, &scom_addr_fops);
- debugfs_create_file("value", 0600, dir, ent, &scom_val_fops);
- debugfs_create_blob("devspec", 0400, dir, &ent->blob);
+ rc = sysfs_create_bin_file(&dir->kobj, &scom_access_attr);
+ if (rc) {
+ pr_err("scom: Failed to create access file for %s, err %d\n",
+ dn->full_name, rc);
+ return rc;
+ }
return 0;
}
-static int scom_debug_init(void)
+static int scom_sysfs_init(void)
{
struct device_node *dn;
- struct dentry *root;
- int i, rc;
+ struct kobject *root = NULL;
+ int i;
- root = debugfs_create_dir("scom", powerpc_debugfs_root);
- if (!root)
- return -1;
-
- i = rc = 0;
+ i = 0;
for_each_node_with_property(dn, "scom-controller") {
int id = of_get_ibm_chip_id(dn);
if (id == -1)
id = i;
- rc |= scom_debug_init_one(root, dn, id);
+ if (!root)
+ root = kobject_create_and_add("scom", firmware_kobj);
+ if (!root) {
+ pr_err("scom: Failed to create sysfs root\n");
+ return -ENOMEM;
+ }
+ scom_sysfs_init_chip(root, dn, id);
i++;
}
-
- return rc;
+ return 0;
}
-device_initcall(scom_debug_init);
-#endif /* CONFIG_SCOM_DEBUGFS */
+subsys_initcall(scom_sysfs_init);
+
+#endif /* CONFIG_SCOM_SYSFS */
^ permalink raw reply related
* [PATCH 1/3] powerpc/scom: Enable 64-bit addresses
From: Benjamin Herrenschmidt @ 2013-10-10 8:18 UTC (permalink / raw)
To: linuxppc-dev
On P8, XSCOM addresses has a special "indirect" form that
requires more than 32-bits, so let's use u64 everywhere in
the code instead of u32.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
This applies on top of my previously posted scom series
arch/powerpc/include/asm/scom.h | 8 ++++----
arch/powerpc/platforms/powernv/opal-xscom.c | 6 +++---
arch/powerpc/platforms/wsp/scom_wsp.c | 4 ++--
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/include/asm/scom.h b/arch/powerpc/include/asm/scom.h
index 07dcdcf..f5cde45 100644
--- a/arch/powerpc/include/asm/scom.h
+++ b/arch/powerpc/include/asm/scom.h
@@ -54,8 +54,8 @@ struct scom_controller {
scom_map_t (*map)(struct device_node *ctrl_dev, u64 reg, u64 count);
void (*unmap)(scom_map_t map);
- int (*read)(scom_map_t map, u32 reg, u64 *value);
- int (*write)(scom_map_t map, u32 reg, u64 value);
+ int (*read)(scom_map_t map, u64 reg, u64 *value);
+ int (*write)(scom_map_t map, u64 reg, u64 value);
};
extern const struct scom_controller *scom_controller;
@@ -137,7 +137,7 @@ static inline void scom_unmap(scom_map_t map)
*
* Returns 0 (success) or a negative error code
*/
-static inline int scom_read(scom_map_t map, u32 reg, u64 *value)
+static inline int scom_read(scom_map_t map, u64 reg, u64 *value)
{
int rc;
@@ -155,7 +155,7 @@ static inline int scom_read(scom_map_t map, u32 reg, u64 *value)
*
* Returns 0 (success) or a negative error code
*/
-static inline int scom_write(scom_map_t map, u32 reg, u64 value)
+static inline int scom_write(scom_map_t map, u64 reg, u64 value)
{
return scom_controller->write(map, reg, value);
}
diff --git a/arch/powerpc/platforms/powernv/opal-xscom.c b/arch/powerpc/platforms/powernv/opal-xscom.c
index 3ed5c64..09a90d8 100644
--- a/arch/powerpc/platforms/powernv/opal-xscom.c
+++ b/arch/powerpc/platforms/powernv/opal-xscom.c
@@ -27,7 +27,7 @@
*/
struct opal_scom_map {
uint32_t chip;
- uint32_t addr;
+ uint64_t addr;
};
static scom_map_t opal_scom_map(struct device_node *dev, u64 reg, u64 count)
@@ -71,7 +71,7 @@ static int opal_xscom_err_xlate(int64_t rc)
}
}
-static int opal_scom_read(scom_map_t map, u32 reg, u64 *value)
+static int opal_scom_read(scom_map_t map, u64 reg, u64 *value)
{
struct opal_scom_map *m = map;
int64_t rc;
@@ -80,7 +80,7 @@ static int opal_scom_read(scom_map_t map, u32 reg, u64 *value)
return opal_xscom_err_xlate(rc);
}
-static int opal_scom_write(scom_map_t map, u32 reg, u64 value)
+static int opal_scom_write(scom_map_t map, u64 reg, u64 value)
{
struct opal_scom_map *m = map;
int64_t rc;
diff --git a/arch/powerpc/platforms/wsp/scom_wsp.c b/arch/powerpc/platforms/wsp/scom_wsp.c
index 54172c4..8928507 100644
--- a/arch/powerpc/platforms/wsp/scom_wsp.c
+++ b/arch/powerpc/platforms/wsp/scom_wsp.c
@@ -50,7 +50,7 @@ static void wsp_scom_unmap(scom_map_t map)
iounmap((void *)map);
}
-static int wsp_scom_read(scom_map_t map, u32 reg, u64 *value)
+static int wsp_scom_read(scom_map_t map, u64 reg, u64 *value)
{
u64 __iomem *addr = (u64 __iomem *)map;
@@ -59,7 +59,7 @@ static int wsp_scom_read(scom_map_t map, u32 reg, u64 *value)
return 0;
}
-static int wsp_scom_write(scom_map_t map, u32 reg, u64 value)
+static int wsp_scom_write(scom_map_t map, u64 reg, u64 value)
{
u64 __iomem *addr = (u64 __iomem *)map;
^ permalink raw reply related
* Re: [PATCH V2 0/6] perf: New conditional branch filter
From: Anshuman Khandual @ 2013-10-10 5:04 UTC (permalink / raw)
To: Stephane Eranian
Cc: Arnaldo Carvalho de Melo, Linux PPC dev, Sukadev Bhattiprolu,
Michael Neuling, LKML
In-Reply-To: <CABPqkBR0koi7Swzqxg2nhjDsXyfLCSdR5qtB7XuBwxfCxaRO0A@mail.gmail.com>
On 09/26/2013 04:44 PM, Stephane Eranian wrote:
> So you are saying that the HW filter is exclusive. That seems odd. But
> I think it is
> because of the choices is ANY. ANY covers all the types of branches. Therefore
> it does not make a difference whether you add COND or not. And
> vice-versa, if you
> set COND, you need to disable ANY. I bet if you add other filters such
> as CALL, RETURN,
> then you could OR them and say: I want RETURN or CALLS.
>
> But that's okay. The API operates in OR mode but if the HW does not
> support it, you
> can check the mask and reject if more than one type is set. That is
> arch-specific code.
> The alternative, if to only capture ANY and emulate the filter in SW.
> This will work, of
> course. But the downside, is that you lose the way to appreciate how
> many, for instance,
> COND branches you sampled out of the total number of COND branches
> retired. Unless
> you can count COND branches separately.
Hey Stephane,
Thanks for your reply. I am working on a solution where PMU will process
all the requested branch filters in HW only if it can filter all of them in an
OR manner else it will just leave the entire thing upto the SW to process and
do no filtering itself. This implies that branch filtering will either happen
completely in HW or completely in SW and never in a mixed manner. This way
it will conform to the OR mode defined in the API. I will post the revised
patch set soon.
Regards
Anshuman
^ permalink raw reply
* Re: [PATCH 2/3] gianfar: Use mpc85xx support for errata detection
From: Scott Wood @ 2013-10-09 20:14 UTC (permalink / raw)
To: Claudiu Manoil; +Cc: netdev, linuxppc-dev, David S. Miller
In-Reply-To: <1381339242-32030-2-git-send-email-claudiu.manoil@freescale.com>
On Wed, 2013-10-09 at 20:20 +0300, Claudiu Manoil wrote:
> +static void gfar_detect_errata(struct gfar_private *priv)
> +{
> + struct device *dev = &priv->ofdev->dev;
> +
> + /* no plans to fix */
> + priv->errata |= GFAR_ERRATA_A002;
> +
> + if (pvr_version_is(PVR_VER_E500V1) || pvr_version_is(PVR_VER_E500V2))
> + __gfar_detect_errata_85xx(priv);
> + else /* non-mpc85xx parts, i.e. e300 core based */
> + __gfar_detect_errata_83xx(priv);
It would be better to use CONFIG_E500 here (note that we do not support
building e500 and 83xx/86xx in the same kernel), on the off chance that
we put a gianfar in a chip with a newer e500 derivative. I suppose it's
harmless as long as the 83xx version checks the full PVR, until such a
chip exists and has an erratum workaround (other than A002) added for
it.
What about 86xx? Are there any gianfar errata there besides A002?
-Scott
^ permalink raw reply
* Re: [v1] powerpc/mpc512x: silence build warning upon disabled DIU
From: Brian Norris @ 2013-10-09 19:29 UTC (permalink / raw)
To: Gerhard Sittig; +Cc: Anatolij Gustschin, linuxppc-dev
In-Reply-To: <1380295718-10700-1-git-send-email-gsi@denx.de>
Hi all,
(Please keep me on the CC list, as I'm not subscribed)
On Fri, Sep 27, 2013 at 05:28:38PM +0200, Gerhard Sittig wrote:
> a disabled Kconfig option results in a reference to a not implemented
> routine when the IS_ENABLED() macro is used for both conditional
> implementation of the routine as well as a C language source code test
> at the call site -- the "if (0) func();" construct only gets eliminated
> later by the optimizer, while the compiler already has emitted its
> warning about "func()" being undeclared
>
> provide an empty implementation for the mpc512x_setup_diu() and
> mpc512x_init_diu() routines in case of the disabled option, to avoid the
> compiler warning which is considered fatal and breaks compilation
>
> the bug appeared with commit 2abbbb63c90ab55ca3f054772c2e5ba7df810c48
> "powerpc/mpc512x: move common code to shared.c file", how to reproduce:
>
> make mpc512x_defconfig
> echo CONFIG_FB_FSL_DIU=n >> .config && make olddefconfig
> make
>
> CC arch/powerpc/platforms/512x/mpc512x_shared.o
> .../arch/powerpc/platforms/512x/mpc512x_shared.c: In function 'mpc512x_init_early':
> .../arch/powerpc/platforms/512x/mpc512x_shared.c:456:3: error: implicit declaration of function 'mpc512x_init_diu' [-Werror=implicit-function-declaration]
> .../arch/powerpc/platforms/512x/mpc512x_shared.c: In function 'mpc512x_setup_arch':
> .../arch/powerpc/platforms/512x/mpc512x_shared.c:469:3: error: implicit declaration of function 'mpc512x_setup_diu' [-Werror=implicit-function-declaration]
> cc1: all warnings being treated as errors
> make[4]: *** [arch/powerpc/platforms/512x/mpc512x_shared.o] Error 1
I just ran across this same compile issue, and I have a few thoughts
below.
> Signed-off-by: Gerhard Sittig <gsi@denx.de>
> CC: <stable@vger.kernel.org> # v3.11
>
> ---
> arch/powerpc/platforms/512x/mpc512x_shared.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/powerpc/platforms/512x/mpc512x_shared.c b/arch/powerpc/platforms/512x/mpc512x_shared.c
> index a82a41b..1a7b1d0 100644
> --- a/arch/powerpc/platforms/512x/mpc512x_shared.c
> +++ b/arch/powerpc/platforms/512x/mpc512x_shared.c
> @@ -303,6 +303,9 @@ void __init mpc512x_setup_diu(void)
> diu_ops.release_bootmem = mpc512x_release_bootmem;
> }
>
> +#else
> +void __init mpc512x_setup_diu(void) { /* EMPTY */ }
> +void __init mpc512x_init_diu(void) { /* EMPTY */ }
> #endif
>
> void __init mpc512x_init_IRQ(void)
I see an alternative solution:
Can't almost all of the code in mpc512x_shared.c be declared 'static'?
Then, you can get the real benefit of IS_ENABLED() by removing the
#if IS_ENABLED(CONFIG_FB_FSL_DIU)
from around all the DIU code, and it will automatically be removed by
the compiler when it is not used.
I think the current patch is necessary for immediate use, and it can be
sent to stable. But I might suggest a follow-up patch or 2 that makes
the functions static and kills the #ifdef entirely.
Thanks,
Brian
^ permalink raw reply
* Re: [PATCH 3/3] gianfar: Enable eTSEC-20 erratum w/a for P2020 Rev1
From: David Miller @ 2013-10-09 18:02 UTC (permalink / raw)
To: claudiu.manoil; +Cc: netdev, linuxppc-dev
In-Reply-To: <1381339242-32030-3-git-send-email-claudiu.manoil@freescale.com>
From: Claudiu Manoil <claudiu.manoil@freescale.com>
Date: Wed, 9 Oct 2013 20:20:42 +0300
> Enable workaround for P2020/P2010 erratum eTSEC 20,
> "Excess delays when transmitting TOE=1 large frames".
> The impact is that frames lager than 2500-bytes for which
> TOE (i.e. TCP/IP hw accelerations like Tx csum) is enabled
> may see excess delay before start of transmission.
> This erratum was fixed in Rev 2.0.
>
> Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
Applied.
^ permalink raw reply
* Re: [PATCH 2/3] gianfar: Use mpc85xx support for errata detection
From: David Miller @ 2013-10-09 18:02 UTC (permalink / raw)
To: claudiu.manoil; +Cc: netdev, linuxppc-dev
In-Reply-To: <1381339242-32030-2-git-send-email-claudiu.manoil@freescale.com>
From: Claudiu Manoil <claudiu.manoil@freescale.com>
Date: Wed, 9 Oct 2013 20:20:41 +0300
> Use the macros and defines from mpc85xx.h to simplify
> and prevent errors in identifying a mpc85xx based SoC
> for errata detection.
> This should help enabling (and identifying) workarounds
> for various mpc85xx based chips and revisions.
> For instance, express MPC8548 Rev.2 as:
> (SVR_SOC_VER(svr) == SVR_8548) && (SVR_REV(svr) == 0x20)
> instead of:
> (pvr == 0x80210020 && mod == 0x8030 && rev == 0x0020)
>
> Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
Applied.
^ permalink raw reply
* Re: [PATCH 1/3] gianfar: Enable eTSEC-A002 erratum w/a for all parts
From: David Miller @ 2013-10-09 18:02 UTC (permalink / raw)
To: claudiu.manoil; +Cc: netdev, linuxppc-dev
In-Reply-To: <1381339242-32030-1-git-send-email-claudiu.manoil@freescale.com>
From: Claudiu Manoil <claudiu.manoil@freescale.com>
Date: Wed, 9 Oct 2013 20:20:40 +0300
> A002 is still in "no plans to fix" state, and applies to all
> the current P1/P2 parts as well, so it's resonable to enable
> its workaround by default, for all the soc's with etsec.
> The impact of not enabling this workaround for affected parts
> is that under certain conditons (runt frames or even frames
> with RX error detected at PHY level) during controller reset,
> the controller might fail to indicate Rx reset (GRS) completion.
>
> Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
Applied.
^ permalink raw reply
* [PATCH 3/3] gianfar: Enable eTSEC-20 erratum w/a for P2020 Rev1
From: Claudiu Manoil @ 2013-10-09 17:20 UTC (permalink / raw)
To: netdev, David S. Miller; +Cc: linuxppc-dev
In-Reply-To: <1381339242-32030-1-git-send-email-claudiu.manoil@freescale.com>
Enable workaround for P2020/P2010 erratum eTSEC 20,
"Excess delays when transmitting TOE=1 large frames".
The impact is that frames lager than 2500-bytes for which
TOE (i.e. TCP/IP hw accelerations like Tx csum) is enabled
may see excess delay before start of transmission.
This erratum was fixed in Rev 2.0.
Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
---
drivers/net/ethernet/freescale/gianfar.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index 329a206..9fbe4dd 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -968,6 +968,9 @@ static void __gfar_detect_errata_85xx(struct gfar_private *priv)
if ((SVR_SOC_VER(svr) == SVR_8548) && (SVR_REV(svr) == 0x20))
priv->errata |= GFAR_ERRATA_12;
+ if (((SVR_SOC_VER(svr) == SVR_P2020) && (SVR_REV(svr) < 0x20)) ||
+ ((SVR_SOC_VER(svr) == SVR_P2010) && (SVR_REV(svr) < 0x20)))
+ priv->errata |= GFAR_ERRATA_76; /* aka eTSEC 20 */
}
static void gfar_detect_errata(struct gfar_private *priv)
--
1.7.11.7
^ permalink raw reply related
* [PATCH 2/3] gianfar: Use mpc85xx support for errata detection
From: Claudiu Manoil @ 2013-10-09 17:20 UTC (permalink / raw)
To: netdev, David S. Miller; +Cc: linuxppc-dev
In-Reply-To: <1381339242-32030-1-git-send-email-claudiu.manoil@freescale.com>
Use the macros and defines from mpc85xx.h to simplify
and prevent errors in identifying a mpc85xx based SoC
for errata detection.
This should help enabling (and identifying) workarounds
for various mpc85xx based chips and revisions.
For instance, express MPC8548 Rev.2 as:
(SVR_SOC_VER(svr) == SVR_8548) && (SVR_REV(svr) == 0x20)
instead of:
(pvr == 0x80210020 && mod == 0x8030 && rev == 0x0020)
Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
---
drivers/net/ethernet/freescale/gianfar.c | 33 ++++++++++++++++++++++++--------
1 file changed, 25 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index db5fc7b..329a206 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -88,6 +88,7 @@
#include <asm/io.h>
#include <asm/reg.h>
+#include <asm/mpc85xx.h>
#include <asm/irq.h>
#include <asm/uaccess.h>
#include <linux/module.h>
@@ -939,17 +940,13 @@ static void gfar_init_filer_table(struct gfar_private *priv)
}
}
-static void gfar_detect_errata(struct gfar_private *priv)
+static void __gfar_detect_errata_83xx(struct gfar_private *priv)
{
- struct device *dev = &priv->ofdev->dev;
unsigned int pvr = mfspr(SPRN_PVR);
unsigned int svr = mfspr(SPRN_SVR);
unsigned int mod = (svr >> 16) & 0xfff6; /* w/o E suffix */
unsigned int rev = svr & 0xffff;
- /* no plans to fix */
- priv->errata |= GFAR_ERRATA_A002;
-
/* MPC8313 Rev 2.0 and higher; All MPC837x */
if ((pvr == 0x80850010 && mod == 0x80b0 && rev >= 0x0020) ||
(pvr == 0x80861010 && (mod & 0xfff9) == 0x80c0))
@@ -960,10 +957,30 @@ static void gfar_detect_errata(struct gfar_private *priv)
(pvr == 0x80861010 && (mod & 0xfff9) == 0x80c0))
priv->errata |= GFAR_ERRATA_76;
- /* MPC8313 Rev < 2.0, MPC8548 rev 2.0 */
- if ((pvr == 0x80850010 && mod == 0x80b0 && rev < 0x0020) ||
- (pvr == 0x80210020 && mod == 0x8030 && rev == 0x0020))
+ /* MPC8313 Rev < 2.0 */
+ if (pvr == 0x80850010 && mod == 0x80b0 && rev < 0x0020)
+ priv->errata |= GFAR_ERRATA_12;
+}
+
+static void __gfar_detect_errata_85xx(struct gfar_private *priv)
+{
+ unsigned int svr = mfspr(SPRN_SVR);
+
+ if ((SVR_SOC_VER(svr) == SVR_8548) && (SVR_REV(svr) == 0x20))
priv->errata |= GFAR_ERRATA_12;
+}
+
+static void gfar_detect_errata(struct gfar_private *priv)
+{
+ struct device *dev = &priv->ofdev->dev;
+
+ /* no plans to fix */
+ priv->errata |= GFAR_ERRATA_A002;
+
+ if (pvr_version_is(PVR_VER_E500V1) || pvr_version_is(PVR_VER_E500V2))
+ __gfar_detect_errata_85xx(priv);
+ else /* non-mpc85xx parts, i.e. e300 core based */
+ __gfar_detect_errata_83xx(priv);
if (priv->errata)
dev_info(dev, "enabled errata workarounds, flags: 0x%x\n",
--
1.7.11.7
^ permalink raw reply related
* [PATCH 1/3] gianfar: Enable eTSEC-A002 erratum w/a for all parts
From: Claudiu Manoil @ 2013-10-09 17:20 UTC (permalink / raw)
To: netdev, David S. Miller; +Cc: linuxppc-dev
A002 is still in "no plans to fix" state, and applies to all
the current P1/P2 parts as well, so it's resonable to enable
its workaround by default, for all the soc's with etsec.
The impact of not enabling this workaround for affected parts
is that under certain conditons (runt frames or even frames
with RX error detected at PHY level) during controller reset,
the controller might fail to indicate Rx reset (GRS) completion.
Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
---
drivers/net/ethernet/freescale/gianfar.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index c4eaade..db5fc7b 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -947,6 +947,9 @@ static void gfar_detect_errata(struct gfar_private *priv)
unsigned int mod = (svr >> 16) & 0xfff6; /* w/o E suffix */
unsigned int rev = svr & 0xffff;
+ /* no plans to fix */
+ priv->errata |= GFAR_ERRATA_A002;
+
/* MPC8313 Rev 2.0 and higher; All MPC837x */
if ((pvr == 0x80850010 && mod == 0x80b0 && rev >= 0x0020) ||
(pvr == 0x80861010 && (mod & 0xfff9) == 0x80c0))
@@ -957,11 +960,6 @@ static void gfar_detect_errata(struct gfar_private *priv)
(pvr == 0x80861010 && (mod & 0xfff9) == 0x80c0))
priv->errata |= GFAR_ERRATA_76;
- /* MPC8313 and MPC837x all rev */
- if ((pvr == 0x80850010 && mod == 0x80b0) ||
- (pvr == 0x80861010 && (mod & 0xfff9) == 0x80c0))
- priv->errata |= GFAR_ERRATA_A002;
-
/* MPC8313 Rev < 2.0, MPC8548 rev 2.0 */
if ((pvr == 0x80850010 && mod == 0x80b0 && rev < 0x0020) ||
(pvr == 0x80210020 && mod == 0x8030 && rev == 0x0020))
@@ -1599,7 +1597,7 @@ static int __gfar_is_rx_idle(struct gfar_private *priv)
/* Normaly TSEC should not hang on GRS commands, so we should
* actually wait for IEVENT_GRSC flag.
*/
- if (likely(!gfar_has_errata(priv, GFAR_ERRATA_A002)))
+ if (!gfar_has_errata(priv, GFAR_ERRATA_A002))
return 0;
/* Read the eTSEC register at offset 0xD1C. If bits 7-14 are
--
1.7.11.7
^ permalink raw reply related
* Re: [PATCH RFC 00/77] Re-design MSI/MSI-X interrupts enablement pattern
From: Tejun Heo @ 2013-10-09 15:57 UTC (permalink / raw)
To: Alexander Gordeev
Cc: linux-mips, VMware, Inc., linux-nvme, linux-ide, linux-s390,
Andy King, linux-scsi, linux-rdma, x86, Ingo Molnar, linux-pci,
iss_storagedev, linux-driver, Bjorn Helgaas, Dan Williams,
Jon Mason, Solarflare linux maintainers, netdev, linux-kernel,
Ralf Baechle, e1000-devel, Martin Schwidefsky, linux390,
linuxppc-dev
In-Reply-To: <20131008090716.GA10561@dhcp-26-207.brq.redhat.com>
Hello,
On Tue, Oct 08, 2013 at 11:07:16AM +0200, Alexander Gordeev wrote:
> Multipe MSIs is just a handful of drivers, really. MSI-X impact still
Yes, so it's pretty nice to try out things there before going full-on.
> will be huge. But if we opt a different name for the new pci_enable_msix()
> then we could first update pci/msi, then drivers (in few stages possibly)
> and finally remove the old implementation.
Yes, that probably should be the steps to follow eventually. My point
was that you don't have to submit patches for all 7x conversions for
an RFC round. Scanning them and seeing what would be necessary
definitely is a good idea but just giving summary of the full
conversion with several examples should be good enough before settling
on the way forward, which should be easier for all involved.
Thanks a lot for your effort!
--
tejun
^ permalink raw reply
* Re: [PATCH RFC 07/77] PCI/MSI: Re-design MSI/MSI-X interrupts enablement pattern
From: Tejun Heo @ 2013-10-09 15:54 UTC (permalink / raw)
To: Alexander Gordeev
Cc: linux-mips, linux-doc, VMware, Inc., linux-nvme, linux-ide,
linux-s390, Andy King, linux-scsi, linux-rdma, x86, Ingo Molnar,
linux-pci, iss_storagedev, linux-driver, Bjorn Helgaas,
Dan Williams, Jon Mason, Solarflare linux maintainers, netdev,
linux-kernel, Ralf Baechle, e1000-devel, Martin Schwidefsky,
linux390, linuxppc-dev
In-Reply-To: <20131008074826.GD10669@dhcp-26-207.brq.redhat.com>
Hello, Alexander.
On Tue, Oct 08, 2013 at 09:48:26AM +0200, Alexander Gordeev wrote:
> > If there are many which duplicate the above pattern, it'd probably be
> > worthwhile to provide a helper? It's usually a good idea to reduce
> > the amount of boilerplate code in drivers.
>
> I wanted to limit discussion in v1 to as little changes as possible.
> I 'planned' those helper(s) for a separate effort if/when the most
> important change is accepted and soaked a bit.
The thing is doing it this way generates more churns and noises. Once
the simpler ones live behind a wrapper which can be built on the
existing interface, we can have both reduced cost and more latitude on
the complex cases.
> > If we do things this way, it breaks all drivers using this interface
> > until they're converted, right?
>
> Right. And the rest of the series does it.
Which breaks bisection which we shouldn't do.
> > Also, it probably isn't the best idea
> > to flip the behavior like this as this can go completely unnoticed (no
> > compiler warning or anything, the same function just behaves
> > differently). Maybe it'd be a better idea to introduce a simpler
> > interface that most can be converted to?
>
> Well, an *other* interface is a good idea. What do you mean with the
> simpler here?
I'm still talking about a simpler wrapper for common cases, which is
the important part anyway.
Thanks.
--
tejun
^ 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