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 2021/11/15 15:26:57 UTC
[GitHub] [incubator-nuttx] Donny9 opened a new pull request #4833: driver/motor: add upperhalf structure
Donny9 opened a new pull request #4833:
URL: https://github.com/apache/incubator-nuttx/pull/4833
## Summary
driver/motor: add upperhalf structure and optimize code.
## Impact
optimize code
## Testing
daily test
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #4833: driver/motor: add upperhalf structure
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #4833:
URL: https://github.com/apache/incubator-nuttx/pull/4833
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Donny9 commented on a change in pull request #4833: driver/motor: add upperhalf structure
Posted by GitBox <gi...@apache.org>.
Donny9 commented on a change in pull request #4833:
URL: https://github.com/apache/incubator-nuttx/pull/4833#discussion_r749864567
##########
File path: drivers/motor/foc/foc_dev.c
##########
@@ -147,19 +142,13 @@ static int foc_open(FAR struct file *filep)
if (tmp == 1)
{
- /* Yes.. perform one time driver setup */
-
- flags = enter_critical_section();
-
Review comment:
yes, I also think the critical section should be done by lower half of the driver, more detailed control.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #4833: driver/motor: add upperhalf structure
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4833:
URL: https://github.com/apache/incubator-nuttx/pull/4833#discussion_r750558401
##########
File path: drivers/motor/foc/foc_dev.c
##########
@@ -147,19 +142,13 @@ static int foc_open(FAR struct file *filep)
if (tmp == 1)
{
- /* Yes.. perform one time driver setup */
-
- flags = enter_critical_section();
-
Review comment:
Sure, let's fix it one by one.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #4833: driver/motor: add upperhalf structure
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4833:
URL: https://github.com/apache/incubator-nuttx/pull/4833#discussion_r749501297
##########
File path: drivers/motor/foc/foc_dev.c
##########
@@ -147,19 +142,13 @@ static int foc_open(FAR struct file *filep)
if (tmp == 1)
{
- /* Yes.. perform one time driver setup */
-
- flags = enter_critical_section();
-
Review comment:
The code is already locked by nxsem_wait.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] acassis commented on a change in pull request #4833: driver/motor: add upperhalf structure
Posted by GitBox <gi...@apache.org>.
acassis commented on a change in pull request #4833:
URL: https://github.com/apache/incubator-nuttx/pull/4833#discussion_r749466961
##########
File path: drivers/motor/foc/foc_dev.c
##########
@@ -147,19 +142,13 @@ static int foc_open(FAR struct file *filep)
if (tmp == 1)
{
- /* Yes.. perform one time driver setup */
-
- flags = enter_critical_section();
-
Review comment:
@xiaoxiang781216 @Donny9 why the critical section is not necessary anymore? Should this be done by the lower half of the driver?
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] acassis commented on pull request #4833: driver/motor: add upperhalf structure
Posted by GitBox <gi...@apache.org>.
acassis commented on pull request #4833:
URL: https://github.com/apache/incubator-nuttx/pull/4833#issuecomment-972889889
Please rebase to fix the issue in the simulator
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] raiden00pl commented on a change in pull request #4833: driver/motor: add upperhalf structure
Posted by GitBox <gi...@apache.org>.
raiden00pl commented on a change in pull request #4833:
URL: https://github.com/apache/incubator-nuttx/pull/4833#discussion_r750021442
##########
File path: drivers/motor/foc/foc_dev.c
##########
@@ -147,19 +142,13 @@ static int foc_open(FAR struct file *filep)
if (tmp == 1)
{
- /* Yes.. perform one time driver setup */
-
- flags = enter_critical_section();
-
Review comment:
I think it's ok. The same logic is present in many other drivers, so probably this change also applies to them
##########
File path: drivers/motor/foc/foc_dev.c
##########
@@ -862,19 +830,14 @@ int foc_register(FAR const char *path, FAR struct foc_dev_s *dev)
if (dev->devno > CONFIG_MOTOR_FOC_INST)
{
mtrerr("Unsupported foc devno %d\n\n", dev->devno);
- set_errno(EINVAL);
- ret = ERROR;
+ ret = -EINVAL;
goto errout;
}
/* Reset counter */
dev->ocount = 0;
- /* Store device number */
-
- dev->devno = g_devno_cntr;
Review comment:
At this moment arch/sim/src/sim/up_foc.c and arch/arm/src/stm32/stm32_foc.c depend on on this variable.
So it'll be better to remove it with simultaneous change in lower-half implementations.
##########
File path: drivers/motor/foc/foc_dev.c
##########
@@ -862,19 +830,14 @@ int foc_register(FAR const char *path, FAR struct foc_dev_s *dev)
if (dev->devno > CONFIG_MOTOR_FOC_INST)
{
mtrerr("Unsupported foc devno %d\n\n", dev->devno);
- set_errno(EINVAL);
- ret = ERROR;
+ ret = -EINVAL;
goto errout;
}
/* Reset counter */
dev->ocount = 0;
- /* Store device number */
-
- dev->devno = g_devno_cntr;
Review comment:
And it makes the condition above is useless
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #4833: driver/motor: add upperhalf structure
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4833:
URL: https://github.com/apache/incubator-nuttx/pull/4833#discussion_r750869953
##########
File path: drivers/motor/foc/foc_dev.c
##########
@@ -862,19 +830,14 @@ int foc_register(FAR const char *path, FAR struct foc_dev_s *dev)
if (dev->devno > CONFIG_MOTOR_FOC_INST)
{
mtrerr("Unsupported foc devno %d\n\n", dev->devno);
- set_errno(EINVAL);
- ret = ERROR;
+ ret = -EINVAL;
goto errout;
}
/* Reset counter */
dev->ocount = 0;
- /* Store device number */
-
- dev->devno = g_devno_cntr;
Review comment:
Done.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org