Hi Konrad,
On 9/5/24 16:57, Konrad Dybcio wrote:
On 4.09.2024 4:04 AM, Richard Acayan wrote:
From: Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx>
It is common practice in the downstream and upstream CCI dt to set CCI
clock rates to 19.2 MHz. It appears to be fairly common for initial code to
set the CCI clock rate to 37.5 MHz.
Applying the widely used CCI clock rates from downstream ought not to cause
warning messages in the upstream kernel where our general policy is to
usually copy downstream hardware clock rates across the range of Qualcomm
drivers.
Drop the warning it is pervasive across CAMSS users but doesn't add any
information or warrant any changes to the DT to align the DT clock rate to
the bootloader clock rate.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx>
Reviewed-by: Vladimir Zapolskiy <vladimir.zapolskiy@xxxxxxxxxx>
Link: https://lore.kernel.org/linux-arm-msm/20240824115900.40702-1-bryan.odonoghue@xxxxxxxxxx
Signed-off-by: Richard Acayan <mailingradian@xxxxxxxxx>
---
I.. am not sure this is really a problem? On some platforms the core
clock is only 19.2 Mhz, but e.g. on sdm845 we have:
static const struct freq_tbl ftbl_cam_cc_cci_clk_src[] = {
F(19200000, P_BI_TCXO, 1, 0, 0),
F(37500000, P_CAM_CC_PLL0_OUT_EVEN, 16, 0, 0),
F(50000000, P_CAM_CC_PLL0_OUT_EVEN, 12, 0, 0),
F(100000000, P_CAM_CC_PLL0_OUT_EVEN, 6, 0, 0),
{ }
};
Shouldn't this be somehow dynamically calculated?
I believe the problem fixed by the change is an unnecessary dev_warn(), in
addition it's unclear why the CCI clock rate shall be strictly 37500000 for
all "CCI v2" platforms.
If the latter is a necessity, then it would be better to set the rate
explicitly, however since it's not done for any such platforms, I would say
that it is not needed.
And if it is not needed, or a default and universal 19.2MHz rate is good
enough, then I would suggest to remove everything 'cci_clk_rate' related
from the driver, and this makes the change incomplete...
--
Best wishes,
Vladimir
