Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] media: cec: extron-da-hd-4k-plus: lost PA update when gated only on update_pa --
commit 83f5e1abc765867f0cdd4fc7af830331a61a4349 Author: Hans Verkuil <[email protected]> media: cec: extron-da-hd-4k-plus: call cec_s_phys_addr only if EDID is present This commit updates extron_irq_work_handler() to call cec_s_phys_addr() when an EDID is present and cec_phys_addr_invalidate() when no EDID is present, so the CEC framework can distinguish between a missing EDID and an EDID without a physical address. > diff --git > a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c > b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c > index 1f8ed7ff31e52..0a0724fd89c0c 100644 > --- a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c > +++ b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c > @@ -643,8 +643,12 @@ static void extron_irq_work_handler(struct work_struct > *work) > extron_read_edid(port); > } > } > - if (update_pa) > - cec_s_phys_addr(port->adap, pa, false); > + if (update_pa) { > + if (has_edid) > + cec_s_phys_addr(port->adap, pa, false); > + else > + cec_phys_addr_invalidate(port->adap); > + } [Severity: High] Can gating this block solely on update_pa cause physical address updates or EDID-presence transitions to be lost when Pceco and Hdcp notifications are processed in separate extron_irq_work_handler() runs? port->phys_addr and port->has_edid are updated by independent asynchronous messages via extron_phys_addr_change() and extron_process_edid_change() -> extron_port_edid_change(). In extron_phys_addr_change(), update_phys_addr is only set when port->phys_addr changes: if (!port->update_phys_addr && port->phys_addr != pa) { update = true; port->update_phys_addr = true; } port->phys_addr = pa; During extron_setup(), CEC on output ports is enabled before extron->is_ready is set and before WHDCP queries initial EDID presence: err = extron_send_and_wait(extron, NULL, "WO20*CCEC", "CcecO20"); if (err) return err; ... extron->is_ready = true; /* Query HDCP and Signal states, used to update the initial state */ err = extron_send_and_wait(extron, NULL, "WHDCP", "Hdcp"); If Pceco arrives for a connected output sink while port->has_edid is still false (either during extron_setup() before WHDCP or on hotplug before Hdcp), the first run of extron_irq_work_handler() sees update_pa == true and has_edid == false, clears port->update_phys_addr, and calls cec_phys_addr_invalidate(port->adap). When WHDCP or Hdcp later sets port->has_edid = true and port->update_has_edid = true, the next run of extron_irq_work_handler() sees update_pa == false (and port->phys_addr already equals pa), leaving port->adap stuck at CEC_PHYS_ADDR_INVALID. Similarly, on hot-unplug, if Pceco with 0xffff is processed before Hdcp clears port->has_edid, the first work run sees update_pa == true and has_edid == true and calls cec_s_phys_addr(port->adap, CEC_PHYS_ADDR_INVALID, false), while the subsequent work run for update_has_edid == true sees update_pa == false and never calls cec_phys_addr_invalidate(port->adap). Likewise, connecting or disconnecting a DVI sink whose EDID has no physical address keeps pa at CEC_PHYS_ADDR_INVALID, so port->phys_addr != pa is false and update_pa is never set when update_has_edid triggers. Should this check also run when update_has_edid is true, for example: if (update_pa || update_has_edid) { if (has_edid) cec_s_phys_addr(port->adap, pa, false); else cec_phys_addr_invalidate(port->adap); } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=5
