andrzej-kaczmarek commented on a change in pull request #1507: [RFC] Add generic serial bus driver URL: https://github.com/apache/mynewt-core/pull/1507#discussion_r234258876
########## File path: hw/bus/spi/src/spi.c ########## @@ -0,0 +1,225 @@ +/* + * 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 "defs/error.h" +#include "hal/hal_gpio.h" +#include "hal/hal_spi.h" +#include "bus/bus.h" +#include "bus/bus_debug.h" +#include "bus/spi.h" + +static int +bus_spi_enable(struct bus_dev *bdev) +{ + struct bus_spi_dev *dev = (struct bus_spi_dev *)bdev; + int rc; + + BUS_DEBUG_VERIFY_DEV(dev); + + rc = hal_spi_enable(dev->cfg.spi_num); + if (rc) { + return SYS_EINVAL; + } + + return 0; +} + +static int +bus_spi_configure(struct bus_dev *bdev, struct bus_node *bnode) +{ + struct bus_spi_dev *dev = (struct bus_spi_dev *)bdev; + struct bus_spi_node *node = (struct bus_spi_node *)bnode; + struct bus_spi_node *current_node = (struct bus_spi_node *)bdev->configured_for; + struct hal_spi_settings spi_cfg; + int rc; + + BUS_DEBUG_VERIFY_DEV(dev); + BUS_DEBUG_VERIFY_NODE(node); + + /* No need to reconfigure if already configured with the same settings */ + if (current_node && (current_node->mode == node->mode) && + (current_node->data_order == node->data_order) && + (current_node->freq == node->freq)) { + return 0; + } + + rc = hal_spi_disable(dev->cfg.spi_num); + if (rc) { + goto done; + } + + spi_cfg.data_mode = node->mode; + spi_cfg.data_order = node->data_order; + spi_cfg.baudrate = node->freq; + /* XXX add support for other word sizes */ + spi_cfg.word_size = HAL_SPI_WORD_SIZE_8BIT; + + rc = hal_spi_config(dev->cfg.spi_num, &spi_cfg); + if (rc) { + goto done; + } + + rc = hal_spi_enable(dev->cfg.spi_num); + +done: + if (rc) { + rc = SYS_EIO; + } + + return rc; +} + +static int +bus_spi_read(struct bus_dev *bdev, struct bus_node *bnode, uint8_t *buf, + uint16_t length, os_time_t timeout, uint16_t flags) +{ + struct bus_spi_dev *dev = (struct bus_spi_dev *)bdev; + struct bus_spi_node *node = (struct bus_spi_node *)bnode; + uint8_t val; + int i; + int rc; + + BUS_DEBUG_VERIFY_DEV(dev); + BUS_DEBUG_VERIFY_NODE(node); + + rc = 0; + + hal_gpio_write(node->pin_cs, 0); + + for (i = 0; i < length; i++) { + val = hal_spi_tx_val(dev->cfg.spi_num, 0xAA); + if (val == 0xFFFF) { + rc = SYS_EINVAL; + break; + } + + buf[i] = val; + } + + if (rc || !(flags & BUS_F_NOSTOP)) { + hal_gpio_write(node->pin_cs, 1); + } + + return rc; +} + +static int +bus_spi_write(struct bus_dev *bdev, struct bus_node *bnode, const uint8_t *buf, + uint16_t length, os_time_t timeout, uint16_t flags) +{ + struct bus_spi_dev *dev = (struct bus_spi_dev *)bdev; + struct bus_spi_node *node = (struct bus_spi_node *)bnode; + int rc; + + BUS_DEBUG_VERIFY_DEV(dev); + BUS_DEBUG_VERIFY_NODE(node); + + hal_gpio_write(node->pin_cs, 0); + + /* XXX update HAL to accept const instead */ + rc = hal_spi_txrx(dev->cfg.spi_num, (uint8_t *)buf, NULL, length); + + if (!(flags & BUS_F_NOSTOP)) { + hal_gpio_write(node->pin_cs, 1); + } + + return rc; +} + +static int bus_spi_disable(struct bus_dev *bdev) +{ + struct bus_spi_dev *dev = (struct bus_spi_dev *)bdev; + int rc; + + BUS_DEBUG_VERIFY_DEV(dev); + + rc = hal_spi_disable(dev->cfg.spi_num); + if (rc) { + return SYS_EINVAL; + } + + return 0; +} + +static const struct bus_dev_ops bus_spi_ops = { + .enable = bus_spi_enable, + .configure = bus_spi_configure, + .read = bus_spi_read, + .write = bus_spi_write, + .disable = bus_spi_disable, Review comment: `bus_node_write_read_transact` does write-then-read to simplify perhaps one of most common transaction for peripheral devices. I also had write-and-read API prototype but removed it as I did not implement it since I don't really know device which would require it (I've only seen ones that can use it, but not require). This can be added later as a `bus_node` API and as a new op here (like `write_read`). My original idea was that this would work as expected for SPI bus and return `SYS_ENOTSUP` for I2C bus. For now, as you mentioned, one can just lock bus manually via `bus_dev_lock_by_node` (btw, I don't like this name, thinking of changing it) and use HAL if really necessary. Once I add async APIs which can use non-blocking HAL calls I think it will require to add non-blocking versions of read, write (and read-write) ops as well. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on 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 With regards, Apache Git Services
