Re: [PATCH 2/2] net/ibm/emac: fix size of emac dump memory areas

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Tue, 2015-05-12 at 17:12 +0400, Ivan Mikhaylov wrote:
> Fix in send of emac regs dump to ethtool which
> causing wrong data interpretation on ethtool
> layer for MII and EMAC.

Please provide a better explanation. You removed the difference
in register space size between EMAC and EMAC4, why ? You also changed a
test regarding which version of the structure to use from EMAC4 to
EMAC4SYNC, why ?

> Signed-off-by: Ivan Mikhaylov <ivan@ru.ibm.com>
> ---
>  drivers/net/ethernet/ibm/emac/core.c |   14 ++++----------
>  drivers/net/ethernet/ibm/emac/core.h |    4 ----
>  2 files changed, 4 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ibm/emac/core.c b/drivers/net/ethernet/ibm/emac/core.c
> index de79193..3dc6534 100644
> --- a/drivers/net/ethernet/ibm/emac/core.c
> +++ b/drivers/net/ethernet/ibm/emac/core.c
> @@ -2084,12 +2084,8 @@ static void emac_ethtool_get_pauseparam(struct net_device *ndev,
>  
>  static int emac_get_regs_len(struct emac_instance *dev)
>  {
> -	if (emac_has_feature(dev, EMAC_FTR_EMAC4))
> -		return sizeof(struct emac_ethtool_regs_subhdr) +
> -			EMAC4_ETHTOOL_REGS_SIZE(dev);
> -	else
>  		return sizeof(struct emac_ethtool_regs_subhdr) +
> -			EMAC_ETHTOOL_REGS_SIZE(dev);
> +			sizeof(struct emac_regs);
>  }
>  
>  static int emac_ethtool_get_regs_len(struct net_device *ndev)
> @@ -2114,15 +2110,13 @@ static void *emac_dump_regs(struct emac_instance *dev, void *buf)
>  	struct emac_ethtool_regs_subhdr *hdr = buf;
>  
>  	hdr->index = dev->cell_index;
> -	if (emac_has_feature(dev, EMAC_FTR_EMAC4)) {
> +	if (emac_has_feature(dev, EMAC_FTR_EMAC4SYNC)) {
>  		hdr->version = EMAC4_ETHTOOL_REGS_VER;
> -		memcpy_fromio(hdr + 1, dev->emacp, EMAC4_ETHTOOL_REGS_SIZE(dev));
> -		return (void *)(hdr + 1) + EMAC4_ETHTOOL_REGS_SIZE(dev);
>  	} else {
>  		hdr->version = EMAC_ETHTOOL_REGS_VER;
> -		memcpy_fromio(hdr + 1, dev->emacp, EMAC_ETHTOOL_REGS_SIZE(dev));
> -		return (void *)(hdr + 1) + EMAC_ETHTOOL_REGS_SIZE(dev);
>  	}
> +	memcpy_fromio(hdr + 1, dev->emacp, sizeof(struct emac_regs));
> +	return (void *)(hdr + 1) + sizeof(struct emac_regs);
>  }
>  
>  static void emac_ethtool_get_regs(struct net_device *ndev,
> diff --git a/drivers/net/ethernet/ibm/emac/core.h b/drivers/net/ethernet/ibm/emac/core.h
> index 67f342a..db90819 100644
> --- a/drivers/net/ethernet/ibm/emac/core.h
> +++ b/drivers/net/ethernet/ibm/emac/core.h
> @@ -461,10 +461,6 @@ struct emac_ethtool_regs_subhdr {
>  };
>  
>  #define EMAC_ETHTOOL_REGS_VER		0
> -#define EMAC_ETHTOOL_REGS_SIZE(dev) 	((dev)->rsrc_regs.end - \
> -					 (dev)->rsrc_regs.start + 1)
>  #define EMAC4_ETHTOOL_REGS_VER      	1
> -#define EMAC4_ETHTOOL_REGS_SIZE(dev)	((dev)->rsrc_regs.end -	\
> -					 (dev)->rsrc_regs.start + 1)
>  
>  #endif /* __IBM_NEWEMAC_CORE_H */