dm: core: allow drivers to refuse to bind
Subsystems (3)
PATCH 1/2 dm: core: allow drivers to refuse to bind
From: Stephen Warren <[email protected]>
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.
--- drivers/core/lists.c | 4 ++++ 1 file changed, 4 insertions(+)
drivers/core/lists.c +4 -0
@@ -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);
Signature · 1 line
2.8.1
PATCH 1/2 dm: core: allow drivers to refuse to bind
Quoted 9 lines · Stephen Warren
From: Stephen Warren <[email protected]>
In some cases, drivers may not want to bind to a device. Allow
bind()to return-ENODEVin 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?
PATCH 1/2 dm: core: allow drivers to refuse to bind
Hi Stephen,
Quoted 13 lines · Stephen Warren
Quoted 10 lines · Stephen Warren
From: Stephen Warren <[email protected]>
In some cases, drivers may not want to bind to a device. Allow
bind()to return-ENODEVin 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.
Quoted 2 lines
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
PATCH 1/2 dm: core: allow drivers to refuse to bind
Quoted 16 lines · Simon Glass
Hi Stephen,
Quoted 13 lines · Stephen Warren
Quoted 10 lines · Stephen Warren
From: Stephen Warren <[email protected]>
In some cases, drivers may not want to bind to a device. Allow
bind()to return-ENODEVin 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.
Quoted 4 lines
Quoted 2 lines
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.
PATCH 1/2 dm: core: allow drivers to refuse to bind
+Tom Rini
Hi Stephen,
Quoted 27 lines · Stephen Warren
Quoted 26 lines · Simon Glass
Hi Stephen,
Quoted 20 lines · Stephen Warren
Quoted 4 lines · Stephen Warren
From: Stephen Warren <[email protected]>
Quoted 8 lines
In some cases, drivers may not want to bind to a device. Allow
bind()to return-ENODEVin 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?
Quoted 7 lines
Quoted 1 line
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
PATCH 1/2 dm: core: allow drivers to refuse to bind
Quoted 35 lines · Simon Glass
+Tom Rini
Hi Stephen,
Quoted 30 lines · Stephen Warren
Quoted 27 lines · Simon Glass
Hi Stephen,
Quoted 17 lines · Stephen Warren
Quoted 4 lines · Stephen Warren
From: Stephen Warren <[email protected]>
Quoted 8 lines
In some cases, drivers may not want to bind to a device. Allow
bind()to return-ENODEVin 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.Quoted 3 lines
Simon, this series is assigned to you in patchwork. Are you the right person
to apply it?Yes. but not for this release, right?
Quoted 7 lines
Quoted 4 lines
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?
PATCH 1/2 dm: core: allow drivers to refuse to bind
Hi Stephen,
Quoted 58 lines · Stephen Warren
Quoted 53 lines · Simon Glass
+Tom Rini
Hi Stephen,
Quoted 40 lines · Stephen Warren
Quoted 28 lines · Simon Glass
Hi Stephen,
Quoted 23 lines · Stephen Warren
Quoted 16 lines · Stephen Warren
From: Stephen Warren <[email protected]>
In some cases, drivers may not want to bind to a device. Allow
bind()to
return-ENODEVin 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.Quoted 7 lines
Quoted 3 lines
Simon, this series is assigned to you in patchwork. Are you the right person
to apply it?Yes. but not for this release, right?
Quoted 5 lines
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.Quoted 2 lines
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.
Quoted 5 lines
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
PATCH 1/2 dm: core: allow drivers to refuse to bind
Quoted 67 lines · Simon Glass
Hi Stephen,
Quoted 58 lines · Stephen Warren
Quoted 53 lines · Simon Glass
+Tom Rini
Hi Stephen,
Quoted 40 lines · Stephen Warren
Quoted 28 lines · Simon Glass
Hi Stephen,
Quoted 23 lines · Stephen Warren
Quoted 16 lines · Stephen Warren
From: Stephen Warren <[email protected]>
In some cases, drivers may not want to bind to a device. Allow
bind()to
return-ENODEVin 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.Quoted 7 lines
Quoted 3 lines
Simon, this series is assigned to you in patchwork. Are you the right person
to apply it?Yes. but not for this release, right?
Quoted 5 lines
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.Quoted 2 lines
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.
Quoted 5 lines
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?
Quoted 6 lines
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!
PATCH 1/2 dm: core: allow drivers to refuse to bind
In reply to Stephen WarrenQuoted 10 lines · Stephen Warren
From: Stephen Warren <[email protected]>
In some cases, drivers may not want to bind to a device. Allow
bind()to return-ENODEVin 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.
Quoted 4 lines
Signed-off-by Stephen Warren <[email protected]>--- drivers/core/lists.c | 4 ++++ 1 file changed, 4 insertions(+)
PATCH 2/2 video: tegra: refuse to bind to disabled dcs
In reply to Stephen WarrenFrom: Stephen Warren <[email protected]>
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.
---
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(-)
arch/arm/dts/tegra20-seaboard.dts +0 -4
@@ -40,10 +40,6 @@ nvidia,panel = <&lcd_panel>; }; }; - - dc at 54240000 { - status = "disabled"; - }; }; /* This is not used in U-Boot, but is expected to be in kernel .dts */
drivers/video/tegra.c +7 -0
@@ -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 ;
Signature · 1 line
2.8.1
PATCH 2/2 video: tegra: refuse to bind to disabled dcs
Quoted 23 lines · Stephen Warren
From: Stephen Warren <[email protected]>
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.
Quoted 6 lines
Cc Thierry Reding <[email protected]>Signed-off-by Stephen Warren <[email protected]>---
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.
Quoted 25 lines
--- arch/arm/dts/tegra20-seaboard.dts | 4 ---- drivers/video/tegra.c | 7 +++++++ 2 files changed, 7 insertions(+), 4 deletions(-)arch/arm/dts/tegra20-seaboard.dts +0
-4@@ -40,10 +40,6 @@ nvidia,panel = <&lcd_panel>; }; }; - - dc at 54240000 { - status = "disabled"; - }; }; /* This is not used in U-Boot, but is expected to be in kernel .dts */drivers/video/tegra.c +0
-0
drivers/video/tegra.c static int tegra_lcd_ofdata_to_platdata(struct udevice *dev) · line 629
Quoted 10 lines
drivers/video/tegra.c +7
-0@@ -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:
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160420/698c26c3/attachment.sig>
PATCH 2/2 video: tegra: refuse to bind to disabled dcs
In reply to Stephen WarrenQuoted 25 lines · Stephen Warren
From: Stephen Warren <[email protected]>
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 <[email protected]>Signed-off-by Stephen Warren <[email protected]>
Quoted 8 lines
---
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.
PATCH 2/2 video: tegra: refuse to bind to disabled dcs
Quoted 35 lines · Simon Glass
Quoted 25 lines · Stephen Warren
From: Stephen Warren <[email protected]>
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 <[email protected]>Signed-off-by Stephen Warren <[email protected]>Quoted 8 lines
---
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(-)
Quoted 3 lines
Well it seems reasonable to me.
Reviewed-by Simon Glass <[email protected]>
Applied to u-boot-dm, thanks!