All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [Qemu-devel] [patch v4 13/16] e1000: add busy flag to anti broken device state
From: Avi Kivity @ 2012-10-25  9:04 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Liu Ping Fan, Jan Kiszka, Marcelo Tosatti, qemu-devel@nongnu.org,
	liu ping fan, Anthony Liguori, Stefan Hajnoczi, Paolo Bonzini
In-Reply-To: <CAFEAcA_rXqKDXjyHe3ap8oYonOdP-2dFS8V6Cos+NnBxBvQcbg@mail.gmail.com>

On 10/25/2012 11:00 AM, Peter Maydell wrote:
> On 23 October 2012 10:37, Avi Kivity <avi@redhat.com> wrote:
>> On 10/23/2012 11:32 AM, liu ping fan wrote:
>>> On Tue, Oct 23, 2012 at 5:07 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>> On 2012-10-23 07:52, liu ping fan wrote:
>>>>> On Mon, Oct 22, 2012 at 6:40 PM, Avi Kivity <avi@redhat.com> wrote:
>>>>>> On 10/22/2012 11:23 AM, Liu Ping Fan wrote:
>>>>> It will only record and fix the issue on one thread. But guest can
>>>>> touch the emulated device on muti-threads.
>>>>
>>>> Sorry, what does that mean? A second VCPU accessing the device will
>>>> simply be ignored when it races with another VCPU? Specifically
>>>>
>>> Yes, just ignored.  For device which support many logic in parallel,
>>> it should use independent busy flag for each logic
>>
>> We don't actually know that e1000 doesn't.  Why won't writing into
>> different registers in parallel work?
> 
> Unless the device we're emulating supports multiple in
> parallel accesses (and I bet 99.9% of the devices we model
> don't) then the memory framework needs to serialise the
> loads/stores. Otherwise it's just going to be excessively
> hard to write a reliable device model.

That's why we have a per-device lock.  The busy flag breaks that model
by discarding accesses that occur in parallel.


-- 
error compiling committee.c: too many arguments to function

^ permalink raw reply

* [PATCH 3/5] pinctrl: dove: fix iomem and pdma clock
From: Sebastian Hesselbarth @ 2012-10-25  9:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CACRpkdYQ+kk1jJ-GfOBd_5pahu-KOTL4GA3_zWP85E6v+h51pA@mail.gmail.com>

On 10/25/2012 09:04 AM, Linus Walleij wrote:
> On Wed, Oct 24, 2012 at 4:25 PM, Andrew Lunn<andrew@lunn.ch>  wrote:
>
>> Since 3.7 readl/writel require register addresses to be const void*
>> instead of unsigned int. The register addresses are converted using
>> IOMEM() and offsets are added instead of OR'ed.
>> Also a workaround for the pdma clock is added, that is required as
>> there is still no DT clock provider available on Dove.
>
> So essentially two unrelated patches squashed into one, and I
> would apply the first one right off but now the clock change makes
> me hesitate.
>
>>          clk = devm_clk_get(&pdev->dev, NULL);
>> +
>> +       /* Currently there is no DT clock provider for pdma clock,
>> +          this fallback ensures pdma clock is ticking */
>
> /*
>   * I prefer comment style like so because it's easier to read.
>   * Maybe it's a bit pedantic but bear with me.
>   */
>
>> +       if (IS_ERR(clk))
>> +               clk = clk_get_sys("dove-pdma", NULL);
>> +
>
> This is a horrible hack. But if the Marvell people agree about
> it I will live with it.

Unfortunately, it is. This is an chicken-egg-problem here, no
DT clk-provider, no clocks properties..

While writing pinctrl-dove I was planing to enable it after
the clock provider but with Andrew pushing forward - for a good
reason - there comes the trouble ;)

I don't like the hack either but the clk-gate is there and
if I don't enable it pinctrl-dove will hang the SoC.

I have a clk-dove DT clk-provider in my pocket somewhere but
didn't find the time to prepare it for posting..

Sebastian

^ permalink raw reply

* Re: [PATCH 3/3] PCI: Add new default PCI-E MPS bus state
From: Yijing Wang @ 2012-10-25  9:02 UTC (permalink / raw)
  To: Jon Mason; +Cc: Bjorn Helgaas, linux-pci
In-Reply-To: <CAPoiz9xbiC5NKb5ZXGOLdMuWxnngO9HC3HfsFWX6pBwRnMbVJA@mail.gmail.com>

Hi Jon,
   I think we can do a little more about inconsistent mps problem found in boot path(BIOS configure bug for mps)
or after hotplug device.As shown in your comment on line 1451, it's unsafe to modifying the MPS on the running devices.
Since bus child devices is not running when pcie_bus_configure_settings be called. So we can try to configure child device's
mps according to the mps of bus.
there are two situations:
1、now current running bus mps is larger than child devices mpss can support;
2、now cuurent running bus mps is smaller than child devices mpss can support;

We cannot modifying the MPS for 1, it's not safe; but we can show message to user to add boot parameter pci=pcie_bus_safe.
The second situation we can modify all child devices mps as the mps value of running bus, it's safe and can correct mps problem automatically
for users.

I wrote a draft patch about this solution, if there is some thing wrong with my understanding, let me know.

Thanks very much!
Yijing


diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 5485883..59036a8 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -78,7 +78,7 @@ unsigned long pci_cardbus_mem_size = DEFAULT_CARDBUS_MEM_SIZE;
 unsigned long pci_hotplug_io_size  = DEFAULT_HOTPLUG_IO_SIZE;
 unsigned long pci_hotplug_mem_size = DEFAULT_HOTPLUG_MEM_SIZE;

