netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/12] qlcnic: patches for new adapter - Qlogic 83XX CNA
@ 2012-08-31  6:28 Sony Chacko
  2012-08-31  6:28 ` [PATCH 01/12] qlcnic: Refactoring - template based hardware interface Sony Chacko
                   ` (11 more replies)
  0 siblings, 12 replies; 19+ messages in thread
From: Sony Chacko @ 2012-08-31  6:28 UTC (permalink / raw)
  To: davem; +Cc: netdev, Dept_NX_Linux_NIC_Driver, Sony Chacko

From: Sony Chacko <sony.chacko@qlogic.com>

>From: David Miller [mailto:davem@davemloft.net]

>Firstly, I'm not going to let you create arbitrary new sysfs
>and debugfs crap for facilities that are already supported by
>the kernel via other interfaces.

>For example, providing a facility to read and write the PCI BARs
>is completely pointless.  Use the PCI config space access APIs for
>this if you want to do this from userspace.

>The patches are also much more verbose than they need to be.
>When you move an operation to the new hwops, keep the existing function
>name but make it an inline function that invokes the hwop.

>That way you won't need hundreds of lines in your patch that look like this:

>-	qlcnic_clear_lb_mode(adapter);
>+	ahw->hw_ops->clear_loopback(adapter, mode);

>Instead you'd have:

>static inline void qlcnic_clear_lb_mode(struct qlcnic_adapter *adapter,
>							u8 mode) {
>	struct qlcnic_hardware_context *ahw = &adapter->ahw;

>	ahw->hw_ops->clear_loopback(adapter, mode); }

I have made the changes as you suggested above.
Please apply the updated pacth series to net-next.

Thanks,
Sony

^ permalink raw reply	[flat|nested] 19+ messages in thread
* [PATCH net-next 0/12] qlcnic: patches for new adapter - Qlogic 83XX CNA
@ 2012-08-31 22:36 Sony Chacko
  2012-08-31 22:36 ` [PATCH 01/12] qlcnic: Refactoring - template based hardware interface Sony Chacko
  0 siblings, 1 reply; 19+ messages in thread
From: Sony Chacko @ 2012-08-31 22:36 UTC (permalink / raw)
  To: davem; +Cc: netdev, Dept_NX_Linux_NIC_Driver, Sony Chacko

From: Sony Chacko <sony.chacko@qlogic.com>

>From: David Miller [mailto:davem@davemloft.net]

>Firstly, I'm not going to let you create arbitrary new sysfs
>and debugfs crap for facilities that are already supported by
>the kernel via other interfaces.

>For example, providing a facility to read and write the PCI BARs
>is completely pointless.  Use the PCI config space access APIs for
>this if you want to do this from userspace.

>The patches are also much more verbose than they need to be.
>When you move an operation to the new hwops, keep the existing function
>name but make it an inline function that invokes the hwop.

>That way you won't need hundreds of lines in your patch that look like this:

>-	qlcnic_clear_lb_mode(adapter);
>+	ahw->hw_ops->clear_loopback(adapter, mode);

>Instead you'd have:

>static inline void qlcnic_clear_lb_mode(struct qlcnic_adapter *adapter,
>							u8 mode) {
>	struct qlcnic_hardware_context *ahw = &adapter->ahw;

>	ahw->hw_ops->clear_loopback(adapter, mode); }

>From: David Miller [mailto:davem@davemloft.net] 
>Sent: Friday, August 31, 2012 1:05 PM

> +static inline int
> +qlcnic_config_led(struct qlcnic_adapter *adapter, u32 state, u32 
> +rate) {
> +	return adapter->nic_ops->config_led(adapter, state, rate);
> +
> +}
> +

>Please get rid of those unnecessary empty lines in the function bodies.


I have made the changes as you suggested above,
please drop all the previous 12 patches.
We are not creating new sysfs nodes.

Please apply the updated patch series to net-next.

Thanks,
Sony

^ permalink raw reply	[flat|nested] 19+ messages in thread
* [PATCH 0/12] qlcnic: patches for new adapter - Qlogic 83XX CNA
@ 2012-09-06  6:21 Sony Chacko
  2012-09-06  6:21 ` [PATCH 01/12] qlcnic: Refactoring - template based hardware interface Sony Chacko
  0 siblings, 1 reply; 19+ messages in thread
From: Sony Chacko @ 2012-09-06  6:21 UTC (permalink / raw)
  To: davem; +Cc: netdev, Dept_NX_Linux_NIC_Driver, Sony Chacko

From: Sony Chacko <sony.chacko@qlogic.com>

>From: David Miller [mailto:davem@davemloft.net]

>From: David Miller [mailto:davem@davemloft.net] 
>Sent: Friday, August 31, 2012 1:05 PM

> +static inline int
> +qlcnic_config_led(struct qlcnic_adapter *adapter, u32 state, u32 
> +rate) {
> +	return adapter->nic_ops->config_led(adapter, state, rate);
> +
> +}
> +

>Please get rid of those unnecessary empty lines in the function bodies.

>From: David Miller <davem@davemloft.net>
>Date: Fri, 31 Aug 2012 20:00:35 -0400 (EDT)

>Also in patch #5:
>-	qlcnic_config_ipaddr(adapter, ifa->ifa_address, QLCNIC_IP_UP);
>+	qlcnic_config_ipaddr(adapter, ifa->ifa_address,
>+						 QLCNIC_IP_UP);

>This is not the correct way to format multi-line function calls,
>the correct way is:

>	qlcnic_config_ipaddr(adapter, ifa->ifa_address,
>			     QLCNIC_IP_UP);

I have made the changes as you suggested above.
Please apply the updated 12 patch series to net-next.

Thanks,
Sony

^ permalink raw reply	[flat|nested] 19+ messages in thread
* [PATCH 0/12] qlcnic: patches for new adapter - Qlogic 83XX CNA
@ 2012-09-07 23:01 Sony Chacko
  2012-09-07 23:01 ` [PATCH 01/12] qlcnic: Refactoring - template based hardware interface Sony Chacko
  0 siblings, 1 reply; 19+ messages in thread
From: Sony Chacko @ 2012-09-07 23:01 UTC (permalink / raw)
  To: davem; +Cc: netdev, Dept_NX_Linux_NIC_Driver, Sony Chacko

From: Sony Chacko <sony.chacko@qlogic.com>

Re: [PATCH 11/12] qlcnic: 83xx adpater ethtool
>From: Ben Hutchings [mailto:bhutchings@solarflare.com]

>	#define QLCNIC_ETHTOOL_REGS_VER	2

> I think QLCNIC_ETHTOOL_REGS_VER needs to be changed,
> as you appear to be dumping more registers from the
> existing hardware as well.

> +static int qlcnic_set_channels(struct net_device *dev

> So you removed the body of qlcnic_set_channels() in an
> earlier patch, and now you're moving it and putting the body
> back how it was...  Please clean up the patch series
> so it doesn't have noise like this in it.

Re: [PATCH 10/12] qlcnic: register dump utility
> *dev,  static int qlcnic_set_channels(struct net_device *dev,
>  		struct ethtool_channels *channel)
>  {
> +	return 0;
>  }
[...]

> No, I don't think so. 
> If you're going to remove support for this operation
> then delete the function entirely.
> And don't put it in a patch that's supposed
> to do something unrelated.

Re: [PATCH 06/12] qlcnic: 83xx data path and HW interfaces routines
> +int qlcnic_83xx_set_settings(struct qlcnic_adapter *adapter,
> +			     struct ethtool_cmd *ecmd)
> +		return -EIO;

> Should be -EINVAL.

> +qlcnic_83xx_fill_stats(struct qlcnic_adapter *adapter,

> +	default:
> +		dev_info(&adapter->pdev->dev, "Unknown get statistics mode\n");

> It seems like this default case can only be reached
> in case of a driver bug, so WARN would be more appropriate.


We have addressed all the issues raised by Ben Hutchings.
Please apply the updated 12 patch series to net-next.

Thanks,
Sony

^ permalink raw reply	[flat|nested] 19+ messages in thread
* [PATCH 0/12] qlcnic: patches for new adapter - Qlogic 83XX CNA
@ 2012-09-11  1:19 Sony Chacko
  2012-09-11  1:19 ` [PATCH 01/12] qlcnic: Refactoring - template based hardware interface Sony Chacko
  0 siblings, 1 reply; 19+ messages in thread
From: Sony Chacko @ 2012-09-11  1:19 UTC (permalink / raw)
  To: davem; +Cc: netdev, Dept_NX_Linux_NIC_Driver, Sony Chacko

From: Sony Chacko <sony.chacko@qlogic.com>

> From: Ben Hutchings [mailto:bhutchings@solarflare.com]
> Re: [PATCH 06/12] qlcnic: 83xx data path and HW interface routines

> -static int load_fw_file;
> +int load_fw_file;
> module_param(load_fw_file, int, 0444); MODULE_PARM_DESC(load_fw_file, 
> "Load firmware from (0=flash, 1=file");

> All the variables being declared extern need to be renamed to have a 'qlcnic_' prefix.
> (The parameters shouldn't be renamed, though - use module_param_named().)

> +int qlcnic_83xx_set_pauseparam(struct qlcnic_adapter *adapter,
> +			       struct ethtool_pauseparam *pause) {
> +	int status = 0;
> +	u32 config = adapter->ahw->port_config;
> +
> +	if ((pause->rx_pause) || (pause->tx_pause) || (pause->autoneg))
> +		adapter->ahw->port_config |= QLC_83XX_CFG_PAUSE_STD;
> +	else if (!(pause->rx_pause && pause->tx_pause && pause->autoneg))

> Since you appear to support only all-enabled or all-disabled,
> this second condition should be:
> !pause->rx_pause && !pause->tx_pause && !pause->autoneg

We have updated the patches.
Please apply the updated 12 patch series to net-next.

Thanks,
Sony

^ permalink raw reply	[flat|nested] 19+ messages in thread
end of thread, other threads:[~2012-09-11  6:18 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-31  6:28 [PATCH net-next 0/12] qlcnic: patches for new adapter - Qlogic 83XX CNA Sony Chacko
2012-08-31  6:28 ` [PATCH 01/12] qlcnic: Refactoring - template based hardware interface Sony Chacko
2012-08-31 20:05   ` David Miller
2012-08-31  6:28 ` [PATCH 02/12] qlcnic: Refactoring - Tx/Rx code path separation Sony Chacko
2012-08-31  6:28 ` [PATCH 03/12] qlcnic: Refactoring - Moving hw specific data to hardware context Sony Chacko
2012-08-31  6:28 ` [PATCH 04/12] qlcnic: remove 82xx specific register dump utility Sony Chacko
2012-08-31  6:28 ` [PATCH 05/12] qlcnic: change driver firmware interface mechanism Sony Chacko
2012-08-31  6:28 ` [PATCH 06/12] qlcnic: 83xx data path and HW interfaces routines Sony Chacko
2012-08-31  6:28 ` [PATCH 07/12] qlcnic: 83xx adpater flash interface routines Sony Chacko
2012-08-31  6:28 ` [PATCH 08/12] qlcnic: 83xx CNA inter driver communication mechanism Sony Chacko
2012-08-31  6:28 ` [PATCH 09/12] qlcnic: enable 83xx virtual NIC mode Sony Chacko
2012-08-31  6:28 ` [PATCH 10/12] qlcnic: register dump utility Sony Chacko
2012-08-31  6:29 ` [PATCH 11/12] qlcnic: 83xx adpater ethtool Sony Chacko
2012-08-31  6:29 ` [PATCH 12/12] qlcnic: update driver version - 5.1.30 Sony Chacko
  -- strict thread matches above, loose matches on Subject: below --
2012-08-31 22:36 [PATCH net-next 0/12] qlcnic: patches for new adapter - Qlogic 83XX CNA Sony Chacko
2012-08-31 22:36 ` [PATCH 01/12] qlcnic: Refactoring - template based hardware interface Sony Chacko
2012-09-06  6:21 [PATCH 0/12] qlcnic: patches for new adapter - Qlogic 83XX CNA Sony Chacko
2012-09-06  6:21 ` [PATCH 01/12] qlcnic: Refactoring - template based hardware interface Sony Chacko
2012-09-07 23:01 [PATCH 0/12] qlcnic: patches for new adapter - Qlogic 83XX CNA Sony Chacko
2012-09-07 23:01 ` [PATCH 01/12] qlcnic: Refactoring - template based hardware interface Sony Chacko
2012-09-11  1:19 [PATCH 0/12] qlcnic: patches for new adapter - Qlogic 83XX CNA Sony Chacko
2012-09-11  1:19 ` [PATCH 01/12] qlcnic: Refactoring - template based hardware interface Sony Chacko
2012-09-11  6:01   ` Francois Romieu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).