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  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <37b8ec86b98706984019b418bc20f0d0883ed555.camel@mailbox.org>
Date: Mon, 14 Apr 2025 11:19:11 +0200
From: Philipp Stanner <phasta@...lbox.org>
To: Frank Sae <Frank.Sae@...or-comm.com>, Jakub Kicinski <kuba@...nel.org>, 
 Paolo Abeni <pabeni@...hat.com>, Andrew Lunn <andrew@...n.ch>, Heiner
 Kallweit <hkallweit1@...il.com>,  Russell King <linux@...linux.org.uk>,
 "David S . Miller" <davem@...emloft.net>, Eric Dumazet
 <edumazet@...gle.com>, netdev@...r.kernel.org
Cc: Masahiro Yamada <masahiroy@...nel.org>, 
 Parthiban.Veerasooran@...rochip.com, linux-kernel@...r.kernel.org, 
 "andrew+netdev @ lunn . ch" <andrew+netdev@...n.ch>, lee@...ger.us,
 horms@...nel.org,  linux-doc@...r.kernel.org, corbet@....net,
 geert+renesas@...der.be,  xiaogang.fan@...or-comm.com,
 fei.zhang@...or-comm.com, hua.sun@...or-comm.com
Subject: Re: [PATCH net-next v4 01/14] yt6801: Add support for a pci table
 in this module

On Tue, 2025-04-08 at 17:28 +0800, Frank Sae wrote:
> Add support for a pci table in this module, and implement pci_driver
>  function to initialize this driver, remove this driver or shutdown
> this
>  driver.
> Implement the fxgmac_drv_probe function to init interrupts, register
> mdio
>  and netdev.
> 
> Signed-off-by: Frank Sae <Frank.Sae@...or-comm.com>
> ---
>  .../ethernet/motorcomm/yt6801/yt6801_main.c   | 194
> ++++++++++++++++++
>  .../ethernet/motorcomm/yt6801/yt6801_type.h   | 114 ++++++++++
>  2 files changed, 308 insertions(+)
>  create mode 100644
> drivers/net/ethernet/motorcomm/yt6801/yt6801_main.c
>  create mode 100644
> drivers/net/ethernet/motorcomm/yt6801/yt6801_type.h
> 
> diff --git a/drivers/net/ethernet/motorcomm/yt6801/yt6801_main.c
> b/drivers/net/ethernet/motorcomm/yt6801/yt6801_main.c
> new file mode 100644
> index 000000000..10d63a8ed
> --- /dev/null
> +++ b/drivers/net/ethernet/motorcomm/yt6801/yt6801_main.c
> @@ -0,0 +1,194 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/* Copyright (c) 2022 - 2024 Motorcomm Electronic Technology
> Co.,Ltd.
> + *
> + * Below is a simplified block diagram of YT6801 chip and its
> relevant
> + * interfaces.
> + *                      ||
> + *  ********************++**********************
> + *  *            | PCIE Endpoint |             *
> + *  *            +---------------+             *
> + *  *                | GMAC |                  *
> + *  *                +--++--+                  *
> + *  *                  |**|                    *
> + *  *         GMII --> |**| <-- MDIO           *
> + *  *                 +-++--+                  *
> + *  *            | Integrated PHY |  YT8531S   *
> + *  *                 +-++-+                   *
> + *  ********************||******************* **
> + */
> +
> +#include <linux/module.h>
> +#include "yt6801_type.h"
> +
> +static void fxgmac_phy_release(struct fxgmac_pdata *priv)
> +{
> +	fxgmac_io_wr_bits(priv, EPHY_CTRL, EPHY_CTRL_RESET, 1);
> +	fsleep(100);
> +
> +static void fxgmac_phy_reset(struct fxgmac_pdata *priv)
> +{
> +	fxgmac_io_wr_bits(priv, EPHY_CTRL, EPHY_CTRL_RESET, 0);
> +	fsleep(1500);
> +}
> +
> +static void fxgmac_init_interrupt_scheme(struct fxgmac_pdata *priv)
> +{
> +	struct pci_dev *pdev = to_pci_dev(priv->dev);
> +	int req_vectors = FXGMAC_MAX_DMA_CHANNELS;
> +
> +	/* Since we have FXGMAC_MAX_DMA_CHANNELS channels, we must
> ensure the
> +	 * number of cpu core is ok. otherwise, just roll back to
> legacy.
> +	 */
> +	if (num_online_cpus() < FXGMAC_MAX_DMA_CHANNELS - 1)
> +		goto enable_msi_interrupt;
> +
> +	priv->msix_entries =
> +		kcalloc(req_vectors, sizeof(struct msix_entry),
> GFP_KERNEL);
> +	if (!priv->msix_entries)
> +		goto enable_msi_interrupt;
> +
> +	for (u32 i = 0; i < req_vectors; i++)
> +		priv->msix_entries[i].entry = i;
> +
> +	if (pci_enable_msix_exact(pdev, priv->msix_entries,
> req_vectors) < 0) {
> +		/* Roll back to msi */
> +		kfree(priv->msix_entries);
> +		priv->msix_entries = NULL;
> +		dev_err(priv->dev, "Enable MSIx failed, clear msix
> entries.\n");
> +		goto enable_msi_interrupt;
> +	}
> +
> +	priv->int_flag &= ~INT_FLAG_INTERRUPT;
> +	priv->int_flag |= INT_FLAG_MSIX;
> +	priv->per_channel_irq = 1;
> +	return;
> +
> +enable_msi_interrupt:
> +	priv->int_flag &= ~INT_FLAG_INTERRUPT;
> +	if (pci_enable_msi(pdev) < 0) {
> +		priv->int_flag |= INT_FLAG_LEGACY;
> +		dev_err(priv->dev, "rollback to LEGACY.\n");
> +	} else {
> +		priv->int_flag |= INT_FLAG_MSI;
> +		dev_err(priv->dev, "rollback to MSI.\n");
> +		priv->dev_irq = pdev->irq;
> +	}
> +}
> +
> +static int fxgmac_drv_probe(struct device *dev, struct
> fxgmac_resources *res)
> +{
> +	struct fxgmac_pdata *priv;
> +	struct net_device *ndev;
> +	int ret;
> +
> +	ndev = alloc_etherdev_mq(sizeof(struct fxgmac_pdata),
> +				 FXGMAC_MAX_DMA_RX_CHANNELS);
> +	if (!ndev)
> +		return -ENOMEM;
> +
> +	SET_NETDEV_DEV(ndev, dev);
> +	priv = netdev_priv(ndev);
> +
> +	priv->dev = dev;
> +	priv->ndev = ndev;
> +	priv->dev_irq = res->irq;
> +	priv->hw_addr = res->addr;
> +	priv->msg_enable = NETIF_MSG_DRV;
> +	priv->dev_state = FXGMAC_DEV_PROBE;
> +
> +	/* Default to legacy interrupt */
> +	priv->int_flag &= ~INT_FLAG_INTERRUPT;
> +	priv->int_flag |= INT_FLAG_LEGACY;
> +
> +	pci_set_drvdata(to_pci_dev(priv->dev), priv);
> +
> +	if (IS_ENABLED(CONFIG_PCI_MSI))
> +		fxgmac_init_interrupt_scheme(priv);
> +
> +	ret = fxgmac_init(priv, true);
> +	if (ret < 0) {
> +		dev_err(dev, "fxgmac init failed:%d\n", ret);
> +		goto err_free_netdev;
> +	}
> +
> +	fxgmac_phy_reset(priv);
> +	fxgmac_phy_release(priv);
> +	ret = fxgmac_mdio_register(priv);
> +	if (ret < 0) {
> +		dev_err(dev, "Register fxgmac mdio failed:%d\n",
> ret);
> +		goto err_free_netdev;
> +	}
> +
> +	netif_carrier_off(ndev);
> +	ret = register_netdev(ndev);
> +	if (ret) {
> +		dev_err(dev, "Register ndev failed:%d\n", ret);
> +		goto err_free_netdev;
> +	}
> +
> +	return 0;
> +
> +err_free_netdev:
> +	free_netdev(ndev);
> +	return ret;
> +}
> +
> +static int fxgmac_probe(struct pci_dev *pcidev, const struct
> pci_device_id *id)
> +{
> +	struct fxgmac_resources res;
> +	int err;
> +
> +	err = pcim_enable_device(pcidev);
> +	if (err)
> +		return err;
> +
> +	memset(&res, 0, sizeof(res));
> +	res.irq = pcidev->irq;
> +	res.addr  = pcim_iomap_region(pcidev, 0, pci_name(pcidev));

This is actually a slight misuse: the "name" parameter should be your
driver's name, not the PCI device's name.

That string gets printed in case of a request collision regarding that
device, and the print is only useful if it says who stole the region,
not on which device sth was stolen.

(pcim_iomap_region() doesn't copy the string, so be careful to put it
in the TEXT segment or sth like that)

Regards
P.


> +	err = PTR_ERR_OR_ZERO(res.addr);
> +	if (err)
> +		return err;
> +
> +	pci_set_master(pcidev);
> +	return fxgmac_drv_probe(&pcidev->dev, &res);
> +}
> +
> +static void fxgmac_remove(struct pci_dev *pcidev)
> +{
> +	struct fxgmac_pdata *priv = dev_get_drvdata(&pcidev->dev);
> +	struct net_device *ndev = priv->ndev;
> +
> +	unregister_netdev(ndev);
> +	fxgmac_phy_reset(priv);
> +	free_netdev(ndev);
> +
> +	if (IS_ENABLED(CONFIG_PCI_MSI) &&
> +	    FIELD_GET(INT_FLAG_MSIX, priv->int_flag)) {
> +		pci_disable_msix(pcidev);
> +		kfree(priv->msix_entries);
> +		priv->msix_entries = NULL;
> +	}
> +}
> +
> +#define MOTORCOMM_PCI_ID			0x1f0a
> +#define YT6801_PCI_DEVICE_ID			0x6801
> +
> +static const struct pci_device_id fxgmac_pci_tbl[] = {
> +	{ PCI_DEVICE(MOTORCOMM_PCI_ID, YT6801_PCI_DEVICE_ID) },
> +	{ 0 }
> +};
> +
> +MODULE_DEVICE_TABLE(pci, fxgmac_pci_tbl);
> +
> +static struct pci_driver fxgmac_pci_driver = {
> +	.name		= FXGMAC_DRV_NAME,
> +	.id_table	= fxgmac_pci_tbl,
> +	.probe		= fxgmac_probe,
> +	.remove		= fxgmac_remove,
> +};
> +
> +module_pci_driver(fxgmac_pci_driver);
> +
> +MODULE_AUTHOR("Motorcomm Electronic Tech. Co., Ltd.");
> +MODULE_DESCRIPTION(FXGMAC_DRV_DESC);
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/net/ethernet/motorcomm/yt6801/yt6801_type.h
> b/drivers/net/ethernet/motorcomm/yt6801/yt6801_type.h
> new file mode 100644
> index 000000000..bb6c2640a
> --- /dev/null
> +++ b/drivers/net/ethernet/motorcomm/yt6801/yt6801_type.h
> @@ -0,0 +1,114 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/* Copyright (c) 2022 - 2024 Motorcomm Electronic Technology
> Co.,Ltd. */
> +
> +#ifndef YT6801_TYPE_H
> +#define YT6801_TYPE_H
> +
> +#include <linux/netdevice.h>
> +#include <linux/types.h>
> +#include <linux/pci.h>
> +
> +#define FXGMAC_DRV_NAME		"yt6801"
> +#define FXGMAC_DRV_DESC		"Motorcomm Gigabit Ethernet
> Driver"
> +
> +#define FXGMAC_RX_BUF_ALIGN	64
> +#define FXGMAC_TX_MAX_BUF_SIZE	(0x3fff & ~(FXGMAC_RX_BUF_ALIGN -
> 1))
> +#define FXGMAC_RX_MIN_BUF_SIZE	(ETH_FRAME_LEN + ETH_FCS_LEN +
> VLAN_HLEN)
> +
> +/* Descriptors required for maximum contiguous TSO/GSO packet */
> +#define FXGMAC_TX_MAX_SPLIT	((GSO_MAX_SIZE /
> FXGMAC_TX_MAX_BUF_SIZE) + 1)
> +
> +/* Maximum possible descriptors needed for a SKB */
> +#define FXGMAC_TX_MAX_DESC_NR	(MAX_SKB_FRAGS + FXGMAC_TX_MAX_SPLIT
> + 2)
> +
> +#define FXGMAC_DMA_STOP_TIMEOUT		5
> +#define FXGMAC_JUMBO_PACKET_MTU		9014
> +#define FXGMAC_MAX_DMA_RX_CHANNELS	4
> +#define FXGMAC_MAX_DMA_TX_CHANNELS	1
> +#define
> FXGMAC_MAX_DMA_CHANNELS                                           \
> +	(FXGMAC_MAX_DMA_RX_CHANNELS + FXGMAC_MAX_DMA_TX_CHANNELS)
> +
> +#define EPHY_CTRL				0x1004
> +#define EPHY_CTRL_RESET				BIT(0)
> +#define EPHY_CTRL_STA_LINKUP			BIT(1)
> +#define EPHY_CTRL_STA_DUPLEX			BIT(2)
> +#define EPHY_CTRL_STA_SPEED			GENMASK(4, 3)
> +
> +struct fxgmac_resources {
> +	void __iomem *addr;
> +	int irq;
> +};
> +
> +enum fxgmac_dev_state {
> +	FXGMAC_DEV_OPEN		= 0x0,
> +	FXGMAC_DEV_CLOSE	= 0x1,
> +	FXGMAC_DEV_STOP		= 0x2,
> +	FXGMAC_DEV_START	= 0x3,
> +	FXGMAC_DEV_SUSPEND	= 0x4,
> +	FXGMAC_DEV_RESUME	= 0x5,
> +	FXGMAC_DEV_PROBE	= 0xFF,
> +};
> +
> +struct fxgmac_pdata {
> +	struct net_device *ndev;
> +	struct device *dev;
> +	struct phy_device *phydev;
> +
> +	void __iomem *hw_addr;			/* Registers base */
> +
> +	/* Device interrupt */
> +	int dev_irq;
> +	unsigned int per_channel_irq;
> +	u32 channel_irq[FXGMAC_MAX_DMA_CHANNELS];
> +	struct msix_entry *msix_entries;
> +#define INT_FLAG_INTERRUPT		GENMASK(4, 0)
> +#define INT_FLAG_MSI			BIT(1)
> +#define INT_FLAG_MSIX			BIT(3)
> +#define INT_FLAG_LEGACY			BIT(4)
> +#define INT_FLAG_RX0_NAPI		BIT(18)
> +#define INT_FLAG_RX1_NAPI		BIT(19)
> +#define INT_FLAG_RX2_NAPI		BIT(20)
> +#define INT_FLAG_RX3_NAPI		BIT(21)
> +#define INT_FLAG_RX0_IRQ		BIT(22)
> +#define INT_FLAG_RX1_IRQ		BIT(23)
> +#define INT_FLAG_RX2_IRQ		BIT(24)
> +#define INT_FLAG_RX3_IRQ		BIT(25)
> +#define INT_FLAG_TX_NAPI		BIT(26)
> +#define INT_FLAG_TX_IRQ			BIT(27)
> +#define INT_FLAG_LEGACY_NAPI		BIT(30)
> +#define INT_FLAG_LEGACY_IRQ		BIT(31)
> +	u32 int_flag;		/* interrupt flag */
> +
> +	u32 msg_enable;
> +	enum fxgmac_dev_state dev_state;
> +};
> +
> +static inline u32 fxgmac_io_rd(struct fxgmac_pdata *priv, u32 reg)
> +{
> +	return ioread32(priv->hw_addr + reg);
> +}
> +
> +static inline u32
> +fxgmac_io_rd_bits(struct fxgmac_pdata *priv, u32 reg, u32 mask)
> +{
> +	u32 cfg = fxgmac_io_rd(priv, reg);
> +
> +	return FIELD_GET(mask, cfg);
> +}
> +
> +static inline void fxgmac_io_wr(struct fxgmac_pdata *priv, u32 reg,
> u32 set)
> +{
> +	iowrite32(set, priv->hw_addr + reg);
> +}
> +
> +static inline void
> +fxgmac_io_wr_bits(struct fxgmac_pdata *priv, u32 reg, u32 mask, u32
> set)
> +{
> +	u32 cfg = fxgmac_io_rd(priv, reg);
> +
> +	cfg &= ~mask;
> +	cfg |= FIELD_PREP(mask, set);
> +	fxgmac_io_wr(priv, reg, cfg);
> +}
> +
> +#endif /* YT6801_TYPE_H */


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