-enum pcie_bus_config_types pcie_bus_config = PCIE_BUS_TUNE_OFF;
+enum pcie_bus_config_types pcie_bus_config = PCIE_BUS_AUTO;

 /*
  * The default CLS is used if arch didn't set CLS explicitly and not
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index e8b7d5e..6cbfbe1 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1467,6 +1467,19 @@ static int pcie_find_smpss(struct pci_dev *dev, void *data)
 	return 0;
 }

+static int pcie_find_min_mpss(struct pci_dev *dev, void *data)
+{
+	u8 *mpss = data;
+
+	if (!pci_is_pcie(dev))
+		return 0;
+
+	if (*mpss > dev->pcie_mpss)
+		*mpss = dev->pcie_mpss;
+
+	return 0;
+}
+
 static void pcie_write_mps(struct pci_dev *dev, int mps)
 {
 	int rc;
@@ -1560,6 +1573,7 @@ static int pcie_bus_configure_set(struct pci_dev *dev, void *data)
 void pcie_bus_configure_settings(struct pci_bus *bus, u8 mpss)
 {
 	u8 smpss;
+	u8 mps = pcie_get_mps(bus->self) >> 8;

 	if (!pci_is_pcie(bus->self))
 		return;
@@ -1581,7 +1595,19 @@ void pcie_bus_configure_settings(struct pci_bus *bus, u8 mpss)
 	case PCIE_BUS_PEER2PEER:
 		smpss = 0;
 		break;
-
+	case PCIE_BUS_AUTO:
+		smpss = bus->self->pcie_mpss;
+		pci_walk_bus(bus, pcie_find_min_mpss, &smpss);
+		if (mps > smpss) {
+			dev_info(&bus->dev,
+				"Current mps %d used in bus 0x%02x is larger than children devices mpss %d support\n"
+				"Please use the pci=pcie_bus_safe boot parameter for safe\n",
+				128 << mpss, bus->number, 128 << smpss);
+			return;
+		}
+		else
+			pci_walk_bus(bus, pcie_bus_configure_set, &mps);
+		return;
 	case PCIE_BUS_SAFE:
 		smpss = mpss;

@@ -1594,8 +1620,6 @@ void pcie_bus_configure_settings(struct pci_bus *bus, u8 mpss)
 		return;
 	}

-	}
-
 	pcie_bus_configure_set(bus->self, &smpss);
 	pci_walk_bus(bus, pcie_bus_configure_set, &smpss);
 }
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 7a451ff..539b7e4 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -2787,7 +2787,8 @@ static void __devinit quirk_intel_mc_errata(struct pci_dev *dev)
 	int err;
 	u16 rcc;

-	if (pcie_bus_config == PCIE_BUS_TUNE_OFF)
+	if (pcie_bus_config == PCIE_BUS_TUNE_OFF ||
+		pcie_bus_config == PCIE_AUTO_AUTO)
 		return;

 	/* Intel errata specifies bits to change but does not say what they are.
diff --git a/include/linux/pci.h b/include/linux/pci.h
index be1de01..84c4ab1 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -661,6 +661,7 @@ extern void pcie_bus_configure_settings(struct pci_bus *bus, u8 smpss);

 enum pcie_bus_config_types {
 	PCIE_BUS_TUNE_OFF,
+	PCIE_BUS_AUTO,
 	PCIE_BUS_SAFE,
 	PCIE_BUS_PERFORMANCE,
 	PCIE_BUS_PEER2PEER,
-- 
1.7.1


On 2012/10/24 6:57, Jon Mason wrote:
>> pcieport 0000:47:09.0 device mps current is 256 and mpss is 1024;
>> the newly hot added igb device 0000:4b:00.0 and 0000:4b:00.1 mps are 128 and mpss is 512;
>> 1、"pci_bus 0000:4b: Non-optimal PCI-E Bus MPS value of 128 being used instead of 1024."  should ".......instead of 256(bridge mps)"?
> 
> The print out shows a bug in the code (which I will push in a second
> version of the patch shortly), but having 128 instead of 256 is a
> separate issue.  That is an existing limitation due to the hotplug
> slot not being connected to the root port.  See the comment on line
> 1454.  Since PCIE_BUS_SAFE/PERFORMANCE is not being used, it is not
> ratcheting down the MPS on the bus like it should (per the comment).
> 
>> 2、“use the pci=pcie_bus_safe boot parameter for better performance”  should "....use pci=pcie_bus_safe for safe"?
> 
> The reason for the user to add the boot parameter is to get better
> performance than they would by default.  For theoretically best
> performance, they would want to use "pcie_bus_performance".  With this
> in mind, I believe "better" is the correct language.
> 
>> 3、above logs is  duplicate.
> 
> The duplicate log would only be caused by pcie_bus_configure_settings
> being called twice.  Is this only being seen on hotplug devices or on
> every device?
> 
>>
>> igb 0000:4b:00.0: enabling device (0100 -> 0102)
>> igb 0000:4b:00.0: Intel(R) Gigabit Ethernet Network Connection
>> igb 0000:4b:00.0: eth4: (PCIe:2.5Gb/s:Width x4) 00:0e:0c:ff:ff:ff
>> igb 0000:4b:00.0: eth4: PBA No: FFFFFF-0FF
>> igb 0000:4b:00.0: Using MSI-X interrupts. 8 rx queue(s), 8 tx queue(s)
>> igb 0000:4b:00.1: enabling device (0100 -> 0102)
>> GSI 63 (level, low) -> CPU 27 (0x3300) vector 137
>> igb 0000:4b:00.1: Intel(R) Gigabit Ethernet Network Connection
>> igb 0000:4b:00.1: eth4: (PCIe:2.5Gb/s:Width x4) 00:0e:0c:ff:ff:fe
>> igb 0000:4b:00.1: eth4: PBA No: FFFFFF-0FF
>> igb 0000:4b:00.1: Using MSI-X interrupts. 8 rx queue(s), 8 tx queue(s)
>>
>>
>> Thanks!
>> Yijing
>>
>>
>>> +             return;
>>>       }
>>>
>>>       pcie_bus_configure_set(bus->self, &smpss);
>>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
>>> index 5155317..e4eede0 100644
>>> --- a/drivers/pci/quirks.c
>>> +++ b/drivers/pci/quirks.c
>>> @@ -2787,7 +2787,8 @@ static void __devinit quirk_intel_mc_errata(struct pci_dev *dev)
>>>       int err;
>>>       u16 rcc;
>>>
>>> -     if (pcie_bus_config == PCIE_BUS_TUNE_OFF)
>>> +     if (pcie_bus_config == PCIE_BUS_TUNE_OFF ||
>>> +         pcie_bus_config == PCIE_BUS_WARN)
>>>               return;
>>>
>>>       /* Intel errata specifies bits to change but does not say what they are.
>>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>>> index 5faa831..410eaf9 100644
>>> --- a/include/linux/pci.h
>>> +++ b/include/linux/pci.h
>>> @@ -662,6 +662,7 @@ extern void pcie_bus_configure_settings(struct pci_bus *bus, u8 smpss);
>>>
>>>  enum pcie_bus_config_types {
>>>       PCIE_BUS_TUNE_OFF,
>>> +     PCIE_BUS_WARN,
>>>       PCIE_BUS_SAFE,
>>>       PCIE_BUS_PERFORMANCE,
>>>       PCIE_BUS_PEER2PEER,
>>>
>>
>>
>> --
>> Thanks!
>> Yijing
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> .
> 


-- 
Thanks!
Yijing


^ permalink raw reply related

* mfd: rc5t583-irq.c build warning: array subscript is above array bounds
From: Axel Lin @ 2012-10-25  9:03 UTC (permalink / raw)
  To: Laxman Dewangan; +Cc: Samuel Ortiz, linux-kernel

hi Laxman,

I got below compile warning when build this driver.

  CC      drivers/mfd/rc5t583-irq.o
drivers/mfd/rc5t583-irq.c: In function 'rc5t583_irq_sync_unlock':
drivers/mfd/rc5t583-irq.c:227: warning: array subscript is above array bounds
drivers/mfd/rc5t583-irq.c: In function 'rc5t583_irq_init':
drivers/mfd/rc5t583-irq.c:349: warning: array subscript is above array bounds

Regards,
Axel



^ permalink raw reply

* [Buildroot] [PATCH] gqview: Fix build failure due to missing -lm
From: Will Newton @ 2012-10-25  9:02 UTC (permalink / raw)
  To: buildroot
In-Reply-To: <50885F26.8000701@gmail.com>

On Wed, Oct 24, 2012 at 10:35 PM, Valentine Barshak <gvaxon@gmail.com> wrote:
> On 10/25/2012 12:51 AM, Will Newton wrote:
>>
>> On Wed, Oct 24, 2012 at 8:22 PM, Valentine Barshak <gvaxon@gmail.com>
>> wrote:
>>>
>>> On 10/07/2012 12:35 AM, Valentine Barshak wrote:
>>>>
>>>>
>>>> Signed-off-by: Valentine Barshak <gvaxon@gmail.com>
>>>> ---
>>>>    package/gqview/gqview.mk | 1 +
>>>>    1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/package/gqview/gqview.mk b/package/gqview/gqview.mk
>>>> index 2f64cd0..7d09fda 100644
>>>> --- a/package/gqview/gqview.mk
>>>> +++ b/package/gqview/gqview.mk
>>>> @@ -7,6 +7,7 @@ GQVIEW_VERSION = 2.1.5
>>>>    GQVIEW_SOURCE = gqview-$(GQVIEW_VERSION).tar.gz
>>>>    GQVIEW_SITE = http://prdownloads.sourceforge.net/gqview
>>>>    GQVIEW_DEPENDENCIES = host-pkg-config libgtk2
>>>> +GQVIEW_CONF_ENV = LIBS="-lm"
>>>>
>>>>    $(eval $(autotools-package))
>>>>
>>>>
>>>
>>> Does this work for everyone else or is it just deprecated and nobody
>>> should
>>> use it?
>>>
>>> I can't build without -lm neither in BR nor using my native host tools.
>>
>>
>> Which version of binutils are you using? 2.22?
>>
>
> Yes, 2.22.

I believe this is caused by an issue with linking indirectly with the
newer binutils. gqview relies on a library that is linked against libm
but does not explicitly link against libm itself. Before 2.22 binutils
would copy the DT_NEEDED entries from the library into gqview but it
does not do this any more.

A number of packages are affected by this. I believe the correct fix
is to modify the affected packages to link against the libraries they
use explicitly.

^ permalink raw reply

* Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000
From: Avi Kivity @ 2012-10-25  9:01 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: Liu Ping Fan, Stefan Hajnoczi, Marcelo Tosatti,
	qemu-devel@nongnu.org, liu ping fan, Anthony Liguori,
	Paolo Bonzini
In-Reply-To: <50879606.3070004@web.de>

On 10/24/2012 09:17 AM, Jan Kiszka wrote:
>>>
>>> This is ugly for many reasons. First of all, it is racy as the register
>>> content may change while dropping the device lock, no? Then you would
>>> raise or clear an IRQ spuriously.
>>>
>> Device state's intact is protected by busy flag, and will not broken
> 
> Except that the busy flag concept is broken in itself.

How do we fix an mmio that ends up mmio'ing back to itself, perhaps
indirectly?  Note this is broken in mainline too, but in a different way.

Do we introduce clever locks that can detect deadlocks?

> I see that we have a all-or-nothing problem here: to address this
> properly, we need to convert the IRQ path to lock-less (or at least
> compatible with holding per-device locks) as well.

There is a transitional path where writing to a register that can cause
IRQ changes takes both the big lock and the local lock.

Eventually, though, of course all inner subsystems must be threaded for
this work to have value.

-- 
error compiling committee.c: too many arguments to function

^ permalink raw reply

* Re: [PATCH] git-send-email: skip RFC2047 quoting for ASCII subjects
From: Jeff King @ 2012-10-25  9:01 UTC (permalink / raw)
  To: Krzysztof Mazur; +Cc: gitster, git
In-Reply-To: <20121024210826.GA23562@shrek.podlesie.net>

On Wed, Oct 24, 2012 at 11:08:26PM +0200, Krzysztof Mazur wrote:

> ok, I'm sending a version that just adds quote_subject() without
> changing any logic, so now we still have in first case:
> 
>  /[^[:ascii:]]/
> 
> and in the latter case:
>  
>  !is_rfc2047_quoted($subject) && /^[:ascii:]]/
> 
> 
> In the next patch I will just add matching for "=?" in 
> subject_needs_rfc2047_quoting() and we will have:
> 
>    /=?/ || /[^[:ascii:]]/
> 
> and in the latter case:
>  
>    !is_rfc2047_quoted($subject) && (/=\?/ || /^[:ascii:]]/)
> 
> This will also add quoting for any rfc2047 quoted subject or any
> other rfc2047-like subject, as you suggested.

Thanks, the two-patch series you outline makes a lot of sense to me.

> Krzysiek
> -- 
> From a70c5385f9b4da69a8ce00a1448f87f63bbd500d Mon Sep 17 00:00:00 2001
> From: Krzysztof Mazur <krzysiek@podlesie.net>
> Date: Wed, 24 Oct 2012 22:46:00 +0200
> Subject: [PATCH] git-send-email: introduce quote_subject()

When sending a patch following some cover letter material, please cut
out any non-essential headers and use the scissors symbol, like this:

  -- >8 --
  Subject: [PATCH] this subject overrides the whole email's subject

  the regular body and diff go here...

That format is understood by "git am" and means I do not have to
manually munge it, which saves a little work.

> +sub quote_subject {
> + 	local $subject = shift;
> + 	my $encoding = shift || 'UTF-8';
> +
> + 	if (subject_needs_rfc2047_quoting($subject)) {
> +		return quote_rfc2047($subject, $encoding);
> + 	}
> + 	return $subject;
> +}

There is some funny whitespace here (space followed by tab).

> -	if ($broken_encoding{$t} && !is_rfc2047_quoted($subject) &&
> -			($subject =~ /[^[:ascii:]]/)) {
> -		$subject = quote_rfc2047($subject, $auto_8bit_encoding);
> +	if ($broken_encoding{$t} && !is_rfc2047_quoted($subject)) {
> +		$subject = quote_subject($subject, $auto_8bit_encoding);
>  	}

Hmm. What is this patch on top of? It looks like it is on top of your
original patch, but when I tried it on top of that, it does not apply
either, and the index lines in the patch do not mention a sha1 that I do
not have.

Do you mind re-rolling a final 2-patch series with:

  1. Your original patch and this one squashed together, with an
     appropriate commit message.

  2. The second "quote when we see '=?'" patch.

Thanks.

-Peff

^ permalink raw reply

* [Buildroot] [PATCH 1/1] toolchain/uClibc: correct a slip of the pen
From: Thomas Petazzoni @ 2012-10-25  9:01 UTC (permalink / raw)
  To: buildroot
In-Reply-To: <1351153843-16204-1-git-send-email-xinglong.liao@gmail.com>

Dear Xinglong Liao,

On Thu, 25 Oct 2012 16:30:43 +0800, Xinglong Liao wrote:
> UCLIB_EXTRA_CFLAGS -> UCLIBC_EXTRA_CFLAGS
> 
> Signed-off-by: Xinglong Liao <xinglong.liao@gmail.com>

Acked-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply

* Re: [Qemu-devel] [patch v4 13/16] e1000: add busy flag to anti broken device state
From: Peter Maydell @ 2012-10-25  9:00 UTC (permalink / raw)
  To: Avi Kivity
  Cc: Liu Ping Fan, Jan Kiszka, Marcelo Tosatti, qemu-devel@nongnu.org,
	liu ping fan, Anthony Liguori, Stefan Hajnoczi, Paolo Bonzini
In-Reply-To: <5086656A.6060603@redhat.com>

On 23 October 2012 10:37, Avi Kivity <avi@redhat.com> wrote:
> On 10/23/2012 11:32 AM, liu ping fan wrote:
>> On Tue, Oct 23, 2012 at 5:07 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>> On 2012-10-23 07:52, liu ping fan wrote:
>>>> On Mon, Oct 22, 2012 at 6:40 PM, Avi Kivity <avi@redhat.com> wrote:
>>>>> On 10/22/2012 11:23 AM, Liu Ping Fan wrote:
>>>> It will only record and fix the issue on one thread. But guest can
>>>> touch the emulated device on muti-threads.
>>>
>>> Sorry, what does that mean? A second VCPU accessing the device will
>>> simply be ignored when it races with another VCPU? Specifically
>>>
>> Yes, just ignored.  For device which support many logic in parallel,
>> it should use independent busy flag for each logic
>
> We don't actually know that e1000 doesn't.  Why won't writing into
> different registers in parallel work?

Unless the device we're emulating supports multiple in
parallel accesses (and I bet 99.9% of the devices we model
don't) then the memory framework needs to serialise the
loads/stores. Otherwise it's just going to be excessively
hard to write a reliable device model.

-- PMM

^ permalink raw reply

* [PATCH] i2c: omap: ensure writes to dev->buf_len are ordered
From: Felipe Balbi @ 2012-10-25  9:00 UTC (permalink / raw)
  To: linux-arm-kernel

if we allow compiler reorder our writes, we could
fall into a situation where dev->buf_len is reset
for no apparent reason.

This bug was found with a simple script which would
transfer data to an i2c client from 1 to 1024 bytes
(a simple for loop), when we got to transfer sizes
bigger than the fifo size, dev->buf_len was reset
to zero before we had an oportunity to handle XDR
Interrupt. Because dev->buf_len was zero, we entered
omap_i2c_transmit_data() to transfer zero bytes,
which would mean we would just silently exit
omap_i2c_transmit_data() without actually writing
anything to DATA register. That would cause XDR
IRQ to trigger forever and we would never transfer
the remaining bytes.

After adding the memory barrier, we also drop resetting
dev->buf_len to zero in omap_i2c_xfer_msg() because
both omap_i2c_transmit_data() and omap_i2c_receive_data()
will act until dev->buf_len reaches zero, rendering the
other write in omap_i2c_xfer_msg() redundant.

This patch has been tested with pandaboard for a few
iterations of the script mentioned above.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---

This bug has been there forever, but it's quite annoying.
I think it deserves being pushed upstream during this -rc
cycle, but if Wolfram decides to wait until v3.8, I don't
mind.

 drivers/i2c/busses/i2c-omap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index db31eae..1ec4e6e 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -521,6 +521,7 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
 	/* REVISIT: Could the STB bit of I2C_CON be used with probing? */
 	dev->buf = msg->buf;
 	dev->buf_len = msg->len;
