Skip to content

drivers: qcom: rpmh: split target specific configuration for Lemans #55

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
Shivam Sanjay (shvm-ap) wants to merge 1 commit into
base: qcom-next
Choose a base branch
from
Open

drivers: qcom: rpmh: split target specific configuration for Lemans #55

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 8 additions & 0 deletions core/arch/arm/plat-qcom/hoya/lemans/target_config.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,14 @@
#define AOP_CMD_DB_BASE UL(0x90860000)

@IDineshChoudhary IDineshChoudhary Sep 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

generally C requires U alias at last to indicate it's unsigned int why it's unsigned long here that maps to 64bit right?

@shvm-ap Shivam Sanjay (shvm-ap) Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, UL is not required here. However, it aligns with the existing coding style. May prevent unwanted overflows while adding new entries.

#define AOP_CMD_DB_SIZE UL(0x00020000)

#define AOP_MSG_RAM_BASE UL(0x0C300000)
#define AOP_MSG_RAM_SIZE UL(0x00100000)
#define MSG_RAM_SECTION_SIZE UL(0x00010000)
#define AOP_BOOT_COOKIE_SECTION UL(0xf)

#define RPMH_BASE_ADDR UL(0x18200000)
#define RPMH_RSC_SIZE UL(0x40000)

#define CFG_SEC_ELF_DDR_ADDR UL(0x908FF000)
#define CFG_SEC_ELF_DDR_SIZE UL(0x1000)

Expand Down
0 core/drivers/qcom/rpmh/rpmh_drv_config.c → ...rivers/qcom/rpmh/lemans/rpmh_drv_config.c
View file
Open in desktop
File renamed without changes.
54 changes: 54 additions & 0 deletions core/drivers/qcom/rpmh/lemans/rpmh_hwio.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/* SPDX-License-Identifier: BSD-2-Clause */

@IDineshChoudhary IDineshChoudhary Sep 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I am thinking to club this existing header itself rather than having a new one

@shvm-ap Shivam Sanjay (shvm-ap) Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We can have a rpmh_target.h header like the one used in #57, another possibility is clubbing in target_config.h but that is supposed to be a higher level description, I believe.

@zelvam95 Selvam (zelvam95) Sep 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, IDineshChoudhary; I discussed with Shivam as well & once he rebases his PR on top of the other PR which we're reviewing in upstream/qcom-next (raised by me), some of these comments would get addressed; We plan to do that once the other PR merges;

@IDineshChoudhary IDineshChoudhary Sep 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

also can for micros defined if this if any unused ones there and clean them up.

@shvm-ap Shivam Sanjay (shvm-ap) Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Will do

/*
* Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
*/

#ifndef __RPMH_HWIO_H__
#define __RPMH_HWIO_H__

#include <util.h>

#define DRV_STRIDE 0x10000
#define RSC_DRV_IRQ_ENABLE 0x0d00
#define RSC_DRV_IRQ_STATUS 0x0d04
#define RSC_DRV_IRQ_CLEAR 0x0d08

#define RSC_DRV_TCS_CONFIG 0x0C
#define TCS_BASE_OFFSET 0x0D10
#define TCS_STRIDE 0x2A0

#define TCS_CONTROL_OFFSET 0x04
#define TCS_STATUS_OFFSET 0x08
#define TCS_CMD_ENABLE_OFFSET 0x0C

#define TCS_CMD_BASE_OFFSET 0x20
#define TCS_CMDn_MSGID_OFFSET 0x00
#define TCS_CMDn_ADDR_OFFSET 0x04
#define TCS_CMDn_DATA_OFFSET 0x08
#define TCS_CMD_STRIDE 0x14

#define TCS_CONTROL_AMC_MODE_TRIGGER BIT(24)
#define TCS_CONTROL_AMC_MODE_EN BIT(16)

#define TCS_STATUS_CONTROLLER_IDLE BIT(0)

#define RSC_DRV_ERROR_IRQ_ENABLE 0xD8

#define EPCB_TIMEOUT_IRQ_EN_MASK BIT(20)
#define EPCB_TIMEOUT_THRESHOLD_SHIFT 0x0
#define EPCB_TIMEOUT_THRESHOLD_MASK 0xFFFF

#define MSGID_READ_OR_WRITE_SHIFT 0x10
#define MSGID_RES_REQ_SHIFT 0x8
#define MSGID_MSG_LENGTH_SHIFT 0x0

#define MSGID_READ 0x0
#define MSGID_WRITE 0x1

/* Each RPMh command transfers a single 32-bit word (encoded as 8 bytes). */
#define MSGID_MSG_LENGTH_VALUE 0x8

#define ADDR_SLV_ID_SHIFT 0x10
#define ADDR_OFFSET_SHIFT 0x0

#endif /* __RPMH_HWIO_H__ */
0 core/drivers/qcom/rpmh/rpmh_target_config.h → ...ers/qcom/rpmh/lemans/rpmh_target_config.h
View file
Open in desktop
File renamed without changes.
4 changes: 2 additions & 2 deletions core/drivers/qcom/rpmh/rpmh_client.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "rpmh_hal.h"
#include "rpmh_resource_commands.h"
#include "rpmh_tcs.h"
#include "rpmh_hwio.h"

register_phys_mem_pgdir(MEM_AREA_IO_NSEC, AOP_MSG_RAM_BASE,
CORE_MMU_PGDIR_SIZE);
Expand Down Expand Up @@ -66,7 +67,6 @@ struct client_queue {
};

#define AOP_BOOT_COOKIE 0xA0C00C1E
#define MSG_RAM_SECTION_SIZE 0x10000

