netdev - RE: [PATCH 1/6] drivers: net: ethernet: cpsw: add support for CPSW register offset changes in different IP version

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <EB1619762EAF8B4E97A227FB77B7E0293E9FD69E@DBDE01.ent.ti.com>
Date:	Mon, 22 Oct 2012 10:39:51 +0000
From:	"N, Mugunthan V" <mugunthanvnm@...com>
To:	Richard Cochran <richardcochran@...il.com>
CC:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"davem@...emloft.net" <davem@...emloft.net>
Subject: RE: [PATCH 1/6] drivers: net: ethernet: cpsw: add support for CPSW
 register offset changes in different IP version

> -----Original Message-----
> From: Richard Cochran [mailto:richardcochran@...il.com]
> Sent: Thursday, October 18, 2012 8:15 AM
> To: N, Mugunthan V
> Cc: netdev@...r.kernel.org; davem@...emloft.net
> Subject: Re: [PATCH 1/6] drivers: net: ethernet: cpsw: add support for
> CPSW register offset changes in different IP version
> 
> On Wed, Oct 17, 2012 at 04:15:13AM +0530, Mugunthan V N wrote:
> >

> > +
> > +static const u32 slave_reg_map_ip_v2[] = {
> > +	[PORT_CONTROL]	= 0x00,
> > +	[TS_CONTROL]	= 0x04,
> 
> You don't make use of this register in your driver, so what is the
> point?

This register is used in next version of CPSW. Since I don't use it in
current version I will remove the same in next version patch series.

> 
> > +	[MAX_BLKS]	= 0x08,
> > +	[BLK_CNT]	= 0x0c,
> > +	[FLOW_THRESH]	= 0x10,
> > +	[PORT_VLAN]	= 0x14,
> > +	[TX_PRI_MAP]	= 0x18,
> > +	[TS_SEQ_MTYPE]	= 0x1c,
> > +	[SA_LO]		= 0x20,
> > +	[SA_HI]		= 0x24,
> > +};
> > +
> 
> This is wasting memory with unused static stables. There is a better
> way to handle this issue.

I have taken the code reference from the following driver.
drivers/i2c/busses/i2c-omap.c

Can you refer other better solution to handle this?

> 
> >  static int debug_level;
> >  module_param(debug_level, int, 0);
> >  MODULE_PARM_DESC(debug_level, "cpsw debug level (NETIF_MSG bits)");

>  	pm_runtime_get_sync(&priv->pdev->dev);
> >
> > -	reg = __raw_readl(&priv->regs->id_ver);
> > +	reg = readl(&priv->regs->id_ver);
> > +	priv->cpsw_version = reg;
> > +	if (reg == CPSW_VERSION_1)
> > +		priv->slave_reg_ofs = (u32 *)slave_reg_map_ip_v1;
> > +	else
> > +		priv->slave_reg_ofs = (u32 *)slave_reg_map_ip_v2;
> 
> You didn't provide a way to even use this code, like a dts for a
> non-am335x board with the older version.
> 
> I think it would be better to start off supporting one version and
> have that fully working, and then add the older version, but *really*
> add it so that it is actually working.
> 

Since version info from hardware registers can be used to differentiate between
the CPSW versions so I don't think there is a need to provide the same through DT.

Regards
Mugunthan V N

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html