* [PATCH v6 05/17] media: rkisp1: add Rockchip ISP1 subdev driver
From: Baruch Siach @ 2018-05-07 6:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAAFQd5CvBv4hkE=PSHBJTYa9Lj0SyggxpMBEAYD=if0=T0uzHw@mail.gmail.com>
Hi Tomasz,
On Mon, May 07, 2018 at 06:13:27AM +0000, Tomasz Figa wrote:
> On Thu, May 3, 2018 at 6:09 PM Baruch Siach <baruch@tkos.co.il> wrote:
> > On Thu, Mar 08, 2018 at 05:47:55PM +0800, Jacob Chen wrote:
> > > +static int rkisp1_isp_sd_s_power(struct v4l2_subdev *sd, int on)
> > > +{
> > > + struct rkisp1_device *isp_dev = sd_to_isp_dev(sd);
> > > + int ret;
> > > +
> > > + v4l2_dbg(1, rkisp1_debug, &isp_dev->v4l2_dev, "s_power: %d\n",
> on);
> > > +
> > > + if (on) {
> > > + ret = pm_runtime_get_sync(isp_dev->dev);
> > > + if (ret < 0)
> > > + return ret;
> > > +
> > > + rkisp1_config_clk(isp_dev);
> > > + } else {
> > > + ret = pm_runtime_put(isp_dev->dev);
>
> > I commented this line out to make more than one STREAMON work. Otherwise,
> > the second STREAMON hangs. I guess the bug is not this driver. Probably
> > something in drivers/soc/rockchip/pm_domains.c. Just noting that in case
> > you or someone on Cc would like to investigate it further.
> >
> > I tested v4.16-rc4 on the Tinkerboard.
>
> Looks like that version doesn't include the IOMMU PM and clock handling
> rework [1], which should fix a lot of runtime PM issues. FWIW, linux-next
> seems to already include it.
>
> [1] https://lkml.org/lkml/2018/3/23/44
Thanks for the reference.
It looks like the iommu driver part is in Linus' tree already. The DT part is
in the v4.18-armsoc/dts32 branch of Heiko's tree. Am I missing anything?
Anyway, I'll take a look.
Thanks again,
baruch
--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
^ permalink raw reply
* [PATCH 1/2] ARM: dts: imx28-cfa10049: Remove unnecessary #address/#size-cells
From: Maxime Ripard @ 2018-05-07 6:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1525655020-31260-1-git-send-email-festevam@gmail.com>
On Sun, May 06, 2018 at 10:03:39PM -0300, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@nxp.com>
>
> Remove unnecessary #address-cells/#size-cells to fix the following
> DTC warnings:
>
> arch/arm/boot/dts/imx28-cfa10049.dtb: Warning (avoid_unnecessary_addr_size): /gpio_keys: unnecessary #address-cells/#size-cells without "ranges" or child "reg" property
>
> Cc: Maxime Ripard <maxime.ripard@bootlin.com>
> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>
Thanks!
Maxime
--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180507/3cafc633/attachment.sig>
^ permalink raw reply
* [PATCH v6 05/17] media: rkisp1: add Rockchip ISP1 subdev driver
From: Tomasz Figa @ 2018-05-07 6:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180507063814.vweb4p3nfgnoc3td@tarshish>
On Mon, May 7, 2018 at 3:38 PM Baruch Siach <baruch@tkos.co.il> wrote:
> Hi Tomasz,
> On Mon, May 07, 2018 at 06:13:27AM +0000, Tomasz Figa wrote:
> > On Thu, May 3, 2018 at 6:09 PM Baruch Siach <baruch@tkos.co.il> wrote:
> > > On Thu, Mar 08, 2018 at 05:47:55PM +0800, Jacob Chen wrote:
> > > > +static int rkisp1_isp_sd_s_power(struct v4l2_subdev *sd, int on)
> > > > +{
> > > > + struct rkisp1_device *isp_dev = sd_to_isp_dev(sd);
> > > > + int ret;
> > > > +
> > > > + v4l2_dbg(1, rkisp1_debug, &isp_dev->v4l2_dev, "s_power: %d\n",
> > on);
> > > > +
> > > > + if (on) {
> > > > + ret = pm_runtime_get_sync(isp_dev->dev);
> > > > + if (ret < 0)
> > > > + return ret;
> > > > +
> > > > + rkisp1_config_clk(isp_dev);
> > > > + } else {
> > > > + ret = pm_runtime_put(isp_dev->dev);
> >
> > > I commented this line out to make more than one STREAMON work.
Otherwise,
> > > the second STREAMON hangs. I guess the bug is not this driver.
Probably
> > > something in drivers/soc/rockchip/pm_domains.c. Just noting that in
case
> > > you or someone on Cc would like to investigate it further.
> > >
> > > I tested v4.16-rc4 on the Tinkerboard.
> >
> > Looks like that version doesn't include the IOMMU PM and clock handling
> > rework [1], which should fix a lot of runtime PM issues. FWIW,
linux-next
> > seems to already include it.
> >
> > [1] https://lkml.org/lkml/2018/3/23/44
> Thanks for the reference.
> It looks like the iommu driver part is in Linus' tree already. The DT
part is
> in the v4.18-armsoc/dts32 branch of Heiko's tree. Am I missing anything?
You're right, most of the series made it in time for 4.17. However, the DT
part (precisely, the clocks properties added to IOMMU nodes) is crucial for
the fixes to be effective.
> Anyway, I'll take a look.
Thanks for testing. :) (Forgot to mention in my previous email...)
Best regards,
Tomasz
^ permalink raw reply
* [RFC PATCH] locking/atomics/x86/64: Clean up and fix details of <asm/atomic64_64.h>
From: Ingo Molnar @ 2018-05-07 6:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180505093237.GD12217@hirez.programming.kicks-ass.net>
* Peter Zijlstra <peterz@infradead.org> wrote:
> On Sat, May 05, 2018 at 11:05:51AM +0200, Dmitry Vyukov wrote:
> > On Sat, May 5, 2018 at 10:47 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> > > And I seriously hate this one:
> > >
> > > ba1c9f83f633 ("locking/atomic/x86: Un-macro-ify atomic ops implementation")
> > >
> > > and will likely undo that the moment I need to change anything there.
>
> > That was asked by Ingo:
> > https://groups.google.com/d/msg/kasan-dev/3sNHjjb4GCI/Xz1uVWaaAAAJ
> >
> > I think in the end all of current options suck in one way or another,
> > so we are just going in circles.
>
> Yeah, and I disagree with him, but didn't have the energy to fight at
> that time (and still don't really, I'm just complaining).
Ok, this negative side effect of the un-macro-ifying still bothers me, so I tried
to do something about it:
1 file changed, 81 insertions(+), 172 deletions(-)
That's actually better than what we had before ba1c9f83f633, which did:
3 files changed, 147 insertions(+), 70 deletions(-)
( Also consider that my patch adds new DocBook comments and only touches the
64-bit side, so if we do the same to the 32-bit header as well we'll gain even
more. )
But I don't think you'll like my solution: it requires twice as wide terminals to
look at those particular functions...
The trick is to merge the C functions into a single line: this makes it look a bit
weird, but it's still very readable (because the functions are simple), and, most
importantly, the _differences_ are now easily visible.
This is how that part looks like on my terminal:
... do { } while (!arch_atomic64_try_cmpxchg(v, &val, val & i)); return val; }
... do { } while (!arch_atomic64_try_cmpxchg(v, &val, val | i)); return val; }
... do { } while (!arch_atomic64_try_cmpxchg(v, &val, val ^ i)); return val; }
...
... { asm(LOCK_PREFIX "orq %1,%0" : "+m" (v->counter) : "er" (i) : "memory"); }
... { asm(LOCK_PREFIX "xorq %1,%0" : "+m" (v->counter) : "er" (i) : "memory"); }
... { asm(LOCK_PREFIX "andq %1,%0" : "+m" (v->counter) : "er" (i) : "memory"); }
Which makes the '&|^' and orq/xorq/andq differences easily visible. I kept all
those long lines at the end of the header, to make it easy not to look at it,
unless someone wants to actively change or understand the code.
The prototype/parameters are visible even with col80 terminals.
But please give it a try and at least _look_ at the result in a double-wide
terminal.
There's also a ton of other small fixes and enhancements I did to the header file
- see the changelog.
But I won't commit this without your explicit Acked-by.
Thanks,
Ingo
================>
From: Ingo Molnar <mingo@kernel.org>
Date: Mon, 7 May 2018 07:59:12 +0200
Subject: [PATCH] locking/atomics/x86/64: Clean up and fix details of <asm/atomic64_64.h>
PeterZ complained that the following macro elimination:
ba1c9f83f633: locking/atomic/x86: Un-macro-ify atomic ops implementation
bloated the header and made it less readable/maintainable.
Try to undo some of that damage, without reintroducing a macro mess:
- Merge the previously macro generated C inline functions into a single line.
While this looks weird on smaller terminals because of line wraps
(and violates the col80 style rule brutally), it's surprisingly readable
on larger terminals. An advantage with this format is that the repeated
patterns are obviously visible, while the actual differences stick out
very clearly. To me this is much more readable than the old macro solution.
While at it, also do a no-prisoners taken cleanup pass of all the code and comments
in this file:
- Fix the DocBook comment of arch_atomic64_add_unless(), which incorrectly claimed:
"Returns the old value of @v."
In reality the function returns true if the operation was performed, or false otherwise.
- Fix __always_inline use: where one member of a 'pair' of an operation has it,
add it to both. There's no reason for them to ever deviate.
- Fix/clarify the arch_atomic64_read() documentation: it not only reads but also
returns the value.
- Fix DocBook comments that referred to 'i' and 'v' instead of '@i' and '@v'.
- Remove unnecessary parentheses from arch_atomic64_read() that was probably
inherited from ancient macro versions.
- Add DocBook description for arch_atomic64_[add|sub]_return() and
arch_atomic64_fetch_[add|sub]().
- Harmonize to a single variant of referring to atomic64_t pointers in
comments, instead of these two variants:
"@v: pointer of type atomic64_t"
"@v: pointer to type atomic64_t"
Use a single, shorter variant:
"@v: pointer to atomic64_t"
(Because the _t already implies that this is a type - no need to repeat that.)
- Harmonize local variable naming, from a sometimes inconsistent selection of ad-hoc
and sometimes cryptic variable names ('c', 'val', 'dec', 'val_old', etc.), to the
following set of standardized local variable names:
'i' for integer arguments
'val_old' for old atomic values
'val_new' for old atomic values
From now on the name of the local variable is "obviously descriptive" of its
role in the code.
- Add newlines after local variable definitions and before return statements, consistently.
- Change weird "@i: required value" phrase (all arguments to function calls are
required, what does 'required' mean here?) to '@i: new value'
- Add "Doesn't imply a write memory barrier" to arch_atomic64_set(), to
mirror the read barrier comment of arch_atomic64_read().
- Change the weird "or false for all other cases" phrase to the simpler "or false otherwise"
formulation that is standard in DocBook comments.
- Harmonize the punctuation of DocBook comments: detailed descriptions always end with a period.
- Change the order of addition in the arch_atomic64_[add|sub]_return() code,
to make it match the order in the description. (No change in functionality,
addition is commutative.)
- Remove unnecessary double newlines.
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: linux-kernel at vger.kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/include/asm/atomic64_64.h | 253 ++++++++++++-------------------------
1 file changed, 81 insertions(+), 172 deletions(-)
diff --git a/arch/x86/include/asm/atomic64_64.h b/arch/x86/include/asm/atomic64_64.h
index 6106b59d3260..1c451871b391 100644
--- a/arch/x86/include/asm/atomic64_64.h
+++ b/arch/x86/include/asm/atomic64_64.h
@@ -12,22 +12,23 @@
/**
* arch_atomic64_read - read atomic64 variable
- * @v: pointer of type atomic64_t
+ * @v: pointer to atomic64_t
*
- * Atomically reads the value of @v.
+ * Atomically reads and returns the value of @v.
* Doesn't imply a read memory barrier.
*/
static inline long arch_atomic64_read(const atomic64_t *v)
{
- return READ_ONCE((v)->counter);
+ return READ_ONCE(v->counter);
}
/**
* arch_atomic64_set - set atomic64 variable
- * @v: pointer to type atomic64_t
- * @i: required value
+ * @v: pointer to atomic64_t
+ * @i: new value
*
* Atomically sets the value of @v to @i.
+ * Doesn't imply a write memory barrier.
*/
static inline void arch_atomic64_set(atomic64_t *v, long i)
{
@@ -35,41 +36,22 @@ static inline void arch_atomic64_set(atomic64_t *v, long i)
}
/**
- * arch_atomic64_add - add integer to atomic64 variable
- * @i: integer value to add
- * @v: pointer to type atomic64_t
+ * arch_atomic64_[add|sub] - add|subtract integer to/from atomic64 variable
+ * @i: integer value to add/subtract
+ * @v: pointer to atomic64_t
*
- * Atomically adds @i to @v.
+ * Atomically adds/subtracts @i to/from @v.
*/
-static __always_inline void arch_atomic64_add(long i, atomic64_t *v)
-{
- asm volatile(LOCK_PREFIX "addq %1,%0"
- : "=m" (v->counter)
- : "er" (i), "m" (v->counter));
-}
-
-/**
- * arch_atomic64_sub - subtract the atomic64 variable
- * @i: integer value to subtract
- * @v: pointer to type atomic64_t
- *
- * Atomically subtracts @i from @v.
- */
-static inline void arch_atomic64_sub(long i, atomic64_t *v)
-{
- asm volatile(LOCK_PREFIX "subq %1,%0"
- : "=m" (v->counter)
- : "er" (i), "m" (v->counter));
-}
+static __always_inline void arch_atomic64_sub(long i, atomic64_t *v) { asm(LOCK_PREFIX "subq %1,%0" : "=m" (v->counter) : "er" (i), "m" (v->counter)); }
+static __always_inline void arch_atomic64_add(long i, atomic64_t *v) { asm(LOCK_PREFIX "addq %1,%0" : "=m" (v->counter) : "er" (i), "m" (v->counter)); }
/**
* arch_atomic64_sub_and_test - subtract value from variable and test result
* @i: integer value to subtract
- * @v: pointer to type atomic64_t
+ * @v: pointer to atomic64_t
*
* Atomically subtracts @i from @v and returns
- * true if the result is zero, or false for all
- * other cases.
+ * true if the result is zero, or false otherwise.
*/
static inline bool arch_atomic64_sub_and_test(long i, atomic64_t *v)
{
@@ -77,133 +59,101 @@ static inline bool arch_atomic64_sub_and_test(long i, atomic64_t *v)
}
/**
- * arch_atomic64_inc - increment atomic64 variable
- * @v: pointer to type atomic64_t
+ * arch_atomic64_[inc|dec] - increment/decrement atomic64 variable
+ * @v: pointer to atomic64_t
*
- * Atomically increments @v by 1.
+ * Atomically increments/decrements @v by 1.
*/
-static __always_inline void arch_atomic64_inc(atomic64_t *v)
-{
- asm volatile(LOCK_PREFIX "incq %0"
- : "=m" (v->counter)
- : "m" (v->counter));
-}
+static __always_inline void arch_atomic64_inc(atomic64_t *v) { asm(LOCK_PREFIX "incq %0" : "=m" (v->counter) : "m" (v->counter)); }
+static __always_inline void arch_atomic64_dec(atomic64_t *v) { asm(LOCK_PREFIX "decq %0" : "=m" (v->counter) : "m" (v->counter)); }
/**
- * arch_atomic64_dec - decrement atomic64 variable
- * @v: pointer to type atomic64_t
+ * arch_atomic64_[inc|dec]_and_test - increment/decrement and test
+ * @v: pointer to atomic64_t
*
- * Atomically decrements @v by 1.
+ * Atomically increments/decrements @v by 1 and
+ * returns true if the result is 0, or false otherwise.
*/
-static __always_inline void arch_atomic64_dec(atomic64_t *v)
-{
- asm volatile(LOCK_PREFIX "decq %0"
- : "=m" (v->counter)
- : "m" (v->counter));
-}
+static inline bool arch_atomic64_dec_and_test(atomic64_t *v) { GEN_UNARY_RMWcc(LOCK_PREFIX "decq", v->counter, "%0", e); }
+static inline bool arch_atomic64_inc_and_test(atomic64_t *v) { GEN_UNARY_RMWcc(LOCK_PREFIX "incq", v->counter, "%0", e); }
/**
- * arch_atomic64_dec_and_test - decrement and test
- * @v: pointer to type atomic64_t
+ * arch_atomic64_add_negative - add and test if negative
+ * @i: integer value to add
+ * @v: pointer to atomic64_t
*
- * Atomically decrements @v by 1 and
- * returns true if the result is 0, or false for all other
- * cases.
+ * Atomically adds @i to @v and returns true
+ * if the result is negative, or false otherwise.
*/
-static inline bool arch_atomic64_dec_and_test(atomic64_t *v)
+static inline bool arch_atomic64_add_negative(long i, atomic64_t *v)
{
- GEN_UNARY_RMWcc(LOCK_PREFIX "decq", v->counter, "%0", e);
+ GEN_BINARY_RMWcc(LOCK_PREFIX "addq", v->counter, "er", i, "%0", s);
}
/**
- * arch_atomic64_inc_and_test - increment and test
- * @v: pointer to type atomic64_t
+ * arch_atomic64_[add|sub]_return - add/subtract and return
+ * @i: integer value to add/subtract
+ * @v: pointer to atomic64_t
*
- * Atomically increments @v by 1
- * and returns true if the result is zero, or false for all
- * other cases.
+ * Atomically adds/subtracts @i to/from @v and returns the new value of @v.
*/
-static inline bool arch_atomic64_inc_and_test(atomic64_t *v)
-{
- GEN_UNARY_RMWcc(LOCK_PREFIX "incq", v->counter, "%0", e);
-}
+static __always_inline long arch_atomic64_add_return(long i, atomic64_t *v) { return xadd(&v->counter, +i) + i; }
+static __always_inline long arch_atomic64_sub_return(long i, atomic64_t *v) { return xadd(&v->counter, -i) - i; }
/**
- * arch_atomic64_add_negative - add and test if negative
- * @i: integer value to add
- * @v: pointer to type atomic64_t
+ * arch_atomic64_fetch_[add|sub]_return - add/subtract and return old value
+ * @i: integer value to add/subtract
+ * @v: pointer to atomic64_t
*
- * Atomically adds @i to @v and returns true
- * if the result is negative, or false when
- * result is greater than or equal to zero.
+ * Atomically adds/subtracts @i to/from @v and returns the old value of @v.
*/
-static inline bool arch_atomic64_add_negative(long i, atomic64_t *v)
-{
- GEN_BINARY_RMWcc(LOCK_PREFIX "addq", v->counter, "er", i, "%0", s);
-}
+static __always_inline long arch_atomic64_fetch_add (long i, atomic64_t *v) { return xadd(&v->counter, +i); }
+static __always_inline long arch_atomic64_fetch_sub (long i, atomic64_t *v) { return xadd(&v->counter, -i); }
/**
- * arch_atomic64_add_return - add and return
- * @i: integer value to add
- * @v: pointer to type atomic64_t
+ * arch_atomic64_[inc|dec]_return - increment/decrement and return
+ * @v: pointer to atomic64_t
*
- * Atomically adds @i to @v and returns @i + @v
+ * Atomically increments/decrements @v and returns the new value of @v.
*/
-static __always_inline long arch_atomic64_add_return(long i, atomic64_t *v)
-{
- return i + xadd(&v->counter, i);
-}
-
-static inline long arch_atomic64_sub_return(long i, atomic64_t *v)
-{
- return arch_atomic64_add_return(-i, v);
-}
+#define arch_atomic64_inc_return(v) arch_atomic64_add_return(1, (v))
+#define arch_atomic64_dec_return(v) arch_atomic64_sub_return(1, (v))
-static inline long arch_atomic64_fetch_add(long i, atomic64_t *v)
+static inline long arch_atomic64_cmpxchg(atomic64_t *v, long val_old, long val_new)
{
- return xadd(&v->counter, i);
-}
-
-static inline long arch_atomic64_fetch_sub(long i, atomic64_t *v)
-{
- return xadd(&v->counter, -i);
-}
-
-#define arch_atomic64_inc_return(v) (arch_atomic64_add_return(1, (v)))
-#define arch_atomic64_dec_return(v) (arch_atomic64_sub_return(1, (v)))
-
-static inline long arch_atomic64_cmpxchg(atomic64_t *v, long old, long new)
-{
- return arch_cmpxchg(&v->counter, old, new);
+ return arch_cmpxchg(&v->counter, val_old, val_new);
}
#define arch_atomic64_try_cmpxchg arch_atomic64_try_cmpxchg
-static __always_inline bool arch_atomic64_try_cmpxchg(atomic64_t *v, s64 *old, long new)
+
+static __always_inline bool arch_atomic64_try_cmpxchg(atomic64_t *v, s64 *val_old, long val_new)
{
- return try_cmpxchg(&v->counter, old, new);
+ return try_cmpxchg(&v->counter, val_old, val_new);
}
-static inline long arch_atomic64_xchg(atomic64_t *v, long new)
+static inline long arch_atomic64_xchg(atomic64_t *v, long val_new)
{
- return xchg(&v->counter, new);
+ return xchg(&v->counter, val_new);
}
/**
* arch_atomic64_add_unless - add unless the number is a given value
- * @v: pointer of type atomic64_t
- * @a: the amount to add to v...
- * @u: ...unless v is equal to u.
+ * @v: pointer to atomic64_t
+ * @i: the amount to add to @v...
+ * @u: ...unless @v is equal to @u
*
- * Atomically adds @a to @v, so long as it was not @u.
- * Returns the old value of @v.
+ * Atomically adds @i to @v, so long as @v was not @u.
+ * Returns true if the operation was performed, or false otherwise.
*/
-static inline bool arch_atomic64_add_unless(atomic64_t *v, long a, long u)
+static inline bool arch_atomic64_add_unless(atomic64_t *v, long i, long u)
{
- s64 c = arch_atomic64_read(v);
+ s64 val_old = arch_atomic64_read(v);
+
do {
- if (unlikely(c == u))
+ if (unlikely(val_old == u))
return false;
- } while (!arch_atomic64_try_cmpxchg(v, &c, c + a));
+ } while (!arch_atomic64_try_cmpxchg(v, &val_old, val_old + i));
+
return true;
}
@@ -211,71 +161,30 @@ static inline bool arch_atomic64_add_unless(atomic64_t *v, long a, long u)
/*
* arch_atomic64_dec_if_positive - decrement by 1 if old value positive
- * @v: pointer of type atomic_t
+ * @v: pointer to type atomic_t
*
* The function returns the old value of *v minus 1, even if
- * the atomic variable, v, was not decremented.
+ * @v was not decremented.
*/
static inline long arch_atomic64_dec_if_positive(atomic64_t *v)
{
- s64 dec, c = arch_atomic64_read(v);
- do {
- dec = c - 1;
- if (unlikely(dec < 0))
- break;
- } while (!arch_atomic64_try_cmpxchg(v, &c, dec));
- return dec;
-}
-
-static inline void arch_atomic64_and(long i, atomic64_t *v)
-{
- asm volatile(LOCK_PREFIX "andq %1,%0"
- : "+m" (v->counter)
- : "er" (i)
- : "memory");
-}
-
-static inline long arch_atomic64_fetch_and(long i, atomic64_t *v)
-{
- s64 val = arch_atomic64_read(v);
-
- do {
- } while (!arch_atomic64_try_cmpxchg(v, &val, val & i));
- return val;
-}
-
-static inline void arch_atomic64_or(long i, atomic64_t *v)
-{
- asm volatile(LOCK_PREFIX "orq %1,%0"
- : "+m" (v->counter)
- : "er" (i)
- : "memory");
-}
-
-static inline long arch_atomic64_fetch_or(long i, atomic64_t *v)
-{
- s64 val = arch_atomic64_read(v);
+ s64 val_new, val = arch_atomic64_read(v);
do {
- } while (!arch_atomic64_try_cmpxchg(v, &val, val | i));
- return val;
-}
+ val_new = val - 1;
+ if (unlikely(val_new < 0))
+ break;
+ } while (!arch_atomic64_try_cmpxchg(v, &val, val_new));
-static inline void arch_atomic64_xor(long i, atomic64_t *v)
-{
- asm volatile(LOCK_PREFIX "xorq %1,%0"
- : "+m" (v->counter)
- : "er" (i)
- : "memory");
+ return val_new;
}
-static inline long arch_atomic64_fetch_xor(long i, atomic64_t *v)
-{
- s64 val = arch_atomic64_read(v);
+static inline long arch_atomic64_fetch_and(long i, atomic64_t *v) { s64 val = arch_atomic64_read(v); do { } while (!arch_atomic64_try_cmpxchg(v, &val, val & i)); return val; }
+static inline long arch_atomic64_fetch_or (long i, atomic64_t *v) { s64 val = arch_atomic64_read(v); do { } while (!arch_atomic64_try_cmpxchg(v, &val, val | i)); return val; }
+static inline long arch_atomic64_fetch_xor(long i, atomic64_t *v) { s64 val = arch_atomic64_read(v); do { } while (!arch_atomic64_try_cmpxchg(v, &val, val ^ i)); return val; }
- do {
- } while (!arch_atomic64_try_cmpxchg(v, &val, val ^ i));
- return val;
-}
+static inline void arch_atomic64_or (long i, atomic64_t *v) { asm(LOCK_PREFIX "orq %1,%0" : "+m" (v->counter) : "er" (i) : "memory"); }
+static inline void arch_atomic64_xor(long i, atomic64_t *v) { asm(LOCK_PREFIX "xorq %1,%0" : "+m" (v->counter) : "er" (i) : "memory"); }
+static inline void arch_atomic64_and(long i, atomic64_t *v) { asm(LOCK_PREFIX "andq %1,%0" : "+m" (v->counter) : "er" (i) : "memory"); }
#endif /* _ASM_X86_ATOMIC64_64_H */
^ permalink raw reply related
* [PATCH] locking/atomics/powerpc: Move cmpxchg helpers to asm/cmpxchg.h and define the full set of cmpxchg APIs
From: Ingo Molnar @ 2018-05-07 6:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1525655045.499830.1362901928.0EBFEACA@webmail.messagingengine.com>
* Boqun Feng <boqun.feng@gmail.com> wrote:
>
>
> On Sun, May 6, 2018, at 8:11 PM, Ingo Molnar wrote:
> >
> > * Boqun Feng <boqun.feng@gmail.com> wrote:
> >
> > > > The only change I made beyond a trivial build fix is that I also added the release
> > > > atomics variants explicitly:
> > > >
> > > > +#define atomic_cmpxchg_release(v, o, n) \
> > > > + cmpxchg_release(&((v)->counter), (o), (n))
> > > > +#define atomic64_cmpxchg_release(v, o, n) \
> > > > + cmpxchg_release(&((v)->counter), (o), (n))
> > > >
> > > > It has passed a PowerPC cross-build test here, but no runtime tests.
> > > >
> > >
> > > Do you have the commit at any branch in tip tree? I could pull it and
> > > cross-build and check the assembly code of lib/atomic64_test.c, that way
> > > I could verify whether we mess something up.
> > >
> > > > Does this patch look good to you?
> > > >
> > >
> > > Yep!
> >
> > Great - I have pushed the commits out into the locking tree, they can be
> > found in:
> >
> > git fetch git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
> > locking/core
> >
>
> Thanks! My compile test told me that we need to remove the definitions of
> atomic_xchg and atomic64_xchg in ppc's asm/atomic.h: they are now
> duplicate, and will prevent the generation of _release and _acquire in the
> new logic.
>
> If you need a updated patch for this from me, I could send later today.
> (I don't have a handy environment for patch sending now, so...)
That would be cool, thanks! My own cross-build testing didn't trigger that build
failure.
> Other than this, the modification looks fine, the lib/atomic64_test.c
> generated the same asm before and after the patches.
Cool, thanks for checking!
Thanks,
Ingo
^ permalink raw reply
* [PATCH v3 1/3] drm/panel: Add RGB666 variant of Innolux AT070TN90
From: Maxime Ripard @ 2018-05-07 7:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180506214901.23429-1-contact@paulk.fr>
Hi,
On Sun, May 06, 2018 at 11:48:59PM +0200, Paul Kocialkowski wrote:
> This adds timings for the RGB666 variant of the Innolux AT070TN90 panel,
> as found on the Ainol AW1 tablet.
>
> The panel also supports RGB888 output. When RGB666 mode is used instead,
> the two extra lanes per component are grounded.
>
> In the future, it might become necessary to introduce a dedicated
> device-tree property to specify the bus format and maybe specify it in
> the mode description instead of panel description so that the
> appropriate mode can be selected for each bus format.
>
> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
A change log would be nice. Also, you mentionned in your first version
that the screen was an AT070TN92, and now you mention that it is an
AT070TN90, which one is it?
Maxime
> ---
> drivers/gpu/drm/panel/panel-simple.c | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c
> index cbf1ab404ee7..351742df8ee1 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -1086,6 +1086,29 @@ static const struct panel_desc innolux_at070tn92 = {
> .bus_format = MEDIA_BUS_FMT_RGB888_1X24,
> };
>
> +static const struct drm_display_mode innolux_at070tn90_mode = {
> + .clock = 40000,
> + .hdisplay = 800,
> + .hsync_start = 800 + 112,
> + .hsync_end = 800 + 112 + 1,
> + .htotal = 800 + 112 + 1 + 87,
> + .vdisplay = 480,
> + .vsync_start = 480 + 141,
> + .vsync_end = 480 + 141 + 1,
> + .vtotal = 480 + 141 + 1 + 38,
> + .vrefresh = 60,
> +};
> +
> +static const struct panel_desc innolux_at070tn90 = {
> + .modes = &innolux_at070tn90_mode,
> + .num_modes = 1,
> + .size = {
> + .width = 154,
> + .height = 86,
> + },
> + .bus_format = MEDIA_BUS_FMT_RGB666_1X18,
> +};
> +
> static const struct display_timing innolux_g101ice_l01_timing = {
> .pixelclock = { 60400000, 71100000, 74700000 },
> .hactive = { 1280, 1280, 1280 },
> @@ -2154,6 +2177,9 @@ static const struct of_device_id platform_of_match[] = {
> }, {
> .compatible = "innolux,at070tn92",
> .data = &innolux_at070tn92,
> + }, {
> + .compatible = "innolux,at070tn90",
> + .data = &innolux_at070tn90,
This should be ordered alphabetically.
Maxime
--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180507/c2eb471e/attachment.sig>
^ permalink raw reply
* [PATCH v3 3/3] ARM: dts: sun7i: Add support for the Ainol AW1 tablet
From: Maxime Ripard @ 2018-05-07 7:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180506214901.23429-3-contact@paulk.fr>
Hi,
On Sun, May 06, 2018 at 11:49:01PM +0200, Paul Kocialkowski wrote:
> This adds support for the Ainol AW1, an A20-based 7" tablet from Ainol.
>
> The following board-specific features are supported:
> * LCD panel
> * Backlight
> * USB OTG
> * Buttons
> * Touchscreen (doesn't work without non-free firmware)
> * Accelerometer
> * Battery
>
> The following are untested:
> * Audio output
> * Audio speakers
> * USB via SPCI connector
>
> The following are not supported:
> * Wi-Fi
> * Bluetooth
> * NAND
> * Audio via SPCI connector
> * Audio via Bluetooth I2S
>
> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> ---
> arch/arm/boot/dts/Makefile | 1 +
> arch/arm/boot/dts/sun7i-a20-ainol-aw1.dts | 275 ++++++++++++++++++++++
> 2 files changed, 276 insertions(+)
> create mode 100644 arch/arm/boot/dts/sun7i-a20-ainol-aw1.dts
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 7e2424957809..4a80971f2bc9 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -946,6 +946,7 @@ dtb-$(CONFIG_MACH_SUN6I) += \
> sun6i-a31s-sinovoip-bpi-m2.dtb \
> sun6i-a31s-yones-toptech-bs1078-v2.dtb
> dtb-$(CONFIG_MACH_SUN7I) += \
> + sun7i-a20-ainol-aw1.dtb \
> sun7i-a20-bananapi.dtb \
> sun7i-a20-bananapi-m1-plus.dtb \
> sun7i-a20-bananapro.dtb \
> diff --git a/arch/arm/boot/dts/sun7i-a20-ainol-aw1.dts b/arch/arm/boot/dts/sun7i-a20-ainol-aw1.dts
> new file mode 100644
> index 000000000000..9a1d54a9f9a0
> --- /dev/null
> +++ b/arch/arm/boot/dts/sun7i-a20-ainol-aw1.dts
> @@ -0,0 +1,275 @@
> +/*
> + * Copyright (C) 2018 Paul Kocialkowski <contact@paulk.fr>
> + *
> + * SPDX-License-Identifier: GPL-2.0+
This should be your first line. Also, we usually license our DT under
a dual license (GPL and MIT) so that other projects (like FreeBSD) can
use them as well, instead of duplicating them. It would be great if
you'd consider it.
> + */
> +
> +/dts-v1/;
> +#include "sun7i-a20.dtsi"
> +#include "sunxi-common-regulators.dtsi"
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/pwm/pwm.h>
> +
> +/ {
> + model = "Ainol AW1";
> + compatible = "ainol,ainol-aw1", "allwinner,sun7i-a20";
> +
> + aliases {
> + serial0 = &uart0;
> + };
> +
> + chosen {
> + stdout-path = "serial0:115200n8";
> + };
> +
> + backlight: backlight {
> + compatible = "pwm-backlight";
> + pwms = <&pwm 0 50000 PWM_POLARITY_INVERTED>;
> + brightness-levels = <0 10 20 30 40 50 60 70 80 90 100>;
The increase in perceived brightness should be linear. Usually, for
PWMs backed backlight, an exponential list is a much better
approximation.
> + default-brightness-level = <5>;
> + enable-gpios = <&pio 7 7 GPIO_ACTIVE_HIGH>; /* PH7 */
> + };
> +
> + panel: panel {
> + compatible = "innolux,at070tn90";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + power-supply = <&panel_power>;
> + backlight = <&backlight>;
> +
> + port at 0 {
> + reg = <0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + panel_input: endpoint at 0 {
> + reg = <0>;
> + remote-endpoint = <&tcon0_out_panel>;
> + };
> + };
> + };
> +
> + panel_power: panel_power {
> + compatible = "regulator-fixed";
> + pinctrl-names = "default";
> + pinctrl-0 = <&panel_power_pin>;
> + regulator-name = "panel-power";
> + regulator-min-microvolt = <10400000>;
> + regulator-max-microvolt = <10400000>;
> + gpio = <&pio 7 8 GPIO_ACTIVE_HIGH>; /* PH8 */
> + enable-active-high;
> + regulator-boot-on;
> + };
> +};
> +
> +&codec {
> + allwinner,pa-gpios = <&pio 7 15 GPIO_ACTIVE_HIGH>; /* PH15 */
> + status = "okay";
> +};
> +
> +&cpu0 {
> + cpu-supply = <®_dcdc2>;
> +};
> +
> +&de {
> + status = "okay";
> +};
> +
> +&ehci0 {
> + status = "okay";
> +};
> +
> +&ehci1 {
> + status = "okay";
> +};
> +
> +&i2c0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c0_pins_a>;
> + status = "okay";
> +
> + axp209: pmic at 34 {
> + reg = <0x34>;
> + interrupt-parent = <&nmi_intc>;
> + interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> + };
> +};
> +
> +&i2c1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c1_pins_a>;
> + status = "okay";
> +
> + lis3dh: accelerometer at 18 {
> + compatible = "st,lis3dh-accel";
> + reg = <0x18>;
> + vdd-supply = <®_vcc3v3>;
> + vddio-supply = <®_vcc3v3>;
> + st,drdy-int-pin = <1>;
> + };
> +};
> +
> +&i2c2 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&i2c2_pins_a>;
> + status = "okay";
> + clock-frequency = <400000>;
> +
> + gsl1680: touchscreen at 40 {
> + compatible = "silead,gsl1680";
> + reg = <0x40>;
> + interrupt-parent = <&pio>;
> + interrupts = <7 21 IRQ_TYPE_EDGE_FALLING>; /* EINT21 (PH21) */
> + power-gpios = <&pio 7 20 GPIO_ACTIVE_HIGH>; /* PH20 */
> + firmware-name = "gsl1680-ainol-aw1.fw";
> + touchscreen-size-x = <480>;
> + touchscreen-size-y = <800>;
> + touchscreen-swapped-x-y;
> + touchscreen-inverted-y;
> + silead,max-fingers = <5>;
> + };
> +};
> +
> +&lradc {
> + vref-supply = <®_ldo2>;
> + status = "okay";
> +
> + button at 571 {
> + label = "Volume Up";
> + linux,code = <KEY_VOLUMEUP>;
> + channel = <0>;
> + voltage = <571428>;
> + };
> +
> + button at 761 {
> + label = "Volume Down";
> + linux,code = <KEY_VOLUMEDOWN>;
> + channel = <0>;
> + voltage = <761904>;
> + };
> +
> + button at 952 {
> + label = "Home";
> + linux,code = <KEY_HOME>;
> + channel = <0>;
> + voltage = <952380>;
> + };
> +};
> +
> +&mmc0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&mmc0_pins_a>;
> + vmmc-supply = <®_vcc3v3>;
You have the regulators described in your DT, you'd better use them
instead of the one coming from sunxi-common-regulators.dtsi.
> + bus-width = <4>;
> + cd-gpios = <&pio 7 1 GPIO_ACTIVE_HIGH>; /* PH1 */
> + cd-inverted;
> + status = "okay";
> +};
> +
> +&ohci0 {
> + status = "okay";
> +};
> +
> +&ohci1 {
> + status = "okay";
> +};
> +
> +&otg_sram {
> + status = "okay";
> +};
> +
> +&pio {
> + panel_power_pin: panel_power_pin at 0 {
> + pins = "PH8";
> + function = "gpio_out";
> + };
> +};
You don't need that pinctrl node.
Maxime
--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180507/19ecef0e/attachment.sig>
^ permalink raw reply
* [PATCH v9 06/11] arm64: kexec_file: allow for loading Image-format kernel
From: AKASHI Takahiro @ 2018-05-07 7:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <c604b578-6667-3532-eac9-55c428001025@arm.com>
James,
On Tue, May 01, 2018 at 06:46:11PM +0100, James Morse wrote:
> Hi Akashi,
>
> On 25/04/18 07:26, AKASHI Takahiro wrote:
> > This patch provides kexec_file_ops for "Image"-format kernel. In this
> > implementation, a binary is always loaded with a fixed offset identified
> > in text_offset field of its header.
>
>
> > diff --git a/arch/arm64/include/asm/kexec.h b/arch/arm64/include/asm/kexec.h
> > index e4de1223715f..3cba4161818a 100644
> > --- a/arch/arm64/include/asm/kexec.h
> > +++ b/arch/arm64/include/asm/kexec.h
> > @@ -102,6 +102,56 @@ struct kimage_arch {
> > void *dtb_buf;
> > };
> >
> > +/**
> > + * struct arm64_image_header - arm64 kernel image header
> > + *
> > + * @pe_sig: Optional PE format 'MZ' signature
> > + * @branch_code: Instruction to branch to stext
> > + * @text_offset: Image load offset, little endian
> > + * @image_size: Effective image size, little endian
> > + * @flags:
> > + * Bit 0: Kernel endianness. 0=little endian, 1=big endian
>
> Page size? What about 'phys_base'?, (whatever that is...)
> Probably best to refer to Documentation/arm64/booting.txt here, its the
> authoritative source of what these fields mean.
While we don't care other bit fields for now, I will add the reference
to the Documentation file.
>
> > + * @reserved: Reserved
> > + * @magic: Magic number, "ARM\x64"
> > + * @pe_header: Optional offset to a PE format header
> > + **/
> > +
> > +struct arm64_image_header {
> > + u8 pe_sig[2];
> > + u8 pad[2];
> > + u32 branch_code;
> > + u64 text_offset;
> > + u64 image_size;
> > + u64 flags;
>
> __le64 as appropriate here would let tools like sparse catch any missing endian
> conversion bugs.
OK.
>
> > + u64 reserved[3];
> > + u8 magic[4];
> > + u32 pe_header;
> > +};
>
> I'm surprised we don't have a definition for this already, I guess its always
> done in asm. We have kernel/image.h that holds some of this stuff, if we are
> going to validate the flags, is it worth adding the code there, (and moving it
> to include/asm)?
A comment at the beginning of this file says,
#ifndef LINKER_SCRIPT
#error This file should only be included in vmlinux.lds.S
#endif
Let me think about.
>
> > +static const u8 arm64_image_magic[4] = {'A', 'R', 'M', 0x64U};
>
> Any chance this magic could be a pre-processor symbol shared with head.S?
OK.
>
> > +
> > +/**
> > + * arm64_header_check_magic - Helper to check the arm64 image header.
> > + *
> > + * Returns non-zero if header is OK.
> > + */
> > +
> > +static inline int arm64_header_check_magic(const struct arm64_image_header *h)
> > +{
> > + if (!h)
> > + return 0;
> > +
> > + if (!h->text_offset)
> > + return 0;
> > +
> > + return (h->magic[0] == arm64_image_magic[0]
> > + && h->magic[1] == arm64_image_magic[1]
> > + && h->magic[2] == arm64_image_magic[2]
> > + && h->magic[3] == arm64_image_magic[3]);
>
> memcmp()? Or just define it as a 32bit value?
OK. As you know, I always tried to keep the code not diverted
from kexec-tools for maintainability reason.
> I guess you skip the MZ prefix as its not present for !EFI?
CONFIG_KEXEC_IMAGE_VERIFY_SIG depends on the fact that the file
format is PE (that is, EFI is enabled).
> Could we check branch_code is non-zero, and text-offset points within image-size?
We could do it, but I don't think this check is very useful.
>
> We could check that this platform supports the page-size/endian config that this
> Image was built with... We get a message from the EFI stub if the page-size
> can't be supported, it would be nice to do the same here (as we can).
There is no restriction on page-size or endianness for kexec.
What will be the purpose of this check?
> (no idea if kexec-tool checks this stuff, it probably can't get at the id
> registers to know)
>
>
> > diff --git a/arch/arm64/kernel/kexec_image.c b/arch/arm64/kernel/kexec_image.c
> > new file mode 100644
> > index 000000000000..4dd524ad6611
> > --- /dev/null
> > +++ b/arch/arm64/kernel/kexec_image.c
> > @@ -0,0 +1,79 @@
>
> > +static void *image_load(struct kimage *image,
> > + char *kernel, unsigned long kernel_len,
> > + char *initrd, unsigned long initrd_len,
> > + char *cmdline, unsigned long cmdline_len)
> > +{
> > + struct kexec_buf kbuf;
> > + struct arm64_image_header *h = (struct arm64_image_header *)kernel;
> > + unsigned long text_offset;
> > + int ret;
> > +
> > + /* Load the kernel */
> > + kbuf.image = image;
> > + kbuf.buf_min = 0;
> > + kbuf.buf_max = ULONG_MAX;
> > + kbuf.top_down = false;
> > +
> > + kbuf.buffer = kernel;
> > + kbuf.bufsz = kernel_len;
> > + kbuf.memsz = le64_to_cpu(h->image_size);
> > + text_offset = le64_to_cpu(h->text_offset);
> > + kbuf.buf_align = SZ_2M;
>
> > + /* Adjust kernel segment with TEXT_OFFSET */
> > + kbuf.memsz += text_offset;
> > +
> > + ret = kexec_add_buffer(&kbuf);
> > + if (ret)
> > + goto out;
> > +
> > + image->arch.kern_segment = image->nr_segments - 1;
>
> You only seem to use kern_segment here, and in load_other_segments() called
> below. Could it not be a local variable passed in? Instead of arch-specific data
> we keep forever?
No, kern_segment is also used in load_other_segments() in machine_kexec_file.c.
To optimize memory hole allocation logic in locate_mem_hole_callback(),
we need to know the exact range of kernel image (start and end).
(Known drawback in this code is that Image only occupies one segment, but
once vmlinux might be supported, it would occupy two segments for text and
data.)
>
> > + image->segment[image->arch.kern_segment].mem += text_offset;
> > + image->segment[image->arch.kern_segment].memsz -= text_offset;
> > + image->start = image->segment[image->arch.kern_segment].mem;
> > +
> > + pr_debug("Loaded kernel at 0x%lx bufsz=0x%lx memsz=0x%lx\n",
> > + image->segment[image->arch.kern_segment].mem,
> > + kbuf.bufsz, kbuf.memsz);
> > +
> > + /* Load additional data */
> > + ret = load_other_segments(image, initrd, initrd_len,
> > + cmdline, cmdline_len);
> > +
> > +out:
> > + return ERR_PTR(ret);
> > +}
> Looks good,
Thank you for thorough review.
-Takahiro AKASHI
> Thanks,
>
> James
^ permalink raw reply
* [PATCH 7/7] ARM: dts: sun7i: Add dts file for the A20-linova1-7 HMI
From: Maxime Ripard @ 2018-05-07 7:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <afc78821-2d08-3f4f-d008-5b2d216e96c0@micronovasrl.com>
On Fri, May 04, 2018 at 11:52:59PM +0200, Giulio Benetti wrote:
> Hi Maxime!
>
> Il 04/05/2018 10:06, Maxime Ripard ha scritto:
> > Hi,
> >
> > On Wed, May 02, 2018 at 06:41:34PM +0200, Giulio Benetti wrote:
> > > > > You don't have to handcode the fragments anymore with the new syntax,
> > > > > and U-Boot makes it really trivial to use if you use the FIT image
> > > > > format to have multiple overlays bundled in the same image. You can
> > > > > choose to apply them dynamically, for example based on an EEPROM or
> > > > > some other metric to see which combination you have.
> > > >
> > > > Ah, this is interesting. I'm going to experiment with that.
> > > >
> > >
> > > I'm struggling against this, I don't really know how to proceed,
> > > except keeping monolithic dts files including other dtsi files.
> > >
> > > About dt-overlays I've tried to look around lot of time,
> > > but the only thing I've found is this:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/tree/arch/arm/boot/dts?h=topic/renesas-overlays
> > >
> > > where they use .dtso tagging them as "/plugin/;"
> > > and compile all .dtso found in dts folder.
> > > Then they obtain .dtbo files that should be the dt-overlays we have spoken
> > > about right?
> >
> > Yes. You don't have to do that though, you can just rely on dtc to
> > compile them, outside of the linux build system.
> >
> > > What I can't understand is if there's a real standard at this time to
> > > follow, because on renesas-driver they use their way to handle all .dtso
> > > files, but on mainline there seems to be nothing about that.
> >
> > I'm not sure what you mean here. It's just fragments of device tree,
> > that have to be compiled using dtc, that's it. You can use the Linux
> > build system infrastructure to do that, or you can build your own
> > simpler one. That's really up to you. See for example
> > https://github.com/NextThingCo/CHIP-dt-overlays/blob/master/Makefile
> >
> > (even though the overlays themselves use the legacy syntax and
> > shouldn't really be used an examples)
>
> Everything works now!
> Thank you very much!
> I've setted up a Repo on Github to give an example on how make it work with
> no pain:
> https://github.com/micronovasrl/linova-dtoverlays
>
> At the moment it's a mess all around, but it's working and give an idea on
> how to make it work. Though I'm going to clean it up well as a base for
> linova dtoverlays.
>
> Ah, btw, can you confirm me that base dts file must be compiled outside
> kernel with:
> dtc -@ ....
> Otherwise as in-tree dts with make dtbs "-@" argument is not passed.
> Right?
You should use DTC_FLAGS='-@'
Maxime
--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180507/73b6662a/attachment.sig>
^ permalink raw reply
* [PATCH] arm: Convert arm boot_lock to raw
From: Sebastian Andrzej Siewior @ 2018-05-07 7:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180506182325.w7yvrpp3hcii54pj@kozik-lap>
On 2018-05-06 20:23:25 [+0200], Krzysztof Kozlowski wrote:
> > arch/arm/mach-actions/platsmp.c | 6 +++---
> > arch/arm/mach-exynos/platsmp.c | 12 ++++++------
>
> I assume this is needed for -rt patches?
correct.
> For Exynos:
> Acked-by: Krzysztof Kozlowski <krzk@kernel.org>
> Tested on Exynos5422 (Odroid HC1) with Linaro PM-QA:
> Tested-by: Krzysztof Kozlowski <krzk@kernel.org>
thank you.
> Best regards,
> Krzysztof
Sebastian
^ permalink raw reply
* [PATCH] arm: oxnas: remove non-necessary flush_cache_all()
From: Jisheng Zhang @ 2018-05-07 7:42 UTC (permalink / raw)
To: linux-arm-kernel
I believe the flush_cache_all() after scu_enable() is to "Ensure that
the data accessed by CPU0 before the SCU was initialised is visible
to the other CPUs." as commented in scu_enable(). So flush_cache_all()
here is a duplication of the one in scu_enable(), remove it.
Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
---
arch/arm/mach-oxnas/platsmp.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/arm/mach-oxnas/platsmp.c b/arch/arm/mach-oxnas/platsmp.c
index 442cc8a2f7dc..1a8e1e9fc222 100644
--- a/arch/arm/mach-oxnas/platsmp.c
+++ b/arch/arm/mach-oxnas/platsmp.c
@@ -85,7 +85,6 @@ static void __init ox820_smp_prepare_cpus(unsigned int max_cpus)
goto unmap_scu;
scu_enable(scu_base);
- flush_cache_all();
unmap_scu:
iounmap(scu_base);
--
2.17.0
^ permalink raw reply related
* [PATCH] arm: berlin: remove non-necessary flush_cache_all()
From: Jisheng Zhang @ 2018-05-07 7:44 UTC (permalink / raw)
To: linux-arm-kernel
I believe the flush_cache_all() after scu_enable() is to "Ensure that
the data accessed by CPU0 before the SCU was initialised is visible
to the other CPUs." as commented in scu_enable(). So flush_cache_all()
here is a duplication of the one in scu_enable(), remove it.
Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
---
arch/arm/mach-berlin/platsmp.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/arm/mach-berlin/platsmp.c b/arch/arm/mach-berlin/platsmp.c
index 7586b7aec272..a8ae4a566d99 100644
--- a/arch/arm/mach-berlin/platsmp.c
+++ b/arch/arm/mach-berlin/platsmp.c
@@ -81,7 +81,6 @@ static void __init berlin_smp_prepare_cpus(unsigned int max_cpus)
goto unmap_scu;
scu_enable(scu_base);
- flush_cache_all();
/*
* Write the first instruction the CPU will execute after being reset
--
2.17.0
^ permalink raw reply related
* [PATCH v2] tty: implement led triggers
From: Johan Hovold @ 2018-05-07 8:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180503201952.16592-1-u.kleine-koenig@pengutronix.de>
On Thu, May 03, 2018 at 10:19:52PM +0200, Uwe Kleine-K?nig wrote:
> The rx trigger fires when data is pushed to the ldisc. This is a bit later
> than the actual receiving of data but has the nice benefit that it
> doesn't need adaption for each driver and isn't in the hot path.
>
> Similarily the tx trigger fires when data taken from the ldisc.
You meant copied from user space, or written to the ldisc, here.
Note that the rx-path is shared with serdev, but the write path is not.
So with this series, serdev devices would only trigger the rx-led.
> Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> ---
> Changes since v1, sent with Message-Id:
> 20180503100448.1350-1-u.kleine-koenig at pengutronix.de:
>
> - implement tx trigger;
> - introduce Kconfig symbol for conditional compilation;
> - set trigger to NULL if allocating the name failed to not free random
> pointers in case the port struct wasn't zeroed;
> - use if/else instead of goto
> @@ -499,6 +500,7 @@ static void flush_to_ldisc(struct work_struct *work)
> struct tty_buffer *head = buf->head;
> struct tty_buffer *next;
> int count;
> + unsigned long delay = 50 /* ms */;
Comment after the semicolon?
>
> /* Ldisc or user is trying to gain exclusive access */
> if (atomic_read(&buf->priority))
> diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
> index 25d736880013..f042879a597c 100644
> --- a/drivers/tty/tty_port.c
> +++ b/drivers/tty/tty_port.c
> @@ -16,6 +16,7 @@
> #include <linux/wait.h>
> #include <linux/bitops.h>
> #include <linux/delay.h>
> +#include <linux/leds.h>
> #include <linux/module.h>
> #include <linux/serdev.h>
>
> @@ -157,6 +158,30 @@ struct device *tty_port_register_device_attr_serdev(struct tty_port *port,
>
> tty_port_link_device(port, driver, index);
>
> +#ifdef CONFIG_TTY_LEDS_TRIGGER
> + port->led_trigger_rx_name = kasprintf(GFP_KERNEL, "%s%d-rx",
> + driver->name, index);
> + if (port->led_trigger_rx_name) {
> + led_trigger_register_simple(port->led_trigger_rx_name,
> + &port->led_trigger_rx);
> + } else {
> + port->led_trigger_rx = NULL;
> + pr_err("Failed to allocate trigger name for %s%d\n",
> + driver->name, index);
> + }
> +
> + port->led_trigger_tx_name = kasprintf(GFP_KERNEL, "%s%d-tx",
> + driver->name, index);
> + if (port->led_trigger_tx_name) {
> + led_trigger_register_simple(port->led_trigger_tx_name,
> + &port->led_trigger_tx);
> + } else {
> + port->led_trigger_tx = NULL;
> + pr_err("Failed to allocate trigger name for %s%d\n",
> + driver->name, index);
> + }
> +#endif
Besides the ugly ifdefs here, you're leaking the above LED resources in
the error paths below (e.g. on probe deferrals).
> dev = serdev_tty_port_register(port, device, driver, index);
> if (PTR_ERR(dev) != -ENODEV) {
> /* Skip creating cdev if we registered a serdev device */
> @@ -249,6 +249,13 @@ struct tty_port {
> set to size of fifo */
> struct kref kref; /* Ref counter */
> void *client_data;
> +
> +#ifdef CONFIG_TTY_LEDS_TRIGGER
> + struct led_trigger *led_trigger_rx;
> + char *led_trigger_rx_name;
const?
> + struct led_trigger *led_trigger_tx;
> + char *led_trigger_tx_name;
ditto.
> +#endif
Johan
^ permalink raw reply
* [PATCH 2/2] ARM: dts: imx28-duckbill-2-enocean: Remove unnecessary #address/#size-cells
From: Stefan Wahren @ 2018-05-07 8:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1525655020-31260-2-git-send-email-festevam@gmail.com>
Am 07.05.2018 um 03:03 schrieb Fabio Estevam:
> From: Fabio Estevam <fabio.estevam@nxp.com>
>
> Remove unnecessary #address-cells/#size-cells to fix the following
> DTC warnings:
>
> arch/arm/boot/dts/imx28-duckbill-2-enocean.dtb: Warning (avoid_unnecessary_addr_size): /gpio-keys: unnecessary #address-cells/#size-cells without "ranges" or child "reg" propert
>
> Cc: Stefan Wahren <stefan.wahren@i2se.com>
> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
Acked-by: Stefan Wahren <stefan.wahren@i2se.com>
Thanks
Stefan
^ permalink raw reply
* [PATCH v2] tty: implement led triggers
From: Uwe Kleine-König @ 2018-05-07 8:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180507080252.GO2285@localhost>
Hello Johan,
thanks for your feedback.
On Mon, May 07, 2018 at 10:02:52AM +0200, Johan Hovold wrote:
> On Thu, May 03, 2018 at 10:19:52PM +0200, Uwe Kleine-K?nig wrote:
> > The rx trigger fires when data is pushed to the ldisc. This is a bit later
> > than the actual receiving of data but has the nice benefit that it
> > doesn't need adaption for each driver and isn't in the hot path.
> >
> > Similarily the tx trigger fires when data taken from the ldisc.
>
> You meant copied from user space, or written to the ldisc, here.
ack.
> Note that the rx-path is shared with serdev, but the write path is not.
> So with this series, serdev devices would only trigger the rx-led.
Where would be the right place to put the tx trigger to catch serdev,
too?
> > Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> > ---
> > Changes since v1, sent with Message-Id:
> > 20180503100448.1350-1-u.kleine-koenig at pengutronix.de:
> >
> > - implement tx trigger;
> > - introduce Kconfig symbol for conditional compilation;
> > - set trigger to NULL if allocating the name failed to not free random
> > pointers in case the port struct wasn't zeroed;
> > - use if/else instead of goto
>
> > @@ -499,6 +500,7 @@ static void flush_to_ldisc(struct work_struct *work)
> > struct tty_buffer *head = buf->head;
> > struct tty_buffer *next;
> > int count;
> > + unsigned long delay = 50 /* ms */;
>
> Comment after the semicolon?
Given that this comment is about the 50 and not the delay member, I
prefer it before the ;.
> >
> > /* Ldisc or user is trying to gain exclusive access */
> > if (atomic_read(&buf->priority))
>
> > diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
> > index 25d736880013..f042879a597c 100644
> > --- a/drivers/tty/tty_port.c
> > +++ b/drivers/tty/tty_port.c
> > @@ -16,6 +16,7 @@
> > #include <linux/wait.h>
> > #include <linux/bitops.h>
> > #include <linux/delay.h>
> > +#include <linux/leds.h>
> > #include <linux/module.h>
> > #include <linux/serdev.h>
> >
> > @@ -157,6 +158,30 @@ struct device *tty_port_register_device_attr_serdev(struct tty_port *port,
> >
> > tty_port_link_device(port, driver, index);
> >
> > +#ifdef CONFIG_TTY_LEDS_TRIGGER
> > + port->led_trigger_rx_name = kasprintf(GFP_KERNEL, "%s%d-rx",
> > + driver->name, index);
> > + if (port->led_trigger_rx_name) {
> > + led_trigger_register_simple(port->led_trigger_rx_name,
> > + &port->led_trigger_rx);
> > + } else {
> > + port->led_trigger_rx = NULL;
> > + pr_err("Failed to allocate trigger name for %s%d\n",
> > + driver->name, index);
> > + }
> > +
> > + port->led_trigger_tx_name = kasprintf(GFP_KERNEL, "%s%d-tx",
> > + driver->name, index);
> > + if (port->led_trigger_tx_name) {
> > + led_trigger_register_simple(port->led_trigger_tx_name,
> > + &port->led_trigger_tx);
> > + } else {
> > + port->led_trigger_tx = NULL;
> > + pr_err("Failed to allocate trigger name for %s%d\n",
> > + driver->name, index);
> > + }
> > +#endif
>
> Besides the ugly ifdefs here, you're leaking the above LED resources in
> the error paths below (e.g. on probe deferrals).
ack, the ifdevs are ugly. I'm working on an idea to get rid of them.
Will think about how to plug the leak.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* [PATCH v2] tty: implement led triggers
From: Johan Hovold @ 2018-05-07 9:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180507084127.ekpd3ze2itkzo7fd@pengutronix.de>
On Mon, May 07, 2018 at 10:41:27AM +0200, Uwe Kleine-K?nig wrote:
> Hello Johan,
>
> thanks for your feedback.
>
> On Mon, May 07, 2018 at 10:02:52AM +0200, Johan Hovold wrote:
> > On Thu, May 03, 2018 at 10:19:52PM +0200, Uwe Kleine-K?nig wrote:
> > > The rx trigger fires when data is pushed to the ldisc. This is a bit later
> > > than the actual receiving of data but has the nice benefit that it
> > > doesn't need adaption for each driver and isn't in the hot path.
> > >
> > > Similarily the tx trigger fires when data taken from the ldisc.
> >
> > You meant copied from user space, or written to the ldisc, here.
>
> ack.
>
> > Note that the rx-path is shared with serdev, but the write path is not.
> > So with this series, serdev devices would only trigger the rx-led.
>
> Where would be the right place to put the tx trigger to catch serdev,
> too?
I haven't given this much thought, but do we really want this for serdev
at all? I'm thinking whatever driver or subsystem is using serdev should
have their own triggers (e.g. bluetooth or net).
So it might be better to move the rx-blinking to the default tty-port
client receive_buf callback instead (i.e.
tty_port_default_receive_buf()).
And then the resource allocations would need to go after the serdev
registration in tty_port_register_device_attr_serdev().
> > > Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> > > ---
> > > Changes since v1, sent with Message-Id:
> > > 20180503100448.1350-1-u.kleine-koenig at pengutronix.de:
> > >
> > > - implement tx trigger;
> > > - introduce Kconfig symbol for conditional compilation;
> > > - set trigger to NULL if allocating the name failed to not free random
> > > pointers in case the port struct wasn't zeroed;
> > > - use if/else instead of goto
> >
> > > @@ -499,6 +500,7 @@ static void flush_to_ldisc(struct work_struct *work)
> > > struct tty_buffer *head = buf->head;
> > > struct tty_buffer *next;
> > > int count;
> > > + unsigned long delay = 50 /* ms */;
> >
> > Comment after the semicolon?
>
> Given that this comment is about the 50 and not the delay member, I
> prefer it before the ;.
Hmm. I personally find it hard to read and can only find about 30
instances of this comment style (for assignments) in the kernel. And
arguably the comment applies equally well to the delay variable in this
case too.
> > Besides the ugly ifdefs here, you're leaking the above LED resources in
> > the error paths below (e.g. on probe deferrals).
> ack, the ifdevs are ugly. I'm working on an idea to get rid of them.
Sounds good.
Thanks,
Johan
^ permalink raw reply
* [PATCH v2 03/13] drm/kms/mode/exynos-dsi: using helper func drm_display_mode_to_videomode for calculating timing parameters
From: Andrzej Hajda @ 2018-05-07 9:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1525663938-4172-1-git-send-email-satendra.t@samsung.com>
Hi Satendra Singh Thakur,
On 07.05.2018 05:32, Satendra Singh Thakur wrote:
> To avoid duplicate logic for the same
>
> Signed-off-by: Satendra Singh Thakur <satendra.t@samsung.com>
> Acked-by: Madhur Verma <madhur.verma@samsung.com>
> Cc: Hemanshu Srivastava <hemanshu.s@samsung.com>
Whole exynos_dsi_mode_set callback is redundant, so I have posted patch
removing it [1], so this patch can be dropped.
[1]: https://marc.info/?l=dri-devel&m=152568538400712
Regards
Andrzej
> ---
>
> v2: Removed Mr Robin from reviewed-by field
>
> drivers/gpu/drm/exynos/exynos_drm_dsi.c | 13 ++-----------
> 1 file changed, 2 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> index 7904ffa..7fe84fd 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> @@ -1490,17 +1490,8 @@ static void exynos_dsi_mode_set(struct drm_encoder *encoder,
> struct drm_display_mode *adjusted_mode)
> {
> struct exynos_dsi *dsi = encoder_to_dsi(encoder);
> - struct videomode *vm = &dsi->vm;
> - struct drm_display_mode *m = adjusted_mode;
> -
> - vm->hactive = m->hdisplay;
> - vm->vactive = m->vdisplay;
> - vm->vfront_porch = m->vsync_start - m->vdisplay;
> - vm->vback_porch = m->vtotal - m->vsync_end;
> - vm->vsync_len = m->vsync_end - m->vsync_start;
> - vm->hfront_porch = m->hsync_start - m->hdisplay;
> - vm->hback_porch = m->htotal - m->hsync_end;
> - vm->hsync_len = m->hsync_end - m->hsync_start;
> +
> + drm_display_mode_to_videomode(adjusted_mode, &dsi->vm);
> }
>
> static const struct drm_encoder_helper_funcs exynos_dsi_encoder_helper_funcs = {
^ permalink raw reply
* [PATCH] locking/atomics: Simplify the op definitions in atomic.h some more
From: Andrea Parri @ 2018-05-07 9:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180506145726.y4jxhvfolzvbuft5@gmail.com>
On Sun, May 06, 2018 at 04:57:27PM +0200, Ingo Molnar wrote:
>
> * Andrea Parri <andrea.parri@amarulasolutions.com> wrote:
>
> > Hi Ingo,
> >
> > > From 5affbf7e91901143f84f1b2ca64f4afe70e210fd Mon Sep 17 00:00:00 2001
> > > From: Ingo Molnar <mingo@kernel.org>
> > > Date: Sat, 5 May 2018 10:23:23 +0200
> > > Subject: [PATCH] locking/atomics: Simplify the op definitions in atomic.h some more
> > >
> > > Before:
> > >
> > > #ifndef atomic_fetch_dec_relaxed
> > > # ifndef atomic_fetch_dec
> > > # define atomic_fetch_dec(v) atomic_fetch_sub(1, (v))
> > > # define atomic_fetch_dec_relaxed(v) atomic_fetch_sub_relaxed(1, (v))
> > > # define atomic_fetch_dec_acquire(v) atomic_fetch_sub_acquire(1, (v))
> > > # define atomic_fetch_dec_release(v) atomic_fetch_sub_release(1, (v))
> > > # else
> > > # define atomic_fetch_dec_relaxed atomic_fetch_dec
> > > # define atomic_fetch_dec_acquire atomic_fetch_dec
> > > # define atomic_fetch_dec_release atomic_fetch_dec
> > > # endif
> > > #else
> > > # ifndef atomic_fetch_dec_acquire
> > > # define atomic_fetch_dec_acquire(...) __atomic_op_acquire(atomic_fetch_dec, __VA_ARGS__)
> > > # endif
> > > # ifndef atomic_fetch_dec_release
> > > # define atomic_fetch_dec_release(...) __atomic_op_release(atomic_fetch_dec, __VA_ARGS__)
> > > # endif
> > > # ifndef atomic_fetch_dec
> > > # define atomic_fetch_dec(...) __atomic_op_fence(atomic_fetch_dec, __VA_ARGS__)
> > > # endif
> > > #endif
> > >
> > > After:
> > >
> > > #ifndef atomic_fetch_dec_relaxed
> > > # ifndef atomic_fetch_dec
> > > # define atomic_fetch_dec(v) atomic_fetch_sub(1, (v))
> > > # define atomic_fetch_dec_relaxed(v) atomic_fetch_sub_relaxed(1, (v))
> > > # define atomic_fetch_dec_acquire(v) atomic_fetch_sub_acquire(1, (v))
> > > # define atomic_fetch_dec_release(v) atomic_fetch_sub_release(1, (v))
> > > # else
> > > # define atomic_fetch_dec_relaxed atomic_fetch_dec
> > > # define atomic_fetch_dec_acquire atomic_fetch_dec
> > > # define atomic_fetch_dec_release atomic_fetch_dec
> > > # endif
> > > #else
> > > # ifndef atomic_fetch_dec
> > > # define atomic_fetch_dec(...) __atomic_op_fence(atomic_fetch_dec, __VA_ARGS__)
> > > # define atomic_fetch_dec_acquire(...) __atomic_op_acquire(atomic_fetch_dec, __VA_ARGS__)
> > > # define atomic_fetch_dec_release(...) __atomic_op_release(atomic_fetch_dec, __VA_ARGS__)
> > > # endif
> > > #endif
> > >
> > > The idea is that because we already group these APIs by certain defines
> > > such as atomic_fetch_dec_relaxed and atomic_fetch_dec in the primary
> > > branches - we can do the same in the secondary branch as well.
> > >
> > > ( Also remove some unnecessarily duplicate comments, as the API
> > > group defines are now pretty much self-documenting. )
> > >
> > > No change in functionality.
> > >
> > > Cc: Peter Zijlstra <peterz@infradead.org>
> > > Cc: Linus Torvalds <torvalds@linux-foundation.org>
> > > Cc: Andrew Morton <akpm@linux-foundation.org>
> > > Cc: Thomas Gleixner <tglx@linutronix.de>
> > > Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > > Cc: Will Deacon <will.deacon@arm.com>
> > > Cc: linux-kernel at vger.kernel.org
> > > Signed-off-by: Ingo Molnar <mingo@kernel.org>
> >
> > This breaks compilation on RISC-V. (For some of its atomics, the arch
> > currently defines the _relaxed and the full variants and it relies on
> > the generic definitions for the _acquire and the _release variants.)
>
> I don't have cross-compilation for RISC-V, which is a relatively new arch.
> (Is there any RISC-V set of cross-compilation tools on kernel.org somewhere?)
I'm using the toolchain from:
https://riscv.org/software-tools/
(adding Palmer and Albert in Cc:)
>
> Could you please send a patch that defines those variants against Linus's tree,
> like the PowerPC patch that does something similar:
>
> 0476a632cb3a: locking/atomics/powerpc: Move cmpxchg helpers to asm/cmpxchg.h and define the full set of cmpxchg APIs
>
> ?
Yes, please see below for a first RFC.
(BTW, get_maintainer.pl says that that patch missed Benjamin, Paul, Michael
and linuxppc-dev at lists.ozlabs.org: FWIW, I'm Cc-ing the maintainers here.)
Andrea
>From 411f05a44e0b53a435331b977ff864fba7501a95 Mon Sep 17 00:00:00 2001
From: Andrea Parri <andrea.parri@amarulasolutions.com>
Date: Mon, 7 May 2018 10:59:20 +0200
Subject: [RFC PATCH] riscv/atomic: Defines _acquire/_release variants
In preparation for Ingo's renovation of the generic atomic.h header [1],
define the _acquire/_release variants in the arch's header.
No change in code generation.
[1] http://lkml.kernel.org/r/20180505081100.nsyrqrpzq2vd27bk at gmail.com
http://lkml.kernel.org/r/20180505083635.622xmcvb42dw5xxh at gmail.com
Suggested-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Andrea Parri <andrea.parri@amarulasolutions.com>
Cc: Palmer Dabbelt <palmer@sifive.com>
Cc: Albert Ou <albert@sifive.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Boqun Feng <boqun.feng@gmail.com>
Cc: linux-riscv at lists.infradead.org
---
arch/riscv/include/asm/atomic.h | 88 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 88 insertions(+)
diff --git a/arch/riscv/include/asm/atomic.h b/arch/riscv/include/asm/atomic.h
index 855115ace98c8..7cbd8033dfb5d 100644
--- a/arch/riscv/include/asm/atomic.h
+++ b/arch/riscv/include/asm/atomic.h
@@ -153,22 +153,54 @@ ATOMIC_OPS(sub, add, +, -i)
#define atomic_add_return_relaxed atomic_add_return_relaxed
#define atomic_sub_return_relaxed atomic_sub_return_relaxed
+#define atomic_add_return_acquire(...) \
+ __atomic_op_acquire(atomic_add_return, __VA_ARGS__)
+#define atomic_sub_return_acquire(...) \
+ __atomic_op_acquire(atomic_sub_return, __VA_ARGS__)
+#define atomic_add_return_release(...) \
+ __atomic_op_release(atomic_add_return, __VA_ARGS__)
+#define atomic_sub_return_release(...) \
+ __atomic_op_release(atomic_sub_return, __VA_ARGS__)
#define atomic_add_return atomic_add_return
#define atomic_sub_return atomic_sub_return
#define atomic_fetch_add_relaxed atomic_fetch_add_relaxed
#define atomic_fetch_sub_relaxed atomic_fetch_sub_relaxed
+#define atomic_fetch_add_acquire(...) \
+ __atomic_op_acquire(atomic_fetch_add, __VA_ARGS__)
+#define atomic_fetch_sub_acquire(...) \
+ __atomic_op_acquire(atomic_fetch_sub, __VA_ARGS__)
+#define atomic_fetch_add_release(...) \
+ __atomic_op_release(atomic_fetch_add, __VA_ARGS__)
+#define atomic_fetch_sub_release(...) \
+ __atomic_op_release(atomic_fetch_sub, __VA_ARGS__)
#define atomic_fetch_add atomic_fetch_add
#define atomic_fetch_sub atomic_fetch_sub
#ifndef CONFIG_GENERIC_ATOMIC64
#define atomic64_add_return_relaxed atomic64_add_return_relaxed
#define atomic64_sub_return_relaxed atomic64_sub_return_relaxed
+#define atomic64_add_return_acquire(...) \
+ __atomic_op_acquire(atomic64_add_return, __VA_ARGS__)
+#define atomic64_sub_return_acquire(...) \
+ __atomic_op_acquire(atomic64_sub_return, __VA_ARGS__)
+#define atomic64_add_return_release(...) \
+ __atomic_op_release(atomic64_add_return, __VA_ARGS__)
+#define atomic64_sub_return_release(...) \
+ __atomic_op_release(atomic64_sub_return, __VA_ARGS__)
#define atomic64_add_return atomic64_add_return
#define atomic64_sub_return atomic64_sub_return
#define atomic64_fetch_add_relaxed atomic64_fetch_add_relaxed
#define atomic64_fetch_sub_relaxed atomic64_fetch_sub_relaxed
+#define atomic64_fetch_add_acquire(...) \
+ __atomic_op_acquire(atomic64_fetch_add, __VA_ARGS__)
+#define atomic64_fetch_sub_acquire(...) \
+ __atomic_op_acquire(atomic64_fetch_sub, __VA_ARGS__)
+#define atomic64_fetch_add_release(...) \
+ __atomic_op_release(atomic64_fetch_add, __VA_ARGS__)
+#define atomic64_fetch_sub_release(...) \
+ __atomic_op_release(atomic64_fetch_sub, __VA_ARGS__)
#define atomic64_fetch_add atomic64_fetch_add
#define atomic64_fetch_sub atomic64_fetch_sub
#endif
@@ -191,6 +223,18 @@ ATOMIC_OPS(xor, xor, i)
#define atomic_fetch_and_relaxed atomic_fetch_and_relaxed
#define atomic_fetch_or_relaxed atomic_fetch_or_relaxed
#define atomic_fetch_xor_relaxed atomic_fetch_xor_relaxed
+#define atomic_fetch_and_acquire(...) \
+ __atomic_op_acquire(atomic_fetch_and, __VA_ARGS__)
+#define atomic_fetch_or_acquire(...) \
+ __atomic_op_acquire(atomic_fetch_or, __VA_ARGS__)
+#define atomic_fetch_xor_acquire(...) \
+ __atomic_op_acquire(atomic_fetch_xor, __VA_ARGS__)
+#define atomic_fetch_and_release(...) \
+ __atomic_op_release(atomic_fetch_and, __VA_ARGS__)
+#define atomic_fetch_or_release(...) \
+ __atomic_op_release(atomic_fetch_or, __VA_ARGS__)
+#define atomic_fetch_xor_release(...) \
+ __atomic_op_release(atomic_fetch_xor, __VA_ARGS__)
#define atomic_fetch_and atomic_fetch_and
#define atomic_fetch_or atomic_fetch_or
#define atomic_fetch_xor atomic_fetch_xor
@@ -199,6 +243,18 @@ ATOMIC_OPS(xor, xor, i)
#define atomic64_fetch_and_relaxed atomic64_fetch_and_relaxed
#define atomic64_fetch_or_relaxed atomic64_fetch_or_relaxed
#define atomic64_fetch_xor_relaxed atomic64_fetch_xor_relaxed
+#define atomic64_fetch_and_acquire(...) \
+ __atomic_op_acquire(atomic64_fetch_and, __VA_ARGS__)
+#define atomic64_fetch_or_acquire(...) \
+ __atomic_op_acquire(atomic64_fetch_or, __VA_ARGS__)
+#define atomic64_fetch_xor_acquire(...) \
+ __atomic_op_acquire(atomic64_fetch_xor, __VA_ARGS__)
+#define atomic64_fetch_and_release(...) \
+ __atomic_op_release(atomic64_fetch_and, __VA_ARGS__)
+#define atomic64_fetch_or_release(...) \
+ __atomic_op_release(atomic64_fetch_or, __VA_ARGS__)
+#define atomic64_fetch_xor_release(...) \
+ __atomic_op_release(atomic64_fetch_xor, __VA_ARGS__)
#define atomic64_fetch_and atomic64_fetch_and
#define atomic64_fetch_or atomic64_fetch_or
#define atomic64_fetch_xor atomic64_fetch_xor
@@ -290,22 +346,54 @@ ATOMIC_OPS(dec, add, +, -1)
#define atomic_inc_return_relaxed atomic_inc_return_relaxed
#define atomic_dec_return_relaxed atomic_dec_return_relaxed
+#define atomic_inc_return_acquire(...) \
+ __atomic_op_acquire(atomic_inc_return, __VA_ARGS__)
+#define atomic_dec_return_acquire(...) \
+ __atomic_op_acquire(atomic_dec_return, __VA_ARGS__)
+#define atomic_inc_return_release(...) \
+ __atomic_op_release(atomic_inc_return, __VA_ARGS__)
+#define atomic_dec_return_release(...) \
+ __atomic_op_release(atomic_dec_return, __VA_ARGS__)
#define atomic_inc_return atomic_inc_return
#define atomic_dec_return atomic_dec_return
#define atomic_fetch_inc_relaxed atomic_fetch_inc_relaxed
#define atomic_fetch_dec_relaxed atomic_fetch_dec_relaxed
+#define atomic_fetch_inc_acquire(...) \
+ __atomic_op_acquire(atomic_fetch_inc, __VA_ARGS__)
+#define atomic_fetch_dec_acquire(...) \
+ __atomic_op_acquire(atomic_fetch_dec, __VA_ARGS__)
+#define atomic_fetch_inc_release(...) \
+ __atomic_op_release(atomic_fetch_inc, __VA_ARGS__)
+#define atomic_fetch_dec_release(...) \
+ __atomic_op_release(atomic_fetch_dec, __VA_ARGS__)
#define atomic_fetch_inc atomic_fetch_inc
#define atomic_fetch_dec atomic_fetch_dec
#ifndef CONFIG_GENERIC_ATOMIC64
#define atomic64_inc_return_relaxed atomic64_inc_return_relaxed
#define atomic64_dec_return_relaxed atomic64_dec_return_relaxed
+#define atomic64_inc_return_acquire(...) \
+ __atomic_op_acquire(atomic64_inc_return, __VA_ARGS__)
+#define atomic64_dec_return_acquire(...) \
+ __atomic_op_acquire(atomic64_dec_return, __VA_ARGS__)
+#define atomic64_inc_return_release(...) \
+ __atomic_op_release(atomic64_inc_return, __VA_ARGS__)
+#define atomic64_dec_return_release(...) \
+ __atomic_op_release(atomic64_dec_return, __VA_ARGS__)
#define atomic64_inc_return atomic64_inc_return
#define atomic64_dec_return atomic64_dec_return
#define atomic64_fetch_inc_relaxed atomic64_fetch_inc_relaxed
#define atomic64_fetch_dec_relaxed atomic64_fetch_dec_relaxed
+#define atomic64_fetch_inc_acquire(...) \
+ __atomic_op_acquire(atomic64_fetch_inc, __VA_ARGS__)
+#define atomic64_fetch_dec_acquire(...) \
+ __atomic_op_acquire(atomic64_fetch_dec, __VA_ARGS__)
+#define atomic64_fetch_inc_release(...) \
+ __atomic_op_release(atomic64_fetch_inc, __VA_ARGS__)
+#define atomic64_fetch_dec_release(...) \
+ __atomic_op_release(atomic64_fetch_dec, __VA_ARGS__)
#define atomic64_fetch_inc atomic64_fetch_inc
#define atomic64_fetch_dec atomic64_fetch_dec
#endif
--
2.7.4
>
> ... and I'll integrate it into the proper place to make it all bisectable, etc.
>
> Thanks,
>
> Ingo
^ permalink raw reply related
* [PATCH] tee: shm: fix use-after-free via temporarily dropped reference
From: Jens Wiklander @ 2018-05-07 9:58 UTC (permalink / raw)
To: linux-arm-kernel
From: Jann Horn <jannh@google.com>
Bump the file's refcount before moving the reference into the fd table,
not afterwards. The old code could drop the file's refcount to zero for a
short moment before calling get_file() via get_dma_buf().
This code can only be triggered on ARM systems that use Linaro's OP-TEE.
Fixes: 967c9cca2cc5 ("tee: generic TEE subsystem")
Signed-off-by: Jann Horn <jannh@google.com>
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
---
drivers/tee/tee_shm.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
index 556960a1bab3..07d3be6f0780 100644
--- a/drivers/tee/tee_shm.c
+++ b/drivers/tee/tee_shm.c
@@ -360,9 +360,10 @@ int tee_shm_get_fd(struct tee_shm *shm)
if (!(shm->flags & TEE_SHM_DMA_BUF))
return -EINVAL;
+ get_dma_buf(shm->dmabuf);
fd = dma_buf_fd(shm->dmabuf, O_CLOEXEC);
- if (fd >= 0)
- get_dma_buf(shm->dmabuf);
+ if (fd < 0)
+ dma_buf_put(shm->dmabuf);
return fd;
}
--
2.17.0
^ permalink raw reply related
* [PATCH v4 5/9] iio: adc: at91-sama5d2_adc: add support for position and pressure channels
From: Alexandre Belloni @ 2018-05-07 10:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <eaeced9e-390f-0eea-0466-66c789bacdd6@microchip.com>
On 07/05/2018 09:18:39+0300, Eugen Hristev wrote:
> On 06.05.2018 20:59, Alexandre Belloni wrote:
> > Hi,
> >
> > On 06/05/2018 18:29:53+0100, Jonathan Cameron wrote:
> > > On Mon, 30 Apr 2018 13:32:11 +0300
> > > Eugen Hristev <eugen.hristev@microchip.com> wrote:
> > >
> > > > This implements the support for position and pressure for the included
> > > > touchscreen support in the SAMA5D2 SOC ADC block.
> > > > Two position channels are added and one for pressure.
> > > > They can be read in raw format, or through a buffer.
> > > > A normal use case is for a consumer driver to register a callback buffer
> > > > for these channels.
> > > > When the touchscreen channels are in the active scan mask,
> > > > the driver will start the touchscreen sampling and push the data to the
> > > > buffer.
> > > >
> > > > Some parts of this patch are based on initial original work by
> > > > Mohamed Jamsheeth Hajanajubudeen and Bandaru Venkateswara Swamy
> > > >
> > > > Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
> > > Looks good to me now.
> > >
> > > I'm assuming that once Dmitry and others are happy, I'll take the
> > > series through the IIO tree. Will reply to the cover letter if the
> > > rest of the patches look good to me to let everyone know that without
> > > having to catch this comment down in here!
> > >
> >
> > I'm planning to take both DT patches through the at91 tree once you take
> > the DT bindings patches.
>
> Please take into consideration that those DT patches do not build
> stand-alone, they depend on
> [PATCH v4 7/9] dt-bindings: iio: adc: at91-sama5d2_adc: add channel specific
> consumer info
>
> (the DT patches add an include statement of a file which is created in this
> patch).
>
So the proper way is to actually have the values in the dt instead of
the define and then patch it on the next version of the kernel.
Or we take the dts patches on the next version.
--
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* dma-debug cleanups, including removing the arch hook
From: Christoph Hellwig @ 2018-05-07 10:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180424140235.9125-1-hch@lst.de>
Any comments? I'd like to move forward with this rather sooner
than later, so any reviews welcome!
On Tue, Apr 24, 2018 at 04:02:32PM +0200, Christoph Hellwig wrote:
> Hi all,
>
> this series has a few dma-debug cleanups, most notably removing the need
> for architectures to explicitly initialize dma-debug.
> _______________________________________________
> iommu mailing list
> iommu at lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
---end quoted text---
^ permalink raw reply
* [PATCH v4 5/9] iio: adc: at91-sama5d2_adc: add support for position and pressure channels
From: Eugen Hristev @ 2018-05-07 10:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180507102810.GO10960@piout.net>
On 07.05.2018 13:28, Alexandre Belloni wrote:
> On 07/05/2018 09:18:39+0300, Eugen Hristev wrote:
>> On 06.05.2018 20:59, Alexandre Belloni wrote:
>>> Hi,
>>>
>>> On 06/05/2018 18:29:53+0100, Jonathan Cameron wrote:
>>>> On Mon, 30 Apr 2018 13:32:11 +0300
>>>> Eugen Hristev <eugen.hristev@microchip.com> wrote:
>>>>
>>>>> This implements the support for position and pressure for the included
>>>>> touchscreen support in the SAMA5D2 SOC ADC block.
>>>>> Two position channels are added and one for pressure.
>>>>> They can be read in raw format, or through a buffer.
>>>>> A normal use case is for a consumer driver to register a callback buffer
>>>>> for these channels.
>>>>> When the touchscreen channels are in the active scan mask,
>>>>> the driver will start the touchscreen sampling and push the data to the
>>>>> buffer.
>>>>>
>>>>> Some parts of this patch are based on initial original work by
>>>>> Mohamed Jamsheeth Hajanajubudeen and Bandaru Venkateswara Swamy
>>>>>
>>>>> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
>>>> Looks good to me now.
>>>>
>>>> I'm assuming that once Dmitry and others are happy, I'll take the
>>>> series through the IIO tree. Will reply to the cover letter if the
>>>> rest of the patches look good to me to let everyone know that without
>>>> having to catch this comment down in here!
>>>>
>>>
>>> I'm planning to take both DT patches through the at91 tree once you take
>>> the DT bindings patches.
>>
>> Please take into consideration that those DT patches do not build
>> stand-alone, they depend on
>> [PATCH v4 7/9] dt-bindings: iio: adc: at91-sama5d2_adc: add channel specific
>> consumer info
>>
>> (the DT patches add an include statement of a file which is created in this
>> patch).
>>
>
> So the proper way is to actually have the values in the dt instead of
> the define and then patch it on the next version of the kernel.
>
> Or we take the dts patches on the next version.
Taking the DTS patches on the next version is fine for me.
Thanks
>
^ permalink raw reply
* [PATCH 1/4] drm/rockchip: add transfer function for cdn-dp
From: Enric Balletbo Serra @ 2018-05-07 11:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1525421338-1021-1-git-send-email-hl@rock-chips.com>
Hi Lin,
I am interested in these patches, could you cc me on newer versions? Thanks.
Some comments below.
2018-05-04 10:08 GMT+02:00 Lin Huang <hl@rock-chips.com>:
> From: Chris Zhong <zyw@rock-chips.com>
>
> We may support training outside firmware, so we need support
> dpcd read/write to get the message or do some setting with
> display.
>
> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> Signed-off-by: Lin Huang <hl@rock-chips.com>
> ---
> drivers/gpu/drm/rockchip/cdn-dp-core.c | 55 ++++++++++++++++++++++++----
> drivers/gpu/drm/rockchip/cdn-dp-core.h | 1 +
> drivers/gpu/drm/rockchip/cdn-dp-reg.c | 66 +++++++++++++++++++++++++++++-----
> drivers/gpu/drm/rockchip/cdn-dp-reg.h | 14 ++++++--
> 4 files changed, 119 insertions(+), 17 deletions(-)
>
In general, for this patch and all the other patches in the series I
saw that checkpatch spits out some warnings, could you fix these and
ideally run checkpatch with the --strict --subjective option?
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> index c6fbdcd..268c190 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> @@ -176,8 +176,8 @@ static int cdn_dp_get_sink_count(struct cdn_dp_device *dp, u8 *sink_count)
> u8 value;
>
> *sink_count = 0;
> - ret = cdn_dp_dpcd_read(dp, DP_SINK_COUNT, &value, 1);
> - if (ret)
> + ret = drm_dp_dpcd_read(&dp->aux, DP_SINK_COUNT, &value, 1);
> + if (ret < 0)
> return ret;
>
> *sink_count = DP_GET_SINK_COUNT(value);
> @@ -374,9 +374,9 @@ static int cdn_dp_get_sink_capability(struct cdn_dp_device *dp)
> if (!cdn_dp_check_sink_connection(dp))
> return -ENODEV;
>
> - ret = cdn_dp_dpcd_read(dp, DP_DPCD_REV, dp->dpcd,
> - DP_RECEIVER_CAP_SIZE);
> - if (ret) {
> + ret = drm_dp_dpcd_read(&dp->aux, DP_DPCD_REV, dp->dpcd,
> + sizeof(dp->dpcd));
> + if (ret < 0) {
> DRM_DEV_ERROR(dp->dev, "Failed to get caps %d\n", ret);
> return ret;
> }
> @@ -582,8 +582,8 @@ static bool cdn_dp_check_link_status(struct cdn_dp_device *dp)
> if (!port || !dp->link.rate || !dp->link.num_lanes)
> return false;
>
> - if (cdn_dp_dpcd_read(dp, DP_LANE0_1_STATUS, link_status,
> - DP_LINK_STATUS_SIZE)) {
> + if (drm_dp_dpcd_read_link_status(&dp->aux, link_status) !=
> + DP_LINK_STATUS_SIZE) {
> DRM_ERROR("Failed to get link status\n");
> return false;
> }
> @@ -1012,6 +1012,40 @@ static int cdn_dp_pd_event(struct notifier_block *nb,
> return NOTIFY_DONE;
> }
>
> +static ssize_t cdn_dp_aux_transfer(struct drm_dp_aux *aux,
> + struct drm_dp_aux_msg *msg)
> +{
> + struct cdn_dp_device *dp = container_of(aux, struct cdn_dp_device, aux);
> + int ret;
> + u8 status;
> +
> + switch (msg->request & ~DP_AUX_I2C_MOT) {
> + case DP_AUX_NATIVE_WRITE:
> + case DP_AUX_I2C_WRITE:
> + case DP_AUX_I2C_WRITE_STATUS_UPDATE:
> + ret = cdn_dp_dpcd_write(dp, msg->address, msg->buffer,
> + msg->size);
> + break;
> + case DP_AUX_NATIVE_READ:
> + case DP_AUX_I2C_READ:
> + ret = cdn_dp_dpcd_read(dp, msg->address, msg->buffer,
> + msg->size);
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + status = cdn_dp_get_aux_status(dp);
> + if (status == AUX_STAUS_ACK)
> + msg->reply = DP_AUX_NATIVE_REPLY_ACK;
> + else if (status == AUX_STAUS_NACK)
> + msg->reply = DP_AUX_NATIVE_REPLY_NACK;
> + else if (status == AUX_STAUS_DEFER)
> + msg->reply = DP_AUX_NATIVE_REPLY_DEFER;
> +
I think that you would mean STATUS instead of STAUS on these defines.
What happens if the status is AUX_STATUS_SINK_ERROR or AUX_STATUS_BUS_ERROR?
> + return ret;
> +}
> +
> static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
> {
> struct cdn_dp_device *dp = dev_get_drvdata(dev);
> @@ -1030,6 +1064,13 @@ static int cdn_dp_bind(struct device *dev, struct device *master, void *data)
> dp->active = false;
> dp->active_port = -1;
> dp->fw_loaded = false;
> + dp->aux.name = "DP-AUX";
> + dp->aux.transfer = cdn_dp_aux_transfer;
> + dp->aux.dev = dev;
> +
> + ret = drm_dp_aux_register(&dp->aux);
> + if (ret)
> + return ret;
>
> INIT_WORK(&dp->event_work, cdn_dp_pd_event_work);
>
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.h b/drivers/gpu/drm/rockchip/cdn-dp-core.h
> index f57e296..46159b2 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.h
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.h
> @@ -78,6 +78,7 @@ struct cdn_dp_device {
> struct platform_device *audio_pdev;
> struct work_struct event_work;
> struct edid *edid;
> + struct drm_dp_aux aux;
>
> struct mutex lock;
> bool connected;
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-reg.c b/drivers/gpu/drm/rockchip/cdn-dp-reg.c
> index eb3042c..b2f532a 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-reg.c
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-reg.c
> @@ -221,7 +221,11 @@ static int cdn_dp_reg_write_bit(struct cdn_dp_device *dp, u16 addr,
> sizeof(field), field);
> }
>
> -int cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len)
> +/*
> + * Returns the number of bytes transferred on success, or a negative error
> + * code on failure. -ETIMEDOUT is returned if mailbox message not send success;
> + */
Returns the number of bytes or -ETIMEDOUT, no other negative errors
can be returned, right?
I am not English native but the last phrase sounds incorrect to me,
I'd rephrase it: (open to suggestions)
-ETIMEDOUT if fails to receive the mailbox message.
> +ssize_t cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len)
> {
> u8 msg[5], reg[5];
> int ret;
> @@ -247,24 +251,40 @@ int cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len)
> goto err_dpcd_read;
>
> ret = cdn_dp_mailbox_read_receive(dp, data, len);
> + if (!ret)
> + return len;
>
> err_dpcd_read:
> + DRM_DEV_ERROR(dp->dev, "dpcd read failed: %d\n", ret);
> return ret;
> }
>
> -int cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 value)
> +#define CDN_AUX_HEADER_SIZE 5
> +#define CDN_AUX_MSG_SIZE 20
> +/*
> + * Returns the number of bytes transferred on success, or a negative error
> + * code on failure. -ETIMEDOUT is returned if mailbox message not send success;
Same as above. Sounds incorrect to me.
> + * -EINVAL is return if get the wrong data size after message send.
Same here.
> + */
> +ssize_t cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len)
> {
> - u8 msg[6], reg[5];
> + u8 msg[CDN_AUX_MSG_SIZE + CDN_AUX_HEADER_SIZE];
> + u8 reg[CDN_AUX_HEADER_SIZE];
> int ret;
>
> - msg[0] = 0;
> - msg[1] = 1;
> + if (WARN_ON(len > CDN_AUX_MSG_SIZE) || WARN_ON(len <= 0))
> + return -EINVAL;
> +
> + msg[0] = (len >> 8) & 0xff;
> + msg[1] = len & 0xff;
> msg[2] = (addr >> 16) & 0xff;
> msg[3] = (addr >> 8) & 0xff;
> msg[4] = addr & 0xff;
> - msg[5] = value;
> +
> + memcpy(msg + CDN_AUX_HEADER_SIZE, data, len);
> +
> ret = cdn_dp_mailbox_send(dp, MB_MODULE_ID_DP_TX, DPTX_WRITE_DPCD,
> - sizeof(msg), msg);
> + CDN_AUX_HEADER_SIZE + len, msg);
> if (ret)
> goto err_dpcd_write;
>
> @@ -277,8 +297,12 @@ int cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 value)
> if (ret)
> goto err_dpcd_write;
>
> - if (addr != (reg[2] << 16 | reg[3] << 8 | reg[4]))
> + if ((len != (reg[0] << 8 | reg[1])) ||
> + (addr != (reg[2] << 16 | reg[3] << 8 | reg[4]))) {
> ret = -EINVAL;
> + } else {
> + return len;
> + }
>
> err_dpcd_write:
> if (ret)
> @@ -286,6 +310,32 @@ int cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 value)
> return ret;
> }
>
> +int cdn_dp_get_aux_status(struct cdn_dp_device *dp)
> +{
> + u8 status;
> + int ret;
> +
> + ret = cdn_dp_mailbox_send(dp, MB_MODULE_ID_DP_TX, DPTX_GET_LAST_AUX_STAUS,
> + 0, NULL);
> + if (ret)
> + goto err_get_hpd;
> +
> + ret = cdn_dp_mailbox_validate_receive(dp, MB_MODULE_ID_DP_TX,
> + DPTX_GET_LAST_AUX_STAUS, sizeof(status));
> + if (ret)
> + goto err_get_hpd;
> +
> + ret = cdn_dp_mailbox_read_receive(dp, &status, sizeof(status));
> + if (ret)
> + goto err_get_hpd;
> +
> + return status;
> +
> +err_get_hpd:
> + DRM_DEV_ERROR(dp->dev, "get aux status failed: %d\n", ret);
> + return ret;
> +}
> +
> int cdn_dp_load_firmware(struct cdn_dp_device *dp, const u32 *i_mem,
> u32 i_size, const u32 *d_mem, u32 d_size)
> {
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-reg.h b/drivers/gpu/drm/rockchip/cdn-dp-reg.h
> index c4bbb4a83..aedf2dc 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-reg.h
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-reg.h
> @@ -328,6 +328,13 @@
> #define GENERAL_BUS_SETTINGS 0x03
> #define GENERAL_TEST_ACCESS 0x04
>
> +/* AUX status*/
> +#define AUX_STAUS_ACK 0
> +#define AUX_STAUS_NACK 1
> +#define AUX_STAUS_DEFER 2
> +#define AUX_STAUS_SINK_ERROR 3
> +#define AUX_STAUS_BUS_ERROR 4
> +
For the five defines, s/STAUS/STATUS/
> #define DPTX_SET_POWER_MNG 0x00
> #define DPTX_SET_HOST_CAPABILITIES 0x01
> #define DPTX_GET_EDID 0x02
> @@ -469,8 +476,11 @@ int cdn_dp_set_host_cap(struct cdn_dp_device *dp, u8 lanes, bool flip);
> int cdn_dp_event_config(struct cdn_dp_device *dp);
> u32 cdn_dp_get_event(struct cdn_dp_device *dp);
> int cdn_dp_get_hpd_status(struct cdn_dp_device *dp);
> -int cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr, u8 value);
> -int cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr, u8 *data, u16 len);
> +ssize_t cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr,
> + u8 *data, u16 len);
> +ssize_t cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr,
> + u8 *data, u16 len);
> +int cdn_dp_get_aux_status(struct cdn_dp_device *dp);
> int cdn_dp_get_edid_block(void *dp, u8 *edid,
> unsigned int block, size_t length);
> int cdn_dp_train_link(struct cdn_dp_device *dp);
> --
> 2.7.4
>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
^ permalink raw reply
* [PATCH 4/4] drm/rockchip: support dp training outside dp firmware
From: Enric Balletbo Serra @ 2018-05-07 11:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1525421338-1021-4-git-send-email-hl@rock-chips.com>
Hi Lin,
Thanks for the patch.
2018-05-04 10:08 GMT+02:00 Lin Huang <hl@rock-chips.com>:
> DP firware use fix phy config value to do training, but some
s/fiware/firmware/
> board need to adjust these value to fit for their hardware design,
> so we use new phy config to do training outside firmware to meet
> this situation, if there have new phy config pass from dts, it will
> use training outside firmware.
>
maybe you can rewrite all this in a better way.
ooi, which boards needs this?
> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> Signed-off-by: Lin Huang <hl@rock-chips.com>
> ---
> drivers/gpu/drm/rockchip/Makefile | 3 +-
> drivers/gpu/drm/rockchip/cdn-dp-core.c | 23 +-
> drivers/gpu/drm/rockchip/cdn-dp-core.h | 2 +
> drivers/gpu/drm/rockchip/cdn-dp-link-training.c | 398 ++++++++++++++++++++++++
> drivers/gpu/drm/rockchip/cdn-dp-reg.c | 33 +-
> drivers/gpu/drm/rockchip/cdn-dp-reg.h | 38 ++-
> 6 files changed, 480 insertions(+), 17 deletions(-)
> create mode 100644 drivers/gpu/drm/rockchip/cdn-dp-link-training.c
>
> diff --git a/drivers/gpu/drm/rockchip/Makefile b/drivers/gpu/drm/rockchip/Makefile
> index a314e21..b932f62 100644
> --- a/drivers/gpu/drm/rockchip/Makefile
> +++ b/drivers/gpu/drm/rockchip/Makefile
> @@ -9,7 +9,8 @@ rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \
> rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o
>
> rockchipdrm-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o
> -rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o
> +rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o \
> + cdn-dp-link-training.o
> rockchipdrm-$(CONFIG_ROCKCHIP_DW_HDMI) += dw_hdmi-rockchip.o
> rockchipdrm-$(CONFIG_ROCKCHIP_DW_MIPI_DSI) += dw-mipi-dsi.o
> rockchipdrm-$(CONFIG_ROCKCHIP_INNO_HDMI) += inno_hdmi.o
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.c b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> index 268c190..a2a4208 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.c
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.c
> @@ -629,11 +629,13 @@ static void cdn_dp_encoder_enable(struct drm_encoder *encoder)
> goto out;
> }
> }
> -
> - ret = cdn_dp_set_video_status(dp, CONTROL_VIDEO_IDLE);
> - if (ret) {
> - DRM_DEV_ERROR(dp->dev, "Failed to idle video %d\n", ret);
> - goto out;
> + if (dp->sw_training_success == false) {
> + ret = cdn_dp_set_video_status(dp, CONTROL_VIDEO_IDLE);
> + if (ret) {
> + DRM_DEV_ERROR(dp->dev,
> + "Failed to idle video %d\n", ret);
> + goto out;
> + }
> }
>
> ret = cdn_dp_config_video(dp);
> @@ -642,11 +644,14 @@ static void cdn_dp_encoder_enable(struct drm_encoder *encoder)
> goto out;
> }
>
> - ret = cdn_dp_set_video_status(dp, CONTROL_VIDEO_VALID);
> - if (ret) {
> - DRM_DEV_ERROR(dp->dev, "Failed to valid video %d\n", ret);
> - goto out;
> + if (dp->sw_training_success == false) {
> + ret = cdn_dp_set_video_status(dp, CONTROL_VIDEO_VALID);
> + if (ret) {
> + DRM_DEV_ERROR(dp->dev, "Failed to valid video %d\n", ret);
> + goto out;
> + }
> }
> +
> out:
> mutex_unlock(&dp->lock);
> }
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-core.h b/drivers/gpu/drm/rockchip/cdn-dp-core.h
> index 46159b2..c6050ab 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-core.h
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-core.h
> @@ -84,6 +84,7 @@ struct cdn_dp_device {
> bool connected;
> bool active;
> bool suspended;
> + bool sw_training_success;
>
> const struct firmware *fw; /* cdn dp firmware */
> unsigned int fw_version; /* cdn fw version */
> @@ -106,6 +107,7 @@ struct cdn_dp_device {
> u8 ports;
> u8 lanes;
> int active_port;
> + u8 train_set[4];
>
> u8 dpcd[DP_RECEIVER_CAP_SIZE];
> bool sink_has_audio;
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-link-training.c b/drivers/gpu/drm/rockchip/cdn-dp-link-training.c
> new file mode 100644
> index 0000000..558c945
> --- /dev/null
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-link-training.c
> @@ -0,0 +1,398 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
For a C source file the format is:
(https://www.kernel.org/doc/html/latest/process/license-rules.html)
// SPDX-License-Identifier: <SPDX License Expression>
> +/*
> + * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd
> + * Author: Chris Zhong <zyw@rock-chips.com>
> + */
> +
> +#include <linux/arm-smccc.h>
Why you need this include?
> +#include <linux/clk.h>
> +#include <linux/device.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/phy/phy.h>
> +#include <linux/reset.h>
> +#include <soc/rockchip/rockchip_phy_typec.h>
> +
In fact, I think that there are other includes that can be removed,
please review.
> +#include "cdn-dp-core.h"
> +#include "cdn-dp-reg.h"
> +
> +static void cdn_dp_set_signal_levels(struct cdn_dp_device *dp)
> +{
> + struct cdn_dp_port *port = dp->port[dp->active_port];
> + struct rockchip_typec_phy *tcphy = phy_get_drvdata(port->phy);
> +
> + int rate = drm_dp_bw_code_to_link_rate(dp->link.rate);
> + u8 swing = (dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK) >>
> + DP_TRAIN_VOLTAGE_SWING_SHIFT;
> + u8 pre_emphasis = (dp->train_set[0] & DP_TRAIN_PRE_EMPHASIS_MASK)
> + >> DP_TRAIN_PRE_EMPHASIS_SHIFT;
> +
> + tcphy->typec_phy_config(port->phy, rate, dp->link.num_lanes,
> + swing, pre_emphasis);
> +}
> +
> +static int cdn_dp_set_pattern(struct cdn_dp_device *dp, uint8_t dp_train_pat)
> +{
> + u32 phy_config, global_config;
> + int ret;
> + uint8_t pattern = dp_train_pat & DP_TRAINING_PATTERN_MASK;
> +
> + global_config = NUM_LANES(dp->link.num_lanes - 1) | SST_MODE |
> + GLOBAL_EN | RG_EN | ENC_RST_DIS | WR_VHSYNC_FALL;
> +
> + phy_config = DP_TX_PHY_ENCODER_BYPASS(0) |
> + DP_TX_PHY_SKEW_BYPASS(0) |
> + DP_TX_PHY_DISPARITY_RST(0) |
> + DP_TX_PHY_LANE0_SKEW(0) |
> + DP_TX_PHY_LANE1_SKEW(1) |
> + DP_TX_PHY_LANE2_SKEW(2) |
> + DP_TX_PHY_LANE3_SKEW(3) |
> + DP_TX_PHY_10BIT_ENABLE(0);
> +
> + if (pattern != DP_TRAINING_PATTERN_DISABLE) {
> + global_config |= NO_VIDEO;
> + phy_config |= DP_TX_PHY_TRAINING_ENABLE(1) |
> + DP_TX_PHY_SCRAMBLER_BYPASS(1) |
> + DP_TX_PHY_TRAINING_PATTERN(pattern);
> + }
> +
> + ret = cdn_dp_reg_write(dp, DP_FRAMER_GLOBAL_CONFIG, global_config);
> + if (ret) {
> + DRM_ERROR("fail to set DP_FRAMER_GLOBAL_CONFIG, error: %d\n",
> + ret);
> + return ret;
> + }
> +
> + ret = cdn_dp_reg_write(dp, DP_TX_PHY_CONFIG_REG, phy_config);
> + if (ret) {
> + DRM_ERROR("fail to set DP_TX_PHY_CONFIG_REG, error: %d\n",
> + ret);
> + return ret;
> + }
> +
> + ret = cdn_dp_reg_write(dp, DPTX_LANE_EN, BIT(dp->link.num_lanes) - 1);
> + if (ret) {
> + DRM_ERROR("fail to set DPTX_LANE_EN, error: %d\n", ret);
> + return ret;
> + }
> +
> + if (drm_dp_enhanced_frame_cap(dp->dpcd))
> + ret = cdn_dp_reg_write(dp, DPTX_ENHNCD, 1);
> + else
> + ret = cdn_dp_reg_write(dp, DPTX_ENHNCD, 0);
> + if (ret)
> + DRM_ERROR("failed to set DPTX_ENHNCD, error: %x\n", ret);
> +
> + return ret;
> +}
> +
> +static u8 cdn_dp_pre_emphasis_max(u8 voltage_swing)
> +{
> + switch (voltage_swing & DP_TRAIN_VOLTAGE_SWING_MASK) {
> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_0:
> + return DP_TRAIN_PRE_EMPH_LEVEL_3;
> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_1:
> + return DP_TRAIN_PRE_EMPH_LEVEL_2;
> + case DP_TRAIN_VOLTAGE_SWING_LEVEL_2:
> + return DP_TRAIN_PRE_EMPH_LEVEL_1;
> + default:
> + return DP_TRAIN_PRE_EMPH_LEVEL_0;
> + }
> +}
> +
> +static void cdn_dp_get_adjust_train(struct cdn_dp_device *dp,
> + uint8_t link_status[DP_LINK_STATUS_SIZE])
> +{
> + int i;
> + uint8_t v = 0, p = 0;
> + uint8_t preemph_max;
> +
> + for (i = 0; i < dp->link.num_lanes; i++) {
> + v = max(v, drm_dp_get_adjust_request_voltage(link_status, i));
> + p = max(p, drm_dp_get_adjust_request_pre_emphasis(link_status,
> + i));
> + }
> +
> + if (v >= VOLTAGE_LEVEL_2)
> + v = VOLTAGE_LEVEL_2 | DP_TRAIN_MAX_SWING_REACHED;
> +
> + preemph_max = cdn_dp_pre_emphasis_max(v);
> + if (p >= preemph_max)
> + p = preemph_max | DP_TRAIN_MAX_PRE_EMPHASIS_REACHED;
> +
> + for (i = 0; i < dp->link.num_lanes; i++)
> + dp->train_set[i] = v | p;
> +}
> +
> +/*
> + * Pick training pattern for channel equalization. Training Pattern 3 for HBR2
> + * or 1.2 devices that support it, Training Pattern 2 otherwise.
> + */
> +static u32 cdn_dp_select_chaneq_pattern(struct cdn_dp_device *dp)
> +{
> + u32 training_pattern = DP_TRAINING_PATTERN_2;
> +
> + /*
> + * cdn dp support HBR2 also support TPS3. TPS3 support is also mandatory
> + * for downstream devices that support HBR2. However, not all sinks
> + * follow the spec.
> + */
> + if (drm_dp_tps3_supported(dp->dpcd))
> + training_pattern = DP_TRAINING_PATTERN_3;
> + else
> + DRM_DEBUG_KMS("5.4 Gbps link rate without sink TPS3 support\n");
> +
> + return training_pattern;
> +}
> +
> +
> +static bool cdn_dp_link_max_vswing_reached(struct cdn_dp_device *dp)
> +{
> + int lane;
> +
> + for (lane = 0; lane < dp->link.num_lanes; lane++)
> + if ((dp->train_set[lane] & DP_TRAIN_MAX_SWING_REACHED) == 0)
> + return false;
> +
> + return true;
> +}
> +
> +static int cdn_dp_update_link_train(struct cdn_dp_device *dp)
> +{
> + int ret;
> +
> + cdn_dp_set_signal_levels(dp);
> +
> + ret = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_LANE0_SET,
> + dp->train_set, dp->link.num_lanes);
> + if (ret != dp->link.num_lanes)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int cdn_dp_set_link_train(struct cdn_dp_device *dp,
> + uint8_t dp_train_pat)
> +{
> + uint8_t buf[sizeof(dp->train_set) + 1];
> + int ret, len;
> +
> + buf[0] = dp_train_pat;
> + if ((dp_train_pat & DP_TRAINING_PATTERN_MASK) ==
> + DP_TRAINING_PATTERN_DISABLE) {
> + /* don't write DP_TRAINING_LANEx_SET on disable */
> + len = 1;
> + } else {
> + /* DP_TRAINING_LANEx_SET follow DP_TRAINING_PATTERN_SET */
> + memcpy(buf + 1, dp->train_set, dp->link.num_lanes);
> + len = dp->link.num_lanes + 1;
> + }
> +
> + ret = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_PATTERN_SET,
> + buf, len);
> + if (ret != len)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int cdn_dp_reset_link_train(struct cdn_dp_device *dp,
> + uint8_t dp_train_pat)
> +{
> + int ret;
> +
> + memset(dp->train_set, 0, sizeof(dp->train_set));
> +
> + cdn_dp_set_signal_levels(dp);
> +
> + ret = cdn_dp_set_pattern(dp, dp_train_pat);
> + if (ret)
> + return ret;
> +
> + return cdn_dp_set_link_train(dp, dp_train_pat);
> +}
> +
> +/* Enable corresponding port and start training pattern 1 */
> +static int cdn_dp_link_training_clock_recovery(struct cdn_dp_device *dp)
> +{
> + u8 voltage, link_config[2];
> + u8 link_status[DP_LINK_STATUS_SIZE];
> + u32 voltage_tries, max_vswing_tries;
> + u32 rate, sink_max, source_max;
> + int ret;
> +
> + source_max = dp->lanes;
> + sink_max = drm_dp_max_lane_count(dp->dpcd);
> + dp->link.num_lanes = min(source_max, sink_max);
> +
> + source_max = drm_dp_bw_code_to_link_rate(CDN_DP_MAX_LINK_RATE);
> + sink_max = drm_dp_max_link_rate(dp->dpcd);
> + rate = min(source_max, sink_max);
> + dp->link.rate = drm_dp_link_rate_to_bw_code(rate);
> +
> + /* Write the link configuration data */
> + link_config[0] = dp->link.rate;
> + link_config[1] = dp->link.num_lanes;
> + if (drm_dp_enhanced_frame_cap(dp->dpcd))
> + link_config[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
> + drm_dp_dpcd_write(&dp->aux, DP_LINK_BW_SET, link_config, 2);
> +
> + link_config[0] = 0;
> + link_config[1] = 0;
> + if (dp->dpcd[DP_MAIN_LINK_CHANNEL_CODING] & 0x01)
> + link_config[1] = DP_SET_ANSI_8B10B;
> +
> + drm_dp_dpcd_write(&dp->aux, DP_DOWNSPREAD_CTRL, link_config, 2);
> +
> + /* clock recovery */
> + ret = cdn_dp_reset_link_train(dp, DP_TRAINING_PATTERN_1 |
> + DP_LINK_SCRAMBLING_DISABLE);
> + if (ret) {
> + DRM_ERROR("failed to start link train\n");
> + return ret;
> + }
> +
> + voltage_tries = 1;
> + max_vswing_tries = 0;
> + for (;;) {
> + drm_dp_link_train_clock_recovery_delay(dp->dpcd);
> + if (drm_dp_dpcd_read_link_status(&dp->aux, link_status) !=
> + DP_LINK_STATUS_SIZE) {
> + DRM_ERROR("failed to get link status\n");
> + return -EINVAL;
> + }
> +
> + if (drm_dp_clock_recovery_ok(link_status, dp->link.num_lanes)) {
> + DRM_DEBUG_KMS("clock recovery OK\n");
> + return 0;
> + }
> +
> + if (voltage_tries >= 5) {
> + DRM_DEBUG_KMS("Same voltage tried 5 times\n");
> + return -EINVAL;
> + }
> +
> + if (max_vswing_tries >= 1) {
> + DRM_DEBUG_KMS("Max Voltage Swing reached\n");
> + return -EINVAL;
> + }
> +
> + voltage = dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK;
> +
> + /* Update training set as requested by target */
> + cdn_dp_get_adjust_train(dp, link_status);
> + if (cdn_dp_update_link_train(dp)) {
> + DRM_ERROR("failed to update link training\n");
> + return -EINVAL;
> + }
> +
> + if ((dp->train_set[0] & DP_TRAIN_VOLTAGE_SWING_MASK) ==
> + voltage)
> + ++voltage_tries;
> + else
> + voltage_tries = 1;
> +
> + if (cdn_dp_link_max_vswing_reached(dp))
> + ++max_vswing_tries;
> + }
> +}
> +
> +static int cdn_dp_link_training_channel_equalization(struct cdn_dp_device *dp)
> +{
> + int tries, ret;
> + u32 training_pattern;
> + uint8_t link_status[DP_LINK_STATUS_SIZE];
> +
> + training_pattern = cdn_dp_select_chaneq_pattern(dp);
> + training_pattern |= DP_LINK_SCRAMBLING_DISABLE;
> +
> + ret = cdn_dp_set_pattern(dp, training_pattern);
> + if (ret)
> + return ret;
> +
> + ret = cdn_dp_set_link_train(dp, training_pattern);
> + if (ret) {
> + DRM_ERROR("failed to start channel equalization\n");
> + return ret;
> + }
> +
> + for (tries = 0; tries < 5; tries++) {
> + drm_dp_link_train_channel_eq_delay(dp->dpcd);
> + if (drm_dp_dpcd_read_link_status(&dp->aux, link_status) !=
> + DP_LINK_STATUS_SIZE) {
> + DRM_ERROR("failed to get link status\n");
> + break;
> + }
> +
> + /* Make sure clock is still ok */
> + if (!drm_dp_clock_recovery_ok(link_status, dp->link.num_lanes)) {
> + DRM_DEBUG_KMS("Clock recovery check failed, cannot "
> + "continue channel equalization\n");
> + break;
> + }
> +
> + if (drm_dp_channel_eq_ok(link_status, dp->link.num_lanes)) {
> + DRM_DEBUG_KMS("Channel EQ done. DP Training "
> + "successful\n");
> + return 0;
> + }
> +
> + /* Update training set as requested by target */
> + cdn_dp_get_adjust_train(dp, link_status);
> + if (cdn_dp_update_link_train(dp)) {
> + DRM_ERROR("failed to update link training\n");
> + break;
> + }
> + }
> +
> + /* Try 5 times, else fail and try at lower BW */
> + if (tries == 5)
> + DRM_DEBUG_KMS("Channel equalization failed 5 times\n");
> +
> + return -EINVAL;
> +
> +}
> +
> +static int cdn_dp_stop_link_train(struct cdn_dp_device *dp)
> +{
> + int ret = cdn_dp_set_pattern(dp, DP_TRAINING_PATTERN_DISABLE);
> +
> + if (ret)
> + return ret;
> +
> + return cdn_dp_set_link_train(dp, DP_TRAINING_PATTERN_DISABLE);
> +}
> +
> +int cdn_dp_software_train_link(struct cdn_dp_device *dp)
> +{
> + int ret, stop_err;
> +
> + ret = drm_dp_dpcd_read(&dp->aux, DP_DPCD_REV, dp->dpcd,
> + sizeof(dp->dpcd));
> + if (ret < 0) {
> + DRM_DEV_ERROR(dp->dev, "Failed to get caps %d\n", ret);
> + return ret;
> + }
> +
> + ret = cdn_dp_link_training_clock_recovery(dp);
> + if (ret) {
> + DRM_ERROR("training clock recovery fail, error: %d\n", ret);
> + goto out;
> + }
> +
> + ret = cdn_dp_link_training_channel_equalization(dp);
> + if (ret) {
> + DRM_ERROR("training channel equalization fail, error: %d\n",
> + ret);
> + goto out;
> + }
> +out:
> + stop_err = cdn_dp_stop_link_train(dp);
> + if (stop_err) {
> + DRM_ERROR("stop training fail, error: %d\n", stop_err);
> + return stop_err;
> + }
> +
> + return ret;
> +}
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-reg.c b/drivers/gpu/drm/rockchip/cdn-dp-reg.c
> index b2f532a..72780f1 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-reg.c
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-reg.c
> @@ -17,7 +17,9 @@
> #include <linux/delay.h>
> #include <linux/io.h>
> #include <linux/iopoll.h>
> +#include <linux/phy/phy.h>
> #include <linux/reset.h>
> +#include <soc/rockchip/rockchip_phy_typec.h>
>
> #include "cdn-dp-core.h"
> #include "cdn-dp-reg.h"
> @@ -189,7 +191,7 @@ static int cdn_dp_mailbox_send(struct cdn_dp_device *dp, u8 module_id,
> return 0;
> }
>
> -static int cdn_dp_reg_write(struct cdn_dp_device *dp, u16 addr, u32 val)
> +int cdn_dp_reg_write(struct cdn_dp_device *dp, u16 addr, u32 val)
> {
> u8 msg[6];
>
> @@ -605,22 +607,41 @@ static int cdn_dp_get_training_status(struct cdn_dp_device *dp)
> int cdn_dp_train_link(struct cdn_dp_device *dp)
> {
> int ret;
> + struct cdn_dp_port *port = dp->port[dp->active_port];
> + struct rockchip_typec_phy *tcphy = phy_get_drvdata(port->phy);
>
> + /*
> + * DP firmware uses fixed phy config values to do training, but some
> + * boards need to adjust these values to fit for their unique hardware
> + * design. So if the phy is using custom config values, do software
> + * link training instead of relying on firmware, if software training
> + * fail, keep firmware training as a fallback if sw training fails.
> + */
> + if (tcphy->need_software_training) {
> + ret = cdn_dp_software_train_link(dp);
> + if (ret) {
> + DRM_DEV_ERROR(dp->dev,
> + "Failed to do software training %d\n", ret);
> + goto do_fw_training;
> + }
> + cdn_dp_reg_write(dp, SOURCE_HDTX_CAR, 0xf);
Check for an error here and act accordingly? Or doesn't matter?
> + dp->sw_training_success = true;
> + return 0;
> + }
> +
nit: Personally I don't like this goto. Maybe you can refactor this.
if (tcphy->need_software_training) {
ret = cdn_dp_software_train_link(dp);
if (!ret) {
cdn_dp_reg_write(dp, SOURCE_HDTX_CAR, 0xf);
dp->sw_training_success = true;
return 0;
}
DRM_DEV_ERROR(dp->dev, "Failed to do software training
%d\n", ret);
}
> +do_fw_training:
Then remove the goto label.
> + dp->sw_training_success = false;
> ret = cdn_dp_training_start(dp);
> if (ret) {
> DRM_DEV_ERROR(dp->dev, "Failed to start training %d\n", ret);
> return ret;
> }
> -
Do not remove the new line.
> ret = cdn_dp_get_training_status(dp);
> if (ret) {
> DRM_DEV_ERROR(dp->dev, "Failed to get training stat %d\n", ret);
> return ret;
> }
> -
> - DRM_DEV_DEBUG_KMS(dp->dev, "rate:0x%x, lanes:%d\n", dp->link.rate,
> - dp->link.num_lanes);
Why you remove this debug message?
> - return ret;
> + return 0;
> }
>
> int cdn_dp_set_video_status(struct cdn_dp_device *dp, int active)
> diff --git a/drivers/gpu/drm/rockchip/cdn-dp-reg.h b/drivers/gpu/drm/rockchip/cdn-dp-reg.h
> index aedf2dc..b60a6b2 100644
> --- a/drivers/gpu/drm/rockchip/cdn-dp-reg.h
> +++ b/drivers/gpu/drm/rockchip/cdn-dp-reg.h
> @@ -137,7 +137,7 @@
> #define HPD_EVENT_MASK 0x211c
> #define HPD_EVENT_DET 0x2120
>
> -/* dpyx framer addr */
> +/* dptx framer addr */
> #define DP_FRAMER_GLOBAL_CONFIG 0x2200
> #define DP_SW_RESET 0x2204
> #define DP_FRAMER_TU 0x2208
> @@ -431,6 +431,40 @@
> /* Reference cycles when using lane clock as reference */
> #define LANE_REF_CYC 0x8000
>
> +/* register CM_VID_CTRL */
> +#define LANE_VID_REF_CYC(x) (((x) & (BIT(24) - 1)) << 0)
> +#define NMVID_MEAS_TOLERANCE(x) (((x) & 0xf) << 24)
> +
> +/* register DP_TX_PHY_CONFIG_REG */
> +#define DP_TX_PHY_TRAINING_ENABLE(x) ((x) & 1)
> +#define DP_TX_PHY_TRAINING_TYPE_PRBS7 (0 << 1)
> +#define DP_TX_PHY_TRAINING_TYPE_TPS1 (1 << 1)
> +#define DP_TX_PHY_TRAINING_TYPE_TPS2 (2 << 1)
> +#define DP_TX_PHY_TRAINING_TYPE_TPS3 (3 << 1)
> +#define DP_TX_PHY_TRAINING_TYPE_TPS4 (4 << 1)
> +#define DP_TX_PHY_TRAINING_TYPE_PLTPAT (5 << 1)
> +#define DP_TX_PHY_TRAINING_TYPE_D10_2 (6 << 1)
> +#define DP_TX_PHY_TRAINING_TYPE_HBR2CPAT (8 << 1)
> +#define DP_TX_PHY_TRAINING_PATTERN(x) ((x) << 1)
> +#define DP_TX_PHY_SCRAMBLER_BYPASS(x) (((x) & 1) << 5)
> +#define DP_TX_PHY_ENCODER_BYPASS(x) (((x) & 1) << 6)
> +#define DP_TX_PHY_SKEW_BYPASS(x) (((x) & 1) << 7)
> +#define DP_TX_PHY_DISPARITY_RST(x) (((x) & 1) << 8)
> +#define DP_TX_PHY_LANE0_SKEW(x) (((x) & 7) << 9)
> +#define DP_TX_PHY_LANE1_SKEW(x) (((x) & 7) << 12)
> +#define DP_TX_PHY_LANE2_SKEW(x) (((x) & 7) << 15)
> +#define DP_TX_PHY_LANE3_SKEW(x) (((x) & 7) << 18)
> +#define DP_TX_PHY_10BIT_ENABLE(x) (((x) & 1) << 21)
> +
> +/* register DP_FRAMER_GLOBAL_CONFIG */
> +#define NUM_LANES(x) ((x) & 3)
> +#define SST_MODE (0 << 2)
> +#define GLOBAL_EN (1 << 3)
> +#define RG_EN (0 << 4)
> +#define NO_VIDEO (1 << 5)
> +#define ENC_RST_DIS (1 << 6)
> +#define WR_VHSYNC_FALL (1 << 7)
> +
Use the BIT() macros for these.
> enum voltage_swing_level {
> VOLTAGE_LEVEL_0,
> VOLTAGE_LEVEL_1,
> @@ -476,6 +510,7 @@ int cdn_dp_set_host_cap(struct cdn_dp_device *dp, u8 lanes, bool flip);
> int cdn_dp_event_config(struct cdn_dp_device *dp);
> u32 cdn_dp_get_event(struct cdn_dp_device *dp);
> int cdn_dp_get_hpd_status(struct cdn_dp_device *dp);
> +int cdn_dp_reg_write(struct cdn_dp_device *dp, u16 addr, u32 val);
> ssize_t cdn_dp_dpcd_write(struct cdn_dp_device *dp, u32 addr,
> u8 *data, u16 len);
> ssize_t cdn_dp_dpcd_read(struct cdn_dp_device *dp, u32 addr,
> @@ -489,4 +524,5 @@ int cdn_dp_config_video(struct cdn_dp_device *dp);
> int cdn_dp_audio_stop(struct cdn_dp_device *dp, struct audio_info *audio);
> int cdn_dp_audio_mute(struct cdn_dp_device *dp, bool enable);
> int cdn_dp_audio_config(struct cdn_dp_device *dp, struct audio_info *audio);
> +int cdn_dp_software_train_link(struct cdn_dp_device *dp);
> #endif /* _CDN_DP_REG_H */
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH] clk: davinci: psc-da830: fix USB0 48MHz PHY clock registration
From: Sekhar Nori @ 2018-05-07 11:34 UTC (permalink / raw)
To: linux-arm-kernel
USB0 48MHz PHY clock registration fails on DA830 because the
da8xx-cfgchip clock driver cannot get a reference to USB0
LPSC clock.
The USB0 LPSC needs to be enabled during PHY clock enable. Setup
the clock lookup correctly to fix this.
Signed-off-by: Sekhar Nori <nsekhar@ti.com>
---
drivers/clk/davinci/psc-da830.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/davinci/psc-da830.c b/drivers/clk/davinci/psc-da830.c
index f61abf5632ff..081b039fcb02 100644
--- a/drivers/clk/davinci/psc-da830.c
+++ b/drivers/clk/davinci/psc-da830.c
@@ -55,7 +55,8 @@ const struct davinci_psc_init_data da830_psc0_init_data = {
.psc_init = &da830_psc0_init,
};
-LPSC_CLKDEV2(usb0_clkdev, NULL, "musb-da8xx",
+LPSC_CLKDEV3(usb0_clkdev, "fck", "da830-usb-phy-clks",
+ NULL, "musb-da8xx",
NULL, "cppi41-dmaengine");
LPSC_CLKDEV1(usb1_clkdev, NULL, "ohci-da8xx");
/* REVISIT: gpio-davinci.c should be modified to drop con_id */
--
2.16.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox