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

Add support for ltc7871 #2402

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

AAbacan2
Copy link

@AAbacan2 AAbacan2 commented Dec 18, 2024

Pull Request Description

Please replace this with a detailed description and motivation of the changes.
You can tick the checkboxes below with an 'x' between square brackets or just check them after publishing the PR.
If this PR contains a breaking change, list dependent PRs and try to push all related PRs at the same time.

PR Type

  • Bug fix (change that fixes an issue)
  • New feature (change that adds new functionality)
  • Breaking change (has dependencies in other repos or will cause CI to fail)

PR Checklist

  • I have followed the Coding style guidelines
  • I have performed a self-review of the changes
  • I have commented my code, at least hard-to-understand parts
  • I have build all projects affected by the changes in this PR
  • I have tested in hardware affected projects, at the relevant boards
  • I have signed off all commits from this PR
  • I have updated the documentation (wiki pages, ReadMe etc), if applies

@CLAassistant
Copy link

CLAassistant commented Dec 18, 2024

CLA assistant check
All committers have signed the CLA.

@kister-jimenez
Copy link
Collaborator

/AzurePipelines run

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@AAbacan2 AAbacan2 closed this Dec 19, 2024
@AAbacan2 AAbacan2 force-pushed the add_support_for_LTC7871 branch from 35762fe to a75c351 Compare December 19, 2024 06:39
@AAbacan2 AAbacan2 reopened this Dec 19, 2024
@kister-jimenez
Copy link
Collaborator

/AzurePipelines run

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@AAbacan2 AAbacan2 force-pushed the add_support_for_LTC7871 branch from c5f7e3a to 1849638 Compare December 20, 2024 03:24
@kister-jimenez
Copy link
Collaborator

/AzurePipelines run

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@AAbacan2 AAbacan2 closed this Jan 10, 2025
@AAbacan2 AAbacan2 force-pushed the add_support_for_LTC7871 branch from 021ed00 to d01e5b9 Compare January 10, 2025 07:09
@AAbacan2 AAbacan2 reopened this Jan 13, 2025
@kister-jimenez
Copy link
Collaborator

/AzurePipelines run

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@AAbacan2 AAbacan2 force-pushed the add_support_for_LTC7871 branch from c8a969a to 6eb93c2 Compare January 17, 2025 08:05
@AAbacan2 AAbacan2 marked this pull request as ready for review January 17, 2025 08:54
@kister-jimenez
Copy link
Collaborator

/AzurePipelines run

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

Copy link
Contributor

@buha buha left a comment

Choose a reason for hiding this comment

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

It looks good but you'll have to update the project to the new project infrastructure.

Here's the PR with the changes with 3 modified projects as example: #2412

And here's a pending PR where I gave feedback with pretty much that needs to be changed, same applies to your PR.
#2411

Add initial header and source files for LTC7871 driver.

Signed-off-by: Aldrin Abacan <[email protected]>
@AAbacan2 AAbacan2 force-pushed the add_support_for_LTC7871 branch from c799fa9 to c8f58c7 Compare January 24, 2025 03:08
Add initial header and source file for LTC7871 IIO driver.

Signed-off-by: Aldrin Abacan <[email protected]>
@kister-jimenez
Copy link
Collaborator

/AzurePipelines run

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

Add README.rst documentation file for LTC7871 alongside other
documentation related files.

Signed-off-by: Aldrin Abacan <[email protected]>
Add initial project files for both basic and IIO examples for LTC7871.

Signed-off-by: Aldrin Abacan <[email protected]>
Add README.rst documentation file for project alongside other
documentation related files.

Signed-off-by: Aldrin Abacan <[email protected]>
@kister-jimenez
Copy link
Collaborator

/AzurePipelines run

Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@AAbacan2 AAbacan2 requested a review from buha January 25, 2025 00:22
if (!desc)
return -ENODEV;

no_os_free(desc->iio_dev->channels);
Copy link
Contributor

Choose a reason for hiding this comment

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

This array is statically allocated

pr_info("IDAC_Vlow: %duA\n", idac);
} else {
pr_info("Device on Boost mode\n");
//TODO:
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this

#include "maxim_uart.h"
#include "maxim_uart_stdio.h"

#ifdef IIO_SUPPORT
Copy link
Contributor

Choose a reason for hiding this comment

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

i think the ifdef is not needed, you may delete it

.. code-block:: bash

# Select the example you want to enable by choosing y for enabling and n for disabling
BASIC_EXAMPLE = y
Copy link
Contributor

Choose a reason for hiding this comment

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

please update this, this is not true anymore

.. code-block:: bash

# Select the example you want to enable by choosing y for enabling and n for disabling
BASIC_EXAMPLE = n
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

# to delete current build
make PLATFORM=maxim TARGET=max32690 reset
# to build the project and flash the code
make PLATFORM=maxim TARGET=max32690 run
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe split this in two, dont build and run on the same line, it works but may confuse readers

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