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 2021/05/27 13:57:08 UTC

[GitHub] [mynewt-core] kasjer opened a new pull request #2605: hw/drivers/uart: Add MAX3107 driver

kasjer opened a new pull request #2605:
URL: https://github.com/apache/mynewt-core/pull/2605


   This adds driver for max3107 (SPI to UART device).


-- 
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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2605: hw/drivers/uart: Add MAX3107 driver

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2605:
URL: https://github.com/apache/mynewt-core/pull/2605#discussion_r640866970



##########
File path: hw/drivers/uart/max3107/src/max3107.c
##########
@@ -0,0 +1,1059 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 <assert.h>
+#include <string.h>
+
+#include <os/mynewt.h>
+#include <bsp/bsp.h>
+#include <hal/hal_gpio.h>
+#include <hal/hal_uart.h>
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+#include <bus/drivers/spi_common.h>
+#endif
+#include <max3107/max3107.h>
+#include "max3107_priv.h"
+
+#include <uart/uart.h>
+
+static inline int
+max3107_lock(struct max3107_dev *dev)
+{
+    return os_error_to_sys(os_mutex_pend(&dev->lock,
+                                         os_time_ms_to_ticks32(MYNEWT_VAL(MAX3107_LOCK_TIMEOUT))));
+}
+
+static inline void
+max3107_unlock(struct max3107_dev *dev)
+{
+    int rc = os_mutex_release(&dev->lock);
+    assert(rc == 0);
+}
+
+void
+max3107_cs_activate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = (struct bus_spi_node *)&dev->dev;
+    hal_gpio_write(node->pin_cs, 0);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 0);
+#endif
+}
+
+void
+max3107_cs_deactivate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = &dev->dev;
+    hal_gpio_write(node->pin_cs, 1);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 1);
+#endif
+}
+
+int
+max3107_read_regs(struct max3107_dev *dev, uint8_t addr, void *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+#if !MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    uint8_t fast_buf[8];
+#endif
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;
+
+    if (size > 0) {
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+        rc = bus_node_simple_write_read_transact((struct os_dev *)&dev->dev,
+                                                 &addr, 1, buf, size);
+#else
+        max3107_cs_activate(dev);
+
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memset(fast_buf + 1, 0xFF, size);
+            /* Send command + address */
+            rc = hal_spi_txrx(dev->cfg.spi_num, fast_buf, fast_buf, size + 1);
+            if (rc == 0) {
+                memcpy(buf, fast_buf + 1, size);
+            }
+        } else {
+            /* Send command + address */
+            rc = hal_spi_txrx(dev->cfg.spi_num, &addr, NULL, 1);
+            if (rc == 0) {
+                /* For security mostly, do not output random data, fill it with FF */
+                memset(buf, 0xFF, size);
+                /* Tx buf does not matter, for simplicity pass read buffer */
+                rc = hal_spi_txrx(dev->cfg.spi_num, buf, buf, (int)size);
+            }
+        }
+
+        max3107_cs_deactivate(dev);
+#endif
+    }
+
+    if (locked) {
+        max3107_unlock(dev);
+    }
+
+    return rc;
+}
+
+int
+max3107_write_regs(struct max3107_dev *dev, uint8_t addr, const uint8_t *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+    uint8_t fast_buf[17];
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;
+
+    if (size) {
+        /* For writes set MSB */
+        addr |= 0x80;
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+        rc = bus_node_lock((struct os_dev *)&dev->dev,
+                           BUS_NODE_LOCK_DEFAULT_TIMEOUT);
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memcpy(fast_buf + 1, buf, size);
+            rc = bus_node_simple_write((struct os_dev *)&dev->dev, fast_buf, size + 1);
+        } else {
+            if (rc == 0) {
+                rc = bus_node_write((struct os_dev *)&dev->dev,
+                                    &addr, 1,
+                                    BUS_NODE_LOCK_DEFAULT_TIMEOUT, BUS_F_NOSTOP);
+                if (rc == 0) {
+                    rc = bus_node_simple_write((struct os_dev *)&dev->dev, buf, size);
+                }
+            }
+        }
+        (void)bus_node_unlock((struct os_dev *)&dev->dev);
+#else
+        max3107_cs_activate(dev);
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memcpy(fast_buf + 1, buf, size);
+            rc = hal_spi_txrx(dev->cfg.spi_num, fast_buf, NULL, size + 1);
+        } else {
+            rc = hal_spi_txrx(dev->cfg.spi_num, &addr, NULL, 1);
+            if (rc == 0) {
+                rc = hal_spi_txrx(dev->cfg.spi_num, (void *)buf, NULL, (int)size);
+            }
+        }
+        max3107_cs_deactivate(dev);
+#endif
+    }
+    if (locked) {
+        max3107_unlock(dev);
+    }
+
+    return rc;
+}
+
+static int
+max3107_write_reg(struct max3107_dev *dev, uint8_t addr, uint8_t val)
+{
+    return max3107_write_regs(dev, addr, &val, 1);
+}
+
+static int
+max3107_write_fifo(struct max3107_dev *dev, const uint8_t *buf, uint32_t size)
+{
+    return max3107_write_regs(dev, 0, buf, size);
+}
+
+int
+max3107_read_fifo(struct max3107_dev *dev, uint8_t *buf, uint32_t size)
+{
+    return max3107_read_regs(dev, 0, buf, size);
+}
+
+static const uint8_t factors[5] = { 1, 6, 48, 96, 144 };
+/* "F PLL in" from datasheet: Table 4. PLLFactor[1:0] Selection Guide */
+static const uint32_t fpllin_min[5] = { 1, 500000,  850000,  425000, 390000 };
+static const uint32_t fpllin_max[5] = { 1, 800000, 1200000, 1000000, 666666 };
+
+static uint32_t
+max3107_compute_clock_config(uint32_t clockf, uint32_t br, struct max3107_clock_cfg *cfg)
+{
+    uint32_t div;
+    uint32_t pre_div;
+    uint32_t pre_div_min;
+    uint32_t pre_div_max;
+    uint32_t mul;
+    uint32_t actual_br;
+    uint32_t best_br = 1;
+    uint32_t fref;
+    int factor_ix;
+    int max_factor = (cfg->clk_source & CLCSOURCE_PLLBYPASS) ? 1 : 5;
+    uint32_t mode_mul = (cfg->brg_config & BRGCONFIG_4XMODE) ? 4 : (cfg->brg_config & BRGCONFIG_2XMODE) ? 2 : 1;
+
+    /* Clock will be needed. */
+    cfg->clk_source |= CLCSOURCE_CLOCKEN;
+
+    /* If bypass was not disable at the start try finding more accurate clock settings. */
+    for (factor_ix = 0; factor_ix < max_factor; ++factor_ix) {
+        /*
+         * Pre divider does not apply when PLL is not used. Set lower and upper
+         * limits to 1 to have same code for PLL/non PLL.
+         */
+        if (factor_ix == 0) {
+            pre_div_min = 1;
+            pre_div_max = 1;
+        } else {
+            /* Lower and upper frequency limits used to get pre divider range */
+            pre_div_max = clockf / fpllin_min[factor_ix];
+            pre_div_min = (clockf + fpllin_max[factor_ix] - 1) / fpllin_max[factor_ix];
+            /* Make sure divider is in correct range. */
+            pre_div_min = min(63, pre_div_min);
+            pre_div_min = max(1, pre_div_min);
+            pre_div_max = min(63, pre_div_max);
+            pre_div_max = max(1, pre_div_max);
+        }
+        /* Loop for x1, x2 and x4 modes. */
+        for (mul = 1; mul <= mode_mul; mul <<= 1) {
+            for (pre_div = pre_div_min; pre_div <= pre_div_max; ++pre_div) {
+                fref = (clockf / pre_div) * factors[factor_ix];
+                div = (fref * mul + (br / 2)) / br;
+                div = max(div, 16);
+
+                actual_br = mul * fref / div;
+                if (abs((int)(actual_br - br)) < abs((int)(best_br - br))) {
+                    best_br = actual_br;
+                    cfg->div_lsb = (uint8_t)(div >> 4);
+                    cfg->div_msb = (uint8_t)(div >> 12);
+                    cfg->brg_config = (div & 0xF) | ((mul == 4) ? 0x20 : ((mul == 2) ? 0x10 : 0));
+                    /* If best choice is factor_ix == 0, no need for PLL */
+                    if (factor_ix == 0) {
+                        cfg->clk_source |= CLCSOURCE_PLLBYPASS;
+                        cfg->clk_source &= ~CLCSOURCE_PLLEN;
+                    } else {
+                        cfg->pll_config = pre_div | ((factor_ix - 1) << 6);
+                        cfg->clk_source &= ~CLCSOURCE_PLLBYPASS;
+                        cfg->clk_source |= CLCSOURCE_PLLEN;
+                    }
+                }
+            }
+        }
+    }
+
+    return best_br;
+}
+
+int
+max3107_config_uart(struct max3107_dev *dev, const struct uart_conf_port *conf)
+{
+    int rc;
+
+    if (dev->cfg.crystal_en) {
+        dev->regs.clock.clk_source |= CLCSOURCE_CRYSTALEN;
+    } else {
+        dev->regs.clock.clk_source &= ~CLCSOURCE_CRYSTALEN;
+    }
+    if (dev->cfg.no_pll) {
+        dev->regs.clock.clk_source |= CLCSOURCE_PLLBYPASS;
+    } else {
+        dev->regs.clock.clk_source &= ~CLCSOURCE_PLLBYPASS;
+    }
+    if (dev->cfg.allow_mul_4) {
+        dev->regs.clock.brg_config = BRGCONFIG_4XMODE;
+    } else if (dev->cfg.allow_mul_2) {
+        dev->regs.clock.brg_config = BRGCONFIG_2XMODE;
+    } else {
+        dev->regs.clock.brg_config = 0;
+    }
+    max3107_compute_clock_config(dev->cfg.osc_freq, conf->uc_speed, &dev->regs.clock);
+
+    rc = max3107_write_regs(dev, MAX3107_REG_PLLCONFIG, &dev->regs.clock.pll_config, 5);
+    if (rc) {
+        goto end;
+    }
+
+    dev->regs.modes.mode1 = MODE1_IRQSEL | (conf->uc_flow_ctl ? 0 : MODE1_RTSHIZ);
+    dev->regs.modes.mode2 = 0;
+    dev->regs.modes.lcr = (dev->uart.ud_conf_port.uc_stopbits == 1 ? 0 : LCR_STOPBITS) |
+                          (dev->uart.ud_conf_port.uc_databits - 5) |
+                          ((dev->uart.ud_conf_port.uc_parity != UART_PARITY_NONE) ? LCR_PARITYEN : 0 ) |
+                          ((dev->uart.ud_conf_port.uc_parity == UART_PARITY_EVEN) ? LCR_EVENPARITY : 0);
+    dev->regs.modes.rxtimeout = 2;
+    dev->regs.modes.hdplxdelay = 0;
+    dev->regs.modes.irda = 0;
+
+    rc = max3107_write_regs(dev, MAX3107_REG_MODE1, &dev->regs.modes.mode1, 6);
+    if (rc) {
+        goto end;
+    }
+
+    /* RTS activated at FIFO level 120. */
+    dev->regs.fifo.flow_lvl = 0xFF;
+    dev->regs.fifo.fifo_trg_lvl = 0x38;
+    rc = max3107_write_regs(dev, MAX3107_REG_FLOWLVL, &dev->regs.fifo.flow_lvl, 2);
+    if (rc) {
+        goto end;
+    }
+
+    dev->regs.flow.flow_ctrl = conf->uc_flow_ctl ? (FLOWCTRL_AUTOCTS | FLOWCTRL_AUTORTS) : 0;
+    max3107_write_reg(dev, MAX3107_REG_FLOWCTRL, dev->regs.flow.flow_ctrl);
+
+    dev->regs.ints.irq_en = IRQEN_LSRERRIEN | IRQEN_RXTRGIEN;
+    dev->regs.ints.lsr_int_en = LSRINTEN_FRAMEERRIEN | LSRINTEN_PARITYIEN |
+                                LSRINTEN_RBREAKIEN | LSRINTEN_ROVERRIEN | LSRINTEN_RTIMEOUTIEN;
+    rc = max3107_write_regs(dev, MAX3107_REG_IRQEN, &dev->regs.ints.irq_en, 3);
+    assert(dev->regs.ints.irq_en & IRQEN_LSRERRIEN);
+end:
+    return rc;
+}
+
+int
+max3107_rx_available(struct max3107_dev *dev)
+{
+    int rc = 0;
+
+    if (dev->regs.fifo.rx_fifo_lvl == 0) {
+        rc = max3107_read_regs(dev, MAX3107_REG_RXFIFOLVL, &dev->regs.fifo.rx_fifo_lvl, 1);
+    }
+    if (rc == 0) {
+        rc = dev->regs.fifo.rx_fifo_lvl;
+    }
+
+    return rc;
+}
+
+int
+max3107_tx_available(struct max3107_dev *dev)
+{
+    int rc = 0;
+
+    if (dev->regs.fifo.tx_fifo_lvl >= 128) {
+        rc = max3107_read_regs(dev, MAX3107_REG_TXFIFOLVL, &dev->regs.fifo.tx_fifo_lvl, 1);
+    }
+    if (rc == 0) {
+        rc = 128 - dev->regs.fifo.tx_fifo_lvl;
+    }
+
+    return rc;
+}
+
+static int
+max3107_irqen_set(struct max3107_dev *dev, uint8_t enabled_bits)
+{
+    int rc = 0;
+
+    uint8_t irq_en = dev->regs.ints.irq_en | enabled_bits;
+
+    if (irq_en != dev->regs.ints.irq_en) {
+        dev->regs.ints.irq_en = irq_en;
+        rc = max3107_write_reg(dev, MAX3107_REG_IRQEN, dev->regs.ints.irq_en);
+    }
+
+    return rc;
+}
+
+static int
+max3107_irqen_clear(struct max3107_dev *dev, uint8_t cleared_bits)
+{
+    int rc = 0;
+
+    uint8_t irq_en = dev->regs.ints.irq_en & ~cleared_bits;
+
+    if (irq_en != dev->regs.ints.irq_en) {
+        dev->regs.ints.irq_en = irq_en;
+        rc = max3107_write_reg(dev, MAX3107_REG_IRQEN, dev->regs.ints.irq_en);
+    }
+
+    return rc;
+}
+
+int
+max3107_read(struct max3107_dev *dev, void *buf, size_t size)
+{
+    int rc = 0;
+
+    if (dev->regs.fifo.rx_fifo_lvl == 0) {
+        /* Read number of bytes currently in RX FIFO first. */
+        rc = max3107_read_regs(dev, MAX3107_REG_RXFIFOLVL, &dev->regs.fifo.rx_fifo_lvl, 1);
+    }
+
+    if (rc == 0) {
+        size = min(dev->regs.fifo.rx_fifo_lvl, size);
+        if (size > 0) {
+            rc = max3107_read_fifo(dev, buf, size);
+        }
+        if (rc == 0) {
+            dev->readable_notified = false;
+            /* Update FIFO level for potential future use. */
+            dev->regs.fifo.rx_fifo_lvl -= size;
+            /*
+             * If FIFO was possible emptied, read FIFO level in next interrupt,
+             * in case data was coming while FIFO was being read.
+             */
+            dev->rx_fifo_emptied = dev->regs.fifo.rx_fifo_lvl == 0;
+            rc = (int)size;
+        }
+    }
+    return rc;
+}
+
+int
+max3107_write(struct max3107_dev *dev, const void *buf, size_t size)
+{
+    size_t fifo_space = 128 - dev->regs.fifo.tx_fifo_lvl;
+    int rc = 0;
+
+    /*
+     * If FIFO level was read before and there is enough data, there is
+     * no need to check FIFO level again.
+     */
+    if (size > fifo_space) {
+        /* Read number of bytes currently in TX FIFO */
+        rc = max3107_read_regs(dev, MAX3107_REG_TXFIFOLVL, &dev->regs.fifo.tx_fifo_lvl, 1);
+        fifo_space = 128 - dev->regs.fifo.tx_fifo_lvl;
+    }
+    if (rc == 0) {
+        size = min(size, fifo_space);
+        if (size) {
+            rc = max3107_write_fifo(dev, buf, size);
+            dev->regs.fifo.tx_fifo_lvl += size;
+            dev->writable_notified = false;
+        }
+        if (rc == 0) {
+            rc = (int)size;
+        }
+    }
+
+    return rc;
+}
+
+static void
+max3107_isr_cb(struct max3107_dev *dev)
+{
+    int rc;
+    int sr;
+    bool read_irq;
+    uint8_t isr = dev->regs.ints.isr;
+
+    OS_ENTER_CRITICAL(sr);
+    read_irq = dev->irq_pending;
+    dev->irq_pending = false;
+    OS_EXIT_CRITICAL(sr);
+
+    if (read_irq) {
+        /* Read ISR, LSR, (and also LSRIntEn which is between them) */
+        rc = max3107_read_regs(dev, MAX3107_REG_ISR, &dev->regs.ints.isr, 3);
+        if (rc) {
+            dev->irq_pending = true;
+            goto end;
+        } else {
+            /*
+             * In usual case reading ISR clears interrupt status bits on the device.
+             * However it's possible that interrupt raised during read of interrupt registers.
+             * In that case irq pin will remain low. MCU interrupt is set to be triggered on
+             * falling edge, read irq pin, if it stays low, new interrupt just arrived.
+             */
+            dev->irq_pending = hal_gpio_read(dev->cfg.irq_pin) == 0;
+        }
+    }
+
+    if (dev->regs.ints.lsr & LSR_RXBREAK) {
+        if (dev->client && dev->client->break_detected) {
+            dev->client->break_detected(dev);
+        }
+    }
+    if (dev->regs.ints.lsr & LSR_RXOVERRUN) {
+        if (dev->client && dev->client->error) {
+            dev->client->error(dev);
+        }
+    }
+    if (dev->regs.ints.lsr & LSR_FRAMEERR) {
+        if (dev->client && dev->client->error) {
+            dev->client->error(dev);
+        }
+    }
+    /* TX Empty interrupt, no need to read TX FIFO level */
+    if (dev->regs.ints.isr & ISR_TXEMPTYINT) {
+        dev->regs.fifo.tx_fifo_lvl = 0;
+    } else if (0 != (isr & ISR_TFIFOTRIGINT) &&
+               0 == (dev->regs.ints.isr & ISR_TFIFOTRIGINT) &&
+               (dev->regs.fifo.fifo_trg_lvl & FIFOTRGLVL_TXTRIG) < (dev->regs.fifo.tx_fifo_lvl >> 3)) {
+        dev->regs.fifo.tx_fifo_lvl = (dev->regs.fifo.fifo_trg_lvl & FIFOTRGLVL_TXTRIG) << 3;
+    }
+    /* RX Empty interrupt, no need to read RX FIFO level */
+    if (dev->regs.ints.isr & ISR_RXEMPTYINT) {
+        dev->regs.fifo.rx_fifo_lvl = 0;
+        dev->rx_fifo_emptied = false;
+    }
+    /* RX Trigger level interrupt, there are some bytes in FIFO, RX FIFO level read can be skipped */
+    if (dev->regs.ints.isr & ISR_RFIFOTRIGINT) {
+        if (dev->regs.fifo.rx_fifo_lvl < ((dev->regs.fifo.fifo_trg_lvl & FIFOTRGLVL_RXTRIG) >> 1)) {
+            dev->regs.fifo.rx_fifo_lvl = (dev->regs.fifo.fifo_trg_lvl & FIFOTRGLVL_RXTRIG) >> 1;
+        }
+    }
+    /*
+     * Read both FIFO levels when:
+     * - RX timeout interrupt arrived, more RX related interrupts may never come
+     * - TX FIFO level as went above half the size FIFO, if transmission is ongoing
+     *   tx_fifi_lvl is not updated by software so periodically read TX FIFO level
+     *   instead waiting for FIFO empty interrupt.
+     * - All know data from RX FIFO was read, there may be more bytes that arrived
+     *   after FIFO level was checked, to avoid data overwrite (or timeout with
+     *   flow control enabled) check FIFO level now.
+     */
+    if ((dev->regs.ints.lsr & LSR_RTIMEOUT) ||
+        (dev->regs.fifo.tx_fifo_lvl > 64) ||
+        dev->rx_fifo_emptied) {
+        rc = max3107_read_regs(dev, MAX3107_REG_TXFIFOLVL, &dev->regs.fifo.tx_fifo_lvl, 2);
+        if (rc == 0) {
+            dev->rx_fifo_emptied = false;
+        }
+    }
+    while (dev->regs.fifo.rx_fifo_lvl > 0 && dev->client && !dev->readable_notified) {
+        dev->readable_notified = true;
+        dev->client->readable(dev);
+    }
+    if (dev->regs.fifo.tx_fifo_lvl < 128) {
+        if (dev->client && !dev->writable_notified) {
+            dev->writable_notified = true;
+            dev->client->writable(dev);
+        }
+    }
+end:
+    if (dev->irq_pending) {
+        os_eventq_put(dev->event_queue, &dev->irq_event);
+    }
+}
+
+static void
+max3107_disable_rx_int(struct max3107_dev *dev)
+{
+    max3107_irqen_clear(dev, IRQEN_RXTRGIEN | IRQEN_LSRERRIEN);
+}
+
+static void
+max3107_enable_rx_int(struct max3107_dev *dev)
+{
+    max3107_irqen_set(dev, IRQEN_RXTRGIEN | IRQEN_LSRERRIEN);
+}
+
+static void
+max3107_disable_tx_int(struct max3107_dev *dev)
+{
+    max3107_irqen_clear(dev, IRQEN_TXEMTYIEN | IRQEN_TXTRGIEN);
+}
+
+static void
+max3107_enable_tx_int(struct max3107_dev *dev)
+{
+    max3107_irqen_set(dev, IRQEN_TXEMTYIEN | IRQEN_TXTRGIEN);
+}
+
+static int
+max3107_drain_rx_buffer(struct max3107_dev *dev)
+{
+    int i;
+
+    for (i = 0; i < dev->rx_buf_count; ++i) {
+        if (dev->uc_rx_char(dev->uc_cb_arg, dev->rx_buf[i]) < 0) {
+            dev->readable_notified = true;
+            if (i) {
+                memmove(dev->rx_buf, dev->rx_buf + i, dev->rx_buf_count - i);
+            }
+            break;
+        }
+    }
+    dev->rx_buf_count -= i;
+
+    return i;
+}
+
+static int
+max3107_drain_tx_cache(struct max3107_dev *dev)
+{
+    int rc = 0;
+
+    if (dev->tx_buf_count) {
+        rc = max3107_write(dev, dev->tx_buf, dev->tx_buf_count);
+        if (rc > 0) {
+            dev->tx_buf_count -= rc;
+            if (dev->tx_buf_count) {
+                memmove(dev->tx_buf, dev->tx_buf + rc, dev->tx_buf_count);
+            }
+        }
+    }
+
+    return rc;
+}
+
+int
+max3107_set_client(struct max3107_dev *dev, const struct max3107_client *client)
+{
+    hal_gpio_irq_disable(dev->cfg.irq_pin);
+
+    dev->client = client;
+
+    if (client) {
+        hal_gpio_irq_enable(dev->cfg.irq_pin);
+        max3107_isr_cb(dev);

Review comment:
       Handle error here.




-- 
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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2605: hw/drivers/uart: Add MAX3107 driver

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2605:
URL: https://github.com/apache/mynewt-core/pull/2605#discussion_r640901509



##########
File path: hw/drivers/uart/max3107/include/max3107/max3107.h
##########
@@ -0,0 +1,172 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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.
+ */
+
+#ifndef __DRIVERS_MAX3107_H_
+#define __DRIVERS_MAX3107_H_
+
+#include <stdint.h>
+#include <uart/uart.h>
+#include <hal/hal_spi.h>
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+#include <bus/drivers/spi_common.h>
+#endif
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+struct max3107_dev;
+
+struct max3107_cfg {
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node_cfg node_cfg;
+#else
+    struct hal_spi_settings spi_settings;
+    int spi_num;
+    int ss_pin;
+#endif
+    /**< External clock/oscillator frequency in Hz */
+    uint32_t osc_freq;
+    /**< IRQ Pin */
+    int8_t irq_pin;
+    int8_t ldoen_pin;
+    /**< 1 - External crystal oscillator, 0 - external clock */
+    int8_t crystal_en: 1;
+    /**< 1 - To disable PLL */
+    uint8_t no_pll: 1;
+    /**< 1 - To enable x 4 mode */
+    uint8_t allow_mul_4: 1;
+    /**< 1 - To enable x 4 mode */
+    uint8_t allow_mul_2: 1;
+};
+
+struct max3107_client {
+    /**< Function that will be called from process context when data can be read. */
+    void (*readable)(struct max3107_dev *dev);

Review comment:
       How is the driver going to detect read/write errors without a return value ?




-- 
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



[GitHub] [mynewt-core] vrahane commented on pull request #2605: hw/drivers/uart: Add MAX3107 driver

Posted by GitBox <gi...@apache.org>.
vrahane commented on pull request #2605:
URL: https://github.com/apache/mynewt-core/pull/2605#issuecomment-849849497


   Also, a basic CLI to query registers of the chip would be useful for debugging.


-- 
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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2605: hw/drivers/uart: Add MAX3107 driver

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2605:
URL: https://github.com/apache/mynewt-core/pull/2605#discussion_r640865835



##########
File path: hw/drivers/uart/max3107/src/max3107.c
##########
@@ -0,0 +1,1059 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 <assert.h>
+#include <string.h>
+
+#include <os/mynewt.h>
+#include <bsp/bsp.h>
+#include <hal/hal_gpio.h>
+#include <hal/hal_uart.h>
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+#include <bus/drivers/spi_common.h>
+#endif
+#include <max3107/max3107.h>
+#include "max3107_priv.h"
+
+#include <uart/uart.h>
+
+static inline int
+max3107_lock(struct max3107_dev *dev)
+{
+    return os_error_to_sys(os_mutex_pend(&dev->lock,
+                                         os_time_ms_to_ticks32(MYNEWT_VAL(MAX3107_LOCK_TIMEOUT))));
+}
+
+static inline void
+max3107_unlock(struct max3107_dev *dev)
+{
+    int rc = os_mutex_release(&dev->lock);
+    assert(rc == 0);
+}
+
+void
+max3107_cs_activate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = (struct bus_spi_node *)&dev->dev;
+    hal_gpio_write(node->pin_cs, 0);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 0);
+#endif
+}
+
+void
+max3107_cs_deactivate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = &dev->dev;
+    hal_gpio_write(node->pin_cs, 1);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 1);
+#endif
+}
+
+int
+max3107_read_regs(struct max3107_dev *dev, uint8_t addr, void *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+#if !MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    uint8_t fast_buf[8];
+#endif
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;
+
+    if (size > 0) {
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+        rc = bus_node_simple_write_read_transact((struct os_dev *)&dev->dev,
+                                                 &addr, 1, buf, size);
+#else
+        max3107_cs_activate(dev);
+
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memset(fast_buf + 1, 0xFF, size);
+            /* Send command + address */
+            rc = hal_spi_txrx(dev->cfg.spi_num, fast_buf, fast_buf, size + 1);
+            if (rc == 0) {
+                memcpy(buf, fast_buf + 1, size);
+            }
+        } else {
+            /* Send command + address */
+            rc = hal_spi_txrx(dev->cfg.spi_num, &addr, NULL, 1);
+            if (rc == 0) {
+                /* For security mostly, do not output random data, fill it with FF */
+                memset(buf, 0xFF, size);
+                /* Tx buf does not matter, for simplicity pass read buffer */
+                rc = hal_spi_txrx(dev->cfg.spi_num, buf, buf, (int)size);
+            }
+        }
+
+        max3107_cs_deactivate(dev);
+#endif
+    }
+
+    if (locked) {
+        max3107_unlock(dev);
+    }
+
+    return rc;
+}
+
+int
+max3107_write_regs(struct max3107_dev *dev, uint8_t addr, const uint8_t *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+    uint8_t fast_buf[17];
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;
+
+    if (size) {
+        /* For writes set MSB */
+        addr |= 0x80;
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+        rc = bus_node_lock((struct os_dev *)&dev->dev,
+                           BUS_NODE_LOCK_DEFAULT_TIMEOUT);
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memcpy(fast_buf + 1, buf, size);
+            rc = bus_node_simple_write((struct os_dev *)&dev->dev, fast_buf, size + 1);
+        } else {
+            if (rc == 0) {
+                rc = bus_node_write((struct os_dev *)&dev->dev,
+                                    &addr, 1,
+                                    BUS_NODE_LOCK_DEFAULT_TIMEOUT, BUS_F_NOSTOP);
+                if (rc == 0) {
+                    rc = bus_node_simple_write((struct os_dev *)&dev->dev, buf, size);
+                }
+            }
+        }
+        (void)bus_node_unlock((struct os_dev *)&dev->dev);
+#else
+        max3107_cs_activate(dev);
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memcpy(fast_buf + 1, buf, size);
+            rc = hal_spi_txrx(dev->cfg.spi_num, fast_buf, NULL, size + 1);
+        } else {
+            rc = hal_spi_txrx(dev->cfg.spi_num, &addr, NULL, 1);
+            if (rc == 0) {
+                rc = hal_spi_txrx(dev->cfg.spi_num, (void *)buf, NULL, (int)size);
+            }
+        }
+        max3107_cs_deactivate(dev);
+#endif
+    }
+    if (locked) {
+        max3107_unlock(dev);
+    }
+
+    return rc;
+}
+
+static int
+max3107_write_reg(struct max3107_dev *dev, uint8_t addr, uint8_t val)
+{
+    return max3107_write_regs(dev, addr, &val, 1);
+}
+
+static int
+max3107_write_fifo(struct max3107_dev *dev, const uint8_t *buf, uint32_t size)
+{
+    return max3107_write_regs(dev, 0, buf, size);
+}
+
+int
+max3107_read_fifo(struct max3107_dev *dev, uint8_t *buf, uint32_t size)
+{
+    return max3107_read_regs(dev, 0, buf, size);
+}
+
+static const uint8_t factors[5] = { 1, 6, 48, 96, 144 };
+/* "F PLL in" from datasheet: Table 4. PLLFactor[1:0] Selection Guide */
+static const uint32_t fpllin_min[5] = { 1, 500000,  850000,  425000, 390000 };
+static const uint32_t fpllin_max[5] = { 1, 800000, 1200000, 1000000, 666666 };
+
+static uint32_t
+max3107_compute_clock_config(uint32_t clockf, uint32_t br, struct max3107_clock_cfg *cfg)
+{
+    uint32_t div;
+    uint32_t pre_div;
+    uint32_t pre_div_min;
+    uint32_t pre_div_max;
+    uint32_t mul;
+    uint32_t actual_br;
+    uint32_t best_br = 1;
+    uint32_t fref;
+    int factor_ix;
+    int max_factor = (cfg->clk_source & CLCSOURCE_PLLBYPASS) ? 1 : 5;
+    uint32_t mode_mul = (cfg->brg_config & BRGCONFIG_4XMODE) ? 4 : (cfg->brg_config & BRGCONFIG_2XMODE) ? 2 : 1;
+
+    /* Clock will be needed. */
+    cfg->clk_source |= CLCSOURCE_CLOCKEN;
+
+    /* If bypass was not disable at the start try finding more accurate clock settings. */
+    for (factor_ix = 0; factor_ix < max_factor; ++factor_ix) {
+        /*
+         * Pre divider does not apply when PLL is not used. Set lower and upper
+         * limits to 1 to have same code for PLL/non PLL.
+         */
+        if (factor_ix == 0) {
+            pre_div_min = 1;
+            pre_div_max = 1;
+        } else {
+            /* Lower and upper frequency limits used to get pre divider range */
+            pre_div_max = clockf / fpllin_min[factor_ix];
+            pre_div_min = (clockf + fpllin_max[factor_ix] - 1) / fpllin_max[factor_ix];
+            /* Make sure divider is in correct range. */
+            pre_div_min = min(63, pre_div_min);
+            pre_div_min = max(1, pre_div_min);
+            pre_div_max = min(63, pre_div_max);
+            pre_div_max = max(1, pre_div_max);
+        }
+        /* Loop for x1, x2 and x4 modes. */
+        for (mul = 1; mul <= mode_mul; mul <<= 1) {
+            for (pre_div = pre_div_min; pre_div <= pre_div_max; ++pre_div) {
+                fref = (clockf / pre_div) * factors[factor_ix];
+                div = (fref * mul + (br / 2)) / br;
+                div = max(div, 16);
+
+                actual_br = mul * fref / div;
+                if (abs((int)(actual_br - br)) < abs((int)(best_br - br))) {
+                    best_br = actual_br;
+                    cfg->div_lsb = (uint8_t)(div >> 4);
+                    cfg->div_msb = (uint8_t)(div >> 12);
+                    cfg->brg_config = (div & 0xF) | ((mul == 4) ? 0x20 : ((mul == 2) ? 0x10 : 0));
+                    /* If best choice is factor_ix == 0, no need for PLL */
+                    if (factor_ix == 0) {
+                        cfg->clk_source |= CLCSOURCE_PLLBYPASS;
+                        cfg->clk_source &= ~CLCSOURCE_PLLEN;
+                    } else {
+                        cfg->pll_config = pre_div | ((factor_ix - 1) << 6);
+                        cfg->clk_source &= ~CLCSOURCE_PLLBYPASS;
+                        cfg->clk_source |= CLCSOURCE_PLLEN;
+                    }
+                }
+            }
+        }
+    }
+
+    return best_br;
+}
+
+int
+max3107_config_uart(struct max3107_dev *dev, const struct uart_conf_port *conf)
+{
+    int rc;
+
+    if (dev->cfg.crystal_en) {
+        dev->regs.clock.clk_source |= CLCSOURCE_CRYSTALEN;
+    } else {
+        dev->regs.clock.clk_source &= ~CLCSOURCE_CRYSTALEN;
+    }
+    if (dev->cfg.no_pll) {
+        dev->regs.clock.clk_source |= CLCSOURCE_PLLBYPASS;
+    } else {
+        dev->regs.clock.clk_source &= ~CLCSOURCE_PLLBYPASS;
+    }
+    if (dev->cfg.allow_mul_4) {
+        dev->regs.clock.brg_config = BRGCONFIG_4XMODE;
+    } else if (dev->cfg.allow_mul_2) {
+        dev->regs.clock.brg_config = BRGCONFIG_2XMODE;
+    } else {
+        dev->regs.clock.brg_config = 0;
+    }
+    max3107_compute_clock_config(dev->cfg.osc_freq, conf->uc_speed, &dev->regs.clock);
+
+    rc = max3107_write_regs(dev, MAX3107_REG_PLLCONFIG, &dev->regs.clock.pll_config, 5);
+    if (rc) {
+        goto end;
+    }
+
+    dev->regs.modes.mode1 = MODE1_IRQSEL | (conf->uc_flow_ctl ? 0 : MODE1_RTSHIZ);
+    dev->regs.modes.mode2 = 0;
+    dev->regs.modes.lcr = (dev->uart.ud_conf_port.uc_stopbits == 1 ? 0 : LCR_STOPBITS) |
+                          (dev->uart.ud_conf_port.uc_databits - 5) |
+                          ((dev->uart.ud_conf_port.uc_parity != UART_PARITY_NONE) ? LCR_PARITYEN : 0 ) |
+                          ((dev->uart.ud_conf_port.uc_parity == UART_PARITY_EVEN) ? LCR_EVENPARITY : 0);
+    dev->regs.modes.rxtimeout = 2;
+    dev->regs.modes.hdplxdelay = 0;
+    dev->regs.modes.irda = 0;
+
+    rc = max3107_write_regs(dev, MAX3107_REG_MODE1, &dev->regs.modes.mode1, 6);
+    if (rc) {
+        goto end;
+    }
+
+    /* RTS activated at FIFO level 120. */
+    dev->regs.fifo.flow_lvl = 0xFF;
+    dev->regs.fifo.fifo_trg_lvl = 0x38;
+    rc = max3107_write_regs(dev, MAX3107_REG_FLOWLVL, &dev->regs.fifo.flow_lvl, 2);
+    if (rc) {
+        goto end;
+    }
+
+    dev->regs.flow.flow_ctrl = conf->uc_flow_ctl ? (FLOWCTRL_AUTOCTS | FLOWCTRL_AUTORTS) : 0;
+    max3107_write_reg(dev, MAX3107_REG_FLOWCTRL, dev->regs.flow.flow_ctrl);

Review comment:
       We need to handle error here.




-- 
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



[GitHub] [mynewt-core] sjanc commented on a change in pull request #2605: hw/drivers/uart: Add MAX3107 driver

Posted by GitBox <gi...@apache.org>.
sjanc commented on a change in pull request #2605:
URL: https://github.com/apache/mynewt-core/pull/2605#discussion_r649086710



##########
File path: hw/drivers/uart/max3107/src/max3107.c
##########
@@ -0,0 +1,1090 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 <assert.h>
+#include <string.h>
+
+#include <os/mynewt.h>
+#include <bsp/bsp.h>
+#include <hal/hal_gpio.h>
+#include <hal/hal_uart.h>
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+#include <bus/drivers/spi_common.h>
+#endif
+#include <max3107/max3107.h>
+#include "max3107_priv.h"
+
+#include <uart/uart.h>
+
+static inline int
+max3107_lock(struct max3107_dev *dev)
+{
+    return os_error_to_sys(os_mutex_pend(&dev->lock,
+                                         os_time_ms_to_ticks32(MYNEWT_VAL(MAX3107_LOCK_TIMEOUT))));
+}
+
+static inline void
+max3107_unlock(struct max3107_dev *dev)
+{
+    int rc = os_mutex_release(&dev->lock);
+    assert(rc == 0);
+}
+
+void
+max3107_cs_activate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = (struct bus_spi_node *)&dev->dev;
+    hal_gpio_write(node->pin_cs, 0);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 0);
+#endif
+}
+
+void
+max3107_cs_deactivate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = &dev->dev;
+    hal_gpio_write(node->pin_cs, 1);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 1);
+#endif
+}
+
+int
+max3107_read_regs(struct max3107_dev *dev, uint8_t addr, void *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+#if !MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    uint8_t fast_buf[8];
+#endif
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;
+
+    if (size > 0) {
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+        rc = bus_node_simple_write_read_transact((struct os_dev *)&dev->dev,
+                                                 &addr, 1, buf, size);
+#else
+        max3107_cs_activate(dev);
+
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memset(fast_buf + 1, 0xFF, size);
+            /* Send command + address */
+            rc = hal_spi_txrx(dev->cfg.spi_num, fast_buf, fast_buf, size + 1);
+            if (rc == 0) {
+                memcpy(buf, fast_buf + 1, size);
+            }
+        } else {
+            /* Send command + address */
+            rc = hal_spi_txrx(dev->cfg.spi_num, &addr, NULL, 1);
+            if (rc == 0) {
+                /* For security mostly, do not output random data, fill it with FF */
+                memset(buf, 0xFF, size);
+                /* Tx buf does not matter, for simplicity pass read buffer */
+                rc = hal_spi_txrx(dev->cfg.spi_num, buf, buf, (int)size);
+            }
+        }
+
+        max3107_cs_deactivate(dev);
+#endif
+    }
+
+    if (locked) {
+        max3107_unlock(dev);
+    }
+
+    return rc;
+}
+
+int
+max3107_write_regs(struct max3107_dev *dev, uint8_t addr, const uint8_t *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+    uint8_t fast_buf[17];
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;
+
+    if (size) {
+        /* For writes set MSB */
+        addr |= 0x80;
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+        rc = bus_node_lock((struct os_dev *)&dev->dev,
+                           BUS_NODE_LOCK_DEFAULT_TIMEOUT);
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memcpy(fast_buf + 1, buf, size);
+            rc = bus_node_simple_write((struct os_dev *)&dev->dev, fast_buf, size + 1);
+        } else {
+            if (rc == 0) {
+                rc = bus_node_write((struct os_dev *)&dev->dev,
+                                    &addr, 1,
+                                    BUS_NODE_LOCK_DEFAULT_TIMEOUT, BUS_F_NOSTOP);
+                if (rc == 0) {
+                    rc = bus_node_simple_write((struct os_dev *)&dev->dev, buf, size);
+                }
+            }
+        }
+        (void)bus_node_unlock((struct os_dev *)&dev->dev);
+#else
+        max3107_cs_activate(dev);
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memcpy(fast_buf + 1, buf, size);
+            rc = hal_spi_txrx(dev->cfg.spi_num, fast_buf, NULL, size + 1);
+        } else {
+            rc = hal_spi_txrx(dev->cfg.spi_num, &addr, NULL, 1);
+            if (rc == 0) {
+                rc = hal_spi_txrx(dev->cfg.spi_num, (void *)buf, NULL, (int)size);
+            }
+        }
+        max3107_cs_deactivate(dev);
+#endif
+    }
+    if (locked) {
+        max3107_unlock(dev);
+    }
+
+    return rc;
+}
+
+static int
+max3107_write_reg(struct max3107_dev *dev, uint8_t addr, uint8_t val)
+{
+    return max3107_write_regs(dev, addr, &val, 1);
+}
+
+static int
+max3107_write_fifo(struct max3107_dev *dev, const uint8_t *buf, uint32_t size)
+{
+    return max3107_write_regs(dev, 0, buf, size);
+}
+
+static int
+max3107_read_fifo(struct max3107_dev *dev, uint8_t *buf, uint32_t size)
+{
+    return max3107_read_regs(dev, 0, buf, size);
+}
+
+static const uint8_t factors[5] = { 1, 6, 48, 96, 144 };
+/* "F PLL in" from datasheet: Table 4. PLLFactor[1:0] Selection Guide */
+static const uint32_t fpllin_min[5] = { 1, 500000,  850000,  425000, 390000 };
+static const uint32_t fpllin_max[5] = { 1, 800000, 1200000, 1000000, 666666 };
+
+static uint32_t
+max3107_compute_clock_config(uint32_t clockf, uint32_t br, struct max3107_clock_cfg *cfg)
+{
+    uint32_t div;
+    uint32_t pre_div;
+    uint32_t pre_div_min;
+    uint32_t pre_div_max;
+    uint32_t mul;
+    uint32_t actual_br;
+    uint32_t best_br = 1;
+    uint32_t fref;
+    int factor_ix;
+    int max_factor = (cfg->clk_source & CLCSOURCE_PLLBYPASS) ? 1 : 5;
+    uint32_t mode_mul = (cfg->brg_config & BRGCONFIG_4XMODE) ? 4 : (cfg->brg_config & BRGCONFIG_2XMODE) ? 2 : 1;
+
+    /* Clock will be needed. */
+    cfg->clk_source |= CLCSOURCE_CLOCKEN;
+
+    /* If bypass was not disable at the start try finding more accurate clock settings. */
+    for (factor_ix = 0; factor_ix < max_factor; ++factor_ix) {
+        /*
+         * Pre divider does not apply when PLL is not used. Set lower and upper
+         * limits to 1 to have same code for PLL/non PLL.
+         */
+        if (factor_ix == 0) {
+            pre_div_min = 1;
+            pre_div_max = 1;
+        } else {
+            /* Lower and upper frequency limits used to get pre divider range */
+            pre_div_max = clockf / fpllin_min[factor_ix];
+            pre_div_min = (clockf + fpllin_max[factor_ix] - 1) / fpllin_max[factor_ix];
+            /* Make sure divider is in correct range. */
+            pre_div_min = min(63, pre_div_min);
+            pre_div_min = max(1, pre_div_min);
+            pre_div_max = min(63, pre_div_max);
+            pre_div_max = max(1, pre_div_max);
+        }
+        /* Loop for x1, x2 and x4 modes. */
+        for (mul = 1; mul <= mode_mul; mul <<= 1) {
+            for (pre_div = pre_div_min; pre_div <= pre_div_max; ++pre_div) {
+                fref = (clockf / pre_div) * factors[factor_ix];
+                div = (fref * mul + (br / 2)) / br;
+                div = max(div, 16);
+
+                actual_br = mul * fref / div;
+                if (abs((int)(actual_br - br)) < abs((int)(best_br - br))) {
+                    best_br = actual_br;
+                    cfg->div_lsb = (uint8_t)(div >> 4);
+                    cfg->div_msb = (uint8_t)(div >> 12);
+                    cfg->brg_config = (div & 0xF) | ((mul == 4) ? 0x20 : ((mul == 2) ? 0x10 : 0));
+                    /* If best choice is factor_ix == 0, no need for PLL */
+                    if (factor_ix == 0) {
+                        cfg->clk_source |= CLCSOURCE_PLLBYPASS;
+                        cfg->clk_source &= ~CLCSOURCE_PLLEN;
+                    } else {
+                        cfg->pll_config = pre_div | ((factor_ix - 1) << 6);
+                        cfg->clk_source &= ~CLCSOURCE_PLLBYPASS;
+                        cfg->clk_source |= CLCSOURCE_PLLEN;
+                    }
+                }
+            }
+        }
+    }
+
+    return best_br;
+}
+
+int
+max3107_config_uart(struct max3107_dev *dev, const struct uart_conf_port *conf)
+{
+    int rc;
+
+    if (dev->cfg.crystal_en) {
+        dev->regs.clock.clk_source |= CLCSOURCE_CRYSTALEN;
+    } else {
+        dev->regs.clock.clk_source &= ~CLCSOURCE_CRYSTALEN;
+    }
+    if (dev->cfg.no_pll) {
+        dev->regs.clock.clk_source |= CLCSOURCE_PLLBYPASS;
+    } else {
+        dev->regs.clock.clk_source &= ~CLCSOURCE_PLLBYPASS;
+    }
+    if (dev->cfg.allow_mul_4) {
+        dev->regs.clock.brg_config = BRGCONFIG_4XMODE;
+    } else if (dev->cfg.allow_mul_2) {
+        dev->regs.clock.brg_config = BRGCONFIG_2XMODE;
+    } else {
+        dev->regs.clock.brg_config = 0;
+    }
+    max3107_compute_clock_config(dev->cfg.osc_freq, conf->uc_speed, &dev->regs.clock);
+
+    rc = max3107_write_regs(dev, MAX3107_REG_PLLCONFIG, &dev->regs.clock.pll_config, 5);
+    if (rc) {
+        goto end;
+    }
+
+    dev->regs.modes.mode1 = MODE1_IRQSEL | (conf->uc_flow_ctl ? 0 : MODE1_RTSHIZ);
+    dev->regs.modes.mode2 = 0;
+    dev->regs.modes.lcr = (dev->uart.ud_conf_port.uc_stopbits == 1 ? 0 : LCR_STOPBITS) |
+                          (dev->uart.ud_conf_port.uc_databits - 5) |
+                          ((dev->uart.ud_conf_port.uc_parity != UART_PARITY_NONE) ? LCR_PARITYEN : 0 ) |
+                          ((dev->uart.ud_conf_port.uc_parity == UART_PARITY_EVEN) ? LCR_EVENPARITY : 0);
+    dev->regs.modes.rxtimeout = 2;
+    dev->regs.modes.hdplxdelay = 0;
+    dev->regs.modes.irda = 0;
+
+    rc = max3107_write_regs(dev, MAX3107_REG_MODE1, &dev->regs.modes.mode1, 6);
+    if (rc) {
+        goto end;
+    }
+
+    /* RTS activated at FIFO level 120. */
+    dev->regs.fifo.flow_lvl = 0xFF;
+    dev->regs.fifo.fifo_trg_lvl = (dev->cfg.rx_trigger_level >> 3) << 4 | (dev->cfg.tx_trigger_level >> 3);
+    rc = max3107_write_regs(dev, MAX3107_REG_FLOWLVL, &dev->regs.fifo.flow_lvl, 2);
+    if (rc) {
+        goto end;
+    }
+
+    dev->regs.flow.flow_ctrl = conf->uc_flow_ctl ? (FLOWCTRL_AUTOCTS | FLOWCTRL_AUTORTS) : 0;
+    rc = max3107_write_reg(dev, MAX3107_REG_FLOWCTRL, dev->regs.flow.flow_ctrl);
+    if (rc) {
+        goto end;
+    }
+
+    dev->regs.ints.irq_en = IRQEN_LSRERRIEN | IRQEN_RXTRGIEN;
+    dev->regs.ints.lsr_int_en = LSRINTEN_FRAMEERRIEN | LSRINTEN_PARITYIEN |
+                                LSRINTEN_RBREAKIEN | LSRINTEN_ROVERRIEN | LSRINTEN_RTIMEOUTIEN;
+    rc = max3107_write_regs(dev, MAX3107_REG_IRQEN, &dev->regs.ints.irq_en, 3);
+end:

Review comment:
       alright, fair enough




-- 
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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2605: hw/drivers/uart: Add MAX3107 driver

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2605:
URL: https://github.com/apache/mynewt-core/pull/2605#discussion_r640903818



##########
File path: hw/drivers/uart/max3107/src/max3107.c
##########
@@ -0,0 +1,1059 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 <assert.h>
+#include <string.h>
+
+#include <os/mynewt.h>
+#include <bsp/bsp.h>
+#include <hal/hal_gpio.h>
+#include <hal/hal_uart.h>
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+#include <bus/drivers/spi_common.h>
+#endif
+#include <max3107/max3107.h>
+#include "max3107_priv.h"
+
+#include <uart/uart.h>
+
+static inline int
+max3107_lock(struct max3107_dev *dev)
+{
+    return os_error_to_sys(os_mutex_pend(&dev->lock,
+                                         os_time_ms_to_ticks32(MYNEWT_VAL(MAX3107_LOCK_TIMEOUT))));
+}
+
+static inline void
+max3107_unlock(struct max3107_dev *dev)
+{
+    int rc = os_mutex_release(&dev->lock);
+    assert(rc == 0);
+}
+
+void
+max3107_cs_activate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = (struct bus_spi_node *)&dev->dev;
+    hal_gpio_write(node->pin_cs, 0);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 0);
+#endif
+}
+
+void
+max3107_cs_deactivate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = &dev->dev;
+    hal_gpio_write(node->pin_cs, 1);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 1);
+#endif
+}
+
+int
+max3107_read_regs(struct max3107_dev *dev, uint8_t addr, void *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+#if !MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    uint8_t fast_buf[8];
+#endif
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;
+
+    if (size > 0) {
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+        rc = bus_node_simple_write_read_transact((struct os_dev *)&dev->dev,
+                                                 &addr, 1, buf, size);
+#else
+        max3107_cs_activate(dev);
+
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memset(fast_buf + 1, 0xFF, size);
+            /* Send command + address */
+            rc = hal_spi_txrx(dev->cfg.spi_num, fast_buf, fast_buf, size + 1);
+            if (rc == 0) {
+                memcpy(buf, fast_buf + 1, size);
+            }
+        } else {
+            /* Send command + address */
+            rc = hal_spi_txrx(dev->cfg.spi_num, &addr, NULL, 1);
+            if (rc == 0) {
+                /* For security mostly, do not output random data, fill it with FF */
+                memset(buf, 0xFF, size);
+                /* Tx buf does not matter, for simplicity pass read buffer */
+                rc = hal_spi_txrx(dev->cfg.spi_num, buf, buf, (int)size);
+            }
+        }
+
+        max3107_cs_deactivate(dev);
+#endif
+    }
+
+    if (locked) {
+        max3107_unlock(dev);
+    }
+
+    return rc;
+}
+
+int
+max3107_write_regs(struct max3107_dev *dev, uint8_t addr, const uint8_t *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+    uint8_t fast_buf[17];
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;
+
+    if (size) {
+        /* For writes set MSB */
+        addr |= 0x80;
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+        rc = bus_node_lock((struct os_dev *)&dev->dev,
+                           BUS_NODE_LOCK_DEFAULT_TIMEOUT);
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memcpy(fast_buf + 1, buf, size);
+            rc = bus_node_simple_write((struct os_dev *)&dev->dev, fast_buf, size + 1);
+        } else {
+            if (rc == 0) {
+                rc = bus_node_write((struct os_dev *)&dev->dev,
+                                    &addr, 1,
+                                    BUS_NODE_LOCK_DEFAULT_TIMEOUT, BUS_F_NOSTOP);
+                if (rc == 0) {
+                    rc = bus_node_simple_write((struct os_dev *)&dev->dev, buf, size);
+                }
+            }
+        }
+        (void)bus_node_unlock((struct os_dev *)&dev->dev);
+#else
+        max3107_cs_activate(dev);
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memcpy(fast_buf + 1, buf, size);
+            rc = hal_spi_txrx(dev->cfg.spi_num, fast_buf, NULL, size + 1);
+        } else {
+            rc = hal_spi_txrx(dev->cfg.spi_num, &addr, NULL, 1);
+            if (rc == 0) {
+                rc = hal_spi_txrx(dev->cfg.spi_num, (void *)buf, NULL, (int)size);
+            }
+        }
+        max3107_cs_deactivate(dev);
+#endif
+    }
+    if (locked) {
+        max3107_unlock(dev);
+    }
+
+    return rc;
+}
+
+static int
+max3107_write_reg(struct max3107_dev *dev, uint8_t addr, uint8_t val)
+{
+    return max3107_write_regs(dev, addr, &val, 1);
+}
+
+static int
+max3107_write_fifo(struct max3107_dev *dev, const uint8_t *buf, uint32_t size)
+{
+    return max3107_write_regs(dev, 0, buf, size);
+}
+
+int
+max3107_read_fifo(struct max3107_dev *dev, uint8_t *buf, uint32_t size)
+{
+    return max3107_read_regs(dev, 0, buf, size);
+}
+
+static const uint8_t factors[5] = { 1, 6, 48, 96, 144 };
+/* "F PLL in" from datasheet: Table 4. PLLFactor[1:0] Selection Guide */
+static const uint32_t fpllin_min[5] = { 1, 500000,  850000,  425000, 390000 };
+static const uint32_t fpllin_max[5] = { 1, 800000, 1200000, 1000000, 666666 };
+
+static uint32_t
+max3107_compute_clock_config(uint32_t clockf, uint32_t br, struct max3107_clock_cfg *cfg)
+{
+    uint32_t div;
+    uint32_t pre_div;
+    uint32_t pre_div_min;
+    uint32_t pre_div_max;
+    uint32_t mul;
+    uint32_t actual_br;
+    uint32_t best_br = 1;
+    uint32_t fref;
+    int factor_ix;
+    int max_factor = (cfg->clk_source & CLCSOURCE_PLLBYPASS) ? 1 : 5;
+    uint32_t mode_mul = (cfg->brg_config & BRGCONFIG_4XMODE) ? 4 : (cfg->brg_config & BRGCONFIG_2XMODE) ? 2 : 1;
+
+    /* Clock will be needed. */
+    cfg->clk_source |= CLCSOURCE_CLOCKEN;
+
+    /* If bypass was not disable at the start try finding more accurate clock settings. */
+    for (factor_ix = 0; factor_ix < max_factor; ++factor_ix) {
+        /*
+         * Pre divider does not apply when PLL is not used. Set lower and upper
+         * limits to 1 to have same code for PLL/non PLL.
+         */
+        if (factor_ix == 0) {
+            pre_div_min = 1;
+            pre_div_max = 1;
+        } else {
+            /* Lower and upper frequency limits used to get pre divider range */
+            pre_div_max = clockf / fpllin_min[factor_ix];
+            pre_div_min = (clockf + fpllin_max[factor_ix] - 1) / fpllin_max[factor_ix];
+            /* Make sure divider is in correct range. */
+            pre_div_min = min(63, pre_div_min);
+            pre_div_min = max(1, pre_div_min);
+            pre_div_max = min(63, pre_div_max);
+            pre_div_max = max(1, pre_div_max);
+        }
+        /* Loop for x1, x2 and x4 modes. */
+        for (mul = 1; mul <= mode_mul; mul <<= 1) {
+            for (pre_div = pre_div_min; pre_div <= pre_div_max; ++pre_div) {
+                fref = (clockf / pre_div) * factors[factor_ix];
+                div = (fref * mul + (br / 2)) / br;
+                div = max(div, 16);
+
+                actual_br = mul * fref / div;
+                if (abs((int)(actual_br - br)) < abs((int)(best_br - br))) {
+                    best_br = actual_br;
+                    cfg->div_lsb = (uint8_t)(div >> 4);
+                    cfg->div_msb = (uint8_t)(div >> 12);
+                    cfg->brg_config = (div & 0xF) | ((mul == 4) ? 0x20 : ((mul == 2) ? 0x10 : 0));
+                    /* If best choice is factor_ix == 0, no need for PLL */
+                    if (factor_ix == 0) {
+                        cfg->clk_source |= CLCSOURCE_PLLBYPASS;
+                        cfg->clk_source &= ~CLCSOURCE_PLLEN;
+                    } else {
+                        cfg->pll_config = pre_div | ((factor_ix - 1) << 6);
+                        cfg->clk_source &= ~CLCSOURCE_PLLBYPASS;
+                        cfg->clk_source |= CLCSOURCE_PLLEN;
+                    }
+                }
+            }
+        }
+    }
+
+    return best_br;
+}
+
+int
+max3107_config_uart(struct max3107_dev *dev, const struct uart_conf_port *conf)
+{
+    int rc;
+
+    if (dev->cfg.crystal_en) {
+        dev->regs.clock.clk_source |= CLCSOURCE_CRYSTALEN;
+    } else {
+        dev->regs.clock.clk_source &= ~CLCSOURCE_CRYSTALEN;
+    }
+    if (dev->cfg.no_pll) {
+        dev->regs.clock.clk_source |= CLCSOURCE_PLLBYPASS;
+    } else {
+        dev->regs.clock.clk_source &= ~CLCSOURCE_PLLBYPASS;
+    }
+    if (dev->cfg.allow_mul_4) {
+        dev->regs.clock.brg_config = BRGCONFIG_4XMODE;
+    } else if (dev->cfg.allow_mul_2) {
+        dev->regs.clock.brg_config = BRGCONFIG_2XMODE;
+    } else {
+        dev->regs.clock.brg_config = 0;
+    }
+    max3107_compute_clock_config(dev->cfg.osc_freq, conf->uc_speed, &dev->regs.clock);

Review comment:
       I see, makes sense. some comment or a log statement will be helpful in understanding what the real Vs specified config is.




-- 
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



[GitHub] [mynewt-core] kasjer commented on a change in pull request #2605: hw/drivers/uart: Add MAX3107 driver

Posted by GitBox <gi...@apache.org>.
kasjer commented on a change in pull request #2605:
URL: https://github.com/apache/mynewt-core/pull/2605#discussion_r648328693



##########
File path: hw/drivers/uart/max3107/src/max3107.c
##########
@@ -0,0 +1,1090 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 <assert.h>
+#include <string.h>
+
+#include <os/mynewt.h>
+#include <bsp/bsp.h>
+#include <hal/hal_gpio.h>
+#include <hal/hal_uart.h>
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+#include <bus/drivers/spi_common.h>
+#endif
+#include <max3107/max3107.h>
+#include "max3107_priv.h"
+
+#include <uart/uart.h>
+
+static inline int
+max3107_lock(struct max3107_dev *dev)
+{
+    return os_error_to_sys(os_mutex_pend(&dev->lock,
+                                         os_time_ms_to_ticks32(MYNEWT_VAL(MAX3107_LOCK_TIMEOUT))));
+}
+
+static inline void
+max3107_unlock(struct max3107_dev *dev)
+{
+    int rc = os_mutex_release(&dev->lock);
+    assert(rc == 0);
+}
+
+void
+max3107_cs_activate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = (struct bus_spi_node *)&dev->dev;
+    hal_gpio_write(node->pin_cs, 0);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 0);
+#endif
+}
+
+void
+max3107_cs_deactivate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = &dev->dev;
+    hal_gpio_write(node->pin_cs, 1);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 1);
+#endif
+}
+
+int
+max3107_read_regs(struct max3107_dev *dev, uint8_t addr, void *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+#if !MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    uint8_t fast_buf[8];
+#endif
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;

Review comment:
       yes




-- 
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



[GitHub] [mynewt-core] kasjer commented on a change in pull request #2605: hw/drivers/uart: Add MAX3107 driver

Posted by GitBox <gi...@apache.org>.
kasjer commented on a change in pull request #2605:
URL: https://github.com/apache/mynewt-core/pull/2605#discussion_r640889054



##########
File path: hw/drivers/uart/max3107/syscfg.yml
##########
@@ -0,0 +1,134 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you 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.
+#
+
+syscfg.defs:
+    MAX3107_0:
+        description: >
+            Create SPI RAM device instance.
+        value: 0
+
+    MAX3107_0_NAME:
+        description: >
+            MAX3107 device name.
+        value: '"max3107_0"'
+
+    MAX3107_0_UART_NAME:
+        description: >
+            MAX3107 UART device name.
+        value: '"uart4"'
+
+    MAX3107_0_SPI_BAUDRATE:
+        description: >
+            MAX3107 SPI clock speed in kHz.
+        value: 4000
+
+    MAX3107_0_UART_BAUDRATE:
+        description: >
+            MAX3107 UART clock speed in bits/second.
+        value: 115200
+
+    MAX3107_0_UART_DATA_BITS:
+        description: >
+            MAX3107 number of data bits.
+        range: 5,6,7,8
+        value: 8
+
+    MAX3107_0_UART_PARITY:
+        description: >
+            MAX3107 parity 0 - no parity, 1 - odd, 2 - even.
+        range: 0,1,2
+        value: 0
+
+    MAX3107_0_UART_STOP_BITS:
+        description: >
+            MAX3107 UART stop bits 1 or 2.
+        range: 1,2
+        value: 1
+
+    MAX3107_0_UART_FLOW_CONTROL:
+        description: >
+            MAX3107 UART flow control 0 - none, 1 - RTS CTS.
+        range: 0,1
+        value: 0
+
+    MAX3107_0_CS_PIN:
+        description: >
+            Chip select pin for MAX3107.
+        value: -1
+
+    MAX3107_0_SPI_BUS:
+        description: >
+            SPI interface bus name for MAX3107.
+        value : '"spi0"'
+
+    MAX3107_0_SPI_NUM:
+        description: >
+            SPI interface used for SPI RAM.
+        value: 0
+
+    MAX3107_0_OSC_FREQ:
+        description: >
+            External clock/oscillator frequency.
+        value:

Review comment:
       There is not default value, it must be set according to actual external component value.




-- 
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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2605: hw/drivers/uart: Add MAX3107 driver

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2605:
URL: https://github.com/apache/mynewt-core/pull/2605#discussion_r640864963



##########
File path: hw/drivers/uart/max3107/src/max3107.c
##########
@@ -0,0 +1,1059 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 <assert.h>
+#include <string.h>
+
+#include <os/mynewt.h>
+#include <bsp/bsp.h>
+#include <hal/hal_gpio.h>
+#include <hal/hal_uart.h>
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+#include <bus/drivers/spi_common.h>
+#endif
+#include <max3107/max3107.h>
+#include "max3107_priv.h"
+
+#include <uart/uart.h>
+
+static inline int
+max3107_lock(struct max3107_dev *dev)
+{
+    return os_error_to_sys(os_mutex_pend(&dev->lock,
+                                         os_time_ms_to_ticks32(MYNEWT_VAL(MAX3107_LOCK_TIMEOUT))));
+}
+
+static inline void
+max3107_unlock(struct max3107_dev *dev)
+{
+    int rc = os_mutex_release(&dev->lock);
+    assert(rc == 0);
+}
+
+void
+max3107_cs_activate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = (struct bus_spi_node *)&dev->dev;
+    hal_gpio_write(node->pin_cs, 0);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 0);
+#endif
+}
+
+void
+max3107_cs_deactivate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = &dev->dev;
+    hal_gpio_write(node->pin_cs, 1);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 1);
+#endif
+}
+
+int
+max3107_read_regs(struct max3107_dev *dev, uint8_t addr, void *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+#if !MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    uint8_t fast_buf[8];
+#endif
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;
+
+    if (size > 0) {
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+        rc = bus_node_simple_write_read_transact((struct os_dev *)&dev->dev,
+                                                 &addr, 1, buf, size);
+#else
+        max3107_cs_activate(dev);
+
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memset(fast_buf + 1, 0xFF, size);
+            /* Send command + address */
+            rc = hal_spi_txrx(dev->cfg.spi_num, fast_buf, fast_buf, size + 1);
+            if (rc == 0) {
+                memcpy(buf, fast_buf + 1, size);
+            }
+        } else {
+            /* Send command + address */
+            rc = hal_spi_txrx(dev->cfg.spi_num, &addr, NULL, 1);
+            if (rc == 0) {
+                /* For security mostly, do not output random data, fill it with FF */
+                memset(buf, 0xFF, size);
+                /* Tx buf does not matter, for simplicity pass read buffer */
+                rc = hal_spi_txrx(dev->cfg.spi_num, buf, buf, (int)size);
+            }
+        }
+
+        max3107_cs_deactivate(dev);
+#endif
+    }
+
+    if (locked) {
+        max3107_unlock(dev);
+    }
+
+    return rc;
+}
+
+int
+max3107_write_regs(struct max3107_dev *dev, uint8_t addr, const uint8_t *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+    uint8_t fast_buf[17];
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;
+
+    if (size) {
+        /* For writes set MSB */
+        addr |= 0x80;
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+        rc = bus_node_lock((struct os_dev *)&dev->dev,
+                           BUS_NODE_LOCK_DEFAULT_TIMEOUT);
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memcpy(fast_buf + 1, buf, size);
+            rc = bus_node_simple_write((struct os_dev *)&dev->dev, fast_buf, size + 1);
+        } else {
+            if (rc == 0) {
+                rc = bus_node_write((struct os_dev *)&dev->dev,
+                                    &addr, 1,
+                                    BUS_NODE_LOCK_DEFAULT_TIMEOUT, BUS_F_NOSTOP);
+                if (rc == 0) {
+                    rc = bus_node_simple_write((struct os_dev *)&dev->dev, buf, size);
+                }
+            }
+        }
+        (void)bus_node_unlock((struct os_dev *)&dev->dev);
+#else
+        max3107_cs_activate(dev);
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memcpy(fast_buf + 1, buf, size);
+            rc = hal_spi_txrx(dev->cfg.spi_num, fast_buf, NULL, size + 1);
+        } else {
+            rc = hal_spi_txrx(dev->cfg.spi_num, &addr, NULL, 1);
+            if (rc == 0) {
+                rc = hal_spi_txrx(dev->cfg.spi_num, (void *)buf, NULL, (int)size);
+            }
+        }
+        max3107_cs_deactivate(dev);
+#endif
+    }
+    if (locked) {
+        max3107_unlock(dev);
+    }
+
+    return rc;
+}
+
+static int
+max3107_write_reg(struct max3107_dev *dev, uint8_t addr, uint8_t val)
+{
+    return max3107_write_regs(dev, addr, &val, 1);
+}
+
+static int
+max3107_write_fifo(struct max3107_dev *dev, const uint8_t *buf, uint32_t size)
+{
+    return max3107_write_regs(dev, 0, buf, size);
+}
+
+int
+max3107_read_fifo(struct max3107_dev *dev, uint8_t *buf, uint32_t size)
+{
+    return max3107_read_regs(dev, 0, buf, size);
+}
+
+static const uint8_t factors[5] = { 1, 6, 48, 96, 144 };
+/* "F PLL in" from datasheet: Table 4. PLLFactor[1:0] Selection Guide */
+static const uint32_t fpllin_min[5] = { 1, 500000,  850000,  425000, 390000 };
+static const uint32_t fpllin_max[5] = { 1, 800000, 1200000, 1000000, 666666 };
+
+static uint32_t
+max3107_compute_clock_config(uint32_t clockf, uint32_t br, struct max3107_clock_cfg *cfg)
+{
+    uint32_t div;
+    uint32_t pre_div;
+    uint32_t pre_div_min;
+    uint32_t pre_div_max;
+    uint32_t mul;
+    uint32_t actual_br;
+    uint32_t best_br = 1;
+    uint32_t fref;
+    int factor_ix;
+    int max_factor = (cfg->clk_source & CLCSOURCE_PLLBYPASS) ? 1 : 5;
+    uint32_t mode_mul = (cfg->brg_config & BRGCONFIG_4XMODE) ? 4 : (cfg->brg_config & BRGCONFIG_2XMODE) ? 2 : 1;
+
+    /* Clock will be needed. */
+    cfg->clk_source |= CLCSOURCE_CLOCKEN;
+
+    /* If bypass was not disable at the start try finding more accurate clock settings. */
+    for (factor_ix = 0; factor_ix < max_factor; ++factor_ix) {
+        /*
+         * Pre divider does not apply when PLL is not used. Set lower and upper
+         * limits to 1 to have same code for PLL/non PLL.
+         */
+        if (factor_ix == 0) {
+            pre_div_min = 1;
+            pre_div_max = 1;
+        } else {
+            /* Lower and upper frequency limits used to get pre divider range */
+            pre_div_max = clockf / fpllin_min[factor_ix];
+            pre_div_min = (clockf + fpllin_max[factor_ix] - 1) / fpllin_max[factor_ix];
+            /* Make sure divider is in correct range. */
+            pre_div_min = min(63, pre_div_min);
+            pre_div_min = max(1, pre_div_min);
+            pre_div_max = min(63, pre_div_max);
+            pre_div_max = max(1, pre_div_max);
+        }
+        /* Loop for x1, x2 and x4 modes. */
+        for (mul = 1; mul <= mode_mul; mul <<= 1) {
+            for (pre_div = pre_div_min; pre_div <= pre_div_max; ++pre_div) {
+                fref = (clockf / pre_div) * factors[factor_ix];
+                div = (fref * mul + (br / 2)) / br;
+                div = max(div, 16);
+
+                actual_br = mul * fref / div;
+                if (abs((int)(actual_br - br)) < abs((int)(best_br - br))) {
+                    best_br = actual_br;
+                    cfg->div_lsb = (uint8_t)(div >> 4);
+                    cfg->div_msb = (uint8_t)(div >> 12);
+                    cfg->brg_config = (div & 0xF) | ((mul == 4) ? 0x20 : ((mul == 2) ? 0x10 : 0));
+                    /* If best choice is factor_ix == 0, no need for PLL */
+                    if (factor_ix == 0) {
+                        cfg->clk_source |= CLCSOURCE_PLLBYPASS;
+                        cfg->clk_source &= ~CLCSOURCE_PLLEN;
+                    } else {
+                        cfg->pll_config = pre_div | ((factor_ix - 1) << 6);
+                        cfg->clk_source &= ~CLCSOURCE_PLLBYPASS;
+                        cfg->clk_source |= CLCSOURCE_PLLEN;
+                    }
+                }
+            }
+        }
+    }
+
+    return best_br;
+}
+
+int
+max3107_config_uart(struct max3107_dev *dev, const struct uart_conf_port *conf)
+{
+    int rc;
+
+    if (dev->cfg.crystal_en) {
+        dev->regs.clock.clk_source |= CLCSOURCE_CRYSTALEN;
+    } else {
+        dev->regs.clock.clk_source &= ~CLCSOURCE_CRYSTALEN;
+    }
+    if (dev->cfg.no_pll) {
+        dev->regs.clock.clk_source |= CLCSOURCE_PLLBYPASS;
+    } else {
+        dev->regs.clock.clk_source &= ~CLCSOURCE_PLLBYPASS;
+    }
+    if (dev->cfg.allow_mul_4) {
+        dev->regs.clock.brg_config = BRGCONFIG_4XMODE;
+    } else if (dev->cfg.allow_mul_2) {
+        dev->regs.clock.brg_config = BRGCONFIG_2XMODE;
+    } else {
+        dev->regs.clock.brg_config = 0;
+    }
+    max3107_compute_clock_config(dev->cfg.osc_freq, conf->uc_speed, &dev->regs.clock);

Review comment:
       You are returning `best_br` here, should we be using it instead of our config or something ? Do we need to verify it with our specified config.




-- 
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



[GitHub] [mynewt-core] kasjer commented on pull request #2605: hw/drivers/uart: Add MAX3107 driver

Posted by GitBox <gi...@apache.org>.
kasjer commented on pull request #2605:
URL: https://github.com/apache/mynewt-core/pull/2605#issuecomment-858539315


   > So, the code looks really clean, we do need to handle errors in various places. I think adding STATS and LOG messages for figuring out what is going wrong would be helpful here.
   
   I will add separate PR for that if needed.


-- 
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



[GitHub] [mynewt-core] kasjer commented on a change in pull request #2605: hw/drivers/uart: Add MAX3107 driver

Posted by GitBox <gi...@apache.org>.
kasjer commented on a change in pull request #2605:
URL: https://github.com/apache/mynewt-core/pull/2605#discussion_r648330590



##########
File path: hw/drivers/uart/max3107/src/max3107.c
##########
@@ -0,0 +1,1090 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 <assert.h>
+#include <string.h>
+
+#include <os/mynewt.h>
+#include <bsp/bsp.h>
+#include <hal/hal_gpio.h>
+#include <hal/hal_uart.h>
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+#include <bus/drivers/spi_common.h>
+#endif
+#include <max3107/max3107.h>
+#include "max3107_priv.h"
+
+#include <uart/uart.h>
+
+static inline int
+max3107_lock(struct max3107_dev *dev)
+{
+    return os_error_to_sys(os_mutex_pend(&dev->lock,
+                                         os_time_ms_to_ticks32(MYNEWT_VAL(MAX3107_LOCK_TIMEOUT))));
+}
+
+static inline void
+max3107_unlock(struct max3107_dev *dev)
+{
+    int rc = os_mutex_release(&dev->lock);
+    assert(rc == 0);
+}
+
+void
+max3107_cs_activate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = (struct bus_spi_node *)&dev->dev;
+    hal_gpio_write(node->pin_cs, 0);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 0);
+#endif
+}
+
+void
+max3107_cs_deactivate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = &dev->dev;
+    hal_gpio_write(node->pin_cs, 1);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 1);
+#endif
+}
+
+int
+max3107_read_regs(struct max3107_dev *dev, uint8_t addr, void *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+#if !MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    uint8_t fast_buf[8];
+#endif
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;
+
+    if (size > 0) {
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+        rc = bus_node_simple_write_read_transact((struct os_dev *)&dev->dev,
+                                                 &addr, 1, buf, size);
+#else
+        max3107_cs_activate(dev);
+
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memset(fast_buf + 1, 0xFF, size);
+            /* Send command + address */
+            rc = hal_spi_txrx(dev->cfg.spi_num, fast_buf, fast_buf, size + 1);
+            if (rc == 0) {
+                memcpy(buf, fast_buf + 1, size);
+            }
+        } else {
+            /* Send command + address */
+            rc = hal_spi_txrx(dev->cfg.spi_num, &addr, NULL, 1);
+            if (rc == 0) {
+                /* For security mostly, do not output random data, fill it with FF */
+                memset(buf, 0xFF, size);
+                /* Tx buf does not matter, for simplicity pass read buffer */
+                rc = hal_spi_txrx(dev->cfg.spi_num, buf, buf, (int)size);
+            }
+        }
+
+        max3107_cs_deactivate(dev);
+#endif
+    }
+
+    if (locked) {
+        max3107_unlock(dev);
+    }
+
+    return rc;
+}
+
+int
+max3107_write_regs(struct max3107_dev *dev, uint8_t addr, const uint8_t *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+    uint8_t fast_buf[17];
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;
+
+    if (size) {
+        /* For writes set MSB */
+        addr |= 0x80;
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+        rc = bus_node_lock((struct os_dev *)&dev->dev,
+                           BUS_NODE_LOCK_DEFAULT_TIMEOUT);
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memcpy(fast_buf + 1, buf, size);
+            rc = bus_node_simple_write((struct os_dev *)&dev->dev, fast_buf, size + 1);
+        } else {
+            if (rc == 0) {
+                rc = bus_node_write((struct os_dev *)&dev->dev,
+                                    &addr, 1,
+                                    BUS_NODE_LOCK_DEFAULT_TIMEOUT, BUS_F_NOSTOP);
+                if (rc == 0) {
+                    rc = bus_node_simple_write((struct os_dev *)&dev->dev, buf, size);
+                }
+            }
+        }
+        (void)bus_node_unlock((struct os_dev *)&dev->dev);
+#else
+        max3107_cs_activate(dev);
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memcpy(fast_buf + 1, buf, size);
+            rc = hal_spi_txrx(dev->cfg.spi_num, fast_buf, NULL, size + 1);
+        } else {
+            rc = hal_spi_txrx(dev->cfg.spi_num, &addr, NULL, 1);
+            if (rc == 0) {
+                rc = hal_spi_txrx(dev->cfg.spi_num, (void *)buf, NULL, (int)size);
+            }
+        }
+        max3107_cs_deactivate(dev);
+#endif
+    }
+    if (locked) {
+        max3107_unlock(dev);
+    }
+
+    return rc;
+}
+
+static int
+max3107_write_reg(struct max3107_dev *dev, uint8_t addr, uint8_t val)
+{
+    return max3107_write_regs(dev, addr, &val, 1);
+}
+
+static int
+max3107_write_fifo(struct max3107_dev *dev, const uint8_t *buf, uint32_t size)
+{
+    return max3107_write_regs(dev, 0, buf, size);
+}
+
+static int
+max3107_read_fifo(struct max3107_dev *dev, uint8_t *buf, uint32_t size)
+{
+    return max3107_read_regs(dev, 0, buf, size);
+}
+
+static const uint8_t factors[5] = { 1, 6, 48, 96, 144 };
+/* "F PLL in" from datasheet: Table 4. PLLFactor[1:0] Selection Guide */
+static const uint32_t fpllin_min[5] = { 1, 500000,  850000,  425000, 390000 };
+static const uint32_t fpllin_max[5] = { 1, 800000, 1200000, 1000000, 666666 };
+
+static uint32_t
+max3107_compute_clock_config(uint32_t clockf, uint32_t br, struct max3107_clock_cfg *cfg)
+{
+    uint32_t div;
+    uint32_t pre_div;
+    uint32_t pre_div_min;
+    uint32_t pre_div_max;
+    uint32_t mul;
+    uint32_t actual_br;
+    uint32_t best_br = 1;
+    uint32_t fref;
+    int factor_ix;
+    int max_factor = (cfg->clk_source & CLCSOURCE_PLLBYPASS) ? 1 : 5;
+    uint32_t mode_mul = (cfg->brg_config & BRGCONFIG_4XMODE) ? 4 : (cfg->brg_config & BRGCONFIG_2XMODE) ? 2 : 1;
+
+    /* Clock will be needed. */
+    cfg->clk_source |= CLCSOURCE_CLOCKEN;
+
+    /* If bypass was not disable at the start try finding more accurate clock settings. */
+    for (factor_ix = 0; factor_ix < max_factor; ++factor_ix) {
+        /*
+         * Pre divider does not apply when PLL is not used. Set lower and upper
+         * limits to 1 to have same code for PLL/non PLL.
+         */
+        if (factor_ix == 0) {
+            pre_div_min = 1;
+            pre_div_max = 1;
+        } else {
+            /* Lower and upper frequency limits used to get pre divider range */
+            pre_div_max = clockf / fpllin_min[factor_ix];
+            pre_div_min = (clockf + fpllin_max[factor_ix] - 1) / fpllin_max[factor_ix];
+            /* Make sure divider is in correct range. */
+            pre_div_min = min(63, pre_div_min);
+            pre_div_min = max(1, pre_div_min);
+            pre_div_max = min(63, pre_div_max);
+            pre_div_max = max(1, pre_div_max);
+        }
+        /* Loop for x1, x2 and x4 modes. */
+        for (mul = 1; mul <= mode_mul; mul <<= 1) {
+            for (pre_div = pre_div_min; pre_div <= pre_div_max; ++pre_div) {
+                fref = (clockf / pre_div) * factors[factor_ix];
+                div = (fref * mul + (br / 2)) / br;
+                div = max(div, 16);
+
+                actual_br = mul * fref / div;
+                if (abs((int)(actual_br - br)) < abs((int)(best_br - br))) {
+                    best_br = actual_br;
+                    cfg->div_lsb = (uint8_t)(div >> 4);
+                    cfg->div_msb = (uint8_t)(div >> 12);
+                    cfg->brg_config = (div & 0xF) | ((mul == 4) ? 0x20 : ((mul == 2) ? 0x10 : 0));
+                    /* If best choice is factor_ix == 0, no need for PLL */
+                    if (factor_ix == 0) {
+                        cfg->clk_source |= CLCSOURCE_PLLBYPASS;
+                        cfg->clk_source &= ~CLCSOURCE_PLLEN;
+                    } else {
+                        cfg->pll_config = pre_div | ((factor_ix - 1) << 6);
+                        cfg->clk_source &= ~CLCSOURCE_PLLBYPASS;
+                        cfg->clk_source |= CLCSOURCE_PLLEN;
+                    }
+                }
+            }
+        }
+    }
+
+    return best_br;
+}
+
+int
+max3107_config_uart(struct max3107_dev *dev, const struct uart_conf_port *conf)
+{
+    int rc;
+
+    if (dev->cfg.crystal_en) {
+        dev->regs.clock.clk_source |= CLCSOURCE_CRYSTALEN;
+    } else {
+        dev->regs.clock.clk_source &= ~CLCSOURCE_CRYSTALEN;
+    }
+    if (dev->cfg.no_pll) {
+        dev->regs.clock.clk_source |= CLCSOURCE_PLLBYPASS;
+    } else {
+        dev->regs.clock.clk_source &= ~CLCSOURCE_PLLBYPASS;
+    }
+    if (dev->cfg.allow_mul_4) {
+        dev->regs.clock.brg_config = BRGCONFIG_4XMODE;
+    } else if (dev->cfg.allow_mul_2) {
+        dev->regs.clock.brg_config = BRGCONFIG_2XMODE;
+    } else {
+        dev->regs.clock.brg_config = 0;
+    }
+    max3107_compute_clock_config(dev->cfg.osc_freq, conf->uc_speed, &dev->regs.clock);
+
+    rc = max3107_write_regs(dev, MAX3107_REG_PLLCONFIG, &dev->regs.clock.pll_config, 5);
+    if (rc) {
+        goto end;
+    }
+
+    dev->regs.modes.mode1 = MODE1_IRQSEL | (conf->uc_flow_ctl ? 0 : MODE1_RTSHIZ);
+    dev->regs.modes.mode2 = 0;
+    dev->regs.modes.lcr = (dev->uart.ud_conf_port.uc_stopbits == 1 ? 0 : LCR_STOPBITS) |
+                          (dev->uart.ud_conf_port.uc_databits - 5) |
+                          ((dev->uart.ud_conf_port.uc_parity != UART_PARITY_NONE) ? LCR_PARITYEN : 0 ) |
+                          ((dev->uart.ud_conf_port.uc_parity == UART_PARITY_EVEN) ? LCR_EVENPARITY : 0);
+    dev->regs.modes.rxtimeout = 2;
+    dev->regs.modes.hdplxdelay = 0;
+    dev->regs.modes.irda = 0;
+
+    rc = max3107_write_regs(dev, MAX3107_REG_MODE1, &dev->regs.modes.mode1, 6);
+    if (rc) {
+        goto end;
+    }
+
+    /* RTS activated at FIFO level 120. */
+    dev->regs.fifo.flow_lvl = 0xFF;
+    dev->regs.fifo.fifo_trg_lvl = (dev->cfg.rx_trigger_level >> 3) << 4 | (dev->cfg.tx_trigger_level >> 3);
+    rc = max3107_write_regs(dev, MAX3107_REG_FLOWLVL, &dev->regs.fifo.flow_lvl, 2);
+    if (rc) {
+        goto end;
+    }
+
+    dev->regs.flow.flow_ctrl = conf->uc_flow_ctl ? (FLOWCTRL_AUTOCTS | FLOWCTRL_AUTORTS) : 0;
+    rc = max3107_write_reg(dev, MAX3107_REG_FLOWCTRL, dev->regs.flow.flow_ctrl);
+    if (rc) {
+        goto end;
+    }
+
+    dev->regs.ints.irq_en = IRQEN_LSRERRIEN | IRQEN_RXTRGIEN;
+    dev->regs.ints.lsr_int_en = LSRINTEN_FRAMEERRIEN | LSRINTEN_PARITYIEN |
+                                LSRINTEN_RBREAKIEN | LSRINTEN_ROVERRIEN | LSRINTEN_RTIMEOUTIEN;
+    rc = max3107_write_regs(dev, MAX3107_REG_IRQEN, &dev->regs.ints.irq_en, 3);
+end:

Review comment:
       if code was to be modified with some kind of tracing single place with return is easier to manage.
   My local version (with changes that are for debug purposes only) does just that.




-- 
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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2605: hw/drivers/uart: Add MAX3107 driver

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2605:
URL: https://github.com/apache/mynewt-core/pull/2605#discussion_r640865510



##########
File path: hw/drivers/uart/max3107/src/max3107.c
##########
@@ -0,0 +1,1059 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 <assert.h>
+#include <string.h>
+
+#include <os/mynewt.h>
+#include <bsp/bsp.h>
+#include <hal/hal_gpio.h>
+#include <hal/hal_uart.h>
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+#include <bus/drivers/spi_common.h>
+#endif
+#include <max3107/max3107.h>
+#include "max3107_priv.h"
+
+#include <uart/uart.h>
+
+static inline int
+max3107_lock(struct max3107_dev *dev)
+{
+    return os_error_to_sys(os_mutex_pend(&dev->lock,
+                                         os_time_ms_to_ticks32(MYNEWT_VAL(MAX3107_LOCK_TIMEOUT))));
+}
+
+static inline void
+max3107_unlock(struct max3107_dev *dev)
+{
+    int rc = os_mutex_release(&dev->lock);
+    assert(rc == 0);
+}
+
+void
+max3107_cs_activate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = (struct bus_spi_node *)&dev->dev;
+    hal_gpio_write(node->pin_cs, 0);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 0);
+#endif
+}
+
+void
+max3107_cs_deactivate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = &dev->dev;
+    hal_gpio_write(node->pin_cs, 1);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 1);
+#endif
+}
+
+int
+max3107_read_regs(struct max3107_dev *dev, uint8_t addr, void *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+#if !MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    uint8_t fast_buf[8];
+#endif
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;
+
+    if (size > 0) {
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+        rc = bus_node_simple_write_read_transact((struct os_dev *)&dev->dev,
+                                                 &addr, 1, buf, size);
+#else
+        max3107_cs_activate(dev);
+
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memset(fast_buf + 1, 0xFF, size);
+            /* Send command + address */
+            rc = hal_spi_txrx(dev->cfg.spi_num, fast_buf, fast_buf, size + 1);
+            if (rc == 0) {
+                memcpy(buf, fast_buf + 1, size);
+            }
+        } else {
+            /* Send command + address */
+            rc = hal_spi_txrx(dev->cfg.spi_num, &addr, NULL, 1);
+            if (rc == 0) {
+                /* For security mostly, do not output random data, fill it with FF */
+                memset(buf, 0xFF, size);
+                /* Tx buf does not matter, for simplicity pass read buffer */
+                rc = hal_spi_txrx(dev->cfg.spi_num, buf, buf, (int)size);
+            }
+        }
+
+        max3107_cs_deactivate(dev);
+#endif
+    }
+
+    if (locked) {
+        max3107_unlock(dev);
+    }
+
+    return rc;
+}
+
+int
+max3107_write_regs(struct max3107_dev *dev, uint8_t addr, const uint8_t *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+    uint8_t fast_buf[17];
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;
+
+    if (size) {
+        /* For writes set MSB */
+        addr |= 0x80;
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+        rc = bus_node_lock((struct os_dev *)&dev->dev,
+                           BUS_NODE_LOCK_DEFAULT_TIMEOUT);
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memcpy(fast_buf + 1, buf, size);
+            rc = bus_node_simple_write((struct os_dev *)&dev->dev, fast_buf, size + 1);
+        } else {
+            if (rc == 0) {
+                rc = bus_node_write((struct os_dev *)&dev->dev,
+                                    &addr, 1,
+                                    BUS_NODE_LOCK_DEFAULT_TIMEOUT, BUS_F_NOSTOP);
+                if (rc == 0) {
+                    rc = bus_node_simple_write((struct os_dev *)&dev->dev, buf, size);
+                }
+            }
+        }
+        (void)bus_node_unlock((struct os_dev *)&dev->dev);
+#else
+        max3107_cs_activate(dev);
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memcpy(fast_buf + 1, buf, size);
+            rc = hal_spi_txrx(dev->cfg.spi_num, fast_buf, NULL, size + 1);
+        } else {
+            rc = hal_spi_txrx(dev->cfg.spi_num, &addr, NULL, 1);
+            if (rc == 0) {
+                rc = hal_spi_txrx(dev->cfg.spi_num, (void *)buf, NULL, (int)size);
+            }
+        }
+        max3107_cs_deactivate(dev);
+#endif
+    }
+    if (locked) {
+        max3107_unlock(dev);
+    }
+
+    return rc;
+}
+
+static int
+max3107_write_reg(struct max3107_dev *dev, uint8_t addr, uint8_t val)
+{
+    return max3107_write_regs(dev, addr, &val, 1);
+}
+
+static int
+max3107_write_fifo(struct max3107_dev *dev, const uint8_t *buf, uint32_t size)
+{
+    return max3107_write_regs(dev, 0, buf, size);
+}
+
+int
+max3107_read_fifo(struct max3107_dev *dev, uint8_t *buf, uint32_t size)
+{
+    return max3107_read_regs(dev, 0, buf, size);
+}
+
+static const uint8_t factors[5] = { 1, 6, 48, 96, 144 };
+/* "F PLL in" from datasheet: Table 4. PLLFactor[1:0] Selection Guide */
+static const uint32_t fpllin_min[5] = { 1, 500000,  850000,  425000, 390000 };
+static const uint32_t fpllin_max[5] = { 1, 800000, 1200000, 1000000, 666666 };
+
+static uint32_t
+max3107_compute_clock_config(uint32_t clockf, uint32_t br, struct max3107_clock_cfg *cfg)
+{
+    uint32_t div;
+    uint32_t pre_div;
+    uint32_t pre_div_min;
+    uint32_t pre_div_max;
+    uint32_t mul;
+    uint32_t actual_br;
+    uint32_t best_br = 1;
+    uint32_t fref;
+    int factor_ix;
+    int max_factor = (cfg->clk_source & CLCSOURCE_PLLBYPASS) ? 1 : 5;
+    uint32_t mode_mul = (cfg->brg_config & BRGCONFIG_4XMODE) ? 4 : (cfg->brg_config & BRGCONFIG_2XMODE) ? 2 : 1;
+
+    /* Clock will be needed. */
+    cfg->clk_source |= CLCSOURCE_CLOCKEN;
+
+    /* If bypass was not disable at the start try finding more accurate clock settings. */
+    for (factor_ix = 0; factor_ix < max_factor; ++factor_ix) {
+        /*
+         * Pre divider does not apply when PLL is not used. Set lower and upper
+         * limits to 1 to have same code for PLL/non PLL.
+         */
+        if (factor_ix == 0) {
+            pre_div_min = 1;
+            pre_div_max = 1;
+        } else {
+            /* Lower and upper frequency limits used to get pre divider range */
+            pre_div_max = clockf / fpllin_min[factor_ix];
+            pre_div_min = (clockf + fpllin_max[factor_ix] - 1) / fpllin_max[factor_ix];
+            /* Make sure divider is in correct range. */
+            pre_div_min = min(63, pre_div_min);
+            pre_div_min = max(1, pre_div_min);
+            pre_div_max = min(63, pre_div_max);
+            pre_div_max = max(1, pre_div_max);
+        }
+        /* Loop for x1, x2 and x4 modes. */
+        for (mul = 1; mul <= mode_mul; mul <<= 1) {
+            for (pre_div = pre_div_min; pre_div <= pre_div_max; ++pre_div) {
+                fref = (clockf / pre_div) * factors[factor_ix];
+                div = (fref * mul + (br / 2)) / br;
+                div = max(div, 16);
+
+                actual_br = mul * fref / div;
+                if (abs((int)(actual_br - br)) < abs((int)(best_br - br))) {
+                    best_br = actual_br;
+                    cfg->div_lsb = (uint8_t)(div >> 4);
+                    cfg->div_msb = (uint8_t)(div >> 12);
+                    cfg->brg_config = (div & 0xF) | ((mul == 4) ? 0x20 : ((mul == 2) ? 0x10 : 0));
+                    /* If best choice is factor_ix == 0, no need for PLL */
+                    if (factor_ix == 0) {
+                        cfg->clk_source |= CLCSOURCE_PLLBYPASS;
+                        cfg->clk_source &= ~CLCSOURCE_PLLEN;
+                    } else {
+                        cfg->pll_config = pre_div | ((factor_ix - 1) << 6);
+                        cfg->clk_source &= ~CLCSOURCE_PLLBYPASS;
+                        cfg->clk_source |= CLCSOURCE_PLLEN;
+                    }
+                }
+            }
+        }
+    }
+
+    return best_br;
+}
+
+int
+max3107_config_uart(struct max3107_dev *dev, const struct uart_conf_port *conf)
+{
+    int rc;
+
+    if (dev->cfg.crystal_en) {
+        dev->regs.clock.clk_source |= CLCSOURCE_CRYSTALEN;
+    } else {
+        dev->regs.clock.clk_source &= ~CLCSOURCE_CRYSTALEN;
+    }
+    if (dev->cfg.no_pll) {
+        dev->regs.clock.clk_source |= CLCSOURCE_PLLBYPASS;
+    } else {
+        dev->regs.clock.clk_source &= ~CLCSOURCE_PLLBYPASS;
+    }
+    if (dev->cfg.allow_mul_4) {
+        dev->regs.clock.brg_config = BRGCONFIG_4XMODE;
+    } else if (dev->cfg.allow_mul_2) {
+        dev->regs.clock.brg_config = BRGCONFIG_2XMODE;
+    } else {
+        dev->regs.clock.brg_config = 0;
+    }
+    max3107_compute_clock_config(dev->cfg.osc_freq, conf->uc_speed, &dev->regs.clock);
+
+    rc = max3107_write_regs(dev, MAX3107_REG_PLLCONFIG, &dev->regs.clock.pll_config, 5);
+    if (rc) {
+        goto end;
+    }
+
+    dev->regs.modes.mode1 = MODE1_IRQSEL | (conf->uc_flow_ctl ? 0 : MODE1_RTSHIZ);
+    dev->regs.modes.mode2 = 0;
+    dev->regs.modes.lcr = (dev->uart.ud_conf_port.uc_stopbits == 1 ? 0 : LCR_STOPBITS) |
+                          (dev->uart.ud_conf_port.uc_databits - 5) |
+                          ((dev->uart.ud_conf_port.uc_parity != UART_PARITY_NONE) ? LCR_PARITYEN : 0 ) |
+                          ((dev->uart.ud_conf_port.uc_parity == UART_PARITY_EVEN) ? LCR_EVENPARITY : 0);
+    dev->regs.modes.rxtimeout = 2;
+    dev->regs.modes.hdplxdelay = 0;
+    dev->regs.modes.irda = 0;
+
+    rc = max3107_write_regs(dev, MAX3107_REG_MODE1, &dev->regs.modes.mode1, 6);
+    if (rc) {
+        goto end;
+    }
+
+    /* RTS activated at FIFO level 120. */
+    dev->regs.fifo.flow_lvl = 0xFF;
+    dev->regs.fifo.fifo_trg_lvl = 0x38;

Review comment:
       Does this `0x38` come from the datasheet ?




-- 
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



[GitHub] [mynewt-core] kasjer commented on a change in pull request #2605: hw/drivers/uart: Add MAX3107 driver

Posted by GitBox <gi...@apache.org>.
kasjer commented on a change in pull request #2605:
URL: https://github.com/apache/mynewt-core/pull/2605#discussion_r642385675



##########
File path: hw/drivers/uart/max3107/src/max3107.c
##########
@@ -0,0 +1,1059 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 <assert.h>
+#include <string.h>
+
+#include <os/mynewt.h>
+#include <bsp/bsp.h>
+#include <hal/hal_gpio.h>
+#include <hal/hal_uart.h>
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+#include <bus/drivers/spi_common.h>
+#endif
+#include <max3107/max3107.h>
+#include "max3107_priv.h"
+
+#include <uart/uart.h>
+
+static inline int
+max3107_lock(struct max3107_dev *dev)
+{
+    return os_error_to_sys(os_mutex_pend(&dev->lock,
+                                         os_time_ms_to_ticks32(MYNEWT_VAL(MAX3107_LOCK_TIMEOUT))));
+}
+
+static inline void
+max3107_unlock(struct max3107_dev *dev)
+{
+    int rc = os_mutex_release(&dev->lock);
+    assert(rc == 0);
+}
+
+void
+max3107_cs_activate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = (struct bus_spi_node *)&dev->dev;
+    hal_gpio_write(node->pin_cs, 0);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 0);
+#endif
+}
+
+void
+max3107_cs_deactivate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = &dev->dev;
+    hal_gpio_write(node->pin_cs, 1);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 1);
+#endif
+}
+
+int
+max3107_read_regs(struct max3107_dev *dev, uint8_t addr, void *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+#if !MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    uint8_t fast_buf[8];
+#endif
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;
+
+    if (size > 0) {
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+        rc = bus_node_simple_write_read_transact((struct os_dev *)&dev->dev,
+                                                 &addr, 1, buf, size);
+#else
+        max3107_cs_activate(dev);
+
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memset(fast_buf + 1, 0xFF, size);
+            /* Send command + address */
+            rc = hal_spi_txrx(dev->cfg.spi_num, fast_buf, fast_buf, size + 1);
+            if (rc == 0) {
+                memcpy(buf, fast_buf + 1, size);
+            }
+        } else {
+            /* Send command + address */
+            rc = hal_spi_txrx(dev->cfg.spi_num, &addr, NULL, 1);
+            if (rc == 0) {
+                /* For security mostly, do not output random data, fill it with FF */
+                memset(buf, 0xFF, size);
+                /* Tx buf does not matter, for simplicity pass read buffer */
+                rc = hal_spi_txrx(dev->cfg.spi_num, buf, buf, (int)size);
+            }
+        }
+
+        max3107_cs_deactivate(dev);
+#endif
+    }
+
+    if (locked) {
+        max3107_unlock(dev);
+    }
+
+    return rc;
+}
+
+int
+max3107_write_regs(struct max3107_dev *dev, uint8_t addr, const uint8_t *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+    uint8_t fast_buf[17];
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;
+
+    if (size) {
+        /* For writes set MSB */
+        addr |= 0x80;
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+        rc = bus_node_lock((struct os_dev *)&dev->dev,
+                           BUS_NODE_LOCK_DEFAULT_TIMEOUT);
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memcpy(fast_buf + 1, buf, size);
+            rc = bus_node_simple_write((struct os_dev *)&dev->dev, fast_buf, size + 1);
+        } else {
+            if (rc == 0) {
+                rc = bus_node_write((struct os_dev *)&dev->dev,
+                                    &addr, 1,
+                                    BUS_NODE_LOCK_DEFAULT_TIMEOUT, BUS_F_NOSTOP);
+                if (rc == 0) {
+                    rc = bus_node_simple_write((struct os_dev *)&dev->dev, buf, size);
+                }
+            }
+        }
+        (void)bus_node_unlock((struct os_dev *)&dev->dev);
+#else
+        max3107_cs_activate(dev);
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memcpy(fast_buf + 1, buf, size);
+            rc = hal_spi_txrx(dev->cfg.spi_num, fast_buf, NULL, size + 1);
+        } else {
+            rc = hal_spi_txrx(dev->cfg.spi_num, &addr, NULL, 1);
+            if (rc == 0) {
+                rc = hal_spi_txrx(dev->cfg.spi_num, (void *)buf, NULL, (int)size);
+            }
+        }
+        max3107_cs_deactivate(dev);
+#endif
+    }
+    if (locked) {
+        max3107_unlock(dev);
+    }
+
+    return rc;
+}
+
+static int
+max3107_write_reg(struct max3107_dev *dev, uint8_t addr, uint8_t val)
+{
+    return max3107_write_regs(dev, addr, &val, 1);
+}
+
+static int
+max3107_write_fifo(struct max3107_dev *dev, const uint8_t *buf, uint32_t size)
+{
+    return max3107_write_regs(dev, 0, buf, size);
+}
+
+int
+max3107_read_fifo(struct max3107_dev *dev, uint8_t *buf, uint32_t size)
+{
+    return max3107_read_regs(dev, 0, buf, size);
+}
+
+static const uint8_t factors[5] = { 1, 6, 48, 96, 144 };
+/* "F PLL in" from datasheet: Table 4. PLLFactor[1:0] Selection Guide */
+static const uint32_t fpllin_min[5] = { 1, 500000,  850000,  425000, 390000 };
+static const uint32_t fpllin_max[5] = { 1, 800000, 1200000, 1000000, 666666 };
+
+static uint32_t
+max3107_compute_clock_config(uint32_t clockf, uint32_t br, struct max3107_clock_cfg *cfg)
+{
+    uint32_t div;
+    uint32_t pre_div;
+    uint32_t pre_div_min;
+    uint32_t pre_div_max;
+    uint32_t mul;
+    uint32_t actual_br;
+    uint32_t best_br = 1;
+    uint32_t fref;
+    int factor_ix;
+    int max_factor = (cfg->clk_source & CLCSOURCE_PLLBYPASS) ? 1 : 5;
+    uint32_t mode_mul = (cfg->brg_config & BRGCONFIG_4XMODE) ? 4 : (cfg->brg_config & BRGCONFIG_2XMODE) ? 2 : 1;
+
+    /* Clock will be needed. */
+    cfg->clk_source |= CLCSOURCE_CLOCKEN;
+
+    /* If bypass was not disable at the start try finding more accurate clock settings. */
+    for (factor_ix = 0; factor_ix < max_factor; ++factor_ix) {
+        /*
+         * Pre divider does not apply when PLL is not used. Set lower and upper
+         * limits to 1 to have same code for PLL/non PLL.
+         */
+        if (factor_ix == 0) {
+            pre_div_min = 1;
+            pre_div_max = 1;
+        } else {
+            /* Lower and upper frequency limits used to get pre divider range */
+            pre_div_max = clockf / fpllin_min[factor_ix];
+            pre_div_min = (clockf + fpllin_max[factor_ix] - 1) / fpllin_max[factor_ix];
+            /* Make sure divider is in correct range. */
+            pre_div_min = min(63, pre_div_min);
+            pre_div_min = max(1, pre_div_min);
+            pre_div_max = min(63, pre_div_max);
+            pre_div_max = max(1, pre_div_max);
+        }
+        /* Loop for x1, x2 and x4 modes. */
+        for (mul = 1; mul <= mode_mul; mul <<= 1) {
+            for (pre_div = pre_div_min; pre_div <= pre_div_max; ++pre_div) {
+                fref = (clockf / pre_div) * factors[factor_ix];
+                div = (fref * mul + (br / 2)) / br;
+                div = max(div, 16);
+
+                actual_br = mul * fref / div;
+                if (abs((int)(actual_br - br)) < abs((int)(best_br - br))) {
+                    best_br = actual_br;
+                    cfg->div_lsb = (uint8_t)(div >> 4);
+                    cfg->div_msb = (uint8_t)(div >> 12);
+                    cfg->brg_config = (div & 0xF) | ((mul == 4) ? 0x20 : ((mul == 2) ? 0x10 : 0));
+                    /* If best choice is factor_ix == 0, no need for PLL */
+                    if (factor_ix == 0) {
+                        cfg->clk_source |= CLCSOURCE_PLLBYPASS;
+                        cfg->clk_source &= ~CLCSOURCE_PLLEN;
+                    } else {
+                        cfg->pll_config = pre_div | ((factor_ix - 1) << 6);
+                        cfg->clk_source &= ~CLCSOURCE_PLLBYPASS;
+                        cfg->clk_source |= CLCSOURCE_PLLEN;
+                    }
+                }
+            }
+        }
+    }
+
+    return best_br;
+}
+
+int
+max3107_config_uart(struct max3107_dev *dev, const struct uart_conf_port *conf)
+{
+    int rc;
+
+    if (dev->cfg.crystal_en) {
+        dev->regs.clock.clk_source |= CLCSOURCE_CRYSTALEN;
+    } else {
+        dev->regs.clock.clk_source &= ~CLCSOURCE_CRYSTALEN;
+    }
+    if (dev->cfg.no_pll) {
+        dev->regs.clock.clk_source |= CLCSOURCE_PLLBYPASS;
+    } else {
+        dev->regs.clock.clk_source &= ~CLCSOURCE_PLLBYPASS;
+    }
+    if (dev->cfg.allow_mul_4) {
+        dev->regs.clock.brg_config = BRGCONFIG_4XMODE;
+    } else if (dev->cfg.allow_mul_2) {
+        dev->regs.clock.brg_config = BRGCONFIG_2XMODE;
+    } else {
+        dev->regs.clock.brg_config = 0;
+    }
+    max3107_compute_clock_config(dev->cfg.osc_freq, conf->uc_speed, &dev->regs.clock);
+
+    rc = max3107_write_regs(dev, MAX3107_REG_PLLCONFIG, &dev->regs.clock.pll_config, 5);
+    if (rc) {
+        goto end;
+    }
+
+    dev->regs.modes.mode1 = MODE1_IRQSEL | (conf->uc_flow_ctl ? 0 : MODE1_RTSHIZ);
+    dev->regs.modes.mode2 = 0;
+    dev->regs.modes.lcr = (dev->uart.ud_conf_port.uc_stopbits == 1 ? 0 : LCR_STOPBITS) |
+                          (dev->uart.ud_conf_port.uc_databits - 5) |
+                          ((dev->uart.ud_conf_port.uc_parity != UART_PARITY_NONE) ? LCR_PARITYEN : 0 ) |
+                          ((dev->uart.ud_conf_port.uc_parity == UART_PARITY_EVEN) ? LCR_EVENPARITY : 0);
+    dev->regs.modes.rxtimeout = 2;
+    dev->regs.modes.hdplxdelay = 0;
+    dev->regs.modes.irda = 0;
+
+    rc = max3107_write_regs(dev, MAX3107_REG_MODE1, &dev->regs.modes.mode1, 6);
+    if (rc) {
+        goto end;
+    }
+
+    /* RTS activated at FIFO level 120. */
+    dev->regs.fifo.flow_lvl = 0xFF;
+    dev->regs.fifo.fifo_trg_lvl = 0x38;

Review comment:
       Fifo trigger level now can be set with syscfg, so user can experiment with other values.




-- 
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



[GitHub] [mynewt-core] kasjer commented on a change in pull request #2605: hw/drivers/uart: Add MAX3107 driver

Posted by GitBox <gi...@apache.org>.
kasjer commented on a change in pull request #2605:
URL: https://github.com/apache/mynewt-core/pull/2605#discussion_r640873730



##########
File path: hw/drivers/uart/max3107/include/max3107/max3107.h
##########
@@ -0,0 +1,172 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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.
+ */
+
+#ifndef __DRIVERS_MAX3107_H_
+#define __DRIVERS_MAX3107_H_
+
+#include <stdint.h>
+#include <uart/uart.h>
+#include <hal/hal_spi.h>
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+#include <bus/drivers/spi_common.h>
+#endif
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+struct max3107_dev;
+
+struct max3107_cfg {
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node_cfg node_cfg;
+#else
+    struct hal_spi_settings spi_settings;
+    int spi_num;
+    int ss_pin;
+#endif
+    /**< External clock/oscillator frequency in Hz */
+    uint32_t osc_freq;
+    /**< IRQ Pin */
+    int8_t irq_pin;
+    int8_t ldoen_pin;
+    /**< 1 - External crystal oscillator, 0 - external clock */
+    int8_t crystal_en: 1;
+    /**< 1 - To disable PLL */
+    uint8_t no_pll: 1;
+    /**< 1 - To enable x 4 mode */
+    uint8_t allow_mul_4: 1;
+    /**< 1 - To enable x 4 mode */
+    uint8_t allow_mul_2: 1;
+};
+
+struct max3107_client {
+    /**< Function that will be called from process context when data can be read. */
+    void (*readable)(struct max3107_dev *dev);

Review comment:
       Driver does not care if client reacts to notifications. So return value is not needed.




-- 
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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2605: hw/drivers/uart: Add MAX3107 driver

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2605:
URL: https://github.com/apache/mynewt-core/pull/2605#discussion_r640862391



##########
File path: hw/drivers/uart/max3107/include/max3107/max3107.h
##########
@@ -0,0 +1,172 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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.
+ */
+
+#ifndef __DRIVERS_MAX3107_H_
+#define __DRIVERS_MAX3107_H_
+
+#include <stdint.h>
+#include <uart/uart.h>
+#include <hal/hal_spi.h>
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+#include <bus/drivers/spi_common.h>
+#endif
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+struct max3107_dev;
+
+struct max3107_cfg {
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node_cfg node_cfg;
+#else
+    struct hal_spi_settings spi_settings;
+    int spi_num;
+    int ss_pin;
+#endif
+    /**< External clock/oscillator frequency in Hz */
+    uint32_t osc_freq;
+    /**< IRQ Pin */
+    int8_t irq_pin;
+    int8_t ldoen_pin;
+    /**< 1 - External crystal oscillator, 0 - external clock */
+    int8_t crystal_en: 1;
+    /**< 1 - To disable PLL */
+    uint8_t no_pll: 1;
+    /**< 1 - To enable x 4 mode */
+    uint8_t allow_mul_4: 1;
+    /**< 1 - To enable x 4 mode */
+    uint8_t allow_mul_2: 1;
+};
+
+struct max3107_client {
+    /**< Function that will be called from process context when data can be read. */
+    void (*readable)(struct max3107_dev *dev);

Review comment:
       I think we do need to return `int` here and for all the function pointers below. 




-- 
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



[GitHub] [mynewt-core] kasjer commented on a change in pull request #2605: hw/drivers/uart: Add MAX3107 driver

Posted by GitBox <gi...@apache.org>.
kasjer commented on a change in pull request #2605:
URL: https://github.com/apache/mynewt-core/pull/2605#discussion_r642387668



##########
File path: hw/drivers/uart/max3107/syscfg.yml
##########
@@ -0,0 +1,134 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you 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.
+#
+
+syscfg.defs:
+    MAX3107_0:
+        description: >
+            Create SPI RAM device instance.
+        value: 0
+
+    MAX3107_0_NAME:
+        description: >
+            MAX3107 device name.
+        value: '"max3107_0"'
+
+    MAX3107_0_UART_NAME:
+        description: >
+            MAX3107 UART device name.
+        value: '"uart4"'
+
+    MAX3107_0_SPI_BAUDRATE:
+        description: >
+            MAX3107 SPI clock speed in kHz.
+        value: 4000
+
+    MAX3107_0_UART_BAUDRATE:
+        description: >
+            MAX3107 UART clock speed in bits/second.
+        value: 115200
+
+    MAX3107_0_UART_DATA_BITS:
+        description: >
+            MAX3107 number of data bits.
+        range: 5,6,7,8
+        value: 8
+
+    MAX3107_0_UART_PARITY:
+        description: >
+            MAX3107 parity 0 - no parity, 1 - odd, 2 - even.
+        range: 0,1,2
+        value: 0
+
+    MAX3107_0_UART_STOP_BITS:
+        description: >
+            MAX3107 UART stop bits 1 or 2.
+        range: 1,2
+        value: 1
+
+    MAX3107_0_UART_FLOW_CONTROL:
+        description: >
+            MAX3107 UART flow control 0 - none, 1 - RTS CTS.
+        range: 0,1
+        value: 0
+
+    MAX3107_0_CS_PIN:
+        description: >
+            Chip select pin for MAX3107.
+        value: -1
+
+    MAX3107_0_SPI_BUS:
+        description: >
+            SPI interface bus name for MAX3107.
+        value : '"spi0"'
+
+    MAX3107_0_SPI_NUM:
+        description: >
+            SPI interface used for SPI RAM.
+        value: 0
+
+    MAX3107_0_OSC_FREQ:
+        description: >
+            External clock/oscillator frequency.
+        value:

Review comment:
       Default value is not provided.
   Range check is added to allow for valid frequencies 0.5-35 MHz.




-- 
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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2605: hw/drivers/uart: Add MAX3107 driver

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2605:
URL: https://github.com/apache/mynewt-core/pull/2605#discussion_r640864963



##########
File path: hw/drivers/uart/max3107/src/max3107.c
##########
@@ -0,0 +1,1059 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 <assert.h>
+#include <string.h>
+
+#include <os/mynewt.h>
+#include <bsp/bsp.h>
+#include <hal/hal_gpio.h>
+#include <hal/hal_uart.h>
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+#include <bus/drivers/spi_common.h>
+#endif
+#include <max3107/max3107.h>
+#include "max3107_priv.h"
+
+#include <uart/uart.h>
+
+static inline int
+max3107_lock(struct max3107_dev *dev)
+{
+    return os_error_to_sys(os_mutex_pend(&dev->lock,
+                                         os_time_ms_to_ticks32(MYNEWT_VAL(MAX3107_LOCK_TIMEOUT))));
+}
+
+static inline void
+max3107_unlock(struct max3107_dev *dev)
+{
+    int rc = os_mutex_release(&dev->lock);
+    assert(rc == 0);
+}
+
+void
+max3107_cs_activate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = (struct bus_spi_node *)&dev->dev;
+    hal_gpio_write(node->pin_cs, 0);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 0);
+#endif
+}
+
+void
+max3107_cs_deactivate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = &dev->dev;
+    hal_gpio_write(node->pin_cs, 1);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 1);
+#endif
+}
+
+int
+max3107_read_regs(struct max3107_dev *dev, uint8_t addr, void *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+#if !MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    uint8_t fast_buf[8];
+#endif
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;
+
+    if (size > 0) {
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+        rc = bus_node_simple_write_read_transact((struct os_dev *)&dev->dev,
+                                                 &addr, 1, buf, size);
+#else
+        max3107_cs_activate(dev);
+
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memset(fast_buf + 1, 0xFF, size);
+            /* Send command + address */
+            rc = hal_spi_txrx(dev->cfg.spi_num, fast_buf, fast_buf, size + 1);
+            if (rc == 0) {
+                memcpy(buf, fast_buf + 1, size);
+            }
+        } else {
+            /* Send command + address */
+            rc = hal_spi_txrx(dev->cfg.spi_num, &addr, NULL, 1);
+            if (rc == 0) {
+                /* For security mostly, do not output random data, fill it with FF */
+                memset(buf, 0xFF, size);
+                /* Tx buf does not matter, for simplicity pass read buffer */
+                rc = hal_spi_txrx(dev->cfg.spi_num, buf, buf, (int)size);
+            }
+        }
+
+        max3107_cs_deactivate(dev);
+#endif
+    }
+
+    if (locked) {
+        max3107_unlock(dev);
+    }
+
+    return rc;
+}
+
+int
+max3107_write_regs(struct max3107_dev *dev, uint8_t addr, const uint8_t *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+    uint8_t fast_buf[17];
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;
+
+    if (size) {
+        /* For writes set MSB */
+        addr |= 0x80;
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+        rc = bus_node_lock((struct os_dev *)&dev->dev,
+                           BUS_NODE_LOCK_DEFAULT_TIMEOUT);
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memcpy(fast_buf + 1, buf, size);
+            rc = bus_node_simple_write((struct os_dev *)&dev->dev, fast_buf, size + 1);
+        } else {
+            if (rc == 0) {
+                rc = bus_node_write((struct os_dev *)&dev->dev,
+                                    &addr, 1,
+                                    BUS_NODE_LOCK_DEFAULT_TIMEOUT, BUS_F_NOSTOP);
+                if (rc == 0) {
+                    rc = bus_node_simple_write((struct os_dev *)&dev->dev, buf, size);
+                }
+            }
+        }
+        (void)bus_node_unlock((struct os_dev *)&dev->dev);
+#else
+        max3107_cs_activate(dev);
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memcpy(fast_buf + 1, buf, size);
+            rc = hal_spi_txrx(dev->cfg.spi_num, fast_buf, NULL, size + 1);
+        } else {
+            rc = hal_spi_txrx(dev->cfg.spi_num, &addr, NULL, 1);
+            if (rc == 0) {
+                rc = hal_spi_txrx(dev->cfg.spi_num, (void *)buf, NULL, (int)size);
+            }
+        }
+        max3107_cs_deactivate(dev);
+#endif
+    }
+    if (locked) {
+        max3107_unlock(dev);
+    }
+
+    return rc;
+}
+
+static int
+max3107_write_reg(struct max3107_dev *dev, uint8_t addr, uint8_t val)
+{
+    return max3107_write_regs(dev, addr, &val, 1);
+}
+
+static int
+max3107_write_fifo(struct max3107_dev *dev, const uint8_t *buf, uint32_t size)
+{
+    return max3107_write_regs(dev, 0, buf, size);
+}
+
+int
+max3107_read_fifo(struct max3107_dev *dev, uint8_t *buf, uint32_t size)
+{
+    return max3107_read_regs(dev, 0, buf, size);
+}
+
+static const uint8_t factors[5] = { 1, 6, 48, 96, 144 };
+/* "F PLL in" from datasheet: Table 4. PLLFactor[1:0] Selection Guide */
+static const uint32_t fpllin_min[5] = { 1, 500000,  850000,  425000, 390000 };
+static const uint32_t fpllin_max[5] = { 1, 800000, 1200000, 1000000, 666666 };
+
+static uint32_t
+max3107_compute_clock_config(uint32_t clockf, uint32_t br, struct max3107_clock_cfg *cfg)
+{
+    uint32_t div;
+    uint32_t pre_div;
+    uint32_t pre_div_min;
+    uint32_t pre_div_max;
+    uint32_t mul;
+    uint32_t actual_br;
+    uint32_t best_br = 1;
+    uint32_t fref;
+    int factor_ix;
+    int max_factor = (cfg->clk_source & CLCSOURCE_PLLBYPASS) ? 1 : 5;
+    uint32_t mode_mul = (cfg->brg_config & BRGCONFIG_4XMODE) ? 4 : (cfg->brg_config & BRGCONFIG_2XMODE) ? 2 : 1;
+
+    /* Clock will be needed. */
+    cfg->clk_source |= CLCSOURCE_CLOCKEN;
+
+    /* If bypass was not disable at the start try finding more accurate clock settings. */
+    for (factor_ix = 0; factor_ix < max_factor; ++factor_ix) {
+        /*
+         * Pre divider does not apply when PLL is not used. Set lower and upper
+         * limits to 1 to have same code for PLL/non PLL.
+         */
+        if (factor_ix == 0) {
+            pre_div_min = 1;
+            pre_div_max = 1;
+        } else {
+            /* Lower and upper frequency limits used to get pre divider range */
+            pre_div_max = clockf / fpllin_min[factor_ix];
+            pre_div_min = (clockf + fpllin_max[factor_ix] - 1) / fpllin_max[factor_ix];
+            /* Make sure divider is in correct range. */
+            pre_div_min = min(63, pre_div_min);
+            pre_div_min = max(1, pre_div_min);
+            pre_div_max = min(63, pre_div_max);
+            pre_div_max = max(1, pre_div_max);
+        }
+        /* Loop for x1, x2 and x4 modes. */
+        for (mul = 1; mul <= mode_mul; mul <<= 1) {
+            for (pre_div = pre_div_min; pre_div <= pre_div_max; ++pre_div) {
+                fref = (clockf / pre_div) * factors[factor_ix];
+                div = (fref * mul + (br / 2)) / br;
+                div = max(div, 16);
+
+                actual_br = mul * fref / div;
+                if (abs((int)(actual_br - br)) < abs((int)(best_br - br))) {
+                    best_br = actual_br;
+                    cfg->div_lsb = (uint8_t)(div >> 4);
+                    cfg->div_msb = (uint8_t)(div >> 12);
+                    cfg->brg_config = (div & 0xF) | ((mul == 4) ? 0x20 : ((mul == 2) ? 0x10 : 0));
+                    /* If best choice is factor_ix == 0, no need for PLL */
+                    if (factor_ix == 0) {
+                        cfg->clk_source |= CLCSOURCE_PLLBYPASS;
+                        cfg->clk_source &= ~CLCSOURCE_PLLEN;
+                    } else {
+                        cfg->pll_config = pre_div | ((factor_ix - 1) << 6);
+                        cfg->clk_source &= ~CLCSOURCE_PLLBYPASS;
+                        cfg->clk_source |= CLCSOURCE_PLLEN;
+                    }
+                }
+            }
+        }
+    }
+
+    return best_br;
+}
+
+int
+max3107_config_uart(struct max3107_dev *dev, const struct uart_conf_port *conf)
+{
+    int rc;
+
+    if (dev->cfg.crystal_en) {
+        dev->regs.clock.clk_source |= CLCSOURCE_CRYSTALEN;
+    } else {
+        dev->regs.clock.clk_source &= ~CLCSOURCE_CRYSTALEN;
+    }
+    if (dev->cfg.no_pll) {
+        dev->regs.clock.clk_source |= CLCSOURCE_PLLBYPASS;
+    } else {
+        dev->regs.clock.clk_source &= ~CLCSOURCE_PLLBYPASS;
+    }
+    if (dev->cfg.allow_mul_4) {
+        dev->regs.clock.brg_config = BRGCONFIG_4XMODE;
+    } else if (dev->cfg.allow_mul_2) {
+        dev->regs.clock.brg_config = BRGCONFIG_2XMODE;
+    } else {
+        dev->regs.clock.brg_config = 0;
+    }
+    max3107_compute_clock_config(dev->cfg.osc_freq, conf->uc_speed, &dev->regs.clock);

Review comment:
       You are returning `best_br` here, should we be using it instead for our config or something ? Do we need to verify it with our specified config.




-- 
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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2605: hw/drivers/uart: Add MAX3107 driver

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2605:
URL: https://github.com/apache/mynewt-core/pull/2605#discussion_r640871273



##########
File path: hw/drivers/uart/max3107/syscfg.yml
##########
@@ -0,0 +1,134 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you 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.
+#
+
+syscfg.defs:
+    MAX3107_0:
+        description: >
+            Create SPI RAM device instance.
+        value: 0
+
+    MAX3107_0_NAME:
+        description: >
+            MAX3107 device name.
+        value: '"max3107_0"'
+
+    MAX3107_0_UART_NAME:
+        description: >
+            MAX3107 UART device name.
+        value: '"uart4"'
+
+    MAX3107_0_SPI_BAUDRATE:
+        description: >
+            MAX3107 SPI clock speed in kHz.
+        value: 4000
+
+    MAX3107_0_UART_BAUDRATE:
+        description: >
+            MAX3107 UART clock speed in bits/second.
+        value: 115200
+
+    MAX3107_0_UART_DATA_BITS:
+        description: >
+            MAX3107 number of data bits.
+        range: 5,6,7,8
+        value: 8
+
+    MAX3107_0_UART_PARITY:
+        description: >
+            MAX3107 parity 0 - no parity, 1 - odd, 2 - even.
+        range: 0,1,2
+        value: 0
+
+    MAX3107_0_UART_STOP_BITS:
+        description: >
+            MAX3107 UART stop bits 1 or 2.
+        range: 1,2
+        value: 1
+
+    MAX3107_0_UART_FLOW_CONTROL:
+        description: >
+            MAX3107 UART flow control 0 - none, 1 - RTS CTS.
+        range: 0,1
+        value: 0
+
+    MAX3107_0_CS_PIN:
+        description: >
+            Chip select pin for MAX3107.
+        value: -1
+
+    MAX3107_0_SPI_BUS:
+        description: >
+            SPI interface bus name for MAX3107.
+        value : '"spi0"'
+
+    MAX3107_0_SPI_NUM:
+        description: >
+            SPI interface used for SPI RAM.
+        value: 0
+
+    MAX3107_0_OSC_FREQ:
+        description: >
+            External clock/oscillator frequency.
+        value:

Review comment:
       This value needs to be set to a default cause `clock_config` uses it, or you might need to handle the case where this is not set.




-- 
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



[GitHub] [mynewt-core] kasjer commented on a change in pull request #2605: hw/drivers/uart: Add MAX3107 driver

Posted by GitBox <gi...@apache.org>.
kasjer commented on a change in pull request #2605:
URL: https://github.com/apache/mynewt-core/pull/2605#discussion_r642385054



##########
File path: hw/drivers/uart/max3107/include/max3107/max3107.h
##########
@@ -0,0 +1,172 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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.
+ */
+
+#ifndef __DRIVERS_MAX3107_H_
+#define __DRIVERS_MAX3107_H_
+
+#include <stdint.h>
+#include <uart/uart.h>
+#include <hal/hal_spi.h>
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+#include <bus/drivers/spi_common.h>
+#endif
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+struct max3107_dev;
+
+struct max3107_cfg {
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node_cfg node_cfg;
+#else
+    struct hal_spi_settings spi_settings;
+    int spi_num;
+    int ss_pin;
+#endif
+    /**< External clock/oscillator frequency in Hz */
+    uint32_t osc_freq;
+    /**< IRQ Pin */
+    int8_t irq_pin;
+    int8_t ldoen_pin;
+    /**< 1 - External crystal oscillator, 0 - external clock */
+    int8_t crystal_en: 1;
+    /**< 1 - To disable PLL */
+    uint8_t no_pll: 1;
+    /**< 1 - To enable x 4 mode */
+    uint8_t allow_mul_4: 1;
+    /**< 1 - To enable x 4 mode */
+    uint8_t allow_mul_2: 1;
+};
+
+struct max3107_client {
+    /**< Function that will be called from process context when data can be read. */
+    void (*readable)(struct max3107_dev *dev);

Review comment:
       Driver does not care if client reacts to notifications. So return value is not needed.




-- 
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



[GitHub] [mynewt-core] kasjer commented on a change in pull request #2605: hw/drivers/uart: Add MAX3107 driver

Posted by GitBox <gi...@apache.org>.
kasjer commented on a change in pull request #2605:
URL: https://github.com/apache/mynewt-core/pull/2605#discussion_r642386685



##########
File path: hw/drivers/uart/max3107/src/max3107.c
##########
@@ -0,0 +1,1059 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 <assert.h>
+#include <string.h>
+
+#include <os/mynewt.h>
+#include <bsp/bsp.h>
+#include <hal/hal_gpio.h>
+#include <hal/hal_uart.h>
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+#include <bus/drivers/spi_common.h>
+#endif
+#include <max3107/max3107.h>
+#include "max3107_priv.h"
+
+#include <uart/uart.h>
+
+static inline int
+max3107_lock(struct max3107_dev *dev)
+{
+    return os_error_to_sys(os_mutex_pend(&dev->lock,
+                                         os_time_ms_to_ticks32(MYNEWT_VAL(MAX3107_LOCK_TIMEOUT))));
+}
+
+static inline void
+max3107_unlock(struct max3107_dev *dev)
+{
+    int rc = os_mutex_release(&dev->lock);
+    assert(rc == 0);
+}
+
+void
+max3107_cs_activate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = (struct bus_spi_node *)&dev->dev;
+    hal_gpio_write(node->pin_cs, 0);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 0);
+#endif
+}
+
+void
+max3107_cs_deactivate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = &dev->dev;
+    hal_gpio_write(node->pin_cs, 1);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 1);
+#endif
+}
+
+int
+max3107_read_regs(struct max3107_dev *dev, uint8_t addr, void *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+#if !MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    uint8_t fast_buf[8];
+#endif
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;
+
+    if (size > 0) {
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+        rc = bus_node_simple_write_read_transact((struct os_dev *)&dev->dev,
+                                                 &addr, 1, buf, size);
+#else
+        max3107_cs_activate(dev);
+
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memset(fast_buf + 1, 0xFF, size);
+            /* Send command + address */
+            rc = hal_spi_txrx(dev->cfg.spi_num, fast_buf, fast_buf, size + 1);
+            if (rc == 0) {
+                memcpy(buf, fast_buf + 1, size);
+            }
+        } else {
+            /* Send command + address */
+            rc = hal_spi_txrx(dev->cfg.spi_num, &addr, NULL, 1);
+            if (rc == 0) {
+                /* For security mostly, do not output random data, fill it with FF */
+                memset(buf, 0xFF, size);
+                /* Tx buf does not matter, for simplicity pass read buffer */
+                rc = hal_spi_txrx(dev->cfg.spi_num, buf, buf, (int)size);
+            }
+        }
+
+        max3107_cs_deactivate(dev);
+#endif
+    }
+
+    if (locked) {
+        max3107_unlock(dev);
+    }
+
+    return rc;
+}
+
+int
+max3107_write_regs(struct max3107_dev *dev, uint8_t addr, const uint8_t *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+    uint8_t fast_buf[17];
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;
+
+    if (size) {
+        /* For writes set MSB */
+        addr |= 0x80;
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+        rc = bus_node_lock((struct os_dev *)&dev->dev,
+                           BUS_NODE_LOCK_DEFAULT_TIMEOUT);
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memcpy(fast_buf + 1, buf, size);
+            rc = bus_node_simple_write((struct os_dev *)&dev->dev, fast_buf, size + 1);
+        } else {
+            if (rc == 0) {
+                rc = bus_node_write((struct os_dev *)&dev->dev,
+                                    &addr, 1,
+                                    BUS_NODE_LOCK_DEFAULT_TIMEOUT, BUS_F_NOSTOP);
+                if (rc == 0) {
+                    rc = bus_node_simple_write((struct os_dev *)&dev->dev, buf, size);
+                }
+            }
+        }
+        (void)bus_node_unlock((struct os_dev *)&dev->dev);
+#else
+        max3107_cs_activate(dev);
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memcpy(fast_buf + 1, buf, size);
+            rc = hal_spi_txrx(dev->cfg.spi_num, fast_buf, NULL, size + 1);
+        } else {
+            rc = hal_spi_txrx(dev->cfg.spi_num, &addr, NULL, 1);
+            if (rc == 0) {
+                rc = hal_spi_txrx(dev->cfg.spi_num, (void *)buf, NULL, (int)size);
+            }
+        }
+        max3107_cs_deactivate(dev);
+#endif
+    }
+    if (locked) {
+        max3107_unlock(dev);
+    }
+
+    return rc;
+}
+
+static int
+max3107_write_reg(struct max3107_dev *dev, uint8_t addr, uint8_t val)
+{
+    return max3107_write_regs(dev, addr, &val, 1);
+}
+
+static int
+max3107_write_fifo(struct max3107_dev *dev, const uint8_t *buf, uint32_t size)
+{
+    return max3107_write_regs(dev, 0, buf, size);
+}
+
+int
+max3107_read_fifo(struct max3107_dev *dev, uint8_t *buf, uint32_t size)
+{
+    return max3107_read_regs(dev, 0, buf, size);
+}
+
+static const uint8_t factors[5] = { 1, 6, 48, 96, 144 };
+/* "F PLL in" from datasheet: Table 4. PLLFactor[1:0] Selection Guide */
+static const uint32_t fpllin_min[5] = { 1, 500000,  850000,  425000, 390000 };
+static const uint32_t fpllin_max[5] = { 1, 800000, 1200000, 1000000, 666666 };
+
+static uint32_t
+max3107_compute_clock_config(uint32_t clockf, uint32_t br, struct max3107_clock_cfg *cfg)
+{
+    uint32_t div;
+    uint32_t pre_div;
+    uint32_t pre_div_min;
+    uint32_t pre_div_max;
+    uint32_t mul;
+    uint32_t actual_br;
+    uint32_t best_br = 1;
+    uint32_t fref;
+    int factor_ix;
+    int max_factor = (cfg->clk_source & CLCSOURCE_PLLBYPASS) ? 1 : 5;
+    uint32_t mode_mul = (cfg->brg_config & BRGCONFIG_4XMODE) ? 4 : (cfg->brg_config & BRGCONFIG_2XMODE) ? 2 : 1;
+
+    /* Clock will be needed. */
+    cfg->clk_source |= CLCSOURCE_CLOCKEN;
+
+    /* If bypass was not disable at the start try finding more accurate clock settings. */
+    for (factor_ix = 0; factor_ix < max_factor; ++factor_ix) {
+        /*
+         * Pre divider does not apply when PLL is not used. Set lower and upper
+         * limits to 1 to have same code for PLL/non PLL.
+         */
+        if (factor_ix == 0) {
+            pre_div_min = 1;
+            pre_div_max = 1;
+        } else {
+            /* Lower and upper frequency limits used to get pre divider range */
+            pre_div_max = clockf / fpllin_min[factor_ix];
+            pre_div_min = (clockf + fpllin_max[factor_ix] - 1) / fpllin_max[factor_ix];
+            /* Make sure divider is in correct range. */
+            pre_div_min = min(63, pre_div_min);
+            pre_div_min = max(1, pre_div_min);
+            pre_div_max = min(63, pre_div_max);
+            pre_div_max = max(1, pre_div_max);
+        }
+        /* Loop for x1, x2 and x4 modes. */
+        for (mul = 1; mul <= mode_mul; mul <<= 1) {
+            for (pre_div = pre_div_min; pre_div <= pre_div_max; ++pre_div) {
+                fref = (clockf / pre_div) * factors[factor_ix];
+                div = (fref * mul + (br / 2)) / br;
+                div = max(div, 16);
+
+                actual_br = mul * fref / div;
+                if (abs((int)(actual_br - br)) < abs((int)(best_br - br))) {
+                    best_br = actual_br;
+                    cfg->div_lsb = (uint8_t)(div >> 4);
+                    cfg->div_msb = (uint8_t)(div >> 12);
+                    cfg->brg_config = (div & 0xF) | ((mul == 4) ? 0x20 : ((mul == 2) ? 0x10 : 0));
+                    /* If best choice is factor_ix == 0, no need for PLL */
+                    if (factor_ix == 0) {
+                        cfg->clk_source |= CLCSOURCE_PLLBYPASS;
+                        cfg->clk_source &= ~CLCSOURCE_PLLEN;
+                    } else {
+                        cfg->pll_config = pre_div | ((factor_ix - 1) << 6);
+                        cfg->clk_source &= ~CLCSOURCE_PLLBYPASS;
+                        cfg->clk_source |= CLCSOURCE_PLLEN;
+                    }
+                }
+            }
+        }
+    }
+
+    return best_br;
+}
+
+int
+max3107_config_uart(struct max3107_dev *dev, const struct uart_conf_port *conf)
+{
+    int rc;
+
+    if (dev->cfg.crystal_en) {
+        dev->regs.clock.clk_source |= CLCSOURCE_CRYSTALEN;
+    } else {
+        dev->regs.clock.clk_source &= ~CLCSOURCE_CRYSTALEN;
+    }
+    if (dev->cfg.no_pll) {
+        dev->regs.clock.clk_source |= CLCSOURCE_PLLBYPASS;
+    } else {
+        dev->regs.clock.clk_source &= ~CLCSOURCE_PLLBYPASS;
+    }
+    if (dev->cfg.allow_mul_4) {
+        dev->regs.clock.brg_config = BRGCONFIG_4XMODE;
+    } else if (dev->cfg.allow_mul_2) {
+        dev->regs.clock.brg_config = BRGCONFIG_2XMODE;
+    } else {
+        dev->regs.clock.brg_config = 0;
+    }
+    max3107_compute_clock_config(dev->cfg.osc_freq, conf->uc_speed, &dev->regs.clock);
+
+    rc = max3107_write_regs(dev, MAX3107_REG_PLLCONFIG, &dev->regs.clock.pll_config, 5);
+    if (rc) {
+        goto end;
+    }
+
+    dev->regs.modes.mode1 = MODE1_IRQSEL | (conf->uc_flow_ctl ? 0 : MODE1_RTSHIZ);
+    dev->regs.modes.mode2 = 0;
+    dev->regs.modes.lcr = (dev->uart.ud_conf_port.uc_stopbits == 1 ? 0 : LCR_STOPBITS) |
+                          (dev->uart.ud_conf_port.uc_databits - 5) |
+                          ((dev->uart.ud_conf_port.uc_parity != UART_PARITY_NONE) ? LCR_PARITYEN : 0 ) |
+                          ((dev->uart.ud_conf_port.uc_parity == UART_PARITY_EVEN) ? LCR_EVENPARITY : 0);
+    dev->regs.modes.rxtimeout = 2;
+    dev->regs.modes.hdplxdelay = 0;
+    dev->regs.modes.irda = 0;
+
+    rc = max3107_write_regs(dev, MAX3107_REG_MODE1, &dev->regs.modes.mode1, 6);
+    if (rc) {
+        goto end;
+    }
+
+    /* RTS activated at FIFO level 120. */
+    dev->regs.fifo.flow_lvl = 0xFF;
+    dev->regs.fifo.fifo_trg_lvl = 0x38;
+    rc = max3107_write_regs(dev, MAX3107_REG_FLOWLVL, &dev->regs.fifo.flow_lvl, 2);
+    if (rc) {
+        goto end;
+    }
+
+    dev->regs.flow.flow_ctrl = conf->uc_flow_ctl ? (FLOWCTRL_AUTOCTS | FLOWCTRL_AUTORTS) : 0;
+    max3107_write_reg(dev, MAX3107_REG_FLOWCTRL, dev->regs.flow.flow_ctrl);

Review comment:
       All cases are now handled.
   Client will be notified about catastrophic failures in which case closing and opening device should be 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [mynewt-core] kasjer commented on a change in pull request #2605: hw/drivers/uart: Add MAX3107 driver

Posted by GitBox <gi...@apache.org>.
kasjer commented on a change in pull request #2605:
URL: https://github.com/apache/mynewt-core/pull/2605#discussion_r642385351



##########
File path: hw/drivers/uart/max3107/src/max3107.c
##########
@@ -0,0 +1,1059 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 <assert.h>
+#include <string.h>
+
+#include <os/mynewt.h>
+#include <bsp/bsp.h>
+#include <hal/hal_gpio.h>
+#include <hal/hal_uart.h>
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+#include <bus/drivers/spi_common.h>
+#endif
+#include <max3107/max3107.h>
+#include "max3107_priv.h"
+
+#include <uart/uart.h>
+
+static inline int
+max3107_lock(struct max3107_dev *dev)
+{
+    return os_error_to_sys(os_mutex_pend(&dev->lock,
+                                         os_time_ms_to_ticks32(MYNEWT_VAL(MAX3107_LOCK_TIMEOUT))));
+}
+
+static inline void
+max3107_unlock(struct max3107_dev *dev)
+{
+    int rc = os_mutex_release(&dev->lock);
+    assert(rc == 0);
+}
+
+void
+max3107_cs_activate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = (struct bus_spi_node *)&dev->dev;
+    hal_gpio_write(node->pin_cs, 0);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 0);
+#endif
+}
+
+void
+max3107_cs_deactivate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = &dev->dev;
+    hal_gpio_write(node->pin_cs, 1);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 1);
+#endif
+}
+
+int
+max3107_read_regs(struct max3107_dev *dev, uint8_t addr, void *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+#if !MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    uint8_t fast_buf[8];
+#endif
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;
+
+    if (size > 0) {
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+        rc = bus_node_simple_write_read_transact((struct os_dev *)&dev->dev,
+                                                 &addr, 1, buf, size);
+#else
+        max3107_cs_activate(dev);
+
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memset(fast_buf + 1, 0xFF, size);
+            /* Send command + address */
+            rc = hal_spi_txrx(dev->cfg.spi_num, fast_buf, fast_buf, size + 1);
+            if (rc == 0) {
+                memcpy(buf, fast_buf + 1, size);
+            }
+        } else {
+            /* Send command + address */
+            rc = hal_spi_txrx(dev->cfg.spi_num, &addr, NULL, 1);
+            if (rc == 0) {
+                /* For security mostly, do not output random data, fill it with FF */
+                memset(buf, 0xFF, size);
+                /* Tx buf does not matter, for simplicity pass read buffer */
+                rc = hal_spi_txrx(dev->cfg.spi_num, buf, buf, (int)size);
+            }
+        }
+
+        max3107_cs_deactivate(dev);
+#endif
+    }
+
+    if (locked) {
+        max3107_unlock(dev);
+    }
+
+    return rc;
+}
+
+int
+max3107_write_regs(struct max3107_dev *dev, uint8_t addr, const uint8_t *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+    uint8_t fast_buf[17];
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;
+
+    if (size) {
+        /* For writes set MSB */
+        addr |= 0x80;
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+        rc = bus_node_lock((struct os_dev *)&dev->dev,
+                           BUS_NODE_LOCK_DEFAULT_TIMEOUT);
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memcpy(fast_buf + 1, buf, size);
+            rc = bus_node_simple_write((struct os_dev *)&dev->dev, fast_buf, size + 1);
+        } else {
+            if (rc == 0) {
+                rc = bus_node_write((struct os_dev *)&dev->dev,
+                                    &addr, 1,
+                                    BUS_NODE_LOCK_DEFAULT_TIMEOUT, BUS_F_NOSTOP);
+                if (rc == 0) {
+                    rc = bus_node_simple_write((struct os_dev *)&dev->dev, buf, size);
+                }
+            }
+        }
+        (void)bus_node_unlock((struct os_dev *)&dev->dev);
+#else
+        max3107_cs_activate(dev);
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memcpy(fast_buf + 1, buf, size);
+            rc = hal_spi_txrx(dev->cfg.spi_num, fast_buf, NULL, size + 1);
+        } else {
+            rc = hal_spi_txrx(dev->cfg.spi_num, &addr, NULL, 1);
+            if (rc == 0) {
+                rc = hal_spi_txrx(dev->cfg.spi_num, (void *)buf, NULL, (int)size);
+            }
+        }
+        max3107_cs_deactivate(dev);
+#endif
+    }
+    if (locked) {
+        max3107_unlock(dev);
+    }
+
+    return rc;
+}
+
+static int
+max3107_write_reg(struct max3107_dev *dev, uint8_t addr, uint8_t val)
+{
+    return max3107_write_regs(dev, addr, &val, 1);
+}
+
+static int
+max3107_write_fifo(struct max3107_dev *dev, const uint8_t *buf, uint32_t size)
+{
+    return max3107_write_regs(dev, 0, buf, size);
+}
+
+int
+max3107_read_fifo(struct max3107_dev *dev, uint8_t *buf, uint32_t size)
+{
+    return max3107_read_regs(dev, 0, buf, size);
+}
+
+static const uint8_t factors[5] = { 1, 6, 48, 96, 144 };
+/* "F PLL in" from datasheet: Table 4. PLLFactor[1:0] Selection Guide */
+static const uint32_t fpllin_min[5] = { 1, 500000,  850000,  425000, 390000 };
+static const uint32_t fpllin_max[5] = { 1, 800000, 1200000, 1000000, 666666 };
+
+static uint32_t
+max3107_compute_clock_config(uint32_t clockf, uint32_t br, struct max3107_clock_cfg *cfg)
+{
+    uint32_t div;
+    uint32_t pre_div;
+    uint32_t pre_div_min;
+    uint32_t pre_div_max;
+    uint32_t mul;
+    uint32_t actual_br;
+    uint32_t best_br = 1;
+    uint32_t fref;
+    int factor_ix;
+    int max_factor = (cfg->clk_source & CLCSOURCE_PLLBYPASS) ? 1 : 5;
+    uint32_t mode_mul = (cfg->brg_config & BRGCONFIG_4XMODE) ? 4 : (cfg->brg_config & BRGCONFIG_2XMODE) ? 2 : 1;
+
+    /* Clock will be needed. */
+    cfg->clk_source |= CLCSOURCE_CLOCKEN;
+
+    /* If bypass was not disable at the start try finding more accurate clock settings. */
+    for (factor_ix = 0; factor_ix < max_factor; ++factor_ix) {
+        /*
+         * Pre divider does not apply when PLL is not used. Set lower and upper
+         * limits to 1 to have same code for PLL/non PLL.
+         */
+        if (factor_ix == 0) {
+            pre_div_min = 1;
+            pre_div_max = 1;
+        } else {
+            /* Lower and upper frequency limits used to get pre divider range */
+            pre_div_max = clockf / fpllin_min[factor_ix];
+            pre_div_min = (clockf + fpllin_max[factor_ix] - 1) / fpllin_max[factor_ix];
+            /* Make sure divider is in correct range. */
+            pre_div_min = min(63, pre_div_min);
+            pre_div_min = max(1, pre_div_min);
+            pre_div_max = min(63, pre_div_max);
+            pre_div_max = max(1, pre_div_max);
+        }
+        /* Loop for x1, x2 and x4 modes. */
+        for (mul = 1; mul <= mode_mul; mul <<= 1) {
+            for (pre_div = pre_div_min; pre_div <= pre_div_max; ++pre_div) {
+                fref = (clockf / pre_div) * factors[factor_ix];
+                div = (fref * mul + (br / 2)) / br;
+                div = max(div, 16);
+
+                actual_br = mul * fref / div;
+                if (abs((int)(actual_br - br)) < abs((int)(best_br - br))) {
+                    best_br = actual_br;
+                    cfg->div_lsb = (uint8_t)(div >> 4);
+                    cfg->div_msb = (uint8_t)(div >> 12);
+                    cfg->brg_config = (div & 0xF) | ((mul == 4) ? 0x20 : ((mul == 2) ? 0x10 : 0));
+                    /* If best choice is factor_ix == 0, no need for PLL */
+                    if (factor_ix == 0) {
+                        cfg->clk_source |= CLCSOURCE_PLLBYPASS;
+                        cfg->clk_source &= ~CLCSOURCE_PLLEN;
+                    } else {
+                        cfg->pll_config = pre_div | ((factor_ix - 1) << 6);
+                        cfg->clk_source &= ~CLCSOURCE_PLLBYPASS;
+                        cfg->clk_source |= CLCSOURCE_PLLEN;
+                    }
+                }
+            }
+        }
+    }
+
+    return best_br;
+}
+
+int
+max3107_config_uart(struct max3107_dev *dev, const struct uart_conf_port *conf)
+{
+    int rc;
+
+    if (dev->cfg.crystal_en) {
+        dev->regs.clock.clk_source |= CLCSOURCE_CRYSTALEN;
+    } else {
+        dev->regs.clock.clk_source &= ~CLCSOURCE_CRYSTALEN;
+    }
+    if (dev->cfg.no_pll) {
+        dev->regs.clock.clk_source |= CLCSOURCE_PLLBYPASS;
+    } else {
+        dev->regs.clock.clk_source &= ~CLCSOURCE_PLLBYPASS;
+    }
+    if (dev->cfg.allow_mul_4) {
+        dev->regs.clock.brg_config = BRGCONFIG_4XMODE;
+    } else if (dev->cfg.allow_mul_2) {
+        dev->regs.clock.brg_config = BRGCONFIG_2XMODE;
+    } else {
+        dev->regs.clock.brg_config = 0;
+    }
+    max3107_compute_clock_config(dev->cfg.osc_freq, conf->uc_speed, &dev->regs.clock);

Review comment:
       Separate function to get actual baudrate added




-- 
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



[GitHub] [mynewt-core] sjanc commented on a change in pull request #2605: hw/drivers/uart: Add MAX3107 driver

Posted by GitBox <gi...@apache.org>.
sjanc commented on a change in pull request #2605:
URL: https://github.com/apache/mynewt-core/pull/2605#discussion_r648027114



##########
File path: hw/drivers/uart/max3107/src/max3107.c
##########
@@ -0,0 +1,1090 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 <assert.h>
+#include <string.h>
+
+#include <os/mynewt.h>
+#include <bsp/bsp.h>
+#include <hal/hal_gpio.h>
+#include <hal/hal_uart.h>
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+#include <bus/drivers/spi_common.h>
+#endif
+#include <max3107/max3107.h>
+#include "max3107_priv.h"
+
+#include <uart/uart.h>
+
+static inline int
+max3107_lock(struct max3107_dev *dev)
+{
+    return os_error_to_sys(os_mutex_pend(&dev->lock,
+                                         os_time_ms_to_ticks32(MYNEWT_VAL(MAX3107_LOCK_TIMEOUT))));
+}
+
+static inline void
+max3107_unlock(struct max3107_dev *dev)
+{
+    int rc = os_mutex_release(&dev->lock);
+    assert(rc == 0);
+}
+
+void
+max3107_cs_activate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = (struct bus_spi_node *)&dev->dev;
+    hal_gpio_write(node->pin_cs, 0);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 0);
+#endif
+}
+
+void
+max3107_cs_deactivate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = &dev->dev;
+    hal_gpio_write(node->pin_cs, 1);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 1);
+#endif
+}
+
+int
+max3107_read_regs(struct max3107_dev *dev, uint8_t addr, void *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+#if !MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    uint8_t fast_buf[8];
+#endif
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;
+
+    if (size > 0) {
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+        rc = bus_node_simple_write_read_transact((struct os_dev *)&dev->dev,
+                                                 &addr, 1, buf, size);
+#else
+        max3107_cs_activate(dev);
+
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memset(fast_buf + 1, 0xFF, size);
+            /* Send command + address */
+            rc = hal_spi_txrx(dev->cfg.spi_num, fast_buf, fast_buf, size + 1);
+            if (rc == 0) {
+                memcpy(buf, fast_buf + 1, size);
+            }
+        } else {
+            /* Send command + address */
+            rc = hal_spi_txrx(dev->cfg.spi_num, &addr, NULL, 1);
+            if (rc == 0) {
+                /* For security mostly, do not output random data, fill it with FF */
+                memset(buf, 0xFF, size);
+                /* Tx buf does not matter, for simplicity pass read buffer */
+                rc = hal_spi_txrx(dev->cfg.spi_num, buf, buf, (int)size);
+            }
+        }
+
+        max3107_cs_deactivate(dev);
+#endif
+    }
+
+    if (locked) {
+        max3107_unlock(dev);
+    }
+
+    return rc;
+}
+
+int
+max3107_write_regs(struct max3107_dev *dev, uint8_t addr, const uint8_t *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+    uint8_t fast_buf[17];
+
+    rc = max3107_lock(dev);

Review comment:
       ditto

##########
File path: hw/drivers/uart/max3107/src/max3107.c
##########
@@ -0,0 +1,1090 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 <assert.h>
+#include <string.h>
+
+#include <os/mynewt.h>
+#include <bsp/bsp.h>
+#include <hal/hal_gpio.h>
+#include <hal/hal_uart.h>
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+#include <bus/drivers/spi_common.h>
+#endif
+#include <max3107/max3107.h>
+#include "max3107_priv.h"
+
+#include <uart/uart.h>
+
+static inline int
+max3107_lock(struct max3107_dev *dev)
+{
+    return os_error_to_sys(os_mutex_pend(&dev->lock,
+                                         os_time_ms_to_ticks32(MYNEWT_VAL(MAX3107_LOCK_TIMEOUT))));
+}
+
+static inline void
+max3107_unlock(struct max3107_dev *dev)
+{
+    int rc = os_mutex_release(&dev->lock);
+    assert(rc == 0);
+}
+
+void
+max3107_cs_activate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = (struct bus_spi_node *)&dev->dev;
+    hal_gpio_write(node->pin_cs, 0);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 0);
+#endif
+}
+
+void
+max3107_cs_deactivate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = &dev->dev;
+    hal_gpio_write(node->pin_cs, 1);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 1);
+#endif
+}
+
+int
+max3107_read_regs(struct max3107_dev *dev, uint8_t addr, void *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+#if !MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    uint8_t fast_buf[8];
+#endif
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;

Review comment:
       shouldn't this return if failed to lock device?

##########
File path: hw/drivers/uart/max3107/src/max3107.c
##########
@@ -0,0 +1,1090 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 <assert.h>
+#include <string.h>
+
+#include <os/mynewt.h>
+#include <bsp/bsp.h>
+#include <hal/hal_gpio.h>
+#include <hal/hal_uart.h>
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+#include <bus/drivers/spi_common.h>
+#endif
+#include <max3107/max3107.h>
+#include "max3107_priv.h"
+
+#include <uart/uart.h>
+
+static inline int
+max3107_lock(struct max3107_dev *dev)
+{
+    return os_error_to_sys(os_mutex_pend(&dev->lock,
+                                         os_time_ms_to_ticks32(MYNEWT_VAL(MAX3107_LOCK_TIMEOUT))));
+}
+
+static inline void
+max3107_unlock(struct max3107_dev *dev)
+{
+    int rc = os_mutex_release(&dev->lock);
+    assert(rc == 0);
+}
+
+void
+max3107_cs_activate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = (struct bus_spi_node *)&dev->dev;
+    hal_gpio_write(node->pin_cs, 0);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 0);
+#endif
+}
+
+void
+max3107_cs_deactivate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = &dev->dev;
+    hal_gpio_write(node->pin_cs, 1);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 1);
+#endif
+}
+
+int
+max3107_read_regs(struct max3107_dev *dev, uint8_t addr, void *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+#if !MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    uint8_t fast_buf[8];
+#endif
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;
+
+    if (size > 0) {
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+        rc = bus_node_simple_write_read_transact((struct os_dev *)&dev->dev,
+                                                 &addr, 1, buf, size);
+#else
+        max3107_cs_activate(dev);
+
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memset(fast_buf + 1, 0xFF, size);
+            /* Send command + address */
+            rc = hal_spi_txrx(dev->cfg.spi_num, fast_buf, fast_buf, size + 1);
+            if (rc == 0) {
+                memcpy(buf, fast_buf + 1, size);
+            }
+        } else {
+            /* Send command + address */
+            rc = hal_spi_txrx(dev->cfg.spi_num, &addr, NULL, 1);
+            if (rc == 0) {
+                /* For security mostly, do not output random data, fill it with FF */
+                memset(buf, 0xFF, size);
+                /* Tx buf does not matter, for simplicity pass read buffer */
+                rc = hal_spi_txrx(dev->cfg.spi_num, buf, buf, (int)size);
+            }
+        }
+
+        max3107_cs_deactivate(dev);
+#endif
+    }
+
+    if (locked) {
+        max3107_unlock(dev);
+    }
+
+    return rc;
+}
+
+int
+max3107_write_regs(struct max3107_dev *dev, uint8_t addr, const uint8_t *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+    uint8_t fast_buf[17];
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;
+
+    if (size) {
+        /* For writes set MSB */
+        addr |= 0x80;
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+        rc = bus_node_lock((struct os_dev *)&dev->dev,
+                           BUS_NODE_LOCK_DEFAULT_TIMEOUT);
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memcpy(fast_buf + 1, buf, size);
+            rc = bus_node_simple_write((struct os_dev *)&dev->dev, fast_buf, size + 1);
+        } else {
+            if (rc == 0) {
+                rc = bus_node_write((struct os_dev *)&dev->dev,
+                                    &addr, 1,
+                                    BUS_NODE_LOCK_DEFAULT_TIMEOUT, BUS_F_NOSTOP);
+                if (rc == 0) {
+                    rc = bus_node_simple_write((struct os_dev *)&dev->dev, buf, size);
+                }
+            }
+        }
+        (void)bus_node_unlock((struct os_dev *)&dev->dev);
+#else
+        max3107_cs_activate(dev);
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memcpy(fast_buf + 1, buf, size);
+            rc = hal_spi_txrx(dev->cfg.spi_num, fast_buf, NULL, size + 1);
+        } else {
+            rc = hal_spi_txrx(dev->cfg.spi_num, &addr, NULL, 1);
+            if (rc == 0) {
+                rc = hal_spi_txrx(dev->cfg.spi_num, (void *)buf, NULL, (int)size);
+            }
+        }
+        max3107_cs_deactivate(dev);
+#endif
+    }
+    if (locked) {
+        max3107_unlock(dev);
+    }
+
+    return rc;
+}
+
+static int
+max3107_write_reg(struct max3107_dev *dev, uint8_t addr, uint8_t val)
+{
+    return max3107_write_regs(dev, addr, &val, 1);
+}
+
+static int
+max3107_write_fifo(struct max3107_dev *dev, const uint8_t *buf, uint32_t size)
+{
+    return max3107_write_regs(dev, 0, buf, size);
+}
+
+static int
+max3107_read_fifo(struct max3107_dev *dev, uint8_t *buf, uint32_t size)
+{
+    return max3107_read_regs(dev, 0, buf, size);
+}
+
+static const uint8_t factors[5] = { 1, 6, 48, 96, 144 };
+/* "F PLL in" from datasheet: Table 4. PLLFactor[1:0] Selection Guide */
+static const uint32_t fpllin_min[5] = { 1, 500000,  850000,  425000, 390000 };
+static const uint32_t fpllin_max[5] = { 1, 800000, 1200000, 1000000, 666666 };
+
+static uint32_t
+max3107_compute_clock_config(uint32_t clockf, uint32_t br, struct max3107_clock_cfg *cfg)
+{
+    uint32_t div;
+    uint32_t pre_div;
+    uint32_t pre_div_min;
+    uint32_t pre_div_max;
+    uint32_t mul;
+    uint32_t actual_br;
+    uint32_t best_br = 1;
+    uint32_t fref;
+    int factor_ix;
+    int max_factor = (cfg->clk_source & CLCSOURCE_PLLBYPASS) ? 1 : 5;
+    uint32_t mode_mul = (cfg->brg_config & BRGCONFIG_4XMODE) ? 4 : (cfg->brg_config & BRGCONFIG_2XMODE) ? 2 : 1;
+
+    /* Clock will be needed. */
+    cfg->clk_source |= CLCSOURCE_CLOCKEN;
+
+    /* If bypass was not disable at the start try finding more accurate clock settings. */
+    for (factor_ix = 0; factor_ix < max_factor; ++factor_ix) {
+        /*
+         * Pre divider does not apply when PLL is not used. Set lower and upper
+         * limits to 1 to have same code for PLL/non PLL.
+         */
+        if (factor_ix == 0) {
+            pre_div_min = 1;
+            pre_div_max = 1;
+        } else {
+            /* Lower and upper frequency limits used to get pre divider range */
+            pre_div_max = clockf / fpllin_min[factor_ix];
+            pre_div_min = (clockf + fpllin_max[factor_ix] - 1) / fpllin_max[factor_ix];
+            /* Make sure divider is in correct range. */
+            pre_div_min = min(63, pre_div_min);
+            pre_div_min = max(1, pre_div_min);
+            pre_div_max = min(63, pre_div_max);
+            pre_div_max = max(1, pre_div_max);
+        }
+        /* Loop for x1, x2 and x4 modes. */
+        for (mul = 1; mul <= mode_mul; mul <<= 1) {
+            for (pre_div = pre_div_min; pre_div <= pre_div_max; ++pre_div) {
+                fref = (clockf / pre_div) * factors[factor_ix];
+                div = (fref * mul + (br / 2)) / br;
+                div = max(div, 16);
+
+                actual_br = mul * fref / div;
+                if (abs((int)(actual_br - br)) < abs((int)(best_br - br))) {
+                    best_br = actual_br;
+                    cfg->div_lsb = (uint8_t)(div >> 4);
+                    cfg->div_msb = (uint8_t)(div >> 12);
+                    cfg->brg_config = (div & 0xF) | ((mul == 4) ? 0x20 : ((mul == 2) ? 0x10 : 0));
+                    /* If best choice is factor_ix == 0, no need for PLL */
+                    if (factor_ix == 0) {
+                        cfg->clk_source |= CLCSOURCE_PLLBYPASS;
+                        cfg->clk_source &= ~CLCSOURCE_PLLEN;
+                    } else {
+                        cfg->pll_config = pre_div | ((factor_ix - 1) << 6);
+                        cfg->clk_source &= ~CLCSOURCE_PLLBYPASS;
+                        cfg->clk_source |= CLCSOURCE_PLLEN;
+                    }
+                }
+            }
+        }
+    }
+
+    return best_br;
+}
+
+int
+max3107_config_uart(struct max3107_dev *dev, const struct uart_conf_port *conf)
+{
+    int rc;
+
+    if (dev->cfg.crystal_en) {
+        dev->regs.clock.clk_source |= CLCSOURCE_CRYSTALEN;
+    } else {
+        dev->regs.clock.clk_source &= ~CLCSOURCE_CRYSTALEN;
+    }
+    if (dev->cfg.no_pll) {
+        dev->regs.clock.clk_source |= CLCSOURCE_PLLBYPASS;
+    } else {
+        dev->regs.clock.clk_source &= ~CLCSOURCE_PLLBYPASS;
+    }
+    if (dev->cfg.allow_mul_4) {
+        dev->regs.clock.brg_config = BRGCONFIG_4XMODE;
+    } else if (dev->cfg.allow_mul_2) {
+        dev->regs.clock.brg_config = BRGCONFIG_2XMODE;
+    } else {
+        dev->regs.clock.brg_config = 0;
+    }
+    max3107_compute_clock_config(dev->cfg.osc_freq, conf->uc_speed, &dev->regs.clock);
+
+    rc = max3107_write_regs(dev, MAX3107_REG_PLLCONFIG, &dev->regs.clock.pll_config, 5);
+    if (rc) {
+        goto end;
+    }
+
+    dev->regs.modes.mode1 = MODE1_IRQSEL | (conf->uc_flow_ctl ? 0 : MODE1_RTSHIZ);
+    dev->regs.modes.mode2 = 0;
+    dev->regs.modes.lcr = (dev->uart.ud_conf_port.uc_stopbits == 1 ? 0 : LCR_STOPBITS) |
+                          (dev->uart.ud_conf_port.uc_databits - 5) |
+                          ((dev->uart.ud_conf_port.uc_parity != UART_PARITY_NONE) ? LCR_PARITYEN : 0 ) |
+                          ((dev->uart.ud_conf_port.uc_parity == UART_PARITY_EVEN) ? LCR_EVENPARITY : 0);
+    dev->regs.modes.rxtimeout = 2;
+    dev->regs.modes.hdplxdelay = 0;
+    dev->regs.modes.irda = 0;
+
+    rc = max3107_write_regs(dev, MAX3107_REG_MODE1, &dev->regs.modes.mode1, 6);
+    if (rc) {
+        goto end;
+    }
+
+    /* RTS activated at FIFO level 120. */
+    dev->regs.fifo.flow_lvl = 0xFF;
+    dev->regs.fifo.fifo_trg_lvl = (dev->cfg.rx_trigger_level >> 3) << 4 | (dev->cfg.tx_trigger_level >> 3);
+    rc = max3107_write_regs(dev, MAX3107_REG_FLOWLVL, &dev->regs.fifo.flow_lvl, 2);
+    if (rc) {
+        goto end;
+    }
+
+    dev->regs.flow.flow_ctrl = conf->uc_flow_ctl ? (FLOWCTRL_AUTOCTS | FLOWCTRL_AUTORTS) : 0;
+    rc = max3107_write_reg(dev, MAX3107_REG_FLOWCTRL, dev->regs.flow.flow_ctrl);
+    if (rc) {
+        goto end;
+    }
+
+    dev->regs.ints.irq_en = IRQEN_LSRERRIEN | IRQEN_RXTRGIEN;
+    dev->regs.ints.lsr_int_en = LSRINTEN_FRAMEERRIEN | LSRINTEN_PARITYIEN |
+                                LSRINTEN_RBREAKIEN | LSRINTEN_ROVERRIEN | LSRINTEN_RTIMEOUTIEN;
+    rc = max3107_write_regs(dev, MAX3107_REG_IRQEN, &dev->regs.ints.irq_en, 3);
+end:

Review comment:
       this label doesn't do any cleanup etc so maybe avoid all those gotos?




-- 
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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2605: hw/drivers/uart: Add MAX3107 driver

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2605:
URL: https://github.com/apache/mynewt-core/pull/2605#discussion_r640905555



##########
File path: hw/drivers/uart/max3107/syscfg.yml
##########
@@ -0,0 +1,134 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you 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.
+#
+
+syscfg.defs:
+    MAX3107_0:
+        description: >
+            Create SPI RAM device instance.
+        value: 0
+
+    MAX3107_0_NAME:
+        description: >
+            MAX3107 device name.
+        value: '"max3107_0"'
+
+    MAX3107_0_UART_NAME:
+        description: >
+            MAX3107 UART device name.
+        value: '"uart4"'
+
+    MAX3107_0_SPI_BAUDRATE:
+        description: >
+            MAX3107 SPI clock speed in kHz.
+        value: 4000
+
+    MAX3107_0_UART_BAUDRATE:
+        description: >
+            MAX3107 UART clock speed in bits/second.
+        value: 115200
+
+    MAX3107_0_UART_DATA_BITS:
+        description: >
+            MAX3107 number of data bits.
+        range: 5,6,7,8
+        value: 8
+
+    MAX3107_0_UART_PARITY:
+        description: >
+            MAX3107 parity 0 - no parity, 1 - odd, 2 - even.
+        range: 0,1,2
+        value: 0
+
+    MAX3107_0_UART_STOP_BITS:
+        description: >
+            MAX3107 UART stop bits 1 or 2.
+        range: 1,2
+        value: 1
+
+    MAX3107_0_UART_FLOW_CONTROL:
+        description: >
+            MAX3107 UART flow control 0 - none, 1 - RTS CTS.
+        range: 0,1
+        value: 0
+
+    MAX3107_0_CS_PIN:
+        description: >
+            Chip select pin for MAX3107.
+        value: -1
+
+    MAX3107_0_SPI_BUS:
+        description: >
+            SPI interface bus name for MAX3107.
+        value : '"spi0"'
+
+    MAX3107_0_SPI_NUM:
+        description: >
+            SPI interface used for SPI RAM.
+        value: 0
+
+    MAX3107_0_OSC_FREQ:
+        description: >
+            External clock/oscillator frequency.
+        value:

Review comment:
       Yeah, you are right since this is an external oscillator but there needs to be a range or something associated with this.




-- 
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



[GitHub] [mynewt-core] kasjer commented on pull request #2605: hw/drivers/uart: Add MAX3107 driver

Posted by GitBox <gi...@apache.org>.
kasjer commented on pull request #2605:
URL: https://github.com/apache/mynewt-core/pull/2605#issuecomment-858539553


   > Also, a basic CLI to query registers of the chip would be useful for debugging.
   
   I will add separate PR for that that.


-- 
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



[GitHub] [mynewt-core] vrahane merged pull request #2605: hw/drivers/uart: Add MAX3107 driver

Posted by GitBox <gi...@apache.org>.
vrahane merged pull request #2605:
URL: https://github.com/apache/mynewt-core/pull/2605


   


-- 
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



[GitHub] [mynewt-core] kasjer commented on a change in pull request #2605: hw/drivers/uart: Add MAX3107 driver

Posted by GitBox <gi...@apache.org>.
kasjer commented on a change in pull request #2605:
URL: https://github.com/apache/mynewt-core/pull/2605#discussion_r640888012



##########
File path: hw/drivers/uart/max3107/src/max3107.c
##########
@@ -0,0 +1,1059 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 <assert.h>
+#include <string.h>
+
+#include <os/mynewt.h>
+#include <bsp/bsp.h>
+#include <hal/hal_gpio.h>
+#include <hal/hal_uart.h>
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+#include <bus/drivers/spi_common.h>
+#endif
+#include <max3107/max3107.h>
+#include "max3107_priv.h"
+
+#include <uart/uart.h>
+
+static inline int
+max3107_lock(struct max3107_dev *dev)
+{
+    return os_error_to_sys(os_mutex_pend(&dev->lock,
+                                         os_time_ms_to_ticks32(MYNEWT_VAL(MAX3107_LOCK_TIMEOUT))));
+}
+
+static inline void
+max3107_unlock(struct max3107_dev *dev)
+{
+    int rc = os_mutex_release(&dev->lock);
+    assert(rc == 0);
+}
+
+void
+max3107_cs_activate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = (struct bus_spi_node *)&dev->dev;
+    hal_gpio_write(node->pin_cs, 0);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 0);
+#endif
+}
+
+void
+max3107_cs_deactivate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = &dev->dev;
+    hal_gpio_write(node->pin_cs, 1);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 1);
+#endif
+}
+
+int
+max3107_read_regs(struct max3107_dev *dev, uint8_t addr, void *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+#if !MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    uint8_t fast_buf[8];
+#endif
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;
+
+    if (size > 0) {
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+        rc = bus_node_simple_write_read_transact((struct os_dev *)&dev->dev,
+                                                 &addr, 1, buf, size);
+#else
+        max3107_cs_activate(dev);
+
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memset(fast_buf + 1, 0xFF, size);
+            /* Send command + address */
+            rc = hal_spi_txrx(dev->cfg.spi_num, fast_buf, fast_buf, size + 1);
+            if (rc == 0) {
+                memcpy(buf, fast_buf + 1, size);
+            }
+        } else {
+            /* Send command + address */
+            rc = hal_spi_txrx(dev->cfg.spi_num, &addr, NULL, 1);
+            if (rc == 0) {
+                /* For security mostly, do not output random data, fill it with FF */
+                memset(buf, 0xFF, size);
+                /* Tx buf does not matter, for simplicity pass read buffer */
+                rc = hal_spi_txrx(dev->cfg.spi_num, buf, buf, (int)size);
+            }
+        }
+
+        max3107_cs_deactivate(dev);
+#endif
+    }
+
+    if (locked) {
+        max3107_unlock(dev);
+    }
+
+    return rc;
+}
+
+int
+max3107_write_regs(struct max3107_dev *dev, uint8_t addr, const uint8_t *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+    uint8_t fast_buf[17];
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;
+
+    if (size) {
+        /* For writes set MSB */
+        addr |= 0x80;
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+        rc = bus_node_lock((struct os_dev *)&dev->dev,
+                           BUS_NODE_LOCK_DEFAULT_TIMEOUT);
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memcpy(fast_buf + 1, buf, size);
+            rc = bus_node_simple_write((struct os_dev *)&dev->dev, fast_buf, size + 1);
+        } else {
+            if (rc == 0) {
+                rc = bus_node_write((struct os_dev *)&dev->dev,
+                                    &addr, 1,
+                                    BUS_NODE_LOCK_DEFAULT_TIMEOUT, BUS_F_NOSTOP);
+                if (rc == 0) {
+                    rc = bus_node_simple_write((struct os_dev *)&dev->dev, buf, size);
+                }
+            }
+        }
+        (void)bus_node_unlock((struct os_dev *)&dev->dev);
+#else
+        max3107_cs_activate(dev);
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memcpy(fast_buf + 1, buf, size);
+            rc = hal_spi_txrx(dev->cfg.spi_num, fast_buf, NULL, size + 1);
+        } else {
+            rc = hal_spi_txrx(dev->cfg.spi_num, &addr, NULL, 1);
+            if (rc == 0) {
+                rc = hal_spi_txrx(dev->cfg.spi_num, (void *)buf, NULL, (int)size);
+            }
+        }
+        max3107_cs_deactivate(dev);
+#endif
+    }
+    if (locked) {
+        max3107_unlock(dev);
+    }
+
+    return rc;
+}
+
+static int
+max3107_write_reg(struct max3107_dev *dev, uint8_t addr, uint8_t val)
+{
+    return max3107_write_regs(dev, addr, &val, 1);
+}
+
+static int
+max3107_write_fifo(struct max3107_dev *dev, const uint8_t *buf, uint32_t size)
+{
+    return max3107_write_regs(dev, 0, buf, size);
+}
+
+int
+max3107_read_fifo(struct max3107_dev *dev, uint8_t *buf, uint32_t size)
+{
+    return max3107_read_regs(dev, 0, buf, size);
+}
+
+static const uint8_t factors[5] = { 1, 6, 48, 96, 144 };
+/* "F PLL in" from datasheet: Table 4. PLLFactor[1:0] Selection Guide */
+static const uint32_t fpllin_min[5] = { 1, 500000,  850000,  425000, 390000 };
+static const uint32_t fpllin_max[5] = { 1, 800000, 1200000, 1000000, 666666 };
+
+static uint32_t
+max3107_compute_clock_config(uint32_t clockf, uint32_t br, struct max3107_clock_cfg *cfg)
+{
+    uint32_t div;
+    uint32_t pre_div;
+    uint32_t pre_div_min;
+    uint32_t pre_div_max;
+    uint32_t mul;
+    uint32_t actual_br;
+    uint32_t best_br = 1;
+    uint32_t fref;
+    int factor_ix;
+    int max_factor = (cfg->clk_source & CLCSOURCE_PLLBYPASS) ? 1 : 5;
+    uint32_t mode_mul = (cfg->brg_config & BRGCONFIG_4XMODE) ? 4 : (cfg->brg_config & BRGCONFIG_2XMODE) ? 2 : 1;
+
+    /* Clock will be needed. */
+    cfg->clk_source |= CLCSOURCE_CLOCKEN;
+
+    /* If bypass was not disable at the start try finding more accurate clock settings. */
+    for (factor_ix = 0; factor_ix < max_factor; ++factor_ix) {
+        /*
+         * Pre divider does not apply when PLL is not used. Set lower and upper
+         * limits to 1 to have same code for PLL/non PLL.
+         */
+        if (factor_ix == 0) {
+            pre_div_min = 1;
+            pre_div_max = 1;
+        } else {
+            /* Lower and upper frequency limits used to get pre divider range */
+            pre_div_max = clockf / fpllin_min[factor_ix];
+            pre_div_min = (clockf + fpllin_max[factor_ix] - 1) / fpllin_max[factor_ix];
+            /* Make sure divider is in correct range. */
+            pre_div_min = min(63, pre_div_min);
+            pre_div_min = max(1, pre_div_min);
+            pre_div_max = min(63, pre_div_max);
+            pre_div_max = max(1, pre_div_max);
+        }
+        /* Loop for x1, x2 and x4 modes. */
+        for (mul = 1; mul <= mode_mul; mul <<= 1) {
+            for (pre_div = pre_div_min; pre_div <= pre_div_max; ++pre_div) {
+                fref = (clockf / pre_div) * factors[factor_ix];
+                div = (fref * mul + (br / 2)) / br;
+                div = max(div, 16);
+
+                actual_br = mul * fref / div;
+                if (abs((int)(actual_br - br)) < abs((int)(best_br - br))) {
+                    best_br = actual_br;
+                    cfg->div_lsb = (uint8_t)(div >> 4);
+                    cfg->div_msb = (uint8_t)(div >> 12);
+                    cfg->brg_config = (div & 0xF) | ((mul == 4) ? 0x20 : ((mul == 2) ? 0x10 : 0));
+                    /* If best choice is factor_ix == 0, no need for PLL */
+                    if (factor_ix == 0) {
+                        cfg->clk_source |= CLCSOURCE_PLLBYPASS;
+                        cfg->clk_source &= ~CLCSOURCE_PLLEN;
+                    } else {
+                        cfg->pll_config = pre_div | ((factor_ix - 1) << 6);
+                        cfg->clk_source &= ~CLCSOURCE_PLLBYPASS;
+                        cfg->clk_source |= CLCSOURCE_PLLEN;
+                    }
+                }
+            }
+        }
+    }
+
+    return best_br;
+}
+
+int
+max3107_config_uart(struct max3107_dev *dev, const struct uart_conf_port *conf)
+{
+    int rc;
+
+    if (dev->cfg.crystal_en) {
+        dev->regs.clock.clk_source |= CLCSOURCE_CRYSTALEN;
+    } else {
+        dev->regs.clock.clk_source &= ~CLCSOURCE_CRYSTALEN;
+    }
+    if (dev->cfg.no_pll) {
+        dev->regs.clock.clk_source |= CLCSOURCE_PLLBYPASS;
+    } else {
+        dev->regs.clock.clk_source &= ~CLCSOURCE_PLLBYPASS;
+    }
+    if (dev->cfg.allow_mul_4) {
+        dev->regs.clock.brg_config = BRGCONFIG_4XMODE;
+    } else if (dev->cfg.allow_mul_2) {
+        dev->regs.clock.brg_config = BRGCONFIG_2XMODE;
+    } else {
+        dev->regs.clock.brg_config = 0;
+    }
+    max3107_compute_clock_config(dev->cfg.osc_freq, conf->uc_speed, &dev->regs.clock);

Review comment:
       Other drivers silently set register values from requested baud rate. NRF5x for example outlines in product specification what is the actual baud rate for requested one. For MCUs, code calculates it in much simpler way since there is only one clock involved (or takes values from table) and you don't get information what is the actual speed at all.
   In MAX3107 case which external clock/oscillator, code finds most accurate settings and returns it so user can verify it during the development phase by just reading return value of this functions instead of checking registers and computing it manually.
   So this value is informative only. For example on my dev board that has 36868400 clock I could get as high as 19,7 MHz and not 24MHz that seems to be possible from MAX3107 datasheet.




-- 
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



[GitHub] [mynewt-core] vrahane commented on a change in pull request #2605: hw/drivers/uart: Add MAX3107 driver

Posted by GitBox <gi...@apache.org>.
vrahane commented on a change in pull request #2605:
URL: https://github.com/apache/mynewt-core/pull/2605#discussion_r640866571



##########
File path: hw/drivers/uart/max3107/src/max3107.c
##########
@@ -0,0 +1,1059 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 <assert.h>
+#include <string.h>
+
+#include <os/mynewt.h>
+#include <bsp/bsp.h>
+#include <hal/hal_gpio.h>
+#include <hal/hal_uart.h>
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+#include <bus/drivers/spi_common.h>
+#endif
+#include <max3107/max3107.h>
+#include "max3107_priv.h"
+
+#include <uart/uart.h>
+
+static inline int
+max3107_lock(struct max3107_dev *dev)
+{
+    return os_error_to_sys(os_mutex_pend(&dev->lock,
+                                         os_time_ms_to_ticks32(MYNEWT_VAL(MAX3107_LOCK_TIMEOUT))));
+}
+
+static inline void
+max3107_unlock(struct max3107_dev *dev)
+{
+    int rc = os_mutex_release(&dev->lock);
+    assert(rc == 0);
+}
+
+void
+max3107_cs_activate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = (struct bus_spi_node *)&dev->dev;
+    hal_gpio_write(node->pin_cs, 0);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 0);
+#endif
+}
+
+void
+max3107_cs_deactivate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = &dev->dev;
+    hal_gpio_write(node->pin_cs, 1);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 1);
+#endif
+}
+
+int
+max3107_read_regs(struct max3107_dev *dev, uint8_t addr, void *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+#if !MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    uint8_t fast_buf[8];
+#endif
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;
+
+    if (size > 0) {
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+        rc = bus_node_simple_write_read_transact((struct os_dev *)&dev->dev,
+                                                 &addr, 1, buf, size);
+#else
+        max3107_cs_activate(dev);
+
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memset(fast_buf + 1, 0xFF, size);
+            /* Send command + address */
+            rc = hal_spi_txrx(dev->cfg.spi_num, fast_buf, fast_buf, size + 1);
+            if (rc == 0) {
+                memcpy(buf, fast_buf + 1, size);
+            }
+        } else {
+            /* Send command + address */
+            rc = hal_spi_txrx(dev->cfg.spi_num, &addr, NULL, 1);
+            if (rc == 0) {
+                /* For security mostly, do not output random data, fill it with FF */
+                memset(buf, 0xFF, size);
+                /* Tx buf does not matter, for simplicity pass read buffer */
+                rc = hal_spi_txrx(dev->cfg.spi_num, buf, buf, (int)size);
+            }
+        }
+
+        max3107_cs_deactivate(dev);
+#endif
+    }
+
+    if (locked) {
+        max3107_unlock(dev);
+    }
+
+    return rc;
+}
+
+int
+max3107_write_regs(struct max3107_dev *dev, uint8_t addr, const uint8_t *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+    uint8_t fast_buf[17];
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;
+
+    if (size) {
+        /* For writes set MSB */
+        addr |= 0x80;
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+        rc = bus_node_lock((struct os_dev *)&dev->dev,
+                           BUS_NODE_LOCK_DEFAULT_TIMEOUT);
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memcpy(fast_buf + 1, buf, size);
+            rc = bus_node_simple_write((struct os_dev *)&dev->dev, fast_buf, size + 1);
+        } else {
+            if (rc == 0) {
+                rc = bus_node_write((struct os_dev *)&dev->dev,
+                                    &addr, 1,
+                                    BUS_NODE_LOCK_DEFAULT_TIMEOUT, BUS_F_NOSTOP);
+                if (rc == 0) {
+                    rc = bus_node_simple_write((struct os_dev *)&dev->dev, buf, size);
+                }
+            }
+        }
+        (void)bus_node_unlock((struct os_dev *)&dev->dev);
+#else
+        max3107_cs_activate(dev);
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memcpy(fast_buf + 1, buf, size);
+            rc = hal_spi_txrx(dev->cfg.spi_num, fast_buf, NULL, size + 1);
+        } else {
+            rc = hal_spi_txrx(dev->cfg.spi_num, &addr, NULL, 1);
+            if (rc == 0) {
+                rc = hal_spi_txrx(dev->cfg.spi_num, (void *)buf, NULL, (int)size);
+            }
+        }
+        max3107_cs_deactivate(dev);
+#endif
+    }
+    if (locked) {
+        max3107_unlock(dev);
+    }
+
+    return rc;
+}
+
+static int
+max3107_write_reg(struct max3107_dev *dev, uint8_t addr, uint8_t val)
+{
+    return max3107_write_regs(dev, addr, &val, 1);
+}
+
+static int
+max3107_write_fifo(struct max3107_dev *dev, const uint8_t *buf, uint32_t size)
+{
+    return max3107_write_regs(dev, 0, buf, size);
+}
+
+int
+max3107_read_fifo(struct max3107_dev *dev, uint8_t *buf, uint32_t size)
+{
+    return max3107_read_regs(dev, 0, buf, size);
+}
+
+static const uint8_t factors[5] = { 1, 6, 48, 96, 144 };
+/* "F PLL in" from datasheet: Table 4. PLLFactor[1:0] Selection Guide */
+static const uint32_t fpllin_min[5] = { 1, 500000,  850000,  425000, 390000 };
+static const uint32_t fpllin_max[5] = { 1, 800000, 1200000, 1000000, 666666 };
+
+static uint32_t
+max3107_compute_clock_config(uint32_t clockf, uint32_t br, struct max3107_clock_cfg *cfg)
+{
+    uint32_t div;
+    uint32_t pre_div;
+    uint32_t pre_div_min;
+    uint32_t pre_div_max;
+    uint32_t mul;
+    uint32_t actual_br;
+    uint32_t best_br = 1;
+    uint32_t fref;
+    int factor_ix;
+    int max_factor = (cfg->clk_source & CLCSOURCE_PLLBYPASS) ? 1 : 5;
+    uint32_t mode_mul = (cfg->brg_config & BRGCONFIG_4XMODE) ? 4 : (cfg->brg_config & BRGCONFIG_2XMODE) ? 2 : 1;
+
+    /* Clock will be needed. */
+    cfg->clk_source |= CLCSOURCE_CLOCKEN;
+
+    /* If bypass was not disable at the start try finding more accurate clock settings. */
+    for (factor_ix = 0; factor_ix < max_factor; ++factor_ix) {
+        /*
+         * Pre divider does not apply when PLL is not used. Set lower and upper
+         * limits to 1 to have same code for PLL/non PLL.
+         */
+        if (factor_ix == 0) {
+            pre_div_min = 1;
+            pre_div_max = 1;
+        } else {
+            /* Lower and upper frequency limits used to get pre divider range */
+            pre_div_max = clockf / fpllin_min[factor_ix];
+            pre_div_min = (clockf + fpllin_max[factor_ix] - 1) / fpllin_max[factor_ix];
+            /* Make sure divider is in correct range. */
+            pre_div_min = min(63, pre_div_min);
+            pre_div_min = max(1, pre_div_min);
+            pre_div_max = min(63, pre_div_max);
+            pre_div_max = max(1, pre_div_max);
+        }
+        /* Loop for x1, x2 and x4 modes. */
+        for (mul = 1; mul <= mode_mul; mul <<= 1) {
+            for (pre_div = pre_div_min; pre_div <= pre_div_max; ++pre_div) {
+                fref = (clockf / pre_div) * factors[factor_ix];
+                div = (fref * mul + (br / 2)) / br;
+                div = max(div, 16);
+
+                actual_br = mul * fref / div;
+                if (abs((int)(actual_br - br)) < abs((int)(best_br - br))) {
+                    best_br = actual_br;
+                    cfg->div_lsb = (uint8_t)(div >> 4);
+                    cfg->div_msb = (uint8_t)(div >> 12);
+                    cfg->brg_config = (div & 0xF) | ((mul == 4) ? 0x20 : ((mul == 2) ? 0x10 : 0));
+                    /* If best choice is factor_ix == 0, no need for PLL */
+                    if (factor_ix == 0) {
+                        cfg->clk_source |= CLCSOURCE_PLLBYPASS;
+                        cfg->clk_source &= ~CLCSOURCE_PLLEN;
+                    } else {
+                        cfg->pll_config = pre_div | ((factor_ix - 1) << 6);
+                        cfg->clk_source &= ~CLCSOURCE_PLLBYPASS;
+                        cfg->clk_source |= CLCSOURCE_PLLEN;
+                    }
+                }
+            }
+        }
+    }
+
+    return best_br;
+}
+
+int
+max3107_config_uart(struct max3107_dev *dev, const struct uart_conf_port *conf)
+{
+    int rc;
+
+    if (dev->cfg.crystal_en) {
+        dev->regs.clock.clk_source |= CLCSOURCE_CRYSTALEN;
+    } else {
+        dev->regs.clock.clk_source &= ~CLCSOURCE_CRYSTALEN;
+    }
+    if (dev->cfg.no_pll) {
+        dev->regs.clock.clk_source |= CLCSOURCE_PLLBYPASS;
+    } else {
+        dev->regs.clock.clk_source &= ~CLCSOURCE_PLLBYPASS;
+    }
+    if (dev->cfg.allow_mul_4) {
+        dev->regs.clock.brg_config = BRGCONFIG_4XMODE;
+    } else if (dev->cfg.allow_mul_2) {
+        dev->regs.clock.brg_config = BRGCONFIG_2XMODE;
+    } else {
+        dev->regs.clock.brg_config = 0;
+    }
+    max3107_compute_clock_config(dev->cfg.osc_freq, conf->uc_speed, &dev->regs.clock);
+
+    rc = max3107_write_regs(dev, MAX3107_REG_PLLCONFIG, &dev->regs.clock.pll_config, 5);
+    if (rc) {
+        goto end;
+    }
+
+    dev->regs.modes.mode1 = MODE1_IRQSEL | (conf->uc_flow_ctl ? 0 : MODE1_RTSHIZ);
+    dev->regs.modes.mode2 = 0;
+    dev->regs.modes.lcr = (dev->uart.ud_conf_port.uc_stopbits == 1 ? 0 : LCR_STOPBITS) |
+                          (dev->uart.ud_conf_port.uc_databits - 5) |
+                          ((dev->uart.ud_conf_port.uc_parity != UART_PARITY_NONE) ? LCR_PARITYEN : 0 ) |
+                          ((dev->uart.ud_conf_port.uc_parity == UART_PARITY_EVEN) ? LCR_EVENPARITY : 0);
+    dev->regs.modes.rxtimeout = 2;
+    dev->regs.modes.hdplxdelay = 0;
+    dev->regs.modes.irda = 0;
+
+    rc = max3107_write_regs(dev, MAX3107_REG_MODE1, &dev->regs.modes.mode1, 6);
+    if (rc) {
+        goto end;
+    }
+
+    /* RTS activated at FIFO level 120. */
+    dev->regs.fifo.flow_lvl = 0xFF;
+    dev->regs.fifo.fifo_trg_lvl = 0x38;
+    rc = max3107_write_regs(dev, MAX3107_REG_FLOWLVL, &dev->regs.fifo.flow_lvl, 2);
+    if (rc) {
+        goto end;
+    }
+
+    dev->regs.flow.flow_ctrl = conf->uc_flow_ctl ? (FLOWCTRL_AUTOCTS | FLOWCTRL_AUTORTS) : 0;
+    max3107_write_reg(dev, MAX3107_REG_FLOWCTRL, dev->regs.flow.flow_ctrl);
+
+    dev->regs.ints.irq_en = IRQEN_LSRERRIEN | IRQEN_RXTRGIEN;
+    dev->regs.ints.lsr_int_en = LSRINTEN_FRAMEERRIEN | LSRINTEN_PARITYIEN |
+                                LSRINTEN_RBREAKIEN | LSRINTEN_ROVERRIEN | LSRINTEN_RTIMEOUTIEN;
+    rc = max3107_write_regs(dev, MAX3107_REG_IRQEN, &dev->regs.ints.irq_en, 3);
+    assert(dev->regs.ints.irq_en & IRQEN_LSRERRIEN);
+end:
+    return rc;
+}
+
+int
+max3107_rx_available(struct max3107_dev *dev)
+{
+    int rc = 0;
+
+    if (dev->regs.fifo.rx_fifo_lvl == 0) {
+        rc = max3107_read_regs(dev, MAX3107_REG_RXFIFOLVL, &dev->regs.fifo.rx_fifo_lvl, 1);
+    }
+    if (rc == 0) {
+        rc = dev->regs.fifo.rx_fifo_lvl;
+    }
+
+    return rc;
+}
+
+int
+max3107_tx_available(struct max3107_dev *dev)
+{
+    int rc = 0;
+
+    if (dev->regs.fifo.tx_fifo_lvl >= 128) {
+        rc = max3107_read_regs(dev, MAX3107_REG_TXFIFOLVL, &dev->regs.fifo.tx_fifo_lvl, 1);
+    }
+    if (rc == 0) {
+        rc = 128 - dev->regs.fifo.tx_fifo_lvl;
+    }
+
+    return rc;
+}
+
+static int
+max3107_irqen_set(struct max3107_dev *dev, uint8_t enabled_bits)
+{
+    int rc = 0;
+
+    uint8_t irq_en = dev->regs.ints.irq_en | enabled_bits;
+
+    if (irq_en != dev->regs.ints.irq_en) {
+        dev->regs.ints.irq_en = irq_en;
+        rc = max3107_write_reg(dev, MAX3107_REG_IRQEN, dev->regs.ints.irq_en);
+    }
+
+    return rc;
+}
+
+static int
+max3107_irqen_clear(struct max3107_dev *dev, uint8_t cleared_bits)
+{
+    int rc = 0;
+
+    uint8_t irq_en = dev->regs.ints.irq_en & ~cleared_bits;
+
+    if (irq_en != dev->regs.ints.irq_en) {
+        dev->regs.ints.irq_en = irq_en;
+        rc = max3107_write_reg(dev, MAX3107_REG_IRQEN, dev->regs.ints.irq_en);
+    }
+
+    return rc;
+}
+
+int
+max3107_read(struct max3107_dev *dev, void *buf, size_t size)
+{
+    int rc = 0;
+
+    if (dev->regs.fifo.rx_fifo_lvl == 0) {
+        /* Read number of bytes currently in RX FIFO first. */
+        rc = max3107_read_regs(dev, MAX3107_REG_RXFIFOLVL, &dev->regs.fifo.rx_fifo_lvl, 1);
+    }
+
+    if (rc == 0) {
+        size = min(dev->regs.fifo.rx_fifo_lvl, size);
+        if (size > 0) {
+            rc = max3107_read_fifo(dev, buf, size);
+        }
+        if (rc == 0) {
+            dev->readable_notified = false;
+            /* Update FIFO level for potential future use. */
+            dev->regs.fifo.rx_fifo_lvl -= size;
+            /*
+             * If FIFO was possible emptied, read FIFO level in next interrupt,
+             * in case data was coming while FIFO was being read.
+             */
+            dev->rx_fifo_emptied = dev->regs.fifo.rx_fifo_lvl == 0;
+            rc = (int)size;
+        }
+    }
+    return rc;
+}
+
+int
+max3107_write(struct max3107_dev *dev, const void *buf, size_t size)
+{
+    size_t fifo_space = 128 - dev->regs.fifo.tx_fifo_lvl;
+    int rc = 0;
+
+    /*
+     * If FIFO level was read before and there is enough data, there is
+     * no need to check FIFO level again.
+     */
+    if (size > fifo_space) {
+        /* Read number of bytes currently in TX FIFO */
+        rc = max3107_read_regs(dev, MAX3107_REG_TXFIFOLVL, &dev->regs.fifo.tx_fifo_lvl, 1);
+        fifo_space = 128 - dev->regs.fifo.tx_fifo_lvl;
+    }
+    if (rc == 0) {
+        size = min(size, fifo_space);
+        if (size) {
+            rc = max3107_write_fifo(dev, buf, size);
+            dev->regs.fifo.tx_fifo_lvl += size;
+            dev->writable_notified = false;
+        }
+        if (rc == 0) {
+            rc = (int)size;
+        }
+    }
+
+    return rc;
+}
+
+static void
+max3107_isr_cb(struct max3107_dev *dev)
+{
+    int rc;
+    int sr;
+    bool read_irq;
+    uint8_t isr = dev->regs.ints.isr;
+
+    OS_ENTER_CRITICAL(sr);
+    read_irq = dev->irq_pending;
+    dev->irq_pending = false;
+    OS_EXIT_CRITICAL(sr);
+
+    if (read_irq) {
+        /* Read ISR, LSR, (and also LSRIntEn which is between them) */
+        rc = max3107_read_regs(dev, MAX3107_REG_ISR, &dev->regs.ints.isr, 3);
+        if (rc) {
+            dev->irq_pending = true;
+            goto end;
+        } else {
+            /*
+             * In usual case reading ISR clears interrupt status bits on the device.
+             * However it's possible that interrupt raised during read of interrupt registers.
+             * In that case irq pin will remain low. MCU interrupt is set to be triggered on
+             * falling edge, read irq pin, if it stays low, new interrupt just arrived.
+             */
+            dev->irq_pending = hal_gpio_read(dev->cfg.irq_pin) == 0;
+        }
+    }
+
+    if (dev->regs.ints.lsr & LSR_RXBREAK) {
+        if (dev->client && dev->client->break_detected) {
+            dev->client->break_detected(dev);
+        }
+    }
+    if (dev->regs.ints.lsr & LSR_RXOVERRUN) {
+        if (dev->client && dev->client->error) {
+            dev->client->error(dev);
+        }
+    }
+    if (dev->regs.ints.lsr & LSR_FRAMEERR) {
+        if (dev->client && dev->client->error) {
+            dev->client->error(dev);
+        }
+    }
+    /* TX Empty interrupt, no need to read TX FIFO level */
+    if (dev->regs.ints.isr & ISR_TXEMPTYINT) {
+        dev->regs.fifo.tx_fifo_lvl = 0;
+    } else if (0 != (isr & ISR_TFIFOTRIGINT) &&
+               0 == (dev->regs.ints.isr & ISR_TFIFOTRIGINT) &&
+               (dev->regs.fifo.fifo_trg_lvl & FIFOTRGLVL_TXTRIG) < (dev->regs.fifo.tx_fifo_lvl >> 3)) {
+        dev->regs.fifo.tx_fifo_lvl = (dev->regs.fifo.fifo_trg_lvl & FIFOTRGLVL_TXTRIG) << 3;
+    }
+    /* RX Empty interrupt, no need to read RX FIFO level */
+    if (dev->regs.ints.isr & ISR_RXEMPTYINT) {
+        dev->regs.fifo.rx_fifo_lvl = 0;
+        dev->rx_fifo_emptied = false;
+    }
+    /* RX Trigger level interrupt, there are some bytes in FIFO, RX FIFO level read can be skipped */
+    if (dev->regs.ints.isr & ISR_RFIFOTRIGINT) {
+        if (dev->regs.fifo.rx_fifo_lvl < ((dev->regs.fifo.fifo_trg_lvl & FIFOTRGLVL_RXTRIG) >> 1)) {
+            dev->regs.fifo.rx_fifo_lvl = (dev->regs.fifo.fifo_trg_lvl & FIFOTRGLVL_RXTRIG) >> 1;
+        }
+    }
+    /*
+     * Read both FIFO levels when:
+     * - RX timeout interrupt arrived, more RX related interrupts may never come
+     * - TX FIFO level as went above half the size FIFO, if transmission is ongoing
+     *   tx_fifi_lvl is not updated by software so periodically read TX FIFO level
+     *   instead waiting for FIFO empty interrupt.
+     * - All know data from RX FIFO was read, there may be more bytes that arrived
+     *   after FIFO level was checked, to avoid data overwrite (or timeout with
+     *   flow control enabled) check FIFO level now.
+     */
+    if ((dev->regs.ints.lsr & LSR_RTIMEOUT) ||
+        (dev->regs.fifo.tx_fifo_lvl > 64) ||
+        dev->rx_fifo_emptied) {
+        rc = max3107_read_regs(dev, MAX3107_REG_TXFIFOLVL, &dev->regs.fifo.tx_fifo_lvl, 2);
+        if (rc == 0) {
+            dev->rx_fifo_emptied = false;
+        }
+    }
+    while (dev->regs.fifo.rx_fifo_lvl > 0 && dev->client && !dev->readable_notified) {
+        dev->readable_notified = true;
+        dev->client->readable(dev);
+    }
+    if (dev->regs.fifo.tx_fifo_lvl < 128) {
+        if (dev->client && !dev->writable_notified) {
+            dev->writable_notified = true;
+            dev->client->writable(dev);
+        }
+    }
+end:
+    if (dev->irq_pending) {
+        os_eventq_put(dev->event_queue, &dev->irq_event);
+    }
+}
+
+static void
+max3107_disable_rx_int(struct max3107_dev *dev)
+{
+    max3107_irqen_clear(dev, IRQEN_RXTRGIEN | IRQEN_LSRERRIEN);
+}
+
+static void
+max3107_enable_rx_int(struct max3107_dev *dev)
+{
+    max3107_irqen_set(dev, IRQEN_RXTRGIEN | IRQEN_LSRERRIEN);

Review comment:
       Handle error here.

##########
File path: hw/drivers/uart/max3107/src/max3107.c
##########
@@ -0,0 +1,1059 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 <assert.h>
+#include <string.h>
+
+#include <os/mynewt.h>
+#include <bsp/bsp.h>
+#include <hal/hal_gpio.h>
+#include <hal/hal_uart.h>
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+#include <bus/drivers/spi_common.h>
+#endif
+#include <max3107/max3107.h>
+#include "max3107_priv.h"
+
+#include <uart/uart.h>
+
+static inline int
+max3107_lock(struct max3107_dev *dev)
+{
+    return os_error_to_sys(os_mutex_pend(&dev->lock,
+                                         os_time_ms_to_ticks32(MYNEWT_VAL(MAX3107_LOCK_TIMEOUT))));
+}
+
+static inline void
+max3107_unlock(struct max3107_dev *dev)
+{
+    int rc = os_mutex_release(&dev->lock);
+    assert(rc == 0);
+}
+
+void
+max3107_cs_activate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = (struct bus_spi_node *)&dev->dev;
+    hal_gpio_write(node->pin_cs, 0);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 0);
+#endif
+}
+
+void
+max3107_cs_deactivate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = &dev->dev;
+    hal_gpio_write(node->pin_cs, 1);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 1);
+#endif
+}
+
+int
+max3107_read_regs(struct max3107_dev *dev, uint8_t addr, void *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+#if !MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    uint8_t fast_buf[8];
+#endif
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;
+
+    if (size > 0) {
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+        rc = bus_node_simple_write_read_transact((struct os_dev *)&dev->dev,
+                                                 &addr, 1, buf, size);
+#else
+        max3107_cs_activate(dev);
+
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memset(fast_buf + 1, 0xFF, size);
+            /* Send command + address */
+            rc = hal_spi_txrx(dev->cfg.spi_num, fast_buf, fast_buf, size + 1);
+            if (rc == 0) {
+                memcpy(buf, fast_buf + 1, size);
+            }
+        } else {
+            /* Send command + address */
+            rc = hal_spi_txrx(dev->cfg.spi_num, &addr, NULL, 1);
+            if (rc == 0) {
+                /* For security mostly, do not output random data, fill it with FF */
+                memset(buf, 0xFF, size);
+                /* Tx buf does not matter, for simplicity pass read buffer */
+                rc = hal_spi_txrx(dev->cfg.spi_num, buf, buf, (int)size);
+            }
+        }
+
+        max3107_cs_deactivate(dev);
+#endif
+    }
+
+    if (locked) {
+        max3107_unlock(dev);
+    }
+
+    return rc;
+}
+
+int
+max3107_write_regs(struct max3107_dev *dev, uint8_t addr, const uint8_t *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+    uint8_t fast_buf[17];
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;
+
+    if (size) {
+        /* For writes set MSB */
+        addr |= 0x80;
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+        rc = bus_node_lock((struct os_dev *)&dev->dev,
+                           BUS_NODE_LOCK_DEFAULT_TIMEOUT);
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memcpy(fast_buf + 1, buf, size);
+            rc = bus_node_simple_write((struct os_dev *)&dev->dev, fast_buf, size + 1);
+        } else {
+            if (rc == 0) {
+                rc = bus_node_write((struct os_dev *)&dev->dev,
+                                    &addr, 1,
+                                    BUS_NODE_LOCK_DEFAULT_TIMEOUT, BUS_F_NOSTOP);
+                if (rc == 0) {
+                    rc = bus_node_simple_write((struct os_dev *)&dev->dev, buf, size);
+                }
+            }
+        }
+        (void)bus_node_unlock((struct os_dev *)&dev->dev);
+#else
+        max3107_cs_activate(dev);
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memcpy(fast_buf + 1, buf, size);
+            rc = hal_spi_txrx(dev->cfg.spi_num, fast_buf, NULL, size + 1);
+        } else {
+            rc = hal_spi_txrx(dev->cfg.spi_num, &addr, NULL, 1);
+            if (rc == 0) {
+                rc = hal_spi_txrx(dev->cfg.spi_num, (void *)buf, NULL, (int)size);
+            }
+        }
+        max3107_cs_deactivate(dev);
+#endif
+    }
+    if (locked) {
+        max3107_unlock(dev);
+    }
+
+    return rc;
+}
+
+static int
+max3107_write_reg(struct max3107_dev *dev, uint8_t addr, uint8_t val)
+{
+    return max3107_write_regs(dev, addr, &val, 1);
+}
+
+static int
+max3107_write_fifo(struct max3107_dev *dev, const uint8_t *buf, uint32_t size)
+{
+    return max3107_write_regs(dev, 0, buf, size);
+}
+
+int
+max3107_read_fifo(struct max3107_dev *dev, uint8_t *buf, uint32_t size)
+{
+    return max3107_read_regs(dev, 0, buf, size);
+}
+
+static const uint8_t factors[5] = { 1, 6, 48, 96, 144 };
+/* "F PLL in" from datasheet: Table 4. PLLFactor[1:0] Selection Guide */
+static const uint32_t fpllin_min[5] = { 1, 500000,  850000,  425000, 390000 };
+static const uint32_t fpllin_max[5] = { 1, 800000, 1200000, 1000000, 666666 };
+
+static uint32_t
+max3107_compute_clock_config(uint32_t clockf, uint32_t br, struct max3107_clock_cfg *cfg)
+{
+    uint32_t div;
+    uint32_t pre_div;
+    uint32_t pre_div_min;
+    uint32_t pre_div_max;
+    uint32_t mul;
+    uint32_t actual_br;
+    uint32_t best_br = 1;
+    uint32_t fref;
+    int factor_ix;
+    int max_factor = (cfg->clk_source & CLCSOURCE_PLLBYPASS) ? 1 : 5;
+    uint32_t mode_mul = (cfg->brg_config & BRGCONFIG_4XMODE) ? 4 : (cfg->brg_config & BRGCONFIG_2XMODE) ? 2 : 1;
+
+    /* Clock will be needed. */
+    cfg->clk_source |= CLCSOURCE_CLOCKEN;
+
+    /* If bypass was not disable at the start try finding more accurate clock settings. */
+    for (factor_ix = 0; factor_ix < max_factor; ++factor_ix) {
+        /*
+         * Pre divider does not apply when PLL is not used. Set lower and upper
+         * limits to 1 to have same code for PLL/non PLL.
+         */
+        if (factor_ix == 0) {
+            pre_div_min = 1;
+            pre_div_max = 1;
+        } else {
+            /* Lower and upper frequency limits used to get pre divider range */
+            pre_div_max = clockf / fpllin_min[factor_ix];
+            pre_div_min = (clockf + fpllin_max[factor_ix] - 1) / fpllin_max[factor_ix];
+            /* Make sure divider is in correct range. */
+            pre_div_min = min(63, pre_div_min);
+            pre_div_min = max(1, pre_div_min);
+            pre_div_max = min(63, pre_div_max);
+            pre_div_max = max(1, pre_div_max);
+        }
+        /* Loop for x1, x2 and x4 modes. */
+        for (mul = 1; mul <= mode_mul; mul <<= 1) {
+            for (pre_div = pre_div_min; pre_div <= pre_div_max; ++pre_div) {
+                fref = (clockf / pre_div) * factors[factor_ix];
+                div = (fref * mul + (br / 2)) / br;
+                div = max(div, 16);
+
+                actual_br = mul * fref / div;
+                if (abs((int)(actual_br - br)) < abs((int)(best_br - br))) {
+                    best_br = actual_br;
+                    cfg->div_lsb = (uint8_t)(div >> 4);
+                    cfg->div_msb = (uint8_t)(div >> 12);
+                    cfg->brg_config = (div & 0xF) | ((mul == 4) ? 0x20 : ((mul == 2) ? 0x10 : 0));
+                    /* If best choice is factor_ix == 0, no need for PLL */
+                    if (factor_ix == 0) {
+                        cfg->clk_source |= CLCSOURCE_PLLBYPASS;
+                        cfg->clk_source &= ~CLCSOURCE_PLLEN;
+                    } else {
+                        cfg->pll_config = pre_div | ((factor_ix - 1) << 6);
+                        cfg->clk_source &= ~CLCSOURCE_PLLBYPASS;
+                        cfg->clk_source |= CLCSOURCE_PLLEN;
+                    }
+                }
+            }
+        }
+    }
+
+    return best_br;
+}
+
+int
+max3107_config_uart(struct max3107_dev *dev, const struct uart_conf_port *conf)
+{
+    int rc;
+
+    if (dev->cfg.crystal_en) {
+        dev->regs.clock.clk_source |= CLCSOURCE_CRYSTALEN;
+    } else {
+        dev->regs.clock.clk_source &= ~CLCSOURCE_CRYSTALEN;
+    }
+    if (dev->cfg.no_pll) {
+        dev->regs.clock.clk_source |= CLCSOURCE_PLLBYPASS;
+    } else {
+        dev->regs.clock.clk_source &= ~CLCSOURCE_PLLBYPASS;
+    }
+    if (dev->cfg.allow_mul_4) {
+        dev->regs.clock.brg_config = BRGCONFIG_4XMODE;
+    } else if (dev->cfg.allow_mul_2) {
+        dev->regs.clock.brg_config = BRGCONFIG_2XMODE;
+    } else {
+        dev->regs.clock.brg_config = 0;
+    }
+    max3107_compute_clock_config(dev->cfg.osc_freq, conf->uc_speed, &dev->regs.clock);
+
+    rc = max3107_write_regs(dev, MAX3107_REG_PLLCONFIG, &dev->regs.clock.pll_config, 5);
+    if (rc) {
+        goto end;
+    }
+
+    dev->regs.modes.mode1 = MODE1_IRQSEL | (conf->uc_flow_ctl ? 0 : MODE1_RTSHIZ);
+    dev->regs.modes.mode2 = 0;
+    dev->regs.modes.lcr = (dev->uart.ud_conf_port.uc_stopbits == 1 ? 0 : LCR_STOPBITS) |
+                          (dev->uart.ud_conf_port.uc_databits - 5) |
+                          ((dev->uart.ud_conf_port.uc_parity != UART_PARITY_NONE) ? LCR_PARITYEN : 0 ) |
+                          ((dev->uart.ud_conf_port.uc_parity == UART_PARITY_EVEN) ? LCR_EVENPARITY : 0);
+    dev->regs.modes.rxtimeout = 2;
+    dev->regs.modes.hdplxdelay = 0;
+    dev->regs.modes.irda = 0;
+
+    rc = max3107_write_regs(dev, MAX3107_REG_MODE1, &dev->regs.modes.mode1, 6);
+    if (rc) {
+        goto end;
+    }
+
+    /* RTS activated at FIFO level 120. */
+    dev->regs.fifo.flow_lvl = 0xFF;
+    dev->regs.fifo.fifo_trg_lvl = 0x38;
+    rc = max3107_write_regs(dev, MAX3107_REG_FLOWLVL, &dev->regs.fifo.flow_lvl, 2);
+    if (rc) {
+        goto end;
+    }
+
+    dev->regs.flow.flow_ctrl = conf->uc_flow_ctl ? (FLOWCTRL_AUTOCTS | FLOWCTRL_AUTORTS) : 0;
+    max3107_write_reg(dev, MAX3107_REG_FLOWCTRL, dev->regs.flow.flow_ctrl);
+
+    dev->regs.ints.irq_en = IRQEN_LSRERRIEN | IRQEN_RXTRGIEN;
+    dev->regs.ints.lsr_int_en = LSRINTEN_FRAMEERRIEN | LSRINTEN_PARITYIEN |
+                                LSRINTEN_RBREAKIEN | LSRINTEN_ROVERRIEN | LSRINTEN_RTIMEOUTIEN;
+    rc = max3107_write_regs(dev, MAX3107_REG_IRQEN, &dev->regs.ints.irq_en, 3);
+    assert(dev->regs.ints.irq_en & IRQEN_LSRERRIEN);
+end:
+    return rc;
+}
+
+int
+max3107_rx_available(struct max3107_dev *dev)
+{
+    int rc = 0;
+
+    if (dev->regs.fifo.rx_fifo_lvl == 0) {
+        rc = max3107_read_regs(dev, MAX3107_REG_RXFIFOLVL, &dev->regs.fifo.rx_fifo_lvl, 1);
+    }
+    if (rc == 0) {
+        rc = dev->regs.fifo.rx_fifo_lvl;
+    }
+
+    return rc;
+}
+
+int
+max3107_tx_available(struct max3107_dev *dev)
+{
+    int rc = 0;
+
+    if (dev->regs.fifo.tx_fifo_lvl >= 128) {
+        rc = max3107_read_regs(dev, MAX3107_REG_TXFIFOLVL, &dev->regs.fifo.tx_fifo_lvl, 1);
+    }
+    if (rc == 0) {
+        rc = 128 - dev->regs.fifo.tx_fifo_lvl;
+    }
+
+    return rc;
+}
+
+static int
+max3107_irqen_set(struct max3107_dev *dev, uint8_t enabled_bits)
+{
+    int rc = 0;
+
+    uint8_t irq_en = dev->regs.ints.irq_en | enabled_bits;
+
+    if (irq_en != dev->regs.ints.irq_en) {
+        dev->regs.ints.irq_en = irq_en;
+        rc = max3107_write_reg(dev, MAX3107_REG_IRQEN, dev->regs.ints.irq_en);
+    }
+
+    return rc;
+}
+
+static int
+max3107_irqen_clear(struct max3107_dev *dev, uint8_t cleared_bits)
+{
+    int rc = 0;
+
+    uint8_t irq_en = dev->regs.ints.irq_en & ~cleared_bits;
+
+    if (irq_en != dev->regs.ints.irq_en) {
+        dev->regs.ints.irq_en = irq_en;
+        rc = max3107_write_reg(dev, MAX3107_REG_IRQEN, dev->regs.ints.irq_en);
+    }
+
+    return rc;
+}
+
+int
+max3107_read(struct max3107_dev *dev, void *buf, size_t size)
+{
+    int rc = 0;
+
+    if (dev->regs.fifo.rx_fifo_lvl == 0) {
+        /* Read number of bytes currently in RX FIFO first. */
+        rc = max3107_read_regs(dev, MAX3107_REG_RXFIFOLVL, &dev->regs.fifo.rx_fifo_lvl, 1);
+    }
+
+    if (rc == 0) {
+        size = min(dev->regs.fifo.rx_fifo_lvl, size);
+        if (size > 0) {
+            rc = max3107_read_fifo(dev, buf, size);
+        }
+        if (rc == 0) {
+            dev->readable_notified = false;
+            /* Update FIFO level for potential future use. */
+            dev->regs.fifo.rx_fifo_lvl -= size;
+            /*
+             * If FIFO was possible emptied, read FIFO level in next interrupt,
+             * in case data was coming while FIFO was being read.
+             */
+            dev->rx_fifo_emptied = dev->regs.fifo.rx_fifo_lvl == 0;
+            rc = (int)size;
+        }
+    }
+    return rc;
+}
+
+int
+max3107_write(struct max3107_dev *dev, const void *buf, size_t size)
+{
+    size_t fifo_space = 128 - dev->regs.fifo.tx_fifo_lvl;
+    int rc = 0;
+
+    /*
+     * If FIFO level was read before and there is enough data, there is
+     * no need to check FIFO level again.
+     */
+    if (size > fifo_space) {
+        /* Read number of bytes currently in TX FIFO */
+        rc = max3107_read_regs(dev, MAX3107_REG_TXFIFOLVL, &dev->regs.fifo.tx_fifo_lvl, 1);
+        fifo_space = 128 - dev->regs.fifo.tx_fifo_lvl;
+    }
+    if (rc == 0) {
+        size = min(size, fifo_space);
+        if (size) {
+            rc = max3107_write_fifo(dev, buf, size);
+            dev->regs.fifo.tx_fifo_lvl += size;
+            dev->writable_notified = false;
+        }
+        if (rc == 0) {
+            rc = (int)size;
+        }
+    }
+
+    return rc;
+}
+
+static void
+max3107_isr_cb(struct max3107_dev *dev)
+{
+    int rc;
+    int sr;
+    bool read_irq;
+    uint8_t isr = dev->regs.ints.isr;
+
+    OS_ENTER_CRITICAL(sr);
+    read_irq = dev->irq_pending;
+    dev->irq_pending = false;
+    OS_EXIT_CRITICAL(sr);
+
+    if (read_irq) {
+        /* Read ISR, LSR, (and also LSRIntEn which is between them) */
+        rc = max3107_read_regs(dev, MAX3107_REG_ISR, &dev->regs.ints.isr, 3);
+        if (rc) {
+            dev->irq_pending = true;
+            goto end;
+        } else {
+            /*
+             * In usual case reading ISR clears interrupt status bits on the device.
+             * However it's possible that interrupt raised during read of interrupt registers.
+             * In that case irq pin will remain low. MCU interrupt is set to be triggered on
+             * falling edge, read irq pin, if it stays low, new interrupt just arrived.
+             */
+            dev->irq_pending = hal_gpio_read(dev->cfg.irq_pin) == 0;
+        }
+    }
+
+    if (dev->regs.ints.lsr & LSR_RXBREAK) {
+        if (dev->client && dev->client->break_detected) {
+            dev->client->break_detected(dev);
+        }
+    }
+    if (dev->regs.ints.lsr & LSR_RXOVERRUN) {
+        if (dev->client && dev->client->error) {
+            dev->client->error(dev);
+        }
+    }
+    if (dev->regs.ints.lsr & LSR_FRAMEERR) {
+        if (dev->client && dev->client->error) {
+            dev->client->error(dev);
+        }
+    }
+    /* TX Empty interrupt, no need to read TX FIFO level */
+    if (dev->regs.ints.isr & ISR_TXEMPTYINT) {
+        dev->regs.fifo.tx_fifo_lvl = 0;
+    } else if (0 != (isr & ISR_TFIFOTRIGINT) &&
+               0 == (dev->regs.ints.isr & ISR_TFIFOTRIGINT) &&
+               (dev->regs.fifo.fifo_trg_lvl & FIFOTRGLVL_TXTRIG) < (dev->regs.fifo.tx_fifo_lvl >> 3)) {
+        dev->regs.fifo.tx_fifo_lvl = (dev->regs.fifo.fifo_trg_lvl & FIFOTRGLVL_TXTRIG) << 3;
+    }
+    /* RX Empty interrupt, no need to read RX FIFO level */
+    if (dev->regs.ints.isr & ISR_RXEMPTYINT) {
+        dev->regs.fifo.rx_fifo_lvl = 0;
+        dev->rx_fifo_emptied = false;
+    }
+    /* RX Trigger level interrupt, there are some bytes in FIFO, RX FIFO level read can be skipped */
+    if (dev->regs.ints.isr & ISR_RFIFOTRIGINT) {
+        if (dev->regs.fifo.rx_fifo_lvl < ((dev->regs.fifo.fifo_trg_lvl & FIFOTRGLVL_RXTRIG) >> 1)) {
+            dev->regs.fifo.rx_fifo_lvl = (dev->regs.fifo.fifo_trg_lvl & FIFOTRGLVL_RXTRIG) >> 1;
+        }
+    }
+    /*
+     * Read both FIFO levels when:
+     * - RX timeout interrupt arrived, more RX related interrupts may never come
+     * - TX FIFO level as went above half the size FIFO, if transmission is ongoing
+     *   tx_fifi_lvl is not updated by software so periodically read TX FIFO level
+     *   instead waiting for FIFO empty interrupt.
+     * - All know data from RX FIFO was read, there may be more bytes that arrived
+     *   after FIFO level was checked, to avoid data overwrite (or timeout with
+     *   flow control enabled) check FIFO level now.
+     */
+    if ((dev->regs.ints.lsr & LSR_RTIMEOUT) ||
+        (dev->regs.fifo.tx_fifo_lvl > 64) ||
+        dev->rx_fifo_emptied) {
+        rc = max3107_read_regs(dev, MAX3107_REG_TXFIFOLVL, &dev->regs.fifo.tx_fifo_lvl, 2);
+        if (rc == 0) {
+            dev->rx_fifo_emptied = false;
+        }
+    }
+    while (dev->regs.fifo.rx_fifo_lvl > 0 && dev->client && !dev->readable_notified) {
+        dev->readable_notified = true;
+        dev->client->readable(dev);
+    }
+    if (dev->regs.fifo.tx_fifo_lvl < 128) {
+        if (dev->client && !dev->writable_notified) {
+            dev->writable_notified = true;
+            dev->client->writable(dev);
+        }
+    }
+end:
+    if (dev->irq_pending) {
+        os_eventq_put(dev->event_queue, &dev->irq_event);
+    }
+}
+
+static void
+max3107_disable_rx_int(struct max3107_dev *dev)
+{
+    max3107_irqen_clear(dev, IRQEN_RXTRGIEN | IRQEN_LSRERRIEN);
+}
+
+static void
+max3107_enable_rx_int(struct max3107_dev *dev)
+{
+    max3107_irqen_set(dev, IRQEN_RXTRGIEN | IRQEN_LSRERRIEN);
+}
+
+static void
+max3107_disable_tx_int(struct max3107_dev *dev)
+{
+    max3107_irqen_clear(dev, IRQEN_TXEMTYIEN | IRQEN_TXTRGIEN);

Review comment:
       Handle error here.

##########
File path: hw/drivers/uart/max3107/src/max3107.c
##########
@@ -0,0 +1,1059 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you 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 <assert.h>
+#include <string.h>
+
+#include <os/mynewt.h>
+#include <bsp/bsp.h>
+#include <hal/hal_gpio.h>
+#include <hal/hal_uart.h>
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+#include <bus/drivers/spi_common.h>
+#endif
+#include <max3107/max3107.h>
+#include "max3107_priv.h"
+
+#include <uart/uart.h>
+
+static inline int
+max3107_lock(struct max3107_dev *dev)
+{
+    return os_error_to_sys(os_mutex_pend(&dev->lock,
+                                         os_time_ms_to_ticks32(MYNEWT_VAL(MAX3107_LOCK_TIMEOUT))));
+}
+
+static inline void
+max3107_unlock(struct max3107_dev *dev)
+{
+    int rc = os_mutex_release(&dev->lock);
+    assert(rc == 0);
+}
+
+void
+max3107_cs_activate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = (struct bus_spi_node *)&dev->dev;
+    hal_gpio_write(node->pin_cs, 0);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 0);
+#endif
+}
+
+void
+max3107_cs_deactivate(struct max3107_dev *dev)
+{
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    struct bus_spi_node *node = &dev->dev;
+    hal_gpio_write(node->pin_cs, 1);
+#else
+    hal_gpio_write(dev->cfg.ss_pin, 1);
+#endif
+}
+
+int
+max3107_read_regs(struct max3107_dev *dev, uint8_t addr, void *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+#if !MYNEWT_VAL(BUS_DRIVER_PRESENT)
+    uint8_t fast_buf[8];
+#endif
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;
+
+    if (size > 0) {
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+        rc = bus_node_simple_write_read_transact((struct os_dev *)&dev->dev,
+                                                 &addr, 1, buf, size);
+#else
+        max3107_cs_activate(dev);
+
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memset(fast_buf + 1, 0xFF, size);
+            /* Send command + address */
+            rc = hal_spi_txrx(dev->cfg.spi_num, fast_buf, fast_buf, size + 1);
+            if (rc == 0) {
+                memcpy(buf, fast_buf + 1, size);
+            }
+        } else {
+            /* Send command + address */
+            rc = hal_spi_txrx(dev->cfg.spi_num, &addr, NULL, 1);
+            if (rc == 0) {
+                /* For security mostly, do not output random data, fill it with FF */
+                memset(buf, 0xFF, size);
+                /* Tx buf does not matter, for simplicity pass read buffer */
+                rc = hal_spi_txrx(dev->cfg.spi_num, buf, buf, (int)size);
+            }
+        }
+
+        max3107_cs_deactivate(dev);
+#endif
+    }
+
+    if (locked) {
+        max3107_unlock(dev);
+    }
+
+    return rc;
+}
+
+int
+max3107_write_regs(struct max3107_dev *dev, uint8_t addr, const uint8_t *buf, uint32_t size)
+{
+    int rc;
+    bool locked;
+    uint8_t fast_buf[17];
+
+    rc = max3107_lock(dev);
+    locked = rc == 0;
+
+    if (size) {
+        /* For writes set MSB */
+        addr |= 0x80;
+
+#if MYNEWT_VAL(BUS_DRIVER_PRESENT)
+        rc = bus_node_lock((struct os_dev *)&dev->dev,
+                           BUS_NODE_LOCK_DEFAULT_TIMEOUT);
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memcpy(fast_buf + 1, buf, size);
+            rc = bus_node_simple_write((struct os_dev *)&dev->dev, fast_buf, size + 1);
+        } else {
+            if (rc == 0) {
+                rc = bus_node_write((struct os_dev *)&dev->dev,
+                                    &addr, 1,
+                                    BUS_NODE_LOCK_DEFAULT_TIMEOUT, BUS_F_NOSTOP);
+                if (rc == 0) {
+                    rc = bus_node_simple_write((struct os_dev *)&dev->dev, buf, size);
+                }
+            }
+        }
+        (void)bus_node_unlock((struct os_dev *)&dev->dev);
+#else
+        max3107_cs_activate(dev);
+        if (size < sizeof(fast_buf)) {
+            fast_buf[0] = addr;
+            memcpy(fast_buf + 1, buf, size);
+            rc = hal_spi_txrx(dev->cfg.spi_num, fast_buf, NULL, size + 1);
+        } else {
+            rc = hal_spi_txrx(dev->cfg.spi_num, &addr, NULL, 1);
+            if (rc == 0) {
+                rc = hal_spi_txrx(dev->cfg.spi_num, (void *)buf, NULL, (int)size);
+            }
+        }
+        max3107_cs_deactivate(dev);
+#endif
+    }
+    if (locked) {
+        max3107_unlock(dev);
+    }
+
+    return rc;
+}
+
+static int
+max3107_write_reg(struct max3107_dev *dev, uint8_t addr, uint8_t val)
+{
+    return max3107_write_regs(dev, addr, &val, 1);
+}
+
+static int
+max3107_write_fifo(struct max3107_dev *dev, const uint8_t *buf, uint32_t size)
+{
+    return max3107_write_regs(dev, 0, buf, size);
+}
+
+int
+max3107_read_fifo(struct max3107_dev *dev, uint8_t *buf, uint32_t size)
+{
+    return max3107_read_regs(dev, 0, buf, size);
+}
+
+static const uint8_t factors[5] = { 1, 6, 48, 96, 144 };
+/* "F PLL in" from datasheet: Table 4. PLLFactor[1:0] Selection Guide */
+static const uint32_t fpllin_min[5] = { 1, 500000,  850000,  425000, 390000 };
+static const uint32_t fpllin_max[5] = { 1, 800000, 1200000, 1000000, 666666 };
+
+static uint32_t
+max3107_compute_clock_config(uint32_t clockf, uint32_t br, struct max3107_clock_cfg *cfg)
+{
+    uint32_t div;
+    uint32_t pre_div;
+    uint32_t pre_div_min;
+    uint32_t pre_div_max;
+    uint32_t mul;
+    uint32_t actual_br;
+    uint32_t best_br = 1;
+    uint32_t fref;
+    int factor_ix;
+    int max_factor = (cfg->clk_source & CLCSOURCE_PLLBYPASS) ? 1 : 5;
+    uint32_t mode_mul = (cfg->brg_config & BRGCONFIG_4XMODE) ? 4 : (cfg->brg_config & BRGCONFIG_2XMODE) ? 2 : 1;
+
+    /* Clock will be needed. */
+    cfg->clk_source |= CLCSOURCE_CLOCKEN;
+
+    /* If bypass was not disable at the start try finding more accurate clock settings. */
+    for (factor_ix = 0; factor_ix < max_factor; ++factor_ix) {
+        /*
+         * Pre divider does not apply when PLL is not used. Set lower and upper
+         * limits to 1 to have same code for PLL/non PLL.
+         */
+        if (factor_ix == 0) {
+            pre_div_min = 1;
+            pre_div_max = 1;
+        } else {
+            /* Lower and upper frequency limits used to get pre divider range */
+            pre_div_max = clockf / fpllin_min[factor_ix];
+            pre_div_min = (clockf + fpllin_max[factor_ix] - 1) / fpllin_max[factor_ix];
+            /* Make sure divider is in correct range. */
+            pre_div_min = min(63, pre_div_min);
+            pre_div_min = max(1, pre_div_min);
+            pre_div_max = min(63, pre_div_max);
+            pre_div_max = max(1, pre_div_max);
+        }
+        /* Loop for x1, x2 and x4 modes. */
+        for (mul = 1; mul <= mode_mul; mul <<= 1) {
+            for (pre_div = pre_div_min; pre_div <= pre_div_max; ++pre_div) {
+                fref = (clockf / pre_div) * factors[factor_ix];
+                div = (fref * mul + (br / 2)) / br;
+                div = max(div, 16);
+
+                actual_br = mul * fref / div;
+                if (abs((int)(actual_br - br)) < abs((int)(best_br - br))) {
+                    best_br = actual_br;
+                    cfg->div_lsb = (uint8_t)(div >> 4);
+                    cfg->div_msb = (uint8_t)(div >> 12);
+                    cfg->brg_config = (div & 0xF) | ((mul == 4) ? 0x20 : ((mul == 2) ? 0x10 : 0));
+                    /* If best choice is factor_ix == 0, no need for PLL */
+                    if (factor_ix == 0) {
+                        cfg->clk_source |= CLCSOURCE_PLLBYPASS;
+                        cfg->clk_source &= ~CLCSOURCE_PLLEN;
+                    } else {
+                        cfg->pll_config = pre_div | ((factor_ix - 1) << 6);
+                        cfg->clk_source &= ~CLCSOURCE_PLLBYPASS;
+                        cfg->clk_source |= CLCSOURCE_PLLEN;
+                    }
+                }
+            }
+        }
+    }
+
+    return best_br;
+}
+
+int
+max3107_config_uart(struct max3107_dev *dev, const struct uart_conf_port *conf)
+{
+    int rc;
+
+    if (dev->cfg.crystal_en) {
+        dev->regs.clock.clk_source |= CLCSOURCE_CRYSTALEN;
+    } else {
+        dev->regs.clock.clk_source &= ~CLCSOURCE_CRYSTALEN;
+    }
+    if (dev->cfg.no_pll) {
+        dev->regs.clock.clk_source |= CLCSOURCE_PLLBYPASS;
+    } else {
+        dev->regs.clock.clk_source &= ~CLCSOURCE_PLLBYPASS;
+    }
+    if (dev->cfg.allow_mul_4) {
+        dev->regs.clock.brg_config = BRGCONFIG_4XMODE;
+    } else if (dev->cfg.allow_mul_2) {
+        dev->regs.clock.brg_config = BRGCONFIG_2XMODE;
+    } else {
+        dev->regs.clock.brg_config = 0;
+    }
+    max3107_compute_clock_config(dev->cfg.osc_freq, conf->uc_speed, &dev->regs.clock);
+
+    rc = max3107_write_regs(dev, MAX3107_REG_PLLCONFIG, &dev->regs.clock.pll_config, 5);
+    if (rc) {
+        goto end;
+    }
+
+    dev->regs.modes.mode1 = MODE1_IRQSEL | (conf->uc_flow_ctl ? 0 : MODE1_RTSHIZ);
+    dev->regs.modes.mode2 = 0;
+    dev->regs.modes.lcr = (dev->uart.ud_conf_port.uc_stopbits == 1 ? 0 : LCR_STOPBITS) |
+                          (dev->uart.ud_conf_port.uc_databits - 5) |
+                          ((dev->uart.ud_conf_port.uc_parity != UART_PARITY_NONE) ? LCR_PARITYEN : 0 ) |
+                          ((dev->uart.ud_conf_port.uc_parity == UART_PARITY_EVEN) ? LCR_EVENPARITY : 0);
+    dev->regs.modes.rxtimeout = 2;
+    dev->regs.modes.hdplxdelay = 0;
+    dev->regs.modes.irda = 0;
+
+    rc = max3107_write_regs(dev, MAX3107_REG_MODE1, &dev->regs.modes.mode1, 6);
+    if (rc) {
+        goto end;
+    }
+
+    /* RTS activated at FIFO level 120. */
+    dev->regs.fifo.flow_lvl = 0xFF;
+    dev->regs.fifo.fifo_trg_lvl = 0x38;
+    rc = max3107_write_regs(dev, MAX3107_REG_FLOWLVL, &dev->regs.fifo.flow_lvl, 2);
+    if (rc) {
+        goto end;
+    }
+
+    dev->regs.flow.flow_ctrl = conf->uc_flow_ctl ? (FLOWCTRL_AUTOCTS | FLOWCTRL_AUTORTS) : 0;
+    max3107_write_reg(dev, MAX3107_REG_FLOWCTRL, dev->regs.flow.flow_ctrl);
+
+    dev->regs.ints.irq_en = IRQEN_LSRERRIEN | IRQEN_RXTRGIEN;
+    dev->regs.ints.lsr_int_en = LSRINTEN_FRAMEERRIEN | LSRINTEN_PARITYIEN |
+                                LSRINTEN_RBREAKIEN | LSRINTEN_ROVERRIEN | LSRINTEN_RTIMEOUTIEN;
+    rc = max3107_write_regs(dev, MAX3107_REG_IRQEN, &dev->regs.ints.irq_en, 3);
+    assert(dev->regs.ints.irq_en & IRQEN_LSRERRIEN);
+end:
+    return rc;
+}
+
+int
+max3107_rx_available(struct max3107_dev *dev)
+{
+    int rc = 0;
+
+    if (dev->regs.fifo.rx_fifo_lvl == 0) {
+        rc = max3107_read_regs(dev, MAX3107_REG_RXFIFOLVL, &dev->regs.fifo.rx_fifo_lvl, 1);
+    }
+    if (rc == 0) {
+        rc = dev->regs.fifo.rx_fifo_lvl;
+    }
+
+    return rc;
+}
+
+int
+max3107_tx_available(struct max3107_dev *dev)
+{
+    int rc = 0;
+
+    if (dev->regs.fifo.tx_fifo_lvl >= 128) {
+        rc = max3107_read_regs(dev, MAX3107_REG_TXFIFOLVL, &dev->regs.fifo.tx_fifo_lvl, 1);
+    }
+    if (rc == 0) {
+        rc = 128 - dev->regs.fifo.tx_fifo_lvl;
+    }
+
+    return rc;
+}
+
+static int
+max3107_irqen_set(struct max3107_dev *dev, uint8_t enabled_bits)
+{
+    int rc = 0;
+
+    uint8_t irq_en = dev->regs.ints.irq_en | enabled_bits;
+
+    if (irq_en != dev->regs.ints.irq_en) {
+        dev->regs.ints.irq_en = irq_en;
+        rc = max3107_write_reg(dev, MAX3107_REG_IRQEN, dev->regs.ints.irq_en);
+    }
+
+    return rc;
+}
+
+static int
+max3107_irqen_clear(struct max3107_dev *dev, uint8_t cleared_bits)
+{
+    int rc = 0;
+
+    uint8_t irq_en = dev->regs.ints.irq_en & ~cleared_bits;
+
+    if (irq_en != dev->regs.ints.irq_en) {
+        dev->regs.ints.irq_en = irq_en;
+        rc = max3107_write_reg(dev, MAX3107_REG_IRQEN, dev->regs.ints.irq_en);
+    }
+
+    return rc;
+}
+
+int
+max3107_read(struct max3107_dev *dev, void *buf, size_t size)
+{
+    int rc = 0;
+
+    if (dev->regs.fifo.rx_fifo_lvl == 0) {
+        /* Read number of bytes currently in RX FIFO first. */
+        rc = max3107_read_regs(dev, MAX3107_REG_RXFIFOLVL, &dev->regs.fifo.rx_fifo_lvl, 1);
+    }
+
+    if (rc == 0) {
+        size = min(dev->regs.fifo.rx_fifo_lvl, size);
+        if (size > 0) {
+            rc = max3107_read_fifo(dev, buf, size);
+        }
+        if (rc == 0) {
+            dev->readable_notified = false;
+            /* Update FIFO level for potential future use. */
+            dev->regs.fifo.rx_fifo_lvl -= size;
+            /*
+             * If FIFO was possible emptied, read FIFO level in next interrupt,
+             * in case data was coming while FIFO was being read.
+             */
+            dev->rx_fifo_emptied = dev->regs.fifo.rx_fifo_lvl == 0;
+            rc = (int)size;
+        }
+    }
+    return rc;
+}
+
+int
+max3107_write(struct max3107_dev *dev, const void *buf, size_t size)
+{
+    size_t fifo_space = 128 - dev->regs.fifo.tx_fifo_lvl;
+    int rc = 0;
+
+    /*
+     * If FIFO level was read before and there is enough data, there is
+     * no need to check FIFO level again.
+     */
+    if (size > fifo_space) {
+        /* Read number of bytes currently in TX FIFO */
+        rc = max3107_read_regs(dev, MAX3107_REG_TXFIFOLVL, &dev->regs.fifo.tx_fifo_lvl, 1);
+        fifo_space = 128 - dev->regs.fifo.tx_fifo_lvl;
+    }
+    if (rc == 0) {
+        size = min(size, fifo_space);
+        if (size) {
+            rc = max3107_write_fifo(dev, buf, size);
+            dev->regs.fifo.tx_fifo_lvl += size;
+            dev->writable_notified = false;
+        }
+        if (rc == 0) {
+            rc = (int)size;
+        }
+    }
+
+    return rc;
+}
+
+static void
+max3107_isr_cb(struct max3107_dev *dev)
+{
+    int rc;
+    int sr;
+    bool read_irq;
+    uint8_t isr = dev->regs.ints.isr;
+
+    OS_ENTER_CRITICAL(sr);
+    read_irq = dev->irq_pending;
+    dev->irq_pending = false;
+    OS_EXIT_CRITICAL(sr);
+
+    if (read_irq) {
+        /* Read ISR, LSR, (and also LSRIntEn which is between them) */
+        rc = max3107_read_regs(dev, MAX3107_REG_ISR, &dev->regs.ints.isr, 3);
+        if (rc) {
+            dev->irq_pending = true;
+            goto end;
+        } else {
+            /*
+             * In usual case reading ISR clears interrupt status bits on the device.
+             * However it's possible that interrupt raised during read of interrupt registers.
+             * In that case irq pin will remain low. MCU interrupt is set to be triggered on
+             * falling edge, read irq pin, if it stays low, new interrupt just arrived.
+             */
+            dev->irq_pending = hal_gpio_read(dev->cfg.irq_pin) == 0;
+        }
+    }
+
+    if (dev->regs.ints.lsr & LSR_RXBREAK) {
+        if (dev->client && dev->client->break_detected) {
+            dev->client->break_detected(dev);
+        }
+    }
+    if (dev->regs.ints.lsr & LSR_RXOVERRUN) {
+        if (dev->client && dev->client->error) {
+            dev->client->error(dev);
+        }
+    }
+    if (dev->regs.ints.lsr & LSR_FRAMEERR) {
+        if (dev->client && dev->client->error) {
+            dev->client->error(dev);
+        }
+    }
+    /* TX Empty interrupt, no need to read TX FIFO level */
+    if (dev->regs.ints.isr & ISR_TXEMPTYINT) {
+        dev->regs.fifo.tx_fifo_lvl = 0;
+    } else if (0 != (isr & ISR_TFIFOTRIGINT) &&
+               0 == (dev->regs.ints.isr & ISR_TFIFOTRIGINT) &&
+               (dev->regs.fifo.fifo_trg_lvl & FIFOTRGLVL_TXTRIG) < (dev->regs.fifo.tx_fifo_lvl >> 3)) {
+        dev->regs.fifo.tx_fifo_lvl = (dev->regs.fifo.fifo_trg_lvl & FIFOTRGLVL_TXTRIG) << 3;
+    }
+    /* RX Empty interrupt, no need to read RX FIFO level */
+    if (dev->regs.ints.isr & ISR_RXEMPTYINT) {
+        dev->regs.fifo.rx_fifo_lvl = 0;
+        dev->rx_fifo_emptied = false;
+    }
+    /* RX Trigger level interrupt, there are some bytes in FIFO, RX FIFO level read can be skipped */
+    if (dev->regs.ints.isr & ISR_RFIFOTRIGINT) {
+        if (dev->regs.fifo.rx_fifo_lvl < ((dev->regs.fifo.fifo_trg_lvl & FIFOTRGLVL_RXTRIG) >> 1)) {
+            dev->regs.fifo.rx_fifo_lvl = (dev->regs.fifo.fifo_trg_lvl & FIFOTRGLVL_RXTRIG) >> 1;
+        }
+    }
+    /*
+     * Read both FIFO levels when:
+     * - RX timeout interrupt arrived, more RX related interrupts may never come
+     * - TX FIFO level as went above half the size FIFO, if transmission is ongoing
+     *   tx_fifi_lvl is not updated by software so periodically read TX FIFO level
+     *   instead waiting for FIFO empty interrupt.
+     * - All know data from RX FIFO was read, there may be more bytes that arrived
+     *   after FIFO level was checked, to avoid data overwrite (or timeout with
+     *   flow control enabled) check FIFO level now.
+     */
+    if ((dev->regs.ints.lsr & LSR_RTIMEOUT) ||
+        (dev->regs.fifo.tx_fifo_lvl > 64) ||
+        dev->rx_fifo_emptied) {
+        rc = max3107_read_regs(dev, MAX3107_REG_TXFIFOLVL, &dev->regs.fifo.tx_fifo_lvl, 2);
+        if (rc == 0) {
+            dev->rx_fifo_emptied = false;
+        }
+    }
+    while (dev->regs.fifo.rx_fifo_lvl > 0 && dev->client && !dev->readable_notified) {
+        dev->readable_notified = true;
+        dev->client->readable(dev);
+    }
+    if (dev->regs.fifo.tx_fifo_lvl < 128) {
+        if (dev->client && !dev->writable_notified) {
+            dev->writable_notified = true;
+            dev->client->writable(dev);
+        }
+    }
+end:
+    if (dev->irq_pending) {
+        os_eventq_put(dev->event_queue, &dev->irq_event);
+    }
+}
+
+static void
+max3107_disable_rx_int(struct max3107_dev *dev)
+{
+    max3107_irqen_clear(dev, IRQEN_RXTRGIEN | IRQEN_LSRERRIEN);
+}
+
+static void
+max3107_enable_rx_int(struct max3107_dev *dev)
+{
+    max3107_irqen_set(dev, IRQEN_RXTRGIEN | IRQEN_LSRERRIEN);
+}
+
+static void
+max3107_disable_tx_int(struct max3107_dev *dev)
+{
+    max3107_irqen_clear(dev, IRQEN_TXEMTYIEN | IRQEN_TXTRGIEN);
+}
+
+static void
+max3107_enable_tx_int(struct max3107_dev *dev)
+{
+    max3107_irqen_set(dev, IRQEN_TXEMTYIEN | IRQEN_TXTRGIEN);

Review comment:
       Handle error here.




-- 
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