Re: [PATCH 3/4] clk: qcom: Add support for Display Clock Controller on Shikra

[Dmitry Baryshkov]

On Fri, May 22, 2026 at 02:39:01PM +0530, Imran Shaik wrote:
> 
> 
> On 20-05-2026 09:59 pm, Dmitry Baryshkov wrote:
> > On Tue, May 19, 2026 at 09:34:09AM +0530, Imran Shaik wrote:
> > > 
> > > 
> > > On 13-05-2026 08:38 pm, Dmitry Baryshkov wrote:
> > > > On Wed, May 13, 2026 at 04:51:03PM +0200, Konrad Dybcio wrote:
> > > > > On 5/13/26 4:06 PM, Dmitry Baryshkov wrote:
> > > > > > On Wed, May 13, 2026 at 05:01:16PM +0300, Dmitry Baryshkov wrote:
> > > > > > > On Wed, May 13, 2026 at 07:10:38PM +0530, Imran Shaik wrote:
> > > > > > > > Add a driver for the Display clock controller on Qualcomm Shikra SoC.
> > > > > > > > 
> > > > > > > > Signed-off-by: Imran Shaik <imran.shaik@xxxxxxxxxxxxxxxx>
> > > > > > > > ---
> > > > > > > >    drivers/clk/qcom/Kconfig         |  10 +
> > > > > > > >    drivers/clk/qcom/Makefile        |   1 +
> > > > > > > >    drivers/clk/qcom/dispcc-shikra.c | 565 +++++++++++++++++++++++++++++++++++++++
> > > > > > > >    3 files changed, 576 insertions(+)
> > > > > > > > 
> > > > > > > 
> > > > > > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxxxxxxxx>
> > > > > > 
> > > > > > After comparing the files...
> > > > > > 
> > > > > > Can we use dispcc-qcm2290.c instead? It uses clock-names instead of
> > > > > > clock-indices, but I think it should be fine to use clock-names as a
> > > > > > one-off.
> > > > > 
> > > > > Or we can convert it to use indices, since those are stable for agatti
> > > > > too - the names would remain in the binding, just unused by the driver
> > > > 
> > > > Either is fine for me.
> > > > 
> > > 
> > > Hi,
> > > 
> > > In Agatti, apart from the clock-names difference, I see that the AHB/XO
> > > clocks are not handled as always-on via the probe and instead rely on pm_clk
> > 
> > There is no pm_clk handling in Agatti driver.
> > 
> > > style handling, whereas Shikra follows the newer pattern by marking required
> > > CBCRs as critical during probe. I think that attempting to modify this
> > > approach into Agatti may introduce unnecessary complexity.
> > 
> > Well, you can start by explaining what caused the difference and the
> > result of those differences.
> > 
> > > 
> > > And the Agatti DISPCC doesn't have the DT_DSI1 bindings exposed, and
> > > updating this might break the ABI with respect to bindings, and DT.
> > 
> > You can add Shikra-specific bindings. See how it's handled for other
> > dispcc drivers.
> > 
> > > Given these and considering that Agatti is already stable, keeping the
> > > Shikra as separate GPUCC/DISPCC drivers is better to avoid the risk of
> > > regressions and complexity.
> > 
> > I think you've provided arguments for merging two drivers. It would
> > allow us to modernize Agatti driver and also to make sure that both
> > platforms use the well-tested code pattern.
> > 
> 
> Sure Dmitry, but we would like to proceed with Shikra as-is now since it
> already follows the latest upstream conventions, and will handle Agatti
> modernization as a follow-up series to align and reuse Shikra drivers.

Having two drivers for the (almost) same hw is a bad idea. Please either
refresh Agatti driver to follow the conventions and then add Shikra
support or add Shikra into the existing driver and then update it to
follow the standards. "we would like to proceed" is not a technical
argument, it's you trying to override the review for the managerial
reasons, which don't apply upstream.

-- 
With best wishes
Dmitry