Re: [PATCH 02/19] soundwire: amd: Add support for AMD Master driver

On 1/11/2023 10:02 AM, Vijendar Mukunda wrote:

AMD ACP IP block has two soundwire controller devices. Add support for

• Master driver probe & remove sequence
• Helper functions to enable/disable interrupts, Initialize sdw controller, enable sdw pads
• Master driver sdw_master_ops & port_ops callbacks

Signed-off-by: Vijendar Mukunda Vijendar.Mukunda@amd.com

...

+static int amd_sdwc_compute_params(struct sdw_bus *bus) +{

• struct sdw_transport_data t_data = {0};
• struct sdw_master_runtime *m_rt;
• struct sdw_port_runtime *p_rt;
• struct sdw_bus_params *b_params = &bus->params;
• int port_bo, hstart, hstop, sample_int;
• unsigned int rate, bps;
• port_bo = 0;

Double space before '='.

• hstart = 1;
• hstop = bus->params.col - 1;
• t_data.hstop = hstop;
• t_data.hstart = hstart;
• list_for_each_entry(m_rt, &bus->m_rt_list, bus_node) {

• rate = m_rt->stream->params.rate;
  

• bps = m_rt->stream->params.bps;
  

• sample_int = (bus->params.curr_dr_freq / rate);
  

• list_for_each_entry(p_rt, &m_rt->port_list, port_node) {
  

• 	port_bo = (p_rt->num * 64) + 1;
  

• 	dev_dbg(bus->dev, "p_rt->num=%d hstart=%d hstop=%d port_bo=%d\n",
  

• 		p_rt->num, hstart, hstop, port_bo);
  

• 	sdw_fill_xport_params(&p_rt->transport_params, p_rt->num,
  

• 			      false, SDW_BLK_GRP_CNT_1, sample_int,
  

• 			      port_bo, port_bo >> 8, hstart, hstop,
  

• 			      SDW_BLK_PKG_PER_PORT, 0x0);
  

• 	sdw_fill_port_params(&p_rt->port_params,
  

• 			     p_rt->num, bps,
  

• 			     SDW_PORT_FLOW_MODE_ISOCH,
  

• 			     b_params->m_data_mode);
  

• 	t_data.hstart = hstart;
  

• 	t_data.hstop = hstop;
  

• 	t_data.block_offset = port_bo;
  

• 	t_data.sub_block_offset = 0;
  

• }
  

• amd_sdwc_compute_slave_ports(m_rt, &t_data);
  

• }
• return 0;

+}

...

+static int amd_sdwc_port_enable(struct sdw_bus *bus,

• 		struct sdw_enable_ch *enable_ch,
  

• 		unsigned int bank)
  

+{

• struct amd_sdwc_ctrl *ctrl = to_amd_sdw(bus);
• u32 dpn_ch_enable;
• u32 ch_enable_reg, channel_type;
• switch (ctrl->instance) {
• case ACP_SDW0:

• channel_type = enable_ch->port_num;
  

• break;
  

• case ACP_SDW1:

• channel_type = enable_ch->port_num + ACP_SDW0_MAX_DAI;
  

• break;
  

• default:

• return -EINVAL;
  

• }
• switch (channel_type) {
• case ACP_SDW0_AUDIO_TX:

• ch_enable_reg = ACP_SW_AUDIO_TX_CHANNEL_ENABLE_DP0;
  

• break;
  

• case ACP_SDW0_HS_TX:

• ch_enable_reg = ACP_SW_HEADSET_TX_CHANNEL_ENABLE_DP0;
  

• break;
  

• case ACP_SDW0_BT_TX:

• ch_enable_reg = ACP_SW_BT_TX_CHANNEL_ENABLE_DP0;
  

• break;
  

• case ACP_SDW1_BT_TX:

• ch_enable_reg = ACP_P1_SW_BT_TX_CHANNEL_ENABLE_DP0;
  

• break;
  

• case ACP_SDW0_AUDIO_RX:

• ch_enable_reg = ACP_SW_AUDIO_RX_CHANNEL_ENABLE_DP0;
  

• break;
  

• case ACP_SDW0_HS_RX:

• ch_enable_reg = ACP_SW_HEADSET_RX_CHANNEL_ENABLE_DP0;
  

• break;
  

• case ACP_SDW0_BT_RX:

• ch_enable_reg = ACP_SW_BT_RX_CHANNEL_ENABLE_DP0;
  

• break;
  

• case ACP_SDW1_BT_RX:

• ch_enable_reg = ACP_P1_SW_BT_RX_CHANNEL_ENABLE_DP0;
  

• break;
  

• default:

• dev_err(bus->dev, "%s:Invalid channel:%d\n", __func__, channel_type);
  

• return -EINVAL;
  

• }
• dpn_ch_enable = acp_reg_readl(ctrl->mmio + ch_enable_reg);

Double space after '='.

• u32p_replace_bits(&dpn_ch_enable, enable_ch->ch_mask, AMD_DPN_CH_EN_CHMASK);
• if (enable_ch->enable)

• acp_reg_writel(dpn_ch_enable, ctrl->mmio + ch_enable_reg);
  

• else

• acp_reg_writel(0, ctrl->mmio + ch_enable_reg);
  

• return 0;

+}

...

+static void amd_sdwc_probe_work(struct work_struct *work) +{

• struct amd_sdwc_ctrl *ctrl = container_of(work, struct amd_sdwc_ctrl, probe_work);

Double space before '='.

• struct sdw_master_prop *prop;
• int ret;
• prop = &ctrl->bus.prop;
• if (!prop->hw_disabled) {

• ret = amd_enable_sdw_pads(ctrl);
  

• if (ret)
  

• 	return;
  

• ret = amd_init_sdw_controller(ctrl);
  

• if (ret)
  

• 	return;
  

• amd_enable_sdw_interrupts(ctrl);
  

• ret = amd_enable_sdw_controller(ctrl);
  

• if (ret)
  

• 	return;
  

• ret = amd_sdwc_set_frameshape(ctrl, 50, 10);
  

• if (!ret)
  

• 	ctrl->startup_done = true;
  

• }

+}

+static int amd_sdwc_probe(struct platform_device *pdev) +{

• const struct acp_sdw_pdata *pdata = pdev->dev.platform_data;
• struct resource *res;
• struct device *dev = &pdev->dev;

Same as in previous patch, you assign dev here, but keep using &pdev->dev below?

• struct sdw_master_prop *prop;
• struct sdw_bus_params *params;
• struct amd_sdwc_ctrl *ctrl;
• int ret;
• if (!pdev->dev.platform_data) {

• dev_err(&pdev->dev, "platform_data not retrieved\n");
  

• return -ENODEV;
  

• }
• ctrl = devm_kzalloc(&pdev->dev, sizeof(struct amd_sdwc_ctrl), GFP_KERNEL);
• if (!ctrl)

• return -ENOMEM;
  

• res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
• if (!res) {

• dev_err(&pdev->dev, "IORESOURCE_MEM FAILED\n");
  

• return -ENOMEM;
  

• }
• ctrl->mmio = devm_ioremap(&pdev->dev, res->start, resource_size(res));
• if (IS_ERR(ctrl->mmio)) {

• dev_err(&pdev->dev, "mmio not found\n");
  

• return PTR_ERR(ctrl->mmio);
  

• }
• ctrl->instance = pdata->instance;
• ctrl->sdw_lock = pdata->sdw_lock;

Double space before '='.

• ctrl->rows_index = sdw_find_row_index(50);
• ctrl->cols_index = sdw_find_col_index(10);
• ctrl->dev = dev;
• dev_set_drvdata(&pdev->dev, ctrl);
• ctrl->bus.ops = &amd_sdwc_ops;
• ctrl->bus.port_ops = &amd_sdwc_port_ops;
• ctrl->bus.compute_params = &amd_sdwc_compute_params;
• ctrl->bus.clk_stop_timeout = 1;
• switch (ctrl->instance) {
• case ACP_SDW0:

• ctrl->num_dout_ports =  AMD_SDW0_MAX_TX_PORTS;
  

Double space after '='.

• ctrl->num_din_ports = AMD_SDW0_MAX_RX_PORTS;
  

• break;
  

• case ACP_SDW1:

• ctrl->num_dout_ports = AMD_SDW1_MAX_TX_PORTS;
  

• ctrl->num_din_ports = AMD_SDW1_MAX_RX_PORTS;
  

• break;
  

• default:

• return -EINVAL;
  

• }
• params = &ctrl->bus.params;
• params->max_dr_freq = AMD_SDW_DEFAULT_CLK_FREQ * 2;
• params->curr_dr_freq = AMD_SDW_DEFAULT_CLK_FREQ * 2;
• params->col = 10;
• params->row = 50;
• prop = &ctrl->bus.prop;
• prop->clk_freq = &amd_sdwc_freq_tbl[0];
• prop->mclk_freq = AMD_SDW_BUS_BASE_FREQ;
• ctrl->bus.link_id = ctrl->instance;
• ret = sdw_bus_master_add(&ctrl->bus, dev, dev->fwnode);
• if (ret) {

• dev_err(dev, "Failed to register Soundwire controller (%d)\n",
  

• 	ret);
  

• return ret;
  

• }
• INIT_WORK(&ctrl->probe_work, amd_sdwc_probe_work);
• schedule_work(&ctrl->probe_work);
• return 0;

+}

+static int amd_sdwc_remove(struct platform_device *pdev) +{

• struct amd_sdwc_ctrl *ctrl = dev_get_drvdata(&pdev->dev);
• int ret;

You may need to cancel work if someone tries to unload driver before probe work completes. Something like

cancel_work_sync(&ctrl->probe_work);

should probably work here.

• amd_disable_sdw_interrupts(ctrl);
• sdw_bus_master_delete(&ctrl->bus);
• ret = amd_disable_sdw_controller(ctrl);
• return ret;

+}