Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* NFS client broken in Linus' tip
From: Trond Myklebust @ 2014-02-02 22:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140202122757.GC26684@n2100.arm.linux.org.uk>

On Sun, 2014-02-02 at 12:27 +0000, Russell King - ARM Linux wrote:
> On Sat, Feb 01, 2014 at 01:03:28AM +0000, Russell King - ARM Linux wrote:
> > On Fri, Jan 31, 2014 at 03:59:30PM -0500, Trond Myklebust wrote:
> > > On Thu, 2014-01-30 at 15:38 +0000, Russell King - ARM Linux wrote:
> > > > On Thu, Jan 30, 2014 at 06:32:08AM -0800, Christoph Hellwig wrote:
> > > > > On Thu, Jan 30, 2014 at 02:27:52PM +0000, Russell King - ARM Linux wrote:
> > > > > > Yes and no.  I still end up with an empty /etc/mtab, but the file now
> > > > > > exists.  However, I can create and echo data into /etc/mtab, but it seems
> > > > > > that can't happen at boot time.
> > > > > 
> > > > > Odd.  Can you disable CONFIG_NFSD_V3_ACL for now to isolate the issue?
> > > > 
> > > > Unfortunately, that results in some problem at boot time, which
> > > > ultimately ends up with the other three CPUs being stopped, and
> > > > hence the original reason scrolls off the screen before it can be
> > > > read... even at 1920p.
> > > > 
> > > Hi Russell,
> > > 
> > > The following patch fixes the issue for me.
> > 
> > It doesn't entirely fix the issue for me, instead we've got even weirder
> > behaviour:
> > 
> > root at cubox-i4:~# ls -al test
> > ls: cannot access test: No such file or directory
> > root at cubox-i4:~# touch test
> > root at cubox-i4:~# ls -al test
> > -rw-r--r-- 1 root root 0 Feb  1 01:01 test
> > root at cubox-i4:~# echo foo > test
> > root at cubox-i4:~# ls -al test
> > -rw-r--r-- 1 root root 4 Feb  1 01:01 test
> > root at cubox-i4:~# cat test
> > foo
> > root at cubox-i4:~# rm test
> > root at cubox-i4:~# echo foo > test
> > -bash: test: Operation not supported
> > root at cubox-i4:~# ls -al test
> > -rw-r--r-- 1 root root 0 Feb  1 01:01 test
> 
> FYI, I just tested Linus' tip, and NFS is still broken.
> 
Hi Russell,

The following patch should fix the above problem. It needs to be applied
on top of the one I sent you previously. In addition, you will want to
apply Noah Massey's patch from
http://lkml.kernel.org/r/1391135472-9639-1-git-send-email-Noah.Massey at gmail.com 

Cheers,
  Trond

8<------------------------------------------------------------------------------
>From f9ad7a7b43554bc36eccf03d33617c05b8a2c77d Mon Sep 17 00:00:00 2001
From: Trond Myklebust <trond.myklebust@primarydata.com>
Date: Sun, 2 Feb 2014 14:36:42 -0500
Subject: [PATCH] NFSv3: Fix return value of nfs3_proc_setacls

nfs3_proc_setacls is used internally by the NFSv3 create operations
to set the acl after the file has been created. If the operation
fails because the server doesn't support acls, then it must return '0',
not -EOPNOTSUPP.

Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/nfs/nfs3acl.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/nfs3acl.c b/fs/nfs/nfs3acl.c
index 9271a6bb9a41..871d6eda8dba 100644
--- a/fs/nfs/nfs3acl.c
+++ b/fs/nfs/nfs3acl.c
@@ -113,7 +113,7 @@ getout:
 	return ERR_PTR(status);
 }
 
-int nfs3_proc_setacls(struct inode *inode, struct posix_acl *acl,
+static int __nfs3_proc_setacls(struct inode *inode, struct posix_acl *acl,
 		struct posix_acl *dfacl)
 {
 	struct nfs_server *server = NFS_SERVER(inode);
@@ -198,6 +198,15 @@ out:
 	return status;
 }
 
+int nfs3_proc_setacls(struct inode *inode, struct posix_acl *acl,
+		struct posix_acl *dfacl)
+{
+	int ret;
+	ret = __nfs3_proc_setacls(inode, acl, dfacl);
+	return (ret == -EOPNOTSUPP) ? 0 : ret;
+
+}
+
 int nfs3_set_acl(struct inode *inode, struct posix_acl *acl, int type)
 {
 	struct posix_acl *alloc = NULL, *dfacl = NULL;
@@ -225,7 +234,7 @@ int nfs3_set_acl(struct inode *inode, struct posix_acl *acl, int type)
 		if (IS_ERR(alloc))
 			goto fail;
 	}
-	status = nfs3_proc_setacls(inode, acl, dfacl);
+	status = __nfs3_proc_setacls(inode, acl, dfacl);
 	posix_acl_release(alloc);
 	return status;
 
-- 
1.8.5.3


-- 
Trond Myklebust
Linux NFS client maintainer

^ permalink raw reply related

* [PATCH v2 RESEND 2/3] ARM: dts: clps711x: Add bindings documentation for CLPS711X irqchip driver
From: Rob Herring @ 2014-02-02 21:48 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1391328520-30923-1-git-send-email-shc_work@mail.ru>

On Sun, Feb 2, 2014 at 2:08 AM, Alexander Shiyan <shc_work@mail.ru> wrote:
> Add OF document for Cirrus Logic CLPS711X irqchip driver.
>
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> ---
>  .../interrupt-controller/cirrus,clps711x-intc.txt  | 41 ++++++++++++++++++++++

Acked-by: Rob Herring <robh@kernel.org>


>  1 file changed, 41 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/cirrus,clps711x-intc.txt
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/cirrus,clps711x-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/cirrus,clps711x-intc.txt
> new file mode 100644
> index 0000000..759339c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/cirrus,clps711x-intc.txt
> @@ -0,0 +1,41 @@
> +Cirrus Logic CLPS711X Interrupt Controller
> +
> +Required properties:
> +
> +- compatible: Should be "cirrus,clps711x-intc".
> +- reg: Specifies base physical address of the registers set.
> +- interrupt-controller: Identifies the node as an interrupt controller.
> +- #interrupt-cells: Specifies the number of cells needed to encode an
> +  interrupt source. The value shall be 1.
> +
> +The interrupt sources are as follows:
> +ID     Name    Description
> +---------------------------
> +1:     BLINT   Battery low (FIQ)
> +3:     MCINT   Media changed (FIQ)
> +4:     CSINT   CODEC sound
> +5:     EINT1   External 1
> +6:     EINT2   External 2
> +7:     EINT3   External 3
> +8:     TC1OI   TC1 under flow
> +9:     TC2OI   TC2 under flow
> +10:    RTCMI   RTC compare match
> +11:    TINT    64Hz tick
> +12:    UTXINT1 UART1 transmit FIFO half empty
> +13:    URXINT1 UART1 receive FIFO half full
> +14:    UMSINT  UART1 modem status changed
> +15:    SSEOTI  SSI1 end of transfer
> +16:    KBDINT  Keyboard
> +17:    SS2RX   SSI2 receive FIFO half or greater full
> +18:    SS2TX   SSI2 transmit FIFO less than half empty
> +28:    UTXINT2 UART2 transmit FIFO half empty
> +29:    URXINT2 UART2 receive FIFO half full
> +32:    DAIINT  DAI interface (FIQ)
> +
> +Example:
> +       intc: interrupt-controller {
> +               compatible = "cirrus,clps711x-intc";
> +               reg = <0x80000000 0x4000>;
> +               interrupt-controller;
> +               #interrupt-cells = <1>;
> +       };
> --
> 1.8.3.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH] arm64: KVM: Add VGIC device control for arm64
From: Christoffer Dall @ 2014-02-02 21:41 UTC (permalink / raw)
  To: linux-arm-kernel