+	wmb();
 
 	omap_i2c_write_reg(dev, OMAP_I2C_CNT_REG, dev->buf_len);
 
@@ -579,7 +580,6 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
 	 */
 	timeout = wait_for_completion_timeout(&dev->cmd_complete,
 						OMAP_I2C_TIMEOUT);
-	dev->buf_len = 0;
 	if (timeout == 0) {
 		dev_err(dev->dev, "controller timed out\n");
 		omap_i2c_init(dev);
-- 
1.8.0

^ permalink raw reply related

* [PATCH] i2c: omap: ensure writes to dev->buf_len are ordered
From: Felipe Balbi @ 2012-10-25  9:00 UTC (permalink / raw)
  To: w.sang-bIcnvbaLZ9MEGnE8C9+IrQ, ben-linux-elnMNo+KYs3YtjvyW6yDsg
  Cc: Tony Lindgren, Linux OMAP Mailing List,
	Linux ARM Kernel Mailing List, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	Felipe Balbi

if we allow compiler reorder our writes, we could
fall into a situation where dev->buf_len is reset
for no apparent reason.

This bug was found with a simple script which would
transfer data to an i2c client from 1 to 1024 bytes
(a simple for loop), when we got to transfer sizes
bigger than the fifo size, dev->buf_len was reset
to zero before we had an oportunity to handle XDR
Interrupt. Because dev->buf_len was zero, we entered
omap_i2c_transmit_data() to transfer zero bytes,
which would mean we would just silently exit
omap_i2c_transmit_data() without actually writing
anything to DATA register. That would cause XDR
IRQ to trigger forever and we would never transfer
the remaining bytes.

After adding the memory barrier, we also drop resetting
dev->buf_len to zero in omap_i2c_xfer_msg() because
both omap_i2c_transmit_data() and omap_i2c_receive_data()
will act until dev->buf_len reaches zero, rendering the
other write in omap_i2c_xfer_msg() redundant.

This patch has been tested with pandaboard for a few
iterations of the script mentioned above.

Signed-off-by: Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
---

This bug has been there forever, but it's quite annoying.
I think it deserves being pushed upstream during this -rc
cycle, but if Wolfram decides to wait until v3.8, I don't
mind.

 drivers/i2c/busses/i2c-omap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index db31eae..1ec4e6e 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -521,6 +521,7 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
 	/* REVISIT: Could the STB bit of I2C_CON be used with probing? */
 	dev->buf = msg->buf;
 	dev->buf_len = msg->len;
+	wmb();
 
 	omap_i2c_write_reg(dev, OMAP_I2C_CNT_REG, dev->buf_len);
 
@@ -579,7 +580,6 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
 	 */
 	timeout = wait_for_completion_timeout(&dev->cmd_complete,
 						OMAP_I2C_TIMEOUT);
-	dev->buf_len = 0;
 	if (timeout == 0) {
 		dev_err(dev->dev, "controller timed out\n");
 		omap_i2c_init(dev);
-- 
1.8.0

^ permalink raw reply related

* [PATCH] x86: remove obsolete comment from asm/xen/hypervisor.h
From: Olaf Hering @ 2012-10-25  9:00 UTC (permalink / raw)
  To: Jeremy Fitzhardinge, Konrad Rzeszutek Wilk
  Cc: linux-kernel, xen-devel, Olaf Hering

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 arch/x86/include/asm/xen/hypervisor.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/x86/include/asm/xen/hypervisor.h b/arch/x86/include/asm/xen/hypervisor.h
index 66d0fff..125f344 100644
--- a/arch/x86/include/asm/xen/hypervisor.h
+++ b/arch/x86/include/asm/xen/hypervisor.h
@@ -33,7 +33,6 @@
 #ifndef _ASM_X86_XEN_HYPERVISOR_H
 #define _ASM_X86_XEN_HYPERVISOR_H
 
-/* arch/i386/kernel/setup.c */
 extern struct shared_info *HYPERVISOR_shared_info;
 extern struct start_info *xen_start_info;
 
-- 
1.7.12.4


^ permalink raw reply related

* Re: [RFT/PATCH] Input: omap4-keypad - switch to use managed resources
From: Sourav @ 2012-10-25  8:59 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input
In-Reply-To: <20121025074559.GA17710@core.coreip.homeip.net>

Hi Dmitry,
On Thursday 25 October 2012 01:16 PM, Dmitry Torokhov wrote:
> Now that input core supports devres-managed input devices we can clean
> up this driver a bit.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>
> Just compiled, no hardware to test.
Which branch are yout trying it on?
I applied this on mainline as well as on your next branch, but getting 
the following
build error..
drivers/input/keyboard/omap4-keypad.c: In function 'omap4_keypad_probe':
drivers/input/keyboard/omap4-keypad.c:340: error: implicit declaration 
of function 'devm_input_allocate_device'
drivers/input/keyboard/omap4-keypad.c:340: warning: assignment makes 
pointer from integer without a cast
make[3]: *** [drivers/input/keyboard/omap4-keypad.o] Error 1


~Sourav

>   drivers/input/keyboard/omap4-keypad.c | 182 ++++++++++++++--------------------
>   1 file changed, 76 insertions(+), 106 deletions(-)
>
> diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c
> index c05f98c..327388a 100644
> --- a/drivers/input/keyboard/omap4-keypad.c
> +++ b/drivers/input/keyboard/omap4-keypad.c
> @@ -241,17 +241,60 @@ static inline int omap4_keypad_parse_dt(struct device *dev,
>   }
>   #endif
>   
> +static int __devinit omap4_keypad_setup_regs(struct device *dev,
> +					     struct omap4_keypad *keypad_data)
> +{
> +	unsigned int rev;
> +	int retval;
> +
> +	/*
> +	 * Enable clocks for the keypad module so that we can read
> +	 * revision register.
> +	 */
> +	pm_runtime_enable(dev);
> +
> +	retval = pm_runtime_get_sync(dev);
> +	if (retval) {
> +		dev_err(dev, "%s: pm_runtime_get_sync() failed: %d\n",
> +			__func__, retval);
> +		goto out;
> +	}
> +
> +	rev = __raw_readl(keypad_data->base + OMAP4_KBD_REVISION);
> +	rev &= 0x03 << 30;
> +	rev >>= 30;
> +	switch (rev) {
> +	case KBD_REVISION_OMAP4:
> +		keypad_data->reg_offset = 0x00;
> +		keypad_data->irqreg_offset = 0x00;
> +		break;
> +	case KBD_REVISION_OMAP5:
> +		keypad_data->reg_offset = 0x10;
> +		keypad_data->irqreg_offset = 0x0c;
> +		break;
> +	default:
> +		dev_err(dev, "Keypad reports unsupported revision %d", rev);
> +		retval = -EINVAL;
> +		break;
> +	}
> +
> +	pm_runtime_put_sync(dev);
> +
> +out:
> +	pm_runtime_disable(dev);
> +	return retval;
> +}
> +
>   static int __devinit omap4_keypad_probe(struct platform_device *pdev)
>   {
> -	const struct omap4_keypad_platform_data *pdata =
> -				dev_get_platdata(&pdev->dev);
> +	struct device *dev = &pdev->dev;
> +	const struct omap4_keypad_platform_data *pdata = dev_get_platdata(dev);
>   	const struct matrix_keymap_data *keymap_data =
>   				pdata ? pdata->keymap_data : NULL;
>   	struct omap4_keypad *keypad_data;
>   	struct input_dev *input_dev;
>   	struct resource *res;
> -	unsigned int max_keys;
> -	int rev;
> +	size_t keymap_size;
>   	int irq;
>   	int error;
>   
> @@ -267,9 +310,10 @@ static int __devinit omap4_keypad_probe(struct platform_device *pdev)
>   		return -EINVAL;
>   	}
>   
> -	keypad_data = kzalloc(sizeof(struct omap4_keypad), GFP_KERNEL);
> +	keypad_data = devm_kzalloc(dev, sizeof(struct omap4_keypad),
> +				   GFP_KERNEL);
>   	if (!keypad_data) {
> -		dev_err(&pdev->dev, "keypad_data memory allocation failed\n");
> +		dev_err(dev, "keypad_data memory allocation failed\n");
>   		return -ENOMEM;
>   	}
>   
> @@ -279,64 +323,25 @@ static int __devinit omap4_keypad_probe(struct platform_device *pdev)
>   		keypad_data->rows = pdata->rows;
>   		keypad_data->cols = pdata->cols;
>   	} else {
> -		error = omap4_keypad_parse_dt(&pdev->dev, keypad_data);
> +		error = omap4_keypad_parse_dt(dev, keypad_data);
>   		if (error)
>   			return error;
>   	}
>   
> -	res = request_mem_region(res->start, resource_size(res), pdev->name);
> -	if (!res) {
> -		dev_err(&pdev->dev, "can't request mem region\n");
> -		error = -EBUSY;
> -		goto err_free_keypad;
> -	}
> -
> -	keypad_data->base = ioremap(res->start, resource_size(res));
> -	if (!keypad_data->base) {
> -		dev_err(&pdev->dev, "can't ioremap mem resource\n");
> -		error = -ENOMEM;
> -		goto err_release_mem;
> -	}
> +	keypad_data->base = devm_request_and_ioremap(dev, res);
> +	if (!keypad_data->base)
> +		return -EBUSY;
>   
> -
> -	/*
> -	 * Enable clocks for the keypad module so that we can read
> -	 * revision register.
> -	 */
> -	pm_runtime_enable(&pdev->dev);
> -	error = pm_runtime_get_sync(&pdev->dev);
> -	if (error) {
> -		dev_err(&pdev->dev, "pm_runtime_get_sync() failed\n");
> -		goto err_unmap;
> -	}
> -	rev = __raw_readl(keypad_data->base + OMAP4_KBD_REVISION);
> -	rev &= 0x03 << 30;
> -	rev >>= 30;
> -	switch (rev) {
> -	case KBD_REVISION_OMAP4:
> -		keypad_data->reg_offset = 0x00;
> -		keypad_data->irqreg_offset = 0x00;
> -		break;
> -	case KBD_REVISION_OMAP5:
> -		keypad_data->reg_offset = 0x10;
> -		keypad_data->irqreg_offset = 0x0c;
> -		break;
> -	default:
> -		dev_err(&pdev->dev,
> -			"Keypad reports unsupported revision %d", rev);
> -		error = -EINVAL;
> -		goto err_pm_put_sync;
> -	}
> +	error = omap4_keypad_setup_regs(dev, keypad_data);
> +	if (error)
> +		return error;
>   
>   	/* input device allocation */
> -	keypad_data->input = input_dev = input_allocate_device();
> -	if (!input_dev) {
> -		error = -ENOMEM;
> -		goto err_pm_put_sync;
> -	}
> +	keypad_data->input = input_dev = devm_input_allocate_device(dev);
> +	if (!input_dev)
> +		return -ENOMEM;
>   
>   	input_dev->name = pdev->name;
> -	input_dev->dev.parent = &pdev->dev;
>   	input_dev->id.bustype = BUS_HOST;
>   	input_dev->id.vendor = 0x0001;
>   	input_dev->id.product = 0x0001;
> @@ -352,81 +357,46 @@ static int __devinit omap4_keypad_probe(struct platform_device *pdev)
>   	input_set_drvdata(input_dev, keypad_data);
>   
>   	keypad_data->row_shift = get_count_order(keypad_data->cols);
> -	max_keys = keypad_data->rows << keypad_data->row_shift;
> -	keypad_data->keymap = kzalloc(max_keys * sizeof(keypad_data->keymap[0]),
> -				      GFP_KERNEL);
> +	keymap_size = (keypad_data->rows << keypad_data->row_shift) *
> +			sizeof(keypad_data->keymap[0]);
> +	keypad_data->keymap = devm_kzalloc(dev, keymap_size, GFP_KERNEL);
>   	if (!keypad_data->keymap) {
> -		dev_err(&pdev->dev, "Not enough memory for keymap\n");
> -		error = -ENOMEM;
> -		goto err_free_input;
> +		dev_err(dev, "Not enough memory for keymap\n");
> +		return -ENOMEM;
>   	}
>   
>   	error = matrix_keypad_build_keymap(keymap_data, NULL,
>   					   keypad_data->rows, keypad_data->cols,
>   					   keypad_data->keymap, input_dev);
>   	if (error) {
> -		dev_err(&pdev->dev, "failed to build keymap\n");
> -		goto err_free_keymap;
> +		dev_err(dev, "failed to build keymap\n");
> +		return error;
>   	}
>   
> -	error = request_irq(keypad_data->irq, omap4_keypad_interrupt,
> -			     IRQF_TRIGGER_RISING,
> -			     "omap4-keypad", keypad_data);
> +	error = devm_request_irq(dev, keypad_data->irq, omap4_keypad_interrupt,
> +				 IRQF_TRIGGER_RISING,
> +				 "omap4-keypad", keypad_data);
>   	if (error) {
> -		dev_err(&pdev->dev, "failed to register interrupt\n");
> -		goto err_free_input;
> +		dev_err(dev, "failed to register interrupt\n");
> +		return error;
>   	}
>   
> -	pm_runtime_put_sync(&pdev->dev);
> -
>   	error = input_register_device(keypad_data->input);
>   	if (error < 0) {
> -		dev_err(&pdev->dev, "failed to register input device\n");
> -		goto err_pm_disable;
> +		dev_err(dev, "failed to register input device\n");
> +		return error;
>   	}
>   
>   	platform_set_drvdata(pdev, keypad_data);
> -	return 0;
> +	pm_runtime_enable(dev);
>   
> -err_pm_disable:
> -	pm_runtime_disable(&pdev->dev);
> -	free_irq(keypad_data->irq, keypad_data);
> -err_free_keymap:
> -	kfree(keypad_data->keymap);
> -err_free_input:
> -	input_free_device(input_dev);
> -err_pm_put_sync:
> -	pm_runtime_put_sync(&pdev->dev);
> -err_unmap:
> -	iounmap(keypad_data->base);
> -err_release_mem:
> -	release_mem_region(res->start, resource_size(res));
> -err_free_keypad:
> -	kfree(keypad_data);
> -	return error;
> +	return 0;
>   }
>   
>   static int __devexit omap4_keypad_remove(struct platform_device *pdev)
>   {
> -	struct omap4_keypad *keypad_data = platform_get_drvdata(pdev);
> -	struct resource *res;
> -
> -	free_irq(keypad_data->irq, keypad_data);
> -
>   	pm_runtime_disable(&pdev->dev);
>   
> -	input_unregister_device(keypad_data->input);
> -
> -	iounmap(keypad_data->base);
> -
> -	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	release_mem_region(res->start, resource_size(res));
> -
> -	kfree(keypad_data->keymap);
> -	kfree(keypad_data);
> -
> -	platform_set_drvdata(pdev, NULL);
> -
>   	return 0;
>   }
>   


