* [PATCH 1/2] OMAP2+: IOMMU: change OMAP2+ error message to dev_dbg()
From: David Cohen @ 2011-02-15 14:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110215142923.GC11199@n2100.arm.linux.org.uk>
On Tue, Feb 15, 2011 at 4:29 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Tue, Feb 15, 2011 at 04:38:32PM +0300, Sergei Shtylyov wrote:
>>> diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c
>>> index 14ee686..4244a07 100644
>>> --- a/arch/arm/mach-omap2/iommu2.c
>>> +++ b/arch/arm/mach-omap2/iommu2.c
>>> @@ -163,13 +163,13 @@ static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 *ra)
>>> ? ? ?da = iommu_read_reg(obj, MMU_FAULT_AD);
>>> ? ? ?*ra = da;
>>>
>>> - ? ?dev_err(obj->dev, "%s:\tda:%08x ", __func__, da);
>>> + ? ?dev_dbg(obj->dev, "%s:\tda:%08x ", __func__, da);
>>
>> ? ?Note that dev_dbg() will only print something if either DEBUG or
>> CONFIG_DYNAMIC_DEBUG are defined...
>>
>>>
>>> ? ? ?for (i = 0; i< ?ARRAY_SIZE(err_msg); i++) {
>>> ? ? ? ? ? ? ?if (stat & (1<< ?i))
>>> - ? ? ? ? ? ? ? ? ? ?printk("%s ", err_msg[i]);
>>> + ? ? ? ? ? ? ? ? ? ?printk(KERN_DEBUG "%s ", err_msg[i]);
>>
>> ? ?... unlike printk(KERN_DEBUG...). You probably want to use pr_debug() instead.
>
> No - this isn't starting a new line. ?pr_cont() here.
But pr_cont() would be wrong in case of DEBUG isn't set, isn't it?
>
>>> ? ? ?}
>>> - ? ?printk("\n");
>>> + ? ?printk(KERN_DEBUG "\n");
>>
>> ? ?Here too... Although wait, it should be KERN_CONT instead! Debug
>> levels are only attributed to the whole lines.
>
> And pr_cont() here too. ?If you care about using KERN_CONT which is
> just a static marker to allow automated printk level checking easier.
The same situation here.
But this patch was dropped in the next version.
Br,
David
>
^ permalink raw reply
* [PATCH 1/2] OMAP2+: IOMMU: change OMAP2+ error message to dev_dbg()
From: Jarkko Nikula @ 2011-02-15 14:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <AANLkTim3+PFgJUU_-YqOt1M5NkSyHo6wUwF_XwC9MWSP@mail.gmail.com>
On Tue, 15 Feb 2011 16:08:32 +0200
David Cohen <dacohen@gmail.com> wrote:
> > So it's sure that a developer won't need these error dumps when
> > receiving an error report? I.e. IOMMU upper level errors give enough
> > information to start doing own debugging?
>
> Yes, developers do need this information.
> But it's a bit useless tell only we've got an iommu fault, due to many
> places might be causing it. My purpose is to let the debug
> responsibility to IOMMU users. They have access to the iovmm layer as
> well and can provide a much more useful information.
> e.g. OMAP3 ISP has many submodules using IOMMU. With a fault callback,
> it can dump all the iovm areas and the faulty 'da' too. It might
> indicate which submodule was responsible for the issue.
>
> Of course we can just let this debug messages the way they are and
> print this redundant information. But IMO it's not necessary.
>
Sounds fair enough and if I understood correctly this is not something
what end user will hit but more like another developer. In that case
the debug messages are the right thing.
--
Jarkko
^ permalink raw reply
* [PATCH 1/2] OMAP2+: IOMMU: change OMAP2+ error message to dev_dbg()
From: Russell King - ARM Linux @ 2011-02-15 14:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4D5A81D8.2080800@ru.mvista.com>
On Tue, Feb 15, 2011 at 04:38:32PM +0300, Sergei Shtylyov wrote:
>> diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c
>> index 14ee686..4244a07 100644
>> --- a/arch/arm/mach-omap2/iommu2.c
>> +++ b/arch/arm/mach-omap2/iommu2.c
>> @@ -163,13 +163,13 @@ static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 *ra)
>> da = iommu_read_reg(obj, MMU_FAULT_AD);
>> *ra = da;
>>
>> - dev_err(obj->dev, "%s:\tda:%08x ", __func__, da);
>> + dev_dbg(obj->dev, "%s:\tda:%08x ", __func__, da);
>
> Note that dev_dbg() will only print something if either DEBUG or
> CONFIG_DYNAMIC_DEBUG are defined...
>
>>
>> for (i = 0; i< ARRAY_SIZE(err_msg); i++) {
>> if (stat & (1<< i))
>> - printk("%s ", err_msg[i]);
>> + printk(KERN_DEBUG "%s ", err_msg[i]);
>
> ... unlike printk(KERN_DEBUG...). You probably want to use pr_debug() instead.
No - this isn't starting a new line. pr_cont() here.
>> }
>> - printk("\n");
>> + printk(KERN_DEBUG "\n");
>
> Here too... Although wait, it should be KERN_CONT instead! Debug
> levels are only attributed to the whole lines.
And pr_cont() here too. If you care about using KERN_CONT which is
just a static marker to allow automated printk level checking easier.
^ permalink raw reply
* [PATCH v4 16/19] ARM: LPAE: Use generic dma_addr_t type definition
From: Russell King - ARM Linux @ 2011-02-15 14:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1297688490.31111.38.camel@e102109-lin.cambridge.arm.com>
On Mon, Feb 14, 2011 at 01:01:30PM +0000, Catalin Marinas wrote:
> Maybe we could make the dma_addr_t size configurable (and disabled by
> default) since I expect there'll be platforms capable of >32-bit DMA.
It would be far better to fix the dma_addr_t abuses. I've already fixed
those in the pl08x driver:
struct lli {
dma_addr_t src;
dma_addr_t dst;
dma_addr_t next;
u32 cctl;
};
became:
struct pl08x_lli {
u32 src;
u32 dst;
u32 lli;
u32 cctl;
};
and similar needs to be done elsewhere in ARM specific drivers.
dma_addr_t has no business being in structures that describe data which
hardware accesses.
^ permalink raw reply
* [PATCH v4 15/19] ARM: LPAE: use phys_addr_t instead of unsigned long for physical addresses
From: Russell King - ARM Linux @ 2011-02-15 14:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1297777027.1293.2.camel@e102144-lin.cambridge.arm.com>
On Tue, Feb 15, 2011 at 01:37:07PM +0000, Will Deacon wrote:
> I should've spotted this either way. I've superseded the old patch with
> 6674/1.
One additional thing that I think has been lost. I said in the original
reply to Catalin:
| asm/memory.h will conflict non-trivially with p2v patch set, but I think
| we can merge the changes to everything but __virt_to_phys/__phys_to_virt.
So 6670/1 which I'm intending to apply to the p2v branch can't be merged
as-is. The ideal solution would be a version of 6670/1 to apply on top
of the existing p2v branch.
Thanks.
^ permalink raw reply
* [PATCH 1/2] OMAP2+: IOMMU: change OMAP2+ error message to dev_dbg()
From: David Cohen @ 2011-02-15 14:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <AANLkTim3+PFgJUU_-YqOt1M5NkSyHo6wUwF_XwC9MWSP@mail.gmail.com>
On Tue, Feb 15, 2011 at 4:08 PM, David Cohen <dacohen@gmail.com> wrote:
> On Tue, Feb 15, 2011 at 3:59 PM, Jarkko Nikula <jhnikula@gmail.com> wrote:
>> On Tue, 15 Feb 2011 15:44:27 +0200
>> David Cohen <dacohen@gmail.com> wrote:
>>
>>> >> @@ -163,13 +163,13 @@ static u32 omap2_iommu_fault_isr(struct iommu *obj,
>>> >> u32 *ra)
>>> >> ? ? ? ?da = iommu_read_reg(obj, MMU_FAULT_AD);
>>> >> ? ? ? ?*ra = da;
>>> >>
>>> >> - ? ? ? dev_err(obj->dev, "%s:\tda:%08x ", __func__, da);
>>> >> + ? ? ? dev_dbg(obj->dev, "%s:\tda:%08x ", __func__, da);
>>> >
>>> > ? Note that dev_dbg() will only print something if either DEBUG or
>>> > CONFIG_DYNAMIC_DEBUG are defined...
>>>
>>> That's my plan.
>>>
>> So it's sure that a developer won't need these error dumps when
>> receiving an error report? I.e. IOMMU upper level errors give enough
>> information to start doing own debugging?
>
> Yes, developers do need this information.
> But it's a bit useless tell only we've got an iommu fault, due to many
> places might be causing it. My purpose is to let the debug
> responsibility to IOMMU users. They have access to the iovmm layer as
> well and can provide a much more useful information.
> e.g. OMAP3 ISP has many submodules using IOMMU. With a fault callback,
> it can dump all the iovm areas and the faulty 'da' too. It might
> indicate which submodule was responsible for the issue.
>
> Of course we can just let this debug messages the way they are and
> print this redundant information. But IMO it's not necessary.
Indeed, we can leave this discussion for future. My main purpose now
is the fault callback. I'll drop this patch 1/2 for now.
Regards,
David
>
> Regards,
>
> David
>
>>
>> Just my 2 cents.
>>
>> --
>> Jarkko
>>
>
^ permalink raw reply
* [PATCH] i.MX23/28 framebuffer driver
From: Clark, Rob @ 2011-02-15 14:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201102091731.07794.arnd@arndb.de>
On Wed, Feb 9, 2011 at 10:31 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday 09 February 2011, Sascha Hauer wrote:
>
>> The driver patch itself is Cced to linux-fbdev, only the introductory
>> mail is not.
>
> Ok, I see.
>
>> > Did you consider making the driver a KMS driver instead of
>> > a frame buffer? I think the recommendation these days is
>> > to start out with KMS for new drivers, which will be somewhat
>> > simpler and give you a frame buffer device as well. I don't
>> > think that there is a need to change over any drivers from
>> > fb to kms though, since you've already done the work.
>>
>> I tried doing so for the i.MX51 which supports multiple displays on dvi
>> and vga outputs and thus could make good use of KMS and friends. Anyway,
>> I got stuck quite fast. The KMS stuff is tightly coupled with DRM/DRI
>> and needs many many callbacks to implement. Additionally the userspace
>> tools expect a nvidia/amd/intel driver and do not have a generic
>> fallback. I think this stuff is good for implementing a full blown
>> graphics driver, but is lacking support for simple framebuffer grapics.
>> I'd love to go this way but it still requires a lot of work.
>
> Ok. This sounds like a lot of upfront work indeed, to make KMS more
> generic, though I think a number of driver would benefit from it
> eventually. It could be something for the Linaro graphics working
> group to look at in the following 11.11 release, depending on how
> many other people are interested in getting there.
>
fwiw, it seems to me like xorg could have some more common code to
deal with the KMS part of DRM.. the various userspace xorg drivers end
up having a lot of very similar code to deal with enumerating
CRTCs/outputs and modes, handle hotplug, etc.
I'd been experimenting a bit on the side w/ the DRM driver framework (
http://gitorious.com/~robclark/pandaboard/robclarks-kernel-omap4/commits/omap_gpu
), but had to add a good chunk of mostly boilerplate code to our xorg
driver in order just to test it. Maybe some generic support for KMS
in xf86-video-fbdev would have made this easier to develop the kernel
part without in parallel having to implement the userspace part. I'm
not sure if this is the sort of thing the linaro-wg has in mind?
BR,
-R
^ permalink raw reply
* [PATCH v2 1/7] mmc: mxs-mmc: add mmc host driver for i.MX23/28
From: Russell King - ARM Linux @ 2011-02-15 14:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110215220311.GH10990@S2100-06.ap.freescale.net>
On Wed, Feb 16, 2011 at 06:03:12AM +0800, Shawn Guo wrote:
> On Mon, Feb 14, 2011 at 04:59:59PM +0000, Russell King - ARM Linux wrote:
> > You really should be testing for MMC_RSP_PRESENT and MMC_RSP_136
> > here. The response types are made up of a bitfield which allows
> > you to detect what's required rather than having to decode the
> > response type itself.
> >
> Please review the update.
>
> if (mmc_resp_type(cmd) & MMC_RSP_PRESENT) {
> if (mmc_resp_type(cmd) & MMC_RSP_136) {
> cmd->resp[3] = readl(host->base + HW_SSP_SDRESP0);
> cmd->resp[2] = readl(host->base + HW_SSP_SDRESP1);
> cmd->resp[1] = readl(host->base + HW_SSP_SDRESP2);
> cmd->resp[0] = readl(host->base + HW_SSP_SDRESP3);
> } else {
> cmd->resp[0] = readl(host->base + HW_SSP_SDRESP0);
> }
> }
Looks right to me.
^ permalink raw reply
* [RFC,PATCH 1/3] Add a common struct clk
From: Richard Zhao @ 2011-02-15 14:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201102151733.30332.jeremy.kerr@canonical.com>
On Tue, Feb 15, 2011 at 05:33:29PM +0800, Jeremy Kerr wrote:
> Hi Russell,
>
> > > Why is that? Consider two devices using one clock; one does some
> > > initialisation based on the return value of clk_get_rate(), the other
> > > calls clk_set_rate() some time later. Now the first device is
> > > incorrectly initialised.
> >
> > What about a clock sourced from a PLL which provides the dotclock for a
> > framebuffer device? On every mode set, should the clk have to be disabled,
> > unprepared, rate set, re-prepared and re-enabled?
>
> Sounds heavy-handed, but I honestly have no idea if that's reasonable or not.
>
> Other options are:
>
> * Require that the driver has called clk_prepare, and that prepare_count
> is 1 during the set_rate call (indicating that this is the only user); or
>
> * Leave the set_rate and set_parent semantics as-is and assume that anything
> calling either knows what it's doing (and that it won't affect other
> devices)
I vote the second option. Two reasons:
1. Has any mach specific clock restricted clk_set_rate use? I don't hear any.
2. In my opinion, clk_set_rate is not called very often by drivers, especially
for the clock which has child clocks. Leaf clock are seldom shared. Even if it's
shared, we can let drivers handle it case by case .
Thanks
Richard
>
> Are you OK if we address this separately to the API unification though?
>
> Cheers,
>
>
> Jeremy
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [RFC] ARM: dma-mapping: outer cache is invalidated twice
From: Russell King - ARM Linux @ 2011-02-15 14:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <AANLkTi=Fs5+3yG-_4u0oU8WK4K4FU_8zN38H=RNcgRNU@mail.gmail.com>
On Tue, Feb 15, 2011 at 02:54:21PM +0100, Per Forlin wrote:
> I don't fully understand this yet. I think you are right but I need a
> little help to get there myself.
> I agree, the cache (L1 and L2) must be invalidated after the DMA has completed.
> Before starting the DMA the write buffers must be drained (cache_sync).
>
> Why invalidate the cache before starting the DMA?
Think about what happens if you have dirty cache lines in the DMA region.
These can be evicted when other cache lines are loaded, which will result
in them overwriting contents of memory. If the DMA device has already
written to that memory, the result is data corruption.
So, the invalidate prior to DMA is to get rid of any dirty cache lines
which could be written back to memory.
^ permalink raw reply
* [PATCH 1/2] OMAP2+: IOMMU: change OMAP2+ error message to dev_dbg()
From: David Cohen @ 2011-02-15 14:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110215155948.32b564d5.jhnikula@gmail.com>
On Tue, Feb 15, 2011 at 3:59 PM, Jarkko Nikula <jhnikula@gmail.com> wrote:
> On Tue, 15 Feb 2011 15:44:27 +0200
> David Cohen <dacohen@gmail.com> wrote:
>
>> >> @@ -163,13 +163,13 @@ static u32 omap2_iommu_fault_isr(struct iommu *obj,
>> >> u32 *ra)
>> >> ? ? ? ?da = iommu_read_reg(obj, MMU_FAULT_AD);
>> >> ? ? ? ?*ra = da;
>> >>
>> >> - ? ? ? dev_err(obj->dev, "%s:\tda:%08x ", __func__, da);
>> >> + ? ? ? dev_dbg(obj->dev, "%s:\tda:%08x ", __func__, da);
>> >
>> > ? Note that dev_dbg() will only print something if either DEBUG or
>> > CONFIG_DYNAMIC_DEBUG are defined...
>>
>> That's my plan.
>>
> So it's sure that a developer won't need these error dumps when
> receiving an error report? I.e. IOMMU upper level errors give enough
> information to start doing own debugging?
Yes, developers do need this information.
But it's a bit useless tell only we've got an iommu fault, due to many
places might be causing it. My purpose is to let the debug
responsibility to IOMMU users. They have access to the iovmm layer as
well and can provide a much more useful information.
e.g. OMAP3 ISP has many submodules using IOMMU. With a fault callback,
it can dump all the iovm areas and the faulty 'da' too. It might
indicate which submodule was responsible for the issue.
Of course we can just let this debug messages the way they are and
print this redundant information. But IMO it's not necessary.
Regards,
David
>
> Just my 2 cents.
>
> --
> Jarkko
>
^ permalink raw reply
* [PATCH 1/2] OMAP2+: IOMMU: change OMAP2+ error message to dev_dbg()
From: David Cohen @ 2011-02-15 14:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4D5A85F4.5060103@ru.mvista.com>
On Tue, Feb 15, 2011 at 3:56 PM, Sergei Shtylyov <sshtylyov@mvista.com> wrote:
> On 15-02-2011 16:44, David Cohen wrote:
>
>>>> IOMMU upper layer is already printing error message. OMAP2+ specific
>>>> layer may print error message only for debug purpose.
>
>>>> Signed-off-by: David Cohen<dacohen@gmail.com>
>>>> ---
>>>> ?arch/arm/mach-omap2/iommu2.c | ? ?6 +++---
>>>> ?1 files changed, 3 insertions(+), 3 deletions(-)
>
>>>> diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c
>>>> index 14ee686..4244a07 100644
>>>> --- a/arch/arm/mach-omap2/iommu2.c
>>>> +++ b/arch/arm/mach-omap2/iommu2.c
>>>> @@ -163,13 +163,13 @@ static u32 omap2_iommu_fault_isr(struct iommu
>>>> *obj,
>>>> u32 *ra)
>>>> ? ? ? ?da = iommu_read_reg(obj, MMU_FAULT_AD);
>>>> ? ? ? ?*ra = da;
>>>>
>>>> - ? ? ? dev_err(obj->dev, "%s:\tda:%08x ", __func__, da);
>>>> + ? ? ? dev_dbg(obj->dev, "%s:\tda:%08x ", __func__, da);
>
>>> ? Note that dev_dbg() will only print something if either DEBUG or
>>> CONFIG_DYNAMIC_DEBUG are defined...
>
>> That's my plan.
>
>>>> ? ? ? ?for (i = 0; i < ARRAY_SIZE(err_msg); i++) {
>>>> ? ? ? ? ? ? ? ?if (stat& ?(1<< ? ?i))
>>>> - ? ? ? ? ? ? ? ? ? ? ? printk("%s ", err_msg[i]);
>>>> + ? ? ? ? ? ? ? ? ? ? ? printk(KERN_DEBUG "%s ", err_msg[i]);
>
>>> ? ... unlike printk(KERN_DEBUG...). You probably want to use pr_debug()
>>> instead.
>
>>>> ? ? ? ?}
>>>> - ? ? ? printk("\n");
>>>> + ? ? ? printk(KERN_DEBUG "\n");
>
>>> ? Here too... Although wait, it should be KERN_CONT instead! Debug levels
>>> are only attributed to the whole lines.
>
>> But your observation is correct. I'll resend it with KERN_CONT instead.
>
> ? This won't play out correctly anyway. If DEBUG is not #define'd, the
> beginning of line won't be printed but the continuations will. You just
> can't do it correctly with dev_dbg(), unless you break the single line into
> several ones.
Yes, I got this situation. I'm coming with a proper solution on next version.
Br,
David
>
>> Regards,
>
>> David
>
> WBR, Sergei
>
^ permalink raw reply
* [PATCH 1/2] OMAP2+: IOMMU: change OMAP2+ error message to dev_dbg()
From: Jarkko Nikula @ 2011-02-15 13:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <AANLkTinQX59iyX1La=99Urgh5_JF0iZacBDqQz1VQfhi@mail.gmail.com>
On Tue, 15 Feb 2011 15:44:27 +0200
David Cohen <dacohen@gmail.com> wrote:
> >> @@ -163,13 +163,13 @@ static u32 omap2_iommu_fault_isr(struct iommu *obj,
> >> u32 *ra)
> >> ? ? ? ?da = iommu_read_reg(obj, MMU_FAULT_AD);
> >> ? ? ? ?*ra = da;
> >>
> >> - ? ? ? dev_err(obj->dev, "%s:\tda:%08x ", __func__, da);
> >> + ? ? ? dev_dbg(obj->dev, "%s:\tda:%08x ", __func__, da);
> >
> > ? Note that dev_dbg() will only print something if either DEBUG or
> > CONFIG_DYNAMIC_DEBUG are defined...
>
> That's my plan.
>
So it's sure that a developer won't need these error dumps when
receiving an error report? I.e. IOMMU upper level errors give enough
information to start doing own debugging?
Just my 2 cents.
--
Jarkko
^ permalink raw reply
* [PATCH] i2c-pxa: fix unbinding/rebinding of the device
From: Dmitry Eremin-Solenikov @ 2011-02-15 13:59 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110108161828.GC8237@pengutronix.de>
Hello, all
2011/1/8 Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>:
> On Sat, Jan 08, 2011 at 07:06:44PM +0300, Dmitry Eremin-Solenikov wrote:
>> When a "pxa2xx-i2c" device was unbound (e.g. via sysfs) the remove
>> callback was not called because it was discarded when linking the
>> kernel. ?Because of that the resources allocated in probe were not
>> freed and so a clean unbind/rebind was impossible.
>>
>> Fix that by moving the remove callback to .devexit.text.
>>
>> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> Acked-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
I see this patch is still not pushed to neither linus nor linux-next.
Should I put it into rmk's patch system? Or will it find it's way to main kernel
via other paths?
>
> Thanks
> Uwe
>
> --
> Pengutronix e.K. ? ? ? ? ? ? ? ? ? ? ? ? ? | Uwe Kleine-K?nig ? ? ? ? ? ?|
> Industrial Linux Solutions ? ? ? ? ? ? ? ? | http://www.pengutronix.de/ ?|
>
--
With best wishes
Dmitry
^ permalink raw reply
* [PATCH 1/2] OMAP2+: IOMMU: change OMAP2+ error message to dev_dbg()
From: Sergei Shtylyov @ 2011-02-15 13:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <AANLkTinQX59iyX1La=99Urgh5_JF0iZacBDqQz1VQfhi@mail.gmail.com>
On 15-02-2011 16:44, David Cohen wrote:
>>> IOMMU upper layer is already printing error message. OMAP2+ specific
>>> layer may print error message only for debug purpose.
>>> Signed-off-by: David Cohen<dacohen@gmail.com>
>>> ---
>>> arch/arm/mach-omap2/iommu2.c | 6 +++---
>>> 1 files changed, 3 insertions(+), 3 deletions(-)
>>> diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c
>>> index 14ee686..4244a07 100644
>>> --- a/arch/arm/mach-omap2/iommu2.c
>>> +++ b/arch/arm/mach-omap2/iommu2.c
>>> @@ -163,13 +163,13 @@ static u32 omap2_iommu_fault_isr(struct iommu *obj,
>>> u32 *ra)
>>> da = iommu_read_reg(obj, MMU_FAULT_AD);
>>> *ra = da;
>>>
>>> - dev_err(obj->dev, "%s:\tda:%08x ", __func__, da);
>>> + dev_dbg(obj->dev, "%s:\tda:%08x ", __func__, da);
>> Note that dev_dbg() will only print something if either DEBUG or
>> CONFIG_DYNAMIC_DEBUG are defined...
> That's my plan.
>>> for (i = 0; i < ARRAY_SIZE(err_msg); i++) {
>>> if (stat& (1<< i))
>>> - printk("%s ", err_msg[i]);
>>> + printk(KERN_DEBUG "%s ", err_msg[i]);
>> ... unlike printk(KERN_DEBUG...). You probably want to use pr_debug()
>> instead.
>>> }
>>> - printk("\n");
>>> + printk(KERN_DEBUG "\n");
>> Here too... Although wait, it should be KERN_CONT instead! Debug levels
>> are only attributed to the whole lines.
> But your observation is correct. I'll resend it with KERN_CONT instead.
This won't play out correctly anyway. If DEBUG is not #define'd, the
beginning of line won't be printed but the continuations will. You just can't
do it correctly with dev_dbg(), unless you break the single line into several
ones.
> Regards,
> David
WBR, Sergei
^ permalink raw reply
* [RFC] ARM: dma-mapping: outer cache is invalidated twice
From: Per Forlin @ 2011-02-15 13:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110215134134.GA17208@n2100.arm.linux.org.uk>
On 15 February 2011 14:41, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Tue, Feb 15, 2011 at 01:32:32PM +0000, Russell King - ARM Linux wrote:
>> On Tue, Feb 15, 2011 at 02:14:55PM +0100, Per Forlin wrote:
>> > outer_inv_range () is called twice for DMA_FROM_DEVICE.
>> > The first time to "get rid of potential writebacks" and the second
>> > time to "get rid of any stale speculative prefetches"
>>
>> Correct.
>>
>> > outer_inv_range() is a rather expensive operation. In the first case
>> > isn't it enough to just call cache_sync()?
>>
>> No. ?If the CPU speculatively fetches data from the DMA buffer after
>> it's been mapped for DMA, it will bring data into the L2 cache. ?This
>> data may or may not be up to date with the DMA buffer contents once
>> DMA has completed.
>>
>> As there is no way to know, we have to invalidate the L2 cache (and the
>> L1 cache) after the DMA has completed to avoid any possibility of data
>> corruption.
>
> I should add: the solution to all of this is to have cache coherent DMA.
>
> As the CPUs become more complex and start playing tricks like speculative
> prefetching, we have seen cache maintainence for DMA becomes more expensive.
> The only way to reduce the cost of that is to have cache coherency for DMA.
>
> There is no way to safely avoid the double-invalidate for DMA_FROM_DEVICE.
>
I don't fully understand this yet. I think you are right but I need a
little help
to get there myself.
I agree, the cache (L1 and L2) must be invalidated after the DMA has completed.
Before starting the DMA the write buffers must be drained (cache_sync).
Why invalidate the cache before starting the DMA?
The user shouldn't care about the cache until the DMA has finished and
the cache is
invalidated.
I don't unsderstand how the DMA transfered data can be corrupt from a
CPU perspective
if the cache is invalidated after DMA transfer is done, but _not_
before DMA is started?
Thanks for your fast response
/Per
^ permalink raw reply
* [PATCH] ARM: PXA: Make PXA27x/PXA3xx overlay actually work
From: Vasily Khoruzhick @ 2011-02-15 13:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <AANLkTinu2LT4t=f=Y0ZBmT6=nsoiQ9HVVTYmzgLMpDXR@mail.gmail.com>
On Tuesday 15 February 2011 15:36:07 you wrote:
> Then maybe in this way? (I'd rather keep this bit in overlay specific
> code, and make it valid not only to pxa27x)
>
> @@ -925,6 +925,8 @@ static int __devinit pxafb_overlay_init(struct
> pxafb_info *fbi)
>
> /* place overlay(s) on top of base */
> fbi->lccr0 |= LCCR0_OUC;
> + lcd_writel(fbi, LCCR0, fbi->lccr0 & ~LCCR0_ENB);
> +
> pr_info("PXA Overlay driver loaded successfully!\n");
> return 0;
I tried it, it doesn't work that way (I got garbage on screen). Maybe it's not
right time to modify LCCR0 reg?
^ permalink raw reply
* [PATCH 4/8] ARM: etm: Allow range selection
From: Alexander Shishkin @ 2011-02-15 13:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1297750276-12475-4-git-send-email-arve@android.com>
On Mon, Feb 14, 2011 at 10:11:12PM -0800, Arve Hj?nnev?g wrote:
> Trace kernel text segment by default as before, allow tracing of other
> ranges by writing a range to /sys/devices/etm/trace_range, or to trace
> everything by writing 0 0.
Since you're adding this, I think it might make sense to add a 3rd optional
ctx id field, so that userspace tracing is also possible, especially when
on-chip ETB sizes increase to something more sensible than what they are
these days.
Thanks!
>
> Signed-off-by: Arve Hj?nnev?g <arve@android.com>
> ---
> arch/arm/kernel/etm.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++---
> 1 files changed, 46 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/kernel/etm.c b/arch/arm/kernel/etm.c
> index bc7d8f2..8a1c422 100644
> --- a/arch/arm/kernel/etm.c
> +++ b/arch/arm/kernel/etm.c
> @@ -40,12 +40,17 @@ struct tracectx {
> unsigned long flags;
> int ncmppairs;
> int etm_portsz;
> + unsigned long range_start;
> + unsigned long range_end;
> struct device *dev;
> struct clk *emu_clk;
> struct mutex mutex;
> };
>
> -static struct tracectx tracer;
> +static struct tracectx tracer = {
> + .range_start = (unsigned long)_stext,
> + .range_end = (unsigned long)_etext,
> +};
>
> static inline bool trace_isrunning(struct tracectx *t)
> {
> @@ -115,8 +120,12 @@ static int trace_start(struct tracectx *t)
> return -EFAULT;
> }
>
> - etm_setup_address_range(t, 1, (unsigned long)_stext,
> - (unsigned long)_etext, 0, 0);
> + if (t->range_start || t->range_end)
> + etm_setup_address_range(t, 1,
> + t->range_start, t->range_end, 0, 0);
> + else
> + etm_writel(t, ETMTE_INCLEXCL, ETMR_TRACEENCTRL);
> +
> etm_writel(t, 0, ETMR_TRACEENCTRL2);
> etm_writel(t, 0, ETMR_TRACESSCTRL);
> etm_writel(t, 0x6f, ETMR_TRACEENEVT);
> @@ -525,6 +534,35 @@ static ssize_t trace_mode_store(struct kobject *kobj,
> static struct kobj_attribute trace_mode_attr =
> __ATTR(trace_mode, 0644, trace_mode_show, trace_mode_store);
>
> +static ssize_t trace_range_show(struct kobject *kobj,
> + struct kobj_attribute *attr,
> + char *buf)
> +{
> + return sprintf(buf, "%08lx %08lx\n",
> + tracer.range_start, tracer.range_end);
> +}
> +
> +static ssize_t trace_range_store(struct kobject *kobj,
> + struct kobj_attribute *attr,
> + const char *buf, size_t n)
> +{
> + unsigned long range_start, range_end;
> +
> + if (sscanf(buf, "%lx %lx", &range_start, &range_end) != 2)
> + return -EINVAL;
> +
> + mutex_lock(&tracer.mutex);
> + tracer.range_start = range_start;
> + tracer.range_end = range_end;
> + mutex_unlock(&tracer.mutex);
> +
> + return n;
> +}
> +
> +
> +static struct kobj_attribute trace_range_attr =
> + __ATTR(trace_range, 0644, trace_range_show, trace_range_store);
> +
> static int __init etm_probe(struct amba_device *dev, struct amba_id *id)
> {
> struct tracectx *t = &tracer;
> @@ -576,6 +614,10 @@ static int __init etm_probe(struct amba_device *dev, struct amba_id *id)
> if (ret)
> dev_dbg(&dev->dev, "Failed to create trace_mode in sysfs\n");
>
> + ret = sysfs_create_file(&dev->dev.kobj, &trace_range_attr.attr);
> + if (ret)
> + dev_dbg(&dev->dev, "Failed to create trace_range in sysfs\n");
> +
> dev_dbg(t->dev, "ETM AMBA driver initialized.\n");
>
> out:
> @@ -605,6 +647,7 @@ static int etm_remove(struct amba_device *dev)
> sysfs_remove_file(&dev->dev.kobj, &trace_running_attr.attr);
> sysfs_remove_file(&dev->dev.kobj, &trace_info_attr.attr);
> sysfs_remove_file(&dev->dev.kobj, &trace_mode_attr.attr);
> + sysfs_remove_file(&dev->dev.kobj, &trace_range_attr.attr);
>
> return 0;
> }
> --
> 1.7.3.1
>
^ permalink raw reply
* [PATCH 1/2] OMAP2+: IOMMU: change OMAP2+ error message to dev_dbg()
From: David Cohen @ 2011-02-15 13:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4D5A81D8.2080800@ru.mvista.com>
On Tue, Feb 15, 2011 at 3:38 PM, Sergei Shtylyov <sshtylyov@mvista.com> wrote:
> Hello.
Hi,
>
> On 15-02-2011 16:20, David Cohen wrote:
>
>> IOMMU upper layer is already printing error message. OMAP2+ specific
>> layer may print error message only for debug purpose.
>
>> Signed-off-by: David Cohen<dacohen@gmail.com>
>> ---
>> ?arch/arm/mach-omap2/iommu2.c | ? ?6 +++---
>> ?1 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c
>> index 14ee686..4244a07 100644
>> --- a/arch/arm/mach-omap2/iommu2.c
>> +++ b/arch/arm/mach-omap2/iommu2.c
>> @@ -163,13 +163,13 @@ static u32 omap2_iommu_fault_isr(struct iommu *obj,
>> u32 *ra)
>> ? ? ? ?da = iommu_read_reg(obj, MMU_FAULT_AD);
>> ? ? ? ?*ra = da;
>>
>> - ? ? ? dev_err(obj->dev, "%s:\tda:%08x ", __func__, da);
>> + ? ? ? dev_dbg(obj->dev, "%s:\tda:%08x ", __func__, da);
>
> ? Note that dev_dbg() will only print something if either DEBUG or
> CONFIG_DYNAMIC_DEBUG are defined...
That's my plan.
>
>>
>> ? ? ? ?for (i = 0; i< ?ARRAY_SIZE(err_msg); i++) {
>> ? ? ? ? ? ? ? ?if (stat & (1<< ?i))
>> - ? ? ? ? ? ? ? ? ? ? ? printk("%s ", err_msg[i]);
>> + ? ? ? ? ? ? ? ? ? ? ? printk(KERN_DEBUG "%s ", err_msg[i]);
>
> ? ... unlike printk(KERN_DEBUG...). You probably want to use pr_debug()
> instead.
>
>> ? ? ? ?}
>> - ? ? ? printk("\n");
>> + ? ? ? printk(KERN_DEBUG "\n");
>
> ? Here too... Although wait, it should be KERN_CONT instead! Debug levels
> are only attributed to the whole lines.
But your observation is correct. I'll resend it with KERN_CONT instead.
Regards,
David
>
> WBR, Sergei
>
^ permalink raw reply
* [RFC] ARM: dma-mapping: outer cache is invalidated twice
From: Russell King - ARM Linux @ 2011-02-15 13:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20110215133232.GI4152@n2100.arm.linux.org.uk>
On Tue, Feb 15, 2011 at 01:32:32PM +0000, Russell King - ARM Linux wrote:
> On Tue, Feb 15, 2011 at 02:14:55PM +0100, Per Forlin wrote:
> > outer_inv_range () is called twice for DMA_FROM_DEVICE.
> > The first time to "get rid of potential writebacks" and the second
> > time to "get rid of any stale speculative prefetches"
>
> Correct.
>
> > outer_inv_range() is a rather expensive operation. In the first case
> > isn't it enough to just call cache_sync()?
>
> No. If the CPU speculatively fetches data from the DMA buffer after
> it's been mapped for DMA, it will bring data into the L2 cache. This
> data may or may not be up to date with the DMA buffer contents once
> DMA has completed.
>
> As there is no way to know, we have to invalidate the L2 cache (and the
> L1 cache) after the DMA has completed to avoid any possibility of data
> corruption.
I should add: the solution to all of this is to have cache coherent DMA.
As the CPUs become more complex and start playing tricks like speculative
prefetching, we have seen cache maintainence for DMA becomes more expensive.
The only way to reduce the cost of that is to have cache coherency for DMA.
There is no way to safely avoid the double-invalidate for DMA_FROM_DEVICE.
^ permalink raw reply
* [PATCH 1/2] OMAP2+: IOMMU: change OMAP2+ error message to dev_dbg()
From: Sergei Shtylyov @ 2011-02-15 13:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1297776033-8579-2-git-send-email-dacohen@gmail.com>
Hello.
On 15-02-2011 16:20, David Cohen wrote:
> IOMMU upper layer is already printing error message. OMAP2+ specific
> layer may print error message only for debug purpose.
> Signed-off-by: David Cohen<dacohen@gmail.com>
> ---
> arch/arm/mach-omap2/iommu2.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/iommu2.c b/arch/arm/mach-omap2/iommu2.c
> index 14ee686..4244a07 100644
> --- a/arch/arm/mach-omap2/iommu2.c
> +++ b/arch/arm/mach-omap2/iommu2.c
> @@ -163,13 +163,13 @@ static u32 omap2_iommu_fault_isr(struct iommu *obj, u32 *ra)
> da = iommu_read_reg(obj, MMU_FAULT_AD);
> *ra = da;
>
> - dev_err(obj->dev, "%s:\tda:%08x ", __func__, da);
> + dev_dbg(obj->dev, "%s:\tda:%08x ", __func__, da);
Note that dev_dbg() will only print something if either DEBUG or
CONFIG_DYNAMIC_DEBUG are defined...
>
> for (i = 0; i< ARRAY_SIZE(err_msg); i++) {
> if (stat & (1<< i))
> - printk("%s ", err_msg[i]);
> + printk(KERN_DEBUG "%s ", err_msg[i]);
... unlike printk(KERN_DEBUG...). You probably want to use pr_debug() instead.
> }
> - printk("\n");
> + printk(KERN_DEBUG "\n");
Here too... Although wait, it should be KERN_CONT instead! Debug levels
are only attributed to the whole lines.
WBR, Sergei
^ permalink raw reply
* [PATCH v4 15/19] ARM: LPAE: use phys_addr_t instead of unsigned long for physical addresses
From: Will Deacon @ 2011-02-15 13:37 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1297773569.14691.36.camel@e102109-lin.cambridge.arm.com>
On Tue, 2011-02-15 at 12:39 +0000, Catalin Marinas wrote:
> On Tue, 2011-02-15 at 12:35 +0000, Russell King - ARM Linux wrote:
> > On Tue, Feb 15, 2011 at 11:52:22AM +0000, Will Deacon wrote:
> > > Excellent! I've split the patch up into four distinct parts, as per your
> > > suggestions. I've submitted these to your patch system (6670/1-6673/1)
> > > alongside a fixed version of the printf format patch (6669/1) because
> > > without that, you get a bunch of compiler warnings.
> >
> > Except 6669/1 still suffers from "%#08llx". For a value of one, that prints:
> >
> > 0x000001
> >
> > five zeros following the 0x rather than seven. The width in the format
> > string includes the 0x prefix.
>
> Ah, sorry, I only fixed one case and forgot about the rest (and
> misleading Will).
>
I should've spotted this either way. I've superseded the old patch with
6674/1.
Apologies for the confusion,
Will
^ permalink raw reply
* [PATCH] ARM: PXA: Make PXA27x/PXA3xx overlay actually work
From: Eric Miao @ 2011-02-15 13:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201102151358.47203.anarsoul@gmail.com>
On Tue, Feb 15, 2011 at 7:58 PM, Vasily Khoruzhick <anarsoul@gmail.com> wrote:
> On Tuesday 15 February 2011 13:51:06 Eric Miao wrote:
>> >> > ? ? ? ?lcd_writel(fbi, FDADR0, fbi->fdadr[0]);
>> >> > - ? ? ? lcd_writel(fbi, FDADR1, fbi->fdadr[1]);
>> >> > + ? ? ? if (fbi->lccr0 & LCCR0_SDS)
>> >> > + ? ? ? ? ? ? ? lcd_writel(fbi, FDADR1, fbi->fdadr[1]);
>> >>
>> >> My original intention was to simplify the code a bit by ignoring
>> >> LCCR0_SDS, as FDADR1 would not take effect if not enabled even
>> >> if it's being read/written.
>> >
>> > It leads to potential race condition when you try to reconfigure main
>> > plane and overlay1 simultaneously.
>>
>> You are right on this.
>>
>> >> > +#ifdef CONFIG_FB_PXA_OVERLAY
>> >> > + ? ? ? if (cpu_is_pxa27x())
>> >> > + ? ? ? ? ? ? ? fbi->lccr0 |= LCCR0_OUC;
>> >> > +#endif
>> >> > +
>> >>
>> >> I seem to remember LCCR0_OUC is still valid on pxa3xx, did you
>> >> do some test on pxa3xx as well?
>> >
>> > Sorry, I have no any pxa3xx boards.
>>
>> That's all right, I can give it a test later. The point is, why
>> did you move the code here from pxafb_overlay_init()?
>
> Because otherwise correct plane order (overlays on top) will be selected only
> on next main plane reconfigure.
>
Then maybe in this way? (I'd rather keep this bit in overlay specific
code, and make it valid not only to pxa27x)
@@ -925,6 +925,8 @@ static int __devinit pxafb_overlay_init(struct
pxafb_info *fbi)
/* place overlay(s) on top of base */
fbi->lccr0 |= LCCR0_OUC;
+ lcd_writel(fbi, LCCR0, fbi->lccr0 & ~LCCR0_ENB);
+
pr_info("PXA Overlay driver loaded successfully!\n");
return 0;
> Regards
> Vasily
>
^ permalink raw reply
* [RFC] ARM: dma-mapping: outer cache is invalidated twice
From: Russell King - ARM Linux @ 2011-02-15 13:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <AANLkTi=rZJq0CPQD0Q1HMgih7UP0Tqoh9sPEYb2NEV_X@mail.gmail.com>
On Tue, Feb 15, 2011 at 02:14:55PM +0100, Per Forlin wrote:
> outer_inv_range () is called twice for DMA_FROM_DEVICE.
> The first time to "get rid of potential writebacks" and the second
> time to "get rid of any stale speculative prefetches"
Correct.
> outer_inv_range() is a rather expensive operation. In the first case
> isn't it enough to just call cache_sync()?
No. If the CPU speculatively fetches data from the DMA buffer after
it's been mapped for DMA, it will bring data into the L2 cache. This
data may or may not be up to date with the DMA buffer contents once
DMA has completed.
As there is no way to know, we have to invalidate the L2 cache (and the
L1 cache) after the DMA has completed to avoid any possibility of data
corruption.
^ permalink raw reply
* [PATCH 0/2] IOMMU fault callback support
From: David Cohen @ 2011-02-15 13:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1297776033-8579-1-git-send-email-dacohen@gmail.com>
A missing prefix in the cover letter's subject. It's: OMAP: IOMMU:
Br,
David
On Tue, Feb 15, 2011 at 3:20 PM, David Cohen <dacohen@gmail.com> wrote:
> Hi,
>
> This patch set adds fault callback support to allow IOMMU users to debug or
> react when a fault happens.
> IOMMU faults might be very difficult to reproduce and then to figure out
> the source of the problem. Currently IOMMU driver prints not so useful
> debug message and does not notice user about such issue.
> With a fault callback, IOMMU user may debug much more useful information
> and/or react to go back to a valid state.
>
> Br,
>
> David
> ---
>
> David Cohen (2):
> ?OMAP2+: IOMMU: change OMAP2+ error message to dev_dbg()
> ?OMAP: IOMMU: add support to callback during fault handling
>
> ?arch/arm/mach-omap2/iommu2.c ? ? ? ? ? ?| ? 27 ++++++++++++++++----
> ?arch/arm/plat-omap/include/plat/iommu.h | ? 15 ++++++++++-
> ?arch/arm/plat-omap/iommu.c ? ? ? ? ? ? ?| ? 41 ++++++++++++++++++++++++++++---
> ?3 files changed, 72 insertions(+), 11 deletions(-)
>
> --
> 1.7.2.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>
^ 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