struct aop_msg_ram_dict {
uint32_t boot_cookie_offset;
Expand Down Expand Up @@ -242,7 +242,7 @@ static TEE_Result check_aop_init(void)
return TEE_ERROR_GENERIC;
}

dict_addr = base + AOP_MSG_RAM_SIZE - MSG_RAM_SECTION_SIZE;
dict_addr = base + AOP_BOOT_COOKIE_SECTION * MSG_RAM_SECTION_SIZE;
dict = (struct aop_msg_ram_dict *)dict_addr;
cookie_addr = base + dict->boot_cookie_offset;

Expand Down
1 change: 1 addition & 0 deletions core/drivers/qcom/rpmh/rpmh_hal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <util.h>

#include "rpmh_hal.h"
#include "rpmh_hwio.h"

static struct {
vaddr_t rsc_base;
Expand Down
39 changes: 0 additions & 39 deletions core/drivers/qcom/rpmh/rpmh_hal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,45 +17,6 @@ enum hal_status {
HAL_STATUS_INVALID_PARAM = 2,
};

#define DRV_STRIDE 0x10000
#define RSC_DRV_IRQ_ENABLE 0x0d00
#define RSC_DRV_IRQ_STATUS 0x0d04
#define RSC_DRV_IRQ_CLEAR 0x0d08

#define RSC_DRV_TCS_CONFIG 0x0C
#define TCS_BASE_OFFSET 0x0D10 /* CMD_WAIT_FOR_CMPL base */
#define TCS_STRIDE 0x2A0

#define TCS_CONTROL_OFFSET 0x04
#define TCS_STATUS_OFFSET 0x08
#define TCS_CMD_ENABLE_OFFSET 0x0C

#define TCS_CMD_BASE_OFFSET 0x20
#define TCS_CMDn_MSGID_OFFSET 0x00
#define TCS_CMDn_ADDR_OFFSET 0x04
#define TCS_CMDn_DATA_OFFSET 0x08
#define TCS_CMD_STRIDE 0x14

#define TCS_CONTROL_AMC_MODE_TRIGGER BIT(24)
#define TCS_CONTROL_AMC_MODE_EN BIT(16)

#define TCS_STATUS_CONTROLLER_IDLE BIT(0)

#define RSC_DRV_ERROR_IRQ_STATUS 0xD0
#define RSC_DRV_ERROR_IRQ_ENABLE 0xD8
#define RSC_DRV_ERROR_IRQ_CLEAR 0xD4

#define EPCB_TIMEOUT_IRQ_EN_MASK BIT(20)
#define EPCB_TIMEOUT_THRESHOLD_SHIFT 0x0
#define EPCB_TIMEOUT_THRESHOLD_MASK 0xFFFF

#define MSGID_READ_OR_WRITE_SHIFT 0x10
#define MSGID_RES_REQ_SHIFT 0x8
#define MSGID_MSG_LENGTH_SHIFT 0x0

#define ADDR_SLV_ID_SHIFT 0x10
#define ADDR_OFFSET_SHIFT 0x0

enum hal_status hal_rpmh_init(vaddr_t rsc_base);
enum hal_status hal_rpmh_register_drv(enum rsc_drv_id drv_id);
enum hal_status hal_rpmh_read_config(enum rsc_drv_id drv_id,
Expand Down
6 changes: 4 additions & 2 deletions core/drivers/qcom/rpmh/sub.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,7 @@ srcs-y += rpmh_tcs.c
# Resource command tracking and management
srcs-y += rpmh_resource_commands.c

# DRV configuration
srcs-y += rpmh_drv_config.c
# Per-target DRV configuration, register layout, target config
srcs-y += $(PLATFORM_FLAVOR)/rpmh_drv_config.c
global-incdirs-y += .
global-incdirs-y += $(PLATFORM_FLAVOR)

@IDineshChoudhary IDineshChoudhary Sep 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is good.
Selvam (@zelvam95) your pull request require this change too then.

@zelvam95 Selvam (zelvam95) Sep 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actually, the includes in the same directory can be directly used using #include "" pattern. The includes inside chipset folder it won't be appropriate to include with chipset name in C File (since it won't scale) and hence we've used global-incdirs-y pattern.

The required includes are present in the PR I've raised & its compilation tested + unit tested on Lemans Device. Shivam has also unit tested my PR on Nord without this PR so its taken care.

@zelvam95 Selvam (zelvam95) Sep 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I have updated as response to the comments given in that PR clarifying the same as well;

@shvm-ap Shivam Sanjay (shvm-ap) Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Actually IDineshChoudhary, rpmh_drv_config has been removed (as of now) from #57, that's why that may be different. Also regarding the "global-incdirs-y += .", Selvam (@zelvam95) , should we use this and follow include <> or simply use the include "" pattern, and remove this entry?

@zelvam95 Selvam (zelvam95) Sep 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If the header file is in same driver file path, you could just do "".
If the header file is in some other patch, you can use global includes as required.
Especially if this header would be included by other drivers etc, you could use global includes.

In general - You could refer to existing upstream drivers/patterns and align to that;

@zelvam95 Selvam (zelvam95) Sep 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

And yes, IDineshChoudhary, We plan to rebase this PR on top of the other PR I have raised once that is merged. We could re-use some of the header files I have introduced etc. here and move macros there. That would make it cleaner. Maybe you could hold this review for a bit until the other PR is merged, so it'll be cleaner;

Shivam Sanjay (@shvm-ap), If it'd make sense, you could mark this as draft until the other PR is merged, since you will need the headers etc used in other PR over here for moving the MSG RAM Size etc.