Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2 1/5] arm: mvebu: Added support for coherency fabric in mach-mvebu
From: Gregory CLEMENT @ 2012-11-15 16:49 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121115162123.GC5885@mudshark.cambridge.arm.com>

On 11/15/2012 05:21 PM, Will Deacon wrote:
> Hi Gregory,
> 
> On Thu, Nov 15, 2012 at 03:54:39PM +0000, Gregory CLEMENT wrote:
>> On 11/15/2012 11:17 AM, Will Deacon wrote:
>>> Interesting, thanks for asking them about this. Does this mean that:
>>
>> Here come the answers to your new questions
> 
> Great, thanks for the quick turn-around!
> 
>>> 	1. When not running coherently (i.e. before initialising the
>>> 	   coherency fabric), memory is treated as non-shareable,
>>> 	   non-cacheable?
>>
>> It can be cacheable. The shared memory (as defined on the page table)
>> will NOT be coherent by HW.
> 
> Ok, so we really are incoherent before enabling the fabric.
> 
>>> 	2. If (1), then are exclusive accesses the only way to achieve
>>> 	   coherent memory accesses in this scenario?
>>
>> I quote: "I suspect there is terminology miss-use: exclusive accesses
>> are NOT used to achieve memory coherency - they are used to achieve
>> atomicity. To achieve memory coherency while fabric is configured to
>> be non-coherent, SW should use maintenance operations over the L1
>> caches."
> 
> Ok, so if I'm understanding correctly then I don't really see the usefulness
> of having working exclusives that are incoherent. Surely it means that you
> can guarantee mutual exclusion on a lock variable, but the value you actually
> end up reading from the lock is junk unless you litter the accessors with cache
> clean operations?
> 
> Anyway, that's by-the-by as this is all called early enough that we
> shouldn't care. The thing I don't like now is that the fabric initialisation
> is done entirely differently on the primary CPU than the secondaries. The
> primary probes the device-tree (well, it's also now hard-coded for v2) and
> accesses the registers from a C function(armada_370_xp_set_cpu_coherent) whilst
> the secondaries have hardcoded addresses and access via asm
> (armada_xp_secondary_startup).


Now it is hardcoded in both case as you pointed it. So the last
difference is setup from a C function or via asm.

The differences between primary and secondary CPU when they enable the
coherency, is due to the fact that we really are in a different
situation. For primary CPU, as it is the only CPU online it doesn't
need to enable the coherency from the beginning, so we can wait to
have MMU enable and convenient feature. Whereas for the secondary CPU
they need the coherency from the very beginning are by definition they
won't be alone. That's why this very first instruction are written in
asm and they use physical address.

I don't see how to handle it in a different way.

Gregory

^ permalink raw reply

* Configure the USB device/host on sam9g25 module
From: Martin Peevski @ 2012-11-15 16:34 UTC (permalink / raw)
  To: linux-arm-kernel

      Hi,

      I'm working with sam9g25 module placed on sam9x5-ek. sam9g25 has 2 
host USB's and 1 device/host USB (on the micro USB of sam9x5-ek). Both 
USB hosts accept mass storage devices and works correct. It was 
impossible to do 2 things with the USB device/host (placed on the micro 
USB of sam9x5-ek):

      1. Put in it mass storage device - the kernel want not to 
enumerate it and give it an name (for example sda, sdb, ...). What I 
must do in the config file of kernel to be recognized the inserted mass 
storage device?

     2. Connect cable from the micro USB to any of the host USB's and 
detect USB serial gadget.

I tryed to make some configs for both problems in the kernel but it 
don't works. Can you, please, help me solve these problems?

^ permalink raw reply

* [PATCH] arm-dt: Enable DT proc updates.
From: Grant Likely @ 2012-11-15 16:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5091D8A8.9010307@gmail.com>

On Wed, 31 Oct 2012 21:04:24 -0500, Rob Herring <robherring2@gmail.com> wrote:
> On 10/31/2012 10:57 AM, Pantelis Antoniou wrote:
> > This simple patch enables dynamic changes of the DT tree on runtime
> > to be visible to the device-tree proc interface.
> > 
> > Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
> 
> Acked-by: Rob Herring <rob.herring@calxeda.com>

Applied, thanks.

g.

^ permalink raw reply

* [GIT PULL 1/6] omap cleanup fixes for v3.8 merge window
From: Arnd Bergmann @ 2012-11-15 16:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <pull-1352828737-887235>

On Tuesday 13 November 2012, Tony Lindgren wrote:
> The following changes since commit edf8dde393f879fc2d8c22d4bc01ff8d37b80e1a:
> 
>   Merge branch 'linus' into omap-for-v3.8/cleanup-headers-prepare-multiplatform-v3 (2012-11-09 14:58:01 -0800)
> 
> are available in the git repository at:
> 
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap tags/omap-for-v3.8/cleanup-fixes-signed

Pulled all six branches from this series, thanks!

I'll have a look at the other five next, but might not get to that today.

	Arnd

^ permalink raw reply

* [PATCH V2 1/5] arm: mvebu: Added support for coherency fabric in mach-mvebu
From: Will Deacon @ 2012-11-15 16:21 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <50A5103F.1040903@free-electrons.com>

Hi Gregory,

On Thu, Nov 15, 2012 at 03:54:39PM +0000, Gregory CLEMENT wrote:
> On 11/15/2012 11:17 AM, Will Deacon wrote:
> > Interesting, thanks for asking them about this. Does this mean that:
> 
> Here come the answers to your new questions

Great, thanks for the quick turn-around!

> > 	1. When not running coherently (i.e. before initialising the
> > 	   coherency fabric), memory is treated as non-shareable,
> > 	   non-cacheable?
> 
> It can be cacheable. The shared memory (as defined on the page table)
> will NOT be coherent by HW.

Ok, so we really are incoherent before enabling the fabric.

> > 	2. If (1), then are exclusive accesses the only way to achieve
> > 	   coherent memory accesses in this scenario?
> 
> I quote: "I suspect there is terminology miss-use: exclusive accesses
> are NOT used to achieve memory coherency - they are used to achieve
> atomicity. To achieve memory coherency while fabric is configured to
> be non-coherent, SW should use maintenance operations over the L1
> caches."

Ok, so if I'm understanding correctly then I don't really see the usefulness
of having working exclusives that are incoherent. Surely it means that you
can guarantee mutual exclusion on a lock variable, but the value you actually
end up reading from the lock is junk unless you litter the accessors with cache
clean operations?

Anyway, that's by-the-by as this is all called early enough that we
shouldn't care. The thing I don't like now is that the fabric initialisation
is done entirely differently on the primary CPU than the secondaries. The
primary probes the device-tree (well, it's also now hard-coded for v2) and
accesses the registers from a C function(armada_370_xp_set_cpu_coherent) whilst
the secondaries have hardcoded addresses and access via asm
(armada_xp_secondary_startup).

Will

^ permalink raw reply

* [RFC] dt/platform: Use cell-index for device naming if available
From: Grant Likely @ 2012-11-15 16:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <50A1B50B.4020509@codeaurora.org>

On Mon, 12 Nov 2012 18:48:43 -0800, Stepan Moskovchenko <stepanm@codeaurora.org> wrote:
> On 11/11/2012 5:45 PM, Stepan Moskovchenko wrote:
> >
> >> On Sun, Nov 11, 2012 at 2:32 AM, Rob Herring <robherring2@gmail.com>
> >> wrote:
> >>> On 11/09/2012 06:48 PM, Stepan Moskovchenko wrote:
> >>>> Use the cell-index property to construct names for platform
> >>>> devices, falling back on the existing scheme of using the
> >>>> device register address if cell-index is not specified.
> >>>>
> >>>> The cell-index property is a more useful device identifier,
> >>>> especially in systems containing several numbered instances
> >>>> of a particular hardware block, since it more easily
> >>>> illustrates how devices relate to each other.
> >>>>
> >>>> Additionally, userspace software may rely on the classic
> >>>> <name>.<id> naming scheme to access device attributes in
> >>>> sysfs, without having to know the physical addresses of
> >>>> that device on every platform the userspace software may
> >>>> support. Using cell-index for device naming allows the
> >>>> device addresses to be hidden from userspace and to be
> >>>> exposed by logical device number without having to rely on
> >>>> auxdata to perform name overrides. This allows userspace to
> >>>> make assumptions about which sysfs nodes map to which
> >>>> logical instance of a specific hardware block.
> >>>>
> >>>> Signed-off-by: Stepan Moskovchenko <stepanm@codeaurora.org>
> >>>> ---
> >>>> I had also considered using something like the linux,label property to
> >>>> allow
> >>>> custom names for platform devices without resorting to auxdata, but the
> >>>> cell-index approach seems more in line with what cell-index was
> >>>> intended for
> >>>> and with what the pre-DT platform device naming scheme used to be.
> >>>> Please let
> >>>> me know if you think there is a better way to accomplish this.
> >>>>
> >>>> This is just being sent out as an RFC for now. If there are no
> >>>> objections, I
> >>>> will send this out as an official patch, along with (or combined with)
> >>>> a patch
> >>>> to fix up the device names in things like clock tables of any affected
> >>>> platforms.
> >>>
> >>> cell-index is basically deprecated. This has been discussed multiple
> >>> times in the past. You can use auxdata if you really need to have the
> >>> old name.
> >>
> >> Actually, I think it would be fine to use an /aliases entry to set the
> >> device name. That's the place to put global namespace information.
> >>
> >> g.
> >>
> >
> > Ah, thank you. I would prefer to stay away from auxdata, since it involves
> > placing more platform-specific data into the kernel, and it is my
> > understanding that auxdata is intended as a temporary measure. The
> > /aliases approach looks interesting, and I'll see what I can do with it -
> > hopefully I can have an RFC / patch soon. It looks like we would want an
> > "inverse" alias lookup- that is, we would need to know which alias
> > corresponds to a given node. Is it possible for a node to have multiple
> > aliases?

yes

> > If so, which shall we use to create the device name? Anyway, I
> > will further look into how these aliases work.

Well, why exactly do you want to control the names of devices? Is it so
that devices match up with what they are, or is it to make things match
up with things like clocks and regulators. If it is the latter, then no,
don't do this. Use auxdata. When the kernel requires a specific name for
a device it is very much a kernel *internal* detail. It does not make
sense to encode that into the device tree when it isn't something part
of the binding.


> >
> > Steve
> 
> Hi Grant,
> 
> Looking through the alias code, I see that the stem and the alias ID are 
> stored and parsed separately. For the current way of using aliases, this 
> makes sense. However, can you please clarify what you meant by using an 
> /aliases entry to set the device name?
> 
> The first and most straightforward approach would be to use the entire 
> alias name as the device name, making no distinction between the alias 
> stem and ID. However, since it is possible to have multiple aliases to 
> the same device, which of the aliases shall we use to construct the 
> device name? Additionally, this may cause possible problems for legacy 
> software that expects names in the format of <name>.<ID>, since '.' is 
> not a valid character for alias names as defined by the DT spec, 
> although strictly speaking this approach would successfully solve the 
> problem of giving devices predictable and controllable names.
> 
> Another way an /aliases entry could be used to set the device name is to 
> have a <name>.<ID> naming scheme, where the name comes from node->name 
> (as is done in of_device_make_bus_id) and the ID gets queried using 
> of_alias_get_id(). We would need to create a new alias stem for this 
> purpose, and suppose that something like "platform" would work. The 
> name-setting code would then roughly look as follows:
> 
> +	alias_id = of_alias_get_id(node, "platform");
> +	if (alias_id != -ENODEV) {
> +		dev_set_name(dev, "%s.%d", node->name, alias_id);
> +		return;
> +	}
> 
> The downside to this approach is that it imposes the restriction that 
> device ID numbers now have to be unique throughout the system, whereas 
> before only the <name>.<ID> combinations had to be unique. This is the 
> result of only the ID number being present in the alias table, with each 
> such ID number having the "platform" stem, and the restriction that node 
> properties shall have unique names.

Again, this all looks like trying to manipulate names to keep the kernel
happy. Since the kernel has the restrictions on naming, that is where
the fixups should be made. Either by devres or by changing the expected
name in the clk/regulator tables.

> A third possible solution is to use an alias stem prefix for defining 
> the device name. That is, the alias to set the device name would have 
> some prefix (such as "platform-" for example) and the aliases would look 
> something like platform-<name><ID>. The code to assign device names 
> would find the matching alias containing the "platform-" prefix, strip 
> the prefix, and use the resulting name and ID to construct the device 
> name. This approach would make it more obvious as to which of several 
> aliases is used to set the device name, but it imposes additional 
> structure on the stem names and causes any aliases starting with 
> "platform-" to become magical, which bothers me slightly.

And specific to the current Linux implementation details.

> Do any of these describe what you intended when you suggested using the 
> /aliases node to set device names, or is there another approach that I 
> have missed? Can you please elaborate further?

Really, all I was thinking about was allowing the device that has an
alias "eth0" to be given a name with 'eth0' in it somewhere. Since names
like that are a global namespace, /aliases is the place to get them
because there is no chance of colision with that approach.

g.

^ permalink raw reply

* [PATCH] ARM: Fix errata 751472 handling on Cortex-A9 r1p*
From: Catalin Marinas @ 2012-11-15 16:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <50A50C24.9010702@gmail.com>

On Thu, Nov 15, 2012 at 03:37:08PM +0000, Rob Herring wrote:
> On 11/15/2012 08:37 AM, Catalin Marinas wrote:
> > On Thu, Nov 15, 2012 at 02:31:33PM +0000, Rob Herring wrote:
> >> Does that work for Versatile Express CA9? It needs ARM_ERRATA_751472.
> > 
> > On VE Linux runs in secure mode, so it's fine.
> 
> WTF? You are contradicting yourself.

No, it was just a statement that you can enable this workaround on VE
CA9. Didn't realise you were referring to the MULTI_PLATFORM case (it's
afternoon and coffee here not that great).

> Don't determine secure mode or not,
> but apply work-arounds only in secure mode? How does a kernel built to
> boot on secure and non-secure chips know that? The requirement would be
> that every platform have proper work-arounds setup by the bootloader
> regardless of running in secure or non-secure mode.

Yes, so for such workarounds that require secure register access just
make them depend on !MULTI_PLATFORM. Of course, VE CA9 would be affected
since neither the boot loader nor boot monitor touch those bits (yet).
But they definitely should as I don't see any other way to support
multi-platform, especially when such bits need to be set long before the
DT is parsed.

-- 
Catalin

^ permalink raw reply

* [PATCH] pinctrl: nomadik: Staticize non-exported symbols
From: Axel Lin @ 2012-11-15 16:01 UTC (permalink / raw)
  To: linux-arm-kernel

They are not referenced outside of this file, make them static.

Signed-off-by: Axel Lin <axel.lin@ingics.com>
---
 drivers/pinctrl/pinctrl-nomadik.c |   26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-nomadik.c b/drivers/pinctrl/pinctrl-nomadik.c
index 238060e..cf99e0a 100644
--- a/drivers/pinctrl/pinctrl-nomadik.c
+++ b/drivers/pinctrl/pinctrl-nomadik.c
@@ -1285,8 +1285,8 @@ void nmk_gpio_read_pull(int gpio_bank, u32 *pull_up)
 	}
 }
 
