diff mbox series

[v2,07/10] net: stmmac: dwmac-loongson: Add LS7A support

Message ID dd88ed0f53e9ee0f653ddeb78b326f8eb44bdbd1.1690439335.git.chenfeiyang@loongson.cn (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series stmmac: Add Loongson platform support | expand

Checks

Context Check Description
netdev/tree_selection success Guessing tree name failed - patch did not apply

Commit Message

Feiyang Chen July 27, 2023, 7:18 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: Huacai Chen <chenhuacai@loongson.cn>
Signed-off-by: Feiyang Chen <chenfeiyang@loongson.cn>
---
 .../ethernet/stmicro/stmmac/dwmac-loongson.c  | 149 ++++++++++--------
 1 file changed, 85 insertions(+), 64 deletions(-)

Comments

Andrew Lunn July 27, 2023, 9:18 a.m. UTC | #1 
> +static void common_default_data(struct pci_dev *pdev,
> +				struct plat_stmmacenet_data *plat)
>  {
> +	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;
>  
>  	/* Set default value for multicast hash bins */
> -	plat->multicast_filter_bins = HASH_TABLE_SIZE;
> +	plat->multicast_filter_bins = 256;

HASH_TABLE_SIZE is 64. You appear to be changing it to 256 for
everybody, not just your platform. I would expect something like
common_default_data() is called first, and then you change values in a
loongson specific function.

	 Andrew
Andrew Lunn July 27, 2023, 10:35 a.m. UTC | #2 
> +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);
> -
> -	if (!np) {
> -		pr_info("dwmac_loongson_pci: No OF node\n");
> -		return -ENODEV;
> -	}
> -
> -	if (!of_device_is_compatible(np, "loongson, pci-gmac")) {
> -		pr_info("dwmac_loongson_pci: Incompatible OF node\n");
> -		return -ENODEV;
> -	}

There are a lot of changes here, and it is not easy to review.  I
would suggest you first do a refactoring patch, moving all handling of
DT into a helper. Since it is just moving code around, it should be
easy to review. Then you can add support for platforms which don't
support DT.

	Andrew
Feiyang Chen July 28, 2023, 1:59 a.m. UTC | #3 
On Thu, Jul 27, 2023 at 5:18 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > +static void common_default_data(struct pci_dev *pdev,
> > +                             struct plat_stmmacenet_data *plat)
> >  {
> > +     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;
> >
> >       /* Set default value for multicast hash bins */
> > -     plat->multicast_filter_bins = HASH_TABLE_SIZE;
> > +     plat->multicast_filter_bins = 256;
>
> HASH_TABLE_SIZE is 64. You appear to be changing it to 256 for
> everybody, not just your platform. I would expect something like
> common_default_data() is called first, and then you change values in a
> loongson specific function.
>

Hi, Andrew,

The common_default_data() here is defined in our platform driver. We
have tested on our platforms (LS7A and LS2K) and it can be safely
changed to 256.

Thanks,
Feiyang

>          Andrew
Feiyang Chen July 28, 2023, 2 a.m. UTC | #4 
On Thu, Jul 27, 2023 at 6:35 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > +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);
> > -
> > -     if (!np) {
> > -             pr_info("dwmac_loongson_pci: No OF node\n");
> > -             return -ENODEV;
> > -     }
> > -
> > -     if (!of_device_is_compatible(np, "loongson, pci-gmac")) {
> > -             pr_info("dwmac_loongson_pci: Incompatible OF node\n");
> > -             return -ENODEV;
> > -     }
>
> There are a lot of changes here, and it is not easy to review.  I
> would suggest you first do a refactoring patch, moving all handling of
> DT into a helper. Since it is just moving code around, it should be
> easy to review. Then you can add support for platforms which don't
> support DT.
>

Hi, Andrew,

OK.

Thanks,
Feiyang

>         Andrew
Andrew Lunn July 28, 2023, 8:46 a.m. UTC | #5 
On Fri, Jul 28, 2023 at 09:59:53AM +0800, Feiyang Chen wrote:
> On Thu, Jul 27, 2023 at 5:18 PM Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > > +static void common_default_data(struct pci_dev *pdev,
> > > +                             struct plat_stmmacenet_data *plat)
> > >  {
> > > +     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;
> > >
> > >       /* Set default value for multicast hash bins */
> > > -     plat->multicast_filter_bins = HASH_TABLE_SIZE;
> > > +     plat->multicast_filter_bins = 256;
> >
> > HASH_TABLE_SIZE is 64. You appear to be changing it to 256 for
> > everybody, not just your platform. I would expect something like
> > common_default_data() is called first, and then you change values in a
> > loongson specific function.
> >
> 
> Hi, Andrew,
> 
> The common_default_data() here is defined in our platform driver. We
> have tested on our platforms (LS7A and LS2K) and it can be safely
> changed to 256.

Then add a new #define.

     Andrew
Feiyang Chen July 31, 2023, 9:42 a.m. UTC | #6 
On Fri, Jul 28, 2023 at 4:46 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Fri, Jul 28, 2023 at 09:59:53AM +0800, Feiyang Chen wrote:
> > On Thu, Jul 27, 2023 at 5:18 PM Andrew Lunn <andrew@lunn.ch> wrote:
> > >
> > > > +static void common_default_data(struct pci_dev *pdev,
> > > > +                             struct plat_stmmacenet_data *plat)
> > > >  {
> > > > +     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;
> > > >
> > > >       /* Set default value for multicast hash bins */
> > > > -     plat->multicast_filter_bins = HASH_TABLE_SIZE;
> > > > +     plat->multicast_filter_bins = 256;
> > >
> > > HASH_TABLE_SIZE is 64. You appear to be changing it to 256 for
> > > everybody, not just your platform. I would expect something like
> > > common_default_data() is called first, and then you change values in a
> > > loongson specific function.
> > >
> >
> > Hi, Andrew,
> >
> > The common_default_data() here is defined in our platform driver. We
> > have tested on our platforms (LS7A and LS2K) and it can be safely
> > changed to 256.
>
> Then add a new #define.
>

Hi, Andrew,

Should I add the new  #define in our platform driver or in common.h?

Thanks,
Feiyang

>      Andrew
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 a25c187d3185..3ab55340a6b8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-loongson.c
@@ -9,14 +9,21 @@ 
 #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 common_default_data(struct pci_dev *pdev,
+				struct plat_stmmacenet_data *plat)
 {
+	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;
 
 	/* Set default value for multicast hash bins */
-	plat->multicast_filter_bins = HASH_TABLE_SIZE;
+	plat->multicast_filter_bins = 256;
 
 	/* Set default value for unicast filter entries */
 	plat->unicast_filter_entries = 1;
@@ -35,59 +42,61 @@  static int loongson_default_data(struct plat_stmmacenet_data *plat)
 	/* Disable RX queues routing by default */
 	plat->rx_queues_cfg[0].pkt_route = 0x0;
 
-	/* Default to phy auto-detection */
-	plat->phy_addr = -1;
-
 	plat->dma_cfg->pbl = 32;
 	plat->dma_cfg->pblx8 = true;
 
-	plat->multicast_filter_bins = 256;
+	plat->clk_ref_rate = 125000000;
+	plat->clk_ptp_rate = 125000000;
+
+	plat->has_lgmac = 1;
+}
+
+static int loongson_gmac_data(struct pci_dev *pdev,
+			      struct plat_stmmacenet_data *plat)
+{
+	common_default_data(pdev, plat);
+
+	plat->mdio_bus_data->phy_mask = 0;
+
+	plat->phy_addr = -1;
+	plat->phy_interface = PHY_INTERFACE_MODE_RGMII_ID;
+
 	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);
-
-	if (!np) {
-		pr_info("dwmac_loongson_pci: No OF node\n");
-		return -ENODEV;
-	}
-
-	if (!of_device_is_compatible(np, "loongson, pci-gmac")) {
-		pr_info("dwmac_loongson_pci: Incompatible OF node\n");
-		return -ENODEV;
-	}
 
 	plat = devm_kzalloc(&pdev->dev, sizeof(*plat), GFP_KERNEL);
 	if (!plat)
 		return -ENOMEM;
 
+	plat->mdio_bus_data = devm_kzalloc(&pdev->dev,
+				   sizeof(*plat->mdio_bus_data), GFP_KERNEL);
+	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;
+
+	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");
-
-		plat->mdio_bus_data = devm_kzalloc(&pdev->dev,
-						   sizeof(*plat->mdio_bus_data),
-						   GFP_KERNEL);
-		if (!plat->mdio_bus_data) {
-			ret = -ENOMEM;
-			goto err_put_node;
-		}
 		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) {
@@ -105,45 +114,55 @@  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);
 
-	phy_mode = device_get_phy_mode(&pdev->dev);
-	if (phy_mode < 0) {
-		dev_err(&pdev->dev, "phy_mode not found\n");
-		ret = phy_mode;
+	info = (struct stmmac_pci_info *)id->driver_data;
+	ret = info->setup(pdev, plat);
+	if (ret)
 		goto err_disable_device;
-	}
 
-	plat->phy_interface = phy_mode;
-	plat->interface = PHY_INTERFACE_MODE_GMII;
+	if (np) {
+		bus_id = of_alias_get_id(np, "ethernet");
+		if (bus_id >= 0)
+			plat->bus_id = bus_id;
 
-	pci_set_master(pdev);
+		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;
+	}
 
-	loongson_default_data(plat);
 	pci_enable_msi(pdev);
+
 	memset(&res, 0, sizeof(res));
 	res.addr = pcim_iomap_table(pdev)[0];
+	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.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.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;
+		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);
@@ -219,8 +238,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);