lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
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

Powered by Openwall GNU/*/Linux Powered by OpenVZ