[Edgecore][as4500-52p][as4581-52p] Add new platform#285

Open

brandonchuang wants to merge 2 commits intodentproject:mainfrom

brandonchuang:as4560-52p

Contributor

brandonchuang commented Oct 6, 2023 •

edited

Loading

CPU:
[as4500-52p] Marvell 98DX3530 with integrated CPU
[as4581-52p] COMe CPU Module

MAC: Marvell 98DX3530
PHY: Marvell 88E1780 x 4 (1G port 16~32)
Marvell 88E2780 x 2 (Migi-G port 33-48)
DRAM: 8GB(MAC) DDR4 SDRAM
AirFlow: Front To Back
Function port: 1 x USB port
1 x RJ45 Mgmt port
1 x RJ45 Console port
Ethernet Port: 48 x 1G
Uplink port: 4xSFP+
PoE: Microsemi PD69208M x 12 + PD69210 x 2

DTS:
The DTS for as4500-52p/as4581-52p
dentproject/linux#12
dentproject/linux#11

Signed-off-by: Brandon Chuang brandon_chuang@edge-core.com

KanjiMonster requested changes

View reviewed changes

Contributor

KanjiMonster left a comment

Please make use of managed functions, and properly model the devices in the devicetree (it exists for a reason), if you haven't.

packages/base/any/kernels/5.15-lts/configs/arm64-all/arm64-all.config

    
              # CONFIG_USB_RTL8150 is not set

              # CONFIG_USB_RTL8152 is not set

              # CONFIG_USB_LAN78XX is not set

              CONFIG_USB_LAN78XX=y

Contributor

KanjiMonster Oct 6, 2023

why do you need this? Does this device use an USB-LAN adapter?

Contributor Author

brandonchuang Feb 21, 2024

as4560 use this USB-LAN adapter

packages/base/any/kernels/5.15-lts/configs/arm64-all/arm64-all.config Outdated

Comment on lines 7387 to 7392

    
              # CONFIG_CRYPTO_MANAGER_DISABLE_TESTS is not set

              CONFIG_CRYPTO_MANAGER_DISABLE_TESTS=y

Contributor

KanjiMonster Oct 6, 2023

Why do you need to disable them?

Contributor Author

brandonchuang Feb 21, 2024

Reverted.

packages/base/any/kernels/5.15-lts/kconfig.mk

               K_GIT := 1
               K_GIT_URL := https://github.com/dentproject/linux.git
-              K_GIT_BRANCH := dent-linux-5.15.y
+              K_GIT_BRANCH := dent-linux-5.15.105

Contributor

KanjiMonster Oct 6, 2023

This shouldn't be part of this PR.

Contributor Author

brandonchuang Feb 21, 2024

May I know if there is any plan to change default branch to "dent-linux-5.15.105"?

packages/platforms/accton/arm64/as456x-52p/src/modules/optoe.c

Comment on lines 1 to 3

