Re: [PATCH 0/3] clk: qcom: add MSM8x60 Multimedia Clock Controller (MMCC) driver

[Herman van Hazendonk]

Hi,

Two things I need to flag against this v1 before further review:

1. Missed prerequisite (build break on a clean tree).

   mmcc-msm8660.c references LEGACY_FOOTSWITCH (4 GDSCs: ROT, IJPEG,
   VFE, VPE) and RPM_ALWAYS_ON (GFX3D), both of which are not in
   mainline gdsc.{c,h} today. I was supposed to send the GDSC
   framework groundwork series first; I sent mmcc by mistake before
   it. The prereq is incoming on linux-clk in a separate thread:

       Subject: [PATCH 0/2] clk: qcom: gdsc: groundwork for
                MSM8x60 power domains

   v2 of this MMCC series will declare the dependency explicitly in
   the cover letter and will not be sent before the gdsc series has
   review traction.

2. mmcc_msm8660_unhalt_fabric_ports() silent-fail bug.

   The function returns silently on every "RPM supplier not ready"
   path (no qcom,rpm-msm8660 node, of_find_device_by_node() returns
   NULL, device_link_add() fails, dev_get_drvdata() returns NULL).
   probe() ignores it. The "downstream clients will be enabled on
   demand" comment is wrong: mainline GDSC does not re-issue the
   unhalt on power-domain enable, so a system where qcom_rpm has
   not yet bound when mmcc probes ends up with the MMSS AXI fabric
   permanently halted. First MMSS DMA (MDP / CAMSS / GFX / JPEG /
   VPE / HDCODEC) silently freezes.

   v2 will:
     - change unhalt_fabric_ports() to return int,
     - return -EPROBE_DEFER on every "supplier not ready" path,
     - return -ENODEV via dev_err_probe() when the DT node is
       absent entirely,
     - propagate the result through probe() so the driver core
       retries when qcom_rpm finally binds.

   The patch is in my local tree and on-device validated; I will
   roll it into v2 once initial review feedback on the rest of the
   series has had a chance to settle (and once the gdsc prereq is
   on-list).

Apologies for the noise. v2 is coming, just not immediately.

Thanks,
Herman