On 4.12.2024 12:33 PM, Varadarajan Narayanan wrote:
> From: Nitheesh Sekar <quic_nsekar@xxxxxxxxxxx>
>
> Add Qualcomm PCIe UNIPHY 28LP driver support present
> in Qualcomm IPQ5332 SoC and the phy init sequence.
>
> Signed-off-by: Nitheesh Sekar <quic_nsekar@xxxxxxxxxxx>
> Signed-off-by: Varadarajan Narayanan <quic_varada@xxxxxxxxxxx>
> ---
[...]
> +struct qcom_uniphy_pcie_regs {
> + unsigned int offset;
> + unsigned int val;
u32
> +};
> +
> +struct qcom_uniphy_pcie_data {
> + int lanes;
> + /* 2nd lane offset */
> + int lane_offset;
'lanes', 'lane_offset' and '2nd lane' together imply one of:
- there can be more lines, all at an equal offset
- there can only ever be two lines
Please specify which one is the case
> + unsigned int phy_type;
> + const struct qcom_uniphy_pcie_regs *init_seq;
> + unsigned int init_seq_num;
> + unsigned int pipe_clk_rate;
> +};
> +
> +struct qcom_uniphy_pcie {
> + struct phy phy;
> + struct device *dev;
> + const struct qcom_uniphy_pcie_data *data;
> + struct clk_bulk_data *clks;
> + int num_clks;
> + struct reset_control *resets;
> + void __iomem *base;
> +};
> +
> +#define phy_to_dw_phy(x) container_of((x), struct qca_uni_pcie_phy, phy)
A space after #define, please
> +
> +static const struct qcom_uniphy_pcie_regs ipq5332_regs[] = {
> + {
> + .offset = PHY_CFG_PLLCFG,
> + .val = 0x30,
> + }, {
> + .offset = PHY_CFG_EIOS_DTCT_REG,
> + .val = 0x53ef,
> + }, {
> + .offset = PHY_CFG_GEN3_ALIGN_HOLDOFF_TIME,
> + .val = 0xCf,
mixed case hex.. please make it lowercase
> + },
> +};
> +
> +static const struct qcom_uniphy_pcie_data ipq5332_x1_data = {
> + .lanes = 1,
> + .phy_type = PHY_TYPE_PCIE_GEN3,
> + .init_seq = ipq5332_regs,
> + .init_seq_num = ARRAY_SIZE(ipq5332_regs),
> + .pipe_clk_rate = 250000000,
> +};
> +
> +static const struct qcom_uniphy_pcie_data ipq5332_x2_data = {
> + .lanes = 2,
> + .lane_offset = 0x800,
> + .phy_type = PHY_TYPE_PCIE_GEN3,
> + .init_seq = ipq5332_regs,
> + .init_seq_num = ARRAY_SIZE(ipq5332_regs),
> + .pipe_clk_rate = 250000000,
> +};
Are there going to be more UNIPHY-equipped SoCs?
> +
> +static void qcom_uniphy_pcie_init(struct qcom_uniphy_pcie *phy)
> +{
> + const struct qcom_uniphy_pcie_data *data = phy->data;
> + const struct qcom_uniphy_pcie_regs *init_seq;
> + void __iomem *base = phy->base;
> + int lane, i;
> +
> + for (lane = 0; lane != data->lanes; lane++) {
while effectively the same, < would be less eyebrow-raising
> + init_seq = data->init_seq;
> +
> + for (i = 0; i < data->init_seq_num; i++, init_seq++)
> + writel(init_seq->val, base + init_seq->offset);
writel(init_seq[i].val, ...)
> +
> + base += data->lane_offset;
> + }
> +}
> +
> +static int qcom_uniphy_pcie_power_off(struct phy *x)
> +{
> + struct qcom_uniphy_pcie *phy = phy_get_drvdata(x);
> +
> + clk_bulk_disable_unprepare(phy->num_clks, phy->clks);
> +
> + reset_control_assert(phy->resets);
This can fail, return it instead of zero
[...]
> +MODULE_LICENSE("Dual BSD/GPL");
I think this is too vague, there are many BSD variants
Konrad