^ permalink raw reply

* dma_map_page and dma_map_single
From: ratheesh kannoth @ 2012-10-25  8:58 UTC (permalink / raw)
  To: e1000-devel, netdev

Hi ,

I was looking at tx_map ( igb-3.3.6  ) in igb driver. I could see that
dma_map_single is used for first buffer and dma_map_page is used for
the fragments.

What is the real difference  between these api ?  we can replace
dma_map_single with dma_map_page. then  why  we used dma_map_single
here ?.

Thanks,
Ratheesh

^ permalink raw reply

* Re: [Qemu-devel] [patch v4 07/16] memory: make mmio dispatch able to be out of biglock
From: Avi Kivity @ 2012-10-25  8:57 UTC (permalink / raw)
  To: liu ping fan
  Cc: Jan Kiszka, Marcelo Tosatti, qemu-devel@nongnu.org,
	Anthony Liguori, Stefan Hajnoczi, Paolo Bonzini
In-Reply-To: <CAJnKYQk7vbE5L6bKm40wpj2SdCwRJHscRKGLzBYHY4DRUMikTA@mail.gmail.com>

On 10/24/2012 08:56 AM, liu ping fan wrote:
>>>
>> Oh, here is a bug, need unref.  As to unbalanced refcount, it will be
>> adopted for virtio-blk listener (not implement in this patchset)
>>
> It is like cpu_physical_memory_map/unmap, the map will hold the
> unbalanced ref, and unmap release it.

Those APIs have symmetric names.  map/unmap, ref/unref, lock/unlock.
But here we have lookup and no unlookup.


-- 
error compiling committee.c: too many arguments to function

^ permalink raw reply

* [PATCH 2/5] ARM: Kirkwood: Allow use of MVEBU GPIO driver.
From: Thomas Petazzoni @ 2012-10-25  8:57 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1351088724-11142-3-git-send-email-andrew@lunn.ch>

Dear Andrew Lunn,

On Wed, 24 Oct 2012 16:25:21 +0200, Andrew Lunn wrote:

>  config GPIO_MVEBU
>  	def_bool y
> -	depends on ARCH_MVEBU
> +	depends on ARCH_MVEBU || ARCH_KIRKWOOD
>  	select GPIO_GENERIC
>  	select GENERIC_IRQ_CHIP

Should we turn the depends on ARCH_MVEBU into a depends on PLAT_ORION,
like I did for the pinctrl driver (patch accepted by Linus Walleij
yesterday) ? I think the dependency on ARCH_MVEBU was making too much
of a shortcut, by assuming that we would very quickly move everybody
into mach-mvebu. As we rather decided to move gradually
mach-{kirkwood,dove,orion5x,mv78xx0} to those infrastructure while
keeping them in their respective directories, I think a dependency on
PLAT_ORION is more appropriate here.

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply

* [Buildroot] [PATCH] netatalk: bump version
From: Maxime Hadjinlian @ 2012-10-25  8:57 UTC (permalink / raw)
  To: buildroot
In-Reply-To: <20121025085355.379e9cb3@skate>

On Thu, Oct 25, 2012 at 8:53 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Maxime Hadjinlian,
Hi Thomas,
>
> On Mon,  1 Oct 2012 18:23:36 +0200, Maxime Hadjinlian wrote:
>> Netatalk goes to version 3.0.
>> The startup script has changed too, there is now only one binary called
>> netatalk. No more afpd, cnid, ... at startup ! They are executed by netatalk.
>>
>> All the config is done within /etc/afp.conf, look at :
>> http://netatalk.sourceforge.net/3.0/htmldocs/upgrade.html
>> for more info about the upgrade process.
>> ---
>>  package/netatalk/S50netatalk |   32 +++++++++++---------------------
>>  package/netatalk/netatalk.mk |    4 ++--
>>  2 files changed, 13 insertions(+), 23 deletions(-)
>
> This update is apparently breaking the build in certain configurations:
> http://autobuild.buildroot.net/results/4fada48332abc7f9bcd49a70debbc7950bc2263a/build-end.log
>
> I see that your package as a dependency on berkeyleydb, but somehow,
> the configure script fails to find it.
>
> Can you look into it?
I will !
>
> See
> http://autobuild.buildroot.net/results/4fada48332abc7f9bcd49a70debbc7950bc2263a/
> for the full configuration files.
Thanks
>
> Thanks,
>
> Thomas
> --
> Thomas Petazzoni, Free Electrons
> Kernel, drivers, real-time and embedded Linux
> development, consulting, training and support.
> http://free-electrons.com

^ permalink raw reply

* Re: [Qemu-devel] [patch v4 13/16] e1000: add busy flag to anti broken device state
From: Avi Kivity @ 2012-10-25  8:55 UTC (permalink / raw)
  To: liu ping fan
  Cc: Liu Ping Fan, Jan Kiszka, Marcelo Tosatti, qemu-devel@nongnu.org,
	Anthony Liguori, Stefan Hajnoczi, Paolo Bonzini
In-Reply-To: <CAJnKYQnaOqTaBdGdbccdh-vdL3n2=-ej+zRu8WTidzsmbGB2_Q@mail.gmail.com>

On 10/24/2012 08:36 AM, liu ping fan wrote:
> On Tue, Oct 23, 2012 at 5:37 PM, Avi Kivity <avi@redhat.com> wrote:
>> On 10/23/2012 11:32 AM, liu ping fan wrote:
>>> On Tue, Oct 23, 2012 at 5:07 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>> On 2012-10-23 07:52, liu ping fan wrote:
>>>>> On Mon, Oct 22, 2012 at 6:40 PM, Avi Kivity <avi@redhat.com> wrote:
>>>>>> On 10/22/2012 11:23 AM, Liu Ping Fan wrote:
>>>>>>> The broken device state is caused by releasing local lock before acquiring
>>>>>>> big lock. To fix this issue, we have two choice:
>>>>>>>   1.use busy flag to protect the state
>>>>>>>     The drawback is that we will introduce independent busy flag for each
>>>>>>>     independent device's logic unit.
>>>>>>>   2.reload the device's state
>>>>>>>     The drawback is if the call chain is too deep, the action to reload will
>>>>>>>     touch each layer. Also the reloading means to recaculate the intermediate
>>>>>>>     result based on device's regs.
>>>>>>>
>>>>>>> This patch adopt the solution 1 to fix the issue.
>>>>>>
>>>>>> Doesn't the nested mmio patch detect this?
>>>>>>
>>>>> It will only record and fix the issue on one thread. But guest can
>>>>> touch the emulated device on muti-threads.
>>>>
>>>> Sorry, what does that mean? A second VCPU accessing the device will
>>>> simply be ignored when it races with another VCPU? Specifically
>>>>
>>> Yes, just ignored.  For device which support many logic in parallel,
>>> it should use independent busy flag for each logic
>>
>> We don't actually know that e1000 doesn't.  Why won't writing into
>> different registers in parallel work?
>>
> I think e1000 has only one transfer logic, so one busy flag is enough.
> And the normal guest's driver will access the registers one by one.
> But anyway, it may have parallel modules.  So what about model it like
> this
> if busy:
>   wait
> 
> clear busy:
>    wakeup
> 

You mean lock()/unlock()?

Again I suggest to ignore this issue for now.  We need to make progress
and we can't get everything perfect (or even agree on everything).  When
we have converted a few devices, we will have more information and can
think of a good solution.

-- 
error compiling committee.c: too many arguments to function

^ permalink raw reply

* [Powertop] Internationalization problem with abstract field "type"
From: Igor Zhbanov @ 2012-10-25  8:55 UTC (permalink / raw)
  To: powertop

[-- Attachment #1: Type: text/plain, Size: 1583 bytes --]

Hi Chris,

It seems that there is a problem with internationalization with the last commit.
Please look here:
- report.addf(__("Package %i"), _package->get_number());
+ report.addf(__("%s %i"), _package->get_type(), _package->get_number());

I have introduced the macro __() /* Double underscore */ to be a conditional 
variant
of classical _() /* Single underscore */ in the report refactoring commit.

Usual underscore macro calls the gettext() function. And we need some sort 
of conditional
gettext() -- we want to print untranslated strings to CSV-report. (Of 
course, we could simply
set English locale for CSV-reports. But this will look strange.)

So I have introduced the double underscore macro which calls gettext() for 
HTML-reports and
return original strings for CSV.

After your patch the gettext will try to translate the string "%s %i" and 
not the "Package %i",
so we have lost the internationalization here.

We could fix it by internationalizing the "type" field in the class:
   ret->set_type(__("Package"));

But I'm not sure that it is good to have the type depending on current 
locale (especially
if in some future there will be need to compare the value of this field by 
strcmp()).

What you think?

P.S. Why you not sending patches to mailing list for discussion? ;-)

-- 
Best regards,
Igor Zhbanov,
Technical Leader,
phone: +7 (495) 797 25 00 ext 3806
e-mail: i.zhbanov(a)samsung.com

Mobile group, Moscow R&D center, Samsung Electronics
12 Dvintsev street, building 1
127018, Moscow, Russian Federation


^ permalink raw reply

* Re: [PATCH 01/26] pstore: allow for big files
From: Tatulea, Dragos @ 2012-10-25  8:54 UTC (permalink / raw)
  To: Kees Cook
  Cc: Irina Tirdea, Anton Vorontsov, Colin Cross, Tony Luck, Chris Ball,
	linux-kernel, Adrian Hunter, Octavian Purdila
In-Reply-To: <CAGXu5jLNyOufoekZCzvc0RyjbqzShAacnMMAuNab5AozTdM+Dw@mail.gmail.com>

