Skip to contentSkip to conversation

staging: imx-drm-core: skip components whose parent device is disabled

PATCH Superseded Shawn Guo 5 participants 14 messages Apr 14, 2014 – Apr 26, 2014

Subsystems (2)
lore
Share & tools
lore

Shawn Guo

author
#0 lore raw

PATCH v2 1/1 staging: imx-drm-core: skip components whose parent device is disabled

In a board setup which disables LDB device node completely by changing status to 'disabled', and only enables HDMI device, we're running into the problem that imx-drm master never succeeds in binding, and hence HDMI does not come up either.

&ldb {
	status = "disabled";

	lvds-channel at 1 {
		...
		status = "okay";
	};
};

The imx-drm-core should really skip the LVDS channels no matter what lvds-channel's status is, if LDB device is disabled. Let's consider such setup a misconfiguration, give a warning in there and not add the component.

---
Changes since v1:
* Put a warning on such misconfiguration

 drivers/staging/imx-drm/imx-drm-core.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/staging/imx-drm/imx-drm-core.c +5 -0
index 4144a75..0da8a8a 100644
@@ -675,6 +675,11 @@ static int imx_drm_platform_probe(struct platform_device *pdev)
 			if (!remote || !of_device_is_available(remote)) {
 				of_node_put(remote);
 				continue;
+			} else if (!of_device_is_available(remote->parent)) {
+				dev_warn(&pdev->dev, "parent device of %s is not available\n",
+					 remote->full_name);
+				of_node_put(remote);
+				continue;
 			}
 
 			ret = imx_drm_add_component(&pdev->dev, remote);
Signature · 1 line
1.8.3.2
#1 lore raw

PATCH v2 1/1 staging: imx-drm-core: skip components whose parent device is disabled

Am Montag, den 14.04.2014, 10:02 +0800 schrieb Shawn Guo:

Quoted 19 lines

In a board setup which disables LDB device node completely by changing status to 'disabled', and only enables HDMI device, we're running into the problem that imx-drm master never succeeds in binding, and hence HDMI does not come up either.

&ldb {
	status = "disabled";

	lvds-channel at 1 {
		...
		status = "okay";
	};
};

The imx-drm-core should really skip the LVDS channels no matter what lvds-channel's status is, if LDB device is disabled. Let's consider such setup a misconfiguration, give a warning in there and not add the component.

Quoted 4 lines

---
Changes since v1:
* Put a warning on such misconfiguration

#2 lore raw

PATCH v2 1/1 staging: imx-drm-core: skip components whose parent device is disabled

In reply to Shawn Guo
Quoted 19 lines · Shawn Guo

In a board setup which disables LDB device node completely by changing status to 'disabled', and only enables HDMI device, we're running into the problem that imx-drm master never succeeds in binding, and hence HDMI does not come up either.

&ldb {
	status = "disabled";

	lvds-channel at 1 {
		...
		status = "okay";
	};
};

The imx-drm-core should really skip the LVDS channels no matter what lvds-channel's status is, if LDB device is disabled. Let's consider such setup a misconfiguration, give a warning in there and not add the component.

Quoted 4 lines

---
Changes since v1:
* Put a warning on such misconfiguration

Thanks Shawn, I've added it to my queue for Greg.

Signature · 2 lines
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
author
#3 lore raw

PATCH v2 1/1 staging: imx-drm-core: skip components whose parent device is disabled

Quoted 20 lines · Russell King - ARM Linux
Quoted 19 lines · Shawn Guo

In a board setup which disables LDB device node completely by changing status to 'disabled', and only enables HDMI device, we're running into the problem that imx-drm master never succeeds in binding, and hence HDMI does not come up either.

&ldb {
	status = "disabled";

	lvds-channel at 1 {
		...
		status = "okay";
	};
};

The imx-drm-core should really skip the LVDS channels no matter what lvds-channel's status is, if LDB device is disabled. Let's consider such setup a misconfiguration, give a warning in there and not add the component.

Quoted 6 lines
Quoted 4 lines

---
Changes since v1:
* Put a warning on such misconfiguration

Thanks Shawn, I've added it to my queue for Greg.

Thanks, Russell.

BTW, can you take a look at the following two patches, or are they already in your queue?

imx-drm: imx-drm-core: Fix imx_drm_encoder_get_mux_id
https://lkml.org/lkml/2014/4/7/58

mx-drm: imx-tve: correct DDC property name to 'ddc-i2c-bus' ('i' is missing from the prefix word, and it should be 'imx-drm')
http://www.spinics.net/lists/arm-kernel/msg321057.html

Shawn

#4 lore raw

PATCH v2 1/1 staging: imx-drm-core: skip components whose parent device is disabled

Quoted 5 lines · Shawn Guo

BTW, can you take a look at the following two patches, or are they already in your queue?

imx-drm: imx-drm-core: Fix imx_drm_encoder_get_mux_id
https://lkml.org/lkml/2014/4/7/58

This one now is.

Quoted 3 lines

mx-drm: imx-tve: correct DDC property name to 'ddc-i2c-bus' ('i' is missing from the prefix word, and it should be 'imx-drm')
http://www.spinics.net/lists/arm-kernel/msg321057.html

This one touches arch/arm/boot/dts, which Arnd moaned at me for touching via the staging tree during the merge window. So, while I can take the imx-tve part, I can't take the arch/arm/boot/dts part. This patch needs to be split.

It also means that the MBA platform will have its TVE output broken while the two commits take their different paths, but I guess that's what you get for this kind of sillyness about git trees having exclusive ownership of various files in the kernel tree - more breakage not less.

There's one which I posted yesterday which I think should be merged along with these - imx-drm: fix hdmi hotplug detection initial state

Signature · 2 lines
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
author
#5 lore raw

PATCH v2 1/1 staging: imx-drm-core: skip components whose parent device is disabled

Quoted 18 lines · Russell King - ARM Linux
Quoted 5 lines · Shawn Guo

BTW, can you take a look at the following two patches, or are they already in your queue?

imx-drm: imx-drm-core: Fix imx_drm_encoder_get_mux_id
https://lkml.org/lkml/2014/4/7/58

This one now is.

Quoted 3 lines

mx-drm: imx-tve: correct DDC property name to 'ddc-i2c-bus' ('i' is missing from the prefix word, and it should be 'imx-drm')
http://www.spinics.net/lists/arm-kernel/msg321057.html

This one touches arch/arm/boot/dts, which Arnd moaned at me for touching via the staging tree during the merge window. So, while I can take the imx-tve part, I can't take the arch/arm/boot/dts part. This patch needs to be split.

Quoted 4 lines

It also means that the MBA platform will have its TVE output broken while the two commits take their different paths, but I guess that's what you get for this kind of sillyness about git trees having exclusive ownership of various files in the kernel tree - more breakage not less.

I think Arnd's main concern is about merge conflict, which shouldn't be a problem for this particular case. The change on dts file is pretty trivial. And I assume this is a fix to be landed on mainline before -rc5 or so. Then I can rebase my DT branch on top of it. That said, merge conflict is not really a problem for this patch. Arnd?

Shawn

Quoted 9 lines

There's one which I posted yesterday which I think should be merged along with these - imx-drm: fix hdmi hotplug detection initial state

Signature · 2 lines
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
#6 lore raw

PATCH v2 1/1 staging: imx-drm-core: skip components whose parent device is disabled

Quoted 24 lines · Shawn Guo
Quoted 18 lines · Russell King - ARM Linux
Quoted 5 lines · Shawn Guo

BTW, can you take a look at the following two patches, or are they already in your queue?

imx-drm: imx-drm-core: Fix imx_drm_encoder_get_mux_id
https://lkml.org/lkml/2014/4/7/58

This one now is.

Quoted 3 lines

mx-drm: imx-tve: correct DDC property name to 'ddc-i2c-bus' ('i' is missing from the prefix word, and it should be 'imx-drm')
http://www.spinics.net/lists/arm-kernel/msg321057.html

This one touches arch/arm/boot/dts, which Arnd moaned at me for touching via the staging tree during the merge window. So, while I can take the imx-tve part, I can't take the arch/arm/boot/dts part. This patch needs to be split.

Quoted 4 lines

It also means that the MBA platform will have its TVE output broken while the two commits take their different paths, but I guess that's what you get for this kind of sillyness about git trees having exclusive ownership of various files in the kernel tree - more breakage not less.

Quoted 5 lines

I think Arnd's main concern is about merge conflict, which shouldn't be a problem for this particular case. The change on dts file is pretty trivial. And I assume this is a fix to be landed on mainline before -rc5 or so. Then I can rebase my DT branch on top of it. That said, merge conflict is not really a problem for this patch. Arnd?

I'm not Arnd, but that should work well.

-Olof

author
#7 lore raw

PATCH v2 1/1 staging: imx-drm-core: skip components whose parent device is disabled

Quoted 25 lines · Olof Johansson
Quoted 24 lines · Shawn Guo
Quoted 22 lines · Russell King - ARM Linux
Quoted 5 lines · Shawn Guo

BTW, can you take a look at the following two patches, or are they already in your queue?

imx-drm: imx-drm-core: Fix imx_drm_encoder_get_mux_id
https://lkml.org/lkml/2014/4/7/58

This one now is.

Quoted 3 lines

mx-drm: imx-tve: correct DDC property name to 'ddc-i2c-bus' ('i' is missing from the prefix word, and it should be 'imx-drm')
http://www.spinics.net/lists/arm-kernel/msg321057.html

This one touches arch/arm/boot/dts, which Arnd moaned at me for touching via the staging tree during the merge window. So, while I can take the imx-tve part, I can't take the arch/arm/boot/dts part. This patch needs to be split.

It also means that the MBA platform will have its TVE output broken while the two commits take their different paths, but I guess that's what you get for this kind of sillyness about git trees having exclusive ownership of various files in the kernel tree - more breakage not less.

Quoted 7 lines
Quoted 5 lines

I think Arnd's main concern is about merge conflict, which shouldn't be a problem for this particular case. The change on dts file is pretty trivial. And I assume this is a fix to be landed on mainline before -rc5 or so. Then I can rebase my DT branch on top of it. That said, merge conflict is not really a problem for this patch. Arnd?

I'm not Arnd, but that should work well.

Thanks, Olof.

Shawn

#8 lore raw

PATCH v2 1/1 staging: imx-drm-core: skip components whose parent device is disabled

Quoted 32 lines · Shawn Guo
Quoted 31 lines · Olof Johansson
Quoted 29 lines · Shawn Guo
Quoted 18 lines · Russell King - ARM Linux
Quoted 5 lines · Shawn Guo

BTW, can you take a look at the following two patches, or are they already in your queue?

imx-drm: imx-drm-core: Fix imx_drm_encoder_get_mux_id
https://lkml.org/lkml/2014/4/7/58

This one now is.

Quoted 3 lines

mx-drm: imx-tve: correct DDC property name to 'ddc-i2c-bus' ('i' is missing from the prefix word, and it should be 'imx-drm')
http://www.spinics.net/lists/arm-kernel/msg321057.html

This one touches arch/arm/boot/dts, which Arnd moaned at me for touching via the staging tree during the merge window. So, while I can take the imx-tve part, I can't take the arch/arm/boot/dts part. This patch needs to be split.

Quoted 4 lines

It also means that the MBA platform will have its TVE output broken while the two commits take their different paths, but I guess that's what you get for this kind of sillyness about git trees having exclusive ownership of various files in the kernel tree - more breakage not less.

I think Arnd's main concern is about merge conflict, which shouldn't be a problem for this particular case. The change on dts file is pretty trivial. And I assume this is a fix to be landed on mainline before -rc5 or so. Then I can rebase my DT branch on top of it. That said, merge conflict is not really a problem for this patch. Arnd?

Quoted 3 lines
Quoted 1 line

I'm not Arnd, but that should work well.

Thanks, Olof.

Just to clarify, we generally want to avoid three things that happened with this driver during the merge window:

- incompatible DT binding changes
- known merge conflicts
- dts changes going through another tree (because they tend to cause merge conflicts)

However, none of these are hard rules, we can bend them for good reasons if it's clearly spelled out in the patch description and we are aware of the change (ideally with an explicit Ack).

Arnd

#9 lore raw

PATCH v2 1/1 staging: imx-drm-core: skip components whose parent device is disabled

Quoted 4 lines · Arnd Bergmann

Just to clarify, we generally want to avoid three things that happened with this driver during the merge window:

- incompatible DT binding changes

FFS, this is a _staging_ driver which hasn't had the bindings settled. They're _still_ not settled, because there's _still_ the discussion on the graph bindings to finally sort out and resolve. So I wouldn't be surprised if there are _more_ incompatible binding changes to come.

