Re: [PATCH 4/7] phy: qcom-mipi-csi2: Add sa8775p D-PHY support

[Anusha Arun Nandi]

Hi Bryan,

Thank you for the comments. Let me make the patches specific to the
functionality and make it more granular in the next version of the
patchset.
I agree opp_clk is supposed to be optional, I will make a patch for it
as well.

Best Regards,
Anusha

On 10/19/2026 3:01 AM, Bryan O'Donoghue wrote:
> On 17/10/2026 00:54, Anusha Arun Nandi wrote:
>> The timer-clock lookup in phy_qcom_mipi_csi2_probe() and the OPP
>> clkname passed to devm_pm_opp_set_clkname() were both hardcoded
>> string literals sized to x1e80100 ("timer"/"core"). Add
>> timer_clk_name/opp_clk_name to struct mipi_csi2phy_soc_cfg so each
>> SoC can declare its own clock-name strings; x1e80100's soc_cfg now
>> declares "timer"/"core" explicitly, with no behaviour change.
>> SA8775P reuses the same "timer"/"core" names as x1e80100 for this
>> pair of clocks.
> 
> LoL I had that in the last design and review said "hard code it" happy
> to agree with myself in the past, however this change should be
> standalone and precede the Lemans work.
> 
>> SA8775P's CSIPHY has a single power-domain, the Titan Top GDSC,
>> matching the legacy in-tree CAMSS driver's sa8775p_resources.pd_name
>> = "top" and the existing lemans.dtsi camss node. Like x1e80100's own
>> "top" domain, it is not OPP-scaled, only kept enabled.
>>
>> Signed-off-by: Anusha Arun Nandi<anusha.nandi@xxxxxxxxxxxxxxxx>
>> ---
>>   .../qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c    | 112 ++++++++++++++++++
>>   .../phy/qualcomm/phy-qcom-mipi-csi2-core.c    |   5 +-
>>   drivers/phy/qualcomm/phy-qcom-mipi-csi2.h     |   4 +
>>   3 files changed, 119 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c b/
>> drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c
>> index 966d79c98f9b..85e89235f7c9 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c
>> @@ -380,6 +380,118 @@ const struct mipi_csi2phy_soc_cfg
>> mipi_csi2_dphy_4nm_x1e = {
>>       .num_supplies = ARRAY_SIZE(x1e_supplies),
>>       .clk_names = (const char **)x1e_clks,
>>       .num_clk = ARRAY_SIZE(x1e_clks),
>> +    .timer_clk_name = "timer",
>> +    .opp_clk_name = "core",
>>       .genpds = x1e_genpds,
>>       .num_genpds = ARRAY_SIZE(x1e_genpds),
>>   };
> 
> Yes but split it up - "remove hard coded timer and opp names" - actually
> you want to make the OPP optional no ? In which case you'd expect to see
> a patch to make OPPs optional - instead of folding that into a
> functionality drop.
> 
> Because say we want to revert or re-order the addition of Lemans in the
> git log - we then have the problem of mixed functionality.
> 
> Or say I want to cherry-pick a backport - I don't necessarily want to
> include lots of SoC specific stuff that doesn't affect my part.
> 
> => More granular patches.
> 
> ---
> bod