You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by GitBox <gi...@apache.org> on 2020/03/31 08:30:17 UTC
[GitHub] [mynewt-core] caspermeijn opened a new pull request #2255: Add
SGM4056 charger driver to pinetime BSP
caspermeijn opened a new pull request #2255: Add SGM4056 charger driver to pinetime BSP
URL: https://github.com/apache/mynewt-core/pull/2255
This adds a driver for the SGM4056 driver. The first commit adds the base driver, second commit adds charge control integration to the base driver. The last commit adds driver initialization to the pinetime BSP.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-core] apache-mynewt-bot commented on issue #2255: Add
SGM4056 charger driver to pinetime BSP
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on issue #2255: Add SGM4056 charger driver to pinetime BSP
URL: https://github.com/apache/mynewt-core/pull/2255#issuecomment-607083823
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-core] apache-mynewt-bot removed a comment on issue #2255:
Add SGM4056 charger driver to pinetime BSP
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on issue #2255: Add SGM4056 charger driver to pinetime BSP
URL: https://github.com/apache/mynewt-core/pull/2255#issuecomment-606501796
<!-- style-bot -->
## Style check summary
### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
#### hw/drivers/chg_ctrl/sgm4056/include/sgm4056/sgm4056.h
<details>
```diff
@@ -28,24 +28,24 @@
struct sgm4056_dev {
struct os_dev dev;
#if MYNEWT_VAL(SGM4056_USE_CHARGE_CONTROL)
- struct charge_control chg_ctrl;
+ struct charge_control chg_ctrl;
struct os_event interrupt_event;
#endif
struct sgm4056_dev_config config;
};
/**
-* Init function for SGM4056 charger
-* @return 0 on success, non-zero on failure
-*/
+ * Init function for SGM4056 charger
+ * @return 0 on success, non-zero on failure
+ */
int sgm4056_dev_init(struct os_dev *dev, void *arg);
/**
- * Reads the state of the power presence indication. Value is 1 when the input
+ * Reads the state of the power presence indication. Value is 1 when the input
* voltage is above the POR threshold but below the OVP threshold and 0 otherwise.
*
* @param dev The sgm4056 device to read on
- * @param power_present Power presence indication to be returned by the
+ * @param power_present Power presence indication to be returned by the
* function (0 if input voltage is not detected, 1 if it is detected)
*
* @return 0 on success, non-zero on failure
@@ -53,11 +53,11 @@
int sgm4056_get_power_presence(struct sgm4056_dev *dev, int *power_present);
/**
- * Reads the state of the charge indication. Value is 1 when a charge cycle
+ * Reads the state of the charge indication. Value is 1 when a charge cycle
* started and will be 0 when the end-of-charge (EOC) condition is met.
*
* @param dev The sgm4056 device to read on
- * @param charging Charge indication to be returned by the
+ * @param charging Charge indication to be returned by the
* function (0 if battery is not charging, 1 if it is charging)
*
* @return 0 on success, non-zero on failure
@@ -65,7 +65,7 @@
int sgm4056_get_charge_indicator(struct sgm4056_dev *dev, int *charging);
/**
- * Reads the state of the charger. This is a combination of the power presence
+ * Reads the state of the charger. This is a combination of the power presence
* and charge indications. Value is either:
* - CHARGE_CONTROL_STATUS_NO_SOURCE, when no power present
* - CHARGE_CONTROL_STATUS_CHARGING, when power present and charging
```
</details>
#### hw/drivers/chg_ctrl/sgm4056/src/sgm4056.c
<details>
```diff
@@ -19,7 +19,7 @@
#if MYNEWT_VAL(SGM4056_USE_CHARGE_CONTROL)
-static int
+static int
sgm4056_chg_ctrl_get_status(struct charge_control * chg_ctrl, int * status)
{
struct sgm4056_dev *dev;
@@ -37,10 +37,10 @@
return rc;
}
-static int
-sgm4056_chg_ctrl_read(struct charge_control * chg_ctrl,
- charge_control_type_t type, charge_control_data_func_t data_func,
- void *data_arg, uint32_t timeout)
+static int
+sgm4056_chg_ctrl_read(struct charge_control * chg_ctrl,
+ charge_control_type_t type, charge_control_data_func_t data_func,
+ void *data_arg, uint32_t timeout)
{
int rc = 0;
int status;
@@ -51,8 +51,8 @@
goto err;
}
- if(data_func) {
- data_func(chg_ctrl, data_arg, (void*)&status, CHARGE_CONTROL_TYPE_STATUS);
+ if (data_func) {
+ data_func(chg_ctrl, data_arg, (void *)&status, CHARGE_CONTROL_TYPE_STATUS);
}
}
@@ -80,7 +80,7 @@
NULL, NULL, OS_TIMEOUT_NEVER);
}
-static void
+static void
sgm4056_irq_handler(void *arg)
{
struct sgm4056_dev *dev = (struct sgm4056_dev *)arg;
@@ -90,8 +90,9 @@
}
#endif
-int
-sgm4056_dev_init(struct os_dev *odev, void *arg) {
+int
+sgm4056_dev_init(struct os_dev *odev, void *arg)
+{
struct sgm4056_dev *dev = (struct sgm4056_dev *)odev;
const struct sgm4056_dev_config *cfg = arg;
int rc = 0;
@@ -107,7 +108,8 @@
dev->interrupt_event.ev_cb = sgm4056_interrupt_event_handler;
dev->interrupt_event.ev_arg = dev;
- rc = hal_gpio_irq_init(dev->config.power_presence_pin, sgm4056_irq_handler, dev, HAL_GPIO_TRIG_BOTH, HAL_GPIO_PULL_NONE);
+ rc = hal_gpio_irq_init(dev->config.power_presence_pin, sgm4056_irq_handler, dev, HAL_GPIO_TRIG_BOTH,
+ HAL_GPIO_PULL_NONE);
hal_gpio_irq_enable(dev->config.power_presence_pin);
#else
rc = hal_gpio_init_in(dev->config.power_presence_pin, HAL_GPIO_PULL_NONE);
@@ -128,7 +130,7 @@
}
rc = charge_control_set_driver(&dev->chg_ctrl, CHARGE_CONTROL_TYPE_STATUS,
- (struct charge_control_driver *)&sgm4056_chg_ctrl_driver);
+ (struct charge_control_driver *)&sgm4056_chg_ctrl_driver);
if (rc) {
goto err;
}
@@ -148,21 +150,27 @@
return rc;
}
-int sgm4056_get_power_presence(struct sgm4056_dev *dev, int *power_present) {
+int
+sgm4056_get_power_presence(struct sgm4056_dev *dev, int *power_present)
+{
int gpio_value = hal_gpio_read(dev->config.power_presence_pin);
*power_present = (gpio_value == 0);
return 0;
}
-int sgm4056_get_charge_indicator(struct sgm4056_dev *dev, int *charging) {
+int
+sgm4056_get_charge_indicator(struct sgm4056_dev *dev, int *charging)
+{
int gpio_value = hal_gpio_read(dev->config.charge_indicator_pin);
*charging = (gpio_value == 0);
return 0;
}
-int sgm4056_get_charger_status(struct sgm4056_dev *dev, charge_control_status_t *charger_status) {
+int
+sgm4056_get_charger_status(struct sgm4056_dev *dev, charge_control_status_t *charger_status)
+{
int power_present;
int charging;
int rc;
```
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-core] mlaz commented on a change in pull request #2255: Add
SGM4056 charger driver to pinetime BSP
Posted by GitBox <gi...@apache.org>.
mlaz commented on a change in pull request #2255: Add SGM4056 charger driver to pinetime BSP
URL: https://github.com/apache/mynewt-core/pull/2255#discussion_r401104381
##########
File path: hw/drivers/chg_ctrl/sgm4056/src/sgm4056.c
##########
@@ -0,0 +1,192 @@
+/*
+ * Copyright 2020 Casper Meijn <ca...@meijn.net>
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "sgm4056/sgm4056.h"
+#include "hal/hal_gpio.h"
+
+#if MYNEWT_VAL(SGM4056_USE_CHARGE_CONTROL)
+
+static int
+sgm4056_chg_ctrl_get_status(struct charge_control * chg_ctrl, int * status)
+{
+ struct sgm4056_dev *dev;
+ int rc;
+
+ dev = (struct sgm4056_dev *)CHARGE_CONTROL_GET_DEVICE(chg_ctrl);
+ if (dev == NULL) {
+ rc = SYS_ENODEV;
+ goto err;
+ }
+
+ rc = sgm4056_get_charger_status(dev, (charge_control_status_t *)status);
+
+err:
+ return rc;
+}
+
+static int
+sgm4056_chg_ctrl_read(struct charge_control * chg_ctrl,
+ charge_control_type_t type, charge_control_data_func_t data_func,
+ void *data_arg, uint32_t timeout)
Review comment:
This indentation looks strange, I think it should start below `struct charge_control`, also same thing regarding the space between * and variable name.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-core] mlaz commented on a change in pull request #2255: Add
SGM4056 charger driver to pinetime BSP
Posted by GitBox <gi...@apache.org>.
mlaz commented on a change in pull request #2255: Add SGM4056 charger driver to pinetime BSP
URL: https://github.com/apache/mynewt-core/pull/2255#discussion_r401102369
##########
File path: hw/drivers/chg_ctrl/sgm4056/src/sgm4056.c
##########
@@ -0,0 +1,192 @@
+/*
+ * Copyright 2020 Casper Meijn <ca...@meijn.net>
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "sgm4056/sgm4056.h"
+#include "hal/hal_gpio.h"
+
+#if MYNEWT_VAL(SGM4056_USE_CHARGE_CONTROL)
+
+static int
+sgm4056_chg_ctrl_get_status(struct charge_control * chg_ctrl, int * status)
+{
+ struct sgm4056_dev *dev;
+ int rc;
+
+ dev = (struct sgm4056_dev *)CHARGE_CONTROL_GET_DEVICE(chg_ctrl);
+ if (dev == NULL) {
+ rc = SYS_ENODEV;
+ goto err;
+ }
+
+ rc = sgm4056_get_charger_status(dev, (charge_control_status_t *)status);
+
+err:
+ return rc;
+}
+
+static int
+sgm4056_chg_ctrl_read(struct charge_control * chg_ctrl,
+ charge_control_type_t type, charge_control_data_func_t data_func,
+ void *data_arg, uint32_t timeout)
+{
+ int rc = 0;
+ int status;
+
+ if (type & CHARGE_CONTROL_TYPE_STATUS) {
+ rc = sgm4056_chg_ctrl_get_status(chg_ctrl, &status);
+ if (rc) {
+ goto err;
+ }
+
+ if(data_func) {
+ data_func(chg_ctrl, data_arg, (void*)&status, CHARGE_CONTROL_TYPE_STATUS);
+ }
+ }
+
+err:
+ return rc;
+}
+
+static const struct charge_control_driver sgm4056_chg_ctrl_driver = {
+ .ccd_read = sgm4056_chg_ctrl_read,
+ .ccd_get_config = NULL,
+ .ccd_set_config = NULL,
+ .ccd_get_status = sgm4056_chg_ctrl_get_status,
+ .ccd_get_fault = NULL,
+ .ccd_enable = NULL,
+ .ccd_disable = NULL,
+};
+
+static void
+sgm4056_interrupt_event_handler(struct os_event *ev)
+{
+ struct sgm4056_dev *dev = (struct sgm4056_dev *)ev->ev_arg;
+ assert(dev);
+
+ charge_control_read(&dev->chg_ctrl, CHARGE_CONTROL_TYPE_STATUS,
+ NULL, NULL, OS_TIMEOUT_NEVER);
+}
+
+static void
+sgm4056_irq_handler(void *arg)
+{
+ struct sgm4056_dev *dev = (struct sgm4056_dev *)arg;
+ assert(dev);
+
+ os_eventq_put(os_eventq_dflt_get(), &dev->interrupt_event);
+}
+#endif
+
+int
+sgm4056_dev_init(struct os_dev *odev, void *arg) {
Review comment:
This curl bracket should be on the next line.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-core] mlaz commented on a change in pull request #2255: Add
SGM4056 charger driver to pinetime BSP
Posted by GitBox <gi...@apache.org>.
mlaz commented on a change in pull request #2255: Add SGM4056 charger driver to pinetime BSP
URL: https://github.com/apache/mynewt-core/pull/2255#discussion_r401104807
##########
File path: hw/drivers/chg_ctrl/sgm4056/src/sgm4056.c
##########
@@ -0,0 +1,192 @@
+/*
+ * Copyright 2020 Casper Meijn <ca...@meijn.net>
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "sgm4056/sgm4056.h"
+#include "hal/hal_gpio.h"
+
+#if MYNEWT_VAL(SGM4056_USE_CHARGE_CONTROL)
+
+static int
+sgm4056_chg_ctrl_get_status(struct charge_control * chg_ctrl, int * status)
+{
+ struct sgm4056_dev *dev;
+ int rc;
+
+ dev = (struct sgm4056_dev *)CHARGE_CONTROL_GET_DEVICE(chg_ctrl);
+ if (dev == NULL) {
+ rc = SYS_ENODEV;
+ goto err;
+ }
+
+ rc = sgm4056_get_charger_status(dev, (charge_control_status_t *)status);
+
+err:
+ return rc;
+}
+
+static int
+sgm4056_chg_ctrl_read(struct charge_control * chg_ctrl,
+ charge_control_type_t type, charge_control_data_func_t data_func,
+ void *data_arg, uint32_t timeout)
+{
+ int rc = 0;
+ int status;
+
+ if (type & CHARGE_CONTROL_TYPE_STATUS) {
+ rc = sgm4056_chg_ctrl_get_status(chg_ctrl, &status);
+ if (rc) {
+ goto err;
+ }
+
+ if(data_func) {
+ data_func(chg_ctrl, data_arg, (void*)&status, CHARGE_CONTROL_TYPE_STATUS);
+ }
+ }
+
+err:
+ return rc;
+}
+
+static const struct charge_control_driver sgm4056_chg_ctrl_driver = {
+ .ccd_read = sgm4056_chg_ctrl_read,
+ .ccd_get_config = NULL,
+ .ccd_set_config = NULL,
+ .ccd_get_status = sgm4056_chg_ctrl_get_status,
+ .ccd_get_fault = NULL,
+ .ccd_enable = NULL,
+ .ccd_disable = NULL,
+};
+
+static void
+sgm4056_interrupt_event_handler(struct os_event *ev)
+{
+ struct sgm4056_dev *dev = (struct sgm4056_dev *)ev->ev_arg;
+ assert(dev);
+
+ charge_control_read(&dev->chg_ctrl, CHARGE_CONTROL_TYPE_STATUS,
+ NULL, NULL, OS_TIMEOUT_NEVER);
+}
+
+static void
+sgm4056_irq_handler(void *arg)
+{
+ struct sgm4056_dev *dev = (struct sgm4056_dev *)arg;
+ assert(dev);
+
+ os_eventq_put(os_eventq_dflt_get(), &dev->interrupt_event);
+}
+#endif
+
+int
+sgm4056_dev_init(struct os_dev *odev, void *arg) {
+ struct sgm4056_dev *dev = (struct sgm4056_dev *)odev;
+ const struct sgm4056_dev_config *cfg = arg;
+ int rc = 0;
+
+ if (!dev) {
+ rc = SYS_ENODEV;
+ goto err;
+ }
+
+ dev->config = *cfg;
+
+#if MYNEWT_VAL(SGM4056_USE_CHARGE_CONTROL)
+ dev->interrupt_event.ev_cb = sgm4056_interrupt_event_handler;
+ dev->interrupt_event.ev_arg = dev;
+
+ rc = hal_gpio_irq_init(dev->config.power_presence_pin, sgm4056_irq_handler, dev, HAL_GPIO_TRIG_BOTH, HAL_GPIO_PULL_NONE);
+ hal_gpio_irq_enable(dev->config.power_presence_pin);
+#else
+ rc = hal_gpio_init_in(dev->config.power_presence_pin, HAL_GPIO_PULL_NONE);
+#endif
+ if (rc) {
+ goto err;
+ }
+
+ rc = hal_gpio_init_in(dev->config.charge_indicator_pin, HAL_GPIO_PULL_NONE);
+ if (rc) {
+ goto err;
+ }
+
+#if MYNEWT_VAL(SGM4056_USE_CHARGE_CONTROL)
+ rc = charge_control_init(&dev->chg_ctrl, odev);
+ if (rc) {
+ goto err;
+ }
+
+ rc = charge_control_set_driver(&dev->chg_ctrl, CHARGE_CONTROL_TYPE_STATUS,
+ (struct charge_control_driver *)&sgm4056_chg_ctrl_driver);
+ if (rc) {
+ goto err;
+ }
+
+ rc = charge_control_set_type_mask(&dev->chg_ctrl, CHARGE_CONTROL_TYPE_STATUS);
+ if (rc) {
+ goto err;
+ }
+
+ rc = charge_control_mgr_register(&dev->chg_ctrl);
+ if (rc) {
+ goto err;
+ }
+#endif
+
+err:
+ return rc;
+}
+
+int sgm4056_get_power_presence(struct sgm4056_dev *dev, int *power_present) {
Review comment:
Curl brackets supposed to be on the next line.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-core] mlaz commented on issue #2255: Add SGM4056 charger
driver to pinetime BSP
Posted by GitBox <gi...@apache.org>.
mlaz commented on issue #2255: Add SGM4056 charger driver to pinetime BSP
URL: https://github.com/apache/mynewt-core/pull/2255#issuecomment-607317147
Looks good, thanks for the changes, @caspermeijn
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-core] mlaz commented on a change in pull request #2255: Add
SGM4056 charger driver to pinetime BSP
Posted by GitBox <gi...@apache.org>.
mlaz commented on a change in pull request #2255: Add SGM4056 charger driver to pinetime BSP
URL: https://github.com/apache/mynewt-core/pull/2255#discussion_r401104932
##########
File path: hw/drivers/chg_ctrl/sgm4056/src/sgm4056.c
##########
@@ -0,0 +1,192 @@
+/*
+ * Copyright 2020 Casper Meijn <ca...@meijn.net>
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "sgm4056/sgm4056.h"
+#include "hal/hal_gpio.h"
+
+#if MYNEWT_VAL(SGM4056_USE_CHARGE_CONTROL)
+
+static int
+sgm4056_chg_ctrl_get_status(struct charge_control * chg_ctrl, int * status)
+{
+ struct sgm4056_dev *dev;
+ int rc;
+
+ dev = (struct sgm4056_dev *)CHARGE_CONTROL_GET_DEVICE(chg_ctrl);
+ if (dev == NULL) {
+ rc = SYS_ENODEV;
+ goto err;
+ }
+
+ rc = sgm4056_get_charger_status(dev, (charge_control_status_t *)status);
+
+err:
+ return rc;
+}
+
+static int
+sgm4056_chg_ctrl_read(struct charge_control * chg_ctrl,
+ charge_control_type_t type, charge_control_data_func_t data_func,
+ void *data_arg, uint32_t timeout)
+{
+ int rc = 0;
+ int status;
+
+ if (type & CHARGE_CONTROL_TYPE_STATUS) {
+ rc = sgm4056_chg_ctrl_get_status(chg_ctrl, &status);
+ if (rc) {
+ goto err;
+ }
+
+ if(data_func) {
+ data_func(chg_ctrl, data_arg, (void*)&status, CHARGE_CONTROL_TYPE_STATUS);
+ }
+ }
+
+err:
+ return rc;
+}
+
+static const struct charge_control_driver sgm4056_chg_ctrl_driver = {
+ .ccd_read = sgm4056_chg_ctrl_read,
+ .ccd_get_config = NULL,
+ .ccd_set_config = NULL,
+ .ccd_get_status = sgm4056_chg_ctrl_get_status,
+ .ccd_get_fault = NULL,
+ .ccd_enable = NULL,
+ .ccd_disable = NULL,
+};
+
+static void
+sgm4056_interrupt_event_handler(struct os_event *ev)
+{
+ struct sgm4056_dev *dev = (struct sgm4056_dev *)ev->ev_arg;
+ assert(dev);
+
+ charge_control_read(&dev->chg_ctrl, CHARGE_CONTROL_TYPE_STATUS,
+ NULL, NULL, OS_TIMEOUT_NEVER);
+}
+
+static void
+sgm4056_irq_handler(void *arg)
+{
+ struct sgm4056_dev *dev = (struct sgm4056_dev *)arg;
+ assert(dev);
+
+ os_eventq_put(os_eventq_dflt_get(), &dev->interrupt_event);
+}
+#endif
+
+int
+sgm4056_dev_init(struct os_dev *odev, void *arg) {
+ struct sgm4056_dev *dev = (struct sgm4056_dev *)odev;
+ const struct sgm4056_dev_config *cfg = arg;
+ int rc = 0;
+
+ if (!dev) {
+ rc = SYS_ENODEV;
+ goto err;
+ }
+
+ dev->config = *cfg;
+
+#if MYNEWT_VAL(SGM4056_USE_CHARGE_CONTROL)
+ dev->interrupt_event.ev_cb = sgm4056_interrupt_event_handler;
+ dev->interrupt_event.ev_arg = dev;
+
+ rc = hal_gpio_irq_init(dev->config.power_presence_pin, sgm4056_irq_handler, dev, HAL_GPIO_TRIG_BOTH, HAL_GPIO_PULL_NONE);
+ hal_gpio_irq_enable(dev->config.power_presence_pin);
+#else
+ rc = hal_gpio_init_in(dev->config.power_presence_pin, HAL_GPIO_PULL_NONE);
+#endif
+ if (rc) {
+ goto err;
+ }
+
+ rc = hal_gpio_init_in(dev->config.charge_indicator_pin, HAL_GPIO_PULL_NONE);
+ if (rc) {
+ goto err;
+ }
+
+#if MYNEWT_VAL(SGM4056_USE_CHARGE_CONTROL)
+ rc = charge_control_init(&dev->chg_ctrl, odev);
+ if (rc) {
+ goto err;
+ }
+
+ rc = charge_control_set_driver(&dev->chg_ctrl, CHARGE_CONTROL_TYPE_STATUS,
+ (struct charge_control_driver *)&sgm4056_chg_ctrl_driver);
+ if (rc) {
+ goto err;
+ }
+
+ rc = charge_control_set_type_mask(&dev->chg_ctrl, CHARGE_CONTROL_TYPE_STATUS);
+ if (rc) {
+ goto err;
+ }
+
+ rc = charge_control_mgr_register(&dev->chg_ctrl);
+ if (rc) {
+ goto err;
+ }
+#endif
+
+err:
+ return rc;
+}
+
+int sgm4056_get_power_presence(struct sgm4056_dev *dev, int *power_present) {
+ int gpio_value = hal_gpio_read(dev->config.power_presence_pin);
+
+ *power_present = (gpio_value == 0);
+ return 0;
+}
+
+int sgm4056_get_charge_indicator(struct sgm4056_dev *dev, int *charging) {
Review comment:
Same as above.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-core] apache-mynewt-bot commented on issue #2255: Add
SGM4056 charger driver to pinetime BSP
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on issue #2255: Add SGM4056 charger driver to pinetime BSP
URL: https://github.com/apache/mynewt-core/pull/2255#issuecomment-606501796
<!-- style-bot -->
## Style check summary
### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
#### hw/drivers/chg_ctrl/sgm4056/include/sgm4056/sgm4056.h
<details>
```diff
@@ -28,24 +28,24 @@
struct sgm4056_dev {
struct os_dev dev;
#if MYNEWT_VAL(SGM4056_USE_CHARGE_CONTROL)
- struct charge_control chg_ctrl;
+ struct charge_control chg_ctrl;
struct os_event interrupt_event;
#endif
struct sgm4056_dev_config config;
};
/**
-* Init function for SGM4056 charger
-* @return 0 on success, non-zero on failure
-*/
+ * Init function for SGM4056 charger
+ * @return 0 on success, non-zero on failure
+ */
int sgm4056_dev_init(struct os_dev *dev, void *arg);
/**
- * Reads the state of the power presence indication. Value is 1 when the input
+ * Reads the state of the power presence indication. Value is 1 when the input
* voltage is above the POR threshold but below the OVP threshold and 0 otherwise.
*
* @param dev The sgm4056 device to read on
- * @param power_present Power presence indication to be returned by the
+ * @param power_present Power presence indication to be returned by the
* function (0 if input voltage is not detected, 1 if it is detected)
*
* @return 0 on success, non-zero on failure
@@ -53,11 +53,11 @@
int sgm4056_get_power_presence(struct sgm4056_dev *dev, int *power_present);
/**
- * Reads the state of the charge indication. Value is 1 when a charge cycle
+ * Reads the state of the charge indication. Value is 1 when a charge cycle
* started and will be 0 when the end-of-charge (EOC) condition is met.
*
* @param dev The sgm4056 device to read on
- * @param charging Charge indication to be returned by the
+ * @param charging Charge indication to be returned by the
* function (0 if battery is not charging, 1 if it is charging)
*
* @return 0 on success, non-zero on failure
@@ -65,7 +65,7 @@
int sgm4056_get_charge_indicator(struct sgm4056_dev *dev, int *charging);
/**
- * Reads the state of the charger. This is a combination of the power presence
+ * Reads the state of the charger. This is a combination of the power presence
* and charge indications. Value is either:
* - CHARGE_CONTROL_STATUS_NO_SOURCE, when no power present
* - CHARGE_CONTROL_STATUS_CHARGING, when power present and charging
```
</details>
#### hw/drivers/chg_ctrl/sgm4056/src/sgm4056.c
<details>
```diff
@@ -19,7 +19,7 @@
#if MYNEWT_VAL(SGM4056_USE_CHARGE_CONTROL)
-static int
+static int
sgm4056_chg_ctrl_get_status(struct charge_control * chg_ctrl, int * status)
{
struct sgm4056_dev *dev;
@@ -37,10 +37,10 @@
return rc;
}
-static int
-sgm4056_chg_ctrl_read(struct charge_control * chg_ctrl,
- charge_control_type_t type, charge_control_data_func_t data_func,
- void *data_arg, uint32_t timeout)
+static int
+sgm4056_chg_ctrl_read(struct charge_control * chg_ctrl,
+ charge_control_type_t type, charge_control_data_func_t data_func,
+ void *data_arg, uint32_t timeout)
{
int rc = 0;
int status;
@@ -51,8 +51,8 @@
goto err;
}
- if(data_func) {
- data_func(chg_ctrl, data_arg, (void*)&status, CHARGE_CONTROL_TYPE_STATUS);
+ if (data_func) {
+ data_func(chg_ctrl, data_arg, (void *)&status, CHARGE_CONTROL_TYPE_STATUS);
}
}
@@ -80,7 +80,7 @@
NULL, NULL, OS_TIMEOUT_NEVER);
}
-static void
+static void
sgm4056_irq_handler(void *arg)
{
struct sgm4056_dev *dev = (struct sgm4056_dev *)arg;
@@ -90,8 +90,9 @@
}
#endif
-int
-sgm4056_dev_init(struct os_dev *odev, void *arg) {
+int
+sgm4056_dev_init(struct os_dev *odev, void *arg)
+{
struct sgm4056_dev *dev = (struct sgm4056_dev *)odev;
const struct sgm4056_dev_config *cfg = arg;
int rc = 0;
@@ -107,7 +108,8 @@
dev->interrupt_event.ev_cb = sgm4056_interrupt_event_handler;
dev->interrupt_event.ev_arg = dev;
- rc = hal_gpio_irq_init(dev->config.power_presence_pin, sgm4056_irq_handler, dev, HAL_GPIO_TRIG_BOTH, HAL_GPIO_PULL_NONE);
+ rc = hal_gpio_irq_init(dev->config.power_presence_pin, sgm4056_irq_handler, dev, HAL_GPIO_TRIG_BOTH,
+ HAL_GPIO_PULL_NONE);
hal_gpio_irq_enable(dev->config.power_presence_pin);
#else
rc = hal_gpio_init_in(dev->config.power_presence_pin, HAL_GPIO_PULL_NONE);
@@ -128,7 +130,7 @@
}
rc = charge_control_set_driver(&dev->chg_ctrl, CHARGE_CONTROL_TYPE_STATUS,
- (struct charge_control_driver *)&sgm4056_chg_ctrl_driver);
+ (struct charge_control_driver *)&sgm4056_chg_ctrl_driver);
if (rc) {
goto err;
}
@@ -148,21 +150,27 @@
return rc;
}
-int sgm4056_get_power_presence(struct sgm4056_dev *dev, int *power_present) {
+int
+sgm4056_get_power_presence(struct sgm4056_dev *dev, int *power_present)
+{
int gpio_value = hal_gpio_read(dev->config.power_presence_pin);
*power_present = (gpio_value == 0);
return 0;
}
-int sgm4056_get_charge_indicator(struct sgm4056_dev *dev, int *charging) {
+int
+sgm4056_get_charge_indicator(struct sgm4056_dev *dev, int *charging)
+{
int gpio_value = hal_gpio_read(dev->config.charge_indicator_pin);
*charging = (gpio_value == 0);
return 0;
}
-int sgm4056_get_charger_status(struct sgm4056_dev *dev, charge_control_status_t *charger_status) {
+int
+sgm4056_get_charger_status(struct sgm4056_dev *dev, charge_control_status_t *charger_status)
+{
int power_present;
int charging;
int rc;
```
</details>
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-core] mlaz commented on a change in pull request #2255: Add
SGM4056 charger driver to pinetime BSP
Posted by GitBox <gi...@apache.org>.
mlaz commented on a change in pull request #2255: Add SGM4056 charger driver to pinetime BSP
URL: https://github.com/apache/mynewt-core/pull/2255#discussion_r401102875
##########
File path: hw/drivers/chg_ctrl/sgm4056/src/sgm4056.c
##########
@@ -0,0 +1,192 @@
+/*
+ * Copyright 2020 Casper Meijn <ca...@meijn.net>
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "sgm4056/sgm4056.h"
+#include "hal/hal_gpio.h"
+
+#if MYNEWT_VAL(SGM4056_USE_CHARGE_CONTROL)
+
+static int
+sgm4056_chg_ctrl_get_status(struct charge_control * chg_ctrl, int * status)
+{
+ struct sgm4056_dev *dev;
+ int rc;
+
+ dev = (struct sgm4056_dev *)CHARGE_CONTROL_GET_DEVICE(chg_ctrl);
+ if (dev == NULL) {
+ rc = SYS_ENODEV;
+ goto err;
+ }
+
+ rc = sgm4056_get_charger_status(dev, (charge_control_status_t *)status);
+
+err:
+ return rc;
+}
+
+static int
+sgm4056_chg_ctrl_read(struct charge_control * chg_ctrl,
+ charge_control_type_t type, charge_control_data_func_t data_func,
+ void *data_arg, uint32_t timeout)
+{
+ int rc = 0;
+ int status;
+
+ if (type & CHARGE_CONTROL_TYPE_STATUS) {
+ rc = sgm4056_chg_ctrl_get_status(chg_ctrl, &status);
+ if (rc) {
+ goto err;
+ }
+
+ if(data_func) {
+ data_func(chg_ctrl, data_arg, (void*)&status, CHARGE_CONTROL_TYPE_STATUS);
+ }
+ }
+
+err:
+ return rc;
+}
+
+static const struct charge_control_driver sgm4056_chg_ctrl_driver = {
+ .ccd_read = sgm4056_chg_ctrl_read,
+ .ccd_get_config = NULL,
+ .ccd_set_config = NULL,
+ .ccd_get_status = sgm4056_chg_ctrl_get_status,
+ .ccd_get_fault = NULL,
+ .ccd_enable = NULL,
+ .ccd_disable = NULL,
+};
+
+static void
+sgm4056_interrupt_event_handler(struct os_event *ev)
+{
+ struct sgm4056_dev *dev = (struct sgm4056_dev *)ev->ev_arg;
+ assert(dev);
+
+ charge_control_read(&dev->chg_ctrl, CHARGE_CONTROL_TYPE_STATUS,
+ NULL, NULL, OS_TIMEOUT_NEVER);
+}
+
+static void
+sgm4056_irq_handler(void *arg)
+{
+ struct sgm4056_dev *dev = (struct sgm4056_dev *)arg;
+ assert(dev);
+
+ os_eventq_put(os_eventq_dflt_get(), &dev->interrupt_event);
+}
+#endif
+
+int
+sgm4056_dev_init(struct os_dev *odev, void *arg) {
+ struct sgm4056_dev *dev = (struct sgm4056_dev *)odev;
+ const struct sgm4056_dev_config *cfg = arg;
+ int rc = 0;
+
+ if (!dev) {
+ rc = SYS_ENODEV;
+ goto err;
+ }
+
+ dev->config = *cfg;
+
+#if MYNEWT_VAL(SGM4056_USE_CHARGE_CONTROL)
+ dev->interrupt_event.ev_cb = sgm4056_interrupt_event_handler;
+ dev->interrupt_event.ev_arg = dev;
+
+ rc = hal_gpio_irq_init(dev->config.power_presence_pin, sgm4056_irq_handler, dev, HAL_GPIO_TRIG_BOTH, HAL_GPIO_PULL_NONE);
+ hal_gpio_irq_enable(dev->config.power_presence_pin);
+#else
+ rc = hal_gpio_init_in(dev->config.power_presence_pin, HAL_GPIO_PULL_NONE);
+#endif
+ if (rc) {
+ goto err;
+ }
+
+ rc = hal_gpio_init_in(dev->config.charge_indicator_pin, HAL_GPIO_PULL_NONE);
+ if (rc) {
+ goto err;
+ }
+
+#if MYNEWT_VAL(SGM4056_USE_CHARGE_CONTROL)
+ rc = charge_control_init(&dev->chg_ctrl, odev);
+ if (rc) {
+ goto err;
+ }
+
+ rc = charge_control_set_driver(&dev->chg_ctrl, CHARGE_CONTROL_TYPE_STATUS,
+ (struct charge_control_driver *)&sgm4056_chg_ctrl_driver);
+ if (rc) {
+ goto err;
+ }
+
+ rc = charge_control_set_type_mask(&dev->chg_ctrl, CHARGE_CONTROL_TYPE_STATUS);
+ if (rc) {
+ goto err;
+ }
+
+ rc = charge_control_mgr_register(&dev->chg_ctrl);
+ if (rc) {
+ goto err;
+ }
+#endif
+
+err:
+ return rc;
+}
+
+int sgm4056_get_power_presence(struct sgm4056_dev *dev, int *power_present) {
+ int gpio_value = hal_gpio_read(dev->config.power_presence_pin);
+
+ *power_present = (gpio_value == 0);
+ return 0;
+}
+
+int sgm4056_get_charge_indicator(struct sgm4056_dev *dev, int *charging) {
+ int gpio_value = hal_gpio_read(dev->config.charge_indicator_pin);
+
+ *charging = (gpio_value == 0);
+ return 0;
+}
+
+int sgm4056_get_charger_status(struct sgm4056_dev *dev, charge_control_status_t *charger_status) {
Review comment:
Curl bracket should be on the next line and line is longer than 80 columns.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-core] apache-mynewt-bot commented on issue #2255: Add
SGM4056 charger driver to pinetime BSP
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on issue #2255: Add SGM4056 charger driver to pinetime BSP
URL: https://github.com/apache/mynewt-core/pull/2255#issuecomment-607092226
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-core] mlaz merged pull request #2255: Add SGM4056 charger
driver to pinetime BSP
Posted by GitBox <gi...@apache.org>.
mlaz merged pull request #2255: Add SGM4056 charger driver to pinetime BSP
URL: https://github.com/apache/mynewt-core/pull/2255
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-core] caspermeijn commented on a change in pull request
#2255: Add SGM4056 charger driver to pinetime BSP
Posted by GitBox <gi...@apache.org>.
caspermeijn commented on a change in pull request #2255: Add SGM4056 charger driver to pinetime BSP
URL: https://github.com/apache/mynewt-core/pull/2255#discussion_r401421249
##########
File path: hw/drivers/chg_ctrl/sgm4056/src/sgm4056.c
##########
@@ -0,0 +1,192 @@
+/*
+ * Copyright 2020 Casper Meijn <ca...@meijn.net>
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "sgm4056/sgm4056.h"
+#include "hal/hal_gpio.h"
+
+#if MYNEWT_VAL(SGM4056_USE_CHARGE_CONTROL)
+
+static int
+sgm4056_chg_ctrl_get_status(struct charge_control * chg_ctrl, int * status)
Review comment:
I fix this. The rebase was more complex then I expected, but I managed to figure it out ;-)
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-core] caspermeijn commented on issue #2255: Add SGM4056
charger driver to pinetime BSP
Posted by GitBox <gi...@apache.org>.
caspermeijn commented on issue #2255: Add SGM4056 charger driver to pinetime BSP
URL: https://github.com/apache/mynewt-core/pull/2255#issuecomment-607093858
I had some problems with the rebase and squash. But I fixed the history now ;-)
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-core] apache-mynewt-bot removed a comment on issue #2255:
Add SGM4056 charger driver to pinetime BSP
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on issue #2255: Add SGM4056 charger driver to pinetime BSP
URL: https://github.com/apache/mynewt-core/pull/2255#issuecomment-607083823
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-core] kasjer commented on a change in pull request #2255:
Add SGM4056 charger driver to pinetime BSP
Posted by GitBox <gi...@apache.org>.
kasjer commented on a change in pull request #2255: Add SGM4056 charger driver to pinetime BSP
URL: https://github.com/apache/mynewt-core/pull/2255#discussion_r400836974
##########
File path: hw/drivers/chg_ctrl/sgm4056/src/sgm4056.c
##########
@@ -0,0 +1,192 @@
+/*
+ * Copyright 2020 Casper Meijn <ca...@meijn.net>
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "sgm4056/sgm4056.h"
+#include "hal/hal_gpio.h"
+
+#if MYNEWT_VAL(SGM4056_USE_CHARGE_CONTROL)
+
+static int
+sgm4056_chg_ctrl_get_status(struct charge_control * chg_ctrl, int * status)
Review comment:
Mynewt code usually does not have space between * and variable even though it's not enforce by style checker as I see.
If you fill confident about force-pushing to you branch you can do it, then you changes will look perfect in git history once merged.
If not just add commit with minor style changes that checker complained about.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-core] kasjer commented on issue #2255: Add SGM4056 charger
driver to pinetime BSP
Posted by GitBox <gi...@apache.org>.
kasjer commented on issue #2255: Add SGM4056 charger driver to pinetime BSP
URL: https://github.com/apache/mynewt-core/pull/2255#issuecomment-607136582
> I had some problems with the rebase and squash. But I fixed the history now ;-)
Thanks, as soon as @mlaz is satisfied with his change requests it will be merged.
----------------------------------------------------------------
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:
users@infra.apache.org
With regards,
Apache Git Services