On 07/04/2022 01:36, Brad Larson wrote:
> Add Pensando common and Elba SoC specific device nodes
>
> Signed-off-by: Brad Larson <brad@xxxxxxxxxxx>
> ---
> Change from V3:
> - Changed to dual copyright (GPL-2.0+ OR MIT)
> - Minor changes from review input
Thank you for your patch. There is something to discuss/improve.
(...)
> +&i2c0 {
> + clock-frequency = <100000>;
> + status = "okay";
> + rtc@51 {
> + compatible = "nxp,pcf85263";
> + reg = <0x51>;
> + };
> +};
> +
> +&spi0 {
> + num-cs = <4>;
> + cs-gpios = <0>, <0>, <&porta 1 GPIO_ACTIVE_LOW>,
> + <&porta 7 GPIO_ACTIVE_LOW>;
> + status = "okay";
> + spi0_cs0@0 {
Generic node name needed matching the class of a devicxe.
> + compatible = "semtech,sx1301"; /* Enable spidev */
This comment is a bit odd... did you just use random compatible from
spidev instead of defining proper compatible?
> + #address-cells = <1>;
> + #size-cells = <1>;
Why address/size cells?
> + spi-max-frequency = <12000000>;
> + reg = <0>;
Please put reg just after compatible. It's the most common pattern.
> + };
> +
> + spi0_cs1@1 {
> + compatible = "semtech,sx1301";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + spi-max-frequency = <12000000>;
> + reg = <1>;
> + };
> +
(...)
> +
> + emmc: mmc@30440000 {
> + compatible = "pensando,elba-sd4hc", "cdns,sd4hc";
> + clocks = <&emmc_clk>;
> + interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
> + reg = <0x0 0x30440000 0x0 0x10000>,
> + <0x0 0x30480044 0x0 0x4>; /* byte-lane ctrl */
> + cdns,phy-input-delay-sd-highspeed = <0x4>;
> + cdns,phy-input-delay-legacy = <0x4>;
> + cdns,phy-input-delay-sd-uhs-sdr50 = <0x6>;
> + cdns,phy-input-delay-sd-uhs-ddr50 = <0x16>;
> + mmc-ddr-1_8v;
> + status = "disabled";
> + };
> +
> + mssoc: mssoc@307c0000 {
Generic node name.
> + compatible = "syscon", "simple-mfd";
This does not look correct. Syscon is okay, but why do you need
simple-mfd (there are no children here)?
> + reg = <0x0 0x307c0000 0x0 0x3000>;
> + };
> + };
> +};
Best regards,
Krzysztof
