Hi Krzysztof,
Thank you for your review!
On Thu, Nov 10, 2022 at 10:31 PM Krzysztof Kozlowski
<krzysztof.kozlowski@xxxxxxxxxx> wrote:
>
> On 10/11/2022 14:36, Zhe Wang wrote:
> > From: Zhe Wang <zhe.wang1@xxxxxxxxxx>
> >
> > Add driver code for Unisoc ufs host controller, along with ufs
> > initialization.
>
> (...)
>
> > +
> > +static struct platform_driver ufs_sprd_pltform = {
> > + .probe = ufs_sprd_probe,
> > + .remove = ufs_sprd_remove,
> > + .shutdown = ufshcd_pltfrm_shutdown,
> > + .driver = {
> > + .name = "ufshcd-sprd",
> > + .pm = &ufs_sprd_pm_ops,
> > + .of_match_table = of_match_ptr(ufs_sprd_of_match),
>
> Drop of_match_ptr
>
I'll drop this.
> > + },
> > +};
> > +module_platform_driver(ufs_sprd_pltform);
> > +
> > +MODULE_AUTHOR("Zhe Wang <zhe.wang1@xxxxxxxxxx>");
> > +MODULE_DESCRIPTION("Unisoc UFS Host Driver");
> > +MODULE_LICENSE("GPL v2");
> > diff --git a/drivers/ufs/host/ufs-sprd.h b/drivers/ufs/host/ufs-sprd.h
> > new file mode 100644
> > index 000000000000..215e7483d1e8
> > --- /dev/null
> > +++ b/drivers/ufs/host/ufs-sprd.h
> > @@ -0,0 +1,125 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * UNISOC UFS Host Controller driver
> > + *
> > + * Copyright (C) 2022 Unisoc, Inc.
> > + * Author: Zhe Wang <zhe.wang1@xxxxxxxxxx>
> > + */
> > +
> > +#ifndef _UFS_SPRD_H_
> > +#define _UFS_SPRD_H_
> > +
> > +#define APB_UFSDEV_REG 0xCE8
> > +#define APB_UFSDEV_REFCLK_EN 0x2
> > +#define APB_USB31PLL_CTRL 0xCFC
> > +#define APB_USB31PLLV_REF2MPHY 0x1
> > +
> > +#define SPRD_SIP_SVC_STORAGE_UFS_CRYPTO_ENABLE \
> > + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \
> > + ARM_SMCCC_SMC_32, \
> > + ARM_SMCCC_OWNER_SIP, \
> > + 0x0301)
> > +
> > +enum SPRD_UFS_CLK_INDEX {
> > + SPRD_UFS_HCLK,
> > + SPRD_UFS_HCLK_SOURCE,
> > +
> > + SPRD_UFS_CLK_MAX
> > +};
> > +
> > +enum SPRD_UFS_RST_INDEX {
> > + SPRD_UFSHCI_SOFT_RST,
> > + SPRD_UFS_DEV_RST,
> > +
> > + SPRD_UFS_RST_MAX
> > +};
> > +
> > +enum SPRD_UFS_SYSCON_INDEX {
> > + SPRD_UFS_ANLG_REG,
> > + SPRD_UFS_AON_APB,
> > +
> > + SPRD_UFS_SYSCON_MAX
> > +};
> > +
> > +enum SPRD_UFS_VREG_INDEX {
> > + SPRD_UFS_VDD_MPHY,
> > +
> > + SPRD_UFS_VREG_MAX
> > +};
> > +
> > +struct ufs_sprd_clk {
> > + const char *name;
> > + struct clk *clk;
> > +};
> > +
> > +struct ufs_sprd_rst {
> > + const char *name;
> > + struct reset_control *rc;
> > +};
> > +
> > +struct ufs_sprd_syscon {
> > + const char *name;
> > + struct regmap *regmap;
> > +};
> > +
> > +struct ufs_sprd_vreg {
> > + const char *name;
> > + struct regulator *vreg;
> > +};
> > +
> > +struct ufs_sprd_priv {
> > + struct ufs_sprd_clk clki[SPRD_UFS_CLK_MAX];
> > + struct ufs_sprd_rst rci[SPRD_UFS_RST_MAX];
> > + struct ufs_sprd_syscon sysci[SPRD_UFS_SYSCON_MAX];
> > + struct ufs_sprd_vreg vregi[SPRD_UFS_VREG_MAX];
> > + const struct ufs_hba_variant_ops ufs_hba_sprd_vops;
> > +};
> > +
> > +struct ufs_sprd_host {
> > + struct ufs_hba *hba;
> > + struct ufs_sprd_priv *priv;
> > + void __iomem *ufs_dbg_mmio;
> > +
> > + enum ufs_unipro_ver unipro_ver;
> > +};
> > +
> > +static inline struct ufs_sprd_priv *ufs_sprd_get_priv_data(struct ufs_hba *hba)
> > +{
> > + struct ufs_sprd_host *host = ufshcd_get_variant(hba);
> > +
> > + WARN_ON(!host->priv);
> > + return host->priv;
> > +}
> > +
> > +static inline void ufs_sprd_regmap_update(struct ufs_sprd_priv *priv, unsigned int index,
> > + unsigned int reg, unsigned int bits, unsigned int val)
> > +{
> > + regmap_update_bits(priv->sysci[index].regmap, reg, bits, val);
> > +}
> > +
> > +static inline void ufs_sprd_regmap_read(struct ufs_sprd_priv *priv, unsigned int index,
> > + unsigned int reg, unsigned int *val)
> > +{
> > + regmap_read(priv->sysci[index].regmap, reg, val);
> > +}
> > +
> > +static inline void ufs_sprd_get_unipro_ver(struct ufs_hba *hba)
> > +{
> > + struct ufs_sprd_host *host = ufshcd_get_variant(hba);
> > +
> > + if (ufshcd_dme_get(hba, UIC_ARG_MIB(PA_LOCALVERINFO), &host->unipro_ver))
> > + host->unipro_ver = 0;
> > +}
> > +
> > +static inline void ufs_sprd_ctrl_uic_compl(struct ufs_hba *hba, bool enable)
> > +{
> > + u32 set = ufshcd_readl(hba, REG_INTERRUPT_ENABLE);
> > +
> > + if (enable == true)
> > + set |= UIC_COMMAND_COMPL;
> > + else
> > + set &= ~UIC_COMMAND_COMPL;
> > + ufshcd_writel(hba, set, REG_INTERRUPT_ENABLE);
> > +}
>
> Drop functions from headers. These are not stubs and your task is not to
> micro-optimize code.
>
Will move these functions to c file.
Best regards,
Zhe Wang
> > +
> > +#endif /* _UFS_SPRD_H_ */
>
> Best regards,
> Krzysztof
>