This fixes the build breakage introduced by
c07a0191ef2de1f9510f12d1f88e3b0b5cd8d66f and adds support for the device
control API and save/restore of the VGIC state for ARMv8.

The defines were simply missing from the arm64 header files and
uaccess.h must be implicitly imported from somewhere else on arm.

Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
---
 arch/arm64/include/uapi/asm/kvm.h | 9 +++++++++
 virt/kvm/arm/vgic.c               | 1 +
 2 files changed, 10 insertions(+)

diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index 495ab6f..eaf54a3 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -148,6 +148,15 @@ struct kvm_arch_memory_slot {
 #define KVM_REG_ARM_TIMER_CNT		ARM64_SYS_REG(3, 3, 14, 3, 2)
 #define KVM_REG_ARM_TIMER_CVAL		ARM64_SYS_REG(3, 3, 14, 0, 2)
 
+/* Device Control API: ARM VGIC */
+#define KVM_DEV_ARM_VGIC_GRP_ADDR	0
+#define KVM_DEV_ARM_VGIC_GRP_DIST_REGS	1
+#define KVM_DEV_ARM_VGIC_GRP_CPU_REGS	2
+#define   KVM_DEV_ARM_VGIC_CPUID_SHIFT	32
+#define   KVM_DEV_ARM_VGIC_CPUID_MASK	(0xffULL << KVM_DEV_ARM_VGIC_CPUID_SHIFT)
+#define   KVM_DEV_ARM_VGIC_OFFSET_SHIFT	0
+#define   KVM_DEV_ARM_VGIC_OFFSET_MASK	(0xffffffffULL << KVM_DEV_ARM_VGIC_OFFSET_SHIFT)
+
 /* KVM_IRQ_LINE irq field index values */
 #define KVM_ARM_IRQ_TYPE_SHIFT		24
 #define KVM_ARM_IRQ_TYPE_MASK		0xff
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index be456ce..8ca405c 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -24,6 +24,7 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
+#include <linux/uaccess.h>
 
 #include <linux/irqchip/arm-gic.h>
 
-- 
1.8.5.2

^ permalink raw reply related

* [PATCH v5 00/23]
From: Jean-Francois Moine @ 2014-02-02 20:07 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140202191505.GK26684@n2100.arm.linux.org.uk>

On Sun, 2 Feb 2014 19:15:05 +0000
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:

> In which case, it may be better to reorder the remaining patches such
> that the DT changes are at the very end - which means we can still
> benefit from the rest of the patches if the DT solution remains an
> open question.
> 
> We do have another option now that my generic component support is in
> mainline (merged during this window), which should result in a much
> cleaner solution.  If we convert TDA998x to a component, armada DRM
> to a component master (and same for other tda998x users) then we don't
> need the drm_encoder_slave stuff - all that goes away since it's no
> longer necessary.
> 
> We also solve this problem as well - because we're then not messing
> around with working out if there's a DT node present: the TDA998x
> device must pre-exist.  For non-DT setups, this can be done when
> the I2C bus is created - devices on it would be created using the
> standard mechanisms already present via the i2c_board_data array.
> For DT setups, the devices are created by parsing the I2C bus node
> in DT.
> 
> Both cases result in a component being registered upon invocation of
> tda998x_probe(), and removal of the component when tda998x_remove()
> is called.  The tda998x driver becomes a standard I2C driver.
> 
> This is something I've been intending to look at now that the component
> stuff is in place - as I said previously when the questions around DT
> and Armada DRM were first posed, we need to solve these issues in a
> generic way first, rather than hacking around them.

Agree. I was looking in the same direction...

-- 
Ken ar c'henta?	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

^ permalink raw reply

* [PATCH v5 00/23]
From: Russell King - ARM Linux @ 2014-02-02 19:15 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140202195400.073f4eb4@armhf>

On Sun, Feb 02, 2014 at 07:54:00PM +0100, Jean-Francois Moine wrote:
> I explained how to use the tda998x in a DT context in a message to Jyri
> Sarha:
> 
> http://lists.freedesktop.org/archives/dri-devel/2014-January/052936.html

Okay, so there's a bunch of changes required to the DRM slave support
which aren't in place yet.

In which case, it may be better to reorder the remaining patches such
that the DT changes are at the very end - which means we can still
benefit from the rest of the patches if the DT solution remains an
open question.

We do have another option now that my generic component support is in
mainline (merged during this window), which should result in a much
cleaner solution.  If we convert TDA998x to a component, armada DRM
to a component master (and same for other tda998x users) then we don't
need the drm_encoder_slave stuff - all that goes away since it's no
longer necessary.

We also solve this problem as well - because we're then not messing
around with working out if there's a DT node present: the TDA998x
device must pre-exist.  For non-DT setups, this can be done when
the I2C bus is created - devices on it would be created using the
standard mechanisms already present via the i2c_board_data array.
For DT setups, the devices are created by parsing the I2C bus node
in DT.

Both cases result in a component being registered upon invocation of
tda998x_probe(), and removal of the component when tda998x_remove()
is called.  The tda998x driver becomes a standard I2C driver.

This is something I've been intending to look at now that the component
stuff is in place - as I said previously when the questions around DT
and Armada DRM were first posed, we need to solve these issues in a
generic way first, rather than hacking around them.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

^ permalink raw reply

* [PATCH v5 00/23]
From: Jean-Francois Moine @ 2014-02-02 19:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140202180433.GI26684@n2100.arm.linux.org.uk>

On Sun, 2 Feb 2014 18:04:34 +0000
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:

> So, in summary, I'm pretty happy with this again - and it's all been
> tested here with no apparant detrimental effects.  All committed and
> queued up here:
> 
> http://ftp.arm.linux.org.uk/cgit/linux-cubox.git/log/?h=tda998x-devel
> 
> If you want to pull it back to check:
> 
>   git://ftp.arm.linux.org.uk/~rmk/linux-cubox.git tda998x-devel
> 
> I'm proposing to send everything up to the tda998x-fixes tag next week
> for 3.13-rc kernels.

Everything is OK. Thank you.

-- 
Ken ar c'henta?	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

^ permalink raw reply

* [PATCH v5 00/23]
From: Jean-Francois Moine @ 2014-02-02 18:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140202182349.GJ26684@n2100.arm.linux.org.uk>

On Sun, 2 Feb 2014 18:23:49 +0000
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:

> On Sun, Feb 02, 2014 at 07:06:06PM +0100, Jean-Francois Moine wrote:
> > On Sun, 2 Feb 2014 12:43:58 +0000
> > Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> > 
> > > On Wed, Jan 29, 2014 at 10:01:22AM +0100, Jean-Francois Moine wrote:
> > > > This patch set contains various extensions to the tda998x driver:
> > > > 
> > > > - simplify the i2c read/write
> > > > - code cleanup and fix some small errors
> > > > - use global constants
> > > > - don't read write-only registers
> > > > - add DT support
> > > > - use IRQ for connection status and EDID read
> > > 
> > > I discussed these patches with Rob Clark recently, and the conclusion
> > > we came to is that I'll merge them into a git tree, test them, and
> > > once I'm happy I'll send a pull request as appropriate.
> > > 
> > > I'll go through them later today.  Those patches which have been re-
> > > posted without any change for the last few times (the first few) I'll
> > > take into my git tree today so you don't have to keep re-posting them
> > > (more importantly, I won't have to keep on looking at them either.)
> > 
> > Thanks.
> > 
> > BTW, I found some problems with the patch 12 'add DT support' you
> > already acked:
> > 
> > - the .of_match_table is not needed because the i2c client is created by
> >   the i2c subsystem from the 'reg' in the DT,
> 
> Okay - may it be a good idea to have someone knowledgable of I2C give it
> a review?

Sure! There is still a lot of magic in the DT.

I used the tda998x in the DT for a long time without .of_match_table
and it loaded correctly. I added it in the patch just because my other
modules had it.

A few days ago, when I moved the tda998x audio codec description in the
DT as a subnode of the tda998x i2c, the codec module was not loaded. An
of_platform_populate() of the subnodes was needed in the tda998x i2c
driver. I could not find why...

> > - on encoder_destroy(), the function drm_i2c_encoder_destroy()
> >   unregisters the i2c client, so, with a DT, a second encoder_init()
> >   would crash.
> 
> I think this is one of the down-sides of trying to bolt DT into this:
> the drm encoder slave support is not designed to cope with an i2c client
> device pre-created.
> 
> In fact, I can't see how this stuff comes anywhere close to working in
> a DT setup: in such a scenario, you declare that there's a tda998x
> device in DT.  I2C parses this, and creates an i2c_client itself for
> the tda998x.
> 
> When the TDA998x driver initialises, it finds this i2c client and
> binds to it, calling tda998x_probe(), which does nothing.
> 
> However, the only way to attach a slave encoder to a DRM device is via
> a call to drm_i2c_encoder_init(), which unconditionally calls
> i2c_new_device().  This creates a _new_ i2c_client structure, again
> unconditionally, for the tda998x.  This must be bound by the I2C
> subsystem to a driver - hopefully the tda998x driver, which then
> calls it's encoder_init function.
> 
> None of this will happen if DT has already created an i2c_client at
> the appropriate address, because DRMs i2c_new_device() will fail.
> 
> My hypothesis is that you have other patches to I2C and/or DRM to
> sort this out which you haven't been posting with this series.  So,
> could you please provide some hints as to how this works?

I explained how to use the tda998x in a DT context in a message to Jyri
Sarha:

http://lists.freedesktop.org/archives/dri-devel/2014-January/052936.html

-- 
Ken ar c'henta?	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

^ permalink raw reply

* [PATCH v5 00/23]
From: Sebastian Hesselbarth @ 2014-02-02 18:41 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140202182349.GJ26684@n2100.arm.linux.org.uk>

On 02/02/2014 07:23 PM, Russell King - ARM Linux wrote:
> On Sun, Feb 02, 2014 at 07:06:06PM +0100, Jean-Francois Moine wrote:
>> - on encoder_destroy(), the function drm_i2c_encoder_destroy()
>>    unregisters the i2c client, so, with a DT, a second encoder_init()
>>    would crash.
>
> I think this is one of the down-sides of trying to bolt DT into this:
> the drm encoder slave support is not designed to cope with an i2c client
> device pre-created.
>
> In fact, I can't see how this stuff comes anywhere close to working in
> a DT setup: in such a scenario, you declare that there's a tda998x
> device in DT.  I2C parses this, and creates an i2c_client itself for
> the tda998x.
>
> When the TDA998x driver initialises, it finds this i2c client and
> binds to it, calling tda998x_probe(), which does nothing.
>
> However, the only way to attach a slave encoder to a DRM device is via
> a call to drm_i2c_encoder_init(), which unconditionally calls
> i2c_new_device().  This creates a _new_ i2c_client structure, again
> unconditionally, for the tda998x.  This must be bound by the I2C
> subsystem to a driver - hopefully the tda998x driver, which then
> calls it's encoder_init function.
>
> None of this will happen if DT has already created an i2c_client at
> the appropriate address, because DRMs i2c_new_device() will fail.

drm_i2c_encoder_init() could look at .of_node of the i2c_board_info.
If it is there, do not try to i2c_new_device as it has already been
registered by DT i2c auto-probing.

Sebastian

^ permalink raw reply

* [PATCH v5 00/23]
From: Russell King - ARM Linux @ 2014-02-02 18:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140202190606.6fa193ce@armhf>

On Sun, Feb 02, 2014 at 07:06:06PM +0100, Jean-Francois Moine wrote:
> On Sun, 2 Feb 2014 12:43:58 +0000
> Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> 
> > On Wed, Jan 29, 2014 at 10:01:22AM +0100, Jean-Francois Moine wrote:
> > > This patch set contains various extensions to the tda998x driver:
> > > 
> > > - simplify the i2c read/write
> > > - code cleanup and fix some small errors
> > > - use global constants
> > > - don't read write-only registers
> > > - add DT support
> > > - use IRQ for connection status and EDID read
> > 
> > I discussed these patches with Rob Clark recently, and the conclusion
> > we came to is that I'll merge them into a git tree, test them, and
> > once I'm happy I'll send a pull request as appropriate.
> > 
> > I'll go through them later today.  Those patches which have been re-
> > posted without any change for the last few times (the first few) I'll
> > take into my git tree today so you don't have to keep re-posting them
> > (more importantly, I won't have to keep on looking at them either.)
> 
> Thanks.
> 
> BTW, I found some problems with the patch 12 'add DT support' you
> already acked:
> 
> - the .of_match_table is not needed because the i2c client is created by
>   the i2c subsystem from the 'reg' in the DT,

Okay - may it be a good idea to have someone knowledgable of I2C give it
a review?

> - on encoder_destroy(), the function drm_i2c_encoder_destroy()
>   unregisters the i2c client, so, with a DT, a second encoder_init()
>   would crash.

I think this is one of the down-sides of trying to bolt DT into this:
the drm encoder slave support is not designed to cope with an i2c client
device pre-created.

In fact, I can't see how this stuff comes anywhere close to working in
a DT setup: in such a scenario, you declare that there's a tda998x
device in DT.  I2C parses this, and creates an i2c_client itself for
the tda998x.

When the TDA998x driver initialises, it finds this i2c client and
binds to it, calling tda998x_probe(), which does nothing.

However, the only way to attach a slave encoder to a DRM device is via
a call to drm_i2c_encoder_init(), which unconditionally calls
i2c_new_device().  This creates a _new_ i2c_client structure, again
unconditionally, for the tda998x.  This must be bound by the I2C
subsystem to a driver - hopefully the tda998x driver, which then
calls it's encoder_init function.

None of this will happen if DT has already created an i2c_client at
the appropriate address, because DRMs i2c_new_device() will fail.

My hypothesis is that you have other patches to I2C and/or DRM to
sort this out which you haven't been posting with this series.  So,
could you please provide some hints as to how this works?

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

^ permalink raw reply

* [PATCH v5 00/23]
From: Jean-Francois Moine @ 2014-02-02 18:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140202124358.GD26684@n2100.arm.linux.org.uk>

On Sun, 2 Feb 2014 12:43:58 +0000
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:

> On Wed, Jan 29, 2014 at 10:01:22AM +0100, Jean-Francois Moine wrote:
> > This patch set contains various extensions to the tda998x driver:
> > 
> > - simplify the i2c read/write
> > - code cleanup and fix some small errors
> > - use global constants
> > - don't read write-only registers
> > - add DT support
> > - use IRQ for connection status and EDID read
> 
> I discussed these patches with Rob Clark recently, and the conclusion
> we came to is that I'll merge them into a git tree, test them, and
> once I'm happy I'll send a pull request as appropriate.
> 
> I'll go through them later today.  Those patches which have been re-
> posted without any change for the last few times (the first few) I'll
> take into my git tree today so you don't have to keep re-posting them
> (more importantly, I won't have to keep on looking at them either.)

Thanks.

BTW, I found some problems with the patch 12 'add DT support' you
already acked:

- the .of_match_table is not needed because the i2c client is created by
  the i2c subsystem from the 'reg' in the DT,

- on encoder_destroy(), the function drm_i2c_encoder_destroy()
  unregisters the i2c client, so, with a DT, a second encoder_init()
  would crash.

Do you better like a new patch or a fix?

-- 
Ken ar c'henta?	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

^ permalink raw reply

* [PATCH v5 00/23]
From: Russell King - ARM Linux @ 2014-02-02 18:04 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140202124358.GD26684@n2100.arm.linux.org.uk>

On Sun, Feb 02, 2014 at 12:43:58PM +0000, Russell King - ARM Linux wrote:
> On Wed, Jan 29, 2014 at 10:01:22AM +0100, Jean-Francois Moine wrote:
> > This patch set contains various extensions to the tda998x driver:
> > 
> > - simplify the i2c read/write
> > - code cleanup and fix some small errors
> > - use global constants
> > - don't read write-only registers
> > - add DT support
> > - use IRQ for connection status and EDID read
> 
> I discussed these patches with Rob Clark recently, and the conclusion
> we came to is that I'll merge them into a git tree, test them, and
> once I'm happy I'll send a pull request as appropriate.
> 
> I'll go through them later today.  Those patches which have been re-
> posted without any change for the last few times (the first few) I'll
> take into my git tree today so you don't have to keep re-posting them
> (more importantly, I won't have to keep on looking at them either.)

Okay, out of your patches, I would like to queue up the following
for submission into -rc kernels:

drm/i2c: tda998x: fix bad value in the AIF
drm/i2c: tda998x: check the CEC device creation
drm/i2c: tda998x: free the CEC device on encoder_destroy
drm/i2c: tda998x: force the page register at startup time
drm/i2c: tda998x: set the PLL division factor in range 0..3
drm/i2c: tda998x: fix the ENABLE_SPACE register

since these fix real bugs.  Moving them to the front of the queue isn't
that big a deal (I've done it here in my git tree - yes, there were a
few conflicts, but nothing serious.)

I'll also consider these for -rc too:

drm/i2c: tda998x: use HDMI constants
drm/i2c: tda998x: add the active aspect in HDMI AVI frame
drm/i2c: tda998x: change the frequence in the audio channel

if we have reports of display devices affected by the info frame errors.

As far as the last summary line goes, I'll change it to:

	"Use ALSA IEC958 definitions and update audio frequency"

Note that in general, bug fixes should always be towards the beginning
of a patch series, so that they can be applied independently of the
remainder of the development, which also makes them easier to backport
to stable kernels if that's deemed necessary.

As for the remainder, I think moving the DT documentation patch
immediately after the addition of DT code (or even just before it) is
good form.

So, in summary, I'm pretty happy with this again - and it's all been
tested here with no apparant detrimental effects.  All committed and
queued up here:

http://ftp.arm.linux.org.uk/cgit/linux-cubox.git/log/?h=tda998x-devel

If you want to pull it back to check:

  git://ftp.arm.linux.org.uk/~rmk/linux-cubox.git tda998x-devel

I'm proposing to send everything up to the tda998x-fixes tag next week
for 3.13-rc kernels.

Rob, do you want to provide any acks for this or are you happy?

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

^ permalink raw reply

* [PATCH v5 09/23] drm/i2c: tda998x: don't read write-only registers
From: Russell King - ARM Linux @ 2014-02-02 17:57 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140202184512.7fa2e3cf@armhf>

On Sun, Feb 02, 2014 at 06:45:12PM +0100, Jean-Francois Moine wrote:
> On Sun, 2 Feb 2014 16:23:09 +0000
> Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> 
> > On Sat, Jan 25, 2014 at 06:14:42PM +0100, Jean-Francois Moine wrote:  
> > > This patch takes care of the write-only registers of the tda998x.
> > > 
> > > The registers SOFTRESET, TBG_CNTRL_0 and TBG_CNTRL_1 have all bits
> > > cleared after reset, so, they may be fully re-written.
> > > 
> > > The register MAT_CONTRL is set to
> > > 	MAT_CONTRL_MAT_BP | MAT_CONTRL_MAT_SC(1)
> > > after reset, so, it may be fully set again to this value.  
> > 
> > I said in v3 of this patch, which seems to remain unaddressed:
> >   
> > >       /* must be last register set: */
> > > -     reg_clear(priv, REG_TBG_CNTRL_0, TBG_CNTRL_0_SYNC_ONCE);
> > > +     reg_write(priv, REG_TBG_CNTRL_0, 0);  
> > 
> > Register changes which have a potential effect shouldn't be part of a
> > patch which is really only trying to avoid reading from write only
> > registers.
> > 
> > This could be a potential functional change - and it's probably one
> > which Rob Clark should at least be made aware of.  As I commented last
> > time, when you're changing register values in an otherwise innocuous
> > patch, you should comment about them in the patch description.  
> 
> According to the tda9983b documentation, the register TBG_CNTRL_0 is
> set to 0 at reset time. I think that it is the same for all the tda998x
> family. In the other hand, this register is supposed to be write only,
> so reading it may return any value, and the reg_clear() function may
> set any other bits. Then, clearing one bit is less secure than clearing
> the full register.

Okay, in that case I'm happy with this patch.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

^ permalink raw reply

* [PATCH v5 02/23] drm/i2c: tda998x: check more I/O errors
From: Russell King - ARM Linux @ 2014-02-02 17:56 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140202183000.1cf9ca9c@armhf>

On Sun, Feb 02, 2014 at 06:30:00PM +0100, Jean-Francois Moine wrote:
> On Sun, 2 Feb 2014 16:20:58 +0000
> Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> 
> > On Sat, Jan 25, 2014 at 06:14:45PM +0100, Jean-Francois Moine wrote:
> > > This patch adds more error checking inn I2C I/O functions.
> > > In case of I/O error, this permits to avoid writing in bad controller
> > > pages, a bad chipset detection or looping when getting the EDID.
> > 
> > I've just looked at this again, and spotted something:
> > 
> > > -static uint8_t
> > > +static int
> > >  reg_read(struct tda998x_priv *priv, uint16_t reg)
> > >  {
> > >  	uint8_t val = 0;
> > > -	reg_read_range(priv, reg, &val, sizeof(val));
> > > +	int ret;
> > > +
> > > +	ret = reg_read_range(priv, reg, &val, sizeof(val));
> > > +	if (ret < 0)
> > > +		return ret;
> > 
> > So yes, this can return negative numbers.
> > 
> > > @@ -1158,8 +1184,11 @@ tda998x_encoder_init(struct i2c_client *client,
> > >  	tda998x_reset(priv);
> > >  
> > >  	/* read version: */
> > > -	priv->rev = reg_read(priv, REG_VERSION_LSB) |
> > > -			reg_read(priv, REG_VERSION_MSB) << 8;
> > > +	ret = reg_read(priv, REG_VERSION_LSB) |
> > > +		(reg_read(priv, REG_VERSION_MSB) << 8);
> > > +	if (ret < 0)
> > > +		goto fail;
> > > +	priv->rev = ret;
> > 
> > Two issues here:
> > 
> > 1. The additional parens are /really/ not required.
> > 2. What if reg_read(priv, REG_VERSION_MSB) returns a negative number?
> > 
> > If we're going to the extent of attempting to make the read/write
> > functions return errors, we should at least handle errors generated
> > by them properly, otherwise it's pointless making them return errors.
> > 
> > 	ret = reg_read(priv, REG_VERSION_LSB);
> > 	if (ret < 0)
> > 		goto fail;
> > 
> > 	priv->rev = ret;
> > 
> > 	ret = reg_read(priv, REG_VERSION_MSB);
> > 	if (ret < 0)
> > 		goto fail;
> > 
> > 	priv->rev |= ret << 8;
> > 
> > If you want it to look slightly nicer:
> > 
> > 	int rev_lo, rev_hi;
> > 
> > 	rev_lo = reg_read(priv, REG_VERSION_LSB);
> > 	rev_hi = reg_read(priv, REG_VERSION_MSB);
> > 	if (rev_lo < 0 || rev_hi < 0) {
> > 		ret = rev_lo < 0 ? rev_lo : rev_hi;
> > 		goto fail;
> > 	}
> > 
> > 	priv->rev = rev_lo | rev_hi << 8;
> > 
> > I'm happy to commit such a change after this patch to clean it up, or if
> > you want to regenerate your patch 2 and post /just/ that incorporating
> > this change.
> 
> I think that my code works correctly: when there is an error, the
> result of reg_read() is minus the error code, and this error code is
> always lower than 8388607 (0x7fffff). Then, reg_read() << 8 will always
> be negative.

The issue I'm pointing out is not whether it will be interpreted as an
error or not, it's whether the value is a correct error code.  If we
don't care about the reason why an error occurs, we might as well just
return -1.

I've added my own patch which adjusts it as per above to my tree anyway,
so I'm not that worried about this.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

^ permalink raw reply

* [PATCH v5 09/23] drm/i2c: tda998x: don't read write-only registers
From: Jean-Francois Moine @ 2014-02-02 17:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140202162309.GF26684@n2100.arm.linux.org.uk>

On Sun, 2 Feb 2014 16:23:09 +0000
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:

> On Sat, Jan 25, 2014 at 06:14:42PM +0100, Jean-Francois Moine wrote:  
> > This patch takes care of the write-only registers of the tda998x.
> > 
> > The registers SOFTRESET, TBG_CNTRL_0 and TBG_CNTRL_1 have all bits
> > cleared after reset, so, they may be fully re-written.
> > 
> > The register MAT_CONTRL is set to
> > 	MAT_CONTRL_MAT_BP | MAT_CONTRL_MAT_SC(1)
> > after reset, so, it may be fully set again to this value.  
> 
> I said in v3 of this patch, which seems to remain unaddressed:
>   
> >       /* must be last register set: */
> > -     reg_clear(priv, REG_TBG_CNTRL_0, TBG_CNTRL_0_SYNC_ONCE);
> > +     reg_write(priv, REG_TBG_CNTRL_0, 0);  
> 
> Register changes which have a potential effect shouldn't be part of a
> patch which is really only trying to avoid reading from write only
> registers.
> 
> This could be a potential functional change - and it's probably one
> which Rob Clark should at least be made aware of.  As I commented last
> time, when you're changing register values in an otherwise innocuous
> patch, you should comment about them in the patch description.  

According to the tda9983b documentation, the register TBG_CNTRL_0 is
set to 0 at reset time. I think that it is the same for all the tda998x
family. In the other hand, this register is supposed to be write only,
so reading it may return any value, and the reg_clear() function may
set any other bits. Then, clearing one bit is less secure than clearing
the full register.

-- 
Ken ar c'henta?	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

^ permalink raw reply

* [PATCH v5 02/23] drm/i2c: tda998x: check more I/O errors
From: Jean-Francois Moine @ 2014-02-02 17:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140202162057.GE26684@n2100.arm.linux.org.uk>

On Sun, 2 Feb 2014 16:20:58 +0000
Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:

> On Sat, Jan 25, 2014 at 06:14:45PM +0100, Jean-Francois Moine wrote:
> > This patch adds more error checking inn I2C I/O functions.
> > In case of I/O error, this permits to avoid writing in bad controller
> > pages, a bad chipset detection or looping when getting the EDID.
> 
> I've just looked at this again, and spotted something:
> 
> > -static uint8_t
> > +static int
> >  reg_read(struct tda998x_priv *priv, uint16_t reg)
> >  {
> >  	uint8_t val = 0;
> > -	reg_read_range(priv, reg, &val, sizeof(val));
> > +	int ret;
> > +
> > +	ret = reg_read_range(priv, reg, &val, sizeof(val));
> > +	if (ret < 0)
> > +		return ret;
> 
> So yes, this can return negative numbers.
> 
> > @@ -1158,8 +1184,11 @@ tda998x_encoder_init(struct i2c_client *client,
> >  	tda998x_reset(priv);
> >  
> >  	/* read version: */
> > -	priv->rev = reg_read(priv, REG_VERSION_LSB) |
> > -			reg_read(priv, REG_VERSION_MSB) << 8;
> > +	ret = reg_read(priv, REG_VERSION_LSB) |
> > +		(reg_read(priv, REG_VERSION_MSB) << 8);
> > +	if (ret < 0)
> > +		goto fail;
> > +	priv->rev = ret;
> 
> Two issues here:
> 
> 1. The additional parens are /really/ not required.
> 2. What if reg_read(priv, REG_VERSION_MSB) returns a negative number?
> 
> If we're going to the extent of attempting to make the read/write
> functions return errors, we should at least handle errors generated
> by them properly, otherwise it's pointless making them return errors.
> 
> 	ret = reg_read(priv, REG_VERSION_LSB);
> 	if (ret < 0)
> 		goto fail;
> 
> 	priv->rev = ret;
> 
> 	ret = reg_read(priv, REG_VERSION_MSB);
> 	if (ret < 0)
> 		goto fail;
> 
> 	priv->rev |= ret << 8;
> 
> If you want it to look slightly nicer:
> 
> 	int rev_lo, rev_hi;
> 
> 	rev_lo = reg_read(priv, REG_VERSION_LSB);
> 	rev_hi = reg_read(priv, REG_VERSION_MSB);
> 	if (rev_lo < 0 || rev_hi < 0) {
> 		ret = rev_lo < 0 ? rev_lo : rev_hi;
> 		goto fail;
> 	}
> 
> 	priv->rev = rev_lo | rev_hi << 8;
> 
> I'm happy to commit such a change after this patch to clean it up, or if
> you want to regenerate your patch 2 and post /just/ that incorporating
> this change.

I think that my code works correctly: when there is an error, the
result of reg_read() is minus the error code, and this error code is
always lower than 8388607 (0x7fffff). Then, reg_read() << 8 will always
be negative.

Otherwise, I may redo a patch about the useless parenthesis.

-- 
Ken ar c'henta?	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/

^ permalink raw reply

* [PATCH v5 09/23] drm/i2c: tda998x: don't read write-only registers
From: Russell King - ARM Linux @ 2014-02-02 16:23 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <5269e596e16dfe40253dce38ceb0dc4a617384c1.1390986083.git.moinejf@free.fr>

On Sat, Jan 25, 2014 at 06:14:42PM +0100, Jean-Francois Moine wrote:
> This patch takes care of the write-only registers of the tda998x.
> 
> The registers SOFTRESET, TBG_CNTRL_0 and TBG_CNTRL_1 have all bits
> cleared after reset, so, they may be fully re-written.
> 
> The register MAT_CONTRL is set to
> 	MAT_CONTRL_MAT_BP | MAT_CONTRL_MAT_SC(1)
> after reset, so, it may be fully set again to this value.

I said in v3 of this patch, which seems to remain unaddressed:

>       /* must be last register set: */
> -     reg_clear(priv, REG_TBG_CNTRL_0, TBG_CNTRL_0_SYNC_ONCE);
> +     reg_write(priv, REG_TBG_CNTRL_0, 0);

Register changes which have a potential effect shouldn't be part of a
patch which is really only trying to avoid reading from write only
registers.

This could be a potential functional change - and it's probably one
which Rob Clark should at least be made aware of.  As I commented last
time, when you're changing register values in an otherwise innocuous
patch, you should comment about them in the patch description.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

^ permalink raw reply

* [PATCH v5 02/23] drm/i2c: tda998x: check more I/O errors
From: Russell King - ARM Linux @ 2014-02-02 16:20 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <9ebf1fdd613b5747aefb3ee21c9c246d102f1a47.1390986082.git.moinejf@free.fr>

On Sat, Jan 25, 2014 at 06:14:45PM +0100, Jean-Francois Moine wrote:
> This patch adds more error checking inn I2C I/O functions.
> In case of I/O error, this permits to avoid writing in bad controller
> pages, a bad chipset detection or looping when getting the EDID.

I've just looked at this again, and spotted something:

> -static uint8_t
> +static int
>  reg_read(struct tda998x_priv *priv, uint16_t reg)
>  {
>  	uint8_t val = 0;
> -	reg_read_range(priv, reg, &val, sizeof(val));
> +	int ret;
> +
> +	ret = reg_read_range(priv, reg, &val, sizeof(val));
> +	if (ret < 0)
> +		return ret;

So yes, this can return negative numbers.

> @@ -1158,8 +1184,11 @@ tda998x_encoder_init(struct i2c_client *client,
>  	tda998x_reset(priv);
>  
>  	/* read version: */
> -	priv->rev = reg_read(priv, REG_VERSION_LSB) |
> -			reg_read(priv, REG_VERSION_MSB) << 8;
> +	ret = reg_read(priv, REG_VERSION_LSB) |
> +		(reg_read(priv, REG_VERSION_MSB) << 8);
> +	if (ret < 0)
> +		goto fail;
> +	priv->rev = ret;

Two issues here:

1. The additional parens are /really/ not required.
2. What if reg_read(priv, REG_VERSION_MSB) returns a negative number?

If we're going to the extent of attempting to make the read/write
functions return errors, we should at least handle errors generated
by them properly, otherwise it's pointless making them return errors.

	ret = reg_read(priv, REG_VERSION_LSB);
	if (ret < 0)
		goto fail;

	priv->rev = ret;

	ret = reg_read(priv, REG_VERSION_MSB);
	if (ret < 0)
		goto fail;

	priv->rev |= ret << 8;

If you want it to look slightly nicer:

	int rev_lo, rev_hi;

	rev_lo = reg_read(priv, REG_VERSION_LSB);
	rev_hi = reg_read(priv, REG_VERSION_MSB);
	if (rev_lo < 0 || rev_hi < 0) {
		ret = rev_lo < 0 ? rev_lo : rev_hi;
		goto fail;
	}

	priv->rev = rev_lo | rev_hi << 8;

I'm happy to commit such a change after this patch to clean it up, or if
you want to regenerate your patch 2 and post /just/ that incorporating
this change.

-- 
FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up.  Estimation
in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad.
Estimate before purchase was "up to 13.2Mbit".

^ permalink raw reply

* [PATCH 0/3] spi: core: Introduce devm_spi_alloc_master
From: Maxime Ripard @ 2014-02-02 14:54 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20140201173841.GU22609@sirena.org.uk>

Hi,

On Sat, Feb 01, 2014 at 05:38:41PM +0000, Mark Brown wrote:
> On Fri, Jan 31, 2014 at 02:31:11PM +0100, Maxime Ripard wrote:
> > On Fri, Jan 31, 2014 at 12:12:15PM +0000, Mark Brown wrote:
> 
> > > This seems confusing - the idea here is that if we've handed the
> > > device off to the managed function then the managed function
> > > deals with destroying it.  Note that spi_alloc_master() says
> > > that the put is only required after errors adding the device
> > > (which would be the expected behaviour if you look at other
> > > APIs).  Looking at the code I think there is an issue here but
> > > I'm not at all clear that this is the best fix.
> 
> > Ah, right, spi_master_put doesn't free the memory either...
> 
> The memory is freed by the driver core calling the
> spi_master_release() callback when it's safe to do so (after the
> last reference to the device has gone away).
> 
> > I guess we have a few choices here, either:
> >   - Add a devm_kzalloc to spi_alloc_master, since most of the drivers
> >     I've been looking at fail to free the memory, this would be the
> >     least intrusive solution. We'd still have to remove all the kfree
> >     calls in the driver that rightfully free the memory.
> >   - Make devm_unregister_master also call kfree on the master
> >   - Add a kfree to my devm_put_master so that the memory is reclaimed,
> >     which isn't the case for now.
> 
> > I don't have a strong preference here, maybe for the third one, since
> > it makes obvious that it's managed and you don't have to do anything
> > about it, while the other do not.
> 
> None of the above, definitely nothing to do with calling kfree() once
> the device is registered.  We already have that free, it's not the
> issue.  The issue is making sure that we hold references when we need
> them and drop them when we don't.  I (or someone) needs to sit down and
> think it through since things are a bit confused in the code.

Ok. I'll drop the patches and repost the A31 SPI patches.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140202/2205cab3/attachment.sig>

^ permalink raw reply

* [PATCH 3/3] ARM: sunxi: dt: Update the watchdog compatibles
From: Maxime Ripard @ 2014-02-02 13:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1391349325-11132-1-git-send-email-maxime.ripard@free-electrons.com>

The watchdog compatibles were following a different pattern than the one found
in the other devices. Now that the driver supports the right pattern, switch to
it in the DT.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/sun4i-a10.dtsi  | 2 +-
 arch/arm/boot/dts/sun5i-a10s.dtsi | 2 +-
 arch/arm/boot/dts/sun5i-a13.dtsi  | 2 +-
 arch/arm/boot/dts/sun6i-a31.dtsi  | 2 +-
 arch/arm/boot/dts/sun7i-a20.dtsi  | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index 040bb0e..f1eb8718 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -410,7 +410,7 @@
 		};
 
 		wdt: watchdog at 01c20c90 {
-			compatible = "allwinner,sun4i-wdt";
+			compatible = "allwinner,sun4i-a10-wdt";
 			reg = <0x01c20c90 0x10>;
 		};
 
diff --git a/arch/arm/boot/dts/sun5i-a10s.dtsi b/arch/arm/boot/dts/sun5i-a10s.dtsi
index ea16054..c4a4b23 100644
--- a/arch/arm/boot/dts/sun5i-a10s.dtsi
+++ b/arch/arm/boot/dts/sun5i-a10s.dtsi
@@ -373,7 +373,7 @@
 		};
 
 		wdt: watchdog at 01c20c90 {
-			compatible = "allwinner,sun4i-wdt";
+			compatible = "allwinner,sun4i-a10-wdt";
 			reg = <0x01c20c90 0x10>;
 		};
 
diff --git a/arch/arm/boot/dts/sun5i-a13.dtsi b/arch/arm/boot/dts/sun5i-a13.dtsi
index 320335a..143656b 100644
--- a/arch/arm/boot/dts/sun5i-a13.dtsi
+++ b/arch/arm/boot/dts/sun5i-a13.dtsi
@@ -336,7 +336,7 @@
 		};
 
 		wdt: watchdog at 01c20c90 {
-			compatible = "allwinner,sun4i-wdt";
+			compatible = "allwinner,sun4i-a10-wdt";
 			reg = <0x01c20c90 0x10>;
 		};
 
diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
index 5256ad9..cc691d1 100644
--- a/arch/arm/boot/dts/sun6i-a31.dtsi
+++ b/arch/arm/boot/dts/sun6i-a31.dtsi
@@ -242,7 +242,7 @@
 		};
 
 		wdt1: watchdog at 01c20ca0 {
-			compatible = "allwinner,sun6i-wdt";
+			compatible = "allwinner,sun6i-a31-wdt";
 			reg = <0x01c20ca0 0x20>;
 		};
 
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index 119f066..aaa7b3d 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -447,7 +447,7 @@
 		};
 
 		wdt: watchdog at 01c20c90 {
-			compatible = "allwinner,sun4i-wdt";
+			compatible = "allwinner,sun4i-a10-wdt";
 			reg = <0x01c20c90 0x10>;
 		};
 
-- 
1.8.4.2

^ permalink raw reply related

* [PATCH 2/3] ARM: sunxi: Add the new watchog compatibles to the reboot code
From: Maxime Ripard @ 2014-02-02 13:55 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1391349325-11132-1-git-send-email-maxime.ripard@free-electrons.com>

Now that the watchdog driver has new compatibles, we need to support them in
the machine reboot code too.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/mach-sunxi/sunxi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/mach-sunxi/sunxi.c b/arch/arm/mach-sunxi/sunxi.c
index aeea6ce..c310df6 100644
--- a/arch/arm/mach-sunxi/sunxi.c
+++ b/arch/arm/mach-sunxi/sunxi.c
@@ -95,7 +95,9 @@ static void sun6i_restart(enum reboot_mode mode, const char *cmd)
 
 static struct of_device_id sunxi_restart_ids[] = {
 	{ .compatible = "allwinner,sun4i-wdt" },
+	{ .compatible = "allwinner,sun4i-a10-wdt" },
 	{ .compatible = "allwinner,sun6i-wdt" },
+	{ .compatible = "allwinner,sun6i-a31-wdt" },
 	{ /*sentinel*/ }
 };
 
-- 
1.8.4.2

^ permalink raw reply related

* [PATCH 1/3] wdt: sunxi: Introduce a new compatible for the A10 and A31
From: Maxime Ripard @ 2014-02-02 13:55 UTC (permalink / raw)
  To: linux-arm-kernel

For historical reasons, the Allwinner A10 compatibles are not following the
patterns used for this other Allwinner SoCs.

Introduce a new compatible following the usual pattern, and deprecate the olders.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt | 7 ++++---
 drivers/watchdog/sunxi_wdt.c                             | 1 +
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt b/Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt
index e39cb26..6e8c937 100644
--- a/Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt
+++ b/Documentation/devicetree/bindings/watchdog/sunxi-wdt.txt
@@ -2,13 +2,14 @@ Allwinner SoCs Watchdog timer
 
 Required properties:
 
-- compatible : should be "allwinner,<soc-family>-wdt", the currently supported
-  SoC families being sun4i and sun6i
+- compatible : should be either "allwinner,sun4i-a10-wdt" or
+               "allwinner,sun6i-a31-wdt" (deprecated:
+               "allwinner,sun4i-wdt", "allwinner,sun6i-wdt")
 - reg : Specifies base physical address and size of the registers.
 
 Example:
 
 wdt: watchdog at 01c20c90 {
-	compatible = "allwinner,sun4i-wdt";
+	compatible = "allwinner,sun4i-a10-wdt";
 	reg = <0x01c20c90 0x10>;
 };
diff --git a/drivers/watchdog/sunxi_wdt.c b/drivers/watchdog/sunxi_wdt.c
index 76332d8..7c8923d 100644
--- a/drivers/watchdog/sunxi_wdt.c
+++ b/drivers/watchdog/sunxi_wdt.c
@@ -206,6 +206,7 @@ static void sunxi_wdt_shutdown(struct platform_device *pdev)
 
 static const struct of_device_id sunxi_wdt_dt_ids[] = {
 	{ .compatible = "allwinner,sun4i-wdt" },
+	{ .compatible = "allwinner,sun4i-a10-wdt" },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, sunxi_wdt_dt_ids);
-- 
1.8.4.2

^ permalink raw reply related

* [PATCH] ARM: sunxi: dt: Change the touchscreen compatibles
From: Maxime Ripard @ 2014-02-02 13:52 UTC (permalink / raw)
  To: linux-arm-kernel

Switch the device tree touchscreen compatibles to have a common pattern accross
all Allwinner SoCs. Since the touchscreen driver has not been merged yet, it
has no side effect.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/sun4i-a10.dtsi  | 2 +-
 arch/arm/boot/dts/sun5i-a10s.dtsi | 2 +-
 arch/arm/boot/dts/sun5i-a13.dtsi  | 2 +-
 arch/arm/boot/dts/sun7i-a20.dtsi  | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index 040bb0e..e3ff64c 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -426,7 +426,7 @@
 		};
 
 		rtp: rtp at 01c25000 {
-			compatible = "allwinner,sun4i-ts";
+			compatible = "allwinner,sun4i-a10-ts";
 			reg = <0x01c25000 0x100>;
 			interrupts = <29>;
 		};
diff --git a/arch/arm/boot/dts/sun5i-a10s.dtsi b/arch/arm/boot/dts/sun5i-a10s.dtsi
index ea16054..8e57a28 100644
--- a/arch/arm/boot/dts/sun5i-a10s.dtsi
+++ b/arch/arm/boot/dts/sun5i-a10s.dtsi
@@ -383,7 +383,7 @@
 		};
 
 		rtp: rtp at 01c25000 {
-			compatible = "allwinner,sun4i-ts";
+			compatible = "allwinner,sun4i-a10-ts";
 			reg = <0x01c25000 0x100>;
 			interrupts = <29>;
 		};
diff --git a/arch/arm/boot/dts/sun5i-a13.dtsi b/arch/arm/boot/dts/sun5i-a13.dtsi
index 320335a..c463fd7 100644
--- a/arch/arm/boot/dts/sun5i-a13.dtsi
+++ b/arch/arm/boot/dts/sun5i-a13.dtsi
@@ -346,7 +346,7 @@
 		};
 
 		rtp: rtp at 01c25000 {
-			compatible = "allwinner,sun4i-ts";
+			compatible = "allwinner,sun4i-a10-ts";
 			reg = <0x01c25000 0x100>;
 			interrupts = <29>;
 		};
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index 119f066..ccd3790 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -463,7 +463,7 @@
 		};
 
 		rtp: rtp at 01c25000 {
-			compatible = "allwinner,sun4i-ts";
+			compatible = "allwinner,sun4i-a10-ts";
 			reg = <0x01c25000 0x100>;
 			interrupts = <0 29 4>;
 		};
-- 
1.8.4.2

^ permalink raw reply related

* [PATCH 2/2] ARM: sunxi: dt: Convert to the new SID compatibles
From: Maxime Ripard @ 2014-02-02 13:52 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1391349126-10903-1-git-send-email-maxime.ripard@free-electrons.com>

Switch the device tree to the new compatibles introduced in the SID drivers
to have a common pattern accross all Allwinner SoCs.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/sun4i-a10.dtsi  | 2 +-
 arch/arm/boot/dts/sun5i-a10s.dtsi | 2 +-
 arch/arm/boot/dts/sun5i-a13.dtsi  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index 040bb0e..7b10f85 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -421,7 +421,7 @@
 		};
 
 		sid: eeprom at 01c23800 {
-			compatible = "allwinner,sun4i-sid";
+			compatible = "allwinner,sun4i-a10-sid";
 			reg = <0x01c23800 0x10>;
 		};
 
diff --git a/arch/arm/boot/dts/sun5i-a10s.dtsi b/arch/arm/boot/dts/sun5i-a10s.dtsi
index ea16054..6d287a0 100644
--- a/arch/arm/boot/dts/sun5i-a10s.dtsi
+++ b/arch/arm/boot/dts/sun5i-a10s.dtsi
@@ -378,7 +378,7 @@
 		};
 
 		sid: eeprom at 01c23800 {
-			compatible = "allwinner,sun4i-sid";
+			compatible = "allwinner,sun4i-a10-sid";
 			reg = <0x01c23800 0x10>;
 		};
 
diff --git a/arch/arm/boot/dts/sun5i-a13.dtsi b/arch/arm/boot/dts/sun5i-a13.dtsi
index 320335a..f49eb13 100644
--- a/arch/arm/boot/dts/sun5i-a13.dtsi
+++ b/arch/arm/boot/dts/sun5i-a13.dtsi
@@ -341,7 +341,7 @@
 		};
 
 		sid: eeprom at 01c23800 {
-			compatible = "allwinner,sun4i-sid";
+			compatible = "allwinner,sun4i-a10-sid";
 			reg = <0x01c23800 0x10>;
 		};
 
-- 
1.8.4.2

^ permalink raw reply related

* [PATCH 1/2] misc: eeprom: sunxi: Add new compatibles
From: Maxime Ripard @ 2014-02-02 13:52 UTC (permalink / raw)
  To: linux-arm-kernel

The Allwinner A10 compatibles were following a slightly different compatible
patterns than the rest of the SoCs for historical reasons. Add compatibles
matching the other pattern to the SID driver for consistency, and keep the
older one for backward compatibility.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 Documentation/devicetree/bindings/misc/allwinner,sunxi-sid.txt | 5 +++--
 drivers/misc/eeprom/sunxi_sid.c                                | 5 ++++-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/misc/allwinner,sunxi-sid.txt b/Documentation/devicetree/bindings/misc/allwinner,sunxi-sid.txt
index 68ba372..c10a61a 100644
--- a/Documentation/devicetree/bindings/misc/allwinner,sunxi-sid.txt
+++ b/Documentation/devicetree/bindings/misc/allwinner,sunxi-sid.txt
@@ -1,12 +1,13 @@
 Allwinner sunxi-sid
 
 Required properties:
-- compatible: "allwinner,sun4i-sid" or "allwinner,sun7i-a20-sid".
+- compatible: "allwinner,sun4i-a10-sid" or "allwinner,sun7i-a20-sid"
+              (Deprecated: "allwinner,sun4i-sid").
 - reg: Should contain registers location and length
 
 Example for sun4i:
 	sid at 01c23800 {
-		compatible = "allwinner,sun4i-sid";
+		compatible = "allwinner,sun4i-a10-sid";
 		reg = <0x01c23800 0x10>
 	};
 
diff --git a/drivers/misc/eeprom/sunxi_sid.c b/drivers/misc/eeprom/sunxi_sid.c
index 9c34e57..e137e75 100644
--- a/drivers/misc/eeprom/sunxi_sid.c
+++ b/drivers/misc/eeprom/sunxi_sid.c
@@ -96,8 +96,11 @@ static int sunxi_sid_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id sunxi_sid_of_match[] = {
-	{ .compatible = "allwinner,sun4i-sid", .data = (void *)16},
+	{ .compatible = "allwinner,sun4i-a10-sid", .data = (void *)16},
 	{ .compatible = "allwinner,sun7i-a20-sid", .data = (void *)512},
+
+	/* Deprecated */
+	{ .compatible = "allwinner,sun4i-sid", .data = (void *)16},
 	{/* sentinel */},
 };
 MODULE_DEVICE_TABLE(of, sunxi_sid_of_match);
-- 
1.8.4.2

^ permalink raw reply related

* [PATCH 2/2] ARM: sunxi: dt: Convert to the new RTC compatibles
From: Maxime Ripard @ 2014-02-02 13:50 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1391349028-10828-1-git-send-email-maxime.ripard@free-electrons.com>

Switch the device tree to the new compatibles introduced in the RTC drivers
to have a common pattern accross all Allwinner SoCs.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/sun4i-a10.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index 040bb0e..2123934 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -415,7 +415,7 @@
 		};
 
 		rtc: rtc at 01c20d00 {
-			compatible = "allwinner,sun4i-rtc";
+			compatible = "allwinner,sun4i-a10-rtc";
 			reg = <0x01c20d00 0x20>;
 			interrupts = <24>;
 		};
-- 
1.8.4.2

^ permalink raw reply related


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