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

Force ADMV8818 soft reset and SDO initialization #2700

Merged
merged 1 commit into from
Feb 10, 2025

Conversation

swinchen
Copy link
Contributor

@swinchen swinchen commented Jan 24, 2025

When a weak pull-up is present on the SDO line, regmap_update_bits fails to write both the SOFTRESET and SDOACTIVE bits because it incorrectly reads them as already set.

Since the soft reset disables the SDO line, performing a read-modify-write operation on ADI_SPI_CONFIG_A to enable the SDO line doesn't make sense. This change directly writes to the register instead of using regmap_update_bits.

PR Type

  • Bug fix (a change that fixes an issue)
  • New feature (a change that adds new functionality)
  • Breaking change (a change that affects other repos or cause CIs to fail)

PR Checklist

  • I have conducted a self-review of my own code changes
  • I have tested the changes on the relevant hardware
  • I have updated the documentation outside this repo accordingly (if there is the case)

Copy link
Collaborator

@nunojsa nunojsa left a comment

Choose a reason for hiding this comment

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

Hi @swinchen, thanks for contributing... Change looks good to me. The only thing I want to mention is the commit message. Please run git log --oneline drivers/iio and follow the same style for the commit subject.

Another thing I'd like to mention is that I would like to see this patch first sent upstream. Are you comfortable with that?

@swinchen swinchen force-pushed the force_configure_admv8818 branch from aee2d29 to f851ce8 Compare January 24, 2025 17:06
@swinchen
Copy link
Contributor Author

Hi @swinchen, thanks for contributing... Change looks good to me. The only thing I want to mention is the commit message. Please run git log --oneline drivers/iio and follow the same style for the commit subject.

Another thing I'd like to mention is that I would like to see this patch first sent upstream. Are you comfortable with that?

Hi @nunojsa - I have corrected the commit one line commit message. Sorry I missed that in my first past.

By upstream, do you mean Torvalds? I checked the maintainers list and the other option might be this list: linux-iio@vger.kernel.org

If you point me to the right place I am happy to push it upstream unless that is something you handle.

@nunojsa
Copy link
Collaborator

nunojsa commented Jan 26, 2025

Hi,

Sorry I missed it. Anyways, I can see you already figured it out and the patch is already applied. Please cherry-pick the applied patch and update this PR. It should be available here after merge window is over:

https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/

@swinchen
Copy link
Contributor Author

Hi,

Sorry I missed it. Anyways, I can see you already figured it out and the patch is already applied. Please cherry-pick the applied patch and update this PR. It should be available here after merge window is over:

https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/

No worries - I hope I did it correctly. Would you like me close this branch and re-open a new PR with the cherry-picked commit?

Where can you see that the patch was already applied? I do not see any activity of the v2 patch.

Thanks for the assistance.

@nunojsa
Copy link
Collaborator

nunojsa commented Jan 27, 2025

No worries - I hope I did it correctly. Would you like me close this branch and re-open a new PR with the cherry-picked commit?

As you prefer... You can just update this PR with the cherry-picked patch and force push it...

Where can you see that the patch was already applied? I do not see any activity of the v2 patch.

It should be available in one of the branches (likely the fixes one) in here:

https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/

But should be only available next week after merge window is over.

@nunojsa
Copy link
Collaborator

nunojsa commented Feb 10, 2025

@swinchen your patch is already pushed in IIO tree:

https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/commit/?h=fixes-togreg&id=cc2c3540d9477a9931fb0fd851fcaeba524a5b35

Could you cherry-pick and update this? Otherwise I can also do it...

When a weak pull-up is present on the SDO line, regmap_update_bits fails
to write both the SOFTRESET and SDOACTIVE bits because it incorrectly
reads them as already set.

Since the soft reset disables the SDO line, performing a
read-modify-write operation on ADI_SPI_CONFIG_A to enable the SDO line
doesn't make sense. This change directly writes to the register instead
of using regmap_update_bits.

Fixes: f34fe88 ("iio:filter:admv8818: add support for ADMV8818")
Signed-off-by: Sam Winchenbach <swinchenbach@arka.org>
Link: https://patch.msgid.link/SA1P110MB106904C961B0F3FAFFED74C0BCF5A@SA1P110MB1069.NAMP110.PROD.OUTLOOK.COM
Cc: <Stable@vger.kernel.org>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
@swinchen swinchen force-pushed the force_configure_admv8818 branch from f851ce8 to 6b24ab9 Compare February 10, 2025 13:46
@swinchen
Copy link
Contributor Author

@swinchen your patch is already pushed in IIO tree:

https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/commit/?h=fixes-togreg&id=cc2c3540d9477a9931fb0fd851fcaeba524a5b35

Could you cherry-pick and update this? Otherwise I can also do it...

Cherry-picked and pushed. Sorry this took so long.

Copy link
Collaborator

@nunojsa nunojsa left a comment

Choose a reason for hiding this comment

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

Thx!

@nunojsa nunojsa merged commit 477b811 into analogdevicesinc:main Feb 10, 2025
13 checks passed
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.

2 participants