On Thursday, February 26, 2009 11:43 PM, Daniel Mack wrote:

On Thu, Feb 26, 2009 at 10:18:40PM -0500, hartleys wrote:

quoted

+static struct rotary_encoder_platform_data my_rotary_encoder_info =

{

quoted

+	.steps = 24,
+	.gpio_a = GPIO_ROTARY_A,
+	.gpio_b = GPIO_ROTARY_B,
+	.inverted_a = 0,
+	.inverted_b = 0,
+};
+
+static struct platform_device rotary_encoder_device = {
+	.name		= "rotary-encoder",
+	.id		= 0,
+	.num_resources	= ARRAY_SIZE(rotary_encoder_resources),
+	.resource	= rotary_encoder_resources,
+	.dev		= {
+		.platform_data = &my_rotary_encoder_info,
+	}
+};
+

IRQ_GPIO appears to be only defined for mach-imx and mach-pxa.

Why not just use the platform_data information to get the irq for
the gpio at runtime with gpio_to_irq()?

Yes, true. But wouldn't be using the resources only and irq_to_gpio()
be the even cleaner solution?

I really don't know which one is cleaner or the correct choice.

The new example in your updated patch will not work for a generic
solution.

+/* board support file example */
+
+#define GPIO_ROTARY_A 1
+#define GPIO_ROTARY_B 2
+
+static struct resource rotary_encoder_resources[] = {
+	[0] = {
+		.flags = IORESOURCE_IRQ,
+		.start = gpio_to_irq(GPIO_ROTARY_A)
+	},
+	[1] = {
+		.flags = IORESOURCE_IRQ,
+		.start = gpio_to_irq(GPIO_ROTARY_B)
+	}
+};

Unless gpio_to_irq() is defined as a macro that returns a constant at
compile time you will get an error similar to:

error: initializer element is not constant
error: (near initialization for 'rotary_encoder_resources[0].start')
error: initializer element is not constant
error: (near initialization for 'rotary_encoder_resources[1].start')

quoted

+#include <linux/rotary-encoder.h>

This file is missing in the patch.

Will be added in the next version.

Cool.

I was able to compile and test your driver on an ep93xx based system
with a Bourns ECW1J-B24-BC0024 rotary encoder connected to the EGPIO8
and EGPIO9 pins.  But I had to make the following changes to your
original patch.

Keep gpio_a and gpio_b in the rotary_encoder_platform_data.
Removed the struct resource from the platform setup.
Following is the platform setup I used:

---

#define GPIO_ROTARY_A	EP93XX_GPIO_LINE_EGPIO8
#define GPIO_ROTARY_B	EP93XX_GPIO_LINE_EGPIO9

static struct rotary_encoder_platform_data my_rotary_encoder_info = {
	.steps		= 24,
	.gpio_a		= GPIO_ROTARY_A,
	.gpio_b		= GPIO_ROTARY_B,
	.inverted_a	= 0,
	.inverted_b	= 0,
};

static struct platform_device rotary_encoder_device = {
	.name		= "rotary-encoder",
	.id		= 0,
	.dev		= {
		.platform_data = &my_rotary_encoder_info,
	},
};

---

I also had to modified the probe routine as follows:

---

static int __devinit rotary_encoder_probe(struct platform_device *pdev)
{
	struct rotary_encoder *encoder;
-	struct resource *irq_a_res;
-	struct resource *irq_b_res;
-	unsigned long irq_flags;
	int err;

	encoder = kzalloc(sizeof(struct rotary_encoder), GFP_KERNEL);
	if (!encoder) {
		dev_err(&pdev->dev, "failed to allocate driver data\n");
		err = -ENOMEM;
		goto exit;
	}

-	irq_flags = IORESOURCE_IRQ_HIGHEDGE | IORESOURCE_IRQ_LOWEDGE;
-	irq_a_res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-	irq_b_res = platform_get_resource(pdev, IORESOURCE_IRQ, 1);
	encoder->pdata = pdev->dev.platform_data;
-
-	if (!irq_a_res || !irq_b_res || !encoder->pdata) {
+	if (!encoder->pdata) {
		dev_err(&pdev->dev, "insufficient resources\n");
		err = -ENOENT;
		goto exit_free_mem;
	}

-	encoder->irq_a = irq_a_res->start;
-	encoder->irq_b = irq_b_res->start;
+	encoder->irq_a = gpio_to_irq(encoder->pdata->gpio_a);
+	encoder->irq_b = gpio_to_irq(encoder->pdata->gpio_b);

	/* create and register the input driver */
	encoder->input = input_allocate_device();
	if (!encoder->input) {
		dev_err(&pdev->dev, "failed to allocate input
device\n");
		err = -ENOMEM;
		goto exit_free_mem;
	}

	encoder->input->name = pdev->name;
	encoder->input->id.bustype = BUS_HOST;
	encoder->input->dev.parent = &pdev->dev;
	encoder->input->evbit[0] = BIT_MASK(EV_ABS);
	encoder->input->absbit[0] = BIT_MASK(ABS_X);
	input_set_abs_params(encoder->input, ABS_X, 0,
			     encoder->pdata->steps, 0, 1);
	platform_set_drvdata(pdev, encoder);

	err = input_register_device(encoder->input);
	if (err) {
		dev_err(&pdev->dev, "failed to register input
device\n");
		goto exit_free_input;
	}

	/* request the GPIOs */
	err = gpio_request(encoder->pdata->gpio_a, DRV_NAME);
	if (err) {
		dev_err(&pdev->dev, "unable to request GPIO %d\n",
			encoder->pdata->gpio_a);
		goto exit_free_input;
	}

	err = gpio_request(encoder->pdata->gpio_b, DRV_NAME);
	if (err) {
		dev_err(&pdev->dev, "unable to request GPIO %d\n",
			encoder->pdata->gpio_b);
		goto exit_free_gpio_a;
	}

	/* request the IRQs */
	err = request_irq(encoder->irq_a, &rotary_encoder_irq,
-			  irq_flags, DRV_NAME, encoder);
+			  IORESOURCE_IRQ_HIGHEDGE |
IORESOURCE_IRQ_LOWEDGE,
+			  DRV_NAME, encoder);
	if (err) {
		dev_err(&pdev->dev, "unable to request IRQ %d\n",
			encoder->irq_a);
		goto exit_free_gpio;
	}

	err = request_irq(encoder->irq_b, &rotary_encoder_irq,
-			  irq_flags, DRV_NAME, encoder);
+			  IORESOURCE_IRQ_HIGHEDGE |
IORESOURCE_IRQ_LOWEDGE,
+			  DRV_NAME, encoder);	if (err) {
		dev_err(&pdev->dev, "unable to request IRQ %d\n",
			encoder->irq_b);
		goto exit_free_irq_a;
	}

	return 0;

exit_free_irq_a:
	free_irq(encoder->irq_a, encoder);
exit_free_gpio:
	gpio_free(encoder->pdata->gpio_b);
exit_free_gpio_a:
	gpio_free(encoder->pdata->gpio_a);
exit_free_input:
	input_free_device(encoder->input);
exit_free_mem:
	kfree(encoder);
exit:
	return err;
}

Regards,
Hartley