intel / ipu6-drivers

152 stars 49 forks source link

ov2740: Fix link-frequency check #240

Closed jwrdegoede closed 2 weeks ago

jwrdegoede commented 4 weeks ago

It is sufficient for 1 of the link-frequencies reported in the bus-cfg to match with one of the link-frequencies from the link-freq menu.

This fixes the ipu6-driver version of the ov2740 driver not working when combined with the in kernel version of the ipu-bridge code.

hao-yao commented 3 weeks ago

Thank you @jwrdegoede , do we still need this under kernel < v6.8? I suppose after that we can directly use upstream driver.

jwrdegoede commented 3 weeks ago

Right this is still needed for older kernels.

hao-yao commented 2 weeks ago

Hi @jwrdegoede , checked upstream ov2740.c but seems not the same as here. How about us directly copy that version here?

    for (i = 0; i < ARRAY_SIZE(link_freq_menu_items); i++) {
        for (j = 0; j < bus_cfg.nr_of_link_frequencies; j++) {
            if (link_freq_menu_items[i] ==
                bus_cfg.link_frequencies[j])
                break;
        }

        if (j == bus_cfg.nr_of_link_frequencies)
            continue;

        switch (i) {
        case OV2740_LINK_FREQ_360MHZ_INDEX:
            ov2740->supported_modes = supported_modes_360mhz;
            ov2740->supported_modes_count =
                ARRAY_SIZE(supported_modes_360mhz);
            break;
        case OV2740_LINK_FREQ_180MHZ_INDEX:
            ov2740->supported_modes = supported_modes_180mhz;
            ov2740->supported_modes_count =
                ARRAY_SIZE(supported_modes_180mhz);
            break;
        }

        break; /* Prefer modes from first available link-freq */
    }

    if (!ov2740->supported_modes)
        ret = dev_err_probe(dev, -EINVAL,
                    "no supported link frequencies\n");
jwrdegoede commented 2 weeks ago

The ov2740.c copy in ipu6-drivers does not have the supported_modes and supported_modes_count fields. So to use the upstream check would require a full sync with the upstream driver.

A full sync seems fine with me, but I tried to keep the changes minimal.