Re: [PATCH 6/10] VIOC: New Network Device Driver

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

 



On Sunday 08 October 2006 12:27 am, Pavel Machek wrote:
> Hi!
>
> > +	ecmd->phy_address = 0;	/* !!! Stole from e1000 */
> > +	ecmd->speed = 3;	/* !!! Stole from e1000 */
>
> Eh?
You are right. Will fix.
>
> > +static void vnic_get_regs(struct net_device *netdev,
> > +			  struct ethtool_regs *regs, void *p)
> > +{
> > +	struct vnic_device *vnicdev = netdev->priv;
> > +	struct vioc_device *viocdev = vnicdev->viocdev;
> > +	char *regs_buff = p;
> > +
> > +	memset(regs_buff, 0, VNIC_REGS_CNT * VNIC_REGS_LINE_LEN);
> > +
> > +	regs->version = 1;
> > +
> > +	sprintf(regs_buff, "%08Lx = %08x\n",
> > +		GETRELADDR(VREG_BMC_GLOBAL, VIOC_BMC, 0),
> > +		VIOC_READ_REG(VREG_BMC_GLOBAL, VIOC_BMC, 0, viocdev));
> > +	regs_buff += strlen(regs_buff);
> > +
> > +	sprintf(regs_buff, "%08Lx = %08x\n",
> > +		GETRELADDR(VREG_BMC_DEBUG, VIOC_BMC, 0),
> > +		VIOC_READ_REG(VREG_BMC_DEBUG, VIOC_BMC, 0, viocdev));
> > +	regs_buff += strlen(regs_buff);
> > +
> > +	sprintf(regs_buff, "%08Lx = %08x\n",
> > +		GETRELADDR(VREG_BMC_DEBUGPRIV, VIOC_BMC, 0),
> > +		VIOC_READ_REG(VREG_BMC_DEBUGPRIV, VIOC_BMC, 0, viocdev));
> > +	regs_buff += strlen(regs_buff);
> > +
> > +	sprintf(regs_buff, "%08Lx = %08x\n",
> > +		GETRELADDR(VREG_BMC_FABRIC, VIOC_BMC, 0),
> > +		VIOC_READ_REG(VREG_BMC_FABRIC, VIOC_BMC, 0, viocdev));
> > +	regs_buff += strlen(regs_buff);
> > +
> > +	sprintf(regs_buff, "%08Lx = %08x\n",
> > +		GETRELADDR(VREG_BMC_VNIC_EN, VIOC_BMC, 0),
> > +		VIOC_READ_REG(VREG_BMC_VNIC_EN, VIOC_BMC, 0, viocdev));
> > +	regs_buff += strlen(regs_buff);
> > +
> > +	sprintf(regs_buff, "%08Lx = %08x\n",
> > +		GETRELADDR(VREG_BMC_PORT_EN, VIOC_BMC, 0),
> > +		VIOC_READ_REG(VREG_BMC_PORT_EN, VIOC_BMC, 0, viocdev));
> > +	regs_buff += strlen(regs_buff);
> > +
> > +	sprintf(regs_buff, "%08Lx = %08x\n",
> > +		GETRELADDR(VREG_BMC_VNIC_CFG, VIOC_BMC, 0),
> > +		VIOC_READ_REG(VREG_BMC_VNIC_CFG, VIOC_BMC, 0, viocdev));
> > +	regs_buff += strlen(regs_buff);
> > +
> > +	sprintf(regs_buff, "%08Lx = %08x\n",
> > +		GETRELADDR(VREG_IHCU_RXDQEN, VIOC_IHCU, 0),
> > +		VIOC_READ_REG(VREG_IHCU_RXDQEN, VIOC_IHCU, 0, viocdev));
> > +	regs_buff += strlen(regs_buff);
> > +
> > +	sprintf(regs_buff, "%08Lx = %08x\n",
> > +		GETRELADDR(VREG_VENG_VLANTAG, VIOC_VENG, vnicdev->vnic_id),
> > +		VIOC_READ_REG(VREG_VENG_VLANTAG, VIOC_VENG, vnicdev->vnic_id,
> > +			      viocdev));
> > +	regs_buff += strlen(regs_buff);
> > +
> > +	sprintf(regs_buff, "%08Lx = %08x\n",
> > +		GETRELADDR(VREG_VENG_TXD_CTL, VIOC_VENG, vnicdev->vnic_id),
> > +		VIOC_READ_REG(VREG_VENG_TXD_CTL, VIOC_VENG, vnicdev->vnic_id,
> > +			      viocdev));
> > +	regs_buff += strlen(regs_buff);
> > +
> > +}
>
> This looks ugly. What interface is that?
This is the interface between  the driver and ethtool.
Using the text buffer is one way to keep changed limited to one side (driver). Ultimately, I think that this ethtool function (dumping hw registers) should become more generic,
as opposed to what it is now - unique for every individual driver.
> 							Pavel

-- 
Misha Tomushev
[email protected]


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux