* [PATCH v12 3/5] arm64: pgtable: Add p*d_page_vaddr helper macros
From: Chintan Pandya @ 2018-06-04 13:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180604121351.GJ9482@arm.com>
On 6/4/2018 5:43 PM, Will Deacon wrote:
> On Fri, Jun 01, 2018 at 06:09:16PM +0530, Chintan Pandya wrote:
>> Add helper macros to give virtual references to page
>> tables. These will be used while freeing dangling
>> page tables.
>>
>> Signed-off-by: Chintan Pandya <cpandya@codeaurora.org>
>> ---
>> arch/arm64/include/asm/pgtable.h | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
>> index 7c4c8f3..ef4047f 100644
>> --- a/arch/arm64/include/asm/pgtable.h
>> +++ b/arch/arm64/include/asm/pgtable.h
>> @@ -580,6 +580,9 @@ static inline phys_addr_t pgd_page_paddr(pgd_t pgd)
>>
>> #endif /* CONFIG_PGTABLE_LEVELS > 3 */
>>
>> +#define pmd_page_vaddr(pmd) __va(pmd_page_paddr(pmd))
>> +#define pud_page_vaddr(pud) __va(pud_page_paddr(pud))
>
> Are these actually needed, or do pte_offset_kernel and pmd_offset do the
> job already?
>
I introduced these macros for consistency across different arch.
Looking at pte_offset_kernel, it seems to use READ_ONCE() which looks
little costly for its intended use (in next patch) where we already have
dereferenced value. Do you still suggest to remove this ?
> Will
>
Chintan
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum, a Linux Foundation
Collaborative Project
^ permalink raw reply
* [PATCH v12 4/5] arm64: Implement page table free interfaces
From: Chintan Pandya @ 2018-06-04 13:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180604121328.GG9482@arm.com>
On 6/4/2018 5:43 PM, Will Deacon wrote:
> On Fri, Jun 01, 2018 at 06:09:17PM +0530, Chintan Pandya wrote:
>> Implement pud_free_pmd_page() and pmd_free_pte_page().
>>
>> Implementation requires,
>> 1) Clearing off the current pud/pmd entry
>> 2) Invalidate TLB which could have previously
>> valid but not stale entry
>> 3) Freeing of the un-used next level page tables
>
> Please can you rewrite this describing the problem that you're solving,
> rather than a brief summary of some requirements?
Okay. I'll fix this in v13.
>
>> Signed-off-by: Chintan Pandya <cpandya@codeaurora.org>
>> ---
>> arch/arm64/mm/mmu.c | 38 ++++++++++++++++++++++++++++++++++----
>> 1 file changed, 34 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>> index 8ae5d7a..6e7e16c 100644
>> --- a/arch/arm64/mm/mmu.c
>> +++ b/arch/arm64/mm/mmu.c
>> @@ -45,6 +45,7 @@
>> #include <asm/memblock.h>
>> #include <asm/mmu_context.h>
>> #include <asm/ptdump.h>
>> +#include <asm/tlbflush.h>
>>
>> #define NO_BLOCK_MAPPINGS BIT(0)
>> #define NO_CONT_MAPPINGS BIT(1)
>> @@ -977,12 +978,41 @@ int pmd_clear_huge(pmd_t *pmdp)
>> return 1;
>> }
>>
>> -int pud_free_pmd_page(pud_t *pud, unsigned long addr)
>> +int pmd_free_pte_page(pmd_t *pmdp, unsigned long addr)
>> {
>> - return pud_none(*pud);
>> + pte_t *table;
>> + pmd_t pmd;
>> +
>> + pmd = READ_ONCE(*pmdp);
>> + if (pmd_present(pmd)) {
>> + table = pmd_page_vaddr(pmd);
>> + pmd_clear(pmdp);
>> + __flush_tlb_kernel_pgtable(addr);
>> + pte_free_kernel(NULL, table);
>> + }
>> + return 1;
>> }
>>
>> -int pmd_free_pte_page(pmd_t *pmd, unsigned long addr)
>> +int pud_free_pmd_page(pud_t *pudp, unsigned long addr)
>> {
>> - return pmd_none(*pmd);
>> + pmd_t *table;
>> + pmd_t *entry;
>> + pud_t pud;
>> + unsigned long next, end;
>> +
>> + pud = READ_ONCE(*pudp);
>> + if (pud_present(pud)) {
>
> Just some stylistic stuff, but please can you rewrite this as:
>
> if (!pud_present(pud) || VM_WARN_ON(!pud_table(pud)))
> return 1;
>
> similarly for the pmd/pte code above.
Okay. v13 will have this.
>
>> + table = pud_page_vaddr(pud);
>> + entry = table;
>
> Could you rename entry -> pmdp, please?
Sure.
>
>> + next = addr;
>> + end = addr + PUD_SIZE;
>> + do {
>> + pmd_free_pte_page(entry, next);
>> + } while (entry++, next += PMD_SIZE, next != end);
>> +
>> + pud_clear(pudp);
>> + __flush_tlb_kernel_pgtable(addr);
>> + pmd_free(NULL, table);
>> + }
>> + return 1;
>
> So with these patches, we only ever return 1 from these helpers. It looks
> like the same is true for x86, so how about we make them void and move the
> calls inside the conditionals in lib/ioremap.c? Obviously, this would be a
> separate patch on the end.
That sounds valid code churn to me. But since x86 discussion is not
concluded yet, I would wait to share until that gets resolved. May be
not in v13 but separate effort. Would that be okay to you ?
>
> Will
>
Chintan
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum, a Linux Foundation
Collaborative Project
^ permalink raw reply
* [PATCH v12 5/5] arm64: Allow huge io mappings again
From: Chintan Pandya @ 2018-06-04 13:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180604121452.GK9482@arm.com>
On 6/4/2018 5:44 PM, Will Deacon wrote:
> On Fri, Jun 01, 2018 at 06:09:18PM +0530, Chintan Pandya wrote:
>> Huge mappings have had stability issues due to stale
>> TLB entry and memory leak issues. Since, those are
>> addressed in this series of patches, it is now safe
>> to allow huge mappings.
>>
>> Signed-off-by: Chintan Pandya <cpandya@codeaurora.org>
>> ---
>> arch/arm64/mm/mmu.c | 18 ++----------------
>> 1 file changed, 2 insertions(+), 16 deletions(-)
>>
>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>> index 6e7e16c..c65abc4 100644
>> --- a/arch/arm64/mm/mmu.c
>> +++ b/arch/arm64/mm/mmu.c
>> @@ -934,15 +934,8 @@ int pud_set_huge(pud_t *pudp, phys_addr_t phys, pgprot_t prot)
>> {
>> pgprot_t sect_prot = __pgprot(PUD_TYPE_SECT |
>> pgprot_val(mk_sect_prot(prot)));
>> - pud_t new_pud = pfn_pud(__phys_to_pfn(phys), sect_prot);
>> -
>> - /* Only allow permission changes for now */
>> - if (!pgattr_change_is_safe(READ_ONCE(pud_val(*pudp)),
>> - pud_val(new_pud)))
>> - return 0;
>
> Do you actually need to remove these checks? If we're doing
> break-before-make properly, then the check won't fire but it would be
> good to keep it there so we can catch misuse of these in future.
>
> In other words, can we drop this patch?
Yes, we don't need this patch as BBM is happening before this. I missed
that. I'll remove this patch in v13.
>
> Will
>
Chintan
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center,
Inc. is a member of the Code Aurora Forum, a Linux Foundation
Collaborative Project
^ permalink raw reply
* [PATCH] media: atmel-isi: move of_node_put() to cover success branch as well
From: Ludovic Desroches @ 2018-06-04 13:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527859814-30410-1-git-send-email-hofrat@opentech.at>
On Fri, Jun 01, 2018 at 01:30:14PM +0000, Nicholas Mc Guire wrote:
> From: Nicholas Mc Guire <hofrat@osadl.org>
>
> The of_node_put() was only covering the error branch but missed the
> success branch so the refcount for ep which
> of_graph_get_remote_port_parent() incremented on success would was
> not being decremented.
>
> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com>
Thanks
> ---
>
> This patch is on top of: "media: atmel-isi: drop unnecessary while loop"
>
> Patch was compile tested with: x86_64_defconfig + CONFIG_MEDIA_SUPPORT=y
> MEDIA_CAMERA_SUPPORT=y, CONFIG_MEDIA_CONTROLLER=y, V4L_PLATFORM_DRIVERS=y
> OF=y, CONFIG_COMPILE_TEST=y, CONFIG_VIDEO_ATMEL_ISI=y
>
> Compile testing atmel-isi.c shows some sparse warnings. Seems to be due to
> sizeof operator being applied to a union (not related to the function being
> changed though).
>
> Patch is against 4.17-rc7 (localversion-next is next-20180531)
>
> drivers/media/platform/atmel/atmel-isi.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/atmel/atmel-isi.c b/drivers/media/platform/atmel/atmel-isi.c
> index 85fc7b9..e8db4df 100644
> --- a/drivers/media/platform/atmel/atmel-isi.c
> +++ b/drivers/media/platform/atmel/atmel-isi.c
> @@ -1111,10 +1111,9 @@ static int isi_graph_parse(struct atmel_isi *isi, struct device_node *node)
> return -EINVAL;
>
> remote = of_graph_get_remote_port_parent(ep);
> - if (!remote) {
> - of_node_put(ep);
> + of_node_put(ep);
> + if (!remote)
> return -EINVAL;
> - }
>
> /* Remote node to connect */
> isi->entity.node = remote;
> --
> 2.1.4
>
^ permalink raw reply
* [Resend PATCH 1/1] mtd: ubi: Update ubi-media.h to dual license
From: Richard Weinberger @ 2018-06-04 13:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180604132137.32136-2-lionel.debieve@st.com>
Am Montag, 4. Juni 2018, 15:21:37 CEST schrieb Lionel Debieve:
> Update license template using SPDX. Move the global layout
> of UBI headers to dual license helping UBI to be the standard
> solution for raw NAND management.
>
> Signed-off-by: Lionel Debieve <lionel.debieve@st.com>
> Acked-by: Thomas Gleixner <tglx@linutronix.de>
> Acked-by: Artem Bityutskiy <dedekind1@gmail.com>
> Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> ---
> drivers/mtd/ubi/ubi-media.h | 22 +++-------------------
> 1 file changed, 3 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/mtd/ubi/ubi-media.h b/drivers/mtd/ubi/ubi-media.h
> index bfceae5a890e..195ff8ca8211 100644
> --- a/drivers/mtd/ubi/ubi-media.h
> +++ b/drivers/mtd/ubi/ubi-media.h
> @@ -1,28 +1,12 @@
> +/* SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause) */
> /*
> - * Copyright (c) International Business Machines Corp., 2006
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or
> - * (at your option) any later version.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
> - * the GNU General Public License for more details.
> - *
> - * You should have received a copy of the GNU General Public License
> - * along with this program; if not, write to the Free Software
> - * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> - *
> + * Copyright (C) International Business Machines Corp., 2006
> * Authors: Artem Bityutskiy (???????????????? ??????????)
> * Thomas Gleixner
> * Frank Haverkamp
> * Oliver Lohmann
> * Andreas Arnez
> - */
> -
> -/*
> + *
> * This file defines the layout of UBI headers and all the other UBI on-flash
> * data structures.
> */
>
Applied.
Thanks everyone for sorting this out!
Thanks,
//richard
^ permalink raw reply
* [PATCH] media: atmel-isi: drop unnecessary while loop
From: Ludovic Desroches @ 2018-06-04 13:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527857174-24616-1-git-send-email-hofrat@opentech.at>
On Fri, Jun 01, 2018 at 12:46:14PM +0000, Nicholas Mc Guire wrote:
> From: Nicholas Mc Guire <hofrat@osadl.org>
>
> As there is no way this can loop it actually makes no sense to have
> a while(1){} around the body - all three possible paths end in a return
> statement.
>
> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
> Fixes: commit c1d82b895380 "[media] atmel-isi: move out of soc_camera to atmel"
Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com>
Thanks.
> ---
>
> The diff output is unfortunately not that readable - essentially only
> the outer while(1){ } was removed.
>
> Patch was compile tested with: x86_64_defconfig + CONFIG_MEDIA_SUPPORT=y
> MEDIA_CAMERA_SUPPORT=y, CONFIG_MEDIA_CONTROLLER=y, V4L_PLATFORM_DRIVERS=y
> OF=y, CONFIG_COMPILE_TEST=y, CONFIG_VIDEO_ATMEL_ISI=y
>
> Compile testing atmel-isi.c shows some sparse warnings. Seems to be due to
> sizeof operator being applied to a union (not related to the function being
> changed though).
>
> Patch is against 4.17-rc7 (localversion-next is next-20180531)
>
> drivers/media/platform/atmel/atmel-isi.c | 28 +++++++++++++---------------
> 1 file changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/media/platform/atmel/atmel-isi.c b/drivers/media/platform/atmel/atmel-isi.c
> index e5be21a..85fc7b9 100644
> --- a/drivers/media/platform/atmel/atmel-isi.c
> +++ b/drivers/media/platform/atmel/atmel-isi.c
> @@ -1106,23 +1106,21 @@ static int isi_graph_parse(struct atmel_isi *isi, struct device_node *node)
> struct device_node *ep = NULL;
> struct device_node *remote;
>
> - while (1) {
> - ep = of_graph_get_next_endpoint(node, ep);
> - if (!ep)
> - return -EINVAL;
> -
> - remote = of_graph_get_remote_port_parent(ep);
> - if (!remote) {
> - of_node_put(ep);
> - return -EINVAL;
> - }
> + ep = of_graph_get_next_endpoint(node, ep);
> + if (!ep)
> + return -EINVAL;
>
> - /* Remote node to connect */
> - isi->entity.node = remote;
> - isi->entity.asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
> - isi->entity.asd.match.fwnode = of_fwnode_handle(remote);
> - return 0;
> + remote = of_graph_get_remote_port_parent(ep);
> + if (!remote) {
> + of_node_put(ep);
> + return -EINVAL;
> }
> +
> + /* Remote node to connect */
> + isi->entity.node = remote;
> + isi->entity.asd.match_type = V4L2_ASYNC_MATCH_FWNODE;
> + isi->entity.asd.match.fwnode = of_fwnode_handle(remote);
> + return 0;
> }
>
> static int isi_graph_init(struct atmel_isi *isi)
> --
> 2.1.4
>
^ permalink raw reply
* [Resend PATCH 1/1] mtd: ubi: Update ubi-media.h to dual license
From: Lionel Debieve @ 2018-06-04 13:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180604132137.32136-1-lionel.debieve@st.com>
Update license template using SPDX. Move the global layout
of UBI headers to dual license helping UBI to be the standard
solution for raw NAND management.
Signed-off-by: Lionel Debieve <lionel.debieve@st.com>
Acked-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Artem Bityutskiy <dedekind1@gmail.com>
Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
drivers/mtd/ubi/ubi-media.h | 22 +++-------------------
1 file changed, 3 insertions(+), 19 deletions(-)
diff --git a/drivers/mtd/ubi/ubi-media.h b/drivers/mtd/ubi/ubi-media.h
index bfceae5a890e..195ff8ca8211 100644
--- a/drivers/mtd/ubi/ubi-media.h
+++ b/drivers/mtd/ubi/ubi-media.h
@@ -1,28 +1,12 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause) */
/*
- * Copyright (c) International Business Machines Corp., 2006
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
- * the GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
- *
+ * Copyright (C) International Business Machines Corp., 2006
* Authors: Artem Bityutskiy (???????????????? ??????????)
* Thomas Gleixner
* Frank Haverkamp
* Oliver Lohmann
* Andreas Arnez
- */
-
-/*
+ *
* This file defines the layout of UBI headers and all the other UBI on-flash
* data structures.
*/
--
2.15.1
^ permalink raw reply related
* [Resend PATCH 0/1] mtd: ubi: Update ubi-media.h to dual license
From: Lionel Debieve @ 2018-06-04 13:21 UTC (permalink / raw)
To: linux-arm-kernel
Resend the patch about license change, with ack from IBM.
Following previous mail about the dual license update for ubi-media.h file
(ie http://lists.infradead.org/pipermail/linux-mtd/2018-January/078878.html),
this is the corresponding patch about the requested change.
Lionel Debieve (1):
mtd: ubi: Update ubi-media.h to dual license
Update license template using SPDX. Move the global layout of UBI
headers to dual license helping UBI to be the standard solution for
raw NAND management.
drivers/mtd/ubi/ubi-media.h | 22 +++-------------------
1 file changed, 3 insertions(+), 19 deletions(-)
--
2.15.1
^ permalink raw reply
* [PATCH v4.9-stable] serial: pl011: add console matching function
From: Ard Biesheuvel @ 2018-06-04 13:16 UTC (permalink / raw)
To: linux-arm-kernel
From: Aleksey Makarov <aleksey.makarov@linaro.org>
Commit 10879ae5f12e9cab3c4e8e9504c1aaa8a033bde7 upstream.
This patch adds function pl011_console_match() that implements
method match of struct console. It allows to match consoles against
data specified in a string, for example taken from command line or
compiled by ACPI SPCR table handler.
This patch was merged to tty-next but then reverted because of
conflict with
commit 46e36683f433 ("serial: earlycon: Extend earlycon command line option to support 64-bit addresses")
Now it is fixed.
Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
Tested-by: Christopher Covington <cov@codeaurora.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
Please consider for v4.9-stable. It is the missing puzzle piece for SPCR
support on arm64 ACPI systems, which got merged for v4.9 [0]. Now that more
systems are becoming available to people working in the kernel community, it
turns out that v4.9 distro installers (e.g., Debian Stretch) won't work
unless you pass a 'console=' parameter explicitly, which is annoying.
Given that it was clearly the intent to include this code at the time,
I hope it will be considered for backporting.
[0] To quote the tty maintainer:
Also in here is the long-suffering ACPI SPCR patchset, which was
passed around from maintainer to maintainer like a hot-potato. Seems I
was the sucker^Wlucky one. All of those patches have been acked by the
various subsystem maintainers as well.
drivers/tty/serial/amba-pl011.c | 55 ++++++++++++++++++++
1 file changed, 55 insertions(+)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index b42d7f1c9089..6b1863293fe1 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2320,12 +2320,67 @@ static int __init pl011_console_setup(struct console *co, char *options)
return uart_set_options(&uap->port, co, baud, parity, bits, flow);
}
+/**
+ * pl011_console_match - non-standard console matching
+ * @co: registering console
+ * @name: name from console command line
+ * @idx: index from console command line
+ * @options: ptr to option string from console command line
+ *
+ * Only attempts to match console command lines of the form:
+ * console=pl011,mmio|mmio32,<addr>[,<options>]
+ * console=pl011,0x<addr>[,<options>]
+ * This form is used to register an initial earlycon boot console and
+ * replace it with the amba_console at pl011 driver init.
+ *
+ * Performs console setup for a match (as required by interface)
+ * If no <options> are specified, then assume the h/w is already setup.
+ *
+ * Returns 0 if console matches; otherwise non-zero to use default matching
+ */
+static int __init pl011_console_match(struct console *co, char *name, int idx,
+ char *options)
+{
+ unsigned char iotype;
+ resource_size_t addr;
+ int i;
+
+ if (strcmp(name, "pl011") != 0)
+ return -ENODEV;
+
+ if (uart_parse_earlycon(options, &iotype, &addr, &options))
+ return -ENODEV;
+
+ if (iotype != UPIO_MEM && iotype != UPIO_MEM32)
+ return -ENODEV;
+
+ /* try to match the port specified on the command line */
+ for (i = 0; i < ARRAY_SIZE(amba_ports); i++) {
+ struct uart_port *port;
+
+ if (!amba_ports[i])
+ continue;
+
+ port = &amba_ports[i]->port;
+
+ if (port->mapbase != addr)
+ continue;
+
+ co->index = i;
+ port->cons = co;
+ return pl011_console_setup(co, options);
+ }
+
+ return -ENODEV;
+}
+
static struct uart_driver amba_reg;
static struct console amba_console = {
.name = "ttyAMA",
.write = pl011_console_write,
.device = uart_console_device,
.setup = pl011_console_setup,
+ .match = pl011_console_match,
.flags = CON_PRINTBUFFER,
.index = -1,
.data = &amba_reg,
--
2.17.0
^ permalink raw reply related
* [PATCH v6 6/9] dt-bindings: counter: Document stm32 quadrature encoder
From: Benjamin Gaignard @ 2018-06-04 13:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180518162815.GA24966@rob-hp-laptop>
2018-05-18 18:28 GMT+02:00 Rob Herring <robh@kernel.org>:
> On Thu, May 17, 2018 at 08:59:40PM +0200, Benjamin Gaignard wrote:
>> 2018-05-17 18:23 GMT+02:00 Rob Herring <robh+dt@kernel.org>:
>> > On Wed, May 16, 2018 at 12:51 PM, William Breathitt Gray
>> > <vilhelm.gray@gmail.com> wrote:
>> >> From: Benjamin Gaignard <benjamin.gaignard@st.com>
>> >
>> > v6? Where's v1-v5?
>> >
>> >> Add bindings for STM32 Timer quadrature encoder.
>> >> It is a sub-node of STM32 Timer which implement the
>> >> counter part of the hardware.
>> >>
>> >> Cc: Rob Herring <robh+dt@kernel.org>
>> >> Cc: Mark Rutland <mark.rutland@arm.com>
>> >> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>> >> Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
>> >> ---
>> >> .../bindings/counter/stm32-timer-cnt.txt | 26 +++++++++++++++++++
>> >> .../devicetree/bindings/mfd/stm32-timers.txt | 7 +++++
>> >> 2 files changed, 33 insertions(+)
>> >> create mode 100644 Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt b/Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt
>> >> new file mode 100644
>> >> index 000000000000..377728128bef
>> >> --- /dev/null
>> >> +++ b/Documentation/devicetree/bindings/counter/stm32-timer-cnt.txt
>> >> @@ -0,0 +1,26 @@
>> >> +STMicroelectronics STM32 Timer quadrature encoder
>> >> +
>> >> +STM32 Timer provides quadrature encoder counter mode to detect
>> >
>> > 'mode' does not sound like a sub-block of the timers block.
>>
>> quadrature encoding is one of the counting modes of this hardware
>> block which is enable to count on other signals/triggers
>
> You don't need a child node and compatible to set a mode.
"mode" isn't the good word here because quadratic encoder enable a
sub-block of this hardware.
Timer internal counter input could be internal or external clocks,
some IIO triggers
or the output of the quadratic encoder sub-block.
It is a child like pwm or IIO trigger.
>
>> >> +angular position and direction of rotary elements,
>> >> +from IN1 and IN2 input signals.
>> >> +
>> >> +Must be a sub-node of an STM32 Timer device tree node.
>> >> +See ../mfd/stm32-timers.txt for details about the parent node.
>> >> +
>> >> +Required properties:
>> >> +- compatible: Must be "st,stm32-timer-counter".
>> >> +- pinctrl-names: Set to "default".
>> >> +- pinctrl-0: List of phandles pointing to pin configuration nodes,
>> >> + to set IN1/IN2 pins in mode of operation for Low-Power
>> >> + Timer input on external pin.
>> >> +
>> >> +Example:
>> >> + timers at 40010000 {
>> >> + compatible = "st,stm32-timers";
>> >> + ...
>> >> + counter {
>> >> + compatible = "st,stm32-timer-counter";
>> >
>> > Is there only 1? How is the counter addressed?
>>
>> Yes there is only one counter per hardware block.
>> Counter is addressed like the two others sub-nodes and the details
>> about parent mode are describe in stm32-timers.txt
>> Should I add them here too ? so example will be like that:
>
> No, you should drop the child node and add pinctrl to the parent.
>
> Any other functions this block has that you plan on adding? Please make
> bindings as complete as possible, not what you currently have drivers
> for.
Counter framework didn't exist when I pushed timer node but thanks to
William's effort
it will allow us to use this kindf of hardware
Benjamin
>
>> timers at 40010000 {
>> #address-cells = <1>;
>> #size-cells = <0>;
>> compatible = "st,stm32-timers";
>> reg = <0x40010000 0x400>;
>> clocks = <&rcc 0 160>;
>> clock-names = "int";
>> counter {
>> compatible = "st,stm32-timer-counter";
>> pinctrl-names = "default";
>> pinctrl-0 = <&tim1_in_pins>;
>> };
>> };
>>
>> Benjamin
>> >
>> > _______________________________________________
>> > linux-arm-kernel mailing list
>> > linux-arm-kernel at lists.infradead.org
>> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
Benjamin Gaignard
Graphic Study Group
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* qcom: add firmware file for Venus on SDM845
From: Josh Boyer @ 2018-06-04 12:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <bcb4ba3ccd3b139ce03b6736d8d07cbe@codeaurora.org>
On Sat, May 26, 2018 at 4:18 AM Vikash Garodia <vgarodia@codeaurora.org> wrote:
>
> Hi Josh,
>
> On 2018-05-25 17:34, Josh Boyer wrote:
> > On Fri, May 25, 2018 at 7:03 AM Vikash Garodia
> > <vgarodia@codeaurora.org>
> > wrote:
> >
> >> This pull request adds firmware files for Venus h/w codec found on the
> > Qualcomm SDM845 chipset.
> >
> >> The following changes since commit
> > 2a9b2cf50fb32e36e4fc1586c2f6f1421913b553:
> >
> >> Merge branch 'for-upstreaming-v1.7.2' of
> > https://github.com/felix-cavium/linux-firmware (2018-05-18 08:35:22
> > -0400)
> >
> >> are available in the git repository at:
> >
> >
> >> https://github.com/vgarodia/linux-firmware master
> >
> >> for you to fetch changes up to
> >> d6088b9c9d7f49d3c6c43681190889eca0abdcce:
> >
> >> qcom: add venus firmware files for v5.2 (2018-05-25 15:16:43 +0530)
> >
> >> ----------------------------------------------------------------
> >> Vikash Garodia (1):
> >> qcom: add venus firmware files for v5.2
> >
> >> WHENCE | 9 +++++++++
> >> qcom/venus-5.2/venus.b00 | Bin 0 -> 212 bytes
> >> qcom/venus-5.2/venus.b01 | Bin 0 -> 6600 bytes
> >> qcom/venus-5.2/venus.b02 | Bin 0 -> 819552 bytes
> >> qcom/venus-5.2/venus.b03 | Bin 0 -> 33536 bytes
> >> qcom/venus-5.2/venus.b04 | 1 +
> >> qcom/venus-5.2/venus.mbn | Bin 0 -> 865408 bytes
> >> qcom/venus-5.2/venus.mdt | Bin 0 -> 6812 bytes
> >> 8 files changed, 10 insertions(+)
> >> create mode 100644 qcom/venus-5.2/venus.b00
> >> create mode 100644 qcom/venus-5.2/venus.b01
> >> create mode 100644 qcom/venus-5.2/venus.b02
> >> create mode 100644 qcom/venus-5.2/venus.b03
> >> create mode 100644 qcom/venus-5.2/venus.b04
> >> create mode 100644 qcom/venus-5.2/venus.mbn
> >> create mode 100644 qcom/venus-5.2/venus.mdt
> >
> > The venus.mbn file isn't mentioned in WHENCE:
> >
> > [jwboyer at vader linux-firmware]$ ./check_whence.py
> > E: qcom/venus-5.2/venus.mbn not listed in WHENCE
> > [jwboyer at vader linux-firmware]$
> >
> > Can you fix that up and let me know when to re-pull?
> I have fixed the error and is ready to be re-pulled from the same git
> repository.
> I have noted the process to run check_whence.py before uploading the
> firmwares.
>
> Do i need to resend the pull request as the end commit is now changed to
> 7602644358157e4b89960472b6d59ffcc040ca52 ?
Nope. Pulled and pushed out now. Thanks.
josh
^ permalink raw reply
* [PATCH 0/6] ARM: shmobile: rcar: Drop lehacy SYSC fallbacks
From: Simon Horman @ 2018-06-04 12:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180604101307.gwakbdqnka6arhxc@verge.net.au>
On Mon, Jun 04, 2018 at 12:13:08PM +0200, Simon Horman wrote:
> On Wed, May 30, 2018 at 05:25:10PM +0200, Geert Uytterhoeven wrote:
> > Hi Simon, Magnus,
> >
> > When DT SYSC support was introduced in v4.7, legacy fallbacks were kept
> > to keep secondary CPUs working on R-Car H1, H2, and M2-W using old DTBs.
> > However, the time has come to drop these fallbacks, and clean up the
> > resulting code.
> >
> > Most of this was written when I worked on DT SYSC support, but postponed
> > until the time was ripe. So basically I've been running this during the
> > last +2 years.
> >
> > I avoided touching drivers/soc/renesas and arch/arm/mach-shmobile code
> > in the same patch. If you prefer some patches to be squashed, please
> > let me know.
> >
> > Tested on Marzen (R-Car H1), Lager (R-Car H2), and Koelsch (R-Car M2-W).
> >
> > Thanks!
> >
> > Geert Uytterhoeven (6):
> > ARM: shmobile: rcar-gen2: Remove explicit SYSC config and init
> > ARM: shmobile: r8a7779: Stop powering down secondary CPUs during early
> > boot
> > soc: renesas: rcar-sysc: Provide helpers to power up/down CPUs
> > ARM: shmobile: r8a7779: Use rcar_sysc_power_{down,up}_cpu()
> > ARM: shmobile: r8a7779: Remove explicit SYSC config and init
> > soc: renesas: rcar-sysc: Drop legacy handling
> >
> > arch/arm/mach-shmobile/Makefile | 2 +-
> > arch/arm/mach-shmobile/pm-r8a7779.c | 41 ----------------------
> > arch/arm/mach-shmobile/pm-rcar-gen2.c | 25 --------------
> > arch/arm/mach-shmobile/r8a7779.h | 2 --
> > arch/arm/mach-shmobile/smp-r8a7779.c | 54 ++++-------------------------
> > drivers/soc/renesas/rcar-sysc.c | 64 ++++++++++++++++++++++-------------
> > include/linux/soc/renesas/rcar-sysc.h | 13 ++-----
> > 7 files changed, 50 insertions(+), 151 deletions(-)
> > delete mode 100644 arch/arm/mach-shmobile/pm-r8a7779.c
>
> Thanks, I have applied this and intend to push shortly.
>
> I had a conflict in the Makefile when applying patch 5/6.
> Please check to make sure I applied that patch correctly.
Sorry for the noise. I found that the conflict was caused by
an error on my part. I fixed that and the series applied cleanly.
The result is in renesas-devel-20180604-v4.17-rc7.
^ permalink raw reply
* [PATCH] rtc: sunxi: fix possible race condition
From: Maxime Ripard @ 2018-06-04 12:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180604120540.5782-1-alexandre.belloni@bootlin.com>
On Mon, Jun 04, 2018 at 02:05:40PM +0200, Alexandre Belloni wrote:
> The IRQ is requested before the struct rtc is allocated and registered, but
> this struct is used in the IRQ handler. This may lead to a NULL pointer
> dereference.
>
> Switch to devm_rtc_allocate_device/rtc_register_device to allocate the rtc
> before requesting the IRQ.
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>
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/20180604/e4f3741d/attachment.sig>
^ permalink raw reply
* [PATCH v3 8/8] ARM: dts: rcar-gen2: Remove unused VIN properties
From: Niklas Söderlund @ 2018-06-04 12:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527606359-19261-9-git-send-email-jacopo+renesas@jmondi.org>
Hi Jacopo,
Thanks for your work.
On 2018-05-29 17:05:59 +0200, Jacopo Mondi wrote:
> The 'bus-width' and 'pclk-sample' properties are not parsed by the VIN
> driver and only confuse users. Remove them in all Gen2 SoC that use
> them.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
The more I think about this the more I lean towards that this patch
should be dropped. The properties accurately describes the hardware and
I think there is value in that. That the driver currently don't parse or
make use of them don't in my view reduce there value. Maybe you should
break out this patch to a separate series?
> ---
> v3:
> - remove bus-width from dt-bindings example
> ---
> Documentation/devicetree/bindings/media/rcar_vin.txt | 1 -
> arch/arm/boot/dts/r8a7790-lager.dts | 3 ---
> arch/arm/boot/dts/r8a7791-koelsch.dts | 3 ---
> arch/arm/boot/dts/r8a7791-porter.dts | 1 -
> arch/arm/boot/dts/r8a7793-gose.dts | 3 ---
> arch/arm/boot/dts/r8a7794-alt.dts | 1 -
> arch/arm/boot/dts/r8a7794-silk.dts | 1 -
> 7 files changed, 13 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index 024c109..c6d7f60 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -128,7 +128,6 @@ Board setup example for Gen2 platforms (vin1 composite video input)
>
> vin1ep0: endpoint {
> remote-endpoint = <&adv7180>;
> - bus-width = <8>;
> };
> };
> };
> diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
> index 092610e..9cdabfcf 100644
> --- a/arch/arm/boot/dts/r8a7790-lager.dts
> +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> @@ -885,10 +885,8 @@
> port {
> vin0ep2: endpoint {
> remote-endpoint = <&adv7612_out>;
> - bus-width = <24>;
> hsync-active = <0>;
> vsync-active = <0>;
> - pclk-sample = <1>;
> data-active = <1>;
> };
> };
> @@ -904,7 +902,6 @@
> port {
> vin1ep0: endpoint {
> remote-endpoint = <&adv7180>;
> - bus-width = <8>;
> };
> };
> };
> diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
> index 8ab793d..033c9e3 100644
> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> @@ -857,10 +857,8 @@
> port {
> vin0ep2: endpoint {
> remote-endpoint = <&adv7612_out>;
> - bus-width = <24>;
> hsync-active = <0>;
> vsync-active = <0>;
> - pclk-sample = <1>;
> data-active = <1>;
> };
> };
> @@ -875,7 +873,6 @@
> port {
> vin1ep: endpoint {
> remote-endpoint = <&adv7180>;
> - bus-width = <8>;
> };
> };
> };
> diff --git a/arch/arm/boot/dts/r8a7791-porter.dts b/arch/arm/boot/dts/r8a7791-porter.dts
> index a01101b..c16e870 100644
> --- a/arch/arm/boot/dts/r8a7791-porter.dts
> +++ b/arch/arm/boot/dts/r8a7791-porter.dts
> @@ -388,7 +388,6 @@
> port {
> vin0ep: endpoint {
> remote-endpoint = <&adv7180>;
> - bus-width = <8>;
> };
> };
> };
> diff --git a/arch/arm/boot/dts/r8a7793-gose.dts b/arch/arm/boot/dts/r8a7793-gose.dts
> index aa209f6..60aaddb 100644
> --- a/arch/arm/boot/dts/r8a7793-gose.dts
> +++ b/arch/arm/boot/dts/r8a7793-gose.dts
> @@ -765,10 +765,8 @@
> port {
> vin0ep2: endpoint {
> remote-endpoint = <&adv7612_out>;
> - bus-width = <24>;
> hsync-active = <0>;
> vsync-active = <0>;
> - pclk-sample = <1>;
> data-active = <1>;
> };
> };
> @@ -784,7 +782,6 @@
> port {
> vin1ep: endpoint {
> remote-endpoint = <&adv7180_out>;
> - bus-width = <8>;
> };
> };
> };
> diff --git a/arch/arm/boot/dts/r8a7794-alt.dts b/arch/arm/boot/dts/r8a7794-alt.dts
> index e170275..8ed7a71 100644
> --- a/arch/arm/boot/dts/r8a7794-alt.dts
> +++ b/arch/arm/boot/dts/r8a7794-alt.dts
> @@ -388,7 +388,6 @@
> port {
> vin0ep: endpoint {
> remote-endpoint = <&adv7180>;
> - bus-width = <8>;
> };
> };
> };
> diff --git a/arch/arm/boot/dts/r8a7794-silk.dts b/arch/arm/boot/dts/r8a7794-silk.dts
> index 7808aae..6adfcd6 100644
> --- a/arch/arm/boot/dts/r8a7794-silk.dts
> +++ b/arch/arm/boot/dts/r8a7794-silk.dts
> @@ -477,7 +477,6 @@
> port {
> vin0ep: endpoint {
> remote-endpoint = <&adv7180>;
> - bus-width = <8>;
> };
> };
> };
> --
> 2.7.4
>
--
Regards,
Niklas S?derlund
^ permalink raw reply
* [PATCH v3 6/8] dt-bindings: rcar-vin: Add 'hsync-as-de' custom prop
From: Niklas Söderlund @ 2018-06-04 12:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527606359-19261-7-git-send-email-jacopo+renesas@jmondi.org>
Hi Jacopo,
Thanks for your work.
On 2018-05-29 17:05:57 +0200, Jacopo Mondi wrote:
> Document the boolean custom property 'renesas,hsync-as-de' that indicates
> that the HSYNC signal is internally used as data-enable, when the
> CLKENB signal is not connected.
>
> As this is a VIN specificity create a custom property specific to the R-Car
> VIN driver.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
> v3:
> - new patch
> ---
> Documentation/devicetree/bindings/media/rcar_vin.txt | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index ff53226..024c109 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -60,6 +60,9 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
> - vsync-active: see [1] for description. Default is active high.
> - data-enable-active: polarity of CLKENB signal, see [1] for
> description. Default is active high.
> + - renesas,hsync-as-de: a boolean property to indicate that HSYNC signal
> + is internally used as data-enable when the CLKENB signal is
> + not available.
I'm not sure I like this, is there really a need to add a custom
property for this? The datasheet states that when the CLKENB pin is not
connected the driver should enable 'Clock Enable Hsync Select (CHS)'.
With the new generic property 'data-enable-active' which describes the
polarity of the CLKENB pin we also gain the knowledge if the CLKENB pin
is connected or not.
I propose we drop this custom property and instead let the driver check
if the CLKENB polarity is described or not and use that to determine if
CHS bit should be set or not. IMHO that is much simpler then having two
properties describing the same pin.
>
> If both HSYNC and VSYNC polarities are not specified, embedded
> synchronization is selected.
> --
> 2.7.4
>
--
Regards,
Niklas S?derlund
^ permalink raw reply
* [PATCH 5/7] iommu/dma: add support for non-strict mode
From: Leizhen (ThunderTown) @ 2018-06-04 12:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201806020106.eRUp2Ehc%fengguang.wu@intel.com>
On 2018/6/2 1:51, kbuild test robot wrote:
> Hi Zhen,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on linus/master]
> [also build test WARNING on v4.17-rc7 next-20180601]
> [cannot apply to iommu/next]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Zhen-Lei/add-non-strict-mode-support-for-arm-smmu-v3/20180602-000418
> config: x86_64-randconfig-x008-201821 (attached as .config)
> compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=x86_64
>
> All warnings (new ones prefixed by >>):
>
> drivers//iommu/amd_iommu.c: In function 'amd_iommu_capable':
>>> drivers//iommu/amd_iommu.c:3053:2: warning: enumeration value 'IOMMU_CAP_NON_STRICT' not handled in switch [-Wswitch]
> switch (cap) {
> ^~~~~~
>
> vim +/IOMMU_CAP_NON_STRICT +3053 drivers//iommu/amd_iommu.c
>
> 645c4c8d arch/x86/kernel/amd_iommu.c Joerg Roedel 2008-12-02 3050
> ab636481 drivers/iommu/amd_iommu.c Joerg Roedel 2014-09-05 3051 static bool amd_iommu_capable(enum iommu_cap cap)
> dbb9fd86 arch/x86/kernel/amd_iommu.c Sheng Yang 2009-03-18 3052 {
> 80a506b8 arch/x86/kernel/amd_iommu.c Joerg Roedel 2010-07-27 @3053 switch (cap) {
> 80a506b8 arch/x86/kernel/amd_iommu.c Joerg Roedel 2010-07-27 3054 case IOMMU_CAP_CACHE_COHERENCY:
> ab636481 drivers/iommu/amd_iommu.c Joerg Roedel 2014-09-05 3055 return true;
> bdddadcb drivers/iommu/amd_iommu.c Joerg Roedel 2012-07-02 3056 case IOMMU_CAP_INTR_REMAP:
> ab636481 drivers/iommu/amd_iommu.c Joerg Roedel 2014-09-05 3057 return (irq_remapping_enabled == 1);
> cfdeec22 drivers/iommu/amd_iommu.c Will Deacon 2014-10-27 3058 case IOMMU_CAP_NOEXEC:
It seems that it's better to change this to 'default'.
> cfdeec22 drivers/iommu/amd_iommu.c Will Deacon 2014-10-27 3059 return false;
> 80a506b8 arch/x86/kernel/amd_iommu.c Joerg Roedel 2010-07-27 3060 }
> 80a506b8 arch/x86/kernel/amd_iommu.c Joerg Roedel 2010-07-27 3061
> ab636481 drivers/iommu/amd_iommu.c Joerg Roedel 2014-09-05 3062 return false;
> dbb9fd86 arch/x86/kernel/amd_iommu.c Sheng Yang 2009-03-18 3063 }
> dbb9fd86 arch/x86/kernel/amd_iommu.c Sheng Yang 2009-03-18 3064
>
> :::::: The code at line 3053 was first introduced by commit
> :::::: 80a506b8fdcfa868bb53eb740f928217d0966fc1 x86/amd-iommu: Export cache-coherency capability
>
> :::::: TO: Joerg Roedel <joerg.roedel@amd.com>
> :::::: CC: Joerg Roedel <joerg.roedel@amd.com>
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
>
--
Thanks!
BestRegards
^ permalink raw reply
* [PATCH v12 5/5] arm64: Allow huge io mappings again
From: Will Deacon @ 2018-06-04 12:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527856758-27169-6-git-send-email-cpandya@codeaurora.org>
On Fri, Jun 01, 2018 at 06:09:18PM +0530, Chintan Pandya wrote:
> Huge mappings have had stability issues due to stale
> TLB entry and memory leak issues. Since, those are
> addressed in this series of patches, it is now safe
> to allow huge mappings.
>
> Signed-off-by: Chintan Pandya <cpandya@codeaurora.org>
> ---
> arch/arm64/mm/mmu.c | 18 ++----------------
> 1 file changed, 2 insertions(+), 16 deletions(-)
>
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 6e7e16c..c65abc4 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -934,15 +934,8 @@ int pud_set_huge(pud_t *pudp, phys_addr_t phys, pgprot_t prot)
> {
> pgprot_t sect_prot = __pgprot(PUD_TYPE_SECT |
> pgprot_val(mk_sect_prot(prot)));
> - pud_t new_pud = pfn_pud(__phys_to_pfn(phys), sect_prot);
> -
> - /* Only allow permission changes for now */
> - if (!pgattr_change_is_safe(READ_ONCE(pud_val(*pudp)),
> - pud_val(new_pud)))
> - return 0;
Do you actually need to remove these checks? If we're doing
break-before-make properly, then the check won't fire but it would be
good to keep it there so we can catch misuse of these in future.
In other words, can we drop this patch?
Will
^ permalink raw reply
* [PATCH v12 3/5] arm64: pgtable: Add p*d_page_vaddr helper macros
From: Will Deacon @ 2018-06-04 12:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527856758-27169-4-git-send-email-cpandya@codeaurora.org>
On Fri, Jun 01, 2018 at 06:09:16PM +0530, Chintan Pandya wrote:
> Add helper macros to give virtual references to page
> tables. These will be used while freeing dangling
> page tables.
>
> Signed-off-by: Chintan Pandya <cpandya@codeaurora.org>
> ---
> arch/arm64/include/asm/pgtable.h | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index 7c4c8f3..ef4047f 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -580,6 +580,9 @@ static inline phys_addr_t pgd_page_paddr(pgd_t pgd)
>
> #endif /* CONFIG_PGTABLE_LEVELS > 3 */
>
> +#define pmd_page_vaddr(pmd) __va(pmd_page_paddr(pmd))
> +#define pud_page_vaddr(pud) __va(pud_page_paddr(pud))
Are these actually needed, or do pte_offset_kernel and pmd_offset do the
job already?
Will
^ permalink raw reply
* [PATCH v12 4/5] arm64: Implement page table free interfaces
From: Will Deacon @ 2018-06-04 12:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527856758-27169-5-git-send-email-cpandya@codeaurora.org>
On Fri, Jun 01, 2018 at 06:09:17PM +0530, Chintan Pandya wrote:
> Implement pud_free_pmd_page() and pmd_free_pte_page().
>
> Implementation requires,
> 1) Clearing off the current pud/pmd entry
> 2) Invalidate TLB which could have previously
> valid but not stale entry
> 3) Freeing of the un-used next level page tables
Please can you rewrite this describing the problem that you're solving,
rather than a brief summary of some requirements?
> Signed-off-by: Chintan Pandya <cpandya@codeaurora.org>
> ---
> arch/arm64/mm/mmu.c | 38 ++++++++++++++++++++++++++++++++++----
> 1 file changed, 34 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 8ae5d7a..6e7e16c 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -45,6 +45,7 @@
> #include <asm/memblock.h>
> #include <asm/mmu_context.h>
> #include <asm/ptdump.h>
> +#include <asm/tlbflush.h>
>
> #define NO_BLOCK_MAPPINGS BIT(0)
> #define NO_CONT_MAPPINGS BIT(1)
> @@ -977,12 +978,41 @@ int pmd_clear_huge(pmd_t *pmdp)
> return 1;
> }
>
> -int pud_free_pmd_page(pud_t *pud, unsigned long addr)
> +int pmd_free_pte_page(pmd_t *pmdp, unsigned long addr)
> {
> - return pud_none(*pud);
> + pte_t *table;
> + pmd_t pmd;
> +
> + pmd = READ_ONCE(*pmdp);
> + if (pmd_present(pmd)) {
> + table = pmd_page_vaddr(pmd);
> + pmd_clear(pmdp);
> + __flush_tlb_kernel_pgtable(addr);
> + pte_free_kernel(NULL, table);
> + }
> + return 1;
> }
>
> -int pmd_free_pte_page(pmd_t *pmd, unsigned long addr)
> +int pud_free_pmd_page(pud_t *pudp, unsigned long addr)
> {
> - return pmd_none(*pmd);
> + pmd_t *table;
> + pmd_t *entry;
> + pud_t pud;
> + unsigned long next, end;
> +
> + pud = READ_ONCE(*pudp);
> + if (pud_present(pud)) {
Just some stylistic stuff, but please can you rewrite this as:
if (!pud_present(pud) || VM_WARN_ON(!pud_table(pud)))
return 1;
similarly for the pmd/pte code above.
> + table = pud_page_vaddr(pud);
> + entry = table;
Could you rename entry -> pmdp, please?
> + next = addr;
> + end = addr + PUD_SIZE;
> + do {
> + pmd_free_pte_page(entry, next);
> + } while (entry++, next += PMD_SIZE, next != end);
> +
> + pud_clear(pudp);
> + __flush_tlb_kernel_pgtable(addr);
> + pmd_free(NULL, table);
> + }
> + return 1;
So with these patches, we only ever return 1 from these helpers. It looks
like the same is true for x86, so how about we make them void and move the
calls inside the conditionals in lib/ioremap.c? Obviously, this would be a
separate patch on the end.
Will
^ permalink raw reply
* [PATCH v3 5/8] media: rcar-vin: Handle data-enable polarity
From: Niklas Söderlund @ 2018-06-04 12:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527606359-19261-6-git-send-email-jacopo+renesas@jmondi.org>
Hi Jacopo,
Thanks for your work.
On 2018-05-29 17:05:56 +0200, Jacopo Mondi wrote:
> Handle data-enable signal polarity. If the polarity is not specifically
> requested to be active low, use the active high default.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Acked-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> v3:
> - use new property to set the CES bit
> ---
> drivers/media/platform/rcar-vin/rcar-dma.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
> index d2b7002..9145b56 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -123,6 +123,7 @@
> /* Video n Data Mode Register 2 bits */
> #define VNDMR2_VPS (1 << 30)
> #define VNDMR2_HPS (1 << 29)
> +#define VNDMR2_CES (1 << 28)
> #define VNDMR2_FTEV (1 << 17)
> #define VNDMR2_VLV(n) ((n & 0xf) << 12)
>
> @@ -698,6 +699,10 @@ static int rvin_setup(struct rvin_dev *vin)
> /* Vsync Signal Polarity Select */
> if (!(vin->parallel->mbus_flags & V4L2_MBUS_VSYNC_ACTIVE_LOW))
> dmr2 |= VNDMR2_VPS;
> +
> + /* Data Enable Polarity Select */
> + if (vin->parallel->mbus_flags & V4L2_MBUS_DATA_ENABLE_LOW)
> + dmr2 |= VNDMR2_CES;
> }
>
> /*
> --
> 2.7.4
>
--
Regards,
Niklas S?derlund
^ permalink raw reply
* [PATCH v3 4/8] dt-bindings: media: rcar-vin: Add 'data-enable-active'
From: Niklas Söderlund @ 2018-06-04 12:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527606359-19261-5-git-send-email-jacopo+renesas@jmondi.org>
Hi Jacopo,
Thanks for your patch.
On 2018-05-29 17:05:55 +0200, Jacopo Mondi wrote:
> Describe optional endpoint property 'data-enable-active' for R-Car VIN.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>
> ---
> v3:
> - new patch
> ---
>
> Documentation/devicetree/bindings/media/rcar_vin.txt | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/media/rcar_vin.txt b/Documentation/devicetree/bindings/media/rcar_vin.txt
> index 4d91a36..ff53226 100644
> --- a/Documentation/devicetree/bindings/media/rcar_vin.txt
> +++ b/Documentation/devicetree/bindings/media/rcar_vin.txt
> @@ -58,6 +58,8 @@ from local SoC CSI-2 receivers (port1) depending on SoC.
> - Optional properties for endpoint nodes of port at 0:
> - hsync-active: see [1] for description. Default is active high.
> - vsync-active: see [1] for description. Default is active high.
> + - data-enable-active: polarity of CLKENB signal, see [1] for
> + description. Default is active high.
>
> If both HSYNC and VSYNC polarities are not specified, embedded
> synchronization is selected.
> --
> 2.7.4
>
--
Regards,
Niklas S?derlund
^ permalink raw reply
* [PATCH v3 3/8] media: v4l2-fwnode: parse 'data-enable-active' prop
From: Niklas Söderlund @ 2018-06-04 12:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527606359-19261-4-git-send-email-jacopo+renesas@jmondi.org>
Hi Jacopo,
Thanks for your patch.
On 2018-05-29 17:05:54 +0200, Jacopo Mondi wrote:
> Parse the newly defined 'data-enable-active' property in parallel endpoint
> parsing function.
>
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
Reviewed-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>
>
> ---
> v3:
> - new patch
> ---
> drivers/media/v4l2-core/v4l2-fwnode.c | 4 ++++
> include/media/v4l2-mediabus.h | 2 ++
> 2 files changed, 6 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> index 3f77aa3..6105191 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -154,6 +154,10 @@ static void v4l2_fwnode_endpoint_parse_parallel_bus(
> flags |= v ? V4L2_MBUS_VIDEO_SOG_ACTIVE_HIGH :
> V4L2_MBUS_VIDEO_SOG_ACTIVE_LOW;
>
> + if (!fwnode_property_read_u32(fwnode, "data-enable-active", &v))
> + flags |= v ? V4L2_MBUS_DATA_ENABLE_HIGH :
> + V4L2_MBUS_DATA_ENABLE_LOW;
> +
> bus->flags = flags;
>
> }
> diff --git a/include/media/v4l2-mediabus.h b/include/media/v4l2-mediabus.h
> index 4d8626c..4bbb5f3 100644
> --- a/include/media/v4l2-mediabus.h
> +++ b/include/media/v4l2-mediabus.h
> @@ -45,6 +45,8 @@
> /* Active state of Sync-on-green (SoG) signal, 0/1 for LOW/HIGH respectively. */
> #define V4L2_MBUS_VIDEO_SOG_ACTIVE_HIGH BIT(12)
> #define V4L2_MBUS_VIDEO_SOG_ACTIVE_LOW BIT(13)
> +#define V4L2_MBUS_DATA_ENABLE_HIGH BIT(14)
> +#define V4L2_MBUS_DATA_ENABLE_LOW BIT(15)
>
> /* Serial flags */
> /* How many lanes the client can use */
> --
> 2.7.4
>
--
Regards,
Niklas S?derlund
^ permalink raw reply
* [PATCH] rtc: sunxi: fix possible race condition
From: Alexandre Belloni @ 2018-06-04 12:05 UTC (permalink / raw)
To: linux-arm-kernel
The IRQ is requested before the struct rtc is allocated and registered, but
this struct is used in the IRQ handler. This may lead to a NULL pointer
dereference.
Switch to devm_rtc_allocate_device/rtc_register_device to allocate the rtc
before requesting the IRQ.
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
drivers/rtc/rtc-sunxi.c | 23 +++++++++--------------
1 file changed, 9 insertions(+), 14 deletions(-)
diff --git a/drivers/rtc/rtc-sunxi.c b/drivers/rtc/rtc-sunxi.c
index dadbf8b324ad..21865d3d8fe8 100644
--- a/drivers/rtc/rtc-sunxi.c
+++ b/drivers/rtc/rtc-sunxi.c
@@ -445,6 +445,10 @@ static int sunxi_rtc_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, chip);
chip->dev = &pdev->dev;
+ chip->rtc = devm_rtc_allocate_device(&pdev->dev);
+ if (IS_ERR(chip->rtc))
+ return PTR_ERR(chip->rtc);
+
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
chip->base = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(chip->base))
@@ -481,11 +485,12 @@ static int sunxi_rtc_probe(struct platform_device *pdev)
writel(SUNXI_ALRM_IRQ_STA_CNT_IRQ_PEND, chip->base +
SUNXI_ALRM_IRQ_STA);
- chip->rtc = rtc_device_register("rtc-sunxi", &pdev->dev,
- &sunxi_rtc_ops, THIS_MODULE);
- if (IS_ERR(chip->rtc)) {
+ chip->rtc->ops = &sunxi_rtc_ops;
+
+ ret = rtc_register_device(chip->rtc);
+ if (ret) {
dev_err(&pdev->dev, "unable to register device\n");
- return PTR_ERR(chip->rtc);
+ return ret;
}
dev_info(&pdev->dev, "RTC enabled\n");
@@ -493,18 +498,8 @@ static int sunxi_rtc_probe(struct platform_device *pdev)
return 0;
}
-static int sunxi_rtc_remove(struct platform_device *pdev)
-{
- struct sunxi_rtc_dev *chip = platform_get_drvdata(pdev);
-
- rtc_device_unregister(chip->rtc);
-
- return 0;
-}
-
static struct platform_driver sunxi_rtc_driver = {
.probe = sunxi_rtc_probe,
- .remove = sunxi_rtc_remove,
.driver = {
.name = "sunxi-rtc",
.of_match_table = sunxi_rtc_dt_ids,
--
2.17.1
^ permalink raw reply related
* [PATCH 5/7] iommu/dma: add support for non-strict mode
From: Leizhen (ThunderTown) @ 2018-06-04 12:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <65cfe2f9-eb23-d81c-270e-ae80e96b6009@arm.com>
On 2018/5/31 21:04, Robin Murphy wrote:
> On 31/05/18 08:42, Zhen Lei wrote:
>> 1. Save the related domain pointer in struct iommu_dma_cookie, make iovad
>> capable call domain->ops->flush_iotlb_all to flush TLB.
>> 2. Define a new iommu capable: IOMMU_CAP_NON_STRICT, which used to indicate
>> that the iommu domain support non-strict mode.
>> 3. During the iommu domain initialization phase, call capable() to check
>> whether it support non-strcit mode. If so, call init_iova_flush_queue
>> to register iovad->flush_cb callback.
>> 4. All unmap(contains iova-free) APIs will finally invoke __iommu_dma_unmap
>> -->iommu_dma_free_iova. Use iovad->flush_cb to check whether its related
>> iommu support non-strict mode or not, and call IOMMU_DOMAIN_IS_STRICT to
>> make sure the IOMMU_DOMAIN_UNMANAGED domain always follow strict mode.
>
> Once again, this is a whole load of complexity for a property which could just be statically encoded at allocation, e.g. in the cookie type.
That's right. Pass domain to the static function iommu_dma_free_iova will be better.
>
>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
>> ---
>> drivers/iommu/dma-iommu.c | 29 ++++++++++++++++++++++++++---
>> include/linux/iommu.h | 3 +++
>> 2 files changed, 29 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
>> index 4e885f7..2e116d9 100644
>> --- a/drivers/iommu/dma-iommu.c
>> +++ b/drivers/iommu/dma-iommu.c
>> @@ -55,6 +55,7 @@ struct iommu_dma_cookie {
>> };
>> struct list_head msi_page_list;
>> spinlock_t msi_lock;
>> + struct iommu_domain *domain;
>> };
>> static inline size_t cookie_msi_granule(struct iommu_dma_cookie *cookie)
>> @@ -64,7 +65,8 @@ static inline size_t cookie_msi_granule(struct iommu_dma_cookie *cookie)
>> return PAGE_SIZE;
>> }
>> -static struct iommu_dma_cookie *cookie_alloc(enum iommu_dma_cookie_type type)
>> +static struct iommu_dma_cookie *cookie_alloc(struct iommu_domain *domain,
>> + enum iommu_dma_cookie_type type)
>> {
>> struct iommu_dma_cookie *cookie;
>> @@ -73,6 +75,7 @@ static struct iommu_dma_cookie *cookie_alloc(enum iommu_dma_cookie_type type)
>> spin_lock_init(&cookie->msi_lock);
>> INIT_LIST_HEAD(&cookie->msi_page_list);
>> cookie->type = type;
>> + cookie->domain = domain;
>> }
>> return cookie;
>> }
>> @@ -94,7 +97,7 @@ int iommu_get_dma_cookie(struct iommu_domain *domain)
>> if (domain->iova_cookie)
>> return -EEXIST;
>> - domain->iova_cookie = cookie_alloc(IOMMU_DMA_IOVA_COOKIE);
>> + domain->iova_cookie = cookie_alloc(domain, IOMMU_DMA_IOVA_COOKIE);
>> if (!domain->iova_cookie)
>> return -ENOMEM;
>> @@ -124,7 +127,7 @@ int iommu_get_msi_cookie(struct iommu_domain *domain, dma_addr_t base)
>> if (domain->iova_cookie)
>> return -EEXIST;
>> - cookie = cookie_alloc(IOMMU_DMA_MSI_COOKIE);
>> + cookie = cookie_alloc(domain, IOMMU_DMA_MSI_COOKIE);
>> if (!cookie)
>> return -ENOMEM;
>> @@ -261,6 +264,17 @@ static int iova_reserve_iommu_regions(struct device *dev,
>> return ret;
>> }
>> +static void iova_flush_iotlb_all(struct iova_domain *iovad)
>
> iommu_dma_flush...
OK
>
>> +{
>> + struct iommu_dma_cookie *cookie;
>> + struct iommu_domain *domain;
>> +
>> + cookie = container_of(iovad, struct iommu_dma_cookie, iovad);
>> + domain = cookie->domain;
>> +
>> + domain->ops->flush_iotlb_all(domain);
>> +}
>> +
>> /**
>> * iommu_dma_init_domain - Initialise a DMA mapping domain
>> * @domain: IOMMU domain previously prepared by iommu_get_dma_cookie()
>> @@ -276,6 +290,7 @@ static int iova_reserve_iommu_regions(struct device *dev,
>> int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
>> u64 size, struct device *dev)
>> {
>> + const struct iommu_ops *ops = domain->ops;
>> struct iommu_dma_cookie *cookie = domain->iova_cookie;
>> struct iova_domain *iovad = &cookie->iovad;
>> unsigned long order, base_pfn, end_pfn;
>> @@ -313,6 +328,11 @@ int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
>> init_iova_domain(iovad, 1UL << order, base_pfn);
>> + if (ops->capable && ops->capable(IOMMU_CAP_NON_STRICT)) {
>> + BUG_ON(!ops->flush_iotlb_all);
>> + init_iova_flush_queue(iovad, iova_flush_iotlb_all, NULL);
>> + }
>> +
>> return iova_reserve_iommu_regions(dev, domain);
>> }
>> EXPORT_SYMBOL(iommu_dma_init_domain);
>> @@ -392,6 +412,9 @@ static void iommu_dma_free_iova(struct iommu_dma_cookie *cookie,
>> /* The MSI case is only ever cleaning up its most recent allocation */
>> if (cookie->type == IOMMU_DMA_MSI_COOKIE)
>> cookie->msi_iova -= size;
>> + else if (!IOMMU_DOMAIN_IS_STRICT(cookie->domain) && iovad->flush_cb)
>> + queue_iova(iovad, iova_pfn(iovad, iova),
>> + size >> iova_shift(iovad), 0);
>> else
>> free_iova_fast(iovad, iova_pfn(iovad, iova),
>> size >> iova_shift(iovad));
>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>> index 39b3150..01ff569 100644
>> --- a/include/linux/iommu.h
>> +++ b/include/linux/iommu.h
>> @@ -87,6 +87,8 @@ struct iommu_domain_geometry {
>> __IOMMU_DOMAIN_DMA_API)
>> #define IOMMU_STRICT 1
>> +#define IOMMU_DOMAIN_IS_STRICT(domain) \
>> + (domain->type == IOMMU_DOMAIN_UNMANAGED)
>> struct iommu_domain {
>> unsigned type;
>> @@ -103,6 +105,7 @@ enum iommu_cap {
>> transactions */
>> IOMMU_CAP_INTR_REMAP, /* IOMMU supports interrupt isolation */
>> IOMMU_CAP_NOEXEC, /* IOMMU_NOEXEC flag */
>> + IOMMU_CAP_NON_STRICT, /* IOMMU supports non-strict mode */
>
> This isn't a property of the IOMMU, it depends purely on the driver implementation. I think it also doesn't matter anyway - if a caller asks for lazy unmapping on their domain but the IOMMU driver just does strict unmaps anyway because that's all it supports, there's no actual harm done.
>
> Robin.
>
>> };
>> /*
>>
>
> .
>
--
Thanks!
BestRegards
^ permalink raw reply
* [PATCH 06/15] drm/sun4i: tcon: Add support for tcon-top
From: Maxime Ripard @ 2018-06-04 11:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGb2v667pdjfHXYpBk1ER5sC8vgpcaOFKEbEByWoD1zh9Z0cyg@mail.gmail.com>
On Fri, Jun 01, 2018 at 09:19:43AM -0700, Chen-Yu Tsai wrote:
> On Fri, Jun 1, 2018 at 8:29 AM, Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > On Thu, May 31, 2018 at 07:54:08PM +0200, Jernej ?krabec wrote:
> >> Dne ?etrtek, 31. maj 2018 ob 11:21:33 CEST je Maxime Ripard napisal(a):
> >> > On Thu, May 24, 2018 at 03:01:09PM -0700, Chen-Yu Tsai wrote:
> >> > > >> > > + if (tcon->quirks->needs_tcon_top) {
> >> > > >> > > + struct device_node *np;
> >> > > >> > > +
> >> > > >> > > + np = of_parse_phandle(dev->of_node, "allwinner,tcon-top",
> >> > > >> > > 0);
> >> > > >> > > + if (np) {
> >> > > >> > > + struct platform_device *pdev;
> >> > > >> > > +
> >> > > >> > > + pdev = of_find_device_by_node(np);
> >> > > >> > > + if (pdev)
> >> > > >> > > + tcon->tcon_top =
> >> > > >> > > platform_get_drvdata(pdev);
> >> > > >> > > + of_node_put(np);
> >> > > >> > > +
> >> > > >> > > + if (!tcon->tcon_top)
> >> > > >> > > + return -EPROBE_DEFER;
> >> > > >> > > + }
> >> > > >> > > + }
> >> > > >> > > +
> >> > > >> >
> >> > > >> > I might have missed it, but I've not seen the bindings additions for
> >> > > >> > that property. This shouldn't really be done that way anyway, instead
> >> > > >> > of using a direct phandle, you should be using the of-graph, with the
> >> > > >> > TCON-top sitting where it belongs in the flow of data.
> >> > > >>
> >> > > >> Just to answer to the first question, it did describe it in "[PATCH
> >> > > >> 07/15] dt- bindings: display: sun4i-drm: Add R40 HDMI pipeline".
> >> > > >>
> >> > > >> As why I designed it that way - HW representation could be described
> >> > > >> that way> >>
> >> > > >> (ASCII art makes sense when fixed width font is used to view it):
> >> > > >> / LCD0/LVDS0
> >> > > >>
> >> > > >> / TCON-LCD0
> >> > > >>
> >> > > >> | \ MIPI DSI
> >> > > >>
> >> > > >> mixer0 |
> >> > > >>
> >> > > >> \ / TCON-LCD1 - LCD1/LVDS1
> >> > > >>
> >> > > >> TCON-TOP
> >> > > >>
> >> > > >> / \ TCON-TV0 - TVE0/RGB
> >> > > >>
> >> > > >> mixer1 | \
> >> > > >>
> >> > > >> | TCON-TOP - HDMI
> >> > > >> |
> >> > > >> | /
> >> > > >>
> >> > > >> \ TCON-TV1 - TVE1/RGB
> >> > > >>
> >> > > >> This is a bit simplified, since there is also TVE-TOP, which is
> >> > > >> responsible
> >> > > >> for sharing 4 DACs between both TVE encoders. You can have two TV outs
> >> > > >> (PAL/ NTSC) or TVE0 as TV out and TVE1 as RGB or vice versa. It even
> >> > > >> seems that you can arbitrarly choose which DAC is responsible for
> >> > > >> which signal, so there is a ton of possible end combinations, but I'm
> >> > > >> not 100% sure.
> >> > > >>
> >> > > >> Even though I wrote TCON-TOP twice, this is same unit in HW. R40 manual
> >> > > >> suggest more possibilities, although some of them seem wrong, like RGB
> >> > > >> feeding from LCD TCON. That is confirmed to be wrong when checking BSP
> >> > > >> code.
> >> > > >>
> >> > > >> Additionally, TCON-TOP comes in the middle of TVE0 and LCD0, TVE1 and
> >> > > >> LCD1 for pin muxing, although I'm not sure why is that needed at all,
> >> > > >> since according to R40 datasheet, TVE0 and TVE1 pins are dedicated and
> >> > > >> not on PORT D and PORT H, respectively, as TCON-TOP documentation
> >> > > >> suggest. However, HSYNC and PSYNC lines might be shared between TVE
> >> > > >> (when it works in RGB mode) and LCD. But that is just my guess since
> >> > > >> I'm not really familiar with RGB and LCD interfaces.
> >> > > >>
> >> > > >> I'm really not sure what would be the best representation in OF-graph.
> >> > > >> Can you suggest one?
> >> > > >
> >> > > > Rob might disagree on this one, but I don't see anything wrong with
> >> > > > having loops in the graph. If the TCON-TOP can be both the input and
> >> > > > output of the TCONs, then so be it, and have it described that way in
> >> > > > the graph.
> >> > > >
> >> > > > The code is already able to filter out nodes that have already been
> >> > > > added to the list of devices we need to wait for in the component
> >> > > > framework, so that should work as well.
> >> > > >
> >> > > > And we'd need to describe TVE-TOP as well, even though we don't have a
> >> > > > driver for it yet. That will simplify the backward compatibility later
> >> > > > on.
> >> > >
> >> > > I'm getting the feeling that TCON-TOP / TVE-TOP is the glue layer that
> >> > > binds everything together, and provides signal routing, kind of like
> >> > > DE-TOP on A64. So the signal mux controls that were originally found
> >> > > in TCON0 and TVE0 were moved out.
> >> > >
> >> > > The driver needs to know about that, but the graph about doesn't make
> >> > > much sense directly. Without looking at the manual, I understand it to
> >> > > likely be one mux between the mixers and TCONs, and one between the
> >> > > TCON-TVs and HDMI. Would it make more sense to just have the graph
> >> > > connections between the muxed components, and remove TCON-TOP from
> >> > > it, like we had in the past? A phandle could be used to reference
> >> > > the TCON-TOP for mux controls, in addition to the clocks and resets.
> >> > >
> >> > > For TVE, we would need something to represent each of the output pins,
> >> > > so the device tree can actually describe what kind of signal, be it
> >> > > each component of RGB/YUV or composite video, is wanted on each pin,
> >> > > if any. This is also needed on the A20 for the Cubietruck, so we can
> >> > > describe which pins are tied to the VGA connector, and which one does
> >> > > R, G, or B.
> >> >
> >> > I guess we'll see how the DT maintainers feel about this, but my
> >> > impression is that the OF graph should model the flow of data between
> >> > the devices. If there's a mux somewhere, then the data is definitely
> >> > going through it, and as such it should be part of the graph.
> >>
> >> I concur, but I spent few days thinking how to represent this sanely in graph,
> >> but I didn't find any good solution. I'll represent here my idea and please
> >> tell your opinion before I start implementing it.
> >>
> >> First, let me be clear that mixer0 and mixer1 don't have same capabilities
> >> (different number of planes, mixer0 supports writeback, mixer1 does not,
> >> etc.). Thus, it does matter which mixer is connected to which TCON/encoder.
> >> mixer0 is meant to be connected to main display and mixer1 to auxiliary. That
> >> obviously depends on end system.
> >>
> >> So, TCON TOP has 3 muxes, which have to be represented in graph. Two of them
> >> are for mixer/TCON relationship (each of them 1 input and 4 possible outputs)
> >> and one for TV TCON/HDMI pair selection (2 possible inputs, 1 output).
> >>
> >> According to current practice in sun4i-drm driver, graph has to have port 0,
> >> representing input and port 1, representing output. This would mean that graph
> >> looks something like that:
> >>
> >> tcon_top: tcon-top at 1c70000 {
> >> compatible = "allwinner,sun8i-r40-tcon-top";
> >> ...
> >> ports {
> >> #address-cells = <1>;
> >> #size-cells = <0>;
> >>
> >> tcon_top_in: port at 0 {
> >> #address-cells = <1>;
> >> #size-cells = <0>;
> >> reg = <0>;
> >>
> >> tcon_top_in_mixer0: endpoint at 0 {
> >> reg = <0>;
> >> remote-endpoint = <&mixer0_out_tcon_top>;
> >> };
> >>
> >> tcon_top_in_mixer1: endpoint at 1 {
> >> reg = <1>;
> >> remote-endpoint = <&mixer1_out_tcon_top>;
> >> };
> >>
> >> tcon_top_in_tcon_tv: endpoint at 2 {
> >> reg = <2>;
> >> // here is HDMI input connection, part of board DTS
> >> remote-endpoint = <board specific phandle to TV TCON output>;
> >> };
> >> };
> >>
> >> tcon_top_out: port at 1 {
> >> #address-cells = <1>;
> >> #size-cells = <0>;
> >> reg = <1>;
> >>
> >> tcon_top_out_tcon0: endpoint at 0 {
> >> reg = <0>;
> >> // here is mixer0 output connection, part of board DTS
> >> remote-endpoint = <board specific phandle to TCON>;
> >> };
> >>
> >> tcon_top_out_tcon1: endpoint at 1 {
> >> reg = <1>;
> >> // here is mixer1 output connection, part of board DTS
> >> remote-endpoint = <board specific phandle to TCON>;
> >> };
> >>
> >> tcon_top_out_hdmi: endpoint at 2 {
> >> reg = <2>;
> >> remote-endpoint = <&hdmi_in_tcon_top>;
> >> };
> >> };
> >> };
> >> };
> >
> > IIRC, each port is supposed to be one route for the data, so we would
> > have multiple ports, one for the mixers in input and for the tcon in
> > output, and one for the TCON in input and for the HDMI/TV in
> > output. Rob might correct me here.
> >
> >> tcon_tv0: lcd-controller at 1c73000 {
> >> compatible = "allwinner,sun8i-r40-tcon-tv-0";
> >> ...
> >> ports {
> >> #address-cells = <1>;
> >> #size-cells = <0>;
> >>
> >> tcon_tv0_in: port at 0 {
> >> reg = <0>;
> >>
> >> tcon_tv0_in_tcon_top: endpoint {
> >> // endpoint depends on board, part of board DTS
> >> remote-endpoint = <phandle to one of tcon_top_out_tcon>;
> >
> > Just curious, what would be there?
> >
> >> };
> >> };
> >>
> >> tcon_tv0_out: port at 1 {
> >> #address-cells = <1>;
> >> #size-cells = <0>;
> >> reg = <1>;
> >>
> >> // endpoints to TV TOP and TCON TOP HDMI input
> >> ...
> >> };
> >> };
> >> };
> >>
> >> tcon_tv1: lcd-controller at 1c74000 {
> >> compatible = "allwinner,sun8i-r40-tcon-tv-1";
> >> ...
> >> ports {
> >> #address-cells = <1>;
> >> #size-cells = <0>;
> >>
> >> tcon_tv1_in: port at 0 {
> >> reg = <0>;
> >>
> >> tcon_tv1_in_tcon_top: endpoint {
> >> // endpoint depends on board, part of board DTS
> >> remote-endpoint = <phandle to one of tcon_top_out_tcon>;
> >> };
> >> };
> >>
> >> tcon_tv1_out: port at 1 {
> >> #address-cells = <1>;
> >> #size-cells = <0>;
> >> reg = <1>;
> >>
> >> // endpoints to TV TOP and TCON TOP HDMI input
> >> ...
> >> };
> >> };
> >> };
> >>
> >> tcon_lcd0 and tcon_lcd1 would have similar connections, except that for
> >> outputs would be LCD or LVDS panels or MIPI DSI encoder.
> >>
> >> Please note that each TCON (there are 4 of them) would need to have unique
> >> compatible and have HW index stored in quirks data. It would be used by TCON
> >> TOP driver for configuring muxes.
> >
> > Can't we use the port/endpoint ID instead? If the mux is the only
> > thing that changes, the compatible has no reason to. It's the same IP,
> > and the only thing that changes is something that is not part of that
> > IP.
>
> I agree. Endpoint IDs should provide that information. I'm still not
> sure How to encode multiple in/out mux groups in a device node though.
I guess we can do that through different ports?
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/20180604/39f48a03/attachment-0001.sig>
^ permalink raw reply
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