Add Cameo board based on Innovium chip #3639

Hanly-Cameo · 2019-10-21T10:49:05Z

Cameo New Project:
Platform: x86_64-cameo_esc601_32q-r0 (32x100G)
HwSKU: esc601-32q
ASIC: Innovium

- What I did
Add platform support for esc601-32q project
- How I did it
Add configuration and platform drivers for esc601-32q project
Update configuration and platform drivers for esc601-32q project
- How to verify it
sfputil show presence
show interfaces transceiver eeprom
show platform psustatus
show platform syseeprom
sfputil show lpmode
show interfaces status
show environment
show version
- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

lguohan · 2019-10-24T21:51:03Z

platform/innovium/sonic-platform-modules-cameo/esc601-32q/modules/x86-64-cameo-esc601-32q.c

+{ .name="qsfp_eeprom_32", .portid=32, .i2c_switch1_val=0x80, .i2c_switch2_val=0x80 },
+};
+
+static ssize_t sfp_bin_read(struct file *filp, struct kobject *kobj,


should use optoe driver.

lguohan · 2019-10-24T21:54:06Z

platform/innovium/sonic-platform-modules-cameo/esc601-32q/modules/x86-64-cameo-esc601-32q.c

+    u8 status = -EPERM;
+    u8 res = 0x1;
+    int i;
+    struct sensor_device_attribute *attr = to_sensor_dev_attr(da


remove commented code.

remove commented code.

You mean remove all of the comment ?

lguohan · 2019-10-24T21:56:22Z

platform/innovium/sonic-platform-modules-cameo/esc601-32q/modules/x86-64-cameo-esc601-32q.h

+static ssize_t low_power_set(struct device *dev, struct device_attribute *da, const char *buf, size_t count);
+static ssize_t qsfp_reset_set(struct device *dev, struct device_attribute *da, const char *buf, size_t count);
+static ssize_t qsfp_status_get(struct device *dev, struct device_attribute *da, char *buf);
+static ssize_t qsfp_temp_get(struct device *dev, struct device_attribute *da, char *buf);


use optoe driver.

use optoe driver.

How to use optoe driver? Can you give an example? I have read about optoe driver source code,
but seem not fit our hardware arch.

as comments

Already updated to use optoe driver for accessing qsfp module.

lguohan

as comments

Hanly-Cameo · 2019-11-06T00:37:05Z

as comments
@lguohan
Hi lguohan,
we already update driver to use 'optoe' ,and test with the following commands.
sfputil show presence
show interfaces transceiver eeprom
sensors
show platform psustatus

Please verify.
thanks.

lguohan · 2019-11-08T00:19:33Z

platform/innovium/sonic-platform-modules-cameo/esc601-32q/modules/mcp3422.c

@@ -0,0 +1,429 @@
+/*
+ * mcp3422.c - driver for the Microchip mcp3421/2/3/4/5/6/7/8 chip family


can you use linux kernel driver?

if so , I need to modify kenerl default config. "# CONFIG_MCP3422 is not set" is default value in current kernel config.

@lguohan any advice?

yes, can you submit pr to enable the kernel driver?

Hanly-Cameo · 2019-11-21T05:11:42Z

Hi lguohan,
I got a information from our hardware team: the mcp3425 is a standard I2C device, but our I2C controller driver is intel "I801" .It is SMbus only.
So, we changed the mcp3422.c to mcp3425_smbus.c. It is only for our platform.
What I did

we remove I2C function check.
using 'i2c_smbus_read_i2c_block_data" instead of "i2c_master_recv"
How to verify it:
"cat in_voltage0_raw" and check the value.

maxliu37 · 2020-03-13T05:17:51Z

Hi lguohan,

We have modified the code as your request sometime back and not been reviewed/confirmed by you for several months. WIll you advise where is the problem blocking the pull request? Thanks.

lguohan · 2020-03-23T23:39:17Z

I do not understand why you need to rename the mcp3422 driver to a new name. why cannot reuse the driver from linux?

lguohan · 2020-03-23T23:40:02Z

platform/innovium/sonic-platform-modules-cameo/esc601-32q/modules/at24_smbus.c

@@ -0,0 +1,847 @@
+/*
+ * at24.c - handle most I2C EEPROMs


linux kernel has at24 driver. why not use the original one?

Hi Iguohan,
we are using at24c16 on our board, 24c16 is 16bit address mode. In current driver at24.c.
it will check I2c Function bit with 16bit address mode. our cpu's I2c bus is SMBUS, but 24c16 need I2C. so we changed it.
if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
//if (chip.flags & AT24_FLAG_ADDR16)
// return -EPFNOSUPPORT;

for mcp3422 chip , it has the same i2c issue.(need I2C_FUNC_I2C).

Our CPU is using SMBUS not pure I2C, therefore we have to marked out the functionality check. Otherwise it shows us init checking error. No other company with the same issue? I mean that some minor change required with native driver.

lguohan · 2020-03-23T23:42:18Z

platform/innovium/sonic-platform-modules-cameo/esc601-32q/modules/tps53681.c

+
+module_i2c_driver(tps53679_driver);
+
+MODULE_AUTHOR("Vadim Pasternak <[email protected]>");


this looks like mellanox driver. is it already in linux kernel?

As we know, the tps53679 driver is already in Linux kernel and we change to use it directly instead of another driver for tps53681.

lguohan · 2020-03-23T23:46:06Z

platform/innovium/sonic-platform-modules-cameo/esc601-32q/modules/x86-64-cameo-esc601-32q.h

+void show_eeprom(u_int8_t *eeprom, char *buf,int tlv_len);
+#endif
+
+#ifdef QSFP_WANTED


what is this QSFP_WANTED. I think you need to remove them completely since you are using the optoe driver now.

We are still using the driver(defined QSFP_WANTED) in ONL system for easy debugging and share the source code with that in SONiC system. That's why we keep the driver option here.
Is there any concern or possible impact with the compiler option?

lgtm-com · 2020-05-13T02:58:45Z

This pull request introduces 12 alerts when merging 7b00d0a403329b95be52c44c6d098df84d67204b into f52e59a - view on LGTM.com

new alerts:

11 for Unused import
1 for Unused local variable

lgtm-com · 2020-06-30T12:00:39Z

This pull request introduces 894 alerts when merging d059f250951da5502fa3437ca4d498d571600c8c into d04ad41 - view on LGTM.com

new alerts:

308 for Unused local variable
215 for Testing equality to None
94 for Unreachable code
67 for Use of 'global' at module level
48 for Variable defined multiple times
47 for Unnecessary 'else' clause in loop
27 for Except block handles 'BaseException'
19 for Unused import
12 for Suspicious unused loop iteration variable
8 for Redundant comparison
8 for Unhashable object hashed
8 for Nested loops with same variable
7 for Duplicate key in dict literal
7 for Redundant assignment
6 for Unnecessary pass
4 for Wrong number of arguments in a call
3 for Module is imported more than once
2 for Use of the return value of a procedure
2 for Duplication in regular expression character class
2 for Modification of parameter with default

lguohan · 2020-07-21T10:51:12Z

there seems to be too many lgtm alerts, can you fix all of the lgtm alerts?

maxliu37 · 2020-07-22T01:19:05Z

there seems to be too many lgtm alerts, can you fix all of the lgtm alerts?

It may take time to fix the lgtm alerts, therefore we take away the platform 128Q request from #3639 and left it dedicated for x86_64-cameo_esc601_32q-r0 platform. Will issue another request for 128Q later.
For the x86_64-cameo_esc601_32q-r0 platform, we think it is almost to the end. Please help speeding up the review for us. Thanks.

lguohan · 2020-07-22T01:27:41Z

I just notice that this pr is raised against 201811 branch. We do not do such pr, please always raise against master branch first, we first merge the platform there and then do backport to release branch if neccessary.

maxliu37 · 2020-07-22T01:44:14Z

I just notice that this pr is raised against 201811 branch. We do not do such pr, please always raise against master branch first, we first merge the platform there and then do backport to release branch if neccessary.

Do you mean to request for merging into master branch first? I thought the master is the up-to-date branch, which may be in between 201911 and 202006 for now. But we need to support 201811 which our customers are working on.

…lly (#22790) #### Why I did it src/sonic-swss ``` * 5e07127a - (HEAD -> master, origin/master, origin/HEAD) [dashhaorch]: Fix error: stack protector not protecting local variables: variable length buffer (#3643) (4 hours ago) [Nazarii Hnydyn] * d589d8d9 - [swss]: IcmpOrch to support ICMP session offload to ASIC (#3535) (6 hours ago) [manamand2020] * f05e8e9e - [SRv6] add MySID counters support (#3601) (6 hours ago) [Yakiv Huryk] * a0bd39e5 - Skip "port doesn't exist" SWSS_LOG_INFO messages for local ports (#3553) (31 hours ago) [HP] * 74b2cc61 - [ci]: Skip publishing of asan vstest summary (#3669) (32 hours ago) [prabhataravind] * 398161b4 - [Dynamic Buffer][Mellanox] Fix an issue when handling 2-digit queue ID in the Lua plugin (#3588) (2 days ago) [Stephen Sun] * 7106cc0a - Fixing macsecmgrd memory corruption (#3611) (2 days ago) [sivanuka-arista] * e830a491 - [fpmsyncd]Fixing blackhole route to publish protocol field to APPL_DB (#3655) (2 days ago) [Sudharsan Dhamal Gopalarathnam] * de5b8e51 - Setting default nexthop weight to 1 in `fpmsyncd` (#3636) (3 days ago) [mramezani95] * 176bcea9 - Change Log Level for BFD Offload Capability Implementation (#3641) (3 days ago) [Sai Rama Mohan Reddy S] * f9f7ff0e - Fix NextHopGroupEntry class data member not initialized bug (#3644) (3 days ago) [Hua Liu] * c8c597cf - Install symlink to Python 3 to work around AzP diff coverage issue (#3670) (6 days ago) [Saikrishna Arcot] * 3a5efa38 - [tests]: Fix `test_MirrorDestMoveLag` test failure (#3639) (6 days ago) [Carmine Scarpitta] * 13d559d4 - Revert "Set Port UPDATE_DSCP attribute when TC_TO_DSCP map is attached (#3517)" (#3666) (6 days ago) [Kumaresh Perumal] * 1c601cb8 - Changes to unblock swss pipeline tests (#3664) (7 days ago) [prabhataravind] * b31500b2 - [build] Support optionally using other container registries instead of DockerHub (#3668) (7 days ago) [Saikrishna Arcot] ``` #### How I did it #### How to verify it #### Description for the changelog

lguohan reviewed Oct 24, 2019

View reviewed changes

lguohan requested changes Oct 24, 2019

View reviewed changes

lguohan reviewed Nov 8, 2019

View reviewed changes

lguohan reviewed Mar 23, 2020

View reviewed changes

Hanly-Cameo closed this Jul 24, 2020

ghost deleted a comment from msftclas Jul 24, 2020

		@@ -0,0 +1,429 @@
		/*
		* mcp3422.c - driver for the Microchip mcp3421/2/3/4/5/6/7/8 chip family


		module_i2c_driver(tps53679_driver);

		MODULE_AUTHOR("Vadim Pasternak <[email protected]>");

Add Cameo board based on Innovium chip #3639

Add Cameo board based on Innovium chip #3639

Uh oh!

Conversation

Hanly-Cameo commented Oct 21, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lguohan left a comment

Choose a reason for hiding this comment

Uh oh!

Hanly-Cameo commented Nov 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hanly-Cameo commented Nov 21, 2019

Uh oh!

maxliu37 commented Mar 13, 2020

Uh oh!

lguohan commented Mar 23, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hanly-Cameo Mar 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lgtm-com bot commented May 13, 2020

Uh oh!

lgtm-com bot commented Jun 30, 2020

Uh oh!

lguohan commented Jul 21, 2020

Uh oh!

maxliu37 commented Jul 22, 2020

Uh oh!

lguohan commented Jul 22, 2020

Uh oh!

maxliu37 commented Jul 22, 2020

Uh oh!

Uh oh!

Hanly-Cameo commented Nov 6, 2019 •

edited

Loading

Hanly-Cameo Mar 24, 2020 •

edited

Loading