You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/05/06 23:28:56 UTC
[GitHub] [incubator-nuttx] Ouss4 opened a new pull request #987: Implement the up_tls_info for the rest of the architectures
Ouss4 opened a new pull request #987:
URL: https://github.com/apache/incubator-nuttx/pull/987
## Summary
This PR implements the up_tls_info for the rest of the architectures. Only sim and arm used to have an implementation.
A small organization was needed for architectures that don't share the same way to retrieve the SP.
z16 and z80 are still a WIP since I don't know how to get the stack pointer there.
## Impact
TLS is still conditioned so for now no impact in any architecture.
## Testing
Testing was performed with the sim ostest (with TLS enabled).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on pull request #987: Implement the up_tls_info for the rest of the architectures
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #987:
URL: https://github.com/apache/incubator-nuttx/pull/987#issuecomment-624960891
PS.: Don't worry about the horrible commits. I'll provide a better history when everything passes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #987: Implement the up_tls_info for the rest of the architectures
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #987:
URL: https://github.com/apache/incubator-nuttx/pull/987#discussion_r421162160
##########
File path: arch/arm/include/tls.h
##########
@@ -37,12 +37,12 @@
****************************************************************************/
/****************************************************************************
- * Name: up_getsp
+ * Name: arm_getsp
Review comment:
> Maybe I can do something similar as RISC-V for instance and take the arm_getsp to the arch.h file.
Don't bother. It would still be exported to all files and should still have the up_ prefix. There is no way to avoid avoid that. TLS requires that that function be used outside of the ARM code. Same is true for all of the other architectures.
I don't recommend changing the name, but not for that reason. I don't recommend changing the because it is not a documented, controlled interface. If is private for use within the tls.h header file. it does not have a consist protoype, it is not prototyped in include/nuttx/arch.h so it doesn't matter what you call it.
up_ would be more arhicturually correct probably, but it is not important. All MCU names used outide of the architecutre-/board-specific logic should be called up_
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on pull request #987: Implement the up_tls_info for the rest of the architectures
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #987:
URL: https://github.com/apache/incubator-nuttx/pull/987#issuecomment-624953041
Otherwise, it all looks good to me. That was a very big effort. Thanks for the nice contribution.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #987: Implement the up_tls_info for the rest of the architectures
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #987:
URL: https://github.com/apache/incubator-nuttx/pull/987#discussion_r421154488
##########
File path: arch/avr/include/avr/arch.h
##########
@@ -0,0 +1,87 @@
+/****************************************************************************
+ * arch/avr/include/avr/arch.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* This file should never be included directly but, rather,
+ * only indirectly through nuttx/arch.h
+ */
+
+#ifndef __ARCH_AVR_INCLUDE_AVR_ARCH_H
+#define __ARCH_AVR_INCLUDE_AVR_ARCH_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+#ifndef __ASSEMBLY__
+
+/****************************************************************************
+ * Inline functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: avr_getsp
+ ****************************************************************************/
+
+static inline uint16_t avr_getsp(void)
+{
+ uint8_t spl;
+ uint8_t sph;
+
+ __asm__ __volatile__
+ (
+ "in %0, __SP_L__\n\t"
+ "in %1, __SP_H__\n"
+ : "=r" (spl), "=r" (sph)
+ :
+ );
+ return (uint16_t)sph << 8 | spl;
Review comment:
Misssing blank line. I assume that uint16_t is correct for an 8-bit AVR? Sounds righr.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #987: Implement the up_tls_info for the rest of the architectures
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #987:
URL: https://github.com/apache/incubator-nuttx/pull/987#discussion_r421159895
##########
File path: arch/arm/include/tls.h
##########
@@ -37,12 +37,12 @@
****************************************************************************/
/****************************************************************************
- * Name: up_getsp
+ * Name: arm_getsp
Review comment:
Maybe I can do something similar as RISC-V for instance and take the arm_getsp to the arch.h file.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on pull request #987: Implement the up_tls_info for the rest of the architectures
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #987:
URL: https://github.com/apache/incubator-nuttx/pull/987#issuecomment-624952836
As you mentioned, the implementation is incomplete for z16 and z80 because you are missing assembly language functions to get the SP in most of those cases.
I do not think this is a problem for those architectures. The stack pointer is only needed in the ALIGNED TLS case and the ALIGNED TLS case is not appropriate for the z80 and z180 architectures. z16f and ez80 are possibilities but not likely. We do needed the unaligned TLS case implemented. My recommendation for now is to add logic like:
#ifdef CONFIG_TLS_ALIGNED
# errno Aligned TLS not supported
#else
# define up_tls_info() tls_get_info()
#endf
That provided everything that is needed from now and if some crazy person wants to implement aligned TLS on these tiny chips in the future, then they will have to provide the missing logic.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #987: Implement the up_tls_info for the rest of the architectures
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #987:
URL: https://github.com/apache/incubator-nuttx/pull/987#discussion_r421154971
##########
File path: arch/avr/include/tls.h
##########
@@ -0,0 +1,78 @@
+/****************************************************************************
+ * arch/avr/include/tls.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __ARCH_AVR_INCLUDE_TLS_H
+#define __ARCH_AVR_INCLUDE_TLS_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <assert.h>
+#include <nuttx/arch.h>
+#include <nuttx/tls.h>
+
+#ifdef CONFIG_TLS
+
+/****************************************************************************
+ * Inline Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: up_tls_info
+ *
+ * Description:
+ * Return the TLS information structure for the currently executing thread.
+ * When TLS is enabled, up_createstack() will align allocated stacks to
+ * the TLS_STACK_ALIGN value. An instance of the following structure will
+ * be implicitly positioned at the "lower" end of the stack. Assuming a
+ * "push down" stack, this is at the "far" end of the stack (and can be
+ * clobbered if the stack overflows).
+ *
+ * If an MCU has a "push up" then that TLS structure will lie at the top
+ * of the stack and stack allocation and initialization logic must take
+ * care to preserve this structure content.
+ *
+ * The stack memory is fully accessible to user mode threads.
+ *
+ * Input Parameters:
+ * None
+ *
+ * Returned Value:
+ * A pointer to TLS info structure at the beginning of the STACK memory
+ * allocation. This is essentially an application of the TLS_INFO(sp)
+ * macro and has a platform dependency only in the manner in which the
+ * stack pointer (sp) is obtained and interpreted.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_TLS_ALIGNED
+static inline FAR struct tls_info_s *up_tls_info(void)
+{
+ DEBUGASSERT(!up_interrupt_context());
+ return TLS_INFO((uintptr_t)avr_getsp());
+}
Review comment:
For avr you get a uint16_t but for avr32 you get a uint32_t. I don't know the size of uintptr_t in each architecture but I suppose this works out okay in TLS_INFO?
Ignore me? I am just thinking out loud.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #987: Implement the up_tls_info for the rest of the architectures
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #987:
URL: https://github.com/apache/incubator-nuttx/pull/987#discussion_r421152756
##########
File path: arch/arm/include/tls.h
##########
@@ -37,12 +37,12 @@
****************************************************************************/
/****************************************************************************
- * Name: up_getsp
+ * Name: arm_getsp
Review comment:
Actually, in this case, up_getsp() is the correct name since it is a common interface that is exported outside of the ARM architecture. Same is true of other locations.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #987: Implement the up_tls_info for the rest of the architectures
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #987:
URL: https://github.com/apache/incubator-nuttx/pull/987#discussion_r421157741
##########
File path: arch/avr/src/avr/up_createstack.c
##########
@@ -118,9 +137,42 @@ int up_create_stack(FAR struct tcb_s *tcb, size_t stack_size, uint8_t ttype)
{
/* Allocate the stack. If DEBUG is enabled (but not stack debug),
* then create a zeroed stack to make stack dumps easier to trace.
+ * If TLS is enabled, then we must allocate aligned stacks.
*/
+#ifdef CONFIG_TLS_ALIGNED
Review comment:
There is another #ifdef just after, so no blank line needed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #987: Implement the up_tls_info for the rest of the architectures
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #987:
URL: https://github.com/apache/incubator-nuttx/pull/987#discussion_r421158100
##########
File path: arch/arm/include/tls.h
##########
@@ -37,12 +37,12 @@
****************************************************************************/
/****************************************************************************
- * Name: up_getsp
+ * Name: arm_getsp
Review comment:
It is an internal private function and not part of the well-defined interface, so it really is not important.
It is exported outside of ARM, however, if any code includes:
#include <arch/tls.h>
This it will get this code. That could be from anywhere in the OS or even in applications.
But since it is not a part of the standard interface and since its prototype differs for each architecture, I don't think that the up_ naming is required.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #987: Implement the up_tls_info for the rest of the architectures
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #987:
URL: https://github.com/apache/incubator-nuttx/pull/987#discussion_r421162160
##########
File path: arch/arm/include/tls.h
##########
@@ -37,12 +37,12 @@
****************************************************************************/
/****************************************************************************
- * Name: up_getsp
+ * Name: arm_getsp
Review comment:
> Maybe I can do something similar as RISC-V for instance and take the arm_getsp to the arch.h file.
Don't bother. It would still be exported to all files and should still require the up_ prefix. There is no way to avoid avoid that. TLS requires that that function be used outside of the ARM code. Same is true for all of the other architectures.
I don't recommend changing the name, but not for that reason. I don't recommend changing the name because it is not a documented, controlled interface. It is a private private for use within the tls.h header file. it does not have a consistent protoype, it is not prototyped in include/nuttx/arch.h so it doesn't matter what you call it.
up_ would be more architecturually correct probably, but it is not important. All MCU names used outide of the architecutre-/board-specific logic should be called up_
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #987: Implement the up_tls_info for the rest of the architectures
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #987:
URL: https://github.com/apache/incubator-nuttx/pull/987#discussion_r421154061
##########
File path: arch/avr/include/avr32/arch.h
##########
@@ -0,0 +1,84 @@
+/****************************************************************************
+ * arch/avr/include/avr32/arch.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* This file should never be included directly but, rather,
+ * only indirectly through nuttx/arch.h
+ */
+
+#ifndef __ARCH_AVR_INCLUDE_AVR32_ARCH_H
+#define __ARCH_AVR_INCLUDE_AVR32_ARCH_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+#ifndef __ASSEMBLY__
+
+/****************************************************************************
+ * Inline functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: avr_getsp
+ ****************************************************************************/
+
+static inline uint32_t avr_getsp(void)
+{
+ uint32_t retval;
+ __asm__ __volatile__
+ (
+ "mov\t%0,sp\n\t"
+ : "=r" (retval)
+ :
+ );
+ return retval;
Review comment:
Missing blank line.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #987: Implement the up_tls_info for the rest of the architectures
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #987:
URL: https://github.com/apache/incubator-nuttx/pull/987#discussion_r421153489
##########
File path: arch/avr/src/avr/up_createstack.c
##########
@@ -118,9 +137,42 @@ int up_create_stack(FAR struct tcb_s *tcb, size_t stack_size, uint8_t ttype)
{
/* Allocate the stack. If DEBUG is enabled (but not stack debug),
* then create a zeroed stack to make stack dumps easier to trace.
+ * If TLS is enabled, then we must allocate aligned stacks.
*/
+#ifdef CONFIG_TLS_ALIGNED
Review comment:
Is there a blank line missing here?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on pull request #987: Implement the up_tls_info for the rest of the architectures
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #987:
URL: https://github.com/apache/incubator-nuttx/pull/987#issuecomment-624957493
```
#ifdef CONFIG_TLS_ALIGNED
# error Aligned TLS not supported
#else
# define up_tls_info() tls_get_info()
#endf
```
Okay, I did that.
> Don't expect everything to build. The build system is broken right now. See discussion in #985
Let's keep it open until all the checks pass, I'm concerned about the architectures that I don't use. I might have introduced an error somewhere.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #987: Implement the up_tls_info for the rest of the architectures
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #987:
URL: https://github.com/apache/incubator-nuttx/pull/987#discussion_r421162160
##########
File path: arch/arm/include/tls.h
##########
@@ -37,12 +37,12 @@
****************************************************************************/
/****************************************************************************
- * Name: up_getsp
+ * Name: arm_getsp
Review comment:
> Maybe I can do something similar as RISC-V for instance and take the arm_getsp to the arch.h file.
Don't bother. It would still be exported to all files and should still require the up_ prefix. There is no way to avoid avoid that. TLS requires that that function be used outside of the ARM code. Same is true for all of the other architectures.
I don't recommend changing the name, but not for that reason. I don't recommend changing the because it is not a documented, controlled interface. If is private for use within the tls.h header file. it does not have a consist protoype, it is not prototyped in include/nuttx/arch.h so it doesn't matter what you call it.
up_ would be more architecturually correct probably, but it is not important. All MCU names used outide of the architecutre-/board-specific logic should be called up_
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #987: Implement the up_tls_info for the rest of the architectures
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #987:
URL: https://github.com/apache/incubator-nuttx/pull/987#discussion_r421156496
##########
File path: arch/arm/include/tls.h
##########
@@ -37,12 +37,12 @@
****************************************************************************/
/****************************************************************************
- * Name: up_getsp
+ * Name: arm_getsp
Review comment:
The function is in a header file that's exported but I don't think it will be used outside the ARM directory.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on pull request #987: Implement the up_tls_info for the rest of the architectures
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #987:
URL: https://github.com/apache/incubator-nuttx/pull/987#issuecomment-624955829
Don't expect everything to build. The build system is broken right now. See discussion in #985
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #987: Implement the up_tls_info for the rest of the architectures
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #987:
URL: https://github.com/apache/incubator-nuttx/pull/987#discussion_r421154061
##########
File path: arch/avr/include/avr32/arch.h
##########
@@ -0,0 +1,84 @@
+/****************************************************************************
+ * arch/avr/include/avr32/arch.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* This file should never be included directly but, rather,
+ * only indirectly through nuttx/arch.h
+ */
+
+#ifndef __ARCH_AVR_INCLUDE_AVR32_ARCH_H
+#define __ARCH_AVR_INCLUDE_AVR32_ARCH_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+#ifndef __ASSEMBLY__
+
+/****************************************************************************
+ * Inline functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: avr_getsp
+ ****************************************************************************/
+
+static inline uint32_t avr_getsp(void)
+{
+ uint32_t retval;
+ __asm__ __volatile__
+ (
+ "mov\t%0,sp\n\t"
+ : "=r" (retval)
+ :
+ );
+ return retval;
Review comment:
Missing blank line. Is a 32-bit value always used to represent the stack pointer?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #987: Implement the up_tls_info for the rest of the architectures
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #987:
URL: https://github.com/apache/incubator-nuttx/pull/987#issuecomment-624992718
@Ouss4 PR need rebase to the latest mainline to pass sim check since the change in apps side require the kernel side update.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #987: Implement the up_tls_info for the rest of the architectures
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #987:
URL: https://github.com/apache/incubator-nuttx/pull/987#discussion_r421154971
##########
File path: arch/avr/include/tls.h
##########
@@ -0,0 +1,78 @@
+/****************************************************************************
+ * arch/avr/include/tls.h
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+#ifndef __ARCH_AVR_INCLUDE_TLS_H
+#define __ARCH_AVR_INCLUDE_TLS_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <assert.h>
+#include <nuttx/arch.h>
+#include <nuttx/tls.h>
+
+#ifdef CONFIG_TLS
+
+/****************************************************************************
+ * Inline Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: up_tls_info
+ *
+ * Description:
+ * Return the TLS information structure for the currently executing thread.
+ * When TLS is enabled, up_createstack() will align allocated stacks to
+ * the TLS_STACK_ALIGN value. An instance of the following structure will
+ * be implicitly positioned at the "lower" end of the stack. Assuming a
+ * "push down" stack, this is at the "far" end of the stack (and can be
+ * clobbered if the stack overflows).
+ *
+ * If an MCU has a "push up" then that TLS structure will lie at the top
+ * of the stack and stack allocation and initialization logic must take
+ * care to preserve this structure content.
+ *
+ * The stack memory is fully accessible to user mode threads.
+ *
+ * Input Parameters:
+ * None
+ *
+ * Returned Value:
+ * A pointer to TLS info structure at the beginning of the STACK memory
+ * allocation. This is essentially an application of the TLS_INFO(sp)
+ * macro and has a platform dependency only in the manner in which the
+ * stack pointer (sp) is obtained and interpreted.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_TLS_ALIGNED
+static inline FAR struct tls_info_s *up_tls_info(void)
+{
+ DEBUGASSERT(!up_interrupt_context());
+ return TLS_INFO((uintptr_t)avr_getsp());
+}
Review comment:
For avr you get a uint16_t but for avr32 you get a uint32_t. I don't know the size of uintptr_t in each architecture but I suppose this works out okay in TLS_INFO?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org