Re: [PATCH 2/3] staging: octeon: avoid needless device allocation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



>  	num_interfaces = cvmx_helper_get_number_of_interfaces();
>  	for (interface = 0; interface < num_interfaces; interface++) {
> +		int num_ports, port_index;
> +		const struct net_device_ops *ops;
> +		const char *name;
> +		phy_interface_t phy_mode = PHY_INTERFACE_MODE_NA;
>  		cvmx_helper_interface_mode_t imode =
> -		    cvmx_helper_interface_get_mode(interface);
> -		int num_ports = cvmx_helper_ports_on_interface(interface);
> -		int port_index;
> +			cvmx_helper_interface_get_mode(interface);
> +
> +		switch (imode) {
> +		case CVMX_HELPER_INTERFACE_MODE_NPI:
> +			ops = &cvm_oct_npi_netdev_ops;
> +			name = "npi%d";

In general, the kernel does not give the interface names other than
ethX. userspace can rename them, e.g. systemd with its persistent
names. So as part of getting this driver out of staging, i would throw
this naming code away.

> +		num_ports = cvmx_helper_ports_on_interface(interface);
>  		for (port_index = 0,
>  		     port = cvmx_helper_get_ipd_port(interface, 0);
>  		     port < cvmx_helper_get_ipd_port(interface, num_ports);
>  		     port_index++, port++) {
>  			struct octeon_ethernet *priv;
>  			struct net_device *dev =
> -			    alloc_etherdev(sizeof(struct octeon_ethernet));
> +				alloc_etherdev(sizeof(struct octeon_ethernet));

Please try to avoid white space changed. Put such white space changes
into a patch of their own, with a commit message saying it just
contains whitespace cleanup.


>  			if (!dev) {
>  				pr_err("Failed to allocate ethernet device for port %d\n",
>  				       port);
> @@ -830,7 +875,12 @@ static int cvm_oct_probe(struct platform_device *pdev)
>  			priv->port = port;
>  			priv->queue = cvmx_pko_get_base_queue(priv->port);
>  			priv->fau = fau - cvmx_pko_get_num_queues(port) * 4;
> -			priv->phy_mode = PHY_INTERFACE_MODE_NA;
> +			priv->phy_mode = phy_mode;

You should be getting phy_mode from DT.

Ideally, you want lots of small patches which are obviously
correct. So i would try to break this up into smaller changes.

I also wounder if you are addresses issues in the correct order. This
driver is in staging for a reason. It needs a lot of work. You might
be better off first cleaning it up. And then consider moving it to
phylink.

	 Andrew



[Index of Archives]     [LKML Archive]     [Linux ARM Kernel]     [Linux ARM]     [Git]     [Yosemite News]     [Linux SCSI]     [Linux Hams]

  Powered by Linux