On 09/07/2025 02:10, ew.kim@samsung.com wrote:
+/**
- @cnotice
- @prdcode
- @Sub_SW_Component{abox generic}
- @ALM_Link {work item url}
- @purpose "Disbaling the abox generic"
- @logic "Disbale the abox generic"
- \image html
- @params
- @param{in, pdev->dev, struct::device, !NULL}
- @endparam
- @noret
- */
+static void samsung_abox_generic_remove(struct platform_device *pdev) +{
- struct device *dev = &pdev->dev;
- struct abox_generic_data *data = dev_get_drvdata(dev);
- dev_info(dev, "%s\n", __func__);
This is just poor code. Clean it up from all such oddities popular in downstream. Look at upstream code. Do you see such code there? No.
- if (!data) {
dev_err(dev, "%s: Invalid abox generic data\n", __func__);return;- }
- return;
+}
+/**
- @cnotice
- @prdcode
- @Sub_SW_Component{abox generic}
- @ALM_Link {work item url}
- @purpose "shutdown of the abox generic"
- @logic "Disbale the abox hardware by calling the following function
- pm_runtime_disable(dev)"
- \image html
- @params
- @param{in, pdev->dev, struct:: device, !NULL}
- @endparam
- @noret
- */
+static void samsung_abox_generic_shutdown(struct platform_device *pdev) +{
- struct device *dev = &pdev->dev;
- struct abox_generic_data *data = dev_get_drvdata(dev);
- if (!data) {
dev_err(dev, "%s: Invalid abox generic data\n", __func__);return;- }
- return;
+}
+static const struct of_device_id samsung_abox_generic_match[] = {
- {
.compatible = "samsung,abox_generic",- },
- {},
+}; +MODULE_DEVICE_TABLE(of, samsung_abox_generic_match);
+static const struct dev_pm_ops samsung_abox_generic_pm = {
- SET_SYSTEM_SLEEP_PM_OPS(abox_generic_suspend, abox_generic_resume)
+};
+struct platform_driver samsung_abox_generic_driver = {
- .probe = samsung_abox_generic_probe,
- .remove = samsung_abox_generic_remove,
- .shutdown = samsung_abox_generic_shutdown,
- .driver = {
.name = "samsung-abox-generic",.owner = THIS_MODULE,
So that's indeed 2013 code you upstream. We really want you to clean it up before you post some ancient stuff like that.
.of_match_table = of_match_ptr(samsung_abox_generic_match),.pm = &samsung_abox_generic_pm,- },
+};
+module_platform_driver(samsung_abox_generic_driver); +/* Module information */
Useless comment.
+MODULE_AUTHOR("Eunwoo Kim, ew.kim@samsung.com"); +MODULE_DESCRIPTION("Samsung ASoC A-Box Generic Driver"); +MODULE_ALIAS("platform:samsung-abox-generic");
No, drop. This was raised so many times already...
+MODULE_LICENSE("GPL v2");
diff --git a/sound/soc/samsung/auto_abox/generic/include/abox_generic.h b/sound/soc/samsung/auto_abox/generic/include/abox_generic.h new file mode 100644 index 000000000000..1c954272e2b5 --- /dev/null +++ b/sound/soc/samsung/auto_abox/generic/include/abox_generic.h @@ -0,0 +1,87 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/*
- ALSA SoC - Samsung ABOX Share Function and Data structure
- for Exynos specific extensions
- Copyright (C) 2013-2020 Samsung Electronics Co., Ltd.
- EXYNOS - sound/soc/samsung/abox/include/abox_generic.h
Same with paths. Do you see them anywhere in the kernel?
- */
+#ifndef __SND_SOC_ABOX_GENERIC_BASE_H +#define __SND_SOC_ABOX_GENERIC_BASE_H
+#define ABOX_GENERIC_DATA abox_generic_get_abox_generic_data();
+struct snd_soc_pcm_runtime;
+enum abox_soc_ioctl_cmd {
- ABOX_SOC_IOCTL_GET_NUM_OF_RDMA,
- ABOX_SOC_IOCTL_GET_NUM_OF_WDMA,
- ABOX_SOC_IOCTL_GET_NUM_OF_UAIF,
- ABOX_SOC_IOCTL_GET_SOC_TIMER,
- ABOX_SOC_IOCTL_SET_DMA_BUFFER,
- ABOX_SOC_IOCTL_SET_PP_POINTER,
- ABOX_SOC_IOCTL_SET_PERF_PERIOD,
- ABOX_SOC_IOCTL_CHECK_TIME_MUTEX,
- ABOX_SOC_IOCTL_CHECK_TIME_NO_MUTEX,
- ABOX_SOC_IOCTL_PCM_DUMP_INTR,
- ABOX_SOC_IOCTL_PCM_DUMP_CLOSE,
- ABOX_SOC_IOCTL_PCM_DUMP_ADD_CONTROL,
- ABOX_SOC_IOCTL_MAX
+};
+typedef int (*SOC_IOCTL)(struct device *soc_dev, enum abox_soc_ioctl_cmd cmd, void *data);
Follow coding style.
+struct abox_generic_data {
- struct platform_device *pdev;
- struct platform_device **pdev_pcm_playback;
- struct platform_device **pdev_pcm_capture;
- unsigned int num_of_pcm_playback;
- unsigned int num_of_pcm_capture;
- unsigned int num_of_i2s_dummy;
- unsigned int num_of_rdma;
- unsigned int num_of_wdma;
- unsigned int num_of_uaif;
- struct device *soc_dev;
- SOC_IOCTL soc_ioctl;
+};
+/************ Internal API ************/
Then why do you expose it via header?
+struct abox_generic_data *abox_generic_get_abox_generic_data(void);
+int abox_generic_set_dma_buffer(struct device *pcm_dev);
+int abox_generic_request_soc_ioctl(struct device *generic_dev, enum abox_soc_ioctl_cmd cmd,
- void *data);
+int abox_generic_set_pp_pointer(struct device *pcm_dev);
+/************ External API ************/
+extern struct device *abox_generic_find_fe_dev_from_rtd(struct snd_soc_pcm_runtime *be);
You cannot have external API. All API is internal first.
+extern struct platform_device *abox_generic_get_pcm_platform_dev(int pcm_id,
- int stream_type);
Best regards, Krzysztof