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

Fix ES8388 vendor & Add I2S Bus driver #14

Merged
merged 5 commits into from
Nov 28, 2023
Merged

Fix ES8388 vendor & Add I2S Bus driver #14

merged 5 commits into from
Nov 28, 2023

Conversation

coolstar
Copy link
Collaborator

@coolstar coolstar commented Nov 27, 2023

Add I2S Bus driver that will be used by the WDM audio driver to read ACPI properties

Supported properties:

  • rockchip,dma -> DMA Controller Name
  • rockchip,tplg -> Name of topology (e.g. "hdmi" or "i2s-jack")
  • rockchip,tx -> DMA Channel number to use for TX
  • rockchip,rx -> DMA Channel number to use for RX

@mariobalanica
Copy link
Member

Nice!

Can we use the FixedDMA macro instead of encoding these properties into _DSD?

The info can be then read from CmResourceTypeDma's fields: https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/ns-wdm-_cm_partial_resource_descriptor

First resource could be for TX, the other one for RX.

drivers/audio/rk3xi2sbus/rk-tplg.h Show resolved Hide resolved
RtlZeroMemory(&inputBuffer, sizeof(inputBuffer));

inputBuffer.Signature = ACPI_EVAL_INPUT_BUFFER_SIGNATURE_EX;
status = RtlStringCchPrintfA(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since there aren't any format inserts in "_DSD", and since you're using sizeof for the buffer size, Consider RtlStringCbCopyA.

As a matter of taste, since RtlStringCbCopyA fails in a generally-safe mode (it truncates the copy and nul-terminates the output buffer), I tend to feel it's ok to ignore the return code from it. The only way it can fail is if you passed invalid parameters (which you don't), and it fails in a safe way even if you do (truncates the output), so checking the result doesn't do much good and just makes the code longer.

drivers/audio/rk3xi2sbus/rk-tplg.cpp Outdated Show resolved Hide resolved
{
UNREFERENCED_PARAMETER(ResourcesRaw);

BOOLEAN fMmioFound = FALSE;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Redundant - this is tracked by fdoCtx->m_MMIO.Base.Base == NULL


WDF_DECLARE_CONTEXT_TYPE_WITH_NAME(PDO_DEVICE_DATA, PdoGetData)

extern "C" {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think these really need to be extern "C".

drivers/audio/rk3xi2sbus/buspdo.cpp Show resolved Hide resolved
@coolstar
Copy link
Collaborator Author

Nice!

Can we use the FixedDMA macro instead of encoding these properties into _DSD?

The info can be then read from CmResourceTypeDma's fields: https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/ns-wdm-_cm_partial_resource_descriptor

First resource could be for TX, the other one for RX.

Just tried using FixedDMA in _CRS, and it causes the device to Code 12 since we don't implement a HalExt (and probably can't since we need a cyclic DMA buffer for audio)

@coolstar coolstar merged commit cca6df9 into master Nov 28, 2023
2 checks passed
@coolstar coolstar deleted the i2s-bus branch November 28, 2023 23:14
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