linguini1 commented on code in PR #17344:
URL: https://github.com/apache/nuttx/pull/17344#discussion_r2540358893
##########
drivers/eeprom/spi_xx25xx.c:
##########
@@ -310,27 +316,39 @@ static void ee25xx_lock(FAR struct spi_dev_s *dev)
* bus is unlocked.
*/
- SPI_LOCK(dev, true);
+ SPI_LOCK(priv->spi, true);
/* After locking the SPI bus, the we also need call the setfrequency,
* setbits, and setmode methods to make sure that the SPI is properly
* configured for the device. If the SPI bus is being shared, then it may
* have been left in an incompatible state.
*/
- SPI_SETMODE(dev, CONFIG_EE25XX_SPIMODE);
- SPI_SETBITS(dev, 8);
- SPI_HWFEATURES(dev, 0);
- SPI_SETFREQUENCY(dev, CONFIG_EE25XX_FREQUENCY);
+ SPI_SETMODE(priv->spi, CONFIG_EE25XX_SPIMODE);
+ SPI_SETBITS(priv->spi, 8);
+ SPI_HWFEATURES(priv->spi, 0);
+ SPI_SETFREQUENCY(priv->spi, priv->freq);
+#ifdef CONFIG_SPI_DELAY_CONTROL
+ SPI_SETDELAY(priv->spi, CONFIG_EE25XX_START_DELAY,
Review Comment:
It looks like this PR does more than just adding the new IOCTL commands, it
also adds this SPI delay? Please include that in the commit/PR description.
##########
include/nuttx/eeprom/eeprom.h:
##########
@@ -40,24 +40,31 @@
#include <nuttx/fs/ioctl.h>
-/************************************************************************************
+/****************************************************************************
* Pre-processor Definitions
-
************************************************************************************/
+ ****************************************************************************/
-/* EEPROM IOCTL Commands
************************************************************/
+/* EEPROM IOCTL Commands ****************************************************/
#define EEPIOC_GEOMETRY _EEPIOC(0x000) /* Similar to BIOC_GEOMETRY:
- * Return the geometry of the
EEPROM
- * device.
- * IN: Pointer to writable
instance of
- * struct eeprom_geometry_s
in which
- * to return the geometry.
- * OUT: Data return in
user-provided
- * buffer. */
+ * Return the geometry of the
+ * EEPROM device.
+ * IN: Pointer to writable
+ * instance of struct
+ * eeprom_geometry_s to be
+ * populated
+ * OUT: Data return in user-
+ * provided buffer. */
-/************************************************************************************
+#define EEPIOC_SETSPEED _EEPIOC(0x001) /* Overwrite the SPI/I2C bus speed
+ * IN: Bus speed in Hz
+ * OUT: None (ioctl return value
+ * provides success/failure
+ * indication). */
+
+/****************************************************************************
* Type Definitions
-
************************************************************************************/
Review Comment:
Are all these formatting diffs necessary to pass the linting check?
Otherwise please revert them, it clutters the diff and the git history.
##########
Documentation/components/drivers/character/eeprom.rst:
##########
@@ -155,7 +155,8 @@ IOCTL Commands
The full list of ``ioctl()`` commands can be found in
``include/nuttx/eeprom/eeprom.h``.
-- ``EEPIOC_GEOMETRY``: Get the EEPROM geometry
+- ``EEPIOC_GEOMETRY``: Get the EEPROM geometry
Review Comment:
It would be useful for both commands if you could mention the type of the
argument that needs to be passed
--
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]
