* [PATCH v2] net: ibm: emac: support RGMII-[RX|TX]ID phymode
@ 2017-12-20 16:02 Christian Lamparter
2017-12-20 20:10 ` David Miller
0 siblings, 1 reply; 5+ messages in thread
From: Christian Lamparter @ 2017-12-20 16:02 UTC (permalink / raw)
To: netdev; +Cc: David S . Miller, Andrew Lunn, Christophe Jaillet
The RGMII spec allows compliance for devices that implement an internal
delay on TXC and/or RXC inside the transmitter. This patch adds the
necessary RGMII_[RX|TX]ID mode code to handle such PHYs with the
emac driver.
Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
---
v2: - utilize phy_interface_mode_is_rgmii()
---
drivers/net/ethernet/ibm/emac/core.c | 4 ++--
drivers/net/ethernet/ibm/emac/emac.h | 3 +++
drivers/net/ethernet/ibm/emac/rgmii.c | 10 ++++++++--
3 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/ibm/emac/core.c b/drivers/net/ethernet/ibm/emac/core.c
index 7feff2450ed6..043e72e28bba 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -199,8 +199,8 @@ static void __emac_set_multicast_list(struct emac_instance *dev);
static inline int emac_phy_supports_gige(int phy_mode)
{
- return phy_mode == PHY_MODE_GMII ||
- phy_mode == PHY_MODE_RGMII ||
+ return phy_interface_mode_is_rgmii(phy_mode) ||
+ phy_mode == PHY_MODE_GMII ||
phy_mode == PHY_MODE_SGMII ||
phy_mode == PHY_MODE_TBI ||
phy_mode == PHY_MODE_RTBI;
diff --git a/drivers/net/ethernet/ibm/emac/emac.h b/drivers/net/ethernet/ibm/emac/emac.h
index 5afcc27ceebb..8c6d2af7281b 100644
--- a/drivers/net/ethernet/ibm/emac/emac.h
+++ b/drivers/net/ethernet/ibm/emac/emac.h
@@ -112,6 +112,9 @@ struct emac_regs {
#define PHY_MODE_RMII PHY_INTERFACE_MODE_RMII
#define PHY_MODE_SMII PHY_INTERFACE_MODE_SMII
#define PHY_MODE_RGMII PHY_INTERFACE_MODE_RGMII
+#define PHY_MODE_RGMII_ID PHY_INTERFACE_MODE_RGMII_ID
+#define PHY_MODE_RGMII_RXID PHY_INTERFACE_MODE_RGMII_RXID
+#define PHY_MODE_RGMII_TXID PHY_INTERFACE_MODE_RGMII_TXID
#define PHY_MODE_TBI PHY_INTERFACE_MODE_TBI
#define PHY_MODE_GMII PHY_INTERFACE_MODE_GMII
#define PHY_MODE_RTBI PHY_INTERFACE_MODE_RTBI
diff --git a/drivers/net/ethernet/ibm/emac/rgmii.c b/drivers/net/ethernet/ibm/emac/rgmii.c
index c4a1ac38bba8..124b0473d2b7 100644
--- a/drivers/net/ethernet/ibm/emac/rgmii.c
+++ b/drivers/net/ethernet/ibm/emac/rgmii.c
@@ -52,9 +52,9 @@
/* RGMII bridge supports only GMII/TBI and RGMII/RTBI PHYs */
static inline int rgmii_valid_mode(int phy_mode)
{
- return phy_mode == PHY_MODE_GMII ||
+ return phy_interface_mode_is_rgmii(phy_mode) ||
+ phy_mode == PHY_MODE_GMII ||
phy_mode == PHY_MODE_MII ||
- phy_mode == PHY_MODE_RGMII ||
phy_mode == PHY_MODE_TBI ||
phy_mode == PHY_MODE_RTBI;
}
@@ -63,6 +63,9 @@ static inline const char *rgmii_mode_name(int mode)
{
switch (mode) {
case PHY_MODE_RGMII:
+ case PHY_MODE_RGMII_ID:
+ case PHY_MODE_RGMII_RXID:
+ case PHY_MODE_RGMII_TXID:
return "RGMII";
case PHY_MODE_TBI:
return "TBI";
@@ -81,6 +84,9 @@ static inline u32 rgmii_mode_mask(int mode, int input)
{
switch (mode) {
case PHY_MODE_RGMII:
+ case PHY_MODE_RGMII_ID:
+ case PHY_MODE_RGMII_RXID:
+ case PHY_MODE_RGMII_TXID:
return RGMII_FER_RGMII(input);
case PHY_MODE_TBI:
return RGMII_FER_TBI(input);
--
2.15.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v2] net: ibm: emac: support RGMII-[RX|TX]ID phymode
2017-12-20 16:02 [PATCH v2] net: ibm: emac: support RGMII-[RX|TX]ID phymode Christian Lamparter
@ 2017-12-20 20:10 ` David Miller
2017-12-20 21:07 ` Christian Lamparter
0 siblings, 1 reply; 5+ messages in thread
From: David Miller @ 2017-12-20 20:10 UTC (permalink / raw)
To: chunkeey; +Cc: netdev, andrew, christophe.jaillet
From: Christian Lamparter <chunkeey@gmail.com>
Date: Wed, 20 Dec 2017 17:02:01 +0100
> diff --git a/drivers/net/ethernet/ibm/emac/emac.h b/drivers/net/ethernet/ibm/emac/emac.h
> index 5afcc27ceebb..8c6d2af7281b 100644
> --- a/drivers/net/ethernet/ibm/emac/emac.h
> +++ b/drivers/net/ethernet/ibm/emac/emac.h
> @@ -112,6 +112,9 @@ struct emac_regs {
> #define PHY_MODE_RMII PHY_INTERFACE_MODE_RMII
> #define PHY_MODE_SMII PHY_INTERFACE_MODE_SMII
> #define PHY_MODE_RGMII PHY_INTERFACE_MODE_RGMII
> +#define PHY_MODE_RGMII_ID PHY_INTERFACE_MODE_RGMII_ID
> +#define PHY_MODE_RGMII_RXID PHY_INTERFACE_MODE_RGMII_RXID
> +#define PHY_MODE_RGMII_TXID PHY_INTERFACE_MODE_RGMII_TXID
> #define PHY_MODE_TBI PHY_INTERFACE_MODE_TBI
> #define PHY_MODE_GMII PHY_INTERFACE_MODE_GMII
> #define PHY_MODE_RTBI PHY_INTERFACE_MODE_RTBI
Why does this driver do all of this CPP macro aliasing?
It's really cruddy because anyone who now reads this code:
> static inline int rgmii_valid_mode(int phy_mode)
> {
> - return phy_mode == PHY_MODE_GMII ||
> + return phy_interface_mode_is_rgmii(phy_mode) ||
> + phy_mode == PHY_MODE_GMII ||
> phy_mode == PHY_MODE_MII ||
> - phy_mode == PHY_MODE_RGMII ||
> phy_mode == PHY_MODE_TBI ||
> phy_mode == PHY_MODE_RTBI;
> }
will read this and say "Oh the function tests against these weird
PHY_MODE_* aliases, but the helper function phy_interface_mode_is_rgmii()
tests against PHY_INTERFACE_MODE_*, what is going on?"
I hate to do this to you, but please get rid of these confusing and
completely unnecessary PHY_MODE_* CPP aliases first, and just use the
proper PHY_INTERFACE_MODE_* values consistently.
Thank you.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] net: ibm: emac: support RGMII-[RX|TX]ID phymode
2017-12-20 20:10 ` David Miller
@ 2017-12-20 21:07 ` Christian Lamparter
2017-12-20 21:20 ` David Miller
2017-12-20 22:04 ` Benjamin Herrenschmidt
0 siblings, 2 replies; 5+ messages in thread
From: Christian Lamparter @ 2017-12-20 21:07 UTC (permalink / raw)
To: David Miller; +Cc: netdev, andrew, christophe.jaillet, Benjamin Herrenschmidt
On Wednesday, December 20, 2017 3:10:46 PM CET David Miller wrote:
> From: Christian Lamparter <chunkeey@gmail.com>
> Date: Wed, 20 Dec 2017 17:02:01 +0100
>
> > diff --git a/drivers/net/ethernet/ibm/emac/emac.h b/drivers/net/ethernet/ibm/emac/emac.h
> > index 5afcc27ceebb..8c6d2af7281b 100644
> > --- a/drivers/net/ethernet/ibm/emac/emac.h
> > +++ b/drivers/net/ethernet/ibm/emac/emac.h
> > @@ -112,6 +112,9 @@ struct emac_regs {
> > #define PHY_MODE_RMII PHY_INTERFACE_MODE_RMII
> > #define PHY_MODE_SMII PHY_INTERFACE_MODE_SMII
> > #define PHY_MODE_RGMII PHY_INTERFACE_MODE_RGMII
> > +#define PHY_MODE_RGMII_ID PHY_INTERFACE_MODE_RGMII_ID
> > +#define PHY_MODE_RGMII_RXID PHY_INTERFACE_MODE_RGMII_RXID
> > +#define PHY_MODE_RGMII_TXID PHY_INTERFACE_MODE_RGMII_TXID
> > #define PHY_MODE_TBI PHY_INTERFACE_MODE_TBI
> > #define PHY_MODE_GMII PHY_INTERFACE_MODE_GMII
> > #define PHY_MODE_RTBI PHY_INTERFACE_MODE_RTBI
>
> Why does this driver do all of this CPP macro aliasing?
Ah, well the emac driver is almost as old as dirt ;).
I added Benjamin Herrenschmidt since he seems to be the
original author. maybe he can provide some valuable insights
in this archaeological dig.
I don't know when the ibm_emac driver was added, but it does
predate the shared PHY_INTERFACE_MODE_ macros by a few years.
For example 2.6.11 had the driver and the defines.:
<http://elixir.free-electrons.com/linux/v2.6.11/source/drivers/net/ibm_emac/ibm_emac_phy.h#L41>
But there's no PHY_INTERFACE_MODE_* yet.
Fast forward to 2011:
The patch <https://patchwork.kernel.org/patch/945642/> wedded the
PHY_MODE_* macros to the PHY_INTERFACE_MODE_ enums. But this was
not a complete convertion.
So, as far as I can tell, these definitions have been there since
the beginning.
>
> It's really cruddy because anyone who now reads this code:
>
> > static inline int rgmii_valid_mode(int phy_mode)
> > {
> > - return phy_mode == PHY_MODE_GMII ||
> > + return phy_interface_mode_is_rgmii(phy_mode) ||
> > + phy_mode == PHY_MODE_GMII ||
> > phy_mode == PHY_MODE_MII ||
> > - phy_mode == PHY_MODE_RGMII ||
> > phy_mode == PHY_MODE_TBI ||
> > phy_mode == PHY_MODE_RTBI;
> > }
>
> will read this and say "Oh the function tests against these weird
> PHY_MODE_* aliases, but the helper function phy_interface_mode_is_rgmii()
> tests against PHY_INTERFACE_MODE_*, what is going on?"
>
> I hate to do this to you, but please get rid of these confusing and
> completely unnecessary PHY_MODE_* CPP aliases first, and just use the
> proper PHY_INTERFACE_MODE_* values consistently.
Yeah, I can do that. no problem.
Question is, should I also replace the rgmii_mode_name() with phy_modes() too?
The only user of rgmii_mode_name() is this notice printk in rgmii_attach():
<http://elixir.free-electrons.com/linux/latest/source/drivers/net/ethernet/ibm/emac/rgmii.c#L117>
Regards,
Christian
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] net: ibm: emac: support RGMII-[RX|TX]ID phymode
2017-12-20 21:07 ` Christian Lamparter
@ 2017-12-20 21:20 ` David Miller
2017-12-20 22:04 ` Benjamin Herrenschmidt
1 sibling, 0 replies; 5+ messages in thread
From: David Miller @ 2017-12-20 21:20 UTC (permalink / raw)
To: chunkeey; +Cc: netdev, andrew, christophe.jaillet, benh
From: Christian Lamparter <chunkeey@gmail.com>
Date: Wed, 20 Dec 2017 22:07:43 +0100
> Yeah, I can do that. no problem.
>
> Question is, should I also replace the rgmii_mode_name() with phy_modes() too?
>
> The only user of rgmii_mode_name() is this notice printk in rgmii_attach():
> <http://elixir.free-electrons.com/linux/latest/source/drivers/net/ethernet/ibm/emac/rgmii.c#L117>
I would recommend you use phy_modes() instead of that custom rgmii_mode_name() thing,
yes.
Thanks for asking.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] net: ibm: emac: support RGMII-[RX|TX]ID phymode
2017-12-20 21:07 ` Christian Lamparter
2017-12-20 21:20 ` David Miller
@ 2017-12-20 22:04 ` Benjamin Herrenschmidt
1 sibling, 0 replies; 5+ messages in thread
From: Benjamin Herrenschmidt @ 2017-12-20 22:04 UTC (permalink / raw)
To: Christian Lamparter, David Miller; +Cc: netdev, andrew, christophe.jaillet
On Wed, 2017-12-20 at 22:07 +0100, Christian Lamparter wrote:
>
> > will read this and say "Oh the function tests against these weird
> > PHY_MODE_* aliases, but the helper function phy_interface_mode_is_rgmii()
> > tests against PHY_INTERFACE_MODE_*, what is going on?"
> >
> > I hate to do this to you, but please get rid of these confusing and
> > completely unnecessary PHY_MODE_* CPP aliases first, and just use the
> > proper PHY_INTERFACE_MODE_* values consistently.
>
> Yeah, I can do that. no problem.
>
> Question is, should I also replace the rgmii_mode_name() with phy_modes() too?
>
> The only user of rgmii_mode_name() is this notice printk in rgmii_attach():
> <http://elixir.free-electrons.com/linux/latest/source/drivers/net/ethernet/ibm/emac/rgmii.c#L117>
Yup, this is all pre-hisorical gunk, feel free to replace it.
Cheers,
Ben.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-12-20 23:08 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-20 16:02 [PATCH v2] net: ibm: emac: support RGMII-[RX|TX]ID phymode Christian Lamparter
2017-12-20 20:10 ` David Miller
2017-12-20 21:07 ` Christian Lamparter
2017-12-20 21:20 ` David Miller
2017-12-20 22:04 ` Benjamin Herrenschmidt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).