Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MKS CANable V2.0 #81366

Merged
merged 2 commits into from
Dec 4, 2024
Merged

Conversation

KozhinovAlexander
Copy link
Collaborator

add new board MKS CANable V2.0 Please refer to added documentation for more information.
The hardware of the board can be dound under: https://github.com/makerbase-mks/CANable-MKS

@KozhinovAlexander
Copy link
Collaborator Author

@henrikbrixandersen You will see an overlay for this board in CANnectivity later. ;) Thus please take a look too.

@KozhinovAlexander KozhinovAlexander force-pushed the mks_canable_v2 branch 2 times, most recently from ce12700 to b6f5079 Compare November 13, 2024 23:59
@KozhinovAlexander KozhinovAlexander requested review from erwango, decsny and galak and removed request for galak and decsny November 13, 2024 23:59
@KozhinovAlexander KozhinovAlexander changed the title Add new board MKS CANable V2.0 MKS CANable V2.0 Nov 14, 2024
Copy link
Collaborator

@nordicjm nordicjm left a comment

Choose a reason for hiding this comment

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

Board is OK, please fix ordering issue

@KozhinovAlexander
Copy link
Collaborator Author

Board is OK, please fix ordering issue

Thank you for you review. Great to see a feedback from hwv2 guy :) I'll do necessary changes today late night.

boards/makerbase/mks_canable_v20/doc/index.rst Outdated Show resolved Hide resolved
boards/makerbase/mks_canable_v20/doc/index.rst Outdated Show resolved Hide resolved
boards/makerbase/mks_canable_v20/doc/index.rst Outdated Show resolved Hide resolved
boards/makerbase/mks_canable_v20/doc/index.rst Outdated Show resolved Hide resolved
boards/makerbase/mks_canable_v20/doc/index.rst Outdated Show resolved Hide resolved
boards/makerbase/mks_canable_v20/mks_canable_v20.dts Outdated Show resolved Hide resolved
Copy link
Collaborator

@nordicjm nordicjm left a comment

Choose a reason for hiding this comment

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

Updates need squashing i.e. 2 commits

@KozhinovAlexander
Copy link
Collaborator Author

Updates need squashing i.e. 2 commits

Thank you. It is done now.

@nordicjm
Copy link
Collaborator

Updates need squashing i.e. 2 commits

Thank you. It is done now.

So the second vendor update commit needs squashing into the first vendor update commit and the second board commit needs squashing into the first board commit. Something like this:

git rebase -i 40cd35e56d43716bdbdeafafa6c356407121ca2e
  change first to edit, third to fixup, forth to drop
git cherry-pick ddf9bfdf02006c2dc4e2012ae5ad93a3a0fde584
git rebase --continue
git rebase -i 40cd35e56d43716bdbdeafafa6c356407121ca2e
  change second to fixup

Now you should have 2 commits

@KozhinovAlexander
Copy link
Collaborator Author

Updates need squashing i.e. 2 commits

Thank you. It is done now.

So the second vendor update commit needs squashing into the first vendor update commit and the second board commit needs squashing into the first board commit. Something like this:

git rebase -i 40cd35e56d43716bdbdeafafa6c356407121ca2e
  change first to edit, third to fixup, forth to drop
git cherry-pick ddf9bfdf02006c2dc4e2012ae5ad93a3a0fde584
git rebase --continue
git rebase -i 40cd35e56d43716bdbdeafafa6c356407121ca2e
  change second to fixup

Now you should have 2 commits

Got it. Will do.

nordicjm
nordicjm previously approved these changes Nov 18, 2024
Copy link
Member

@henrikbrixandersen henrikbrixandersen left a comment

Choose a reason for hiding this comment

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

Given that upstream threats this as one of multiple board variants (see https://github.com/makerbase-mks/CANable-MKS), I think we might want to do the same (bu renaming the board folder etc.). Support for the V1.0 and the PRO editions can then be added later on. What do you think?

boards/makerbase/index.rst Show resolved Hide resolved
boards/makerbase/mks_canable_v20/board.yml Outdated Show resolved Hide resolved
boards/makerbase/mks_canable_v20/doc/index.rst Outdated Show resolved Hide resolved
boards/makerbase/mks_canable_v20/doc/index.rst Outdated Show resolved Hide resolved
boards/makerbase/mks_canable_v20/mks_canable_v20.yaml Outdated Show resolved Hide resolved
boards/makerbase/mks_canable_v20/mks_canable_v20.yaml Outdated Show resolved Hide resolved
boards/makerbase/mks_canable_v20/mks_canable_v20_defconfig Outdated Show resolved Hide resolved
boards/makerbase/mks_canable_v20/support/openocd.cfg Outdated Show resolved Hide resolved
dts/bindings/vendor-prefixes.txt Show resolved Hide resolved
@KozhinovAlexander KozhinovAlexander force-pushed the mks_canable_v2 branch 2 times, most recently from 6813aed to 5b24ca1 Compare December 1, 2024 17:14
@nordicjm
Copy link
Collaborator

nordicjm commented Dec 2, 2024

Given that upstream threats this as one of multiple board variants (see https://github.com/makerbase-mks/CANable-MKS), I think we might want to do the same (bu renaming the board folder etc.). Support for the V1.0 and the PRO editions can then be added later on. What do you think?

There are a lot of differences between each version of the HW. To me their versioning seems more as an tagging of evolution approach and not of progress.

Yes, this. I very much agree with the point of view that we shouldn't try to "force" boards into hwmv2 variants/revisions just because naming scheme from a given vendor seems to call for using them. Sometimes ProductFoo and ProductFoo X just happen to be very different beasts (see ex how we have rpi_pico2 coming up as its own board).

How does it not fit? The schematics for v1.00 and v1.00 pro are the same except one has a TJA1051T and the other has a ADM3053 on page 2 of the schematic, the other 5 pages of the schematics are exactly the same. That's the definition of a board revision. The v2.00 seems to be completely different but that doesn't mean v1.00 and v1.00 pro can't use board revisions. And rpi_pico2 is a different board, no-where as anyone or the company themselves said it's just a revised PCB

@KozhinovAlexander
Copy link
Collaborator Author

Given that upstream threats this as one of multiple board variants (see https://github.com/makerbase-mks/CANable-MKS), I think we might want to do the same (bu renaming the board folder etc.). Support for the V1.0 and the PRO editions can then be added later on. What do you think?

There are a lot of differences between each version of the HW. To me their versioning seems more as an tagging of evolution approach and not of progress.

Yes, this. I very much agree with the point of view that we shouldn't try to "force" boards into hwmv2 variants/revisions just because naming scheme from a given vendor seems to call for using them. Sometimes ProductFoo and ProductFoo X just happen to be very different beasts (see ex how we have rpi_pico2 coming up as its own board).

How does it not fit? The schematics for v1.00 and v1.00 pro are the same except one has a TJA1051T and the other has a ADM3053 on page 2 of the schematic, the other 5 pages of the schematics are exactly the same. That's the definition of a board revision. The v2.00 seems to be completely different but that doesn't mean v1.00 and v1.00 pro can't use board revisions. And rpi_pico2 is a different board, no-where as anyone or the company themselves said it's just a revised PCB

Thank you for taking a look on it.
Agree, marker base MKS v1.00 and v1.00 pro may be pretty same. But here v2.00 will be integrated.
In general it definitely would be possible to add v2.00 that way, that with some DTS overlays v1.00 and v2.00 could be covered. But I see it also out of scope of this PR.
Still, I am completely open if someone will add v1.00 right way and do the whole necessary work. I will support, provide review, test.

@kartben
Copy link
Collaborator

kartben commented Dec 2, 2024

Given that upstream threats this as one of multiple board variants (see https://github.com/makerbase-mks/CANable-MKS), I think we might want to do the same (bu renaming the board folder etc.). Support for the V1.0 and the PRO editions can then be added later on. What do you think?

There are a lot of differences between each version of the HW. To me their versioning seems more as an tagging of evolution approach and not of progress.

Yes, this. I very much agree with the point of view that we shouldn't try to "force" boards into hwmv2 variants/revisions just because naming scheme from a given vendor seems to call for using them. Sometimes ProductFoo and ProductFoo X just happen to be very different beasts (see ex how we have rpi_pico2 coming up as its own board).

How does it not fit? The schematics for v1.00 and v1.00 pro are the same except one has a TJA1051T and the other has a ADM3053 on page 2 of the schematic, the other 5 pages of the schematics are exactly the same. That's the definition of a board revision. The v2.00 seems to be completely different but that doesn't mean v1.00 and v1.00 pro can't use board revisions. And rpi_pico2 is a different board, no-where as anyone or the company themselves said it's just a revised PCB

But this is the PR introducing V2, so I was commenting on that, Jamie. And you seem to agree it's completely different, and pretty the same story as Pico vs. Pico 2?

@nordicjm
Copy link
Collaborator

nordicjm commented Dec 2, 2024

Given that upstream threats this as one of multiple board variants (see https://github.com/makerbase-mks/CANable-MKS), I think we might want to do the same (bu renaming the board folder etc.). Support for the V1.0 and the PRO editions can then be added later on. What do you think?

There are a lot of differences between each version of the HW. To me their versioning seems more as an tagging of evolution approach and not of progress.

Yes, this. I very much agree with the point of view that we shouldn't try to "force" boards into hwmv2 variants/revisions just because naming scheme from a given vendor seems to call for using them. Sometimes ProductFoo and ProductFoo X just happen to be very different beasts (see ex how we have rpi_pico2 coming up as its own board).

How does it not fit? The schematics for v1.00 and v1.00 pro are the same except one has a TJA1051T and the other has a ADM3053 on page 2 of the schematic, the other 5 pages of the schematics are exactly the same. That's the definition of a board revision. The v2.00 seems to be completely different but that doesn't mean v1.00 and v1.00 pro can't use board revisions. And rpi_pico2 is a different board, no-where as anyone or the company themselves said it's just a revised PCB

But this is the PR introducing V2, so I was commenting on that, Jamie. And you seem to agree it's completely different, and pretty the same story as Pico vs. Pico 2?

For this board yes, the original comment refers to v1.00 and pro

nordicjm
nordicjm previously approved these changes Dec 2, 2024
boards/makerbase/mks_canable_v20/doc/index.rst Outdated Show resolved Hide resolved
Comment on lines +121 to +135
The argument ``-S rtt-console`` is needed for debug purposes with SEGGER RTT protocol.
This option is optional and may be omitted. Omitting it frees up RAM space but prevents RTT usage.

If option ``-S rtt-console`` is selected, the connection to the target can be established as follows:

.. code-block:: console

$ telnet localhost 9090

You should see the following message on the console:

.. code-block:: console

$ Hello World! mks_canable_v20/stm32g431xx

.. note::

Current OpenOCD config will skip Segger RTT for OpenOCD under 0.12.0.
Copy link
Member

Choose a reason for hiding this comment

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

boards/makerbase/mks_canable_v20/mks_canable_v20.dts Outdated Show resolved Hide resolved
boards/makerbase/mks_canable_v20/mks_canable_v20.dts Outdated Show resolved Hide resolved
boards/makerbase/mks_canable_v20/mks_canable_v20.dts Outdated Show resolved Hide resolved
boards/makerbase/mks_canable_v20/mks_canable_v20.dts Outdated Show resolved Hide resolved
boards/makerbase/mks_canable_v20/support/openocd.cfg Outdated Show resolved Hide resolved
@KozhinovAlexander
Copy link
Collaborator Author

KozhinovAlexander commented Dec 2, 2024

@henrikbrixandersen Thank you for your input. I've done the chnages requested. The OpenOCD config will be moved to separate PR in few minutes.

UPDATE:
Regarding Seger RTT console sample from this link: https://docs.zephyrproject.org/latest/snippets/rtt-console/README.html I can not see step-by-step guide there like my documentation for this board do. Thus I prefer to leave the section about RTT console connection over telnet.
Moreover the link does not give any useful information except KConfig entry description :)

Add Makerbase Co., Ltd. board vendor

Signed-off-by: Alexander Kozhinov <[email protected]>
@KozhinovAlexander KozhinovAlexander force-pushed the mks_canable_v2 branch 3 times, most recently from 00335b6 to ec5cd79 Compare December 2, 2024 20:21
A cheap and affordable stm32g4 based simple CAN and CAN-FD to usb
adapter board

Signed-off-by: Alexander Kozhinov <[email protected]>
@kartben
Copy link
Collaborator

kartben commented Dec 4, 2024

@erwango please revisit

@kartben kartben merged commit e642b3b into zephyrproject-rtos:main Dec 4, 2024
25 checks passed
@KozhinovAlexander KozhinovAlexander deleted the mks_canable_v2 branch December 4, 2024 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants