linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/5] phy: core: rework phy_set_mode to accept phy mode and submode
@ 2018-11-20  1:24 Grygorii Strashko
  2018-11-20  1:24 ` [PATCH v3 1/5] " Grygorii Strashko
                   ` (6 more replies)
  0 siblings, 7 replies; 14+ messages in thread
From: Grygorii Strashko @ 2018-11-20  1:24 UTC (permalink / raw)
  To: David S. Miller, Kishon Vijay Abraham I, Antoine Tenart,
	Quentin Schulz, Russell King - ARM Linux, Maxime Chevallier
  Cc: netdev, Sekhar Nori, linux-kernel, linux-arm-kernel,
	Tony Lindgren, linux-amlogic, linux-mediatek, Alexandre Belloni,
	Vivek Gautam, Maxime Ripard, Chen-Yu Tsai, Carlo Caione,
	Chunfeng Yun, Matthias Brugger, Manu Gautam, Grygorii Strashko

Hi Kishon, All,

Thank you for your review.
I've not added "Tested-by"/"Acked-by" tags due to code changes in v3.

As was discussed in [1] I'm posting series which introduces rework of
phy_set_mode to accept phy mode and submode. I've dropped TI specific patches as
this change is pretty big by itself.

Patch 1 is cumulative change which refactors PHY framework code to
support dual level PHYs mode configuration - PHY mode and PHY submode. It
extends .set_mode() callback to support additional parameter "int submode"
and converts all corresponding PHY drivers to support new .set_mode()
callback declaration.
The new extended PHY API
 int phy_set_mode_ext(struct phy *phy, enum phy_mode mode, int submode)
is introduced to support dual level PHYs mode configuration and existing
phy_set_mode() API is converted to macros, so PHY framework consumers do
not need to be changed (~21 matches).

Patches 2-4: Add new PHY's mode to be used by Ethernet PHY interface drivers or
multipurpose PHYs like serdes and convert ocelot-serdes and mvebu-cp110-comphy
PHY drivers to use recently introduced PHY_MODE_ETHERNET and phy_set_mode_ext().

Patch 5 - removes unused, ethernet specific phy modes from enum phy_mode.

Testing:
 - series tested on TI am335x/am437x/am5(dra7) paltforms.
 - other driver build tested.

changes in v3:
- mux tables for PHY ocelot-serdes driver updated to store PHY mode and submode
- mux tables for PHY mvebu-cp110-comphy driver updated to store PHY mode
  and submode
changes in v2:
- marvell PHY and net drivers updated as recommended by Russell King

v2: https://lkml.org/lkml/2018/11/10/220
v1: https://lkml.org/lkml/2018/11/8/260

[1] https://lkml.org/lkml/2018/10/25/366

Grygorii Strashko (5):
  phy: core: rework phy_set_mode to accept phy mode and submode
  phy: core: add PHY_MODE_ETHERNET
  phy: ocelot-serdes: convert to use eth phy mode and submode
  phy: mvebu-cp110-comphy: convert to use eth phy mode and submode
  phy: core: clean up unused ethernet specific phy modes

 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 19 +----
 drivers/net/ethernet/mscc/ocelot.c              |  9 +--
 drivers/phy/allwinner/phy-sun4i-usb.c           |  3 +-
 drivers/phy/amlogic/phy-meson-gxl-usb2.c        |  5 +-
 drivers/phy/amlogic/phy-meson-gxl-usb3.c        |  5 +-
 drivers/phy/marvell/phy-mvebu-cp110-comphy.c    | 93 ++++++++++++++-----------
 drivers/phy/mediatek/phy-mtk-tphy.c             |  2 +-
 drivers/phy/mediatek/phy-mtk-xsphy.c            |  2 +-
 drivers/phy/mscc/phy-ocelot-serdes.c            | 24 +++++--
 drivers/phy/phy-core.c                          |  6 +-
 drivers/phy/qualcomm/phy-qcom-qmp.c             |  3 +-
 drivers/phy/qualcomm/phy-qcom-qusb2.c           |  3 +-
 drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c    |  3 +-
 drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c    |  3 +-
 drivers/phy/qualcomm/phy-qcom-usb-hs.c          |  3 +-
 drivers/phy/ti/phy-da8xx-usb.c                  |  3 +-
 drivers/phy/ti/phy-tusb1210.c                   |  2 +-
 include/linux/phy/phy.h                         | 18 +++--
 18 files changed, 111 insertions(+), 95 deletions(-)

-- 
2.10.5


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 1/5] phy: core: rework phy_set_mode to accept phy mode and submode
  2018-11-20  1:24 [PATCH v3 0/5] phy: core: rework phy_set_mode to accept phy mode and submode Grygorii Strashko
@ 2018-11-20  1:24 ` Grygorii Strashko
  2018-11-20  1:24 ` [PATCH v3 2/5] phy: core: add PHY_MODE_ETHERNET Grygorii Strashko
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Grygorii Strashko @ 2018-11-20  1:24 UTC (permalink / raw)
  To: David S. Miller, Kishon Vijay Abraham I, Antoine Tenart,
	Quentin Schulz, Russell King - ARM Linux, Maxime Chevallier
  Cc: netdev, Sekhar Nori, linux-kernel, linux-arm-kernel,
	Tony Lindgren, linux-amlogic, linux-mediatek, Alexandre Belloni,
	Vivek Gautam, Maxime Ripard, Chen-Yu Tsai, Carlo Caione,
	Chunfeng Yun, Matthias Brugger, Manu Gautam, Grygorii Strashko

Currently the attempt to add support for Ethernet interface mode PHY
(MII/GMII/RGMII) will lead to the necessity of extending enum phy_mode and
duplicate there values from phy_interface_t enum (or introduce more PHY
callbacks) [1]. Both approaches are ineffective and would lead to fast
bloating of enum phy_mode or struct phy_ops in the process of adding more
PHYs for different subsystems which will make them unmaintainable.

As discussed in [1] the solution could be to introduce dual level PHYs mode
configuration - PHY mode and PHY submode. The PHY mode will define generic
PHY type (subsystem - PCIE/ETHERNET/USB_) while the PHY submode - subsystem
specific interface mode. The last is usually already defined in
corresponding subsystem headers (phy_interface_t for Ethernet, enum
usb_device_speed for USB).

This patch is cumulative change which refactors PHY framework code to
support dual level PHYs mode configuration - PHY mode and PHY submode. It
extends .set_mode() callback to support additional parameter "int submode"
and converts all corresponding PHY drivers to support new .set_mode()
callback declaration.
The new extended PHY API
 int phy_set_mode_ext(struct phy *phy, enum phy_mode mode, int submode)
is introduced to support dual level PHYs mode configuration and existing
phy_set_mode() API is converted to macros, so PHY framework consumers do
not need to be changed (~21 matches).

[1] https://lkml.org/lkml/2018/10/25/366
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/phy/allwinner/phy-sun4i-usb.c        |  3 ++-
 drivers/phy/amlogic/phy-meson-gxl-usb2.c     |  5 +++--
 drivers/phy/amlogic/phy-meson-gxl-usb3.c     |  5 +++--
 drivers/phy/marvell/phy-mvebu-cp110-comphy.c |  3 ++-
 drivers/phy/mediatek/phy-mtk-tphy.c          |  2 +-
 drivers/phy/mediatek/phy-mtk-xsphy.c         |  2 +-
 drivers/phy/mscc/phy-ocelot-serdes.c         |  2 +-
 drivers/phy/phy-core.c                       |  6 +++---
 drivers/phy/qualcomm/phy-qcom-qmp.c          |  3 ++-
 drivers/phy/qualcomm/phy-qcom-qusb2.c        |  3 ++-
 drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c |  3 ++-
 drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c |  3 ++-
 drivers/phy/qualcomm/phy-qcom-usb-hs.c       |  3 ++-
 drivers/phy/ti/phy-da8xx-usb.c               |  3 ++-
 drivers/phy/ti/phy-tusb1210.c                |  2 +-
 include/linux/phy/phy.h                      | 13 ++++++++++---
 16 files changed, 39 insertions(+), 22 deletions(-)

diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c
index d4dcd39..1acdd73 100644
--- a/drivers/phy/allwinner/phy-sun4i-usb.c
+++ b/drivers/phy/allwinner/phy-sun4i-usb.c
@@ -474,7 +474,8 @@ static int sun4i_usb_phy_power_off(struct phy *_phy)
 	return 0;
 }
 
-static int sun4i_usb_phy_set_mode(struct phy *_phy, enum phy_mode mode)
+static int sun4i_usb_phy_set_mode(struct phy *_phy,
+				  enum phy_mode mode, int submode)
 {
 	struct sun4i_usb_phy *phy = phy_get_drvdata(_phy);
 	struct sun4i_usb_phy_data *data = to_sun4i_usb_phy_data(phy);
diff --git a/drivers/phy/amlogic/phy-meson-gxl-usb2.c b/drivers/phy/amlogic/phy-meson-gxl-usb2.c
index 9f9b541..148ef0b 100644
--- a/drivers/phy/amlogic/phy-meson-gxl-usb2.c
+++ b/drivers/phy/amlogic/phy-meson-gxl-usb2.c
@@ -152,7 +152,8 @@ static int phy_meson_gxl_usb2_reset(struct phy *phy)
 	return 0;
 }
 
-static int phy_meson_gxl_usb2_set_mode(struct phy *phy, enum phy_mode mode)
+static int phy_meson_gxl_usb2_set_mode(struct phy *phy,
+				       enum phy_mode mode, int submode)
 {
 	struct phy_meson_gxl_usb2_priv *priv = phy_get_drvdata(phy);
 
@@ -209,7 +210,7 @@ static int phy_meson_gxl_usb2_power_on(struct phy *phy)
 	/* power on the PHY by taking it out of reset mode */
 	regmap_update_bits(priv->regmap, U2P_R0, U2P_R0_POWER_ON_RESET, 0);
 
-	ret = phy_meson_gxl_usb2_set_mode(phy, priv->mode);
+	ret = phy_meson_gxl_usb2_set_mode(phy, priv->mode, 0);
 	if (ret) {
 		phy_meson_gxl_usb2_power_off(phy);
 
diff --git a/drivers/phy/amlogic/phy-meson-gxl-usb3.c b/drivers/phy/amlogic/phy-meson-gxl-usb3.c
index d37d94d..c0e9e4c 100644
--- a/drivers/phy/amlogic/phy-meson-gxl-usb3.c
+++ b/drivers/phy/amlogic/phy-meson-gxl-usb3.c
@@ -119,7 +119,8 @@ static int phy_meson_gxl_usb3_power_off(struct phy *phy)
 	return 0;
 }
 
-static int phy_meson_gxl_usb3_set_mode(struct phy *phy, enum phy_mode mode)
+static int phy_meson_gxl_usb3_set_mode(struct phy *phy,
+				       enum phy_mode mode, int submode)
 {
 	struct phy_meson_gxl_usb3_priv *priv = phy_get_drvdata(phy);
 
@@ -164,7 +165,7 @@ static int phy_meson_gxl_usb3_init(struct phy *phy)
 	if (ret)
 		goto err_disable_clk_phy;
 
-	ret = phy_meson_gxl_usb3_set_mode(phy, priv->mode);
+	ret = phy_meson_gxl_usb3_set_mode(phy, priv->mode, 0);
 	if (ret)
 		goto err_disable_clk_peripheral;
 
diff --git a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
index 86a5f7b..79b52c3 100644
--- a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
+++ b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
@@ -512,7 +512,8 @@ static int mvebu_comphy_power_on(struct phy *phy)
 	return ret;
 }
 
-static int mvebu_comphy_set_mode(struct phy *phy, enum phy_mode mode)
+static int mvebu_comphy_set_mode(struct phy *phy,
+				 enum phy_mode mode, int submode)
 {
 	struct mvebu_comphy_lane *lane = phy_get_drvdata(phy);
 
diff --git a/drivers/phy/mediatek/phy-mtk-tphy.c b/drivers/phy/mediatek/phy-mtk-tphy.c
index 3eb8e1b..5b6a470 100644
--- a/drivers/phy/mediatek/phy-mtk-tphy.c
+++ b/drivers/phy/mediatek/phy-mtk-tphy.c
@@ -971,7 +971,7 @@ static int mtk_phy_exit(struct phy *phy)
 	return 0;
 }
 
-static int mtk_phy_set_mode(struct phy *phy, enum phy_mode mode)
+static int mtk_phy_set_mode(struct phy *phy, enum phy_mode mode, int submode)
 {
 	struct mtk_phy_instance *instance = phy_get_drvdata(phy);
 	struct mtk_tphy *tphy = dev_get_drvdata(phy->dev.parent);
diff --git a/drivers/phy/mediatek/phy-mtk-xsphy.c b/drivers/phy/mediatek/phy-mtk-xsphy.c
index 020cd02..8c51131 100644
--- a/drivers/phy/mediatek/phy-mtk-xsphy.c
+++ b/drivers/phy/mediatek/phy-mtk-xsphy.c
@@ -426,7 +426,7 @@ static int mtk_phy_exit(struct phy *phy)
 	return 0;
 }
 
-static int mtk_phy_set_mode(struct phy *phy, enum phy_mode mode)
+static int mtk_phy_set_mode(struct phy *phy, enum phy_mode mode, int submode)
 {
 	struct xsphy_instance *inst = phy_get_drvdata(phy);
 	struct mtk_xsphy *xsphy = dev_get_drvdata(phy->dev.parent);
diff --git a/drivers/phy/mscc/phy-ocelot-serdes.c b/drivers/phy/mscc/phy-ocelot-serdes.c
index cbb49d9..c61a9890 100644
--- a/drivers/phy/mscc/phy-ocelot-serdes.c
+++ b/drivers/phy/mscc/phy-ocelot-serdes.c
@@ -158,7 +158,7 @@ static const struct serdes_mux ocelot_serdes_muxes[] = {
 		   HSIO_HW_CFG_PCIE_ENA),
 };
 
-static int serdes_set_mode(struct phy *phy, enum phy_mode mode)
+static int serdes_set_mode(struct phy *phy, enum phy_mode mode, int submode)
 {
 	struct serdes_macro *macro = phy_get_drvdata(phy);
 	unsigned int i;
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 35fd38c..df3d4ba 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -360,7 +360,7 @@ int phy_power_off(struct phy *phy)
 }
 EXPORT_SYMBOL_GPL(phy_power_off);
 
-int phy_set_mode(struct phy *phy, enum phy_mode mode)
+int phy_set_mode_ext(struct phy *phy, enum phy_mode mode, int submode)
 {
 	int ret;
 
@@ -368,14 +368,14 @@ int phy_set_mode(struct phy *phy, enum phy_mode mode)
 		return 0;
 
 	mutex_lock(&phy->mutex);
-	ret = phy->ops->set_mode(phy, mode);
+	ret = phy->ops->set_mode(phy, mode, submode);
 	if (!ret)
 		phy->attrs.mode = mode;
 	mutex_unlock(&phy->mutex);
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(phy_set_mode);
+EXPORT_SYMBOL_GPL(phy_set_mode_ext);
 
 int phy_reset(struct phy *phy)
 {
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp.c b/drivers/phy/qualcomm/phy-qcom-qmp.c
index a833324..514db72 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp.c
@@ -1365,7 +1365,8 @@ static int qcom_qmp_phy_poweron(struct phy *phy)
 	return ret;
 }
 
-static int qcom_qmp_phy_set_mode(struct phy *phy, enum phy_mode mode)
+static int qcom_qmp_phy_set_mode(struct phy *phy,
+				 enum phy_mode mode, int submode)
 {
 	struct qmp_phy *qphy = phy_get_drvdata(phy);
 	struct qcom_qmp *qmp = qphy->qmp;
diff --git a/drivers/phy/qualcomm/phy-qcom-qusb2.c b/drivers/phy/qualcomm/phy-qcom-qusb2.c
index 9ce5311..098d793 100644
--- a/drivers/phy/qualcomm/phy-qcom-qusb2.c
+++ b/drivers/phy/qualcomm/phy-qcom-qusb2.c
@@ -423,7 +423,8 @@ static void qusb2_phy_set_tune2_param(struct qusb2_phy *qphy)
 
 }
 
-static int qusb2_phy_set_mode(struct phy *phy, enum phy_mode mode)
+static int qusb2_phy_set_mode(struct phy *phy,
+			      enum phy_mode mode, int submode)
 {
 	struct qusb2_phy *qphy = phy_get_drvdata(phy);
 
diff --git a/drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c b/drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c
index ba1895b..1e0d4f2 100644
--- a/drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c
+++ b/drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c
@@ -65,7 +65,8 @@ static int ufs_qcom_phy_qmp_14nm_exit(struct phy *generic_phy)
 }
 
 static
-int ufs_qcom_phy_qmp_14nm_set_mode(struct phy *generic_phy, enum phy_mode mode)
+int ufs_qcom_phy_qmp_14nm_set_mode(struct phy *generic_phy,
+				   enum phy_mode mode, int submode)
 {
 	struct ufs_qcom_phy *phy_common = get_ufs_qcom_phy(generic_phy);
 
diff --git a/drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c b/drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c
index 49f435c..aef40f7 100644
--- a/drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c
+++ b/drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c
@@ -84,7 +84,8 @@ static int ufs_qcom_phy_qmp_20nm_exit(struct phy *generic_phy)
 }
 
 static
-int ufs_qcom_phy_qmp_20nm_set_mode(struct phy *generic_phy, enum phy_mode mode)
+int ufs_qcom_phy_qmp_20nm_set_mode(struct phy *generic_phy,
+				   enum phy_mode mode, int submode)
 {
 	struct ufs_qcom_phy *phy_common = get_ufs_qcom_phy(generic_phy);
 
diff --git a/drivers/phy/qualcomm/phy-qcom-usb-hs.c b/drivers/phy/qualcomm/phy-qcom-usb-hs.c
index abbbe75..04934f8 100644
--- a/drivers/phy/qualcomm/phy-qcom-usb-hs.c
+++ b/drivers/phy/qualcomm/phy-qcom-usb-hs.c
@@ -42,7 +42,8 @@ struct qcom_usb_hs_phy {
 	struct notifier_block vbus_notify;
 };
 
-static int qcom_usb_hs_phy_set_mode(struct phy *phy, enum phy_mode mode)
+static int qcom_usb_hs_phy_set_mode(struct phy *phy,
+				    enum phy_mode mode, int submode)
 {
 	struct qcom_usb_hs_phy *uphy = phy_get_drvdata(phy);
 	u8 addr;
diff --git a/drivers/phy/ti/phy-da8xx-usb.c b/drivers/phy/ti/phy-da8xx-usb.c
index befb886..d5f4fbc 100644
--- a/drivers/phy/ti/phy-da8xx-usb.c
+++ b/drivers/phy/ti/phy-da8xx-usb.c
@@ -93,7 +93,8 @@ static int da8xx_usb20_phy_power_off(struct phy *phy)
 	return 0;
 }
 
-static int da8xx_usb20_phy_set_mode(struct phy *phy, enum phy_mode mode)
+static int da8xx_usb20_phy_set_mode(struct phy *phy,
+				    enum phy_mode mode, int submode)
 {
 	struct da8xx_usb_phy *d_phy = phy_get_drvdata(phy);
 	u32 val;
diff --git a/drivers/phy/ti/phy-tusb1210.c b/drivers/phy/ti/phy-tusb1210.c
index b8ec39a..329fb93 100644
--- a/drivers/phy/ti/phy-tusb1210.c
+++ b/drivers/phy/ti/phy-tusb1210.c
@@ -53,7 +53,7 @@ static int tusb1210_power_off(struct phy *phy)
 	return 0;
 }
 
-static int tusb1210_set_mode(struct phy *phy, enum phy_mode mode)
+static int tusb1210_set_mode(struct phy *phy, enum phy_mode mode, int submode)
 {
 	struct tusb1210 *tusb = phy_get_drvdata(phy);
 	int ret;
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 03b319f..b17e770 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -60,7 +60,7 @@ struct phy_ops {
 	int	(*exit)(struct phy *phy);
 	int	(*power_on)(struct phy *phy);
 	int	(*power_off)(struct phy *phy);
-	int	(*set_mode)(struct phy *phy, enum phy_mode mode);
+	int	(*set_mode)(struct phy *phy, enum phy_mode mode, int submode);
 	int	(*reset)(struct phy *phy);
 	int	(*calibrate)(struct phy *phy);
 	struct module *owner;
@@ -164,7 +164,10 @@ int phy_init(struct phy *phy);
 int phy_exit(struct phy *phy);
 int phy_power_on(struct phy *phy);
 int phy_power_off(struct phy *phy);
-int phy_set_mode(struct phy *phy, enum phy_mode mode);
+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)
+
 static inline enum phy_mode phy_get_mode(struct phy *phy)
 {
 	return phy->attrs.mode;
@@ -278,13 +281,17 @@ static inline int phy_power_off(struct phy *phy)
 	return -ENOSYS;
 }
 
-static inline int phy_set_mode(struct phy *phy, enum phy_mode mode)
+static inline int phy_set_mode_ext(struct phy *phy, enum phy_mode mode,
+				   int submode)
 {
 	if (!phy)
 		return 0;
 	return -ENOSYS;
 }
 
+#define phy_set_mode(phy, mode) \
+	phy_set_mode_ext(phy, mode, 0)
+
 static inline enum phy_mode phy_get_mode(struct phy *phy)
 {
 	return PHY_MODE_INVALID;
-- 
2.10.5


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 2/5] phy: core: add PHY_MODE_ETHERNET
  2018-11-20  1:24 [PATCH v3 0/5] phy: core: rework phy_set_mode to accept phy mode and submode Grygorii Strashko
  2018-11-20  1:24 ` [PATCH v3 1/5] " Grygorii Strashko
@ 2018-11-20  1:24 ` Grygorii Strashko
  2018-11-20  1:24 ` [PATCH v3 3/5] phy: ocelot-serdes: convert to use eth phy mode and submode Grygorii Strashko
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Grygorii Strashko @ 2018-11-20  1:24 UTC (permalink / raw)
  To: David S. Miller, Kishon Vijay Abraham I, Antoine Tenart,
	Quentin Schulz, Russell King - ARM Linux, Maxime Chevallier
  Cc: netdev, Sekhar Nori, linux-kernel, linux-arm-kernel,
	Tony Lindgren, linux-amlogic, linux-mediatek, Alexandre Belloni,
	Vivek Gautam, Maxime Ripard, Chen-Yu Tsai, Carlo Caione,
	Chunfeng Yun, Matthias Brugger, Manu Gautam, Grygorii Strashko

Add new PHY's mode to be used by Ethernet PHY interface drivers or
multipurpose PHYs like serdes. It will be reused in further changes.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 include/linux/phy/phy.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index b17e770..02c9ef0 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -42,6 +42,7 @@ enum phy_mode {
 	PHY_MODE_UFS_HS_A,
 	PHY_MODE_UFS_HS_B,
 	PHY_MODE_PCIE,
+	PHY_MODE_ETHERNET,
 };
 
 /**
-- 
2.10.5


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 3/5] phy: ocelot-serdes: convert to use eth phy mode and submode
  2018-11-20  1:24 [PATCH v3 0/5] phy: core: rework phy_set_mode to accept phy mode and submode Grygorii Strashko
  2018-11-20  1:24 ` [PATCH v3 1/5] " Grygorii Strashko
  2018-11-20  1:24 ` [PATCH v3 2/5] phy: core: add PHY_MODE_ETHERNET Grygorii Strashko
@ 2018-11-20  1:24 ` Grygorii Strashko
  2018-11-20  9:18   ` Quentin Schulz
  2018-11-20  1:24 ` [PATCH v3 4/5] phy: mvebu-cp110-comphy: " Grygorii Strashko
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Grygorii Strashko @ 2018-11-20  1:24 UTC (permalink / raw)
  To: David S. Miller, Kishon Vijay Abraham I, Antoine Tenart,
	Quentin Schulz, Russell King - ARM Linux, Maxime Chevallier
  Cc: netdev, Sekhar Nori, linux-kernel, linux-arm-kernel,
	Tony Lindgren, linux-amlogic, linux-mediatek, Alexandre Belloni,
	Vivek Gautam, Maxime Ripard, Chen-Yu Tsai, Carlo Caione,
	Chunfeng Yun, Matthias Brugger, Manu Gautam, Grygorii Strashko

Convert ocelot-serdes PHY driver to use recently introduced
PHY_MODE_ETHERNET and phy_set_mode_ext().

Cc: Quentin Schulz <quentin.schulz@bootlin.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/net/ethernet/mscc/ocelot.c   |  9 ++-------
 drivers/phy/mscc/phy-ocelot-serdes.c | 22 ++++++++++++++++------
 2 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index 3238b9e..3edb608 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -472,7 +472,6 @@ static int ocelot_port_open(struct net_device *dev)
 {
 	struct ocelot_port *port = netdev_priv(dev);
 	struct ocelot *ocelot = port->ocelot;
-	enum phy_mode phy_mode;
 	int err;
 
 	/* Enable receiving frames on the port, and activate auto-learning of
@@ -484,12 +483,8 @@ static int ocelot_port_open(struct net_device *dev)
 			 ANA_PORT_PORT_CFG, port->chip_port);
 
 	if (port->serdes) {
-		if (port->phy_mode == PHY_INTERFACE_MODE_SGMII)
-			phy_mode = PHY_MODE_SGMII;
-		else
-			phy_mode = PHY_MODE_QSGMII;
-
-		err = phy_set_mode(port->serdes, phy_mode);
+		err = phy_set_mode_ext(port->serdes, PHY_MODE_ETHERNET,
+				       port->phy_mode);
 		if (err) {
 			netdev_err(dev, "Could not set mode of SerDes\n");
 			return err;
diff --git a/drivers/phy/mscc/phy-ocelot-serdes.c b/drivers/phy/mscc/phy-ocelot-serdes.c
index c61a9890..77c46f6 100644
--- a/drivers/phy/mscc/phy-ocelot-serdes.c
+++ b/drivers/phy/mscc/phy-ocelot-serdes.c
@@ -11,6 +11,7 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
+#include <linux/phy.h>
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
@@ -104,20 +105,24 @@ struct serdes_mux {
 	u8			idx;
 	u8			port;
 	enum phy_mode		mode;
+	int			submode;
 	u32			mask;
 	u32			mux;
 };
 
-#define SERDES_MUX(_idx, _port, _mode, _mask, _mux) {		\
+#define SERDES_MUX(_idx, _port, _mode, _submode, _mask, _mux) {		\
 	.idx = _idx,						\
 	.port = _port,						\
 	.mode = _mode,						\
+	.submode = _submode,					\
 	.mask = _mask,						\
 	.mux = _mux,						\
 }
 
-#define SERDES_MUX_SGMII(i, p, m, c) SERDES_MUX(i, p, PHY_MODE_SGMII, m, c)
-#define SERDES_MUX_QSGMII(i, p, m, c) SERDES_MUX(i, p, PHY_MODE_QSGMII, m, c)
+#define SERDES_MUX_SGMII(i, p, m, c) \
+	SERDES_MUX(i, p, PHY_MODE_ETHERNET, PHY_INTERFACE_MODE_SGMII, m, c)
+#define SERDES_MUX_QSGMII(i, p, m, c) \
+	SERDES_MUX(i, p, PHY_MODE_ETHERNET, PHY_INTERFACE_MODE_QSGMII, m, c)
 
 static const struct serdes_mux ocelot_serdes_muxes[] = {
 	SERDES_MUX_SGMII(SERDES1G(0), 0, 0, 0),
@@ -154,7 +159,7 @@ static const struct serdes_mux ocelot_serdes_muxes[] = {
 	SERDES_MUX_SGMII(SERDES6G(1), 8, 0, 0),
 	SERDES_MUX_SGMII(SERDES6G(2), 10, HSIO_HW_CFG_PCIE_ENA |
 			 HSIO_HW_CFG_DEV2G5_10_MODE, 0),
-	SERDES_MUX(SERDES6G(2), 10, PHY_MODE_PCIE, HSIO_HW_CFG_PCIE_ENA,
+	SERDES_MUX(SERDES6G(2), 10, PHY_MODE_PCIE, 0, HSIO_HW_CFG_PCIE_ENA,
 		   HSIO_HW_CFG_PCIE_ENA),
 };
 
@@ -164,12 +169,17 @@ static int serdes_set_mode(struct phy *phy, enum phy_mode mode, int submode)
 	unsigned int i;
 	int ret;
 
+	/* As of now only PHY_MODE_ETHERNET is supported */
+	if (mode != PHY_MODE_ETHERNET)
+		return -EOPNOTSUPP;
+
 	for (i = 0; i < ARRAY_SIZE(ocelot_serdes_muxes); i++) {
 		if (macro->idx != ocelot_serdes_muxes[i].idx ||
-		    mode != ocelot_serdes_muxes[i].mode)
+		    mode != ocelot_serdes_muxes[i].mode ||
+		    submode != ocelot_serdes_muxes[i].submode)
 			continue;
 
-		if (mode != PHY_MODE_QSGMII &&
+		if (submode != PHY_INTERFACE_MODE_QSGMII &&
 		    macro->port != ocelot_serdes_muxes[i].port)
 			continue;
 
-- 
2.10.5


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 4/5] phy: mvebu-cp110-comphy: convert to use eth phy mode and submode
  2018-11-20  1:24 [PATCH v3 0/5] phy: core: rework phy_set_mode to accept phy mode and submode Grygorii Strashko
                   ` (2 preceding siblings ...)
  2018-11-20  1:24 ` [PATCH v3 3/5] phy: ocelot-serdes: convert to use eth phy mode and submode Grygorii Strashko
@ 2018-11-20  1:24 ` Grygorii Strashko
  2018-11-21  7:38   ` Kishon Vijay Abraham I
  2018-11-21  8:02   ` Antoine Tenart
  2018-11-20  1:24 ` [PATCH v3 5/5] phy: core: clean up unused ethernet specific phy modes Grygorii Strashko
                   ` (2 subsequent siblings)
  6 siblings, 2 replies; 14+ messages in thread
From: Grygorii Strashko @ 2018-11-20  1:24 UTC (permalink / raw)
  To: David S. Miller, Kishon Vijay Abraham I, Antoine Tenart,
	Quentin Schulz, Russell King - ARM Linux, Maxime Chevallier
  Cc: netdev, Sekhar Nori, linux-kernel, linux-arm-kernel,
	Tony Lindgren, linux-amlogic, linux-mediatek, Alexandre Belloni,
	Vivek Gautam, Maxime Ripard, Chen-Yu Tsai, Carlo Caione,
	Chunfeng Yun, Matthias Brugger, Manu Gautam, Grygorii Strashko

Convert mvebu-cp110-comphy PHY driver to use recently introduced
PHY_MODE_ETHERNET and phy_set_mode_ext().

Cc: Russell King - ARM Linux <linux@armlinux.org.uk>
Cc: Maxime Chevallier <maxime.chevallier@bootlin.com>
Cc: Antoine Tenart <antoine.tenart@free-electrons.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 19 +-----
 drivers/phy/marvell/phy-mvebu-cp110-comphy.c    | 90 ++++++++++++++-----------
 2 files changed, 53 insertions(+), 56 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 7a37a37..731793a 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -1165,28 +1165,13 @@ static void mvpp22_gop_setup_irq(struct mvpp2_port *port)
  */
 static int mvpp22_comphy_init(struct mvpp2_port *port)
 {
-	enum phy_mode mode;
 	int ret;
 
 	if (!port->comphy)
 		return 0;
 
-	switch (port->phy_interface) {
-	case PHY_INTERFACE_MODE_SGMII:
-	case PHY_INTERFACE_MODE_1000BASEX:
-		mode = PHY_MODE_SGMII;
-		break;
-	case PHY_INTERFACE_MODE_2500BASEX:
-		mode = PHY_MODE_2500SGMII;
-		break;
-	case PHY_INTERFACE_MODE_10GKR:
-		mode = PHY_MODE_10GKR;
-		break;
-	default:
-		return -EINVAL;
-	}
-
-	ret = phy_set_mode(port->comphy, mode);
+	ret = phy_set_mode_ext(port->comphy, PHY_MODE_ETHERNET,
+			       port->phy_interface);
 	if (ret)
 		return ret;
 
diff --git a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
index 79b52c3..2b4462a 100644
--- a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
+++ b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
@@ -9,6 +9,7 @@
 #include <linux/iopoll.h>
 #include <linux/mfd/syscon.h>
 #include <linux/module.h>
+#include <linux/phy.h>
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
@@ -116,41 +117,43 @@
 
 struct mvebu_comhy_conf {
 	enum phy_mode mode;
+	int submode;
 	unsigned lane;
 	unsigned port;
 	u32 mux;
 };
 
-#define MVEBU_COMPHY_CONF(_lane, _port, _mode, _mux)	\
+#define MVEBU_COMPHY_CONF(_lane, _port, _submode, _mux)	\
 	{						\
 		.lane = _lane,				\
 		.port = _port,				\
-		.mode = _mode,				\
+		.mode = PHY_MODE_ETHERNET,		\
+		.submode = _submode,			\
 		.mux = _mux,				\
 	}
 
 static const struct mvebu_comhy_conf mvebu_comphy_cp110_modes[] = {
 	/* lane 0 */
-	MVEBU_COMPHY_CONF(0, 1, PHY_MODE_SGMII, 0x1),
-	MVEBU_COMPHY_CONF(0, 1, PHY_MODE_2500SGMII, 0x1),
+	MVEBU_COMPHY_CONF(0, 1, PHY_INTERFACE_MODE_SGMII, 0x1),
+	MVEBU_COMPHY_CONF(0, 1, PHY_INTERFACE_MODE_2500BASEX, 0x1),
 	/* lane 1 */
-	MVEBU_COMPHY_CONF(1, 2, PHY_MODE_SGMII, 0x1),
-	MVEBU_COMPHY_CONF(1, 2, PHY_MODE_2500SGMII, 0x1),
+	MVEBU_COMPHY_CONF(1, 2, PHY_INTERFACE_MODE_SGMII, 0x1),
+	MVEBU_COMPHY_CONF(1, 2, PHY_INTERFACE_MODE_2500BASEX, 0x1),
 	/* lane 2 */
-	MVEBU_COMPHY_CONF(2, 0, PHY_MODE_SGMII, 0x1),
-	MVEBU_COMPHY_CONF(2, 0, PHY_MODE_2500SGMII, 0x1),
-	MVEBU_COMPHY_CONF(2, 0, PHY_MODE_10GKR, 0x1),
+	MVEBU_COMPHY_CONF(2, 0, PHY_INTERFACE_MODE_SGMII, 0x1),
+	MVEBU_COMPHY_CONF(2, 0, PHY_INTERFACE_MODE_2500BASEX, 0x1),
+	MVEBU_COMPHY_CONF(2, 0, PHY_INTERFACE_MODE_10GKR, 0x1),
 	/* lane 3 */
-	MVEBU_COMPHY_CONF(3, 1, PHY_MODE_SGMII, 0x2),
-	MVEBU_COMPHY_CONF(3, 1, PHY_MODE_2500SGMII, 0x2),
+	MVEBU_COMPHY_CONF(3, 1, PHY_INTERFACE_MODE_SGMII, 0x2),
+	MVEBU_COMPHY_CONF(3, 1, PHY_INTERFACE_MODE_2500BASEX, 0x2),
 	/* lane 4 */
-	MVEBU_COMPHY_CONF(4, 0, PHY_MODE_SGMII, 0x2),
-	MVEBU_COMPHY_CONF(4, 0, PHY_MODE_2500SGMII, 0x2),
-	MVEBU_COMPHY_CONF(4, 0, PHY_MODE_10GKR, 0x2),
-	MVEBU_COMPHY_CONF(4, 1, PHY_MODE_SGMII, 0x1),
+	MVEBU_COMPHY_CONF(4, 0, PHY_INTERFACE_MODE_SGMII, 0x2),
+	MVEBU_COMPHY_CONF(4, 0, PHY_INTERFACE_MODE_2500BASEX, 0x2),
+	MVEBU_COMPHY_CONF(4, 0, PHY_INTERFACE_MODE_10GKR, 0x2),
+	MVEBU_COMPHY_CONF(4, 1, PHY_INTERFACE_MODE_SGMII, 0x1),
 	/* lane 5 */
-	MVEBU_COMPHY_CONF(5, 2, PHY_MODE_SGMII, 0x1),
-	MVEBU_COMPHY_CONF(5, 2, PHY_MODE_2500SGMII, 0x1),
+	MVEBU_COMPHY_CONF(5, 2, PHY_INTERFACE_MODE_SGMII, 0x1),
+	MVEBU_COMPHY_CONF(5, 2, PHY_INTERFACE_MODE_2500BASEX, 0x1),
 };
 
 struct mvebu_comphy_priv {
@@ -163,10 +166,12 @@ struct mvebu_comphy_lane {
 	struct mvebu_comphy_priv *priv;
 	unsigned id;
 	enum phy_mode mode;
+	int submode;
 	int port;
 };
 
-static int mvebu_comphy_get_mux(int lane, int port, enum phy_mode mode)
+static int mvebu_comphy_get_mux(int lane, int port,
+				enum phy_mode mode, int submode)
 {
 	int i, n = ARRAY_SIZE(mvebu_comphy_cp110_modes);
 
@@ -177,7 +182,8 @@ static int mvebu_comphy_get_mux(int lane, int port, enum phy_mode mode)
 	for (i = 0; i < n; i++) {
 		if (mvebu_comphy_cp110_modes[i].lane == lane &&
 		    mvebu_comphy_cp110_modes[i].port == port &&
-		    mvebu_comphy_cp110_modes[i].mode == mode)
+		    mvebu_comphy_cp110_modes[i].mode == mode &&
+		    mvebu_comphy_cp110_modes[i].submode == submode)
 			break;
 	}
 
@@ -187,8 +193,7 @@ static int mvebu_comphy_get_mux(int lane, int port, enum phy_mode mode)
 	return mvebu_comphy_cp110_modes[i].mux;
 }
 
-static void mvebu_comphy_ethernet_init_reset(struct mvebu_comphy_lane *lane,
-					     enum phy_mode mode)
+static void mvebu_comphy_ethernet_init_reset(struct mvebu_comphy_lane *lane)
 {
 	struct mvebu_comphy_priv *priv = lane->priv;
 	u32 val;
@@ -206,14 +211,14 @@ static void mvebu_comphy_ethernet_init_reset(struct mvebu_comphy_lane *lane,
 		 MVEBU_COMPHY_SERDES_CFG0_HALF_BUS |
 		 MVEBU_COMPHY_SERDES_CFG0_GEN_RX(0xf) |
 		 MVEBU_COMPHY_SERDES_CFG0_GEN_TX(0xf));
-	if (mode == PHY_MODE_10GKR)
+	if (lane->submode == PHY_INTERFACE_MODE_10GKR)
 		val |= MVEBU_COMPHY_SERDES_CFG0_GEN_RX(0xe) |
 		       MVEBU_COMPHY_SERDES_CFG0_GEN_TX(0xe);
-	else if (mode == PHY_MODE_2500SGMII)
+	else if (lane->submode == PHY_INTERFACE_MODE_2500BASEX)
 		val |= MVEBU_COMPHY_SERDES_CFG0_GEN_RX(0x8) |
 		       MVEBU_COMPHY_SERDES_CFG0_GEN_TX(0x8) |
 		       MVEBU_COMPHY_SERDES_CFG0_HALF_BUS;
-	else if (mode == PHY_MODE_SGMII)
+	else if (lane->submode == PHY_INTERFACE_MODE_SGMII)
 		val |= MVEBU_COMPHY_SERDES_CFG0_GEN_RX(0x6) |
 		       MVEBU_COMPHY_SERDES_CFG0_GEN_TX(0x6) |
 		       MVEBU_COMPHY_SERDES_CFG0_HALF_BUS;
@@ -243,7 +248,7 @@ static void mvebu_comphy_ethernet_init_reset(struct mvebu_comphy_lane *lane,
 	/* refclk selection */
 	val = readl(priv->base + MVEBU_COMPHY_MISC_CTRL0(lane->id));
 	val &= ~MVEBU_COMPHY_MISC_CTRL0_REFCLK_SEL;
-	if (mode == PHY_MODE_10GKR)
+	if (lane->submode == PHY_INTERFACE_MODE_10GKR)
 		val |= MVEBU_COMPHY_MISC_CTRL0_ICP_FORCE;
 	writel(val, priv->base + MVEBU_COMPHY_MISC_CTRL0(lane->id));
 
@@ -261,8 +266,7 @@ static void mvebu_comphy_ethernet_init_reset(struct mvebu_comphy_lane *lane,
 	writel(val, priv->base + MVEBU_COMPHY_LOOPBACK(lane->id));
 }
 
-static int mvebu_comphy_init_plls(struct mvebu_comphy_lane *lane,
-				  enum phy_mode mode)
+static int mvebu_comphy_init_plls(struct mvebu_comphy_lane *lane)
 {
 	struct mvebu_comphy_priv *priv = lane->priv;
 	u32 val;
@@ -303,13 +307,13 @@ static int mvebu_comphy_init_plls(struct mvebu_comphy_lane *lane,
 	return 0;
 }
 
-static int mvebu_comphy_set_mode_sgmii(struct phy *phy, enum phy_mode mode)
+static int mvebu_comphy_set_mode_sgmii(struct phy *phy)
 {
 	struct mvebu_comphy_lane *lane = phy_get_drvdata(phy);
 	struct mvebu_comphy_priv *priv = lane->priv;
 	u32 val;
 
-	mvebu_comphy_ethernet_init_reset(lane, mode);
+	mvebu_comphy_ethernet_init_reset(lane);
 
 	val = readl(priv->base + MVEBU_COMPHY_RX_CTRL1(lane->id));
 	val &= ~MVEBU_COMPHY_RX_CTRL1_CLK8T_EN;
@@ -330,7 +334,7 @@ static int mvebu_comphy_set_mode_sgmii(struct phy *phy, enum phy_mode mode)
 	val |= MVEBU_COMPHY_GEN1_S0_TX_EMPH(0x1);
 	writel(val, priv->base + MVEBU_COMPHY_GEN1_S0(lane->id));
 
-	return mvebu_comphy_init_plls(lane, PHY_MODE_SGMII);
+	return mvebu_comphy_init_plls(lane);
 }
 
 static int mvebu_comphy_set_mode_10gkr(struct phy *phy)
@@ -339,7 +343,7 @@ static int mvebu_comphy_set_mode_10gkr(struct phy *phy)
 	struct mvebu_comphy_priv *priv = lane->priv;
 	u32 val;
 
-	mvebu_comphy_ethernet_init_reset(lane, PHY_MODE_10GKR);
+	mvebu_comphy_ethernet_init_reset(lane);
 
 	val = readl(priv->base + MVEBU_COMPHY_RX_CTRL1(lane->id));
 	val |= MVEBU_COMPHY_RX_CTRL1_RXCLK2X_SEL |
@@ -469,7 +473,7 @@ static int mvebu_comphy_set_mode_10gkr(struct phy *phy)
 	val |= MVEBU_COMPHY_EXT_SELV_RX_SAMPL(0x1a);
 	writel(val, priv->base + MVEBU_COMPHY_EXT_SELV(lane->id));
 
-	return mvebu_comphy_init_plls(lane, PHY_MODE_10GKR);
+	return mvebu_comphy_init_plls(lane);
 }
 
 static int mvebu_comphy_power_on(struct phy *phy)
@@ -479,7 +483,8 @@ static int mvebu_comphy_power_on(struct phy *phy)
 	int ret, mux;
 	u32 val;
 
-	mux = mvebu_comphy_get_mux(lane->id, lane->port, lane->mode);
+	mux = mvebu_comphy_get_mux(lane->id, lane->port,
+				   lane->mode, lane->submode);
 	if (mux < 0)
 		return -ENOTSUPP;
 
@@ -492,12 +497,12 @@ static int mvebu_comphy_power_on(struct phy *phy)
 	val |= mux << MVEBU_COMPHY_SELECTOR_PHY(lane->id);
 	regmap_write(priv->regmap, MVEBU_COMPHY_SELECTOR, val);
 
-	switch (lane->mode) {
-	case PHY_MODE_SGMII:
-	case PHY_MODE_2500SGMII:
-		ret = mvebu_comphy_set_mode_sgmii(phy, lane->mode);
+	switch (lane->submode) {
+	case PHY_INTERFACE_MODE_SGMII:
+	case PHY_INTERFACE_MODE_2500BASEX:
+		ret = mvebu_comphy_set_mode_sgmii(phy);
 		break;
-	case PHY_MODE_10GKR:
+	case PHY_INTERFACE_MODE_10GKR:
 		ret = mvebu_comphy_set_mode_10gkr(phy);
 		break;
 	default:
@@ -517,10 +522,17 @@ static int mvebu_comphy_set_mode(struct phy *phy,
 {
 	struct mvebu_comphy_lane *lane = phy_get_drvdata(phy);
 
-	if (mvebu_comphy_get_mux(lane->id, lane->port, mode) < 0)
+	if (mode != PHY_MODE_ETHERNET)
+		return -EINVAL;
+
+	if (submode == PHY_INTERFACE_MODE_1000BASEX)
+		submode = PHY_INTERFACE_MODE_SGMII;
+
+	if (mvebu_comphy_get_mux(lane->id, lane->port, mode, submode) < 0)
 		return -EINVAL;
 
 	lane->mode = mode;
+	lane->submode = submode;
 	return 0;
 }
 
-- 
2.10.5


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 5/5] phy: core: clean up unused ethernet specific phy modes
  2018-11-20  1:24 [PATCH v3 0/5] phy: core: rework phy_set_mode to accept phy mode and submode Grygorii Strashko
                   ` (3 preceding siblings ...)
  2018-11-20  1:24 ` [PATCH v3 4/5] phy: mvebu-cp110-comphy: " Grygorii Strashko
@ 2018-11-20  1:24 ` Grygorii Strashko
  2018-11-21  8:55 ` [PATCH v3 0/5] phy: core: rework phy_set_mode to accept phy mode and submode Kishon Vijay Abraham I
  2018-12-17 14:06 ` Maxime Ripard
  6 siblings, 0 replies; 14+ messages in thread
From: Grygorii Strashko @ 2018-11-20  1:24 UTC (permalink / raw)
  To: David S. Miller, Kishon Vijay Abraham I, Antoine Tenart,
	Quentin Schulz, Russell King - ARM Linux, Maxime Chevallier
  Cc: netdev, Sekhar Nori, linux-kernel, linux-arm-kernel,
	Tony Lindgren, linux-amlogic, linux-mediatek, Alexandre Belloni,
	Vivek Gautam, Maxime Ripard, Chen-Yu Tsai, Carlo Caione,
	Chunfeng Yun, Matthias Brugger, Manu Gautam, Grygorii Strashko

After recent changes PHY_MODE_SGMII, PHY_MODE_2500SGMII, PHY_MODE_QSGMII,
PHY_MODE_10GKR are not used any more and can be removed. Hence - remove
them.

Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 include/linux/phy/phy.h | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 02c9ef0..79da05a 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -35,10 +35,6 @@ enum phy_mode {
 	PHY_MODE_USB_DEVICE_HS,
 	PHY_MODE_USB_DEVICE_SS,
 	PHY_MODE_USB_OTG,
-	PHY_MODE_SGMII,
-	PHY_MODE_2500SGMII,
-	PHY_MODE_QSGMII,
-	PHY_MODE_10GKR,
 	PHY_MODE_UFS_HS_A,
 	PHY_MODE_UFS_HS_B,
 	PHY_MODE_PCIE,
-- 
2.10.5


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 3/5] phy: ocelot-serdes: convert to use eth phy mode and submode
  2018-11-20  1:24 ` [PATCH v3 3/5] phy: ocelot-serdes: convert to use eth phy mode and submode Grygorii Strashko
@ 2018-11-20  9:18   ` Quentin Schulz
  0 siblings, 0 replies; 14+ messages in thread
From: Quentin Schulz @ 2018-11-20  9:18 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: David S. Miller, Kishon Vijay Abraham I, Antoine Tenart,
	Russell King - ARM Linux, Maxime Chevallier, netdev, Sekhar Nori,
	linux-kernel, linux-arm-kernel, Tony Lindgren, linux-amlogic,
	linux-mediatek, Alexandre Belloni, Vivek Gautam, Maxime Ripard,
	Chen-Yu Tsai, Carlo Caione, Chunfeng Yun, Matthias Brugger,
	Manu Gautam

[-- Attachment #1: Type: text/plain, Size: 3240 bytes --]

Hi Grygorii,

Thanks for the patch!

On Mon, Nov 19, 2018 at 07:24:22PM -0600, Grygorii Strashko wrote:
> Convert ocelot-serdes PHY driver to use recently introduced
> PHY_MODE_ETHERNET and phy_set_mode_ext().
> 
> Cc: Quentin Schulz <quentin.schulz@bootlin.com>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>

Reviewed-by: Quentin Schulz <quentin.schulz@bootlin.com>
Tested-by: Quentin Schulz <quentin.schulz@bootlin.com>

Tested on top of latest master branch of net-next
(e432abfb99e5642a7e7fcaa1c8cb0e80c8fcf58e) on a PCB120 with VSC8584 PHYs
(for reference if we ever find out there is a problem with this patch).

> diff --git a/drivers/phy/mscc/phy-ocelot-serdes.c b/drivers/phy/mscc/phy-ocelot-serdes.c
> index c61a9890..77c46f6 100644
> --- a/drivers/phy/mscc/phy-ocelot-serdes.c
> +++ b/drivers/phy/mscc/phy-ocelot-serdes.c
> @@ -11,6 +11,7 @@
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_platform.h>
> +#include <linux/phy.h>
>  #include <linux/phy/phy.h>
>  #include <linux/platform_device.h>
>  #include <linux/regmap.h>
> @@ -104,20 +105,24 @@ struct serdes_mux {
>  	u8			idx;
>  	u8			port;
>  	enum phy_mode		mode;
> +	int			submode;
>  	u32			mask;
>  	u32			mux;
>  };
>  
> -#define SERDES_MUX(_idx, _port, _mode, _mask, _mux) {		\
> +#define SERDES_MUX(_idx, _port, _mode, _submode, _mask, _mux) {		\
>  	.idx = _idx,						\
>  	.port = _port,						\
>  	.mode = _mode,						\
> +	.submode = _submode,					\
>  	.mask = _mask,						\
>  	.mux = _mux,						\
>  }
>  
> -#define SERDES_MUX_SGMII(i, p, m, c) SERDES_MUX(i, p, PHY_MODE_SGMII, m, c)
> -#define SERDES_MUX_QSGMII(i, p, m, c) SERDES_MUX(i, p, PHY_MODE_QSGMII, m, c)
> +#define SERDES_MUX_SGMII(i, p, m, c) \
> +	SERDES_MUX(i, p, PHY_MODE_ETHERNET, PHY_INTERFACE_MODE_SGMII, m, c)
> +#define SERDES_MUX_QSGMII(i, p, m, c) \
> +	SERDES_MUX(i, p, PHY_MODE_ETHERNET, PHY_INTERFACE_MODE_QSGMII, m, c)
>  
>  static const struct serdes_mux ocelot_serdes_muxes[] = {
>  	SERDES_MUX_SGMII(SERDES1G(0), 0, 0, 0),
> @@ -154,7 +159,7 @@ static const struct serdes_mux ocelot_serdes_muxes[] = {
>  	SERDES_MUX_SGMII(SERDES6G(1), 8, 0, 0),
>  	SERDES_MUX_SGMII(SERDES6G(2), 10, HSIO_HW_CFG_PCIE_ENA |
>  			 HSIO_HW_CFG_DEV2G5_10_MODE, 0),
> -	SERDES_MUX(SERDES6G(2), 10, PHY_MODE_PCIE, HSIO_HW_CFG_PCIE_ENA,
> +	SERDES_MUX(SERDES6G(2), 10, PHY_MODE_PCIE, 0, HSIO_HW_CFG_PCIE_ENA,
>  		   HSIO_HW_CFG_PCIE_ENA),
>  };
>  
> @@ -164,12 +169,17 @@ static int serdes_set_mode(struct phy *phy, enum phy_mode mode, int submode)
>  	unsigned int i;
>  	int ret;
>  
> +	/* As of now only PHY_MODE_ETHERNET is supported */
> +	if (mode != PHY_MODE_ETHERNET)
> +		return -EOPNOTSUPP;
> +
>  	for (i = 0; i < ARRAY_SIZE(ocelot_serdes_muxes); i++) {
>  		if (macro->idx != ocelot_serdes_muxes[i].idx ||
> -		    mode != ocelot_serdes_muxes[i].mode)
> +		    mode != ocelot_serdes_muxes[i].mode ||
> +		    submode != ocelot_serdes_muxes[i].submode)
>  			continue;

We will most likely need to rework this to ignore the submode of the
PCIe muxing if the mode is PCIe but let´s figure this out when we add
support for PCIe muxing.

Thanks,
Quentin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 4/5] phy: mvebu-cp110-comphy: convert to use eth phy mode and submode
  2018-11-20  1:24 ` [PATCH v3 4/5] phy: mvebu-cp110-comphy: " Grygorii Strashko
@ 2018-11-21  7:38   ` Kishon Vijay Abraham I
  2018-11-21  7:52     ` Quentin Schulz
  2018-11-21  8:02   ` Antoine Tenart
  1 sibling, 1 reply; 14+ messages in thread
From: Kishon Vijay Abraham I @ 2018-11-21  7:38 UTC (permalink / raw)
  To: Grygorii Strashko, David S. Miller, Antoine Tenart,
	Quentin Schulz, Russell King - ARM Linux, Maxime Chevallier
  Cc: netdev, Sekhar Nori, linux-kernel, linux-arm-kernel,
	Tony Lindgren, linux-amlogic, linux-mediatek, Alexandre Belloni,
	Vivek Gautam, Maxime Ripard, Chen-Yu Tsai, Carlo Caione,
	Chunfeng Yun, Matthias Brugger, Manu Gautam

Antoine,

On 20/11/18 6:54 AM, Grygorii Strashko wrote:
> Convert mvebu-cp110-comphy PHY driver to use recently introduced
> PHY_MODE_ETHERNET and phy_set_mode_ext().

Care to give ACK for this patch?

Thanks
kishon
> 
> Cc: Russell King - ARM Linux <linux@armlinux.org.uk>
> Cc: Maxime Chevallier <maxime.chevallier@bootlin.com>
> Cc: Antoine Tenart <antoine.tenart@free-electrons.com>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>  drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 19 +-----
>  drivers/phy/marvell/phy-mvebu-cp110-comphy.c    | 90 ++++++++++++++-----------
>  2 files changed, 53 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> index 7a37a37..731793a 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> @@ -1165,28 +1165,13 @@ static void mvpp22_gop_setup_irq(struct mvpp2_port *port)
>   */
>  static int mvpp22_comphy_init(struct mvpp2_port *port)
>  {
> -	enum phy_mode mode;
>  	int ret;
>  
>  	if (!port->comphy)
>  		return 0;
>  
> -	switch (port->phy_interface) {
> -	case PHY_INTERFACE_MODE_SGMII:
> -	case PHY_INTERFACE_MODE_1000BASEX:
> -		mode = PHY_MODE_SGMII;
> -		break;
> -	case PHY_INTERFACE_MODE_2500BASEX:
> -		mode = PHY_MODE_2500SGMII;
> -		break;
> -	case PHY_INTERFACE_MODE_10GKR:
> -		mode = PHY_MODE_10GKR;
> -		break;
> -	default:
> -		return -EINVAL;
> -	}
> -
> -	ret = phy_set_mode(port->comphy, mode);
> +	ret = phy_set_mode_ext(port->comphy, PHY_MODE_ETHERNET,
> +			       port->phy_interface);
>  	if (ret)
>  		return ret;
>  
> diff --git a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> index 79b52c3..2b4462a 100644
> --- a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> +++ b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> @@ -9,6 +9,7 @@
>  #include <linux/iopoll.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/module.h>
> +#include <linux/phy.h>
>  #include <linux/phy/phy.h>
>  #include <linux/platform_device.h>
>  #include <linux/regmap.h>
> @@ -116,41 +117,43 @@
>  
>  struct mvebu_comhy_conf {
>  	enum phy_mode mode;
> +	int submode;
>  	unsigned lane;
>  	unsigned port;
>  	u32 mux;
>  };
>  
> -#define MVEBU_COMPHY_CONF(_lane, _port, _mode, _mux)	\
> +#define MVEBU_COMPHY_CONF(_lane, _port, _submode, _mux)	\
>  	{						\
>  		.lane = _lane,				\
>  		.port = _port,				\
> -		.mode = _mode,				\
> +		.mode = PHY_MODE_ETHERNET,		\
> +		.submode = _submode,			\
>  		.mux = _mux,				\
>  	}
>  
>  static const struct mvebu_comhy_conf mvebu_comphy_cp110_modes[] = {
>  	/* lane 0 */
> -	MVEBU_COMPHY_CONF(0, 1, PHY_MODE_SGMII, 0x1),
> -	MVEBU_COMPHY_CONF(0, 1, PHY_MODE_2500SGMII, 0x1),
> +	MVEBU_COMPHY_CONF(0, 1, PHY_INTERFACE_MODE_SGMII, 0x1),
> +	MVEBU_COMPHY_CONF(0, 1, PHY_INTERFACE_MODE_2500BASEX, 0x1),
>  	/* lane 1 */
> -	MVEBU_COMPHY_CONF(1, 2, PHY_MODE_SGMII, 0x1),
> -	MVEBU_COMPHY_CONF(1, 2, PHY_MODE_2500SGMII, 0x1),
> +	MVEBU_COMPHY_CONF(1, 2, PHY_INTERFACE_MODE_SGMII, 0x1),
> +	MVEBU_COMPHY_CONF(1, 2, PHY_INTERFACE_MODE_2500BASEX, 0x1),
>  	/* lane 2 */
> -	MVEBU_COMPHY_CONF(2, 0, PHY_MODE_SGMII, 0x1),
> -	MVEBU_COMPHY_CONF(2, 0, PHY_MODE_2500SGMII, 0x1),
> -	MVEBU_COMPHY_CONF(2, 0, PHY_MODE_10GKR, 0x1),
> +	MVEBU_COMPHY_CONF(2, 0, PHY_INTERFACE_MODE_SGMII, 0x1),
> +	MVEBU_COMPHY_CONF(2, 0, PHY_INTERFACE_MODE_2500BASEX, 0x1),
> +	MVEBU_COMPHY_CONF(2, 0, PHY_INTERFACE_MODE_10GKR, 0x1),
>  	/* lane 3 */
> -	MVEBU_COMPHY_CONF(3, 1, PHY_MODE_SGMII, 0x2),
> -	MVEBU_COMPHY_CONF(3, 1, PHY_MODE_2500SGMII, 0x2),
> +	MVEBU_COMPHY_CONF(3, 1, PHY_INTERFACE_MODE_SGMII, 0x2),
> +	MVEBU_COMPHY_CONF(3, 1, PHY_INTERFACE_MODE_2500BASEX, 0x2),
>  	/* lane 4 */
> -	MVEBU_COMPHY_CONF(4, 0, PHY_MODE_SGMII, 0x2),
> -	MVEBU_COMPHY_CONF(4, 0, PHY_MODE_2500SGMII, 0x2),
> -	MVEBU_COMPHY_CONF(4, 0, PHY_MODE_10GKR, 0x2),
> -	MVEBU_COMPHY_CONF(4, 1, PHY_MODE_SGMII, 0x1),
> +	MVEBU_COMPHY_CONF(4, 0, PHY_INTERFACE_MODE_SGMII, 0x2),
> +	MVEBU_COMPHY_CONF(4, 0, PHY_INTERFACE_MODE_2500BASEX, 0x2),
> +	MVEBU_COMPHY_CONF(4, 0, PHY_INTERFACE_MODE_10GKR, 0x2),
> +	MVEBU_COMPHY_CONF(4, 1, PHY_INTERFACE_MODE_SGMII, 0x1),
>  	/* lane 5 */
> -	MVEBU_COMPHY_CONF(5, 2, PHY_MODE_SGMII, 0x1),
> -	MVEBU_COMPHY_CONF(5, 2, PHY_MODE_2500SGMII, 0x1),
> +	MVEBU_COMPHY_CONF(5, 2, PHY_INTERFACE_MODE_SGMII, 0x1),
> +	MVEBU_COMPHY_CONF(5, 2, PHY_INTERFACE_MODE_2500BASEX, 0x1),
>  };
>  
>  struct mvebu_comphy_priv {
> @@ -163,10 +166,12 @@ struct mvebu_comphy_lane {
>  	struct mvebu_comphy_priv *priv;
>  	unsigned id;
>  	enum phy_mode mode;
> +	int submode;
>  	int port;
>  };
>  
> -static int mvebu_comphy_get_mux(int lane, int port, enum phy_mode mode)
> +static int mvebu_comphy_get_mux(int lane, int port,
> +				enum phy_mode mode, int submode)
>  {
>  	int i, n = ARRAY_SIZE(mvebu_comphy_cp110_modes);
>  
> @@ -177,7 +182,8 @@ static int mvebu_comphy_get_mux(int lane, int port, enum phy_mode mode)
>  	for (i = 0; i < n; i++) {
>  		if (mvebu_comphy_cp110_modes[i].lane == lane &&
>  		    mvebu_comphy_cp110_modes[i].port == port &&
> -		    mvebu_comphy_cp110_modes[i].mode == mode)
> +		    mvebu_comphy_cp110_modes[i].mode == mode &&
> +		    mvebu_comphy_cp110_modes[i].submode == submode)
>  			break;
>  	}
>  
> @@ -187,8 +193,7 @@ static int mvebu_comphy_get_mux(int lane, int port, enum phy_mode mode)
>  	return mvebu_comphy_cp110_modes[i].mux;
>  }
>  
> -static void mvebu_comphy_ethernet_init_reset(struct mvebu_comphy_lane *lane,
> -					     enum phy_mode mode)
> +static void mvebu_comphy_ethernet_init_reset(struct mvebu_comphy_lane *lane)
>  {
>  	struct mvebu_comphy_priv *priv = lane->priv;
>  	u32 val;
> @@ -206,14 +211,14 @@ static void mvebu_comphy_ethernet_init_reset(struct mvebu_comphy_lane *lane,
>  		 MVEBU_COMPHY_SERDES_CFG0_HALF_BUS |
>  		 MVEBU_COMPHY_SERDES_CFG0_GEN_RX(0xf) |
>  		 MVEBU_COMPHY_SERDES_CFG0_GEN_TX(0xf));
> -	if (mode == PHY_MODE_10GKR)
> +	if (lane->submode == PHY_INTERFACE_MODE_10GKR)
>  		val |= MVEBU_COMPHY_SERDES_CFG0_GEN_RX(0xe) |
>  		       MVEBU_COMPHY_SERDES_CFG0_GEN_TX(0xe);
> -	else if (mode == PHY_MODE_2500SGMII)
> +	else if (lane->submode == PHY_INTERFACE_MODE_2500BASEX)
>  		val |= MVEBU_COMPHY_SERDES_CFG0_GEN_RX(0x8) |
>  		       MVEBU_COMPHY_SERDES_CFG0_GEN_TX(0x8) |
>  		       MVEBU_COMPHY_SERDES_CFG0_HALF_BUS;
> -	else if (mode == PHY_MODE_SGMII)
> +	else if (lane->submode == PHY_INTERFACE_MODE_SGMII)
>  		val |= MVEBU_COMPHY_SERDES_CFG0_GEN_RX(0x6) |
>  		       MVEBU_COMPHY_SERDES_CFG0_GEN_TX(0x6) |
>  		       MVEBU_COMPHY_SERDES_CFG0_HALF_BUS;
> @@ -243,7 +248,7 @@ static void mvebu_comphy_ethernet_init_reset(struct mvebu_comphy_lane *lane,
>  	/* refclk selection */
>  	val = readl(priv->base + MVEBU_COMPHY_MISC_CTRL0(lane->id));
>  	val &= ~MVEBU_COMPHY_MISC_CTRL0_REFCLK_SEL;
> -	if (mode == PHY_MODE_10GKR)
> +	if (lane->submode == PHY_INTERFACE_MODE_10GKR)
>  		val |= MVEBU_COMPHY_MISC_CTRL0_ICP_FORCE;
>  	writel(val, priv->base + MVEBU_COMPHY_MISC_CTRL0(lane->id));
>  
> @@ -261,8 +266,7 @@ static void mvebu_comphy_ethernet_init_reset(struct mvebu_comphy_lane *lane,
>  	writel(val, priv->base + MVEBU_COMPHY_LOOPBACK(lane->id));
>  }
>  
> -static int mvebu_comphy_init_plls(struct mvebu_comphy_lane *lane,
> -				  enum phy_mode mode)
> +static int mvebu_comphy_init_plls(struct mvebu_comphy_lane *lane)
>  {
>  	struct mvebu_comphy_priv *priv = lane->priv;
>  	u32 val;
> @@ -303,13 +307,13 @@ static int mvebu_comphy_init_plls(struct mvebu_comphy_lane *lane,
>  	return 0;
>  }
>  
> -static int mvebu_comphy_set_mode_sgmii(struct phy *phy, enum phy_mode mode)
> +static int mvebu_comphy_set_mode_sgmii(struct phy *phy)
>  {
>  	struct mvebu_comphy_lane *lane = phy_get_drvdata(phy);
>  	struct mvebu_comphy_priv *priv = lane->priv;
>  	u32 val;
>  
> -	mvebu_comphy_ethernet_init_reset(lane, mode);
> +	mvebu_comphy_ethernet_init_reset(lane);
>  
>  	val = readl(priv->base + MVEBU_COMPHY_RX_CTRL1(lane->id));
>  	val &= ~MVEBU_COMPHY_RX_CTRL1_CLK8T_EN;
> @@ -330,7 +334,7 @@ static int mvebu_comphy_set_mode_sgmii(struct phy *phy, enum phy_mode mode)
>  	val |= MVEBU_COMPHY_GEN1_S0_TX_EMPH(0x1);
>  	writel(val, priv->base + MVEBU_COMPHY_GEN1_S0(lane->id));
>  
> -	return mvebu_comphy_init_plls(lane, PHY_MODE_SGMII);
> +	return mvebu_comphy_init_plls(lane);
>  }
>  
>  static int mvebu_comphy_set_mode_10gkr(struct phy *phy)
> @@ -339,7 +343,7 @@ static int mvebu_comphy_set_mode_10gkr(struct phy *phy)
>  	struct mvebu_comphy_priv *priv = lane->priv;
>  	u32 val;
>  
> -	mvebu_comphy_ethernet_init_reset(lane, PHY_MODE_10GKR);
> +	mvebu_comphy_ethernet_init_reset(lane);
>  
>  	val = readl(priv->base + MVEBU_COMPHY_RX_CTRL1(lane->id));
>  	val |= MVEBU_COMPHY_RX_CTRL1_RXCLK2X_SEL |
> @@ -469,7 +473,7 @@ static int mvebu_comphy_set_mode_10gkr(struct phy *phy)
>  	val |= MVEBU_COMPHY_EXT_SELV_RX_SAMPL(0x1a);
>  	writel(val, priv->base + MVEBU_COMPHY_EXT_SELV(lane->id));
>  
> -	return mvebu_comphy_init_plls(lane, PHY_MODE_10GKR);
> +	return mvebu_comphy_init_plls(lane);
>  }
>  
>  static int mvebu_comphy_power_on(struct phy *phy)
> @@ -479,7 +483,8 @@ static int mvebu_comphy_power_on(struct phy *phy)
>  	int ret, mux;
>  	u32 val;
>  
> -	mux = mvebu_comphy_get_mux(lane->id, lane->port, lane->mode);
> +	mux = mvebu_comphy_get_mux(lane->id, lane->port,
> +				   lane->mode, lane->submode);
>  	if (mux < 0)
>  		return -ENOTSUPP;
>  
> @@ -492,12 +497,12 @@ static int mvebu_comphy_power_on(struct phy *phy)
>  	val |= mux << MVEBU_COMPHY_SELECTOR_PHY(lane->id);
>  	regmap_write(priv->regmap, MVEBU_COMPHY_SELECTOR, val);
>  
> -	switch (lane->mode) {
> -	case PHY_MODE_SGMII:
> -	case PHY_MODE_2500SGMII:
> -		ret = mvebu_comphy_set_mode_sgmii(phy, lane->mode);
> +	switch (lane->submode) {
> +	case PHY_INTERFACE_MODE_SGMII:
> +	case PHY_INTERFACE_MODE_2500BASEX:
> +		ret = mvebu_comphy_set_mode_sgmii(phy);
>  		break;
> -	case PHY_MODE_10GKR:
> +	case PHY_INTERFACE_MODE_10GKR:
>  		ret = mvebu_comphy_set_mode_10gkr(phy);
>  		break;
>  	default:
> @@ -517,10 +522,17 @@ static int mvebu_comphy_set_mode(struct phy *phy,
>  {
>  	struct mvebu_comphy_lane *lane = phy_get_drvdata(phy);
>  
> -	if (mvebu_comphy_get_mux(lane->id, lane->port, mode) < 0)
> +	if (mode != PHY_MODE_ETHERNET)
> +		return -EINVAL;
> +
> +	if (submode == PHY_INTERFACE_MODE_1000BASEX)
> +		submode = PHY_INTERFACE_MODE_SGMII;
> +
> +	if (mvebu_comphy_get_mux(lane->id, lane->port, mode, submode) < 0)
>  		return -EINVAL;
>  
>  	lane->mode = mode;
> +	lane->submode = submode;
>  	return 0;
>  }
>  
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 4/5] phy: mvebu-cp110-comphy: convert to use eth phy mode and submode
  2018-11-21  7:38   ` Kishon Vijay Abraham I
@ 2018-11-21  7:52     ` Quentin Schulz
  2018-11-21  7:58       ` Antoine Tenart
  0 siblings, 1 reply; 14+ messages in thread
From: Quentin Schulz @ 2018-11-21  7:52 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Grygorii Strashko, David S. Miller, Antoine Tenart,
	Russell King - ARM Linux, Maxime Chevallier, netdev, Sekhar Nori,
	linux-kernel, linux-arm-kernel, Tony Lindgren, linux-amlogic,
	linux-mediatek, Alexandre Belloni, Vivek Gautam, Maxime Ripard,
	Chen-Yu Tsai, Carlo Caione, Chunfeng Yun, Matthias Brugger,
	Manu Gautam

[-- Attachment #1: Type: text/plain, Size: 424 bytes --]

Hi Kishon,

On Wed, Nov 21, 2018 at 01:08:42PM +0530, Kishon Vijay Abraham I wrote:
> Antoine,
> 
> On 20/11/18 6:54 AM, Grygorii Strashko wrote:
> > Convert mvebu-cp110-comphy PHY driver to use recently introduced
> > PHY_MODE_ETHERNET and phy_set_mode_ext().
> 
> Care to give ACK for this patch?
> 

Acked-by Antoine and Tested-by Maxime given in v2:
https://patchwork.kernel.org/patch/10676749/

Quentin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 4/5] phy: mvebu-cp110-comphy: convert to use eth phy mode and submode
  2018-11-21  7:52     ` Quentin Schulz
@ 2018-11-21  7:58       ` Antoine Tenart
  0 siblings, 0 replies; 14+ messages in thread
From: Antoine Tenart @ 2018-11-21  7:58 UTC (permalink / raw)
  To: Quentin Schulz
  Cc: Kishon Vijay Abraham I, Grygorii Strashko, David S. Miller,
	Antoine Tenart, Russell King - ARM Linux, Maxime Chevallier,
	netdev, Sekhar Nori, linux-kernel, linux-arm-kernel,
	Tony Lindgren, linux-amlogic, linux-mediatek, Alexandre Belloni,
	Vivek Gautam, Maxime Ripard, Chen-Yu Tsai, Carlo Caione,
	Chunfeng Yun, Matthias Brugger, Manu Gautam

Hi Quentin,

On Wed, Nov 21, 2018 at 08:52:20AM +0100, Quentin Schulz wrote:
> On Wed, Nov 21, 2018 at 01:08:42PM +0530, Kishon Vijay Abraham I wrote:
> > 
> > On 20/11/18 6:54 AM, Grygorii Strashko wrote:
> > > Convert mvebu-cp110-comphy PHY driver to use recently introduced
> > > PHY_MODE_ETHERNET and phy_set_mode_ext().
> > 
> > Care to give ACK for this patch?
> > 
> 
> Acked-by Antoine and Tested-by Maxime given in v2:
> https://patchwork.kernel.org/patch/10676749/

The Acks were removed from v3 due to changes in the patches.

Thanks,
Antoine

-- 
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 4/5] phy: mvebu-cp110-comphy: convert to use eth phy mode and submode
  2018-11-20  1:24 ` [PATCH v3 4/5] phy: mvebu-cp110-comphy: " Grygorii Strashko
  2018-11-21  7:38   ` Kishon Vijay Abraham I
@ 2018-11-21  8:02   ` Antoine Tenart
  1 sibling, 0 replies; 14+ messages in thread
From: Antoine Tenart @ 2018-11-21  8:02 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: David S. Miller, Kishon Vijay Abraham I, Antoine Tenart,
	Quentin Schulz, Russell King - ARM Linux, Maxime Chevallier,
	netdev, Sekhar Nori, linux-kernel, linux-arm-kernel,
	Tony Lindgren, linux-amlogic, linux-mediatek, Alexandre Belloni,
	Vivek Gautam, Maxime Ripard, Chen-Yu Tsai, Carlo Caione,
	Chunfeng Yun, Matthias Brugger, Manu Gautam

Hi Grygorii,

On Mon, Nov 19, 2018 at 07:24:23PM -0600, Grygorii Strashko wrote:
> Convert mvebu-cp110-comphy PHY driver to use recently introduced
> PHY_MODE_ETHERNET and phy_set_mode_ext().
> 
> Cc: Russell King - ARM Linux <linux@armlinux.org.uk>
> Cc: Maxime Chevallier <maxime.chevallier@bootlin.com>
> Cc: Antoine Tenart <antoine.tenart@free-electrons.com>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>

Thanks for the changes, this looks good. I tested this on a
MacchiatoBin.

Acked-by: Antoine Tenart <antoine.tenart@bootlin.com>

> ---
>  drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 19 +-----
>  drivers/phy/marvell/phy-mvebu-cp110-comphy.c    | 90 ++++++++++++++-----------
>  2 files changed, 53 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> index 7a37a37..731793a 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> @@ -1165,28 +1165,13 @@ static void mvpp22_gop_setup_irq(struct mvpp2_port *port)
>   */
>  static int mvpp22_comphy_init(struct mvpp2_port *port)
>  {
> -	enum phy_mode mode;
>  	int ret;
>  
>  	if (!port->comphy)
>  		return 0;
>  
> -	switch (port->phy_interface) {
> -	case PHY_INTERFACE_MODE_SGMII:
> -	case PHY_INTERFACE_MODE_1000BASEX:
> -		mode = PHY_MODE_SGMII;
> -		break;
> -	case PHY_INTERFACE_MODE_2500BASEX:
> -		mode = PHY_MODE_2500SGMII;
> -		break;
> -	case PHY_INTERFACE_MODE_10GKR:
> -		mode = PHY_MODE_10GKR;
> -		break;
> -	default:
> -		return -EINVAL;
> -	}
> -
> -	ret = phy_set_mode(port->comphy, mode);
> +	ret = phy_set_mode_ext(port->comphy, PHY_MODE_ETHERNET,
> +			       port->phy_interface);
>  	if (ret)
>  		return ret;
>  
> diff --git a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> index 79b52c3..2b4462a 100644
> --- a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> +++ b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
> @@ -9,6 +9,7 @@
>  #include <linux/iopoll.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/module.h>
> +#include <linux/phy.h>
>  #include <linux/phy/phy.h>
>  #include <linux/platform_device.h>
>  #include <linux/regmap.h>
> @@ -116,41 +117,43 @@
>  
>  struct mvebu_comhy_conf {
>  	enum phy_mode mode;
> +	int submode;
>  	unsigned lane;
>  	unsigned port;
>  	u32 mux;
>  };
>  
> -#define MVEBU_COMPHY_CONF(_lane, _port, _mode, _mux)	\
> +#define MVEBU_COMPHY_CONF(_lane, _port, _submode, _mux)	\
>  	{						\
>  		.lane = _lane,				\
>  		.port = _port,				\
> -		.mode = _mode,				\
> +		.mode = PHY_MODE_ETHERNET,		\
> +		.submode = _submode,			\
>  		.mux = _mux,				\
>  	}
>  
>  static const struct mvebu_comhy_conf mvebu_comphy_cp110_modes[] = {
>  	/* lane 0 */
> -	MVEBU_COMPHY_CONF(0, 1, PHY_MODE_SGMII, 0x1),
> -	MVEBU_COMPHY_CONF(0, 1, PHY_MODE_2500SGMII, 0x1),
> +	MVEBU_COMPHY_CONF(0, 1, PHY_INTERFACE_MODE_SGMII, 0x1),
> +	MVEBU_COMPHY_CONF(0, 1, PHY_INTERFACE_MODE_2500BASEX, 0x1),
>  	/* lane 1 */
> -	MVEBU_COMPHY_CONF(1, 2, PHY_MODE_SGMII, 0x1),
> -	MVEBU_COMPHY_CONF(1, 2, PHY_MODE_2500SGMII, 0x1),
> +	MVEBU_COMPHY_CONF(1, 2, PHY_INTERFACE_MODE_SGMII, 0x1),
> +	MVEBU_COMPHY_CONF(1, 2, PHY_INTERFACE_MODE_2500BASEX, 0x1),
>  	/* lane 2 */
> -	MVEBU_COMPHY_CONF(2, 0, PHY_MODE_SGMII, 0x1),
> -	MVEBU_COMPHY_CONF(2, 0, PHY_MODE_2500SGMII, 0x1),
> -	MVEBU_COMPHY_CONF(2, 0, PHY_MODE_10GKR, 0x1),
> +	MVEBU_COMPHY_CONF(2, 0, PHY_INTERFACE_MODE_SGMII, 0x1),
> +	MVEBU_COMPHY_CONF(2, 0, PHY_INTERFACE_MODE_2500BASEX, 0x1),
> +	MVEBU_COMPHY_CONF(2, 0, PHY_INTERFACE_MODE_10GKR, 0x1),
>  	/* lane 3 */
> -	MVEBU_COMPHY_CONF(3, 1, PHY_MODE_SGMII, 0x2),
> -	MVEBU_COMPHY_CONF(3, 1, PHY_MODE_2500SGMII, 0x2),
> +	MVEBU_COMPHY_CONF(3, 1, PHY_INTERFACE_MODE_SGMII, 0x2),
> +	MVEBU_COMPHY_CONF(3, 1, PHY_INTERFACE_MODE_2500BASEX, 0x2),
>  	/* lane 4 */
> -	MVEBU_COMPHY_CONF(4, 0, PHY_MODE_SGMII, 0x2),
> -	MVEBU_COMPHY_CONF(4, 0, PHY_MODE_2500SGMII, 0x2),
> -	MVEBU_COMPHY_CONF(4, 0, PHY_MODE_10GKR, 0x2),
> -	MVEBU_COMPHY_CONF(4, 1, PHY_MODE_SGMII, 0x1),
> +	MVEBU_COMPHY_CONF(4, 0, PHY_INTERFACE_MODE_SGMII, 0x2),
> +	MVEBU_COMPHY_CONF(4, 0, PHY_INTERFACE_MODE_2500BASEX, 0x2),
> +	MVEBU_COMPHY_CONF(4, 0, PHY_INTERFACE_MODE_10GKR, 0x2),
> +	MVEBU_COMPHY_CONF(4, 1, PHY_INTERFACE_MODE_SGMII, 0x1),
>  	/* lane 5 */
> -	MVEBU_COMPHY_CONF(5, 2, PHY_MODE_SGMII, 0x1),
> -	MVEBU_COMPHY_CONF(5, 2, PHY_MODE_2500SGMII, 0x1),
> +	MVEBU_COMPHY_CONF(5, 2, PHY_INTERFACE_MODE_SGMII, 0x1),
> +	MVEBU_COMPHY_CONF(5, 2, PHY_INTERFACE_MODE_2500BASEX, 0x1),
>  };
>  
>  struct mvebu_comphy_priv {
> @@ -163,10 +166,12 @@ struct mvebu_comphy_lane {
>  	struct mvebu_comphy_priv *priv;
>  	unsigned id;
>  	enum phy_mode mode;
> +	int submode;
>  	int port;
>  };
>  
> -static int mvebu_comphy_get_mux(int lane, int port, enum phy_mode mode)
> +static int mvebu_comphy_get_mux(int lane, int port,
> +				enum phy_mode mode, int submode)
>  {
>  	int i, n = ARRAY_SIZE(mvebu_comphy_cp110_modes);
>  
> @@ -177,7 +182,8 @@ static int mvebu_comphy_get_mux(int lane, int port, enum phy_mode mode)
>  	for (i = 0; i < n; i++) {
>  		if (mvebu_comphy_cp110_modes[i].lane == lane &&
>  		    mvebu_comphy_cp110_modes[i].port == port &&
> -		    mvebu_comphy_cp110_modes[i].mode == mode)
> +		    mvebu_comphy_cp110_modes[i].mode == mode &&
> +		    mvebu_comphy_cp110_modes[i].submode == submode)
>  			break;
>  	}
>  
> @@ -187,8 +193,7 @@ static int mvebu_comphy_get_mux(int lane, int port, enum phy_mode mode)
>  	return mvebu_comphy_cp110_modes[i].mux;
>  }
>  
> -static void mvebu_comphy_ethernet_init_reset(struct mvebu_comphy_lane *lane,
> -					     enum phy_mode mode)
> +static void mvebu_comphy_ethernet_init_reset(struct mvebu_comphy_lane *lane)
>  {
>  	struct mvebu_comphy_priv *priv = lane->priv;
>  	u32 val;
> @@ -206,14 +211,14 @@ static void mvebu_comphy_ethernet_init_reset(struct mvebu_comphy_lane *lane,
>  		 MVEBU_COMPHY_SERDES_CFG0_HALF_BUS |
>  		 MVEBU_COMPHY_SERDES_CFG0_GEN_RX(0xf) |
>  		 MVEBU_COMPHY_SERDES_CFG0_GEN_TX(0xf));
> -	if (mode == PHY_MODE_10GKR)
> +	if (lane->submode == PHY_INTERFACE_MODE_10GKR)
>  		val |= MVEBU_COMPHY_SERDES_CFG0_GEN_RX(0xe) |
>  		       MVEBU_COMPHY_SERDES_CFG0_GEN_TX(0xe);
> -	else if (mode == PHY_MODE_2500SGMII)
> +	else if (lane->submode == PHY_INTERFACE_MODE_2500BASEX)
>  		val |= MVEBU_COMPHY_SERDES_CFG0_GEN_RX(0x8) |
>  		       MVEBU_COMPHY_SERDES_CFG0_GEN_TX(0x8) |
>  		       MVEBU_COMPHY_SERDES_CFG0_HALF_BUS;
> -	else if (mode == PHY_MODE_SGMII)
> +	else if (lane->submode == PHY_INTERFACE_MODE_SGMII)
>  		val |= MVEBU_COMPHY_SERDES_CFG0_GEN_RX(0x6) |
>  		       MVEBU_COMPHY_SERDES_CFG0_GEN_TX(0x6) |
>  		       MVEBU_COMPHY_SERDES_CFG0_HALF_BUS;
> @@ -243,7 +248,7 @@ static void mvebu_comphy_ethernet_init_reset(struct mvebu_comphy_lane *lane,
>  	/* refclk selection */
>  	val = readl(priv->base + MVEBU_COMPHY_MISC_CTRL0(lane->id));
>  	val &= ~MVEBU_COMPHY_MISC_CTRL0_REFCLK_SEL;
> -	if (mode == PHY_MODE_10GKR)
> +	if (lane->submode == PHY_INTERFACE_MODE_10GKR)
>  		val |= MVEBU_COMPHY_MISC_CTRL0_ICP_FORCE;
>  	writel(val, priv->base + MVEBU_COMPHY_MISC_CTRL0(lane->id));
>  
> @@ -261,8 +266,7 @@ static void mvebu_comphy_ethernet_init_reset(struct mvebu_comphy_lane *lane,
>  	writel(val, priv->base + MVEBU_COMPHY_LOOPBACK(lane->id));
>  }
>  
> -static int mvebu_comphy_init_plls(struct mvebu_comphy_lane *lane,
> -				  enum phy_mode mode)
> +static int mvebu_comphy_init_plls(struct mvebu_comphy_lane *lane)
>  {
>  	struct mvebu_comphy_priv *priv = lane->priv;
>  	u32 val;
> @@ -303,13 +307,13 @@ static int mvebu_comphy_init_plls(struct mvebu_comphy_lane *lane,
>  	return 0;
>  }
>  
> -static int mvebu_comphy_set_mode_sgmii(struct phy *phy, enum phy_mode mode)
> +static int mvebu_comphy_set_mode_sgmii(struct phy *phy)
>  {
>  	struct mvebu_comphy_lane *lane = phy_get_drvdata(phy);
>  	struct mvebu_comphy_priv *priv = lane->priv;
>  	u32 val;
>  
> -	mvebu_comphy_ethernet_init_reset(lane, mode);
> +	mvebu_comphy_ethernet_init_reset(lane);
>  
>  	val = readl(priv->base + MVEBU_COMPHY_RX_CTRL1(lane->id));
>  	val &= ~MVEBU_COMPHY_RX_CTRL1_CLK8T_EN;
> @@ -330,7 +334,7 @@ static int mvebu_comphy_set_mode_sgmii(struct phy *phy, enum phy_mode mode)
>  	val |= MVEBU_COMPHY_GEN1_S0_TX_EMPH(0x1);
>  	writel(val, priv->base + MVEBU_COMPHY_GEN1_S0(lane->id));
>  
> -	return mvebu_comphy_init_plls(lane, PHY_MODE_SGMII);
> +	return mvebu_comphy_init_plls(lane);
>  }
>  
>  static int mvebu_comphy_set_mode_10gkr(struct phy *phy)
> @@ -339,7 +343,7 @@ static int mvebu_comphy_set_mode_10gkr(struct phy *phy)
>  	struct mvebu_comphy_priv *priv = lane->priv;
>  	u32 val;
>  
> -	mvebu_comphy_ethernet_init_reset(lane, PHY_MODE_10GKR);
> +	mvebu_comphy_ethernet_init_reset(lane);
>  
>  	val = readl(priv->base + MVEBU_COMPHY_RX_CTRL1(lane->id));
>  	val |= MVEBU_COMPHY_RX_CTRL1_RXCLK2X_SEL |
> @@ -469,7 +473,7 @@ static int mvebu_comphy_set_mode_10gkr(struct phy *phy)
>  	val |= MVEBU_COMPHY_EXT_SELV_RX_SAMPL(0x1a);
>  	writel(val, priv->base + MVEBU_COMPHY_EXT_SELV(lane->id));
>  
> -	return mvebu_comphy_init_plls(lane, PHY_MODE_10GKR);
> +	return mvebu_comphy_init_plls(lane);
>  }
>  
>  static int mvebu_comphy_power_on(struct phy *phy)
> @@ -479,7 +483,8 @@ static int mvebu_comphy_power_on(struct phy *phy)
>  	int ret, mux;
>  	u32 val;
>  
> -	mux = mvebu_comphy_get_mux(lane->id, lane->port, lane->mode);
> +	mux = mvebu_comphy_get_mux(lane->id, lane->port,
> +				   lane->mode, lane->submode);
>  	if (mux < 0)
>  		return -ENOTSUPP;
>  
> @@ -492,12 +497,12 @@ static int mvebu_comphy_power_on(struct phy *phy)
>  	val |= mux << MVEBU_COMPHY_SELECTOR_PHY(lane->id);
>  	regmap_write(priv->regmap, MVEBU_COMPHY_SELECTOR, val);
>  
> -	switch (lane->mode) {
> -	case PHY_MODE_SGMII:
> -	case PHY_MODE_2500SGMII:
> -		ret = mvebu_comphy_set_mode_sgmii(phy, lane->mode);
> +	switch (lane->submode) {
> +	case PHY_INTERFACE_MODE_SGMII:
> +	case PHY_INTERFACE_MODE_2500BASEX:
> +		ret = mvebu_comphy_set_mode_sgmii(phy);
>  		break;
> -	case PHY_MODE_10GKR:
> +	case PHY_INTERFACE_MODE_10GKR:
>  		ret = mvebu_comphy_set_mode_10gkr(phy);
>  		break;
>  	default:
> @@ -517,10 +522,17 @@ static int mvebu_comphy_set_mode(struct phy *phy,
>  {
>  	struct mvebu_comphy_lane *lane = phy_get_drvdata(phy);
>  
> -	if (mvebu_comphy_get_mux(lane->id, lane->port, mode) < 0)
> +	if (mode != PHY_MODE_ETHERNET)
> +		return -EINVAL;
> +
> +	if (submode == PHY_INTERFACE_MODE_1000BASEX)
> +		submode = PHY_INTERFACE_MODE_SGMII;
> +
> +	if (mvebu_comphy_get_mux(lane->id, lane->port, mode, submode) < 0)
>  		return -EINVAL;
>  
>  	lane->mode = mode;
> +	lane->submode = submode;
>  	return 0;
>  }
>  
> -- 
> 2.10.5
> 

-- 
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 0/5] phy: core: rework phy_set_mode to accept phy mode and submode
  2018-11-20  1:24 [PATCH v3 0/5] phy: core: rework phy_set_mode to accept phy mode and submode Grygorii Strashko
                   ` (4 preceding siblings ...)
  2018-11-20  1:24 ` [PATCH v3 5/5] phy: core: clean up unused ethernet specific phy modes Grygorii Strashko
@ 2018-11-21  8:55 ` Kishon Vijay Abraham I
  2018-11-21 18:23   ` Grygorii Strashko
  2018-12-17 14:06 ` Maxime Ripard
  6 siblings, 1 reply; 14+ messages in thread
From: Kishon Vijay Abraham I @ 2018-11-21  8:55 UTC (permalink / raw)
  To: Grygorii Strashko, David S. Miller, Antoine Tenart,
	Quentin Schulz, Russell King - ARM Linux, Maxime Chevallier
  Cc: netdev, Sekhar Nori, linux-kernel, linux-arm-kernel,
	Tony Lindgren, linux-amlogic, linux-mediatek, Alexandre Belloni,
	Vivek Gautam, Maxime Ripard, Chen-Yu Tsai, Carlo Caione,
	Chunfeng Yun, Matthias Brugger, Manu Gautam



On 20/11/18 6:54 AM, Grygorii Strashko wrote:
> Hi Kishon, All,
> 
> Thank you for your review.
> I've not added "Tested-by"/"Acked-by" tags due to code changes in v3.

merged after adding the required ACKs.

Thanks
Kishon
> 
> As was discussed in [1] I'm posting series which introduces rework of
> phy_set_mode to accept phy mode and submode. I've dropped TI specific patches as
> this change is pretty big by itself.
> 
> Patch 1 is cumulative change which refactors PHY framework code to
> support dual level PHYs mode configuration - PHY mode and PHY submode. It
> extends .set_mode() callback to support additional parameter "int submode"
> and converts all corresponding PHY drivers to support new .set_mode()
> callback declaration.
> The new extended PHY API
>  int phy_set_mode_ext(struct phy *phy, enum phy_mode mode, int submode)
> is introduced to support dual level PHYs mode configuration and existing
> phy_set_mode() API is converted to macros, so PHY framework consumers do
> not need to be changed (~21 matches).
> 
> Patches 2-4: Add new PHY's mode to be used by Ethernet PHY interface drivers or
> multipurpose PHYs like serdes and convert ocelot-serdes and mvebu-cp110-comphy
> PHY drivers to use recently introduced PHY_MODE_ETHERNET and phy_set_mode_ext().
> 
> Patch 5 - removes unused, ethernet specific phy modes from enum phy_mode.
> 
> Testing:
>  - series tested on TI am335x/am437x/am5(dra7) paltforms.
>  - other driver build tested.
> 
> changes in v3:
> - mux tables for PHY ocelot-serdes driver updated to store PHY mode and submode
> - mux tables for PHY mvebu-cp110-comphy driver updated to store PHY mode
>   and submode
> changes in v2:
> - marvell PHY and net drivers updated as recommended by Russell King
> 
> v2: https://lkml.org/lkml/2018/11/10/220
> v1: https://lkml.org/lkml/2018/11/8/260
> 
> [1] https://lkml.org/lkml/2018/10/25/366
> 
> Grygorii Strashko (5):
>   phy: core: rework phy_set_mode to accept phy mode and submode
>   phy: core: add PHY_MODE_ETHERNET
>   phy: ocelot-serdes: convert to use eth phy mode and submode
>   phy: mvebu-cp110-comphy: convert to use eth phy mode and submode
>   phy: core: clean up unused ethernet specific phy modes
> 
>  drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 19 +----
>  drivers/net/ethernet/mscc/ocelot.c              |  9 +--
>  drivers/phy/allwinner/phy-sun4i-usb.c           |  3 +-
>  drivers/phy/amlogic/phy-meson-gxl-usb2.c        |  5 +-
>  drivers/phy/amlogic/phy-meson-gxl-usb3.c        |  5 +-
>  drivers/phy/marvell/phy-mvebu-cp110-comphy.c    | 93 ++++++++++++++-----------
>  drivers/phy/mediatek/phy-mtk-tphy.c             |  2 +-
>  drivers/phy/mediatek/phy-mtk-xsphy.c            |  2 +-
>  drivers/phy/mscc/phy-ocelot-serdes.c            | 24 +++++--
>  drivers/phy/phy-core.c                          |  6 +-
>  drivers/phy/qualcomm/phy-qcom-qmp.c             |  3 +-
>  drivers/phy/qualcomm/phy-qcom-qusb2.c           |  3 +-
>  drivers/phy/qualcomm/phy-qcom-ufs-qmp-14nm.c    |  3 +-
>  drivers/phy/qualcomm/phy-qcom-ufs-qmp-20nm.c    |  3 +-
>  drivers/phy/qualcomm/phy-qcom-usb-hs.c          |  3 +-
>  drivers/phy/ti/phy-da8xx-usb.c                  |  3 +-
>  drivers/phy/ti/phy-tusb1210.c                   |  2 +-
>  include/linux/phy/phy.h                         | 18 +++--
>  18 files changed, 111 insertions(+), 95 deletions(-)
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 0/5] phy: core: rework phy_set_mode to accept phy mode and submode
  2018-11-21  8:55 ` [PATCH v3 0/5] phy: core: rework phy_set_mode to accept phy mode and submode Kishon Vijay Abraham I
@ 2018-11-21 18:23   ` Grygorii Strashko
  0 siblings, 0 replies; 14+ messages in thread
From: Grygorii Strashko @ 2018-11-21 18:23 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, David S. Miller, Antoine Tenart,
	Quentin Schulz, Russell King - ARM Linux, Maxime Chevallier
  Cc: netdev, Sekhar Nori, linux-kernel, linux-arm-kernel,
	Tony Lindgren, linux-amlogic, linux-mediatek, Alexandre Belloni,
	Vivek Gautam, Maxime Ripard, Chen-Yu Tsai, Carlo Caione,
	Chunfeng Yun, Matthias Brugger, Manu Gautam



On 11/21/18 2:55 AM, Kishon Vijay Abraham I wrote:
> 
> 
> On 20/11/18 6:54 AM, Grygorii Strashko wrote:
>> Hi Kishon, All,
>>
>> Thank you for your review.
>> I've not added "Tested-by"/"Acked-by" tags due to code changes in v3.
> 
> merged after adding the required ACKs.
> 

Thank you all. I'll send TI specific patches soon.


-- 
regards,
-grygorii

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 0/5] phy: core: rework phy_set_mode to accept phy mode and submode
  2018-11-20  1:24 [PATCH v3 0/5] phy: core: rework phy_set_mode to accept phy mode and submode Grygorii Strashko
                   ` (5 preceding siblings ...)
  2018-11-21  8:55 ` [PATCH v3 0/5] phy: core: rework phy_set_mode to accept phy mode and submode Kishon Vijay Abraham I
@ 2018-12-17 14:06 ` Maxime Ripard
  6 siblings, 0 replies; 14+ messages in thread
From: Maxime Ripard @ 2018-12-17 14:06 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: David S. Miller, Kishon Vijay Abraham I, Antoine Tenart,
	Quentin Schulz, Russell King - ARM Linux, Maxime Chevallier,
	netdev, Sekhar Nori, linux-kernel, linux-arm-kernel,
	Tony Lindgren, linux-amlogic, linux-mediatek, Alexandre Belloni,
	Vivek Gautam, Chen-Yu Tsai, Carlo Caione, Chunfeng Yun,
	Matthias Brugger, Manu Gautam

[-- Attachment #1: Type: text/plain, Size: 3253 bytes --]

Hi Grygorii, Kishon,

On Mon, Nov 19, 2018 at 07:24:19PM -0600, Grygorii Strashko wrote:
> Thank you for your review.
> I've not added "Tested-by"/"Acked-by" tags due to code changes in v3.
> 
> As was discussed in [1] I'm posting series which introduces rework of
> phy_set_mode to accept phy mode and submode. I've dropped TI specific patches as
> this change is pretty big by itself.
> 
> Patch 1 is cumulative change which refactors PHY framework code to
> support dual level PHYs mode configuration - PHY mode and PHY submode. It
> extends .set_mode() callback to support additional parameter "int submode"
> and converts all corresponding PHY drivers to support new .set_mode()
> callback declaration.
> The new extended PHY API
>  int phy_set_mode_ext(struct phy *phy, enum phy_mode mode, int submode)
> is introduced to support dual level PHYs mode configuration and existing
> phy_set_mode() API is converted to macros, so PHY framework consumers do
> not need to be changed (~21 matches).
> 
> Patches 2-4: Add new PHY's mode to be used by Ethernet PHY interface drivers or
> multipurpose PHYs like serdes and convert ocelot-serdes and mvebu-cp110-comphy
> PHY drivers to use recently introduced PHY_MODE_ETHERNET and phy_set_mode_ext().
> 
> Patch 5 - removes unused, ethernet specific phy modes from enum phy_mode.
> 
> Testing:
>  - series tested on TI am335x/am437x/am5(dra7) paltforms.
>  - other driver build tested.

I realise I'm a bit late to the party, but while working on the D-PHY
support, I noticed a few things that could be improved.

I guess the main issue is that the sub-mode is completely opaque to
the generic phy framework now. This might not be a big issue, and I
assume that it has been done that way because the net framework
already has a define for the submode it wants.

However, this creates a bunch of drawbacks at the phy framework level:

 - phy_set_mode will now pass a submode of 0, all the time. This means
   that the behaviour is undefined for all the modes not using the
   submodes at the moment, and phy_interface_t seems to have the value
   PHY_INTERFACE_MODE_NA matching 0, but I guess this could change in
   the future (or the guarantee is not documented anywhere).
 - on a similar note, there's no documentation for which value to pass
   to phy_set_mode_ext when used with something else than a
   PHY_MODE_ETHERNET.
 - at the provider level, if you're supporting a phy that isn't using
   the submodes, you have no way to filter out or reject any subnode,
   since you have no idea what the "no submode" value is.

I guess this can be addressed by:

A) defining a generic phy framework wide unused / invalid phy submode,
that wouldn't collide with the subnode values (such as -1?), and
making phy_set_mode_ext use that.

B) moving the phy submodes definition to the generic phy headers. This
would allow to have a documented, obvious link between a mode and its
subnodes, for all the actors involved (consumer, provider, and
framework) without prior knowledge.

C) Document what the submodes expectations are

What do you think?
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2018-12-17 14:06 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-20  1:24 [PATCH v3 0/5] phy: core: rework phy_set_mode to accept phy mode and submode Grygorii Strashko
2018-11-20  1:24 ` [PATCH v3 1/5] " Grygorii Strashko
2018-11-20  1:24 ` [PATCH v3 2/5] phy: core: add PHY_MODE_ETHERNET Grygorii Strashko
2018-11-20  1:24 ` [PATCH v3 3/5] phy: ocelot-serdes: convert to use eth phy mode and submode Grygorii Strashko
2018-11-20  9:18   ` Quentin Schulz
2018-11-20  1:24 ` [PATCH v3 4/5] phy: mvebu-cp110-comphy: " Grygorii Strashko
2018-11-21  7:38   ` Kishon Vijay Abraham I
2018-11-21  7:52     ` Quentin Schulz
2018-11-21  7:58       ` Antoine Tenart
2018-11-21  8:02   ` Antoine Tenart
2018-11-20  1:24 ` [PATCH v3 5/5] phy: core: clean up unused ethernet specific phy modes Grygorii Strashko
2018-11-21  8:55 ` [PATCH v3 0/5] phy: core: rework phy_set_mode to accept phy mode and submode Kishon Vijay Abraham I
2018-11-21 18:23   ` Grygorii Strashko
2018-12-17 14:06 ` Maxime Ripard

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).