diff mbox series

[net-next,v8,02/11] net: stmmac: dwmac-loongson: Refactor code for loongson_dwmac_probe()

Message ID 6a66fdf816665c9d91c4611f47ffe3108b9bd39a.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, 118 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 
The driver function is not changed, but the code location is
adjusted to prepare for adding more loongson drivers.

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  | 61 +++++++++++++------
 1 file changed, 42 insertions(+), 19 deletions(-)

Comments

Simon Horman Feb. 2, 2024, 12:33 p.m. UTC | #1 
On Tue, Jan 30, 2024 at 04:43:22PM +0800, Yanteng Si wrote:
> The driver function is not changed, but the code location is
> adjusted to prepare for adding more loongson drivers.
> 
> Signed-off-by: Yanteng Si <siyanteng@loongson.cn>
> Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn>
> Signed-off-by: Yinggang Gu <guyinggang@loongson.cn>

...

> -static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> +static struct stmmac_pci_info loongson_gmac_pci_info = {
> +	.setup = loongson_gmac_data,
> +};
> +
> +static int loongson_dwmac_probe(struct pci_dev *pdev,
> +				const struct pci_device_id *id)
>  {
> +	int ret, i, bus_id, phy_mode;
>  	struct plat_stmmacenet_data *plat;
> +	struct stmmac_pci_info *info;
>  	struct stmmac_resources res;
>  	struct device_node *np;
> -	int ret, i, phy_mode;

nit: Please consider preserving reverse xmas tree order - longest line
     to shortest - for local variable declarations in Networking code.

This tool can be helpful here:
https://github.com/ecree-solarflare/xmastree

...
Yanteng Si Feb. 4, 2024, 8:47 a.m. UTC | #2 
在 2024/2/2 20:33, Simon Horman 写道:
> On Tue, Jan 30, 2024 at 04:43:22PM +0800, Yanteng Si wrote:
>> The driver function is not changed, but the code location is
>> adjusted to prepare for adding more loongson drivers.
>>
>> Signed-off-by: Yanteng Si <siyanteng@loongson.cn>
>> Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn>
>> Signed-off-by: Yinggang Gu <guyinggang@loongson.cn>
> ...
>
>> -static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>> +static struct stmmac_pci_info loongson_gmac_pci_info = {
>> +	.setup = loongson_gmac_data,
>> +};
>> +
>> +static int loongson_dwmac_probe(struct pci_dev *pdev,
>> +				const struct pci_device_id *id)
>>   {
>> +	int ret, i, bus_id, phy_mode;
>>   	struct plat_stmmacenet_data *plat;
>> +	struct stmmac_pci_info *info;
>>   	struct stmmac_resources res;
>>   	struct device_node *np;
>> -	int ret, i, phy_mode;
> nit: Please consider preserving reverse xmas tree order - longest line
>       to shortest - for local variable declarations in Networking code.
>
> This tool can be helpful here:
> https://github.com/ecree-solarflare/xmastree

Okey, thank you!


Thanks,

Yanteng

>
> ...
Serge Semin Feb. 5, 2024, 2:43 p.m. UTC | #3 
On Tue, Jan 30, 2024 at 04:43:22PM +0800, Yanteng Si wrote:
> The driver function is not changed, but the code location is
> adjusted to prepare for adding more loongson drivers.

Having the word "refactoring" in the subject is always suspicious
because submitters very often try to hind behind it many small
changes they didn't want to/didn't know how to unpin from a more bulky
change. Moreover if there is no detailed explanation what is done and
why, it raises too many review questions and makes the reviewers life
much harder. So it would have been much better for us if you split up
this change into the smaller patches (see my last comment for a
presumable subset of the patches) to simplify the review process and
improve the driver bisectability especially seeing there actually are
functional changes introduced here despite of what is said in the
commit log.

> 
> 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  | 61 +++++++++++++------
>  1 file changed, 42 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> index 9e40c28d453a..e2dcb339b8b0 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
> @@ -9,7 +9,12 @@
>  #include <linux/of_irq.h>
>  #include "stmmac.h"
>  
> -static int loongson_default_data(struct plat_stmmacenet_data *plat)
> +struct stmmac_pci_info {
> +	int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat);
> +};
> +
> +static void loongson_default_data(struct pci_dev *pdev,
> +				  struct plat_stmmacenet_data *plat)
>  {
>  	plat->clk_csr = 2;	/* clk_csr_i = 20-35MHz & MDC = clk_csr_i/16 */
>  	plat->has_gmac = 1;
> @@ -34,23 +39,37 @@ static int loongson_default_data(struct plat_stmmacenet_data *plat)
>  
>  	/* Disable RX queues routing by default */
>  	plat->rx_queues_cfg[0].pkt_route = 0x0;
> +}
> +
> +static int loongson_gmac_data(struct pci_dev *pdev,
> +			      struct plat_stmmacenet_data *plat)
> +{
> +	loongson_default_data(pdev, plat);
> +

> +	plat->multicast_filter_bins = 256;

Why do you need to move this here from the function tail?

> +

> +	plat->mdio_bus_data->phy_mask = 0;

This is already zero. Why do you need this?

>  

> -	/* Default to phy auto-detection */

What is wrong with this comment?

>  	plat->phy_addr = -1;
>  
>  	plat->dma_cfg->pbl = 32;
>  	plat->dma_cfg->pblx8 = true;
>  
> -	plat->multicast_filter_bins = 256;
>  	return 0;
>  }
>  
> -static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> +static struct stmmac_pci_info loongson_gmac_pci_info = {
> +	.setup = loongson_gmac_data,
> +};
> +
> +static int loongson_dwmac_probe(struct pci_dev *pdev,
> +				const struct pci_device_id *id)
>  {

> +	int ret, i, bus_id, phy_mode;
>  	struct plat_stmmacenet_data *plat;
> +	struct stmmac_pci_info *info;
>  	struct stmmac_resources res;
>  	struct device_node *np;
> -	int ret, i, phy_mode;

Reverse xmas tree order please.

>  
>  	np = dev_of_node(&pdev->dev);
>  
> @@ -69,18 +88,17 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id
>  	if (!plat->mdio_bus_data)
>  		return -ENOMEM;
>  

> +	plat->dma_cfg = devm_kzalloc(&pdev->dev, sizeof(*plat->dma_cfg),
> +				     GFP_KERNEL);
> +	if (!plat->dma_cfg)
> +		return -ENOMEM;
> +

Why do you need this moved above the mdio_node getting procedure? They
seem independent.

>  	plat->mdio_node = of_get_child_by_name(np, "mdio");
>  	if (plat->mdio_node) {
>  		dev_info(&pdev->dev, "Found MDIO subnode\n");
>  		plat->mdio_bus_data->needs_reset = true;
>  	}
>  
> -	plat->dma_cfg = devm_kzalloc(&pdev->dev, sizeof(*plat->dma_cfg), GFP_KERNEL);
> -	if (!plat->dma_cfg) {
> -		ret = -ENOMEM;
> -		goto err_put_node;
> -	}
> -
>  	/* Enable pci device */
>  	ret = pci_enable_device(pdev);
>  	if (ret) {
> @@ -98,9 +116,16 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id
>  		break;
>  	}
>  

> -	plat->bus_id = of_alias_get_id(np, "ethernet");
> -	if (plat->bus_id < 0)
> -		plat->bus_id = pci_dev_id(pdev);

This is a functional change because further bus_id is no longer
initialized by the pci_dev_id() method as a fallback case. If you are
sure this is required please unpin to a separate patch and explain.

> +	pci_set_master(pdev);
> +
> +	info = (struct stmmac_pci_info *)id->driver_data;
> +	ret = info->setup(pdev, plat);
> +	if (ret)
> +		goto err_disable_device;
> +
> +	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) {
> @@ -110,11 +135,7 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id
>  	}
>  
>  	plat->phy_interface = phy_mode;

> -	plat->mac_interface = PHY_INTERFACE_MODE_GMII;

This is just dropped. Are you sure that the driver will work correctly
after this change is applied? Russell already asked you about this change
here:
https://lore.kernel.org/netdev/ZZPnaziDZEcv5GGw@shell.armlinux.org.uk/

Anyway please unpin it to a separate patch and explain.

>  
> -	pci_set_master(pdev);
> -
> -	loongson_default_data(plat);
>  	pci_enable_msi(pdev);
>  	memset(&res, 0, sizeof(res));
>  	res.addr = pcim_iomap_table(pdev)[0];
> @@ -212,8 +233,10 @@ static int __maybe_unused loongson_dwmac_resume(struct device *dev)
>  static SIMPLE_DEV_PM_OPS(loongson_dwmac_pm_ops, loongson_dwmac_suspend,
>  			 loongson_dwmac_resume);
>  
> +#define PCI_DEVICE_ID_LOONGSON_GMAC	0x7a03
> +
>  static const struct pci_device_id loongson_dwmac_id_table[] = {
> -	{ PCI_VDEVICE(LOONGSON, 0x7a03) },
> +	{ PCI_DEVICE_DATA(LOONGSON, GMAC, &loongson_gmac_pci_info) },

If I were you and needed to preserve all the changes I would have
split the patch up into the next patches:
1. Use PCI_DEVICE_DATA() macro for device identification
2. Drop mac-interface initialization
3. Don't initialize MDIO bus ID with PCIe device ID
4. Introduce device-specific setup callback

-Serge(y)

>  	{}
>  };
>  MODULE_DEVICE_TABLE(pci, loongson_dwmac_id_table);
> -- 
> 2.31.4
>
Yanteng Si April 5, 2024, 11:13 a.m. UTC | #4 
Hi Serge,


Sorry, I seem to have forgotten to reply to the comments on this patch.

在 2024/2/5 22:43, Serge Semin 写道:
> On Tue, Jan 30, 2024 at 04:43:22PM +0800, Yanteng Si wrote:
>> The driver function is not changed, but the code location is
>> adjusted to prepare for adding more loongson drivers.
> Having the word "refactoring" in the subject is always suspicious
> because submitters very often try to hind behind it many small
> changes they didn't want to/didn't know how to unpin from a more bulky
> change. Moreover if there is no detailed explanation what is done and
> why, it raises too many review questions and makes the reviewers life
> much harder. So it would have been much better for us if you split up
> this change into the smaller patches (see my last comment for a
> presumable subset of the patches) to simplify the review process and
> improve the driver bisectability especially seeing there actually are
> functional changes introduced here despite of what is said in the
> commit log.
OK. I will resplit it.
>> 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  | 61 +++++++++++++------
>>   1 file changed, 42 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
>> index 9e40c28d453a..e2dcb339b8b0 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
>> @@ -9,7 +9,12 @@
>>   #include <linux/of_irq.h>
>>   #include "stmmac.h"
>>   
>> -static int loongson_default_data(struct plat_stmmacenet_data *plat)
>> +struct stmmac_pci_info {
>> +	int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat);
>> +};
>> +
>> +static void loongson_default_data(struct pci_dev *pdev,
>> +				  struct plat_stmmacenet_data *plat)
>>   {
>>   	plat->clk_csr = 2;	/* clk_csr_i = 20-35MHz & MDC = clk_csr_i/16 */
>>   	plat->has_gmac = 1;
>> @@ -34,23 +39,37 @@ static int loongson_default_data(struct plat_stmmacenet_data *plat)
>>   
>>   	/* Disable RX queues routing by default */
>>   	plat->rx_queues_cfg[0].pkt_route = 0x0;
>> +}
>> +
>> +static int loongson_gmac_data(struct pci_dev *pdev,
>> +			      struct plat_stmmacenet_data *plat)
>> +{
>> +	loongson_default_data(pdev, plat);
>> +
>> +	plat->multicast_filter_bins = 256;
> Why do you need to move this here from the function tail?
OK, restore it.
>
>> +
>> +	plat->mdio_bus_data->phy_mask = 0;
> This is already zero. Why do you need this?
OK, drop it.
>
>>   
>> -	/* Default to phy auto-detection */
> What is wrong with this comment?
Sorry, restore it.
>
>>   	plat->phy_addr = -1;
>>   
>>   	plat->dma_cfg->pbl = 32;
>>   	plat->dma_cfg->pblx8 = true;
>>   
>> -	plat->multicast_filter_bins = 256;
>>   	return 0;
>>   }
>>   
>> -static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>> +static struct stmmac_pci_info loongson_gmac_pci_info = {
>> +	.setup = loongson_gmac_data,
>> +};
>> +
>> +static int loongson_dwmac_probe(struct pci_dev *pdev,
>> +				const struct pci_device_id *id)
>>   {
>> +	int ret, i, bus_id, phy_mode;
>>   	struct plat_stmmacenet_data *plat;
>> +	struct stmmac_pci_info *info;
>>   	struct stmmac_resources res;
>>   	struct device_node *np;
>> -	int ret, i, phy_mode;
> Reverse xmas tree order please.
OK.
>
>>   
>>   	np = dev_of_node(&pdev->dev);
>>   
>> @@ -69,18 +88,17 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id
>>   	if (!plat->mdio_bus_data)
>>   		return -ENOMEM;
>>   
>> +	plat->dma_cfg = devm_kzalloc(&pdev->dev, sizeof(*plat->dma_cfg),
>> +				     GFP_KERNEL);
>> +	if (!plat->dma_cfg)
>> +		return -ENOMEM;
>> +
> Why do you need this moved above the mdio_node getting procedure? They
> seem independent.
Sorry, restore it.
>
>>   	plat->mdio_node = of_get_child_by_name(np, "mdio");
>>   	if (plat->mdio_node) {
>>   		dev_info(&pdev->dev, "Found MDIO subnode\n");
>>   		plat->mdio_bus_data->needs_reset = true;
>>   	}
>>   
>> -	plat->dma_cfg = devm_kzalloc(&pdev->dev, sizeof(*plat->dma_cfg), GFP_KERNEL);
>> -	if (!plat->dma_cfg) {
>> -		ret = -ENOMEM;
>> -		goto err_put_node;
>> -	}
>> -
>>   	/* Enable pci device */
>>   	ret = pci_enable_device(pdev);
>>   	if (ret) {
>> @@ -98,9 +116,16 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id
>>   		break;
>>   	}
>>   
>> -	plat->bus_id = of_alias_get_id(np, "ethernet");
>> -	if (plat->bus_id < 0)
>> -		plat->bus_id = pci_dev_id(pdev);
> This is a functional change because further bus_id is no longer
> initialized by the pci_dev_id() method as a fallback case. If you are
> sure this is required please unpin to a separate patch and explain.
Hmm, I will merge it into  [PATCH net-next 03/11] .
>
>> +	pci_set_master(pdev);
>> +
>> +	info = (struct stmmac_pci_info *)id->driver_data;
>> +	ret = info->setup(pdev, plat);
>> +	if (ret)
>> +		goto err_disable_device;
>> +
>> +	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) {
>> @@ -110,11 +135,7 @@ static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id
>>   	}
>>   
>>   	plat->phy_interface = phy_mode;
>> -	plat->mac_interface = PHY_INTERFACE_MODE_GMII;
> This is just dropped. Are you sure that the driver will work correctly
Yes, We only need to set phy_interface.
> after this change is applied? Russell already asked you about this change
> here:
> https://lore.kernel.org/netdev/ZZPnaziDZEcv5GGw@shell.armlinux.org.uk/
>
> Anyway please unpin it to a separate patch and explain.
OK.
>
>>   
>> -	pci_set_master(pdev);
>> -
>> -	loongson_default_data(plat);
>>   	pci_enable_msi(pdev);
>>   	memset(&res, 0, sizeof(res));
>>   	res.addr = pcim_iomap_table(pdev)[0];
>> @@ -212,8 +233,10 @@ static int __maybe_unused loongson_dwmac_resume(struct device *dev)
>>   static SIMPLE_DEV_PM_OPS(loongson_dwmac_pm_ops, loongson_dwmac_suspend,
>>   			 loongson_dwmac_resume);
>>   
>> +#define PCI_DEVICE_ID_LOONGSON_GMAC	0x7a03
>> +
>>   static const struct pci_device_id loongson_dwmac_id_table[] = {
>> -	{ PCI_VDEVICE(LOONGSON, 0x7a03) },
>> +	{ PCI_DEVICE_DATA(LOONGSON, GMAC, &loongson_gmac_pci_info) },
> If I were you and needed to preserve all the changes I would have
> split the patch up into the next patches:
> 1. Use PCI_DEVICE_DATA() macro for device identification
> 2. Drop mac-interface initialization
> 3. Don't initialize MDIO bus ID with PCIe device ID
> 4. Introduce device-specific setup callback

OK, I will.


Thanks,

Yanteng
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 9e40c28d453a..e2dcb339b8b0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
@@ -9,7 +9,12 @@ 
 #include <linux/of_irq.h>
 #include "stmmac.h"
 
-static int loongson_default_data(struct plat_stmmacenet_data *plat)
+struct stmmac_pci_info {
+	int (*setup)(struct pci_dev *pdev, struct plat_stmmacenet_data *plat);
+};
+
+static void loongson_default_data(struct pci_dev *pdev,
+				  struct plat_stmmacenet_data *plat)
 {
 	plat->clk_csr = 2;	/* clk_csr_i = 20-35MHz & MDC = clk_csr_i/16 */
 	plat->has_gmac = 1;
@@ -34,23 +39,37 @@  static int loongson_default_data(struct plat_stmmacenet_data *plat)
 
 	/* Disable RX queues routing by default */
 	plat->rx_queues_cfg[0].pkt_route = 0x0;
+}
+
+static int loongson_gmac_data(struct pci_dev *pdev,
+			      struct plat_stmmacenet_data *plat)
+{
+	loongson_default_data(pdev, plat);
+
+	plat->multicast_filter_bins = 256;
+
+	plat->mdio_bus_data->phy_mask = 0;
 
-	/* Default to phy auto-detection */
 	plat->phy_addr = -1;
 
 	plat->dma_cfg->pbl = 32;
 	plat->dma_cfg->pblx8 = true;
 
-	plat->multicast_filter_bins = 256;
 	return 0;
 }
 
-static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id *id)
+static struct stmmac_pci_info loongson_gmac_pci_info = {
+	.setup = loongson_gmac_data,
+};
+
+static int loongson_dwmac_probe(struct pci_dev *pdev,
+				const struct pci_device_id *id)
 {
+	int ret, i, bus_id, phy_mode;
 	struct plat_stmmacenet_data *plat;
+	struct stmmac_pci_info *info;
 	struct stmmac_resources res;
 	struct device_node *np;
-	int ret, i, phy_mode;
 
 	np = dev_of_node(&pdev->dev);
 
@@ -69,18 +88,17 @@  static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id
 	if (!plat->mdio_bus_data)
 		return -ENOMEM;
 
+	plat->dma_cfg = devm_kzalloc(&pdev->dev, sizeof(*plat->dma_cfg),
+				     GFP_KERNEL);
+	if (!plat->dma_cfg)
+		return -ENOMEM;
+
 	plat->mdio_node = of_get_child_by_name(np, "mdio");
 	if (plat->mdio_node) {
 		dev_info(&pdev->dev, "Found MDIO subnode\n");
 		plat->mdio_bus_data->needs_reset = true;
 	}
 
-	plat->dma_cfg = devm_kzalloc(&pdev->dev, sizeof(*plat->dma_cfg), GFP_KERNEL);
-	if (!plat->dma_cfg) {
-		ret = -ENOMEM;
-		goto err_put_node;
-	}
-
 	/* Enable pci device */
 	ret = pci_enable_device(pdev);
 	if (ret) {
@@ -98,9 +116,16 @@  static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id
 		break;
 	}
 
-	plat->bus_id = of_alias_get_id(np, "ethernet");
-	if (plat->bus_id < 0)
-		plat->bus_id = pci_dev_id(pdev);
+	pci_set_master(pdev);
+
+	info = (struct stmmac_pci_info *)id->driver_data;
+	ret = info->setup(pdev, plat);
+	if (ret)
+		goto err_disable_device;
+
+	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) {
@@ -110,11 +135,7 @@  static int loongson_dwmac_probe(struct pci_dev *pdev, const struct pci_device_id
 	}
 
 	plat->phy_interface = phy_mode;
-	plat->mac_interface = PHY_INTERFACE_MODE_GMII;
 
-	pci_set_master(pdev);
-
-	loongson_default_data(plat);
 	pci_enable_msi(pdev);
 	memset(&res, 0, sizeof(res));
 	res.addr = pcim_iomap_table(pdev)[0];
@@ -212,8 +233,10 @@  static int __maybe_unused loongson_dwmac_resume(struct device *dev)
 static SIMPLE_DEV_PM_OPS(loongson_dwmac_pm_ops, loongson_dwmac_suspend,
 			 loongson_dwmac_resume);
 
+#define PCI_DEVICE_ID_LOONGSON_GMAC	0x7a03
+
 static const struct pci_device_id loongson_dwmac_id_table[] = {
-	{ PCI_VDEVICE(LOONGSON, 0x7a03) },
+	{ PCI_DEVICE_DATA(LOONGSON, GMAC, &loongson_gmac_pci_info) },
 	{}
 };
 MODULE_DEVICE_TABLE(pci, loongson_dwmac_id_table);