[U-Boot,1/4] phy: marvell: Support changing SERDES map in board file

Message ID 20180516143942.2309-2-marek.behun@nic.cz
State New
Delegated to: Stefan Roese
Headers show
Series
  • Updates for Turris Mox
Related show

Commit Message

Marek BehĂșn May 16, 2018, 2:39 p.m.
This adds a weak definition of board_update_comphy_map to comphy_core,
which does nothing. If this function is defined elsewhere, for example
in board file, the board file can change some parameters of SERDES
configuration.

This is needed on Turris Mox, where the SERDES speed on lane 1 has to
be set differently when SFP module is connected and when Topaz Switch
module is connected.

This is a temporary solution. When the comphy driver for armada-3720
will be added to the kernel, the comphy driver in u-boot shall also be
updated and this should be done differently then.

Signed-off-by: Marek Behun <marek.behun@nic.cz>

 rename drivers/phy/marvell/{comphy.h => comphy_core.h} (96%)
 create mode 100644 include/comphy.h

Comments

Stefan Roese May 17, 2018, 9:26 a.m. | #1
On 16.05.2018 16:39, Marek BehĂșn wrote:
> This adds a weak definition of board_update_comphy_map to comphy_core,
> which does nothing. If this function is defined elsewhere, for example
> in board file, the board file can change some parameters of SERDES
> configuration.
> 
> This is needed on Turris Mox, where the SERDES speed on lane 1 has to
> be set differently when SFP module is connected and when Topaz Switch
> module is connected.
> 
> This is a temporary solution. When the comphy driver for armada-3720
> will be added to the kernel, the comphy driver in u-boot shall also be
> updated and this should be done differently then.
> 
> Signed-off-by: Marek Behun <marek.behun@nic.cz>
> 
>   rename drivers/phy/marvell/{comphy.h => comphy_core.h} (96%)
>   create mode 100644 include/comphy.h
> 
> diff --git a/drivers/phy/marvell/comphy_a3700.h b/drivers/phy/marvell/comphy_a3700.h
> index a14767d809..b0941ffb37 100644
> --- a/drivers/phy/marvell/comphy_a3700.h
> +++ b/drivers/phy/marvell/comphy_a3700.h
> @@ -6,7 +6,7 @@
>   #ifndef _COMPHY_A3700_H_
>   #define _COMPHY_A3700_H_
>   
> -#include "comphy.h"
> +#include "comphy_core.h"
>   #include "comphy_hpipe.h"
>   
>   #define MVEBU_REG(offs)			\
> diff --git a/drivers/phy/marvell/comphy_core.c b/drivers/phy/marvell/comphy_core.c
> index c6e2cc8897..74b9f11b08 100644
> --- a/drivers/phy/marvell/comphy_core.c
> +++ b/drivers/phy/marvell/comphy_core.c
> @@ -11,7 +11,7 @@
>   #include <linux/errno.h>
>   #include <asm/io.h>
>   
> -#include "comphy.h"
> +#include "comphy_core.h"
>   
>   #define COMPHY_MAX_CHIP 4
>   
> @@ -66,6 +66,10 @@ void comphy_print(struct chip_serdes_phy_config *chip_cfg,
>   	}
>   }
>   
> +__weak void board_update_comphy_map(struct comphy_map *serdes_map, int count)
> +{
> +}
> +

Perhaps its better to move "comphy_" to the beginning of this
function name, like:

comphy_update_map()

What do you think?

>   static int comphy_probe(struct udevice *dev)
>   {
>   	const void *blob = gd->fdt_blob;
> @@ -143,6 +147,8 @@ static int comphy_probe(struct udevice *dev)
>   		lane++;
>   	}
>   
> +	board_update_comphy_map(comphy_map_data, chip_cfg->comphy_lanes_count);
> +

I would prefer to add a return code this this function and bail
out here, if something goes wrong.

>   	/* Save CP index for MultiCP devices (A8K) */
>   	chip_cfg->cp_index = current_idx++;
>   	/* PHY power UP sequence */
> diff --git a/drivers/phy/marvell/comphy.h b/drivers/phy/marvell/comphy_core.h
> similarity index 96%
> rename from drivers/phy/marvell/comphy.h
> rename to drivers/phy/marvell/comphy_core.h
> index b588ae41f0..e1da90e75b 100644
> --- a/drivers/phy/marvell/comphy.h
> +++ b/drivers/phy/marvell/comphy_core.h
> @@ -3,11 +3,11 @@
>    * Copyright (C) 2015-2016 Marvell International Ltd.
>    */
>   
> -#ifndef _COMPHY_H_
> -#define _COMPHY_H_
> +#ifndef _COMPHY_CORE_H_
> +#define _COMPHY_CORE_H_
>   
> -#include <dt-bindings/comphy/comphy_data.h>
>   #include <fdtdec.h>
> +#include <comphy.h>
>   
>   #if defined(DEBUG)
>   #define debug_enter()	printf("----> Enter %s\n", __func__);
> @@ -80,14 +80,6 @@ struct comphy_mux_data {
>   	struct comphy_mux_options mux_values[MAX_LANE_OPTIONS];
>   };
>   
> -struct comphy_map {
> -	u32 type;
> -	u32 speed;
> -	u32 invert;
> -	bool clk_src;
> -	bool end_point;
> -};
> -
>   struct chip_serdes_phy_config {
>   	struct comphy_mux_data *mux_data;
>   	int (*ptr_comphy_chip_init)(struct chip_serdes_phy_config *,
> @@ -183,5 +175,5 @@ void comphy_pcie_config_detect(u32 comphy_max_count,
>   			       struct comphy_map *serdes_map);
>   void comphy_pcie_unit_general_config(u32 pex_index);
>   
> -#endif /* _COMPHY_H_ */
> +#endif /* _COMPHY_CORE_H_ */
>   
> diff --git a/drivers/phy/marvell/comphy_cp110.c b/drivers/phy/marvell/comphy_cp110.c
> index b0d5d5ca26..6a60da3df0 100644
> --- a/drivers/phy/marvell/comphy_cp110.c
> +++ b/drivers/phy/marvell/comphy_cp110.c
> @@ -9,7 +9,7 @@
>   #include <asm/arch/cpu.h>
>   #include <asm/arch/soc.h>
>   
> -#include "comphy.h"
> +#include "comphy_core.h"
>   #include "comphy_hpipe.h"
>   #include "sata.h"
>   #include "utmi_phy.h"
> diff --git a/drivers/phy/marvell/comphy_mux.c b/drivers/phy/marvell/comphy_mux.c
> index 1f757d8e04..c67ba99762 100644
> --- a/drivers/phy/marvell/comphy_mux.c
> +++ b/drivers/phy/marvell/comphy_mux.c
> @@ -6,7 +6,7 @@
>   #include <common.h>
>   #include <asm/io.h>
>   
> -#include "comphy.h"
> +#include "comphy_core.h"
>   #include "comphy_hpipe.h"
>   
>   /*
> diff --git a/include/comphy.h b/include/comphy.h
> new file mode 100644
> index 0000000000..2ebb50d418
> --- /dev/null
> +++ b/include/comphy.h
> @@ -0,0 +1,22 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (C) 2015-2016 Marvell International Ltd.
> + */
> +
> +#ifndef _COMPHY_H_
> +#define _COMPHY_H_
> +
> +#include <dt-bindings/comphy/comphy_data.h>
> +
> +struct comphy_map {
> +	u32 type;
> +	u32 speed;
> +	u32 invert;
> +	bool clk_src;
> +	bool end_point;
> +};
> +
> +void board_update_comphy_map(struct comphy_map *serdes_map, int count);
> +
> +#endif /* _COMPHY_H_ */

I'm not so happy with "polluting" the common "include" directory with
board / platform specific files. Please at least make this file name
platform specific, like "mvebu_comphy.h". Or add a mvebu subdirectory
here.

Thanks,
Stefan

Patch

diff --git a/drivers/phy/marvell/comphy_a3700.h b/drivers/phy/marvell/comphy_a3700.h
index a14767d809..b0941ffb37 100644
--- a/drivers/phy/marvell/comphy_a3700.h
+++ b/drivers/phy/marvell/comphy_a3700.h
@@ -6,7 +6,7 @@ 
 #ifndef _COMPHY_A3700_H_
 #define _COMPHY_A3700_H_
 
-#include "comphy.h"
+#include "comphy_core.h"
 #include "comphy_hpipe.h"
 
 #define MVEBU_REG(offs)			\
diff --git a/drivers/phy/marvell/comphy_core.c b/drivers/phy/marvell/comphy_core.c
index c6e2cc8897..74b9f11b08 100644
--- a/drivers/phy/marvell/comphy_core.c
+++ b/drivers/phy/marvell/comphy_core.c
@@ -11,7 +11,7 @@ 
 #include <linux/errno.h>
 #include <asm/io.h>
 
-#include "comphy.h"
+#include "comphy_core.h"
 
 #define COMPHY_MAX_CHIP 4
 
@@ -66,6 +66,10 @@  void comphy_print(struct chip_serdes_phy_config *chip_cfg,
 	}
 }
 
+__weak void board_update_comphy_map(struct comphy_map *serdes_map, int count)
+{
+}
+
 static int comphy_probe(struct udevice *dev)
 {
 	const void *blob = gd->fdt_blob;
@@ -143,6 +147,8 @@  static int comphy_probe(struct udevice *dev)
 		lane++;
 	}
 
+	board_update_comphy_map(comphy_map_data, chip_cfg->comphy_lanes_count);
+
 	/* Save CP index for MultiCP devices (A8K) */
 	chip_cfg->cp_index = current_idx++;
 	/* PHY power UP sequence */
diff --git a/drivers/phy/marvell/comphy.h b/drivers/phy/marvell/comphy_core.h
similarity index 96%
rename from drivers/phy/marvell/comphy.h
rename to drivers/phy/marvell/comphy_core.h
index b588ae41f0..e1da90e75b 100644
--- a/drivers/phy/marvell/comphy.h
+++ b/drivers/phy/marvell/comphy_core.h
@@ -3,11 +3,11 @@ 
  * Copyright (C) 2015-2016 Marvell International Ltd.
  */
 
-#ifndef _COMPHY_H_
-#define _COMPHY_H_
+#ifndef _COMPHY_CORE_H_
+#define _COMPHY_CORE_H_
 
-#include <dt-bindings/comphy/comphy_data.h>
 #include <fdtdec.h>
+#include <comphy.h>
 
 #if defined(DEBUG)
 #define debug_enter()	printf("----> Enter %s\n", __func__);
@@ -80,14 +80,6 @@  struct comphy_mux_data {
 	struct comphy_mux_options mux_values[MAX_LANE_OPTIONS];
 };
 
-struct comphy_map {
-	u32 type;
-	u32 speed;
-	u32 invert;
-	bool clk_src;
-	bool end_point;
-};
-
 struct chip_serdes_phy_config {
 	struct comphy_mux_data *mux_data;
 	int (*ptr_comphy_chip_init)(struct chip_serdes_phy_config *,
@@ -183,5 +175,5 @@  void comphy_pcie_config_detect(u32 comphy_max_count,
 			       struct comphy_map *serdes_map);
 void comphy_pcie_unit_general_config(u32 pex_index);
 
-#endif /* _COMPHY_H_ */
+#endif /* _COMPHY_CORE_H_ */
 
diff --git a/drivers/phy/marvell/comphy_cp110.c b/drivers/phy/marvell/comphy_cp110.c
index b0d5d5ca26..6a60da3df0 100644
--- a/drivers/phy/marvell/comphy_cp110.c
+++ b/drivers/phy/marvell/comphy_cp110.c
@@ -9,7 +9,7 @@ 
 #include <asm/arch/cpu.h>
 #include <asm/arch/soc.h>
 
-#include "comphy.h"
+#include "comphy_core.h"
 #include "comphy_hpipe.h"
 #include "sata.h"
 #include "utmi_phy.h"
diff --git a/drivers/phy/marvell/comphy_mux.c b/drivers/phy/marvell/comphy_mux.c
index 1f757d8e04..c67ba99762 100644
--- a/drivers/phy/marvell/comphy_mux.c
+++ b/drivers/phy/marvell/comphy_mux.c
@@ -6,7 +6,7 @@ 
 #include <common.h>
 #include <asm/io.h>
 
-#include "comphy.h"
+#include "comphy_core.h"
 #include "comphy_hpipe.h"
 
 /*
diff --git a/include/comphy.h b/include/comphy.h
new file mode 100644
index 0000000000..2ebb50d418
--- /dev/null
+++ b/include/comphy.h
@@ -0,0 +1,22 @@ 
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (C) 2015-2016 Marvell International Ltd.
+ */
+
+#ifndef _COMPHY_H_
+#define _COMPHY_H_
+
+#include <dt-bindings/comphy/comphy_data.h>
+
+struct comphy_map {
+	u32 type;
+	u32 speed;
+	u32 invert;
+	bool clk_src;
+	bool end_point;
+};
+
+void board_update_comphy_map(struct comphy_map *serdes_map, int count);
+
+#endif /* _COMPHY_H_ */
+