On Tue, Oct 23, 2012 at 7:48 PM, Kees Cook <keescook@chromium.org> wrote:
>
> On Tue, Oct 23, 2012 at 6:47 AM, Irina Tirdea <irina.tirdea@intel.com> wrote:
> > From: Adrian Hunter <adrian.hunter@intel.com>
> >
> > pstore reads all files into memory at mount time. To allow for back ends
> > that will store arbitrarily large files, enhance pstore also to be able
> > to read from the back end as needed.
> >
> > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> > Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> > ---
> >  drivers/acpi/apei/erst.c |   16 +++++++++-------
> >  fs/pstore/inode.c        |   26 ++++++++++++++++++++------
> >  fs/pstore/internal.h     |    5 +++--
> >  fs/pstore/platform.c     |   11 +++++++----
> >  fs/pstore/ram.c          |   15 +++++++--------
> >  include/linux/pstore.h   |    7 +++++--
> >  6 files changed, 51 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/acpi/apei/erst.c b/drivers/acpi/apei/erst.c
> > index e4d9d24..f70ba37 100644
> > --- a/drivers/acpi/apei/erst.c
> > +++ b/drivers/acpi/apei/erst.c
> > @@ -931,9 +931,9 @@ static int erst_check_table(struct acpi_table_erst *erst_tab)
> >
> >  static int erst_open_pstore(struct pstore_info *psi);
> >  static int erst_close_pstore(struct pstore_info *psi);
> > -static ssize_t erst_reader(u64 *id, enum pstore_type_id *type,
> > -                          struct timespec *time, char **buf,
> > -                          struct pstore_info *psi);
> > +static int erst_reader(u64 *id, enum pstore_type_id *type,
> > +                      struct timespec *time, char **buf, loff_t *size,
> > +                      struct pstore_info *psi);
> >  static int erst_writer(enum pstore_type_id type, enum kmsg_dump_reason reason,
> >                        u64 *id, unsigned int part,
> >                        size_t size, struct pstore_info *psi);
> > @@ -987,9 +987,9 @@ static int erst_close_pstore(struct pstore_info *psi)
> >         return 0;
> >  }
> >
> > -static ssize_t erst_reader(u64 *id, enum pstore_type_id *type,
> > -                          struct timespec *time, char **buf,
> > -                          struct pstore_info *psi)
> > +static int erst_reader(u64 *id, enum pstore_type_id *type,
> > +                      struct timespec *time, char **buf, loff_t *size,
> > +                      struct pstore_info *psi)
> >  {
> >         int rc;
> >         ssize_t len = 0;
> > @@ -1051,7 +1051,9 @@ skip:
> >
> >  out:
> >         kfree(rcd);
> > -       return (rc < 0) ? rc : (len - sizeof(*rcd));
> > +       if (!rc)
> > +               *size = len - sizeof(*rcd);
> > +       return rc;
> >  }
> >
> >  static int erst_writer(enum pstore_type_id type, enum kmsg_dump_reason reason,
> > diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
> > index 4ab572e..ff0970f 100644
> > --- a/fs/pstore/inode.c
> > +++ b/fs/pstore/inode.c
> > @@ -49,7 +49,8 @@ struct pstore_private {
> >         struct pstore_info *psi;
> >         enum pstore_type_id type;
> >         u64     id;
> > -       ssize_t size;
> > +       loff_t  size;
> > +       bool    big;
> >         char    data[];
> >  };
> >
> > @@ -65,12 +66,13 @@ static void *pstore_ftrace_seq_start(struct seq_file *s, loff_t *pos)
> >  {
> >         struct pstore_private *ps = s->private;
> >         struct pstore_ftrace_seq_data *data;
> > +       loff_t size = ps->size;
> >
> >         data = kzalloc(sizeof(*data), GFP_KERNEL);
> >         if (!data)
> >                 return NULL;
> >
> > -       data->off = ps->size % REC_SIZE;
> > +       data->off = do_div(size,  REC_SIZE);
>
> This modifies ps->size. I don't think you want that?
>
ps->size is not modified. The locally stored "size" variable is used
to avoid modifying ps->size.

Thanks,
Dragos

^ permalink raw reply

* Re: [PATCH] x86/MCE: Implement clearbank callback for AMD
From: Jan Beulich @ 2012-10-25  8:55 UTC (permalink / raw)
  To: Christoph Egger; +Cc: xen-devel@lists.xen.org
In-Reply-To: <5088F5BF.2090603@amd.com>

>>> On 25.10.12 at 10:18, Christoph Egger <Christoph.Egger@amd.com> wrote:
> On 10/24/12 14:18, Jan Beulich wrote:
>>>>> On 12.10.12 at 16:46, Christoph Egger <Christoph.Egger@amd.com> wrote:
>>> +static int k8_need_clearbank_scan(enum mca_source who, uint64_t status)
>>> +{
>>> +	switch (who) {
>>> +	case MCA_MCE_SCAN:
>>> +	case MCA_MCE_HANDLER:
>>> +		break;
>>> +	default:
>>> +		return 1;
>>> +	}
>>> +
>>> +	/* For fatal error, it shouldn't be cleared so that sticky bank
>>> +	 * have chance to be handled after reboot by polling.
>>> +	 */
>>> +	if ( (status & MCi_STATUS_UC) && (status & MCi_STATUS_PCC) )
>>> +		return 0;
>>> +	/* Spurious need clear bank */
>>> +	if ( !(status & MCi_STATUS_OVER)
>>> +	    && (status & MCi_STATUS_UC) && !(status & MCi_STATUS_EN))
>>> +		return 1;
>>> +
>>> +	return 1;
>>> }
>> 
>> So what's the purpose of first conditionally returning 1, and then
>> also doing so unconditionally? Do anticipate to insert code between
>> the two parts within the very near future? Otherwise I'd drop the
>> whole if() construct.
> 
> This function is derived from  intel_need_clearbank_scan().
> I just took over the relevant parts for AMD.

But that would suggest that the final return be "return 0" rather
than "return 1".

Further, the Intel code does no extra checking for the
MCA_MCE_HANDLER case, so I'd like you to confirm that this is
indeed to be different for your CPUs.

Jan

^ permalink raw reply

* [PATCH 1/5] ARM: Kirkwood: Allow use of pinctrl
From: Thomas Petazzoni @ 2012-10-25  8:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1351088724-11142-2-git-send-email-andrew@lunn.ch>

Dear Andrew Lunn,

On Wed, 24 Oct 2012 16:25:20 +0200, Andrew Lunn wrote:

> index 7bf914d..5682c96 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -188,7 +188,7 @@ config PINCTRL_EXYNOS4
>  
>  config PINCTRL_MVEBU
>  	bool
> -	depends on ARCH_MVEBU
> +	depends on ARCH_MVEBU || ARCH_KIRKWOOD
>  	select PINMUX
>  	select PINCONF

Linus Walleij has accepted a patch I have sent yesterday evening that
makes PINCTRL_MVEBU available for all PLAT_ORION platforms, so this
chunk is no longer needed.

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply

* Re: [media-workshop] Tentative Agenda for the November workshop
From: Hans Verkuil @ 2012-10-25  8:52 UTC (permalink / raw)
  To: Sylwester Nawrocki; +Cc: Laurent Pinchart, media-workshop, linux-media
In-Reply-To: <5088FBCF.5080507@samsung.com>

On Thu October 25 2012 10:43:59 Sylwester Nawrocki wrote:
> Hi Laurent,
> 
> On 10/25/2012 12:42 AM, Laurent Pinchart wrote:
> >>>> Sylwester, would Samsung be able to prepare for a brainstorming session
> >>>> on Monday or Tuesday? Both Laurent and myself have presentations on
> >>>> Wednesday, so that's not the best day for such a session.
> >>
> >> Kamil has presentation on Tuesday so there would be only Monday left.
> >>
> >>>> Do you think we should do a half-day or a full day session on this?
> >>>
> >>> Half a day should be more than enough to start with. The topic is quite
> >>> complex, and we'll need to sleep over it, several times. A full day would
> >>> just result in brain overheat. I was thinking more in the line of
> >>> starting our thought process, so maybe twice an hour or two hours would
> >>> be good. That would allow us to attend the ELCE talks as well :-)
> >>> (there's definitely a couple of them that I would like to listen to).
> >>
> >> I agree, I wouldn't like to loose whole day of the conference for that
> >> as well. One, two hours for the starters could be sufficient. So we can
> >> possibly agree on some initial idea and could get back to it later.
> >> I don't think I have already material for a full day session.
> > 
> > One hour, possibly twice, would have my preference. That shouldn't be too 
> > difficult to organize. When will you and Kamil arrive ?
> 
> Sounds good to me. We'll arrive on Sunday afternoon, and staying until
> next Sunday.

I've tried to get a small room for Monday, but they were all gone, so we will
have to find some other place in the hotel.

Regards,

	Hans

^ permalink raw reply

* Re: Performance improvements and machine build configuration
From: Elvis Dowson @ 2012-10-25  8:53 UTC (permalink / raw)
  To: Chris Tapp; +Cc: Yocto Discussion Mailing List
In-Reply-To: <CB0328DE-EE56-4CBA-A27B-43048135AE0F@keylevel.com>

Hi Chris,
                  
On Oct 23, 2012, at 11:39 PM, Chris Tapp <opensource@keylevel.com> wrote:

> On 23 Oct 2012, at 19:45, Elvis Dowson wrote:
> 
>> I noticed that between commits 
>> 
>> http://git.yoctoproject.org/cgit/cgit.cgi/poky/commit/?id=0260bb5c6978839c068007fcff2f704937805faf
>> 
>> and 
>> 
>> http://git.yoctoproject.org/cgit/cgit.cgi/poky/commit/?id=a3d5e9e6b7729319c518dcaf25bbe0643bfb25db
>> 
>> the build time has improved by around 7 minutes for my machine configuration, for building a core-image-minimal rootfs for the Xilinx ZC-702 FPGA with dual ARM Cortex A-9 CPUs.
>> 
>> commit id 0260bb5c6978839c068007fcff2f704937805faf        took 29 minutes
>> commit id a3d5e9e6b7729319c518dcaf25bbe0643bfb25db  took 22 minutes
>> 
>> The machine configuration is an Intel i7 3770K over-clocked to 4.2GHz, with 16GB RAM at 1600Mhz, two 120GB SSDs configured into a striped disk array (Intel 330 series SSDs) with a write performance of 838MB/s and read performance of around 600MB/s, in RAID0 configuration, with a Corsair HT100 liquid CPU cooler keeping the CPU cool at around 52 degree centigrade during the build process. The motherboard is a gigabyte GA-Z77X-UP5TH
>> 
>> http://www.gigabyte.com/products/product-page.aspx?pid=4279#ov
>> 
>> This motherboard has a thunderbolt display port, so I can re-use my existing Apple Thunderbolt display. I've run Ubuntu 12.04.1 LTS and Ubuntu 12.10, and it appears to work after a few tweaks.
>> 
>> The only curious thing that I've noticed is that I don't see a large performance improvement using a standard 3TB Seagate Barracuda 7200 RPM HDD, and the two Intel Series 330 SSDs in a striped RAID0 configuration. The read (600MB/s) / write (838MB/s) figures are impressive, although I expected the read performance to be higher than write performance, as is normally with a single SSD. I'm using the motherboard's hardware RAID support on a 6GB/s SATA 3 port.
>> 
>> The 3TB HDD took the approximately 2 or 3 minutes longer than the 120GB x 2 RAID0 SSD configuration for commit id 0260bb5c6978839c068007fcff2f704937805faf (31 minutes vs. 29 minutes).
>> 
>> My local.conf parallelism settings were set to 6 threads for bitbake and make, for the quad-core (virtual 8 cpu cores)system.
>> 
>> Has anyone tried yocto builds with a 6-core, 8-core or 10-core Xeon processor system? How do those figures fare? I'm thinking my current bottleneck might be the CPU and not the HDD (?!), for the yocto build workloads, which I find curious and would like to confirm. 
> 
> 
> I did quite a bit of experimenting with this a while back (similar spec, but with nearly 1000MB/s read/write SDD array). CPU was quad core with hyper-threading, so 8 virtual cores. I generally run with 16 threads, 16 parallel make as I find that the main performance hit is running out of stuff to keep all the cores busy.
> 
> Most of the time all 8 cores are maxed out, but around when the kernel gets built (and cross tools needed for it) I see the total CPU use drop to about 25%. This isn't because the system is I/O bound; it simply doesn't have enough tasks ready to run at that point in time.
> 
> I estimate that my 55 min build times would come down by 10 to 15 minutes if I could keep the CPUs busy (still, much better than the 10 hour build times on my previous system!).
> 

With the poky/master branch commit 33440ee70623394d06a4b214c2be10788cba6d08, which is the tip master branch, I tried two builds 

01. parallelism set to 16, which took 23 minutes 21 seconds.

02. parallelism set to 6, which took less time at 22 minutes 13 seconds.

Therefore, for a quad core machine (Intel i7-3770K @ 4.2GHz over-clocked, 16GB 1600MHz RAM), setting the parallelism parameters to 6 appears to be better than setting it to 16. 


Run # 01
========

BB_NUMBER_THREADS = "16"
PARALLEL_MAKE = "-j 16"

Build Configuration:
BB_VERSION        = "1.16.0"
TARGET_ARCH       = "arm"
TARGET_OS         = "linux-gnueabi"
MACHINE           = "zynq-zc702"
DISTRO            = "poky"
DISTRO_VERSION    = "1.3+snapshot-20121025"
TUNE_FEATURES     = "armv7a vfp neon cortexa9"
TARGET_FPU        = "vfp-neon"
meta              
meta-yocto        = "master:33440ee70623394d06a4b214c2be10788cba6d08"
toolchain-layer   = "master:55855cd569fbff7182974ca08b1de8435bf0f597"
meta-zynq-balister = "master-xilinx-zc702-gcc-4.7:d168cea411034d1f1530e4eacf6eb3ce4affd1c8"

NOTE: Resolving any missing task queue dependencies
NOTE: Preparing runqueue
NOTE: Executing SetScene Tasks
NOTE: Executing RunQueue Tasks
NOTE: validating kernel configuration
cat: meta/cfg/standard/zynq-zc702/specified.cfg: No such file or directory
cat: meta/cfg/standard/zynq-zc702/specified.cfg: No such file or directory
** NOTE: There were 0 required options requested that do not
         have a corresponding value present in the final ".config" file.
         This is a violation of the policy defined by the higher level config
The full list can be found in your kernel src dir at:
meta/cfg/standard/zynq-zc702/missing_required.cfg
NOTE: Tasks Summary: Attempted 1396 tasks of which 227 didn't need to be rerun and all succeeded.

real	23m21.545s
user	99m42.990s
sys	11m20.835s



Run # 02
========

BB_NUMBER_THREADS = "6"
PARALLEL_MAKE = "-j 6"

Build Configuration:
BB_VERSION        = "1.16.0"
TARGET_ARCH       = "arm"
TARGET_OS         = "linux-gnueabi"
MACHINE           = "zynq-zc702"
DISTRO            = "poky"
DISTRO_VERSION    = "1.3+snapshot-20121025"
TUNE_FEATURES     = "armv7a vfp neon cortexa9"
TARGET_FPU        = "vfp-neon"
meta              
meta-yocto        = "master:33440ee70623394d06a4b214c2be10788cba6d08"
toolchain-layer   = "master:55855cd569fbff7182974ca08b1de8435bf0f597"
meta-zynq-balister = "master-xilinx-zc702-gcc-4.7:d168cea411034d1f1530e4eacf6eb3ce4affd1c8"

NOTE: Resolving any missing task queue dependencies
NOTE: Preparing runqueue
NOTE: Executing SetScene Tasks
NOTE: Executing RunQueue Tasks
NOTE: validating kernel configuration
cat: meta/cfg/standard/zynq-zc702/specified.cfg: No such file or directory
cat: meta/cfg/standard/zynq-zc702/specified.cfg: No such file or directory
** NOTE: There were 0 required options requested that do not
         have a corresponding value present in the final ".config" file.
         This is a violation of the policy defined by the higher level config
The full list can be found in your kernel src dir at:
meta/cfg/standard/zynq-zc702/missing_required.cfg
NOTE: Tasks Summary: Attempted 1396 tasks of which 227 didn't need to be rerun and all succeeded.

real	22m13.749s
user	96m16.053s
sys	11m40.320s


Best regards,

Elvis Dowson




^ permalink raw reply

* Re: [RFC v2 0/2] vmevent: A bit reworked pressure attribute + docs + man page
From: Minchan Kim @ 2012-10-25  8:53 UTC (permalink / raw)
  To: Pekka Enberg
  Cc: Anton Vorontsov, Mel Gorman, Leonid Moiseichuk, KOSAKI Motohiro,
	Bartlomiej Zolnierkiewicz, John Stultz,
	linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linaro-kernel-cunTk1MwBs8s++Sfvej+rw,
	patches-QSEj5FYQhm4dnm+yROfE0A,
	kernel-team-z5hGa2qSFaRBDgjK7y7TUQ,
	linux-man-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAOJsxLGsjTe13WjY_Q=BLBELwQXOjuwo7PiEKwONHUfR4mQmig-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hi Pekka,

On Thu, Oct 25, 2012 at 09:44:52AM +0300, Pekka Enberg wrote:
> On Thu, Oct 25, 2012 at 9:40 AM, Minchan Kim <minchan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > Your description doesn't include why we need new vmevent_fd(2).
> > Of course, it's very flexible and potential to add new VM knob easily but
> > the thing we is about to use now is only VMEVENT_ATTR_PRESSURE.
> > Is there any other use cases for swap or free? or potential user?
> > Adding vmevent_fd without them is rather overkill.
> 
> What ABI would you use instead?

I thought /dev/some_knob like mem_notify and epoll is enough but please keep in mind
that I'm not against vmevent_fd strongly. My point is that description should include
explain about why other candidate is not good or why vmevent_fd is better.
(But at least, I don't like vmevent timer polling still and I hope we use it
as last resort once we can find another)

> 
> On Thu, Oct 25, 2012 at 9:40 AM, Minchan Kim <minchan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > I don't object but we need rationale for adding new system call which should
> > be maintained forever once we add it.
> 
> Agreed.
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo-Bw31MaZKKs0EbZ0PF+XxCw@public.gmane.org  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org"> email-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org </a>

-- 
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply


This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.