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

[Richard Cochran <richardcochran@gmail.com>]

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