-
Notifications
You must be signed in to change notification settings - Fork 5
Add Advantech QCM6490 ASoC machine driver support #11
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
base: scarthgap
Are you sure you want to change the base?
Conversation
|
thanks for the patch! I believe this is the first patch to enable a 3rd party hardware, congrats :) I am expecting some more changes in order to move forward with this patch:
|
|
@advdarren thanks for the submission, can you provide us a bit more details about this target? As Nico said, the best way forward will be to create a new machine configuration that describes your target, so we can isolate the changes to it, and avoid causing side effects on the other targets available. |
| + {.compatible = "adv,qcs6490-rb3gen2-ia-sndcard", .data = &qcs6490_rb3gen2_ia_data}, | ||
| + {.compatible = "adv,qcs6490-rb3gen2-ptz-sndcard", .data = &qcs6490_rb3gen2_ptz_data}, | ||
| + {.compatible = "adv,qcs6490-rb3gen2-video-sndcard", .data = &qcs6490_rb3gen2_video_data}, | ||
| + {.compatible = "adv,qcs6490-rb3gen2-vision-sndcard", .data = &qcs6490_rb3gen2_vision_data}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These comat strings define Advantech RB3 Gen2, which is probably not what you've intended. Could you possibly describe, why you are adding new driver instead of extending an existing one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The driver is machine driver which is board specific, which need customization based the board design. Qualcomm machine driver (QCM6490.c) would not merge any customization from OEM/ODM that why customer need to create their own machine driver and do whatever customization they needed.
|
Hi @ndechesne and @ricardosalveti, Thanks for your feedback! I have added a dedicated machine configuration for our Advantech QCS6490 board (meta-advantech/conf/machine/qcs6490mio5355a1.conf) and updated the kernel recipe to only apply the 0001-ASoC-qcom-add-Advantech-QCM6490-machine-driver-suppo.patch when MACHINE = qcs6490mio5355a1. This should isolate the changes to our hardware and avoid impacting existing QCM6490-based boards (such as RB3 Gen2). For the new recipes-kernel/linux/linux-qcom-custom_%.bbappend as the following: FILESEXTRAPATHS:prepend := "${THISDIR}/${PN}:" SRC_URI:append:qcs6490mio5355a1 = " Is this correction appropriate? Thanks. |
|
Hi @lumag, Thanks for your feedback! Since the audio codec used on our board is not supported by the original RB3Gen2 audio machine driver, we have implemented our own audio machine driver to enable Canonical to integrate it into their Git repository. |
The overall approach looks good. I think it's slightly better if you use ${THISDIR}/ for the folder with the patches. Or at least THISDIR/ so that you get your own namespace for patches that apply to your platforms only. |
I think you have too many compat strings there. Most likely you don't actually need all of them. |
Are you planning on sending the machine configuration as a patch to be included as part of this layer? This would be ideal to get a wider reach (your own layer could depend on this one), but we will also request other pieces to be made available as well (e.g. boot firmware, etc). |
|
can not patch any existing machine driver which may break RB3G2 functionality because there is customization for OEM/ODM board. hence need to create new. |
Hi @ndechesne, Thank you for the suggestion. I plan to do the following modification:
FILESEXTRAPATHS:prepend := "${THISDIR}/${MACHINE}:" SRC_URI:append:qcs6490mio5355a1 = "
meta-qcom-3rdparty/recipes-kernel/linux/qcs6490mio5355a1/0001-ASoC-qcom-add-Advantech-QCM6490-machine-driver-suppo.patch Is this correction appropriate? Thanks. |
Hi @ricardosalveti , We do not plan to include the machine configuration in meta-qcom-3rdparty, because we already maintain our own meta-advantech layer, which contains the qcs6490mio5355a1.conf machine configuration. |
Can you please share the link of this OE layer? |
We do expect machine-specific changes to be aligned with a machine-specific configuration file which is also available as part of this layer, as our goal here is to enable additional hardware platforms but still be aligned with Qualcomm Linux itself. Your layer probably already depends on meta-qcom / meta-qcom-hwe, you would just need to add a dependency on meta-qcom-3rdparty and have your machine configuration provided by this layer instead. You should still be able to provide additional configuration, enablement and sample images via your own layer. |
Add a new patch to enable Advantech QCM6490 ASoC machine driver in the Qualcomm kernel. This patch integrates the driver configuration into the kernel build to support audio on Advantech QCM6490-based platforms. Changes: - Add 0001-ASoC-qcom-add-Advantech-QCM6490-machine-driver-suppo.patch - Update linux-qcom-custom_%.bbappend to include the new patch Signed-off-by: Darren Huang <[email protected]>
|
Hi @ndechesne and @ricardosalveti, Thanks for your feedback. Please help review the updated patch. Thanks! |
| require conf/machine/include/qcom-qcs6490.inc | ||
|
|
||
| # Assign machine arch to download qualcomm prebuild binary ex: fastrpc | ||
| MACHINE_ARCH = "qcs6490_rb3gen2_vision_kit" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not correct, as per yocto documentation, this variable cannot be hand-edited, see:
:term:`MACHINE_ARCH`
Specifies the name of the machine-specific architecture. This
variable is set automatically from :term:`MACHINE` or
:term:`TUNE_PKGARCH`. You should not hand-edit
the :term:`MACHINE_ARCH` variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you trying to share all other binaries from the rb3gen2?
I assume there are custom binaries (such as the boot firmware ones) which are specific to your hardware platform, can you confirm?
|
|
||
| # This DT currently exist only as patches against linux-qcom-base recipe. | ||
| KERNEL_DEVICETREE:pn-linux-qcom-base = " \ | ||
| qcom/qcs6490-rb3gen2.dtb \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does that mean that there is no specific DTB for your board, and you are directly using the same DTB as RB3G2?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also including a lot of dtbs that are really specific to rb3gen2, ideally yours should only contain the dtbs (and overlays) which are validated on your target.
Can you break this change into 2 commits? One just for the board config, and another just for your extra patch. Can you also explain how you are managing the DTBs used by your platform, and also how you are generating and using the pre-built binaries for it (e.g. firmware, boot firmware, etc)? |
Add a new patch to enable Advantech QCM6490 ASoC machine driver in the Qualcomm kernel. This patch integrates the driver configuration into the kernel build to support audio on Advantech QCM6490-based platforms.
Changes: