Re: [PATCH 2/2] scsi: ufs-unisoc: Add support for Unisoc UFS host controller

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
>



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux