Tegra Harmony: Device Tree Initial Steps

RFC In review John Bonesio 7 participants 15 messages May 10, 2011 – May 11, 2011 2 messages not in the archive

Subsystems (6)

Device tree bindings Arm (32-bit) Arm sub-architectures Tegra architecture support +2 more subsystems

lore

Share & tools

lore

John Bonesio

author

2011-05-10 20:14 UTC #0 lore raw

cover Tegra Harmony: Device Tree Initial Steps

These patches are the first steps towards getting the board specific devices on the Tegra Harmony board to be discovered and initialized using only a device tree - rather than platform specific code. In particular, this patch set is getting ready to initialize i2c devices based solely on a device tree.

This patch series is presented at this time only for review and comments.

---

John Bonesio (2):
ARM: Tegra: Device Tree Support: Update how sdhci devices are initialized
ARM: Tegra: Device Tree Support: Add i2c devices

 arch/arm/boot/dts/tegra250.dtsi |   33 +++++++++++
 arch/arm/mach-tegra/Makefile    |    1 
 arch/arm/mach-tegra/board-dt.c  |  120 ++++++++++++++++-----------------------
 drivers/i2c/busses/i2c-tegra.c  |    4 +
 4 files changed, 87 insertions(+), 71 deletions(-)

Signature · 1 line

- John

John Bonesio

author

2011-05-10 20:14 UTC #1 lore raw

RFC 1/2 ARM: Tegra: Device Tree Support: Update how sdhci devices are initialized

This patch changes how sdhci devices are initialized so that a later patch can easily add support for i2c devies.

Signed-off-by John Bonesio <bones-s3s/[email protected]>

---

 arch/arm/mach-tegra/Makefile   |    1 
 arch/arm/mach-tegra/board-dt.c |  106 ++++++++++------------------------------
 2 files changed, 27 insertions(+), 80 deletions(-)

arch/arm/mach-tegra/Makefile +1 -0

index 842ec1a..54d2215 100644
@@ -31,6 +31,7 @@ obj-${CONFIG_MACH_SEABOARD}             += board-seaboard.o
 obj-${CONFIG_MACH_SEABOARD}             += board-seaboard-pinmux.o
 
 obj-${CONFIG_MACH_TEGRA_DT}             += board-dt.o
+obj-${CONFIG_MACH_TEGRA_DT}             += board-harmony-pinmux.o
 
 obj-${CONFIG_MACH_TRIMSLICE}            += board-trimslice.o
 obj-${CONFIG_MACH_TRIMSLICE}            += board-trimslice-pinmux.o

arch/arm/mach-tegra/board-dt.c +26 ~~-80~~

128 lines not on this page · raw message

Stephen Warren

2011-05-10 22:03 UTC #2 lore raw

RE: [RFC 1/2] ARM: Tegra: Device Tree Support: Update how sdhci devices are initialized

John Bonesio wrote at Tuesday, May 10, 2011 2:14 PM:

Quoted 19 lines

This patch changes how sdhci devices are initialized so that a later patch can easily add support for i2c devies.
Signed-off-by John Bonesio <bones-s3s/[email protected]>
arch/arm/mach-tegra/board-dt.c +9 -0
@@ -134,6 +57,25 @@ static __initdata struct tegra_clk_init_table
 tegra_dt_clk_init_table[] = {
 	{ NULL,		NULL,		0,		0},
 };
 
+static struct tegra_sdhci_platform_data sdhci_pdata1 = {
+	.cd_gpio        = -1,
+	.wp_gpio        = -1,
+	.power_gpio     = -1,
+};
+
+static struct tegra_sdhci_platform_data sdhci_pdata2 = {
+	.cd_gpio        = TEGRA_GPIO_SD2_CD,
+	.wp_gpio        = TEGRA_GPIO_SD2_WP,

arch/arm/mach-tegra/board-dt.c static __initdata struct tegra_clk_init_table · line 79

Quoted 10 lines

arch/arm/mach-tegra/board-dt.c +10 -0

+	.power_gpio     = TEGRA_GPIO_SD2_POWER,
+};
+
+static struct tegra_sdhci_platform_data sdhci_pdata4 = {
+	.cd_gpio        = TEGRA_GPIO_SD4_CD,
+	.wp_gpio        = TEGRA_GPIO_SD4_WP,
+	.power_gpio     = TEGRA_GPIO_SD4_POWER,
+	.is_8bit        = 1,
+};
+

So I understand why board-dt is itself registering the SDHCI platform devices itself; so that the pdev initialization path is identical between fully hard-coded boards and board-dt.c.

However, I'm surprised that we would include the platform data for those devices within that argument; if we do that, we'll end up needing different platform data for every board, and devicetree won't have bought us anything.

In other words, I understood that board-dt would register all the platform devices within Tegra, but not set up any of the plataform data, and the device drivers for those devices would be updated to get their platform data from devicetree if available, then fall back to old-style platform data. In other words, the way the SDHCI drivers work today in the devicetree/test branch.

If my understanding is correct, shouldn't the chunk above and the one below be removed?

Quoted 14 lines

arch/arm/mach-tegra/board-dt.c +0 -0

 static struct of_device_id tegra_dt_match_table[] __initdata = {
 	{ .compatible = "simple-bus", },

arch/arm/mach-tegra/board-dt.c +4 -0

{}

@@ -164,6 +106,10 @@ static void __init tegra_dt_init(void)
 
 	harmony_pinmux_init();
 
+	tegra_sdhci_device1.dev.platform_data = &sdhci_pdata1;
+	tegra_sdhci_device2.dev.platform_data = &sdhci_pdata2;
+	tegra_sdhci_device4.dev.platform_data = &sdhci_pdata4;
+
 	platform_add_devices(harmony_devices, ARRAY_SIZE(harmony_devices));
 
 	/*

Signature · 1 line

nvpublic

A message that is not in the archive (sent off-list or not yet received): 74CDBE0F657A3D45AFBB94109FB122FF04986AA06A-C7FfzLzN0UxDw2glCA4ptUEOCMrvLtNR@public.gmane.org

Olof Johansson

2011-05-10 22:06 UTC #4 lore raw

Re: [RFC 1/2] ARM: Tegra: Device Tree Support: Update how sdhci devices are initialized

Quoted 48 lines · Stephen Warren

John Bonesio wrote at Tuesday, May 10, 2011 2:14 PM:
Quoted 1 line
This patch changes how sdhci devices are initialized so that a later
patch can
Quoted 18 lines
easily add support for i2c devies.
Signed-off-by John Bonesio <bones-s3s/[email protected]>
arch/arm/mach-tegra/board-dt.c +9 -0
@@ -134,6 +57,25 @@ static __initdata struct tegra_clk_init_table
 tegra_dt_clk_init_table[] = {
      { NULL,         NULL,           0,              0},
 };
 
+static struct tegra_sdhci_platform_data sdhci_pdata1 = {
+     .cd_gpio        = -1,
+     .wp_gpio        = -1,
+     .power_gpio     = -1,
+};
+
+static struct tegra_sdhci_platform_data sdhci_pdata2 = {
+     .cd_gpio        = TEGRA_GPIO_SD2_CD,
+     .wp_gpio        = TEGRA_GPIO_SD2_WP,
arch/arm/mach-tegra/board-dt.c static __initdata struct tegra_clk_init_table · line 79
Quoted 10 lines
arch/arm/mach-tegra/board-dt.c +10 -0
+     .power_gpio     = TEGRA_GPIO_SD2_POWER,
+};
+
+static struct tegra_sdhci_platform_data sdhci_pdata4 = {
+     .cd_gpio        = TEGRA_GPIO_SD4_CD,
+     .wp_gpio        = TEGRA_GPIO_SD4_WP,
+     .power_gpio     = TEGRA_GPIO_SD4_POWER,
+     .is_8bit        = 1,
+};
+
So I understand why board-dt is itself registering the SDHCI platform devices itself; so that the pdev initialization path is identical between fully hard-coded boards and board-dt.c.
However, I'm surprised that we would include the platform data for those devices within that argument; if we do that, we'll end up needing different platform data for every board, and devicetree won't have bought us anything.
In other words, I understood that board-dt would register all the platform devices within Tegra, but not set up any of the plataform data, and the device drivers for those devices would be updated to get their platform data from devicetree if available, then fall back to old-style platform data. In other words, the way the SDHCI drivers work today in the devicetree/test branch.

Quoted 3 lines

If my understanding is correct, shouldn't the chunk above and the one below be removed?

That's if we cut everything over, I think. until then, this code should fill in the platform_device and register it. But the values should come from the device tree, and it should only register the controllers that are in the tree (and not marked as disabled).

-Olof

_______________________________________________
devicetree-discuss mailing list devicetree-discuss-uLR06cmDAlY/[email protected]
https://lists.ozlabs.org/listinfo/devicetree-discuss

Stephen Warren

2011-05-10 22:16 UTC #5 lore raw

RFC 1/2 ARM: Tegra: Device Tree Support: Update how sdhci devices are initialized

Olof Johansson wrote at Tuesday, May 10, 2011 4:07 PM:

Quoted 27 lines

Quoted 27 lines · Stephen Warren
John Bonesio wrote at Tuesday, May 10, 2011 2:14 PM:
arch/arm/mach-tegra/board-dt.c static __initdata struct tegra_clk_init_table tegra_dt_clk_init_table[] = { · line 65
Quoted 9 lines
This patch changes how sdhci devices are initialized so that a later patch can easily add support for i2c devies.
arch/arm/mach-tegra/board-dt.c +6 -0
+static struct tegra_sdhci_platform_data sdhci_pdata1 = {
+ ? ? .cd_gpio ? ? ? ?= -1,
+ ? ? .wp_gpio ? ? ? ?= -1,
+ ? ? .power_gpio ? ? = -1,
+};
+
So I understand why board-dt is itself registering the SDHCI platform devices itself; so that the pdev initialization path is identical between fully hard-coded boards and board-dt.c.
However, I'm surprised that we would include the platform data for those devices within that argument; if we do that, we'll end up needing different platform data for every board, and devicetree won't have bought us anything.
In other words, I understood that board-dt would register all the platform devices within Tegra, but not set up any of the plataform data, and the device drivers for those devices would be updated to get their platform data from devicetree if available, then fall back to old-style platform data. In other words, the way the SDHCI drivers work today in the devicetree/test branch.

Quoted 7 lines

Quoted 2 lines
If my understanding is correct, shouldn't the chunk above and the one below be removed?
That's if we cut everything over, I think. until then, this code should fill in the platform_device and register it. But the values should come from the device tree, and it should only register the controllers that are in the tree (and not marked as disabled).

But doesn't that require board-dt.c to walk and parse the devicetree, extract the platform data from the DT, and fill in the platform data structures? I thought we'd explicitly talked about that and decided that was a bad idea, since it'd duplicate each driver's DT parsing into board-dt.c?

Signature · 1 line

nvpublic

John Bonesio

2011-05-10 22:33 UTC #6 lore raw

RFC 1/2 ARM: Tegra: Device Tree Support: Update how sdhci devices are initialized

Quoted 36 lines · Stephen Warren

Olof Johansson wrote at Tuesday, May 10, 2011 4:07 PM:
Quoted 27 lines
Quoted 27 lines · Stephen Warren
John Bonesio wrote at Tuesday, May 10, 2011 2:14 PM:
arch/arm/mach-tegra/board-dt.c static __initdata struct tegra_clk_init_table tegra_dt_clk_init_table[] = { · line 65
Quoted 9 lines
This patch changes how sdhci devices are initialized so that a later patch can easily add support for i2c devies.
arch/arm/mach-tegra/board-dt.c +6 -0
+static struct tegra_sdhci_platform_data sdhci_pdata1 = {
+     .cd_gpio        = -1,
+     .wp_gpio        = -1,
+     .power_gpio     = -1,
+};
+
So I understand why board-dt is itself registering the SDHCI platform devices itself; so that the pdev initialization path is identical between fully hard-coded boards and board-dt.c.
However, I'm surprised that we would include the platform data for those devices within that argument; if we do that, we'll end up needing different platform data for every board, and devicetree won't have bought us anything.
In other words, I understood that board-dt would register all the platform devices within Tegra, but not set up any of the plataform data, and the device drivers for those devices would be updated to get their platform data from devicetree if available, then fall back to old-style platform data. In other words, the way the SDHCI drivers work today in the devicetree/test branch.
Quoted 7 lines
Quoted 2 lines
If my understanding is correct, shouldn't the chunk above and the one below be removed?
That's if we cut everything over, I think. until then, this code should fill in the platform_device and register it. But the values should come from the device tree, and it should only register the controllers that are in the tree (and not marked as disabled).

Quoted 6 lines

But doesn't that require board-dt.c to walk and parse the devicetree, extract the platform data from the DT, and fill in the platform data structures? I thought we'd explicitly talked about that and decided that was a bad idea, since it'd duplicate each driver's DT parsing into board-dt.c?

The goal I'm working towards right now is to have the platform specific code continue to register the devices that are inside the SoC. Then use the device tree to register all other on board devices.

This patch doesn't accomplish the whole goal in one step, but gets the code a little closer toward this goal.

To accomplish the goal, nodes need to be in the device tree for the devices inside the SoC. This is then used to attach device tree nodes to the existing devices - gives us a "hook" on which to "hang" the board specific devices. There is only one device tree used, so everything needs to be connected into the same device tree.

These patches just get the i2c controller and driver instantiated. This includes having the platform code to register the i2c controller in the "normal" fashion. This patch also gets a device tree node attached to the i2c device.

A following patch, which I'll send later this week/early next week, will instantiate the wm8903 audio codec device solely from the device tree.

Hopefully this makes the plan a little clearer. Let me know if you have any questions.

- John

Mark Brown

2011-05-10 22:42 UTC #7 lore raw

RFC 1/2 ARM: Tegra: Device Tree Support: Update how sdhci devices are initialized

Quoted 2 lines · John Bonesio

A following patch, which I'll send later this week/early next week, will instantiate the wm8903 audio codec device solely from the device tree.

Please CC me on this work - I've got an interest both from the chip and subsystem point of view in how this stuff ends up looking and there's some fun and games with audio in device tree.

Stephen Warren

2011-05-10 23:22 UTC #8 lore raw

RFC 1/2 ARM: Tegra: Device Tree Support: Update how sdhci devices are initialized

In reply to John Bonesio

John Bonesio wrote at Tuesday, May 10, 2011 4:33 PM:

Quoted 3 lines

The goal I'm working towards right now is to have the platform specific code continue to register the devices that are inside the SoC. Then use the device tree to register all other on board devices.

Sure, that sounds right.

Which branch are your patches aimed at? My discussions are all based on my experience with Grant's devicetree/test branch.

Quoted 2 lines

This patch doesn't accomplish the whole goal in one step, but gets the code a little closer toward this goal.

So what's confusing me is that I think the devicetree/test branch is already there; board-dt.c registers all 4 SDHCI controllers, tegra250.dtsi defines each controller's memory map etc., and disables them all, then tegra-harmony.dts and tegra-seaboard.dts enable the relevant subset of those controllers, and provides the required platform data for them.

When adding the I2C controllers, it seems like they'd just work the same way as the existing SDHCI code.

So, all this already works without putting the board-specific platform data definitions into board-dt.c as an temporary measure.

Signature · 1 line

nvpublic

John Bonesio

2011-05-10 23:33 UTC #9 lore raw

RFC 1/2 ARM: Tegra: Device Tree Support: Update how sdhci devices are initialized

Quoted 22 lines · Stephen Warren

John Bonesio wrote at Tuesday, May 10, 2011 4:33 PM:
Quoted 3 lines
The goal I'm working towards right now is to have the platform specific code continue to register the devices that are inside the SoC. Then use the device tree to register all other on board devices.
Sure, that sounds right.
Which branch are your patches aimed at? My discussions are all based on my experience with Grant's devicetree/test branch.
Quoted 2 lines
This patch doesn't accomplish the whole goal in one step, but gets the code a little closer toward this goal.
So what's confusing me is that I think the devicetree/test branch is already there; board-dt.c registers all 4 SDHCI controllers, tegra250.dtsi defines each controller's memory map etc., and disables them all, then tegra-harmony.dts and tegra-seaboard.dts enable the relevant subset of those controllers, and provides the required platform data for them.
When adding the I2C controllers, it seems like they'd just work the same way as the existing SDHCI code.

Stephen Warren

2011-05-11 05:01 UTC #10 lore raw

RFC 1/2 ARM: Tegra: Device Tree Support: Update how sdhci devices are initialized

John Bonesio wrote at Tuesday, May 10, 2011 5:33 PM:

Quoted 24 lines · Stephen Warren

Quoted 20 lines
John Bonesio wrote at Tuesday, May 10, 2011 4:33 PM:
Quoted 3 lines
The goal I'm working towards right now is to have the platform specific code continue to register the devices that are inside the SoC. Then use the device tree to register all other on board devices.
Sure, that sounds right.
Which branch are your patches aimed at? My discussions are all based on my experience with Grant's devicetree/test branch.
Quoted 2 lines
This patch doesn't accomplish the whole goal in one step, but gets the code a little closer toward this goal.
So what's confusing me is that I think the devicetree/test branch is already there; board-dt.c registers all 4 SDHCI controllers, tegra250.dtsi defines each controller's memory map etc., and disables them all, then tegra-harmony.dts and tegra-seaboard.dts enable the relevant subset of those controllers, and provides the required platform data for them.
Quoted 2 lines
When adding the I2C controllers, it seems like they'd just work the same way as the existing SDHCI code.

Quoted 7 lines

From a philosophical point of view, they should be initialized the same way.
Are you thinking that the i2c controller should be in the harmony_devices array and initialized with the other devices? If so i2c is different in the kernel. An i2c controller isn't registered with platform_device_register().

Um. Your patch (and all existing code I'm familiar with) *does* use platform_device_register for it:

+ platform_device_register(&tegra_i2c_device1);

But, I'm not talking about registering the platform devices, i.e. I'm not talking about the line of code quoted above. I'm talking about the platform data setup for those devices, i.e. the following from your SDHCI patch:

+static struct tegra_sdhci_platform_data sdhci_pdata1 = {

+	.cd_gpio        = -1,
+	.wp_gpio        = -1,
+	.power_gpio     = -1,

+};
...
+ tegra_sdhci_device1.dev.platform_data = &sdhci_pdata1;

The SDHCI driver has already been updated to get this platform data from the OF node in the platform device; it doesn't need hard-coded platform data to be passed to it.

Similarly, the following from your I2C patch shouldn't be required:

+static struct tegra_i2c_platform_data harmony_i2c1_platform_data = {
+ .bus_clk_rate = 400000,
+};
...
+ tegra_i2c_device1.dev.platform_data = &harmony_i2c1_platform_data;

Instead, the I2C driver should be modified to get its data from the OF node in the platform device just like the SDHCI driver does. This should be a pretty trivial amount of code to add to the I2C patch.

Quoted 6 lines

I may not be understanding your question.
Quoted 4 lines
So, all this already works without putting the board-specific platform data definitions into board-dt.c as an temporary measure.

Signature · 1 line

nvpublic

John Bonesio

2011-05-11 17:40 UTC #11 lore raw

RFC 1/2 ARM: Tegra: Device Tree Support: Update how sdhci devices are initialized

Quoted 28 lines · Stephen Warren

John Bonesio wrote at Tuesday, May 10, 2011 5:33 PM:
Quoted 27 lines · Stephen Warren
Quoted 20 lines
John Bonesio wrote at Tuesday, May 10, 2011 4:33 PM:
Quoted 3 lines
The goal I'm working towards right now is to have the platform specific code continue to register the devices that are inside the SoC. Then use the device tree to register all other on board devices.
Sure, that sounds right.
Which branch are your patches aimed at? My discussions are all based on my experience with Grant's devicetree/test branch.
Quoted 2 lines
This patch doesn't accomplish the whole goal in one step, but gets the code a little closer toward this goal.
So what's confusing me is that I think the devicetree/test branch is already there; board-dt.c registers all 4 SDHCI controllers, tegra250.dtsi defines each controller's memory map etc., and disables them all, then tegra-harmony.dts and tegra-seaboard.dts enable the relevant subset of those controllers, and provides the required platform data for them.
Quoted 2 lines
When adding the I2C controllers, it seems like they'd just work the same way as the existing SDHCI code.
From a philosophical point of view, they should be initialized the same way.

Quoted 7 lines

Quoted 4 lines
Are you thinking that the i2c controller should be in the harmony_devices array and initialized with the other devices? If so i2c is different in the kernel. An i2c controller isn't registered with platform_device_register().
Um. Your patch (and all existing code I'm familiar with) *does* use platform_device_register for it:

oops. Right. I was confusing this with the way i2c devices are registered. I copied the way i2c was initialized in board-harmony.c

Quoted 28 lines

(quoted diff) +1 -0
+	platform_device_register(&tegra_i2c_device1);
But, I'm not talking about registering the platform devices, i.e. I'm not talking about the line of code quoted above. I'm talking about the platform data setup for those devices, i.e. the following from your SDHCI patch:
(quoted diff) +6 -0
+static struct tegra_sdhci_platform_data sdhci_pdata1 = {
+	.cd_gpio        = -1,
+	.wp_gpio        = -1,
+	.power_gpio     = -1,
+};
...
+	tegra_sdhci_device1.dev.platform_data = &sdhci_pdata1;
The SDHCI driver has already been updated to get this platform data from the OF node in the platform device; it doesn't need hard-coded platform data to be passed to it.
Similarly, the following from your I2C patch shouldn't be required:
(quoted diff) +4 -0
+static struct tegra_i2c_platform_data harmony_i2c1_platform_data = {
+	.bus_clk_rate   = 400000,
+};
...
+	tegra_i2c_device1.dev.platform_data = &harmony_i2c1_platform_data;

Quoted 3 lines

Instead, the I2C driver should be modified to get its data from the OF node in the platform device just like the SDHCI driver does. This should be a pretty trivial amount of code to add to the I2C patch.

Grant and I have chatted about this. Our plan was to get to a intermediate step where all of the devices inside the SoC including the i2c controller (I believe the i2c controller itself is inside the SoC) continue to be initialized using the current static method (platform_device_register).

The implementation works like this ...

In tegra_dt_init(), of_platform_prepare() is called before devices are registered. of_platform_prepare() makes note of all the device tree nodes without registering anything.

When platform devices are registered (platform_device_register()), of_platform is notified and it attaches the device tree node to the device struct.

Then when all the SoC internal devices are done being registered using the static method, there remains a set of device tree nodes that haven't been registered as devices in the system. These device tree nodes should be primarily for devices on the board and not in the SoC.

At this point, of_platform_populate() is called, this registers the remaining devices in the device tree node so that drivers can be loaded for them using device tree methods.

The reason for this approach was that this step would make for minimal impact to existing systems. Yet it would still provide more flexibility for board specific devices.

The thought was that once this is in place we can collectively re-evaluate if we want to take the next step and initialize everything from the device tree.

Quoted 8 lines

Quoted 7 lines
I may not be understanding your question.
Quoted 4 lines
So, all this already works without putting the board-specific platform data definitions into board-dt.c as an temporary measure.

Grant Likely

2011-05-11 18:21 UTC #12 lore raw

RFC 1/2 ARM: Tegra: Device Tree Support: Update how sdhci devices are initialized

In reply to Stephen Warren

Quoted 15 lines · Stephen Warren

John Bonesio wrote at Tuesday, May 10, 2011 5:33 PM:
(quoted diff) +6 -0
+static struct tegra_sdhci_platform_data sdhci_pdata1 = {
+ ? ? ? .cd_gpio ? ? ? ?= -1,
+ ? ? ? .wp_gpio ? ? ? ?= -1,
+ ? ? ? .power_gpio ? ? = -1,
+};
...
+ ? ? ? tegra_sdhci_device1.dev.platform_data = &sdhci_pdata1;
The SDHCI driver has already been updated to get this platform data from the OF node in the platform device; it doesn't need hard-coded platform data to be passed to it.
Similarly, the following from your I2C patch shouldn't be required:

Review

Quoted 5 lines

(quoted diff) +4 -0

+static struct tegra_i2c_platform_data harmony_i2c1_platform_data = {
+ ? ? ? .bus_clk_rate ? = 400000,
+};
...
+ ? ? ? tegra_i2c_device1.dev.platform_data = &harmony_i2c1_platform_data;

Correct. John, you /should/ be able to simply drop the hunk in this patch that adds platform_data bits. All you're doing with this patch is dropping the local declaration of the platform_devices and using the ones defined in arch/arm/mach-tegra/devices.c. Setting the platform_data pointer is not required.

John Bonesio

author

2011-05-10 20:14 UTC #13 lore raw

RFC 2/2 ARM: Tegra: Device Tree Support: Add i2c devices

In reply to John Bonesio

This patch initializes i2c controller devices in board-dt.c. The i2c controller is added to tegra250.dtsi so later on-board i2c devices can be found and initialized based on the device tree information.

Signed-off-by John Bonesio <bones-s3s/[email protected]>

---

 arch/arm/boot/dts/tegra250.dtsi |   33 +++++++++++++++++++++++++++++++++
 arch/arm/mach-tegra/board-dt.c  |   32 ++++++++++++++++++++++++++++++++
 drivers/i2c/busses/i2c-tegra.c  |    4 ++++
 3 files changed, 69 insertions(+), 0 deletions(-)

arch/arm/boot/dts/tegra250.dtsi +33 -0

index f1801b8..7d8fc51 100644
@@ -20,6 +20,39 @@
 		};
 	};
 
+
+	i2c@7000C000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "nvidia,tegra250-i2c";
+		reg = <0x7000C000 0x100>;
+		interrupts = < 70 >;
+	};
+
+	i2c@7000C400 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "nvidia,tegra250-i2c";
+		reg = <0x7000C400 0x100>;
+		interrupts = < 116 >;
+	};
+
+	i2c@7000C500 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "nvidia,tegra250-i2c";
+		reg = <0x7000C500 0x100>;
+		interrupts = < 124 >;
+	};
+
+	i2c@7000D000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "nvidia,tegra250-i2c";
+		reg = <0x7000D000 0x200>;
+		interrupts = < 85 >;
+	};
+
 	gpio: gpio@6000d000 {
 		compatible = "nvidia,tegra250-gpio";
 		reg = < 0x6000d000 0x1000 >;

arch/arm/mach-tegra/board-dt.c +32 -0

index e61d563..cc7903b 100644
@@ -28,6 +28,8 @@
 #include <linux/of_platform.h>
 #include <linux/pda_power.h>
 #include <linux/io.h>
+#include <linux/i2c.h>
+#include <linux/i2c-tegra.h>
 
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>

@@ -44,6 +46,35 @@
 #include "devices.h"
 #include "gpio-names.h"
 
+static struct tegra_i2c_platform_data harmony_i2c1_platform_data = {
+	.bus_clk_rate   = 400000,
+};
+
+static struct tegra_i2c_platform_data harmony_i2c2_platform_data = {
+	.bus_clk_rate   = 400000,
+};
+
+static struct tegra_i2c_platform_data harmony_i2c3_platform_data = {
+	.bus_clk_rate   = 400000,
+};
+
+static struct tegra_i2c_platform_data harmony_dvc_platform_data = {
+	.bus_clk_rate   = 400000,
+};
+
+static void __init harmony_i2c_init(void)
+{
+	tegra_i2c_device1.dev.platform_data = &harmony_i2c1_platform_data;
+	tegra_i2c_device2.dev.platform_data = &harmony_i2c2_platform_data;
+	tegra_i2c_device3.dev.platform_data = &harmony_i2c3_platform_data;
+	tegra_i2c_device4.dev.platform_data = &harmony_dvc_platform_data;
+
+	platform_device_register(&tegra_i2c_device1);
+	platform_device_register(&tegra_i2c_device2);
+	platform_device_register(&tegra_i2c_device3);
+	platform_device_register(&tegra_i2c_device4);
+}
+
 static struct platform_device *harmony_devices[] __initdata = {
 	&tegra_sdhci_device1,
 	&tegra_sdhci_device2,

@@ -111,6 +142,7 @@ static void __init tegra_dt_init(void)
 	tegra_sdhci_device4.dev.platform_data = &sdhci_pdata4;
 
 	platform_add_devices(harmony_devices, ARRAY_SIZE(harmony_devices));
+	harmony_i2c_init();
 
 	/*
 	 * Finished with the static registrations now; fill in the missing

drivers/i2c/busses/i2c-tegra.c +4 -0

index b4ab39b..a2f3ec6 100644
@@ -26,6 +26,7 @@
 #include <linux/delay.h>
 #include <linux/slab.h>
 #include <linux/i2c-tegra.h>
+#include <linux/of_i2c.h>
 
 #include <asm/unaligned.h>

@@ -598,6 +599,7 @@ static int tegra_i2c_probe(struct platform_device *pdev)
 	i2c_dev->adapter.algo = &tegra_i2c_algo;
 	i2c_dev->adapter.dev.parent = &pdev->dev;
 	i2c_dev->adapter.nr = pdev->id;
+	i2c_dev->adapter.dev.of_node = of_node_get(pdev->dev.of_node);
 
 	ret = i2c_add_numbered_adapter(&i2c_dev->adapter);
 	if (ret) {

@@ -605,6 +607,8 @@ static int tegra_i2c_probe(struct platform_device *pdev)
 		goto err_free_irq;
 	}
 
+	of_i2c_register_devices(&i2c_dev->adapter);
+
 	return 0;
 err_free_irq:
 	free_irq(i2c_dev->irq, i2c_dev);

Stephen Warren

2011-05-10 22:10 UTC #14 lore raw

RE: [RFC 2/2] ARM: Tegra: Device Tree Support: Add i2c devices

John Bonesio wrote at Tuesday, May 10, 2011 2:15 PM:

Quoted 6 lines

This patch initializes i2c controller devices in board-dt.c. The i2c controller
is added to tegra250.dtsi so later on-board i2c devices can be found and initialized based on the device tree information.

arch/arm/boot/dts/tegra250.dtsi line 30

Quoted 9 lines

arch/arm/boot/dts/tegra250.dtsi +8 -0

 
+
+	i2c@7000C000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "nvidia,tegra250-i2c";
+		reg = <0x7000C000 0x100>;
+		interrupts = < 70 >;
+	};

Could we add 'status = "disabled"' for all of these in the core Tegra SoC file. Then, the board-specific files (e.g. tegra-harmony.dts) will set 'status = "ok"' for just the particular controllers that are actually used by the board. The SDHCI controllers are setup this way now per my recent patches.

arch/arm/mach-tegra/board-dt.c line 51

Quoted 5 lines

arch/arm/mach-tegra/board-dt.c +3 -0

 
+static struct tegra_i2c_platform_data harmony_i2c1_platform_data = {
+	.bus_clk_rate   = 400000,
+};

Similar to my comments on the SDHCI patch, shouldn't the platform data come only from devicetree?

Quoted 1 line

+static void __init harmony_i2c_init(void)

It'd be nice not to name stuff in board-dt.c after "harmony", since the idea is for it to work on arbitrary boards given the appropriate devicetree.

drivers/i2c/busses/i2c-tegra.c

Quoted 1 line

drivers/i2c/busses/i2c-tegra.c +0 -0

Since the driver and board files go to separate subsystems, shouldn't the changes to this file be a separate patch, to aid eventual upstreaming?

BTW, thanks for working on Tegra!

Signature · 1 line

nvpublic

A message that is not in the archive (sent off-list or not yet received): 74CDBE0F657A3D45AFBB94109FB122FF04986AA070-C7FfzLzN0UxDw2glCA4ptUEOCMrvLtNR@public.gmane.org

John Bonesio

author

2011-05-10 22:41 UTC #16 lore raw

Re: [RFC 2/2] ARM: Tegra: Device Tree Support: Add i2c devices

My comments are inline below.

- John

Quoted 8 lines · Stephen Warren

John Bonesio wrote at Tuesday, May 10, 2011 2:15 PM:
Quoted 6 lines
This patch initializes i2c controller devices in board-dt.c. The i2c controller
is added to tegra250.dtsi so later on-board i2c devices can be found and initialized based on the device tree information.
arch/arm/boot/dts/tegra250.dtsi +0 -0

Quoted 14 lines

arch/arm/boot/dts/tegra250.dtsi line 30
Quoted 8 lines
arch/arm/boot/dts/tegra250.dtsi +8 -0
+
+	i2c@7000C000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "nvidia,tegra250-i2c";
+		reg = <0x7000C000 0x100>;
+		interrupts = < 70 >;
+	};
Could we add 'status = "disabled"' for all of these in the core Tegra SoC file. Then, the board-specific files (e.g. tegra-harmony.dts) will set 'status = "ok"' for just the particular controllers that are actually used by the board. The SDHCI controllers are setup this way now per my recent patches.

Good idea.

Quoted 10 lines

arch/arm/mach-tegra/board-dt.c line 51
Quoted 6 lines
arch/arm/mach-tegra/board-dt.c +3 -0
 
+static struct tegra_i2c_platform_data harmony_i2c1_platform_data = {
+	.bus_clk_rate   = 400000,
+};
Similar to my comments on the SDHCI patch, shouldn't the platform data come only from devicetree?

Quoted 5 lines

Quoted 1 line
+static void __init harmony_i2c_init(void)
It'd be nice not to name stuff in board-dt.c after "harmony", since the idea is for it to work on arbitrary boards given the appropriate devicetree.

Yep. I'll fix this in the next revision.

Quoted 6 lines

drivers/i2c/busses/i2c-tegra.c
Quoted 2 lines
drivers/i2c/busses/i2c-tegra.c +0 -0
Since the driver and board files go to separate subsystems, shouldn't the changes to this file be a separate patch, to aid eventual upstreaming?

Maybe. Let's see what others think.

Quoted 3 lines

BTW, thanks for working on Tegra!

:-)