LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: Ethernet over PCIe driver for Inter-Processor Communication
From: David Hawkins @ 2013-08-25 22:38 UTC (permalink / raw)
  To: Saravanan S; +Cc: linuxppc-dev@lists.ozlabs.org, Ira W. Snyder
In-Reply-To: <CAEqOc-SdBZgiB1HZOQAJhgq_epdGsqMXaiSO0Axcd9ZBq6kLDw@mail.gmail.com>

Hi S.Saravanan,

>> Root complex's would normally interrupt a device via a PCIe write
>> to a register in a BAR on the end-point (or in extended configuration
>> space registers depending on the hardware implementation).
>
> MPC8640 End point implements only the Type 0 header (Page 1116) . The
> header implements five BARs (Page 1165).

One of those BARs typically provides access to the PowerPC memory
mapped registers (or at least a 1MB window onto those registers).
This is how your root complex can write to some form of messaging
register.

>> PCIe drivers need some way to interrupt the processor, so there must
>> be an option somewhere ... for example, what are the message register
>> interrupts intended for? See p479
>>
>> http://cache.freescale.com/files/32bit/doc/ref_manual/MPC8641DRM.pdf
>>
>> (Ira and myself have not used the MPC8640 so are not familiar with
>> its user manual).
>
> Message registers are for interrupting the processor. A write to
> them sends an interrupt to the processor.  Actually message registers
> are used by the RC to enable interrupts to the processor when an EP
> sends an MSI transaction to RC. In RC driver i register separately for
> the msi interrupts from all three EPs.

This is pretty much what you are looking for then right?

The end-points interrrupt the root-complex using PCIe MSI interrupts,
whereas the root-complex interrupts an end-point by writing directly
to its MSI interrupt.

> To access them in the EP from the RC  i will have to set an inbound
> window mapping the PIC register space in the EP to the PCI mem space
> assigned to it . An inbound window maps a PCI address on the bus
> received by the PCIe controller to a platform address. I will try that
> and let u know .

Right, as I comment above, one of the BARs typically exposes the PowerPC
internal registers.

>> Feel free to discuss your ideas for your PCIe driver (eg., why start
>> with rionet rather than Ira's driver), either on-list, or email Ira
>> and myself directly
>
> To be frank with you there was no particular reason in starting with
> rionet. Maybe because our board also had SRIO interface and we are using
> rionet driver successfully. I had looked at Ira's driver later. I will
> study that also and try   to come back with a skeleton for my driver.

Its always a good idea to discuss different options, and to stub out
drivers or create minimal (but functional) drivers. That way you'll
be able to see how similar your new driver is to other drivers, and
you'll quickly discover if there is a hardware feature in the
existing driver that you cannot emulate (eg., some SRIO feature
used by the rionet driver).

>> One further note. You might want to look at rproc/rpmsg and their virtio
>> driver support. That seems to be where the Linux world is moving for
>> inter-processor communications. See for example the ARM CPUs interfacing
>> with DSPs.
>
> I will study that as i am not familiar with virtio .

Follow Ira's advice. Talk to the guys working on virtio, tell them what
you are trying to do. They'll likely have good advice for you.

Good luck!

Cheers,
Dave

^ permalink raw reply

* Re: [PATCH] powerpc/kvm: Handle the boundary condition correctly
From: Alexander Graf @ 2013-08-25 18:16 UTC (permalink / raw)
  To: Aneesh Kumar K.V; +Cc: paulus, linuxppc-dev, kvm-ppc
In-Reply-To: <87haeh13s2.fsf@linux.vnet.ibm.com>


On 23.08.2013, at 04:31, Aneesh Kumar K.V wrote:

> Alexander Graf <agraf@suse.de> writes:
>=20
>> On 22.08.2013, at 12:37, Aneesh Kumar K.V wrote:
>>=20
>>> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>>=20
>> Isn't this you?
>=20
> Yes. The patches are generated using git format-patch and sent by
> git send-email. That's how it always created patches for me. I am not =
sure if
> there is a config I can change to avoid having From:
>=20
>>=20
>>>=20
>>> We should be able to copy upto count bytes
>>=20
>> Why?
>>=20
>=20
> Without this we end up doing
>=20
> +    struct kvm_get_htab_buf {
> +        struct kvm_get_htab_header header;
> +        /*
> +         * Older kernel required one extra byte.
> +         */
> +        unsigned long hpte[3];
> +    } hpte_buf;
>=20
>=20
> even though we are only looking for one hpte entry.

Ok, please give me an example with real numbers and why it breaks.


Alex

>=20
> =
http://mid.gmane.org/1376995766-16526-4-git-send-email-aneesh.kumar@linux.=
vnet.ibm.com
>=20
>>=20
>> Alex
>>=20
>>>=20
>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>>> ---
>>> arch/powerpc/kvm/book3s_64_mmu_hv.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>=20
>>> diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c =
b/arch/powerpc/kvm/book3s_64_mmu_hv.c
>>> index 710d313..0ae6bb6 100644
>>> --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
>>> +++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
>>> @@ -1362,7 +1362,7 @@ static ssize_t kvm_htab_read(struct file =
*file, char __user *buf,
>>> 	lbuf =3D (unsigned long __user *)buf;
>>>=20
>>> 	nb =3D 0;
>>> -	while (nb + sizeof(hdr) + HPTE_SIZE < count) {
>>> +	while (nb + sizeof(hdr) + HPTE_SIZE <=3D count) {
>>> 		/* Initialize header */
>>> 		hptr =3D (struct kvm_get_htab_header __user *)buf;
>>> 		hdr.n_valid =3D 0;
>>> @@ -1385,7 +1385,7 @@ static ssize_t kvm_htab_read(struct file =
*file, char __user *buf,
>>> 		/* Grab a series of valid entries */
>>> 		while (i < kvm->arch.hpt_npte &&
>>> 		       hdr.n_valid < 0xffff &&
>>> -		       nb + HPTE_SIZE < count &&
>>> +		       nb + HPTE_SIZE <=3D count &&
>>> 		       record_hpte(flags, hptp, hpte, revp, 1, =
first_pass)) {
>>> 			/* valid entry, write it out */
>>> 			++hdr.n_valid;
>>> --=20
>>> 1.8.1.2
>>>=20
>=20
> --
> To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] powerpc/kvm: Handle the boundary condition correctly
From: Alexander Graf @ 2013-08-25 18:09 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, paulus, Aneesh Kumar K.V, kvm-ppc
In-Reply-To: <1377232085.3613.0.camel@pasglop>


On 23.08.2013, at 05:28, Benjamin Herrenschmidt wrote:

> On Fri, 2013-08-23 at 09:01 +0530, Aneesh Kumar K.V wrote:
>> Alexander Graf <agraf@suse.de> writes:
>>=20
>>> On 22.08.2013, at 12:37, Aneesh Kumar K.V wrote:
>>>=20
>>>> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>>>=20
>>> Isn't this you?
>>=20
>> Yes. The patches are generated using git format-patch and sent by
>> git send-email. That's how it always created patches for me. I am not =
sure if
>> there is a config I can change to avoid having From:
>=20
> Don't bother, that's perfectly fine, and git am will do the right =
thing.

It will, but it's an indicator that something in his git config is =
misconfigured. Usually when git sees "Author =3D=3D Sender" it will omit =
the From: line.


Alex

^ permalink raw reply

* Re: Ethernet over PCIe driver for Inter-Processor Communication
From: Saravanan S @ 2013-08-25 15:20 UTC (permalink / raw)
  To: Ira W. Snyder, scottwood; +Cc: linuxppc-dev@lists.ozlabs.org, David Hawkins
In-Reply-To: <20130822222951.GA13201@ovro.caltech.edu>

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

Hi All,
         First of all thank you  all for taking your time out to reply



On Fri, Aug 23, 2013 at 3:59 AM, Ira W. Snyder <iws@ovro.caltech.edu> wrote:

> On Thu, Aug 22, 2013 at 02:43:38PM -0700, David Hawkins wrote:
> > Hi S.Saravanan,
> >
> > > I have a custom board  with four MPC8640 nodes connected over
> > > a transparent PCI express switch . In this configuration one node is
> > > configured as host(Root Complex) and others as agents(End Point). Thus
> > > the legacy PCI software works fine . However the mainline kernel lacks
> > > any standard support for Inter-processor communication over PCI. I am
> > > in the process of developing an Ethernet over  PCI driver for the same
> > > on the lines of rionet . However I am facing the following problems.
> > >
> > > a)   I can generate MSI interrupts from End Point to Root Complex over
> > > PCI . But the vice-versa is not possible . However i need a method to
> > > interrupt the End Point from the Root Complex to complete my driver.
> >
> > Root complex's would normally interrupt a device via a PCIe write
> > to a register in a BAR on the end-point (or in extended configuration
> > space registers depending on the hardware implementation).
>
> MPC8640 End point implements only the Type 0 header (Page 1116) . The
header implements five BARs (Page 1165).


> > > Only previous references I can find are this post
> > >
> http://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg25765.html
> > > However this uses doorbells and I think may not be possible in MPC8640.
> >
> > PCIe drivers need some way to interrupt the processor, so there must
> > be an option somewhere ... for example, what are the message register
> > interrupts intended for? See p479
> >
> > http://cache.freescale.com/files/32bit/doc/ref_manual/MPC8641DRM.pdf
> >
> > (Ira and myself have not used the MPC8640 so are not familiar with
> > its user manual).
>

   Message registers  are for interrupting the processor . A write to them
sends an interrupt to the processor .  Actually message registers are used
by the RC to enable interrupts to the processor when an EP sends an MSI
transaction to RC.In RC driver  i register separately for the msi
interrupts from all three EPs.

 To access them in the EP from the RC  i will have to set an inbound window
mapping the PIC register space in the EP  to the PCI mem space  assigned to
it . An inbound window maps a PCI address on the bus received by the PCIe
controller to a platform address. I will try that and let u know .

> >
> > > Any pointers on this issue and guidance on this driver development
> would
> > > be helpful .
> >
> > We use the Ethernet-over-PCI driver that Ira developed. Our next boards
> > will use an MPC8308, but we don't currently have any in a PCIe device
> > form-factor (just the MPC8038RDB), so he has not ported it to PCIe.
> >
> > Feel free to discuss your ideas for your PCIe driver (eg., why start
> > with rionet rather than Ira's driver), either on-list, or email Ira
> > and myself directly
>

To be frank with you there was no particular reason in starting with
rionet. Maybe because our board also had SRIO interface and we are using
rionet driver successfully. I had looked at Ira's driver later. I will
study that also and try   to come back with a skeleton for my driver.


>
> One further note. You might want to look at rproc/rpmsg and their virtio
> driver support. That seems to be where the Linux world is moving for
> inter-processor communications. See for example the ARM CPUs interfacing
> with DSPs.
>
> Ira
>

I will study that as i am not familiar with virtio .

Regards,

S.Saravanan

[-- Attachment #2: Type: text/html, Size: 5499 bytes --]

^ permalink raw reply

* Re: [PATCH 2/2] ppc: kvm: use anon_inode_getfd() with O_CLOEXEC flag
From: Alexander Graf @ 2013-08-25 15:04 UTC (permalink / raw)
  To: Yann Droneaud
  Cc: kvm, Gleb Natapov, linux-kernel, kvm-ppc, Alex Williamson,
	Paul Mackerras, Paolo Bonzini, linuxppc-dev
In-Reply-To: <b7ae23be3a3d4a2f57eca6cfc7f9361da883ceca.1377372576.git.ydroneaud@opteya.com>


On 24.08.2013, at 21:14, Yann Droneaud wrote:

> KVM uses anon_inode_get() to allocate file descriptors as part
> of some of its ioctls. But those ioctls are lacking a flag argument
> allowing userspace to choose options for the newly opened file =
descriptor.
>=20
> In such case it's advised to use O_CLOEXEC by default so that
> userspace is allowed to choose, without race, if the file descriptor
> is going to be inherited across exec().
>=20
> This patch set O_CLOEXEC flag on all file descriptors created
> with anon_inode_getfd() to not leak file descriptors across exec().
>=20
> Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
> Link: =
http://lkml.kernel.org/r/cover.1377372576.git.ydroneaud@opteya.com

Reviewed-by: Alexander Graf <agraf@suse.de>

Would it make sense to simply inherit the O_CLOEXEC flag from the parent =
kvm fd instead? That would give user space the power to keep fds across =
exec() if it wants to.


Alex

^ permalink raw reply

* [PATCH 1/5] jump_label: factor out the base part of jump_label.h to a separate file
From: Kevin Hao @ 2013-08-25  7:15 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc, linux-kernel
In-Reply-To: <1377414952-15995-1-git-send-email-haokexin@gmail.com>

We plan to use the jump label in the cpu/mmu feature check on ppc.
This will need to include the jump_label.h in several very basic header
files of ppc which seems to be included by most of the other head
files implicitly or explicitly. But in the current jump_label.h,
it also include the "linux/workqueue.h" and this will cause recursive
inclusion. In order to fix this, we choose to factor out the base
part of jump_label.h to a separate header file and we can include
that file instead of jump_label.h to avoid the recursive inclusion.
No functional change.

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
 include/linux/jump_label.h      | 132 +------------------------------------
 include/linux/jump_label_base.h | 142 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 144 insertions(+), 130 deletions(-)
 create mode 100644 include/linux/jump_label_base.h

diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index 0976fc4..14bae65 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -46,20 +46,11 @@
  *
 */
 
-#include <linux/types.h>
-#include <linux/compiler.h>
 #include <linux/workqueue.h>
+#include <linux/jump_label_base.h>
 
-#if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_JUMP_LABEL)
 
-struct static_key {
-	atomic_t enabled;
-/* Set lsb bit to 1 if branch is default true, 0 ot */
-	struct jump_entry *entries;
-#ifdef CONFIG_MODULES
-	struct static_key_mod *next;
-#endif
-};
+#ifdef HAVE_JUMP_LABEL
 
 struct static_key_deferred {
 	struct static_key key;
@@ -67,145 +58,26 @@ struct static_key_deferred {
 	struct delayed_work work;
 };
 
-# include <asm/jump_label.h>
-# define HAVE_JUMP_LABEL
-#endif	/* CC_HAVE_ASM_GOTO && CONFIG_JUMP_LABEL */
-
-enum jump_label_type {
-	JUMP_LABEL_DISABLE = 0,
-	JUMP_LABEL_ENABLE,
-};
-
-struct module;
-
-#ifdef HAVE_JUMP_LABEL
-
-#define JUMP_LABEL_TRUE_BRANCH 1UL
-
-static
-inline struct jump_entry *jump_label_get_entries(struct static_key *key)
-{
-	return (struct jump_entry *)((unsigned long)key->entries
-						& ~JUMP_LABEL_TRUE_BRANCH);
-}
-
-static inline bool jump_label_get_branch_default(struct static_key *key)
-{
-	if ((unsigned long)key->entries & JUMP_LABEL_TRUE_BRANCH)
-		return true;
-	return false;
-}
-
-static __always_inline bool static_key_false(struct static_key *key)
-{
-	return arch_static_branch(key);
-}
-
-static __always_inline bool static_key_true(struct static_key *key)
-{
-	return !static_key_false(key);
-}
-
-extern struct jump_entry __start___jump_table[];
-extern struct jump_entry __stop___jump_table[];
-
-extern void jump_label_init(void);
-extern void jump_label_lock(void);
-extern void jump_label_unlock(void);
-extern void arch_jump_label_transform(struct jump_entry *entry,
-				      enum jump_label_type type);
-extern void arch_jump_label_transform_static(struct jump_entry *entry,
-					     enum jump_label_type type);
-extern int jump_label_text_reserved(void *start, void *end);
-extern void static_key_slow_inc(struct static_key *key);
-extern void static_key_slow_dec(struct static_key *key);
 extern void static_key_slow_dec_deferred(struct static_key_deferred *key);
-extern void jump_label_apply_nops(struct module *mod);
 extern void
 jump_label_rate_limit(struct static_key_deferred *key, unsigned long rl);
 
-#define STATIC_KEY_INIT_TRUE ((struct static_key) \
-	{ .enabled = ATOMIC_INIT(1), .entries = (void *)1 })
-#define STATIC_KEY_INIT_FALSE ((struct static_key) \
-	{ .enabled = ATOMIC_INIT(0), .entries = (void *)0 })
-
 #else  /* !HAVE_JUMP_LABEL */
 
-#include <linux/atomic.h>
-
-struct static_key {
-	atomic_t enabled;
-};
-
-static __always_inline void jump_label_init(void)
-{
-}
-
 struct static_key_deferred {
 	struct static_key  key;
 };
 
-static __always_inline bool static_key_false(struct static_key *key)
-{
-	if (unlikely(atomic_read(&key->enabled)) > 0)
-		return true;
-	return false;
-}
-
-static __always_inline bool static_key_true(struct static_key *key)
-{
-	if (likely(atomic_read(&key->enabled)) > 0)
-		return true;
-	return false;
-}
-
-static inline void static_key_slow_inc(struct static_key *key)
-{
-	atomic_inc(&key->enabled);
-}
-
-static inline void static_key_slow_dec(struct static_key *key)
-{
-	atomic_dec(&key->enabled);
-}
-
 static inline void static_key_slow_dec_deferred(struct static_key_deferred *key)
 {
 	static_key_slow_dec(&key->key);
 }
 
-static inline int jump_label_text_reserved(void *start, void *end)
-{
-	return 0;
-}
-
-static inline void jump_label_lock(void) {}
-static inline void jump_label_unlock(void) {}
-
-static inline int jump_label_apply_nops(struct module *mod)
-{
-	return 0;
-}
-
 static inline void
 jump_label_rate_limit(struct static_key_deferred *key,
 		unsigned long rl)
 {
 }
 
-#define STATIC_KEY_INIT_TRUE ((struct static_key) \
-		{ .enabled = ATOMIC_INIT(1) })
-#define STATIC_KEY_INIT_FALSE ((struct static_key) \
-		{ .enabled = ATOMIC_INIT(0) })
-
 #endif	/* HAVE_JUMP_LABEL */
-
-#define STATIC_KEY_INIT STATIC_KEY_INIT_FALSE
-#define jump_label_enabled static_key_enabled
-
-static inline bool static_key_enabled(struct static_key *key)
-{
-	return (atomic_read(&key->enabled) > 0);
-}
-
 #endif	/* _LINUX_JUMP_LABEL_H */
diff --git a/include/linux/jump_label_base.h b/include/linux/jump_label_base.h
new file mode 100644
index 0000000..20df08f
--- /dev/null
+++ b/include/linux/jump_label_base.h
@@ -0,0 +1,142 @@
+#ifndef _LINUX_JUMP_LABEL_BASE_H
+#define _LINUX_JUMP_LABEL_BASE_H
+
+
+#include <linux/types.h>
+#include <linux/compiler.h>
+
+#if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_JUMP_LABEL)
+
+struct static_key {
+	atomic_t enabled;
+/* Set lsb bit to 1 if branch is default true, 0 ot */
+	struct jump_entry *entries;
+#ifdef CONFIG_MODULES
+	struct static_key_mod *next;
+#endif
+};
+
+# include <asm/jump_label.h>
+# define HAVE_JUMP_LABEL
+#endif	/* CC_HAVE_ASM_GOTO && CONFIG_JUMP_LABEL */
+
+enum jump_label_type {
+	JUMP_LABEL_DISABLE = 0,
+	JUMP_LABEL_ENABLE,
+};
+
+struct module;
+
+#ifdef HAVE_JUMP_LABEL
+
+#define JUMP_LABEL_TRUE_BRANCH 1UL
+
+static
+inline struct jump_entry *jump_label_get_entries(struct static_key *key)
+{
+	return (struct jump_entry *)((unsigned long)key->entries
+						& ~JUMP_LABEL_TRUE_BRANCH);
+}
+
+static inline bool jump_label_get_branch_default(struct static_key *key)
+{
+	if ((unsigned long)key->entries & JUMP_LABEL_TRUE_BRANCH)
+		return true;
+	return false;
+}
+
+static __always_inline bool static_key_false(struct static_key *key)
+{
+	return arch_static_branch(key);
+}
+
+static __always_inline bool static_key_true(struct static_key *key)
+{
+	return !static_key_false(key);
+}
+
+extern struct jump_entry __start___jump_table[];
+extern struct jump_entry __stop___jump_table[];
+
+extern void jump_label_init(void);
+extern void jump_label_lock(void);
+extern void jump_label_unlock(void);
+extern void arch_jump_label_transform(struct jump_entry *entry,
+				      enum jump_label_type type);
+extern void arch_jump_label_transform_static(struct jump_entry *entry,
+					     enum jump_label_type type);
+extern int jump_label_text_reserved(void *start, void *end);
+extern void static_key_slow_inc(struct static_key *key);
+extern void static_key_slow_dec(struct static_key *key);
+extern void jump_label_apply_nops(struct module *mod);
+
+#define STATIC_KEY_INIT_TRUE ((struct static_key) \
+	{ .enabled = ATOMIC_INIT(1), .entries = (void *)1 })
+#define STATIC_KEY_INIT_FALSE ((struct static_key) \
+	{ .enabled = ATOMIC_INIT(0), .entries = (void *)0 })
+
+#else  /* !HAVE_JUMP_LABEL */
+
+#include <linux/atomic.h>
+
+struct static_key {
+	atomic_t enabled;
+};
+
+static __always_inline void jump_label_init(void)
+{
+}
+
+static __always_inline bool static_key_false(struct static_key *key)
+{
+	if (unlikely(atomic_read(&key->enabled)) > 0)
+		return true;
+	return false;
+}
+
+static __always_inline bool static_key_true(struct static_key *key)
+{
+	if (likely(atomic_read(&key->enabled)) > 0)
+		return true;
+	return false;
+}
+
+static inline void static_key_slow_inc(struct static_key *key)
+{
+	atomic_inc(&key->enabled);
+}
+
+static inline void static_key_slow_dec(struct static_key *key)
+{
+	atomic_dec(&key->enabled);
+}
+
+static inline int jump_label_text_reserved(void *start, void *end)
+{
+	return 0;
+}
+
+static inline void jump_label_lock(void) {}
+static inline void jump_label_unlock(void) {}
+
+static inline int jump_label_apply_nops(struct module *mod)
+{
+	return 0;
+}
+
+#define STATIC_KEY_INIT_TRUE ((struct static_key) \
+		{ .enabled = ATOMIC_INIT(1) })
+#define STATIC_KEY_INIT_FALSE ((struct static_key) \
+		{ .enabled = ATOMIC_INIT(0) })
+
+#endif	/* HAVE_JUMP_LABEL */
+
+#define STATIC_KEY_INIT STATIC_KEY_INIT_FALSE
+#define jump_label_enabled static_key_enabled
+
+static inline bool static_key_enabled(struct static_key *key)
+{
+	return (atomic_read(&key->enabled) > 0);
+}
+
+#endif	/* _LINUX_JUMP_LABEL_BASE_H */
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH 5/5] powerpc: use jump label for mmu_has_feature
From: Kevin Hao @ 2013-08-25  7:15 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc
In-Reply-To: <1377414952-15995-1-git-send-email-haokexin@gmail.com>

