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
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
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
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
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
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;
> +
> +
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.
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:
>
>
> +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