Re: [PATCH v2 2/5] soundwire: qcom: update port map allocation bit mask

[Srinivas Kandagatla]

On 09/03/2021 15:55, Pierre-Louis Bossart wrote:
> On 3/9/21 8:15 AM, Srinivas Kandagatla wrote:
> > currently the internal bitmask used for allocating ports starts with 
> > offset 0.
> > This is bit confusing as data port numbers on Qualcomm controller are 
> > valid
> > from 1 to 14. So adjust this bit mask accordingly, this will also help 
> > while
> > adding static port map support.
> >
> > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx>
> > ---
> >   drivers/soundwire/qcom.c | 11 +++++++----
> >   1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
> > index 6d22df01f354..f4f1c5f2af0b 100644
> > --- a/drivers/soundwire/qcom.c
> > +++ b/drivers/soundwire/qcom.c
> > @@ -519,7 +519,7 @@ static void qcom_swrm_stream_free_ports(struct 
> > qcom_swrm_ctrl *ctrl,
> >               port_mask = &ctrl->din_port_mask;
> >           list_for_each_entry(p_rt, &m_rt->port_list, port_node)
> > -            clear_bit(p_rt->num - 1, port_mask);
> > +            clear_bit(p_rt->num, port_mask);
> >       }
> >       mutex_unlock(&ctrl->port_lock);
> > @@ -552,13 +552,13 @@ static int qcom_swrm_stream_alloc_ports(struct 
> > qcom_swrm_ctrl *ctrl,
> >               list_for_each_entry(p_rt, &s_rt->port_list, port_node) {
> >                   /* Port numbers start from 1 - 14*/
> >                   pn = find_first_zero_bit(port_mask, maxport);
> > -                if (pn > (maxport - 1)) {
> > +                if (pn > (maxport)) {
>
> nit-pick: useless parentheses

I agree, will remove this and address the other comment too in next spin!

--srini
> >                       dev_err(ctrl->dev, "All ports busy\n");
> >                       ret = -EBUSY;
> >                       goto err;
> >                   }
> >                   set_bit(pn, port_mask);
> > -                pconfig[nports].num = pn + 1;
> > +                pconfig[nports].num = pn;
> >                   pconfig[nports].ch_mask = p_rt->ch_mask;
> >                   nports++;
> >               }
> > @@ -580,7 +580,7 @@ static int qcom_swrm_stream_alloc_ports(struct 
> > qcom_swrm_ctrl *ctrl,
> >   err:
> >       if (ret) {
> >           for (i = 0; i < nports; i++)
> > -            clear_bit(pconfig[i].num - 1, port_mask);
> > +            clear_bit(pconfig[i].num, port_mask);
> >       }
> >       mutex_unlock(&ctrl->port_lock);
> > @@ -754,6 +754,9 @@ static int qcom_swrm_get_port_config(struct 
> > qcom_swrm_ctrl *ctrl)
> >       ctrl->num_dout_ports = val;
> >       nports = ctrl->num_dout_ports + ctrl->num_din_ports;
> > +    /* port numbers are non zero, so mark port 0 */
>
> mask?
>
> > +    set_bit(0, &ctrl->dout_port_mask);
> > +    set_bit(0, &ctrl->din_port_mask);
> >       ret = of_property_read_u8_array(np, "qcom,ports-offset1",
> >                       off1, nports);