pkarashchenko commented on code in PR #6965: URL: https://github.com/apache/incubator-nuttx/pull/6965#discussion_r989315174
########## drivers/mtd/smart.c: ########## @@ -736,8 +736,8 @@ int smart_checkfree(FAR struct smart_struct_s *dev, int lineno) uint16_t blockrelease; static uint16_t prev_freesectors = 0; static uint16_t prev_releasesectors = 0; - static uint8_t *prev_freecount = NULL; - static uint8_t *prev_releasecount = NULL; + static uint8_t *prev_freecount = NULL; + static uint8_t *prev_releasecount = NULL; Review Comment: ```suggestion static FAR uint8_t *prev_freecount = NULL; static FAR uint8_t *prev_releasecount = NULL; ``` ########## drivers/mtd/smart.c: ########## @@ -4150,7 +4150,7 @@ static int smart_write_wearstatus(struct smart_struct_s *dev) if (dev->uneven_wearcount != 0) { - *((uint32_t *) &buffer[4]) = dev->uneven_wearcount; + *((uint32_t *)&buffer[4]) = dev->uneven_wearcount; Review Comment: ```suggestion *((FAR uint32_t *)&buffer[4]) = dev->uneven_wearcount; ``` ########## wireless/ieee802154/mac802154_internal.h: ########## @@ -510,62 +511,12 @@ void mac802154_notify(FAR struct ieee802154_privmac_s *priv, /* General helpers **********************************************************/ -#define mac802154_givesem(s) nxsem_post(s) - -static inline int mac802154_takesem(sem_t *sem, bool allowinterrupt) -{ - if (allowinterrupt) - { - return nxsem_wait(sem); - } - else - { - return nxsem_wait_uninterruptible(sem); - } -} - -#ifdef CONFIG_MAC802154_LOCK_VERBOSE -#define mac802154_unlock(dev) \ - mac802154_givesem(&dev->exclsem); \ - wlinfo("MAC unlocked\n"); -#else -#define mac802154_unlock(dev) \ - mac802154_givesem(&dev->exclsem); -#endif - -#define mac802154_lock(dev, allowinterrupt) \ - mac802154_lockpriv(dev, allowinterrupt, __FUNCTION__) - -static inline int -mac802154_lockpriv(FAR struct ieee802154_privmac_s *dev, - bool allowinterrupt, FAR const char *funcname) -{ Review Comment: I think this changes the logic significantly since driver was allowing to handle signal interrupt. I'm not sure if such situation is relevant for kernel at all and this driver does not seems to run as a user space application. So anyway seems to be fine to use nxmutex instead ########## wireless/ieee802154/mac802154_netdev.c: ########## @@ -362,7 +363,7 @@ static int macnet_notify(FAR struct mac802154_maccb_s *maccb, * back to trying to get access again */ - while (nxsem_wait(&priv->md_exclsem) < 0); + while (nxmutex_lock(&priv->md_lock) < 0); Review Comment: is while really needed here? is it intended to handle ECANCEL? ########## arch/arm/src/a1x/hardware/a10_memorymap.h: ########## @@ -502,7 +502,7 @@ * LI PAGE TABLE * ADDRESS RANGE SIZE ENTRIES SECTIONS * ----------------------- ------- -------------- --------- - * 0xc000:0000-0xffef:ffff 1022MB *0x3000-0x3ff8 1022 + * 0xc000:0000-0xffef:ffff 1022MB 0x3000-0x3ff8 1022 Review Comment: Please fix inclusion of board_memorymap.h ########## drivers/mtd/smart.c: ########## @@ -211,7 +211,7 @@ struct smart_cache_s #ifdef CONFIG_MTD_SMART_ENABLE_CRC struct smart_allocsector_s { - struct smart_allocsector_s *next; /* Pointer to next alloc sector */ + struct smart_allocsector_s *next; /* Pointer to next alloc sector */ Review Comment: ```suggestion FAR struct smart_allocsector_s *next; /* Pointer to next alloc sector */ ``` ########## drivers/mtd/smart.c: ########## @@ -1375,9 +1375,9 @@ static int smart_setsectorsize(FAR struct smart_struct_s *dev, uint16_t size) ****************************************************************************/ static ssize_t smart_bytewrite(FAR struct smart_struct_s *dev, size_t offset, - int nbytes, FAR const uint8_t *buffer) + int nbytes, FAR const uint8_t *buffer) Review Comment: ```suggestion int nbytes, FAR const uint8_t *buffer) ``` ########## libs/libc/stdio/lib_rewind.c: ########## @@ -46,8 +46,8 @@ void rewind(FAR FILE *stream) return; } - lib_take_semaphore(stream); + lib_take_lock(stream); (void) fseek(stream, 0L, SEEK_SET); Review Comment: ```suggestion (void)fseek(stream, 0L, SEEK_SET); ``` ########## drivers/mtd/smart.c: ########## @@ -4271,11 +4271,11 @@ static inline int smart_read_wearstatus(FAR struct smart_struct_s *dev) /* Get the uneven wearcount value */ - dev->uneven_wearcount = *((uint32_t *) &buffer[4]); + dev->uneven_wearcount = *((uint32_t *)&buffer[4]); Review Comment: ```suggestion dev->uneven_wearcount = *((FAR uint32_t *)&buffer[4]); ``` ########## mm/mm_heap/mm_free.c: ########## @@ -84,10 +84,10 @@ void mm_free(FAR struct mm_heap_s *heap, FAR void *mem) return; } - if (mm_takesemaphore(heap) == false) + if (mm_lock(heap) == false) Review Comment: ```suggestion if (!mm_lock(heap)) ``` -- 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: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org