netdev - Re: [PATCH net-next v2 1/3] net: phy: mediatek: Add 2.5Gphy firmware dt-bindings and dts node

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c5728ec30db963c97b6e292b51e73e2c075d1757.camel@mediatek.com>
Date: Wed, 26 Feb 2025 04:14:56 +0000
From: SkyLake Huang (黃啟澤)
	<SkyLake.Huang@...iatek.com>
To: "andrew@...n.ch" <andrew@...n.ch>
CC: Steven Liu (劉人豪) <steven.liu@...iatek.com>,
	"dqfext@...il.com" <dqfext@...il.com>, "davem@...emloft.net"
	<davem@...emloft.net>, AngeloGioacchino Del Regno
	<angelogioacchino.delregno@...labora.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "edumazet@...gle.com" <edumazet@...gle.com>,
	"pabeni@...hat.com" <pabeni@...hat.com>, "linux-mediatek@...ts.infradead.org"
	<linux-mediatek@...ts.infradead.org>, "linux@...linux.org.uk"
	<linux@...linux.org.uk>, "hkallweit1@...il.com" <hkallweit1@...il.com>,
	"horms@...nel.org" <horms@...nel.org>, "kuba@...nel.org" <kuba@...nel.org>,
	"daniel@...rotopia.org" <daniel@...rotopia.org>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "matthias.bgg@...il.com" <matthias.bgg@...il.com>
Subject: Re: [PATCH net-next v2 1/3] net: phy: mediatek: Add 2.5Gphy firmware
 dt-bindings and dts node

On Tue, 2025-02-25 at 14:51 +0100, Andrew Lunn wrote:
> 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> > > Would using a 'reserved-memory' region be an option maybe?
> > Or maybe just leave those mapped registers' addresses in driver
> > code
> > (mtk-2p5ge.c)? Like:
> > #define MT7988_2P5GE_PMB_BASE (0x0f100000)
> > #define MT7988_2P5GE_PMB_LEN  (0x20000)
> 
> The problem with hard coding them is you need some way to know which
> set of hard coded values to use, because the hardware engineers will
> not guarantee to never move them, or change the bit layout for the
> next generation of devices.
> 
> PHYs don't use compatibles because they have an ID in register 2 and
> 3. Is this ID specific to the MT7988?
> 
>         Andrew
Do you mean that "are MT7988_2P5GE_PMB_BASE & MT7988_2P5GE_PMB_LEN
specific to MT7988"? There'a another our new platform, MT7987, has
almost the same built-in 2.5Gphy. It will use the same "PMB" base
address to load firmware.

So I guess I can do the following according to the previous discussion:
1) Reserve a memory region in mt7988.dtsi
reserved-memory {
	#address-cells = <2>;
	#size-celss = <2>;
	ranges;

	/* 0x0f0100000~0x0f1f0024 are specific for built-in 2.5Gphy.
	 * In this range, it includes "PMB_FW_BASE"(0x0f100000)
	 * and "MCU_CSR_BASE"(0x0f0f0000)
	 */
	i2p5g: i2p5g@...00000 {
		reg = <0 0x0f010000 0 0x1e0024>;
		no-map;
	};
};

Reserve a memory region in mt7987.dtsi
reserved-memory {
	#address-cells = <2>;
	#size-celss = <2>;
	ranges;

	i2p5g: i2p5g@...00000 {
		reg = <0 0x0f010000 0 0x1e0024>;
		no-map;
	};

	/* For built-in 2.5Gphy's top reset */
	i2p5g_apb: i2p5g_apb@...30000 {
		reg = <0 0x11c30000 0 0x10c>;
		no-map;
	};
};

2) Since PHYs don't use compatibles, hardcode address in mtk-2p5ge.c:
/* MTK_ prefix means that the macro is used for both MT7988 & MT7987*/
#define MTK_2P5GPHY_PMB_FW_BASE		(0x0f100000)
#define MT7988_2P5GE_PMB_FW_LEN		(0x20000)
#define MT7987_2P5GE_PMB_FW_LEN		(0x18000)
#define MTK_2P5GPHY_MCU_CSR_BASE	(0x0f0f0000)
#define MTK_2P5GPHY_MCU_CSR_LEN		(0x20)

/* On MT7987, we separate firmware bin to 2 files and total size
 * is decreased from 128KB(mediatek/mt7988/i2p5ge-phy-pmb.bin) to
 * 96KB(mediatek/mt7987/i2p5ge-phy-pmb.bin)+
 * 28KB(mediatek/mt7987/i2p5ge-phy-DSPBitTb.bin)
 * And i2p5ge-phy-DSPBitTb.bin will be loaded to
 * MT7987_2P5GE_XBZ_PMA_RX_BASE
 */
#define MT7987_2P5GE_XBZ_PMA_RX_BASE	(0x0f080000)
#define MT7987_2P5GE_XBZ_PMA_RX_LEN	(0x5228)
#define MT7987_2P5GE_DSPBITTB_SIZE	(0x7000)

/* MT7987 requires these base addresses to manipulate some
 * registers before loading firmware.
 */
#define MT7987_2P5GE_APB_BASE		(0x11c30000)
#define MT7987_2P5GE_APB_LEN		(0x9c)
#define MT7987_2P5GE_PMD_REG_BASE	(0x0f010000)
#define MT7987_2P5GE_PMD_REG_LEN	(0x210)
#define MT7987_2P5GE_XBZ_PCS_REG_BASE	(0x0f030000)
#define MT7987_2P5GE_XBZ_PCS_REG_LEN	(0x844)
#define MT7987_2P5GE_CHIP_SCU_BASE	(0x0f0cf800)
#define MT7987_2P5GE_CHIP_SCU_LEN	(0x12c)

static int mt7988_2p5ge_phy_load_fw(struct phy_device *phydev)
{
	struct mtk_i2p5ge_phy_priv *priv = phydev->priv;
	void __iomem *mcu_csr_base, *pmb_addr;
	struct device *dev = &phydev->mdio.dev;
	const struct firmware *fw;
	int ret, i;
	u32 reg;

	if (priv->fw_loaded)
		return 0;

	pmb_addr = ioremap(MTK_2P5GPHY_PMB_FW_BASE,
			   MT7988_2P5GE_PMB_FW_LEN);
	if (!pmb_addr)
		return -ENOMEM;
	mcu_csr_base = ioremap(MTK_2P5GPHY_MCU_CSR_BASE,
			       MTK_2P5GPHY_MCU_CSR_LEN);
	if (!mcu_csr_base) {
		ret = -ENOMEM;
		goto free_pmb;
	}
...
free:
	iounmap(mcu_csr_base);
free_pmb:
	iounmap(pmb_addr);
...
}

BRs,
Sky