[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <30b8ee6e-d890-a76a-96fd-080042af1d7a@189.cn>
Date: Mon, 28 Mar 2022 09:52:30 +0800
From: Sui Jingfeng <15330273260@....cn>
To: Jiaxun Yang <jiaxun.yang@...goat.com>,
Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>,
Roland Scheidegger <sroland@...are.com>,
Zack Rusin <zackr@...are.com>,
Christian Gmeiner <christian.gmeiner@...il.com>,
David Airlie <airlied@...ux.ie>,
Daniel Vetter <daniel@...ll.ch>,
Rob Herring <robh+dt@...nel.org>,
Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
Dan Carpenter <dan.carpenter@...cle.com>,
Krzysztof Kozlowski <krzk@...nel.org>,
Andrey Zhizhikin <andrey.zhizhikin@...ca-geosystems.com>,
Sam Ravnborg <sam@...nborg.org>,
"David S . Miller" <davem@...emloft.net>,
Lucas Stach <l.stach@...gutronix.de>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Ilia Mirkin <imirkin@...m.mit.edu>,
Qing Zhang <zhangqing@...ngson.cn>,
suijingfeng <suijingfeng@...ngson.cn>
Cc: linux-mips@...r.kernel.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH v12 3/6] dt-bindings: display: Add Loongson display
controller
On 2022/3/27 20:54, Jiaxun Yang wrote:
>
>
> 在 2022/3/27 12:38, Sui Jingfeng 写道:
>> Add DT bindings and simple usages for Loongson display controller
>> found in LS7A1000 bridges chip and LS2k1000 SoC.
>>
>> Signed-off-by: Sui Jingfeng <15330273260@....cn>
> [...]
>> +
>> + - |
>> + #include <dt-bindings/interrupt-controller/irq.h>
>> + bus {
>> +
>> + #address-cells = <3>;
>> + #size-cells = <2>;
>> + #interrupt-cells = <2>;
>> +
>> + display-controller@6,1 {
>> + compatible = "loongson,ls7a1000-dc";
>> + reg = <0x3100 0x0 0x0 0x0 0x0>;
>> + interrupts = <28 IRQ_TYPE_LEVEL_HIGH>;
>> +
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + i2c@6 {
>> + compatible = "loongson,gpio-i2c";
>> + reg = <0x00001650 0x00000020>;
> Hi Jingfeng,
>
> Thanks for your patch.
>
> Just curious about what is this "reg" for?
Hi, Jiaxun
Thanks for you take valuable time to review my patch.
Without it make dt_binding_check generate warnings:
Documentation/devicetree/bindings/display/loongson/loongson,display-controller.example.dts:65.23-73.19:
Warning (unit_address_vs_reg):
/example-1/bus/display-controller@6,1/i2c@6: node has a unit name, but
no reg or ranges property
reg are the control register offset and size of the dedicate GPIO, they
are not get used by the driver currently,
put in there just for silence the warning .
>> + loongson,nr = <6>;
> Why nr start from 6?
Bus number greater than 6 is safe because ls7a1000 bridge have 6 hardware I2C controller integrated. but
the driver for it is not upstream yet. To avoid potential conflict with the bus number of the hardware I2C driver.
In the future, if someone contribute the hardware I2C driver to upstream,
you don't need change it. Let me give you an example to show what it will be:
aliases {
i2c0 = &i2c0;
i2c1 = &i2c1;
i2c2 = &i2c2;
i2c3 = &i2c3;
i2c4 = &i2c4;
i2c5 = &i2c5;
i2c6 = &i2c6;
i2c7 = &i2c7;
};
i2c0: i2c@...90000 {
compatible = "loongson,ls-i2c";
reg = <0x10090000 0x8>;
interrupts = <73>;
interrupt-parent = <&platic>;
#address-cells = <1>;
#size-cells = <0>;
};
i2c1: i2c@...90100 {
compatible = "loongson,ls-i2c";
reg = <0x10090100 0x8>;
interrupts = <73>;
interrupt-parent = <&platic>;
#address-cells = <1>;
#size-cells = <0>;
};
i2c2: i2c@...90200 {
compatible = "loongson,ls-i2c";
reg = <0x10090200 0x8>;
interrupts = <73>;
interrupt-parent = <&platic>;
#address-cells = <1>;
#size-cells = <0>;
};
i2c3: i2c@...90300 {
compatible = "loongson,ls-i2c";
reg = <0x10090300 0x8>;
interrupts = <73>;
interrupt-parent = <&platic>;
#address-cells = <1>;
#size-cells = <0>;
};
i2c4: i2c@...90400 {
compatible = "loongson,ls-i2c";
reg = <0x10090400 0x8>;
interrupts = <73>;
interrupt-parent = <&platic>;
#address-cells = <1>;
#size-cells = <0>;
};
i2c5: i2c@...90500 {
compatible = "loongson,ls-i2c";
reg = <0x10090500 0x8>;
interrupts = <73>;
interrupt-parent = <&platic>;
#address-cells = <1>;
#size-cells = <0>;
};
> The approach you are handling I2C seems to be wired..
>
It is not wired, you can change it to 0 or 1 it you like currently,
you can even remove loongson,nr = <6> and loongson,nr = <7>,
then the i2c core driver will automatically allocate one for you.
It is very flexible actually.
> Actually you can reference how network subsystem is handling
> MDIO controller built-in into Ethernet controller [1] in this case. It is
> basically the same problem.
>
> [1]:
> https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>
> Thanks.
> - Jiaxun
>
>> + loongson,sda = <0>;
>> + loongson,scl = <1>;
>> + loongson,udelay = <5>;
>> + loongson,timeout = <2200>;
>> + };
>> +
>> + i2c@7 {
>> + compatible = "loongson,gpio-i2c";
>> + reg = <0x00001650 0x00000020>;
>> + loongson,nr = <7>;
>> + loongson,sda = <2>;
>> + loongson,scl = <3>;
>> + loongson,udelay = <5>;
>> + loongson,timeout = <2200>;
>> + };
>> +
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + port@0 {
>> + reg = <0>;
>> + endpoint {
>> + remote-endpoint = <&vga_encoder_in>;
>> + };
>> + };
>> +
>> + port@1 {
>> + reg = <1>;
>> + endpoint {
>> + remote-endpoint = <&dvi_encoder_in>;
>> + };
>> + };
>> + };
>> + };
>> + };
>> +
>> + - |
>> + #include <dt-bindings/interrupt-controller/irq.h>
>> + bus {
>> +
>> + #address-cells = <3>;
>> + #size-cells = <2>;
>> + #interrupt-cells = <2>;
>> +
>> + display-controller@6,0 {
>> + compatible = "loongson,ls2k1000-dc";
>> + reg = <0x3100 0x0 0x0 0x0 0x0>;
>> + interrupts = <28 IRQ_TYPE_LEVEL_HIGH>;
>> +
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + port@0 {
>> + reg = <0>;
>> + endpoint {
>> + remote-endpoint = <&panel_in>;
>> + };
>> + };
>> +
>> + port@1 {
>> + reg = <1>;
>> + endpoint {
>> + remote-endpoint = <&hdmi_encoder_in>;
>> + };
>> + };
>> + };
>> + };
>> + };
>> +...
>
Powered by blists - more mailing lists