Re: [PATCH] gpiodev-keys, gpiodev-diagonal + button patches for h5000

Hello Milan,

        Sorry for delay with processing your patches.

Sunday, April 8, 2007, 11:59:36 PM, you wrote:

> Hello,

> first of all, I apologize for sending patch, which adds both generic
> and port-specific functionality at once. In the future I will try to
> avoid this, but it's quite late now:) I would also like to thank
> psokolovsky and bd2 for their patience with me.

        I think you did the great job with these drivers, and there's no need to try to have the most general solution from the first time ;-).

> Patch and files, which can be found at
> http://handhelds.org/~mmp/h5000/gpiodev/ , make following changes to
> kernel tree:
> 1) "Upgrade" from gpio-keys to gpiodev-keys, which uses gpio_dev . Of
> course, not removing gpio-keys .
> 2) Add gpiodev-diagonal driver. This driver implements special case
> where joypad's GPIOs are connected in diagonal, instead of normal way.
> 3) Modifications for drivers/soc/samcop_base.c, so it implements
> gpio_dev.
> 4) Modifications to arch/arm/mach-pxa/h5400/h5400.c, which replace
> gpio-keys code with gpiodev-keys and gpiodev-diagonal.

        All patches were applied, except for few hunks in this file. Specifically, update to use gpiodev-keys/gpiodev-diagonal had conflicts, can you cvs update, resolev them, and resubmit? Btw, this:

+ { _KEY_RECORD, { NULL, SAMCOP_GPA_RECORD }, 1, "Record button" },

+ for (i = 1; i < h5400_pxa_keys_data.nbuttons; i++)
+ h5400_button_table[i].gpio.gpio_dev = &h5400_samcop;

        Is now how gpiodev API was intended to use. Even though GPIO id's are structured, they are still constant (link-time constant though). So, it is intended that it will be written as:

+ { _KEY_RECORD, { &h5400_samcop.dev, SAMCOP_GPA_RECORD }, 1, "Record button" },

        Other issue is this hunk:

 static int h5400_dma_needs_bounce (struct device *dev, dma_addr_t addr, size_t size)
 {
- return (addr >= H5400_SAMCOP_BASE + _SAMCOP_SRAM_Base + SAMCOP_SRAM_SIZE);
+ return (addr + size >= H5400_SAMCOP_BASE + _SAMCOP_SRAM_Base + SAMCOP_SRAM_SIZE);
 };

        Are you suer this is correct still? What it said before is that bouncing was required when block start lies after the SAMCOP SRAM. With your change, it is "debouncing is required if next byte after block end lies after the SAMCOP SRAM". Doesn't make much sense, IMHO, unless it's some kind of arithmetic optimization, in which case it's worth some comment. Please reconfirm.

> 5) Modification go cpu-pxa.c to export pxagpio_device
> 6) Fix to include/asm/hardware/ipaq-samcop.h, thanks to which IRQ
> handling works now correctly (much thanks rv6502 for pointing out the
> problem).

> Patch should be applicable to the most recent kernel tree, header
> files go to include/linux and .c files to drivers/input/keyboard. I hope
> I did not forget anything. If so, or there are any objections, let me
> know:)

> Milan

> _______________________________________________
> H5400-port mailing list
> H5400-port_at_handhelds.org
> https://www.handhelds.org/mailman/listinfo/h5400-port

-- 
Best regards,
 Paul                            mailto:pmiscml_at_gmail.com