linux-kernel - Re: [PATCH v3 10/12] phy: exynos: Add PCIe PHY support for FSD SoC

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] [day] [month] [year] [list]

Message-ID: <aLWNhv0eLj7LRrvM@vaman>
Date: Mon, 1 Sep 2025 17:41:50 +0530
From: Vinod Koul <vkoul@...nel.org>
To: Shradha Todi <shradha.t@...sung.com>
Cc: linux-pci@...r.kernel.org, devicetree@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org,
	linux-samsung-soc@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-phy@...ts.infradead.org, mani@...nel.org,
	lpieralisi@...nel.org, kwilczynski@...nel.org, robh@...nel.org,
	bhelgaas@...gle.com, jingoohan1@...il.com, krzk+dt@...nel.org,
	conor+dt@...nel.org, alim.akhtar@...sung.com, kishon@...nel.org,
	arnd@...db.de, m.szyprowski@...sung.com, jh80.chung@...sung.com,
	pankaj.dubey@...sung.com
Subject: Re: [PATCH v3 10/12] phy: exynos: Add PCIe PHY support for FSD SoC

On 11-08-25, 21:16, Shradha Todi wrote:
> Add PCIe PHY support for Tesla FSD SoC.

Can you pls add a bit more description of what you are adding, helps to
understand the change

> +/* FSD: PCIe PCS registers */
> +#define FSD_PCIE_PCS_BRF_0		0x0004
> +#define FSD_PCIE_PCS_BRF_1		0x0804
> +#define FSD_PCIE_PCS_CLK		0x0180
> +
> +/* FSD: PCIe SYSREG registers */
> +#define FSD_PCIE_SYSREG_PHY_0_CON			0x042c
> +#define FSD_PCIE_SYSREG_PHY_0_CON_MASK			0x03ff
> +#define FSD_PCIE_SYSREG_PHY_0_REF_SEL			(0x2 << 0)

Use GENMASK() please here and elsewhere

> +static int fsd_pcie_phy0_reset(struct phy *phy)
> +{
> +	struct exynos_pcie_phy *phy_ctrl = phy_get_drvdata(phy);
> +
> +	writel(0x1, phy_ctrl->pcs_base + FSD_PCIE_PCS_CLK);
> +
> +	regmap_update_bits(phy_ctrl->fsysreg, FSD_PCIE_SYSREG_PHY_0_CON,
> +			FSD_PCIE_SYSREG_PHY_0_CON_MASK, 0x0);
> +	regmap_update_bits(phy_ctrl->fsysreg, FSD_PCIE_SYSREG_PHY_0_CON,
> +		FSD_PCIE_SYSREG_PHY_0_AUX_EN, FSD_PCIE_SYSREG_PHY_0_AUX_EN);
> +	regmap_update_bits(phy_ctrl->fsysreg, FSD_PCIE_SYSREG_PHY_0_CON,
> +		FSD_PCIE_SYSREG_PHY_0_REF_SEL_MASK, FSD_PCIE_SYSREG_PHY_0_REF_SEL);
> +	regmap_update_bits(phy_ctrl->fsysreg, FSD_PCIE_SYSREG_PHY_0_CON,
> +		FSD_PCIE_SYSREG_PHY_0_INIT_RSTN, FSD_PCIE_SYSREG_PHY_0_INIT_RSTN);

pls conform to coding style for these

> +
> +	return 0;

why return a value when this wont ever return anything else than 0?

> +
> +	writel(0x2, pbase + FSD_PCIE_PHY_CMN_RESET);
> +
> +	writel(0x00, phy_ctrl->pcs_base + FSD_PCIE_PCS_BRF_0);
> +	writel(0x00, phy_ctrl->pcs_base + FSD_PCIE_PCS_BRF_1);
> +	writel(0x00, pbase + FSD_PCIE_PHY_AGG_BIF_RESET);
> +	writel(0x00, pbase + FSD_PCIE_PHY_AGG_BIF_CLOCK);
> +
> +	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG07B_LN_N, 0x20);
> +	fsd_pcie_phy_writel(phy_ctrl, FSD_PCIE_PHY_TRSV_REG052_LN_N, 0x00);
> +	writel(0xaa, pbase + FSD_PCIE_PHY_TRSV_CMN_REG01E);
> +	writel(0x28, pbase + FSD_PCIE_PHY_TRSV_CMN_REG02D);
> +	writel(0x28, pbase + FSD_PCIE_PHY_TRSV_CMN_REG031);
> +	writel(0x21, pbase + FSD_PCIE_PHY_TRSV_CMN_REG036);
> +	writel(0x12, pbase + FSD_PCIE_PHY_TRSV_CMN_REG05F);
> +	writel(0x23, pbase + FSD_PCIE_PHY_TRSV_CMN_REG060);
> +	writel(0x0, pbase + FSD_PCIE_PHY_TRSV_CMN_REG061);
> +	writel(0x0, pbase + FSD_PCIE_PHY_TRSV_CMN_REG062);
> +	writel(0x15, pbase + FSD_PCIE_PHY_TRSV_CMN_REG03);

Magic numbers?
-- 
~Vinod