gpiolib: fix compiling when CONFIG_GPIO_CDEV_V1 is not defined

Message ID 20221202140454.273333-1-vincent@systemli.org
State New
Headers
Series gpiolib: fix compiling when CONFIG_GPIO_CDEV_V1 is not defined |

Commit Message

Nick Dec. 2, 2022, 2:04 p.m. UTC
  If CONFIG_GPIO_CDEV_V1 is not defined compiling will fail with:

drivers/gpio/gpiolib-cdev.c: In function 'linereq_ioctl':
drivers/gpio/gpiolib-cdev.c:1468:16: error: implicit declaration of
 function 'call_ioctl_locked' [-Werror=implicit-function-declaration]
 1468 |         return call_ioctl_locked(file, cmd, arg, lr->gdev,
      |                ^~~~~~~~~~~~~~~~~
drivers/gpio/gpiolib-cdev.c: In function 'linereq_poll':
drivers/gpio/gpiolib-cdev.c:1503:16: error: implicit declaration of
 function 'call_poll_locked'; did you mean 'wake_up_all_locked'?
 [-Werror=implicit-function-declaration]
 1503 |         return call_poll_locked(file, wait, lr->gdev,
                                      linereq_poll_unlocked);
      |                ^~~~~~~~~~~~~~~~
      |                wake_up_all_locked
drivers/gpio/gpiolib-cdev.c: In function 'linereq_read':
drivers/gpio/gpiolib-cdev.c:1566:16: error: implicit declaration of
 function 'call_read_locked'; did you mean 'xa_head_locked'?
 [-Werror=implicit-function-declaration]
 1566 |         return call_read_locked(file, buf, count, f_ps, lr->gdev,
      |                ^~~~~~~~~~~~~~~~
      |                xa_head_locked

Move "call_poll_locked", "call_ioctl_locked", "call_read_locked" and
the necessary typedefs "poll_fn", "ioctl_fn", "read_fn" in front of the
ifdef-statement checking CONFIG_GPIO_CDEV_V1.

Fixes: 98d8b93c6171 ("gpiolib: protect the GPIO device against being dropped while in use by user-space")

Signed-off-by: Nick Hainke <vincent@systemli.org>
---
 drivers/gpio/gpiolib-cdev.c | 50 ++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 25 deletions(-)
  

Comments

Andy Shevchenko Dec. 2, 2022, 2:30 p.m. UTC | #1
On Fri, Dec 02, 2022 at 03:04:54PM +0100, Nick Hainke wrote:

Thank you for the report!

I believe Bart needs to fold it into his patch / PR, because
it's not good to send a broken PR to Linus.

> If CONFIG_GPIO_CDEV_V1 is not defined compiling will fail with:
> 
> drivers/gpio/gpiolib-cdev.c: In function 'linereq_ioctl':
> drivers/gpio/gpiolib-cdev.c:1468:16: error: implicit declaration of
>  function 'call_ioctl_locked' [-Werror=implicit-function-declaration]
>  1468 |         return call_ioctl_locked(file, cmd, arg, lr->gdev,
>       |                ^~~~~~~~~~~~~~~~~
> drivers/gpio/gpiolib-cdev.c: In function 'linereq_poll':
> drivers/gpio/gpiolib-cdev.c:1503:16: error: implicit declaration of
>  function 'call_poll_locked'; did you mean 'wake_up_all_locked'?
>  [-Werror=implicit-function-declaration]
>  1503 |         return call_poll_locked(file, wait, lr->gdev,
>                                       linereq_poll_unlocked);
>       |                ^~~~~~~~~~~~~~~~
>       |                wake_up_all_locked
> drivers/gpio/gpiolib-cdev.c: In function 'linereq_read':
> drivers/gpio/gpiolib-cdev.c:1566:16: error: implicit declaration of
>  function 'call_read_locked'; did you mean 'xa_head_locked'?
>  [-Werror=implicit-function-declaration]
>  1566 |         return call_read_locked(file, buf, count, f_ps, lr->gdev,
>       |                ^~~~~~~~~~~~~~~~
>       |                xa_head_locked

Above is a bit too noisy for the commit message. It can be trimmed 3x times.

> Move "call_poll_locked", "call_ioctl_locked", "call_read_locked" and
> the necessary typedefs "poll_fn", "ioctl_fn", "read_fn" in front of the
> ifdef-statement checking CONFIG_GPIO_CDEV_V1.

> Fixes: 98d8b93c6171 ("gpiolib: protect the GPIO device against being dropped while in use by user-space")
> 
> Signed-off-by: Nick Hainke <vincent@systemli.org>

Mustn't be blank line(s) in the tag block.
  
Andy Shevchenko Dec. 2, 2022, 2:33 p.m. UTC | #2
On Fri, Dec 02, 2022 at 04:30:33PM +0200, Andy Shevchenko wrote:
> On Fri, Dec 02, 2022 at 03:04:54PM +0100, Nick Hainke wrote:

...


> > drivers/gpio/gpiolib-cdev.c:1468:16: error: implicit declaration of
> >  function 'call_ioctl_locked' [-Werror=implicit-function-declaration]

> > drivers/gpio/gpiolib-cdev.c:1503:16: error: implicit declaration of
> >  function 'call_poll_locked'; did you mean 'wake_up_all_locked'?
> >  [-Werror=implicit-function-declaration]

> > drivers/gpio/gpiolib-cdev.c:1566:16: error: implicit declaration of
> >  function 'call_read_locked'; did you mean 'xa_head_locked'?
> >  [-Werror=implicit-function-declaration]

> Above is a bit too noisy for the commit message. It can be trimmed 3x times.

To be precise leave only the above and make them as is (i.e. unwrapped even
if they are so long), because it's a citation from the compiler.
  
Bartosz Golaszewski Dec. 2, 2022, 2:48 p.m. UTC | #3
On Fri, Dec 2, 2022 at 3:30 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Fri, Dec 02, 2022 at 03:04:54PM +0100, Nick Hainke wrote:
>
> Thank you for the report!
>
> I believe Bart needs to fold it into his patch / PR, because
> it's not good to send a broken PR to Linus.
>

Ah cr*p, I saw today's next build went fine, so I assumed the patch
was good to go and sent the PR.

Yeah, if Nick is fine with that, I'd like to squash it with the
offending commit or we'll break bisectability.

Thanks for the report.

Bartosz
  
Bartosz Golaszewski Dec. 2, 2022, 2:55 p.m. UTC | #4
On Fri, Dec 2, 2022 at 3:06 PM Nick Hainke <vincent@systemli.org> wrote:
>
> If CONFIG_GPIO_CDEV_V1 is not defined compiling will fail with:
>
> drivers/gpio/gpiolib-cdev.c: In function 'linereq_ioctl':
> drivers/gpio/gpiolib-cdev.c:1468:16: error: implicit declaration of
>  function 'call_ioctl_locked' [-Werror=implicit-function-declaration]
>  1468 |         return call_ioctl_locked(file, cmd, arg, lr->gdev,
>       |                ^~~~~~~~~~~~~~~~~
> drivers/gpio/gpiolib-cdev.c: In function 'linereq_poll':
> drivers/gpio/gpiolib-cdev.c:1503:16: error: implicit declaration of
>  function 'call_poll_locked'; did you mean 'wake_up_all_locked'?
>  [-Werror=implicit-function-declaration]
>  1503 |         return call_poll_locked(file, wait, lr->gdev,
>                                       linereq_poll_unlocked);
>       |                ^~~~~~~~~~~~~~~~
>       |                wake_up_all_locked
> drivers/gpio/gpiolib-cdev.c: In function 'linereq_read':
> drivers/gpio/gpiolib-cdev.c:1566:16: error: implicit declaration of
>  function 'call_read_locked'; did you mean 'xa_head_locked'?
>  [-Werror=implicit-function-declaration]
>  1566 |         return call_read_locked(file, buf, count, f_ps, lr->gdev,
>       |                ^~~~~~~~~~~~~~~~
>       |                xa_head_locked
>
> Move "call_poll_locked", "call_ioctl_locked", "call_read_locked" and
> the necessary typedefs "poll_fn", "ioctl_fn", "read_fn" in front of the
> ifdef-statement checking CONFIG_GPIO_CDEV_V1.
>
> Fixes: 98d8b93c6171 ("gpiolib: protect the GPIO device against being dropped while in use by user-space")
>
> Signed-off-by: Nick Hainke <vincent@systemli.org>
> ---
>  drivers/gpio/gpiolib-cdev.c | 50 ++++++++++++++++++-------------------
>  1 file changed, 25 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
> index e8079c1d2e1b..067e26a00775 100644
> --- a/drivers/gpio/gpiolib-cdev.c
> +++ b/drivers/gpio/gpiolib-cdev.c
> @@ -61,31 +61,6 @@ static_assert(IS_ALIGNED(sizeof(struct gpio_v2_line_values), 8));
>   * GPIO line handle management
>   */

This comment refers to the below code so should be moved too. Don't
worry about it, I'll resend my series with that included.

Bartosz

>
> -#ifdef CONFIG_GPIO_CDEV_V1
> -/**
> - * struct linehandle_state - contains the state of a userspace handle
> - * @gdev: the GPIO device the handle pertains to
> - * @label: consumer label used to tag descriptors
> - * @descs: the GPIO descriptors held by this handle
> - * @num_descs: the number of descriptors held in the descs array
> - */
> -struct linehandle_state {
> -       struct gpio_device *gdev;
> -       const char *label;
> -       struct gpio_desc *descs[GPIOHANDLES_MAX];
> -       u32 num_descs;
> -};
> -
> -#define GPIOHANDLE_REQUEST_VALID_FLAGS \
> -       (GPIOHANDLE_REQUEST_INPUT | \
> -       GPIOHANDLE_REQUEST_OUTPUT | \
> -       GPIOHANDLE_REQUEST_ACTIVE_LOW | \
> -       GPIOHANDLE_REQUEST_BIAS_PULL_UP | \
> -       GPIOHANDLE_REQUEST_BIAS_PULL_DOWN | \
> -       GPIOHANDLE_REQUEST_BIAS_DISABLE | \
> -       GPIOHANDLE_REQUEST_OPEN_DRAIN | \
> -       GPIOHANDLE_REQUEST_OPEN_SOURCE)
> -
>  typedef __poll_t (*poll_fn)(struct file *, struct poll_table_struct *);
>  typedef long (*ioctl_fn)(struct file *, unsigned int, unsigned long);
>  typedef ssize_t (*read_fn)(struct file *, char __user *,
> @@ -133,6 +108,31 @@ static ssize_t call_read_locked(struct file *file, char __user *buf,
>         return ret;
>  }
>
> +#ifdef CONFIG_GPIO_CDEV_V1
> +/**
> + * struct linehandle_state - contains the state of a userspace handle
> + * @gdev: the GPIO device the handle pertains to
> + * @label: consumer label used to tag descriptors
> + * @descs: the GPIO descriptors held by this handle
> + * @num_descs: the number of descriptors held in the descs array
> + */
> +struct linehandle_state {
> +       struct gpio_device *gdev;
> +       const char *label;
> +       struct gpio_desc *descs[GPIOHANDLES_MAX];
> +       u32 num_descs;
> +};
> +
> +#define GPIOHANDLE_REQUEST_VALID_FLAGS \
> +       (GPIOHANDLE_REQUEST_INPUT | \
> +       GPIOHANDLE_REQUEST_OUTPUT | \
> +       GPIOHANDLE_REQUEST_ACTIVE_LOW | \
> +       GPIOHANDLE_REQUEST_BIAS_PULL_UP | \
> +       GPIOHANDLE_REQUEST_BIAS_PULL_DOWN | \
> +       GPIOHANDLE_REQUEST_BIAS_DISABLE | \
> +       GPIOHANDLE_REQUEST_OPEN_DRAIN | \
> +       GPIOHANDLE_REQUEST_OPEN_SOURCE)
> +
>  static int linehandle_validate_flags(u32 flags)
>  {
>         /* Return an error if an unknown flag is set */
> --
> 2.38.1
>
  
Nick Dec. 2, 2022, 4:37 p.m. UTC | #5
On 12/2/22 15:48, Bartosz Golaszewski wrote:

> Yeah, if Nick is fine with that, I'd like to squash it with the
> offending commit or we'll break bisectability.
Go for it. :)
  

Patch

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index e8079c1d2e1b..067e26a00775 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -61,31 +61,6 @@  static_assert(IS_ALIGNED(sizeof(struct gpio_v2_line_values), 8));
  * GPIO line handle management
  */
 
-#ifdef CONFIG_GPIO_CDEV_V1
-/**
- * struct linehandle_state - contains the state of a userspace handle
- * @gdev: the GPIO device the handle pertains to
- * @label: consumer label used to tag descriptors
- * @descs: the GPIO descriptors held by this handle
- * @num_descs: the number of descriptors held in the descs array
- */
-struct linehandle_state {
-	struct gpio_device *gdev;
-	const char *label;
-	struct gpio_desc *descs[GPIOHANDLES_MAX];
-	u32 num_descs;
-};
-
-#define GPIOHANDLE_REQUEST_VALID_FLAGS \
-	(GPIOHANDLE_REQUEST_INPUT | \
-	GPIOHANDLE_REQUEST_OUTPUT | \
-	GPIOHANDLE_REQUEST_ACTIVE_LOW | \
-	GPIOHANDLE_REQUEST_BIAS_PULL_UP | \
-	GPIOHANDLE_REQUEST_BIAS_PULL_DOWN | \
-	GPIOHANDLE_REQUEST_BIAS_DISABLE | \
-	GPIOHANDLE_REQUEST_OPEN_DRAIN | \
-	GPIOHANDLE_REQUEST_OPEN_SOURCE)
-
 typedef __poll_t (*poll_fn)(struct file *, struct poll_table_struct *);
 typedef long (*ioctl_fn)(struct file *, unsigned int, unsigned long);
 typedef ssize_t (*read_fn)(struct file *, char __user *,
@@ -133,6 +108,31 @@  static ssize_t call_read_locked(struct file *file, char __user *buf,
 	return ret;
 }
 
+#ifdef CONFIG_GPIO_CDEV_V1
+/**
+ * struct linehandle_state - contains the state of a userspace handle
+ * @gdev: the GPIO device the handle pertains to
+ * @label: consumer label used to tag descriptors
+ * @descs: the GPIO descriptors held by this handle
+ * @num_descs: the number of descriptors held in the descs array
+ */
+struct linehandle_state {
+	struct gpio_device *gdev;
+	const char *label;
+	struct gpio_desc *descs[GPIOHANDLES_MAX];
+	u32 num_descs;
+};
+
+#define GPIOHANDLE_REQUEST_VALID_FLAGS \
+	(GPIOHANDLE_REQUEST_INPUT | \
+	GPIOHANDLE_REQUEST_OUTPUT | \
+	GPIOHANDLE_REQUEST_ACTIVE_LOW | \
+	GPIOHANDLE_REQUEST_BIAS_PULL_UP | \
+	GPIOHANDLE_REQUEST_BIAS_PULL_DOWN | \
+	GPIOHANDLE_REQUEST_BIAS_DISABLE | \
+	GPIOHANDLE_REQUEST_OPEN_DRAIN | \
+	GPIOHANDLE_REQUEST_OPEN_SOURCE)
+
 static int linehandle_validate_flags(u32 flags)
 {
 	/* Return an error if an unknown flag is set */