[RFC PATCH 0/2] net: ixgbe: disable relaxed ordering at runtime

[Gabriele Paoloni]

From: gabriele paoloni <gabriele.paol...@huawei.com>

This patchset is based on Casey's patchset:
https://www.spinics.net/lists/arm-kernel/msg578927.html

It is a proposal to convert the ixgbe driver to disable RO at runtime
depending on the root port flags.

TODO: Casey's patchset above add quirks to disable RO only for Intel
E5-26xx and AMD ARM A1100, whereas currently the Intel ixgbe driver
disable RO by default. Therefore there may be some other host quirks
to be added in drivers/pci/quirks

gabriele paoloni (2):
  net: revert commit 1a8b6d76dc5b
  net: ixgbe: disable RO depending on the root port flags

 arch/Kconfig|  3 ---
 arch/sparc/Kconfig  |  1 -
 drivers/net/ethernet/intel/ixgbe/ixgbe_common.c | 35 ++---
 3 files changed, 20 insertions(+), 19 deletions(-)

-- 
2.7.4

[RFC PATCH 2/2] net: ixgbe: disable RO depending on the root port flags

[Gabriele Paoloni]

From: gabriele paoloni <gabriele.paol...@huawei.com>

Intel ixgbe drivers currently disable relaxed ordering at
compilation time depending on the host architecture. This is wrong
as RO support depends either on the root complex implementation
and/or on the EP itself and/or both.

This patch checks at runtime the root port flag
PCI_DEV_FLAGS_NO_RELAXED_ORDERING to be set in order to disable RO.

Signed-off-by: Gabriele Paoloni <gabriele.paol...@huawei.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_common.c | 35 ++---
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c 
b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
index 094e1d6..597cb7b 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
@@ -342,6 +342,8 @@ s32 ixgbe_start_hw_generic(struct ixgbe_hw *hw)
 s32 ixgbe_start_hw_gen2(struct ixgbe_hw *hw)
 {
u32 i;
+   struct pci_dev *root;
+   struct ixgbe_adapter *adapter;
 
/* Clear the rate limiters */
for (i = 0; i < hw->mac.max_tx_queues; i++) {
@@ -350,25 +352,28 @@ s32 ixgbe_start_hw_gen2(struct ixgbe_hw *hw)
}
IXGBE_WRITE_FLUSH(hw);
 
-#ifndef CONFIG_SPARC
-   /* Disable relaxed ordering */
-   for (i = 0; i < hw->mac.max_tx_queues; i++) {
-   u32 regval;
+   adapter = container_of(hw, struct ixgbe_adapter, hw);
+   root = pci_find_pcie_root_port(adapter->pdev);
 
-   regval = IXGBE_READ_REG(hw, IXGBE_DCA_TXCTRL_82599(i));
-   regval &= ~IXGBE_DCA_TXCTRL_DESC_WRO_EN;
-   IXGBE_WRITE_REG(hw, IXGBE_DCA_TXCTRL_82599(i), regval);
-   }
+   if (root && (root->dev_flags & PCI_DEV_FLAGS_NO_RELAXED_ORDERING)) {
+   /* Disable relaxed ordering */
+   for (i = 0; i < hw->mac.max_tx_queues; i++) {
+   u32 regval;
 
-   for (i = 0; i < hw->mac.max_rx_queues; i++) {
-   u32 regval;
+   regval = IXGBE_READ_REG(hw, IXGBE_DCA_TXCTRL_82599(i));
+   regval &= ~IXGBE_DCA_TXCTRL_DESC_WRO_EN;
+   IXGBE_WRITE_REG(hw, IXGBE_DCA_TXCTRL_82599(i), regval);
+   }
+
+   for (i = 0; i < hw->mac.max_rx_queues; i++) {
+   u32 regval;
 
-   regval = IXGBE_READ_REG(hw, IXGBE_DCA_RXCTRL(i));
-   regval &= ~(IXGBE_DCA_RXCTRL_DATA_WRO_EN |
-   IXGBE_DCA_RXCTRL_HEAD_WRO_EN);
-   IXGBE_WRITE_REG(hw, IXGBE_DCA_RXCTRL(i), regval);
+   regval = IXGBE_READ_REG(hw, IXGBE_DCA_RXCTRL(i));
+   regval &= ~(IXGBE_DCA_RXCTRL_DATA_WRO_EN |
+   IXGBE_DCA_RXCTRL_HEAD_WRO_EN);
+   IXGBE_WRITE_REG(hw, IXGBE_DCA_RXCTRL(i), regval);
+   }
}
-#endif
return 0;
 }
 
-- 
2.7.4

[RFC PATCH 1/2] net: revert commit 1a8b6d76dc5b

[Gabriele Paoloni]

From: gabriele paoloni <gabriele.paol...@huawei.com>

commit 1a8b6d76dc5b was introducing relaxed ordering as global
symbol. This does not make sense as relaxed ordering support
depends on the PCIe EP and on the Root Complex (therefore it is
not related to the CPU architecture)

Signed-off-by: Gabriele Paoloni <gabriele.paol...@huawei.com>
---
 arch/Kconfig| 3 ---
 arch/sparc/Kconfig  | 1 -
 drivers/net/ethernet/intel/ixgbe/ixgbe_common.c | 2 +-
 3 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index c4d6833..c0b118d 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -851,7 +851,4 @@ config STRICT_MODULE_RWX
  and non-text memory will be made non-executable. This provides
  protection against certain security exploits (e.g. writing to text)
 
-config ARCH_WANT_RELAX_ORDER
-   bool
-
 source "kernel/gcov/Kconfig"
diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index ed96869..3f47142 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -43,7 +43,6 @@ config SPARC
select ARCH_HAS_SG_CHAIN
select CPU_NO_EFFICIENT_FFS
select LOCKDEP_SMALL if LOCKDEP
-   select ARCH_WANT_RELAX_ORDER
 
 config SPARC32
def_bool !64BIT
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c 
b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
index c38d50c..094e1d6 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
@@ -350,7 +350,7 @@ s32 ixgbe_start_hw_gen2(struct ixgbe_hw *hw)
}
IXGBE_WRITE_FLUSH(hw);
 
-#ifndef CONFIG_ARCH_WANT_RELAX_ORDER
+#ifndef CONFIG_SPARC
/* Disable relaxed ordering */
for (i = 0; i < hw->mac.max_tx_queues; i++) {
u32 regval;
-- 
2.7.4

RE: [PATCH net-next 1/4] ixgbe: sparc: rename the ARCH_WANT_RELAX_ORDER to IXGBE_ALLOW_RELAXED_ORDER

[Gabriele Paoloni]

Hi Casey

Many thanks for the detailed explanation

> -Original Message-
> From: Casey Leedom [mailto:lee...@chelsio.com]
> Sent: 27 April 2017 21:35
> To: Bjorn Helgaas; Alexander Duyck
> Cc: Dingtianhong; Mark Rutland; Amir Ancel; Gabriele Paoloni; linux-
> p...@vger.kernel.org; Catalin Marinas; Will Deacon; Linuxarm; David
> Laight; jeffrey.t.kirs...@intel.com; netdev@vger.kernel.org; Robin
> Murphy; da...@davemloft.net; linux-arm-ker...@lists.infradead.org
> Subject: Re: [PATCH net-next 1/4] ixgbe: sparc: rename the
> ARCH_WANT_RELAX_ORDER to IXGBE_ALLOW_RELAXED_ORDER
> 
> | From: Bjorn Helgaas <helg...@kernel.org>
> | Sent: Thursday, April 27, 2017 10:19 AM
> |
> | Are you hinting that the PCI core or arch code could actually
> *enable*
> | Relaxed Ordering without the driver doing anything?  Is it safe to do
> that?
> | Is there such a thing as a device that is capable of using RO, but
> where the
> | driver must be aware of it being enabled, so it programs the device
> | appropriately?
> 
>   I forgot to reply to this portion of Bjorn's email.
> 
>   The PCI Configuration Space PCI Capability Device Control[Enable
> Relaxed
> Ordering] bit governs enabling the _ability_ for the PCIe Device to
> send
> TLPs with the Relaxed Ordering Attribute set.  It does not _cause_ RO
> to be
> set on TLPs.  Doing that would almost certainly cause Data Corruption
> Bugs
> since you only want a subset of TLPs to have RO set.
> 
>   For instance, we typically use RO for Ingress Packet Data delivery
> but
> non-RO for messages notifying the Host that an Ingress Packet has been
> delivered.  This ensures that the "Ingress Packet Delivered" non-RO TLP
> is
> processed _after_ any preceding RO TLPs delivering the actual Ingress
> Packet
> Data.
> 
>   In the above scenario, if one were to turn off Enable Relaxed
> Ordering via
> the PCIe Capability, then the on-chip PCIe engine would simply never
> send a
> TLP with the Relaxed Ordering Attribute set, regardless of any other
> chip
> programming.
> 
>   And finally, just to be absolutely clear, using Relaxed Ordering
> isn't and
> "Architecture Thing".  It's a PCIe Fabric End Point Thing.  Many End
> Points
> simply ignore the Relaxed Ordering Attribute (except to reflect it back
> in
> Response TLPs).  In this sense, Relaxed Ordering simply provides
> potentially useful optimization information to the PCIe End Point.

I think your view matches what I found out about the current usage of the
"Enable Relaxed Ordering" bit in Linux mainline: i.e. looking at where and
why the other drivers set/clear the "Enable Relaxed Ordering" they do not
look for any global symbol, nor they look at the host architecture.

So with respect to this specific ixgbe driver I guess the main question is
why RO was disabled by default by Intel for this EP (commit 3d5c520727ce
mentions issues with "some chipsets"), then why it is safe to enable it back
on SPARC?

Thanks
Gab

> 
> Casey

RE: Re: [PATCH net-next 1/4] ixgbe: sparc: rename the ARCH_WANT_RELAX_ORDER to IXGBE_ALLOW_RELAXED_ORDER

[Gabriele Paoloni]

Hi Amir
 
> From: Amir Ancel [mailto:am...@mellanox.com]
> Sent: 18 April 2017 21:18
> To: David Laight; Gabriele Paoloni; da...@davemloft.net
> Cc: Catalin Marinas; Will Deacon; Mark Rutland; Robin Murphy;
> jeffrey.t.kirs...@intel.com; alexander.du...@gmail.com; linux-arm-
> ker...@lists.infradead.org; netdev@vger.kernel.org; Dingtianhong;
> Linuxarm
> Subject: Re: Re: [PATCH net-next 1/4] ixgbe: sparc: rename the
> ARCH_WANT_RELAX_ORDER to IXGBE_ALLOW_RELAXED_ORDER
> 
> Hi,
> mlx5 driver is planned to have RO support this year.
> I believe drivers should be able to query whether the arch support it

I guess that here when you say query you mean having a config symbol
that is set accordingly to the host architecture, right?

As already said I have looked around a bit and other drivers do not seem
to enable/disable RO for their EP on the basis of the host architecture.
So why should mlx5 do it according to the host?

Also my understating is that some architectures (like ARM64 for example)
can have different PCI host controller implementations depending on the
vendor...therefore maybe it is not appropriate there to have a Kconfig
symbol selected by the architecture...  

Thanks
Gab

> or not and enable it in the network adapter accordingly.
> 
> -Amir
> 
> From: netdev-ow...@vger.kernel.org <netdev-ow...@vger.kernel.org> on
> behalf of David Laight <david.lai...@aculab.com>
> Sent: Tuesday, April 18, 2017 4:25:44 PM
> To: 'Gabriele Paoloni'; da...@davemloft.net
> Cc: Catalin Marinas; Will Deacon; Mark Rutland; Robin Murphy;
> jeffrey.t.kirs...@intel.com; alexander.du...@gmail.com; linux-arm-
> ker...@lists.infradead.org; netdev@vger.kernel.org; Dingtianhong;
> Linuxarm
> Subject: RE: Re: [PATCH net-next 1/4] ixgbe: sparc: rename the
> ARCH_WANT_RELAX_ORDER to IXGBE_ALLOW_RELAXED_ORDER
> 
> From: Gabriele Paoloni
> > Sent: 13 April 2017 10:11
> > > > Till now only the Intel ixgbe could support enable
> > > > Relaxed ordering in the drivers for special architecture,
> > > > but the ARCH_WANT_RELAX_ORDER is looks like a general name
> > > > for all arch, so rename to a specific name for intel
> > > > card looks more appropriate.
> > > >
> > > > Signed-off-by: Ding Tianhong <dingtianh...@huawei.com>
> > >
> > > This is not a driver specific facility.
> > >
> > > Any driver can test this symbol and act accordingly.
> > >
> > > Just because IXGBE is the first and only user, doesn't mean
> > > the facility is driver specific.
> >
> >
> > Please correct me if I am wrong but my understanding is that the
> standard
> > way to enable/disable relaxed ordering is to set/clear bit 4 of the
> Device
> > Control Register (PCI_EXP_DEVCTL_RELAX_EN 0x0010 /* Enable relaxed
> > ordering */).
> > Now I have looked up for all drivers either enabling or disabling
> relaxed
> > ordering and none of them seems to need a symbol to decide whether to
> > enable it or not.
> > Also it seems to me enabling/disabling relaxed ordering is never
> bound to the
> > host architecture.
> >
> > So why this should be (or it is expected to be) a generic symbol?
> > Wouldn't it be more correct to have this as a driver specific symbol
> now and
> > move it to a generic one later once we have other drivers requiring
> it?
> 
> 'Relaxed ordering' is a bit in the TLP header.
> A device (like the ixgbe hardware) can set it for some transactions and
> still have the transactions 'ordered enough' for the driver to work.
> (If all transactions have 'relaxed ordering' set then I suspect it is
> almost impossible to write a working driver.)
> The host side could (probably does) have a bit to enable 'relaxed
> ordering',
> it could also enforce stronger ordering than required by the PCIe spec
> (eg keeping reads in order).
> 
> Clearly, on some sparc64 systems, ixgbe needs to use 'relaxed
> ordering'.
> To me this requires two separate bits be enabled:
> 1) to the ixgbe driver to generate the 'relaxed' TLP.
> 2) to the host to actually act on them.
> If the ixgbe driver works when both are enabled why have options to
> disable either (except for bug-finding)?
> 
>     David

RE: Re: [PATCH net-next 1/4] ixgbe: sparc: rename the ARCH_WANT_RELAX_ORDER to IXGBE_ALLOW_RELAXED_ORDER

[Gabriele Paoloni]

Hi David

Many thanks for your reply

> -Original Message-
> From: David Laight [mailto:david.lai...@aculab.com]
> Sent: 18 April 2017 14:26
> To: Gabriele Paoloni; da...@davemloft.net
> Cc: Catalin Marinas; Will Deacon; Mark Rutland; Robin Murphy;
> jeffrey.t.kirs...@intel.com; alexander.du...@gmail.com; linux-arm-
> ker...@lists.infradead.org; netdev@vger.kernel.org; Dingtianhong;
> Linuxarm
> Subject: RE: Re: [PATCH net-next 1/4] ixgbe: sparc: rename the
> ARCH_WANT_RELAX_ORDER to IXGBE_ALLOW_RELAXED_ORDER
> 
> From: Gabriele Paoloni
> > Sent: 13 April 2017 10:11
> > > > Till now only the Intel ixgbe could support enable
> > > > Relaxed ordering in the drivers for special architecture,
> > > > but the ARCH_WANT_RELAX_ORDER is looks like a general name
> > > > for all arch, so rename to a specific name for intel
> > > > card looks more appropriate.
> > > >
> > > > Signed-off-by: Ding Tianhong <dingtianh...@huawei.com>
> > >
> > > This is not a driver specific facility.
> > >
> > > Any driver can test this symbol and act accordingly.
> > >
> > > Just because IXGBE is the first and only user, doesn't mean
> > > the facility is driver specific.
> >
> >
> > Please correct me if I am wrong but my understanding is that the
> standard
> > way to enable/disable relaxed ordering is to set/clear bit 4 of the
> Device
> > Control Register (PCI_EXP_DEVCTL_RELAX_EN 0x0010 /* Enable relaxed
> > ordering */).
> > Now I have looked up for all drivers either enabling or disabling
> relaxed
> > ordering and none of them seems to need a symbol to decide whether to
> > enable it or not.
> > Also it seems to me enabling/disabling relaxed ordering is never
> bound to the
> > host architecture.
> >
> > So why this should be (or it is expected to be) a generic symbol?
> > Wouldn't it be more correct to have this as a driver specific symbol
> now and
> > move it to a generic one later once we have other drivers requiring
> it?
> 
> 'Relaxed ordering' is a bit in the TLP header.
> A device (like the ixgbe hardware) can set it for some transactions and
> still have the transactions 'ordered enough' for the driver to work.
> (If all transactions have 'relaxed ordering' set then I suspect it is
> almost impossible to write a working driver.)
> The host side could (probably does) have a bit to enable 'relaxed
> ordering',
> it could also enforce stronger ordering than required by the PCIe spec
> (eg keeping reads in order).

