TimJTi commented on code in PR #9252: URL: https://github.com/apache/nuttx/pull/9252#discussion_r1193623147
########## include/nuttx/efuse/sama5_sfc_fuses.h: ########## @@ -0,0 +1,247 @@ +/**************************************************************************** + * include/nuttx/efuse/sama5_sfc_fuses.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 __INCLUDE_NUTTX_EFUSE_SAMA5_FUSES_H +#define __INCLUDE_NUTTX_EFUSE_SAMA5_FUSES_H + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +#if defined(ATSAMA5D2) +#define SAM_SFC_EFUSE_MAX_LEN 544 /* Max length of sfc area. */ +#elif defined ATSAMA5D4 +#define SAM_SFC_EFUSE_MAX_LEN 512 /* Max length of sfc area. */ +#endif + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +/**************************************************************************** + * Type Definitions + ****************************************************************************/ + +#if defined(CONFIG_EFUSE) && defined(CONFIG_SAMA5_SFC) Review Comment: I agree about the has defines. But the static consts are the basic, default, field descriptions that can be used by an app so they need to be outside the arch directories I think? I asked about this when I submitted the PR and @acassis noted that it ought to be in the efuse driver directory as per comments in the efuse driver files? So I left it in the sama5_sfc_efuses.h file. If it is agreed the declarations should be moved to an arch .h file then the comments in the efuse driver files will need to be amended. Or I misunderstood the comments and they need clarifying :) ########## include/nuttx/efuse/sama5_sfc_fuses.h: ########## @@ -0,0 +1,247 @@ +/**************************************************************************** + * include/nuttx/efuse/sama5_sfc_fuses.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 __INCLUDE_NUTTX_EFUSE_SAMA5_FUSES_H +#define __INCLUDE_NUTTX_EFUSE_SAMA5_FUSES_H + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +#if defined(ATSAMA5D2) +#define SAM_SFC_EFUSE_MAX_LEN 544 /* Max length of sfc area. */ +#elif defined ATSAMA5D4 +#define SAM_SFC_EFUSE_MAX_LEN 512 /* Max length of sfc area. */ +#endif + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +/**************************************************************************** + * Type Definitions + ****************************************************************************/ + +#if defined(CONFIG_EFUSE) && defined(CONFIG_SAMA5_SFC) Review Comment: I agree about the #defines. But the static consts are the basic, default, field descriptions that can be used by an app so they need to be outside the arch directories I think? I asked about this when I submitted the PR and @acassis noted that it ought to be in the efuse driver directory as per comments in the efuse driver files? So I left it in the sama5_sfc_efuses.h file. If it is agreed the declarations should be moved to an arch .h file then the comments in the efuse driver files will need to be amended. Or I misunderstood the comments and they need clarifying :) -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
