On Wed, Jan 04, 2017 at 04:17:13PM -0700, Al Stone wrote:
On 01/04/2017 04:49 AM, graeme.gregory@linaro.org wrote:
On Wed, Dec 28, 2016 at 04:47:55PM +0800, Heyi Guo wrote:
Hi Leif,
Refine this path according you and Graeme's comments,
please help to review, thanks.
Thanks and Regards,
Heyi
在 2016/12/28 16:42, Chenhui Sun 写道:
From: Heyi Guo heyi.guo@linaro.org
D05 has 4 physical NIC ports, which can be extended to 8 ports using an expansion card. However, the existing ACPI tables unconditionally describes the potential maximum of 8.
This patch add _STA method to the extra 4 ports from ACPI. The _STA will return the actual value when code has been added to dynamically detect whether they are present.
This patch resolves https://bugs.linaro.org/show_bug.cgi?id=2714.
Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Heyi Guo heyi.guo@linaro.org --- Chips/Hisilicon/Hi1616/D05AcpiTables/Dsdt/D05Hns.asl | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/Chips/Hisilicon/Hi1616/D05AcpiTables/Dsdt/D05Hns.asl b/Chips/Hisilicon/Hi1616/D05AcpiTables/Dsdt/D05Hns.asl index 046257b..14a905d 100644 --- a/Chips/Hisilicon/Hi1616/D05AcpiTables/Dsdt/D05Hns.asl +++ b/Chips/Hisilicon/Hi1616/D05AcpiTables/Dsdt/D05Hns.asl @@ -662,6 +662,10 @@ Scope(_SB) Package () {"port-idx-in-ae", 2}, } })
I think you misunderstood what I meant.
I meant something like (untested)
Name (NICE, 0)
Method (_STA, 0x0, NotSerialized) { Return (NICE); }
Then if my thinking is correct a later SSDT loaded when the extender is present could then
Store (\_SB.XXXX.NICE, 1)
To change the status.
Otherwise if this does not work how I think it should this patch is fine for me.
Thanks
Graeme
The only caution with this I would provide is that all object definitions in an SSDT that is dynamically loaded by default *replace* any existing object definitions with the same name. So, while I quite like the use of Name(NICE,0) as suggested, that would only work if the SSDT being loaded does *not* contain Device definitions for the ports.
I think you have several options here.
Let's assume you have Device ETH0-3 that are the devices always present, and devices ETH4-7 that are the ones being dynamically added. In all cases, I would define ETH0-3 in the DSDT since they will always be present.
Option 1: define ETH4-7 in the DSDT *and* a dynamically loaded SSDT
In this case, the DSDT might contain something like:
Device (ETH4) { ... Method (_STA, 0x0, NotSerialized) { Return (0) } ... }
and the SSDT that is loaded later on might look like this:
Device (ETH4) { ... Method (_STA, 0x0, NotSerialized) { Return (1) } ... }
When the SSDT is loaded, the definitions of ETH4-7 are replaced and the new _STA method is used.
And this is why I ask experts, I had totally forgotten about that, and in fact if Heyi wants to use Option 1: this file already does that!
Graeme
Option 2: only define ETH4-7 in a dynamically loaded SSDT
For this case, there would be no definition of ETH4-7 in the DSDT. They would not exist at all since there is nothing to be enumerated in the ASL namespace.
In the SSDT that gets loaded, each device would look something like this:
Device (ETH4) { Name (_HID, xxxxxxxx) .... }
An _STA method could be added to Return(1) but would not be required since the ACPI interpreter will create it by default when Device(ETH4) is created in the namespace. I personally would include just so I could put comments around it and explain what's happening.
Option 3: define ETH0-7 in the DSDT, add a dummy device in the SSDT
In this case, we're using the Name(NICE) idea above. The DSDT would contain a bit mask of define ETHx devices, and the _STA methods would test for their value. Something like this:
Scope (_SB) { Name (MASK, 0x0f) /* Defines only ETH0-3 as active */ ... Device (ETH0) { ... Method (_STA, 0x0, NotSerialized) { If (And (_SB.MASK, 0x1)) { Return (0x1) } Return (0x0) } ... } ...
Device (ETH1) { ... Method (_STA, 0x0, NotSerialized) { If (And (\_SB.MASK, 0x2)) { Return (0x1) } Return (0x0) } ... } ...Device (ETH7) { ... Method (_STA, 0x0, NotSerialized) { If (And (_SB.MASK, 0x80)) { Return (0x1) } Return (0x0) } ... } In the SSDT, some new dummy device would have to be defined, and that might look like this:
... Device (DUMY) { ... Method (_INI, 0x0, Serialized) { Store(_SB.MASK, 0xff) /* Enable all ETHx */ } ... }
When the SSDT gets loaded, the bit mask gets changed and the next time the _STA values are checked, the devices will be enabled. You will have to force the device drivers to enable the new ports somehow (perhaps check the value of the _SB.MASK object after the SSDT is loaded...).
Or, finally, Option 4: provide a simple status register somewhere
This is essentially the same as Option 3. The difference is that instead of defining Name(_SB.MASK) as a variable, it's a hardware register in some OperationRegion. If there is no expander, this read-only register returns 0x0f; if there is an expander, it returns 0xff. The DSDT _STA methods would simply test the register (the ASL might even be identical). If the expander can only be added with power off, there would be no need to load an SSDT.
Of all these, Option 2 is simplest, but could allow the firmware to lie; if the SSDT is loaded without the expander card being present, it is now incorrect.
Option 4 is probably optimal; the firmware and hardware can always be in sync and consistent. If you can't do Option 4, Option 3 is probably the most flexible and maintainable over time, but could also cause problems if somehow the SSDT is loaded without the expander being present.
- Method (_STA, 0x0, NotSerialized) + { + Return (0) + }
} Device (ETH3) { Name(_HID, "HISI00C2") @@ -674,6 +678,10 @@ Scope(_SB) Package () {"port-idx-in-ae", 3}, } }) + Method (_STA, 0x0, NotSerialized) + { + Return (0) + } } Device (ROCE) { @@ -1216,6 +1224,10 @@ Scope(_SB) Package () {"port-idx-in-ae", 0}, } }) + Method (_STA, 0x0, NotSerialized) + { + Return (0) + } } Device (ETH7) { Name(_HID, "HISI00C2") @@ -1228,6 +1240,10 @@ Scope(_SB) Package () {"port-idx-in-ae", 1}, } }) + Method (_STA, 0x0, NotSerialized) + { + Return (0) + } } }
_______________________________________________ Linaro-uefi mailing list Linaro-uefi@lists.linaro.org https://lists.linaro.org/mailman/listinfo/linaro-uefi
-- ciao, al
Al Stone Software Engineer Linaro Enterprise Group al.stone@linaro.org