-int nmk_gpio_irq_map(struct irq_domain *d, unsigned int irq,
-			  irq_hw_number_t hwirq)
+static int nmk_gpio_irq_map(struct irq_domain *d, unsigned int irq,
+			    irq_hw_number_t hwirq)
 {
 	struct nmk_gpio_chip *nmk_chip = d->host_data;
 
@@ -1670,9 +1670,9 @@ static void nmk_pmx_disable(struct pinctrl_dev *pctldev,
 	dev_dbg(npct->dev, "disable group %s, %u pins\n", g->name, g->npins);
 }
 
-int nmk_gpio_request_enable(struct pinctrl_dev *pctldev,
-			    struct pinctrl_gpio_range *range,
-			    unsigned offset)
+static int nmk_gpio_request_enable(struct pinctrl_dev *pctldev,
+				   struct pinctrl_gpio_range *range,
+				   unsigned offset)
 {
 	struct nmk_pinctrl *npct = pinctrl_dev_get_drvdata(pctldev);
 	struct nmk_gpio_chip *nmk_chip;
@@ -1701,9 +1701,9 @@ int nmk_gpio_request_enable(struct pinctrl_dev *pctldev,
 	return 0;
 }
 
-void nmk_gpio_disable_free(struct pinctrl_dev *pctldev,
-			   struct pinctrl_gpio_range *range,
-			   unsigned offset)
+static void nmk_gpio_disable_free(struct pinctrl_dev *pctldev,
+				  struct pinctrl_gpio_range *range,
+				  unsigned offset)
 {
 	struct nmk_pinctrl *npct = pinctrl_dev_get_drvdata(pctldev);
 
@@ -1721,17 +1721,15 @@ static struct pinmux_ops nmk_pinmux_ops = {
 	.gpio_disable_free = nmk_gpio_disable_free,
 };
 
-int nmk_pin_config_get(struct pinctrl_dev *pctldev,
-		       unsigned pin,
-		       unsigned long *config)
+static int nmk_pin_config_get(struct pinctrl_dev *pctldev, unsigned pin,
+			      unsigned long *config)
 {
 	/* Not implemented */
 	return -EINVAL;
 }
 
-int nmk_pin_config_set(struct pinctrl_dev *pctldev,
-		       unsigned pin,
-		       unsigned long config)
+static int nmk_pin_config_set(struct pinctrl_dev *pctldev, unsigned pin,
+			      unsigned long config)
 {
 	static const char *pullnames[] = {
 		[NMK_GPIO_PULL_NONE]	= "none",
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH] bindings: i2c: use consistent naming for i2c binding descriptions
From: Grant Likely @ 2012-11-15 15:58 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1352827003-24287-1-git-send-email-wolfram@the-dreams.de>

On Tue, 13 Nov 2012 18:16:43 +0100, Wolfram Sang <wolfram@the-dreams.de> wrote:
> Filenames of devictree binding documentation seems to be arbitrary and
> for me it is unneeded hazzle to find the corresponding documentation for
> a specific driver.
> 
> Naming the description the same as the driver is a lot easier and makes
> sense to me since the driver defines the binding it understands.
> 
> Also, remove a reference in one source to the binding documentation, since path
> information easily gets stale.
> 
> Signed-off-by: Wolfram Sang <wolfram@the-dreams.de>
> Cc: Rob Herring <robherring2@gmail.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>

Applied, thanks.

g.

^ permalink raw reply

* [PATCH V2 1/5] arm: mvebu: Added support for coherency fabric in mach-mvebu
From: Gregory CLEMENT @ 2012-11-15 15:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121115101752.GA26453@mudshark.cambridge.arm.com>

On 11/15/2012 11:17 AM, Will Deacon wrote:
> On Wed, Nov 14, 2012 at 08:00:32PM +0000, Gregory CLEMENT wrote:
>> On 11/13/2012 11:43 AM, Will Deacon wrote:
>>> On Mon, Nov 12, 2012 at 08:21:07PM +0000, Gregory CLEMENT wrote:
>>>> On 11/05/2012 03:02 PM, Will Deacon wrote:
>>>>> These writels may expand to code containing calls to outer_sync(), which
>>>>> will attempt to take a spinlock for the aurora l2. Given that the CPU isn't
>>>>> coherent, how does this play out with the exclusive store instruction in the
>>>>> lock?
>>>>
>>>> I dug a little this subject: and I am not sure there is problem. In SMP mode,
>>>> only the system cache mode of Aurora is used. In this mode, outer_cache.sync
>>>> is void then outer_sync() won't call any function, so there will be no
>>>> access to any spinlock.
>>>
>>> Hmm, that is pretty subtle and it doesn't really solve the bigger picture.
>>> printk takes logbuf_lock, for example, and I'm sure that by the time you get
>>> to this code you will have relied on exclusives behaving correctly.
>>>
>>
>> Hi Will,
>> I get an answer from Marvell engineers:
>> "STREX on non-shareable and/or non-cacheable memory regions is supported."
> 
> Interesting, thanks for asking them about this. Does this mean that:

Here come the answers to your new questions


>
> 	1. When not running coherently (i.e. before initialising the
> 	   coherency fabric), memory is treated as non-shareable,
> 	   non-cacheable?

It can be cacheable. The shared memory (as defined on the page table)
will NOT be coherent by HW.

>
> 	2. If (1), then are exclusive accesses the only way to achieve
> 	   coherent memory accesses in this scenario?

I quote: "I suspect there is terminology miss-use: exclusive accesses
are NOT used to achieve memory coherency - they are used to achieve
atomicity. To achieve memory coherency while fabric is configured to
be non-coherent, SW should use maintenance operations over the L1
caches.suspect there is terminology miss-use: exclusive accesses are
NOT used to achieve memory coherency - "they are used to achieve
atomicity. To achieve memory coherency while fabric is configured to
be non-coherent, SW should use maintenance operations over the L1
caches.

> If so, you still have a problem with write locks, where the unlock code does
> a regular str to clear the status. atomic_{read,set} also uses regular
> memory accesses, so I think you'll get some surprises there when you add
> explicit memory barriers and expect things to be visible between threads.
>
> Do memory barriers have different semantics depending on the state of your
> coherency fabric?

No


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

^ permalink raw reply

* [PATCH 1/1] mtd: physmap_of: allow to specify the mtd name for retro compatiblity
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-11-15 15:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121115131926.47F853E0B12@localhost>

On 13:19 Thu 15 Nov     , Grant Likely wrote:
> On Sun,  4 Nov 2012 21:57:21 +0100, Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> wrote:
> > linux,mtd-name allow to specify the mtd name for retro capability with
> > physmap-flash drivers as boot loader pass the mtd partition via the old
> > device name physmap-flash.
> 
> Ummm. I don't follow. Could you explain more about how this change will
> be used?
the bootloader pass the nor flash (aka physmap) partitions via cmdline

when switching to DT the mtd name change so the kernel does not found the
partition table anymore as the mtd name is now the node name

and no I can not change the bootloader (product on the market)

Best Regards,
J.

^ permalink raw reply

* [PATCH] ARM: Fix errata 751472 handling on Cortex-A9 r1p*
From: Rob Herring @ 2012-11-15 15:37 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121115143714.GF25985@arm.com>

On 11/15/2012 08:37 AM, Catalin Marinas wrote:
> On Thu, Nov 15, 2012 at 02:31:33PM +0000, Rob Herring wrote:
>> On 11/15/2012 05:01 AM, Catalin Marinas wrote:
>>> On Thu, Nov 15, 2012 at 12:54:48AM +0000, Rob Herring wrote:
>>>> On 11/14/2012 04:21 PM, Tony Lindgren wrote:
>>>>> * Rob Herring <robherring2@gmail.com> [121114 13:59]:
>>>>>> On 11/14/2012 02:32 PM, Tony Lindgren wrote:
>>>>>>>
>>>>>>> Checking for the bit already set should work in this case, I'll post
>>>>>>> a patch for that shortly.
>>>>>>
>>>>>> Can you actually read the state of the diagnostic register in non-secure
>>>>>> mode? If you can on the A9, is the same true on A8 or others?
>>>>>
>>>>> Looks like it can be read on at least TI omap 4430 which is A9.
>>>>> But it reads as zero, so the below patch is what I came up with.
>>>>>
>>>>> No idea if assuming that zero value for the diagnostic register
>>>>> is safe.. What's the default value of the diagnostic register supposed
>>>>> to be?
>>>>
>>>> RTFM. Oh, wait it's a super secret, undocumented register. We shouldn't
>>>> even be talking about it.
>>>>
>>>> It could vary by rev, but I see 0 for the reset value, so this would not
>>>> work if the bootloader did not do any setup of the diagnostic register.
>>>>
>>>> One way to determine secure mode on the A9 would be seeing if you can
>>>> change the auxcr register. Something like this (untested):
>>>>
>>>> mrc	p15, 0, r0, c1, c0, 1; Read ACTLR
>>>> eor	r1, r0, #0x100		; Modify alloc in 1 way
>>>> mcr	p15, 0, r1, c1, c0, 1
>>>> mrc	p15, 0, r2, c1, c0, 1; Read ACTLR
>>>> mcr	p15, 0, r0, c1, c0, 1	; Restore original value
>>>> cmp	r1, r2
>>>> bne	skip_errata
>>>
>>> This would fail on platforms where Linux runs in non-secure mode. What
>>> we do for some errata workarounds is to test whether the bit was already
>>> set and avoid writing the register. But this assumes that, for a given
>>> workaround in the kernel, there is a corresponding workaround in the
>>> code running before the kernel (boot-loader, firmware) which sets that
>>> bit.
>>>
>>> Since the kernel will run more often in non-secure mode (on Cortex-A15
>>> you need this for the virtualisation extensions) I strongly suggest that
>>> the workaround (usually undocumented bit setting) is done before the
>>> kernel is started and we simply remove it from Linux (or add a clear
>>> comment that it only works if running in secure mode; if unsure say
>>> 'N').
>>>
>>> I don't think it's worth the hassle detecting whether the kernel runs in
>>> secure or non-secure mode, just assume the latter and get SoC vendors to
>>> update the boot loaders or firmware (if possible) with any errata
>>> workarounds.
>>
>> There's other places we need to know secure vs. non-secure mode like
>> whether we can do smc calls or not.
>>
>> So we should make all these work-arounds depend on !MULTI_PLATFORM then.
> 
> Only the workarounds that set bits in secure-only registers.

Right.

>> Does that work for Versatile Express CA9? It needs ARM_ERRATA_751472.
> 
> On VE Linux runs in secure mode, so it's fine.

WTF? You are contradicting yourself. Don't determine secure mode or not,
but apply work-arounds only in secure mode? How does a kernel built to
boot on secure and non-secure chips know that? The requirement would be
that every platform have proper work-arounds setup by the bootloader
regardless of running in secure or non-secure mode.

Rob

^ permalink raw reply

* [PATCH] remoteproc: fix error path of ->find_vqs
From: Ohad Ben-Cohen @ 2012-11-15 15:36 UTC (permalink / raw)
  To: linux-arm-kernel

Eliminate an erroneous invocation of rproc_shutdown inside
the error path of rproc_virtio_find_vqs.

Reported-by: Ido Yariv <ido@wizery.com>
Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
---
 drivers/remoteproc/remoteproc_virtio.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
index e7a4780..9e198e5 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -120,15 +120,11 @@ static struct virtqueue *rp_find_vq(struct virtio_device *vdev,
 	return vq;
 }
 
-static void rproc_virtio_del_vqs(struct virtio_device *vdev)
+static void __rproc_virtio_del_vqs(struct virtio_device *vdev)
 {
 	struct virtqueue *vq, *n;
-	struct rproc *rproc = vdev_to_rproc(vdev);
 	struct rproc_vring *rvring;
 
-	/* power down the remote processor before deleting vqs */
-	rproc_shutdown(rproc);
-
 	list_for_each_entry_safe(vq, n, &vdev->vqs, list) {
 		rvring = vq->priv;
 		rvring->vq = NULL;
@@ -137,6 +133,16 @@ static void rproc_virtio_del_vqs(struct virtio_device *vdev)
 	}
 }
 
+static void rproc_virtio_del_vqs(struct virtio_device *vdev)
+{
+	struct rproc *rproc = vdev_to_rproc(vdev);
+
+	/* power down the remote processor before deleting vqs */
+	rproc_shutdown(rproc);
+
+	__rproc_virtio_del_vqs(vdev);
+}
+
 static int rproc_virtio_find_vqs(struct virtio_device *vdev, unsigned nvqs,
 		       struct virtqueue *vqs[],
 		       vq_callback_t *callbacks[],
@@ -163,7 +169,7 @@ static int rproc_virtio_find_vqs(struct virtio_device *vdev, unsigned nvqs,
 	return 0;
 
 error:
-	rproc_virtio_del_vqs(vdev);
+	__rproc_virtio_del_vqs(vdev);
 	return ret;
 }
 
-- 
1.7.10.rc3.1067.gb129051

^ permalink raw reply related

* [PATCH 4/4] pinctrl: nomadik: Prevent NULL dereference if of_match_device returns NULL
From: Lee Jones @ 2012-11-15 15:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CACRpkdbWqFrP7RvXbaR5Bcz1htCL_wSqwUvOi67LmoF_PBjvhg@mail.gmail.com>

On Thu, 15 Nov 2012, Linus Walleij wrote:

> On Thu, Nov 15, 2012 at 5:56 AM, Axel Lin <axel.lin@ingics.com> wrote:
> 
> > of_match_device() may return NULL.
> >
> > Signed-off-by: Axel Lin <axel.lin@ingics.com>
> > ---
> >  drivers/pinctrl/pinctrl-nomadik.c |   11 ++++++++---
> >  1 file changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/pinctrl/pinctrl-nomadik.c b/drivers/pinctrl/pinctrl-nomadik.c
> > index 238060e..40bd1b3 100644
> > --- a/drivers/pinctrl/pinctrl-nomadik.c
> > +++ b/drivers/pinctrl/pinctrl-nomadik.c
> > @@ -1863,9 +1863,14 @@ static int __devinit nmk_pinctrl_probe(struct platform_device *pdev)
> >
> >         if (platid)
> >                 version = platid->driver_data;
> > -       else if (np)
> > -               version = (unsigned int)
> > -                       of_match_device(nmk_pinctrl_match, &pdev->dev)->data;
> > +       else if (np) {
> > +               const struct of_device_id *match;
> > +
> > +               match = of_match_device(nmk_pinctrl_match, &pdev->dev);
> > +               if (!match)
> > +                       return -ENODEV;
> > +               version = (unsigned int) match->data;
> > +       }
> 
> AFAICT this can actually happen so patch applied, unless Lee
> speaks against it.

At the very least it make the code easy to read.

Acked-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* [PATCH] ARM: Fix errata 751472 handling on Cortex-A9 r1p*
From: Måns Rullgård @ 2012-11-15 15:14 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121115135246.GB25985@arm.com>

Catalin Marinas <catalin.marinas@arm.com> writes:

> On Thu, Nov 15, 2012 at 12:41:43PM +0000, Siarhei Siamashka wrote:
>> BTW, I always wondered about what could be preventing TI and the other
>> silicon vendors from using something like an SMC API based on
>> asymmetric cryptography? My understanding is that for OMAP4 GP chips,
>> ROM code switches to non-secure mode before passing control to the
>> bootloader and there is simply no way to workaround bugs like this.
>
> AFAIK, there are some SMCs to the OMAP secure firmware that allow such
> bits to be set (see omap4_l2x0_set_debug() for example). I'm not sure
> they are documented.

The trouble with OMAP is that the secure ROM API only allows access to a
tiny subset of the registers we'd need.  In part this can be explained
by the important OMAP customers all using the HS chips with full access
to secure mode.

-- 
M?ns Rullg?rd
mans at mansr.com

^ permalink raw reply

* [PATCH] ARM: OMAP4: hwmod data: ipu and dsp to use parent clocks instead of leaf clocks
From: Omar Ramirez Luna @ 2012-11-15 15:10 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <alpine.DEB.2.00.1211141542280.16047@utopia.booyaka.com>

Hi,

On 14 November 2012 09:44, Paul Walmsley <paul@pwsan.com> wrote:
> Hi
>
> On Tue, 13 Nov 2012, Omar Ramirez Luna wrote:
>
>> This prevents hwmod _enable_clocks...omap2_dflt_clk_enable path
>> from enabling modulemode inside CLKCTRL using its clk->enable_reg
>> field. Instead is left to _omap4_enable_module though soc_ops, as
>> the one in charge of this setting.
>>
>> According to comments received[1] for related patches the idea is
>> to get rid of leaf clocks in future. So remove these two while at it.
>>
>> [1] http://lkml.org/lkml/2012/8/20/226
>>
>> Signed-off-by: Omar Ramirez Luna <omar.luna@linaro.org>
>
> Does this one belong as part of the "OMAP: iommu: hwmod, reset
> handling and runtime PM" series?  Looks like applying this one separately
> might cause these clocks not to be disabled by either the clock code's
> disable-unused-clocks code, or the hwmod code?

You are right, if applied right now, without the others it will break
the current clocks iommu code uses.

I will put it along with the other series or wait for the acceptance
of the other patches first.

Thanks,

Omar

^ permalink raw reply

* [PATCH 4/4] pinctrl: nomadik: Prevent NULL dereference if of_match_device returns NULL
From: Linus Walleij @ 2012-11-15 14:42 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1352955365.6793.6.camel@phoenix>

On Thu, Nov 15, 2012 at 5:56 AM, Axel Lin <axel.lin@ingics.com> wrote:

> of_match_device() may return NULL.
>
> Signed-off-by: Axel Lin <axel.lin@ingics.com>
> ---
>  drivers/pinctrl/pinctrl-nomadik.c |   11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-nomadik.c b/drivers/pinctrl/pinctrl-nomadik.c
> index 238060e..40bd1b3 100644
> --- a/drivers/pinctrl/pinctrl-nomadik.c
> +++ b/drivers/pinctrl/pinctrl-nomadik.c
> @@ -1863,9 +1863,14 @@ static int __devinit nmk_pinctrl_probe(struct platform_device *pdev)
>
>         if (platid)
>                 version = platid->driver_data;
> -       else if (np)
> -               version = (unsigned int)
> -                       of_match_device(nmk_pinctrl_match, &pdev->dev)->data;
> +       else if (np) {
> +               const struct of_device_id *match;
> +
> +               match = of_match_device(nmk_pinctrl_match, &pdev->dev);
> +               if (!match)
> +                       return -ENODEV;
> +               version = (unsigned int) match->data;
> +       }

AFAICT this can actually happen so patch applied, unless Lee
speaks against it.

Patches 1-3/4 are dropped though, as the maintainers didn't seem
to like them.

Yours,
Linus Walleij

^ permalink raw reply

* [PATCH] ARM: add get_user() support for 8 byte types
From: Arnd Bergmann @ 2012-11-15 14:39 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAF6AEGuJReNtwUZX6HVZCE8GHuyEAyC8bM0GqZuVMV1=DrGJVw@mail.gmail.com>

On Thursday 15 November 2012, Rob Clark wrote:
> > I still haven't heard a conclusive argument why we need to use get_user()
> > rather than copy_from_user() in the DRM code. Is this about a fast path
> > where you want to shave off a few cycles for each call, or does this
> > simplify the code structure, or something else?
> 
> well, it is mostly because it seemed like a good idea to first try to
> solve the root issue, rather than having to fix things up in each
> driver when someone from x86-world introduces a 64b get_user()..

As pointed out by hpa earlier, x86-32 doesn't have a 64b get_user
either. I don't think we have a lot of drivers that are used only
on 64-bit x86 and on 32-bit ARM but not on 32-bit x86.

> There are some other arch's that don't have 64b get_user(), but I
> don't think any that have any DRM drivers.  If 64b get_user() is
> really not intended to be supported by all archs, it is better to
> remove it from x86 and the other arch's that do currently support it,
> rather than making it possible to write drivers that are broken on
> some archs.

The majority of architectures we support have PCI and should be able
to build the regular (radeon, nouveau, MGA, VIA, ...) DRM drivers
AFAICT.

	Arnd

^ permalink raw reply

* [PATCH 1/2] Boottime: A tool for automatic measurement of kernel/bootloader boot time
From: Lee Jones @ 2012-11-15 14:37 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAPcvp5ER_rMnyzbKR8r8NJf_naNs6y8gBi0jg5TtOwq88Xgh9Q@mail.gmail.com>

On Thu, 15 Nov 2012, Andrew Murray wrote:

> On 15 November 2012 10:04, Lee Jones <lee.jones@linaro.org> wrote:
> > The overhead is very low and the results will be found under
> > sysfs/bootime, as well as detailed results in debugfs under
> > boottime/. The bootgraph* files are compatible with
> > scripts/bootgraph.pl. The reason for this patch is to provide
> > data (sysfs/boottime) suitable for automatic test-cases as
> > well as help for developers to reduce the boot time (debugfs).
> >
> > Based heavily on the original driver by Jonas Aaberg.
> >
> 
> > +
> > +static LIST_HEAD(boottime_list);
> > +static DEFINE_SPINLOCK(boottime_list_lock);
> > +static struct boottime_timer boottime_timer;
> > +static int num_const_boottime_list;
> > +static struct boottime_list const_boottime_list[NUM_STATIC_BOOTTIME_ENTRIES];
> > +static unsigned long time_kernel_done;
> > +static unsigned long time_bootloader_done;
> > +static bool system_up;
> > +static bool boottime_done;
> > +
> > +int __attribute__((weak)) boottime_arch_startup(void)
> > +{
> > +       return 0;
> > +}
> > +
> > +int __attribute__((weak)) boottime_bootloader_idle(void)
> > +{
> > +       return 0;
> > +}
> 
> You may wish to use the __weak macro (include/linux/compiler*) instead
> of directly using GCC attributes here.

Can do, thanks Andy.

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* [PATCH] ARM: Fix errata 751472 handling on Cortex-A9 r1p*
From: Catalin Marinas @ 2012-11-15 14:37 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <50A4FCC5.2080604@gmail.com>

On Thu, Nov 15, 2012 at 02:31:33PM +0000, Rob Herring wrote:
> On 11/15/2012 05:01 AM, Catalin Marinas wrote:
> > On Thu, Nov 15, 2012 at 12:54:48AM +0000, Rob Herring wrote:
> >> On 11/14/2012 04:21 PM, Tony Lindgren wrote:
> >>> * Rob Herring <robherring2@gmail.com> [121114 13:59]:
> >>>> On 11/14/2012 02:32 PM, Tony Lindgren wrote:
> >>>>>
> >>>>> Checking for the bit already set should work in this case, I'll post
> >>>>> a patch for that shortly.
> >>>>
> >>>> Can you actually read the state of the diagnostic register in non-secure
> >>>> mode? If you can on the A9, is the same true on A8 or others?
> >>>
> >>> Looks like it can be read on at least TI omap 4430 which is A9.
> >>> But it reads as zero, so the below patch is what I came up with.
> >>>
> >>> No idea if assuming that zero value for the diagnostic register
> >>> is safe.. What's the default value of the diagnostic register supposed
> >>> to be?
> >>
> >> RTFM. Oh, wait it's a super secret, undocumented register. We shouldn't
> >> even be talking about it.
> >>
> >> It could vary by rev, but I see 0 for the reset value, so this would not
> >> work if the bootloader did not do any setup of the diagnostic register.
> >>
> >> One way to determine secure mode on the A9 would be seeing if you can
> >> change the auxcr register. Something like this (untested):
> >>
> >> mrc	p15, 0, r0, c1, c0, 1; Read ACTLR
> >> eor	r1, r0, #0x100		; Modify alloc in 1 way
> >> mcr	p15, 0, r1, c1, c0, 1
> >> mrc	p15, 0, r2, c1, c0, 1; Read ACTLR
> >> mcr	p15, 0, r0, c1, c0, 1	; Restore original value
> >> cmp	r1, r2
> >> bne	skip_errata
> > 
> > This would fail on platforms where Linux runs in non-secure mode. What
> > we do for some errata workarounds is to test whether the bit was already
> > set and avoid writing the register. But this assumes that, for a given
> > workaround in the kernel, there is a corresponding workaround in the
> > code running before the kernel (boot-loader, firmware) which sets that
> > bit.
> > 
> > Since the kernel will run more often in non-secure mode (on Cortex-A15
> > you need this for the virtualisation extensions) I strongly suggest that
> > the workaround (usually undocumented bit setting) is done before the
> > kernel is started and we simply remove it from Linux (or add a clear
> > comment that it only works if running in secure mode; if unsure say
> > 'N').
> > 
> > I don't think it's worth the hassle detecting whether the kernel runs in
> > secure or non-secure mode, just assume the latter and get SoC vendors to
> > update the boot loaders or firmware (if possible) with any errata
> > workarounds.
> 
> There's other places we need to know secure vs. non-secure mode like
> whether we can do smc calls or not.
> 
> So we should make all these work-arounds depend on !MULTI_PLATFORM then.

Only the workarounds that set bits in secure-only registers.

> Does that work for Versatile Express CA9? It needs ARM_ERRATA_751472.

On VE Linux runs in secure mode, so it's fine.

-- 
Catalin

^ permalink raw reply

* [PATCH] ARM: Fix errata 751472 handling on Cortex-A9 r1p*
From: Rob Herring @ 2012-11-15 14:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121115110137.GA25985@arm.com>

On 11/15/2012 05:01 AM, Catalin Marinas wrote:
> On Thu, Nov 15, 2012 at 12:54:48AM +0000, Rob Herring wrote:
>> On 11/14/2012 04:21 PM, Tony Lindgren wrote:
>>> * Rob Herring <robherring2@gmail.com> [121114 13:59]:
>>>> On 11/14/2012 02:32 PM, Tony Lindgren wrote:
>>>>>
>>>>> Checking for the bit already set should work in this case, I'll post
>>>>> a patch for that shortly.
>>>>
>>>> Can you actually read the state of the diagnostic register in non-secure
>>>> mode? If you can on the A9, is the same true on A8 or others?
>>>
>>> Looks like it can be read on at least TI omap 4430 which is A9.
>>> But it reads as zero, so the below patch is what I came up with.
>>>
>>> No idea if assuming that zero value for the diagnostic register
>>> is safe.. What's the default value of the diagnostic register supposed
>>> to be?
>>
>> RTFM. Oh, wait it's a super secret, undocumented register. We shouldn't
>> even be talking about it.
>>
>> It could vary by rev, but I see 0 for the reset value, so this would not
>> work if the bootloader did not do any setup of the diagnostic register.
>>
>> One way to determine secure mode on the A9 would be seeing if you can
>> change the auxcr register. Something like this (untested):
>>
>> mrc	p15, 0, r0, c1, c0, 1; Read ACTLR
>> eor	r1, r0, #0x100		; Modify alloc in 1 way
>> mcr	p15, 0, r1, c1, c0, 1
>> mrc	p15, 0, r2, c1, c0, 1; Read ACTLR
>> mcr	p15, 0, r0, c1, c0, 1	; Restore original value
>> cmp	r1, r2
>> bne	skip_errata
> 
> This would fail on platforms where Linux runs in non-secure mode. What
> we do for some errata workarounds is to test whether the bit was already
> set and avoid writing the register. But this assumes that, for a given
> workaround in the kernel, there is a corresponding workaround in the
> code running before the kernel (boot-loader, firmware) which sets that
> bit.
> 
> Since the kernel will run more often in non-secure mode (on Cortex-A15
> you need this for the virtualisation extensions) I strongly suggest that
> the workaround (usually undocumented bit setting) is done before the
> kernel is started and we simply remove it from Linux (or add a clear
> comment that it only works if running in secure mode; if unsure say
> 'N').
> 
> I don't think it's worth the hassle detecting whether the kernel runs in
> secure or non-secure mode, just assume the latter and get SoC vendors to
> update the boot loaders or firmware (if possible) with any errata
> workarounds.

There's other places we need to know secure vs. non-secure mode like
whether we can do smc calls or not.

So we should make all these work-arounds depend on !MULTI_PLATFORM then.
Does that work for Versatile Express CA9? It needs ARM_ERRATA_751472.

Rob

> 
> Having a common SMC API for errata workarounds is not feasible since not
> all registers are public, most are implementation specific and it could
> have secure implications with exposing them.
> 

^ permalink raw reply

* [PATCH] RFC: pinctrl: grab default handler with bus notifiers
From: Thomas Petazzoni @ 2012-11-15 14:26 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CACRpkdY-tye5LNOwxQ-XrjM-x9RffAvj-tPfsFQA3ZWOY8P-Vw@mail.gmail.com>

Linus,

On Thu, 15 Nov 2012 15:03:40 +0100, Linus Walleij wrote:

> > I have to say that I agree with this, notifiers seem to make life more
> > complicated for limited gain.  Otherwise I guess we could enhance
> > notifiers so that they're able to trigger deferrals?
> 
> OK I'll have to come up with a patch to the device core
> instead... it'll be much simpler anyway and if both of you guys
> can back it I guess Greg might be OK with it too.

At ELCE, I've discussed with one person having an interesting use case:
they are using the same pin for two different purposes during the
system operation.

At boot time, this pin is muxed as a SPI pin and is used to program the
bitstream of a FPGA. This is done in Linux, through a userspace
application.

Then, once it is done, this pin is remuxed as a UART pin, and used to
communicate with another device on the board.

If the pinctrl configuration is done at probe() time inside the driver
core, then how can this kind of use case be supported? If each driver
does its own muxing, we could think of letting the SPI and UART driver
only do their muxing when the device is opened, and remove their muxing
when the device is closed, so that such sharing of pins for two
separate functions could be supported. Is this something we want to
support?

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

* [PATCH 01/14] pinctrl: SPEAr: add spi chipselect control driver
From: Viresh Kumar @ 2012-11-15 14:19 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CACRpkdb9tSUUGQjWdHqRr5rUSyiPgWm2i3-QZq9VbWFY2EAQeQ@mail.gmail.com>

On 15 November 2012 19:47, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Sun, Nov 11, 2012 at 5:39 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
>> From: Shiraz Hashim <shiraz.hashim@st.com>
>>
>> SPEAr platform provides a provision to control chipselects of ARM PL022
>> Prime Cell spi controller through its system registers, which otherwise
>> remains under PL022 control which some protocols do not want.
>>
>> This commit intends to provide the spi chipselect control in software
>> over gpiolib interface. Since it is tied to pinctrl, we place it under
>> 'drivers/pinctrl/spear/' directory.
>>
>> spi chip drivers can use the exported gpiolib interface to define their
>> chipselect through DT or platform data.
>>
>> Cc: Linus Walleij <linus.walleij@linaro.org>
>> Signed-off-by: Shiraz Hashim <shiraz.hashim@st.com>
>> Reviewed-by: Vipin Kumar <vipin.kumar@st.com>
>> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>
> This sure looks good, sorry for the delay.
>
> Just one question: since this driver is not using any
> pinctrl interfaces, why is it under drivers/pinctrl/*?
>
> Shouldn't it be under drivers/gpio from a technical
> point of view?
>
> I think I'd accept it under drivers/pinctrl/spear/*
> if you just want this to keep everything SPEAr-related
> in one place though, so enlighten me.

That's because they are controlling few pads. They are not
actually gpio's but just pads that we are controlling.

That's why we thought they are better in this place.

--
viresh

^ permalink raw reply

* [PATCH v3 1/5] ARM: kernel: smp_setup_processor_id() updates
From: Lorenzo Pieralisi @ 2012-11-15 14:18 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20121115134951.GF5570@mudshark.cambridge.arm.com>

[CC'ed Vincent following MPIDR mask query]

On Thu, Nov 15, 2012 at 01:49:51PM +0000, Will Deacon wrote:
> On Thu, Nov 15, 2012 at 12:46:50PM +0000, Lorenzo Pieralisi wrote:
> > This patch applies some basic changes to the smp_setup_processor_id()
> > ARM implementation to make the code that builds cpu_logical_map more
> > uniform across the kernel.
> > 
> > The function now prints the full extent of the boot CPU MPIDR[23:0] and
> > initializes the cpu_logical_map for CPUs up to nr_cpu_ids.
> > 
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > ---
> >  arch/arm/kernel/setup.c | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> > index da1d1aa..d0df6c8 100644
> > --- a/arch/arm/kernel/setup.c
> > +++ b/arch/arm/kernel/setup.c
> > @@ -426,13 +426,14 @@ int __cpu_logical_map[NR_CPUS];
> >  void __init smp_setup_processor_id(void)
> >  {
> >  	int i;
> > -	u32 cpu = is_smp() ? read_cpuid_mpidr() & 0xff : 0;
> > +	u32 mpidr = is_smp() ? read_cpuid_mpidr() & 0xffffff : 0;
> > +	u32 cpu = mpidr & 0xff;
> 
> Can we move the #defines out of topology.c and use those instead of magic
> masks please? You could use MPIDR_HWID_BITMASK, although the LEVEL mask
> definitions over there don't look right to me (MPIDR_LEVEL0_MASK is 0x3?)

Yes, I should have done that, you are right. LEVEL0 and LEVEL1 masks look
wrong to me.

#define MPIDR_LEVEL0_MASK 0x3
#define MPIDR_LEVEL1_MASK 0xF

Vincent, why are they defined with those values ? They should all be
defined as 0xFF.

I will fix them and move them to a header file.

Thanks,
Lorenzo

^ permalink raw reply

* [PATCH 01/14] pinctrl: SPEAr: add spi chipselect control driver
From: Linus Walleij @ 2012-11-15 14:17 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <4a92290e8a3b1a19c3a5e864edfa7badfc2af5d0.1352608333.git.viresh.kumar@linaro.org>

On Sun, Nov 11, 2012 at 5:39 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:

> From: Shiraz Hashim <shiraz.hashim@st.com>
>
> SPEAr platform provides a provision to control chipselects of ARM PL022
> Prime Cell spi controller through its system registers, which otherwise
> remains under PL022 control which some protocols do not want.
>
> This commit intends to provide the spi chipselect control in software
> over gpiolib interface. Since it is tied to pinctrl, we place it under
> 'drivers/pinctrl/spear/' directory.
>
> spi chip drivers can use the exported gpiolib interface to define their
> chipselect through DT or platform data.
>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Shiraz Hashim <shiraz.hashim@st.com>
> Reviewed-by: Vipin Kumar <vipin.kumar@st.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

This sure looks good, sorry for the delay.

Just one question: since this driver is not using any
pinctrl interfaces, why is it under drivers/pinctrl/*?

Shouldn't it be under drivers/gpio from a technical
point of view?

I think I'd accept it under drivers/pinctrl/spear/*
if you just want this to keep everything SPEAr-related
in one place though, so enlighten me.

Yours,
Linus Walleij

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox