* Re: [PATCH v4 18/26] arm64: mte: Add PTRACE_{PEEK,POKE}MTETAGS support
From: Catalin Marinas @ 2020-06-01 12:07 UTC (permalink / raw)
To: Luis Machado
Cc: linux-arch, Omair Javaid, Szabolcs Nagy, Andrey Konovalov,
Kevin Brodsky, Peter Collingbourne, linux-mm, Alan Hayward,
Vincenzo Frascino, Will Deacon, Dave P Martin, linux-arm-kernel
In-Reply-To: <a6fb329c-b4ad-9ffa-5344-601348978c34@linaro.org>
On Fri, May 29, 2020 at 06:25:14PM -0300, Luis Machado wrote:
> I have a question about siginfo MTE information. I suppose SEGV_MTESERR will
> be the most useful setting for debugging, right? Does si_addr contain the
> tagged pointer with the logical tag, a zero-tagged memory address or a
> tagged pointer with the allocation tag?
The si_addr is zero-tagged currently. We were planning to expose the tag
in FAR_EL1 as a separate siginfo field. See these discussions:
https://lore.kernel.org/linux-arm-kernel/20200513180914.50892-1-pcc@google.com/
https://lore.kernel.org/linux-arm-kernel/20200521022943.195898-1-pcc@google.com/
In theory, we could add the tag to si_addr for SEGV_MTESERR, it
shouldn't break the existing ABI (well, it depends on how you look at
it).
> From the debugger user's perspective, one would want to see both the logical
> tag and the allocation tag. And it would be handy to have both available in
> siginfo. Does that make sense?
The debugger can access the allocation tag via PTRACE_PEEKMTETAGS. I
don't think the kernel should provide this in siginfo. Also, the signal
handler can do an LDG and read the allocation tag directly, no need for
it to be in siginfo.
> Also, when would we see SEGV_MTEAERR, for example? That would provide no
> additional information about a particular memory address, which is not that
> useful for the debugger.
Yeah, we can't really do much here since the hardware doesn't provide us
such information. The async mode is only useful as a general test to see
if your program has MTE faults but for actual debugging you'd have to
switch to synchronous. For glibc at least, I think the mode can be
driven by an environment variable.
--
Catalin
_______________________________________________
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 v8 0/5] support reserving crashkernel above 4G on arm64 kdump
From: Prabhakar Kushwaha @ 2020-06-01 12:02 UTC (permalink / raw)
To: Chen Zhou
Cc: horms, John Donnelly, bhe, Will Deacon, devicetree,
Catalin Marinas, Linux Doc Mailing List, kexec mailing list,
Linux Kernel Mailing List, robh+dt, mingo, arnd, guohanjun,
Thomas Gleixner, Prabhakar Kushwaha, dyoung, linux-arm-kernel
In-Reply-To: <20200521093805.64398-1-chenzhou10@huawei.com>
Hi Chen,
On Thu, May 21, 2020 at 3:05 PM Chen Zhou <chenzhou10@huawei.com> wrote:
>
> This patch series enable reserving crashkernel above 4G in arm64.
>
> There are following issues in arm64 kdump:
> 1. We use crashkernel=X to reserve crashkernel below 4G, which will fail
> when there is no enough low memory.
> 2. Currently, crashkernel=Y@X can be used to reserve crashkernel above 4G,
> in this case, if swiotlb or DMA buffers are required, crash dump kernel
> will boot failure because there is no low memory available for allocation.
>
> To solve these issues, introduce crashkernel=X,low to reserve specified
> size low memory.
> Crashkernel=X tries to reserve memory for the crash dump kernel under
> 4G. If crashkernel=Y,low is specified simultaneously, reserve spcified
> size low memory for crash kdump kernel devices firstly and then reserve
> memory above 4G.
>
> When crashkernel is reserved above 4G in memory, that is, crashkernel=X,low
> is specified simultaneously, kernel should reserve specified size low memory
> for crash dump kernel devices. So there may be two crash kernel regions, one
> is below 4G, the other is above 4G.
> In order to distinct from the high region and make no effect to the use of
> kexec-tools, rename the low region as "Crash kernel (low)", and add DT property
> "linux,low-memory-range" to crash dump kernel's dtb to pass the low region.
>
> Besides, we need to modify kexec-tools:
> arm64: kdump: add another DT property to crash dump kernel's dtb(see [1])
>
> The previous changes and discussions can be retrieved from:
>
> Changes since [v7]
> - Move x86 CRASH_ALIGN to 2M
> Suggested by Dave and do some test, move x86 CRASH_ALIGN to 2M.
> - Update Documentation/devicetree/bindings/chosen.txt
> Add corresponding documentation to Documentation/devicetree/bindings/chosen.txt suggested by Arnd.
> - Add Tested-by from Jhon and pk
>
> Changes since [v6]
> - Fix build errors reported by kbuild test robot.
>
> Changes since [v5]
> - Move reserve_crashkernel_low() into kernel/crash_core.c.
> - Delete crashkernel=X,high.
> - Modify crashkernel=X,low.
> If crashkernel=X,low is specified simultaneously, reserve spcified size low
> memory for crash kdump kernel devices firstly and then reserve memory above 4G.
> In addition, rename crashk_low_res as "Crash kernel (low)" for arm64, and then
> pass to crash dump kernel by DT property "linux,low-memory-range".
> - Update Documentation/admin-guide/kdump/kdump.rst.
>
> Changes since [v4]
> - Reimplement memblock_cap_memory_ranges for multiple ranges by Mike.
>
> Changes since [v3]
> - Add memblock_cap_memory_ranges back for multiple ranges.
> - Fix some compiling warnings.
>
> Changes since [v2]
> - Split patch "arm64: kdump: support reserving crashkernel above 4G" as
> two. Put "move reserve_crashkernel_low() into kexec_core.c" in a separate
> patch.
>
> Changes since [v1]:
> - Move common reserve_crashkernel_low() code into kernel/kexec_core.c.
> - Remove memblock_cap_memory_ranges() i added in v1 and implement that
> in fdt_enforce_memory_region().
> There are at most two crash kernel regions, for two crash kernel regions
> case, we cap the memory range [min(regs[*].start), max(regs[*].end)]
> and then remove the memory range in the middle.
>
> [1]: http://lists.infradead.org/pipermail/kexec/2020-May/025128.html
> [v1]: https://lkml.org/lkml/2019/4/2/1174
> [v2]: https://lkml.org/lkml/2019/4/9/86
> [v3]: https://lkml.org/lkml/2019/4/9/306
> [v4]: https://lkml.org/lkml/2019/4/15/273
> [v5]: https://lkml.org/lkml/2019/5/6/1360
> [v6]: https://lkml.org/lkml/2019/8/30/142
> [v7]: https://lkml.org/lkml/2019/12/23/411
>
> Chen Zhou (5):
> x86: kdump: move reserve_crashkernel_low() into crash_core.c
> arm64: kdump: reserve crashkenel above 4G for crash dump kernel
> arm64: kdump: add memory for devices by DT property, low-memory-range
> kdump: update Documentation about crashkernel on arm64
> dt-bindings: chosen: Document linux,low-memory-range for arm64 kdump
>
We are getting "warn_alloc" [1] warning during boot of kdump kernel
with bootargs as [2] of primary kernel.
This error observed on ThunderX2 ARM64 platform.
It is observed with latest upstream tag (v5.7-rc3) with this patch set
and https://lists.infradead.org/pipermail/kexec/2020-May/025128.html
Also **without** this patch-set
"https://www.spinics.net/lists/arm-kernel/msg806882.html"
This issue comes whenever crashkernel memory is reserved after 0xc000_0000.
More details discussed earlier in
https://www.spinics.net/lists/arm-kernel/msg806882.html without any
solution
This patch-set is expected to solve similar kind of issue.
i.e. low memory is only targeted for DMA, swiotlb; So above mentioned
observation should be considered/fixed. .
--pk
[1]
[ 30.366695] DMI: Cavium Inc. Saber/Saber, BIOS
TX2-FW-Release-3.1-build_01-2803-g74253a541a mm/dd/yyyy
[ 30.367696] NET: Registered protocol family 16
[ 30.369973] swapper/0: page allocation failure: order:6,
mode:0x1(GFP_DMA), nodemask=(null),cpuset=/,mems_allowed=0
[ 30.369980] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.7.0-rc3+ #121
[ 30.369981] Hardware name: Cavium Inc. Saber/Saber, BIOS
TX2-FW-Release-3.1-build_01-2803-g74253a541a mm/dd/yyyy
[ 30.369984] Call trace:
[ 30.369989] dump_backtrace+0x0/0x1f8
[ 30.369991] show_stack+0x20/0x30
[ 30.369997] dump_stack+0xc0/0x10c
[ 30.370001] warn_alloc+0x10c/0x178
[ 30.370004] __alloc_pages_slowpath.constprop.111+0xb10/0xb50
[ 30.370006] __alloc_pages_nodemask+0x2b4/0x300
[ 30.370008] alloc_page_interleave+0x24/0x98
[ 30.370011] alloc_pages_current+0xe4/0x108
[ 30.370017] dma_atomic_pool_init+0x44/0x1a4
[ 30.370020] do_one_initcall+0x54/0x228
[ 30.370027] kernel_init_freeable+0x228/0x2cc
[ 30.370031] kernel_init+0x1c/0x110
[ 30.370034] ret_from_fork+0x10/0x18
[ 30.370036] Mem-Info:
[ 30.370064] active_anon:0 inactive_anon:0 isolated_anon:0
[ 30.370064] active_file:0 inactive_file:0 isolated_file:0
[ 30.370064] unevictable:0 dirty:0 writeback:0 unstable:0
[ 30.370064] slab_reclaimable:34 slab_unreclaimable:4438
[ 30.370064] mapped:0 shmem:0 pagetables:14 bounce:0
[ 30.370064] free:1537719 free_pcp:219 free_cma:0
[ 30.370070] Node 0 active_anon:0kB inactive_anon:0kB
active_file:0kB inactive_file:0kB unevictable:0kB isolated(anon):0kB
isolated(file):0kB mapped:0kB dirty:0kB writeback:0kB shmem:0kB
shmem_thp: 0kB shmem_pmdmapped: 0kB anon_thp: 0kB writeback_tmp:0kB
unstable:0kB all_unreclaimable? no
[ 30.370073] Node 1 active_anon:0kB inactive_anon:0kB
active_file:0kB inactive_file:0kB unevictable:0kB isolated(anon):0kB
isolated(file):0kB mapped:0kB dirty:0kB writeback:0kB shmem:0kB
shmem_thp: 0kB shmem_pmdmapped: 0kB anon_thp: 0kB writeback_tmp:0kB
unstable:0kB all_unreclaimable? no
[ 30.370079] Node 0 DMA free:0kB min:0kB low:0kB high:0kB
reserved_highatomic:0KB active_anon:0kB inactive_anon:0kB
active_file:0kB inactive_file:0kB unevictable:0kB writepending:0kB
present:128kB managed:0kB mlocked:0kB kernel_stack:0kB pagetables:0kB
bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB
[ 30.370084] lowmem_reserve[]: 0 250 6063 6063
[ 30.370090] Node 0 DMA32 free:256000kB min:408kB low:664kB
high:920kB reserved_highatomic:0KB active_anon:0kB inactive_anon:0kB
active_file:0kB inactive_file:0kB unevictable:0kB writepending:0kB
present:269700kB managed:256000kB mlocked:0kB kernel_stack:0kB
pagetables:0kB bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB
[ 30.370094] lowmem_reserve[]: 0 0 5813 5813
[ 30.370100] Node 0 Normal free:5894876kB min:9552kB low:15504kB
high:21456kB reserved_highatomic:0KB active_anon:0kB inactive_anon:0kB
active_file:0kB inactive_file:0kB unevictable:0kB writepending:0kB
present:8388608kB managed:5953112kB mlocked:0kB kernel_stack:21672kB
pagetables:56kB bounce:0kB free_pcp:876kB local_pcp:176kB free_cma:0kB
[ 30.370104] lowmem_reserve[]: 0 0 0 0
[ 30.370107] Node 0 DMA: 0*4kB 0*8kB 0*16kB 0*32kB 0*64kB 0*128kB
0*256kB 0*512kB 0*1024kB 0*2048kB 0*4096kB = 0kB
[ 30.370113] Node 0 DMA32: 0*4kB 0*8kB 0*16kB 0*32kB 0*64kB 0*128kB
0*256kB 0*512kB 0*1024kB 1*2048kB (M) 62*4096kB (M) = 256000kB
[ 30.370119] Node 0 Normal: 2*4kB (M) 3*8kB (ME) 2*16kB (UE) 3*32kB
(UM) 1*64kB (U) 2*128kB (M) 2*256kB (ME) 3*512kB (ME) 3*1024kB (ME)
3*2048kB (UME) 1436*4096kB (M) = 5893600kB
[ 30.370129] Node 0 hugepages_total=0 hugepages_free=0
hugepages_surp=0 hugepages_size=1048576kB
[ 30.370130] 0 total pagecache pages
[ 30.370132] 0 pages in swap cache
[ 30.370134] Swap cache stats: add 0, delete 0, find 0/0
[ 30.370135] Free swap = 0kB
[ 30.370136] Total swap = 0kB
[ 30.370137] 2164609 pages RAM
[ 30.370139] 0 pages HighMem/MovableOnly
[ 30.370140] 612331 pages reserved
[ 30.370141] 0 pages hwpoisoned
[ 30.370143] DMA: failed to allocate 256 KiB pool for atomic
coherent allocation
[2]
root@localhost$ dmesg | grep crash
[ 0.000000] Reserving 250MB of low memory at 3724MB for crashkernel
(System low RAM: 2029MB)
[ 0.000000] crashkernel reserved: 0x0000000e00000000 -
0x0000001000000000 (8192 MB)
[ 0.000000] Kernel command line:
BOOT_IMAGE=(hd11,gpt2)/vmlinuz-5.7.0-rc3+
root=UUID=e5c34f86-6727-4668-81f9-f41433555df6 ro crashkernel=250M,low
crashkernel=8G nowatchdog console=ttyAMA0 default_hugepagesz=1G
hugepagesz=1G hugepages=2
[ 44.019393] crashkernel=8G
_______________________________________________
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 5/6] mm: tlb: Provide flush_*_tlb_range wrappers
From: Catalin Marinas @ 2020-06-01 11:56 UTC (permalink / raw)
To: Zhenyu Ye
Cc: mark.rutland, peterz, linux-mm, guohanjun, will, linux-arch,
yuzhao, aneesh.kumar, steven.price, arm, Dave.Martin, arnd,
suzuki.poulose, npiggin, zhangshaokun, broonie, rostedt,
prime.zeng, kuhn.chenqun, tglx, linux-arm-kernel, xiexiangyou,
linux-kernel, maz, akpm
In-Reply-To: <0c6f79e4-f29a-d373-2e43-c4f87cf78b49@huawei.com>
Hi Zhenyu,
On Sat, May 30, 2020 at 06:24:21PM +0800, Zhenyu Ye wrote:
> On 2020/5/26 22:52, Catalin Marinas wrote:
> > On Mon, May 25, 2020 at 03:19:42PM +0800, Zhenyu Ye wrote:
> >> tlb_flush_##_pxx##_range() is used to set tlb->cleared_*,
> >> flush_##_pxx##_tlb_range() will actually flush the TLB entry.
> >>
> >> In arch64, tlb_flush_p?d_range() is defined as:
> >>
> >> #define flush_pmd_tlb_range(vma, addr, end) flush_tlb_range(vma, addr, end)
> >> #define flush_pud_tlb_range(vma, addr, end) flush_tlb_range(vma, addr, end)
> >
> > Currently, flush_p??_tlb_range() are generic and defined as above. I
> > think in the generic code they can remain an alias for
> > flush_tlb_range().
> >
> > On arm64, we can redefine them as:
> >
> > #define flush_pte_tlb_range(vma, addr, end) __flush_tlb_range(vma, addr, end, 3)
> > #define flush_pmd_tlb_range(vma, addr, end) __flush_tlb_range(vma, addr, end, 2)
> > #define flush_pud_tlb_range(vma, addr, end) __flush_tlb_range(vma, addr, end, 1)
> > #define flush_p4d_tlb_range(vma, addr, end) __flush_tlb_range(vma, addr, end, 0)
> >
> > (unless the compiler optimises away all the mmu_gather stuff in your
> > macro above but they don't look trivial to me)
>
> I changed generic code before considering that other structures may also
> use this feature, such as Power9. And Peter may want to replace all
> flush_tlb_range() by tlb_flush() in the future, see [1] for details.
>
> If only enable this feature on aarch64, your codes are better.
>
> [1] https://lore.kernel.org/linux-arm-kernel/20200402163849.GM20713@hirez.programming.kicks-ass.net/
But we change the semantics slightly if we implement these as
mmu_gather. For example, tlb_end_vma() -> tlb_flush_mmu_tlbonly() ends
up calling mmu_notifier_invalidate_range() which it didn't before. I
think we end up invoking the notifier unnecessarily in some cases (see
the comment in __split_huge_pmd()) or we end up calling the notifier
twice (e.g. pmdp_huge_clear_flush_notify()).
> > Also, I don't see the new flush_pte_* and flush_p4d_* macros used
> > anywhere and I don't think they are needed. The pte equivalent is
> > flush_tlb_page() (we need to make sure it's not used on a pmd in the
> > hugetlb context).
>
> flush_tlb_page() is used to flush only one page. If we add the
> flush_pte_tlb_range(), then we can use it to flush a range of pages in
> the future.
If we know flush_tlb_page() is only called on a small page, could we add
TTL information here as well?
> But flush_pte_* and flush_p4d_* macros are really not used anywhere. I
> will remove them in next version of series, and add them if someone
> needs.
I think it makes sense.
--
Catalin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH RFC 2/2] iommu/arm-smmu-v3: Remove cmpxchg() in arm_smmu_cmdq_issue_cmdlist()
From: John Garry @ 2020-06-01 11:50 UTC (permalink / raw)
To: will, robin.murphy
Cc: song.bao.hua, maz, joro, John Garry, iommu, linux-arm-kernel
In-Reply-To: <1591012248-37956-1-git-send-email-john.garry@huawei.com>
It has been shown that the cmpxchg() for finding space in the cmdq can
be a bottleneck:
- For more CPUs contenting the cmdq, cmpxchg() will fail more often.
- Since the software-maintained cons pointer is updated on the same 64b
memory region, the chance of cmpxchg() failure increases again.
The cmpxchg() is removed as part of 2 related changes:
- If a CMD_SYNC is always issued for a batch, the cmdq can - in theory -
never fill, since we always wait for a CMD_SYNC to be consumed. We must
issue the CMD_SYNC so that a CPU will be always limited to issuing
max_cmd_per_batch commands. Generally for DMA unmap ops, a CMD_SYNC
is always issued anyway.
As such, the cmdq locking is removed, and we only longer maintain cons
in software (this needs to be revised for !MSI support).
- Update prod and cmdq owner in a single operation. For this, we count the
prod and owner in separate regions in arm_smmu_ll_queue.prod.
As with simple binary counting, once the prod+wrap fields overflow, they
will zero. They will overflow into "owner" region, but this is ok as we
have accounted for the extra value.
As for the "owner", we now count this value, instead of setting a flag.
Similar to before, once the owner has finished gathering, it will clear
this mask. As such, a CPU declares itself as the "owner" when it reads
zero for this field. This zeroing will also clear possible overflow in
wrap+prod region.
Signed-off-by: John Garry <john.garry@huawei.com>
---
drivers/iommu/arm-smmu-v3.c | 58 +++++++++++----------------------------------
1 file changed, 14 insertions(+), 44 deletions(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index e607ab5a4cbd..d6c7d82f9cf8 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1375,7 +1375,7 @@ static int arm_smmu_cmdq_issue_cmdlist(struct arm_smmu_device *smmu,
u64 *cmds, int n, bool sync)
{
u64 cmd_sync[CMDQ_ENT_DWORDS];
- u32 prod;
+ u32 prod, prodx;
unsigned long flags;
bool owner;
struct arm_smmu_cmdq *cmdq = &smmu->cmdq;
@@ -1383,33 +1383,21 @@ static int arm_smmu_cmdq_issue_cmdlist(struct arm_smmu_device *smmu,
.max_n_shift = cmdq->q.llq.max_n_shift,
}, head = llq;
int ret = 0;
+ u32 owner_val = 1 << cmdq->q.llq.owner_count_shift;
+ u32 prod_mask = GENMASK(cmdq->q.llq.max_n_shift, 0);
+ u32 owner_mask = GENMASK(30, cmdq->q.llq.owner_count_shift);
+
+ /* always issue a CMD_SYNC TODO: fixup callers for this */
+ sync = true;
/* 1. Allocate some space in the queue */
local_irq_save(flags);
- llq.val = READ_ONCE(cmdq->q.llq.val);
- do {
- u64 old;
-
- while (!queue_has_space(&llq, n + sync)) {
- local_irq_restore(flags);
- if (arm_smmu_cmdq_poll_until_not_full(smmu, &llq))
- dev_err_ratelimited(smmu->dev, "CMDQ timeout\n");
- local_irq_save(flags);
- }
- head.cons = llq.cons;
- head.prod = queue_inc_prod_n(&llq, n + sync) |
- CMDQ_PROD_OWNED_FLAG;
+ prodx = atomic_fetch_add(n + sync + owner_val, &cmdq->q.llq.atomic.prod);
- old = cmpxchg_relaxed(&cmdq->q.llq.val, llq.val, head.val);
- if (old == llq.val)
- break;
-
- llq.val = old;
- } while (1);
- owner = !(llq.prod & CMDQ_PROD_OWNED_FLAG);
- head.prod &= ~CMDQ_PROD_OWNED_FLAG;
- llq.prod &= ~CMDQ_PROD_OWNED_FLAG;
+ owner = !(prodx & owner_mask);
+ llq.prod = prod_mask & prodx;
+ head.prod = queue_inc_prod_n(&llq, n + sync);
/*
* 2. Write our commands into the queue As with simple binary counting, once the prod+wrap fields overflow, they
will zero. They will overflow into "owner" region, but this is ok as we
have accounted for the extra value.
@@ -1420,14 +1408,6 @@ static int arm_smmu_cmdq_issue_cmdlist(struct arm_smmu_device *smmu,
prod = queue_inc_prod_n(&llq, n);
arm_smmu_cmdq_build_sync_cmd(cmd_sync, smmu, prod);
queue_write(Q_ENT(&cmdq->q, prod), cmd_sync, CMDQ_ENT_DWORDS);
-
- /*
- * In order to determine completion of our CMD_SYNC, we must
- * ensure that the queue can't wrap twice without us noticing.
- * We achieve that by taking the cmdq lock as shared before
- * marking our slot as valid.
- */
- arm_smmu_cmdq_shared_lock(cmdq);
}
/* 3. Mark our slots as valid, ensuring commands are visible first */
@@ -1439,11 +1419,10 @@ static int arm_smmu_cmdq_issue_cmdlist(struct arm_smmu_device *smmu,
/* a. Wait for previous owner to finish */
atomic_cond_read_relaxed(&cmdq->owner_prod, VAL == llq.prod);
- /* b. Stop gathering work by clearing the owned flag */
- prod = atomic_fetch_andnot_relaxed(CMDQ_PROD_OWNED_FLAG,
+ /* b. Stop gathering work by clearing the owned mask */
+ prod = atomic_fetch_andnot_relaxed(owner_mask,
&cmdq->q.llq.atomic.prod);
- prod &= ~CMDQ_PROD_OWNED_FLAG;
-
+ prod &= prod_mask;
/*
* c. Wait for any gathered work to be written to the queue.
* Note that we read our own entries so that we have the control
@@ -1476,15 +1455,6 @@ static int arm_smmu_cmdq_issue_cmdlist(struct arm_smmu_device *smmu,
readl_relaxed(cmdq->q.prod_reg),
readl_relaxed(cmdq->q.cons_reg));
}
-
- /*
- * Try to unlock the cmq lock. This will fail if we're the last
- * reader, in which case we can safely update cmdq->q.llq.cons
- */
- if (!arm_smmu_cmdq_shared_tryunlock(cmdq)) {
- WRITE_ONCE(cmdq->q.llq.cons, llq.cons);
- arm_smmu_cmdq_shared_unlock(cmdq);
- }
}
local_irq_restore(flags);
--
2.16.4
_______________________________________________
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 RFC 1/2] iommu/arm-smmu-v3: Calculate bits for prod and owner
From: John Garry @ 2020-06-01 11:50 UTC (permalink / raw)
To: will, robin.murphy
Cc: song.bao.hua, maz, joro, John Garry, iommu, linux-arm-kernel
In-Reply-To: <1591012248-37956-1-git-send-email-john.garry@huawei.com>
Since the arm_smmu_ll_queue.prod will be for counting the "owner" value
and also HW prod pointer, calculate how many bits are available for and
used by each.
This is based on the number of possible CPUs in the system. And we require
that each CPU can issue a minimum of 2 commands per batch - 1 x CMD_SYNC
and at least 1 x other.
Ignoring limits of HW max_n_shift and HW cmdq memory allocation, approx 16K
is the max supported CPUs. For this, max_n_shift would be 15.
Signed-off-by: John Garry <john.garry@huawei.com>
---
drivers/iommu/arm-smmu-v3.c | 34 +++++++++++++++++++++++++++++++++-
1 file changed, 33 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 82508730feb7..e607ab5a4cbd 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -530,6 +530,8 @@ struct arm_smmu_ll_queue {
u8 __pad[SMP_CACHE_BYTES];
} ____cacheline_aligned_in_smp;
u32 max_n_shift;
+ u32 max_cmd_per_batch;
+ u32 owner_count_shift;
};
struct arm_smmu_queue {
@@ -1512,7 +1514,10 @@ static void arm_smmu_cmdq_batch_add(struct arm_smmu_device *smmu,
struct arm_smmu_cmdq_batch *cmds,
struct arm_smmu_cmdq_ent *cmd)
{
- if (cmds->num == CMDQ_BATCH_ENTRIES) {
+ struct arm_smmu_cmdq *q = &smmu->cmdq;
+ struct arm_smmu_ll_queue *llq = &q->q.llq;
+
+ if (cmds->num == llq->max_cmd_per_batch) {
arm_smmu_cmdq_issue_cmdlist(smmu, cmds->cmds, cmds->num, false);
cmds->num = 0;
}
@@ -3156,8 +3161,24 @@ static int arm_smmu_init_one_queue(struct arm_smmu_device *smmu,
unsigned long cons_off,
size_t dwords, const char *name)
{
+ int cpus = num_possible_cpus();
size_t qsz;
+ /*
+ * We can get the number of bits required for owner counting by
+ * log2(nr possible cpus) + 1, but we have to take into account that he
+ * wrap+prod could overflow before the owner zeroes, so add 1
+ * more (to cpus) for bits_for_cmdq_owner calculation.
+ */
+ int bits_for_cmdq_owner = ilog2(cpus + 1) + 1;
+ /* 1-bit for overflow, 1-bit for wrap */
+ int bits_available_for_prod = 32 - 2 - bits_for_cmdq_owner;
+ int entries_for_prod;
+
+ if (bits_available_for_prod < 1) /* this would be insane - how many CPUs? */
+ return -ENOMEM;
+
+ q->llq.max_n_shift = min_t(int, q->llq.max_n_shift, bits_available_for_prod);
do {
qsz = ((1 << q->llq.max_n_shift) * dwords) << 3;
q->base = dmam_alloc_coherent(smmu->dev, qsz, &q->base_dma,
@@ -3167,6 +3188,17 @@ static int arm_smmu_init_one_queue(struct arm_smmu_device *smmu,
q->llq.max_n_shift--;
} while (1);
+ entries_for_prod = 1 << q->llq.max_n_shift;
+
+ /*
+ * We need at least 2 commands in a batch (1 x CMD_SYNC and 1 x whatever else).
+ * Assuming orig max_n_shift >= 17, this would mean ~16K CPUs max.
+ */
+ if (entries_for_prod < 2 * cpus)
+ return -ENOMEM;
+
+ q->llq.max_cmd_per_batch = min_t(u32, entries_for_prod / cpus, CMDQ_BATCH_ENTRIES);
+ q->llq.owner_count_shift = q->llq.max_n_shift + 1;
if (!q->base) {
dev_err(smmu->dev,
--
2.16.4
_______________________________________________
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 RFC 0/2] iommu/arm-smmu-v3: Improve cmdq lock efficiency
From: John Garry @ 2020-06-01 11:50 UTC (permalink / raw)
To: will, robin.murphy
Cc: song.bao.hua, maz, joro, John Garry, iommu, linux-arm-kernel
As mentioned in [0], the CPU may consume many cycles processing
arm_smmu_cmdq_issue_cmdlist(). One issue we find is the cmpxchg() loop to
get space on the queue takes approx 25% of the cycles for this function.
The cmpxchg() is removed as follows:
- We assume that the cmdq can never fill with changes to limit the
batch size (where necessary) and always issue a CMD_SYNC for a batch
We need to do this since we no longer maintain the cons value in
software, and we cannot deal with no available space properly.
- Replace cmpxchg() with atomic inc operation, to maintain the prod
and owner values.
Early experiments have shown that we may see a 25% boost in throughput
IOPS for my NVMe test with these changes. And some CPUs, which were
loaded at ~55%, now see a ~45% load.
So, even though the changes are incomplete and other parts of the driver
will need fixing up (and it looks maybe broken for !MSI support), the
performance boost seen would seem to be worth the effort of exploring
this.
Comments requested please.
Thanks
[0] https://lore.kernel.org/linux-iommu/B926444035E5E2439431908E3842AFD24B86DB@DGGEMI525-MBS.china.huawei.com/T/#ma02e301c38c3e94b7725e685757c27e39c7cbde3
John Garry (2):
iommu/arm-smmu-v3: Calculate bits for prod and owner
iommu/arm-smmu-v3: Remove cmpxchg() in arm_smmu_cmdq_issue_cmdlist()
drivers/iommu/arm-smmu-v3.c | 92 +++++++++++++++++++++++----------------------
1 file changed, 47 insertions(+), 45 deletions(-)
--
2.16.4
_______________________________________________
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 v4 11/11] thermal: Rename set_mode() to change_mode()
From: Peter Kästle @ 2020-06-01 11:38 UTC (permalink / raw)
To: Andrzej Pietrasiewicz, linux-pm, linux-acpi, netdev,
linux-wireless, platform-driver-x86, linux-arm-kernel,
linux-renesas-soc, linux-rockchip
Cc: Emmanuel Grumbach, Heiko Stuebner, Vishal Kulkarni, Luca Coelho,
Miquel Raynal, kernel, Fabio Estevam, Amit Kucheria,
Chunyan Zhang, Daniel Lezcano, Allison Randal, NXP Linux Team,
Darren Hart, Zhang Rui, Gayatri Kammela, Len Brown, Johannes Berg,
Intel Linux Wireless, Sascha Hauer, Ido Schimmel, Baolin Wang,
Jiri Pirko, Orson Zhai, Thomas Gleixner, Kalle Valo,
Support Opensource, Enrico Weigelt, Rafael J . Wysocki,
Sebastian Reichel, Bartlomiej Zolnierkiewicz,
Pengutronix Kernel Team, Niklas Söderlund, Shawn Guo,
David S . Miller, Andy Shevchenko
In-Reply-To: <20200528192051.28034-12-andrzej.p@collabora.com>
28. Mai 2020 21:22, "Andrzej Pietrasiewicz" <andrzej.p@collabora.com> schrieb:
> set_mode() is only called when tzd's mode is about to change. Actual
> setting is performed in thermal_core, in thermal_zone_device_set_mode().
> The meaning of set_mode() callback is actually to notify the driver about
> the mode being changed and giving the driver a chance to oppose such
> change.
>
> To better reflect the purpose of the method rename it to change_mode()
>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> ---
> drivers/platform/x86/acerhdf.c | 6 +++---
Acked-by: Peter Kaestle <peter@piie.net>
_______________________________________________
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 v4 10/11] thermal: Simplify or eliminate unnecessary set_mode() methods
From: Peter Kästle @ 2020-06-01 11:38 UTC (permalink / raw)
To: Andrzej Pietrasiewicz, linux-pm, linux-acpi, netdev,
linux-wireless, platform-driver-x86, linux-arm-kernel,
linux-renesas-soc, linux-rockchip
Cc: Emmanuel Grumbach, Heiko Stuebner, Vishal Kulkarni, Luca Coelho,
Miquel Raynal, kernel, Fabio Estevam, Amit Kucheria,
Chunyan Zhang, Daniel Lezcano, Allison Randal, NXP Linux Team,
Darren Hart, Zhang Rui, Gayatri Kammela, Len Brown, Johannes Berg,
Intel Linux Wireless, Sascha Hauer, Ido Schimmel, Baolin Wang,
Jiri Pirko, Orson Zhai, Thomas Gleixner, Kalle Valo,
Support Opensource, Enrico Weigelt, Rafael J . Wysocki,
Sebastian Reichel, Bartlomiej Zolnierkiewicz,
Pengutronix Kernel Team, Niklas Söderlund, Shawn Guo,
David S . Miller, Andy Shevchenko
In-Reply-To: <20200528192051.28034-11-andrzej.p@collabora.com>
28. Mai 2020 21:22, "Andrzej Pietrasiewicz" <andrzej.p@collabora.com> schrieb:
> Setting polling_delay is now done at thermal_core level (by not polling
> DISABLED devices), so no need to repeat this code.
>
> int340x: Checking for an impossible enum value is unnecessary.
> acpi/thermal: It only prints debug messages.
>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> ---
[...]
> drivers/platform/x86/acerhdf.c | 3 --
Acked-by: Peter Kaestle <peter@piie.net>
_______________________________________________
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 v4 07/11] thermal: Use mode helpers in drivers
From: Peter Kästle @ 2020-06-01 11:37 UTC (permalink / raw)
To: Andrzej Pietrasiewicz, linux-pm, linux-acpi, netdev,
linux-wireless, platform-driver-x86, linux-arm-kernel,
linux-renesas-soc, linux-rockchip
Cc: Emmanuel Grumbach, Heiko Stuebner, Vishal Kulkarni, Luca Coelho,
Miquel Raynal, kernel, Fabio Estevam, Amit Kucheria,
Chunyan Zhang, Daniel Lezcano, Allison Randal, NXP Linux Team,
Darren Hart, Zhang Rui, Gayatri Kammela, Len Brown, Johannes Berg,
Intel Linux Wireless, Sascha Hauer, Ido Schimmel, Baolin Wang,
Jiri Pirko, Orson Zhai, Thomas Gleixner, Kalle Valo,
Support Opensource, Enrico Weigelt, Rafael J . Wysocki,
Sebastian Reichel, Bartlomiej Zolnierkiewicz,
Pengutronix Kernel Team, Niklas Söderlund, Shawn Guo,
David S . Miller, Andy Shevchenko
In-Reply-To: <20200528192051.28034-8-andrzej.p@collabora.com>
28. Mai 2020 21:22, "Andrzej Pietrasiewicz" <andrzej.p@collabora.com> schrieb:
> Use thermal_zone_device_{en|dis}able() and thermal_zone_device_is_enabled().
>
> Consequently, all set_mode() implementations in drivers:
>
> - can stop modifying tzd's "mode" member,
> - shall stop taking tzd's lock, as it is taken in the helpers
> - shall stop calling thermal_zone_device_update() as it is called in the
> helpers
> - can assume they are called when the mode truly changes, so checks to
> verify that can be dropped
>
> Not providing set_mode() by a driver no longer prevents the core from
> being able to set tzd's mode, so the relevant check in mode_store() is
> removed.
>
> Other comments:
>
> - acpi/thermal.c: tz->thermal_zone->mode will be updated only after we
> return from set_mode(), so use function parameter in thermal_set_mode()
> instead, no need to call acpi_thermal_check() in set_mode()
> - thermal/imx_thermal.c: regmap writes and mode assignment are done in
> thermal_zone_device_{en|dis}able() and set_mode() callback
> - thermal/intel/intel_quark_dts_thermal.c: soc_dts_{en|dis}able() are a
> part of set_mode() callback, so they don't need to modify tzd->mode, and
> don't need to fall back to the opposite mode if unsuccessful, as the return
> value will be propagated to thermal_zone_device_{en|dis}able() and
> ultimately tzd's member will not be changed in thermal_zone_device_set_mode().
> - thermal/of-thermal.c: no need to set zone->mode to DISABLED in
> of_parse_thermal_zones() as a tzd is kzalloc'ed so mode is DISABLED anyway
>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> ---
[...]
> drivers/platform/x86/acerhdf.c | 17 +++++----
Acked-by: Peter Kaestle <peter@piie.net>
_______________________________________________
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 v4 05/11] thermal: remove get_mode() operation of drivers
From: Peter Kästle @ 2020-06-01 11:37 UTC (permalink / raw)
To: Andrzej Pietrasiewicz, linux-pm, linux-acpi, netdev,
linux-wireless, platform-driver-x86, linux-arm-kernel,
linux-renesas-soc, linux-rockchip
Cc: Emmanuel Grumbach, Heiko Stuebner, Vishal Kulkarni, Luca Coelho,
Miquel Raynal, kernel, Fabio Estevam, Amit Kucheria,
Chunyan Zhang, Daniel Lezcano, Allison Randal, NXP Linux Team,
Darren Hart, Zhang Rui, Gayatri Kammela, Len Brown, Johannes Berg,
Intel Linux Wireless, Sascha Hauer, Ido Schimmel, Baolin Wang,
Jiri Pirko, Orson Zhai, Thomas Gleixner, Kalle Valo,
Support Opensource, Enrico Weigelt, Rafael J . Wysocki,
Sebastian Reichel, Bartlomiej Zolnierkiewicz,
Pengutronix Kernel Team, Niklas Söderlund, Shawn Guo,
David S . Miller, Andy Shevchenko
In-Reply-To: <20200528192051.28034-6-andrzej.p@collabora.com>
28. Mai 2020 21:21, "Andrzej Pietrasiewicz" <andrzej.p@collabora.com> schrieb:
> get_mode() is now redundant, as the state is stored in struct
> thermal_zone_device.
>
> Consequently the "mode" attribute in sysfs can always be visible, because
> it is always possible to get the mode from struct tzd.
>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> ---
[...]
> drivers/platform/x86/acerhdf.c | 12 --------
Acked-by: Peter Kaestle <peter@piie.net>
_______________________________________________
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 04/12] dt-bindings: irqchip: ti, sci-intr: Update bindings to drop the usage of gic as parent
From: Lokesh Vutla @ 2020-06-01 11:36 UTC (permalink / raw)
To: Marc Zyngier
Cc: Nishanth Menon, Rob Herring, Grygorii Strashko,
Device Tree Mailing List, Tero Kristo, Sekhar Nori,
Peter Ujfalusi, Santosh Shilimkar, Thomas Gleixner,
Linux ARM Mailing List
In-Reply-To: <4ea8c6110a16900220a65f1d44145146@kernel.org>
Hi Marc,
On 29/05/20 3:48 pm, Marc Zyngier wrote:
> On 2020-05-29 11:14, Lokesh Vutla wrote:
>> Hi Rob,
>>
>> On 29/05/20 3:44 am, Rob Herring wrote:
>>> On Wed, May 20, 2020 at 06:14:46PM +0530, Lokesh Vutla wrote:
>>>> Drop the firmware related dt-bindings and use the hardware specified
>>>> interrupt numbers within Interrupt Router. This ensures interrupt router
>>>> DT node need not assume any interrupt parent type.
>>>
>>> I didn't like this binding to begin with, but now you're breaking
>>> compatibility.
>>
>> Yes, I do agree that this change is breaking backward compatibility. But IMHO,
>> this does cleanup of firmware specific properties from DT. Since this is not
>> deployed out yet in the wild market, I took the leverage of breaking backward
>> compatibility. Before accepting these changes from firmware team, I did
>> discuss[0] with Marc on this topic.
>
> And I assume that should anyone complain about the kernel being broken
> because they have an old firmware, you'll be OK with the patches being
> reverted, right?
I am assuming there is no one to complain as there is no product available yet
with upstream kernel. Internally everyone is aware of the changes. So, yes I
would agree with you to revert the changes if someone really needs it. :)
Thanks and regards,
Lokesh
_______________________________________________
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 v4 04/11] thermal: Store device mode in struct thermal_zone_device
From: Peter Kästle @ 2020-06-01 11:36 UTC (permalink / raw)
To: Andrzej Pietrasiewicz, linux-pm, linux-acpi, netdev,
linux-wireless, platform-driver-x86, linux-arm-kernel,
linux-renesas-soc, linux-rockchip
Cc: Emmanuel Grumbach, Heiko Stuebner, Vishal Kulkarni, Luca Coelho,
Miquel Raynal, kernel, Fabio Estevam, Amit Kucheria,
Chunyan Zhang, Daniel Lezcano, Allison Randal, NXP Linux Team,
Darren Hart, Zhang Rui, Gayatri Kammela, Len Brown, Johannes Berg,
Intel Linux Wireless, Sascha Hauer, Ido Schimmel, Baolin Wang,
Jiri Pirko, Orson Zhai, Thomas Gleixner, Kalle Valo,
Support Opensource, Enrico Weigelt, Rafael J . Wysocki,
Sebastian Reichel, Bartlomiej Zolnierkiewicz,
Pengutronix Kernel Team, Niklas Söderlund, Shawn Guo,
David S . Miller, Andy Shevchenko
In-Reply-To: <20200528192051.28034-5-andrzej.p@collabora.com>
28. Mai 2020 21:21, "Andrzej Pietrasiewicz" <andrzej.p@collabora.com> schrieb:
> Prepare for eliminating get_mode().
>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> ---
[...]
> drivers/platform/x86/acerhdf.c | 15 ++++++-------
Acked-by: Peter Kaestle <peter@piie.net>
_______________________________________________
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 v4 02/11] thermal: Store thermal mode in a dedicated enum
From: Peter Kästle @ 2020-06-01 11:36 UTC (permalink / raw)
To: Andrzej Pietrasiewicz, linux-pm, linux-acpi, netdev,
linux-wireless, platform-driver-x86, linux-arm-kernel,
linux-renesas-soc, linux-rockchip
Cc: Emmanuel Grumbach, Heiko Stuebner, Vishal Kulkarni, Luca Coelho,
Miquel Raynal, kernel, Fabio Estevam, Amit Kucheria,
Chunyan Zhang, Daniel Lezcano, Allison Randal, NXP Linux Team,
Darren Hart, Zhang Rui, Gayatri Kammela, Len Brown, Johannes Berg,
Intel Linux Wireless, Sascha Hauer, Ido Schimmel, Baolin Wang,
Jiri Pirko, Orson Zhai, Thomas Gleixner, Kalle Valo,
Support Opensource, Enrico Weigelt, Rafael J . Wysocki,
Sebastian Reichel, Bartlomiej Zolnierkiewicz,
Pengutronix Kernel Team, Niklas Söderlund, Shawn Guo,
David S . Miller, Andy Shevchenko
In-Reply-To: <20200528192051.28034-3-andrzej.p@collabora.com>
28. Mai 2020 21:21, "Andrzej Pietrasiewicz" <andrzej.p@collabora.com> schrieb:
> Prepare for storing mode in struct thermal_zone_device.
>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> ---
[...]
> drivers/platform/x86/acerhdf.c | 8 ++++--
Acked-by: Peter Kaestle <peter@piie.net>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] iommu/arm-smmu-v3: allocate the memory of queues in local numa node
From: Barry Song @ 2020-06-01 11:31 UTC (permalink / raw)
To: hch, m.szyprowski, robin.murphy, will
Cc: Barry Song, iommu, linuxarm, linux-arm-kernel
dmam_alloc_coherent() will usually allocate memory from the default CMA. For
a common arm64 defconfig without reserved memory in device tree, there is only
one CMA close to address 0.
dma_alloc_contiguous() will allocate memory without any idea of NUMA and smmu
has no customized per-numa cma_area.
struct page *dma_alloc_contiguous(struct device *dev, size_t size, gfp_t gfp)
{
size_t count = size >> PAGE_SHIFT;
struct page *page = NULL;
struct cma *cma = NULL;
if (dev && dev->cma_area)
cma = dev->cma_area;
else if (count > 1)
cma = dma_contiguous_default_area;
...
return page;
}
if there are N numa nodes, N-1 nodes will put command/evt queues etc in a
remote node the default CMA belongs to,probably node 0. Tests show, after
sending CMD_SYNC in an empty command queue,
on Node2, without this patch, it takes 550ns to wait for the completion
of CMD_SYNC; with this patch, it takes 250ns to wait for the completion
of CMD_SYNC.
Signed-off-by: Barry Song <song.bao.hua@hisilicon.com>
---
drivers/iommu/arm-smmu-v3.c | 63 ++++++++++++++++++++++++++++---------
1 file changed, 48 insertions(+), 15 deletions(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 82508730feb7..58295423e1d7 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -3157,21 +3157,23 @@ static int arm_smmu_init_one_queue(struct arm_smmu_device *smmu,
size_t dwords, const char *name)
{
size_t qsz;
+ struct page *page;
- do {
- qsz = ((1 << q->llq.max_n_shift) * dwords) << 3;
- q->base = dmam_alloc_coherent(smmu->dev, qsz, &q->base_dma,
- GFP_KERNEL);
- if (q->base || qsz < PAGE_SIZE)
- break;
-
- q->llq.max_n_shift--;
- } while (1);
+ qsz = ((1 << q->llq.max_n_shift) * dwords) << 3;
+ page = alloc_pages_node(dev_to_node(smmu->dev), GFP_KERNEL,
+ get_order(qsz));
+ if (!page) {
+ dev_err(smmu->dev,
+ "failed to allocate queue (0x%zx bytes) for %s\n",
+ qsz, name);
+ return -ENOMEM;
+ }
- if (!q->base) {
+ q->base = page_address(page);
+ q->base_dma = dma_map_single(smmu->dev, q->base, qsz, DMA_BIDIRECTIONAL);
+ if (dma_mapping_error(smmu->dev, q->base_dma)) {
dev_err(smmu->dev,
- "failed to allocate queue (0x%zx bytes) for %s\n",
- qsz, name);
+ "failed to dma map for %s\n", name);
return -ENOMEM;
}
@@ -3192,6 +3194,18 @@ static int arm_smmu_init_one_queue(struct arm_smmu_device *smmu,
return 0;
}
+static int arm_smmu_deinit_one_queue(struct arm_smmu_device *smmu,
+ struct arm_smmu_queue *q,
+ size_t dwords)
+{
+ size_t qsz = ((1 << q->llq.max_n_shift) * dwords) << 3;
+
+ dma_unmap_single(smmu->dev, q->base_dma, qsz, DMA_BIDIRECTIONAL);
+ free_page((unsigned long)q->base);
+
+ return 0;
+}
+
static void arm_smmu_cmdq_free_bitmap(void *data)
{
unsigned long *bitmap = data;
@@ -3233,22 +3247,40 @@ static int arm_smmu_init_queues(struct arm_smmu_device *smmu)
ret = arm_smmu_cmdq_init(smmu);
if (ret)
- return ret;
+ goto deinit_cmdq;
/* evtq */
ret = arm_smmu_init_one_queue(smmu, &smmu->evtq.q, ARM_SMMU_EVTQ_PROD,
ARM_SMMU_EVTQ_CONS, EVTQ_ENT_DWORDS,
"evtq");
if (ret)
- return ret;
+ goto deinit_cmdq;
/* priq */
if (!(smmu->features & ARM_SMMU_FEAT_PRI))
return 0;
- return arm_smmu_init_one_queue(smmu, &smmu->priq.q, ARM_SMMU_PRIQ_PROD,
+ ret = arm_smmu_init_one_queue(smmu, &smmu->priq.q, ARM_SMMU_PRIQ_PROD,
ARM_SMMU_PRIQ_CONS, PRIQ_ENT_DWORDS,
"priq");
+ if (ret)
+ goto deinit_evtq;
+
+ return 0;
+
+deinit_evtq:
+ arm_smmu_deinit_one_queue(smmu, &smmu->evtq.q, EVTQ_ENT_DWORDS);
+deinit_cmdq:
+ arm_smmu_deinit_one_queue(smmu, &smmu->cmdq.q, CMDQ_ENT_DWORDS);
+ return ret;
+}
+
+static void arm_smmu_deinit_queues(struct arm_smmu_device *smmu)
+{
+ arm_smmu_deinit_one_queue(smmu, &smmu->cmdq.q, CMDQ_ENT_DWORDS);
+ arm_smmu_deinit_one_queue(smmu, &smmu->evtq.q, EVTQ_ENT_DWORDS);
+ if (smmu->features & ARM_SMMU_FEAT_PRI)
+ arm_smmu_deinit_one_queue(smmu, &smmu->priq.q, PRIQ_ENT_DWORDS);
}
static int arm_smmu_init_l1_strtab(struct arm_smmu_device *smmu)
@@ -4121,6 +4153,7 @@ static int arm_smmu_device_remove(struct platform_device *pdev)
arm_smmu_set_bus_ops(NULL);
iommu_device_unregister(&smmu->iommu);
iommu_device_sysfs_remove(&smmu->iommu);
+ arm_smmu_deinit_queues(smmu);
arm_smmu_device_disable(smmu);
return 0;
--
2.23.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 v15 04/11] pwm: clps711x: Use 64-bit division macro
From: Daniel Thompson @ 2020-06-01 11:33 UTC (permalink / raw)
To: Guru Das Srinagesh
Cc: linux-arm-kernel, linux-pwm, Arnd Bergmann, David Collins,
Stephen Boyd, linux-kernel, Thierry Reding, Geert Uytterhoeven,
Dan Carpenter, Uwe Kleine-König, Joe Perches,
Subbaraman Narayanamurthy, Lee Jones, Guenter Roeck
In-Reply-To: <20200528203341.GA8065@codeaurora.org>
On Thu, May 28, 2020 at 01:33:41PM -0700, Guru Das Srinagesh wrote:
> On Tue, May 26, 2020 at 10:35:04AM -0700, Guru Das Srinagesh wrote:
> > Since the PWM framework is switching struct pwm_args.period's datatype
> > to u64, prepare for this transition by using DIV64_U64_ROUND_CLOSEST to
> > handle a 64-bit divisor.
> >
> > Cc: Daniel Thompson <daniel.thompson@linaro.org>
> > Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
> > ---
> > drivers/pwm/pwm-clps711x.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/pwm/pwm-clps711x.c b/drivers/pwm/pwm-clps711x.c
> > index 924d39a..ba9500a 100644
> > --- a/drivers/pwm/pwm-clps711x.c
> > +++ b/drivers/pwm/pwm-clps711x.c
> > @@ -43,7 +43,7 @@ static void clps711x_pwm_update_val(struct clps711x_chip *priv, u32 n, u32 v)
> > static unsigned int clps711x_get_duty(struct pwm_device *pwm, unsigned int v)
> > {
> > /* Duty cycle 0..15 max */
> > - return DIV_ROUND_CLOSEST(v * 0xf, pwm->args.period);
> > + return DIV64_U64_ROUND_CLOSEST(v * 0xf, pwm->args.period);
> > }
> >
> > static int clps711x_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
> > --
>
> Hi Daniel,
>
> Could you please review this patch when you get a chance to?
I don't normally review PWM patches... but this no longer has the bug
there was there when I first read this patch.
Daniel.
_______________________________________________
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] pinctrl: pxa: pxa2xx: Remove 'pxa2xx_pinctrl_exit()' which is unused and broken
From: Christophe JAILLET @ 2020-06-01 11:31 UTC (permalink / raw)
To: Robert Jarzmik
Cc: linus.walleij, kernel-janitors, linux-kernel, haojian.zhuang,
linux-gpio, daniel, linux-arm-kernel
In-Reply-To: <87h7vvb1s3.fsf@belgarion.home>
Le 01/06/2020 à 10:58, Robert Jarzmik a écrit :
> Christophe JAILLET <christophe.jaillet@wanadoo.fr> writes:
>
>> Commit 6d33ee7a0534 ("pinctrl: pxa: Use devm_pinctrl_register() for pinctrl registration")
>> has turned a 'pinctrl_register()' into 'devm_pinctrl_register()' in
>> 'pxa2xx_pinctrl_init()'.
>> However, the corresponding 'pinctrl_unregister()' call in
>> 'pxa2xx_pinctrl_exit()' has not been removed.
>>
>> This is not an issue, because 'pxa2xx_pinctrl_exit()' is unused.
>> Remove it now to avoid some wondering in the future and save a few LoC.
>>
>> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>
>
> Would be even a better patch with a :
> Fixes: 6d33ee7a0534 ("pinctrl: pxa: Use devm_pinctrl_register() for pinctrl registration")
I was wondering it was was needed in this case.
The patch does not really fix anything, as the function is unused. Or it
fixes things on a theoretical point of view.
CJ
> Cheers.
>
> --
> Robert
>
_______________________________________________
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 v4 06/11] thermal: Add mode helpers
From: Andrzej Pietrasiewicz @ 2020-06-01 11:16 UTC (permalink / raw)
To: Guenter Roeck
Cc: Emmanuel Grumbach, Heiko Stuebner, Kalle Valo, linux-wireless,
Peter Kaestle, platform-driver-x86, Vishal Kulkarni, Luca Coelho,
Miquel Raynal, Shawn Guo, kernel, Fabio Estevam, Amit Kucheria,
linux-rockchip, Chunyan Zhang, Daniel Lezcano, linux-acpi,
linux-arm-kernel, Darren Hart, Zhang Rui, Gayatri Kammela,
NXP Linux Team, Johannes Berg, linux-pm, Sascha Hauer,
Intel Linux Wireless, Ido Schimmel, Niklas Söderlund,
Jiri Pirko, Orson Zhai, Thomas Gleixner, Allison Randal,
Support Opensource, netdev, Rafael J . Wysocki, Sebastian Reichel,
linux-renesas-soc, Bartlomiej Zolnierkiewicz,
Pengutronix Kernel Team, Baolin Wang, Len Brown, Enrico Weigelt,
David S . Miller, Andy Shevchenko
In-Reply-To: <20200529155206.GA158553@roeck-us.net>
Hi Guenter,
W dniu 29.05.2020 o 17:52, Guenter Roeck pisze:
> On Thu, May 28, 2020 at 09:20:46PM +0200, Andrzej Pietrasiewicz wrote:
>> Prepare for making the drivers not access tzd's private members.
>>
>> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
<snip>
>> +
> Nit: unnecessary empty line.
>
>> + return ret;
<snip>
>> + return thermal_zone_device_set_mode(tz, THERMAL_DEVICE_ENABLED);
>> +}
>> +EXPORT_SYMBOL(thermal_zone_device_enable);
>
> Other exports in thermal/ use EXPORT_SYMBOL_GPL.
Other than that does it look good to you?
I can send a v5 where the two above will be corrected, but did you have
a chance to review patches 7-11?
Andrzej
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v3 2/5] scsi: ufs-mediatek: Do not gate clocks if auto-hibern8 is not entered yet
From: Stanley Chu @ 2020-06-01 10:46 UTC (permalink / raw)
To: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb
Cc: Stanley Chu, bvanassche, andy.teng, cc.chou, chun-hung.wu,
kuohong.wang, linux-kernel, cang, linux-mediatek, peter.wang,
matthias.bgg, beanhuo, chaotian.jing, linux-arm-kernel, asutoshd
In-Reply-To: <20200601104646.15436-1-stanley.chu@mediatek.com>
There are some chances that link enters hibern8 lately by auto-hibern8
scheme during the clock-gating flow. Clocks shall not be gated if link
is still active otherwise host or device may hang.
Fix this by returning error code to the caller __ufshcd_setup_clocks()
to skip gating clocks there if link is not confirmed in hibern8
state yet.
Also allow some waiting time for the hibern8 state transition.
Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
Reviewed-by: Andy Teng <andy.teng@mediatek.com>
---
drivers/scsi/ufs/ufs-mediatek.c | 36 ++++++++++++++++++++++++---------
1 file changed, 27 insertions(+), 9 deletions(-)
diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c
index 523ee5573921..3c85f5e97dea 100644
--- a/drivers/scsi/ufs/ufs-mediatek.c
+++ b/drivers/scsi/ufs/ufs-mediatek.c
@@ -178,15 +178,30 @@ static void ufs_mtk_setup_ref_clk_wait_us(struct ufs_hba *hba,
host->ref_clk_ungating_wait_us = ungating_us;
}
-static u32 ufs_mtk_link_get_state(struct ufs_hba *hba)
+int ufs_mtk_wait_link_state(struct ufs_hba *hba, u32 state,
+ unsigned long max_wait_ms)
{
+ ktime_t timeout, time_checked;
u32 val;
- ufshcd_writel(hba, 0x20, REG_UFS_DEBUG_SEL);
- val = ufshcd_readl(hba, REG_UFS_PROBE);
- val = val >> 28;
+ timeout = ktime_add_us(ktime_get(), ms_to_ktime(max_wait_ms));
+ do {
+ time_checked = ktime_get();
+ ufshcd_writel(hba, 0x20, REG_UFS_DEBUG_SEL);
+ val = ufshcd_readl(hba, REG_UFS_PROBE);
+ val = val >> 28;
+
+ if (val == state)
+ return 0;
- return val;
+ /* Sleep for max. 200us */
+ usleep_range(100, 200);
+ } while (ktime_before(time_checked, timeout));
+
+ if (val == state)
+ return 0;
+
+ return -ETIMEDOUT;
}
/**
@@ -221,10 +236,13 @@ static int ufs_mtk_setup_clocks(struct ufs_hba *hba, bool on,
* triggered by Auto-Hibern8.
*/
if (!ufshcd_can_hibern8_during_gating(hba) &&
- ufshcd_is_auto_hibern8_enabled(hba) &&
- ufs_mtk_link_get_state(hba) ==
- VS_LINK_HIBERN8)
- ufs_mtk_setup_ref_clk(hba, on);
+ ufshcd_is_auto_hibern8_enabled(hba)) {
+ ret = ufs_mtk_wait_link_state(hba,
+ VS_LINK_HIBERN8,
+ 15);
+ if (!ret)
+ ufs_mtk_setup_ref_clk(hba, on);
+ }
}
} else if (on && status == POST_CHANGE) {
ret = phy_power_on(host->mphy);
--
2.18.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 v3 4/5] scsi: ufs-mediatek: Fix unbalanced clock on/off
From: Stanley Chu @ 2020-06-01 10:46 UTC (permalink / raw)
To: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb
Cc: Stanley Chu, bvanassche, andy.teng, cc.chou, chun-hung.wu,
kuohong.wang, linux-kernel, cang, linux-mediatek, peter.wang,
matthias.bgg, beanhuo, chaotian.jing, linux-arm-kernel, asutoshd
In-Reply-To: <20200601104646.15436-1-stanley.chu@mediatek.com>
MediaTek UFS clocks are separated to two parts and controlled
by different modules: ufs-mediatek and phy-ufs-mediatek.
If both Auto-Hibern8 and clk-gating feature are enabled, mphy
power control is not balanced thus unbalanced control also
happens to the clocks probed by phy-ufs-mediatek module.
Fix this issue by
- Promise usage of phy_power_on/off balanced
- Remove phy_power_on/off control in suspend/resume vops since
both can be handled in setup_clock vops only
Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
Reviewed-by: Peter Wang <peter.wang@mediatek.com>
---
drivers/scsi/ufs/ufs-mediatek.c | 60 ++++++++++++++++++++-------------
drivers/scsi/ufs/ufs-mediatek.h | 1 +
2 files changed, 38 insertions(+), 23 deletions(-)
diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c
index 5f41b7b7db8f..1cc7bea1468b 100644
--- a/drivers/scsi/ufs/ufs-mediatek.c
+++ b/drivers/scsi/ufs/ufs-mediatek.c
@@ -205,6 +205,23 @@ int ufs_mtk_wait_link_state(struct ufs_hba *hba, u32 state,
return -ETIMEDOUT;
}
+static void ufs_mtk_mphy_power_on(struct ufs_hba *hba, bool on)
+{
+ struct ufs_mtk_host *host = ufshcd_get_variant(hba);
+ struct phy *mphy = host->mphy;
+
+ if (!mphy)
+ return;
+
+ if (on && !host->mphy_powered_on)
+ phy_power_on(mphy);
+ else if (!on && host->mphy_powered_on)
+ phy_power_off(mphy);
+ else
+ return;
+ host->mphy_powered_on = on;
+}
+
/**
* ufs_mtk_setup_clocks - enables/disable clocks
* @hba: host controller instance
@@ -218,6 +235,7 @@ static int ufs_mtk_setup_clocks(struct ufs_hba *hba, bool on,
{
struct ufs_mtk_host *host = ufshcd_get_variant(hba);
int ret = 0;
+ bool clk_pwr_off = false;
/*
* In case ufs_mtk_init() is not yet done, simply ignore.
@@ -228,25 +246,29 @@ static int ufs_mtk_setup_clocks(struct ufs_hba *hba, bool on,
return 0;
if (!on && status == PRE_CHANGE) {
- if (!ufshcd_is_link_active(hba)) {
- ufs_mtk_setup_ref_clk(hba, on);
- ret = phy_power_off(host->mphy);
- } else {
+ if (ufshcd_is_link_off(hba)) {
+ clk_pwr_off = true;
+ } else if (ufshcd_is_link_hibern8(hba) ||
+ (!ufshcd_can_hibern8_during_gating(hba) &&
+ ufshcd_is_auto_hibern8_enabled(hba))) {
/*
- * Gate ref-clk if link state is in Hibern8
- * triggered by Auto-Hibern8.
+ * Gate ref-clk and poweroff mphy if link state is in
+ * OFF or Hibern8 by either Auto-Hibern8 or
+ * ufshcd_link_state_transition().
*/
- if (!ufshcd_can_hibern8_during_gating(hba) &&
- ufshcd_is_auto_hibern8_enabled(hba)) {
- ret = ufs_mtk_wait_link_state(hba,
- VS_LINK_HIBERN8,
- 15);
- if (!ret)
- ufs_mtk_setup_ref_clk(hba, on);
- }
+ ret = ufs_mtk_wait_link_state(hba,
+ VS_LINK_HIBERN8,
+ 15);
+ if (!ret)
+ clk_pwr_off = true;
+ }
+
+ if (clk_pwr_off) {
+ ufs_mtk_setup_ref_clk(hba, on);
+ ufs_mtk_mphy_power_on(hba, on);
}
} else if (on && status == POST_CHANGE) {
- ret = phy_power_on(host->mphy);
+ ufs_mtk_mphy_power_on(hba, on);
ufs_mtk_setup_ref_clk(hba, on);
}
@@ -538,7 +560,6 @@ static void ufs_mtk_vreg_set_lpm(struct ufs_hba *hba, bool lpm)
static int ufs_mtk_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
{
int err;
- struct ufs_mtk_host *host = ufshcd_get_variant(hba);
if (ufshcd_is_link_hibern8(hba)) {
err = ufs_mtk_link_set_lpm(hba);
@@ -559,20 +580,13 @@ static int ufs_mtk_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
ufs_mtk_vreg_set_lpm(hba, true);
}
- if (!ufshcd_is_link_active(hba))
- phy_power_off(host->mphy);
-
return 0;
}
static int ufs_mtk_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
{
- struct ufs_mtk_host *host = ufshcd_get_variant(hba);
int err;
- if (!ufshcd_is_link_active(hba))
- phy_power_on(host->mphy);
-
if (ufshcd_is_link_hibern8(hba)) {
ufs_mtk_vreg_set_lpm(hba, false);
err = ufs_mtk_link_set_hpm(hba);
diff --git a/drivers/scsi/ufs/ufs-mediatek.h b/drivers/scsi/ufs/ufs-mediatek.h
index fc42dcbfd800..6052ec105aba 100644
--- a/drivers/scsi/ufs/ufs-mediatek.h
+++ b/drivers/scsi/ufs/ufs-mediatek.h
@@ -91,6 +91,7 @@ enum {
struct ufs_mtk_host {
struct ufs_hba *hba;
struct phy *mphy;
+ bool mphy_powered_on;
bool unipro_lpm;
bool ref_clk_enabled;
u16 ref_clk_ungating_wait_us;
--
2.18.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 v3 0/5] scsi: ufs-mediatek: Fix clk-gating and introduce low-power mode for vccq2
From: Stanley Chu @ 2020-06-01 10:46 UTC (permalink / raw)
To: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb
Cc: Stanley Chu, bvanassche, andy.teng, cc.chou, chun-hung.wu,
kuohong.wang, linux-kernel, cang, linux-mediatek, peter.wang,
matthias.bgg, beanhuo, chaotian.jing, linux-arm-kernel, asutoshd
Hi,
This series fixes clk-gating issues and introduces low-power mode for vccq2 in MediaTek platforms.
v2 -> v3:
- Fix (add back) linkoff support in patch [4] since previous version incorrectly removed linkoff support
v1 -> v2:
- Add patch [4] and [5]
Stanley Chu (5):
scsi: ufs-mediatek: Fix imprecise waiting time for ref-clk control
scsi: ufs-mediatek: Do not gate clocks if auto-hibern8 is not entered
yet
scsi: ufs-mediatek: Introduce low-power mode for device power supply
scsi: ufs-mediatek: Fix unbalanced clock on/off
scsi: ufs-mediatek: Allow unbound mphy
drivers/scsi/ufs/ufs-mediatek.c | 116 ++++++++++++++++++++++++--------
drivers/scsi/ufs/ufs-mediatek.h | 3 +-
2 files changed, 90 insertions(+), 29 deletions(-)
--
2.18.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 v3 1/5] scsi: ufs-mediatek: Fix imprecise waiting time for ref-clk control
From: Stanley Chu @ 2020-06-01 10:46 UTC (permalink / raw)
To: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb
Cc: Stanley Chu, bvanassche, andy.teng, cc.chou, chun-hung.wu,
kuohong.wang, linux-kernel, cang, linux-mediatek, peter.wang,
matthias.bgg, beanhuo, chaotian.jing, linux-arm-kernel, asutoshd
In-Reply-To: <20200601104646.15436-1-stanley.chu@mediatek.com>
Currently ref-clk control timeout is implemented by Jiffies. However
jiffies is not accurate enough thus "false timeout" may happen.
Use more accurate delay mechanism instead, for example, ktime.
Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
Reviewed-by: Andy Teng <andy.teng@mediatek.com>
Reviewed-by: Avri Altman <avri.altman@wdc.com>
---
drivers/scsi/ufs/ufs-mediatek.c | 7 ++++---
drivers/scsi/ufs/ufs-mediatek.h | 2 +-
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c
index d56ce8d97d4e..523ee5573921 100644
--- a/drivers/scsi/ufs/ufs-mediatek.c
+++ b/drivers/scsi/ufs/ufs-mediatek.c
@@ -120,7 +120,7 @@ static int ufs_mtk_setup_ref_clk(struct ufs_hba *hba, bool on)
{
struct ufs_mtk_host *host = ufshcd_get_variant(hba);
struct arm_smccc_res res;
- unsigned long timeout;
+ ktime_t timeout, time_checked;
u32 value;
if (host->ref_clk_enabled == on)
@@ -135,8 +135,9 @@ static int ufs_mtk_setup_ref_clk(struct ufs_hba *hba, bool on)
}
/* Wait for ack */
- timeout = jiffies + msecs_to_jiffies(REFCLK_REQ_TIMEOUT_MS);
+ timeout = ktime_add_us(ktime_get(), REFCLK_REQ_TIMEOUT_US);
do {
+ time_checked = ktime_get();
value = ufshcd_readl(hba, REG_UFS_REFCLK_CTRL);
/* Wait until ack bit equals to req bit */
@@ -144,7 +145,7 @@ static int ufs_mtk_setup_ref_clk(struct ufs_hba *hba, bool on)
goto out;
usleep_range(100, 200);
- } while (time_before(jiffies, timeout));
+ } while (ktime_before(time_checked, timeout));
dev_err(hba->dev, "missing ack of refclk req, reg: 0x%x\n", value);
diff --git a/drivers/scsi/ufs/ufs-mediatek.h b/drivers/scsi/ufs/ufs-mediatek.h
index 5bbd3e9cbae2..fc42dcbfd800 100644
--- a/drivers/scsi/ufs/ufs-mediatek.h
+++ b/drivers/scsi/ufs/ufs-mediatek.h
@@ -28,7 +28,7 @@
#define REFCLK_REQUEST BIT(0)
#define REFCLK_ACK BIT(1)
-#define REFCLK_REQ_TIMEOUT_MS 3
+#define REFCLK_REQ_TIMEOUT_US 3000
/*
* Vendor specific pre-defined parameters
--
2.18.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 v3 3/5] scsi: ufs-mediatek: Introduce low-power mode for device power supply
From: Stanley Chu @ 2020-06-01 10:46 UTC (permalink / raw)
To: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb
Cc: Stanley Chu, bvanassche, andy.teng, cc.chou, chun-hung.wu,
kuohong.wang, linux-kernel, cang, linux-mediatek, peter.wang,
matthias.bgg, beanhuo, chaotian.jing, linux-arm-kernel, asutoshd
In-Reply-To: <20200601104646.15436-1-stanley.chu@mediatek.com>
Allow device power supply to enter low-power mode if device will
do nothing to save more power.
Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
Reviewed-by: Pengshun Zhao <pengshun.zhao@mediatek.com>
---
drivers/scsi/ufs/ufs-mediatek.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c
index 3c85f5e97dea..5f41b7b7db8f 100644
--- a/drivers/scsi/ufs/ufs-mediatek.c
+++ b/drivers/scsi/ufs/ufs-mediatek.c
@@ -12,6 +12,7 @@
#include <linux/of_address.h>
#include <linux/phy/phy.h>
#include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
#include <linux/soc/mediatek/mtk_sip_svc.h>
#include "ufshcd.h"
@@ -521,6 +522,19 @@ static int ufs_mtk_link_set_lpm(struct ufs_hba *hba)
return 0;
}
+static void ufs_mtk_vreg_set_lpm(struct ufs_hba *hba, bool lpm)
+{
+ if (!hba->vreg_info.vccq2)
+ return;
+
+ if (lpm & !hba->vreg_info.vcc->enabled)
+ regulator_set_mode(hba->vreg_info.vccq2->reg,
+ REGULATOR_MODE_IDLE);
+ else if (!lpm)
+ regulator_set_mode(hba->vreg_info.vccq2->reg,
+ REGULATOR_MODE_NORMAL);
+}
+
static int ufs_mtk_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
{
int err;
@@ -537,6 +551,12 @@ static int ufs_mtk_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
ufshcd_set_link_off(hba);
return -EAGAIN;
}
+ /*
+ * Make sure no error will be returned to prevent
+ * ufshcd_suspend() re-enabling regulators while vreg is still
+ * in low-power mode.
+ */
+ ufs_mtk_vreg_set_lpm(hba, true);
}
if (!ufshcd_is_link_active(hba))
@@ -554,6 +574,7 @@ static int ufs_mtk_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
phy_power_on(host->mphy);
if (ufshcd_is_link_hibern8(hba)) {
+ ufs_mtk_vreg_set_lpm(hba, false);
err = ufs_mtk_link_set_hpm(hba);
if (err) {
err = ufshcd_link_recovery(hba);
--
2.18.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 v3 5/5] scsi: ufs-mediatek: Allow unbound mphy
From: Stanley Chu @ 2020-06-01 10:46 UTC (permalink / raw)
To: linux-scsi, martin.petersen, avri.altman, alim.akhtar, jejb
Cc: Stanley Chu, bvanassche, andy.teng, cc.chou, chun-hung.wu,
kuohong.wang, linux-kernel, cang, linux-mediatek, peter.wang,
matthias.bgg, beanhuo, chaotian.jing, linux-arm-kernel, asutoshd
In-Reply-To: <20200601104646.15436-1-stanley.chu@mediatek.com>
Allow unbound MPHY module since not every MediaTek UFS platform
needs specific MPHY control.
Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
Reviewed-by: Peter Wang <peter.wang@mediatek.com>
---
drivers/scsi/ufs/ufs-mediatek.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/scsi/ufs/ufs-mediatek.c b/drivers/scsi/ufs/ufs-mediatek.c
index 1cc7bea1468b..9a4432c9f7dc 100644
--- a/drivers/scsi/ufs/ufs-mediatek.c
+++ b/drivers/scsi/ufs/ufs-mediatek.c
@@ -113,6 +113,12 @@ static int ufs_mtk_bind_mphy(struct ufs_hba *hba)
if (err)
host->mphy = NULL;
+ /*
+ * Allow unbound mphy because not every platform needs specific
+ * mphy control.
+ */
+ if (err == -ENODEV)
+ err = 0;
return err;
}
--
2.18.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: [RFC PATCH V4 4/4] platform: mtk-isp: Add Mediatek FD driver
From: Jerry-ch Chen @ 2020-06-01 10:37 UTC (permalink / raw)
To: Tomasz Figa
Cc: linux-devicetree, Sean Cheng (鄭昇弘),
Laurent Pinchart, zwisler, srv_heupstream,
Christie Yu (游雅惠), Hans Verkuil,
Jungo Lin (林明俊), Sj Huang, yuzhao,
Hans Verkuil, Pi-Hsun Shih,
Frederic Chen (陳俊元), Matthias Brugger,
moderated list:ARM/Mediatek SoC support, Mauro Carvalho Chehab,
list@263.net:IOMMU DRIVERS <iommu@lists.linux-foundation.org>, Joerg Roedel <joro@8bytes.org>, ,
Linux Media Mailing List
In-Reply-To: <CAAFQd5BBfapVv_3cwGte=p=6G8QXZQP=-ciZ8NBZZeSBGrHmCA@mail.gmail.com>
On Fri, 2020-05-29 at 14:59 +0200, Tomasz Figa wrote:
> On Fri, May 29, 2020 at 2:26 PM Jerry-ch Chen
> <Jerry-ch.Chen@mediatek.com> wrote:
> >
> > Hi Tomasz,
> >
> > I Appreciate your review comments, here's the reply.
> >
> > On Mon, 2020-05-25 at 14:24 +0200, Tomasz Figa wrote:
> > > r
> > >
> > > On Fri, May 22, 2020 at 4:11 PM Jerry-ch Chen
> > > <Jerry-ch.Chen@mediatek.com> wrote:
> > > >
> > > > Hi Tomasz,
> > > >
> > > > On Thu, 2020-05-21 at 18:28 +0000, Tomasz Figa wrote:
> > > > > Hi Jerry,
> > > > >
> > > > > On Wed, Dec 04, 2019 at 08:47:32PM +0800, Jerry-ch Chen wrote:
> [snip]
> > > Isn't still a need to clamp() width and height to min/max, though?
> > Yes, I'll add them back.
> >
> > This function will be refined as :
> >
> > static void mtk_fd_fill_pixfmt_mp(struct v4l2_pix_format_mplane *dfmt,
> > u32 pixfmt)
> > {
> > v4l2_fill_pixfmt_mp(dfmt, pixfmt, dfmt->width, dfmt->height);
> >
> > dfmt->field = V4L2_FIELD_NONE;
> > dfmt->colorspace = V4L2_COLORSPACE_BT2020;
> > dfmt->ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;
> > dfmt->quantization = V4L2_QUANTIZATION_DEFAULT;
> > dfmt->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(dfmt->colorspace);
> >
> > /* Keep user setting as possible */
> > dfmt->width = clamp(dfmt->width,
> > MTK_FD_OUTPUT_MIN_WIDTH,
> > MTK_FD_OUTPUT_MAX_WIDTH);
> > dfmt->height = clamp(dfmt->height,
> > MTK_FD_OUTPUT_MIN_HEIGHT,
> > MTK_FD_OUTPUT_MAX_HEIGHT);
>
> Note that this would cause the other fields of dfmt to be inconsistent
> with width and height. The correct way to do this would be to first
> clamp and then call v4l2_fill_pixfmt_mp().
>
Ok, I will fix it.
Thanks and Best regards,
Jerry
> Best regards,
> Tomasz
_______________________________________________
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 0/4] Introduce the Counter character device interface
From: Jonathan Cameron @ 2020-06-01 10:31 UTC (permalink / raw)
To: William Breathitt Gray
Cc: kamel.bouhara, gwendal, david, linux-iio, patrick.havelange,
alexandre.belloni, linux-kernel, linux-arm-kernel,
mcoquelin.stm32, fabrice.gasnier, syednwaris, linux-stm32,
Jonathan Cameron, alexandre.torgue
In-Reply-To: <20200531171351.GA10597@shinobu>
On Sun, 31 May 2020 13:14:06 -0400
William Breathitt Gray <vilhelm.gray@gmail.com> wrote:
> On Sun, May 31, 2020 at 04:18:13PM +0100, Jonathan Cameron wrote:
> > On Sun, 24 May 2020 13:54:39 -0400
> > William Breathitt Gray <vilhelm.gray@gmail.com> wrote:
> > > After giving this some more thought, I believe human-readable sysfs
> > > attributes are the way to go to support configuration of the character
> > > device. I am thinking of a system like this:
> > >
> > > * Users configure the counter character device via a sysfs attribute
> > > such as /sys/bus/counter/devices/counterX/chrdev_format or similar.
> > >
> > > * Users may write to this sysfs attribute to select the components they
> > > want to interface -- the layout can be determined as well from the
> > > order. For example:
> > >
> > > # echo "C0 C3 C2" > /sys/bus/counter/devices/counter0/chrdev_format
> >
> > I guess that 'just' meets the sysfs requirement of one file => one thing.
>
> We can massage this further to make it more apt, but the main idea here
> is that configuration should be separate from our data; and that
> configuration should be performed via sysfs.
>
> > > This would select Counts 0, 3, and 2 (in that order) to be available
> > > in the /dev/counter0 node as a contiguous memory region.
> > >
> > > You can select extensions in a similar fashion:
> > >
> > > # echo "C4E2 S1E0" > /sys/bus/counter/devices/counter0/chrdev_format
> > >
> > > This would select extension 2 from Count 4, and extension 0 from
> > > Signal 1.
> >
> > I'm not totally clear why we'd want to have a chrdev access to extensions.
> > To be honest I'm not totally sure what an extension is today as it's been
> > a week ;)
>
> In the context of the Counter subsystem, an extension is data/control
> that is not one of the core components of the Counter paradigm (i.e. not
> a Counter, Signal, nor Synapse). Extensions essentially represent
> configuration options for the counter device and auxiliary
> functionality.
>
> The "Implementation" section of the Generic Counter documentation
> Documentation/driver-api/generic-counter.rst file gives some good
> examples of extensions, but I'll provide an example here for the sake of
> this new character device interface.
>
> Suppose we have a robot controlling a laser on a dual-axes positioning
> table. A counter device is used to track the position of the laser:
> Count 0 represents position on the X axis, while Count 1 represents
> position on the Y axis. Because this machine is moving across two axes
> at the same time, we want to grab both counts together via the
> character device subsystem (grabbing them separately via sysfs would be
> imprecise due to the inherent latency).
>
> The motors are physically able the robot out of the work area, which is
> not something we want to happen. A common setup in systems like this is
> to set soft boundaries on the counter device to represent the edge of
> the work area; when the boundary is passed, a flag is set high on the
> device to indicate the position is out-of-bounds.
>
> On the Counter subsystem side, this counter device would appear as
> having four sysfs attributes: count0/count, count0/boundary,
> count1/count, and count1/boundary. In terms of the character device
> interface, we could perform a setup like this:
>
> # echo "C0E0 C0 C1E1 C1" > counter0/chrdev_format
>
> Yielding the following /dev/counter0 memory layout:
>
> +------------+-----------------+------------+-------------------+
> | Byte 0 | Byte 1 - Byte 8 | Byte 9 | Byte 10 - Byte 17 |
> +------------+-----------------+------------+-------------------+
> | Boundary 0 | Count 0 | Boundary 1 | Count 1 |
> +------------+-----------------+------------+-------------------+
>
> Now a single read() operation can grab the counts together as well as
> their respective boundary flags to verify whether the current counts are
> valid. This is a scenario where using sysfs wouldn't be viable to use:
> we could check the count0/boundary sysfs attribute first, but by the
> time we read the count0/count sysfs attribute second, the robot has
> already moved to a new (possibly invalid) position.
Ok. So typically something like a condition flag. Data that indeed makes
sense to be associated with a set of counter values.
>
> > Perhaps an example? I see timestamp below. What is that attached to?
> > If we gave multiple counters, do they each have a timestamp?
>
> Some counter devices feature "timestamp" functionality. I haven't yet
> implemented this in the Counter subsystem because it's new functionality
> and I want to keep this patchset limited to the existing Counter
> subsystem functionality support.
>
> However, to briefly go into the topic (we'll need to discuss this more
> in-depth before committing to a Counter subsystem design), some counter
> devices can keep track of historic counts based on various events; we
> call these "timestamps", although they may not necessary be tied to a
> wall-clock time.
>
> For example, quadrature encoders often have an "index" signal in
> addition to the quadrature A and B lines. This index signal may be used
> to home a positioning device, or perhaps to indicate that a full
> revolution -- or some other event -- has occurred. It's common for
> quadrature counter devices to provide a FIFO buffer that logs these
> "index" events by saving the current count when that respective index
> signal goes high. Thus we have a timestamp buffer.
That doesn't sound like a timestamp buffer. You are logging counts,
not the current time. If they are going through a FIFO you might also
capture a freerunning timer though.
>
> In the context of the Counter subsystem, I believe we will end up
> implementing these timestamps as Count extensions (or Device extensions
> if it's a single buffer for the entire device). I'm not sure yet what
> the sysfs attribute will display, but I'm guessing we'll have a
> respective /sys/bus/counter/devices/counterX/countX/timestamps sysfs
> attribute or similar.
>
> The character device implementation should be more straight forward I
> would imagine. Since it's a memory buffer, I think we can provide access
> to that buffer directly in the chrdev:
>
> # echo "C0E0 C1E1" > /sys/bus/counter/devices/counter0/chrdev_format
>
> Yielding the following /dev/counter0 memory layout for 32-byte
> timestamps:
>
> +---------------------+---------------------+
> | Byte 0 - Byte 31 | Byte 32 - Byte 63 |
> +---------------------+---------------------+
> | Timestamps Buffer 0 | Timestamps Buffer 1 |
> +---------------------+---------------------+
As you say, will need more discussion. Lots of fun questions around timestamps
such as what the timebase is, how to relate it to system time etc.
>
> > > * Users may read from this chrdev_format sysfs attribute in order to see
> > > the currently configured format of the character device.
> > >
> > > * Users may perform read/write operations on the /dev/counterX node
> > > directly; the layout of the data is what they user has configured via
> > > the chrdev_format sysfs attribute. For example:
> > >
> > > # echo "C0 C1 S0 S1" > /sys/bus/counter/devices/counter0/chrdev_format
> > >
> > > Yields the following /dev/counter0 memory layout:
> > >
> > > +-----------------+------------------+----------+----------+
> > > | Byte 0 - Byte 7 | Byte 8 - Byte 15 | Byte 16 | Byte 17 |
> > > +-----------------+------------------+----------+----------+
> > > | Count 0 | Count 1 | Signal 0 | Signal 2 |
> > > +-----------------+------------------+----------+----------+
> > >
> > > * Users may perform select/poll operations on the /dev/counterX node.
> > > Users can be notified if data is available or events have occurred.
> >
> > One thing to think about early if watermarks. We bolted them on
> > late in IIO and maybe we could have done it better from the start.
> > I'd almost guarantee someone will want one fairly soon - particularly
> > as it's more than possible you'll have a counter device with a
> > hardware fifo. I have some vague recollection that ti-ecap
> > stuff could be presented as a short fifo for starters.
> >
> > >
> > > The benefit of this design is that the format is robust so users can
> > > choose the components they want to interface and in the layout they
> > > want. For example, if I am writing a userspace application to control a
> > > dual-axis positioning table, I can select the two counts I care about
> > > for the position axes. This allows me to read just those two values
> > > directly from /dev/counterX with a simple read() call, without having to
> > > fumble around seeking to an offset and parsing the layout.
> >
> > I wonder if I'm over thinking things for counters, but you may run into
> > the complexity of different counters having different sampling frequencies.
> > Here you are suggesting a scheme that I think ends up closer to IIO than
> > input. That makes this case a pain. Input takes the view that it's
> > fine to have data coming in any order and frequency because every
> > record is self describing. I'm not sure it matters here, but it is
> > a nice layer of flexibility, but you do loose the efficiency of
> > the description being external to the data flow.
>
> I think one of the downsides to using a single a single character device
> node to represent the entire counter device is that the frequency of two
> individual counts may differ from each other. For example, using the
> dual-axes positioning scenario from earlier, one axis might change more
> frequently than the other (e.g. a conveyor belt situation where X is
> always moving forward, while Y only changes when a part appears within
> the work area).
One option is to allow for either multiple opening of the chardev, or
creation of anon file handles via an IOCTL (like we do for events in IIO).
Makes the sysfs interface more complex, but would allow you to handle multiple
independent raw data streams.
>
> In these cases, I think the Input subsystem approach might be better
> because the user can just wait for events at large and handle each event
> as it comes in, rather than try to coordinate between them all in a
> shared memory space. The shortcoming with this approach is that we lose
> the ability to grab Counts together at the same time, such as we require
> in the constantly moving robot example earlier.
There are approaches like adding a sequence number to allow multiple self
describing records to be associated.
>
> Perhaps what might work is to implement Counter events (perhaps even
> timestamps) using the Input subsystem, and leave the Counter character
> device interface to simple read/write operations. But we'll need to
> investigate this further because we lack a concept of "events" right now
> in the Counter subsystem.
>
> > > Similarly, support for future extensions is simple to implement. When
> > > timestamp support is implemented, users can just select the desired
> > > timestamp extension and read it directly from the /dev/counterX node;
> > > they should also be able to perform a select()/poll() call to be
> > > notified on new timestamps.
> > >
> > > So what do you think of this sort of design? I think there is a useful
> > > robustness to the simplicity of performing a single read/write call on
> > > /dev/counterX.
> >
> > It seems like a reasonable solution to me. The only blurry
> > boundary to my mind is what level of buffering is behind this.
> > The things you can do are open, non blocking read, blocking read and select.
> >
> > If we have a counter that is sampled on demand, then
> > 1) Non blocking read - makes not sense, fair enough I guess, could make it
> > the same as a blocking read.
> > 2) Blocking read - reads from the sensor.
> > 3) Select, meaningless as all reads are done on demand - so I guess you
> > hardwire it to return immediately.
> > 4) open. Nothing special
> >
> > If you have a counter that is self clocking then data gets pushed into some
> > software structure (probably kfifo)
> > 1) Blocking read, question of semantics to resolve
> > a) Return when 'some' data is available (like a socket)
> > b) Return when 'requested amount of data is available'?
> > 2) Non blocking read. Return whatever happens to be available.
> > 3) Select. Semantics to be defined.
> > a) Some data?
> > b) Watermark based (default watermark is 0 so any data triggers it)
> > 4) Open. Starts up sampling of configured set - (typically turns on the
> > device, enables interrupt output etc.)
> >
> > So some corners to resolve but should all work.
>
> I'm not familiar with the "watermark" terminology. Would you be able to
> explain it bit more for me. Is this simply a flag that indicates if data
> has changed from the last time it was checked?
If you have a FIFO, watermark is the fill level at which you indicate you
have data. For a HW FIFO this is usually an interrupt at say 10 elements
queued up. The software is then expected to drain at least 10 elements.
For software fifo the concept is the same, but you are expecting userspace
to drain N elements on each event.
If you are going to deal with remotely high sampling rates you'll need to
support this for both software and hardware FIFOs.
>
> > > > > Moving this selection to a sysfs attribute and dedicating the
> > > > > character device to just data transfer might be a better design. If
> > > > > such a design is chosen, should the selection attribute be
> > > > > human-readable or binary?
> > > >
> > > > Sysfs basically requires things are more or less human readable.
> > > > So if you go that way I think it needs to be.
> > > >
> > > > >
> > > > > 2. How much space should allotted for strings?
> > > > >
> > > > > Each Counter component and extension has a respective size allotted
> > > > > for its data (u8 data is allotted 1 byte, u64 data is allotted 8
> > > > > bytes, etc.); I have arbitrarily chosen to allot 64 bytes for
> > > > > strings. Is this an apt size, or should string data be allotted more
> > > > > or less space?
> > > >
> > > > I'd go with that being big enough, but try to keep the expose interface
> > > > such that the size can change it it needs to the in the future.
> > >
> > > Following along with the separation of control vs data as discussed
> > > above, we could support a more variable size by exposing it through a
> > > sysfs attribute (maybe a chrdev_string_size attribute or similar).
> >
> > I'm unconvinced you'd ever want to return a string via the chardev.
> > People are using the chrdev to get efficiency. String based data flows
> > are rarely that!
>
> That's a good point. I don't think there is a situation right now where
> we need to deliver strings via the character device interface, so I
> think I'll remove that in v3.
Cool. This all seems to be heading in a sensible direction, but as you say
lots still to consider.
Jonathan
>
> William Breathitt Gray
>
_______________________________________________
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