Skip to content

Refactor: Generalize Code by Removing Vendor and Android-Specific Elements #154

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.

Sign up for GitHub

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

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

Refactor: Generalize Code by Removing Vendor and Android-Specific Elements #154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3f3d24c
Remove vendor-specific DSP path definitions and fallback logic
quic-vkatoch Mar 18, 2025
b1d98fe
Remove Android reference from fastrpc_apps_user.h header guards
quic-vkatoch Apr 25, 2025
e7ff063
Rename listener_android to listener and update all references
quic-vkatoch Apr 25, 2025
f83c16d
Clean up unused code, TODO comments, and Android references
quic-vkatoch Apr 25, 2025
e26a068
Remove Android property system and build type dependencies
quic-vkatoch Apr 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion inc/AEEStdErr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@
#define AEE_EINVALIDITEM (AEE_EOFFSET + 0x02A) ///< Current item is invalid, it can be a switch case or a pointer to memory
#define AEE_ENOTALLOWED (AEE_EOFFSET + 0x02B) ///< Not allowed to perform the
///< operation
#define AEE_EINVHANDLE (AEE_EOFFSET + 0x02C) ///< Invalid handle - adding here as its defined in vendor AEEStdErr.h - needed to check valid handle in stub.c
#define AEE_EINVHANDLE (AEE_EOFFSET + 0x02C) ///< Invalid handle - needed to check valid handle in stub.c
#define AEE_EOUTOFHANDLES (AEE_EOFFSET + 0x02D) ///< Out of handles (Handle list is already full)
//Hole here
#define AEE_ENOMORE (AEE_EOFFSET + 0x02F) ///< No more items available --
Expand Down
2 changes: 1 addition & 1 deletion inc/Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ noinst_HEADERS = \
fastrpc_procbuf.h \
fastrpc_process_attributes.h \
fastrpc_trace.h \
listener_android.h \
listener.h \
listener_buf.h \
log_config.h \
mod_table.h \
Expand Down
9 changes: 1 addition & 8 deletions inc/apps_std_internal.h
View file
Open in desktop
Copy link

@Chennak-quic Chennak-quic Sep 2, 2025

Choose a reason for hiding this comment

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

If we are removing the support for /vendor/dsp, where is shell expected to be pushed for android targets ? if it is /usr/lib/dsp/ like other LE targets, is /usr partition available on android targets ?

how is this change tested on android targets ?

Copy link
Contributor Author

@quic-vkatoch quic-vkatoch Oct 23, 2025

Choose a reason for hiding this comment

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

I have to check where the shell should be placed if /vendor/dsp support is removed, or if we actually need to remove it in the first place.

Regarding testing: earlier, the daemons were already running (opened the shell) and the tests passed. After killing and updating with new daemons, the tests are now failing. I will check this further and update.

Copy link
Contributor

@lumag lumag Oct 23, 2025

Choose a reason for hiding this comment

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

The shell is loaded from the same folder as all other DSP libraries. What's the question?

Copy link
Contributor

@lumag lumag Oct 24, 2025

Choose a reason for hiding this comment

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

If we are removing the support for /vendor/dsp, where is shell expected to be pushed for android targets ? if it is /usr/lib/dsp/ like other LE targets, is /usr partition available on android targets ?

All DSP binaries are located in the subdirs of /usr/share/qcom, one subdir per device.

Copy link
Contributor

@lumag lumag Oct 24, 2025

Choose a reason for hiding this comment

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

On Android systems you can reuse the same approach, changing /usr/share/qcom/ to /vendor or its subdir. All config files should be relative to that path,

Original file line number Diff line number Diff line change
Expand Up @@ -39,16 +39,9 @@
#endif
#endif /* ENABLE_UPSTREAM_DRIVER_INTERFACE */

#ifndef VENDOR_DSP_LOCATION
Copy link
Contributor

@lumag lumag Oct 14, 2025

Choose a reason for hiding this comment

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

How are these two changes related to each other?

Copy link
Contributor Author

@quic-vkatoch quic-vkatoch Oct 14, 2025

Choose a reason for hiding this comment

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

These changes are unrelated; I will update the PR.

#define VENDOR_DSP_LOCATION "/vendor/dsp/"
#endif
#ifndef VENDOR_DOM_LOCATION
#define VENDOR_DOM_LOCATION "/vendor/dsp/xdsp/"
#endif

// Search path used by fastRPC to search skel library, .debugconfig and .farf files
#ifndef DSP_SEARCH_PATH
#define DSP_SEARCH_PATH ";/usr/lib/rfsa/adsp;/vendor/lib/rfsa/adsp;/vendor/dsp/;/usr/lib/dsp/;"
#define DSP_SEARCH_PATH ";/usr/lib/rfsa/adsp;/usr/lib/dsp/;"
#endif

// Search path used by fastRPC for acdb path
Expand Down
6 changes: 3 additions & 3 deletions inc/fastrpc_apps_user.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
// Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
// SPDX-License-Identifier: BSD-3-Clause

#ifndef FASTRPC_ANDROID_USER_H
#define FASTRPC_ANDROID_USER_H
#ifndef FASTRPC_USER_H
#define FASTRPC_USER_H

#include <assert.h>
#include <fcntl.h>
Expand Down Expand Up @@ -134,4 +134,4 @@ int get_unsigned_pd_attribute(uint32_t domain, int *unsigned_module);
*/
int is_userspace_allocation_supported();

#endif //FASTRPC_ANDROID_USER_H
#endif //FASTRPC_USER_H
1 change: 0 additions & 1 deletion inc/fastrpc_common.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,6 @@
FASTRPC_ENABLE_SYSTRACE = 6, //to enable tracing using Systrace
FASTRPC_DEBUG_PDDUMP = 7, // to enable pd dump debug data collection on rooted device for signed/unsigned pd
FASTRPC_PROCESS_ATTRS_PERSISTENT = 8, // to set proc attr as persistent
FASTRPC_BUILD_TYPE = 9 // Fetch build type of firmware image. It gives the details if its debug or prod build
}fastrpc_properties;

/**
Expand Down
3 changes: 0 additions & 3 deletions inc/fastrpc_trace.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@
#ifndef FASTRPC_TRACE_H
#define FASTRPC_TRACE_H

#if ((defined _ANDROID) || (defined ANDROID)) || (defined DISABLE_ATRACE) && !defined(LE_ENABLE)
Copy link
Contributor

@lumag lumag Oct 14, 2025

Choose a reason for hiding this comment

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

Again, totally unrelated changes

Copy link
Contributor Author

@quic-vkatoch quic-vkatoch Oct 14, 2025

Choose a reason for hiding this comment

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

These changes are unrelated; I will update the PR.

//TODO: Bharath #include "cutils/trace.h" //for systrace support
#endif
#include "HAP_farf.h"

//for systrace support
Expand Down
12 changes: 6 additions & 6 deletions inc/listener_android.h → inc/listener.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
// Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
// SPDX-License-Identifier: BSD-3-Clause

#ifndef LISTENER_ANDROID_H
#define LISTENER_ANDROID_H
#ifndef LISTENER_H
#define LISTENER_H

#include <semaphore.h>
#include <dlfcn.h>
Expand All @@ -22,18 +22,18 @@
/*
* API to initialize globals and internal data structures used in listener modules
*/
int listener_android_init(void);
int listener_init(void);
/*
* API to de-initialize globals and internal data structures used in listener modules
*/
void listener_android_deinit(void);
void listener_deinit(void);
/*
* API to initialize domain specific data strcutures used in listener modules
*/
int listener_android_domain_init(int domain, int update_requested, sem_t *r_sem);
int listener_domain_init(int domain, int update_requested, sem_t *r_sem);
/*
* API to de-initialize domain specific data strcutures used in listener modules
*/
void listener_android_domain_deinit(int domain);
void listener_domain_deinit(int domain);

#endif
2 changes: 1 addition & 1 deletion src/Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ LIBDSPRPC_SOURCES = \
dspsignal.c \
dspqueue/dspqueue_cpu.c \
dspqueue/dspqueue_rpc_stub.c \
listener_android.c \
listener.c \
Copy link
Contributor

@lumag lumag Oct 14, 2025

Choose a reason for hiding this comment

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

Sould this be a part of the previous commit?

Copy link
Contributor Author

@quic-vkatoch quic-vkatoch Oct 14, 2025

Choose a reason for hiding this comment

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

You're right, I'll do that.

apps_std_imp.c \
apps_mem_imp.c \
apps_mem_skel.c \
Expand Down
21 changes: 0 additions & 21 deletions src/adspmsgd_printf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,27 +20,6 @@ typedef struct __attribute__((packed)) {
char file[LOG_FILENAME_SIZE];
} LogNode;

#if 0
static inline android_LogPriority convert_level_to_android_priority(
enum adspmsgd_apps_Level level)
{
switch (level) {
case LOW:
return LOW;
case MEDIUM:
return MEDIUM;
case HIGH:
return HIGH;
case ERROR:
return ERROR;
case FATAL:
return FATAL;
default:
return 0;
}
}
#endif

int adspmsgd_apps_log(const unsigned char *log_message_buffer,
int log_message_bufferLen) {
LogNode *logMessage = (LogNode *)log_message_buffer;
Expand Down
13 changes: 3 additions & 10 deletions src/apps_std_imp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1088,7 +1088,7 @@ __QAIC_IMPL_EXPORT int __QAIC_IMPL(apps_std_fopen_with_env)(
}
FREEIF(absName);

// fallback: If not found in domain path /vendor/dsp/adsp try in /vendor/dsp
// fallback to open the file without domain
absNameLen = strlen(dirName) + strlen(name) + 2;
VERIFYC(NULL != (absName = (char *)malloc(sizeof(char) * absNameLen)),
AEE_ENOMEMORY);
Expand Down Expand Up @@ -1157,14 +1157,7 @@ __QAIC_IMPL_EXPORT int __QAIC_IMPL(apps_std_fopen_with_env_fd)(
VERIFYC(NULL != delim, AEE_EBADPARM);
VERIFYC(NULL != name, AEE_EBADPARM);
VERIFYC(NULL != envvarname, AEE_EBADPARM);
#if 0 //TODO: Bharath
char *tempName = name;
tempName += 2;
if (tempName[0] == '\0') {
nErr = AEE_EBADPARM;
goto bail;
}
#endif

FASTRPC_ATRACE_BEGIN_L("%s for %s in %s mode from path in environment "
"variable %s delimited with %s",
__func__, name, mode, envvarname, delim);
Expand Down Expand Up @@ -1223,7 +1216,7 @@ __QAIC_IMPL_EXPORT int __QAIC_IMPL(apps_std_fopen_with_env_fd)(
}
FREEIF(absName);

// fallback: If not found in domain path /vendor/dsp/adsp try in /vendor/dsp
// fallback to open the file without domain
absNameLen = strlen(dirName) + strlen(name) + 2;
VERIFYC(NULL != (absName = (char *)malloc(sizeof(char) * absNameLen)),
AEE_ENOMEMORY);
Expand Down
Loading
Loading