[riot-notifications] [RIOT-OS/RIOT] drivers: add bq2429x power management IC driver (#15523)

benpicco notifications at github.com
Sun Jan 17 00:08:53 CET 2021


@benpicco commented on this pull request.

Looks good, the documentation could need some more explanations, e.g. when setting a limit it should be clear if that is an upper or a lower limit. 

> +
+/**
+ * @brief   Get device faults.
+ *
+ * @pre @p dev != NULL && @p fault != NULL
+ *
+ * @param[in]   dev         Device descriptor.
+ * @param[out]  fault       Pointer where device faults will be stored.
+ *
+ * @return BQ2429X_OK on success.
+ * @return BQ2429X_ERR_I2C on I2C error.
+ */
+int bq2429x_get_fault(const bq2429x_t *dev, bq2429x_fault_t *fault);
+
+/**
+ * @brief   Enable OTG.

What does it mean to enable OTG?
Charge via USB? Give charge to a USB device? 

> +int bq2429x_enable_charge(const bq2429x_t *dev);
+
+/**
+ * @brief   Disable battery charging.
+ *
+ * @pre @p dev != NULL
+ *
+ * @param[in]   dev Device descriptor.
+ *
+ * @return BQ2429X_OK on success.
+ * @return BQ2429X_ERR_I2C on I2C failure.
+ */
+int bq2429x_disable_charge(const bq2429x_t *dev);
+
+/**
+ * @brief   Set Input Voltage Limit.

Is this an upper or a lower limit? 

> +/**
+ * @brief   Get Charge Voltage Limit.
+ *
+ * @pre @p dev != NULL && @p vreg != NULL
+ *
+ * @param[in]   dev     Device descriptor.
+ * @param[out]  vreg    Voltage limit.
+ *
+ * @return BQ2429X_OK on success.
+ * @return BQ2429X_ERR_I2C on I2C failure.
+ */
+int bq2429x_get_vreg(const bq2429x_t *dev,
+                     bq2429x_charge_voltage_limit_t *vreg);
+
+/**
+ * @brief   Update charge parameters on the device.

So this must be called after setting all those limits? 

> +typedef enum {
+    BQ2429x_CHRG_FAULT_NORMAL = 0,              /**< No fault, normal */
+    BQ2429x_CHRG_FAULT_INPUT,                   /**< Input fault (OVP or bad
+                                                     source) */
+    BQ2429x_CHRG_FAULT_THERMAL_SHUTDOWN,        /**< Thermal shutdown */
+    BQ2429x_CHRG_FAULT_CHARGE_TIMER_EXPIRATION, /**< Charge timer expiration */
+} bq2429x_chrg_fault_t;
+
+/**
+ * @brief   Device faults
+ */
+typedef struct {
+    /**
+     * @brief   Watchdog fault.
+     * @details false = Normal.
+     *          true  = Watchdog timer expiration.

Can the Watchdog timer be set by the user? 

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/15523#pullrequestreview-569973672
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20210116/189ca4a6/attachment-0001.htm>


More information about the notifications mailing list