| Message ID | 1397440946-2303-1-git-send-email-shawn.guo@freescale.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@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>
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@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.
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@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
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
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. > >
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
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
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
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.
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.
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.
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
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.
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);
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@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(+)