input: MXC: add mxc-keypad driver to support the Keypad Port present in the mxc application processors family.

 arch/arm/plat-mxc/include/mach/mxc_keypad.h |   18 +
 drivers/input/keyboard/Kconfig              |    9 +
 drivers/input/keyboard/Makefile             |    1 +
 drivers/input/keyboard/mxc_keypad.c         |  516 +++++++++++++++++++++++++++
 4 files changed, 544 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/plat-mxc/include/mach/mxc_keypad.h
 create mode 100644 drivers/input/keyboard/mxc_keypad.c

arch/arm/plat-mxc/include/mach/mxc_keypad.h new file +0 -0

new file mode 100644
index 0000000..4509103

arch/arm/plat-mxc/include/mach/mxc_keypad.h new file +25 -0

+
+/*
+ * Keypad Controller registers (halfword)
+ */
+#define KPCR		0x00 /* Keypad Control Register */
+
+#define KPSR		0x02 /* Keypad Status Register */
+#define KBD_STAT_KPKD	(0x1 << 0) /* Key Press Interrupt Status bit */
+#define KBD_STAT_KPKR	(0x1 << 1) /* Key Release Interrupt Status bit */
+#define KBD_STAT_KDSC	(0x1 << 2) /* Key Depress Synch Chain Status bit */
+#define KBD_STAT_KRSS	(0x1 << 3) /* Key Release Synch Status bit */
+#define KBD_STAT_KDIE	(0x1 << 8) /* Key Depress Interrupt Enable Status bit */
+#define KBD_STAT_KRIE	(0x1 << 9) /* Key Release Interrupt Enable */
+#define KBD_STAT_KPPEN	(0x1 << 10) /* Keypad Clock Enable */
+
+#define KDDR		0x04 /* Keypad Data Direction Register */
+#define KPDR		0x06 /* Keypad Data Register */
+
+#define MAX_MATRIX_KEY_NUM	(MAX_MATRIX_KEY_ROWS * MAX_MATRIX_KEY_COLS)
+
+struct mxc_keypad {
+	struct mxc_keypad_platform_data *pdata;
+
+	struct clk *clk;
+	struct input_dev *input_dev;

arch/arm/plat-mxc/include/mach/mxc_keypad.h new file +169 -0

+};
+
+/* Return 0 if no changes are detected in matrix */
+static int mxc_keypad_scan_matrix(struct mxc_keypad *keypad)
+{
+	struct input_dev *input_dev = keypad->input_dev;
+	int row, col, changed = 0;
+	unsigned short new_state[MAX_MATRIX_KEY_COLS];
+	unsigned short reg_val;
+
+	memset(new_state, 0, sizeof(new_state));
+
+	for (col = 0; col < MAX_MATRIX_KEY_COLS; col++) {
+		if ((keypad->cols_en_mask & (1 << col)) == 0)
+			continue;
+		/* Discharge keypad capacitance:
+		 * 2. write 1s on column data.
+		 * 3. configure columns as totem-pole to discharge capacitance.
+		 * 4. configure columns as open-drain.*/
+		reg_val = readw(keypad->mmio_base + KPDR);
+		reg_val |= 0xff00;
+		writew(reg_val, keypad->mmio_base + KPDR);
+
+		reg_val = readw(keypad->mmio_base + KPCR);
+		reg_val &= ~((keypad->cols_en_mask & 0xff) << 8);
+		writew(reg_val, keypad->mmio_base + KPCR);
+
+		udelay(2);
+
+		reg_val = readw(keypad->mmio_base + KPCR);
+		reg_val |= (keypad->cols_en_mask & 0xff) << 8;
+		writew(reg_val, keypad->mmio_base + KPCR);
+
+		/*
+		 * 5. Write a single column to 0, others to 1.
+		 * 6. Sample row inputs and save data.
+		 * 7. Repeat steps 2 - 6 for remaining columns.
+		 */
+		reg_val = readw(keypad->mmio_base + KPDR);
+		reg_val &= ~(1 << (8 + col));
+		writew(reg_val, keypad->mmio_base + KPDR);
+
+		/* Delay added to avoid propagating the 0 from column to row
+		 * when scanning. */
+		udelay(5);
+
+		/* 1s in state detect a key pressure */
+		new_state[col] = (~readw(keypad->mmio_base + KPDR)) & 0x00ff;
+	}
+
+	/* Test the state changes */
+	for (col = 0; col < MAX_MATRIX_KEY_COLS; col++) {
+		unsigned short bits_changed;
+		int code;
+
+		if ((keypad->cols_en_mask & (1 << col)) == 0)
+			continue;
+
+		bits_changed = keypad->matrix_key_state[col] ^ new_state[col];
+		if (bits_changed == 0)
+			continue;
+
+		changed = 1;
+		for (row = 0; row < MAX_MATRIX_KEY_ROWS; row++) {
+			if ((keypad->rows_en_mask & (1 << row)) == 0)
+				continue;
+			if ((bits_changed & (1 << row)) == 0)
+				continue;
+
+			code = MATRIX_SCAN_CODE(row, col, MATRIX_ROW_SHIFT);
+			input_event(input_dev, EV_MSC, MSC_SCAN, code);
+			input_report_key(input_dev, keypad->keycodes[code],
+					 new_state[col] & (1 << row));
+			dev_dbg(&input_dev->dev, "Event code: %d, val: %d",
+					keypad->keycodes[code],
+					new_state[col] & (1 << row));
+		}
+	}
+	input_sync(input_dev);
+	memcpy(keypad->matrix_key_state, new_state, sizeof(new_state));
+
+	/* Return in standby mode:
+	 * 9. write 0s to columns */
+	reg_val = readw(keypad->mmio_base + KPDR);
+	reg_val &= 0x00ff;
+	writew(reg_val, keypad->mmio_base + KPDR);
+
+	return changed;
+}
+
+static void mxc_keypad_relax_timer_handler(unsigned long data)
+{
+	struct mxc_keypad *keypad = (struct mxc_keypad *) data;
+	unsigned short reg_val;
+
+	/* 10. Clear KPKD and KPKR status bits
+	 *     Set the KPKR sync chain and clear the KPKD sync chain */
+	reg_val = readw(keypad->mmio_base + KPSR);
+	reg_val |= KBD_STAT_KPKD | KBD_STAT_KPKR |
+		   KBD_STAT_KDSC | KBD_STAT_KRSS;
+	writew(reg_val, keypad->mmio_base + KPSR);
+
+	/* Re enable interrupts and clear sync reset bits.
+	 * Next KDI is used for detect multiple pressures. */
+	reg_val = readw(keypad->mmio_base + KPSR);
+	reg_val &= ~(KBD_STAT_KDSC | KBD_STAT_KRSS);
+	writew(reg_val, keypad->mmio_base + KPSR);
+
+	reg_val |= KBD_STAT_KDIE | KBD_STAT_KRIE;
+	if (keypad->irq_type == MXC_IRQ_KRI)
+		reg_val &= ~KBD_STAT_KRIE;
+	writew(reg_val, keypad->mmio_base + KPSR);
+}
+
+static void mxc_keypad_scan_timer_handler(unsigned long data)
+{
+	struct mxc_keypad *keypad = (struct mxc_keypad *) data;
+	struct input_dev *idev = keypad->input_dev;
+
+	dev_dbg(&idev->dev, "Handling Interrupt of type %x\n",
+						keypad->irq_type);
+
+	/* Do the scan routine and Keep track for how many time we got
+	 * interrupt that make no change */
+	if (mxc_keypad_scan_matrix(keypad))
+		keypad->irq_since_last_change = 0;
+	else
+		keypad->irq_since_last_change++;
+
+	/* If the key is pressed since too many time, relax the update period */
+	if (keypad->irq_since_last_change > 2)
+		mod_timer(&keypad->relax_timer,
+			  jiffies + msecs_to_jiffies(60));
+	else
+		mxc_keypad_relax_timer_handler((unsigned long) keypad);
+}
+
+static irqreturn_t mxc_keypad_irq_handler(int irq, void *dev_id)
+{
+	struct mxc_keypad *keypad = dev_id;
+	struct mxc_keypad_platform_data *pdata = keypad->pdata;
+	unsigned short reg_val;
+
+	/* Disable every keypad interrupt */
+	reg_val = readw(keypad->mmio_base + KPSR);
+	reg_val &= ~(KBD_STAT_KRIE | KBD_STAT_KDIE);
+	writew(reg_val, keypad->mmio_base + KPSR);
+
+	keypad->irq_type = reg_val & KBD_STAT_KPKD ?
+			   MXC_IRQ_KDI : MXC_IRQ_KRI;
+
+	/* Schedule the scanning procedure */
+	mod_timer(&keypad->scan_timer,
+		  jiffies + msecs_to_jiffies(pdata->debounce_ms));
+
+	return IRQ_HANDLED;
+}
+
+static void mxc_keypad_config(struct mxc_keypad *keypad)
+{
+	unsigned short reg_val;
+
+	/* Enable number of rows in keypad (KPCR[7:0])
+	 * Configure keypad columns as open-drain (KPCR[15:8])
+	 */
+	reg_val = readw(keypad->mmio_base + KPCR);
+	reg_val |= keypad->rows_en_mask & 0xff;		/* rows */
+	reg_val |= (keypad->cols_en_mask & 0xff) << 8;	/* cols */
+	writew(reg_val, keypad->mmio_base + KPCR);

arch/arm/plat-mxc/include/mach/mxc_keypad.h new file +37 -0

+	writew(reg_val, keypad->mmio_base + KDDR);
+
+	/* Clear Key Depress and Key Release status bit.
+	 * Clear synchronizer chain.
+	 * */
+	reg_val = readw(keypad->mmio_base + KPSR);
+	reg_val |= KBD_STAT_KPKR | KBD_STAT_KPKD |
+		   KBD_STAT_KDSC | KBD_STAT_KRSS;
+	writew(reg_val, keypad->mmio_base + KPSR);
+
+	/* Set the KDIE control bit, and clear the KRIE control bit
+	 * (avoid false release events). */
+	reg_val |= KBD_STAT_KDIE;
+	reg_val &= ~KBD_STAT_KRIE;
+	writew(reg_val, keypad->mmio_base + KPSR);
+}
+
+static void mxc_keypad_inhibit(struct mxc_keypad *keypad)
+{
+	unsigned short reg_val;
+
+	/* Colums as open drain and disable rows */
+	writew(0xff00, keypad->mmio_base + KPCR);
+
+	/* Clear the KPKD status flag and synchronizer chain.
+	 * Clear KDIE control bit and KRIE control bit.
+	 */
+	reg_val = readw(keypad->mmio_base + KPSR);
+	reg_val |= KBD_STAT_KPKD | KBD_STAT_KDSC;
+	reg_val &= ~(KBD_STAT_KRIE | KBD_STAT_KDIE);
+	writew(reg_val, keypad->mmio_base + KPSR);
+}
+
+static int mxc_keypad_open(struct input_dev *dev)
+{
+	struct mxc_keypad *keypad = input_get_drvdata(dev);
+

arch/arm/plat-mxc/include/mach/mxc_keypad.h new file +31 -0

+		goto open_err;
+	}
+
+	return 0;
+
+open_err:
+	mxc_keypad_inhibit(keypad);
+	return -EIO;
+}
+
+static void mxc_keypad_close(struct input_dev *dev)
+{
+	struct mxc_keypad *keypad = input_get_drvdata(dev);
+
+	dev_dbg(&dev->dev, "%s\n", __func__);
+
+	mxc_keypad_inhibit(keypad);
+
+	/* Disable clock unit */
+	clk_disable(keypad->clk);
+}
+
+static int __devinit mxc_keypad_probe(struct platform_device *pdev)
+{
+	struct mxc_keypad_platform_data *pdata = pdev->dev.platform_data;
+	struct mxc_keypad *keypad;
+	struct input_dev *input_dev;
+	struct resource *res;
+	int irq, error, i;
+	const struct matrix_keymap_data *keymap_data;
+

arch/arm/plat-mxc/include/mach/mxc_keypad.h new file +20 -0

+	}
+
+	res = request_mem_region(res->start, resource_size(res), pdev->name);
+	if (res == NULL) {
+		dev_err(&pdev->dev, "failed to request I/O memory\n");
+		return -EBUSY;
+	}
+
+	input_dev = input_allocate_device();
+	if (!input_dev) {
+		dev_err(&pdev->dev, "failed to allocate the input device\n");
+		error = -ENOMEM;
+		goto failed_rel_mem;
+	}
+
+	keypad = kzalloc(sizeof(struct mxc_keypad), GFP_KERNEL);
+	if (!keypad) {
+		dev_err(&pdev->dev, "not enough memory for driver data\n");
+		error = -ENOMEM;
+		goto failed_free_input;

arch/arm/plat-mxc/include/mach/mxc_keypad.h new file +137 -0

+	if (IS_ERR(keypad->clk)) {
+		dev_err(&pdev->dev, "failed to get keypad clock\n");
+		error = PTR_ERR(keypad->clk);
+		goto failed_unmap;
+	}
+
+	/*
+	 * Search for rows and cols enabled
+	 */
+	keymap_data = pdata->keymap_data;
+	for (i = 0; i < keymap_data->keymap_size; i++) {
+		keypad->rows_en_mask |= 1 << KEY_ROW(keymap_data->keymap[i]);
+		keypad->cols_en_mask |= 1 << KEY_COL(keymap_data->keymap[i]);
+	}
+
+	if (keypad->rows_en_mask > ((1 << MAX_MATRIX_KEY_ROWS) - 1) ||
+	   keypad->cols_en_mask > ((1 << MAX_MATRIX_KEY_COLS) - 1)) {
+		dev_err(&pdev->dev, "Invalid key data (too rows or colums)\n");
+		error = -EINVAL;
+		goto failed_clock_put;
+	}
+	dev_dbg(&pdev->dev, "enabled rows mask: %x\n", keypad->rows_en_mask);
+	dev_dbg(&pdev->dev, "enabled cols mask: %x\n", keypad->cols_en_mask);
+
+	/* Init Keypad timers */
+	init_timer(&keypad->scan_timer);
+	keypad->scan_timer.function = mxc_keypad_scan_timer_handler;
+	keypad->scan_timer.data = (unsigned long) keypad;
+
+	init_timer(&keypad->relax_timer);
+	keypad->relax_timer.function = mxc_keypad_relax_timer_handler;
+	keypad->relax_timer.data = (unsigned long) keypad;
+
+	/* Init the Input device */
+	input_dev->name = pdev->name;
+	input_dev->id.bustype = BUS_HOST;
+	input_dev->dev.parent = &pdev->dev;
+	input_dev->open = mxc_keypad_open;
+	input_dev->close = mxc_keypad_close;
+	input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_REP);
+	input_dev->keycode = keypad->keycodes;
+	input_dev->keycodesize = sizeof(keypad->keycodes[0]);
+	input_dev->keycodemax = ARRAY_SIZE(keypad->keycodes);
+
+	matrix_keypad_build_keymap(pdata->keymap_data, MATRIX_ROW_SHIFT,
+				keypad->keycodes, input_dev->keybit);
+
+	input_set_capability(input_dev, EV_MSC, MSC_SCAN);
+	input_set_drvdata(input_dev, keypad);
+
+	error = request_irq(irq, mxc_keypad_irq_handler, IRQF_DISABLED,
+			pdev->name, keypad);
+	if (error) {
+		dev_err(&pdev->dev, "failed to request IRQ\n");
+		goto failed_clock_put;
+	}
+
+	/* Register the input device */
+	error = input_register_device(input_dev);
+	if (error) {
+		dev_err(&pdev->dev, "failed to register input device\n");
+		goto failed_free_irq;
+	}
+
+	platform_set_drvdata(pdev, keypad);
+	device_init_wakeup(&pdev->dev, 1);
+
+	dev_info(&pdev->dev, "device probed\n");
+
+	return 0;
+
+failed_free_irq:
+	free_irq(irq, pdev);
+failed_clock_put:
+	clk_put(keypad->clk);
+failed_unmap:
+	iounmap(keypad->mmio_base);
+failed_free_priv:
+	kfree(keypad);
+failed_free_input:
+	input_free_device(input_dev);
+failed_rel_mem:
+	release_mem_region(res->start, resource_size(res));
+	return error;
+}
+
+static int __devexit mxc_keypad_remove(struct platform_device *pdev)
+{
+	struct mxc_keypad *keypad = platform_get_drvdata(pdev);
+	struct resource *res;
+
+	platform_set_drvdata(pdev, NULL);
+
+	free_irq(keypad->irq, keypad);
+	clk_put(keypad->clk);
+	del_timer_sync(&keypad->scan_timer);
+	del_timer_sync(&keypad->relax_timer);
+
+	input_unregister_device(keypad->input_dev);
+
+	iounmap(keypad->mmio_base);
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	release_mem_region(res->start, resource_size(res));
+
+	kfree(keypad);
+
+	dev_info(&pdev->dev, "device removed\n");
+
+	return 0;
+}
+
+static struct platform_driver mxc_keypad_driver = {
+	.driver		= {
+		.name	= "mxc-keypad",
+		.owner	= THIS_MODULE,
+	},
+	.probe		= mxc_keypad_probe,
+	.remove		= __devexit_p(mxc_keypad_remove),
+};
+
+static int __init mxc_keypad_init(void)
+{
+	return platform_driver_register(&mxc_keypad_driver);
+}
+
+static void __exit mxc_keypad_exit(void)
+{
+	platform_driver_unregister(&mxc_keypad_driver);
+}
+
+module_init(mxc_keypad_init);
+module_exit(mxc_keypad_exit);
+
+MODULE_AUTHOR("Alberto Panizzo <[email protected]>");
+MODULE_DESCRIPTION("MXC Keypad Port Driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:mxc-keypad");

Signature · 9 lines

1.6.3.3




_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

drivers/input/keyboard/mxc_keypad.c line 379

Quoted 2 lines

drivers/input/keyboard/mxc_keypad.c new file +1 -0

+	keypad->clk = clk_get(NULL, "kpp");

clk_get(&pdev->dev, "kpp");

Hi,

Alberto Panizzo writes:
[...]

arch/arm/plat-mxc/include/mach/mxc_keypad.h line 9

Quoted 6 lines

arch/arm/plat-mxc/include/mach/mxc_keypad.h new file +6 -0

+#include <linux/input/matrix_keypad.h>
+
+#define MAX_MATRIX_KEY_ROWS	(8)
+#define MAX_MATRIX_KEY_COLS	(8)
+#define MATRIX_ROW_SHIFT	(3)
+

pointless '()'

drivers/input/keyboard/mxc_keypad.c line 252

Quoted 7 lines

[...]

drivers/input/keyboard/mxc_keypad.c new file +5 -0

+
+	/* Configure columns as output, rows as input (KDDR[15:0]) */
+	reg_val = readw(keypad->mmio_base + KDDR);
+	reg_val |= 0xff00;
+	reg_val &= 0xff00;

This is effectively the same as: 'reg_val = 0xff00;' which makes the readw() above pointless. Was this really intended?

drivers/input/keyboard/mxc_keypad.c line 253

Quoted 1 line

drivers/input/keyboard/mxc_keypad.c new file +1 -0

+	writew(reg_val, keypad->mmio_base + KDDR);

drivers/input/keyboard/mxc_keypad.c line 299

Quoted 6 lines

[...]

drivers/input/keyboard/mxc_keypad.c new file +4 -0

+	/* Sanity control, not all the rows must be to 0s now. */
+	if ((readw(keypad->mmio_base + KPDR) & keypad->rows_en_mask) == 0) {
+		dev_err(&dev->dev, "Too much keys pressed for now, "
+				"control pins initialisation\n");

It helps grepping for a message, if it is not split into multiple lines.

drivers/input/keyboard/mxc_keypad.c line 340

Quoted 9 lines

[...]

drivers/input/keyboard/mxc_keypad.c new file +6 -0

+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "failed to get keypad irq\n");
+		return -ENXIO;
+	}

This should be -ENODEV.

drivers/input/keyboard/mxc_keypad.c line 345

Quoted 6 lines

drivers/input/keyboard/mxc_keypad.c new file +5 -0

+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (res == NULL) {
+		dev_err(&pdev->dev, "failed to get I/O memory\n");
+		return -ENXIO;

same as above.

drivers/input/keyboard/mxc_keypad.c line 375

Quoted 10 lines

drivers/input/keyboard/mxc_keypad.c new file +9 -0

+
+	keypad->pdata = pdata;
+	keypad->input_dev = input_dev;
+	keypad->irq = irq;
+
+	keypad->mmio_base = ioremap(res->start, resource_size(res));
+	if (keypad->mmio_base == NULL) {
+		dev_err(&pdev->dev, "failed to remap I/O memory\n");
+		error = -ENXIO;

-ENOMEM;

drivers/input/keyboard/mxc_keypad.c line 379

Quoted 6 lines

drivers/input/keyboard/mxc_keypad.c new file +4 -0

+		goto failed_free_priv;
+	}
+
+	keypad->clk = clk_get(NULL, "kpp");

clk_get(&pdev->dev, "kpp");

Hallo Alberto,

Quoted 3 lines · Lothar Wa?mann

drivers/input/keyboard/mxc_keypad.c line 379

Quoted 2 lines

drivers/input/keyboard/mxc_keypad.c new file +1 -0

+	keypad->clk = clk_get(NULL, "kpp");

clk_get(&pdev->dev, "kpp");

Maybe even

	clk_get(&pdev->dev, NULL);

Quoted 8 lines

drivers/input/keyboard/mxc_keypad.c line 340

Quoted 6 lines

drivers/input/keyboard/mxc_keypad.c new file +5 -0

+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "failed to get keypad irq\n");
+		return -ENXIO;
+	}

This should be -ENODEV.

Lot of reference keyboard driver use -ENXIO..
May should be better: return irq ?

Hi,

Quoted 8 lines · Alberto Panizzo

Quoted 8 lines

drivers/input/keyboard/mxc_keypad.c line 340

Quoted 6 lines

drivers/input/keyboard/mxc_keypad.c new file +5 -0

+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "failed to get keypad irq\n");
+		return -ENXIO;
+	}

This should be -ENODEV.

Quoted 3 lines

Lot of reference keyboard driver use -ENXIO..
May should be better: return irq ?

Yes, of course. If a function returns an error code that should be promoted to the caller instead of inventing a new error code.

Lothar Wa?mann

Quoted 18 lines · Lothar Wa?mann

Hi,

Quoted 11 lines · Alberto Panizzo

Quoted 8 lines

drivers/input/keyboard/mxc_keypad.c line 340

Quoted 6 lines

drivers/input/keyboard/mxc_keypad.c new file +5 -0

+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "failed to get keypad irq\n");
+		return -ENXIO;
+	}

This should be -ENODEV.

Lot of reference keyboard driver use -ENXIO..
May should be better: return irq ?

Yes, of course. If a function returns an error code that should be promoted to the caller instead of inventing a new error code.

Quoted 1 line

Lothar Wa?mann

But, errno.h say:

#define ENXIO           6                // Device not configured
#define ENODEV          19               // Operation not supported by device

Hi,

Quoted 19 lines · Alberto Panizzo

Quoted 18 lines · Lothar Wa?mann

Hi,

Quoted 11 lines · Alberto Panizzo

Quoted 8 lines

drivers/input/keyboard/mxc_keypad.c line 340

Quoted 6 lines

drivers/input/keyboard/mxc_keypad.c new file +5 -0

+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "failed to get keypad irq\n");
+		return -ENXIO;
+	}

This should be -ENODEV.

Lot of reference keyboard driver use -ENXIO..
May should be better: return irq ?

Yes, of course. If a function returns an error code that should be promoted to the caller instead of inventing a new error code.

Quoted 6 lines

Quoted 1 line

Lothar Wa?mann

But, errno.h say:

#define ENXIO           6                // Device not configured
#define ENODEV          19               // Operation not supported by device

What errno.h file is that?
I have:

./include/asm-generic/errno-base.h:#define      ENXIO            6      /* No such device or address */
./include/asm-generic/errno-base.h:#define      ENODEV          19      /* No such device */

AFAIK ENXIO is used when actual I/O has been attempted. But in this case the driver is still being configured and did not do any I/O yet.

Lothar Wa?mann

Quoted 30 lines · Lothar Wa?mann

Hi,

Quoted 19 lines · Alberto Panizzo

Quoted 18 lines · Lothar Wa?mann

Hi,

Quoted 11 lines · Alberto Panizzo

Quoted 8 lines

Review

Quoted 6 lines

(quoted diff) +5 -0

+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "failed to get keypad irq\n");
+		return -ENXIO;
+	}

This should be -ENODEV.

Lot of reference keyboard driver use -ENXIO..
May should be better: return irq ?

Yes, of course. If a function returns an error code that should be promoted to the caller instead of inventing a new error code.

Quoted 6 lines

Quoted 1 line

Lothar Wa?mann

But, errno.h say:

#define ENXIO           6                // Device not configured
#define ENODEV          19               // Operation not supported by device

What errno.h file is that?
I have:

Quoted 8 lines

./include/asm-generic/errno-base.h:#define      ENXIO            6      /* No such device or address */
./include/asm-generic/errno-base.h:#define      ENODEV          19      /* No such device */

AFAIK ENXIO is used when actual I/O has been attempted. But in this case the driver is still being configured and did not do any I/O yet.

Lothar Wa?mann

The errno.h that I propose is a googled one and the kernel-one do not explain well.. Not for fighting, I wont understand.

In drivers/base/platform.c:

/**
* platform_get_irq - get an IRQ for a device
* @dev: platform device
* @num: IRQ number index
*/
int platform_get_irq(struct platform_device *dev, unsigned int num)

{
	struct resource *r = platform_get_resource(dev, IORESOURCE_IRQ, num);

	return r ? r->start : -ENXIO;
}

If there isn't the irq resource asked platform_get_irq return ENXIO.

Hi,

Quoted 51 lines · Alberto Panizzo

Quoted 30 lines · Lothar Wa?mann

Hi,

Quoted 19 lines · Alberto Panizzo

Quoted 18 lines · Lothar Wa?mann

Hi,

Quoted 11 lines · Alberto Panizzo

Quoted 8 lines

Review

Quoted 6 lines

(quoted diff) +5 -0

+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "failed to get keypad irq\n");
+		return -ENXIO;
+	}

This should be -ENODEV.

Lot of reference keyboard driver use -ENXIO..
May should be better: return irq ?

Yes, of course. If a function returns an error code that should be promoted to the caller instead of inventing a new error code.

Quoted 6 lines

Quoted 1 line

Lothar Wa?mann

But, errno.h say:

#define ENXIO           6                // Device not configured
#define ENODEV          19               // Operation not supported by device

What errno.h file is that?
I have:

Quoted 8 lines

./include/asm-generic/errno-base.h:#define      ENXIO            6      /* No such device or address */
./include/asm-generic/errno-base.h:#define      ENODEV          19      /* No such device */

AFAIK ENXIO is used when actual I/O has been attempted. But in this case the driver is still being configured and did not do any I/O yet.

Lothar Wa?mann

The errno.h that I propose is a googled one and the kernel-one do not explain well.. Not for fighting, I wont understand.

In drivers/base/platform.c:

/**
* platform_get_irq - get an IRQ for a device
* @dev: platform device
* @num: IRQ number index
*/
int platform_get_irq(struct platform_device *dev, unsigned int num)

Quoted 8 lines

{
	struct resource *r = platform_get_resource(dev, IORESOURCE_IRQ, num);

	return r ? r->start : -ENXIO;
}

If there isn't the irq resource asked platform_get_irq return ENXIO.

The POSIX spec says:

|[ENXIO]
|    No such device or address. Input or output on a special file
|refers to a device that does not exist, or makes a request beyond the
|capabilities of the device. It may also occur when, for example, a
|tape drive is not on-line. 

http://www.opengroup.org/onlinepubs/000095399/functions/xsh_chap02_03.html

Hi,

Quoted 57 lines · Alberto Panizzo

Quoted 30 lines · Lothar Wa?mann

Hi,

Quoted 19 lines · Alberto Panizzo

Quoted 18 lines · Lothar Wa?mann

Hi,

Quoted 11 lines · Alberto Panizzo

Quoted 8 lines

Review

Quoted 6 lines

(quoted diff) +5 -0

+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(&pdev->dev, "failed to get keypad irq\n");
+		return -ENXIO;
+	}

This should be -ENODEV.

Lot of reference keyboard driver use -ENXIO..
May should be better: return irq ?

Yes, of course. If a function returns an error code that should be promoted to the caller instead of inventing a new error code.

Quoted 6 lines

Quoted 1 line

Lothar Wa?mann

But, errno.h say:

#define ENXIO           6                // Device not configured
#define ENODEV          19               // Operation not supported by device

What errno.h file is that?
I have:

Quoted 8 lines

./include/asm-generic/errno-base.h:#define      ENXIO            6      /* No such device or address */
./include/asm-generic/errno-base.h:#define      ENODEV          19      /* No such device */

AFAIK ENXIO is used when actual I/O has been attempted. But in this case the driver is still being configured and did not do any I/O yet.

Lothar Wa?mann

The errno.h that I propose is a googled one and the kernel-one do not explain well.. Not for fighting, I wont understand.

In drivers/base/platform.c:

/**
* platform_get_irq - get an IRQ for a device
* @dev: platform device
* @num: IRQ number index
*/
int platform_get_irq(struct platform_device *dev, unsigned int num)

{
	struct resource *r = platform_get_resource(dev, IORESOURCE_IRQ, num);

	return r ? r->start : -ENXIO;
}

Quoted 2 lines

If there isn't the irq resource asked platform_get_irq return ENXIO.

The POSIX spec says:

|[ENXIO]
|    No such device or address. Input or output on a special file
|refers to a device that does not exist, or makes a request beyond the
|capabilities of the device. It may also occur when, for example, a
|tape drive is not on-line. 

http://www.opengroup.org/onlinepubs/000095399/functions/xsh_chap02_03.html