[Intel-wired-lan] [PATCH] intel-wired-lan: igc: set TP bit in ethtool_link_ksettings.supported field

[Intel-wired-lan] [PATCH] intel-wired-lan: igc: set TP bit in ethtool_link_ksettings.supported field

[prasad]

From: Prasad Koya <prasad@arista.com>

if the physical media is twisted pair copper, set the TP bit in the
'supported' field

Signed-off-by: Prasad Koya <prasad@arista.com>
---
 drivers/net/ethernet/intel/igc/igc_ethtool.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c b/drivers/net/ethernet/intel/igc/igc_ethtool.c
index 8cc077b712ad..7d197fa80d5d 100644
--- a/drivers/net/ethernet/intel/igc/igc_ethtool.c
+++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c
@@ -1707,6 +1707,8 @@ static int igc_ethtool_get_link_ksettings(struct net_device *netdev,
 	/* twisted pair */
 	cmd->base.port = PORT_TP;
 	cmd->base.phy_address = hw->phy.addr;
+	if (hw->phy.media_type == igc_media_type_copper)
+		ethtool_link_ksettings_add_link_mode(cmd, supported, TP);
 
 	/* advertising link modes */
 	if (hw->phy.autoneg_advertised & ADVERTISE_10_HALF)
-- 
2.25.1

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Re: [Intel-wired-lan] [PATCH] intel-wired-lan: igc: set TP bit in ethtool_link_ksettings.supported field

[Neftin, Sasha]

On 6/1/2023 21:53, prasad@arista.com wrote:
> From: Prasad Koya <prasad@arista.com>
> 
> if the physical media is twisted pair copper, set the TP bit in the
> 'supported' field
> 
> Signed-off-by: Prasad Koya <prasad@arista.com>
> ---
>   drivers/net/ethernet/intel/igc/igc_ethtool.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c b/drivers/net/ethernet/intel/igc/igc_ethtool.c
> index 8cc077b712ad..7d197fa80d5d 100644
> --- a/drivers/net/ethernet/intel/igc/igc_ethtool.c
> +++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c
> @@ -1707,6 +1707,8 @@ static int igc_ethtool_get_link_ksettings(struct net_device *netdev,
>   	/* twisted pair */
>   	cmd->base.port = PORT_TP;
>   	cmd->base.phy_address = hw->phy.addr;
> +	if (hw->phy.media_type == igc_media_type_copper)
Thank you Prasad. i225/6 parts supported only copper media type. We can 
drop the "if" condition.
> +		ethtool_link_ksettings_add_link_mode(cmd, supported, TP);
Do you want to see: "Supported ports: [ TP ]"? That's right.
>   
>   	/* advertising link modes */
>   	if (hw->phy.autoneg_advertised & ADVERTISE_10_HALF)

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Re: [Intel-wired-lan] [PATCH] intel-wired-lan: igc: set TP bit in ethtool_link_ksettings.supported field

[Prasad Koya]

[-- Attachment #1.1: Type: text/plain, Size: 1755 bytes --]

Thanks for the quick review.

Yes, we use ETHTOOL_GLINKSETTINGS ioctl to retrieve interface settings and
expect to see one of TP or MII set in the 'supported' bitmask.

I'll send out a new patch removing the if(). Would you accept the patch
into your staging tree and later push it to the stable kernel branch? This
is my first time sending to intel-wired-lan. Not sure how it works.

Thank you.

On Fri, Jun 2, 2023 at 12:34 AM Neftin, Sasha <sasha.neftin@intel.com>
wrote:

> On 6/1/2023 21:53, prasad@arista.com wrote:
> > From: Prasad Koya <prasad@arista.com>
> >
> > if the physical media is twisted pair copper, set the TP bit in the
> > 'supported' field
> >
> > Signed-off-by: Prasad Koya <prasad@arista.com>
> > ---
> >   drivers/net/ethernet/intel/igc/igc_ethtool.c | 2 ++
> >   1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c
> b/drivers/net/ethernet/intel/igc/igc_ethtool.c
> > index 8cc077b712ad..7d197fa80d5d 100644
> > --- a/drivers/net/ethernet/intel/igc/igc_ethtool.c
> > +++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c
> > @@ -1707,6 +1707,8 @@ static int igc_ethtool_get_link_ksettings(struct
> net_device *netdev,
> >       /* twisted pair */
> >       cmd->base.port = PORT_TP;
> >       cmd->base.phy_address = hw->phy.addr;
> > +     if (hw->phy.media_type == igc_media_type_copper)
> Thank you Prasad. i225/6 parts supported only copper media type. We can
> drop the "if" condition.
> > +             ethtool_link_ksettings_add_link_mode(cmd, supported, TP);
> Do you want to see: "Supported ports: [ TP ]"? That's right.
> >
> >       /* advertising link modes */
> >       if (hw->phy.autoneg_advertised & ADVERTISE_10_HALF)
>
>

[-- Attachment #1.2: Type: text/html, Size: 2497 bytes --]

[-- Attachment #2: Type: text/plain, Size: 162 bytes --]

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

[Intel-wired-lan] [PATCH] intel-wired-lan: igc: set TP bit in ethtool_link_ksettings.supported field

[prasad]

From: Prasad Koya <prasad@arista.com>

set the TP bit in the 'supported' field since I226 only supports twisted pair

Signed-off-by: Prasad Koya <prasad@arista.com>
---
 drivers/net/ethernet/intel/igc/igc_ethtool.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c b/drivers/net/ethernet/intel/igc/igc_ethtool.c
index 8cc077b712ad..c796bd805092 100644
--- a/drivers/net/ethernet/intel/igc/igc_ethtool.c
+++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c
@@ -1707,6 +1707,7 @@ static int igc_ethtool_get_link_ksettings(struct net_device *netdev,
 	/* twisted pair */
 	cmd->base.port = PORT_TP;
 	cmd->base.phy_address = hw->phy.addr;
+	ethtool_link_ksettings_add_link_mode(cmd, supported, TP);
 
 	/* advertising link modes */
 	if (hw->phy.autoneg_advertised & ADVERTISE_10_HALF)
-- 
2.25.1

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Re: [Intel-wired-lan] [PATCH] intel-wired-lan: igc: set TP bit in ethtool_link_ksettings.supported field

[Prasad Koya]

[-- Attachment #1.1: Type: text/plain, Size: 2141 bytes --]

Hi Sasha

In our internal review, we found that igb and other vendors' drivers set
the TP (or other media) bit in the 'advertising' field as well. So I made
the and tested the change in 6.1.31 kernel. Will send that patch for
review.

Thank you.

On Fri, Jun 2, 2023 at 12:49 AM Prasad Koya <prasad@arista.com> wrote:

> Thanks for the quick review.
>
> Yes, we use ETHTOOL_GLINKSETTINGS ioctl to retrieve interface settings and
> expect to see one of TP or MII set in the 'supported' bitmask.
>
> I'll send out a new patch removing the if(). Would you accept the patch
> into your staging tree and later push it to the stable kernel branch? This
> is my first time sending to intel-wired-lan. Not sure how it works.
>
> Thank you.
>
> On Fri, Jun 2, 2023 at 12:34 AM Neftin, Sasha <sasha.neftin@intel.com>
> wrote:
>
>> On 6/1/2023 21:53, prasad@arista.com wrote:
>> > From: Prasad Koya <prasad@arista.com>
>> >
>> > if the physical media is twisted pair copper, set the TP bit in the
>> > 'supported' field
>> >
>> > Signed-off-by: Prasad Koya <prasad@arista.com>
>> > ---
>> >   drivers/net/ethernet/intel/igc/igc_ethtool.c | 2 ++
>> >   1 file changed, 2 insertions(+)
>> >
>> > diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c
>> b/drivers/net/ethernet/intel/igc/igc_ethtool.c
>> > index 8cc077b712ad..7d197fa80d5d 100644
>> > --- a/drivers/net/ethernet/intel/igc/igc_ethtool.c
>> > +++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c
>> > @@ -1707,6 +1707,8 @@ static int igc_ethtool_get_link_ksettings(struct
>> net_device *netdev,
>> >       /* twisted pair */
>> >       cmd->base.port = PORT_TP;
>> >       cmd->base.phy_address = hw->phy.addr;
>> > +     if (hw->phy.media_type == igc_media_type_copper)
>> Thank you Prasad. i225/6 parts supported only copper media type. We can
>> drop the "if" condition.
>> > +             ethtool_link_ksettings_add_link_mode(cmd, supported, TP);
>> Do you want to see: "Supported ports: [ TP ]"? That's right.
>> >
>> >       /* advertising link modes */
>> >       if (hw->phy.autoneg_advertised & ADVERTISE_10_HALF)
>>
>>

[-- Attachment #1.2: Type: text/html, Size: 3173 bytes --]

[-- Attachment #2: Type: text/plain, Size: 162 bytes --]

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Re: [Intel-wired-lan] [PATCH] intel-wired-lan: igc: set TP bit in ethtool_link_ksettings.supported field

[Neftin, Sasha]

On 6/3/2023 09:05, Prasad Koya wrote:
> Hi Sasha
> 
> In our internal review, we found that igb and other vendors' drivers set 
> the TP (or other media) bit in the 'advertising' field as well. So I 
> made the and tested the change in 6.1.31 kernel. Will send that patch 
> for review.
> 
> Thank you.
> 
> On Fri, Jun 2, 2023 at 12:49 AM Prasad Koya <prasad@arista.com 
> <mailto:prasad@arista.com>> wrote:
> 
>     Thanks for the quick review.
> 
>     Yes, we use ETHTOOL_GLINKSETTINGS ioctl to retrieve interface
>     settings and expect to see one of TP or MII set in the 'supported'
>     bitmask.
> 
>     I'll send out a new patch removing the if(). Would you accept the
>     patch into your staging tree and later push it to the stable kernel
>     branch? This is my first time sending to intel-wired-lan. Not sure
>     how it works.

1. yes, definitely we will accept.
2. please, use the [iwl-net] prefix, for example:
[iwl-net, v2] igc: set TP bit in ethtool_link_ksettings.supported field
3. you might add the fixes tag. Example:
Fixes: 8c5ad0dae93c9 ("igc: Add ethtool support")
Thanks,
Sasha

> 
>     Thank you.
> 
>     On Fri, Jun 2, 2023 at 12:34 AM Neftin, Sasha
>     <sasha.neftin@intel.com <mailto:sasha.neftin@intel.com>> wrote:
> 
>         On 6/1/2023 21:53, prasad@arista.com <mailto:prasad@arista.com>
>         wrote:
>          > From: Prasad Koya <prasad@arista.com <mailto:prasad@arista.com>>
>          >
>          > if the physical media is twisted pair copper, set the TP bit
>         in the
>          > 'supported' field
>          >
>          > Signed-off-by: Prasad Koya <prasad@arista.com
>         <mailto:prasad@arista.com>>
>          > ---
>          >   drivers/net/ethernet/intel/igc/igc_ethtool.c | 2 ++
>          >   1 file changed, 2 insertions(+)
>          >
>          > diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c
>         b/drivers/net/ethernet/intel/igc/igc_ethtool.c
>          > index 8cc077b712ad..7d197fa80d5d 100644
>          > --- a/drivers/net/ethernet/intel/igc/igc_ethtool.c
>          > +++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c
>          > @@ -1707,6 +1707,8 @@ static int
>         igc_ethtool_get_link_ksettings(struct net_device *netdev,
>          >       /* twisted pair */
>          >       cmd->base.port = PORT_TP;
>          >       cmd->base.phy_address = hw->phy.addr;
>          > +     if (hw->phy.media_type == igc_media_type_copper)
>         Thank you Prasad. i225/6 parts supported only copper media type.
>         We can
>         drop the "if" condition.
>          > +             ethtool_link_ksettings_add_link_mode(cmd,
>         supported, TP);
>         Do you want to see: "Supported ports: [ TP ]"? That's right.
>          >
>          >       /* advertising link modes */
>          >       if (hw->phy.autoneg_advertised & ADVERTISE_10_HALF)
> 

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

Re: [Intel-wired-lan] [PATCH] intel-wired-lan: igc: set TP bit in ethtool_link_ksettings.supported field

[Prasad Koya]

[-- Attachment #1.1: Type: text/plain, Size: 3474 bytes --]

>>>
2. please, use the [iwl-net] prefix, for example:
[iwl-net, v2] igc: set TP bit in ethtool_link_ksettings.supported field
3. you might add the fixes tag. Example:
Fixes: 8c5ad0dae93c9 ("igc: Add ethtool support")
<<<

Sent the current version after adding (2) and (3)

Thank you.

On Sun, Jun 4, 2023 at 12:20 AM Neftin, Sasha <sasha.neftin@intel.com>
wrote:

> On 6/3/2023 09:05, Prasad Koya wrote:
> > Hi Sasha
> >
> > In our internal review, we found that igb and other vendors' drivers set
> > the TP (or other media) bit in the 'advertising' field as well. So I
> > made the and tested the change in 6.1.31 kernel. Will send that patch
> > for review.
> >
> > Thank you.
> >
> > On Fri, Jun 2, 2023 at 12:49 AM Prasad Koya <prasad@arista.com
> > <mailto:prasad@arista.com>> wrote:
> >
> >     Thanks for the quick review.
> >
> >     Yes, we use ETHTOOL_GLINKSETTINGS ioctl to retrieve interface
> >     settings and expect to see one of TP or MII set in the 'supported'
> >     bitmask.
> >
> >     I'll send out a new patch removing the if(). Would you accept the
> >     patch into your staging tree and later push it to the stable kernel
> >     branch? This is my first time sending to intel-wired-lan. Not sure
> >     how it works.
>
> 1. yes, definitely we will accept.
> 2. please, use the [iwl-net] prefix, for example:
> [iwl-net, v2] igc: set TP bit in ethtool_link_ksettings.supported field
> 3. you might add the fixes tag. Example:
> Fixes: 8c5ad0dae93c9 ("igc: Add ethtool support")
> Thanks,
> Sasha
>
> >
> >     Thank you.
> >
> >     On Fri, Jun 2, 2023 at 12:34 AM Neftin, Sasha
> >     <sasha.neftin@intel.com <mailto:sasha.neftin@intel.com>> wrote:
> >
> >         On 6/1/2023 21:53, prasad@arista.com <mailto:prasad@arista.com>
> >         wrote:
> >          > From: Prasad Koya <prasad@arista.com <mailto:
> prasad@arista.com>>
> >          >
> >          > if the physical media is twisted pair copper, set the TP bit
> >         in the
> >          > 'supported' field
> >          >
> >          > Signed-off-by: Prasad Koya <prasad@arista.com
> >         <mailto:prasad@arista.com>>
> >          > ---
> >          >   drivers/net/ethernet/intel/igc/igc_ethtool.c | 2 ++
> >          >   1 file changed, 2 insertions(+)
> >          >
> >          > diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c
> >         b/drivers/net/ethernet/intel/igc/igc_ethtool.c
> >          > index 8cc077b712ad..7d197fa80d5d 100644
> >          > --- a/drivers/net/ethernet/intel/igc/igc_ethtool.c
> >          > +++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c
> >          > @@ -1707,6 +1707,8 @@ static int
> >         igc_ethtool_get_link_ksettings(struct net_device *netdev,
> >          >       /* twisted pair */
> >          >       cmd->base.port = PORT_TP;
> >          >       cmd->base.phy_address = hw->phy.addr;
> >          > +     if (hw->phy.media_type == igc_media_type_copper)
> >         Thank you Prasad. i225/6 parts supported only copper media type.
> >         We can
> >         drop the "if" condition.
> >          > +             ethtool_link_ksettings_add_link_mode(cmd,
> >         supported, TP);
> >         Do you want to see: "Supported ports: [ TP ]"? That's right.
> >          >
> >          >       /* advertising link modes */
> >          >       if (hw->phy.autoneg_advertised & ADVERTISE_10_HALF)
> >
>
>

[-- Attachment #1.2: Type: text/html, Size: 5206 bytes --]

[-- Attachment #2: Type: text/plain, Size: 162 bytes --]

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan