All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC 0/7] ARM: OMAP: add DT binding for gpmc-smsc911x
@ 2013-02-09 20:44 ` Javier Martinez Canillas
  0 siblings, 0 replies; 46+ messages in thread
From: Javier Martinez Canillas @ 2013-02-09 20:44 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Benoît Cousson, Russell King, Linus Walleij,
	Greg Kroah-Hartman, Enric Balletbo i Serra, Ezequiel Garcia,
	devicetree-discuss, linux-omap, linux-arm-kernel

Hello,

This is an RFC to add Device Tree support for SMSC LAN chips connected
to OMAP processors though its General-Purpose Memory Controller.

The patch-set is composed of the following patches:

[PATCH RFC 1/7] platform: add a device node
[PATCH RFC 2/7] net: smsc911x: add pinctrl support
[PATCH RFC 3/7] ARM: OMAP: gpmc-smsc911x: add DT dev node init function
[PATCH RFC 4/7] ARM: OMAP: gpmc-smsc911x: pass a dev node to platform
[PATCH RFC 5/7] ARM: OMAP: gpmc: add support for gpmc-smsc911x child
[PATCH RFC 6/7] ARM: dts: OMAP: Add an GPMC node for OMAP3
[PATCH RFC 7/7] ARM: dts: omap3-igep0020: Add SMSC911x LAN chip support

It is an RFC because I had to modify the platform_device_register_resndata()
platform device registration function to allow passing a device node and
associate to the platform device after allocation. The patch-set updates all
current users and this shouldn't have a function effect on them.

I think that the need for platform code to pass the DT device node to the
platform registration infraestructure will be needed for other devices too
whose probe function needs to access the device node associated with the
device.

The binding has been tested on an TI OMAP3 based board (IGEPv2) and added
support is included on the RFC as an example.

It is based on Linus' latest master branch + linux-omap/omap-for-v3.9/gpmc
+ linux-omap-dt/for_3.9/dts

Thanks a lot and best regards,
Javier

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH RFC 0/7] ARM: OMAP: add DT binding for gpmc-smsc911x
@ 2013-02-09 20:44 ` Javier Martinez Canillas
  0 siblings, 0 replies; 46+ messages in thread
From: Javier Martinez Canillas @ 2013-02-09 20:44 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

This is an RFC to add Device Tree support for SMSC LAN chips connected
to OMAP processors though its General-Purpose Memory Controller.

The patch-set is composed of the following patches:

[PATCH RFC 1/7] platform: add a device node
[PATCH RFC 2/7] net: smsc911x: add pinctrl support
[PATCH RFC 3/7] ARM: OMAP: gpmc-smsc911x: add DT dev node init function
[PATCH RFC 4/7] ARM: OMAP: gpmc-smsc911x: pass a dev node to platform
[PATCH RFC 5/7] ARM: OMAP: gpmc: add support for gpmc-smsc911x child
[PATCH RFC 6/7] ARM: dts: OMAP: Add an GPMC node for OMAP3
[PATCH RFC 7/7] ARM: dts: omap3-igep0020: Add SMSC911x LAN chip support

It is an RFC because I had to modify the platform_device_register_resndata()
platform device registration function to allow passing a device node and
associate to the platform device after allocation. The patch-set updates all
current users and this shouldn't have a function effect on them.

I think that the need for platform code to pass the DT device node to the
platform registration infraestructure will be needed for other devices too
whose probe function needs to access the device node associated with the
device.

The binding has been tested on an TI OMAP3 based board (IGEPv2) and added
support is included on the RFC as an example.

It is based on Linus' latest master branch + linux-omap/omap-for-v3.9/gpmc
+ linux-omap-dt/for_3.9/dts

Thanks a lot and best regards,
Javier

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH RFC 1/7] platform: add a device node
  2013-02-09 20:44 ` Javier Martinez Canillas
@ 2013-02-09 20:44   ` Javier Martinez Canillas
  -1 siblings, 0 replies; 46+ messages in thread
From: Javier Martinez Canillas @ 2013-02-09 20:44 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Benoît Cousson, Russell King, Linus Walleij,
	Greg Kroah-Hartman, Enric Balletbo i Serra, Ezequiel Garcia,
	devicetree-discuss, linux-omap, linux-arm-kernel,
	Javier Martinez Canillas

When using Device Trees, it is necessary to associate a
device node with a platform device.

Usually this device node has to used in the device probe
function (e.g: to initizalize the pinctrl pads assocaited
with the device).

So, platform code needs to pass a device node as a platform
device info to the platform device registration function.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 arch/arm/mach-imx/devices/platform-gpio-mxc.c |    2 +-
 arch/arm/mach-imx/devices/platform-imx-dma.c  |    4 ++--
 arch/arm/mach-imx/mach-armadillo5x0.c         |    2 +-
 arch/arm/mach-imx/mach-imx27_visstrim_m10.c   |    3 ++-
 arch/arm/mach-imx/mach-mx1ads.c               |    2 +-
 arch/arm/mach-nomadik/cpu-8815.c              |    2 +-
 arch/arm/mach-omap2/fb.c                      |    2 +-
 arch/arm/mach-omap2/gpmc-smsc911x.c           |    2 +-
 arch/arm/mach-ux500/board-mop500-audio.c      |    2 +-
 arch/arm/mach-ux500/devices-common.c          |    3 ++-
 arch/arm/mach-ux500/devices-db8500.h          |    2 +-
 arch/unicore32/kernel/puv3-core.c             |    2 +-
 arch/unicore32/kernel/puv3-nb0916.c           |    2 +-
 drivers/base/platform.c                       |    1 +
 drivers/leds/leds-gpio-register.c             |    2 +-
 drivers/virtio/virtio_mmio.c                  |    2 +-
 include/linux/platform_device.h               |    9 ++++++---
 sound/soc/samsung/i2s.c                       |    2 +-
 18 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/arch/arm/mach-imx/devices/platform-gpio-mxc.c b/arch/arm/mach-imx/devices/platform-gpio-mxc.c
index 26483fa..4f4d8f9 100644
--- a/arch/arm/mach-imx/devices/platform-gpio-mxc.c
+++ b/arch/arm/mach-imx/devices/platform-gpio-mxc.c
@@ -28,5 +28,5 @@ struct platform_device *__init mxc_register_gpio(char *name, int id,
 	};
 
 	return platform_device_register_resndata(&mxc_aips_bus,
-			name, id, res, ARRAY_SIZE(res), NULL, 0);
+			name, id, res, ARRAY_SIZE(res), NULL, 0, NULL);
 }
diff --git a/arch/arm/mach-imx/devices/platform-imx-dma.c b/arch/arm/mach-imx/devices/platform-imx-dma.c
index ccdb5dc..1e3838c 100644
--- a/arch/arm/mach-imx/devices/platform-imx-dma.c
+++ b/arch/arm/mach-imx/devices/platform-imx-dma.c
@@ -28,7 +28,7 @@ struct platform_device __init __maybe_unused *imx_add_imx_dma(char *name,
 	};
 
 	return platform_device_register_resndata(&mxc_ahb_bus,
-			name, -1, res, ARRAY_SIZE(res), NULL, 0);
+			name, -1, res, ARRAY_SIZE(res), NULL, 0, NULL);
 }
 
 struct platform_device __init __maybe_unused *imx_add_imx_sdma(char *name,
@@ -47,5 +47,5 @@ struct platform_device __init __maybe_unused *imx_add_imx_sdma(char *name,
 	};
 
 	return platform_device_register_resndata(&mxc_ahb_bus, name,
-			-1, res, ARRAY_SIZE(res), pdata, sizeof(*pdata));
+			-1, res, ARRAY_SIZE(res), pdata, sizeof(*pdata), NULL);
 }
diff --git a/arch/arm/mach-imx/mach-armadillo5x0.c b/arch/arm/mach-imx/mach-armadillo5x0.c
index 59bd6b0..e664681 100644
--- a/arch/arm/mach-imx/mach-armadillo5x0.c
+++ b/arch/arm/mach-imx/mach-armadillo5x0.c
@@ -519,7 +519,7 @@ static void __init armadillo5x0_init(void)
 	platform_device_register_resndata(NULL, "physmap-flash", -1,
 			&armadillo5x0_nor_flash_resource, 1,
 			&armadillo5x0_nor_flash_pdata,
-			sizeof(armadillo5x0_nor_flash_pdata));
+			sizeof(armadillo5x0_nor_flash_pdata), NULL);
 
 	/* Register NAND Flash */
 	imx31_add_mxc_nand(&armadillo5x0_nand_board_info);
diff --git a/arch/arm/mach-imx/mach-imx27_visstrim_m10.c b/arch/arm/mach-imx/mach-imx27_visstrim_m10.c
index 318bd8d..5065f67 100644
--- a/arch/arm/mach-imx/mach-imx27_visstrim_m10.c
+++ b/arch/arm/mach-imx/mach-imx27_visstrim_m10.c
@@ -570,7 +570,8 @@ static void __init visstrim_m10_board_init(void)
 	imx_add_platform_device("mx27vis", 0, NULL, 0, &snd_mx27vis_pdata,
 				sizeof(snd_mx27vis_pdata));
 	platform_device_register_resndata(NULL, "soc-camera-pdrv", 0, NULL, 0,
-				      &iclink_tvp5150, sizeof(iclink_tvp5150));
+				      &iclink_tvp5150, sizeof(iclink_tvp5150),
+				      NULL);
 	gpio_led_register_device(0, &visstrim_m10_led_data);
 
 	/* Use mother board version to decide what video devices we shall use */
diff --git a/arch/arm/mach-imx/mach-mx1ads.c b/arch/arm/mach-imx/mach-mx1ads.c
index 06b4837..9ce64a5 100644
--- a/arch/arm/mach-imx/mach-mx1ads.c
+++ b/arch/arm/mach-imx/mach-mx1ads.c
@@ -118,7 +118,7 @@ static void __init mx1ads_init(void)
 	/* Physmap flash */
 	platform_device_register_resndata(NULL, "physmap-flash", 0,
 			&flash_resource, 1,
-			&mx1ads_flash_data, sizeof(mx1ads_flash_data));
+			&mx1ads_flash_data, sizeof(mx1ads_flash_data), NULL);
 
 	/* I2C */
 	i2c_register_board_info(0, mx1ads_i2c_devices,
diff --git a/arch/arm/mach-nomadik/cpu-8815.c b/arch/arm/mach-nomadik/cpu-8815.c
index 1273931..b117d6c 100644
--- a/arch/arm/mach-nomadik/cpu-8815.c
+++ b/arch/arm/mach-nomadik/cpu-8815.c
@@ -65,7 +65,7 @@ cpu8815_add_gpio(int id, resource_size_t addr, int irq,
 
 	return platform_device_register_resndata(NULL, "gpio", id,
 				resources, ARRAY_SIZE(resources),
-				pdata, sizeof(*pdata));
+				pdata, sizeof(*pdata), NULL);
 }
 
 void cpu8815_add_gpios(resource_size_t *base, int num, int irq,
diff --git a/arch/arm/mach-omap2/fb.c b/arch/arm/mach-omap2/fb.c
index 190ae49..ba2e5ae 100644
--- a/arch/arm/mach-omap2/fb.c
+++ b/arch/arm/mach-omap2/fb.c
@@ -81,7 +81,7 @@ static int __init omap_init_vrfb(void)
 	}
 
 	pdev = platform_device_register_resndata(NULL, "omapvrfb", -1,
-			res, num_res, NULL, 0);
+			res, num_res, NULL, 0, NULL);
 
 	if (IS_ERR(pdev))
 		return PTR_ERR(pdev);
diff --git a/arch/arm/mach-omap2/gpmc-smsc911x.c b/arch/arm/mach-omap2/gpmc-smsc911x.c
index ef99011..5ce00ad2 100644
--- a/arch/arm/mach-omap2/gpmc-smsc911x.c
+++ b/arch/arm/mach-omap2/gpmc-smsc911x.c
@@ -82,7 +82,7 @@ void __init gpmc_smsc911x_init(struct omap_smsc911x_platform_data *gpmc_cfg)
 
 	pdev = platform_device_register_resndata(NULL, "smsc911x", gpmc_cfg->id,
 		 gpmc_smsc911x_resources, ARRAY_SIZE(gpmc_smsc911x_resources),
-		 &gpmc_smsc911x_config, sizeof(gpmc_smsc911x_config));
+		 &gpmc_smsc911x_config, sizeof(gpmc_smsc911x_config), NULL);
 	if (!pdev) {
 		pr_err("Unable to register platform device\n");
 		gpio_free(gpmc_cfg->gpio_reset);
diff --git a/arch/arm/mach-ux500/board-mop500-audio.c b/arch/arm/mach-ux500/board-mop500-audio.c
index 7209db7..5ebbc65 100644
--- a/arch/arm/mach-ux500/board-mop500-audio.c
+++ b/arch/arm/mach-ux500/board-mop500-audio.c
@@ -129,7 +129,7 @@ static struct platform_device *db8500_add_msp_i2s(struct device *parent,
 		id, irq);
 	pdev = platform_device_register_resndata(parent, "ux500-msp-i2s", id,
 						res, ARRAY_SIZE(res),
-						pdata, sizeof(*pdata));
+						pdata, sizeof(*pdata), NULL);
 	if (!pdev) {
 		pr_err("Failed to register platform-device 'ux500-msp-i2s.%d'!\n",
 			id);
diff --git a/arch/arm/mach-ux500/devices-common.c b/arch/arm/mach-ux500/devices-common.c
index 16b5f71..dcbe6c2 100644
--- a/arch/arm/mach-ux500/devices-common.c
+++ b/arch/arm/mach-ux500/devices-common.c
@@ -42,7 +42,8 @@ dbx500_add_gpio(struct device *parent, int id, resource_size_t addr, int irq,
 		resources,
 		ARRAY_SIZE(resources),
 		pdata,
-		sizeof(*pdata));
+		sizeof(*pdata),
+		NULL);
 }
 
 void dbx500_add_gpios(struct device *parent, resource_size_t *base, int num,
diff --git a/arch/arm/mach-ux500/devices-db8500.h b/arch/arm/mach-ux500/devices-db8500.h
index a5e05f6..5a8ed73 100644
--- a/arch/arm/mach-ux500/devices-db8500.h
+++ b/arch/arm/mach-ux500/devices-db8500.h
@@ -26,7 +26,7 @@ db8500_add_ske_keypad(struct device *parent,
 	};
 
 	return platform_device_register_resndata(parent, "nmk-ske-keypad", -1,
-						 resources, 2, pdata, size);
+						 resources, 2, pdata, size, NULL);
 }
 
 static inline struct amba_device *
diff --git a/arch/unicore32/kernel/puv3-core.c b/arch/unicore32/kernel/puv3-core.c
index 254adee..8143a18 100644
--- a/arch/unicore32/kernel/puv3-core.c
+++ b/arch/unicore32/kernel/puv3-core.c
@@ -274,6 +274,6 @@ void __init puv3_core_init(void)
 	platform_device_register_simple("PKUnity-v3-AC97", -1, NULL, 0);
 	platform_device_register_resndata(&platform_bus, "musb_hdrc", -1,
 			puv3_usb_resources, ARRAY_SIZE(puv3_usb_resources),
-			&puv3_usb_plat, sizeof(puv3_usb_plat));
+			&puv3_usb_plat, sizeof(puv3_usb_plat), NULL);
 }
 
diff --git a/arch/unicore32/kernel/puv3-nb0916.c b/arch/unicore32/kernel/puv3-nb0916.c
index 181108b..b2a55cb 100644
--- a/arch/unicore32/kernel/puv3-nb0916.c
+++ b/arch/unicore32/kernel/puv3-nb0916.c
@@ -119,7 +119,7 @@ int __init mach_nb0916_init(void)
 
 	platform_device_register_resndata(&platform_bus, "physmap-flash", -1,
 			&physmap_flash_resource, 1,
-			&physmap_flash_data, sizeof(physmap_flash_data));
+			&physmap_flash_data, sizeof(physmap_flash_data), NULL);
 
 	if (request_irq(gpio_to_irq(GPI_LCD_CASE_OFF),
 		&nb0916_lcdcaseoff_handler,
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index c0b8df3..79ba66a 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -437,6 +437,7 @@ struct platform_device *platform_device_register_full(
 		goto err_alloc;
 
 	pdev->dev.parent = pdevinfo->parent;
+	pdev->dev.of_node = pdevinfo->of_node;
 	ACPI_HANDLE_SET(&pdev->dev, pdevinfo->acpi_node.handle);
 
 	if (pdevinfo->dma_mask) {
diff --git a/drivers/leds/leds-gpio-register.c b/drivers/leds/leds-gpio-register.c
index 1c4ed55..e1f1c15 100644
--- a/drivers/leds/leds-gpio-register.c
+++ b/drivers/leds/leds-gpio-register.c
@@ -34,7 +34,7 @@ struct platform_device *__init gpio_led_register_device(
 		return ERR_PTR(-ENOMEM);
 
 	ret = platform_device_register_resndata(NULL, "leds-gpio", id,
-			NULL, 0, &_pdata, sizeof(_pdata));
+			NULL, 0, &_pdata, sizeof(_pdata), NULL);
 	if (IS_ERR(ret))
 		kfree(_pdata.leds);
 
diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index 31f966f..0709ad6 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -566,7 +566,7 @@ static int vm_cmdline_set(const char *device,
 
 	pdev = platform_device_register_resndata(&vm_cmdline_parent,
 			"virtio-mmio", vm_cmdline_id++,
-			resources, ARRAY_SIZE(resources), NULL, 0);
+			resources, ARRAY_SIZE(resources), NULL, 0, NULL);
 	if (IS_ERR(pdev))
 		return PTR_ERR(pdev);
 
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index a9ded9a..29393bd 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -66,6 +66,7 @@ struct platform_device_info {
 		const void *data;
 		size_t size_data;
 		u64 dma_mask;
+		struct device_node *of_node;
 };
 extern struct platform_device *platform_device_register_full(
 		const struct platform_device_info *pdevinfo);
@@ -81,13 +82,14 @@ extern struct platform_device *platform_device_register_full(
  * @num: number of resources
  * @data: platform specific data for this platform device
  * @size: size of platform specific data
+ * @of_node: device node of this platform device
  *
  * Returns &struct platform_device pointer on success, or ERR_PTR() on error.
  */
 static inline struct platform_device *platform_device_register_resndata(
 		struct device *parent, const char *name, int id,
 		const struct resource *res, unsigned int num,
-		const void *data, size_t size) {
+		const void *data, size_t size, struct device_node *of_node) {
 
 	struct platform_device_info pdevinfo = {
 		.parent = parent,
@@ -98,6 +100,7 @@ static inline struct platform_device *platform_device_register_resndata(
 		.data = data,
 		.size_data = size,
 		.dma_mask = 0,
+		.of_node = of_node,
 	};
 
 	return platform_device_register_full(&pdevinfo);
@@ -130,7 +133,7 @@ static inline struct platform_device *platform_device_register_simple(
 		const struct resource *res, unsigned int num)
 {
 	return platform_device_register_resndata(NULL, name, id,
-			res, num, NULL, 0);
+			res, num, NULL, 0, NULL);
 }
 
 /**
@@ -154,7 +157,7 @@ static inline struct platform_device *platform_device_register_data(
 		const void *data, size_t size)
 {
 	return platform_device_register_resndata(parent, name, id,
-			NULL, 0, data, size);
+			NULL, 0, data, size, NULL);
 }
 
 extern struct platform_device *platform_device_alloc(const char *name, int id);
diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
index d2d124f..e6eabb9 100644
--- a/sound/soc/samsung/i2s.c
+++ b/sound/soc/samsung/i2s.c
@@ -982,7 +982,7 @@ static struct i2s_dai *i2s_alloc_dai(struct platform_device *pdev, bool sec)
 	} else {	/* Create a new platform_device for Secondary */
 		i2s->pdev = platform_device_register_resndata(NULL,
 				pdev->name, pdev->id + SAMSUNG_I2S_SECOFF,
-				NULL, 0, NULL, 0);
+				NULL, 0, NULL, 0, NULL);
 		if (IS_ERR(i2s->pdev))
 			return NULL;
 	}
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH RFC 1/7] platform: add a device node
@ 2013-02-09 20:44   ` Javier Martinez Canillas
  0 siblings, 0 replies; 46+ messages in thread
From: Javier Martinez Canillas @ 2013-02-09 20:44 UTC (permalink / raw)
  To: linux-arm-kernel

When using Device Trees, it is necessary to associate a
device node with a platform device.

Usually this device node has to used in the device probe
function (e.g: to initizalize the pinctrl pads assocaited
with the device).

So, platform code needs to pass a device node as a platform
device info to the platform device registration function.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 arch/arm/mach-imx/devices/platform-gpio-mxc.c |    2 +-
 arch/arm/mach-imx/devices/platform-imx-dma.c  |    4 ++--
 arch/arm/mach-imx/mach-armadillo5x0.c         |    2 +-
 arch/arm/mach-imx/mach-imx27_visstrim_m10.c   |    3 ++-
 arch/arm/mach-imx/mach-mx1ads.c               |    2 +-
 arch/arm/mach-nomadik/cpu-8815.c              |    2 +-
 arch/arm/mach-omap2/fb.c                      |    2 +-
 arch/arm/mach-omap2/gpmc-smsc911x.c           |    2 +-
 arch/arm/mach-ux500/board-mop500-audio.c      |    2 +-
 arch/arm/mach-ux500/devices-common.c          |    3 ++-
 arch/arm/mach-ux500/devices-db8500.h          |    2 +-
 arch/unicore32/kernel/puv3-core.c             |    2 +-
 arch/unicore32/kernel/puv3-nb0916.c           |    2 +-
 drivers/base/platform.c                       |    1 +
 drivers/leds/leds-gpio-register.c             |    2 +-
 drivers/virtio/virtio_mmio.c                  |    2 +-
 include/linux/platform_device.h               |    9 ++++++---
 sound/soc/samsung/i2s.c                       |    2 +-
 18 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/arch/arm/mach-imx/devices/platform-gpio-mxc.c b/arch/arm/mach-imx/devices/platform-gpio-mxc.c
index 26483fa..4f4d8f9 100644
--- a/arch/arm/mach-imx/devices/platform-gpio-mxc.c
+++ b/arch/arm/mach-imx/devices/platform-gpio-mxc.c
@@ -28,5 +28,5 @@ struct platform_device *__init mxc_register_gpio(char *name, int id,
 	};
 
 	return platform_device_register_resndata(&mxc_aips_bus,
-			name, id, res, ARRAY_SIZE(res), NULL, 0);
+			name, id, res, ARRAY_SIZE(res), NULL, 0, NULL);
 }
diff --git a/arch/arm/mach-imx/devices/platform-imx-dma.c b/arch/arm/mach-imx/devices/platform-imx-dma.c
index ccdb5dc..1e3838c 100644
--- a/arch/arm/mach-imx/devices/platform-imx-dma.c
+++ b/arch/arm/mach-imx/devices/platform-imx-dma.c
@@ -28,7 +28,7 @@ struct platform_device __init __maybe_unused *imx_add_imx_dma(char *name,
 	};
 
 	return platform_device_register_resndata(&mxc_ahb_bus,
-			name, -1, res, ARRAY_SIZE(res), NULL, 0);
+			name, -1, res, ARRAY_SIZE(res), NULL, 0, NULL);
 }
 
 struct platform_device __init __maybe_unused *imx_add_imx_sdma(char *name,
@@ -47,5 +47,5 @@ struct platform_device __init __maybe_unused *imx_add_imx_sdma(char *name,
 	};
 
 	return platform_device_register_resndata(&mxc_ahb_bus, name,
-			-1, res, ARRAY_SIZE(res), pdata, sizeof(*pdata));
+			-1, res, ARRAY_SIZE(res), pdata, sizeof(*pdata), NULL);
 }
diff --git a/arch/arm/mach-imx/mach-armadillo5x0.c b/arch/arm/mach-imx/mach-armadillo5x0.c
index 59bd6b0..e664681 100644
--- a/arch/arm/mach-imx/mach-armadillo5x0.c
+++ b/arch/arm/mach-imx/mach-armadillo5x0.c
@@ -519,7 +519,7 @@ static void __init armadillo5x0_init(void)
 	platform_device_register_resndata(NULL, "physmap-flash", -1,
 			&armadillo5x0_nor_flash_resource, 1,
 			&armadillo5x0_nor_flash_pdata,
-			sizeof(armadillo5x0_nor_flash_pdata));
+			sizeof(armadillo5x0_nor_flash_pdata), NULL);
 
 	/* Register NAND Flash */
 	imx31_add_mxc_nand(&armadillo5x0_nand_board_info);
diff --git a/arch/arm/mach-imx/mach-imx27_visstrim_m10.c b/arch/arm/mach-imx/mach-imx27_visstrim_m10.c
index 318bd8d..5065f67 100644
--- a/arch/arm/mach-imx/mach-imx27_visstrim_m10.c
+++ b/arch/arm/mach-imx/mach-imx27_visstrim_m10.c
@@ -570,7 +570,8 @@ static void __init visstrim_m10_board_init(void)
 	imx_add_platform_device("mx27vis", 0, NULL, 0, &snd_mx27vis_pdata,
 				sizeof(snd_mx27vis_pdata));
 	platform_device_register_resndata(NULL, "soc-camera-pdrv", 0, NULL, 0,
-				      &iclink_tvp5150, sizeof(iclink_tvp5150));
+				      &iclink_tvp5150, sizeof(iclink_tvp5150),
+				      NULL);
 	gpio_led_register_device(0, &visstrim_m10_led_data);
 
 	/* Use mother board version to decide what video devices we shall use */
diff --git a/arch/arm/mach-imx/mach-mx1ads.c b/arch/arm/mach-imx/mach-mx1ads.c
index 06b4837..9ce64a5 100644
--- a/arch/arm/mach-imx/mach-mx1ads.c
+++ b/arch/arm/mach-imx/mach-mx1ads.c
@@ -118,7 +118,7 @@ static void __init mx1ads_init(void)
 	/* Physmap flash */
 	platform_device_register_resndata(NULL, "physmap-flash", 0,
 			&flash_resource, 1,
-			&mx1ads_flash_data, sizeof(mx1ads_flash_data));
+			&mx1ads_flash_data, sizeof(mx1ads_flash_data), NULL);
 
 	/* I2C */
 	i2c_register_board_info(0, mx1ads_i2c_devices,
diff --git a/arch/arm/mach-nomadik/cpu-8815.c b/arch/arm/mach-nomadik/cpu-8815.c
index 1273931..b117d6c 100644
--- a/arch/arm/mach-nomadik/cpu-8815.c
+++ b/arch/arm/mach-nomadik/cpu-8815.c
@@ -65,7 +65,7 @@ cpu8815_add_gpio(int id, resource_size_t addr, int irq,
 
 	return platform_device_register_resndata(NULL, "gpio", id,
 				resources, ARRAY_SIZE(resources),
-				pdata, sizeof(*pdata));
+				pdata, sizeof(*pdata), NULL);
 }
 
 void cpu8815_add_gpios(resource_size_t *base, int num, int irq,
diff --git a/arch/arm/mach-omap2/fb.c b/arch/arm/mach-omap2/fb.c
index 190ae49..ba2e5ae 100644
--- a/arch/arm/mach-omap2/fb.c
+++ b/arch/arm/mach-omap2/fb.c
@@ -81,7 +81,7 @@ static int __init omap_init_vrfb(void)
 	}
 
 	pdev = platform_device_register_resndata(NULL, "omapvrfb", -1,
-			res, num_res, NULL, 0);
+			res, num_res, NULL, 0, NULL);
 
 	if (IS_ERR(pdev))
 		return PTR_ERR(pdev);
diff --git a/arch/arm/mach-omap2/gpmc-smsc911x.c b/arch/arm/mach-omap2/gpmc-smsc911x.c
index ef99011..5ce00ad2 100644
--- a/arch/arm/mach-omap2/gpmc-smsc911x.c
+++ b/arch/arm/mach-omap2/gpmc-smsc911x.c
@@ -82,7 +82,7 @@ void __init gpmc_smsc911x_init(struct omap_smsc911x_platform_data *gpmc_cfg)
 
 	pdev = platform_device_register_resndata(NULL, "smsc911x", gpmc_cfg->id,
 		 gpmc_smsc911x_resources, ARRAY_SIZE(gpmc_smsc911x_resources),
-		 &gpmc_smsc911x_config, sizeof(gpmc_smsc911x_config));
+		 &gpmc_smsc911x_config, sizeof(gpmc_smsc911x_config), NULL);
 	if (!pdev) {
 		pr_err("Unable to register platform device\n");
 		gpio_free(gpmc_cfg->gpio_reset);
diff --git a/arch/arm/mach-ux500/board-mop500-audio.c b/arch/arm/mach-ux500/board-mop500-audio.c
index 7209db7..5ebbc65 100644
--- a/arch/arm/mach-ux500/board-mop500-audio.c
+++ b/arch/arm/mach-ux500/board-mop500-audio.c
@@ -129,7 +129,7 @@ static struct platform_device *db8500_add_msp_i2s(struct device *parent,
 		id, irq);
 	pdev = platform_device_register_resndata(parent, "ux500-msp-i2s", id,
 						res, ARRAY_SIZE(res),
-						pdata, sizeof(*pdata));
+						pdata, sizeof(*pdata), NULL);
 	if (!pdev) {
 		pr_err("Failed to register platform-device 'ux500-msp-i2s.%d'!\n",
 			id);
diff --git a/arch/arm/mach-ux500/devices-common.c b/arch/arm/mach-ux500/devices-common.c
index 16b5f71..dcbe6c2 100644
--- a/arch/arm/mach-ux500/devices-common.c
+++ b/arch/arm/mach-ux500/devices-common.c
@@ -42,7 +42,8 @@ dbx500_add_gpio(struct device *parent, int id, resource_size_t addr, int irq,
 		resources,
 		ARRAY_SIZE(resources),
 		pdata,
-		sizeof(*pdata));
+		sizeof(*pdata),
+		NULL);
 }
 
 void dbx500_add_gpios(struct device *parent, resource_size_t *base, int num,
diff --git a/arch/arm/mach-ux500/devices-db8500.h b/arch/arm/mach-ux500/devices-db8500.h
index a5e05f6..5a8ed73 100644
--- a/arch/arm/mach-ux500/devices-db8500.h
+++ b/arch/arm/mach-ux500/devices-db8500.h
@@ -26,7 +26,7 @@ db8500_add_ske_keypad(struct device *parent,
 	};
 
 	return platform_device_register_resndata(parent, "nmk-ske-keypad", -1,
-						 resources, 2, pdata, size);
+						 resources, 2, pdata, size, NULL);
 }
 
 static inline struct amba_device *
diff --git a/arch/unicore32/kernel/puv3-core.c b/arch/unicore32/kernel/puv3-core.c
index 254adee..8143a18 100644
--- a/arch/unicore32/kernel/puv3-core.c
+++ b/arch/unicore32/kernel/puv3-core.c
@@ -274,6 +274,6 @@ void __init puv3_core_init(void)
 	platform_device_register_simple("PKUnity-v3-AC97", -1, NULL, 0);
 	platform_device_register_resndata(&platform_bus, "musb_hdrc", -1,
 			puv3_usb_resources, ARRAY_SIZE(puv3_usb_resources),
-			&puv3_usb_plat, sizeof(puv3_usb_plat));
+			&puv3_usb_plat, sizeof(puv3_usb_plat), NULL);
 }
 
diff --git a/arch/unicore32/kernel/puv3-nb0916.c b/arch/unicore32/kernel/puv3-nb0916.c
index 181108b..b2a55cb 100644
--- a/arch/unicore32/kernel/puv3-nb0916.c
+++ b/arch/unicore32/kernel/puv3-nb0916.c
@@ -119,7 +119,7 @@ int __init mach_nb0916_init(void)
 
 	platform_device_register_resndata(&platform_bus, "physmap-flash", -1,
 			&physmap_flash_resource, 1,
-			&physmap_flash_data, sizeof(physmap_flash_data));
+			&physmap_flash_data, sizeof(physmap_flash_data), NULL);
 
 	if (request_irq(gpio_to_irq(GPI_LCD_CASE_OFF),
 		&nb0916_lcdcaseoff_handler,
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index c0b8df3..79ba66a 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -437,6 +437,7 @@ struct platform_device *platform_device_register_full(
 		goto err_alloc;
 
 	pdev->dev.parent = pdevinfo->parent;
+	pdev->dev.of_node = pdevinfo->of_node;
 	ACPI_HANDLE_SET(&pdev->dev, pdevinfo->acpi_node.handle);
 
 	if (pdevinfo->dma_mask) {
diff --git a/drivers/leds/leds-gpio-register.c b/drivers/leds/leds-gpio-register.c
index 1c4ed55..e1f1c15 100644
--- a/drivers/leds/leds-gpio-register.c
+++ b/drivers/leds/leds-gpio-register.c
@@ -34,7 +34,7 @@ struct platform_device *__init gpio_led_register_device(
 		return ERR_PTR(-ENOMEM);
 
 	ret = platform_device_register_resndata(NULL, "leds-gpio", id,
-			NULL, 0, &_pdata, sizeof(_pdata));
+			NULL, 0, &_pdata, sizeof(_pdata), NULL);
 	if (IS_ERR(ret))
 		kfree(_pdata.leds);
 
diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index 31f966f..0709ad6 100644
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -566,7 +566,7 @@ static int vm_cmdline_set(const char *device,
 
 	pdev = platform_device_register_resndata(&vm_cmdline_parent,
 			"virtio-mmio", vm_cmdline_id++,
-			resources, ARRAY_SIZE(resources), NULL, 0);
+			resources, ARRAY_SIZE(resources), NULL, 0, NULL);
 	if (IS_ERR(pdev))
 		return PTR_ERR(pdev);
 
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index a9ded9a..29393bd 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -66,6 +66,7 @@ struct platform_device_info {
 		const void *data;
 		size_t size_data;
 		u64 dma_mask;
+		struct device_node *of_node;
 };
 extern struct platform_device *platform_device_register_full(
 		const struct platform_device_info *pdevinfo);
@@ -81,13 +82,14 @@ extern struct platform_device *platform_device_register_full(
  * @num: number of resources
  * @data: platform specific data for this platform device
  * @size: size of platform specific data
+ * @of_node: device node of this platform device
  *
  * Returns &struct platform_device pointer on success, or ERR_PTR() on error.
  */
 static inline struct platform_device *platform_device_register_resndata(
 		struct device *parent, const char *name, int id,
 		const struct resource *res, unsigned int num,
-		const void *data, size_t size) {
+		const void *data, size_t size, struct device_node *of_node) {
 
 	struct platform_device_info pdevinfo = {
 		.parent = parent,
@@ -98,6 +100,7 @@ static inline struct platform_device *platform_device_register_resndata(
 		.data = data,
 		.size_data = size,
 		.dma_mask = 0,
+		.of_node = of_node,
 	};
 
 	return platform_device_register_full(&pdevinfo);
@@ -130,7 +133,7 @@ static inline struct platform_device *platform_device_register_simple(
 		const struct resource *res, unsigned int num)
 {
 	return platform_device_register_resndata(NULL, name, id,
-			res, num, NULL, 0);
+			res, num, NULL, 0, NULL);
 }
 
 /**
@@ -154,7 +157,7 @@ static inline struct platform_device *platform_device_register_data(
 		const void *data, size_t size)
 {
 	return platform_device_register_resndata(parent, name, id,
-			NULL, 0, data, size);
+			NULL, 0, data, size, NULL);
 }
 
 extern struct platform_device *platform_device_alloc(const char *name, int id);
diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
index d2d124f..e6eabb9 100644
--- a/sound/soc/samsung/i2s.c
+++ b/sound/soc/samsung/i2s.c
@@ -982,7 +982,7 @@ static struct i2s_dai *i2s_alloc_dai(struct platform_device *pdev, bool sec)
 	} else {	/* Create a new platform_device for Secondary */
 		i2s->pdev = platform_device_register_resndata(NULL,
 				pdev->name, pdev->id + SAMSUNG_I2S_SECOFF,
-				NULL, 0, NULL, 0);
+				NULL, 0, NULL, 0, NULL);
 		if (IS_ERR(i2s->pdev))
 			return NULL;
 	}
-- 
1.7.7.6

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH RFC 2/7] net: smsc911x: add pinctrl support
  2013-02-09 20:44 ` Javier Martinez Canillas
@ 2013-02-09 20:44   ` Javier Martinez Canillas
  -1 siblings, 0 replies; 46+ messages in thread
From: Javier Martinez Canillas @ 2013-02-09 20:44 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Benoît Cousson, Russell King, Linus Walleij,
	Greg Kroah-Hartman, Enric Balletbo i Serra, Ezequiel Garcia,
	devicetree-discuss, linux-omap, linux-arm-kernel,
	Javier Martinez Canillas

If no pinctrl is available just report a warning since
it may not needed in some cases (e.g: non-DT kernels).

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 drivers/net/ethernet/smsc/smsc911x.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
index e112877..40766c7 100644
--- a/drivers/net/ethernet/smsc/smsc911x.c
+++ b/drivers/net/ethernet/smsc/smsc911x.c
@@ -59,6 +59,7 @@
 #include <linux/of_device.h>
 #include <linux/of_gpio.h>
 #include <linux/of_net.h>
+#include <linux/pinctrl/consumer.h>
 #include "smsc911x.h"
 
 #define SMSC_CHIPNAME		"smsc911x"
@@ -2352,6 +2353,7 @@ static int smsc911x_drv_probe(struct platform_device *pdev)
 	struct smsc911x_data *pdata;
 	struct smsc911x_platform_config *config = pdev->dev.platform_data;
 	struct resource *res, *irq_res;
+	struct pinctrl *pinctrl;
 	unsigned int intcfg = 0;
 	int res_size, irq_flags;
 	int retval;
@@ -2381,6 +2383,15 @@ static int smsc911x_drv_probe(struct platform_device *pdev)
 		goto out_0;
 	}
 
+	pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
+	if (IS_ERR(pinctrl)) {
+		retval = PTR_ERR(pinctrl);
+		if (retval == -EPROBE_DEFER)
+			goto out_0;
+
+		dev_warn(&pdev->dev, "No pinctrl provided\n");
+	}
+
 	dev = alloc_etherdev(sizeof(struct smsc911x_data));
 	if (!dev) {
 		retval = -ENOMEM;
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH RFC 2/7] net: smsc911x: add pinctrl support
@ 2013-02-09 20:44   ` Javier Martinez Canillas
  0 siblings, 0 replies; 46+ messages in thread
From: Javier Martinez Canillas @ 2013-02-09 20:44 UTC (permalink / raw)
  To: linux-arm-kernel

If no pinctrl is available just report a warning since
it may not needed in some cases (e.g: non-DT kernels).

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 drivers/net/ethernet/smsc/smsc911x.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c
index e112877..40766c7 100644
--- a/drivers/net/ethernet/smsc/smsc911x.c
+++ b/drivers/net/ethernet/smsc/smsc911x.c
@@ -59,6 +59,7 @@
 #include <linux/of_device.h>
 #include <linux/of_gpio.h>
 #include <linux/of_net.h>
+#include <linux/pinctrl/consumer.h>
 #include "smsc911x.h"
 
 #define SMSC_CHIPNAME		"smsc911x"
@@ -2352,6 +2353,7 @@ static int smsc911x_drv_probe(struct platform_device *pdev)
 	struct smsc911x_data *pdata;
 	struct smsc911x_platform_config *config = pdev->dev.platform_data;
 	struct resource *res, *irq_res;
+	struct pinctrl *pinctrl;
 	unsigned int intcfg = 0;
 	int res_size, irq_flags;
 	int retval;
@@ -2381,6 +2383,15 @@ static int smsc911x_drv_probe(struct platform_device *pdev)
 		goto out_0;
 	}
 
+	pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
+	if (IS_ERR(pinctrl)) {
+		retval = PTR_ERR(pinctrl);
+		if (retval == -EPROBE_DEFER)
+			goto out_0;
+
+		dev_warn(&pdev->dev, "No pinctrl provided\n");
+	}
+
 	dev = alloc_etherdev(sizeof(struct smsc911x_data));
 	if (!dev) {
 		retval = -ENOMEM;
-- 
1.7.7.6

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH RFC 3/7] ARM: OMAP: gpmc-smsc911x: add DT dev node init function
  2013-02-09 20:44 ` Javier Martinez Canillas
@ 2013-02-09 20:44   ` Javier Martinez Canillas
  -1 siblings, 0 replies; 46+ messages in thread
From: Javier Martinez Canillas @ 2013-02-09 20:44 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Benoît Cousson, Russell King, Linus Walleij,
	Greg Kroah-Hartman, Enric Balletbo i Serra, Ezequiel Garcia,
	devicetree-discuss, linux-omap, linux-arm-kernel,
	Javier Martinez Canillas

This patch adds a helper function to parse a device node that
contains all the properties needed to initialize an smsc911x
device connected to an OMAP processor through OMAP's GPMC.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 .../devicetree/bindings/net/gpmc-smsc911x.txt      |   39 ++++++++++++++++++++
 arch/arm/mach-omap2/gpmc-smsc911x.c                |   29 +++++++++++++++
 arch/arm/mach-omap2/gpmc-smsc911x.h                |    6 +++
 3 files changed, 74 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/gpmc-smsc911x.txt

diff --git a/Documentation/devicetree/bindings/net/gpmc-smsc911x.txt b/Documentation/devicetree/bindings/net/gpmc-smsc911x.txt
new file mode 100644
index 0000000..8bb0df2
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/gpmc-smsc911x.txt
@@ -0,0 +1,39 @@
+* GPMC connected Smart Mixed-Signal Connectivity (SMSC) LAN911x/912x Controller
+
+GPMC connected SMSC LAN911x/912x Controllers are represented as child nodes
+of the OMAP General-Purpose Memory Controller with a name of "gpmc_smsc911x".
+
+All timing relevant properties as well as generic gpmc child properties are
+explained in a separate documents - please refer to
+Documentation/devicetree/bindings/bus/ti-gpmc.txt
+
+Required properties:
+- gpmc,cs : The chip select line the pheripheral is connected to
+- gpmc,gpio_irq : The GPIO pin that is connected to the SMSC LAN IRQ line
+
+Optional properties:
+- gpmc,flags : SMSC LAN flags - please refer to include/linux/smsc911x.h
+- gpmc,gpio_reset : The GPIO pin connected to the SMSC LAN reset line
+
+Note: Besides these properties, the gpmc_smsc911x device node could need
+aditional setup such as pin/pad mux settings and voltage regulators. This
+depend on how the pheripheral is wired and his board specific.
+
+Example (for an OMAP3 board):
+
+gpmc@6e000000 {
+	      compatible = "ti,omap3430-gpmc";
+	      ti,hwmods = "gpmc";
+	      reg = <0x6e000000 0x1000000>;
+	      interrupts = <20>;
+	      gpmc,num-cs = <8>;
+	      gpmc,num-waitpins = <4>;
+	      #address-cells = <2>;
+	      #size-cells = <1>;
+
+	      gpmc_smsc911x@0 {
+			      gpmc,cs = <5>;
+			      gpmc,gpio_irq = <176>;
+			      gpmc,flags = <4>; /* SMSC911X_USE_32BIT */
+	      };
+};
diff --git a/arch/arm/mach-omap2/gpmc-smsc911x.c b/arch/arm/mach-omap2/gpmc-smsc911x.c
index 5ce00ad2..59a2ee4 100644
--- a/arch/arm/mach-omap2/gpmc-smsc911x.c
+++ b/arch/arm/mach-omap2/gpmc-smsc911x.c
@@ -19,6 +19,7 @@
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/smsc911x.h>
+#include <linux/of.h>
 
 #include "gpmc.h"
 #include "gpmc-smsc911x.h"
@@ -98,3 +99,31 @@ free1:
 
 	pr_err("Could not initialize smsc911x device\n");
 }
+
+int gpmc_smsc911x_init_dt(struct device_node *node)
+{
+	struct omap_smsc911x_platform_data gpmc_cfg;
+
+	if (WARN_ON(!node))
+		return -ENODEV;
+
+	if (of_property_read_u32(node, "gpmc,cs", &gpmc_cfg.cs)) {
+		pr_err("Unable to get GPMC smsc911x chip select\n");
+		return -EINVAL;
+	}
+
+	if (of_property_read_u32(node, "gpmc,gpio_irq", &gpmc_cfg.gpio_irq)) {
+		pr_err("Unable to get GPMC smsc911x GPIO IRQ\n");
+		return -EINVAL;
+	}
+
+	if (of_property_read_u32(node, "gpmc,gpio_reset", &gpmc_cfg.gpio_reset))
+		gpmc_cfg.gpio_reset = -EINVAL;
+
+	if (of_property_read_u32(node, "gpmc,flags", &gpmc_cfg.flags))
+		gpmc_cfg.flags = 0;
+
+	gpmc_smsc911x_init(&gpmc_cfg);
+
+	return 0;
+}
diff --git a/arch/arm/mach-omap2/gpmc-smsc911x.h b/arch/arm/mach-omap2/gpmc-smsc911x.h
index ea6c9c8..bbcb8bc 100644
--- a/arch/arm/mach-omap2/gpmc-smsc911x.h
+++ b/arch/arm/mach-omap2/gpmc-smsc911x.h
@@ -25,11 +25,17 @@ struct omap_smsc911x_platform_data {
 
 extern void gpmc_smsc911x_init(struct omap_smsc911x_platform_data *d);
 
+extern int gpmc_smsc911x_init_dt(struct device_node *node);
+
 #else
 
 static inline void gpmc_smsc911x_init(struct omap_smsc911x_platform_data *d)
 {
 }
 
+static inline int gpmc_smsc911x_init_dt(struct device_node *node)
+{
+}
+
 #endif
 #endif
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH RFC 3/7] ARM: OMAP: gpmc-smsc911x: add DT dev node init function
@ 2013-02-09 20:44   ` Javier Martinez Canillas
  0 siblings, 0 replies; 46+ messages in thread
From: Javier Martinez Canillas @ 2013-02-09 20:44 UTC (permalink / raw)
  To: linux-arm-kernel

This patch adds a helper function to parse a device node that
contains all the properties needed to initialize an smsc911x
device connected to an OMAP processor through OMAP's GPMC.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 .../devicetree/bindings/net/gpmc-smsc911x.txt      |   39 ++++++++++++++++++++
 arch/arm/mach-omap2/gpmc-smsc911x.c                |   29 +++++++++++++++
 arch/arm/mach-omap2/gpmc-smsc911x.h                |    6 +++
 3 files changed, 74 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/gpmc-smsc911x.txt

diff --git a/Documentation/devicetree/bindings/net/gpmc-smsc911x.txt b/Documentation/devicetree/bindings/net/gpmc-smsc911x.txt
new file mode 100644
index 0000000..8bb0df2
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/gpmc-smsc911x.txt
@@ -0,0 +1,39 @@
+* GPMC connected Smart Mixed-Signal Connectivity (SMSC) LAN911x/912x Controller
+
+GPMC connected SMSC LAN911x/912x Controllers are represented as child nodes
+of the OMAP General-Purpose Memory Controller with a name of "gpmc_smsc911x".
+
+All timing relevant properties as well as generic gpmc child properties are
+explained in a separate documents - please refer to
+Documentation/devicetree/bindings/bus/ti-gpmc.txt
+
+Required properties:
+- gpmc,cs : The chip select line the pheripheral is connected to
+- gpmc,gpio_irq : The GPIO pin that is connected to the SMSC LAN IRQ line
+
+Optional properties:
+- gpmc,flags : SMSC LAN flags - please refer to include/linux/smsc911x.h
+- gpmc,gpio_reset : The GPIO pin connected to the SMSC LAN reset line
+
+Note: Besides these properties, the gpmc_smsc911x device node could need
+aditional setup such as pin/pad mux settings and voltage regulators. This
+depend on how the pheripheral is wired and his board specific.
+
+Example (for an OMAP3 board):
+
+gpmc at 6e000000 {
+	      compatible = "ti,omap3430-gpmc";
+	      ti,hwmods = "gpmc";
+	      reg = <0x6e000000 0x1000000>;
+	      interrupts = <20>;
+	      gpmc,num-cs = <8>;
+	      gpmc,num-waitpins = <4>;
+	      #address-cells = <2>;
+	      #size-cells = <1>;
+
+	      gpmc_smsc911x at 0 {
+			      gpmc,cs = <5>;
+			      gpmc,gpio_irq = <176>;
+			      gpmc,flags = <4>; /* SMSC911X_USE_32BIT */
+	      };
+};
diff --git a/arch/arm/mach-omap2/gpmc-smsc911x.c b/arch/arm/mach-omap2/gpmc-smsc911x.c
index 5ce00ad2..59a2ee4 100644
--- a/arch/arm/mach-omap2/gpmc-smsc911x.c
+++ b/arch/arm/mach-omap2/gpmc-smsc911x.c
@@ -19,6 +19,7 @@
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/smsc911x.h>
+#include <linux/of.h>
 
 #include "gpmc.h"
 #include "gpmc-smsc911x.h"
@@ -98,3 +99,31 @@ free1:
 
 	pr_err("Could not initialize smsc911x device\n");
 }
+
+int gpmc_smsc911x_init_dt(struct device_node *node)
+{
+	struct omap_smsc911x_platform_data gpmc_cfg;
+
+	if (WARN_ON(!node))
+		return -ENODEV;
+
+	if (of_property_read_u32(node, "gpmc,cs", &gpmc_cfg.cs)) {
+		pr_err("Unable to get GPMC smsc911x chip select\n");
+		return -EINVAL;
+	}
+
+	if (of_property_read_u32(node, "gpmc,gpio_irq", &gpmc_cfg.gpio_irq)) {
+		pr_err("Unable to get GPMC smsc911x GPIO IRQ\n");
+		return -EINVAL;
+	}
+
+	if (of_property_read_u32(node, "gpmc,gpio_reset", &gpmc_cfg.gpio_reset))
+		gpmc_cfg.gpio_reset = -EINVAL;
+
+	if (of_property_read_u32(node, "gpmc,flags", &gpmc_cfg.flags))
+		gpmc_cfg.flags = 0;
+
+	gpmc_smsc911x_init(&gpmc_cfg);
+
+	return 0;
+}
diff --git a/arch/arm/mach-omap2/gpmc-smsc911x.h b/arch/arm/mach-omap2/gpmc-smsc911x.h
index ea6c9c8..bbcb8bc 100644
--- a/arch/arm/mach-omap2/gpmc-smsc911x.h
+++ b/arch/arm/mach-omap2/gpmc-smsc911x.h
@@ -25,11 +25,17 @@ struct omap_smsc911x_platform_data {
 
 extern void gpmc_smsc911x_init(struct omap_smsc911x_platform_data *d);
 
+extern int gpmc_smsc911x_init_dt(struct device_node *node);
+
 #else
 
 static inline void gpmc_smsc911x_init(struct omap_smsc911x_platform_data *d)
 {
 }
 
+static inline int gpmc_smsc911x_init_dt(struct device_node *node)
+{
+}
+
 #endif
 #endif
-- 
1.7.7.6

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH RFC 4/7] ARM: OMAP: gpmc-smsc911x: pass a dev node to platform registration
  2013-02-09 20:44 ` Javier Martinez Canillas
@ 2013-02-09 20:44   ` Javier Martinez Canillas
  -1 siblings, 0 replies; 46+ messages in thread
From: Javier Martinez Canillas @ 2013-02-09 20:44 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Benoît Cousson, Russell King, Linus Walleij,
	Greg Kroah-Hartman, Enric Balletbo i Serra, Ezequiel Garcia,
	devicetree-discuss, linux-omap, linux-arm-kernel,
	Javier Martinez Canillas

The smsc911x driver needs the GPMC smsc911x associated device
node to set the OMAP mux pins using the pinctrl framework.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 arch/arm/mach-omap2/gpmc-smsc911x.c |    5 ++++-
 arch/arm/mach-omap2/gpmc-smsc911x.h |    1 +
 2 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc-smsc911x.c b/arch/arm/mach-omap2/gpmc-smsc911x.c
index 59a2ee4..9f3b0a5 100644
--- a/arch/arm/mach-omap2/gpmc-smsc911x.c
+++ b/arch/arm/mach-omap2/gpmc-smsc911x.c
@@ -83,7 +83,8 @@ void __init gpmc_smsc911x_init(struct omap_smsc911x_platform_data *gpmc_cfg)
 
 	pdev = platform_device_register_resndata(NULL, "smsc911x", gpmc_cfg->id,
 		 gpmc_smsc911x_resources, ARRAY_SIZE(gpmc_smsc911x_resources),
-		 &gpmc_smsc911x_config, sizeof(gpmc_smsc911x_config), NULL);
+		 &gpmc_smsc911x_config, sizeof(gpmc_smsc911x_config),
+		 gpmc_cfg->of_node);
 	if (!pdev) {
 		pr_err("Unable to register platform device\n");
 		gpio_free(gpmc_cfg->gpio_reset);
@@ -107,6 +108,8 @@ int gpmc_smsc911x_init_dt(struct device_node *node)
 	if (WARN_ON(!node))
 		return -ENODEV;
 
+	gpmc_cfg.of_node = node;
+
 	if (of_property_read_u32(node, "gpmc,cs", &gpmc_cfg.cs)) {
 		pr_err("Unable to get GPMC smsc911x chip select\n");
 		return -EINVAL;
diff --git a/arch/arm/mach-omap2/gpmc-smsc911x.h b/arch/arm/mach-omap2/gpmc-smsc911x.h
index bbcb8bc..32a7df0 100644
--- a/arch/arm/mach-omap2/gpmc-smsc911x.h
+++ b/arch/arm/mach-omap2/gpmc-smsc911x.h
@@ -19,6 +19,7 @@ struct omap_smsc911x_platform_data {
 	int	gpio_irq;
 	int	gpio_reset;
 	u32	flags;
+	struct device_node *of_node;
 };
 
 #if defined(CONFIG_SMSC911X) || defined(CONFIG_SMSC911X_MODULE)
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH RFC 4/7] ARM: OMAP: gpmc-smsc911x: pass a dev node to platform registration
@ 2013-02-09 20:44   ` Javier Martinez Canillas
  0 siblings, 0 replies; 46+ messages in thread
From: Javier Martinez Canillas @ 2013-02-09 20:44 UTC (permalink / raw)
  To: linux-arm-kernel

The smsc911x driver needs the GPMC smsc911x associated device
node to set the OMAP mux pins using the pinctrl framework.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 arch/arm/mach-omap2/gpmc-smsc911x.c |    5 ++++-
 arch/arm/mach-omap2/gpmc-smsc911x.h |    1 +
 2 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc-smsc911x.c b/arch/arm/mach-omap2/gpmc-smsc911x.c
index 59a2ee4..9f3b0a5 100644
--- a/arch/arm/mach-omap2/gpmc-smsc911x.c
+++ b/arch/arm/mach-omap2/gpmc-smsc911x.c
@@ -83,7 +83,8 @@ void __init gpmc_smsc911x_init(struct omap_smsc911x_platform_data *gpmc_cfg)
 
 	pdev = platform_device_register_resndata(NULL, "smsc911x", gpmc_cfg->id,
 		 gpmc_smsc911x_resources, ARRAY_SIZE(gpmc_smsc911x_resources),
-		 &gpmc_smsc911x_config, sizeof(gpmc_smsc911x_config), NULL);
+		 &gpmc_smsc911x_config, sizeof(gpmc_smsc911x_config),
+		 gpmc_cfg->of_node);
 	if (!pdev) {
 		pr_err("Unable to register platform device\n");
 		gpio_free(gpmc_cfg->gpio_reset);
@@ -107,6 +108,8 @@ int gpmc_smsc911x_init_dt(struct device_node *node)
 	if (WARN_ON(!node))
 		return -ENODEV;
 
+	gpmc_cfg.of_node = node;
+
 	if (of_property_read_u32(node, "gpmc,cs", &gpmc_cfg.cs)) {
 		pr_err("Unable to get GPMC smsc911x chip select\n");
 		return -EINVAL;
diff --git a/arch/arm/mach-omap2/gpmc-smsc911x.h b/arch/arm/mach-omap2/gpmc-smsc911x.h
index bbcb8bc..32a7df0 100644
--- a/arch/arm/mach-omap2/gpmc-smsc911x.h
+++ b/arch/arm/mach-omap2/gpmc-smsc911x.h
@@ -19,6 +19,7 @@ struct omap_smsc911x_platform_data {
 	int	gpio_irq;
 	int	gpio_reset;
 	u32	flags;
+	struct device_node *of_node;
 };
 
 #if defined(CONFIG_SMSC911X) || defined(CONFIG_SMSC911X_MODULE)
-- 
1.7.7.6

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH RFC 5/7] ARM: OMAP: gpmc: add support for gpmc-smsc911x child nodes
  2013-02-09 20:44 ` Javier Martinez Canillas
@ 2013-02-09 20:44   ` Javier Martinez Canillas
  -1 siblings, 0 replies; 46+ messages in thread
From: Javier Martinez Canillas @ 2013-02-09 20:44 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Benoît Cousson, Russell King, Linus Walleij,
	Greg Kroah-Hartman, Enric Balletbo i Serra, Ezequiel Garcia,
	devicetree-discuss, linux-omap, linux-arm-kernel,
	Javier Martinez Canillas

Besides being used to interface with external memory devices,
the General-Purpose Memory Controller can be used to connect
Pseudo-SRAM devices to the OMAP processor that use the GPMC
as a data bus. One of these devices is the smsc911x LAN chip.

This patch allows an smsc911x LAN pheripheral to be defined
as an GPMC child node an call its corresponding setup function.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 arch/arm/mach-omap2/gpmc.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index f55b504..fd22c62 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -40,6 +40,7 @@
 #include "gpmc.h"
 #include "gpmc-nand.h"
 #include "gpmc-onenand.h"
+#include "gpmc-smsc911x.h"
 
 #define	DEVICE_NAME		"omap-gpmc"
 
@@ -1320,6 +1321,14 @@ static int gpmc_probe_dt(struct platform_device *pdev)
 			return ret;
 		}
 	}
+
+	for_each_node_by_name(child, "gpmc_smsc911x") {
+		ret = gpmc_smsc911x_init_dt(child);
+		if (ret < 0) {
+			of_node_put(child);
+			return ret;
+		}
+	}
 	return 0;
 }
 #else
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH RFC 5/7] ARM: OMAP: gpmc: add support for gpmc-smsc911x child nodes
@ 2013-02-09 20:44   ` Javier Martinez Canillas
  0 siblings, 0 replies; 46+ messages in thread
From: Javier Martinez Canillas @ 2013-02-09 20:44 UTC (permalink / raw)
  To: linux-arm-kernel

Besides being used to interface with external memory devices,
the General-Purpose Memory Controller can be used to connect
Pseudo-SRAM devices to the OMAP processor that use the GPMC
as a data bus. One of these devices is the smsc911x LAN chip.

This patch allows an smsc911x LAN pheripheral to be defined
as an GPMC child node an call its corresponding setup function.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 arch/arm/mach-omap2/gpmc.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index f55b504..fd22c62 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -40,6 +40,7 @@
 #include "gpmc.h"
 #include "gpmc-nand.h"
 #include "gpmc-onenand.h"
+#include "gpmc-smsc911x.h"
 
 #define	DEVICE_NAME		"omap-gpmc"
 
@@ -1320,6 +1321,14 @@ static int gpmc_probe_dt(struct platform_device *pdev)
 			return ret;
 		}
 	}
+
+	for_each_node_by_name(child, "gpmc_smsc911x") {
+		ret = gpmc_smsc911x_init_dt(child);
+		if (ret < 0) {
+			of_node_put(child);
+			return ret;
+		}
+	}
 	return 0;
 }
 #else
-- 
1.7.7.6

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH RFC 6/7] ARM: dts: OMAP: Add an GPMC node for OMAP3
  2013-02-09 20:44 ` Javier Martinez Canillas
@ 2013-02-09 20:44   ` Javier Martinez Canillas
  -1 siblings, 0 replies; 46+ messages in thread
From: Javier Martinez Canillas @ 2013-02-09 20:44 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Benoît Cousson, Russell King, Linus Walleij,
	Greg Kroah-Hartman, Enric Balletbo i Serra, Ezequiel Garcia,
	devicetree-discuss, linux-omap, linux-arm-kernel,
	Javier Martinez Canillas

This patch adds a General-Purpose Memory Controller (GPMC)
device node to be used for OMAP3 based SoC boards.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 arch/arm/boot/dts/omap3.dtsi |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
index 6c63118..38b52b3 100644
--- a/arch/arm/boot/dts/omap3.dtsi
+++ b/arch/arm/boot/dts/omap3.dtsi
@@ -403,5 +403,16 @@
 			ti,timer-alwon;
 			ti,timer-secure;
 		};
+
+		gpmc: gpmc@6e000000 {
+		        compatible = "ti,omap3430-gpmc";
+			ti,hwmods = "gpmc";
+			reg = <0x6e000000 0x1000000>;
+			interrupts = <20>; /* M_IRQ_20 interrupt */
+			gpmc,num-cs = <8>;
+			gpmc,num-waitpins = <4>;
+			#address-cells = <2>;
+			#size-cells = <1>;
+		};
 	};
 };
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH RFC 6/7] ARM: dts: OMAP: Add an GPMC node for OMAP3
@ 2013-02-09 20:44   ` Javier Martinez Canillas
  0 siblings, 0 replies; 46+ messages in thread
From: Javier Martinez Canillas @ 2013-02-09 20:44 UTC (permalink / raw)
  To: linux-arm-kernel

This patch adds a General-Purpose Memory Controller (GPMC)
device node to be used for OMAP3 based SoC boards.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 arch/arm/boot/dts/omap3.dtsi |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
index 6c63118..38b52b3 100644
--- a/arch/arm/boot/dts/omap3.dtsi
+++ b/arch/arm/boot/dts/omap3.dtsi
@@ -403,5 +403,16 @@
 			ti,timer-alwon;
 			ti,timer-secure;
 		};
+
+		gpmc: gpmc at 6e000000 {
+		        compatible = "ti,omap3430-gpmc";
+			ti,hwmods = "gpmc";
+			reg = <0x6e000000 0x1000000>;
+			interrupts = <20>; /* M_IRQ_20 interrupt */
+			gpmc,num-cs = <8>;
+			gpmc,num-waitpins = <4>;
+			#address-cells = <2>;
+			#size-cells = <1>;
+		};
 	};
 };
-- 
1.7.7.6

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH RFC 7/7] ARM: dts: omap3-igep0020: Add SMSC911x LAN chip support
  2013-02-09 20:44 ` Javier Martinez Canillas
@ 2013-02-09 20:44   ` Javier Martinez Canillas
  -1 siblings, 0 replies; 46+ messages in thread
From: Javier Martinez Canillas @ 2013-02-09 20:44 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Benoît Cousson, Russell King, Linus Walleij,
	Greg Kroah-Hartman, Enric Balletbo i Serra, Ezequiel Garcia,
	devicetree-discuss, linux-omap, linux-arm-kernel,
	Javier Martinez Canillas

IGEPv2 boards has an SMSC LAN9221i ethernet chip connected to
the OMAP3 processor though the General-Purpose Memory Controller.

This patch adds a device node for the ethernet chip so the GPMC
driver will call the gpmc-smsc911x setup code.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 arch/arm/boot/dts/omap3-igep.dtsi    |    6 ++++++
 arch/arm/boot/dts/omap3-igep0020.dts |   24 ++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/omap3-igep.dtsi b/arch/arm/boot/dts/omap3-igep.dtsi
index 100eb41..2f02581 100644
--- a/arch/arm/boot/dts/omap3-igep.dtsi
+++ b/arch/arm/boot/dts/omap3-igep.dtsi
@@ -48,6 +48,12 @@
 			0x126 0x0100	/* sdmmc1_dat7.sdmmc1_dat7 INPUT | MODE 0 */
 		>;
 	};
+
+        smsc911x_pins: pinmux_smsc911x_pins {
+                pinctrl-single,pins = <
+                        0x1a2 0x0104    /* mcspi1_cs2.gpio_176 INPUT | MODE4 */
+                >;
+        };
 };
 
 &i2c1 {
diff --git a/arch/arm/boot/dts/omap3-igep0020.dts b/arch/arm/boot/dts/omap3-igep0020.dts
index e2b9849..68b7cf3 100644
--- a/arch/arm/boot/dts/omap3-igep0020.dts
+++ b/arch/arm/boot/dts/omap3-igep0020.dts
@@ -40,6 +40,18 @@
 			gpios = <&twl_gpio 19 1>;
 		};
 	};
+
+        vddvario: regulator-vddvario {
+                compatible = "regulator-fixed";
+                regulator-name = "vddvario";
+                regulator-always-on;
+        };
+
+        vdd33a: regulator-vdd33a {
+                compatible = "regulator-fixed";
+                regulator-name = "vdd33a";
+                regulator-always-on;
+        };
 };
 
 &i2c3 {
@@ -54,3 +66,15 @@
 		reg = <0x50>;
 	};
 };
+
+&gpmc {
+	gpmc_smsc911x@0 {
+		pinctrl-names = "default";
+		pinctrl-0 = <&smsc911x_pins>;
+		gpmc,cs = <5>; /* IGEP2_SMSC911X_CS */
+		gpmc,gpio_irq = <176>; /* IGEP2_SMSC911X_GPIO */
+		gpmc,flags = <18>; /* SMSC911X_USE_32BIT | SMSC911X_SAVE_MAC_ADDRESS */
+		vmmc-supply = <&vddvario>;
+		vmmc_aux-supply = <&vdd33a>;
+      };
+};
-- 
1.7.7.6


^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [PATCH RFC 7/7] ARM: dts: omap3-igep0020: Add SMSC911x LAN chip support
@ 2013-02-09 20:44   ` Javier Martinez Canillas
  0 siblings, 0 replies; 46+ messages in thread
From: Javier Martinez Canillas @ 2013-02-09 20:44 UTC (permalink / raw)
  To: linux-arm-kernel

IGEPv2 boards has an SMSC LAN9221i ethernet chip connected to
the OMAP3 processor though the General-Purpose Memory Controller.

This patch adds a device node for the ethernet chip so the GPMC
driver will call the gpmc-smsc911x setup code.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 arch/arm/boot/dts/omap3-igep.dtsi    |    6 ++++++
 arch/arm/boot/dts/omap3-igep0020.dts |   24 ++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/omap3-igep.dtsi b/arch/arm/boot/dts/omap3-igep.dtsi
index 100eb41..2f02581 100644
--- a/arch/arm/boot/dts/omap3-igep.dtsi
+++ b/arch/arm/boot/dts/omap3-igep.dtsi
@@ -48,6 +48,12 @@
 			0x126 0x0100	/* sdmmc1_dat7.sdmmc1_dat7 INPUT | MODE 0 */
 		>;
 	};
+
+        smsc911x_pins: pinmux_smsc911x_pins {
+                pinctrl-single,pins = <
+                        0x1a2 0x0104    /* mcspi1_cs2.gpio_176 INPUT | MODE4 */
+                >;
+        };
 };
 
 &i2c1 {
diff --git a/arch/arm/boot/dts/omap3-igep0020.dts b/arch/arm/boot/dts/omap3-igep0020.dts
index e2b9849..68b7cf3 100644
--- a/arch/arm/boot/dts/omap3-igep0020.dts
+++ b/arch/arm/boot/dts/omap3-igep0020.dts
@@ -40,6 +40,18 @@
 			gpios = <&twl_gpio 19 1>;
 		};
 	};
+
+        vddvario: regulator-vddvario {
+                compatible = "regulator-fixed";
+                regulator-name = "vddvario";
+                regulator-always-on;
+        };
+
+        vdd33a: regulator-vdd33a {
+                compatible = "regulator-fixed";
+                regulator-name = "vdd33a";
+                regulator-always-on;
+        };
 };
 
 &i2c3 {
@@ -54,3 +66,15 @@
 		reg = <0x50>;
 	};
 };
+
+&gpmc {
+	gpmc_smsc911x at 0 {
+		pinctrl-names = "default";
+		pinctrl-0 = <&smsc911x_pins>;
+		gpmc,cs = <5>; /* IGEP2_SMSC911X_CS */
+		gpmc,gpio_irq = <176>; /* IGEP2_SMSC911X_GPIO */
+		gpmc,flags = <18>; /* SMSC911X_USE_32BIT | SMSC911X_SAVE_MAC_ADDRESS */
+		vmmc-supply = <&vddvario>;
+		vmmc_aux-supply = <&vdd33a>;
+      };
+};
-- 
1.7.7.6

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* Re: [PATCH RFC 1/7] platform: add a device node
  2013-02-09 20:44   ` Javier Martinez Canillas
@ 2013-02-10  1:02     ` Greg Kroah-Hartman
  -1 siblings, 0 replies; 46+ messages in thread
From: Greg Kroah-Hartman @ 2013-02-10  1:02 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: Tony Lindgren, Benoît Cousson, Russell King, Linus Walleij,
	Enric Balletbo i Serra, Ezequiel Garcia, devicetree-discuss,
	linux-omap, linux-arm-kernel

On Sat, Feb 09, 2013 at 09:44:25PM +0100, Javier Martinez Canillas wrote:
> When using Device Trees, it is necessary to associate a
> device node with a platform device.
> 
> Usually this device node has to used in the device probe
> function (e.g: to initizalize the pinctrl pads assocaited
> with the device).
> 
> So, platform code needs to pass a device node as a platform
> device info to the platform device registration function.

Really?  We don't already have enough other pointers in the platform
device structure that we need another one?

I don't buy it, sorry.  Any reason you didn't run this by Grant as well?

greg k-h

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH RFC 1/7] platform: add a device node
@ 2013-02-10  1:02     ` Greg Kroah-Hartman
  0 siblings, 0 replies; 46+ messages in thread
From: Greg Kroah-Hartman @ 2013-02-10  1:02 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Feb 09, 2013 at 09:44:25PM +0100, Javier Martinez Canillas wrote:
> When using Device Trees, it is necessary to associate a
> device node with a platform device.
> 
> Usually this device node has to used in the device probe
> function (e.g: to initizalize the pinctrl pads assocaited
> with the device).
> 
> So, platform code needs to pass a device node as a platform
> device info to the platform device registration function.

Really?  We don't already have enough other pointers in the platform
device structure that we need another one?

I don't buy it, sorry.  Any reason you didn't run this by Grant as well?

greg k-h

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH RFC 1/7] platform: add a device node
  2013-02-10  1:02     ` Greg Kroah-Hartman
@ 2013-02-10  1:49       ` Javier Martinez Canillas
  -1 siblings, 0 replies; 46+ messages in thread
From: Javier Martinez Canillas @ 2013-02-10  1:49 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Tony Lindgren, Benoît Cousson, Russell King, Linus Walleij,
	Enric Balletbo i Serra, Ezequiel Garcia, devicetree-discuss,
	linux-omap, linux-arm-kernel, Grant Likely

Hi Greg,

Thanks a lot for your feedback.

On 02/10/2013 02:02 AM, Greg Kroah-Hartman wrote:
> On Sat, Feb 09, 2013 at 09:44:25PM +0100, Javier Martinez Canillas wrote:
>> When using Device Trees, it is necessary to associate a
>> device node with a platform device.
>> 
>> Usually this device node has to used in the device probe
>> function (e.g: to initizalize the pinctrl pads assocaited
>> with the device).
>> 
>> So, platform code needs to pass a device node as a platform
>> device info to the platform device registration function.
> 
> Really?  We don't already have enough other pointers in the platform
> device structure that we need another one?
>

I knew this would be controversial and that's why I didn't mean it to be a patch
but a RFC :)

The problem basically is that you have to associate the platform device with its
corresponding DT device node because it can be used in the driver probe function.

But you can't do this before calling platform_device_register_resndata() since
the platform_device has not been allocated yet and you can't do it after since
by then the driver probe function has been executed already.

You could certainly pass the device node as a platform device resource by for
example storing the dev node pointer in the struct resource .start member or by
adding it to the structure (struct smsc911x_platform_config in this case) that
gets copied to the struct platform_device_info void *data.

Then in the driver probe function you could get the device node from either the
platform_data or a dev->resource.

But I expect most users of platform_device_register_resndata() will have a
similar issue as more and more platform drivers are migrated to DT.

So, to avoid each driver to do the same (encode and decode the DT device node
using platform_data or a device resource) I thought that it could make sense to
add yet another pointer to the struct platform_device_info structure and set:

pdev->dev.of_node = pdevinfo->of_node

inside platform_device_register_full().

> I don't buy it, sorry.  Any reason you didn't run this by Grant as well?
> 

No reason at all, it is just me being goofy. I thought that Grant was cc'ed
already and I just realized that he wasn't. cc'ing him as well.

> greg k-h
> 

Thanks a lot and best regards,
Javier

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH RFC 1/7] platform: add a device node
@ 2013-02-10  1:49       ` Javier Martinez Canillas
  0 siblings, 0 replies; 46+ messages in thread
From: Javier Martinez Canillas @ 2013-02-10  1:49 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Greg,

Thanks a lot for your feedback.

On 02/10/2013 02:02 AM, Greg Kroah-Hartman wrote:
> On Sat, Feb 09, 2013 at 09:44:25PM +0100, Javier Martinez Canillas wrote:
>> When using Device Trees, it is necessary to associate a
>> device node with a platform device.
>> 
>> Usually this device node has to used in the device probe
>> function (e.g: to initizalize the pinctrl pads assocaited
>> with the device).
>> 
>> So, platform code needs to pass a device node as a platform
>> device info to the platform device registration function.
> 
> Really?  We don't already have enough other pointers in the platform
> device structure that we need another one?
>

I knew this would be controversial and that's why I didn't mean it to be a patch
but a RFC :)

The problem basically is that you have to associate the platform device with its
corresponding DT device node because it can be used in the driver probe function.

But you can't do this before calling platform_device_register_resndata() since
the platform_device has not been allocated yet and you can't do it after since
by then the driver probe function has been executed already.

You could certainly pass the device node as a platform device resource by for
example storing the dev node pointer in the struct resource .start member or by
adding it to the structure (struct smsc911x_platform_config in this case) that
gets copied to the struct platform_device_info void *data.

Then in the driver probe function you could get the device node from either the
platform_data or a dev->resource.

But I expect most users of platform_device_register_resndata() will have a
similar issue as more and more platform drivers are migrated to DT.

So, to avoid each driver to do the same (encode and decode the DT device node
using platform_data or a device resource) I thought that it could make sense to
add yet another pointer to the struct platform_device_info structure and set:

pdev->dev.of_node = pdevinfo->of_node

inside platform_device_register_full().

> I don't buy it, sorry.  Any reason you didn't run this by Grant as well?
> 

No reason at all, it is just me being goofy. I thought that Grant was cc'ed
already and I just realized that he wasn't. cc'ing him as well.

> greg k-h
> 

Thanks a lot and best regards,
Javier

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH RFC 1/7] platform: add a device node
  2013-02-10  1:49       ` Javier Martinez Canillas
@ 2013-02-10  9:37         ` Russell King - ARM Linux
  -1 siblings, 0 replies; 46+ messages in thread
From: Russell King - ARM Linux @ 2013-02-10  9:37 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: Greg Kroah-Hartman, Tony Lindgren, Benoît Cousson,
	Linus Walleij, Enric Balletbo i Serra, Ezequiel Garcia,
	devicetree-discuss, linux-omap, linux-arm-kernel, Grant Likely

On Sun, Feb 10, 2013 at 02:49:21AM +0100, Javier Martinez Canillas wrote:
> I knew this would be controversial and that's why I didn't mean it to be a patch
> but a RFC :)
> 
> The problem basically is that you have to associate the platform device with its
> corresponding DT device node because it can be used in the driver probe function.

When DT is being used, doesn't DT create the platform devices for you,
with the device node already set correctly?

Manually creating platform devices and adding DT nodes to it sounds like
the wrong thing to be doing.

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH RFC 1/7] platform: add a device node
@ 2013-02-10  9:37         ` Russell King - ARM Linux
  0 siblings, 0 replies; 46+ messages in thread
From: Russell King - ARM Linux @ 2013-02-10  9:37 UTC (permalink / raw)
  To: linux-arm-kernel

On Sun, Feb 10, 2013 at 02:49:21AM +0100, Javier Martinez Canillas wrote:
> I knew this would be controversial and that's why I didn't mean it to be a patch
> but a RFC :)
> 
> The problem basically is that you have to associate the platform device with its
> corresponding DT device node because it can be used in the driver probe function.

When DT is being used, doesn't DT create the platform devices for you,
with the device node already set correctly?

Manually creating platform devices and adding DT nodes to it sounds like
the wrong thing to be doing.

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH RFC 1/7] platform: add a device node
  2013-02-10  9:37         ` Russell King - ARM Linux
@ 2013-02-10 11:35           ` Javier Martinez Canillas
  -1 siblings, 0 replies; 46+ messages in thread
From: Javier Martinez Canillas @ 2013-02-10 11:35 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Javier Martinez Canillas, Greg Kroah-Hartman, Tony Lindgren,
	Benoît Cousson, Linus Walleij, Enric Balletbo i Serra,
	Ezequiel Garcia, devicetree-discuss, linux-omap,
	linux-arm-kernel, Grant Likely

On Sun, Feb 10, 2013 at 10:37 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Sun, Feb 10, 2013 at 02:49:21AM +0100, Javier Martinez Canillas wrote:
>> I knew this would be controversial and that's why I didn't mean it to be a patch
>> but a RFC :)
>>
>> The problem basically is that you have to associate the platform device with its
>> corresponding DT device node because it can be used in the driver probe function.
>
> When DT is being used, doesn't DT create the platform devices for you,
> with the device node already set correctly?
>

Well they usually do but not always just like usually you have a
platform_device in your board code and call platform_device_register().

But in some configurations you can't just define the platform_device
required resources (I/O memory, IRQ, etc) as static platform data.
In some cases you need a level of indirection.

In this particular case a SMSC ethernet chip is connected to an
OMAP3 processor through its General-Purpose Memory Controller.

You can't just define the I/O memory used by the device since you first
need to request that address to the GPMC. The same happens with the
IRQ line since a OMAP GPIO pin is used so you have to first configure
the GPIO line as input.

So in platform code you call to gpmc_smsc911x_init() passing all the
GPMC specific configurations (GPIO used for IRQ, GPMC chip select, etc)

Then gpmc_smsc911x_init() does all the needed setup, fills a platform_data
for the real platform_device and calls  platform_device_register_resndata().

>From the smsc911x platform_driver probe function point of view it just have
resources and doesn't know if it's I/O memory is directly mapped or is
through a memory controller as the GPMC. It also doesn't know if the IRQ is
a GPIO or not.

Another approach could be extending the current SMSC LAN DT binding with
GPMC-specific properties so the platform driver could do this setup when the
device node is found and the probe function is called.

> Manually creating platform devices and adding DT nodes to it sounds like
> the wrong thing to be doing.
> --

Yes, I don't like this approach to much either but I didn't find a better way to
do it. That was the main point of this RFC.

I'm still learning about Device Trees so I hope that someone with more
experience with DT or more familiarized with OMAP GPMC and the SMSC
driver could take a step forward and help me giving a better idea.

Thanks a lot for your feedback and best regards,
Javier

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH RFC 1/7] platform: add a device node
@ 2013-02-10 11:35           ` Javier Martinez Canillas
  0 siblings, 0 replies; 46+ messages in thread
From: Javier Martinez Canillas @ 2013-02-10 11:35 UTC (permalink / raw)
  To: linux-arm-kernel

On Sun, Feb 10, 2013 at 10:37 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Sun, Feb 10, 2013 at 02:49:21AM +0100, Javier Martinez Canillas wrote:
>> I knew this would be controversial and that's why I didn't mean it to be a patch
>> but a RFC :)
>>
>> The problem basically is that you have to associate the platform device with its
>> corresponding DT device node because it can be used in the driver probe function.
>
> When DT is being used, doesn't DT create the platform devices for you,
> with the device node already set correctly?
>

Well they usually do but not always just like usually you have a
platform_device in your board code and call platform_device_register().

But in some configurations you can't just define the platform_device
required resources (I/O memory, IRQ, etc) as static platform data.
In some cases you need a level of indirection.

In this particular case a SMSC ethernet chip is connected to an
OMAP3 processor through its General-Purpose Memory Controller.

You can't just define the I/O memory used by the device since you first
need to request that address to the GPMC. The same happens with the
IRQ line since a OMAP GPIO pin is used so you have to first configure
the GPIO line as input.

So in platform code you call to gpmc_smsc911x_init() passing all the
GPMC specific configurations (GPIO used for IRQ, GPMC chip select, etc)

Then gpmc_smsc911x_init() does all the needed setup, fills a platform_data
for the real platform_device and calls  platform_device_register_resndata().

>From the smsc911x platform_driver probe function point of view it just have
resources and doesn't know if it's I/O memory is directly mapped or is
through a memory controller as the GPMC. It also doesn't know if the IRQ is
a GPIO or not.

Another approach could be extending the current SMSC LAN DT binding with
GPMC-specific properties so the platform driver could do this setup when the
device node is found and the probe function is called.

> Manually creating platform devices and adding DT nodes to it sounds like
> the wrong thing to be doing.
> --

Yes, I don't like this approach to much either but I didn't find a better way to
do it. That was the main point of this RFC.

I'm still learning about Device Trees so I hope that someone with more
experience with DT or more familiarized with OMAP GPMC and the SMSC
driver could take a step forward and help me giving a better idea.

Thanks a lot for your feedback and best regards,
Javier

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH RFC 1/7] platform: add a device node
  2013-02-10 11:35           ` Javier Martinez Canillas
@ 2013-02-11  8:16             ` Sascha Hauer
  -1 siblings, 0 replies; 46+ messages in thread
From: Sascha Hauer @ 2013-02-11  8:16 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: Russell King - ARM Linux, Greg Kroah-Hartman, devicetree-discuss,
	Enric Balletbo i Serra, linux-omap, Javier Martinez Canillas,
	linux-arm-kernel

On Sun, Feb 10, 2013 at 12:35:43PM +0100, Javier Martinez Canillas wrote:
> On Sun, Feb 10, 2013 at 10:37 AM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Sun, Feb 10, 2013 at 02:49:21AM +0100, Javier Martinez Canillas wrote:
> >> I knew this would be controversial and that's why I didn't mean it to be a patch
> >> but a RFC :)
> >>
> >> The problem basically is that you have to associate the platform device with its
> >> corresponding DT device node because it can be used in the driver probe function.
> >
> > When DT is being used, doesn't DT create the platform devices for you,
> > with the device node already set correctly?
> >
> 
> Well they usually do but not always just like usually you have a
> platform_device in your board code and call platform_device_register().
> 
> But in some configurations you can't just define the platform_device
> required resources (I/O memory, IRQ, etc) as static platform data.
> In some cases you need a level of indirection.
> 
> In this particular case a SMSC ethernet chip is connected to an
> OMAP3 processor through its General-Purpose Memory Controller.
> 
> You can't just define the I/O memory used by the device since you first
> need to request that address to the GPMC. The same happens with the
> IRQ line since a OMAP GPIO pin is used so you have to first configure
> the GPIO line as input.

For the gpio interrupt you can use, example taken from omap4-var-som.dts:

	interrupt-parent = <&gpio6>;
	interrupts = <11>; /* gpio line 171 */

Other architectures allow to specify the edge/level hi/low active
parameters from the devicetree aswell. The gpio direction should be
handled by the gpio driver as necessary, at least that's what done on
other architectures.

If the SMSC hangs on the GPMC then the SMSC should be a child node of
the GPMC. The GPMC would then act as a bus driver and configure the
chipselects and timings for its children automatically, maybe based
on timing information from the devicetree. I've never tried this before,
but I think that's the way things should be.

No need to manually create platform devices from the devicetree.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH RFC 1/7] platform: add a device node
@ 2013-02-11  8:16             ` Sascha Hauer
  0 siblings, 0 replies; 46+ messages in thread
From: Sascha Hauer @ 2013-02-11  8:16 UTC (permalink / raw)
  To: linux-arm-kernel

On Sun, Feb 10, 2013 at 12:35:43PM +0100, Javier Martinez Canillas wrote:
> On Sun, Feb 10, 2013 at 10:37 AM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Sun, Feb 10, 2013 at 02:49:21AM +0100, Javier Martinez Canillas wrote:
> >> I knew this would be controversial and that's why I didn't mean it to be a patch
> >> but a RFC :)
> >>
> >> The problem basically is that you have to associate the platform device with its
> >> corresponding DT device node because it can be used in the driver probe function.
> >
> > When DT is being used, doesn't DT create the platform devices for you,
> > with the device node already set correctly?
> >
> 
> Well they usually do but not always just like usually you have a
> platform_device in your board code and call platform_device_register().
> 
> But in some configurations you can't just define the platform_device
> required resources (I/O memory, IRQ, etc) as static platform data.
> In some cases you need a level of indirection.
> 
> In this particular case a SMSC ethernet chip is connected to an
> OMAP3 processor through its General-Purpose Memory Controller.
> 
> You can't just define the I/O memory used by the device since you first
> need to request that address to the GPMC. The same happens with the
> IRQ line since a OMAP GPIO pin is used so you have to first configure
> the GPIO line as input.

For the gpio interrupt you can use, example taken from omap4-var-som.dts:

	interrupt-parent = <&gpio6>;
	interrupts = <11>; /* gpio line 171 */

Other architectures allow to specify the edge/level hi/low active
parameters from the devicetree aswell. The gpio direction should be
handled by the gpio driver as necessary, at least that's what done on
other architectures.

If the SMSC hangs on the GPMC then the SMSC should be a child node of
the GPMC. The GPMC would then act as a bus driver and configure the
chipselects and timings for its children automatically, maybe based
on timing information from the devicetree. I've never tried this before,
but I think that's the way things should be.

No need to manually create platform devices from the devicetree.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH RFC 3/7] ARM: OMAP: gpmc-smsc911x: add DT dev node init function
  2013-02-09 20:44   ` Javier Martinez Canillas
@ 2013-02-11 10:30     ` Mark Rutland
  -1 siblings, 0 replies; 46+ messages in thread
From: Mark Rutland @ 2013-02-11 10:30 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: Tony Lindgren, Russell King, Benoît Cousson,
	Enric Balletbo i Serra, Greg Kroah-Hartman, Linus Walleij,
	Ezequiel Garcia, linux-omap, devicetree-discuss,
	linux-arm-kernel

Hi,

I have a few comments on the binding and the way it's parsed.

On Sat, Feb 09, 2013 at 08:44:27PM +0000, Javier Martinez Canillas wrote:
> This patch adds a helper function to parse a device node that
> contains all the properties needed to initialize an smsc911x
> device connected to an OMAP processor through OMAP's GPMC.
> 
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> ---
>  .../devicetree/bindings/net/gpmc-smsc911x.txt      |   39 ++++++++++++++++++++
>  arch/arm/mach-omap2/gpmc-smsc911x.c                |   29 +++++++++++++++
>  arch/arm/mach-omap2/gpmc-smsc911x.h                |    6 +++
>  3 files changed, 74 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/net/gpmc-smsc911x.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/gpmc-smsc911x.txt b/Documentation/devicetree/bindings/net/gpmc-smsc911x.txt
> new file mode 100644
> index 0000000..8bb0df2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/gpmc-smsc911x.txt
> @@ -0,0 +1,39 @@
> +* GPMC connected Smart Mixed-Signal Connectivity (SMSC) LAN911x/912x Controller
> +
> +GPMC connected SMSC LAN911x/912x Controllers are represented as child nodes
> +of the OMAP General-Purpose Memory Controller with a name of "gpmc_smsc911x".
> +
> +All timing relevant properties as well as generic gpmc child properties are
> +explained in a separate documents - please refer to
> +Documentation/devicetree/bindings/bus/ti-gpmc.txt
> +
> +Required properties:
> +- gpmc,cs : The chip select line the pheripheral is connected to
> +- gpmc,gpio_irq : The GPIO pin that is connected to the SMSC LAN IRQ line

In devicetree land, we use '-' in preference of '_' in property names.

So this should be "gpio-irq"

> +
> +Optional properties:
> +- gpmc,flags : SMSC LAN flags - please refer to include/linux/smsc911x.h

Please don't do this. It should only be necessary to read binding documents and
hardware datasheets to write a dts. Referring to Linux internals creates a
flaky ABI that's only going to break when things get renamed/moved/modified.

The flags variable used internally by the driver don't have to have a 1-1
correspondence with a single property in the binding. You can have boolean
properties (named, but without a value) in the dt specifying each flag
individually. That way the dts is easier to read, is less tied to linux
internals (and every property is *fully* documented), and it's far easier to
add new flags in future if necessary.

> +- gpmc,gpio_reset : The GPIO pin connected to the SMSC LAN reset line

Preferably: "gpio-reset".

> +
> +Note: Besides these properties, the gpmc_smsc911x device node could need
> +aditional setup such as pin/pad mux settings and voltage regulators. This
> +depend on how the pheripheral is wired and his board specific.
> +
> +Example (for an OMAP3 board):
> +
> +gpmc@6e000000 {
> +	      compatible = "ti,omap3430-gpmc";
> +	      ti,hwmods = "gpmc";
> +	      reg = <0x6e000000 0x1000000>;
> +	      interrupts = <20>;
> +	      gpmc,num-cs = <8>;
> +	      gpmc,num-waitpins = <4>;
> +	      #address-cells = <2>;
> +	      #size-cells = <1>;
> +
> +	      gpmc_smsc911x@0 {
> +			      gpmc,cs = <5>;
> +			      gpmc,gpio_irq = <176>;
> +			      gpmc,flags = <4>; /* SMSC911X_USE_32BIT */

Here you could have a boolean property "gpmc,use-32bit" (or possibly
"gpmc,use-bits", which can either be 32 or 16).

> +	      };
> +};
> diff --git a/arch/arm/mach-omap2/gpmc-smsc911x.c b/arch/arm/mach-omap2/gpmc-smsc911x.c
> index 5ce00ad2..59a2ee4 100644
> --- a/arch/arm/mach-omap2/gpmc-smsc911x.c
> +++ b/arch/arm/mach-omap2/gpmc-smsc911x.c
> @@ -19,6 +19,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
>  #include <linux/smsc911x.h>
> +#include <linux/of.h>
>  
>  #include "gpmc.h"
>  #include "gpmc-smsc911x.h"
> @@ -98,3 +99,31 @@ free1:
>  
>  	pr_err("Could not initialize smsc911x device\n");
>  }
> +
> +int gpmc_smsc911x_init_dt(struct device_node *node)
> +{
> +	struct omap_smsc911x_platform_data gpmc_cfg;
> +
> +	if (WARN_ON(!node))
> +		return -ENODEV;
> +
> +	if (of_property_read_u32(node, "gpmc,cs", &gpmc_cfg.cs)) {
> +		pr_err("Unable to get GPMC smsc911x chip select\n");
> +		return -EINVAL;
> +	}
> +
> +	if (of_property_read_u32(node, "gpmc,gpio_irq", &gpmc_cfg.gpio_irq)) {
> +		pr_err("Unable to get GPMC smsc911x GPIO IRQ\n");
> +		return -EINVAL;
> +	}
> +
> +	if (of_property_read_u32(node, "gpmc,gpio_reset", &gpmc_cfg.gpio_reset))
> +		gpmc_cfg.gpio_reset = -EINVAL;
> +
> +	if (of_property_read_u32(node, "gpmc,flags", &gpmc_cfg.flags))
> +		gpmc_cfg.flags = 0;

Is no sanity checking required for any of the above properties?

To handle separate flags here, you can use something like:

if (of_property_read_bool(node, "gpmc,use-32bit")
	flags |= SMSC911X_USE_32BIT;

[...]

Thanks,
Mark.


^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH RFC 3/7] ARM: OMAP: gpmc-smsc911x: add DT dev node init function
@ 2013-02-11 10:30     ` Mark Rutland
  0 siblings, 0 replies; 46+ messages in thread
From: Mark Rutland @ 2013-02-11 10:30 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

I have a few comments on the binding and the way it's parsed.

On Sat, Feb 09, 2013 at 08:44:27PM +0000, Javier Martinez Canillas wrote:
> This patch adds a helper function to parse a device node that
> contains all the properties needed to initialize an smsc911x
> device connected to an OMAP processor through OMAP's GPMC.
> 
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> ---
>  .../devicetree/bindings/net/gpmc-smsc911x.txt      |   39 ++++++++++++++++++++
>  arch/arm/mach-omap2/gpmc-smsc911x.c                |   29 +++++++++++++++
>  arch/arm/mach-omap2/gpmc-smsc911x.h                |    6 +++
>  3 files changed, 74 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/net/gpmc-smsc911x.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/gpmc-smsc911x.txt b/Documentation/devicetree/bindings/net/gpmc-smsc911x.txt
> new file mode 100644
> index 0000000..8bb0df2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/gpmc-smsc911x.txt
> @@ -0,0 +1,39 @@
> +* GPMC connected Smart Mixed-Signal Connectivity (SMSC) LAN911x/912x Controller
> +
> +GPMC connected SMSC LAN911x/912x Controllers are represented as child nodes
> +of the OMAP General-Purpose Memory Controller with a name of "gpmc_smsc911x".
> +
> +All timing relevant properties as well as generic gpmc child properties are
> +explained in a separate documents - please refer to
> +Documentation/devicetree/bindings/bus/ti-gpmc.txt
> +
> +Required properties:
> +- gpmc,cs : The chip select line the pheripheral is connected to
> +- gpmc,gpio_irq : The GPIO pin that is connected to the SMSC LAN IRQ line

In devicetree land, we use '-' in preference of '_' in property names.

So this should be "gpio-irq"

> +
> +Optional properties:
> +- gpmc,flags : SMSC LAN flags - please refer to include/linux/smsc911x.h

Please don't do this. It should only be necessary to read binding documents and
hardware datasheets to write a dts. Referring to Linux internals creates a
flaky ABI that's only going to break when things get renamed/moved/modified.

The flags variable used internally by the driver don't have to have a 1-1
correspondence with a single property in the binding. You can have boolean
properties (named, but without a value) in the dt specifying each flag
individually. That way the dts is easier to read, is less tied to linux
internals (and every property is *fully* documented), and it's far easier to
add new flags in future if necessary.

> +- gpmc,gpio_reset : The GPIO pin connected to the SMSC LAN reset line

Preferably: "gpio-reset".

> +
> +Note: Besides these properties, the gpmc_smsc911x device node could need
> +aditional setup such as pin/pad mux settings and voltage regulators. This
> +depend on how the pheripheral is wired and his board specific.
> +
> +Example (for an OMAP3 board):
> +
> +gpmc at 6e000000 {
> +	      compatible = "ti,omap3430-gpmc";
> +	      ti,hwmods = "gpmc";
> +	      reg = <0x6e000000 0x1000000>;
> +	      interrupts = <20>;
> +	      gpmc,num-cs = <8>;
> +	      gpmc,num-waitpins = <4>;
> +	      #address-cells = <2>;
> +	      #size-cells = <1>;
> +
> +	      gpmc_smsc911x at 0 {
> +			      gpmc,cs = <5>;
> +			      gpmc,gpio_irq = <176>;
> +			      gpmc,flags = <4>; /* SMSC911X_USE_32BIT */

Here you could have a boolean property "gpmc,use-32bit" (or possibly
"gpmc,use-bits", which can either be 32 or 16).

> +	      };
> +};
> diff --git a/arch/arm/mach-omap2/gpmc-smsc911x.c b/arch/arm/mach-omap2/gpmc-smsc911x.c
> index 5ce00ad2..59a2ee4 100644
> --- a/arch/arm/mach-omap2/gpmc-smsc911x.c
> +++ b/arch/arm/mach-omap2/gpmc-smsc911x.c
> @@ -19,6 +19,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
>  #include <linux/smsc911x.h>
> +#include <linux/of.h>
>  
>  #include "gpmc.h"
>  #include "gpmc-smsc911x.h"
> @@ -98,3 +99,31 @@ free1:
>  
>  	pr_err("Could not initialize smsc911x device\n");
>  }
> +
> +int gpmc_smsc911x_init_dt(struct device_node *node)
> +{
> +	struct omap_smsc911x_platform_data gpmc_cfg;
> +
> +	if (WARN_ON(!node))
> +		return -ENODEV;
> +
> +	if (of_property_read_u32(node, "gpmc,cs", &gpmc_cfg.cs)) {
> +		pr_err("Unable to get GPMC smsc911x chip select\n");
> +		return -EINVAL;
> +	}
> +
> +	if (of_property_read_u32(node, "gpmc,gpio_irq", &gpmc_cfg.gpio_irq)) {
> +		pr_err("Unable to get GPMC smsc911x GPIO IRQ\n");
> +		return -EINVAL;
> +	}
> +
> +	if (of_property_read_u32(node, "gpmc,gpio_reset", &gpmc_cfg.gpio_reset))
> +		gpmc_cfg.gpio_reset = -EINVAL;
> +
> +	if (of_property_read_u32(node, "gpmc,flags", &gpmc_cfg.flags))
> +		gpmc_cfg.flags = 0;

Is no sanity checking required for any of the above properties?

To handle separate flags here, you can use something like:

if (of_property_read_bool(node, "gpmc,use-32bit")
	flags |= SMSC911X_USE_32BIT;

[...]

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH RFC 1/7] platform: add a device node
  2013-02-11  8:16             ` Sascha Hauer
@ 2013-02-11 10:33               ` Javier Martinez Canillas
  -1 siblings, 0 replies; 46+ messages in thread
From: Javier Martinez Canillas @ 2013-02-11 10:33 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Russell King - ARM Linux, Greg Kroah-Hartman, devicetree-discuss,
	Enric Balletbo i Serra, linux-omap, Javier Martinez Canillas,
	linux-arm-kernel

On Mon, Feb 11, 2013 at 9:16 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Sun, Feb 10, 2013 at 12:35:43PM +0100, Javier Martinez Canillas wrote:
>> On Sun, Feb 10, 2013 at 10:37 AM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > On Sun, Feb 10, 2013 at 02:49:21AM +0100, Javier Martinez Canillas wrote:
>> >> I knew this would be controversial and that's why I didn't mean it to be a patch
>> >> but a RFC :)
>> >>
>> >> The problem basically is that you have to associate the platform device with its
>> >> corresponding DT device node because it can be used in the driver probe function.
>> >
>> > When DT is being used, doesn't DT create the platform devices for you,
>> > with the device node already set correctly?
>> >
>>
>> Well they usually do but not always just like usually you have a
>> platform_device in your board code and call platform_device_register().
>>
>> But in some configurations you can't just define the platform_device
>> required resources (I/O memory, IRQ, etc) as static platform data.
>> In some cases you need a level of indirection.
>>
>> In this particular case a SMSC ethernet chip is connected to an
>> OMAP3 processor through its General-Purpose Memory Controller.
>>
>> You can't just define the I/O memory used by the device since you first
>> need to request that address to the GPMC. The same happens with the
>> IRQ line since a OMAP GPIO pin is used so you have to first configure
>> the GPIO line as input.
>
> For the gpio interrupt you can use, example taken from omap4-var-som.dts:
>
>         interrupt-parent = <&gpio6>;
>         interrupts = <11>; /* gpio line 171 */
>
> Other architectures allow to specify the edge/level hi/low active
> parameters from the devicetree aswell. The gpio direction should be
> handled by the gpio driver as necessary, at least that's what done on
> other architectures.
>
> If the SMSC hangs on the GPMC then the SMSC should be a child node of
> the GPMC. The GPMC would then act as a bus driver and configure the
> chipselects and timings for its children automatically, maybe based
> on timing information from the devicetree. I've never tried this before,
> but I think that's the way things should be.
>

Hi Sasha,

The SMSC is already a child node of the GPMC in the device tree but instead
using the generic SMSC binding I added a GPMC-specific SMSC binding.

Since the SMSC binding doesn't have a chip select property and it expects
the I/O memory address to be explicitly defined in the reg property while
the GPMC needs to request this memory according to the chip select used.

> No need to manually create platform devices from the devicetree.
>

Thanks a lot for your feedback, I'll go back and think more about this and try
to solve this using a different approach that won't require changing
the platform
device info structure and reusing the generic SMSC LAN DT binding.

> Sascha
>

Best regards,
Javier

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH RFC 1/7] platform: add a device node
@ 2013-02-11 10:33               ` Javier Martinez Canillas
  0 siblings, 0 replies; 46+ messages in thread
From: Javier Martinez Canillas @ 2013-02-11 10:33 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Feb 11, 2013 at 9:16 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Sun, Feb 10, 2013 at 12:35:43PM +0100, Javier Martinez Canillas wrote:
>> On Sun, Feb 10, 2013 at 10:37 AM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > On Sun, Feb 10, 2013 at 02:49:21AM +0100, Javier Martinez Canillas wrote:
>> >> I knew this would be controversial and that's why I didn't mean it to be a patch
>> >> but a RFC :)
>> >>
>> >> The problem basically is that you have to associate the platform device with its
>> >> corresponding DT device node because it can be used in the driver probe function.
>> >
>> > When DT is being used, doesn't DT create the platform devices for you,
>> > with the device node already set correctly?
>> >
>>
>> Well they usually do but not always just like usually you have a
>> platform_device in your board code and call platform_device_register().
>>
>> But in some configurations you can't just define the platform_device
>> required resources (I/O memory, IRQ, etc) as static platform data.
>> In some cases you need a level of indirection.
>>
>> In this particular case a SMSC ethernet chip is connected to an
>> OMAP3 processor through its General-Purpose Memory Controller.
>>
>> You can't just define the I/O memory used by the device since you first
>> need to request that address to the GPMC. The same happens with the
>> IRQ line since a OMAP GPIO pin is used so you have to first configure
>> the GPIO line as input.
>
> For the gpio interrupt you can use, example taken from omap4-var-som.dts:
>
>         interrupt-parent = <&gpio6>;
>         interrupts = <11>; /* gpio line 171 */
>
> Other architectures allow to specify the edge/level hi/low active
> parameters from the devicetree aswell. The gpio direction should be
> handled by the gpio driver as necessary, at least that's what done on
> other architectures.
>
> If the SMSC hangs on the GPMC then the SMSC should be a child node of
> the GPMC. The GPMC would then act as a bus driver and configure the
> chipselects and timings for its children automatically, maybe based
> on timing information from the devicetree. I've never tried this before,
> but I think that's the way things should be.
>

Hi Sasha,

The SMSC is already a child node of the GPMC in the device tree but instead
using the generic SMSC binding I added a GPMC-specific SMSC binding.

Since the SMSC binding doesn't have a chip select property and it expects
the I/O memory address to be explicitly defined in the reg property while
the GPMC needs to request this memory according to the chip select used.

> No need to manually create platform devices from the devicetree.
>

Thanks a lot for your feedback, I'll go back and think more about this and try
to solve this using a different approach that won't require changing
the platform
device info structure and reusing the generic SMSC LAN DT binding.

> Sascha
>

Best regards,
Javier

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH RFC 3/7] ARM: OMAP: gpmc-smsc911x: add DT dev node init function
  2013-02-11 10:30     ` Mark Rutland
@ 2013-02-11 10:40       ` Javier Martinez Canillas
  -1 siblings, 0 replies; 46+ messages in thread
From: Javier Martinez Canillas @ 2013-02-11 10:40 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Javier Martinez Canillas, Tony Lindgren, Russell King,
	Benoît Cousson, Enric Balletbo i Serra, Greg Kroah-Hartman,
	Linus Walleij, Ezequiel Garcia, linux-omap, devicetree-discuss,
	linux-arm-kernel

On Mon, Feb 11, 2013 at 11:30 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> Hi,
>
> I have a few comments on the binding and the way it's parsed.
>
> On Sat, Feb 09, 2013 at 08:44:27PM +0000, Javier Martinez Canillas wrote:
>> This patch adds a helper function to parse a device node that
>> contains all the properties needed to initialize an smsc911x
>> device connected to an OMAP processor through OMAP's GPMC.
>>
>> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>> ---
>>  .../devicetree/bindings/net/gpmc-smsc911x.txt      |   39 ++++++++++++++++++++
>>  arch/arm/mach-omap2/gpmc-smsc911x.c                |   29 +++++++++++++++
>>  arch/arm/mach-omap2/gpmc-smsc911x.h                |    6 +++
>>  3 files changed, 74 insertions(+), 0 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/net/gpmc-smsc911x.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/gpmc-smsc911x.txt b/Documentation/devicetree/bindings/net/gpmc-smsc911x.txt
>> new file mode 100644
>> index 0000000..8bb0df2
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/gpmc-smsc911x.txt
>> @@ -0,0 +1,39 @@
>> +* GPMC connected Smart Mixed-Signal Connectivity (SMSC) LAN911x/912x Controller
>> +
>> +GPMC connected SMSC LAN911x/912x Controllers are represented as child nodes
>> +of the OMAP General-Purpose Memory Controller with a name of "gpmc_smsc911x".
>> +
>> +All timing relevant properties as well as generic gpmc child properties are
>> +explained in a separate documents - please refer to
>> +Documentation/devicetree/bindings/bus/ti-gpmc.txt
>> +
>> +Required properties:
>> +- gpmc,cs : The chip select line the pheripheral is connected to
>> +- gpmc,gpio_irq : The GPIO pin that is connected to the SMSC LAN IRQ line
>
> In devicetree land, we use '-' in preference of '_' in property names.
>
> So this should be "gpio-irq"
>
>> +
>> +Optional properties:
>> +- gpmc,flags : SMSC LAN flags - please refer to include/linux/smsc911x.h
>
> Please don't do this. It should only be necessary to read binding documents and
> hardware datasheets to write a dts. Referring to Linux internals creates a
> flaky ABI that's only going to break when things get renamed/moved/modified.
>
> The flags variable used internally by the driver don't have to have a 1-1
> correspondence with a single property in the binding. You can have boolean
> properties (named, but without a value) in the dt specifying each flag
> individually. That way the dts is easier to read, is less tied to linux
> internals (and every property is *fully* documented), and it's far easier to
> add new flags in future if necessary.
>
>> +- gpmc,gpio_reset : The GPIO pin connected to the SMSC LAN reset line
>
> Preferably: "gpio-reset".
>
>> +
>> +Note: Besides these properties, the gpmc_smsc911x device node could need
>> +aditional setup such as pin/pad mux settings and voltage regulators. This
>> +depend on how the pheripheral is wired and his board specific.
>> +
>> +Example (for an OMAP3 board):
>> +
>> +gpmc@6e000000 {
>> +           compatible = "ti,omap3430-gpmc";
>> +           ti,hwmods = "gpmc";
>> +           reg = <0x6e000000 0x1000000>;
>> +           interrupts = <20>;
>> +           gpmc,num-cs = <8>;
>> +           gpmc,num-waitpins = <4>;
>> +           #address-cells = <2>;
>> +           #size-cells = <1>;
>> +
>> +           gpmc_smsc911x@0 {
>> +                           gpmc,cs = <5>;
>> +                           gpmc,gpio_irq = <176>;
>> +                           gpmc,flags = <4>; /* SMSC911X_USE_32BIT */
>
> Here you could have a boolean property "gpmc,use-32bit" (or possibly
> "gpmc,use-bits", which can either be 32 or 16).
>
>> +           };
>> +};
>> diff --git a/arch/arm/mach-omap2/gpmc-smsc911x.c b/arch/arm/mach-omap2/gpmc-smsc911x.c
>> index 5ce00ad2..59a2ee4 100644
>> --- a/arch/arm/mach-omap2/gpmc-smsc911x.c
>> +++ b/arch/arm/mach-omap2/gpmc-smsc911x.c
>> @@ -19,6 +19,7 @@
>>  #include <linux/interrupt.h>
>>  #include <linux/io.h>
>>  #include <linux/smsc911x.h>
>> +#include <linux/of.h>
>>
>>  #include "gpmc.h"
>>  #include "gpmc-smsc911x.h"
>> @@ -98,3 +99,31 @@ free1:
>>
>>       pr_err("Could not initialize smsc911x device\n");
>>  }
>> +
>> +int gpmc_smsc911x_init_dt(struct device_node *node)
>> +{
>> +     struct omap_smsc911x_platform_data gpmc_cfg;
>> +
>> +     if (WARN_ON(!node))
>> +             return -ENODEV;
>> +
>> +     if (of_property_read_u32(node, "gpmc,cs", &gpmc_cfg.cs)) {
>> +             pr_err("Unable to get GPMC smsc911x chip select\n");
>> +             return -EINVAL;
>> +     }
>> +
>> +     if (of_property_read_u32(node, "gpmc,gpio_irq", &gpmc_cfg.gpio_irq)) {
>> +             pr_err("Unable to get GPMC smsc911x GPIO IRQ\n");
>> +             return -EINVAL;
>> +     }
>> +
>> +     if (of_property_read_u32(node, "gpmc,gpio_reset", &gpmc_cfg.gpio_reset))
>> +             gpmc_cfg.gpio_reset = -EINVAL;
>> +
>> +     if (of_property_read_u32(node, "gpmc,flags", &gpmc_cfg.flags))
>> +             gpmc_cfg.flags = 0;
>
> Is no sanity checking required for any of the above properties?
>
> To handle separate flags here, you can use something like:
>
> if (of_property_read_bool(node, "gpmc,use-32bit")
>         flags |= SMSC911X_USE_32BIT;
>
> [...]
>
> Thanks,
> Mark.
>

Hello Mark,

I really appreciate your feedback.

I'll drop this binding anyways and try to reuse the generic SMSC LAN binding
so it won't be necessary to manually asign the device node to the platform
device.

But I'll keep in mind your comments for the next time I need to write
a DT binding

Thanks a lot and best regards,
Javier

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH RFC 3/7] ARM: OMAP: gpmc-smsc911x: add DT dev node init function
@ 2013-02-11 10:40       ` Javier Martinez Canillas
  0 siblings, 0 replies; 46+ messages in thread
From: Javier Martinez Canillas @ 2013-02-11 10:40 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Feb 11, 2013 at 11:30 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> Hi,
>
> I have a few comments on the binding and the way it's parsed.
>
> On Sat, Feb 09, 2013 at 08:44:27PM +0000, Javier Martinez Canillas wrote:
>> This patch adds a helper function to parse a device node that
>> contains all the properties needed to initialize an smsc911x
>> device connected to an OMAP processor through OMAP's GPMC.
>>
>> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>> ---
>>  .../devicetree/bindings/net/gpmc-smsc911x.txt      |   39 ++++++++++++++++++++
>>  arch/arm/mach-omap2/gpmc-smsc911x.c                |   29 +++++++++++++++
>>  arch/arm/mach-omap2/gpmc-smsc911x.h                |    6 +++
>>  3 files changed, 74 insertions(+), 0 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/net/gpmc-smsc911x.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/gpmc-smsc911x.txt b/Documentation/devicetree/bindings/net/gpmc-smsc911x.txt
>> new file mode 100644
>> index 0000000..8bb0df2
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/gpmc-smsc911x.txt
>> @@ -0,0 +1,39 @@
>> +* GPMC connected Smart Mixed-Signal Connectivity (SMSC) LAN911x/912x Controller
>> +
>> +GPMC connected SMSC LAN911x/912x Controllers are represented as child nodes
>> +of the OMAP General-Purpose Memory Controller with a name of "gpmc_smsc911x".
>> +
>> +All timing relevant properties as well as generic gpmc child properties are
>> +explained in a separate documents - please refer to
>> +Documentation/devicetree/bindings/bus/ti-gpmc.txt
>> +
>> +Required properties:
>> +- gpmc,cs : The chip select line the pheripheral is connected to
>> +- gpmc,gpio_irq : The GPIO pin that is connected to the SMSC LAN IRQ line
>
> In devicetree land, we use '-' in preference of '_' in property names.
>
> So this should be "gpio-irq"
>
>> +
>> +Optional properties:
>> +- gpmc,flags : SMSC LAN flags - please refer to include/linux/smsc911x.h
>
> Please don't do this. It should only be necessary to read binding documents and
> hardware datasheets to write a dts. Referring to Linux internals creates a
> flaky ABI that's only going to break when things get renamed/moved/modified.
>
> The flags variable used internally by the driver don't have to have a 1-1
> correspondence with a single property in the binding. You can have boolean
> properties (named, but without a value) in the dt specifying each flag
> individually. That way the dts is easier to read, is less tied to linux
> internals (and every property is *fully* documented), and it's far easier to
> add new flags in future if necessary.
>
>> +- gpmc,gpio_reset : The GPIO pin connected to the SMSC LAN reset line
>
> Preferably: "gpio-reset".
>
>> +
>> +Note: Besides these properties, the gpmc_smsc911x device node could need
>> +aditional setup such as pin/pad mux settings and voltage regulators. This
>> +depend on how the pheripheral is wired and his board specific.
>> +
>> +Example (for an OMAP3 board):
>> +
>> +gpmc at 6e000000 {
>> +           compatible = "ti,omap3430-gpmc";
>> +           ti,hwmods = "gpmc";
>> +           reg = <0x6e000000 0x1000000>;
>> +           interrupts = <20>;
>> +           gpmc,num-cs = <8>;
>> +           gpmc,num-waitpins = <4>;
>> +           #address-cells = <2>;
>> +           #size-cells = <1>;
>> +
>> +           gpmc_smsc911x at 0 {
>> +                           gpmc,cs = <5>;
>> +                           gpmc,gpio_irq = <176>;
>> +                           gpmc,flags = <4>; /* SMSC911X_USE_32BIT */
>
> Here you could have a boolean property "gpmc,use-32bit" (or possibly
> "gpmc,use-bits", which can either be 32 or 16).
>
>> +           };
>> +};
>> diff --git a/arch/arm/mach-omap2/gpmc-smsc911x.c b/arch/arm/mach-omap2/gpmc-smsc911x.c
>> index 5ce00ad2..59a2ee4 100644
>> --- a/arch/arm/mach-omap2/gpmc-smsc911x.c
>> +++ b/arch/arm/mach-omap2/gpmc-smsc911x.c
>> @@ -19,6 +19,7 @@
>>  #include <linux/interrupt.h>
>>  #include <linux/io.h>
>>  #include <linux/smsc911x.h>
>> +#include <linux/of.h>
>>
>>  #include "gpmc.h"
>>  #include "gpmc-smsc911x.h"
>> @@ -98,3 +99,31 @@ free1:
>>
>>       pr_err("Could not initialize smsc911x device\n");
>>  }
>> +
>> +int gpmc_smsc911x_init_dt(struct device_node *node)
>> +{
>> +     struct omap_smsc911x_platform_data gpmc_cfg;
>> +
>> +     if (WARN_ON(!node))
>> +             return -ENODEV;
>> +
>> +     if (of_property_read_u32(node, "gpmc,cs", &gpmc_cfg.cs)) {
>> +             pr_err("Unable to get GPMC smsc911x chip select\n");
>> +             return -EINVAL;
>> +     }
>> +
>> +     if (of_property_read_u32(node, "gpmc,gpio_irq", &gpmc_cfg.gpio_irq)) {
>> +             pr_err("Unable to get GPMC smsc911x GPIO IRQ\n");
>> +             return -EINVAL;
>> +     }
>> +
>> +     if (of_property_read_u32(node, "gpmc,gpio_reset", &gpmc_cfg.gpio_reset))
>> +             gpmc_cfg.gpio_reset = -EINVAL;
>> +
>> +     if (of_property_read_u32(node, "gpmc,flags", &gpmc_cfg.flags))
>> +             gpmc_cfg.flags = 0;
>
> Is no sanity checking required for any of the above properties?
>
> To handle separate flags here, you can use something like:
>
> if (of_property_read_bool(node, "gpmc,use-32bit")
>         flags |= SMSC911X_USE_32BIT;
>
> [...]
>
> Thanks,
> Mark.
>

Hello Mark,

I really appreciate your feedback.

I'll drop this binding anyways and try to reuse the generic SMSC LAN binding
so it won't be necessary to manually asign the device node to the platform
device.

But I'll keep in mind your comments for the next time I need to write
a DT binding

Thanks a lot and best regards,
Javier

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH RFC 1/7] platform: add a device node
  2013-02-11 10:33               ` Javier Martinez Canillas
@ 2013-02-11 11:24                 ` Sascha Hauer
  -1 siblings, 0 replies; 46+ messages in thread
From: Sascha Hauer @ 2013-02-11 11:24 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: Russell King - ARM Linux, Greg Kroah-Hartman, devicetree-discuss,
	Enric Balletbo i Serra, linux-omap, Javier Martinez Canillas,
	linux-arm-kernel

On Mon, Feb 11, 2013 at 11:33:19AM +0100, Javier Martinez Canillas wrote:
> On Mon, Feb 11, 2013 at 9:16 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > On Sun, Feb 10, 2013 at 12:35:43PM +0100, Javier Martinez Canillas wrote:
> >> On Sun, Feb 10, 2013 at 10:37 AM, Russell King - ARM Linux
> >> <linux@arm.linux.org.uk> wrote:
> >> > On Sun, Feb 10, 2013 at 02:49:21AM +0100, Javier Martinez Canillas wrote:
> >> >> I knew this would be controversial and that's why I didn't mean it to be a patch
> >> >> but a RFC :)
> >> >>
> >> >> The problem basically is that you have to associate the platform device with its
> >> >> corresponding DT device node because it can be used in the driver probe function.
> >> >
> >> > When DT is being used, doesn't DT create the platform devices for you,
> >> > with the device node already set correctly?
> >> >
> >>
> >> Well they usually do but not always just like usually you have a
> >> platform_device in your board code and call platform_device_register().
> >>
> >> But in some configurations you can't just define the platform_device
> >> required resources (I/O memory, IRQ, etc) as static platform data.
> >> In some cases you need a level of indirection.
> >>
> >> In this particular case a SMSC ethernet chip is connected to an
> >> OMAP3 processor through its General-Purpose Memory Controller.
> >>
> >> You can't just define the I/O memory used by the device since you first
> >> need to request that address to the GPMC. The same happens with the
> >> IRQ line since a OMAP GPIO pin is used so you have to first configure
> >> the GPIO line as input.
> >
> > For the gpio interrupt you can use, example taken from omap4-var-som.dts:
> >
> >         interrupt-parent = <&gpio6>;
> >         interrupts = <11>; /* gpio line 171 */
> >
> > Other architectures allow to specify the edge/level hi/low active
> > parameters from the devicetree aswell. The gpio direction should be
> > handled by the gpio driver as necessary, at least that's what done on
> > other architectures.
> >
> > If the SMSC hangs on the GPMC then the SMSC should be a child node of
> > the GPMC. The GPMC would then act as a bus driver and configure the
> > chipselects and timings for its children automatically, maybe based
> > on timing information from the devicetree. I've never tried this before,
> > but I think that's the way things should be.
> >
> 
> Hi Sasha,
> 
> The SMSC is already a child node of the GPMC in the device tree but instead
> using the generic SMSC binding I added a GPMC-specific SMSC binding.
> 
> Since the SMSC binding doesn't have a chip select property and it expects
> the I/O memory address to be explicitly defined in the reg property while
> the GPMC needs to request this memory according to the chip select used.

So you probably have this:

	gpmc {
		compatible = "ti,gpmc", "simple-bus";
		ranges;

		smsc911x {
			compatible = "smsc,91x";
		}
	}

If you remove the simple-bus property the gpmc devices would not be
probed. If then you add a driver which matches "ti,gpmc" you can
configure the chip selects in its probe callback. After this you
can call of_platform_populate() starting from the gpmc device node
to instantiate the gpmc child devices.

Please somebody interrupt me if I'm talking total rubbish here. I never
tried this and only assume it should work like this.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH RFC 1/7] platform: add a device node
@ 2013-02-11 11:24                 ` Sascha Hauer
  0 siblings, 0 replies; 46+ messages in thread
From: Sascha Hauer @ 2013-02-11 11:24 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Feb 11, 2013 at 11:33:19AM +0100, Javier Martinez Canillas wrote:
> On Mon, Feb 11, 2013 at 9:16 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > On Sun, Feb 10, 2013 at 12:35:43PM +0100, Javier Martinez Canillas wrote:
> >> On Sun, Feb 10, 2013 at 10:37 AM, Russell King - ARM Linux
> >> <linux@arm.linux.org.uk> wrote:
> >> > On Sun, Feb 10, 2013 at 02:49:21AM +0100, Javier Martinez Canillas wrote:
> >> >> I knew this would be controversial and that's why I didn't mean it to be a patch
> >> >> but a RFC :)
> >> >>
> >> >> The problem basically is that you have to associate the platform device with its
> >> >> corresponding DT device node because it can be used in the driver probe function.
> >> >
> >> > When DT is being used, doesn't DT create the platform devices for you,
> >> > with the device node already set correctly?
> >> >
> >>
> >> Well they usually do but not always just like usually you have a
> >> platform_device in your board code and call platform_device_register().
> >>
> >> But in some configurations you can't just define the platform_device
> >> required resources (I/O memory, IRQ, etc) as static platform data.
> >> In some cases you need a level of indirection.
> >>
> >> In this particular case a SMSC ethernet chip is connected to an
> >> OMAP3 processor through its General-Purpose Memory Controller.
> >>
> >> You can't just define the I/O memory used by the device since you first
> >> need to request that address to the GPMC. The same happens with the
> >> IRQ line since a OMAP GPIO pin is used so you have to first configure
> >> the GPIO line as input.
> >
> > For the gpio interrupt you can use, example taken from omap4-var-som.dts:
> >
> >         interrupt-parent = <&gpio6>;
> >         interrupts = <11>; /* gpio line 171 */
> >
> > Other architectures allow to specify the edge/level hi/low active
> > parameters from the devicetree aswell. The gpio direction should be
> > handled by the gpio driver as necessary, at least that's what done on
> > other architectures.
> >
> > If the SMSC hangs on the GPMC then the SMSC should be a child node of
> > the GPMC. The GPMC would then act as a bus driver and configure the
> > chipselects and timings for its children automatically, maybe based
> > on timing information from the devicetree. I've never tried this before,
> > but I think that's the way things should be.
> >
> 
> Hi Sasha,
> 
> The SMSC is already a child node of the GPMC in the device tree but instead
> using the generic SMSC binding I added a GPMC-specific SMSC binding.
> 
> Since the SMSC binding doesn't have a chip select property and it expects
> the I/O memory address to be explicitly defined in the reg property while
> the GPMC needs to request this memory according to the chip select used.

So you probably have this:

	gpmc {
		compatible = "ti,gpmc", "simple-bus";
		ranges;

		smsc911x {
			compatible = "smsc,91x";
		}
	}

If you remove the simple-bus property the gpmc devices would not be
probed. If then you add a driver which matches "ti,gpmc" you can
configure the chip selects in its probe callback. After this you
can call of_platform_populate() starting from the gpmc device node
to instantiate the gpmc child devices.

Please somebody interrupt me if I'm talking total rubbish here. I never
tried this and only assume it should work like this.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH RFC 1/7] platform: add a device node
  2013-02-11 11:24                 ` Sascha Hauer
@ 2013-02-11 11:38                   ` Javier Martinez Canillas
  -1 siblings, 0 replies; 46+ messages in thread
From: Javier Martinez Canillas @ 2013-02-11 11:38 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Russell King - ARM Linux, Greg Kroah-Hartman, devicetree-discuss,
	Enric Balletbo i Serra, linux-omap, Javier Martinez Canillas,
	linux-arm-kernel

On Mon, Feb 11, 2013 at 12:24 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Mon, Feb 11, 2013 at 11:33:19AM +0100, Javier Martinez Canillas wrote:
>> On Mon, Feb 11, 2013 at 9:16 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
>> > On Sun, Feb 10, 2013 at 12:35:43PM +0100, Javier Martinez Canillas wrote:
>> >> On Sun, Feb 10, 2013 at 10:37 AM, Russell King - ARM Linux
>> >> <linux@arm.linux.org.uk> wrote:
>> >> > On Sun, Feb 10, 2013 at 02:49:21AM +0100, Javier Martinez Canillas wrote:
>> >> >> I knew this would be controversial and that's why I didn't mean it to be a patch
>> >> >> but a RFC :)
>> >> >>
>> >> >> The problem basically is that you have to associate the platform device with its
>> >> >> corresponding DT device node because it can be used in the driver probe function.
>> >> >
>> >> > When DT is being used, doesn't DT create the platform devices for you,
>> >> > with the device node already set correctly?
>> >> >
>> >>
>> >> Well they usually do but not always just like usually you have a
>> >> platform_device in your board code and call platform_device_register().
>> >>
>> >> But in some configurations you can't just define the platform_device
>> >> required resources (I/O memory, IRQ, etc) as static platform data.
>> >> In some cases you need a level of indirection.
>> >>
>> >> In this particular case a SMSC ethernet chip is connected to an
>> >> OMAP3 processor through its General-Purpose Memory Controller.
>> >>
>> >> You can't just define the I/O memory used by the device since you first
>> >> need to request that address to the GPMC. The same happens with the
>> >> IRQ line since a OMAP GPIO pin is used so you have to first configure
>> >> the GPIO line as input.
>> >
>> > For the gpio interrupt you can use, example taken from omap4-var-som.dts:
>> >
>> >         interrupt-parent = <&gpio6>;
>> >         interrupts = <11>; /* gpio line 171 */
>> >
>> > Other architectures allow to specify the edge/level hi/low active
>> > parameters from the devicetree aswell. The gpio direction should be
>> > handled by the gpio driver as necessary, at least that's what done on
>> > other architectures.
>> >
>> > If the SMSC hangs on the GPMC then the SMSC should be a child node of
>> > the GPMC. The GPMC would then act as a bus driver and configure the
>> > chipselects and timings for its children automatically, maybe based
>> > on timing information from the devicetree. I've never tried this before,
>> > but I think that's the way things should be.
>> >
>>
>> Hi Sasha,
>>
>> The SMSC is already a child node of the GPMC in the device tree but instead
>> using the generic SMSC binding I added a GPMC-specific SMSC binding.
>>
>> Since the SMSC binding doesn't have a chip select property and it expects
>> the I/O memory address to be explicitly defined in the reg property while
>> the GPMC needs to request this memory according to the chip select used.
>
> So you probably have this:
>
>         gpmc {
>                 compatible = "ti,gpmc", "simple-bus";
>                 ranges;
>
>                 smsc911x {
>                         compatible = "smsc,91x";
>                 }
>         }
>

Yes

> If you remove the simple-bus property the gpmc devices would not be
> probed. If then you add a driver which matches "ti,gpmc" you can
> configure the chip selects in its probe callback. After this you
> can call of_platform_populate() starting from the gpmc device node
> to instantiate the gpmc child devices.
>
> Please somebody interrupt me if I'm talking total rubbish here. I never
> tried this and only assume it should work like this.
>

Nice, I haven't thought about that and I don't see why it shouldn't work.

I won't have time to work on this for at least the next 3 weeks but I'll try
once I have more free time and post another RFC if I got things working.

> Sascha
>

Thanks a lot and best regards,
Javier

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH RFC 1/7] platform: add a device node
@ 2013-02-11 11:38                   ` Javier Martinez Canillas
  0 siblings, 0 replies; 46+ messages in thread
From: Javier Martinez Canillas @ 2013-02-11 11:38 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Feb 11, 2013 at 12:24 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Mon, Feb 11, 2013 at 11:33:19AM +0100, Javier Martinez Canillas wrote:
>> On Mon, Feb 11, 2013 at 9:16 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
>> > On Sun, Feb 10, 2013 at 12:35:43PM +0100, Javier Martinez Canillas wrote:
>> >> On Sun, Feb 10, 2013 at 10:37 AM, Russell King - ARM Linux
>> >> <linux@arm.linux.org.uk> wrote:
>> >> > On Sun, Feb 10, 2013 at 02:49:21AM +0100, Javier Martinez Canillas wrote:
>> >> >> I knew this would be controversial and that's why I didn't mean it to be a patch
>> >> >> but a RFC :)
>> >> >>
>> >> >> The problem basically is that you have to associate the platform device with its
>> >> >> corresponding DT device node because it can be used in the driver probe function.
>> >> >
>> >> > When DT is being used, doesn't DT create the platform devices for you,
>> >> > with the device node already set correctly?
>> >> >
>> >>
>> >> Well they usually do but not always just like usually you have a
>> >> platform_device in your board code and call platform_device_register().
>> >>
>> >> But in some configurations you can't just define the platform_device
>> >> required resources (I/O memory, IRQ, etc) as static platform data.
>> >> In some cases you need a level of indirection.
>> >>
>> >> In this particular case a SMSC ethernet chip is connected to an
>> >> OMAP3 processor through its General-Purpose Memory Controller.
>> >>
>> >> You can't just define the I/O memory used by the device since you first
>> >> need to request that address to the GPMC. The same happens with the
>> >> IRQ line since a OMAP GPIO pin is used so you have to first configure
>> >> the GPIO line as input.
>> >
>> > For the gpio interrupt you can use, example taken from omap4-var-som.dts:
>> >
>> >         interrupt-parent = <&gpio6>;
>> >         interrupts = <11>; /* gpio line 171 */
>> >
>> > Other architectures allow to specify the edge/level hi/low active
>> > parameters from the devicetree aswell. The gpio direction should be
>> > handled by the gpio driver as necessary, at least that's what done on
>> > other architectures.
>> >
>> > If the SMSC hangs on the GPMC then the SMSC should be a child node of
>> > the GPMC. The GPMC would then act as a bus driver and configure the
>> > chipselects and timings for its children automatically, maybe based
>> > on timing information from the devicetree. I've never tried this before,
>> > but I think that's the way things should be.
>> >
>>
>> Hi Sasha,
>>
>> The SMSC is already a child node of the GPMC in the device tree but instead
>> using the generic SMSC binding I added a GPMC-specific SMSC binding.
>>
>> Since the SMSC binding doesn't have a chip select property and it expects
>> the I/O memory address to be explicitly defined in the reg property while
>> the GPMC needs to request this memory according to the chip select used.
>
> So you probably have this:
>
>         gpmc {
>                 compatible = "ti,gpmc", "simple-bus";
>                 ranges;
>
>                 smsc911x {
>                         compatible = "smsc,91x";
>                 }
>         }
>

Yes

> If you remove the simple-bus property the gpmc devices would not be
> probed. If then you add a driver which matches "ti,gpmc" you can
> configure the chip selects in its probe callback. After this you
> can call of_platform_populate() starting from the gpmc device node
> to instantiate the gpmc child devices.
>
> Please somebody interrupt me if I'm talking total rubbish here. I never
> tried this and only assume it should work like this.
>

Nice, I haven't thought about that and I don't see why it shouldn't work.

I won't have time to work on this for at least the next 3 weeks but I'll try
once I have more free time and post another RFC if I got things working.

> Sascha
>

Thanks a lot and best regards,
Javier

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH RFC 2/7] net: smsc911x: add pinctrl support
  2013-02-09 20:44   ` Javier Martinez Canillas
@ 2013-02-11 14:23     ` Linus Walleij
  -1 siblings, 0 replies; 46+ messages in thread
From: Linus Walleij @ 2013-02-11 14:23 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: Tony Lindgren, Benoît Cousson, Russell King,
	Greg Kroah-Hartman, Enric Balletbo i Serra, Ezequiel Garcia,
	devicetree-discuss, linux-omap, linux-arm-kernel

On Sat, Feb 9, 2013 at 9:44 PM, Javier Martinez Canillas
<javier.martinez@collabora.co.uk> wrote:

> If no pinctrl is available just report a warning since
> it may not needed in some cases (e.g: non-DT kernels).
>
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
(...)
> +       struct pinctrl *pinctrl;
(...)
> +       pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
> +       if (IS_ERR(pinctrl)) {
> +               retval = PTR_ERR(pinctrl);
> +               if (retval == -EPROBE_DEFER)
> +                       goto out_0;
> +
> +               dev_warn(&pdev->dev, "No pinctrl provided\n");
> +       }

NACK.

This will be handled from the device core after the v3.9 merge
window.

See:
http://marc.info/?l=linux-kernel&m=135887740715083&w=2

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH RFC 2/7] net: smsc911x: add pinctrl support
@ 2013-02-11 14:23     ` Linus Walleij
  0 siblings, 0 replies; 46+ messages in thread
From: Linus Walleij @ 2013-02-11 14:23 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Feb 9, 2013 at 9:44 PM, Javier Martinez Canillas
<javier.martinez@collabora.co.uk> wrote:

> If no pinctrl is available just report a warning since
> it may not needed in some cases (e.g: non-DT kernels).
>
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
(...)
> +       struct pinctrl *pinctrl;
(...)
> +       pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
> +       if (IS_ERR(pinctrl)) {
> +               retval = PTR_ERR(pinctrl);
> +               if (retval == -EPROBE_DEFER)
> +                       goto out_0;
> +
> +               dev_warn(&pdev->dev, "No pinctrl provided\n");
> +       }

NACK.

This will be handled from the device core after the v3.9 merge
window.

See:
http://marc.info/?l=linux-kernel&m=135887740715083&w=2

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH RFC 2/7] net: smsc911x: add pinctrl support
  2013-02-11 14:23     ` Linus Walleij
@ 2013-02-11 14:29       ` Javier Martinez Canillas
  -1 siblings, 0 replies; 46+ messages in thread
From: Javier Martinez Canillas @ 2013-02-11 14:29 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Javier Martinez Canillas, Tony Lindgren, Benoît Cousson,
	Russell King, Greg Kroah-Hartman, Enric Balletbo i Serra,
	Ezequiel Garcia, devicetree-discuss, linux-omap,
	linux-arm-kernel

On Mon, Feb 11, 2013 at 3:23 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Sat, Feb 9, 2013 at 9:44 PM, Javier Martinez Canillas
> <javier.martinez@collabora.co.uk> wrote:
>
>> If no pinctrl is available just report a warning since
>> it may not needed in some cases (e.g: non-DT kernels).
>>
>> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> (...)
>> +       struct pinctrl *pinctrl;
> (...)
>> +       pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
>> +       if (IS_ERR(pinctrl)) {
>> +               retval = PTR_ERR(pinctrl);
>> +               if (retval == -EPROBE_DEFER)
>> +                       goto out_0;
>> +
>> +               dev_warn(&pdev->dev, "No pinctrl provided\n");
>> +       }
>
> NACK.
>
> This will be handled from the device core after the v3.9 merge
> window.
>
> See:
> http://marc.info/?l=linux-kernel&m=135887740715083&w=2
>

Great, I also wondered why it couldn't be part of the device core
since we basically duplicate this in all the DT aware drivers and
pinctrl was not used anywhere after setting to its default state.

This is a nice improvement.

> Yours,
> Linus Walleij
> --

Thanks a lot and best regards,
Javier

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH RFC 2/7] net: smsc911x: add pinctrl support
@ 2013-02-11 14:29       ` Javier Martinez Canillas
  0 siblings, 0 replies; 46+ messages in thread
From: Javier Martinez Canillas @ 2013-02-11 14:29 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Feb 11, 2013 at 3:23 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Sat, Feb 9, 2013 at 9:44 PM, Javier Martinez Canillas
> <javier.martinez@collabora.co.uk> wrote:
>
>> If no pinctrl is available just report a warning since
>> it may not needed in some cases (e.g: non-DT kernels).
>>
>> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> (...)
>> +       struct pinctrl *pinctrl;
> (...)
>> +       pinctrl = devm_pinctrl_get_select_default(&pdev->dev);
>> +       if (IS_ERR(pinctrl)) {
>> +               retval = PTR_ERR(pinctrl);
>> +               if (retval == -EPROBE_DEFER)
>> +                       goto out_0;
>> +
>> +               dev_warn(&pdev->dev, "No pinctrl provided\n");
>> +       }
>
> NACK.
>
> This will be handled from the device core after the v3.9 merge
> window.
>
> See:
> http://marc.info/?l=linux-kernel&m=135887740715083&w=2
>

Great, I also wondered why it couldn't be part of the device core
since we basically duplicate this in all the DT aware drivers and
pinctrl was not used anywhere after setting to its default state.

This is a nice improvement.

> Yours,
> Linus Walleij
> --

Thanks a lot and best regards,
Javier

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH RFC 1/7] platform: add a device node
  2013-02-10  9:37         ` Russell King - ARM Linux
@ 2013-02-18 13:33           ` Grant Likely
  -1 siblings, 0 replies; 46+ messages in thread
From: Grant Likely @ 2013-02-18 13:33 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Javier Martinez Canillas, Greg Kroah-Hartman, Tony Lindgren,
	Benoît Cousson, Linus Walleij, Enric Balletbo i Serra,
	Ezequiel Garcia, devicetree-discuss, linux-omap,
	linux-arm-kernel

On Sun, Feb 10, 2013 at 9:37 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Sun, Feb 10, 2013 at 02:49:21AM +0100, Javier Martinez Canillas wrote:
>> I knew this would be controversial and that's why I didn't mean it to be a patch
>> but a RFC :)
>>
>> The problem basically is that you have to associate the platform device with its
>> corresponding DT device node because it can be used in the driver probe function.
>
> When DT is being used, doesn't DT create the platform devices for you,
> with the device node already set correctly?
>
> Manually creating platform devices and adding DT nodes to it sounds like
> the wrong thing to be doing.

Right; I'd expect your platform device creation to go through the
of_platform_populate() route. What is your use-case?

g.



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH RFC 1/7] platform: add a device node
@ 2013-02-18 13:33           ` Grant Likely
  0 siblings, 0 replies; 46+ messages in thread
From: Grant Likely @ 2013-02-18 13:33 UTC (permalink / raw)
  To: linux-arm-kernel

On Sun, Feb 10, 2013 at 9:37 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Sun, Feb 10, 2013 at 02:49:21AM +0100, Javier Martinez Canillas wrote:
>> I knew this would be controversial and that's why I didn't mean it to be a patch
>> but a RFC :)
>>
>> The problem basically is that you have to associate the platform device with its
>> corresponding DT device node because it can be used in the driver probe function.
>
> When DT is being used, doesn't DT create the platform devices for you,
> with the device node already set correctly?
>
> Manually creating platform devices and adding DT nodes to it sounds like
> the wrong thing to be doing.

Right; I'd expect your platform device creation to go through the
of_platform_populate() route. What is your use-case?

g.



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH RFC 1/7] platform: add a device node
  2013-02-10 11:35           ` Javier Martinez Canillas
@ 2013-02-18 13:51             ` Grant Likely
  -1 siblings, 0 replies; 46+ messages in thread
From: Grant Likely @ 2013-02-18 13:51 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: Russell King - ARM Linux, Javier Martinez Canillas,
	Greg Kroah-Hartman, Tony Lindgren, Benoît Cousson,
	Linus Walleij, Enric Balletbo i Serra, Ezequiel Garcia,
	devicetree-discuss, linux-omap, linux-arm-kernel

On Sun, Feb 10, 2013 at 11:35 AM, Javier Martinez Canillas
<martinez.javier@gmail.com> wrote:
> On Sun, Feb 10, 2013 at 10:37 AM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
>> On Sun, Feb 10, 2013 at 02:49:21AM +0100, Javier Martinez Canillas wrote:
>>> I knew this would be controversial and that's why I didn't mean it to be a patch
>>> but a RFC :)
>>>
>>> The problem basically is that you have to associate the platform device with its
>>> corresponding DT device node because it can be used in the driver probe function.
>>
>> When DT is being used, doesn't DT create the platform devices for you,
>> with the device node already set correctly?
>>
>
> Well they usually do but not always just like usually you have a
> platform_device in your board code and call platform_device_register().
>
> But in some configurations you can't just define the platform_device
> required resources (I/O memory, IRQ, etc) as static platform data.
> In some cases you need a level of indirection.
>
> In this particular case a SMSC ethernet chip is connected to an
> OMAP3 processor through its General-Purpose Memory Controller.
>
> You can't just define the I/O memory used by the device since you first
> need to request that address to the GPMC. The same happens with the
> IRQ line since a OMAP GPIO pin is used so you have to first configure
> the GPIO line as input.
>
> So in platform code you call to gpmc_smsc911x_init() passing all the
> GPMC specific configurations (GPIO used for IRQ, GPMC chip select, etc)
>
> Then gpmc_smsc911x_init() does all the needed setup, fills a platform_data
> for the real platform_device and calls  platform_device_register_resndata().
>
> From the smsc911x platform_driver probe function point of view it just have
> resources and doesn't know if it's I/O memory is directly mapped or is
> through a memory controller as the GPMC. It also doesn't know if the IRQ is
> a GPIO or not.

It's still the same difference as far as the device is concerned.
External bus chip-select lines are well understood. The key here is to
encode the CS line number into the reg property of the child node so
that the GPMC driver has the information it needs to configure the
chip selects. You do this by setting #address-cells to '2' in the GPMC
node, and  use the ranges property to map from the gpmc address space
to the cpu address space. Like so (if you had 2 Ethernet controllers)

        gpmc {
                #address-cells = <2>;  // First cell is CS#, second
cell is offset from CS base
                #size-cells = <1>;
                compatible = "ti,gpmc";
                ranges = <0 0 0xf1000000 0x1000>, //CS0 mapped to
0xf1000000..0xf1000fff
                                <1 0 0xf1001000 0x1000>; //CS1 mapped
to 0xf1001000..0xf1001fff

                ethernet@0,0 {
                        compatible = "smsc,91c111";
                        reg = <0 0 0x1000>;
                }
                ethernet@1,0 {
                        compatible = "smsc,91c111";
                        reg = <1 0 0x1000>;
               }
      }

The GPMC driver can use the information in the ranges property for
setting up the chip select mappings. For the smsc,91c111 driver the
mapping becomes transparent.

You can see another example of this in
arch/powerpc/boot/dts/media5200.dts in the localbus node.

g.

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH RFC 1/7] platform: add a device node
@ 2013-02-18 13:51             ` Grant Likely
  0 siblings, 0 replies; 46+ messages in thread
From: Grant Likely @ 2013-02-18 13:51 UTC (permalink / raw)
  To: linux-arm-kernel

On Sun, Feb 10, 2013 at 11:35 AM, Javier Martinez Canillas
<martinez.javier@gmail.com> wrote:
> On Sun, Feb 10, 2013 at 10:37 AM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
>> On Sun, Feb 10, 2013 at 02:49:21AM +0100, Javier Martinez Canillas wrote:
>>> I knew this would be controversial and that's why I didn't mean it to be a patch
>>> but a RFC :)
>>>
>>> The problem basically is that you have to associate the platform device with its
>>> corresponding DT device node because it can be used in the driver probe function.
>>
>> When DT is being used, doesn't DT create the platform devices for you,
>> with the device node already set correctly?
>>
>
> Well they usually do but not always just like usually you have a
> platform_device in your board code and call platform_device_register().
>
> But in some configurations you can't just define the platform_device
> required resources (I/O memory, IRQ, etc) as static platform data.
> In some cases you need a level of indirection.
>
> In this particular case a SMSC ethernet chip is connected to an
> OMAP3 processor through its General-Purpose Memory Controller.
>
> You can't just define the I/O memory used by the device since you first
> need to request that address to the GPMC. The same happens with the
> IRQ line since a OMAP GPIO pin is used so you have to first configure
> the GPIO line as input.
>
> So in platform code you call to gpmc_smsc911x_init() passing all the
> GPMC specific configurations (GPIO used for IRQ, GPMC chip select, etc)
>
> Then gpmc_smsc911x_init() does all the needed setup, fills a platform_data
> for the real platform_device and calls  platform_device_register_resndata().
>
> From the smsc911x platform_driver probe function point of view it just have
> resources and doesn't know if it's I/O memory is directly mapped or is
> through a memory controller as the GPMC. It also doesn't know if the IRQ is
> a GPIO or not.

It's still the same difference as far as the device is concerned.
External bus chip-select lines are well understood. The key here is to
encode the CS line number into the reg property of the child node so
that the GPMC driver has the information it needs to configure the
chip selects. You do this by setting #address-cells to '2' in the GPMC
node, and  use the ranges property to map from the gpmc address space
to the cpu address space. Like so (if you had 2 Ethernet controllers)

        gpmc {
                #address-cells = <2>;  // First cell is CS#, second
cell is offset from CS base
                #size-cells = <1>;
                compatible = "ti,gpmc";
                ranges = <0 0 0xf1000000 0x1000>, //CS0 mapped to
0xf1000000..0xf1000fff
                                <1 0 0xf1001000 0x1000>; //CS1 mapped
to 0xf1001000..0xf1001fff

                ethernet at 0,0 {
                        compatible = "smsc,91c111";
                        reg = <0 0 0x1000>;
                }
                ethernet at 1,0 {
                        compatible = "smsc,91c111";
                        reg = <1 0 0x1000>;
               }
      }

The GPMC driver can use the information in the ranges property for
setting up the chip select mappings. For the smsc,91c111 driver the
mapping becomes transparent.

You can see another example of this in
arch/powerpc/boot/dts/media5200.dts in the localbus node.

g.

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [PATCH RFC 1/7] platform: add a device node
  2013-02-18 13:51             ` Grant Likely
@ 2013-02-18 13:56               ` Javier Martinez Canillas
  -1 siblings, 0 replies; 46+ messages in thread
From: Javier Martinez Canillas @ 2013-02-18 13:56 UTC (permalink / raw)
  To: Grant Likely
  Cc: Javier Martinez Canillas, Russell King - ARM Linux,
	Greg Kroah-Hartman, Tony Lindgren, Benoît Cousson,
	Linus Walleij, Enric Balletbo i Serra, Ezequiel Garcia,
	devicetree-discuss, linux-omap, linux-arm-kernel

On 02/18/2013 02:51 PM, Grant Likely wrote:
> On Sun, Feb 10, 2013 at 11:35 AM, Javier Martinez Canillas
> <martinez.javier@gmail.com> wrote:
>> On Sun, Feb 10, 2013 at 10:37 AM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>>> On Sun, Feb 10, 2013 at 02:49:21AM +0100, Javier Martinez Canillas wrote:
>>>> I knew this would be controversial and that's why I didn't mean it to be a patch
>>>> but a RFC :)
>>>>
>>>> The problem basically is that you have to associate the platform device with its
>>>> corresponding DT device node because it can be used in the driver probe function.
>>>
>>> When DT is being used, doesn't DT create the platform devices for you,
>>> with the device node already set correctly?
>>>
>>
>> Well they usually do but not always just like usually you have a
>> platform_device in your board code and call platform_device_register().
>>
>> But in some configurations you can't just define the platform_device
>> required resources (I/O memory, IRQ, etc) as static platform data.
>> In some cases you need a level of indirection.
>>
>> In this particular case a SMSC ethernet chip is connected to an
>> OMAP3 processor through its General-Purpose Memory Controller.
>>
>> You can't just define the I/O memory used by the device since you first
>> need to request that address to the GPMC. The same happens with the
>> IRQ line since a OMAP GPIO pin is used so you have to first configure
>> the GPIO line as input.
>>
>> So in platform code you call to gpmc_smsc911x_init() passing all the
>> GPMC specific configurations (GPIO used for IRQ, GPMC chip select, etc)
>>
>> Then gpmc_smsc911x_init() does all the needed setup, fills a platform_data
>> for the real platform_device and calls  platform_device_register_resndata().
>>
>> From the smsc911x platform_driver probe function point of view it just have
>> resources and doesn't know if it's I/O memory is directly mapped or is
>> through a memory controller as the GPMC. It also doesn't know if the IRQ is
>> a GPIO or not.
> 
> It's still the same difference as far as the device is concerned.
> External bus chip-select lines are well understood. The key here is to
> encode the CS line number into the reg property of the child node so
> that the GPMC driver has the information it needs to configure the
> chip selects. You do this by setting #address-cells to '2' in the GPMC
> node, and  use the ranges property to map from the gpmc address space
> to the cpu address space. Like so (if you had 2 Ethernet controllers)
> 
>         gpmc {
>                 #address-cells = <2>;  // First cell is CS#, second
> cell is offset from CS base
>                 #size-cells = <1>;
>                 compatible = "ti,gpmc";
>                 ranges = <0 0 0xf1000000 0x1000>, //CS0 mapped to
> 0xf1000000..0xf1000fff
>                                 <1 0 0xf1001000 0x1000>; //CS1 mapped
> to 0xf1001000..0xf1001fff
> 
>                 ethernet@0,0 {
>                         compatible = "smsc,91c111";
>                         reg = <0 0 0x1000>;
>                 }
>                 ethernet@1,0 {
>                         compatible = "smsc,91c111";
>                         reg = <1 0 0x1000>;
>                }
>       }
> 
> The GPMC driver can use the information in the ranges property for
> setting up the chip select mappings. For the smsc,91c111 driver the
> mapping becomes transparent.
> 
> You can see another example of this in
> arch/powerpc/boot/dts/media5200.dts in the localbus node.
> 
> g.
> 

Hello Grant,

Thanks a lot for your explanation. Now is very clear to me how this has to be
done. I'll work on an v2 of this patch-set that do it correctly and will resend.

Best regards,
Javier

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [PATCH RFC 1/7] platform: add a device node
@ 2013-02-18 13:56               ` Javier Martinez Canillas
  0 siblings, 0 replies; 46+ messages in thread
From: Javier Martinez Canillas @ 2013-02-18 13:56 UTC (permalink / raw)
  To: linux-arm-kernel

On 02/18/2013 02:51 PM, Grant Likely wrote:
> On Sun, Feb 10, 2013 at 11:35 AM, Javier Martinez Canillas
> <martinez.javier@gmail.com> wrote:
>> On Sun, Feb 10, 2013 at 10:37 AM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>>> On Sun, Feb 10, 2013 at 02:49:21AM +0100, Javier Martinez Canillas wrote:
>>>> I knew this would be controversial and that's why I didn't mean it to be a patch
>>>> but a RFC :)
>>>>
>>>> The problem basically is that you have to associate the platform device with its
>>>> corresponding DT device node because it can be used in the driver probe function.
>>>
>>> When DT is being used, doesn't DT create the platform devices for you,
>>> with the device node already set correctly?
>>>
>>
>> Well they usually do but not always just like usually you have a
>> platform_device in your board code and call platform_device_register().
>>
>> But in some configurations you can't just define the platform_device
>> required resources (I/O memory, IRQ, etc) as static platform data.
>> In some cases you need a level of indirection.
>>
>> In this particular case a SMSC ethernet chip is connected to an
>> OMAP3 processor through its General-Purpose Memory Controller.
>>
>> You can't just define the I/O memory used by the device since you first
>> need to request that address to the GPMC. The same happens with the
>> IRQ line since a OMAP GPIO pin is used so you have to first configure
>> the GPIO line as input.
>>
>> So in platform code you call to gpmc_smsc911x_init() passing all the
>> GPMC specific configurations (GPIO used for IRQ, GPMC chip select, etc)
>>
>> Then gpmc_smsc911x_init() does all the needed setup, fills a platform_data
>> for the real platform_device and calls  platform_device_register_resndata().
>>
>> From the smsc911x platform_driver probe function point of view it just have
>> resources and doesn't know if it's I/O memory is directly mapped or is
>> through a memory controller as the GPMC. It also doesn't know if the IRQ is
>> a GPIO or not.
> 
> It's still the same difference as far as the device is concerned.
> External bus chip-select lines are well understood. The key here is to
> encode the CS line number into the reg property of the child node so
> that the GPMC driver has the information it needs to configure the
> chip selects. You do this by setting #address-cells to '2' in the GPMC
> node, and  use the ranges property to map from the gpmc address space
> to the cpu address space. Like so (if you had 2 Ethernet controllers)
> 
>         gpmc {
>                 #address-cells = <2>;  // First cell is CS#, second
> cell is offset from CS base
>                 #size-cells = <1>;
>                 compatible = "ti,gpmc";
>                 ranges = <0 0 0xf1000000 0x1000>, //CS0 mapped to
> 0xf1000000..0xf1000fff
>                                 <1 0 0xf1001000 0x1000>; //CS1 mapped
> to 0xf1001000..0xf1001fff
> 
>                 ethernet at 0,0 {
>                         compatible = "smsc,91c111";
>                         reg = <0 0 0x1000>;
>                 }
>                 ethernet at 1,0 {
>                         compatible = "smsc,91c111";
>                         reg = <1 0 0x1000>;
>                }
>       }
> 
> The GPMC driver can use the information in the ranges property for
> setting up the chip select mappings. For the smsc,91c111 driver the
> mapping becomes transparent.
> 
> You can see another example of this in
> arch/powerpc/boot/dts/media5200.dts in the localbus node.
> 
> g.
> 

Hello Grant,

Thanks a lot for your explanation. Now is very clear to me how this has to be
done. I'll work on an v2 of this patch-set that do it correctly and will resend.

Best regards,
Javier

^ permalink raw reply	[flat|nested] 46+ messages in thread

end of thread, other threads:[~2013-02-18 13:56 UTC | newest]

Thread overview: 46+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-09 20:44 [PATCH RFC 0/7] ARM: OMAP: add DT binding for gpmc-smsc911x Javier Martinez Canillas
2013-02-09 20:44 ` Javier Martinez Canillas
2013-02-09 20:44 ` [PATCH RFC 1/7] platform: add a device node Javier Martinez Canillas
2013-02-09 20:44   ` Javier Martinez Canillas
2013-02-10  1:02   ` Greg Kroah-Hartman
2013-02-10  1:02     ` Greg Kroah-Hartman
2013-02-10  1:49     ` Javier Martinez Canillas
2013-02-10  1:49       ` Javier Martinez Canillas
2013-02-10  9:37       ` Russell King - ARM Linux
2013-02-10  9:37         ` Russell King - ARM Linux
2013-02-10 11:35         ` Javier Martinez Canillas
2013-02-10 11:35           ` Javier Martinez Canillas
2013-02-11  8:16           ` Sascha Hauer
2013-02-11  8:16             ` Sascha Hauer
2013-02-11 10:33             ` Javier Martinez Canillas
2013-02-11 10:33               ` Javier Martinez Canillas
2013-02-11 11:24               ` Sascha Hauer
2013-02-11 11:24                 ` Sascha Hauer
2013-02-11 11:38                 ` Javier Martinez Canillas
2013-02-11 11:38                   ` Javier Martinez Canillas
2013-02-18 13:51           ` Grant Likely
2013-02-18 13:51             ` Grant Likely
2013-02-18 13:56             ` Javier Martinez Canillas
2013-02-18 13:56               ` Javier Martinez Canillas
2013-02-18 13:33         ` Grant Likely
2013-02-18 13:33           ` Grant Likely
2013-02-09 20:44 ` [PATCH RFC 2/7] net: smsc911x: add pinctrl support Javier Martinez Canillas
2013-02-09 20:44   ` Javier Martinez Canillas
2013-02-11 14:23   ` Linus Walleij
2013-02-11 14:23     ` Linus Walleij
2013-02-11 14:29     ` Javier Martinez Canillas
2013-02-11 14:29       ` Javier Martinez Canillas
2013-02-09 20:44 ` [PATCH RFC 3/7] ARM: OMAP: gpmc-smsc911x: add DT dev node init function Javier Martinez Canillas
2013-02-09 20:44   ` Javier Martinez Canillas
2013-02-11 10:30   ` Mark Rutland
2013-02-11 10:30     ` Mark Rutland
2013-02-11 10:40     ` Javier Martinez Canillas
2013-02-11 10:40       ` Javier Martinez Canillas
2013-02-09 20:44 ` [PATCH RFC 4/7] ARM: OMAP: gpmc-smsc911x: pass a dev node to platform registration Javier Martinez Canillas
2013-02-09 20:44   ` Javier Martinez Canillas
2013-02-09 20:44 ` [PATCH RFC 5/7] ARM: OMAP: gpmc: add support for gpmc-smsc911x child nodes Javier Martinez Canillas
2013-02-09 20:44   ` Javier Martinez Canillas
2013-02-09 20:44 ` [PATCH RFC 6/7] ARM: dts: OMAP: Add an GPMC node for OMAP3 Javier Martinez Canillas
2013-02-09 20:44   ` Javier Martinez Canillas
2013-02-09 20:44 ` [PATCH RFC 7/7] ARM: dts: omap3-igep0020: Add SMSC911x LAN chip support Javier Martinez Canillas
2013-02-09 20:44   ` Javier Martinez Canillas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.