The mmu features are fixed once the probe of mmu features are done.
And the function mmu_has_feature() does be used in some hot path.
The checking of the mmu features for each time of invoking of
mmu_has_feature() seems suboptimal. This tries to reduce this
overhead of this check by using jump label. But we can only use
the jump label for this check only after the execution of
jump_label_init(), so we introduce another jump label to
still do the feature check by default before all the mmu
feature jump labels are initialized.

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
 arch/powerpc/include/asm/mmu.h | 19 +++++++++++++++++++
 arch/powerpc/kernel/cputable.c | 20 ++++++++++++++++++++
 2 files changed, 39 insertions(+)

diff --git a/arch/powerpc/include/asm/mmu.h b/arch/powerpc/include/asm/mmu.h
index 691fd8a..163e9b1 100644
--- a/arch/powerpc/include/asm/mmu.h
+++ b/arch/powerpc/include/asm/mmu.h
@@ -121,10 +121,29 @@
 DECLARE_PER_CPU(int, next_tlbcam_idx);
 #endif
 
+#ifdef CONFIG_JUMP_LABEL
+#include <linux/jump_label_base.h>
+
+#define MAX_MMU_FEATURES	32
+
+extern struct static_key mmu_feat_keys[MAX_MMU_FEATURES];
+extern struct static_key mmu_feat_keys_enabled;
+
+static inline int mmu_has_feature(unsigned long feature)
+{
+	if (static_key_false(&mmu_feat_keys_enabled)) {
+		int i = __builtin_ctzl(feature);
+
+		return static_key_false(&mmu_feat_keys[i]);
+	} else
+		return !!(cur_cpu_spec->mmu_features & feature);
+}
+#else
 static inline int mmu_has_feature(unsigned long feature)
 {
 	return (cur_cpu_spec->mmu_features & feature);
 }
+#endif
 
 static inline void mmu_clear_feature(unsigned long feature)
 {
diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
index 2014ab7..f25eb1d 100644
--- a/arch/powerpc/kernel/cputable.c
+++ b/arch/powerpc/kernel/cputable.c
@@ -2280,4 +2280,24 @@ static __init int cpu_feat_keys_init(void)
 	return 0;
 }
 early_initcall(cpu_feat_keys_init);
+
+struct static_key mmu_feat_keys[MAX_MMU_FEATURES];
+struct static_key mmu_feat_keys_enabled;
+
+static __init int mmu_feat_keys_init(void)
+{
+	int i;
+
+	for (i = 0; i < MAX_MMU_FEATURES; i++) {
+		unsigned long f = 1 << i;
+
+		if (cur_cpu_spec->mmu_features & f)
+			static_key_slow_inc(&mmu_feat_keys[i]);
+	}
+
+	static_key_slow_inc(&mmu_feat_keys_enabled);
+
+	return 0;
+}
+early_initcall(mmu_feat_keys_init);
 #endif
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH 4/5] powerpc: use the jump label for cpu_has_feature
From: Kevin Hao @ 2013-08-25  7:15 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc
In-Reply-To: <1377414952-15995-1-git-send-email-haokexin@gmail.com>

The cpu features are fixed once the probe of cpu features are done.
And the function cpu_has_feature() does be used in some hot path.
The checking of the cpu features for each time of invoking of
cpu_has_feature() seems suboptimal. This tries to reduce this
overhead of this check by using jump label. But we can only use
the jump label for this check only after the execution of
jump_label_init(), so we introduce another jump label to
still do the feature check by default before all the cpu
feature jump labels are initialized.

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
 arch/powerpc/include/asm/cpufeatures.h | 28 ++++++++++++++++++++++++++++
 arch/powerpc/kernel/cputable.c         | 23 +++++++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/arch/powerpc/include/asm/cpufeatures.h b/arch/powerpc/include/asm/cpufeatures.h
index 37650db..598ac91 100644
--- a/arch/powerpc/include/asm/cpufeatures.h
+++ b/arch/powerpc/include/asm/cpufeatures.h
@@ -2,7 +2,34 @@
 #define __ASM_POWERPC_CPUFEATURES_H
 
 #include <asm/cputable.h>
+#ifdef CONFIG_JUMP_LABEL
+#include <linux/atomic.h>
+#include <linux/jump_label_base.h>
 
+#ifdef __powerpc64__
+#define MAX_CPU_FEATURES	64
+#else
+#define MAX_CPU_FEATURES	32
+#endif
+extern struct static_key cpu_feat_keys[MAX_CPU_FEATURES];
+extern struct static_key cpu_feat_keys_enabled;
+
+static inline int cpu_has_feature(unsigned long feature)
+{
+	if (CPU_FTRS_ALWAYS & feature)
+		return 1;
+
+	if (!(CPU_FTRS_POSSIBLE | feature))
+		return 0;
+
+	if (static_key_false(&cpu_feat_keys_enabled)) {
+		int i = __builtin_ctzl(feature);
+
+		return static_key_false(&cpu_feat_keys[i]);
+	} else
+		return !!(cur_cpu_spec->cpu_features & feature);
+}
+#else
 static inline int cpu_has_feature(unsigned long feature)
 {
 	return (CPU_FTRS_ALWAYS & feature) ||
@@ -10,5 +37,6 @@ static inline int cpu_has_feature(unsigned long feature)
 		& cur_cpu_spec->cpu_features
 		& feature);
 }
+#endif
 
 #endif /* __ASM_POWERPC_CPUFEATURE_H */
diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
index 22973a7..2014ab7 100644
--- a/arch/powerpc/kernel/cputable.c
+++ b/arch/powerpc/kernel/cputable.c
@@ -21,6 +21,7 @@
 #include <asm/prom.h>		/* for PTRRELOC on ARCH=ppc */
 #include <asm/mmu.h>
 #include <asm/setup.h>
+#include <asm/cpufeatures.h>
 
 struct cpu_spec* cur_cpu_spec = NULL;
 EXPORT_SYMBOL(cur_cpu_spec);
@@ -2258,3 +2259,25 @@ struct cpu_spec * __init identify_cpu(unsigned long offset, unsigned int pvr)
 
 	return NULL;
 }
+
+#ifdef CONFIG_JUMP_LABEL
+struct static_key cpu_feat_keys[MAX_CPU_FEATURES];
+struct static_key cpu_feat_keys_enabled;
+
+static __init int cpu_feat_keys_init(void)
+{
+	int i;
+
+	for (i = 0; i < MAX_CPU_FEATURES; i++) {
+		unsigned long f = 1 << i;
+
+		if (cur_cpu_spec->cpu_features & f)
+			static_key_slow_inc(&cpu_feat_keys[i]);
+	}
+
+	static_key_slow_inc(&cpu_feat_keys_enabled);
+
+	return 0;
+}
+early_initcall(cpu_feat_keys_init);
+#endif
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH 3/5] powerpc: move the cpu_has_feature to a separate file
From: Kevin Hao @ 2013-08-25  7:15 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc
In-Reply-To: <1377414952-15995-1-git-send-email-haokexin@gmail.com>

We plan to use jump label for cpu_has_feature. In order to implement
this we need to include the linux/jump_label_base.h in asm/cputable.h.
But it seems that asm/cputable.h is so basic header file for ppc that
it is almost included by all the other header files. The including of
the linux/jump_label_base.h will introduces various recursive inclusion.
And it is very hard to fix that. So we choose to move the function
cpu_has_feature to a separate header file before using the jump label
for it. No functional change.

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
 arch/powerpc/include/asm/cacheflush.h   |  1 +
 arch/powerpc/include/asm/cpufeatures.h  | 14 ++++++++++++++
 arch/powerpc/include/asm/cputable.h     |  8 --------
 arch/powerpc/include/asm/cputime.h      |  1 +
 arch/powerpc/include/asm/dbell.h        |  1 +
 arch/powerpc/include/asm/dcr-native.h   |  1 +
 arch/powerpc/include/asm/mman.h         |  1 +
 arch/powerpc/include/asm/time.h         |  1 +
 arch/powerpc/kernel/align.c             |  1 +
 arch/powerpc/kernel/irq.c               |  1 +
 arch/powerpc/kernel/process.c           |  1 +
 arch/powerpc/kernel/setup-common.c      |  1 +
 arch/powerpc/kernel/setup_32.c          |  1 +
 arch/powerpc/kernel/smp.c               |  1 +
 arch/powerpc/oprofile/op_model_rs64.c   |  1 +
 arch/powerpc/platforms/cell/pervasive.c |  1 +
 arch/powerpc/xmon/ppc-dis.c             |  1 +
 17 files changed, 29 insertions(+), 8 deletions(-)
 create mode 100644 arch/powerpc/include/asm/cpufeatures.h

diff --git a/arch/powerpc/include/asm/cacheflush.h b/arch/powerpc/include/asm/cacheflush.h
index b843e35..540b32e 100644
--- a/arch/powerpc/include/asm/cacheflush.h
+++ b/arch/powerpc/include/asm/cacheflush.h
@@ -11,6 +11,7 @@
 
 #include <linux/mm.h>
 #include <asm/cputable.h>
+#include <asm/cpufeatures.h>
 
 /*
  * No cache flushing is required when address mappings are changed,
diff --git a/arch/powerpc/include/asm/cpufeatures.h b/arch/powerpc/include/asm/cpufeatures.h
new file mode 100644
index 0000000..37650db
--- /dev/null
+++ b/arch/powerpc/include/asm/cpufeatures.h
@@ -0,0 +1,14 @@
+#ifndef __ASM_POWERPC_CPUFEATURES_H
+#define __ASM_POWERPC_CPUFEATURES_H
+
+#include <asm/cputable.h>
+
+static inline int cpu_has_feature(unsigned long feature)
+{
+	return (CPU_FTRS_ALWAYS & feature) ||
+	       (CPU_FTRS_POSSIBLE
+		& cur_cpu_spec->cpu_features
+		& feature);
+}
+
+#endif /* __ASM_POWERPC_CPUFEATURE_H */
diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
index 6f3887d..ab0813d 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -541,14 +541,6 @@ enum {
 };
 #endif /* __powerpc64__ */
 
-static inline int cpu_has_feature(unsigned long feature)
-{
-	return (CPU_FTRS_ALWAYS & feature) ||
-	       (CPU_FTRS_POSSIBLE
-		& cur_cpu_spec->cpu_features
-		& feature);
-}
-
 #define HBP_NUM 1
 
 #endif /* !__ASSEMBLY__ */
diff --git a/arch/powerpc/include/asm/cputime.h b/arch/powerpc/include/asm/cputime.h
index 607559a..15481e2 100644
--- a/arch/powerpc/include/asm/cputime.h
+++ b/arch/powerpc/include/asm/cputime.h
@@ -28,6 +28,7 @@ static inline void setup_cputime_one_jiffy(void) { }
 #include <asm/div64.h>
 #include <asm/time.h>
 #include <asm/param.h>
+#include <asm/cpufeatures.h>
 
 typedef u64 __nocast cputime_t;
 typedef u64 __nocast cputime64_t;
diff --git a/arch/powerpc/include/asm/dbell.h b/arch/powerpc/include/asm/dbell.h
index 5fa6b20..2d9eae3 100644
--- a/arch/powerpc/include/asm/dbell.h
+++ b/arch/powerpc/include/asm/dbell.h
@@ -16,6 +16,7 @@
 #include <linux/threads.h>
 
 #include <asm/ppc-opcode.h>
+#include <asm/cpufeatures.h>
 
 #define PPC_DBELL_MSG_BRDCAST	(0x04000000)
 #define PPC_DBELL_TYPE(x)	(((x) & 0xf) << (63-36))
diff --git a/arch/powerpc/include/asm/dcr-native.h b/arch/powerpc/include/asm/dcr-native.h
index 7d2e623..3372650 100644
--- a/arch/powerpc/include/asm/dcr-native.h
+++ b/arch/powerpc/include/asm/dcr-native.h
@@ -24,6 +24,7 @@
 
 #include <linux/spinlock.h>
 #include <asm/cputable.h>
+#include <asm/cpufeatures.h>
 
 typedef struct {
 	unsigned int base;
diff --git a/arch/powerpc/include/asm/mman.h b/arch/powerpc/include/asm/mman.h
index 8565c25..74922ad 100644
--- a/arch/powerpc/include/asm/mman.h
+++ b/arch/powerpc/include/asm/mman.h
@@ -13,6 +13,7 @@
 
 #include <asm/cputable.h>
 #include <linux/mm.h>
+#include <asm/cpufeatures.h>
 
 /*
  * This file is included by linux/mman.h, so we can't use cacl_vm_prot_bits()
diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
index c1f2676..20e6ee9 100644
--- a/arch/powerpc/include/asm/time.h
+++ b/arch/powerpc/include/asm/time.h
@@ -18,6 +18,7 @@
 #include <linux/percpu.h>
 
 #include <asm/processor.h>
+#include <asm/cpufeatures.h>
 
 /* time.c */
 extern unsigned long tb_ticks_per_jiffy;
diff --git a/arch/powerpc/kernel/align.c b/arch/powerpc/kernel/align.c
index ee5b690..ca4169a 100644
--- a/arch/powerpc/kernel/align.c
+++ b/arch/powerpc/kernel/align.c
@@ -25,6 +25,7 @@
 #include <asm/cputable.h>
 #include <asm/emulated_ops.h>
 #include <asm/switch_to.h>
+#include <asm/cpufeatures.h>
 
 struct aligninfo {
 	unsigned char len;
diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index c69440c..164a9ad 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -75,6 +75,7 @@
 #endif
 #define CREATE_TRACE_POINTS
 #include <asm/trace.h>
+#include <asm/cpufeatures.h>
 
 DEFINE_PER_CPU_SHARED_ALIGNED(irq_cpustat_t, irq_stat);
 EXPORT_PER_CPU_SYMBOL(irq_stat);
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 8083be2..90b7a27 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -57,6 +57,7 @@
 #endif
 #include <linux/kprobes.h>
 #include <linux/kdebug.h>
+#include <asm/cpufeatures.h>
 
 /* Transactional Memory debug */
 #ifdef TM_DEBUG_SW
diff --git a/arch/powerpc/kernel/setup-common.c b/arch/powerpc/kernel/setup-common.c
index 63d051f..d47b170 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -61,6 +61,7 @@
 #include <asm/cputhreads.h>
 #include <mm/mmu_decl.h>
 #include <asm/fadump.h>
+#include <asm/cpufeatures.h>
 
 #include "setup.h"
 
diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c
index a8f54ec..2f95dda 100644
--- a/arch/powerpc/kernel/setup_32.c
+++ b/arch/powerpc/kernel/setup_32.c
@@ -38,6 +38,7 @@
 #include <asm/serial.h>
 #include <asm/udbg.h>
 #include <asm/mmu_context.h>
+#include <asm/cpufeatures.h>
 
 #include "setup.h"
 
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 38b0ba6..1c15302 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -50,6 +50,7 @@
 #endif
 #include <asm/vdso.h>
 #include <asm/debug.h>
+#include <asm/cpufeatures.h>
 
 #ifdef DEBUG
 #include <asm/udbg.h>
diff --git a/arch/powerpc/oprofile/op_model_rs64.c b/arch/powerpc/oprofile/op_model_rs64.c
index 9b801b8..924b66c8 100644
--- a/arch/powerpc/oprofile/op_model_rs64.c
+++ b/arch/powerpc/oprofile/op_model_rs64.c
@@ -14,6 +14,7 @@
 #include <asm/processor.h>
 #include <asm/cputable.h>
 #include <asm/oprofile_impl.h>
+#include <asm/cpufeatures.h>
 
 #define dbg(args...)
 
diff --git a/arch/powerpc/platforms/cell/pervasive.c b/arch/powerpc/platforms/cell/pervasive.c
index d17e98b..036215b 100644
--- a/arch/powerpc/platforms/cell/pervasive.c
+++ b/arch/powerpc/platforms/cell/pervasive.c
@@ -35,6 +35,7 @@
 #include <asm/pgtable.h>
 #include <asm/reg.h>
 #include <asm/cell-regs.h>
+#include <asm/cpufeatures.h>
 
 #include "pervasive.h"
 
diff --git a/arch/powerpc/xmon/ppc-dis.c b/arch/powerpc/xmon/ppc-dis.c
index 89098f32..a642155 100644
--- a/arch/powerpc/xmon/ppc-dis.c
+++ b/arch/powerpc/xmon/ppc-dis.c
@@ -24,6 +24,7 @@ Software Foundation, 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, US
 #include "ansidecl.h"
 #include "ppc.h"
 #include "dis-asm.h"
+#include <asm/cpufeatures.h>
 
 /* Print a PowerPC or POWER instruction.  */
 
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH 2/5] jump_label: also include linux/atomic.h when jump label is enabled
From: Kevin Hao @ 2013-08-25  7:15 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc, linux-kernel
In-Reply-To: <1377414952-15995-1-git-send-email-haokexin@gmail.com>

The struct static_key will have a atomic_t type member no matter
whether jump label is enabled or not. We would include linux/atomic.h
when jump label is not enabled. But it also does make sense to include
this header file when jump label is enabled.

Signed-off-by: Kevin Hao <haokexin@gmail.com>
---
 include/linux/jump_label_base.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/jump_label_base.h b/include/linux/jump_label_base.h
index 20df08f..d5c8f4b 100644
--- a/include/linux/jump_label_base.h
+++ b/include/linux/jump_label_base.h
@@ -5,6 +5,8 @@
 #include <linux/types.h>
 #include <linux/compiler.h>
 
+#include <linux/atomic.h>
+
 #if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_JUMP_LABEL)
 
 struct static_key {
@@ -77,8 +79,6 @@ extern void jump_label_apply_nops(struct module *mod);
 
 #else  /* !HAVE_JUMP_LABEL */
 
-#include <linux/atomic.h>
-
 struct static_key {
 	atomic_t enabled;
 };
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH 0/5] powerpc: use jump label for cpu/mmu_has_feature
From: Kevin Hao @ 2013-08-25  7:15 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc, linux-kernel

Inspired by Benjamin Herrenschmidt, this patch series try to reduce the
cpu/mmu feature checking overhead by using jump label. The following is
the difference of the run path of cpu_has_feature between before and after
applying these patches:

   before                             after
  addis   r10,r2,1                   b xxx
  addi    r9,r10,-2280               b xxx (This will also be omitted if the
  ld      r9,0(r9)                          feature is not set)
  ld      r9,16(r9)
  rldicl. r8,r9,55,63
  beq     c000000000037c94

This patch series passed the build test for almost all the defconfig of ppc.
There does have some broken for some configs. But they are not related to this
change. This also passed allyesconfig for x86. Boot test on p2020rdb and
p5020ds boards.

Kevin Hao (5):
  jump_label: factor out the base part of jump_label.h to a separate    
    file
  jump_label: also include linux/atomic.h when jump label is enabled
  powerpc: move the cpu_has_feature to a separate file
  powerpc: use the jump label for cpu_has_feature
  powerpc: use jump label for mmu_has_feature

 arch/powerpc/include/asm/cacheflush.h   |   1 +
 arch/powerpc/include/asm/cpufeatures.h  |  42 ++++++++++
 arch/powerpc/include/asm/cputable.h     |   8 --
 arch/powerpc/include/asm/cputime.h      |   1 +
 arch/powerpc/include/asm/dbell.h        |   1 +
 arch/powerpc/include/asm/dcr-native.h   |   1 +
 arch/powerpc/include/asm/mman.h         |   1 +
 arch/powerpc/include/asm/mmu.h          |  19 +++++
 arch/powerpc/include/asm/time.h         |   1 +
 arch/powerpc/kernel/align.c             |   1 +
 arch/powerpc/kernel/cputable.c          |  43 ++++++++++
 arch/powerpc/kernel/irq.c               |   1 +
 arch/powerpc/kernel/process.c           |   1 +
 arch/powerpc/kernel/setup-common.c      |   1 +
 arch/powerpc/kernel/setup_32.c          |   1 +
 arch/powerpc/kernel/smp.c               |   1 +
 arch/powerpc/oprofile/op_model_rs64.c   |   1 +
 arch/powerpc/platforms/cell/pervasive.c |   1 +
 arch/powerpc/xmon/ppc-dis.c             |   1 +
 include/linux/jump_label.h              | 132 +----------------------------
 include/linux/jump_label_base.h         | 142 ++++++++++++++++++++++++++++++++
 21 files changed, 263 insertions(+), 138 deletions(-)
 create mode 100644 arch/powerpc/include/asm/cpufeatures.h
 create mode 100644 include/linux/jump_label_base.h

-- 
1.8.3.1

^ permalink raw reply

* Re: [PATCH V3] mmc:of_spi: Update the code of getting voltage-ranges
From: Chris Ball @ 2013-08-25  4:19 UTC (permalink / raw)
  To: Haijun Zhang; +Cc: linux-mmc, cbouatmailru, scottwood, linuxppc-dev, X.Xie
In-Reply-To: <1376271546-25085-3-git-send-email-Haijun.Zhang@freescale.com>

Hi Haijun,

On Sun, Aug 11 2013, Haijun Zhang wrote:
> Using function mmc_of_parse_voltage() to get voltage-ranges.
>
> Signed-off-by: Haijun Zhang <haijun.zhang@freescale.com>

The patchset contains patches 1-3 of 3, and also this unnumbered patch
v3.  Which order should I use to apply this patch?

Thanks,

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>

^ permalink raw reply

* [PATCH 2/2] ppc: kvm: use anon_inode_getfd() with O_CLOEXEC flag
From: Yann Droneaud @ 2013-08-24 20:14 UTC (permalink / raw)
  To: Alexander Graf, Gleb Natapov, Paolo Bonzini,
	Benjamin Herrenschmidt, Paul Mackerras
  Cc: Yann Droneaud, kvm, linux-kernel, kvm-ppc, Alex Williamson,
	linuxppc-dev
In-Reply-To: <cover.1377372576.git.ydroneaud@opteya.com>

KVM uses anon_inode_get() to allocate file descriptors as part
of some of its ioctls. But those ioctls are lacking a flag argument
allowing userspace to choose options for the newly opened file descriptor.

In such case it's advised to use O_CLOEXEC by default so that
userspace is allowed to choose, without race, if the file descriptor
is going to be inherited across exec().

This patch set O_CLOEXEC flag on all file descriptors created
with anon_inode_getfd() to not leak file descriptors across exec().

Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
Link: http://lkml.kernel.org/r/cover.1377372576.git.ydroneaud@opteya.com

---
 arch/powerpc/kvm/book3s_64_mmu_hv.c | 2 +-
 arch/powerpc/kvm/book3s_64_vio.c    | 2 +-
 arch/powerpc/kvm/book3s_hv.c        | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
index 710d313..f7c9e8a 100644
--- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
@@ -1579,7 +1579,7 @@ int kvm_vm_ioctl_get_htab_fd(struct kvm *kvm, struct kvm_get_htab_fd *ghf)
 	ctx->first_pass = 1;
 
 	rwflag = (ghf->flags & KVM_GET_HTAB_WRITE) ? O_WRONLY : O_RDONLY;
-	ret = anon_inode_getfd("kvm-htab", &kvm_htab_fops, ctx, rwflag);
+	ret = anon_inode_getfd("kvm-htab", &kvm_htab_fops, ctx, rwflag | O_CLOEXEC);
 	if (ret < 0) {
 		kvm_put_kvm(kvm);
 		return ret;
diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c
index b2d3f3b..54cf9bc 100644
--- a/arch/powerpc/kvm/book3s_64_vio.c
+++ b/arch/powerpc/kvm/book3s_64_vio.c
@@ -136,7 +136,7 @@ long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,
 	mutex_unlock(&kvm->lock);
 
 	return anon_inode_getfd("kvm-spapr-tce", &kvm_spapr_tce_fops,
-				stt, O_RDWR);
+				stt, O_RDWR | O_CLOEXEC);
 
 fail:
 	if (stt) {
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index e8d51cb..3503829 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -1556,7 +1556,7 @@ long kvm_vm_ioctl_allocate_rma(struct kvm *kvm, struct kvm_allocate_rma *ret)
 	if (!ri)
 		return -ENOMEM;
 
-	fd = anon_inode_getfd("kvm-rma", &kvm_rma_fops, ri, O_RDWR);
+	fd = anon_inode_getfd("kvm-rma", &kvm_rma_fops, ri, O_RDWR | O_CLOEXEC);
 	if (fd < 0)
 		kvm_release_rma(ri);
 
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH 0/2] kvm: use anon_inode_getfd() with O_CLOEXEC flag
From: Yann Droneaud @ 2013-08-24 20:14 UTC (permalink / raw)
  To: Alexander Graf, Gleb Natapov, Paolo Bonzini,
	Benjamin Herrenschmidt, Paul Mackerras
  Cc: Yann Droneaud, kvm, linux-kernel, kvm-ppc, Alex Williamson,
	linuxppc-dev

Hi,

Following a patchset asking to change calls to get_unused_flag() [1]
to use O_CLOEXEC, Alex Williamson [2][3] decided to change VFIO
to use the flag.

Since it's a related subsystem to KVM, using O_CLOEXEC for
file descriptors created by KVM might be applicable too.

I'm suggesting to change calls to anon_inode_getfd() to use O_CLOEXEC
as default flag.

This patchset should be reviewed to not break existing userspace program.

BTW, if it's not applicable, I would suggest that new ioctls be added to
KVM subsystem, those ioctls would have a "flag" field added to their arguments.
Such "flag" would let userspace choose the open flag to use.
See for example other APIs using anon_inode_getfd() such as fanotify,
inotify, signalfd and timerfd.

You might be interested to read:

- Secure File Descriptor Handling (Ulrich Drepper, 2008)
  http://udrepper.livejournal.com/20407.html

- Excuse me son, but your code is leaking !!! (Dan Walsh, March 2012) 
  http://danwalsh.livejournal.com/53603.html

Regards.

[1] http://lkml.kernel.org/r/cover.1376327678.git.ydroneaud@opteya.com
[2] http://lkml.kernel.org/r/1377186804.25163.17.camel@ul30vt.home
[3] http://lkml.kernel.org/r/20130822171744.1297.13711.stgit@bling.home

Yann Droneaud (2):
  kvm: use anon_inode_getfd() with O_CLOEXEC flag
  ppc: kvm: use anon_inode_getfd() with O_CLOEXEC flag

 arch/powerpc/kvm/book3s_64_mmu_hv.c | 2 +-
 arch/powerpc/kvm/book3s_64_vio.c    | 2 +-
 arch/powerpc/kvm/book3s_hv.c        | 2 +-
 virt/kvm/kvm_main.c                 | 6 +++---
 4 files changed, 6 insertions(+), 6 deletions(-)

-- 
1.8.3.1

^ permalink raw reply

* Re: Loading kernel on MPC86x
From: Martin Hinner @ 2013-08-24 17:15 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <CAPVwjkzkuw4cbo9YxnsskdoAda-k+Gb5sdVEJiZTVAD1nR5OOg@mail.gmail.com>

Hello again,

  just a quick update: I have spent some more time on this and now I
can boot into kernel (it works even with initramfs and simple assembly
HelloWorld, so it's time to compile userland now). The problem was
that kernel must be at location 0. Another problem was that interrupts
got re-enabled during execution of my bootloader (I am doing some
syscalls -> goes  to Cisco rom), otherwise it crashed randomly. And
finally it seems that stack must be < 8M. After this kernel boots.

Anyway, I would still appreciate clarification on vmlinux:__start
entry conditions:

- must be loaded at 0 (but why arch/powerpc/boot/main.c has option to
allocate space for kernel at nonzero addr ?)
- stack? Does it really have to be < 8M ?
- interrupts disabled (really ;-) )
- MSR.PR=0/LE=0/EE=0, but what other bits (RI? IP=0? ME?)
- IMMR 0xff000000
- and of course correct entry arguments in registers

anything else?

I am also curious why CONFIG_PPC_EARLY_DEBUG_CPM uses
CONFIG_PPC_EARLY_DEBUG_CPM_ADDR as pointer to transmit SMC buffer and
not address of CPM/SCM parameter RAM ? TX buffer address can be read
from SMC parameter RAM. Wouldn't this solution be more portable? At
least this way I do it when I take over console from Cisco
startup/rommon.

Martin

^ permalink raw reply

* Loading kernel on MPC86x
From: Martin Hinner @ 2013-08-24 14:54 UTC (permalink / raw)
  To: linuxppc-dev

Hi,

  I am trying to load kernel 2.6.39 on MPC86x-based Cisco 8xx router
(I need development platform for another embedded project and this
router was first suitable device I have).

So far I have:

- built crosscompiler
- created simple bootloader based on CILO code that can load ELF
(zImage) into memory, relocate and start it
- Modified Linux kernel arch/powerpc/boot directory so as it can
decompress kernel into memory and start it (__start gets executed)
- Created dts for the device (arch/powerpc/boot/main.c is making
printf via correct device without any hacks)

Most of tasks were accomplished by creating temporary debug output to
cpm-uart and finding problems step-by-step, however I am a bit stuck
with kernel now and I would appreciate some help from someone who has
already done similar task.

Hardware info: The MPC86x has 32MB SDRAM located at 0x00000 and
0x80000000 (I have not checked how this is done but it looks like it's
alias as both memory regions have the same data). IMMR is 0xff000000,
ROM is at 0xfff00000. Initial stack is in first 64kB (I left it as
Cisco rommon has set it up).

I had to modify arch/powerpc/boot/main.c so as it loads kernel to a
different location than 0 and does not overwrite rommon's exception
vectors & stack.

arch/powerpc/boot/main.c starts succesfully kernel (I can see debug
output produced by modified '__start' at
arch/powerpc/kernel/head_8xx.S). Unfortunately call to "bl
initial_mmu" fails (there is no exception that rommon catches,
processor just freezes and as I do not have BDM attached to it I have
no idea what is going wrong).

