Re: [PATCH 3/3] clk: qcom: add MSM8x60 MMCC driver

[Dmitry Baryshkov]

On Tue, Jun 09, 2026 at 03:44:39PM +0200, Konrad Dybcio wrote:
> On 5/30/26 3:58 PM, Herman van Hazendonk wrote:
> > Add a clock driver for the Multimedia Clock Controller (MMCC) on the
> > MSM8x60 family (MSM8260/MSM8660/APQ8060) - the Scorpion-class
> > generation that preceded MSM8960's Krait CPUs.
> > 
> > The MMCC layout on MSM8x60 differs from MSM8960 in several ways that
> > make a separate driver cleaner than parameterising mmcc-msm8960.c:
> > 
> >   - the pix_rdi mux requires a custom set_parent op that temporarily
> >     enables both parents during the glitch-free transition;
> >   - the IJPEG GDSC requires releasing AXI, AHB and CORE resets;
> >   - several rate-source pairs (MDP pixel, GFX2D/3D) only exist on
> >     8x60 (e.g. PLL2-derived 228571000/266667000 for graphics);
> >   - the camera CSI / VFE / JPEG / VPE / ROT clock topology lacks the
> >     later 8960 reorganisation.
> > 
> > Used on the HP TouchPad (Tenderloin) for graphics (Adreno A220),
> > display (MDP4), camera (CSI/VFE), JPEG (Gemini), VIDC, VPE and
> > rotator. Reset IDs are exposed via a separate header so consumers
> > can reset the GDSCs and individual blocks.
> > 
> > Signed-off-by: Herman van Hazendonk <github.com@xxxxxxxxxx>
> > ---
> 
> [...]
> 
> > +	.clkr.hw.init = &(struct clk_init_data){
> > +		.name = "pll2",
> > +		.parent_data = (const struct clk_parent_data[]){
> > +			{ .fw_name = "pxo", .name = "pxo_board" },
> 
> Please drop .name (the kernel-global clock lookup), this is only a
> backwards compatiblity measure on existing drivers. For new entries,
> .index is the best (because well, it's the fastest)
> 
> [...]
> 
> > +static struct clk_branch camclk0_clk = {
> > +	.halt_reg = 0x01e8,

>From msm-3.0:

+#define DBG_BUS_VEC_I_REG                      REG_MM(0x01E8)


So, it seems, it is a debug reg, not an actual halt reg (but I might be
mistaken here, I haven't looked into the details).

> > +	.halt_bit = 15,
> > +	/*
> > +	 * The legacy webOS kernel used halt_reg = NULL for this clock,
> > +	 * meaning it never checked the halt status. The hardware doesn't
> > +	 * properly report the clock state via the halt register. Use
> > +	 * BRANCH_HALT_SKIP to avoid the "status stuck at 'off'" warning.
> 
> It may be that some piece of hw is holding this clock online behind the
> scenes. Is there perhaps a Qualcomm-authored commit that mentions the hw
> bug, or is it a guess? Due to the age of this chip I would imagine I
> won't be find an answer if you don't have one..
> 
> [...]
> 
> I see you have a lot of inline note-to-self comments, please strip some
> of them.
> 
> Konrad

-- 
With best wishes
Dmitry