On 6/13/23 07:42, Mukunda,Vijendar wrote:
On 12/06/23 23:39, Pierre-Louis Bossart wrote:
=
+static int sdw_amd_scan_controller(struct device *dev) +{
- struct acp63_dev_data *acp_data;
- struct fwnode_handle *link;
- char name[32];
- u32 sdw_manager_bitmap;
- u8 count = 0;
- u32 acp_sdw_power_mode = 0;
- int index;
- int ret;
- acp_data = dev_get_drvdata(dev);
- /*
* Current implementation is based on MIPI DisCo 2.0 spec.* Found controller, find links supported.*/- ret = fwnode_property_read_u32_array((acp_data->sdw_fw_node), "mipi-sdw-manager-list",
&sdw_manager_bitmap, 1);- if (ret) {
dev_err(dev, "Failed to read mipi-sdw-manager-list: %d\n", ret);return -EINVAL;- }
- count = hweight32(sdw_manager_bitmap);
- /* Check count is within bounds */
- if (count > AMD_SDW_MAX_MANAGERS) {
dev_err(dev, "Manager count %d exceeds max %d\n", count, AMD_SDW_MAX_MANAGERS);return -EINVAL;- }
nit-pick: the count is not enough, you should also check that only bits 0 and 1 are set in mipi-sdw-manager-list...
As per our design for PS platform, we will go with two bit map values as 0x03 and 0x01.
- As per ACP PIN CONFIG, we support Single SDW Manager instance
which refers to SW0 manager instance. For this, we need to use bitmap value as 0x01. 2. Other bit map value - 0x03 will be used to populate two SoundWire manager instances. We have extra sub property "amd-sdw-enable" to invoke the init sequence for SoundWire manager.
As we are supporting two bit map value combinations here, it's not required to check bit set value. count value is enough to know manager instance count. It doesn't break anything.
Not a blocker but you underestimate the creativity of UEFI BIOS writers...