Skip to content
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

[RFC] Read lane mapping information for soundwire peripherals #4704

Closed
wants to merge 2 commits into from
Closed

[RFC] Read lane mapping information for soundwire peripherals #4704

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6abd077
soundwire: mipi_disco: read lane mapping properties from ACPI
Nov 14, 2023
8708d90
ASoC: rt722-sdca-sdw: read DisCo properties in read_prop ops
Nov 14, 2023
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
31 changes: 31 additions & 0 deletions drivers/soundwire/mipi_disco.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,35 @@ static int sdw_slave_read_dpn(struct sdw_slave *slave,
return 0;
}

/*
* In MIPI DisCo spec for SoundWire, lane mapping for a slave device is done with
* mipi-sdw-lane-x-mapping properties, where x is 1..7, and the values for those
* properties are mipi-sdw-manager-lane-x or mipi-sdw-peripheral-link-y, where x
* is an integer between 1 to 7 if the lane is connected to a master lane, y is a
* character between A to E if the lane is connected to another slave lane.
Copy link
Member

Choose a reason for hiding this comment

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

be consistent and use manager/peripheral

*/
static int sdw_slave_read_lane_mapping(struct sdw_slave *slave)
{
struct sdw_slave_prop *prop = &slave->prop;
struct device *dev = &slave->dev;
char prop_name[30];
Copy link
Member

Choose a reason for hiding this comment

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

add a define and explanation for the size

const char *prop_val;
size_t len;
int ret, i;

for (i = 0; i < SDW_MAX_LANES; i++) {
Copy link
Member

Choose a reason for hiding this comment

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

i=1, lane0 is always connected

snprintf(prop_name, sizeof(prop_name), "mipi-sdw-lane-%d-mapping", i);
ret = device_property_read_string(dev, prop_name, &prop_val);
Copy link
Member

Choose a reason for hiding this comment

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

why read a string if you are trying to read an hex value?

Copy link
Collaborator Author

@aiChaoSONG aiChaoSONG Nov 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

That's the dilemma here, the possible values for the property mipi-sdw-lane-x-mapping are mipi-sdw-manager-lane-y or mipi-sdw-peripheral-link-z in the DisCo spec, where y is an integet between 1 to 7, z is an char between A to E.

With the last character y or z, the link is able to be identified, so I just use the last character from the property value string. Here I store them as character, '1', '2', ... '7', 'A',...,'E'. A second step to transform them to hex value is possible.
Or we could store the intact string mipi-sdw-manager-lane-y/mipi-sdw-peripheral-link-z with a string array, but I think it will be hard to use later, because I think the 1..7 number also denote the lane ID for master device, parsing is needed later.

Copy link
Collaborator

Choose a reason for hiding this comment

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

why read a string if you are trying to read an hex value?

Because the value shall be mipi-sdw-manager-lane-<m>, where is an integer between 1 and 7 if the lane connects to Manager, or mipi-sdw-peripheral-link-<tag>, where is 1 to 5 characters that uniquely identify the Link in the system if the line connects to Peripheral.

Copy link
Member

Choose a reason for hiding this comment

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

Ah yes, I missed this in the spec, thanks for enlightening me @bardliao and @aiChaoSONG !

I would report a warning on mipi-sdw-peripheral-link- and ignore it because we don't have support for device-to-device communication. We may add this in the future but this requires a complete rework of the 'stream' definition where there is no manager involved. I don't even know how we would represent this with DPCM, all dai links have cpu- and codec DAIs.

if (ret)
continue;
len = strlen(prop_val);
/* The last character is enough to identify the connection */
prop->lane_maps[i] = prop_val[len - 1];
dev_err(dev, "[Chao] %c\n", prop->lane_maps[i]);
}
return 0;
}

/**
* sdw_slave_read_prop() - Read Slave properties
* @slave: SDW Slave
Expand Down Expand Up @@ -382,6 +411,8 @@ int sdw_slave_read_prop(struct sdw_slave *slave)
sdw_slave_read_dpn(slave, prop->sink_dpn_prop, nval,
prop->sink_ports, "sink");

sdw_slave_read_lane_mapping(slave);

return 0;
}
EXPORT_SYMBOL(sdw_slave_read_prop);
4 changes: 4 additions & 0 deletions include/linux/soundwire/sdw.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ struct sdw_slave;
#define SDW_MAX_PORTS 15
#define SDW_VALID_PORT_RANGE(n) ((n) < SDW_MAX_PORTS && (n) >= 1)

#define SDW_MAX_LANES 8

enum {
SDW_PORT_DIRN_SINK = 0,
SDW_PORT_DIRN_SOURCE,
Expand Down Expand Up @@ -361,6 +363,7 @@ struct sdw_dpn_prop {
* @p15_behave: Slave behavior when the Master attempts a read to the Port15
* alias
* @lane_control_support: Slave supports lane control
* @lane_maps: Lane mapping for the slave
* @master_count: Number of Masters present on this Slave
* @source_ports: Bitmap identifying source ports
* @sink_ports: Bitmap identifying sink ports
Expand Down Expand Up @@ -388,6 +391,7 @@ struct sdw_slave_prop {
bool bank_delay_support;
enum sdw_p15_behave p15_behave;
bool lane_control_support;
u8 lane_maps[SDW_MAX_LANES];
u32 master_count;
u32 source_ports;
u32 sink_ports;
Expand Down
2 changes: 2 additions & 0 deletions sound/soc/codecs/rt722-sdca-sdw.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,8 @@ static int rt722_sdca_read_prop(struct sdw_slave *slave)
unsigned long addr;
struct sdw_dpn_prop *dpn;

sdw_slave_read_prop(slave);

plbossart marked this conversation as resolved.
Show resolved Hide resolved
prop->scp_int1_mask = SDW_SCP_INT1_BUS_CLASH | SDW_SCP_INT1_PARITY;
prop->quirks = SDW_SLAVE_QUIRKS_INVALID_INITIAL_PARITY;

Expand Down