+              /*
+               * optoe.c - A driver to read and write the EEPROM on optical transceivers
+               * (SFP, QSFP and similar I2C based devices)

Contributor

KanjiMonster Oct 6, 2023

I'd rather have the changes needed added to the main/default optoe.c instead of a copy with unknown changes, which will then never receive any fixes or updates.

packages/platforms/accton/arm64/as456x-52p/src/modules/arm64-accton-as456x-52p-sfp.c Outdated

Comment on lines 340 to 344

+              	data->pdev = platform_device_register_simple(DRVNAME, -1, NULL, 0);
+              	if (IS_ERR(data->pdev)) {
+              		ret = PTR_ERR(data->pdev);
+              		goto dev_reg_err;
+              	}

Contributor

KanjiMonster Oct 6, 2023

Since this driver/device relies on the CPLD present, it would make more sense for the CPLD driver to register the platform device on probe instead of this module.

Contributor

KanjiMonster Oct 23, 2023

FWIW, to show what I want I did this for the AS4610 drivers (which use a similar pattern) with opencomputeproject/OpenNetworkLinux#963 - this should even simplify the drivers.

Contributor Author

brandonchuang Feb 21, 2024

The driver has been modified to follow the design of the AS4610.

packages/platforms/accton/arm64/as456x-52p/src/modules/arm64-accton-as456x-52p-mux.c Outdated

Comment on lines 151 to 159

+              	case as456x_cpldm_mux:
+              		ret = as456x_mux_write(muxc->parent, client,
+              				CPLD_CHANNEL_SELECT_REG, CPLD_CHANNEL_DESELECT_VAL);
+              		break;
+              	case pca9641_mux:
+              		ret = as456x_mux_write(muxc->parent, client, 0x1, 0);
+              		break;
+              	default:
+              		break;

Contributor

KanjiMonster Oct 6, 2023

Likewise. Or add select_reg and deselect_val to struct as456x_mux_data, then you don't need a switch statement here at all.

Contributor Author

brandonchuang Feb 21, 2024

Modified.

packages/platforms/accton/arm64/as456x-52p/src/modules/arm64-accton-as456x-52p-mux.c Outdated

+              				CPLD_CHANNEL_SELECT_REG, CPLD_CHANNEL_DESELECT_VAL);
+              		break;
+              	case pca9641_mux:
+              		ret = as456x_mux_write(muxc->parent, client, 0x1, 0);

Contributor

KanjiMonster Oct 6, 2023

Suggested change

      
            		ret = as456x_mux_write(muxc->parent, client, 0x1, 0);
          
            		ret = as456x_mux_write(muxc->parent, client,
          
            				PCA9641_DESELECT_CHANNEL_REG, PCA9641_DESELECT_CHANNEL_VAL);

Contributor Author

brandonchuang Feb 21, 2024

Modified.

packages/platforms/accton/arm64/as456x-52p/src/modules/arm64-accton-as456x-52p-mux.c Outdated

+              	switch (data->type) {
+              	case as456x_cpldm_mux:
+              		ret = as456x_mux_write(muxc->parent, client,
+              				CPLD_CHANNEL_SELECT_REG, 1 << chan);

Contributor

KanjiMonster Oct 6, 2023

Suggested change

      
            				CPLD_CHANNEL_SELECT_REG, 1 << chan);
          
            				CPLD_CHANNEL_SELECT_REG, BIT(chan));

Contributor Author

brandonchuang Feb 21, 2024

Modified.

packages/platforms/accton/arm64/as456x-52p/src/modules/arm64-accton-as456x-52p-mux.c Outdated

+              				ret = -EIO;
+              				/* Try to close the mux channel */
+              				as456x_mux_write(muxc->parent, client, 0x1, 0);

Contributor

KanjiMonster Oct 6, 2023

Suggested change

      
            				as456x_mux_write(muxc->parent, client, 0x1, 0);
          
            				as456x_mux_write(muxc->parent, client, PCA9641_DESELECT_CHANNEL_REG, PCA9641_DESELECT_CHANNEL_VAL);

Contributor Author

brandonchuang Feb 21, 2024

Modified.

packages/platforms/accton/arm64/as456x-52p/src/modules/arm64-accton-as456x-52p-mux.c Outdated

Comment on lines 201 to 208

+              		if (ret)
+              			goto exit_mux;
+              	}
+              	return 0;
+              exit_mux:
+              	as456x_mux_cleanup(muxc);

Contributor

KanjiMonster Oct 6, 2023

To avoid a goto here:

Suggested change

      
            		if (ret)
          
            			goto exit_mux;
          
            	}
          
            	return 0;
          
            exit_mux:
          
            	as456x_mux_cleanup(muxc);
          
            		if (ret)
          
            			break;
          
            	}
          
            	if (ret)
          
            		as456x_mux_cleanup(muxc);

Contributor Author

brandonchuang Feb 21, 2024

Modified.

Contributor

KanjiMonster commented Oct 6, 2023

Since this PR depends on the devicetree file being added to the kernel, you should mention it in your description and link the appropriate PR.

KanjiMonster reviewed

View reviewed changes

packages/platforms/accton/arm64/as456x-52p/src/modules/arm64-accton-as456x-52p-mux.c

+              MODULE_DEVICE_TABLE(i2c, as456x_mux_id);
+              static const struct of_device_id as456x_mux_of_match[] = {
+              	{ .compatible = "edgecore,pca9641_mux", .data = &chips[pca9641_mux] },

Contributor

KanjiMonster Oct 13, 2023

Is there anything special about this PCA9641, or could this be replaced by importing e.g. https://patchwork.ozlabs.org/project/linux-i2c/patch/20190306231521.29367-6-peda@axentia.se/#2184718 into our kernel tree (with a bit of clean-up/forward porting)?

If the latter, I would heavily suggest doing so instead, and maybe even pick up the work of submitting the support again (not necessarily by you).

CosmicPenguin Mar 13, 2024

If it matters, the series that you linked seems to time out the i2c controller. I think it might have something to do with the way this mux is being used (there are two controllers attached but only one is actually powered at a time) - I didn't dive too deeply but it looks like the chip doesn't arbitrate correctly if there aren't two controllers on it. Directly programming the mux like this driver doesn't have the same effect. More investigation is probably needed to be sure but there seems to be reasonable technical reason not to use the other driver.

Contributor

paulmenzel Mar 18, 2024

Thank you for testing this. Could you please also relay this test result to the LKML?

brandonchuang force-pushed the as4560-52p branch from 2b6f91c to c90ed82 Compare

October 20, 2023 07:22

brandonchuang force-pushed the as4560-52p branch from c90ed82 to 8fb0d1d Compare

February 21, 2024 01:49

brandonchuang changed the title ~~[Edgecore][as4560-52p] Add new platform~~ [Edgecore][as4560-52p][as4561-52p] Add new platform

brandonchuang added a commit to brandonchuang/dentOS-linux that referenced this pull request


          [Edgecore] Add as4500-52p DTS

fee8f91

CPU: Marvell 98DX3530 with integrated CPU
MAC: Marvell 98DX3530
PHY: Marvell 88E1780 x 4 (1G port 16~32)
Marvell 88E2780 x 2 (Migi-G port 33-48)
DRAM: 8GB(MAC) DDR4 SDRAM
AirFlow: Front To Back
Function port: 1 x USB port
1 x RJ45 Mgmt port
1 x RJ45 Console port
Ethernet Port: 48 x 1G
Uplink port: 4xSFP+
PoE: Microsemi PD69208M x 12 + PD69210 x 2

The DTS is for the PR:
dentproject/dentOS#285

Signed-off-by: Brandon Chuang <brandon_chuang@edge-core.com>

brandonchuang added a commit to brandonchuang/dentOS-linux that referenced this pull request


          [Edgecore] Add as4581-52p DTS

1ab5319

CPU: COMe CPU Module
MAC: Marvell 98DX3530
PHY: Marvell 88E1780 x 4 (1G port 16~32)
Marvell 88E2780 x 2 (Migi-G port 33-48)
DRAM: 8GB(MAC) DDR4 SDRAM
AirFlow: Front To Back
Function port: 1 x USB port
1 x RJ45 Mgmt port
1 x RJ45 Console port
Ethernet Port: 48 x 1G
Uplink port: 4xSFP+
PoE: Microsemi PD69208M x 12 + PD69210 x 2

The DTS is for the PR:
dentproject/dentOS#285

Signed-off-by: Brandon Chuang <brandon_chuang@edge-core.com>

brandonchuang force-pushed the as4560-52p branch from 8fb0d1d to f14df8e Compare

March 20, 2024 03:35

brandonchuang mentioned this pull request

[Edgecore] Add as4581-52p DTS dentproject/linux#12

Open

brandonchuang changed the title ~~[Edgecore][as4560-52p][as4561-52p] Add new platform~~ [Edgecore][as4500-52p][as4581-52p] Add new platform

Contributor Author

brandonchuang commented Mar 20, 2024

Updated the PR due to the platform name changed:
as4560 -> as4500
as4561 -> as4581

brandonchuang added a commit to brandonchuang/dentOS-linux that referenced this pull request


          [Edgecore] Add as4581-52p DTS

f6ffb86

CPU: COMe CPU Module
MAC: Marvell 98DX3530
PHY: Marvell 88E1780 x 4 (1G port 16~32)
Marvell 88E2780 x 2 (Migi-G port 33-48)
DRAM: 8GB(MAC) DDR4 SDRAM
AirFlow: Front To Back
Function port: 1 x USB port
1 x RJ45 Mgmt port
1 x RJ45 Console port
Ethernet Port: 48 x 1G
Uplink port: 4xSFP+
PoE: Microsemi PD69208M x 12 + PD69210 x 2

The DTS is for the PR:
dentproject/dentOS#285

Signed-off-by: Brandon Chuang <brandon_chuang@edge-core.com>
Signed-off-by: Brandon Chuang <brandon_chuang@accton.com>


          [Edgecore][as4500-52p][as4581-52p] Add new platform

103ba8d

CPU:
[as4500-52p] Marvell 98DX3530 with integrated CPU
[as4581-52p] COMe CPU Module

MAC: Marvell 98DX3530
PHY: Marvell 88E1780 x 4 (1G port 16~32)
     Marvell 88E2780 x 2 (Migi-G port 33-48)
DRAM: 8GB(MAC) DDR4 SDRAM
AirFlow: Front To Back
Function port: 1 x USB port
	   1 x RJ45 Mgmt port
	   1 x RJ45 Console port
Ethernet Port: 48 x 1G
Uplink port: 4xSFP+
PoE: Microsemi PD69208M x 12 + PD69210 x 2

DTS:
The DTS for as4500-52p/as4581-52p
dentproject/linux#12
dentproject/linux#11

Signed-off-by: Brandon Chuang <brandon_chuang@accton.com>

brandonchuang force-pushed the as4560-52p branch from f14df8e to 103ba8d Compare

June 17, 2024 08:10


          [as4500-52p][as4581-52p] Add thermal policy

62ce694

as4581-52p(COMe + LTE SKU):
   U48(0x48)+U19(0x49)+U106(0x4B)+U135(0x4C) + ASC10(ASC_TMON1) < 197 = 10% FAN speed
   U48(0x48)+U19(0x49)+U106(0x4B)+U135(0x4C) + ASC10(ASC_TMON1) > 200 = 15% FAN speed
   U48(0x48)+U19(0x49)+U106(0x4B)+U135(0x4C) + ASC10(ASC_TMON1) < 232 = 15% FAN speed
   U48(0x48)+U19(0x49)+U106(0x4B)+U135(0x4C) + ASC10(ASC_TMON1) > 235 = 25% FAN speed
   U48(0x48)+U19(0x49)+U106(0x4B)+U135(0x4C) + ASC10(ASC_TMON1) < 262 = 25% FAN speed
   U48(0x48)+U19(0x49)+U106(0x4B)+U135(0x4C) + ASC10(ASC_TMON1) > 265 = 35% FAN speed
   COMe ASC10 location is U38 (ASC_TMON1)
   One FAN failure = 50% FAN speed
   Two FAN failure = 80% FAN speed

as4500-52p(Without COMe):
   U48(0x48)+U19(0x49)+U106(0x4B)+U135(0x4C) < 196.5 = 10% FAN speed
   U48(0x48)+U19(0x49)+U106(0x4B)+U135(0x4C) > 199 = 20% FAN speed
   U48(0x48)+U19(0x49)+U106(0x4B)+U135(0x4C) < 214 = 20% FAN speed
   U48(0x48)+U19(0x49)+U106(0x4B)+U135(0x4C) > 217 = 30% FAN speed
   U48(0x48)+U19(0x49)+U106(0x4B)+U135(0x4C) < 227.5 = 30% FAN speed
   U48(0x48)+U19(0x49)+U106(0x4B)+U135(0x4C) > 230 = 35% FAN speed
   One FAN failure = 35% FAN speed
   Two FAN failure = 50% FAN speed

   Note1: 0x48, 0x49, 0x4B, 0x4C are i2c address
   Note2: Critical components with shutdown rules when it is over temperature.
   1. COMe CPU: NXP LX2080SE71826B > 105C Tj, it will be shutdown
   2. PSU G1482-0920WNA > 55C , it will be shutdwon

Signed-off-by: Brandon Chuang <brandon_chuang@accton.com>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet