[U-Boot] [PATCH 1/2] dm: core: allow drivers to refuse to bind
From: Stephen Warren swarren@nvidia.com
In some cases, drivers may not want to bind to a device. Allow bind() to return -ENODEV in this case, and don't treat this as an error. This can be useful in situations where some information source other than the DT node's main status property indicates whether the device should be enabled, for example other DT properties might indicate this, or the driver might query non-DT sources such as system fuses or a version number register.
Signed-off-by: Stephen Warren swarren@nvidia.com --- drivers/core/lists.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/core/lists.c b/drivers/core/lists.c index c4fc216340d8..a72db13a119a 100644 --- a/drivers/core/lists.c +++ b/drivers/core/lists.c @@ -171,6 +171,10 @@ int lists_bind_fdt(struct udevice *parent, const void *blob, int offset,
dm_dbg(" - found match at '%s'\n", entry->name); ret = device_bind(parent, entry, name, NULL, offset, &dev); + if (ret == -ENODEV) { + dm_dbg("Driver '%s' refuses to bind\n", entry->name); + continue; + } if (ret) { dm_warn("Error binding driver '%s': %d\n", entry->name, ret);
From: Stephen Warren swarren@nvidia.com
This prevents the following boot-time message on any board where only the first DC is in use, yet the DC's DT node is enabled:
stdio_add_devices: Video device failed (ret=-22)
(This happens on at least Harmony, Ventana, and likely any other Tegra20 board with display enabled other than Seaboard).
The Tegra DC's DT node represents a display controller. It may itself drive an integrated RGB display output, or be used by some other display controller such as HDMI. For this reason the DC node itself is not enabled/disabled in DT; the DC itself is considered a shared resource, not the final (board-specific) display output. The node should instantiate a display output driver only if the rgb subnode is enabled. Other output drivers are free to use the DC if they are enabled and their DT node references the DC's DT node. Adapt the Tegra display drivers' bind() routine to only bind to the DC's DT node if the RGB subnode is enabled.
Now that the display driver does the right thing, remove the workaround for this issue from Seaboard's DT file.
Cc: Thierry Reding treding@nvidia.com Signed-off-by: Stephen Warren swarren@nvidia.com --- Thierry, I assume this is how the DC nodes are intended to be interpreted? It's certainly how the kernel's DT files and driver work, even if by accident. --- arch/arm/dts/tegra20-seaboard.dts | 4 ---- drivers/video/tegra.c | 7 +++++++ 2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/arch/arm/dts/tegra20-seaboard.dts b/arch/arm/dts/tegra20-seaboard.dts index eada59073efc..5893d2a3f84e 100644 --- a/arch/arm/dts/tegra20-seaboard.dts +++ b/arch/arm/dts/tegra20-seaboard.dts @@ -40,10 +40,6 @@ nvidia,panel = <&lcd_panel>; }; }; - - dc@54240000 { - status = "disabled"; - }; };
/* This is not used in U-Boot, but is expected to be in kernel .dts */ diff --git a/drivers/video/tegra.c b/drivers/video/tegra.c index 7fd10e6af35e..c01809e89e14 100644 --- a/drivers/video/tegra.c +++ b/drivers/video/tegra.c @@ -620,6 +620,13 @@ static int tegra_lcd_ofdata_to_platdata(struct udevice *dev) static int tegra_lcd_bind(struct udevice *dev) { struct video_uc_platdata *plat = dev_get_uclass_platdata(dev); + const void *blob = gd->fdt_blob; + int node = dev->of_offset; + int rgb; + + rgb = fdt_subnode_offset(blob, node, "rgb"); + if ((rgb < 0) || !fdtdec_get_is_enabled(blob, rgb)) + return -ENODEV;
plat->size = LCD_MAX_WIDTH * LCD_MAX_HEIGHT * (1 << LCD_MAX_LOG2_BPP) / 8;
On Tue, Apr 19, 2016 at 04:19:30PM -0600, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
This prevents the following boot-time message on any board where only the first DC is in use, yet the DC's DT node is enabled:
stdio_add_devices: Video device failed (ret=-22)
(This happens on at least Harmony, Ventana, and likely any other Tegra20 board with display enabled other than Seaboard).
The Tegra DC's DT node represents a display controller. It may itself drive an integrated RGB display output, or be used by some other display controller such as HDMI. For this reason the DC node itself is not enabled/disabled in DT; the DC itself is considered a shared resource, not the final (board-specific) display output. The node should instantiate a display output driver only if the rgb subnode is enabled. Other output drivers are free to use the DC if they are enabled and their DT node references the DC's DT node. Adapt the Tegra display drivers' bind() routine to only bind to the DC's DT node if the RGB subnode is enabled.
Now that the display driver does the right thing, remove the workaround for this issue from Seaboard's DT file.
Cc: Thierry Reding treding@nvidia.com Signed-off-by: Stephen Warren swarren@nvidia.com
Thierry, I assume this is how the DC nodes are intended to be interpreted? It's certainly how the kernel's DT files and driver work, even if by accident.
No, it's entirely intentional. The rationale is, as you already hinted at in the commit message, that the display controllers don't define an active display output, but rather display outputs (such as RGB) will request a display controller for use in building the display pipeline.
RGB is a bit of a special case because it's tied to the respective display controllers (their registers are part of the DC register space) whereas the other types of output (HDMI, DSI, eDP, ...) can typically take pixel data from either of the display controllers.
arch/arm/dts/tegra20-seaboard.dts | 4 ---- drivers/video/tegra.c | 7 +++++++ 2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/arch/arm/dts/tegra20-seaboard.dts b/arch/arm/dts/tegra20-seaboard.dts index eada59073efc..5893d2a3f84e 100644 --- a/arch/arm/dts/tegra20-seaboard.dts +++ b/arch/arm/dts/tegra20-seaboard.dts @@ -40,10 +40,6 @@ nvidia,panel = <&lcd_panel>; }; };
dc@54240000 {status = "disabled";};};
/* This is not used in U-Boot, but is expected to be in kernel .dts */
diff --git a/drivers/video/tegra.c b/drivers/video/tegra.c index 7fd10e6af35e..c01809e89e14 100644 --- a/drivers/video/tegra.c +++ b/drivers/video/tegra.c @@ -620,6 +620,13 @@ static int tegra_lcd_ofdata_to_platdata(struct udevice *dev) static int tegra_lcd_bind(struct udevice *dev) { struct video_uc_platdata *plat = dev_get_uclass_platdata(dev);
- const void *blob = gd->fdt_blob;
- int node = dev->of_offset;
- int rgb;
- rgb = fdt_subnode_offset(blob, node, "rgb");
- if ((rgb < 0) || !fdtdec_get_is_enabled(blob, rgb))
return -ENODEV;
I'm not intimately familiar with the structure of the display driver in U-Boot, but it seems like we'd need to conditionalize this depending on what output the display pipeline uses. I suppose it would be up to its driver to register a stdio device, while making use of some library code to configure the display controller. My understanding was from looking at this a long time ago, that U-Boot only supports RGB output on Tegra20 through Tegra114, and there's a completely separate driver that supports eDP on Tegra124 (Nyan Big I believe), and presumably that will handle this entirely differently.
Anyway, this looks like the right fix for RGB outputs (such as on Seaboard), so:
Acked-by: Thierry Reding treding@nvidia.com
On 19 April 2016 at 16:19, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
This prevents the following boot-time message on any board where only the first DC is in use, yet the DC's DT node is enabled:
stdio_add_devices: Video device failed (ret=-22)
(This happens on at least Harmony, Ventana, and likely any other Tegra20 board with display enabled other than Seaboard).
The Tegra DC's DT node represents a display controller. It may itself drive an integrated RGB display output, or be used by some other display controller such as HDMI. For this reason the DC node itself is not enabled/disabled in DT; the DC itself is considered a shared resource, not the final (board-specific) display output. The node should instantiate a display output driver only if the rgb subnode is enabled. Other output drivers are free to use the DC if they are enabled and their DT node references the DC's DT node. Adapt the Tegra display drivers' bind() routine to only bind to the DC's DT node if the RGB subnode is enabled.
Now that the display driver does the right thing, remove the workaround for this issue from Seaboard's DT file.
Cc: Thierry Reding treding@nvidia.com Signed-off-by: Stephen Warren swarren@nvidia.com
Thierry, I assume this is how the DC nodes are intended to be interpreted? It's certainly how the kernel's DT files and driver work, even if by accident.
arch/arm/dts/tegra20-seaboard.dts | 4 ---- drivers/video/tegra.c | 7 +++++++ 2 files changed, 7 insertions(+), 4 deletions(-)
Well it seems reasonable to me.
Reviewed-by: Simon Glass sjg@chromium.org
On 20 April 2016 at 07:16, Simon Glass sjg@chromium.org wrote:
On 19 April 2016 at 16:19, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
This prevents the following boot-time message on any board where only the first DC is in use, yet the DC's DT node is enabled:
stdio_add_devices: Video device failed (ret=-22)
(This happens on at least Harmony, Ventana, and likely any other Tegra20 board with display enabled other than Seaboard).
The Tegra DC's DT node represents a display controller. It may itself drive an integrated RGB display output, or be used by some other display controller such as HDMI. For this reason the DC node itself is not enabled/disabled in DT; the DC itself is considered a shared resource, not the final (board-specific) display output. The node should instantiate a display output driver only if the rgb subnode is enabled. Other output drivers are free to use the DC if they are enabled and their DT node references the DC's DT node. Adapt the Tegra display drivers' bind() routine to only bind to the DC's DT node if the RGB subnode is enabled.
Now that the display driver does the right thing, remove the workaround for this issue from Seaboard's DT file.
Cc: Thierry Reding treding@nvidia.com Signed-off-by: Stephen Warren swarren@nvidia.com
Thierry, I assume this is how the DC nodes are intended to be interpreted? It's certainly how the kernel's DT files and driver work, even if by accident.
arch/arm/dts/tegra20-seaboard.dts | 4 ---- drivers/video/tegra.c | 7 +++++++ 2 files changed, 7 insertions(+), 4 deletions(-)
Well it seems reasonable to me.
Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot-dm, thanks!
On 19 April 2016 at 16:19, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
In some cases, drivers may not want to bind to a device. Allow bind() to return -ENODEV in this case, and don't treat this as an error. This can be useful in situations where some information source other than the DT node's main status property indicates whether the device should be enabled, for example other DT properties might indicate this, or the driver might query non-DT sources such as system fuses or a version number register.
Signed-off-by: Stephen Warren swarren@nvidia.com
drivers/core/lists.c | 4 ++++ 1 file changed, 4 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
On 04/19/2016 04:19 PM, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
In some cases, drivers may not want to bind to a device. Allow bind() to return -ENODEV in this case, and don't treat this as an error. This can be useful in situations where some information source other than the DT node's main status property indicates whether the device should be enabled, for example other DT properties might indicate this, or the driver might query non-DT sources such as system fuses or a version number register.
Simon, this series is assigned to you in patchwork. Are you the right person to apply it?
Hi Stephen,
On 4 May 2016 at 12:57, Stephen Warren swarren@wwwdotorg.org wrote:
On 04/19/2016 04:19 PM, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
In some cases, drivers may not want to bind to a device. Allow bind() to return -ENODEV in this case, and don't treat this as an error. This can be useful in situations where some information source other than the DT node's main status property indicates whether the device should be enabled, for example other DT properties might indicate this, or the driver might query non-DT sources such as system fuses or a version number register.
Simon, this series is assigned to you in patchwork. Are you the right person to apply it?
Yes. but not for this release, right?
Regards, Simon
On 05/04/2016 01:31 PM, Simon Glass wrote:
Hi Stephen,
On 4 May 2016 at 12:57, Stephen Warren swarren@wwwdotorg.org wrote:
On 04/19/2016 04:19 PM, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
In some cases, drivers may not want to bind to a device. Allow bind() to return -ENODEV in this case, and don't treat this as an error. This can be useful in situations where some information source other than the DT node's main status property indicates whether the device should be enabled, for example other DT properties might indicate this, or the driver might query non-DT sources such as system fuses or a version number register.
Simon, this series is assigned to you in patchwork. Are you the right person to apply it?
Yes. but not for this release, right?
Patch 2 in the series (which depends on this patch) fixes a bug for Tegra boards with LCD panels. Admittedly it appears to be only cosmetic (an error message is printed at boot), but "it's a bug" seems to satisfy the requirement to apply it for this release.
+Tom Rini
Hi Stephen,
On 4 May 2016 at 13:46, Stephen Warren swarren@wwwdotorg.org wrote:
On 05/04/2016 01:31 PM, Simon Glass wrote:
Hi Stephen,
On 4 May 2016 at 12:57, Stephen Warren swarren@wwwdotorg.org wrote:
On 04/19/2016 04:19 PM, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
In some cases, drivers may not want to bind to a device. Allow bind() to return -ENODEV in this case, and don't treat this as an error. This can be useful in situations where some information source other than the DT node's main status property indicates whether the device should be enabled, for example other DT properties might indicate this, or the driver might query non-DT sources such as system fuses or a version number register.
Simon, this series is assigned to you in patchwork. Are you the right person to apply it?
Yes. but not for this release, right?
Patch 2 in the series (which depends on this patch) fixes a bug for Tegra boards with LCD panels. Admittedly it appears to be only cosmetic (an error message is printed at boot), but "it's a bug" seems to satisfy the requirement to apply it for this release.
Sorry, I didn't know that. Given the core nature of this patch I would rather wait, and apply it next week. Let me know if you disagree.
Regards, Simon
On 05/04/2016 01:48 PM, Simon Glass wrote:
+Tom Rini
Hi Stephen,
On 4 May 2016 at 13:46, Stephen Warren swarren@wwwdotorg.org wrote:
On 05/04/2016 01:31 PM, Simon Glass wrote:
Hi Stephen,
On 4 May 2016 at 12:57, Stephen Warren swarren@wwwdotorg.org wrote:
On 04/19/2016 04:19 PM, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
In some cases, drivers may not want to bind to a device. Allow bind() to return -ENODEV in this case, and don't treat this as an error. This can be useful in situations where some information source other than the DT node's main status property indicates whether the device should be enabled, for example other DT properties might indicate this, or the driver might query non-DT sources such as system fuses or a version number register.
Simon, this series is assigned to you in patchwork. Are you the right person to apply it?
Yes. but not for this release, right?
Patch 2 in the series (which depends on this patch) fixes a bug for Tegra boards with LCD panels. Admittedly it appears to be only cosmetic (an error message is printed at boot), but "it's a bug" seems to satisfy the requirement to apply it for this release.
Sorry, I didn't know that. Given the core nature of this patch I would rather wait, and apply it next week. Let me know if you disagree.
I suppose that it's been broken long enough that another release won't matter.
Was my explanation of the bug in the description of patch 2/2 not clear in some way?
Hi Stephen,
On 4 May 2016 at 14:02, Stephen Warren swarren@wwwdotorg.org wrote:
On 05/04/2016 01:48 PM, Simon Glass wrote:
+Tom Rini
Hi Stephen,
On 4 May 2016 at 13:46, Stephen Warren swarren@wwwdotorg.org wrote:
On 05/04/2016 01:31 PM, Simon Glass wrote:
Hi Stephen,
On 4 May 2016 at 12:57, Stephen Warren swarren@wwwdotorg.org wrote:
On 04/19/2016 04:19 PM, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
In some cases, drivers may not want to bind to a device. Allow bind() to return -ENODEV in this case, and don't treat this as an error. This can be useful in situations where some information source other than the DT node's main status property indicates whether the device should be enabled, for example other DT properties might indicate this, or the driver might query non-DT sources such as system fuses or a version number register.
Simon, this series is assigned to you in patchwork. Are you the right person to apply it?
Yes. but not for this release, right?
Patch 2 in the series (which depends on this patch) fixes a bug for Tegra boards with LCD panels. Admittedly it appears to be only cosmetic (an error message is printed at boot), but "it's a bug" seems to satisfy the requirement to apply it for this release.
Sorry, I didn't know that. Given the core nature of this patch I would rather wait, and apply it next week. Let me know if you disagree.
I suppose that it's been broken long enough that another release won't matter.
Was my explanation of the bug in the description of patch 2/2 not clear in some way?
Looks good to me. Were you expecting me to apply both as a bug fix? If so I'd prefer to have Tom Warren's ACK. Even so, a core patch like this really needs the full test cycle IMO.
Regards, Simon
On 4 May 2016 at 14:09, Simon Glass sjg@chromium.org wrote:
Hi Stephen,
On 4 May 2016 at 14:02, Stephen Warren swarren@wwwdotorg.org wrote:
On 05/04/2016 01:48 PM, Simon Glass wrote:
+Tom Rini
Hi Stephen,
On 4 May 2016 at 13:46, Stephen Warren swarren@wwwdotorg.org wrote:
On 05/04/2016 01:31 PM, Simon Glass wrote:
Hi Stephen,
On 4 May 2016 at 12:57, Stephen Warren swarren@wwwdotorg.org wrote:
On 04/19/2016 04:19 PM, Stephen Warren wrote: > > > > From: Stephen Warren swarren@nvidia.com > > In some cases, drivers may not want to bind to a device. Allow bind() > to > return -ENODEV in this case, and don't treat this as an error. This > can > be useful in situations where some information source other than the > DT > node's main status property indicates whether the device should be > enabled, for example other DT properties might indicate this, or the > driver might query non-DT sources such as system fuses or a version > number > register.
Simon, this series is assigned to you in patchwork. Are you the right person to apply it?
Yes. but not for this release, right?
Patch 2 in the series (which depends on this patch) fixes a bug for Tegra boards with LCD panels. Admittedly it appears to be only cosmetic (an error message is printed at boot), but "it's a bug" seems to satisfy the requirement to apply it for this release.
Sorry, I didn't know that. Given the core nature of this patch I would rather wait, and apply it next week. Let me know if you disagree.
I suppose that it's been broken long enough that another release won't matter.
Was my explanation of the bug in the description of patch 2/2 not clear in some way?
Looks good to me. Were you expecting me to apply both as a bug fix? If so I'd prefer to have Tom Warren's ACK. Even so, a core patch like this really needs the full test cycle IMO.
Regards, Simon
Applied to u-boot-dm, thanks!
participants (3)
-
Simon Glass -
Stephen Warren -
Thierry Reding