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