| Message ID | 1418740800.10278.1.camel@phoenix |
|---|---|
| State | New, archived |
| Headers | show |
On 12/16/2014 04:40 PM, Axel Lin wrote: > dln2_gpio_direction_output() ignored the state passed into it. Fix it. > > Signed-off-by: Axel Lin <axel.lin@ingics.com> Tested-by: Daniel Baluta <daniel.baluta@intel.com> > --- > I don't have this hardware handy, so only compile test. > drivers/gpio/gpio-dln2.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpio/gpio-dln2.c b/drivers/gpio/gpio-dln2.c > index 978b51e..1434844 100644 > --- a/drivers/gpio/gpio-dln2.c > +++ b/drivers/gpio/gpio-dln2.c > @@ -267,6 +267,7 @@ static int dln2_gpio_direction_input(struct gpio_chip *chip, unsigned offset) > static int dln2_gpio_direction_output(struct gpio_chip *chip, unsigned offset, > int value) > { > + dln2_gpio_set(chip, offset, value); > return dln2_gpio_set_direction(chip, offset, DLN2_GPIO_DIRECTION_OUT); > } > > -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Dec 17, 2014 at 1:52 AM, Daniel Baluta <daniel.baluta@intel.com> wrote: > > > On 12/16/2014 04:40 PM, Axel Lin wrote: >> >> dln2_gpio_direction_output() ignored the state passed into it. Fix it. >> >> Signed-off-by: Axel Lin <axel.lin@ingics.com> > > > Tested-by: Daniel Baluta <daniel.baluta@intel.com> Acked-by: Alexandre Courbot <acourbot@nvidia.com> But this seems to apply to patches in mid-flight, could it be squashed there maybe? -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Dec 17, 2014 at 10:50 AM, Alexandre Courbot <gnurou@gmail.com> wrote: > On Wed, Dec 17, 2014 at 1:52 AM, Daniel Baluta <daniel.baluta@intel.com> wrote: >> >> >> On 12/16/2014 04:40 PM, Axel Lin wrote: >>> >>> dln2_gpio_direction_output() ignored the state passed into it. Fix it. >>> >>> Signed-off-by: Axel Lin <axel.lin@ingics.com> >> >> >> Tested-by: Daniel Baluta <daniel.baluta@intel.com> > > Acked-by: Alexandre Courbot <acourbot@nvidia.com> > > But this seems to apply to patches in mid-flight, could it be squashed > there maybe? Sure, I can add it to the existing series, but I prefer to keep it as separate patch. Is that ok with you? -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Dec 17, 2014 at 5:57 PM, Octavian Purdila <octavian.purdila@intel.com> wrote: > On Wed, Dec 17, 2014 at 10:50 AM, Alexandre Courbot <gnurou@gmail.com> wrote: >> On Wed, Dec 17, 2014 at 1:52 AM, Daniel Baluta <daniel.baluta@intel.com> wrote: >>> >>> >>> On 12/16/2014 04:40 PM, Axel Lin wrote: >>>> >>>> dln2_gpio_direction_output() ignored the state passed into it. Fix it. >>>> >>>> Signed-off-by: Axel Lin <axel.lin@ingics.com> >>> >>> >>> Tested-by: Daniel Baluta <daniel.baluta@intel.com> >> >> Acked-by: Alexandre Courbot <acourbot@nvidia.com> >> >> But this seems to apply to patches in mid-flight, could it be squashed >> there maybe? > > Sure, I can add it to the existing series, but I prefer to keep it as > separate patch. Is that ok with you? Why? This is clearly a fix, so if the series is not merged yet, doesn't it make more sense to squash it and have the desired functionality from the start? -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Dec 17, 2014 at 10:59 AM, Alexandre Courbot <gnurou@gmail.com> wrote: > On Wed, Dec 17, 2014 at 5:57 PM, Octavian Purdila > <octavian.purdila@intel.com> wrote: >> On Wed, Dec 17, 2014 at 10:50 AM, Alexandre Courbot <gnurou@gmail.com> wrote: >>> On Wed, Dec 17, 2014 at 1:52 AM, Daniel Baluta <daniel.baluta@intel.com> wrote: >>>> >>>> >>>> On 12/16/2014 04:40 PM, Axel Lin wrote: >>>>> >>>>> dln2_gpio_direction_output() ignored the state passed into it. Fix it. >>>>> >>>>> Signed-off-by: Axel Lin <axel.lin@ingics.com> >>>> >>>> >>>> Tested-by: Daniel Baluta <daniel.baluta@intel.com> >>> >>> Acked-by: Alexandre Courbot <acourbot@nvidia.com> >>> >>> But this seems to apply to patches in mid-flight, could it be squashed >>> there maybe? >> >> Sure, I can add it to the existing series, but I prefer to keep it as >> separate patch. Is that ok with you? > > Why? This is clearly a fix, so if the series is not merged yet, > doesn't it make more sense to squash it and have the desired > functionality from the start? The fix is not for issues introduced by the series, but for an issue existing in the already merged code. Also it is a separate issue then the one fixed in the other patches in the series. AFAIK each fix should be in a separate patch, am I missing something? -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Dec 17, 2014 at 6:10 PM, Octavian Purdila <octavian.purdila@intel.com> wrote: > On Wed, Dec 17, 2014 at 10:59 AM, Alexandre Courbot <gnurou@gmail.com> wrote: >> On Wed, Dec 17, 2014 at 5:57 PM, Octavian Purdila >> <octavian.purdila@intel.com> wrote: >>> On Wed, Dec 17, 2014 at 10:50 AM, Alexandre Courbot <gnurou@gmail.com> wrote: >>>> On Wed, Dec 17, 2014 at 1:52 AM, Daniel Baluta <daniel.baluta@intel.com> wrote: >>>>> >>>>> >>>>> On 12/16/2014 04:40 PM, Axel Lin wrote: >>>>>> >>>>>> dln2_gpio_direction_output() ignored the state passed into it. Fix it. >>>>>> >>>>>> Signed-off-by: Axel Lin <axel.lin@ingics.com> >>>>> >>>>> >>>>> Tested-by: Daniel Baluta <daniel.baluta@intel.com> >>>> >>>> Acked-by: Alexandre Courbot <acourbot@nvidia.com> >>>> >>>> But this seems to apply to patches in mid-flight, could it be squashed >>>> there maybe? >>> >>> Sure, I can add it to the existing series, but I prefer to keep it as >>> separate patch. Is that ok with you? >> >> Why? This is clearly a fix, so if the series is not merged yet, >> doesn't it make more sense to squash it and have the desired >> functionality from the start? > > The fix is not for issues introduced by the series, but for an issue > existing in the already merged code. Also it is a separate issue then > the one fixed in the other patches in the series. Allright, I thought none of the DL2 support has been fixed yet (as I cannot see it in Linus W.'s tree). You know better what the state of your patches is, so please do what you think is best. -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Dec 16, 2014 at 6:52 PM, Daniel Baluta <daniel.baluta@intel.com> wrote: > > > On 12/16/2014 04:40 PM, Axel Lin wrote: Hi Axel, Nice catch! >> >> dln2_gpio_direction_output() ignored the state passed into it. Fix it. >> >> Signed-off-by: Axel Lin <axel.lin@ingics.com> > > > Tested-by: Daniel Baluta <daniel.baluta@intel.com> > >> --- >> I don't have this hardware handy, so only compile test. >> drivers/gpio/gpio-dln2.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/gpio/gpio-dln2.c b/drivers/gpio/gpio-dln2.c >> index 978b51e..1434844 100644 >> --- a/drivers/gpio/gpio-dln2.c >> +++ b/drivers/gpio/gpio-dln2.c >> @@ -267,6 +267,7 @@ static int dln2_gpio_direction_input(struct gpio_chip >> *chip, unsigned offset) >> static int dln2_gpio_direction_output(struct gpio_chip *chip, unsigned >> offset, >> int value) >> { >> + dln2_gpio_set(chip, offset, value); Could you change dln2_gpio_pin_set_out_val's signature to return an error code than use it instead of dln2_gpio_set here? Then we can check the error value. With that: Reviewed-by: Octavian Purdila <octavian.purdila@intel.com> -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/gpio/gpio-dln2.c b/drivers/gpio/gpio-dln2.c index 978b51e..1434844 100644 --- a/drivers/gpio/gpio-dln2.c +++ b/drivers/gpio/gpio-dln2.c @@ -267,6 +267,7 @@ static int dln2_gpio_direction_input(struct gpio_chip *chip, unsigned offset) static int dln2_gpio_direction_output(struct gpio_chip *chip, unsigned offset, int value) { + dln2_gpio_set(chip, offset, value); return dln2_gpio_set_direction(chip, offset, DLN2_GPIO_DIRECTION_OUT); }
dln2_gpio_direction_output() ignored the state passed into it. Fix it. Signed-off-by: Axel Lin <axel.lin@ingics.com> --- I don't have this hardware handy, so only compile test. drivers/gpio/gpio-dln2.c | 1 + 1 file changed, 1 insertion(+)