Dne nedelja, 27. november 2022 ob 20:22:35 CET je Samuel Holland napisal(a):
> On 11/27/22 11:41, Andre Przywara wrote:
> > On Fri, 25 Nov 2022 17:46:48 -0600
> >
> > Samuel Holland <samuel@xxxxxxxxxxxx> wrote:
> >> D1 (aka D1-H), D1s (aka F133), R528, and T113 are a family of SoCs based
> >> on a single die, or at a pair of dies derived from the same design.
> >>
> >> D1 and D1s contain a single T-HEAD Xuantie C906 CPU, whereas R528 and
> >> T113 contain a pair of Cortex-A7's. D1 and R528 are the full version of
> >> the chip with a BGA package, whereas D1s and T113 are low-pin-count QFP
> >> variants.
> >>
> >> Because the original design supported both ARM and RISC-V CPUs, some
> >> peripherals are duplicated. In addition, all variants except D1s contain
> >> a HiFi 4 DSP with its own set of peripherals.
> >>
> >> The devicetrees are organized to minimize duplication:
> >> - Common perhiperals are described in sunxi-d1s-t113.dtsi
> >
> > So I compared all the reg and interrupts properties against the T113
> > manual, they match, as far as they are described there. The undocumented
> > rest matches what we already have in other SoCs.
> >
> > I noticed two things, though, mentioned inline below:
> >
> > [...]
> >
> >> + display_clocks: clock-controller@5000000 {
> >
> > The clocks and the two mixers are not children of a bus node anymore,
> > IIUC correctly this was to manage the SRAM control. Is that now handled
> > differently, or does the D1 generation not require this anymore?
>
> The D1 family uses the DSP SRAM for extra space during early boot --
> this applies even to D1s, where the DSP is fused off. Since the DE SRAM
> is not used for this purpose, the "SRAM C" aka "boot mode" bit in the
> SRAM controller is cleared by default, thus mapping the DE SRAM to the
> peripheral. So the DE works without touching the syscon registers.
>
> However, if I set the SRAM C bit, the DE stops working. So having the
> bus node would make some sense. But I do not know any address for the
> SRAM -- there is no "boot" address, and the "peripheral" address may not
> even be accessible to the CPU. So it is not possible to represent this
> with a mmio-sram node like the binding requires.
>
> So I suppose we should either change the binding to allow a child
> allwinner,sun50i-a64-sram-c node with no address (the driver should work
> fine with this); or leave out the bus, and people who go poking around
> in the syscon registers get to keep the pieces. :)
Just leave out the bus. I think this is just a leftover...
Acked-by: Jernej Skrabec <jernej.skrabec@xxxxxxxxx>
Best regards,
Jernej
>
> >> + compatible = "allwinner,sun20i-d1-de2-clk",
> >> + "allwinner,sun50i-h5-de2-
clk";
> >> + reg = <0x5000000 0x10000>;
> >> + clocks = <&ccu CLK_BUS_DE>, <&ccu CLK_DE>;
> >> + clock-names = "bus", "mod";
> >> + resets = <&ccu RST_BUS_DE>;
> >> + #clock-cells = <1>;
> >> + #reset-cells = <1>;
> >> + };
> >> +
> >> + mixer0: mixer@5100000 {
> >> + compatible = "allwinner,sun20i-d1-de2-
mixer-0";
> >> + reg = <0x5100000 0x100000>;
> >> + clocks = <&display_clocks CLK_BUS_MIXER0>,
> >> + <&display_clocks CLK_MIXER0>;
> >> + clock-names = "bus", "mod";
> >> + resets = <&display_clocks RST_MIXER0>;
> >> +
> >> + ports {
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> +
> >> + mixer0_out: port@1 {
> >> + reg = <1>;
> >> +
> >> +
mixer0_out_tcon_top_mixer0: endpoint {
> >> + remote-
endpoint = <&tcon_top_mixer0_in_mixer0>;
> >> + };
> >> + };
> >> + };
> >> + };
> >> +
> >> + mixer1: mixer@5200000 {
> >> + compatible = "allwinner,sun20i-d1-de2-
mixer-1";
> >> + reg = <0x5200000 0x100000>;
> >> + clocks = <&display_clocks CLK_BUS_MIXER1>,
> >> + <&display_clocks CLK_MIXER1>;
> >> + clock-names = "bus", "mod";
> >> + resets = <&display_clocks RST_MIXER1>;
> >> +
> >> + ports {
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> +
> >> + mixer1_out: port@1 {
> >> + reg = <1>;
> >> +
> >> +
mixer1_out_tcon_top_mixer1: endpoint {
> >> + remote-
endpoint = <&tcon_top_mixer1_in_mixer1>;
> >> + };
> >> + };
> >> + };
> >> + };
> >> +
> >> + dsi: dsi@5450000 {
> >> + compatible = "allwinner,sun20i-d1-mipi-dsi",
> >> + "allwinner,sun50i-a100-mipi-
dsi";
> >> + reg = <0x5450000 0x1000>;
> >> + interrupts = <SOC_PERIPHERAL_IRQ(92)
IRQ_TYPE_LEVEL_HIGH>;
> >> + clocks = <&ccu CLK_BUS_MIPI_DSI>,
> >> + <&tcon_top CLK_TCON_TOP_DSI>;
> >> + clock-names = "bus", "mod";
> >> + resets = <&ccu RST_BUS_MIPI_DSI>;
> >> + phys = <&dphy>;
> >> + phy-names = "dphy";
> >> + status = "disabled";
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> +
> >> + port {
> >> + dsi_in_tcon_lcd0: endpoint {
> >> + remote-endpoint =
<&tcon_lcd0_out_dsi>;
> >> + };
> >> + };
> >> + };
> >> +
> >> + dphy: phy@5451000 {
> >> + compatible = "allwinner,sun20i-d1-mipi-
dphy",
> >> + "allwinner,sun50i-a100-mipi-
dphy";
> >> + reg = <0x5451000 0x1000>;
> >> + interrupts = <SOC_PERIPHERAL_IRQ(92)
IRQ_TYPE_LEVEL_HIGH>;
> >
> > This is the same interrupt number as for the DSI controller above. Is
> > that correct, and can the drivers handle that?
>
> Yes, it is correct. Currently, neither driver uses the interrupt, so we
> will just need to keep the sharing in mind if/when that happens.
>
> Regards,
> Samuel
