* [PATCH v4 07/13] mm/gup: Refactor record_subpages() to find 1st small page
From: peterx @ 2024-03-27 15:23 UTC (permalink / raw)
To: linux-mm, linux-kernel
Cc: Yang Shi, Kirill A . Shutemov, Mike Kravetz, John Hubbard,
Michael Ellerman, peterx, Andrew Jones, Muchun Song, linux-riscv,
linuxppc-dev, Christophe Leroy, Andrew Morton, Christoph Hellwig,
Lorenzo Stoakes, Matthew Wilcox, Rik van Riel, linux-arm-kernel,
Andrea Arcangeli, David Hildenbrand, Aneesh Kumar K . V,
Vlastimil Babka, James Houghton, Jason Gunthorpe, Mike Rapoport,
Axel Rasmussen
In-Reply-To: <20240327152332.950956-1-peterx@redhat.com>
From: Peter Xu <peterx@redhat.com>
All the fast-gup functions take a tail page to operate, always need to do
page mask calculations before feeding that into record_subpages().
Merge that logic into record_subpages(), so that it will do the nth_page()
calculation.
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
mm/gup.c | 25 ++++++++++++++-----------
1 file changed, 14 insertions(+), 11 deletions(-)
diff --git a/mm/gup.c b/mm/gup.c
index db35b056fc9a..c2881772216b 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -2779,13 +2779,16 @@ static int __gup_device_huge_pud(pud_t pud, pud_t *pudp, unsigned long addr,
}
#endif
-static int record_subpages(struct page *page, unsigned long addr,
- unsigned long end, struct page **pages)
+static int record_subpages(struct page *page, unsigned long sz,
+ unsigned long addr, unsigned long end,
+ struct page **pages)
{
+ struct page *start_page;
int nr;
+ start_page = nth_page(page, (addr & (sz - 1)) >> PAGE_SHIFT);
for (nr = 0; addr != end; nr++, addr += PAGE_SIZE)
- pages[nr] = nth_page(page, nr);
+ pages[nr] = nth_page(start_page, nr);
return nr;
}
@@ -2820,8 +2823,8 @@ static int gup_hugepte(pte_t *ptep, unsigned long sz, unsigned long addr,
/* hugepages are never "special" */
VM_BUG_ON(!pfn_valid(pte_pfn(pte)));
- page = nth_page(pte_page(pte), (addr & (sz - 1)) >> PAGE_SHIFT);
- refs = record_subpages(page, addr, end, pages + *nr);
+ page = pte_page(pte);
+ refs = record_subpages(page, sz, addr, end, pages + *nr);
folio = try_grab_folio(page, refs, flags);
if (!folio)
@@ -2894,8 +2897,8 @@ static int gup_huge_pmd(pmd_t orig, pmd_t *pmdp, unsigned long addr,
pages, nr);
}
- page = nth_page(pmd_page(orig), (addr & ~PMD_MASK) >> PAGE_SHIFT);
- refs = record_subpages(page, addr, end, pages + *nr);
+ page = pmd_page(orig);
+ refs = record_subpages(page, PMD_SIZE, addr, end, pages + *nr);
folio = try_grab_folio(page, refs, flags);
if (!folio)
@@ -2938,8 +2941,8 @@ static int gup_huge_pud(pud_t orig, pud_t *pudp, unsigned long addr,
pages, nr);
}
- page = nth_page(pud_page(orig), (addr & ~PUD_MASK) >> PAGE_SHIFT);
- refs = record_subpages(page, addr, end, pages + *nr);
+ page = pud_page(orig);
+ refs = record_subpages(page, PUD_SIZE, addr, end, pages + *nr);
folio = try_grab_folio(page, refs, flags);
if (!folio)
@@ -2978,8 +2981,8 @@ static int gup_huge_pgd(pgd_t orig, pgd_t *pgdp, unsigned long addr,
BUILD_BUG_ON(pgd_devmap(orig));
- page = nth_page(pgd_page(orig), (addr & ~PGDIR_MASK) >> PAGE_SHIFT);
- refs = record_subpages(page, addr, end, pages + *nr);
+ page = pgd_page(orig);
+ refs = record_subpages(page, PGDIR_SIZE, addr, end, pages + *nr);
folio = try_grab_folio(page, refs, flags);
if (!folio)
--
2.44.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v4 03/13] mm: Make HPAGE_PXD_* macros even if !THP
From: peterx @ 2024-03-27 15:23 UTC (permalink / raw)
To: linux-mm, linux-kernel
Cc: Yang Shi, Kirill A . Shutemov, Mike Kravetz, John Hubbard,
Michael Ellerman, peterx, Andrew Jones, Muchun Song, linux-riscv,
linuxppc-dev, Christophe Leroy, Andrew Morton, Christoph Hellwig,
Lorenzo Stoakes, Matthew Wilcox, Rik van Riel, linux-arm-kernel,
Andrea Arcangeli, David Hildenbrand, Aneesh Kumar K . V,
Vlastimil Babka, James Houghton, Jason Gunthorpe, Mike Rapoport,
Axel Rasmussen
In-Reply-To: <20240327152332.950956-1-peterx@redhat.com>
From: Peter Xu <peterx@redhat.com>
These macros can be helpful when we plan to merge hugetlb code into generic
code. Move them out and define them as long as PGTABLE_HAS_HUGE_LEAVES is
selected, because there are systems that only define HUGETLB_PAGE not THP.
One note here is HPAGE_PMD_SHIFT must be defined even if PMD_SHIFT is not
defined (e.g. !CONFIG_MMU case); it (or in other forms, like HPAGE_PMD_NR)
is already used in lots of common codes without ifdef guards. Use the old
trick to let complations work.
Here we only need to differenciate HPAGE_PXD_SHIFT definitions. All the
rest macros will be defined based on it. When at it, move HPAGE_PMD_NR /
HPAGE_PMD_ORDER over together.
Signed-off-by: Peter Xu <peterx@redhat.com>
---
include/linux/huge_mm.h | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
index 7576025db55d..d3bb25c39482 100644
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -64,9 +64,6 @@ ssize_t single_hugepage_flag_show(struct kobject *kobj,
enum transparent_hugepage_flag flag);
extern struct kobj_attribute shmem_enabled_attr;
-#define HPAGE_PMD_ORDER (HPAGE_PMD_SHIFT-PAGE_SHIFT)
-#define HPAGE_PMD_NR (1<<HPAGE_PMD_ORDER)
-
/*
* Mask of all large folio orders supported for anonymous THP; all orders up to
* and including PMD_ORDER, except order-0 (which is not "huge") and order-1
@@ -87,14 +84,25 @@ extern struct kobj_attribute shmem_enabled_attr;
#define thp_vma_allowable_order(vma, vm_flags, smaps, in_pf, enforce_sysfs, order) \
(!!thp_vma_allowable_orders(vma, vm_flags, smaps, in_pf, enforce_sysfs, BIT(order)))
-#ifdef CONFIG_TRANSPARENT_HUGEPAGE
+#ifdef CONFIG_PGTABLE_HAS_HUGE_LEAVES
#define HPAGE_PMD_SHIFT PMD_SHIFT
-#define HPAGE_PMD_SIZE ((1UL) << HPAGE_PMD_SHIFT)
+#define HPAGE_PUD_SHIFT PUD_SHIFT
+#else
+#define HPAGE_PMD_SHIFT ({ BUILD_BUG(); 0; })
+#define HPAGE_PUD_SHIFT ({ BUILD_BUG(); 0; })
+#endif
+
+#define HPAGE_PMD_ORDER (HPAGE_PMD_SHIFT-PAGE_SHIFT)
+#define HPAGE_PMD_NR (1<<HPAGE_PMD_ORDER)
#define HPAGE_PMD_MASK (~(HPAGE_PMD_SIZE - 1))
+#define HPAGE_PMD_SIZE ((1UL) << HPAGE_PMD_SHIFT)
-#define HPAGE_PUD_SHIFT PUD_SHIFT
-#define HPAGE_PUD_SIZE ((1UL) << HPAGE_PUD_SHIFT)
+#define HPAGE_PUD_ORDER (HPAGE_PUD_SHIFT-PAGE_SHIFT)
+#define HPAGE_PUD_NR (1<<HPAGE_PUD_ORDER)
#define HPAGE_PUD_MASK (~(HPAGE_PUD_SIZE - 1))
+#define HPAGE_PUD_SIZE ((1UL) << HPAGE_PUD_SHIFT)
+
+#ifdef CONFIG_TRANSPARENT_HUGEPAGE
extern unsigned long transparent_hugepage_flags;
extern unsigned long huge_anon_orders_always;
@@ -377,13 +385,6 @@ static inline bool thp_migration_supported(void)
}
#else /* CONFIG_TRANSPARENT_HUGEPAGE */
-#define HPAGE_PMD_SHIFT ({ BUILD_BUG(); 0; })
-#define HPAGE_PMD_MASK ({ BUILD_BUG(); 0; })
-#define HPAGE_PMD_SIZE ({ BUILD_BUG(); 0; })
-
-#define HPAGE_PUD_SHIFT ({ BUILD_BUG(); 0; })
-#define HPAGE_PUD_MASK ({ BUILD_BUG(); 0; })
-#define HPAGE_PUD_SIZE ({ BUILD_BUG(); 0; })
static inline bool folio_test_pmd_mappable(struct folio *folio)
{
--
2.44.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH RFC 0/3] mm/gup: consistently call it GUP-fast
From: Peter Xu @ 2024-03-27 15:21 UTC (permalink / raw)
To: David Hildenbrand
Cc: linux-kernel, Andrew Morton, Mike Rapoport, Jason Gunthorpe,
John Hubbard, linux-arm-kernel, loongarch, linux-mips,
linuxppc-dev, linux-s390, linux-sh, linux-mm, linux-perf-users,
linux-fsdevel, x86
In-Reply-To: <20240327130538.680256-1-david@redhat.com>
On Wed, Mar 27, 2024 at 02:05:35PM +0100, David Hildenbrand wrote:
> Some cleanups around function names, comments and the config option of
> "GUP-fast" -- GUP without "lock" safety belts on.
>
> With this cleanup it's easy to judge which functions are GUP-fast specific.
> We now consistently call it "GUP-fast", avoiding mixing it with "fast GUP",
> "lockless", or simply "gup" (which I always considered confusing in the
> ode).
>
> So the magic now happens in functions that contain "gup_fast", whereby
> gup_fast() is the entry point into that magic. Comments consistently
> reference either "GUP-fast" or "gup_fast()".
>
> Based on mm-unstable from today. I won't CC arch maintainers, but only
> arch mailing lists, to reduce noise.
>
> Tested on x86_64, cross compiled on a bunch of archs, whereby some of them
> don't properly even compile on mm-unstable anymore in my usual setup
> (alpha, arc, parisc64, sh) ... maybe the cross compilers are outdated,
> but there are no new ones around. Hm.
I'm not sure what config you tried there; as I am doing some build tests
recently, I found turning off CONFIG_SAMPLES + CONFIG_GCC_PLUGINS could
avoid a lot of issues, I think it's due to libc missing. But maybe not the
case there.
The series makes sense to me, the naming is confusing. Btw, thanks for
posting this as RFC. This definitely has a conflict with the other gup
series that I had; I'll post v4 of that shortly.
--
Peter Xu
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 1/1] arm64: mm: swap: support THP_SWAP on hardware with MTE
From: Ryan Roberts @ 2024-03-27 15:15 UTC (permalink / raw)
To: David Hildenbrand, Matthew Wilcox, Barry Song
Cc: catalin.marinas, will, akpm, hughd, linux-mm, linux-arm-kernel,
chrisl, mark.rutland, steven.price, Barry Song, Kemeng Shi,
Anshuman Khandual, Peter Collingbourne, Yosry Ahmed, Peter Xu,
Lorenzo Stoakes, Mike Rapoport (IBM), Aneesh Kumar K.V,
Rick Edgecombe
In-Reply-To: <46ba09c5-4186-4e03-81cc-ca27c0301fef@arm.com>
On 27/03/2024 15:13, Ryan Roberts wrote:
> On 27/03/2024 14:57, David Hildenbrand wrote:
>> On 27.03.24 15:53, Matthew Wilcox wrote:
>>> On Sat, Mar 23, 2024 at 12:41:36AM +1300, Barry Song wrote:
>>>> Commit d0637c505f8a1 ("arm64: enable THP_SWAP for arm64") brings up
>>>> THP_SWAP on ARM64, but it doesn't enable THP_SWP on hardware with
>>>> MTE as the MTE code works with the assumption tags save/restore is
>>>> always handling a folio with only one page.
>>>>
>>>> The limitation should be removed as more and more ARM64 SoCs have
>>>> this feature. Co-existence of MTE and THP_SWAP becomes more and
>>>> more important.
>>>>
>>>> This patch makes MTE tags saving support large folios, then we don't
>>>> need to split large folios into base pages for swapping out on ARM64
>>>> SoCs with MTE any more.
>>>
>>> Can we go further than this patch and only support PG_mte_tagged and
>>> PG_mte_lock on folio->flags instead of page->flags? We're down to using
Also, I wouldn't want to hold this patch up in order to do this extra work. I
think the 2 things are orthogonal? (supporting THP swap with MTE vs not using
tail page flags for MTE). Can we discuss and handle it separately?
>>
>> I think we discussed that already and what I learned is that it "gets a bit
>> complicated". But I'm hoping we can get that discussion started again.
>
> The original conversation starts here:
> https://lore.kernel.org/linux-mm/fb34d312-1049-4932-8f2b-d7f33cfc297c@arm.com/
>
> The issue is that you can have a large folio mapped to user space, and user
> space only wants to activate MTE for a portion of it. So at that point, you
> either have to deal with only part of it being tagged (as we do today with the
> per-page flag) or you have to split the folio.
>
> I haven't re-read the entire thread - so might be forgetting some important details.
>
>>
>>> page->flags for these two MTE bits, a whole lot of s390 junk, PG_hwpoison,
>>> PG_head, PG_anon_exclusive and Zone/Section/Node/KASAN/last_cpupid.
>>
>> ... just like PG_anon_exclusive "gets a bit complicated". Well, I think I might
>> have finally found a way to make it work, I'll only have to uglify fork() a bit.
>>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 1/1] arm64: mm: swap: support THP_SWAP on hardware with MTE
From: Ryan Roberts @ 2024-03-27 15:13 UTC (permalink / raw)
To: David Hildenbrand, Matthew Wilcox, Barry Song
Cc: catalin.marinas, will, akpm, hughd, linux-mm, linux-arm-kernel,
chrisl, mark.rutland, steven.price, Barry Song, Kemeng Shi,
Anshuman Khandual, Peter Collingbourne, Yosry Ahmed, Peter Xu,
Lorenzo Stoakes, Mike Rapoport (IBM), Aneesh Kumar K.V,
Rick Edgecombe
In-Reply-To: <1006392c-c437-46c0-9a2e-e25e52236b67@redhat.com>
On 27/03/2024 14:57, David Hildenbrand wrote:
> On 27.03.24 15:53, Matthew Wilcox wrote:
>> On Sat, Mar 23, 2024 at 12:41:36AM +1300, Barry Song wrote:
>>> Commit d0637c505f8a1 ("arm64: enable THP_SWAP for arm64") brings up
>>> THP_SWAP on ARM64, but it doesn't enable THP_SWP on hardware with
>>> MTE as the MTE code works with the assumption tags save/restore is
>>> always handling a folio with only one page.
>>>
>>> The limitation should be removed as more and more ARM64 SoCs have
>>> this feature. Co-existence of MTE and THP_SWAP becomes more and
>>> more important.
>>>
>>> This patch makes MTE tags saving support large folios, then we don't
>>> need to split large folios into base pages for swapping out on ARM64
>>> SoCs with MTE any more.
>>
>> Can we go further than this patch and only support PG_mte_tagged and
>> PG_mte_lock on folio->flags instead of page->flags? We're down to using
>
> I think we discussed that already and what I learned is that it "gets a bit
> complicated". But I'm hoping we can get that discussion started again.
The original conversation starts here:
https://lore.kernel.org/linux-mm/fb34d312-1049-4932-8f2b-d7f33cfc297c@arm.com/
The issue is that you can have a large folio mapped to user space, and user
space only wants to activate MTE for a portion of it. So at that point, you
either have to deal with only part of it being tagged (as we do today with the
per-page flag) or you have to split the folio.
I haven't re-read the entire thread - so might be forgetting some important details.
>
>> page->flags for these two MTE bits, a whole lot of s390 junk, PG_hwpoison,
>> PG_head, PG_anon_exclusive and Zone/Section/Node/KASAN/last_cpupid.
>
> ... just like PG_anon_exclusive "gets a bit complicated". Well, I think I might
> have finally found a way to make it work, I'll only have to uglify fork() a bit.
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 12/14] sh: Add support for suppressing warning backtraces
From: Guenter Roeck @ 2024-03-27 15:10 UTC (permalink / raw)
To: Simon Horman
Cc: linux-kselftest, David Airlie, Arnd Bergmann, Maíra Canal,
Dan Carpenter, Kees Cook, Daniel Diaz, David Gow, Arthur Grillo,
Brendan Higgins, Naresh Kamboju, Maarten Lankhorst, Andrew Morton,
Maxime Ripard, Ville Syrjälä, Daniel Vetter,
Thomas Zimmermann, dri-devel, kunit-dev, linux-arch,
linux-arm-kernel, linux-doc, linux-kernel, linux-parisc,
linuxppc-dev, linux-riscv, linux-s390, linux-sh, loongarch,
netdev, Linux Kernel Functional Testing
In-Reply-To: <20240327144431.GL403975@kernel.org>
On 3/27/24 07:44, Simon Horman wrote:
> On Mon, Mar 25, 2024 at 10:52:46AM -0700, Guenter Roeck wrote:
>> Add name of functions triggering warning backtraces to the __bug_table
>> object section to enable support for suppressing WARNING backtraces.
>>
>> To limit image size impact, the pointer to the function name is only added
>> to the __bug_table section if both CONFIG_KUNIT_SUPPRESS_BACKTRACE and
>> CONFIG_DEBUG_BUGVERBOSE are enabled. Otherwise, the __func__ assembly
>> parameter is replaced with a (dummy) NULL parameter to avoid an image size
>> increase due to unused __func__ entries (this is necessary because __func__
>> is not a define but a virtual variable).
>>
>> Tested-by: Linux Kernel Functional Testing <lkft@linaro.org>
>> Acked-by: Dan Carpenter <dan.carpenter@linaro.org>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>> - Rebased to v6.9-rc1
>> - Added Tested-by:, Acked-by:, and Reviewed-by: tags
>> - Introduced KUNIT_SUPPRESS_BACKTRACE configuration option
>>
>> arch/sh/include/asm/bug.h | 26 ++++++++++++++++++++++----
>> 1 file changed, 22 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/sh/include/asm/bug.h b/arch/sh/include/asm/bug.h
>> index 05a485c4fabc..470ce6567d20 100644
>> --- a/arch/sh/include/asm/bug.h
>> +++ b/arch/sh/include/asm/bug.h
>> @@ -24,21 +24,36 @@
>> * The offending file and line are encoded in the __bug_table section.
>> */
>> #ifdef CONFIG_DEBUG_BUGVERBOSE
>> +
>> +#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
>> +# define HAVE_BUG_FUNCTION
>> +# define __BUG_FUNC_PTR "\t.long %O2\n"
>> +#else
>> +# define __BUG_FUNC_PTR
>> +#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
>> +
>
> Hi Guenter,
>
> a minor nit from my side: this change results in a Kernel doc warning.
>
> .../bug.h:29: warning: expecting prototype for _EMIT_BUG_ENTRY(). Prototype was for HAVE_BUG_FUNCTION() instead
>
> Perhaps either the new code should be placed above the Kernel doc,
> or scripts/kernel-doc should be enhanced?
>
Thanks a lot for the feedback.
The definition block needs to be inside CONFIG_DEBUG_BUGVERBOSE,
so it would be a bit odd to move it above the documentation
just to make kerneldoc happy. I am not really sure that to do
about it.
I'll wait for comments from others before making any changes.
Thanks,
Guenter
>> #define _EMIT_BUG_ENTRY \
>> "\t.pushsection __bug_table,\"aw\"\n" \
>> "2:\t.long 1b, %O1\n" \
>> - "\t.short %O2, %O3\n" \
>> - "\t.org 2b+%O4\n" \
>> + __BUG_FUNC_PTR \
>> + "\t.short %O3, %O4\n" \
>> + "\t.org 2b+%O5\n" \
>> "\t.popsection\n"
>> #else
>> #define _EMIT_BUG_ENTRY \
>> "\t.pushsection __bug_table,\"aw\"\n" \
>> "2:\t.long 1b\n" \
>> - "\t.short %O3\n" \
>> - "\t.org 2b+%O4\n" \
>> + "\t.short %O4\n" \
>> + "\t.org 2b+%O5\n" \
>> "\t.popsection\n"
>> #endif
>>
>> +#ifdef HAVE_BUG_FUNCTION
>> +# define __BUG_FUNC __func__
>> +#else
>> +# define __BUG_FUNC NULL
>> +#endif
>> +
>> #define BUG() \
>> do { \
>> __asm__ __volatile__ ( \
>
> ...
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH RFC 3/3] mm: use "GUP-fast" instead "fast GUP" in remaining comments
From: Mike Rapoport @ 2024-03-27 15:03 UTC (permalink / raw)
To: David Hildenbrand
Cc: linux-kernel, Andrew Morton, Jason Gunthorpe, John Hubbard,
Peter Xu, linux-arm-kernel, loongarch, linux-mips, linuxppc-dev,
linux-s390, linux-sh, linux-mm, linux-perf-users, linux-fsdevel,
x86
In-Reply-To: <20240327130538.680256-4-david@redhat.com>
On Wed, Mar 27, 2024 at 02:05:38PM +0100, David Hildenbrand wrote:
> Let's fixup the remaining comments to consistently call that thing
> "GUP-fast". With this change, we consistently call it "GUP-fast".
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Mike Rapoport (IBM) <rppt@kernel.org>
> ---
> mm/filemap.c | 2 +-
> mm/khugepaged.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 387b394754fa..c668e11cd6ef 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -1810,7 +1810,7 @@ EXPORT_SYMBOL(page_cache_prev_miss);
> * C. Return the page to the page allocator
> *
> * This means that any page may have its reference count temporarily
> - * increased by a speculative page cache (or fast GUP) lookup as it can
> + * increased by a speculative page cache (or GUP-fast) lookup as it can
> * be allocated by another user before the RCU grace period expires.
> * Because the refcount temporarily acquired here may end up being the
> * last refcount on the page, any page allocation must be freeable by
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index 38830174608f..6972fa05132e 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -1169,7 +1169,7 @@ static int collapse_huge_page(struct mm_struct *mm, unsigned long address,
> * huge and small TLB entries for the same virtual address to
> * avoid the risk of CPU bugs in that area.
> *
> - * Parallel fast GUP is fine since fast GUP will back off when
> + * Parallel GUP-fast is fine since GUP-fast will back off when
> * it detects PMD is changed.
> */
> _pmd = pmdp_collapse_flush(vma, address, pmd);
> --
> 2.43.2
>
--
Sincerely yours,
Mike.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH RFC 2/3] mm/treewide: rename CONFIG_HAVE_FAST_GUP to CONFIG_HAVE_GUP_FAST
From: Mike Rapoport @ 2024-03-27 15:02 UTC (permalink / raw)
To: David Hildenbrand
Cc: linux-kernel, Andrew Morton, Jason Gunthorpe, John Hubbard,
Peter Xu, linux-arm-kernel, loongarch, linux-mips, linuxppc-dev,
linux-s390, linux-sh, linux-mm, linux-perf-users, linux-fsdevel,
x86
In-Reply-To: <20240327130538.680256-3-david@redhat.com>
On Wed, Mar 27, 2024 at 02:05:37PM +0100, David Hildenbrand wrote:
> Nowadays, we call it "GUP-fast", the external interface includes
> functions like "get_user_pages_fast()", and we renamed all internal
> functions to reflect that as well.
>
> Let's make the config option reflect that.
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Mike Rapoport (IBM) <rppt@kernel.org>
> ---
> arch/arm/Kconfig | 2 +-
> arch/arm64/Kconfig | 2 +-
> arch/loongarch/Kconfig | 2 +-
> arch/mips/Kconfig | 2 +-
> arch/powerpc/Kconfig | 2 +-
> arch/s390/Kconfig | 2 +-
> arch/sh/Kconfig | 2 +-
> arch/x86/Kconfig | 2 +-
> include/linux/rmap.h | 8 ++++----
> kernel/events/core.c | 4 ++--
> mm/Kconfig | 2 +-
> mm/gup.c | 6 +++---
> mm/internal.h | 2 +-
> 13 files changed, 19 insertions(+), 19 deletions(-)
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH net-next v5 10/10] net: ti: icssg-prueth: Add ICSSG Ethernet driver for AM65x SR1.0 platforms
From: Simon Horman @ 2024-03-27 15:03 UTC (permalink / raw)
To: Diogo Ivo
Cc: davem, edumazet, kuba, pabeni, danishanwar, rogerq, vigneshr,
arnd, wsa+renesas, vladimir.oltean, andrew, dan.carpenter, netdev,
linux-arm-kernel, jan.kiszka
In-Reply-To: <20240326110709.26165-11-diogo.ivo@siemens.com>
On Tue, Mar 26, 2024 at 11:07:00AM +0000, Diogo Ivo wrote:
> Add the PRUeth driver for the ICSSG subsystem found in AM65x SR1.0 devices.
> The main differences that set SR1.0 and SR2.0 apart are the missing TXPRU
> core in SR1.0, two extra DMA channels for management purposes and different
> firmware that needs to be configured accordingly.
>
> Based on the work of Roger Quadros, Vignesh Raghavendra and
> Grygorii Strashko in TI's 5.10 SDK [1].
>
> [1]: https://git.ti.com/cgit/ti-linux-kernel/ti-linux-kernel/tree/?h=ti-linux-5.10.y
>
> Co-developed-by: Jan Kiszka <jan.kiszka@siemens.com>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> Signed-off-by: Diogo Ivo <diogo.ivo@siemens.com>
> Reviewed-by: MD Danish Anwar <danishanwar@ti.com>
...
> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c b/drivers/net/ethernet/ti/icssg/icssg_prueth_sr1.c
...
> +static void prueth_tx_ts_sr1(struct prueth_emac *emac,
> + struct emac_tx_ts_response_sr1 *tsr)
> +{
> + struct skb_shared_hwtstamps ssh;
> + u32 hi_ts, lo_ts, cookie;
> + struct sk_buff *skb;
> + u64 ns;
> +
> + hi_ts = le32_to_cpu(tsr->hi_ts);
> + lo_ts = le32_to_cpu(tsr->lo_ts);
> +
> + ns = (u64)hi_ts << 32 | lo_ts;
> +
> + cookie = le32_to_cpu(tsr->cookie);
Hi,
le32_to_cpu() expects a __le32 argument.
But the type of tsr->hi_ts, tsr->lo_ts and tsr->cookie is u32.
This seems to indicate that host byte order values
are being used as little endian values, which does not seem correct.
Flagged by Sparse.
> + if (cookie >= PRUETH_MAX_TX_TS_REQUESTS) {
> + netdev_dbg(emac->ndev, "Invalid TX TS cookie 0x%x\n",
> + cookie);
> + return;
> + }
> +
> + skb = emac->tx_ts_skb[cookie];
> + emac->tx_ts_skb[cookie] = NULL; /* free slot */
> +
> + memset(&ssh, 0, sizeof(ssh));
> + ssh.hwtstamp = ns_to_ktime(ns);
> +
> + skb_tstamp_tx(skb, &ssh);
> + dev_consume_skb_any(skb);
> +}
...
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v1 0/3] Speed up boot with faster linear map creation
From: Ryan Roberts @ 2024-03-27 15:01 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Catalin Marinas, Will Deacon, Mark Rutland, David Hildenbrand,
Donald Dutile, Eric Chanudet, linux-arm-kernel, linux-kernel
In-Reply-To: <CAMj1kXGtNYce5cOwUc+X5ceyxLGzD1xUEx7JRXqg7+4XQMAORw@mail.gmail.com>
On 27/03/2024 13:36, Ard Biesheuvel wrote:
> On Wed, 27 Mar 2024 at 12:43, Ryan Roberts <ryan.roberts@arm.com> wrote:
>>
>> On 27/03/2024 10:09, Ard Biesheuvel wrote:
>>> Hi Ryan,
>>>
>>> On Tue, 26 Mar 2024 at 12:15, Ryan Roberts <ryan.roberts@arm.com> wrote:
>>>>
>>>> Hi All,
>>>>
>>>> It turns out that creating the linear map can take a significant proportion of
>>>> the total boot time, especially when rodata=full. And a large portion of the
>>>> time it takes to create the linear map is issuing TLBIs. This series reworks the
>>>> kernel pgtable generation code to significantly reduce the number of TLBIs. See
>>>> each patch for details.
>>>>
>>>> The below shows the execution time of map_mem() across a couple of different
>>>> systems with different RAM configurations. We measure after applying each patch
>>>> and show the improvement relative to base (v6.9-rc1):
>>>>
>>>> | Apple M2 VM | Ampere Altra| Ampere Altra| Ampere Altra
>>>> | VM, 16G | VM, 64G | VM, 256G | Metal, 512G
>>>> ---------------|-------------|-------------|-------------|-------------
>>>> | ms (%) | ms (%) | ms (%) | ms (%)
>>>> ---------------|-------------|-------------|-------------|-------------
>>>> base | 151 (0%) | 2191 (0%) | 8990 (0%) | 17443 (0%)
>>>> no-cont-remap | 77 (-49%) | 429 (-80%) | 1753 (-80%) | 3796 (-78%)
>>>> no-alloc-remap | 77 (-49%) | 375 (-83%) | 1532 (-83%) | 3366 (-81%)
>>>> lazy-unmap | 63 (-58%) | 330 (-85%) | 1312 (-85%) | 2929 (-83%)
>>>>
>>>> This series applies on top of v6.9-rc1. All mm selftests pass. I haven't yet
>>>> tested all VA size configs (although I don't anticipate any issues); I'll do
>>>> this as part of followup.
>>>>
>>>
>>> These are very nice results!
>>>
>>> Before digging into the details: do we still have a strong case for
>>> supporting contiguous PTEs and PMDs in these routines?
>>
>> We are currently using contptes and pmds for the linear map when rodata=[on|off]
>> IIRC?
>
> In principle, yes. In practice?
>
>> I don't see a need to remove the capability personally.
>>
>
> Since we are making changes here, it is a relevant question to ask imho.
>
>> Also I was talking with Mark R yesterday and he suggested that an even better
>> solution might be to create a temp pgtable that maps the linear map with pmds,
>> switch to it, then create the real pgtable that maps the linear map with ptes,
>> then switch to that. The benefit being that we can avoid the fixmap entirely
>> when creating the second pgtable - we think this would likely be significantly
>> faster still.
>>
>
> If this is going to be a temporary mapping for the duration of the
> initial population of the linear map page tables, we might just as
> well use a 1:1 TTBR0 mapping here, which would be completely disjoint
> from swapper. And we'd only need to map memory that is being used for
> page tables, so on those large systems we'd need to map only a small
> slice. Maybe it's time to bring back the memblock alloc limit so we
> can manage this more easily?
>
>> My second patch adds the infrastructure to make this possible. But your changes
>> for LPA2 make it significantly more effort; since that change we are now using
>> the swapper pgtable when we populate the linear map into it - the kernel is
>> already mapped and that isn't done in paging_init() anymore. So I'm not quite
>> sure how we can easily make that work at the moment.
>>
>
> I think a mix of the fixmap approach with a 1:1 map could work here:
> - use TTBR0 to create a temp 1:1 map of DRAM
> - map page tables lazily as they are allocated but using a coarse mapping
> - avoid all TLB maintenance except at the end when tearing down the 1:1 mapping.
Yes that could work I think. So to make sure I've understood:
- create a 1:1 map for all of DRAM using block and cont mappings where possible
- use memblock_phys_alloc_*() to allocate pgtable memory
- access via fixmap (should be minimal due to block mappings)
- install it in TTBR0
- create all the swapper mappings as normal (no block or cont mappings)
- use memblock_phys_alloc_*() to alloc pgtable memory
- phys address is also virtual address due to installed 1:1 map
- Remove 1:1 map from TTBR0
- memblock_phys_free() all the memory associated with 1:1 map
That sounds doable on top of the first 2 patches in this series - I'll have a
crack. The only missing piece is depth-first 1:1 map traversal to free the
tables. I'm guessing something already exists that I can repurpose?
Thanks,
Ryan
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 1/1] arm64: mm: swap: support THP_SWAP on hardware with MTE
From: David Hildenbrand @ 2024-03-27 14:57 UTC (permalink / raw)
To: Matthew Wilcox, Barry Song
Cc: catalin.marinas, will, akpm, hughd, linux-mm, linux-arm-kernel,
chrisl, mark.rutland, ryan.roberts, steven.price, Barry Song,
Kemeng Shi, Anshuman Khandual, Peter Collingbourne, Yosry Ahmed,
Peter Xu, Lorenzo Stoakes, Mike Rapoport (IBM), Aneesh Kumar K.V,
Rick Edgecombe
In-Reply-To: <ZgQy24OJqkILGRSO@casper.infradead.org>
On 27.03.24 15:53, Matthew Wilcox wrote:
> On Sat, Mar 23, 2024 at 12:41:36AM +1300, Barry Song wrote:
>> Commit d0637c505f8a1 ("arm64: enable THP_SWAP for arm64") brings up
>> THP_SWAP on ARM64, but it doesn't enable THP_SWP on hardware with
>> MTE as the MTE code works with the assumption tags save/restore is
>> always handling a folio with only one page.
>>
>> The limitation should be removed as more and more ARM64 SoCs have
>> this feature. Co-existence of MTE and THP_SWAP becomes more and
>> more important.
>>
>> This patch makes MTE tags saving support large folios, then we don't
>> need to split large folios into base pages for swapping out on ARM64
>> SoCs with MTE any more.
>
> Can we go further than this patch and only support PG_mte_tagged and
> PG_mte_lock on folio->flags instead of page->flags? We're down to using
I think we discussed that already and what I learned is that it "gets a
bit complicated". But I'm hoping we can get that discussion started again.
> page->flags for these two MTE bits, a whole lot of s390 junk, PG_hwpoison,
> PG_head, PG_anon_exclusive and Zone/Section/Node/KASAN/last_cpupid.
... just like PG_anon_exclusive "gets a bit complicated". Well, I think
I might have finally found a way to make it work, I'll only have to
uglify fork() a bit.
--
Cheers,
David / dhildenb
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 1/1] arm64: mm: swap: support THP_SWAP on hardware with MTE
From: Matthew Wilcox @ 2024-03-27 14:53 UTC (permalink / raw)
To: Barry Song
Cc: catalin.marinas, will, akpm, hughd, linux-mm, linux-arm-kernel,
chrisl, mark.rutland, ryan.roberts, steven.price, david,
Barry Song, Kemeng Shi, Anshuman Khandual, Peter Collingbourne,
Yosry Ahmed, Peter Xu, Lorenzo Stoakes, Mike Rapoport (IBM),
Aneesh Kumar K.V, Rick Edgecombe
In-Reply-To: <20240322114136.61386-2-21cnbao@gmail.com>
On Sat, Mar 23, 2024 at 12:41:36AM +1300, Barry Song wrote:
> Commit d0637c505f8a1 ("arm64: enable THP_SWAP for arm64") brings up
> THP_SWAP on ARM64, but it doesn't enable THP_SWP on hardware with
> MTE as the MTE code works with the assumption tags save/restore is
> always handling a folio with only one page.
>
> The limitation should be removed as more and more ARM64 SoCs have
> this feature. Co-existence of MTE and THP_SWAP becomes more and
> more important.
>
> This patch makes MTE tags saving support large folios, then we don't
> need to split large folios into base pages for swapping out on ARM64
> SoCs with MTE any more.
Can we go further than this patch and only support PG_mte_tagged and
PG_mte_lock on folio->flags instead of page->flags? We're down to using
page->flags for these two MTE bits, a whole lot of s390 junk, PG_hwpoison,
PG_head, PG_anon_exclusive and Zone/Section/Node/KASAN/last_cpupid.
Looking at some of the callers, the call in copy_highpage() would need
to be lifted to its caller so that we only set the tags once per folio
rather than try to set them per page of a folio ... might be a bit of
churn, and I'd hate to try to do it myself without being able to test it.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v2 1/2] media: dt-binding: media: Document rk3588’s VEPU121
From: Emmanuel Gil Peyrot @ 2024-03-27 13:41 UTC (permalink / raw)
To: linux-kernel
Cc: Emmanuel Gil Peyrot, Ezequiel Garcia, Philipp Zabel,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiko Stuebner, Joerg Roedel, Will Deacon,
Robin Murphy, Sebastian Reichel, Cristian Ciocaltea, Dragan Simic,
Shreeya Patel, Chris Morgan, Andy Yan, Nicolas Frattaroli,
linux-media, linux-rockchip, devicetree, linux-arm-kernel, iommu
In-Reply-To: <20240327134115.424846-1-linkmauve@linkmauve.fr>
This encoder-only device is present four times on this SoC, and should
support everything the rk3568 vepu supports (so JPEG, H.264 and VP8
encoding).
According to the TRM[1], there is also the VEPU580 encoder which
supports H.264 and H.265, and various VDPU* decoders, of which only the
VDPU981 is currently supported. This patch describes only the VEPU121.
[1] https://github.com/FanX-Tek/rk3588-TRM-and-Datasheet
Signed-off-by: Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>
---
.../devicetree/bindings/media/rockchip,rk3568-vepu.yaml | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/media/rockchip,rk3568-vepu.yaml b/Documentation/devicetree/bindings/media/rockchip,rk3568-vepu.yaml
index 9d90d8d0565a..4c6cb21da041 100644
--- a/Documentation/devicetree/bindings/media/rockchip,rk3568-vepu.yaml
+++ b/Documentation/devicetree/bindings/media/rockchip,rk3568-vepu.yaml
@@ -15,8 +15,12 @@ description:
properties:
compatible:
- enum:
- - rockchip,rk3568-vepu
+ oneOf:
+ - const: rockchip,rk3568-vepu
+ - items:
+ - enum:
+ - rockchip,rk3588-vepu121
+ - const: rockchip,rk3568-vepu
reg:
maxItems: 1
--
2.44.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH 04/25] clk: meson: a1: add the audio clock controller driver
From: Jerome Brunet @ 2024-03-27 12:57 UTC (permalink / raw)
To: Jan Dakinevich
Cc: Jerome Brunet, Neil Armstrong, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel,
Kevin Hilman, Martin Blumenstingl, Liam Girdwood, Mark Brown,
Linus Walleij, Jaroslav Kysela, Takashi Iwai, linux-amlogic,
linux-clk, devicetree, linux-kernel, linux-arm-kernel, alsa-devel,
linux-sound, linux-gpio, kernel
In-Reply-To: <dc4ed700-623d-4823-9a41-de9be78afa64@salutedevices.com>
On Tue 26 Mar 2024 at 21:44, Jan Dakinevich <jan.dakinevich@salutedevices.com> wrote:
> On 3/26/24 18:26, Jerome Brunet wrote:
>>
>> On Sat 23 Mar 2024 at 21:02, Jan Dakinevich <jan.dakinevich@salutedevices.com> wrote:
>>
>>> Jerome, I have reworked my driver reusing axg-audio code as most as I
>>> could and now I have one more question. Lets see on this definition from
>>> axg-audio:
>>>
>>> #define AUD_MST_MUX(_name, _reg, _flag) \
>>> AUD_MUX(_name##_sel, _reg, 0x7, 24, _flag, \
>>> mst_mux_parent_data, 0)
>>>
>>> #define AUD_MST_MCLK_MUX(_name, _reg) \
>>> AUD_MST_MUX(_name, _reg, CLK_MUX_ROUND_CLOSEST)
>>>
>>> CLK_SET_RATE_PARENT is not set here. But why? It means, that topmost pll
>>> clock will not be reconfigured at runtime to satisfy the rate that was
>>> requested from axg-tdm.
>>>
>>
>> Yes, that is by design. It is another area where mainline audio differs
>> greatly from AML vendor code. The PLLs are expected be to fixed and the
>> audio master clock will reparent to the most adequate PLL source
>> depending on the use case.
>>
>> This is how we manage to satisfy all audio interfaces with a very
>> limited number of PLLs
>>
>> On AXG/G12 there is at most 6 concurrent interfaces (3 FRDDR/TODDR) - 8
>> on sm1 - and we can satisfy on that with 3 PLLs. That would not be
>> possible if interfaces were having their way with the PLLs, reseting it
>> everytime a stream is started.
>> > The PLL rate should be carefully chosen so it can be derived easily. On
>> AXG/G12/SM1 that is:
>> * one PLL per rate family, to maximize clock precision
>> * x24 x32: to handle different sample sizes
>> * x2 until we reach the PLL limits to allow higher rates such as 384kHz
>> or even higher
>>
>
> Thank you. Now it has become much clearer.
>
>> If you have less PLLs on A1, you'll have to make compromises, like a less
>> precise clock to support multiple family with one PLL.
>> This is why the PLLs are set for each platform in DT because that choice
>> may depend on the platform use case.
>>
>
> Unfortunately, on A1 we have only one PLL.
>
That where compromises comes in. Pick a rate known as 'audio friendly'
which match some rates and appromixate others, or use codec clock master.
> Yes, for us it would be better to have hifi_pll with predefined rate.
> For instance it will allow to avoid that ugly workaround in PDM (sysrate
> property, etc).
That is another problem entirely.
Krzysztof and I already covered what you should do for this.
The exact rate the PDM system clock does not matter at all because the
driver queries the actual rate of the clocks and adapts.
You have problems here only because you added CLK_SET_RATE_PARENT and
you are triggering the concurrent usage problem I explain below.
>
> But what whould be preferred for upstream? I can imagine a scenario
> where samples with different rate should be played, PDM attached to
> fclk_divN and there are no conflicts with TDM.
You are considering only PDM and 1 TDM. The SoC has 2 TDMs which could
be active concurrently a different rates.
> In this case
> reconfiguration of hifi_pll on demand could better satisfy somebody's
> requirements.
No this is not possible. Doing so does not allow all the interfaces to be
used concurrently.
* If the PLL is not protected, existing streams get broken by new
starting stream when the PLL is reconfigured
* If the PLL is protected, new stream effectively starve because no
clock may provide a 'good enough' rate for them
This is why the reference rate must carefully chosen, something CCF
cannot do for you.
For example, a source of 12.288MHz (and its powers of 2) allows to match
48 and 32kHz sample rates and approximate 44.1kHz rates with an
acceptable drift.
>
>>>
>>> On 3/19/24 11:30, Jerome Brunet wrote:
>>>>
>>>> On Tue 19 Mar 2024 at 04:47, Jan Dakinevich <jan.dakinevich@salutedevices.com> wrote:
>>>>
>>>>> Let's start from the end:
>>>>>
>>>>>> No - Looks to me you just have two clock controllers you are trying
>>>>> force into one.
>>>>>
>>>>>> Again, this shows 2 devices. The one related to your 'map0' should
>>>>> request AUD2_CLKID_AUDIOTOP as input and enable it right away.
>>>>>
>>>>> Most of fishy workarounds that you commented is caused the fact the mmio
>>>>> of this clock controller is divided into two parts. Compare it with
>>>>> axg-audio driver, things that was part of contigous memory region (like
>>>>> pdm) here are moved to second region. Is this enough to make a guess
>>>>> that these are two devices?
>>>>
>>>> I see obsolutely no reason to think it is a single device nor to add all the quirks
>>>> you have the way you did. So yes, in that case, 2 zones, 2 devices.
>>>>
>>>>>
>>>>> Concerning AUD2_CLKID_AUDIOTOP clock, as it turned out, it must be
>>>>> enabled before enabling of clocks from second region too. That is
>>>>> AUD2_CLKID_AUDIOTOP clock feeds both parts of this clock controller.
>>>>>
>>>>
>>>> Yes. I understood the first time around and already commented on that.
>>>>
>>>>>
>>>>> On 3/15/24 12:20, Jerome Brunet wrote:
>>>>>>
>>>>>> On Fri 15 Mar 2024 at 02:21, Jan Dakinevich <jan.dakinevich@salutedevices.com> wrote:
>>>>>>
>>>>>>> This controller provides clocks and reset functionality for audio
>>>>>>> peripherals on Amlogic A1 SoC family.
>>>>>>>
>>>>>>> The driver is almost identical to 'axg-audio', however it would be better
>>>>>>> to keep it separate due to following reasons:
>>>>>>>
>>>>>>> - significant amount of bits has another definition. I will bring there
>>>>>>> a mess of new defines with A1_ suffixes.
>>>>>>>
>>>>>>> - registers of this controller are located in two separate regions. It
>>>>>>> will give a lot of complications for 'axg-audio' to support this.
>>>>>>>
>>>>>>> Signed-off-by: Jan Dakinevich <jan.dakinevich@salutedevices.com>
>>>>>>> ---
>>>>>>> drivers/clk/meson/Kconfig | 13 +
>>>>>>> drivers/clk/meson/Makefile | 1 +
>>>>>>> drivers/clk/meson/a1-audio.c | 556 +++++++++++++++++++++++++++++++++++
>>>>>>> drivers/clk/meson/a1-audio.h | 58 ++++
>>>>>>> 4 files changed, 628 insertions(+)
>>>>>>> create mode 100644 drivers/clk/meson/a1-audio.c
>>>>>>> create mode 100644 drivers/clk/meson/a1-audio.h
>>>>>>>
>>>>>>> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
>>>>>>> index d6a2fa5f7e88..80c4a18c83d2 100644
>>>>>>> --- a/drivers/clk/meson/Kconfig
>>>>>>> +++ b/drivers/clk/meson/Kconfig
>>>>>>> @@ -133,6 +133,19 @@ config COMMON_CLK_A1_PERIPHERALS
>>>>>>> device, A1 SoC Family. Say Y if you want A1 Peripherals clock
>>>>>>> controller to work.
>>>>>>>
>>>>>>> +config COMMON_CLK_A1_AUDIO
>>>>>>> + tristate "Amlogic A1 SoC Audio clock controller support"
>>>>>>> + depends on ARM64
>>>>>>> + select COMMON_CLK_MESON_REGMAP
>>>>>>> + select COMMON_CLK_MESON_CLKC_UTILS
>>>>>>> + select COMMON_CLK_MESON_PHASE
>>>>>>> + select COMMON_CLK_MESON_SCLK_DIV
>>>>>>> + select COMMON_CLK_MESON_AUDIO_RSTC
>>>>>>> + help
>>>>>>> + Support for the Audio clock controller on Amlogic A113L based
>>>>>>> + device, A1 SoC Family. Say Y if you want A1 Audio clock controller
>>>>>>> + to work.
>>>>>>> +
>>>>>>> config COMMON_CLK_G12A
>>>>>>> tristate "G12 and SM1 SoC clock controllers support"
>>>>>>> depends on ARM64
>>>>>>> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
>>>>>>> index 88d94921a4dc..4968fc7ad555 100644
>>>>>>> --- a/drivers/clk/meson/Makefile
>>>>>>> +++ b/drivers/clk/meson/Makefile
>>>>>>> @@ -20,6 +20,7 @@ obj-$(CONFIG_COMMON_CLK_AXG) += axg.o axg-aoclk.o
>>>>>>> obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
>>>>>>> obj-$(CONFIG_COMMON_CLK_A1_PLL) += a1-pll.o
>>>>>>> obj-$(CONFIG_COMMON_CLK_A1_PERIPHERALS) += a1-peripherals.o
>>>>>>> +obj-$(CONFIG_COMMON_CLK_A1_AUDIO) += a1-audio.o
>>>>>>> obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o
>>>>>>> obj-$(CONFIG_COMMON_CLK_G12A) += g12a.o g12a-aoclk.o
>>>>>>> obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o meson8-ddr.o
>>>>>>> diff --git a/drivers/clk/meson/a1-audio.c b/drivers/clk/meson/a1-audio.c
>>>>>>> new file mode 100644
>>>>>>> index 000000000000..6039116c93ba
>>>>>>> --- /dev/null
>>>>>>> +++ b/drivers/clk/meson/a1-audio.c
>>>>>>> @@ -0,0 +1,556 @@
>>>>>>> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
>>>>>>> +/*
>>>>>>> + * Copyright (c) 2024, SaluteDevices. All Rights Reserved.
>>>>>>> + *
>>>>>>> + * Author: Jan Dakinevich <jan.dakinevich@salutedevices.com>
>>>>>>> + */
>>>>>>> +
>>>>>>> +#include <linux/clk.h>
>>>>>>> +#include <linux/clk-provider.h>
>>>>>>> +#include <linux/init.h>
>>>>>>> +#include <linux/of_device.h>
>>>>>>> +#include <linux/module.h>
>>>>>>> +#include <linux/platform_device.h>
>>>>>>> +#include <linux/regmap.h>
>>>>>>> +#include <linux/reset.h>
>>>>>>> +#include <linux/reset-controller.h>
>>>>>>> +#include <linux/slab.h>
>>>>>>> +
>>>>>>> +#include "meson-clkc-utils.h"
>>>>>>> +#include "meson-audio-rstc.h"
>>>>>>> +#include "clk-regmap.h"
>>>>>>> +#include "clk-phase.h"
>>>>>>> +#include "sclk-div.h"
>>>>>>> +#include "a1-audio.h"
>>>>>>> +
>>>>>>> +#define AUDIO_PDATA(_name) \
>>>>>>> + ((const struct clk_parent_data[]) { { .hw = &(_name).hw } })
>>>>>>
>>>>>> Not a fan - yet another level of macro.
>>>>>>
>>>>>>> +
>>>>>>> +#define AUDIO_MUX(_name, _reg, _mask, _shift, _pdata) \
>>>>>>> +static struct clk_regmap _name = { \
>>>>>>> + .map = AUDIO_REG_MAP(_reg), \
>>>>>>> + .data = &(struct clk_regmap_mux_data){ \
>>>>>>> + .offset = AUDIO_REG_OFFSET(_reg), \
>>>>>>> + .mask = (_mask), \
>>>>>>> + .shift = (_shift), \
>>>>>>> + }, \
>>>>>>> + .hw.init = &(struct clk_init_data) { \
>>>>>>> + .name = #_name, \
>>>>>>> + .ops = &clk_regmap_mux_ops, \
>>>>>>> + .parent_data = (_pdata), \
>>>>>>> + .num_parents = ARRAY_SIZE(_pdata), \
>>>>>>> + .flags = CLK_SET_RATE_PARENT, \
>>>>>>> + }, \
>>>>>>> +}
>>>>>>> +
>>>>>>> +#define AUDIO_DIV(_name, _reg, _shift, _width, _pdata) \
>>>>>>> +static struct clk_regmap _name = { \
>>>>>>> + .map = AUDIO_REG_MAP(_reg), \
>>>>>>> + .data = &(struct clk_regmap_div_data){ \
>>>>>>> + .offset = AUDIO_REG_OFFSET(_reg), \
>>>>>>> + .shift = (_shift), \
>>>>>>> + .width = (_width), \
>>>>>>> + }, \
>>>>>>> + .hw.init = &(struct clk_init_data) { \
>>>>>>> + .name = #_name, \
>>>>>>> + .ops = &clk_regmap_divider_ops, \
>>>>>>> + .parent_data = (_pdata), \
>>>>>>> + .num_parents = 1, \
>>>>>>> + .flags = CLK_SET_RATE_PARENT, \
>>>>>>> + }, \
>>>>>>> +}
>>>>>>> +
>>>>>>> +#define AUDIO_GATE(_name, _reg, _bit, _pdata) \
>>>>>>> +static struct clk_regmap _name = { \
>>>>>>> + .map = AUDIO_REG_MAP(_reg), \
>>>>>>> + .data = &(struct clk_regmap_gate_data){ \
>>>>>>> + .offset = AUDIO_REG_OFFSET(_reg), \
>>>>>>> + .bit_idx = (_bit), \
>>>>>>> + }, \
>>>>>>> + .hw.init = &(struct clk_init_data) { \
>>>>>>> + .name = #_name, \
>>>>>>> + .ops = &clk_regmap_gate_ops, \
>>>>>>> + .parent_data = (_pdata), \
>>>>>>> + .num_parents = 1, \
>>>>>>> + .flags = CLK_SET_RATE_PARENT, \
>>>>>>> + }, \
>>>>>>> +}
>>>>>>> +
>>>>>>> +#define AUDIO_SCLK_DIV(_name, _reg, _div_shift, _div_width, \
>>>>>>> + _hi_shift, _hi_width, _pdata, _set_rate_parent) \
>>>>>>> +static struct clk_regmap _name = { \
>>>>>>> + .map = AUDIO_REG_MAP(_reg), \
>>>>>>> + .data = &(struct meson_sclk_div_data) { \
>>>>>>> + .div = { \
>>>>>>> + .reg_off = AUDIO_REG_OFFSET(_reg), \
>>>>>>> + .shift = (_div_shift), \
>>>>>>> + .width = (_div_width), \
>>>>>>> + }, \
>>>>>>> + .hi = { \
>>>>>>> + .reg_off = AUDIO_REG_OFFSET(_reg), \
>>>>>>> + .shift = (_hi_shift), \
>>>>>>> + .width = (_hi_width), \
>>>>>>> + }, \
>>>>>>> + }, \
>>>>>>> + .hw.init = &(struct clk_init_data) { \
>>>>>>> + .name = #_name, \
>>>>>>> + .ops = &meson_sclk_div_ops, \
>>>>>>> + .parent_data = (_pdata), \
>>>>>>> + .num_parents = 1, \
>>>>>>> + .flags = (_set_rate_parent) ? CLK_SET_RATE_PARENT : 0, \
>>>>>>
>>>>>> Does not help readeability. Just pass the flag as axg-audio does.
>>>>>>
>>>>>>> + }, \
>>>>>>> +}
>>>>>>> +
>>>>>>> +#define AUDIO_TRIPHASE(_name, _reg, _width, _shift0, _shift1, _shift2, \
>>>>>>> + _pdata) \
>>>>>>> +static struct clk_regmap _name = { \
>>>>>>> + .map = AUDIO_REG_MAP(_reg), \
>>>>>>> + .data = &(struct meson_clk_triphase_data) { \
>>>>>>> + .ph0 = { \
>>>>>>> + .reg_off = AUDIO_REG_OFFSET(_reg), \
>>>>>>> + .shift = (_shift0), \
>>>>>>> + .width = (_width), \
>>>>>>> + }, \
>>>>>>> + .ph1 = { \
>>>>>>> + .reg_off = AUDIO_REG_OFFSET(_reg), \
>>>>>>> + .shift = (_shift1), \
>>>>>>> + .width = (_width), \
>>>>>>> + }, \
>>>>>>> + .ph2 = { \
>>>>>>> + .reg_off = AUDIO_REG_OFFSET(_reg), \
>>>>>>> + .shift = (_shift2), \
>>>>>>> + .width = (_width), \
>>>>>>> + }, \
>>>>>>> + }, \
>>>>>>> + .hw.init = &(struct clk_init_data) { \
>>>>>>> + .name = #_name, \
>>>>>>> + .ops = &meson_clk_triphase_ops, \
>>>>>>> + .parent_data = (_pdata), \
>>>>>>> + .num_parents = 1, \
>>>>>>> + .flags = CLK_SET_RATE_PARENT | CLK_DUTY_CYCLE_PARENT, \
>>>>>>> + }, \
>>>>>>> +}
>>>>>>> +
>>>>>>> +#define AUDIO_SCLK_WS(_name, _reg, _width, _shift_ph, _shift_ws, \
>>>>>>> + _pdata) \
>>>>>>> +static struct clk_regmap _name = { \
>>>>>>> + .map = AUDIO_REG_MAP(_reg), \
>>>>>>> + .data = &(struct meson_sclk_ws_inv_data) { \
>>>>>>> + .ph = { \
>>>>>>> + .reg_off = AUDIO_REG_OFFSET(_reg), \
>>>>>>> + .shift = (_shift_ph), \
>>>>>>> + .width = (_width), \
>>>>>>> + }, \
>>>>>>> + .ws = { \
>>>>>>> + .reg_off = AUDIO_REG_OFFSET(_reg), \
>>>>>>> + .shift = (_shift_ws), \
>>>>>>> + .width = (_width), \
>>>>>>> + }, \
>>>>>>> + }, \
>>>>>>> + .hw.init = &(struct clk_init_data) { \
>>>>>>> + .name = #_name, \
>>>>>>> + .ops = &meson_sclk_ws_inv_ops, \
>>>>>>> + .parent_data = (_pdata), \
>>>>>>> + .num_parents = 1, \
>>>>>>> + .flags = CLK_SET_RATE_PARENT | CLK_DUTY_CYCLE_PARENT, \
>>>>>>> + }, \
>>>>>>> +}
>>>>>>
>>>>>> All the above does essentially the same things as the macro of
>>>>>> axg-audio, to some minor differences. Yet it is another set to maintain.
>>>>>>
>>>>>
>>>>> Except one thing... Here I keep memory identifier to which this clock
>>>>> belongs:
>>>>>
>>>>> .map = AUDIO_REG_MAP(_reg),
>>>>>
>>>>> It is workaround, but ->map the only common field in clk_regmap that
>>>>> could be used for this purpose.
>>>>>
>>>>>
>>>>>> I'd much prefer if you put the axg-audio macro in a header a re-used
>>>>>> those. There would a single set to maintain. You may then specialize the
>>>>>> included in the driver C file, to avoid redundant parameters
>>>>>>
>>>>>> Rework axg-audio to use clk_parent_data if you must, but not in the same
>>>>>> series please.
>>>>>>
>>>>>>> +
>>>>>>> +static const struct clk_parent_data a1_pclk_pdata[] = {
>>>>>>> + { .fw_name = "pclk", },
>>>>>>> +};
>>>>>>> +
>>>>>>> +AUDIO_GATE(audio_ddr_arb, AUDIO_CLK_GATE_EN0, 0, a1_pclk_pdata);
>>>>>>> +AUDIO_GATE(audio_tdmin_a, AUDIO_CLK_GATE_EN0, 1, a1_pclk_pdata);
>>>>>>> +AUDIO_GATE(audio_tdmin_b, AUDIO_CLK_GATE_EN0, 2, a1_pclk_pdata);
>>>>>>> +AUDIO_GATE(audio_tdmin_lb, AUDIO_CLK_GATE_EN0, 3, a1_pclk_pdata);
>>>>>>> +AUDIO_GATE(audio_loopback, AUDIO_CLK_GATE_EN0, 4, a1_pclk_pdata);
>>>>>>> +AUDIO_GATE(audio_tdmout_a, AUDIO_CLK_GATE_EN0, 5, a1_pclk_pdata);
>>>>>>> +AUDIO_GATE(audio_tdmout_b, AUDIO_CLK_GATE_EN0, 6, a1_pclk_pdata);
>>>>>>> +AUDIO_GATE(audio_frddr_a, AUDIO_CLK_GATE_EN0, 7, a1_pclk_pdata);
>>>>>>> +AUDIO_GATE(audio_frddr_b, AUDIO_CLK_GATE_EN0, 8, a1_pclk_pdata);
>>>>>>> +AUDIO_GATE(audio_toddr_a, AUDIO_CLK_GATE_EN0, 9, a1_pclk_pdata);
>>>>>>> +AUDIO_GATE(audio_toddr_b, AUDIO_CLK_GATE_EN0, 10, a1_pclk_pdata);
>>>>>>> +AUDIO_GATE(audio_spdifin, AUDIO_CLK_GATE_EN0, 11, a1_pclk_pdata);
>>>>>>> +AUDIO_GATE(audio_resample, AUDIO_CLK_GATE_EN0, 12, a1_pclk_pdata);
>>>>>>> +AUDIO_GATE(audio_eqdrc, AUDIO_CLK_GATE_EN0, 13, a1_pclk_pdata);
>>>>>>> +AUDIO_GATE(audio_audiolocker, AUDIO_CLK_GATE_EN0, 14, a1_pclk_pdata);
>>>>>> This is what I mean by redundant parameter ^
>>>>>>
>>>>>
>>>>> Yep. I could define something like AUDIO_PCLK_GATE().
>>>>>
>>>>>>> +
>>>>>>> +AUDIO_GATE(audio2_ddr_arb, AUDIO2_CLK_GATE_EN0, 0, a1_pclk_pdata);
>>>>>>> +AUDIO_GATE(audio2_pdm, AUDIO2_CLK_GATE_EN0, 1, a1_pclk_pdata);
>>>>>>> +AUDIO_GATE(audio2_tdmin_vad, AUDIO2_CLK_GATE_EN0, 2, a1_pclk_pdata);
>>>>>>> +AUDIO_GATE(audio2_toddr_vad, AUDIO2_CLK_GATE_EN0, 3, a1_pclk_pdata);
>>>>>>> +AUDIO_GATE(audio2_vad, AUDIO2_CLK_GATE_EN0, 4, a1_pclk_pdata);
>>>>>>> +AUDIO_GATE(audio2_audiotop, AUDIO2_CLK_GATE_EN0, 7, a1_pclk_pdata);
>>>>>>> +
>>>>>>> +static const struct clk_parent_data a1_mst_pdata[] = {
>>>>>>> + { .fw_name = "dds_in" },
>>>>>>> + { .fw_name = "fclk_div2" },
>>>>>>> + { .fw_name = "fclk_div3" },
>>>>>>> + { .fw_name = "hifi_pll" },
>>>>>>> + { .fw_name = "xtal" },
>>>>>>> +};
>>>>>>> +
>>>>>>> +#define AUDIO_MST_MCLK(_name, _reg) \
>>>>>>> + AUDIO_MUX(_name##_mux, (_reg), 0x7, 24, a1_mst_pdata); \
>>>>>>> + AUDIO_DIV(_name##_div, (_reg), 0, 16, \
>>>>>>> + AUDIO_PDATA(_name##_mux)); \
>>>>>>> + AUDIO_GATE(_name, (_reg), 31, AUDIO_PDATA(_name##_div))
>>>>>>> +
>>>>>>> +AUDIO_MST_MCLK(audio_mst_a_mclk, AUDIO_MCLK_A_CTRL);
>>>>>>> +AUDIO_MST_MCLK(audio_mst_b_mclk, AUDIO_MCLK_B_CTRL);
>>>>>>> +AUDIO_MST_MCLK(audio_mst_c_mclk, AUDIO_MCLK_C_CTRL);
>>>>>>> +AUDIO_MST_MCLK(audio_mst_d_mclk, AUDIO_MCLK_D_CTRL);
>>>>>>> +AUDIO_MST_MCLK(audio_spdifin_clk, AUDIO_CLK_SPDIFIN_CTRL);
>>>>>>> +AUDIO_MST_MCLK(audio_eqdrc_clk, AUDIO_CLK_EQDRC_CTRL);
>>>>>>> +
>>>>>>> +AUDIO_MUX(audio_resample_clk_mux, AUDIO_CLK_RESAMPLE_CTRL, 0xf, 24,
>>>>>>> + a1_mst_pdata);
>>>>>>> +AUDIO_DIV(audio_resample_clk_div, AUDIO_CLK_RESAMPLE_CTRL, 0, 8,
>>>>>>> + AUDIO_PDATA(audio_resample_clk_mux));
>>>>>>> +AUDIO_GATE(audio_resample_clk, AUDIO_CLK_RESAMPLE_CTRL, 31,
>>>>>>> + AUDIO_PDATA(audio_resample_clk_div));
>>>>>>> +
>>>>>>> +AUDIO_MUX(audio_locker_in_clk_mux, AUDIO_CLK_LOCKER_CTRL, 0xf, 8,
>>>>>>> + a1_mst_pdata);
>>>>>>> +AUDIO_DIV(audio_locker_in_clk_div, AUDIO_CLK_LOCKER_CTRL, 0, 8,
>>>>>>> + AUDIO_PDATA(audio_locker_in_clk_mux));
>>>>>>> +AUDIO_GATE(audio_locker_in_clk, AUDIO_CLK_LOCKER_CTRL, 15,
>>>>>>> + AUDIO_PDATA(audio_locker_in_clk_div));
>>>>>>> +
>>>>>>> +AUDIO_MUX(audio_locker_out_clk_mux, AUDIO_CLK_LOCKER_CTRL, 0xf, 24,
>>>>>>> + a1_mst_pdata);
>>>>>>> +AUDIO_DIV(audio_locker_out_clk_div, AUDIO_CLK_LOCKER_CTRL, 16, 8,
>>>>>>> + AUDIO_PDATA(audio_locker_out_clk_mux));
>>>>>>> +AUDIO_GATE(audio_locker_out_clk, AUDIO_CLK_LOCKER_CTRL, 31,
>>>>>>> + AUDIO_PDATA(audio_locker_out_clk_div));
>>>>>>> +
>>>>>>> +AUDIO_MST_MCLK(audio2_vad_mclk, AUDIO2_MCLK_VAD_CTRL);
>>>>>>> +AUDIO_MST_MCLK(audio2_vad_clk, AUDIO2_CLK_VAD_CTRL);
>>>>>>> +AUDIO_MST_MCLK(audio2_pdm_dclk, AUDIO2_CLK_PDMIN_CTRL0);
>>>>>>> +AUDIO_MST_MCLK(audio2_pdm_sysclk, AUDIO2_CLK_PDMIN_CTRL1);
>>>>>>> +
>>>>>>> +#define AUDIO_MST_SCLK(_name, _reg0, _reg1, _pdata) \
>>>>>>> + AUDIO_GATE(_name##_pre_en, (_reg0), 31, (_pdata)); \
>>>>>>> + AUDIO_SCLK_DIV(_name##_div, (_reg0), 20, 10, 0, 0, \
>>>>>>> + AUDIO_PDATA(_name##_pre_en), true); \
>>>>>>> + AUDIO_GATE(_name##_post_en, (_reg0), 30, \
>>>>>>> + AUDIO_PDATA(_name##_div)); \
>>>>>>> + AUDIO_TRIPHASE(_name, (_reg1), 1, 0, 2, 4, \
>>>>>>> + AUDIO_PDATA(_name##_post_en))
>>>>>>> +
>>>>>>
>>>>>> Again, I'm not a fan of this many levels of macro. I can live with it
>>>>>> but certainly don't want the burden of reviewing and maintaining for
>>>>>> clock driver. AXG / G12 and A1 are obviously closely related, so make it common.
>>>>>>
>>>>>>> +#define AUDIO_MST_LRCLK(_name, _reg0, _reg1, _pdata) \
>>>>>>> + AUDIO_SCLK_DIV(_name##_div, (_reg0), 0, 10, 10, 10, \
>>>>>>> + (_pdata), false); \
>>>>>>> + AUDIO_TRIPHASE(_name, (_reg1), 1, 1, 3, 5, \
>>>>>>> + AUDIO_PDATA(_name##_div))
>>>>>>> +
>>>>>>> +AUDIO_MST_SCLK(audio_mst_a_sclk, AUDIO_MST_A_SCLK_CTRL0, AUDIO_MST_A_SCLK_CTRL1,
>>>>>>> + AUDIO_PDATA(audio_mst_a_mclk));
>>>>>>> +AUDIO_MST_SCLK(audio_mst_b_sclk, AUDIO_MST_B_SCLK_CTRL0, AUDIO_MST_B_SCLK_CTRL1,
>>>>>>> + AUDIO_PDATA(audio_mst_b_mclk));
>>>>>>> +AUDIO_MST_SCLK(audio_mst_c_sclk, AUDIO_MST_C_SCLK_CTRL0, AUDIO_MST_C_SCLK_CTRL1,
>>>>>>> + AUDIO_PDATA(audio_mst_c_mclk));
>>>>>>> +AUDIO_MST_SCLK(audio_mst_d_sclk, AUDIO_MST_D_SCLK_CTRL0, AUDIO_MST_D_SCLK_CTRL1,
>>>>>>> + AUDIO_PDATA(audio_mst_d_mclk));
>>>>>>> +
>>>>>>> +AUDIO_MST_LRCLK(audio_mst_a_lrclk, AUDIO_MST_A_SCLK_CTRL0, AUDIO_MST_A_SCLK_CTRL1,
>>>>>>> + AUDIO_PDATA(audio_mst_a_sclk_post_en));
>>>>>>> +AUDIO_MST_LRCLK(audio_mst_b_lrclk, AUDIO_MST_B_SCLK_CTRL0, AUDIO_MST_B_SCLK_CTRL1,
>>>>>>> + AUDIO_PDATA(audio_mst_b_sclk_post_en));
>>>>>>> +AUDIO_MST_LRCLK(audio_mst_c_lrclk, AUDIO_MST_C_SCLK_CTRL0, AUDIO_MST_C_SCLK_CTRL1,
>>>>>>> + AUDIO_PDATA(audio_mst_c_sclk_post_en));
>>>>>>> +AUDIO_MST_LRCLK(audio_mst_d_lrclk, AUDIO_MST_D_SCLK_CTRL0, AUDIO_MST_D_SCLK_CTRL1,
>>>>>>> + AUDIO_PDATA(audio_mst_d_sclk_post_en));
>>>>>>> +
>>>>>>> +static const struct clk_parent_data a1_mst_sclk_pdata[] = {
>>>>>>> + { .hw = &audio_mst_a_sclk.hw },
>>>>>>> + { .hw = &audio_mst_b_sclk.hw },
>>>>>>> + { .hw = &audio_mst_c_sclk.hw },
>>>>>>> + { .hw = &audio_mst_d_sclk.hw },
>>>>>>> + { .fw_name = "slv_sclk0" },
>>>>>>> + { .fw_name = "slv_sclk1" },
>>>>>>> + { .fw_name = "slv_sclk2" },
>>>>>>> + { .fw_name = "slv_sclk3" },
>>>>>>> + { .fw_name = "slv_sclk4" },
>>>>>>> + { .fw_name = "slv_sclk5" },
>>>>>>> + { .fw_name = "slv_sclk6" },
>>>>>>> + { .fw_name = "slv_sclk7" },
>>>>>>> + { .fw_name = "slv_sclk8" },
>>>>>>> + { .fw_name = "slv_sclk9" },
>>>>>>> +};
>>>>>>> +
>>>>>>> +static const struct clk_parent_data a1_mst_lrclk_pdata[] = {
>>>>>>> + { .hw = &audio_mst_a_lrclk.hw },
>>>>>>> + { .hw = &audio_mst_b_lrclk.hw },
>>>>>>> + { .hw = &audio_mst_c_lrclk.hw },
>>>>>>> + { .hw = &audio_mst_d_lrclk.hw },
>>>>>>> + { .fw_name = "slv_lrclk0" },
>>>>>>> + { .fw_name = "slv_lrclk1" },
>>>>>>> + { .fw_name = "slv_lrclk2" },
>>>>>>> + { .fw_name = "slv_lrclk3" },
>>>>>>> + { .fw_name = "slv_lrclk4" },
>>>>>>> + { .fw_name = "slv_lrclk5" },
>>>>>>> + { .fw_name = "slv_lrclk6" },
>>>>>>> + { .fw_name = "slv_lrclk7" },
>>>>>>> + { .fw_name = "slv_lrclk8" },
>>>>>>> + { .fw_name = "slv_lrclk9" },
>>>>>>> +};
>>>>>>> +
>>>>>>> +#define AUDIO_TDM_SCLK(_name, _reg) \
>>>>>>> + AUDIO_MUX(_name##_mux, (_reg), 0xf, 24, a1_mst_sclk_pdata); \
>>>>>>> + AUDIO_GATE(_name##_pre_en, (_reg), 31, \
>>>>>>> + AUDIO_PDATA(_name##_mux)); \
>>>>>>> + AUDIO_GATE(_name##_post_en, (_reg), 30, \
>>>>>>> + AUDIO_PDATA(_name##_pre_en)); \
>>>>>>> + AUDIO_SCLK_WS(_name, (_reg), 1, 29, 28, \
>>>>>>> + AUDIO_PDATA(_name##_post_en))
>>>>>>> +
>>>>>>> +#define AUDIO_TDM_LRCLK(_name, _reg) \
>>>>>>> + AUDIO_MUX(_name, (_reg), 0xf, 20, a1_mst_lrclk_pdata)
>>>>>>> +
>>>>>>> +AUDIO_TDM_SCLK(audio_tdmin_a_sclk, AUDIO_CLK_TDMIN_A_CTRL);
>>>>>>> +AUDIO_TDM_SCLK(audio_tdmin_b_sclk, AUDIO_CLK_TDMIN_B_CTRL);
>>>>>>> +AUDIO_TDM_SCLK(audio_tdmin_lb_sclk, AUDIO_CLK_TDMIN_LB_CTRL);
>>>>>>> +AUDIO_TDM_SCLK(audio_tdmout_a_sclk, AUDIO_CLK_TDMOUT_A_CTRL);
>>>>>>> +AUDIO_TDM_SCLK(audio_tdmout_b_sclk, AUDIO_CLK_TDMOUT_B_CTRL);
>>>>>>> +
>>>>>>> +AUDIO_TDM_LRCLK(audio_tdmin_a_lrclk, AUDIO_CLK_TDMIN_A_CTRL);
>>>>>>> +AUDIO_TDM_LRCLK(audio_tdmin_b_lrclk, AUDIO_CLK_TDMIN_B_CTRL);
>>>>>>> +AUDIO_TDM_LRCLK(audio_tdmin_lb_lrclk, AUDIO_CLK_TDMIN_LB_CTRL);
>>>>>>> +AUDIO_TDM_LRCLK(audio_tdmout_a_lrclk, AUDIO_CLK_TDMOUT_A_CTRL);
>>>>>>> +AUDIO_TDM_LRCLK(audio_tdmout_b_lrclk, AUDIO_CLK_TDMOUT_B_CTRL);
>>>>>>> +
>>>>>>> +static struct clk_hw *a1_audio_hw_clks[] = {
>>>>>>> + [AUD_CLKID_DDR_ARB] = &audio_ddr_arb.hw,
>>>>>>> + [AUD_CLKID_TDMIN_A] = &audio_tdmin_a.hw,
>>>>>>> + [AUD_CLKID_TDMIN_B] = &audio_tdmin_b.hw,
>>>>>>> + [AUD_CLKID_TDMIN_LB] = &audio_tdmin_lb.hw,
>>>>>>> + [AUD_CLKID_LOOPBACK] = &audio_loopback.hw,
>>>>>>> + [AUD_CLKID_TDMOUT_A] = &audio_tdmout_a.hw,
>>>>>>> + [AUD_CLKID_TDMOUT_B] = &audio_tdmout_b.hw,
>>>>>>> + [AUD_CLKID_FRDDR_A] = &audio_frddr_a.hw,
>>>>>>> + [AUD_CLKID_FRDDR_B] = &audio_frddr_b.hw,
>>>>>>> + [AUD_CLKID_TODDR_A] = &audio_toddr_a.hw,
>>>>>>> + [AUD_CLKID_TODDR_B] = &audio_toddr_b.hw,
>>>>>>> + [AUD_CLKID_SPDIFIN] = &audio_spdifin.hw,
>>>>>>> + [AUD_CLKID_RESAMPLE] = &audio_resample.hw,
>>>>>>> + [AUD_CLKID_EQDRC] = &audio_eqdrc.hw,
>>>>>>> + [AUD_CLKID_LOCKER] = &audio_audiolocker.hw,
>>>>>>> + [AUD_CLKID_MST_A_MCLK_SEL] = &audio_mst_a_mclk_mux.hw,
>>>>>>> + [AUD_CLKID_MST_A_MCLK_DIV] = &audio_mst_a_mclk_div.hw,
>>>>>>> + [AUD_CLKID_MST_A_MCLK] = &audio_mst_a_mclk.hw,
>>>>>>> + [AUD_CLKID_MST_B_MCLK_SEL] = &audio_mst_b_mclk_mux.hw,
>>>>>>> + [AUD_CLKID_MST_B_MCLK_DIV] = &audio_mst_b_mclk_div.hw,
>>>>>>> + [AUD_CLKID_MST_B_MCLK] = &audio_mst_b_mclk.hw,
>>>>>>> + [AUD_CLKID_MST_C_MCLK_SEL] = &audio_mst_c_mclk_mux.hw,
>>>>>>> + [AUD_CLKID_MST_C_MCLK_DIV] = &audio_mst_c_mclk_div.hw,
>>>>>>> + [AUD_CLKID_MST_C_MCLK] = &audio_mst_c_mclk.hw,
>>>>>>> + [AUD_CLKID_MST_D_MCLK_SEL] = &audio_mst_d_mclk_mux.hw,
>>>>>>> + [AUD_CLKID_MST_D_MCLK_DIV] = &audio_mst_d_mclk_div.hw,
>>>>>>> + [AUD_CLKID_MST_D_MCLK] = &audio_mst_d_mclk.hw,
>>>>>>> + [AUD_CLKID_RESAMPLE_CLK_SEL] = &audio_resample_clk_mux.hw,
>>>>>>> + [AUD_CLKID_RESAMPLE_CLK_DIV] = &audio_resample_clk_div.hw,
>>>>>>> + [AUD_CLKID_RESAMPLE_CLK] = &audio_resample_clk.hw,
>>>>>>> + [AUD_CLKID_LOCKER_IN_CLK_SEL] = &audio_locker_in_clk_mux.hw,
>>>>>>> + [AUD_CLKID_LOCKER_IN_CLK_DIV] = &audio_locker_in_clk_div.hw,
>>>>>>> + [AUD_CLKID_LOCKER_IN_CLK] = &audio_locker_in_clk.hw,
>>>>>>> + [AUD_CLKID_LOCKER_OUT_CLK_SEL] = &audio_locker_out_clk_mux.hw,
>>>>>>> + [AUD_CLKID_LOCKER_OUT_CLK_DIV] = &audio_locker_out_clk_div.hw,
>>>>>>> + [AUD_CLKID_LOCKER_OUT_CLK] = &audio_locker_out_clk.hw,
>>>>>>> + [AUD_CLKID_SPDIFIN_CLK_SEL] = &audio_spdifin_clk_mux.hw,
>>>>>>> + [AUD_CLKID_SPDIFIN_CLK_DIV] = &audio_spdifin_clk_div.hw,
>>>>>>> + [AUD_CLKID_SPDIFIN_CLK] = &audio_spdifin_clk.hw,
>>>>>>> + [AUD_CLKID_EQDRC_CLK_SEL] = &audio_eqdrc_clk_mux.hw,
>>>>>>> + [AUD_CLKID_EQDRC_CLK_DIV] = &audio_eqdrc_clk_div.hw,
>>>>>>> + [AUD_CLKID_EQDRC_CLK] = &audio_eqdrc_clk.hw,
>>>>>>> + [AUD_CLKID_MST_A_SCLK_PRE_EN] = &audio_mst_a_sclk_pre_en.hw,
>>>>>>> + [AUD_CLKID_MST_A_SCLK_DIV] = &audio_mst_a_sclk_div.hw,
>>>>>>> + [AUD_CLKID_MST_A_SCLK_POST_EN] = &audio_mst_a_sclk_post_en.hw,
>>>>>>> + [AUD_CLKID_MST_A_SCLK] = &audio_mst_a_sclk.hw,
>>>>>>> + [AUD_CLKID_MST_B_SCLK_PRE_EN] = &audio_mst_b_sclk_pre_en.hw,
>>>>>>> + [AUD_CLKID_MST_B_SCLK_DIV] = &audio_mst_b_sclk_div.hw,
>>>>>>> + [AUD_CLKID_MST_B_SCLK_POST_EN] = &audio_mst_b_sclk_post_en.hw,
>>>>>>> + [AUD_CLKID_MST_B_SCLK] = &audio_mst_b_sclk.hw,
>>>>>>> + [AUD_CLKID_MST_C_SCLK_PRE_EN] = &audio_mst_c_sclk_pre_en.hw,
>>>>>>> + [AUD_CLKID_MST_C_SCLK_DIV] = &audio_mst_c_sclk_div.hw,
>>>>>>> + [AUD_CLKID_MST_C_SCLK_POST_EN] = &audio_mst_c_sclk_post_en.hw,
>>>>>>> + [AUD_CLKID_MST_C_SCLK] = &audio_mst_c_sclk.hw,
>>>>>>> + [AUD_CLKID_MST_D_SCLK_PRE_EN] = &audio_mst_d_sclk_pre_en.hw,
>>>>>>> + [AUD_CLKID_MST_D_SCLK_DIV] = &audio_mst_d_sclk_div.hw,
>>>>>>> + [AUD_CLKID_MST_D_SCLK_POST_EN] = &audio_mst_d_sclk_post_en.hw,
>>>>>>> + [AUD_CLKID_MST_D_SCLK] = &audio_mst_d_sclk.hw,
>>>>>>> + [AUD_CLKID_MST_A_LRCLK_DIV] = &audio_mst_a_lrclk_div.hw,
>>>>>>> + [AUD_CLKID_MST_A_LRCLK] = &audio_mst_a_lrclk.hw,
>>>>>>> + [AUD_CLKID_MST_B_LRCLK_DIV] = &audio_mst_b_lrclk_div.hw,
>>>>>>> + [AUD_CLKID_MST_B_LRCLK] = &audio_mst_b_lrclk.hw,
>>>>>>> + [AUD_CLKID_MST_C_LRCLK_DIV] = &audio_mst_c_lrclk_div.hw,
>>>>>>> + [AUD_CLKID_MST_C_LRCLK] = &audio_mst_c_lrclk.hw,
>>>>>>> + [AUD_CLKID_MST_D_LRCLK_DIV] = &audio_mst_d_lrclk_div.hw,
>>>>>>> + [AUD_CLKID_MST_D_LRCLK] = &audio_mst_d_lrclk.hw,
>>>>>>> + [AUD_CLKID_TDMIN_A_SCLK_SEL] = &audio_tdmin_a_sclk_mux.hw,
>>>>>>> + [AUD_CLKID_TDMIN_A_SCLK_PRE_EN] = &audio_tdmin_a_sclk_pre_en.hw,
>>>>>>> + [AUD_CLKID_TDMIN_A_SCLK_POST_EN] = &audio_tdmin_a_sclk_post_en.hw,
>>>>>>> + [AUD_CLKID_TDMIN_A_SCLK] = &audio_tdmin_a_sclk.hw,
>>>>>>> + [AUD_CLKID_TDMIN_A_LRCLK] = &audio_tdmin_a_lrclk.hw,
>>>>>>> + [AUD_CLKID_TDMIN_B_SCLK_SEL] = &audio_tdmin_b_sclk_mux.hw,
>>>>>>> + [AUD_CLKID_TDMIN_B_SCLK_PRE_EN] = &audio_tdmin_b_sclk_pre_en.hw,
>>>>>>> + [AUD_CLKID_TDMIN_B_SCLK_POST_EN] = &audio_tdmin_b_sclk_post_en.hw,
>>>>>>> + [AUD_CLKID_TDMIN_B_SCLK] = &audio_tdmin_b_sclk.hw,
>>>>>>> + [AUD_CLKID_TDMIN_B_LRCLK] = &audio_tdmin_b_lrclk.hw,
>>>>>>> + [AUD_CLKID_TDMIN_LB_SCLK_SEL] = &audio_tdmin_lb_sclk_mux.hw,
>>>>>>> + [AUD_CLKID_TDMIN_LB_SCLK_PRE_EN] = &audio_tdmin_lb_sclk_pre_en.hw,
>>>>>>> + [AUD_CLKID_TDMIN_LB_SCLK_POST_EN] = &audio_tdmin_lb_sclk_post_en.hw,
>>>>>>> + [AUD_CLKID_TDMIN_LB_SCLK] = &audio_tdmin_lb_sclk.hw,
>>>>>>> + [AUD_CLKID_TDMIN_LB_LRCLK] = &audio_tdmin_lb_lrclk.hw,
>>>>>>> + [AUD_CLKID_TDMOUT_A_SCLK_SEL] = &audio_tdmout_a_sclk_mux.hw,
>>>>>>> + [AUD_CLKID_TDMOUT_A_SCLK_PRE_EN] = &audio_tdmout_a_sclk_pre_en.hw,
>>>>>>> + [AUD_CLKID_TDMOUT_A_SCLK_POST_EN] = &audio_tdmout_a_sclk_post_en.hw,
>>>>>>> + [AUD_CLKID_TDMOUT_A_SCLK] = &audio_tdmout_a_sclk.hw,
>>>>>>> + [AUD_CLKID_TDMOUT_A_LRCLK] = &audio_tdmout_a_lrclk.hw,
>>>>>>> + [AUD_CLKID_TDMOUT_B_SCLK_SEL] = &audio_tdmout_b_sclk_mux.hw,
>>>>>>> + [AUD_CLKID_TDMOUT_B_SCLK_PRE_EN] = &audio_tdmout_b_sclk_pre_en.hw,
>>>>>>> + [AUD_CLKID_TDMOUT_B_SCLK_POST_EN] = &audio_tdmout_b_sclk_post_en.hw,
>>>>>>> + [AUD_CLKID_TDMOUT_B_SCLK] = &audio_tdmout_b_sclk.hw,
>>>>>>> + [AUD_CLKID_TDMOUT_B_LRCLK] = &audio_tdmout_b_lrclk.hw,
>>>>>>> +
>>>>>>> + [AUD2_CLKID_DDR_ARB] = &audio2_ddr_arb.hw,
>>>>>>> + [AUD2_CLKID_PDM] = &audio2_pdm.hw,
>>>>>>> + [AUD2_CLKID_TDMIN_VAD] = &audio2_tdmin_vad.hw,
>>>>>>> + [AUD2_CLKID_TODDR_VAD] = &audio2_toddr_vad.hw,
>>>>>>> + [AUD2_CLKID_VAD] = &audio2_vad.hw,
>>>>>>> + [AUD2_CLKID_AUDIOTOP] = &audio2_audiotop.hw,
>>>>>>> + [AUD2_CLKID_VAD_MCLK_SEL] = &audio2_vad_mclk_mux.hw,
>>>>>>> + [AUD2_CLKID_VAD_MCLK_DIV] = &audio2_vad_mclk_div.hw,
>>>>>>> + [AUD2_CLKID_VAD_MCLK] = &audio2_vad_mclk.hw,
>>>>>>> + [AUD2_CLKID_VAD_CLK_SEL] = &audio2_vad_clk_mux.hw,
>>>>>>> + [AUD2_CLKID_VAD_CLK_DIV] = &audio2_vad_clk_div.hw,
>>>>>>> + [AUD2_CLKID_VAD_CLK] = &audio2_vad_clk.hw,
>>>>>>> + [AUD2_CLKID_PDM_DCLK_SEL] = &audio2_pdm_dclk_mux.hw,
>>>>>>> + [AUD2_CLKID_PDM_DCLK_DIV] = &audio2_pdm_dclk_div.hw,
>>>>>>> + [AUD2_CLKID_PDM_DCLK] = &audio2_pdm_dclk.hw,
>>>>>>> + [AUD2_CLKID_PDM_SYSCLK_SEL] = &audio2_pdm_sysclk_mux.hw,
>>>>>>> + [AUD2_CLKID_PDM_SYSCLK_DIV] = &audio2_pdm_sysclk_div.hw,
>>>>>>> + [AUD2_CLKID_PDM_SYSCLK] = &audio2_pdm_sysclk.hw,
>>>>>>> +};
>>>>>>> +
>>>>>>> +static struct meson_clk_hw_data a1_audio_clks = {
>>>>>>> + .hws = a1_audio_hw_clks,
>>>>>>> + .num = ARRAY_SIZE(a1_audio_hw_clks),
>>>>>>> +};
>>>>>>> +
>>>>>>> +static struct regmap *a1_audio_map(struct platform_device *pdev,
>>>>>>> + unsigned int index)
>>>>>>> +{
>>>>>>> + char name[32];
>>>>>>> + const struct regmap_config cfg = {
>>>>>>> + .reg_bits = 32,
>>>>>>> + .val_bits = 32,
>>>>>>> + .reg_stride = 4,
>>>>>>> + .name = name,
>>>>>>
>>>>>> Not necessary
>>>>>>
>>>>>
>>>>> This implementation uses two regmaps, and this field allow to avoid
>>>>> errors like this:
>>>>>
>>>>> [ 0.145530] debugfs: Directory 'fe050000.audio-clock-controller' with
>>>>> parent 'regmap' already present!
>>>>>
>>>>>>> + };
>>>>>>> + void __iomem *base;
>>>>>>> +
>>>>>>> + base = devm_platform_ioremap_resource(pdev, index);
>>>>>>> + if (IS_ERR(base))
>>>>>>> + return base;
>>>>>>> +
>>>>>>> + scnprintf(name, sizeof(name), "%d", index);
>>>>>>> + return devm_regmap_init_mmio(&pdev->dev, base, &cfg);
>>>>>>> +}
>>>>>>
>>>>>> That is overengineered. Please keep it simple. Declare the regmap_config
>>>>>> as static const global, and do it like axg-audio please.
>>>>>>
>>>>>
>>>>> This only reason why it is not "static const" because I need to set
>>>>> unique name for each regmap.
>>>>>
>>>>>>> +
>>>>>>> +static int a1_register_clk(struct platform_device *pdev,
>>>>>>> + struct regmap *map0, struct regmap *map1,
>>>>>>> + struct clk_hw *hw)
>>>>>>> +{
>>>>>>> + struct clk_regmap *clk = container_of(hw, struct clk_regmap, hw);
>>>>>>> +
>>>>>>> + if (!hw)
>>>>>>> + return 0;
>>>>>>> +
>>>>>>> + switch ((unsigned long)clk->map) {
>>>>>>> + case AUDIO_RANGE_0:
>>>>>>> + clk->map = map0;
>>>>>>> + break;
>>>>>>> + case AUDIO_RANGE_1:
>>>>>>> + clk->map = map1;
>>>>>>> + break;
>>>>>>
>>>>>> ... fishy
>>>>>>
>>>>>>> + default:
>>>>>>> + WARN_ON(1);
>>>>>>> + return -EINVAL;
>>>>>>> + }
>>>>>>> +
>>>>>>> + return devm_clk_hw_register(&pdev->dev, hw);
>>>>>>> +}
>>>>>>> +
>>>>>>> +static int a1_audio_clkc_probe(struct platform_device *pdev)
>>>>>>> +{
>>>>>>> + struct regmap *map0, *map1;
>>>>>>> + struct clk *clk;
>>>>>>> + unsigned int i;
>>>>>>> + int ret;
>>>>>>> +
>>>>>>> + clk = devm_clk_get_enabled(&pdev->dev, "pclk");
>>>>>>> + if (WARN_ON(IS_ERR(clk)))
>>>>>>> + return PTR_ERR(clk);
>>>>>>> +
>>>>>>> + map0 = a1_audio_map(pdev, 0);
>>>>>>> + if (IS_ERR(map0))
>>>>>>> + return PTR_ERR(map0);
>>>>>>> +
>>>>>>> + map1 = a1_audio_map(pdev, 1);
>>>>>>> + if (IS_ERR(map1))
>>>>>>> + return PTR_ERR(map1);
>>>>>>
>>>>>> No - Looks to me you just have two clock controllers you are trying
>>>>>> force into one.
>>>>>>
>>>>>
>>>>> See the begining.
>>>>>
>>>>>>> +
>>>>>>> + /*
>>>>>>> + * Register and enable AUD2_CLKID_AUDIOTOP clock first. Unless
>>>>>>> + * it is enabled any read/write to 'map0' hangs the CPU.
>>>>>>> + */
>>>>>>> +
>>>>>>> + ret = a1_register_clk(pdev, map0, map1,
>>>>>>> + a1_audio_clks.hws[AUD2_CLKID_AUDIOTOP]);
>>>>>>> + if (ret)
>>>>>>> + return ret;
>>>>>>> +
>>>>>>> + ret = clk_prepare_enable(a1_audio_clks.hws[AUD2_CLKID_AUDIOTOP]->clk);
>>>>>>> + if (ret)
>>>>>>> + return ret;
>>>>>>
>>>>>> Again, this shows 2 devices. The one related to your 'map0' should
>>>>>> request AUD2_CLKID_AUDIOTOP as input and enable it right away.
>>>>>>
>>>>>
>>>>> See the begining.
>>>>>
>>>>>>> +
>>>>>>> + for (i = 0; i < a1_audio_clks.num; i++) {
>>>>>>> + if (i == AUD2_CLKID_AUDIOTOP)
>>>>>>> + continue;
>>>>>>> +
>>>>>>> + ret = a1_register_clk(pdev, map0, map1, a1_audio_clks.hws[i]);
>>>>>>> + if (ret)
>>>>>>> + return ret;
>>>>>>> + }
>>>>>>> +
>>>>>>> + ret = devm_of_clk_add_hw_provider(&pdev->dev, meson_clk_hw_get,
>>>>>>> + &a1_audio_clks);
>>>>>>> + if (ret)
>>>>>>> + return ret;
>>>>>>> +
>>>>>>> + BUILD_BUG_ON((unsigned long)AUDIO_REG_MAP(AUDIO_SW_RESET0) !=
>>>>>>> + AUDIO_RANGE_0);
>>>>>>
>>>>>> Why is that necessary ?
>>>>>>
>>>>>
>>>>> A little paranoia. Here AUDIO_SW_RESET0 is handled as map0's register,
>>>>> and I want to assert it.
>>>>>
>>>>>>> + return meson_audio_rstc_register(&pdev->dev, map0,
>>>>>>> + AUDIO_REG_OFFSET(AUDIO_SW_RESET0), 32);
>>>>>>> +}
>>>>>>> +
>>>>>>> +static const struct of_device_id a1_audio_clkc_match_table[] = {
>>>>>>> + { .compatible = "amlogic,a1-audio-clkc", },
>>>>>>> + {}
>>>>>>> +};
>>>>>>> +MODULE_DEVICE_TABLE(of, a1_audio_clkc_match_table);
>>>>>>> +
>>>>>>> +static struct platform_driver a1_audio_clkc_driver = {
>>>>>>> + .probe = a1_audio_clkc_probe,
>>>>>>> + .driver = {
>>>>>>> + .name = "a1-audio-clkc",
>>>>>>> + .of_match_table = a1_audio_clkc_match_table,
>>>>>>> + },
>>>>>>> +};
>>>>>>> +module_platform_driver(a1_audio_clkc_driver);
>>>>>>> +
>>>>>>> +MODULE_DESCRIPTION("Amlogic A1 Audio Clock driver");
>>>>>>> +MODULE_AUTHOR("Jan Dakinevich <jan.dakinevich@salutedevices.com>");
>>>>>>> +MODULE_LICENSE("GPL");
>>>>>>> diff --git a/drivers/clk/meson/a1-audio.h b/drivers/clk/meson/a1-audio.h
>>>>>>> new file mode 100644
>>>>>>> index 000000000000..f994e87276cd
>>>>>>> --- /dev/null
>>>>>>> +++ b/drivers/clk/meson/a1-audio.h
>>>>>>> @@ -0,0 +1,58 @@
>>>>>>> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
>>>>>>> +/*
>>>>>>> + * Copyright (c) 2024, SaluteDevices. All Rights Reserved.
>>>>>>> + *
>>>>>>> + * Author: Jan Dakinevich <jan.dakinevich@salutedevices.com>
>>>>>>> + */
>>>>>>> +
>>>>>>> +#ifndef __A1_AUDIO_H
>>>>>>> +#define __A1_AUDIO_H
>>>>>>> +
>>>>>>> +#define AUDIO_RANGE_0 0xa
>>>>>>> +#define AUDIO_RANGE_1 0xb
>>>>>>> +#define AUDIO_RANGE_SHIFT 16
>>>>>>> +
>>>>>>> +#define AUDIO_REG(_range, _offset) \
>>>>>>> + (((_range) << AUDIO_RANGE_SHIFT) + (_offset))
>>>>>>> +
>>>>>>> +#define AUDIO_REG_OFFSET(_reg) \
>>>>>>> + ((_reg) & ((1 << AUDIO_RANGE_SHIFT) - 1))
>>>>>>> +
>>>>>>> +#define AUDIO_REG_MAP(_reg) \
>>>>>>> + ((void *)((_reg) >> AUDIO_RANGE_SHIFT))
>>>>>>
>>>>>> That is seriouly overengineered.
>>>>>> The following are offset. Just write what they are.
>>>>>>
>>>>>
>>>>> This is all in order to keep range's identifier together with offset and
>>>>> then use it to store the identifier in clk_regmaps.
>>>>>
>>>>>> There is not reason to put that into a header. It is only going to be
>>>>>> used by a single driver.
>>>>>>>> +
>>>>>>> +#define AUDIO_CLK_GATE_EN0 AUDIO_REG(AUDIO_RANGE_0, 0x000)
>>>>>>> +#define AUDIO_MCLK_A_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x008)
>>>>>>> +#define AUDIO_MCLK_B_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x00c)
>>>>>>> +#define AUDIO_MCLK_C_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x010)
>>>>>>> +#define AUDIO_MCLK_D_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x014)
>>>>>>> +#define AUDIO_MCLK_E_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x018)
>>>>>>> +#define AUDIO_MCLK_F_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x01c)
>>>>>>> +#define AUDIO_SW_RESET0 AUDIO_REG(AUDIO_RANGE_0, 0x028)
>>>>>>> +#define AUDIO_MST_A_SCLK_CTRL0 AUDIO_REG(AUDIO_RANGE_0, 0x040)
>>>>>>> +#define AUDIO_MST_A_SCLK_CTRL1 AUDIO_REG(AUDIO_RANGE_0, 0x044)
>>>>>>> +#define AUDIO_MST_B_SCLK_CTRL0 AUDIO_REG(AUDIO_RANGE_0, 0x048)
>>>>>>> +#define AUDIO_MST_B_SCLK_CTRL1 AUDIO_REG(AUDIO_RANGE_0, 0x04c)
>>>>>>> +#define AUDIO_MST_C_SCLK_CTRL0 AUDIO_REG(AUDIO_RANGE_0, 0x050)
>>>>>>> +#define AUDIO_MST_C_SCLK_CTRL1 AUDIO_REG(AUDIO_RANGE_0, 0x054)
>>>>>>> +#define AUDIO_MST_D_SCLK_CTRL0 AUDIO_REG(AUDIO_RANGE_0, 0x058)
>>>>>>> +#define AUDIO_MST_D_SCLK_CTRL1 AUDIO_REG(AUDIO_RANGE_0, 0x05c)
>>>>>>> +#define AUDIO_CLK_TDMIN_A_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x080)
>>>>>>> +#define AUDIO_CLK_TDMIN_B_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x084)
>>>>>>> +#define AUDIO_CLK_TDMIN_LB_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x08c)
>>>>>>> +#define AUDIO_CLK_TDMOUT_A_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x090)
>>>>>>> +#define AUDIO_CLK_TDMOUT_B_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x094)
>>>>>>> +#define AUDIO_CLK_SPDIFIN_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x09c)
>>>>>>> +#define AUDIO_CLK_RESAMPLE_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x0a4)
>>>>>>> +#define AUDIO_CLK_LOCKER_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x0a8)
>>>>>>> +#define AUDIO_CLK_EQDRC_CTRL AUDIO_REG(AUDIO_RANGE_0, 0x0c0)
>>>>>>> +
>>>>>>> +#define AUDIO2_CLK_GATE_EN0 AUDIO_REG(AUDIO_RANGE_1, 0x00c)
>>>>>>> +#define AUDIO2_MCLK_VAD_CTRL AUDIO_REG(AUDIO_RANGE_1, 0x040)
>>>>>>> +#define AUDIO2_CLK_VAD_CTRL AUDIO_REG(AUDIO_RANGE_1, 0x044)
>>>>>>> +#define AUDIO2_CLK_PDMIN_CTRL0 AUDIO_REG(AUDIO_RANGE_1, 0x058)
>>>>>>> +#define AUDIO2_CLK_PDMIN_CTRL1 AUDIO_REG(AUDIO_RANGE_1, 0x05c)
>>>>>>> +
>>>>>>> +#include <dt-bindings/clock/amlogic,a1-audio-clkc.h>
>>>>>>> +
>>>>>>> +#endif /* __A1_AUDIO_H */
>>>>>>
>>>>>>
>>>>
>>>>
>>
>>
--
Jerome
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH RFC 1/3] mm/gup: consistently name GUP-fast functions
From: Mike Rapoport @ 2024-03-27 14:43 UTC (permalink / raw)
To: David Hildenbrand
Cc: linux-kernel, Andrew Morton, Jason Gunthorpe, John Hubbard,
Peter Xu, linux-arm-kernel, loongarch, linux-mips, linuxppc-dev,
linux-s390, linux-sh, linux-mm, linux-perf-users, linux-fsdevel,
x86
In-Reply-To: <20240327130538.680256-2-david@redhat.com>
On Wed, Mar 27, 2024 at 02:05:36PM +0100, David Hildenbrand wrote:
> Let's consistently call the "fast-only" part of GUP "GUP-fast" and rename
> all relevant internal functions to start with "gup_fast", to make it
> clearer that this is not ordinary GUP. The current mixture of
> "lockless", "gup" and "gup_fast" is confusing.
>
> Further, avoid the term "huge" when talking about a "leaf" -- for
> example, we nowadays check pmd_leaf() because pmd_huge() is gone. For the
> "hugepd"/"hugepte" stuff, it's part of the name ("is_hugepd"), so that
> says.
typo: stays
> What remains is the "external" interface:
> * get_user_pages_fast_only()
> * get_user_pages_fast()
> * pin_user_pages_fast()
>
> And the "internal" interface that handles GUP-fast + fallback:
> * internal_get_user_pages_fast()
>
> The high-level internal function for GUP-fast is now:
> * gup_fast()
>
> The basic GUP-fast walker functions:
> * gup_pgd_range() -> gup_fast_pgd_range()
> * gup_p4d_range() -> gup_fast_p4d_range()
> * gup_pud_range() -> gup_fast_pud_range()
> * gup_pmd_range() -> gup_fast_pmd_range()
> * gup_pte_range() -> gup_fast_pte_range()
> * gup_huge_pgd() -> gup_fast_pgd_leaf()
> * gup_huge_pud() -> gup_fast_pud_leaf()
> * gup_huge_pmd() -> gup_fast_pmd_leaf()
>
> The weird hugepd stuff:
> * gup_huge_pd() -> gup_fast_hugepd()
> * gup_hugepte() -> gup_fast_hugepte()
>
> The weird devmap stuff:
> * __gup_device_huge_pud() -> gup_fast_devmap_pud_leaf()
> * __gup_device_huge_pmd -> gup_fast_devmap_pmd_leaf()
> * __gup_device_huge() -> gup_fast_devmap_leaf()
>
> Helper functions:
> * unpin_user_pages_lockless() -> gup_fast_unpin_user_pages()
> * gup_fast_folio_allowed() is already properly named
> * gup_fast_permitted() is already properly named
>
> With "gup_fast()", we now even have a function that is referred to in
> comment in mm/mmu_gather.c.
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Mike Rapoport (IBM) <rppt@kernel.org>
> ---
> mm/gup.c | 164 ++++++++++++++++++++++++++++---------------------------
> 1 file changed, 84 insertions(+), 80 deletions(-)
>
> diff --git a/mm/gup.c b/mm/gup.c
> index 03b74b148e30..c293aff30c5d 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -440,7 +440,7 @@ void unpin_user_page_range_dirty_lock(struct page *page, unsigned long npages,
> }
> EXPORT_SYMBOL(unpin_user_page_range_dirty_lock);
>
> -static void unpin_user_pages_lockless(struct page **pages, unsigned long npages)
> +static void gup_fast_unpin_user_pages(struct page **pages, unsigned long npages)
> {
> unsigned long i;
> struct folio *folio;
> @@ -2431,7 +2431,7 @@ long get_user_pages_unlocked(unsigned long start, unsigned long nr_pages,
> EXPORT_SYMBOL(get_user_pages_unlocked);
>
> /*
> - * Fast GUP
> + * GUP-fast
> *
> * get_user_pages_fast attempts to pin user pages by walking the page
> * tables directly and avoids taking locks. Thus the walker needs to be
> @@ -2445,7 +2445,7 @@ EXPORT_SYMBOL(get_user_pages_unlocked);
> *
> * Another way to achieve this is to batch up page table containing pages
> * belonging to more than one mm_user, then rcu_sched a callback to free those
> - * pages. Disabling interrupts will allow the fast_gup walker to both block
> + * pages. Disabling interrupts will allow the gup_fast() walker to both block
> * the rcu_sched callback, and an IPI that we broadcast for splitting THPs
> * (which is a relatively rare event). The code below adopts this strategy.
> *
> @@ -2589,9 +2589,9 @@ static void __maybe_unused undo_dev_pagemap(int *nr, int nr_start,
> * also check pmd here to make sure pmd doesn't change (corresponds to
> * pmdp_collapse_flush() in the THP collapse code path).
> */
> -static int gup_pte_range(pmd_t pmd, pmd_t *pmdp, unsigned long addr,
> - unsigned long end, unsigned int flags,
> - struct page **pages, int *nr)
> +static int gup_fast_pte_range(pmd_t pmd, pmd_t *pmdp, unsigned long addr,
> + unsigned long end, unsigned int flags, struct page **pages,
> + int *nr)
> {
> struct dev_pagemap *pgmap = NULL;
> int nr_start = *nr, ret = 0;
> @@ -2688,20 +2688,19 @@ static int gup_pte_range(pmd_t pmd, pmd_t *pmdp, unsigned long addr,
> *
> * For a futex to be placed on a THP tail page, get_futex_key requires a
> * get_user_pages_fast_only implementation that can pin pages. Thus it's still
> - * useful to have gup_huge_pmd even if we can't operate on ptes.
> + * useful to have gup_fast_pmd_leaf even if we can't operate on ptes.
> */
> -static int gup_pte_range(pmd_t pmd, pmd_t *pmdp, unsigned long addr,
> - unsigned long end, unsigned int flags,
> - struct page **pages, int *nr)
> +static int gup_fast_pte_range(pmd_t pmd, pmd_t *pmdp, unsigned long addr,
> + unsigned long end, unsigned int flags, struct page **pages,
> + int *nr)
> {
> return 0;
> }
> #endif /* CONFIG_ARCH_HAS_PTE_SPECIAL */
>
> #if defined(CONFIG_ARCH_HAS_PTE_DEVMAP) && defined(CONFIG_TRANSPARENT_HUGEPAGE)
> -static int __gup_device_huge(unsigned long pfn, unsigned long addr,
> - unsigned long end, unsigned int flags,
> - struct page **pages, int *nr)
> +static int gup_fast_devmap_leaf(unsigned long pfn, unsigned long addr,
> + unsigned long end, unsigned int flags, struct page **pages, int *nr)
> {
> int nr_start = *nr;
> struct dev_pagemap *pgmap = NULL;
> @@ -2734,15 +2733,15 @@ static int __gup_device_huge(unsigned long pfn, unsigned long addr,
> return addr == end;
> }
>
> -static int __gup_device_huge_pmd(pmd_t orig, pmd_t *pmdp, unsigned long addr,
> - unsigned long end, unsigned int flags,
> - struct page **pages, int *nr)
> +static int gup_fast_devmap_pmd_leaf(pmd_t orig, pmd_t *pmdp, unsigned long addr,
> + unsigned long end, unsigned int flags, struct page **pages,
> + int *nr)
> {
> unsigned long fault_pfn;
> int nr_start = *nr;
>
> fault_pfn = pmd_pfn(orig) + ((addr & ~PMD_MASK) >> PAGE_SHIFT);
> - if (!__gup_device_huge(fault_pfn, addr, end, flags, pages, nr))
> + if (!gup_fast_devmap_leaf(fault_pfn, addr, end, flags, pages, nr))
> return 0;
>
> if (unlikely(pmd_val(orig) != pmd_val(*pmdp))) {
> @@ -2752,15 +2751,15 @@ static int __gup_device_huge_pmd(pmd_t orig, pmd_t *pmdp, unsigned long addr,
> return 1;
> }
>
> -static int __gup_device_huge_pud(pud_t orig, pud_t *pudp, unsigned long addr,
> - unsigned long end, unsigned int flags,
> - struct page **pages, int *nr)
> +static int gup_fast_devmap_pud_leaf(pud_t orig, pud_t *pudp, unsigned long addr,
> + unsigned long end, unsigned int flags, struct page **pages,
> + int *nr)
> {
> unsigned long fault_pfn;
> int nr_start = *nr;
>
> fault_pfn = pud_pfn(orig) + ((addr & ~PUD_MASK) >> PAGE_SHIFT);
> - if (!__gup_device_huge(fault_pfn, addr, end, flags, pages, nr))
> + if (!gup_fast_devmap_leaf(fault_pfn, addr, end, flags, pages, nr))
> return 0;
>
> if (unlikely(pud_val(orig) != pud_val(*pudp))) {
> @@ -2770,17 +2769,17 @@ static int __gup_device_huge_pud(pud_t orig, pud_t *pudp, unsigned long addr,
> return 1;
> }
> #else
> -static int __gup_device_huge_pmd(pmd_t orig, pmd_t *pmdp, unsigned long addr,
> - unsigned long end, unsigned int flags,
> - struct page **pages, int *nr)
> +static int gup_fast_devmap_pmd_leaf(pmd_t orig, pmd_t *pmdp, unsigned long addr,
> + unsigned long end, unsigned int flags, struct page **pages,
> + int *nr)
> {
> BUILD_BUG();
> return 0;
> }
>
> -static int __gup_device_huge_pud(pud_t pud, pud_t *pudp, unsigned long addr,
> - unsigned long end, unsigned int flags,
> - struct page **pages, int *nr)
> +static int gup_fast_devmap_pud_leaf(pud_t pud, pud_t *pudp, unsigned long addr,
> + unsigned long end, unsigned int flags, struct page **pages,
> + int *nr)
> {
> BUILD_BUG();
> return 0;
> @@ -2806,9 +2805,9 @@ static unsigned long hugepte_addr_end(unsigned long addr, unsigned long end,
> return (__boundary - 1 < end - 1) ? __boundary : end;
> }
>
> -static int gup_hugepte(pte_t *ptep, unsigned long sz, unsigned long addr,
> - unsigned long end, unsigned int flags,
> - struct page **pages, int *nr)
> +static int gup_fast_hugepte(pte_t *ptep, unsigned long sz, unsigned long addr,
> + unsigned long end, unsigned int flags, struct page **pages,
> + int *nr)
> {
> unsigned long pte_end;
> struct page *page;
> @@ -2855,7 +2854,7 @@ static int gup_hugepte(pte_t *ptep, unsigned long sz, unsigned long addr,
> return 1;
> }
>
> -static int gup_huge_pd(hugepd_t hugepd, unsigned long addr,
> +static int gup_fast_hugepd(hugepd_t hugepd, unsigned long addr,
> unsigned int pdshift, unsigned long end, unsigned int flags,
> struct page **pages, int *nr)
> {
> @@ -2866,14 +2865,14 @@ static int gup_huge_pd(hugepd_t hugepd, unsigned long addr,
> ptep = hugepte_offset(hugepd, addr, pdshift);
> do {
> next = hugepte_addr_end(addr, end, sz);
> - if (!gup_hugepte(ptep, sz, addr, end, flags, pages, nr))
> + if (!gup_fast_hugepte(ptep, sz, addr, end, flags, pages, nr))
> return 0;
> } while (ptep++, addr = next, addr != end);
>
> return 1;
> }
> #else
> -static inline int gup_huge_pd(hugepd_t hugepd, unsigned long addr,
> +static inline int gup_fast_hugepd(hugepd_t hugepd, unsigned long addr,
> unsigned int pdshift, unsigned long end, unsigned int flags,
> struct page **pages, int *nr)
> {
> @@ -2881,9 +2880,9 @@ static inline int gup_huge_pd(hugepd_t hugepd, unsigned long addr,
> }
> #endif /* CONFIG_ARCH_HAS_HUGEPD */
>
> -static int gup_huge_pmd(pmd_t orig, pmd_t *pmdp, unsigned long addr,
> - unsigned long end, unsigned int flags,
> - struct page **pages, int *nr)
> +static int gup_fast_pmd_leaf(pmd_t orig, pmd_t *pmdp, unsigned long addr,
> + unsigned long end, unsigned int flags, struct page **pages,
> + int *nr)
> {
> struct page *page;
> struct folio *folio;
> @@ -2895,8 +2894,8 @@ static int gup_huge_pmd(pmd_t orig, pmd_t *pmdp, unsigned long addr,
> if (pmd_devmap(orig)) {
> if (unlikely(flags & FOLL_LONGTERM))
> return 0;
> - return __gup_device_huge_pmd(orig, pmdp, addr, end, flags,
> - pages, nr);
> + return gup_fast_devmap_pmd_leaf(orig, pmdp, addr, end, flags,
> + pages, nr);
> }
>
> page = nth_page(pmd_page(orig), (addr & ~PMD_MASK) >> PAGE_SHIFT);
> @@ -2925,9 +2924,9 @@ static int gup_huge_pmd(pmd_t orig, pmd_t *pmdp, unsigned long addr,
> return 1;
> }
>
> -static int gup_huge_pud(pud_t orig, pud_t *pudp, unsigned long addr,
> - unsigned long end, unsigned int flags,
> - struct page **pages, int *nr)
> +static int gup_fast_pud_leaf(pud_t orig, pud_t *pudp, unsigned long addr,
> + unsigned long end, unsigned int flags, struct page **pages,
> + int *nr)
> {
> struct page *page;
> struct folio *folio;
> @@ -2939,8 +2938,8 @@ static int gup_huge_pud(pud_t orig, pud_t *pudp, unsigned long addr,
> if (pud_devmap(orig)) {
> if (unlikely(flags & FOLL_LONGTERM))
> return 0;
> - return __gup_device_huge_pud(orig, pudp, addr, end, flags,
> - pages, nr);
> + return gup_fast_devmap_pud_leaf(orig, pudp, addr, end, flags,
> + pages, nr);
> }
>
> page = nth_page(pud_page(orig), (addr & ~PUD_MASK) >> PAGE_SHIFT);
> @@ -2970,9 +2969,9 @@ static int gup_huge_pud(pud_t orig, pud_t *pudp, unsigned long addr,
> return 1;
> }
>
> -static int gup_huge_pgd(pgd_t orig, pgd_t *pgdp, unsigned long addr,
> - unsigned long end, unsigned int flags,
> - struct page **pages, int *nr)
> +static int gup_fast_pgd_leaf(pgd_t orig, pgd_t *pgdp, unsigned long addr,
> + unsigned long end, unsigned int flags, struct page **pages,
> + int *nr)
> {
> int refs;
> struct page *page;
> @@ -3010,8 +3009,9 @@ static int gup_huge_pgd(pgd_t orig, pgd_t *pgdp, unsigned long addr,
> return 1;
> }
>
> -static int gup_pmd_range(pud_t *pudp, pud_t pud, unsigned long addr, unsigned long end,
> - unsigned int flags, struct page **pages, int *nr)
> +static int gup_fast_pmd_range(pud_t *pudp, pud_t pud, unsigned long addr,
> + unsigned long end, unsigned int flags, struct page **pages,
> + int *nr)
> {
> unsigned long next;
> pmd_t *pmdp;
> @@ -3025,11 +3025,11 @@ static int gup_pmd_range(pud_t *pudp, pud_t pud, unsigned long addr, unsigned lo
> return 0;
>
> if (unlikely(pmd_leaf(pmd))) {
> - /* See gup_pte_range() */
> + /* See gup_fast_pte_range() */
> if (pmd_protnone(pmd))
> return 0;
>
> - if (!gup_huge_pmd(pmd, pmdp, addr, next, flags,
> + if (!gup_fast_pmd_leaf(pmd, pmdp, addr, next, flags,
> pages, nr))
> return 0;
>
> @@ -3038,18 +3038,20 @@ static int gup_pmd_range(pud_t *pudp, pud_t pud, unsigned long addr, unsigned lo
> * architecture have different format for hugetlbfs
> * pmd format and THP pmd format
> */
> - if (!gup_huge_pd(__hugepd(pmd_val(pmd)), addr,
> - PMD_SHIFT, next, flags, pages, nr))
> + if (!gup_fast_hugepd(__hugepd(pmd_val(pmd)), addr,
> + PMD_SHIFT, next, flags, pages, nr))
> return 0;
> - } else if (!gup_pte_range(pmd, pmdp, addr, next, flags, pages, nr))
> + } else if (!gup_fast_pte_range(pmd, pmdp, addr, next, flags,
> + pages, nr))
> return 0;
> } while (pmdp++, addr = next, addr != end);
>
> return 1;
> }
>
> -static int gup_pud_range(p4d_t *p4dp, p4d_t p4d, unsigned long addr, unsigned long end,
> - unsigned int flags, struct page **pages, int *nr)
> +static int gup_fast_pud_range(p4d_t *p4dp, p4d_t p4d, unsigned long addr,
> + unsigned long end, unsigned int flags, struct page **pages,
> + int *nr)
> {
> unsigned long next;
> pud_t *pudp;
> @@ -3062,22 +3064,24 @@ static int gup_pud_range(p4d_t *p4dp, p4d_t p4d, unsigned long addr, unsigned lo
> if (unlikely(!pud_present(pud)))
> return 0;
> if (unlikely(pud_leaf(pud))) {
> - if (!gup_huge_pud(pud, pudp, addr, next, flags,
> - pages, nr))
> + if (!gup_fast_pud_leaf(pud, pudp, addr, next, flags,
> + pages, nr))
> return 0;
> } else if (unlikely(is_hugepd(__hugepd(pud_val(pud))))) {
> - if (!gup_huge_pd(__hugepd(pud_val(pud)), addr,
> - PUD_SHIFT, next, flags, pages, nr))
> + if (!gup_fast_hugepd(__hugepd(pud_val(pud)), addr,
> + PUD_SHIFT, next, flags, pages, nr))
> return 0;
> - } else if (!gup_pmd_range(pudp, pud, addr, next, flags, pages, nr))
> + } else if (!gup_fast_pmd_range(pudp, pud, addr, next, flags,
> + pages, nr))
> return 0;
> } while (pudp++, addr = next, addr != end);
>
> return 1;
> }
>
> -static int gup_p4d_range(pgd_t *pgdp, pgd_t pgd, unsigned long addr, unsigned long end,
> - unsigned int flags, struct page **pages, int *nr)
> +static int gup_fast_p4d_range(pgd_t *pgdp, pgd_t pgd, unsigned long addr,
> + unsigned long end, unsigned int flags, struct page **pages,
> + int *nr)
> {
> unsigned long next;
> p4d_t *p4dp;
> @@ -3091,17 +3095,18 @@ static int gup_p4d_range(pgd_t *pgdp, pgd_t pgd, unsigned long addr, unsigned lo
> return 0;
> BUILD_BUG_ON(p4d_leaf(p4d));
> if (unlikely(is_hugepd(__hugepd(p4d_val(p4d))))) {
> - if (!gup_huge_pd(__hugepd(p4d_val(p4d)), addr,
> - P4D_SHIFT, next, flags, pages, nr))
> + if (!gup_fast_hugepd(__hugepd(p4d_val(p4d)), addr,
> + P4D_SHIFT, next, flags, pages, nr))
> return 0;
> - } else if (!gup_pud_range(p4dp, p4d, addr, next, flags, pages, nr))
> + } else if (!gup_fast_pud_range(p4dp, p4d, addr, next, flags,
> + pages, nr))
> return 0;
> } while (p4dp++, addr = next, addr != end);
>
> return 1;
> }
>
> -static void gup_pgd_range(unsigned long addr, unsigned long end,
> +static void gup_fast_pgd_range(unsigned long addr, unsigned long end,
> unsigned int flags, struct page **pages, int *nr)
> {
> unsigned long next;
> @@ -3115,19 +3120,20 @@ static void gup_pgd_range(unsigned long addr, unsigned long end,
> if (pgd_none(pgd))
> return;
> if (unlikely(pgd_leaf(pgd))) {
> - if (!gup_huge_pgd(pgd, pgdp, addr, next, flags,
> - pages, nr))
> + if (!gup_fast_pgd_leaf(pgd, pgdp, addr, next, flags,
> + pages, nr))
> return;
> } else if (unlikely(is_hugepd(__hugepd(pgd_val(pgd))))) {
> - if (!gup_huge_pd(__hugepd(pgd_val(pgd)), addr,
> - PGDIR_SHIFT, next, flags, pages, nr))
> + if (!gup_fast_hugepd(__hugepd(pgd_val(pgd)), addr,
> + PGDIR_SHIFT, next, flags, pages, nr))
> return;
> - } else if (!gup_p4d_range(pgdp, pgd, addr, next, flags, pages, nr))
> + } else if (!gup_fast_p4d_range(pgdp, pgd, addr, next, flags,
> + pages, nr))
> return;
> } while (pgdp++, addr = next, addr != end);
> }
> #else
> -static inline void gup_pgd_range(unsigned long addr, unsigned long end,
> +static inline void gup_fast_pgd_range(unsigned long addr, unsigned long end,
> unsigned int flags, struct page **pages, int *nr)
> {
> }
> @@ -3144,10 +3150,8 @@ static bool gup_fast_permitted(unsigned long start, unsigned long end)
> }
> #endif
>
> -static unsigned long lockless_pages_from_mm(unsigned long start,
> - unsigned long end,
> - unsigned int gup_flags,
> - struct page **pages)
> +static unsigned long gup_fast(unsigned long start, unsigned long end,
> + unsigned int gup_flags, struct page **pages)
> {
> unsigned long flags;
> int nr_pinned = 0;
> @@ -3175,16 +3179,16 @@ static unsigned long lockless_pages_from_mm(unsigned long start,
> * that come from THPs splitting.
> */
> local_irq_save(flags);
> - gup_pgd_range(start, end, gup_flags, pages, &nr_pinned);
> + gup_fast_pgd_range(start, end, gup_flags, pages, &nr_pinned);
> local_irq_restore(flags);
>
> /*
> * When pinning pages for DMA there could be a concurrent write protect
> - * from fork() via copy_page_range(), in this case always fail fast GUP.
> + * from fork() via copy_page_range(), in this case always fail GUP-fast.
> */
> if (gup_flags & FOLL_PIN) {
> if (read_seqcount_retry(¤t->mm->write_protect_seq, seq)) {
> - unpin_user_pages_lockless(pages, nr_pinned);
> + gup_fast_unpin_user_pages(pages, nr_pinned);
> return 0;
> } else {
> sanity_check_pinned_pages(pages, nr_pinned);
> @@ -3224,7 +3228,7 @@ static int internal_get_user_pages_fast(unsigned long start,
> if (unlikely(!access_ok((void __user *)start, len)))
> return -EFAULT;
>
> - nr_pinned = lockless_pages_from_mm(start, end, gup_flags, pages);
> + nr_pinned = gup_fast(start, end, gup_flags, pages);
> if (nr_pinned == nr_pages || gup_flags & FOLL_FAST_ONLY)
> return nr_pinned;
>
> --
> 2.43.2
>
--
Sincerely yours,
Mike.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 12/14] sh: Add support for suppressing warning backtraces
From: Simon Horman @ 2024-03-27 14:44 UTC (permalink / raw)
To: Guenter Roeck
Cc: linux-kselftest, David Airlie, Arnd Bergmann, Maíra Canal,
Dan Carpenter, Kees Cook, Daniel Diaz, David Gow, Arthur Grillo,
Brendan Higgins, Naresh Kamboju, Maarten Lankhorst, Andrew Morton,
Maxime Ripard, Ville Syrjälä, Daniel Vetter,
Thomas Zimmermann, dri-devel, kunit-dev, linux-arch,
linux-arm-kernel, linux-doc, linux-kernel, linux-parisc,
linuxppc-dev, linux-riscv, linux-s390, linux-sh, loongarch,
netdev, Linux Kernel Functional Testing
In-Reply-To: <20240325175248.1499046-13-linux@roeck-us.net>
On Mon, Mar 25, 2024 at 10:52:46AM -0700, Guenter Roeck wrote:
> Add name of functions triggering warning backtraces to the __bug_table
> object section to enable support for suppressing WARNING backtraces.
>
> To limit image size impact, the pointer to the function name is only added
> to the __bug_table section if both CONFIG_KUNIT_SUPPRESS_BACKTRACE and
> CONFIG_DEBUG_BUGVERBOSE are enabled. Otherwise, the __func__ assembly
> parameter is replaced with a (dummy) NULL parameter to avoid an image size
> increase due to unused __func__ entries (this is necessary because __func__
> is not a define but a virtual variable).
>
> Tested-by: Linux Kernel Functional Testing <lkft@linaro.org>
> Acked-by: Dan Carpenter <dan.carpenter@linaro.org>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> - Rebased to v6.9-rc1
> - Added Tested-by:, Acked-by:, and Reviewed-by: tags
> - Introduced KUNIT_SUPPRESS_BACKTRACE configuration option
>
> arch/sh/include/asm/bug.h | 26 ++++++++++++++++++++++----
> 1 file changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/arch/sh/include/asm/bug.h b/arch/sh/include/asm/bug.h
> index 05a485c4fabc..470ce6567d20 100644
> --- a/arch/sh/include/asm/bug.h
> +++ b/arch/sh/include/asm/bug.h
> @@ -24,21 +24,36 @@
> * The offending file and line are encoded in the __bug_table section.
> */
> #ifdef CONFIG_DEBUG_BUGVERBOSE
> +
> +#ifdef CONFIG_KUNIT_SUPPRESS_BACKTRACE
> +# define HAVE_BUG_FUNCTION
> +# define __BUG_FUNC_PTR "\t.long %O2\n"
> +#else
> +# define __BUG_FUNC_PTR
> +#endif /* CONFIG_KUNIT_SUPPRESS_BACKTRACE */
> +
Hi Guenter,
a minor nit from my side: this change results in a Kernel doc warning.
.../bug.h:29: warning: expecting prototype for _EMIT_BUG_ENTRY(). Prototype was for HAVE_BUG_FUNCTION() instead
Perhaps either the new code should be placed above the Kernel doc,
or scripts/kernel-doc should be enhanced?
> #define _EMIT_BUG_ENTRY \
> "\t.pushsection __bug_table,\"aw\"\n" \
> "2:\t.long 1b, %O1\n" \
> - "\t.short %O2, %O3\n" \
> - "\t.org 2b+%O4\n" \
> + __BUG_FUNC_PTR \
> + "\t.short %O3, %O4\n" \
> + "\t.org 2b+%O5\n" \
> "\t.popsection\n"
> #else
> #define _EMIT_BUG_ENTRY \
> "\t.pushsection __bug_table,\"aw\"\n" \
> "2:\t.long 1b\n" \
> - "\t.short %O3\n" \
> - "\t.org 2b+%O4\n" \
> + "\t.short %O4\n" \
> + "\t.org 2b+%O5\n" \
> "\t.popsection\n"
> #endif
>
> +#ifdef HAVE_BUG_FUNCTION
> +# define __BUG_FUNC __func__
> +#else
> +# define __BUG_FUNC NULL
> +#endif
> +
> #define BUG() \
> do { \
> __asm__ __volatile__ ( \
...
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3] NUMA: Early use of cpu_to_node() returns 0 instead of the correct node id
From: Palmer Dabbelt @ 2024-03-27 14:36 UTC (permalink / raw)
To: shijie, akpm
Cc: Greg KH, patches, rafael, Paul Walmsley, aou, yury.norov, kuba,
vschneid, mingo, akpm, vbabka, rppt, tglx, jpoimboe, ndesaulniers,
mikelley, mhiramat, Arnd Bergmann, linux-kernel, linux-riscv,
linux-arm-kernel, Catalin Marinas, Will Deacon, Mark Rutland, mpe,
linuxppc-dev, chenhuacai, jiaxun.yang, linux-mips, cl, shijie
In-Reply-To: <20240126064451.5465-1-shijie@os.amperecomputing.com>
On Thu, 25 Jan 2024 22:44:51 PST (-0800), shijie@os.amperecomputing.com wrote:
> During the kernel booting, the generic cpu_to_node() is called too early in
> arm64, powerpc and riscv when CONFIG_NUMA is enabled.
>
> There are at least four places in the common code where
> the generic cpu_to_node() is called before it is initialized:
> 1.) early_trace_init() in kernel/trace/trace.c
> 2.) sched_init() in kernel/sched/core.c
> 3.) init_sched_fair_class() in kernel/sched/fair.c
> 4.) workqueue_init_early() in kernel/workqueue.c
>
> In order to fix the bug, the patch introduces early_numa_node_init()
> which is called after smp_prepare_boot_cpu() in start_kernel.
> early_numa_node_init will initialize the "numa_node" as soon as
> the early_cpu_to_node() is ready, before the cpu_to_node() is called
> at the first time.
>
> Signed-off-by: Huang Shijie <shijie@os.amperecomputing.com>
> ---
> v2 --> v3:
> Do not change the cpu_to_node to function pointer.
> Introduce early_numa_node_init() which initialize
> the numa_node at an early stage.
>
> v2: https://lore.kernel.org/all/20240123045843.75969-1-shijie@os.amperecomputing.com/
>
> v1 --> v2:
> In order to fix the x86 compiling error, move the cpu_to_node()
> from driver/base/arch_numa.c to driver/base/node.c.
>
> v1: http://lists.infradead.org/pipermail/linux-arm-kernel/2024-January/896160.html
>
> An old different title patch:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2024-January/895963.html
>
> ---
> init/main.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/init/main.c b/init/main.c
> index e24b0780fdff..39efe5ed58a0 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -870,6 +870,19 @@ static void __init print_unknown_bootoptions(void)
> memblock_free(unknown_options, len);
> }
>
> +static void __init early_numa_node_init(void)
> +{
> +#ifdef CONFIG_USE_PERCPU_NUMA_NODE_ID
> +#ifndef cpu_to_node
> + int cpu;
> +
> + /* The early_cpu_to_node() should be ready here. */
> + for_each_possible_cpu(cpu)
> + set_cpu_numa_node(cpu, early_cpu_to_node(cpu));
> +#endif
> +#endif
> +}
> +
> asmlinkage __visible __init __no_sanitize_address __noreturn __no_stack_protector
> void start_kernel(void)
> {
> @@ -900,6 +913,7 @@ void start_kernel(void)
> setup_nr_cpu_ids();
> setup_per_cpu_areas();
> smp_prepare_boot_cpu(); /* arch-specific boot-cpu hooks */
> + early_numa_node_init();
> boot_cpu_hotplug_init();
>
> pr_notice("Kernel command line: %s\n", saved_command_line);
Acked-by: Palmer Dabbelt <palmer@rivosinc.com> # RISC-V
I don't really understand the init/main.c stuff all that well, I'm
adding Andrew as it looks like he's been merging stuff here.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] ARM: vfp: use asm volatile for FP control register accesses
From: Nathan Chancellor @ 2024-03-27 14:41 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-arm-kernel, linux, arnd, linus.walleij, stable
In-Reply-To: <CAMj1kXGeMJ2PuYUtzFxv723HX9aovvtE7w4uYJCjPX4cHYC6tw@mail.gmail.com>
On Wed, Mar 27, 2024 at 09:05:17AM +0200, Ard Biesheuvel wrote:
> On Wed, 27 Mar 2024 at 01:55, Nathan Chancellor <nathan@kernel.org> wrote:
> >
> > On Mon, Mar 18, 2024 at 10:30:05AM +0100, Ard Biesheuvel wrote:
> > > From: Ard Biesheuvel <ardb@kernel.org>
> > >
> > > Clang may reorder FP control register reads and writes, due to the fact
> > > that the inline asm() blocks in the read/write wrappers are not volatile
> > > qualified, and the compiler has no idea that these reads and writes may
> > > have side effects.
> > >
> > > In particular, reads of FPSCR may generate an UNDEF exception if a
> > > floating point exception is pending, and the FP emulation code in
> > > VFP_bounce() explicitly clears FP exceptions temporarily in order to be
> > > able to perform the emulation on behalf of user space. This requires
> > > that the writes to FPEXC are never reordered with respect to accesses to
> > > other FP control registers, such as FPSCR.
> > >
> > > So use asm volatile for both the read and the write helpers.
> > >
> > > Cc: <stable@kernel.org>
> > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> >
> > This seems reasonable to me based on my understanding of GCC's
> > documentation. However, their documentation states "the compiler can
> > move even volatile asm instructions relative to other code, including
> > across jump instructions" and I feel like there was some discussion
> > around this sentence in the past but I can't remember what the
> > conclusion was, although I want to say Clang did not have the same
> > behavior.
>
> The only thing that matters here is whether two asm blocks are emitted
> in a different order than they appear in the program, and I would be
> very surprised if volatile permits that. Otherwise, we might introduce
> a fake input dependency or a memory clobber instead.
Yeah, I think it is reasonable to go with this approach and fall back to
your follow up suggestion if this proves not to be robust enough.
> > Regardless:
> >
> > Acked-by: Nathan Chancellor <nathan@kernel.org>
> >
>
> Thanks.
>
> > I am just curious, how was this discovered or noticed? Was there a
> > report I missed?
> >
>
> I noticed this when building a recent kernel for the original
> Raspberry Pi, which is ARMv6 not ARMv7, and has a VFP which partially
> relies on emulation. On more recent cores, we never hit the issue
> because emulation is never needed. On older cores, there is no VFP so
> we never reach this code path either.
Ah, good to know. I boot test -next on a Pi 3 with a 32-bit kernel, so
that explains why I have not seen any issues. Just wanted to make sure I
did not miss something :)
Cheers,
Nathan
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 2/5] arm64, powerpc, riscv, s390, x86: ptdump: Refactor CONFIG_DEBUG_WX
From: Palmer Dabbelt @ 2024-03-27 14:38 UTC (permalink / raw)
To: christophe.leroy
Cc: akpm, keescook, christophe.leroy, linux, Catalin Marinas,
Will Deacon, mpe, npiggin, aneesh.kumar, naveen.n.rao,
Paul Walmsley, aou, hca, gor, agordeev, borntraeger, svens,
gerald.schaefer, tglx, mingo, bp, dave.hansen, x86, hpa, luto,
peterz, linux-arm-kernel, linux-kernel, linuxppc-dev, linux-riscv,
linux-s390, linux-mm, steven.price, tranmanphong, Mark Rutland,
greg, alexghiti
In-Reply-To: <a59b102d7964261d31ead0316a9f18628e4e7a8e.1706610398.git.christophe.leroy@csgroup.eu>
On Tue, 30 Jan 2024 02:34:33 PST (-0800), christophe.leroy@csgroup.eu wrote:
> All architectures using the core ptdump functionality also implement
> CONFIG_DEBUG_WX, and they all do it more or less the same way, with a
> function called debug_checkwx() that is called by mark_rodata_ro(),
> which is a substitute to ptdump_check_wx() when CONFIG_DEBUG_WX is
> set and a no-op otherwise.
>
> Refactor by centraly defining debug_checkwx() in linux/ptdump.h and
> call debug_checkwx() immediately after calling mark_rodata_ro()
> instead of calling it at the end of every mark_rodata_ro().
>
> On x86_32, mark_rodata_ro() first checks __supported_pte_mask has
> _PAGE_NX before calling debug_checkwx(). Now the check is inside the
> callee ptdump_walk_pgd_level_checkwx().
>
> On powerpc_64, mark_rodata_ro() bails out early before calling
> ptdump_check_wx() when the MMU doesn't have KERNEL_RO feature. The
> check is now also done in ptdump_check_wx() as it is called outside
> mark_rodata_ro().
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> ---
> v2: For x86 change macro ptdump_check_wx() to ptdump_check_wx
> ---
> arch/arm64/include/asm/ptdump.h | 7 -------
> arch/arm64/mm/mmu.c | 2 --
> arch/powerpc/mm/mmu_decl.h | 6 ------
> arch/powerpc/mm/pgtable_32.c | 4 ----
> arch/powerpc/mm/pgtable_64.c | 3 ---
> arch/powerpc/mm/ptdump/ptdump.c | 3 +++
> arch/riscv/include/asm/ptdump.h | 22 ----------------------
> arch/riscv/mm/init.c | 3 ---
> arch/riscv/mm/ptdump.c | 1 -
> arch/s390/include/asm/ptdump.h | 14 --------------
> arch/s390/mm/dump_pagetables.c | 1 -
> arch/s390/mm/init.c | 2 --
> arch/x86/include/asm/pgtable.h | 3 +--
> arch/x86/mm/dump_pagetables.c | 3 +++
> arch/x86/mm/init_32.c | 2 --
> arch/x86/mm/init_64.c | 2 --
> include/linux/ptdump.h | 7 +++++++
> init/main.c | 2 ++
> 18 files changed, 16 insertions(+), 71 deletions(-)
> delete mode 100644 arch/riscv/include/asm/ptdump.h
> delete mode 100644 arch/s390/include/asm/ptdump.h
>
> diff --git a/arch/arm64/include/asm/ptdump.h b/arch/arm64/include/asm/ptdump.h
> index 581caac525b0..5b1701c76d1c 100644
> --- a/arch/arm64/include/asm/ptdump.h
> +++ b/arch/arm64/include/asm/ptdump.h
> @@ -29,13 +29,6 @@ void __init ptdump_debugfs_register(struct ptdump_info *info, const char *name);
> static inline void ptdump_debugfs_register(struct ptdump_info *info,
> const char *name) { }
> #endif
> -void ptdump_check_wx(void);
> #endif /* CONFIG_PTDUMP_CORE */
>
> -#ifdef CONFIG_DEBUG_WX
> -#define debug_checkwx() ptdump_check_wx()
> -#else
> -#define debug_checkwx() do { } while (0)
> -#endif
> -
> #endif /* __ASM_PTDUMP_H */
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 1ac7467d34c9..3a27d887f7dd 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -632,8 +632,6 @@ void mark_rodata_ro(void)
> section_size = (unsigned long)__init_begin - (unsigned long)__start_rodata;
> update_mapping_prot(__pa_symbol(__start_rodata), (unsigned long)__start_rodata,
> section_size, PAGE_KERNEL_RO);
> -
> - debug_checkwx();
> }
>
> static void __init map_kernel_segment(pgd_t *pgdp, void *va_start, void *va_end,
> diff --git a/arch/powerpc/mm/mmu_decl.h b/arch/powerpc/mm/mmu_decl.h
> index 72341b9fb552..90dcc2844056 100644
> --- a/arch/powerpc/mm/mmu_decl.h
> +++ b/arch/powerpc/mm/mmu_decl.h
> @@ -171,12 +171,6 @@ static inline void mmu_mark_rodata_ro(void) { }
> void __init mmu_mapin_immr(void);
> #endif
>
> -#ifdef CONFIG_DEBUG_WX
> -void ptdump_check_wx(void);
> -#else
> -static inline void ptdump_check_wx(void) { }
> -#endif
> -
> static inline bool debug_pagealloc_enabled_or_kfence(void)
> {
> return IS_ENABLED(CONFIG_KFENCE) || debug_pagealloc_enabled();
> diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
> index 5c02fd08d61e..12498017da8e 100644
> --- a/arch/powerpc/mm/pgtable_32.c
> +++ b/arch/powerpc/mm/pgtable_32.c
> @@ -153,7 +153,6 @@ void mark_rodata_ro(void)
>
> if (v_block_mapped((unsigned long)_stext + 1)) {
> mmu_mark_rodata_ro();
> - ptdump_check_wx();
> return;
> }
>
> @@ -166,9 +165,6 @@ void mark_rodata_ro(void)
> PFN_DOWN((unsigned long)_stext);
>
> set_memory_ro((unsigned long)_stext, numpages);
> -
> - // mark_initmem_nx() should have already run by now
> - ptdump_check_wx();
> }
> #endif
>
> diff --git a/arch/powerpc/mm/pgtable_64.c b/arch/powerpc/mm/pgtable_64.c
> index 5ac1fd30341b..1b366526f4f2 100644
> --- a/arch/powerpc/mm/pgtable_64.c
> +++ b/arch/powerpc/mm/pgtable_64.c
> @@ -150,9 +150,6 @@ void mark_rodata_ro(void)
> radix__mark_rodata_ro();
> else
> hash__mark_rodata_ro();
> -
> - // mark_initmem_nx() should have already run by now
> - ptdump_check_wx();
> }
>
> void mark_initmem_nx(void)
> diff --git a/arch/powerpc/mm/ptdump/ptdump.c b/arch/powerpc/mm/ptdump/ptdump.c
> index 2313053fe679..620d4917ebe8 100644
> --- a/arch/powerpc/mm/ptdump/ptdump.c
> +++ b/arch/powerpc/mm/ptdump/ptdump.c
> @@ -343,6 +343,9 @@ void ptdump_check_wx(void)
> }
> };
>
> + if (IS_ENABLED(CONFIG_PPC_BOOK3S_64) && !mmu_has_feature(MMU_FTR_KERNEL_RO))
> + return;
> +
> ptdump_walk_pgd(&st.ptdump, &init_mm, NULL);
>
> if (st.wx_pages)
> diff --git a/arch/riscv/include/asm/ptdump.h b/arch/riscv/include/asm/ptdump.h
> deleted file mode 100644
> index 3c9ea6dd5af7..000000000000
> --- a/arch/riscv/include/asm/ptdump.h
> +++ /dev/null
> @@ -1,22 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -/*
> - * Copyright (C) 2019 SiFive
> - */
> -
> -#ifndef _ASM_RISCV_PTDUMP_H
> -#define _ASM_RISCV_PTDUMP_H
> -
> -void ptdump_check_wx(void);
> -
> -#ifdef CONFIG_DEBUG_WX
> -static inline void debug_checkwx(void)
> -{
> - ptdump_check_wx();
> -}
> -#else
> -static inline void debug_checkwx(void)
> -{
> -}
> -#endif
> -
> -#endif /* _ASM_RISCV_PTDUMP_H */
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 32cad6a65ccd..c5c69f38d11e 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -29,7 +29,6 @@
> #include <asm/io.h>
> #include <asm/numa.h>
> #include <asm/pgtable.h>
> -#include <asm/ptdump.h>
> #include <asm/sections.h>
> #include <asm/soc.h>
> #include <asm/tlbflush.h>
> @@ -723,8 +722,6 @@ void mark_rodata_ro(void)
> if (IS_ENABLED(CONFIG_64BIT))
> set_kernel_memory(lm_alias(__start_rodata), lm_alias(_data),
> set_memory_ro);
> -
> - debug_checkwx();
> }
> #else
> static __init pgprot_t pgprot_from_va(uintptr_t va)
> diff --git a/arch/riscv/mm/ptdump.c b/arch/riscv/mm/ptdump.c
> index 657c27bc07a7..075265603313 100644
> --- a/arch/riscv/mm/ptdump.c
> +++ b/arch/riscv/mm/ptdump.c
> @@ -9,7 +9,6 @@
> #include <linux/seq_file.h>
> #include <linux/ptdump.h>
>
> -#include <asm/ptdump.h>
> #include <linux/pgtable.h>
> #include <asm/kasan.h>
>
> diff --git a/arch/s390/include/asm/ptdump.h b/arch/s390/include/asm/ptdump.h
> deleted file mode 100644
> index f960b2896606..000000000000
> --- a/arch/s390/include/asm/ptdump.h
> +++ /dev/null
> @@ -1,14 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -
> -#ifndef _ASM_S390_PTDUMP_H
> -#define _ASM_S390_PTDUMP_H
> -
> -void ptdump_check_wx(void);
> -
> -static inline void debug_checkwx(void)
> -{
> - if (IS_ENABLED(CONFIG_DEBUG_WX))
> - ptdump_check_wx();
> -}
> -
> -#endif /* _ASM_S390_PTDUMP_H */
> diff --git a/arch/s390/mm/dump_pagetables.c b/arch/s390/mm/dump_pagetables.c
> index d37a8f607b71..8dcb4e0c71bd 100644
> --- a/arch/s390/mm/dump_pagetables.c
> +++ b/arch/s390/mm/dump_pagetables.c
> @@ -6,7 +6,6 @@
> #include <linux/mm.h>
> #include <linux/kfence.h>
> #include <linux/kasan.h>
> -#include <asm/ptdump.h>
> #include <asm/kasan.h>
> #include <asm/abs_lowcore.h>
> #include <asm/nospec-branch.h>
> diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
> index 43e612bc2bcd..d2e5eff9d1de 100644
> --- a/arch/s390/mm/init.c
> +++ b/arch/s390/mm/init.c
> @@ -37,7 +37,6 @@
> #include <asm/pgalloc.h>
> #include <asm/ctlreg.h>
> #include <asm/kfence.h>
> -#include <asm/ptdump.h>
> #include <asm/dma.h>
> #include <asm/abs_lowcore.h>
> #include <asm/tlb.h>
> @@ -109,7 +108,6 @@ void mark_rodata_ro(void)
>
> __set_memory_ro(__start_ro_after_init, __end_ro_after_init);
> pr_info("Write protected read-only-after-init data: %luk\n", size >> 10);
> - debug_checkwx();
> }
>
> int set_memory_encrypted(unsigned long vaddr, int numpages)
> diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
> index 9d077bca6a10..6c979028e521 100644
> --- a/arch/x86/include/asm/pgtable.h
> +++ b/arch/x86/include/asm/pgtable.h
> @@ -32,6 +32,7 @@ void ptdump_walk_pgd_level(struct seq_file *m, struct mm_struct *mm);
> void ptdump_walk_pgd_level_debugfs(struct seq_file *m, struct mm_struct *mm,
> bool user);
> void ptdump_walk_pgd_level_checkwx(void);
> +#define ptdump_check_wx ptdump_walk_pgd_level_checkwx
> void ptdump_walk_user_pgd_level_checkwx(void);
>
> /*
> @@ -41,10 +42,8 @@ void ptdump_walk_user_pgd_level_checkwx(void);
> #define pgprot_decrypted(prot) __pgprot(cc_mkdec(pgprot_val(prot)))
>
> #ifdef CONFIG_DEBUG_WX
> -#define debug_checkwx() ptdump_walk_pgd_level_checkwx()
> #define debug_checkwx_user() ptdump_walk_user_pgd_level_checkwx()
> #else
> -#define debug_checkwx() do { } while (0)
> #define debug_checkwx_user() do { } while (0)
> #endif
>
> diff --git a/arch/x86/mm/dump_pagetables.c b/arch/x86/mm/dump_pagetables.c
> index e1b599ecbbc2..0008524eebe9 100644
> --- a/arch/x86/mm/dump_pagetables.c
> +++ b/arch/x86/mm/dump_pagetables.c
> @@ -433,6 +433,9 @@ void ptdump_walk_user_pgd_level_checkwx(void)
>
> void ptdump_walk_pgd_level_checkwx(void)
> {
> + if (!(__supported_pte_mask & _PAGE_NX))
> + return;
> +
> ptdump_walk_pgd_level_core(NULL, &init_mm, INIT_PGD, true, false);
> }
>
> diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
> index b63403d7179d..5c736b707cae 100644
> --- a/arch/x86/mm/init_32.c
> +++ b/arch/x86/mm/init_32.c
> @@ -800,6 +800,4 @@ void mark_rodata_ro(void)
> set_pages_ro(virt_to_page(start), size >> PAGE_SHIFT);
> #endif
> mark_nxdata_nx();
> - if (__supported_pte_mask & _PAGE_NX)
> - debug_checkwx();
> }
> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
> index a0dffaca6d2b..ebdbcae48011 100644
> --- a/arch/x86/mm/init_64.c
> +++ b/arch/x86/mm/init_64.c
> @@ -1412,8 +1412,6 @@ void mark_rodata_ro(void)
> (void *)text_end, (void *)rodata_start);
> free_kernel_image_pages("unused kernel image (rodata/data gap)",
> (void *)rodata_end, (void *)_sdata);
> -
> - debug_checkwx();
> }
>
> /*
> diff --git a/include/linux/ptdump.h b/include/linux/ptdump.h
> index 2a3a95586425..c10513739bf9 100644
> --- a/include/linux/ptdump.h
> +++ b/include/linux/ptdump.h
> @@ -19,5 +19,12 @@ struct ptdump_state {
> };
>
> void ptdump_walk_pgd(struct ptdump_state *st, struct mm_struct *mm, pgd_t *pgd);
> +void ptdump_check_wx(void);
> +
> +static inline void debug_checkwx(void)
> +{
> + if (IS_ENABLED(CONFIG_DEBUG_WX))
> + ptdump_check_wx();
> +}
>
> #endif /* _LINUX_PTDUMP_H */
> diff --git a/init/main.c b/init/main.c
> index e24b0780fdff..749a9f8d2c9b 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -99,6 +99,7 @@
> #include <linux/init_syscalls.h>
> #include <linux/stackdepot.h>
> #include <linux/randomize_kstack.h>
> +#include <linux/ptdump.h>
> #include <net/net_namespace.h>
>
> #include <asm/io.h>
> @@ -1408,6 +1409,7 @@ static void mark_readonly(void)
> */
> rcu_barrier();
> mark_rodata_ro();
> + debug_checkwx();
> rodata_test();
> } else
> pr_info("Kernel memory protection disabled.\n");
Acked-by: Palmer Dabbelt <palmer@rivosinc.com> # RISC-V
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] irqchip/alpine-msi: Fix off-by-one on allocation error path
From: Zenghui Yu @ 2024-03-27 14:23 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel
Cc: tglx, maz, wanghaibin.wang, Zenghui Yu, Tsahee Zidenberg,
Antoine Tenart
When alpine_msix_gic_domain_alloc() returns an error, there is an
off-by-one in the number of IRQs to be freed.
Fix it by passing the number of successfully allocated IRQs, instead of the
relative index of the last allocated one.
Fixes: 3841245e8498 ("irqchip/alpine-msi: Fix freeing of interrupts on allocation error path")
Cc: Tsahee Zidenberg <tsahee@annapurnalabs.com>
Cc: Antoine Tenart <atenart@kernel.org>
Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
---
drivers/irqchip/irq-alpine-msi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-alpine-msi.c b/drivers/irqchip/irq-alpine-msi.c
index 9c8b1349ee17..a1430ab60a8a 100644
--- a/drivers/irqchip/irq-alpine-msi.c
+++ b/drivers/irqchip/irq-alpine-msi.c
@@ -165,7 +165,7 @@ static int alpine_msix_middle_domain_alloc(struct irq_domain *domain,
return 0;
err_sgi:
- irq_domain_free_irqs_parent(domain, virq, i - 1);
+ irq_domain_free_irqs_parent(domain, virq, i);
alpine_msix_free_sgi(priv, sgi, nr_irqs);
return err;
}
--
2.33.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH RFC 1/3] mm/gup: consistently name GUP-fast functions
From: David Hildenbrand @ 2024-03-27 13:56 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: linux-kernel, Andrew Morton, Mike Rapoport, John Hubbard,
Peter Xu, linux-arm-kernel, loongarch, linux-mips, linuxppc-dev,
linux-s390, linux-sh, linux-mm, linux-perf-users, linux-fsdevel,
x86
In-Reply-To: <20240327135256.GG946323@nvidia.com>
On 27.03.24 14:52, Jason Gunthorpe wrote:
> On Wed, Mar 27, 2024 at 02:05:36PM +0100, David Hildenbrand wrote:
>> Let's consistently call the "fast-only" part of GUP "GUP-fast" and rename
>> all relevant internal functions to start with "gup_fast", to make it
>> clearer that this is not ordinary GUP. The current mixture of
>> "lockless", "gup" and "gup_fast" is confusing.
>>
>> Further, avoid the term "huge" when talking about a "leaf" -- for
>> example, we nowadays check pmd_leaf() because pmd_huge() is gone. For the
>> "hugepd"/"hugepte" stuff, it's part of the name ("is_hugepd"), so that
>> says.
>>
>> What remains is the "external" interface:
>> * get_user_pages_fast_only()
>> * get_user_pages_fast()
>> * pin_user_pages_fast()
>>
>> And the "internal" interface that handles GUP-fast + fallback:
>> * internal_get_user_pages_fast()
>
> This would like a better name too. How about gup_fast_fallback() ?
Yes, I was not able to come up with something I liked. But I do like
your proposal, so I'll do that!
[...]
>
> I think it is a great idea, it always takes a moment to figure out if
> a function is part of the fast callchain or not..
>
> (even better would be to shift the fast stuff into its own file, but I
> expect that is too much)
Yes, one step at a time :)
>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Thanks Jason!
--
Cheers,
David / dhildenb
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH RFC 1/3] mm/gup: consistently name GUP-fast functions
From: Jason Gunthorpe @ 2024-03-27 13:52 UTC (permalink / raw)
To: David Hildenbrand
Cc: linux-kernel, Andrew Morton, Mike Rapoport, John Hubbard,
Peter Xu, linux-arm-kernel, loongarch, linux-mips, linuxppc-dev,
linux-s390, linux-sh, linux-mm, linux-perf-users, linux-fsdevel,
x86
In-Reply-To: <20240327130538.680256-2-david@redhat.com>
On Wed, Mar 27, 2024 at 02:05:36PM +0100, David Hildenbrand wrote:
> Let's consistently call the "fast-only" part of GUP "GUP-fast" and rename
> all relevant internal functions to start with "gup_fast", to make it
> clearer that this is not ordinary GUP. The current mixture of
> "lockless", "gup" and "gup_fast" is confusing.
>
> Further, avoid the term "huge" when talking about a "leaf" -- for
> example, we nowadays check pmd_leaf() because pmd_huge() is gone. For the
> "hugepd"/"hugepte" stuff, it's part of the name ("is_hugepd"), so that
> says.
>
> What remains is the "external" interface:
> * get_user_pages_fast_only()
> * get_user_pages_fast()
> * pin_user_pages_fast()
>
> And the "internal" interface that handles GUP-fast + fallback:
> * internal_get_user_pages_fast()
This would like a better name too. How about gup_fast_fallback() ?
> The high-level internal function for GUP-fast is now:
> * gup_fast()
>
> The basic GUP-fast walker functions:
> * gup_pgd_range() -> gup_fast_pgd_range()
> * gup_p4d_range() -> gup_fast_p4d_range()
> * gup_pud_range() -> gup_fast_pud_range()
> * gup_pmd_range() -> gup_fast_pmd_range()
> * gup_pte_range() -> gup_fast_pte_range()
> * gup_huge_pgd() -> gup_fast_pgd_leaf()
> * gup_huge_pud() -> gup_fast_pud_leaf()
> * gup_huge_pmd() -> gup_fast_pmd_leaf()
>
> The weird hugepd stuff:
> * gup_huge_pd() -> gup_fast_hugepd()
> * gup_hugepte() -> gup_fast_hugepte()
>
> The weird devmap stuff:
> * __gup_device_huge_pud() -> gup_fast_devmap_pud_leaf()
> * __gup_device_huge_pmd -> gup_fast_devmap_pmd_leaf()
> * __gup_device_huge() -> gup_fast_devmap_leaf()
>
> Helper functions:
> * unpin_user_pages_lockless() -> gup_fast_unpin_user_pages()
> * gup_fast_folio_allowed() is already properly named
> * gup_fast_permitted() is already properly named
>
> With "gup_fast()", we now even have a function that is referred to in
> comment in mm/mmu_gather.c.
>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> mm/gup.c | 164 ++++++++++++++++++++++++++++---------------------------
> 1 file changed, 84 insertions(+), 80 deletions(-)
I think it is a great idea, it always takes a moment to figure out if
a function is part of the fast callchain or not..
(even better would be to shift the fast stuff into its own file, but I
expect that is too much)
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Jason
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v2 0/2] Enable JPEG encoding on rk3588
From: Emmanuel Gil Peyrot @ 2024-03-27 13:41 UTC (permalink / raw)
To: linux-kernel
Cc: Emmanuel Gil Peyrot, Ezequiel Garcia, Philipp Zabel,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiko Stuebner, Joerg Roedel, Will Deacon,
Robin Murphy, Sebastian Reichel, Cristian Ciocaltea, Dragan Simic,
Shreeya Patel, Chris Morgan, Andy Yan, Nicolas Frattaroli,
linux-media, linux-rockchip, devicetree, linux-arm-kernel, iommu
Only the JPEG encoder is available for now, although there are patches
for the undocumented VP8 encoder floating around[0].
This has been tested on a rock-5b, resulting in four /dev/video*
encoders. The userspace program I’ve been using to test them is
Onix[1], using the jpeg-encoder example, it will pick one of these four
at random (but displays the one it picked):
% ffmpeg -i <input image> -pix_fmt yuvj420p temp.yuv
% jpeg-encoder temp.yuv <width> <height> NV12 <quality> output.jpeg
[0] https://patchwork.kernel.org/project/linux-rockchip/list/?series=789885
[1] https://crates.io/crates/onix
Changes since v1:
- Dropped patches 1 and 4.
- Use the proper compatible form, since this device should be fully
compatible with the VEPU of rk356x.
- Describe where the VEPU121 name comes from, and list other encoders
and decoders present in this SoC.
- Properly test the device tree changes, I previously couldn’t since I
was using a too recent version of python-jsonschema…
Emmanuel Gil Peyrot (2):
media: dt-binding: media: Document rk3588’s VEPU121
arm64: dts: rockchip: Add VEPU121 to rk3588
.../bindings/media/rockchip,rk3568-vepu.yaml | 8 +-
arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 80 +++++++++++++++++++
2 files changed, 86 insertions(+), 2 deletions(-)
--
2.44.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v2 2/2] arm64: dts: rockchip: Add VEPU121 to rk3588
From: Emmanuel Gil Peyrot @ 2024-03-27 13:41 UTC (permalink / raw)
To: linux-kernel
Cc: Emmanuel Gil Peyrot, Ezequiel Garcia, Philipp Zabel,
Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Heiko Stuebner, Joerg Roedel, Will Deacon,
Robin Murphy, Sebastian Reichel, Cristian Ciocaltea, Dragan Simic,
Shreeya Patel, Chris Morgan, Andy Yan, Nicolas Frattaroli,
linux-media, linux-rockchip, devicetree, linux-arm-kernel, iommu
In-Reply-To: <20240327134115.424846-1-linkmauve@linkmauve.fr>
The TRM (version 1.0 page 385) lists five VEPU121 cores, but only four
interrupts are listed (on page 24), so I’ve only enabled four of them
for now.
Signed-off-by: Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>
---
arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 80 +++++++++++++++++++++++
1 file changed, 80 insertions(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
index 87b83c87bd55..510ed3db9d01 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
@@ -2488,6 +2488,86 @@ gpio4: gpio@fec50000 {
};
};
+ jpeg_enc0: video-codec@fdba0000 {
+ compatible = "rockchip,rk3588-vepu121", "rockchip,rk3568-vepu";
+ reg = <0x0 0xfdba0000 0x0 0x800>;
+ interrupts = <GIC_SPI 122 IRQ_TYPE_LEVEL_HIGH 0>;
+ clocks = <&cru ACLK_JPEG_ENCODER0>, <&cru HCLK_JPEG_ENCODER0>;
+ clock-names = "aclk", "hclk";
+ iommus = <&jpeg_enc0_mmu>;
+ power-domains = <&power RK3588_PD_VDPU>;
+ };
+
+ jpeg_enc0_mmu: iommu@fdba0800 {
+ compatible = "rockchip,rk3588-iommu", "rockchip,rk3568-iommu";
+ reg = <0x0 0xfdba0800 0x0 0x40>;
+ interrupts = <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH 0>;
+ clocks = <&cru ACLK_JPEG_ENCODER0>, <&cru HCLK_JPEG_ENCODER0>;
+ clock-names = "aclk", "iface";
+ power-domains = <&power RK3588_PD_VDPU>;
+ #iommu-cells = <0>;
+ };
+
+ jpeg_enc1: video-codec@fdba4000 {
+ compatible = "rockchip,rk3588-vepu121", "rockchip,rk3568-vepu";
+ reg = <0x0 0xfdba4000 0x0 0x800>;
+ interrupts = <GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH 0>;
+ clocks = <&cru ACLK_JPEG_ENCODER1>, <&cru HCLK_JPEG_ENCODER1>;
+ clock-names = "aclk", "hclk";
+ iommus = <&jpeg_enc1_mmu>;
+ power-domains = <&power RK3588_PD_VDPU>;
+ };
+
+ jpeg_enc1_mmu: iommu@fdba4800 {
+ compatible = "rockchip,rk3588-iommu", "rockchip,rk3568-iommu";
+ reg = <0x0 0xfdba4800 0x0 0x40>;
+ interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH 0>;
+ clocks = <&cru ACLK_JPEG_ENCODER1>, <&cru HCLK_JPEG_ENCODER1>;
+ clock-names = "aclk", "iface";
+ power-domains = <&power RK3588_PD_VDPU>;
+ #iommu-cells = <0>;
+ };
+
+ jpeg_enc2: video-codec@fdba8000 {
+ compatible = "rockchip,rk3588-vepu121", "rockchip,rk3568-vepu";
+ reg = <0x0 0xfdba8000 0x0 0x800>;
+ interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_HIGH 0>;
+ clocks = <&cru ACLK_JPEG_ENCODER2>, <&cru HCLK_JPEG_ENCODER2>;
+ clock-names = "aclk", "hclk";
+ iommus = <&jpeg_enc2_mmu>;
+ power-domains = <&power RK3588_PD_VDPU>;
+ };
+
+ jpeg_enc2_mmu: iommu@fdba8800 {
+ compatible = "rockchip,rk3588-iommu", "rockchip,rk3568-iommu";
+ reg = <0x0 0xfdba8800 0x0 0x40>;
+ interrupts = <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH 0>;
+ clocks = <&cru ACLK_JPEG_ENCODER2>, <&cru HCLK_JPEG_ENCODER2>;
+ clock-names = "aclk", "iface";
+ power-domains = <&power RK3588_PD_VDPU>;
+ #iommu-cells = <0>;
+ };
+
+ jpeg_enc3: video-codec@fdbac000 {
+ compatible = "rockchip,rk3588-vepu121", "rockchip,rk3568-vepu";
+ reg = <0x0 0xfdbac000 0x0 0x800>;
+ interrupts = <GIC_SPI 128 IRQ_TYPE_LEVEL_HIGH 0>;
+ clocks = <&cru ACLK_JPEG_ENCODER3>, <&cru HCLK_JPEG_ENCODER3>;
+ clock-names = "aclk", "hclk";
+ iommus = <&jpeg_enc3_mmu>;
+ power-domains = <&power RK3588_PD_VDPU>;
+ };
+
+ jpeg_enc3_mmu: iommu@fdbac800 {
+ compatible = "rockchip,rk3588-iommu", "rockchip,rk3568-iommu";
+ reg = <0x0 0xfdbac800 0x0 0x40>;
+ interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_HIGH 0>;
+ clocks = <&cru ACLK_JPEG_ENCODER3>, <&cru HCLK_JPEG_ENCODER3>;
+ clock-names = "aclk", "iface";
+ power-domains = <&power RK3588_PD_VDPU>;
+ #iommu-cells = <0>;
+ };
+
av1d: video-codec@fdc70000 {
compatible = "rockchip,rk3588-av1-vpu";
reg = <0x0 0xfdc70000 0x0 0x800>;
--
2.44.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [RESEND][PATCH v2 4/4] soc: samsung: exynos-asv: Update Energy Model after adjusting voltage
From: Lukasz Luba @ 2024-03-27 13:39 UTC (permalink / raw)
To: Dietmar Eggemann
Cc: linux-arm-kernel, sboyd, nm, linux-samsung-soc, daniel.lezcano,
rafael, viresh.kumar, krzysztof.kozlowski, alim.akhtar,
m.szyprowski, mhiramat, linux-kernel, linux-pm
In-Reply-To: <e02ca745-52df-4210-b175-f4ef278d81d8@arm.com>
On 3/27/24 12:56, Dietmar Eggemann wrote:
> On 26/03/2024 21:12, Lukasz Luba wrote:
>> Hi Dietmar,
>>
>> On 3/26/24 11:20, Dietmar Eggemann wrote:
>>> On 22/03/2024 12:08, Lukasz Luba wrote:
>>>
>>> [...]
>>>
>>>> @@ -97,9 +98,17 @@ static int exynos_asv_update_opps(struct
>>>> exynos_asv *asv)
>>>> last_opp_table = opp_table;
>>>> ret = exynos_asv_update_cpu_opps(asv, cpu);
>>>> - if (ret < 0)
>>>> + if (!ret) {
>>>> + /*
>>>> + * When the voltage for OPPs successfully
>>>> + * changed, update the EM power values to
>>>> + * reflect the reality and not use stale data
>>>
>>> At this point, can we really say that the voltage has changed?
>>>
>>> exynos_asv_update_cpu_opps()
>>>
>>> ...
>>> ret = dev_pm_opp_adjust_voltage()
>>> if (!ret)
>>> em_dev_update_chip_binning()
>>> ...
>>>
>>> dev_pm_opp_adjust_voltage() also returns 0 when the voltage value stays
>>> the same?
>>>
>>> [...]
>>
>> The comment for the dev_pm_opp_adjust_voltage() says that it
>> returns 0 if no modification was done or modification was
>> successful. So I cannot distinguish in that driver code, but
>> also there is no additional need to do it IMO (even framework
>> doesn't do this).
>
> Precisely. That's why the added comment in exynos_asv_update_opps():
> "When the voltage for OPPs successfully __changed__, ..." is somehow
> misleading IMHO.
>
I got your point. Let me re-phrase that comment to reflect this
OPP function return properly.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ 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