Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v12 2/4] phy: Add ethernet serdes configuration option
To:     Steen Hegelund <steen.hegelund@microchip.com>,
        Vinod Koul <vkoul@kernel.org>
CC:     Alexandre Belloni <alexandre.belloni@bootlin.com>,
        Lars Povlsen <lars.povlsen@microchip.com>,
        Bjarni Jonasson <bjarni.jonasson@microchip.com>,
        Microchip UNG Driver List <UNGLinuxDriver@microchip.com>,
        <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        Andrew Lunn <andrew@lunn.ch>
References: <20210107091924.1569575-1-steen.hegelund@microchip.com>
 <20210107091924.1569575-3-steen.hegelund@microchip.com>
From:   Kishon Vijay Abraham I <kishon@ti.com>
Message-ID: <41e40e7b-6a3a-cdb9-adfc-e42f6627ea7b@ti.com>
Date:   Fri, 15 Jan 2021 14:14:17 +0530
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <20210107091924.1569575-3-steen.hegelund@microchip.com>
Content-Type: text/plain; charset="utf-8"
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi,

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

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

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

Thanks
Kishon

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