On Sun, 3 Nov 2019 at 16:27, evan chime <chime.evan.dri.devel@gmail.com> wrote:On Sun, 3 Nov 2019 at 16:18, Julia Lawall <julia.lawall@lip6.fr> wrote:
On Sun, 3 Nov 2019, Evan Chime wrote:
> Include outreachy mailing list in "cc" list. No change
>
> Evan Chime (3):
> Staging: media: omap4iss: Add comment to memory barrier
> Staging: media: omap4iss: Add comment to memory barrier
> Staging: media: omap4iss: Add comment to memory barrier
Each patch should have a different subject. If you end up with patches
with the same subject, you should make the subjects more specific or merge
them.
In this case, though, I think that this is an issue that only someone with
a deep knowledge of the code could really address. The question is not
what a memory barrier does, but why one is needed at a particular place in
the code.
juliaThanks for the feedback JuliaKind RegardsEvanPlease I have studied the function. From what I understand, the function checks if ISS submodule needs to wait for next interrupt. If yes, makes the caller to sleep while waiting for such event.
The atomic_set(stopping, 1) indicates whether the submodule needs to wait, where stopping is the flag, and atomic_set(stopping, 0) indicates no need to wait
According to the comment before the first atomic_set() call and the smp_wmb() call that follows which says:
/* Atomic set() doesn’t include memory barrier on ARM platform for SMP scenario. We’ll call it here to avoid race conditions */, the smp_wmb() call is I think, to avoid reordering of the atomic_set() operation
So can I use one of the following, as the comment
A) prevent reordering of atomic_set()
B) sync module with it’s idle state(from the comment that introduced the function)
C) ensures *stopping is persistent
D) none of the above :)
Please forgive me for the first email, just noticed I didn’t say who it was coming from at the end.
I have studied the function. From what I understand, the function checks if ISS submodule needs to wait for next interrupt. If yes, makes the caller to sleep while waiting for such event.
The atomic_set(stopping, 1) indicates whether the submodule needs to wait, where stopping is the flag, and atomic_set(stopping, 0) indicates no need to wait
According to the comment before the first atomic_set() call and the smp_wmb() call that follows which says:
/* Atomic set() doesn’t include memory barrier on ARM platform for SMP scenario. We’ll call it here to avoid race conditions */, the smp_wmb() call is I think, to avoid reordering of the atomic_set() operation
So can I use one of the following, as the comment
A) prevent reordering of atomic_set()
B) sync module with it’s idle state(from the comment that introduced the function)
C) ensures *stopping is persistent
D) none of the above :)
Kind regards
Evan
>
> drivers/staging/media/omap4iss/iss.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> --
> 2.17.1
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/cover.1572797322.git.chime.evan.dri.devel%40gmail.com.
>
--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/alpine.DEB.2.21.1911031716390.2557%40hadrien.