RE: [PATCH v4 net-next 04/10] net/ncsi: Ethtool operation to get NCSI topology

2017-05-08 Thread David Laight
From: Gavin Shan > Sent: 08 May 2017 01:20 ... > >Why 16 bits? > >You are just making life hard for the compiler and possibly generating > >random padding. > > > > It's because there are 256 NCSI channels to maximal degree. So 16-bits > is the minial data width to hold it in signed format. Yes, I

Re: [PATCH v4 net-next 04/10] net/ncsi: Ethtool operation to get NCSI topology

2017-05-07 Thread Gavin Shan
On Thu, May 04, 2017 at 09:31:20AM +, David Laight wrote: >From: Gavin Shan >> Sent: 04 May 2017 07:16 >> On Wed, May 03, 2017 at 10:19:44PM -0700, Stephen Hemminger wrote: >> >On Wed, 3 May 2017 14:44:35 +1000 >> >Gavin Shan wrote: >... >> >> +{ >> >> + struct

RE: [PATCH v4 net-next 04/10] net/ncsi: Ethtool operation to get NCSI topology

2017-05-04 Thread David Laight
From: Gavin Shan > Sent: 04 May 2017 07:16 > On Wed, May 03, 2017 at 10:19:44PM -0700, Stephen Hemminger wrote: > >On Wed, 3 May 2017 14:44:35 +1000 > >Gavin Shan wrote: ... > >> +{ > >> + struct ethtool_ncsi_channels *enc; > >> + short nr_channels; > >Should be

Re: [PATCH v4 net-next 04/10] net/ncsi: Ethtool operation to get NCSI topology

2017-05-04 Thread Gavin Shan
On Wed, May 03, 2017 at 10:21:11PM -0700, Stephen Hemminger wrote: >On Wed, 3 May 2017 14:44:35 +1000 >Gavin Shan wrote: > >> +void ncsi_ethtool_register_dev(struct net_device *dev) >> +{ >> +struct ethtool_ops *ops; >> + >> +ops = (struct ethtool_ops

Re: [PATCH v4 net-next 04/10] net/ncsi: Ethtool operation to get NCSI topology

2017-05-04 Thread Gavin Shan
On Wed, May 03, 2017 at 10:19:44PM -0700, Stephen Hemminger wrote: >On Wed, 3 May 2017 14:44:35 +1000 >Gavin Shan wrote: > >> +static int ethtool_get_ncsi_channels(struct net_device *dev, >> + void __user *useraddr) > >Please don't use

Re: [PATCH v4 net-next 04/10] net/ncsi: Ethtool operation to get NCSI topology

2017-05-03 Thread Stephen Hemminger
On Wed, 3 May 2017 14:44:35 +1000 Gavin Shan wrote: > +void ncsi_ethtool_register_dev(struct net_device *dev) > +{ > + struct ethtool_ops *ops; > + > + ops = (struct ethtool_ops *)(dev->ethtool_ops); > + if (!ops) > + return; > + > +

Re: [PATCH v4 net-next 04/10] net/ncsi: Ethtool operation to get NCSI topology

2017-05-03 Thread Stephen Hemminger
On Wed, 3 May 2017 14:44:35 +1000 Gavin Shan wrote: > +static int ethtool_get_ncsi_channels(struct net_device *dev, > + void __user *useraddr) Please don't use an opaque type for this. See how other ethtool operations take a struct.

Re: [PATCH v4 net-next 04/10] net/ncsi: Ethtool operation to get NCSI topology

2017-05-03 Thread Gavin Shan
On Thu, May 04, 2017 at 02:49:33AM +0200, Andrew Lunn wrote: >> +void ncsi_ethtool_register_dev(struct net_device *dev) >> +{ >> +struct ethtool_ops *ops; >> + >> +ops = (struct ethtool_ops *)(dev->ethtool_ops); > >Why do you need the cast here? > >Ah, is it because net_device has: > >

Re: [PATCH v4 net-next 04/10] net/ncsi: Ethtool operation to get NCSI topology

2017-05-03 Thread Andrew Lunn
> +void ncsi_ethtool_register_dev(struct net_device *dev) > +{ > + struct ethtool_ops *ops; > + > + ops = (struct ethtool_ops *)(dev->ethtool_ops); Why do you need the cast here? Ah, is it because net_device has: const struct ethtool_ops *ethtool_ops; i.e. you are casting off