My feeling is that I should load vmlinux to address 0 as there is no
relocation code anywhere. Is this correct? I can live with it (stack
will be relocated in my bootloader, but before I start doing this work
(more than stack relocation is involved) I would like to make sure it
is really my problem).

Second, I would like to ask what else do I have to set-up before
jumping to arch/powerpc/kernel/head_8xx.S ; is vmlinux sitting at
0x00000 and "temporary" stack at safe place (end of mem) enough ? How
about interrupts ? What if some of them were enabled by Cisco
startup/rommon ?

Do I understand correctly that
arch/powerpc/kernel/head_8xx.S:initial_mmu (and then real mm handler)
sets-up MMU so as I can always access IMMR region and print debug
characters on console? (I am sorry I am not yet familiar with MPC8xx
MMU so the code is a bit confusing for me).

I do not understand how kernel base 0xC0000000 works; kernel is
normally loaded at 0, however all references go to 0xC0000000. Does
initial_mmu set-up "mapping" from 0xC0000000 to 0x00000 or do I miss
anything? When switch from 0 to 0xC0000000 happens?

Maybe a link to document describing how kernel on PowerPC starts would
clarify all my question, but all I have found is focused on using
existing bootloaders (U-Boot) with no detailed description ...

Thank you,

Martin

^ permalink raw reply

* Re: [PATCH V3] i2c: move of helpers into the core
From: Mauro Carvalho Chehab @ 2013-08-24 11:10 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: devel, devicetree, davinci-linux-open-source, linux-samsung-soc,
	alsa-devel, linux-doc, linux-kernel, dri-devel, linux-acpi,
	linux-i2c, linux-tegra, linux-omap, linuxppc-dev,
	linux-arm-kernel, linux-media
In-Reply-To: <1377187217-31820-1-git-send-email-wsa@the-dreams.de>

Em Thu, 22 Aug 2013 18:00:14 +0200
Wolfram Sang <wsa@the-dreams.de> escreveu:

> I2C of helpers used to live in of_i2c.c but experience (from SPI) shows
> that it is much cleaner to have this in the core. This also removes a
> circular dependency between the helpers and the core, and so we can
> finally register child nodes in the core instead of doing this manually
> in each driver. So, fix the drivers and documentation, too.
> 
> Acked-by: Rob Herring <rob.herring@calxeda.com>
> Reviewed-by: Felipe Balbi <balbi@ti.com>
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Tested-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Signed-off-by: Wolfram Sang <wsa@the-dreams.de>

Acked-by: Mauro Carvalho Chehab <m.chehab@samsung.com>

