[PATCH v2] staging: imx-drm-core: skip components whose parent device is disabled

[PATCH v2] staging: imx-drm-core: skip components whose parent device is disabled

[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.

Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
---
Changes since v1:
* Put a warning on such misconfiguration

 drivers/staging/imx-drm/imx-drm-core.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/staging/imx-drm/imx-drm-core.c b/drivers/staging/imx-drm/imx-drm-core.c
index 4144a75..0da8a8a 100644
--- a/drivers/staging/imx-drm/imx-drm-core.c
+++ b/drivers/staging/imx-drm/imx-drm-core.c
@@ -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);
-- 
1.8.3.2

[PATCH v2] staging: imx-drm-core: skip components whose parent device is disabled

[Philipp Zabel]

Am Montag, den 14.04.2014, 10:02 +0800 schrieb 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.
> 
> Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> ---
> Changes since v1:
> * Put a warning on such misconfiguration

Acked-by: Philipp Zabel <p.zabel@pengutronix.de>

[PATCH v2] staging: imx-drm-core: skip components whose parent device is disabled

[Russell King - ARM Linux]

On Mon, Apr 14, 2014 at 10:02:26AM +0800, Shawn Guo wrote:
> 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.
> 
> Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> ---
> Changes since v1:
> * Put a warning on such misconfiguration

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

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

[PATCH v2] staging: imx-drm-core: skip components whose parent device is disabled

[Shawn Guo]

On Fri, Apr 18, 2014 at 09:42:49PM +0100, Russell King - ARM Linux wrote:
> On Mon, Apr 14, 2014 at 10:02:26AM +0800, Shawn Guo wrote:
> > 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.
> > 
> > Signed-off-by: Shawn Guo <shawn.guo@freescale.com>
> > ---
> > 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

[PATCH v2] staging: imx-drm-core: skip components whose parent device is disabled

[Russell King - ARM Linux]

On Sat, Apr 19, 2014 at 01:53:08PM +0800, Shawn Guo wrote:
> 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.

> 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

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

[PATCH v2] staging: imx-drm-core: skip components whose parent device is disabled

[Shawn Guo]

On Sat, Apr 19, 2014 at 09:42:29AM +0100, Russell King - ARM Linux wrote:
> On Sat, Apr 19, 2014 at 01:53:08PM +0800, Shawn Guo wrote:
> > 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.
> 
> > 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.

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

> 
> There's one which I posted yesterday which I think should be merged along
> with these - imx-drm: fix hdmi hotplug detection initial state
> 
> -- 
> FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
> improving, and getting towards what was expected from it.
> 
> 

[PATCH v2] staging: imx-drm-core: skip components whose parent device is disabled

[Olof Johansson]

On Sat, Apr 19, 2014 at 4:00 AM, Shawn Guo <shawn.guo@freescale.com> wrote:
> On Sat, Apr 19, 2014 at 09:42:29AM +0100, Russell King - ARM Linux wrote:
>> On Sat, Apr 19, 2014 at 01:53:08PM +0800, Shawn Guo wrote:
>> > 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.
>>
>> > 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.
>
> 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

[PATCH v2] staging: imx-drm-core: skip components whose parent device is disabled

[Shawn Guo]

On Mon, Apr 21, 2014 at 07:22:52AM -0700, Olof Johansson wrote:
> On Sat, Apr 19, 2014 at 4:00 AM, Shawn Guo <shawn.guo@freescale.com> wrote:
> > On Sat, Apr 19, 2014 at 09:42:29AM +0100, Russell King - ARM Linux wrote:
> >> On Sat, Apr 19, 2014 at 01:53:08PM +0800, Shawn Guo wrote:
> >> > 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.
> >>
> >> > 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.
> >
> > 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

[PATCH v2] staging: imx-drm-core: skip components whose parent device is disabled

[Arnd Bergmann]

On Tuesday 22 April 2014, Shawn Guo wrote:
> On Mon, Apr 21, 2014 at 07:22:52AM -0700, Olof Johansson wrote:
> > On Sat, Apr 19, 2014 at 4:00 AM, Shawn Guo <shawn.guo@freescale.com> wrote:
> > > On Sat, Apr 19, 2014 at 09:42:29AM +0100, Russell King - ARM Linux wrote:
> > >> On Sat, Apr 19, 2014 at 01:53:08PM +0800, Shawn Guo wrote:
> > >> > 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.
> > >>
> > >> > 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.
> > >
> > > 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.

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

[PATCH v2] staging: imx-drm-core: skip components whose parent device is disabled

[Russell King - ARM Linux]

On Tue, Apr 22, 2014 at 10:49:07PM +0200, Arnd Bergmann wrote:
> 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.

> - 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.

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

[PATCH v2] staging: imx-drm-core: skip components whose parent device is disabled

[Shawn Guo]

On Tue, Apr 22, 2014 at 10:55:06PM +0100, Russell King - ARM Linux wrote:
> 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

> 
> 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.

[PATCH v2] staging: imx-drm-core: skip components whose parent device is disabled

[Russell King - ARM Linux]

On Wed, Apr 23, 2014 at 03:44:53PM +0800, Shawn Guo wrote:
> On Tue, Apr 22, 2014 at 10:55:06PM +0100, Russell King - ARM Linux wrote:
> > 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].

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.

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

[PATCH v2] staging: imx-drm-core: skip components whose parent device is disabled

[Arnd Bergmann]

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):
> 
>    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.

> 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:

Acked-by: Arnd Bergmann <arnd@arndb.de>

One of should normally be enough really.

	Arnd

[PATCH v2] staging: imx-drm-core: skip components whose parent device is disabled

[Russell King - ARM Linux]

On Thu, Apr 24, 2014 at 11:36:42PM +0200, Arnd Bergmann wrote:
> 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):
> > 
> >    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.
> 
> > 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:
> 
> Acked-by: Arnd Bergmann <arnd@arndb.de>

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.

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