Re: [PATCHv5 1/2] gpio: sch: Consolidate similar algorithms
On Thu, Jan 15, 2015 at 6:49 PM, Linus Walleij wrote:
> On Mon, Dec 8, 2014 at 10:38 AM, Chang Rebecca Swee Fun
> wrote:
>
>> Consolidating similar algorithms into common functions to make
>> GPIO SCH simpler and manageable.
>>
>> Signed-off-by: Chang Rebecca Swee Fun
>> Reviewed-by: Mika Westerberg
>> Reviewed-by: Alexandre Courbot
>
> I have removed this patch from the tree. It breaks completely
> in build and looks strange:
>
>> +static void sch_gpio_reg_set(struct gpio_chip *gc, unsigned gpio, unsigned
>> reg,
>> +int val)
>
> Takes struct gpio_chip * as argument...
>
>> +static int sch_gpio_direction_in(struct gpio_chip *gc, unsigned gpio_num)
>> +{
>> + struct sch_gpio *sch = to_sch_gpio(gc);
>>
>> + spin_lock(>lock);
>> + sch_gpio_reg_set(sch, gpio_num, GIO, 1);
>
> Passes something else as argument.
>
>> static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num, int val)
>> {
>> struct sch_gpio *sch = to_sch_gpio(gc);
>> - u8 curr_vals;
>> - unsigned short offset, bit;
>>
>> spin_lock(>lock);
>> -
>> - offset = sch_gpio_offset(sch, gpio_num, GLV);
>> - bit = sch_gpio_bit(sch, gpio_num);
>> -
>> - curr_vals = inb(sch->iobase + offset);
>> -
>> - if (val)
>> - outb(curr_vals | (1 << bit), sch->iobase + offset);
>> - else
>> - outb((curr_vals & ~(1 << bit)), sch->iobase + offset);
>> -
>> + sch_gpio_reg_set(gc, gpio_num, GLV, val);
>
> Here it is correct.
>
>> @@ -139,18 +123,9 @@ static int sch_gpio_direction_out(struct gpio_chip *gc,
>> unsigned gpio_num,
>> int val)
>> {
>> struct sch_gpio *sch = to_sch_gpio(gc);
>> - u8 curr_dirs;
>> - unsigned short offset, bit;
>>
>> spin_lock(>lock);
>> -
>> - offset = sch_gpio_offset(sch, gpio_num, GIO);
>> - bit = sch_gpio_bit(sch, gpio_num);
>> -
>> - curr_dirs = inb(sch->iobase + offset);
>> - if (curr_dirs & (1 << bit))
>> - outb(curr_dirs & ~(1 << bit), sch->iobase + offset);
>> -
>> + sch_gpio_reg_set(sch, gpio_num, GIO, 0);
>
> Wrong again. Etc.
>
> Makes me suspect that this patch is not tested at all, so dropped.
IIRC at least one of the previous versions had the same issue and at
that time I already pointed out that *compiling and testing* patches
before sending them in the wild is common sense.
Ironically the cover letter says "The patches has been verifed and
tested working on Galileo Board". One may wonder...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv5 1/2] gpio: sch: Consolidate similar algorithms
On Mon, Dec 8, 2014 at 10:38 AM, Chang Rebecca Swee Fun
wrote:
> Consolidating similar algorithms into common functions to make
> GPIO SCH simpler and manageable.
>
> Signed-off-by: Chang Rebecca Swee Fun
> Reviewed-by: Mika Westerberg
> Reviewed-by: Alexandre Courbot
I have removed this patch from the tree. It breaks completely
in build and looks strange:
> +static void sch_gpio_reg_set(struct gpio_chip *gc, unsigned gpio, unsigned
> reg,
> +int val)
Takes struct gpio_chip * as argument...
> +static int sch_gpio_direction_in(struct gpio_chip *gc, unsigned gpio_num)
> +{
> + struct sch_gpio *sch = to_sch_gpio(gc);
>
> + spin_lock(>lock);
> + sch_gpio_reg_set(sch, gpio_num, GIO, 1);
Passes something else as argument.
> static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num, int val)
> {
> struct sch_gpio *sch = to_sch_gpio(gc);
> - u8 curr_vals;
> - unsigned short offset, bit;
>
> spin_lock(>lock);
> -
> - offset = sch_gpio_offset(sch, gpio_num, GLV);
> - bit = sch_gpio_bit(sch, gpio_num);
> -
> - curr_vals = inb(sch->iobase + offset);
> -
> - if (val)
> - outb(curr_vals | (1 << bit), sch->iobase + offset);
> - else
> - outb((curr_vals & ~(1 << bit)), sch->iobase + offset);
> -
> + sch_gpio_reg_set(gc, gpio_num, GLV, val);
Here it is correct.
> @@ -139,18 +123,9 @@ static int sch_gpio_direction_out(struct gpio_chip *gc,
> unsigned gpio_num,
> int val)
> {
> struct sch_gpio *sch = to_sch_gpio(gc);
> - u8 curr_dirs;
> - unsigned short offset, bit;
>
> spin_lock(>lock);
> -
> - offset = sch_gpio_offset(sch, gpio_num, GIO);
> - bit = sch_gpio_bit(sch, gpio_num);
> -
> - curr_dirs = inb(sch->iobase + offset);
> - if (curr_dirs & (1 << bit))
> - outb(curr_dirs & ~(1 << bit), sch->iobase + offset);
> -
> + sch_gpio_reg_set(sch, gpio_num, GIO, 0);
Wrong again. Etc.
Makes me suspect that this patch is not tested at all, so dropped.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv5 1/2] gpio: sch: Consolidate similar algorithms
On Mon, Dec 8, 2014 at 10:38 AM, Chang Rebecca Swee Fun
rebecca.swee.fun.ch...@intel.com wrote:
Consolidating similar algorithms into common functions to make
GPIO SCH simpler and manageable.
Signed-off-by: Chang Rebecca Swee Fun rebecca.swee.fun.ch...@intel.com
Reviewed-by: Mika Westerberg mika.westerb...@linux.intel.com
Reviewed-by: Alexandre Courbot acour...@nvidia.com
I have removed this patch from the tree. It breaks completely
in build and looks strange:
+static void sch_gpio_reg_set(struct gpio_chip *gc, unsigned gpio, unsigned
reg,
+int val)
Takes struct gpio_chip * as argument...
+static int sch_gpio_direction_in(struct gpio_chip *gc, unsigned gpio_num)
+{
+ struct sch_gpio *sch = to_sch_gpio(gc);
+ spin_lock(sch-lock);
+ sch_gpio_reg_set(sch, gpio_num, GIO, 1);
Passes something else as argument.
static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num, int val)
{
struct sch_gpio *sch = to_sch_gpio(gc);
- u8 curr_vals;
- unsigned short offset, bit;
spin_lock(sch-lock);
-
- offset = sch_gpio_offset(sch, gpio_num, GLV);
- bit = sch_gpio_bit(sch, gpio_num);
-
- curr_vals = inb(sch-iobase + offset);
-
- if (val)
- outb(curr_vals | (1 bit), sch-iobase + offset);
- else
- outb((curr_vals ~(1 bit)), sch-iobase + offset);
-
+ sch_gpio_reg_set(gc, gpio_num, GLV, val);
Here it is correct.
@@ -139,18 +123,9 @@ static int sch_gpio_direction_out(struct gpio_chip *gc,
unsigned gpio_num,
int val)
{
struct sch_gpio *sch = to_sch_gpio(gc);
- u8 curr_dirs;
- unsigned short offset, bit;
spin_lock(sch-lock);
-
- offset = sch_gpio_offset(sch, gpio_num, GIO);
- bit = sch_gpio_bit(sch, gpio_num);
-
- curr_dirs = inb(sch-iobase + offset);
- if (curr_dirs (1 bit))
- outb(curr_dirs ~(1 bit), sch-iobase + offset);
-
+ sch_gpio_reg_set(sch, gpio_num, GIO, 0);
Wrong again. Etc.
Makes me suspect that this patch is not tested at all, so dropped.
Yours,
Linus Walleij
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv5 1/2] gpio: sch: Consolidate similar algorithms
On Thu, Jan 15, 2015 at 6:49 PM, Linus Walleij linus.wall...@linaro.org wrote:
On Mon, Dec 8, 2014 at 10:38 AM, Chang Rebecca Swee Fun
rebecca.swee.fun.ch...@intel.com wrote:
Consolidating similar algorithms into common functions to make
GPIO SCH simpler and manageable.
Signed-off-by: Chang Rebecca Swee Fun rebecca.swee.fun.ch...@intel.com
Reviewed-by: Mika Westerberg mika.westerb...@linux.intel.com
Reviewed-by: Alexandre Courbot acour...@nvidia.com
I have removed this patch from the tree. It breaks completely
in build and looks strange:
+static void sch_gpio_reg_set(struct gpio_chip *gc, unsigned gpio, unsigned
reg,
+int val)
Takes struct gpio_chip * as argument...
+static int sch_gpio_direction_in(struct gpio_chip *gc, unsigned gpio_num)
+{
+ struct sch_gpio *sch = to_sch_gpio(gc);
+ spin_lock(sch-lock);
+ sch_gpio_reg_set(sch, gpio_num, GIO, 1);
Passes something else as argument.
static void sch_gpio_set(struct gpio_chip *gc, unsigned gpio_num, int val)
{
struct sch_gpio *sch = to_sch_gpio(gc);
- u8 curr_vals;
- unsigned short offset, bit;
spin_lock(sch-lock);
-
- offset = sch_gpio_offset(sch, gpio_num, GLV);
- bit = sch_gpio_bit(sch, gpio_num);
-
- curr_vals = inb(sch-iobase + offset);
-
- if (val)
- outb(curr_vals | (1 bit), sch-iobase + offset);
- else
- outb((curr_vals ~(1 bit)), sch-iobase + offset);
-
+ sch_gpio_reg_set(gc, gpio_num, GLV, val);
Here it is correct.
@@ -139,18 +123,9 @@ static int sch_gpio_direction_out(struct gpio_chip *gc,
unsigned gpio_num,
int val)
{
struct sch_gpio *sch = to_sch_gpio(gc);
- u8 curr_dirs;
- unsigned short offset, bit;
spin_lock(sch-lock);
-
- offset = sch_gpio_offset(sch, gpio_num, GIO);
- bit = sch_gpio_bit(sch, gpio_num);
-
- curr_dirs = inb(sch-iobase + offset);
- if (curr_dirs (1 bit))
- outb(curr_dirs ~(1 bit), sch-iobase + offset);
-
+ sch_gpio_reg_set(sch, gpio_num, GIO, 0);
Wrong again. Etc.
Makes me suspect that this patch is not tested at all, so dropped.
IIRC at least one of the previous versions had the same issue and at
that time I already pointed out that *compiling and testing* patches
before sending them in the wild is common sense.
Ironically the cover letter says The patches has been verifed and
tested working on Galileo Board. One may wonder...
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv5 1/2] gpio: sch: Consolidate similar algorithms
On Mon, Dec 8, 2014 at 10:38 AM, Chang Rebecca Swee Fun wrote: > Consolidating similar algorithms into common functions to make > GPIO SCH simpler and manageable. > > Signed-off-by: Chang Rebecca Swee Fun > Reviewed-by: Mika Westerberg > Reviewed-by: Alexandre Courbot Patch applied. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCHv5 1/2] gpio: sch: Consolidate similar algorithms
On Mon, Dec 8, 2014 at 10:38 AM, Chang Rebecca Swee Fun rebecca.swee.fun.ch...@intel.com wrote: Consolidating similar algorithms into common functions to make GPIO SCH simpler and manageable. Signed-off-by: Chang Rebecca Swee Fun rebecca.swee.fun.ch...@intel.com Reviewed-by: Mika Westerberg mika.westerb...@linux.intel.com Reviewed-by: Alexandre Courbot acour...@nvidia.com Patch applied. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
