Re: [PATCH WIP v4 9/9] media: qcom: camss: Account for C-PHY when calculating link frequency

On 20/03/2026 20:48, Cory Keitz wrote:
> On Sun, Mar 01, 2026 at 01:51:28AM +0100, David Heidelberg via B4 Relay wrote:
>> -static int csid_set_clock_rates(struct csid_device *csid)
>> +static int csid_set_clock_rates(struct v4l2_subdev *sd, struct csid_device *csid)
>>   {
>>   	struct device *dev = csid->camss->dev;
>> +	struct csiphy_device *csiphy = v4l2_get_subdevdata(sd);
>> +	struct csiphy_lanes_cfg *lane_cfg = &csiphy->cfg.csi2->lane_cfg;
> 
> I believe there's a regression here from v3. I've been testing v4 of
> this series on SA8775P (kernel 6.18) with a GMSL2 camera pipeline and
> hit an invalid pointer dereference in csid_set_clock_rates() during
> pipeline power-up:
> 
>    pc : camss_get_link_freq+0x1c/0xc4 [qcom_camss]
>    lr : csid_set_power+0xc8/0x404 [qcom_camss]
>    Code: 910003fd a90153f3 aa0003f4 a9025bf5 (b9400040)
> 
> The sd passed here from csid_set_power() is the csid subdev, so the
> subdevdata is a csid_device rather than a csiphy_device. The csid_device
> stores its linked csiphy_id during csid_link_setup(), so the fix is to
> look it up through the camss device array:
> 
>      static int csid_set_clock_rates(struct csid_device *csid)
>      {
>          struct csiphy_device *csiphy = &csid->camss->csiphy[csid->phy.csiphy_id];
>          struct csiphy_lanes_cfg *lane_cfg = &csiphy->cfg.csi2->lane_cfg;
> 
> This also lets us drop the v4l2_subdev parameter entirely since it's
> no longer needed. With the above change my pipeline powers on and
> streams correctly.

Thank you for catching this, until I'll sent next revision, it's fixed in the 
git branch [1].

David

[1] https://codeberg.org/sdm845/linux/commits/branch/b4/qcom-cphy

> 
> Tested-by: Cory Keitz <[email protected]>
> 
> ---
> 
> Regards,
> Cory

-- 
David Heidelberg