Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Driver-specific spinlock for cxd56_serial.c #2830

Merged
merged 2 commits into from Feb 9, 2021

Conversation

masayuki2009
Copy link
Contributor

@masayuki2009 masayuki2009 commented Feb 9, 2021

Summary

  • This PR consists of the following 2 commits
  • commit 1: include: nuttx: Introduce spinlock_t for #ifndef CONFIG_SPINLOCK
    • This commit introduces spinlock_t for #ifndef CONFIG_SPINLOCK
      which is useful for the non-SMP case because it does not consume
      memory
  • commit 2: arch: cxd56xx: Introduce driver specific spinlock in cxd56_serial.c
    • This commit introduces driver-specific spinlock in cxd56_serial.c
      to improve performance
  • NOTE: Implement the fine-grained spin lock #2213

Impact

  • SMP only

Testing

  • Tested with spresense:wifi and spresense:wifi_smp

Summary:
- This commit introduces spinlock_t for #ifndef CONFIG_SPINLOCK
  which is useful for the non-SMP case because it does not consume
  memory

Impact:
- None:

Testing:
- N/A

Signed-off-by: Masayuki Ishikawa <Masayuki.Ishikawa@jp.sony.com>
Summary:
- This commit introduces driver-specific spinlock in cxd56_serial.c
  to improve performance

Impact:
- SMP only

Testing:
- Tested with spresense:wifi and spresense:wifi_smp

Signed-off-by: Masayuki Ishikawa <Masayuki.Ishikawa@jp.sony.com>
@xiaoxiang781216 xiaoxiang781216 merged commit c024b41 into apache:master Feb 9, 2021
@@ -30,7 +30,11 @@
#include <sys/types.h>
#include <stdint.h>

#ifdef CONFIG_SPINLOCK
#ifndef CONFIG_SPINLOCK
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm trying to understand the outcome of this change when CONFIG_SPINLOCK is not defined.
From what I understood, &priv->lock should always evaluate to NULL when CONFIG_SPINLOCK is not defined.
I've tried to isolate this behavior in an example, but I couldn't achieve the same result. Instead, the pointer to the empty struct is never NULL.

https://godbolt.org/z/467afd

Did I do something wrong in my test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @gustavonihei,

Actually, when CONFIG_SPINLOCK is not defined CONFIG_SMP is not defined either.
So, finally, spin_lock_irqsave(l) will be just up_irq_save().

include/nuttx/irq.h

...
#if defined(CONFIG_SMP)                                                                                                                                                
irqstate_t spin_lock_irqsave(spinlock_t *lock);                                                                                                                        
#else                                                                                                                                                                  
#  define spin_lock_irqsave(l) up_irq_save()                                                                                                                           
#endif  
...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the clarification, @masayuki2009!

@masayuki2009 masayuki2009 deleted the spinlock branch March 23, 2021 22:53
@btashton btashton added this to To-Add in Release Notes - 10.1.0 Apr 12, 2021
@jerpelea jerpelea moved this from To-Add to arch in Release Notes - 10.1.0 Apr 13, 2021
@jerpelea jerpelea moved this from arch to Added in Release Notes - 10.1.0 Apr 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants