linux-kernel - Re: [PATCH v3 1/5] scsi: ufs: add Hisilicon ufs driver code

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

Message-ID: <CAK8P3a0vV94g=iNLfHXXgR-XFHaaK-mHoQQ45FDYG+FVQhix6Q@mail.gmail.com>
Date:   Thu, 7 Sep 2017 00:46:36 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Li Wei <liwei213@...wei.com>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Wei Xu <xuwei5@...ilicon.com>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will.deacon@....com>,
        Vinayak Holikatti <vinholikatti@...il.com>,
        "James E.J. Bottomley" <jejb@...ux.vnet.ibm.com>,
        "Martin K. Petersen" <martin.petersen@...cle.com>,
        Kevin Hilman <khilman@...libre.com>,
        Gregory CLEMENT <gregory.clement@...e-electrons.com>,
        Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>,
        Masahiro Yamada <yamada.masahiro@...ionext.com>,
        Riku Voipio <riku.voipio@...aro.org>,
        Thierry Reding <treding@...dia.com>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        Eric Anholt <eric@...olt.net>, devicetree@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        linux-scsi <linux-scsi@...r.kernel.org>,
        Guodong Xu <guodong.xu@...aro.org>, fengbaopeng@...ilicon.com
Subject: Re: [PATCH v3 1/5] scsi: ufs: add Hisilicon ufs driver code

On Tue, Aug 29, 2017 at 10:41 AM, Li Wei <liwei213@...wei.com> wrote:
itel(host, UFS_ARESET, PERRSTDIS3_OFFSET);
> +
> +       /* disable lp_reset_n */
> +       ufs_sys_ctrl_set_bits(host, BIT_SYSCTRL_LP_RESET_N, RESET_CTRL_EN);
> +       mdelay(1);
> +
> +       if (gpio_is_valid(host->reset_gpio))
> +               gpio_direction_output(host->reset_gpio, 1);
> +
> +       ufs_sys_ctrl_writel(host, MASK_UFS_DEVICE_RESET | BIT_UFS_DEVICE_RESET,
> +               UFS_DEVICE_RESET_CTRL);
> +
> +       mdelay(20);

Could those mdelay() be turned into msleep() functions?

> +static int ufs_hisi_get_resource(struct ufs_hisi_host *host)
> +{
> +       struct resource *mem_res;
> +       struct device_node *np = NULL;
> +       struct device *dev = host->hba->dev;
> +       struct platform_device *pdev = to_platform_device(dev);
> +
> +       /* get resource of ufs sys ctrl */
> +       mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> +       host->ufs_sys_ctrl = devm_ioremap_resource(dev, mem_res);
> +       if (IS_ERR(host->ufs_sys_ctrl))
> +               return PTR_ERR(host->ufs_sys_ctrl);
> +
> +       np = of_find_compatible_node(NULL, NULL, "hisilicon,hi3660-crgctrl");

It's generally not a good idea to look up one device by its "compatible"
string. What is the "crgctrl"? Does it have a proper DT binding?
Maybe there should be a driver for it, or you could make it a "syscon"
device and look it up by phandle instead.

> diff --git a/drivers/scsi/ufs/ufs-hisi.h b/drivers/scsi/ufs/ufs-hisi.h
> new file mode 100644
> index 000000000000..52430a2aca90
> --- /dev/null
> +++ b/drivers/scsi/ufs/ufs-hisi.h


If the header is only used in one file, you don't need it, just move
the definitions
into the other file.

      Arnd