Re: [PATCH 4/4] powerpc/Makefile: Auto detect cross compiler
Segher Boessenkool writes: > On Wed, Dec 06, 2023 at 10:55:48PM +1100, Michael Ellerman wrote: >> Look for various combinations, matching: >> powerpc(64(le)?)?(-unknown)?-linux(-gnu)?- >> >> There are more possibilities, but the above is known to find a compiler >> on Fedora and Ubuntu (which use linux-gnu-), and also detects the >> kernel.org cross compilers (which use linux-). > > $ sh ../config.sub powerpc64-linux > powerpc64-unknown-linux-gnu > > I am very very lazy, so buildall uses short names everywhere :-) I used to use just ppc64-gcc :) I can add more variants but didn't want to add too many unless someone is actually using them. > Btw, can you build the kernel for a config that differs in 32/64 or > BE/LE with the default your compiler uses? There is no reason this > shouldn't work (you don't use any system libraries), but you need to > use the correct compiler command-line flags for that always. Yes it should work. I've tested at least ppc64le/ppc64/ppc defconfigs. There have been bugs in the past but AFAIK we have fixed all of them, though there's probably some still lurking for more obscure configs. > Acked-by: Segher Boessenkool Thanks. cheers
Re: [PATCH v2 01/10] devm-helpers: introduce devm_mutex_init
On 12/6/23 19:37, George Stark wrote:
Hello Waiman
Thanks for the review.
On 12/7/23 00:02, Waiman Long wrote:
On 12/6/23 14:55, Hans de Goede wrote:
Hi,
On 12/6/23 19:58, George Stark wrote:
Hello Hans
Thanks for the review.
On 12/6/23 18:01, Hans de Goede wrote:
Hi George,
...
mutex_destroy() only actually does anything if CONFIG_DEBUG_MUTEXES
is set, otherwise it is an empty inline-stub.
Adding a devres resource to the device just to call an empty inline
stub which is a no-op seems like a waste of resources. IMHO it
would be better to change this to:
static inline int devm_mutex_init(struct device *dev, struct mutex
*lock)
{
mutex_init(lock);
#ifdef CONFIG_DEBUG_MUTEXES
return devm_add_action_or_reset(dev, devm_mutex_release, lock);
#else
return 0;
#endif
}
To avoid the unnecessary devres allocation when
CONFIG_DEBUG_MUTEXES is not set.
Honestly saying I don't like unnecessary devres allocation either
but the proposed approach has its own price:
1) we'll have more than one place with branching if mutex_destroy
is empty or not using indirect condition. If suddenly
mutex_destroy is extended for non-debug code (in upstream branch or
e.g. by someone for local debug) than there'll be a problem.
2) If mutex_destroy is empty or not depends on CONFIG_PREEMPT_RT
option too. When CONFIG_PREEMPT_RT is on mutex_destroy is always
empty.
As I see it only the mutex interface (mutex.h) has to say
definitely if mutex_destroy must be called. Probably we could add
some define to include/linux/mutex.h,like IS_MUTEX_DESTROY_REQUIRED
and declare it near mutex_destroy definition itself.
That (a IS_MUTEX_DESTROY_REQUIRED define) is an interesting idea.
Lets s>
Adding a devres resource to the device just to call an empty inline
stub which is a no-op seems like a waste of resources. IMHO it
would be better to change this to:
static inline int devm_mutex_init(struct device *dev, struct mutex
*lock)
{
mutex_init(lock);
#ifdef CONFIG_DEBUG_MUTEXES
return devm_add_action_or_reset(dev, devm_mutex_release, lock);
#else
return 0;
#endif
}
ee for v3 if the mutex maintainers will accept that and if not
then I guess we will just need to live with the unnecessary devres
allocation.
The purpose of calling mutex_destroy() is to mark a mutex as being
destroyed so that any subsequent call to mutex_lock/unlock will cause
a warning to be printed when CONFIG_DEBUG_MUTEXES is defined. I would
not say that mutex_destroy() is required. Rather it is a nice to have
for catching programming error.
This is quite understandable but probably mutex_destroy() is not the
best name for an optional API. Questions are asked over and over again
if it can be safely ignored taking into account that it could be
extended in the future. Every maintainer makes decision on that question
in his own way and it leads to inconsistency.
devm_mutex_init could take responsibility for calling/dropping
mutex_destroy() on its own.
The DEBUG_MUTEXES code is relatively old and there was no major change
to it for a number of years. I don't see we will make major change to it
in the near future. Of course, thing may change if there are new
requirement that may affect the DEBUG_MUTEXES code.
Cheers,
Longman
Cheers,
Longman
Re: [PATCH] ocxl: fix driver function comment typo
> Date: Tue, 5 Dec 2023 11:00:16 + [thread overview] > Message-ID: <2f2aca95-f5a6-45fa-9e3b-37aecf657...@csgroup.eu> (raw) > In-Reply-To: <20231205094319.32114-1-jiangyuns...@kylinos.cn> > Please add a description explaining why you want to do that change. > When I grep I see cxlflash driver, I don't know what ocxlflash driver is: > $ git grep ocxl_config_read_afu > ... > drivers/scsi/cxlflash/ocxl_hw.c:rc = ocxl_config_read_afu(pdev, > fcfg, acfg, 0); > drivers/scsi/cxlflash/ocxl_hw.c:dev_err(dev, "%s: > ocxl_config_read_afu failed rc=%d\n", > include/misc/ocxl.h:int ocxl_config_read_afu(struct pci_dev *dev, > Christophe I checked my commit again and found I'd mismatched the driver's name with other things. As you said it doesn't make sense to change cxlflash to ocxlflash. Sorry for the inconvenience, you could just drop my commit please. Thanks, Yunshui > --- > Reported-by: k2ci > Signed-off-by: yunshui > --- > include/misc/ocxl.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h > index 3ed736da02c8..ef7d26009a36 100644 > --- a/include/misc/ocxl.h > +++ b/include/misc/ocxl.h > @@ -324,7 +324,7 @@ int ocxl_global_mmio_clear32(struct ocxl_afu *afu, size_t > offset, > int ocxl_global_mmio_clear64(struct ocxl_afu *afu, size_t offset, > enum ocxl_endian endian, u64 mask); > > -// Functions left here are for compatibility with the cxlflash driver > +// Functions left here are for compatibility with the ocxlflash driver > > /* >* Read the configuration space of a function for the AFU specified by > -- > 2.25.1 >
[PATCH v4 10/12] KVM: x86: never write to memory from kvm_vcpu_check_block()
kvm_vcpu_check_block() is called while not in TASK_RUNNING, and therefore
it cannot sleep. Writing to guest memory is therefore forbidden, but it
can happen on AMD processors if kvm_check_nested_events() causes a vmexit.
Fortunately, all events that are caught by kvm_check_nested_events() are
also recognized by kvm_vcpu_has_events() through vendor callbacks such as
kvm_x86_interrupt_allowed() or kvm_x86_ops.nested_ops->has_events(), so
remove the call and postpone the actual processing to vcpu_block().
Opportunistically honor the return of kvm_check_nested_events(). KVM
punted on the check in kvm_vcpu_running() because the only error path is
if vmx_complete_nested_posted_interrupt() fails, in which case KVM exits
to userspace with "internal error" i.e. the VM is likely dead anyways so
it wasn't worth overloading the return of kvm_vcpu_running().
Add the check mostly so that KVM is consistent with itself; the return of
the call via kvm_apic_accept_events()=>kvm_check_nested_events() that
immediately follows _is_ checked.
Reported-by: Maxim Levitsky
Signed-off-by: Paolo Bonzini
[sean: check and handle return of kvm_check_nested_events()]
Signed-off-by: Sean Christopherson
---
arch/x86/kvm/x86.c | 14 +++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index dcc675d4e44b..8aeacbc2bff9 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -10815,6 +10815,17 @@ static inline int vcpu_block(struct kvm_vcpu *vcpu)
return 1;
}
+ /*
+* Evaluate nested events before exiting the halted state. This allows
+* the halt state to be recorded properly in the VMCS12's activity
+* state field (AMD does not have a similar field and a VM-Exit always
+* causes a spurious wakeup from HLT).
+*/
+ if (is_guest_mode(vcpu)) {
+ if (kvm_check_nested_events(vcpu) < 0)
+ return 0;
+ }
+
if (kvm_apic_accept_events(vcpu) < 0)
return 0;
switch(vcpu->arch.mp_state) {
@@ -10837,9 +10848,6 @@ static inline int vcpu_block(struct kvm_vcpu *vcpu)
static inline bool kvm_vcpu_running(struct kvm_vcpu *vcpu)
{
- if (is_guest_mode(vcpu))
- kvm_check_nested_events(vcpu);
-
return (vcpu->arch.mp_state == KVM_MP_STATE_RUNNABLE &&
!vcpu->arch.apf.halted);
}
This commit breaks delivery of a (virtualized) posted interrupt from
an L1 vCPU to a halted L2 vCPU.
Looking back at commit e6c67d8cf117 ("KVM: nVMX: Wake blocked vCPU in
guest-mode if pending interrupt in virtual APICv"), Liran wrote:
Note that this also handles the case of nested posted-interrupt by the
fact RVI is updated in vmx_complete_nested_posted_interrupt() which is
called from kvm_vcpu_check_block() -> kvm_arch_vcpu_runnable() ->
kvm_vcpu_running() -> vmx_check_nested_events() ->
vmx_complete_nested_posted_interrupt().
Clearly, that is no longer the case.
Re: [PATCH v4 05/13] powerpc/rtas: Facilitate high-level call sequences
Nathan Lynch writes:
> Nathan Lynch writes:
>> Michael Ellerman writes:
>>> Nathan Lynch writes:
Michael Ellerman writes:
> Nathan Lynch writes:
>> Michael Ellerman writes:
>>> Nathan Lynch via B4 Relay
>>> writes:
From: Nathan Lynch
On RTAS platforms there is a general restriction that the OS must not
enter RTAS on more than one CPU at a time. This low-level
serialization requirement is satisfied by holding a spin
lock (rtas_lock) across most RTAS function invocations.
>>> ...
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 1fc0b3fffdd1..52f2242d0c28 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -581,6 +652,28 @@ static const struct rtas_function
*rtas_token_to_function(s32 token)
return NULL;
}
+static void __rtas_function_lock(struct rtas_function *func)
+{
+ if (func && func->lock)
+ mutex_lock(func->lock);
+}
>>>
>>> This is obviously going to defeat most static analysis tools.
>>
>> I guess it's not that obvious to me :-) Is it because the mutex_lock()
>> is conditional? I'll improve this if it's possible.
>
> Well maybe I'm not giving modern static analysis tools enough credit :)
>
> But what I mean that it's not easy to reason about what the function
> does in isolation. ie. all you can say is that it may or may not lock a
> mutex, and you can't say which mutex.
I've pulled the thread on this a little bit and here is what I can do:
* Discard rtas_lock_function() and rtas_unlock_function() and make the
function mutexes extern as needed. As of now only
rtas_ibm_get_vpd_lock will need to be exposed. This enables us to put
__acquires(), __releases(), and __must_hold() annotations in
papr-vpd.c since it explicitly manipulates the mutex.
* Then sys_rtas() becomes the only site that needs
__rtas_function_lock() and __rtas_function_unlock(), which can be
open-coded and commented (and, one hopes, not emulated elsewhere).
This will look something like:
SYSCALL_DEFINE1(rtas, struct rtas_args __user *, uargs)
{
struct rtas_function *func = rtas_token_to_function(token);
if (func->lock)
mutex_lock(func->lock);
[ ... acquire rtas_lock, enter RTAS, fetch any errors ... ]
if (func->lock)
mutex_unlock(func->lock);
The indirection seems unavoidable since we're working backwards from a
token value (supplied by the user and not known at build time) to the
function descriptor.
Is that tolerable for now?
>>>
>>> Yeah. Thanks for looking into it.
>>>
>>> I wasn't unhappy with the original version, but just slightly uneasy
>>> about the locking via pointer.
>>>
>>> But that new proposal sounds good, more code will have static lock
>>> annotations, and only sys_rtas() which is already weird, will have the
>>> dynamic stuff.
>>
>> OK, I'll work that up then.
>
> Well, apparently the annotations aren't useful with mutexes; see these
> threads:
>
> https://lore.kernel.org/all/8e8d93ee2125c739caabe5986f40fa2156c8b4ce.1579893447.git.jbi.oct...@gmail.com/
> https://lore.kernel.org/all/20200601184552.23128-5-jbi.oct...@gmail.com/
>
> And indeed I can't get sparse to accept them when added to the papr-vpd
> code:
>
> $ make C=2 arch/powerpc/platforms/pseries/papr-vpd.o
> CHECK scripts/mod/empty.c
> CALLscripts/checksyscalls.sh
> CHECK arch/powerpc/platforms/pseries/papr-vpd.c
> arch/powerpc/platforms/pseries/papr-vpd.c:235:13: warning: context
> imbalance in 'vpd_sequence_begin' - wrong count at exit
> arch/powerpc/platforms/pseries/papr-vpd.c:269:13: warning: context
> imbalance in 'vpd_sequence_end' - wrong count at exit
Oof. Sorry to send you on that goose chase.
> I don't think it's my own mistake since I see existing code with the
> same problem, such as net/core/sock.c:
>
> static void *proto_seq_start(struct seq_file *seq, loff_t *pos)
> __acquires(proto_list_mutex)
> {
> mutex_lock(_list_mutex);
> return seq_list_start_head(_list, *pos);
> }
>
> static void proto_seq_stop(struct seq_file *seq, void *v)
> __releases(proto_list_mutex)
> {
> mutex_unlock(_list_mutex);
> }
>
> which yields:
>
> net/core/sock.c:4018:13: warning: context imbalance in 'proto_seq_start'
> - wrong count at exit
> net/core/sock.c:4030:13: warning: context imbalance in 'proto_seq_stop'
> - wrong count at exit
>
> So I'll give up on static annotations for this series and look for
> opportunities to add lockdep_assert_held() etc.
OK. There are other static analysers than sparse, and even for lockdep
having more
Re: [PATCH v2 01/10] devm-helpers: introduce devm_mutex_init
Hello Waiman
Thanks for the review.
On 12/7/23 00:02, Waiman Long wrote:
On 12/6/23 14:55, Hans de Goede wrote:
Hi,
On 12/6/23 19:58, George Stark wrote:
Hello Hans
Thanks for the review.
On 12/6/23 18:01, Hans de Goede wrote:
Hi George,
...
mutex_destroy() only actually does anything if CONFIG_DEBUG_MUTEXES
is set, otherwise it is an empty inline-stub.
Adding a devres resource to the device just to call an empty inline
stub which is a no-op seems like a waste of resources. IMHO it
would be better to change this to:
static inline int devm_mutex_init(struct device *dev, struct mutex
*lock)
{
mutex_init(lock);
#ifdef CONFIG_DEBUG_MUTEXES
return devm_add_action_or_reset(dev, devm_mutex_release, lock);
#else
return 0;
#endif
}
To avoid the unnecessary devres allocation when
CONFIG_DEBUG_MUTEXES is not set.
Honestly saying I don't like unnecessary devres allocation either but
the proposed approach has its own price:
1) we'll have more than one place with branching if mutex_destroy is
empty or not using indirect condition. If suddenly mutex_destroy is
extended for non-debug code (in upstream branch or e.g. by someone
for local debug) than there'll be a problem.
2) If mutex_destroy is empty or not depends on CONFIG_PREEMPT_RT
option too. When CONFIG_PREEMPT_RT is on mutex_destroy is always empty.
As I see it only the mutex interface (mutex.h) has to say definitely
if mutex_destroy must be called. Probably we could add some define to
include/linux/mutex.h,like IS_MUTEX_DESTROY_REQUIRED and declare it
near mutex_destroy definition itself.
That (a IS_MUTEX_DESTROY_REQUIRED define) is an interesting idea.
Lets s>
Adding a devres resource to the device just to call an empty inline
stub which is a no-op seems like a waste of resources. IMHO it
would be better to change this to:
static inline int devm_mutex_init(struct device *dev, struct mutex
*lock)
{
mutex_init(lock);
#ifdef CONFIG_DEBUG_MUTEXES
return devm_add_action_or_reset(dev, devm_mutex_release, lock);
#else
return 0;
#endif
}
ee for v3 if the mutex maintainers will accept that and if not
then I guess we will just need to live with the unnecessary devres
allocation.
The purpose of calling mutex_destroy() is to mark a mutex as being
destroyed so that any subsequent call to mutex_lock/unlock will cause a
warning to be printed when CONFIG_DEBUG_MUTEXES is defined. I would not
say that mutex_destroy() is required. Rather it is a nice to have for
catching programming error.
This is quite understandable but probably mutex_destroy() is not the
best name for an optional API. Questions are asked over and over again
if it can be safely ignored taking into account that it could be
extended in the future. Every maintainer makes decision on that question
in his own way and it leads to inconsistency.
devm_mutex_init could take responsibility for calling/dropping
mutex_destroy() on its own.
Cheers,
Longman
--
Best regards
George
Re: [PATCH v2 01/10] devm-helpers: introduce devm_mutex_init
Hello Christophe
On 12/7/23 01:37, Christophe Leroy wrote:
Le 06/12/2023 à 23:14, Christophe Leroy a écrit :
Le 06/12/2023 à 19:58, George Stark a écrit :
[Vous ne recevez pas souvent de courriers de
gnst...@salutedevices.com. Découvrez pourquoi ceci est important à
https://aka.ms/LearnAboutSenderIdentification ]
Hello Hans
Thanks for the review.
On 12/6/23 18:01, Hans de Goede wrote:
Hi George,
On 12/4/23 19:05, George Stark wrote:
Using of devm API leads to certain order of releasing resources.
So all dependent resources which are not devm-wrapped should be deleted
with respect to devm-release order. Mutex is one of such objects that
often is bound to other resources and has no own devm wrapping.
Since mutex_destroy() actually does nothing in non-debug builds
frequently calling mutex_destroy() is just ignored which is safe for
now
but wrong formally and can lead to a problem if mutex_destroy() is
extended so introduce devm_mutex_init().
Signed-off-by: George Stark
---
include/linux/devm-helpers.h | 18 ++
1 file changed, 18 insertions(+)
diff --git a/include/linux/devm-helpers.h
b/include/linux/devm-helpers.h
index 74891802200d..2f56e476776f 100644
--- a/include/linux/devm-helpers.h
+++ b/include/linux/devm-helpers.h
@@ -76,4 +76,22 @@ static inline int devm_work_autocancel(struct
device *dev,
return devm_add_action(dev, devm_work_drop, w);
}
+static inline void devm_mutex_release(void *res)
+{
+ mutex_destroy(res);
+}
+
+/**
+ * devm_mutex_init - Resource-managed mutex initialization
+ * @dev: Device which lifetime work is bound to
+ * @lock: Pointer to a mutex
+ *
+ * Initialize mutex which is automatically destroyed when driver is
detached.
+ */
+static inline int devm_mutex_init(struct device *dev, struct mutex
*lock)
+{
+ mutex_init(lock);
+ return devm_add_action_or_reset(dev, devm_mutex_release, lock);
+}
+
#endif
mutex_destroy() only actually does anything if CONFIG_DEBUG_MUTEXES
is set, otherwise it is an empty inline-stub.
Adding a devres resource to the device just to call an empty inline
stub which is a no-op seems like a waste of resources. IMHO it
would be better to change this to:
static inline int devm_mutex_init(struct device *dev, struct mutex
*lock)
{
mutex_init(lock);
#ifdef CONFIG_DEBUG_MUTEXES
return devm_add_action_or_reset(dev, devm_mutex_release, lock);
#else
return 0;
#endif
}
To avoid the unnecessary devres allocation when
CONFIG_DEBUG_MUTEXES is not set.
Honestly saying I don't like unnecessary devres allocation either but
the proposed approach has its own price:
1) we'll have more than one place with branching if mutex_destroy is
empty or not using indirect condition. If suddenly mutex_destroy is
extended for non-debug code (in upstream branch or e.g. by someone for
local debug) than there'll be a problem.
2) If mutex_destroy is empty or not depends on CONFIG_PREEMPT_RT option
too. When CONFIG_PREEMPT_RT is on mutex_destroy is always empty.
As I see it only the mutex interface (mutex.h) has to say definitely if
mutex_destroy must be called. Probably we could add some define to
include/linux/mutex.h,like IS_MUTEX_DESTROY_REQUIRED and declare it near
mutex_destroy definition itself.
I tried to put devm_mutex_init itself in mutex.h and it could've helped
too but it's not the place for devm API.
What do you mean by "it's not the place for devm API" ?
If you do a 'grep devm_ include/linux/' you'll find devm_ functions in
almost 100 .h files. Why wouldn't mutex.h be the place for
devm_mutex_init() ?
mutex.h's maintainers believe so.
https://lore.kernel.org/lkml/070c174c-057a-46de-ae8e-836e9e20e...@salutedevices.com/T/#mb42e1d7760816b0cedd3130e08f29690496b5ac2
Looking at it closer, I have the feeling that you want to do similar to
devm_gpio_request() in linux/gpio.h :
In linux/mutex.h, add a prototype for devm_mutex_init() when
CONFIG_DEBUG_MUTEXES is defined and an empty static inline otherwise.
Then define devm_mutex_init() in kernel/locking/mutex-debug.c
Yes, this would be almost perfect decision. BTW just as in linux/gpio.h
we wouldn't have to include whole "linux/device.h" into mutex.h, only
add forward declaration of struct device;
Wouldn't that work ?
Christophe
--
Best regards
George
Re: [PATCH v3 5/7] kexec_file, ricv: print out debugging message if required
On 12/06/23 at 04:54pm, Conor Dooley wrote:
> On Wed, Dec 06, 2023 at 11:37:52PM +0800, Baoquan He wrote:
> > On 12/04/23 at 04:14pm, Conor Dooley wrote:
> > > On Mon, Dec 04, 2023 at 11:38:05PM +0800, Baoquan He wrote:
> > > > On 12/01/23 at 10:38am, Conor Dooley wrote:
> > > > > On Thu, Nov 30, 2023 at 10:39:53AM +0800, Baoquan He wrote:
> > > > >
> > > > > $subject has a typo in the arch bit :)
> > > >
> > > > Indeed, will fix if need report. Thanks for careful checking.
> > > >
> > > > >
> > > > > > Replace pr_debug() with the newly added kexec_dprintk() in
> > > > > > kexec_file
> > > > > > loading related codes.
> > > > >
> > > > > Commit messages should be understandable in isolation, but this only
> > > > > explains (part of) what is obvious in the diff. Why is this change
> > > > > being made?
> > > >
> > > > The purpose has been detailedly described in cover letter and patch 1
> > > > log. Andrew has picked these patches into his tree and grabbed the cover
> > > > letter log into the relevant commit for people's later checking. All
> > > > these seven patches will be present in mainline together. This is common
> > > > way when posting patch series? Please let me know if I misunderstand
> > > > anything.
> > >
> > > Each patch having a commit message that explains why a change is being
> > > made is the expectation. It is especially useful to explain the why
> > > here, since it is not just a mechanical conversion of pr_debug()s as the
> > > commit message suggests.
> >
> > Sounds reasonable. I rephrase the patch 3 log as below, do you think
> > it's OK to you?
>
> Yes, but with one comment.
>
> >
> > I will also adjust patch logs on other ARCH once this one is done.
> > Thanks.
> >
> > =
> > Subject: [PATCH v3 5/7] kexec_file, ricv: print out debugging message if
> > required
> >
> > Then when specifying '-d' for kexec_file_load interface, loaded
> > locations of kernel/initrd/cmdline etc can be printed out to help debug.
> >
> > Here replace pr_debug() with the newly added kexec_dprintk() in kexec_file
> > loading related codes.
> >
>
> > And also replace pr_notice() with kexec_dprintk() in elf_kexec_load()
> > because it's make sense to always print out loaded location of purgatory
~
> > and device tree even though users don't expect the message.
Fixed typo:
==
And also replace pr_notice() with kexec_dprintk() in elf_kexec_load()
because it doesn't make sense to always print out loaded location of
purgatory and device tree even though users don't expect the message.
>
> This seems to contradict what you said in your earlier mail, about
> moving these from notice to debug. I think you missed a negation in your
> new version of the commit message. What you said in response to me seems
> like a more complete explanation anyway:
Ah, I made mistake when typing, these printing is only for debugging,
so always printing out them is not suggested.
> always printing out the loaded location of purgatory and
> device tree doesn't make sense. It will be confusing when users
> see these even when they do normal kexec/kdump loading.
>
> Thanks,
> Conor.
>
> > And also remove kexec_image_info() because the content has been printed
> > out in generic code.
> >
> >
> >
> > >
> > > > >
> > > > > >
> > > > > > And also remove kexec_image_info() because the content has been
> > > > > > printed
> > > > > > out in generic code.
> > > > > >
> > > > > > Signed-off-by: Baoquan He
> > > > > > ---
> > > > > > arch/riscv/kernel/elf_kexec.c | 11 ++-
> > > > > > arch/riscv/kernel/machine_kexec.c | 26 --
> > > > > > 2 files changed, 6 insertions(+), 31 deletions(-)
> > > > > >
> > > > > > diff --git a/arch/riscv/kernel/elf_kexec.c
> > > > > > b/arch/riscv/kernel/elf_kexec.c
> > > > > > index e60fbd8660c4..5bd1ec3341fe 100644
> > > > > > --- a/arch/riscv/kernel/elf_kexec.c
> > > > > > +++ b/arch/riscv/kernel/elf_kexec.c
> > > > > > @@ -216,7 +216,6 @@ static void *elf_kexec_load(struct kimage
> > > > > > *image, char *kernel_buf,
> > > > > > if (ret)
> > > > > > goto out;
> > > > > > kernel_start = image->start;
> > > > > > - pr_notice("The entry point of kernel at 0x%lx\n", image->start);
> > > > > >
> > > > > > /* Add the kernel binary to the image */
> > > > > > ret = riscv_kexec_elf_load(image, , _info,
> > > > > > @@ -252,8 +251,8 @@ static void *elf_kexec_load(struct kimage
> > > > > > *image, char *kernel_buf,
> > > > > > image->elf_load_addr = kbuf.mem;
> > > > > > image->elf_headers_sz = headers_sz;
> > > > > >
> > > > > > - pr_debug("Loaded elf core header at 0x%lx bufsz=0x%lx
> > > > > > memsz=0x%lx\n",
> > > > > > -image->elf_load_addr, kbuf.bufsz, kbuf.memsz);
> > > > > > + kexec_dprintk("Loaded elf core header at 0x%lx
> > > > > > bufsz=0x%lx
[PATCH 3/3] PCI/AER: Use explicit register sizes for struct members
From: Bjorn Helgaas
aer_irq() reads the AER Root Error Status and Error Source Identification
(PCI_ERR_ROOT_STATUS and PCI_ERR_ROOT_ERR_SRC) registers directly into
struct aer_err_source. Both registers are 32 bits, so declare the members
explicitly as "u32" instead of "unsigned int".
Similarly, aer_get_device_error_info() reads the AER Header Log
(PCI_ERR_HEADER_LOG) registers, which are also 32 bits, into struct
aer_header_log_regs. Declare those members as "u32" as well.
No functional changes intended.
Signed-off-by: Bjorn Helgaas
---
drivers/pci/pcie/aer.c | 4 ++--
include/linux/aer.h| 8
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 2ff6bac9979f..60f84414ec2a 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -41,8 +41,8 @@
#define AER_MAX_TYPEOF_UNCOR_ERRS 27 /* as per PCI_ERR_UNCOR_STATUS*/
struct aer_err_source {
- unsigned int status;
- unsigned int id;
+ u32 status; /* PCI_ERR_ROOT_STATUS */
+ u32 id; /* PCI_ERR_ROOT_ERR_SRC */
};
struct aer_rpc {
diff --git a/include/linux/aer.h b/include/linux/aer.h
index f6ea2f57d808..ae0fae70d4bd 100644
--- a/include/linux/aer.h
+++ b/include/linux/aer.h
@@ -19,10 +19,10 @@
struct pci_dev;
struct aer_header_log_regs {
- unsigned int dw0;
- unsigned int dw1;
- unsigned int dw2;
- unsigned int dw3;
+ u32 dw0;
+ u32 dw1;
+ u32 dw2;
+ u32 dw3;
};
struct aer_capability_regs {
--
2.34.1
[PATCH 2/3] PCI/AER: Decode Requester ID when no error info found
From: Bjorn Helgaas
When a device with AER detects an error, it logs error information in its
own AER Error Status registers. It may send an Error Message to the Root
Port (RCEC in the case of an RCiEP), which logs the fact that an Error
Message was received (Root Error Status) and the Requester ID of the
message source (Error Source Identification).
aer_print_port_info() prints the Requester ID from the Root Port Error
Source in the usual Linux "bb:dd.f" format, but when find_source_device()
finds no error details in the hierarchy below the Root Port, it printed the
raw Requester ID without decoding it.
Decode the Requester ID in the usual Linux format so it matches other
messages.
Sample message changes:
- pcieport :00:1c.5: AER: Correctable error received: :00:1c.5
- pcieport :00:1c.5: AER: can't find device of ID00e5
+ pcieport :00:1c.5: AER: Correctable error message received from
:00:1c.5
+ pcieport :00:1c.5: AER: found no error details for :00:1c.5
Signed-off-by: Bjorn Helgaas
---
drivers/pci/pcie/aer.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 20db80018b5d..2ff6bac9979f 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -740,7 +740,7 @@ static void aer_print_port_info(struct pci_dev *dev, struct
aer_err_info *info)
u8 bus = info->id >> 8;
u8 devfn = info->id & 0xff;
- pci_info(dev, "%s%s error received: %04x:%02x:%02x.%d\n",
+ pci_info(dev, "%s%s error message received from %04x:%02x:%02x.%d\n",
info->multi_error_valid ? "Multiple " : "",
aer_error_severity_string[info->severity],
pci_domain_nr(dev->bus), bus, PCI_SLOT(devfn),
@@ -929,7 +929,12 @@ static bool find_source_device(struct pci_dev *parent,
pci_walk_bus(parent->subordinate, find_device_iter, e_info);
if (!e_info->error_dev_num) {
- pci_info(parent, "can't find device of ID%04x\n", e_info->id);
+ u8 bus = e_info->id >> 8;
+ u8 devfn = e_info->id & 0xff;
+
+ pci_info(parent, "found no error details for
%04x:%02x:%02x.%d\n",
+pci_domain_nr(parent->bus), bus, PCI_SLOT(devfn),
+PCI_FUNC(devfn));
return false;
}
return true;
--
2.34.1
[PATCH 1/3] PCI/AER: Use 'Correctable' and 'Uncorrectable' spec terms for errors
From: Bjorn Helgaas
The PCIe spec classifies errors as either "Correctable" or "Uncorrectable".
Previously we printed these as "Corrected" or "Uncorrected". To avoid
confusion, use the same terms as the spec.
One confusing situation is when one agent detects an error, but another
agent is responsible for recovery, e.g., by re-attempting the operation.
The first agent may log a "correctable" error but it has not yet been
corrected. The recovery agent must report an uncorrectable error if it is
unable to recover. If we print the first agent's error as "Corrected", it
gives the false impression that it has already been resolved.
Sample message change:
- pcieport :00:1c.5: AER: Corrected error received: :00:1c.5
+ pcieport :00:1c.5: AER: Correctable error received: :00:1c.5
Signed-off-by: Bjorn Helgaas
---
drivers/pci/pcie/aer.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 42a3bd35a3e1..20db80018b5d 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -436,9 +436,9 @@ void pci_aer_exit(struct pci_dev *dev)
* AER error strings
*/
static const char *aer_error_severity_string[] = {
- "Uncorrected (Non-Fatal)",
- "Uncorrected (Fatal)",
- "Corrected"
+ "Uncorrectable (Non-Fatal)",
+ "Uncorrectable (Fatal)",
+ "Correctable"
};
static const char *aer_error_layer[] = {
--
2.34.1
[PATCH 0/3] PCI/AER: Clean up logging
From: Bjorn Helgaas Clean up some minor AER logging issues: - Log as "Correctable errors", not "Corrected errors" - Decode the Requester ID when we couldn't find detail error info Bjorn Helgaas (3): PCI/AER: Use 'Correctable' and 'Uncorrectable' spec terms for errors PCI/AER: Decode Requester ID when no error info found PCI/AER: Use explicit register sizes for struct members drivers/pci/pcie/aer.c | 19 --- include/linux/aer.h| 8 2 files changed, 16 insertions(+), 11 deletions(-) -- 2.34.1
Re: [PATCH v2 01/10] devm-helpers: introduce devm_mutex_init
Le 06/12/2023 à 23:14, Christophe Leroy a écrit :
>
>
> Le 06/12/2023 à 19:58, George Stark a écrit :
>> [Vous ne recevez pas souvent de courriers de
>> gnst...@salutedevices.com. Découvrez pourquoi ceci est important à
>> https://aka.ms/LearnAboutSenderIdentification ]
>>
>> Hello Hans
>>
>> Thanks for the review.
>>
>> On 12/6/23 18:01, Hans de Goede wrote:
>>> Hi George,
>>>
>>> On 12/4/23 19:05, George Stark wrote:
Using of devm API leads to certain order of releasing resources.
So all dependent resources which are not devm-wrapped should be deleted
with respect to devm-release order. Mutex is one of such objects that
often is bound to other resources and has no own devm wrapping.
Since mutex_destroy() actually does nothing in non-debug builds
frequently calling mutex_destroy() is just ignored which is safe for
now
but wrong formally and can lead to a problem if mutex_destroy() is
extended so introduce devm_mutex_init().
Signed-off-by: George Stark
---
include/linux/devm-helpers.h | 18 ++
1 file changed, 18 insertions(+)
diff --git a/include/linux/devm-helpers.h
b/include/linux/devm-helpers.h
index 74891802200d..2f56e476776f 100644
--- a/include/linux/devm-helpers.h
+++ b/include/linux/devm-helpers.h
@@ -76,4 +76,22 @@ static inline int devm_work_autocancel(struct
device *dev,
return devm_add_action(dev, devm_work_drop, w);
}
+static inline void devm_mutex_release(void *res)
+{
+ mutex_destroy(res);
+}
+
+/**
+ * devm_mutex_init - Resource-managed mutex initialization
+ * @dev: Device which lifetime work is bound to
+ * @lock: Pointer to a mutex
+ *
+ * Initialize mutex which is automatically destroyed when driver is
detached.
+ */
+static inline int devm_mutex_init(struct device *dev, struct mutex
*lock)
+{
+ mutex_init(lock);
+ return devm_add_action_or_reset(dev, devm_mutex_release, lock);
+}
+
#endif
>>>
>>> mutex_destroy() only actually does anything if CONFIG_DEBUG_MUTEXES
>>> is set, otherwise it is an empty inline-stub.
>>>
>>> Adding a devres resource to the device just to call an empty inline
>>> stub which is a no-op seems like a waste of resources. IMHO it
>>> would be better to change this to:
>>>
>>> static inline int devm_mutex_init(struct device *dev, struct mutex
>>> *lock)
>>> {
>>> mutex_init(lock);
>>> #ifdef CONFIG_DEBUG_MUTEXES
>>> return devm_add_action_or_reset(dev, devm_mutex_release, lock);
>>> #else
>>> return 0;
>>> #endif
>>> }
>>>
>>> To avoid the unnecessary devres allocation when
>>> CONFIG_DEBUG_MUTEXES is not set.
>>
>> Honestly saying I don't like unnecessary devres allocation either but
>> the proposed approach has its own price:
>>
>> 1) we'll have more than one place with branching if mutex_destroy is
>> empty or not using indirect condition. If suddenly mutex_destroy is
>> extended for non-debug code (in upstream branch or e.g. by someone for
>> local debug) than there'll be a problem.
>>
>> 2) If mutex_destroy is empty or not depends on CONFIG_PREEMPT_RT option
>> too. When CONFIG_PREEMPT_RT is on mutex_destroy is always empty.
>>
>> As I see it only the mutex interface (mutex.h) has to say definitely if
>> mutex_destroy must be called. Probably we could add some define to
>> include/linux/mutex.h,like IS_MUTEX_DESTROY_REQUIRED and declare it near
>> mutex_destroy definition itself.
>>
>> I tried to put devm_mutex_init itself in mutex.h and it could've helped
>> too but it's not the place for devm API.
>>
>
> What do you mean by "it's not the place for devm API" ?
>
> If you do a 'grep devm_ include/linux/' you'll find devm_ functions in
> almost 100 .h files. Why wouldn't mutex.h be the place for
> devm_mutex_init() ?
Looking at it closer, I have the feeling that you want to do similar to
devm_gpio_request() in linux/gpio.h :
In linux/mutex.h, add a prototype for devm_mutex_init() when
CONFIG_DEBUG_MUTEXES is defined and an empty static inline otherwise.
Then define devm_mutex_init() in kernel/locking/mutex-debug.c
Wouldn't that work ?
Christophe
[PATCH v2] i2c: cpm: Remove linux,i2c-index conversion from be32
sparse reports an error on some data that gets converted from be32. That's because that data is typed u32 instead of __be32. The type is correct, the be32_to_cpu() conversion is not. Remove the conversion. Reported-by: kernel test robot Closes: https://lore.kernel.org/oe-kbuild-all/202312042210.ql4da8av-...@intel.com/ Signed-off-by: Christophe Leroy --- v2: Use u32 directly, remove be32_to_cpu(). --- drivers/i2c/busses/i2c-cpm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/i2c/busses/i2c-cpm.c b/drivers/i2c/busses/i2c-cpm.c index 9a664abf734d..771d60bc8d71 100644 --- a/drivers/i2c/busses/i2c-cpm.c +++ b/drivers/i2c/busses/i2c-cpm.c @@ -658,7 +658,7 @@ static int cpm_i2c_probe(struct platform_device *ofdev) /* register new adapter to i2c module... */ data = of_get_property(ofdev->dev.of_node, "linux,i2c-index", ); - cpm->adap.nr = (data && len == 4) ? be32_to_cpup(data) : -1; + cpm->adap.nr = (data && len == 4) ? *data : -1; result = i2c_add_numbered_adapter(>adap); if (result < 0) -- 2.41.0
Re: [PATCH v2 01/10] devm-helpers: introduce devm_mutex_init
Le 06/12/2023 à 19:58, George Stark a écrit :
> [Vous ne recevez pas souvent de courriers de gnst...@salutedevices.com.
> Découvrez pourquoi ceci est important à
> https://aka.ms/LearnAboutSenderIdentification ]
>
> Hello Hans
>
> Thanks for the review.
>
> On 12/6/23 18:01, Hans de Goede wrote:
>> Hi George,
>>
>> On 12/4/23 19:05, George Stark wrote:
>>> Using of devm API leads to certain order of releasing resources.
>>> So all dependent resources which are not devm-wrapped should be deleted
>>> with respect to devm-release order. Mutex is one of such objects that
>>> often is bound to other resources and has no own devm wrapping.
>>> Since mutex_destroy() actually does nothing in non-debug builds
>>> frequently calling mutex_destroy() is just ignored which is safe for now
>>> but wrong formally and can lead to a problem if mutex_destroy() is
>>> extended so introduce devm_mutex_init().
>>>
>>> Signed-off-by: George Stark
>>> ---
>>> include/linux/devm-helpers.h | 18 ++
>>> 1 file changed, 18 insertions(+)
>>>
>>> diff --git a/include/linux/devm-helpers.h b/include/linux/devm-helpers.h
>>> index 74891802200d..2f56e476776f 100644
>>> --- a/include/linux/devm-helpers.h
>>> +++ b/include/linux/devm-helpers.h
>>> @@ -76,4 +76,22 @@ static inline int devm_work_autocancel(struct
>>> device *dev,
>>> return devm_add_action(dev, devm_work_drop, w);
>>> }
>>>
>>> +static inline void devm_mutex_release(void *res)
>>> +{
>>> + mutex_destroy(res);
>>> +}
>>> +
>>> +/**
>>> + * devm_mutex_init - Resource-managed mutex initialization
>>> + * @dev: Device which lifetime work is bound to
>>> + * @lock: Pointer to a mutex
>>> + *
>>> + * Initialize mutex which is automatically destroyed when driver is
>>> detached.
>>> + */
>>> +static inline int devm_mutex_init(struct device *dev, struct mutex
>>> *lock)
>>> +{
>>> + mutex_init(lock);
>>> + return devm_add_action_or_reset(dev, devm_mutex_release, lock);
>>> +}
>>> +
>>> #endif
>>
>> mutex_destroy() only actually does anything if CONFIG_DEBUG_MUTEXES
>> is set, otherwise it is an empty inline-stub.
>>
>> Adding a devres resource to the device just to call an empty inline
>> stub which is a no-op seems like a waste of resources. IMHO it
>> would be better to change this to:
>>
>> static inline int devm_mutex_init(struct device *dev, struct mutex *lock)
>> {
>> mutex_init(lock);
>> #ifdef CONFIG_DEBUG_MUTEXES
>> return devm_add_action_or_reset(dev, devm_mutex_release, lock);
>> #else
>> return 0;
>> #endif
>> }
>>
>> To avoid the unnecessary devres allocation when
>> CONFIG_DEBUG_MUTEXES is not set.
>
> Honestly saying I don't like unnecessary devres allocation either but
> the proposed approach has its own price:
>
> 1) we'll have more than one place with branching if mutex_destroy is
> empty or not using indirect condition. If suddenly mutex_destroy is
> extended for non-debug code (in upstream branch or e.g. by someone for
> local debug) than there'll be a problem.
>
> 2) If mutex_destroy is empty or not depends on CONFIG_PREEMPT_RT option
> too. When CONFIG_PREEMPT_RT is on mutex_destroy is always empty.
>
> As I see it only the mutex interface (mutex.h) has to say definitely if
> mutex_destroy must be called. Probably we could add some define to
> include/linux/mutex.h,like IS_MUTEX_DESTROY_REQUIRED and declare it near
> mutex_destroy definition itself.
>
> I tried to put devm_mutex_init itself in mutex.h and it could've helped
> too but it's not the place for devm API.
>
What do you mean by "it's not the place for devm API" ?
If you do a 'grep devm_ include/linux/' you'll find devm_ functions in
almost 100 .h files. Why wouldn't mutex.h be the place for
devm_mutex_init() ?
Christophe
Re: [PATCH v2 01/10] devm-helpers: introduce devm_mutex_init
Hi,
On 12/6/23 19:58, George Stark wrote:
>
> Hello Hans
>
> Thanks for the review.
>
> On 12/6/23 18:01, Hans de Goede wrote:
>> Hi George,
>>
>> On 12/4/23 19:05, George Stark wrote:
>>> Using of devm API leads to certain order of releasing resources.
>>> So all dependent resources which are not devm-wrapped should be deleted
>>> with respect to devm-release order. Mutex is one of such objects that
>>> often is bound to other resources and has no own devm wrapping.
>>> Since mutex_destroy() actually does nothing in non-debug builds
>>> frequently calling mutex_destroy() is just ignored which is safe for now
>>> but wrong formally and can lead to a problem if mutex_destroy() is
>>> extended so introduce devm_mutex_init().
>>>
>>> Signed-off-by: George Stark
>>> ---
>>> include/linux/devm-helpers.h | 18 ++
>>> 1 file changed, 18 insertions(+)
>>>
>>> diff --git a/include/linux/devm-helpers.h b/include/linux/devm-helpers.h
>>> index 74891802200d..2f56e476776f 100644
>>> --- a/include/linux/devm-helpers.h
>>> +++ b/include/linux/devm-helpers.h
>>> @@ -76,4 +76,22 @@ static inline int devm_work_autocancel(struct device
>>> *dev,
>>> return devm_add_action(dev, devm_work_drop, w);
>>> }
>>> +static inline void devm_mutex_release(void *res)
>>> +{
>>> + mutex_destroy(res);
>>> +}
>>> +
>>> +/**
>>> + * devm_mutex_init - Resource-managed mutex initialization
>>> + * @dev: Device which lifetime work is bound to
>>> + * @lock: Pointer to a mutex
>>> + *
>>> + * Initialize mutex which is automatically destroyed when driver is
>>> detached.
>>> + */
>>> +static inline int devm_mutex_init(struct device *dev, struct mutex *lock)
>>> +{
>>> + mutex_init(lock);
>>> + return devm_add_action_or_reset(dev, devm_mutex_release, lock);
>>> +}
>>> +
>>> #endif
>>
>> mutex_destroy() only actually does anything if CONFIG_DEBUG_MUTEXES
>> is set, otherwise it is an empty inline-stub.
>>
>> Adding a devres resource to the device just to call an empty inline
>> stub which is a no-op seems like a waste of resources. IMHO it
>> would be better to change this to:
>>
>> static inline int devm_mutex_init(struct device *dev, struct mutex *lock)
>> {
>> mutex_init(lock);
>> #ifdef CONFIG_DEBUG_MUTEXES
>> return devm_add_action_or_reset(dev, devm_mutex_release, lock);
>> #else
>> return 0;
>> #endif
>> }
>>
>> To avoid the unnecessary devres allocation when
>> CONFIG_DEBUG_MUTEXES is not set.
>
> Honestly saying I don't like unnecessary devres allocation either but the
> proposed approach has its own price:
>
> 1) we'll have more than one place with branching if mutex_destroy is empty or
> not using indirect condition. If suddenly mutex_destroy is extended for
> non-debug code (in upstream branch or e.g. by someone for local debug) than
> there'll be a problem.
>
> 2) If mutex_destroy is empty or not depends on CONFIG_PREEMPT_RT option too.
> When CONFIG_PREEMPT_RT is on mutex_destroy is always empty.
>
> As I see it only the mutex interface (mutex.h) has to say definitely if
> mutex_destroy must be called. Probably we could add some define to
> include/linux/mutex.h,like IS_MUTEX_DESTROY_REQUIRED and declare it near
> mutex_destroy definition itself.
That (a IS_MUTEX_DESTROY_REQUIRED define) is an interesting idea. Lets see for
v3 if the mutex maintainers will accept that and if not then I guess we will
just need to live with the unnecessary devres allocation.
Regards,
Hans
Re: [PATCH v2 01/10] devm-helpers: introduce devm_mutex_init
Hi George,
On 12/4/23 19:05, George Stark wrote:
> Using of devm API leads to certain order of releasing resources.
> So all dependent resources which are not devm-wrapped should be deleted
> with respect to devm-release order. Mutex is one of such objects that
> often is bound to other resources and has no own devm wrapping.
> Since mutex_destroy() actually does nothing in non-debug builds
> frequently calling mutex_destroy() is just ignored which is safe for now
> but wrong formally and can lead to a problem if mutex_destroy() is
> extended so introduce devm_mutex_init().
>
> Signed-off-by: George Stark
> ---
> include/linux/devm-helpers.h | 18 ++
> 1 file changed, 18 insertions(+)
>
> diff --git a/include/linux/devm-helpers.h b/include/linux/devm-helpers.h
> index 74891802200d..2f56e476776f 100644
> --- a/include/linux/devm-helpers.h
> +++ b/include/linux/devm-helpers.h
> @@ -76,4 +76,22 @@ static inline int devm_work_autocancel(struct device *dev,
> return devm_add_action(dev, devm_work_drop, w);
> }
>
> +static inline void devm_mutex_release(void *res)
> +{
> + mutex_destroy(res);
> +}
> +
> +/**
> + * devm_mutex_init - Resource-managed mutex initialization
> + * @dev: Device which lifetime work is bound to
> + * @lock:Pointer to a mutex
> + *
> + * Initialize mutex which is automatically destroyed when driver is detached.
> + */
> +static inline int devm_mutex_init(struct device *dev, struct mutex *lock)
> +{
> + mutex_init(lock);
> + return devm_add_action_or_reset(dev, devm_mutex_release, lock);
> +}
> +
> #endif
mutex_destroy() only actually does anything if CONFIG_DEBUG_MUTEXES
is set, otherwise it is an empty inline-stub.
Adding a devres resource to the device just to call an empty inline
stub which is a no-op seems like a waste of resources. IMHO it
would be better to change this to:
static inline int devm_mutex_init(struct device *dev, struct mutex *lock)
{
mutex_init(lock);
#ifdef CONFIG_DEBUG_MUTEXES
return devm_add_action_or_reset(dev, devm_mutex_release, lock);
#else
return 0;
#endif
}
To avoid the unnecessary devres allocation when
CONFIG_DEBUG_MUTEXES is not set.
Regards,
Hans
Re: [PATCH v2 01/10] devm-helpers: introduce devm_mutex_init
Hi, On 12/6/23 08:56, George Stark wrote: > Hello Andy > > Thanks for the review. > > On 12/4/23 21:11, Andy Shevchenko wrote: >> On Mon, Dec 4, 2023 at 8:07 PM George Stark >> wrote: >>> >>> Using of devm API leads to certain order of releasing resources. >>> So all dependent resources which are not devm-wrapped should be deleted >>> with respect to devm-release order. Mutex is one of such objects that >>> often is bound to other resources and has no own devm wrapping. >>> Since mutex_destroy() actually does nothing in non-debug builds >>> frequently calling mutex_destroy() is just ignored which is safe for now >>> but wrong formally and can lead to a problem if mutex_destroy() is >>> extended so introduce devm_mutex_init(). >> >> ... >> >> Do you need to include mutex.h? > It's already included in linux/device.h which is included in devm-helpers. > Should I include mutex.h explicitly? Yes you must explicitly include all headers you use definitions from. Relying on other headers to do this for you is error prone. Regards, Hans
Re: [PATCH v2 01/10] devm-helpers: introduce devm_mutex_init
On 12/6/23 14:55, Hans de Goede wrote:
Hi,
On 12/6/23 19:58, George Stark wrote:
Hello Hans
Thanks for the review.
On 12/6/23 18:01, Hans de Goede wrote:
Hi George,
On 12/4/23 19:05, George Stark wrote:
Using of devm API leads to certain order of releasing resources.
So all dependent resources which are not devm-wrapped should be deleted
with respect to devm-release order. Mutex is one of such objects that
often is bound to other resources and has no own devm wrapping.
Since mutex_destroy() actually does nothing in non-debug builds
frequently calling mutex_destroy() is just ignored which is safe for now
but wrong formally and can lead to a problem if mutex_destroy() is
extended so introduce devm_mutex_init().
Signed-off-by: George Stark
---
include/linux/devm-helpers.h | 18 ++
1 file changed, 18 insertions(+)
diff --git a/include/linux/devm-helpers.h b/include/linux/devm-helpers.h
index 74891802200d..2f56e476776f 100644
--- a/include/linux/devm-helpers.h
+++ b/include/linux/devm-helpers.h
@@ -76,4 +76,22 @@ static inline int devm_work_autocancel(struct device *dev,
return devm_add_action(dev, devm_work_drop, w);
}
+static inline void devm_mutex_release(void *res)
+{
+ mutex_destroy(res);
+}
+
+/**
+ * devm_mutex_init - Resource-managed mutex initialization
+ * @dev: Device which lifetime work is bound to
+ * @lock: Pointer to a mutex
+ *
+ * Initialize mutex which is automatically destroyed when driver is detached.
+ */
+static inline int devm_mutex_init(struct device *dev, struct mutex *lock)
+{
+ mutex_init(lock);
+ return devm_add_action_or_reset(dev, devm_mutex_release, lock);
+}
+
#endif
mutex_destroy() only actually does anything if CONFIG_DEBUG_MUTEXES
is set, otherwise it is an empty inline-stub.
Adding a devres resource to the device just to call an empty inline
stub which is a no-op seems like a waste of resources. IMHO it
would be better to change this to:
static inline int devm_mutex_init(struct device *dev, struct mutex *lock)
{
mutex_init(lock);
#ifdef CONFIG_DEBUG_MUTEXES
return devm_add_action_or_reset(dev, devm_mutex_release, lock);
#else
return 0;
#endif
}
To avoid the unnecessary devres allocation when
CONFIG_DEBUG_MUTEXES is not set.
Honestly saying I don't like unnecessary devres allocation either but the
proposed approach has its own price:
1) we'll have more than one place with branching if mutex_destroy is empty or
not using indirect condition. If suddenly mutex_destroy is extended for
non-debug code (in upstream branch or e.g. by someone for local debug) than
there'll be a problem.
2) If mutex_destroy is empty or not depends on CONFIG_PREEMPT_RT option too.
When CONFIG_PREEMPT_RT is on mutex_destroy is always empty.
As I see it only the mutex interface (mutex.h) has to say definitely if
mutex_destroy must be called. Probably we could add some define to
include/linux/mutex.h,like IS_MUTEX_DESTROY_REQUIRED and declare it near
mutex_destroy definition itself.
That (a IS_MUTEX_DESTROY_REQUIRED define) is an interesting idea. Lets see for
v3 if the mutex maintainers will accept that and if not then I guess we will
just need to live with the unnecessary devres allocation.
The purpose of calling mutex_destroy() is to mark a mutex as being
destroyed so that any subsequent call to mutex_lock/unlock will cause a
warning to be printed when CONFIG_DEBUG_MUTEXES is defined. I would not
say that mutex_destroy() is required. Rather it is a nice to have for
catching programming error.
Cheers,
Longman
Re: [PATCH v2 01/10] devm-helpers: introduce devm_mutex_init
Hello Hans
Thanks for the review.
On 12/6/23 18:01, Hans de Goede wrote:
Hi George,
On 12/4/23 19:05, George Stark wrote:
Using of devm API leads to certain order of releasing resources.
So all dependent resources which are not devm-wrapped should be deleted
with respect to devm-release order. Mutex is one of such objects that
often is bound to other resources and has no own devm wrapping.
Since mutex_destroy() actually does nothing in non-debug builds
frequently calling mutex_destroy() is just ignored which is safe for now
but wrong formally and can lead to a problem if mutex_destroy() is
extended so introduce devm_mutex_init().
Signed-off-by: George Stark
---
include/linux/devm-helpers.h | 18 ++
1 file changed, 18 insertions(+)
diff --git a/include/linux/devm-helpers.h b/include/linux/devm-helpers.h
index 74891802200d..2f56e476776f 100644
--- a/include/linux/devm-helpers.h
+++ b/include/linux/devm-helpers.h
@@ -76,4 +76,22 @@ static inline int devm_work_autocancel(struct device *dev,
return devm_add_action(dev, devm_work_drop, w);
}
+static inline void devm_mutex_release(void *res)
+{
+ mutex_destroy(res);
+}
+
+/**
+ * devm_mutex_init - Resource-managed mutex initialization
+ * @dev: Device which lifetime work is bound to
+ * @lock: Pointer to a mutex
+ *
+ * Initialize mutex which is automatically destroyed when driver is detached.
+ */
+static inline int devm_mutex_init(struct device *dev, struct mutex *lock)
+{
+ mutex_init(lock);
+ return devm_add_action_or_reset(dev, devm_mutex_release, lock);
+}
+
#endif
mutex_destroy() only actually does anything if CONFIG_DEBUG_MUTEXES
is set, otherwise it is an empty inline-stub.
Adding a devres resource to the device just to call an empty inline
stub which is a no-op seems like a waste of resources. IMHO it
would be better to change this to:
static inline int devm_mutex_init(struct device *dev, struct mutex *lock)
{
mutex_init(lock);
#ifdef CONFIG_DEBUG_MUTEXES
return devm_add_action_or_reset(dev, devm_mutex_release, lock);
#else
return 0;
#endif
}
To avoid the unnecessary devres allocation when
CONFIG_DEBUG_MUTEXES is not set.
Honestly saying I don't like unnecessary devres allocation either but
the proposed approach has its own price:
1) we'll have more than one place with branching if mutex_destroy is
empty or not using indirect condition. If suddenly mutex_destroy is
extended for non-debug code (in upstream branch or e.g. by someone for
local debug) than there'll be a problem.
2) If mutex_destroy is empty or not depends on CONFIG_PREEMPT_RT option
too. When CONFIG_PREEMPT_RT is on mutex_destroy is always empty.
As I see it only the mutex interface (mutex.h) has to say definitely if
mutex_destroy must be called. Probably we could add some define to
include/linux/mutex.h,like IS_MUTEX_DESTROY_REQUIRED and declare it near
mutex_destroy definition itself.
I tried to put devm_mutex_init itself in mutex.h and it could've helped
too but it's not the place for devm API.
Regards,
Hans
--
Best regards
George
Re: [PATCH v3 5/7] kexec_file, ricv: print out debugging message if required
On Wed, Dec 06, 2023 at 11:37:52PM +0800, Baoquan He wrote:
> On 12/04/23 at 04:14pm, Conor Dooley wrote:
> > On Mon, Dec 04, 2023 at 11:38:05PM +0800, Baoquan He wrote:
> > > On 12/01/23 at 10:38am, Conor Dooley wrote:
> > > > On Thu, Nov 30, 2023 at 10:39:53AM +0800, Baoquan He wrote:
> > > >
> > > > $subject has a typo in the arch bit :)
> > >
> > > Indeed, will fix if need report. Thanks for careful checking.
> > >
> > > >
> > > > > Replace pr_debug() with the newly added kexec_dprintk() in kexec_file
> > > > > loading related codes.
> > > >
> > > > Commit messages should be understandable in isolation, but this only
> > > > explains (part of) what is obvious in the diff. Why is this change
> > > > being made?
> > >
> > > The purpose has been detailedly described in cover letter and patch 1
> > > log. Andrew has picked these patches into his tree and grabbed the cover
> > > letter log into the relevant commit for people's later checking. All
> > > these seven patches will be present in mainline together. This is common
> > > way when posting patch series? Please let me know if I misunderstand
> > > anything.
> >
> > Each patch having a commit message that explains why a change is being
> > made is the expectation. It is especially useful to explain the why
> > here, since it is not just a mechanical conversion of pr_debug()s as the
> > commit message suggests.
>
> Sounds reasonable. I rephrase the patch 3 log as below, do you think
> it's OK to you?
Yes, but with one comment.
>
> I will also adjust patch logs on other ARCH once this one is done.
> Thanks.
>
> =
> Subject: [PATCH v3 5/7] kexec_file, ricv: print out debugging message if
> required
>
> Then when specifying '-d' for kexec_file_load interface, loaded
> locations of kernel/initrd/cmdline etc can be printed out to help debug.
>
> Here replace pr_debug() with the newly added kexec_dprintk() in kexec_file
> loading related codes.
>
> And also replace pr_notice() with kexec_dprintk() in elf_kexec_load()
> because it's make sense to always print out loaded location of purgatory
> and device tree even though users don't expect the message.
This seems to contradict what you said in your earlier mail, about
moving these from notice to debug. I think you missed a negation in your
new version of the commit message. What you said in response to me seems
like a more complete explanation anyway:
always printing out the loaded location of purgatory and
device tree doesn't make sense. It will be confusing when users
see these even when they do normal kexec/kdump loading.
Thanks,
Conor.
> And also remove kexec_image_info() because the content has been printed
> out in generic code.
>
>
>
> >
> > > >
> > > > >
> > > > > And also remove kexec_image_info() because the content has been
> > > > > printed
> > > > > out in generic code.
> > > > >
> > > > > Signed-off-by: Baoquan He
> > > > > ---
> > > > > arch/riscv/kernel/elf_kexec.c | 11 ++-
> > > > > arch/riscv/kernel/machine_kexec.c | 26 --
> > > > > 2 files changed, 6 insertions(+), 31 deletions(-)
> > > > >
> > > > > diff --git a/arch/riscv/kernel/elf_kexec.c
> > > > > b/arch/riscv/kernel/elf_kexec.c
> > > > > index e60fbd8660c4..5bd1ec3341fe 100644
> > > > > --- a/arch/riscv/kernel/elf_kexec.c
> > > > > +++ b/arch/riscv/kernel/elf_kexec.c
> > > > > @@ -216,7 +216,6 @@ static void *elf_kexec_load(struct kimage *image,
> > > > > char *kernel_buf,
> > > > > if (ret)
> > > > > goto out;
> > > > > kernel_start = image->start;
> > > > > - pr_notice("The entry point of kernel at 0x%lx\n", image->start);
> > > > >
> > > > > /* Add the kernel binary to the image */
> > > > > ret = riscv_kexec_elf_load(image, , _info,
> > > > > @@ -252,8 +251,8 @@ static void *elf_kexec_load(struct kimage *image,
> > > > > char *kernel_buf,
> > > > > image->elf_load_addr = kbuf.mem;
> > > > > image->elf_headers_sz = headers_sz;
> > > > >
> > > > > - pr_debug("Loaded elf core header at 0x%lx bufsz=0x%lx
> > > > > memsz=0x%lx\n",
> > > > > - image->elf_load_addr, kbuf.bufsz, kbuf.memsz);
> > > > > + kexec_dprintk("Loaded elf core header at 0x%lx
> > > > > bufsz=0x%lx memsz=0x%lx\n",
> > > > > + image->elf_load_addr, kbuf.bufsz,
> > > > > kbuf.memsz);
> > > > >
> > > > > /* Setup cmdline for kdump kernel case */
> > > > > modified_cmdline = setup_kdump_cmdline(image, cmdline,
> > > > > @@ -275,6 +274,8 @@ static void *elf_kexec_load(struct kimage *image,
> > > > > char *kernel_buf,
> > > > > pr_err("Error loading purgatory ret=%d\n", ret);
> > > > > goto out;
> > > > > }
> > > > > + kexec_dprintk("Loaded purgatory at 0x%lx\n", kbuf.mem);
> > > > > +
> > > > > ret =
Re: [PATCH 4/4] powerpc/Makefile: Auto detect cross compiler
On Wed, Dec 06, 2023 at 10:55:48PM +1100, Michael Ellerman wrote: > Look for various combinations, matching: > powerpc(64(le)?)?(-unknown)?-linux(-gnu)?- > > There are more possibilities, but the above is known to find a compiler > on Fedora and Ubuntu (which use linux-gnu-), and also detects the > kernel.org cross compilers (which use linux-). $ sh ../config.sub powerpc64-linux powerpc64-unknown-linux-gnu I am very very lazy, so buildall uses short names everywhere :-) Btw, can you build the kernel for a config that differs in 32/64 or BE/LE with the default your compiler uses? There is no reason this shouldn't work (you don't use any system libraries), but you need to use the correct compiler command-line flags for that always. Acked-by: Segher Boessenkool Segher
Re: [PATCH 2/4] powerpc/vdso: No need to undef powerpc for 64-bit build
On Wed, Dec 06, 2023 at 10:55:46PM +1100, Michael Ellerman wrote: > But the 64-bit compiler doesn't define powerpc in the first place, Yes, only __powerpc__ (and __powerpc64__) :-) Segher
[PATCH v1 4/4] powerpc: Stop using of_root
Replace all usages of of_root by of_find_node_by_path("/")
Signed-off-by: Christophe Leroy
---
arch/powerpc/kernel/secure_boot.c| 8 ++--
arch/powerpc/kexec/ranges.c | 8 +---
arch/powerpc/mm/drmem.c | 10 +-
arch/powerpc/mm/numa.c | 6 --
arch/powerpc/platforms/52xx/efika.c | 4 +++-
arch/powerpc/platforms/pasemi/pci.c | 4 +++-
arch/powerpc/platforms/pseries/lparcfg.c | 6 +-
arch/powerpc/platforms/pseries/setup.c | 12 +---
8 files changed, 40 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/kernel/secure_boot.c
b/arch/powerpc/kernel/secure_boot.c
index f9af305d9579..9e0efb657f39 100644
--- a/arch/powerpc/kernel/secure_boot.c
+++ b/arch/powerpc/kernel/secure_boot.c
@@ -32,8 +32,10 @@ bool is_ppc_secureboot_enabled(void)
if (enabled)
goto out;
- if (!of_property_read_u32(of_root, "ibm,secure-boot", ))
+ node = of_find_node_by_path("/");
+ if (!of_property_read_u32(node, "ibm,secure-boot", ))
enabled = (secureboot > 1);
+ of_node_put(node);
out:
pr_info("Secure boot mode %s\n", enabled ? "enabled" : "disabled");
@@ -54,8 +56,10 @@ bool is_ppc_trustedboot_enabled(void)
if (enabled)
goto out;
- if (!of_property_read_u32(of_root, "ibm,trusted-boot", ))
+ node = of_find_node_by_path("/");
+ if (!of_property_read_u32(node, "ibm,trusted-boot", ))
enabled = (trustedboot > 0);
+ of_node_put(node);
out:
pr_info("Trusted boot mode %s\n", enabled ? "enabled" : "disabled");
diff --git a/arch/powerpc/kexec/ranges.c b/arch/powerpc/kexec/ranges.c
index fb3e12f15214..33b780049aaf 100644
--- a/arch/powerpc/kexec/ranges.c
+++ b/arch/powerpc/kexec/ranges.c
@@ -385,14 +385,16 @@ int add_opal_mem_range(struct crash_mem **mem_ranges)
int add_reserved_mem_ranges(struct crash_mem **mem_ranges)
{
int n_mem_addr_cells, n_mem_size_cells, i, len, cells, ret = 0;
+ struct device_node *root = of_find_node_by_path("/");
const __be32 *prop;
- prop = of_get_property(of_root, "reserved-ranges", );
+ prop = of_get_property(root, "reserved-ranges", );
+ n_mem_addr_cells = of_n_addr_cells(root);
+ n_mem_size_cells = of_n_size_cells(root);
+ of_node_put(root);
if (!prop)
return 0;
- n_mem_addr_cells = of_n_addr_cells(of_root);
- n_mem_size_cells = of_n_size_cells(of_root);
cells = n_mem_addr_cells + n_mem_size_cells;
/* Each reserved range is an (address,size) pair */
diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
index fde7790277f7..c110ab8fa8a3 100644
--- a/arch/powerpc/mm/drmem.c
+++ b/arch/powerpc/mm/drmem.c
@@ -393,17 +393,17 @@ static const __be32 *of_get_usable_memory(struct
device_node *dn)
int walk_drmem_lmbs(struct device_node *dn, void *data,
int (*func)(struct drmem_lmb *, const __be32 **, void *))
{
+ struct device_node *root = of_find_node_by_path("/");
const __be32 *prop, *usm;
int ret = -ENODEV;
- if (!of_root)
+ if (!root)
return ret;
/* Get the address & size cells */
- of_node_get(of_root);
- n_root_addr_cells = of_n_addr_cells(of_root);
- n_root_size_cells = of_n_size_cells(of_root);
- of_node_put(of_root);
+ n_root_addr_cells = of_n_addr_cells(root);
+ n_root_size_cells = of_n_size_cells(root);
+ of_node_put(root);
if (init_drmem_lmb_size(dn))
return ret;
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index f6c4ace3b221..a490724e84ad 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -,7 +,7 @@ static void __init setup_node_data(int nid, u64
start_pfn, u64 end_pfn)
static void __init find_possible_nodes(void)
{
- struct device_node *rtas;
+ struct device_node *rtas, *root;
const __be32 *domains = NULL;
int prop_length, max_nodes;
u32 i;
@@ -1132,10 +1132,12 @@ static void __init find_possible_nodes(void)
* If the LPAR is migratable, new nodes might be activated after a LPM,
* so we should consider the max number in that case.
*/
- if (!of_get_property(of_root, "ibm,migratable-partition", NULL))
+ root = of_find_node_by_path("/");
+ if (!of_get_property(root, "ibm,migratable-partition", NULL))
domains = of_get_property(rtas,
"ibm,current-associativity-domains",
_length);
+ of_node_put(root);
if (!domains) {
domains = of_get_property(rtas, "ibm,max-associativity-domains",
_length);
diff --git a/arch/powerpc/platforms/52xx/efika.c
b/arch/powerpc/platforms/52xx/efika.c
index
[PATCH v1 3/4] powerpc/machdep: Define 'compatibles' property in ppc_md and use it
Most probe functions that do not use the 'compatible' string do
nothing else than checking whether the machine is compatible with
one of the strings in a NULL terminated table of strings.
Define that table of strings in ppc_md structure and check it directly
from probe_machine() instead of using ppc_md.probe() for that.
Keep checking in ppc_md.probe() only for more complex probing.
All .compatible could be replaced with a single element NULL
terminated list but that's not worth the churn. Can be do incrementaly
in follow-up patches.
Signed-off-by: Christophe Leroy
---
arch/powerpc/include/asm/machdep.h| 1 +
arch/powerpc/kernel/setup-common.c| 2 ++
arch/powerpc/platforms/40x/ppc40x_simple.c| 9 +++--
arch/powerpc/platforms/512x/mpc512x_generic.c | 4 +---
arch/powerpc/platforms/52xx/lite5200.c| 10 +-
arch/powerpc/platforms/52xx/mpc5200_simple.c | 10 +-
arch/powerpc/platforms/83xx/mpc830x_rdb.c | 10 +-
arch/powerpc/platforms/83xx/mpc831x_rdb.c | 10 +-
arch/powerpc/platforms/83xx/mpc837x_rdb.c | 10 +-
arch/powerpc/platforms/85xx/corenet_generic.c | 2 +-
arch/powerpc/platforms/85xx/tqm85xx.c | 10 +-
11 files changed, 14 insertions(+), 64 deletions(-)
diff --git a/arch/powerpc/include/asm/machdep.h
b/arch/powerpc/include/asm/machdep.h
index d31a5ec1550d..1862f94335ee 100644
--- a/arch/powerpc/include/asm/machdep.h
+++ b/arch/powerpc/include/asm/machdep.h
@@ -22,6 +22,7 @@ struct pci_host_bridge;
struct machdep_calls {
const char *name;
const char *compatible;
+ const char * const *compatibles;
#ifdef CONFIG_PPC64
#ifdef CONFIG_PM
void(*iommu_restore)(void);
diff --git a/arch/powerpc/kernel/setup-common.c
b/arch/powerpc/kernel/setup-common.c
index 9b142b9d5187..ec02f9d8f55d 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -616,6 +616,8 @@ static __init void probe_machine(void)
DBG(" %s ...\n", machine_id->name);
if (machine_id->compatible &&
!of_machine_is_compatible(machine_id->compatible))
continue;
+ if (machine_id->compatibles &&
!of_machine_compatible_match(machine_id->compatibles))
+ continue;
memcpy(_md, machine_id, sizeof(struct machdep_calls));
if (ppc_md.probe && !ppc_md.probe())
continue;
diff --git a/arch/powerpc/platforms/40x/ppc40x_simple.c
b/arch/powerpc/platforms/40x/ppc40x_simple.c
index e454e9d2eff1..294ab2728588 100644
--- a/arch/powerpc/platforms/40x/ppc40x_simple.c
+++ b/arch/powerpc/platforms/40x/ppc40x_simple.c
@@ -59,16 +59,13 @@ static const char * const board[] __initconst = {
static int __init ppc40x_probe(void)
{
- if (of_device_compatible_match(of_root, board)) {
- pci_set_flags(PCI_REASSIGN_ALL_RSRC);
- return 1;
- }
-
- return 0;
+ pci_set_flags(PCI_REASSIGN_ALL_RSRC);
+ return 1;
}
define_machine(ppc40x_simple) {
.name = "PowerPC 40x Platform",
+ .compatibles = board,
.probe = ppc40x_probe,
.progress = udbg_progress,
.init_IRQ = uic_init_tree,
diff --git a/arch/powerpc/platforms/512x/mpc512x_generic.c
b/arch/powerpc/platforms/512x/mpc512x_generic.c
index 0d58ab257cd9..d4fa6c302ccf 100644
--- a/arch/powerpc/platforms/512x/mpc512x_generic.c
+++ b/arch/powerpc/platforms/512x/mpc512x_generic.c
@@ -32,9 +32,6 @@ static const char * const board[] __initconst = {
*/
static int __init mpc512x_generic_probe(void)
{
- if (!of_device_compatible_match(of_root, board))
- return 0;
-
mpc512x_init_early();
return 1;
@@ -42,6 +39,7 @@ static int __init mpc512x_generic_probe(void)
define_machine(mpc512x_generic) {
.name = "MPC512x generic",
+ .compatibles= board,
.probe = mpc512x_generic_probe,
.init = mpc512x_init,
.setup_arch = mpc512x_setup_arch,
diff --git a/arch/powerpc/platforms/52xx/lite5200.c
b/arch/powerpc/platforms/52xx/lite5200.c
index 0fd67b3ffc3e..0a161d82a3a8 100644
--- a/arch/powerpc/platforms/52xx/lite5200.c
+++ b/arch/powerpc/platforms/52xx/lite5200.c
@@ -172,17 +172,9 @@ static const char * const board[] __initconst = {
NULL,
};
-/*
- * Called very early, MMU is off, device-tree isn't unflattened
- */
-static int __init lite5200_probe(void)
-{
- return of_device_compatible_match(of_root, board);
-}
-
define_machine(lite5200) {
.name = "lite5200",
- .probe = lite5200_probe,
+ .compatibles= board,
.setup_arch = lite5200_setup_arch,
.discover_phbs = mpc52xx_setup_pci,
.init = mpc52xx_declare_of_platform_devices,
diff --git
[PATCH v1 2/4] of: Reimplement of_machine_is_compatible() using of_machine_compatible_match()
of_machine_compatible_match() works with a table of strings.
of_machine_is_compatible() is a simplier version with only one string.
Re-implement of_machine_is_compatible() by setting a table of strings
with a single string then using of_machine_compatible_match().
Suggested-by: Rob Herring
Signed-off-by: Christophe Leroy
---
drivers/of/base.c | 22 +-
include/linux/of.h | 15 ++-
2 files changed, 15 insertions(+), 22 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 9020be2eb4d5..73c3a754bad1 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -414,27 +414,7 @@ bool of_machine_compatible_match(const char *const
*compats)
return rc != 0;
}
-
-/**
- * of_machine_is_compatible - Test root of device tree for a given compatible
value
- * @compat: compatible string to look for in root node's compatible property.
- *
- * Return: A positive integer if the root node has the given value in its
- * compatible property.
- */
-int of_machine_is_compatible(const char *compat)
-{
- struct device_node *root;
- int rc = 0;
-
- root = of_find_node_by_path("/");
- if (root) {
- rc = of_device_is_compatible(root, compat);
- of_node_put(root);
- }
- return rc;
-}
-EXPORT_SYMBOL(of_machine_is_compatible);
+EXPORT_SYMBOL(of_machine_compatible_match);
/**
* __of_device_is_available - check if a device is available for use
diff --git a/include/linux/of.h b/include/linux/of.h
index e3418babc203..a0f70be5007f 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -402,9 +402,22 @@ extern void of_alias_scan(void * (*dt_alloc)(u64 size, u64
align));
extern int of_alias_get_id(struct device_node *np, const char *stem);
extern int of_alias_get_highest_id(const char *stem);
-extern int of_machine_is_compatible(const char *compat);
bool of_machine_compatible_match(const char *const *compats);
+/**
+ * of_machine_is_compatible - Test root of device tree for a given compatible
value
+ * @compat: compatible string to look for in root node's compatible property.
+ *
+ * Return: A positive integer if the root node has the given value in its
+ * compatible property.
+ */
+static inline int of_machine_is_compatible(const char *compat)
+{
+ const char *compats[] = { compat, NULL };
+
+ return of_machine_compatible_match(compats);
+}
+
extern int of_add_property(struct device_node *np, struct property *prop);
extern int of_remove_property(struct device_node *np, struct property *prop);
extern int of_update_property(struct device_node *np, struct property
*newprop);
--
2.41.0
[PATCH v1 1/4] of: Add of_machine_compatible_match()
From: Michael Ellerman
We have of_machine_is_compatible() to check if a machine is compatible
with a single compatible string. However some code is able to support
multiple compatible boards, and so wants to check for one of many
compatible strings.
So add of_machine_compatible_match() which takes a NULL terminated
array of compatible strings to check against the root node's
compatible property.
Compared to an open coded match this is slightly more self
documenting, and also avoids the caller needing to juggle the root
node either directly or via of_find_node_by_path().
Signed-off-by: Michael Ellerman
Signed-off-by: Christophe Leroy
---
drivers/of/base.c | 21 +
include/linux/of.h | 6 ++
2 files changed, 27 insertions(+)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 8d93cb6ea9cd..9020be2eb4d5 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -394,6 +394,27 @@ int of_device_compatible_match(const struct device_node
*device,
}
EXPORT_SYMBOL_GPL(of_device_compatible_match);
+/**
+ * of_machine_compatible_match - Test root of device tree against a compatible
array
+ * @compats: NULL terminated array of compatible strings to look for in root
node's compatible property.
+ *
+ * Returns true if the root node has any of the given compatible values in its
+ * compatible property.
+ */
+bool of_machine_compatible_match(const char *const *compats)
+{
+ struct device_node *root;
+ int rc = 0;
+
+ root = of_find_node_by_path("/");
+ if (root) {
+ rc = of_device_compatible_match(root, compats);
+ of_node_put(root);
+ }
+
+ return rc != 0;
+}
+
/**
* of_machine_is_compatible - Test root of device tree for a given compatible
value
* @compat: compatible string to look for in root node's compatible property.
diff --git a/include/linux/of.h b/include/linux/of.h
index 6a9ddf20e79a..e3418babc203 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -403,6 +403,7 @@ extern int of_alias_get_id(struct device_node *np, const
char *stem);
extern int of_alias_get_highest_id(const char *stem);
extern int of_machine_is_compatible(const char *compat);
+bool of_machine_compatible_match(const char *const *compats);
extern int of_add_property(struct device_node *np, struct property *prop);
extern int of_remove_property(struct device_node *np, struct property *prop);
@@ -808,6 +809,11 @@ static inline int of_remove_property(struct device_node
*np, struct property *pr
return 0;
}
+static inline bool of_machine_compatible_match(const char *const *compats)
+{
+ return false;
+}
+
static inline bool of_console_check(const struct device_node *dn, const char
*name, int index)
{
return false;
--
2.41.0
Re: [PATCH v3 5/7] kexec_file, ricv: print out debugging message if required
On 12/04/23 at 04:14pm, Conor Dooley wrote:
> On Mon, Dec 04, 2023 at 11:38:05PM +0800, Baoquan He wrote:
> > On 12/01/23 at 10:38am, Conor Dooley wrote:
> > > On Thu, Nov 30, 2023 at 10:39:53AM +0800, Baoquan He wrote:
> > >
> > > $subject has a typo in the arch bit :)
> >
> > Indeed, will fix if need report. Thanks for careful checking.
> >
> > >
> > > > Replace pr_debug() with the newly added kexec_dprintk() in kexec_file
> > > > loading related codes.
> > >
> > > Commit messages should be understandable in isolation, but this only
> > > explains (part of) what is obvious in the diff. Why is this change
> > > being made?
> >
> > The purpose has been detailedly described in cover letter and patch 1
> > log. Andrew has picked these patches into his tree and grabbed the cover
> > letter log into the relevant commit for people's later checking. All
> > these seven patches will be present in mainline together. This is common
> > way when posting patch series? Please let me know if I misunderstand
> > anything.
>
> Each patch having a commit message that explains why a change is being
> made is the expectation. It is especially useful to explain the why
> here, since it is not just a mechanical conversion of pr_debug()s as the
> commit message suggests.
Sounds reasonable. I rephrase the patch 3 log as below, do you think
it's OK to you?
I will also adjust patch logs on other ARCH once this one is done.
Thanks.
=
Subject: [PATCH v3 5/7] kexec_file, ricv: print out debugging message if
required
Then when specifying '-d' for kexec_file_load interface, loaded
locations of kernel/initrd/cmdline etc can be printed out to help debug.
Here replace pr_debug() with the newly added kexec_dprintk() in kexec_file
loading related codes.
And also replace pr_notice() with kexec_dprintk() in elf_kexec_load()
because it's make sense to always print out loaded location of purgatory
and device tree even though users don't expect the message.
And also remove kexec_image_info() because the content has been printed
out in generic code.
>
> > >
> > > >
> > > > And also remove kexec_image_info() because the content has been printed
> > > > out in generic code.
> > > >
> > > > Signed-off-by: Baoquan He
> > > > ---
> > > > arch/riscv/kernel/elf_kexec.c | 11 ++-
> > > > arch/riscv/kernel/machine_kexec.c | 26 --
> > > > 2 files changed, 6 insertions(+), 31 deletions(-)
> > > >
> > > > diff --git a/arch/riscv/kernel/elf_kexec.c
> > > > b/arch/riscv/kernel/elf_kexec.c
> > > > index e60fbd8660c4..5bd1ec3341fe 100644
> > > > --- a/arch/riscv/kernel/elf_kexec.c
> > > > +++ b/arch/riscv/kernel/elf_kexec.c
> > > > @@ -216,7 +216,6 @@ static void *elf_kexec_load(struct kimage *image,
> > > > char *kernel_buf,
> > > > if (ret)
> > > > goto out;
> > > > kernel_start = image->start;
> > > > - pr_notice("The entry point of kernel at 0x%lx\n", image->start);
> > > >
> > > > /* Add the kernel binary to the image */
> > > > ret = riscv_kexec_elf_load(image, , _info,
> > > > @@ -252,8 +251,8 @@ static void *elf_kexec_load(struct kimage *image,
> > > > char *kernel_buf,
> > > > image->elf_load_addr = kbuf.mem;
> > > > image->elf_headers_sz = headers_sz;
> > > >
> > > > - pr_debug("Loaded elf core header at 0x%lx bufsz=0x%lx
> > > > memsz=0x%lx\n",
> > > > -image->elf_load_addr, kbuf.bufsz, kbuf.memsz);
> > > > + kexec_dprintk("Loaded elf core header at 0x%lx
> > > > bufsz=0x%lx memsz=0x%lx\n",
> > > > + image->elf_load_addr, kbuf.bufsz,
> > > > kbuf.memsz);
> > > >
> > > > /* Setup cmdline for kdump kernel case */
> > > > modified_cmdline = setup_kdump_cmdline(image, cmdline,
> > > > @@ -275,6 +274,8 @@ static void *elf_kexec_load(struct kimage *image,
> > > > char *kernel_buf,
> > > > pr_err("Error loading purgatory ret=%d\n", ret);
> > > > goto out;
> > > > }
> > > > + kexec_dprintk("Loaded purgatory at 0x%lx\n", kbuf.mem);
> > > > +
> > > > ret = kexec_purgatory_get_set_symbol(image,
> > > > "riscv_kernel_entry",
> > > > _start,
> > > > sizeof(kernel_start), 0);
> > > > @@ -293,7 +294,7 @@ static void *elf_kexec_load(struct kimage *image,
> > > > char *kernel_buf,
> > > > if (ret)
> > > > goto out;
> > > > initrd_pbase = kbuf.mem;
> > >
> > > > - pr_notice("Loaded initrd at 0x%lx\n", initrd_pbase);
> > > > + kexec_dprintk("Loaded initrd at 0x%lx\n", initrd_pbase);
> > >
> > > This is not a pr_debug().
> > >
> > > > }
> > > >
> > > > /* Add the DTB to the image */
> > > > @@ -318,7
Re: [PATCH V4] tools/perf: Add perf binary dependent rule for shellcheck log in Makefile.perf
Em Wed, Dec 06, 2023 at 10:15:06AM +0530, Athira Rajeev escreveu: > > > > On 06-Dec-2023, at 3:20 AM, Arnaldo Carvalho de Melo > > wrote: > > > > Em Mon, Nov 27, 2023 at 11:12:57AM +, James Clark escreveu: > >> On 23/11/2023 16:02, Athira Rajeev wrote: > >>> --- a/tools/perf/Makefile.perf > >>> @@ -1134,6 +1152,7 @@ bpf-skel-clean: > >>> $(call QUIET_CLEAN, bpf-skel) $(RM) -r $(SKEL_TMP_OUT) $(SKELETONS) > >>> > >>> clean:: $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clean > >>> $(LIBSYMBOL)-clean $(LIBPERF)-clean fixdep-clean python-clean > >>> bpf-skel-clean tests-coresight-targets-clean > >>> + $(Q)$(MAKE) -f $(srctree)/tools/perf/tests/Makefile.tests clean > >>> $(call QUIET_CLEAN, core-objs) $(RM) $(LIBPERF_A) $(OUTPUT)perf-archive > >>> $(OUTPUT)perf-iostat $(LANG_BINDINGS) > >>> $(Q)find $(or $(OUTPUT),.) -name '*.o' -delete -o -name '\.*.cmd' -delete > >>> -o -name '\.*.d' -delete > >>> $(Q)$(RM) $(OUTPUT).config-detected > > > > While merging perf-tools-next with torvalds/master I noticed that maybe > > we better have the above added line as: > > > > + $(call QUIET_CLEAN, tests) $(Q)$(MAKE) -f > > $(srctree)/tools/perf/tests/Makefile.tests clean > > > > No? > > > > Anyway I'm merging as-is, but it just hit my eye while merging, > > > > - Arnaldo > Hi Arnaldo > > As Ian pointed we removed Makefile.tests as part of : > https://lore.kernel.org/lkml/20231129213428.2227448-1-irog...@google.com/ Right, thanks for checking, see my reply to Ian for further details. - Arnaldo
Re: [PATCH V4] tools/perf: Add perf binary dependent rule for shellcheck log in Makefile.perf
Em Tue, Dec 05, 2023 at 02:09:01PM -0800, Ian Rogers escreveu: > On Tue, Dec 5, 2023 at 1:50 PM Arnaldo Carvalho de Melo > wrote: > > > > Em Mon, Nov 27, 2023 at 11:12:57AM +, James Clark escreveu: > > > On 23/11/2023 16:02, Athira Rajeev wrote: > > > > --- a/tools/perf/Makefile.perf > > > > @@ -1134,6 +1152,7 @@ bpf-skel-clean: > > > > $(call QUIET_CLEAN, bpf-skel) $(RM) -r $(SKEL_TMP_OUT) $(SKELETONS) > > > > > > > > clean:: $(LIBAPI)-clean $(LIBBPF)-clean $(LIBSUBCMD)-clean > > > > $(LIBSYMBOL)-clean $(LIBPERF)-clean fixdep-clean python-clean > > > > bpf-skel-clean tests-coresight-targets-clean > > > > + $(Q)$(MAKE) -f $(srctree)/tools/perf/tests/Makefile.tests clean > > > > $(call QUIET_CLEAN, core-objs) $(RM) $(LIBPERF_A) > > > > $(OUTPUT)perf-archive $(OUTPUT)perf-iostat $(LANG_BINDINGS) > > > > $(Q)find $(or $(OUTPUT),.) -name '*.o' -delete -o -name '\.*.cmd' > > > > -delete -o -name '\.*.d' -delete > > > > $(Q)$(RM) $(OUTPUT).config-detected > > > > While merging perf-tools-next with torvalds/master I noticed that maybe > > we better have the above added line as: > > > > + $(call QUIET_CLEAN, tests) $(Q)$(MAKE) -f > > $(srctree)/tools/perf/tests/Makefile.tests clean > > > > No? > > > > Anyway I'm merging as-is, but it just hit my eye while merging, > > > > - Arnaldo > > Makefile.tests was removed in these recent patches adding support for > the OUTPUT directory: > https://lore.kernel.org/lkml/9c33887f-8a88-4973-8593-7936e36af...@linux.vnet.ibm.com/ Right, I made a mistake and was doing the merge on a different branch, now that I tried it on my latest perf-tools-next local branch all is well and the other merge conflict gets auto-resolved (arm64-sysreg-defs-clean stuff in tools/perf/Makefile.perf "clean" target). Thanks for checking, - Arnaldo
[PATCH 4/4] powerpc/Makefile: Auto detect cross compiler
If no cross compiler is specified, try to auto detect one. Look for various combinations, matching: powerpc(64(le)?)?(-unknown)?-linux(-gnu)?- There are more possibilities, but the above is known to find a compiler on Fedora and Ubuntu (which use linux-gnu-), and also detects the kernel.org cross compilers (which use linux-). This allows cross compiling with simply: # Ubuntu $ sudo apt install gcc-powerpc-linux-gnu # Fedora $ sudo dnf install gcc-powerpc64-linux-gnu $ make ARCH=powerpc defconfig $ make ARCH=powerpc -j 4 Inspired by arch/parisc/Makefile. Signed-off-by: Michael Ellerman --- arch/powerpc/Makefile | 11 +++ 1 file changed, 11 insertions(+) diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile index 48c06f5a0dc1..051247027da0 100644 --- a/arch/powerpc/Makefile +++ b/arch/powerpc/Makefile @@ -10,6 +10,17 @@ # Rewritten by Cort Dougan and Paul Mackerras # +ifdef cross_compiling + ifeq ($(CROSS_COMPILE),) +# Auto detect cross compiler prefix. +# Look for: (powerpc(64(le)?)?)(-unknown)?-linux(-gnu)?- +CC_ARCHES := powerpc powerpc64 powerpc64le +CC_SUFFIXES := linux linux-gnu unknown-linux-gnu +CROSS_COMPILE := $(call cc-cross-prefix, $(foreach a,$(CC_ARCHES), \ + $(foreach s,$(CC_SUFFIXES),$(a)-$(s)-))) + endif +endif + HAS_BIARCH := $(call cc-option-yn, -m32) # Set default 32 bits cross compilers for vdso and boot wrapper -- 2.43.0
[PATCH 3/4] powerpc/Makefile: Default to ppc64le_defconfig when cross building
If the kernel is being cross compiled, there is no information from uname on which defconfig is most appropriate, so the Makefile defaults to ppc64. However these days almost all distros that support powerpc are little endian, so it's more likely that defaulting to ppc64le_defconfig will produce something useful for a user. Signed-off-by: Michael Ellerman --- arch/powerpc/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile index b0bc17c35ed7..48c06f5a0dc1 100644 --- a/arch/powerpc/Makefile +++ b/arch/powerpc/Makefile @@ -16,9 +16,9 @@ HAS_BIARCH:= $(call cc-option-yn, -m32) CROSS32_COMPILE ?= # If we're on a ppc/ppc64/ppc64le machine use that defconfig, otherwise just use -# ppc64_defconfig because we have nothing better to go on. +# ppc64le_defconfig because we have nothing better to go on. uname := $(shell uname -m) -KBUILD_DEFCONFIG := $(if $(filter ppc%,$(uname)),$(uname),ppc64)_defconfig +KBUILD_DEFCONFIG := $(if $(filter ppc%,$(uname)),$(uname),ppc64le)_defconfig new_nm := $(shell if $(NM) --help 2>&1 | grep -- '--synthetic' > /dev/null; then echo y; else echo n; fi) -- 2.43.0
[PATCH 2/4] powerpc/vdso: No need to undef powerpc for 64-bit build
The vdso Makefile adds -U$(ARCH) to CPPFLAGS for the vdso64.lds linker script. ARCH is always powerpc, so it becomes -Upowerpc, which means undefine the "powerpc" symbol. But the 64-bit compiler doesn't define powerpc in the first place, compare: $ gcc-5.1.0-nolibc/powerpc64-linux/bin/powerpc64-linux-gcc -m32 -E -dM - --- arch/powerpc/kernel/vdso/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/kernel/vdso/Makefile b/arch/powerpc/kernel/vdso/Makefile index 0c7d82c270c3..1b93655c2857 100644 --- a/arch/powerpc/kernel/vdso/Makefile +++ b/arch/powerpc/kernel/vdso/Makefile @@ -71,7 +71,7 @@ AS64FLAGS := -D__VDSO64__ targets += vdso32.lds CPPFLAGS_vdso32.lds += -P -C -Upowerpc targets += vdso64.lds -CPPFLAGS_vdso64.lds += -P -C -U$(ARCH) +CPPFLAGS_vdso64.lds += -P -C # link rule for the .so file, .lds has to be first $(obj)/vdso32.so.dbg: $(src)/vdso32.lds $(obj-vdso32) $(obj)/vgettimeofday-32.o FORCE -- 2.43.0
[PATCH 1/4] powerpc/Makefile: Don't use $(ARCH) unnecessarily
There's no need to use $(ARCH) for references to the arch directory in the source tree, it is always arch/powerpc. Signed-off-by: Michael Ellerman --- arch/powerpc/Makefile | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile index f19dbaa1d541..b0bc17c35ed7 100644 --- a/arch/powerpc/Makefile +++ b/arch/powerpc/Makefile @@ -161,7 +161,7 @@ CFLAGS-y += $(CONFIG_TUNE_CPU) asinstr := $(call as-instr,lis 9$(comma)foo@high,-DHAVE_AS_ATHIGH=1) -KBUILD_CPPFLAGS+= -I $(srctree)/arch/$(ARCH) $(asinstr) +KBUILD_CPPFLAGS+= -I $(srctree)/arch/powerpc $(asinstr) KBUILD_AFLAGS += $(AFLAGS-y) KBUILD_CFLAGS += $(call cc-option,-msoft-float) KBUILD_CFLAGS += $(CFLAGS-y) @@ -232,7 +232,7 @@ BOOT_TARGETS2 := zImage% dtbImage% treeImage.% cuImage.% simpleImage.% uImage.% PHONY += $(BOOT_TARGETS1) $(BOOT_TARGETS2) -boot := arch/$(ARCH)/boot +boot := arch/powerpc/boot $(BOOT_TARGETS1): vmlinux $(Q)$(MAKE) $(build)=$(boot) $(patsubst %,$(boot)/%,$@) @@ -336,7 +336,7 @@ PHONY += $(generated_configs) define archhelp echo '* zImage - Build default images selected by kernel config' - echo ' zImage.*- Compressed kernel image (arch/$(ARCH)/boot/zImage.*)' + echo ' zImage.*- Compressed kernel image (arch/powerpc/boot/zImage.*)' echo ' uImage - U-Boot native image format' echo ' cuImage.- Backwards compatible U-Boot image for older' echo 'versions which do not support device trees' @@ -347,12 +347,12 @@ define archhelp echo '(your) ~/bin/$(INSTALLKERNEL) or' echo '(distribution) /sbin/$(INSTALLKERNEL) or' echo 'install to $$(INSTALL_PATH) and run lilo' - echo ' *_defconfig - Select default config from arch/$(ARCH)/configs' + echo ' *_defconfig - Select default config from arch/powerpc/configs' echo '' echo ' Targets with embed a device tree blob inside the image' echo ' These targets support board with firmware that does not' echo ' support passing a device tree directly. Replace with the' - echo ' name of a dts file from the arch/$(ARCH)/boot/dts/ directory' + echo ' name of a dts file from the arch/powerpc/boot/dts/ directory' echo ' (minus the .dts extension).' echo $(foreach cfg,$(generated_configs), -- 2.43.0
Re: [PATCH v11 00/15] Add audio support in v4l2 framework
On Wed, Dec 6, 2023 at 4:09 PM Hans Verkuil wrote: > > Hi Shengjiu, > > On 22/11/2023 08:23, Shengjiu Wang wrote: > > Audio signal processing also has the requirement for memory to > > memory similar as Video. > > > > This asrc memory to memory (memory ->asrc->memory) case is a non > > real time use case. > > > > User fills the input buffer to the asrc module, after conversion, then asrc > > sends back the output buffer to user. So it is not a traditional ALSA > > playback > > and capture case. > > > > It is a specific use case, there is no reference in current kernel. > > v4l2 memory to memory is the closed implementation, v4l2 current > > support video, image, radio, tuner, touch devices, so it is not > > complicated to add support for this specific audio case. > > > > Because we had implemented the "memory -> asrc ->i2s device-> codec" > > use case in ALSA. Now the "memory->asrc->memory" needs > > to reuse the code in asrc driver, so the first 3 patches is for refining > > the code to make it can be shared by the "memory->asrc->memory" > > driver. > > > > The main change is in the v4l2 side, A /dev/vl4-audioX will be created, > > user applications only use the ioctl of v4l2 framework. > > > > Other change is to add memory to memory support for two kinds of i.MX ASRC > > module. > > I just wanted to let you know that this will have to be postponed until v6.8. > I need more time to work on the v4l-utils changes, esp. for the fraction bits > feature, and realistically that is not going to happen before my Christmas > vacation starts. No problem. Have a good vacation! Thanks. Best Regards Shengjiu Wang > > Apologies for the delay. > > > Regards, > > Hans > > > > > changes in v11 > > - add add-fixed-point-test-controls in vivid. > > - add v4l2_ctrl_fp_compose() helper function for min and max > > > > changes in v10 > > - remove FIXED_POINT type > > - change code base on media: v4l2-ctrls: add support for fraction_bits > > - fix issue reported by kernel test robot > > - remove module_alias > > > > changes in v9: > > - add MEDIA_ENT_F_PROC_AUDIO_RESAMPLER. > > - add MEDIA_INTF_T_V4L_AUDIO > > - add media controller support > > - refine the vim2m-audio to support 8k<->16k conversion. > > > > changes in v8: > > - refine V4L2_CAP_AUDIO_M2M to be 0x0008 > > - update doc for FIXED_POINT > > - address comments for imx-asrc > > > > changes in v7: > > - add acked-by from Mark > > - separate commit for fixed point, m2m audio class, audio rate controls > > - use INTEGER_MENU for rate, FIXED_POINT for rate offset > > - remove used fmts > > - address other comments for Hans > > > > changes in v6: > > - use m2m_prepare/m2m_unprepare/m2m_start/m2m_stop to replace > > m2m_start_part_one/m2m_stop_part_one, > > m2m_start_part_two/m2m_stop_part_two. > > - change V4L2_CTRL_TYPE_ASRC_RATE to V4L2_CTRL_TYPE_FIXED_POINT > > - fix warning by kernel test rebot > > - remove some unused format V4L2_AUDIO_FMT_XX > > - Get SNDRV_PCM_FORMAT from V4L2_AUDIO_FMT in driver. > > - rename audm2m to viaudm2m. > > > > changes in v5: > > - remove V4L2_AUDIO_FMT_LPCM > > - define audio pixel format like V4L2_AUDIO_FMT_S8... > > - remove rate and format in struct v4l2_audio_format. > > - Add V4L2_CID_ASRC_SOURCE_RATE and V4L2_CID_ASRC_DEST_RATE controls > > - updata document accordingly. > > > > changes in v4: > > - update document style > > - separate V4L2_AUDIO_FMT_LPCM and V4L2_CAP_AUDIO_M2M in separate commit > > > > changes in v3: > > - Modify documents for adding audio m2m support > > - Add audio virtual m2m driver > > - Defined V4L2_AUDIO_FMT_LPCM format type for audio. > > - Defined V4L2_CAP_AUDIO_M2M capability type for audio m2m case. > > - with modification in v4l-utils, pass v4l2-compliance test. > > > > changes in v2: > > - decouple the implementation in v4l2 and ALSA > > - implement the memory to memory driver as a platfrom driver > > and move it to driver/media > > - move fsl_asrc_common.h to include/sound folder > > > > Shengjiu Wang (15): > > ASoC: fsl_asrc: define functions for memory to memory usage > > ASoC: fsl_easrc: define functions for memory to memory usage > > ASoC: fsl_asrc: move fsl_asrc_common.h to include/sound > > ASoC: fsl_asrc: register m2m platform device > > ASoC: fsl_easrc: register m2m platform device > > media: uapi: Add V4L2_CAP_AUDIO_M2M capability flag > > media: v4l2: Add audio capture and output support > > media: uapi: Define audio sample format fourcc type > > media: uapi: Add V4L2_CTRL_CLASS_M2M_AUDIO > > media: uapi: Add audio rate controls support > > media: uapi: Declare interface types for Audio > > media: uapi: Add an entity type for audio resampler > > media: vivid: add fixed point test controls > > media: imx-asrc: Add memory to memory driver > > media: vim2m-audio: add virtual driver for audio memory to memory > > > > .../media/mediactl/media-types.rst| 11 + > > .../userspace-api/media/v4l/buffer.rst|6 + > >
Re: [PATCH 12/27] tty: hvc: convert to u8 and size_t
Le 06/12/2023 à 08:36, Jiri Slaby (SUSE) a écrit : > Switch character types to u8 and sizes to size_t. To conform to > characters/sizes in the rest of the tty layer. > > Signed-off-by: Jiri Slaby (SUSE) > Cc: Michael Ellerman > Cc: Nicholas Piggin > Cc: Christophe Leroy > Cc: Amit Shah > Cc: Arnd Bergmann > Cc: Paul Walmsley > Cc: Palmer Dabbelt > Cc: Albert Ou > Cc: linuxppc-dev@lists.ozlabs.org > Cc: virtualizat...@lists.linux.dev > Cc: linux-ri...@lists.infradead.org > --- > arch/powerpc/include/asm/hvconsole.h | 4 ++-- > arch/powerpc/include/asm/hvsi.h| 18 > arch/powerpc/include/asm/opal.h| 8 +--- > arch/powerpc/platforms/powernv/opal.c | 14 +++-- > arch/powerpc/platforms/pseries/hvconsole.c | 4 ++-- > drivers/char/virtio_console.c | 10 - > drivers/tty/hvc/hvc_console.h | 4 ++-- > drivers/tty/hvc/hvc_dcc.c | 24 +++--- > drivers/tty/hvc/hvc_iucv.c | 18 > drivers/tty/hvc/hvc_opal.c | 5 +++-- > drivers/tty/hvc/hvc_riscv_sbi.c| 9 > drivers/tty/hvc/hvc_rtas.c | 11 +- > drivers/tty/hvc/hvc_udbg.c | 9 > drivers/tty/hvc/hvc_vio.c | 18 > drivers/tty/hvc/hvc_xen.c | 23 +++-- > drivers/tty/hvc/hvsi_lib.c | 20 ++ > 16 files changed, 107 insertions(+), 92 deletions(-) > > diff --git a/arch/powerpc/include/asm/hvconsole.h > b/arch/powerpc/include/asm/hvconsole.h > index ccb2034506f0..d841a97010a0 100644 > --- a/arch/powerpc/include/asm/hvconsole.h > +++ b/arch/powerpc/include/asm/hvconsole.h > @@ -21,8 +21,8 @@ >* Vio firmware always attempts to fetch MAX_VIO_GET_CHARS chars. The > 'count' >* parm is included to conform to put_chars() function pointer template >*/ > -extern int hvc_get_chars(uint32_t vtermno, char *buf, int count); > -extern int hvc_put_chars(uint32_t vtermno, const char *buf, int count); > +extern ssize_t hvc_get_chars(uint32_t vtermno, u8 *buf, size_t count); > +extern ssize_t hvc_put_chars(uint32_t vtermno, const u8 *buf, size_t count); Would be a good opportunity to drop this pointless deprecated 'extern' keyword on all function prototypes you are changing. Christophe
Re: [PATCH v2 02/10] leds: aw2013: unlock mutex before destroying it
Hello Christophe
Thanks for the review
On 12/5/23 02:09, Christophe Leroy wrote:
Le 04/12/2023 à 19:05, George Stark a écrit :
In the probe() callback in case of error mutex is destroyed being locked
which is not allowed so unlock the mute before destroying.
Should there be a fixes: tag ? For instance on 59ea3c9faf32 ("leds: add
aw2013 driver") ?
Ack
Signed-off-by: George Stark
---
drivers/leds/leds-aw2013.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/leds/leds-aw2013.c b/drivers/leds/leds-aw2013.c
index 59765640b70f..c2bc0782c0cd 100644
--- a/drivers/leds/leds-aw2013.c
+++ b/drivers/leds/leds-aw2013.c
@@ -397,6 +397,7 @@ static int aw2013_probe(struct i2c_client *client)
regulator_disable(chip->vcc_regulator);
error:
+ mutex_unlock(>mutex);
mutex_destroy(>mutex);
return ret;
}
--
Best regards
George
Re: [PATCH v11 00/15] Add audio support in v4l2 framework
Hi Shengjiu, On 22/11/2023 08:23, Shengjiu Wang wrote: > Audio signal processing also has the requirement for memory to > memory similar as Video. > > This asrc memory to memory (memory ->asrc->memory) case is a non > real time use case. > > User fills the input buffer to the asrc module, after conversion, then asrc > sends back the output buffer to user. So it is not a traditional ALSA playback > and capture case. > > It is a specific use case, there is no reference in current kernel. > v4l2 memory to memory is the closed implementation, v4l2 current > support video, image, radio, tuner, touch devices, so it is not > complicated to add support for this specific audio case. > > Because we had implemented the "memory -> asrc ->i2s device-> codec" > use case in ALSA. Now the "memory->asrc->memory" needs > to reuse the code in asrc driver, so the first 3 patches is for refining > the code to make it can be shared by the "memory->asrc->memory" > driver. > > The main change is in the v4l2 side, A /dev/vl4-audioX will be created, > user applications only use the ioctl of v4l2 framework. > > Other change is to add memory to memory support for two kinds of i.MX ASRC > module. I just wanted to let you know that this will have to be postponed until v6.8. I need more time to work on the v4l-utils changes, esp. for the fraction bits feature, and realistically that is not going to happen before my Christmas vacation starts. Apologies for the delay. Regards, Hans > > changes in v11 > - add add-fixed-point-test-controls in vivid. > - add v4l2_ctrl_fp_compose() helper function for min and max > > changes in v10 > - remove FIXED_POINT type > - change code base on media: v4l2-ctrls: add support for fraction_bits > - fix issue reported by kernel test robot > - remove module_alias > > changes in v9: > - add MEDIA_ENT_F_PROC_AUDIO_RESAMPLER. > - add MEDIA_INTF_T_V4L_AUDIO > - add media controller support > - refine the vim2m-audio to support 8k<->16k conversion. > > changes in v8: > - refine V4L2_CAP_AUDIO_M2M to be 0x0008 > - update doc for FIXED_POINT > - address comments for imx-asrc > > changes in v7: > - add acked-by from Mark > - separate commit for fixed point, m2m audio class, audio rate controls > - use INTEGER_MENU for rate, FIXED_POINT for rate offset > - remove used fmts > - address other comments for Hans > > changes in v6: > - use m2m_prepare/m2m_unprepare/m2m_start/m2m_stop to replace > m2m_start_part_one/m2m_stop_part_one, m2m_start_part_two/m2m_stop_part_two. > - change V4L2_CTRL_TYPE_ASRC_RATE to V4L2_CTRL_TYPE_FIXED_POINT > - fix warning by kernel test rebot > - remove some unused format V4L2_AUDIO_FMT_XX > - Get SNDRV_PCM_FORMAT from V4L2_AUDIO_FMT in driver. > - rename audm2m to viaudm2m. > > changes in v5: > - remove V4L2_AUDIO_FMT_LPCM > - define audio pixel format like V4L2_AUDIO_FMT_S8... > - remove rate and format in struct v4l2_audio_format. > - Add V4L2_CID_ASRC_SOURCE_RATE and V4L2_CID_ASRC_DEST_RATE controls > - updata document accordingly. > > changes in v4: > - update document style > - separate V4L2_AUDIO_FMT_LPCM and V4L2_CAP_AUDIO_M2M in separate commit > > changes in v3: > - Modify documents for adding audio m2m support > - Add audio virtual m2m driver > - Defined V4L2_AUDIO_FMT_LPCM format type for audio. > - Defined V4L2_CAP_AUDIO_M2M capability type for audio m2m case. > - with modification in v4l-utils, pass v4l2-compliance test. > > changes in v2: > - decouple the implementation in v4l2 and ALSA > - implement the memory to memory driver as a platfrom driver > and move it to driver/media > - move fsl_asrc_common.h to include/sound folder > > Shengjiu Wang (15): > ASoC: fsl_asrc: define functions for memory to memory usage > ASoC: fsl_easrc: define functions for memory to memory usage > ASoC: fsl_asrc: move fsl_asrc_common.h to include/sound > ASoC: fsl_asrc: register m2m platform device > ASoC: fsl_easrc: register m2m platform device > media: uapi: Add V4L2_CAP_AUDIO_M2M capability flag > media: v4l2: Add audio capture and output support > media: uapi: Define audio sample format fourcc type > media: uapi: Add V4L2_CTRL_CLASS_M2M_AUDIO > media: uapi: Add audio rate controls support > media: uapi: Declare interface types for Audio > media: uapi: Add an entity type for audio resampler > media: vivid: add fixed point test controls > media: imx-asrc: Add memory to memory driver > media: vim2m-audio: add virtual driver for audio memory to memory > > .../media/mediactl/media-types.rst| 11 + > .../userspace-api/media/v4l/buffer.rst|6 + > .../userspace-api/media/v4l/common.rst|1 + > .../media/v4l/dev-audio-mem2mem.rst | 71 + > .../userspace-api/media/v4l/devices.rst |1 + > .../media/v4l/ext-ctrls-audio-m2m.rst | 41 + > .../userspace-api/media/v4l/pixfmt-audio.rst | 87 ++ > .../userspace-api/media/v4l/pixfmt.rst|1 + >