Meanwhile, we _have_ to keep the driver in a working state so that people can use and develop against it, and most importantly, resolve problems with the bindings.

Coming up with _decent_ bindings for subsystems like DRM is still very much a work in progress.

I guess we could keep this stuff 100% out of the tree, which blocks a whole load of people, forces people towards vendor kernels, or just forces people away from Linux because Linux is "too hard" and people are "too difficult" to work with, and it takes _far_ too long to get stuff into a working state.

How long does it take to make any kind of progress with a driver in a mainline kernel? One cycle? Two cycles? A year?

The reality is that it takes around _ONE_ year or longer _PER_ driver.

I wonder why people give up and just end up using vendor kernels... because they just can't be bothered with mainline.

Quoted 7 lines

- known merge conflicts
- dts changes going through another tree (because they tend to cause merge conflicts)

However, none of these are hard rules, we can bend them for good reasons if it's clearly spelled out in the patch description and we are aware of the change (ideally with an explicit Ack).

So care to explain why there were _zero_ merge conflicts from -rc time before the merge window up to about a week _into_ the merge window, and then suddenly arm-soc starts conflicting not only locally here but _also_ in linux-next.

That stinks of arm-soc merging new stuff *during* the merge window.

And don't try to deny that - remember, I run a build system here locally, and I _manually_ merge Linus' tip into my tip (which contains *everything* including the staged changes for imx-drm and those binding changes) _and_ finally arm-soc's for-next branch, and there were _zero_ conflicts at the start of the merge window.

Signature · 2 lines
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
author
#10 lore raw

PATCH v2 1/1 staging: imx-drm-core: skip components whose parent device is disabled

Quoted 6 lines · Russell King - ARM Linux

So care to explain why there were _zero_ merge conflicts from -rc time before the merge window up to about a week _into_ the merge window, and then suddenly arm-soc starts conflicting not only locally here but _also_ in linux-next.

That stinks of arm-soc merging new stuff *during* the merge window.

I'm not sure that's the case.

It seems to me that the merge conflict was reported by Stephen Rothwell one month ahead of the merge window [1].

Shawn

[1] https://lkml.org/lkml/2014/2/25/21

Quoted 6 lines

And don't try to deny that - remember, I run a build system here locally, and I _manually_ merge Linus' tip into my tip (which contains *everything* including the staged changes for imx-drm and those binding changes) _and_ finally arm-soc's for-next branch, and there were _zero_ conflicts at the start of the merge window.

#11 lore raw

PATCH v2 1/1 staging: imx-drm-core: skip components whose parent device is disabled

Quoted 6 lines · Shawn Guo
Quoted 5 lines · Russell King - ARM Linux

So care to explain why there were _zero_ merge conflicts from -rc time before the merge window up to about a week _into_ the merge window, and then suddenly arm-soc starts conflicting not only locally here but _also_ in linux-next.

Quoted 6 lines
Quoted 1 line

That stinks of arm-soc merging new stuff *during* the merge window.

I'm not sure that's the case.

It seems to me that the merge conflict was reported by Stephen Rothwell one month ahead of the merge window [1].

Yes, those ones were trivial to sort out, and as you point out were well known well before the merge window. Those aren't the ones I'm actually talking about - half way through the merge window, some major conflicts cropped up which caused _me_ to drop arm-soc out of my autobuilder, and caused Stephen to ignore arm-soc's version of the file(s):

Today's linux-next merge of the arm-soc tree got conflicts in arch/arm/boot/dts/qcom-msm8960.dtsi, arch/arm/boot/dts/qcom-msm8974.dtsi, arch/arm/mach-omap2/pdata-quirks.c, arch/arm/mach-zynq/Kconfig, drivers/watchdog/Kconfig and sound/soc/kirkwood/Kconfig between various merge commits from Linus' tree and various merge commits from the arm-soc tree.

I used the versions from Linus' tree.

In comparison to those, both Stephen and myself came up with the same trivial resolutions for the DT files. Moreover, as I've already said to Arnd, the reason these changes went via the staging tree is so that we could _keep_ imx-drm workable and testable for everyone - splitting the change up into the DT bits for arm-soc and the rest for the staging tree would have resulted in breakage.

So... whatever way I don't think arm-soc has a leg to stand on. They cocked up during the merge window.

What is really interesting is that there are no acks from arm-soc people for this patch. If they want me to take it, they can damned well provide an ack after having their moan at me during the merge window. And I'm not talking just about Arnd, but Olof as well. I'm going to require *both* to ack any changes to arch/arm/boot/dts in future so that I can be certain that they are *both* aware of what's going on.

Signature · 2 lines
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
#12 lore raw

PATCH v2 1/1 staging: imx-drm-core: skip components whose parent device is disabled

Quoted 7 lines · On Wednesday 23 April 2014 10:00:14 Russell King - ARM Linux wrote:

Yes, those ones were trivial to sort out, and as you point out were well known well before the merge window. Those aren't the ones I'm actually talking about - half way through the merge window, some major conflicts cropped up which caused _me_ to drop arm-soc out of my autobuilder, and caused Stephen to ignore arm-soc's version of the file(s):

Quoted 9 lines

Today's linux-next merge of the arm-soc tree got conflicts in arch/arm/boot/dts/qcom-msm8960.dtsi, arch/arm/boot/dts/qcom-msm8974.dtsi, arch/arm/mach-omap2/pdata-quirks.c, arch/arm/mach-zynq/Kconfig, drivers/watchdog/Kconfig and sound/soc/kirkwood/Kconfig between various merge commits from Linus' tree and various merge commits from the arm-soc tree.

I used the versions from Linus' tree.

That was the day after Linus had merged arm-soc and used a different set of resolutions than what I had put into for-next as examples for him to look at.

The conflicts that Stephen saw were between Linus' resolution and mine, but at that point there were no more unmerged patches in arm-soc, just stale merge changesets.

Quoted 6 lines

What is really interesting is that there are no acks from arm-soc people for this patch. If they want me to take it, they can damned well provide an ack after having their moan at me during the merge window. And I'm not talking just about Arnd, but Olof as well. I'm going to require *both* to ack any changes to arch/arm/boot/dts in future so that I can be certain that they are *both* aware of what's going on.

Olof already gave an (informal) Ack. Here is mine as well:

One of should normally be enough really.

Arnd

#13 lore raw

PATCH v2 1/1 staging: imx-drm-core: skip components whose parent device is disabled

Quoted 33 lines · Arnd Bergmann
Quoted 7 lines · On Wednesday 23 April 2014 10:00:14 Russell King - ARM Linux wrote:

Yes, those ones were trivial to sort out, and as you point out were well known well before the merge window. Those aren't the ones I'm actually talking about - half way through the merge window, some major conflicts cropped up which caused _me_ to drop arm-soc out of my autobuilder, and caused Stephen to ignore arm-soc's version of the file(s):

Quoted 9 lines

Today's linux-next merge of the arm-soc tree got conflicts in arch/arm/boot/dts/qcom-msm8960.dtsi, arch/arm/boot/dts/qcom-msm8974.dtsi, arch/arm/mach-omap2/pdata-quirks.c, arch/arm/mach-zynq/Kconfig, drivers/watchdog/Kconfig and sound/soc/kirkwood/Kconfig between various merge commits from Linus' tree and various merge commits from the arm-soc tree.

I used the versions from Linus' tree.

That was the day after Linus had merged arm-soc and used a different set of resolutions than what I had put into for-next as examples for him to look at.

The conflicts that Stephen saw were between Linus' resolution and mine, but at that point there were no more unmerged patches in arm-soc, just stale merge changesets.

Quoted 6 lines

What is really interesting is that there are no acks from arm-soc people for this patch. If they want me to take it, they can damned well provide an ack after having their moan at me during the merge window. And I'm not talking just about Arnd, but Olof as well. I'm going to require *both* to ack any changes to arch/arm/boot/dts in future so that I can be certain that they are *both* aware of what's going on.

Quoted 3 lines

Olof already gave an (informal) Ack. Here is mine as well:

I'll take it with your ack now, but I'm not adding an ack for Olof given the vagueness of his reply in this thread. To do so would be to invite accusations of adding acks where none was intended, and I decided right from the beginning of these attributations that they *must* be explicit to avoid any possibility of confusion and ambiguity (and I said as much.)

"informal" acks are not acks.

Signature · 2 lines
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.