diff mbox series

[v12,2/4] phy: Add ethernet serdes configuration option

Message ID 20210107091924.1569575-3-steen.hegelund@microchip.com (mailing list archive)
State Not Applicable
Headers show
Series Adding the Sparx5 Serdes driver | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Guessed tree name to be net-next
netdev/subject_prefix success Link
netdev/cc_maintainers success CCed 2 of 2 maintainers
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 194 this patch: 194
netdev/kdoc success Errors and warnings before: 1 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch warning WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
netdev/build_allmodconfig_warn success Errors and warnings before: 194 this patch: 194
netdev/header_inline success Link
netdev/stable success Stable not CCed

Commit Message

Steen Hegelund Jan. 7, 2021, 9:19 a.m. UTC
Provide a new ethernet phy configuration structure, that
allow PHYs used for ethernet to be configured with
speed, media type and clock information.

Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
Signed-off-by: Steen Hegelund <steen.hegelund@microchip.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
 include/linux/phy/phy-ethernet-serdes.h | 30 +++++++++++++++++++++++++
 include/linux/phy/phy.h                 |  4 ++++
 2 files changed, 34 insertions(+)
 create mode 100644 include/linux/phy/phy-ethernet-serdes.h

Comments

Alexandre Belloni Jan. 14, 2021, 11:02 a.m. UTC | #1
On 07/01/2021 10:19:22+0100, Steen Hegelund wrote:
> Provide a new ethernet phy configuration structure, that
> allow PHYs used for ethernet to be configured with
> speed, media type and clock information.
> 
> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
> Signed-off-by: Steen Hegelund <steen.hegelund@microchip.com>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

> ---
>  include/linux/phy/phy-ethernet-serdes.h | 30 +++++++++++++++++++++++++
>  include/linux/phy/phy.h                 |  4 ++++
>  2 files changed, 34 insertions(+)
>  create mode 100644 include/linux/phy/phy-ethernet-serdes.h
> 
> diff --git a/include/linux/phy/phy-ethernet-serdes.h b/include/linux/phy/phy-ethernet-serdes.h
> new file mode 100644
> index 000000000000..d2462fadf179
> --- /dev/null
> +++ b/include/linux/phy/phy-ethernet-serdes.h
> @@ -0,0 +1,30 @@
> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
> +/*
> + * Microchip Sparx5 Ethernet SerDes driver
> + *
> + * Copyright (c) 2020 Microschip Inc
> + */
> +#ifndef __PHY_ETHERNET_SERDES_H_
> +#define __PHY_ETHERNET_SERDES_H_
> +
> +#include <linux/types.h>
> +
> +enum ethernet_media_type {
> +	ETH_MEDIA_DEFAULT,
> +	ETH_MEDIA_SR,
> +	ETH_MEDIA_DAC,
> +};
> +
> +/**
> + * struct phy_configure_opts_eth_serdes - Ethernet SerDes This structure is used
> + * to represent the configuration state of a Ethernet Serdes PHY.
> + * @speed: Speed of the serdes interface in Mbps
> + * @media_type: Specifies which media the serdes will be using
> + */
> +struct phy_configure_opts_eth_serdes {
> +	u32                        speed;
> +	enum ethernet_media_type   media_type;
> +};
> +
> +#endif
> +
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> index e435bdb0bab3..78ecb375cede 100644
> --- a/include/linux/phy/phy.h
> +++ b/include/linux/phy/phy.h
> @@ -18,6 +18,7 @@
>  
>  #include <linux/phy/phy-dp.h>
>  #include <linux/phy/phy-mipi-dphy.h>
> +#include <linux/phy/phy-ethernet-serdes.h>
>  
>  struct phy;
>  
> @@ -49,11 +50,14 @@ enum phy_mode {
>   *
>   * @mipi_dphy:	Configuration set applicable for phys supporting
>   *		the MIPI_DPHY phy mode.
> + * @eth_serdes: Configuration set applicable for phys supporting
> + *		the ethernet serdes.
>   * @dp:		Configuration set applicable for phys supporting
>   *		the DisplayPort protocol.
>   */
>  union phy_configure_opts {
>  	struct phy_configure_opts_mipi_dphy	mipi_dphy;
> +	struct phy_configure_opts_eth_serdes	eth_serdes;
>  	struct phy_configure_opts_dp		dp;
>  };
>  
> -- 
> 2.29.2
>
Kishon Vijay Abraham I Jan. 15, 2021, 8:44 a.m. UTC | #2
Hi,

On 07/01/21 2:49 pm, Steen Hegelund wrote:
> Provide a new ethernet phy configuration structure, that
> allow PHYs used for ethernet to be configured with
> speed, media type and clock information.
> 
> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
> Signed-off-by: Steen Hegelund <steen.hegelund@microchip.com>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  include/linux/phy/phy-ethernet-serdes.h | 30 +++++++++++++++++++++++++
>  include/linux/phy/phy.h                 |  4 ++++
>  2 files changed, 34 insertions(+)
>  create mode 100644 include/linux/phy/phy-ethernet-serdes.h
> 
> diff --git a/include/linux/phy/phy-ethernet-serdes.h b/include/linux/phy/phy-ethernet-serdes.h
> new file mode 100644
> index 000000000000..d2462fadf179
> --- /dev/null
> +++ b/include/linux/phy/phy-ethernet-serdes.h
> @@ -0,0 +1,30 @@
> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
> +/*
> + * Microchip Sparx5 Ethernet SerDes driver
> + *
> + * Copyright (c) 2020 Microschip Inc
> + */
> +#ifndef __PHY_ETHERNET_SERDES_H_
> +#define __PHY_ETHERNET_SERDES_H_
> +
> +#include <linux/types.h>
> +
> +enum ethernet_media_type {
> +	ETH_MEDIA_DEFAULT,
> +	ETH_MEDIA_SR,
> +	ETH_MEDIA_DAC,
> +};
> +
> +/**
> + * struct phy_configure_opts_eth_serdes - Ethernet SerDes This structure is used
> + * to represent the configuration state of a Ethernet Serdes PHY.
> + * @speed: Speed of the serdes interface in Mbps
> + * @media_type: Specifies which media the serdes will be using
> + */
> +struct phy_configure_opts_eth_serdes {
> +	u32                        speed;
> +	enum ethernet_media_type   media_type;
> +};

Is media type going to be determined dynamically by the Ethernet
controller. If it's not determined dynamically, it shouldn't be in PHY
ops but rather as a DT parameter.

phy_configure_opts is mostly used with things like DP where the
controller probes the configurations supported by SERDES using the
configure and validate ops. I don't think for Ethernet it is required.

Thanks
Kishon

> +
> +#endif
> +
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> index e435bdb0bab3..78ecb375cede 100644
> --- a/include/linux/phy/phy.h
> +++ b/include/linux/phy/phy.h
> @@ -18,6 +18,7 @@
>  
>  #include <linux/phy/phy-dp.h>
>  #include <linux/phy/phy-mipi-dphy.h>
> +#include <linux/phy/phy-ethernet-serdes.h>
>  
>  struct phy;
>  
> @@ -49,11 +50,14 @@ enum phy_mode {
>   *
>   * @mipi_dphy:	Configuration set applicable for phys supporting
>   *		the MIPI_DPHY phy mode.
> + * @eth_serdes: Configuration set applicable for phys supporting
> + *		the ethernet serdes.
>   * @dp:		Configuration set applicable for phys supporting
>   *		the DisplayPort protocol.
>   */
>  union phy_configure_opts {
>  	struct phy_configure_opts_mipi_dphy	mipi_dphy;
> +	struct phy_configure_opts_eth_serdes	eth_serdes;
>  	struct phy_configure_opts_dp		dp;
>  };
>  
>
Steen Hegelund Jan. 15, 2021, 9:08 a.m. UTC | #3
Hi Kishon,


On Fri, 2021-01-15 at 14:14 +0530, Kishon Vijay Abraham I wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> Hi,
> 
> On 07/01/21 2:49 pm, Steen Hegelund wrote:
> > Provide a new ethernet phy configuration structure, that
> > allow PHYs used for ethernet to be configured with
> > speed, media type and clock information.
> > 
> > Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
> > Signed-off-by: Steen Hegelund <steen.hegelund@microchip.com>
> > Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> > ---
> >  include/linux/phy/phy-ethernet-serdes.h | 30
> > +++++++++++++++++++++++++
> >  include/linux/phy/phy.h                 |  4 ++++
> >  2 files changed, 34 insertions(+)
> >  create mode 100644 include/linux/phy/phy-ethernet-serdes.h
> > 
> > diff --git a/include/linux/phy/phy-ethernet-serdes.h
> > b/include/linux/phy/phy-ethernet-serdes.h
> > new file mode 100644
> > index 000000000000..d2462fadf179
> > --- /dev/null
> > +++ b/include/linux/phy/phy-ethernet-serdes.h
> > @@ -0,0 +1,30 @@
> > +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
> > +/*
> > + * Microchip Sparx5 Ethernet SerDes driver
> > + *
> > + * Copyright (c) 2020 Microschip Inc
> > + */
> > +#ifndef __PHY_ETHERNET_SERDES_H_
> > +#define __PHY_ETHERNET_SERDES_H_
> > +
> > +#include <linux/types.h>
> > +
> > +enum ethernet_media_type {
> > +     ETH_MEDIA_DEFAULT,
> > +     ETH_MEDIA_SR,
> > +     ETH_MEDIA_DAC,
> > +};
> > +
> > +/**
> > + * struct phy_configure_opts_eth_serdes - Ethernet SerDes This
> > structure is used
> > + * to represent the configuration state of a Ethernet Serdes PHY.
> > + * @speed: Speed of the serdes interface in Mbps
> > + * @media_type: Specifies which media the serdes will be using
> > + */
> > +struct phy_configure_opts_eth_serdes {
> > +     u32                        speed;
> > +     enum ethernet_media_type   media_type;
> > +};
> 
> Is media type going to be determined dynamically by the Ethernet
> controller. If it's not determined dynamically, it shouldn't be in
> PHY
> ops but rather as a DT parameter.

Yes the media type is dynamic, as it will be determined by the feedback
from the attached SFP or DAC attached, which can be changed at any
time, so it is not static in a way that allows it to be part of the DT.

> 
> phy_configure_opts is mostly used with things like DP where the
> controller probes the configurations supported by SERDES using the
> configure and validate ops. I don't think for Ethernet it is
> required.

From what you explained I think the situation is very similar with the
Ethernet SerDes in that the actual media (and speed) is not known in
advance, but will be obtained "out-of-band" by the client and may
change at any point in time when the user changes the physical setup
(e.g cables).

> 
> Thanks
> Kishon
> 
> > +
> > +#endif
> > +
> > diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> > index e435bdb0bab3..78ecb375cede 100644
> > --- a/include/linux/phy/phy.h
> > +++ b/include/linux/phy/phy.h
> > @@ -18,6 +18,7 @@
> > 
> >  #include <linux/phy/phy-dp.h>
> >  #include <linux/phy/phy-mipi-dphy.h>
> > +#include <linux/phy/phy-ethernet-serdes.h>
> > 
> >  struct phy;
> > 
> > @@ -49,11 +50,14 @@ enum phy_mode {
> >   *
> >   * @mipi_dphy:       Configuration set applicable for phys
> > supporting
> >   *           the MIPI_DPHY phy mode.
> > + * @eth_serdes: Configuration set applicable for phys supporting
> > + *           the ethernet serdes.
> >   * @dp:              Configuration set applicable for phys
> > supporting
> >   *           the DisplayPort protocol.
> >   */
> >  union phy_configure_opts {
> >       struct phy_configure_opts_mipi_dphy     mipi_dphy;
> > +     struct phy_configure_opts_eth_serdes    eth_serdes;
> >       struct phy_configure_opts_dp            dp;
> >  };
> > 
> > 

BR
Steen
Kishon Vijay Abraham I Jan. 15, 2021, 3:52 p.m. UTC | #4
Hi,

On 07/01/21 2:49 pm, Steen Hegelund wrote:
> Provide a new ethernet phy configuration structure, that
> allow PHYs used for ethernet to be configured with
> speed, media type and clock information.
> 
> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
> Signed-off-by: Steen Hegelund <steen.hegelund@microchip.com>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> ---
>  include/linux/phy/phy-ethernet-serdes.h | 30 +++++++++++++++++++++++++
>  include/linux/phy/phy.h                 |  4 ++++
>  2 files changed, 34 insertions(+)
>  create mode 100644 include/linux/phy/phy-ethernet-serdes.h
> 
> diff --git a/include/linux/phy/phy-ethernet-serdes.h b/include/linux/phy/phy-ethernet-serdes.h
> new file mode 100644
> index 000000000000..d2462fadf179
> --- /dev/null
> +++ b/include/linux/phy/phy-ethernet-serdes.h
> @@ -0,0 +1,30 @@
> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
> +/*
> + * Microchip Sparx5 Ethernet SerDes driver
> + *
> + * Copyright (c) 2020 Microschip Inc
> + */
> +#ifndef __PHY_ETHERNET_SERDES_H_
> +#define __PHY_ETHERNET_SERDES_H_
> +
> +#include <linux/types.h>
> +
> +enum ethernet_media_type {
> +	ETH_MEDIA_DEFAULT,
> +	ETH_MEDIA_SR,
> +	ETH_MEDIA_DAC,
> +};

I'm not familiar with Ethernet. Are these generic media types? what does
SR or DAC refer to? Are there other media types? What is the out-of-band
mechanism by which the controller gets the media type? Why was this not
required for other existing Ethernet SERDES? Are you aware of any other
vendors who might require this?

Thanks
Kishon
> +
> +/**
> + * struct phy_configure_opts_eth_serdes - Ethernet SerDes This structure is used
> + * to represent the configuration state of a Ethernet Serdes PHY.
> + * @speed: Speed of the serdes interface in Mbps
> + * @media_type: Specifies which media the serdes will be using
> + */
> +struct phy_configure_opts_eth_serdes {
> +	u32                        speed;
> +	enum ethernet_media_type   media_type;
> +};
> +
> +#endif
> +
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> index e435bdb0bab3..78ecb375cede 100644
> --- a/include/linux/phy/phy.h
> +++ b/include/linux/phy/phy.h
> @@ -18,6 +18,7 @@
>  
>  #include <linux/phy/phy-dp.h>
>  #include <linux/phy/phy-mipi-dphy.h>
> +#include <linux/phy/phy-ethernet-serdes.h>
>  
>  struct phy;
>  
> @@ -49,11 +50,14 @@ enum phy_mode {
>   *
>   * @mipi_dphy:	Configuration set applicable for phys supporting
>   *		the MIPI_DPHY phy mode.
> + * @eth_serdes: Configuration set applicable for phys supporting
> + *		the ethernet serdes.
>   * @dp:		Configuration set applicable for phys supporting
>   *		the DisplayPort protocol.
>   */
>  union phy_configure_opts {
>  	struct phy_configure_opts_mipi_dphy	mipi_dphy;
> +	struct phy_configure_opts_eth_serdes	eth_serdes;
>  	struct phy_configure_opts_dp		dp;
>  };
>  
>
Steen Hegelund Jan. 15, 2021, 4:14 p.m. UTC | #5
Hi Kishon,

On Fri, 2021-01-15 at 21:22 +0530, Kishon Vijay Abraham I wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> Hi,
> 
> On 07/01/21 2:49 pm, Steen Hegelund wrote:
> > Provide a new ethernet phy configuration structure, that
> > allow PHYs used for ethernet to be configured with
> > speed, media type and clock information.
> > 
> > Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
> > Signed-off-by: Steen Hegelund <steen.hegelund@microchip.com>
> > Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> > ---
> >  include/linux/phy/phy-ethernet-serdes.h | 30
> > +++++++++++++++++++++++++
> >  include/linux/phy/phy.h                 |  4 ++++
> >  2 files changed, 34 insertions(+)
> >  create mode 100644 include/linux/phy/phy-ethernet-serdes.h
> > 
> > diff --git a/include/linux/phy/phy-ethernet-serdes.h
> > b/include/linux/phy/phy-ethernet-serdes.h
> > new file mode 100644
> > index 000000000000..d2462fadf179
> > --- /dev/null
> > +++ b/include/linux/phy/phy-ethernet-serdes.h
> > @@ -0,0 +1,30 @@
> > +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
> > +/*
> > + * Microchip Sparx5 Ethernet SerDes driver
> > + *
> > + * Copyright (c) 2020 Microschip Inc
> > + */
> > +#ifndef __PHY_ETHERNET_SERDES_H_
> > +#define __PHY_ETHERNET_SERDES_H_
> > +
> > +#include <linux/types.h>
> > +
> > +enum ethernet_media_type {
> > +     ETH_MEDIA_DEFAULT,
> > +     ETH_MEDIA_SR,
> > +     ETH_MEDIA_DAC,
> > +};
> 
> I'm not familiar with Ethernet. Are these generic media types? what
> does
> SR or DAC refer to? 

The SR stands for Short Reach and is a fiber type connection used by
SFPs.  There also other "reach" variants.

DAC stands for Direct Attach Copper and is a type of cable that plugs
into an SFP cage and provides information back to the user via its
EEPROM regarding supported speed and capabilities in general.  These
typically supports speed of 5G or more.

The SFP/Phylink is the "out-of-band" method that provides the type of
connection: speed and media type that allows the client to adapt the
SerDes configuration to the type of media selected by the user.

> Are there other media types? What is the out-of-band
> mechanism by which the controller gets the media type? Why was this
> not
> required for other existing Ethernet SERDES? 

This is probably a matter of the interface speed are now getting higher
and the amount of configuration needed for the SerDes have increased,
at the same time as this is not being a static setup, because the user
an plug and unplug media to the SFP cage.

> Are you aware of any other
> vendors who might require this?

I suspect that going forward it will become more widespread, at least
we have more chips in the pipeline that need this SerDes for high speed
connectivity.


> 
> Thanks
> Kishon
> > +
> > +/**
> > + * struct phy_configure_opts_eth_serdes - Ethernet SerDes This
> > structure is used
> > + * to represent the configuration state of a Ethernet Serdes PHY.
> > + * @speed: Speed of the serdes interface in Mbps
> > + * @media_type: Specifies which media the serdes will be using
> > + */
> > +struct phy_configure_opts_eth_serdes {
> > +     u32                        speed;
> > +     enum ethernet_media_type   media_type;
> > +};
> > +
> > +#endif
> > +
> > diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> > index e435bdb0bab3..78ecb375cede 100644
> > --- a/include/linux/phy/phy.h
> > +++ b/include/linux/phy/phy.h
> > @@ -18,6 +18,7 @@
> > 
> >  #include <linux/phy/phy-dp.h>
> >  #include <linux/phy/phy-mipi-dphy.h>
> > +#include <linux/phy/phy-ethernet-serdes.h>
> > 
> >  struct phy;
> > 
> > @@ -49,11 +50,14 @@ enum phy_mode {
> >   *
> >   * @mipi_dphy:       Configuration set applicable for phys
> > supporting
> >   *           the MIPI_DPHY phy mode.
> > + * @eth_serdes: Configuration set applicable for phys supporting
> > + *           the ethernet serdes.
> >   * @dp:              Configuration set applicable for phys
> > supporting
> >   *           the DisplayPort protocol.
> >   */
> >  union phy_configure_opts {
> >       struct phy_configure_opts_mipi_dphy     mipi_dphy;
> > +     struct phy_configure_opts_eth_serdes    eth_serdes;
> >       struct phy_configure_opts_dp            dp;
> >  };
> > 
> > 

Best Regards
Steen
Steen Hegelund Jan. 22, 2021, 3:07 p.m. UTC | #6
Hi Kishon and Vinod,

On Fri, 2021-01-15 at 21:22 +0530, Kishon Vijay Abraham I wrote:
> > 
> > 

I was just wanted to know if there are any outstanding items that you
would like me to handle, or you think that the driver is acceptable as
it is now?

BR
Steen
Kishon Vijay Abraham I Jan. 27, 2021, 12:34 p.m. UTC | #7
Hi Steen,

On 15/01/21 9:44 pm, Steen Hegelund wrote:
> Hi Kishon,
> 
> On Fri, 2021-01-15 at 21:22 +0530, Kishon Vijay Abraham I wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you
>> know the content is safe
>>
>> Hi,
>>
>> On 07/01/21 2:49 pm, Steen Hegelund wrote:
>>> Provide a new ethernet phy configuration structure, that
>>> allow PHYs used for ethernet to be configured with
>>> speed, media type and clock information.
>>>
>>> Signed-off-by: Lars Povlsen <lars.povlsen@microchip.com>
>>> Signed-off-by: Steen Hegelund <steen.hegelund@microchip.com>
>>> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
>>> ---
>>>  include/linux/phy/phy-ethernet-serdes.h | 30
>>> +++++++++++++++++++++++++
>>>  include/linux/phy/phy.h                 |  4 ++++
>>>  2 files changed, 34 insertions(+)
>>>  create mode 100644 include/linux/phy/phy-ethernet-serdes.h
>>>
>>> diff --git a/include/linux/phy/phy-ethernet-serdes.h
>>> b/include/linux/phy/phy-ethernet-serdes.h
>>> new file mode 100644
>>> index 000000000000..d2462fadf179
>>> --- /dev/null
>>> +++ b/include/linux/phy/phy-ethernet-serdes.h
>>> @@ -0,0 +1,30 @@
>>> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
>>> +/*
>>> + * Microchip Sparx5 Ethernet SerDes driver
>>> + *
>>> + * Copyright (c) 2020 Microschip Inc
>>> + */
>>> +#ifndef __PHY_ETHERNET_SERDES_H_
>>> +#define __PHY_ETHERNET_SERDES_H_
>>> +
>>> +#include <linux/types.h>
>>> +
>>> +enum ethernet_media_type {
>>> +     ETH_MEDIA_DEFAULT,
>>> +     ETH_MEDIA_SR,
>>> +     ETH_MEDIA_DAC,
>>> +};
>>
>> I'm not familiar with Ethernet. Are these generic media types? what
>> does
>> SR or DAC refer to? 
> 
> The SR stands for Short Reach and is a fiber type connection used by
> SFPs.  There also other "reach" variants.
> 
> DAC stands for Direct Attach Copper and is a type of cable that plugs
> into an SFP cage and provides information back to the user via its
> EEPROM regarding supported speed and capabilities in general.  These
> typically supports speed of 5G or more.
> 
> The SFP/Phylink is the "out-of-band" method that provides the type of
> connection: speed and media type that allows the client to adapt the
> SerDes configuration to the type of media selected by the user.
> 
>> Are there other media types? What is the out-of-band
>> mechanism by which the controller gets the media type? Why was this
>> not
>> required for other existing Ethernet SERDES? 
> 
> This is probably a matter of the interface speed are now getting higher
> and the amount of configuration needed for the SerDes have increased,
> at the same time as this is not being a static setup, because the user
> an plug and unplug media to the SFP cage.
> 
>> Are you aware of any other
>> vendors who might require this?
> 
> I suspect that going forward it will become more widespread, at least
> we have more chips in the pipeline that need this SerDes for high speed
> connectivity.

For this case I would recommend to add new API, something like
phy_set_media(). Configure() and Validate() is more for probing
something that is supported by SERDES and changing the parameters. But
in this case, I'd think the media type is determined by the cable that
is connected and cannot be changed.

Thanks
Kishon
Steen Hegelund Jan. 27, 2021, 2:45 p.m. UTC | #8
Hi Kishon,

On Wed, 2021-01-27 at 18:04 +0530, Kishon Vijay Abraham I wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 

...

> > > 
> > > I'm not familiar with Ethernet. Are these generic media types? what
> > > does
> > > SR or DAC refer to?
> > 
> > The SR stands for Short Reach and is a fiber type connection used by
> > SFPs.  There also other "reach" variants.
> > 
> > DAC stands for Direct Attach Copper and is a type of cable that plugs
> > into an SFP cage and provides information back to the user via its
> > EEPROM regarding supported speed and capabilities in general.  These
> > typically supports speed of 5G or more.
> > 
> > The SFP/Phylink is the "out-of-band" method that provides the type of
> > connection: speed and media type that allows the client to adapt the
> > SerDes configuration to the type of media selected by the user.
> > 
> > > Are there other media types? What is the out-of-band
> > > mechanism by which the controller gets the media type? Why was this
> > > not
> > > required for other existing Ethernet SERDES?
> > 
> > This is probably a matter of the interface speed are now getting higher
> > and the amount of configuration needed for the SerDes have increased,
> > at the same time as this is not being a static setup, because the user
> > an plug and unplug media to the SFP cage.
> > 
> > > Are you aware of any other
> > > vendors who might require this?
> > 
> > I suspect that going forward it will become more widespread, at least
> > we have more chips in the pipeline that need this SerDes for high speed
> > connectivity.
> 
> For this case I would recommend to add new API, something like
> phy_set_media(). Configure() and Validate() is more for probing
> something that is supported by SERDES and changing the parameters. But
> in this case, I'd think the media type is determined by the cable that
> is connected and cannot be changed.
> 
> Thanks
> Kishon

I assume that you would like a separate interface for the speed information as well?

Thanks for your comments.

BR
Steen
diff mbox series

Patch

diff --git a/include/linux/phy/phy-ethernet-serdes.h b/include/linux/phy/phy-ethernet-serdes.h
new file mode 100644
index 000000000000..d2462fadf179
--- /dev/null
+++ b/include/linux/phy/phy-ethernet-serdes.h
@@ -0,0 +1,30 @@ 
+/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
+/*
+ * Microchip Sparx5 Ethernet SerDes driver
+ *
+ * Copyright (c) 2020 Microschip Inc
+ */
+#ifndef __PHY_ETHERNET_SERDES_H_
+#define __PHY_ETHERNET_SERDES_H_
+
+#include <linux/types.h>
+
+enum ethernet_media_type {
+	ETH_MEDIA_DEFAULT,
+	ETH_MEDIA_SR,
+	ETH_MEDIA_DAC,
+};
+
+/**
+ * struct phy_configure_opts_eth_serdes - Ethernet SerDes This structure is used
+ * to represent the configuration state of a Ethernet Serdes PHY.
+ * @speed: Speed of the serdes interface in Mbps
+ * @media_type: Specifies which media the serdes will be using
+ */
+struct phy_configure_opts_eth_serdes {
+	u32                        speed;
+	enum ethernet_media_type   media_type;
+};
+
+#endif
+
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index e435bdb0bab3..78ecb375cede 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -18,6 +18,7 @@ 
 
 #include <linux/phy/phy-dp.h>
 #include <linux/phy/phy-mipi-dphy.h>
+#include <linux/phy/phy-ethernet-serdes.h>
 
 struct phy;
 
@@ -49,11 +50,14 @@  enum phy_mode {
  *
  * @mipi_dphy:	Configuration set applicable for phys supporting
  *		the MIPI_DPHY phy mode.
+ * @eth_serdes: Configuration set applicable for phys supporting
+ *		the ethernet serdes.
  * @dp:		Configuration set applicable for phys supporting
  *		the DisplayPort protocol.
  */
 union phy_configure_opts {
 	struct phy_configure_opts_mipi_dphy	mipi_dphy;
+	struct phy_configure_opts_eth_serdes	eth_serdes;
 	struct phy_configure_opts_dp		dp;
 };