* Re: [WIP 0/3] Memory model and atomic API in Rust
From: Kent Overstreet @ 2024-03-26 3:28 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: Linus Torvalds, Philipp Stanner, Boqun Feng, rust-for-linux,
linux-kernel, linux-arch, llvm, Miguel Ojeda, Alex Gaynor,
Wedson Almeida Filho, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Alan Stern,
Andrea Parri, Will Deacon, Peter Zijlstra, Nicholas Piggin,
David Howells, Jade Alglave, Luc Maranget, Paul E. McKenney,
Akira Yokosawa, Daniel Lustig, Joel Fernandes, Nathan Chancellor,
Nick Desaulniers, kent.overstreet, Greg Kroah-Hartman, elver,
Mark Rutland, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Catalin Marinas,
linux-arm-kernel, linux-fsdevel
In-Reply-To: <ZgImcq2vRcDZtF6z@gallifrey>
On Tue, Mar 26, 2024 at 01:35:46AM +0000, Dr. David Alan Gilbert wrote:
> OK, so that's essentially the opposite worry of what I was saying; I was
> worrying about people forgetting to use an atomic access to a shared
> variable; I think you're worrying about people forgetting to mark
> a variable shared and since the accesses are the same nothing shouts?
In biological evolution, novel useful traits are generally not
accessible via a single mutation; many neutral mutations are required
first.
Evolution is able to proceed quickly because there are a great many
neutral mutations (that is, evolution quickly searches all possible
paths to find accessible positive mutations), and because negative
mutations are culled quickly - often before the first cell division.
(The most common mutation being the addition or deletion of a base pair;
but amino acids are coded for by groups of three base pairs, so that
shifts everything on the chromosone after the mutation so that it codes
for completely different amino acids. That cell won't live to divide
again).
Actual genetic diseases that significantly impair fitness are quite
rare, and if they weren't we'd have a major problem.
Programming at scale is million monkeys stuff - we're all hammering on
our keyboards at random, the good programs survive and the bad programs
are forgotten.
Similarly to biological evolution, we want most edits to a program to
result in a program that either still works, or fails immediately -
fails to compile, or is caught immediately by basic testing.
If edits can result in latent undefined behaviour or programs that
_mostly_ work, and then explode in unpredictable ways weeks/months/years
later - that's a huge problem. In the worst case, those bugs/negative
mutations accumulate faster than they can be culled.
Thank god we have source control.
Places where we're working with extremely loose synchronization - no
locking, raw memory barriers - are the worst kind of hand grenade, they
result in bugs that are impossible to cull quickly.
In kernel programming, we're always walking around with live hand
grenades.
So what do we do?
We slow down, we take every step slowly and intentionally while telling
everyone not to bump us because we're holding a live hand grenade - raw
atomics, raw unlocked variables, memory barriers, they all get special
care and extra comments.
And we all have fuck-tons of code we need to be able to understand,
review, debug and maintain, so we always try to write our code in a
style where the if it's wrong, we'll see that _locally_, without having
to go through and remember how _everything_ out of the possibly
thousands of relevant lines work.
I'm personally responsible for over 100k LOC of highly intricate code
with high consequences for failure, and regularly have to debug issues
arising somewhere in north of a million LOC - and when something goes
wrong I have to be able to fully debug it _quickly_.
What C++ does is like taking those hand grenades, with the pin already
out - and leaving one under the couch cushions, another in the
silverware drawer, another in the laundry basket - and expecting you to
remember where you put them.
Going back to the C++ example, the really fatal thing with how they do
it is how a change in one line of code can completely change the
semantics of a bunch of different code, and no human reviewer can be
expected to catch bugs that might introduce and the compiler certainly
won't.
Now imagine multiple people working on the same code, at different
times.
Now imagine patches getting mixed up, reordered, one of them getting
lost, merge conflicts - i.e. shit that happens all the time, and what
happens if you're using C++ style atomics.
Terrifying stuff.
_______________________________________________
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 v3] net: ll_temac: platform_get_resource replaced by wrong function
From: patchwork-bot+netdevbpf @ 2024-03-26 3:10 UTC (permalink / raw)
To: Claus Hansen Ries
Cc: netdev, davem, edumazet, kuba, pabeni, michal.simek, wei.fang,
yangyingliang, robh, harini.katakam, dan.carpenter,
u.kleine-koenig, wanghai38, linux-arm-kernel, linux-kernel, horms
In-Reply-To: <cca18f9c630a41c18487729770b492bb@terma.com>
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Thu, 21 Mar 2024 13:08:59 +0000 you wrote:
> The function platform_get_resource was replaced with
> devm_platform_ioremap_resource_byname and is called using 0 as name.
>
> This eventually ends up in platform_get_resource_byname in the call
> stack, where it causes a null pointer in strcmp.
>
> if (type == resource_type(r) && !strcmp(r->name, name))
>
> [...]
Here is the summary with links:
- [net,v3] net: ll_temac: platform_get_resource replaced by wrong function
https://git.kernel.org/netdev/net/c/3a38a829c8bc
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
_______________________________________________
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 v5 2/3] KVM: selftests: aarch64: Introduce pmu_event_filter_test
From: Shaoqin Huang @ 2024-03-26 3:06 UTC (permalink / raw)
To: Eric Auger, Oliver Upton, Marc Zyngier, kvmarm
Cc: Paolo Bonzini, Shuah Khan, James Morse, Suzuki K Poulose,
Zenghui Yu, linux-kernel, kvm, linux-kselftest, linux-arm-kernel
In-Reply-To: <f003ce3e-12d0-42fd-84a9-ebb3baac3860@redhat.com>
Hi Eric,
Thanks for your review.
On 3/20/24 04:58, Eric Auger wrote:
> Hi Shaoqin,
>
> On 2/29/24 07:56, Shaoqin Huang wrote:
>> Introduce pmu_event_filter_test for arm64 platforms. The test configures
>> PMUv3 for a vCPU, and sets different pmu event filters for the vCPU, and
>> check if the guest can see those events which user allow and can't use
>> those events which use deny.
>>
>> This test refactor the create_vpmu_vm() and make it a wrapper for
>> __create_vpmu_vm(), which allows some extra init code before
>> KVM_ARM_VCPU_PMU_V3_INIT.
>>
>> And this test use the KVM_ARM_VCPU_PMU_V3_FILTER attribute to set the
>> pmu event filter in KVM. And choose to filter two common event
>> branches_retired and instructions_retired, and let the guest to check if
>> it see the right pmceid register.
>>
>> Signed-off-by: Shaoqin Huang <shahuang@redhat.com>
>> ---
>> tools/testing/selftests/kvm/Makefile | 1 +
>> .../kvm/aarch64/pmu_event_filter_test.c | 287 ++++++++++++++++++
>> 2 files changed, 288 insertions(+)
>> create mode 100644 tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c
>>
>> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
>> index 492e937fab00..732ca5f8bfc0 100644
>> --- a/tools/testing/selftests/kvm/Makefile
>> +++ b/tools/testing/selftests/kvm/Makefile
>> @@ -147,6 +147,7 @@ TEST_GEN_PROGS_aarch64 += aarch64/arch_timer
>> TEST_GEN_PROGS_aarch64 += aarch64/debug-exceptions
>> TEST_GEN_PROGS_aarch64 += aarch64/hypercalls
>> TEST_GEN_PROGS_aarch64 += aarch64/page_fault_test
>> +TEST_GEN_PROGS_aarch64 += aarch64/pmu_event_filter_test
>> TEST_GEN_PROGS_aarch64 += aarch64/psci_test
>> TEST_GEN_PROGS_aarch64 += aarch64/set_id_regs
>> TEST_GEN_PROGS_aarch64 += aarch64/smccc_filter
>> diff --git a/tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c b/tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c
>> new file mode 100644
>> index 000000000000..2dd8ea418f47
>> --- /dev/null
>> +++ b/tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c
>> @@ -0,0 +1,287 @@
>> +
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * pmu_event_filter_test - Test user limit pmu event for guest.
>> + *
>> + * Copyright (c) 2023 Red Hat, Inc.
>> + *
>> + * This test checks if the guest only see the limited pmu event that userspace
>> + * sets, if the guest can use those events which user allow, and if the guest
>> + * can't use those events which user deny.
>> + * This test runs only when KVM_CAP_ARM_PMU_V3, KVM_ARM_VCPU_PMU_V3_FILTER
>> + * is supported on the host.
>> + */
>> +#include <kvm_util.h>
>> +#include <processor.h>
>> +#include <vgic.h>
>> +#include <vpmu.h>
>> +#include <test_util.h>
>> +#include <perf/arm_pmuv3.h>
>> +
>> +struct pmu_common_event_ids {
>> + uint64_t pmceid0;
>> + uint64_t pmceid1;
>> +} max_pmce, expected_pmce;
>> +
>> +struct vpmu_vm {
>> + struct kvm_vm *vm;
>> + struct kvm_vcpu *vcpu;
>> + int gic_fd;
>> +};
>> +
>> +static struct vpmu_vm vpmu_vm;
>> +
>> +#define FILTER_NR 10
>> +
>> +struct test_desc {
>> + const char *name;
>> + struct kvm_pmu_event_filter filter[FILTER_NR];
>> +};
>> +
>> +#define __DEFINE_FILTER(base, num, act) \
>> + ((struct kvm_pmu_event_filter) { \
>> + .base_event = base, \
>> + .nevents = num, \
>> + .action = act, \
>> + })
>> +
>> +#define DEFINE_FILTER(base, act) __DEFINE_FILTER(base, 1, act)
>> +
>> +static void guest_code(void)
>> +{
>> + uint64_t pmceid0 = read_sysreg(pmceid0_el0);
>> + uint64_t pmceid1 = read_sysreg(pmceid1_el0);
>> +
>> + GUEST_ASSERT_EQ(expected_pmce.pmceid0, pmceid0);
>> + GUEST_ASSERT_EQ(expected_pmce.pmceid1, pmceid1);
>> +
>> + GUEST_DONE();
>> +}
>> +
>> +static void guest_get_pmceid(void)
>> +{
>> + max_pmce.pmceid0 = read_sysreg(pmceid0_el0);
>> + max_pmce.pmceid1 = read_sysreg(pmceid1_el0);
>> +
>> + GUEST_DONE();
>> +}
>> +
>> +static void run_vcpu(struct kvm_vcpu *vcpu)
>> +{
>> + struct ucall uc;
>> +
>> + while (1) {
>> + vcpu_run(vcpu);
>> + switch (get_ucall(vcpu, &uc)) {
>> + case UCALL_DONE:
>> + return;
>> + case UCALL_ABORT:
>> + REPORT_GUEST_ASSERT(uc);
>> + break;
>> + default:
>> + TEST_FAIL("Unknown ucall %lu", uc.cmd);
>> + }
>> + }
>> +}
>> +
>> +static void set_pmce(struct pmu_common_event_ids *pmce, int action, int event)
>> +{
>> + int base = 0;
>> + uint64_t *pmceid = NULL;
>> +
>> + if (event >= 0x4000) {
>> + event -= 0x4000;
>> + base = 32;
>> + }
>> +
>> + if (event >= 0 && event <= 0x1F) {
>> + pmceid = &pmce->pmceid0;
>> + } else if (event >= 0x20 && event <= 0x3F) {
>> + event -= 0x20;
>> + pmceid = &pmce->pmceid1;
>> + } else {
>> + return;
>> + }
>> +
>> + event += base;
>> + if (action == KVM_PMU_EVENT_ALLOW)
>> + *pmceid |= BIT(event);
>> + else
>> + *pmceid &= ~BIT(event);
>> +}
>> +
>> +static void prepare_expected_pmce(struct kvm_pmu_event_filter *filter)
>> +{
>> + struct pmu_common_event_ids pmce_mask = { ~0, ~0 };
>> + bool first_filter = true;
>> +
>> + while (filter && filter->nevents != 0) {
> isn't filter->nevents != 0 sufficient?
No. Because the filter might be NULL.
>> + if (first_filter) {
>> + if (filter->action == KVM_PMU_EVENT_ALLOW)
>> + memset(&pmce_mask, 0, sizeof(pmce_mask));
>> + first_filter = false;
>> + }
>> +
>> + set_pmce(&pmce_mask, filter->action, filter->base_event);
>> + filter++;
>> + }
>> +
>> + expected_pmce.pmceid0 = max_pmce.pmceid0 & pmce_mask.pmceid0;
>> + expected_pmce.pmceid1 = max_pmce.pmceid1 & pmce_mask.pmceid1;
>> +}
>> +
>> +static void pmu_event_filter_init(struct kvm_pmu_event_filter *filter)
>> +{
>> + while (filter && filter->nevents != 0) {
>> + kvm_device_attr_set(vpmu_vm.vcpu->fd,
>> + KVM_ARM_VCPU_PMU_V3_CTRL,
>> + KVM_ARM_VCPU_PMU_V3_FILTER,
>> + filter);
>> + filter++;
>> + }
>> +}
>> +
>> +#define GICD_BASE_GPA 0x8000000ULL
>> +#define GICR_BASE_GPA 0x80A0000ULL
> I think Oliver suggested to move those defines to GIC headers?
There is another patch[1] which move those definition to the GIC
headers, but it hasn't been merged.
[1]https://lore.kernel.org/all/20240307183907.1184775-2-coltonlewis@google.com/
>> +
>> +/* Create a VM that has one vCPU with PMUv3 configured. */
>> +static void create_vpmu_vm_with_filter(void *guest_code,
>> + struct kvm_pmu_event_filter *filter)
>> +{
>> + uint64_t irq = 23;
>> +
>> + /* The test creates the vpmu_vm multiple times. Ensure a clean state */
>> + memset(&vpmu_vm, 0, sizeof(vpmu_vm));
>> +
>> + vpmu_vm.vm = vm_create(1);
>> + vpmu_vm.vcpu = vm_vcpu_add_with_vpmu(vpmu_vm.vm, 0, guest_code);
>> + vpmu_vm.gic_fd = vgic_v3_setup(vpmu_vm.vm, 1, 64,
>> + GICD_BASE_GPA, GICR_BASE_GPA);
>> + __TEST_REQUIRE(vpmu_vm.gic_fd >= 0,
>> + "Failed to create vgic-v3, skipping");
>> +
>> + pmu_event_filter_init(filter);
>> +
>> + /* Initialize vPMU */
>> + vpmu_set_irq(vpmu_vm.vcpu, irq);
>> + vpmu_init(vpmu_vm.vcpu);
>> +}
>> +
>> +static void create_vpmu_vm(void *guest_code)
>> +{
>> + create_vpmu_vm_with_filter(guest_code, NULL);
>> +}
>> +
>> +static void destroy_vpmu_vm(void)
>> +{
>> + close(vpmu_vm.gic_fd);
>> + kvm_vm_free(vpmu_vm.vm);
>> +}
>> +
>> +static void run_test(struct test_desc *t)
>> +{
>> + pr_info("Test: %s\n", t->name);
>> +
>> + create_vpmu_vm_with_filter(guest_code, t->filter);
>> + prepare_expected_pmce(t->filter);
>> + sync_global_to_guest(vpmu_vm.vm, expected_pmce);
>> +
>> + run_vcpu(vpmu_vm.vcpu);
>> +
>> + destroy_vpmu_vm();
>> +}
>> +
>> +static struct test_desc tests[] = {
>> + {
>> + .name = "without_filter",
>> + .filter = {
>> + { 0 }
>> + },
>> + },
>> + {
>> + .name = "member_allow_filter",
>> + .filter = {
>> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_SW_INCR, 0),
>> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_INST_RETIRED, 0),
>> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_BR_RETIRED, 0),
>> + { 0 },
>> + },
>> + },
>> + {
>> + .name = "member_deny_filter",
>> + .filter = {
>> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_SW_INCR, 1),
>> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_INST_RETIRED, 1),
>> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_BR_RETIRED, 1),
>> + { 0 },
>> + },
>> + },
>> + {
>> + .name = "not_member_deny_filter",
>> + .filter = {
>> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_SW_INCR, 1),
>> + { 0 },
>> + },
>> + },
>> + {
>> + .name = "not_member_allow_filter",
>> + .filter = {
>> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_SW_INCR, 0),
>> + { 0 },
>> + },
>> + },
>> + {
>> + .name = "deny_chain_filter",
>> + .filter = {
>> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_CHAIN, 1),
>> + { 0 },
>> + },
>> + },
>> + {
>> + .name = "deny_cpu_cycles_filter",
>> + .filter = {
>> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_CPU_CYCLES, 1),
>> + { 0 },
>> + },
>> + },
>> + {
>> + .name = "cancle_filter",
> cancel
Thanks for catching it.
>> + .filter = {
>> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_CPU_CYCLES, 0),
>> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_CPU_CYCLES, 1),
>> + },
>> + },
>> + { 0 }
>> +};
>> +
>> +static void run_tests(void)
>> +{
>> + struct test_desc *t;
>> +
>> + for (t = &tests[0]; t->name; t++)
>> + run_test(t);
>> +}
>> +
>> +static bool kvm_pmu_support_events(void)
>> +{
>> + create_vpmu_vm(guest_get_pmceid);
>> +
>> + memset(&max_pmce, 0, sizeof(max_pmce));
>> + sync_global_to_guest(vpmu_vm.vm, max_pmce);
>> + run_vcpu(vpmu_vm.vcpu);
>> + sync_global_from_guest(vpmu_vm.vm, max_pmce);
>> + destroy_vpmu_vm();
>> +
>> + return max_pmce.pmceid0 &
>> + (ARMV8_PMUV3_PERFCTR_BR_RETIRED |
>> + ARMV8_PMUV3_PERFCTR_INST_RETIRED |
>> + ARMV8_PMUV3_PERFCTR_CHAIN);
> I understood Oliver suggested to use sysfs to know if te events were
> supported and to use KVM_ARM_VCPU_PMU_V3_SET_PMU. On my end I am fine
> with your method though.
>
I think get the supported pmu events from KVM is better than get the
host supported pmu events even they are the same currently.
Thanks,
Shaoqin
> Thanks
>
> Eric
>> +}
>> +
>> +int main(void)
>> +{
>> + TEST_REQUIRE(kvm_has_cap(KVM_CAP_ARM_PMU_V3));
>> + TEST_REQUIRE(kvm_pmu_support_events());
>> +
>> + run_tests();
>> +}
>
--
Shaoqin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RESEND][PATCH v2 1/4] OPP: OF: Export dev_opp_pm_calc_power() for usage from EM
From: Viresh Kumar @ 2024-03-26 2:51 UTC (permalink / raw)
To: Lukasz Luba
Cc: linux-kernel, linux-pm, dietmar.eggemann, linux-arm-kernel, sboyd,
nm, linux-samsung-soc, daniel.lezcano, rafael,
krzysztof.kozlowski, alim.akhtar, m.szyprowski, mhiramat
In-Reply-To: <20240322110850.77086-2-lukasz.luba@arm.com>
On 22-03-24, 11:08, Lukasz Luba wrote:
> There are device drivers which can modify voltage values for OPPs. It
> could be due to the chip binning and those drivers have specific chip
> knowledge about it. This adjustment can happen after Energy Model is
> registered, thus EM can have stale data about power.
>
> Export dev_opp_pm_calc_power() which can be used by Energy Model to
> calculate new power with the new voltage for OPPs.
>
> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
> ---
> drivers/opp/of.c | 17 ++++++++++++-----
> include/linux/pm_opp.h | 8 ++++++++
> 2 files changed, 20 insertions(+), 5 deletions(-)
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
--
viresh
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [WIP 0/3] Memory model and atomic API in Rust
From: Boqun Feng @ 2024-03-26 2:51 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: Linus Torvalds, Kent Overstreet, Philipp Stanner, rust-for-linux,
linux-kernel, linux-arch, llvm, Miguel Ojeda, Alex Gaynor,
Wedson Almeida Filho, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Alan Stern,
Andrea Parri, Will Deacon, Peter Zijlstra, Nicholas Piggin,
David Howells, Jade Alglave, Luc Maranget, Paul E. McKenney,
Akira Yokosawa, Daniel Lustig, Joel Fernandes, Nathan Chancellor,
Nick Desaulniers, kent.overstreet, Greg Kroah-Hartman, elver,
Mark Rutland, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Catalin Marinas,
linux-arm-kernel, linux-fsdevel
In-Reply-To: <ZgIRXL5YM2AwBD0Y@gallifrey>
On Tue, Mar 26, 2024 at 12:05:48AM +0000, Dr. David Alan Gilbert wrote:
> * Linus Torvalds (torvalds@linux-foundation.org) wrote:
>
> <snip>
>
> > IOW, the whole access size problem that Boqun described is
> > *inherently* tied to the fact that the C++ and Rust memory model is
> > badly designed from the wrong principles.
> >
> > Instead of designing it as a "this is an atomic object that you can do
> > these operations on", it should have been "this is an atomic access,
> > and you can use this simple object model to have the compiler generate
> > the accesses for you".
>
> Isn't one of the aims of the Rust/C++ idea that you can't forget to access
> a shared piece of data atomically?
>
> If you want to have 'atomic accesses' explicitly, how do you tell the compiler
> what you can use them on, and when it should stop you mixing them with
> normal accesses on the same object?
>
Well, you can just wrap it in your own atomic types, can't you?
If the atomic primitives that a language provides is access-based, users
can create their own atomic types or language can provide via standard
library, but mixed usage is still allowed when it makes sense (debug
functionality, low level concurrent code that utilizes races, etc.) But
if the atomic primitives that a language provides is type-based, then
you're limited to what you can do. It might be totally fine as Linus
pointed out, if you just write a portable library, and don't want to
care about architectural details. But that's not the case in Linux
kernel.
Regards,
Boqun
> Dave
>
> > This is why I claim that LKMM is fundamentally better. It didn't start
> > out from a bass-ackwards starting point of marking objects "atomic".
> >
> > And yes, the LKMM is a bit awkward, because we don't have the
> > shorthands, so you have to write out "atomic_read()" and friends.
> >
> > Tough. It's better to be correct than to be simple.
> >
> > Linus
> >
> --
> -----Open up your eyes, open up your mind, open up your code -------
> / Dr. David Alan Gilbert | Running GNU/Linux | Happy \
> \ dave @ treblig.org | | In Hex /
> \ _________________________|_____ http://www.treblig.org |_______/
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v4 10/14] treewide: Use initializer for struct vm_unmapped_area_info
From: Rick Edgecombe @ 2024-03-26 2:16 UTC (permalink / raw)
To: Liam.Howlett, akpm, bp, broonie, christophe.leroy, dave.hansen,
debug, hpa, keescook, kirill.shutemov, luto, mingo, peterz, tglx,
x86
Cc: rick.p.edgecombe, linux-kernel, linux-mm, linux-alpha,
linux-snps-arc, linux-arm-kernel, linux-csky, loongarch,
linux-mips, linux-s390, linux-sh, sparclinux
In-Reply-To: <20240326021656.202649-1-rick.p.edgecombe@intel.com>
Future changes will need to add a new member to struct
vm_unmapped_area_info. This would cause trouble for any call site that
doesn't initialize the struct. Currently every caller sets each member
manually, so if new ones are added they will be uninitialized and the
core code parsing the struct will see garbage in the new member.
It could be possible to initialize the new member manually to 0 at each
call site. This and a couple other options were discussed. Having some
struct vm_unmapped_area_info instances not zero initialized will put
those sites at risk of feeding garbage into vm_unmapped_area(), if the
convention is to zero initialize the struct and any new field addition
missed a call site that initializes each field manually. So it is
useful to do things similar across the kernel.
The consensus (see links) was that in general the best way to accomplish
taking into account both code cleanliness and minimizing the chance of
introducing bugs, was to do C99 static initialization. As in:
struct vm_unmapped_area_info info = {};
With this method of initialization, the whole struct will be zero
initialized, and any statements setting fields to zero will be unneeded.
The change should not leave cleanup at the call sides.
While iterating though the possible solutions a few archs kindly acked
other variations that still zero initialized the struct. These sites have
been modified in previous changes using the pattern acked by the respective
arch.
So to be reduce the chance of bugs via uninitialized fields, perform a
tree wide change using the consensus for the best general way to do this
change. Use C99 static initializing to zero the struct and remove and
statements that simply set members to zero.
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Cc: linux-mm@kvack.org
Cc: linux-alpha@vger.kernel.org
Cc: linux-snps-arc@lists.infradead.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-csky@vger.kernel.org
Cc: loongarch@lists.linux.dev
Cc: linux-mips@vger.kernel.org
Cc: linux-s390@vger.kernel.org
Cc: linux-sh@vger.kernel.org
Cc: sparclinux@vger.kernel.org
Link: https://lore.kernel.org/lkml/202402280912.33AEE7A9CF@keescook/#t
Link: https://lore.kernel.org/lkml/j7bfvig3gew3qruouxrh7z7ehjjafrgkbcmg6tcghhfh3rhmzi@wzlcoecgy5rs/
Link: https://lore.kernel.org/lkml/ec3e377a-c0a0-4dd3-9cb9-96517e54d17e@csgroup.eu/
---
v4:
- Trivial rebase conflict in s390
Hi archs,
For some context, this is part of a larger series to improve shadow stack
guard gaps. It involves plumbing a new field via
struct vm_unmapped_area_info. The first user is x86, but arm and riscv may
likely use it as well. The change is compile tested only for non-x86.
Thanks,
Rick
---
arch/alpha/kernel/osf_sys.c | 5 +----
arch/arc/mm/mmap.c | 4 +---
arch/arm/mm/mmap.c | 5 ++---
arch/loongarch/mm/mmap.c | 3 +--
arch/mips/mm/mmap.c | 3 +--
arch/s390/mm/hugetlbpage.c | 7 ++-----
arch/s390/mm/mmap.c | 5 ++---
arch/sh/mm/mmap.c | 5 ++---
arch/sparc/kernel/sys_sparc_32.c | 3 +--
arch/sparc/kernel/sys_sparc_64.c | 5 ++---
arch/sparc/mm/hugetlbpage.c | 7 ++-----
arch/x86/kernel/sys_x86_64.c | 7 ++-----
arch/x86/mm/hugetlbpage.c | 7 ++-----
fs/hugetlbfs/inode.c | 7 ++-----
mm/mmap.c | 9 ++-------
15 files changed, 25 insertions(+), 57 deletions(-)
diff --git a/arch/alpha/kernel/osf_sys.c b/arch/alpha/kernel/osf_sys.c
index 5db88b627439..e5f881bc8288 100644
--- a/arch/alpha/kernel/osf_sys.c
+++ b/arch/alpha/kernel/osf_sys.c
@@ -1218,14 +1218,11 @@ static unsigned long
arch_get_unmapped_area_1(unsigned long addr, unsigned long len,
unsigned long limit)
{
- struct vm_unmapped_area_info info;
+ struct vm_unmapped_area_info info = {};
- info.flags = 0;
info.length = len;
info.low_limit = addr;
info.high_limit = limit;
- info.align_mask = 0;
- info.align_offset = 0;
return vm_unmapped_area(&info);
}
diff --git a/arch/arc/mm/mmap.c b/arch/arc/mm/mmap.c
index 3c1c7ae73292..69a915297155 100644
--- a/arch/arc/mm/mmap.c
+++ b/arch/arc/mm/mmap.c
@@ -27,7 +27,7 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
{
struct mm_struct *mm = current->mm;
struct vm_area_struct *vma;
- struct vm_unmapped_area_info info;
+ struct vm_unmapped_area_info info = {};
/*
* We enforce the MAP_FIXED case.
@@ -51,11 +51,9 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
return addr;
}
- info.flags = 0;
info.length = len;
info.low_limit = mm->mmap_base;
info.high_limit = TASK_SIZE;
- info.align_mask = 0;
info.align_offset = pgoff << PAGE_SHIFT;
return vm_unmapped_area(&info);
}
diff --git a/arch/arm/mm/mmap.c b/arch/arm/mm/mmap.c
index a0f8a0ca0788..d65d0e6ed10a 100644
--- a/arch/arm/mm/mmap.c
+++ b/arch/arm/mm/mmap.c
@@ -34,7 +34,7 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
struct vm_area_struct *vma;
int do_align = 0;
int aliasing = cache_is_vipt_aliasing();
- struct vm_unmapped_area_info info;
+ struct vm_unmapped_area_info info = {};
/*
* We only need to do colour alignment if either the I or D
@@ -68,7 +68,6 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
return addr;
}
- info.flags = 0;
info.length = len;
info.low_limit = mm->mmap_base;
info.high_limit = TASK_SIZE;
@@ -87,7 +86,7 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
unsigned long addr = addr0;
int do_align = 0;
int aliasing = cache_is_vipt_aliasing();
- struct vm_unmapped_area_info info;
+ struct vm_unmapped_area_info info = {};
/*
* We only need to do colour alignment if either the I or D
diff --git a/arch/loongarch/mm/mmap.c b/arch/loongarch/mm/mmap.c
index a9630a81b38a..4bbd449b4a47 100644
--- a/arch/loongarch/mm/mmap.c
+++ b/arch/loongarch/mm/mmap.c
@@ -24,7 +24,7 @@ static unsigned long arch_get_unmapped_area_common(struct file *filp,
struct vm_area_struct *vma;
unsigned long addr = addr0;
int do_color_align;
- struct vm_unmapped_area_info info;
+ struct vm_unmapped_area_info info = {};
if (unlikely(len > TASK_SIZE))
return -ENOMEM;
@@ -82,7 +82,6 @@ static unsigned long arch_get_unmapped_area_common(struct file *filp,
*/
}
- info.flags = 0;
info.low_limit = mm->mmap_base;
info.high_limit = TASK_SIZE;
return vm_unmapped_area(&info);
diff --git a/arch/mips/mm/mmap.c b/arch/mips/mm/mmap.c
index 00fe90c6db3e..7e11d7b58761 100644
--- a/arch/mips/mm/mmap.c
+++ b/arch/mips/mm/mmap.c
@@ -34,7 +34,7 @@ static unsigned long arch_get_unmapped_area_common(struct file *filp,
struct vm_area_struct *vma;
unsigned long addr = addr0;
int do_color_align;
- struct vm_unmapped_area_info info;
+ struct vm_unmapped_area_info info = {};
if (unlikely(len > TASK_SIZE))
return -ENOMEM;
@@ -92,7 +92,6 @@ static unsigned long arch_get_unmapped_area_common(struct file *filp,
*/
}
- info.flags = 0;
info.low_limit = mm->mmap_base;
info.high_limit = TASK_SIZE;
return vm_unmapped_area(&info);
diff --git a/arch/s390/mm/hugetlbpage.c b/arch/s390/mm/hugetlbpage.c
index 219d906fe830..46de7a4c0309 100644
--- a/arch/s390/mm/hugetlbpage.c
+++ b/arch/s390/mm/hugetlbpage.c
@@ -258,14 +258,12 @@ static unsigned long hugetlb_get_unmapped_area_bottomup(struct file *file,
unsigned long pgoff, unsigned long flags)
{
struct hstate *h = hstate_file(file);
- struct vm_unmapped_area_info info;
+ struct vm_unmapped_area_info info = {};
- info.flags = 0;
info.length = len;
info.low_limit = current->mm->mmap_base;
info.high_limit = TASK_SIZE;
info.align_mask = PAGE_MASK & ~huge_page_mask(h);
- info.align_offset = 0;
return vm_unmapped_area(&info);
}
@@ -274,7 +272,7 @@ static unsigned long hugetlb_get_unmapped_area_topdown(struct file *file,
unsigned long pgoff, unsigned long flags)
{
struct hstate *h = hstate_file(file);
- struct vm_unmapped_area_info info;
+ struct vm_unmapped_area_info info = {};
unsigned long addr;
info.flags = VM_UNMAPPED_AREA_TOPDOWN;
@@ -282,7 +280,6 @@ static unsigned long hugetlb_get_unmapped_area_topdown(struct file *file,
info.low_limit = PAGE_SIZE;
info.high_limit = current->mm->mmap_base;
info.align_mask = PAGE_MASK & ~huge_page_mask(h);
- info.align_offset = 0;
addr = vm_unmapped_area(&info);
/*
diff --git a/arch/s390/mm/mmap.c b/arch/s390/mm/mmap.c
index 6b2e4436ad4a..206756946589 100644
--- a/arch/s390/mm/mmap.c
+++ b/arch/s390/mm/mmap.c
@@ -86,7 +86,7 @@ unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr,
{
struct mm_struct *mm = current->mm;
struct vm_area_struct *vma;
- struct vm_unmapped_area_info info;
+ struct vm_unmapped_area_info info = {};
if (len > TASK_SIZE - mmap_min_addr)
return -ENOMEM;
@@ -102,7 +102,6 @@ unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr,
goto check_asce_limit;
}
- info.flags = 0;
info.length = len;
info.low_limit = mm->mmap_base;
info.high_limit = TASK_SIZE;
@@ -122,7 +121,7 @@ unsigned long arch_get_unmapped_area_topdown(struct file *filp, unsigned long ad
{
struct vm_area_struct *vma;
struct mm_struct *mm = current->mm;
- struct vm_unmapped_area_info info;
+ struct vm_unmapped_area_info info = {};
/* requested length too big for entire address space */
if (len > TASK_SIZE - mmap_min_addr)
diff --git a/arch/sh/mm/mmap.c b/arch/sh/mm/mmap.c
index b82199878b45..bee329d4149a 100644
--- a/arch/sh/mm/mmap.c
+++ b/arch/sh/mm/mmap.c
@@ -57,7 +57,7 @@ unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr,
struct mm_struct *mm = current->mm;
struct vm_area_struct *vma;
int do_colour_align;
- struct vm_unmapped_area_info info;
+ struct vm_unmapped_area_info info = {};
if (flags & MAP_FIXED) {
/* We do not accept a shared mapping if it would violate
@@ -88,7 +88,6 @@ unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr,
return addr;
}
- info.flags = 0;
info.length = len;
info.low_limit = TASK_UNMAPPED_BASE;
info.high_limit = TASK_SIZE;
@@ -106,7 +105,7 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
struct mm_struct *mm = current->mm;
unsigned long addr = addr0;
int do_colour_align;
- struct vm_unmapped_area_info info;
+ struct vm_unmapped_area_info info = {};
if (flags & MAP_FIXED) {
/* We do not accept a shared mapping if it would violate
diff --git a/arch/sparc/kernel/sys_sparc_32.c b/arch/sparc/kernel/sys_sparc_32.c
index 082a551897ed..08a19727795c 100644
--- a/arch/sparc/kernel/sys_sparc_32.c
+++ b/arch/sparc/kernel/sys_sparc_32.c
@@ -41,7 +41,7 @@ SYSCALL_DEFINE0(getpagesize)
unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr, unsigned long len, unsigned long pgoff, unsigned long flags)
{
- struct vm_unmapped_area_info info;
+ struct vm_unmapped_area_info info = {};
if (flags & MAP_FIXED) {
/* We do not accept a shared mapping if it would violate
@@ -59,7 +59,6 @@ unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr, unsi
if (!addr)
addr = TASK_UNMAPPED_BASE;
- info.flags = 0;
info.length = len;
info.low_limit = addr;
info.high_limit = TASK_SIZE;
diff --git a/arch/sparc/kernel/sys_sparc_64.c b/arch/sparc/kernel/sys_sparc_64.c
index 1dbf7211666e..d9c3b34ca744 100644
--- a/arch/sparc/kernel/sys_sparc_64.c
+++ b/arch/sparc/kernel/sys_sparc_64.c
@@ -93,7 +93,7 @@ unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr, unsi
struct vm_area_struct * vma;
unsigned long task_size = TASK_SIZE;
int do_color_align;
- struct vm_unmapped_area_info info;
+ struct vm_unmapped_area_info info = {};
if (flags & MAP_FIXED) {
/* We do not accept a shared mapping if it would violate
@@ -126,7 +126,6 @@ unsigned long arch_get_unmapped_area(struct file *filp, unsigned long addr, unsi
return addr;
}
- info.flags = 0;
info.length = len;
info.low_limit = TASK_UNMAPPED_BASE;
info.high_limit = min(task_size, VA_EXCLUDE_START);
@@ -154,7 +153,7 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
unsigned long task_size = STACK_TOP32;
unsigned long addr = addr0;
int do_color_align;
- struct vm_unmapped_area_info info;
+ struct vm_unmapped_area_info info = {};
/* This should only ever run for 32-bit processes. */
BUG_ON(!test_thread_flag(TIF_32BIT));
diff --git a/arch/sparc/mm/hugetlbpage.c b/arch/sparc/mm/hugetlbpage.c
index 38a1bef47efb..4caf56b32e26 100644
--- a/arch/sparc/mm/hugetlbpage.c
+++ b/arch/sparc/mm/hugetlbpage.c
@@ -31,17 +31,15 @@ static unsigned long hugetlb_get_unmapped_area_bottomup(struct file *filp,
{
struct hstate *h = hstate_file(filp);
unsigned long task_size = TASK_SIZE;
- struct vm_unmapped_area_info info;
+ struct vm_unmapped_area_info info = {};
if (test_thread_flag(TIF_32BIT))
task_size = STACK_TOP32;
- info.flags = 0;
info.length = len;
info.low_limit = TASK_UNMAPPED_BASE;
info.high_limit = min(task_size, VA_EXCLUDE_START);
info.align_mask = PAGE_MASK & ~huge_page_mask(h);
- info.align_offset = 0;
addr = vm_unmapped_area(&info);
if ((addr & ~PAGE_MASK) && task_size > VA_EXCLUDE_END) {
@@ -63,7 +61,7 @@ hugetlb_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
struct hstate *h = hstate_file(filp);
struct mm_struct *mm = current->mm;
unsigned long addr = addr0;
- struct vm_unmapped_area_info info;
+ struct vm_unmapped_area_info info = {};
/* This should only ever run for 32-bit processes. */
BUG_ON(!test_thread_flag(TIF_32BIT));
@@ -73,7 +71,6 @@ hugetlb_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
info.low_limit = PAGE_SIZE;
info.high_limit = mm->mmap_base;
info.align_mask = PAGE_MASK & ~huge_page_mask(h);
- info.align_offset = 0;
addr = vm_unmapped_area(&info);
/*
diff --git a/arch/x86/kernel/sys_x86_64.c b/arch/x86/kernel/sys_x86_64.c
index cb9fa1d5c66f..96b9d29aead0 100644
--- a/arch/x86/kernel/sys_x86_64.c
+++ b/arch/x86/kernel/sys_x86_64.c
@@ -118,7 +118,7 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
{
struct mm_struct *mm = current->mm;
struct vm_area_struct *vma;
- struct vm_unmapped_area_info info;
+ struct vm_unmapped_area_info info = {};
unsigned long begin, end;
if (flags & MAP_FIXED)
@@ -137,11 +137,9 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
return addr;
}
- info.flags = 0;
info.length = len;
info.low_limit = begin;
info.high_limit = end;
- info.align_mask = 0;
info.align_offset = pgoff << PAGE_SHIFT;
if (filp) {
info.align_mask = get_align_mask();
@@ -158,7 +156,7 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
struct vm_area_struct *vma;
struct mm_struct *mm = current->mm;
unsigned long addr = addr0;
- struct vm_unmapped_area_info info;
+ struct vm_unmapped_area_info info = {};
/* requested length too big for entire address space */
if (len > TASK_SIZE)
@@ -203,7 +201,6 @@ arch_get_unmapped_area_topdown(struct file *filp, const unsigned long addr0,
if (addr > DEFAULT_MAP_WINDOW && !in_32bit_syscall())
info.high_limit += TASK_SIZE_MAX - DEFAULT_MAP_WINDOW;
- info.align_mask = 0;
info.align_offset = pgoff << PAGE_SHIFT;
if (filp) {
info.align_mask = get_align_mask();
diff --git a/arch/x86/mm/hugetlbpage.c b/arch/x86/mm/hugetlbpage.c
index 6d77c0039617..fb600949a355 100644
--- a/arch/x86/mm/hugetlbpage.c
+++ b/arch/x86/mm/hugetlbpage.c
@@ -51,9 +51,8 @@ static unsigned long hugetlb_get_unmapped_area_bottomup(struct file *file,
unsigned long pgoff, unsigned long flags)
{
struct hstate *h = hstate_file(file);
- struct vm_unmapped_area_info info;
+ struct vm_unmapped_area_info info = {};
- info.flags = 0;
info.length = len;
info.low_limit = get_mmap_base(1);
@@ -65,7 +64,6 @@ static unsigned long hugetlb_get_unmapped_area_bottomup(struct file *file,
task_size_32bit() : task_size_64bit(addr > DEFAULT_MAP_WINDOW);
info.align_mask = PAGE_MASK & ~huge_page_mask(h);
- info.align_offset = 0;
return vm_unmapped_area(&info);
}
@@ -74,7 +72,7 @@ static unsigned long hugetlb_get_unmapped_area_topdown(struct file *file,
unsigned long pgoff, unsigned long flags)
{
struct hstate *h = hstate_file(file);
- struct vm_unmapped_area_info info;
+ struct vm_unmapped_area_info info = {};
info.flags = VM_UNMAPPED_AREA_TOPDOWN;
info.length = len;
@@ -89,7 +87,6 @@ static unsigned long hugetlb_get_unmapped_area_topdown(struct file *file,
info.high_limit += TASK_SIZE_MAX - DEFAULT_MAP_WINDOW;
info.align_mask = PAGE_MASK & ~huge_page_mask(h);
- info.align_offset = 0;
addr = vm_unmapped_area(&info);
/*
diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index 3dee18bf47ed..2f4e88552d3f 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -176,14 +176,12 @@ hugetlb_get_unmapped_area_bottomup(struct file *file, unsigned long addr,
unsigned long len, unsigned long pgoff, unsigned long flags)
{
struct hstate *h = hstate_file(file);
- struct vm_unmapped_area_info info;
+ struct vm_unmapped_area_info info = {};
- info.flags = 0;
info.length = len;
info.low_limit = current->mm->mmap_base;
info.high_limit = arch_get_mmap_end(addr, len, flags);
info.align_mask = PAGE_MASK & ~huge_page_mask(h);
- info.align_offset = 0;
return vm_unmapped_area(&info);
}
@@ -192,14 +190,13 @@ hugetlb_get_unmapped_area_topdown(struct file *file, unsigned long addr,
unsigned long len, unsigned long pgoff, unsigned long flags)
{
struct hstate *h = hstate_file(file);
- struct vm_unmapped_area_info info;
+ struct vm_unmapped_area_info info = {};
info.flags = VM_UNMAPPED_AREA_TOPDOWN;
info.length = len;
info.low_limit = PAGE_SIZE;
info.high_limit = arch_get_mmap_base(addr, current->mm->mmap_base);
info.align_mask = PAGE_MASK & ~huge_page_mask(h);
- info.align_offset = 0;
addr = vm_unmapped_area(&info);
/*
diff --git a/mm/mmap.c b/mm/mmap.c
index f734e4fa6d94..609c087bba8e 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1705,7 +1705,7 @@ generic_get_unmapped_area(struct file *filp, unsigned long addr,
{
struct mm_struct *mm = current->mm;
struct vm_area_struct *vma, *prev;
- struct vm_unmapped_area_info info;
+ struct vm_unmapped_area_info info = {};
const unsigned long mmap_end = arch_get_mmap_end(addr, len, flags);
if (len > mmap_end - mmap_min_addr)
@@ -1723,12 +1723,9 @@ generic_get_unmapped_area(struct file *filp, unsigned long addr,
return addr;
}
- info.flags = 0;
info.length = len;
info.low_limit = mm->mmap_base;
info.high_limit = mmap_end;
- info.align_mask = 0;
- info.align_offset = 0;
return vm_unmapped_area(&info);
}
@@ -1753,7 +1750,7 @@ generic_get_unmapped_area_topdown(struct file *filp, unsigned long addr,
{
struct vm_area_struct *vma, *prev;
struct mm_struct *mm = current->mm;
- struct vm_unmapped_area_info info;
+ struct vm_unmapped_area_info info = {};
const unsigned long mmap_end = arch_get_mmap_end(addr, len, flags);
/* requested length too big for entire address space */
@@ -1777,8 +1774,6 @@ generic_get_unmapped_area_topdown(struct file *filp, unsigned long addr,
info.length = len;
info.low_limit = PAGE_SIZE;
info.high_limit = arch_get_mmap_base(addr, mm->mmap_base);
- info.align_mask = 0;
- info.align_offset = 0;
addr = vm_unmapped_area(&info);
/*
--
2.34.1
_______________________________________________
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 2/2] soc: mediatek: pm-domains: support smi clamp protection
From: Yu-chang Lee (李禹璋) @ 2024-03-26 2:00 UTC (permalink / raw)
To: ulf.hansson@linaro.org, matthias.bgg@gmail.com,
angelogioacchino.delregno@collabora.com
Cc: linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
linux-pm@vger.kernel.org, MandyJH Liu (劉人僖),
Project_Global_Chrome_Upstream_Group,
Xiufeng Li (李秀峰),
linux-arm-kernel@lists.infradead.org, Fan Chen (陳凡)
In-Reply-To: <a6f54fdf-f0a9-4edc-9054-50d5204a6898@collabora.com>
On Mon, 2024-03-25 at 14:05 +0100, AngeloGioacchino Del Regno wrote:
> Il 25/03/24 13:19, yu-chang.lee ha scritto:
> > In order to avoid power glitch, this patch use smi clamp
> > to disable/enable smi common port.
> >
> > Signed-off-by: yu-chang.lee <yu-chang.lee@mediatek.com>
> > ---
> > drivers/pmdomain/mediatek/mt8188-pm-domains.h | 41 ++++-
> > drivers/pmdomain/mediatek/mtk-pm-domains.c | 147
> > ++++++++++++++----
> > drivers/pmdomain/mediatek/mtk-pm-domains.h | 1 +
> > 3 files changed, 156 insertions(+), 33 deletions(-)
> >
> > diff --git a/drivers/pmdomain/mediatek/mt8188-pm-domains.h
> > b/drivers/pmdomain/mediatek/mt8188-pm-domains.h
> > index 7bbba4d56a77..39f057dca92c 100644
> > --- a/drivers/pmdomain/mediatek/mt8188-pm-domains.h
> > +++ b/drivers/pmdomain/mediatek/mt8188-pm-domains.h
> > @@ -573,6 +573,18 @@ static const struct scpsys_domain_data
> > scpsys_domain_data_mt8188[] = {
> > .pwr_sta2nd_offs = 0x170,
> > .sram_pdn_bits = BIT(8),
> > .sram_pdn_ack_bits = BIT(12),
> > + .bp_cfg = {
> > + BUS_PROT_WR(SMI,
> > + MT8188_SMI_COMMON_SMI_CLAMP_DIP_TO_
> > VDO0,
> > + MT8188_SMI_COMMON_CLAMP_EN_SET,
> > + MT8188_SMI_COMMON_CLAMP_EN_CLR,
> > + MT8188_SMI_COMMON_CLAMP_EN_STA),
> > + BUS_PROT_WR(SMI,
> > + MT8188_SMI_COMMON_SMI_CLAMP_DIP_TO_
> > VPP1,
> > + MT8188_SMI_COMMON_CLAMP_EN_SET,
> > + MT8188_SMI_COMMON_CLAMP_EN_CLR,
> > + MT8188_SMI_COMMON_CLAMP_EN_STA),
> > + },
> > .reset_smi = {
> > SMI_RESET_WR(MT8188_SMI_LARB10_RESET,
> > MT8188_SMI_LARB10_RESET_ADDR),
> > @@ -585,7 +597,7 @@ static const struct scpsys_domain_data
> > scpsys_domain_data_mt8188[] = {
> > SMI_RESET_WR(MT8188_SMI_LARB15_RESET,
> > MT8188_SMI_LARB15_RESET_ADDR),
> > },
> > - .caps = MTK_SCPD_KEEP_DEFAULT_OFF,
> > + .caps = MTK_SCPD_KEEP_DEFAULT_OFF |
> > MTK_SCPD_CLAMP_PROTECTION,
> > },
> > [MT8188_POWER_DOMAIN_IPE] = {
> > .name = "ipe",
> > @@ -595,11 +607,18 @@ static const struct scpsys_domain_data
> > scpsys_domain_data_mt8188[] = {
> > .pwr_sta2nd_offs = 0x170,
> > .sram_pdn_bits = BIT(8),
> > .sram_pdn_ack_bits = BIT(12),
> > + .bp_cfg = {
> > + BUS_PROT_WR(SMI,
> > + MT8188_SMI_COMMON_SMI_CLAMP_IPE_TO_
> > VPP1,
> > + MT8188_SMI_COMMON_CLAMP_EN_SET,
> > + MT8188_SMI_COMMON_CLAMP_EN_CLR,
> > + MT8188_SMI_COMMON_CLAMP_EN_STA),
> > + },
> > .reset_smi = {
> > SMI_RESET_WR(MT8188_SMI_LARB12_RESET,
> > MT8188_SMI_LARB12_RESET_ADDR),
> > },
> > - .caps = MTK_SCPD_KEEP_DEFAULT_OFF,
> > + .caps = MTK_SCPD_KEEP_DEFAULT_OFF |
> > MTK_SCPD_CLAMP_PROTECTION,
> > },
> > [MT8188_POWER_DOMAIN_CAM_VCORE] = {
> > .name = "cam_vcore",
> > @@ -676,13 +695,20 @@ static const struct scpsys_domain_data
> > scpsys_domain_data_mt8188[] = {
> > .pwr_sta2nd_offs = 0x170,
> > .sram_pdn_bits = BIT(8),
> > .sram_pdn_ack_bits = BIT(12),
> > + .bp_cfg = {
> > + BUS_PROT_WR(SMI,
> > + MT8188_SMI_COMMON_SMI_CLAMP_IPE_TO_
> > VPP1,
> > + MT8188_SMI_COMMON_CLAMP_EN_SET,
> > + MT8188_SMI_COMMON_CLAMP_EN_CLR,
> > + MT8188_SMI_COMMON_CLAMP_EN_STA),
> > + },
> > .reset_smi = {
> > SMI_RESET_WR(MT8188_SMI_LARB16A_RESET,
> > MT8188_SMI_LARB16A_RESET_ADDR),
> > SMI_RESET_WR(MT8188_SMI_LARB17A_RESET,
> > MT8188_SMI_LARB17A_RESET_ADDR),
> > },
> > - .caps = MTK_SCPD_KEEP_DEFAULT_OFF,
> > + .caps = MTK_SCPD_KEEP_DEFAULT_OFF |
> > MTK_SCPD_CLAMP_PROTECTION,
> > },
> > [MT8188_POWER_DOMAIN_CAM_SUBB] = {
> > .name = "cam_subb",
> > @@ -692,13 +718,20 @@ static const struct scpsys_domain_data
> > scpsys_domain_data_mt8188[] = {
> > .pwr_sta2nd_offs = 0x170,
> > .sram_pdn_bits = BIT(8),
> > .sram_pdn_ack_bits = BIT(12),
> > + .bp_cfg = {
> > + BUS_PROT_WR(SMI,
> > + MT8188_SMI_COMMON_SMI_CLAMP_CAM_SUB
> > B_TO_VDO0,
> > + MT8188_SMI_COMMON_CLAMP_EN_SET,
> > + MT8188_SMI_COMMON_CLAMP_EN_CLR,
> > + MT8188_SMI_COMMON_CLAMP_EN_STA),
> > + },
> > .reset_smi = {
> > SMI_RESET_WR(MT8188_SMI_LARB16B_RESET,
> > MT8188_SMI_LARB16B_RESET_ADDR),
> > SMI_RESET_WR(MT8188_SMI_LARB17B_RESET,
> > MT8188_SMI_LARB17B_RESET_ADDR),
> > },
> > - .caps = MTK_SCPD_KEEP_DEFAULT_OFF,
> > + .caps = MTK_SCPD_KEEP_DEFAULT_OFF |
> > MTK_SCPD_CLAMP_PROTECTION,
> > },
> > };
> >
> > diff --git a/drivers/pmdomain/mediatek/mtk-pm-domains.c
> > b/drivers/pmdomain/mediatek/mtk-pm-domains.c
> > index 9ab6fa105c8c..3c797e136c0e 100644
> > --- a/drivers/pmdomain/mediatek/mtk-pm-domains.c
> > +++ b/drivers/pmdomain/mediatek/mtk-pm-domains.c
> > @@ -47,9 +47,10 @@ struct scpsys_domain {
> > struct clk_bulk_data *subsys_clks;
> > struct regmap *infracfg_nao;
> > struct regmap *infracfg;
> > - struct regmap *smi;
> > + struct regmap **smi;
> > struct regmap **larb;
> > int num_larb;
> > + int num_smi;
> > struct regulator *supply;
> > };
> >
> > @@ -122,29 +123,19 @@ static int scpsys_sram_disable(struct
> > scpsys_domain *pd)
> > MTK_POLL_TIMEOUT);
> > }
> >
> > -static struct regmap *scpsys_bus_protect_get_regmap(struct
> > scpsys_domain *pd,
> > - const struct
> > scpsys_bus_prot_data *bpd)
> > -{
> > - if (bpd->flags & BUS_PROT_COMPONENT_SMI)
> > - return pd->smi;
> > - else
> > - return pd->infracfg;
> > -}
> > -
> > static struct regmap *scpsys_bus_protect_get_sta_regmap(struct
> > scpsys_domain *pd,
> > const struct
> > scpsys_bus_prot_data *bpd)
> > {
> > if (bpd->flags & BUS_PROT_STA_COMPONENT_INFRA_NAO)
> > return pd->infracfg_nao;
> > else
> > - return scpsys_bus_protect_get_regmap(pd, bpd);
> > + return pd->infracfg;
> > }
> >
> > static int scpsys_bus_protect_clear(struct scpsys_domain *pd,
> > - const struct scpsys_bus_prot_data
> > *bpd)
> > + const struct scpsys_bus_prot_data
> > *bpd,
> > + struct regmap *sta_regmap,
> > struct regmap *regmap)
> > {
> > - struct regmap *sta_regmap =
> > scpsys_bus_protect_get_sta_regmap(pd, bpd);
> > - struct regmap *regmap = scpsys_bus_protect_get_regmap(pd, bpd);
> > u32 sta_mask = bpd->bus_prot_sta_mask;
> > u32 expected_ack;
> > u32 val;
> > @@ -165,10 +156,9 @@ static int scpsys_bus_protect_clear(struct
> > scpsys_domain *pd,
> > }
> >
> > static int scpsys_bus_protect_set(struct scpsys_domain *pd,
> > - const struct scpsys_bus_prot_data
> > *bpd)
> > + const struct scpsys_bus_prot_data
> > *bpd,
> > + struct regmap *sta_regmap, struct
> > regmap *regmap)
> > {
> > - struct regmap *sta_regmap =
> > scpsys_bus_protect_get_sta_regmap(pd, bpd);
> > - struct regmap *regmap = scpsys_bus_protect_get_regmap(pd, bpd);
> > u32 sta_mask = bpd->bus_prot_sta_mask;
> > u32 val;
> >
> > @@ -182,19 +172,32 @@ static int scpsys_bus_protect_set(struct
> > scpsys_domain *pd,
> > MTK_POLL_DELAY_US,
> > MTK_POLL_TIMEOUT);
> > }
> >
> > -static int scpsys_bus_protect_enable(struct scpsys_domain *pd)
> > +static int _scpsys_clamp_bus_protection_enable(struct
> > scpsys_domain *pd, bool is_smi)
> > {
> > + int smi_count = 0;
> > +
> > for (int i = 0; i < SPM_MAX_BUS_PROT_DATA; i++) {
> > const struct scpsys_bus_prot_data *bpd = &pd->data-
> > >bp_cfg[i];
> > + struct regmap *sta_regmap, *regmap;
> > + bool is_smi = bpd->flags & BUS_PROT_COMPONENT_SMI;
> > int ret;
> >
> > if (!bpd->bus_prot_set_clr_mask)
> > break;
> >
> > + if (is_smi) {
> > + sta_regmap = pd->smi[smi_count];
> > + regmap = pd->smi[smi_count];
> > + smi_count++;
> > + } else {
> > + sta_regmap =
> > scpsys_bus_protect_get_sta_regmap(pd, bpd);
> > + regmap = pd->infracfg;
> > + }
> > +
> > if (bpd->flags & BUS_PROT_INVERTED)
> > - ret = scpsys_bus_protect_clear(pd, bpd);
> > + ret = scpsys_bus_protect_clear(pd, bpd,
> > sta_regmap, regmap);
> > else
> > - ret = scpsys_bus_protect_set(pd, bpd);
> > + ret = scpsys_bus_protect_set(pd, bpd,
> > sta_regmap, regmap);
> > if (ret)
> > return ret;
> > }
> > @@ -202,19 +205,32 @@ static int scpsys_bus_protect_enable(struct
> > scpsys_domain *pd)
> > return 0;
> > }
> >
> > -static int scpsys_bus_protect_disable(struct scpsys_domain *pd)
> > +static int _scpsys_clamp_bus_protection_disable(struct
> > scpsys_domain *pd, bool is_smi)
> > {
> > + int smi_count = pd->num_smi - 1;
> > +
> > for (int i = SPM_MAX_BUS_PROT_DATA - 1; i >= 0; i--) {
> > const struct scpsys_bus_prot_data *bpd = &pd->data-
> > >bp_cfg[i];
> > + struct regmap *sta_regmap, *regmap;
> > + bool is_smi = bpd->flags & BUS_PROT_COMPONENT_SMI;
> > int ret;
> >
> > if (!bpd->bus_prot_set_clr_mask)
> > continue;
> >
> > + if (is_smi) {
> > + sta_regmap = pd->smi[smi_count];
> > + regmap = pd->smi[smi_count];
> > + smi_count--;
> > + } else {
> > + sta_regmap =
> > scpsys_bus_protect_get_sta_regmap(pd, bpd);
> > + regmap = pd->infracfg;
> > + }
> > +
> > if (bpd->flags & BUS_PROT_INVERTED)
> > - ret = scpsys_bus_protect_set(pd, bpd);
> > + ret = scpsys_bus_protect_set(pd, bpd,
> > sta_regmap, regmap);
> > else
> > - ret = scpsys_bus_protect_clear(pd, bpd);
> > + ret = scpsys_bus_protect_clear(pd, bpd,
> > sta_regmap, regmap);
> > if (ret)
> > return ret;
> > }
> > @@ -222,6 +238,50 @@ static int scpsys_bus_protect_disable(struct
> > scpsys_domain *pd)
> > return 0;
> > }
> >
> > +static int scpsys_clamp_protection(struct scpsys_domain *pd)
> > +{
> > + int ret;
> > +
>
> You can directly call _scpsys_clamp_bus_protection_enable(), no need
> for a helper.
>
> > + ret = _scpsys_clamp_bus_protection_enable(pd, true);
> > + if (ret)
> > + return ret;
> > +
> > + return 0;
> > +}
> > +
> > +static int scpsys_clamp_protection_disable(struct scpsys_domain
> > *pd)
> > +{
> > + int ret;
> > +
> > + ret = _scpsys_clamp_bus_protection_disable(pd, true);
> > + if (ret)
> > + return ret;
> > +
> > + return 0;
> > +}
> > +
> > +static int scpsys_bus_protect_enable(struct scpsys_domain *pd)
>
> Unused function, please remove.
I think this is used in scpsys_power_off function. Do you mean I
should directly call _scpsys_clamp_bus_protection_disable?
>
> > +{
> > + int ret;
> > +
> > + ret = _scpsys_clamp_bus_protection_enable(pd, false);
> > + if (ret)
> > + return ret;
> > +
> > + return 0;
> > +}
> > +
> > +static int scpsys_bus_protect_disable(struct scpsys_domain *pd)
> > +{
>
> Unused function, please remove.
Same here, I think this is used in scpsys_power_on function.
>
> > + int ret;
> > +
> > + ret = _scpsys_clamp_bus_protection_disable(pd, false);
> > + if (ret)
> > + return ret;
> > +
> > + return 0;
> > +}
> > +
> > static int scpsys_regulator_enable(struct regulator *supply)
> > {
> > return supply ? regulator_enable(supply) : 0;
> > @@ -272,6 +332,12 @@ static int scpsys_power_on(struct
> > generic_pm_domain *genpd)
> > bool tmp;
> > int ret;
> >
> > + if (MTK_SCPD_CAPS(pd, MTK_SCPD_CLAMP_PROTECTION)) {
> > + ret = scpsys_clamp_protection(pd);
>
> ret = scpsys_clamp_bus_protection_enable(pd, true);
>
> > + if (ret)
> > + return ret;
> > + }
> > +
> > ret = scpsys_regulator_enable(pd->supply);
> > if (ret)
> > return ret;
> > @@ -318,6 +384,12 @@ static int scpsys_power_on(struct
> > generic_pm_domain *genpd)
> > if (ret < 0)
> > goto err_disable_subsys_clks;
> >
> > + if (MTK_SCPD_CAPS(pd, MTK_SCPD_CLAMP_PROTECTION)) {
> > + ret = scpsys_clamp_protection_disable(pd);
>
> ret = scpsys_clamp_bus_protection_disable(pd, true);
>
> > + if (ret)
> > + return ret;
> > + }
> > +
> > ret = scpsys_bus_protect_disable(pd);
> > if (ret < 0)
> > goto err_disable_sram;
> > @@ -353,6 +425,12 @@ static int scpsys_power_off(struct
> > generic_pm_domain *genpd)
> > bool tmp;
> > int ret;
> >
> > + if (MTK_SCPD_CAPS(pd, MTK_SCPD_CLAMP_PROTECTION)) {
> > + ret = scpsys_clamp_protection(pd);
>
> ret = scpsys_clamp_bus_protection_enable(pd, true);
>
> > + if (ret)
> > + return ret;
> > + }
> > +
> > ret = scpsys_bus_protect_enable(pd);
> > if (ret < 0)
> > return ret;
>
> Regards,
> Angelo
>
Best Regards,
yu-chang.lee
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [WIP 0/3] Memory model and atomic API in Rust
From: Dr. David Alan Gilbert @ 2024-03-26 1:35 UTC (permalink / raw)
To: Kent Overstreet
Cc: Linus Torvalds, Philipp Stanner, Boqun Feng, rust-for-linux,
linux-kernel, linux-arch, llvm, Miguel Ojeda, Alex Gaynor,
Wedson Almeida Filho, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Alan Stern,
Andrea Parri, Will Deacon, Peter Zijlstra, Nicholas Piggin,
David Howells, Jade Alglave, Luc Maranget, Paul E. McKenney,
Akira Yokosawa, Daniel Lustig, Joel Fernandes, Nathan Chancellor,
Nick Desaulniers, kent.overstreet, Greg Kroah-Hartman, elver,
Mark Rutland, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Catalin Marinas,
linux-arm-kernel, linux-fsdevel
In-Reply-To: <vevxfv67ureybf7sjwfxzdvl4tt62khyn2gfzn7o74ke2m554s@xxddzz6nurbn>
* Kent Overstreet (kent.overstreet@linux.dev) wrote:
> On Tue, Mar 26, 2024 at 12:05:48AM +0000, Dr. David Alan Gilbert wrote:
> > * Linus Torvalds (torvalds@linux-foundation.org) wrote:
> >
> > <snip>
> >
> > > IOW, the whole access size problem that Boqun described is
> > > *inherently* tied to the fact that the C++ and Rust memory model is
> > > badly designed from the wrong principles.
> > >
> > > Instead of designing it as a "this is an atomic object that you can do
> > > these operations on", it should have been "this is an atomic access,
> > > and you can use this simple object model to have the compiler generate
> > > the accesses for you".
> >
> > Isn't one of the aims of the Rust/C++ idea that you can't forget to access
> > a shared piece of data atomically?
> >
> > If you want to have 'atomic accesses' explicitly, how do you tell the compiler
> > what you can use them on, and when it should stop you mixing them with
> > normal accesses on the same object?
>
> "can't forget to access data atomically" - that's only half of it. And
> atomic accesses loads/stores are not a thing under the hood, they're
> just loads and stores (possibly, but not necessarily, with memory
> barriers).
That's quite architecturally specific isn't it?
Or is this the distinction between accesses that are implicitly atomic
(i.e. naturally aligned word) and things that are locked/exclusive?
(either with a 'lock' on x86 or load-exclusive/store exclusive on some others)?
Which are we talking about here?
> The other half is at the _source_ level you don't want to treat accesses
> to volatiles/atomics like accesses to normal variables, you really want
> those to be explicit, and not look like normal variable accesses.
>
> std:atomic_int is way better than volatile in the sense that it's not a
> barely specified mess, but adding operator overloading was just
> gratuitious and unnecessary.
>
> This is a theme with C++ - they add a _ton_ of magic to make things
> concise and pretty, but you have to understand in intimate detail what
> all that magic is doing or you're totally fucked.
>
> std::atomic_int makes it such that just changing a single line of code
> in a single location in your program will change the semantics of your
> _entire_ program and the only obserable result will be that it's faster
> but a ticking time bomb because you just introduced a ton of races.
>
> With Rust - I honestly haven't looked at whether they added operator
> overlaoding for their atomics, but it's _much_ less of a concern because
> changing the type to the non-atomic version means your program won't
> compile if it's now racy.
OK, so that's essentially the opposite worry of what I was saying; I was
worrying about people forgetting to use an atomic access to a shared
variable; I think you're worrying about people forgetting to mark
a variable shared and since the accesses are the same nothing shouts?
Dave
--
-----Open up your eyes, open up your mind, open up your code -------
/ Dr. David Alan Gilbert | Running GNU/Linux | Happy \
\ dave @ treblig.org | | In Hex /
\ _________________________|_____ http://www.treblig.org |_______/
_______________________________________________
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 2/7] iopoll: Do not use timekeeping in read_poll_timeout_atomic()
From: Zong Li @ 2024-03-26 1:31 UTC (permalink / raw)
To: geert+renesas
Cc: arnd, hca, iommu, joro, jstultz, kai.heng.feng, krzk,
linux-arm-kernel, linux-clk, linux-kernel, linux-pm,
linux-renesas-soc, linux, magnus.damm, mturquette, npiggin,
peterz, rafael.j.wysocki, robin.murphy, s.nawrocki, sboyd,
tero.kristo, tglx, tomasz.figa, tony, ulf.hansson,
vincent.guittot, will, wsa+renesas, yoshihiro.shimoda.uh,
zhengdejin5
In-Reply-To: <3d2a2f4e553489392d871108797c3be08f88300b.1685692810.git.geert+renesas@glider.be>
On Fri, Jun 02, 2023 at 10:50:37AM +0200, Geert Uytterhoeven wrote:
> read_poll_timeout_atomic() uses ktime_get() to implement the timeout
> feature, just like its non-atomic counterpart. However, there are
> several issues with this, due to its use in atomic contexts:
>
> 1. When called in the s2ram path (as typically done by clock or PM
> domain drivers), timekeeping may be suspended, triggering the
> WARN_ON(timekeeping_suspended) in ktime_get():
>
> WARNING: CPU: 0 PID: 654 at kernel/time/timekeeping.c:843 ktime_get+0x28/0x78
>
> Calling ktime_get_mono_fast_ns() instead of ktime_get() would get
> rid of that warning. However, that would break timeout handling,
> as (at least on systems with an ARM architectured timer), the time
> returned by ktime_get_mono_fast_ns() does not advance while
> timekeeping is suspended.
> Interestingly, (on the same ARM systems) the time returned by
> ktime_get() does advance while timekeeping is suspended, despite
> the warning.
>
> 2. Depending on the actual clock source, and especially before a
> high-resolution clocksource (e.g. the ARM architectured timer)
> becomes available, time may not advance in atomic contexts, thus
> breaking timeout handling.
>
> Fix this by abandoning the idea that one can rely on timekeeping to
> implement timeout handling in all atomic contexts, and switch from a
> global time-based to a locally-estimated timeout handling. In most
> (all?) cases the timeout condition is exceptional and an error
> condition, hence any additional delays due to underestimating wall clock
> time are irrelevant.
>
Hi Geert,
I tested this patch on the FPGA, and I noticed the timeout duration
was much longer than expected. I tested it by removing the op operation
and break condition for avoiding the influence of other factors.
The code would look like as follows:
for (;;) {
if (__timeout_us && __left_ns < 0)
break;
if (__delay_us) {
udelay(__delay_us);
if (__timeout_us)
__left_ns -= __delay_ns;;
cpu_relex();
if (__timeout_us)
__left_ns--;
}
}
Despite setting the timeout to 1 second, it actually takes 25 seconds
to reach the specified timeout value. I displayed the value of
__left_ns when a timeout occurred. As follows: __delay_us is 1, when
__left_ns counts down to -1, the system has run for 25 seconds.
[ 26.016213] __timeout_us: 1000000 __left_ns: -1
[ 50.818585] __timeout_us: 1000000 __left_ns: -1
[ 75.620467] __timeout_us: 1000000 __left_ns: -1
[ 100.422664] __timeout_us: 1000000 __left_ns: -1
[ 125.224775] __timeout_us: 1000000 __left_ns: -1
...
I attempted to blend the two versions (e.g., ktime version and the
current version) for discarding the value of __left_ns. The resulting
output is as follows: __delay_us is 1, when it exceeds 1 second
according to ktime, __left_ns only counts around 40 ms.
[ 6.734482] __timeout_us: 1000000 __left_ns: 961699000
[ 7.738485] __timeout_us: 1000000 __left_ns: 961228000
[ 8.812797] __timeout_us: 1000000 __left_ns: 961755000
[ 9.814021] __timeout_us: 1000000 __left_ns: 961542000
[ 10.815373] __timeout_us: 1000000 __left_ns: 962464000
[ 11.816184] __timeout_us: 1000000 __left_ns: 961536000
[ 12.817137] __timeout_us: 1000000 __left_ns: 961121000
...
Per your suggestion, I attempted to increase delay_us to 10 us,
it really helps to eliminate the underestimation. The actual
timeout became 3 secs on the FPGA.
I moved on my host x86 machine, the timeout has been reduced to
2 seconds even if the delay_us is 1. And the timeout can be
precise 1 seconds when delay_us is 10. I'm not sure if the clock
frequency or RTC frequency might also determine the underestimation
of wall clock time? Is there a suggested value of delay_us for a
driver that runs on various platforms?
What is your perspective for those situation?
Thanks.
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Reviewed-by: Tony Lindgren <tony@atomide.com>
> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> The first issue was seen with the rcar-sysc driver in the BSP, as the
> BSP contains modifications to the resume sequence of various PM Domains.
>
> v3:
> - Add Acked-by, Reviewed-by,
> - Add comment about not using timekeeping, and its impact,
>
> v2:
> - New.
> ---
> include/linux/iopoll.h | 22 +++++++++++++++++-----
> 1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/iopoll.h b/include/linux/iopoll.h
> index 0417360a6db9b0d6..19a7b00baff43595 100644
> --- a/include/linux/iopoll.h
> +++ b/include/linux/iopoll.h
> @@ -74,6 +74,10 @@
> * Returns 0 on success and -ETIMEDOUT upon a timeout. In either
> * case, the last read value at @args is stored in @val.
> *
> + * This macro does not rely on timekeeping. Hence it is safe to call even when
> + * timekeeping is suspended, at the expense of an underestimation of wall clock
> + * time, which is rather minimal with a non-zero delay_us.
> + *
> * When available, you'll probably want to use one of the specialized
> * macros defined below rather than this macro directly.
> */
> @@ -81,22 +85,30 @@
> delay_before_read, args...) \
> ({ \
> u64 __timeout_us = (timeout_us); \
> + s64 __left_ns = __timeout_us * NSEC_PER_USEC; \
> unsigned long __delay_us = (delay_us); \
> - ktime_t __timeout = ktime_add_us(ktime_get(), __timeout_us); \
> - if (delay_before_read && __delay_us) \
> + u64 __delay_ns = __delay_us * NSEC_PER_USEC; \
> + if (delay_before_read && __delay_us) { \
> udelay(__delay_us); \
> + if (__timeout_us) \
> + __left_ns -= __delay_ns; \
> + } \
> for (;;) { \
> (val) = op(args); \
> if (cond) \
> break; \
> - if (__timeout_us && \
> - ktime_compare(ktime_get(), __timeout) > 0) { \
> + if (__timeout_us && __left_ns < 0) { \
> (val) = op(args); \
> break; \
> } \
> - if (__delay_us) \
> + if (__delay_us) { \
> udelay(__delay_us); \
> + if (__timeout_us) \
> + __left_ns -= __delay_ns; \
> + } \
> cpu_relax(); \
> + if (__timeout_us) \
> + __left_ns--; \
> } \
> (cond) ? 0 : -ETIMEDOUT; \
> })
> --
> 2.34.1
>
_______________________________________________
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 3/3] iommu/arm-smmu-v3: support suspend/resume
From: Peng Fan @ 2024-03-26 1:26 UTC (permalink / raw)
To: Thomas Gleixner, Jason Gunthorpe, Peng Fan (OSS)
Cc: Will Deacon, Robin Murphy, Joerg Roedel, Marc Zyngier, Bixuan Cui,
linux-arm-kernel@lists.infradead.org, iommu@lists.linux.dev,
linux-kernel@vger.kernel.org
In-Reply-To: <87bk72fcf4.ffs@tglx>
> Subject: Re: [PATCH 3/3] iommu/arm-smmu-v3: support suspend/resume
>
> On Mon, Mar 25 2024 at 10:48, Jason Gunthorpe wrote:
> > On Sun, Mar 24, 2024 at 08:29:00PM +0800, Peng Fan (OSS) wrote:
> >> +static void arm_smmu_resume_unique_irqs(struct arm_smmu_device
> >> +*smmu) {
> >> + struct device *dev = smmu->dev;
> >> + struct msi_desc *desc;
> >> + struct msi_msg msg;
> >> +
> >> + if (!dev->msi.domain)
> >> + return;
> >> +
> >> + desc = irq_get_msi_desc(smmu->evtq.q.irq);
> >> + if (desc) {
> >> + get_cached_msi_msg(smmu->evtq.q.irq, &msg);
> >> + arm_smmu_write_msi_msg(desc, &msg);
> >> + }
> >> +
> >> + desc = irq_get_msi_desc(smmu->gerr_irq);
> >> + if (desc) {
> >> + get_cached_msi_msg(smmu->gerr_irq, &msg);
> >> + arm_smmu_write_msi_msg(desc, &msg);
> >> + }
> >> +
> >> + if (smmu->features & ARM_SMMU_FEAT_PRI) {
> >> + desc = irq_get_msi_desc(smmu->priq.q.irq);
> >> + if (desc) {
> >> + get_cached_msi_msg(smmu->priq.q.irq, &msg);
> >> + arm_smmu_write_msi_msg(desc, &msg);
> >> + }
> >> + }
> >> +}
> >
> > I wonder if this should be done instead by converting the driver away
> > from platform MSI to the new MSI mechanism?
>
> There is work in progress for that. Should come around in the next weeks.
Then I need to wait for your patches, and rebase this patchset, or could
the non-msi part be reviewed first?
Thanks,
Peng.
_______________________________________________
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 05/14] drm: Suppress intentional warning backtraces in scaling unit tests
From: Maíra Canal @ 2024-03-26 1:09 UTC (permalink / raw)
To: Guenter Roeck
Cc: linux-kselftest, David Airlie, Arnd Bergmann, 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: <e880828b-552e-488e-9f31-3989bec276ae@roeck-us.net>
On 3/25/24 16:24, Guenter Roeck wrote:
> Hi,
>
> On Mon, Mar 25, 2024 at 04:05:06PM -0300, Maíra Canal wrote:
>> Hi Guenter,
>>
>> On 3/25/24 14:52, Guenter Roeck wrote:
>>> The drm_test_rect_calc_hscale and drm_test_rect_calc_vscale unit tests
>>> intentionally trigger warning backtraces by providing bad parameters to
>>> the tested functions. What is tested is the return value, not the existence
>>> of a warning backtrace. Suppress the backtraces to avoid clogging the
>>> kernel log.
>>>
>>> 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
>>>
>>> drivers/gpu/drm/tests/drm_rect_test.c | 6 ++++++
>>> 1 file changed, 6 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/tests/drm_rect_test.c b/drivers/gpu/drm/tests/drm_rect_test.c
>>> index 76332cd2ead8..75614cb4deb5 100644
>>> --- a/drivers/gpu/drm/tests/drm_rect_test.c
>>> +++ b/drivers/gpu/drm/tests/drm_rect_test.c
>>> @@ -406,22 +406,28 @@ KUNIT_ARRAY_PARAM(drm_rect_scale, drm_rect_scale_cases, drm_rect_scale_case_desc
>>> static void drm_test_rect_calc_hscale(struct kunit *test)
>>> {
>>> + DEFINE_SUPPRESSED_WARNING(drm_calc_scale);
>>> const struct drm_rect_scale_case *params = test->param_value;
>>> int scaling_factor;
>>> + START_SUPPRESSED_WARNING(drm_calc_scale);
>>
>> I'm not sure if it is not that obvious only to me, but it would be nice
>> to have a comment here, remembering that we provide bad parameters in
>> some test cases.
>
> Sure. Something like this ?
>
> /*
> * drm_rect_calc_hscale() generates a warning backtrace whenever bad
> * parameters are passed to it. This affects all unit tests with an
> * error code in expected_scaling_factor.
> */
>
Yeah, perfect. With that, feel free to add my
Acked-by: Maíra Canal <mcanal@igalia.com>
Best Regards,
- Maíra
> Thanks,
> Guenter
_______________________________________________
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 2/3] genirq/msi: cache the last msi msg
From: Peng Fan @ 2024-03-26 1:02 UTC (permalink / raw)
To: Thomas Gleixner, Peng Fan (OSS), Will Deacon, Robin Murphy,
Joerg Roedel, Marc Zyngier
Cc: Bixuan Cui, linux-arm-kernel@lists.infradead.org,
iommu@lists.linux.dev, linux-kernel@vger.kernel.org
In-Reply-To: <878r26fbv9.ffs@tglx>
> Subject: Re: [PATCH 2/3] genirq/msi: cache the last msi msg
>
> On Sun, Mar 24 2024 at 20:28, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > Cache the last msi msg which will be used for ARM SMMU V3 resume
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> > kernel/irq/msi.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c index
> > f90952ebc494..9d7e0a675089 100644
> > --- a/kernel/irq/msi.c
> > +++ b/kernel/irq/msi.c
> > @@ -617,6 +617,9 @@ static unsigned int msi_domain_get_hwsize(struct
> > device *dev, unsigned int domid static inline void
> irq_chip_write_msi_msg(struct irq_data *data,
> > struct msi_msg *msg)
> > {
> > + struct msi_desc *desc = irq_data_get_msi_desc(data);
> > +
> > + desc->msg = *msg;
> > data->chip->irq_write_msi_msg(data, msg); }
>
> This should remove the very same line in the PCI/MSI code, no?
I think no, but you have more knowledge than me, please correct if I am
wrong.
To zpci_irq_chip.irq_set_affinity
zpci_set_irq_affinity->pci_write_msi_msg->cache the msg
In kernel/irq/msi.c
msi_domain_update_chip_ops
|-> if (!chip->irq_set_affinity)
chip->irq_set_affinity = msi_domain_set_affinity
From above code, the pci/msi code should keep the caching code.
Thanks,
Peng.
>
> Thanks,
>
> tglx
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 2/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add gpio child node
From: Laurent Pinchart @ 2024-03-26 0:49 UTC (permalink / raw)
To: devicetree, linux-rpi-kernel, linux-arm-kernel, linux-gpio
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
Broadcom internal kernel review list, Ray Jui, Scott Branden,
Linus Walleij, Bartosz Golaszewski, Eric Anholt, Stefan Wahren
In-Reply-To: <20240326004902.17054-1-laurent.pinchart@ideasonboard.com>
Unlike the other child nodes of the raspberrypi,bcm2835-firmware device,
the gpio child is documented in a legacy text-based binding in
gpio/raspberrypi,firmware-gpio.txt. This causes DT validation failures:
arch/arm64/boot/dts/broadcom/bcm2711-rpi-4-b.dtb: 'gpio' does not match any of the regexes: 'pinctrl-[0-9]+'
from schema $id: http://devicetree.org/schemas/arm/bcm/raspberrypi,bcm2835-firmware.yaml#
Convert the binding to YAML and move it to
raspberrypi,bcm2835-firmware.yaml.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
.../arm/bcm/raspberrypi,bcm2835-firmware.yaml | 29 ++++++++++++++++++
.../gpio/raspberrypi,firmware-gpio.txt | 30 -------------------
2 files changed, 29 insertions(+), 30 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/gpio/raspberrypi,firmware-gpio.txt
diff --git a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
index dc38f2be7ad6..999e1bc49539 100644
--- a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
+++ b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
@@ -54,6 +54,29 @@ properties:
- compatible
- "#clock-cells"
+ gpio:
+ type: object
+ additionalProperties: false
+
+ properties:
+ compatible:
+ const: raspberrypi,firmware-gpio
+
+ gpio-controller: true
+
+ "#gpio-cells":
+ const: 2
+ description:
+ The first cell is the pin number, and the second cell is used to
+ specify the gpio polarity (GPIO_ACTIVE_HIGH or GPIO_ACTIVE_LOW).
+
+ gpio-line-names: true
+
+ required:
+ - compatible
+ - gpio-controller
+ - "#gpio-cells"
+
reset:
type: object
additionalProperties: false
@@ -112,6 +135,12 @@ examples:
#clock-cells = <1>;
};
+ expgpio: gpio {
+ compatible = "raspberrypi,firmware-gpio";
+ gpio-controller;
+ #gpio-cells = <2>;
+ };
+
reset: reset {
compatible = "raspberrypi,firmware-reset";
#reset-cells = <1>;
diff --git a/Documentation/devicetree/bindings/gpio/raspberrypi,firmware-gpio.txt b/Documentation/devicetree/bindings/gpio/raspberrypi,firmware-gpio.txt
deleted file mode 100644
index ce97265e23ba..000000000000
--- a/Documentation/devicetree/bindings/gpio/raspberrypi,firmware-gpio.txt
+++ /dev/null
@@ -1,30 +0,0 @@
-Raspberry Pi GPIO expander
-
-The Raspberry Pi 3 GPIO expander is controlled by the VC4 firmware. The
-firmware exposes a mailbox interface that allows the ARM core to control the
-GPIO lines on the expander.
-
-The Raspberry Pi GPIO expander node must be a child node of the Raspberry Pi
-firmware node.
-
-Required properties:
-
-- compatible : Should be "raspberrypi,firmware-gpio"
-- gpio-controller : Marks the device node as a gpio controller
-- #gpio-cells : Should be two. The first cell is the pin number, and
- the second cell is used to specify the gpio polarity:
- 0 = active high
- 1 = active low
-
-Example:
-
-firmware: firmware-rpi {
- compatible = "raspberrypi,bcm2835-firmware";
- mboxes = <&mailbox>;
-
- expgpio: gpio {
- compatible = "raspberrypi,firmware-gpio";
- gpio-controller;
- #gpio-cells = <2>;
- };
-};
--
Regards,
Laurent Pinchart
_______________________________________________
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 1/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add missing properties
From: Laurent Pinchart @ 2024-03-26 0:49 UTC (permalink / raw)
To: devicetree, linux-rpi-kernel, linux-arm-kernel, linux-gpio
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
Broadcom internal kernel review list, Ray Jui, Scott Branden,
Linus Walleij, Bartosz Golaszewski, Eric Anholt, Stefan Wahren
In-Reply-To: <20240326004902.17054-1-laurent.pinchart@ideasonboard.com>
The raspberrypi,bcm2835-firmware devices requires a dma-ranges property,
and, as a result, also needs to specify #address-cells and #size-cells.
Those properties have been added to thebcm2835-rpi.dtsi in commits
be08d278eb09 ("ARM: dts: bcm283x: Add cells encoding format to firmware
bus") and 55c7c0621078 ("ARM: dts: bcm283x: Fix vc4's firmware bus DMA
limitations"), but the DT bindings haven't been updated, resulting in
validation errors:
arch/arm64/boot/dts/broadcom/bcm2711-rpi-4-b.dtb: firmware: '#address-cells', '#size-cells', 'dma-ranges', 'gpio' do not match any of the regexes: 'pinctrl-[0-9]+'
from schema $id: http://devicetree.org/schemas/arm/bcm/raspberrypi,bcm2835-firmware.yaml#
Fix this by adding the properties to the bindings.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
.../arm/bcm/raspberrypi,bcm2835-firmware.yaml | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
index 39e3c248f5b7..dc38f2be7ad6 100644
--- a/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
+++ b/Documentation/devicetree/bindings/arm/bcm/raspberrypi,bcm2835-firmware.yaml
@@ -25,6 +25,14 @@ properties:
- const: raspberrypi,bcm2835-firmware
- const: simple-mfd
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 1
+
+ dma-ranges: true
+
mboxes:
maxItems: 1
@@ -81,6 +89,9 @@ properties:
required:
- compatible
+ - "#address-cells"
+ - "#size-cells"
+ - dma-ranges
- mboxes
additionalProperties: false
@@ -89,6 +100,11 @@ examples:
- |
firmware {
compatible = "raspberrypi,bcm2835-firmware", "simple-mfd";
+
+ #address-cells = <1>;
+ #size-cells = <1>;
+ dma-ranges;
+
mboxes = <&mailbox>;
firmware_clocks: clocks {
--
Regards,
Laurent Pinchart
_______________________________________________
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 0/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Drive-by fixes
From: Laurent Pinchart @ 2024-03-26 0:49 UTC (permalink / raw)
To: devicetree, linux-rpi-kernel, linux-arm-kernel, linux-gpio
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
Broadcom internal kernel review list, Ray Jui, Scott Branden,
Linus Walleij, Bartosz Golaszewski, Eric Anholt, Stefan Wahren
Hello,
This small series includes two drive-by fixes to the
raspberrypi,bcm2835-firmware DT bindings that fix validation errors with
the Raspberry Pi 4 device tree sources. I noticed those issues when
working on the Raspberry Pi Unicam driver, but two patches are
independent of that work, they can thus be applied separately.
Laurent Pinchart (2):
dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add missing
properties
dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add gpio child
node
.../arm/bcm/raspberrypi,bcm2835-firmware.yaml | 45 +++++++++++++++++++
.../gpio/raspberrypi,firmware-gpio.txt | 30 -------------
2 files changed, 45 insertions(+), 30 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/gpio/raspberrypi,firmware-gpio.txt
--
Regards,
Laurent Pinchart
_______________________________________________
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 2/3] iio: adc: meson: consistently use bool/enum in struct meson_sar_adc_param
From: George Stark @ 2024-03-26 0:41 UTC (permalink / raw)
To: Martin Blumenstingl
Cc: linux-arm-kernel, linux-kernel, neil.armstrong, lars, jic23,
linux-amlogic, kernel@salutedevices.com
In-Reply-To: <20240323231309.415425-3-martin.blumenstingl@googlemail.com>
Hello Martin
On 3/24/24 02:13, Martin Blumenstingl wrote:
> Consistently use bool for any register bit that enables/disables
> functionality and enum for register values where there's a choice
> between different settings. The aim is to make the code easier to read
> and understand by being more consistent. No functional changes intended.
>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
> drivers/iio/adc/meson_saradc.c | 47 +++++++++++++++++++---------------
> 1 file changed, 27 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> index 2615d74534df..6b2af0c2bbc7 100644
> --- a/drivers/iio/adc/meson_saradc.c
> +++ b/drivers/iio/adc/meson_saradc.c
> @@ -156,9 +156,9 @@
> #define MESON_SAR_ADC_REG11 0x2c
> #define MESON_SAR_ADC_REG11_BANDGAP_EN BIT(13)
> #define MESON_SAR_ADC_REG11_CMV_SEL BIT(6)
> - #define MESON_SAR_ADC_REG11_VREF_VOLTAGE BIT(5)
> - #define MESON_SAR_ADC_REG11_EOC BIT(1)
> - #define MESON_SAR_ADC_REG11_VREF_SEL BIT(0)
> + #define MESON_SAR_ADC_REG11_VREF_VOLTAGE BIT(5)
> + #define MESON_SAR_ADC_REG11_EOC BIT(1)
> + #define MESON_SAR_ADC_REG11_VREF_SEL BIT(0)
>
> #define MESON_SAR_ADC_REG13 0x34
> #define MESON_SAR_ADC_REG13_12BIT_CALIBRATION_MASK GENMASK(13, 8)
> @@ -224,6 +224,11 @@ enum meson_sar_adc_vref_sel {
> VREF_VDDA = 1,
> };
>
> +enum meson_sar_adc_vref_voltage {
> + VREF_VOLTAGE_0V9 = 0,
> + VREF_VOLTAGE_1V8 = 1,
> +};
> +
> enum meson_sar_adc_avg_mode {
> NO_AVERAGING = 0x0,
> MEAN_AVERAGING = 0x1,
> @@ -321,13 +326,13 @@ struct meson_sar_adc_param {
> u8 temperature_trimming_bits;
> unsigned int temperature_multiplier;
> unsigned int temperature_divider;
> - u8 disable_ring_counter;
> + bool disable_ring_counter;
> bool has_reg11;
> bool has_vref_select;
> - u8 vref_select;
> - u8 cmv_select;
> - u8 adc_eoc;
The reason to choose u8 type over bool was that those are not actually
bool values but direct values of hw register bits. We have little
information about real meaning of these bits so it won't help much to
add bool layer and keep real values in the init code instead of param
section (adc_eoc, cmv_select).
bool disable_ring_counter will look deceptive too because it doesn't
say whether disable ring_counter or not (we always disable it) but
how to disable it (write 0 or 1)
I think the poor choice was not the type of variables but their names:
u8 adc_eoc_bit;
u8 cmv_select_bit;
u8 disable_ring_counter_bit;
would be clearer.
> - enum meson_sar_adc_vref_sel vref_voltage;
> + bool cmv_select;
> + bool adc_eoc;
> + enum meson_sar_adc_vref_sel vref_select;
> + enum meson_sar_adc_vref_voltage vref_voltage;
> };
>
> struct meson_sar_adc_data {
> @@ -982,14 +987,16 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
> MESON_SAR_ADC_DELTA_10_TS_REVE0, 0);
> }
>
> - regval = FIELD_PREP(MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN,
> - priv->param->disable_ring_counter);
> + if (priv->param->disable_ring_counter)
> + regval = MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN;
> + else
> + regval = 0;
> regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG3,
> MESON_SAR_ADC_REG3_CTRL_CONT_RING_COUNTER_EN,
> regval);
>
> if (priv->param->has_reg11) {
> - regval = FIELD_PREP(MESON_SAR_ADC_REG11_EOC, priv->param->adc_eoc);
> + regval = priv->param->adc_eoc ? MESON_SAR_ADC_REG11_EOC : 0;
> regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
> MESON_SAR_ADC_REG11_EOC, regval);
>
> @@ -1005,8 +1012,7 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
> regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
> MESON_SAR_ADC_REG11_VREF_VOLTAGE, regval);
>
> - regval = FIELD_PREP(MESON_SAR_ADC_REG11_CMV_SEL,
> - priv->param->cmv_select);
> + regval = priv->param->cmv_select ? MESON_SAR_ADC_REG11_CMV_SEL : 0;
> regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
> MESON_SAR_ADC_REG11_CMV_SEL, regval);
> }
> @@ -1225,8 +1231,8 @@ static const struct meson_sar_adc_param meson_sar_adc_gxbb_param = {
> .regmap_config = &meson_sar_adc_regmap_config_gxbb,
> .resolution = 10,
> .has_reg11 = true,
> - .vref_voltage = 1,
> - .cmv_select = 1,
> + .vref_voltage = VREF_VOLTAGE_1V8,
> + .cmv_select = true,
> };
>
> static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
> @@ -1237,8 +1243,8 @@ static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
> .resolution = 12,
> .disable_ring_counter = 1,
> .has_reg11 = true,
> - .vref_voltage = 1,
> - .cmv_select = 1,
> + .vref_voltage = VREF_VOLTAGE_1V8,
> + .cmv_select = true,
> };
>
> static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
> @@ -1249,10 +1255,10 @@ static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
> .resolution = 12,
> .disable_ring_counter = 1,
> .has_reg11 = true,
> - .vref_voltage = 1,
> + .vref_voltage = VREF_VOLTAGE_1V8,
> .has_vref_select = true,
> .vref_select = VREF_VDDA,
> - .cmv_select = 1,
> + .cmv_select = true,
> };
>
> static const struct meson_sar_adc_param meson_sar_adc_g12a_param = {
> @@ -1263,7 +1269,8 @@ static const struct meson_sar_adc_param meson_sar_adc_g12a_param = {
> .resolution = 12,
> .disable_ring_counter = 1,
> .has_reg11 = true,
> - .adc_eoc = 1,
> + .vref_voltage = VREF_VOLTAGE_0V9,
> + .adc_eoc = true,
> .has_vref_select = true,
> .vref_select = VREF_VDDA,
> };
--
Best regards
George
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [WIP 0/3] Memory model and atomic API in Rust
From: Kent Overstreet @ 2024-03-26 0:36 UTC (permalink / raw)
To: Dr. David Alan Gilbert
Cc: Linus Torvalds, Philipp Stanner, Boqun Feng, rust-for-linux,
linux-kernel, linux-arch, llvm, Miguel Ojeda, Alex Gaynor,
Wedson Almeida Filho, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Alan Stern,
Andrea Parri, Will Deacon, Peter Zijlstra, Nicholas Piggin,
David Howells, Jade Alglave, Luc Maranget, Paul E. McKenney,
Akira Yokosawa, Daniel Lustig, Joel Fernandes, Nathan Chancellor,
Nick Desaulniers, kent.overstreet, Greg Kroah-Hartman, elver,
Mark Rutland, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Catalin Marinas,
linux-arm-kernel, linux-fsdevel
In-Reply-To: <ZgIRXL5YM2AwBD0Y@gallifrey>
On Tue, Mar 26, 2024 at 12:05:48AM +0000, Dr. David Alan Gilbert wrote:
> * Linus Torvalds (torvalds@linux-foundation.org) wrote:
>
> <snip>
>
> > IOW, the whole access size problem that Boqun described is
> > *inherently* tied to the fact that the C++ and Rust memory model is
> > badly designed from the wrong principles.
> >
> > Instead of designing it as a "this is an atomic object that you can do
> > these operations on", it should have been "this is an atomic access,
> > and you can use this simple object model to have the compiler generate
> > the accesses for you".
>
> Isn't one of the aims of the Rust/C++ idea that you can't forget to access
> a shared piece of data atomically?
>
> If you want to have 'atomic accesses' explicitly, how do you tell the compiler
> what you can use them on, and when it should stop you mixing them with
> normal accesses on the same object?
"can't forget to access data atomically" - that's only half of it. And
atomic accesses loads/stores are not a thing under the hood, they're
just loads and stores (possibly, but not necessarily, with memory
barriers).
The other half is at the _source_ level you don't want to treat accesses
to volatiles/atomics like accesses to normal variables, you really want
those to be explicit, and not look like normal variable accesses.
std:atomic_int is way better than volatile in the sense that it's not a
barely specified mess, but adding operator overloading was just
gratuitious and unnecessary.
This is a theme with C++ - they add a _ton_ of magic to make things
concise and pretty, but you have to understand in intimate detail what
all that magic is doing or you're totally fucked.
std::atomic_int makes it such that just changing a single line of code
in a single location in your program will change the semantics of your
_entire_ program and the only obserable result will be that it's faster
but a ticking time bomb because you just introduced a ton of races.
With Rust - I honestly haven't looked at whether they added operator
overlaoding for their atomics, but it's _much_ less of a concern because
changing the type to the non-atomic version means your program won't
compile if it's now racy.
_______________________________________________
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 00/64] i2c: reword i2c_algorithm according to newest specification
From: Andi Shyti @ 2024-03-26 0:36 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-i2c, asahi, chrome-platform, imx, linux-actions,
linux-amlogic, linux-arm-kernel, linux-arm-msm, linux-aspeed,
linux-kernel, linux-mediatek, linux-mips, linux-omap,
linuxppc-dev, linux-renesas-soc, linux-riscv, linux-rockchip,
linux-rpi-kernel, linux-samsung-soc, linux-stm32, linux-sunxi,
linux-tegra, openbmc, virtualization
In-Reply-To: <ug266trshvhhbsln3eoh53fmsuj3l63ziz6gavcl7rv2jhjr5t@3av5givh5n7m>
Hi Wolfram,
> > @Andi: are you okay with this approach? It means you'd need to merge
> > -rc2 into your for-next branch. Or rebase if all fails.
>
> I think it's a good plan, I'll try to support you with it.
Do you feel more comfortable if I take the patches as soon as
they are reviewd?
So far I have tagged patch 1-4 and I can already merge 2,3,4 as
long as you merge patch 1.
Andi
_______________________________________________
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 05/64] i2c: aspeed: reword according to newest specification
From: Andi Shyti @ 2024-03-26 0:17 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-i2c, Brendan Higgins, Benjamin Herrenschmidt, Joel Stanley,
Andrew Jeffery, openbmc, linux-arm-kernel, linux-aspeed,
linux-kernel
In-Reply-To: <20240322132619.6389-6-wsa+renesas@sang-engineering.com>
Hi Wolfram,
On Fri, Mar 22, 2024 at 02:24:58PM +0100, Wolfram Sang wrote:
> Match the wording of this driver wrt. the newest I2C v7, SMBus 3.2, I3C
> specifications and replace "master/slave" with more appropriate terms.
> They are also more specific because we distinguish now between a remote
> entity ("client") and a local one ("target").
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
> drivers/i2c/busses/i2c-aspeed.c | 26 +++++++++++++-------------
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
> index ce8c4846b7fa..4e6ea4a5cab9 100644
> --- a/drivers/i2c/busses/i2c-aspeed.c
> +++ b/drivers/i2c/busses/i2c-aspeed.c
> @@ -159,7 +159,7 @@ struct aspeed_i2c_bus {
> bool send_stop;
> int cmd_err;
> /* Protected only by i2c_lock_bus */
> - int master_xfer_result;
> + int xfer_result;
> /* Multi-master */
> bool multi_master;
> #if IS_ENABLED(CONFIG_I2C_SLAVE)
> @@ -608,9 +608,9 @@ static u32 aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus, u32 irq_status)
> out_complete:
> bus->msgs = NULL;
> if (bus->cmd_err)
> - bus->master_xfer_result = bus->cmd_err;
> + bus->xfer_result = bus->cmd_err;
> else
> - bus->master_xfer_result = bus->msgs_index + 1;
> + bus->xfer_result = bus->msgs_index + 1;
> complete(&bus->cmd_complete);
> out_no_complete:
> return irq_handled;
> @@ -679,7 +679,7 @@ static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
> return irq_remaining ? IRQ_NONE : IRQ_HANDLED;
> }
>
> -static int aspeed_i2c_master_xfer(struct i2c_adapter *adap,
> +static int aspeed_i2c_xfer(struct i2c_adapter *adap,
> struct i2c_msg *msgs, int num)
here the alignment goes a bi off.
> {
> struct aspeed_i2c_bus *bus = i2c_get_adapdata(adap);
> @@ -738,7 +738,7 @@ static int aspeed_i2c_master_xfer(struct i2c_adapter *adap,
> return -ETIMEDOUT;
> }
>
> - return bus->master_xfer_result;
> + return bus->xfer_result;
> }
>
> static u32 aspeed_i2c_functionality(struct i2c_adapter *adap)
> @@ -748,7 +748,7 @@ static u32 aspeed_i2c_functionality(struct i2c_adapter *adap)
>
> #if IS_ENABLED(CONFIG_I2C_SLAVE)
> /* precondition: bus.lock has been acquired. */
> -static void __aspeed_i2c_reg_slave(struct aspeed_i2c_bus *bus, u16 slave_addr)
> +static void __aspeed_i2c_reg_target(struct aspeed_i2c_bus *bus, u16 slave_addr)
We have the word master/slave forgotten here and there, but as
we are here, /slave_addr/target_addr/
> {
> u32 addr_reg_val, func_ctrl_reg_val;
>
> @@ -770,7 +770,7 @@ static void __aspeed_i2c_reg_slave(struct aspeed_i2c_bus *bus, u16 slave_addr)
> bus->slave_state = ASPEED_I2C_SLAVE_INACTIVE;
> }
>
> -static int aspeed_i2c_reg_slave(struct i2c_client *client)
> +static int aspeed_i2c_reg_target(struct i2c_client *client)
> {
> struct aspeed_i2c_bus *bus = i2c_get_adapdata(client->adapter);
> unsigned long flags;
> @@ -781,7 +781,7 @@ static int aspeed_i2c_reg_slave(struct i2c_client *client)
> return -EINVAL;
> }
>
> - __aspeed_i2c_reg_slave(bus, client->addr);
> + __aspeed_i2c_reg_target(bus, client->addr);
>
> bus->slave = client;
> spin_unlock_irqrestore(&bus->lock, flags);
> @@ -789,7 +789,7 @@ static int aspeed_i2c_reg_slave(struct i2c_client *client)
> return 0;
> }
>
> -static int aspeed_i2c_unreg_slave(struct i2c_client *client)
> +static int aspeed_i2c_unreg_target(struct i2c_client *client)
> {
> struct aspeed_i2c_bus *bus = i2c_get_adapdata(client->adapter);
> u32 func_ctrl_reg_val;
> @@ -814,11 +814,11 @@ static int aspeed_i2c_unreg_slave(struct i2c_client *client)
> #endif /* CONFIG_I2C_SLAVE */
>
> static const struct i2c_algorithm aspeed_i2c_algo = {
> - .master_xfer = aspeed_i2c_master_xfer,
> + .xfer = aspeed_i2c_xfer,
here the alignment goes a bit off.
Andi
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [WIP 0/3] Memory model and atomic API in Rust
From: Dr. David Alan Gilbert @ 2024-03-26 0:05 UTC (permalink / raw)
To: Linus Torvalds
Cc: Kent Overstreet, Philipp Stanner, Boqun Feng, rust-for-linux,
linux-kernel, linux-arch, llvm, Miguel Ojeda, Alex Gaynor,
Wedson Almeida Filho, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Alan Stern,
Andrea Parri, Will Deacon, Peter Zijlstra, Nicholas Piggin,
David Howells, Jade Alglave, Luc Maranget, Paul E. McKenney,
Akira Yokosawa, Daniel Lustig, Joel Fernandes, Nathan Chancellor,
Nick Desaulniers, kent.overstreet, Greg Kroah-Hartman, elver,
Mark Rutland, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin, Catalin Marinas,
linux-arm-kernel, linux-fsdevel
In-Reply-To: <CAHk-=wgLGWBXvNODAkzkVHEj7zrrnTq_hzMft62nKNkaL89ZGQ@mail.gmail.com>
* Linus Torvalds (torvalds@linux-foundation.org) wrote:
<snip>
> IOW, the whole access size problem that Boqun described is
> *inherently* tied to the fact that the C++ and Rust memory model is
> badly designed from the wrong principles.
>
> Instead of designing it as a "this is an atomic object that you can do
> these operations on", it should have been "this is an atomic access,
> and you can use this simple object model to have the compiler generate
> the accesses for you".
Isn't one of the aims of the Rust/C++ idea that you can't forget to access
a shared piece of data atomically?
If you want to have 'atomic accesses' explicitly, how do you tell the compiler
what you can use them on, and when it should stop you mixing them with
normal accesses on the same object?
Dave
> This is why I claim that LKMM is fundamentally better. It didn't start
> out from a bass-ackwards starting point of marking objects "atomic".
>
> And yes, the LKMM is a bit awkward, because we don't have the
> shorthands, so you have to write out "atomic_read()" and friends.
>
> Tough. It's better to be correct than to be simple.
>
> Linus
>
--
-----Open up your eyes, open up your mind, open up your code -------
/ Dr. David Alan Gilbert | Running GNU/Linux | Happy \
\ dave @ treblig.org | | In Hex /
\ _________________________|_____ http://www.treblig.org |_______/
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [RFC PATCH v2] ASoC: meson: g12a-toacodec: rework the definition of bits
From: Jan Dakinevich @ 2024-03-25 23:53 UTC (permalink / raw)
To: Jan Dakinevich, Jerome Brunet, Liam Girdwood, Mark Brown,
Jaroslav Kysela, Takashi Iwai, Neil Armstrong, Kevin Hilman,
Martin Blumenstingl, alsa-devel, linux-sound, linux-arm-kernel,
linux-amlogic, linux-kernel
There is a lot of defines, but almost all of them are not used. Lets
rework them:
- keep separate the definition for different platforms to make easier
checking that they match documentation.
- use LSB/MSB sufixes for uniformity.
- don't use hard-coded values for already declared defines.
Signed-off-by: Jan Dakinevich <jan.dakinevich@salutedevices.com>
---
Links:
[1] https://lore.kernel.org/lkml/20240314232201.2102178-1-jan.dakinevich@salutedevices.com/
Changes v1 -> v2:
- Detached from v1's series (patch 7).
- Fixed my wrong understanding of SOC_SINGLE's input parameters.
sound/soc/meson/g12a-toacodec.c | 79 ++++++++++++++++++++-------------
1 file changed, 49 insertions(+), 30 deletions(-)
diff --git a/sound/soc/meson/g12a-toacodec.c b/sound/soc/meson/g12a-toacodec.c
index 531bb8707a3e..22181f4bab72 100644
--- a/sound/soc/meson/g12a-toacodec.c
+++ b/sound/soc/meson/g12a-toacodec.c
@@ -20,26 +20,37 @@
#define G12A_TOACODEC_DRV_NAME "g12a-toacodec"
#define TOACODEC_CTRL0 0x0
-#define CTRL0_ENABLE_SHIFT 31
-#define CTRL0_DAT_SEL_SM1_MSB 19
-#define CTRL0_DAT_SEL_SM1_LSB 18
-#define CTRL0_DAT_SEL_MSB 15
-#define CTRL0_DAT_SEL_LSB 14
-#define CTRL0_LANE_SEL_SM1 16
-#define CTRL0_LANE_SEL 12
-#define CTRL0_LRCLK_SEL_SM1_MSB 14
-#define CTRL0_LRCLK_SEL_SM1_LSB 12
-#define CTRL0_LRCLK_SEL_MSB 9
-#define CTRL0_LRCLK_SEL_LSB 8
-#define CTRL0_LRCLK_INV_SM1 BIT(10)
-#define CTRL0_BLK_CAP_INV_SM1 BIT(9)
-#define CTRL0_BLK_CAP_INV BIT(7)
-#define CTRL0_BCLK_O_INV_SM1 BIT(8)
-#define CTRL0_BCLK_O_INV BIT(6)
-#define CTRL0_BCLK_SEL_SM1_MSB 6
-#define CTRL0_BCLK_SEL_MSB 5
-#define CTRL0_BCLK_SEL_LSB 4
-#define CTRL0_MCLK_SEL GENMASK(2, 0)
+
+/* Common bits */
+#define CTRL0_ENABLE_SHIFT 31
+#define CTRL0_MCLK_SEL GENMASK(2, 0)
+
+/* G12A bits */
+#define CTRL0_DAT_SEL_G12A_MSB 15
+#define CTRL0_DAT_SEL_G12A_LSB 14
+#define CTRL0_LANE_SEL_G12A_MSB 13
+#define CTRL0_LANE_SEL_G12A_LSB 12
+#define CTRL0_LANE_SEL_G12A_MAX 3
+#define CTRL0_LRCLK_SEL_G12A_MSB 9
+#define CTRL0_LRCLK_SEL_G12A_LSB 8
+#define CTRL0_BLK_CAP_INV_G12A BIT(7)
+#define CTRL0_BCLK_O_INV_G12A BIT(6)
+#define CTRL0_BCLK_SEL_G12A_MSB 5
+#define CTRL0_BCLK_SEL_G12A_LSB 4
+
+/* SM1 bits */
+#define CTRL0_DAT_SEL_SM1_MSB 19
+#define CTRL0_DAT_SEL_SM1_LSB 18
+#define CTRL0_LANE_SEL_SM1_MSB 17
+#define CTRL0_LANE_SEL_SM1_LSB 16
+#define CTRL0_LANE_SEL_SM1_MAX 3
+#define CTRL0_LRCLK_SEL_SM1_MSB 14
+#define CTRL0_LRCLK_SEL_SM1_LSB 12
+#define CTRL0_LRCLK_INV_SM1 BIT(10)
+#define CTRL0_BLK_CAP_INV_SM1 BIT(9)
+#define CTRL0_BCLK_O_INV_SM1 BIT(8)
+#define CTRL0_BCLK_SEL_SM1_MSB 6
+#define CTRL0_BCLK_SEL_SM1_LSB 4
#define TOACODEC_OUT_CHMAX 2
@@ -108,7 +119,7 @@ static int g12a_toacodec_mux_put_enum(struct snd_kcontrol *kcontrol,
}
static SOC_ENUM_SINGLE_DECL(g12a_toacodec_mux_enum, TOACODEC_CTRL0,
- CTRL0_DAT_SEL_LSB,
+ CTRL0_DAT_SEL_G12A_LSB,
g12a_toacodec_mux_texts);
static SOC_ENUM_SINGLE_DECL(sm1_toacodec_mux_enum, TOACODEC_CTRL0,
@@ -210,7 +221,7 @@ static int g12a_toacodec_component_probe(struct snd_soc_component *c)
{
/* Initialize the static clock parameters */
return snd_soc_component_write(c, TOACODEC_CTRL0,
- CTRL0_BLK_CAP_INV);
+ CTRL0_BLK_CAP_INV_G12A);
}
static int sm1_toacodec_component_probe(struct snd_soc_component *c)
@@ -229,11 +240,13 @@ static const struct snd_soc_dapm_route g12a_toacodec_routes[] = {
};
static const struct snd_kcontrol_new g12a_toacodec_controls[] = {
- SOC_SINGLE("Lane Select", TOACODEC_CTRL0, CTRL0_LANE_SEL, 3, 0),
+ SOC_SINGLE("Lane Select", TOACODEC_CTRL0, CTRL0_LANE_SEL_G12A_LSB,
+ CTRL0_LANE_SEL_G12A_MAX, 0),
};
static const struct snd_kcontrol_new sm1_toacodec_controls[] = {
- SOC_SINGLE("Lane Select", TOACODEC_CTRL0, CTRL0_LANE_SEL_SM1, 3, 0),
+ SOC_SINGLE("Lane Select", TOACODEC_CTRL0, CTRL0_LANE_SEL_SM1_LSB,
+ CTRL0_LANE_SEL_SM1_MAX, 0),
};
static const struct snd_soc_component_driver g12a_toacodec_component_drv = {
@@ -266,16 +279,22 @@ static const struct regmap_config g12a_toacodec_regmap_cfg = {
static const struct g12a_toacodec_match_data g12a_toacodec_match_data = {
.component_drv = &g12a_toacodec_component_drv,
- .field_dat_sel = REG_FIELD(TOACODEC_CTRL0, 14, 15),
- .field_lrclk_sel = REG_FIELD(TOACODEC_CTRL0, 8, 9),
- .field_bclk_sel = REG_FIELD(TOACODEC_CTRL0, 4, 5),
+ .field_dat_sel = REG_FIELD(TOACODEC_CTRL0, CTRL0_DAT_SEL_G12A_LSB,
+ CTRL0_DAT_SEL_G12A_MSB),
+ .field_lrclk_sel = REG_FIELD(TOACODEC_CTRL0, CTRL0_LRCLK_SEL_G12A_LSB,
+ CTRL0_LRCLK_SEL_G12A_MSB),
+ .field_bclk_sel = REG_FIELD(TOACODEC_CTRL0, CTRL0_BCLK_SEL_G12A_LSB,
+ CTRL0_BCLK_SEL_G12A_MSB),
};
static const struct g12a_toacodec_match_data sm1_toacodec_match_data = {
.component_drv = &sm1_toacodec_component_drv,
- .field_dat_sel = REG_FIELD(TOACODEC_CTRL0, 18, 19),
- .field_lrclk_sel = REG_FIELD(TOACODEC_CTRL0, 12, 14),
- .field_bclk_sel = REG_FIELD(TOACODEC_CTRL0, 4, 6),
+ .field_dat_sel = REG_FIELD(TOACODEC_CTRL0, CTRL0_DAT_SEL_SM1_LSB,
+ CTRL0_DAT_SEL_SM1_MSB),
+ .field_lrclk_sel = REG_FIELD(TOACODEC_CTRL0, CTRL0_LRCLK_SEL_SM1_LSB,
+ CTRL0_LRCLK_SEL_SM1_MSB),
+ .field_bclk_sel = REG_FIELD(TOACODEC_CTRL0, CTRL0_BCLK_SEL_SM1_LSB,
+ CTRL0_BCLK_SEL_SM1_MSB),
};
static const struct of_device_id g12a_toacodec_of_match[] = {
--
2.34.1
_______________________________________________
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 v1 1/3] iio: adc: meson: fix voltage reference selection field name typo
From: George Stark @ 2024-03-25 23:45 UTC (permalink / raw)
To: Martin Blumenstingl
Cc: linux-arm-kernel, linux-kernel, neil.armstrong, lars, jic23,
linux-amlogic, kernel@salutedevices.com
In-Reply-To: <20240323231309.415425-2-martin.blumenstingl@googlemail.com>
Hello Martin
Thanks for the patch
Should the tag
Fixes: 90c6241860bf ("iio: adc: meson: init voltage control bits")
be added?
On 3/24/24 02:13, Martin Blumenstingl wrote:
> The field should be called "vref_voltage", without a typo in the word
> voltage. No functional changes intended.
>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
> drivers/iio/adc/meson_saradc.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> index 13b473d8c6c7..2615d74534df 100644
> --- a/drivers/iio/adc/meson_saradc.c
> +++ b/drivers/iio/adc/meson_saradc.c
> @@ -327,7 +327,7 @@ struct meson_sar_adc_param {
> u8 vref_select;
> u8 cmv_select;
> u8 adc_eoc;
> - enum meson_sar_adc_vref_sel vref_volatge;
> + enum meson_sar_adc_vref_sel vref_voltage;
> };
>
> struct meson_sar_adc_data {
> @@ -1001,7 +1001,7 @@ static int meson_sar_adc_init(struct iio_dev *indio_dev)
> }
>
> regval = FIELD_PREP(MESON_SAR_ADC_REG11_VREF_VOLTAGE,
> - priv->param->vref_volatge);
> + priv->param->vref_voltage);
> regmap_update_bits(priv->regmap, MESON_SAR_ADC_REG11,
> MESON_SAR_ADC_REG11_VREF_VOLTAGE, regval);
>
> @@ -1225,7 +1225,7 @@ static const struct meson_sar_adc_param meson_sar_adc_gxbb_param = {
> .regmap_config = &meson_sar_adc_regmap_config_gxbb,
> .resolution = 10,
> .has_reg11 = true,
> - .vref_volatge = 1,
> + .vref_voltage = 1,
> .cmv_select = 1,
> };
>
> @@ -1237,7 +1237,7 @@ static const struct meson_sar_adc_param meson_sar_adc_gxl_param = {
> .resolution = 12,
> .disable_ring_counter = 1,
> .has_reg11 = true,
> - .vref_volatge = 1,
> + .vref_voltage = 1,
> .cmv_select = 1,
> };
>
> @@ -1249,7 +1249,7 @@ static const struct meson_sar_adc_param meson_sar_adc_axg_param = {
> .resolution = 12,
> .disable_ring_counter = 1,
> .has_reg11 = true,
> - .vref_volatge = 1,
> + .vref_voltage = 1,
> .has_vref_select = true,
> .vref_select = VREF_VDDA,
> .cmv_select = 1,
--
Best regards
George
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [WIP 0/3] Memory model and atomic API in Rust
From: Boqun Feng @ 2024-03-25 23:41 UTC (permalink / raw)
To: Kent Overstreet
Cc: Linus Torvalds, Philipp Stanner, rust-for-linux, linux-kernel,
linux-arch, llvm, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Alan Stern, Andrea Parri, Will Deacon, Peter Zijlstra,
Nicholas Piggin, David Howells, Jade Alglave, Luc Maranget,
Paul E. McKenney, Akira Yokosawa, Daniel Lustig, Joel Fernandes,
Nathan Chancellor, Nick Desaulniers, kent.overstreet,
Greg Kroah-Hartman, elver, Mark Rutland, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Catalin Marinas, linux-arm-kernel, linux-fsdevel
In-Reply-To: <qo6z3j2h32rzwr6bcnkrrw7irdqwakgymcf2jodnq5nz3simwq@tqydlv4mctrv>
On Mon, Mar 25, 2024 at 07:02:12PM -0400, Kent Overstreet wrote:
> On Mon, Mar 25, 2024 at 03:38:32PM -0700, Boqun Feng wrote:
> > On Mon, Mar 25, 2024 at 06:09:19PM -0400, Kent Overstreet wrote:
> > > On Mon, Mar 25, 2024 at 02:37:14PM -0700, Boqun Feng wrote:
> > > > On Mon, Mar 25, 2024 at 05:14:41PM -0400, Kent Overstreet wrote:
> > > > > On Mon, Mar 25, 2024 at 12:44:34PM -0700, Linus Torvalds wrote:
> > > > > > On Mon, 25 Mar 2024 at 11:59, Kent Overstreet <kent.overstreet@linux.dev> wrote:
> > > > > > >
> > > > > > > To be fair, "volatile" dates from an era when we didn't have the haziest
> > > > > > > understanding of what a working memory model for C would look like or
> > > > > > > why we'd even want one.
> > > > > >
> > > > > > I don't disagree, but I find it very depressing that now that we *do*
> > > > > > know about memory models etc, the C++ memory model basically doubled
> > > > > > down on the same "object" model.
> > > > > >
> > > > > > > The way the kernel uses volatile in e.g. READ_ONCE() is fully in line
> > > > > > > with modern thinking, just done with the tools available at the time. A
> > > > > > > more modern version would be just
> > > > > > >
> > > > > > > __atomic_load_n(ptr, __ATOMIC_RELAXED)
> > > >
> > > > Note that Rust does have something similiar:
> > > >
> > > > https://doc.rust-lang.org/std/ptr/fn.read_volatile.html
> > > >
> > > > pub unsafe fn read_volatile<T>(src: *const T) -> T
> > > >
> > > > (and also write_volatile()). So they made a good design putting the
> > > > volatile on the accesses rather than the type. However, per the current
> > > > Rust memory model these two primitives will be UB when data races happen
> > > > :-(
> > > >
> > > > I mean, sure, if I use read_volatile() on an enum (whose valid values
> > > > are only 0, 1, 2), and I get a value 3, and the compiler says "you have
> > > > a logic bug and I refuse to compile the program correctly", I'm OK. But
> > > > if I use read_volatile() to read something like a u32, and I know it's
> > > > racy so my program actually handle that, I don't know any sane compiler
> > > > would miss-compile, so I don't know why that has to be a UB.
> > >
> > > Well, if T is too big to read/write atomically then you'll get torn
> > > reads, including potentially a bit representation that is not a valid T.
> > >
> > > Which is why the normal read_volatile<> or Volatile<> should disallow
> > > that.
> > >
> >
> > Well, why a racy read_volatile<> is UB on a T who is valid for all bit
> > representations is what I was complaining about ;-)
>
> yeah, that should not be considered UB; that should be an easy fix. Are
> you talking to Rust compiler people about this stuff? I've been meaning
Here you go:
https://rust-lang.zulipchat.com/#narrow/stream/136281-t-opsem/topic/UB.20caused.20by.20races.20on.20.60.7Bread.2Cwrite.7D_volatile.60/near/399343771
but maybe instead of Rust, LLVM is the best one to talk with on this.
Because my take from the communication with Rust folks is more like
"it's more up to LLVM on this".
> to make my own contacts there, but - sadly, busy as hell.
>
> > > > > where T is any type that fits in a machine word, and the only operations
> > > > > it supports are get(), set(), xchg() and cmpxchG().
> > > > >
> > > > > You DO NOT want it to be possible to transparantly use Volatile<T> in
> > > > > place of a regular T - in exactly the same way as an atomic_t can't be
> > > > > used in place of a regular integer.
> > > >
> > > > Yes, this is useful. But no it's not that useful, how could you use that
> > > > to read another CPU's stack during some debug functions in a way you
> > > > know it's racy?
> > >
> > > That's a pretty difficult thing to do, because you don't know the
> > > _layout_ of the other CPU's stack, and even if you do it's going to be
> > > changing underneath you without locking.
> > >
> >
> > It's a debug function, I don't care whether the data is accurate, I just
> > want to get much information as possible.
>
> yeah, if you just want the typical backtrace functionality where you're
> just looking for instruction pointers - that's perfectly
> straightforward.
>
> > This kinda of usage, along
> > with cases where the alorigthms are racy themselves are the primary
> > reasons of volatile _accesses_ instead of volatile _types_. For example,
> > you want to read ahead of a counter protected by a lock:
> >
> > if (unlikely(READ_ONCE(cnt))) {
> > spin_lock(lock);
> > int c = cnt; // update of the cnt is protected by a lock.
> > ...
> > }
> >
> > because you want to skip the case where cnt == 0 in a hotpath, and you
> > know someone is going to check this again in some slowpath, so
> > inaccurate data doesn't matter.
>
> That's an interesting one because in Rust cnt is "inside" the lock, you
> can't access it at all without taking the lock - and usually that's
> exactly right.
>
(Now you mention that, once I was trying to construct a deadlock case
with some Rust code, but I couldn't since the locks are naturally
hierarchical because of the types, therefore it's impossible to reverse
the lock ordering. Yes, you still can have deadlocks in Rust, but that
hierarchial type trees really help a lot).
> So to allow this we'd annotate in the type definition (with an
> attribute) which fields we allow read access to without the lock, then
> with some proc macro wizardry we'd get accessors that we can call without
> the lock held.
>
Right, that's field projection:
https://internals.rust-lang.org/t/pre-rfc-field-projection/17383
> So that probably wouldn't be a Volatile<T> thing, that'd be coupled with
> the lock implementation because that's where the accessors would hang
> off of and they'd internally probably just use mem::volatile_read().
>
So we can play the type game as deep as we can, and I'm sure it'll be
helpful, but in the same time, having a reasonable
{read,write}_volatile() semantics on UB and races is also what we need.
> > > So the races thare are equivalent to a bad mem::transmute(), and that is
> > > very much UB.
> > >
> > > For a more typical usage of volatile, consider a ringbuffer with one
> > > thread producing and another thread consuming. Then you've got head and
> > > tail pointers, each written by one thread and read by another.
> > >
> > > You don't need any locking, just memory barriers and
> > > READ_ONCE()/WRITE_ONCE() to update the head and tail pointers. If you
> > > were writing this in Rust today the easy way would be an atomic integer,
> > > but that's not really correct - you're not doing atomic operations
> > > (locked arithmetic), just volatile reads and writes.
> > >
> >
> > Confused, I don't see how Volatile<T> is better than just atomic in this
> > case, since atomc_load() and atomic_store() are also not locked in any
> > memory model if lockless implementation is available.
>
> It certainly compiles to the same code, yeah. But Volatile<T> really is
> the more primitive/generic concept, Atomic<T> is a specialization.
>
> > > Volatile<T> would be Send and Sync, just like atomic integers. You don't
> > > need locking if you're just working with single values that are small
> > > enough for the machine to read/write atomically.
> >
> > So to me Volatile<T> can help in the cases where we know some memory is
> > "external", for example a MMIO address, or ringbuffer between guests and
> > hypervisor. But it doesn't really fix the missing functionality here:
> > allow generating a plain "mov" instruction on x86 for example on _any
> > valid memory_, and programmers can take care of the result.
>
> You're talking about going completely outside the type system, though.
> There is a need for that, but it's very rare and something we really
> want to discourage. Usually, even with volatile access, you do know the
Hey, in memory ordering model areas, we are supposed to work on these
rare cases ;-) These are building blocks for high level synchronization
constructions, so I'm entitled to complain ;-) But yes, to your point,
type system can help a lot, however, there are still cases we need to
roll our own.
Regard,
Boqun
> type - and even if you don't, you have to treat it as _something_ so
> Volatile<ulong> is probably as good as anything.
_______________________________________________
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 4/8] media: platform: ti: use for_each_endpoint_of_node()
From: Kuninori Morimoto @ 2024-03-25 23:25 UTC (permalink / raw)
To: Dan Carpenter
Cc: Alexandre Belloni, Alexander Shishkin, Tomi Valkeinen,
linux-fbdev, dri-devel, Lad, Prabhakar, Laurent Pinchart,
David Airlie, linux-omap, Helge Deller, linux-staging,
linux-media, Daniel Vetter, Suzuki K Poulose, coresight,
Maarten Lankhorst, Maxime Ripard, Rob Herring, Eugen Hristev,
Mauro Carvalho Chehab, Michal Simek, linux-arm-kernel,
Greg Kroah-Hartman, Claudiu Beznea, Thomas Zimmermann
In-Reply-To: <02d6efba-6d1c-465f-a06d-cf7c02656e21@moroto.mountain>
Hi Dan
> > We already have for_each_endpoint_of_node(), don't use
> > of_graph_get_next_endpoint() directly. Replace it.
> >
> > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > ---
(snip)
> > - for (i = 0; ; i++) {
> > + for_each_endpoint_of_node(dev->of_node, endpoint) {
> > struct v4l2_fwnode_endpoint bus_cfg = { .bus_type = 0 };
> > struct device_node *rem;
> >
> > - endpoint = of_graph_get_next_endpoint(dev->of_node, endpoint);
> > - if (!endpoint)
> > - break;
> > -
> > sdinfo = &pdata->sub_devs[i];
> ^
> "i" is uninitialized now.
>
> Also in the initializer it has "struct device_node *endpoint = NULL;"
> which is unnecessary now. And at the end it has:
>
> of_node_put(endpoint);
> return pdata;
>
> Since endpoint is NULL this was always a pointless no-op but now it's
> more obvious, so lets delete that.
Thank you for pointing it.
I will wait other feedback, and post v2 patch in next week.
Best regards
---
Renesas Electronics
Ph.D. Kuninori Morimoto
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [WIP 0/3] Memory model and atomic API in Rust
From: Kent Overstreet @ 2024-03-25 23:02 UTC (permalink / raw)
To: Boqun Feng
Cc: Linus Torvalds, Philipp Stanner, rust-for-linux, linux-kernel,
linux-arch, llvm, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Alan Stern, Andrea Parri, Will Deacon, Peter Zijlstra,
Nicholas Piggin, David Howells, Jade Alglave, Luc Maranget,
Paul E. McKenney, Akira Yokosawa, Daniel Lustig, Joel Fernandes,
Nathan Chancellor, Nick Desaulniers, kent.overstreet,
Greg Kroah-Hartman, elver, Mark Rutland, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Catalin Marinas, linux-arm-kernel, linux-fsdevel
In-Reply-To: <ZgH86EUON30lUk6m@boqun-archlinux>
On Mon, Mar 25, 2024 at 03:38:32PM -0700, Boqun Feng wrote:
> On Mon, Mar 25, 2024 at 06:09:19PM -0400, Kent Overstreet wrote:
> > On Mon, Mar 25, 2024 at 02:37:14PM -0700, Boqun Feng wrote:
> > > On Mon, Mar 25, 2024 at 05:14:41PM -0400, Kent Overstreet wrote:
> > > > On Mon, Mar 25, 2024 at 12:44:34PM -0700, Linus Torvalds wrote:
> > > > > On Mon, 25 Mar 2024 at 11:59, Kent Overstreet <kent.overstreet@linux.dev> wrote:
> > > > > >
> > > > > > To be fair, "volatile" dates from an era when we didn't have the haziest
> > > > > > understanding of what a working memory model for C would look like or
> > > > > > why we'd even want one.
> > > > >
> > > > > I don't disagree, but I find it very depressing that now that we *do*
> > > > > know about memory models etc, the C++ memory model basically doubled
> > > > > down on the same "object" model.
> > > > >
> > > > > > The way the kernel uses volatile in e.g. READ_ONCE() is fully in line
> > > > > > with modern thinking, just done with the tools available at the time. A
> > > > > > more modern version would be just
> > > > > >
> > > > > > __atomic_load_n(ptr, __ATOMIC_RELAXED)
> > >
> > > Note that Rust does have something similiar:
> > >
> > > https://doc.rust-lang.org/std/ptr/fn.read_volatile.html
> > >
> > > pub unsafe fn read_volatile<T>(src: *const T) -> T
> > >
> > > (and also write_volatile()). So they made a good design putting the
> > > volatile on the accesses rather than the type. However, per the current
> > > Rust memory model these two primitives will be UB when data races happen
> > > :-(
> > >
> > > I mean, sure, if I use read_volatile() on an enum (whose valid values
> > > are only 0, 1, 2), and I get a value 3, and the compiler says "you have
> > > a logic bug and I refuse to compile the program correctly", I'm OK. But
> > > if I use read_volatile() to read something like a u32, and I know it's
> > > racy so my program actually handle that, I don't know any sane compiler
> > > would miss-compile, so I don't know why that has to be a UB.
> >
> > Well, if T is too big to read/write atomically then you'll get torn
> > reads, including potentially a bit representation that is not a valid T.
> >
> > Which is why the normal read_volatile<> or Volatile<> should disallow
> > that.
> >
>
> Well, why a racy read_volatile<> is UB on a T who is valid for all bit
> representations is what I was complaining about ;-)
yeah, that should not be considered UB; that should be an easy fix. Are
you talking to Rust compiler people about this stuff? I've been meaning
to make my own contacts there, but - sadly, busy as hell.
> > > > where T is any type that fits in a machine word, and the only operations
> > > > it supports are get(), set(), xchg() and cmpxchG().
> > > >
> > > > You DO NOT want it to be possible to transparantly use Volatile<T> in
> > > > place of a regular T - in exactly the same way as an atomic_t can't be
> > > > used in place of a regular integer.
> > >
> > > Yes, this is useful. But no it's not that useful, how could you use that
> > > to read another CPU's stack during some debug functions in a way you
> > > know it's racy?
> >
> > That's a pretty difficult thing to do, because you don't know the
> > _layout_ of the other CPU's stack, and even if you do it's going to be
> > changing underneath you without locking.
> >
>
> It's a debug function, I don't care whether the data is accurate, I just
> want to get much information as possible.
yeah, if you just want the typical backtrace functionality where you're
just looking for instruction pointers - that's perfectly
straightforward.
> This kinda of usage, along
> with cases where the alorigthms are racy themselves are the primary
> reasons of volatile _accesses_ instead of volatile _types_. For example,
> you want to read ahead of a counter protected by a lock:
>
> if (unlikely(READ_ONCE(cnt))) {
> spin_lock(lock);
> int c = cnt; // update of the cnt is protected by a lock.
> ...
> }
>
> because you want to skip the case where cnt == 0 in a hotpath, and you
> know someone is going to check this again in some slowpath, so
> inaccurate data doesn't matter.
That's an interesting one because in Rust cnt is "inside" the lock, you
can't access it at all without taking the lock - and usually that's
exactly right.
So to allow this we'd annotate in the type definition (with an
attribute) which fields we allow read access to without the lock, then
with some proc macro wizardry we'd get accessors that we can call without
the lock held.
So that probably wouldn't be a Volatile<T> thing, that'd be coupled with
the lock implementation because that's where the accessors would hang
off of and they'd internally probably just use mem::volatile_read().
> > So the races thare are equivalent to a bad mem::transmute(), and that is
> > very much UB.
> >
> > For a more typical usage of volatile, consider a ringbuffer with one
> > thread producing and another thread consuming. Then you've got head and
> > tail pointers, each written by one thread and read by another.
> >
> > You don't need any locking, just memory barriers and
> > READ_ONCE()/WRITE_ONCE() to update the head and tail pointers. If you
> > were writing this in Rust today the easy way would be an atomic integer,
> > but that's not really correct - you're not doing atomic operations
> > (locked arithmetic), just volatile reads and writes.
> >
>
> Confused, I don't see how Volatile<T> is better than just atomic in this
> case, since atomc_load() and atomic_store() are also not locked in any
> memory model if lockless implementation is available.
It certainly compiles to the same code, yeah. But Volatile<T> really is
the more primitive/generic concept, Atomic<T> is a specialization.
> > Volatile<T> would be Send and Sync, just like atomic integers. You don't
> > need locking if you're just working with single values that are small
> > enough for the machine to read/write atomically.
>
> So to me Volatile<T> can help in the cases where we know some memory is
> "external", for example a MMIO address, or ringbuffer between guests and
> hypervisor. But it doesn't really fix the missing functionality here:
> allow generating a plain "mov" instruction on x86 for example on _any
> valid memory_, and programmers can take care of the result.
You're talking about going completely outside the type system, though.
There is a need for that, but it's very rare and something we really
want to discourage. Usually, even with volatile access, you do know the
type - and even if you don't, you have to treat it as _something_ so
Volatile<ulong> is probably as good as anything.
_______________________________________________
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