* [PATCH]qonverge/usb: Add first usb controller node
From: Ramneek Mehresh @ 2013-02-28 8:46 UTC (permalink / raw)
To: linuxppc-dev, devicetree-discuss; +Cc: kumar.gala, Ramneek Mehresh
Add first usb controller node for qonverge qoriq platforms like
B4860, etc
Signed-off-by: Ramneek Mehresh <ramneek.mehresh@freescale.com>
---
Applies on git://git.kernel.org/pub/scm/linux/kernel/git/galak/powerpc.git
(branch next)
arch/powerpc/boot/dts/fsl/qonverge-usb2-dr-0.dtsi | 41 +++++++++++++++++++++++
1 file changed, 41 insertions(+)
create mode 100644 arch/powerpc/boot/dts/fsl/qonverge-usb2-dr-0.dtsi
diff --git a/arch/powerpc/boot/dts/fsl/qonverge-usb2-dr-0.dtsi b/arch/powerpc/boot/dts/fsl/qonverge-usb2-dr-0.dtsi
new file mode 100644
index 0000000..29dad72
--- /dev/null
+++ b/arch/powerpc/boot/dts/fsl/qonverge-usb2-dr-0.dtsi
@@ -0,0 +1,41 @@
+/*
+ * QorIQ Qonverge USB Host device tree stub [ controller @ offset 0x210000 ]
+ *
+ * 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.
+ */
+
+usb@210000 {
+ compatible = "fsl-usb2-dr";
+ reg = <0x210000 0x1000>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ interrupts = <44 0x2 0 0>;
+};
--
1.7.11.4
^ permalink raw reply related
* Re: [PATCH -V1 05/24] powerpc: Move the pte free routines from common header
From: Paul Mackerras @ 2013-02-28 8:36 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: linuxppc-dev, linux-mm
In-Reply-To: <1361865914-13911-6-git-send-email-aneesh.kumar@linux.vnet.ibm.com>
On Tue, Feb 26, 2013 at 01:34:55PM +0530, Aneesh Kumar K.V wrote:
> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>
> This patch move the common code to 32/64 bit headers. We will
^ moves
> later change the 64 bit version to support smaller PTE fragments
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Acked-by: Paul Mackerras <paulus@samba.org>
^ permalink raw reply
* Re: [PATCH 6/6 v8] iommu/fsl: Freescale PAMU driver and IOMMU API implementation.
From: Stuart Yoder @ 2013-02-28 0:03 UTC (permalink / raw)
To: Varun Sethi
Cc: Joerg Roedel, Stuart Yoder, linux-kernel, iommu, Scott Wood,
linuxppc-dev
In-Reply-To: <1361191939-21260-7-git-send-email-Varun.Sethi@freescale.com>
Some more comments...
On Mon, Feb 18, 2013 at 6:52 AM, Varun Sethi <Varun.Sethi@freescale.com> wrote:
> +/* Handling access violations */
> +#define make64(high, low) (((u64)(high) << 32) | (low))
> +
> +struct pamu_isr_data {
> + void __iomem *pamu_reg_base; /* Base address of PAMU regs*/
> + unsigned int count; /* The number of PAMUs */
> +};
> +
> +static struct paace *ppaact;
> +static struct paace *spaact;
> +static struct ome *omt;
> +
> +/* maximum subwindows permitted per liodn */
> +unsigned int max_subwindow_count;
> +/* Number of SPAACT entries */
> +unsigned long max_subwins;
I don't like that these variables are not static... and they are
referenced directly
from code in fsl_pamu_domain.c. It would be better if fsl_pamu_domain.c
called an accessor function-- like pamu_get_max_subwins.
> +/* Pool for fspi allocation */
> +struct gen_pool *spaace_pool;
spaace_pool should be static?
I'm wondering if you should change pamu_isr_data into a more
general struct analagous to struct intel_iommu. You could
put in there the max # of subwins, etc. You could then
provide an accessor to get at that data.
[cut]
> +/**
> + * pamu_get_fspi_and_allocate() - Allocates fspi index and reserves subwindows
> + * required for primary PAACE in the secondary
> + * PAACE table.
> + * @subwin_cnt: Number of subwindows to be reserved.
> + *
> + * A PPAACE entry may have a number of associated subwindows. A subwindow
> + * corresponds to a SPAACE entry in the SPAACT table. Each PAACE entry stores
> + * the index (fspi) of the first SPAACE entry in the SPAACT table. This
> + * function returns the index of the first SPAACE entry. The remaining
> + * SPAACE entries are reserved contiguously from that index.
> + *
> + * Returns a valid fspi index in the range of 0 - max_subwins on success.
> + * If no SPAACE entry is available or the allocator can not reserve the required
> + * number of contiguous entries function returns ULONG_MAX indicating a failure.
> + *
> +*/
> +static unsigned long pamu_get_fspi_and_allocate(u32 subwin_cnt)
> +{
> + unsigned long spaace_addr;
> +
> + spaace_addr = gen_pool_alloc(spaace_pool, subwin_cnt * sizeof(struct paace));
> + if (!spaace_addr)
> + return ULONG_MAX;
> +
> + return (spaace_addr - (unsigned long)spaact) / (sizeof(struct paace));
> +}
In order to keep things symmetric (with the free function) can we just
call the above
function:
pamu_alloc_subwins()
> +/* Release the subwindows reserved for a particular LIODN */
> +void pamu_free_subwins(int liodn)
> +{
> + struct paace *ppaace;
> + u32 subwin_cnt, size;
> +
> + ppaace = pamu_get_ppaace(liodn);
> + if (!ppaace) {
> + pr_err("Invalid liodn entry\n");
> + return;
> + }
> +
> + if (get_bf(ppaace->addr_bitfields, PPAACE_AF_MW)) {
> + subwin_cnt = 1UL << (get_bf(ppaace->impl_attr, PAACE_IA_WCE) + 1);
> + size = (subwin_cnt - 1) * sizeof(struct paace);
> + gen_pool_free(spaace_pool, (unsigned long)&spaact[ppaace->fspi], size);
> + set_bf(ppaace->addr_bitfields, PPAACE_AF_MW, 0);
> + }
> +}
[cut]
> +/**
> + * get_stash_id - Returns stash destination id corresponding to a
> + * cache type and vcpu.
> + * @stash_dest_hint: L1, L2 or L3
> + * @vcpu: vpcu target for a particular cache type.
> + *
> + * Returs stash on success or ~(u32)0 on failure.
> + *
> + */
> +u32 get_stash_id(u32 stash_dest_hint, u32 vcpu)
> +{
The stash dest is really not a hint, right? It's the requested stash
destination. So maybe just drop 'hint' from the name.
The CPU here is really a physical CPU number and has nothing to do
with vcpus I think. vcpu implies the index is inside a virtual machine...but
this API is generic and may or may not be used with KVM.
> +
> +/*
> + * Get the maximum number of PAACT table entries
> + * and subwindows supported by PAMU
> + */
> +static void get_pamu_cap_values(unsigned long pamu_reg_base)
> +{
> + u32 pc_val;
> +
> + pc_val = in_be32((u32 *)(pamu_reg_base + PAMU_PC3));
> + /* Maximum number of subwindows per liodn */
> + max_subwindow_count = 1 << (1 + PAMU_PC3_MWCE(pc_val));
> + /* Total number of SPACCT entries */
> + max_subwins = PAACE_NUMBER_ENTRIES * max_subwindow_count;
> +}
If you follow the suggestion at the top of this file, this function
would become something like-- init_pamu_capabilities(). And then
create an accessor function to access max subwins, etc.
Also, BTW, I don't see any support for the DOMAIN_ATTR_WINDOWS
attribute in your patch. Was that coming in a later patch?
[cut
> +static int __init fsl_pamu_probe(struct platform_device *pdev)
> +{
> + void __iomem *pamu_regs = NULL;
> + struct ccsr_guts __iomem *guts_regs = NULL;
> + u32 pamubypenr, pamu_counter;
> + unsigned long pamu_reg_off;
> + unsigned long pamu_reg_base;
> + struct pamu_isr_data *data;
> + struct device_node *guts_node;
> + u64 size;
> + struct page *p;
> + int ret = 0;
> + int irq;
> + phys_addr_t ppaact_phys;
> + phys_addr_t spaact_phys;
> + phys_addr_t omt_phys;
> + size_t mem_size = 0;
> + unsigned int order = 0;
> + u32 csd_port_id = 0;
> + unsigned i;
> + /*
> + * enumerate all PAMUs and allocate and setup PAMU tables
> + * for each of them,
> + * NOTE : All PAMUs share the same LIODN tables.
> + */
> +
> + pamu_regs = of_iomap(pdev->dev.of_node, 0);
> + if (!pamu_regs) {
> + dev_err(&pdev->dev, "ioremap of PAMU node failed\n");
> + return -ENOMEM;
Any reason not to be consistent with the other error handling-- set ret and
goto error"?
> + }
> + of_get_address(pdev->dev.of_node, 0, &size, NULL);
> +
> + irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
> + if (irq == NO_IRQ) {
> + dev_warn(&pdev->dev, "no interrupts listed in PAMU node\n");
> + goto error;
> + }
> +
> + data = kzalloc(sizeof(struct pamu_isr_data), GFP_KERNEL);
> + if (!data) {
> + iounmap(pamu_regs);
> + return -ENOMEM;
Any reason not to be consistent with the other error handling-- set ret and
goto error"?
Stuart
^ permalink raw reply
* Re: [PATCH -V1 03/24] powerpc: Don't hard code the size of pte page
From: Paul Mackerras @ 2013-02-27 23:09 UTC (permalink / raw)
To: Aneesh Kumar K.V; +Cc: linuxppc-dev, linux-mm
In-Reply-To: <1361865914-13911-4-git-send-email-aneesh.kumar@linux.vnet.ibm.com>
On Tue, Feb 26, 2013 at 01:34:53PM +0530, Aneesh Kumar K.V wrote:
> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>
> USE PTRS_PER_PTE to indicate the size of pte page. To support THP,
> later patches will be changing PTRS_PER_PTE value.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Acked-by: Paul Mackerras <paulus@samba.org>
^ permalink raw reply
* Re: [PATCH v6 04/46] percpu_rwlock: Implement the core design of Per-CPU Reader-Writer Locks
From: Srivatsa S. Bhat @ 2013-02-27 21:19 UTC (permalink / raw)
To: Lai Jiangshan
Cc: linux-doc, peterz, fweisbec, linux-kernel, Michel Lespinasse,
mingo, linux-arch, linux, xiaoguangrong, wangyun, paulmck, nikunj,
linux-pm, rusty, rostedt, rjw, namhyung, tglx, linux-arm-kernel,
netdev, oleg, vincent.guittot, sbw, tj, akpm, linuxppc-dev
In-Reply-To: <CACvQF53ph2ixdM-otzetjM5cR+2kcZj6sckOp3TnzzA_x-EZ8w@mail.gmail.com>
On 02/27/2013 06:03 AM, Lai Jiangshan wrote:
> On Wed, Feb 27, 2013 at 3:30 AM, Srivatsa S. Bhat
> <srivatsa.bhat@linux.vnet.ibm.com> wrote:
>> On 02/26/2013 09:55 PM, Lai Jiangshan wrote:
>>> On Tue, Feb 26, 2013 at 10:22 PM, Srivatsa S. Bhat
>>> <srivatsa.bhat@linux.vnet.ibm.com> wrote:
>>>>
>>>> Hi Lai,
>>>>
>>>> I'm really not convinced that piggy-backing on lglocks would help
>>>> us in any way. But still, let me try to address some of the points
>>>> you raised...
>>>>
>>>> On 02/26/2013 06:29 PM, Lai Jiangshan wrote:
>>>>> On Tue, Feb 26, 2013 at 5:02 PM, Srivatsa S. Bhat
>>>>> <srivatsa.bhat@linux.vnet.ibm.com> wrote:
>>>>>> On 02/26/2013 05:47 AM, Lai Jiangshan wrote:
>>>>>>> On Tue, Feb 26, 2013 at 3:26 AM, Srivatsa S. Bhat
>>>>>>> <srivatsa.bhat@linux.vnet.ibm.com> wrote:
>>>>>>>> Hi Lai,
>>>>>>>>
>>>>>>>> On 02/25/2013 09:23 PM, Lai Jiangshan wrote:
>>>>>>>>> Hi, Srivatsa,
>>>>>>>>>
>>>>>>>>> The target of the whole patchset is nice for me.
>>>>>>>>
>>>>>>>> Cool! Thanks :-)
>>>>>>>>
>>>>>> [...]
>>>>>>
>>>>>> Unfortunately, I see quite a few issues with the code above. IIUC, the
>>>>>> writer and the reader both increment the same counters. So how will the
>>>>>> unlock() code in the reader path know when to unlock which of the locks?
>>>>>
>>>>> The same as your code, the reader(which nested in write C.S.) just dec
>>>>> the counters.
>>>>
>>>> And that works fine in my case because the writer and the reader update
>>>> _two_ _different_ counters.
>>>
>>> I can't find any magic in your code, they are the same counter.
>>>
>>> /*
>>> * It is desirable to allow the writer to acquire the percpu-rwlock
>>> * for read (if necessary), without deadlocking or getting complaints
>>> * from lockdep. To achieve that, just increment the reader_refcnt of
>>> * this CPU - that way, any attempt by the writer to acquire the
>>> * percpu-rwlock for read, will get treated as a case of nested percpu
>>> * reader, which is safe, from a locking perspective.
>>> */
>>> this_cpu_inc(pcpu_rwlock->rw_state->reader_refcnt);
>>>
>>
>> Whoa! Hold on, were you really referring to _this_ increment when you said
>> that, in your patch you would increment the refcnt at the writer? Then I guess
>> there is a major disconnect in our conversations. (I had assumed that you were
>> referring to the update of writer_signal, and were just trying to have a single
>> refcnt instead of reader_refcnt and writer_signal).
>
> https://github.com/laijs/linux/commit/53e5053d5b724bea7c538b11743d0f420d98f38d
>
> Sorry the name "fallback_reader_refcnt" misled you.
>
[...]
>>> All I was considered is "nested reader is seldom", so I always
>>> fallback to rwlock when nested.
>>> If you like, I can add 6 lines of code, the overhead is
>>> 1 spin_try_lock()(fast path) + N __this_cpu_inc()
>>>
>>
>> I'm assuming that calculation is no longer valid, considering that
>> we just discussed how the per-cpu refcnt that you were using is quite
>> unnecessary and can be removed.
>>
>> IIUC, the overhead with your code, as per above discussion would be:
>> 1 spin_try_lock() [non-nested] + N read_lock(global_rwlock).
>
> https://github.com/laijs/linux/commit/46334544bb7961550b7065e015da76f6dab21f16
>
> Again, I'm so sorry the name "fallback_reader_refcnt" misled you.
>
At this juncture I really have to admit that I don't understand your
intentions at all. What are you really trying to prove? Without giving
a single good reason why my code is inferior, why are you even bringing
up the discussion about a complete rewrite of the synchronization code?
http://article.gmane.org/gmane.linux.kernel.cross-arch/17103
http://article.gmane.org/gmane.linux.power-management.general/31345
I'm beginning to add 2 + 2 together based on the kinds of questions you
have been asking...
You posted a patch in this thread and started a discussion around it without
even establishing a strong reason to do so. Now you point me to your git
tree where your patches have even more traces of ideas being borrowed from
my patchset (apart from my own ideas/code, there are traces of others' ideas
being borrowed too - for example, it was Oleg who originally proposed the
idea of splitting up the counter into 2 parts and I'm seeing that it is
slowly crawling into your code with no sign of appropriate credits).
http://article.gmane.org/gmane.linux.network/260288
And in reply to my mail pointing out the performance implications of the
global read_lock at the reader side in your code, you said you'll come up
with a comparison between that and my patchset.
http://article.gmane.org/gmane.linux.network/260288
The issue has been well-documented in my patch description of patch 4.
http://article.gmane.org/gmane.linux.kernel/1443258
Are you really trying to pit bits and pieces of my own ideas/versions
against one another and claiming them as your own?
You projected the work involved in handling the locking issues pertaining
to CPU_DYING notifiers etc as a TODO, despite the fact that I had explicitly
noted in my cover letter that I had audited and taken care of all of them.
http://article.gmane.org/gmane.linux.documentation/9727
http://article.gmane.org/gmane.linux.documentation/9520
You failed to acknowledge (on purpose?) that I had done a tree-wide
conversion despite the fact that you were replying to the very thread which
had the 46 patches which did exactly that (and I had also mentioned it
explicitly in my cover letter).
http://article.gmane.org/gmane.linux.documentation/9727
http://article.gmane.org/gmane.linux.documentation/9520
You then started probing more and more about the technique I used to do
the tree-wide conversion.
http://article.gmane.org/gmane.linux.kernel.cross-arch/17111
You also retorted saying you did go through my patch descriptions, so
its not like you have missed reading them.
http://article.gmane.org/gmane.linux.power-management.general/31345
Each of these when considered individually, might appear like innocuous and
honest attempts at evaluating my code. But when put together, I'm beginning
to sense a whole different angle to it altogether, as if you are trying
to spin your own patch series, complete with the locking framework _and_
the tree-wide conversion, heavily borrowed from mine. At the beginning of
this discussion, I predicted that the lglock version that you are proposing
would end up being either less efficient than my version or look very similar
to my version. http://article.gmane.org/gmane.linux.kernel/1447139
I thought it was just the former till now, but its not hard to see how it
is getting closer to becoming the latter too. So yeah, I'm not amused.
Maybe (and hopefully) you are just trying out different ideas on your own,
and I'm just being paranoid. I really hope that is the case. If you are just
trying to review my code, then please stop sending patches with borrowed ideas
with your sole Signed-off-by, and purposefully ignoring the work already done
in my patchset, because it is really starting to look suspicious, at least
to me.
Don't get me wrong - I'll whole-heartedly acknowledge and appreciate if
_your_ code is better than mine. I just don't like the idea of somebody
plagiarizing my ideas/code (or even others' ideas for that matter).
However, I sincerely apologize in advance if I misunderstood/misjudged your
intentions; I just wanted to voice my concerns out loud at this point,
considering the bad feeling I got by looking at your responses collectively.
Regards,
Srivatsa S. Bhat
^ permalink raw reply
* [PATCH]: powerpc: Avoid link stack corruption in MMU on syscall entry path
From: Michael Neuling @ 2013-02-27 20:45 UTC (permalink / raw)
To: benh; +Cc: Linux PPC dev, anton, matt
Currently we use the link register to branch up high in the early MMU on
syscall entry path. Unfortunately, this trashes the link stack as the
address we are going to is not associated with the earlier mflr.
This patch simply converts us to used the count register (volatile over
syscalls anyway) instead. This is much better at predicting in this
scenario and doesn't trash link stack causing a bunch of additional
branch mispredicts later. Benchmarking this on POWER8 saves a bunch of
cycles on Anton's null syscall benchmark here:
http://ozlabs.org/~anton/junkcode/null_syscall.c
Signed-off-by: Michael Neuling <mikey@neuling.org>
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index a8a5361..87ef8f5 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -74,13 +74,13 @@ END_FTR_SECTION_IFSET(CPU_FTR_REAL_LE) \
mflr r10 ; \
ld r12,PACAKBASE(r13) ; \
LOAD_HANDLER(r12, system_call_entry_direct) ; \
- mtlr r12 ; \
+ mtctr r12 ; \
mfspr r12,SPRN_SRR1 ; \
/* Re-use of r13... No spare regs to do this */ \
li r13,MSR_RI ; \
mtmsrd r13,1 ; \
GET_PACA(r13) ; /* get r13 back */ \
- blr ;
+ bctr ;
#else
/* We can branch directly */
#define SYSCALL_PSERIES_2_DIRECT \
^ permalink raw reply related
* Re: [PATCH] Enhanced support for MPC8xx/8xxx watchdog
From: Wim Van Sebroeck @ 2013-02-27 19:52 UTC (permalink / raw)
To: Christophe Leroy; +Cc: linuxppc-dev, linux-kernel, linux-watchdog
In-Reply-To: <201302131519.r1DFJtGm032487@localhost.localdomain>
Hi Christophe,
> This patch modifies the behaviour of the MPC8xx/8xxx watchdog. On the MPC8xx,
> at 133Mhz, the maximum timeout of the watchdog timer is 1s, which means it must
> be pinged twice a second. This is not in line with the Linux watchdog concept
> which is based on a default watchdog timeout around 60s.
> This patch introduces an intermediate layer between the CPU and the userspace.
> The kernel pings the watchdog at the required frequency at the condition that
> userspace tools refresh it regularly.
> The new parameter 'heartbeat' allows to set up the userspace timeout.
> The driver also implements the WDIOC_SETTIMEOUT ioctl.
No, no, no... we should standardise on naming. heartbeat should be the thing that
actually keeps the dog alive, whereas timeout is the userspace timeout that the user
can play with...
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>
> diff -ur linux-3.7.7/drivers/watchdog/mpc8xxx_wdt.c linux/drivers/watchdog/mpc8xxx_wdt.c
> --- linux-3.7.7/drivers/watchdog/mpc8xxx_wdt.c 2013-02-11 18:05:09.000000000 +0100
> +++ linux/drivers/watchdog/mpc8xxx_wdt.c 2013-02-13 15:55:22.000000000 +0100
> @@ -52,10 +52,17 @@
> static struct mpc8xxx_wdt __iomem *wd_base;
> static int mpc8xxx_wdt_init_late(void);
>
> +#define WD_TIMO 10 /* Default heartbeat = 10 seconds */
> +
> +static int heartbeat = WD_TIMO;
> +module_param(heartbeat, int, 0);
> +MODULE_PARM_DESC(heartbeat,
> + "Watchdog SW heartbeat in seconds. (0 < heartbeat < 65536s, default="
> + __MODULE_STRING(WD_TIMO) "s)");
and the userspace timeout should be an unsigned int to be honoust.
> static u16 timeout = 0xffff;
> module_param(timeout, ushort, 0);
> MODULE_PARM_DESC(timeout,
> - "Watchdog timeout in ticks. (0<timeout<65536, default=65535)");
> + "Watchdog HW timeout in ticks. (0<timeout<65536, default=65535)");
>
> static bool reset = 1;
> module_param(reset, bool, 0);
> @@ -74,8 +81,10 @@
> static int prescale = 1;
> static unsigned int timeout_sec;
>
> +static int wdt_auto = 1;
> static unsigned long wdt_is_open;
> static DEFINE_SPINLOCK(wdt_spinlock);
> +static unsigned long wdt_last_ping;
>
> static void mpc8xxx_wdt_keepalive(void)
> {
> @@ -91,9 +100,20 @@
>
> static void mpc8xxx_wdt_timer_ping(unsigned long arg)
> {
> - mpc8xxx_wdt_keepalive();
> - /* We're pinging it twice faster than needed, just to be sure. */
> - mod_timer(&wdt_timer, jiffies + HZ * timeout_sec / 2);
> + if (wdt_auto)
> + wdt_last_ping = jiffies;
> +
> + if (jiffies - wdt_last_ping <= heartbeat * HZ) {
> + mpc8xxx_wdt_keepalive();
> + /* We're pinging it twice faster than needed, to be sure. */
> + mod_timer(&wdt_timer, jiffies + HZ * timeout_sec / 2);
> + }
> +}
> +
> +static void mpc8xxx_wdt_sw_keepalive(void)
> +{
> + wdt_last_ping = jiffies;
> + mpc8xxx_wdt_timer_ping(0);
> }
>
> static void mpc8xxx_wdt_pr_warn(const char *msg)
> @@ -106,7 +126,7 @@
> size_t count, loff_t *ppos)
> {
> if (count)
> - mpc8xxx_wdt_keepalive();
> + mpc8xxx_wdt_sw_keepalive();
> return count;
> }
>
> @@ -130,7 +150,7 @@
>
> out_be32(&wd_base->swcrr, tmp);
>
> - del_timer_sync(&wdt_timer);
> + wdt_auto = 0;
>
> return nonseekable_open(inode, file);
> }
> @@ -138,7 +158,8 @@
> static int mpc8xxx_wdt_release(struct inode *inode, struct file *file)
> {
> if (!nowayout)
> - mpc8xxx_wdt_timer_ping(0);
> + wdt_auto = 1;
> +
> else
> mpc8xxx_wdt_pr_warn("watchdog closed");
> clear_bit(0, &wdt_is_open);
> @@ -163,10 +184,12 @@
> case WDIOC_GETBOOTSTATUS:
> return put_user(0, p);
> case WDIOC_KEEPALIVE:
> - mpc8xxx_wdt_keepalive();
> + mpc8xxx_wdt_sw_keepalive();
> return 0;
> case WDIOC_GETTIMEOUT:
> - return put_user(timeout_sec, p);
> + return put_user(heartbeat, p);
> + case WDIOC_SETTIMEOUT:
> + return get_user(heartbeat, p);
> default:
> return -ENOTTY;
> }
> @@ -215,6 +238,8 @@
> ret = -ENOSYS;
> goto err_unmap;
> }
> + if (enabled)
> + timeout = in_be32(&wd_base->swcrr) >> 16;
>
> /* Calculate the timeout in seconds */
> if (prescale)
> @@ -273,6 +298,7 @@
> .compatible = "fsl,mpc823-wdt",
> .data = &(struct mpc8xxx_wdt_type) {
> .prescaler = 0x800,
> + .hw_enabled = true,
> },
> },
> {},
The rest of the code is OK and when above comments are corrected,
I will add the patch to improve the userspace experience.
Kind regards,
Wim.
^ permalink raw reply
* Re: [PATCH v6 04/46] percpu_rwlock: Implement the core design of Per-CPU Reader-Writer Locks
From: Oleg Nesterov @ 2013-02-27 19:25 UTC (permalink / raw)
To: Michel Lespinasse
Cc: Lai Jiangshan, linux-doc, peterz, fweisbec, mingo, linux-arch,
linux, xiaoguangrong, wangyun, paulmck, nikunj, linux-pm, rusty,
rostedt, rjw, namhyung, tglx, linux-arm-kernel, netdev,
linux-kernel, vincent.guittot, sbw, Srivatsa S. Bhat, tj, akpm,
linuxppc-dev
In-Reply-To: <CANN689FuuEb9wpV43DkTtgv6syvBYL-LQxByWznyAq8dkrhzBw@mail.gmail.com>
On 02/27, Michel Lespinasse wrote:
>
> +void lg_rwlock_local_read_lock(struct lgrwlock *lgrw)
> +{
> + preempt_disable();
> +
> + if (__this_cpu_read(*lgrw->local_refcnt) ||
> + arch_spin_trylock(this_cpu_ptr(lgrw->lglock->lock))) {
> + __this_cpu_inc(*lgrw->local_refcnt);
Please look at __this_cpu_generic_to_op(). You need this_cpu_inc()
to avoid the race with irs. The same for _read_unlock.
But otherwise I agree, looks like a clever and working idea to me.
And simple!
> There is an interesting case where lg_rwlock_local_read_lock could be
> interrupted after getting the local lglock but before incrementing
> local_refcnt to 1; if that happens any nested readers within that
> interrupt will have to take the global rwlock read side. I think this
> is perfectly acceptable
Agreed.
Or interrupt can do spin_trylock(percpu-lock) after we take the global
->fallback_rwlock (if we race with write_lock + write_unlock), but I do
not see any possible deadlock in this case.
Oleg.
^ permalink raw reply
* RE: [PATCH 1/6 v8] iommu/fsl: Store iommu domain information pointer in archdata.
From: Sethi Varun-B16395 @ 2013-02-27 12:04 UTC (permalink / raw)
To: Kumar Gala, Benjamin Herrenschmidt
Cc: Wood Scott-B07421, Joerg Roedel, linux-kernel@vger.kernel.org,
Yoder Stuart-B08248, iommu@lists.linux-foundation.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20130227113045.GH26252@8bytes.org>
Hi Kumar,Ben,
I am implementing the Freescale PAMU (IOMMU) driver using the Linux IOMMU A=
PI. In this particular patch, I have added a new field to dev_archdata stru=
cture to store the dma domain information.
This field is updated whenever we attach a device to an iommu domain.
Regards
Varun
> -----Original Message-----
> From: Joerg Roedel [mailto:joro@8bytes.org]
> Sent: Wednesday, February 27, 2013 5:01 PM
> To: Sethi Varun-B16395
> Cc: iommu@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org;
> linux-kernel@vger.kernel.org; Wood Scott-B07421; Yoder Stuart-B08248
> Subject: Re: [PATCH 1/6 v8] iommu/fsl: Store iommu domain information
> pointer in archdata.
>=20
> On Mon, Feb 18, 2013 at 06:22:14PM +0530, Varun Sethi wrote:
> > Add a new field in the device (powerpc) archdata structure for storing
> > iommu domain information pointer. This pointer is stored when the
> > device is attached to a particular domain.
> >
> >
> > Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
> > ---
> > - no change.
> > arch/powerpc/include/asm/device.h | 4 ++++
> > 1 files changed, 4 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/device.h
> > b/arch/powerpc/include/asm/device.h
> > index 77e97dd..6dc79fe 100644
> > --- a/arch/powerpc/include/asm/device.h
> > +++ b/arch/powerpc/include/asm/device.h
> > @@ -28,6 +28,10 @@ struct dev_archdata {
> > void *iommu_table_base;
> > } dma_data;
> >
> > + /* IOMMU domain information pointer. This would be set
> > + * when this device is attached to an iommu_domain.
> > + */
> > + void *iommu_domain;
>=20
> Please Cc the PowerPC Maintainers on this, so that they can have a look
> at it. This also must be put this into an #ifdef CONFIG_IOMMU_API.
>=20
>=20
> Joerg
>=20
>=20
^ permalink raw reply
* RE: [PATCH 6/6 v8] iommu/fsl: Freescale PAMU driver and IOMMU API implementation.
From: Sethi Varun-B16395 @ 2013-02-27 11:56 UTC (permalink / raw)
To: Stuart Yoder
Cc: Wood Scott-B07421, Joerg Roedel, linux-kernel@vger.kernel.org,
Yoder Stuart-B08248, iommu@lists.linux-foundation.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <CALRxmdBBO2AoAnKSTNNTYhHZ9fitJKWWeZMUj1ObtFhsZS5iSw@mail.gmail.com>
> -----Original Message-----
> From: Stuart Yoder [mailto:b08248@gmail.com]
> Sent: Wednesday, February 27, 2013 4:03 AM
> To: Sethi Varun-B16395
> Cc: iommu@lists.linux-foundation.org; linuxppc-dev@lists.ozlabs.org;
> linux-kernel@vger.kernel.org; Wood Scott-B07421; Joerg Roedel; Yoder
> Stuart-B08248
> Subject: Re: [PATCH 6/6 v8] iommu/fsl: Freescale PAMU driver and IOMMU
> API implementation.
>=20
> Have not got through the entire file, but have a few comments...
>=20
> +/*
> + * Set the PAACE type as primary and set the coherency required domain
> + * attribute
> + */
> +static void pamu_setup_default_xfer_to_host_ppaace(struct paace
> +*ppaace) {
> + set_bf(ppaace->addr_bitfields, PAACE_AF_PT, PAACE_PT_PRIMARY);
> +
> + set_bf(ppaace->domain_attr.to_host.coherency_required,
> PAACE_DA_HOST_CR,
> + PAACE_M_COHERENCE_REQ);
> +}
> +
> +/*
> + * Set the PAACE type as secondary and set the coherency required
> +domain
> + * attribute.
> + */
> +static void pamu_setup_default_xfer_to_host_spaace(struct paace
> +*spaace) {
> + set_bf(spaace->addr_bitfields, PAACE_AF_PT, PAACE_PT_SECONDARY);
> + set_bf(spaace->domain_attr.to_host.coherency_required,
> PAACE_DA_HOST_CR,
> + PAACE_M_COHERENCE_REQ);
> +}
>=20
> Can we change the names of the above functions... I know there is some
> history
> with the name, but "xfer_to_host" is confusing.
>=20
> Maybe just call them:
>=20
> pamu_init_paace()
> pamu_init_spaace()
>=20
[Sethi Varun-B16395] ok, will change the function names.
> > +/**
> > + * pamu_config_spaace() - Sets up SPAACE entry for specified
> > +subwindow
> > + *
> > + * @liodn: Logical IO device number
> > + * @subwin_cnt: number of sub-windows associated with dma-window
> > + * @subwin_addr: starting address of subwindow
> > + * @subwin_size: size of subwindow
> > + * @omi: Operation mapping index
> > + * @rpn: real (true physical) page number
> > + * @snoopid: snoop id for hardware coherency -- if ~snoopid =3D=3D 0 t=
hen
> > + * snoopid not defined
> > + * @stashid: cache stash id for associated cpu
> > + * @enable: enable/disable subwindow after reconfiguration
> > + * @prot: sub window permissions
> > + *
> > + * Returns 0 upon success else error code < 0 returned */ int
> > +pamu_config_spaace(int liodn, u32 subwin_cnt, u32 subwin_addr,
> > + phys_addr_t subwin_size, u32 omi, unsigned long
> rpn,
> > + u32 snoopid, u32 stashid, int enable, int prot)
> > +{
> > + struct paace *paace;
> > +
> > + /* setup sub-windows */
> > + if (!subwin_cnt) {
> > + pr_err("Invalid subwindow count\n");
> > + return -EINVAL;
> > + }
> > +
> > + paace =3D pamu_get_ppaace(liodn);
> > + if (subwin_addr > 0 && subwin_addr < subwin_cnt && paace) {
>=20
> Why is the comparison subwin_addr < subwin_cnt? Seems wrong...
>=20
[Sethi Varun-B16395] It's actually the subwindow index. I will rename the v=
ariable.
> > + paace =3D pamu_get_spaace(paace, subwin_addr - 1);
> > +
> > + if (paace && !(paace->addr_bitfields & PAACE_V_VALID))
> {
> > + pamu_setup_default_xfer_to_host_spaace(paace);
> > + set_bf(paace->addr_bitfields, SPAACE_AF_LIODN,
> liodn);
> > + }
> > + }
> > +
> > + if (!paace) {
> > + pr_err("Invalid liodn entry\n");
> > + return -ENOENT;
> > + }
> > +
> > + if (!enable && prot =3D=3D PAACE_AP_PERMS_DENIED) {
> > + if (subwin_addr > 0)
> > + set_bf(paace->addr_bitfields, PAACE_AF_V,
> > + PAACE_V_INVALID);
> > + else
> > + set_bf(paace->addr_bitfields, PAACE_AF_AP,
> > + prot);
> > + mb();
> > + return 0;
> > + }
>=20
> Can you add a comment to the above if statement...when is this function
> called with PAACE_AP_PERMS_DENIED?
>=20
[Sethi Varun-B16395] Actually, this piece of code is redundant in case of t=
he window based API. I will remove this. PAACE_AP_PERMS_DENIED is primarily=
used for disabling the primary subwindow.
-Varun
^ permalink raw reply
* Re: [PATCH 5/6 v8] iommu/fsl: Add addtional attributes specific to the PAMU driver.
From: Joerg Roedel @ 2013-02-27 11:38 UTC (permalink / raw)
To: Varun Sethi; +Cc: scottwood, iommu, linuxppc-dev, linux-kernel, stuart.yoder
In-Reply-To: <1361191939-21260-6-git-send-email-Varun.Sethi@freescale.com>
On Mon, Feb 18, 2013 at 06:22:18PM +0530, Varun Sethi wrote:
> Added the following domain attributes for the FSL PAMU driver:
> 1. Added new iommu stash attribute, which allows setting of the
> LIODN specific stash id parameter through IOMMU API.
> 2. Added an attribute for enabling/disabling DMA to a particular
> memory window.
> 3. Added domain attribute to check for PAMUV1 specific constraints.
>
>
> Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
> ---
> include/linux/iommu.h | 33 +++++++++++++++++++++++++++++++++
> 1 files changed, 33 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 529987c..c44e38b 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -40,6 +40,23 @@ struct notifier_block;
> typedef int (*iommu_fault_handler_t)(struct iommu_domain *,
> struct device *, unsigned long, int, void *);
>
> +/* cache stash targets */
> +#define IOMMU_ATTR_CACHE_L1 1
> +#define IOMMU_ATTR_CACHE_L2 2
> +#define IOMMU_ATTR_CACHE_L3 3
> +
> +/* This attribute corresponds to IOMMUs capable of generating
> + * a stash transaction. A stash transaction is typically a
> + * hardware initiated prefetch of data from memory to cache.
> + * This attribute allows configuring stashig specific parameters
> + * in the IOMMU hardware.
> + */
> +
> +struct iommu_stash_attribute {
> + u32 cpu; /* cpu number */
> + u32 cache; /* cache to stash to: L1,L2,L3 */
> +};
Please make the cache-attribute an enum instead of using defines.
Joerg
^ permalink raw reply
* Re: [PATCH 3/6] powerpc/fsl_pci: Added defines for the FSL PCI controller BRR1 register.
From: Joerg Roedel @ 2013-02-27 11:33 UTC (permalink / raw)
To: Varun Sethi; +Cc: scottwood, iommu, linuxppc-dev, linux-kernel, stuart.yoder
In-Reply-To: <1361191939-21260-4-git-send-email-Varun.Sethi@freescale.com>
On Mon, Feb 18, 2013 at 06:22:16PM +0530, Varun Sethi wrote:
> Macros for checking FSL PCI controller version.
>
> Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
> ---
> arch/powerpc/include/asm/pci-bridge.h | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
> index 025a130..c12ed78 100644
> --- a/arch/powerpc/include/asm/pci-bridge.h
> +++ b/arch/powerpc/include/asm/pci-bridge.h
> @@ -14,6 +14,10 @@
>
> struct device_node;
>
> +/* FSL PCI controller BRR1 register */
> +#define PCI_FSL_BRR1 0xbf8
> +#define PCI_FSL_BRR1_VER 0xffff
> +
Please merge this patch with the one where you actually make use of
these defines for the first time.
Joerg
^ permalink raw reply
* Re: [PATCH 1/6 v8] iommu/fsl: Store iommu domain information pointer in archdata.
From: Joerg Roedel @ 2013-02-27 11:30 UTC (permalink / raw)
To: Varun Sethi; +Cc: scottwood, iommu, linuxppc-dev, linux-kernel, stuart.yoder
In-Reply-To: <1361191939-21260-2-git-send-email-Varun.Sethi@freescale.com>
On Mon, Feb 18, 2013 at 06:22:14PM +0530, Varun Sethi wrote:
> Add a new field in the device (powerpc) archdata structure for storing iommu domain
> information pointer. This pointer is stored when the device is attached to a particular
> domain.
>
>
> Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
> ---
> - no change.
> arch/powerpc/include/asm/device.h | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/device.h b/arch/powerpc/include/asm/device.h
> index 77e97dd..6dc79fe 100644
> --- a/arch/powerpc/include/asm/device.h
> +++ b/arch/powerpc/include/asm/device.h
> @@ -28,6 +28,10 @@ struct dev_archdata {
> void *iommu_table_base;
> } dma_data;
>
> + /* IOMMU domain information pointer. This would be set
> + * when this device is attached to an iommu_domain.
> + */
> + void *iommu_domain;
Please Cc the PowerPC Maintainers on this, so that they can have a look
at it. This also must be put this into an #ifdef CONFIG_IOMMU_API.
Joerg
^ permalink raw reply
* Re: [PATCH v6 04/46] percpu_rwlock: Implement the core design of Per-CPU Reader-Writer Locks
From: Michel Lespinasse @ 2013-02-27 11:11 UTC (permalink / raw)
To: Srivatsa S. Bhat
Cc: Lai Jiangshan, linux-doc, peterz, fweisbec, linux-kernel, mingo,
linux-arch, linux, xiaoguangrong, wangyun, paulmck, nikunj,
linux-pm, rusty, rostedt, rjw, namhyung, tglx, linux-arm-kernel,
netdev, oleg, vincent.guittot, sbw, tj, akpm, linuxppc-dev
In-Reply-To: <512D0D67.9010609@linux.vnet.ibm.com>
Hi Srivatsa,
I think there is some elegance in Lai's proposal of using a local
trylock for the reader uncontended case and global rwlock to deal with
the contended case without deadlocks. He apparently didn't realize
initially that nested read locks are common, and he seems to have
confused you because of that, but I think his proposal could be
changed easily to account for that and result in short, easily
understood code. What about the following:
- local_refcnt is a local lock count; it indicates how many recursive
locks are taken using the local lglock
- lglock is used by readers for local locking; it must be acquired
before local_refcnt becomes nonzero and released after local_refcnt
goes back to zero.
- fallback_rwlock is used by readers for global locking; it is
acquired when fallback_reader_refcnt is zero and the trylock fails on
lglock
+void lg_rwlock_local_read_lock(struct lgrwlock *lgrw)
+{
+ preempt_disable();
+
+ if (__this_cpu_read(*lgrw->local_refcnt) ||
+ arch_spin_trylock(this_cpu_ptr(lgrw->lglock->lock))) {
+ __this_cpu_inc(*lgrw->local_refcnt);
+
rwlock_acquire_read(&lgrw->fallback_rwlock->lock_dep_map, 0, 0,
_RET_IP_);
+ } else {
+ read_lock(&lgrw->fallback_rwlock);
+ }
+}
+EXPORT_SYMBOL(lg_rwlock_local_read_lock);
+
+void lg_rwlock_local_read_unlock(struct lgrwlock *lgrw)
+{
+ if (likely(__this_cpu_read(*lgrw->local_refcnt))) {
+ rwlock_release(&lgrw->fallback_rwlock->lock_dep_map,
1, _RET_IP_);
+ if (!__this_cpu_dec_return(*lgrw->local_refcnt))
+ arch_spin_unlock(this_cpu_ptr(lgrw->lglock->lock));
+ } else {
+ read_unlock(&lgrw->fallback_rwlock);
+ }
+
+ preempt_enable();
+}
+EXPORT_SYMBOL(lg_rwlock_local_read_unlock);
+
+void lg_rwlock_global_write_lock(struct lgrwlock *lgrw)
+{
+ int i;
+
+ preempt_disable();
+
+ for_each_possible_cpu(i)
+ arch_spin_lock(per_cpu_ptr(lgrw->lglock->lock, i));
+ write_lock(&lgrw->fallback_rwlock);
+}
+EXPORT_SYMBOL(lg_rwlock_global_write_lock);
+
+void lg_rwlock_global_write_unlock(struct lgrwlock *lgrw)
+{
+ int i;
+
+ write_unlock(&lgrw->fallback_rwlock);
+ for_each_possible_cpu(i)
+ arch_spin_unlock(per_cpu_ptr(lgrw->lglock->lock, i));
+
+ preempt_enable();
+}
+EXPORT_SYMBOL(lg_rwlock_global_write_unlock);
This is to me relatively easier to understand than Srivatsa's
proposal. Now I'm not sure who wins efficiency wise, but I think it
should be relatively close as readers at least don't touch shared
state in the uncontended case (even with some recursion going on).
There is an interesting case where lg_rwlock_local_read_lock could be
interrupted after getting the local lglock but before incrementing
local_refcnt to 1; if that happens any nested readers within that
interrupt will have to take the global rwlock read side. I think this
is perfectly acceptable as this should not be a common case though
(and thus the global rwlock cache line probably wouldn't even bounce
between cpus then).
--
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.
^ permalink raw reply
* Re: [PATCH 2/6] powerpc/fsl_pci: Store the platform device information corresponding to the pci controller.
From: Joerg Roedel @ 2013-02-27 10:45 UTC (permalink / raw)
To: Sethi Varun-B16395
Cc: Wood Scott-B07421, Stuart Yoder, linux-kernel@vger.kernel.org,
Yoder Stuart-B08248, iommu@lists.linux-foundation.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <C5ECD7A89D1DC44195F34B25E172658D3BD338@039-SN2MPN1-013.039d.mgd.msft.net>
On Tue, Feb 26, 2013 at 06:16:10AM +0000, Sethi Varun-B16395 wrote:
> This patch is not present in Joerg's tree and the add_device API in
> the PAMU driver requires this patch.
Will this patch be part of v3.9-rc1?
Joerg
^ permalink raw reply
* RE: [PATCH 2/6] powerpc/fsl_pci: Store the platform device information corresponding to the pci controller.
From: Sethi Varun-B16395 @ 2013-02-27 10:56 UTC (permalink / raw)
To: Joerg Roedel
Cc: Wood Scott-B07421, Stuart Yoder, linux-kernel@vger.kernel.org,
Yoder Stuart-B08248, iommu@lists.linux-foundation.org,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20130227104505.GE26252@8bytes.org>
This patch is present in the "next branch" of linux ppc tree maintained by =
Kumar Gala.
Following is the commit id:
52c5affc545053d37c0b05224bbf70f5336caa20
I am not sure if this would be part of 3.9-rc1.
Regards
varun
> -----Original Message-----
> From: Joerg Roedel [mailto:joro@8bytes.org]
> Sent: Wednesday, February 27, 2013 4:15 PM
> To: Sethi Varun-B16395
> Cc: Stuart Yoder; iommu@lists.linux-foundation.org; linuxppc-
> dev@lists.ozlabs.org; linux-kernel@vger.kernel.org; Wood Scott-B07421;
> Yoder Stuart-B08248
> Subject: Re: [PATCH 2/6] powerpc/fsl_pci: Store the platform device
> information corresponding to the pci controller.
>=20
> On Tue, Feb 26, 2013 at 06:16:10AM +0000, Sethi Varun-B16395 wrote:
> > This patch is not present in Joerg's tree and the add_device API in
> > the PAMU driver requires this patch.
>=20
> Will this patch be part of v3.9-rc1?
>=20
>=20
> Joerg
>=20
>=20
^ permalink raw reply
* [v3][PATCH 6/6] powerpc/kgdb: remove copying the thread_info
From: Tiejun Chen @ 2013-02-27 3:09 UTC (permalink / raw)
To: benh, galak; +Cc: linuxppc-dev, linux-kernel, jason.wessel
In-Reply-To: <1361934568-31989-1-git-send-email-tiejun.chen@windriver.com>
Currently BookE and Book3E always copy the thread_info from
the kernel stack when we enter the debug exception, so we can
remove these action here to avoid copying again.
Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
---
arch/powerpc/kernel/kgdb.c | 28 ----------------------------
1 file changed, 28 deletions(-)
diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c
index 1a57307..e954888 100644
--- a/arch/powerpc/kernel/kgdb.c
+++ b/arch/powerpc/kernel/kgdb.c
@@ -153,39 +153,11 @@ static int kgdb_handle_breakpoint(struct pt_regs *regs)
static int kgdb_singlestep(struct pt_regs *regs)
{
- struct thread_info *thread_info, *exception_thread_info;
- struct thread_info *backup_current_thread_info;
-
if (user_mode(regs))
return 0;
- backup_current_thread_info = (struct thread_info *)kmalloc(sizeof(struct thread_info), GFP_KERNEL);
- /*
- * On Book E and perhaps other processors, singlestep is handled on
- * the critical exception stack. This causes current_thread_info()
- * to fail, since it it locates the thread_info by masking off
- * the low bits of the current stack pointer. We work around
- * this issue by copying the thread_info from the kernel stack
- * before calling kgdb_handle_exception, and copying it back
- * afterwards. On most processors the copy is avoided since
- * exception_thread_info == thread_info.
- */
- thread_info = (struct thread_info *)(regs->gpr[1] & ~(THREAD_SIZE-1));
- exception_thread_info = current_thread_info();
-
- if (thread_info != exception_thread_info) {
- /* Save the original current_thread_info. */
- memcpy(backup_current_thread_info, exception_thread_info, sizeof *thread_info);
- memcpy(exception_thread_info, thread_info, sizeof *thread_info);
- }
-
kgdb_handle_exception(0, SIGTRAP, 0, regs);
- if (thread_info != exception_thread_info)
- /* Restore current_thread_info lastly. */
- memcpy(exception_thread_info, backup_current_thread_info, sizeof *thread_info);
-
- kfree(backup_current_thread_info);
return 1;
}
--
1.7.9.5
^ permalink raw reply related
* [v3][PATCH 5/6] kgdb/kgdbts: support ppc64
From: Tiejun Chen @ 2013-02-27 3:09 UTC (permalink / raw)
To: benh, galak; +Cc: linuxppc-dev, linux-kernel, jason.wessel
In-Reply-To: <1361934568-31989-1-git-send-email-tiejun.chen@windriver.com>
We can't look up the address of the entry point of the function simply
via that function symbol for all architectures.
For PPC64 ABI, actually there is a function descriptors structure.
A function descriptor is a three doubleword data structure that contains
the following values:
* The first doubleword contains the address of the entry point of
the function.
* The second doubleword contains the TOC base address for
the function.
* The third doubleword contains the environment pointer for
languages such as Pascal and PL/1.
So we should call a wapperred dereference_function_descriptor() to get
the address of the entry point of the function.
Note this is also safe for other architecture after refer to
"include/asm-generic/sections.h" since:
dereference_function_descriptor(p) always is (p) if without arched definition.
Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
---
drivers/misc/kgdbts.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/misc/kgdbts.c b/drivers/misc/kgdbts.c
index 3aa9a96..4799e1f 100644
--- a/drivers/misc/kgdbts.c
+++ b/drivers/misc/kgdbts.c
@@ -103,6 +103,7 @@
#include <linux/delay.h>
#include <linux/kthread.h>
#include <linux/module.h>
+#include <asm/sections.h>
#define v1printk(a...) do { \
if (verbose) \
@@ -222,6 +223,7 @@ static unsigned long lookup_addr(char *arg)
addr = (unsigned long)do_fork;
else if (!strcmp(arg, "hw_break_val"))
addr = (unsigned long)&hw_break_val;
+ addr = (unsigned long )dereference_function_descriptor((void *)addr);
return addr;
}
--
1.7.9.5
^ permalink raw reply related
* [v3][PATCH 4/6] powerpc/book3e: support kgdb for kernel space
From: Tiejun Chen @ 2013-02-27 3:09 UTC (permalink / raw)
To: benh, galak; +Cc: linuxppc-dev, linux-kernel, jason.wessel
In-Reply-To: <1361934568-31989-1-git-send-email-tiejun.chen@windriver.com>
Currently we need to skip this for supporting KGDB.
Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
---
arch/powerpc/kernel/exceptions-64e.S | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
index 7df9a1f..800e2a3 100644
--- a/arch/powerpc/kernel/exceptions-64e.S
+++ b/arch/powerpc/kernel/exceptions-64e.S
@@ -598,11 +598,14 @@ kernel_dbg_exc:
rfdi
/* Normal debug exception */
+1:
+#ifndef CONFIG_KGDB
/* XXX We only handle coming from userspace for now since we can't
* quite save properly an interrupted kernel state yet
*/
-1: andi. r14,r11,MSR_PR; /* check for userspace again */
+ andi. r14,r11,MSR_PR; /* check for userspace again */
beq kernel_dbg_exc; /* if from kernel mode */
+#endif
/* Now we mash up things to make it look like we are coming on a
* normal exception
--
1.7.9.5
^ permalink raw reply related
* [v3][PATCH 3/6] book3e/kgdb: update thread's dbcr0
From: Tiejun Chen @ 2013-02-27 3:09 UTC (permalink / raw)
To: benh, galak; +Cc: linuxppc-dev, linux-kernel, jason.wessel
In-Reply-To: <1361934568-31989-1-git-send-email-tiejun.chen@windriver.com>
gdb always need to generate a single step properly to invoke
a kgdb state. But with lazy interrupt, book3e can't always
trigger a debug exception with a single step since the current
is blocked for handling those pending exception, then we miss
that expected dbcr configuration at last to generate a debug
exception.
So here we also update thread's dbcr0 to make sure the current
can go back with that missed dbcr0 configuration.
Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
---
arch/powerpc/kernel/kgdb.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c
index 5ca82cd..1a57307 100644
--- a/arch/powerpc/kernel/kgdb.c
+++ b/arch/powerpc/kernel/kgdb.c
@@ -410,7 +410,7 @@ int kgdb_arch_handle_exception(int vector, int signo, int err_code,
struct pt_regs *linux_regs)
{
char *ptr = &remcom_in_buffer[1];
- unsigned long addr;
+ unsigned long addr, dbcr0;
switch (remcom_in_buffer[0]) {
/*
@@ -427,8 +427,15 @@ int kgdb_arch_handle_exception(int vector, int signo, int err_code,
/* set the trace bit if we're stepping */
if (remcom_in_buffer[0] == 's') {
#ifdef CONFIG_PPC_ADV_DEBUG_REGS
- mtspr(SPRN_DBCR0,
- mfspr(SPRN_DBCR0) | DBCR0_IC | DBCR0_IDM);
+ dbcr0 = mfspr(SPRN_DBCR0) | DBCR0_IC | DBCR0_IDM;
+ mtspr(SPRN_DBCR0, dbcr0);
+#ifdef CONFIG_PPC_BOOK3E_64
+ /* With lazy interrut we have to update thread dbcr0 here
+ * to make sure we can set debug properly at last to invoke
+ * kgdb again to work well.
+ */
+ current->thread.dbcr0 = dbcr0;
+#endif
linux_regs->msr |= MSR_DE;
#else
linux_regs->msr |= MSR_SE;
--
1.7.9.5
^ permalink raw reply related
* [v3][PATCH 2/6] powerpc/book3e: store critical/machine/debug exception thread info
From: Tiejun Chen @ 2013-02-27 3:09 UTC (permalink / raw)
To: benh, galak; +Cc: linuxppc-dev, linux-kernel, jason.wessel
In-Reply-To: <1361934568-31989-1-git-send-email-tiejun.chen@windriver.com>
We need to store thread info to these exception thread info like something
we already did for PPC32.
Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
---
arch/powerpc/kernel/exceptions-64e.S | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
index 7fd6af0..7df9a1f 100644
--- a/arch/powerpc/kernel/exceptions-64e.S
+++ b/arch/powerpc/kernel/exceptions-64e.S
@@ -67,6 +67,18 @@
std r10,PACA_##level##_STACK(r13);
#endif
+/* Store something to exception thread info */
+#define BOOK3E_STORE_EXC_LEVEL_THEAD_INFO(type) \
+ ld r14,PACAKSAVE(r13); \
+ CURRENT_THREAD_INFO(r14, r14); \
+ CURRENT_THREAD_INFO(r15, r1); \
+ ld r10,TI_FLAGS(r14); \
+ std r10,TI_FLAGS(r15); \
+ ld r10,TI_PREEMPT(r14); \
+ std r10,TI_PREEMPT(r15); \
+ ld r10,TI_TASK(r14); \
+ std r10,TI_TASK(r15);
+
/* Exception prolog code for all exceptions */
#define EXCEPTION_PROLOG(n, intnum, type, addition) \
mtspr SPRN_SPRG_##type##_SCRATCH,r13; /* get spare registers */ \
@@ -104,6 +116,7 @@
BOOK3E_LOAD_EXC_LEVEL_STACK(CRIT); \
ld r1,PACA_CRIT_STACK(r13); \
subi r1,r1,SPECIAL_EXC_FRAME_SIZE; \
+ BOOK3E_STORE_EXC_LEVEL_THEAD_INFO(CRIT); \
1:
#define SPRN_CRIT_SRR0 SPRN_CSRR0
#define SPRN_CRIT_SRR1 SPRN_CSRR1
@@ -114,6 +127,7 @@
BOOK3E_LOAD_EXC_LEVEL_STACK(DBG); \
ld r1,PACA_DBG_STACK(r13); \
subi r1,r1,SPECIAL_EXC_FRAME_SIZE; \
+ BOOK3E_STORE_EXC_LEVEL_THEAD_INFO(DBG); \
1:
#define SPRN_DBG_SRR0 SPRN_DSRR0
#define SPRN_DBG_SRR1 SPRN_DSRR1
@@ -124,6 +138,7 @@
BOOK3E_LOAD_EXC_LEVEL_STACK(MC); \
ld r1,PACA_MC_STACK(r13); \
subi r1,r1,SPECIAL_EXC_FRAME_SIZE; \
+ BOOK3E_STORE_EXC_LEVEL_THEAD_INFO(MC); \
1:
#define SPRN_MC_SRR0 SPRN_MCSRR0
#define SPRN_MC_SRR1 SPRN_MCSRR1
--
1.7.9.5
^ permalink raw reply related
* [v3][PATCH 1/6] powerpc/book3e: load critical/machine/debug exception stack
From: Tiejun Chen @ 2013-02-27 3:09 UTC (permalink / raw)
To: benh, galak; +Cc: linuxppc-dev, linux-kernel, jason.wessel
In-Reply-To: <1361934568-31989-1-git-send-email-tiejun.chen@windriver.com>
We always alloc critical/machine/debug check exceptions. This is
different from the normal exception. So we should load these exception
stack properly like we did for booke.
Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
---
arch/powerpc/kernel/exceptions-64e.S | 49 +++++++++++++++++++++++++++++++---
1 file changed, 46 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
index 1e7782b..7fd6af0 100644
--- a/arch/powerpc/kernel/exceptions-64e.S
+++ b/arch/powerpc/kernel/exceptions-64e.S
@@ -36,6 +36,37 @@
*/
#define SPECIAL_EXC_FRAME_SIZE INT_FRAME_SIZE
+/* only on book3e */
+#define DBG_STACK_BASE dbgirq_ctx
+#define MC_STACK_BASE mcheckirq_ctx
+#define CRIT_STACK_BASE critirq_ctx
+
+#ifdef CONFIG_RELOCATABLE
+#define LOAD_STACK_BASE(reg, level) \
+ tovirt(r2,r2); \
+ LOAD_REG_ADDR(reg, level##_STACK_BASE);
+#else
+#define LOAD_STACK_BASE(reg, level) \
+ LOAD_REG_IMMEDIATE(reg, level##_STACK_BASE);
+#endif
+
+#ifdef CONFIG_SMP
+#define BOOK3E_LOAD_EXC_LEVEL_STACK(level) \
+ mfspr r14,SPRN_PIR; \
+ slwi r14,r14,3; \
+ LOAD_STACK_BASE(r10, level); \
+ add r10,r10,r14; \
+ ld r10,0(r10); \
+ addi r10,r10,THREAD_SIZE; \
+ std r10,PACA_##level##_STACK(r13);
+#else
+#define BOOK3E_LOAD_EXC_LEVEL_STACK(level) \
+ LOAD_STACK_BASE(r10, level); \
+ ld r10,0(r10); \
+ addi r10,r10,THREAD_SIZE; \
+ std r10,PACA_##level##_STACK(r13);
+#endif
+
/* Exception prolog code for all exceptions */
#define EXCEPTION_PROLOG(n, intnum, type, addition) \
mtspr SPRN_SPRG_##type##_SCRATCH,r13; /* get spare registers */ \
@@ -68,20 +99,32 @@
#define SPRN_GDBELL_SRR1 SPRN_GSRR1
#define CRIT_SET_KSTACK \
+ andi. r10,r11,MSR_PR; \
+ bne 1f; \
+ BOOK3E_LOAD_EXC_LEVEL_STACK(CRIT); \
ld r1,PACA_CRIT_STACK(r13); \
- subi r1,r1,SPECIAL_EXC_FRAME_SIZE;
+ subi r1,r1,SPECIAL_EXC_FRAME_SIZE; \
+1:
#define SPRN_CRIT_SRR0 SPRN_CSRR0
#define SPRN_CRIT_SRR1 SPRN_CSRR1
#define DBG_SET_KSTACK \
+ andi. r10,r11,MSR_PR; \
+ bne 1f; \
+ BOOK3E_LOAD_EXC_LEVEL_STACK(DBG); \
ld r1,PACA_DBG_STACK(r13); \
- subi r1,r1,SPECIAL_EXC_FRAME_SIZE;
+ subi r1,r1,SPECIAL_EXC_FRAME_SIZE; \
+1:
#define SPRN_DBG_SRR0 SPRN_DSRR0
#define SPRN_DBG_SRR1 SPRN_DSRR1
#define MC_SET_KSTACK \
+ andi. r10,r11,MSR_PR; \
+ bne 1f; \
+ BOOK3E_LOAD_EXC_LEVEL_STACK(MC); \
ld r1,PACA_MC_STACK(r13); \
- subi r1,r1,SPECIAL_EXC_FRAME_SIZE;
+ subi r1,r1,SPECIAL_EXC_FRAME_SIZE; \
+1:
#define SPRN_MC_SRR0 SPRN_MCSRR0
#define SPRN_MC_SRR1 SPRN_MCSRR1
--
1.7.9.5
^ permalink raw reply related
* [v3][PATCH 0/6] powerpc/book3e: make kgdb to work well
From: Tiejun Chen @ 2013-02-27 3:09 UTC (permalink / raw)
To: benh, galak; +Cc: linuxppc-dev, linux-kernel, jason.wessel
This patchset is used to support kgdb/gdb on book3e.
Validated on p4080ds and p5040ds with test single step and breakpoint
v3:
* make work when enable CONFIG_RELOCATABLE
* fix one typo in patch,
"powerpc/book3e: store critical/machine/debug exception thread info":
ld r1,PACAKSAVE(r13);
-> ld r14,PACAKSAVE(r13);
* remove copying the thread_info since booke and book3e always copy
the thead_info now when we enter the debug exception, and so drop
the v2 patch, "book3e/kgdb: Fix a single stgep case of lazy IRQ"
v2:
* Make sure we cover CONFIG_PPC_BOOK3E_64 safely
* Use LOAD_REG_IMMEDIATE() to load properly
the value of the constant expression in load debug exception stack
* Copy thread infor form the kernel stack coming from usr
* Rebase latest powerpc git tree
v1:
* Copy thread info only when we are from !user mode since we'll get kernel stack
coming from usr directly.
* remove save/restore EX_R14/EX_R15 since DBG_EXCEPTION_PROLOG already covered
this.
* use CURRENT_THREAD_INFO() conveniently to get thread.
* fix some typos
* add a patch to make sure gdb can generate a single step properly to invoke a
kgdb state.
* add a patch to if we need to replay an interrupt, we shouldn't restore that
previous backup thread info to make sure we can replay an interrupt lately
with a proper thread info.
* rebase latest powerpc git tree
v0:
This patchset is used to support kgdb for book3e.
------
Tiejun Chen (6):
powerpc/book3e: load critical/machine/debug exception stack
powerpc/book3e: store critical/machine/debug exception thread info
book3e/kgdb: update thread's dbcr0
powerpc/book3e: support kgdb for kernel space
kgdb/kgdbts: support ppc64
powerpc/kgdb: remove copying the thread_info
arch/powerpc/kernel/exceptions-64e.S | 69 ++++++++++++++++++++++++++++++++--
arch/powerpc/kernel/kgdb.c | 41 +++++---------------
drivers/misc/kgdbts.c | 2 +
3 files changed, 77 insertions(+), 35 deletions(-)
Tiejun
^ permalink raw reply
* Re: [v3][PATCH 0/6] powerpc/book3e: make kgdb to work well
From: tiejun.chen @ 2013-02-27 3:09 UTC (permalink / raw)
To: Tiejun Chen; +Cc: linuxppc-dev
In-Reply-To: <1361934261-31840-1-git-send-email-tiejun.chen@windriver.com>
On 02/27/2013 11:04 AM, Tiejun Chen wrote:
> This patchset is used to support kgdb/gdb on book3e.
>
> Validated on p4080ds and p5040ds with test single step and breakpoint
Please ignore this thread since looks I'm missing to CC Jason :(
Tiejun
>
> v3:
>
> * make work when enable CONFIG_RELOCATABLE
> * fix one typo in patch,
> "powerpc/book3e: store critical/machine/debug exception thread info":
> ld r1,PACAKSAVE(r13);
> -> ld r14,PACAKSAVE(r13);
> * remove copying the thread_info since booke and book3e always copy
> the thead_info now when we enter the debug exception, and so drop
> the v2 patch, "book3e/kgdb: Fix a single stgep case of lazy IRQ"
>
> v2:
>
> * Make sure we cover CONFIG_PPC_BOOK3E_64 safely
> * Use LOAD_REG_IMMEDIATE() to load properly
> the value of the constant expression in load debug exception stack
> * Copy thread infor form the kernel stack coming from usr
> * Rebase latest powerpc git tree
>
> v1:
> * Copy thread info only when we are from !user mode since we'll get kernel stack
> coming from usr directly.
> * remove save/restore EX_R14/EX_R15 since DBG_EXCEPTION_PROLOG already covered
> this.
> * use CURRENT_THREAD_INFO() conveniently to get thread.
> * fix some typos
> * add a patch to make sure gdb can generate a single step properly to invoke a
> kgdb state.
> * add a patch to if we need to replay an interrupt, we shouldn't restore that
> previous backup thread info to make sure we can replay an interrupt lately
> with a proper thread info.
> * rebase latest powerpc git tree
>
> v0:
> This patchset is used to support kgdb for book3e.
>
> ------
> Tiejun Chen (6):
> powerpc/book3e: load critical/machine/debug exception stack
> powerpc/book3e: store critical/machine/debug exception thread info
> book3e/kgdb: update thread's dbcr0
> powerpc/book3e: support kgdb for kernel space
> kgdb/kgdbts: support ppc64
> powerpc/kgdb: remove copying the thread_info
>
> arch/powerpc/kernel/exceptions-64e.S | 69 ++++++++++++++++++++++++++++++++--
> arch/powerpc/kernel/kgdb.c | 41 +++++---------------
> drivers/misc/kgdbts.c | 2 +
> 3 files changed, 77 insertions(+), 35 deletions(-)
>
> Tiejun
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
>
^ permalink raw reply
* [v3][PATCH 6/6] powerpc/kgdb: remove copying the thread_info
From: Tiejun Chen @ 2013-02-27 3:04 UTC (permalink / raw)
To: benh, galak; +Cc: linuxppc-dev
In-Reply-To: <1361934261-31840-1-git-send-email-tiejun.chen@windriver.com>
Currently BookE and Book3E always copy the thread_info from
the kernel stack when we enter the debug exception, so we can
remove these action here to avoid copying again.
Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
---
arch/powerpc/kernel/kgdb.c | 28 ----------------------------
1 file changed, 28 deletions(-)
diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c
index 1a57307..e954888 100644
--- a/arch/powerpc/kernel/kgdb.c
+++ b/arch/powerpc/kernel/kgdb.c
@@ -153,39 +153,11 @@ static int kgdb_handle_breakpoint(struct pt_regs *regs)
static int kgdb_singlestep(struct pt_regs *regs)
{
- struct thread_info *thread_info, *exception_thread_info;
- struct thread_info *backup_current_thread_info;
-
if (user_mode(regs))
return 0;
- backup_current_thread_info = (struct thread_info *)kmalloc(sizeof(struct thread_info), GFP_KERNEL);
- /*
- * On Book E and perhaps other processors, singlestep is handled on
- * the critical exception stack. This causes current_thread_info()
- * to fail, since it it locates the thread_info by masking off
- * the low bits of the current stack pointer. We work around
- * this issue by copying the thread_info from the kernel stack
- * before calling kgdb_handle_exception, and copying it back
- * afterwards. On most processors the copy is avoided since
- * exception_thread_info == thread_info.
- */
- thread_info = (struct thread_info *)(regs->gpr[1] & ~(THREAD_SIZE-1));
- exception_thread_info = current_thread_info();
-
- if (thread_info != exception_thread_info) {
- /* Save the original current_thread_info. */
- memcpy(backup_current_thread_info, exception_thread_info, sizeof *thread_info);
- memcpy(exception_thread_info, thread_info, sizeof *thread_info);
- }
-
kgdb_handle_exception(0, SIGTRAP, 0, regs);
- if (thread_info != exception_thread_info)
- /* Restore current_thread_info lastly. */
- memcpy(exception_thread_info, backup_current_thread_info, sizeof *thread_info);
-
- kfree(backup_current_thread_info);
return 1;
}
--
1.7.9.5
^ permalink raw reply related
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