lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Fri, 27 Apr 2018 11:09:59 +0800
From:   Yixun Lan <yixun.lan@...ogic.com>
To:     Jerome Brunet <jbrunet@...libre.com>,
        "David S. Miller" <davem@...emloft.net>, <netdev@...r.kernel.org>
CC:     <yixun.lan@...ogic.com>, Kevin Hilman <khilman@...libre.com>,
        Carlo Caione <carlo@...one.org>, Rob Herring <robh@...nel.org>,
        Martin Blumenstingl <martin.blumenstingl@...glemail.com>,
        <linux-amlogic@...ts.infradead.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] net: stmmac: dwmac-meson: extend phy mode setting

Hi Jerome
On 04/26/18 16:47, Jerome Brunet wrote:
> On Thu, 2018-04-26 at 16:05 +0000, Yixun Lan wrote:
>>   In the Meson-AXG SoC, the phy mode setting of PRG_ETH0 in the glue layer
>> is extended from bit[0] to bit[2:0].
>>   There is no problem if we configure it to the RGMII 1000M PHY mode,
>> since the register setting is coincidentally compatible with previous one,
>> but for the RMII 100M PHY mode, the configuration need to be changed to
>> value - b100.
>>   This patch was verified with a RTL8201F 100M ethernet PHY.
>>
>> Signed-off-by: Yixun Lan <yixun.lan@...ogic.com>
>> ---
>>  .../ethernet/stmicro/stmmac/dwmac-meson8b.c   | 95 ++++++++++++++++---
>>  1 file changed, 84 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
>> index 7cb794094a70..e3688b6dd87c 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
>> @@ -18,6 +18,7 @@
>>  #include <linux/io.h>
>>  #include <linux/ioport.h>
>>  #include <linux/module.h>
>> +#include <linux/of_device.h>
>>  #include <linux/of_net.h>
>>  #include <linux/mfd/syscon.h>
>>  #include <linux/platform_device.h>
>> @@ -29,6 +30,10 @@
>>  
>>  #define PRG_ETH0_RGMII_MODE		BIT(0)
>>  
>> +#define PRG_ETH0_EXT_PHY_MODE_MASK	GENMASK(2, 0)
>> +#define PRG_ETH0_EXT_RGMII_MODE		1
>> +#define PRG_ETH0_EXT_RMII_MODE		4
>> +
>>  /* mux to choose between fclk_div2 (bit unset) and mpll2 (bit set) */
>>  #define PRG_ETH0_CLK_M250_SEL_SHIFT	4
>>  #define PRG_ETH0_CLK_M250_SEL_MASK	GENMASK(4, 4)
>> @@ -46,10 +51,16 @@
>>  #define PRG_ETH0_TX_AND_PHY_REF_CLK	BIT(12)
>>  
>>  #define MUX_CLK_NUM_PARENTS		2
>> +struct meson8b_dwmac_data {
>> +	bool ext_phy_mode;
>> +};
>>  
>>  struct meson8b_dwmac {
>>  	struct device		*dev;
>>  	void __iomem		*regs;
>> +
>> +	const struct meson8b_dwmac_data *data;
>> +
>>  	phy_interface_t		phy_mode;
>>  	struct clk		*rgmii_tx_clk;
>>  	u32			tx_delay_ns;
>> @@ -171,6 +182,46 @@ static int meson8b_init_rgmii_tx_clk(struct meson8b_dwmac *dwmac)
>>  	return 0;
>>  }
>>  
>> +static int meson8b_init_set_mode(struct meson8b_dwmac *dwmac)
>> +{
>> +	bool ext_phy_mode = dwmac->data->ext_phy_mode;
>> +
>> +	switch (dwmac->phy_mode) {
>> +	case PHY_INTERFACE_MODE_RGMII:
>> +	case PHY_INTERFACE_MODE_RGMII_RXID:
>> +	case PHY_INTERFACE_MODE_RGMII_ID:
>> +	case PHY_INTERFACE_MODE_RGMII_TXID:
>> +		/* enable RGMII mode */
>> +		if (ext_phy_mode)
> 
> Looks weird to have this if target at a specific SoC withing a function named
> after another SoC
> 
> Couldn't you make one function per soc type, and pass that function pointer in
> the match data ?
> 
sounds good, I can do this

>> +			meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
>> +						PRG_ETH0_EXT_PHY_MODE_MASK,
>> +						PRG_ETH0_EXT_RGMII_MODE);
>> +		else
>> +			meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
>> +						PRG_ETH0_RGMII_MODE,
>> +						PRG_ETH0_RGMII_MODE);
>> +
>> +		break;
>> +	case PHY_INTERFACE_MODE_RMII:
>> +		/* disable RGMII mode -> enables RMII mode */
>> +		if (ext_phy_mode)
>> +			meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
>> +						PRG_ETH0_EXT_PHY_MODE_MASK,
>> +						PRG_ETH0_EXT_RMII_MODE);
>> +		else
>> +			meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
>> +						PRG_ETH0_RGMII_MODE, 0);
>> +
>> +		break;
>> +	default:
>> +		dev_err(dwmac->dev, "fail to set phy-mode %s\n",
>> +			phy_modes(dwmac->phy_mode));
>> +		return -EINVAL;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>  static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
>>  {
>>  	int ret;
>> @@ -188,10 +239,6 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
>>  
>>  	case PHY_INTERFACE_MODE_RGMII_ID:
>>  	case PHY_INTERFACE_MODE_RGMII_TXID:
>> -		/* enable RGMII mode */
>> -		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_RGMII_MODE,
>> -					PRG_ETH0_RGMII_MODE);
>> -
>>  		/* only relevant for RMII mode -> disable in RGMII mode */
>>  		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
>>  					PRG_ETH0_INVERTED_RMII_CLK, 0);
>> @@ -224,10 +271,6 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
>>  		break;
>>  
>>  	case PHY_INTERFACE_MODE_RMII:
>> -		/* disable RGMII mode -> enables RMII mode */
>> -		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_RGMII_MODE,
>> -					0);
>> -
>>  		/* invert internal clk_rmii_i to generate 25/2.5 tx_rx_clk */
>>  		meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
>>  					PRG_ETH0_INVERTED_RMII_CLK,
>> @@ -274,6 +317,11 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
>>  		goto err_remove_config_dt;
>>  	}
>>  
>> +	dwmac->data = (const struct meson8b_dwmac_data *)
>> +		of_device_get_match_data(&pdev->dev);
>> +	if (!dwmac->data)
>> +		return -EINVAL;
>> +
>>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>>  	dwmac->regs = devm_ioremap_resource(&pdev->dev, res);
>>  	if (IS_ERR(dwmac->regs)) {
>> @@ -298,6 +346,10 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
>>  	if (ret)
>>  		goto err_remove_config_dt;
>>  
>> +	ret = meson8b_init_set_mode(dwmac);
>> +	if (ret)
>> +		goto err_remove_config_dt;
>> +
>>  	ret = meson8b_init_prg_eth(dwmac);
>>  	if (ret)
>>  		goto err_remove_config_dt;
>> @@ -316,10 +368,31 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
>>  	return ret;
>>  }
>>  
>> +static const struct meson8b_dwmac_data meson8b_dwmac_data = {
>> +	.ext_phy_mode = false,
>> +};
>> +
>> +static const struct meson8b_dwmac_data meson_axg_dwmac_data = {
>> +	.ext_phy_mode = true,
>> +};
>> +
>>  static const struct of_device_id meson8b_dwmac_match[] = {
>> -	{ .compatible = "amlogic,meson8b-dwmac" },
>> -	{ .compatible = "amlogic,meson8m2-dwmac" },
>> -	{ .compatible = "amlogic,meson-gxbb-dwmac" },
>> +	{
>> +		.compatible = "amlogic,meson8b-dwmac",
>> +		.data = &meson8b_dwmac_data,
>> +	},
>> +	{
>> +		.compatible = "amlogic,meson8m2-dwmac",
>> +		.data = &meson8b_dwmac_data,
>> +	},
>> +	{
>> +		.compatible = "amlogic,meson-gxbb-dwmac",
>> +		.data = &meson8b_dwmac_data,
>> +	},
>> +	{
>> +		.compatible = "amlogic,meson-axg-dwmac",
>> +		.data = &meson_axg_dwmac_data,
>> +	},
>>  	{ }
>>  };
>>  MODULE_DEVICE_TABLE(of, meson8b_dwmac_match);
> 
> .
> 

Powered by blists - more mailing lists