linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] scsi: ufs: Add driver for TI wrapper for Cadence UFS IP
@ 2019-10-10  8:33 Vignesh Raghavendra
  2019-10-10  8:33 ` [PATCH v2 1/2] dt-bindings: ufs: ti,j721e-ufs.yaml: Add binding for TI UFS wrapper Vignesh Raghavendra
  2019-10-10  8:33 ` [PATCH v2 2/2] scsi: ufs: Add driver for TI wrapper for Cadence UFS IP Vignesh Raghavendra
  0 siblings, 2 replies; 6+ messages in thread
From: Vignesh Raghavendra @ 2019-10-10  8:33 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, jejb, Martin K Petersen
  Cc: Alim Akhtar, Avri Altman, Pedro Sousa, Janek Kotas, devicetree,
	linux-kernel, linux-scsi, Vignesh Raghavendra, nsekhar

This series add DT bindings and driver for TI wrapper for Cadence UFS
IP that is present on TI's J721e SoC

Vignesh Raghavendra (2):
  dt-bindings: ufs: ti,j721e-ufs.yaml: Add binding for TI UFS wrapper
  scsi: ufs: Add driver for TI wrapper for Cadence UFS IP

 .../devicetree/bindings/ufs/ti,j721e-ufs.yaml | 68 ++++++++++++++
 drivers/scsi/ufs/Kconfig                      | 10 +++
 drivers/scsi/ufs/Makefile                     |  1 +
 drivers/scsi/ufs/ti-j721e-ufs.c               | 90 +++++++++++++++++++
 4 files changed, 169 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ufs/ti,j721e-ufs.yaml
 create mode 100644 drivers/scsi/ufs/ti-j721e-ufs.c

-- 
2.23.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/2] dt-bindings: ufs: ti,j721e-ufs.yaml: Add binding for TI UFS wrapper
  2019-10-10  8:33 [PATCH v2 0/2] scsi: ufs: Add driver for TI wrapper for Cadence UFS IP Vignesh Raghavendra
@ 2019-10-10  8:33 ` Vignesh Raghavendra
  2019-10-14 18:09   ` Rob Herring
  2019-10-10  8:33 ` [PATCH v2 2/2] scsi: ufs: Add driver for TI wrapper for Cadence UFS IP Vignesh Raghavendra
  1 sibling, 1 reply; 6+ messages in thread
From: Vignesh Raghavendra @ 2019-10-10  8:33 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, jejb, Martin K Petersen
  Cc: Alim Akhtar, Avri Altman, Pedro Sousa, Janek Kotas, devicetree,
	linux-kernel, linux-scsi, Vignesh Raghavendra, nsekhar

Add binding documentation of TI wrapper for Cadence UFS Controller.

Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
---

v2:
Define Cadence UFS controller as child node of the wrapper as suggested
by Rob H

 .../devicetree/bindings/ufs/ti,j721e-ufs.yaml | 68 +++++++++++++++++++
 1 file changed, 68 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ufs/ti,j721e-ufs.yaml

diff --git a/Documentation/devicetree/bindings/ufs/ti,j721e-ufs.yaml b/Documentation/devicetree/bindings/ufs/ti,j721e-ufs.yaml
new file mode 100644
index 000000000000..c8a2a92074df
--- /dev/null
+++ b/Documentation/devicetree/bindings/ufs/ti,j721e-ufs.yaml
@@ -0,0 +1,68 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/ufs/ti,j721e-ufs.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI J721e UFS Host Controller Glue Driver
+
+maintainers:
+  - Vignesh Raghavendra <vigneshr@ti.com>
+
+properties:
+  compatible:
+    items:
+      - const: ti,j721e-ufs
+
+  reg:
+    maxItems: 1
+    description: address of TI UFS glue registers
+
+  clocks:
+    maxItems: 1
+    description: phandle to the M-PHY clock
+
+  power-domains:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - power-domains
+
+patternProperties:
+  "^ufs@[0-9a-f]+$":
+    type: object
+    description: |
+      Cadence UFS controller node must be the child node. Refer
+      Documentation/devicetree/bindings/ufs/cdns,ufshc.txt for binding
+      documentation of child node
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    ufs_wrapper: ufs-wrapper@4e80000 {
+       compatible = "ti,j721e-ufs";
+       reg = <0x0 0x4e80000 0x0 0x100>;
+       power-domains = <&k3_pds 277>;
+       clocks = <&k3_clks 277 1>;
+       assigned-clocks = <&k3_clks 277 1>;
+       assigned-clock-parents = <&k3_clks 277 4>;
+       #address-cells = <2>;
+       #size-cells = <2>;
+
+       ufs@4e84000 {
+          compatible = "cdns,ufshc-m31-16nm", "jedec,ufs-2.0";
+          reg = <0x0 0x4e84000 0x0 0x10000>;
+          interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
+          freq-table-hz = <19200000 19200000>;
+          power-domains = <&k3_pds 277>;
+          clocks = <&k3_clks 277 1>;
+          assigned-clocks = <&k3_clks 277 1>;
+          assigned-clock-parents = <&k3_clks 277 4>;
+          clock-names = "core_clk";
+       };
+    };
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 2/2] scsi: ufs: Add driver for TI wrapper for Cadence UFS IP
  2019-10-10  8:33 [PATCH v2 0/2] scsi: ufs: Add driver for TI wrapper for Cadence UFS IP Vignesh Raghavendra
  2019-10-10  8:33 ` [PATCH v2 1/2] dt-bindings: ufs: ti,j721e-ufs.yaml: Add binding for TI UFS wrapper Vignesh Raghavendra
@ 2019-10-10  8:33 ` Vignesh Raghavendra
  2019-10-15  1:34   ` Alim Akhtar
  1 sibling, 1 reply; 6+ messages in thread
From: Vignesh Raghavendra @ 2019-10-10  8:33 UTC (permalink / raw)
  To: Rob Herring, Mark Rutland, jejb, Martin K Petersen
  Cc: Alim Akhtar, Avri Altman, Pedro Sousa, Janek Kotas, devicetree,
	linux-kernel, linux-scsi, Vignesh Raghavendra, nsekhar

TI's J721e SoC has a Cadence UFS IP with a TI specific wrapper. This is
a minimal driver to configure the wrapper. It releases the UFS slave
device out of reset and sets up registers to indicate PHY reference
clock input frequency before probing child Cadence UFS driver.

Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
---

v2: No change

 drivers/scsi/ufs/Kconfig        | 10 ++++
 drivers/scsi/ufs/Makefile       |  1 +
 drivers/scsi/ufs/ti-j721e-ufs.c | 90 +++++++++++++++++++++++++++++++++
 3 files changed, 101 insertions(+)
 create mode 100644 drivers/scsi/ufs/ti-j721e-ufs.c

diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig
index 0b845ab7c3bf..d14c2243e02a 100644
--- a/drivers/scsi/ufs/Kconfig
+++ b/drivers/scsi/ufs/Kconfig
@@ -132,6 +132,16 @@ config SCSI_UFS_HISI
 	  Select this if you have UFS controller on Hisilicon chipset.
 	  If unsure, say N.
 
+config SCSI_UFS_TI_J721E
+	tristate "TI glue layer for Cadence UFS Controller"
+	depends on OF && HAS_IOMEM && (ARCH_K3 || COMPILE_TEST)
+	help
+	  This selects driver for TI glue layer for Cadence UFS Host
+	  Controller IP.
+
+	  Selects this if you have TI platform with UFS controller.
+	  If unsure, say N.
+
 config SCSI_UFS_BSG
 	bool "Universal Flash Storage BSG device node"
 	depends on SCSI_UFSHCD
diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
index 2a9097939bcb..94c6c5d7334b 100644
--- a/drivers/scsi/ufs/Makefile
+++ b/drivers/scsi/ufs/Makefile
@@ -11,3 +11,4 @@ obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o
 obj-$(CONFIG_SCSI_UFSHCD_PLATFORM) += ufshcd-pltfrm.o
 obj-$(CONFIG_SCSI_UFS_HISI) += ufs-hisi.o
 obj-$(CONFIG_SCSI_UFS_MEDIATEK) += ufs-mediatek.o
+obj-$(CONFIG_SCSI_UFS_TI_J721E) += ti-j721e-ufs.o
diff --git a/drivers/scsi/ufs/ti-j721e-ufs.c b/drivers/scsi/ufs/ti-j721e-ufs.c
new file mode 100644
index 000000000000..a653bf1902f3
--- /dev/null
+++ b/drivers/scsi/ufs/ti-j721e-ufs.c
@@ -0,0 +1,90 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/
+//
+
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+
+#define UFS_SS_CTRL		0x4
+#define UFS_SS_RST_N_PCS	BIT(0)
+#define UFS_SS_CLK_26MHZ	BIT(4)
+
+static int ti_j721e_ufs_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	unsigned long clk_rate;
+	void __iomem *regbase;
+	struct clk *clk;
+	u32 reg = 0;
+	int ret;
+
+	regbase = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(regbase))
+		return PTR_ERR(regbase);
+
+	/* Select MPHY refclk frequency */
+	clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(clk)) {
+		dev_err(dev, "Cannot claim MPHY clock.\n");
+		return PTR_ERR(clk);
+	}
+	clk_rate = clk_get_rate(clk);
+	if (clk_rate == 26000000)
+		reg |= UFS_SS_CLK_26MHZ;
+	devm_clk_put(dev, clk);
+
+	pm_runtime_enable(dev);
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(dev);
+		return ret;
+	}
+
+	/*  Take UFS slave device out of reset */
+	reg |= UFS_SS_RST_N_PCS;
+	writel(reg, regbase + UFS_SS_CTRL);
+
+	ret = of_platform_populate(pdev->dev.of_node, NULL, NULL,
+				   dev);
+	if (ret) {
+		dev_err(dev, "failed to populate child nodes %d\n", ret);
+		pm_runtime_put_sync(dev);
+	}
+
+	return ret;
+}
+
+static int ti_j721e_ufs_remove(struct platform_device *pdev)
+{
+	of_platform_depopulate(&pdev->dev);
+	pm_runtime_put_sync(&pdev->dev);
+
+	return 0;
+}
+
+static const struct of_device_id ti_j721e_ufs_of_match[] = {
+	{
+		.compatible = "ti,j721e-ufs",
+	},
+	{ },
+};
+
+static struct platform_driver ti_j721e_ufs_driver = {
+	.probe	= ti_j721e_ufs_probe,
+	.remove	= ti_j721e_ufs_remove,
+	.driver	= {
+		.name   = "ti-j721e-ufs",
+		.of_match_table = ti_j721e_ufs_of_match,
+	},
+};
+module_platform_driver(ti_j721e_ufs_driver);
+
+MODULE_AUTHOR("Vignesh Raghavendra <vigneshr@ti.com>");
+MODULE_DESCRIPTION("TI UFS host controller glue driver");
+MODULE_LICENSE("GPL v2");
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: ufs: ti,j721e-ufs.yaml: Add binding for TI UFS wrapper
  2019-10-10  8:33 ` [PATCH v2 1/2] dt-bindings: ufs: ti,j721e-ufs.yaml: Add binding for TI UFS wrapper Vignesh Raghavendra
@ 2019-10-14 18:09   ` Rob Herring
  0 siblings, 0 replies; 6+ messages in thread
From: Rob Herring @ 2019-10-14 18:09 UTC (permalink / raw)
  To: Vignesh Raghavendra
  Cc: Mark Rutland, jejb, Martin K Petersen, Alim Akhtar, Avri Altman,
	Pedro Sousa, Janek Kotas, devicetree, linux-kernel, linux-scsi,
	Vignesh Raghavendra, nsekhar

On Thu, 10 Oct 2019 14:03:56 +0530, Vignesh Raghavendra wrote:
> Add binding documentation of TI wrapper for Cadence UFS Controller.
> 
> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
> ---
> 
> v2:
> Define Cadence UFS controller as child node of the wrapper as suggested
> by Rob H
> 
>  .../devicetree/bindings/ufs/ti,j721e-ufs.yaml | 68 +++++++++++++++++++
>  1 file changed, 68 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/ufs/ti,j721e-ufs.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/2] scsi: ufs: Add driver for TI wrapper for Cadence UFS IP
  2019-10-10  8:33 ` [PATCH v2 2/2] scsi: ufs: Add driver for TI wrapper for Cadence UFS IP Vignesh Raghavendra
@ 2019-10-15  1:34   ` Alim Akhtar
  2019-10-15  4:50     ` Vignesh Raghavendra
  0 siblings, 1 reply; 6+ messages in thread
From: Alim Akhtar @ 2019-10-15  1:34 UTC (permalink / raw)
  To: Vignesh Raghavendra
  Cc: Rob Herring, Mark Rutland, James E.J. Bottomley,
	Martin K Petersen, Alim Akhtar, Avri Altman, Pedro Sousa,
	Janek Kotas, devicetree, linux-kernel, linux-scsi, nsekhar

Hi Vignesh

On Thu, Oct 10, 2019 at 2:05 PM Vignesh Raghavendra <vigneshr@ti.com> wrote:
>
> TI's J721e SoC has a Cadence UFS IP with a TI specific wrapper. This is
> a minimal driver to configure the wrapper. It releases the UFS slave
> device out of reset and sets up registers to indicate PHY reference
> clock input frequency before probing child Cadence UFS driver.
>
> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
> ---
>
> v2: No change
>
>  drivers/scsi/ufs/Kconfig        | 10 ++++
>  drivers/scsi/ufs/Makefile       |  1 +
>  drivers/scsi/ufs/ti-j721e-ufs.c | 90 +++++++++++++++++++++++++++++++++
>  3 files changed, 101 insertions(+)
>  create mode 100644 drivers/scsi/ufs/ti-j721e-ufs.c
>
> diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig
> index 0b845ab7c3bf..d14c2243e02a 100644
> --- a/drivers/scsi/ufs/Kconfig
> +++ b/drivers/scsi/ufs/Kconfig
> @@ -132,6 +132,16 @@ config SCSI_UFS_HISI
>           Select this if you have UFS controller on Hisilicon chipset.
>           If unsure, say N.
>
> +config SCSI_UFS_TI_J721E
> +       tristate "TI glue layer for Cadence UFS Controller"
> +       depends on OF && HAS_IOMEM && (ARCH_K3 || COMPILE_TEST)
> +       help
> +         This selects driver for TI glue layer for Cadence UFS Host
> +         Controller IP.
> +
> +         Selects this if you have TI platform with UFS controller.
> +         If unsure, say N.
> +
>  config SCSI_UFS_BSG
>         bool "Universal Flash Storage BSG device node"
>         depends on SCSI_UFSHCD
> diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
> index 2a9097939bcb..94c6c5d7334b 100644
> --- a/drivers/scsi/ufs/Makefile
> +++ b/drivers/scsi/ufs/Makefile
> @@ -11,3 +11,4 @@ obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o
>  obj-$(CONFIG_SCSI_UFSHCD_PLATFORM) += ufshcd-pltfrm.o
>  obj-$(CONFIG_SCSI_UFS_HISI) += ufs-hisi.o
>  obj-$(CONFIG_SCSI_UFS_MEDIATEK) += ufs-mediatek.o
> +obj-$(CONFIG_SCSI_UFS_TI_J721E) += ti-j721e-ufs.o
> diff --git a/drivers/scsi/ufs/ti-j721e-ufs.c b/drivers/scsi/ufs/ti-j721e-ufs.c
> new file mode 100644
> index 000000000000..a653bf1902f3
> --- /dev/null
> +++ b/drivers/scsi/ufs/ti-j721e-ufs.c
> @@ -0,0 +1,90 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/
> +//
> +
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +
> +#define UFS_SS_CTRL            0x4
> +#define UFS_SS_RST_N_PCS       BIT(0)
> +#define UFS_SS_CLK_26MHZ       BIT(4)
> +
These looks like vendor specific defines, if so, please add TI_* suffix.

> +static int ti_j721e_ufs_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       unsigned long clk_rate;
> +       void __iomem *regbase;
> +       struct clk *clk;
> +       u32 reg = 0;
> +       int ret;
> +
> +       regbase = devm_platform_ioremap_resource(pdev, 0);
> +       if (IS_ERR(regbase))
> +               return PTR_ERR(regbase);
> +
> +       /* Select MPHY refclk frequency */
> +       clk = devm_clk_get(dev, NULL);
> +       if (IS_ERR(clk)) {
> +               dev_err(dev, "Cannot claim MPHY clock.\n");
> +               return PTR_ERR(clk);
> +       }
No need to enable MPHY clock? Moreover this clock belongs to MPHY and
should be handled using generic PHY framework to do that.
> +       clk_rate = clk_get_rate(clk);
> +       if (clk_rate == 26000000)
> +               reg |= UFS_SS_CLK_26MHZ;
> +       devm_clk_put(dev, clk);
> +
Is this only needed to select one bit in UFS_SS_CLK_26MHz? if so, just
have a DT property and get this selection from there.

> +       pm_runtime_enable(dev);
> +       ret = pm_runtime_get_sync(dev);
> +       if (ret < 0) {
> +               pm_runtime_put_noidle(dev);
> +               return ret;
> +       }
> +
> +       /*  Take UFS slave device out of reset */
> +       reg |= UFS_SS_RST_N_PCS;
What is the default value of UFS_SS_CLK_26MHZ bit above? Incase 26MHZ
is not set, then what is default?
> +       writel(reg, regbase + UFS_SS_CTRL);
> +
> +       ret = of_platform_populate(pdev->dev.of_node, NULL, NULL,
> +                                  dev);
> +       if (ret) {
> +               dev_err(dev, "failed to populate child nodes %d\n", ret);
> +               pm_runtime_put_sync(dev);
> +       }
> +
> +       return ret;
> +}
> +
> +static int ti_j721e_ufs_remove(struct platform_device *pdev)
> +{
> +       of_platform_depopulate(&pdev->dev);
> +       pm_runtime_put_sync(&pdev->dev);
> +
> +       return 0;
> +}
> +
> +static const struct of_device_id ti_j721e_ufs_of_match[] = {
> +       {
> +               .compatible = "ti,j721e-ufs",
> +       },
> +       { },
> +};
> +
> +static struct platform_driver ti_j721e_ufs_driver = {
> +       .probe  = ti_j721e_ufs_probe,
> +       .remove = ti_j721e_ufs_remove,
> +       .driver = {
> +               .name   = "ti-j721e-ufs",
> +               .of_match_table = ti_j721e_ufs_of_match,
> +       },
> +};
> +module_platform_driver(ti_j721e_ufs_driver);
> +
> +MODULE_AUTHOR("Vignesh Raghavendra <vigneshr@ti.com>");
> +MODULE_DESCRIPTION("TI UFS host controller glue driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.23.0
>


-- 
Regards,
Alim

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/2] scsi: ufs: Add driver for TI wrapper for Cadence UFS IP
  2019-10-15  1:34   ` Alim Akhtar
@ 2019-10-15  4:50     ` Vignesh Raghavendra
  0 siblings, 0 replies; 6+ messages in thread
From: Vignesh Raghavendra @ 2019-10-15  4:50 UTC (permalink / raw)
  To: Alim Akhtar
  Cc: Rob Herring, Mark Rutland, James E.J. Bottomley,
	Martin K Petersen, Alim Akhtar, Avri Altman, Pedro Sousa,
	Janek Kotas, devicetree, linux-kernel, linux-scsi, nsekhar

Hi Alim,

On 15/10/19 7:04 AM, Alim Akhtar wrote:
> Hi Vignesh
> 
> On Thu, Oct 10, 2019 at 2:05 PM Vignesh Raghavendra <vigneshr@ti.com> wrote:
>>
>> TI's J721e SoC has a Cadence UFS IP with a TI specific wrapper. This is
>> a minimal driver to configure the wrapper. It releases the UFS slave
>> device out of reset and sets up registers to indicate PHY reference
>> clock input frequency before probing child Cadence UFS driver.
>>
>> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
>> ---
>>
>> v2: No change
>>
>>  drivers/scsi/ufs/Kconfig        | 10 ++++
>>  drivers/scsi/ufs/Makefile       |  1 +
>>  drivers/scsi/ufs/ti-j721e-ufs.c | 90 +++++++++++++++++++++++++++++++++
>>  3 files changed, 101 insertions(+)
>>  create mode 100644 drivers/scsi/ufs/ti-j721e-ufs.c
>>
>> diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig
>> index 0b845ab7c3bf..d14c2243e02a 100644
>> --- a/drivers/scsi/ufs/Kconfig
>> +++ b/drivers/scsi/ufs/Kconfig
>> @@ -132,6 +132,16 @@ config SCSI_UFS_HISI
>>           Select this if you have UFS controller on Hisilicon chipset.
>>           If unsure, say N.
>>
>> +config SCSI_UFS_TI_J721E
>> +       tristate "TI glue layer for Cadence UFS Controller"
>> +       depends on OF && HAS_IOMEM && (ARCH_K3 || COMPILE_TEST)
>> +       help
>> +         This selects driver for TI glue layer for Cadence UFS Host
>> +         Controller IP.
>> +
>> +         Selects this if you have TI platform with UFS controller.
>> +         If unsure, say N.
>> +
>>  config SCSI_UFS_BSG
>>         bool "Universal Flash Storage BSG device node"
>>         depends on SCSI_UFSHCD
>> diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile
>> index 2a9097939bcb..94c6c5d7334b 100644
>> --- a/drivers/scsi/ufs/Makefile
>> +++ b/drivers/scsi/ufs/Makefile
>> @@ -11,3 +11,4 @@ obj-$(CONFIG_SCSI_UFSHCD_PCI) += ufshcd-pci.o
>>  obj-$(CONFIG_SCSI_UFSHCD_PLATFORM) += ufshcd-pltfrm.o
>>  obj-$(CONFIG_SCSI_UFS_HISI) += ufs-hisi.o
>>  obj-$(CONFIG_SCSI_UFS_MEDIATEK) += ufs-mediatek.o
>> +obj-$(CONFIG_SCSI_UFS_TI_J721E) += ti-j721e-ufs.o
>> diff --git a/drivers/scsi/ufs/ti-j721e-ufs.c b/drivers/scsi/ufs/ti-j721e-ufs.c
>> new file mode 100644
>> index 000000000000..a653bf1902f3
>> --- /dev/null
>> +++ b/drivers/scsi/ufs/ti-j721e-ufs.c
>> @@ -0,0 +1,90 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +//
>> +// Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/
>> +//
>> +
>> +#include <linux/clk.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/pm_runtime.h>
>> +
>> +#define UFS_SS_CTRL            0x4
>> +#define UFS_SS_RST_N_PCS       BIT(0)
>> +#define UFS_SS_CLK_26MHZ       BIT(4)
>> +
> These looks like vendor specific defines, if so, please add TI_* suffix.
> 

OK, will fix this in v2

>> +static int ti_j721e_ufs_probe(struct platform_device *pdev)
>> +{
>> +       struct device *dev = &pdev->dev;
>> +       unsigned long clk_rate;
>> +       void __iomem *regbase;
>> +       struct clk *clk;
>> +       u32 reg = 0;
>> +       int ret;
>> +
>> +       regbase = devm_platform_ioremap_resource(pdev, 0);
>> +       if (IS_ERR(regbase))
>> +               return PTR_ERR(regbase);
>> +
>> +       /* Select MPHY refclk frequency */
>> +       clk = devm_clk_get(dev, NULL);
>> +       if (IS_ERR(clk)) {
>> +               dev_err(dev, "Cannot claim MPHY clock.\n");
>> +               return PTR_ERR(clk);
>> +       }
> No need to enable MPHY clock? Moreover this clock belongs to MPHY and
> should be handled using generic PHY framework to do that.

pm_runtime_get_sync() call below will make sure all required clocks of
the module are enabled and also Cadence UFS controller/UFSHCD will
enable clocks explicitly. But what is needed here is to setup up wrapper
bit that informs MPHY module what is the frequency of its input clock
(whether its 19.2 MHz or 26MHz). Also this bit is not part of MPHY
address space so it cannot be modeled as PHY driver.

>> +       clk_rate = clk_get_rate(clk);
>> +       if (clk_rate == 26000000)
>> +               reg |= UFS_SS_CLK_26MHZ;
>> +       devm_clk_put(dev, clk);
>> +
> Is this only needed to select one bit in UFS_SS_CLK_26MHz? if so, just
> have a DT property and get this selection from there.
> 

Yes its a single bit. But I don't think DT property is right way to do
especially when bit can be configured at runtime by querying clock
frequency using clk APIs.
In past I have received feedback from DT folks, to have DT describe only
generic properties (such as reg, interrupts, clocks etc) and handle most
other things in driver whenever possible.

>> +       pm_runtime_enable(dev);
>> +       ret = pm_runtime_get_sync(dev);
>> +       if (ret < 0) {
>> +               pm_runtime_put_noidle(dev);
>> +               return ret;
>> +       }
>> +
>> +       /*  Take UFS slave device out of reset */
>> +       reg |= UFS_SS_RST_N_PCS;
> What is the default value of UFS_SS_CLK_26MHZ bit above? Incase 26MHZ
> is not set, then what is default?

Default is of this bit is 0 => 19.2MHz (0 => 192.MHz and 1 => 26MHz)

Let me know if this addresses your comments about UFS_SS_CLK_26MHZ bit
or if any change is needed?
Thanks for the review!

Regard
Vignesh

[...]


-- 
Regards
Vignesh

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-10-15  4:50 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-10  8:33 [PATCH v2 0/2] scsi: ufs: Add driver for TI wrapper for Cadence UFS IP Vignesh Raghavendra
2019-10-10  8:33 ` [PATCH v2 1/2] dt-bindings: ufs: ti,j721e-ufs.yaml: Add binding for TI UFS wrapper Vignesh Raghavendra
2019-10-14 18:09   ` Rob Herring
2019-10-10  8:33 ` [PATCH v2 2/2] scsi: ufs: Add driver for TI wrapper for Cadence UFS IP Vignesh Raghavendra
2019-10-15  1:34   ` Alim Akhtar
2019-10-15  4:50     ` Vignesh Raghavendra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).