Netdev List
 help / color / mirror / Atom feed
* [PATCH net v3 0/9] net: aquantia: Atlantic driver 12/2017 updates
From: Igor Russkikh @ 2017-12-14  9:34 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, David Arcari, Pavel Belous, Nadezhda Krupnina,
	Simon Edelhaus, Igor Russkikh

The patchset contains important hardware fix for machines with large MRRS
and couple of improvement in stats and capabilities reporting

patch v3:
 - Fixed patch #7 after Andrew's finding. NIC level stats actually
   have to be cleaned only on hw struct creation (and this is done
   in kzalloc). On each hwinit we only have to reset link state
   to make sure hw stats update will not increment nic stats during init.

patch v2:
 - split into more detailed commits

Comment from David on wrong defines case will be submitted separately later

Igor Russkikh (9):
  net: aquantia: Fix actual speed capabilities reporting
  net: aquantia: Fix hardware DMA stream overload on large MRRS
  net: aquantia: Extend stat counters to 64bit values
  net: aquantia: Fill ndev stat couters from hardware
  net: aquantia: Fill in multicast counter in ndev stats from hardware
  net: aquantia: Improve link state and statistics check interval
    callback
  net: aquantia: Update hw counters on hw init
  net: aquantia: Fix typo in ethtool statistics names
  net: aquantia: Increment driver version

 drivers/net/ethernet/aquantia/atlantic/aq_cfg.h    |  5 +-
 .../net/ethernet/aquantia/atlantic/aq_ethtool.c    | 16 ++---
 drivers/net/ethernet/aquantia/atlantic/aq_hw.h     | 29 +++++++-
 drivers/net/ethernet/aquantia/atlantic/aq_nic.c    | 82 +++++++++++++++-------
 drivers/net/ethernet/aquantia/atlantic/aq_nic.h    |  2 +-
 .../net/ethernet/aquantia/atlantic/aq_pci_func.c   |  5 +-
 .../ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c  | 17 ++++-
 .../ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c  | 29 +++++++-
 .../aquantia/atlantic/hw_atl/hw_atl_llh_internal.h |  6 ++
 .../aquantia/atlantic/hw_atl/hw_atl_utils.c        | 80 +++++++--------------
 .../aquantia/atlantic/hw_atl/hw_atl_utils.h        |  6 +-
 drivers/net/ethernet/aquantia/atlantic/ver.h       |  6 +-
 12 files changed, 177 insertions(+), 106 deletions(-)

-- 
2.7.4

^ permalink raw reply

* Re: [PATCH 1/3] net: phy: add support to detect 100BASE-T1 capability
From: Lucas Stach @ 2017-12-14  9:21 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: Florian Fainelli, netdev, kernel, patchwork-lst
In-Reply-To: <20171213201110.GB932@lunn.ch>

Hi Andrew,

Am Mittwoch, den 13.12.2017, 21:11 +0100 schrieb Andrew Lunn:
> On Wed, Dec 13, 2017 at 06:37:49PM +0100, Lucas Stach wrote:
> > 100BASE-T1 is the automotive ethernet standard 802.3bw-2015.
> > Currently
> > we don't detect any valid modes for PHYs, which only support this
> > standard. Add support to detect the common 100Mbit full-duplex
> > mode.
> > 
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > ---
> >  drivers/net/phy/phy_device.c | 2 ++
> >  include/uapi/linux/mii.h     | 1 +
> >  2 files changed, 3 insertions(+)
> > 
> > diff --git a/drivers/net/phy/phy_device.c
> > b/drivers/net/phy/phy_device.c
> > index 67f25ac29025..8ef48b38d97b 100644
> > --- a/drivers/net/phy/phy_device.c
> > +++ b/drivers/net/phy/phy_device.c
> > @@ -1607,6 +1607,8 @@ int genphy_config_init(struct phy_device
> > *phydev)
> >  		if (val < 0)
> >  			return val;
> >  
> > +		if (val & ESTATUS_100T1_FULL)
> > +			features |= SUPPORTED_100baseT_Full;
> 
> Hi Lucas
> 
> Why did you decide to do this, and not add a SUPPORTED_100baseT1?
> 
> Could a device support both 100-BASE-T and 100-BASE-T1?  If at some
> point we need to differentiate between them, it is going to be
> hard. Especially since this is part of the kernel ABI.

Networking and especially PHY isn't really my primary area of
expertise, so excuse my ignorance. My reasoning was that we don't
differentiate between 100BASE-T2 and 100BASE-T4 in the kernel today, so
I thought it was fine to handle T1 the same way.

There are PHYs that can both do regular 100/1000 MBit Ethernet and
100BASE-T1, but definitely not at the same time or over the same
electrical wiring. 100BASE-T1 is really different in that it uses
capacitive coupling, instead of magnetic like on regular Ethernet. So
it is really a board level decision what gets used and is not something
I would expect to change at runtime.

I'll leave it to your judgment if this patch seems fine with the above
information in mind. Happy to rework if needed.

Regards,
Lucas

^ permalink raw reply

* net-next libbpf broken on prev kernel release
From: Eric Leblond @ 2017-12-14  9:16 UTC (permalink / raw)
  To: netdev, Martin KaFai Lau

Hello,

It seems that the following patch did break libbpf (in net-next
version) which is not able to load anymore a program on a 4.14:

tree 5096ddd73981e33a2164606461a45b56a189889c
parent ad5b177bd73f5107d97c36f56395c4281fb6f089
author Martin KaFai Lau <kafai@fb.com> Wed Sep 27 14:37:54 2017 -0700
committer David S. Miller <davem@davemloft.net> Fri Sep 29 06:17:05 2017 +0100

bpf: libbpf: Provide basic API support to specify BPF obj name

The problem comes from

-int bpf_load_program(enum bpf_prog_type type, const struct bpf_insn *insns,
-		     size_t insns_cnt, const char *license,
-		     __u32 kern_version, char *log_buf, size_t log_buf_sz)
+int bpf_load_program_name(enum bpf_prog_type type, const char *name,
+			  const struct bpf_insn *insns,
+			  size_t insns_cnt, const char *license,
+			  __u32 kern_version, char *log_buf,
+			  size_t log_buf_sz)
 {
 	int fd;
 	union bpf_attr attr;
+	__u32 name_len = name ? strlen(name) : 0;
 
 	bzero(&attr, sizeof(attr));
 	attr.prog_type = type;
@@ -130,6 +151,7 @@ int bpf_load_program(enum bpf_prog_type type, const struct bpf_insn *insns,
 	attr.log_size = 0;
 	attr.log_level = 0;
 	attr.kern_version = kern_version;
+	memcpy(attr.prog_name, name, min(name_len, BPF_OBJ_NAME_LEN - 1));

If I comment the memcpy then the eBPF program is loading correctly.

Is this a wanted behavior to have libbpf that needs to be in sync with
kernel ? or should it be fixed ?

BR,
-- 
Eric Leblond <eric@regit.org>

^ permalink raw reply

* [patch iproute2] tc: fix command "tc actions del" hang issue
From: Chris Mi @ 2017-12-14  9:09 UTC (permalink / raw)
  To: netdev; +Cc: jiri

If command is RTM_DELACTION, a non-NULL pointer is passed to rtnl_talk().
Then flag NLM_F_ACK is not set on n->nlmsg_flags and netlink_ack() will
not be called. Command tc will wait for the reply for ever.

Fixes: 86bf43c7c2fd ("lib/libnetlink: update rtnl_talk to support malloc
buff at run time")
Signed-off-by: Chris Mi <chrism@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
---
 tc/m_action.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tc/m_action.c b/tc/m_action.c
index afb6cfad..986ef7d0 100644
--- a/tc/m_action.c
+++ b/tc/m_action.c
@@ -529,7 +529,7 @@ static int tc_action_gd(int cmd, unsigned int flags,
 
 	req.n.nlmsg_seq = rth.dump = ++rth.seq;
 
-	if (rtnl_talk(&rth, &req.n, &ans) < 0) {
+	if (rtnl_talk(&rth, &req.n, cmd == RTM_DELACTION ? NULL : &ans) < 0) {
 		fprintf(stderr, "We have an error talking to the kernel\n");
 		return 1;
 	}
-- 
2.14.3

^ permalink raw reply related

* Re: [PATCH ipsec-next] xfrm: check for xdo_dev_state_free
From: kbuild test robot @ 2017-12-14  8:51 UTC (permalink / raw)
  To: Shannon Nelson; +Cc: kbuild-all, steffen.klassert, netdev
In-Reply-To: <1513025842-12064-1-git-send-email-shannon.nelson@oracle.com>

[-- Attachment #1: Type: text/plain, Size: 1941 bytes --]

Hi Shannon,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on ipsec-next/master]

url:    https://github.com/0day-ci/linux/commits/Shannon-Nelson/xfrm-check-for-xdo_dev_state_free/20171214-150202
base:   https://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec-next.git master
config: score-spct6600_defconfig (attached as .config)
compiler: score-elf-gcc (GCC) 4.9.4
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=score 

All errors (new ones prefixed by >>):

   net/xfrm/xfrm_device.c: In function 'xfrm_dev_register':
>> net/xfrm/xfrm_device.c:157:24: error: dereferencing pointer to incomplete type
           dev->xfrmdev_ops->xdo_dev_state_add &&
                           ^
   net/xfrm/xfrm_device.c:158:24: error: dereferencing pointer to incomplete type
           dev->xfrmdev_ops->xdo_dev_state_delete)) {
                           ^

vim +157 net/xfrm/xfrm_device.c

   144	
   145	static int xfrm_dev_register(struct net_device *dev)
   146	{
   147		if (!(dev->features & NETIF_F_HW_ESP)) {
   148			if (dev->features & NETIF_F_HW_ESP_TX_CSUM) {
   149				netdev_err(dev, "NETIF_F_HW_ESP_TX_CSUM without NETIF_F_HW_ESP\n");
   150				return NOTIFY_BAD;
   151			} else {
   152				return NOTIFY_DONE;
   153			}
   154		}
   155	
   156		if (!(dev->xfrmdev_ops &&
 > 157		      dev->xfrmdev_ops->xdo_dev_state_add &&
   158		      dev->xfrmdev_ops->xdo_dev_state_delete)) {
   159			netdev_err(dev, "add or delete function missing from xfrmdev_ops\n");
   160			return NOTIFY_BAD;
   161		}
   162	
   163		return NOTIFY_DONE;
   164	}
   165	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 8154 bytes --]

^ permalink raw reply

* Re: [PATCH] net: thunderx: add support for rgmii internal delay
From: Andrew Lunn @ 2017-12-14  8:45 UTC (permalink / raw)
  To: Tim Harvey; +Cc: Sunil Goutham, netdev
In-Reply-To: <CAJ+vNU3C2KWUBMU_-kMABwb8wfox_xV_3+9bivi=Adpd_vXCDg@mail.gmail.com>

On Wed, Dec 13, 2017 at 03:28:33PM -0800, Tim Harvey wrote:
> On Wed, Dec 13, 2017 at 3:10 AM, Andrew Lunn <andrew@lunn.ch> wrote:
> >> +void xcv_init_hw(int phy_mode)
> >>  {
> >>       u64  cfg;
> >>
> >> @@ -81,12 +81,31 @@ void xcv_init_hw(void)
> >>       /* Wait for DLL to lock */
> >>       msleep(1);
> >>
> >> -     /* Configure DLL - enable or bypass
> >> -      * TX no bypass, RX bypass
> >> -      */
> >> +     /* enable/bypass DLL providing MAC based internal TX/RX delays */
> >>       cfg = readq_relaxed(xcv->reg_base + XCV_DLL_CTL);
> >> -     cfg &= ~0xFF03;
> >> -     cfg |= CLKRX_BYP;
> >> +     cfg &= ~0xffff00;
> >> +     switch (phy_mode) {
> >> +     /* RX and TX delays are added by the MAC */
> >> +     case PHY_INTERFACE_MODE_RGMII:
> >> +             break;
> >> +     /* internal RX and TX delays provided by the PHY */
> >> +     case PHY_INTERFACE_MODE_RGMII_ID:
> >> +             cfg |= CLKRX_BYP;
> >> +             cfg |= CLKTX_BYP;
> >> +             break;
> >> +     /* internal RX delay provided by the PHY, the MAC
> >> +      * should not add an RX delay in this case
> >> +      */
> >> +     case PHY_INTERFACE_MODE_RGMII_RXID:
> >> +             cfg |= CLKRX_BYP;
> >> +             break;
> >> +     /* internal TX delay provided by the PHY, the MAC
> >> +      * should not add an TX delay in this case
> >> +      */
> >> +     case PHY_INTERFACE_MODE_RGMII_TXID:
> >> +             cfg |= CLKRX_BYP;
> >> +             break;
> >> +     }
> >
> > Hi Tim
> >
> > This i don't get. Normally, you leave the PHY to handle delays, if
> > needed. The MAC should not add any. Here you seem to assume a delay is
> > always needed, and if the PHY is not providing it, the MAC should.
> >
> >        Andrew
> 
> Andrew,
> 
> The thunder RGX inserts a delay via an on-board DLL. The 'bypass'
> register will bypass this DLL and not insert a delay from the MAC
> side. By default out of reset CLKTX_BYP=1 causing the RGX transmit
> interface to not introduce a delay and CLKRX_BYP=0 causing the RGX
> receive interface to introduce a delay.

Hi Tim

So the MAC by default is doing PHY_INTERFACE_MODE_RGMII_RXID.  And it
calls phy_connect_direct() passing PHY_INTERFACE_MODE_RGMII. It does
not get anything from device tree. So it looks like we have a chance
to clean this up.

So the correct thing to do is set the MAC to PHY_INTERFACE_MODE_RGMII,
i.e. no delays. By default call phy_connect_direct()
PHY_INTERFACE_MODE_RGMII_RXID. That should give you the same behaviour
as today.

Then add code to look in device tree, to find a per board setting. In
your case, you want PHY_INTERFACE_MODE_RGMII_ID. And make sure the PHY
driver respects the value passed.

       Andrew
 

^ permalink raw reply

* Re: [RFC PATCH] reuseport: compute the ehash only if needed
From: Paolo Abeni @ 2017-12-14  8:29 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, kraig, edumazet
In-Reply-To: <20171213.150855.2054919319089098824.davem@davemloft.net>

Hi,

On Wed, 2017-12-13 at 15:08 -0500, David Miller wrote:
> From: Paolo Abeni <pabeni@redhat.com>
> Date: Tue, 12 Dec 2017 14:09:28 +0100
> 
> > When a reuseport socket group is using a BPF filter to distribute
> > the packets among the sockets, we don't need to compute any hash
> > value, but the current reuseport_select_sock() requires the
> > caller to compute such hash in advance.
> > 
> > This patch reworks reuseport_select_sock() to compute the hash value
> > only if needed - missing or failing BPF filter. Since different
> > hash functions have different argument types - ipv4 addresses vs ipv6
> > ones - to avoid over-complicate the interface, reuseport_select_sock()
> > is now a macro.
> > 
> > Additionally, the sk_reuseport test is move inside reuseport_select_sock,
> > to avoid some code duplication.
> > 
> > Overall this gives small but measurable performance improvement
> > under UDP flood while using SO_REUSEPORT + BPF.
> > 
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> 
> I don't doubt that this improves the case where the hash is elided, but
> I suspect it makes things slower othewise.
> 
> You're doing two function calls for an operation that used to require
> just one in the bottom of the call chain.
> 
> You're also putting something onto the stack that the compiler can't
> possibly optimize into purely using cpu registers to hold.

Thank you for the feedback.

I was unable to measure any performance regression for the hash based
demultiplexing, and I think that the number of function calls is
unchanged in such scenario (with vanilla kernel we have ehash() and
reuseport_select_sock(), with the patched one __reuseport_get_info()
and ehash()). 

I agree you are right about the additional stack usage introduced by
this patch.

Overall I see we need something better than this.

Thanks,

Paolo

^ permalink raw reply

* Re: [PATCH net-next v4 4/5] bnx2x: Use NETIF_F_GRO_HW.
From: Michael Chan @ 2017-12-14  7:59 UTC (permalink / raw)
  To: Chopra, Manish
  Cc: davem@davemloft.net, netdev@vger.kernel.org,
	andrew.gospodarek@broadcom.com, Elior, Ariel,
	Dept-Eng Everest Linux L2
In-Reply-To: <BN3PR0701MB14122C945B07240DB4679FB9890A0@BN3PR0701MB1412.namprd07.prod.outlook.com>

On Wed, Dec 13, 2017 at 11:46 PM, Chopra, Manish
<Manish.Chopra@cavium.com> wrote:

>
> 2). In bnx2x_fix_features() we used to do before these changes -
>
>         /* TPA requires Rx CSUM offloading */
>         if (!(features & NETIF_F_RXCSUM)) {
>                 features &= ~NETIF_F_LRO;
>                 features &= ~NETIF_F_GRO;
>         }
>
> I think we should not turn off SW gro here, we should turn off HW gro here now ?

OK.  I will remove GRO.

GRO_HW will be turned off by netdev_fix_features() when RXCSUM is not on.

^ permalink raw reply

* RE: [PATCH net-next v4 4/5] bnx2x: Use NETIF_F_GRO_HW.
From: Chopra, Manish @ 2017-12-14  7:46 UTC (permalink / raw)
  To: Michael Chan
  Cc: davem@davemloft.net, netdev@vger.kernel.org,
	andrew.gospodarek@broadcom.com, Elior, Ariel,
	Dept-Eng Everest Linux L2
In-Reply-To: <CACKFLi=nXxt6CzZOySUMqcwwJyS5zdruFA7V0tKqLDds=osiUw@mail.gmail.com>

> -----Original Message-----
> From: Michael Chan [mailto:michael.chan@broadcom.com]
> Sent: Thursday, December 14, 2017 2:16 AM
> To: Chopra, Manish <Manish.Chopra@cavium.com>
> Cc: davem@davemloft.net; netdev@vger.kernel.org;
> andrew.gospodarek@broadcom.com; Elior, Ariel <Ariel.Elior@cavium.com>;
> Dept-Eng Everest Linux L2 <Dept-EngEverestLinuxL2@cavium.com>
> Subject: Re: [PATCH net-next v4 4/5] bnx2x: Use NETIF_F_GRO_HW.
> 
> On Wed, Dec 13, 2017 at 1:08 AM, Chopra, Manish
> <Manish.Chopra@cavium.com> wrote:
> >
> > Hi Michael,  There seems a behavioral change here. This driver support
> > two HW aggregation modes [LRO and GRO] With the changes, Interfaces
> come with HW GRO enabled and LRO disabled by default as opposed to earlier
> where interfaces used to come with LRO enabled.
> 
> Right.  Before, you had both NETIF_F_GRO and NETIF_F_LRO set and the code
> looked at NETIF_F_LRO first and turned on LRO.
> 
> Now, we set NETIF_F_GRO and NETIF_F_GRO_HW by default.  NETIF_F_LRO is
> turned off since NETIF_F_GRO_HW is on.
> 
> If you want, I can change it back to the old default.

Yes please, I think we should not make any default behavior change.
Few more comments -

1). This driver uses module param "disable_tpa" also to completely disable TPA [whether it be LRO or HW GRO] along the initialization flow.

For ex. 

/*Set TPA flags*/
if (bp->disable_tpa) {
        bp->dev->hw_features &= ~NETIF_F_LRO;
        bp->dev->features &= ~NETIF_F_LRO;
}

I think we should also disable HW gro here as well, so that device will come up with no TPA at all.

2). In bnx2x_fix_features() we used to do before these changes -

        /* TPA requires Rx CSUM offloading */
        if (!(features & NETIF_F_RXCSUM)) {
                features &= ~NETIF_F_LRO;
                features &= ~NETIF_F_GRO;
        }

I think we should not turn off SW gro here, we should turn off HW gro here now ?
	
> 
> >
> > Also, there seems some problem when turning on LRO even after GRO disable.
> > When I tried to disable GRO and then tried to enable LRO it didn't go well. Not
> sure why ?
> 
> I just put an old BCM57810 card into my machine and it works for me.
> As long as I turn off GRO or GRO_HW, I can turn on LRO.
> 
> [root@localhost bnx2x]# ethtool -K p1p1 lro on Cannot get device udp-
> fragmentation-offload settings: Operation not supported Cannot get device
> udp-fragmentation-offload settings: Operation not supported Could not change
> any device features [root@localhost bnx2x]# ethtool -K p1p1 gro off Cannot get
> device udp-fragmentation-offload settings: Operation not supported Cannot get
> device udp-fragmentation-offload settings: Operation not supported Actual
> changes:
> generic-receive-offload: off
> large-receive-offload: on
> rx-gro-hw: off [requested on]
> [root@localhost bnx2x]# ethtool -K p1p1 lro on Cannot get device udp-
> fragmentation-offload settings: Operation not supported Cannot get device
> udp-fragmentation-offload settings: Operation not supported

Doesn't sound like HW specific.
Probably, I was testing on your older series, I will check again on latest series.

^ permalink raw reply

* Re: [PATCH ipsec-next] xfrm: check for xdo_dev_state_free
From: kbuild test robot @ 2017-12-14  7:45 UTC (permalink / raw)
  To: Shannon Nelson; +Cc: kbuild-all, steffen.klassert, netdev
In-Reply-To: <1513025842-12064-1-git-send-email-shannon.nelson@oracle.com>

[-- Attachment #1: Type: text/plain, Size: 5842 bytes --]

Hi Shannon,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on ipsec-next/master]

url:    https://github.com/0day-ci/linux/commits/Shannon-Nelson/xfrm-check-for-xdo_dev_state_free/20171214-150202
base:   https://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec-next.git master
config: x86_64-randconfig-x002-201750 (attached as .config)
compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from include/linux/kernel.h:10:0,
                    from include/linux/list.h:9,
                    from include/linux/module.h:9,
                    from net/xfrm/xfrm_device.c:16:
   net/xfrm/xfrm_device.c: In function 'xfrm_dev_register':
   net/xfrm/xfrm_device.c:157:24: error: dereferencing pointer to incomplete type 'const struct xfrmdev_ops'
           dev->xfrmdev_ops->xdo_dev_state_add &&
                           ^
   include/linux/compiler.h:58:30: note: in definition of macro '__trace_if'
     if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
                                 ^~~~
>> net/xfrm/xfrm_device.c:156:2: note: in expansion of macro 'if'
     if (!(dev->xfrmdev_ops &&
     ^~

vim +/if +156 net/xfrm/xfrm_device.c

  > 16	#include <linux/module.h>
    17	#include <linux/netdevice.h>
    18	#include <linux/skbuff.h>
    19	#include <linux/slab.h>
    20	#include <linux/spinlock.h>
    21	#include <net/dst.h>
    22	#include <net/xfrm.h>
    23	#include <linux/notifier.h>
    24	
    25	#ifdef CONFIG_XFRM_OFFLOAD
    26	int validate_xmit_xfrm(struct sk_buff *skb, netdev_features_t features)
    27	{
    28		int err;
    29		struct xfrm_state *x;
    30		struct xfrm_offload *xo = xfrm_offload(skb);
    31	
    32		if (skb_is_gso(skb))
    33			return 0;
    34	
    35		if (xo) {
    36			x = skb->sp->xvec[skb->sp->len - 1];
    37			if (xo->flags & XFRM_GRO || x->xso.flags & XFRM_OFFLOAD_INBOUND)
    38				return 0;
    39	
    40			x->outer_mode->xmit(x, skb);
    41	
    42			err = x->type_offload->xmit(x, skb, features);
    43			if (err) {
    44				XFRM_INC_STATS(xs_net(x), LINUX_MIB_XFRMOUTSTATEPROTOERROR);
    45				return err;
    46			}
    47	
    48			skb_push(skb, skb->data - skb_mac_header(skb));
    49		}
    50	
    51		return 0;
    52	}
    53	EXPORT_SYMBOL_GPL(validate_xmit_xfrm);
    54	
    55	int xfrm_dev_state_add(struct net *net, struct xfrm_state *x,
    56			       struct xfrm_user_offload *xuo)
    57	{
    58		int err;
    59		struct dst_entry *dst;
    60		struct net_device *dev;
    61		struct xfrm_state_offload *xso = &x->xso;
    62		xfrm_address_t *saddr;
    63		xfrm_address_t *daddr;
    64	
    65		if (!x->type_offload)
    66			return -EINVAL;
    67	
    68		/* We don't yet support UDP encapsulation, TFC padding and ESN. */
    69		if (x->encap || x->tfcpad || (x->props.flags & XFRM_STATE_ESN))
    70			return -EINVAL;
    71	
    72		dev = dev_get_by_index(net, xuo->ifindex);
    73		if (!dev) {
    74			if (!(xuo->flags & XFRM_OFFLOAD_INBOUND)) {
    75				saddr = &x->props.saddr;
    76				daddr = &x->id.daddr;
    77			} else {
    78				saddr = &x->id.daddr;
    79				daddr = &x->props.saddr;
    80			}
    81	
    82			dst = __xfrm_dst_lookup(net, 0, 0, saddr, daddr,
    83						x->props.family, x->props.output_mark);
    84			if (IS_ERR(dst))
    85				return 0;
    86	
    87			dev = dst->dev;
    88	
    89			dev_hold(dev);
    90			dst_release(dst);
    91		}
    92	
    93		if (!dev->xfrmdev_ops || !dev->xfrmdev_ops->xdo_dev_state_add) {
    94			xso->dev = NULL;
    95			dev_put(dev);
    96			return 0;
    97		}
    98	
    99		xso->dev = dev;
   100		xso->num_exthdrs = 1;
   101		xso->flags = xuo->flags;
   102	
   103		err = dev->xfrmdev_ops->xdo_dev_state_add(x);
   104		if (err) {
   105			dev_put(dev);
   106			return err;
   107		}
   108	
   109		return 0;
   110	}
   111	EXPORT_SYMBOL_GPL(xfrm_dev_state_add);
   112	
   113	bool xfrm_dev_offload_ok(struct sk_buff *skb, struct xfrm_state *x)
   114	{
   115		int mtu;
   116		struct dst_entry *dst = skb_dst(skb);
   117		struct xfrm_dst *xdst = (struct xfrm_dst *)dst;
   118		struct net_device *dev = x->xso.dev;
   119	
   120		if (!x->type_offload || x->encap)
   121			return false;
   122	
   123		if ((x->xso.offload_handle && (dev == dst->path->dev)) &&
   124		     !dst->child->xfrm && x->type->get_mtu) {
   125			mtu = x->type->get_mtu(x, xdst->child_mtu_cached);
   126	
   127			if (skb->len <= mtu)
   128				goto ok;
   129	
   130			if (skb_is_gso(skb) && skb_gso_validate_mtu(skb, mtu))
   131				goto ok;
   132		}
   133	
   134		return false;
   135	
   136	ok:
   137		if (dev && dev->xfrmdev_ops && dev->xfrmdev_ops->xdo_dev_offload_ok)
   138			return x->xso.dev->xfrmdev_ops->xdo_dev_offload_ok(skb, x);
   139	
   140		return true;
   141	}
   142	EXPORT_SYMBOL_GPL(xfrm_dev_offload_ok);
   143	#endif
   144	
   145	static int xfrm_dev_register(struct net_device *dev)
   146	{
   147		if (!(dev->features & NETIF_F_HW_ESP)) {
   148			if (dev->features & NETIF_F_HW_ESP_TX_CSUM) {
   149				netdev_err(dev, "NETIF_F_HW_ESP_TX_CSUM without NETIF_F_HW_ESP\n");
   150				return NOTIFY_BAD;
   151			} else {
   152				return NOTIFY_DONE;
   153			}
   154		}
   155	
 > 156		if (!(dev->xfrmdev_ops &&
   157		      dev->xfrmdev_ops->xdo_dev_state_add &&
   158		      dev->xfrmdev_ops->xdo_dev_state_delete)) {
   159			netdev_err(dev, "add or delete function missing from xfrmdev_ops\n");
   160			return NOTIFY_BAD;
   161		}
   162	
   163		return NOTIFY_DONE;
   164	}
   165	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30405 bytes --]

^ permalink raw reply

* Re: [PATCH v3 net-next 6/9] net: Generic resolver backend
From: kbuild test robot @ 2017-12-14  7:16 UTC (permalink / raw)
  To: Tom Herbert; +Cc: kbuild-all, davem, netdev, roopa, rohit, Tom Herbert
In-Reply-To: <20171211203837.2540-7-tom@quantonium.net>

[-- Attachment #1: Type: text/plain, Size: 1731 bytes --]

Hi Tom,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Tom-Herbert/net-Generic-network-resolver-backend-and-ILA-resolver/20171214-142440
config: i386-randconfig-x016-201750 (attached as .config)
compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

>> net/Kconfig:404: warning: menuconfig statement without prompt

vim +404 net/Kconfig

3908c690 Sjur Braendeland 2010-03-30  403  
499a2425 Roopa Prabhu     2015-07-21 @404  config LWTUNNEL
499a2425 Roopa Prabhu     2015-07-21  405  	bool "Network light weight tunnels"
499a2425 Roopa Prabhu     2015-07-21  406  	---help---
499a2425 Roopa Prabhu     2015-07-21  407  	  This feature provides an infrastructure to support light weight
499a2425 Roopa Prabhu     2015-07-21  408  	  tunnels like mpls. There is no netdevice associated with a light
499a2425 Roopa Prabhu     2015-07-21  409  	  weight tunnel endpoint. Tunnel encapsulation parameters are stored
499a2425 Roopa Prabhu     2015-07-21  410  	  with light weight tunnel state associated with fib routes.
cf4328cd Ivo van Doorn    2007-05-07  411  

:::::: The code at line 404 was first introduced by commit
:::::: 499a24256862714539e902c0499b67da2bb3ab72 lwtunnel: infrastructure for handling light weight tunnels like mpls

:::::: TO: Roopa Prabhu <roopa@cumulusnetworks.com>
:::::: CC: David S. Miller <davem@davemloft.net>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24002 bytes --]

^ permalink raw reply

* Re: [PATCH v9 0/5] Add the ability to do BPF directed error injection
From: Masami Hiramatsu @ 2017-12-14  6:46 UTC (permalink / raw)
  To: Josef Bacik
  Cc: Darrick J. Wong, Masami Hiramatsu, rostedt, mingo, davem, netdev,
	linux-kernel, ast, kernel-team, daniel, linux-btrfs
In-Reply-To: <20171213185744.f5ysznzs3drmp6ef@destiny>

On Wed, 13 Dec 2017 13:57:45 -0500
Josef Bacik <josef@toxicpanda.com> wrote:

> On Wed, Dec 13, 2017 at 10:07:32AM -0800, Darrick J. Wong wrote:
> > On Wed, Dec 13, 2017 at 01:03:57PM -0500, Josef Bacik wrote:
> > > On Tue, Dec 12, 2017 at 03:11:50PM -0800, Darrick J. Wong wrote:
> > > > On Mon, Dec 11, 2017 at 11:36:45AM -0500, Josef Bacik wrote:
> > > > > This is the same as v8, just rebased onto the bpf tree.
> > > > > 
> > > > > v8->v9:
> > > > > - rebased onto the bpf tree.
> > > > > 
> > > > > v7->v8:
> > > > > - removed the _ASM_KPROBE_ERROR_INJECT since it was not needed.
> > > > > 
> > > > > v6->v7:
> > > > > - moved the opt-in macro to bpf.h out of kprobes.h.
> > > > > 
> > > > > v5->v6:
> > > > > - add BPF_ALLOW_ERROR_INJECTION() tagging for functions that will support this
> > > > >   feature.  This way only functions that opt-in will be allowed to be
> > > > >   overridden.
> > > > > - added a btrfs patch to allow error injection for open_ctree() so that the bpf
> > > > >   sample actually works.
> > > > > 
> > > > > v4->v5:
> > > > > - disallow kprobe_override programs from being put in the prog map array so we
> > > > >   don't tail call into something we didn't check.  This allows us to make the
> > > > >   normal path still fast without a bunch of percpu operations.
> > > > > 
> > > > > v3->v4:
> > > > > - fix a build error found by kbuild test bot (I didn't wait long enough
> > > > >   apparently.)
> > > > > - Added a warning message as per Daniels suggestion.
> > > > > 
> > > > > v2->v3:
> > > > > - added a ->kprobe_override flag to bpf_prog.
> > > > > - added some sanity checks to disallow attaching bpf progs that have
> > > > >   ->kprobe_override set that aren't for ftrace kprobes.
> > > > > - added the trace_kprobe_ftrace helper to check if the trace_event_call is a
> > > > >   ftrace kprobe.
> > > > > - renamed bpf_kprobe_state to bpf_kprobe_override, fixed it so we only read this
> > > > >   value in the kprobe path, and thus only write to it if we're overriding or
> > > > >   clearing the override.
> > > > > 
> > > > > v1->v2:
> > > > > - moved things around to make sure that bpf_override_return could really only be
> > > > >   used for an ftrace kprobe.
> > > > > - killed the special return values from trace_call_bpf.
> > > > > - renamed pc_modified to bpf_kprobe_state so bpf_override_return could tell if
> > > > >   it was being called from an ftrace kprobe context.
> > > > > - reworked the logic in kprobe_perf_func to take advantage of bpf_kprobe_state.
> > > > > - updated the test as per Alexei's review.
> > > > > 
> > > > > - Original message -
> > > > > 
> > > > > A lot of our error paths are not well tested because we have no good way of
> > > > > injecting errors generically.  Some subystems (block, memory) have ways to
> > > > > inject errors, but they are random so it's hard to get reproduceable results.
> > > > > 
> > > > > With BPF we can add determinism to our error injection.  We can use kprobes and
> > > > > other things to verify we are injecting errors at the exact case we are trying
> > > > > to test.  This patch gives us the tool to actual do the error injection part.
> > > > > It is very simple, we just set the return value of the pt_regs we're given to
> > > > > whatever we provide, and then override the PC with a dummy function that simply
> > > > > returns.
> > > > 
> > > > Heh, this looks cool.  I decided to try it to see what happens, and saw
> > > > a bunch of dmesg pasted in below.  Is that supposed to happen?  Or am I
> > > > the only fs developer still running with lockdep enabled? :)
> > > > 
> > > > It looks like bpf_override_return has some sort of side effect such that
> > > > we get the splat, since commenting it out makes the symptom go away.
> > > > 
> > > > <shrug>
> > > > 
> > > > --D
> > > > 
> > > > [ 1847.769183] BTRFS error (device (null)): open_ctree failed
> > > > [ 1847.770130] BUG: sleeping function called from invalid context at /storage/home/djwong/cdev/work/linux-xfs/kernel/locking/rwsem.c:69
> > > > [ 1847.771976] in_atomic(): 1, irqs_disabled(): 0, pid: 1524, name: mount
> > > > [ 1847.773016] 1 lock held by mount/1524:
> > > > [ 1847.773530]  #0:  (&type->s_umount_key#34/1){+.+.}, at: [<00000000653a9bb4>] sget_userns+0x302/0x4f0
> > > > [ 1847.774731] Preemption disabled at:
> > > > [ 1847.774735] [<          (null)>]           (null)
> > > > [ 1847.777009] CPU: 2 PID: 1524 Comm: mount Tainted: G        W        4.15.0-rc3-xfsx #3
> > > > [ 1847.778800] Call Trace:
> > > > [ 1847.779047]  dump_stack+0x7c/0xbe
> > > > [ 1847.779361]  ___might_sleep+0x1f7/0x260
> > > > [ 1847.779720]  down_write+0x29/0xb0
> > > > [ 1847.780046]  unregister_shrinker+0x15/0x70
> > > > [ 1847.780427]  deactivate_locked_super+0x2e/0x60
> > > > [ 1847.780935]  btrfs_mount+0xbb6/0x1000 [btrfs]
> > > > [ 1847.781353]  ? __lockdep_init_map+0x5c/0x1d0
> > > > [ 1847.781750]  ? mount_fs+0xf/0x80
> > > > [ 1847.782065]  ? alloc_vfsmnt+0x1a1/0x230
> > > > [ 1847.782429]  mount_fs+0xf/0x80
> > > > [ 1847.782733]  vfs_kern_mount+0x62/0x160
> > > > [ 1847.783128]  btrfs_mount+0x3d3/0x1000 [btrfs]
> > > > [ 1847.783493]  ? __lockdep_init_map+0x5c/0x1d0
> > > > [ 1847.783849]  ? __lockdep_init_map+0x5c/0x1d0
> > > > [ 1847.784207]  ? mount_fs+0xf/0x80
> > > > [ 1847.784502]  mount_fs+0xf/0x80
> > > > [ 1847.784835]  vfs_kern_mount+0x62/0x160
> > > > [ 1847.785235]  do_mount+0x1b1/0xd50
> > > > [ 1847.785594]  ? _copy_from_user+0x5b/0x90
> > > > [ 1847.786028]  ? memdup_user+0x4b/0x70
> > > > [ 1847.786501]  SyS_mount+0x85/0xd0
> > > > [ 1847.786835]  entry_SYSCALL_64_fastpath+0x1f/0x96
> > > > [ 1847.787311] RIP: 0033:0x7f6ebecc1b5a
> > > > [ 1847.787691] RSP: 002b:00007ffc7bd1c958 EFLAGS: 00000202 ORIG_RAX: 00000000000000a5
> > > > [ 1847.788383] RAX: ffffffffffffffda RBX: 00007f6ebefba63a RCX: 00007f6ebecc1b5a
> > > > [ 1847.789106] RDX: 0000000000bfd010 RSI: 0000000000bfa230 RDI: 0000000000bfa210
> > > > [ 1847.789807] RBP: 0000000000bfa0f0 R08: 0000000000000000 R09: 0000000000000014
> > > > [ 1847.790511] R10: 00000000c0ed0000 R11: 0000000000000202 R12: 00007f6ebf1ca83c
> > > > [ 1847.791211] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000001
> > > > [ 1847.792029] BUG: scheduling while atomic: mount/1524/0x00000002
> > > > [ 1847.792680] 1 lock held by mount/1524:
> > > > [ 1847.793087]  #0:  (rcu_preempt_state.exp_mutex){+.+.}, at: [<00000000a6c536a9>] _synchronize_rcu_expedited+0x1ce/0x400
> > > > [ 1847.794129] Modules linked in: xfs libcrc32c btrfs xor zstd_decompress zstd_compress xxhash lzo_compress lzo_decompress zlib_deflate raid6_pq dax_pmem device_dax nd_pmem sch_fq_codel af_packet [last unloaded: xfs]
> > > > [ 1847.795949] Preemption disabled at:
> > > > [ 1847.795951] [<          (null)>]           (null)
> > > > [ 1847.796844] CPU: 2 PID: 1524 Comm: mount Tainted: G        W        4.15.0-rc3-xfsx #3
> > > > [ 1847.797621] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.10.2-1ubuntu1djwong0 04/01/2014
> > > > [ 1847.798510] Call Trace:
> > > > [ 1847.798786]  dump_stack+0x7c/0xbe
> > > > [ 1847.799134]  __schedule_bug+0x88/0xe0
> > > > [ 1847.799517]  __schedule+0x78c/0xb20
> > > > [ 1847.799890]  ? trace_hardirqs_on_caller+0x119/0x180
> > > > [ 1847.800391]  schedule+0x40/0x90
> > > > [ 1847.800729]  _synchronize_rcu_expedited+0x36b/0x400
> > > > [ 1847.801218]  ? rcu_preempt_qs+0xa0/0xa0
> > > > [ 1847.801616]  ? remove_wait_queue+0x60/0x60
> > > > [ 1847.802040]  ? rcu_preempt_qs+0xa0/0xa0
> > > > [ 1847.802433]  ? rcu_exp_wait_wake+0x630/0x630
> > > > [ 1847.802872]  ? __lock_acquire+0xfb9/0x1120
> > > > [ 1847.803302]  ? __lock_acquire+0x534/0x1120
> > > > [ 1847.803725]  ? bdi_unregister+0x57/0x1a0
> > > > [ 1847.804135]  bdi_unregister+0x5c/0x1a0
> > > > [ 1847.804519]  bdi_put+0xcb/0xe0
> > > > [ 1847.804746]  generic_shutdown_super+0xe2/0x110
> > > > [ 1847.805066]  kill_anon_super+0xe/0x20
> > > > [ 1847.805344]  btrfs_kill_super+0x12/0xa0 [btrfs]
> > > > [ 1847.805664]  deactivate_locked_super+0x34/0x60
> > > > [ 1847.806111]  btrfs_mount+0xbb6/0x1000 [btrfs]
> > > > [ 1847.806476]  ? __lockdep_init_map+0x5c/0x1d0
> > > > [ 1847.806824]  ? mount_fs+0xf/0x80
> > > > [ 1847.807104]  ? alloc_vfsmnt+0x1a1/0x230
> > > > [ 1847.807416]  mount_fs+0xf/0x80
> > > > [ 1847.807712]  vfs_kern_mount+0x62/0x160
> > > > [ 1847.808112]  btrfs_mount+0x3d3/0x1000 [btrfs]
> > > > [ 1847.808565]  ? __lockdep_init_map+0x5c/0x1d0
> > > > [ 1847.809005]  ? __lockdep_init_map+0x5c/0x1d0
> > > > [ 1847.809425]  ? mount_fs+0xf/0x80
> > > > [ 1847.809731]  mount_fs+0xf/0x80
> > > > [ 1847.810070]  vfs_kern_mount+0x62/0x160
> > > > [ 1847.810469]  do_mount+0x1b1/0xd50
> > > > [ 1847.810821]  ? _copy_from_user+0x5b/0x90
> > > > [ 1847.811237]  ? memdup_user+0x4b/0x70
> > > > [ 1847.811622]  SyS_mount+0x85/0xd0
> > > > [ 1847.811996]  entry_SYSCALL_64_fastpath+0x1f/0x96
> > > > [ 1847.812465] RIP: 0033:0x7f6ebecc1b5a
> > > > [ 1847.812840] RSP: 002b:00007ffc7bd1c958 EFLAGS: 00000202 ORIG_RAX: 00000000000000a5
> > > > [ 1847.813615] RAX: ffffffffffffffda RBX: 00007f6ebefba63a RCX: 00007f6ebecc1b5a
> > > > [ 1847.814302] RDX: 0000000000bfd010 RSI: 0000000000bfa230 RDI: 0000000000bfa210
> > > > [ 1847.814770] RBP: 0000000000bfa0f0 R08: 0000000000000000 R09: 0000000000000014
> > > > [ 1847.815246] R10: 00000000c0ed0000 R11: 0000000000000202 R12: 00007f6ebf1ca83c
> > > > [ 1847.815720] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000001
> > > i> 
> > > 
> > > Looks like this is new, Masami this is happening because of your change here
> > > 
> > > 5bb4fc2d8641 ("kprobes/x86: Disable preemption in ftrace-based jprobes")
> > > 
> > > which makes it not do the preempt_enable() if the handler returns 1.  Why is
> > > that?

Yes, because this (return 1) is expected to be done only by jprobe.

> > >  Should I be doing preempt_enable_no_resched() from the handler before
> > > returning 1? Or is this just an oversight on your part?  Thanks,

Yes, or you have to hook after return path and fixup preempt count as
jprobe did.

(And now jprobe is coming to an end.)

> > 
> > FWIW I shut up the preemption imbalance warnings with the attached
> > coarse bandaid.  No idea if that's the correct fix...

No, this is not correct way to fix this issue.
I guess your BPF extention is trying to change instrunction pointer to
another address (right?). If so, you have to carefully do followings
before returning to modified address.

- reset current_kprobes
- call preempt_enable_no_resched()


> > 
> > --D
> > 
> > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> > index 5db8498..fd948e3 100644
> > --- a/kernel/trace/trace_kprobe.c
> > +++ b/kernel/trace/trace_kprobe.c
> > @@ -1215,8 +1215,10 @@ kprobe_perf_func(struct trace_kprobe *tk, struct pt_regs *regs)
> >  		if (__this_cpu_read(bpf_kprobe_override)) {
> >  			__this_cpu_write(bpf_kprobe_override, 0);
> >  			reset_current_kprobe();
> > +			preempt_enable();
> >  			return 1;
> >  		}
> > +		preempt_enable();
> >  		if (!ret)
> >  			return 0;
> >  	}
> 
> Yeah I'd like to avoid doing this and know why exactly we leave a unpaired
> preempt_disable() in kprobe_ftrace_handler() so we don't do something like this
> only to have the handler change again in the future and break us again.  Thanks,

Ah, I see. kprobe_perf_func invokes BPF and BPF changes instruction address.
In that case, only what you need is adding preempt_enable_no_resched() at
right after the reset_current_kprobe().

Anyway, Could you CC the series to me?

Thank you,


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply

* Re: [bpf-next V1-RFC PATCH 02/14] xdp/mlx5: setup xdp_rxq_info and extend with qtype
From: Jesper Dangaard Brouer @ 2017-12-14  6:46 UTC (permalink / raw)
  To: Saeed Mahameed
  Cc: Tariq Toukan, Daniel Borkmann, Alexei Starovoitov, netdev,
	dsahern, Matan Barak, gospo, bjorn.topel, michael.chan, brouer
In-Reply-To: <19c7c7f3-2db5-6f90-648f-78e8da92d862@mellanox.com>


On Wed, 13 Dec 2017 15:03:33 -0800 Saeed Mahameed <saeedm@mellanox.com> wrote:

> >>> @@ -707,6 +714,8 @@ static void mlx5e_free_rq(struct mlx5e_rq *rq)
> >>>    	if (rq->xdp_prog)
> >>>    		bpf_prog_put(rq->xdp_prog);
> >>>    
> >>> +	xdp_rxq_info_unreg(&rq->xdp_rxq);
> >>> +
> >>>    	switch (rq->wq_type) {
> >>>    	case MLX5_WQ_TYPE_LINKED_LIST_STRIDING_RQ:
> >>>    		mlx5e_rq_free_mpwqe_info(rq);
> >>> @@ -2768,6 +2777,11 @@ static int mlx5e_alloc_drop_rq(struct
> >>> mlx5_core_dev *mdev, if (err)
> >>>    		return err;
> >>>    
> >>> +	/* XDP RX-queue info for "Drop-RQ", packets never reach
> >>> XDP */
> >>> +	xdp_rxq_info_init(&rq->xdp_rxq);
> >>> +	xdp_rxq_info_type(&rq->xdp_rxq, RXQ_TYPE_SINK);
> >>> +	xdp_rxq_info_reg(&rq->xdp_rxq);
> >>> +  
> 
> I don't see why you need this, This RQ is not even assigned to any 
> netdev_rxq! it is a pure HW object that drops traffic in HW when netdev 
> is down, it even has no buffers or napi handling, just ignore it's 
> existence for the sake of mlx5 xdp_rxq_info reg/unreg stuff and remove 
> RXQ_TYPE_SINK, bottom line it is not a real RQ and for sure XDP has 
> nothing to do with it.

I need it here, because the take-down/free code-path is the same for
these two types of RQ's.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* Re: [PATCH ipsec-next] xfrm: check for xdo_dev_state_free
From: Steffen Klassert @ 2017-12-14  6:20 UTC (permalink / raw)
  To: Shannon Nelson; +Cc: netdev
In-Reply-To: <1513025842-12064-1-git-send-email-shannon.nelson@oracle.com>

On Mon, Dec 11, 2017 at 12:57:22PM -0800, Shannon Nelson wrote:
> The current XFRM code assumes that we've implemented the
> xdo_dev_state_free() callback, even if it is meaningless to the driver.
> This patch adds a check for it before calling, as done in other APIs,
> and is done for the xdo_state_offload_ok() callback.
> 
> Also, we add a check for the required add and delete functions up front
> at registration time to be sure both are defined, and complain if not.
> 
> Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
> ---
>  include/net/xfrm.h     |  3 ++-
>  net/xfrm/xfrm_device.c | 18 ++++++++++++++----
>  2 files changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/include/net/xfrm.h b/include/net/xfrm.h
> index e015e16..dfabd04 100644
> --- a/include/net/xfrm.h
> +++ b/include/net/xfrm.h
> @@ -1891,7 +1891,8 @@ static inline void xfrm_dev_state_free(struct xfrm_state *x)
>  	 struct net_device *dev = xso->dev;
>  
>  	if (dev && dev->xfrmdev_ops) {
> -		dev->xfrmdev_ops->xdo_dev_state_free(x);
> +		if (dev->xfrmdev_ops->xdo_dev_state_free)
> +			dev->xfrmdev_ops->xdo_dev_state_free(x);
>  		xso->dev = NULL;
>  		dev_put(dev);
>  	}
> diff --git a/net/xfrm/xfrm_device.c b/net/xfrm/xfrm_device.c
> index 30e5746..0df1cc2 100644
> --- a/net/xfrm/xfrm_device.c
> +++ b/net/xfrm/xfrm_device.c
> @@ -144,11 +144,21 @@ EXPORT_SYMBOL_GPL(xfrm_dev_offload_ok);
>  
>  static int xfrm_dev_register(struct net_device *dev)
>  {
> -	if ((dev->features & NETIF_F_HW_ESP) && !dev->xfrmdev_ops)
> -		return NOTIFY_BAD;
> -	if ((dev->features & NETIF_F_HW_ESP_TX_CSUM) &&
> -	    !(dev->features & NETIF_F_HW_ESP))
> +	if (!(dev->features & NETIF_F_HW_ESP)) {
> +		if (dev->features & NETIF_F_HW_ESP_TX_CSUM) {
> +			netdev_err(dev, "NETIF_F_HW_ESP_TX_CSUM without NETIF_F_HW_ESP\n");
> +			return NOTIFY_BAD;
> +		} else {
> +			return NOTIFY_DONE;
> +		}
> +	}
> +
> +	if (!(dev->xfrmdev_ops &&
> +	      dev->xfrmdev_ops->xdo_dev_state_add &&
> +	      dev->xfrmdev_ops->xdo_dev_state_delete)) {
> +		netdev_err(dev, "add or delete function missing from xfrmdev_ops\n");

Please remove these error printings, this is not relevant for normal
users.

^ permalink raw reply

* Re: [PATCH 1/2] hp100: Fix a possible sleep-in-atomic bug in hp100_login_to_vg_hub
From: Jia-Ju Bai @ 2017-12-14  3:56 UTC (permalink / raw)
  To: David Miller; +Cc: perex, floeff, acme, netdev, linux-kernel
In-Reply-To: <20171213.223432.393103915049464102.davem@davemloft.net>

Sorry,

I think I know your meaning now.

Maybe we can unlock the spinlock before "schedule_timeout_interruptible" 
and then lock again?
Like:
     spin_unlock(...);
     schedule_timeout_interruptible(1);
     spin_lock(...);


Best wishes,
Jia-Ju Bai


On 2017/12/14 11:34, David Miller wrote:
> From: Jia-Ju Bai <baijiaju1990@gmail.com>
> Date: Thu, 14 Dec 2017 11:13:15 +0800
>
>> Thanks for reply :)
>> I think I should use "udelay(100000/HZ)" instead, do you think it is
>> right?
> The delay is too long, please do not ignore that part of my critique
> of your change.
>
> You cannot delay so long under a lock, that's why the code is trying
> to use a sleeping delay.
>
> I'm not going to explain this problem another time.

^ permalink raw reply

* Re: [PATCH 1/2] hp100: Fix a possible sleep-in-atomic bug in hp100_login_to_vg_hub
From: David Miller @ 2017-12-14  3:34 UTC (permalink / raw)
  To: baijiaju1990; +Cc: perex, floeff, acme, netdev, linux-kernel
In-Reply-To: <18472c42-4eaf-7189-a16f-489a706cba3e@gmail.com>

From: Jia-Ju Bai <baijiaju1990@gmail.com>
Date: Thu, 14 Dec 2017 11:13:15 +0800

> Thanks for reply :)
> I think I should use "udelay(100000/HZ)" instead, do you think it is
> right?

The delay is too long, please do not ignore that part of my critique
of your change.

You cannot delay so long under a lock, that's why the code is trying
to use a sleeping delay.

I'm not going to explain this problem another time.

^ permalink raw reply

* Re: [PATCH 1/2] hp100: Fix a possible sleep-in-atomic bug in hp100_login_to_vg_hub
From: Jia-Ju Bai @ 2017-12-14  3:31 UTC (permalink / raw)
  To: David Miller; +Cc: perex, floeff, acme, netdev, linux-kernel
In-Reply-To: <18472c42-4eaf-7189-a16f-489a706cba3e@gmail.com>

Sorry, I made a mistake in last e-mail.

Maybe "mdelay(1000/HZ)" or "udelay(1000000/HZ)" .
Which one do you think is right?


Thanks,
Jia-Ju Bai

On 2017/12/14 11:13, Jia-Ju Bai wrote:
> Thanks for reply :)
> I think I should use "udelay(100000/HZ)" instead, do you think it is 
> right?
>
>
> Thanks,
> Jia-Ju Bai
>
>
> On 2017/12/14 5:20, David Miller wrote:
>> I want you to review all of your patches and resend them after you
>> have checked them carefully.
>>
>> The first patch I even looked at, this one, is buggy.
>>
>> You changed a schedule_timeout_interruptible(1) into a udelay(10)
>>
>> That's not right.
>>
>> schedule_timeout_interruptible() takes a "jiffies" argument, which
>> is a completely different unit than udelay() takes.  You would have
>> to scale the argument to udelay() in some way using HZ.
>>
>> Furthermore, the udelay argument you would come up with would
>> be way too long to be appropirate in this atomic context.
>>
>> That's why the code tries to use a sleeping timeout, a long wait is
>> necessary here.
>

^ permalink raw reply

* [PATCH v4 13/14] nubus: Add expansion_type values for various Mac models
From: Finn Thain @ 2017-12-14  3:16 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: linux-m68k, linux-kernel, David S. Miller, netdev
In-Reply-To: <cover.1513220582.git.fthain@telegraphics.com.au>

Add an expansion slot attribute to allow drivers to properly handle
cards like Comm Slot cards and PDS cards without declaration ROMs.
This clarifies the logic for the Centris 610 model which has no
Comm Slot but has an optional on-board SONIC device.

Cc: "David S. Miller" <davem@davemloft.net>
Tested-by: Stan Johnson <userm57@yahoo.com>
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
 arch/m68k/include/asm/macintosh.h       |   9 ++-
 arch/m68k/mac/config.c                  | 110 +++++++++++++-------------------
 drivers/net/ethernet/natsemi/macsonic.c |   8 +--
 3 files changed, 54 insertions(+), 73 deletions(-)

diff --git a/arch/m68k/include/asm/macintosh.h b/arch/m68k/include/asm/macintosh.h
index f42c27400dbc..9b840c03ebb7 100644
--- a/arch/m68k/include/asm/macintosh.h
+++ b/arch/m68k/include/asm/macintosh.h
@@ -33,7 +33,7 @@ struct mac_model
 	char ide_type;
 	char scc_type;
 	char ether_type;
-	char nubus_type;
+	char expansion_type;
 	char floppy_type;
 };
 
@@ -73,8 +73,11 @@ struct mac_model
 #define MAC_ETHER_SONIC		1
 #define MAC_ETHER_MACE		2
 
-#define MAC_NO_NUBUS		0
-#define MAC_NUBUS		1
+#define MAC_EXP_NONE		0
+#define MAC_EXP_PDS		1 /* Accepts only a PDS card */
+#define MAC_EXP_NUBUS		2 /* Accepts only NuBus card(s) */
+#define MAC_EXP_PDS_NUBUS	3 /* Accepts PDS card and/or NuBus card(s) */
+#define MAC_EXP_PDS_COMM	4 /* Accepts PDS card or Comm Slot card */
 
 #define MAC_FLOPPY_IWM		0
 #define MAC_FLOPPY_SWIM_ADDR1	1
diff --git a/arch/m68k/mac/config.c b/arch/m68k/mac/config.c
index 16cd5cea5207..d3d435248a24 100644
--- a/arch/m68k/mac/config.c
+++ b/arch/m68k/mac/config.c
@@ -212,7 +212,7 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_II,
 		.scsi_type	= MAC_SCSI_OLD,
 		.scc_type	= MAC_SCC_II,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_NUBUS,
 		.floppy_type	= MAC_FLOPPY_IWM,
 	},
 
@@ -227,7 +227,7 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_II,
 		.scsi_type	= MAC_SCSI_OLD,
 		.scc_type	= MAC_SCC_II,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_NUBUS,
 		.floppy_type	= MAC_FLOPPY_IWM,
 	}, {
 		.ident		= MAC_MODEL_IIX,
@@ -236,7 +236,7 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_II,
 		.scsi_type	= MAC_SCSI_OLD,
 		.scc_type	= MAC_SCC_II,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	}, {
 		.ident		= MAC_MODEL_IICX,
@@ -245,7 +245,7 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_II,
 		.scsi_type	= MAC_SCSI_OLD,
 		.scc_type	= MAC_SCC_II,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	}, {
 		.ident		= MAC_MODEL_SE30,
@@ -254,7 +254,7 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_II,
 		.scsi_type	= MAC_SCSI_OLD,
 		.scc_type	= MAC_SCC_II,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_PDS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	},
 
@@ -272,7 +272,7 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_IICI,
 		.scsi_type	= MAC_SCSI_OLD,
 		.scc_type	= MAC_SCC_II,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	}, {
 		.ident		= MAC_MODEL_IIFX,
@@ -281,7 +281,7 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_IICI,
 		.scsi_type	= MAC_SCSI_IIFX,
 		.scc_type	= MAC_SCC_IOP,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_PDS_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_IOP,
 	}, {
 		.ident		= MAC_MODEL_IISI,
@@ -290,7 +290,7 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_IICI,
 		.scsi_type	= MAC_SCSI_OLD,
 		.scc_type	= MAC_SCC_II,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_PDS_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	}, {
 		.ident		= MAC_MODEL_IIVI,
@@ -299,7 +299,7 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_IICI,
 		.scsi_type	= MAC_SCSI_LC,
 		.scc_type	= MAC_SCC_II,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	}, {
 		.ident		= MAC_MODEL_IIVX,
@@ -308,7 +308,7 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_IICI,
 		.scsi_type	= MAC_SCSI_LC,
 		.scc_type	= MAC_SCC_II,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	},
 
@@ -323,7 +323,6 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_IICI,
 		.scsi_type	= MAC_SCSI_LC,
 		.scc_type	= MAC_SCC_II,
-		.nubus_type	= MAC_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	}, {
 		.ident		= MAC_MODEL_CCL,
@@ -332,7 +331,7 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_IICI,
 		.scsi_type	= MAC_SCSI_LC,
 		.scc_type	= MAC_SCC_II,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_PDS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	}, {
 		.ident		= MAC_MODEL_CCLII,
@@ -341,7 +340,7 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_IICI,
 		.scsi_type	= MAC_SCSI_LC,
 		.scc_type	= MAC_SCC_II,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_PDS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	},
 
@@ -356,7 +355,7 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_IICI,
 		.scsi_type	= MAC_SCSI_LC,
 		.scc_type	= MAC_SCC_II,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_PDS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	}, {
 		.ident		= MAC_MODEL_LCII,
@@ -365,7 +364,7 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_IICI,
 		.scsi_type	= MAC_SCSI_LC,
 		.scc_type	= MAC_SCC_II,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_PDS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	}, {
 		.ident		= MAC_MODEL_LCIII,
@@ -374,7 +373,7 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_IICI,
 		.scsi_type	= MAC_SCSI_LC,
 		.scc_type	= MAC_SCC_II,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_PDS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	},
 
@@ -395,7 +394,7 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_QUADRA,
 		.scsi_type	= MAC_SCSI_QUADRA,
 		.scc_type	= MAC_SCC_QUADRA,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_PDS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR1,
 	}, {
 		.ident		= MAC_MODEL_Q605_ACC,
@@ -404,7 +403,7 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_QUADRA,
 		.scsi_type	= MAC_SCSI_QUADRA,
 		.scc_type	= MAC_SCC_QUADRA,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_PDS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR1,
 	}, {
 		.ident		= MAC_MODEL_Q610,
@@ -414,7 +413,7 @@ static struct mac_model mac_data_table[] = {
 		.scsi_type	= MAC_SCSI_QUADRA,
 		.scc_type	= MAC_SCC_QUADRA,
 		.ether_type	= MAC_ETHER_SONIC,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_PDS_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR1,
 	}, {
 		.ident		= MAC_MODEL_Q630,
@@ -424,8 +423,7 @@ static struct mac_model mac_data_table[] = {
 		.scsi_type	= MAC_SCSI_QUADRA,
 		.ide_type	= MAC_IDE_QUADRA,
 		.scc_type	= MAC_SCC_QUADRA,
-		.ether_type	= MAC_ETHER_SONIC,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_PDS_COMM,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR1,
 	}, {
 		.ident		= MAC_MODEL_Q650,
@@ -435,7 +433,7 @@ static struct mac_model mac_data_table[] = {
 		.scsi_type	= MAC_SCSI_QUADRA,
 		.scc_type	= MAC_SCC_QUADRA,
 		.ether_type	= MAC_ETHER_SONIC,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_PDS_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR1,
 	},
 	/* The Q700 does have a NS Sonic */
@@ -447,7 +445,7 @@ static struct mac_model mac_data_table[] = {
 		.scsi_type	= MAC_SCSI_QUADRA2,
 		.scc_type	= MAC_SCC_QUADRA,
 		.ether_type	= MAC_ETHER_SONIC,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_PDS_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR1,
 	}, {
 		.ident		= MAC_MODEL_Q800,
@@ -457,7 +455,7 @@ static struct mac_model mac_data_table[] = {
 		.scsi_type	= MAC_SCSI_QUADRA,
 		.scc_type	= MAC_SCC_QUADRA,
 		.ether_type	= MAC_ETHER_SONIC,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_PDS_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR1,
 	}, {
 		.ident		= MAC_MODEL_Q840,
@@ -467,7 +465,7 @@ static struct mac_model mac_data_table[] = {
 		.scsi_type	= MAC_SCSI_QUADRA3,
 		.scc_type	= MAC_SCC_PSC,
 		.ether_type	= MAC_ETHER_MACE,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_NUBUS,
 		.floppy_type	= MAC_FLOPPY_AV,
 	}, {
 		.ident		= MAC_MODEL_Q900,
@@ -477,7 +475,7 @@ static struct mac_model mac_data_table[] = {
 		.scsi_type	= MAC_SCSI_QUADRA2,
 		.scc_type	= MAC_SCC_IOP,
 		.ether_type	= MAC_ETHER_SONIC,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_PDS_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_IOP,
 	}, {
 		.ident		= MAC_MODEL_Q950,
@@ -487,7 +485,7 @@ static struct mac_model mac_data_table[] = {
 		.scsi_type	= MAC_SCSI_QUADRA2,
 		.scc_type	= MAC_SCC_IOP,
 		.ether_type	= MAC_ETHER_SONIC,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_PDS_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_IOP,
 	},
 
@@ -502,7 +500,7 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_IICI,
 		.scsi_type	= MAC_SCSI_LC,
 		.scc_type	= MAC_SCC_II,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_PDS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	}, {
 		.ident		= MAC_MODEL_P475,
@@ -511,7 +509,7 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_QUADRA,
 		.scsi_type	= MAC_SCSI_QUADRA,
 		.scc_type	= MAC_SCC_II,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_PDS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR1,
 	}, {
 		.ident		= MAC_MODEL_P475F,
@@ -520,7 +518,7 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_QUADRA,
 		.scsi_type	= MAC_SCSI_QUADRA,
 		.scc_type	= MAC_SCC_II,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_PDS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR1,
 	}, {
 		.ident		= MAC_MODEL_P520,
@@ -529,7 +527,7 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_IICI,
 		.scsi_type	= MAC_SCSI_LC,
 		.scc_type	= MAC_SCC_II,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_PDS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	}, {
 		.ident		= MAC_MODEL_P550,
@@ -538,7 +536,7 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_IICI,
 		.scsi_type	= MAC_SCSI_LC,
 		.scc_type	= MAC_SCC_II,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_PDS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	},
 	/* These have the comm slot, and therefore possibly SONIC ethernet */
@@ -549,8 +547,7 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_QUADRA,
 		.scsi_type	= MAC_SCSI_QUADRA,
 		.scc_type	= MAC_SCC_II,
-		.ether_type	= MAC_ETHER_SONIC,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_PDS_COMM,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR1,
 	}, {
 		.ident		= MAC_MODEL_P588,
@@ -560,8 +557,7 @@ static struct mac_model mac_data_table[] = {
 		.scsi_type	= MAC_SCSI_QUADRA,
 		.ide_type	= MAC_IDE_QUADRA,
 		.scc_type	= MAC_SCC_II,
-		.ether_type	= MAC_ETHER_SONIC,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_PDS_COMM,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR1,
 	}, {
 		.ident		= MAC_MODEL_TV,
@@ -570,7 +566,6 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_IICI,
 		.scsi_type	= MAC_SCSI_LC,
 		.scc_type	= MAC_SCC_II,
-		.nubus_type	= MAC_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	}, {
 		.ident		= MAC_MODEL_P600,
@@ -579,7 +574,7 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_IICI,
 		.scsi_type	= MAC_SCSI_LC,
 		.scc_type	= MAC_SCC_II,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	},
 
@@ -596,7 +591,7 @@ static struct mac_model mac_data_table[] = {
 		.scsi_type	= MAC_SCSI_QUADRA,
 		.scc_type	= MAC_SCC_QUADRA,
 		.ether_type	= MAC_ETHER_SONIC,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_PDS_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR1,
 	}, {
 		.ident		= MAC_MODEL_C650,
@@ -606,7 +601,7 @@ static struct mac_model mac_data_table[] = {
 		.scsi_type	= MAC_SCSI_QUADRA,
 		.scc_type	= MAC_SCC_QUADRA,
 		.ether_type	= MAC_ETHER_SONIC,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_PDS_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR1,
 	}, {
 		.ident		= MAC_MODEL_C660,
@@ -616,7 +611,7 @@ static struct mac_model mac_data_table[] = {
 		.scsi_type	= MAC_SCSI_QUADRA3,
 		.scc_type	= MAC_SCC_PSC,
 		.ether_type	= MAC_ETHER_MACE,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_PDS_NUBUS,
 		.floppy_type	= MAC_FLOPPY_AV,
 	},
 
@@ -633,7 +628,6 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_QUADRA,
 		.scsi_type	= MAC_SCSI_OLD,
 		.scc_type	= MAC_SCC_QUADRA,
-		.nubus_type	= MAC_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	}, {
 		.ident		= MAC_MODEL_PB145,
@@ -642,7 +636,6 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_QUADRA,
 		.scsi_type	= MAC_SCSI_OLD,
 		.scc_type	= MAC_SCC_QUADRA,
-		.nubus_type	= MAC_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	}, {
 		.ident		= MAC_MODEL_PB150,
@@ -652,7 +645,6 @@ static struct mac_model mac_data_table[] = {
 		.scsi_type	= MAC_SCSI_OLD,
 		.ide_type	= MAC_IDE_PB,
 		.scc_type	= MAC_SCC_QUADRA,
-		.nubus_type	= MAC_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	}, {
 		.ident		= MAC_MODEL_PB160,
@@ -661,7 +653,6 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_QUADRA,
 		.scsi_type	= MAC_SCSI_OLD,
 		.scc_type	= MAC_SCC_QUADRA,
-		.nubus_type	= MAC_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	}, {
 		.ident		= MAC_MODEL_PB165,
@@ -670,7 +661,6 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_QUADRA,
 		.scsi_type	= MAC_SCSI_OLD,
 		.scc_type	= MAC_SCC_QUADRA,
-		.nubus_type	= MAC_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	}, {
 		.ident		= MAC_MODEL_PB165C,
@@ -679,7 +669,6 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_QUADRA,
 		.scsi_type	= MAC_SCSI_OLD,
 		.scc_type	= MAC_SCC_QUADRA,
-		.nubus_type	= MAC_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	}, {
 		.ident		= MAC_MODEL_PB170,
@@ -688,7 +677,6 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_QUADRA,
 		.scsi_type	= MAC_SCSI_OLD,
 		.scc_type	= MAC_SCC_QUADRA,
-		.nubus_type	= MAC_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	}, {
 		.ident		= MAC_MODEL_PB180,
@@ -697,7 +685,6 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_QUADRA,
 		.scsi_type	= MAC_SCSI_OLD,
 		.scc_type	= MAC_SCC_QUADRA,
-		.nubus_type	= MAC_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	}, {
 		.ident		= MAC_MODEL_PB180C,
@@ -706,7 +693,6 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_QUADRA,
 		.scsi_type	= MAC_SCSI_OLD,
 		.scc_type	= MAC_SCC_QUADRA,
-		.nubus_type	= MAC_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	}, {
 		.ident		= MAC_MODEL_PB190,
@@ -716,7 +702,6 @@ static struct mac_model mac_data_table[] = {
 		.scsi_type	= MAC_SCSI_LATE,
 		.ide_type	= MAC_IDE_BABOON,
 		.scc_type	= MAC_SCC_QUADRA,
-		.nubus_type	= MAC_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	}, {
 		.ident		= MAC_MODEL_PB520,
@@ -726,7 +711,6 @@ static struct mac_model mac_data_table[] = {
 		.scsi_type	= MAC_SCSI_LATE,
 		.scc_type	= MAC_SCC_QUADRA,
 		.ether_type	= MAC_ETHER_SONIC,
-		.nubus_type	= MAC_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	},
 
@@ -743,7 +727,7 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_IICI,
 		.scsi_type	= MAC_SCSI_DUO,
 		.scc_type	= MAC_SCC_QUADRA,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	}, {
 		.ident		= MAC_MODEL_PB230,
@@ -752,7 +736,7 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_IICI,
 		.scsi_type	= MAC_SCSI_DUO,
 		.scc_type	= MAC_SCC_QUADRA,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	}, {
 		.ident		= MAC_MODEL_PB250,
@@ -761,7 +745,7 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_IICI,
 		.scsi_type	= MAC_SCSI_DUO,
 		.scc_type	= MAC_SCC_QUADRA,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	}, {
 		.ident		= MAC_MODEL_PB270C,
@@ -770,7 +754,7 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_IICI,
 		.scsi_type	= MAC_SCSI_DUO,
 		.scc_type	= MAC_SCC_QUADRA,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	}, {
 		.ident		= MAC_MODEL_PB280,
@@ -779,7 +763,7 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_IICI,
 		.scsi_type	= MAC_SCSI_DUO,
 		.scc_type	= MAC_SCC_QUADRA,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	}, {
 		.ident		= MAC_MODEL_PB280C,
@@ -788,7 +772,7 @@ static struct mac_model mac_data_table[] = {
 		.via_type	= MAC_VIA_IICI,
 		.scsi_type	= MAC_SCSI_DUO,
 		.scc_type	= MAC_SCC_QUADRA,
-		.nubus_type	= MAC_NUBUS,
+		.expansion_type	= MAC_EXP_NUBUS,
 		.floppy_type	= MAC_FLOPPY_SWIM_ADDR2,
 	},
 
@@ -1100,14 +1084,12 @@ int __init mac_platform_init(void)
 	 * Ethernet device
 	 */
 
-	switch (macintosh_config->ether_type) {
-	case MAC_ETHER_SONIC:
+	if (macintosh_config->ether_type == MAC_ETHER_SONIC ||
+	    macintosh_config->expansion_type == MAC_EXP_PDS_COMM)
 		platform_device_register_simple("macsonic", -1, NULL, 0);
-		break;
-	case MAC_ETHER_MACE:
+
+	if (macintosh_config->ether_type == MAC_ETHER_MACE)
 		platform_device_register_simple("macmace", -1, NULL, 0);
-		break;
-	}
 
 	return 0;
 }
diff --git a/drivers/net/ethernet/natsemi/macsonic.c b/drivers/net/ethernet/natsemi/macsonic.c
index 313fe5e0184b..b922ab5cedea 100644
--- a/drivers/net/ethernet/natsemi/macsonic.c
+++ b/drivers/net/ethernet/natsemi/macsonic.c
@@ -311,7 +311,7 @@ static int mac_onboard_sonic_probe(struct net_device *dev)
 {
 	struct sonic_local* lp = netdev_priv(dev);
 	int sr;
-	int commslot = 0;
+	bool commslot = macintosh_config->expansion_type == MAC_EXP_PDS_COMM;
 
 	if (!MACH_IS_MAC)
 		return -ENODEV;
@@ -322,10 +322,7 @@ static int mac_onboard_sonic_probe(struct net_device *dev)
 	   Ethernet (BTW, the Ethernet *is* always at the same
 	   address, and nothing else lives there, at least if Apple's
 	   documentation is to be believed) */
-	if (macintosh_config->ident == MAC_MODEL_Q630 ||
-	    macintosh_config->ident == MAC_MODEL_P588 ||
-	    macintosh_config->ident == MAC_MODEL_P575 ||
-	    macintosh_config->ident == MAC_MODEL_C610) {
+	if (commslot || macintosh_config->ident == MAC_MODEL_C610) {
 		int card_present;
 
 		card_present = hwreg_present((void*)ONBOARD_SONIC_REGISTERS);
@@ -333,7 +330,6 @@ static int mac_onboard_sonic_probe(struct net_device *dev)
 			printk("none.\n");
 			return -ENODEV;
 		}
-		commslot = 1;
 	}
 
 	printk("yes\n");
-- 
2.13.6

^ permalink raw reply related

* [PATCH v4 11/14] nubus: Rename struct nubus_dev
From: Finn Thain @ 2017-12-14  3:16 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: linux-m68k, linux-kernel, David S. Miller,
	Bartlomiej Zolnierkiewicz, netdev, linux-fbdev
In-Reply-To: <cover.1513220582.git.fthain@telegraphics.com.au>

It is misleading to call a functional resource a "device". In adopting
the Linux Driver Model, the struct device will be embedded in struct
nubus_board. That will compound the terminlogy problem because drivers
will bind with boards, not with functional resources. Avoid this by
renaming struct nubus_dev as struct nubus_rsrc. "Functional resource"
is the vendor's terminology so this helps avoid confusion.

Cc: "David S. Miller" <davem@davemloft.net>
Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Tested-by: Stan Johnson <userm57@yahoo.com>
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
 drivers/net/ethernet/8390/mac8390.c     |  26 ++++----
 drivers/net/ethernet/natsemi/macsonic.c |  22 +++----
 drivers/nubus/nubus.c                   | 105 ++++++++++++++++----------------
 drivers/nubus/proc.c                    |  15 ++---
 drivers/video/fbdev/macfb.c             |   2 +-
 include/linux/nubus.h                   |  30 +++++----
 6 files changed, 98 insertions(+), 102 deletions(-)

diff --git a/drivers/net/ethernet/8390/mac8390.c b/drivers/net/ethernet/8390/mac8390.c
index 9497f18eaba0..929ff6419621 100644
--- a/drivers/net/ethernet/8390/mac8390.c
+++ b/drivers/net/ethernet/8390/mac8390.c
@@ -123,7 +123,8 @@ enum mac8390_access {
 };
 
 extern int mac8390_memtest(struct net_device *dev);
-static int mac8390_initdev(struct net_device *dev, struct nubus_dev *ndev,
+static int mac8390_initdev(struct net_device *dev,
+			   struct nubus_rsrc *ndev,
 			   enum mac8390_type type);
 
 static int mac8390_open(struct net_device *dev);
@@ -169,11 +170,11 @@ static void word_memcpy_tocard(unsigned long tp, const void *fp, int count);
 static void word_memcpy_fromcard(void *tp, unsigned long fp, int count);
 static u32 mac8390_msg_enable;
 
-static enum mac8390_type __init mac8390_ident(struct nubus_dev *dev)
+static enum mac8390_type __init mac8390_ident(struct nubus_rsrc *fres)
 {
-	switch (dev->dr_sw) {
+	switch (fres->dr_sw) {
 	case NUBUS_DRSW_3COM:
-		switch (dev->dr_hw) {
+		switch (fres->dr_hw) {
 		case NUBUS_DRHW_APPLE_SONIC_NB:
 		case NUBUS_DRHW_APPLE_SONIC_LC:
 		case NUBUS_DRHW_SONNET:
@@ -184,7 +185,7 @@ static enum mac8390_type __init mac8390_ident(struct nubus_dev *dev)
 		break;
 
 	case NUBUS_DRSW_APPLE:
-		switch (dev->dr_hw) {
+		switch (fres->dr_hw) {
 		case NUBUS_DRHW_ASANTE_LC:
 			return MAC8390_NONE;
 		case NUBUS_DRHW_CABLETRON:
@@ -201,7 +202,7 @@ static enum mac8390_type __init mac8390_ident(struct nubus_dev *dev)
 	case NUBUS_DRSW_TECHWORKS:
 	case NUBUS_DRSW_DAYNA2:
 	case NUBUS_DRSW_DAYNA_LC:
-		if (dev->dr_hw == NUBUS_DRHW_CABLETRON)
+		if (fres->dr_hw == NUBUS_DRHW_CABLETRON)
 			return MAC8390_CABLETRON;
 		else
 			return MAC8390_APPLE;
@@ -212,7 +213,7 @@ static enum mac8390_type __init mac8390_ident(struct nubus_dev *dev)
 		break;
 
 	case NUBUS_DRSW_KINETICS:
-		switch (dev->dr_hw) {
+		switch (fres->dr_hw) {
 		case NUBUS_DRHW_INTERLAN:
 			return MAC8390_INTERLAN;
 		default:
@@ -225,8 +226,8 @@ static enum mac8390_type __init mac8390_ident(struct nubus_dev *dev)
 		 * These correspond to Dayna Sonic cards
 		 * which use the macsonic driver
 		 */
-		if (dev->dr_hw == NUBUS_DRHW_SMC9194 ||
-		    dev->dr_hw == NUBUS_DRHW_INTERLAN)
+		if (fres->dr_hw == NUBUS_DRHW_SMC9194 ||
+		    fres->dr_hw == NUBUS_DRHW_INTERLAN)
 			return MAC8390_NONE;
 		else
 			return MAC8390_DAYNA;
@@ -289,7 +290,8 @@ static int __init mac8390_memsize(unsigned long membase)
 	return i * 0x1000;
 }
 
-static bool __init mac8390_init(struct net_device *dev, struct nubus_dev *ndev,
+static bool __init mac8390_init(struct net_device *dev,
+				struct nubus_rsrc *ndev,
 				enum mac8390_type cardtype)
 {
 	struct nubus_dir dir;
@@ -394,7 +396,7 @@ static bool __init mac8390_init(struct net_device *dev, struct nubus_dev *ndev,
 struct net_device * __init mac8390_probe(int unit)
 {
 	struct net_device *dev;
-	struct nubus_dev *ndev = NULL;
+	struct nubus_rsrc *ndev = NULL;
 	int err = -ENODEV;
 	struct ei_device *ei_local;
 
@@ -489,7 +491,7 @@ static const struct net_device_ops mac8390_netdev_ops = {
 };
 
 static int __init mac8390_initdev(struct net_device *dev,
-				  struct nubus_dev *ndev,
+				  struct nubus_rsrc *ndev,
 				  enum mac8390_type type)
 {
 	static u32 fwrd4_offsets[16] = {
diff --git a/drivers/net/ethernet/natsemi/macsonic.c b/drivers/net/ethernet/natsemi/macsonic.c
index a42433fb6949..14f3fb50dc21 100644
--- a/drivers/net/ethernet/natsemi/macsonic.c
+++ b/drivers/net/ethernet/natsemi/macsonic.c
@@ -428,26 +428,26 @@ static int mac_nubus_sonic_ethernet_addr(struct net_device *dev,
 	return 0;
 }
 
-static int macsonic_ident(struct nubus_dev *ndev)
+static int macsonic_ident(struct nubus_rsrc *fres)
 {
-	if (ndev->dr_hw == NUBUS_DRHW_ASANTE_LC &&
-	    ndev->dr_sw == NUBUS_DRSW_SONIC_LC)
+	if (fres->dr_hw == NUBUS_DRHW_ASANTE_LC &&
+	    fres->dr_sw == NUBUS_DRSW_SONIC_LC)
 		return MACSONIC_DAYNALINK;
-	if (ndev->dr_hw == NUBUS_DRHW_SONIC &&
-	    ndev->dr_sw == NUBUS_DRSW_APPLE) {
+	if (fres->dr_hw == NUBUS_DRHW_SONIC &&
+	    fres->dr_sw == NUBUS_DRSW_APPLE) {
 		/* There has to be a better way to do this... */
-		if (strstr(ndev->board->name, "DuoDock"))
+		if (strstr(fres->board->name, "DuoDock"))
 			return MACSONIC_DUODOCK;
 		else
 			return MACSONIC_APPLE;
 	}
 
-	if (ndev->dr_hw == NUBUS_DRHW_SMC9194 &&
-	    ndev->dr_sw == NUBUS_DRSW_DAYNA)
+	if (fres->dr_hw == NUBUS_DRHW_SMC9194 &&
+	    fres->dr_sw == NUBUS_DRSW_DAYNA)
 		return MACSONIC_DAYNA;
 
-	if (ndev->dr_hw == NUBUS_DRHW_APPLE_SONIC_LC &&
-	    ndev->dr_sw == 0) { /* huh? */
+	if (fres->dr_hw == NUBUS_DRHW_APPLE_SONIC_LC &&
+	    fres->dr_sw == 0) { /* huh? */
 		return MACSONIC_APPLE16;
 	}
 	return -1;
@@ -456,7 +456,7 @@ static int macsonic_ident(struct nubus_dev *ndev)
 static int mac_nubus_sonic_probe(struct net_device *dev)
 {
 	static int slots;
-	struct nubus_dev* ndev = NULL;
+	struct nubus_rsrc *ndev = NULL;
 	struct sonic_local* lp = netdev_priv(dev);
 	unsigned long base_addr, prom_addr;
 	u16 sonic_dcr;
diff --git a/drivers/nubus/nubus.c b/drivers/nubus/nubus.c
index b0d8aa721173..324f6e4407c8 100644
--- a/drivers/nubus/nubus.c
+++ b/drivers/nubus/nubus.c
@@ -32,7 +32,7 @@
 
 /* Globals */
 
-struct nubus_dev *nubus_devices;
+struct nubus_rsrc *nubus_func_rsrcs;
 struct nubus_board *nubus_boards;
 
 /* Meaning of "bytelanes":
@@ -228,12 +228,11 @@ int nubus_get_root_dir(const struct nubus_board *board,
 EXPORT_SYMBOL(nubus_get_root_dir);
 
 /* This is a slyly renamed version of the above */
-int nubus_get_func_dir(const struct nubus_dev *dev,
-		       struct nubus_dir *dir)
+int nubus_get_func_dir(const struct nubus_rsrc *fres, struct nubus_dir *dir)
 {
-	dir->ptr = dir->base = dev->directory;
+	dir->ptr = dir->base = fres->directory;
 	dir->done = 0;
-	dir->mask = dev->board->lanes;
+	dir->mask = fres->board->lanes;
 	return 0;
 }
 EXPORT_SYMBOL(nubus_get_func_dir);
@@ -306,11 +305,10 @@ EXPORT_SYMBOL(nubus_rewinddir);
 
 /* Driver interface functions, more or less like in pci.c */
 
-struct nubus_dev*
-nubus_find_type(unsigned short category, unsigned short type,
-		const struct nubus_dev *from)
+struct nubus_rsrc *nubus_find_type(unsigned short category, unsigned short type,
+                                   const struct nubus_rsrc *from)
 {
-	struct nubus_dev *itor = from ? from->next : nubus_devices;
+	struct nubus_rsrc *itor = from ? from->next : nubus_func_rsrcs;
 
 	while (itor) {
 		if (itor->category == category && itor->type == type)
@@ -321,10 +319,10 @@ nubus_find_type(unsigned short category, unsigned short type,
 }
 EXPORT_SYMBOL(nubus_find_type);
 
-struct nubus_dev*
-nubus_find_slot(unsigned int slot, const struct nubus_dev *from)
+struct nubus_rsrc *nubus_find_slot(unsigned int slot,
+                                   const struct nubus_rsrc *from)
 {
-	struct nubus_dev *itor = from ? from->next : nubus_devices;
+	struct nubus_rsrc *itor = from ? from->next : nubus_func_rsrcs;
 
 	while (itor) {
 		if (itor->board->slot == slot)
@@ -403,19 +401,19 @@ static int __init nubus_get_display_vidmode(struct nubus_board *board,
 	return 0;
 }
 
-static int __init nubus_get_display_resource(struct nubus_dev *dev,
+static int __init nubus_get_display_resource(struct nubus_rsrc *fres,
                                              struct proc_dir_entry *procdir,
                                              const struct nubus_dirent *ent)
 {
 	switch (ent->type) {
 	case NUBUS_RESID_GAMMADIR:
 		pr_debug("    gamma directory offset: 0x%06x\n", ent->data);
-		nubus_get_block_rsrc_dir(dev->board, procdir, ent);
+		nubus_get_block_rsrc_dir(fres->board, procdir, ent);
 		break;
 	case 0x0080 ... 0x0085:
 		pr_debug("    mode 0x%02x info offset: 0x%06x\n",
 			ent->type, ent->data);
-		nubus_get_display_vidmode(dev->board, procdir, ent);
+		nubus_get_display_vidmode(fres->board, procdir, ent);
 		break;
 	default:
 		pr_debug("    unknown resource 0x%02x, data 0x%06x\n",
@@ -425,7 +423,7 @@ static int __init nubus_get_display_resource(struct nubus_dev *dev,
 	return 0;
 }
 
-static int __init nubus_get_network_resource(struct nubus_dev *dev,
+static int __init nubus_get_network_resource(struct nubus_rsrc *fres,
                                              struct proc_dir_entry *procdir,
                                              const struct nubus_dirent *ent)
 {
@@ -447,7 +445,7 @@ static int __init nubus_get_network_resource(struct nubus_dev *dev,
 	return 0;
 }
 
-static int __init nubus_get_cpu_resource(struct nubus_dev *dev,
+static int __init nubus_get_cpu_resource(struct nubus_rsrc *fres,
                                          struct proc_dir_entry *procdir,
                                          const struct nubus_dirent *ent)
 {
@@ -480,19 +478,19 @@ static int __init nubus_get_cpu_resource(struct nubus_dev *dev,
 	return 0;
 }
 
-static int __init nubus_get_private_resource(struct nubus_dev *dev,
+static int __init nubus_get_private_resource(struct nubus_rsrc *fres,
                                              struct proc_dir_entry *procdir,
                                              const struct nubus_dirent *ent)
 {
-	switch (dev->category) {
+	switch (fres->category) {
 	case NUBUS_CAT_DISPLAY:
-		nubus_get_display_resource(dev, procdir, ent);
+		nubus_get_display_resource(fres, procdir, ent);
 		break;
 	case NUBUS_CAT_NETWORK:
-		nubus_get_network_resource(dev, procdir, ent);
+		nubus_get_network_resource(fres, procdir, ent);
 		break;
 	case NUBUS_CAT_CPU:
-		nubus_get_cpu_resource(dev, procdir, ent);
+		nubus_get_cpu_resource(fres, procdir, ent);
 		break;
 	default:
 		pr_debug("    unknown resource 0x%02x, data 0x%06x\n",
@@ -502,24 +500,25 @@ static int __init nubus_get_private_resource(struct nubus_dev *dev,
 	return 0;
 }
 
-static struct nubus_dev * __init
+static struct nubus_rsrc * __init
 nubus_get_functional_resource(struct nubus_board *board, int slot,
 			      const struct nubus_dirent *parent)
 {
 	struct nubus_dir dir;
 	struct nubus_dirent ent;
-	struct nubus_dev *dev;
+	struct nubus_rsrc *fres;
 
 	pr_debug("  Functional resource 0x%02x:\n", parent->type);
 	nubus_get_subdir(parent, &dir);
 	dir.procdir = nubus_proc_add_rsrc_dir(board->procdir, parent, board);
 
 	/* Actually we should probably panic if this fails */
-	if ((dev = kzalloc(sizeof(*dev), GFP_ATOMIC)) == NULL)
+	fres = kzalloc(sizeof(*fres), GFP_ATOMIC);
+	if (!fres)
 		return NULL;
-	dev->resid = parent->type;
-	dev->directory = dir.base;
-	dev->board = board;
+	fres->resid = parent->type;
+	fres->directory = dir.base;
+	fres->board = board;
 
 	while (nubus_readdir(&dir, &ent) != -1) {
 		switch (ent.type) {
@@ -528,10 +527,10 @@ nubus_get_functional_resource(struct nubus_board *board, int slot,
 			unsigned short nbtdata[4];
 
 			nubus_get_rsrc_mem(nbtdata, &ent, 8);
-			dev->category = nbtdata[0];
-			dev->type     = nbtdata[1];
-			dev->dr_sw    = nbtdata[2];
-			dev->dr_hw    = nbtdata[3];
+			fres->category = nbtdata[0];
+			fres->type     = nbtdata[1];
+			fres->dr_sw    = nbtdata[2];
+			fres->dr_hw    = nbtdata[3];
 			pr_debug("    type: [cat 0x%x type 0x%x sw 0x%x hw 0x%x]\n",
 				nbtdata[0], nbtdata[1], nbtdata[2], nbtdata[3]);
 			nubus_proc_add_rsrc_mem(dir.procdir, &ent, 8);
@@ -589,11 +588,11 @@ nubus_get_functional_resource(struct nubus_board *board, int slot,
 		default:
 			/* Local/Private resources have their own
 			   function */
-			nubus_get_private_resource(dev, dir.procdir, &ent);
+			nubus_get_private_resource(fres, dir.procdir, &ent);
 		}
 	}
 
-	return dev;
+	return fres;
 }
 
 /* This is *really* cool. */
@@ -727,7 +726,6 @@ static int __init nubus_get_board_resource(struct nubus_board *board, int slot,
 	return 0;
 }
 
-/* Add a board (might be many devices) to the list */
 static struct nubus_board * __init nubus_add_board(int slot, int bytelanes)
 {
 	struct nubus_board *board;
@@ -799,10 +797,11 @@ static struct nubus_board * __init nubus_add_board(int slot, int bytelanes)
 	pr_debug("Slot %X resources:\n", slot);
 
 	/* Each slot should have one board resource and any number of
-	   functional resources.  So we'll fill in some fields in the
-	   struct nubus_board from the board resource, then walk down
-	   the list of functional resources, spinning out a nubus_dev
-	   for each of them. */
+	 * functional resources.  So we'll fill in some fields in the
+	 * struct nubus_board from the board resource, then walk down
+	 * the list of functional resources, spinning out a nubus_rsrc
+	 * for each of them.
+	 */
 	if (nubus_readdir(&dir, &ent) == -1) {
 		/* We can't have this! */
 		pr_err("Slot %X: Board resource not found!\n", slot);
@@ -817,32 +816,32 @@ static struct nubus_board * __init nubus_add_board(int slot, int bytelanes)
 	nubus_get_board_resource(board, slot, &ent);
 
 	while (nubus_readdir(&dir, &ent) != -1) {
-		struct nubus_dev *dev;
-		struct nubus_dev **devp;
+		struct nubus_rsrc *fres;
+		struct nubus_rsrc **fresp;
 
-		dev = nubus_get_functional_resource(board, slot, &ent);
-		if (dev == NULL)
+		fres = nubus_get_functional_resource(board, slot, &ent);
+		if (fres == NULL)
 			continue;
 
 		/* Resources should appear in ascending ID order. This sanity
 		 * check prevents duplicate resource IDs.
 		 */
-		if (dev->resid <= prev_resid) {
-			kfree(dev);
+		if (fres->resid <= prev_resid) {
+			kfree(fres);
 			continue;
 		}
-		prev_resid = dev->resid;
+		prev_resid = fres->resid;
 
 		/* We zeroed this out above */
-		if (board->first_dev == NULL)
-			board->first_dev = dev;
+		if (board->first_func_rsrc == NULL)
+			board->first_func_rsrc = fres;
 
-		/* Put it on the global NuBus device chain. Keep entries in order. */
-		for (devp = &nubus_devices; *devp != NULL;
-		     devp = &((*devp)->next))
+		/* Put it on the func. resource list. Keep entries in order. */
+		for (fresp = &nubus_func_rsrcs; *fresp != NULL;
+		     fresp = &((*fresp)->next))
 			/* spin */;
-		*devp = dev;
-		dev->next = NULL;
+		*fresp = fres;
+		fres->next = NULL;
 	}
 
 	/* Put it on the global NuBus board chain. Keep entries in order. */
diff --git a/drivers/nubus/proc.c b/drivers/nubus/proc.c
index ad8b0648a6c0..9ffaafdb3d2c 100644
--- a/drivers/nubus/proc.c
+++ b/drivers/nubus/proc.c
@@ -36,17 +36,14 @@
 static int
 nubus_devices_proc_show(struct seq_file *m, void *v)
 {
-	struct nubus_dev *dev = nubus_devices;
+	struct nubus_rsrc *fres = nubus_func_rsrcs;
 
-	while (dev) {
+	while (fres) {
 		seq_printf(m, "%x\t%04x %04x %04x %04x",
-			      dev->board->slot,
-			      dev->category,
-			      dev->type,
-			      dev->dr_sw,
-			      dev->dr_hw);
-		seq_printf(m, "\t%08lx\n", dev->board->slot_addr);
-		dev = dev->next;
+		           fres->board->slot, fres->category, fres->type,
+		           fres->dr_sw, fres->dr_hw);
+		seq_printf(m, "\t%08lx\n", fres->board->slot_addr);
+		fres = fres->next;
 	}
 	return 0;
 }
diff --git a/drivers/video/fbdev/macfb.c b/drivers/video/fbdev/macfb.c
index cda7587cbc86..e86a2796e3d9 100644
--- a/drivers/video/fbdev/macfb.c
+++ b/drivers/video/fbdev/macfb.c
@@ -556,7 +556,7 @@ static void __init iounmap_macfb(void)
 static int __init macfb_init(void)
 {
 	int video_cmap_len, video_is_nubus = 0;
-	struct nubus_dev* ndev = NULL;
+	struct nubus_rsrc *ndev = NULL;
 	char *option = NULL;
 	int err;
 
diff --git a/include/linux/nubus.h b/include/linux/nubus.h
index 0e40c9d6b977..8a40505a52ef 100644
--- a/include/linux/nubus.h
+++ b/include/linux/nubus.h
@@ -33,7 +33,7 @@ struct nubus_dirent {
 
 struct nubus_board {
 	struct nubus_board *next;
-	struct nubus_dev *first_dev;
+	struct nubus_rsrc *first_func_rsrc;
 
 	/* Only 9-E actually exist, though 0-8 are also theoretically
 	   possible, and 0 is a special case which represents the
@@ -62,11 +62,11 @@ struct nubus_board {
 	struct proc_dir_entry *procdir;
 };
 
-struct nubus_dev {
-	/* Next link in device list */
-	struct nubus_dev *next;
+struct nubus_rsrc {
+	/* Next link in list */
+	struct nubus_rsrc *next;
 
-	/* The functional resource ID of this device */
+	/* The functional resource ID */
 	unsigned char resid;
 	/* These are mostly here for convenience; we could always read
 	   them from the ROMs if we wanted to */
@@ -81,8 +81,8 @@ struct nubus_dev {
 	struct nubus_board *board;
 };
 
-/* This is all NuBus devices (used to find devices later on) */
-extern struct nubus_dev *nubus_devices;
+/* This is all NuBus functional resources (used to find devices later on) */
+extern struct nubus_rsrc *nubus_func_rsrcs;
 /* This is all NuBus cards */
 extern struct nubus_board *nubus_boards;
 
@@ -115,13 +115,12 @@ static inline void nubus_proc_add_rsrc(struct proc_dir_entry *procdir,
                                        const struct nubus_dirent *ent) {}
 #endif
 
-/* If we need more precision we can add some more of these */
-struct nubus_dev *nubus_find_type(unsigned short category,
-				  unsigned short type,
-				  const struct nubus_dev *from);
-/* Might have more than one device in a slot, you know... */
-struct nubus_dev *nubus_find_slot(unsigned int slot,
-				  const struct nubus_dev *from);
+struct nubus_rsrc *nubus_find_type(unsigned short category,
+                                   unsigned short type,
+                                   const struct nubus_rsrc *from);
+
+struct nubus_rsrc *nubus_find_slot(unsigned int slot,
+                                   const struct nubus_rsrc *from);
 
 /* These are somewhat more NuBus-specific.  They all return 0 for
    success and -1 for failure, as you'd expect. */
@@ -134,8 +133,7 @@ int nubus_get_root_dir(const struct nubus_board *board,
 int nubus_get_board_dir(const struct nubus_board* board,
 			struct nubus_dir* dir);
 /* The functional directory */
-int nubus_get_func_dir(const struct nubus_dev *dev,
-		       struct nubus_dir *dir);
+int nubus_get_func_dir(const struct nubus_rsrc *fres, struct nubus_dir *dir);
 
 /* These work on any directory gotten via the above */
 int nubus_readdir(struct nubus_dir *dir,
-- 
2.13.6

^ permalink raw reply related

* [PATCH v4 12/14] nubus: Adopt standard linked list implementation
From: Finn Thain @ 2017-12-14  3:16 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: linux-m68k, linux-kernel, David S. Miller,
	Bartlomiej Zolnierkiewicz, netdev, linux-fbdev
In-Reply-To: <cover.1513220582.git.fthain@telegraphics.com.au>

This increases code re-use and improves readability.

Cc: "David S. Miller" <davem@davemloft.net>
Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Tested-by: Stan Johnson <userm57@yahoo.com>
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
 drivers/net/ethernet/8390/mac8390.c     |  7 +++--
 drivers/net/ethernet/cirrus/mac89x0.c   |  6 +++--
 drivers/net/ethernet/natsemi/macsonic.c |  8 +++---
 drivers/nubus/nubus.c                   | 45 ++++++++-------------------------
 drivers/nubus/proc.c                    | 11 +++-----
 drivers/video/fbdev/macfb.c             |  8 +++---
 include/linux/nubus.h                   | 15 +++++------
 7 files changed, 40 insertions(+), 60 deletions(-)

diff --git a/drivers/net/ethernet/8390/mac8390.c b/drivers/net/ethernet/8390/mac8390.c
index 929ff6419621..2f91ce8dc614 100644
--- a/drivers/net/ethernet/8390/mac8390.c
+++ b/drivers/net/ethernet/8390/mac8390.c
@@ -416,8 +416,11 @@ struct net_device * __init mac8390_probe(int unit)
 	if (unit >= 0)
 		sprintf(dev->name, "eth%d", unit);
 
-	while ((ndev = nubus_find_type(NUBUS_CAT_NETWORK, NUBUS_TYPE_ETHERNET,
-				       ndev))) {
+	for_each_func_rsrc(ndev) {
+		if (ndev->category != NUBUS_CAT_NETWORK ||
+		    ndev->type != NUBUS_TYPE_ETHERNET)
+			continue;
+
 		/* Have we seen it already? */
 		if (slots & (1 << ndev->board->slot))
 			continue;
diff --git a/drivers/net/ethernet/cirrus/mac89x0.c b/drivers/net/ethernet/cirrus/mac89x0.c
index f910f0f386d6..977d4c2c759d 100644
--- a/drivers/net/ethernet/cirrus/mac89x0.c
+++ b/drivers/net/ethernet/cirrus/mac89x0.c
@@ -187,6 +187,7 @@ struct net_device * __init mac89x0_probe(int unit)
 	unsigned long ioaddr;
 	unsigned short sig;
 	int err = -ENODEV;
+	struct nubus_rsrc *fres;
 
 	if (!MACH_IS_MAC)
 		return ERR_PTR(-ENODEV);
@@ -207,8 +208,9 @@ struct net_device * __init mac89x0_probe(int unit)
 	/* We might have to parameterize this later */
 	slot = 0xE;
 	/* Get out now if there's a real NuBus card in slot E */
-	if (nubus_find_slot(slot, NULL) != NULL)
-		goto out;
+	for_each_func_rsrc(fres)
+		if (fres->board->slot == slot)
+			goto out;
 
 	/* The pseudo-ISA bits always live at offset 0x300 (gee,
            wonder why...) */
diff --git a/drivers/net/ethernet/natsemi/macsonic.c b/drivers/net/ethernet/natsemi/macsonic.c
index 14f3fb50dc21..313fe5e0184b 100644
--- a/drivers/net/ethernet/natsemi/macsonic.c
+++ b/drivers/net/ethernet/natsemi/macsonic.c
@@ -464,9 +464,11 @@ static int mac_nubus_sonic_probe(struct net_device *dev)
 	int reg_offset, dma_bitmode;
 
 	/* Find the first SONIC that hasn't been initialized already */
-	while ((ndev = nubus_find_type(NUBUS_CAT_NETWORK,
-				       NUBUS_TYPE_ETHERNET, ndev)) != NULL)
-	{
+	for_each_func_rsrc(ndev) {
+		if (ndev->category != NUBUS_CAT_NETWORK ||
+		    ndev->type != NUBUS_TYPE_ETHERNET)
+			continue;
+
 		/* Have we seen it already? */
 		if (slots & (1<<ndev->board->slot))
 			continue;
diff --git a/drivers/nubus/nubus.c b/drivers/nubus/nubus.c
index 324f6e4407c8..380f320c050f 100644
--- a/drivers/nubus/nubus.c
+++ b/drivers/nubus/nubus.c
@@ -32,7 +32,7 @@
 
 /* Globals */
 
-struct nubus_rsrc *nubus_func_rsrcs;
+LIST_HEAD(nubus_func_rsrcs);
 struct nubus_board *nubus_boards;
 
 /* Meaning of "bytelanes":
@@ -305,33 +305,20 @@ EXPORT_SYMBOL(nubus_rewinddir);
 
 /* Driver interface functions, more or less like in pci.c */
 
-struct nubus_rsrc *nubus_find_type(unsigned short category, unsigned short type,
-                                   const struct nubus_rsrc *from)
+struct nubus_rsrc *nubus_first_rsrc_or_null(void)
 {
-	struct nubus_rsrc *itor = from ? from->next : nubus_func_rsrcs;
-
-	while (itor) {
-		if (itor->category == category && itor->type == type)
-			return itor;
-		itor = itor->next;
-	}
-	return NULL;
+	return list_first_entry_or_null(&nubus_func_rsrcs, struct nubus_rsrc,
+	                                list);
 }
-EXPORT_SYMBOL(nubus_find_type);
+EXPORT_SYMBOL(nubus_first_rsrc_or_null);
 
-struct nubus_rsrc *nubus_find_slot(unsigned int slot,
-                                   const struct nubus_rsrc *from)
+struct nubus_rsrc *nubus_next_rsrc_or_null(struct nubus_rsrc *from)
 {
-	struct nubus_rsrc *itor = from ? from->next : nubus_func_rsrcs;
-
-	while (itor) {
-		if (itor->board->slot == slot)
-			return itor;
-		itor = itor->next;
-	}
-	return NULL;
+	if (list_is_last(&from->list, &nubus_func_rsrcs))
+		return NULL;
+	return list_next_entry(from, list);
 }
-EXPORT_SYMBOL(nubus_find_slot);
+EXPORT_SYMBOL(nubus_next_rsrc_or_null);
 
 int
 nubus_find_rsrc(struct nubus_dir *dir, unsigned char rsrc_type,
@@ -817,7 +804,6 @@ static struct nubus_board * __init nubus_add_board(int slot, int bytelanes)
 
 	while (nubus_readdir(&dir, &ent) != -1) {
 		struct nubus_rsrc *fres;
-		struct nubus_rsrc **fresp;
 
 		fres = nubus_get_functional_resource(board, slot, &ent);
 		if (fres == NULL)
@@ -832,16 +818,7 @@ static struct nubus_board * __init nubus_add_board(int slot, int bytelanes)
 		}
 		prev_resid = fres->resid;
 
-		/* We zeroed this out above */
-		if (board->first_func_rsrc == NULL)
-			board->first_func_rsrc = fres;
-
-		/* Put it on the func. resource list. Keep entries in order. */
-		for (fresp = &nubus_func_rsrcs; *fresp != NULL;
-		     fresp = &((*fresp)->next))
-			/* spin */;
-		*fresp = fres;
-		fres->next = NULL;
+		list_add_tail(&fres->list, &nubus_func_rsrcs);
 	}
 
 	/* Put it on the global NuBus board chain. Keep entries in order. */
diff --git a/drivers/nubus/proc.c b/drivers/nubus/proc.c
index 9ffaafdb3d2c..ccaaec4ed291 100644
--- a/drivers/nubus/proc.c
+++ b/drivers/nubus/proc.c
@@ -36,15 +36,12 @@
 static int
 nubus_devices_proc_show(struct seq_file *m, void *v)
 {
-	struct nubus_rsrc *fres = nubus_func_rsrcs;
+	struct nubus_rsrc *fres;
 
-	while (fres) {
-		seq_printf(m, "%x\t%04x %04x %04x %04x",
+	for_each_func_rsrc(fres)
+		seq_printf(m, "%x\t%04x %04x %04x %04x\t%08lx\n",
 		           fres->board->slot, fres->category, fres->type,
-		           fres->dr_sw, fres->dr_hw);
-		seq_printf(m, "\t%08lx\n", fres->board->slot_addr);
-		fres = fres->next;
-	}
+		           fres->dr_sw, fres->dr_hw, fres->board->slot_addr);
 	return 0;
 }
 
diff --git a/drivers/video/fbdev/macfb.c b/drivers/video/fbdev/macfb.c
index e86a2796e3d9..e707e617bf1c 100644
--- a/drivers/video/fbdev/macfb.c
+++ b/drivers/video/fbdev/macfb.c
@@ -670,15 +670,17 @@ static int __init macfb_init(void)
 	 * code is really broken :-)
 	 */
 
-	while ((ndev = nubus_find_type(NUBUS_CAT_DISPLAY,
-				       NUBUS_TYPE_VIDEO, ndev)))
-	{
+	for_each_func_rsrc(ndev) {
 		unsigned long base = ndev->board->slot_addr;
 
 		if (mac_bi_data.videoaddr < base ||
 		    mac_bi_data.videoaddr - base > 0xFFFFFF)
 			continue;
 
+		if (ndev->category != NUBUS_CAT_DISPLAY ||
+		    ndev->type != NUBUS_TYPE_VIDEO)
+			continue;
+
 		video_is_nubus = 1;
 		slot_addr = (unsigned char *)base;
 
diff --git a/include/linux/nubus.h b/include/linux/nubus.h
index 8a40505a52ef..aee7c50e9a2e 100644
--- a/include/linux/nubus.h
+++ b/include/linux/nubus.h
@@ -33,7 +33,6 @@ struct nubus_dirent {
 
 struct nubus_board {
 	struct nubus_board *next;
-	struct nubus_rsrc *first_func_rsrc;
 
 	/* Only 9-E actually exist, though 0-8 are also theoretically
 	   possible, and 0 is a special case which represents the
@@ -63,8 +62,7 @@ struct nubus_board {
 };
 
 struct nubus_rsrc {
-	/* Next link in list */
-	struct nubus_rsrc *next;
+	struct list_head list;
 
 	/* The functional resource ID */
 	unsigned char resid;
@@ -82,7 +80,7 @@ struct nubus_rsrc {
 };
 
 /* This is all NuBus functional resources (used to find devices later on) */
-extern struct nubus_rsrc *nubus_func_rsrcs;
+extern struct list_head nubus_func_rsrcs;
 /* This is all NuBus cards */
 extern struct nubus_board *nubus_boards;
 
@@ -115,12 +113,11 @@ static inline void nubus_proc_add_rsrc(struct proc_dir_entry *procdir,
                                        const struct nubus_dirent *ent) {}
 #endif
 
-struct nubus_rsrc *nubus_find_type(unsigned short category,
-                                   unsigned short type,
-                                   const struct nubus_rsrc *from);
+struct nubus_rsrc *nubus_first_rsrc_or_null(void);
+struct nubus_rsrc *nubus_next_rsrc_or_null(struct nubus_rsrc *from);
 
-struct nubus_rsrc *nubus_find_slot(unsigned int slot,
-                                   const struct nubus_rsrc *from);
+#define for_each_func_rsrc(f) \
+	for (f = nubus_first_rsrc_or_null(); f; f = nubus_next_rsrc_or_null(f))
 
 /* These are somewhat more NuBus-specific.  They all return 0 for
    success and -1 for failure, as you'd expect. */
-- 
2.13.6

^ permalink raw reply related

* Re: [PATCH 1/2] hp100: Fix a possible sleep-in-atomic bug in hp100_login_to_vg_hub
From: Jia-Ju Bai @ 2017-12-14  3:13 UTC (permalink / raw)
  To: David Miller; +Cc: perex, floeff, acme, netdev, linux-kernel
In-Reply-To: <20171213.162032.129766742729458957.davem@davemloft.net>

Thanks for reply :)
I think I should use "udelay(100000/HZ)" instead, do you think it is right?


Thanks,
Jia-Ju Bai


On 2017/12/14 5:20, David Miller wrote:
> I want you to review all of your patches and resend them after you
> have checked them carefully.
>
> The first patch I even looked at, this one, is buggy.
>
> You changed a schedule_timeout_interruptible(1) into a udelay(10)
>
> That's not right.
>
> schedule_timeout_interruptible() takes a "jiffies" argument, which
> is a completely different unit than udelay() takes.  You would have
> to scale the argument to udelay() in some way using HZ.
>
> Furthermore, the udelay argument you would come up with would
> be way too long to be appropirate in this atomic context.
>
> That's why the code tries to use a sleeping timeout, a long wait is
> necessary here.

^ permalink raw reply

* Re: [BUG] skge: a possible sleep-in-atomic bug in skge_remove
From: Jia-Ju Bai @ 2017-12-14  3:06 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: David Miller, mlindner, shemminger, shemminger, netdev,
	linux-kernel
In-Reply-To: <20171213085006.7093c27e@xeon-e3>



On 2017/12/14 0:50, Stephen Hemminger wrote:
> On Wed, 13 Dec 2017 15:42:56 +0800
> Jia-Ju Bai <baijiaju1990@gmail.com> wrote:
>
>> On 2017/12/13 13:18, Stephen Hemminger wrote:
>>> On Tue, 12 Dec 2017 20:57:01 -0500 (EST)
>>> David Miller <davem@davemloft.net> wrote:
>>>   
>>>> From: Stephen Hemminger <stephen@networkplumber.org>
>>>> Date: Tue, 12 Dec 2017 10:22:40 -0800
>>>>   
>>>>> On Tue, 12 Dec 2017 08:34:45 -0500 (EST)
>>>>> David Miller <davem@davemloft.net> wrote:
>>>>>       
>>>>>> From: Jia-Ju Bai <baijiaju1990@gmail.com>
>>>>>> Date: Tue, 12 Dec 2017 16:38:12 +0800
>>>>>>       
>>>>>>> According to drivers/net/ethernet/marvell/skge.c, the driver may sleep
>>>>>>> under a spinlock.
>>>>>>> The function call path is:
>>>>>>> skge_remove (acquire the spinlock)
>>>>>>>     free_irq --> may sleep
>>>>>>>
>>>>>>> I do not find a good way to fix it, so I only report.
>>>>>>> This possible bug is found by my static analysis tool (DSAC) and
>>>>>>> checked by my code review.
>>>>>> This was added by:
>>>>>>
>>>>>> commit a9e9fd7182332d0cf5f3e601df3e71dd431b70d7
>>>>>> Author: Stephen Hemminger <shemminger@vyatta.com>
>>>>>> Date:   Tue Sep 27 13:41:37 2011 -0400
>>>>>>
>>>>>>       skge: handle irq better on single port card
>>>>>>
>>>>>> I think the free_irq() can be moved below the unlock.
>>>>>>
>>>>>> Stephen, please take a look.
>>>>> The IRQ was being free twice.
>>>>> How did you see it, I really doubt any multi-port SKGE cards
>>>>> still exist.
>>>> He sees it by reading the code, please take a look at this
>>>> and move the free_irq() out of the spin locked section since
>>>> it can sleep.
>>> Thanks, I was hoping for some automated static analysis tool.
>> This bug was found by an automated static analysis tool named DSAC,
>> which is written by myself.
>> Then I manually checked driver source code, and finally sent the bug report.
> Thanks.
> Would it be possible to put tool in tools directory and then have
> it automated by kbuild robot?
Hi,

Thanks for your appreciation of my tool :)

I think it is possible. But before releasing, I need to improve it to 
solve some problems:
1. It produces some false positives and negatives. Thus I need 
developer's response and confirmation of my bug reports, to help me to 
improve my tool's accuracy.
2. It is based on Clang-3.2 (a LLVM-based compiler, not GCC), and some 
driver code cannot compile normally. Maybe I should re-implement my tool 
based on a recent Clang version.
3. Some software is needed when running my tool, thus as MySQL (maybe I 
directly can write to files, instead of database in the future) and 
Clang (Clang is necessary).

I think it may be difficult to directly put my tool in "tools" 
directory. But with some configuration, it is possible to run it 
automatically in a testing environment.
I also plan to open this tool in the future, after finishing the 
improvements :)


Thanks,
Jia-Ju Bai

^ permalink raw reply

* [PATCH v2] ARM64: dts: meson-axg: add ethernet mac controller
From: Yixun Lan @ 2017-12-14  3:02 UTC (permalink / raw)
  To: devicetree, Kevin Hilman
  Cc: Neil Armstrong, Jerome Brunet, Giuseppe Cavallaro,
	Alexandre Torgue, Carlo Caione, Yixun Lan, linux-amlogic,
	linux-arm-kernel, linux-kernel, netdev

Add DT info for the stmmac ethernet MAC which found in
the Amlogic's Meson-AXG SoC, also describe the ethernet
pinctrl & clock information here.

This is tested in the S400 dev board which use a RTL8211F PHY,
and the pins connect to the 'eth_rgmii_y_pins' group.

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Yixun Lan <yixun.lan@amlogic.com>

---
Changes in v2 since [1]:
 - rebase to kevin's v4.16/dt64 branch
 - add Neil's Reviewed-by
 - move clock info to board.dts instead of in soc.dtsi
 - drop "meson-axg-dwmac" compatible string, since we didn't use this
   we could re-add it later when we really need.
 - note: to make ethernet work properly,it depend on clock & pinctrl[2],
   to compile the DTS, the patch [3] is required.
   the code part will be taken via clock & pinctrl subsystem tree.

[1]
http://lists.infradead.org/pipermail/linux-amlogic/2017-November/005301.html

[2]
http://lists.infradead.org/pipermail/linux-amlogic/2017-December/005735.html
http://lists.infradead.org/pipermail/linux-amlogic/2017-December/005694.html

[3]
http://lists.infradead.org/pipermail/linux-amlogic/2017-December/005738.html
---
 arch/arm64/boot/dts/amlogic/meson-axg-s400.dts | 11 ++++++
 arch/arm64/boot/dts/amlogic/meson-axg.dtsi     | 50 ++++++++++++++++++++++++++
 2 files changed, 61 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/meson-axg-s400.dts b/arch/arm64/boot/dts/amlogic/meson-axg-s400.dts
index 70eca1f8736a..138de3bc7cc8 100644
--- a/arch/arm64/boot/dts/amlogic/meson-axg-s400.dts
+++ b/arch/arm64/boot/dts/amlogic/meson-axg-s400.dts
@@ -20,3 +20,14 @@
 &uart_AO {
 	status = "okay";
 };
+
+&ethmac {
+	status = "okay";
+	clocks = <&clkc CLKID_ETH>,
+		 <&clkc CLKID_FCLK_DIV2>,
+		 <&clkc CLKID_MPLL2>;
+	clock-names = "stmmaceth", "clkin0", "clkin1";
+	phy-mode = "rgmii";
+	pinctrl-0 = <&eth_rgmii_y_pins>;
+	pinctrl-names = "default";
+};
diff --git a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
index d356ce74ad89..106234fda765 100644
--- a/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
+++ b/arch/arm64/boot/dts/amlogic/meson-axg.dtsi
@@ -7,6 +7,7 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/clock/axg-clkc.h>
 
 / {
 	compatible = "amlogic,meson-axg";
@@ -148,6 +149,15 @@
 			#address-cells = <0>;
 		};
 
+		ethmac: ethernet@ff3f0000 {
+			compatible = "amlogic,meson-gxbb-dwmac", "snps,dwmac";
+			reg = <0x0 0xff3f0000 0x0 0x10000
+				0x0 0xff634540 0x0 0x8>;
+			interrupts = <GIC_SPI 8 IRQ_TYPE_EDGE_RISING>;
+			interrupt-names = "macirq";
+			status = "disabled";
+		};
+
 		hiubus: bus@ff63c000 {
 			compatible = "simple-bus";
 			reg = <0x0 0xff63c000 0x0 0x1c00>;
@@ -194,6 +204,46 @@
 					#gpio-cells = <2>;
 					gpio-ranges = <&pinctrl_periphs 0 0 86>;
 				};
+
+				eth_rgmii_x_pins: eth-x-rgmii {
+					mux {
+						groups = "eth_mdio_x",
+						       "eth_mdc_x",
+						       "eth_rgmii_rx_clk_x",
+						       "eth_rx_dv_x",
+						       "eth_rxd0_x",
+						       "eth_rxd1_x",
+						       "eth_rxd2_rgmii",
+						       "eth_rxd3_rgmii",
+						       "eth_rgmii_tx_clk",
+						       "eth_txen_x",
+						       "eth_txd0_x",
+						       "eth_txd1_x",
+						       "eth_txd2_rgmii",
+						       "eth_txd3_rgmii";
+						function = "eth";
+					};
+				};
+
+				eth_rgmii_y_pins: eth-y-rgmii {
+					mux {
+						groups = "eth_mdio_y",
+						       "eth_mdc_y",
+						       "eth_rgmii_rx_clk_y",
+						       "eth_rx_dv_y",
+						       "eth_rxd0_y",
+						       "eth_rxd1_y",
+						       "eth_rxd2_rgmii",
+						       "eth_rxd3_rgmii",
+						       "eth_rgmii_tx_clk",
+						       "eth_txen_y",
+						       "eth_txd0_y",
+						       "eth_txd1_y",
+						       "eth_txd2_rgmii",
+						       "eth_txd3_rgmii";
+						function = "eth";
+					};
+				};
 			};
 		};
 
-- 
2.15.1

^ permalink raw reply related

* Re: [bpf-next V1-RFC PATCH 01/14] xdp: base API for new XDP rx-queue info concept
From: David Ahern @ 2017-12-14  2:34 UTC (permalink / raw)
  To: Jesper Dangaard Brouer, Daniel Borkmann, Alexei Starovoitov
  Cc: netdev, gospo, bjorn.topel, michael.chan
In-Reply-To: <151316396600.14967.5648145904814220715.stgit@firesoul>

On 12/13/17 4:19 AM, Jesper Dangaard Brouer wrote:
> +
> +void xdp_rxq_info_unreg(struct xdp_rxq_info *xdp_rxq)
> +{
> +	xdp_rxq->reg_state = REG_STATE_UNREGISTRED;
> +}
> +EXPORT_SYMBOL_GPL(xdp_rxq_info_unreg);
> +
> +void xdp_rxq_info_init(struct xdp_rxq_info *xdp_rxq)
> +{
> +	if (xdp_rxq->reg_state == REG_STATE_REGISTRED) {
> +		WARN(1, "Missing unregister, handled but fix driver\n");
> +		xdp_rxq_info_unreg(xdp_rxq);
> +	}
> +	memset(xdp_rxq, 0, sizeof(*xdp_rxq));
> +	xdp_rxq->queue_index = U32_MAX;
> +	xdp_rxq->reg_state = REG_STATE_NEW;
> +}
> +EXPORT_SYMBOL_GPL(xdp_rxq_info_init);
> +
> +void xdp_rxq_info_reg(struct xdp_rxq_info *xdp_rxq)
> +{
> +	WARN(!xdp_rxq->dev, "Missing net_device from driver");
> +	WARN(xdp_rxq->queue_index == U32_MAX, "Miss queue_index from driver");
> +	WARN(!(xdp_rxq->reg_state == REG_STATE_NEW),"API violation, miss init");
> +	xdp_rxq->reg_state = REG_STATE_REGISTRED;
> +}
> +EXPORT_SYMBOL_GPL(xdp_rxq_info_reg);
> 

Rather than WARN()'s why not make the _reg and _init functions return an
int that indicates an error? For example you don't want to continue if
the dev is expected but missing.

^ permalink raw reply

* Re: [bpf-next V1-RFC PATCH 08/14] nfp: setup xdp_rxq_info
From: Jakub Kicinski @ 2017-12-14  2:34 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: Daniel Borkmann, Alexei Starovoitov, dsahern, oss-drivers,
	Simon Horman, netdev, gospo, bjorn.topel, michael.chan
In-Reply-To: <151316400161.14967.13516011331263133183.stgit@firesoul>

On Wed, 13 Dec 2017 12:20:01 +0100, Jesper Dangaard Brouer wrote:
> Driver hook points for xdp_rxq_info:
>  * init+reg: nfp_net_rx_ring_alloc
>  * unreg   : nfp_net_rx_ring_free
> 
> In struct nfp_net_rx_ring moved member @size into a hole on 64-bit.
> Thus, the size remaines the same after adding member @xdp_rxq.
> 
> Cc: oss-drivers@netronome.com
> Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
> Cc: Simon Horman <simon.horman@netronome.com>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>

> diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net.h b/drivers/net/ethernet/netronome/nfp/nfp_net.h
> index 3801c52098d5..0e564cfabe7e 100644
> --- a/drivers/net/ethernet/netronome/nfp/nfp_net.h
> +++ b/drivers/net/ethernet/netronome/nfp/nfp_net.h
> @@ -47,6 +47,7 @@
>  #include <linux/netdevice.h>
>  #include <linux/pci.h>
>  #include <linux/io-64-nonatomic-hi-lo.h>
> +#include <net/xdp.h>
>  
>  #include "nfp_net_ctrl.h"
>  
> @@ -350,6 +351,7 @@ struct nfp_net_rx_buf {
>   * @rxds:       Virtual address of FL/RX ring in host memory
>   * @dma:        DMA address of the FL/RX ring
>   * @size:       Size, in bytes, of the FL/RX ring (needed to free)
> + * @xdp_rxq:    RX-ring info avail for XDP
>   */
>  struct nfp_net_rx_ring {
>  	struct nfp_net_r_vector *r_vec;
> @@ -361,13 +363,14 @@ struct nfp_net_rx_ring {
>  	u32 idx;
>  
>  	int fl_qcidx;
> +	unsigned int size;
>  	u8 __iomem *qcp_fl;
>  
>  	struct nfp_net_rx_buf *rxbufs;
>  	struct nfp_net_rx_desc *rxds;
>  
>  	dma_addr_t dma;
> -	unsigned int size;
> +	struct xdp_rxq_info xdp_rxq;
>  } ____cacheline_aligned;

The @size member is not in the hole on purpose.  IIRC all the members
up to @dma are in the first cacheline.  All things which are not needed
on the fast path are after @dma.  IOW @size is not used on the fast
path and the hole is for fast path stuff :)

>  /**
> diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
> index ad3e9f6a61e5..6474aecd0451 100644
> --- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
> +++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
> @@ -2252,6 +2253,7 @@ static void nfp_net_rx_ring_free(struct nfp_net_rx_ring *rx_ring)
>  	struct nfp_net_r_vector *r_vec = rx_ring->r_vec;
>  	struct nfp_net_dp *dp = &r_vec->nfp_net->dp;
>  
> +	xdp_rxq_info_unreg(&rx_ring->xdp_rxq);
>  	kfree(rx_ring->rxbufs);
>  
>  	if (rx_ring->rxds)
> @@ -2277,6 +2279,12 @@ nfp_net_rx_ring_alloc(struct nfp_net_dp *dp, struct nfp_net_rx_ring *rx_ring)
>  {
>  	int sz;
>  
> +	/* XDP RX-queue info */
> +	xdp_rxq_info_init(&rx_ring->xdp_rxq);
> +	rx_ring->xdp_rxq.dev = dp->netdev;
> +	rx_ring->xdp_rxq.queue_index = rx_ring->idx;
> +	xdp_rxq_info_reg(&rx_ring->xdp_rxq);
> +
>  	rx_ring->cnt = dp->rxd_cnt;
>  	rx_ring->size = sizeof(*rx_ring->rxds) * rx_ring->cnt;
>  	rx_ring->rxds = dma_zalloc_coherent(dp->dev, rx_ring->size,

The nfp driver implements the prepare/commit for reallocating rings.  
I don't think it matters now, but there can be 2 sets of rings with the
same ID allocated during reconfiguration (see nfp_net_ring_reconfig()).
Maybe place the register/unregister in nfp_net_open_stack() and
nfp_net_close_stack() respectively?

Perhaps that won't be necessary, only cleaner :)  I'm not sure how is
the redirect between drivers intended to work WRT freeing rings and
unloading drivers while packets fly...

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox