On 4/21/18 10:53 AM, Vinod Koul wrote:
On Sat, Apr 21, 2018 at 06:47:24AM -0700, Pierre-Louis Bossart wrote:
+static int sdw_enable_disable_slave_ports(struct sdw_bus *bus,
+ struct sdw_slave_runtime *s_rt,
+ struct sdw_port_runtime *p_rt, bool en)
+{
+ struct sdw_transport_params *t_params = &p_rt->transport_params;
+ u32 addr;
+ int ret;
+
+ if (bus->params.next_bank)
+ addr = SDW_DPN_CHANNELEN_B1(p_rt->num);
+ else
+ addr = SDW_DPN_CHANNELEN_B0(p_rt->num);
+
+ /*
+ * Since bus doesn't support sharing a port across two streams,
+ * it is safe to reset this register
+ */
+ if (en)
+ ret = sdw_update(s_rt->slave, addr, 0xFF, p_rt->ch_mask);
+ else
+ ret = sdw_update(s_rt->slave, addr, 0xFF, 0x0);
+
+ if (ret < 0)
+ dev_err(&s_rt->slave->dev,
+ "Slave chn_en reg write failed:%d port:%d",
+ ret, t_params->port_num);
+
+ return ret;
+}
Add a clarification that this routine only sets the enable/disable bits in
the relevant bank, the actual enable/disable is done with a bank switch.
Isn't that something spec tells the reader? this is a SDW concept that we
program bank and then switch to have the effect, so why would i add comment
for that?
To make sure people understand your code and follow the recommendation
from the spec. It's legal to write directly in enable bits but it's not
recommended at all. A standard spec is always interpreted in different
ways, adding a comment helps.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
