Re: [PATCH v1 5/5] ARM: tegra: add MIPI calibration binding for Tegra20/Tegra30

[Mikko Perttunen <mperttunen@nvidia.com>]

On Friday, July 18, 2025 6:26 PM Svyatoslav Ryhel wrote:
> пт, 18 лип. 2025 р. о 12:22 Mikko Perttunen <mperttunen@nvidia.com> пише:
> > On Friday, July 18, 2025 6:15 PM Svyatoslav Ryhel wrote:
> > > пт, 18 лип. 2025 р. о 12:01 Mikko Perttunen <mperttunen@nvidia.com> 
пише:
> > > > On Thursday, July 17, 2025 11:21 PM Svyatoslav Ryhel wrote:
> > > > > Add MIPI calibration device node for Tegra20 and Tegra30.
> > > > > 
> > > > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > > > ---
> > > > > 
> > > > >  arch/arm/boot/dts/nvidia/tegra20.dtsi | 14 ++++++++++++++
> > > > >  arch/arm/boot/dts/nvidia/tegra30.dtsi | 18 ++++++++++++++++++
> > > > >  2 files changed, 32 insertions(+)
> > > > > 
> > > > > diff --git a/arch/arm/boot/dts/nvidia/tegra20.dtsi
> > > > > b/arch/arm/boot/dts/nvidia/tegra20.dtsi index
> > > > > 92d422f83ea4..521261045cc8
> > > > > 100644
> > > > > --- a/arch/arm/boot/dts/nvidia/tegra20.dtsi
> > > > > +++ b/arch/arm/boot/dts/nvidia/tegra20.dtsi
> > > > > @@ -74,6 +74,16 @@ vi@54080000 {
> > > > > 
> > > > >                       status = "disabled";
> > > > >               
> > > > >               };
> > > > > 
> > > > > +             /* DSI MIPI calibration logic is a part of VI/CSI */
> > > > > +             mipi: mipi@54080220 {
> > > > > +                     compatible = "nvidia,tegra20-mipi";
> > > > > +                     reg = <0x54080220 0x100>;
> > > > > +                     clocks = <&tegra_car TEGRA20_CLK_VI>,
> > > > > +                              <&tegra_car TEGRA20_CLK_CSI>;
> > > > > +                     clock-names = "vi", "csi";
> > > > > +                     #nvidia,mipi-calibrate-cells = <1>;
> > > > > +             };
> > > > > +
> > > > 
> > > > As you say in the comment, MIPI calibration on Tegra20/30 is part of
> > > > VI/CSI. We can't add a "mipi" device here since such a device doesn't
> > > > exist in the hardware hierarchy. We already have the VI device in the
> > > > device tree, so we need to use that.
> > > 
> > > I understand your point, but embedding MIPI calibration logic into
> > > VI/CSI driver will bring up another lever of unnecessary complexity
> > > and excessive coding. While approach I have proposed preserves
> > > separation between CSI and DSI and reuses already existing MIPI
> > > calibration framework.
> > 
> > We can consider different driver architectures to simplify things, but the
> > device tree has to conform to hardware. The host1x bus has no 'mipi'
> > device on it so we can't put one in the device tree.
> 
> I assume then placing mipi node as CSI or VI child would resolve your
> concern?

The device tree is not intended to follow software architecture. Having a 
separate device node implies some sort of bus / parent-child hierarchy in 
hardware. So it seems a little difficult to justify.

In this case, it seems to me that it would be pretty easy to add/partially 
copy a small VI driver while integrating with the staging driver, based on how 
simple its probe function is. And given that you're planning on adding CSI 
support as well, utilizing the tegra-video driver seems like it's in the cards 
anyway.

Thanks,
Mikko

> > > > A driver for tegra20-vi already exists in
> > > > staging/drivers/media/tegra-video. We should aim not to break it.
> > > > Perhaps
> > > > bring it out of staging? (At least partially, but then why not the
> > > > whole
> > > > thing.)
> > > 
> > > It does not break VI/CSI, I have a WIP CSI implementation for
> > > Tegra20/Tegra30 which I hope to submit soon.
> > 
> > We have to use the tegra20-vi node as that matches hardware, and each node
> > can only be probed to one device, hence the issue.
> > 
> > Great to see a CSI driver!
> > 
> > Mikko
> > 
> > > > Thanks,
> > > > Mikko