My understanding is that from the host side the host is always allowed
(as long as it complies with the rules specified in sec.2.4.1 of the PCIe
Specs) to set the RO attribute in the TLP and the target function should
be abel to cope with it.

On the device side the device is allowed to set the RO attribute in the
TLP only if bit4 of the "Device Control Register" is set.

> 
> Clearly, on some sparc64 systems, ixgbe needs to use 'relaxed
> ordering'.
> To me this requires two separate bits be enabled:
> 1) to the ixgbe driver to generate the 'relaxed' TLP.
> 2) to the host to actually act on them.

My understanding is that for performance reasons when possible we
should enable relaxed ordering and I think this is up to the host
(i.e. the host somehow should know when he is capable of handling
RO TLPs and therefore it will try to enable it on the driver)

> If the ixgbe driver works when both are enabled why have options to
> disable either (except for bug-finding)?

I think that by default the ixgbe driver disable RO since there are
issues with "some chipsets" according to commit 3d5c520727ce "ixgbe:
move disabling of relaxed ordering in start_hw()".
What this means is a bit obscure to me and seems to be not related to
the host architecture

Also looking at where and why the other drivers set/clear the "Enable
Relaxed Ordering" bit it seems that currently this is not tied to the
host architecture nor to any global symbol; instead it seems purely
dependent on the PCIe device chipset itself.

> 
>   David

RE: Re: [PATCH net-next 1/4] ixgbe: sparc: rename the ARCH_WANT_RELAX_ORDER to IXGBE_ALLOW_RELAXED_ORDER

[Gabriele Paoloni]

Hi David

> -Original Message-
> Subject: Re: [PATCH net-next 1/4] ixgbe: sparc: rename the
> ARCH_WANT_RELAX_ORDER to IXGBE_ALLOW_RELAXED_ORDER
> Date: Sat, 1 Apr 2017 11:26:03 -0700
> From: David Miller 
> To: dingtianh...@huawei.com
> CC: catalin.mari...@arm.com, will.dea...@arm.com, mark.rutl...@arm.com,
> robin.mur...@arm.com, jeffrey.t.kirs...@intel.com,
> alexander.du...@gmail.com, linux-arm-ker...@lists.infradead.org,
> netdev@vger.kernel.org
> 
> From: Ding Tianhong 
> Date: Sat, 1 Apr 2017 15:25:51 +0800
> 
> > Till now only the Intel ixgbe could support enable
> > Relaxed ordering in the drivers for special architecture,
> > but the ARCH_WANT_RELAX_ORDER is looks like a general name
> > for all arch, so rename to a specific name for intel
> > card looks more appropriate.
> >
> > Signed-off-by: Ding Tianhong 
> 
> This is not a driver specific facility.
> 
> Any driver can test this symbol and act accordingly.
> 
> Just because IXGBE is the first and only user, doesn't mean
> the facility is driver specific.


Please correct me if I am wrong but my understanding is that the standard
way to enable/disable relaxed ordering is to set/clear bit 4 of the Device
Control Register (PCI_EXP_DEVCTL_RELAX_EN 0x0010 /* Enable relaxed
ordering */).
Now I have looked up for all drivers either enabling or disabling relaxed
ordering and none of them seems to need a symbol to decide whether to
enable it or not.
Also it seems to me enabling/disabling relaxed ordering is never bound to the
host architecture.

So why this should be (or it is expected to be) a generic symbol?
Wouldn't it be more correct to have this as a driver specific symbol now and
move it to a generic one later once we have other drivers requiring it?
  
Many thanks
Gab

> 
> Thank you.
> 
> .
> 

RE: [bug discuss] fjes driver call trace warning, "PNP0C02" used in fjes seems like a bug,

[Gabriele Paoloni]

+TO: David Miller
+CC: linux-ker...@vger.kernel.org

> -Original Message-
> From: Izumi, Taku [mailto:izumi.t...@jp.fujitsu.com]
> Sent: 08 June 2016 03:27
> To: Gabriele Paoloni; liudongdong (C)
> Cc: Linuxarm; netdev@vger.kernel.org
> Subject: RE: [bug discuss] fjes driver call trace warning, "PNP0C02"
> used in fjes seems like a bug,
> 
> Dear Gab,
> 
> > > > > I think that "PNP0C02" should be used to mark any motherboard
> > > reserved
> > > > > resource and not a specific network driver.
> > > > > It seems like a bug in the "fjes" driver.
> > >
> > >   Extended Socket network device is a shared memory based high-
> speed
> > >   network interface between Extended Partitions of PRIMEQUEST 2000
> E2
> > >   series. To check if firmware supports Extended Socket network
> device,
> > >   we take use of  "PCP0C02" device and special strings in DSDT.
> > >
> > >   "fjes" is not only "platform device driver (mainly act as network
> > >driver" but also "acpi driver" . If "PCP0C02" found and it is
> for
> > >Extended Socket network device, platform_device will be created.
> >
> > From my understanding PNP0C02 is not a valid ACPI device HID but it
> is
> > to be used only to reserve motherboard resources.
> >
> > Can you please explain your identifier choice?
> 
>Sorry for late.
> 
>Extended Socket network device is not a physical device. This is a
>kind of virtual device in memory region which firmware provides.

>From a SW perspective it like an acpi driver that uses "PNP0C02"
as driver ids to perform the driver match in the ACPI table.

>From my understanding this is wrong in principle because that identifier
must be used to reserve motherboard resources (see par 4.1.2 of the PCI
Firmware Specifications v3.2)

Therefore such identifier it is used from
http://lxr.free-electrons.com/source/drivers/pnp/system.c
to reserve such resources.

Basically your driver is breaking any other device that
needs to reserve motherboard resources through system.c
driver.

@David Miller, what is your opinion about this?
I think this driver should be reverted...

Thanks

Gab
 

>This driver retrieves resource information
>(memory reagion address firmware provides and so on)
>via PNP0C02. This resource is firmware reserved resources so we use
>PNP0C02.
> 
>Sincerely,
>Taku Izumi
> 
> >
> > Gab
> >
> > >
> > >   Sincerely,
> > >   Taku Izumi
> > >
> > > > >
> > > > > Thanks
> > > > >
> > > > > Dongdong
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > ___
> > > > > linuxarm mailing list
> > > > > linux...@huawei.com
> > > > > http://rnd-openeuler.huawei.com/mailman/listinfo/linuxarm

RE: [bug discuss] fjes driver call trace warning, "PNP0C02" used in fjes seems like a bug,

[Gabriele Paoloni]

Hi Taku Izumi

> -Original Message-
> From: Izumi, Taku [mailto:izumi.t...@jp.fujitsu.com]
> Sent: 06 June 2016 10:34
> To: Gabriele Paoloni; liudongdong (C)
> Cc: Linuxarm; netdev@vger.kernel.org
> Subject: RE: [bug discuss] fjes driver call trace warning, "PNP0C02"
> used in fjes seems like a bug,
> 
> Dear Liu,
> 
> > -Original Message-
> > From: Gabriele Paoloni [mailto:gabriele.paol...@huawei.com]
> > Sent: Friday, June 03, 2016 6:59 PM
> > To: liudongdong (C); Izumi, Taku/泉 拓
> > Cc: Linuxarm; netdev@vger.kernel.org
> > Subject: RE: [bug discuss] fjes driver call trace warning, "PNP0C02"
> used in fjes seems like a bug,
> >
> > Hi Dongdong
> >
> > Thanks for flagging this
> >
> > +to: Taku Izumi <izumi.t...@jp.fujitsu.com>
> >
> > Gab
> >
> > > -Original Message-
> > > From: linuxarm-boun...@huawei.com [mailto:linuxarm-
> boun...@huawei.com]
> > > On Behalf Of Dongdong Liu
> > > Sent: 03 June 2016 10:38
> > > To: netdev@vger.kernel.org
> > > Cc: Linuxarm
> > > Subject: [bug discuss] fjes driver call trace warning, "PNP0C02"
> used
> > > in fjes seems like a bug,
> > >
> > > Hi all:
> > >
> > > The bug is recorded in https://bugs.linaro.org/show_bug.cgi?id=2292.
> > >
> > > "PNP0C02" attached two modules drivers/pnp/system.c and
> > > drivers/net/fjes/fjes_main.c .
> > > "fjes" driver lead to the call trace.
> > >
> > > system.c:
> > > static const struct pnp_device_id pnp_dev_table[] = {
> > >  /* General ID for reserving resources */
> > >  {"PNP0c02", 0},
> > >  /* memory controller */
> > >  {"PNP0c01", 0},
> > >  {"", 0}
> > > };
> > >
> > > jes_main.c:
> > > static const struct acpi_device_id fjes_acpi_ids[] = {
> > >  {"PNP0C02", 0},
> > >  {"", 0},
> > > };
> > >
> > > Both of the modules use id "PNP0C02" (case insensitive),
> > >
> > > I used "PNP0C02" to mark motherboard reserved resource as below in
> > > UEFI.
> > > Device (RES1)
> > > {
> > >   Name (_HID, "HISI0081") // HiSi PCIe RC config baseaddress
> > >   Name (_CID, "PNP0C02") // Motherboard reserved resource
> > >   Name (_CRS, ResourceTemplate (){
> > >   Memory32Fixed (ReadWrite, 0xb008 , 0x1)
> > >   })
> > > }
> > >
> > > I think that "PNP0C02" should be used to mark any motherboard
> reserved
> > > resource and not a specific network driver.
> > > It seems like a bug in the "fjes" driver.
> 
>   Extended Socket network device is a shared memory based high-speed
>   network interface between Extended Partitions of PRIMEQUEST 2000 E2
>   series. To check if firmware supports Extended Socket network device,
>   we take use of  "PCP0C02" device and special strings in DSDT.
> 
>   "fjes" is not only "platform device driver (mainly act as network
>driver" but also "acpi driver" . If "PCP0C02" found and it is for
>Extended Socket network device, platform_device will be created.

>From my understanding PNP0C02 is not a valid ACPI device HID but it is
to be used only to reserve motherboard resources.

Can you please explain your identifier choice?

Thanks

Gab

> 
>   Sincerely,
>   Taku Izumi
> 
> > >
> > > Thanks
> > >
> > > Dongdong
> > >
> > >
> > >
> > >
> > > ___
> > > linuxarm mailing list
> > > linux...@huawei.com
> > > http://rnd-openeuler.huawei.com/mailman/listinfo/linuxarm

RE: [bug discuss] fjes driver call trace warning, "PNP0C02" used in fjes seems like a bug,

[Gabriele Paoloni]

Hi Dongdong

Thanks for flagging this

+to: Taku Izumi 

Gab

> -Original Message-
> From: linuxarm-boun...@huawei.com [mailto:linuxarm-boun...@huawei.com]
> On Behalf Of Dongdong Liu
> Sent: 03 June 2016 10:38
> To: netdev@vger.kernel.org
> Cc: Linuxarm
> Subject: [bug discuss] fjes driver call trace warning, "PNP0C02" used
> in fjes seems like a bug,
> 
> Hi all:
> 
> The bug is recorded in https://bugs.linaro.org/show_bug.cgi?id=2292.
> 
> "PNP0C02" attached two modules drivers/pnp/system.c and
> drivers/net/fjes/fjes_main.c .
> "fjes" driver lead to the call trace.
> 
> system.c:
> static const struct pnp_device_id pnp_dev_table[] = {
>  /* General ID for reserving resources */
>  {"PNP0c02", 0},
>  /* memory controller */
>  {"PNP0c01", 0},
>  {"", 0}
> };
> 
> jes_main.c:
> static const struct acpi_device_id fjes_acpi_ids[] = {
>  {"PNP0C02", 0},
>  {"", 0},
> };
> 
> Both of the modules use id "PNP0C02" (case insensitive),
> 
> I used "PNP0C02" to mark motherboard reserved resource as below in
> UEFI.
> Device (RES1)
> {
>   Name (_HID, "HISI0081") // HiSi PCIe RC config baseaddress
>   Name (_CID, "PNP0C02") // Motherboard reserved resource
>   Name (_CRS, ResourceTemplate (){
>   Memory32Fixed (ReadWrite, 0xb008 , 0x1)
>   })
> }
> 
> I think that "PNP0C02" should be used to mark any motherboard reserved
> resource and not a specific network driver.
> It seems like a bug in the "fjes" driver.
> 
> Thanks
> 
> Dongdong
> 
> 
> 
> 
> ___
> linuxarm mailing list
> linux...@huawei.com
> http://rnd-openeuler.huawei.com/mailman/listinfo/linuxarm