linux-kernel - Re: [PATCH v13 6/8] media: i2c: add DS90UB960 driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 16 May 2023 13:32:55 +0000
From:   Ludwig Zenz <lzenz@...electronics.com>
To:     Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
CC:     "Matti.Vaittinen@...rohmeurope.com" 
        <Matti.Vaittinen@...rohmeurope.com>,
        "andriy.shevchenko@...el.com" <andriy.shevchenko@...el.com>,
        "andriy.shevchenko@...ux.intel.com" 
        <andriy.shevchenko@...ux.intel.com>,
        "broonie@...nel.org" <broonie@...nel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "hverkuil@...all.nl" <hverkuil@...all.nl>,
        "khalasa@...p.pl" <khalasa@...p.pl>,
        "krzysztof.kozlowski+dt@...aro.org" 
        <krzysztof.kozlowski+dt@...aro.org>,
        "laurent.pinchart+renesas@...asonboard.com" 
        <laurent.pinchart+renesas@...asonboard.com>,
        "lgirdwood@...il.com" <lgirdwood@...il.com>,
        "linux-i2c@...r.kernel.org" <linux-i2c@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
        "luca.ceresoli@...tlin.com" <luca.ceresoli@...tlin.com>,
        "m.tretter@...gutronix.de" <m.tretter@...gutronix.de>,
        "marex@...x.de" <marex@...x.de>,
        "mchehab@...nel.org" <mchehab@...nel.org>,
        "mpagano@...too.org" <mpagano@...too.org>,
        "peda@...ntia.se" <peda@...ntia.se>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "sakari.ailus@...ux.intel.com" <sakari.ailus@...ux.intel.com>,
        "satish.nagireddy@...cruise.com" <satish.nagireddy@...cruise.com>,
        "wsa@...nel.org" <wsa@...nel.org>
Subject: Re: [PATCH v13 6/8] media: i2c: add DS90UB960 driver

> Hi,
> 
> On 16/05/2023 15:35, Ludwig Zenz wrote:
>> On Wed, 26 Apr 2023 14:51:12 +0300, Tomi Valkeinen wrote:
>>
>> [...]
>>
>>>   +static int ub960_configure_ports_for_streaming(struct ub960_data *priv,
>>>   +                                         struct v4l2_subdev_state *state)
>>>   +{
>>>   +  u8 fwd_ctl;
>>>   +  struct {
>>>   +          u32 num_streams;
>>>   +          u8 pixel_dt;
>>>   +          u8 meta_dt;
>>>   +          u32 meta_lines;
>>>   +          u32 tx_port;
>>>   +  } rx_data[UB960_MAX_RX_NPORTS] = {};
>>>   +  u8 vc_map[UB960_MAX_RX_NPORTS] = {};
>>>   +  struct v4l2_subdev_route *route;
>>>   +  unsigned int nport;
>>>   +  int ret;
>>>   +
>>>   +  ret = ub960_validate_stream_vcs(priv);
>>>   +  if (ret)
>>>   +          return ret;
>>>   +
>>>   +  ub960_get_vc_maps(priv, state, vc_map);
>>>   +
>>>   +  for_each_active_route(&state->routing, route) {
>>>   +          struct ub960_rxport *rxport;
>>>   +          struct ub960_txport *txport;
>>>   +          struct v4l2_mbus_framefmt *fmt;
>>>   +          const struct ub960_format_info *ub960_fmt;
>>>   +          unsigned int nport;
>>>   +
>>>   +          nport = ub960_pad_to_port(priv, route->sink_pad);
>>>   +
>>>   +          rxport = priv->rxports[nport];
>>>   +          if (!rxport)
>>>   +                  return -EINVAL;
>>>   +
>>>   +          txport = priv->txports[ub960_pad_to_port(priv, route->source_pad)];
>>>   +          if (!txport)
>>>   +                  return -EINVAL;
>>>   +
>>>   +          rx_data[nport].tx_port = ub960_pad_to_port(priv, route->source_pad);
>>>   +
>>>   +          rx_data[nport].num_streams++;
>>>   +
>>>   +          /* For the rest, we are only interested in parallel busses */
>>>   +          if (rxport->rx_mode == RXPORT_MODE_CSI2_SYNC ||
>>>   +              rxport->rx_mode == RXPORT_MODE_CSI2_ASYNC)
>>>   +                  continue;
>>>   +
>>>   +          if (rx_data[nport].num_streams > 2)
>>>   +                  return -EPIPE;
>>>   +
>>>   +          fmt = v4l2_subdev_state_get_stream_format(state,
>>>   +                                                    route->sink_pad,
>>>   +                                                    route->sink_stream);
>>>   +          if (!fmt)
>>>   +                  return -EPIPE;
>>>   +
>>>   +          ub960_fmt = ub960_find_format(fmt->code);
>>>   +          if (!ub960_fmt)
>>>   +                  return -EPIPE;
>>>   +
>>>   +          if (ub960_fmt->meta) {
>>>   +                  if (fmt->height > 3) {
>>>   +                          dev_err(&priv->client->dev,
>>>   +                                  "rx%u: unsupported metadata height %u\n",
>>>   +                                  nport, fmt->height);
>>>   +                          return -EPIPE;
>>>   +                  }
>>>   +
>>>   +                  rx_data[nport].meta_dt = ub960_fmt->datatype;
>>>   +                  rx_data[nport].meta_lines = fmt->height;
>>>   +          } else {
>>>   +                  rx_data[nport].pixel_dt = ub960_fmt->datatype;
>>>   +          }
>>>   +  }
>>>   +
>>>   +  /* Configure RX ports */
>>>   +
>>>   +  fwd_ctl = 0;
>>
>> Hello, I have only used the first RX port in my setup (ds90ub933 to ds90ub964). The logic for activating/deactivating the Rx ports did not work for me. My suggestion is:
> 
> Why doesn't it work? What happens?
> 
>  Tomi

Hello Tomi,

the port rx0 which I need was disabled and the other ports rx1 to rx3 were enabled. In other words, the exact inverse of the required selection.

>>>  +		/* Forwarding */
>>>  +
>>>  +		fwd_ctl |= BIT(4 + nport); /* forward disable */
According to the data sheet, a set bit4-7 in fwd_ctl means that the channel is disabled. So the comment 'forward disable' is correct. While debugging, however, this code was only reached for the ports to be enabled but not for the ones which should be disabled.

regards,
Ludwig