Re: [PATCH 2/2] powerpc/85xx: p1022ds: enable monitor switching via pixis indirect mode
On Fri, Nov 18, 2011 at 11:00:04AM -0600, Timur Tabi wrote: Scott Wood wrote: How do I update the device tree from platform code? prom_update_property() I assume this needs to be called after the DT has been unflattened, since it takes a device_node* as a parameter. Is there any way I can modify the tree before it's unflattened? I'd like to fixup the tree in an early_param() function. What's wrong with doing it in setup_arch()? Modifying a flat device tree is a more complicated task, and the kernel's code is read-only. It's not worth bringing libfdt or similar complexity in for this, when there are other alternatives. If it really needs to be done early, consider doing it from U-Boot. The bootwrapper would be a decent place as well, but we probably shouldn't require its use just for this. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/2] powerpc/85xx: p1022ds: enable monitor switching via pixis indirect mode
Scott Wood wrote: What's wrong with doing it in setup_arch()? Well, I was hoping to encapsulate everything into one function -- the early_param() callback function. But I guess that's not going to work. -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/2] powerpc/85xx: p1022ds: enable monitor switching via pixis indirect mode
Hi Timur,
On Thu, 17 Nov 2011 16:09:17 -0600 Timur Tabi ti...@freescale.com wrote:
Stephen Rothwell wrote:
+ struct device_node *indirect_node = NULL;
+ struct ccsr_guts_85xx __iomem *guts = NULL;
+ u8 __iomem *lbc_lcs0_ba = NULL;
+ u8 __iomem *lbc_lcs1_ba = NULL;
And if you had multiple error path labels, you could avoid these others
as well (and the NULL checks on the error path).
But I don't want multiple error path labels. The error path could only
happen if someone passed in a broken device tree (i.e. pretty much
never), so I'm not keen on complicating my code just to optimize
something that will never be used.
But you are already optimizing for the error path by doing the
assignments and NULL checks that are unneeded in the non error path.
What I am suggesting is adding a little more code that could end up doing
less at run time.
How about (not even compile tested):
static void p1022ds_set_monitor_port(enum fsl_diu_monitor_port port)
{
struct device_node *guts_node;
struct device_node *indirect_node;
struct ccsr_guts_85xx __iomem *guts;
u8 __iomem *lbc_lcs0_ba;
u8 __iomem *lbc_lcs1_ba;
u8 b;
/* Map the global utilities registers. */
guts_node = of_find_compatible_node(NULL, NULL, fsl,p1022-guts);
if (!guts_node) {
pr_err(p1022ds: missing global utilties device node\n);
return;
}
guts = of_iomap(guts_node, 0);
of_node_put(guts_node);
if (!guts) {
pr_err(p1022ds: could not map global utilties device\n);
return;
}
indirect_node = of_find_compatible_node(NULL, NULL,
fsl,p1022ds-indirect-pixis);
if (!indirect_node) {
pr_err(p1022ds: missing pixis indirect mode node\n);
goto exit_guts_iounmap;
}
lbc_lcs0_ba = of_iomap(indirect_node, 0);
if (!lbc_lcs0_ba) {
pr_err(p1022ds: could not map localbus chip select 0\n);
goto exit_indirect_node;
}
lbc_lcs1_ba = of_iomap(indirect_node, 1);
if (!lbc_lcs1_ba) {
pr_err(p1022ds: could not map localbus chip select 1\n);
goto exit_lcs0_iounmap;
}
/* Make sure we're in indirect mode first. */
if ((in_be32(guts-pmuxcr) PMUXCR_ELBCDIU_MASK) !=
PMUXCR_ELBCDIU_DIU) {
struct device_node *pixis_node;
void __iomem *pixis;
pixis_node =
of_find_compatible_node(NULL, NULL, fsl,p1022ds-fpga);
if (!pixis_node) {
pr_err(p1022ds: missing pixis node\n);
goto exit_lcs1_iounmap;
}
pixis = of_iomap(pixis_node, 0);
of_node_put(pixis_node);
if (!pixis) {
pr_err(p1022ds: could not map pixis registers\n);
goto exit_lcs1_iounmap;
}
/* Enable indirect PIXIS mode. */
setbits8(pixis + PX_CTL, PX_CTL_ALTACC);
iounmap(pixis);
/* Switch the board mux to the DIU */
out_8(lbc_lcs0_ba, PX_BRDCFG0); /* BRDCFG0 */
b = in_8(lbc_lcs1_ba);
b |= PX_BRDCFG0_ELBC_DIU;
out_8(lbc_lcs1_ba, b);
/* Set the chip mux to DIU mode. */
clrsetbits_be32(guts-pmuxcr, PMUXCR_ELBCDIU_MASK,
PMUXCR_ELBCDIU_DIU);
in_be32(guts-pmuxcr);
}
switch (port) {
case FSL_DIU_PORT_DVI:
/* Enable the DVI port, disable the DFP and the backlight */
out_8(lbc_lcs0_ba, PX_BRDCFG1);
b = in_8(lbc_lcs1_ba);
b = ~(PX_BRDCFG1_DFPEN | PX_BRDCFG1_BACKLIGHT);
b |= PX_BRDCFG1_DVIEN;
out_8(lbc_lcs1_ba, b);
break;
case FSL_DIU_PORT_LVDS:
/*
* LVDS also needs backlight enabled, otherwise the display
* will be blank.
*/
/* Enable the DFP port, disable the DVI and the backlight */
out_8(lbc_lcs0_ba, PX_BRDCFG1);
b = in_8(lbc_lcs1_ba);
b = ~PX_BRDCFG1_DVIEN;
b |= PX_BRDCFG1_DFPEN | PX_BRDCFG1_BACKLIGHT;
out_8(lbc_lcs1_ba, b);
break;
default:
pr_err(p1022ds: unsupported monitor port %i\n, port);
}
exit_lcs1_iounmap:
iounmap(lbc_lcs1_ba);
exit_lcs0_iounmap:
iounmap(lbc_lcs0_ba);
exit_indirect_node:
of_node_put(indirect_node);
exit_guts_iounmap:
iounmap(guts);
}
--
Cheers,
Stephen Rothwells...@canb.auug.org.au
Re: [PATCH 2/2] powerpc/85xx: p1022ds: enable monitor switching via pixis indirect mode
Hi Timur,
On Thu, 17 Nov 2011 12:57:39 -0600 Timur Tabi ti...@freescale.com wrote:
@@ -129,44 +144,117 @@ static void p1022ds_set_gamma_table(enum
fsl_diu_monitor_port port,
*/
static void p1022ds_set_monitor_port(enum fsl_diu_monitor_port port)
{
- struct device_node *np;
- void __iomem *pixis;
- u8 __iomem *brdcfg1;
+ struct device_node *guts_node = NULL;
There is no point in initialising this as it is assigned before being
used.
+ struct device_node *indirect_node = NULL;
+ struct ccsr_guts_85xx __iomem *guts = NULL;
+ u8 __iomem *lbc_lcs0_ba = NULL;
+ u8 __iomem *lbc_lcs1_ba = NULL;
And if you had multiple error path labels, you could avoid these others
as well (and the NULL checks on the error path).
--
Cheers,
Stephen Rothwells...@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
pgpNr80x3d08h.pgp
Description: PGP signature
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/2] powerpc/85xx: p1022ds: enable monitor switching via pixis indirect mode
On Thu, Nov 17, 2011 at 12:57:39PM -0600, Timur Tabi wrote: When the P1022's DIU video controller is active, the pixis must be accessed in indirect mode, which uses localbus chip select addresses. Switching between the DVI and LVDS monitor ports is handled by the pixis, so that switching needs to be done via indirect mode. This has the side-effect of no longer requiring U-Boot to enable the DIU. Now Linux can enable the DIU all by itself. Under what circumstances does Linux do this? How does Linux prevent the NOR flash driver from binding to the device when this mode has been or will be used? -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/2] powerpc/85xx: p1022ds: enable monitor switching via pixis indirect mode
Stephen Rothwell wrote:
static void p1022ds_set_monitor_port(enum fsl_diu_monitor_port port)
{
-struct device_node *np;
-void __iomem *pixis;
-u8 __iomem *brdcfg1;
+struct device_node *guts_node = NULL;
There is no point in initialising this as it is assigned before being
used.
Ok.
+struct device_node *indirect_node = NULL;
+struct ccsr_guts_85xx __iomem *guts = NULL;
+u8 __iomem *lbc_lcs0_ba = NULL;
+u8 __iomem *lbc_lcs1_ba = NULL;
And if you had multiple error path labels, you could avoid these others
as well (and the NULL checks on the error path).
But I don't want multiple error path labels. The error path could only happen
if someone passed in a broken device tree (i.e. pretty much never), so I'm not
keen on complicating my code just to optimize something that will never be used.
--
Timur Tabi
Linux kernel developer at Freescale
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/2] powerpc/85xx: p1022ds: enable monitor switching via pixis indirect mode
Scott Wood wrote: This has the side-effect of no longer requiring U-Boot to enable the DIU. Now Linux can enable the DIU all by itself. Under what circumstances does Linux do this? p1022ds_set_monitor_port() is called by the DIU driver when it enables the DIU. This happens on boot, for example, if you enable the framebuffer console. How does Linux prevent the NOR flash driver from binding to the device when this mode has been or will be used? It doesn't. This isn't a simple problem to solve. On the P1022, NOR flash and the DIU are incompatible, and yet that's exactly what we ship on the P1022DS board. We could just remove the NOR flash node from the DTS. -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/2] powerpc/85xx: p1022ds: enable monitor switching via pixis indirect mode
On Thu, Nov 17, 2011 at 04:12:02PM -0600, Timur Tabi wrote: Scott Wood wrote: This has the side-effect of no longer requiring U-Boot to enable the DIU. Now Linux can enable the DIU all by itself. Under what circumstances does Linux do this? p1022ds_set_monitor_port() is called by the DIU driver when it enables the DIU. This happens on boot, for example, if you enable the framebuffer console. How does Linux prevent the NOR flash driver from binding to the device when this mode has been or will be used? It doesn't. This isn't a simple problem to solve. On the P1022, NOR flash and the DIU are incompatible, and yet that's exactly what we ship on the P1022DS board. We could just remove the NOR flash node from the DTS. As we discussed earlier, you could have a kernel boot parameter that indicates what mode you'd like the localbus to run in. Then, platform code could update the device tree before any drivers bind. Or, have U-boot set up the desired mode before entering the kernel, and provide an appropriate device tree. Letting the kernel bind to localbus devices such as NOR flash, and then taking the devices away without notice just because another device gets initialized, is not the way to go. We've already left it just works territory, might as well make the user choose explicitly. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/2] powerpc/85xx: p1022ds: enable monitor switching via pixis indirect mode
Scott Wood wrote: As we discussed earlier, you could have a kernel boot parameter that indicates what mode you'd like the localbus to run in. Then, platform code could update the device tree before any drivers bind. How do I update the device tree from platform code? Or, have U-boot set up the desired mode before entering the kernel, and provide an appropriate device tree. I can both of these features, but not in this patch. -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/2] powerpc/85xx: p1022ds: enable monitor switching via pixis indirect mode
On Thu, Nov 17, 2011 at 04:28:48PM -0600, Timur Tabi wrote: Scott Wood wrote: As we discussed earlier, you could have a kernel boot parameter that indicates what mode you'd like the localbus to run in. Then, platform code could update the device tree before any drivers bind. How do I update the device tree from platform code? prom_update_property() Or, since you shouldn't be declaring these devices directly under a simple-bus node, selectively probe the devices that are accessible. Or, have U-boot set up the desired mode before entering the kernel, and provide an appropriate device tree. I can both of these features, but not in this patch. This patch is the one that is adding a switch to indirect mode. You're adding it to the wrong place (it shouldn't be in a function called by the DIU driver), so it is relevant to this patch. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
