From: Jean-Philippe Brucker <jean-philippe@linaro.org> To: Viresh Kumar <viresh.kumar@linaro.org> Cc: Bartosz Golaszewski <bgolaszewski@baylibre.com>, Linus Walleij <linus.walleij@linaro.org>, "Enrico Weigelt, metux IT consult" <info@metux.net>, Viresh Kumar <vireshk@kernel.org>, "Michael S. Tsirkin" <mst@redhat.com>, Jason Wang <jasowang@redhat.com>, linux-kernel@vger.kernel.org, virtualization@lists.linux-foundation.org, linux-gpio@vger.kernel.org, Stefan Hajnoczi <stefanha@redhat.com>, "Stefano Garzarella --cc virtualization @ lists . linux-foundation . org" <sgarzare@redhat.com>, stratos-dev@op-lists.linaro.org Subject: Re: [PATCH V3 1/3] gpio: Add virtio-gpio driver Date: Thu, 10 Jun 2021 19:40:35 +0200 [thread overview] Message-ID: <YMJOk6RWuztRNBXO@myrica> (raw) In-Reply-To: <01000179f5da7763-2ea817c6-e176-423a-952e-de02443f71e2-000000@email.amazonses.com> Hi, Not being very familiar with GPIO, I just have a few general comments and one on the config space layout On Thu, Jun 10, 2021 at 12:16:46PM +0000, Viresh Kumar via Stratos-dev wrote: > +static int virtio_gpio_req(struct virtio_gpio *vgpio, u16 type, u16 gpio, > + u8 txdata, u8 *rxdata) > +{ > + struct virtio_gpio_response *res = &vgpio->cres; > + struct virtio_gpio_request *req = &vgpio->creq; > + struct scatterlist *sgs[2], req_sg, res_sg; > + struct device *dev = &vgpio->vdev->dev; > + unsigned long time_left; > + unsigned int len; > + int ret; > + > + req->type = cpu_to_le16(type); > + req->gpio = cpu_to_le16(gpio); > + req->data = txdata; > + > + sg_init_one(&req_sg, req, sizeof(*req)); > + sg_init_one(&res_sg, res, sizeof(*res)); > + sgs[0] = &req_sg; > + sgs[1] = &res_sg; > + > + mutex_lock(&vgpio->lock); > + ret = virtqueue_add_sgs(vgpio->command_vq, sgs, 1, 1, res, GFP_KERNEL); > + if (ret) { > + dev_err(dev, "failed to add request to vq\n"); > + goto out; > + } > + > + reinit_completion(&vgpio->completion); > + virtqueue_kick(vgpio->command_vq); > + > + time_left = wait_for_completion_timeout(&vgpio->completion, HZ / 10); > + if (!time_left) { > + dev_err(dev, "virtio GPIO backend timeout\n"); > + return -ETIMEDOUT; mutex is still held > + } > + > + WARN_ON(res != virtqueue_get_buf(vgpio->command_vq, &len)); > + if (unlikely(res->status != VIRTIO_GPIO_STATUS_OK)) { > + dev_err(dev, "virtio GPIO request failed: %d\n", gpio); > + return -EINVAL; and here > + } > + > + if (rxdata) > + *rxdata = res->data; > + > +out: > + mutex_unlock(&vgpio->lock); > + > + return ret; > +} > + > +static int virtio_gpio_request(struct gpio_chip *gc, unsigned int gpio) > +{ > + struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc); > + > + return virtio_gpio_req(vgpio, VIRTIO_GPIO_REQ_ACTIVATE, gpio, 0, NULL); > +} > + > +static void virtio_gpio_free(struct gpio_chip *gc, unsigned int gpio) > +{ > + struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc); > + > + virtio_gpio_req(vgpio, VIRTIO_GPIO_REQ_DEACTIVATE, gpio, 0, NULL); > +} > + > +static int virtio_gpio_get_direction(struct gpio_chip *gc, unsigned int gpio) > +{ > + struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc); > + u8 direction; > + int ret; > + > + ret = virtio_gpio_req(vgpio, VIRTIO_GPIO_REQ_GET_DIRECTION, gpio, 0, > + &direction); > + if (ret) > + return ret; > + > + return direction; > +} > + > +static int virtio_gpio_direction_input(struct gpio_chip *gc, unsigned int gpio) > +{ > + struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc); > + > + return virtio_gpio_req(vgpio, VIRTIO_GPIO_REQ_DIRECTION_IN, gpio, 0, > + NULL); > +} > + > +static int virtio_gpio_direction_output(struct gpio_chip *gc, unsigned int gpio, > + int value) > +{ > + struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc); > + > + return virtio_gpio_req(vgpio, VIRTIO_GPIO_REQ_DIRECTION_OUT, gpio, (u8) (that dangling cast looks a bit odd to me) > + value, NULL); > +} > + > +static int virtio_gpio_get(struct gpio_chip *gc, unsigned int gpio) > +{ > + struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc); > + u8 value; > + int ret; > + > + ret = virtio_gpio_req(vgpio, VIRTIO_GPIO_REQ_GET_VALUE, gpio, 0, &value); > + if (ret) > + return ret; > + > + return value; > +} > + > +static void virtio_gpio_set(struct gpio_chip *gc, unsigned int gpio, int value) > +{ > + struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc); > + > + virtio_gpio_req(vgpio, VIRTIO_GPIO_REQ_SET_VALUE, gpio, value, NULL); > +} > + > +static void virtio_gpio_command(struct virtqueue *vq) > +{ > + struct virtio_gpio *vgpio = vq->vdev->priv; > + > + complete(&vgpio->completion); > +} > + > +static int virtio_gpio_alloc_vqs(struct virtio_device *vdev) > +{ > + struct virtio_gpio *vgpio = vdev->priv; > + const char * const names[] = { "command" }; > + vq_callback_t *cbs[] = { > + virtio_gpio_command, > + }; > + struct virtqueue *vqs[1] = {NULL}; > + int ret; > + > + ret = virtio_find_vqs(vdev, 1, vqs, cbs, names, NULL); > + if (ret) { > + dev_err(&vdev->dev, "failed to allocate vqs: %d\n", ret); > + return ret; > + } > + > + vgpio->command_vq = vqs[0]; > + > + /* Mark the device ready to perform operations from within probe() */ > + virtio_device_ready(vgpio->vdev); May fit better in the parent function > + return 0; > +} > + > +static void virtio_gpio_free_vqs(struct virtio_device *vdev) > +{ > + vdev->config->reset(vdev); > + vdev->config->del_vqs(vdev); > +} > + > +static const char **parse_gpio_names(struct virtio_device *vdev, > + struct virtio_gpio_config *config) > +{ > + struct device *dev = &vdev->dev; > + char *str = config->gpio_names; > + const char **names; > + int i, len; > + > + if (!config->gpio_names_size) > + return NULL; > + > + names = devm_kcalloc(dev, config->ngpio, sizeof(names), GFP_KERNEL); > + if (!names) > + return ERR_PTR(-ENOMEM); > + > + /* NULL terminate the string instead of checking it */ > + config->gpio_names[config->gpio_names_size - 1] = '\0'; > + > + for (i = 0; i < config->ngpio; i++) { > + /* > + * We have already marked the last byte with '\0' > + * earlier, this shall be enough here. > + */ > + if (str == config->gpio_names + config->gpio_names_size) { > + dev_err(dev, "Invalid gpio_names\n"); > + return ERR_PTR(-EINVAL); > + } > + > + len = strlen(str); > + if (!len) { > + dev_err(dev, "Missing GPIO name: %d\n", i); > + return ERR_PTR(-EINVAL); > + } > + > + names[i] = str; > + str += len + 1; > + } > + > + return names; > +} > + > +static int virtio_gpio_probe(struct virtio_device *vdev) > +{ > + struct virtio_gpio_config *config; > + struct device *dev = &vdev->dev; > + struct virtio_gpio *vgpio; > + u32 size; > + int ret; > + > + virtio_cread(vdev, struct virtio_gpio_config, gpio_names_size, &size); > + size = cpu_to_le32(size); le32_to_cpu()? > + > + vgpio = devm_kzalloc(dev, sizeof(*vgpio) + size, GFP_KERNEL); > + if (!vgpio) > + return -ENOMEM; > + > + config = &vgpio->config; > + > + /* Read configuration */ > + virtio_cread_bytes(vdev, 0, config, sizeof(*config) + size); > + > + /* NULL terminate the string instead of checking it */ > + config->name[sizeof(config->name) - 1] = '\0'; > + config->ngpio = cpu_to_le16(config->ngpio); > + config->gpio_names_size = cpu_to_le32(config->gpio_names_size); leXX_to_cpu()? > + > + if (!config->ngpio) { > + dev_err(dev, "Number of GPIOs can't be zero\n"); > + return -EINVAL; > + } > + > + vgpio->vdev = vdev; > + vgpio->gc.request = virtio_gpio_request; > + vgpio->gc.free = virtio_gpio_free; > + vgpio->gc.get_direction = virtio_gpio_get_direction; > + vgpio->gc.direction_input = virtio_gpio_direction_input; > + vgpio->gc.direction_output = virtio_gpio_direction_output; > + vgpio->gc.get = virtio_gpio_get; > + vgpio->gc.set = virtio_gpio_set; > + vgpio->gc.ngpio = config->ngpio; > + vgpio->gc.base = -1; /* Allocate base dynamically */ > + vgpio->gc.label = config->name[0] ? > + config->name : dev_name(dev); > + vgpio->gc.parent = dev; > + vgpio->gc.owner = THIS_MODULE; > + vgpio->gc.can_sleep = true; > + vgpio->gc.names = parse_gpio_names(vdev, config); > + if (IS_ERR(vgpio->gc.names)) > + return PTR_ERR(vgpio->gc.names); > + > + vdev->priv = vgpio; > + mutex_init(&vgpio->lock); > + init_completion(&vgpio->completion); > + > + ret = virtio_gpio_alloc_vqs(vdev); > + if (ret) > + return ret; > + > + ret = gpiochip_add(&vgpio->gc); > + if (ret) { > + virtio_gpio_free_vqs(vdev); > + dev_err(dev, "Failed to add virtio-gpio controller\n"); > + } > + > + return ret; > +} > + > +static void virtio_gpio_remove(struct virtio_device *vdev) > +{ > + struct virtio_gpio *vgpio = vdev->priv; > + > + gpiochip_remove(&vgpio->gc); > + virtio_gpio_free_vqs(vdev); > +} > + > +static const struct virtio_device_id id_table[] = { > + { VIRTIO_ID_GPIO, VIRTIO_DEV_ANY_ID }, > + {}, > +}; > +MODULE_DEVICE_TABLE(virtio, id_table); > + > +static struct virtio_driver virtio_gpio_driver = { > + .id_table = id_table, > + .probe = virtio_gpio_probe, > + .remove = virtio_gpio_remove, > + .driver = { > + .name = KBUILD_MODNAME, > + .owner = THIS_MODULE, > + }, > +}; > +module_virtio_driver(virtio_gpio_driver); > + > +MODULE_AUTHOR("Enrico Weigelt, metux IT consult <info@metux.net>"); > +MODULE_AUTHOR("Viresh Kumar <viresh.kumar@linaro.org>"); > +MODULE_DESCRIPTION("VirtIO GPIO driver"); > +MODULE_LICENSE("GPL"); > diff --git a/include/uapi/linux/virtio_gpio.h b/include/uapi/linux/virtio_gpio.h > new file mode 100644 > index 000000000000..e805e33a79cb > --- /dev/null > +++ b/include/uapi/linux/virtio_gpio.h > @@ -0,0 +1,41 @@ > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ > + > +#ifndef _LINUX_VIRTIO_GPIO_H > +#define _LINUX_VIRTIO_GPIO_H > + > +#include <linux/types.h> > + > +/* Virtio GPIO request types */ > +#define VIRTIO_GPIO_REQ_ACTIVATE 0x01 > +#define VIRTIO_GPIO_REQ_DEACTIVATE 0x02 > +#define VIRTIO_GPIO_REQ_GET_DIRECTION 0x03 > +#define VIRTIO_GPIO_REQ_DIRECTION_IN 0x04 > +#define VIRTIO_GPIO_REQ_DIRECTION_OUT 0x05 > +#define VIRTIO_GPIO_REQ_GET_VALUE 0x06 > +#define VIRTIO_GPIO_REQ_SET_VALUE 0x07 > + > +/* Possible values of the status field */ > +#define VIRTIO_GPIO_STATUS_OK 0x0 > +#define VIRTIO_GPIO_STATUS_ERR 0x1 > + > +struct virtio_gpio_config { > + char name[32]; > + __u16 ngpio; > + __u16 padding; > + __u32 gpio_names_size; > + char gpio_names[0]; A variable-size array here will make it very difficult to append new fields to virtio_gpio_config, when adding new features to the device. (New fields cannot be inserted in between, since older drivers will expect existing fields at a specific offset.) You could replace it with a reference to the string array, for example "__u16 gpio_names_offset" declaring the offset between the beginning of device-specific config and the string array. The 'name' field could also be indirect to avoid setting a fixed 32-char size, but that's not as important. > +} __packed; No need for __packed, because the fields are naturally aligned (as required by the virtio spec) Thanks, Jean > + > +/* Virtio GPIO Request / Response */ > +struct virtio_gpio_request { > + __u16 type; > + __u16 gpio; > + __u8 data; > +}; > + > +struct virtio_gpio_response { > + __u8 status; > + __u8 data; > +}; > + > +#endif /* _LINUX_VIRTIO_GPIO_H */ > diff --git a/include/uapi/linux/virtio_ids.h b/include/uapi/linux/virtio_ids.h > index b89391dff6c9..0c24e8ae2499 100644 > --- a/include/uapi/linux/virtio_ids.h > +++ b/include/uapi/linux/virtio_ids.h > @@ -55,5 +55,6 @@ > #define VIRTIO_ID_PMEM 27 /* virtio pmem */ > #define VIRTIO_ID_MAC80211_HWSIM 29 /* virtio mac80211-hwsim */ > #define VIRTIO_ID_I2C_ADAPTER 34 /* virtio i2c adapter */ > +#define VIRTIO_ID_GPIO 41 /* virtio GPIO */ > > #endif /* _LINUX_VIRTIO_IDS_H */ > -- > 2.31.1.272.g89b43f80a514 > > -- > Stratos-dev mailing list > Stratos-dev@op-lists.linaro.org > https://op-lists.linaro.org/mailman/listinfo/stratos-dev
WARNING: multiple messages have this Message-ID (diff)
From: Jean-Philippe Brucker <jean-philippe@linaro.org> To: Viresh Kumar <viresh.kumar@linaro.org> Cc: Stefan Hajnoczi <stefanha@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Viresh Kumar <vireshk@kernel.org>, Linus Walleij <linus.walleij@linaro.org>, linux-kernel@vger.kernel.org, virtualization@lists.linux-foundation.org, Bartosz Golaszewski <bgolaszewski@baylibre.com>, linux-gpio@vger.kernel.org, stratos-dev@op-lists.linaro.org, "Enrico Weigelt, metux IT consult" <info@metux.net> Subject: Re: [PATCH V3 1/3] gpio: Add virtio-gpio driver Date: Thu, 10 Jun 2021 19:40:35 +0200 [thread overview] Message-ID: <YMJOk6RWuztRNBXO@myrica> (raw) In-Reply-To: <01000179f5da7763-2ea817c6-e176-423a-952e-de02443f71e2-000000@email.amazonses.com> Hi, Not being very familiar with GPIO, I just have a few general comments and one on the config space layout On Thu, Jun 10, 2021 at 12:16:46PM +0000, Viresh Kumar via Stratos-dev wrote: > +static int virtio_gpio_req(struct virtio_gpio *vgpio, u16 type, u16 gpio, > + u8 txdata, u8 *rxdata) > +{ > + struct virtio_gpio_response *res = &vgpio->cres; > + struct virtio_gpio_request *req = &vgpio->creq; > + struct scatterlist *sgs[2], req_sg, res_sg; > + struct device *dev = &vgpio->vdev->dev; > + unsigned long time_left; > + unsigned int len; > + int ret; > + > + req->type = cpu_to_le16(type); > + req->gpio = cpu_to_le16(gpio); > + req->data = txdata; > + > + sg_init_one(&req_sg, req, sizeof(*req)); > + sg_init_one(&res_sg, res, sizeof(*res)); > + sgs[0] = &req_sg; > + sgs[1] = &res_sg; > + > + mutex_lock(&vgpio->lock); > + ret = virtqueue_add_sgs(vgpio->command_vq, sgs, 1, 1, res, GFP_KERNEL); > + if (ret) { > + dev_err(dev, "failed to add request to vq\n"); > + goto out; > + } > + > + reinit_completion(&vgpio->completion); > + virtqueue_kick(vgpio->command_vq); > + > + time_left = wait_for_completion_timeout(&vgpio->completion, HZ / 10); > + if (!time_left) { > + dev_err(dev, "virtio GPIO backend timeout\n"); > + return -ETIMEDOUT; mutex is still held > + } > + > + WARN_ON(res != virtqueue_get_buf(vgpio->command_vq, &len)); > + if (unlikely(res->status != VIRTIO_GPIO_STATUS_OK)) { > + dev_err(dev, "virtio GPIO request failed: %d\n", gpio); > + return -EINVAL; and here > + } > + > + if (rxdata) > + *rxdata = res->data; > + > +out: > + mutex_unlock(&vgpio->lock); > + > + return ret; > +} > + > +static int virtio_gpio_request(struct gpio_chip *gc, unsigned int gpio) > +{ > + struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc); > + > + return virtio_gpio_req(vgpio, VIRTIO_GPIO_REQ_ACTIVATE, gpio, 0, NULL); > +} > + > +static void virtio_gpio_free(struct gpio_chip *gc, unsigned int gpio) > +{ > + struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc); > + > + virtio_gpio_req(vgpio, VIRTIO_GPIO_REQ_DEACTIVATE, gpio, 0, NULL); > +} > + > +static int virtio_gpio_get_direction(struct gpio_chip *gc, unsigned int gpio) > +{ > + struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc); > + u8 direction; > + int ret; > + > + ret = virtio_gpio_req(vgpio, VIRTIO_GPIO_REQ_GET_DIRECTION, gpio, 0, > + &direction); > + if (ret) > + return ret; > + > + return direction; > +} > + > +static int virtio_gpio_direction_input(struct gpio_chip *gc, unsigned int gpio) > +{ > + struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc); > + > + return virtio_gpio_req(vgpio, VIRTIO_GPIO_REQ_DIRECTION_IN, gpio, 0, > + NULL); > +} > + > +static int virtio_gpio_direction_output(struct gpio_chip *gc, unsigned int gpio, > + int value) > +{ > + struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc); > + > + return virtio_gpio_req(vgpio, VIRTIO_GPIO_REQ_DIRECTION_OUT, gpio, (u8) (that dangling cast looks a bit odd to me) > + value, NULL); > +} > + > +static int virtio_gpio_get(struct gpio_chip *gc, unsigned int gpio) > +{ > + struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc); > + u8 value; > + int ret; > + > + ret = virtio_gpio_req(vgpio, VIRTIO_GPIO_REQ_GET_VALUE, gpio, 0, &value); > + if (ret) > + return ret; > + > + return value; > +} > + > +static void virtio_gpio_set(struct gpio_chip *gc, unsigned int gpio, int value) > +{ > + struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc); > + > + virtio_gpio_req(vgpio, VIRTIO_GPIO_REQ_SET_VALUE, gpio, value, NULL); > +} > + > +static void virtio_gpio_command(struct virtqueue *vq) > +{ > + struct virtio_gpio *vgpio = vq->vdev->priv; > + > + complete(&vgpio->completion); > +} > + > +static int virtio_gpio_alloc_vqs(struct virtio_device *vdev) > +{ > + struct virtio_gpio *vgpio = vdev->priv; > + const char * const names[] = { "command" }; > + vq_callback_t *cbs[] = { > + virtio_gpio_command, > + }; > + struct virtqueue *vqs[1] = {NULL}; > + int ret; > + > + ret = virtio_find_vqs(vdev, 1, vqs, cbs, names, NULL); > + if (ret) { > + dev_err(&vdev->dev, "failed to allocate vqs: %d\n", ret); > + return ret; > + } > + > + vgpio->command_vq = vqs[0]; > + > + /* Mark the device ready to perform operations from within probe() */ > + virtio_device_ready(vgpio->vdev); May fit better in the parent function > + return 0; > +} > + > +static void virtio_gpio_free_vqs(struct virtio_device *vdev) > +{ > + vdev->config->reset(vdev); > + vdev->config->del_vqs(vdev); > +} > + > +static const char **parse_gpio_names(struct virtio_device *vdev, > + struct virtio_gpio_config *config) > +{ > + struct device *dev = &vdev->dev; > + char *str = config->gpio_names; > + const char **names; > + int i, len; > + > + if (!config->gpio_names_size) > + return NULL; > + > + names = devm_kcalloc(dev, config->ngpio, sizeof(names), GFP_KERNEL); > + if (!names) > + return ERR_PTR(-ENOMEM); > + > + /* NULL terminate the string instead of checking it */ > + config->gpio_names[config->gpio_names_size - 1] = '\0'; > + > + for (i = 0; i < config->ngpio; i++) { > + /* > + * We have already marked the last byte with '\0' > + * earlier, this shall be enough here. > + */ > + if (str == config->gpio_names + config->gpio_names_size) { > + dev_err(dev, "Invalid gpio_names\n"); > + return ERR_PTR(-EINVAL); > + } > + > + len = strlen(str); > + if (!len) { > + dev_err(dev, "Missing GPIO name: %d\n", i); > + return ERR_PTR(-EINVAL); > + } > + > + names[i] = str; > + str += len + 1; > + } > + > + return names; > +} > + > +static int virtio_gpio_probe(struct virtio_device *vdev) > +{ > + struct virtio_gpio_config *config; > + struct device *dev = &vdev->dev; > + struct virtio_gpio *vgpio; > + u32 size; > + int ret; > + > + virtio_cread(vdev, struct virtio_gpio_config, gpio_names_size, &size); > + size = cpu_to_le32(size); le32_to_cpu()? > + > + vgpio = devm_kzalloc(dev, sizeof(*vgpio) + size, GFP_KERNEL); > + if (!vgpio) > + return -ENOMEM; > + > + config = &vgpio->config; > + > + /* Read configuration */ > + virtio_cread_bytes(vdev, 0, config, sizeof(*config) + size); > + > + /* NULL terminate the string instead of checking it */ > + config->name[sizeof(config->name) - 1] = '\0'; > + config->ngpio = cpu_to_le16(config->ngpio); > + config->gpio_names_size = cpu_to_le32(config->gpio_names_size); leXX_to_cpu()? > + > + if (!config->ngpio) { > + dev_err(dev, "Number of GPIOs can't be zero\n"); > + return -EINVAL; > + } > + > + vgpio->vdev = vdev; > + vgpio->gc.request = virtio_gpio_request; > + vgpio->gc.free = virtio_gpio_free; > + vgpio->gc.get_direction = virtio_gpio_get_direction; > + vgpio->gc.direction_input = virtio_gpio_direction_input; > + vgpio->gc.direction_output = virtio_gpio_direction_output; > + vgpio->gc.get = virtio_gpio_get; > + vgpio->gc.set = virtio_gpio_set; > + vgpio->gc.ngpio = config->ngpio; > + vgpio->gc.base = -1; /* Allocate base dynamically */ > + vgpio->gc.label = config->name[0] ? > + config->name : dev_name(dev); > + vgpio->gc.parent = dev; > + vgpio->gc.owner = THIS_MODULE; > + vgpio->gc.can_sleep = true; > + vgpio->gc.names = parse_gpio_names(vdev, config); > + if (IS_ERR(vgpio->gc.names)) > + return PTR_ERR(vgpio->gc.names); > + > + vdev->priv = vgpio; > + mutex_init(&vgpio->lock); > + init_completion(&vgpio->completion); > + > + ret = virtio_gpio_alloc_vqs(vdev); > + if (ret) > + return ret; > + > + ret = gpiochip_add(&vgpio->gc); > + if (ret) { > + virtio_gpio_free_vqs(vdev); > + dev_err(dev, "Failed to add virtio-gpio controller\n"); > + } > + > + return ret; > +} > + > +static void virtio_gpio_remove(struct virtio_device *vdev) > +{ > + struct virtio_gpio *vgpio = vdev->priv; > + > + gpiochip_remove(&vgpio->gc); > + virtio_gpio_free_vqs(vdev); > +} > + > +static const struct virtio_device_id id_table[] = { > + { VIRTIO_ID_GPIO, VIRTIO_DEV_ANY_ID }, > + {}, > +}; > +MODULE_DEVICE_TABLE(virtio, id_table); > + > +static struct virtio_driver virtio_gpio_driver = { > + .id_table = id_table, > + .probe = virtio_gpio_probe, > + .remove = virtio_gpio_remove, > + .driver = { > + .name = KBUILD_MODNAME, > + .owner = THIS_MODULE, > + }, > +}; > +module_virtio_driver(virtio_gpio_driver); > + > +MODULE_AUTHOR("Enrico Weigelt, metux IT consult <info@metux.net>"); > +MODULE_AUTHOR("Viresh Kumar <viresh.kumar@linaro.org>"); > +MODULE_DESCRIPTION("VirtIO GPIO driver"); > +MODULE_LICENSE("GPL"); > diff --git a/include/uapi/linux/virtio_gpio.h b/include/uapi/linux/virtio_gpio.h > new file mode 100644 > index 000000000000..e805e33a79cb > --- /dev/null > +++ b/include/uapi/linux/virtio_gpio.h > @@ -0,0 +1,41 @@ > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ > + > +#ifndef _LINUX_VIRTIO_GPIO_H > +#define _LINUX_VIRTIO_GPIO_H > + > +#include <linux/types.h> > + > +/* Virtio GPIO request types */ > +#define VIRTIO_GPIO_REQ_ACTIVATE 0x01 > +#define VIRTIO_GPIO_REQ_DEACTIVATE 0x02 > +#define VIRTIO_GPIO_REQ_GET_DIRECTION 0x03 > +#define VIRTIO_GPIO_REQ_DIRECTION_IN 0x04 > +#define VIRTIO_GPIO_REQ_DIRECTION_OUT 0x05 > +#define VIRTIO_GPIO_REQ_GET_VALUE 0x06 > +#define VIRTIO_GPIO_REQ_SET_VALUE 0x07 > + > +/* Possible values of the status field */ > +#define VIRTIO_GPIO_STATUS_OK 0x0 > +#define VIRTIO_GPIO_STATUS_ERR 0x1 > + > +struct virtio_gpio_config { > + char name[32]; > + __u16 ngpio; > + __u16 padding; > + __u32 gpio_names_size; > + char gpio_names[0]; A variable-size array here will make it very difficult to append new fields to virtio_gpio_config, when adding new features to the device. (New fields cannot be inserted in between, since older drivers will expect existing fields at a specific offset.) You could replace it with a reference to the string array, for example "__u16 gpio_names_offset" declaring the offset between the beginning of device-specific config and the string array. The 'name' field could also be indirect to avoid setting a fixed 32-char size, but that's not as important. > +} __packed; No need for __packed, because the fields are naturally aligned (as required by the virtio spec) Thanks, Jean > + > +/* Virtio GPIO Request / Response */ > +struct virtio_gpio_request { > + __u16 type; > + __u16 gpio; > + __u8 data; > +}; > + > +struct virtio_gpio_response { > + __u8 status; > + __u8 data; > +}; > + > +#endif /* _LINUX_VIRTIO_GPIO_H */ > diff --git a/include/uapi/linux/virtio_ids.h b/include/uapi/linux/virtio_ids.h > index b89391dff6c9..0c24e8ae2499 100644 > --- a/include/uapi/linux/virtio_ids.h > +++ b/include/uapi/linux/virtio_ids.h > @@ -55,5 +55,6 @@ > #define VIRTIO_ID_PMEM 27 /* virtio pmem */ > #define VIRTIO_ID_MAC80211_HWSIM 29 /* virtio mac80211-hwsim */ > #define VIRTIO_ID_I2C_ADAPTER 34 /* virtio i2c adapter */ > +#define VIRTIO_ID_GPIO 41 /* virtio GPIO */ > > #endif /* _LINUX_VIRTIO_IDS_H */ > -- > 2.31.1.272.g89b43f80a514 > > -- > Stratos-dev mailing list > Stratos-dev@op-lists.linaro.org > https://op-lists.linaro.org/mailman/listinfo/stratos-dev _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
next prev parent reply other threads:[~2021-06-10 17:42 UTC|newest] Thread overview: 73+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-06-10 12:09 [PATCH V3 0/3] gpio: Add virtio based driver Viresh Kumar 2021-06-10 12:09 ` Viresh Kumar 2021-06-10 12:16 ` [PATCH V3 1/3] gpio: Add virtio-gpio driver Viresh Kumar 2021-06-10 12:16 ` Viresh Kumar 2021-06-10 13:22 ` Arnd Bergmann 2021-06-10 16:00 ` Enrico Weigelt, metux IT consult [not found] ` <01000179f6a7715c-cd106846-7770-4088-bb7c-a696bfcbf83e-000000@email.amazonses.com> 2021-06-10 17:03 ` [Stratos-dev] " Jean-Philippe Brucker 2021-06-10 17:03 ` Jean-Philippe Brucker 2021-06-10 19:41 ` Arnd Bergmann 2021-06-14 10:21 ` Viresh Kumar 2021-06-14 10:21 ` Viresh Kumar 2021-06-14 12:31 ` Arnd Bergmann 2021-06-14 12:49 ` Vincent Guittot [not found] ` <0100017a0a9264cc-57668c56-fdbf-412a-9f82-9bf95f5c653e-000000@email.amazonses.com> 2021-06-14 12:58 ` [Stratos-dev] " Arnd Bergmann 2021-06-14 13:24 ` Vincent Guittot 2021-06-14 20:54 ` Arnd Bergmann 2021-06-15 7:30 ` Vincent Guittot 2021-06-10 15:54 ` Enrico Weigelt, metux IT consult 2021-06-10 16:57 ` Viresh Kumar 2021-06-10 16:57 ` Viresh Kumar 2021-06-10 20:46 ` Linus Walleij 2021-06-10 20:46 ` Linus Walleij 2021-06-11 3:56 ` Viresh Kumar 2021-06-11 3:56 ` Viresh Kumar 2021-06-11 7:42 ` Geert Uytterhoeven 2021-06-11 7:42 ` Geert Uytterhoeven 2021-06-11 8:01 ` Viresh Kumar 2021-06-11 8:01 ` Viresh Kumar 2021-06-11 8:22 ` Geert Uytterhoeven 2021-06-11 8:22 ` Geert Uytterhoeven 2021-06-15 11:15 ` Viresh Kumar 2021-06-15 11:15 ` Viresh Kumar 2021-06-15 11:37 ` Geert Uytterhoeven 2021-06-15 11:37 ` Geert Uytterhoeven 2021-06-15 20:03 ` Linus Walleij 2021-06-15 20:03 ` Linus Walleij 2021-06-16 1:45 ` Viresh Kumar 2021-06-16 1:45 ` Viresh Kumar 2021-06-14 8:07 ` Enrico Weigelt, metux IT consult 2021-06-14 8:12 ` Andy Shevchenko 2021-06-14 8:12 ` Andy Shevchenko 2021-06-14 9:14 ` Viresh Kumar 2021-06-14 9:14 ` Viresh Kumar 2021-06-14 9:17 ` Enrico Weigelt, metux IT consult 2021-06-14 9:52 ` Viresh Kumar 2021-06-14 9:52 ` Viresh Kumar 2021-06-14 9:12 ` Viresh Kumar 2021-06-14 9:12 ` Viresh Kumar 2021-06-14 9:29 ` Enrico Weigelt, metux IT consult 2021-06-14 8:03 ` Enrico Weigelt, metux IT consult 2021-06-14 9:24 ` Viresh Kumar 2021-06-14 9:24 ` Viresh Kumar 2021-06-16 3:30 ` Bjorn Andersson 2021-06-16 3:30 ` Bjorn Andersson 2021-06-16 15:52 ` Enrico Weigelt, metux IT consult 2021-06-18 9:13 ` Linus Walleij 2021-06-18 9:13 ` Linus Walleij 2021-06-21 17:25 ` Bjorn Andersson 2021-06-21 17:25 ` Bjorn Andersson 2021-06-10 12:16 ` [PATCH V3 2/3] gpio: virtio: Add IRQ support Viresh Kumar 2021-06-10 12:16 ` Viresh Kumar 2021-06-10 21:30 ` Linus Walleij 2021-06-10 21:30 ` Linus Walleij 2021-06-14 7:08 ` Viresh Kumar 2021-06-14 7:08 ` Viresh Kumar 2021-06-10 12:16 ` [PATCH V3 3/3] MAINTAINERS: Add entry for Virtio-gpio Viresh Kumar [not found] ` <01000179f5da7763-2ea817c6-e176-423a-952e-de02443f71e2-000000@email.amazonses.com> 2021-06-10 17:40 ` Jean-Philippe Brucker [this message] 2021-06-10 17:40 ` [PATCH V3 1/3] gpio: Add virtio-gpio driver Jean-Philippe Brucker 2021-06-11 3:39 ` Viresh Kumar 2021-06-11 3:39 ` Viresh Kumar [not found] ` <01000179f9276678-ae2bb25f-4c0c-4176-b906-650c585b9753-000000@email.amazonses.com> 2021-06-11 8:34 ` [Stratos-dev] " Arnd Bergmann 2021-06-14 5:26 ` Viresh Kumar 2021-06-14 5:26 ` Viresh Kumar
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=YMJOk6RWuztRNBXO@myrica \ --to=jean-philippe@linaro.org \ --cc=bgolaszewski@baylibre.com \ --cc=info@metux.net \ --cc=jasowang@redhat.com \ --cc=linus.walleij@linaro.org \ --cc=linux-gpio@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mst@redhat.com \ --cc=sgarzare@redhat.com \ --cc=stefanha@redhat.com \ --cc=stratos-dev@op-lists.linaro.org \ --cc=viresh.kumar@linaro.org \ --cc=vireshk@kernel.org \ --cc=virtualization@lists.linux-foundation.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.