Re: [MeeGo-Dev][PATCH] Topcliff: Update PCH_GPIO driver to 2.6.35

[Mark Brown]

On Fri, Sep 03, 2010 at 07:54:00PM +0900, Masayuki Ohtak wrote:

> Topcliff PCH is the platform controller hub that is going to be used in
> Intel's upcoming general embedded platform. All IO peripherals in
> Topcliff PCH are actually devices sitting on AMBA bus.
> Topcliff PCH has GPIO I/F. Using this I/F, it is able to access system
> devices connected to GPIO.

This has a similar issue to the CAN driver - it isn't a standard kernel
GPIO driver, it implements a custom ioctl based userspace ABI.  This
means it's not usable from within the kernel and isn't suitable for
mainline.  You should be implementing support for the device using the
standard interfaces used by the other GPIO drivers in kernel.

> +/**
> + * pch_gpio_handler - Implements the interrupt handler functionality of
> + *		      the gpio module.
> + * @irq:	Contains the irq value
> + * @pData:	Contains the reference to the base gpio address.

Please use the standard kernel coding style conventions for naming
variables.

> +static irqreturn_t pch_gpio_handler(s32 irq, void *pData)
> +{
> +	irqreturn_t ret = IRQ_NONE;
> +	u32 pch_idisp_regval;
> +	struct pch_gpio_chip *chip =  (struct pch_gpio_chip *)pData;

No need to cast away from void.

> +
> +	pch_idisp_regval =
> +			chip->pch_gpio_bit_mask & ioread32(&chip->reg->idisp);
> +	if (pch_idisp_regval != 0) {
> +		/*invoke the callback */
> +		(*pch_gpio_cbr) (pch_idisp_regval, chip);

The interrupts from the controller should be hooked up to genirq, not
using this custom callback mechanism.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/