> ---
> 
> V2->V3: Was trying to be too smart by only fixing includes needed.
> 	Took a more general approach this time, converting of_i2c.h
> 	to i2c.h in case i2c.h was not already there. Otherwise
> 	remove it. Improved my build scripts and no build failures,
> 	no complaints from buildbot as well.
> 
> 
>  Documentation/acpi/enumeration.txt              |    1 -
>  arch/powerpc/platforms/44x/warp.c               |    1 -
>  drivers/gpu/drm/tilcdc/tilcdc_slave.c           |    1 -
>  drivers/gpu/drm/tilcdc/tilcdc_tfp410.c          |    1 -
>  drivers/gpu/host1x/drm/output.c                 |    2 +-
>  drivers/i2c/busses/i2c-at91.c                   |    3 -
>  drivers/i2c/busses/i2c-cpm.c                    |    6 --
>  drivers/i2c/busses/i2c-davinci.c                |    2 -
>  drivers/i2c/busses/i2c-designware-platdrv.c     |    2 -
>  drivers/i2c/busses/i2c-gpio.c                   |    3 -
>  drivers/i2c/busses/i2c-i801.c                   |    2 -
>  drivers/i2c/busses/i2c-ibm_iic.c                |    4 -
>  drivers/i2c/busses/i2c-imx.c                    |    3 -
>  drivers/i2c/busses/i2c-mpc.c                    |    2 -
>  drivers/i2c/busses/i2c-mv64xxx.c                |    3 -
>  drivers/i2c/busses/i2c-mxs.c                    |    3 -
>  drivers/i2c/busses/i2c-nomadik.c                |    3 -
>  drivers/i2c/busses/i2c-ocores.c                 |    3 -
>  drivers/i2c/busses/i2c-octeon.c                 |    3 -
>  drivers/i2c/busses/i2c-omap.c                   |    3 -
>  drivers/i2c/busses/i2c-pnx.c                    |    3 -
>  drivers/i2c/busses/i2c-powermac.c               |    9 +-
>  drivers/i2c/busses/i2c-pxa.c                    |    2 -
>  drivers/i2c/busses/i2c-s3c2410.c                |    2 -
>  drivers/i2c/busses/i2c-sh_mobile.c              |    2 -
>  drivers/i2c/busses/i2c-sirf.c                   |    3 -
>  drivers/i2c/busses/i2c-stu300.c                 |    2 -
>  drivers/i2c/busses/i2c-tegra.c                  |    3 -
>  drivers/i2c/busses/i2c-versatile.c              |    2 -
>  drivers/i2c/busses/i2c-wmt.c                    |    3 -
>  drivers/i2c/busses/i2c-xiic.c                   |    3 -
>  drivers/i2c/i2c-core.c                          |  109 +++++++++++++++++++++-
>  drivers/i2c/i2c-mux.c                           |    3 -
>  drivers/i2c/muxes/i2c-arb-gpio-challenge.c      |    1 -
>  drivers/i2c/muxes/i2c-mux-gpio.c                |    1 -
>  drivers/i2c/muxes/i2c-mux-pinctrl.c             |    1 -
>  drivers/media/platform/exynos4-is/fimc-is-i2c.c |    4 +-
>  drivers/media/platform/exynos4-is/fimc-is.c     |    2 +-
>  drivers/media/platform/exynos4-is/media-dev.c   |    1 -
>  drivers/of/Kconfig                              |    6 --
>  drivers/of/Makefile                             |    1 -
>  drivers/of/of_i2c.c                             |  114 -----------------------
>  drivers/staging/imx-drm/imx-tve.c               |    2 +-
>  include/linux/i2c.h                             |   20 ++++
>  include/linux/of_i2c.h                          |   46 ---------
>  sound/soc/fsl/imx-sgtl5000.c                    |    2 +-
>  sound/soc/fsl/imx-wm8962.c                      |    2 +-
>  47 files changed, 138 insertions(+), 262 deletions(-)
>  delete mode 100644 drivers/of/of_i2c.c
>  delete mode 100644 include/linux/of_i2c.h
> 
> diff --git a/Documentation/acpi/enumeration.txt b/Documentation/acpi/enumeration.txt
> index d9be7a9..958266e 100644
> --- a/Documentation/acpi/enumeration.txt
> +++ b/Documentation/acpi/enumeration.txt
> @@ -238,7 +238,6 @@ An I2C bus (controller) driver does:
>  	if (ret)
>  		/* handle error */
>  
> -	of_i2c_register_devices(adapter);
>  	/* Enumerate the slave devices behind this bus via ACPI */
>  	acpi_i2c_register_devices(adapter);
>  
> diff --git a/arch/powerpc/platforms/44x/warp.c b/arch/powerpc/platforms/44x/warp.c
> index 4cfa499..534574a 100644
> --- a/arch/powerpc/platforms/44x/warp.c
> +++ b/arch/powerpc/platforms/44x/warp.c
> @@ -16,7 +16,6 @@
>  #include <linux/interrupt.h>
>  #include <linux/delay.h>
>  #include <linux/of_gpio.h>
> -#include <linux/of_i2c.h>
>  #include <linux/slab.h>
>  #include <linux/export.h>
>  
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_slave.c b/drivers/gpu/drm/tilcdc/tilcdc_slave.c
> index dfffaf0..a19f657 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_slave.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_slave.c
> @@ -16,7 +16,6 @@
>   */
>  
>  #include <linux/i2c.h>
> -#include <linux/of_i2c.h>
>  #include <linux/pinctrl/pinmux.h>
>  #include <linux/pinctrl/consumer.h>
>  #include <drm/drm_encoder_slave.h>
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
> index 925c7cd..c38b56b 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
> @@ -16,7 +16,6 @@
>   */
>  
>  #include <linux/i2c.h>
> -#include <linux/of_i2c.h>
>  #include <linux/gpio.h>
>  #include <linux/of_gpio.h>
>  #include <linux/pinctrl/pinmux.h>
> diff --git a/drivers/gpu/host1x/drm/output.c b/drivers/gpu/host1x/drm/output.c
> index 8140fc6..137ae81 100644
> --- a/drivers/gpu/host1x/drm/output.c
> +++ b/drivers/gpu/host1x/drm/output.c
> @@ -9,7 +9,7 @@
>  
>  #include <linux/module.h>
>  #include <linux/of_gpio.h>
> -#include <linux/of_i2c.h>
> +#include <linux/i2c.h>
>  
>  #include "drm.h"
>  
> diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
> index 6bb839b..fd05930 100644
> --- a/drivers/i2c/busses/i2c-at91.c
> +++ b/drivers/i2c/busses/i2c-at91.c
> @@ -28,7 +28,6 @@
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> -#include <linux/of_i2c.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  #include <linux/platform_data/dma-atmel.h>
> @@ -775,8 +774,6 @@ static int at91_twi_probe(struct platform_device *pdev)
>  		return rc;
>  	}
>  
> -	of_i2c_register_devices(&dev->adapter);
> -
>  	dev_info(dev->dev, "AT91 i2c bus driver.\n");
>  	return 0;
>  }
> diff --git a/drivers/i2c/busses/i2c-cpm.c b/drivers/i2c/busses/i2c-cpm.c
> index 2e1f7eb..b2b8aa9 100644
> --- a/drivers/i2c/busses/i2c-cpm.c
> +++ b/drivers/i2c/busses/i2c-cpm.c
> @@ -42,7 +42,6 @@
>  #include <linux/dma-mapping.h>
>  #include <linux/of_device.h>
>  #include <linux/of_platform.h>
> -#include <linux/of_i2c.h>
>  #include <sysdev/fsl_soc.h>
>  #include <asm/cpm.h>
>  
> @@ -681,11 +680,6 @@ static int cpm_i2c_probe(struct platform_device *ofdev)
>  	dev_dbg(&ofdev->dev, "hw routines for %s registered.\n",
>  		cpm->adap.name);
>  
> -	/*
> -	 * register OF I2C devices
> -	 */
> -	of_i2c_register_devices(&cpm->adap);
> -
>  	return 0;
>  out_shut:
>  	cpm_i2c_shutdown(cpm);
> diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
> index fa55605..62be3b3 100644
> --- a/drivers/i2c/busses/i2c-davinci.c
> +++ b/drivers/i2c/busses/i2c-davinci.c
> @@ -38,7 +38,6 @@
>  #include <linux/slab.h>
>  #include <linux/cpufreq.h>
>  #include <linux/gpio.h>
> -#include <linux/of_i2c.h>
>  #include <linux/of_device.h>
>  
>  #include <mach/hardware.h>
> @@ -728,7 +727,6 @@ static int davinci_i2c_probe(struct platform_device *pdev)
>  		dev_err(&pdev->dev, "failure adding adapter\n");
>  		goto err_unuse_clocks;
>  	}
> -	of_i2c_register_devices(adap);
>  
>  	return 0;
>  
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> index 4c5fada..27ea436 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -35,7 +35,6 @@
>  #include <linux/err.h>
>  #include <linux/interrupt.h>
>  #include <linux/of.h>
> -#include <linux/of_i2c.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm.h>
>  #include <linux/pm_runtime.h>
> @@ -172,7 +171,6 @@ static int dw_i2c_probe(struct platform_device *pdev)
>  		dev_err(&pdev->dev, "failure adding adapter\n");
>  		return r;
>  	}
> -	of_i2c_register_devices(adap);
>  	acpi_i2c_register_devices(adap);
>  
>  	pm_runtime_set_autosuspend_delay(&pdev->dev, 1000);
> diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
> index bc6e139..e5da9fe 100644
> --- a/drivers/i2c/busses/i2c-gpio.c
> +++ b/drivers/i2c/busses/i2c-gpio.c
> @@ -16,7 +16,6 @@
>  #include <linux/platform_device.h>
>  #include <linux/gpio.h>
>  #include <linux/of_gpio.h>
> -#include <linux/of_i2c.h>
>  
>  struct i2c_gpio_private_data {
>  	struct i2c_adapter adap;
> @@ -224,8 +223,6 @@ static int i2c_gpio_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto err_add_bus;
>  
> -	of_i2c_register_devices(adap);
> -
>  	platform_set_drvdata(pdev, priv);
>  
>  	dev_info(&pdev->dev, "using pins %u (SDA) and %u (SCL%s)\n",
> diff --git a/drivers/i2c/busses/i2c-i801.c b/drivers/i2c/busses/i2c-i801.c
> index 4ebceed..4296d17 100644
> --- a/drivers/i2c/busses/i2c-i801.c
> +++ b/drivers/i2c/busses/i2c-i801.c
> @@ -87,7 +87,6 @@
>  #include <linux/slab.h>
>  #include <linux/wait.h>
>  #include <linux/err.h>
> -#include <linux/of_i2c.h>
>  
>  #if (defined CONFIG_I2C_MUX_GPIO || defined CONFIG_I2C_MUX_GPIO_MODULE) && \
>  		defined CONFIG_DMI
> @@ -1230,7 +1229,6 @@ static int i801_probe(struct pci_dev *dev, const struct pci_device_id *id)
>  		goto exit_free_irq;
>  	}
>  
> -	of_i2c_register_devices(&priv->adapter);
>  	i801_probe_optional_slaves(priv);
>  	/* We ignore errors - multiplexing is optional */
>  	i801_add_mux(priv);
> diff --git a/drivers/i2c/busses/i2c-ibm_iic.c b/drivers/i2c/busses/i2c-ibm_iic.c
> index 973f516..ff3caa0 100644
> --- a/drivers/i2c/busses/i2c-ibm_iic.c
> +++ b/drivers/i2c/busses/i2c-ibm_iic.c
> @@ -42,7 +42,6 @@
>  #include <linux/io.h>
>  #include <linux/i2c.h>
>  #include <linux/of_platform.h>
> -#include <linux/of_i2c.h>
>  
>  #include "i2c-ibm_iic.h"
>  
> @@ -759,9 +758,6 @@ static int iic_probe(struct platform_device *ofdev)
>  	dev_info(&ofdev->dev, "using %s mode\n",
>  		 dev->fast_mode ? "fast (400 kHz)" : "standard (100 kHz)");
>  
> -	/* Now register all the child nodes */
> -	of_i2c_register_devices(adap);
> -
>  	return 0;
>  
>  error_cleanup:
> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
> index e242797..bbbea6b 100644
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -50,7 +50,6 @@
>  #include <linux/slab.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> -#include <linux/of_i2c.h>
>  #include <linux/platform_data/i2c-imx.h>
>  
>  /** Defines ********************************************************************
> @@ -570,8 +569,6 @@ static int __init i2c_imx_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> -	of_i2c_register_devices(&i2c_imx->adapter);
> -
>  	/* Set up platform driver data */
>  	platform_set_drvdata(pdev, i2c_imx);
>  
> diff --git a/drivers/i2c/busses/i2c-mpc.c b/drivers/i2c/busses/i2c-mpc.c
> index 7607dc0..9f2513d 100644
> --- a/drivers/i2c/busses/i2c-mpc.c
> +++ b/drivers/i2c/busses/i2c-mpc.c
> @@ -18,7 +18,6 @@
>  #include <linux/sched.h>
>  #include <linux/init.h>
>  #include <linux/of_platform.h>
> -#include <linux/of_i2c.h>
>  #include <linux/slab.h>
>  
>  #include <linux/io.h>
> @@ -691,7 +690,6 @@ static int fsl_i2c_probe(struct platform_device *op)
>  		dev_err(i2c->dev, "failed to add adapter\n");
>  		goto fail_add;
>  	}
> -	of_i2c_register_devices(&i2c->adap);
>  
>  	return result;
>  
> diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
> index b1f42bf..8220322 100644
> --- a/drivers/i2c/busses/i2c-mv64xxx.c
> +++ b/drivers/i2c/busses/i2c-mv64xxx.c
> @@ -21,7 +21,6 @@
>  #include <linux/of.h>
>  #include <linux/of_device.h>
>  #include <linux/of_irq.h>
> -#include <linux/of_i2c.h>
>  #include <linux/clk.h>
>  #include <linux/err.h>
>  
> @@ -689,8 +688,6 @@ mv64xxx_i2c_probe(struct platform_device *pd)
>  		goto exit_free_irq;
>  	}
>  
> -	of_i2c_register_devices(&drv_data->adapter);
> -
>  	return 0;
>  
>  exit_free_irq:
> diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
> index df8ff5a..62ed07d 100644
> --- a/drivers/i2c/busses/i2c-mxs.c
> +++ b/drivers/i2c/busses/i2c-mxs.c
> @@ -27,7 +27,6 @@
>  #include <linux/stmp_device.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> -#include <linux/of_i2c.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/dmaengine.h>
>  
> @@ -701,8 +700,6 @@ static int mxs_i2c_probe(struct platform_device *pdev)
>  		return err;
>  	}
>  
> -	of_i2c_register_devices(adap);
> -
>  	return 0;
>  }
>  
> diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
> index 512dfe6..519df17 100644
> --- a/drivers/i2c/busses/i2c-nomadik.c
> +++ b/drivers/i2c/busses/i2c-nomadik.c
> @@ -24,7 +24,6 @@
>  #include <linux/pm_runtime.h>
>  #include <linux/platform_data/i2c-nomadik.h>
>  #include <linux/of.h>
> -#include <linux/of_i2c.h>
>  #include <linux/pinctrl/consumer.h>
>  
>  #define DRIVER_NAME "nmk-i2c"
> @@ -1045,8 +1044,6 @@ static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
>  		goto err_add_adap;
>  	}
>  
> -	of_i2c_register_devices(adap);
> -
>  	pm_runtime_put(&adev->dev);
>  
>  	return 0;
> diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c
> index 0e1f824..0a52b78 100644
> --- a/drivers/i2c/busses/i2c-ocores.c
> +++ b/drivers/i2c/busses/i2c-ocores.c
> @@ -24,7 +24,6 @@
>  #include <linux/i2c-ocores.h>
>  #include <linux/slab.h>
>  #include <linux/io.h>
> -#include <linux/of_i2c.h>
>  #include <linux/log2.h>
>  
>  struct ocores_i2c {
> @@ -435,8 +434,6 @@ static int ocores_i2c_probe(struct platform_device *pdev)
>  	if (pdata) {
>  		for (i = 0; i < pdata->num_devices; i++)
>  			i2c_new_device(&i2c->adap, pdata->devices + i);
> -	} else {
> -		of_i2c_register_devices(&i2c->adap);
>  	}
>  
>  	return 0;
> diff --git a/drivers/i2c/busses/i2c-octeon.c b/drivers/i2c/busses/i2c-octeon.c
> index 956fe32..b929ba2 100644
> --- a/drivers/i2c/busses/i2c-octeon.c
> +++ b/drivers/i2c/busses/i2c-octeon.c
> @@ -15,7 +15,6 @@
>  #include <linux/interrupt.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> -#include <linux/of_i2c.h>
>  #include <linux/delay.h>
>  #include <linux/sched.h>
>  #include <linux/slab.h>
> @@ -599,8 +598,6 @@ static int octeon_i2c_probe(struct platform_device *pdev)
>  	}
>  	dev_info(i2c->dev, "version %s\n", DRV_VERSION);
>  
> -	of_i2c_register_devices(&i2c->adap);
> -
>  	return 0;
>  
>  out:
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index 142b694d..a9f0f80 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -38,7 +38,6 @@
>  #include <linux/clk.h>
>  #include <linux/io.h>
>  #include <linux/of.h>
> -#include <linux/of_i2c.h>
>  #include <linux/of_device.h>
>  #include <linux/slab.h>
>  #include <linux/i2c-omap.h>
> @@ -1245,8 +1244,6 @@ omap_i2c_probe(struct platform_device *pdev)
>  	dev_info(dev->dev, "bus %d rev%d.%d at %d kHz\n", adap->nr,
>  		 major, minor, dev->speed);
>  
> -	of_i2c_register_devices(adap);
> -
>  	pm_runtime_mark_last_busy(dev->dev);
>  	pm_runtime_put_autosuspend(dev->dev);
>  
> diff --git a/drivers/i2c/busses/i2c-pnx.c b/drivers/i2c/busses/i2c-pnx.c
> index 5f39c6d..7b57d67 100644
> --- a/drivers/i2c/busses/i2c-pnx.c
> +++ b/drivers/i2c/busses/i2c-pnx.c
> @@ -23,7 +23,6 @@
>  #include <linux/err.h>
>  #include <linux/clk.h>
>  #include <linux/slab.h>
> -#include <linux/of_i2c.h>
>  
>  #define I2C_PNX_TIMEOUT_DEFAULT		10 /* msec */
>  #define I2C_PNX_SPEED_KHZ_DEFAULT	100
> @@ -741,8 +740,6 @@ static int i2c_pnx_probe(struct platform_device *pdev)
>  		goto out_irq;
>  	}
>  
> -	of_i2c_register_devices(&alg_data->adapter);
> -
>  	dev_dbg(&pdev->dev, "%s: Master at %#8x, irq %d.\n",
>  		alg_data->adapter.name, res->start, alg_data->irq);
>  
> diff --git a/drivers/i2c/busses/i2c-powermac.c b/drivers/i2c/busses/i2c-powermac.c
> index 8dc90da..1010f2e 100644
> --- a/drivers/i2c/busses/i2c-powermac.c
> +++ b/drivers/i2c/busses/i2c-powermac.c
> @@ -440,7 +440,9 @@ static int i2c_powermac_probe(struct platform_device *dev)
>  	adapter->algo = &i2c_powermac_algorithm;
>  	i2c_set_adapdata(adapter, bus);
>  	adapter->dev.parent = &dev->dev;
> -	adapter->dev.of_node = dev->dev.of_node;
> +
> +	/* Clear of_node to skip automatic registration of i2c child nodes */
> +	adapter->dev.of_node = NULL;
>  	rc = i2c_add_adapter(adapter);
>  	if (rc) {
>  		printk(KERN_ERR "i2c-powermac: Adapter %s registration "
> @@ -450,9 +452,8 @@ static int i2c_powermac_probe(struct platform_device *dev)
>  
>  	printk(KERN_INFO "PowerMac i2c bus %s registered\n", adapter->name);
>  
> -	/* Cannot use of_i2c_register_devices() due to Apple device-tree
> -	 * funkyness
> -	 */
> +	/* Use custom child registration due to Apple device-tree funkyness */
> +	adapter->dev.of_node = dev->dev.of_node;
>  	i2c_powermac_register_devices(adapter, bus);
>  
>  	return rc;
> diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
> index fbafed2..bc65014 100644
> --- a/drivers/i2c/busses/i2c-pxa.c
> +++ b/drivers/i2c/busses/i2c-pxa.c
> @@ -31,7 +31,6 @@
>  #include <linux/i2c-pxa.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> -#include <linux/of_i2c.h>
>  #include <linux/platform_device.h>
>  #include <linux/err.h>
>  #include <linux/clk.h>
> @@ -1185,7 +1184,6 @@ static int i2c_pxa_probe(struct platform_device *dev)
>  		printk(KERN_INFO "I2C: Failed to add bus\n");
>  		goto eadapt;
>  	}
> -	of_i2c_register_devices(&i2c->adap);
>  
>  	platform_set_drvdata(dev, i2c);
>  
> diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
> index cab1c91..643426e 100644
> --- a/drivers/i2c/busses/i2c-s3c2410.c
> +++ b/drivers/i2c/busses/i2c-s3c2410.c
> @@ -36,7 +36,6 @@
>  #include <linux/cpufreq.h>
>  #include <linux/slab.h>
>  #include <linux/io.h>
> -#include <linux/of_i2c.h>
>  #include <linux/of_gpio.h>
>  #include <linux/pinctrl/consumer.h>
>  
> @@ -1154,7 +1153,6 @@ static int s3c24xx_i2c_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> -	of_i2c_register_devices(&i2c->adap);
>  	platform_set_drvdata(pdev, i2c);
>  
>  	pm_runtime_enable(&pdev->dev);
> diff --git a/drivers/i2c/busses/i2c-sh_mobile.c b/drivers/i2c/busses/i2c-sh_mobile.c
> index debf745..aa1268f 100644
> --- a/drivers/i2c/busses/i2c-sh_mobile.c
> +++ b/drivers/i2c/busses/i2c-sh_mobile.c
> @@ -27,7 +27,6 @@
>  #include <linux/platform_device.h>
>  #include <linux/interrupt.h>
>  #include <linux/i2c.h>
> -#include <linux/of_i2c.h>
>  #include <linux/err.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/clk.h>
> @@ -758,7 +757,6 @@ static int sh_mobile_i2c_probe(struct platform_device *dev)
>  		 "I2C adapter %d with bus speed %lu Hz (L/H=%x/%x)\n",
>  		 adap->nr, pd->bus_speed, pd->iccl, pd->icch);
>  
> -	of_i2c_register_devices(adap);
>  	return 0;
>  
>   err_all:
> diff --git a/drivers/i2c/busses/i2c-sirf.c b/drivers/i2c/busses/i2c-sirf.c
> index a63c7d5..0ff22e2 100644
> --- a/drivers/i2c/busses/i2c-sirf.c
> +++ b/drivers/i2c/busses/i2c-sirf.c
> @@ -12,7 +12,6 @@
>  #include <linux/slab.h>
>  #include <linux/platform_device.h>
>  #include <linux/i2c.h>
> -#include <linux/of_i2c.h>
>  #include <linux/clk.h>
>  #include <linux/err.h>
>  #include <linux/io.h>
> @@ -366,8 +365,6 @@ static int i2c_sirfsoc_probe(struct platform_device *pdev)
>  
>  	clk_disable(clk);
>  
> -	of_i2c_register_devices(adap);
> -
>  	dev_info(&pdev->dev, " I2C adapter ready to operate\n");
>  
>  	return 0;
> diff --git a/drivers/i2c/busses/i2c-stu300.c b/drivers/i2c/busses/i2c-stu300.c
> index d1a6b20..047546c 100644
> --- a/drivers/i2c/busses/i2c-stu300.c
> +++ b/drivers/i2c/busses/i2c-stu300.c
> @@ -17,7 +17,6 @@
>  #include <linux/clk.h>
>  #include <linux/io.h>
>  #include <linux/slab.h>
> -#include <linux/of_i2c.h>
>  
>  /* the name of this kernel module */
>  #define NAME "stu300"
> @@ -936,7 +935,6 @@ stu300_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, dev);
>  	dev_info(&pdev->dev, "ST DDC I2C @ %p, irq %d\n",
>  		 dev->virtbase, dev->irq);
> -	of_i2c_register_devices(adap);
>  
>  	return 0;
>  }
> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> index 9aa1b60..c457cb4 100644
> --- a/drivers/i2c/busses/i2c-tegra.c
> +++ b/drivers/i2c/busses/i2c-tegra.c
> @@ -25,7 +25,6 @@
>  #include <linux/interrupt.h>
>  #include <linux/delay.h>
>  #include <linux/slab.h>
> -#include <linux/of_i2c.h>
>  #include <linux/of_device.h>
>  #include <linux/module.h>
>  #include <linux/clk/tegra.h>
> @@ -802,8 +801,6 @@ static int tegra_i2c_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> -	of_i2c_register_devices(&i2c_dev->adapter);
> -
>  	return 0;
>  }
>  
> diff --git a/drivers/i2c/busses/i2c-versatile.c b/drivers/i2c/busses/i2c-versatile.c
> index f3a8790..6bb3a89 100644
> --- a/drivers/i2c/busses/i2c-versatile.c
> +++ b/drivers/i2c/busses/i2c-versatile.c
> @@ -16,7 +16,6 @@
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
>  #include <linux/io.h>
> -#include <linux/of_i2c.h>
>  
>  #define I2C_CONTROL	0x00
>  #define I2C_CONTROLS	0x00
> @@ -108,7 +107,6 @@ static int i2c_versatile_probe(struct platform_device *dev)
>  	ret = i2c_bit_add_numbered_bus(&i2c->adap);
>  	if (ret >= 0) {
>  		platform_set_drvdata(dev, i2c);
> -		of_i2c_register_devices(&i2c->adap);
>  		return 0;
>  	}
>  
> diff --git a/drivers/i2c/busses/i2c-wmt.c b/drivers/i2c/busses/i2c-wmt.c
> index baaa7d1..c65da3d 100644
> --- a/drivers/i2c/busses/i2c-wmt.c
> +++ b/drivers/i2c/busses/i2c-wmt.c
> @@ -21,7 +21,6 @@
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_address.h>
> -#include <linux/of_i2c.h>
>  #include <linux/of_irq.h>
>  #include <linux/platform_device.h>
>  
> @@ -439,8 +438,6 @@ static int wmt_i2c_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, i2c_dev);
>  
> -	of_i2c_register_devices(adap);
> -
>  	return 0;
>  }
>  
> diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
> index 3d0f052..8823db7 100644
> --- a/drivers/i2c/busses/i2c-xiic.c
> +++ b/drivers/i2c/busses/i2c-xiic.c
> @@ -40,7 +40,6 @@
>  #include <linux/i2c-xiic.h>
>  #include <linux/io.h>
>  #include <linux/slab.h>
> -#include <linux/of_i2c.h>
>  
>  #define DRIVER_NAME "xiic-i2c"
>  
> @@ -752,8 +751,6 @@ static int xiic_i2c_probe(struct platform_device *pdev)
>  			i2c_new_device(&i2c->adap, pdata->devices + i);
>  	}
>  
> -	of_i2c_register_devices(&i2c->adap);
> -
>  	return 0;
>  
>  add_adapter_failed:
> diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> index f32ca29..08ebd78 100644
> --- a/drivers/i2c/i2c-core.c
> +++ b/drivers/i2c/i2c-core.c
> @@ -23,7 +23,11 @@
>     SMBus 2.0 support by Mark Studebaker <mdsxyz123@yahoo.com> and
>     Jean Delvare <khali@linux-fr.org>
>     Mux support by Rodolfo Giometti <giometti@enneenne.com> and
> -   Michael Lawnick <michael.lawnick.ext@nsn.com> */
> +   Michael Lawnick <michael.lawnick.ext@nsn.com>
> +   OF support is copyright (c) 2008 Jochen Friedrich <jochen@scram.de>
> +   (based on a previous patch from Jon Smirl <jonsmirl@gmail.com>) and
> +   (c) 2013  Wolfram Sang <wsa@the-dreams.de>
> + */
>  
>  #include <linux/module.h>
>  #include <linux/kernel.h>
> @@ -35,7 +39,9 @@
>  #include <linux/init.h>
>  #include <linux/idr.h>
>  #include <linux/mutex.h>
> +#include <linux/of.h>
>  #include <linux/of_device.h>
> +#include <linux/of_irq.h>
>  #include <linux/completion.h>
>  #include <linux/hardirq.h>
>  #include <linux/irqflags.h>
> @@ -954,6 +960,104 @@ static void i2c_scan_static_board_info(struct i2c_adapter *adapter)
>  	up_read(&__i2c_board_lock);
>  }
>  
> +/* OF support code */
> +
> +#if IS_ENABLED(CONFIG_OF)
> +static void of_i2c_register_devices(struct i2c_adapter *adap)
> +{
> +	void *result;
> +	struct device_node *node;
> +
> +	/* Only register child devices if the adapter has a node pointer set */
> +	if (!adap->dev.of_node)
> +		return;
> +
> +	dev_dbg(&adap->dev, "of_i2c: walking child nodes\n");
> +
> +	for_each_available_child_of_node(adap->dev.of_node, node) {
> +		struct i2c_board_info info = {};
> +		struct dev_archdata dev_ad = {};
> +		const __be32 *addr;
> +		int len;
> +
> +		dev_dbg(&adap->dev, "of_i2c: register %s\n", node->full_name);
> +
> +		if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) {
> +			dev_err(&adap->dev, "of_i2c: modalias failure on %s\n",
> +				node->full_name);
> +			continue;
> +		}
> +
> +		addr = of_get_property(node, "reg", &len);
> +		if (!addr || (len < sizeof(int))) {
> +			dev_err(&adap->dev, "of_i2c: invalid reg on %s\n",
> +				node->full_name);
> +			continue;
> +		}
> +
> +		info.addr = be32_to_cpup(addr);
> +		if (info.addr > (1 << 10) - 1) {
> +			dev_err(&adap->dev, "of_i2c: invalid addr=%x on %s\n",
> +				info.addr, node->full_name);
> +			continue;
> +		}
> +
> +		info.irq = irq_of_parse_and_map(node, 0);
> +		info.of_node = of_node_get(node);
> +		info.archdata = &dev_ad;
> +
> +		if (of_get_property(node, "wakeup-source", NULL))
> +			info.flags |= I2C_CLIENT_WAKE;
> +
> +		request_module("%s%s", I2C_MODULE_PREFIX, info.type);
> +
> +		result = i2c_new_device(adap, &info);
> +		if (result == NULL) {
> +			dev_err(&adap->dev, "of_i2c: Failure registering %s\n",
> +				node->full_name);
> +			of_node_put(node);
> +			irq_dispose_mapping(info.irq);
> +			continue;
> +		}
> +	}
> +}
> +
> +static int of_dev_node_match(struct device *dev, void *data)
> +{
> +	return dev->of_node == data;
> +}
> +
> +/* must call put_device() when done with returned i2c_client device */
> +struct i2c_client *of_find_i2c_device_by_node(struct device_node *node)
> +{
> +	struct device *dev;
> +
> +	dev = bus_find_device(&i2c_bus_type, NULL, node,
> +					 of_dev_node_match);
> +	if (!dev)
> +		return NULL;
> +
> +	return i2c_verify_client(dev);
> +}
> +EXPORT_SYMBOL(of_find_i2c_device_by_node);
> +
> +/* must call put_device() when done with returned i2c_adapter device */
> +struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node)
> +{
> +	struct device *dev;
> +
> +	dev = bus_find_device(&i2c_bus_type, NULL, node,
> +					 of_dev_node_match);
> +	if (!dev)
> +		return NULL;
> +
> +	return i2c_verify_adapter(dev);
> +}
> +EXPORT_SYMBOL(of_find_i2c_adapter_by_node);
> +#else
> +static void of_i2c_register_devices(struct i2c_adapter *adap) { }
> +#endif /* CONFIG_OF */
> +
>  static int i2c_do_add_adapter(struct i2c_driver *driver,
>  			      struct i2c_adapter *adap)
>  {
> @@ -1058,6 +1162,8 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
>  
>  exit_recovery:
>  	/* create pre-declared device nodes */
> +	of_i2c_register_devices(adap);
> +
>  	if (adap->nr < __i2c_first_dynamic_bus_num)
>  		i2c_scan_static_board_info(adap);
>  
> @@ -1282,7 +1388,6 @@ void i2c_del_adapter(struct i2c_adapter *adap)
>  }
>  EXPORT_SYMBOL(i2c_del_adapter);
>  
> -
>  /* ------------------------------------------------------------------------- */
>  
>  int i2c_for_each_dev(void *data, int (*fn)(struct device *, void *))
> diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
> index 7409ebb..797e311 100644
> --- a/drivers/i2c/i2c-mux.c
> +++ b/drivers/i2c/i2c-mux.c
> @@ -25,7 +25,6 @@
>  #include <linux/i2c.h>
>  #include <linux/i2c-mux.h>
>  #include <linux/of.h>
> -#include <linux/of_i2c.h>
>  
>  /* multiplexer per channel data */
>  struct i2c_mux_priv {
> @@ -185,8 +184,6 @@ struct i2c_adapter *i2c_add_mux_adapter(struct i2c_adapter *parent,
>  	dev_info(&parent->dev, "Added multiplexed i2c bus %d\n",
>  		 i2c_adapter_id(&priv->adap));
>  
> -	of_i2c_register_devices(&priv->adap);
> -
>  	return &priv->adap;
>  }
>  EXPORT_SYMBOL_GPL(i2c_add_mux_adapter);
> diff --git a/drivers/i2c/muxes/i2c-arb-gpio-challenge.c b/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
> index 210b6f7..b901638 100644
> --- a/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
> +++ b/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
> @@ -21,7 +21,6 @@
>  #include <linux/i2c-mux.h>
>  #include <linux/init.h>
>  #include <linux/module.h>
> -#include <linux/of_i2c.h>
>  #include <linux/of_gpio.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
> diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
> index 5a0ce00..128a981 100644
> --- a/drivers/i2c/muxes/i2c-mux-gpio.c
> +++ b/drivers/i2c/muxes/i2c-mux-gpio.c
> @@ -16,7 +16,6 @@
>  #include <linux/module.h>
>  #include <linux/slab.h>
>  #include <linux/gpio.h>
> -#include <linux/of_i2c.h>
>  #include <linux/of_gpio.h>
>  
>  struct gpiomux {
> diff --git a/drivers/i2c/muxes/i2c-mux-pinctrl.c b/drivers/i2c/muxes/i2c-mux-pinctrl.c
> index a43c0ce..859a6d2 100644
> --- a/drivers/i2c/muxes/i2c-mux-pinctrl.c
> +++ b/drivers/i2c/muxes/i2c-mux-pinctrl.c
> @@ -20,7 +20,6 @@
>  #include <linux/i2c-mux.h>
>  #include <linux/init.h>
>  #include <linux/module.h>
> -#include <linux/of_i2c.h>
>  #include <linux/pinctrl/consumer.h>
>  #include <linux/i2c-mux-pinctrl.h>
>  #include <linux/platform_device.h>
> diff --git a/drivers/media/platform/exynos4-is/fimc-is-i2c.c b/drivers/media/platform/exynos4-is/fimc-is-i2c.c
> index 617a798..9930556 100644
> --- a/drivers/media/platform/exynos4-is/fimc-is-i2c.c
> +++ b/drivers/media/platform/exynos4-is/fimc-is-i2c.c
> @@ -12,7 +12,7 @@
>  
>  #include <linux/clk.h>
>  #include <linux/module.h>
> -#include <linux/of_i2c.h>
> +#include <linux/i2c.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/slab.h>
> @@ -67,8 +67,6 @@ static int fimc_is_i2c_probe(struct platform_device *pdev)
>  	pm_runtime_enable(&pdev->dev);
>  	pm_runtime_enable(&i2c_adap->dev);
>  
> -	of_i2c_register_devices(i2c_adap);
> -
>  	return 0;
>  }
>  
> diff --git a/drivers/media/platform/exynos4-is/fimc-is.c b/drivers/media/platform/exynos4-is/fimc-is.c
> index 967f6a9..2276fdc 100644
> --- a/drivers/media/platform/exynos4-is/fimc-is.c
> +++ b/drivers/media/platform/exynos4-is/fimc-is.c
> @@ -21,7 +21,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> -#include <linux/of_i2c.h>
> +#include <linux/i2c.h>
>  #include <linux/of_irq.h>
>  #include <linux/of_address.h>
>  #include <linux/of_platform.h>
> diff --git a/drivers/media/platform/exynos4-is/media-dev.c b/drivers/media/platform/exynos4-is/media-dev.c
> index 19f556c..f8c66b4 100644
> --- a/drivers/media/platform/exynos4-is/media-dev.c
> +++ b/drivers/media/platform/exynos4-is/media-dev.c
> @@ -20,7 +20,6 @@
>  #include <linux/of.h>
>  #include <linux/of_platform.h>
>  #include <linux/of_device.h>
> -#include <linux/of_i2c.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/types.h>
> diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
> index 80e5c13..78cc760 100644
> --- a/drivers/of/Kconfig
> +++ b/drivers/of/Kconfig
> @@ -48,12 +48,6 @@ config OF_IRQ
>  	def_bool y
>  	depends on !SPARC
>  
> -config OF_I2C
> -	def_tristate I2C
> -	depends on I2C
> -	help
> -	  OpenFirmware I2C accessors
> -
>  config OF_NET
>  	depends on NETDEVICES
>  	def_bool y
> diff --git a/drivers/of/Makefile b/drivers/of/Makefile
> index 1f9c0c4..efd0510 100644
> --- a/drivers/of/Makefile
> +++ b/drivers/of/Makefile
> @@ -3,7 +3,6 @@ obj-$(CONFIG_OF_FLATTREE) += fdt.o
>  obj-$(CONFIG_OF_PROMTREE) += pdt.o
>  obj-$(CONFIG_OF_ADDRESS)  += address.o
>  obj-$(CONFIG_OF_IRQ)    += irq.o
> -obj-$(CONFIG_OF_I2C)	+= of_i2c.o
>  obj-$(CONFIG_OF_NET)	+= of_net.o
>  obj-$(CONFIG_OF_SELFTEST) += selftest.o
>  obj-$(CONFIG_OF_MDIO)	+= of_mdio.o
> diff --git a/drivers/of/of_i2c.c b/drivers/of/of_i2c.c
> deleted file mode 100644
> index b667264..0000000
> --- a/drivers/of/of_i2c.c
> +++ /dev/null
> @@ -1,114 +0,0 @@
> -/*
> - * OF helpers for the I2C API
> - *
> - * Copyright (c) 2008 Jochen Friedrich <jochen@scram.de>
> - *
> - * Based on a previous patch from Jon Smirl <jonsmirl@gmail.com>
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or
> - * (at your option) any later version.
> - */
> -
> -#include <linux/i2c.h>
> -#include <linux/irq.h>
> -#include <linux/of.h>
> -#include <linux/of_i2c.h>
> -#include <linux/of_irq.h>
> -#include <linux/module.h>
> -
> -void of_i2c_register_devices(struct i2c_adapter *adap)
> -{
> -	void *result;
> -	struct device_node *node;
> -
> -	/* Only register child devices if the adapter has a node pointer set */
> -	if (!adap->dev.of_node)
> -		return;
> -
> -	dev_dbg(&adap->dev, "of_i2c: walking child nodes\n");
> -
> -	for_each_available_child_of_node(adap->dev.of_node, node) {
> -		struct i2c_board_info info = {};
> -		struct dev_archdata dev_ad = {};
> -		const __be32 *addr;
> -		int len;
> -
> -		dev_dbg(&adap->dev, "of_i2c: register %s\n", node->full_name);
> -
> -		if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) {
> -			dev_err(&adap->dev, "of_i2c: modalias failure on %s\n",
> -				node->full_name);
> -			continue;
> -		}
> -
> -		addr = of_get_property(node, "reg", &len);
> -		if (!addr || (len < sizeof(int))) {
> -			dev_err(&adap->dev, "of_i2c: invalid reg on %s\n",
> -				node->full_name);
> -			continue;
> -		}
> -
> -		info.addr = be32_to_cpup(addr);
> -		if (info.addr > (1 << 10) - 1) {
> -			dev_err(&adap->dev, "of_i2c: invalid addr=%x on %s\n",
> -				info.addr, node->full_name);
> -			continue;
> -		}
> -
> -		info.irq = irq_of_parse_and_map(node, 0);
> -		info.of_node = of_node_get(node);
> -		info.archdata = &dev_ad;
> -
> -		if (of_get_property(node, "wakeup-source", NULL))
> -			info.flags |= I2C_CLIENT_WAKE;
> -
> -		request_module("%s%s", I2C_MODULE_PREFIX, info.type);
> -
> -		result = i2c_new_device(adap, &info);
> -		if (result == NULL) {
> -			dev_err(&adap->dev, "of_i2c: Failure registering %s\n",
> -			        node->full_name);
> -			of_node_put(node);
> -			irq_dispose_mapping(info.irq);
> -			continue;
> -		}
> -	}
> -}
> -EXPORT_SYMBOL(of_i2c_register_devices);
> -
> -static int of_dev_node_match(struct device *dev, void *data)
> -{
> -        return dev->of_node == data;
> -}
> -
> -/* must call put_device() when done with returned i2c_client device */
> -struct i2c_client *of_find_i2c_device_by_node(struct device_node *node)
> -{
> -	struct device *dev;
> -
> -	dev = bus_find_device(&i2c_bus_type, NULL, node,
> -					 of_dev_node_match);
> -	if (!dev)
> -		return NULL;
> -
> -	return i2c_verify_client(dev);
> -}
> -EXPORT_SYMBOL(of_find_i2c_device_by_node);
> -
> -/* must call put_device() when done with returned i2c_adapter device */
> -struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node)
> -{
> -	struct device *dev;
> -
> -	dev = bus_find_device(&i2c_bus_type, NULL, node,
> -					 of_dev_node_match);
> -	if (!dev)
> -		return NULL;
> -
> -	return i2c_verify_adapter(dev);
> -}
> -EXPORT_SYMBOL(of_find_i2c_adapter_by_node);
> -
> -MODULE_LICENSE("GPL");
> diff --git a/drivers/staging/imx-drm/imx-tve.c b/drivers/staging/imx-drm/imx-tve.c
> index a56797d..2d76fd4 100644
> --- a/drivers/staging/imx-drm/imx-tve.c
> +++ b/drivers/staging/imx-drm/imx-tve.c
> @@ -21,7 +21,7 @@
>  #include <linux/clk.h>
>  #include <linux/clk-provider.h>
>  #include <linux/module.h>
> -#include <linux/of_i2c.h>
> +#include <linux/i2c.h>
>  #include <linux/regmap.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/spinlock.h>
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index e988fa9..2189189 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -542,6 +542,26 @@ static inline int i2c_adapter_id(struct i2c_adapter *adap)
>  
>  #endif /* I2C */
>  
> +#if IS_ENABLED(CONFIG_OF)
> +/* must call put_device() when done with returned i2c_client device */
> +extern struct i2c_client *of_find_i2c_device_by_node(struct device_node *node);
> +
> +/* must call put_device() when done with returned i2c_adapter device */
> +extern struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node);
> +
> +#else
> +
> +static inline struct i2c_client *of_find_i2c_device_by_node(struct device_node *node)
> +{
> +	return NULL;
> +}
> +
> +static inline struct i2c_adapter *of_find_i2c_adapter_by_node(struct device_node *node)
> +{
> +	return NULL;
> +}
> +#endif /* CONFIG_OF */
> +
>  #if IS_ENABLED(CONFIG_ACPI_I2C)
>  extern void acpi_i2c_register_devices(struct i2c_adapter *adap);
>  #else
> diff --git a/include/linux/of_i2c.h b/include/linux/of_i2c.h
> deleted file mode 100644
> index cfb545c..0000000
> --- a/include/linux/of_i2c.h
> +++ /dev/null
> @@ -1,46 +0,0 @@
> -/*
> - * Generic I2C API implementation for PowerPC.
> - *
> - * Copyright (c) 2008 Jochen Friedrich <jochen@scram.de>
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or
> - * (at your option) any later version.
> - */
> -
> -#ifndef __LINUX_OF_I2C_H
> -#define __LINUX_OF_I2C_H
> -
> -#if defined(CONFIG_OF_I2C) || defined(CONFIG_OF_I2C_MODULE)
> -#include <linux/i2c.h>
> -
> -extern void of_i2c_register_devices(struct i2c_adapter *adap);
> -
> -/* must call put_device() when done with returned i2c_client device */
> -extern struct i2c_client *of_find_i2c_device_by_node(struct device_node *node);
> -
> -/* must call put_device() when done with returned i2c_adapter device */
> -extern struct i2c_adapter *of_find_i2c_adapter_by_node(
> -						struct device_node *node);
> -
> -#else
> -static inline void of_i2c_register_devices(struct i2c_adapter *adap)
> -{
> -	return;
> -}
> -
> -static inline struct i2c_client *of_find_i2c_device_by_node(struct device_node *node)
> -{
> -	return NULL;
> -}
> -
> -/* must call put_device() when done with returned i2c_adapter device */
> -static inline struct i2c_adapter *of_find_i2c_adapter_by_node(
> -						struct device_node *node)
> -{
> -	return NULL;
> -}
> -#endif /* CONFIG_OF_I2C */
> -
> -#endif /* __LINUX_OF_I2C_H */
> diff --git a/sound/soc/fsl/imx-sgtl5000.c b/sound/soc/fsl/imx-sgtl5000.c
> index 3f726e4..f2fbde9 100644
> --- a/sound/soc/fsl/imx-sgtl5000.c
> +++ b/sound/soc/fsl/imx-sgtl5000.c
> @@ -13,7 +13,7 @@
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_platform.h>
> -#include <linux/of_i2c.h>
> +#include <linux/i2c.h>
>  #include <linux/clk.h>
>  #include <sound/soc.h>
>  
> diff --git a/sound/soc/fsl/imx-wm8962.c b/sound/soc/fsl/imx-wm8962.c
> index 52a36a9..9fd7a65 100644
> --- a/sound/soc/fsl/imx-wm8962.c
> +++ b/sound/soc/fsl/imx-wm8962.c
> @@ -15,7 +15,7 @@
>  
>  #include <linux/module.h>
>  #include <linux/of_platform.h>
> -#include <linux/of_i2c.h>
> +#include <linux/i2c.h>
>  #include <linux/slab.h>
>  #include <linux/clk.h>
>  #include <sound/soc.h>


