Re: [PATCH 3/3] arm64: dts: zynqmp: Wire up the DisplayPort subsystem

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Michal Simek <michal.simek@xilinx.com>
Cc: linux-kernel@vger.kernel.org, monstr@monstr.eu, git@xilinx.com,
	Krzysztof Kozlowski <krzk@kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 3/3] arm64: dts: zynqmp: Wire up the DisplayPort subsystem
Date: Fri, 22 Jan 2021 09:46:41 +0200	[thread overview]
Message-ID: <YAqC4WigXbw6ihqE@pendragon.ideasonboard.com> (raw)
In-Reply-To: <80b7a167-86ad-3012-b080-b380e7013f18@xilinx.com>

Hi Michal,

On Fri, Jan 22, 2021 at 08:19:15AM +0100, Michal Simek wrote:
> On 1/21/21 11:37 PM, Laurent Pinchart wrote:
> > On Thu, Jan 21, 2021 at 01:36:07PM +0100, Michal Simek wrote:
> >> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>
> >> Enable the dpsub device and wire it up to the PS-GTR PHY lanes routed to
> >> the DisplayPort connector.
> >>
> >> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> >> ---
> >>
> >> Wire all the boards
> >>
> >> ---
> >>  .../boot/dts/xilinx/zynqmp-zcu100-revC.dts    | 31 +++++++++++++++++++
> >>  .../boot/dts/xilinx/zynqmp-zcu102-revA.dts    | 10 ++++++
> >>  .../boot/dts/xilinx/zynqmp-zcu104-revA.dts    | 11 +++++++
> >>  .../boot/dts/xilinx/zynqmp-zcu104-revC.dts    | 11 +++++++
> >>  .../boot/dts/xilinx/zynqmp-zcu106-revA.dts    | 11 +++++++
> >>  .../boot/dts/xilinx/zynqmp-zcu111-revA.dts    | 11 +++++++
> >>  6 files changed, 85 insertions(+)
> >>
> >> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts b/arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts
> >> index 71ebcaadb7c8..a53598c3624b 100644
> >> --- a/arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts
> >> +++ b/arch/arm64/boot/dts/xilinx/zynqmp-zcu100-revC.dts
> >> @@ -15,6 +15,7 @@
> >>  #include <dt-bindings/input/input.h>
> >>  #include <dt-bindings/interrupt-controller/irq.h>
> >>  #include <dt-bindings/gpio/gpio.h>
> >> +#include <dt-bindings/phy/phy.h>
> >>  
> >>  / {
> >>  	model = "ZynqMP ZCU100 RevC";
> >> @@ -108,6 +109,18 @@ ina226 {
> >>  		compatible = "iio-hwmon";
> >>  		io-channels = <&u35 0>, <&u35 1>, <&u35 2>, <&u35 3>;
> >>  	};
> >> +
> >> +	si5335a_0: clk26 {
> >> +		compatible = "fixed-clock";
> >> +		#clock-cells = <0>;
> >> +		clock-frequency = <26000000>;
> >> +	};
> >> +
> >> +	si5335a_1: clk27 {
> >> +		compatible = "fixed-clock";
> >> +		#clock-cells = <0>;
> >> +		clock-frequency = <27000000>;
> >> +	};
> > 
> > This is fine as a workaround for now, but I'm still wondering how we'll
> > solve this properly. We can declare the SI5335A in DT without wiring the
> > output that provides the clock to the PS, otherwise it will be disabled
> > as part of the boot process.
> 
> All these clock chips are preprogrammed to certain rate and enabled by
> default. It means there doesn't need to be any SW handling to enable it.
> When driver for these clock chips comes we can change this that's why I
> used labels which are saying which output it is.

Unless I'm mistaken, on the ZCU106 board, the chip is an SI5341B, which
has a driver already. I tried to declare it in DT, but the PS_REF_CLK
then got disabled at the end of boot, and the system wasn't happy about
it :-)

-- 
Regards,

Laurent Pinchart