[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #270: Small I2C fixups

2020-02-13 Thread GitBox
patacongo commented on a change in pull request #270: Small I2C fixups
URL: https://github.com/apache/incubator-nuttx/pull/270#discussion_r378996148
 
 

 ##
 File path: drivers/i2c/Kconfig
 ##
 @@ -54,3 +50,7 @@ config I2CMULTIPLEXER_PCA9540BDP
 endmenu # I2C Multiplexer Support
 
 endif
+
+config ARCH_HAVE_I2CRESET
 
 Review comment:
   You could move the ARCH_HAVE_I2CRESET  item into the drivers/Kconfig file 
before CONFIG_I2C is selected.  That would resolve the problem.
   
   I personally believe that all of the ARCH_HAVE_ definitions belong in the 
arch/Kconfig menu.  That is were they were originally and there is where they 
best belong.  The ARCH naming conventions is only correct if they reside in the 
arch/Kconfig file.  But there were moved all over in a recent PR and now the 
naming is wrong and a lot of dependencies relationships have been broken and 
kludged around like this 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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #270: Small I2C fixups

2020-02-13 Thread GitBox
patacongo commented on a change in pull request #270: Small I2C fixups
URL: https://github.com/apache/incubator-nuttx/pull/270#discussion_r378983337
 
 

 ##
 File path: drivers/i2c/Kconfig
 ##
 @@ -54,3 +50,7 @@ config I2CMULTIPLEXER_PCA9540BDP
 endmenu # I2C Multiplexer Support
 
 endif
+
+config ARCH_HAVE_I2CRESET
 
 Review comment:
   The reason is that  ARCH_HAVE_I2CRESET  is selected unconditionally in all 
architecture configurations that support I2C reset.  That is correct, 
advertising that an architecture supports a feature is conditional.  CONFIG_I2C 
is completely unnecessary to enable architecture I2C support.
   
   If conditions are added to ARCH_HAVE_I2CRESET, the errors referring to unmet 
dependencies will be generated on all configurations that enabled 
architecture-specific I2C support but don't need CONFIG_I2C.
   
   This same change was added a few weeks ago and this resulted in several new 
errors in build testing and had to be fixed by moving ARCH_HAVE_I2CRESET  
outside of I2C.  It must be that way.
   


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:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #270: Small I2C fixups

2020-02-13 Thread GitBox
patacongo commented on a change in pull request #270: Small I2C fixups
URL: https://github.com/apache/incubator-nuttx/pull/270#discussion_r378973957
 
 

 ##
 File path: drivers/i2c/Kconfig
 ##
 @@ -54,3 +50,7 @@ config I2CMULTIPLEXER_PCA9540BDP
 endmenu # I2C Multiplexer Support
 
 endif
+
+config ARCH_HAVE_I2CRESET
 
 Review comment:
   We have been through this one before.  ARCH_HAVE_I2CRESET must be outside of 
if I2C, otherwise Kconfig generates errors.  This cannot be accepted into the 
repository.
   
   That same mistake was made in a previous PR and that had to be reverted 
because of all of the configuration errors it causes.  This cannot be accepted 
into the repository.


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:
us...@infra.apache.org


With regards,
Apache Git Services