-- 

Cheers,
Mauro

^ permalink raw reply

* Re: Detecting LD/ST instruction
From: Sukadev Bhattiprolu @ 2013-08-24  8:47 UTC (permalink / raw)
  To: Michael Neuling; +Cc: eraninan, linuxppc-dev
In-Reply-To: <5184.1377214264@ale.ozlabs.ibm.com>

Michael Neuling [mikey@neuling.org] wrote:
| > I am working on implementing the 'perf mem' command for Power
| > systems. This would for instance, let us know where in the memory
| > hierarchy (L1, L2, Local RAM etc) the data for a load/store
| > instruction was found (hit).
| > 
| > On Power7, if the mcmcra[DCACHE_MISS] is clear _and_ the
| > instruction is a load/store, then it implies a L1-hit.
| > 
| > Unlike on Power8, the Power7 event vector has no indication
| > if the instruction was load/store.
| > 
| > In the context of a PMU interrupt, is there any way to determine
| > if an instruction is a load/store ?
| 
| You could read the instruction from memory and work it out.  
| 
| We do something similar to this in power_pmu_bhrb_to() where we read the
| instruction and work out where the branch is going to.
| 
| If you do this, please use and/or extend the functions in
| arch/powerpc/lib/code-patching.c

Here is a draft of what I could come up with.  With this patch, 
the number of L1 hits on Power7 matches that on Power8 for one
application.

