[libgpiod][PATCH] core: remove buggy flags sanitization from line-config

[libgpiod][PATCH] core: remove buggy flags sanitization from line-config

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

We try to drop potentially set output flags from line config if edge
detection is enabled but we use the library enum instead of the one from
the uAPI. In any case, we should actually loudly complain if user tries
to use the output mode with edge-detection (like we do currently) so just
remove offending lines entirely.

Reported-by: Anne Bezemer <j.a.bezemer@opensourcepartners.nl>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 lib/line-config.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/lib/line-config.c b/lib/line-config.c
index 2749a2a..9302c1b 100644
--- a/lib/line-config.c
+++ b/lib/line-config.c
@@ -381,18 +381,15 @@ static uint64_t make_kernel_flags(struct gpiod_line_settings *settings)
 	case GPIOD_LINE_EDGE_FALLING:
 		flags |= (GPIO_V2_LINE_FLAG_EDGE_FALLING |
 			  GPIO_V2_LINE_FLAG_INPUT);
-		flags &= ~GPIOD_LINE_DIRECTION_OUTPUT;
 		break;
 	case GPIOD_LINE_EDGE_RISING:
 		flags |= (GPIO_V2_LINE_FLAG_EDGE_RISING |
 			  GPIO_V2_LINE_FLAG_INPUT);
-		flags &= ~GPIOD_LINE_DIRECTION_OUTPUT;
 		break;
 	case GPIOD_LINE_EDGE_BOTH:
 		flags |= (GPIO_V2_LINE_FLAG_EDGE_FALLING |
 			  GPIO_V2_LINE_FLAG_EDGE_RISING |
 			  GPIO_V2_LINE_FLAG_INPUT);
-		flags &= ~GPIOD_LINE_DIRECTION_OUTPUT;
 		break;
 	default:
 		break;
-- 
2.40.1

Re: [libgpiod][PATCH] core: remove buggy flags sanitization from line-config

[Kent Gibson]

On Thu, Jan 04, 2024 at 02:50:58PM +0100, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> We try to drop potentially set output flags from line config if edge
> detection is enabled but we use the library enum instead of the one from
> the uAPI. In any case, we should actually loudly complain if user tries
> to use the output mode with edge-detection (like we do currently) so just
> remove offending lines entirely.
>

I don't see any problem with that.
It also explains why we didn't pick it up earlier - it behaves as
expected and has no visible side effects, so this is just tidying up
dead code.

Reviewed-by: Kent Gibson <warthog618@gmail.com>

> Reported-by: Anne Bezemer <j.a.bezemer@opensourcepartners.nl>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
>  lib/line-config.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/lib/line-config.c b/lib/line-config.c
> index 2749a2a..9302c1b 100644
> --- a/lib/line-config.c
> +++ b/lib/line-config.c
> @@ -381,18 +381,15 @@ static uint64_t make_kernel_flags(struct gpiod_line_settings *settings)
>  	case GPIOD_LINE_EDGE_FALLING:
>  		flags |= (GPIO_V2_LINE_FLAG_EDGE_FALLING |
>  			  GPIO_V2_LINE_FLAG_INPUT);
> -		flags &= ~GPIOD_LINE_DIRECTION_OUTPUT;
>  		break;
>  	case GPIOD_LINE_EDGE_RISING:
>  		flags |= (GPIO_V2_LINE_FLAG_EDGE_RISING |
>  			  GPIO_V2_LINE_FLAG_INPUT);
> -		flags &= ~GPIOD_LINE_DIRECTION_OUTPUT;
>  		break;
>  	case GPIOD_LINE_EDGE_BOTH:
>  		flags |= (GPIO_V2_LINE_FLAG_EDGE_FALLING |
>  			  GPIO_V2_LINE_FLAG_EDGE_RISING |
>  			  GPIO_V2_LINE_FLAG_INPUT);
> -		flags &= ~GPIOD_LINE_DIRECTION_OUTPUT;
>  		break;
>  	default:
>  		break;
> --
> 2.40.1
>

Re: [libgpiod][PATCH] core: remove buggy flags sanitization from line-config

[J.A. Bezemer]

On Thu, 4 Jan 2024, Kent Gibson wrote:
> On Thu, Jan 04, 2024 at 02:50:58PM +0100, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > We try to drop potentially set output flags from line config if edge
> > detection is enabled but we use the library enum instead of the one from
> > the uAPI. In any case, we should actually loudly complain if user tries
> > to use the output mode with edge-detection (like we do currently) so just
> > remove offending lines entirely.
> >
> 
> I don't see any problem with that.
> It also explains why we didn't pick it up earlier - it behaves as
> expected and has no visible side effects, so this is just tidying up
> dead code.
> 
> Reviewed-by: Kent Gibson <warthog618@gmail.com>

For the record, this is fine with me too.

Thanks,
Anne Bezemer

Reviewed-by: Anne Bezemer <j.a.bezemer@opensourcepartners.nl>

> > Reported-by: Anne Bezemer <j.a.bezemer@opensourcepartners.nl>
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > ---
> >  lib/line-config.c | 3 ---
> >  1 file changed, 3 deletions(-)
> >
> > diff --git a/lib/line-config.c b/lib/line-config.c
> > index 2749a2a..9302c1b 100644
> > --- a/lib/line-config.c
> > +++ b/lib/line-config.c
> > @@ -381,18 +381,15 @@ static uint64_t make_kernel_flags(struct gpiod_line_settings *settings)
> >  	case GPIOD_LINE_EDGE_FALLING:
> >  		flags |= (GPIO_V2_LINE_FLAG_EDGE_FALLING |
> >  			  GPIO_V2_LINE_FLAG_INPUT);
> > -		flags &= ~GPIOD_LINE_DIRECTION_OUTPUT;
> >  		break;
> >  	case GPIOD_LINE_EDGE_RISING:
> >  		flags |= (GPIO_V2_LINE_FLAG_EDGE_RISING |
> >  			  GPIO_V2_LINE_FLAG_INPUT);
> > -		flags &= ~GPIOD_LINE_DIRECTION_OUTPUT;
> >  		break;
> >  	case GPIOD_LINE_EDGE_BOTH:
> >  		flags |= (GPIO_V2_LINE_FLAG_EDGE_FALLING |
> >  			  GPIO_V2_LINE_FLAG_EDGE_RISING |
> >  			  GPIO_V2_LINE_FLAG_INPUT);
> > -		flags &= ~GPIOD_LINE_DIRECTION_OUTPUT;
> >  		break;
> >  	default:
> >  		break;
> > --
> > 2.40.1
> >

Re: [libgpiod][PATCH] core: remove buggy flags sanitization from line-config

[Bartosz Golaszewski]

On Thu, Jan 4, 2024 at 2:51 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> We try to drop potentially set output flags from line config if edge
> detection is enabled but we use the library enum instead of the one from
> the uAPI. In any case, we should actually loudly complain if user tries
> to use the output mode with edge-detection (like we do currently) so just
> remove offending lines entirely.
>
> Reported-by: Anne Bezemer <j.a.bezemer@opensourcepartners.nl>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
>  lib/line-config.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/lib/line-config.c b/lib/line-config.c
> index 2749a2a..9302c1b 100644
> --- a/lib/line-config.c
> +++ b/lib/line-config.c
> @@ -381,18 +381,15 @@ static uint64_t make_kernel_flags(struct gpiod_line_settings *settings)
>         case GPIOD_LINE_EDGE_FALLING:
>                 flags |= (GPIO_V2_LINE_FLAG_EDGE_FALLING |
>                           GPIO_V2_LINE_FLAG_INPUT);
> -               flags &= ~GPIOD_LINE_DIRECTION_OUTPUT;
>                 break;
>         case GPIOD_LINE_EDGE_RISING:
>                 flags |= (GPIO_V2_LINE_FLAG_EDGE_RISING |
>                           GPIO_V2_LINE_FLAG_INPUT);
> -               flags &= ~GPIOD_LINE_DIRECTION_OUTPUT;
>                 break;
>         case GPIOD_LINE_EDGE_BOTH:
>                 flags |= (GPIO_V2_LINE_FLAG_EDGE_FALLING |
>                           GPIO_V2_LINE_FLAG_EDGE_RISING |
>                           GPIO_V2_LINE_FLAG_INPUT);
> -               flags &= ~GPIOD_LINE_DIRECTION_OUTPUT;
>                 break;
>         default:
>                 break;
> --
> 2.40.1
>

Patch applied.

Bart