| Message ID | 20210210085255.2006824-3-steen.hegelund@microchip.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Adding the Sparx5 Serdes driver | expand |
| 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: 195 this patch: 195 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | WARNING: ENOSYS means 'invalid syscall nr' and nothing else WARNING: Possible repeated word: 'allows' |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 195 this patch: 195 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
From: Steen Hegelund <steen.hegelund@microchip.com> Date: Wed, 10 Feb 2021 09:52:53 +0100 > Provide new phy configuration interfaces for media type and speed that > allows allows e.g. PHYs used for ethernet to be configured with this > 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> > --- > drivers/phy/phy-core.c | 30 ++++++++++++++++++++++++++++++ > include/linux/phy/phy.h | 26 ++++++++++++++++++++++++++ > 2 files changed, 56 insertions(+) > > diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c > index 71cb10826326..ccb575b13777 100644 > --- a/drivers/phy/phy-core.c > +++ b/drivers/phy/phy-core.c > @@ -373,6 +373,36 @@ int phy_set_mode_ext(struct phy *phy, enum phy_mode mode, int submode) > } > EXPORT_SYMBOL_GPL(phy_set_mode_ext); > > +int phy_set_media(struct phy *phy, enum phy_media media) > +{ > + int ret; > + > + if (!phy || !phy->ops->set_media) > + return 0; > + > + mutex_lock(&phy->mutex); > + ret = phy->ops->set_media(phy, media); > + mutex_unlock(&phy->mutex); > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(phy_set_media); > + > +int phy_set_speed(struct phy *phy, int speed) > +{ > + int ret; > + > + if (!phy || !phy->ops->set_speed) > + return 0; > + > + mutex_lock(&phy->mutex); > + ret = phy->ops->set_speed(phy, speed); > + mutex_unlock(&phy->mutex); > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(phy_set_speed); > + > int phy_reset(struct phy *phy) > { > int ret; > diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h > index e435bdb0bab3..e4fd69a1faa7 100644 > --- a/include/linux/phy/phy.h > +++ b/include/linux/phy/phy.h > @@ -44,6 +44,12 @@ enum phy_mode { > PHY_MODE_DP > }; > > +enum phy_media { > + PHY_MEDIA_DEFAULT, > + PHY_MEDIA_SR, > + PHY_MEDIA_DAC, > +}; > + > /** > * union phy_configure_opts - Opaque generic phy configuration > * > @@ -64,6 +70,8 @@ union phy_configure_opts { > * @power_on: powering on the phy > * @power_off: powering off the phy > * @set_mode: set the mode of the phy > + * @set_media: set the media type of the phy (optional) > + * @set_speed: set the speed of the phy (optional) > * @reset: resetting the phy > * @calibrate: calibrate the phy > * @release: ops to be performed while the consumer relinquishes the PHY > @@ -75,6 +83,8 @@ struct phy_ops { > int (*power_on)(struct phy *phy); > int (*power_off)(struct phy *phy); > int (*set_mode)(struct phy *phy, enum phy_mode mode, int submode); > + int (*set_media)(struct phy *phy, enum phy_media media); > + int (*set_speed)(struct phy *phy, int speed); > > /** > * @configure: > @@ -215,6 +225,8 @@ int phy_power_off(struct phy *phy); > int phy_set_mode_ext(struct phy *phy, enum phy_mode mode, int submode); > #define phy_set_mode(phy, mode) \ > phy_set_mode_ext(phy, mode, 0) > +int phy_set_media(struct phy *phy, enum phy_media media); > +int phy_set_speed(struct phy *phy, int speed); > int phy_configure(struct phy *phy, union phy_configure_opts *opts); > int phy_validate(struct phy *phy, enum phy_mode mode, int submode, > union phy_configure_opts *opts); > @@ -344,6 +356,20 @@ static inline int phy_set_mode_ext(struct phy *phy, enum phy_mode mode, > #define phy_set_mode(phy, mode) \ > phy_set_mode_ext(phy, mode, 0) > > +static inline int phy_set_media(struct phy *phy, enum phy_media media) > +{ > + if (!phy) > + return 0; > + return -ENOSYS; > +} Maybe ENODEV instead?
Hi David, On Wed, 2021-02-10 at 15:32 -0800, David Miller wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you > know the content is safe > > From: Steen Hegelund <steen.hegelund@microchip.com> > Date: Wed, 10 Feb 2021 09:52:53 +0100 > > > Provide new phy configuration interfaces for media type and speed > > that > > allows allows e.g. PHYs used for ethernet to be configured with > > this > > 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> > > --- > > drivers/phy/phy-core.c | 30 ++++++++++++++++++++++++++++++ > > include/linux/phy/phy.h | 26 ++++++++++++++++++++++++++ > > 2 files changed, 56 insertions(+) > > > > diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c > > index 71cb10826326..ccb575b13777 100644 > > --- a/drivers/phy/phy-core.c > > +++ b/drivers/phy/phy-core.c > > @@ -373,6 +373,36 @@ int phy_set_mode_ext(struct phy *phy, enum > > phy_mode mode, int submode) > > } > > EXPORT_SYMBOL_GPL(phy_set_mode_ext); > > > > +int phy_set_media(struct phy *phy, enum phy_media media) > > +{ > > + int ret; > > + > > + if (!phy || !phy->ops->set_media) > > + return 0; > > + > > + mutex_lock(&phy->mutex); > > + ret = phy->ops->set_media(phy, media); > > + mutex_unlock(&phy->mutex); > > + > > + return ret; > > +} > > +EXPORT_SYMBOL_GPL(phy_set_media); > > + > > +int phy_set_speed(struct phy *phy, int speed) > > +{ > > + int ret; > > + > > + if (!phy || !phy->ops->set_speed) > > + return 0; > > + > > + mutex_lock(&phy->mutex); > > + ret = phy->ops->set_speed(phy, speed); > > + mutex_unlock(&phy->mutex); > > + > > + return ret; > > +} > > +EXPORT_SYMBOL_GPL(phy_set_speed); > > + > > int phy_reset(struct phy *phy) > > { > > int ret; > > diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h > > index e435bdb0bab3..e4fd69a1faa7 100644 > > --- a/include/linux/phy/phy.h > > +++ b/include/linux/phy/phy.h > > @@ -44,6 +44,12 @@ enum phy_mode { > > PHY_MODE_DP > > }; > > > > +enum phy_media { > > + PHY_MEDIA_DEFAULT, > > + PHY_MEDIA_SR, > > + PHY_MEDIA_DAC, > > +}; > > + > > /** > > * union phy_configure_opts - Opaque generic phy configuration > > * > > @@ -64,6 +70,8 @@ union phy_configure_opts { > > * @power_on: powering on the phy > > * @power_off: powering off the phy > > * @set_mode: set the mode of the phy > > + * @set_media: set the media type of the phy (optional) > > + * @set_speed: set the speed of the phy (optional) > > * @reset: resetting the phy > > * @calibrate: calibrate the phy > > * @release: ops to be performed while the consumer relinquishes > > the PHY > > @@ -75,6 +83,8 @@ struct phy_ops { > > int (*power_on)(struct phy *phy); > > int (*power_off)(struct phy *phy); > > int (*set_mode)(struct phy *phy, enum phy_mode mode, int > > submode); > > + int (*set_media)(struct phy *phy, enum phy_media media); > > + int (*set_speed)(struct phy *phy, int speed); > > > > /** > > * @configure: > > @@ -215,6 +225,8 @@ int phy_power_off(struct phy *phy); > > int phy_set_mode_ext(struct phy *phy, enum phy_mode mode, int > > submode); > > #define phy_set_mode(phy, mode) \ > > phy_set_mode_ext(phy, mode, 0) > > +int phy_set_media(struct phy *phy, enum phy_media media); > > +int phy_set_speed(struct phy *phy, int speed); > > int phy_configure(struct phy *phy, union phy_configure_opts > > *opts); > > int phy_validate(struct phy *phy, enum phy_mode mode, int submode, > > union phy_configure_opts *opts); > > @@ -344,6 +356,20 @@ static inline int phy_set_mode_ext(struct phy > > *phy, enum phy_mode mode, > > #define phy_set_mode(phy, mode) \ > > phy_set_mode_ext(phy, mode, 0) > > > > +static inline int phy_set_media(struct phy *phy, enum phy_media > > media) > > +{ > > + if (!phy) > > + return 0; > > + return -ENOSYS; > > +} > > Maybe ENODEV instead? Sure. I will update that.
Hi Steen, On 10/02/21 2:22 pm, Steen Hegelund wrote: > Provide new phy configuration interfaces for media type and speed that > allows allows e.g. PHYs used for ethernet to be configured with this > 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> > --- > drivers/phy/phy-core.c | 30 ++++++++++++++++++++++++++++++ > include/linux/phy/phy.h | 26 ++++++++++++++++++++++++++ > 2 files changed, 56 insertions(+) > > diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c > index 71cb10826326..ccb575b13777 100644 > --- a/drivers/phy/phy-core.c > +++ b/drivers/phy/phy-core.c > @@ -373,6 +373,36 @@ int phy_set_mode_ext(struct phy *phy, enum phy_mode mode, int submode) > } > EXPORT_SYMBOL_GPL(phy_set_mode_ext); > > +int phy_set_media(struct phy *phy, enum phy_media media) > +{ > + int ret; > + > + if (!phy || !phy->ops->set_media) > + return 0; > + > + mutex_lock(&phy->mutex); > + ret = phy->ops->set_media(phy, media); > + mutex_unlock(&phy->mutex); > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(phy_set_media); > + > +int phy_set_speed(struct phy *phy, int speed) > +{ > + int ret; > + > + if (!phy || !phy->ops->set_speed) > + return 0; > + > + mutex_lock(&phy->mutex); > + ret = phy->ops->set_speed(phy, speed); > + mutex_unlock(&phy->mutex); > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(phy_set_speed); Can't speed derived from mode? Do we need a separate set_speed function? Thanks Kishon > + > int phy_reset(struct phy *phy) > { > int ret; > diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h > index e435bdb0bab3..e4fd69a1faa7 100644 > --- a/include/linux/phy/phy.h > +++ b/include/linux/phy/phy.h > @@ -44,6 +44,12 @@ enum phy_mode { > PHY_MODE_DP > }; > > +enum phy_media { > + PHY_MEDIA_DEFAULT, > + PHY_MEDIA_SR, > + PHY_MEDIA_DAC, > +}; > + > /** > * union phy_configure_opts - Opaque generic phy configuration > * > @@ -64,6 +70,8 @@ union phy_configure_opts { > * @power_on: powering on the phy > * @power_off: powering off the phy > * @set_mode: set the mode of the phy > + * @set_media: set the media type of the phy (optional) > + * @set_speed: set the speed of the phy (optional) > * @reset: resetting the phy > * @calibrate: calibrate the phy > * @release: ops to be performed while the consumer relinquishes the PHY > @@ -75,6 +83,8 @@ struct phy_ops { > int (*power_on)(struct phy *phy); > int (*power_off)(struct phy *phy); > int (*set_mode)(struct phy *phy, enum phy_mode mode, int submode); > + int (*set_media)(struct phy *phy, enum phy_media media); > + int (*set_speed)(struct phy *phy, int speed); > > /** > * @configure: > @@ -215,6 +225,8 @@ int phy_power_off(struct phy *phy); > int phy_set_mode_ext(struct phy *phy, enum phy_mode mode, int submode); > #define phy_set_mode(phy, mode) \ > phy_set_mode_ext(phy, mode, 0) > +int phy_set_media(struct phy *phy, enum phy_media media); > +int phy_set_speed(struct phy *phy, int speed); > int phy_configure(struct phy *phy, union phy_configure_opts *opts); > int phy_validate(struct phy *phy, enum phy_mode mode, int submode, > union phy_configure_opts *opts); > @@ -344,6 +356,20 @@ static inline int phy_set_mode_ext(struct phy *phy, enum phy_mode mode, > #define phy_set_mode(phy, mode) \ > phy_set_mode_ext(phy, mode, 0) > > +static inline int phy_set_media(struct phy *phy, enum phy_media media) > +{ > + if (!phy) > + return 0; > + return -ENOSYS; > +} > + > +static inline int phy_set_speed(struct phy *phy, int speed) > +{ > + if (!phy) > + return 0; > + return -ENOSYS; > +} > + > static inline enum phy_mode phy_get_mode(struct phy *phy) > { > return PHY_MODE_INVALID; >
Hi Kishon, On Fri, 2021-02-12 at 17:02 +0530, Kishon Vijay Abraham I wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you > know the content is safe > > Hi Steen, > > On 10/02/21 2:22 pm, Steen Hegelund wrote: > > Provide new phy configuration interfaces for media type and speed > > that > > allows allows e.g. PHYs used for ethernet to be configured with > > this > > 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> > > --- > > drivers/phy/phy-core.c | 30 ++++++++++++++++++++++++++++++ > > include/linux/phy/phy.h | 26 ++++++++++++++++++++++++++ > > 2 files changed, 56 insertions(+) > > > > diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c > > index 71cb10826326..ccb575b13777 100644 > > --- a/drivers/phy/phy-core.c > > +++ b/drivers/phy/phy-core.c > > @@ -373,6 +373,36 @@ int phy_set_mode_ext(struct phy *phy, enum > > phy_mode mode, int submode) > > } > > EXPORT_SYMBOL_GPL(phy_set_mode_ext); > > > > +int phy_set_media(struct phy *phy, enum phy_media media) > > +{ > > + int ret; > > + > > + if (!phy || !phy->ops->set_media) > > + return 0; > > + > > + mutex_lock(&phy->mutex); > > + ret = phy->ops->set_media(phy, media); > > + mutex_unlock(&phy->mutex); > > + > > + return ret; > > +} > > +EXPORT_SYMBOL_GPL(phy_set_media); > > + > > +int phy_set_speed(struct phy *phy, int speed) > > +{ > > + int ret; > > + > > + if (!phy || !phy->ops->set_speed) > > + return 0; > > + > > + mutex_lock(&phy->mutex); > > + ret = phy->ops->set_speed(phy, speed); > > + mutex_unlock(&phy->mutex); > > + > > + return ret; > > +} > > +EXPORT_SYMBOL_GPL(phy_set_speed); > > Can't speed derived from mode? Do we need a separate set_speed > function? > > Thanks > Kishon Yes the client will need to be able to choose the speed as needed: e.g. lower than the serdes mode supports, in case the the media or the other end is not capable of running that speed. An example is a 10G and 25G serdes connected via DAC and as there is no inband autoneg, the 25G client would have to manually select 10G speed to communicate with its partner. > > > + > > int phy_reset(struct phy *phy) > > { > > int ret; > > diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h > > index e435bdb0bab3..e4fd69a1faa7 100644 > > --- a/include/linux/phy/phy.h > > +++ b/include/linux/phy/phy.h > > @@ -44,6 +44,12 @@ enum phy_mode { > > PHY_MODE_DP > > }; > > > > +enum phy_media { > > + PHY_MEDIA_DEFAULT, > > + PHY_MEDIA_SR, > > + PHY_MEDIA_DAC, > > +}; > > + > > /** > > * union phy_configure_opts - Opaque generic phy configuration > > * > > @@ -64,6 +70,8 @@ union phy_configure_opts { > > * @power_on: powering on the phy > > * @power_off: powering off the phy > > * @set_mode: set the mode of the phy > > + * @set_media: set the media type of the phy (optional) > > + * @set_speed: set the speed of the phy (optional) > > * @reset: resetting the phy > > * @calibrate: calibrate the phy > > * @release: ops to be performed while the consumer relinquishes > > the PHY > > @@ -75,6 +83,8 @@ struct phy_ops { > > int (*power_on)(struct phy *phy); > > int (*power_off)(struct phy *phy); > > int (*set_mode)(struct phy *phy, enum phy_mode mode, int > > submode); > > + int (*set_media)(struct phy *phy, enum phy_media media); > > + int (*set_speed)(struct phy *phy, int speed); > > > > /** > > * @configure: > > @@ -215,6 +225,8 @@ int phy_power_off(struct phy *phy); > > int phy_set_mode_ext(struct phy *phy, enum phy_mode mode, int > > submode); > > #define phy_set_mode(phy, mode) \ > > phy_set_mode_ext(phy, mode, 0) > > +int phy_set_media(struct phy *phy, enum phy_media media); > > +int phy_set_speed(struct phy *phy, int speed); > > int phy_configure(struct phy *phy, union phy_configure_opts > > *opts); > > int phy_validate(struct phy *phy, enum phy_mode mode, int submode, > > union phy_configure_opts *opts); > > @@ -344,6 +356,20 @@ static inline int phy_set_mode_ext(struct phy > > *phy, enum phy_mode mode, > > #define phy_set_mode(phy, mode) \ > > phy_set_mode_ext(phy, mode, 0) > > > > +static inline int phy_set_media(struct phy *phy, enum phy_media > > media) > > +{ > > + if (!phy) > > + return 0; > > + return -ENOSYS; > > +} > > + > > +static inline int phy_set_speed(struct phy *phy, int speed) > > +{ > > + if (!phy) > > + return 0; > > + return -ENOSYS; > > +} > > + > > static inline enum phy_mode phy_get_mode(struct phy *phy) > > { > > return PHY_MODE_INVALID; > > Thanks for your comments.
Hi Steen, On 12/02/21 6:35 pm, Steen Hegelund wrote: > Hi Kishon, > > On Fri, 2021-02-12 at 17:02 +0530, Kishon Vijay Abraham I wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you >> know the content is safe >> >> Hi Steen, >> >> On 10/02/21 2:22 pm, Steen Hegelund wrote: >>> Provide new phy configuration interfaces for media type and speed >>> that >>> allows allows e.g. PHYs used for ethernet to be configured with >>> this >>> 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> >>> --- >>> drivers/phy/phy-core.c | 30 ++++++++++++++++++++++++++++++ >>> include/linux/phy/phy.h | 26 ++++++++++++++++++++++++++ >>> 2 files changed, 56 insertions(+) >>> >>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c >>> index 71cb10826326..ccb575b13777 100644 >>> --- a/drivers/phy/phy-core.c >>> +++ b/drivers/phy/phy-core.c >>> @@ -373,6 +373,36 @@ int phy_set_mode_ext(struct phy *phy, enum >>> phy_mode mode, int submode) >>> } >>> EXPORT_SYMBOL_GPL(phy_set_mode_ext); >>> >>> +int phy_set_media(struct phy *phy, enum phy_media media) >>> +{ >>> + int ret; >>> + >>> + if (!phy || !phy->ops->set_media) >>> + return 0; >>> + >>> + mutex_lock(&phy->mutex); >>> + ret = phy->ops->set_media(phy, media); >>> + mutex_unlock(&phy->mutex); >>> + >>> + return ret; >>> +} >>> +EXPORT_SYMBOL_GPL(phy_set_media); >>> + >>> +int phy_set_speed(struct phy *phy, int speed) >>> +{ >>> + int ret; >>> + >>> + if (!phy || !phy->ops->set_speed) >>> + return 0; >>> + >>> + mutex_lock(&phy->mutex); >>> + ret = phy->ops->set_speed(phy, speed); >>> + mutex_unlock(&phy->mutex); >>> + >>> + return ret; >>> +} >>> +EXPORT_SYMBOL_GPL(phy_set_speed); >> >> Can't speed derived from mode? Do we need a separate set_speed >> function? >> >> Thanks >> Kishon > > Yes the client will need to be able to choose the speed as needed: > e.g. lower than the serdes mode supports, in case the the media or the > other end is not capable of running that speed. > > An example is a 10G and 25G serdes connected via DAC and as there is no > inband autoneg, the 25G client would have to manually select 10G speed > to communicate with its partner. Okay. Is it going to be some sort of manual negotiation where the Ethernet controller invokes set_speed with different speeds? Or the Ethernet controller will get the speed using some out of band mechanism and invokes set_speed once with the actual speed? Thanks Kishon > >> >>> + >>> int phy_reset(struct phy *phy) >>> { >>> int ret; >>> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h >>> index e435bdb0bab3..e4fd69a1faa7 100644 >>> --- a/include/linux/phy/phy.h >>> +++ b/include/linux/phy/phy.h >>> @@ -44,6 +44,12 @@ enum phy_mode { >>> PHY_MODE_DP >>> }; >>> >>> +enum phy_media { >>> + PHY_MEDIA_DEFAULT, >>> + PHY_MEDIA_SR, >>> + PHY_MEDIA_DAC, >>> +}; >>> + >>> /** >>> * union phy_configure_opts - Opaque generic phy configuration >>> * >>> @@ -64,6 +70,8 @@ union phy_configure_opts { >>> * @power_on: powering on the phy >>> * @power_off: powering off the phy >>> * @set_mode: set the mode of the phy >>> + * @set_media: set the media type of the phy (optional) >>> + * @set_speed: set the speed of the phy (optional) >>> * @reset: resetting the phy >>> * @calibrate: calibrate the phy >>> * @release: ops to be performed while the consumer relinquishes >>> the PHY >>> @@ -75,6 +83,8 @@ struct phy_ops { >>> int (*power_on)(struct phy *phy); >>> int (*power_off)(struct phy *phy); >>> int (*set_mode)(struct phy *phy, enum phy_mode mode, int >>> submode); >>> + int (*set_media)(struct phy *phy, enum phy_media media); >>> + int (*set_speed)(struct phy *phy, int speed); >>> >>> /** >>> * @configure: >>> @@ -215,6 +225,8 @@ int phy_power_off(struct phy *phy); >>> int phy_set_mode_ext(struct phy *phy, enum phy_mode mode, int >>> submode); >>> #define phy_set_mode(phy, mode) \ >>> phy_set_mode_ext(phy, mode, 0) >>> +int phy_set_media(struct phy *phy, enum phy_media media); >>> +int phy_set_speed(struct phy *phy, int speed); >>> int phy_configure(struct phy *phy, union phy_configure_opts >>> *opts); >>> int phy_validate(struct phy *phy, enum phy_mode mode, int submode, >>> union phy_configure_opts *opts); >>> @@ -344,6 +356,20 @@ static inline int phy_set_mode_ext(struct phy >>> *phy, enum phy_mode mode, >>> #define phy_set_mode(phy, mode) \ >>> phy_set_mode_ext(phy, mode, 0) >>> >>> +static inline int phy_set_media(struct phy *phy, enum phy_media >>> media) >>> +{ >>> + if (!phy) >>> + return 0; >>> + return -ENOSYS; >>> +} >>> + >>> +static inline int phy_set_speed(struct phy *phy, int speed) >>> +{ >>> + if (!phy) >>> + return 0; >>> + return -ENOSYS; >>> +} >>> + >>> static inline enum phy_mode phy_get_mode(struct phy *phy) >>> { >>> return PHY_MODE_INVALID; >>> > > > Thanks for your comments. > >
On Mon, Feb 15, 2021 at 05:25:10PM +0530, Kishon Vijay Abraham I wrote: > Okay. Is it going to be some sort of manual negotiation where the > Ethernet controller invokes set_speed with different speeds? Or the > Ethernet controller will get the speed using some out of band mechanism > and invokes set_speed once with the actual speed? Hi Kishon There are a few different mechanism possible. The SFP has an EEPROM which contains lots of parameters. One is the maximum baud rate the module supports. PHYLINK will combine this information with the MAC capabilities to determine the default speed. The users can select the mode the MAC works in, e.g. 1000BaseX vs 2500BaseX, via ethtool -s. Different modes needs different speeds. Some copper PHYs will change there host side interface baud rate when the media side interface changes mode. 10GBASE-X for 10G copper, 5GBase-X for 5G COPPER, 2500Base-X for 2.5G copper, and SGMII for old school 10/100/1G Ethernet. Mainline Linux has no support for it, but some 'vendor crap' will do a manual negotiation, simply trying different speeds and see if the SERDES establishes link. There is nothing standardised for this, as far as i know. Andrew
Hi Andrew and Kishon, On Mon, 2021-02-15 at 15:07 +0100, Andrew Lunn wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you > know the content is safe > > On Mon, Feb 15, 2021 at 05:25:10PM +0530, Kishon Vijay Abraham I > wrote: > > Okay. Is it going to be some sort of manual negotiation where the > > Ethernet controller invokes set_speed with different speeds? Or the > > Ethernet controller will get the speed using some out of band > > mechanism > > and invokes set_speed once with the actual speed? > > Hi Kishon > > There are a few different mechanism possible. > > The SFP has an EEPROM which contains lots of parameters. One is the > maximum baud rate the module supports. PHYLINK will combine this > information with the MAC capabilities to determine the default speed. > > The users can select the mode the MAC works in, e.g. 1000BaseX vs > 2500BaseX, via ethtool -s. Different modes needs different speeds. > > Some copper PHYs will change there host side interface baud rate when > the media side interface changes mode. 10GBASE-X for 10G copper, > 5GBase-X for 5G COPPER, 2500Base-X for 2.5G copper, and SGMII for > old school 10/100/1G Ethernet. > > Mainline Linux has no support for it, but some 'vendor crap' will do > a > manual negotiation, simply trying different speeds and see if the > SERDES establishes link. There is nothing standardised for this, as > far as i know. > > Andrew Yes, in case I mention the only way to ensure communication is human intervention to set the speed to the highest common denominator. BR Steen
Hi, On 16/02/21 2:07 pm, Steen Hegelund wrote: > Hi Andrew and Kishon, > > On Mon, 2021-02-15 at 15:07 +0100, Andrew Lunn wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you >> know the content is safe >> >> On Mon, Feb 15, 2021 at 05:25:10PM +0530, Kishon Vijay Abraham I >> wrote: >>> Okay. Is it going to be some sort of manual negotiation where the >>> Ethernet controller invokes set_speed with different speeds? Or the >>> Ethernet controller will get the speed using some out of band >>> mechanism >>> and invokes set_speed once with the actual speed? >> >> Hi Kishon >> >> There are a few different mechanism possible. >> >> The SFP has an EEPROM which contains lots of parameters. One is the >> maximum baud rate the module supports. PHYLINK will combine this >> information with the MAC capabilities to determine the default speed. >> >> The users can select the mode the MAC works in, e.g. 1000BaseX vs >> 2500BaseX, via ethtool -s. Different modes needs different speeds. >> >> Some copper PHYs will change there host side interface baud rate when >> the media side interface changes mode. 10GBASE-X for 10G copper, >> 5GBase-X for 5G COPPER, 2500Base-X for 2.5G copper, and SGMII for >> old school 10/100/1G Ethernet. >> >> Mainline Linux has no support for it, but some 'vendor crap' will do >> a >> manual negotiation, simply trying different speeds and see if the >> SERDES establishes link. There is nothing standardised for this, as >> far as i know. >> >> Andrew > > Yes, in case I mention the only way to ensure communication is human > intervention to set the speed to the highest common denominator. Okay.. is it the same case for set_media as well? Thanks Kishon
Hi Kishon, On Tue, 2021-02-16 at 15:54 +0530, Kishon Vijay Abraham I wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you > know the content is safe > > Hi, > > On 16/02/21 2:07 pm, Steen Hegelund wrote: > > Hi Andrew and Kishon, > > > > On Mon, 2021-02-15 at 15:07 +0100, Andrew Lunn wrote: > > > EXTERNAL EMAIL: Do not click links or open attachments unless you > > > know the content is safe > > > > > > On Mon, Feb 15, 2021 at 05:25:10PM +0530, Kishon Vijay Abraham I > > > wrote: > > > > Okay. Is it going to be some sort of manual negotiation where > > > > the > > > > Ethernet controller invokes set_speed with different speeds? Or > > > > the > > > > Ethernet controller will get the speed using some out of band > > > > mechanism > > > > and invokes set_speed once with the actual speed? > > > > > > Hi Kishon > > > > > > There are a few different mechanism possible. > > > > > > The SFP has an EEPROM which contains lots of parameters. One is > > > the > > > maximum baud rate the module supports. PHYLINK will combine this > > > information with the MAC capabilities to determine the default > > > speed. > > > > > > The users can select the mode the MAC works in, e.g. 1000BaseX vs > > > 2500BaseX, via ethtool -s. Different modes needs different > > > speeds. > > > > > > Some copper PHYs will change there host side interface baud rate > > > when > > > the media side interface changes mode. 10GBASE-X for 10G copper, > > > 5GBase-X for 5G COPPER, 2500Base-X for 2.5G copper, and SGMII for > > > old school 10/100/1G Ethernet. > > > > > > Mainline Linux has no support for it, but some 'vendor crap' will > > > do > > > a > > > manual negotiation, simply trying different speeds and see if the > > > SERDES establishes link. There is nothing standardised for this, > > > as > > > far as i know. > > > > > > Andrew > > > > Yes, in case I mention the only way to ensure communication is > > human > > intervention to set the speed to the highest common denominator. > > Okay.. is it the same case for set_media as well? Yes, but in the media type case, we should be able to get the type from the DAC cable EPPROM information as mentioned by Andrew, so human intervention should not be needed. > > Thanks > Kishon Thanks for your comments.
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c index 71cb10826326..ccb575b13777 100644 --- a/drivers/phy/phy-core.c +++ b/drivers/phy/phy-core.c @@ -373,6 +373,36 @@ int phy_set_mode_ext(struct phy *phy, enum phy_mode mode, int submode) } EXPORT_SYMBOL_GPL(phy_set_mode_ext); +int phy_set_media(struct phy *phy, enum phy_media media) +{ + int ret; + + if (!phy || !phy->ops->set_media) + return 0; + + mutex_lock(&phy->mutex); + ret = phy->ops->set_media(phy, media); + mutex_unlock(&phy->mutex); + + return ret; +} +EXPORT_SYMBOL_GPL(phy_set_media); + +int phy_set_speed(struct phy *phy, int speed) +{ + int ret; + + if (!phy || !phy->ops->set_speed) + return 0; + + mutex_lock(&phy->mutex); + ret = phy->ops->set_speed(phy, speed); + mutex_unlock(&phy->mutex); + + return ret; +} +EXPORT_SYMBOL_GPL(phy_set_speed); + int phy_reset(struct phy *phy) { int ret; diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h index e435bdb0bab3..e4fd69a1faa7 100644 --- a/include/linux/phy/phy.h +++ b/include/linux/phy/phy.h @@ -44,6 +44,12 @@ enum phy_mode { PHY_MODE_DP }; +enum phy_media { + PHY_MEDIA_DEFAULT, + PHY_MEDIA_SR, + PHY_MEDIA_DAC, +}; + /** * union phy_configure_opts - Opaque generic phy configuration * @@ -64,6 +70,8 @@ union phy_configure_opts { * @power_on: powering on the phy * @power_off: powering off the phy * @set_mode: set the mode of the phy + * @set_media: set the media type of the phy (optional) + * @set_speed: set the speed of the phy (optional) * @reset: resetting the phy * @calibrate: calibrate the phy * @release: ops to be performed while the consumer relinquishes the PHY @@ -75,6 +83,8 @@ struct phy_ops { int (*power_on)(struct phy *phy); int (*power_off)(struct phy *phy); int (*set_mode)(struct phy *phy, enum phy_mode mode, int submode); + int (*set_media)(struct phy *phy, enum phy_media media); + int (*set_speed)(struct phy *phy, int speed); /** * @configure: @@ -215,6 +225,8 @@ int phy_power_off(struct phy *phy); int phy_set_mode_ext(struct phy *phy, enum phy_mode mode, int submode); #define phy_set_mode(phy, mode) \ phy_set_mode_ext(phy, mode, 0) +int phy_set_media(struct phy *phy, enum phy_media media); +int phy_set_speed(struct phy *phy, int speed); int phy_configure(struct phy *phy, union phy_configure_opts *opts); int phy_validate(struct phy *phy, enum phy_mode mode, int submode, union phy_configure_opts *opts); @@ -344,6 +356,20 @@ static inline int phy_set_mode_ext(struct phy *phy, enum phy_mode mode, #define phy_set_mode(phy, mode) \ phy_set_mode_ext(phy, mode, 0) +static inline int phy_set_media(struct phy *phy, enum phy_media media) +{ + if (!phy) + return 0; + return -ENOSYS; +} + +static inline int phy_set_speed(struct phy *phy, int speed) +{ + if (!phy) + return 0; + return -ENOSYS; +} + static inline enum phy_mode phy_get_mode(struct phy *phy) { return PHY_MODE_INVALID;