diff mbox series

[net-next,v8,03/11] net: stmmac: dwmac-loongson: Add full PCI support

Message ID bfaed8692d6e03bbb53100d4e3695aa4a9f92633.1706601050.git.siyanteng@loongson.cn (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series stmmac: Add Loongson platform support | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1048 this patch: 1048
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 0 of 0 maintainers
netdev/build_clang success Errors and warnings before: 1065 this patch: 1065
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1065 this patch: 1065
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 120 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Yanteng Si Jan. 30, 2024, 8:43 a.m. UTC 
Current dwmac-loongson only support LS2K in the "probed with PCI and
configured with DT" manner. Add LS7A support on which the devices are
fully PCI (non-DT).

Signed-off-by: Yanteng Si <siyanteng@loongson.cn>
Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn>
Signed-off-by: Yinggang Gu <guyinggang@loongson.cn>
---
 .../ethernet/stmicro/stmmac/dwmac-loongson.c  | 79 +++++++++++--------
 1 file changed, 44 insertions(+), 35 deletions(-)

Comments

Serge Semin Feb. 5, 2024, 4:49 p.m. UTC | #1 
On Tue, Jan 30, 2024 at 04:43:23PM +0800, Yanteng Si wrote:
> Current dwmac-loongson only support LS2K in the "probed with PCI and
> configured with DT" manner. Add LS7A support on which the devices are
> fully PCI (non-DT).

Please add to the commit log more details of what LS7A is like and
bind that description to the changes below like the interface
settings, ref and PTP clock settings, etc. What is the difference
between LS2K and LS7A?

> 
> Signed-off-by: Yanteng Si <siyanteng@loongson.cn>
> Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn>
> Signed-off-by: Yinggang Gu <guyinggang@loongson.cn>
> ---
>  .../ethernet/stmicro/stmmac/dwmac-loongson.c  | 79 +++++++++++--------
>  1 file changed, 44 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> index e2dcb339b8b0..979c9b6dab3f 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> @@ -16,6 +16,10 @@ struct stmmac_pci_info {
>  static void loongson_default_data(struct pci_dev *pdev,
>  				  struct plat_stmmacenet_data *plat)
>  {
> +	/* Get bus_id, this can be overloaded later */
> +	plat->bus_id = (pci_domain_nr(pdev->bus) << 16) |
> +		       PCI_DEVID(pdev->bus->number, pdev->devfn);
> +
>  	plat->clk_csr = 2;	/* clk_csr_i = 20-35MHz & MDC = clk_csr_i/16 */
>  	plat->has_gmac = 1;
>  	plat->force_sf_dma_mode = 1;
> @@ -51,10 +55,14 @@ static int loongson_gmac_data(struct pci_dev *pdev,
>  	plat->mdio_bus_data->phy_mask = 0;
>  
>  	plat->phy_addr = -1;
> +	plat->phy_interface = PHY_INTERFACE_MODE_RGMII_ID;
>  
>  	plat->dma_cfg->pbl = 32;
>  	plat->dma_cfg->pblx8 = true;
>  

> +	plat->clk_ref_rate = 125000000;
> +	plat->clk_ptp_rate = 125000000;
> +

Is this compatible with the LS2K GMAC?

>  	return 0;
>  }
>  
> @@ -71,13 +79,6 @@ static int loongson_dwmac_probe(struct pci_dev *pdev,
>  	struct stmmac_resources res;
>  	struct device_node *np;
>  
> -	np = dev_of_node(&pdev->dev);
> -
> -	if (!np) {
> -		pr_info("dwmac_loongson_pci: No OF node\n");
> -		return -ENODEV;
> -	}
> -
>  	plat = devm_kzalloc(&pdev->dev, sizeof(*plat), GFP_KERNEL);
>  	if (!plat)
>  		return -ENOMEM;
> @@ -93,6 +94,7 @@ static int loongson_dwmac_probe(struct pci_dev *pdev,
>  	if (!plat->dma_cfg)
>  		return -ENOMEM;
>  
> +	np = dev_of_node(&pdev->dev);

>  	plat->mdio_node = of_get_child_by_name(np, "mdio");
>  	if (plat->mdio_node) {
>  		dev_info(&pdev->dev, "Found MDIO subnode\n");

Shouldn't mdio_node setup being done under the "if (np)" clause?

> @@ -123,41 +125,48 @@ static int loongson_dwmac_probe(struct pci_dev *pdev,
>  	if (ret)
>  		goto err_disable_device;
>  
> -	bus_id = of_alias_get_id(np, "ethernet");
> -	if (bus_id >= 0)
> -		plat->bus_id = bus_id;

> +	if (np) {
> +		bus_id = of_alias_get_id(np, "ethernet");
> +		if (bus_id >= 0)
> +			plat->bus_id = bus_id;
>  
> -	phy_mode = device_get_phy_mode(&pdev->dev);
> -	if (phy_mode < 0) {
> -		dev_err(&pdev->dev, "phy_mode not found\n");
> -		ret = phy_mode;
> -		goto err_disable_device;
> +		phy_mode = device_get_phy_mode(&pdev->dev);
> +		if (phy_mode < 0) {
> +			dev_err(&pdev->dev, "phy_mode not found\n");
> +			ret = phy_mode;
> +			goto err_disable_device;
> +		}
> +		plat->phy_interface = phy_mode;
>  	}

Please collect all OF-specific code in the same "if (np)" clause if
possible.

>  
> -	plat->phy_interface = phy_mode;
> -
>  	pci_enable_msi(pdev);
>  	memset(&res, 0, sizeof(res));
>  	res.addr = pcim_iomap_table(pdev)[0];
>  
> -	res.irq = of_irq_get_byname(np, "macirq");
> -	if (res.irq < 0) {
> -		dev_err(&pdev->dev, "IRQ macirq not found\n");
> -		ret = -ENODEV;
> -		goto err_disable_msi;
> -	}
> -
> -	res.wol_irq = of_irq_get_byname(np, "eth_wake_irq");
> -	if (res.wol_irq < 0) {
> -		dev_info(&pdev->dev, "IRQ eth_wake_irq not found, using macirq\n");
> -		res.wol_irq = res.irq;
> -	}
> -
> -	res.lpi_irq = of_irq_get_byname(np, "eth_lpi");
> -	if (res.lpi_irq < 0) {
> -		dev_err(&pdev->dev, "IRQ eth_lpi not found\n");
> -		ret = -ENODEV;
> -		goto err_disable_msi;
> +	if (np) {
> +		res.irq = of_irq_get_byname(np, "macirq");
> +		if (res.irq < 0) {
> +			dev_err(&pdev->dev, "IRQ macirq not found\n");
> +			ret = -ENODEV;
> +			goto err_disable_msi;
> +		}
> +
> +		res.wol_irq = of_irq_get_byname(np, "eth_wake_irq");
> +		if (res.wol_irq < 0) {
> +			dev_info(&pdev->dev,
> +				 "IRQ eth_wake_irq not found, using macirq\n");
> +			res.wol_irq = res.irq;
> +		}
> +
> +		res.lpi_irq = of_irq_get_byname(np, "eth_lpi");
> +		if (res.lpi_irq < 0) {
> +			dev_err(&pdev->dev, "IRQ eth_lpi not found\n");
> +			ret = -ENODEV;
> +			goto err_disable_msi;
> +		}
> +	} else {
> +		res.irq = pdev->irq;

> +		res.wol_irq = pdev->irq;

This seems redundant. If res.wol_irq matches res_irq it won't be used
(see stmmac_request_irq_multi_msi() and stmmac_request_irq_single()).
What about dropping it?

-Serge(y)

>  	}
>  
>  	ret = stmmac_dvr_probe(&pdev->dev, plat, &res);
> -- 
> 2.31.4
>
Yanteng Si Feb. 21, 2024, 10:08 a.m. UTC | #2 
在 2024/2/6 00:49, Serge Semin 写道:
> On Tue, Jan 30, 2024 at 04:43:23PM +0800, Yanteng Si wrote:
>> Current dwmac-loongson only support LS2K in the "probed with PCI and
>> configured with DT" manner. Add LS7A support on which the devices are
>> fully PCI (non-DT).
> Please add to the commit log more details of what LS7A is like and
> bind that description to the changes below like the interface
> settings, ref and PTP clock settings, etc. What is the difference
> between LS2K and LS7A?
OK,
>
>> Signed-off-by: Yanteng Si <siyanteng@loongson.cn>
>> Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn>
>> Signed-off-by: Yinggang Gu <guyinggang@loongson.cn>
>> ---
>>   .../ethernet/stmicro/stmmac/dwmac-loongson.c  | 79 +++++++++++--------
>>   1 file changed, 44 insertions(+), 35 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
>> index e2dcb339b8b0..979c9b6dab3f 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
>> @@ -16,6 +16,10 @@ struct stmmac_pci_info {
>>   static void loongson_default_data(struct pci_dev *pdev,
>>   				  struct plat_stmmacenet_data *plat)
>>   {
>> +	/* Get bus_id, this can be overloaded later */
>> +	plat->bus_id = (pci_domain_nr(pdev->bus) << 16) |
>> +		       PCI_DEVID(pdev->bus->number, pdev->devfn);
>> +
>>   	plat->clk_csr = 2;	/* clk_csr_i = 20-35MHz & MDC = clk_csr_i/16 */
>>   	plat->has_gmac = 1;
>>   	plat->force_sf_dma_mode = 1;
>> @@ -51,10 +55,14 @@ static int loongson_gmac_data(struct pci_dev *pdev,
>>   	plat->mdio_bus_data->phy_mask = 0;
>>   
>>   	plat->phy_addr = -1;
>> +	plat->phy_interface = PHY_INTERFACE_MODE_RGMII_ID;
>>   
>>   	plat->dma_cfg->pbl = 32;
>>   	plat->dma_cfg->pblx8 = true;
>>   
>> +	plat->clk_ref_rate = 125000000;
>> +	plat->clk_ptp_rate = 125000000;
>> +
> Is this compatible with the LS2K GMAC?
Of course!
>
>>   	return 0;
>>   }
>>   
>> @@ -71,13 +79,6 @@ static int loongson_dwmac_probe(struct pci_dev *pdev,
>>   	struct stmmac_resources res;
>>   	struct device_node *np;
>>   
>> -	np = dev_of_node(&pdev->dev);
>> -
>> -	if (!np) {
>> -		pr_info("dwmac_loongson_pci: No OF node\n");
>> -		return -ENODEV;
>> -	}
>> -
>>   	plat = devm_kzalloc(&pdev->dev, sizeof(*plat), GFP_KERNEL);
>>   	if (!plat)
>>   		return -ENOMEM;
>> @@ -93,6 +94,7 @@ static int loongson_dwmac_probe(struct pci_dev *pdev,
>>   	if (!plat->dma_cfg)
>>   		return -ENOMEM;
>>   
>> +	np = dev_of_node(&pdev->dev);
>>   	plat->mdio_node = of_get_child_by_name(np, "mdio");
>>   	if (plat->mdio_node) {
>>   		dev_info(&pdev->dev, "Found MDIO subnode\n");
> Shouldn't mdio_node setup being done under the "if (np)" clause?
Yes, they will be moved there.
>
>> @@ -123,41 +125,48 @@ static int loongson_dwmac_probe(struct pci_dev *pdev,
>>   	if (ret)
>>   		goto err_disable_device;
>>   
>> -	bus_id = of_alias_get_id(np, "ethernet");
>> -	if (bus_id >= 0)
>> -		plat->bus_id = bus_id;
>> +	if (np) {
>> +		bus_id = of_alias_get_id(np, "ethernet");
>> +		if (bus_id >= 0)
>> +			plat->bus_id = bus_id;
>>   
>> -	phy_mode = device_get_phy_mode(&pdev->dev);
>> -	if (phy_mode < 0) {
>> -		dev_err(&pdev->dev, "phy_mode not found\n");
>> -		ret = phy_mode;
>> -		goto err_disable_device;
>> +		phy_mode = device_get_phy_mode(&pdev->dev);
>> +		if (phy_mode < 0) {
>> +			dev_err(&pdev->dev, "phy_mode not found\n");
>> +			ret = phy_mode;
>> +			goto err_disable_device;
>> +		}
>> +		plat->phy_interface = phy_mode;
>>   	}
> Please collect all OF-specific code in the same "if (np)" clause if
> possible.
OK,
>
>>   
>> -	plat->phy_interface = phy_mode;	np = dev_of_node(&pdev->dev);
>> -
>>   	pci_enable_msi(pdev);
>>   	memset(&res, 0, sizeof(res));
>>   	res.addr = pcim_iomap_table(pdev)[0];
>>   
>> -	res.irq = of_irq_get_byname(np, "macirq");
>> -	if (res.irq < 0) {
>> -		dev_err(&pdev->dev, "IRQ macirq not found\n");
>> -		ret = -ENODEV;
>> -		goto err_disable_msi;
>> -	}
>> -
>> -	res.wol_irq = of_irq_get_byname(np, "eth_wake_irq");
>> -	if (res.wol_irq < 0) {
>> -		dev_info(&pdev->dev, "IRQ eth_wake_irq not found, using macirq\n");
>> -		res.wol_irq = res.irq;
>> -	}
>> -
>> -	res.lpi_irq = of_irq_get_byname(np, "eth_lpi");
>> -	if (res.lpi_irq < 0) {
>> -		dev_err(&pdev->dev, "IRQ eth_lpi not found\n");
>> -		ret = -ENODEV;
>> -		goto err_disable_msi;
>> +	if (np) {
>> +		res.irq = of_irq_get_byname(np, "macirq");
>> +		if (res.irq < 0) {
>> +			dev_err(&pdev->dev, "IRQ macirq not found\n");
>> +			ret = -ENODEV;
>> +			goto err_disable_msi;
>> +		}
>> +
>> +		res.wol_irq = of_irq_get_byname(np, "eth_wake_irq");
>> +		if (res.wol_irq < 0) {
>> +			dev_info(&pdev->dev,
>> +				 "IRQ eth_wake_irq not found, using macirq\n");
>> +			res.wol_irq = res.irq;
>> +		}
>> +
>> +		res.lpi_irq = of_irq_get_byname(np, "eth_lpi");
>> +		if (res.lpi_irq < 0) {
>> +			dev_err(&pdev->dev, "IRQ eth_lpi not found\n");
>> +			ret = -ENODEV;
>> +			goto err_disable_msi;
>> +		}
>> +	} else {
>> +		res.irq = pdev->irq;
>> +		res.wol_irq = pdev->irq;
> This seems redundant. If res.wol_irq matches res_irq it won't be used
> (see stmmac_request_irq_multi_msi() and stmmac_request_irq_single()).
> What about dropping it?

Okay, actually, it was removed by subsequent patches.


Thanks,

Yanteng

>
> -Serge(y)
>
>>   	}
>>   
>>   	ret = stmmac_dvr_probe(&pdev->dev, plat, &res);
>> -- 
>> 2.31.4
>>
diff mbox series

Patch

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
index e2dcb339b8b0..979c9b6dab3f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
@@ -16,6 +16,10 @@  struct stmmac_pci_info {
 static void loongson_default_data(struct pci_dev *pdev,
 				  struct plat_stmmacenet_data *plat)
 {
+	/* Get bus_id, this can be overloaded later */
+	plat->bus_id = (pci_domain_nr(pdev->bus) << 16) |
+		       PCI_DEVID(pdev->bus->number, pdev->devfn);
+
 	plat->clk_csr = 2;	/* clk_csr_i = 20-35MHz & MDC = clk_csr_i/16 */
 	plat->has_gmac = 1;
 	plat->force_sf_dma_mode = 1;
@@ -51,10 +55,14 @@  static int loongson_gmac_data(struct pci_dev *pdev,
 	plat->mdio_bus_data->phy_mask = 0;
 
 	plat->phy_addr = -1;
+	plat->phy_interface = PHY_INTERFACE_MODE_RGMII_ID;
 
 	plat->dma_cfg->pbl = 32;
 	plat->dma_cfg->pblx8 = true;
 
+	plat->clk_ref_rate = 125000000;
+	plat->clk_ptp_rate = 125000000;
+
 	return 0;
 }
 
@@ -71,13 +79,6 @@  static int loongson_dwmac_probe(struct pci_dev *pdev,
 	struct stmmac_resources res;
 	struct device_node *np;
 
-	np = dev_of_node(&pdev->dev);
-
-	if (!np) {
-		pr_info("dwmac_loongson_pci: No OF node\n");
-		return -ENODEV;
-	}
-
 	plat = devm_kzalloc(&pdev->dev, sizeof(*plat), GFP_KERNEL);
 	if (!plat)
 		return -ENOMEM;
@@ -93,6 +94,7 @@  static int loongson_dwmac_probe(struct pci_dev *pdev,
 	if (!plat->dma_cfg)
 		return -ENOMEM;
 
+	np = dev_of_node(&pdev->dev);
 	plat->mdio_node = of_get_child_by_name(np, "mdio");
 	if (plat->mdio_node) {
 		dev_info(&pdev->dev, "Found MDIO subnode\n");
@@ -123,41 +125,48 @@  static int loongson_dwmac_probe(struct pci_dev *pdev,
 	if (ret)
 		goto err_disable_device;
 
-	bus_id = of_alias_get_id(np, "ethernet");
-	if (bus_id >= 0)
-		plat->bus_id = bus_id;
+	if (np) {
+		bus_id = of_alias_get_id(np, "ethernet");
+		if (bus_id >= 0)
+			plat->bus_id = bus_id;
 
-	phy_mode = device_get_phy_mode(&pdev->dev);
-	if (phy_mode < 0) {
-		dev_err(&pdev->dev, "phy_mode not found\n");
-		ret = phy_mode;
-		goto err_disable_device;
+		phy_mode = device_get_phy_mode(&pdev->dev);
+		if (phy_mode < 0) {
+			dev_err(&pdev->dev, "phy_mode not found\n");
+			ret = phy_mode;
+			goto err_disable_device;
+		}
+		plat->phy_interface = phy_mode;
 	}
 
-	plat->phy_interface = phy_mode;
-
 	pci_enable_msi(pdev);
 	memset(&res, 0, sizeof(res));
 	res.addr = pcim_iomap_table(pdev)[0];
 
-	res.irq = of_irq_get_byname(np, "macirq");
-	if (res.irq < 0) {
-		dev_err(&pdev->dev, "IRQ macirq not found\n");
-		ret = -ENODEV;
-		goto err_disable_msi;
-	}
-
-	res.wol_irq = of_irq_get_byname(np, "eth_wake_irq");
-	if (res.wol_irq < 0) {
-		dev_info(&pdev->dev, "IRQ eth_wake_irq not found, using macirq\n");
-		res.wol_irq = res.irq;
-	}
-
-	res.lpi_irq = of_irq_get_byname(np, "eth_lpi");
-	if (res.lpi_irq < 0) {
-		dev_err(&pdev->dev, "IRQ eth_lpi not found\n");
-		ret = -ENODEV;
-		goto err_disable_msi;
+	if (np) {
+		res.irq = of_irq_get_byname(np, "macirq");
+		if (res.irq < 0) {
+			dev_err(&pdev->dev, "IRQ macirq not found\n");
+			ret = -ENODEV;
+			goto err_disable_msi;
+		}
+
+		res.wol_irq = of_irq_get_byname(np, "eth_wake_irq");
+		if (res.wol_irq < 0) {
+			dev_info(&pdev->dev,
+				 "IRQ eth_wake_irq not found, using macirq\n");
+			res.wol_irq = res.irq;
+		}
+
+		res.lpi_irq = of_irq_get_byname(np, "eth_lpi");
+		if (res.lpi_irq < 0) {
+			dev_err(&pdev->dev, "IRQ eth_lpi not found\n");
+			ret = -ENODEV;
+			goto err_disable_msi;
+		}
+	} else {
+		res.irq = pdev->irq;
+		res.wol_irq = pdev->irq;
 	}
 
 	ret = stmmac_dvr_probe(&pdev->dev, plat, &res);