diff mbox series

gpio: virtio: Fix sparse warnings

Message ID 32ab7b833743449b21f529cae41f4cbb60dc863c.1630387746.git.viresh.kumar@linaro.org
State Accepted
Commit 17395d7742baa4737e9d3b4672cc3d10e5970998
Headers show
Series gpio: virtio: Fix sparse warnings | expand

Commit Message

Viresh Kumar Aug. 31, 2021, 5:29 a.m. UTC 
Fix warnings reported by sparse, related to type mismatch between u16
and __le16.

Reported-by: kernel test robot <lkp@intel.com>
Fixes: 3a29355a22c0 ("gpio: Add virtio-gpio driver")
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/gpio/gpio-virtio.c       | 41 ++++++++++++++++----------------
 include/uapi/linux/virtio_gpio.h | 10 ++++----
 2 files changed, 25 insertions(+), 26 deletions(-)

-- 
2.31.1.272.g89b43f80a514

Comments

Michael S. Tsirkin Aug. 31, 2021, 6:25 a.m. UTC | #1 
On Tue, Aug 31, 2021 at 10:59:25AM +0530, Viresh Kumar wrote:
> Fix warnings reported by sparse, related to type mismatch between u16

> and __le16.

> 

> Reported-by: kernel test robot <lkp@intel.com>

> Fixes: 3a29355a22c0 ("gpio: Add virtio-gpio driver")

> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>


Acked-by: Michael S. Tsirkin <mst@redhat.com>


I'm not sure which tree has the above commit - can this be squashed?

Also, the driver lacks a MAINTAINERS entry - we want at least
L:      virtualization@lists.linux-foundation.org
on all virtio drivers.


> ---

>  drivers/gpio/gpio-virtio.c       | 41 ++++++++++++++++----------------

>  include/uapi/linux/virtio_gpio.h | 10 ++++----

>  2 files changed, 25 insertions(+), 26 deletions(-)

> 

> diff --git a/drivers/gpio/gpio-virtio.c b/drivers/gpio/gpio-virtio.c

> index d33eb237c0b9..d24f1c9264bc 100644

> --- a/drivers/gpio/gpio-virtio.c

> +++ b/drivers/gpio/gpio-virtio.c

> @@ -32,7 +32,6 @@ struct virtio_gpio {

>  	struct virtio_device *vdev;

>  	struct mutex lock; /* Protects virtqueue operation */

>  	struct gpio_chip gc;

> -	struct virtio_gpio_config config;

>  	struct virtio_gpio_line *lines;

>  	struct virtqueue *request_vq;

>  };

> @@ -57,7 +56,7 @@ static int _virtio_gpio_req(struct virtio_gpio *vgpio, u16 type, u16 gpio,

>  

>  	req->type = cpu_to_le16(type);

>  	req->gpio = cpu_to_le16(gpio);

> -	req->value = txvalue;

> +	req->value = cpu_to_le32(txvalue);

>  

>  	sg_init_one(&req_sg, req, sizeof(*req));

>  	sg_init_one(&res_sg, res, rxlen);

> @@ -233,19 +232,19 @@ static int virtio_gpio_alloc_vqs(struct virtio_gpio *vgpio,

>  	return 0;

>  }

>  

> -static const char **virtio_gpio_get_names(struct virtio_gpio *vgpio)

> +static const char **virtio_gpio_get_names(struct virtio_gpio *vgpio,

> +					  u32 gpio_names_size, u16 ngpio)

>  {

> -	struct virtio_gpio_config *config = &vgpio->config;

>  	struct virtio_gpio_response_get_names *res;

>  	struct device *dev = &vgpio->vdev->dev;

>  	u8 *gpio_names, *str;

>  	const char **names;

>  	int i, ret, len;

>  

> -	if (!config->gpio_names_size)

> +	if (!gpio_names_size)

>  		return NULL;

>  

> -	len = sizeof(*res) + config->gpio_names_size;

> +	len = sizeof(*res) + gpio_names_size;

>  	res = devm_kzalloc(dev, len, GFP_KERNEL);

>  	if (!res)

>  		return NULL;

> @@ -258,18 +257,18 @@ static const char **virtio_gpio_get_names(struct virtio_gpio *vgpio)

>  		return NULL;

>  	}

>  

> -	names = devm_kcalloc(dev, config->ngpio, sizeof(*names), GFP_KERNEL);

> +	names = devm_kcalloc(dev, ngpio, sizeof(*names), GFP_KERNEL);

>  	if (!names)

>  		return NULL;

>  

>  	/* NULL terminate the string instead of checking it */

> -	gpio_names[config->gpio_names_size - 1] = '\0';

> +	gpio_names[gpio_names_size - 1] = '\0';

>  

> -	for (i = 0, str = gpio_names; i < config->ngpio; i++) {

> +	for (i = 0, str = gpio_names; i < ngpio; i++) {

>  		names[i] = str;

>  		str += strlen(str) + 1; /* zero-length strings are allowed */

>  

> -		if (str > gpio_names + config->gpio_names_size) {

> +		if (str > gpio_names + gpio_names_size) {

>  			dev_err(dev, "gpio_names block is too short (%d)\n", i);

>  			return NULL;

>  		}

> @@ -280,31 +279,31 @@ static const char **virtio_gpio_get_names(struct virtio_gpio *vgpio)

>  

>  static int virtio_gpio_probe(struct virtio_device *vdev)

>  {

> -	struct virtio_gpio_config *config;

> +	struct virtio_gpio_config config;

>  	struct device *dev = &vdev->dev;

>  	struct virtio_gpio *vgpio;

> +	u32 gpio_names_size;

> +	u16 ngpio;

>  	int ret, i;

>  

>  	vgpio = devm_kzalloc(dev, sizeof(*vgpio), GFP_KERNEL);

>  	if (!vgpio)

>  		return -ENOMEM;

>  

> -	config = &vgpio->config;

> -

>  	/* Read configuration */

> -	virtio_cread_bytes(vdev, 0, config, sizeof(*config));

> -	config->gpio_names_size = le32_to_cpu(config->gpio_names_size);

> -	config->ngpio = le16_to_cpu(config->ngpio);

> -	if (!config->ngpio) {

> +	virtio_cread_bytes(vdev, 0, &config, sizeof(config));

> +	gpio_names_size = le32_to_cpu(config.gpio_names_size);

> +	ngpio = le16_to_cpu(config.ngpio);

> +	if (!ngpio) {

>  		dev_err(dev, "Number of GPIOs can't be zero\n");

>  		return -EINVAL;

>  	}

>  

> -	vgpio->lines = devm_kcalloc(dev, config->ngpio, sizeof(*vgpio->lines), GFP_KERNEL);

> +	vgpio->lines = devm_kcalloc(dev, ngpio, sizeof(*vgpio->lines), GFP_KERNEL);

>  	if (!vgpio->lines)

>  		return -ENOMEM;

>  

> -	for (i = 0; i < config->ngpio; i++) {

> +	for (i = 0; i < ngpio; i++) {

>  		mutex_init(&vgpio->lines[i].lock);

>  		init_completion(&vgpio->lines[i].completion);

>  	}

> @@ -319,7 +318,7 @@ static int virtio_gpio_probe(struct virtio_device *vdev)

>  	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.ngpio			= ngpio;

>  	vgpio->gc.base			= -1; /* Allocate base dynamically */

>  	vgpio->gc.label			= dev_name(dev);

>  	vgpio->gc.parent		= dev;

> @@ -333,7 +332,7 @@ static int virtio_gpio_probe(struct virtio_device *vdev)

>  	/* Mark the device ready to perform operations from within probe() */

>  	virtio_device_ready(vdev);

>  

> -	vgpio->gc.names = virtio_gpio_get_names(vgpio);

> +	vgpio->gc.names = virtio_gpio_get_names(vgpio, gpio_names_size, ngpio);

>  

>  	ret = gpiochip_add_data(&vgpio->gc, vgpio);

>  	if (ret) {

> diff --git a/include/uapi/linux/virtio_gpio.h b/include/uapi/linux/virtio_gpio.h

> index 844574acf095..0445f905d8cc 100644

> --- a/include/uapi/linux/virtio_gpio.h

> +++ b/include/uapi/linux/virtio_gpio.h

> @@ -22,16 +22,16 @@

>  #define VIRTIO_GPIO_DIRECTION_IN		0x02

>  

>  struct virtio_gpio_config {

> -	__u16 ngpio;

> +	__le16 ngpio;

>  	__u8 padding[2];

> -	__u32 gpio_names_size;

> +	__le32 gpio_names_size;

>  } __packed;

>  

>  /* Virtio GPIO Request / Response */

>  struct virtio_gpio_request {

> -	__u16 type;

> -	__u16 gpio;

> -	__u32 value;

> +	__le16 type;

> +	__le16 gpio;

> +	__le32 value;

>  };

>  

>  struct virtio_gpio_response {

> -- 

> 2.31.1.272.g89b43f80a514
Viresh Kumar Aug. 31, 2021, 6:31 a.m. UTC | #2 
On 31-08-21, 02:25, Michael S. Tsirkin wrote:
> On Tue, Aug 31, 2021 at 10:59:25AM +0530, Viresh Kumar wrote:

> > Fix warnings reported by sparse, related to type mismatch between u16

> > and __le16.

> > 

> > Reported-by: kernel test robot <lkp@intel.com>

> > Fixes: 3a29355a22c0 ("gpio: Add virtio-gpio driver")

> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

> 

> Acked-by: Michael S. Tsirkin <mst@redhat.com>

> 

> I'm not sure which tree has the above commit - can this be squashed?


It has gone via the GPIO tree:

https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git/log/?h=gpio/for-next

I believe it can be squashed, Bartosz can confirm the same though.

> Also, the driver lacks a MAINTAINERS entry - we want at least

> L:      virtualization@lists.linux-foundation.org

> on all virtio drivers.


Sure, I will send a patch for that.

-- 
viresh
Bartosz Golaszewski Aug. 31, 2021, 9:07 a.m. UTC | #3 
On Tue, Aug 31, 2021 at 8:31 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>

> On 31-08-21, 02:25, Michael S. Tsirkin wrote:

> > On Tue, Aug 31, 2021 at 10:59:25AM +0530, Viresh Kumar wrote:

> > > Fix warnings reported by sparse, related to type mismatch between u16

> > > and __le16.

> > >

> > > Reported-by: kernel test robot <lkp@intel.com>

> > > Fixes: 3a29355a22c0 ("gpio: Add virtio-gpio driver")

> > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

> >

> > Acked-by: Michael S. Tsirkin <mst@redhat.com>

> >

> > I'm not sure which tree has the above commit - can this be squashed?

>

> It has gone via the GPIO tree:

>

> https://git.kernel.org/pub/scm/linux/kernel/git/brgl/linux.git/log/?h=gpio/for-next

>

> I believe it can be squashed, Bartosz can confirm the same though.


I just applied it on top of my for-next branch.

Bart

>

> > Also, the driver lacks a MAINTAINERS entry - we want at least

> > L:      virtualization@lists.linux-foundation.org

> > on all virtio drivers.

>

> Sure, I will send a patch for that.

>

> --

> viresh
diff mbox series

Patch

diff --git a/drivers/gpio/gpio-virtio.c b/drivers/gpio/gpio-virtio.c
index d33eb237c0b9..d24f1c9264bc 100644
--- a/drivers/gpio/gpio-virtio.c
+++ b/drivers/gpio/gpio-virtio.c
@@ -32,7 +32,6 @@  struct virtio_gpio {
 	struct virtio_device *vdev;
 	struct mutex lock; /* Protects virtqueue operation */
 	struct gpio_chip gc;
-	struct virtio_gpio_config config;
 	struct virtio_gpio_line *lines;
 	struct virtqueue *request_vq;
 };
@@ -57,7 +56,7 @@  static int _virtio_gpio_req(struct virtio_gpio *vgpio, u16 type, u16 gpio,
 
 	req->type = cpu_to_le16(type);
 	req->gpio = cpu_to_le16(gpio);
-	req->value = txvalue;
+	req->value = cpu_to_le32(txvalue);
 
 	sg_init_one(&req_sg, req, sizeof(*req));
 	sg_init_one(&res_sg, res, rxlen);
@@ -233,19 +232,19 @@  static int virtio_gpio_alloc_vqs(struct virtio_gpio *vgpio,
 	return 0;
 }
 
-static const char **virtio_gpio_get_names(struct virtio_gpio *vgpio)
+static const char **virtio_gpio_get_names(struct virtio_gpio *vgpio,
+					  u32 gpio_names_size, u16 ngpio)
 {
-	struct virtio_gpio_config *config = &vgpio->config;
 	struct virtio_gpio_response_get_names *res;
 	struct device *dev = &vgpio->vdev->dev;
 	u8 *gpio_names, *str;
 	const char **names;
 	int i, ret, len;
 
-	if (!config->gpio_names_size)
+	if (!gpio_names_size)
 		return NULL;
 
-	len = sizeof(*res) + config->gpio_names_size;
+	len = sizeof(*res) + gpio_names_size;
 	res = devm_kzalloc(dev, len, GFP_KERNEL);
 	if (!res)
 		return NULL;
@@ -258,18 +257,18 @@  static const char **virtio_gpio_get_names(struct virtio_gpio *vgpio)
 		return NULL;
 	}
 
-	names = devm_kcalloc(dev, config->ngpio, sizeof(*names), GFP_KERNEL);
+	names = devm_kcalloc(dev, ngpio, sizeof(*names), GFP_KERNEL);
 	if (!names)
 		return NULL;
 
 	/* NULL terminate the string instead of checking it */
-	gpio_names[config->gpio_names_size - 1] = '\0';
+	gpio_names[gpio_names_size - 1] = '\0';
 
-	for (i = 0, str = gpio_names; i < config->ngpio; i++) {
+	for (i = 0, str = gpio_names; i < ngpio; i++) {
 		names[i] = str;
 		str += strlen(str) + 1; /* zero-length strings are allowed */
 
-		if (str > gpio_names + config->gpio_names_size) {
+		if (str > gpio_names + gpio_names_size) {
 			dev_err(dev, "gpio_names block is too short (%d)\n", i);
 			return NULL;
 		}
@@ -280,31 +279,31 @@  static const char **virtio_gpio_get_names(struct virtio_gpio *vgpio)
 
 static int virtio_gpio_probe(struct virtio_device *vdev)
 {
-	struct virtio_gpio_config *config;
+	struct virtio_gpio_config config;
 	struct device *dev = &vdev->dev;
 	struct virtio_gpio *vgpio;
+	u32 gpio_names_size;
+	u16 ngpio;
 	int ret, i;
 
 	vgpio = devm_kzalloc(dev, sizeof(*vgpio), GFP_KERNEL);
 	if (!vgpio)
 		return -ENOMEM;
 
-	config = &vgpio->config;
-
 	/* Read configuration */
-	virtio_cread_bytes(vdev, 0, config, sizeof(*config));
-	config->gpio_names_size = le32_to_cpu(config->gpio_names_size);
-	config->ngpio = le16_to_cpu(config->ngpio);
-	if (!config->ngpio) {
+	virtio_cread_bytes(vdev, 0, &config, sizeof(config));
+	gpio_names_size = le32_to_cpu(config.gpio_names_size);
+	ngpio = le16_to_cpu(config.ngpio);
+	if (!ngpio) {
 		dev_err(dev, "Number of GPIOs can't be zero\n");
 		return -EINVAL;
 	}
 
-	vgpio->lines = devm_kcalloc(dev, config->ngpio, sizeof(*vgpio->lines), GFP_KERNEL);
+	vgpio->lines = devm_kcalloc(dev, ngpio, sizeof(*vgpio->lines), GFP_KERNEL);
 	if (!vgpio->lines)
 		return -ENOMEM;
 
-	for (i = 0; i < config->ngpio; i++) {
+	for (i = 0; i < ngpio; i++) {
 		mutex_init(&vgpio->lines[i].lock);
 		init_completion(&vgpio->lines[i].completion);
 	}
@@ -319,7 +318,7 @@  static int virtio_gpio_probe(struct virtio_device *vdev)
 	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.ngpio			= ngpio;
 	vgpio->gc.base			= -1; /* Allocate base dynamically */
 	vgpio->gc.label			= dev_name(dev);
 	vgpio->gc.parent		= dev;
@@ -333,7 +332,7 @@  static int virtio_gpio_probe(struct virtio_device *vdev)
 	/* Mark the device ready to perform operations from within probe() */
 	virtio_device_ready(vdev);
 
-	vgpio->gc.names = virtio_gpio_get_names(vgpio);
+	vgpio->gc.names = virtio_gpio_get_names(vgpio, gpio_names_size, ngpio);
 
 	ret = gpiochip_add_data(&vgpio->gc, vgpio);
 	if (ret) {
diff --git a/include/uapi/linux/virtio_gpio.h b/include/uapi/linux/virtio_gpio.h
index 844574acf095..0445f905d8cc 100644
--- a/include/uapi/linux/virtio_gpio.h
+++ b/include/uapi/linux/virtio_gpio.h
@@ -22,16 +22,16 @@ 
 #define VIRTIO_GPIO_DIRECTION_IN		0x02
 
 struct virtio_gpio_config {
-	__u16 ngpio;
+	__le16 ngpio;
 	__u8 padding[2];
-	__u32 gpio_names_size;
+	__le32 gpio_names_size;
 } __packed;
 
 /* Virtio GPIO Request / Response */
 struct virtio_gpio_request {
-	__u16 type;
-	__u16 gpio;
-	__u32 value;
+	__le16 type;
+	__le16 gpio;
+	__le32 value;
 };
 
 struct virtio_gpio_response {