Re: Re: [PATCH v4 3/4] dt-bindings: display: rockchip: Add schema for RK3588 HDMI TX Controller

[Conor Dooley]

On Fri, Aug 23, 2024 at 09:01:51AM +0800, Andy Yan wrote:
> 
> Hi，
> 
> 在 2024-08-22 19:59:43，"Cristian Ciocaltea" <cristian.ciocaltea@xxxxxxxxxxxxx> 写道：
> >On 8/22/24 11:41 AM, Conor Dooley wrote:
> >> On Thu, Aug 22, 2024 at 09:01:34AM +0200, Heiko Stübner wrote:
> >>> @Conor: just for me, did some shift happen in our understanding of dt-
> >>> best-practices in terms of syscon via phandle vs. syscon via compatible?
> >>>
> >>> Because Rockchip boards are referencing their GRFs via phandes forever
> >>> but similar to the soc vs non-soc node thing, I'd like to stay on top of
> >>> best-practices ;-)
> >> 
> >> If IP blocks, and thus drivers, are going to be reused between devices,
> >> using the phandles makes sense given that it is unlikely that syscon
> >> nodes can make use of fallback compatibles due to bits within that "glue"
> >> changing between devices. It also makes sense when there are multiple
> >> instances of an IP on the device, which need to use different syscons.
> >> My goal is to ask people why they are using these type of syscons
> >> phandle properties, cos often they are not required at all - for example
> >> with clocks where you effectively need a whole new driver for every
> >> single soc and having a phandle property buys you nothing.
> >
> >That would be also the case for this HDMI controller - need to check the
> >specs for the newer RK3576 SoC, but I expect the syscons would be quite
> >different when compared to RK3588, hence we should keep making use of
> >the phandles.
> 
> 
> Yes，for rk3576，it shares the same HDMI IP block（hdmi controller and PHY），
> of course reuse the driver of rk3588， but it has different GRF to depends on[0]:
> which calls ioc_grf and vo0_grf:
> 
> I also believe that makeing use of phandle beneficial for different devices to reuse the same code.
> 
> hdmi: hdmi@27da0000 {
>                 compatible = "rockchip,rk3576-dw-hdmi";
>                 reg = <0x0 0x27da0000 0x0 0x10000>, <0x0 0x27db0000 0x0 0x10000>;
>                 interrupts = <GIC_SPI 338 IRQ_TYPE_LEVEL_HIGH>,
>                              <GIC_SPI 339 IRQ_TYPE_LEVEL_HIGH>,
>                              <GIC_SPI 340 IRQ_TYPE_LEVEL_HIGH>,
>                              <GIC_SPI 341 IRQ_TYPE_LEVEL_HIGH>,
>                              <GIC_SPI 367 IRQ_TYPE_LEVEL_HIGH>;
> ,            rockchip,grf = <&ioc_grf>;
>              rockchip,vo0_grf = <&vo0_grf>;

btw, I don't particularly like this naming - on another soc in the
future "vo0_grf" could be "vo1_grf". It is better to name them after
what they are providing to the hdmi controller, rather than what the grf
itself is called.

That said, if the grf is changing between socs, the offset within the
grf and what it provides to the hdmi controller may vary completely,
which makes having generic grf reference properties redundant.

>              phys = <&hdptxphy_hdmi>;
>              phy-names = "hdmi";
> 
> 
> [0]https://github.com/armbian/linux-rockchip/blob/rk-6.1-rkr3/arch/arm64/boot/dts/rockchip/rk3576.dtsi#L3122C2-L3123C33
> 
> >
> >_______________________________________________
> >Linux-rockchip mailing list
> >Linux-rockchip@xxxxxxxxxxxxxxxxxxx
> >http://lists.infradead.org/mailman/listinfo/linux-rockchip

Attachment: signature.asc
Description: PGP signature