Skip to content

Supermicro_arm64_1G_Switch_G3748_Added: Patch of dts file to support …#373

Closed
SelvaSupermicro wants to merge 0 commit intosonic-net:masterfrom
SelvaSupermicro:Supermicro_arm64_1G_Switch_G3748_Added
Closed

Supermicro_arm64_1G_Switch_G3748_Added: Patch of dts file to support …#373
SelvaSupermicro wants to merge 0 commit intosonic-net:masterfrom
SelvaSupermicro:Supermicro_arm64_1G_Switch_G3748_Added

Conversation

@SelvaSupermicro
Copy link
Copy Markdown

This patch is to support new platform, Supermicro 1G arm64 switch G3748 which uses marvel AC5X ASIC.
This patch is applied for all arm64 builds and will be effective only for this new platform.

Testing:

  1. Tested running M0/PTF32 test suites for this new platform.
  2. Verified image partitions
  3. Verified uboot and sonic platform functionalities

@saiarcot895
Copy link
Copy Markdown
Contributor

Has/will this been upstreamed?

@SelvaSupermicro
Copy link
Copy Markdown
Author

SelvaSupermicro commented Jan 12, 2024 via email

Copy link
Copy Markdown
Contributor

@paulmenzel paulmenzel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution.

PS: If you reply via email, please remove the cited part, otherwise the comment in the GitHub Web interface is “ugly” to read.

Comment thread patch/0012-arm64-dts-marvell-Add-Supermicro-SSE-G3748-board.patch Outdated
@saiarcot895
Copy link
Copy Markdown
Contributor

Yes, once the PR is reviewed and approved it should automatically get merged to master (upstream)
I am sure you know that process.So I really don’t completely understand your question.
Please help me understand I would be glad to respond.

Hi @SelvaSupermicro,

By upstream, I mean submitting this change into the main Linux kernel. See their contribution page for more details.

@SelvaSupermicro
Copy link
Copy Markdown
Author

Yes, once the PR is reviewed and approved it should automatically get merged to master (upstream)
I am sure you know that process.So I really don’t completely understand your question.
Please help me understand I would be glad to respond.

Hi @SelvaSupermicro,

By upstream, I mean submitting this change into the main Linux kernel. See their contribution page for more details.

Thanks for the clarifications. These code changes are applicable only for SONiC. Hence we dont want to upstream to main Linux kernel.

@SelvaSupermicro SelvaSupermicro force-pushed the Supermicro_arm64_1G_Switch_G3748_Added branch from b0d18fa to ec8e89d Compare January 18, 2024 00:51
@SelvaSupermicro SelvaSupermicro force-pushed the Supermicro_arm64_1G_Switch_G3748_Added branch from 3e52dec to 8f4ff5a Compare February 12, 2024 02:38
@paulmenzel
Copy link
Copy Markdown
Contributor

From your commit message:

hence will not be upstreamed to main ONL repo

SONiC and ONL are two separate projects for a network operating system. No idea, why you reference ONL.

To ease maintenance of the SONiC Linux kernel repository, you have to sent these patches upstream. Often upstream is also giving you a high quality review, that the SONiC developers/maintainers are not capable of.

Also the devicetree has a lot of commented out lines, that also need to be cleaned up.

@SelvaSupermicro
Copy link
Copy Markdown
Author

From your commit message:

hence will not be upstreamed to main ONL repo

SONiC and ONL are two separate projects for a network operating system. No idea, why you reference ONL.

To ease maintenance of the SONiC Linux kernel repository, you have to sent these patches upstream. Often upstream is also giving you a high quality review, that the SONiC developers/maintainers are not capable of.

Also the devicetree has a lot of commented out lines, that also need to be cleaned up.

From your commit message:

hence will not be upstreamed to main ONL repo

SONiC and ONL are two separate projects for a network operating system. No idea, why you reference ONL.

To ease maintenance of the SONiC Linux kernel repository, you have to sent these patches upstream. Often upstream is also giving you a high quality review, that the SONiC developers/maintainers are not capable of.

Also the devicetree has a lot of commented out lines, that also need to be cleaned up.

Sorry for the ONL reference. I meant to say Linux. Anyway, I have cleaned up the patch to include file that was already committed instead creating new..
Why I don't want to upstream to Linux repo,

  1. As you can see this patch file depends on many other patches. The other patch files all are in SONiC repo and not in main Linux repo. So they cant apply my patch as it is invalid in that repo.
  2. Our patch is applicable only for our switch platform running SONiC. So it applicable only for SONiC. It has no value in main Linux.

@prgeor
Copy link
Copy Markdown
Contributor

prgeor commented Mar 22, 2024

@SelvaSupermicro please resolve the conflict

Comment thread patch/0012-arm64-dts-marvell-Add-Supermicro-SSE-G3748-board.patch Outdated

This change adds device tree files for Supermicro SSE-G3748 board.
This dts file is derived from Marvell RD-AC5X board.
- SPI flash partition is changed.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is it changed?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use different SPI flash partitions from what is used in Marvell RD-AC5X dts file.
Supermicro SSE-G3748 smc_sse-g3748.dts uses:
partition@0 {
label = "spi_flash_part0";
reg = <0x0 0x200000>;
};

            parition@1 {
                    label = "spi_flash_part1";
                    reg = <0x200000 0x10000>;
            };

            parition@2 {
                    label = "spi_flash_part2";
                    reg = <0x210000 0xC00000>;
            };

            parition@3 {
                    label = "spi_flash_part3";
                    reg = <0xE10000 0x1F0000>;
            };

Marvell RD-AC5X ac5-98dx35xx-rd.dts uses:
partition@0 {
label = "spi_flash_part0";
reg = <0x0 0x800000>;
};

            parition@1 {
                    label = "spi_flash_part1";
                    reg = <0x800000 0x700000>;
            };

            parition@2 {
                    label = "spi_flash_part2";
                    reg = <0xF00000 0x100000>;
            };

Comment thread patch/0012-arm64-dts-marvell-Add-Supermicro-SSE-G3748-board.patch Outdated
@saiarcot895
Copy link
Copy Markdown
Contributor

Sorry for the delay here, could you rebase your changes on top of the master branch, or merge in the current master branch into your branch? This is to resolve the patch conflict in patch/series.

@SelvaSupermicro
Copy link
Copy Markdown
Author

Sorry for the delay here, could you rebase your changes on top of the master branch, or merge in the current master branch into your branch? This is to resolve the patch conflict in patch/series.

Hi Saikrishna,
Thanks for the comment. Now, I have updated patch so that it can be merged with the latest master.
Let me know if that is all good!

@paulmenzel
Copy link
Copy Markdown
Contributor

Please improve the merge/pull request summary/title.

@ErichYen
Copy link
Copy Markdown
Contributor

ErichYen commented May 7, 2024

Hello SaiKrishna,
We may need your approve on @SelvaSupermicro new update here?
4aadc91

@saiarcot895
Copy link
Copy Markdown
Contributor

Looks like there's one conflict somehow?

@saiarcot895
Copy link
Copy Markdown
Contributor

Please improve the merge/pull request summary/title.

Echoing this, particularly the title.

Comment thread patch/series
0011-arm64-dts-marvell-Add-Nokia-7215-IXS-A1-board.patch
0012-net-mvpp2-clear-BM-pool-before-initialization.patch
0013-mmc-sdhci-xenon-fix-PHY-init-clock-stability.patch
0014-mmc-sdhci-xenon-add-timeout-for-PHY-init-complete.patch
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's too many changes here given that only one patch file being added in this PR. Please recheck the merge conflict resolution

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants