mkiiskila commented on a change in pull request #1507: [RFC] Add generic serial bus driver URL: https://github.com/apache/mynewt-core/pull/1507#discussion_r234151068
########## 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: For SPI devices combined read/write is occasionally needed. I see there's an call API matching combined read/write semantic in bus_node(), but no such thing here? I think there's 2 options here; either add combined read/write to allow that for SPI, or make the bus_node_XXX interface such that the driver writer can take advantage of the locking/reconfig for bus-sharing done there, and then they can call hal_XXX routines of their choice while the device is locked/configured. Including the non-blocking read/write version. Or were you going to roll those as separate interface within bus_dev_ops()? ---------------------------------------------------------------- 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
