diff mbox series

[net-next,v14,10/13] net: dsa: microchip: lan937x: add phylink_get_caps support

Message ID 20220630102041.25555-11-arun.ramadoss@microchip.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: dsa: microchip: DSA Driver support for LAN937x | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 21 of 21 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 31 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Arun Ramadoss June 30, 2022, 10:20 a.m. UTC
The internal phy of the LAN937x are capable of 100Mbps speed. And the
xMII port of switch is capable of 10/100/1000Mbps.

Signed-off-by: Arun Ramadoss <arun.ramadoss@microchip.com>
---
 drivers/net/dsa/microchip/ksz_common.c   |  1 +
 drivers/net/dsa/microchip/lan937x.h      |  2 ++
 drivers/net/dsa/microchip/lan937x_main.c | 12 ++++++++++++
 3 files changed, 15 insertions(+)

Comments

Russell King (Oracle) June 30, 2022, 11:36 a.m. UTC | #1
On Thu, Jun 30, 2022 at 03:50:38PM +0530, Arun Ramadoss wrote:
> The internal phy of the LAN937x are capable of 100Mbps speed. And the

Good English grammar suggests never to start a sentence with "And".

> xMII port of switch is capable of 10/100/1000Mbps.

... and supports flow control?

> +void lan937x_phylink_get_caps(struct ksz_device *dev, int port,
> +			      struct phylink_config *config)
> +{
> +	config->mac_capabilities = MAC_100FD;
> +
> +	if (dev->info->supports_rgmii[port]) {
> +		/* MII/RMII/RGMII ports */
> +		config->mac_capabilities |= MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
> +					    MAC_100HD | MAC_10 | MAC_1000FD;

And SGMII too? (Which seems to be a given because from your list in the
series cover message, SGMII ports also support RGMII).

Thanks.
Arun Ramadoss July 1, 2022, 5:51 a.m. UTC | #2
Hi Russell,
Thanks for the review comment. 

On Thu, 2022-06-30 at 12:36 +0100, Russell King (Oracle) wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> On Thu, Jun 30, 2022 at 03:50:38PM +0530, Arun Ramadoss wrote:
> > The internal phy of the LAN937x are capable of 100Mbps speed. And
> > the
> 
> Good English grammar suggests never to start a sentence with "And".

Ok. I will update the description.

> 
> > xMII port of switch is capable of 10/100/1000Mbps.
> 
> ... and supports flow control?
> 
> > +void lan937x_phylink_get_caps(struct ksz_device *dev, int port,
> > +                           struct phylink_config *config)
> > +{
> > +     config->mac_capabilities = MAC_100FD;
> > +
> > +     if (dev->info->supports_rgmii[port]) {
> > +             /* MII/RMII/RGMII ports */
> > +             config->mac_capabilities |= MAC_ASYM_PAUSE |
> > MAC_SYM_PAUSE |
> > +                                         MAC_100HD | MAC_10 |
> > MAC_1000FD;
> 
> And SGMII too? (Which seems to be a given because from your list in
> the
> series cover message, SGMII ports also support RGMII).

No, SGMII port does not support the RGMII. I have mentioned in the
cover message that LAN9373 has 2 RGMII and 1 SGMII port. No other part
number has SGMII port.

> 
> Thanks.
> 
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Russell King (Oracle) July 1, 2022, 8:07 a.m. UTC | #3
On Fri, Jul 01, 2022 at 05:51:33AM +0000, Arun.Ramadoss@microchip.com wrote:
> > > +void lan937x_phylink_get_caps(struct ksz_device *dev, int port,
> > > +                           struct phylink_config *config)
> > > +{
> > > +     config->mac_capabilities = MAC_100FD;
> > > +
> > > +     if (dev->info->supports_rgmii[port]) {
> > > +             /* MII/RMII/RGMII ports */
> > > +             config->mac_capabilities |= MAC_ASYM_PAUSE |
> > > MAC_SYM_PAUSE |
> > > +                                         MAC_100HD | MAC_10 |
> > > MAC_1000FD;
> > 
> > And SGMII too? (Which seems to be a given because from your list in
> > the
> > series cover message, SGMII ports also support RGMII).
> 
> No, SGMII port does not support the RGMII. I have mentioned in the
> cover message that LAN9373 has 2 RGMII and 1 SGMII port. No other part
> number has SGMII port.

So when using SGMII, there's no way for pause frames to be supported?
It seems a bit weird that the pause frame capability is dependent on
the interface type, as pause frames are just the same as normal
ethernet frames, except they're ggenerally enerated and/or interpreted
by the MAC.
Arun Ramadoss July 1, 2022, 8:37 a.m. UTC | #4
On Fri, 2022-07-01 at 09:07 +0100, Russell King (Oracle) wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> On Fri, Jul 01, 2022 at 05:51:33AM +0000, Arun.Ramadoss@microchip.com
>  wrote:
> > > > +void lan937x_phylink_get_caps(struct ksz_device *dev, int
> > > > port,
> > > > +                           struct phylink_config *config)
> > > > +{
> > > > +     config->mac_capabilities = MAC_100FD;
> > > > +
> > > > +     if (dev->info->supports_rgmii[port]) {
> > > > +             /* MII/RMII/RGMII ports */
> > > > +             config->mac_capabilities |= MAC_ASYM_PAUSE |
> > > > MAC_SYM_PAUSE |
> > > > +                                         MAC_100HD | MAC_10 |
> > > > MAC_1000FD;
> > > 
> > > And SGMII too? (Which seems to be a given because from your list
> > > in
> > > the
> > > series cover message, SGMII ports also support RGMII).
> > 
> > No, SGMII port does not support the RGMII. I have mentioned in the
> > cover message that LAN9373 has 2 RGMII and 1 SGMII port. No other
> > part
> > number has SGMII port.
> 
> So when using SGMII, there's no way for pause frames to be supported?
> It seems a bit weird that the pause frame capability is dependent on
> the interface type, as pause frames are just the same as normal
> ethernet frames, except they're ggenerally enerated and/or
> interpreted
> by the MAC.

As per the comment, I infer that you want to include the capability of
SGMII port as well. As of now, we implemented the dsa for RMII/RGMII.
Yet to implement and test the SGMII port. Since we have not tested the
port, we haven't included it. As per datasheet, SGMII port supports the
flow control.

> 
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
diff mbox series

Patch

diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index fb0de48a3f5e..ca7ca327285d 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -220,6 +220,7 @@  static const struct ksz_dev_ops lan937x_dev_ops = {
 	.vlan_del = ksz9477_port_vlan_del,
 	.mirror_add = ksz9477_port_mirror_add,
 	.mirror_del = ksz9477_port_mirror_del,
+	.get_caps = lan937x_phylink_get_caps,
 	.fdb_dump = ksz9477_fdb_dump,
 	.fdb_add = ksz9477_fdb_add,
 	.fdb_del = ksz9477_fdb_del,
diff --git a/drivers/net/dsa/microchip/lan937x.h b/drivers/net/dsa/microchip/lan937x.h
index 50563874600d..d4207e97a130 100644
--- a/drivers/net/dsa/microchip/lan937x.h
+++ b/drivers/net/dsa/microchip/lan937x.h
@@ -15,4 +15,6 @@  void lan937x_switch_exit(struct ksz_device *dev);
 void lan937x_r_phy(struct ksz_device *dev, u16 addr, u16 reg, u16 *data);
 void lan937x_w_phy(struct ksz_device *dev, u16 addr, u16 reg, u16 val);
 int lan937x_change_mtu(struct ksz_device *dev, int port, int new_mtu);
+void lan937x_phylink_get_caps(struct ksz_device *dev, int port,
+			      struct phylink_config *config);
 #endif
diff --git a/drivers/net/dsa/microchip/lan937x_main.c b/drivers/net/dsa/microchip/lan937x_main.c
index 5917cc11ba59..8cb46caf5340 100644
--- a/drivers/net/dsa/microchip/lan937x_main.c
+++ b/drivers/net/dsa/microchip/lan937x_main.c
@@ -312,6 +312,18 @@  int lan937x_change_mtu(struct ksz_device *dev, int port, int new_mtu)
 	return 0;
 }
 
+void lan937x_phylink_get_caps(struct ksz_device *dev, int port,
+			      struct phylink_config *config)
+{
+	config->mac_capabilities = MAC_100FD;
+
+	if (dev->info->supports_rgmii[port]) {
+		/* MII/RMII/RGMII ports */
+		config->mac_capabilities |= MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
+					    MAC_100HD | MAC_10 | MAC_1000FD;
+	}
+}
+
 int lan937x_setup(struct dsa_switch *ds)
 {
 	struct ksz_device *dev = ds->priv;