Commit Message

The Kconfig for the ePAPR hypervisor byte channel driver has a "depends on PPC",
which means it would compile on all PowerPC platforms, even though it's
only been tested on Freescale platforms. Change the Kconfig to depend on
FSL_SOC instead.
Signed-off-by: Timur Tabi <timur@freescale.com>
---
drivers/tty/Kconfig | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

Comments

On Thu, Aug 25, 2011 at 11:20:45AM -0500, Timur Tabi wrote:
> The Kconfig for the ePAPR hypervisor byte channel driver has a "depends on PPC",> which means it would compile on all PowerPC platforms, even though it's> only been tested on Freescale platforms. Change the Kconfig to depend on> FSL_SOC instead.
tested doesn't mean that it shouldn't still build properly for other
platforms, right?
What is keeping the driver from building on all PPC, or even all arches
today?
greg k-h

Greg KH wrote:
> tested doesn't mean that it shouldn't still build properly for other> platforms, right?
The problem is the dependency on MSR_GS, which is defined only for Book-E
PowerPC chips, not all PowerPC.
So I gave it some more thought, and technically ePAPR extends beyond Book-E, so
it's wrong for the driver to depend on anything specific to Book-E. I've
removed the code that breaks:
/* Check if we're running as a guest of a hypervisor */
if (!(mfmsr() & MSR_GS))
return;
> What is keeping the driver from building on all PPC, or even all arches> today?
I've made a few changes, and it builds on all PPC now. I'll post a new patch.

On Thu, Aug 25, 2011 at 01:02:01PM -0500, Timur Tabi wrote:
> Greg KH wrote:> > tested doesn't mean that it shouldn't still build properly for other> > platforms, right?> > The problem is the dependency on MSR_GS, which is defined only for Book-E> PowerPC chips, not all PowerPC.> > So I gave it some more thought, and technically ePAPR extends beyond Book-E, so> it's wrong for the driver to depend on anything specific to Book-E. I've> removed the code that breaks:> > /* Check if we're running as a guest of a hypervisor */> if (!(mfmsr() & MSR_GS))> return;
But don't you really want this type of check at runtime? What happens
if you load this driver on a machine that is not a guest? Will things
break? Shouldn't you still refuse to load somehow?
thanks,
greg k-h

Greg KH wrote:
> But don't you really want this type of check at runtime? What happens> if you load this driver on a machine that is not a guest? Will things> break? Shouldn't you still refuse to load somehow?
This is in the udbg code, which falls under the category of, "turn this on only
if you know what you're doing."
The udbg code runs very early, before the device tree is available. There's no
way of knowing at this point whether or not we're running under a hypervisor.
If you turn on udbg support, then it means that you're trying to do some very
specific debugging on a specific platform.
So I'm not removing this code just to fix the build break. It really should
never have been there in the first place.

On Thu, Aug 25, 2011 at 01:51:20PM -0500, Timur Tabi wrote:
> Greg KH wrote:> > But don't you really want this type of check at runtime? What happens> > if you load this driver on a machine that is not a guest? Will things> > break? Shouldn't you still refuse to load somehow?> > This is in the udbg code, which falls under the category of, "turn this on only> if you know what you're doing."> > The udbg code runs very early, before the device tree is available. There's no> way of knowing at this point whether or not we're running under a hypervisor.> If you turn on udbg support, then it means that you're trying to do some very> specific debugging on a specific platform.> > So I'm not removing this code just to fix the build break. It really should> never have been there in the first place.
Ok, thanks for the details, I'll queue up the patch in a bit.
greg k-h