All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] net: phy: fix phy_start to consider PHY_IGNORE_INTERRUPT
@ 2018-02-07 18:44 Heiner Kallweit
  2018-02-07 19:06 ` Florian Fainelli
  0 siblings, 1 reply; 5+ messages in thread
From: Heiner Kallweit @ 2018-02-07 18:44 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev

This condition wasn't adjusted when PHY_IGNORE_INTERRUPT (-2) was added
long ago. In case of PHY_IGNORE_INTERRUPT the MAC interrupt indicates
also PHY state changes and we should do what the symbol says.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/phy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index f3313a129..50ed35a45 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -822,7 +822,7 @@ void phy_start(struct phy_device *phydev)
 		phy_resume(phydev);
 
 		/* make sure interrupts are re-enabled for the PHY */
-		if (phydev->irq != PHY_POLL) {
+		if (phydev->irq > 0) {
 			err = phy_enable_interrupts(phydev);
 			if (err < 0)
 				break;
-- 
2.16.1

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

* Re: [PATCH net] net: phy: fix phy_start to consider PHY_IGNORE_INTERRUPT
  2018-02-07 18:44 [PATCH net] net: phy: fix phy_start to consider PHY_IGNORE_INTERRUPT Heiner Kallweit
@ 2018-02-07 19:06 ` Florian Fainelli
  2018-02-07 19:26   ` Heiner Kallweit
  0 siblings, 1 reply; 5+ messages in thread
From: Florian Fainelli @ 2018-02-07 19:06 UTC (permalink / raw)
  To: Heiner Kallweit, Andrew Lunn, David Miller; +Cc: netdev



On 02/07/2018 10:44 AM, Heiner Kallweit wrote:
> This condition wasn't adjusted when PHY_IGNORE_INTERRUPT (-2) was added
> long ago. In case of PHY_IGNORE_INTERRUPT the MAC interrupt indicates
> also PHY state changes and we should do what the symbol says.

Do you use phy_enable_interrupts() to configure how the PHY interrupts
will be flowing through the Ethernet MAC?

> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/net/phy/phy.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index f3313a129..50ed35a45 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -822,7 +822,7 @@ void phy_start(struct phy_device *phydev)
>  		phy_resume(phydev);
>  
>  		/* make sure interrupts are re-enabled for the PHY */
> -		if (phydev->irq != PHY_POLL) {
> +		if (phydev->irq > 0) {
>  			err = phy_enable_interrupts(phydev);
>  			if (err < 0)
>  				break;
> 

-- 
Florian

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

* Re: [PATCH net] net: phy: fix phy_start to consider PHY_IGNORE_INTERRUPT
  2018-02-07 19:06 ` Florian Fainelli
@ 2018-02-07 19:26   ` Heiner Kallweit
  2018-02-07 19:34     ` Florian Fainelli
  0 siblings, 1 reply; 5+ messages in thread
From: Heiner Kallweit @ 2018-02-07 19:26 UTC (permalink / raw)
  To: Florian Fainelli, Andrew Lunn, David Miller; +Cc: netdev

Am 07.02.2018 um 20:06 schrieb Florian Fainelli:
> 
> 
> On 02/07/2018 10:44 AM, Heiner Kallweit wrote:
>> This condition wasn't adjusted when PHY_IGNORE_INTERRUPT (-2) was added
>> long ago. In case of PHY_IGNORE_INTERRUPT the MAC interrupt indicates
>> also PHY state changes and we should do what the symbol says.
> 
> Do you use phy_enable_interrupts() to configure how the PHY interrupts
> will be flowing through the Ethernet MAC?
> 
No. And I'm not sure I understand your question correctly.
The change applies the same behavior as e.g. in phy_connect_direct()
where phy_start_interrupts() is called only if phy_dev->irq > 0.

>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  drivers/net/phy/phy.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
>> index f3313a129..50ed35a45 100644
>> --- a/drivers/net/phy/phy.c
>> +++ b/drivers/net/phy/phy.c
>> @@ -822,7 +822,7 @@ void phy_start(struct phy_device *phydev)
>>  		phy_resume(phydev);
>>  
>>  		/* make sure interrupts are re-enabled for the PHY */
>> -		if (phydev->irq != PHY_POLL) {
>> +		if (phydev->irq > 0) {
>>  			err = phy_enable_interrupts(phydev);
>>  			if (err < 0)
>>  				break;
>>
> 

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

* Re: [PATCH net] net: phy: fix phy_start to consider PHY_IGNORE_INTERRUPT
  2018-02-07 19:26   ` Heiner Kallweit
@ 2018-02-07 19:34     ` Florian Fainelli
  2018-02-07 19:39       ` Heiner Kallweit
  0 siblings, 1 reply; 5+ messages in thread
From: Florian Fainelli @ 2018-02-07 19:34 UTC (permalink / raw)
  To: Heiner Kallweit, Andrew Lunn, David Miller; +Cc: netdev



On 02/07/2018 11:26 AM, Heiner Kallweit wrote:
> Am 07.02.2018 um 20:06 schrieb Florian Fainelli:
>>
>>
>> On 02/07/2018 10:44 AM, Heiner Kallweit wrote:
>>> This condition wasn't adjusted when PHY_IGNORE_INTERRUPT (-2) was added
>>> long ago. In case of PHY_IGNORE_INTERRUPT the MAC interrupt indicates
>>> also PHY state changes and we should do what the symbol says.
>>
>> Do you use phy_enable_interrupts() to configure how the PHY interrupts
>> will be flowing through the Ethernet MAC?
>>
> No. And I'm not sure I understand your question correctly.

No wonder, my question does not make sense, I read the test wrong.

> The change applies the same behavior as e.g. in phy_connect_direct()
> where phy_start_interrupts() is called only if phy_dev->irq > 0.

Not enough coffee, your change is fine, could you consider using
phy_interrupt_is_valid() instead for this test?

> 
>>>
>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>> ---
>>>  drivers/net/phy/phy.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
>>> index f3313a129..50ed35a45 100644
>>> --- a/drivers/net/phy/phy.c
>>> +++ b/drivers/net/phy/phy.c
>>> @@ -822,7 +822,7 @@ void phy_start(struct phy_device *phydev)
>>>  		phy_resume(phydev);
>>>  
>>>  		/* make sure interrupts are re-enabled for the PHY */
>>> -		if (phydev->irq != PHY_POLL) {
>>> +		if (phydev->irq > 0) {
>>>  			err = phy_enable_interrupts(phydev);
>>>  			if (err < 0)
>>>  				break;
>>>
>>
> 

-- 
Florian

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

* Re: [PATCH net] net: phy: fix phy_start to consider PHY_IGNORE_INTERRUPT
  2018-02-07 19:34     ` Florian Fainelli
@ 2018-02-07 19:39       ` Heiner Kallweit
  0 siblings, 0 replies; 5+ messages in thread
From: Heiner Kallweit @ 2018-02-07 19:39 UTC (permalink / raw)
  To: Florian Fainelli, Andrew Lunn, David Miller; +Cc: netdev

Am 07.02.2018 um 20:34 schrieb Florian Fainelli:
> 
> 
> On 02/07/2018 11:26 AM, Heiner Kallweit wrote:
>> Am 07.02.2018 um 20:06 schrieb Florian Fainelli:
>>>
>>>
>>> On 02/07/2018 10:44 AM, Heiner Kallweit wrote:
>>>> This condition wasn't adjusted when PHY_IGNORE_INTERRUPT (-2) was added
>>>> long ago. In case of PHY_IGNORE_INTERRUPT the MAC interrupt indicates
>>>> also PHY state changes and we should do what the symbol says.
>>>
>>> Do you use phy_enable_interrupts() to configure how the PHY interrupts
>>> will be flowing through the Ethernet MAC?
>>>
>> No. And I'm not sure I understand your question correctly.
> 
> No wonder, my question does not make sense, I read the test wrong.
> 
>> The change applies the same behavior as e.g. in phy_connect_direct()
>> where phy_start_interrupts() is called only if phy_dev->irq > 0.
> 
> Not enough coffee, your change is fine, could you consider using
> phy_interrupt_is_valid() instead for this test?
> 
Sure. I was considering this already however wasn't sure because
currently both ways to check for a valid interrupt are used in
phylib.

>>
>>>>
>>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>>> ---
>>>>  drivers/net/phy/phy.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
>>>> index f3313a129..50ed35a45 100644
>>>> --- a/drivers/net/phy/phy.c
>>>> +++ b/drivers/net/phy/phy.c
>>>> @@ -822,7 +822,7 @@ void phy_start(struct phy_device *phydev)
>>>>  		phy_resume(phydev);
>>>>  
>>>>  		/* make sure interrupts are re-enabled for the PHY */
>>>> -		if (phydev->irq != PHY_POLL) {
>>>> +		if (phydev->irq > 0) {
>>>>  			err = phy_enable_interrupts(phydev);
>>>>  			if (err < 0)
>>>>  				break;
>>>>
>>>
>>
> 

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

end of thread, other threads:[~2018-02-07 19:39 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-07 18:44 [PATCH net] net: phy: fix phy_start to consider PHY_IGNORE_INTERRUPT Heiner Kallweit
2018-02-07 19:06 ` Florian Fainelli
2018-02-07 19:26   ` Heiner Kallweit
2018-02-07 19:34     ` Florian Fainelli
2018-02-07 19:39       ` Heiner Kallweit

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.