Skip to content

Conversation

nchatrad
Copy link
Collaborator

splitting the previous pr 135

Copy link
Collaborator

@mahkurap mahkurap left a comment

Choose a reason for hiding this comment

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

Please add commit message with a brief description of the change.

@@ -301,10 +301,8 @@ static int sbtsi_i3c_probe(struct i3c_device *i3cdev)
struct regmap *regmap;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Commit message: please start with drivers:misc

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ack, will update

(i3cdev->desc->info.pid == 0x118) || (i3cdev->desc->info.pid == 0x010118) ||
(i3cdev->desc->info.pid == 0x01000118) || (i3cdev->desc->info.pid == 0x01010118)))
{
if (!(I3C_PID_INSTANCE_ID(i3cdev->desc->info.pid) == 0 ||
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is going to be good for both sp5 and sp7 based platforms . If not Need to look option to Valid PID bases check instead of hard coding.
static const u64 valid_sbtsi_pids[] = {
0x22400000001ULL,
// Add more if needed
};

Copy link
Collaborator

@akky16 akky16 Jul 1, 2025

Choose a reason for hiding this comment

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

@ojayanth
PID: 0x22400000001 is constant till Turin irrespective of socket number or die number.
From Venice the PID is changed to include, socket and die number info.
As there is only one PID till Turin, my understanding is, currently we can have the changes as is.

@@ -421,6 +421,10 @@ static const struct i3c_device_id sbtsi_i3c_id[] = {
I3C_DEVICE_EXTRA_INFO(0, 0x0101, 0x118, NULL), /* P1 - IOD1 - SBTSI */
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please follow commit message align to other commits, in the drivers.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ack, will update

@@ -421,6 +421,10 @@ static const struct i3c_device_id sbtsi_i3c_id[] = {
I3C_DEVICE_EXTRA_INFO(0, 0x0101, 0x118, NULL), /* P1 - IOD1 - SBTSI */
I3C_DEVICE_EXTRA_INFO(0x112, 0, 0x1, NULL),
I3C_DEVICE_EXTRA_INFO(0, 0x0, 0x0, NULL),
I3C_DEVICE_EXTRA_INFO(0x112, 0x0, 0x118, NULL), /* Socket:0, IOD:0 */
Copy link
Collaborator

Choose a reason for hiding this comment

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

what is the diffrence between PX vs Socket?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I am not sure what is P here? is it processor?

we are following terminology as used in PPR

From PPR:
Table 186: SBI Address Encoding
Socket ID / Socket number SB-RMI Address Comment
000b 3Ch IOD0 Socket 0 SB-RMI address
001b 38h IOD0 Socket 1 SB-RMI address
100b 34h IOD1 Socket 0 SB-RMI address
101b 35h IOD1 Socket 1 SB-RMI address

@@ -299,6 +299,7 @@ static int sbtsi_i3c_probe(struct i3c_device *i3cdev)
.val_bits = 8,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Commit message - srtart with driver:misc:

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ack, will update

@@ -324,8 +325,29 @@ static int sbtsi_i3c_probe(struct i3c_device *i3cdev)
tsi_dev->regmap = regmap;
mutex_init(&tsi_dev->lock);

/* Need to verify for the static address for i3cdev */
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use Helper Function + Lookup Table approch to avoid duplication here and line 389

static const char *sbtsi_addr_to_label(u8 addr)
{
switch (addr) {
case 0x44: return "0.1";
case 0x45: return "1.1";
case 0x48: return "1.0";
case 0x4c: return "0.0";
default: return NULL;
}
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ack, will update.

akky16 added 3 commits July 8, 2025 10:24
- From Fam: 0x1A, Model: 0x50, SBTSI device instance ID is 0, additionally
  PID for each silicon is different.
  SBRMI and SBTSI device PID's difference is in Instance ID.
  Modify the probe condition on basis of PID for legacy platform and on
  instance ID from Fam:0x1A Model:0x50.

Reviewed-by: Naveen Krishna Chatradhi <[email protected]>
Signed-off-by: Akshay Gupta <[email protected]>
- SBRMI/TSI devices for new APML has PID as per below description.
[11:0]  Additional ID: 0x118
[15:12] SB-TSI(0x0)
[31:16] [16]: DIE_ID, [25:24]: SOCKET ID
                sock 0 die 0: 0x0
                sock 0 die 1: 0x1
                sock 1 die 0: 0x100
                sock 1 die 1: 0x101
[32]: PID type select, it is 0 now to select fixed vendor value;
[47:33]: Manufacture ID, MIPI alliance has allocated the value for each company,
AMD should be 0x0112

Reviewed-by: Naveen Krishna Chatradhi <[email protected]>
Signed-off-by: Akshay Gupta <[email protected]>
Data name label under hwmon for i2c is sbtsi and for i3c is sbtsi_i3c.
In multi socket, this makes difficult to distinguish between socket 0
and socket 1. Hwmon entry names are updated for i2c/i3c to sbtsi_X.Y,
where X is socket index and Y is IOD index in the socket

Reviewed-by: Naveen Krishna Chatradhi <[email protected]>
Signed-off-by: Akshay Gupta <[email protected]>
@akky16 akky16 force-pushed the alertl_for_sbtsi branch from 79f9f96 to 1f9426e Compare July 8, 2025 10:26
@srgovard srgovard requested a review from ojayanth September 9, 2025 04:06
Copy link
Collaborator

@mahkurap mahkurap left a comment

Choose a reason for hiding this comment

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

why is this file not changed with label? --Pls Ignore this comment.

@mahkurap mahkurap self-requested a review September 10, 2025 19:57
@srgovard srgovard merged commit f59f1ee into dev-a1 Sep 10, 2025
@srgovard srgovard deleted the alertl_for_sbtsi branch September 10, 2025 21:36
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.

5 participants