Re: [PATCH v9 7/7] arm64: dts: qcom: sdm670-google-sargo: add imx355 front camera

[Richard Acayan <mailingradian@gmail.com>]

On Tue, Mar 31, 2026 at 10:02:55AM +0300, Sakari Ailus wrote:
> On Mon, Mar 30, 2026 at 09:27:59PM -0400, Richard Acayan wrote:
> > On Tue, Mar 24, 2026 at 12:57:59PM +0200, Sakari Ailus wrote:
> > > Hi David, Richard,
> > > 
> > > On Tue, Mar 24, 2026 at 10:35:24AM +0100, David Heidelberg wrote:
> > > > 
> > > > 
> > > > On 24/03/2026 03:05, Richard Acayan wrote:
> > > > > On Fri, Mar 13, 2026 at 07:26:47PM +0100, David Heidelberg wrote:
> > > > > > On 17/02/2026 01:27, Richard Acayan wrote:
> > > > > > [...]
> > > > > > 
> > > > > > > +&cci_i2c1 {
> > > > > > > +	camera@1a {
> > > > > > > +		compatible = "sony,imx355";
> > > > > > > +		reg = <0x1a>;
> > > > > > > +
> > > > > > > +		clocks = <&camcc CAM_CC_MCLK2_CLK>;
> > > > > > > +
> > > > > > > +		assigned-clocks = <&camcc CAM_CC_MCLK2_CLK>;
> > > > > > > +		assigned-clock-rates = <19200000>;
> > > > > > 
> > > > > > Extract from #sdm670-mainline:erebion.eu discussion:
> > > > > > The imx355 can operate on 24 MHz (on both Pixel 3 and 3a), but Linux kernel
> > > > > > driver can operate only with 19.2 MHz.
> > > > > > 
> > > > > > I assume it would be worth it mention at least by comment here.
> > > > > 
> > > > > This might set the series back because the devicetree isn't meant to be
> > > > > written for specific software, but it's included in v11 because you
> > > > > already asked twice.
> > > > > 
> > > > 
> > > > I would say node with lower clock frequency is still much better than
> > > > nothing or placeholder saying "i2c camera here". Instead we'll have small
> > > > placeholder that value can be bumped to 24 MHz. Important is this can be
> > > > easily improved when at least one consumer of the device-tree gains support.
> > > > 
> > > > We have very scarce support of cameras on mobile phones in mainline, thus
> > > > leaving a comment that HW can do 24 MHz is reasonable compromise IMHO.
> > > 
> > > The bindings could document the supported frequency range.
> > > 
> > > In DTS it may make sense to set the frequency the vendor uses as it may
> > > affect the link frequencies (albeit I guess they're the same in this
> > > case?).
> > 
> > Is this review relevant to v11?
> 
> I'd think so: it doesn't mention the frequency should be 24 MHz.

Ok, please check the dts comment in v12 when I send it and continue there.