| 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
| ||
|
Message-ID: <1b19af6f-645a-4913-b9db-49b1f3ad54f9@linaro.org>
Date: Tue, 12 Dec 2023 21:25:01 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Ninad Palsule <ninad@...ux.ibm.com>, robh+dt@...nel.org,
krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
joel@....id.au, andrew@...econstruct.com.au, peterhuewe@....de,
jarkko@...nel.org, jgg@...pe.ca, keescook@...omium.org,
tony.luck@...el.com, gpiccoli@...lia.com,
johannes.holland@...ineon.com, linux@...ck-us.net,
broonie@...nel.org
Cc: patrick.rudolph@...ements.com, vincent@...emblay.dev,
peteryin.openbmc@...il.com, lakshmiy@...ibm.com,
bhelgaas@...gle.com, naresh.solanki@...ements.com,
alexander.stein@...tq-group.com, festevam@...x.de,
devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-aspeed@...ts.ozlabs.org, linux-kernel@...r.kernel.org,
linux-integrity@...r.kernel.org, linux-hardening@...r.kernel.org,
geissonator@...oo.com
Subject: Re: [PATCH v1 6/8] ARM: dts: aspeed: System1: GPIO, Fan ctrl, Led
On 12/12/2023 17:40, Ninad Palsule wrote:
> This commit adds following devices to the device tree.
> - GPIO pin assignements, GPIO expansion devices
> - LED brinker devices
> - Fan controllers
>
> Tested:
> This board is tested using the simics simulator.
>
> Signed-off-by: Ninad Palsule <ninad@...ux.ibm.com>
> ---
> .../dts/aspeed/aspeed-bmc-ibm-system1.dts | 547 +++++++++++++++++-
Squash it.
> 1 file changed, 542 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-system1.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-system1.dts
> index b8e7e52d4600..75562aa63701 100644
> --- a/arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-system1.dts
> +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-ibm-system1.dts
> @@ -114,6 +114,99 @@ vga_memory: region@...00000 {
> };
> };
>
> + leds {
> + compatible = "gpio-leds";
> +
> + bmc-ready {
It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).
> + gpios = <&gpio0 ASPEED_GPIO(L, 7) GPIO_ACTIVE_HIGH>;
> + };
> +
> + bmc-hb {
None of these were tested.
> /*A0-A7*/ "","","","","","","","",
> - /*B0-B7*/ "","","","","","","","",
> + /*B0-B7*/ "","","","","bmc-tpm-reset","","","",
Really? You just added these lines. There is no point in adding a new
line and immediately changing it.
This points how your split is artificial and not helpful.
...
> &i2c2 {
> @@ -486,6 +744,20 @@ regulator@43 {
> &i2c6 {
> status = "okay";
>
> + fan-controller@52 {
> + compatible = "maxim,max31785a";
> + reg = <0x52>;
> + #address-cells = <1>;
> + #size-cells = <0>;
Why do you need cells?
> + };
> +
> + fan-controller@54 {
> + compatible = "maxim,max31785a";
> + reg = <0x54>;
> + #address-cells = <1>;
> + #size-cells = <0>;
Why do you need cells?
> + };
> +
> i2c-mux@70 {
> compatible = "nxp,pca9548";
> reg = <0x70>;
> @@ -522,6 +794,48 @@ i2c6mux0chn4: i2c@4 {
> #address-cells = <1>;
> #size-cells = <0>;
> reg = <4>;
> +
> + led-controller@60 {
> + compatible = "nxp,pca9551";
> + reg = <0x60>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + gpio-controller;
> + #gpio-cells = <2>;
> +
> + led@0 {
> + label = "enclosure-id-led";
> + reg = <0>;
> + retain-state-shutdown;
> + default-state = "keep";
> + type = <PCA955X_TYPE_LED>;
> + };
> +
> + led@1 {
> + label = "attention-led";
> + reg = <1>;
> + retain-state-shutdown;
> + default-state = "keep";
> + type = <PCA955X_TYPE_LED>;
> + };
> +
> + led@2 {
> + label = "enclosure-fault-rollup-led";
> + reg = <2>;
> + retain-state-shutdown;
> + default-state = "keep";
> + type = <PCA955X_TYPE_LED>;
> + };
> +
> + led@3 {
> + label = "power-on-led";
> + reg = <3>;
> + retain-state-shutdown;
> + default-state = "keep";
> + type = <PCA955X_TYPE_LED>;
> + };
> + };
> };
>
> i2c6mux0chn5: i2c@5 {
> @@ -542,6 +856,44 @@ i2c6mux0chn7: i2c@7 {
> reg = <7>;
> };
> };
> +
> + pca3: pca9539@74 {
Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> + compatible = "nxp,pca9539";
> + reg = <0x74>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + gpio-controller;
> + #gpio-cells = <2>;
> + };
> +
> + pca4: pca9539@77 {
Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> + compatible = "nxp,pca9539";
> + reg = <0x77>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + gpio-controller;
> + #gpio-cells = <2>;
> +
> + gpio-line-names =
> + "PE_NVMED0_EXP_PRSNT_N",
> + "PE_NVMED1_EXP_PRSNT_N",
> + "PE_NVMED2_EXP_PRSNT_N",
> + "PE_NVMED3_EXP_PRSNT_N",
> + "LED_FAULT_NVMED0",
> + "LED_FAULT_NVMED1",
> + "LED_FAULT_NVMED2",
> + "LED_FAULT_NVMED3",
> + "FAN0_PRESENCE_R_N",
> + "FAN1_PRESENCE_R_N",
> + "FAN2_PRESENCE_R_N",
> + "FAN3_PRESENCE_R_N",
> + "FAN4_PRESENCE_R_N",
> + "FAN5_PRESENCE_N",
> + "FAN6_PRESENCE_N",
> + "";
> + };
> };
>
> &i2c7 {
> @@ -809,6 +1161,191 @@ regulator@41 {
> compatible = "infineon,ir38263";
> reg = <0x41>;
> };
> +
> + led-controller@61 {
> + compatible = "nxp,pca9552";
> + reg = <0x61>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
...
> + led@15 {
> + label = "pe-cp-drv3-perst";
> + reg = <15>;
> + retain-state-shutdown;
> + default-state = "keep";
> + type = <PCA955X_TYPE_LED>;
> + };
> + };
> +
> + pca1: pca9539@75 {
Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
Best regards,
Krzysztof
Powered by blists - more mailing lists