[PATCH net v4] r8169: Bad implementation of netif_carrier_*

[PATCH net v4] r8169: Bad implementation of netif_carrier_*

[Corcodel Marian]

  On probe stage what carrier to stop and rest two situation
  netif_carrier_* is slow and disturbing autonegociation process.

Signed-off-by: Corcodel Marian <asd@marian1000.go.ro>
---
 drivers/net/ethernet/realtek/r8169.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 1f75478..0f1adfa 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -1675,11 +1675,9 @@ static void __rtl8169_check_link_status(struct net_device *dev,
 		/* This is to cancel a scheduled suspend if there's one. */
 		if (pm)
 			pm_request_resume(&tp->pci_dev->dev);
-		netif_carrier_on(dev);
 		if (net_ratelimit())
 			netif_info(tp, ifup, dev, "link up\n");
 	} else {
-		netif_carrier_off(dev);
 		netif_info(tp, ifdown, dev, "link down\n");
 		if (pm)
 			pm_schedule_suspend(&tp->pci_dev->dev, 5000);
@@ -8441,7 +8439,6 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (pci_dev_run_wake(pdev))
 		pm_runtime_put_noidle(&pdev->dev);
 
-	netif_carrier_off(dev);
 
 out:
 	return rc;
-- 
2.5.0

Re: [PATCH net v4] r8169: Bad implementation of netif_carrier_*

[David Miller]

From: Corcodel Marian <asd@marian1000.go.ro>
Date: Thu, 11 Feb 2016 08:27:43 +0200

>   On probe stage what carrier to stop and rest two situation
>   netif_carrier_* is slow and disturbing autonegociation process.
> 
> Signed-off-by: Corcodel Marian <asd@marian1000.go.ro>

This doesn't tell us at all why your change is valid.  You aren't
going into the details of what you mean by "disturbing" and why it
being "slow" is really a problem.

In general, your patches are of a very low quality.  You give never
give us enough information to properly evaluate your patch.

It is unlikely that even one of your patches will be applied by
me until you fix this fundamental problem with your submissions.

Re: [PATCH net v4] r8169: Bad implementation of netif_carrier_*

[Corcodel Marian]

On Thu, 2016-02-11 at 03:39 -0500, David Miller wrote:
> From: Corcodel Marian <asd@marian1000.go.ro>
> Date: Thu, 11 Feb 2016 08:27:43 +0200
> 
> >   On probe stage what carrier to stop and rest two situation
> >   netif_carrier_* is slow and disturbing autonegociation process.
> > 
> > Signed-off-by: Corcodel Marian <asd@marian1000.go.ro>
> 
> This doesn't tell us at all why your change is valid.  You aren't
> going into the details of what you mean by "disturbing" and why it
> being "slow" is really a problem.
> 
> In general, your patches are of a very low quality.  You give never
> give us enough information to properly evaluate your patch.
> 
> It is unlikely that even one of your patches will be applied by
> me until you fix this fundamental problem with your submissions.

May bee netif_carrier_* is good on slow changes of link good status of
interrupt ,is unable to keep track changes from link good interrupt.

Re: [PATCH net v4] r8169: Bad implementation of netif_carrier_*

[David Miller]

From: Corcodel Marian <asd@marian1000.go.ro>
Date: Thu, 11 Feb 2016 12:52:49 +0200

> On Thu, 2016-02-11 at 03:39 -0500, David Miller wrote:
>> From: Corcodel Marian <asd@marian1000.go.ro>
>> Date: Thu, 11 Feb 2016 08:27:43 +0200
>> 
>> >   On probe stage what carrier to stop and rest two situation
>> >   netif_carrier_* is slow and disturbing autonegociation process.
>> > 
>> > Signed-off-by: Corcodel Marian <asd@marian1000.go.ro>
>> 
>> This doesn't tell us at all why your change is valid.  You aren't
>> going into the details of what you mean by "disturbing" and why it
>> being "slow" is really a problem.
>> 
>> In general, your patches are of a very low quality.  You give never
>> give us enough information to properly evaluate your patch.
>> 
>> It is unlikely that even one of your patches will be applied by
>> me until you fix this fundamental problem with your submissions.
> 
> May bee netif_carrier_* is good on slow changes of link good status of
> interrupt ,is unable to keep track changes from link good interrupt.

You're still not telling us anything.

The situation with your submissions is not improving.

Sorry.

Re: [PATCH net v4] r8169: Bad implementation of netif_carrier_*

[Corcodel Marian]

On Thu, 2016-02-11 at 06:12 -0500, David Miller wrote:
> From: Corcodel Marian <asd@marian1000.go.ro>
> Date: Thu, 11 Feb 2016 12:52:49 +0200
> 
> > On Thu, 2016-02-11 at 03:39 -0500, David Miller wrote:
> >> From: Corcodel Marian <asd@marian1000.go.ro>
> >> Date: Thu, 11 Feb 2016 08:27:43 +0200
> >> 
> >> >   On probe stage what carrier to stop and rest two situation
> >> >   netif_carrier_* is slow and disturbing autonegociation
> process.
> >> > 
> >> > Signed-off-by: Corcodel Marian <asd@marian1000.go.ro>
> >> 
> >> This doesn't tell us at all why your change is valid.  You aren't
> >> going into the details of what you mean by "disturbing" and why it
> >> being "slow" is really a problem.
> >> 
> >> In general, your patches are of a very low quality.  You give
> never
> >> give us enough information to properly evaluate your patch.
> >> 
> >> It is unlikely that even one of your patches will be applied by
> >> me until you fix this fundamental problem with your submissions.
> > 
> > May bee netif_carrier_* is good on slow changes of link good status
> of
> > interrupt ,is unable to keep track changes from link good
> interrupt.
> 
> You're still not telling us anything.
> 
> The situation with your submissions is not improving.
> 
> Sorry.
 
On my opinion  dmesg  must have one line with "link down" and one line
with "link up"
One mee result of dmesg | grep r8169 is :

$ dmesg | grep r8169
[    2.476418] r8169: module verification failed: signature and/or
required key missing - tainting kernel
[    2.477605] r8169 Gigabit Ethernet driver 2.3LK-NAPI loaded
[    2.478760] r8169 0000:01:00.0 eth0: RTL8102e at 0xf842a000,
00:22:15:4b:fd:ce, XID 14a00000 IRQ 24
[   39.513256] r8169 0000:01:00.0 eth0: link down
[   39.513284] r8169 0000:01:00.0 eth0: link down
[   53.153284] r8169 0000:01:00.0 eth0: link up

On good cases, on worst case "link down" and "link up" messages will
succeed from 3-4 times and produce unlikely result with netif_carrier_*
on  combination.

Re: [PATCH net v4] r8169: Bad implementation of netif_carrier_*

[Corcodel Marian]

On Thu, 2016-02-11 at 08:27 +0200, Corcodel Marian wrote:
>   On probe stage what carrier to stop and rest two situation
>   netif_carrier_* is slow and disturbing autonegociation process.
> 
> Signed-off-by: Corcodel Marian <asd@marian1000.go.ro>
> ---
>  drivers/net/ethernet/realtek/r8169.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/realtek/r8169.c
> b/drivers/net/ethernet/realtek/r8169.c
> index 1f75478..0f1adfa 100644
> --- a/drivers/net/ethernet/realtek/r8169.c
> +++ b/drivers/net/ethernet/realtek/r8169.c
> @@ -1675,11 +1675,9 @@ static void __rtl8169_check_link_status(struct
> net_device *dev,
>  		/* This is to cancel a scheduled suspend if there's
> one. */
>  		if (pm)
>  			pm_request_resume(&tp->pci_dev->dev);
> -		netif_carrier_on(dev);
>  		if (net_ratelimit())
>  			netif_info(tp, ifup, dev, "link up\n");
>  	} else {
> -		netif_carrier_off(dev);
>  		netif_info(tp, ifdown, dev, "link down\n");
>  		if (pm)
>  			pm_schedule_suspend(&tp->pci_dev->dev,
> 5000);
> @@ -8441,7 +8439,6 @@ static int rtl_init_one(struct pci_dev *pdev,
> const struct pci_device_id *ent)
>  	if (pci_dev_run_wake(pdev))
>  		pm_runtime_put_noidle(&pdev->dev);
>  
> -	netif_carrier_off(dev);
>  
>  out:
>  	return rc;
How is status with this patch-is accepted or not?
I'm interresed by this patch on order to send another patches wich
depend this.

Re: [PATCH net v4] r8169: Bad implementation of netif_carrier_*

[David Miller]

From: Corcodel Marian <asd@marian1000.go.ro>
Date: Fri, 12 Feb 2016 15:40:07 +0200

> How is status with this patch-is accepted or not?
> I'm interresed by this patch on order to send another patches wich
> depend this.

The patch is rejected.  It will not be looked at further.  It is dead.

And you can submit this patch a thousand times in this manner and it
will continue to be rejected.

You must improve the quality of your submissions by making your
explanations complete and understandable by other people.

Currently they are not.