| 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
| ||
|
Date: Mon, 21 Nov 2022 13:11:33 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Shradha Todi <shradha.t@...sung.com>, bhelgaas@...gle.com,
robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
kishon@...com, vkoul@...nel.org, lpieralisi@...nel.org,
kw@...ux.com, mani@...nel.org, arnd@...db.de,
gregkh@...uxfoundation.org, alim.akhtar@...sung.com,
ajaykumar.rs@...sung.com, rcsekar@...sung.com,
sriranjani.p@...sung.com, bharat.uppal@...sung.com,
s.prashar@...sung.com, aswani.reddy@...sung.com,
pankaj.dubey@...sung.com, p.rajanbabu@...sung.com,
niyas.ahmed@...sung.com, chanho61.park@...sung.com
Cc: linux-pci@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-phy@...ts.infradead.org
Subject: Re: [PATCH 5/6] arm64: dts: fsd: Add PCIe support for Tesla FSD SoC
On 21/11/2022 11:52, Shradha Todi wrote:
> Add the support for PCIe controller driver and phy driver
> for Tesla FSD. It includes support for both RC and EP.
>
> Signed-off-by: Niyas Ahmed S T <niyas.ahmed@...sung.com>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@...sung.com>
> Signed-off-by: Shradha Todi <shradha.t@...sung.com>
> ---
> arch/arm64/boot/dts/tesla/fsd-evb.dts | 48 ++++++
> arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi | 65 ++++++++
> arch/arm64/boot/dts/tesla/fsd.dtsi | 171 +++++++++++++++++++++
> 3 files changed, 284 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/tesla/fsd-evb.dts b/arch/arm64/boot/dts/tesla/fsd-evb.dts
> index 1db6ddf03f01..cda72b0f76f8 100644
> --- a/arch/arm64/boot/dts/tesla/fsd-evb.dts
> +++ b/arch/arm64/boot/dts/tesla/fsd-evb.dts
> @@ -41,3 +41,51 @@
> &ufs {
> status = "okay";
> };
> +
> +&pcie_phy0 {
> + status = "disabled";
It's a double disable, isn't it?
> +};
> +
> +&pcie_phy1 {
> + status = "disabled";
> +};
> +
> +&pcie4_rc {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pcie1_clkreq>, <&pcie1_wake>, <&pcie1_preset>,
> + <&pcie0_slot1>;
> + status = "disabled";
???
> +};
> +
> +&pcie4_ep {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pcie1_clkreq>, <&pcie1_wake>, <&pcie1_preset>,
> + <&pcie0_slot1>;
> + status = "disabled";
> +};
> +
> +&pcie0_rc {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake0>, <&pcie0_preset0>,
> + <&pcie0_slot0>;
> + status = "disabled";
> +};
> +
> +&pcie0_ep {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake0>, <&pcie0_preset0>,
> + <&pcie0_slot0>;
> + status = "disabled";
> +};
> +
> +&pcie1_rc {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake1>, <&pcie0_preset0>;
> + status = "disabled";
> +};
> +
> +&pcie1_ep {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pcie0_clkreq>, <&pcie0_wake1>, <&pcie0_preset0>;
> + status = "disabled";
Ordering is broken. All overrides/extends are ordered by label name.
> +};
> diff --git a/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi b/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi
> index d0abb9aa0e9e..edae62dfa987 100644
> --- a/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi
> +++ b/arch/arm64/boot/dts/tesla/fsd-pinctrl.dtsi
> @@ -64,6 +64,27 @@
> samsung,pin-pud = <FSD_PIN_PULL_NONE>;
> samsung,pin-drv = <FSD_PIN_DRV_LV2>;
> };
> +
> + pcie1_clkreq: pcie1-clkreq {
Does not look like you tested the DTS against bindings. Please run `make
dtbs_check` (see Documentation/devicetree/bindings/writing-schema.rst
for instructions).
(...)
>
> &pinctrl_pmu {
> diff --git a/arch/arm64/boot/dts/tesla/fsd.dtsi b/arch/arm64/boot/dts/tesla/fsd.dtsi
> index f35bc5a288c2..2177f6964553 100644
> --- a/arch/arm64/boot/dts/tesla/fsd.dtsi
> +++ b/arch/arm64/boot/dts/tesla/fsd.dtsi
> @@ -32,6 +32,14 @@
> spi0 = &spi_0;
> spi1 = &spi_1;
> spi2 = &spi_2;
> + pciephy0 = &pcie_phy0;
> + pciephy1 = &pcie_phy1;
> + pcierc0 = &pcie0_rc;
> + pcieep0 = &pcie0_ep;
> + pcierc1 = &pcie1_rc;
> + pcieep1 = &pcie1_ep;
> + pcierc2 = &pcie4_rc;
> + pcieep2 = &pcie4_ep;
Since these are disabled, aliases do not belong to DTSI, but to board.
Also, explain why do you need them.
> };
>
> cpus {
> @@ -860,6 +868,169 @@
> clocks = <&clock_fsys0 UFS0_MPHY_REFCLK_IXTAL26>;
> clock-names = "ref_clk";
> };
> +
> + pcie_phy0: pcie-phy@...80000 {
> + compatible = "tesla,fsd-pcie-phy";
> + #phy-cells = <0>;
> + reg = <0x0 0x15080000 0x0 0x2000>,
> + <0x0 0x150A0000 0x0 0x1000>;
> + reg-names = "phy", "pcs";
> + samsung,pmureg-phandle = <&pmu_system_controller>;
> + tesla,pcie-sysreg = <&sysreg_fsys0>;
> + phy-mode = <0>;
> + status = "disabled";
> + };
> +
> + pcie_phy1: pcie-phy@...80000 {
> + compatible = "tesla,fsd-pcie-phy";
> + #phy-cells = <0>;
> + reg = <0x0 0x16880000 0x0 0x2000>,
> + <0x0 0x16860000 0x0 0x1000>;
> + reg-names = "phy", "pcs";
> + samsung,pmureg-phandle = <&pmu_system_controller>;
> + tesla,pcie-sysreg = <&sysreg_fsys1>;
> + phy-mode = <0>;
> + status = "disabled";
> + };
> +
> + pcie4_rc: pcie@...00000 {
Not ordered. Keep nodes sorted by unit address, at least within the
group of devices you add.
> + compatible = "tesla,fsd-pcie";
reg is second property. reg-names and ranges follow.
> + clocks = <&clock_fsys0 PCIE_SUBCTRL_INST0_AUX_CLK_SOC>,
> + <&clock_fsys0 PCIE_SUBCTRL_INST0_DBI_ACLK_SOC>,
> + <&clock_fsys0 PCIE_SUBCTRL_INST0_MSTR_ACLK_SOC>,
> + <&clock_fsys0 PCIE_SUBCTRL_INST0_SLV_ACLK_SOC>;
> + clock-names = "aux_clk", "dbi_clk", "mstr_clk", "slv_clk";
> + #address-cells = <3>;
> + #size-cells = <2>;
Best regards,
Krzysztof
Powered by blists - more mailing lists