But, wondering if there is a more efficient way to do this - there
are over 50 flavors of load and store!

(btw, I will resend my whole patchset after some time-off).
---

>From db90cd382f4c1c0d84a0cfb07c9ffdb05d529456 Mon Sep 17 00:00:00 2001
From: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Date: Fri, 23 Aug 2013 18:35:02 -0700
Subject: [PATCH 1/1] Try to detect load/store instruction on Power7

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/code-patching.h |    1 +
 arch/powerpc/lib/code-patching.c         |   97 ++++++++++++++++++++++++++++++
 arch/powerpc/perf/power7-pmu.c           |   21 +++++++
 3 files changed, 119 insertions(+)

diff --git a/arch/powerpc/include/asm/code-patching.h b/arch/powerpc/include/asm/code-patching.h
index a6f8c7a..3e47fe0 100644
--- a/arch/powerpc/include/asm/code-patching.h
+++ b/arch/powerpc/include/asm/code-patching.h
@@ -34,6 +34,7 @@ int instr_is_branch_to_addr(const unsigned int *instr, unsigned long addr);
 unsigned long branch_target(const unsigned int *instr);
 unsigned int translate_branch(const unsigned int *dest,
 			      const unsigned int *src);
+int instr_is_load_store(const unsigned int *instr);
 
 static inline unsigned long ppc_function_entry(void *func)
 {
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 17e5b23..10e7839 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -159,6 +159,103 @@ unsigned int translate_branch(const unsigned int *dest, const unsigned int *src)
 	return 0;
 }
 
+/*
+ * TODO: this is same as branch_opcode(). Rename that function
+ * and re-use it ?
+ */
+static unsigned int load_store_opcode(const unsigned int instr)
+{
+	return (instr >> 26) & 0X3F;
+}
+
+static unsigned int load_store_xval(const unsigned int instr)
+{
+	return (instr >> 1) & 0x3FF;	/* bits 21..30 */
+}
+
+/*
+ * Values of bits 21:30 of Fixed-point load and store instructions
+ * Reference: PowerISA_V2.06B_Public.pdf, Sections 3.3.2 through 3.3.6
+ * 4.6.2 through 4.6.4.
+ */
+#define	x_lbzx		87
+#define	x_lbzux		119
+#define	x_lhzx		279
+#define	x_lhzux		311
+#define	x_lhax		343
+#define	x_lhaux		375
+#define	x_lwzx		23
+#define	x_lwzux		55
+#define	x_lwax		341
+#define	x_lwaux		373
+#define	x_ldx		21
+#define	x_ldux		53
+#define	x_stbx		215
+#define	x_stbux		247
+#define	x_sthx		407
+#define	x_sthux		439
+#define	x_stwx		151
+#define	x_stwux		183
+#define	x_stdx		149
+#define	x_stdux		181
+#define	x_lhbrx		790
+#define	x_lwbrx		534
+#define	x_sthbrx	918
+#define	x_stwbrx	662
+#define	x_ldbrx		532
+#define	x_stdbrx	660
+#define	x_lswi		597
+#define	x_lswx		533
+#define	x_stswi		725
+#define	x_stswx		661
+#define	x_lfsx		535
+#define	x_lfsux		567
+#define	x_lfdx		599
+#define	x_lfdux		631
+#define	x_lfiwax	855
+#define	x_lfiwzx	887
+#define	x_stfsx		663
+#define	x_stfsux	695
+#define	x_stfdx		727
+#define	x_stfdux	759
+#define	x_stfiwax	983
+#define	x_lfdpx		791
+#define	x_stfdpx	919
+
+static unsigned int x_form_load_store[] = {
+	x_lbzx,     x_lbzux,    x_lhzx,     x_lhzux,    x_lhax,
+	x_lhaux,    x_lwzx,     x_lwzux,    x_lwax,     x_lwaux,
+	x_ldx,      x_ldux,     x_stbx,     x_stbux,    x_sthx,
+	x_sthux,    x_stwx,     x_stwux,    x_stdx,     x_stdux,
+	x_lhbrx,    x_lwbrx,    x_sthbrx,   x_stwbrx,   x_ldbrx,
+	x_stdbrx,   x_lswi,     x_lswx,     x_stswi,    x_stswx,
+	x_lfsx,     x_lfsux,    x_lfdx,     x_lfdux,    x_lfiwax,
+	x_lfiwzx,   x_stfsx,    x_stfsux,   x_stfdx,    x_stfdux,
+	x_stfiwax,  x_lfdpx,    x_stfdpx
+};
+
+int instr_is_load_store(const unsigned int *instr)
+{
+	unsigned int op;
+	int i, n;
+
+	op = load_store_opcode(*instr);
+
+	if ((op >= 32 && op <= 58) || (op == 61 || op == 62))
+		return 1;
+
+	if (op == 31) {
+		n = sizeof(x_form_load_store) / sizeof(int);
+
+		for (i = 0; i < n; i++) {
+			if (x_form_load_store[i] == load_store_xval(*instr))
+				return 1;
+		}
+	}
+
+	return 0;
+}
+
 
 #ifdef CONFIG_CODE_PATCHING_SELFTEST
 
diff --git a/arch/powerpc/perf/power7-pmu.c b/arch/powerpc/perf/power7-pmu.c
index f8143d6..6e1ca90 100644
--- a/arch/powerpc/perf/power7-pmu.c
+++ b/arch/powerpc/perf/power7-pmu.c
@@ -11,8 +11,10 @@
 #include <linux/kernel.h>
 #include <linux/perf_event.h>
 #include <linux/string.h>
+#include <linux/uaccess.h>
 #include <asm/reg.h>
 #include <asm/cputable.h>
