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

[Stefan Roese]

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 at 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