Skip to content

Add support for Winbond W25Q64FV #3855

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

Closed
wants to merge 4 commits into from
Closed

Add support for Winbond W25Q64FV #3855

wants to merge 4 commits into from

Conversation

djix123
Copy link

@djix123 djix123 commented Dec 21, 2020

No description provided.

@skerr92
Copy link

skerr92 commented Dec 21, 2020

Related to issue #3782 and PR #3824

Everything seems in order, probably should add a description to the PR. @tannewt aggregate this with my PR?

Copy link
Collaborator

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

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

  1. Question re SDA/SCL pins.

  2. I'm not sure; did you actually try the W25Q64FV on a board?

Comment on lines 38 to 40

{ MP_ROM_QSTR(MP_QSTR_SDA), MP_ROM_PTR(&pin_PB07) },
{ MP_ROM_QSTR(MP_QSTR_SCL), MP_ROM_PTR(&pin_PB06) },
Copy link
Collaborator

Choose a reason for hiding this comment

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

Did you mean to add these pins in this PR? There is no board.I2C() defined?

Copy link
Author

Choose a reason for hiding this comment

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

  1. The SDA/SCL pins commit looks like it got added to the pull request after I'd created it when I added those to my fork - wasn't the intention that they be part of this pull
  2. Yes, tested W25Q64FV working with build of 6.1.0-beta.2 and head.

Copy link
Collaborator

Choose a reason for hiding this comment

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

could you push a new commit that removes the SDA and SCL pins?

Copy link
Author

Choose a reason for hiding this comment

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

Ok - will try that

@djix123
Copy link
Author

djix123 commented Dec 22, 2020

Resubmitting with only W25Q64FV commit

@djix123 djix123 closed this Dec 22, 2020
@dhalbert dhalbert reopened this Dec 22, 2020
@dhalbert dhalbert closed this Dec 22, 2020
@dhalbert
Copy link
Collaborator

Clean pull request is fine, but not required. I'll use the other one, thanks!

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.

3 participants