[PATCH RFT] gpio: dln2: Fix gpio output value in dln2_gpio_direction_output()

[PATCH RFT] gpio: dln2: Fix gpio output value in dln2_gpio_direction_output()

[Axel Lin]

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(+)

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);
 }
 
-- 
1.9.1

Re: [PATCH RFT] gpio: dln2: Fix gpio output value in dln2_gpio_direction_output()

[Daniel Baluta]

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);
>   }
>
>

Re: [PATCH RFT] gpio: dln2: Fix gpio output value in dln2_gpio_direction_output()

[Alexandre Courbot]

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?

Re: [PATCH RFT] gpio: dln2: Fix gpio output value in dln2_gpio_direction_output()

[Octavian Purdila]

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?

Re: [PATCH RFT] gpio: dln2: Fix gpio output value in dln2_gpio_direction_output()

[Alexandre Courbot]

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?

Re: [PATCH RFT] gpio: dln2: Fix gpio output value in dln2_gpio_direction_output()

[Octavian Purdila]

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?

Re: [PATCH RFT] gpio: dln2: Fix gpio output value in dln2_gpio_direction_output()

[Alexandre Courbot]

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.

Re: [PATCH RFT] gpio: dln2: Fix gpio output value in dln2_gpio_direction_output()

[Octavian Purdila]

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>