+#include <asm/code-patching.h>
 
 /*
  * Bits in event code for POWER7
@@ -383,13 +385,32 @@ static void power7_get_mem_data_src(union perf_mem_data_src *dsrc,
 {
 	u64 idx;
 	u64 mmcra = regs->dsisr;
+	u64 addr;
+	int ret;
+	unsigned int instr;
 
 	if (mmcra & POWER7_MMCRA_DCACHE_MISS) {
 		idx = mmcra & POWER7_MMCRA_DCACHE_SRC_MASK;
 		idx >>= POWER7_MMCRA_DCACHE_SRC_SHIFT;
 
 		dsrc->val |= dcache_src_map[idx];
+		return;
 	}
+
+	instr = 0;
+	addr = perf_instruction_pointer(regs);
+
+	if (is_kernel_addr(addr))
+		instr = *(unsigned int *)addr;
+	else {
+		pagefault_disable();
+		ret = __get_user_inatomic(instr, (unsigned int __user *)addr);
+		pagefault_enable();
+		if (ret)
+			instr = 0;
+	}
+	if (instr && instr_is_load_store(&instr))
+		dsrc->val |= PLH(LVL, L1);
 }
 
 
-- 
1.7.9.5

^ permalink raw reply related

* Re: ppc/sata-fsl: orphan config value: CONFIG_MPC8315_DS
From: Anthony Foiani @ 2013-08-24  8:03 UTC (permalink / raw)
  To: Scott Wood
  Cc: Li Yang-R58472, Robert P.J.Day, linuxppc-dev@lists.ozlabs.org,
	Shaohui.Xie, Adrian Bunk
In-Reply-To: <1377301643.20722.109.camel@snotra.buserror.net>

Scott Wood <scottwood@freescale.com> writes:

> On Fri, 2013-08-23 at 17:41 -0600, Anthony Foiani wrote:
> > In my original patch [...]  I used "fsl,sata-max-gen".  I thought
> > Jeff disliked it, so I changed it be more generic -- but maybe I
> > misread his complaint.  (And while his opinions are still
> > respected, new maintainers might have different tastes.)
>
> I didn't see anything to that effect from Jeff in that thread -- maybe
> it was elsewhere.

I think I'm referring to this message:

  http://article.gmane.org/gmane.linux.ports.ppc.embedded/58720

As he was referring me to generic methods, I inferred that I should be
providing generic knobs...

> The device tree describes the hardware, not the driver -- and thus
> should be free to use clearer wording. :-)

*nod*

> As for fsl-specific versus generic, generic is fine but then it
> needs to be documented in a generic place.

Agreed.  I actually prefer the "generation" nomenclature, as it has a
more direct/straightforward interpretation.  ("speed=1" vs
"generation=1"; the latter is a much bigger clue, IMHO.)

> Sorry, linux-ide.

Ok, thanks.

I'll wait a few days to see if there are any other comments or
concerns, then I'll spin a final version

As always, thanks for the review and insight!

Best regards,
Anthony Foiani

^ permalink raw reply

* Pull request: scottwood/linux.git next
From: Scott Wood @ 2013-08-24  1:07 UTC (permalink / raw)
  To: benh; +Cc: linuxppc-dev

The following changes since commit afbcdd97bf117bc2d01b865a32f78f662437a4d8:

  powerpc/wsp: Fix early debug build (2013-08-16 10:59:27 +1000)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/scottwood/linux.git next

for you to fetch changes up to 622e03eb3498c32ee29de5c1d6d381f443e58fad:

  powerpc/85xx: Add C293PCIE board support (2013-08-23 19:43:24 -0500)

----------------------------------------------------------------
Chunhe Lan (1):
      powerpc/85xx: Add P1023RDB board support

Haijun.Zhang (1):
      powerpc/85xx: Add support for 85xx cpu type detection

Mingkai Hu (3):
      powerpc/85xx: Add SEC6.0 device tree
      powerpc/85xx: Add silicon device tree for C293
      powerpc/85xx: Add C293PCIE board support

Scott Wood (5):
      powerpc/fsl-booke: Work around erratum A-006958
      powerpc: Convert some mftb/mftbu into mfspr
      powerpc/85xx: Remove -Wa,-me500
      powerpc/booke64: Use appropriate -mcpu
      powerpc/e500: Set -mcpu flag for 32-bit e500

Wang Dongsheng (1):
      powerpc: add Book E support to 64-bit hibernation

 .../devicetree/bindings/crypto/fsl-sec6.txt        | 157 ++++++++++++++
 arch/powerpc/Makefile                              |  18 +-
 arch/powerpc/boot/dts/c293pcie.dts                 | 223 ++++++++++++++++++++
 arch/powerpc/boot/dts/fsl/c293si-post.dtsi         | 193 +++++++++++++++++
 arch/powerpc/boot/dts/fsl/c293si-pre.dtsi          |  63 ++++++
 arch/powerpc/boot/dts/fsl/qoriq-sec6.0-0.dtsi      |  56 +++++
 arch/powerpc/boot/dts/p1023rdb.dts                 | 234 +++++++++++++++++++++
 arch/powerpc/boot/ppc_asm.h                        |   3 +
 arch/powerpc/boot/util.S                           |  10 +-
 .../85xx/{p1023rds_defconfig => p1023_defconfig}   |  24 ++-
 arch/powerpc/configs/mpc85xx_defconfig             |   1 +
 arch/powerpc/configs/mpc85xx_smp_defconfig         |   1 +
 arch/powerpc/include/asm/cputable.h                |   9 +-
 arch/powerpc/include/asm/mpc85xx.h                 |  92 ++++++++
 arch/powerpc/include/asm/ppc_asm.h                 |   6 +-
 arch/powerpc/include/asm/reg.h                     |  17 +-
 arch/powerpc/include/asm/timex.h                   |   4 +-
 arch/powerpc/kernel/swsusp_asm64.S                 |  45 +++-
 arch/powerpc/kernel/vdso32/gettimeofday.S          |   6 +-
 arch/powerpc/platforms/85xx/Kconfig                |  10 +-
 arch/powerpc/platforms/85xx/Makefile               |   1 +
 arch/powerpc/platforms/85xx/c293pcie.c             |  75 +++++++
 arch/powerpc/platforms/85xx/p1023_rds.c            |  24 ++-
 arch/powerpc/platforms/85xx/smp.c                  |  25 +++
 arch/powerpc/platforms/Kconfig.cputype             |  13 ++
 25 files changed, 1280 insertions(+), 30 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/crypto/fsl-sec6.txt
 create mode 100644 arch/powerpc/boot/dts/c293pcie.dts
 create mode 100644 arch/powerpc/boot/dts/fsl/c293si-post.dtsi
 create mode 100644 arch/powerpc/boot/dts/fsl/c293si-pre.dtsi
 create mode 100644 arch/powerpc/boot/dts/fsl/qoriq-sec6.0-0.dtsi
 create mode 100644 arch/powerpc/boot/dts/p1023rdb.dts
 rename arch/powerpc/configs/85xx/{p1023rds_defconfig => p1023_defconfig} (88%)
 create mode 100644 arch/powerpc/include/asm/mpc85xx.h
 create mode 100644 arch/powerpc/platforms/85xx/c293pcie.c

^ permalink raw reply

* Re: [PATCH] powerpc/ppc64: remove __volatile__ in  get_current()
From: Benjamin Herrenschmidt @ 2013-08-24  0:22 UTC (permalink / raw)
  To: Scott Wood; +Cc: James Yang, linuxppc-dev
In-Reply-To: <1377301732.20722.110.camel@snotra.buserror.net>

On Fri, 2013-08-23 at 18:48 -0500, Scott Wood wrote:
> Actually, I changed my mind in the other direction in parallel. :-P
> 
> I think it's probably safe.

Yes, I think it is as well ... but only because "current" is special and
whatever the r13 for the thread is, r13->current will always be the same
value for that thread :-)

Note: That would NOT work if we used a C construct such as
local_paca->current, because in that case, gcc might be stupid enough to
*copy* r13 to another reg, and later on dereference using that other
reg. At that point, the paca pointer itself might become stale when
used.

Cheers,
Ben.

^ permalink raw reply

* Re: [alsa-devel] [PATCH v5 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver
From: Mark Brown @ 2013-08-24  0:20 UTC (permalink / raw)
  To: Mike Turquette
  Cc: Mark Rutland, devicetree@vger.kernel.org,
	alsa-devel@alsa-project.org, lars@metafoo.de,
	ian.campbell@citrix.com, Pawel Moll, swarren@wwwdotorg.org,
	linuxppc-dev@lists.ozlabs.org, Sascha Hauer, Nicolin Chen,
	Tomasz Figa, rob.herring@calxeda.com, timur@tabi.org,
	p.zabel@pengutronix.de, galak@codeaurora.org,
	shawn.guo@linaro.org, festevam@gmail.com
In-Reply-To: <20130823214144.8231.2039@quantum>

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

On Fri, Aug 23, 2013 at 02:41:44PM -0700, Mike Turquette wrote:

> Seems like the regulator framework is solving this with the new
> regulator_get_optional() call. This leaves the
> optional-versus-not-optional logic up to the driver.

That is possibly for a slightly different case but perhaps not...

The problem we've got with the regulator API is that an awful lot of
regulators are always on and people generally don't want to go and hook
everything up, especially when drivers don't yet have regulator support.
This means that we end up with lots of complaints about having to add
regultors and lots of pain adding regulator support to widely used
devices, or drivers that just ignore errors which is not awesome.

We don't want to provide dummies since if the driver genuinely does have
optional regulators they tend to break them so we're adding that call to
allow the core to know if it can just provide a stub and assume the
board data was lazy.

My impression with clocks on most platforms is that there are fewer of
this sort of always on clock, though that said I know there has been a
bit of an issue with some of the IP clocks when IPs are used in SoCs
from less power sensitive environments that don't bother implementing
gating even in hardware.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH] powerpc/ppc64: remove __volatile__ in  get_current()
From: Benjamin Herrenschmidt @ 2013-08-24  0:20 UTC (permalink / raw)
  To: James Yang; +Cc: scottwood, linuxppc-dev
In-Reply-To: <alpine.LRH.2.00.1308231832570.4940@ra8135-ec1.am.freescale.net>

On Fri, 2013-08-23 at 18:40 -0500, James Yang wrote:
> Scott's been able to put enough doubt in me to think that this is not 
> entirely safe, even though the testing and code generation show it to 
> work.  Please reject this patch.
> 
> I think there is still value in getting the unnecessary loads to be 
> removed since it would also allow unnecessary conditional branches to 
> be removed.  I'll think about alternate ways to do this.

Hrm, The problem has to do with PACA accesses moving around accross
preempt boundaries, it's a bit tricky, but in the case of "current"
shouldn't be a problem... while the rest of the PACA might change (CPU#
etc...) current remains stable for the point of view of a given thread.

So I think the patch is fine.

Scott ?

Now, we do need some serious rework of PACA accesses. I'm very *VERY*
nervous with what we have now. A bit of grepping shows dozens of cases
where gcc copies r13 into another register or even saves/restores it, it
scares the shit out of me :-)

My thinking is to make r13 a hidden reg like we do (or used to) on ppc32
with r2 and break down paca access into two forms:

 - Direct access of a single field -> asm loads/stores inline

 - Anything else, uses a get_paca/put_paca construct that includes a
preempt_disable/enable (and maybe along with a __get_paca/__put_paca
pair that doesn't). This basically does a mr of r13 into another
register and basically hides the whole lot from gcc.

The former would be used for single fields, the latter, while adding a
potentially unnecessary mr, will be much safer vs. gcc playing games
with r13.

Any volunteer ? Haven't had time to do it myself so far :-)

Cheers,
Ben.

^ permalink raw reply

* Re: [v2] powerpc/85xx: Add P1023RDB board support
From: Scott Wood @ 2013-08-24  0:12 UTC (permalink / raw)
  To: Chunhe Lan; +Cc: linuxppc-dev
In-Reply-To: <20130824000930.GA29088@home.buserror.net>

On Fri, 2013-08-23 at 19:09 -0500, Scott Wood wrote:
> On Tue, Jul 30, 2013 at 07:40:29PM +0800, Chunhe Lan wrote:
> > P1023RDB Specification:
> > -----------------------
> > Memory subsystem:
> >    512MB DDR3 (Fixed DDR on board)
> >    64MB NOR flash
> >    128MB NAND flash
> > 
> > Ethernet:
> >    eTSEC1: Connected to Atheros AR8035 GETH PHY
> >    eTSEC2: Connected to Atheros AR8035 GETH PHY
> > 
> > PCIe:
> >    Three mini-PCIe slots
> > 
> > USB:
> >    Two USB2.0 Type A ports
> > 
> > I2C:
> >    AT24C08 8K Board EEPROM (8 bit address)
> > 
> > Signed-off-by: Chunhe Lan <Chunhe.Lan@freescale.com>
> > Cc: Scott Wood <scottwood@freescale.com>
> > 
> > ---
> 
> No changelog since v1...


Sigh, and now I see there was a v3 right after this that addressed most
of these issues. :-P

-Scott

^ permalink raw reply

* Re: [v2] powerpc/85xx: Add P1023RDB board support
From: Scott Wood @ 2013-08-24  0:09 UTC (permalink / raw)
  To: Chunhe Lan; +Cc: linuxppc-dev
In-Reply-To: <1375184429-22145-1-git-send-email-Chunhe.Lan@freescale.com>

On Tue, Jul 30, 2013 at 07:40:29PM +0800, Chunhe Lan wrote:
> P1023RDB Specification:
> -----------------------
> Memory subsystem:
>    512MB DDR3 (Fixed DDR on board)
>    64MB NOR flash
>    128MB NAND flash
> 
> Ethernet:
>    eTSEC1: Connected to Atheros AR8035 GETH PHY
>    eTSEC2: Connected to Atheros AR8035 GETH PHY
> 
> PCIe:
>    Three mini-PCIe slots
> 
> USB:
>    Two USB2.0 Type A ports
> 
> I2C:
>    AT24C08 8K Board EEPROM (8 bit address)
> 
> Signed-off-by: Chunhe Lan <Chunhe.Lan@freescale.com>
> Cc: Scott Wood <scottwood@freescale.com>
> 
> ---

No changelog since v1...

> +		nor@0,0 {
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			compatible = "cfi-flash";
> +			reg = <0x0 0x0 0x04000000>;
> +			bank-width = <2>;
> +			device-width = <1>;
> +
> +			partition@0 {
> +				/* 48MB for JFFS2 based Root file System */
> +				reg = <0x00000000 0x03000000>;
> +				label = "NOR JFFS2 Root File System";
> +			};

Don't specify JFFS2.

> +			partition@1000000 {
> +				/* 32MB for Compressed Root file System Image */
> +				reg = <0x01000000 0x02000000>;
> +				label = "NAND Compressed RFS Image";
> +			};
> +
> +			partition@3000000 {
> +				/* 64MB for JFFS2 based Root file System */
> +				reg = <0x03000000 0x04000000>;
> +				label = "NAND JFFS2 Root File System";
> +			};
> +
> +			partition@7000000 {
> +				/* 16MB for User Writable Area */
> +				reg = <0x07000000 0x01000000>;
> +				label = "NAND Writable User area";
> +			};

Don't specify JFFS2.

Why three separate partitions instead of one?  At least the two RFS
partitions should be merged.

> +	board_pci1: pci1: pcie@ff609000 {

You don't need more than one label on a node.

> diff --git a/arch/powerpc/configs/85xx/p1023_defconfig b/arch/powerpc/configs/85xx/p1023_defconfig
> new file mode 100644
> index 0000000..ac29fb8
> --- /dev/null
> +++ b/arch/powerpc/configs/85xx/p1023_defconfig

While I can understand p1023 wanting a separate defconfig once we get
datapath support (it's the only e500v2 chip with datapath, so we probably
don't want to burden mpc85xx_smp_defconfig with it), but I don't see why
we need a separate config right now.

> +# CONFIG_DEBUG_BUGVERBOSE is not set

Please don't disable this.  It's very useful for interpreting bug reports
and has minimal cost.

> diff --git a/arch/powerpc/configs/85xx/p1023rds_defconfig b/arch/powerpc/configs/85xx/p1023rds_defconfig
> deleted file mode 100644
> index b80bcc6..0000000
> --- a/arch/powerpc/configs/85xx/p1023rds_defconfig
> +++ /dev/null
> @@ -1,169 +0,0 @@
> -CONFIG_PPC_85xx=y
> -CONFIG_SMP=y
> -CONFIG_NR_CPUS=2

Oh, you were just moving this.  Please use "-M -C" with git format-patch
so such things are more obvious.

> diff --git a/arch/powerpc/platforms/85xx/Kconfig b/arch/powerpc/platforms/85xx/Kconfig
> index efdd37c..d6424e9 100644
> --- a/arch/powerpc/platforms/85xx/Kconfig
> +++ b/arch/powerpc/platforms/85xx/Kconfig
> @@ -112,10 +112,10 @@ config P1022_RDK
>  	  reference board.
>  
>  config P1023_RDS
> -	bool "Freescale P1023 RDS"
> +	bool "Freescale P1023 RDS (P1023 RDB)"
>  	select DEFAULT_UIMAGE
>  	help
> -	  This option enables support for the P1023 RDS board
> +	  This option enables support for the P1023 RDS (P1023 RDB) board
>  
>  config SOCRATES
>  	bool "Socrates"

I'd just say "P1023 RDS/RDB".

> +static int __init p1023_rdb_probe(void)
> +{
> +	unsigned long root = of_get_flat_dt_root();
> +
> +	return of_flat_dt_is_compatible(root, "fsl,P1023RDB");
> +
> +}

Remove the blank line at the end of the function.

-Scott

^ 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