All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/2] ethtool: changes of emac_regs structure accordingly within driver emac_regs structure.
@ 2015-08-05 11:01 Ivan Mikhaylov
  2015-08-27 10:39 ` Ivan Mikhaylov
  2015-09-05 10:41 ` Ben Hutchings
  0 siblings, 2 replies; 5+ messages in thread
From: Ivan Mikhaylov @ 2015-08-05 11:01 UTC (permalink / raw)
  To: netdev, linux-kernel, David S. Miller, Ben Hutchings

* do the redefinition of emac_regs struct from driver structure perspective
  and passing size from actual struct size, not from memory area variable
  which set in dts file.

* passing variable from dts option may cause a problem with output below 
  MII's section which we're fixing with this and 5369c71 commit in kernel.

Signed-off-by: Ivan Mikhaylov <ivan@ru.ibm.com>
---
 ibm_emac.c |  101 +++++++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 73 insertions(+), 28 deletions(-)

diff --git a/ibm_emac.c b/ibm_emac.c
index e128e48..da6c28e 100644
--- a/ibm_emac.c
+++ b/ibm_emac.c
@@ -32,35 +32,78 @@ struct emac_ethtool_regs_subhdr {
 };
 
 struct emac_regs {
-	u32 mr0;
-	u32 mr1;
-	u32 tmr0;
-	u32 tmr1;
-	u32 rmr;
-	u32 isr;
-	u32 iser;
-	u32 iahr;
-	u32 ialr;
-	u32 vtpid;
-	u32 vtci;
-	u32 ptr;
-	u32 iaht1;
-	u32 iaht2;
-	u32 iaht3;
-	u32 iaht4;
-	u32 gaht1;
-	u32 gaht2;
-	u32 gaht3;
-	u32 gaht4;
+	/* Common registers across all EMAC implementations. */
+	u32 mr0;			/* Special 	*/
+	u32 mr1;			/* Reset 	*/
+	u32 tmr0;			/* Special 	*/
+	u32 tmr1;			/* Special 	*/
+	u32 rmr;			/* Reset 	*/
+	u32 isr;			/* Always 	*/
+	u32 iser;			/* Reset 	*/
+	u32 iahr;			/* Reset, R, T 	*/
+	u32 ialr;			/* Reset, R, T 	*/
+	u32 vtpid;			/* Reset, R, T 	*/
+	u32 vtci;			/* Reset, R, T 	*/
+	u32 ptr;			/* Reset,    T 	*/
+	union {
+		/* Registers unique to EMAC4 implementations */
+		struct {
+			u32 iaht1;	/* Reset, R	*/
+			u32 iaht2;	/* Reset, R	*/
+			u32 iaht3;	/* Reset, R	*/
+			u32 iaht4;	/* Reset, R	*/
+			u32 gaht1;	/* Reset, R	*/
+			u32 gaht2;	/* Reset, R	*/
+			u32 gaht3;	/* Reset, R	*/
+			u32 gaht4;	/* Reset, R	*/
+		} emac4;
+		/* Registers unique to EMAC4SYNC implementations */
+		struct {
+			u32 mahr;	/* Reset, R, T  */
+			u32 malr;	/* Reset, R, T  */
+			u32 mmahr;	/* Reset, R, T  */
+			u32 mmalr;	/* Reset, R, T  */
+			u32 rsvd0[4];
+		} emac4sync;
+	} u0;
+	/* Common registers across all EMAC implementations. */
 	u32 lsah;
 	u32 lsal;
-	u32 ipgvr;
-	u32 stacr;
-	u32 trtr;
-	u32 rwmr;
+	u32 ipgvr;			/* Reset,    T 	*/
+	u32 stacr;			/* Special 	*/
+	u32 trtr;			/* Special 	*/
+	u32 rwmr;			/* Reset 	*/
 	u32 octx;
 	u32 ocrx;
-	u32 ipcr;
+	union {
+		/* Registers unique to EMAC4 implementations */
+		struct {
+			u32 ipcr;
+		} emac4;
+		/* Registers unique to EMAC4SYNC implementations */
+		struct {
+			u32 rsvd1;
+			u32 revid;
+			u32 rsvd2[2];
+			u32 iaht1;	/* Reset, R     */
+			u32 iaht2;	/* Reset, R     */
+			u32 iaht3;	/* Reset, R     */
+			u32 iaht4;	/* Reset, R     */
+			u32 iaht5;	/* Reset, R     */
+			u32 iaht6;	/* Reset, R     */
+			u32 iaht7;	/* Reset, R     */
+			u32 iaht8;	/* Reset, R     */
+			u32 gaht1;	/* Reset, R     */
+			u32 gaht2;	/* Reset, R     */
+			u32 gaht3;	/* Reset, R     */
+			u32 gaht4;	/* Reset, R     */
+			u32 gaht5;	/* Reset, R     */
+			u32 gaht6;	/* Reset, R     */
+			u32 gaht7;	/* Reset, R     */
+			u32 gaht8;	/* Reset, R     */
+			u32 tpc;	/* Reset, T     */
+		} emac4sync;
+	} u1;
 };
 
 struct mal_regs {
@@ -132,12 +175,14 @@ static void *print_emac_regs(void *buf)
 	       p->trtr, p->rwmr,
 	       p->iahr, p->ialr,
 	       p->lsah, p->lsal,
-	       p->iaht1, p->iaht2, p->iaht3, p->iaht4,
-	       p->gaht1, p->gaht2, p->gaht3, p->gaht4,
+	       p->u0.emac4.iaht1, p->u0.emac4.iaht2, p->u0.emac4.iaht3,
+	       p->u0.emac4.iaht4,
+	       p->u0.emac4.gaht1, p->u0.emac4.gaht2, p->u0.emac4.gaht3,
+	       p->u0.emac4.gaht4,
 	       p->vtpid, p->vtci, p->ipgvr, p->stacr, p->octx, p->ocrx);
 
 	if (hdr->version)
-		printf(" IPCR = 0x%08x\n\n", p->ipcr);
+		printf(" IPCR = 0x%08x\n\n", p->u1.emac4.ipcr);
 	else {
 		printf("\n\n");
 		res -= sizeof(u32);
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 1/2] ethtool: changes of emac_regs structure accordingly within driver emac_regs structure.
  2015-08-05 11:01 [PATCH v2 1/2] ethtool: changes of emac_regs structure accordingly within driver emac_regs structure Ivan Mikhaylov
@ 2015-08-27 10:39 ` Ivan Mikhaylov
  2015-09-05 10:41 ` Ben Hutchings
  1 sibling, 0 replies; 5+ messages in thread
From: Ivan Mikhaylov @ 2015-08-27 10:39 UTC (permalink / raw)
  To: netdev, linux-kernel, David S. Miller, Ben Hutchings

On Wed, 5 Aug 2015 15:01:53 +0400
Ivan Mikhaylov <ivan@ru.ibm.com> wrote:

ping

> * do the redefinition of emac_regs struct from driver structure
> perspective and passing size from actual struct size, not from memory
> area variable which set in dts file.
> 
> * passing variable from dts option may cause a problem with output
> below MII's section which we're fixing with this and 5369c71 commit
> in kernel.
> 
> Signed-off-by: Ivan Mikhaylov <ivan@ru.ibm.com>
> ---
>  ibm_emac.c |  101
> +++++++++++++++++++++++++++++++++++++++++++----------------- 1 file
> changed, 73 insertions(+), 28 deletions(-)
> 
> diff --git a/ibm_emac.c b/ibm_emac.c
> index e128e48..da6c28e 100644
> --- a/ibm_emac.c
> +++ b/ibm_emac.c
> @@ -32,35 +32,78 @@ struct emac_ethtool_regs_subhdr {
>  };
>  
>  struct emac_regs {
> -	u32 mr0;
> -	u32 mr1;
> -	u32 tmr0;
> -	u32 tmr1;
> -	u32 rmr;
> -	u32 isr;
> -	u32 iser;
> -	u32 iahr;
> -	u32 ialr;
> -	u32 vtpid;
> -	u32 vtci;
> -	u32 ptr;
> -	u32 iaht1;
> -	u32 iaht2;
> -	u32 iaht3;
> -	u32 iaht4;
> -	u32 gaht1;
> -	u32 gaht2;
> -	u32 gaht3;
> -	u32 gaht4;
> +	/* Common registers across all EMAC implementations. */
> +	u32 mr0;			/* Special 	*/
> +	u32 mr1;			/* Reset 	*/
> +	u32 tmr0;			/* Special 	*/
> +	u32 tmr1;			/* Special 	*/
> +	u32 rmr;			/* Reset 	*/
> +	u32 isr;			/* Always 	*/
> +	u32 iser;			/* Reset 	*/
> +	u32 iahr;			/* Reset, R, T 	*/
> +	u32 ialr;			/* Reset, R, T 	*/
> +	u32 vtpid;			/* Reset, R, T 	*/
> +	u32 vtci;			/* Reset, R, T 	*/
> +	u32 ptr;			/* Reset,    T 	*/
> +	union {
> +		/* Registers unique to EMAC4 implementations */
> +		struct {
> +			u32 iaht1;	/* Reset, R	*/
> +			u32 iaht2;	/* Reset, R	*/
> +			u32 iaht3;	/* Reset, R	*/
> +			u32 iaht4;	/* Reset, R	*/
> +			u32 gaht1;	/* Reset, R	*/
> +			u32 gaht2;	/* Reset, R	*/
> +			u32 gaht3;	/* Reset, R	*/
> +			u32 gaht4;	/* Reset, R	*/
> +		} emac4;
> +		/* Registers unique to EMAC4SYNC implementations */
> +		struct {
> +			u32 mahr;	/* Reset, R, T  */
> +			u32 malr;	/* Reset, R, T  */
> +			u32 mmahr;	/* Reset, R, T  */
> +			u32 mmalr;	/* Reset, R, T  */
> +			u32 rsvd0[4];
> +		} emac4sync;
> +	} u0;
> +	/* Common registers across all EMAC implementations. */
>  	u32 lsah;
>  	u32 lsal;
> -	u32 ipgvr;
> -	u32 stacr;
> -	u32 trtr;
> -	u32 rwmr;
> +	u32 ipgvr;			/* Reset,    T 	*/
> +	u32 stacr;			/* Special 	*/
> +	u32 trtr;			/* Special 	*/
> +	u32 rwmr;			/* Reset 	*/
>  	u32 octx;
>  	u32 ocrx;
> -	u32 ipcr;
> +	union {
> +		/* Registers unique to EMAC4 implementations */
> +		struct {
> +			u32 ipcr;
> +		} emac4;
> +		/* Registers unique to EMAC4SYNC implementations */
> +		struct {
> +			u32 rsvd1;
> +			u32 revid;
> +			u32 rsvd2[2];
> +			u32 iaht1;	/* Reset, R     */
> +			u32 iaht2;	/* Reset, R     */
> +			u32 iaht3;	/* Reset, R     */
> +			u32 iaht4;	/* Reset, R     */
> +			u32 iaht5;	/* Reset, R     */
> +			u32 iaht6;	/* Reset, R     */
> +			u32 iaht7;	/* Reset, R     */
> +			u32 iaht8;	/* Reset, R     */
> +			u32 gaht1;	/* Reset, R     */
> +			u32 gaht2;	/* Reset, R     */
> +			u32 gaht3;	/* Reset, R     */
> +			u32 gaht4;	/* Reset, R     */
> +			u32 gaht5;	/* Reset, R     */
> +			u32 gaht6;	/* Reset, R     */
> +			u32 gaht7;	/* Reset, R     */
> +			u32 gaht8;	/* Reset, R     */
> +			u32 tpc;	/* Reset, T     */
> +		} emac4sync;
> +	} u1;
>  };
>  
>  struct mal_regs {
> @@ -132,12 +175,14 @@ static void *print_emac_regs(void *buf)
>  	       p->trtr, p->rwmr,
>  	       p->iahr, p->ialr,
>  	       p->lsah, p->lsal,
> -	       p->iaht1, p->iaht2, p->iaht3, p->iaht4,
> -	       p->gaht1, p->gaht2, p->gaht3, p->gaht4,
> +	       p->u0.emac4.iaht1, p->u0.emac4.iaht2,
> p->u0.emac4.iaht3,
> +	       p->u0.emac4.iaht4,
> +	       p->u0.emac4.gaht1, p->u0.emac4.gaht2,
> p->u0.emac4.gaht3,
> +	       p->u0.emac4.gaht4,
>  	       p->vtpid, p->vtci, p->ipgvr, p->stacr, p->octx,
> p->ocrx); 
>  	if (hdr->version)
> -		printf(" IPCR = 0x%08x\n\n", p->ipcr);
> +		printf(" IPCR = 0x%08x\n\n", p->u1.emac4.ipcr);
>  	else {
>  		printf("\n\n");
>  		res -= sizeof(u32);


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 1/2] ethtool: changes of emac_regs structure accordingly within driver emac_regs structure.
  2015-08-05 11:01 [PATCH v2 1/2] ethtool: changes of emac_regs structure accordingly within driver emac_regs structure Ivan Mikhaylov
  2015-08-27 10:39 ` Ivan Mikhaylov
@ 2015-09-05 10:41 ` Ben Hutchings
  1 sibling, 0 replies; 5+ messages in thread
From: Ben Hutchings @ 2015-09-05 10:41 UTC (permalink / raw)
  To: Ivan Mikhaylov, netdev, linux-kernel, David S. Miller

[-- Attachment #1: Type: text/plain, Size: 870 bytes --]

On Wed, 2015-08-05 at 15:01 +0400, Ivan Mikhaylov wrote:
> * do the redefinition of emac_regs struct from driver structure perspective
>   and passing size from actual struct size, not from memory area variable
>   which set in dts file.
> 
> * passing variable from dts option may cause a problem with output below 
>   MII's section which we're fixing with this and 5369c71 commit in kernel.
[...]

But you still aren't handling the case where only one of the driver and
ethtool is upgraded, even by reporting an error.

I'm never going to apply patches to ethtool that obviously break binary
compatibility, so you are wasting your time sending me new versions
that do that.

At this point you should probably just bump the dump version numbers in
both places.

Ben.

-- 
Ben Hutchings
friends: People who know you well, but like you anyway.


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 1/2] ethtool: changes of emac_regs structure accordingly within driver emac_regs structure.
@ 2015-09-09 13:05 Ivan Mikhaylov
  0 siblings, 0 replies; 5+ messages in thread
From: Ivan Mikhaylov @ 2015-09-09 13:05 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: netdev, linux-kernel, David S. Miller

On Wed, 9 Aug 2015 15:10:00 +0400
Ivan Mikhaylov <ivan@ru.ibm.com> wrote:

>On Wed, 2015-08-05 at 15:01 +0400, Ivan Mikhaylov wrote:
>> * do the redefinition of emac_regs struct from driver structure
>> perspective and passing size from actual struct size, not from memory
>> area variable which set in dts file.
>> 
>> * passing variable from dts option may cause a problem with output
>> below MII's section which we're fixing with this and 5369c71 commit
>> in kernel.
>[...]
>
>But you still aren't handling the case where only one of the driver and
>ethtool is upgraded, even by reporting an error.
>
>I'm never going to apply patches to ethtool that obviously break binary
>compatibility, so you are wasting your time sending me new versions
>that do that.
>
>At this point you should probably just bump the dump version numbers in
>both places.
>
>Ben.
>
>-- 
>Ben Hutchings
>friends: People who know you well, but like you anyway.
>
>[unhandled content-type:application/pgp-signature]

Ben, thanks for looking to that. 

I decided to divide patches for 
1 - structure change which fixing problem
2 - add EMAC4SYNC
May be it wasn't clear from first patch...

There as a note to you with questions without answer:
https://lkml.org/lkml/2015/6/23/466

Also I said in that thread I can prove with table of 
 1. old ethtool old driver
 2. patched ethtool old driver
 3. patched ethtool patched driver
 4. old ethtool patched driver

if you want to see difference in output on machine what I've.
 
Can we talk on some irc channel to resolve that situation,
it will be much faster, where I can find you on irc.debian.org?

Thanks in advance.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 1/2] ethtool: changes of emac_regs structure accordingly within driver emac_regs structure.
@ 2015-09-09 11:28 Ivan Mikhaylov
  0 siblings, 0 replies; 5+ messages in thread
From: Ivan Mikhaylov @ 2015-09-09 11:28 UTC (permalink / raw)
  To: netdev, linux-kernel, David S. Miller, Ben Hutchings

On Wed, 9 Aug 2015 15:10:00 +0400
Ivan Mikhaylov <ivan@ru.ibm.com> wrote:

>On Wed, 2015-08-05 at 15:01 +0400, Ivan Mikhaylov wrote:
>> * do the redefinition of emac_regs struct from driver structure
>> perspective and passing size from actual struct size, not from memory
>> area variable which set in dts file.
>> 
>> * passing variable from dts option may cause a problem with output
>> below MII's section which we're fixing with this and 5369c71 commit
>> in kernel.
>[...]
>
>But you still aren't handling the case where only one of the driver and
>ethtool is upgraded, even by reporting an error.
>
>I'm never going to apply patches to ethtool that obviously break binary
>compatibility, so you are wasting your time sending me new versions
>that do that.
>
>At this point you should probably just bump the dump version numbers in
>both places.
>
>Ben.
>
>-- 
>Ben Hutchings
>friends: People who know you well, but like you anyway.
>
>[unhandled content-type:application/pgp-signature]

Ben, thanks for looking to that. 

I decided to divide patches for 
1 - structure change which fixing problem
2 - add EMAC4SYNC
May be it wasn't clear from first patch...

There as a note to you with questions without answer:
https://lkml.org/lkml/2015/6/23/466

Also I said in that thread I can prove with table of 
 1. old ethtool old driver
 2. patched ethtool old driver
 3. patched ethtool patched driver
 4. old ethtool patched driver

if you want to see difference in output on machine what I've.
 
Can we talk on some irc channel to resolve that situation,
it will be much faster, where I can find you on irc.debian.org?

Thanks in advance.


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-09-09 14:10 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-05 11:01 [PATCH v2 1/2] ethtool: changes of emac_regs structure accordingly within driver emac_regs structure Ivan Mikhaylov
2015-08-27 10:39 ` Ivan Mikhaylov
2015-09-05 10:41 ` Ben Hutchings
2015-09-09 11:28 Ivan Mikhaylov
2015-09-09 13:05 Ivan Mikhaylov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.