Re: [PATCH v1 2/4] arm64: dts: qcom: sm8550: Add JPEG encoder node

[Atanas Filipov <atanas.filipov@oss.qualcomm.com>]

Thank you for the feedback. I understand the reasoning, but I 
respectfully disagree with this approach for the following reasons.

While it is true that the JPEG encoder shares the same camera NOC and 
power domain infrastructure as CAMSS, that is a hardware topology detail 
— not a sufficient justification for imposing a software dependency. The 
driver is a fully
self-contained V4L2 mem2mem encoder, implemented like every other JPEG 
encoder driver currently in the kernel (imx-jpeg, s5p-jpeg, mtk-jpeg, 
nxp-jpeg). None of those are sub-nodes of a parent ISP or camera 
subsystem driver.

Making the JPEG encoder a sub-node of camss would introduce an 
unnecessary and artificial coupling: the JPEG encoder cannot be probed, 
built, or used independently of the CAMSS driver, even on platforms 
where CAMSS is disabled. This directly contradicts the kernel's 
principle of independent, single-purpose drivers.

The shared hardware resources (clocks, interconnects, IOMMU stream IDs, 
power domain) are already fully described in the device tree node and 
handled by the standard kernel frameworks — there is no functional 
reason to nest the node under camss.

For these reasons I would prefer to keep the JPEG encoder as a 
standalone platform device with its own DT node, consistent with how all 
comparable JPEG encoder drivers are structured in the kernel today.

afilipov

On 6/13/2026 2:14 AM, Bryan O'Donoghue wrote:
> On 12/06/2026 20:44, Atanas Filipov wrote:
>> +        qcom_jpeg_enc: jpeg-encoder@ac4e000 {
> 
> One key bit of review feedback I gave in the previous leaked version of 
> this driver is that since the jpeg-encoder is part of the CAMSS block it 
> should be a sub-node of camss as OPE, CSIPHY and other blocks will be.
> 
> Please take that feedback onboard in your v2.
> 
> ---
> bod