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: <20121022112334.GB2285@netboy.at.omicron.at>
Date:	Mon, 22 Oct 2012 13:23:34 +0200
From:	Richard Cochran <richardcochran@...il.com>
To:	"N, Mugunthan V" <mugunthanvnm@...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

On Mon, Oct 22, 2012 at 10:39:51AM +0000, N, Mugunthan V wrote:
> > 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't speak for that driver.

BTW the ALE driver is also horribly wasting space with the "struct
ale_control_info ale_controls[ALE_NUM_CONTROLS]" thing.

> Can you refer other better solution to handle this?

Yes, I can think of two different ways. Maybe you can think of yet
other ways.

1. For those few registers that are not aligned the same way but have
   the same bit layout (and you actually use in the driver), keep a
   separate pointer in your driver's private struct.

2. Make two different declarations of structs corresponding to two
   register layouts and use a cast in the access function based on
   version.

I object to the tables of offsets because these take up twice the
memory of the registers themselves, even if you don't use all of the
registers.

> > 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.

I did not say to put the versions into the DT.

What I meant was that there is no need to add code that tests the
version and acts differently, if there are no users of the special
cases.

Thanks,
Richard

--
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