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 imx3112 mux driver #59290

Closed

Conversation

ramiroderojas
Copy link

@ramiroderojas ramiroderojas commented Jun 15, 2023

Change adds driver support for I3C 2:1 mux splitter Renesas IMX3112: https://www.renesas.com/us/en/products/memory-logic/memory-interface-products/memory-multiplexers/imx3112-i3c-basic-12-bus-multiplexer

The first commit adds support for buses with I3C devices ONLY.
Second commit adds support for mixed and I2C-only buses.

Signed-off-by: Ramiro de Rojas Perez [email protected]

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Hello @ramiroderojas, and thank you very much for your first pull request to the Zephyr project!

A project maintainer just triggered our CI pipeline to run it against your PR and ensure it's compliant and doesn't cause any issues. You might want to take this opportunity to review the project's Contributor Expectations and make any updates to your pull request if necessary. 😊

@ramiroderojas
Copy link
Author

@mfischer Can you take a look :)

@ramiroderojas ramiroderojas changed the title Add imx3112 mux Add imx3112 mux driver Jun 15, 2023
Copy link
Member

@XenuIsWatching XenuIsWatching left a comment

Choose a reason for hiding this comment

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

You may want also do what #59163 is doing where it makes sure the 2 priority configs are set right.

drivers/i3c/i3c_renesas_imx3112.c Outdated Show resolved Hide resolved
@mfischer
Copy link
Contributor

mfischer commented Feb 7, 2024

Can we get some eyes on this, this is still relevant @cfriedt :)

@cfriedt
Copy link
Member

cfriedt commented Feb 7, 2024

Can we get some eyes on this, this is still relevant @cfriedt :)

I can review, but we are in feature freeze until after the release is cut.

@ramiroderojas
Copy link
Author

ramiroderojas commented Feb 7, 2024

No problem, this can go on next next release, thanks @cfriedt :)

Copy link
Member

@cfriedt cfriedt left a comment

Choose a reason for hiding this comment

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

Hmm.. I am by no means an i3c expert. Will defer to @XenuIsWatching

@cfriedt cfriedt reopened this Feb 7, 2024
This changes adds I2C support to the IMX3112 mux. That means we
will need to statically choose I2C configuration during driver
macrobatics, and then select the I2C write/read methods

Signed-off-by: Ramiro de Rojas Perez <[email protected]>
@ramiroderojas
Copy link
Author

I believe I did the changes @XenuIsWatching requested, I was just waiting on him to take another look

@github-actions github-actions bot removed the Stale label Feb 8, 2024
Comment on lines +350 to +359
static int i3c_renesas_imx3112_configure(const struct device *dev, enum i3c_config_type type,
void *config)
{
return -ENOSYS;
}

static int i3c_renesas_imx3112_i2c_configure(const struct device *dev, uint32_t dev_config)
{
return -ENOSYS;
}
Copy link
Member

Choose a reason for hiding this comment

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

I don't think thesefunctions are even needed to be defined here at all, as if they are NULL in the api struct, the api will just return -ENOSYS for you anyways

@XenuIsWatching
Copy link
Member

I only have time to skim through it right now (until I return from paternity leave a couple of weeks from now) rather than going line by line.

Skimming through, I don't see how devices are initialized with a valid DA that does not conflict with anything. How is this done?

@ramiroderojas
Copy link
Author

Thanks @XenuIsWatching. I was trying to get in touch with the Renesas folks so they confirm that DAA is supported (or not) in the device. I'll see how much of the datasheet we can share with you :)

Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@decsny
Copy link
Member

decsny commented Apr 15, 2024

bot is confused by the "imx", making it think it is an NXP thing, can you please address this somehow in the maintainer yml to indicate it is a renesas device and not NXP

@github-actions github-actions bot removed the Stale label Apr 16, 2024
Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Jun 15, 2024
@github-actions github-actions bot closed this Jun 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Devicetree Binding PR modifies or adds a Device Tree binding area: I2C area: I3C Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants