Em Sat, 13 Apr 2013 19:46:20 +0200
Frank Schäfer <fschaefer.oss@xxxxxxxxxxxxxx> escreveu:
> Am 13.04.2013 19:04, schrieb Mauro Carvalho Chehab:
> > Em Sat, 13 Apr 2013 17:33:28 +0200
> > Frank Schäfer <fschaefer.oss@xxxxxxxxxxxxxx> escreveu:
> >
> >> Am 13.04.2013 16:41, schrieb Mauro Carvalho Chehab:
> >>> Em Sat, 13 Apr 2013 11:48:39 +0200
> >>> Frank Schäfer <fschaefer.oss@xxxxxxxxxxxxxx> escreveu:
> >>>
> >>>> The GPIO register tracking/caching code is partially broken, because newer
> >>>> devices provide more than one GPIO register and some of them are even using
> >>>> separate registers for read and write access.
> >>>> Making it work would be too complicated.
> >>>> It is also used nowhere and doesn't make sense in cases where input lines are
> >>>> connected to buttons etc.
> >>>>
> >>>> Signed-off-by: Frank Schäfer <fschaefer.oss@xxxxxxxxxxxxxx>
> >>>> ---
> >>>> drivers/media/usb/em28xx/em28xx-cards.c | 12 ------------
> >>>> drivers/media/usb/em28xx/em28xx-core.c | 27 ++-------------------------
> >>>> drivers/media/usb/em28xx/em28xx.h | 6 ------
> >>>> 3 Dateien geändert, 2 Zeilen hinzugefügt(+), 43 Zeilen entfernt(-)
> >>> ...
> >>>
> >>>
> >>>> @@ -231,14 +215,7 @@ int em28xx_write_reg_bits(struct em28xx *dev, u16 reg, u8 val,
> >>>> int oldval;
> >>>> u8 newval;
> >>>>
> >>>> - /* Uses cache for gpo/gpio registers */
> >>>> - if (reg == dev->reg_gpo_num)
> >>>> - oldval = dev->reg_gpo;
> >>>> - else if (reg == dev->reg_gpio_num)
> >>>> - oldval = dev->reg_gpio;
> >>>> - else
> >>>> - oldval = em28xx_read_reg(dev, reg);
> >>>> -
> >>>> + oldval = em28xx_read_reg(dev, reg);
> >>>> if (oldval < 0)
> >>>> return oldval;
> >>> That's plain wrong, as it will break GPIO input.
> >>>
> >>> With GPIO, you can write either 0 or 1 to a GPIO output port. So, your
> >>> code works for output ports.
> >>>
> >>> However, an input port requires an specific value (either 1 or 0 depending
> >>> on the GPIO circuitry). If the wrong value is written there, the input port
> >>> will stop working.
> >>>
> >>> So, you can't simply read a value from a GPIO input and write it. You need
> >>> to shadow the GPIO write values instead.
> >> I don't understand what you mean.
> >> Why can I not read the value of a GPIO input and write it ?
> > Because, depending on the value you write, it can transform the input into an
> > output port.
>
> I don't get it.
> We always write to the GPIO register. That's why these functions are
> called em28xx_write_* ;)
> Whether the write operation is sane or not (e.g. because it modifies the
> bit corresponding to an input line) is not subject of these functions.
Writing is sane: GPIO input lines requires writing as well, in order to
set it to either pull-up or pull-down mode (not sure if em28xx supports
both ways).
So, the driver needs to know if it will write there a 0 or 1, and this is part
of its GPIO configuration.
Let's assume that, on a certain device, you need to write "1" to enable that
input.
A read I/O to that port can return either 0 or 1.
Giving an hypothetical example, please assume this code:
static int write_gpio_bits(u32 out, u32 mask)
{
u32 gpio = (read_gpio_ports() & ~mask) | (out & mask);
write_gpio_ports(gpio);
}
...
/* Use bit 1 as input GPIO */
write_gpio_bits(1, 1);
/* send a reset via bit 2 GPIO */
write_gpio_bits(2, 2);
write_gpio_bits(0, 2);
write_gpio_bits(2, 2);
If, at the time the above code runs, the input bit 1 is at "0" state,
the subsequent calls will disable the input.
If, instead, only the write operations are cached like:
static int write_gpio_bits(u32 out, u32 mask)
{
static u32 shadow_cache;
shadow_cache = (shadow_cache & ~mask) | (out & mask);
write_gpio_ports(gpio);
}
there's no such risk, as it will keep using "1" for the input bit.
See the difference?
Cheers,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
