Re: [PATCH 4/4] media: i2c: ds90ub960: Use VC mapping specific to ub9702

[Jai Luthra]

Quoting Rishikesh Donadkar (2026-06-08 19:59:01)
> Add a dedicated virtual channel mapping for ub9702 deserializers which
> require different VC configuration compared to ub960. Update the
> configuration logic to select the appropriate map based on the
> deserializer type.
> 
> Signed-off-by: Rishikesh Donadkar <r-donadkar@xxxxxx>
> ---
>  drivers/media/i2c/ds90ub960.c | 44 ++++++++++++++++++++++++++++-------
>  1 file changed, 35 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/media/i2c/ds90ub960.c b/drivers/media/i2c/ds90ub960.c
> index d1597659639f..9ce6d9e6e3e8 100644
> --- a/drivers/media/i2c/ds90ub960.c
> +++ b/drivers/media/i2c/ds90ub960.c
> @@ -3482,7 +3482,18 @@ static int ub960_rxport_handle_events(struct ub960_data *priv, u8 nport)
>   * as that is the most common HW configuration found on boards with DS90UB960.
>   * For using both CSI TX0 & TX1 the below method will need significant changes.
>   *
> + * VC mapping differs between ub960 and ub9702 deserializers:
> + * For ub960:
> + *             - Each VC uses 2 bits in the mapping register
> + *             - Supports up to 4 virtual channels (VC0-VC3)
> + * For ub9702:
> + *             - Each VC uses 4 bits in the mapping register
> + *             - Currently uses only 2 virtual channels (VC0, VC1)
> + *
> + * The mapping registers determine which output VC a given input VC
> + * will be mapped to when forwarding data from the deserializer.
>   */
> +
>  static void ub960_get_vc_maps(struct ub960_data *priv, u8 *vc_map)
>  {
>         struct device *dev = &priv->client->dev;
> @@ -3518,14 +3529,26 @@ static void ub960_get_vc_maps(struct ub960_data *priv, u8 *vc_map)
>                 }
>  
>                 /* Start with all channels mapped to first free output */
> -               map = (cur_vc << 6) | (cur_vc << 4) | (cur_vc << 2) |
> -                       (cur_vc << 0);
> +
> +               if (priv->hw_data->chip_type == UB960) {
> +                       map = (cur_vc << 6) | (cur_vc << 4) | (cur_vc << 2) |
> +                               (cur_vc << 0);
> +               } else {
> +                       map = (cur_vc << 4) | (cur_vc << 0);
> +               }
>  

Same comment as last patch, I think this means that UB9702 would be broken
without this patch, which is never a good idea in mainline.

Please squash together patches so that each patch is an independent change
that doesn't break any device.

I'm fine with a Co-developed-by tag in the final single patch as it's been
quite a while since I looked at this :-)

Thanks,
    Jai

>                 /* Map actually used to channels to distinct free outputs */
>                 for (vc = 0; vc < UB960_MAX_VC; ++vc) {
>                         if (used_vc[vc]) {
> -                               map &= ~(0x03 << (2 * vc));
> -                               map |= (cur_vc << (2 * vc));
> +                               if (priv->hw_data->chip_type == UB960) {
> +                                       /* For ub960: 2 bits per VC */
> +                                       map &= ~(0x03 << (2 * vc));
> +                                       map |= (cur_vc << (2 * vc));
> +                               } else {
> +                                       /* For ub9702: 4 bits per VC */
> +                                       map &= ~(0x0f << (4 * vc));
> +                                       map |= (cur_vc << (4 * vc));
> +                               }
>                                 ++cur_vc;
>                         }
>                 }
> @@ -3705,8 +3728,7 @@ static int ub960_configure_ports_for_streaming(struct ub960_data *priv,
>                                 for (i = 0; i < 8; i++)
>                                         ub960_rxport_write(priv, nport,
>                                                            UB9702_RR_VC_ID_MAP(i),
> -                                                          (nport << 4) | nport,
> -                                                          &ret);
> +                                                          vc_map[nport], &ret);
>                         }
>  
>                         break;
> @@ -4018,9 +4040,13 @@ static int ub960_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
>                 fd->entry[fd->num_entries].length = source_entry->length;
>                 fd->entry[fd->num_entries].pixelcode = source_entry->pixelcode;
>  
> -               fd->entry[fd->num_entries].bus.csi2.vc =
> -                       ub960_get_output_vc(vc_map[nport],
> -                                           source_entry->bus.csi2.vc);
> +               if (priv->hw_data->chip_type == UB960)
> +                       fd->entry[fd->num_entries].bus.csi2.vc =
> +                               (vc_map[nport] >> (2 * source_entry->bus.csi2.vc)) & 0x03;
> +               else
> +                       fd->entry[fd->num_entries].bus.csi2.vc =
> +                               (vc_map[nport] >> (4 * source_entry->bus.csi2.vc)) & 0x0f;
> +
>                 dev_dbg(dev, "Mapping sink %d/%d to output VC %d",
>                         route->sink_pad, route->sink_stream,
>                         fd->entry[fd->num_entries].bus.csi2.vc);
> -- 
> 2.34.1
>