[PATCH 1/2] net: rtl8169: add minimal support for 8125B variant

[PATCH 1/2] net: rtl8169: add minimal support for 8125B variant

[Eugen Hristev]

Add minimal support for 8125B version.
Changes are based on the Linux driver.
Tested on Radxa Rock 5B Rk3588 board.

Connection to a laptop worked fine in 100 Mbps mode.
1000 Mbps mode is not working at the moment.

Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>
---
 drivers/net/rtl8169.c | 52 ++++++++++++++++++++++++++++++-------------
 1 file changed, 37 insertions(+), 15 deletions(-)

diff --git a/drivers/net/rtl8169.c b/drivers/net/rtl8169.c
index c9c07a5a8ffe..2276a465e787 100644
--- a/drivers/net/rtl8169.c
+++ b/drivers/net/rtl8169.c
@@ -118,9 +118,9 @@ enum RTL8169_registers {
 	FLASH = 0x30,
 	ERSR = 0x36,
 	ChipCmd = 0x37,
-	TxPoll = 0x38,
-	IntrMask = 0x3C,
-	IntrStatus = 0x3E,
+	TxPoll_8169 = 0x38,
+	IntrMask_8169 = 0x3C,
+	IntrStatus_8169 = 0x3E,
 	TxConfig = 0x40,
 	RxConfig = 0x44,
 	RxMissed = 0x4C,
@@ -148,6 +148,12 @@ enum RTL8169_registers {
 	FuncForceEvent = 0xFC,
 };
 
+enum RTL8125_registers {
+	IntrMask_8125 = 0x38,
+	IntrStatus_8125 = 0x3C,
+	TxPoll_8125 = 0x90,
+};
+
 enum RTL8169_register_content {
 	/*InterruptStatusBits */
 	SYSErr = 0x8000,
@@ -263,6 +269,7 @@ static struct {
 	{"RTL-8101e",		0x34, 0xff7e1880,},
 	{"RTL-8100e",		0x32, 0xff7e1880,},
 	{"RTL-8168h/8111h",	0x54, 0xff7e1880,},
+	{"RTL-8125B",		0x64, 0xff7e1880,},
 };
 
 enum _DescStatusBit {
@@ -347,6 +354,7 @@ static struct pci_device_id supported[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_REALTEK, 0x8167) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_REALTEK, 0x8168) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_REALTEK, 0x8169) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_REALTEK, 0x8125) },
 	{}
 };
 
@@ -517,6 +525,7 @@ static int rtl_recv_common(struct udevice *dev, unsigned long dev_iobase,
 	/* return true if there's an ethernet packet ready to read */
 	/* nic->packet should contain data on return */
 	/* nic->packetlen should contain length of data */
+	struct pci_child_plat *pplat = dev_get_parent_plat(dev);
 	int cur_rx;
 	int length = 0;
 
@@ -558,6 +567,10 @@ static int rtl_recv_common(struct udevice *dev, unsigned long dev_iobase,
 		return length;
 
 	} else {
+		u32 IntrStatus = IntrStatus_8169;
+
+		if (pplat->device == 0x8125)
+			IntrStatus = IntrStatus_8125;
 		ushort sts = RTL_R8(IntrStatus);
 		RTL_W8(IntrStatus, sts & ~(TxErr | RxErr | SYSErr));
 		udelay(100);	/* wait */
@@ -582,6 +595,7 @@ static int rtl_send_common(struct udevice *dev, unsigned long dev_iobase,
 {
 	/* send the packet to destination */
 
+	struct pci_child_plat *pplat = dev_get_parent_plat(dev);
 	u32 to;
 	u8 *ptxb;
 	int entry = tpc->cur_tx % NUM_TX_DESC;
@@ -618,7 +632,10 @@ static int rtl_send_common(struct udevice *dev, unsigned long dev_iobase,
 				    ((len > ETH_ZLEN) ? len : ETH_ZLEN));
 	}
 	rtl_flush_tx_desc(&tpc->TxDescArray[entry]);
-	RTL_W8(TxPoll, 0x40);	/* set polling bit */
+	if (pplat->device == 0x8125)
+		RTL_W8(TxPoll_8125, 0x1);	/* set polling bit */
+	else
+		RTL_W8(TxPoll_8169, 0x40);	/* set polling bit */
 
 	tpc->cur_tx++;
 	to = currticks() + TX_TIMEOUT;
@@ -824,21 +841,26 @@ static int rtl8169_eth_start(struct udevice *dev)
 	return 0;
 }
 
-static void rtl_halt_common(unsigned long dev_iobase)
+static void rtl_halt_common(struct udevice *dev)
 {
+	struct rtl8169_private *priv = dev_get_priv(dev);
+	struct pci_child_plat *pplat = dev_get_parent_plat(dev);
 	int i;
 
 #ifdef DEBUG_RTL8169
 	printf ("%s\n", __FUNCTION__);
 #endif
 
-	ioaddr = dev_iobase;
+	ioaddr = priv->iobase;
 
 	/* Stop the chip's Tx and Rx DMA processes. */
 	RTL_W8(ChipCmd, 0x00);
 
 	/* Disable interrupts by clearing the interrupt mask. */
-	RTL_W16(IntrMask, 0x0000);
+	if (pplat->device == 0x8125)
+		RTL_W16(IntrMask_8125, 0x0000);
+	else
+		RTL_W16(IntrMask_8169, 0x0000);
 
 	RTL_W32(RxMissed, 0);
 
@@ -849,9 +871,7 @@ static void rtl_halt_common(unsigned long dev_iobase)
 
 void rtl8169_eth_stop(struct udevice *dev)
 {
-	struct rtl8169_private *priv = dev_get_priv(dev);
-
-	rtl_halt_common(priv->iobase);
+	rtl_halt_common(dev);
 }
 
 static int rtl8169_write_hwaddr(struct udevice *dev)
@@ -1025,23 +1045,25 @@ static int rtl8169_eth_probe(struct udevice *dev)
 	struct pci_child_plat *pplat = dev_get_parent_plat(dev);
 	struct rtl8169_private *priv = dev_get_priv(dev);
 	struct eth_pdata *plat = dev_get_plat(dev);
-	u32 iobase;
 	int region;
 	int ret;
 
-	debug("rtl8169: REALTEK RTL8169 @0x%x\n", iobase);
 	switch (pplat->device) {
 	case 0x8168:
+	case 0x8125:
 		region = 2;
 		break;
 	default:
 		region = 1;
 		break;
 	}
-	dm_pci_read_config32(dev, PCI_BASE_ADDRESS_0 + region * 4, &iobase);
-	iobase &= ~0xf;
-	priv->iobase = (int)dm_pci_mem_to_phys(dev, iobase);
 
+	priv->iobase = (ulong)dm_pci_map_bar(dev,
+					     PCI_BASE_ADDRESS_0 + region * 4,
+					     0, 0,
+					     PCI_REGION_TYPE, PCI_REGION_MEM);
+
+	debug("rtl8169: REALTEK RTL8169 @0x%lx\n", priv->iobase);
 	ret = rtl_init(priv->iobase, dev->name, plat->enetaddr);
 	if (ret < 0) {
 		printf(pr_fmt("failed to initialize card: %d\n"), ret);
-- 
2.34.1

[PATCH 2/2] configs: rock5b-rk3588: add rtl8169 driver

[Eugen Hristev]

Add the rtl8169 driver, which supports the rtl8125b device, which is
connected on the pciE bus on this board.
Enable also CONFIG_SYS_HAS_NONCACHED_MEMORY to have the descriptors stored.

Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>
---
 configs/rock5b-rk3588_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/configs/rock5b-rk3588_defconfig b/configs/rock5b-rk3588_defconfig
index a14fcd2ee924..bfa48227aee2 100644
--- a/configs/rock5b-rk3588_defconfig
+++ b/configs/rock5b-rk3588_defconfig
@@ -1,5 +1,6 @@
 CONFIG_ARM=y
 CONFIG_SKIP_LOWLEVEL_INIT=y
+CONFIG_SYS_HAS_NONCACHED_MEMORY=y
 CONFIG_COUNTER_FREQUENCY=24000000
 CONFIG_ARCH_ROCKCHIP=y
 CONFIG_TEXT_BASE=0x00a00000
@@ -71,6 +72,7 @@ CONFIG_MMC_SDHCI_SDMA=y
 CONFIG_MMC_SDHCI_ROCKCHIP=y
 CONFIG_SPI_FLASH_MACRONIX=y
 CONFIG_ETH_DESIGNWARE=y
+CONFIG_RTL8169=y
 CONFIG_GMAC_ROCKCHIP=y
 CONFIG_PCI=y
 CONFIG_PCIE_DW_ROCKCHIP=y
-- 
2.34.1

Re: [PATCH 1/2] net: rtl8169: add minimal support for 8125B variant

[Eugen Hristev]

On 4/25/23 16:06, Eugen Hristev wrote:
> Add minimal support for 8125B version.
> Changes are based on the Linux driver.
> Tested on Radxa Rock 5B Rk3588 board.
> 
> Connection to a laptop worked fine in 100 Mbps mode.
> 1000 Mbps mode is not working at the moment.
> 
> Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>
> ---

The one thing that impacts all the rtl chips is the way the pci BAR is 
now mapped.
I could not test this on another platform so help on this matter is 
appreciated.

Thanks!
Eugen

>   drivers/net/rtl8169.c | 52 ++++++++++++++++++++++++++++++-------------
>   1 file changed, 37 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/net/rtl8169.c b/drivers/net/rtl8169.c
> index c9c07a5a8ffe..2276a465e787 100644
> --- a/drivers/net/rtl8169.c
> +++ b/drivers/net/rtl8169.c
> @@ -118,9 +118,9 @@ enum RTL8169_registers {
>   	FLASH = 0x30,
>   	ERSR = 0x36,
>   	ChipCmd = 0x37,
> -	TxPoll = 0x38,
> -	IntrMask = 0x3C,
> -	IntrStatus = 0x3E,
> +	TxPoll_8169 = 0x38,
> +	IntrMask_8169 = 0x3C,
> +	IntrStatus_8169 = 0x3E,
>   	TxConfig = 0x40,
>   	RxConfig = 0x44,
>   	RxMissed = 0x4C,
> @@ -148,6 +148,12 @@ enum RTL8169_registers {
>   	FuncForceEvent = 0xFC,
>   };
>   
> +enum RTL8125_registers {
> +	IntrMask_8125 = 0x38,
> +	IntrStatus_8125 = 0x3C,
> +	TxPoll_8125 = 0x90,
> +};
> +
>   enum RTL8169_register_content {
>   	/*InterruptStatusBits */
>   	SYSErr = 0x8000,
> @@ -263,6 +269,7 @@ static struct {
>   	{"RTL-8101e",		0x34, 0xff7e1880,},
>   	{"RTL-8100e",		0x32, 0xff7e1880,},
>   	{"RTL-8168h/8111h",	0x54, 0xff7e1880,},
> +	{"RTL-8125B",		0x64, 0xff7e1880,},
>   };
>   
>   enum _DescStatusBit {
> @@ -347,6 +354,7 @@ static struct pci_device_id supported[] = {
>   	{ PCI_DEVICE(PCI_VENDOR_ID_REALTEK, 0x8167) },
>   	{ PCI_DEVICE(PCI_VENDOR_ID_REALTEK, 0x8168) },
>   	{ PCI_DEVICE(PCI_VENDOR_ID_REALTEK, 0x8169) },
> +	{ PCI_DEVICE(PCI_VENDOR_ID_REALTEK, 0x8125) },
>   	{}
>   };
>   
> @@ -517,6 +525,7 @@ static int rtl_recv_common(struct udevice *dev, unsigned long dev_iobase,
>   	/* return true if there's an ethernet packet ready to read */
>   	/* nic->packet should contain data on return */
>   	/* nic->packetlen should contain length of data */
> +	struct pci_child_plat *pplat = dev_get_parent_plat(dev);
>   	int cur_rx;
>   	int length = 0;
>   
> @@ -558,6 +567,10 @@ static int rtl_recv_common(struct udevice *dev, unsigned long dev_iobase,
>   		return length;
>   
>   	} else {
> +		u32 IntrStatus = IntrStatus_8169;
> +
> +		if (pplat->device == 0x8125)
> +			IntrStatus = IntrStatus_8125;
>   		ushort sts = RTL_R8(IntrStatus);
>   		RTL_W8(IntrStatus, sts & ~(TxErr | RxErr | SYSErr));
>   		udelay(100);	/* wait */
> @@ -582,6 +595,7 @@ static int rtl_send_common(struct udevice *dev, unsigned long dev_iobase,
>   {
>   	/* send the packet to destination */
>   
> +	struct pci_child_plat *pplat = dev_get_parent_plat(dev);
>   	u32 to;
>   	u8 *ptxb;
>   	int entry = tpc->cur_tx % NUM_TX_DESC;
> @@ -618,7 +632,10 @@ static int rtl_send_common(struct udevice *dev, unsigned long dev_iobase,
>   				    ((len > ETH_ZLEN) ? len : ETH_ZLEN));
>   	}
>   	rtl_flush_tx_desc(&tpc->TxDescArray[entry]);
> -	RTL_W8(TxPoll, 0x40);	/* set polling bit */
> +	if (pplat->device == 0x8125)
> +		RTL_W8(TxPoll_8125, 0x1);	/* set polling bit */
> +	else
> +		RTL_W8(TxPoll_8169, 0x40);	/* set polling bit */
>   
>   	tpc->cur_tx++;
>   	to = currticks() + TX_TIMEOUT;
> @@ -824,21 +841,26 @@ static int rtl8169_eth_start(struct udevice *dev)
>   	return 0;
>   }
>   
> -static void rtl_halt_common(unsigned long dev_iobase)
> +static void rtl_halt_common(struct udevice *dev)
>   {
> +	struct rtl8169_private *priv = dev_get_priv(dev);
> +	struct pci_child_plat *pplat = dev_get_parent_plat(dev);
>   	int i;
>   
>   #ifdef DEBUG_RTL8169
>   	printf ("%s\n", __FUNCTION__);
>   #endif
>   
> -	ioaddr = dev_iobase;
> +	ioaddr = priv->iobase;
>   
>   	/* Stop the chip's Tx and Rx DMA processes. */
>   	RTL_W8(ChipCmd, 0x00);
>   
>   	/* Disable interrupts by clearing the interrupt mask. */
> -	RTL_W16(IntrMask, 0x0000);
> +	if (pplat->device == 0x8125)
> +		RTL_W16(IntrMask_8125, 0x0000);
> +	else
> +		RTL_W16(IntrMask_8169, 0x0000);
>   
>   	RTL_W32(RxMissed, 0);
>   
> @@ -849,9 +871,7 @@ static void rtl_halt_common(unsigned long dev_iobase)
>   
>   void rtl8169_eth_stop(struct udevice *dev)
>   {
> -	struct rtl8169_private *priv = dev_get_priv(dev);
> -
> -	rtl_halt_common(priv->iobase);
> +	rtl_halt_common(dev);
>   }
>   
>   static int rtl8169_write_hwaddr(struct udevice *dev)
> @@ -1025,23 +1045,25 @@ static int rtl8169_eth_probe(struct udevice *dev)
>   	struct pci_child_plat *pplat = dev_get_parent_plat(dev);
>   	struct rtl8169_private *priv = dev_get_priv(dev);
>   	struct eth_pdata *plat = dev_get_plat(dev);
> -	u32 iobase;
>   	int region;
>   	int ret;
>   
> -	debug("rtl8169: REALTEK RTL8169 @0x%x\n", iobase);
>   	switch (pplat->device) {
>   	case 0x8168:
> +	case 0x8125:
>   		region = 2;
>   		break;
>   	default:
>   		region = 1;
>   		break;
>   	}
> -	dm_pci_read_config32(dev, PCI_BASE_ADDRESS_0 + region * 4, &iobase);
> -	iobase &= ~0xf;
> -	priv->iobase = (int)dm_pci_mem_to_phys(dev, iobase);
>   
> +	priv->iobase = (ulong)dm_pci_map_bar(dev,
> +					     PCI_BASE_ADDRESS_0 + region * 4,
> +					     0, 0,
> +					     PCI_REGION_TYPE, PCI_REGION_MEM);
> +
> +	debug("rtl8169: REALTEK RTL8169 @0x%lx\n", priv->iobase);
>   	ret = rtl_init(priv->iobase, dev->name, plat->enetaddr);
>   	if (ret < 0) {
>   		printf(pr_fmt("failed to initialize card: %d\n"), ret);

Re: [PATCH 1/2] net: rtl8169: add minimal support for 8125B variant

[Ramon Fried]

On Tue, Apr 25, 2023 at 4:17 PM Eugen Hristev
<eugen.hristev@collabora.com> wrote:
>
> On 4/25/23 16:06, Eugen Hristev wrote:
> > Add minimal support for 8125B version.
> > Changes are based on the Linux driver.
> > Tested on Radxa Rock 5B Rk3588 board.
> >
> > Connection to a laptop worked fine in 100 Mbps mode.
> > 1000 Mbps mode is not working at the moment.
> >
> > Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>
> > ---
>
> The one thing that impacts all the rtl chips is the way the pci BAR is
> now mapped.
> I could not test this on another platform so help on this matter is
> appreciated.
>
> Thanks!
> Eugen
Let's wait a bit to see if someone can test it. why did you change the
mapping of the BAR ?

Re: [PATCH 1/2] net: rtl8169: add minimal support for 8125B variant

[Eugen Hristev]

On 4/25/23 22:22, Ramon Fried wrote:
> On Tue, Apr 25, 2023 at 4:17 PM Eugen Hristev
> <eugen.hristev@collabora.com> wrote:
>>
>> On 4/25/23 16:06, Eugen Hristev wrote:
>>> Add minimal support for 8125B version.
>>> Changes are based on the Linux driver.
>>> Tested on Radxa Rock 5B Rk3588 board.
>>>
>>> Connection to a laptop worked fine in 100 Mbps mode.
>>> 1000 Mbps mode is not working at the moment.
>>>
>>> Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>
>>> ---
>>
>> The one thing that impacts all the rtl chips is the way the pci BAR is
>> now mapped.
>> I could not test this on another platform so help on this matter is
>> appreciated.
>>
>> Thanks!
>> Eugen
> Let's wait a bit to see if someone can test it. why did you change the
> mapping of the BAR ?

It did not work with the old code. It provided a bad address, to some 
area which did not have the right registers.
I looked into similar drivers and they were using this call, which works 
perfectly for 8125b device

Eugen

Re: [PATCH 1/2] net: rtl8169: add minimal support for 8125B variant

[Ramon Fried]

On Tue, Apr 25, 2023 at 10:47 PM Eugen Hristev
<eugen.hristev@collabora.com> wrote:
>
> On 4/25/23 22:22, Ramon Fried wrote:
> > On Tue, Apr 25, 2023 at 4:17 PM Eugen Hristev
> > <eugen.hristev@collabora.com> wrote:
> >>
> >> On 4/25/23 16:06, Eugen Hristev wrote:
> >>> Add minimal support for 8125B version.
> >>> Changes are based on the Linux driver.
> >>> Tested on Radxa Rock 5B Rk3588 board.
> >>>
> >>> Connection to a laptop worked fine in 100 Mbps mode.
> >>> 1000 Mbps mode is not working at the moment.
> >>>
> >>> Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>
> >>> ---
> >>
> >> The one thing that impacts all the rtl chips is the way the pci BAR is
> >> now mapped.
> >> I could not test this on another platform so help on this matter is
> >> appreciated.
> >>
> >> Thanks!
> >> Eugen
> > Let's wait a bit to see if someone can test it. why did you change the
> > mapping of the BAR ?
>
> It did not work with the old code. It provided a bad address, to some
> area which did not have the right registers.
> I looked into similar drivers and they were using this call, which works
> perfectly for 8125b device
>
> Eugen
Ok.
Hopefully nothing breaks.
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>

Re: [PATCH 2/2] configs: rock5b-rk3588: add rtl8169 driver

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1151 bytes --]

On Tue, Apr 25, 2023 at 04:06:59PM +0300, Eugen Hristev wrote:
> Add the rtl8169 driver, which supports the rtl8125b device, which is
> connected on the pciE bus on this board.
> Enable also CONFIG_SYS_HAS_NONCACHED_MEMORY to have the descriptors stored.
> 
> Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>
> ---
>  configs/rock5b-rk3588_defconfig | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/configs/rock5b-rk3588_defconfig b/configs/rock5b-rk3588_defconfig
> index a14fcd2ee924..bfa48227aee2 100644
> --- a/configs/rock5b-rk3588_defconfig
> +++ b/configs/rock5b-rk3588_defconfig
> @@ -1,5 +1,6 @@
>  CONFIG_ARM=y
>  CONFIG_SKIP_LOWLEVEL_INIT=y
> +CONFIG_SYS_HAS_NONCACHED_MEMORY=y
>  CONFIG_COUNTER_FREQUENCY=24000000
>  CONFIG_ARCH_ROCKCHIP=y
>  CONFIG_TEXT_BASE=0x00a00000
> @@ -71,6 +72,7 @@ CONFIG_MMC_SDHCI_SDMA=y
>  CONFIG_MMC_SDHCI_ROCKCHIP=y
>  CONFIG_SPI_FLASH_MACRONIX=y
>  CONFIG_ETH_DESIGNWARE=y
> +CONFIG_RTL8169=y
>  CONFIG_GMAC_ROCKCHIP=y
>  CONFIG_PCI=y
>  CONFIG_PCIE_DW_ROCKCHIP=y

Does this depend on some series I missed where PCI is enabled on this
config?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 1/2] net: rtl8169: add minimal support for 8125B variant

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 465 bytes --]

On Tue, Apr 25, 2023 at 04:06:58PM +0300, Eugen Hristev wrote:

> Add minimal support for 8125B version.
> Changes are based on the Linux driver.
> Tested on Radxa Rock 5B Rk3588 board.
> 
> Connection to a laptop worked fine in 100 Mbps mode.
> 1000 Mbps mode is not working at the moment.
> 
> Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>
> Reviewed-by: Ramon Fried <rfried.dev@gmail.com>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 2/2] configs: rock5b-rk3588: add rtl8169 driver

[Eugen Hristev]

On 5/6/23 01:03, Tom Rini wrote:
> On Tue, Apr 25, 2023 at 04:06:59PM +0300, Eugen Hristev wrote:
>> Add the rtl8169 driver, which supports the rtl8125b device, which is
>> connected on the pciE bus on this board.
>> Enable also CONFIG_SYS_HAS_NONCACHED_MEMORY to have the descriptors stored.
>>
>> Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>
>> ---
>>   configs/rock5b-rk3588_defconfig | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/configs/rock5b-rk3588_defconfig b/configs/rock5b-rk3588_defconfig
>> index a14fcd2ee924..bfa48227aee2 100644
>> --- a/configs/rock5b-rk3588_defconfig
>> +++ b/configs/rock5b-rk3588_defconfig
>> @@ -1,5 +1,6 @@
>>   CONFIG_ARM=y
>>   CONFIG_SKIP_LOWLEVEL_INIT=y
>> +CONFIG_SYS_HAS_NONCACHED_MEMORY=y
>>   CONFIG_COUNTER_FREQUENCY=24000000
>>   CONFIG_ARCH_ROCKCHIP=y
>>   CONFIG_TEXT_BASE=0x00a00000
>> @@ -71,6 +72,7 @@ CONFIG_MMC_SDHCI_SDMA=y
>>   CONFIG_MMC_SDHCI_ROCKCHIP=y
>>   CONFIG_SPI_FLASH_MACRONIX=y
>>   CONFIG_ETH_DESIGNWARE=y
>> +CONFIG_RTL8169=y
>>   CONFIG_GMAC_ROCKCHIP=y
>>   CONFIG_PCI=y
>>   CONFIG_PCIE_DW_ROCKCHIP=y
> 
> Does this depend on some series I missed where PCI is enabled on this
> config?
> 

Hi Tom,

the network device is connected on PCI express, that is right. I have a 
separate series for that.
However, the network driver itself and the enabling of the network 
driver is somewhat independent, so I sent it as a separate series (this 
one )
I think the config patch can go through rockchip custodian tree.

Thanks,
Eugen

Re: [PATCH 2/2] configs: rock5b-rk3588: add rtl8169 driver

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1831 bytes --]

On Mon, May 15, 2023 at 03:36:24PM +0300, Eugen Hristev wrote:
> On 5/6/23 01:03, Tom Rini wrote:
> > On Tue, Apr 25, 2023 at 04:06:59PM +0300, Eugen Hristev wrote:
> > > Add the rtl8169 driver, which supports the rtl8125b device, which is
> > > connected on the pciE bus on this board.
> > > Enable also CONFIG_SYS_HAS_NONCACHED_MEMORY to have the descriptors stored.
> > > 
> > > Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>
> > > ---
> > >   configs/rock5b-rk3588_defconfig | 2 ++
> > >   1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/configs/rock5b-rk3588_defconfig b/configs/rock5b-rk3588_defconfig
> > > index a14fcd2ee924..bfa48227aee2 100644
> > > --- a/configs/rock5b-rk3588_defconfig
> > > +++ b/configs/rock5b-rk3588_defconfig
> > > @@ -1,5 +1,6 @@
> > >   CONFIG_ARM=y
> > >   CONFIG_SKIP_LOWLEVEL_INIT=y
> > > +CONFIG_SYS_HAS_NONCACHED_MEMORY=y
> > >   CONFIG_COUNTER_FREQUENCY=24000000
> > >   CONFIG_ARCH_ROCKCHIP=y
> > >   CONFIG_TEXT_BASE=0x00a00000
> > > @@ -71,6 +72,7 @@ CONFIG_MMC_SDHCI_SDMA=y
> > >   CONFIG_MMC_SDHCI_ROCKCHIP=y
> > >   CONFIG_SPI_FLASH_MACRONIX=y
> > >   CONFIG_ETH_DESIGNWARE=y
> > > +CONFIG_RTL8169=y
> > >   CONFIG_GMAC_ROCKCHIP=y
> > >   CONFIG_PCI=y
> > >   CONFIG_PCIE_DW_ROCKCHIP=y
> > 
> > Does this depend on some series I missed where PCI is enabled on this
> > config?
> > 
> 
> Hi Tom,
> 
> the network device is connected on PCI express, that is right. I have a
> separate series for that.
> However, the network driver itself and the enabling of the network driver is
> somewhat independent, so I sent it as a separate series (this one )
> I think the config patch can go through rockchip custodian tree.

With 1/2 now applied this results in failure to build due to DM_PCI not
being enabled.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 2/2] configs: rock5b-rk3588: add rtl8169 driver

[Eugen Hristev]

On 5/15/23 15:52, Tom Rini wrote:
> On Mon, May 15, 2023 at 03:36:24PM +0300, Eugen Hristev wrote:
>> On 5/6/23 01:03, Tom Rini wrote:
>>> On Tue, Apr 25, 2023 at 04:06:59PM +0300, Eugen Hristev wrote:
>>>> Add the rtl8169 driver, which supports the rtl8125b device, which is
>>>> connected on the pciE bus on this board.
>>>> Enable also CONFIG_SYS_HAS_NONCACHED_MEMORY to have the descriptors stored.
>>>>
>>>> Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>
>>>> ---
>>>>    configs/rock5b-rk3588_defconfig | 2 ++
>>>>    1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/configs/rock5b-rk3588_defconfig b/configs/rock5b-rk3588_defconfig
>>>> index a14fcd2ee924..bfa48227aee2 100644
>>>> --- a/configs/rock5b-rk3588_defconfig
>>>> +++ b/configs/rock5b-rk3588_defconfig
>>>> @@ -1,5 +1,6 @@
>>>>    CONFIG_ARM=y
>>>>    CONFIG_SKIP_LOWLEVEL_INIT=y
>>>> +CONFIG_SYS_HAS_NONCACHED_MEMORY=y
>>>>    CONFIG_COUNTER_FREQUENCY=24000000
>>>>    CONFIG_ARCH_ROCKCHIP=y
>>>>    CONFIG_TEXT_BASE=0x00a00000
>>>> @@ -71,6 +72,7 @@ CONFIG_MMC_SDHCI_SDMA=y
>>>>    CONFIG_MMC_SDHCI_ROCKCHIP=y
>>>>    CONFIG_SPI_FLASH_MACRONIX=y
>>>>    CONFIG_ETH_DESIGNWARE=y
>>>> +CONFIG_RTL8169=y
>>>>    CONFIG_GMAC_ROCKCHIP=y
>>>>    CONFIG_PCI=y
>>>>    CONFIG_PCIE_DW_ROCKCHIP=y
>>>
>>> Does this depend on some series I missed where PCI is enabled on this
>>> config?
>>>
>>
>> Hi Tom,
>>
>> the network device is connected on PCI express, that is right. I have a
>> separate series for that.
>> However, the network driver itself and the enabling of the network driver is
>> somewhat independent, so I sent it as a separate series (this one )
>> I think the config patch can go through rockchip custodian tree.
> 
> With 1/2 now applied this results in failure to build due to DM_PCI not
> being enabled.
> 

This is most strange. 1/2 are just changes in the driver. How does that 
cause a build failure ?
The driver is not even selected for this config in 2/2, before the 
actual patch.
Do you have the log of the failure so I can have a look ?

Re: [PATCH 2/2] configs: rock5b-rk3588: add rtl8169 driver

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2494 bytes --]

On Mon, May 15, 2023 at 03:57:14PM +0300, Eugen Hristev wrote:
> On 5/15/23 15:52, Tom Rini wrote:
> > On Mon, May 15, 2023 at 03:36:24PM +0300, Eugen Hristev wrote:
> > > On 5/6/23 01:03, Tom Rini wrote:
> > > > On Tue, Apr 25, 2023 at 04:06:59PM +0300, Eugen Hristev wrote:
> > > > > Add the rtl8169 driver, which supports the rtl8125b device, which is
> > > > > connected on the pciE bus on this board.
> > > > > Enable also CONFIG_SYS_HAS_NONCACHED_MEMORY to have the descriptors stored.
> > > > > 
> > > > > Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>
> > > > > ---
> > > > >    configs/rock5b-rk3588_defconfig | 2 ++
> > > > >    1 file changed, 2 insertions(+)
> > > > > 
> > > > > diff --git a/configs/rock5b-rk3588_defconfig b/configs/rock5b-rk3588_defconfig
> > > > > index a14fcd2ee924..bfa48227aee2 100644
> > > > > --- a/configs/rock5b-rk3588_defconfig
> > > > > +++ b/configs/rock5b-rk3588_defconfig
> > > > > @@ -1,5 +1,6 @@
> > > > >    CONFIG_ARM=y
> > > > >    CONFIG_SKIP_LOWLEVEL_INIT=y
> > > > > +CONFIG_SYS_HAS_NONCACHED_MEMORY=y
> > > > >    CONFIG_COUNTER_FREQUENCY=24000000
> > > > >    CONFIG_ARCH_ROCKCHIP=y
> > > > >    CONFIG_TEXT_BASE=0x00a00000
> > > > > @@ -71,6 +72,7 @@ CONFIG_MMC_SDHCI_SDMA=y
> > > > >    CONFIG_MMC_SDHCI_ROCKCHIP=y
> > > > >    CONFIG_SPI_FLASH_MACRONIX=y
> > > > >    CONFIG_ETH_DESIGNWARE=y
> > > > > +CONFIG_RTL8169=y
> > > > >    CONFIG_GMAC_ROCKCHIP=y
> > > > >    CONFIG_PCI=y
> > > > >    CONFIG_PCIE_DW_ROCKCHIP=y
> > > > 
> > > > Does this depend on some series I missed where PCI is enabled on this
> > > > config?
> > > > 
> > > 
> > > Hi Tom,
> > > 
> > > the network device is connected on PCI express, that is right. I have a
> > > separate series for that.
> > > However, the network driver itself and the enabling of the network driver is
> > > somewhat independent, so I sent it as a separate series (this one )
> > > I think the config patch can go through rockchip custodian tree.
> > 
> > With 1/2 now applied this results in failure to build due to DM_PCI not
> > being enabled.
> > 
> 
> This is most strange. 1/2 are just changes in the driver. How does that
> cause a build failure ?
> The driver is not even selected for this config in 2/2, before the actual
> patch.
> Do you have the log of the failure so I can have a look ?

I don't have the log handy right now, please re-test the config changes
on top of master and repost.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 2/2] configs: rock5b-rk3588: add rtl8169 driver

[Eugen Hristev]

On 5/15/23 16:16, Tom Rini wrote:
> On Mon, May 15, 2023 at 03:57:14PM +0300, Eugen Hristev wrote:
>> On 5/15/23 15:52, Tom Rini wrote:
>>> On Mon, May 15, 2023 at 03:36:24PM +0300, Eugen Hristev wrote:
>>>> On 5/6/23 01:03, Tom Rini wrote:
>>>>> On Tue, Apr 25, 2023 at 04:06:59PM +0300, Eugen Hristev wrote:
>>>>>> Add the rtl8169 driver, which supports the rtl8125b device, which is
>>>>>> connected on the pciE bus on this board.
>>>>>> Enable also CONFIG_SYS_HAS_NONCACHED_MEMORY to have the descriptors stored.
>>>>>>
>>>>>> Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>
>>>>>> ---
>>>>>>     configs/rock5b-rk3588_defconfig | 2 ++
>>>>>>     1 file changed, 2 insertions(+)
>>>>>>
>>>>>> diff --git a/configs/rock5b-rk3588_defconfig b/configs/rock5b-rk3588_defconfig
>>>>>> index a14fcd2ee924..bfa48227aee2 100644
>>>>>> --- a/configs/rock5b-rk3588_defconfig
>>>>>> +++ b/configs/rock5b-rk3588_defconfig
>>>>>> @@ -1,5 +1,6 @@
>>>>>>     CONFIG_ARM=y
>>>>>>     CONFIG_SKIP_LOWLEVEL_INIT=y
>>>>>> +CONFIG_SYS_HAS_NONCACHED_MEMORY=y
>>>>>>     CONFIG_COUNTER_FREQUENCY=24000000
>>>>>>     CONFIG_ARCH_ROCKCHIP=y
>>>>>>     CONFIG_TEXT_BASE=0x00a00000
>>>>>> @@ -71,6 +72,7 @@ CONFIG_MMC_SDHCI_SDMA=y
>>>>>>     CONFIG_MMC_SDHCI_ROCKCHIP=y
>>>>>>     CONFIG_SPI_FLASH_MACRONIX=y
>>>>>>     CONFIG_ETH_DESIGNWARE=y
>>>>>> +CONFIG_RTL8169=y
>>>>>>     CONFIG_GMAC_ROCKCHIP=y
>>>>>>     CONFIG_PCI=y
>>>>>>     CONFIG_PCIE_DW_ROCKCHIP=y
>>>>>
>>>>> Does this depend on some series I missed where PCI is enabled on this
>>>>> config?
>>>>>
>>>>
>>>> Hi Tom,
>>>>
>>>> the network device is connected on PCI express, that is right. I have a
>>>> separate series for that.
>>>> However, the network driver itself and the enabling of the network driver is
>>>> somewhat independent, so I sent it as a separate series (this one )
>>>> I think the config patch can go through rockchip custodian tree.
>>>
>>> With 1/2 now applied this results in failure to build due to DM_PCI not
>>> being enabled.
>>>
>>
>> This is most strange. 1/2 are just changes in the driver. How does that
>> cause a build failure ?
>> The driver is not even selected for this config in 2/2, before the actual
>> patch.
>> Do you have the log of the failure so I can have a look ?
> 
> I don't have the log handy right now, please re-test the config changes
> on top of master and repost.
> 


Hi Tom,

I figured out the issue.
My patch is indeed based on another patch that selects CONFIG_PCI.

However, the rtl8169 driver depends on PCI, even if it does not have 
'depends on PCI' in the Kconfig.
So selecting it without PCI, well, the error.

So I sent another patch :

https://patchwork.ozlabs.org/project/uboot/patch/20230517104124.111075-1-eugen.hristev@collabora.com/

... that should have the rtl8169 depend on PCI now.

That being said, this current patch can be applied on kever's rockchip 
master branch on top of PCI series, but if it's to be applied on master, 
it should work on top of the 'rtl8169 depends on PCI' patch now.

I hope it makes sense.
Thanks

Re: [PATCH 1/2] net: rtl8169: add minimal support for 8125B variant

[Eugen Hristev]

On 4/30/23 22:44, Ramon Fried wrote:
> On Tue, Apr 25, 2023 at 10:47 PM Eugen Hristev
> <eugen.hristev@collabora.com> wrote:
>>
>> On 4/25/23 22:22, Ramon Fried wrote:
>>> On Tue, Apr 25, 2023 at 4:17 PM Eugen Hristev
>>> <eugen.hristev@collabora.com> wrote:
>>>>
>>>> On 4/25/23 16:06, Eugen Hristev wrote:
>>>>> Add minimal support for 8125B version.
>>>>> Changes are based on the Linux driver.
>>>>> Tested on Radxa Rock 5B Rk3588 board.
>>>>>
>>>>> Connection to a laptop worked fine in 100 Mbps mode.
>>>>> 1000 Mbps mode is not working at the moment.
>>>>>
>>>>> Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>
>>>>> ---
>>>>
>>>> The one thing that impacts all the rtl chips is the way the pci BAR is
>>>> now mapped.
>>>> I could not test this on another platform so help on this matter is
>>>> appreciated.
>>>>
>>>> Thanks!
>>>> Eugen
>>> Let's wait a bit to see if someone can test it. why did you change the
>>> mapping of the BAR ?
>>
>> It did not work with the old code. It provided a bad address, to some
>> area which did not have the right registers.
>> I looked into similar drivers and they were using this call, which works
>> perfectly for 8125b device
>>
>> Eugen
> Ok.
> Hopefully nothing breaks.
> Reviewed-by: Ramon Fried <rfried.dev@gmail.com>


Hello Ramon,

I have a question if you don't mind, maybe you are better suited to 
answer this :

In the case of rtl8169 which is a pci express device, the probing is 
done on demand (pci enum) or with MISC_INIT_R, anyway, the probing is 
done *after* the board init is done, and in case of rockchip, the board 
init will check if there is any 'ethaddr' set, and if not, set the 
ethaddr by generating an unique cpuid-based MAC. This happens, no 
ethaddr is set, and the MAC is generated.
However, when the rtl8169 probes, it has ofcourse a different hardware 
written MAC, but the uboot subsystem will deny it, and claim ethaddr is 
already set, and it will call the 8169 driver to rewrite its MAC.
This then happens, and Uboot will use the initial ethaddr (the one 
generated) for any kind of packets.
While this is not a problem at first glance, when Linux boots, it will 
read the MAC from rtl8169 ROM (even if uboot attempted to rewrite its 
MAC, being a ROM, it will not be written), and Linux will use a 
different MAC now.
Do you have any idea on how to solve this problem ? I would like that 
once rtl8169 driver probes, the initial ethaddr would be overwritten.

Thanks !

Re: [PATCH 2/2] configs: rock5b-rk3588: add rtl8169 driver

[Kever Yang]

On 2023/4/25 21:06, Eugen Hristev wrote:
> Add the rtl8169 driver, which supports the rtl8125b device, which is
> connected on the pciE bus on this board.
> Enable also CONFIG_SYS_HAS_NONCACHED_MEMORY to have the descriptors stored.
>
> Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever
> ---
>   configs/rock5b-rk3588_defconfig | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/configs/rock5b-rk3588_defconfig b/configs/rock5b-rk3588_defconfig
> index a14fcd2ee924..bfa48227aee2 100644
> --- a/configs/rock5b-rk3588_defconfig
> +++ b/configs/rock5b-rk3588_defconfig
> @@ -1,5 +1,6 @@
>   CONFIG_ARM=y
>   CONFIG_SKIP_LOWLEVEL_INIT=y
> +CONFIG_SYS_HAS_NONCACHED_MEMORY=y
>   CONFIG_COUNTER_FREQUENCY=24000000
>   CONFIG_ARCH_ROCKCHIP=y
>   CONFIG_TEXT_BASE=0x00a00000
> @@ -71,6 +72,7 @@ CONFIG_MMC_SDHCI_SDMA=y
>   CONFIG_MMC_SDHCI_ROCKCHIP=y
>   CONFIG_SPI_FLASH_MACRONIX=y
>   CONFIG_ETH_DESIGNWARE=y
> +CONFIG_RTL8169=y
>   CONFIG_GMAC_ROCKCHIP=y
>   CONFIG_PCI=y
>   CONFIG_PCIE_DW_ROCKCHIP=y

Re: [PATCH 2/2] configs: rock5b-rk3588: add rtl8169 driver

[Eugen Hristev]

On 5/17/23 13:46, Eugen Hristev wrote:
> On 5/15/23 16:16, Tom Rini wrote:
>> On Mon, May 15, 2023 at 03:57:14PM +0300, Eugen Hristev wrote:
>>> On 5/15/23 15:52, Tom Rini wrote:
>>>> On Mon, May 15, 2023 at 03:36:24PM +0300, Eugen Hristev wrote:
>>>>> On 5/6/23 01:03, Tom Rini wrote:
>>>>>> On Tue, Apr 25, 2023 at 04:06:59PM +0300, Eugen Hristev wrote:
>>>>>>> Add the rtl8169 driver, which supports the rtl8125b device, which is
>>>>>>> connected on the pciE bus on this board.
>>>>>>> Enable also CONFIG_SYS_HAS_NONCACHED_MEMORY to have the 
>>>>>>> descriptors stored.
>>>>>>>
>>>>>>> Signed-off-by: Eugen Hristev <eugen.hristev@collabora.com>
>>>>>>> ---
>>>>>>>     configs/rock5b-rk3588_defconfig | 2 ++
>>>>>>>     1 file changed, 2 insertions(+)
>>>>>>>
>>>>>>> diff --git a/configs/rock5b-rk3588_defconfig 
>>>>>>> b/configs/rock5b-rk3588_defconfig
>>>>>>> index a14fcd2ee924..bfa48227aee2 100644
>>>>>>> --- a/configs/rock5b-rk3588_defconfig
>>>>>>> +++ b/configs/rock5b-rk3588_defconfig
>>>>>>> @@ -1,5 +1,6 @@
>>>>>>>     CONFIG_ARM=y
>>>>>>>     CONFIG_SKIP_LOWLEVEL_INIT=y
>>>>>>> +CONFIG_SYS_HAS_NONCACHED_MEMORY=y
>>>>>>>     CONFIG_COUNTER_FREQUENCY=24000000
>>>>>>>     CONFIG_ARCH_ROCKCHIP=y
>>>>>>>     CONFIG_TEXT_BASE=0x00a00000
>>>>>>> @@ -71,6 +72,7 @@ CONFIG_MMC_SDHCI_SDMA=y
>>>>>>>     CONFIG_MMC_SDHCI_ROCKCHIP=y
>>>>>>>     CONFIG_SPI_FLASH_MACRONIX=y
>>>>>>>     CONFIG_ETH_DESIGNWARE=y
>>>>>>> +CONFIG_RTL8169=y
>>>>>>>     CONFIG_GMAC_ROCKCHIP=y
>>>>>>>     CONFIG_PCI=y
>>>>>>>     CONFIG_PCIE_DW_ROCKCHIP=y
>>>>>>
>>>>>> Does this depend on some series I missed where PCI is enabled on this
>>>>>> config?
>>>>>>
>>>>>
>>>>> Hi Tom,
>>>>>
>>>>> the network device is connected on PCI express, that is right. I 
>>>>> have a
>>>>> separate series for that.
>>>>> However, the network driver itself and the enabling of the network 
>>>>> driver is
>>>>> somewhat independent, so I sent it as a separate series (this one )
>>>>> I think the config patch can go through rockchip custodian tree.
>>>>
>>>> With 1/2 now applied this results in failure to build due to DM_PCI not
>>>> being enabled.
>>>>
>>>
>>> This is most strange. 1/2 are just changes in the driver. How does that
>>> cause a build failure ?
>>> The driver is not even selected for this config in 2/2, before the 
>>> actual
>>> patch.
>>> Do you have the log of the failure so I can have a look ?
>>
>> I don't have the log handy right now, please re-test the config changes
>> on top of master and repost.
>>
> 
> 
> Hi Tom,
> 
> I figured out the issue.
> My patch is indeed based on another patch that selects CONFIG_PCI.
> 
> However, the rtl8169 driver depends on PCI, even if it does not have 
> 'depends on PCI' in the Kconfig.
> So selecting it without PCI, well, the error.
> 
> So I sent another patch :
> 
> https://patchwork.ozlabs.org/project/uboot/patch/20230517104124.111075-1-eugen.hristev@collabora.com/
> 
> ... that should have the rtl8169 depend on PCI now.
> 
> That being said, this current patch can be applied on kever's rockchip 
> master branch on top of PCI series, but if it's to be applied on master, 
> it should work on top of the 'rtl8169 depends on PCI' patch now.
> 
> I hope it makes sense.
> Thanks
> 

Hi Kever,

Maybe you somehow missed this patch ? The prerequisites(PCI series, 
driver, and PCI dependency for the driver are already in tree)

Thanks,
Eugen